-
Notifications
You must be signed in to change notification settings - Fork 4.6k
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
Don't panic when an etcd cluster is added #6180
Conversation
Not entirely clear what validation we should do here, but we shouldn't panic! Fix kubernetes#6133
/hold for input on #6133 |
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.
I was just looking at this too! Good fix!
/lgtm
/approve
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: justinsb, mikesplain The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
lol - thanks @mikesplain ! I'm wondering whether it was adding an etcd-cluster, or whether something deeper went wrong - hoping to get some input on the original issue! |
@justinsb Whoah, that was close, good catch on the hold. Yeah I was working on a similar fix. Hopefully this should at least be a stopgap for now. |
Going to remove hold, we want this in 1.12. I guess we want it in 1.11 also. /hold cancel |
Cherry pick of #6180 onto release-1.11
Not entirely clear what validation we should do here, but we shouldn't panic!
Fix #6133