dataflow job update-by-replacement #3387

c2thorn · 2020-04-18T00:02:56Z

Fixes hashicorp/terraform-provider-google#5991

Adds update-by-replacement method
Separates ForceNew functionality based on JOB_TYPE
Triggers diffs based on reading the new template_md5hash

Release Note Template for Downstream PRs (will be copied)

dataflow: Added support for update-by-replacement to `google_dataflow_job`

dataflow: Added drift detection for `google_dataflow_job` `template_gcs_path` and `temp_gcs_location` fields

modular-magician · 2020-04-18T02:26:35Z

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 2 files changed, 198 insertions(+), 13 deletions(-))
Terraform Beta: Diff ( 2 files changed, 198 insertions(+), 13 deletions(-))

modular-magician · 2020-04-20T16:49:03Z

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 3 files changed, 205 insertions(+), 13 deletions(-))
Terraform Beta: Diff ( 3 files changed, 205 insertions(+), 13 deletions(-))

modular-magician · 2020-04-20T17:01:56Z

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 3 files changed, 206 insertions(+), 13 deletions(-))
Terraform Beta: Diff ( 3 files changed, 206 insertions(+), 13 deletions(-))

modular-magician · 2020-04-20T17:02:20Z

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 3 files changed, 206 insertions(+), 13 deletions(-))
Terraform Beta: Diff ( 3 files changed, 206 insertions(+), 13 deletions(-))

third_party/terraform/resources/resource_dataflow_job.go

danawillow · 2020-04-20T19:35:10Z

third_party/terraform/resources/resource_dataflow_job.go

+	params := expandStringMap(d, "parameters")
+	labels := expandStringMap(d, "labels")
+
+	env := dataflow.RuntimeEnvironment{


can this be made into a function that can be called from both create and update?

third_party/terraform/resources/resource_dataflow_job.go

third_party/terraform/tests/resource_dataflow_job_test.go

third_party/terraform/resources/resource_dataflow_job.go

third_party/terraform/website/docs/r/dataflow_job.html.markdown

modular-magician · 2020-04-20T20:10:16Z

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 3 files changed, 206 insertions(+), 13 deletions(-))
Terraform Beta: Diff ( 3 files changed, 206 insertions(+), 13 deletions(-))

modular-magician · 2020-04-20T20:49:30Z

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 3 files changed, 217 insertions(+), 29 deletions(-))
Terraform Beta: Diff ( 3 files changed, 217 insertions(+), 29 deletions(-))

danawillow

For the changelog, I think (but you'll want to verify) you can actually separate it out into two different release note blocks. That way you could do one enhancement block for the new changes, and another note or breaking-change block for the other stuff.

danawillow · 2020-04-21T16:48:48Z

third_party/terraform/tests/resource_dataflow_job_test.go

@@ -182,6 +183,35 @@ func TestAccDataflowJob_withIpConfig(t *testing.T) {
 	})
 }

+func TestAccDataflowJob_streamUpdate(t *testing.T) {


Oh, how possible would it be to do one that tests the contents of the template changing?

Should be possible, and a good test to add. I'll cook one up.

modular-magician · 2020-04-21T23:50:03Z

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 3 files changed, 215 insertions(+), 27 deletions(-))
Terraform Beta: Diff ( 3 files changed, 215 insertions(+), 27 deletions(-))

modular-magician · 2020-04-29T18:15:15Z

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 3 files changed, 192 insertions(+), 13 deletions(-))
Terraform Beta: Diff ( 3 files changed, 192 insertions(+), 13 deletions(-))
TF Conversion: Diff ( 1 file changed, 9 insertions(+))

modular-magician · 2020-04-29T18:32:38Z

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 3 files changed, 198 insertions(+), 13 deletions(-))
Terraform Beta: Diff ( 3 files changed, 198 insertions(+), 13 deletions(-))
TF Conversion: Diff ( 1 file changed, 9 insertions(+))

modular-magician · 2020-04-29T21:51:08Z

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 3 files changed, 214 insertions(+), 32 deletions(-))
Terraform Beta: Diff ( 3 files changed, 214 insertions(+), 32 deletions(-))
TF Conversion: Diff ( 1 file changed, 9 insertions(+))

c2thorn · 2020-04-29T22:00:05Z

Reverted all of the template hash logic, modified the update test, and added a retry to the update-by-replacement call.

danawillow

LGTM assuming tests pass

danawillow · 2020-04-30T18:32:27Z

third_party/terraform/resources/resource_dataflow_job.go

+		return err
+	}
+	optionsMap := sdkPipelineOptions["options"].(map[string]interface{})
+	d.Set("template_gcs_path", optionsMap["templateLocation"])


Can you add to the CHANGELOG that we now detect drift on these fields?

danawillow · 2020-04-30T18:38:08Z

third_party/terraform/tests/resource_dataflow_job_test.go

