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 267c6bf33a6a23b7069b08cfc1e687cec18ef2d2..f20f8ddb7107e6ef02128b613459b73de01af2ba 100644 (file)
--- a/src/ceph-volume/ceph_volume/util/device.py
+++ b/src/ceph-volume/ceph_volume/util/device.py
@@ -593,8 +593,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 4ea21ebdfdb2a07cf2d1398630fce1a886cf4c69..2dfa84df0fed13353b8c8c2be203c77107faaf81 100644 (file)
--- a/src/ceph-volume/ceph_volume/util/disk.py
+++ b/src/ceph-volume/ceph_volume/util/disk.py
@@ -783,8 +783,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')