From 9089b8277c61f50d253381a32da3c56060799370 Mon Sep 17 00:00:00 2001
From: jiuker <2818723467@qq.com>
Date: Mon, 30 Oct 2023 23:17:03 +0800
Subject: [PATCH] fix: create bucket make a conflict status for tenant. (#1837)

* fix https://github.com/minio/operator/issues/1829

* doc

* reanme

* lint

* docs

* rename

---------

Co-authored-by: guozhi.li <guozhi.li@daocloud.io>
---
 pkg/apis/minio.min.io/v2/helper.go | 20 ++++++++++++++++++++
 pkg/controller/main-controller.go  | 16 +++++++++++-----
 pkg/controller/operator.go         | 24 ++++++++++++++----------
 3 files changed, 45 insertions(+), 15 deletions(-)

diff --git a/pkg/apis/minio.min.io/v2/helper.go b/pkg/apis/minio.min.io/v2/helper.go
index 0055884f9ee..60e083b6b2a 100644
--- a/pkg/apis/minio.min.io/v2/helper.go
+++ b/pkg/apis/minio.min.io/v2/helper.go
@@ -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] {
+			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 {
diff --git a/pkg/controller/main-controller.go b/pkg/controller/main-controller.go
index 6ca10b6d7ac..bf6cbec1c34 100644
--- a/pkg/controller/main-controller.go
+++ b/pkg/controller/main-controller.go
@@ -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
diff --git a/pkg/controller/operator.go b/pkg/controller/operator.go
index 2c6c09673d4..b33553b3eaf 100644
--- a/pkg/controller/operator.go
+++ b/pkg/controller/operator.go
@@ -21,6 +21,7 @@ import (
 	"context"
 	"crypto/tls"
 	"crypto/x509"
+	"errors"
 	"fmt"
 	"net"
 	"net/http"
@@ -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
@@ -328,10 +332,6 @@ 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 {
@@ -339,13 +339,17 @@ func (c *Controller) createBuckets(ctx context.Context, tenant *miniov2.Tenant,
 		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