ceph-volume: show correct rejected reason in inventory if device type is not acceptable

If device type is not acceptable in `c-v inventory`, its rejected reason
becomes "Insufficient space (<5GB)" by mistake. It's because sys_api is
empty due to skipping devices that are neither `disk` nor `device`. We
should report the target device is not acceptable in this case.

Fixes: https://tracker.ceph.com/issues/46102
Signed-off-by: Satoru Takeuchi <satoru.takeuchi@gmail.com>
(cherry picked from commit 3e5d91d41f7275d4656019c1ca3fc80927d214c9)

diff --git a/src/ceph-volume/ceph_volume/tests/conftest.py b/src/ceph-volume/ceph_volume/tests/conftest.py
index 195082e7c121fb90875a3a03e1bf18058242574d..c6393cca8f6e41ed7578d733fd9b46655caf1673 100644 (file)
--- a/src/ceph-volume/ceph_volume/tests/conftest.py
+++ b/src/ceph-volume/ceph_volume/tests/conftest.py
@@ -186,18 +186,20 @@ def ceph_parttype(request):
 @pytest.fixture
 def lsblk_ceph_disk_member(monkeypatch, request, ceph_partlabel, ceph_parttype):
     monkeypatch.setattr("ceph_volume.util.device.disk.lsblk",
-                        lambda path: {'PARTLABEL': ceph_partlabel})
+                        lambda path: {'TYPE': 'disk', 'PARTLABEL': ceph_partlabel})
     # setting blkid here too in order to be able to fall back to PARTTYPE based
     # membership
     monkeypatch.setattr("ceph_volume.util.device.disk.blkid",
-                        lambda path: {'PARTLABEL': '',
+                        lambda path: {'TYPE': 'disk',
+                                      'PARTLABEL': '',
                                       'PARTTYPE': ceph_parttype})
 
 
 @pytest.fixture
 def blkid_ceph_disk_member(monkeypatch, request, ceph_partlabel, ceph_parttype):
     monkeypatch.setattr("ceph_volume.util.device.disk.blkid",
-                        lambda path: {'PARTLABEL': ceph_partlabel,
+                        lambda path: {'TYPE': 'disk',
+                                      'PARTLABEL': ceph_partlabel,
                                       'PARTTYPE': ceph_parttype})
 
 
@@ -209,9 +211,11 @@ def blkid_ceph_disk_member(monkeypatch, request, ceph_partlabel, ceph_parttype):
 ])
 def device_info_not_ceph_disk_member(monkeypatch, request):
     monkeypatch.setattr("ceph_volume.util.device.disk.lsblk",
-                        lambda path: {'PARTLABEL': request.param[0]})
+                        lambda path: {'TYPE': 'disk',
+                                      'PARTLABEL': request.param[0]})
     monkeypatch.setattr("ceph_volume.util.device.disk.blkid",
-                        lambda path: {'PARTLABEL': request.param[1]})
+                        lambda path: {'TYPE': 'disk',
+                                      'PARTLABEL': request.param[1]})
 
 @pytest.fixture
 def patched_get_block_devs_lsblk():

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 69a3cb77e2bd3eb28f73bdd76815f89c877e2391..006504d02bcaba881df46a6e4f086e7ba1c56b5c 100644 (file)
--- a/src/ceph-volume/ceph_volume/tests/util/test_device.py
+++ b/src/ceph-volume/ceph_volume/tests/util/test_device.py
@@ -15,7 +15,8 @@ class TestDevice(object):
                             deepcopy(volumes))
 
         data = {"/dev/sda": {"foo": "bar"}}
-        device_info(devices=data)
+        lsblk = {"TYPE": "disk"}
+        device_info(devices=data,lsblk=lsblk)
         disk = device.Device("/dev/sda")
         assert disk.sys_api
         assert "foo" in disk.sys_api
@@ -30,20 +31,23 @@ class TestDevice(object):
 
         # 5GB in size
         data = {"/dev/sda": {"size": "5368709120"}}
-        device_info(devices=data)
+        lsblk = {"TYPE": "disk"}
+        device_info(devices=data,lsblk=lsblk)
         disk = device.Device("/dev/sda")
         assert disk.lvm_size.gb == 4
 
     def test_lvm_size_rounds_down(self, device_info):
         # 5.5GB in size
         data = {"/dev/sda": {"size": "5905580032"}}
