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
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions pkg/apis/minio.min.io/v2/helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -761,6 +761,26 @@ func (t *Tenant) CreateUsers(madmClnt *madmin.AdminClient, userCredentialSecrets
return nil
}

// CheckBucketsExist checks if the given buckets exist in the MinIO server
func (t *Tenant) CheckBucketsExist(minioClient *minio.Client, buckets ...Bucket) error {
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] {
jiuker marked this conversation as resolved.
Show resolved Hide resolved
Comment on lines +766 to +777
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

return fmt.Errorf("bucket %s not found", bucket.Name)
}
}
return nil
}

// CreateBuckets creates buckets and skips if bucket already present
func (t *Tenant) CreateBuckets(minioClient *minio.Client, buckets ...Bucket) error {
for _, bucket := range buckets {
Expand Down
16 changes: 11 additions & 5 deletions pkg/controller/main-controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -1373,12 +1373,18 @@ func (c *Controller) syncHandler(key string) (Result, error) {
// Ensure we are only creating the bucket
if len(tenant.Spec.Buckets) > 0 {
if err := c.createBuckets(ctx, tenant, tenantConfiguration); err != nil {
klog.V(2).Infof("Unable to create MinIO buckets: %v", err)
c.RegisterEvent(ctx, tenant, corev1.EventTypeWarning, "BucketsCreatedFailed", fmt.Sprintf("Buckets creation failed: %s", err))
// retry after 5sec
return WrapResult(Result{RequeueAfter: time.Second * 5}, err)
switch {
case errors.Is(err, ErrBucketsAlreadyExist):
// do noting
default:
klog.V(2).Infof("Unable to create MinIO buckets: %v", err)
c.RegisterEvent(ctx, tenant, corev1.EventTypeWarning, "BucketsCreatedFailed", fmt.Sprintf("Buckets creation failed: %s", err))
// retry after 5sec
return WrapResult(Result{RequeueAfter: time.Second * 5}, err)
}
} else {
c.RegisterEvent(ctx, tenant, corev1.EventTypeNormal, "BucketsCreated", "Buckets created")
}
c.RegisterEvent(ctx, tenant, corev1.EventTypeNormal, "BucketsCreated", "Buckets created")
}

// Finally, we update the status block of the Tenant resource to reflect the
Expand Down
24 changes: 14 additions & 10 deletions pkg/controller/operator.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"context"
"crypto/tls"
"crypto/x509"
"errors"
"fmt"
"net"
"net/http"
Expand Down Expand Up @@ -63,6 +64,9 @@ const (
DefaultOperatorImage = "minio/operator:v5.0.10"
)

// ErrBucketsAlreadyExist - all buckets already exist.
var ErrBucketsAlreadyExist = errors.New("all buckets already exist")

var serverCertsManager *xcerts.Manager

// rolloutRestartDeployment - executes the equivalent to kubectl rollout restart deployment
Expand Down Expand Up @@ -328,24 +332,24 @@ func (c *Controller) createBuckets(ctx context.Context, tenant *miniov2.Tenant,
return nil
}

if _, err := c.updateTenantStatus(ctx, tenant, StatusProvisioningDefaultBuckets, 0); err != nil {
return err
}

// get a new admin client
minioClient, err := tenant.NewMinIOUser(tenantConfiguration, c.getTransport())
if err != nil {
// show the error and continue
klog.Errorf("Error instantiating minio Client: %v ", err)
return err
}

if err := tenant.CreateBuckets(minioClient, tenantBuckets...); err != nil {
klog.V(2).Infof("Unable to create MinIO buckets: %v", err)
return err
err = tenant.CheckBucketsExist(minioClient, tenantBuckets...)
if err != nil {
if _, err = c.updateTenantStatus(ctx, tenant, StatusProvisioningDefaultBuckets, 0); err != nil {
return err
}
if err = tenant.CreateBuckets(minioClient, tenantBuckets...); err != nil {
klog.V(2).Infof("Unable to create MinIO buckets: %v", err)
return err
}
}

return nil
return ErrBucketsAlreadyExist
}

// getOperatorDeploymentName Internal func returns the Operator deployment name from MINIO_OPERATOR_DEPLOYMENT_NAME ENV variable or the default name
Expand Down