ceph: add support for disk selection by full path

Rook-Ceph now supports specify device by their fullpath instead of links
created by the kernel like /dev/sdb.

Closes: rook#1228
Co-authored-by: Sean Micklethwaite <sean@wayve.ai>
Signed-off-by: Sébastien Han <seb@redhat.com>

Documentation/ceph-cluster-crd.md

@@ -238,7 +238,7 @@ Below are the settings available, both at the cluster and individual node level,
   * `^/dev/sd.`: Selects all devices starting with `sd`
   * `^/dev/disk/by-path/pci-.*`: Selects all devices which are connected to PCI bus
 * `devices`: A list of individual device names belonging to this node to include in the storage cluster.
-  * `name`: The name of the device (e.g., `sda`)
+  * `name`: The name of the device (e.g., `sda`), or full udev path (e.g. `/dev/disk/by-id/ata-ST4000DM004-XXXX` - this will not change after reboots).
   * `config`: Device-specific config settings. See the [config settings](#osd-configuration-settings) below
 * `directories`:  A list of directory paths that will be included in the storage cluster. Note that using two directories on the same physical device can cause a negative performance impact. Following paths and any of their subpaths **must not be used**: `/etc/ceph`, `/rook` or `/var/log/ceph`.
   * `path`: The path on disk of the directory (e.g., `/rook/storage-dir`)
@@ -435,7 +435,7 @@ spec:
     - name: "172.17.4.201"
       devices:             # specific devices to use for storage can be specified for each node
       - name: "sdb"
-      - name: "sdc"
+      - name: "/dev/disk/by-id/ata-ST4000DM004-XXXX" # both device name and explicit udev links are supported
       config:         # configuration can be specified at the node level which overrides the cluster level config
         storeType: bluestore
     - name: "172.17.4.301"

PendingReleaseNotes.md

@@ -30,6 +30,7 @@
   - A new environment variable `DISCOVER_DAEMON_UDEV_BLACKLIST` is added through which the user can blacklist the devices
   - If no device is specified, the default values will be used to blacklist the devices
 - Ceph Object Gateway are automatically configured to not run on the same host if hostNetwork is activated
+- Specific devices for OSDs can now be specified using the full udev path (e.g. /dev/disk/by-id/ata-ST4000DM004-XXXX) instead of the device name.
 
 ### EdgeFS
 

cluster/examples/kubernetes/ceph/cluster.yaml

@@ -160,6 +160,7 @@ spec:
 #      - name: "nvme01" # multiple osds can be created on high performance devices
 #        config:
 #          osdsPerDevice: "5"
+#      - name: "/dev/disk/by-id/ata-ST4000DM004-XXXX" # devices can be specified using full udev paths
 #      config: # configuration can be specified at the node level which overrides the cluster level config
 #        storeType: filestore
 #    - name: "172.17.4.301"

pkg/daemon/ceph/osd/daemon.go

@@ -356,6 +356,15 @@ func getAvailableDevices(context *clusterd.Context, desiredDevices []DesiredDevi
 				} else if device.Name == desiredDevice.Name {
 					logger.Infof("%q found in the desired devices", device.Name)
 					matched = true
+				} else if strings.HasPrefix(desiredDevice.Name, "/dev/") {
+					devLinks := strings.Split(device.DevLinks, " ")
+					for _, link := range devLinks {
+						if link == desiredDevice.Name {
+							logger.Infof("%q found in the desired devices (matched by link: %q)", device.Name, link)
+							matched = true
+							break
+						}
+					}
 				}
 				matchedDevice = desiredDevice
 				if matched {

pkg/daemon/ceph/osd/daemon_test.go

@@ -211,6 +211,7 @@ NAME="sdb1" SIZE="30" TYPE="part" PKNAME="sdb"`, nil
 		{Name: "sdb", DevLinks: "/dev/disk/by-id/scsi-4567 /dev/disk/by-path/pci-4:5:6:7-scsi-1"},
 		{Name: "sdc", DevLinks: "/dev/disk/by-id/scsi-89ab /dev/disk/by-path/pci-8:9:a:b-scsi-1"},
 		{Name: "sdd", DevLinks: "/dev/disk/by-id/scsi-cdef /dev/disk/by-path/pci-c:d:e:f-scsi-1"},
+		{Name: "sde", DevLinks: "/dev/disk/by-id/sde-0x0000 /dev/disk/by-path/pci-0000:00:18.0-ata-1"},
 		{Name: "nvme01", DevLinks: "/dev/disk/by-id/nvme-0246 /dev/disk/by-path/pci-0:2:4:6-nvme-1"},
 		{Name: "rda"},
 		{Name: "rdb"},
@@ -220,7 +221,7 @@ NAME="sdb1" SIZE="30" TYPE="part" PKNAME="sdb"`, nil
 	pvcBackedOSD := false
 	mapping, err := getAvailableDevices(context, []DesiredDevice{{Name: "all"}}, "nvme01", pvcBackedOSD)
 	assert.Nil(t, err)
-	assert.Equal(t, 5, len(mapping.Entries))
+	assert.Equal(t, 6, len(mapping.Entries))
 	assert.Equal(t, -1, mapping.Entries["sda"].Data)
 	assert.Equal(t, -1, mapping.Entries["sdd"].Data)
 	assert.Equal(t, -1, mapping.Entries["rda"].Data)
@@ -241,7 +242,7 @@ NAME="sdb1" SIZE="30" TYPE="part" PKNAME="sdb"`, nil
 	// select the sd* devices
 	mapping, err = getAvailableDevices(context, []DesiredDevice{{Name: "^sd.$", IsFilter: true}}, "", pvcBackedOSD)
 	assert.Nil(t, err)
-	assert.Equal(t, 2, len(mapping.Entries))
+	assert.Equal(t, 3, len(mapping.Entries))
 	assert.Equal(t, -1, mapping.Entries["sda"].Data)
 	assert.Equal(t, -1, mapping.Entries["sdd"].Data)
 
@@ -262,7 +263,7 @@ NAME="sdb1" SIZE="30" TYPE="part" PKNAME="sdb"`, nil
 	// select the sd* devices by path names
 	mapping, err = getAvailableDevices(context, []DesiredDevice{{Name: "^/dev/sd.$", IsDevicePathFilter: true}}, "", pvcBackedOSD)
 	assert.Nil(t, err)
-	assert.Equal(t, 2, len(mapping.Entries))
+	assert.Equal(t, 3, len(mapping.Entries))
 	assert.Equal(t, -1, mapping.Entries["sda"].Data)
 	assert.Equal(t, -1, mapping.Entries["sdd"].Data)
 
@@ -271,7 +272,12 @@ NAME="sdb1" SIZE="30" TYPE="part" PKNAME="sdb"`, nil
 	assert.Nil(t, err)
 	assert.Equal(t, 2, len(mapping.Entries))
 	assert.Equal(t, -1, mapping.Entries["sda"].Data)
-	assert.Equal(t, -1, mapping.Entries["sdd"].Data)
+
+	// select a device by explicit link
+	mapping, err = getAvailableDevices(context, []DesiredDevice{{Name: "/dev/disk/by-id/sde-0x0000"}}, "", pvcBackedOSD)
+	assert.Nil(t, err)
+	assert.Equal(t, 1, len(mapping.Entries))
+	assert.Equal(t, -1, mapping.Entries["sde"].Data)
 }
 
 func TestGetRemovedDevices(t *testing.T) {

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

@@ -773,7 +773,11 @@ func (c *Cluster) provisionOSDContainer(osdProps osdProperties, copyBinariesMoun
 			} else {
 				devSuffix += ":"
 			}
-			deviceNames[i] = device.Name + devSuffix
+			deviceID := device.Name
+			if device.FullPath != "" {
+				deviceID = device.FullPath
+			}
+			deviceNames[i] = deviceID + devSuffix
 		}
 		envVars = append(envVars, dataDevicesEnvVar(strings.Join(deviceNames, ",")))
 		devMountNeeded = true