diff --git a/google/resource_compute_instance.go b/google/resource_compute_instance.go index 6a2272d4..d30dd580 100644 --- a/google/resource_compute_instance.go +++ b/google/resource_compute_instance.go @@ -1063,7 +1063,7 @@ func resourceComputeInstanceRead(d *schema.ResourceData, meta interface{}) error d.Set("attached_disk", attachedDisks) d.Set("scratch_disk", scratchDisks) - scheduling, _ := flattenScheduling(instance.Scheduling) + scheduling := flattenScheduling(instance.Scheduling) d.Set("scheduling", scheduling) d.Set("self_link", instance.SelfLink) diff --git a/google/resource_compute_instance_template.go b/google/resource_compute_instance_template.go index b3a10ad5..f4991ed0 100644 --- a/google/resource_compute_instance_template.go +++ b/google/resource_compute_instance_template.go @@ -18,6 +18,8 @@ func resourceComputeInstanceTemplate() *schema.Resource { Importer: &schema.ResourceImporter{ State: schema.ImportStatePassthrough, }, + SchemaVersion: 1, + MigrateState: resourceComputeInstanceTemplateMigrateState, Schema: map[string]*schema.Schema{ "name": &schema.Schema{ @@ -44,6 +46,7 @@ func resourceComputeInstanceTemplate() *schema.Resource { return }, }, + "disk": &schema.Schema{ Type: schema.TypeList, Required: true, @@ -132,11 +135,11 @@ func resourceComputeInstanceTemplate() *schema.Resource { }, "automatic_restart": &schema.Schema{ - Type: schema.TypeBool, - Optional: true, - Default: true, - ForceNew: true, - Deprecated: "Please use `scheduling.automatic_restart` instead", + Type: schema.TypeBool, + Optional: true, + Default: true, + ForceNew: true, + Removed: "Use 'scheduling.automatic_restart' instead.", }, "can_ip_forward": &schema.Schema{ @@ -226,10 +229,10 @@ func resourceComputeInstanceTemplate() *schema.Resource { }, "on_host_maintenance": &schema.Schema{ - Type: schema.TypeString, - Optional: true, - ForceNew: true, - Deprecated: "Please use `scheduling.on_host_maintenance` instead", + Type: schema.TypeString, + Optional: true, + ForceNew: true, + Removed: "Use 'scheduling.on_host_maintenance' instead.", }, "project": &schema.Schema{ @@ -510,15 +513,6 @@ func resourceComputeInstanceTemplateCreate(d *schema.ResourceData, meta interfac instanceProperties.Scheduling = &compute.Scheduling{} instanceProperties.Scheduling.OnHostMaintenance = "MIGRATE" - // Depreciated fields - if v, ok := d.GetOk("automatic_restart"); ok { - instanceProperties.Scheduling.AutomaticRestart = googleapi.Bool(v.(bool)) - } - - if v, ok := d.GetOk("on_host_maintenance"); ok { - instanceProperties.Scheduling.OnHostMaintenance = v.(string) - } - forceSendFieldsScheduling := make([]string, 0, 3) var hasSendMaintenance bool hasSendMaintenance = false @@ -529,10 +523,10 @@ func resourceComputeInstanceTemplateCreate(d *schema.ResourceData, meta interfac } _scheduling := _schedulings[0].(map[string]interface{}) - if vp, okp := _scheduling["automatic_restart"]; okp { - instanceProperties.Scheduling.AutomaticRestart = googleapi.Bool(vp.(bool)) - forceSendFieldsScheduling = append(forceSendFieldsScheduling, "AutomaticRestart") - } + // "automatic_restart" has a default value and is always safe to dereference + automaticRestart := _scheduling["automatic_restart"].(bool) + instanceProperties.Scheduling.AutomaticRestart = googleapi.Bool(automaticRestart) + forceSendFieldsScheduling = append(forceSendFieldsScheduling, "AutomaticRestart") if vp, okp := _scheduling["on_host_maintenance"]; okp { instanceProperties.Scheduling.OnHostMaintenance = vp.(string) @@ -672,7 +666,7 @@ func flattenNetworkInterfaces(networkInterfaces []*compute.NetworkInterface) ([] return result, region } -func flattenScheduling(scheduling *compute.Scheduling) ([]map[string]interface{}, *bool) { +func flattenScheduling(scheduling *compute.Scheduling) []map[string]interface{} { result := make([]map[string]interface{}, 0, 1) schedulingMap := make(map[string]interface{}) if scheduling.AutomaticRestart != nil { @@ -681,8 +675,7 @@ func flattenScheduling(scheduling *compute.Scheduling) ([]map[string]interface{} schedulingMap["on_host_maintenance"] = scheduling.OnHostMaintenance schedulingMap["preemptible"] = scheduling.Preemptible result = append(result, schedulingMap) - // TODO(selmanj) No need to return two values as automatic restart is captured in map - return result, scheduling.AutomaticRestart + return result } func flattenServiceAccounts(serviceAccounts []*compute.ServiceAccount) []map[string]interface{} { @@ -786,13 +779,10 @@ func resourceComputeInstanceTemplateRead(d *schema.ResourceData, meta interface{ } } if instanceTemplate.Properties.Scheduling != nil { - scheduling, autoRestart := flattenScheduling(instanceTemplate.Properties.Scheduling) + scheduling := flattenScheduling(instanceTemplate.Properties.Scheduling) if err = d.Set("scheduling", scheduling); err != nil { return fmt.Errorf("Error setting scheduling: %s", err) } - if err = d.Set("automatic_restart", autoRestart); err != nil { - return fmt.Errorf("Error setting automatic_restart: %s", err) - } } if instanceTemplate.Properties.Tags != nil { if err = d.Set("tags", instanceTemplate.Properties.Tags.Items); err != nil { diff --git a/google/resource_compute_instance_template_migrate.go b/google/resource_compute_instance_template_migrate.go new file mode 100644 index 00000000..0b285027 --- /dev/null +++ b/google/resource_compute_instance_template_migrate.go @@ -0,0 +1,58 @@ +package google + +import ( + "fmt" + "log" + + "github.com/hashicorp/terraform/terraform" +) + +func resourceComputeInstanceTemplateMigrateState( + v int, is *terraform.InstanceState, meta interface{}) (*terraform.InstanceState, error) { + if is.Empty() { + log.Println("[DEBUG] Empty InstanceState; nothing to migrate.") + return is, nil + } + + switch v { + case 0: + log.Println("[INFO] Found Compute Instance Template State v0; migrating to v1") + return migrateComputeInstanceTemplateStateV0toV1(is) + default: + return is, fmt.Errorf("Unexpected schema version: %d", v) + } +} + +func migrateComputeInstanceTemplateStateV0toV1(is *terraform.InstanceState) (*terraform.InstanceState, error) { + log.Printf("[DEBUG] Attributes before migration: %#v", is.Attributes) + + // automatic_restart is stored in two places. The top-level automatic_restart value is deprecated, so let's delete + // it from the state map for now. For paranoia's sake, we compare it to the value stored in scheduling as well. + ar := is.Attributes["automatic_restart"] + delete(is.Attributes, "automatic_restart") + + schedulingCount, ok := is.Attributes["scheduling.#"] + if ok && schedulingCount != "0" && schedulingCount != "1" { + return nil, fmt.Errorf("Found multiple scheduling blocks when there should only be one") + } + + if !ok || schedulingCount == "0" { + // Either scheduling is missing or empty; go ahead and add + is.Attributes["scheduling.#"] = "1" + is.Attributes["scheduling.0.automatic_restart"] = ar + } + + schedAr := is.Attributes["scheduling.0.automatic_restart"] + if ar != schedAr { + // Here we could try to choose one value over the other, but in reality they should never be out of sync; error + // for now + return nil, fmt.Errorf("Found differing values for automatic_restart in state, unsure how to proceed. automatic_restart = %#v, scheduling.0.automatic_restart = %#v", ar, schedAr) + } + + // We also nuke "on_host_maintenance" as it's been deprecated as well. Here we don't check the current value though + // as the authoritative value has always been maintained in the scheduling block. + delete(is.Attributes, "on_host_maintenance") + + log.Printf("[DEBUG] Attributes after migration: %#v", is.Attributes) + return is, nil +} diff --git a/google/resource_compute_instance_template_migrate_test.go b/google/resource_compute_instance_template_migrate_test.go new file mode 100644 index 00000000..cc334b4f --- /dev/null +++ b/google/resource_compute_instance_template_migrate_test.go @@ -0,0 +1,137 @@ +package google + +import ( + "testing" + + "reflect" + "strings" + + "github.com/hashicorp/terraform/terraform" +) + +func TestComputeInstanceTemplateMigrateState(t *testing.T) { + cases := map[string]struct { + StateVersion int + BeforeAttributes map[string]string + AfterAttributes map[string]string + ErrorStringExpected string + Meta interface{} + }{ + "invalid state version": { + StateVersion: -1, + ErrorStringExpected: "Unexpected schema version: -1", + }, + "simple automatic_restart case removal": { + StateVersion: 0, + BeforeAttributes: map[string]string{ + "automatic_restart": "true", + "scheduling.#": "1", + "scheduling.0.automatic_restart": "true", + }, + AfterAttributes: map[string]string{ + "scheduling.#": "1", + "scheduling.0.automatic_restart": "true", + }, + }, + "simple on_host_maintenance removal": { + StateVersion: 0, + BeforeAttributes: map[string]string{ + "on_host_maintenance": "MIGRATE", + "automatic_restart": "true", + "scheduling.#": "1", + "scheduling.0.automatic_restart": "true", + }, + AfterAttributes: map[string]string{ + "scheduling.#": "1", + "scheduling.0.automatic_restart": "true", + }, + }, + "missing scheduling block": { + StateVersion: 0, + BeforeAttributes: map[string]string{ + "automatic_restart": "true", + }, + AfterAttributes: map[string]string{ + "scheduling.#": "1", + "scheduling.0.automatic_restart": "true", + }, + }, + "empty scheduling block": { + StateVersion: 0, + BeforeAttributes: map[string]string{ + "automatic_restart": "true", + "scheduling.#": "0", + }, + AfterAttributes: map[string]string{ + "scheduling.#": "1", + "scheduling.0.automatic_restart": "true", + }, + }, + "error upon multiple scheduling block": { + StateVersion: 0, + BeforeAttributes: map[string]string{ + "automatic_restart": "true", + "scheduling.#": "2", + "scheduling.0.automatic_restart": "true", + "scheduling.1.automatic_restart": "true", + }, + ErrorStringExpected: "Found multiple scheduling blocks when there should only be one", + }, + "error upon differing automatic_restart values": { + StateVersion: 0, + BeforeAttributes: map[string]string{ + "automatic_restart": "true", + "scheduling.#": "1", + "scheduling.0.automatic_restart": "false", + }, + ErrorStringExpected: "Found differing values for automatic_restart in state, unsure how to proceed.", + }, + } + + for tn, tc := range cases { + is := &terraform.InstanceState{ + ID: "i-abc123", + Attributes: tc.BeforeAttributes, + } + is, err := resourceComputeInstanceTemplateMigrateState( + tc.StateVersion, is, tc.Meta) + + if err != nil { + if tc.ErrorStringExpected == "" { + t.Fatalf("bad: %s, err: %#v", tn, err) + } else if !strings.Contains(err.Error(), tc.ErrorStringExpected) { + t.Fatalf("Expected error containing string %s, instead found %#v", tc.ErrorStringExpected, err) + } else { + continue + } + } + + // Compare both maps for identity + if !reflect.DeepEqual(is.Attributes, tc.AfterAttributes) { + t.Fatalf("Expected attributes %#v, got attributes %#v", tc.AfterAttributes, is.Attributes) + } + } +} + +func TestComputeInstanceTemplateMigrateState_empty(t *testing.T) { + var is *terraform.InstanceState + var meta interface{} + + // should handle nil + is, err := resourceComputeInstanceTemplateMigrateState(0, is, meta) + + if err != nil { + t.Fatalf("err: %#v", err) + } + if is != nil { + t.Fatalf("expected nil instancestate, got: %#v", is) + } + + // should handle non-nil but empty + is = &terraform.InstanceState{} + is, err = resourceComputeInstanceTemplateMigrateState(0, is, meta) + + if err != nil { + t.Fatalf("err: %#v", err) + } +} diff --git a/google/resource_compute_instance_template_test.go b/google/resource_compute_instance_template_test.go index 62a8beef..58d745f1 100644 --- a/google/resource_compute_instance_template_test.go +++ b/google/resource_compute_instance_template_test.go @@ -34,6 +34,27 @@ func TestAccComputeInstanceTemplate_basic(t *testing.T) { }) } +func TestAccComputeInstanceTemplate_preemptible(t *testing.T) { + var instanceTemplate compute.InstanceTemplate + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckComputeInstanceTemplateDestroy, + Steps: []resource.TestStep{ + resource.TestStep{ + Config: testAccComputeInstanceTemplate_preemptible, + Check: resource.ComposeTestCheckFunc( + testAccCheckComputeInstanceTemplateExists( + "google_compute_instance_template.foobar", &instanceTemplate), + testAccCheckComputeInstanceTemplateAutomaticRestart(&instanceTemplate, false), + testAccCheckComputeInstanceTemplatePreemptible(&instanceTemplate, true), + ), + }, + }, + }) +} + func TestAccComputeInstanceTemplate_IP(t *testing.T) { var instanceTemplate compute.InstanceTemplate @@ -333,6 +354,28 @@ func testAccCheckComputeInstanceTemplateTag(instanceTemplate *compute.InstanceTe } } +func testAccCheckComputeInstanceTemplatePreemptible(instanceTemplate *compute.InstanceTemplate, preemptible bool) resource.TestCheckFunc { + return func(s *terraform.State) error { + if instanceTemplate.Properties.Scheduling.Preemptible != preemptible { + return fmt.Errorf("Expected preemptible value %v, got %v", preemptible, instanceTemplate.Properties.Scheduling.Preemptible) + } + return nil + } +} + +func testAccCheckComputeInstanceTemplateAutomaticRestart(instanceTemplate *compute.InstanceTemplate, automaticRestart bool) resource.TestCheckFunc { + return func(s *terraform.State) error { + ar := instanceTemplate.Properties.Scheduling.AutomaticRestart + if ar == nil { + return fmt.Errorf("Expected to see a value for AutomaticRestart, but got nil") + } + if *ar != automaticRestart { + return fmt.Errorf("Expected automatic restart value %v, got %v", automaticRestart, ar) + } + return nil + } +} + func testAccCheckComputeInstanceTemplateStartupScript(instanceTemplate *compute.InstanceTemplate, n string) resource.TestCheckFunc { return func(s *terraform.State) error { if instanceTemplate.Properties.Metadata == nil && n == "" { @@ -400,6 +443,37 @@ resource "google_compute_instance_template" "foobar" { } }`, acctest.RandString(10)) +var testAccComputeInstanceTemplate_preemptible = fmt.Sprintf(` +resource "google_compute_instance_template" "foobar" { + name = "instancet-test-%s" + machine_type = "n1-standard-1" + can_ip_forward = false + tags = ["foo", "bar"] + + disk { + source_image = "debian-8-jessie-v20160803" + auto_delete = true + boot = true + } + + network_interface { + network = "default" + } + + scheduling { + preemptible = true + automatic_restart = false + } + + metadata { + foo = "bar" + } + + service_account { + scopes = ["userinfo-email", "compute-ro", "storage-ro"] + } +}`, acctest.RandString(10)) + var testAccComputeInstanceTemplate_ip = fmt.Sprintf(` resource "google_compute_address" "foo" { name = "instancet-test-%s"