-
Notifications
You must be signed in to change notification settings - Fork 333
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
refactor: simplify leader election #2152
base: master
Are you sure you want to change the base?
Conversation
some remarks on this PR:
|
@acuteaura Can you fix the merge conflicts? |
652edb7
to
90f4a4c
Compare
@acuteaura Unit tests failing |
zap.String("namespace", c.namespace), | ||
zap.String("pod", c.name), | ||
) | ||
// rootCancel might be to slow, and controllers may have bugs that cause them to not yield | ||
// the safest way to step down is to simply cause a pod restart | ||
os.Exit(0) |
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.
Makes sense
an e2e test related to leader election was also failing. I reran it |
The unit test is testing specific strings in the output instead of functionality, I think it can go. The e2e test.... I don't even know, it seems to fail before setup, but the Restart Count between the |
@Revolyssup could you have a look at this failed test? I'm not really sure how it could fail in the beforeEach step when that's all shared among tests. Or maybe re-run the tests to make sure this isn't a flake. It's unfortunately not so trivial to run this test locally, but I'm getting to it... |
ping @Revolyssup |
This pull request has been marked as stale due to 60 days of inactivity. It will be closed in 30 days if no further activity occurs. If you think that's incorrect or this pull request should instead be reviewed, please simply write any comment. Even if closed, you can still revive the PR at any time or discuss it on the dev@apisix.apache.org list. Thank you for your contributions. |
mostly based on the upstream example:
https://github.com/kubernetes/client-go/blob/master/examples/leader-election/main.go
this also incidentally fixes bugs where
run
returns nil (i.e. when failing to reach the Admin API) by causing the context to cancel via defer.