From a86e07fabddeb2a331b798f91ea5be12f6ceae7f Mon Sep 17 00:00:00 2001 From: Jan Fajerski Date: Thu, 15 Aug 2019 12:20:00 +0200 Subject: [PATCH] ceph-volume: don't keep device lists as sets This was introduced by #27754. The explicit device lists were cast to sets but other parts of the code where not updated accordingly. To avoid touching all code places, only cast to sets for disjoint test and keep lists otherwise. Fixes: https://tracker.ceph.com/issues/41292 Signed-off-by: Jan Fajerski (cherry picked from commit 0534cf188a671096d5ddb9d48cdae3dccc6c0b18) --- src/ceph-volume/ceph_volume/devices/lvm/batch.py | 14 +++++++------- .../tests/devices/lvm/strategies/test_filestore.py | 9 --------- .../ceph_volume/tests/devices/lvm/test_batch.py | 3 ++- 3 files changed, 9 insertions(+), 17 deletions(-) diff --git a/src/ceph-volume/ceph_volume/devices/lvm/batch.py b/src/ceph-volume/ceph_volume/devices/lvm/batch.py index 223aa4ac99ccb..8685f588c61e0 100644 --- a/src/ceph-volume/ceph_volume/devices/lvm/batch.py +++ b/src/ceph-volume/ceph_volume/devices/lvm/batch.py @@ -254,7 +254,7 @@ class Batch(object): self.args = parser.parse_args(argv) self.parser = parser for dev_list in ['', 'db_', 'wal_', 'journal_']: - setattr(self, '{}usable'.format(dev_list), set()) + setattr(self, '{}usable'.format(dev_list), []) def get_devices(self): # remove devices with partitions @@ -361,8 +361,8 @@ class Batch(object): dev_list_prop = '{}devices'.format(dev_list) if hasattr(self.args, dev_list_prop): usable_dev_list_prop = '{}usable'.format(dev_list) - usable = set([d for d in getattr(self.args, dev_list_prop) if - d.available]) + usable = [d for d in getattr(self.args, dev_list_prop) if + d.available] setattr(self, usable_dev_list_prop, usable) self.filtered_devices.update({d: used_reason for d in getattr(self.args, dev_list_prop) @@ -370,8 +370,8 @@ class Batch(object): def _ensure_disjoint_device_lists(self): # check that all device lists are disjoint with each other - if not(self.usable.isdisjoint(self.db_usable) and - self.usable.isdisjoint(self.wal_usable) and - self.usable.isdisjoint(self.journal_usable) and - self.db_usable.isdisjoint(self.wal_usable)): + if not(set(self.usable).isdisjoint(set(self.db_usable)) and + set(self.usable).isdisjoint(set(self.wal_usable)) and + set(self.usable).isdisjoint(set(self.journal_usable)) and + set(self.db_usable).isdisjoint(set(self.wal_usable))): raise Exception('Device lists are not disjoint') diff --git a/src/ceph-volume/ceph_volume/tests/devices/lvm/strategies/test_filestore.py b/src/ceph-volume/ceph_volume/tests/devices/lvm/strategies/test_filestore.py index 9fb8149509603..bc9afb7068192 100644 --- a/src/ceph-volume/ceph_volume/tests/devices/lvm/strategies/test_filestore.py +++ b/src/ceph-volume/ceph_volume/tests/devices/lvm/strategies/test_filestore.py @@ -223,12 +223,3 @@ class TestMixedType(object): filestore.MixedType.with_auto_devices(args, devices) msg = "Not enough space in fast devices (14.97 GB) to create 2 x 14.77 GB journal LV" assert msg in str(error.value) - - def test_filter_all_data_devs(self, fakedevice, factory): - # in this scenario the user passed a already used device to be used for - # data and an unused device to be used as db device. - db_dev = fakedevice(used_by_ceph=False, is_lvm_member=False, rotational=False, sys_api=dict(size=6073740000)) - data_dev = fakedevice(used_by_ceph=True, is_lvm_member=False, rotational=True, sys_api=dict(size=6073740000)) - args = factory(filtered_devices=[data_dev], osds_per_device=1, - journal_size=None, osd_ids=[]) - filestore.MixedType(args, [], [db_dev]) diff --git a/src/ceph-volume/ceph_volume/tests/devices/lvm/test_batch.py b/src/ceph-volume/ceph_volume/tests/devices/lvm/test_batch.py index f03110289844e..c6b3f3b6d4dfe 100644 --- a/src/ceph-volume/ceph_volume/tests/devices/lvm/test_batch.py +++ b/src/ceph-volume/ceph_volume/tests/devices/lvm/test_batch.py @@ -63,8 +63,9 @@ class TestBatch(object): b.args.devices = [device1, device2] b.args.db_devices = [device2] b._filter_devices() - with pytest.raises(Exception): + with pytest.raises(Exception) as disjoint_ex: b._ensure_disjoint_device_lists() + assert 'Device lists are not disjoint' in str(disjoint_ex.value) class TestFilterDevices(object): -- 2.39.5