From c0918b98982e3e19f6ba764f1bb3e212e914e76d Mon Sep 17 00:00:00 2001 From: Chunyi Lyu Date: Tue, 14 Jun 2022 15:00:57 +0100 Subject: [PATCH] Extract operator defaults to a separate func - to keep Reconciler shorter and more concise --- controllers/rabbitmqcluster_controller.go | 45 +------------ controllers/reconcile_operator_defaults.go | 60 +++++++++++++++++ .../reconcile_operator_defaults_test.go | 64 +++++++++++++++++++ 3 files changed, 126 insertions(+), 43 deletions(-) create mode 100644 controllers/reconcile_operator_defaults.go create mode 100644 controllers/reconcile_operator_defaults_test.go diff --git a/controllers/rabbitmqcluster_controller.go b/controllers/rabbitmqcluster_controller.go index e989cb8c2..fc8d26a0f 100644 --- a/controllers/rabbitmqcluster_controller.go +++ b/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 diff --git a/controllers/reconcile_operator_defaults.go b/controllers/reconcile_operator_defaults.go new file mode 100644 index 000000000..608b080be --- /dev/null +++ b/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 +} diff --git a/controllers/reconcile_operator_defaults_test.go b/controllers/reconcile_operator_defaults_test.go new file mode 100644 index 000000000..96146cb2f --- /dev/null +++ b/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", + }, + }, + )) + }) +})