Skip to content

Commit 7d1b850

Browse files
modular-magicianmodular-magician
authored
fix empty accelerator config permadiff and instance not starting after an update in resource google_workbench_instance (#10961) (#7557)
[upstream:e69e5f7253af4532e5ed75617a8cab176eef4a72] Signed-off-by: Modular Magician <[email protected]>
1 parent 64c446c commit 7d1b850

File tree

4 files changed

+215
-27
lines changed

4 files changed

+215
-27
lines changed

‎.changelog/10961.txt‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
```release-note:bug
2+
workbench: fixed the permadiff caused by empty `accelerator_configs` in resource `google_workbench_instance`
3+
```
4+
```release-note:bug
5+
workbench: fixed the issue that the instance is not starting after an update in resource `google_workbench_instance`
6+
```

‎google-beta/services/workbench/resource_workbench_instance.go‎

Lines changed: 57 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,16 @@ func WorkbenchInstanceTagsDiffSuppress(_, _, _ string, d *schema.ResourceData) b
161161
return false
162162
}
163163

164+
func WorkbenchInstanceAcceleratorDiffSuppress(_, _, _ string, d *schema.ResourceData) bool {
165+
old, new := d.GetChange("gce_setup.0.accelerator_configs")
166+
oldInterface := old.([]interface{})
167+
newInterface := new.([]interface{})
168+
if len(oldInterface) == 0 && len(newInterface) == 1 && newInterface[0] == nil {
169+
return true
170+
}
171+
return false
172+
}
173+
164174
// waitForWorkbenchInstanceActive waits for an workbench instance to become "ACTIVE"
165175
func waitForWorkbenchInstanceActive(d *schema.ResourceData, config *transport_tpg.Config, timeout time.Duration) error {
166176
return retry.Retry(timeout, func() *retry.RetryError {
@@ -267,8 +277,9 @@ func ResourceWorkbenchInstance() *schema.Resource {
267277
Elem: &schema.Resource{
268278
Schema: map[string]*schema.Schema{
269279
"accelerator_configs": {
270-
Type: schema.TypeList,
271-
Optional: true,
280+
Type: schema.TypeList,
281+
Optional: true,
282+
DiffSuppressFunc: WorkbenchInstanceAcceleratorDiffSuppress,
272283
Description: `The hardware accelerators used on this instance. If you use accelerators, make sure that your configuration has
273284
[enough vCPUs and memory to support the 'machine_type' you have selected](https://cloud.google.com/compute/docs/gpus/#gpus-list).
274285
Currently supports only one accelerator configuration.`,
@@ -992,38 +1003,28 @@ func resourceWorkbenchInstanceUpdate(d *schema.ResourceData, meta interface{}) e
9921003
if err != nil {
9931004
return err
9941005
}
995-
name := d.Get("name").(string)
996-
if d.HasChange("gce_setup.0.machine_type") || d.HasChange("gce_setup.0.accelerator_configs") || d.HasChange("gce_setup.0.shielded_instance_config") {
997-
state := d.Get("state").(string)
998-
999-
if state != "STOPPED" {
1000-
dRes, err := modifyWorkbenchInstanceState(config, d, project, billingProject, userAgent, "stop")
1001-
if err != nil {
1002-
return err
1003-
}
1004-
1005-
if err := waitForWorkbenchOperation(config, d, project, billingProject, userAgent, dRes); err != nil {
1006-
return fmt.Errorf("Error stopping Workbench Instance: %s", err)
1007-
}
1008-
1009-
} else {
1010-
log.Printf("[DEBUG] Workbench Instance %q has state %q.", name, state)
1011-
}
1012-
1013-
} else {
1014-
log.Printf("[DEBUG] Workbench Instance %q need not be stopped for the update.", name)
1015-
}
1016-
10171006
// Build custom mask since the notebooks API does not support gce_setup as a valid mask
1007+
stopInstance := false
10181008
newUpdateMask := []string{}
10191009
if d.HasChange("gce_setup.0.machine_type") {
10201010
newUpdateMask = append(newUpdateMask, "gce_setup.machine_type")
1011+
stopInstance = true
10211012
}
10221013
if d.HasChange("gce_setup.0.accelerator_configs") {
10231014
newUpdateMask = append(newUpdateMask, "gce_setup.accelerator_configs")
1015+
stopInstance = true
1016+
}
1017+
if d.HasChange("gce_setup.0.shielded_instance_config.0.enable_secure_boot") {
1018+
newUpdateMask = append(newUpdateMask, "gce_setup.shielded_instance_config.enable_secure_boot")
1019+
stopInstance = true
10241020
}
1025-
if d.HasChange("gce_setup.0.shielded_instance_config") {
1026-
newUpdateMask = append(newUpdateMask, "gce_setup.shielded_instance_config")
1021+
if d.HasChange("gce_setup.0.shielded_instance_config.0.enable_vtpm") {
1022+
newUpdateMask = append(newUpdateMask, "gce_setup.shielded_instance_config.enable_vtpm")
1023+
stopInstance = true
1024+
}
1025+
if d.HasChange("gce_setup.0.shielded_instance_config.0.enable_integrity_monitoring") {
1026+
newUpdateMask = append(newUpdateMask, "gce_setup.shielded_instance_config.enable_integrity_monitoring")
1027+
stopInstance = true
10271028
}
10281029
if d.HasChange("gce_setup.0.metadata") {
10291030
newUpdateMask = append(newUpdateMask, "gceSetup.metadata")
@@ -1038,6 +1039,28 @@ func resourceWorkbenchInstanceUpdate(d *schema.ResourceData, meta interface{}) e
10381039
return err
10391040
}
10401041

1042+
name := d.Get("name").(string)
1043+
if stopInstance {
1044+
state := d.Get("state").(string)
1045+
1046+
if state != "STOPPED" {
1047+
dRes, err := modifyWorkbenchInstanceState(config, d, project, billingProject, userAgent, "stop")
1048+
if err != nil {
1049+
return err
1050+
}
1051+
1052+
if err := waitForWorkbenchOperation(config, d, project, billingProject, userAgent, dRes); err != nil {
1053+
return fmt.Errorf("Error stopping Workbench Instance: %s", err)
1054+
}
1055+
1056+
} else {
1057+
log.Printf("[DEBUG] Workbench Instance %q has state %q.", name, state)
1058+
}
1059+
1060+
} else {
1061+
log.Printf("[DEBUG] Workbench Instance %q need not be stopped for the update.", name)
1062+
}
1063+
10411064
// err == nil indicates that the billing_project value was found
10421065
if bp, err := tpgresource.GetBillingProject(d, config); err == nil {
10431066
billingProject = bp
@@ -1074,7 +1097,7 @@ func resourceWorkbenchInstanceUpdate(d *schema.ResourceData, meta interface{}) e
10741097
state := d.Get("state").(string)
10751098
desired_state := d.Get("desired_state").(string)
10761099

1077-
if state != desired_state {
1100+
if state != desired_state || stopInstance {
10781101
verb := "start"
10791102
if desired_state == "STOPPED" {
10801103
verb = "stop"
@@ -1088,6 +1111,13 @@ func resourceWorkbenchInstanceUpdate(d *schema.ResourceData, meta interface{}) e
10881111
return fmt.Errorf("Error waiting to modify Workbench Instance state: %s", err)
10891112
}
10901113

1114+
if verb == "start" {
1115+
if err := waitForWorkbenchInstanceActive(d, config, d.Timeout(schema.TimeoutUpdate)-time.Minute); err != nil {
1116+
return fmt.Errorf("Workbench instance %q did not reach ACTIVE state: %q", d.Get("name").(string), err)
1117+
}
1118+
1119+
}
1120+
10911121
} else {
10921122
log.Printf("[DEBUG] Workbench Instance %q has state %q.", name, state)
10931123
}

‎google-beta/services/workbench/resource_workbench_instance_shielded_config_test.go‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,10 @@ func TestAccWorkbenchInstance_shielded_config_update(t *testing.T) {
2323
Steps: []resource.TestStep{
2424
{
2525
Config: testAccWorkbenchInstance_shielded_config_false(context),
26+
Check: resource.ComposeTestCheckFunc(
27+
resource.TestCheckResourceAttr(
28+
"google_workbench_instance.instance", "state", "ACTIVE"),
29+
),
2630
},
2731
{
2832
ResourceName: "google_workbench_instance.instance",
@@ -32,6 +36,10 @@ func TestAccWorkbenchInstance_shielded_config_update(t *testing.T) {
3236
},
3337
{
3438
Config: testAccWorkbenchInstance_shielded_config_true(context),
39+
Check: resource.ComposeTestCheckFunc(
40+
resource.TestCheckResourceAttr(
41+
"google_workbench_instance.instance", "state", "ACTIVE"),
42+
),
3543
},
3644
{
3745
ResourceName: "google_workbench_instance.instance",
@@ -56,6 +64,10 @@ func TestAccWorkbenchInstance_shielded_config_remove(t *testing.T) {
5664
Steps: []resource.TestStep{
5765
{
5866
Config: testAccWorkbenchInstance_shielded_config_true(context),
67+
Check: resource.ComposeTestCheckFunc(
68+
resource.TestCheckResourceAttr(
69+
"google_workbench_instance.instance", "state", "ACTIVE"),
70+
),
5971
},
6072
{
6173
ResourceName: "google_workbench_instance.instance",
@@ -65,6 +77,10 @@ func TestAccWorkbenchInstance_shielded_config_remove(t *testing.T) {
6577
},
6678
{
6779
Config: testAccWorkbenchInstance_shielded_config_none(context),
80+
Check: resource.ComposeTestCheckFunc(
81+
resource.TestCheckResourceAttr(
82+
"google_workbench_instance.instance", "state", "ACTIVE"),
83+
),
6884
},
6985
{
7086
ResourceName: "google_workbench_instance.instance",
@@ -89,6 +105,10 @@ func TestAccWorkbenchInstance_shielded_config_double_apply(t *testing.T) {
89105
Steps: []resource.TestStep{
90106
{
91107
Config: testAccWorkbenchInstance_shielded_config_none(context),
108+
Check: resource.ComposeTestCheckFunc(
109+
resource.TestCheckResourceAttr(
110+
"google_workbench_instance.instance", "state", "ACTIVE"),
111+
),
92112
},
93113
{
94114
ResourceName: "google_workbench_instance.instance",
@@ -98,6 +118,10 @@ func TestAccWorkbenchInstance_shielded_config_double_apply(t *testing.T) {
98118
},
99119
{
100120
Config: testAccWorkbenchInstance_shielded_config_none(context),
121+
Check: resource.ComposeTestCheckFunc(
122+
resource.TestCheckResourceAttr(
123+
"google_workbench_instance.instance", "state", "ACTIVE"),
124+
),
101125
},
102126
{
103127
ResourceName: "google_workbench_instance.instance",
@@ -107,6 +131,10 @@ func TestAccWorkbenchInstance_shielded_config_double_apply(t *testing.T) {
107131
},
108132
{
109133
Config: testAccWorkbenchInstance_shielded_config_false(context),
134+
Check: resource.ComposeTestCheckFunc(
135+
resource.TestCheckResourceAttr(
136+
"google_workbench_instance.instance", "state", "ACTIVE"),
137+
),
110138
},
111139
{
112140
ResourceName: "google_workbench_instance.instance",
@@ -116,6 +144,10 @@ func TestAccWorkbenchInstance_shielded_config_double_apply(t *testing.T) {
116144
},
117145
{
118146
Config: testAccWorkbenchInstance_shielded_config_false(context),
147+
Check: resource.ComposeTestCheckFunc(
148+
resource.TestCheckResourceAttr(
149+
"google_workbench_instance.instance", "state", "ACTIVE"),
150+
),
119151
},
120152
{
121153
ResourceName: "google_workbench_instance.instance",
@@ -125,6 +157,10 @@ func TestAccWorkbenchInstance_shielded_config_double_apply(t *testing.T) {
125157
},
126158
{
127159
Config: testAccWorkbenchInstance_shielded_config_true(context),
160+
Check: resource.ComposeTestCheckFunc(
161+
resource.TestCheckResourceAttr(
162+
"google_workbench_instance.instance", "state", "ACTIVE"),
163+
),
128164
},
129165
{
130166
ResourceName: "google_workbench_instance.instance",
@@ -134,6 +170,10 @@ func TestAccWorkbenchInstance_shielded_config_double_apply(t *testing.T) {
134170
},
135171
{
136172
Config: testAccWorkbenchInstance_shielded_config_true(context),
173+
Check: resource.ComposeTestCheckFunc(
174+
resource.TestCheckResourceAttr(
175+
"google_workbench_instance.instance", "state", "ACTIVE"),
176+
),
137177
},
138178
{
139179
ResourceName: "google_workbench_instance.instance",

0 commit comments

Comments
 (0)