Merge pull request #598 from rabbitmq/failed-set-plugins

Fail to set plugins (or any other necessary cli commands) should result in ReconcileSuccess set to false

Author: ChunyiLyu

controllers/rabbitmqcluster_controller.go

@@ -186,23 +186,28 @@ func (r *RabbitmqClusterReconciler) Reconcile(ctx context.Context, req ctrl.Requ
 		return ctrl.Result{RequeueAfter: requeueAfter}, err
 	}
 
-	// Set ReconcileSuccess to true here because all CRUD operations to Kube API related
-	// to child resources returned no error
-	rabbitmqCluster.Status.SetCondition(status.ReconcileSuccess, corev1.ConditionTrue, "Success", "Created or Updated all child resources")
-	if writerErr := r.Status().Update(ctx, rabbitmqCluster); writerErr != nil {
-		logger.Error(writerErr, "Failed to Update Custom Resource status")
-	}
-
 	if err := r.setDefaultUserStatus(ctx, rabbitmqCluster); err != nil {
 		return ctrl.Result{}, err
 	}
 
 	// By this point the StatefulSet may have finished deploying. Run any
 	// post-deploy steps if so, or requeue until the deployment is finished.
 	if requeueAfter, err := r.runRabbitmqCLICommandsIfAnnotated(ctx, rabbitmqCluster); err != nil || requeueAfter > 0 {
+		if err != nil {
+			rabbitmqCluster.Status.SetCondition(status.ReconcileSuccess, corev1.ConditionFalse, "FailedCLICommand", err.Error())
+			if writerErr := r.Status().Update(ctx, rabbitmqCluster); writerErr != nil {
+				logger.Error(writerErr, "Failed to update ReconcileSuccess condition state")
+			}
+		}
 		return ctrl.Result{RequeueAfter: requeueAfter}, err
 	}
 
+	// Set ReconcileSuccess to true after all reconciliation steps have finished with no error
+	rabbitmqCluster.Status.SetCondition(status.ReconcileSuccess, corev1.ConditionTrue, "Success", "Finish reconciling")
+	if writerErr := r.Status().Update(ctx, rabbitmqCluster); writerErr != nil {
+		logger.Error(writerErr, "Failed to Update Custom Resource status")
+	}
+
 	logger.Info("Finished reconciling")
 
 	return ctrl.Result{}, nil

controllers/rabbitmqcluster_controller_test.go

@@ -676,33 +676,6 @@ var _ = Describe("RabbitmqClusterController", func() {
 					return "ReconcileSuccess status: condition not present"
 				}, 5).Should(Equal("ReconcileSuccess status: False"))
 			})
-
-			By("transitioning to True when a valid spec in updated", func() {
-				// We have to Get() the CR again because Reconcile() changes the object
-				// If we try to Update() without getting the latest version of the CR
-				// We are very likely to hit a Conflict error
-				Expect(client.Get(ctx, runtimeClient.ObjectKey{
-					Name:      crName,
-					Namespace: defaultNamespace,
-				}, cluster)).To(Succeed())
-				cluster.Spec.Service.Annotations = map[string]string{"thisIs": "valid"}
-				Expect(client.Update(ctx, cluster)).To(Succeed())
-
-				Eventually(func() string {
-					someRabbit := &rabbitmqv1beta1.RabbitmqCluster{}
-					Expect(client.Get(ctx, runtimeClient.ObjectKey{
-						Name:      crName,
-						Namespace: defaultNamespace,
-					}, someRabbit)).To(Succeed())
-
-					for i := range someRabbit.Status.Conditions {
-						if someRabbit.Status.Conditions[i].Type == status.ReconcileSuccess {
-							return fmt.Sprintf("ReconcileSuccess status: %s", someRabbit.Status.Conditions[i].Status)
-						}
-					}
-					return "ReconcileSuccess status: condition not present"
-				}, 5).Should(Equal("ReconcileSuccess status: True"))
-			})
 		})
 	})
 

controllers/reconcile_cli.go

