diff --git a/Documentation/CRDs/Cluster/ceph-cluster-crd.md b/Documentation/CRDs/Cluster/ceph-cluster-crd.md index d9c7ea56c..5b9f7ade4 100755 --- a/Documentation/CRDs/Cluster/ceph-cluster-crd.md +++ b/Documentation/CRDs/Cluster/ceph-cluster-crd.md @@ -811,6 +811,28 @@ spec: This configuration will split the replication of volumes across unique racks in the data center setup. +## OSD Device Class via Node Label + +The CRUSH device class for all OSDs on a node can be set using the node label `osd.rook.io/device-class`. This label can be applied retroactively on nodes with provisioned OSDs, or on new nodes about to be added to the cluster. + +For example, if a node has `osd.rook.io/device-class: fast`, OSDs on the node will be assigned the `fast` device class. + +This can help group nodes of the same performance tier together in one pool. + +`allowDeviceClassUpdate: true` must be set in the CephCluster storage spec for the device class to be updated on existing OSDs. Without this setting, the node label will only affect newly provisioned OSDs. + +!!! warning + Changing an OSD's device class will cause Ceph to rebalance data for any pools whose CRUSH rules target that device class. Plan device class changes of existing nodes and OSDs carefully to avoid unexpected data movement. + +### Resolving device classes + +The device class for a node is determined as follows: + +* If the per-node `deviceClass` in the CephCluster CR **and** the `osd.rook.io/device-class` node label are both set, the operator will log an error and **skip** that node. The user must remove one to resolve the conflict. +* If only the CR `deviceClass` is set, it is used. +* If only the node label is set, it is used. +* If neither is set, Ceph auto-detects the class (defaults to `hdd`, `ssd`, or `nvme` based on hardware). + ## Deleting a CephCluster During deletion of a CephCluster resource, Rook protects against accidental or premature destruction diff --git a/pkg/operator/ceph/cluster/osd/create.go b/pkg/operator/ceph/cluster/osd/create.go index 80b839305..38d1ce13f 100644 --- a/pkg/operator/ceph/cluster/osd/create.go +++ b/pkg/operator/ceph/cluster/osd/create.go @@ -329,6 +329,12 @@ func (c *Cluster) startProvisioningOverNodes(config *provisionConfig, errs *prov // create the job that prepares osds on the node storeConfig := osdconfig.ToStoreConfig(n.Config) metadataDevice := osdconfig.MetadataDevice(n.Config) + deviceClass, err := c.resolveDeviceClass(storeConfig.DeviceClass, n.Name) + if err != nil { + log.NamespacedError(c.clusterInfo.Namespace, logger, "%v", err) + continue + } + storeConfig.DeviceClass = deviceClass osdProps := osdProperties{ crushHostname: n.Name, devices: n.Devices, diff --git a/pkg/operator/ceph/cluster/osd/create_test.go b/pkg/operator/ceph/cluster/osd/create_test.go index a857b5c1c..76d1673ac 100644 --- a/pkg/operator/ceph/cluster/osd/create_test.go +++ b/pkg/operator/ceph/cluster/osd/create_test.go @@ -639,6 +639,225 @@ func Test_startProvisioningOverNodes(t *testing.T) { }) } +func Test_startProvisioningOverNodes_deviceClassNodeLabel(t *testing.T) { + namespace := "rook-ceph" + dataDirHostPath := "/var/lib/mycluster" + useAllDevices := true + + clusterInfo := &cephclient.ClusterInfo{ + Namespace: namespace, + CephVersion: cephver.Squid, + } + clusterInfo.SetName("mycluster") + clusterInfo.OwnerInfo = cephclient.NewMinimumOwnerInfo(t) + clusterInfo.Context = context.TODO() + + getDeviceClassEnvFromJobs := func(clientset *fake.Clientset) map[string]string { + jobs, err := clientset.BatchV1().Jobs(namespace).List(context.TODO(), metav1.ListOptions{}) + assert.NoError(t, err) + result := map[string]string{} + for _, job := range jobs.Items { + for _, container := range job.Spec.Template.Spec.Containers { + for _, env := range container.Env { + if env.Name == CrushDeviceClassVarName { + nodeName := strings.TrimPrefix(job.Name, "rook-ceph-osd-prepare-") + result[nodeName] = env.Value + } + } + } + } + return result + } + + t.Run("node label sets device class when CR has none", func(t *testing.T) { + clientset := fake.NewClientset() + // Create a node with the deviceclass label + node := &corev1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node0", + Labels: map[string]string{ + corev1.LabelHostname: "node0", + NodeDeviceClassLabelKey: "ssd", + }, + }, + Status: corev1.NodeStatus{ + Conditions: []corev1.NodeCondition{ + {Type: corev1.NodeReady, Status: corev1.ConditionTrue}, + }, + }, + } + _, err := clientset.CoreV1().Nodes().Create(context.TODO(), node, metav1.CreateOptions{}) + assert.NoError(t, err) + + spec := cephv1.ClusterSpec{ + Storage: cephv1.StorageScopeSpec{ + UseAllNodes: false, + Nodes: []cephv1.Node{{Name: "node0"}}, + Selection: cephv1.Selection{UseAllDevices: &useAllDevices}, + }, + DataDirHostPath: dataDirHostPath, + } + ctx := &clusterd.Context{Clientset: clientset} + c := New(ctx, clusterInfo, spec, "rook/rook:master") + config := c.newProvisionConfig() + errs := newProvisionErrors() + + prepareJobsRun, err := c.startProvisioningOverNodes(config, errs) + assert.NoError(t, err) + assert.Zero(t, errs.len()) + assert.Equal(t, 1, prepareJobsRun.Len()) + + deviceClasses := getDeviceClassEnvFromJobs(clientset) + assert.Equal(t, "ssd", deviceClasses["node0"]) + }) + + t.Run("conflict between CR and node label skips the node", func(t *testing.T) { + clientset := fake.NewClientset() + node := &corev1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node0", + Labels: map[string]string{ + corev1.LabelHostname: "node0", + NodeDeviceClassLabelKey: "ssd", + }, + }, + Status: corev1.NodeStatus{ + Conditions: []corev1.NodeCondition{ + {Type: corev1.NodeReady, Status: corev1.ConditionTrue}, + }, + }, + } + _, err := clientset.CoreV1().Nodes().Create(context.TODO(), node, metav1.CreateOptions{}) + assert.NoError(t, err) + + spec := cephv1.ClusterSpec{ + Storage: cephv1.StorageScopeSpec{ + UseAllNodes: false, + Nodes: []cephv1.Node{{ + Name: "node0", + Config: map[string]string{"deviceClass": "hdd"}, + }}, + Selection: cephv1.Selection{UseAllDevices: &useAllDevices}, + }, + DataDirHostPath: dataDirHostPath, + } + ctx := &clusterd.Context{Clientset: clientset} + c := New(ctx, clusterInfo, spec, "rook/rook:master") + config := c.newProvisionConfig() + errs := newProvisionErrors() + + prepareJobsRun, err := c.startProvisioningOverNodes(config, errs) + assert.NoError(t, err) + assert.Zero(t, errs.len()) + assert.Zero(t, prepareJobsRun.Len()) + }) + + t.Run("no label and no CR config results in empty device class", func(t *testing.T) { + clientset := fake.NewClientset() + node := &corev1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node0", + Labels: map[string]string{ + corev1.LabelHostname: "node0", + }, + }, + Status: corev1.NodeStatus{ + Conditions: []corev1.NodeCondition{ + {Type: corev1.NodeReady, Status: corev1.ConditionTrue}, + }, + }, + } + _, err := clientset.CoreV1().Nodes().Create(context.TODO(), node, metav1.CreateOptions{}) + assert.NoError(t, err) + + spec := cephv1.ClusterSpec{ + Storage: cephv1.StorageScopeSpec{ + UseAllNodes: false, + Nodes: []cephv1.Node{{Name: "node0"}}, + Selection: cephv1.Selection{UseAllDevices: &useAllDevices}, + }, + DataDirHostPath: dataDirHostPath, + } + ctx := &clusterd.Context{Clientset: clientset} + c := New(ctx, clusterInfo, spec, "rook/rook:master") + config := c.newProvisionConfig() + errs := newProvisionErrors() + + prepareJobsRun, err := c.startProvisioningOverNodes(config, errs) + assert.NoError(t, err) + assert.Zero(t, errs.len()) + assert.Equal(t, 1, prepareJobsRun.Len()) + + deviceClasses := getDeviceClassEnvFromJobs(clientset) + assert.Equal(t, "", deviceClasses["node0"]) + }) + + t.Run("conflict skips one node but other nodes still provision", func(t *testing.T) { + clientset := fake.NewClientset() + // node0 has both CR config and label (conflict) + node0 := &corev1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node0", + Labels: map[string]string{ + corev1.LabelHostname: "node0", + NodeDeviceClassLabelKey: "ssd", + }, + }, + Status: corev1.NodeStatus{ + Conditions: []corev1.NodeCondition{ + {Type: corev1.NodeReady, Status: corev1.ConditionTrue}, + }, + }, + } + // node1 has only label (no conflict) + node1 := &corev1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node1", + Labels: map[string]string{ + corev1.LabelHostname: "node1", + NodeDeviceClassLabelKey: "fast", + }, + }, + Status: corev1.NodeStatus{ + Conditions: []corev1.NodeCondition{ + {Type: corev1.NodeReady, Status: corev1.ConditionTrue}, + }, + }, + } + _, err := clientset.CoreV1().Nodes().Create(context.TODO(), node0, metav1.CreateOptions{}) + assert.NoError(t, err) + _, err = clientset.CoreV1().Nodes().Create(context.TODO(), node1, metav1.CreateOptions{}) + assert.NoError(t, err) + + spec := cephv1.ClusterSpec{ + Storage: cephv1.StorageScopeSpec{ + UseAllNodes: false, + Nodes: []cephv1.Node{ + {Name: "node0", Config: map[string]string{"deviceClass": "hdd"}}, + {Name: "node1"}, + }, + Selection: cephv1.Selection{UseAllDevices: &useAllDevices}, + }, + DataDirHostPath: dataDirHostPath, + } + ctx := &clusterd.Context{Clientset: clientset} + c := New(ctx, clusterInfo, spec, "rook/rook:master") + config := c.newProvisionConfig() + errs := newProvisionErrors() + + prepareJobsRun, err := c.startProvisioningOverNodes(config, errs) + assert.NoError(t, err) + assert.Zero(t, errs.len()) + // Only node1 should have a prepare job (node0 skipped due to conflict) + assert.Equal(t, 1, prepareJobsRun.Len()) + + deviceClasses := getDeviceClassEnvFromJobs(clientset) + assert.Equal(t, "fast", deviceClasses["node1"]) + _, node0HasJob := deviceClasses["node0"] + assert.False(t, node0HasJob) + }) +} + func newDummyPVC(name, namespace string, capacity string, storageClassName string) cephv1.VolumeClaimTemplate { volMode := corev1.PersistentVolumeBlock return cephv1.VolumeClaimTemplate{ diff --git a/pkg/operator/ceph/cluster/osd/osd.go b/pkg/operator/ceph/cluster/osd/osd.go index 8f744026f..1ee263845 100644 --- a/pkg/operator/ceph/cluster/osd/osd.go +++ b/pkg/operator/ceph/cluster/osd/osd.go @@ -80,6 +80,10 @@ const ( deviceType = "device-type" encrypted = "encrypted" + // NodeDeviceClassLabelKey is the node label the operator reads to assign a device class + // to all OSDs provisioned on that node. + NodeDeviceClassLabelKey = "osd.rook.io/device-class" + // CephxStatus is applied to each OSD deployment as value of this annotation key cephxStatusAnnotationKey = "cephx-status" ) @@ -590,6 +594,11 @@ func (c *Cluster) getOSDPropsForNode(nodeName, deviceClass string) (osdPropertie storeConfig := osdconfig.ToStoreConfig(n.Config) metadataDevice := osdconfig.MetadataDevice(n.Config) + deviceClass, err := c.resolveDeviceClass(storeConfig.DeviceClass, n.Name) + if err != nil { + return osdProperties{}, err + } + storeConfig.DeviceClass = deviceClass osdProps := osdProperties{ crushHostname: n.Name, devices: n.Devices, @@ -987,10 +996,13 @@ func getNode(ctx context.Context, clientset kubernetes.Interface, nodeName strin // try to find by the node by matching the provided nodeName node, err = clientset.CoreV1().Nodes().Get(ctx, nodeName, metav1.GetOptions{}) if kerrors.IsNotFound(err) { - listOpts := metav1.ListOptions{LabelSelector: fmt.Sprintf("%q=%q", k8sutil.LabelHostname(), nodeName)} + listOpts := metav1.ListOptions{LabelSelector: fmt.Sprintf("%s=%s", k8sutil.LabelHostname(), nodeName)} nodeList, err := clientset.CoreV1().Nodes().List(ctx, listOpts) - if err != nil || len(nodeList.Items) < 1 { - return nil, errors.Wrapf(err, "could not find node %q hostname label", nodeName) + if err != nil { + return nil, errors.Wrapf(err, "could not find node %q by hostname label", nodeName) + } + if len(nodeList.Items) != 1 { + return nil, errors.Errorf("could not find node %q by hostname label", nodeName) } return &nodeList.Items[0], nil } else if err != nil { @@ -1000,6 +1012,29 @@ func getNode(ctx context.Context, clientset kubernetes.Interface, nodeName strin return node, nil } +// resolveDeviceClass determines the device class for a node by checking the CR config +// and the osd.rook.io/device-class node label. If both are set, it returns an error +// to avoid ambiguity — the user must choose one or the other. +func (c *Cluster) resolveDeviceClass(crDeviceClass, nodeName string) (string, error) { + node, err := getNode(c.clusterInfo.Context, c.context.Clientset, nodeName) + if err != nil { + return "", errors.Wrapf(err, "failed to get node %q to read device class label", nodeName) + } + labelClass := node.Labels[NodeDeviceClassLabelKey] + if crDeviceClass != "" && labelClass != "" { + return "", errors.Errorf("node %q has both CR device class %q and node label %q set; remove one to resolve the conflict", nodeName, crDeviceClass, labelClass) + } + if crDeviceClass != "" { + log.NamespacedDebug(c.clusterInfo.Namespace, logger, "using device class %q from CR config for node %q", crDeviceClass, nodeName) + return crDeviceClass, nil + } + if labelClass != "" { + log.NamespacedDebug(c.clusterInfo.Namespace, logger, "using device class %q from node label for node %q", labelClass, nodeName) + return labelClass, nil + } + return "", nil +} + func updateLocationWithNodeLabels(location *[]string, nodeLabels map[string]string) string { topology, topologyAffinity := topology.ExtractOSDTopologyFromLabels(nodeLabels) diff --git a/pkg/operator/ceph/cluster/osd/osd_test.go b/pkg/operator/ceph/cluster/osd/osd_test.go index e54113294..90fd1ebdb 100644 --- a/pkg/operator/ceph/cluster/osd/osd_test.go +++ b/pkg/operator/ceph/cluster/osd/osd_test.go @@ -1495,3 +1495,90 @@ func TestPerDeviceClassForOSD(t *testing.T) { }) } } + +func TestResolveDeviceClass(t *testing.T) { + namespace := "rook-ceph" + clusterInfo := &cephclient.ClusterInfo{ + Namespace: namespace, + CephVersion: cephver.Squid, + Context: context.TODO(), + } + clusterInfo.SetName("mycluster") + clusterInfo.OwnerInfo = cephclient.NewMinimumOwnerInfo(t) + + createNode := func(clientset *fake.Clientset, name string, labels map[string]string) { + node := &corev1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Labels: labels, + }, + } + _, err := clientset.CoreV1().Nodes().Create(context.TODO(), node, metav1.CreateOptions{}) + assert.NoError(t, err) + } + + t.Run("only node label set", func(t *testing.T) { + clientset := fake.NewClientset() + createNode(clientset, "node0", map[string]string{ + corev1.LabelHostname: "node0", + NodeDeviceClassLabelKey: "ssd", + }) + ctx := &clusterd.Context{Clientset: clientset} + c := New(ctx, clusterInfo, cephv1.ClusterSpec{}, "rook/rook:master") + + result, err := c.resolveDeviceClass("", "node0") + assert.NoError(t, err) + assert.Equal(t, "ssd", result) + }) + + t.Run("only CR device class set", func(t *testing.T) { + clientset := fake.NewClientset() + createNode(clientset, "node0", map[string]string{ + corev1.LabelHostname: "node0", + }) + ctx := &clusterd.Context{Clientset: clientset} + c := New(ctx, clusterInfo, cephv1.ClusterSpec{}, "rook/rook:master") + + result, err := c.resolveDeviceClass("hdd", "node0") + assert.NoError(t, err) + assert.Equal(t, "hdd", result) + }) + + t.Run("both set returns conflict error", func(t *testing.T) { + clientset := fake.NewClientset() + createNode(clientset, "node0", map[string]string{ + corev1.LabelHostname: "node0", + NodeDeviceClassLabelKey: "ssd", + }) + ctx := &clusterd.Context{Clientset: clientset} + c := New(ctx, clusterInfo, cephv1.ClusterSpec{}, "rook/rook:master") + + result, err := c.resolveDeviceClass("hdd", "node0") + assert.Error(t, err) + assert.Contains(t, err.Error(), "conflict") + assert.Equal(t, "", result) + }) + + t.Run("neither set returns empty", func(t *testing.T) { + clientset := fake.NewClientset() + createNode(clientset, "node0", map[string]string{ + corev1.LabelHostname: "node0", + }) + ctx := &clusterd.Context{Clientset: clientset} + c := New(ctx, clusterInfo, cephv1.ClusterSpec{}, "rook/rook:master") + + result, err := c.resolveDeviceClass("", "node0") + assert.NoError(t, err) + assert.Equal(t, "", result) + }) + + t.Run("node not found returns error", func(t *testing.T) { + clientset := fake.NewClientset() + ctx := &clusterd.Context{Clientset: clientset} + c := New(ctx, clusterInfo, cephv1.ClusterSpec{}, "rook/rook:master") + + result, err := c.resolveDeviceClass("", "nonexistent") + assert.Error(t, err) + assert.Equal(t, "", result) + }) +} diff --git a/pkg/operator/ceph/cluster/osd/spec_test.go b/pkg/operator/ceph/cluster/osd/spec_test.go index f3acacb18..d4a43bbd8 100644 --- a/pkg/operator/ceph/cluster/osd/spec_test.go +++ b/pkg/operator/ceph/cluster/osd/spec_test.go @@ -18,6 +18,7 @@ limitations under the License. package osd import ( + "context" "testing" cephv1 "github.com/rook/rook/pkg/apis/ceph.rook.io/v1" @@ -32,7 +33,9 @@ import ( "github.com/stretchr/testify/assert" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" + kerrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/resource" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes/fake" ) @@ -796,6 +799,17 @@ func getDummyDeploymentOnPVC(clientset *fake.Clientset, c *Cluster, pvcName stri // WARNING! modifies c.ValidStorage func getDummyDeploymentOnNode(clientset *fake.Clientset, c *Cluster, nodeName string, osdID int) *appsv1.Deployment { + // Ensure the node exists in the fake clientset so resolveDeviceClass can look it up + node := &corev1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: nodeName, + Labels: map[string]string{corev1.LabelHostname: nodeName}, + }, + } + if _, err := clientset.CoreV1().Nodes().Create(context.TODO(), node, metav1.CreateOptions{}); err != nil && !kerrors.IsAlreadyExists(err) { + panic(err) + } + osd := &OSDInfo{ ID: osdID, UUID: "some-uuid", @@ -812,6 +826,96 @@ func getDummyDeploymentOnNode(clientset *fake.Clientset, c *Cluster, nodeName st return d } +func TestDeploymentOnNode_DeviceClassFromLabel(t *testing.T) { + clientset := fake.NewClientset() + node := &corev1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node0", + Labels: map[string]string{corev1.LabelHostname: "node0", NodeDeviceClassLabelKey: "fast"}, + }, + } + _, err := clientset.CoreV1().Nodes().Create(context.TODO(), node, metav1.CreateOptions{}) + assert.NoError(t, err) + + clusterInfo := &cephclient.ClusterInfo{ + Namespace: "ns", + CephVersion: cephver.Squid, + } + clusterInfo.SetName("test") + clusterInfo.OwnerInfo = cephclient.NewMinimumOwnerInfo(t) + clusterInfo.Context = context.TODO() + + spec := cephv1.ClusterSpec{ + Storage: cephv1.StorageScopeSpec{ + Nodes: []cephv1.Node{{Name: "node0"}}, + AllowDeviceClassUpdate: true, + }, + DataDirHostPath: "/var/lib/rook", + } + ctx := &clusterd.Context{Clientset: clientset} + c := New(ctx, clusterInfo, spec, "rook/rook:master") + c.ValidStorage.Nodes = []cephv1.Node{{Name: "node0"}} + + osd := &OSDInfo{ID: 0, UUID: "u", BlockPath: "/dev/vda", CVMode: "raw", Store: "bluestore"} + config := c.newProvisionConfig() + + t.Run("node label device class flows into deployment", func(t *testing.T) { + d, err := deploymentOnNode(c, osd, "node0", config) + assert.NoError(t, err) + // Check ROOK_OSD_DEVICE_CLASS env var + for _, container := range d.Spec.Template.Spec.Containers { + for _, env := range container.Env { + if env.Name == "ROOK_OSD_DEVICE_CLASS" { + assert.Equal(t, "fast", env.Value) + return + } + } + } + t.Fatal("ROOK_OSD_DEVICE_CLASS env var not found") + }) + + t.Run("conflict between CR and label returns error", func(t *testing.T) { + specWithCR := cephv1.ClusterSpec{ + Storage: cephv1.StorageScopeSpec{ + Nodes: []cephv1.Node{{Name: "node0", Config: map[string]string{"deviceClass": "hdd"}}}, + }, + DataDirHostPath: "/var/lib/rook", + } + c2 := New(ctx, clusterInfo, specWithCR, "rook/rook:master") + c2.ValidStorage.Nodes = []cephv1.Node{{Name: "node0", Config: map[string]string{"deviceClass": "hdd"}}} + + _, err := deploymentOnNode(c2, osd, "node0", c2.newProvisionConfig()) + assert.Error(t, err) + assert.Contains(t, err.Error(), "conflict") + }) + + t.Run("AllowDeviceClassUpdate false does not change existing OSD class", func(t *testing.T) { + specNoUpdate := cephv1.ClusterSpec{ + Storage: cephv1.StorageScopeSpec{ + Nodes: []cephv1.Node{{Name: "node0"}}, + AllowDeviceClassUpdate: false, + }, + DataDirHostPath: "/var/lib/rook", + } + c3 := New(ctx, clusterInfo, specNoUpdate, "rook/rook:master") + c3.ValidStorage.Nodes = []cephv1.Node{{Name: "node0"}} + + osdWithClass := &OSDInfo{ID: 0, UUID: "u", BlockPath: "/dev/vda", CVMode: "raw", Store: "bluestore", DeviceClass: "hdd"} + d, err := deploymentOnNode(c3, osdWithClass, "node0", c3.newProvisionConfig()) + assert.NoError(t, err) + // OSD should keep "hdd" even though node label says "fast" + for _, container := range d.Spec.Template.Spec.Containers { + for _, env := range container.Env { + if env.Name == "ROOK_OSD_DEVICE_CLASS" { + assert.Equal(t, "hdd", env.Value) + return + } + } + } + t.Fatal("ROOK_OSD_DEVICE_CLASS env var not found") + }) +} + func TestOSDPlacement(t *testing.T) { clientset := fake.NewClientset() clusterInfo := &cephclient.ClusterInfo{ diff --git a/pkg/operator/ceph/cluster/osd/update_test.go b/pkg/operator/ceph/cluster/osd/update_test.go index 8288555af..530680961 100644 --- a/pkg/operator/ceph/cluster/osd/update_test.go +++ b/pkg/operator/ceph/cluster/osd/update_test.go @@ -546,6 +546,79 @@ func Test_updateExistingOSDs(t *testing.T) { assert.Equal(t, 1, osdIDUpdated) updateConfig.osdsToSkipReconcile.Delete("0") }) + + t.Run("conflict between CR and node label skips conflicting OSD but updates others", func(t *testing.T) { + clientset = fake.NewClientset() + + // node0 has no label (no conflict) + node0 := &corev1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node0", + Labels: map[string]string{corev1.LabelHostname: "node0"}, + }, + } + _, err := clientset.CoreV1().Nodes().Create(context.TODO(), node0, metav1.CreateOptions{}) + assert.NoError(t, err) + + // node1 has BOTH label and CR config (conflict) + node1 := &corev1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node1", + Labels: map[string]string{corev1.LabelHostname: "node1", NodeDeviceClassLabelKey: "ssd"}, + }, + } + _, err = clientset.CoreV1().Nodes().Create(context.TODO(), node1, metav1.CreateOptions{}) + assert.NoError(t, err) + + updateQueue = newUpdateQueueWithIDs(0, 1) + existingDeployments = newExistenceListWithIDs(0, 1) + + ctx = &clusterd.Context{ + Clientset: clientset, + Executor: executor, + } + clusterInfo := &cephclient.ClusterInfo{ + Namespace: namespace, + CephVersion: cephver.Squid, + Context: context.TODO(), + } + clusterInfo.SetName("mycluster") + clusterInfo.OwnerInfo = cephclient.NewMinimumOwnerInfo(t) + spec := cephv1.ClusterSpec{ + ContinueUpgradeAfterChecksEvenIfNotHealthy: forceUpgradeIfUnhealthy, + UpgradeOSDRequiresHealthyPGs: requiresHealthyPGs, + Storage: cephv1.StorageScopeSpec{ + Nodes: []cephv1.Node{ + {Name: "node0"}, + {Name: "node1", Config: map[string]string{"deviceClass": "hdd"}}, + }, + }, + } + c = New(ctx, clusterInfo, spec, "rook/rook:master") + c.ValidStorage.Nodes = spec.Storage.Nodes + config := c.newProvisionConfig() + updateConfig = c.newUpdateConfig(config, updateQueue, existingDeployments, sets.New[string]()) + + addDeploymentOnNode("node0", 0) + // Create node1's deployment manually since getDummyDeploymentOnNode would panic on conflict. + // In production, this deployment would have been created before the label was added. + d1 := getDummyDeploymentOnNode(clientset, c, "node0", 1) // use node0 to avoid panic + d1.Name = "rook-ceph-osd-1" + d1.Spec.Template.Spec.NodeSelector = map[string]string{corev1.LabelHostname: "node1"} + _, err = clientset.AppsV1().Deployments(namespace).Create(context.TODO(), d1, metav1.CreateOptions{}) + assert.NoError(t, err) + + osdToBeQueried = 0 + returnOkToStopIDs = []int{0, 1} + deploymentsUpdated = []string{} + errs = newProvisionErrors() + + updateConfig.updateExistingOSDs(errs) + // OSD 1 on node1 should fail due to conflict + assert.Equal(t, 1, errs.len()) + // OSD 0 on node0 should have been updated successfully + assert.Contains(t, deploymentsUpdated, "rook-ceph-osd-0") + }) } func Test_getOSDUpdateInfo(t *testing.T) {