Add a functionality to create a snapshot before a disk is deleted

[karolgorc]

closes hashicorp/terraform-provider-google#15103

Release Note Template for Downstream PRs (will be copied)

See Write release notes for guidance.

compute: added `create_snapshot_before_destroy` to `google_compute_disk` and `google_compute_region_disk` to enable creating a snapshot before disk deletion

compute: added `rsa_encrypted_key` to `google_compute_region_disk`

[github-actions]

Hello! I am a robot. Tests will require approval from a repository maintainer to run.

@rileykarson, a repository maintainer, has been assigned to review your changes. If you have not received review feedback within 2 business days, please leave a comment on this PR asking them to take a look.

You can help make sure that review is quick by doing a self-review and by running impacted tests locally.

[github-actions]

@rileykarson This PR has been waiting for review for 3 weekdays. Please take a look! Use the label disable-review-reminders to disable these notifications.

[github-actions]

@GoogleCloudPlatform/terraform-team @rileykarson This PR has been waiting for review for 1 week. Please take a look! Use the label disable-review-reminders to disable these notifications.

[melinath]

@rileykarson are you able to review this?

[rileykarson]

Been low on cycles, rolled @c2thorn as an alternate reviewer

[c2thorn]

Seems reasonable and in decent enough demand to warrant a virtual field. @karolgorc can you write a test for this functionality?
We should be able to use a dependent google_compute_snapshot data source to verify the snapshot was successfully created.

[karolgorc]

Yes, sure. One question tough. The snapshot that's created is outside of terraform so it won't be deleted after the test finishes. I'm guessing that we don't want to leave artifacts so what i can do is add a delete API query as a check step. Is that good enough?

Also just remembered that i need to add sourceDiskEncryptionKey support into the code here so it will work with encrypted disks.

[c2thorn]

Yes, sure. One question tough. The snapshot that's created is outside of terraform so it won't be deleted after the test finishes. I'm guessing that we don't want to leave artifacts so what i can do is add a delete API query as a check step. Is that good enough?

Good point, yes that would be great in a check step.

[karolgorc]

• Improved documentation
• Added missing fields to region disk so the functionality between disk and region_disk will be the same
• Added tests with all encryption methods and disk naming tested
• Added create_snapshot_before_destroy_prefix

[modular-magician]

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

google provider: Diff ( 8 files changed, 403 insertions(+), 1 deletion(-))
google-beta provider: Diff ( 8 files changed, 403 insertions(+), 1 deletion(-))
terraform-google-conversion: Diff ( 1 file changed, 11 insertions(+))

[modular-magician]

Tests analytics

Total tests: 471
Passed tests: 409
Skipped tests: 61
Affected tests: 1

Click here to see the affected service packages

• compute

#### Non-exercised tests

🔴 Tests were added that are skipped in VCR:

• TestAccComputeDisk_createSnapshotBeforeDestroy

Action taken

Found 1 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests

• TestAccComputeRegionDisk_createSnapshotBeforeDestroy

Get to know how VCR tests work

[modular-magician]

🟢 Tests passed during RECORDING mode:
TestAccComputeRegionDisk_createSnapshotBeforeDestroy [Debug log]

🔴 Tests failed when rerunning REPLAYING mode:
TestAccComputeRegionDisk_createSnapshotBeforeDestroy [Error message] [Debug log]

Tests failed due to non-determinism or randomness when the VCR replayed the response after the HTTP request was made.

Please fix these to complete your PR. If you believe these test failures to be incorrect or unrelated to your change, or if you have any questions, please raise the concern with your reviewer.

---

🟢 All tests passed!

View the build log or the debug log for each test

[c2thorn]

after we remove from VCR I will reconfirm with a manual test run in teamcity

[c2thorn]

running manual compute disk and region disk tests on TC

[karolgorc]

Is there anything that has to be done with this?

[github-actions]

@rileykarson This PR has been waiting for review for 3 weekdays. Please take a look! Use the label disable-review-reminders to disable these notifications.

[c2thorn]

Is there anything that has to be done with this?

Apologies, I was out of office for a bit, but just confirmed both the regional and non regional tests have passed. Unfortunately within that timeframe it seems there is now a conflict in the template. Once we resolve that this should be good to go.

[c2thorn]

I've resolved the conflict in the web editor and will confirm before approving.

[modular-magician]

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

google provider: Diff ( 8 files changed, 450 insertions(+), 32 deletions(-))
google-beta provider: Diff ( 8 files changed, 438 insertions(+), 32 deletions(-))
terraform-google-conversion: Diff ( 1 file changed, 11 insertions(+))

Missing test report

Your PR includes resource fields which are not covered by any test.

Resource: google_compute_disk (139 total tests)
Please add an acceptance test which includes these fields. The test should include the following:

resource "google_compute_disk" "primary" {
  create_snapshot_before_destroy        = # value needed
  create_snapshot_before_destroy_prefix = # value needed
}

[modular-magician]

Tests analytics

Total tests: 0
Passed tests: 0
Skipped tests: 0
Affected tests: 0

Click here to see the affected service packages

• compute

#### Non-exercised tests

🔴 Tests were added that are skipped in VCR:

• TestAccComputeDisk_createSnapshotBeforeDestroy
• TestAccComputeRegionDisk_createSnapshotBeforeDestroy
  🔴 Errors occurred during REPLAYING mode. Please fix them to complete your PR.

View the build log

[karolgorc]

One of the tests didn't have it's brackets closed after resolving the conflicts

E0227 07:13:50.806201     485 template_data.go:231] error formatting /home/test/go/src/github.com/hashicorp/terraform-provider-google/google/services/compute/resource_compute_disk_test.go: 1949:26: missing ',' before newline in argument list (and 9 more errors)

Fixed

[modular-magician]

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

google provider: Diff ( 8 files changed, 403 insertions(+), 1 deletion(-))
google-beta provider: Diff ( 8 files changed, 403 insertions(+), 1 deletion(-))
terraform-google-conversion: Diff ( 1 file changed, 11 insertions(+))

[modular-magician]

Tests analytics

Total tests: 1122
Passed tests: 1043
Skipped tests: 79
Affected tests: 0

Click here to see the affected service packages

• compute

#### Non-exercised tests

🔴 Tests were added that are skipped in VCR:

• TestAccComputeDisk_createSnapshotBeforeDestroy
• TestAccComputeRegionDisk_createSnapshotBeforeDestroy
  🟢 All tests passed!

View the build log

[c2thorn]

One of the tests didn't have it's brackets closed after resolving the conflicts

E0227 07:13:50.806201     485 template_data.go:231] error formatting /home/test/go/src/github.com/hashicorp/terraform-provider-google/google/services/compute/resource_compute_disk_test.go: 1949:26: missing ',' before newline in argument list (and 9 more errors)

Fixed

thanks for the fix!

[c2thorn]

updated the changelog entries. LGTM