-        device_info(devices=data)
+        lsblk = {"TYPE": "disk"}
+        device_info(devices=data,lsblk=lsblk)
         disk = device.Device("/dev/sda")
         assert disk.lvm_size.gb == 4
 
     def test_is_lv(self, device_info):
         data = {"lv_path": "vg/lv", "vg_name": "vg", "name": "lv"}
-        device_info(lv=data)
+        lsblk = {"TYPE": "lvm"}
+        device_info(lv=data,lsblk=lsblk)
         disk = device.Device("vg/lv")
         assert disk.is_lv
 
@@ -119,10 +123,10 @@ class TestDevice(object):
         assert disk.is_device is True
 
     def test_is_partition(self, device_info):
-        data = {"/dev/sda": {"foo": "bar"}}
+        data = {"/dev/sda1": {"foo": "bar"}}
         lsblk = {"TYPE": "part"}
         device_info(devices=data, lsblk=lsblk)
-        disk = device.Device("/dev/sda")
+        disk = device.Device("/dev/sda1")
         assert disk.is_partition
 
     def test_is_not_acceptable_device(self, device_info):
@@ -133,31 +137,34 @@ class TestDevice(object):
         assert not disk.is_device
 
     def test_is_not_lvm_memeber(self, device_info):
-        data = {"/dev/sda": {"foo": "bar"}}
+        data = {"/dev/sda1": {"foo": "bar"}}
         lsblk = {"TYPE": "part"}
         device_info(devices=data, lsblk=lsblk)
-        disk = device.Device("/dev/sda")
+        disk = device.Device("/dev/sda1")
         assert not disk.is_lvm_member
 
     def test_is_lvm_memeber(self, device_info):
-        data = {"/dev/sda": {"foo": "bar"}}
+        data = {"/dev/sda1": {"foo": "bar"}}
         lsblk = {"TYPE": "part"}
         device_info(devices=data, lsblk=lsblk)
-        disk = device.Device("/dev/sda")
+        disk = device.Device("/dev/sda1")
         assert not disk.is_lvm_member
 
     def test_is_mapper_device(self, device_info):
-        device_info()
+        lsblk = {"TYPE": "lvm"}
+        device_info(lsblk=lsblk)
         disk = device.Device("/dev/mapper/foo")
         assert disk.is_mapper
 
     def test_dm_is_mapper_device(self, device_info):
-        device_info()
+        lsblk = {"TYPE": "lvm"}
+        device_info(lsblk=lsblk)
         disk = device.Device("/dev/dm-4")
         assert disk.is_mapper
 
     def test_is_not_mapper_device(self, device_info):
-        device_info()
+        lsblk = {"TYPE": "disk"}
+        device_info(lsblk=lsblk)
         disk = device.Device("/dev/sda")
         assert not disk.is_mapper
 
@@ -170,8 +177,6 @@ class TestDevice(object):
     @pytest.mark.usefixtures("blkid_ceph_disk_member",
                              "disable_kernel_queries")
     def test_is_ceph_disk_blkid(self, monkeypatch, patch_bluestore_label):
-        monkeypatch.setattr("ceph_volume.util.device.disk.lsblk",
-                            lambda path: {'PARTLABEL': ""})
         disk = device.Device("/dev/sda")
         assert disk.is_ceph_disk_member
 
@@ -186,8 +191,6 @@ class TestDevice(object):
     @pytest.mark.usefixtures("blkid_ceph_disk_member",
                              "disable_kernel_queries")
     def test_is_ceph_disk_member_not_available_blkid(self, monkeypatch, patch_bluestore_label):
-        monkeypatch.setattr("ceph_volume.util.device.disk.lsblk",
-                            lambda path: {'PARTLABEL': ""})
         disk = device.Device("/dev/sda")
         assert disk.is_ceph_disk_member
         assert not disk.available
@@ -195,36 +198,50 @@ class TestDevice(object):
 
     def test_reject_removable_device(self, device_info):
         data = {"/dev/sdb": {"removable": 1}}
-        device_info(devices=data)
+        lsblk = {"TYPE": "disk"}
+        device_info(devices=data,lsblk=lsblk)
         disk = device.Device("/dev/sdb")
         assert not disk.available
 
     def test_accept_non_removable_device(self, device_info):
         data = {"/dev/sdb": {"removable": 0, "size": 5368709120}}
-        device_info(devices=data)
+        lsblk = {"TYPE": "disk"}
+        device_info(devices=data,lsblk=lsblk)
         disk = device.Device("/dev/sdb")
         assert disk.available
 
+    def test_reject_not_acceptable_device(self, device_info):
+        data = {"/dev/dm-0": {"foo": "bar"}}
+        lsblk = {"TYPE": "mpath"}
+        device_info(devices=data, lsblk=lsblk)
+        disk = device.Device("/dev/dm-0")
+        assert not disk.available
+
     def test_reject_readonly_device(self, device_info):
         data = {"/dev/cdrom": {"ro": 1}}
-        device_info(devices=data)
+        lsblk = {"TYPE": "disk"}
+        device_info(devices=data,lsblk=lsblk)
         disk = device.Device("/dev/cdrom")
         assert not disk.available
 
     def test_reject_smaller_than_5gb(self, device_info):
         data = {"/dev/sda": {"size": 5368709119}}
-        device_info(devices=data)
+        lsblk = {"TYPE": "disk"}
+        device_info(devices=data,lsblk=lsblk)
         disk = device.Device("/dev/sda")
         assert not disk.available, 'too small device is available'
 
     def test_accept_non_readonly_device(self, device_info):
         data = {"/dev/sda": {"ro": 0, "size": 5368709120}}
-        device_info(devices=data)
+        lsblk = {"TYPE": "disk"}
+        device_info(devices=data,lsblk=lsblk)
         disk = device.Device("/dev/sda")
         assert disk.available
 
-    def test_reject_bluestore_device(self, monkeypatch, patch_bluestore_label):
+    def test_reject_bluestore_device(self, monkeypatch, patch_bluestore_label, device_info):
         patch_bluestore_label.return_value = True
+        lsblk = {"TYPE": "disk"}
+        device_info(lsblk=lsblk)
         disk = device.Device("/dev/sda")
         assert not disk.available
         assert "Has BlueStore device label" in disk.rejected_reasons
@@ -314,7 +331,8 @@ class TestDevice(object):
 
     def test_get_device_id(self, device_info):
         udev = {k:k for k in ['ID_VENDOR', 'ID_MODEL', 'ID_SCSI_SERIAL']}
-        device_info(udevadm=udev)
+        lsblk = {"TYPE": "disk"}
+        device_info(udevadm=udev,lsblk=lsblk)
         disk = device.Device("/dev/sda")
         assert disk._get_device_id() == 'ID_VENDOR_ID_MODEL_ID_SCSI_SERIAL'
 
@@ -443,7 +461,8 @@ class TestDeviceOrdering(object):
         }
 
     def test_valid_before_invalid(self, device_info):
-        device_info(devices=self.data)
+        lsblk = {"TYPE": "disk"}
+        device_info(devices=self.data,lsblk=lsblk)
         sda = device.Device("/dev/sda")
         sdb = device.Device("/dev/sdb")
 
@@ -451,7 +470,8 @@ class TestDeviceOrdering(object):
         assert sdb > sda
 
     def test_valid_alphabetical_ordering(self, device_info):
-        device_info(devices=self.data)
+        lsblk = {"TYPE": "disk"}
+        device_info(devices=self.data,lsblk=lsblk)
         sda = device.Device("/dev/sda")
         sdc = device.Device("/dev/sdc")
 
@@ -459,7 +479,8 @@ class TestDeviceOrdering(object):
         assert sdc > sda
 
     def test_invalid_alphabetical_ordering(self, device_info):
-        device_info(devices=self.data)
+        lsblk = {"TYPE": "disk"}
+        device_info(devices=self.data,lsblk=lsblk)
         sdb = device.Device("/dev/sdb")
         sdd = device.Device("/dev/sdd")
 
@@ -470,16 +491,15 @@ class TestDeviceOrdering(object):
 class TestCephDiskDevice(object):
 
     def test_partlabel_lsblk(self, device_info):
-        lsblk = {"PARTLABEL": ""}
+        lsblk = {"TYPE": "disk", "PARTLABEL": ""}
         device_info(lsblk=lsblk)
         disk = device.CephDiskDevice(device.Device("/dev/sda"))
 
         assert disk.partlabel == ''
 
     def test_partlabel_blkid(self, device_info):
-        lsblk = {"PARTLABEL": ""}
-        blkid = {"PARTLABEL": "ceph data"}
-        device_info(lsblk=lsblk, blkid=blkid)
+        blkid = {"TYPE": "disk", "PARTLABEL": "ceph data"}
+        device_info(blkid=blkid)
         disk = device.CephDiskDevice(device.Device("/dev/sda"))
 
         assert disk.partlabel == 'ceph data'
@@ -487,8 +507,6 @@ class TestCephDiskDevice(object):
     @pytest.mark.usefixtures("blkid_ceph_disk_member",
                              "disable_kernel_queries")
     def test_is_member_blkid(self, monkeypatch, patch_bluestore_label):
-        monkeypatch.setattr("ceph_volume.util.device.disk.lsblk",
-                            lambda path: {'PARTLABEL': ""})
         disk = device.CephDiskDevice(device.Device("/dev/sda"))
 
         assert disk.is_member is True
@@ -501,7 +519,8 @@ class TestCephDiskDevice(object):
 
     def test_accept_non_removable_device(self, device_info):
         data = {"/dev/sdb": {"removable": 0, "size": 5368709120}}
-        device_info(devices=data)
+        lsblk = {"TYPE": "disk"}
+        device_info(devices=data,lsblk=lsblk)
         disk = device.Device("/dev/sdb")
         assert disk.available
 
@@ -519,19 +538,22 @@ class TestCephDiskDevice(object):
 
     def test_accept_non_readonly_device(self, device_info):
         data = {"/dev/sda": {"ro": 0, "size": 5368709120}}
-        device_info(devices=data)
+        lsblk = {"TYPE": "disk"}
+        device_info(devices=data,lsblk=lsblk)
         disk = device.Device("/dev/sda")
         assert disk.available
 
     @pytest.mark.usefixtures("lsblk_ceph_disk_member",
                              "disable_kernel_queries")
-    def test_is_member_lsblk(self, patch_bluestore_label):
+    def test_is_member_lsblk(self, patch_bluestore_label, device_info):
+        lsblk = {"TYPE": "disk", "PARTLABEL": "ceph"}
+        device_info(lsblk=lsblk)
         disk = device.CephDiskDevice(device.Device("/dev/sda"))
 
         assert disk.is_member is True
 
     def test_unknown_type(self, device_info):
-        lsblk = {"PARTLABEL": "gluster"}
+        lsblk = {"TYPE": "disk", "PARTLABEL": "gluster"}
         device_info(lsblk=lsblk)
         disk = device.CephDiskDevice(device.Device("/dev/sda"))
 
@@ -542,8 +564,6 @@ class TestCephDiskDevice(object):
     @pytest.mark.usefixtures("blkid_ceph_disk_member",
                              "disable_kernel_queries")
     def test_type_blkid(self, monkeypatch, device_info, ceph_partlabel):
-        monkeypatch.setattr("ceph_volume.util.device.disk.lsblk",
-                            lambda path: {'PARTLABEL': ''})
         disk = device.CephDiskDevice(device.Device("/dev/sda"))
 
         assert disk.type in self.ceph_types

diff --git a/src/ceph-volume/ceph_volume/util/device.py b/src/ceph-volume/ceph_volume/util/device.py
index 2d00f6da3e59638deda4cf214da1a75151d97672..8ea83c30e711aab058980dc15d5e5aa3453f87ef 100644 (file)
--- a/src/ceph-volume/ceph_volume/util/device.py
+++ b/src/ceph-volume/ceph_volume/util/device.py
@@ -343,17 +343,28 @@ class Device(object):
     def is_partition(self):
         if self.disk_api:
             return self.disk_api['TYPE'] == 'part'
+        elif self.blkid_api:
+            return self.blkid_api['TYPE'] == 'part'
         return False
 
     @property
     def is_device(self):
+        api = None
         if self.disk_api:
-            is_device = self.disk_api['TYPE'] == 'device'
-            is_disk = self.disk_api['TYPE'] == 'disk'
+            api = self.disk_api
+        elif self.blkid_api:
+            api = self.blkid_api
+        if api:
+            is_device = api['TYPE'] == 'device'
+            is_disk = api['TYPE'] == 'disk'
             if is_device or is_disk:
                 return True
         return False
 
+    @property
+    def is_acceptable_device(self):
+        return self.is_device or self.is_partition
+
     @property
     def is_encrypted(self):
         """
@@ -398,9 +409,12 @@ class Device(object):
         ]
         rejected = [reason for (k, v, reason) in reasons if
                     self.sys_api.get(k, '') == v]
-        # reject disks smaller than 5GB
-        if int(self.sys_api.get('size', 0)) < 5368709120:
-            rejected.append('Insufficient space (<5GB)')
+        if self.is_acceptable_device:
+            # reject disks smaller than 5GB
+            if int(self.sys_api.get('size', 0)) < 5368709120:
+                rejected.append('Insufficient space (<5GB)')
+        else:
+            rejected.append("Device type is not acceptable. It should be raw device or partition")
         if self.is_ceph_disk_member:
             rejected.append("Used by ceph-disk")
         if self.has_bluestore_label: