Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add deletion protection to resource_compute_instance #1205

Merged
merged 3 commits into from
Mar 16, 2018
Merged

Add deletion protection to resource_compute_instance #1205

merged 3 commits into from
Mar 16, 2018

Conversation

nickjacques
Copy link
Contributor

@nickjacques nickjacques commented Mar 16, 2018

  • Added deletion_protection boolean parameter for google_compute_instance resources (in resource_compute_instance.go).
    • Parameter is optional, default value is false
  • Updated documentation to describe functionality
  • Add unit tests to confirm functionality

This resolves #956.

@nickjacques nickjacques changed the title Add deletion protection to resource_compute_instance WIP: Add deletion protection to resource_compute_instance Mar 16, 2018
@nickjacques
Copy link
Contributor Author

nickjacques commented Mar 16, 2018

Added unit tests:

  • Added implicit false test to TestAccComputeInstance_basic1
  • Created explicit false test with TestAccComputeInstance_deletionProtectionExplicitFalse
  • Created explicit true test with TestAccComputeInstance_deletionProtectionExplicitTrueAndUpdateFalse
    • This test creates a resource with explicit true, then updates it with explicit false, so we kill two birds with one stone (true and update). Update to false is required because otherwise the test harness can't delete the instance after tests complete.

@danawillow I think this is ready for review!

$ make testacc TEST=./google TESTARGS='-run=TestAccComputeInstance_basic1'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./google -v -run=TestAccComputeInstance_basic1 -timeout 120m
=== RUN   TestAccComputeInstance_basic1
--- PASS: TestAccComputeInstance_basic1 (59.16s)
PASS
ok      github.com/terraform-providers/terraform-provider-google/google 59.188s
$ make testacc TEST=./google TESTARGS='-run=TestAccComputeInstance_deletionProtectionExplicitFalse'                                                      
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./google -v -run=TestAccComputeInstance_deletionProtectionExplicitFalse -timeout 120m
=== RUN   TestAccComputeInstance_deletionProtectionExplicitFalse
--- PASS: TestAccComputeInstance_deletionProtectionExplicitFalse (179.77s)
PASS
ok      github.com/terraform-providers/terraform-provider-google/google 179.803s
$ make testacc TEST=./google TESTARGS='-run=TestAccComputeInstance_deletionProtectionExplicitTrueAndUpdateFalse'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./google -v -run=TestAccComputeInstance_deletionProtectionExplicitTrueAndUpdateFalse -timeout 120m
=== RUN   TestAccComputeInstance_deletionProtectionExplicitTrueAndUpdateFalse
--- PASS: TestAccComputeInstance_deletionProtectionExplicitTrueAndUpdateFalse (81.45s)
PASS
ok      github.com/terraform-providers/terraform-provider-google/google 81.485s

@nickjacques nickjacques changed the title WIP: Add deletion protection to resource_compute_instance Add deletion protection to resource_compute_instance Mar 16, 2018
"google_compute_instance.foobar", &instance),
testAccCheckComputeInstanceHasConfiguredDeletionProtection(&instance, true),
),
},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you toss an ImportStateVerify test step here between step 0 and step 1? That way we can be sure that this works on import. I'm sure it does, it's just something we're trying to do lately.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure! Updating commit incoming.

@nat-henderson
Copy link
Contributor

This looks great! Thanks for the contribution.

@nat-henderson nat-henderson self-assigned this Mar 16, 2018
@nat-henderson
Copy link
Contributor

Great! Running acceptance tests now.

@nat-henderson
Copy link
Contributor

Tests pass, great. :)

@nat-henderson nat-henderson merged commit 46a3d2f into hashicorp:master Mar 16, 2018
ashish-amarnath pushed a commit to ashish-amarnath/terraform-provider-google that referenced this pull request Mar 20, 2018
chrisst pushed a commit to chrisst/terraform-provider-google that referenced this pull request Nov 9, 2018
modular-magician pushed a commit to modular-magician/terraform-provider-google that referenced this pull request Oct 8, 2019
@ghost
Copy link

ghost commented Mar 29, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 hashibot-feedback@hashicorp.com. Thanks!

@ghost ghost locked and limited conversation to collaborators Mar 29, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Request: add support for "accidental VM deletion" in "google_compute_instance"
2 participants