-
Notifications
You must be signed in to change notification settings - Fork 9.8k
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
[release-3.4] server,test: refresh cache on each NewAuthStore #14410
Conversation
- permissions were incorrectly loaded on restarts. - etcd-io#14355 - Backport of etcd-io#14358 Signed-off-by: vivekpatani <[email protected]>
|
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.
LGTM, defer to @ahrtr . Do you think it's better to revisit timeout config?
It seems the new integration test case is failing. It might be related to the
|
@ahrtr It's only failing in linux-amd64-integration-1-cpu. linux-amd64-integration-2-cpu and linux-amd64-integration-4-cpu don't cause the failure and on my local env the test case runs successfully. Probably we should fix the timeout config? How do you think @vivekpatani ? |
@mitake quoting myself from the 3.5 PR, I can't seem to reproduce this, can you by any chance reproduce this? I've tried on docker (amd64) and on my baremetal machine (M1 arch), and have consistently failed and reproducing this. |
Yep, v3.4 tests are definitely flaky. |
testutil.AssertNil(t, err) | ||
|
||
clus.Members[0].Stop(t) | ||
err = clus.Members[0].Restart(t) |
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.
Please try to add code something below, probably we should add a timeout also,
clus.Members[0].WaitOK(t)
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.
Let me do this in a separate PR.
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.
FYI. #14442
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.
LGTM
Thank you @vivekpatani
sorry for the delay, thanks for merging @ahrtr |
Signed-off-by: vivekpatani [email protected]
Please read https://github.com/etcd-io/etcd/blob/main/CONTRIBUTING.md#contribution-flow.