Merge pull request #8381 from subhamkrai/osd-placement-merge

ceph: add an option to enable/disable merge all placement

Documentation/ceph-cluster-crd.md

@@ -40,6 +40,7 @@ spec:
   storage:
     useAllNodes: true
     useAllDevices: true
+    onlyApplyOSDPlacement: false
 ```
 
 ## PVC-based Cluster
@@ -88,6 +89,7 @@ spec:
           volumeMode: Block
           accessModes:
             - ReadWriteOnce
+    onlyApplyOSDPlacement: false
 ```
 
 For a more advanced scenario, such as adding a dedicated device you can refer to the [dedicated metadata device for OSD on PVC section](#dedicated-metadata-and-wal-device-for-osd-on-pvc).
@@ -216,7 +218,9 @@ For more details on the mons and when to choose a number other than `3`, see the
   * `config`: Config settings applied to all OSDs on the node unless overridden by `devices`. See the [config settings](#osd-configuration-settings) below.
   * [storage selection settings](#storage-selection-settings)
   * [Storage Class Device Sets](#storage-class-device-sets)
-* `disruptionManagement`: The section for configuring management of daemon disruptions
+  * `onlyApplyOSDPlacement`: Whether the placement specific for OSDs is merged with the `all` placement. If `false`, the OSD placement will be merged with the `all` placement. If true, the `OSD placement will be applied` and the `all` placement will be ignored. The placement for OSDs is computed from several different places depending on the type of OSD:
+    - For non-PVCs: `placement.all` and `placement.osd`
+    - For PVCs: `placement.all` and inside the storageClassDeviceSet from the `placement` or `preparePlacement`
   * `managePodBudgets`: if `true`, the operator will create and manage PodDisruptionBudgets for OSD, Mon, RGW, and MDS daemons. OSD PDBs are managed dynamically via the strategy outlined in the [design](https://github.com/rook/rook/blob/master/design/ceph/ceph-managed-disruptionbudgets.md). The operator will block eviction of OSDs by default and unblock them safely when drains are detected.
   * `osdMaintenanceTimeout`: is a duration in minutes that determines how long an entire failureDomain like `region/zone/host` will be held in `noout` (in addition to the default DOWN/OUT interval) when it is draining. This is only relevant when  `managePodBudgets` is `true`. The default value is `30` minutes.
   * `manageMachineDisruptionBudgets`: if `true`, the operator will create and manage MachineDisruptionBudgets to ensure OSDs are only fenced when the cluster is healthy. Only available on OpenShift.

cluster/charts/rook-ceph/templates/resources.yaml

@@ -2132,6 +2132,8 @@ spec:
                         type: object
                       nullable: true
                       type: array
+                    onlyApplyOSDPlacement:
+                      type: boolean
                     storageClassDeviceSets:
                       items:
                         description: StorageClassDeviceSet is a storage class device set

cluster/examples/kubernetes/ceph/cluster-on-local-pvc.yaml

@@ -231,6 +231,8 @@ spec:
               volumeMode: Block
               accessModes:
                 - ReadWriteOnce
+    # when onlyApplyOSDPlacement is false, will merge both placement.All() and storageClassDeviceSets.Placement
+    onlyApplyOSDPlacement: false
   resources:
   #  prepareosd:
   #    limits:

cluster/examples/kubernetes/ceph/cluster-on-pvc.yaml

@@ -158,6 +158,8 @@ spec:
         #       - ReadWriteOnce
         # Scheduler name for OSD pod placement
         # schedulerName: osd-scheduler
+    # when onlyApplyOSDPlacement is false, will merge both placement.All() and storageClassDeviceSets.Placement.
+    onlyApplyOSDPlacement: false
   resources:
   #  prepareosd:
   #    limits:

cluster/examples/kubernetes/ceph/cluster.yaml

@@ -229,6 +229,8 @@ spec:
 #      config: # configuration can be specified at the node level which overrides the cluster level config
 #    - name: "172.17.4.301"
 #      deviceFilter: "^sd."
+    # when onlyApplyOSDPlacement is false, will merge both placement.All() and placement.osd
+    onlyApplyOSDPlacement: false
   # The section for configuring management of daemon disruptions during upgrade or fencing.
   disruptionManagement:
     # If true, the operator will create and manage PodDisruptionBudgets for OSD, Mon, RGW, and MDS daemons. OSD PDBs are managed dynamically

cluster/examples/kubernetes/ceph/crds.yaml

@@ -2132,6 +2132,8 @@ spec:
                         type: object
                       nullable: true
                       type: array
+                    onlyApplyOSDPlacement:
+                      type: boolean
                     storageClassDeviceSets:
                       items:
                         description: StorageClassDeviceSet is a storage class device set

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

@@ -1895,6 +1895,8 @@ type StorageScopeSpec struct {
 	Nodes []Node `json:"nodes,omitempty"`
 	// +optional
 	UseAllNodes bool `json:"useAllNodes,omitempty"`
+	// +optional
+	OnlyApplyOSDPlacement bool `json:"onlyApplyOSDPlacement,omitempty"`
 	// +kubebuilder:pruning:PreserveUnknownFields
 	// +nullable
 	// +optional

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

@@ -493,7 +493,7 @@ func TestGetOSDInfo(t *testing.T) {
 	})
 }
 
-func TestOSDPlacement(t *testing.T) {
+func TestGetPreparePlacement(t *testing.T) {
 	// no placement
 	prop := osdProperties{}
 	result := prop.getPreparePlacement()

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

@@ -157,17 +157,13 @@ func (c *Cluster) provisionPodTemplateSpec(osdProps osdProperties, restart v1.Re
 		podSpec.DNSPolicy = v1.DNSClusterFirstWithHostNet
 	}
 	if osdProps.onPVC() {
-		// The "all" placement is applied separately so it will have lower priority.
-		// We want placement from the storageClassDeviceSet to be applied and override
-		// the "all" placement if there are any overlapping placement settings.
-		c.spec.Placement.All().ApplyToPodSpec(&podSpec)
-		// Apply storageClassDeviceSet PreparePlacement
-		// If nodeAffinity is specified both in the device set and "all" placement,
-		// they will be merged.
+		c.applyAllPlacementIfNeeded(&podSpec)
+		// apply storageClassDeviceSets.preparePlacement
 		osdProps.getPreparePlacement().ApplyToPodSpec(&podSpec)
 	} else {
-		p := cephv1.GetOSDPlacement(c.spec.Placement)
-		p.ApplyToPodSpec(&podSpec)
+		c.applyAllPlacementIfNeeded(&podSpec)
+		// apply spec.placement.prepareosd
+		c.spec.Placement[cephv1.KeyOSDPrepare].ApplyToPodSpec(&podSpec)
 	}
 
 	k8sutil.RemoveDuplicateEnvVars(&podSpec)

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

@@ -715,17 +715,13 @@ func (c *Cluster) makeDeployment(osdProps osdProperties, osd OSDInfo, provisionC
 	}
 
 	if osdProps.onPVC() {
-		// the "all" placement is applied separately so it will have lower priority.
-		// We want placement from the storageClassDeviceSet to be applied and override
-		// the "all" placement if there are any overlapping placement settings.
-		c.spec.Placement.All().ApplyToPodSpec(&deployment.Spec.Template.Spec)
-		// apply storageClassDeviceSet Placement
-		// If nodeAffinity is specified both in the device set and "all" placement,
-		// they will be merged.
+		c.applyAllPlacementIfNeeded(&deployment.Spec.Template.Spec)
+		// apply storageClassDeviceSets.Placement
 		osdProps.placement.ApplyToPodSpec(&deployment.Spec.Template.Spec)
 	} else {
-		p := cephv1.GetOSDPlacement(c.spec.Placement)
-		p.ApplyToPodSpec(&deployment.Spec.Template.Spec)
+		c.applyAllPlacementIfNeeded(&deployment.Spec.Template.Spec)
+		// apply c.spec.Placement.osd
+		c.spec.Placement[cephv1.KeyOSD].ApplyToPodSpec(&deployment.Spec.Template.Spec)
 	}
 
 	// portable OSDs must have affinity to the topology where the osd prepare job was executed
@@ -745,6 +741,22 @@ func (c *Cluster) makeDeployment(osdProps osdProperties, osd OSDInfo, provisionC
 	return deployment, nil
 }
 
+// applyAllPlacementIfNeeded apply spec.placement.all if OnlyApplyOSDPlacement set to false
+func (c *Cluster) applyAllPlacementIfNeeded(d *v1.PodSpec) {
+	// The placement for OSDs is computed from several different places:
+	// - 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,
+	// in case of other placement rule like PodAffinity, PodAntiAffinity... it will override last placement with the current placement applied,
+	// See ApplyToPodSpec().
+
+	// apply spec.placement.all when spec.Storage.OnlyApplyOSDPlacement is false
+	if !c.spec.Storage.OnlyApplyOSDPlacement {
+		c.spec.Placement.All().ApplyToPodSpec(d)
+	}
+}
+
 func applyTopologyAffinity(spec *v1.PodSpec, osd OSDInfo) error {
 	if osd.TopologyAffinity == "" {
 		logger.Debugf("no topology affinity to set for osd %d", osd.ID)

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

@@ -32,6 +32,7 @@ import (
 	exectest "github.com/rook/rook/pkg/util/exec/test"
 	"github.com/stretchr/testify/assert"
 	appsv1 "k8s.io/api/apps/v1"
+	corev1 "k8s.io/api/core/v1"
 	v1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/resource"
 	"k8s.io/client-go/kubernetes/fake"
@@ -707,3 +708,164 @@ func getDummyDeploymentOnNode(clientset *fake.Clientset, c *Cluster, nodeName st
 	}
 	return d
 }
+
+func TestOSDPlacement(t *testing.T) {
+	clientset := fake.NewSimpleClientset()
+	clusterInfo := &cephclient.ClusterInfo{
+		Namespace:   "ns",
+		CephVersion: cephver.Nautilus,
+	}
+	clusterInfo.SetName("testing")
+	clusterInfo.OwnerInfo = cephclient.NewMinimumOwnerInfo(t)
+	context := &clusterd.Context{Clientset: clientset, ConfigDir: "/var/lib/rook", Executor: &exectest.MockExecutor{}}
+
+	spec := cephv1.ClusterSpec{
+		Placement: cephv1.PlacementSpec{
+			"all": {
+				NodeAffinity: &v1.NodeAffinity{
+					RequiredDuringSchedulingIgnoredDuringExecution: &v1.NodeSelector{
+						NodeSelectorTerms: []v1.NodeSelectorTerm{
+							{
+								MatchExpressions: []v1.NodeSelectorRequirement{{
+									Key:      "role",
+									Operator: v1.NodeSelectorOpIn,
+									Values:   []string{"storage-node1"},
+								}},
+							},
+						},
+					},
+				},
+			},
+			"osd": {
+				NodeAffinity: &v1.NodeAffinity{
+					RequiredDuringSchedulingIgnoredDuringExecution: &v1.NodeSelector{
+						NodeSelectorTerms: []v1.NodeSelectorTerm{
+							{
+								MatchExpressions: []v1.NodeSelectorRequirement{{
+									Key:      "role",
+									Operator: v1.NodeSelectorOpIn,
+									Values:   []string{"storage-node1"},
+								}},
+							},
+						},
+					},
+				},
+			},
+			"prepareosd": {
+				NodeAffinity: &v1.NodeAffinity{
+					RequiredDuringSchedulingIgnoredDuringExecution: &v1.NodeSelector{
+						NodeSelectorTerms: []v1.NodeSelectorTerm{
+							{
+								MatchExpressions: []v1.NodeSelectorRequirement{{
+									Key:      "role",
+									Operator: v1.NodeSelectorOpIn,
+									Values:   []string{"storage-node1"},
+								}},
+							},
+						},
+					},
+				},
+			},
+		},
+		Storage: cephv1.StorageScopeSpec{
+			OnlyApplyOSDPlacement: false,
+		},
+	}
+
+	osdProps := osdProperties{
+		pvc: v1.PersistentVolumeClaimVolumeSource{
+			ClaimName: "pvc1",
+		},
+	}
+	osdProps.placement = cephv1.Placement{NodeAffinity: &corev1.NodeAffinity{
+		RequiredDuringSchedulingIgnoredDuringExecution: &corev1.NodeSelector{
+			NodeSelectorTerms: []corev1.NodeSelectorTerm{
+				{
+					MatchExpressions: []corev1.NodeSelectorRequirement{
+						{
+							Key:      "role",
+							Operator: v1.NodeSelectorOpIn,
+							Values:   []string{"storage-node3"},
+						},
+					},
+				},
+			},
+		},
+	},
+	}
+
+	osdProps.preparePlacement = &cephv1.Placement{NodeAffinity: &corev1.NodeAffinity{
+		RequiredDuringSchedulingIgnoredDuringExecution: &corev1.NodeSelector{
+			NodeSelectorTerms: []corev1.NodeSelectorTerm{
+				{
+					MatchExpressions: []corev1.NodeSelectorRequirement{
+						{
+							Key:      "role",
+							Operator: v1.NodeSelectorOpIn,
+							Values:   []string{"storage-node3"},
+						},
+					},
+				},
+			},
+		},
+	},
+	}
+
+	c := New(context, clusterInfo, spec, "rook/rook:myversion")
+	osd := OSDInfo{
+		ID:     0,
+		CVMode: "raw",
+	}
+
+	dataPathMap := &provisionConfig{
+		DataPathMap: opconfig.NewDatalessDaemonDataPathMap(c.clusterInfo.Namespace, "/var/lib/rook"),
+	}
+
+	// For OSD daemon
+	// When OnlyApplyOSDPlacement false, in case of PVC
+	r, err := c.makeDeployment(osdProps, osd, dataPathMap)
+	assert.NoError(t, err)
+	assert.Equal(t, 2, len(r.Spec.Template.Spec.Affinity.NodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution.NodeSelectorTerms[0].MatchExpressions))
+
+	// For OSD-prepare job
+	job, err := c.makeJob(osdProps, dataPathMap)
+	assert.NoError(t, err)
+	assert.Equal(t, 2, len(job.Spec.Template.Spec.Affinity.NodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution.NodeSelectorTerms[0].MatchExpressions))
+
+	// When OnlyApplyOSDPlacement true, in case of PVC
+	spec.Storage.OnlyApplyOSDPlacement = true
+	c = New(context, clusterInfo, spec, "rook/rook:myversion")
+	r, err = c.makeDeployment(osdProps, osd, dataPathMap)
+	assert.NoError(t, err)
+	assert.Equal(t, 1, len(r.Spec.Template.Spec.Affinity.NodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution.NodeSelectorTerms[0].MatchExpressions))
+
+	// For OSD-prepare job
+	job, err = c.makeJob(osdProps, dataPathMap)
+	assert.NoError(t, err)
+	assert.Equal(t, 1, len(job.Spec.Template.Spec.Affinity.NodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution.NodeSelectorTerms[0].MatchExpressions))
+
+	// When OnlyApplyOSDPlacement false, in case of non-PVC
+	spec.Storage.OnlyApplyOSDPlacement = false
+	osdProps = osdProperties{}
+	c = New(context, clusterInfo, spec, "rook/rook:myversion")
+	r, err = c.makeDeployment(osdProps, osd, dataPathMap)
+	assert.NoError(t, err)
+	assert.Equal(t, 2, len(r.Spec.Template.Spec.Affinity.NodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution.NodeSelectorTerms[0].MatchExpressions))
+
+	// For OSD-prepare job
+	job, err = c.makeJob(osdProps, dataPathMap)
+	assert.NoError(t, err)
+	assert.Equal(t, 2, len(job.Spec.Template.Spec.Affinity.NodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution.NodeSelectorTerms[0].MatchExpressions))
+
+	// When OnlyApplyOSDPlacement true, in case of non-PVC
+	spec.Storage.OnlyApplyOSDPlacement = true
+	c = New(context, clusterInfo, spec, "rook/rook:myversion")
+	r, err = c.makeDeployment(osdProps, osd, dataPathMap)
+	assert.NoError(t, err)
+	assert.Equal(t, 1, len(r.Spec.Template.Spec.Affinity.NodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution.NodeSelectorTerms[0].MatchExpressions))
+
+	// For OSD-prepare job
+	job, err = c.makeJob(osdProps, dataPathMap)
+	assert.NoError(t, err)
+	assert.Equal(t, 1, len(job.Spec.Template.Spec.Affinity.NodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution.NodeSelectorTerms[0].MatchExpressions))
+}