diff --git a/builtin/providers/google/resource_compute_instance.go b/builtin/providers/google/resource_compute_instance.go index af3e2910a..60762a7c7 100644 --- a/builtin/providers/google/resource_compute_instance.go +++ b/builtin/providers/google/resource_compute_instance.go @@ -75,20 +75,61 @@ func resourceComputeInstance() *schema.Resource { }, }, + "network_interface": &schema.Schema{ + Type: schema.TypeList, + Optional: true, + ForceNew: true, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "network": &schema.Schema{ + Type: schema.TypeString, + Required: true, + ForceNew: true, + }, + + "name": &schema.Schema{ + Type: schema.TypeString, + Computed: true, + }, + + "address": &schema.Schema{ + Type: schema.TypeString, + Computed: true, + }, + + "access_config": &schema.Schema{ + Type: schema.TypeList, + Optional: true, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "nat_ip": &schema.Schema{ + Type: schema.TypeString, + Computed: true, + Optional: true, + }, + }, + }, + }, + }, + }, + }, + "network": &schema.Schema{ Type: schema.TypeList, - Required: true, + Optional: true, ForceNew: true, Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ "source": &schema.Schema{ Type: schema.TypeString, Required: true, + ForceNew: true, }, "address": &schema.Schema{ Type: schema.TypeString, Optional: true, + ForceNew: true, }, "name": &schema.Schema{ @@ -178,6 +219,33 @@ func resourceComputeInstance() *schema.Resource { } } +func resourceOperationWaitZone( + config *Config, op *compute.Operation, zone string, activity string) error { + + w := &OperationWaiter{ + Service: config.clientCompute, + Op: op, + Project: config.Project, + Zone: zone, + Type: OperationWaitZone, + } + state := w.Conf() + state.Delay = 10 * time.Second + state.Timeout = 10 * time.Minute + state.MinTimeout = 2 * time.Second + opRaw, err := state.WaitForState() + if err != nil { + return fmt.Errorf("Error waiting for %s: %s", activity, err) + } + op = opRaw.(*compute.Operation) + if op.Error != nil { + // Return the error + return OperationError(*op.Error) + } + return nil +} + + func resourceComputeInstanceCreate(d *schema.ResourceData, meta interface{}) error { config := meta.(*Config) @@ -263,32 +331,80 @@ func resourceComputeInstanceCreate(d *schema.ResourceData, meta interface{}) err disks = append(disks, &disk) } - // Build up the list of networks networksCount := d.Get("network.#").(int) - networks := make([]*compute.NetworkInterface, 0, networksCount) - for i := 0; i < networksCount; i++ { - prefix := fmt.Sprintf("network.%d", i) - // Load up the name of this network - networkName := d.Get(prefix + ".source").(string) - network, err := config.clientCompute.Networks.Get( - config.Project, networkName).Do() - if err != nil { - return fmt.Errorf( - "Error loading network '%s': %s", - networkName, err) - } + networkInterfacesCount := d.Get("network_interface.#").(int) - // Build the disk - var iface compute.NetworkInterface - iface.AccessConfigs = []*compute.AccessConfig{ - &compute.AccessConfig{ - Type: "ONE_TO_ONE_NAT", - NatIP: d.Get(prefix + ".address").(string), - }, - } - iface.Network = network.SelfLink + if networksCount > 0 && networkInterfacesCount > 0 { + return fmt.Errorf("Error: cannot define both networks and network_interfaces.") + } + if networksCount == 0 && networkInterfacesCount == 0 { + return fmt.Errorf("Error: Must define at least one network_interface.") + } - networks = append(networks, &iface) + var networkInterfaces []*compute.NetworkInterface + + if networksCount > 0 { + // TODO: Delete this block when removing network { } + // Build up the list of networkInterfaces + networkInterfaces = make([]*compute.NetworkInterface, 0, networksCount) + for i := 0; i < networksCount; i++ { + prefix := fmt.Sprintf("network.%d", i) + // Load up the name of this network + networkName := d.Get(prefix + ".source").(string) + network, err := config.clientCompute.Networks.Get( + config.Project, networkName).Do() + if err != nil { + return fmt.Errorf( + "Error loading network '%s': %s", + networkName, err) + } + + // Build the networkInterface + var iface compute.NetworkInterface + iface.AccessConfigs = []*compute.AccessConfig{ + &compute.AccessConfig{ + Type: "ONE_TO_ONE_NAT", + NatIP: d.Get(prefix + ".address").(string), + }, + } + iface.Network = network.SelfLink + + networkInterfaces = append(networkInterfaces, &iface) + } + } + + if networkInterfacesCount > 0 { + // Build up the list of networkInterfaces + networkInterfaces = make([]*compute.NetworkInterface, 0, networkInterfacesCount) + for i := 0; i < networkInterfacesCount; i++ { + prefix := fmt.Sprintf("network_interface.%d", i) + // Load up the name of this network_interfac + networkName := d.Get(prefix + ".network").(string) + network, err := config.clientCompute.Networks.Get( + config.Project, networkName).Do() + if err != nil { + return fmt.Errorf( + "Error referencing network '%s': %s", + networkName, err) + } + + // Build the networkInterface + var iface compute.NetworkInterface + iface.Network = network.SelfLink + + // Handle access_config structs + accessConfigsCount := d.Get(prefix + ".access_config.#").(int) + iface.AccessConfigs = make([]*compute.AccessConfig, accessConfigsCount) + for j := 0; j < accessConfigsCount; j++ { + acPrefix := fmt.Sprintf("%s.access_config.%d", prefix, j) + iface.AccessConfigs[j] = &compute.AccessConfig{ + Type: "ONE_TO_ONE_NAT", + NatIP: d.Get(acPrefix + ".nat_ip").(string), + } + } + + networkInterfaces = append(networkInterfaces, &iface) + } } serviceAccountsCount := d.Get("service_account.#").(int) @@ -319,7 +435,7 @@ func resourceComputeInstanceCreate(d *schema.ResourceData, meta interface{}) err MachineType: machineType.SelfLink, Metadata: resourceInstanceMetadata(d), Name: d.Get("name").(string), - NetworkInterfaces: networks, + NetworkInterfaces: networkInterfaces, Tags: resourceInstanceTags(d), ServiceAccounts: serviceAccounts, } @@ -335,28 +451,11 @@ func resourceComputeInstanceCreate(d *schema.ResourceData, meta interface{}) err d.SetId(instance.Name) // Wait for the operation to complete - w := &OperationWaiter{ - Service: config.clientCompute, - Op: op, - Project: config.Project, - Zone: zone.Name, - Type: OperationWaitZone, - } - state := w.Conf() - state.Delay = 10 * time.Second - state.Timeout = 10 * time.Minute - state.MinTimeout = 2 * time.Second - opRaw, err := state.WaitForState() - if err != nil { - return fmt.Errorf("Error waiting for instance to create: %s", err) - } - op = opRaw.(*compute.Operation) - if op.Error != nil { + waitErr := resourceOperationWaitZone(config, op, zone.Name, "instance to create") + if waitErr != nil { // The resource didn't actually create d.SetId("") - - // Return the error - return OperationError(*op.Error) + return waitErr } return resourceComputeInstanceRead(d, meta) @@ -390,26 +489,85 @@ func resourceComputeInstanceRead(d *schema.ResourceData, meta interface{}) error } } + networksCount := d.Get("network.#").(int) + networkInterfacesCount := d.Get("network_interface.#").(int) + + if networksCount > 0 && networkInterfacesCount > 0 { + return fmt.Errorf("Error: cannot define both networks and network_interfaces.") + } + if networksCount == 0 && networkInterfacesCount == 0 { + return fmt.Errorf("Error: Must define at least one network_interface.") + } + // Set the networks + // Use the first external IP found for the default connection info. externalIP := "" - for i, iface := range instance.NetworkInterfaces { - prefix := fmt.Sprintf("network.%d", i) - d.Set(prefix+".name", iface.Name) + internalIP := "" + if networksCount > 0 { + // TODO: Remove this when realizing deprecation of .network + for i, iface := range instance.NetworkInterfaces { + prefix := fmt.Sprintf("network.%d", i) + d.Set(prefix+".name", iface.Name) + log.Printf(prefix+".name = %s", iface.Name) - // Use the first external IP found for the default connection info. - natIP := resourceInstanceNatIP(iface) - if externalIP == "" && natIP != "" { - externalIP = natIP + var natIP string + for _, config := range iface.AccessConfigs { + if config.Type == "ONE_TO_ONE_NAT" { + natIP = config.NatIP + break + } + } + + if externalIP == "" && natIP != "" { + externalIP = natIP + } + d.Set(prefix+".external_address", natIP) + + d.Set(prefix+".internal_address", iface.NetworkIP) } - d.Set(prefix+".external_address", natIP) + } - d.Set(prefix+".internal_address", iface.NetworkIP) + if networkInterfacesCount > 0 { + for i, iface := range instance.NetworkInterfaces { + + prefix := fmt.Sprintf("network_interface.%d", i) + d.Set(prefix+".name", iface.Name) + + // The first non-empty ip is left in natIP + var natIP string + for j, config := range iface.AccessConfigs { + acPrefix := fmt.Sprintf("%s.access_config.%d", prefix, j) + d.Set(acPrefix+".nat_ip", config.NatIP) + if natIP == "" { + natIP = config.NatIP + } + } + + if externalIP == "" { + externalIP = natIP + } + + d.Set(prefix+".address", iface.NetworkIP) + if internalIP == "" { + internalIP = iface.NetworkIP + } + + + } + } + + // Fall back on internal ip if there is no external ip. This makes sense in the situation where + // terraform is being used on a cloud instance and can therefore access the instances it creates + // via their internal ips. + sshIP := externalIP + if sshIP == "" { + sshIP = internalIP } // Initialize the connection info d.SetConnInfo(map[string]string{ "type": "ssh", - "host": externalIP, + "host": sshIP, }) // Set the metadata fingerprint if there is one. @@ -430,6 +588,21 @@ func resourceComputeInstanceRead(d *schema.ResourceData, meta interface{}) error func resourceComputeInstanceUpdate(d *schema.ResourceData, meta interface{}) error { config := meta.(*Config) + zone := d.Get("zone").(string) + + instance, err := config.clientCompute.Instances.Get( + config.Project, zone, d.Id()).Do() + if err != nil { + if gerr, ok := err.(*googleapi.Error); ok && gerr.Code == 404 { + // The resource doesn't exist anymore + d.SetId("") + + return nil + } + + return fmt.Errorf("Error reading instance: %s", err) + } + // Enable partial mode for the resource since it is possible d.Partial(true) @@ -437,30 +610,15 @@ func resourceComputeInstanceUpdate(d *schema.ResourceData, meta interface{}) err if d.HasChange("metadata") { metadata := resourceInstanceMetadata(d) op, err := config.clientCompute.Instances.SetMetadata( - config.Project, d.Get("zone").(string), d.Id(), metadata).Do() + config.Project, zone, d.Id(), metadata).Do() if err != nil { return fmt.Errorf("Error updating metadata: %s", err) } - w := &OperationWaiter{ - Service: config.clientCompute, - Op: op, - Project: config.Project, - Zone: d.Get("zone").(string), - Type: OperationWaitZone, - } - state := w.Conf() - state.Delay = 1 * time.Second - state.Timeout = 5 * time.Minute - state.MinTimeout = 2 * time.Second - opRaw, err := state.WaitForState() - if err != nil { - return fmt.Errorf("Error waiting for metadata to update: %s", err) - } - op = opRaw.(*compute.Operation) - if op.Error != nil { - // Return the error - return OperationError(*op.Error) + // 1 5 2 + opErr := resourceOperationWaitZone(config, op, zone, "metadata to update") + if opErr != nil { + return opErr } d.SetPartial("metadata") @@ -469,35 +627,80 @@ func resourceComputeInstanceUpdate(d *schema.ResourceData, meta interface{}) err if d.HasChange("tags") { tags := resourceInstanceTags(d) op, err := config.clientCompute.Instances.SetTags( - config.Project, d.Get("zone").(string), d.Id(), tags).Do() + config.Project, zone, d.Id(), tags).Do() if err != nil { return fmt.Errorf("Error updating tags: %s", err) } - w := &OperationWaiter{ - Service: config.clientCompute, - Op: op, - Project: config.Project, - Zone: d.Get("zone").(string), - Type: OperationWaitZone, - } - state := w.Conf() - state.Delay = 1 * time.Second - state.Timeout = 5 * time.Minute - state.MinTimeout = 2 * time.Second - opRaw, err := state.WaitForState() - if err != nil { - return fmt.Errorf("Error waiting for tags to update: %s", err) - } - op = opRaw.(*compute.Operation) - if op.Error != nil { - // Return the error - return OperationError(*op.Error) + opErr := resourceOperationWaitZone(config, op, zone, "tags to update") + if opErr != nil { + return opErr } d.SetPartial("tags") } + networkInterfacesCount := d.Get("network_interface.#").(int) + if networkInterfacesCount > 0 { + // Sanity check + if networkInterfacesCount != len(instance.NetworkInterfaces) { + return fmt.Errorf("Instance had unexpected number of network interfaces: %d", len(instance.NetworkInterfaces)) + } + for i := 0; i < networkInterfacesCount; i++ { + prefix := fmt.Sprintf("network_interface.%d", i) + instNetworkInterface := instance.NetworkInterfaces[i] + networkName := d.Get(prefix+".name").(string) + + // TODO: This sanity check is broken by #929, disabled for now (by forcing the equality) + networkName = instNetworkInterface.Name + // Sanity check + if networkName != instNetworkInterface.Name { + return fmt.Errorf("Instance networkInterface had unexpected name: %s", instNetworkInterface.Name) + } + + if d.HasChange(prefix+".access_config") { + + // TODO: This code deletes then recreates accessConfigs. This is bad because it may + // leave the machine inaccessible from either ip if the creation part fails (network + // timeout etc). However right now there is a GCE limit of 1 accessConfig so it is + // the only way to do it. In future this should be revised to only change what is + // necessary, and also add before removing. + + // Delete any accessConfig that currently exists in instNetworkInterface + for _, ac := range(instNetworkInterface.AccessConfigs) { + op, err := config.clientCompute.Instances.DeleteAccessConfig( + config.Project, zone, d.Id(), networkName, ac.Name).Do(); + if err != nil { + return fmt.Errorf("Error deleting old access_config: %s", err) + } + opErr := resourceOperationWaitZone(config, op, zone, "old access_config to delete") + if opErr != nil { + return opErr + } + } + + // Create new ones + accessConfigsCount := d.Get(prefix + ".access_config.#").(int) + for j := 0; j < accessConfigsCount; j++ { + acPrefix := fmt.Sprintf("%s.access_config.%d", prefix, j) + ac := &compute.AccessConfig{ + Type: "ONE_TO_ONE_NAT", + NatIP: d.Get(acPrefix + ".nat_ip").(string), + } + op, err := config.clientCompute.Instances.AddAccessConfig( + config.Project, zone, d.Id(), networkName, ac).Do(); + if err != nil { + return fmt.Errorf("Error adding new access_config: %s", err) + } + opErr := resourceOperationWaitZone(config, op, zone, "new access_config to add") + if opErr != nil { + return opErr + } + } + } + } + } + // We made it, disable partial mode d.Partial(false) @@ -507,32 +710,16 @@ func resourceComputeInstanceUpdate(d *schema.ResourceData, meta interface{}) err func resourceComputeInstanceDelete(d *schema.ResourceData, meta interface{}) error { config := meta.(*Config) - op, err := config.clientCompute.Instances.Delete( - config.Project, d.Get("zone").(string), d.Id()).Do() + zone := d.Get("zone").(string) + op, err := config.clientCompute.Instances.Delete(config.Project, zone, d.Id()).Do() if err != nil { return fmt.Errorf("Error deleting instance: %s", err) } // Wait for the operation to complete - w := &OperationWaiter{ - Service: config.clientCompute, - Op: op, - Project: config.Project, - Zone: d.Get("zone").(string), - Type: OperationWaitZone, - } - state := w.Conf() - state.Delay = 5 * time.Second - state.Timeout = 5 * time.Minute - state.MinTimeout = 2 * time.Second - opRaw, err := state.WaitForState() - if err != nil { - return fmt.Errorf("Error waiting for instance to delete: %s", err) - } - op = opRaw.(*compute.Operation) - if op.Error != nil { - // Return the error - return OperationError(*op.Error) + opErr := resourceOperationWaitZone(config, op, zone, "instance to delete") + if opErr != nil { + return opErr } d.SetId("") @@ -584,16 +771,3 @@ func resourceInstanceTags(d *schema.ResourceData) *compute.Tags { return tags } - -// resourceInstanceNatIP acquires the first NatIP with a "ONE_TO_ONE_NAT" type -// in the compute.NetworkInterface's AccessConfigs. -func resourceInstanceNatIP(iface *compute.NetworkInterface) (natIP string) { - for _, config := range iface.AccessConfigs { - if config.Type == "ONE_TO_ONE_NAT" { - natIP = config.NatIP - break - } - } - - return natIP -} diff --git a/builtin/providers/google/resource_compute_instance_test.go b/builtin/providers/google/resource_compute_instance_test.go index f765a44c4..226406665 100644 --- a/builtin/providers/google/resource_compute_instance_test.go +++ b/builtin/providers/google/resource_compute_instance_test.go @@ -10,6 +10,28 @@ import ( "github.com/hashicorp/terraform/terraform" ) +func TestAccComputeInstance_basic_deprecated_network(t *testing.T) { + var instance compute.Instance + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckComputeInstanceDestroy, + Steps: []resource.TestStep{ + resource.TestStep{ + Config: testAccComputeInstance_basic_deprecated_network, + Check: resource.ComposeTestCheckFunc( + testAccCheckComputeInstanceExists( + "google_compute_instance.foobar", &instance), + testAccCheckComputeInstanceTag(&instance, "foo"), + testAccCheckComputeInstanceMetadata(&instance, "foo", "bar"), + testAccCheckComputeInstanceDisk(&instance, "terraform-test", true, true), + ), + }, + }, + }) +} + func TestAccComputeInstance_basic(t *testing.T) { var instance compute.Instance @@ -45,7 +67,7 @@ func TestAccComputeInstance_IP(t *testing.T) { Check: resource.ComposeTestCheckFunc( testAccCheckComputeInstanceExists( "google_compute_instance.foobar", &instance), - testAccCheckComputeInstanceNetwork(&instance), + testAccCheckComputeInstanceAccessConfigHasIP(&instance), ), }, }, @@ -73,6 +95,35 @@ func TestAccComputeInstance_disks(t *testing.T) { }) } +func TestAccComputeInstance_update_deprecated_network(t *testing.T) { + var instance compute.Instance + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckComputeInstanceDestroy, + Steps: []resource.TestStep{ + resource.TestStep{ + Config: testAccComputeInstance_basic_deprecated_network, + Check: resource.ComposeTestCheckFunc( + testAccCheckComputeInstanceExists( + "google_compute_instance.foobar", &instance), + ), + }, + resource.TestStep{ + Config: testAccComputeInstance_update_deprecated_network, + Check: resource.ComposeTestCheckFunc( + testAccCheckComputeInstanceExists( + "google_compute_instance.foobar", &instance), + testAccCheckComputeInstanceMetadata( + &instance, "bar", "baz"), + testAccCheckComputeInstanceTag(&instance, "baz"), + ), + }, + }, + }) +} + func TestAccComputeInstance_update(t *testing.T) { var instance compute.Instance @@ -96,6 +147,7 @@ func TestAccComputeInstance_update(t *testing.T) { testAccCheckComputeInstanceMetadata( &instance, "bar", "baz"), testAccCheckComputeInstanceTag(&instance, "baz"), + testAccCheckComputeInstanceAccessConfig(&instance), ), }, }, @@ -173,7 +225,19 @@ func testAccCheckComputeInstanceMetadata( } } -func testAccCheckComputeInstanceNetwork(instance *compute.Instance) resource.TestCheckFunc { +func testAccCheckComputeInstanceAccessConfig(instance *compute.Instance) resource.TestCheckFunc { + return func(s *terraform.State) error { + for _, i := range instance.NetworkInterfaces { + if len(i.AccessConfigs) == 0 { + return fmt.Errorf("no access_config") + } + } + + return nil + } +} + +func testAccCheckComputeInstanceAccessConfigHasIP(instance *compute.Instance) resource.TestCheckFunc { return func(s *terraform.State) error { for _, i := range instance.NetworkInterfaces { for _, c := range i.AccessConfigs { @@ -219,7 +283,7 @@ func testAccCheckComputeInstanceTag(instance *compute.Instance, n string) resour } } -const testAccComputeInstance_basic = ` +const testAccComputeInstance_basic_deprecated_network = ` resource "google_compute_instance" "foobar" { name = "terraform-test" machine_type = "n1-standard-1" @@ -240,7 +304,7 @@ resource "google_compute_instance" "foobar" { } }` -const testAccComputeInstance_update = ` +const testAccComputeInstance_update_deprecated_network = ` resource "google_compute_instance" "foobar" { name = "terraform-test" machine_type = "n1-standard-1" @@ -260,6 +324,49 @@ resource "google_compute_instance" "foobar" { } }` +const testAccComputeInstance_basic = ` +resource "google_compute_instance" "foobar" { + name = "terraform-test" + machine_type = "n1-standard-1" + zone = "us-central1-a" + can_ip_forward = false + tags = ["foo", "bar"] + + disk { + image = "debian-7-wheezy-v20140814" + } + + network_interface { + network = "default" + } + + metadata { + foo = "bar" + } +}` + +// Update metadata, tags, and network_interface +const testAccComputeInstance_update = ` +resource "google_compute_instance" "foobar" { + name = "terraform-test" + machine_type = "n1-standard-1" + zone = "us-central1-a" + tags = ["baz"] + + disk { + image = "debian-7-wheezy-v20140814" + } + + network_interface { + network = "default" + access_config { } + } + + metadata { + bar = "baz" + } +}` + const testAccComputeInstance_ip = ` resource "google_compute_address" "foo" { name = "foo" @@ -275,9 +382,11 @@ resource "google_compute_instance" "foobar" { image = "debian-7-wheezy-v20140814" } - network { - source = "default" - address = "${google_compute_address.foo.address}" + network_interface { + network = "default" + access_config { + nat_ip = "${google_compute_address.foo.address}" + } } metadata { @@ -307,8 +416,8 @@ resource "google_compute_instance" "foobar" { auto_delete = false } - network { - source = "default" + network_interface { + network = "default" } metadata { diff --git a/website/source/docs/providers/google/r/compute_instance.html.markdown b/website/source/docs/providers/google/r/compute_instance.html.markdown index b2ea6baf7..91eb48102 100644 --- a/website/source/docs/providers/google/r/compute_instance.html.markdown +++ b/website/source/docs/providers/google/r/compute_instance.html.markdown @@ -27,8 +27,11 @@ resource "google_compute_instance" "default" { image = "debian-7-wheezy-v20140814" } - network { - source = "default" + network_interface { + network = "default" + access_config { + // Ephemeral IP + } } metadata { @@ -64,7 +67,11 @@ The following arguments are supported: * `metadata` - (Optional) Metadata key/value pairs to make available from within the instance. -* `network` - (Required) Networks to attach to the instance. This can be +* `network_interface` - (Required) Networks to attach to the instance. This can be + specified multiple times for multiple networks. Structure is documented + below. + +* `network` - (DEPRECATED, Required) Networks to attach to the instance. This can be specified multiple times for multiple networks. Structure is documented below. @@ -85,7 +92,22 @@ The `disk` block supports: * `type` - (Optional) The GCE disk type. -The `network` block supports: +The `network_interface` block supports: + +* `network` - (Required) The name of the network to attach this interface to. + +* `access_config` - (Optional) Access configurations, i.e. IPs via which this instance can be + accessed via the Internet. Omit to ensure that the instance is not accessible from the Internet +(this means that ssh provisioners will not work unless you are running Terraform can send traffic to +the instance's network (e.g. via tunnel or because it is running on another cloud instance on that +network). This block can be repeated multiple times. Structure documented below. + +The `access_config` block supports: + +* `nat_ip` - (Optional) The IP address that will be 1:1 mapped to the instance's network ip. If not + given, one will be generated. + +(DEPRECATED) The `network` block supports: * `source` - (Required) The name of the network to attach this interface to.