@@ -25,7 +25,6 @@ func (r *RabbitmqClusterReconciler) runRabbitmqCLICommandsIfAnnotated(ctx contex
 		logger.Info("not all replicas ready yet; requeuing request to run RabbitMQ CLI commands")
 		return 15 * time.Second, nil
 	}
-
 	// Retrieve the plugins config map, if it exists.
 	pluginsConfig, err := r.configMap(ctx, rmq, rmq.ChildResourceName(resource.PluginsConfigName))
 	if client.IgnoreNotFound(err) != nil {
@@ -72,12 +71,10 @@ func (r *RabbitmqClusterReconciler) runEnableFeatureFlagsCommand(ctx context.Con
 	cmd := "set -eo pipefail; rabbitmqctl -s list_feature_flags name state stability | (grep 'disabled\\sstable$' || true) | cut -f 1 | xargs -r -n1 rabbitmqctl enable_feature_flag"
 	stdout, stderr, err := r.exec(rmq.Namespace, podName, "rabbitmq", "bash", "-c", cmd)
 	if err != nil {
-		logger.Error(err, "failed to enable all feature flags",
-			"pod", podName,
-			"command", cmd,
-			"stdout", stdout,
-			"stderr", stderr)
-		return err
+		msg := "failed to enable all feature flags on pod"
+		logger.Error(err, msg, "pod", podName, "command", cmd, "stdout", stdout, "stderr", stderr)
+		r.Recorder.Event(rmq, corev1.EventTypeWarning, "FailedReconcile", fmt.Sprintf("%s %s", msg, podName))
+		return fmt.Errorf("%s %s: %v", msg, podName, err)
 	}
 	logger.Info("successfully enabled all feature flags")
 	return r.deleteAnnotation(ctx, sts, stsCreateAnnotation)
@@ -95,29 +92,26 @@ func (r *RabbitmqClusterReconciler) runSetPluginsCommand(ctx context.Context, rm
 		cmd := fmt.Sprintf("rabbitmq-plugins set %s", plugins.AsString(" "))
 		stdout, stderr, err := r.exec(rmq.Namespace, podName, "rabbitmq", "sh", "-c", cmd)
 		if err != nil {
-			logger.Error(err, "failed to set plugins",
-				"pod", podName,
-				"command", cmd,
-				"stdout", stdout,
-				"stderr", stderr)
-			return err
+			msg := "failed to set plugins on pod"
+			logger.Error(err, msg, "pod", podName, "command", cmd, "stdout", stdout, "stderr", stderr)
+			r.Recorder.Event(rmq, corev1.EventTypeWarning, "FailedReconcile", fmt.Sprintf("%s %s", msg, podName))
+			return fmt.Errorf("%s %s: %v", msg, podName, err)
 		}
 	}
 	logger.Info("successfully set plugins")
 	return r.deleteAnnotation(ctx, configMap, pluginsUpdateAnnotation)
 }
 
 func (r *RabbitmqClusterReconciler) runQueueRebalanceCommand(ctx context.Context, rmq *rabbitmqv1beta1.RabbitmqCluster) error {
+	logger := ctrl.LoggerFrom(ctx)
 	podName := fmt.Sprintf("%s-0", rmq.ChildResourceName("server"))
 	cmd := "rabbitmq-queues rebalance all"
 	stdout, stderr, err := r.exec(rmq.Namespace, podName, "rabbitmq", "sh", "-c", cmd)
 	if err != nil {
-		ctrl.LoggerFrom(ctx).Error(err, "failed to run queue rebalance",
-			"pod", podName,
-			"command", cmd,
-			"stdout", stdout,
-			"stderr", stderr)
-		return err
+		msg := "failed to run queue rebalance on pod"
+		logger.Error(err, msg, "pod", podName, "command", cmd, "stdout", stdout, "stderr", stderr)
+		r.Recorder.Event(rmq, corev1.EventTypeWarning, "FailedReconcile", fmt.Sprintf("%s %s", msg, podName))
+		return fmt.Errorf("%s %s: %v", msg, podName, err)
 	}
 	return r.deleteAnnotation(ctx, rmq, queueRebalanceAnnotation)
 }