Merge pull request #8599 from rook/mergify/bp/release-1.7/pr-8566

ceph: merge toleration for osd/prepareOSD pod if specified both places (backport #8566)

pkg/apis/ceph.rook.io/v1/placement.go

@@ -15,7 +15,9 @@ limitations under the License.
 */
 package v1
 
-import v1 "k8s.io/api/core/v1"
+import (
+	v1 "k8s.io/api/core/v1"
+)
 
 func (p PlacementSpec) All() Placement {
 	return p[KeyAll]
@@ -36,7 +38,7 @@ func (p Placement) ApplyToPodSpec(t *v1.PodSpec) {
 		t.Affinity.PodAntiAffinity = p.PodAntiAffinity.DeepCopy()
 	}
 	if p.Tolerations != nil {
-		t.Tolerations = p.Tolerations
+		t.Tolerations = p.mergeTolerations(t.Tolerations)
 	}
 	if p.TopologySpreadConstraints != nil {
 		t.TopologySpreadConstraints = p.TopologySpreadConstraints
@@ -90,6 +92,15 @@ func (p Placement) mergeNodeAffinity(nodeAffinity *v1.NodeAffinity) *v1.NodeAffi
 	return result
 }
 
+func (p Placement) mergeTolerations(tolerations []v1.Toleration) []v1.Toleration {
+	// no toleration is specified yet, return placement's toleration
+	if tolerations == nil {
+		return p.Tolerations
+	}
+
+	return append(p.Tolerations, tolerations...)
+}
+
 // Merge returns a Placement which results from merging the attributes of the
 // original Placement with the attributes of the supplied one. The supplied
 // Placement's attributes will override the original ones if defined.
@@ -105,7 +116,7 @@ func (p Placement) Merge(with Placement) Placement {
 		ret.PodAntiAffinity = with.PodAntiAffinity
 	}
 	if with.Tolerations != nil {
-		ret.Tolerations = with.Tolerations
+		ret.Tolerations = ret.mergeTolerations(with.Tolerations)
 	}
 	if with.TopologySpreadConstraints != nil {
 		ret.TopologySpreadConstraints = with.TopologySpreadConstraints

pkg/apis/ceph.rook.io/v1/placement_test.go

@@ -165,7 +165,6 @@ func TestPlacementApplyToPodSpec(t *testing.T) {
 		TopologySpreadConstraints: tc,
 	}
 	ps = &v1.PodSpec{
-		Tolerations:               placementTestGetTolerations("bar", "baz"),
 		TopologySpreadConstraints: placementTestGetTopologySpreadConstraints("rack"),
 	}
 	p.ApplyToPodSpec(ps)
@@ -182,6 +181,17 @@ func TestPlacementApplyToPodSpec(t *testing.T) {
 	}
 	p.ApplyToPodSpec(ps)
 	assert.Equal(t, 2, len(ps.Affinity.NodeAffinity.PreferredDuringSchedulingIgnoredDuringExecution))
+
+	p = Placement{NodeAffinity: na, PodAntiAffinity: antiaffinity}
+	to = placementTestGetTolerations("foo", "bar")
+	ps = &v1.PodSpec{
+		Tolerations: to,
+	}
+	p.ApplyToPodSpec(ps)
+	assert.Equal(t, 1, len(ps.Tolerations))
+	p = Placement{Tolerations: to, NodeAffinity: na, PodAntiAffinity: antiaffinity}
+	p.ApplyToPodSpec(ps)
+	assert.Equal(t, 2, len(ps.Tolerations))
 }
 
 func TestPlacementMerge(t *testing.T) {
@@ -218,9 +228,25 @@ func TestPlacementMerge(t *testing.T) {
 		Tolerations:               to,
 		TopologySpreadConstraints: tc,
 	}
+	var ts int64 = 10
 	expected = Placement{
-		NodeAffinity:              na,
-		Tolerations:               to,
+		NodeAffinity: na,
+		Tolerations: []v1.Toleration{
+			{
+				Key:               "bar",
+				Operator:          v1.TolerationOpExists,
+				Value:             "baz",
+				Effect:            v1.TaintEffectNoSchedule,
+				TolerationSeconds: &ts,
+			},
+			{
+				Key:               "foo",
+				Operator:          v1.TolerationOpExists,
+				Value:             "bar",
+				Effect:            v1.TaintEffectNoSchedule,
+				TolerationSeconds: &ts,
+			},
+		},
 		TopologySpreadConstraints: tc,
 	}
 	merged = original.Merge(with)
@@ -302,3 +328,33 @@ func placementTestGenerateNodeAffinity() *v1.NodeAffinity {
 		},
 	}
 }
+
+func TestMergeToleration(t *testing.T) {
+	// placement is nil
+	p := Placement{}
+	result := p.mergeTolerations(nil)
+	assert.Nil(t, result)
+
+	placementToleration := []v1.Toleration{
+		{
+			Key:      "foo",
+			Operator: v1.TolerationOpEqual,
+		},
+	}
+
+	p.Tolerations = placementToleration
+	result = p.mergeTolerations(nil)
+	assert.Equal(t, p.Tolerations, result)
+
+	newToleration := []v1.Toleration{
+		{
+			Key:      "new",
+			Operator: v1.TolerationOpExists,
+		},
+	}
+
+	result = p.mergeTolerations(newToleration)
+	assert.Equal(t, 2, len(result))
+	assert.Equal(t, placementToleration[0].Key, result[0].Key)
+	assert.Equal(t, newToleration[0].Key, result[1].Key)
+}

pkg/operator/ceph/cluster/osd/spec.go

@@ -747,7 +747,7 @@ func (c *Cluster) applyAllPlacementIfNeeded(d *v1.PodSpec) {
 	// - For non-PVCs: `placement.all` and `placement.osd`
 	// - For PVCs: `placement.all` and inside the storageClassDeviceSet from the `placement` or `preparePlacement`
 
-	// The placement from these sources will be merged by default (if onlyApplyOSDPlacement is false) in case of NodeAffinity,
+	// The placement from these sources will be merged by default (if onlyApplyOSDPlacement is false) in case of NodeAffinity and toleration,
 	// in case of other placement rule like PodAffinity, PodAntiAffinity... it will override last placement with the current placement applied,
 	// See ApplyToPodSpec().