fix empty accelerator config permadiff and instance not starting afte…

…r an update in resource `google_workbench_instance` (#10961) (#18464) [upstream:e69e5f7253af4532e5ed75617a8cab176eef4a72] Signed-off-by: Modular Magician <magic-modules@google.com>
hashicorp · Jun 17, 2024 · db268a0 · db268a0
1 parent ffff522
commit db268a0
Show file tree

Hide file tree

Showing 4 changed files with 215 additions and 27 deletions.
diff --git a/.changelog/10961.txt 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`
+```
diff --git 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
@@ -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",