Prevent unnecessary RabbitMQ rolling restarts during scaling and cert rotation

Three changes to align with the upstream rabbitmq-cluster-operator and
prevent quorum queue corruption caused by unnecessary rolling restarts:

1. Remove TLS secret data and replica count from config hash

   Stop hashing TLS secret content (tlsSecretData, caSecretData) and
   the replica count into the pod-template annotation. The secret names
   are still hashed so that switching to a different cert triggers a
   restart, but content changes (cert-manager re-issuance, CA chain
   updates, rotation) no longer cause rolling restarts. TLS certificates
   are mounted via projected volumes that kubelet updates in-place, and
   RabbitMQ 4.x reloads them from disk automatically.

   During initial deployment the playbook scales RabbitMQ from 1 to 3
   replicas, which adds new DNS SANs to the Certificate CR and triggers
   cert-manager to re-issue. When secret data was hashed, this caused a
   rolling restart before quorum queue replicas had synced, leading to
   Raft log divergence and a permanent crash loop (ra_server assertion
   "PLIdx < LastApplied").

2. Make PreStop hook replica-count-independent

   The PreStop command previously varied based on spec.replicas (skipping
   quorum checks for single-replica clusters). This meant scaling from
   1 to 3 changed the pod template, triggering a restart of existing
   pods. Now the quorum check is conditional at runtime: it queries
   rabbit_nodes:list_running() and only runs await_online_quorum_plus_one
   when more than one node is active.

   Timeouts are reduced from 600s to 10s for quorum checks and 30s for
   drain, fitting within the 60s terminationGracePeriodSeconds. If
   quorum is already satisfied (normal rolling restart), the check passes
   instantly; in degraded scenarios it times out quickly rather than
   blocking.

3. Set explicit UpdateStrategy on the StatefulSet

   Always set RollingUpdate with Partition=0, matching the upstream
   cluster-operator. This prevents stale partition values (from migration
   or manual debugging) from silently blocking pod updates.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Author: lmiccini

internal/controller/rabbitmq/rabbitmq_controller.go

@@ -388,45 +388,22 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (result ct
 	}
 
 	// Calculate hash for config tracking.