+	}
+	on_delete = "cancel"
+}
+  `, suffix, suffix, suffix, suffix, testDataflowJobTemplateTextToPubsub, tempLocation)


Should the last two params here be switched?

The field names are confusing, but it is correct as it is. template_gcs_path needs to remain fixed, temp_gcs_location is the field I am actually updating.

c2thorn · 2020-04-30T20:11:18Z

Ran all dataflow tests in TC, looks good.

jcanseco · 2020-05-05T17:57:02Z

third_party/terraform/resources/resource_dataflow_job.go

@@ -100,13 +103,13 @@ func resourceDataflowJob() *schema.Resource {
 				ValidateFunc: validation.StringInSlice([]string{"cancel", "drain"}, false),


Hi @c2thorn, does this mean that changes to on_delete would also trigger an update-by-replacement?

Hi, yes that is true. Previously any changes to on_delete would destroy/recreate both stream or batch jobs. Now it will trigger the update-by-replacement for streaming jobs, while retaining the same destroy/recreate functionality for batch.

Oh that's interesting that we used to ForceNew on that! It probably should be something that can be updatable completely on its own, without triggering a ForceNew or an update-by-replacement.

Ah yeah I was surprised to see that it used to be ForceNew as well.

I agree that this field should be updatable on its own without triggering a ForceNew or update-by-replacement. Would you like me to file this as a separate issue?

Created hashicorp/terraform-provider-google#6301 to track

googlebot added the cla: yes label Apr 18, 2020

c2thorn force-pushed the dataflow-update-by-replacement branch from b8ce119 to 1123f40 Compare April 18, 2020 02:21

c2thorn requested a review from danawillow April 20, 2020 17:10

c2thorn commented Apr 20, 2020

View reviewed changes

third_party/terraform/resources/resource_dataflow_job.go Show resolved Hide resolved

danawillow suggested changes Apr 20, 2020

View reviewed changes

c2thorn requested a review from danawillow April 20, 2020 23:56

danawillow reviewed Apr 21, 2020

View reviewed changes

Dataflow update-by-replacement

b479f2d

c2thorn force-pushed the dataflow-update-by-replacement branch from 3f6bdb5 to b479f2d Compare April 29, 2020 18:09

Return errors

83f5363

added resourceDataflowJobSetupEnv, removed forceNew from new field

0b069a3

c2thorn requested a review from danawillow April 29, 2020 22:00

danawillow approved these changes Apr 30, 2020

View reviewed changes

c2thorn merged commit ced466b into GoogleCloudPlatform:master Apr 30, 2020

c2thorn deleted the dataflow-update-by-replacement branch April 30, 2020 20:11

jcanseco reviewed May 5, 2020

View reviewed changes

nathkn pushed a commit to nathkn/magic-modules that referenced this pull request May 18, 2020

dataflow job update-by-replacement (GoogleCloudPlatform#3387)

dc352ae

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

dataflow job update-by-replacement #3387

dataflow job update-by-replacement #3387

c2thorn commented Apr 18, 2020 •

edited

Loading

modular-magician commented Apr 18, 2020

modular-magician commented Apr 20, 2020

modular-magician commented Apr 20, 2020

modular-magician commented Apr 20, 2020

danawillow Apr 20, 2020

modular-magician commented Apr 20, 2020

modular-magician commented Apr 20, 2020

danawillow left a comment

danawillow Apr 21, 2020

c2thorn Apr 21, 2020

modular-magician commented Apr 21, 2020

modular-magician commented Apr 29, 2020

modular-magician commented Apr 29, 2020

modular-magician commented Apr 29, 2020

c2thorn commented Apr 29, 2020

danawillow left a comment

danawillow Apr 30, 2020

danawillow Apr 30, 2020

c2thorn Apr 30, 2020

c2thorn commented Apr 30, 2020

jcanseco May 5, 2020

c2thorn May 5, 2020 •

edited

Loading

danawillow May 5, 2020

jcanseco May 5, 2020 •

edited

Loading

c2thorn May 5, 2020

jcanseco May 5, 2020

		@@ -100,13 +103,13 @@ func resourceDataflowJob() *schema.Resource {
		ValidateFunc: validation.StringInSlice([]string{"cancel", "drain"}, false),

dataflow job update-by-replacement #3387

dataflow job update-by-replacement #3387

Conversation

c2thorn commented Apr 18, 2020 • edited Loading

modular-magician commented Apr 18, 2020

Diff report:

modular-magician commented Apr 20, 2020

Diff report:

modular-magician commented Apr 20, 2020

Diff report:

modular-magician commented Apr 20, 2020

Diff report:

Choose a reason for hiding this comment

modular-magician commented Apr 20, 2020

Diff report:

modular-magician commented Apr 20, 2020

Diff report:

danawillow left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

modular-magician commented Apr 21, 2020

Diff report:

modular-magician commented Apr 29, 2020

Diff report:

modular-magician commented Apr 29, 2020

Diff report:

modular-magician commented Apr 29, 2020

Diff report:

c2thorn commented Apr 29, 2020

danawillow left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

c2thorn commented Apr 30, 2020

Choose a reason for hiding this comment

c2thorn May 5, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jcanseco May 5, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

c2thorn commented Apr 18, 2020 •

edited

Loading

c2thorn May 5, 2020 •

edited

Loading

jcanseco May 5, 2020 •

edited

Loading