ceph-volume: fix a bug in _check_generic_reject_reasons

The types of removable and ro are wrong. Here, both filters are not
working at all. Changed this from integer to string and corrected the test
data.

Delete redundant logic in get_block_devs_sysfs. Given the name of the
function, I think it is correct to judge from _check_generic_reject_reasons,
and in fact it was before v17.2.4.

Fixes: https://tracker.ceph.com/issues/58591
Signed-off-by: Kim Minjong <make.dirty.code@gmail.com>
(cherry picked from commit a78e660728c6c0442cdbfa65db776b5856aee933)

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 8eef3ff00c1d9ce94258b1762360caa9f0bb58e5..e991b935ab6e7370a8058b9386a30fc7f03db5bd 100644 (file)
--- a/src/ceph-volume/ceph_volume/tests/util/test_device.py
+++ b/src/ceph-volume/ceph_volume/tests/util/test_device.py
@@ -241,7 +241,7 @@ class TestDevice(object):
 
     @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")
@@ -249,7 +249,7 @@ class TestDevice(object):
 
     @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 @@ class TestDevice(object):
 
     @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 @@ class TestDevice(object):
                                       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,7 +304,7 @@ class TestDevice(object):
                                              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")
@@ -312,7 +312,7 @@ class TestDevice(object):
 
     @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 @@ class TestDevice(object):
 
     @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(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)

diff --git a/src/ceph-volume/ceph_volume/util/device.py b/src/ceph-volume/ceph_volume/util/device.py
index 4eaff6e67e73cc00cec9933a7f987b2c6114726c..14fe573622e69c5c94e95c6dec41360d1da62823 100644 (file)
--- a/src/ceph-volume/ceph_volume/util/device.py
+++ b/src/ceph-volume/ceph_volume/util/device.py
@@ -582,8 +582,8 @@ class Device(object):
 
     def _check_generic_reject_reasons(self):
         reasons = [
-            ('removable', 1, 'removable'),
-            ('ro', 1, 'read-only'),
+            ('removable', '1', 'removable'),
+            ('ro', '1', 'read-only'),
         ]
         rejected = [reason for (k, v, reason) in reasons if
                     self.sys_api.get(k, '') == v]

diff --git a/src/ceph-volume/ceph_volume/util/disk.py b/src/ceph-volume/ceph_volume/util/disk.py
index 548de795cd6f858f190e1db30c8351a73787b501..418bfe16188eb2d5c085d1ee17cbb7bde6e9cf6f 100644 (file)
--- a/src/ceph-volume/ceph_volume/util/disk.py
+++ b/src/ceph-volume/ceph_volume/util/disk.py
@@ -779,8 +779,6 @@ def get_block_devs_sysfs(_sys_block_path='/sys/block', _sys_dev_block_path='/sys
             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')