ceph_volume: Adding Device.is_valid()

A block device can be filtered-out/ignored because it have features that
doesn't match Ceph's expectations.

As of today, the current code was rejected removable devices but it was
pretty hidden from the user, and implicit in the get_devices() function.

This patch is creating a new is_valid() function to perform all the
rejection tests and returns if this device can be used in the Ceph
context or not.

If is_valid() is returning False, the 'rejected_reasons' list reports all
the reasons why that devices got rejected.

Signed-off-by: Erwan Velu <erwan@redhat.com>

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 369beeef3a35b1cc9937448a5374efc0e9a5d2dc..1e6766b45a3cc860b8f864cd427f16f391e1bac3 100644 (file)
--- a/src/ceph-volume/ceph_volume/tests/util/test_device.py
+++ b/src/ceph-volume/ceph_volume/tests/util/test_device.py
@@ -150,6 +150,18 @@ class TestCephDiskDevice(object):
 
         assert disk.is_member is True
 
+    def test_reject_removable_device(self, device_info):
+        data = {"/dev/sdb": {"removable": 1}}
+        device_info(devices=data)
+        disk = device.Device("/dev/sdb")
+        assert not disk.is_valid
+
+    def test_accept_non_removable_device(self, device_info):
+        data = {"/dev/sdb": {"removable": 0}}
+        device_info(devices=data)
+        disk = device.Device("/dev/sdb")
+        assert disk.is_valid
+
     @pytest.mark.parametrize("label", ceph_partlabels)
     def test_is_member_lsblk(self, device_info, label):
         lsblk = {"PARTLABEL": label}

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 fd3839307e86a76208a56ff09c77f4075773ebbf..5d1bd82b60217b30a4d04e4c7c1cc2344d97d977 100644 (file)
--- a/src/ceph-volume/ceph_volume/tests/util/test_disk.py
+++ b/src/ceph-volume/ceph_volume/tests/util/test_disk.py
@@ -220,19 +220,6 @@ class TestGetDevices(object):
         assert result[dev_sda_path]['model'] == ''
         assert result[dev_sda_path]['partitions'] == {}
 
-    def test_sda_is_removable_gets_skipped(self, tmpfile, tmpdir):
-        block_path, dev_path, mapper_path = self.setup_paths(tmpdir)
-        dev_sda_path = os.path.join(dev_path, 'sda')
-        block_sda_path = os.path.join(block_path, 'sda')
-        os.makedirs(block_sda_path)
-        os.makedirs(dev_sda_path)
-
-        tmpfile('removable', contents='1', directory=block_sda_path)
-        result = disk.get_devices(
-            _sys_block_path=block_path,
-            _dev_path=dev_path,
-            _mapper_path=mapper_path)
-        assert result == {}
 
     def test_dm_device_is_not_used(self, monkeypatch, tmpdir):
         # the link to the mapper is used instead

diff --git a/src/ceph-volume/ceph_volume/util/device.py b/src/ceph-volume/ceph_volume/util/device.py
index 0977cd0189d52916a2f5047455dedbdd02b8c1bc..a705678157f372f9427cb53f030fabb19fa83b6f 100644 (file)
--- a/src/ceph-volume/ceph_volume/util/device.py
+++ b/src/ceph-volume/ceph_volume/util/device.py
@@ -19,6 +19,8 @@ class Device(object):
         self.sys_api = {}
         self._exists = None
         self._is_lvm_member = None
+        self._valid = False
+        self._rejected_reasons = []
         self._parse()
 
     def _parse(self):
@@ -130,6 +132,20 @@ class Device(object):
         return any(osd_ids)
 
 
+    @property
+    def is_valid(self):
+        def reject_device(item, value, reason):
+            try:
+                if self.sys_api[item] == value:
+                    self._rejected_reasons.append(reason)
+            except KeyError:
+                pass
+        reject_device('removable', 1, 'removable')
+
+        self._valid = len(self._rejected_reasons) == 0
+        return self._valid
+
+
 class CephDiskDevice(object):
     """
     Detect devices that have been created by ceph-disk, report their type

diff --git a/src/ceph-volume/ceph_volume/util/disk.py b/src/ceph-volume/ceph_volume/util/disk.py
index 3c5f4abecf46a8ac825869158c3911a45e03bb2d..dd7c62426104e22bac5ac0a1d11597de8fc9476c 100644 (file)
--- a/src/ceph-volume/ceph_volume/util/disk.py
+++ b/src/ceph-volume/ceph_volume/util/disk.py
@@ -694,10 +694,7 @@ def get_devices(_sys_block_path='/sys/block', _dev_path='/dev', _mapper_path='/d
             if lvm.is_lv(diskname):
                 continue
 
-        # If the device reports itself as 'removable', get it excluded
         metadata['removable'] = get_file_contents(os.path.join(sysdir, 'removable'))
-        if metadata['removable'] == '1':
-            continue
 
         for key in ['vendor', 'model', 'sas_address', 'sas_device_handle']:
             metadata[key] = get_file_contents(sysdir + "/device/" + key)