-
Notifications
You must be signed in to change notification settings - Fork 810
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
Disable deletion protection for Autopilot test clusters #3468
Conversation
Build Succeeded 👏 Build Id: 70940d30-6636-4e3f-aa7d-d255f2e100bb The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website (the last 30 builds are retained): To install this version:
|
This is changing the value is the terraform module, which changes it for both the test clusters but also for anyone that is using our example module: https://github.com/googleforgames/agones/blob/main/examples/terraform-submodules/gke-autopilot/module.tf I think it would make more sense to make it a variable, set it to true by default (to keep the current behavior) and set it to false explicitly in the test configs. |
Build Failed 😱 Build Id: c96ac5f9-70f6-4c25-ad90-ca2dd7f5129c To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
Build Failed 😱 Build Id: 0231c7b5-101b-4001-b093-cd7a4bbabea6 To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
Build Failed 😱 Build Id: 4ee335ff-5608-4225-a49a-4d860a940400 To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
Build Succeeded 👏 Build Id: b695520e-7c63-45b3-a273-45769e51cadd The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website (the last 30 builds are retained): To install this version:
|
Build Succeeded 👏 Build Id: 3ce6182a-fe61-455b-b57d-ceb10fcc24c4 The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website (the last 30 builds are retained): To install this version:
|
Build Failed 😱 Build Id: 18b7e9c9-3284-407d-8776-0a9a5b24dfb6 To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
Build Failed 😱 Build Id: 41f0cf14-5ad0-422d-b74c-ee002c95fc56 To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
Build Succeeded 👏 Build Id: d630507e-2a1d-45b1-a3a1-a1149f5bd92c The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website (the last 30 builds are retained): To install this version:
|
Build Succeeded 👏 Build Id: 1314672e-97b2-4b17-b654-15631deb2722 The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website (the last 30 builds are retained): To install this version:
|
Build Succeeded 👏 Build Id: 71473818-e5bc-4432-a103-63226cfe728c The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website (the last 30 builds are retained): To install this version:
|
Build Succeeded 👏 Build Id: 16079928-0d22-44cd-9d39-46459f02b030 The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website (the last 30 builds are retained): To install this version:
|
Are we good to merge this PR? |
New changes are detected. LGTM label has been removed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wait. One thing I realized for both this and maintenance exclusions is that we injected it into the install/terraform
path. I don't think that's right - I think maintenance exclusions should be optional in the base install and we need this in the e2e specific one. Unless I'm mis-remembering the structure here?
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: gongmax The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Build Succeeded 👏 Build Id: 5c74820c-d304-4689-a0e1-356e4bf1588c The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website (the last 30 builds are retained): To install this version:
|
Build Failed 😱 Build Id: 4d785c88-17c9-4d7c-bf9d-0799e0ebc50c To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
…ke maintainance exclusive window optional
Make sense. Updated accordingly. |
Build Failed 😱 Build Id: eeec2056-e117-4ad9-a94f-d3bb110531e9 To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
Build Succeeded 👏 Build Id: e910af95-dd97-4ea7-870b-039d51d363af The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website (the last 30 builds are retained): To install this version:
|
Build Succeeded 👏 Build Id: ec31fcb4-230b-4a9b-b994-494a2dc18315 The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website (the last 30 builds are retained): To install this version:
|
What type of PR is this?
/kind cleanup
What this PR does / Why we need it:
Seems deletion protection is enabled by default for AP clusters, and we saw cluster couldn't be deleted when we updated k8s version for test clusters. This PR disable it explicitly.
Which issue(s) this PR fixes:
Closes #
Special notes for your reviewer: