From c2e5f523cee728e8f74223c0b6b0e04beec17146 Mon Sep 17 00:00:00 2001 From: Erwan Velu Date: Tue, 9 Oct 2018 21:04:02 +0200 Subject: [PATCH] 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 (cherry picked from commit d8fdf0b7532cdffcae56511c377679e51841caec) --- .../ceph_volume/tests/util/test_device.py | 12 ++++++++++++ .../ceph_volume/tests/util/test_disk.py | 13 ------------- src/ceph-volume/ceph_volume/util/device.py | 16 ++++++++++++++++ src/ceph-volume/ceph_volume/util/disk.py | 3 --- 4 files changed, 28 insertions(+), 16 deletions(-) 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 369beeef3a35b..1e6766b45a3cc 100644 --- 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 fd3839307e86a..5d1bd82b60217 100644 --- 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 0977cd0189d52..a705678157f37 100644 --- 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 2682dfbef3f06..ae1da7095e68f 100644 --- 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) -- 2.39.5