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

[release-3.5] server,test: refresh cache on each NewAuthStore #14409

Merged
merged 1 commit into from
Sep 8, 2022

Conversation

vivekpatani
Copy link
Contributor

@mitake
Copy link
Contributor

mitake commented Sep 2, 2022

TestV3AuthRestartMember is also failing only in linux-amd64-integration-2-cpu. Let me check.

@mitake
Copy link
Contributor

mitake commented Sep 4, 2022

LGTM, the newly added test is a little bit flaky though. If it keeps failing, I think it's valuable to revisit the timeout config.
Minor request: could you revisit the email address for making DCO check success @vivekpatani ? The github one is detected as unacceptable email address.
Defer to @ahrtr

@ahrtr
Copy link
Member

ahrtr commented Sep 4, 2022

Please signoff the commit.

You used the github emal [email protected] to signoff the commit in #14410 as well, but there is no any DCO issue. It's strange.

Based on the feedback from DCO details, it seems Vivek Patani [email protected] is expected.

Anyway, could you please get this sorted out?

@serathius serathius changed the title server,test: refresh cache on each NewAuthStore [release-3.5] server,test: refresh cache on each NewAuthStore Sep 7, 2022
@serathius serathius mentioned this pull request Sep 7, 2022
16 tasks
@serathius
Copy link
Member

This blocks v3.5.5 release. @vivekpatani are you planning to work on this?

@vivekpatani
Copy link
Contributor Author

@serathius yes, there is an issue with a flaky test. Sorry, was OoO.

@ahrtr @mitake I'm not able to reproduce the issue locally (or inside docker). I can increase the timeout, but I'll have to create another PR to fix that, and then rebase this once that one is merged. But given the fact, I can't reproduce the issue correctly, I'm not sure what timeout needs to be set to. Any recommendations?

@ahrtr I'll also look at fixing the signature. The github one is what github.com provides for privacy reasons but let me fix that.

@serathius
Copy link
Member

All tests pass apart of DCO, please sign all commits by running git rebase origin/release-3.5 --signoff, and push the HEAD with force git push origin HEAD:release-3.5 -f

@vivekpatani
Copy link
Contributor Author

@serathius I think there's a flaky test, but I've re pushed the fix for DCO.

- permissions were incorrectly loaded on restarts.
- etcd-io#14355
- Backport of etcd-io#14358

Signed-off-by: vivekpatani <[email protected]>
Copy link
Member

@ahrtr ahrtr left a 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

defer to @mitake to approve & merge this PR.

@ahrtr
Copy link
Member

ahrtr commented Sep 8, 2022

But given the fact, I can't reproduce the issue correctly, I'm not sure what timeout needs to be set to. Any recommendations?

Let's investigate the test failure in #14410

@mitake mitake merged commit bb3fae4 into etcd-io:release-3.5 Sep 8, 2022
@mitake
Copy link
Contributor

mitake commented Sep 8, 2022

LGTM, thanks a lot @vivekpatani . Let’s discuss about the flaky test in #14410

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants