From 98383133989b41dfd4b998dfaa5ce64fcde81b8c Mon Sep 17 00:00:00 2001 From: subhamkrai Date: Mon, 26 Jul 2021 14:58:23 +0530 Subject: [PATCH] ceph: add an option to enable/disable merge all placement earlier, we had the issue for osd placement regarding whether we should merge the placements or not. Now, we have option `skipApplyAllPlacement` to enable/disable osd placement. By default it is false which means it will merge both placement.All() and storageClassDeviceSets.Placement, when true it will only apply storageClassDeviceSets.Placement. Also, adding unit test. Closes: https://github.com/rook/rook/issues/8135 Signed-off-by: subhamkrai --- Documentation/ceph-cluster-crd.md | 6 +- .../charts/rook-ceph/templates/resources.yaml | 2 + .../kubernetes/ceph/cluster-on-local-pvc.yaml | 2 + .../kubernetes/ceph/cluster-on-pvc.yaml | 2 + cluster/examples/kubernetes/ceph/cluster.yaml | 2 + cluster/examples/kubernetes/ceph/crds.yaml | 2 + pkg/apis/ceph.rook.io/v1/types.go | 2 + pkg/operator/ceph/cluster/osd/osd_test.go | 2 +- .../ceph/cluster/osd/provision_spec.go | 14 +- pkg/operator/ceph/cluster/osd/spec.go | 30 +++- pkg/operator/ceph/cluster/osd/spec_test.go | 162 ++++++++++++++++++ 11 files changed, 206 insertions(+), 20 deletions(-) diff --git a/Documentation/ceph-cluster-crd.md b/Documentation/ceph-cluster-crd.md index e8fedd384..6b69c9638 100755 --- a/Documentation/ceph-cluster-crd.md +++ b/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. diff --git a/cluster/charts/rook-ceph/templates/resources.yaml b/cluster/charts/rook-ceph/templates/resources.yaml index bed22ef0a..bb478ba73 100644 --- a/cluster/charts/rook-ceph/templates/resources.yaml +++ b/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 diff --git a/cluster/examples/kubernetes/ceph/cluster-on-local-pvc.yaml b/cluster/examples/kubernetes/ceph/cluster-on-local-pvc.yaml index 97bc0319a..59af9a5c8 100644 --- a/cluster/examples/kubernetes/ceph/cluster-on-local-pvc.yaml +++ b/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: diff --git a/cluster/examples/kubernetes/ceph/cluster-on-pvc.yaml b/cluster/examples/kubernetes/ceph/cluster-on-pvc.yaml index 677ed31e5..eb796fc74 100644 --- a/cluster/examples/kubernetes/ceph/cluster-on-pvc.yaml +++ b/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: diff --git a/cluster/examples/kubernetes/ceph/cluster.yaml b/cluster/examples/kubernetes/ceph/cluster.yaml index a503f1dd1..cb2ac1ea7 100644 --- a/cluster/examples/kubernetes/ceph/cluster.yaml +++ b/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 diff --git a/cluster/examples/kubernetes/ceph/crds.yaml b/cluster/examples/kubernetes/ceph/crds.yaml index 9f313903f..00065717b 100644 --- a/cluster/examples/kubernetes/ceph/crds.yaml +++ b/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 diff --git a/pkg/apis/ceph.rook.io/v1/types.go b/pkg/apis/ceph.rook.io/v1/types.go index 011d83b00..29141e436 100755 --- a/pkg/apis/ceph.rook.io/v1/types.go +++ b/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 diff --git a/pkg/operator/ceph/cluster/osd/osd_test.go b/pkg/operator/ceph/cluster/osd/osd_test.go index 68e42e604..9967ae4fd 100644 --- a/pkg/operator/ceph/cluster/osd/osd_test.go +++ b/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() diff --git a/pkg/operator/ceph/cluster/osd/provision_spec.go b/pkg/operator/ceph/cluster/osd/provision_spec.go index e1f0f50ae..396c1adfb 100644 --- a/pkg/operator/ceph/cluster/osd/provision_spec.go +++ b/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) diff --git a/pkg/operator/ceph/cluster/osd/spec.go b/pkg/operator/ceph/cluster/osd/spec.go index 594626561..85e322da7 100644 --- a/pkg/operator/ceph/cluster/osd/spec.go +++ b/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) diff --git a/pkg/operator/ceph/cluster/osd/spec_test.go b/pkg/operator/ceph/cluster/osd/spec_test.go index e24085e57..030668514 100644 --- a/pkg/operator/ceph/cluster/osd/spec_test.go +++ b/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)) +}