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

BREAKING CHANGE: support maintenance_exclusion #1273

Conversation

ericyz
Copy link
Collaborator

@ericyz ericyz commented May 26, 2022

No description provided.

@ericyz ericyz requested review from a team, Jberlinsky and bharathkkb as code owners May 26, 2022 13:20
@ericyz ericyz force-pushed the feature/maintenance-exclusion branch 12 times, most recently from 5bdd99f to 7e22154 Compare May 31, 2022 01:49
@ericyz
Copy link
Collaborator Author

ericyz commented May 31, 2022

Breaking Change:

  • maintenance scope required

@ericyz
Copy link
Collaborator Author

ericyz commented May 31, 2022

@bharathkkb @Jberlinsky - please review and there will be breaking change on this PR.

@apeabody apeabody changed the title feat: support maintenance_exclusion feat!: support maintenance_exclusion May 31, 2022
Copy link
Member

@bharathkkb bharathkkb left a comment

Choose a reason for hiding this comment

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

Thanks for the PR @ericyz
Looks like this will need a rebase

cluster.tf Outdated
start_time = maintenance_exclusion.value.start_time
end_time = maintenance_exclusion.value.end_time

exclusion_options {
Copy link
Member

Choose a reason for hiding this comment

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

Lets make this a dynamic block created iff maintenance_exclusion.value.exclusion_options is set.

@ericyz
Copy link
Collaborator Author

ericyz commented Jun 3, 2022

One check keeps failing. Would you like to check if it is caused by the configuration change?

Step #67 - "verify simple-autopilot-public-local": expected: "RUNNING"
Step #67 - "verify simple-autopilot-public-local": got: "RECONCILING"

@bharathkkb
Copy link
Member

@ericyz I think that is different, I opened #1280 to track a fix

cluster.tf Outdated
@@ -133,21 +135,25 @@ resource "google_container_cluster" "primary" {
}
}

dynamic "daily_maintenance_window" {
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this still be a dynamic block?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, let me updated that

@ericyz ericyz force-pushed the feature/maintenance-exclusion branch 2 times, most recently from 5507045 to f2ae5d7 Compare June 6, 2022 01:44
@ericyz
Copy link
Collaborator Author

ericyz commented Jun 7, 2022

Please review @bharathkkb @Jberlinsky

@ericyz ericyz force-pushed the feature/maintenance-exclusion branch 3 times, most recently from 6b2dd8d to b52ac6a Compare June 7, 2022 09:29
Copy link
Member

@bharathkkb bharathkkb left a comment

Choose a reason for hiding this comment

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

LGTM, since this is a breaking change let's hold off on merging till our next breaking release.

@ericyz ericyz changed the title feat!: support maintenance_exclusion BREAKING CHANGE: support maintenance_exclusion Jun 24, 2022
@ericyz
Copy link
Collaborator Author

ericyz commented Jun 24, 2022

@bharathkkb - any ETA when the next major version release?

@bharathkkb
Copy link
Member

@ericyz we can do the next release as major version. Could you rebase this and I can merge.

@ericyz
Copy link
Collaborator Author

ericyz commented Jun 24, 2022

Rebased

@ericyz ericyz force-pushed the feature/maintenance-exclusion branch from 1c76e79 to a4526b3 Compare June 24, 2022 03:45
@ericyz ericyz force-pushed the feature/maintenance-exclusion branch from a4526b3 to 2657270 Compare June 24, 2022 06:12
@comment-bot-dev
Copy link

@ericyz
Thanks for the PR! 🚀
✅ Lint checks have passed.

@bharathkkb bharathkkb merged commit 425bf93 into terraform-google-modules:master Jun 24, 2022
CPL-markus pushed a commit to WALTER-GROUP/terraform-google-kubernetes-engine that referenced this pull request Jul 15, 2024
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