ceph-volume: skip internal raid mirror LVs in inventory

ceph-volume inventory started including all LVM mapper devices after
c06bee965f1. On hosts with raid mirrored system volumes, that pulls in
hidden legs like var_rmeta_0 which have no /dev/vg/lv node and makes
cephadm's ceph-volume inventory call fail.

Skip those internal LVs in get_devices() and avoid rewriting the device
path to a missing lv_path in Device._parse().

Fixces: https://tracker.ceph.com/issues/77486

Signed-off-by: Guillaume Abrioux <gabrioux@ibm.com>
(cherry picked from commit daec7e125a167bb8fd0fb9faaa04879f0bb215d2)

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 1667979088f385146ec0a73b2289476bb5cf4b39..b205bcf92ef7c24ffd616e7cec5176b33773d9fb 100644 (file)
--- a/src/ceph-volume/ceph_volume/tests/util/test_device.py
+++ b/src/ceph-volume/ceph_volume/tests/util/test_device.py
@@ -314,11 +314,13 @@ class TestDevice(object):
         assert not disk.available
 
     @patch("ceph_volume.util.disk.has_bluestore_label", lambda x: False)
+    @patch('ceph_volume.util.device.disk.path_is_block_device', return_value=True)
     @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,
+                                         m_path_is_block_device,
                                          device_info,
                                          fake_call):
         m_os_path_islink.return_value = True
@@ -328,6 +330,26 @@ class TestDevice(object):
         device_info(lv=lv,lsblk=lsblk)
         disk = device.Device("/dev/vg/lv")
         assert disk.path == '/dev/vg/lv'
+        m_path_is_block_device.assert_called_with('/dev/vg/lv')
+
+    @patch("ceph_volume.util.disk.has_bluestore_label", lambda x: False)
+    @patch('ceph_volume.util.device.disk.path_is_block_device', return_value=False)
+    @patch('ceph_volume.util.device.os.path.realpath')
+    @patch('ceph_volume.util.device.os.path.islink')
+    def test_lv_keeps_mapper_path_when_lv_path_missing(self,
+                                                       m_os_path_islink,
+                                                       m_os_path_realpath,
+                                                       m_path_is_block_device,
+                                                       device_info,
+                                                       fake_call):
+        m_os_path_islink.return_value = False
+        m_os_path_realpath.return_value = '/dev/mapper/vg-var_rmeta_0'
+        lv = {"lv_path": "/dev/vg/var_rmeta_0", "vg_name": "vg", "name": "var_rmeta_0", "tags": {}}
+        lsblk = {"TYPE": "lvm", "NAME": "vg-var_rmeta_0"}
+        device_info(lv=lv, lsblk=lsblk)
+        disk = device.Device("/dev/mapper/vg-var_rmeta_0")
+        assert disk.path == '/dev/mapper/vg-var_rmeta_0'
+        m_path_is_block_device.assert_called_with('/dev/vg/var_rmeta_0')
 
     @patch("ceph_volume.util.disk.has_bluestore_label", lambda x: False)
     def test_reject_smaller_than_5gb(self, fake_call, device_info):

diff --git a/src/ceph-volume/ceph_volume/tests/util/test_disk.py b/src/ceph-volume/ceph_volume/tests/util/test_disk.py
index 5aa3a609f73829c991eb5ad797143848fab33e7e..29a2fceffda5988667dcdbbdc4469506c893d244 100644 (file)
--- a/src/ceph-volume/ceph_volume/tests/util/test_disk.py
+++ b/src/ceph-volume/ceph_volume/tests/util/test_disk.py
@@ -358,8 +358,10 @@ class TestGetDevices(object):
         fake_filesystem.create_file('/sys/block/dm-0/size', contents='204800')
         fake_filesystem.create_file('/sys/block/dm-0/queue/rotational', contents='1')
         fake_filesystem.create_file('/sys/block/dm-0/queue/hw_sector_size', contents='512')
+        fake_filesystem.create_file(lv_path, st_mode=(stat.S_IFBLK | 0o600))
         with patch("ceph_volume.util.disk.UdevData") as MockUdevData:
             mock_instance = MagicMock()
+            mock_instance.is_internal_lv = False
             mock_instance.preferred_block_path = lv_path
             mock_instance.environment = {}
             MockUdevData.return_value = mock_instance
@@ -368,6 +370,46 @@ class TestGetDevices(object):
         assert result[lv_path]['type'] == 'lvm'
         assert result[lv_path]['human_readable_size'] == '100.00 MB'
 
+    def test_internal_raid_lv_is_excluded(self, patched_get_block_devs_sysfs, fake_filesystem):
+        mapper_path = '/dev/mapper/debian-var_rmeta_0'
+        dm_path = '/dev/dm-5'
+        patched_get_block_devs_sysfs.return_value = [
+            [dm_path, mapper_path, 'lvm', dm_path]
+        ]
+        fake_filesystem.create_dir('/sys/block/dm-5/slaves')
+        fake_filesystem.create_dir('/sys/block/dm-5/queue')
+        fake_filesystem.create_file('/sys/block/dm-5/size', contents='8192')
+        fake_filesystem.create_file('/sys/block/dm-5/queue/rotational', contents='1')
+        fake_filesystem.create_file('/sys/block/dm-5/queue/hw_sector_size', contents='512')
+        with patch("ceph_volume.util.disk.UdevData") as MockUdevData:
+            mock_instance = MagicMock()
+            mock_instance.is_internal_lv = True
+            MockUdevData.return_value = mock_instance
+            result = disk.get_devices()
+        assert mapper_path not in result
+        assert not result
+
+    def test_lvm_device_without_accessible_node_is_excluded(
+        self, patched_get_block_devs_sysfs, fake_filesystem
+    ):
+        mapper_path = '/dev/mapper/debian-var_rmeta_0'
+        dm_path = '/dev/dm-5'
+        patched_get_block_devs_sysfs.return_value = [
+            [dm_path, mapper_path, 'lvm', dm_path]
+        ]
+        fake_filesystem.create_dir('/sys/block/dm-5/slaves')
+        fake_filesystem.create_dir('/sys/block/dm-5/queue')
+        fake_filesystem.create_file('/sys/block/dm-5/size', contents='8192')
+        fake_filesystem.create_file('/sys/block/dm-5/queue/rotational', contents='1')
+        fake_filesystem.create_file('/sys/block/dm-5/queue/hw_sector_size', contents='512')
+        with patch("ceph_volume.util.disk.UdevData") as MockUdevData:
+            mock_instance = MagicMock()
+            mock_instance.is_internal_lv = False
+            mock_instance.preferred_block_path = '/dev/debian/var_rmeta_0'
+            MockUdevData.return_value = mock_instance
+            result = disk.get_devices()
+        assert not result
+
 
 class TestGetBlockDevsSysfs(object):
     def test_optical_device_is_skipped(self, fake_filesystem):
@@ -862,6 +904,36 @@ V:1"""
     def test_dashed_path_with_lvm(self) -> None:
         assert disk.UdevData(self.fake_device).dashed_path == '/dev/mapper/fake_vg1-fake-lv1'
 
+    @patch('ceph_volume.util.disk.os.stat', _udev_data_patched_os_stat)
+    @patch('ceph_volume.util.disk.os.minor', Mock(return_value=1))
+    @patch('ceph_volume.util.disk.os.major', Mock(return_value=998))
+    def test_is_internal_lv_true_for_raid_metadata(self) -> None:
+        udev = disk.UdevData(self.fake_device)
+        udev.environment['DM_LV_NAME'] = 'var_rmeta_0'
+        assert udev.is_internal_lv
+
+    @patch('ceph_volume.util.disk.os.stat', _udev_data_patched_os_stat)
+    @patch('ceph_volume.util.disk.os.minor', Mock(return_value=1))
+    @patch('ceph_volume.util.disk.os.major', Mock(return_value=998))
+    def test_is_internal_lv_true_for_raid_image(self) -> None:
+        udev = disk.UdevData(self.fake_device)
+        udev.environment['DM_LV_NAME'] = 'var_rimage_1'
+        assert udev.is_internal_lv
+
+    @patch('ceph_volume.util.disk.os.stat', _udev_data_patched_os_stat)
+    @patch('ceph_volume.util.disk.os.minor', Mock(return_value=1))
+    @patch('ceph_volume.util.disk.os.major', Mock(return_value=998))
+    def test_is_internal_lv_true_for_layered_lv(self) -> None:
+        udev = disk.UdevData(self.fake_device)
+        udev.environment['DM_LV_LAYER'] = 'var'
+        assert udev.is_internal_lv
+
+    @patch('ceph_volume.util.disk.os.stat', _udev_data_patched_os_stat)
+    @patch('ceph_volume.util.disk.os.minor', Mock(return_value=1))
+    @patch('ceph_volume.util.disk.os.major', Mock(return_value=998))
+    def test_is_internal_lv_false_for_public_lv(self) -> None:
+        assert not disk.UdevData(self.fake_device).is_internal_lv
+
     @patch('ceph_volume.util.disk.os.stat', _udev_data_patched_os_stat)
     @patch('ceph_volume.util.disk.os.minor', Mock(return_value=1))
     @patch('ceph_volume.util.disk.os.major', Mock(return_value=998))

diff --git a/src/ceph-volume/ceph_volume/util/device.py b/src/ceph-volume/ceph_volume/util/device.py
index c5b35e4859861eb52dec41e272e626886f191bb2..5082c6aee2875b7da452786fce0b85a5950a01e3 100644 (file)
--- a/src/ceph-volume/ceph_volume/util/device.py
+++ b/src/ceph-volume/ceph_volume/util/device.py
@@ -236,7 +236,8 @@ class Device(object):
         if lv:
             self.lv_api = lv
             self.lvs = [lv]
-            self.path = lv.lv_path
+            if disk.path_is_block_device(lv.lv_path):
+                self.path = lv.lv_path
             self.vg_name = lv.vg_name
             self.lv_name = lv.name
             self.ceph_device_lvm = lvm.is_ceph_device(lv)
@@ -319,7 +320,10 @@ class Device(object):
         src/common/blkdev.cc
         """
 
