Extract operator defaults to a separate func (#1068)

- to keep Reconciler shorter and more concise

Author: ChunyiLyu

controllers/rabbitmqcluster_controller.go

@@ -117,49 +117,8 @@ func (r *RabbitmqClusterReconciler) Reconcile(ctx context.Context, req ctrl.Requ
 		return ctrl.Result{}, nil
 	}
 
-	if rabbitmqCluster.Spec.Image == "" {
-		rabbitmqCluster.Spec.Image = r.DefaultRabbitmqImage
-		if err = r.Update(ctx, rabbitmqCluster); err != nil {
-			if k8serrors.IsConflict(err) {
-				logger.Info("failed to update image because of conflict; requeueing...",
-					"namespace", rabbitmqCluster.Namespace,
-					"name", rabbitmqCluster.Name)
-				return ctrl.Result{RequeueAfter: 2 * time.Second}, nil
-			}
-			return ctrl.Result{}, err
-		}
-	}
-
-	if rabbitmqCluster.Spec.ImagePullSecrets == nil {
-		// split the comma separated list of default image pull secrets from
-		// the 'DEFAULT_IMAGE_PULL_SECRETS' env var, but ignore empty strings.
-		for _, reference := range strings.Split(r.DefaultImagePullSecrets, ",") {
-			if len(reference) > 0 {
-				rabbitmqCluster.Spec.ImagePullSecrets = append(rabbitmqCluster.Spec.ImagePullSecrets, corev1.LocalObjectReference{Name: reference})
-			}
-		}
-		if err = r.Update(ctx, rabbitmqCluster); err != nil {
-			if k8serrors.IsConflict(err) {
-				logger.Info("failed to update image pull secrets because of conflict; requeueing...",
-					"namespace", rabbitmqCluster.Namespace,
-					"name", rabbitmqCluster.Name)
-				return ctrl.Result{RequeueAfter: 2 * time.Second}, nil
-			}
-			return ctrl.Result{}, err
-		}
-	}
-
-	if rabbitmqCluster.UsesDefaultUserUpdaterImage() {
-		rabbitmqCluster.Spec.SecretBackend.Vault.DefaultUserUpdaterImage = &r.DefaultUserUpdaterImage
-		if err = r.Update(ctx, rabbitmqCluster); err != nil {
-			if k8serrors.IsConflict(err) {
-				logger.Info("failed to update image because of conflict; requeueing...",
-					"namespace", rabbitmqCluster.Namespace,
-					"name", rabbitmqCluster.Name)
-				return ctrl.Result{RequeueAfter: 2 * time.Second}, nil
-			}
-			return ctrl.Result{}, err
-		}
+	if requeueAfter, err := r.reconcileOperatorDefaults(ctx, rabbitmqCluster); err != nil || requeueAfter > 0 {
+		return ctrl.Result{RequeueAfter: requeueAfter}, err
 	}
 
 	// Ensure the resource have a deletion marker

controllers/reconcile_operator_defaults.go

@@ -0,0 +1,60 @@
+package controllers
+
+import (
+	"context"
+	"fmt"
+	rabbitmqv1beta1 "github.com/rabbitmq/cluster-operator/api/v1beta1"
+	corev1 "k8s.io/api/core/v1"
+	k8serrors "k8s.io/apimachinery/pkg/api/errors"
+	ctrl "sigs.k8s.io/controller-runtime"
+	"strings"
+	"time"
+)
+
+// reconcileOperatorDefaults updates current rabbitmqCluster with operator defaults from the Reconciler
+// it handles RabbitMQ image, imagePullSecrets, and user updater image
+func (r *RabbitmqClusterReconciler) reconcileOperatorDefaults(ctx context.Context, rabbitmqCluster *rabbitmqv1beta1.RabbitmqCluster) (time.Duration, error) {
+	if rabbitmqCluster.Spec.Image == "" {
+		rabbitmqCluster.Spec.Image = r.DefaultRabbitmqImage
+		if requeue, err := r.updateRabbitmqCluster(ctx, rabbitmqCluster, "image"); err != nil {
+			return requeue, err
+		}
+	}
+
+	if rabbitmqCluster.Spec.ImagePullSecrets == nil {
+		// split the comma separated list of default image pull secrets from
+		// the 'DEFAULT_IMAGE_PULL_SECRETS' env var, but ignore empty strings.
+		for _, reference := range strings.Split(r.DefaultImagePullSecrets, ",") {
+			if len(reference) > 0 {
+				rabbitmqCluster.Spec.ImagePullSecrets = append(rabbitmqCluster.Spec.ImagePullSecrets, corev1.LocalObjectReference{Name: reference})
+			}
+		}
+		if requeue, err := r.updateRabbitmqCluster(ctx, rabbitmqCluster, "image pull secrets"); err != nil {
+			return requeue, err
+		}
+	}
+
+	if rabbitmqCluster.UsesDefaultUserUpdaterImage() {
+		rabbitmqCluster.Spec.SecretBackend.Vault.DefaultUserUpdaterImage = &r.DefaultUserUpdaterImage
+		if requeue, err := r.updateRabbitmqCluster(ctx, rabbitmqCluster, "default user image"); err != nil {
+			return requeue, err
+		}
+	}
+	return 0, nil
+}
+
+// updateRabbitmqCluster updates a RabbitmqCluster with the given definition
+// it returns a 2 seconds requeue request if update failed due to conflict error
+func (r *RabbitmqClusterReconciler) updateRabbitmqCluster(ctx context.Context, rabbitmqCluster *rabbitmqv1beta1.RabbitmqCluster, updateType string) (time.Duration, error) {
+	logger := ctrl.LoggerFrom(ctx)
+	if err := r.Update(ctx, rabbitmqCluster); err != nil {
+		if k8serrors.IsConflict(err) {
+			logger.Info(fmt.Sprintf("failed to update %s because of conflict; requeueing...", updateType),
+				"namespace", rabbitmqCluster.Namespace,
+				"name", rabbitmqCluster.Name)
+			return 2 * time.Second, nil
+		}
+		return 0, err
+	}
+	return 0, nil
+}

controllers/reconcile_operator_defaults_test.go

@@ -0,0 +1,64 @@
+package controllers_test
+
+import (
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+	. "github.com/onsi/gomega/gstruct"
+	rabbitmqv1beta1 "github.com/rabbitmq/cluster-operator/api/v1beta1"
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/types"
+)
+
+var _ = Describe("ReconcileOperatorDefaults", func() {
+	var cluster *rabbitmqv1beta1.RabbitmqCluster
+
+	BeforeEach(func() {
+		cluster = &rabbitmqv1beta1.RabbitmqCluster{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "test-default",
+				Namespace: "default",
+			},
+			Spec: rabbitmqv1beta1.RabbitmqClusterSpec{
+				SecretBackend: rabbitmqv1beta1.SecretBackend{
+					Vault: &rabbitmqv1beta1.VaultSpec{
+						DefaultUserPath: "some-path",
+					},
+				},
+			},
+		}
+
+		Expect(client.Create(ctx, cluster)).To(Succeed())
+		waitForClusterCreation(ctx, cluster, client)
+	})
+
+	AfterEach(func() {
+		Expect(client.Delete(ctx, cluster)).To(Succeed())
+	})
+
+	It("handles operator defaults correctly", func() {
+		fetchedCluster := &rabbitmqv1beta1.RabbitmqCluster{}
+		Expect(client.Get(ctx, types.NamespacedName{Name: cluster.Name, Namespace: cluster.Namespace}, fetchedCluster)).To(Succeed())
+
+		By("setting the image spec with the default image")
+		Expect(fetchedCluster.Spec.Image).To(Equal(defaultRabbitmqImage))
+
+		By("setting the default user updater image to the controller default")
+		Expect(fetchedCluster.Spec.SecretBackend.Vault.DefaultUserUpdaterImage).To(PointTo(Equal(defaultUserUpdaterImage)))
+
+		By("setting the default imagePullSecrets")
+		Expect(statefulSet(ctx, cluster).Spec.Template.Spec.ImagePullSecrets).To(ConsistOf(
+			[]corev1.LocalObjectReference{
+				{
+					Name: "image-secret-1",
+				},
+				{
+					Name: "image-secret-2",
+				},
+				{
+					Name: "image-secret-3",
+				},
+			},
+		))
+	})
+})