Merge pull request ceph#49954 from caffeinism/fix-bug-in-ceph-volume-device

ceph-volume: fix a bug in _check_generic_reject_reasons

Author: zmc

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

@@ -241,15 +241,15 @@ def test_is_ceph_disk_member_not_available_blkid(self, fake_call, monkeypatch, p
 
     @patch("ceph_volume.util.disk.has_bluestore_label", lambda x: False)
     def test_reject_removable_device(self, fake_call, device_info):
-        data = {"/dev/sdb": {"removable": 1}}
+        data = {"/dev/sdb": {"removable": "1"}}
         lsblk = {"TYPE": "disk", "NAME": "sdb"}
         device_info(devices=data,lsblk=lsblk)
         disk = device.Device("/dev/sdb")
         assert not disk.available
 
     @patch("ceph_volume.util.disk.has_bluestore_label", lambda x: False)
     def test_reject_device_with_gpt_headers(self, fake_call, device_info):
-        data = {"/dev/sdb": {"removable": 0, "size": 5368709120}}
+        data = {"/dev/sdb": {"removable": "0", "size": 5368709120}}
         lsblk = {"TYPE": "disk", "NAME": "sdb"}
         blkid= {"PTTYPE": "gpt"}
         device_info(
@@ -262,7 +262,7 @@ def test_reject_device_with_gpt_headers(self, fake_call, device_info):
 
     @patch("ceph_volume.util.disk.has_bluestore_label", lambda x: False)
     def test_accept_non_removable_device(self, fake_call, device_info):
-        data = {"/dev/sdb": {"removable": 0, "size": 5368709120}}
+        data = {"/dev/sdb": {"removable": "0", "size": 5368709120}}
         lsblk = {"TYPE": "disk", "NAME": "sdb"}
         device_info(devices=data,lsblk=lsblk)
         disk = device.Device("/dev/sdb")
@@ -286,7 +286,7 @@ def test_accept_symlink_to_device(self,
                                       fake_call):
         m_os_path_islink.return_value = True
         m_os_path_realpath.return_value = '/dev/sdb'
-        data = {"/dev/sdb": {"ro": 0, "size": 5368709120}}
+        data = {"/dev/sdb": {"ro": "0", "size": 5368709120}}
         lsblk = {"TYPE": "disk"}
         device_info(devices=data,lsblk=lsblk)
         disk = device.Device("/dev/test_symlink")
@@ -304,15 +304,15 @@ def test_reject_symlink_to_device_mapper(self,
                                              fake_call):
         m_os_path_islink.return_value = True
         m_os_readlink.return_value = '/dev/dm-0'
-        data = {"/dev/mapper/mpatha": {"ro": 0, "size": 5368709120}}
+        data = {"/dev/mapper/mpatha": {"ro": "0", "size": 5368709120}}
         lsblk = {"TYPE": "disk"}
         device_info(devices=data,lsblk=lsblk)
         disk = device.Device("/dev/mapper/mpatha")
         assert disk.available
 
     @patch("ceph_volume.util.disk.has_bluestore_label", lambda x: False)
     def test_reject_readonly_device(self, fake_call, device_info):
-        data = {"/dev/cdrom": {"ro": 1}}
+        data = {"/dev/cdrom": {"ro": "1"}}
         lsblk = {"TYPE": "disk", "NAME": "cdrom"}
         device_info(devices=data,lsblk=lsblk)
         disk = device.Device("/dev/cdrom")
@@ -328,7 +328,7 @@ def test_reject_smaller_than_5gb(self, fake_call, device_info):
 
     @patch("ceph_volume.util.disk.has_bluestore_label", lambda x: False)
     def test_accept_non_readonly_device(self, fake_call, device_info):
-        data = {"/dev/sda": {"ro": 0, "size": 5368709120}}
+        data = {"/dev/sda": {"ro": "0", "size": 5368709120}}
         lsblk = {"TYPE": "disk", "NAME": "sda"}
         device_info(devices=data,lsblk=lsblk)
         disk = device.Device("/dev/sda")
@@ -594,10 +594,10 @@ class TestDeviceOrdering(object):
 
     def setup_method(self):
         self.data = {
-                "/dev/sda": {"removable": 0},
-                "/dev/sdb": {"removable": 1}, # invalid
-                "/dev/sdc": {"removable": 0},
-                "/dev/sdd": {"removable": 1}, # invalid
+                "/dev/sda": {"removable": "0"},
+                "/dev/sdb": {"removable": "1"}, # invalid
+                "/dev/sdc": {"removable": "0"},
+                "/dev/sdd": {"removable": "1"}, # invalid
         }
 
     @patch("ceph_volume.util.disk.has_bluestore_label", lambda x: False)

src/ceph-volume/ceph_volume/util/device.py

@@ -594,8 +594,8 @@ def has_partitions(self):
 
     def _check_generic_reject_reasons(self):
         reasons = [
-            ('removable', 1, 'removable'),
-            ('ro', 1, 'read-only'),
+            ('id_bus', 'usb', 'id_bus'),
+            ('ro', '1', 'read-only'),
         ]
         rejected = [reason for (k, v, reason) in reasons if
                     self.sys_api.get(k, '') == v]

src/ceph-volume/ceph_volume/util/disk.py

@@ -783,8 +783,6 @@ def holder_inner_loop():
             continue
         type_ = 'disk'
         holders = os.listdir(os.path.join(_sys_block_path, dev, 'holders'))
-        if get_file_contents(os.path.join(_sys_block_path, dev, 'removable')) == "1":
-            continue
         if holder_inner_loop():
             continue
         dm_dir_path = os.path.join(_sys_block_path, dev, 'dm')
@@ -923,6 +921,10 @@ def get_devices(_sys_block_path='/sys/block', device=''):
         metadata['path'] = diskname
         metadata['type'] = block[2]
 
+        # some facts from udevadm
+        p = udevadm_property(sysdir)
+        metadata['id_bus'] = p.get('ID_BUS', '')
+
         device_facts[diskname] = metadata
     return device_facts