ceph-volume: do not convert LVs's symlink to real path

This commit:
- Adds a new function `get_lvm_mappers` in `ceph_volume/util/disk.py`
  to retrieve a list of LVM device mappers.
- Updates the `is_lv` property in `ceph_volume/util/device.py`
  to use the new `get_lvm_mappers` function for better accuracy.
- Modifies the symlink handling in `Device` class to properly
  identify LVM logical volumes.
- Adds a new test `test_reject_lv_symlink_to_device` to ensure
  LVM symlinks are correctly identified and handled.
- Updates relevant tests to cover the changes in LVM device detection.

These changes improve the reliability and accuracy of LVM device detection
and handling, ensuring that symlinks to LVM logical volumes are
correctly processed.

Fixes: https://tracker.ceph.com/issues/61597
Signed-off-by: Guillaume Abrioux <gabrioux@ibm.com>
Co-Authored-by: Jerry Pu <jerrypu@qnap.com>
(cherry picked from commit 729c5de4f852f1a1ee90e76b71157e9070af7d99)

diff --git a/src/ceph-volume/ceph_volume/tests/devices/lvm/test_migrate.py b/src/ceph-volume/ceph_volume/tests/devices/lvm/test_migrate.py
index 9396badc3cc6065bb9b97c4336640b72e9e59334..d30ea87398794269bd780e747b5c23b5b4f3feb3 100644 (file)
--- a/src/ceph-volume/ceph_volume/tests/devices/lvm/test_migrate.py
+++ b/src/ceph-volume/ceph_volume/tests/devices/lvm/test_migrate.py
@@ -172,6 +172,7 @@ class TestVolumeTagTracker(object):
         return ('', '', 0)
 
     def test_init(self, monkeypatch):
+        monkeypatch.setattr('ceph_volume.util.device.Device.is_lv', lambda: True)
         source_tags = 'ceph.osd_id=0,ceph.journal_uuid=x,ceph.type=data,ceph.osd_fsid=1234'
         source_db_tags = 'ceph.osd_id=0,journal_uuid=x,ceph.type=db, osd_fsid=1234'
         source_wal_tags = 'ceph.osd_id=0,ceph.journal_uuid=x,ceph.type=wal'
@@ -219,6 +220,7 @@ class TestVolumeTagTracker(object):
         assert 'wal' == t.old_wal_tags['ceph.type']
 
     def test_update_tags_when_lv_create(self, monkeypatch):
+        monkeypatch.setattr('ceph_volume.util.device.Device.is_lv', lambda: True)
         source_tags = \
         'ceph.osd_id=0,ceph.journal_uuid=x,' \
         'ceph.type=data,ceph.osd_fsid=1234'
@@ -277,6 +279,7 @@ class TestVolumeTagTracker(object):
                 '/dev/VolGroup/lv2'] == self.mock_process_input[2]
 
     def test_remove_lvs(self, monkeypatch):
+        monkeypatch.setattr('ceph_volume.util.device.Device.is_lv', lambda: True)
         source_tags = \
         'ceph.osd_id=0,ceph.journal_uuid=x,' \
         'ceph.type=data,ceph.osd_fsid=1234,ceph.wal_uuid=aaaaa'
@@ -336,6 +339,7 @@ class TestVolumeTagTracker(object):
                 '/dev/VolGroup/lv2'] == self.mock_process_input[2]
 
     def test_replace_lvs(self, monkeypatch):
+        monkeypatch.setattr('ceph_volume.util.device.Device.is_lv', lambda: True)
         source_tags = \
         'ceph.osd_id=0,ceph.type=data,ceph.osd_fsid=1234,'\
         'ceph.wal_uuid=wal_uuid,ceph.db_device=/dbdevice'
@@ -412,6 +416,7 @@ class TestVolumeTagTracker(object):
             '/dev/VolGroup/lv_target'].sort()
 
     def test_undo(self, monkeypatch):
+        monkeypatch.setattr('ceph_volume.util.device.Device.is_lv', lambda: True)
         source_tags = 'ceph.osd_id=0,ceph.journal_uuid=x,ceph.type=data,ceph.osd_fsid=1234'
         source_db_tags = 'ceph.osd_id=0,journal_uuid=x,ceph.type=db, osd_fsid=1234'
         source_wal_tags = 'ceph.osd_id=0,ceph.journal_uuid=x,ceph.type=wal'
@@ -571,10 +576,8 @@ class TestNew(object):
         expected = 'Target Logical Volume is already used by ceph: vgname/new_db'
         assert expected in stderr
 
-    @patch('os.getuid')
-    def test_newdb(self, m_getuid, monkeypatch, capsys):
-        m_getuid.return_value = 0
-
+    def test_newdb(self, is_root, monkeypatch, capsys):
+        monkeypatch.setattr('ceph_volume.util.device.Device.is_lv', lambda: True)
         source_tags = \
         'ceph.osd_id=0,ceph.type=data,ceph.osd_fsid=1234,'\
         'ceph.wal_uuid=wal_uuid,ceph.db_device=/dbdevice'
@@ -731,6 +734,7 @@ class TestNew(object):
         assert not stdout
 
     def test_newdb_no_systemd(self, is_root, monkeypatch):
+        monkeypatch.setattr('ceph_volume.util.device.Device.is_lv', lambda: True)
         source_tags = \
         'ceph.osd_id=0,ceph.type=data,ceph.osd_fsid=1234,'\
         'ceph.wal_uuid=wal_uuid,ceph.db_device=/dbdevice'
@@ -819,10 +823,8 @@ class TestNew(object):
             '--dev-target', '/dev/VolGroup/target_volume',
             '--command', 'bluefs-bdev-new-db']
 
-    @patch('os.getuid')
-    def test_newwal(self, m_getuid, monkeypatch, capsys):
-        m_getuid.return_value = 0
-
+    def test_newwal(self, is_root, monkeypatch, capsys):
+        monkeypatch.setattr('ceph_volume.util.device.Device.is_lv', lambda: True)
         source_tags = \
         'ceph.osd_id=0,ceph.type=data,ceph.osd_fsid=1234'
 
@@ -934,6 +936,7 @@ class TestNew(object):
         assert not stdout
 
     def test_newwal_no_systemd(self, is_root, monkeypatch):
+        monkeypatch.setattr('ceph_volume.util.device.Device.is_lv', lambda: True)
         source_tags = \
         'ceph.osd_id=0,ceph.type=data,ceph.osd_fsid=1234'
 
@@ -997,6 +1000,7 @@ class TestNew(object):
 
     @patch('os.getuid')
     def test_newwal_encrypted(self, m_getuid, monkeypatch, capsys):
+        monkeypatch.setattr('ceph_volume.util.device.Device.is_lv', lambda: True)
         m_getuid.return_value = 0
 
         source_tags = \
@@ -1228,6 +1232,7 @@ Example calls for supported scenarios:
 
     @patch.object(Zap, 'main')
     def test_migrate_data_db_to_new_db(self, m_zap, is_root, monkeypatch):
+        monkeypatch.setattr('ceph_volume.util.device.Device.is_lv', lambda: True)
 
         source_tags = 'ceph.osd_id=2,ceph.type=data,ceph.osd_fsid=1234,' \
         'ceph.cluster_name=ceph,ceph.db_uuid=dbuuid,ceph.db_device=db_dev'
@@ -1330,6 +1335,7 @@ Example calls for supported scenarios:
     @patch.object(Zap, 'main')
     @patch('os.getuid')
     def test_migrate_data_db_to_new_db_encrypted(self, m_getuid, m_zap, monkeypatch):
