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 74985e253a1a67642a977a63d0344a73094dfe8d..32da08447f4343d53322606dea8049c197d6ff18 100644 (file)
--- a/src/ceph-volume/ceph_volume/tests/conftest.py
+++ b/src/ceph-volume/ceph_volume/tests/conftest.py
@@ -239,18 +239,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})
 
 
@@ -262,9 +264,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 a8bd07db90501cd199a078ab6eb0eca8158cc3e9..82267fd9339ea6549f47fe19a714ccc682def0fd 100644 (file)
--- a/src/ceph-volume/ceph_volume/tests/util/test_device.py
+++ b/src/ceph-volume/ceph_volume/tests/util/test_device.py
@@ -7,7 +7,8 @@ class TestDevice(object):
 
     def test_sys_api(self, device_info):
         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
@@ -15,20 +16,23 @@ class TestDevice(object):
     def test_lvm_size(self, device_info):
         # 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
 
@@ -101,38 +105,48 @@ class TestDevice(object):
         assert disk.is_device is True
 
     def test_is_partition(self, device_info, pvolumes):
-        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):
+        data = {"/dev/dm-0": {"foo": "bar"}}
+        lsblk = {"TYPE": "mpath"}
+        device_info(devices=data, lsblk=lsblk)
+        disk = device.Device("/dev/dm-0")
+        assert not disk.is_device
+
     def test_is_not_lvm_memeber(self, device_info, pvolumes):
-        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, pvolumes):
-        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
 
@@ -147,8 +161,6 @@ class TestDevice(object):
                              "disable_kernel_queries",
                              "disable_lvm_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
 
@@ -165,8 +177,6 @@ class TestDevice(object):
                              "disable_kernel_queries",
                              "disable_lvm_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
@@ -174,36 +184,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
@@ -281,7 +305,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'
 
@@ -410,7 +435,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")
 
@@ -418,7 +444,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")
 
@@ -426,7 +453,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")
 
@@ -437,16 +465,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'
@@ -455,8 +482,6 @@ class TestCephDiskDevice(object):
                              "disable_kernel_queries",
                              "disable_lvm_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
@@ -464,13 +489,15 @@ class TestCephDiskDevice(object):
     @pytest.mark.usefixtures("lsblk_ceph_disk_member",
                              "disable_kernel_queries",
                              "disable_lvm_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"))
 
@@ -482,8 +509,6 @@ class TestCephDiskDevice(object):
                              "disable_kernel_queries",
                              "disable_lvm_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 f0c46a49b495f061c1648cdad4ebf67c97139d0c..83a2e717cf8939fc4071e828dac0e5fd4173b8c8 100644 (file)
--- a/src/ceph-volume/ceph_volume/util/device.py
+++ b/src/ceph-volume/ceph_volume/util/device.py
@@ -333,17 +333,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):
         """
@@ -388,9 +399,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: