Compute - Add update support for Network IP when changing network/subnetwork #4030

ScottSuarez · 2020-09-29T01:00:45Z

Compute - add support for changing network ip when moving network/subnetwork only
Fixed a scenario where changes access configs at the same time as network could result in an incompatible accessconfig being applied with the previous subnetwork.

Closes 7118

If this PR is for Terraform, I acknowledge that I have:

Searched through the issue tracker for an open issue that this either resolves or contributes to, commented on it to claim it, and written "fixes {url}" or "part of {url}" in this PR description. If there were no relevant open issues, I opened one and commented that I would like to work on it (not necessary for very small changes).
Generated Terraform, and ran make test and make lint to ensure it passes unit and linter tests.
Ensured that all new fields I added that can be set by a user appear in at least one example (for generated resources) or third_party test (for handwritten resources or update tests).
Ran relevant acceptance tests (If the acceptance tests do not yet pass or you are unable to run them, please let your reviewer know).
Read the Release Notes Guide before writing my release note below.

Release Note Template for Downstream PRs (will be copied)

compute: added support for updating `network_interface.[d].network_ip` on `google_compute_instance` when changing network or subnetwork

…network

modular-magician · 2020-09-29T01:08: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, 148 insertions(+), 46 deletions(-))
Terraform Beta: Diff ( 2 files changed, 148 insertions(+), 46 deletions(-))

modular-magician · 2020-09-29T01:08:42Z

I have triggered VCR tests based on this PR's diffs. See the results here: "https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=149271"

modular-magician · 2020-09-29T01:26:00Z

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

Diff report:

Terraform GA: Diff ( 2 files changed, 148 insertions(+), 46 deletions(-))
Terraform Beta: Diff ( 2 files changed, 148 insertions(+), 46 deletions(-))

modular-magician · 2020-09-29T01:26:07Z

I have triggered VCR tests based on this PR's diffs. See the results here: "https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=149271"

modular-magician · 2020-09-29T01:37:59Z

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

Diff report:

Terraform GA: Diff ( 2 files changed, 150 insertions(+), 46 deletions(-))
Terraform Beta: Diff ( 2 files changed, 150 insertions(+), 46 deletions(-))

modular-magician · 2020-09-29T01:38:05Z

I have triggered VCR tests based on this PR's diffs. See the results here: "https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=149271"

c2thorn

mostly suggestions

third_party/terraform/tests/resource_compute_instance_test.go.erb

third_party/terraform/resources/resource_compute_instance.go.erb

danawillow · 2020-09-29T18:30:49Z

also cc @ndmckinley who at one point was looking into what it would take to generate instance- nathan, not sure if you ran into trying to update network interfaces yet but there's a lot of different cases that require different handling, and the new update method won't help in this case (https://cloud.google.com/compute/docs/instances/update-instance-properties#updatable-properties)

ScottSuarez

Fixed some issues. Refactored code

third_party/terraform/resources/resource_compute_instance.go.erb

third_party/terraform/tests/resource_compute_instance_test.go.erb

modular-magician · 2020-09-30T00:46:53Z

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

Diff report:

Terraform GA: Diff ( 3 files changed, 260 insertions(+), 114 deletions(-))
Terraform Beta: Diff ( 3 files changed, 260 insertions(+), 114 deletions(-))

modular-magician · 2020-09-30T00:47:00Z

I have triggered VCR tests based on this PR's diffs. See the results here: "https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=149477"

modular-magician · 2020-09-30T02:59:31Z

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

Diff report:

Terraform GA: Diff ( 3 files changed, 260 insertions(+), 114 deletions(-))
Terraform Beta: Diff ( 3 files changed, 260 insertions(+), 114 deletions(-))

modular-magician · 2020-09-30T02:59:40Z

I have triggered VCR tests based on this PR's diffs. See the results here: "https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=149480"

modular-magician · 2020-09-30T03:52:27Z

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

Diff report:

Terraform GA: Diff ( 3 files changed, 260 insertions(+), 114 deletions(-))
Terraform Beta: Diff ( 3 files changed, 260 insertions(+), 114 deletions(-))

modular-magician · 2020-09-30T03:52:35Z

I have triggered VCR tests based on this PR's diffs. See the results here: "https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=149481"

modular-magician · 2020-09-30T04:23: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, 261 insertions(+), 112 deletions(-))
Terraform Beta: Diff ( 3 files changed, 261 insertions(+), 112 deletions(-))

ScottSuarez · 2020-10-09T09:30:58Z