+        monkeypatch.setattr('ceph_volume.util.device.Device.is_lv', lambda: True)
         m_getuid.return_value = 0
 
         source_tags = 'ceph.osd_id=2,ceph.type=data,ceph.osd_fsid=1234,' \
@@ -1509,6 +1515,7 @@ Example calls for supported scenarios:
 
     @patch.object(Zap, 'main')
     def test_migrate_data_db_to_new_db_no_systemd(self, m_zap, is_root, monkeypatch):
+        monkeypatch.setattr('ceph_volume.util.device.Device.is_lv', lambda: True)
         source_tags = 'ceph.osd_id=2,ceph.type=data,ceph.osd_fsid=1234,' \
         'ceph.cluster_name=ceph,ceph.db_uuid=dbuuid,ceph.db_device=db_dev'
         source_db_tags = 'ceph.osd_id=2,ceph.type=db,ceph.osd_fsid=1234,' \
@@ -1608,6 +1615,7 @@ Example calls for supported scenarios:
 
     @patch.object(Zap, 'main')
     def test_migrate_data_db_to_new_db_skip_wal(self, m_zap, is_root, monkeypatch):
+        monkeypatch.setattr('ceph_volume.util.device.Device.is_lv', lambda: True)
         source_tags = 'ceph.osd_id=2,ceph.type=data,ceph.osd_fsid=1234,' \
         'ceph.cluster_name=ceph,ceph.db_uuid=dbuuid,ceph.db_device=db_dev'
         source_db_tags = 'ceph.osd_id=2,ceph.type=db,ceph.osd_fsid=1234,' \
@@ -1730,6 +1738,7 @@ Example calls for supported scenarios:
 
     @patch.object(Zap, 'main')
     def test_migrate_data_db_wal_to_new_db(self, m_zap, is_root, monkeypatch):
+        monkeypatch.setattr('ceph_volume.util.device.Device.is_lv', lambda: True)
         source_tags = 'ceph.osd_id=2,ceph.type=data,ceph.osd_fsid=1234,' \
         'ceph.cluster_name=ceph,ceph.db_uuid=dbuuid,ceph.db_device=db_dev,' \
         'ceph.wal_uuid=waluuid,ceph.wal_device=wal_dev'
@@ -1858,6 +1867,7 @@ Example calls for supported scenarios:
     @patch.object(Zap, 'main')
     @patch('os.getuid')
     def test_migrate_data_db_wal_to_new_db_encrypted(self, m_getuid, m_zap, monkeypatch):
+        monkeypatch.setattr('ceph_volume.util.device.Device.is_lv', lambda: True)
         m_getuid.return_value = 0
 
         source_tags = 'ceph.osd_id=2,ceph.type=data,ceph.osd_fsid=1234,' \
@@ -2148,8 +2158,7 @@ Example calls for supported scenarios:
                                     m_getuid,
                                     monkeypatch,
                                     capsys):
-        m_getuid.return_value = 0
-
+        monkeypatch.setattr('ceph_volume.util.device.Device.is_lv', lambda: True)
         source_tags = 'ceph.osd_id=2,ceph.type=data,ceph.osd_fsid=1234,' \
         'ceph.cluster_name=ceph,ceph.db_uuid=dbuuid,ceph.db_device=db_dev,' \
         'ceph.wal_uuid=waluuid,ceph.wal_device=wal_dev'
@@ -2297,6 +2306,7 @@ Example calls for supported scenarios:
         assert not stdout
 
     def test_migrate_data_db_to_db_no_systemd(self, is_root, monkeypatch):
+        monkeypatch.setattr('ceph_volume.util.device.Device.is_lv', lambda: True)
         source_tags = 'ceph.osd_id=2,ceph.type=data,ceph.osd_fsid=1234,' \
         'ceph.cluster_name=ceph,ceph.db_uuid=dbuuid,ceph.db_device=db_dev,' \
         'ceph.wal_uuid=waluuid,ceph.wal_device=wal_dev'
@@ -2376,7 +2386,7 @@ Example calls for supported scenarios:
                                     is_root,
                                     monkeypatch,
                                     capsys):
-
+        monkeypatch.setattr('ceph_volume.util.device.Device.is_lv', lambda: True)
         source_tags = 'ceph.osd_id=2,ceph.type=data,ceph.osd_fsid=1234,' \
         'ceph.cluster_name=ceph,ceph.db_uuid=dbuuid,ceph.db_device=db_dev,' \
         'ceph.wal_uuid=waluuid,ceph.wal_device=wal_dev'
@@ -2484,6 +2494,7 @@ Example calls for supported scenarios:
                                m_zap,
                                monkeypatch,
                                capsys):
+        monkeypatch.setattr('ceph_volume.util.device.Device.is_lv', lambda: True)
         m_getuid.return_value = 0
 
         source_tags = 'ceph.osd_id=2,ceph.type=data,ceph.osd_fsid=1234,' \
@@ -2575,6 +2586,7 @@ Example calls for supported scenarios:
                                               m_zap,
                                               monkeypatch,
                                               capsys):
+        monkeypatch.setattr('ceph_volume.util.device.Device.is_lv', lambda: True)
         m_getuid.return_value = 0
 
         source_tags = 'ceph.osd_id=2,ceph.type=data,ceph.osd_fsid=1234,' \
@@ -2760,6 +2772,7 @@ Example calls for supported scenarios:
 
     @patch.object(Zap, 'main')
     def test_migrate_data_wal_to_db_no_systemd(self, m_zap, is_root, monkeypatch):
+        monkeypatch.setattr('ceph_volume.util.device.Device.is_lv', lambda: True)
         source_tags = 'ceph.osd_id=2,ceph.type=data,ceph.osd_fsid=1234,' \
         'ceph.cluster_name=ceph,ceph.db_uuid=dbuuid,ceph.db_device=db_dev,' \
         'ceph.wal_uuid=waluuid,ceph.wal_device=wal_dev'

diff --git a/src/ceph-volume/ceph_volume/tests/util/test_device.py b/src/ceph-volume/ceph_volume/tests/util/test_device.py
index 69b57c21110ee53208128eeb98412f23773fa6ab..9a41d9683213eaf8e90e10e35c2d85b2abd92138 100644 (file)
--- a/src/ceph-volume/ceph_volume/tests/util/test_device.py
+++ b/src/ceph-volume/ceph_volume/tests/util/test_device.py
@@ -47,7 +47,8 @@ class TestDevice(object):
         disk = device.Device("/dev/sda")
         assert disk.lvm_size.gb == 4
 
-    def test_is_lv(self, fake_call, device_info):
+    def test_is_lv(self, fake_call, device_info, monkeypatch):
+        monkeypatch.setattr('ceph_volume.util.device.Device.is_lv', lambda: True)
         data = {"lv_path": "vg/lv", "vg_name": "vg", "name": "lv"}
         lsblk = {"TYPE": "lvm", "NAME": "vg-lv"}
         device_info(lv=data,lsblk=lsblk)
@@ -310,6 +311,22 @@ class TestDevice(object):
         disk = device.Device("/dev/cdrom")
         assert not disk.available
 
+    @patch("ceph_volume.util.disk.has_bluestore_label", lambda x: False)
+    @patch('ceph_volume.util.device.os.path.realpath')
+    @patch('ceph_volume.util.device.os.path.islink')
+    def test_reject_lv_symlink_to_device(self,
+                                         m_os_path_islink,
+                                         m_os_path_realpath,
+                                         device_info,
+                                         fake_call):
+        m_os_path_islink.return_value = True
+        m_os_path_realpath.return_value = '/dev/mapper/vg-lv'
+        lv = {"lv_path": "/dev/vg/lv", "vg_name": "vg", "name": "lv"}
+        lsblk = {"TYPE": "lvm", "NAME": "vg-lv"}
+        device_info(lv=lv,lsblk=lsblk)
+        disk = device.Device("/dev/vg/lv")
+        assert disk.path == '/dev/vg/lv'
+
     @patch("ceph_volume.util.disk.has_bluestore_label", lambda x: False)
     def test_reject_smaller_than_5gb(self, fake_call, device_info):
         data = {"/dev/sda": {"size": 5368709119}}
@@ -528,7 +545,8 @@ class TestDeviceEncryption(object):
         assert disk.is_encrypted is False
 
     @patch("ceph_volume.util.disk.has_bluestore_label", lambda x: False)
-    def test_lv_is_encrypted_blkid(self, fake_call, device_info):
+    def test_lv_is_encrypted_blkid(self, fake_call, device_info, monkeypatch):
+        monkeypatch.setattr('ceph_volume.util.device.Device.is_lv', lambda: True)
         lsblk = {'TYPE': 'lvm', 'NAME': 'sda'}
         blkid = {'TYPE': 'crypto_LUKS'}
         device_info(lsblk=lsblk, blkid=blkid)
@@ -537,7 +555,8 @@ class TestDeviceEncryption(object):
         assert disk.is_encrypted is True
 
     @patch("ceph_volume.util.disk.has_bluestore_label", lambda x: False)
-    def test_lv_is_not_encrypted_blkid(self, fake_call, factory, device_info):
+    def test_lv_is_not_encrypted_blkid(self, fake_call, factory, device_info, monkeypatch):
+        monkeypatch.setattr('ceph_volume.util.device.Device.is_lv', lambda: True)
         lsblk = {'TYPE': 'lvm', 'NAME': 'sda'}
         blkid = {'TYPE': 'xfs'}
         device_info(lsblk=lsblk, blkid=blkid)
@@ -546,7 +565,8 @@ class TestDeviceEncryption(object):
         assert disk.is_encrypted is False
 
     @patch("ceph_volume.util.disk.has_bluestore_label", lambda x: False)
-    def test_lv_is_encrypted_lsblk(self, fake_call, device_info):
+    def test_lv_is_encrypted_lsblk(self, fake_call, device_info, monkeypatch):
+        monkeypatch.setattr('ceph_volume.util.device.Device.is_lv', lambda: True)
         lsblk = {'FSTYPE': 'crypto_LUKS', 'NAME': 'sda', 'TYPE': 'lvm'}
         blkid = {'TYPE': 'mapper'}
         device_info(lsblk=lsblk, blkid=blkid)
@@ -555,7 +575,8 @@ class TestDeviceEncryption(object):
         assert disk.is_encrypted is True
 
     @patch("ceph_volume.util.disk.has_bluestore_label", lambda x: False)
-    def test_lv_is_not_encrypted_lsblk(self, fake_call, factory, device_info):
+    def test_lv_is_not_encrypted_lsblk(self, fake_call, factory, device_info, monkeypatch):
+        monkeypatch.setattr('ceph_volume.util.device.Device.is_lv', lambda: True)
         lsblk = {'FSTYPE': 'xfs', 'NAME': 'sda', 'TYPE': 'lvm'}
         blkid = {'TYPE': 'mapper'}
         device_info(lsblk=lsblk, blkid=blkid)
@@ -564,7 +585,8 @@ class TestDeviceEncryption(object):
         assert disk.is_encrypted is False
 
     @patch("ceph_volume.util.disk.has_bluestore_label", lambda x: False)
-    def test_lv_is_encrypted_lvm_api(self, fake_call, factory, device_info):
+    def test_lv_is_encrypted_lvm_api(self, fake_call, factory, device_info, monkeypatch):
+        monkeypatch.setattr('ceph_volume.util.device.Device.is_lv', lambda: True)
         lsblk = {'FSTYPE': 'xfs', 'NAME': 'sda', 'TYPE': 'lvm'}
         blkid = {'TYPE': 'mapper'}
         device_info(lsblk=lsblk, blkid=blkid)
@@ -573,7 +595,8 @@ class TestDeviceEncryption(object):
         assert disk.is_encrypted is True
 
     @patch("ceph_volume.util.disk.has_bluestore_label", lambda x: False)
-    def test_lv_is_not_encrypted_lvm_api(self, fake_call, factory, device_info):
+    def test_lv_is_not_encrypted_lvm_api(self, fake_call, factory, device_info, monkeypatch):
+        monkeypatch.setattr('ceph_volume.util.device.Device.is_lv', lambda: True)
         lsblk = {'FSTYPE': 'xfs', 'NAME': 'sda', 'TYPE': 'lvm'}
         blkid = {'TYPE': 'mapper'}
         device_info(lsblk=lsblk, blkid=blkid)

diff --git a/src/ceph-volume/ceph_volume/util/device.py b/src/ceph-volume/ceph_volume/util/device.py
index 1b52774d1a1b32ca4eb8d26f982796e7c9d149a1..8299586547700b7765e629d5f2fde950c3a03b5c 100644 (file)
--- a/src/ceph-volume/ceph_volume/util/device.py
+++ b/src/ceph-volume/ceph_volume/util/device.py
@@ -119,7 +119,7 @@ class Device(object):
             self.symlink = self.path
             real_path = os.path.realpath(self.path)
             # check if we are not a device mapper
-            if "dm-" not in real_path:
+            if "dm-" not in real_path and not self.is_lv:
                 self.path = real_path
         if not sys_info.devices.get(self.path):
             sys_info.devices = disk.get_devices()
@@ -468,8 +468,9 @@ class Device(object):
         return self.device_type == 'mpath'
 
     @property
-    def is_lv(self):
-        return self.lv_api is not None
+    def is_lv(self) -> bool:
+        path = os.path.realpath(self.path)
+        return path in disk.get_lvm_mappers()
 
     @property
     def is_partition(self):

diff --git a/src/ceph-volume/ceph_volume/util/disk.py b/src/ceph-volume/ceph_volume/util/disk.py
index a0bc39b9d179ac8c7d8f6fa3c754683c49a9df85..95d69da8d5e174edb9b548c4b82aae81c9afbbcc 100644 (file)
--- a/src/ceph-volume/ceph_volume/util/disk.py
+++ b/src/ceph-volume/ceph_volume/util/disk.py
@@ -931,3 +931,35 @@ def has_bluestore_label(device_path):
         logger.info(f'{device_path} is a directory, skipping.')
 
     return isBluestore
+
+def get_lvm_mappers(sys_block_path: str = '/sys/block') -> List[str]:
+    """
+    Retrieve a list of Logical Volume Manager (LVM) device mappers.
+
+    This function scans the given system block path for device mapper (dm) devices
+    and identifies those that are managed by LVM. For each LVM device found, it adds
+    the corresponding paths to the result list.
+
+    Args:
+        sys_block_path (str, optional): The path to the system block directory. Defaults to '/sys/block'.
+
+    Returns:
+        List[str]: A list of strings representing the paths of LVM device mappers.
+                   Each LVM device will have two entries: the /dev/mapper/ path and the /dev/ path.
+    """
+    result: List[str] = []
+    for device in os.listdir(sys_block_path):
+        path: str = os.path.join(sys_block_path, device, 'dm')
+        uuid_path: str = os.path.join(path, 'uuid')
+        name_path: str = os.path.join(path, 'name')
+
+        if os.path.exists(uuid_path):
+            with open(uuid_path, 'r') as f:
+                mapper_type: str = f.read().split('-')[0]
+
+            if mapper_type == 'LVM':
+                with open(name_path, 'r') as f:
+                    name: str = f.read()
+                    result.append(f'/dev/mapper/{name.strip()}')
+                    result.append(f'/dev/{device}')
+    return result