-        udev_data = disk.UdevData(self.path)
+        try:
+            udev_data = disk.UdevData(self.path)
+        except RuntimeError:
+            return ''
         env = udev_data.environment
         parts: list[str] = []
         model = env.get('ID_MODEL', '')

diff --git a/src/ceph-volume/ceph_volume/util/disk.py b/src/ceph-volume/ceph_volume/util/disk.py
index 6ec1b2a56b7e00889c526d695dc25d4ccc84d82f..d37fe620fe9c67a056a82d65f4ecacd390bdd964 100644 (file)
--- a/src/ceph-volume/ceph_volume/util/disk.py
+++ b/src/ceph-volume/ceph_volume/util/disk.py
@@ -163,6 +163,14 @@ def _stat_is_device(stat_obj):
     return stat.S_ISBLK(stat_obj)
 
 
+def path_is_block_device(path: str) -> bool:
+    """True if ``path`` exists and is a block device (follows symlinks)."""
+    try:
+        return _stat_is_device(os.stat(path).st_mode)
+    except OSError:
+        return False
+
+
 def _lsblk_parser(line):
     """
     Parses lines in lsblk output. Requires output to be in pair mode (``-P`` flag). Lines
@@ -801,7 +809,22 @@ def get_devices(_sys_block_path='/sys/block', device=''):
     for block in block_devs:
         metadata: Dict[str, Any] = {}
         if block[2] == 'lvm':
-            block[1] = UdevData(block[1]).preferred_block_path
+            try:
+                udev_data = UdevData(block[1])
+            except RuntimeError as exc:
+                logger.debug(
+                    'get_devices(): skipping LVM device %s: %s', block[1], exc)
+                continue
+            if udev_data.is_internal_lv:
+                logger.debug(
+                    'get_devices(): skipping internal LVM LV %s', block[1])
+                continue
+            block[1] = udev_data.preferred_block_path
+            if not path_is_block_device(block[1]):
+                logger.debug(
+                    'get_devices(): skipping LVM device without accessible node %s',
+                    block[1])
+                continue
         devname = os.path.basename(block[0])
         diskname = block[1]
         if block[2] not in block_types:
@@ -1416,6 +1439,16 @@ class UdevData:
         """
         return self.environment.get('DM_UUID', '').startswith('LVM')
 
+    @property
+    def is_internal_lv(self) -> bool:
+        lv_name = self.environment.get('DM_LV_NAME', '')
+        if not lv_name:
+            return False
+        for marker in ('_rmeta_', '_rimage_', '_rtmeta_', '_rtimage_'):
+            if marker in lv_name:
+                return True
+        return bool(self.environment.get('DM_LV_LAYER', ''))
+
     @property
     def slashed_path(self) -> str:
         """Get the LVM path structured with slashes.
@@ -1445,14 +1478,6 @@ class UdevData:
             result = f'/dev/mapper/{name}'
         return result
 
-    @staticmethod
-    def _path_is_block_device(path: str) -> bool:
-        """True if ``path`` exists and is a block device (follows symlinks)."""
-        try:
-            return _stat_is_device(os.stat(path).st_mode)
-        except OSError:
-            return False
-
     @property
     def preferred_block_path(self) -> str:
         """Return a device path that exists for typical open(2) / blkid usage.
@@ -1468,9 +1493,9 @@ class UdevData:
         if not self.is_lvm:
             return self.path
         slashed: str = self.slashed_path
-        if self._path_is_block_device(slashed):
+        if path_is_block_device(slashed):
             return slashed
         dashed: str = self.dashed_path
-        if self._path_is_block_device(dashed):
+        if path_is_block_device(dashed):
             return dashed
         return self.path