diff --git a/.changelog/10961.txt b/.changelog/10961.txt new file mode 100644 index 00000000000..792e73c316e --- /dev/null +++ b/.changelog/10961.txt @@ -0,0 +1,6 @@ +```release-note:bug +workbench: fixed the permadiff caused by empty `accelerator_configs` in resource `google_workbench_instance` +``` +```release-note:bug +workbench: fixed the issue that the instance is not starting after an update in resource `google_workbench_instance` +``` \ No newline at end of file diff --git a/google/services/workbench/resource_workbench_instance.go b/google/services/workbench/resource_workbench_instance.go index b5aee9d3a37..efe8edc8c06 100644 --- a/google/services/workbench/resource_workbench_instance.go +++ b/google/services/workbench/resource_workbench_instance.go @@ -161,6 +161,16 @@ func WorkbenchInstanceTagsDiffSuppress(_, _, _ string, d *schema.ResourceData) b return false } +func WorkbenchInstanceAcceleratorDiffSuppress(_, _, _ string, d *schema.ResourceData) bool { + old, new := d.GetChange("gce_setup.0.accelerator_configs") + oldInterface := old.([]interface{}) + newInterface := new.([]interface{}) + if len(oldInterface) == 0 && len(newInterface) == 1 && newInterface[0] == nil { + return true + } + return false +} + // waitForWorkbenchInstanceActive waits for an workbench instance to become "ACTIVE" func waitForWorkbenchInstanceActive(d *schema.ResourceData, config *transport_tpg.Config, timeout time.Duration) error { return retry.Retry(timeout, func() *retry.RetryError { @@ -267,8 +277,9 @@ func ResourceWorkbenchInstance() *schema.Resource { Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ "accelerator_configs": { - Type: schema.TypeList, - Optional: true, + Type: schema.TypeList, + Optional: true, + DiffSuppressFunc: WorkbenchInstanceAcceleratorDiffSuppress, Description: `The hardware accelerators used on this instance. If you use accelerators, make sure that your configuration has [enough vCPUs and memory to support the 'machine_type' you have selected](https://cloud.google.com/compute/docs/gpus/#gpus-list). Currently supports only one accelerator configuration.`, @@ -992,38 +1003,28 @@ func resourceWorkbenchInstanceUpdate(d *schema.ResourceData, meta interface{}) e if err != nil { return err } - name := d.Get("name").(string) - if d.HasChange("gce_setup.0.machine_type") || d.HasChange("gce_setup.0.accelerator_configs") || d.HasChange("gce_setup.0.shielded_instance_config") { - state := d.Get("state").(string) - - if state != "STOPPED" { - dRes, err := modifyWorkbenchInstanceState(config, d, project, billingProject, userAgent, "stop") - if err != nil { - return err - } - - if err := waitForWorkbenchOperation(config, d, project, billingProject, userAgent, dRes); err != nil { - return fmt.Errorf("Error stopping Workbench Instance: %s", err) - } - - } else { - log.Printf("[DEBUG] Workbench Instance %q has state %q.", name, state) - } - - } else { - log.Printf("[DEBUG] Workbench Instance %q need not be stopped for the update.", name) - } - // Build custom mask since the notebooks API does not support gce_setup as a valid mask + stopInstance := false newUpdateMask := []string{} if d.HasChange("gce_setup.0.machine_type") { newUpdateMask = append(newUpdateMask, "gce_setup.machine_type") + stopInstance = true } if d.HasChange("gce_setup.0.accelerator_configs") { newUpdateMask = append(newUpdateMask, "gce_setup.accelerator_configs") + stopInstance = true + } + if d.HasChange("gce_setup.0.shielded_instance_config.0.enable_secure_boot") { + newUpdateMask = append(newUpdateMask, "gce_setup.shielded_instance_config.enable_secure_boot") + stopInstance = true } - if d.HasChange("gce_setup.0.shielded_instance_config") { - newUpdateMask = append(newUpdateMask, "gce_setup.shielded_instance_config") + if d.HasChange("gce_setup.0.shielded_instance_config.0.enable_vtpm") { + newUpdateMask = append(newUpdateMask, "gce_setup.shielded_instance_config.enable_vtpm") + stopInstance = true + } + if d.HasChange("gce_setup.0.shielded_instance_config.0.enable_integrity_monitoring") { + newUpdateMask = append(newUpdateMask, "gce_setup.shielded_instance_config.enable_integrity_monitoring") + stopInstance = true } if d.HasChange("gce_setup.0.metadata") { newUpdateMask = append(newUpdateMask, "gceSetup.metadata") @@ -1038,6 +1039,28 @@ func resourceWorkbenchInstanceUpdate(d *schema.ResourceData, meta interface{}) e return err } + name := d.Get("name").(string) + if stopInstance { + state := d.Get("state").(string) + + if state != "STOPPED" { + dRes, err := modifyWorkbenchInstanceState(config, d, project, billingProject, userAgent, "stop") + if err != nil { + return err + } + + if err := waitForWorkbenchOperation(config, d, project, billingProject, userAgent, dRes); err != nil { + return fmt.Errorf("Error stopping Workbench Instance: %s", err) + } + + } else { + log.Printf("[DEBUG] Workbench Instance %q has state %q.", name, state) + } + + } else { + log.Printf("[DEBUG] Workbench Instance %q need not be stopped for the update.", name) + } + // err == nil indicates that the billing_project value was found if bp, err := tpgresource.GetBillingProject(d, config); err == nil { billingProject = bp @@ -1074,7 +1097,7 @@ func resourceWorkbenchInstanceUpdate(d *schema.ResourceData, meta interface{}) e state := d.Get("state").(string) desired_state := d.Get("desired_state").(string) - if state != desired_state { + if state != desired_state || stopInstance { verb := "start" if desired_state == "STOPPED" { verb = "stop" @@ -1088,6 +1111,13 @@ func resourceWorkbenchInstanceUpdate(d *schema.ResourceData, meta interface{}) e return fmt.Errorf("Error waiting to modify Workbench Instance state: %s", err) } + if verb == "start" { + if err := waitForWorkbenchInstanceActive(d, config, d.Timeout(schema.TimeoutUpdate)-time.Minute); err != nil { + return fmt.Errorf("Workbench instance %q did not reach ACTIVE state: %q", d.Get("name").(string), err) + } + + } + } else { log.Printf("[DEBUG] Workbench Instance %q has state %q.", name, state) } diff --git a/google/services/workbench/resource_workbench_instance_shielded_config_test.go b/google/services/workbench/resource_workbench_instance_shielded_config_test.go index 97e731e0ee7..37ecddcd0db 100644 --- a/google/services/workbench/resource_workbench_instance_shielded_config_test.go +++ b/google/services/workbench/resource_workbench_instance_shielded_config_test.go @@ -23,6 +23,10 @@ func TestAccWorkbenchInstance_shielded_config_update(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccWorkbenchInstance_shielded_config_false(context), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr( + "google_workbench_instance.instance", "state", "ACTIVE"), + ), }, { ResourceName: "google_workbench_instance.instance", @@ -32,6 +36,10 @@ func TestAccWorkbenchInstance_shielded_config_update(t *testing.T) { }, { Config: testAccWorkbenchInstance_shielded_config_true(context), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr( + "google_workbench_instance.instance", "state", "ACTIVE"), + ), }, { ResourceName: "google_workbench_instance.instance", @@ -56,6 +64,10 @@ func TestAccWorkbenchInstance_shielded_config_remove(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccWorkbenchInstance_shielded_config_true(context), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr( + "google_workbench_instance.instance", "state", "ACTIVE"), + ), }, { ResourceName: "google_workbench_instance.instance", @@ -65,6 +77,10 @@ func TestAccWorkbenchInstance_shielded_config_remove(t *testing.T) { }, { Config: testAccWorkbenchInstance_shielded_config_none(context), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr( + "google_workbench_instance.instance", "state", "ACTIVE"), + ), }, { ResourceName: "google_workbench_instance.instance", @@ -89,6 +105,10 @@ func TestAccWorkbenchInstance_shielded_config_double_apply(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccWorkbenchInstance_shielded_config_none(context), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr( + "google_workbench_instance.instance", "state", "ACTIVE"), + ), }, { ResourceName: "google_workbench_instance.instance", @@ -98,6 +118,10 @@ func TestAccWorkbenchInstance_shielded_config_double_apply(t *testing.T) { }, { Config: testAccWorkbenchInstance_shielded_config_none(context), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr( + "google_workbench_instance.instance", "state", "ACTIVE"), + ), }, { ResourceName: "google_workbench_instance.instance", @@ -107,6 +131,10 @@ func TestAccWorkbenchInstance_shielded_config_double_apply(t *testing.T) { }, { Config: testAccWorkbenchInstance_shielded_config_false(context), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr( + "google_workbench_instance.instance", "state", "ACTIVE"), + ), }, { ResourceName: "google_workbench_instance.instance", @@ -116,6 +144,10 @@ func TestAccWorkbenchInstance_shielded_config_double_apply(t *testing.T) { }, { Config: testAccWorkbenchInstance_shielded_config_false(context), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr( + "google_workbench_instance.instance", "state", "ACTIVE"), + ), }, { ResourceName: "google_workbench_instance.instance", @@ -125,6 +157,10 @@ func TestAccWorkbenchInstance_shielded_config_double_apply(t *testing.T) { }, { Config: testAccWorkbenchInstance_shielded_config_true(context), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr( + "google_workbench_instance.instance", "state", "ACTIVE"), + ), }, { ResourceName: "google_workbench_instance.instance", @@ -134,6 +170,10 @@ func TestAccWorkbenchInstance_shielded_config_double_apply(t *testing.T) { }, { Config: testAccWorkbenchInstance_shielded_config_true(context), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr( + "google_workbench_instance.instance", "state", "ACTIVE"), + ), }, { ResourceName: "google_workbench_instance.instance", diff --git a/google/services/workbench/resource_workbench_instance_test.go b/google/services/workbench/resource_workbench_instance_test.go index 35da15c5bd9..e3a9945a58b 100644 --- a/google/services/workbench/resource_workbench_instance_test.go +++ b/google/services/workbench/resource_workbench_instance_test.go @@ -23,6 +23,10 @@ func TestAccWorkbenchInstance_update(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccWorkbenchInstance_basic(context), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr( + "google_workbench_instance.instance", "state", "ACTIVE"), + ), }, { ResourceName: "google_workbench_instance.instance", @@ -32,6 +36,10 @@ func TestAccWorkbenchInstance_update(t *testing.T) { }, { Config: testAccWorkbenchInstance_update(context), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr( + "google_workbench_instance.instance", "state", "ACTIVE"), + ), }, { ResourceName: "google_workbench_instance.instance", @@ -99,6 +107,10 @@ func TestAccWorkbenchInstance_updateGpu(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccWorkbenchInstance_basicGpu(context), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr( + "google_workbench_instance.instance", "state", "ACTIVE"), + ), }, { ResourceName: "google_workbench_instance.instance", @@ -108,6 +120,10 @@ func TestAccWorkbenchInstance_updateGpu(t *testing.T) { }, { Config: testAccWorkbenchInstance_updateGpu(context), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr( + "google_workbench_instance.instance", "state", "ACTIVE"), + ), }, { ResourceName: "google_workbench_instance.instance", @@ -179,6 +195,10 @@ func TestAccWorkbenchInstance_removeGpu(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccWorkbenchInstance_Gpu(context), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr( + "google_workbench_instance.instance", "state", "ACTIVE"), + ), }, { ResourceName: "google_workbench_instance.instance", @@ -188,6 +208,10 @@ func TestAccWorkbenchInstance_removeGpu(t *testing.T) { }, { Config: testAccWorkbenchInstance_updateGpu(context), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr( + "google_workbench_instance.instance", "state", "ACTIVE"), + ), }, { ResourceName: "google_workbench_instance.instance", @@ -244,6 +268,10 @@ func TestAccWorkbenchInstance_updateMetadata(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccWorkbenchInstance_basic(context), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr( + "google_workbench_instance.instance", "state", "ACTIVE"), + ), }, { ResourceName: "google_workbench_instance.instance", @@ -253,6 +281,10 @@ func TestAccWorkbenchInstance_updateMetadata(t *testing.T) { }, { Config: testAccWorkbenchInstance_updateMetadata(context), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr( + "google_workbench_instance.instance", "state", "ACTIVE"), + ), }, { ResourceName: "google_workbench_instance.instance", @@ -297,6 +329,10 @@ func TestAccWorkbenchInstance_updateState(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccWorkbenchInstance_basic(context), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr( + "google_workbench_instance.instance", "state", "ACTIVE"), + ), }, { ResourceName: "google_workbench_instance.instance", @@ -306,6 +342,10 @@ func TestAccWorkbenchInstance_updateState(t *testing.T) { }, { Config: testAccWorkbenchInstance_updateState(context), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr( + "google_workbench_instance.instance", "state", "STOPPED"), + ), }, { ResourceName: "google_workbench_instance.instance", @@ -315,6 +355,10 @@ func TestAccWorkbenchInstance_updateState(t *testing.T) { }, { Config: testAccWorkbenchInstance_basic(context), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr( + "google_workbench_instance.instance", "state", "ACTIVE"), + ), }, { ResourceName: "google_workbench_instance.instance", @@ -337,3 +381,71 @@ resource "google_workbench_instance" "instance" { } `, context) } + +func TestAccWorkbenchInstance_empty_accelerator(t *testing.T) { + t.Parallel() + + context := map[string]interface{}{ + "random_suffix": acctest.RandString(t, 10), + } + + acctest.VcrTest(t, resource.TestCase{ + PreCheck: func() { acctest.AccTestPreCheck(t) }, + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t), + Steps: []resource.TestStep{ + { + Config: testAccWorkbenchInstance_basic(context), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr( + "google_workbench_instance.instance", "state", "ACTIVE"), + ), + }, + { + ResourceName: "google_workbench_instance.instance", + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"name", "instance_owners", "location", "instance_id", "request_id", "labels", "terraform_labels"}, + }, + { + Config: testAccWorkbenchInstance_empty_accelerator(context), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr( + "google_workbench_instance.instance", "state", "ACTIVE"), + ), + }, + { + ResourceName: "google_workbench_instance.instance", + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"name", "instance_owners", "location", "instance_id", "request_id", "labels", "terraform_labels"}, + }, + { + Config: testAccWorkbenchInstance_empty_accelerator(context), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr( + "google_workbench_instance.instance", "state", "ACTIVE"), + ), + }, + { + ResourceName: "google_workbench_instance.instance", + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"name", "instance_owners", "location", "instance_id", "request_id", "labels", "terraform_labels"}, + }, + }, + }) +} + +func testAccWorkbenchInstance_empty_accelerator(context map[string]interface{}) string { + return acctest.Nprintf(` +resource "google_workbench_instance" "instance" { + name = "tf-test-workbench-instance%{random_suffix}" + location = "us-central1-a" + + gce_setup { + accelerator_configs{ + } + } +} +`, context) +}