From: Satoru Takeuchi Date: Fri, 22 May 2020 01:45:32 +0000 (+0000) Subject: ceph-volume: show correct rejected reason in inventory if device type is not acceptable X-Git-Url: http://git.apps.os.sepia.ceph.com/?a=commitdiff_plain;h=6d80ff650fdc73f5a5cd2c6d5a2e402eb3c34f9f;p=ceph.git 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 (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 74985e253a1a6..32da08447f434 100644 --- 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 a8bd07db90501..82267fd9339ea 100644 --- 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 f0c46a49b495f..83a2e717cf893 100644 --- 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: