From 44bf0bec96f8c60fff156401cf0463b4b2f7feb4 Mon Sep 17 00:00:00 2001 From: Paddy Date: Mon, 9 Jan 2017 15:15:50 -0800 Subject: [PATCH 1/2] Read update_strategy before overwriting it. (#11013) As brought up in #10174, our update_strategy property for instance group managers in GCP would always be set to "RESTART" on read, even if the user asked for them to be "NONE" in the config. This adds a test to ensure that the user wishes were respected, which fails until we check for update_strategy in the ResourceData before we update it within the Read function. Because the update_strategy property doesn't map to anything in the API, we never need to read it from anywhere but the config, which means the ResourceData should be considered authoritative by the time we get to the Read function. The fix for this was provided by @JDiPierro in #10198 originally, but was missing tests, so it got squashed into this. --- resource_compute_instance_group_manager.go | 6 +- ...rce_compute_instance_group_manager_test.go | 83 +++++++++++++++++++ 2 files changed, 88 insertions(+), 1 deletion(-) diff --git a/resource_compute_instance_group_manager.go b/resource_compute_instance_group_manager.go index ff39f023..89bff60d 100644 --- a/resource_compute_instance_group_manager.go +++ b/resource_compute_instance_group_manager.go @@ -242,7 +242,11 @@ func resourceComputeInstanceGroupManagerRead(d *schema.ResourceData, meta interf d.Set("instance_group", manager.InstanceGroup) d.Set("target_size", manager.TargetSize) d.Set("self_link", manager.SelfLink) - d.Set("update_strategy", "RESTART") //this field doesn't match the manager api, set to default value + update_strategy, ok := d.GetOk("update_strategy") + if !ok { + update_strategy = "RESTART" + } + d.Set("update_strategy", update_strategy.(string)) return nil } diff --git a/resource_compute_instance_group_manager_test.go b/resource_compute_instance_group_manager_test.go index 16e370b0..a16646db 100644 --- a/resource_compute_instance_group_manager_test.go +++ b/resource_compute_instance_group_manager_test.go @@ -112,6 +112,29 @@ func TestAccInstanceGroupManager_updateLifecycle(t *testing.T) { }, }) } + +func TestAccInstanceGroupManager_updateStrategy(t *testing.T) { + var manager compute.InstanceGroupManager + igm := fmt.Sprintf("igm-test-%s", acctest.RandString(10)) + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckInstanceGroupManagerDestroy, + Steps: []resource.TestStep{ + resource.TestStep{ + Config: testAccInstanceGroupManager_updateStrategy(igm), + Check: resource.ComposeTestCheckFunc( + testAccCheckInstanceGroupManagerExists( + "google_compute_instance_group_manager.igm-update-strategy", &manager), + testAccCheckInstanceGroupManagerUpdateStrategy( + "google_compute_instance_group_manager.igm-update-strategy", "NONE"), + ), + }, + }, + }) +} + func testAccCheckInstanceGroupManagerDestroy(s *terraform.State) error { config := testAccProvider.Meta().(*Config) @@ -268,6 +291,25 @@ func testAccCheckInstanceGroupManagerTemplateTags(n string, tags []string) resou } } +func testAccCheckInstanceGroupManagerUpdateStrategy(n, strategy string) resource.TestCheckFunc { + return func(s *terraform.State) error { + rs, ok := s.RootModule().Resources[n] + if !ok { + return fmt.Errorf("Not found: %s", n) + } + + if rs.Primary.ID == "" { + return fmt.Errorf("No ID is set") + } + + if rs.Primary.Attributes["update_strategy"] != strategy { + return fmt.Errorf("Expected strategy to be %s, got %s", + strategy, rs.Primary.Attributes["update_strategy"]) + } + return nil + } +} + func testAccInstanceGroupManager_basic(template, target, igm1, igm2 string) string { return fmt.Sprintf(` resource "google_compute_instance_template" "igm-basic" { @@ -488,6 +530,47 @@ func testAccInstanceGroupManager_updateLifecycle(tag, igm string) string { }`, tag, igm) } +func testAccInstanceGroupManager_updateStrategy(igm string) string { + return fmt.Sprintf(` + resource "google_compute_instance_template" "igm-update-strategy" { + machine_type = "n1-standard-1" + can_ip_forward = false + tags = ["terraform-testing"] + + disk { + source_image = "debian-cloud/debian-8-jessie-v20160803" + auto_delete = true + boot = true + } + + network_interface { + network = "default" + } + + service_account { + scopes = ["userinfo-email", "compute-ro", "storage-ro"] + } + + lifecycle { + create_before_destroy = true + } + } + + resource "google_compute_instance_group_manager" "igm-update-strategy" { + description = "Terraform test instance group manager" + name = "%s" + instance_template = "${google_compute_instance_template.igm-update-strategy.self_link}" + base_instance_name = "igm-update-strategy" + zone = "us-central1-c" + target_size = 2 + update_strategy = "NONE" + named_port { + name = "customhttp" + port = 8080 + } + }`, igm) +} + func resourceSplitter(resource string) string { splits := strings.Split(resource, "/") From 17af2f69afe5d6bc8c130779f54edfc8327904ae Mon Sep 17 00:00:00 2001 From: zbikmarc Date: Thu, 12 Jan 2017 15:05:13 +0100 Subject: [PATCH 2/2] providers/google: Add subnetwork_project field to enable cross-project networking in instance templates (#11110) * Add subnetwork_project field to allow for XPN in GCE instance templates * Missing os import * Removing unneeded check * fix formatting * Add subnetwork_project to read --- resource_compute_instance_template.go | 18 +++++-- resource_compute_instance_template_test.go | 63 +++++++++++++++++++++- 2 files changed, 77 insertions(+), 4 deletions(-) diff --git a/resource_compute_instance_template.go b/resource_compute_instance_template.go index da0708b3..9b9798dc 100644 --- a/resource_compute_instance_template.go +++ b/resource_compute_instance_template.go @@ -203,6 +203,12 @@ func resourceComputeInstanceTemplate() *schema.Resource { ForceNew: true, }, + "subnetwork_project": &schema.Schema{ + Type: schema.TypeString, + Optional: true, + ForceNew: true, + }, + "access_config": &schema.Schema{ Type: schema.TypeList, Optional: true, @@ -406,14 +412,16 @@ func buildNetworks(d *schema.ResourceData, meta interface{}) ([]*compute.Network for i := 0; i < networksCount; i++ { prefix := fmt.Sprintf("network_interface.%d", i) - var networkName, subnetworkName string + var networkName, subnetworkName, subnetworkProject string if v, ok := d.GetOk(prefix + ".network"); ok { networkName = v.(string) } if v, ok := d.GetOk(prefix + ".subnetwork"); ok { subnetworkName = v.(string) } - + if v, ok := d.GetOk(prefix + ".subnetwork_project"); ok { + subnetworkProject = v.(string) + } if networkName == "" && subnetworkName == "" { return nil, fmt.Errorf("network or subnetwork must be provided") } @@ -435,8 +443,11 @@ func buildNetworks(d *schema.ResourceData, meta interface{}) ([]*compute.Network if err != nil { return nil, err } + if subnetworkProject == "" { + subnetworkProject = project + } subnetwork, err := config.clientCompute.Subnetworks.Get( - project, region, subnetworkName).Do() + subnetworkProject, region, subnetworkName).Do() if err != nil { return nil, fmt.Errorf( "Error referencing subnetwork '%s' in region '%s': %s", @@ -639,6 +650,7 @@ func flattenNetworkInterfaces(networkInterfaces []*compute.NetworkInterface) ([] subnetworkUrl := strings.Split(networkInterface.Subnetwork, "/") networkInterfaceMap["subnetwork"] = subnetworkUrl[len(subnetworkUrl)-1] region = subnetworkUrl[len(subnetworkUrl)-3] + networkInterfaceMap["subnetwork_project"] = subnetworkUrl[len(subnetworkUrl)-5] } if networkInterface.AccessConfigs != nil { diff --git a/resource_compute_instance_template_test.go b/resource_compute_instance_template_test.go index 642e0e57..e287d32e 100644 --- a/resource_compute_instance_template_test.go +++ b/resource_compute_instance_template_test.go @@ -2,6 +2,7 @@ package google import ( "fmt" + "os" "strings" "testing" @@ -115,6 +116,27 @@ func TestAccComputeInstanceTemplate_subnet_custom(t *testing.T) { }) } +func TestAccComputeInstanceTemplate_subnet_xpn(t *testing.T) { + var instanceTemplate compute.InstanceTemplate + var xpn_host = os.Getenv("GOOGLE_XPN_HOST_PROJECT") + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckComputeInstanceTemplateDestroy, + Steps: []resource.TestStep{ + resource.TestStep{ + Config: testAccComputeInstanceTemplate_subnet_xpn(xpn_host), + Check: resource.ComposeTestCheckFunc( + testAccCheckComputeInstanceTemplateExists( + "google_compute_instance_template.foobar", &instanceTemplate), + testAccCheckComputeInstanceTemplateSubnetwork(&instanceTemplate), + ), + }, + }, + }) +} + func TestAccComputeInstanceTemplate_metadata_startup_script(t *testing.T) { var instanceTemplate compute.InstanceTemplate @@ -467,6 +489,45 @@ resource "google_compute_instance_template" "foobar" { } }`, acctest.RandString(10), acctest.RandString(10), acctest.RandString(10)) +func testAccComputeInstanceTemplate_subnet_xpn(xpn_host string) string { + return fmt.Sprintf(` + resource "google_compute_network" "network" { + name = "network-%s" + auto_create_subnetworks = false + project = "%s" + } + + resource "google_compute_subnetwork" "subnetwork" { + name = "subnetwork-%s" + ip_cidr_range = "10.0.0.0/24" + region = "us-central1" + network = "${google_compute_network.network.self_link}" + project = "%s" + } + + resource "google_compute_instance_template" "foobar" { + name = "instance-test-%s" + machine_type = "n1-standard-1" + region = "us-central1" + + disk { + source_image = "debian-8-jessie-v20160803" + auto_delete = true + disk_size_gb = 10 + boot = true + } + + network_interface { + subnetwork = "${google_compute_subnetwork.subnetwork.name}" + subnetwork_project = "${google_compute_subnetwork.subnetwork.project}" + } + + metadata { + foo = "bar" + } + }`, acctest.RandString(10), xpn_host, acctest.RandString(10), xpn_host, acctest.RandString(10)) +} + var testAccComputeInstanceTemplate_startup_script = fmt.Sprintf(` resource "google_compute_instance_template" "foobar" { name = "instance-test-%s" @@ -486,6 +547,6 @@ resource "google_compute_instance_template" "foobar" { network_interface{ network = "default" } - + metadata_startup_script = "echo 'Hello'" }`, acctest.RandString(10))