Don't recreate GCE instances when updating resource_policies property

[hanneshayashi]

This fixes the behaviour described in hashicorp/terraform-provider-google#9981.
Basically, the change removes the ForceNew attribute from the resource_policies property of google_compute_instance and adds code to check for changes of the property. If the property changed, all old values get removed and all new values (if there are any) get added.
The change was previously submitted in hashicorp/terraform-provider-google#10029

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: Fixed recreation of GCE instances when updating `resource_policies` property

Regarding tests: There doesn't seem to be a test specific for this behaviour. Do you need me to create one?

[google-cla]

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.

---

What to do if you already signed the CLA

Individual signers

• It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.

Corporate signers

• Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
• The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
• The email used to register you as an authorized contributor must also be attached to your GitHub account.

ℹ️ Googlers: Go here for more info.

[modular-magician]

Oops! It looks like you're using an unknown release-note type in your changelog entries:

• enhancement:

Please only use the types listed in https://github.com/GoogleCloudPlatform/magic-modules/blob/master/.ci/RELEASE_NOTES_GUIDE.md.

[modular-magician]

Hello! I am a robot who works on Magic Modules PRs.

I have detected that you are a community contributor, so your PR will be assigned to someone with a commit-bit on this repo for initial review.

Thanks for your contribution! A human will be with you soon.

@rileykarson, please review this PR or find an appropriate assignee.

[modular-magician]

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

Diff report:

Terraform GA: Diff ( 1 file changed, 32 insertions(+), 2 deletions(-))
Terraform Beta: Diff ( 1 file changed, 32 insertions(+), 2 deletions(-))

[hanneshayashi]

@googlebot I signed it!

[nat-henderson]

Great! Let's run those tests. /gcbrun

[nat-henderson]

Great, all those tests pass. We're ready to merge as soon as our CI pipeline is back up and healthy. :)

[nat-henderson]

@hanneshayashi Are you able to add an update test for this, or maybe to show that you've tested it and it updates correctly?

mmv1/third_party/terraform/tests/resource_compute_instance_test.go.erb would be where to add the test if you're able.

[hanneshayashi]

Sure, I'll give it a try.

[hanneshayashi]

@ndmckinley I just added a test case that creates a new instance and an instance scheduler. The test has steps to add, update and remove the resource policy from the instance. I sincerely hope that I didn't mess anything up with the Magic Modules stuff. Just in case, I also pushed the test to the original PR (hashicorp/terraform-provider-google#10029).
Obviously I ran the tests locally and they pass for me. Same goes for the actual change.
Please let me know if there's anything else you need from me!

[modular-magician]

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

Diff report:

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

[nat-henderson]

Looks great, let's run those tests. /gcbrun

[modular-magician]

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

Diff report:

Terraform GA: Diff ( 2 files changed, 289 insertions(+), 2 deletions(-))
Terraform Beta: Diff ( 3 files changed, 290 insertions(+), 4 deletions(-))

[nat-henderson]

Ah, CI node unhealthy, those happen intermittently. Let's try again. /gcbrun

[modular-magician]

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

Diff report:

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

[nat-henderson]

Two in a row! PR's full of bad luck. /gcbrun one more try.

[modular-magician]

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

Diff report:

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

[hanneshayashi]

Thanks for running the tests! Assuming the current failure is not due to my code and that this change can be merged soon-ish, what version can the change reasonably be expected to be included in?

[nat-henderson]

/gcbrun

[modular-magician]

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

Diff report:

Terraform GA: Diff ( 2 files changed, 289 insertions(+), 2 deletions(-))
Terraform Beta: Diff ( 3 files changed, 290 insertions(+), 3 deletions(-))

[nat-henderson]

Thanks for running the tests! Assuming the current failure is not due to my code and that this change can be merged soon-ish, what version can the change reasonably be expected to be included in?

Hashicorp CI still seems to be down, but I'll keep trying - if this goes in today, then it will go out on the 27th - otherwise, the 4th. Release cuts are usually Tuesday, for release in the afternoon PST on the following Monday (excepting holidays, surprises, bad luck - we're pretty consistent but not absolutely perfect about it).

[modular-magician]

I have triggered VCR tests in RECORDING mode for the following tests that failed during VCR: TestAccBigQueryDataTable_bigtable|TestAccBigtableAppProfile_bigtableAppProfileSingleclusterExample|TestAccBigtableAppProfile_bigtableAppProfileMulticlusterExample|TestAccComputeForwardingRule_update|TestAccComputeInstance_resourcePolicyUpdate|TestAccComputeServiceAttachment_serviceAttachmentBasicExample|TestAccOrgPolicyPolicy_EnforcePolicy|TestAccOrgPolicyPolicy_FolderPolicy|TestAccOrgPolicyPolicy_OrganizationPolicy|TestAccOrgPolicyPolicy_ProjectPolicy|TestAccTags You can view the result here: https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=206728

[nat-henderson]

@hanneshayashi Okay! I have the tests running and I have verified there are no new failures. However, the test you added is causing this failure:

Error: Error adding resource policies: googleapi: Error 412: Compute Engine System service account service-1067888929963@compute-system.iam.gserviceaccount.com needs to have [compute.instances.start,compute.instances.stop] permissions applied in order to perform this operation., conditionNotMet
on terraform_plugin_test.tf line 7, in resource "google_compute_instance" "foobar":
7: resource "google_compute_instance" "foobar" {
--- FAIL: TestAccComputeInstance_resourcePolicyUpdate (65.31s)

Which must be a permissions difference between your env and ours. Can you tell me what perms you had to give that service account, so I can replicate it on our side? I tried compute.admin but no dice.

[hanneshayashi]

Ah, my bad! I sort of assumed that there already was a test case somewhere for the instance schedulers, so I didn't think about the permissions. According to https://cloud.google.com/compute/docs/instances/schedule-instance-start-stop#before-you-begin, the Service Agent account (so service-PROJECT_NUMBER@compute-system.iam.gserviceaccount.com) needs compute.instances.start and compute.instances.stop, which is included in Compute Admin. Maybe you gave the "Compute Engine default service account" (so PROJECT_NUMBER-compute@developer.gserviceaccount.com) the permissions?

[hanneshayashi]

@ndmckinley have you had a chance to take another look? I suppose I could replace the instance schedulers with something else in the test, if you have a suggestion. I just used them because they are my use-case :)
Anyway, granting the service account the roles/compute.instanceAdmin.v1 role should work according to the support page and my tests.

[nat-henderson]

Ah, admin v1! Thank you, I'll do that.

[nat-henderson]

/gcbrun

[modular-magician]

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

Diff report:

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

[modular-magician]

I have triggered VCR tests in RECORDING mode for the following tests that failed during VCR: TestAccComposerEnvironment_update|TestAccComposerEnvironment_withSoftwareConfig|TestAccComputeInstance_resourcePolicyUpdate|TestAccComputeServiceAttachment_serviceAttachmentBasicExample|TestAccNetworkServicesEdgeCacheOrigin_networkServicesEdgeCacheOriginBasicExample You can view the result here: https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=207791

[nat-henderson]

Great, that test passes now! Let me see what's gone wrong with the linter...

[nat-henderson]

Hm, seems unrelated.

[nat-henderson]

Let's rerun it and see if it was transient? /gcbrun

[modular-magician]

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

Diff report:

Terraform GA: Diff ( 2 files changed, 289 insertions(+), 2 deletions(-))
Terraform Beta: Diff ( 3 files changed, 290 insertions(+), 3 deletions(-))

[nat-henderson]

Aha, yes, confirmed it is unrelated. Merging! Thanks for your contribution.

[modular-magician]

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