From: Guillaume Abrioux Date: Thu, 4 Jul 2024 11:59:37 +0000 (+0000) Subject: ceph-volume: do not convert LVs's symlink to real path X-Git-Tag: testing/wip-xiubli-testing-20240812.075113-squid~31^2~1 X-Git-Url: http://git.apps.os.sepia.ceph.com/?a=commitdiff_plain;h=0f6ceb27fc8cb48c008e70bd59e3edd0e5f5708b;p=ceph-ci.git 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 Co-Authored-by: Jerry Pu (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 99aba1285c3..5f4734e44cd 100644 --- 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' @@ -568,6 +573,7 @@ class TestNew(object): assert expected in stderr 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' @@ -724,6 +730,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' @@ -813,6 +820,7 @@ class TestNew(object): '--command', 'bluefs-bdev-new-db'] 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' @@ -924,6 +932,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' @@ -987,6 +996,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 = \ @@ -1218,6 +1228,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' @@ -1320,6 +1331,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,' \ @@ -1499,6 +1511,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,' \ @@ -1598,6 +1611,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,' \ @@ -1720,6 +1734,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' @@ -1848,6 +1863,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,' \ @@ -2133,6 +2149,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' @@ -2280,6 +2297,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' @@ -2359,6 +2377,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' @@ -2466,6 +2485,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,' \ @@ -2557,6 +2577,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,' \ @@ -2742,6 +2763,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' @@ -2838,4 +2860,4 @@ Example calls for supported scenarios: '--devs-source', '/var/lib/ceph/osd/ceph-2/block', '--devs-source', '/var/lib/ceph/osd/ceph-2/block.wal'] - m_zap.assert_called_once() \ No newline at end of file + m_zap.assert_called_once() 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 69b57c21110..9a41d968321 100644 --- 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 1b52774d1a1..82995865477 100644 --- 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 a0bc39b9d17..95d69da8d5e 100644 --- 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