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

Reload users upon AddUser on peers #6975

Merged
merged 1 commit into from Dec 18, 2018
Merged

Conversation

harshavardhana
Copy link
Member

Description

Reload users upon AddUser on peers

Motivation and Context

Also, migrate ReloadFormat to notification subsystem,
remove GetConfig() we do not use this API anymore

Regression

No

How Has This Been Tested?

Run a distributed setup with a load balancer in front just running

./mc admin user add minio newuser newuser123 customPolicy 
mc: <ERROR> Cannot set user policy for new user The specified user does not exist.

Shouldn't result in an issue for subsequent operations when the calls reach peers.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added unit tests to cover my changes.
  • I have added/updated functional tests in mint. (If yes, add mint PR # here: )
  • All new and existing tests passed.

@harshavardhana
Copy link
Member Author

Fixes #6957

@codecov
Copy link

codecov bot commented Dec 14, 2018

Codecov Report

Merging #6975 into master will increase coverage by 0.1%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #6975      +/-   ##
==========================================
+ Coverage    51.9%   52.01%    +0.1%     
==========================================
  Files         261      271      +10     
  Lines       32728    42912   +10184     
==========================================
+ Hits        16988    22320    +5332     
- Misses      13743    18591    +4848     
- Partials     1997     2001       +4
Impacted Files Coverage Δ
cmd/admin-rpc-client.go 42.65% <ø> (-1.05%) ⬇️
cmd/local-admin-client.go 46.03% <ø> (-13.97%) ⬇️
cmd/admin-rpc-server.go 61.9% <ø> (-10.1%) ⬇️
cmd/peer-rpc-server.go 0% <0%> (ø) ⬆️
cmd/notification.go 12.92% <0%> (-1.48%) ⬇️
cmd/peer-rpc-client.go 2.52% <0%> (-0.82%) ⬇️
cmd/admin-heal-ops.go 46.15% <0%> (-1.56%) ⬇️
cmd/iam.go 16.79% <0%> (-0.93%) ⬇️
cmd/admin-handlers.go 20.94% <0%> (+0.27%) ⬆️
... and 247 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7c9f934...91b484e. Read the comment docs.

@ihamouda
Copy link

@harshavardhana Any chance this can be merged soon?

@harshavardhana
Copy link
Member Author

The change is trivial might get merged soon, no set dates.

Also migrate ReloadFormat to notification subsystem,
remove GetConfig() we do not use this API anymore
@minio-ops
Copy link

Mint Automation

Test Result
mint-tls.sh ✔️
mint-compression-xl.sh ✔️
mint-xl.sh ✔️
mint-compression-fs.sh ✔️
mint-worm.sh ✔️
mint-fs.sh ✔️
mint-dist-xl.sh ✔️
mint-large-bucket.sh more...

6975-91b484e/mint-large-bucket.sh.log:

Running with
SERVER_ENDPOINT:      minikube:31543
ACCESS_KEY:           minio
SECRET_KEY:           ***REDACTED***
ENABLE_HTTPS:         0
SERVER_REGION:        us-east-1
MINT_DATA_DIR:        /mint/data
MINT_MODE:            full
ENABLE_VIRTUAL_STYLE: 0

To get logs, run 'docker cp 8c8958d3295b:/mint/log /tmp/mint-logs'

(1/13) Running awscli tests ... FAILED in 2 minutes and 30 seconds
{
  "name": "awscli",
  "duration": 73408,
  "function": "delete_bucket\n",
  "status": "FAIL",
  "error": "remove_bucket failed: s3://awscli-mint-test-bucket-8215 An error occurred (BucketNotEmpty) when calling the DeleteBucket operation: The bucket you tried to delete is not empty"
}

Executed 0 out of 13 tests successfully.

Copy link
Member

@donatello donatello left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ihamouda
Copy link

any word when this will be merged?

@deekoder
Copy link
Contributor

ignoring mint-largebucket run error. Unrelated as per @harshavardhana

@deekoder deekoder merged commit 4f31a9a into minio:master Dec 18, 2018
@harshavardhana harshavardhana deleted the fix-reload branch December 19, 2018 03:49
richardkiene pushed a commit to richardkiene/minio that referenced this pull request Jan 2, 2019
Also migrate ReloadFormat to notification subsystem,
remove GetConfig() we do not use this API anymore
richardkiene pushed a commit to richardkiene/minio that referenced this pull request Jan 2, 2019
Also migrate ReloadFormat to notification subsystem,
remove GetConfig() we do not use this API anymore
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants