Repository: kubernetes/kubernetes (language: go)

Issue:
iscsi/fc volume with multipath can be incorrectly resolved to partition
### What happened?

If worker nodes are configured so that multipath symlink under `/dev/disk/by-id` points directly to device mapper device (dm-X) the FC/iSCSI volume plugin code in Kubernetes can not handle this well and finds incorrect device.

This is because `FindMultipathDeviceForDevice` function can be called with already resolved symlink and still tries to resolve it further to find its parent - this can resolve in matching a partition of the device, see details below.


If we have a multipath device `scsi-36001405f3392bf76369422cbfd8acd80` pointing to `dm-2`:

```
ls -l /dev/disk/by-id/scsi-36001405f3392bf76369422cbfd8acd80
lrwxrwxrwx. 1 root root 10 Oct 14 14:43 /dev/disk/by-id/scsi-36001405f3392bf76369422cbfd8acd80 -> ../../dm-2
```

This function will try to lookup it's parent, finding `dm-3`:
```
# ls -la /sys/block/dm-3/slaves
total 0
drwxr-xr-x. 2 root root 0 Oct 14 16:29 .
drwxr-xr-x. 9 root root 0 Oct 14 14:43 ..
lrwxrwxrwx. 1 root root 0 Oct 14 14:44 dm-2 -> ../../dm-2
```

Which is actually a partition of the device:
```
# ls -l /dev/disk/by-id/dm-uuid-part1-mpath-36001405f3392bf76369422cbfd8acd80
lrwxrwxrwx. 1 root root 10 Oct 14 14:43 /dev/disk/by-id/dm-uuid-part1-mpath-36001405f3392bf76369422cbfd8acd80 -> ../../dm-3
```

Multipath and partitions configured:
```
# lsblk
NAME              MAJ:MIN RM   SIZE RO TYPE  MOUNTPOINTS
sda                 8:0    0    50M  0 disk
└─mpathh          253:2    0    50M  0 mpath
  └─mpathh1       253:3    0    10M  0 part
sdb                 8:16   0    50M  0 disk
└─mpathh          253:2    0    50M  0 mpath
  └─mpathh1       253:3    0    10M  0 part
  ...
```

Pod has mounted/attached `253:3` device which is the partition:
```
# crictl inspect 7d8b037b74949 | grep -A3 /dev/data
            "path": "/dev/data",
            "type": "b",
            "major": 253,
            "minor": 3,
```

### What did you expect to happen?

Should match multipath device instead:

```
# crictl inspect e003f05e7f406 | grep -A 3 /dev/data
            "path": "/dev/data",
            "type": "b",
            "major": 253,
            "minor": 2,
# lsblk| grep "253:2"
└─mpathh          253:2    0    50M  0 mpath
└─mpathh          253:2    0    50M  0 mpath
```


### How can we reproduce it (as minimally and precisely as possible)?

Locally with `local-up-cluster.sh`, attaching "fake" loopback device as iSCSI:

- create a loopback device -> `dd if=/dev/zero of=/srv/iscsi/file.img count=102400 && losetup /dev/loop1 /srv/iscsi/file.img`
- partition the loopback device -> `fdisk /dev/loop1`
- configure iSCSI initiator using `targetcli` with  `/dev/loop1` as backing block device
- discover and attach iSCSI disk to node using both local and loopback IPs to simulate multipath
- enable multipath with `mpathconf --enable --with_multipathd y`
- start local Kubernetes with `local-up-cluster.sh`
- create PV, PVC and Pod:

```yaml
apiVersion: v1
kind: PersistentVolume
metadata:
  name: fc-pv
  labels:
    volumeName: fc-pv-01
spec:
  volumeMode: Block
  accessModes:
    - ReadWriteOnce
  capacity:
    storage: 50M
  fc:
    wwids: ['36001405f3392bf76369422cbfd8acd80']
```

```yaml
apiVersion: v1
kind: PersistentVolumeClaim
metadata:
  name: fc-pvc
spec:
  volumeMode: Block
  accessModes:
    - ReadWriteOnce
  resources:
    requests:
      storage: 50M
  selector:
    matchLabels:
      volumeName: fc-pv-01
```

```yaml
apiVersion: v1
kind: Pod
metadata:
  name: fc-pod
spec:
  containers:
  - name: busybox
    image: busybox
    command: [ "sh", "-c", "sleep infinity" ]
    volumeDevices:
      - name: fc-storage
        devicePath: /dev/data
  volumes:
  - name: fc-storage
    persistentVolumeClaim:
      claimName: fc-pvc
```

- inspect running pod for attached devices with chosen container runtime -> `crictl inspect <pod_id> | grep -A3 /dev/data`

### Anything else we need to know?

_No response_

### Kubernetes version

<details>

```console
$ kubectl version
Client Version: v1.31.1
Kustomize Version: v5.4.2
Server Version: v1.32.0-alpha.0.1350+fc93fcdcf2dbcd
```

</details>


### Cloud provider

<details>
N/A
</details>


### OS version

<details>

