Merge pull request ceph#65029 from guits/drop-udevadm-subprocess-calls

ceph-volume: drop udevadm subprocess calls

Reviewed-by: Adam King <[email protected]>

Author: adk3798

src/ceph-volume/ceph_volume/tests/conftest.py

@@ -289,7 +289,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=[
@@ -357,17 +356,22 @@ 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)]
         else:
             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)
@@ -378,7 +382,6 @@ def apply(devices=None, lsblk=None, lv=None, blkid=None, udevadm=None,
                                 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])
@@ -387,12 +390,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

src/ceph-volume/ceph_volume/tests/devices/lvm/data_zap.py

@@ -65,17 +65,3 @@
              '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')

src/ceph-volume/ceph_volume/tests/devices/lvm/test_migrate.py

@@ -12,8 +12,6 @@
 
 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:
@@ -120,7 +118,7 @@ def test_no_ceph_lvs_and_no_ceph_raw_found(self, is_root, monkeypatch):
 
     @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)
@@ -137,7 +135,7 @@ def test_lv_is_matched_id(self, mock_zap, monkeypatch, is_root):
     @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)
 
@@ -147,7 +145,7 @@ def test_raw_is_matched_id(self, mock_zap, monkeypatch, is_root):
         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='',
@@ -166,7 +164,7 @@ def test_lv_is_matched_fsid(self, mock_zap, monkeypatch, is_root):
     @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)
 
@@ -177,7 +175,7 @@ def test_raw_is_matched_fsid(self, mock_zap, monkeypatch, is_root):
         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='',
@@ -197,7 +195,7 @@ def test_lv_is_matched_id_fsid(self, mock_zap, monkeypatch, is_root):
     @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)
 
@@ -211,7 +209,7 @@ def test_raw_is_matched_id_fsid(self, mock_zap, monkeypatch, is_root):
     @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'])

src/ceph-volume/ceph_volume/tests/devices/lvm/test_zap.py

@@ -450,7 +450,7 @@ def test__activate_raises_exception(self):
     @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)

src/ceph-volume/ceph_volume/tests/objectstore/test_lvm.py

@@ -8,7 +8,6 @@
 
 
 @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()

src/ceph-volume/ceph_volume/tests/test_inventory.py

@@ -202,22 +202,22 @@ def test_is_not_mapper_device(self, fake_call, device_info):
     @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
 
     @pytest.mark.usefixtures("blkid_ceph_disk_member",
                              "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 @@ def test_is_ceph_disk_member_not_available_lsblk(self, fake_call, monkeypatch, p
                              "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 @@ def test_reject_device_with_oserror(self, fake_call, monkeypatch, patch_bluestor
                              "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 @@ def test_not_used_by_ceph(self, fake_call, device_info, monkeypatch):
 
     @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 @@ def test_partlabel_blkid(self, fake_call, device_info):
                              "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

src/ceph-volume/ceph_volume/tests/util/test_device.py

@@ -102,34 +102,6 @@ def test_parses_translated(self, stub_call):
         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 @@ def test_no_devices_are_found(self, tmpdir, patched_get_block_devs_sysfs):
         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,17 +262,15 @@ def test_sda_block_is_found(self, m_udev_adm_property, patched_get_block_devs_sy
         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')
         result = disk.get_devices()
         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,40 +279,35 @@ def test_sda_sectorsize_fallsback(self, m_udev_adm_property, patched_get_block_d
         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')
         fake_filesystem.create_file('/sys/block/sda/queue/hw_sector_size', contents = '1024')
         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]]