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

fix: create bucket make a conflict status for tenant. #1837

Merged
merged 6 commits into from
Oct 30, 2023

Conversation

jiuker
Copy link
Contributor

@jiuker jiuker commented Oct 30, 2023

fix #1829

  1. check bucket exists.
  2. if some of them not exist will create.
  3. if exist do nothing for them.

@jiuker jiuker requested a review from cniackz October 30, 2023 03:28
pkg/controller/operator.go Outdated Show resolved Hide resolved
pkg/apis/minio.min.io/v2/helper.go Show resolved Hide resolved
@cniackz
Copy link
Contributor

cniackz commented Oct 30, 2023

I review the code and it looks good to me, let me do a quick test on this just to make sure it is not breaking the feature. On it...

@cniackz
Copy link
Contributor

cniackz commented Oct 30, 2023

Create bucket on the fly:

spec:
  buckets:
  - name: cesar-testing

Bucket got created:

Screenshot 2023-10-30 at 10 16 46 AM

Log:

I1030 10:15:32.194824   10001 helper.go:805] Successfully created bucket cesar-testing

This change isn't breaking breaking the intention of creating the bucket on the fly.

cniackz
cniackz previously approved these changes Oct 30, 2023
Copy link
Contributor

@cniackz cniackz left a comment

Choose a reason for hiding this comment

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

It looks good to me @jiuker just address the changes from @shtripat please and then if all is good please merge since tests have passed!. 👍

@cniackz
Copy link
Contributor

cniackz commented Oct 30, 2023

And I can keep on adding more, no issue observed:

Screenshot 2023-10-30 at 10 20 36 AM Screenshot 2023-10-30 at 10 20 54 AM

@cniackz
Copy link
Contributor

cniackz commented Oct 30, 2023

Thank you @jiuker

Copy link
Contributor

@cniackz cniackz left a comment

Choose a reason for hiding this comment

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

LGTM

@cniackz cniackz merged commit 9089b82 into minio:master Oct 30, 2023
24 checks passed
Comment on lines +766 to +777
ctx, cancel := context.WithTimeout(context.Background(), time.Second*20)
defer cancel()
allBuckets, err := minioClient.ListBuckets(ctx)
if err != nil {
return err
}
allBucketsMap := make(map[string]bool)
for _, bucket := range allBuckets {
allBucketsMap[bucket.Name] = true
}
for _, bucket := range buckets {
if !allBucketsMap[bucket.Name] {
Copy link
Member

Choose a reason for hiding this comment

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

why can't we use HeadBucket here?

Copy link
Contributor Author

@jiuker jiuker Oct 31, 2023

Choose a reason for hiding this comment

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

I can't found that. Is there something I missed? @harshavardhana

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

Successfully merging this pull request may close these issues.

Operator keeps trying to create buckets every 5 seconds
4 participants