Merge pull request #17288 from lumir-sliva/relax-stretch-crush-weight-check

mon: relax balanced CRUSH weight check for stretch mode

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.

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)