-	// Include TLS secret content so that certificate rotation (e.g. by
-	// cert-manager) triggers a rolling restart of the StatefulSet pods.
+	// TLS cert content is NOT included: certs are mounted via projected
+	// volumes that kubelet updates in-place, and RabbitMQ 4.x reloads
+	// them automatically. Hashing cert data here would trigger a
+	// rolling restart on every cert-manager update, which can corrupt
+	// quorum queues when replicas haven't fully synced yet.
 	hashInput := map[string]interface{}{
 		"ipv6Enabled":       IPv6Enabled,
 		"fipsEnabled":       fipsEnabled,
 		"tlsSecret":         instance.Spec.TLS.SecretName,
+		"caSecret":          instance.Spec.TLS.CaSecretName,
 		"additionalConfig":  instance.Spec.Rabbitmq.AdditionalConfig,
 		"advancedConfig":    instance.Spec.Rabbitmq.AdvancedConfig,
 		"envConfig":         instance.Spec.Rabbitmq.EnvConfig,
 		"erlangInetConfig":  instance.Spec.Rabbitmq.ErlangInetConfig,
 		"additionalPlugins": instance.Spec.Rabbitmq.AdditionalPlugins,
 		"containerImage":    instance.Spec.ContainerImage,
-		"replicas":          instance.Spec.Replicas,
-	}
-	if instance.Spec.TLS.SecretName != "" {
-		tlsSecret := &corev1.Secret{}
-		if err := r.Get(ctx, types.NamespacedName{
-			Name:      instance.Spec.TLS.SecretName,
-			Namespace: instance.Namespace,
-		}, tlsSecret); err != nil {
-			if !k8s_errors.IsNotFound(err) {
-				return ctrl.Result{}, fmt.Errorf("failed to get TLS secret %s: %w", instance.Spec.TLS.SecretName, err)
-			}
-		} else {
-			hashInput["tlsSecretData"] = tlsSecret.Data
-		}
-		if instance.Spec.TLS.CaSecretName != "" && instance.Spec.TLS.CaSecretName != instance.Spec.TLS.SecretName {
-			caSecret := &corev1.Secret{}
-			if err := r.Get(ctx, types.NamespacedName{
-				Name:      instance.Spec.TLS.CaSecretName,
-				Namespace: instance.Namespace,
-			}, caSecret); err != nil {
-				if !k8s_errors.IsNotFound(err) {
-					return ctrl.Result{}, fmt.Errorf("failed to get CA secret %s: %w", instance.Spec.TLS.CaSecretName, err)
-				}
-			} else {
-				hashInput["caSecretData"] = caSecret.Data
-			}
-		}
 	}
 	configMapHash, err := util.ObjectHash(hashInput)
 	if err != nil {

internal/rabbitmq/statefulset.go

@@ -143,6 +143,12 @@ func StatefulSet(
 			ServiceName:         fmt.Sprintf("%s-nodes", r.Name),
 			Replicas:            replicas,
 			PodManagementPolicy: appsv1.ParallelPodManagement,
+			UpdateStrategy: appsv1.StatefulSetUpdateStrategy{
+				RollingUpdate: &appsv1.RollingUpdateStatefulSetStrategy{
+					Partition: ptr.To(int32(0)),
+				},
+				Type: appsv1.RollingUpdateStatefulSetStrategyType,
+			},
 			Selector: &metav1.LabelSelector{
 				MatchLabels: matchls,
 			},
@@ -391,11 +397,16 @@ func buildReadinessProbe(r *rabbitmqv1.RabbitMq) *corev1.Probe {
 // For multi-replica clusters, the PreStop hook waits for quorum safety
 // before draining. For single-replica clusters, quorum checks are
 // skipped since there are no other nodes to maintain quorum.
-func buildLifecycle(r *rabbitmqv1.RabbitMq) *corev1.Lifecycle {
-	preStopCmd := `if [ ! -z "$(cat /etc/pod-info/skipPreStopChecks)" ]; then exit 0; fi; rabbitmq-upgrade await_online_quorum_plus_one -t 600 && rabbitmq-upgrade await_online_synchronized_mirror -t 600 || true && rabbitmq-upgrade drain -t 600`
-	if r.Spec.Replicas != nil && *r.Spec.Replicas <= 1 {
-		preStopCmd = `if [ ! -z "$(cat /etc/pod-info/skipPreStopChecks)" ]; then exit 0; fi; rabbitmq-upgrade drain -t 600`
-	}
+func buildLifecycle(_ *rabbitmqv1.RabbitMq) *corev1.Lifecycle {
+	// The PreStop command must be identical regardless of the replica count
+	// so that scaling does not change the pod template and trigger a rolling
+	// restart. The quorum check is skipped at runtime when there is only one
+	// running node (where quorum+1 can never be satisfied and would block
+	// until timeout).
+	preStopCmd := `if [ ! -z "$(cat /etc/pod-info/skipPreStopChecks)" ]; then exit 0; fi; ` +
+		`if rabbitmqctl eval 'length(rabbit_nodes:list_running()) > 1.' 2>/dev/null | grep -q true; then ` +
+		`rabbitmq-upgrade await_online_quorum_plus_one -t 10 && rabbitmq-upgrade await_online_synchronized_mirror -t 10 || true; fi; ` +
+		`rabbitmq-upgrade drain -t 30`
 	return &corev1.Lifecycle{
 		PreStop: &corev1.LifecycleHandler{
 			Exec: &corev1.ExecAction{

test/functional/rabbitmq_controller_test.go

@@ -2240,7 +2240,7 @@ var _ = Describe("RabbitMQ Controller", func() {
 		})
 	})
 
-	When("TLS secret is rotated, config hash changes to trigger rolling restart", func() {
+	When("TLS secret data is rotated, config hash remains stable", func() {
 		var certSecret *corev1.Secret
 
 		BeforeEach(func() {
@@ -2254,7 +2254,7 @@ var _ = Describe("RabbitMQ Controller", func() {
 			DeferCleanup(th.DeleteInstance, rabbitmq)
 		})
 
-		It("should update the config-hash annotation on the StatefulSet when TLS secret changes", func() {
+		It("should NOT update the config-hash annotation when TLS secret data changes", func() {
 			SimulateRabbitMQClusterReady(rabbitmqName)
 
 			// Get initial config-hash from the StatefulSet
@@ -2274,12 +2274,14 @@ var _ = Describe("RabbitMQ Controller", func() {
 				g.Expect(th.K8sClient.Update(th.Ctx, secret)).Should(Succeed())
 			}, timeout, interval).Should(Succeed())
 
-			// Verify config-hash changed on the StatefulSet (triggers rolling restart)
-			Eventually(func(g Gomega) {
+			// Verify config-hash did NOT change — cert rotation is handled
+			// by projected volumes and RabbitMQ 4.x automatic reload, not
+			// by rolling restart. Hashing cert data caused quorum queue
+			// corruption during initial deployment.
+			Consistently(func(g Gomega) {
 				sts := GetRabbitMQStatefulSet(rabbitmqName)
 				newHash := sts.Spec.Template.Annotations["config-hash"]
-				g.Expect(newHash).NotTo(BeEmpty())
-				g.Expect(newHash).NotTo(Equal(initialHash), "config-hash should change when TLS secret is rotated")
+				g.Expect(newHash).To(Equal(initialHash), "config-hash should not change on TLS secret data rotation")
 			}, timeout, interval).Should(Succeed())
 		})
 	})