```console
# On Linux:
$ cat /etc/os-release
NAME="Fedora Linux"
VERSION="39 (Workstation Edition)"
ID=fedora
VERSION_ID=39
VERSION_CODENAME=""
PLATFORM_ID="platform:f39"
PRETTY_NAME="Fedora Linux 39 (Workstation Edition)"
ANSI_COLOR="0;38;2;60;110;180"
LOGO=fedora-logo-icon
CPE_NAME="cpe:/o:fedoraproject:fedora:39"
DEFAULT_HOSTNAME="fedora"
HOME_URL="https://fedoraproject.org/"
DOCUMENTATION_URL="https://docs.fedoraproject.org/en-US/fedora/f39/system-administrators-guide/"
SUPPORT_URL="https://ask.fedoraproject.org/"
BUG_REPORT_URL="https://bugzilla.redhat.com/"
REDHAT_BUGZILLA_PRODUCT="Fedora"
REDHAT_BUGZILLA_PRODUCT_VERSION=39
REDHAT_SUPPORT_PRODUCT="Fedora"
REDHAT_SUPPORT_PRODUCT_VERSION=39
SUPPORT_END=2024-11-12
VARIANT="Workstation Edition"
VARIANT_ID=workstation
$ uname -a
Linux fedora 6.9.5-100.fc39.x86_64 #1 SMP PREEMPT_DYNAMIC Sun Jun 16 15:57:19 UTC 2024 x86_64 GNU/Linux

# On Windows:
C:\> wmic os get Caption, Version, BuildNumber, OSArchitecture
# paste output here
```

</details>


### Install tools

<details>
N/A
</details>


### Container runtime (CRI) and version (if applicable)

<details>
$ crictl version
Version:  0.1.0
RuntimeName:  cri-o
RuntimeVersion:  1.27.2
RuntimeApiVersion:  v1
</details>


### Related plugins (CNI, CSI, ...) and versions (if applicable)

<details>
N/A
</details>



Discussion / hints:
This issue is currently awaiting triage.

If a SIG or subproject determines this is a relevant issue, they will accept it by applying the `triage/accepted` label and provide further guidance.

The `triage/accepted` label can be added by org members by writing `/triage accepted` in a comment.


<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository.
</details>
/assign
/sig storage

Fix patch:
diff --git a/pkg/volume/util/device_util_linux.go b/pkg/volume/util/device_util_linux.go
index 66ac77835c3e7..18cbec072f420 100644
--- a/pkg/volume/util/device_util_linux.go
+++ b/pkg/volume/util/device_util_linux.go
@@ -31,8 +31,13 @@ import (
 	"k8s.io/klog/v2"
 )
 
-// FindMultipathDeviceForDevice given a device name like /dev/sdx, find the devicemapper parent
+// FindMultipathDeviceForDevice given a device name like /dev/sdx, find the devicemapper parent. If called with a device
+// already resolved to devicemapper, do nothing.
 func (handler *deviceHandler) FindMultipathDeviceForDevice(device string) string {
+	if strings.HasPrefix(device, "/dev/dm-") {
+		return device
+	}
+
 	io := handler.getIo
 	disk, err := findDeviceForPath(device, io)
 	if err != nil {


Test patch:
diff --git a/pkg/volume/util/device_util_linux_test.go b/pkg/volume/util/device_util_linux_test.go
index ae1ee047e7a91..bb3d113f1a550 100644
--- a/pkg/volume/util/device_util_linux_test.go
+++ b/pkg/volume/util/device_util_linux_test.go
@@ -204,14 +204,42 @@ func (fi *fakeFileInfo) Sys() interface{} {
 }
 
 func TestFindMultipathDeviceForDevice(t *testing.T) {
-	mockDeviceUtil := NewDeviceHandler(&mockOsIOHandler{})
-	dev := mockDeviceUtil.FindMultipathDeviceForDevice("/dev/disk/by-path/127.0.0.1:3260-eui.02004567A425678D-lun-0")
-	if dev != "/dev/dm-1" {
-		t.Fatalf("mpio device not found dm-1 expected got [%s]", dev)
+	tests := []struct {
+		name           string
+		device         string
+		expectedResult string
+	}{
+		{
+			name:           "Device is already a dm device",
+			device:         "/dev/dm-1",
+			expectedResult: "/dev/dm-1",
+		},
+		{
+			name:           "Device has no multipath",
+			device:         "/dev/sdc",
+			expectedResult: "",
+		},
+		{
+			name:           "Device has multipath",
+			device:         "/dev/disk/by-path/127.0.0.1:3260-eui.02004567A425678D-lun-0",
+			expectedResult: "/dev/dm-1",
+		},
+		{
+			name:           "Invalid device path",
+			device:         "/dev/nonexistent",
+			expectedResult: "",
+		},
 	}
-	dev = mockDeviceUtil.FindMultipathDeviceForDevice("/dev/disk/by-path/empty")
-	if dev != "" {
-		t.Fatalf("mpio device not found '' expected got [%s]", dev)
+
+	mockDeviceUtil := NewDeviceHandler(&mockOsIOHandler{})
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			result := mockDeviceUtil.FindMultipathDeviceForDevice(tt.device)
+			if result != tt.expectedResult {
+				t.Errorf("FindMultipathDeviceForDevice(%s) = %s, want %s", tt.device, result, tt.expectedResult)
+			}
+		})
 	}
 }