I've made changes now to extrapolate functions when there is either code reuse or to avoid the creation of an inline function. I'm rather happy with this result.

These should be the changes that in essence are the pure functionality improvements. I've scrapped the refactor and the concept of network interface helper. Thanks for your time and assistance here!

modular-magician · 2020-10-09T09:38:37Z

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

Diff report:

Terraform GA: Diff ( 5 files changed, 303 insertions(+), 60 deletions(-))
Terraform Beta: Diff ( 5 files changed, 303 insertions(+), 60 deletions(-))
TF Conversion: Diff ( 1 file changed, 1 insertion(+))

modular-magician · 2020-10-09T09:38:44Z

I have triggered VCR tests based on this PR's diffs. See the results here: "https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=151409"

modular-magician · 2020-10-09T10:43:43Z

I have triggered VCR tests in RECORDING mode for the following tests that failed during VCR: TestAccProviderMeta_setModuleName You can view the result here: "https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=151411"

danawillow

Getting there! Most of my comments are pretty small at this point, and sorry again for the back-and-forth on the structure/refactoring part.

third_party/terraform/resources/resource_compute_instance.go.erb

third_party/terraform/tests/resource_compute_instance_test.go.erb

third_party/terraform/utils/compute_instance_network_interface_helpers.go

ScottSuarez

Resolved formatting and comment concerns

third_party/terraform/utils/compute_instance_network_interface_helpers.go

third_party/terraform/tests/resource_compute_instance_test.go.erb

third_party/terraform/resources/resource_compute_instance.go.erb

third_party/terraform/utils/compute_instance_network_interface_helpers.go

ScottSuarez · 2020-10-09T17:45:17Z

Getting there! Most of my comments are pretty small at this point, and sorry again for the back-and-forth on the structure/refactoring part.

No worries ! I learned a lot between these refactors. I appreciate your dedication to getting things right and not just getting them in.

modular-magician · 2020-10-09T17:51:40Z

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

Diff report:

Terraform GA: Diff ( 5 files changed, 331 insertions(+), 80 deletions(-))
Terraform Beta: Diff ( 5 files changed, 344 insertions(+), 93 deletions(-))
TF Conversion: Diff ( 1 file changed, 1 insertion(+))

modular-magician · 2020-10-09T17:51:48Z

I have triggered VCR tests based on this PR's diffs. See the results here: "https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=151424"

danawillow

LGTM assuming tests pass!

third_party/terraform/resources/resource_compute_instance.go.erb

…comment friends. Truly truly a beautiful site to behold. Please be well and safe comment because the world needs you. YOU TOO are important

modular-magician · 2020-10-09T18:06:22Z

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

Diff report:

Terraform GA: Diff ( 5 files changed, 326 insertions(+), 74 deletions(-))
Terraform Beta: Diff ( 5 files changed, 339 insertions(+), 87 deletions(-))
TF Conversion: Diff ( 1 file changed, 1 insertion(+))

modular-magician · 2020-10-09T18:06:32Z

I have triggered VCR tests based on this PR's diffs. See the results here: "https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=151427"

modular-magician · 2020-10-09T21:35:23Z

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

Diff report:

Terraform GA: Diff ( 5 files changed, 326 insertions(+), 74 deletions(-))
Terraform Beta: Diff ( 5 files changed, 339 insertions(+), 87 deletions(-))
TF Conversion: Diff ( 1 file changed, 1 insertion(+))

modular-magician · 2020-10-09T21:35:32Z

I have triggered VCR tests based on this PR's diffs. See the results here: "https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=151459"

…-modules into compute_instance_network_ip

modular-magician · 2020-10-12T20:38:28Z

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

Diff report:

Terraform GA: Diff ( 5 files changed, 326 insertions(+), 74 deletions(-))
Terraform Beta: Diff ( 5 files changed, 339 insertions(+), 87 deletions(-))
TF Conversion: Diff ( 1 file changed, 1 insertion(+))

modular-magician · 2020-10-12T20:38:39Z

I have triggered VCR tests based on this PR's diffs. See the results here: "https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=151763"

modular-magician · 2020-10-12T22:16:14Z

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

Diff report:

Terraform GA: Diff ( 5 files changed, 304 insertions(+), 54 deletions(-))
Terraform Beta: Diff ( 5 files changed, 304 insertions(+), 54 deletions(-))
TF Conversion: Diff ( 1 file changed, 1 insertion(+))

modular-magician · 2020-10-12T22:16:21Z

I have triggered VCR tests based on this PR's diffs. See the results here: "https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=151775"

