fix: createBuckets simplify implementation (#1840)

pkg/apis/minio.min.io/v2/helper.go

@@ -49,7 +49,6 @@ import (
 	"github.com/minio/madmin-go/v2"
 	"github.com/minio/minio-go/v7"
 	"github.com/minio/minio-go/v7/pkg/credentials"
-	"github.com/minio/minio-go/v7/pkg/s3utils"
 )
 
 // Webhook API constants
@@ -761,36 +760,14 @@ 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] {
-			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 {
+func (t *Tenant) CreateBuckets(minioClient *minio.Client, buckets ...Bucket) (created bool, err error) {
+	createBucketCount := 0
 	for _, bucket := range buckets {
-		if err := s3utils.CheckValidBucketNameStrict(bucket.Name); err != nil {
-			return err
-		}
 		// create each bucket with a 20 seconds timeout
 		ctx, cancel := context.WithTimeout(context.Background(), time.Second*20)
 		defer cancel()
-		if err := minioClient.MakeBucket(ctx, bucket.Name, minio.MakeBucketOptions{
+		if err = minioClient.MakeBucket(ctx, bucket.Name, minio.MakeBucketOptions{
 			Region:        bucket.Region,
 			ObjectLocking: bucket.ObjectLocking,
 		}); err != nil {
@@ -799,12 +776,13 @@ func (t *Tenant) CreateBuckets(minioClient *minio.Client, buckets ...Bucket) err
 				klog.Infof(err.Error())
 				continue
 			default:
-				return err
+				return false, err
 			}
 		}
+		createBucketCount++
 		klog.Infof("Successfully created bucket %s", bucket.Name)
 	}
-	return nil
+	return createBucketCount > 0, nil
 }
 
 // Validate validate single pool as per MinIO deployment requirements

pkg/controller/main-controller.go

@@ -1372,17 +1372,12 @@ 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 {
-			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 {
+		if create, 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)
+		} else if create {
 			c.RegisterEvent(ctx, tenant, corev1.EventTypeNormal, "BucketsCreated", "Buckets created")
 		}
 	}

pkg/controller/operator.go

@@ -21,7 +21,6 @@ import (
 	"context"
 	"crypto/tls"
 	"crypto/x509"
-	"errors"
 	"fmt"
 	"net"
 	"net/http"
@@ -64,9 +63,6 @@ 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
@@ -318,38 +314,32 @@ func (c *Controller) createUsers(ctx context.Context, tenant *miniov2.Tenant, te
 	return err
 }
 
-func (c *Controller) createBuckets(ctx context.Context, tenant *miniov2.Tenant, tenantConfiguration map[string][]byte) (err error) {
-	defer func() {
-		if err == nil {
-			if _, err = c.updateProvisionedBucketStatus(ctx, tenant, true); err != nil {
-				klog.V(2).Infof(err.Error())
-			}
-		}
-	}()
-
+func (c *Controller) createBuckets(ctx context.Context, tenant *miniov2.Tenant, tenantConfiguration map[string][]byte) (created bool, err error) {
 	tenantBuckets := tenant.Spec.Buckets
 	if len(tenantBuckets) == 0 {
-		return nil
+		return false, nil
 	}
-
 	// 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
+		return false, err
 	}
-	err = tenant.CheckBucketsExist(minioClient, tenantBuckets...)
+	created, err = tenant.CreateBuckets(minioClient, tenantBuckets...)
 	if err != nil {
-		if _, err = c.updateTenantStatus(ctx, tenant, StatusProvisioningDefaultBuckets, 0); err != nil {
-			return err
+		klog.V(2).Infof("Unable to create MinIO buckets: %v", err)
+		if _, terr := c.updateTenantStatus(ctx, tenant, StatusProvisioningDefaultBuckets, 0); terr != nil {
+			return false, err
 		}
-		if err = tenant.CreateBuckets(minioClient, tenantBuckets...); err != nil {
-			klog.V(2).Infof("Unable to create MinIO buckets: %v", err)
-			return err
+		return false, err
+	}
+	if created {
+		if _, err = c.updateProvisionedBucketStatus(ctx, tenant, true); err != nil {
+			klog.V(2).Infof(err.Error())
 		}
 	}
-	return ErrBucketsAlreadyExist
+	return created, err
 }
 
 // getOperatorDeploymentName Internal func returns the Operator deployment name from MINIO_OPERATOR_DEPLOYMENT_NAME ENV variable or the default name