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.

Sign up for GitHub

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

Jump to bottom

server/auth: refresh cache on each NewAuthStore #14358

Merged
merged 1 commit into from
Aug 27, 2022
Merged

server/auth: refresh cache on each NewAuthStore #14358

merged 1 commit into from
Aug 27, 2022

Conversation

vivekpatani
Copy link
Contributor

@vivekpatani vivekpatani mentioned this pull request Aug 18, 2022
Closed
@vivekpatani
Copy link
Contributor Author

Will also add a unit/integration test, to restart cluster and recreate the auth scenario.

@codecov-commenter
Copy link

codecov-commenter commented Aug 18, 2022 •
edited
Loading

Codecov Report

Merging #14358 (ae608da) into main (e9c53a0) will increase coverage by 0.37%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main   #14358      +/-   ##
==========================================
+ Coverage   70.39%   70.77%   +0.37%     
==========================================
  Files         455      455              
  Lines       37146    37147       +1     
==========================================
+ Hits        26150    26289     +139     
+ Misses       9473     9350     -123     
+ Partials     1523     1508      -15
Flag Coverage Δ
all 70.77% <100.00%> (+0.37%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
server/auth/store.go 80.42% <100.00%> (+0.03%) ⬆️
server/proxy/grpcproxy/register.go 69.76% <0.00%> (-11.63%) ⬇️
client/pkg/v3/fileutil/lock_linux.go 72.22% <0.00%> (-8.34%) ⬇️
server/storage/wal/file_pipeline.go 90.69% <0.00%> (-4.66%) ⬇️
client/v3/leasing/cache.go 87.77% <0.00%> (-3.89%) ⬇️
server/etcdserver/metrics.go 80.85% <0.00%> (-2.13%) ⬇️
server/etcdserver/api/v3rpc/interceptor.go 73.43% <0.00%> (-2.09%) ⬇️
server/etcdserver/api/v3rpc/key.go 79.45% <0.00%> (-1.37%) ⬇️
client/v3/maintenance.go 59.74% <0.00%> (-1.26%) ⬇️
server/etcdserver/v3_server.go 70.68% <0.00%> (-1.05%) ⬇️
... and 33 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

ahrtr
ahrtr reviewed Aug 18, 2022
View reviewed changes
server/auth/store.go Outdated Show resolved Hide resolved
@ahrtr
Copy link
Member

ahrtr commented Aug 18, 2022 •
edited
Loading

Will also add a unit/integration test, to restart cluster and recreate the auth scenario.

Yes, please add a common (integration & e2e) test case

Please also signoff the commit.

@vivekpatani
Copy link
Contributor Author

vivekpatani commented Aug 22, 2022 •
edited
Loading

@ahrtr @spzala @mitake requesting review. Thanks.

  • moved the cache refresh in txn lock
  • added e2e test/integration test.

@vivekpatani vivekpatani requested a review from ahrtr August 22, 2022 22:34
ahrtr
ahrtr reviewed Aug 23, 2022
View reviewed changes
tests/e2e/ctl_v3_auth_test.go Outdated Show resolved Hide resolved
tests/e2e/ctl_v3_auth_test.go Outdated Show resolved Hide resolved
tests/integration/v3_auth_test.go Outdated Show resolved Hide resolved
tests/integration/v3_auth_test.go Outdated Show resolved Hide resolved
@vivekpatani vivekpatani requested a review from ahrtr August 23, 2022 21:58
ahrtr
ahrtr reviewed Aug 23, 2022
View reviewed changes
tests/e2e/ctl_v3_auth_test.go Outdated Show resolved Hide resolved
tests/e2e/ctl_v3_auth_test.go Show resolved Hide resolved
@vivekpatani
server,test: refresh cache on each NewAuthStore
ae608da 
- permissions were incorrectly loaded on restarts.
- etcd-io#14355

Signed-off-by: vivekpatani <9080894+vivekpatani@users.noreply.github.com>
@vivekpatani vivekpatani requested a review from ahrtr August 24, 2022 03:15
ahrtr
ahrtr approved these changes Aug 24, 2022
View reviewed changes
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

@ahrtr
Copy link
Member

ahrtr commented Aug 24, 2022

cc @mitake

@vivekpatani
Copy link
Contributor Author

@mitake any updates?

@mitake
Copy link
Contributor

mitake commented Aug 27, 2022

LGTM, thanks a lot @vivekpatani ! Could you open PRs for backporting it to release-3.4 and release-3.5?

@mitake mitake merged commit 9c8326b into etcd-io:main Aug 27, 2022
vivekpatani pushed a commit to vivekpatani/etcd that referenced this pull request Aug 31, 2022
server,test: refresh cache on each NewAuthStore
06e9008 
- permissions were incorrectly loaded on restarts.
- etcd-io#14355
- Backport of etcd-io#14358
vivekpatani pushed a commit to vivekpatani/etcd that referenced this pull request Aug 31, 2022
server,test: refresh cache on each NewAuthStore
a113005 
- permissions were incorrectly loaded on restarts.
- etcd-io#14355
- Backport of etcd-io#14358

Signed-off-by: vivekpatani <9080894+vivekpatani@users.noreply.github.com>
@vivekpatani vivekpatani mentioned this pull request Aug 31, 2022
Merged
vivekpatani added a commit to vivekpatani/etcd that referenced this pull request Aug 31, 2022
@vivekpatani
server,test: refresh cache on each NewAuthStore
c0ef7d5 
- permissions were incorrectly loaded on restarts.
- etcd-io#14355
- Backport of etcd-io#14358

Signed-off-by: vivekpatani <9080894+vivekpatani@users.noreply.github.com>
@vivekpatani vivekpatani mentioned this pull request Aug 31, 2022
Merged
@vivekpatani
Copy link
Contributor Author

Backports:
3.4: #14410
3.5: #14409
@mitake @ahrtr @serathius

@mitake
Copy link
Contributor

mitake commented Sep 2, 2022

Thanks a lot for opening the PRs @vivekpatani , could you also update changelog files like #14228 ?

vivekpatani pushed a commit to vivekpatani/etcd that referenced this pull request Sep 7, 2022
@vivekpatani
server,test: refresh cache on each NewAuthStore
64992c8 
- permissions were incorrectly loaded on restarts.
- etcd-io#14355
- Backport of etcd-io#14358

Signed-off-by: vivekpatani <9080894+vivekpatani@users.noreply.github.com>
vivekpatani pushed a commit to vivekpatani/etcd that referenced this pull request Sep 7, 2022
@vivekpatani
server,test: refresh cache on each NewAuthStore
7639d93 
- permissions were incorrectly loaded on restarts.
- etcd-io#14355
- Backport of etcd-io#14358

Signed-off-by: vivekpatani <9080894+vivekpatani@users.noreply.github.com>
openshift-cherrypick-robot pushed a commit to openshift-cherrypick-robot/etcd that referenced this pull request Oct 7, 2022
server,test: refresh cache on each NewAuthStore
130ebcf 
- permissions were incorrectly loaded on restarts.
- etcd-io#14355
- Backport of etcd-io#14358

Signed-off-by: vivekpatani <9080894+vivekpatani@users.noreply.github.com>
This was referenced Oct 11, 2022
Closed
Closed
@chaochn47 chaochn47 mentioned this pull request May 10, 2023
Merged
tjungblu pushed a commit to tjungblu/etcd that referenced this pull request Jul 26, 2023
@tjungblu
server,test: refresh cache on each NewAuthStore
ab6d267 
- permissions were incorrectly loaded on restarts.
- etcd-io#14355
- Backport of etcd-io#14358

Signed-off-by: vivekpatani <9080894+vivekpatani@users.noreply.github.com>
@serathius serathius mentioned this pull request Oct 27, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ahrtr ahrtr ahrtr approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@vivekpatani @codecov-commenter @ahrtr @mitake