Skip to content

Commit 9e48dd7

Browse files
modular-magicianmodular-magician
authored
ForceNew on labels field when the dataflow job has type JOB_TYPE_BATCH (#9253) (#16248)
* ForceNew on labels field when the dataflow job has type JOB_TYPE_BATCH * Handle the case that no update is needed [upstream:fa3cab8281221fcf9ad5661b3f1c2d9b68a85b7e] Signed-off-by: Modular Magician <[email protected]>
1 parent 591ef2d commit 9e48dd7

File tree

3 files changed

+122
-95
lines changed

3 files changed

+122
-95
lines changed

‎.changelog/9253.txt

+6
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
```release-note:bug
2+
dataflow: fixed the bug that the resource update returns error if only `labels` field has changes for batch `google_dataflow_job`
3+
```
4+
```release-note:bug
5+
dataflow: fixed the bug that the resource update returns error if only `labels` field has changes for batch `google_dataflow_flex_template_job`
6+
```

‎google/services/dataflow/resource_dataflow_job.go

+65-45
Original file line numberDiff line numberDiff line change
@@ -263,7 +263,7 @@ func resourceDataflowJobTypeCustomizeDiff(_ context.Context, d *schema.ResourceD
263263
continue
264264
}
265265

266-
if field != "labels" && field != "terraform_labels" && d.HasChange(field) {
266+
if field != "terraform_labels" && d.HasChange(field) {
267267
if err := d.ForceNew(field); err != nil {
268268
return err
269269
}
@@ -416,57 +416,58 @@ func resourceDataflowJobUpdateByReplacement(d *schema.ResourceData, meta interfa
416416
return nil
417417
}
418418

419-
config := meta.(*transport_tpg.Config)
420-
userAgent, err := tpgresource.GenerateUserAgentString(d, config.UserAgent)
421-
if err != nil {
422-
return err
423-
}
424-
425-
project, err := tpgresource.GetProject(d, config)
426-
if err != nil {
427-
return err
428-
}
419+
if jobHasUpdate(d, ResourceDataflowJob().Schema) {
420+
config := meta.(*transport_tpg.Config)
421+
userAgent, err := tpgresource.GenerateUserAgentString(d, config.UserAgent)
422+
if err != nil {
423+
return err
424+
}
429425

430-
region, err := tpgresource.GetRegion(d, config)
431-
if err != nil {
432-
return err
433-
}
426+
project, err := tpgresource.GetProject(d, config)
427+
if err != nil {
428+
return err
429+
}
434430

435-
params := tpgresource.ExpandStringMap(d, "parameters")
436-
tnamemapping := tpgresource.ExpandStringMap(d, "transform_name_mapping")
431+
region, err := tpgresource.GetRegion(d, config)
432+
if err != nil {
433+
return err
434+
}
437435

438-
env, err := resourceDataflowJobSetupEnv(d, config)
439-
if err != nil {
440-
return err
441-
}
436+
params := tpgresource.ExpandStringMap(d, "parameters")
437+
tnamemapping := tpgresource.ExpandStringMap(d, "transform_name_mapping")
442438

443-
request := dataflow.LaunchTemplateParameters{
444-
JobName: d.Get("name").(string),
445-
Parameters: params,
446-
TransformNameMapping: tnamemapping,
447-
Environment: &env,
448-
Update: true,
449-
}
439+
env, err := resourceDataflowJobSetupEnv(d, config)
440+
if err != nil {
441+
return err
442+
}
450443

451-
var response *dataflow.LaunchTemplateResponse
452-
err = transport_tpg.Retry(transport_tpg.RetryOptions{
453-
RetryFunc: func() (updateErr error) {
454-
response, updateErr = resourceDataflowJobLaunchTemplate(config, project, region, userAgent, d.Get("template_gcs_path").(string), &request)
455-
return updateErr
456-
},
457-
Timeout: time.Minute * time.Duration(5),
458-
ErrorRetryPredicates: []transport_tpg.RetryErrorPredicateFunc{transport_tpg.IsDataflowJobUpdateRetryableError},
459-
})
460-
if err != nil {
461-
return err
462-
}
444+
request := dataflow.LaunchTemplateParameters{
445+
JobName: d.Get("name").(string),
446+
Parameters: params,
447+
TransformNameMapping: tnamemapping,
448+
Environment: &env,
449+
Update: true,
450+
}
463451

464-
if err := waitForDataflowJobToBeUpdated(d, config, response.Job.Id, userAgent, d.Timeout(schema.TimeoutUpdate)); err != nil {
465-
return fmt.Errorf("Error updating job with job ID %q: %v", d.Id(), err)
466-
}
452+
var response *dataflow.LaunchTemplateResponse
453+
err = transport_tpg.Retry(transport_tpg.RetryOptions{
454+
RetryFunc: func() (updateErr error) {
455+
response, updateErr = resourceDataflowJobLaunchTemplate(config, project, region, userAgent, d.Get("template_gcs_path").(string), &request)
456+
return updateErr
457+
},
458+
Timeout: time.Minute * time.Duration(5),
459+
ErrorRetryPredicates: []transport_tpg.RetryErrorPredicateFunc{transport_tpg.IsDataflowJobUpdateRetryableError},
460+
})
461+
if err != nil {
462+
return err
463+
}
467464

468-
d.SetId(response.Job.Id)
465+
if err := waitForDataflowJobToBeUpdated(d, config, response.Job.Id, userAgent, d.Timeout(schema.TimeoutUpdate)); err != nil {
466+
return fmt.Errorf("Error updating job with job ID %q: %v", d.Id(), err)
467+
}
469468

469+
d.SetId(response.Job.Id)
470+
}
470471
return resourceDataflowJobRead(d, meta)
471472
}
472473

@@ -649,7 +650,7 @@ func resourceDataflowJobIsVirtualUpdate(d *schema.ResourceData, resourceSchema m
649650
continue
650651
}
651652

652-
if field != "labels" && field != "terraform_labels" && d.HasChange(field) {
653+
if d.HasChange(field) {
653654
return false
654655
}
655656
}
@@ -660,6 +661,25 @@ func resourceDataflowJobIsVirtualUpdate(d *schema.ResourceData, resourceSchema m
660661
return false
661662
}
662663

664+
// If only fields on_delete, terraform_labels are changing, no update request is needed
665+
func jobHasUpdate(d *schema.ResourceData, resourceSchema map[string]*schema.Schema) bool {
666+
if d.HasChange("on_delete") || d.HasChange("labels") || d.HasChange("terraform_labels") {
667+
for field := range resourceSchema {
668+
if field == "on_delete" || field == "labels" || field == "terraform_labels" {
669+
continue
670+
}
671+
672+
if d.HasChange(field) {
673+
return true
674+
}
675+
}
676+
// on_delete, or terraform_labels are changing, but nothing else
677+
return false
678+
}
679+
680+
return true
681+
}
682+
663683
func waitForDataflowJobToBeUpdated(d *schema.ResourceData, config *transport_tpg.Config, replacementJobID, userAgent string, timeout time.Duration) error {
664684
return resource.Retry(timeout, func() *resource.RetryError {
665685
project, err := tpgresource.GetProject(d, config)

‎google/services/dataflow/resource_dataflow_job_test.go

+51-50
Original file line numberDiff line numberDiff line change
@@ -240,12 +240,12 @@ func TestAccDataflowJob_withLabels(t *testing.T) {
240240
{
241241
Config: testAccDataflowJob_labels(bucket, job, key, value),
242242
Check: resource.ComposeTestCheckFunc(
243-
testAccDataflowJobExists(t, "google_dataflow_job.with_labels"),
244-
testAccDataflowJobHasLabels(t, "google_dataflow_job.with_labels", key),
243+
testAccDataflowJobExists(t, "google_dataflow_job.big_data"),
244+
testAccDataflowJobHasLabels(t, "google_dataflow_job.big_data", key),
245245
),
246246
},
247247
{
248-
ResourceName: "google_dataflow_job.with_labels",
248+
ResourceName: "google_dataflow_job.big_data",
249249
ImportState: true,
250250
ImportStateVerify: true,
251251
ImportStateVerifyIgnore: []string{"on_delete", "parameters", "skip_wait_on_job_termination", "state", "labels", "terraform_labels"},
@@ -273,42 +273,42 @@ func TestAccDataflowJob_withProviderDefaultLabels(t *testing.T) {
273273
{
274274
Config: testAccDataflowJob_withProviderDefaultLabels(bucket, job),
275275
Check: resource.ComposeTestCheckFunc(
276-
resource.TestCheckResourceAttr("google_dataflow_job.with_labels", "labels.%", "2"),
277-
resource.TestCheckResourceAttr("google_dataflow_job.with_labels", "labels.env", "foo"),
278-
resource.TestCheckResourceAttr("google_dataflow_job.with_labels", "labels.default_expiration_ms", "3600000"),
276+
resource.TestCheckResourceAttr("google_dataflow_job.big_data", "labels.%", "2"),
277+
resource.TestCheckResourceAttr("google_dataflow_job.big_data", "labels.env", "foo"),
278+
resource.TestCheckResourceAttr("google_dataflow_job.big_data", "labels.default_expiration_ms", "3600000"),
279279

280-
resource.TestCheckResourceAttr("google_dataflow_job.with_labels", "terraform_labels.%", "3"),
281-
resource.TestCheckResourceAttr("google_dataflow_job.with_labels", "terraform_labels.default_key1", "default_value1"),
282-
resource.TestCheckResourceAttr("google_dataflow_job.with_labels", "terraform_labels.env", "foo"),
283-
resource.TestCheckResourceAttr("google_dataflow_job.with_labels", "terraform_labels.default_expiration_ms", "3600000"),
280+
resource.TestCheckResourceAttr("google_dataflow_job.big_data", "terraform_labels.%", "3"),
281+
resource.TestCheckResourceAttr("google_dataflow_job.big_data", "terraform_labels.default_key1", "default_value1"),
282+
resource.TestCheckResourceAttr("google_dataflow_job.big_data", "terraform_labels.env", "foo"),
283+
resource.TestCheckResourceAttr("google_dataflow_job.big_data", "terraform_labels.default_expiration_ms", "3600000"),
284284

285-
resource.TestCheckResourceAttr("google_dataflow_job.with_labels", "effective_labels.%", "6"),
285+
resource.TestCheckResourceAttr("google_dataflow_job.big_data", "effective_labels.%", "6"),
286286
),
287287
},
288288
{
289-
ResourceName: "google_dataflow_job.with_labels",
289+
ResourceName: "google_dataflow_job.big_data",
290290
ImportState: true,
291291
ImportStateVerify: true,
292292
ImportStateVerifyIgnore: []string{"on_delete", "parameters", "skip_wait_on_job_termination", "state", "labels", "terraform_labels"},
293293
},
294294
{
295295
Config: testAccDataflowJob_resourceLabelsOverridesProviderDefaultLabels(bucket, job),
296296
Check: resource.ComposeTestCheckFunc(
297-
resource.TestCheckResourceAttr("google_dataflow_job.with_labels", "labels.%", "3"),
298-
resource.TestCheckResourceAttr("google_dataflow_job.with_labels", "labels.env", "foo"),
299-
resource.TestCheckResourceAttr("google_dataflow_job.with_labels", "labels.default_expiration_ms", "3600000"),
300-
resource.TestCheckResourceAttr("google_dataflow_job.with_labels", "labels.default_key1", "value1"),
297+
resource.TestCheckResourceAttr("google_dataflow_job.big_data", "labels.%", "3"),
298+
resource.TestCheckResourceAttr("google_dataflow_job.big_data", "labels.env", "foo"),
299+
resource.TestCheckResourceAttr("google_dataflow_job.big_data", "labels.default_expiration_ms", "3600000"),
300+
resource.TestCheckResourceAttr("google_dataflow_job.big_data", "labels.default_key1", "value1"),
301301

302-
resource.TestCheckResourceAttr("google_dataflow_job.with_labels", "terraform_labels.%", "3"),
303-
resource.TestCheckResourceAttr("google_dataflow_job.with_labels", "terraform_labels.default_key1", "value1"),
304-
resource.TestCheckResourceAttr("google_dataflow_job.with_labels", "terraform_labels.env", "foo"),
305-
resource.TestCheckResourceAttr("google_dataflow_job.with_labels", "terraform_labels.default_expiration_ms", "3600000"),
302+
resource.TestCheckResourceAttr("google_dataflow_job.big_data", "terraform_labels.%", "3"),
303+
resource.TestCheckResourceAttr("google_dataflow_job.big_data", "terraform_labels.default_key1", "value1"),
304+
resource.TestCheckResourceAttr("google_dataflow_job.big_data", "terraform_labels.env", "foo"),
305+
resource.TestCheckResourceAttr("google_dataflow_job.big_data", "terraform_labels.default_expiration_ms", "3600000"),
306306

307-
resource.TestCheckResourceAttr("google_dataflow_job.with_labels", "effective_labels.%", "6"),
307+
resource.TestCheckResourceAttr("google_dataflow_job.big_data", "effective_labels.%", "6"),
308308
),
309309
},
310310
{
311-
ResourceName: "google_dataflow_job.with_labels",
311+
ResourceName: "google_dataflow_job.big_data",
312312
ImportState: true,
313313
ImportStateVerify: true,
314314
// The labels field in the state is decided by the configuration.
@@ -318,57 +318,58 @@ func TestAccDataflowJob_withProviderDefaultLabels(t *testing.T) {
318318
{
319319
Config: testAccDataflowJob_moveResourceLabelToProviderDefaultLabels(bucket, job),
320320
Check: resource.ComposeTestCheckFunc(
321-
resource.TestCheckResourceAttr("google_dataflow_job.with_labels", "labels.%", "2"),
322-
resource.TestCheckResourceAttr("google_dataflow_job.with_labels", "labels.default_expiration_ms", "3600000"),
323-
resource.TestCheckResourceAttr("google_dataflow_job.with_labels", "labels.default_key1", "value1"),
321+
resource.TestCheckResourceAttr("google_dataflow_job.big_data", "labels.%", "2"),
322+
resource.TestCheckResourceAttr("google_dataflow_job.big_data", "labels.default_expiration_ms", "3600000"),
323+
resource.TestCheckResourceAttr("google_dataflow_job.big_data", "labels.default_key1", "value1"),
324324

325-
resource.TestCheckResourceAttr("google_dataflow_job.with_labels", "terraform_labels.%", "3"),
326-
resource.TestCheckResourceAttr("google_dataflow_job.with_labels", "terraform_labels.default_key1", "value1"),
327-
resource.TestCheckResourceAttr("google_dataflow_job.with_labels", "terraform_labels.env", "foo"),
328-
resource.TestCheckResourceAttr("google_dataflow_job.with_labels", "terraform_labels.default_expiration_ms", "3600000"),
325+
resource.TestCheckResourceAttr("google_dataflow_job.big_data", "terraform_labels.%", "3"),
326+
resource.TestCheckResourceAttr("google_dataflow_job.big_data", "terraform_labels.default_key1", "value1"),
327+
resource.TestCheckResourceAttr("google_dataflow_job.big_data", "terraform_labels.env", "foo"),
328+
resource.TestCheckResourceAttr("google_dataflow_job.big_data", "terraform_labels.default_expiration_ms", "3600000"),
329329

330-
resource.TestCheckResourceAttr("google_dataflow_job.with_labels", "effective_labels.%", "6"),
330+
resource.TestCheckResourceAttr("google_dataflow_job.big_data", "effective_labels.%", "6"),
331331
),
332332
},
333333
{
334-
ResourceName: "google_dataflow_job.with_labels",
334+
ResourceName: "google_dataflow_job.big_data",
335335
ImportState: true,
336336
ImportStateVerify: true,
337337
ImportStateVerifyIgnore: []string{"on_delete", "parameters", "skip_wait_on_job_termination", "state", "labels", "terraform_labels"},
338338
},
339339
{
340340
Config: testAccDataflowJob_resourceLabelsOverridesProviderDefaultLabels(bucket, job),
341341
Check: resource.ComposeTestCheckFunc(
342-
resource.TestCheckResourceAttr("google_dataflow_job.with_labels", "labels.%", "3"),
343-
resource.TestCheckResourceAttr("google_dataflow_job.with_labels", "labels.env", "foo"),
344-
resource.TestCheckResourceAttr("google_dataflow_job.with_labels", "labels.default_expiration_ms", "3600000"),
345-
resource.TestCheckResourceAttr("google_dataflow_job.with_labels", "labels.default_key1", "value1"),
342+
resource.TestCheckResourceAttr("google_dataflow_job.big_data", "labels.%", "3"),
343+
resource.TestCheckResourceAttr("google_dataflow_job.big_data", "labels.env", "foo"),
344+
resource.TestCheckResourceAttr("google_dataflow_job.big_data", "labels.default_expiration_ms", "3600000"),
345+
resource.TestCheckResourceAttr("google_dataflow_job.big_data", "labels.default_key1", "value1"),
346346

347-
resource.TestCheckResourceAttr("google_dataflow_job.with_labels", "terraform_labels.%", "3"),
348-
resource.TestCheckResourceAttr("google_dataflow_job.with_labels", "terraform_labels.default_key1", "value1"),
349-
resource.TestCheckResourceAttr("google_dataflow_job.with_labels", "terraform_labels.env", "foo"),
350-
resource.TestCheckResourceAttr("google_dataflow_job.with_labels", "terraform_labels.default_expiration_ms", "3600000"),
347+
resource.TestCheckResourceAttr("google_dataflow_job.big_data", "terraform_labels.%", "3"),
348+
resource.TestCheckResourceAttr("google_dataflow_job.big_data", "terraform_labels.default_key1", "value1"),
349+
resource.TestCheckResourceAttr("google_dataflow_job.big_data", "terraform_labels.env", "foo"),
350+
resource.TestCheckResourceAttr("google_dataflow_job.big_data", "terraform_labels.default_expiration_ms", "3600000"),
351351

352-
resource.TestCheckResourceAttr("google_dataflow_job.with_labels", "effective_labels.%", "6"),
352+
resource.TestCheckResourceAttr("google_dataflow_job.big_data", "effective_labels.%", "6"),
353353
),
354354
},
355355
{
356-
ResourceName: "google_dataflow_job.with_labels",
356+
ResourceName: "google_dataflow_job.big_data",
357357
ImportState: true,
358358
ImportStateVerify: true,
359359
ImportStateVerifyIgnore: []string{"on_delete", "parameters", "skip_wait_on_job_termination", "state", "labels", "terraform_labels"},
360360
},
361361
{
362362
Config: testAccDataflowJob_zone(bucket, job, zone),
363363
Check: resource.ComposeTestCheckFunc(
364-
resource.TestCheckNoResourceAttr("google_dataflow_job.with_labels", "labels.%"),
365-
resource.TestCheckResourceAttr("google_dataflow_job.with_labels", "effective_labels.%", "3"),
364+
resource.TestCheckNoResourceAttr("google_dataflow_job.big_data", "labels.%"),
365+
resource.TestCheckResourceAttr("google_dataflow_job.big_data", "effective_labels.%", "3"),
366366
),
367367
},
368368
{
369-
ResourceName: "google_dataflow_job.with_labels",
370-
ImportState: true,
371-
ImportStateVerify: true,
369+
ResourceName: "google_dataflow_job.big_data",
370+
ImportState: true,
371+
ImportStateVerify: true,
372+
ImportStateVerifyIgnore: []string{"on_delete", "parameters", "skip_wait_on_job_termination", "zone", "state"},
372373
},
373374
},
374375
})
@@ -1073,7 +1074,7 @@ resource "google_storage_bucket" "temp" {
10731074
force_destroy = true
10741075
}
10751076
1076-
resource "google_dataflow_job" "with_labels" {
1077+
resource "google_dataflow_job" "big_data" {
10771078
name = "%s"
10781079
10791080
labels = {
@@ -1105,7 +1106,7 @@ resource "google_storage_bucket" "temp" {
11051106
force_destroy = true
11061107
}
11071108
1108-
resource "google_dataflow_job" "with_labels" {
1109+
resource "google_dataflow_job" "big_data" {
11091110
name = "%s"
11101111
11111112
labels = {
@@ -1138,7 +1139,7 @@ resource "google_storage_bucket" "temp" {
11381139
force_destroy = true
11391140
}
11401141
1141-
resource "google_dataflow_job" "with_labels" {
1142+
resource "google_dataflow_job" "big_data" {
11421143
name = "%s"
11431144
11441145
labels = {
@@ -1173,7 +1174,7 @@ resource "google_storage_bucket" "temp" {
11731174
force_destroy = true
11741175
}
11751176
1176-
resource "google_dataflow_job" "with_labels" {
1177+
resource "google_dataflow_job" "big_data" {
11771178
name = "%s"
11781179
11791180
labels = {

0 commit comments

Comments
 (0)