modular-magician · 2020-10-12T23:20:50Z

I have triggered VCR tests in RECORDING mode for the following tests that failed during VCR: TestAccDataSourceGoogleSQLCaCerts_basic|TestAccDataSourceSqlDatabaseInstance_basic|TestAccProviderMeta_setModuleName|TestAccCloudRunService_cloudRunServiceSqlExample|TestAccProjectServiceIdentity_basic|TestAccSQLDatabase_sqlDatabaseBasicExample You can view the result here: "https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=151781"

ScottSuarez added 2 commits September 28, 2020 17:54

Compute - Add update support for Network IP when changing network/sub…

8fa2b40

…network

merge with master

b81129f

ScottSuarez requested review from danawillow and c2thorn September 29, 2020 01:00

google-cla bot added the cla: yes label Sep 29, 2020

add field required by beta provider

c758f19

error check for d.ForceNew

be26563

c2thorn requested changes Sep 29, 2020

View reviewed changes

danawillow suggested changes Sep 29, 2020

View reviewed changes

refactor functions into own file. introduce network-interface-helper

d43335b

ScottSuarez commented Sep 30, 2020

View reviewed changes

ScottSuarez added 2 commits September 29, 2020 17:21

refresh instance after deleting alias config

8de60c0

merge with master

305fad0

correct bad merge

d6a1176

spelling fix

465b8d5

check error

566c7ba

error check

32e332e

danawillow suggested changes Oct 9, 2020

View reviewed changes

resolved some fomatting and comment concerns.

a2d114f

ScottSuarez commented Oct 9, 2020

View reviewed changes

danawillow approved these changes Oct 9, 2020

View reviewed changes

third_party/terraform/resources/resource_compute_instance.go.erb Show resolved Hide resolved

find a comment a nice cozy new home on the hill side amongst all its …

a6adabc

…comment friends. Truly truly a beautiful site to behold. Please be well and safe comment because the world needs you. YOU TOO are important

a comma boi

d4ac363

Merge branch 'master' of https://github.com/GoogleCloudPlatform/magic…

29ee2ff

…-modules into compute_instance_network_ip

another commma for the party

8c800ac

ScottSuarez merged commit 9055425 into GoogleCloudPlatform:master Oct 13, 2020

ScottSuarez mentioned this pull request Dec 15, 2020

Allow changes to network_interface on compute_instance resource hashicorp/terraform-provider-google#7931

Closed

Compute - Add update support for Network IP when changing network/subnetwork #4030

Compute - Add update support for Network IP when changing network/subnetwork #4030

Conversation

ScottSuarez commented Sep 29, 2020 • edited Loading

modular-magician commented Sep 29, 2020

Diff report:

modular-magician commented Sep 29, 2020

modular-magician commented Sep 29, 2020

Diff report:

modular-magician commented Sep 29, 2020

modular-magician commented Sep 29, 2020

Diff report:

modular-magician commented Sep 29, 2020

c2thorn left a comment

Choose a reason for hiding this comment

danawillow commented Sep 29, 2020

ScottSuarez left a comment

Choose a reason for hiding this comment

modular-magician commented Sep 30, 2020

Diff report:

modular-magician commented Sep 30, 2020

modular-magician commented Sep 30, 2020

Diff report:

modular-magician commented Sep 30, 2020

modular-magician commented Sep 30, 2020

Diff report:

modular-magician commented Sep 30, 2020

modular-magician commented Sep 30, 2020

Diff report:

ScottSuarez commented Oct 9, 2020

modular-magician commented Oct 9, 2020

Diff report:

modular-magician commented Oct 9, 2020

modular-magician commented Oct 9, 2020

danawillow left a comment

Choose a reason for hiding this comment

ScottSuarez left a comment

Choose a reason for hiding this comment

ScottSuarez commented Oct 9, 2020

modular-magician commented Oct 9, 2020

Diff report:

modular-magician commented Oct 9, 2020

danawillow left a comment

Choose a reason for hiding this comment

modular-magician commented Oct 9, 2020

Diff report:

modular-magician commented Oct 9, 2020

modular-magician commented Oct 9, 2020

Diff report:

modular-magician commented Oct 9, 2020

modular-magician commented Oct 12, 2020

Diff report:

modular-magician commented Oct 12, 2020

modular-magician commented Oct 12, 2020

Diff report:

modular-magician commented Oct 12, 2020

modular-magician commented Oct 12, 2020

ScottSuarez commented Sep 29, 2020 •

edited

Loading