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 container cluster #9013

Conversation

c2thorn
Copy link
Member

@c2thorn c2thorn commented Sep 20, 2023

fixes hashicorp/terraform-provider-google#10168
upgrade guide: #9087

Creates deletion_protection which is enabled by default.
Commits are separated out for review convenience (except for test fixes)

Essentially used the same approach for deletion_protection and the state upgrader as in https://github.com/GoogleCloudPlatform/magic-modules/pull/3450/files

Release Note Template for Downstream PRs (will be copied)

container: `google_container_cluster` now has `deletion_protection` enabled to `true` by default. When enabled, this field prevents Terraform from deleting the resource.

@modular-magician

This comment was marked as outdated.

@modular-magician

This comment was marked as outdated.

@modular-magician

This comment was marked as outdated.

@c2thorn c2thorn force-pushed the 5.0.0-container-deletion-protection branch from 88914ef to c5e5c8f Compare September 21, 2023 18:08
@c2thorn

This comment was marked as outdated.

@c2thorn c2thorn force-pushed the 5.0.0-container-deletion-protection branch from c5e5c8f to e06b8e5 Compare September 21, 2023 21:38
@modular-magician

This comment was marked as outdated.

@modular-magician

This comment was marked as outdated.

@modular-magician

This comment was marked as outdated.

@modular-magician

This comment was marked as outdated.

@modular-magician

This comment was marked as outdated.

@modular-magician

This comment was marked as outdated.

@c2thorn c2thorn force-pushed the 5.0.0-container-deletion-protection branch from c9fa7e1 to ca2400b Compare September 22, 2023 20:12
@modular-magician

This comment was marked as outdated.

@modular-magician

This comment was marked as outdated.

@modular-magician

This comment was marked as outdated.

@c2thorn
Copy link
Member Author

c2thorn commented Sep 24, 2023

/gcbrun

@modular-magician
Copy link
Collaborator

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.

Terraform GA: Diff ( 31 files changed, 938 insertions(+), 462 deletions(-))
Terraform Beta: Diff ( 32 files changed, 1055 insertions(+), 535 deletions(-))
TF OiCS: Diff ( 6 files changed, 6 insertions(+))

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 3083
Passed tests 2766
Skipped tests: 314
Affected tests: 3

Action taken

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

Get to know how VCR tests work

@modular-magician
Copy link
Collaborator

$\textcolor{green}{\textsf{Tests passed during RECORDING mode:}}$
TestAccDataSourceGoogleServiceAccountAccessToken_basic[Debug log]

Rerun these tests in REPLAYING mode to catch issues

$\textcolor{green}{\textsf{No issues found for passed tests after REPLAYING rerun.}}$


$\textcolor{red}{\textsf{Tests failed during RECORDING mode:}}$
TestAccContainerCluster_withAddons[Error message] [Debug log]
TestAccContainerNodePool_withSandboxConfig[Error message] [Debug log]

$\textcolor{red}{\textsf{Please fix these to complete your PR.}}$
View the build log or the debug log for each test

@c2thorn c2thorn force-pushed the 5.0.0-container-deletion-protection branch from ca2400b to 5a1bcaa Compare September 26, 2023 08:10
@modular-magician
Copy link
Collaborator

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

Breaking Change Detection Failed

The breaking change detector crashed during execution. This is usually due to the downstream provider(s) failing to compile. Please investigate or follow up with your reviewer.

Diff report

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

Terraform GA: Diff ( 33 files changed, 2576 insertions(+), 463 deletions(-))
Terraform Beta: Diff ( 34 files changed, 2884 insertions(+), 536 deletions(-))
TF OiCS: Diff ( 6 files changed, 6 insertions(+))

@modular-magician
Copy link
Collaborator

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.

Terraform GA: Diff ( 33 files changed, 2576 insertions(+), 463 deletions(-))
Terraform Beta: Diff ( 34 files changed, 2884 insertions(+), 536 deletions(-))
TF OiCS: Diff ( 6 files changed, 6 insertions(+))

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 3084
Passed tests 2768
Skipped tests: 314
Affected tests: 2

Action taken

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

Get to know how VCR tests work

@modular-magician
Copy link
Collaborator

$\textcolor{green}{\textsf{Tests passed during RECORDING mode:}}$
TestAccDataSourceGoogleServiceAccountIdToken_impersonation[Debug log]

Rerun these tests in REPLAYING mode to catch issues

$\textcolor{green}{\textsf{No issues found for passed tests after REPLAYING rerun.}}$


$\textcolor{red}{\textsf{Tests failed during RECORDING mode:}}$
TestAccContainerCluster_withAddons[Error message] [Debug log]

$\textcolor{red}{\textsf{Please fix these to complete your PR.}}$
View the build log or the debug log for each test

@c2thorn
Copy link
Member Author

c2thorn commented Sep 26, 2023

TestAccContainerCluster_withAddons is flaky with the same error in nightly

Copy link
Member

@melinath melinath left a comment

Choose a reason for hiding this comment

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

LGTM - have you been able to manually confirm that the state upgrader works as expected?

@c2thorn
Copy link
Member Author

c2thorn commented Sep 26, 2023

LGTM - have you been able to manually confirm that the state upgrader works as expected?

Yes, I've started with a minimally configured cluster on 4.83, tried to upgrade with the state upgrader and got a diff. Upgraded with the state upgrader and there was no diff.

@c2thorn c2thorn merged commit 887cf20 into GoogleCloudPlatform:FEATURE-BRANCH-major-release-5.0.0 Sep 26, 2023
14 of 15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants