Skip to content

Commit

Permalink
ceph-volume: show correct rejected reason in inventory if device type…
Browse files Browse the repository at this point in the history 
… 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 3e5d91d)
  • Loading branch information
@satoru-takeuchi
satoru-takeuchi authored and Jan Fajerski committed Sep 10, 2020
1 parent 85f0260 commit 210ad89
Show file tree
Hide file tree
Showing 3 changed files with 87 additions and 49 deletions.
14 changes: 9 additions & 5 deletions src/ceph-volume/ceph_volume/tests/conftest.py
View file
Expand Up @@ -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})


Expand All @@ -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():
Expand Down
98 changes: 59 additions & 39 deletions src/ceph-volume/ceph_volume/tests/util/test_device.py
View file
Expand Up @@ -15,7 +15,8 @@ def test_sys_api(self, monkeypatch, device_info):
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
Expand All @@ -30,20 +31,23 @@ def test_lvm_size(self, monkeypatch, 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

Expand Down Expand Up @@ -119,10 +123,10 @@ def test_disk_is_device(self, device_info):
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):
Expand All @@ -133,31 +137,34 @@ def test_is_not_acceptable_device(self, device_info):
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

Expand All @@ -170,8 +177,6 @@ def test_is_ceph_disk_lsblk(self, monkeypatch, patch_bluestore_label):
@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

Expand All @@ -186,45 +191,57 @@ def test_is_ceph_disk_member_not_available_lsblk(self, monkeypatch, patch_bluest
@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
assert "Used by ceph-disk" in disk.rejected_reasons

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
Expand Down Expand Up @@ -314,7 +331,8 @@ def test_not_used_by_ceph(self, device_info, monkeypatch):

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'

Expand Down Expand Up @@ -443,23 +461,26 @@ def setup(self):
}

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")

assert sda < sdb
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")

assert sda < sdc
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")

Expand All @@ -470,25 +491,22 @@ def test_invalid_alphabetical_ordering(self, device_info):
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'

@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
Expand All @@ -501,7 +519,8 @@ def test_reject_removable_device(self, device_info):

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

Expand All @@ -519,19 +538,22 @@ def test_reject_smaller_than_5gb(self, device_info):

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"))

Expand All @@ -542,8 +564,6 @@ def test_unknown_type(self, device_info):
@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
Expand Down
24 changes: 19 additions & 5 deletions src/ceph-volume/ceph_volume/util/device.py
View file
Expand Up @@ -343,17 +343,28 @@ def is_lv(self):
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):
"""
Expand Down Expand Up @@ -398,9 +409,12 @@ def _check_generic_reject_reasons(self):
]
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:
Expand Down

0 comments on commit 210ad89

Please sign in to comment.