From 27f95f2febd17b1bbb0f650106ac3248a31cdf08 Mon Sep 17 00:00:00 2001 From: Lumir Sliva <61183145+lumir-sliva@users.noreply.github.com> Date: Wed, 1 Apr 2026 15:13:35 +0200 Subject: [PATCH] mon: relax balanced CRUSH weight check for stretch mode Ceph relaxed the stretch mode CRUSH weight check in ceph/ceph#66580 to allow weights within 10 percent of each other. This change is available in Ceph Umbrella and was backported to tentacle v20.2.2 via ceph/ceph number 67790. The squid backport is still pending. Signed-off-by: Lumir Sliva <61183145+lumir-sliva@users.noreply.github.com> --- pkg/operator/ceph/cluster/mon/mon.go | 38 +++++++++++++-- pkg/operator/ceph/cluster/mon/mon_test.go | 58 ++++++++++++++++++++++- 2 files changed, 92 insertions(+), 4 deletions(-) diff --git a/pkg/operator/ceph/cluster/mon/mon.go b/pkg/operator/ceph/cluster/mon/mon.go index c19bfaa41..ad7b2953d 100644 --- a/pkg/operator/ceph/cluster/mon/mon.go +++ b/pkg/operator/ceph/cluster/mon/mon.go @@ -117,6 +117,11 @@ var ( // hook for tests to override waitForMonitorScheduling = realWaitForMonitorScheduling + + // relaxedStretchCrushWeightCheckVersion is the first Ceph release that allows + // stretch mode to be configured with up to 10% CRUSH weight difference between + // the two zones (ceph/ceph#66580, backported via ceph/ceph#67790 to tentacle v20.2.2). + relaxedStretchCrushWeightCheckVersion = cephver.CephVersion{Major: 20, Minor: 2, Extra: 2} ) // Cluster represents the Rook and environment configuration settings needed to set up Ceph mons. @@ -471,13 +476,20 @@ func (c *Cluster) readyToConfigureArbiter(checkOSDPods bool) (bool, error) { log.NamespacedInfo(c.Namespace, logger, "found %s %q in CRUSH map with weight %d", failureDomain, bucket.Name, bucket.Weight) zoneCount++ - // check that the weights of the failure domains are all the same if zoneWeight == -1 { // found the first matching bucket zoneWeight = bucket.Weight } else if zoneWeight != bucket.Weight { - log.NamespacedInfo(c.Namespace, logger, "found failure domains that have different weights") - return false, nil + // Ceph relaxed this check in ceph/ceph#66580 to allow up to 10% difference. + // The relaxed check is in Umbrella (v21+) and was backported to tentacle + // (ceph/ceph#67790, first appearing in v20.2.2). The squid backport + // (ceph/ceph#67789) is still pending and will be gated here once it lands. + if c.ClusterInfo.CephVersion.IsAtLeast(relaxedStretchCrushWeightCheckVersion) && crushWeightsBalanced(zoneWeight, bucket.Weight) { + log.NamespacedInfo(c.Namespace, logger, "found failure domains with different weights (%d vs %d) but within 10%% tolerance", zoneWeight, bucket.Weight) + } else { + log.NamespacedInfo(c.Namespace, logger, "found failure domains that have different weights (%d vs %d)", zoneWeight, bucket.Weight) + return false, nil + } } } } @@ -492,6 +504,26 @@ func (c *Cluster) readyToConfigureArbiter(checkOSDPods bool) (bool, error) { return true, nil } +// crushWeightsBalanced returns true if two CRUSH bucket weights are within 10% of each other. +// This matches Ceph's mon_stretch_max_bucket_weight_delta default. +func crushWeightsBalanced(a, b int) bool { + if a == b { + return true + } + maxWeight := a + if b > a { + maxWeight = b + } + if maxWeight == 0 { + return false + } + diff := a - b + if diff < 0 { + diff = -diff + } + return float64(diff)/float64(maxWeight) <= 0.1 +} + // ensureMonsRunning is called in two scenarios: // 1. To create a new mon and wait for it to join quorum (requireAllInQuorum = true). This method will be called multiple times // to add a mon until we have reached the desired number of mons. diff --git a/pkg/operator/ceph/cluster/mon/mon_test.go b/pkg/operator/ceph/cluster/mon/mon_test.go index 6a4ed70a1..fa40c923c 100644 --- a/pkg/operator/ceph/cluster/mon/mon_test.go +++ b/pkg/operator/ceph/cluster/mon/mon_test.go @@ -1008,6 +1008,7 @@ func TestCheckIfArbiterReady(t *testing.T) { }, } crushZoneCount := 0 + zone1WeightOverride := 0 // when non-zero, override zone 1's weight (i%2==1) balanced := true executor := &exectest.MockExecutor{ MockExecuteCommandWithOutput: func(command string, args ...string) (string, error) { @@ -1024,6 +1025,9 @@ func TestCheckIfArbiterReady(t *testing.T) { // simulate unbalanced with every other zone having half the weight weight = 1028 } + if zone1WeightOverride != 0 && i%2 == 1 { + weight = zone1WeightOverride + } crushBuckets = crushBuckets + fmt.Sprintf(`,{"id": -%d,"name": "zone%d","type_id": 1,"type_name": "zone","weight": %d} ,{"id": -%d,"name": "zone%d~ssd","type_id": 1,"type_name": "zone","weight": 2056}`, i+5, i, weight, i+6, i) @@ -1053,12 +1057,44 @@ func TestCheckIfArbiterReady(t *testing.T) { assert.True(t, ready) assert.NoError(t, err) - // Valid, except the CRUSH map is not balanced + // Valid, except the CRUSH map is not balanced (50% difference) balanced = false ready, err = c.readyToConfigureArbiter(false) assert.False(t, ready) assert.NoError(t, err) + // Slightly unbalanced within 10%, but rejected on Ceph without the relaxed check + balanced = true + zone1WeightOverride = 2047 // ~0.4% difference from 2056 + ready, err = c.readyToConfigureArbiter(false) + assert.False(t, ready) + assert.NoError(t, err) + + // Still rejected on tentacle v20.2.1 (pre-backport) + c.ClusterInfo.CephVersion = cephver.CephVersion{Major: 20, Minor: 2, Extra: 1} + ready, err = c.readyToConfigureArbiter(false) + assert.False(t, ready) + assert.NoError(t, err) + + // Accepted on tentacle v20.2.2 (ceph/ceph#67790) + c.ClusterInfo.CephVersion = cephver.CephVersion{Major: 20, Minor: 2, Extra: 2} + ready, err = c.readyToConfigureArbiter(false) + assert.True(t, ready) + assert.NoError(t, err) + + // Accepted on Umbrella+ (ceph/ceph#66580) + c.ClusterInfo.CephVersion = cephver.Umbrella + ready, err = c.readyToConfigureArbiter(false) + assert.True(t, ready) + assert.NoError(t, err) + + // Over 10% unbalanced, rejected even on supported versions + zone1WeightOverride = 1850 // ~10.0% difference from 2056 + ready, err = c.readyToConfigureArbiter(false) + assert.False(t, ready) + assert.NoError(t, err) + zone1WeightOverride = 0 + // Too many zones in the CRUSH map crushZoneCount = 3 balanced = true @@ -1067,6 +1103,26 @@ func TestCheckIfArbiterReady(t *testing.T) { assert.Error(t, err) } +func TestCrushWeightsBalanced(t *testing.T) { + // Equal weights + assert.True(t, crushWeightsBalanced(2056, 2056)) + + // Within 10% + assert.True(t, crushWeightsBalanced(2056, 1900)) // ~7.6% diff + assert.True(t, crushWeightsBalanced(2747124, 2747111)) // from the issue report + + // Exactly 10% + assert.True(t, crushWeightsBalanced(1000, 900)) + + // Beyond 10% + assert.False(t, crushWeightsBalanced(2056, 1028)) // 50% diff + assert.False(t, crushWeightsBalanced(1000, 899)) + + // Zero weights + assert.True(t, crushWeightsBalanced(0, 0)) + assert.False(t, crushWeightsBalanced(100, 0)) +} + func TestSkipReconcile(t *testing.T) { c := New(context.TODO(), &clusterd.Context{Clientset: test.New(t, 1), ConfigDir: t.TempDir()}, "ns", cephv1.ClusterSpec{}, cephclient.NewMinimumOwnerInfoWithOwnerRef()) c.ClusterInfo = clienttest.CreateTestClusterInfo(1)