From 83a20249d1edfffb30e75073b653083ecbe323d3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Beno=C3=AEt=20Knecht?= <bknecht@protonmail.ch>
Date: Fri, 23 Aug 2024 06:53:38 +0200
Subject: [PATCH] core: fix Ceph monitor deployment in separate zones
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Previously, Rook would fail to create monitors for a `CephCluster` spec that
required them to be located in distinct failure domains:

```yaml
spec:
  mon:
    count: 3
    failureDomainLabel: topology.kubernetes.io/zone
    zones:
    - eu-central-1a
    - eu-central-1b
    - eu-central-1c
```

This was due to the fact that Rook was mistakenly getting the
`failureDomainLabel` from `spec.mon.stretchCluster.failureDomainLabel`.

This commit ensures `spec.mon.failureDomainLabel` is being used instead, which
fixes the cluster deployment for the spec above.

Signed-off-by: Benoît Knecht <bknecht@protonmail.ch>
(cherry picked from commit 1c45677f5e935e60b35b23d16ff53f7821b63f09)
---
 pkg/operator/ceph/cluster/mon/spec.go      |  2 +-
 pkg/operator/ceph/cluster/mon/spec_test.go | 66 ++++++++++++++++++++++
 2 files changed, 67 insertions(+), 1 deletion(-)

diff --git a/pkg/operator/ceph/cluster/mon/spec.go b/pkg/operator/ceph/cluster/mon/spec.go
index 140657f4d..9d79ace7b 100644
--- a/pkg/operator/ceph/cluster/mon/spec.go
+++ b/pkg/operator/ceph/cluster/mon/spec.go
@@ -79,7 +79,7 @@ func GetFailureDomainLabel(spec cephv1.ClusterSpec) string {
 	}
 
 	if spec.ZonesRequired() && spec.Mon.FailureDomainLabel != "" {
-		return spec.Mon.StretchCluster.FailureDomainLabel
+		return spec.Mon.FailureDomainLabel
 	}
 	// The default topology label is for a zone
 	return corev1.LabelZoneFailureDomainStable
diff --git a/pkg/operator/ceph/cluster/mon/spec_test.go b/pkg/operator/ceph/cluster/mon/spec_test.go
index a4d666429..1384d90de 100644
--- a/pkg/operator/ceph/cluster/mon/spec_test.go
+++ b/pkg/operator/ceph/cluster/mon/spec_test.go
@@ -256,3 +256,69 @@ func TestRequiredDuringScheduling(t *testing.T) {
 	testRequiredDuringScheduling(t, true, true, true)
 	testRequiredDuringScheduling(t, false, true, false)
 }
+
+func TestGetFailureDomainLabel(t *testing.T) {
+	type args struct {
+		spec cephv1.ClusterSpec
+	}
+	tests := []struct {
+		name string
+		args args
+		want string
+	}{
+		{
+			name: "stretch",
+			args: args{
+				spec: cephv1.ClusterSpec{
+					Mon: cephv1.MonSpec{
+						Count: 3,
+						StretchCluster: &cephv1.StretchClusterSpec{
+							FailureDomainLabel: "topology.kubernetes.io/region",
+							Zones: []cephv1.MonZoneSpec{
+								{Name: "eu-central-1"},
+								{Name: "eu-central-2"},
+								{Name: "eu-central-3"},
+							},
+						},
+					},
+				},
+			},
+			want: "topology.kubernetes.io/region",
+		},
+		{
+			name: "zones",
+			args: args{
+				spec: cephv1.ClusterSpec{
+					Mon: cephv1.MonSpec{
+						Count:              3,
+						FailureDomainLabel: "topology.kubernetes.io/zone",
+						Zones: []cephv1.MonZoneSpec{
+							{Name: "eu-central-1a"},
+							{Name: "eu-central-1b"},
+							{Name: "eu-central-1c"},
+						},
+					},
+				},
+			},
+			want: "topology.kubernetes.io/zone",
+		},
+		{
+			name: "default",
+			args: args{
+				spec: cephv1.ClusterSpec{
+					Mon: cephv1.MonSpec{
+						Count: 3,
+					},
+				},
+			},
+			want: "topology.kubernetes.io/zone",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := GetFailureDomainLabel(tt.args.spec); got != tt.want {
+				t.Errorf("GetFailureDomainLabel() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}