From 849d441ed5a1f8c1f9395bc4ef5fc7ea2b6ccf59 Mon Sep 17 00:00:00 2001 From: Andrew Stucki Date: Wed, 21 Oct 2026 11:45:34 -0400 Subject: [PATCH] operator: settle each scale-down step before decommissioning the next Lowering statefulset.replicas by more than one broker deleted several broker Pods at once, and only the first was decommissioned. Since 17434909e, reconcilePools no longer aborts the chain while CheckScale is false, so reconcileDecommission ran mid-scale. The previous step's terminating Pod was still LastPod, and its decommissioned broker looked already removed, so each pass patched away another broker without decommissioning it. Partitions with a majority on those brokers lost quorum. Fixes K8S-955. --- .../unreleased/operator-Fixed-20261021-170000.yaml | 4 ++++ .../controller/redpanda/multicluster_controller.go | 11 +++++++---- .../controller/redpanda/redpanda_controller.go | 7 +++++++ .../controller/redpanda/redpanda_controller_test.go | 11 +++++++---- 4 files changed, 25 insertions(+), 8 deletions(-) create mode 100644 .changes/unreleased/operator-Fixed-20261021-170000.yaml diff --git a/.changes/unreleased/operator-Fixed-20261021-170000.yaml b/.changes/unreleased/operator-Fixed-20261021-170000.yaml new file mode 100644 index 000000000..7a6a94ef1 --- /dev/null +++ b/.changes/unreleased/operator-Fixed-20261021-170000.yaml @@ -0,0 +1,4 @@ +project: operator +kind: Fixed +body: 'Scaling a `Redpanda` or `StretchCluster` StatefulSet down by more than one broker in a single change could delete several broker Pods at once, and only the first broker was decommissioned. Partitions with a majority of replicas on the removed brokers lost quorum, and the remaining decommissions stalled. The operator now waits for each removed Pod to be fully gone before it decommissions the next broker.' +time: 2026-10-21T17:00:00.000000000-07:00 diff --git a/operator/internal/controller/redpanda/multicluster_controller.go b/operator/internal/controller/redpanda/multicluster_controller.go index c442b1d6d..ffae1f39f 100644 --- a/operator/internal/controller/redpanda/multicluster_controller.go +++ b/operator/internal/controller/redpanda/multicluster_controller.go @@ -1394,10 +1394,6 @@ func (r *MulticlusterReconciler) reconcilePools(ctx context.Context, state *stre // which must run precisely while brokers are unready during an outage. The // readiness poll is applied at the end of Reconcile instead — see // PoolTracker.ScaledUpButNoneReady and the tail of Reconcile. - // - // The CheckScale early-return above is a separate, deliberate gate (don't - // stack pool mutations mid-scale); it can still defer Phase 3 during an active - // scale/roll, and recovery there relies on eventual consistency across passes. return ctrl.Result{}, nil } @@ -1524,6 +1520,13 @@ func (r *MulticlusterReconciler) reconcileDecommission(ctx context.Context, stat // next scale down any over-provisioned pools, patching them to use the new spec // and decommissioning any nodes as needed for _, set := range state.pools.ToScaleDown() { + // Until the previous step's pod is gone it is still LastPod, and its + // decommissioned broker reads as already removed, so scaleDown would + // drop the next broker without decommissioning it. + if !state.pools.CheckScale(ctx) { + logger.V(log.DebugLevel).Info("previous scale-down step still settling, deferring") + return ctrl.Result{RequeueAfter: requeueTimeout}, nil + } requeue, err := r.scaleDown(ctx, state.admin, state.cluster, set, brokerMap, downNodes) result := ctrl.Result{} if requeue { diff --git a/operator/internal/controller/redpanda/redpanda_controller.go b/operator/internal/controller/redpanda/redpanda_controller.go index f17179175..70be5503d 100644 --- a/operator/internal/controller/redpanda/redpanda_controller.go +++ b/operator/internal/controller/redpanda/redpanda_controller.go @@ -1077,6 +1077,13 @@ func (r *RedpandaReconciler) reconcileDecommission(ctx context.Context, state *c // next scale down any over-provisioned pools, patching them to use the new spec // and decommissioning any nodes as needed for _, set := range state.pools.ToScaleDown() { + // Until the previous step's pod is gone it is still LastPod, and its + // decommissioned broker reads as already removed, so scaleDown would + // drop the next broker without decommissioning it. + if !state.pools.CheckScale(ctx) { + logger.V(log.DebugLevel).Info("previous scale-down step still settling, deferring") + return ctrl.Result{RequeueAfter: requeueTimeout}, nil + } requeue, err := r.scaleDown(ctx, state.admin, state.cluster, set, brokerMap) result := ctrl.Result{} if requeue { diff --git a/operator/internal/controller/redpanda/redpanda_controller_test.go b/operator/internal/controller/redpanda/redpanda_controller_test.go index e7e407661..46e3da131 100644 --- a/operator/internal/controller/redpanda/redpanda_controller_test.go +++ b/operator/internal/controller/redpanda/redpanda_controller_test.go @@ -792,17 +792,20 @@ func (s *RedpandaControllerSuite) TestScaling() { t, ctx, cancel, c := s.setup() defer cancel() + // Stable only requires Healthy to be set, not True, so a scale-down that + // drops brokers without decommissioning them still reports Stable. isStable := func(obj client.Object, err error) (bool, error) { if err != nil { return false, err } rp := obj.(*redpandav1alpha2.Redpanda) - for _, cond := range rp.Status.Conditions { - if cond.Type == statuses.ClusterStable { - return cond.ObservedGeneration == rp.Generation && cond.Status == metav1.ConditionTrue, nil + for _, condType := range []string{statuses.ClusterStable, statuses.ClusterHealthy} { + cond := apimeta.FindStatusCondition(rp.Status.Conditions, condType) + if cond == nil || cond.ObservedGeneration != rp.Generation || cond.Status != metav1.ConditionTrue { + return false, nil } } - return false, nil + return true, nil } rp := s.minimalRP()