From 44b0767e99de8a99eb2de3d3b783474d5ee4e2f6 Mon Sep 17 00:00:00 2001 From: Guillaume Abrioux Date: Thu, 14 Aug 2025 11:43:40 +0000 Subject: [PATCH] ceph-volume: drop udevadm subprocess calls Calling udevadm via subprocess can cause processes to pile up under heavy load on production clusters. This commit switches to reading udev data directly from /run/udev/data, which is mounted as tmpfs. Signed-off-by: Guillaume Abrioux (cherry picked from commit 727e69db73ef39d51bdd45515536e59d3acff19e) --- src/ceph-volume/ceph_volume/tests/conftest.py | 28 ++++++-- .../ceph_volume/tests/devices/lvm/data_zap.py | 14 ---- .../tests/devices/lvm/test_migrate.py | 68 +++++++++--------- .../ceph_volume/tests/devices/lvm/test_zap.py | 16 ++--- .../tests/objectstore/test_lvmbluestore.py | 2 +- .../ceph_volume/tests/test_inventory.py | 1 - .../ceph_volume/tests/util/test_device.py | 15 ++-- .../ceph_volume/tests/util/test_disk.py | 52 +++----------- src/ceph-volume/ceph_volume/util/device.py | 37 +++++----- src/ceph-volume/ceph_volume/util/disk.py | 69 +++++-------------- 10 files changed, 120 insertions(+), 182 deletions(-) diff --git a/src/ceph-volume/ceph_volume/tests/conftest.py b/src/ceph-volume/ceph_volume/tests/conftest.py index 03b0a6adaae..a58e190b936 100644 --- a/src/ceph-volume/ceph_volume/tests/conftest.py +++ b/src/ceph-volume/ceph_volume/tests/conftest.py @@ -288,7 +288,6 @@ def disable_kernel_queries(monkeypatch): This speeds up calls to Device and Disk ''' monkeypatch.setattr("ceph_volume.util.device.disk.get_devices", lambda device='': {}) - monkeypatch.setattr("ceph_volume.util.disk.udevadm_property", lambda *a, **kw: {}) @pytest.fixture(params=[ @@ -356,9 +355,15 @@ def patch_bluestore_label(): yield p @pytest.fixture -def device_info(monkeypatch, patch_bluestore_label): - def apply(devices=None, lsblk=None, lv=None, blkid=None, udevadm=None, - has_bluestore_label=False): +def patch_udevdata(monkeypatch): + fake_udevdata = MagicMock() + fake_udevdata.environment = {k:k for k in ['ID_VENDOR', 'ID_MODEL', 'ID_SCSI_SERIAL']} + monkeypatch.setattr("ceph_volume.util.disk.UdevData", lambda path: fake_udevdata) + yield + +@pytest.fixture +def device_info(monkeypatch, patch_bluestore_label, patch_udevdata): + def apply(devices=None, lsblk=None, lv=None, blkid=None): if devices: for dev in devices.keys(): devices[dev]['device_nodes'] = [os.path.basename(dev)] @@ -366,7 +371,6 @@ def device_info(monkeypatch, patch_bluestore_label): devices = {} lsblk = lsblk if lsblk else {} blkid = blkid if blkid else {} - udevadm = udevadm if udevadm else {} lv = Factory(**lv) if lv else None monkeypatch.setattr("ceph_volume.sys_info.devices", {}) monkeypatch.setattr("ceph_volume.util.device.disk.get_devices", lambda device='': devices) @@ -377,7 +381,6 @@ def device_info(monkeypatch, patch_bluestore_label): lambda path: [lv]) monkeypatch.setattr("ceph_volume.util.device.disk.lsblk", lambda path: lsblk) monkeypatch.setattr("ceph_volume.util.device.disk.blkid", lambda path: blkid) - monkeypatch.setattr("ceph_volume.util.disk.udevadm_property", lambda *a, **kw: udevadm) return apply @pytest.fixture(params=[0.1, 0.2, 0.3, 0.4, 0.5, 0.6, 0.7, 0.8, 0.9, 0.95, 0.999, 1.0]) @@ -386,12 +389,23 @@ def data_allocate_fraction(request): @pytest.fixture def fake_filesystem(fs): - + fs.create_dir('/dev') fs.create_dir('/sys/block/sda/slaves') fs.create_dir('/sys/block/sda/queue') fs.create_dir('/sys/block/rbd0') fs.create_dir('/var/log/ceph') fs.create_dir('/tmp/osdpath') + fs.create_file('/sys/block/sda/dev', contents='8:0') + fs.create_dir('/run/udev/data') + fs.create_file('/run/udev/data/b8:0', contents=""" +P:8:0 +E:DEVNAME=/dev/sda +E:DEVTYPE=disk +E:ID_MODEL= +E:ID_SERIAL= +E:ID_VENDOR= +""".strip()) + yield fs @pytest.fixture diff --git a/src/ceph-volume/ceph_volume/tests/devices/lvm/data_zap.py b/src/ceph-volume/ceph_volume/tests/devices/lvm/data_zap.py index c971b7776ef..c804137f3ab 100644 --- a/src/ceph-volume/ceph_volume/tests/devices/lvm/data_zap.py +++ b/src/ceph-volume/ceph_volume/tests/devices/lvm/data_zap.py @@ -65,17 +65,3 @@ lsblk_all = ['NAME="/dev/sdb" KNAME="/dev/sdb" PKNAME="" PARTLABEL=""', 'NAME="/dev/sdz" KNAME="/dev/sdz" PKNAME="" PARTLABEL=""'] blkid_output = ['/dev/ceph-1172bba3-3e0e-45e5-ace6-31ae8401221f/osd-block-5050a85c-d1a7-4d66-b4ba-2e9b1a2970ae: TYPE="ceph_bluestore" USAGE="other"'] - -udevadm_property = '''DEVNAME=/dev/sdb -DEVTYPE=disk -ID_ATA=1 -ID_BUS=ata -ID_MODEL=SK_hynix_SC311_SATA_512GB -ID_PART_TABLE_TYPE=gpt -ID_PART_TABLE_UUID=c8f91d57-b26c-4de1-8884-0c9541da288c -ID_PATH=pci-0000:00:17.0-ata-3 -ID_PATH_TAG=pci-0000_00_17_0-ata-3 -ID_REVISION=70000P10 -ID_SERIAL=SK_hynix_SC311_SATA_512GB_MS83N71801150416A -TAGS=:systemd: -USEC_INITIALIZED=16117769'''.split('\n') \ No newline at end of file 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 0ec7c3f26c4..78182a79761 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 @@ -54,7 +54,7 @@ class TestFindAssociatedDevices(object): p = kwargs['filters']['lv_path'] return self.mock_single_volumes[p] - def test_lv_is_matched_id(self, monkeypatch): + def test_lv_is_matched_id(self, monkeypatch, patch_udevdata): tags = 'ceph.osd_id=0,ceph.journal_uuid=x,ceph.type=data,ceph.osd_fsid=1234' vol = api.Volume(lv_name='volume1', lv_uuid='y', vg_name='', lv_path='/dev/VolGroup/lv1', lv_tags=tags) @@ -76,7 +76,7 @@ class TestFindAssociatedDevices(object): assert result[0][0].lvs == [vol] assert result[0][1] == 'block' - def test_lv_is_matched_id2(self, monkeypatch): + def test_lv_is_matched_id2(self, monkeypatch, patch_udevdata): tags = 'ceph.osd_id=0,ceph.journal_uuid=x,ceph.type=data,ceph.osd_fsid=1234' vol = api.Volume(lv_name='volume1', lv_uuid='y', vg_name='vg', lv_path='/dev/VolGroup/lv1', lv_tags=tags) @@ -107,7 +107,7 @@ class TestFindAssociatedDevices(object): else: assert False - def test_lv_is_matched_id3(self, monkeypatch): + def test_lv_is_matched_id3(self, monkeypatch, patch_udevdata): tags = 'ceph.osd_id=0,ceph.journal_uuid=x,ceph.type=data,ceph.osd_fsid=1234' vol = api.Volume(lv_name='volume1', lv_uuid='y', vg_name='vg', lv_path='/dev/VolGroup/lv1', lv_tags=tags) @@ -171,7 +171,7 @@ class TestVolumeTagTracker(object): self.mock_process_input.append(args[0]); return ('', '', 0) - def test_init(self, monkeypatch): + def test_init(self, monkeypatch, patch_udevdata): 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' @@ -219,7 +219,7 @@ class TestVolumeTagTracker(object): assert 3 == len(t.old_wal_tags) assert 'wal' == t.old_wal_tags['ceph.type'] - def test_update_tags_when_lv_create(self, monkeypatch): + def test_update_tags_when_lv_create(self, monkeypatch, patch_udevdata): monkeypatch.setattr('ceph_volume.util.device.Device.is_lv', lambda: True) source_tags = \ 'ceph.osd_id=0,ceph.journal_uuid=x,' \ @@ -278,7 +278,7 @@ class TestVolumeTagTracker(object): '--addtag', 'ceph.wal_device=/dev/VolGroup/lv_target', '/dev/VolGroup/lv2'] == self.mock_process_input[2] - def test_remove_lvs(self, monkeypatch): + def test_remove_lvs(self, monkeypatch, patch_udevdata): monkeypatch.setattr('ceph_volume.util.device.Device.is_lv', lambda: True) source_tags = \ 'ceph.osd_id=0,ceph.journal_uuid=x,' \ @@ -338,7 +338,7 @@ class TestVolumeTagTracker(object): '--deltag', 'ceph.wal_device=aaaaa', '/dev/VolGroup/lv2'] == self.mock_process_input[2] - def test_replace_lvs(self, monkeypatch): + def test_replace_lvs(self, monkeypatch, patch_udevdata): monkeypatch.setattr('ceph_volume.util.device.Device.is_lv', lambda: True) source_tags = \ 'ceph.osd_id=0,ceph.type=data,ceph.osd_fsid=1234,'\ @@ -415,7 +415,7 @@ class TestVolumeTagTracker(object): '--addtag', 'ceph.db_device=/dev/VolGroup/lv_target', '/dev/VolGroup/lv_target'].sort() - def test_undo(self, monkeypatch): + def test_undo(self, monkeypatch, patch_udevdata): 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' @@ -573,7 +573,7 @@ class TestNew(object): expected = 'Target Logical Volume is already used by ceph: vgname/new_db' assert expected in stderr - def test_newdb(self, is_root, monkeypatch, capsys): + def test_newdb(self, is_root, monkeypatch, capsys, patch_udevdata): monkeypatch.setattr('ceph_volume.util.device.Device.is_lv', lambda: True) source_tags = \ 'ceph.osd_id=0,ceph.type=data,ceph.osd_fsid=1234,'\ @@ -730,7 +730,7 @@ class TestNew(object): assert '--> OSD ID is running, stop it with: systemctl stop ceph-osd@1' == stderr.rstrip() assert not stdout - def test_newdb_no_systemd(self, is_root, monkeypatch): + def test_newdb_no_systemd(self, is_root, monkeypatch, patch_udevdata): monkeypatch.setattr('ceph_volume.util.device.Device.is_lv', lambda: True) source_tags = \ 'ceph.osd_id=0,ceph.type=data,ceph.osd_fsid=1234,'\ @@ -820,7 +820,7 @@ class TestNew(object): '--dev-target', '/dev/VolGroup/target_volume', '--command', 'bluefs-bdev-new-db'] - def test_newwal(self, is_root, monkeypatch, capsys): + def test_newwal(self, is_root, monkeypatch, capsys, patch_udevdata): monkeypatch.setattr('ceph_volume.util.device.Device.is_lv', lambda: True) source_tags = \ 'ceph.osd_id=0,ceph.type=data,ceph.osd_fsid=1234' @@ -932,7 +932,7 @@ class TestNew(object): assert '--> OSD ID is running, stop it with: systemctl stop ceph-osd@2' == stderr.rstrip() assert not stdout - def test_newwal_no_systemd(self, is_root, monkeypatch): + def test_newwal_no_systemd(self, is_root, monkeypatch, patch_udevdata): monkeypatch.setattr('ceph_volume.util.device.Device.is_lv', lambda: True) source_tags = \ 'ceph.osd_id=0,ceph.type=data,ceph.osd_fsid=1234' @@ -996,7 +996,7 @@ class TestNew(object): '--command', 'bluefs-bdev-new-wal'] @patch('os.getuid') - def test_newwal_encrypted(self, m_getuid, monkeypatch, capsys): + def test_newwal_encrypted(self, m_getuid, monkeypatch, capsys, patch_udevdata): monkeypatch.setattr('ceph_volume.util.device.Device.is_lv', lambda: True) m_getuid.return_value = 0 @@ -1097,7 +1097,7 @@ class TestMigrate(object): def mock_dmcrypt_close(self, *args, **kwargs): self.mock_dmcrypt_close_uuid.append(kwargs['mapping']) - def test_get_source_devices(self, monkeypatch): + def test_get_source_devices(self, monkeypatch, patch_udevdata): source_tags = 'ceph.osd_id=2,ceph.type=data,ceph.osd_fsid=1234' source_db_tags = 'ceph.osd_id=2,ceph.type=db,ceph.osd_fsid=1234' @@ -1228,7 +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): + def test_migrate_data_db_to_new_db(self, m_zap, is_root, monkeypatch, patch_udevdata): monkeypatch.setattr('ceph_volume.util.device.Device.is_lv', lambda: True) source_tags = 'ceph.osd_id=2,ceph.type=data,ceph.osd_fsid=1234,' \ @@ -1331,7 +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): + def test_migrate_data_db_to_new_db_encrypted(self, m_getuid, m_zap, monkeypatch, patch_udevdata): monkeypatch.setattr('ceph_volume.util.device.Device.is_lv', lambda: True) m_getuid.return_value = 0 @@ -1447,7 +1447,7 @@ Example calls for supported scenarios: m_zap.assert_called_once() - def test_migrate_data_db_to_new_db_active_systemd(self, is_root, monkeypatch, capsys): + def test_migrate_data_db_to_new_db_active_systemd(self, is_root, monkeypatch, capsys, patch_udevdata): 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,' \ @@ -1511,7 +1511,7 @@ Example calls for supported scenarios: assert not stdout @patch.object(Zap, 'main') - def test_migrate_data_db_to_new_db_no_systemd(self, m_zap, is_root, monkeypatch): + def test_migrate_data_db_to_new_db_no_systemd(self, m_zap, is_root, monkeypatch, patch_udevdata): 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' @@ -1611,7 +1611,7 @@ Example calls for supported scenarios: m_zap.assert_called_once() @patch.object(Zap, 'main') - def test_migrate_data_db_to_new_db_skip_wal(self, m_zap, is_root, monkeypatch): + def test_migrate_data_db_to_new_db_skip_wal(self, m_zap, is_root, monkeypatch, patch_udevdata): 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' @@ -1734,7 +1734,7 @@ Example calls for supported scenarios: m_zap.assert_called_once() @patch.object(Zap, 'main') - def test_migrate_data_db_wal_to_new_db(self, m_zap, is_root, monkeypatch): + def test_migrate_data_db_wal_to_new_db(self, m_zap, is_root, monkeypatch, patch_udevdata): 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,' \ @@ -1863,7 +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): + def test_migrate_data_db_wal_to_new_db_encrypted(self, m_getuid, m_zap, monkeypatch, patch_udevdata): monkeypatch.setattr('ceph_volume.util.device.Device.is_lv', lambda: True) m_getuid.return_value = 0 @@ -2010,7 +2010,8 @@ Example calls for supported scenarios: def test_dont_migrate_data_db_wal_to_new_data(self, m_getuid, monkeypatch, - capsys): + capsys, + patch_udevdata): m_getuid.return_value = 0 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' @@ -2076,7 +2077,8 @@ Example calls for supported scenarios: def test_dont_migrate_db_to_wal(self, is_root, monkeypatch, - capsys): + capsys, + patch_udevdata): 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' @@ -2149,7 +2151,8 @@ Example calls for supported scenarios: def test_migrate_data_db_to_db(self, is_root, monkeypatch, - capsys): + capsys, + patch_udevdata): 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,' \ @@ -2226,7 +2229,7 @@ Example calls for supported scenarios: '--command', 'bluefs-bdev-migrate', '--devs-source', '/var/lib/ceph/osd/ceph-2/block'] - def test_migrate_data_db_to_db_active_systemd(self, is_root, monkeypatch, capsys): + def test_migrate_data_db_to_db_active_systemd(self, is_root, monkeypatch, capsys, patch_udevdata): 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,7 +2300,7 @@ Example calls for supported scenarios: assert '--> OSD is running, stop it with: systemctl stop ceph-osd@2' == stderr.rstrip() assert not stdout - def test_migrate_data_db_to_db_no_systemd(self, is_root, monkeypatch): + def test_migrate_data_db_to_db_no_systemd(self, is_root, monkeypatch, patch_udevdata): 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,' \ @@ -2377,7 +2380,8 @@ Example calls for supported scenarios: m_zap, is_root, monkeypatch, - capsys): + capsys, + patch_udevdata): 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,' \ @@ -2485,7 +2489,8 @@ Example calls for supported scenarios: m_getuid, m_zap, monkeypatch, - capsys): + capsys, + patch_udevdata): monkeypatch.setattr('ceph_volume.util.device.Device.is_lv', lambda: True) m_getuid.return_value = 0 @@ -2577,7 +2582,8 @@ Example calls for supported scenarios: m_getuid, m_zap, monkeypatch, - capsys): + capsys, + patch_udevdata): monkeypatch.setattr('ceph_volume.util.device.Device.is_lv', lambda: True) m_getuid.return_value = 0 @@ -2690,7 +2696,7 @@ Example calls for supported scenarios: m_zap.assert_called_once() - def test_migrate_data_wal_to_db_active_systemd(self, is_root, monkeypatch, capsys): + def test_migrate_data_wal_to_db_active_systemd(self, is_root, monkeypatch, capsys, patch_udevdata): 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' @@ -2763,7 +2769,7 @@ Example calls for supported scenarios: assert not stdout @patch.object(Zap, 'main') - def test_migrate_data_wal_to_db_no_systemd(self, m_zap, is_root, monkeypatch): + def test_migrate_data_wal_to_db_no_systemd(self, m_zap, is_root, monkeypatch, patch_udevdata): 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,' \ diff --git a/src/ceph-volume/ceph_volume/tests/devices/lvm/test_zap.py b/src/ceph-volume/ceph_volume/tests/devices/lvm/test_zap.py index a6747f7cca2..c16b88337b4 100644 --- a/src/ceph-volume/ceph_volume/tests/devices/lvm/test_zap.py +++ b/src/ceph-volume/ceph_volume/tests/devices/lvm/test_zap.py @@ -12,8 +12,6 @@ from typing import Tuple, List def process_call(command, **kw): result: Tuple[List[str], List[str], int] = '' - if 'udevadm' in command: - result = data_zap.udevadm_property, [], 0 if 'ceph-bluestore-tool' in command: result = data_zap.ceph_bluestore_tool_output, [], 0 if 'is-active' in command: @@ -115,7 +113,7 @@ class TestZap: @patch('ceph_volume.devices.lvm.zap.Zap.zap') @patch('ceph_volume.process.call', Mock(side_effect=process_call)) - def test_lv_is_matched_id(self, mock_zap, monkeypatch, is_root): + def test_lv_is_matched_id(self, mock_zap, monkeypatch, is_root, patch_udevdata): tags = 'ceph.osd_id=0,ceph.journal_uuid=x,ceph.type=data' osd = api.Volume(lv_name='volume1', lv_uuid='y', vg_name='', lv_path='/dev/VolGroup/lv', lv_tags=tags) @@ -131,7 +129,7 @@ class TestZap: @patch('ceph_volume.devices.lvm.zap.Zap.zap') @patch('ceph_volume.devices.raw.list.List.filter_lvm_osd_devices', Mock(return_value='/dev/sdb')) @patch('ceph_volume.process.call', Mock(side_effect=process_call)) - def test_raw_is_matched_id(self, mock_zap, monkeypatch, is_root): + def test_raw_is_matched_id(self, mock_zap, monkeypatch, is_root, patch_udevdata): volumes = [] monkeypatch.setattr(zap.api, 'get_lvs', lambda **kw: volumes) @@ -141,7 +139,7 @@ class TestZap: mock_zap.assert_called_once() @patch('ceph_volume.devices.lvm.zap.Zap.zap') - def test_lv_is_matched_fsid(self, mock_zap, monkeypatch, is_root): + def test_lv_is_matched_fsid(self, mock_zap, monkeypatch, is_root, patch_udevdata): tags = 'ceph.osd_id=0,ceph.osd_fsid=asdf-lkjh,ceph.journal_uuid=x,' +\ 'ceph.type=data' osd = api.Volume(lv_name='volume1', lv_uuid='y', vg_name='', @@ -159,7 +157,7 @@ class TestZap: @patch('ceph_volume.devices.lvm.zap.Zap.zap') @patch('ceph_volume.devices.raw.list.List.filter_lvm_osd_devices', Mock(return_value='/dev/sdb')) @patch('ceph_volume.process.call', Mock(side_effect=process_call)) - def test_raw_is_matched_fsid(self, mock_zap, monkeypatch, is_root): + def test_raw_is_matched_fsid(self, mock_zap, monkeypatch, is_root, patch_udevdata): volumes = [] monkeypatch.setattr(zap.api, 'get_lvs', lambda **kw: volumes) @@ -170,7 +168,7 @@ class TestZap: mock_zap.assert_called_once @patch('ceph_volume.devices.lvm.zap.Zap.zap') - def test_lv_is_matched_id_fsid(self, mock_zap, monkeypatch, is_root): + def test_lv_is_matched_id_fsid(self, mock_zap, monkeypatch, is_root, patch_udevdata): tags = 'ceph.osd_id=0,ceph.osd_fsid=asdf-lkjh,ceph.journal_uuid=x,' +\ 'ceph.type=data' osd = api.Volume(lv_name='volume1', lv_uuid='y', vg_name='', @@ -189,7 +187,7 @@ class TestZap: @patch('ceph_volume.devices.lvm.zap.Zap.zap') @patch('ceph_volume.devices.raw.list.List.filter_lvm_osd_devices', Mock(return_value='/dev/sdb')) @patch('ceph_volume.process.call', Mock(side_effect=process_call)) - def test_raw_is_matched_id_fsid(self, mock_zap, monkeypatch, is_root): + def test_raw_is_matched_id_fsid(self, mock_zap, monkeypatch, is_root, patch_udevdata): volumes = [] monkeypatch.setattr(zap.api, 'get_lvs', lambda **kw: volumes) @@ -202,7 +200,7 @@ class TestZap: @patch('ceph_volume.devices.lvm.zap.Zap.zap') @patch('ceph_volume.devices.raw.list.List.filter_lvm_osd_devices', Mock(side_effect=['/dev/vdx', '/dev/vdy', '/dev/vdz', None])) @patch('ceph_volume.process.call', Mock(side_effect=process_call)) - def test_raw_multiple_devices(self, mock_zap, monkeypatch, is_root): + def test_raw_multiple_devices(self, mock_zap, monkeypatch, is_root, patch_udevdata): volumes = [] monkeypatch.setattr(zap.api, 'get_lvs', lambda **kw: volumes) z = zap.Zap(['--osd-id', '5']) diff --git a/src/ceph-volume/ceph_volume/tests/objectstore/test_lvmbluestore.py b/src/ceph-volume/ceph_volume/tests/objectstore/test_lvmbluestore.py index 35dd4b680ee..ba80e680b2d 100644 --- a/src/ceph-volume/ceph_volume/tests/objectstore/test_lvmbluestore.py +++ b/src/ceph-volume/ceph_volume/tests/objectstore/test_lvmbluestore.py @@ -419,7 +419,7 @@ class TestLvmBlueStore: @pytest.mark.parametrize("encrypted", ["ceph.encrypted=0", "ceph.encrypted=1"]) def test__activate(self, m_success, m_create_osd_path, - monkeypatch, fake_run, fake_call, encrypted, conf_ceph_stub): + monkeypatch, fake_run, fake_call, encrypted, conf_ceph_stub, patch_udevdata): conf_ceph_stub('[global]\nfsid=asdf-lkjh') monkeypatch.setattr(system, 'chown', lambda path: 0) monkeypatch.setattr('ceph_volume.configuration.load', lambda: None) diff --git a/src/ceph-volume/ceph_volume/tests/test_inventory.py b/src/ceph-volume/ceph_volume/tests/test_inventory.py index 9bb7776c175..98c6e29c2c2 100644 --- a/src/ceph-volume/ceph_volume/tests/test_inventory.py +++ b/src/ceph-volume/ceph_volume/tests/test_inventory.py @@ -8,7 +8,6 @@ import ceph_volume.util.lsmdisk as lsmdisk @pytest.fixture -@patch("ceph_volume.util.disk.has_bluestore_label", lambda x: False) def device_report_keys(device_info): device_info(devices={ # example output of disk.get_devices() 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 cb145d10f2c..0a0571bd41a 100644 --- a/src/ceph-volume/ceph_volume/tests/util/test_device.py +++ b/src/ceph-volume/ceph_volume/tests/util/test_device.py @@ -202,7 +202,7 @@ class TestDevice(object): @pytest.mark.usefixtures("lsblk_ceph_disk_member", "disable_kernel_queries") @patch("ceph_volume.util.disk.has_bluestore_label", lambda x: False) - def test_is_ceph_disk_lsblk(self, fake_call, monkeypatch, patch_bluestore_label): + def test_is_ceph_disk_lsblk(self, patch_udevdata, fake_call, monkeypatch, patch_bluestore_label): disk = device.Device("/dev/sda") assert disk.is_ceph_disk_member @@ -210,14 +210,14 @@ class TestDevice(object): "lsblk_ceph_disk_member", "disable_kernel_queries") @patch("ceph_volume.util.disk.has_bluestore_label", lambda x: False) - def test_is_ceph_disk_blkid(self, fake_call, monkeypatch, patch_bluestore_label): + def test_is_ceph_disk_blkid(self, patch_udevdata, fake_call, monkeypatch, patch_bluestore_label): disk = device.Device("/dev/sda") assert disk.is_ceph_disk_member @pytest.mark.usefixtures("lsblk_ceph_disk_member", "disable_kernel_queries") @patch("ceph_volume.util.disk.has_bluestore_label", lambda x: False) - def test_is_ceph_disk_member_not_available_lsblk(self, fake_call, monkeypatch, patch_bluestore_label): + def test_is_ceph_disk_member_not_available_lsblk(self, patch_udevdata, fake_call, monkeypatch, patch_bluestore_label): disk = device.Device("/dev/sda") assert disk.is_ceph_disk_member assert not disk.available @@ -227,7 +227,7 @@ class TestDevice(object): "lsblk_ceph_disk_member", "disable_kernel_queries") @patch("ceph_volume.util.disk.has_bluestore_label", lambda x: False) - def test_is_ceph_disk_member_not_available_blkid(self, fake_call, monkeypatch, patch_bluestore_label): + def test_is_ceph_disk_member_not_available_blkid(self, patch_udevdata, fake_call, monkeypatch, patch_bluestore_label): disk = device.Device("/dev/sda") assert disk.is_ceph_disk_member assert not disk.available @@ -366,7 +366,7 @@ class TestDevice(object): "device_info_not_ceph_disk_member", "disable_kernel_queries") @patch("ceph_volume.util.disk.has_bluestore_label", lambda x: False) - def test_is_not_ceph_disk_member_lsblk(self, fake_call, patch_bluestore_label): + def test_is_not_ceph_disk_member_lsblk(self, patch_udevdata, fake_call, patch_bluestore_label): disk = device.Device("/dev/sda") assert disk.is_ceph_disk_member is False @@ -456,9 +456,8 @@ class TestDevice(object): @patch("ceph_volume.util.disk.has_bluestore_label", lambda x: False) def test_get_device_id(self, fake_call, device_info): - udev = {k:k for k in ['ID_VENDOR', 'ID_MODEL', 'ID_SCSI_SERIAL']} lsblk = {"TYPE": "disk", "NAME": "sda"} - device_info(udevadm=udev,lsblk=lsblk) + device_info(lsblk=lsblk) disk = device.Device("/dev/sda") assert disk._get_device_id() == 'ID_VENDOR_ID_MODEL_ID_SCSI_SERIAL' @@ -676,7 +675,7 @@ class TestCephDiskDevice(object): "blkid_ceph_disk_member", "disable_kernel_queries") @patch("ceph_volume.util.disk.has_bluestore_label", lambda x: False) - def test_is_member_blkid(self, fake_call, monkeypatch): + def test_is_member_blkid(self, patch_udevdata, fake_call, monkeypatch): disk = device.CephDiskDevice(device.Device("/dev/sda")) assert disk.is_member is True 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 c6244fca4d3..1f359ba8b0c 100644 --- a/src/ceph-volume/ceph_volume/tests/util/test_disk.py +++ b/src/ceph-volume/ceph_volume/tests/util/test_disk.py @@ -102,34 +102,6 @@ class TestBlkid(object): assert result['UUID'] == '62416664-cbaf-40bd-9689-10bd337379c3' assert result['TYPE'] == 'xfs' -class TestUdevadmProperty(object): - - def test_good_output(self, stub_call): - output = """ID_MODEL=SK_hynix_SC311_SATA_512GB -ID_PART_TABLE_TYPE=gpt -ID_SERIAL_SHORT=MS83N71801150416A""".split() - stub_call((output, [], 0)) - result = disk.udevadm_property('dev/sda') - assert result['ID_MODEL'] == 'SK_hynix_SC311_SATA_512GB' - assert result['ID_PART_TABLE_TYPE'] == 'gpt' - assert result['ID_SERIAL_SHORT'] == 'MS83N71801150416A' - - def test_property_filter(self, stub_call): - output = """ID_MODEL=SK_hynix_SC311_SATA_512GB -ID_PART_TABLE_TYPE=gpt -ID_SERIAL_SHORT=MS83N71801150416A""".split() - stub_call((output, [], 0)) - result = disk.udevadm_property('dev/sda', ['ID_MODEL', - 'ID_SERIAL_SHORT']) - assert result['ID_MODEL'] == 'SK_hynix_SC311_SATA_512GB' - assert 'ID_PART_TABLE_TYPE' not in result - - def test_fail_on_broken_output(self, stub_call): - output = ["ID_MODEL:SK_hynix_SC311_SATA_512GB"] - stub_call((output, [], 0)) - with pytest.raises(ValueError): - disk.udevadm_property('dev/sda') - class TestDeviceFamily(object): @@ -281,8 +253,7 @@ class TestGetDevices(object): result = disk.get_devices(_sys_block_path=str(tmpdir)) assert result == {} - @patch('ceph_volume.util.disk.udevadm_property') - def test_sda_block_is_found(self, m_udev_adm_property, patched_get_block_devs_sysfs, fake_filesystem): + def test_sda_block_is_found(self, patched_get_block_devs_sysfs, fake_filesystem): sda_path = '/dev/sda' patched_get_block_devs_sysfs.return_value = [[sda_path, sda_path, 'disk', sda_path]] result = disk.get_devices() @@ -291,8 +262,7 @@ class TestGetDevices(object): assert result[sda_path]['model'] == '' assert result[sda_path]['partitions'] == {} - @patch('ceph_volume.util.disk.udevadm_property') - def test_sda_size(self, m_udev_adm_property, patched_get_block_devs_sysfs, fake_filesystem): + def test_sda_size(self, patched_get_block_devs_sysfs, fake_filesystem): sda_path = '/dev/sda' patched_get_block_devs_sysfs.return_value = [[sda_path, sda_path, 'disk', sda_path]] fake_filesystem.create_file('/sys/block/sda/size', contents = '1024') @@ -300,8 +270,7 @@ class TestGetDevices(object): assert list(result.keys()) == [sda_path] assert result[sda_path]['human_readable_size'] == '512.00 KB' - @patch('ceph_volume.util.disk.udevadm_property') - def test_sda_sectorsize_fallsback(self, m_udev_adm_property, patched_get_block_devs_sysfs, fake_filesystem): + def test_sda_sectorsize_fallsback(self, patched_get_block_devs_sysfs, fake_filesystem): # if no sectorsize, it will use queue/hw_sector_size sda_path = '/dev/sda' patched_get_block_devs_sysfs.return_value = [[sda_path, sda_path, 'disk', sda_path]] @@ -310,16 +279,14 @@ class TestGetDevices(object): assert list(result.keys()) == [sda_path] assert result[sda_path]['sectorsize'] == '1024' - @patch('ceph_volume.util.disk.udevadm_property') - def test_sda_sectorsize_from_logical_block(self, m_udev_adm_property, patched_get_block_devs_sysfs, fake_filesystem): + def test_sda_sectorsize_from_logical_block(self, patched_get_block_devs_sysfs, fake_filesystem): sda_path = '/dev/sda' patched_get_block_devs_sysfs.return_value = [[sda_path, sda_path, 'disk', sda_path]] fake_filesystem.create_file('/sys/block/sda/queue/logical_block_size', contents = '99') result = disk.get_devices() assert result[sda_path]['sectorsize'] == '99' - @patch('ceph_volume.util.disk.udevadm_property') - def test_sda_sectorsize_does_not_fallback(self, m_udev_adm_property, patched_get_block_devs_sysfs, fake_filesystem): + def test_sda_sectorsize_does_not_fallback(self, patched_get_block_devs_sysfs, fake_filesystem): sda_path = '/dev/sda' patched_get_block_devs_sysfs.return_value = [[sda_path, sda_path, 'disk', sda_path]] fake_filesystem.create_file('/sys/block/sda/queue/logical_block_size', contents = '99') @@ -327,23 +294,20 @@ class TestGetDevices(object): result = disk.get_devices() assert result[sda_path]['sectorsize'] == '99' - @patch('ceph_volume.util.disk.udevadm_property') - def test_is_rotational(self, m_udev_adm_property, patched_get_block_devs_sysfs, fake_filesystem): + def test_is_rotational(self, patched_get_block_devs_sysfs, fake_filesystem): sda_path = '/dev/sda' patched_get_block_devs_sysfs.return_value = [[sda_path, sda_path, 'disk', sda_path]] fake_filesystem.create_file('/sys/block/sda/queue/rotational', contents = '1') result = disk.get_devices() assert result[sda_path]['rotational'] == '1' - @patch('ceph_volume.util.disk.udevadm_property') - def test_is_ceph_rbd(self, m_udev_adm_property, patched_get_block_devs_sysfs, fake_filesystem): + def test_is_ceph_rbd(self, patched_get_block_devs_sysfs, fake_filesystem): rbd_path = '/dev/rbd0' patched_get_block_devs_sysfs.return_value = [[rbd_path, rbd_path, 'disk', rbd_path]] result = disk.get_devices() assert rbd_path not in result - @patch('ceph_volume.util.disk.udevadm_property') - def test_actuator_device(self, m_udev_adm_property, patched_get_block_devs_sysfs, fake_filesystem): + def test_actuator_device(self, patched_get_block_devs_sysfs, fake_filesystem): sda_path = '/dev/sda' fake_actuator_nb = 2 patched_get_block_devs_sysfs.return_value = [[sda_path, sda_path, 'disk', sda_path]] diff --git a/src/ceph-volume/ceph_volume/util/device.py b/src/ceph-volume/ceph_volume/util/device.py index 7ce3011eeee..c5b35e48598 100644 --- a/src/ceph-volume/ceph_volume/util/device.py +++ b/src/ceph-volume/ceph_volume/util/device.py @@ -1,6 +1,7 @@ # -*- coding: utf-8 -*- import logging import os +import re from functools import total_ordering from ceph_volume import sys_info, allow_loop_devices, BEING_REPLACED_HEADER from ceph_volume.api import lvm @@ -9,10 +10,8 @@ from ceph_volume.util.lsmdisk import LSMDisk from ceph_volume.util.constants import ceph_disk_guids from typing import Any, Dict, List, Tuple, Optional, Union - logger = logging.getLogger(__name__) - report_template = """ {dev:<25} {size:<12} {device_nodes:<15} {rot!s:<7} {available!s:<9} {model}""" @@ -319,29 +318,33 @@ class Device(object): Please keep this implementation in sync with get_device_id() in src/common/blkdev.cc """ - props = ['ID_VENDOR', 'ID_MODEL', 'ID_MODEL_ENC', 'ID_SERIAL_SHORT', 'ID_SERIAL', - 'ID_SCSI_SERIAL'] - p = disk.udevadm_property(self.path, props) - if p.get('ID_MODEL','').startswith('LVM PV '): - p['ID_MODEL'] = p.get('ID_MODEL_ENC', '').replace('\\x20', ' ').strip() - if 'ID_VENDOR' in p and 'ID_MODEL' in p and 'ID_SCSI_SERIAL' in p: - dev_id = '_'.join([p['ID_VENDOR'], p['ID_MODEL'], - p['ID_SCSI_SERIAL']]) - elif 'ID_MODEL' in p and 'ID_SERIAL_SHORT' in p: - dev_id = '_'.join([p['ID_MODEL'], p['ID_SERIAL_SHORT']]) - elif 'ID_SERIAL' in p: - dev_id = p['ID_SERIAL'] + + udev_data = disk.UdevData(self.path) + env = udev_data.environment + parts: list[str] = [] + model = env.get('ID_MODEL', '') + if model.startswith('LVM PV '): + model = env.get('ID_MODEL_ENC', '').replace('\\x20', ' ').strip() + + if 'ID_VENDOR' in env and 'ID_SCSI_SERIAL' in env: + parts = [env['ID_VENDOR'], model, env['ID_SCSI_SERIAL']] + elif 'ID_SERIAL_SHORT' in env: + parts = [model, env['ID_SERIAL_SHORT']] + elif 'ID_SERIAL' in env: + dev_id = env['ID_SERIAL'] if dev_id.startswith('MTFD'): - # Micron NVMes hide the vendor dev_id = 'Micron_' + dev_id + parts = [dev_id] else: # the else branch should fallback to using sysfs and ioctl to # retrieve device_id on FreeBSD. Still figuring out if/how the # python ioctl implementation does that on FreeBSD dev_id = '' + + dev_id = '_'.join(parts) dev_id = dev_id.replace(' ', '_') - while '__' in dev_id: - dev_id = dev_id.replace('__', '_') + dev_id = re.sub(r'_+', '_', dev_id) + return dev_id def _set_lvm_membership(self) -> None: diff --git a/src/ceph-volume/ceph_volume/util/disk.py b/src/ceph-volume/ceph_volume/util/disk.py index 1d8867019bc..513bca3f78d 100644 --- a/src/ceph-volume/ceph_volume/util/disk.py +++ b/src/ceph-volume/ceph_volume/util/disk.py @@ -141,13 +141,14 @@ def remove_partition(device): # in the output of `udevadm info --query=property`. # Probably not ideal and not the best fix but this allows to get around that issue. # The idea is to make it retry multiple times before actually failing. - for i in range(10): - udev_info = udevadm_property(device.path) - partition_number = udev_info.get('ID_PART_ENTRY_NUMBER') + partition_number = None + for _ in range(10): + udev_data = UdevData(device.path) + partition_number = udev_data.environment.get("ID_PART_ENTRY_NUMBER", None) if partition_number: break time.sleep(0.2) - if not partition_number: + if partition_number is None: raise RuntimeError('Unable to detect the partition number for device: %s' % device.path) process.run( @@ -198,47 +199,6 @@ def device_family(device): return devices -def udevadm_property(device, properties=[]): - """ - Query udevadm for information about device properties. - Optionally pass a list of properties to return. A requested property might - not be returned if not present. - - Expected output format:: - # udevadm info --query=property --name=/dev/sda :( - DEVNAME=/dev/sda - DEVTYPE=disk - ID_ATA=1 - ID_BUS=ata - ID_MODEL=SK_hynix_SC311_SATA_512GB - ID_PART_TABLE_TYPE=gpt - ID_PART_TABLE_UUID=c8f91d57-b26c-4de1-8884-0c9541da288c - ID_PATH=pci-0000:00:17.0-ata-3 - ID_PATH_TAG=pci-0000_00_17_0-ata-3 - ID_REVISION=70000P10 - ID_SERIAL=SK_hynix_SC311_SATA_512GB_MS83N71801150416A - TAGS=:systemd: - USEC_INITIALIZED=16117769 - ... - """ - out = _udevadm_info(device) - ret = {} - for line in out: - p, v = line.split('=', 1) - if not properties or p in properties: - ret[p] = v - return ret - - -def _udevadm_info(device): - """ - Call udevadm and return the output - """ - cmd = ['udevadm', 'info', '--query=property', device] - out, _err, _rc = process.call(cmd) - return out - - def lsblk(device, columns=None, abspath=False): result = [] if not os.path.isdir(device): @@ -907,8 +867,8 @@ def get_devices(_sys_block_path='/sys/block', device=''): metadata['parent'] = block[3] # some facts from udevadm - p = udevadm_property(sysdir) - metadata['id_bus'] = p.get('ID_BUS', '') + udev_data = UdevData(sysdir) + metadata['id_bus'] = udev_data.environment.get("ID_BUS", "") device_facts[diskname] = metadata return device_facts @@ -1372,9 +1332,18 @@ class UdevData: raise RuntimeError(f'{path} not found.') self.path: str = path self.realpath: str = os.path.realpath(self.path) - self.stats: os.stat_result = os.stat(self.realpath) - self.major: int = os.major(self.stats.st_rdev) - self.minor: int = os.minor(self.stats.st_rdev) + + if path.startswith("/sys/block/") and os.path.isdir(path): + dev_file = os.path.join(path, "dev") + if not os.path.exists(dev_file): + raise RuntimeError(f"{dev_file} not found.") + with open(dev_file) as f: + self.major, self.minor = map(int, f.read().strip().split(":")) + else: + self.stats: os.stat_result = os.stat(self.realpath) + self.major: int = os.major(self.stats.st_rdev) + self.minor: int = os.minor(self.stats.st_rdev) + self.udev_data_path: str = f'/run/udev/data/b{self.major}:{self.minor}' self.symlinks: List[str] = [] self.id: str = '' -- 2.39.5