Merge pull request #2983 from openshift-cherrypick-robot/cherry-pick-…

…2973-to-release-4.18 DFBUGS-1285: [release-4.18] Remove duplicate placements for rook-ceph dameons due to specifying with all & specific key
red-hat-storage · Jan 29, 2025 · 19d9fa2 · 19d9fa2
2 parents efb257c + 3082120
commit 19d9fa2
Show file tree

Hide file tree

Showing 4 changed files with 21 additions and 41 deletions.
diff --git a/controllers/defaults/placements.go b/controllers/defaults/placements.go
@@ -42,18 +42,12 @@ var (
 		},
 
 		"osd": {
-			Tolerations: []corev1.Toleration{
-				getOcsToleration(),
-			},
 			TopologySpreadConstraints: []corev1.TopologySpreadConstraint{
 				getTopologySpreadConstraintsSpec(1, []string{osdLabelSelector}),
 			},
 		},
 
 		"osd-prepare": {
-			Tolerations: []corev1.Toleration{
-				getOcsToleration(),
-			},
 			TopologySpreadConstraints: []corev1.TopologySpreadConstraint{
 				getTopologySpreadConstraintsSpec(1, []string{osdLabelSelector, osdPrepareLabelSelector}),
 			},

diff --git a/controllers/storagecluster/cephcluster_test.go b/controllers/storagecluster/cephcluster_test.go
@@ -539,34 +539,29 @@ func TestStorageClassDeviceSetCreation(t *testing.T) {
 	sc3.Spec.LabelSelector = &emptyLabelSelector
 
 	cases := []struct {
-		label                string
-		sc                   *ocsv1.StorageCluster
-		topologyKey          string
-		lenOfMatchExpression int
+		label       string
+		sc          *ocsv1.StorageCluster
+		topologyKey string
 	}{
 		{
-			label:                "case 1",
-			sc:                   sc1,
-			topologyKey:          "zone",
-			lenOfMatchExpression: 1,
+			label:       "case 1",
+			sc:          sc1,
+			topologyKey: "zone",
 		},
 		{
-			label:                "case 2",
-			sc:                   sc2,
-			topologyKey:          "zone",
-			lenOfMatchExpression: 1,
+			label:       "case 2",
+			sc:          sc2,
+			topologyKey: "zone",
 		},
 		{
-			label:                "case 3",
-			sc:                   sc3,
-			topologyKey:          "zone",
-			lenOfMatchExpression: 0,
+			label:       "case 3",
+			sc:          sc3,
+			topologyKey: "zone",
 		},
 		{
-			label:                "case 4",
-			sc:                   sc4,
-			topologyKey:          "rack",
-			lenOfMatchExpression: 1,
+			label:       "case 4",
+			sc:          sc4,
+			topologyKey: "rack",
 		},
 	}
 
@@ -597,12 +592,7 @@ func TestStorageClassDeviceSetCreation(t *testing.T) {
 			} else {
 				assert.DeepEqual(t, getPlacement(c.sc, "osd"), scds.Placement)
 			}
-			if c.lenOfMatchExpression == 0 {
-				assert.Assert(t, is.Nil(scds.Placement.NodeAffinity))
-			} else {
-				matchExpressions := scds.Placement.NodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution.NodeSelectorTerms[0].MatchExpressions
-				assert.Equal(t, c.lenOfMatchExpression, len(matchExpressions))
-			}
+
 			topologyKey := scds.PreparePlacement.TopologySpreadConstraints[0].TopologyKey
 
 			if c.topologyKey == "rack" {

diff --git a/controllers/storagecluster/placement.go b/controllers/storagecluster/placement.go
@@ -62,7 +62,10 @@ func getPlacement(sc *ocsv1.StorageCluster, component string) rookCephv1.Placeme
 	// StorageCluster has no label selector, set the default node
 	// affinity.
 	if placement.NodeAffinity == nil && sc.Spec.LabelSelector == nil {
-		placement.NodeAffinity = defaults.DefaultNodeAffinity
+		// Don't add node affinity again for these rook-ceph daemons as it is already added via the "all" key
+		if component != "mgr" && component != "mon" && component != "osd" && component != "osd-prepare" {
+			placement.NodeAffinity = defaults.DefaultNodeAffinity
+		}
 	}
 
 	// If the StorageCluster specifies a label selector, append it to the

diff --git a/controllers/storagecluster/placement_test.go b/controllers/storagecluster/placement_test.go
@@ -177,7 +177,6 @@ func TestGetPlacement(t *testing.T) {
 					Tolerations:  defaults.DaemonPlacements["all"].Tolerations,
 				},
 				"mon": {
-					NodeAffinity:    defaults.DefaultNodeAffinity,
 					PodAntiAffinity: defaults.DaemonPlacements["mon"].PodAntiAffinity,
 				},
 				"mds": {
@@ -196,9 +195,7 @@ func TestGetPlacement(t *testing.T) {
 				"all": {
 					NodeAffinity: defaults.DefaultNodeAffinity,
 				},
-				"mon": {
-					NodeAffinity: defaults.DefaultNodeAffinity,
-				},
+				"mon": {},
 				"mds": {
 					NodeAffinity: defaults.DefaultNodeAffinity,
 				},
@@ -323,7 +320,6 @@ func TestGetPlacement(t *testing.T) {
 					Tolerations:  defaults.DaemonPlacements["all"].Tolerations,
 				},
 				"mon": {
-					NodeAffinity:    defaults.DefaultNodeAffinity,
 					PodAntiAffinity: customMONPlacement.PodAntiAffinity,
 				},
 				"mds": {
@@ -389,7 +385,6 @@ func TestGetPlacement(t *testing.T) {
 				},
 
 				"mon": {
-					NodeAffinity: defaults.DefaultNodeAffinity,
 					PodAntiAffinity: &corev1.PodAntiAffinity{
 						RequiredDuringSchedulingIgnoredDuringExecution: []corev1.PodAffinityTerm{
 							{
@@ -429,7 +424,6 @@ func TestGetPlacement(t *testing.T) {
 					Tolerations:  defaults.DaemonPlacements["all"].Tolerations,
 				},
 				"mon": {
-					NodeAffinity:    defaults.DefaultNodeAffinity,
 					PodAntiAffinity: &corev1.PodAntiAffinity{},
 				},
 				"mds": {
@@ -475,7 +469,6 @@ func TestGetPlacement(t *testing.T) {
 					Tolerations:  defaults.DaemonPlacements["all"].Tolerations,
 				},
 				"mon": {
-					NodeAffinity: defaults.DefaultNodeAffinity,
 					Tolerations: []corev1.Toleration{
 						getOcsToleration(),
 					},