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

auth: correct initialization in NewAuthStore() #7260

Merged
merged 2 commits into from
Feb 9, 2017

Conversation

mitake
Copy link
Contributor

@mitake mitake commented Feb 1, 2017

Because of my own silly mistake, current NewAuthStore() doesn't
initialize authStore in a correct manner. For example, after recovery
from snapshot, it cannot revive the flag of enabled/disabled. This
commit fixes the problem.

Fix #7165

/cc @ekle @yudai

@xiang90
Copy link
Contributor

xiang90 commented Feb 1, 2017

Can we get a test covering the bad case?

@mitake
Copy link
Contributor Author

mitake commented Feb 2, 2017

@xiang90 I added the test case as TestRecoverFromSnapshot() of auth_store.go. Could you take a look?

Because of my own silly mistake, current NewAuthStore() doesn't
initialize authStore in a correct manner. For example, after recovery
from snapshot, it cannot revive the flag of enabled/disabled. This
commit fixes the problem.

Fix etcd-io#7165
@mitake
Copy link
Contributor Author

mitake commented Feb 6, 2017

@xiang90 updated for the problem of leaking goroutines, could you take a look? It seems that jenkins-proxy-ci is failing with other reasons (I opened an issue for the problem here:#7279)

@mitake
Copy link
Contributor Author

mitake commented Feb 8, 2017

kindly ping? @xiang90

@xiang90
Copy link
Contributor

xiang90 commented Feb 8, 2017

Sorry for the delay. I will take a look tomorrow.

@mitake
Copy link
Contributor Author

mitake commented Feb 8, 2017

No problem, thanks!

@xiang90
Copy link
Contributor

xiang90 commented Feb 9, 2017

LGTM

@xiang90 xiang90 merged commit c4fc8c0 into etcd-io:master Feb 9, 2017
@mitake mitake deleted the auth-state branch February 9, 2017 03:17
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.

auth enabled/disabled state can be lost after node reboot
4 participants