From 026d5b80fd01cf01ad5add867188a3fef129bec2 Mon Sep 17 00:00:00 2001 From: Jan Fajerski Date: Sat, 26 Jan 2019 11:08:01 +0100 Subject: [PATCH] ceph-volume: don't crash on unusable data devices Also add test case for a MixedStrategy with unusable data devices. Signed-off-by: Jan Fajerski --- src/ceph-volume/ceph_volume/devices/lvm/batch.py | 2 ++ .../devices/lvm/strategies/bluestore.py | 15 ++++++++------- .../devices/lvm/strategies/test_bluestore.py | 13 +++++++++++++ .../devices/lvm/strategies/test_filestore.py | 9 +++++++++ 4 files changed, 32 insertions(+), 7 deletions(-) diff --git a/src/ceph-volume/ceph_volume/devices/lvm/batch.py b/src/ceph-volume/ceph_volume/devices/lvm/batch.py index a7d2cbea8496..18c947766c8a 100644 --- a/src/ceph-volume/ceph_volume/devices/lvm/batch.py +++ b/src/ceph-volume/ceph_volume/devices/lvm/batch.py @@ -253,6 +253,8 @@ 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), []) def get_devices(self): # remove devices with partitions diff --git a/src/ceph-volume/ceph_volume/devices/lvm/strategies/bluestore.py b/src/ceph-volume/ceph_volume/devices/lvm/strategies/bluestore.py index 07116661c57f..db3eff7ea3e8 100644 --- a/src/ceph-volume/ceph_volume/devices/lvm/strategies/bluestore.py +++ b/src/ceph-volume/ceph_volume/devices/lvm/strategies/bluestore.py @@ -7,7 +7,6 @@ from .strategies import MixedStrategy from ceph_volume.devices.lvm.create import Create from ceph_volume.devices.lvm.prepare import Prepare from ceph_volume.util import templates -from ceph_volume.util.prepare import osd_id_available from ceph_volume.exceptions import SizeAllocationError @@ -132,6 +131,8 @@ class MixedType(MixedStrategy): self.block_db_size = self.get_block_db_size() self.block_wal_size = self.get_block_wal_size() self.system_vgs = lvm.VolumeGroups() + self.common_vg = None + self.common_wal_vg = None self.dbs_needed = len(self.data_devs) * self.osds_per_device self.wals_needed = self.dbs_needed self.use_large_block_db = self.use_large_block_wal = False @@ -227,7 +228,7 @@ class MixedType(MixedStrategy): def compute(self): osds = self.computed['osds'] - if self.db_or_journal_devs: + if self.data_devs and self.db_or_journal_devs: if not self.common_vg: # there isn't a common vg, so a new one must be created with all # the blank db devs @@ -244,7 +245,7 @@ class MixedType(MixedStrategy): else: vg_name = self.common_vg.name - if self.wal_devs: + if self.data_devs and self.wal_devs: if not self.common_wal_vg: # there isn't a common vg, so a new one must be created with all # the blank wal devs @@ -308,7 +309,7 @@ class MixedType(MixedStrategy): vg = lvm.create_vg(osd['data']['path'], name_prefix='ceph-block') data_vgs[osd['data']['path']] = vg - if self.db_or_journal_devs: + if self.data_devs and self.db_or_journal_devs: blank_db_dev_paths = [d.abspath for d in self.blank_db_devs] # no common vg is found, create one with all the blank SSDs @@ -330,7 +331,7 @@ class MixedType(MixedStrategy): else: db_lv_extents = db_vg.sizing(size=self.block_db_size.gb.as_int())['extents'] - if self.wal_devs: + if self.data_devs and self.wal_devs: blank_wal_dev_paths = [d.abspath for d in self.blank_wal_devs] if not self.common_wal_vg: @@ -405,10 +406,10 @@ class MixedType(MixedStrategy): # make sure that data devices do not have any LVs validators.no_lvm_membership(self.data_devs) - if self.db_or_journal_devs: + if self.data_devs and self.db_or_journal_devs: self._validate_db_devs() - if self.wal_devs: + if self.data_devs and self.wal_devs: self._validate_wal_devs() if self.osd_ids: diff --git a/src/ceph-volume/ceph_volume/tests/devices/lvm/strategies/test_bluestore.py b/src/ceph-volume/ceph_volume/tests/devices/lvm/strategies/test_bluestore.py index 4f19405a7623..ef752c131ab8 100644 --- a/src/ceph-volume/ceph_volume/tests/devices/lvm/strategies/test_bluestore.py +++ b/src/ceph-volume/ceph_volume/tests/devices/lvm/strategies/test_bluestore.py @@ -49,6 +49,19 @@ class TestSingleType(object): assert 'Unable to use device, already a member of LVM' in str(error) +class TestMixedType(object): + + 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, sys_api=dict(rotational='0', size=6073740000)) + data_dev = fakedevice(used_by_ceph=True, is_lvm_member=False, sys_api=dict(rotational='1', size=6073740000)) + args = factory(filtered_devices=[data_dev], osds_per_device=1, + block_db_size=None, block_wal_size=None, + osd_ids=[]) + bluestore.MixedType(args, [], [db_dev], []) + + class TestMixedTypeConfiguredSize(object): # uses a block.db size that has been configured via ceph.conf, instead of # defaulting to 'as large as possible' 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 02891cc607a7..b9e928bfa447 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,3 +223,12 @@ 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) + + 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, sys_api=dict(rotational='0', size=6073740000)) + data_dev = fakedevice(used_by_ceph=True, is_lvm_member=False, sys_api=dict(rotational='1', size=6073740000)) + args = factory(filtered_devices=[data_dev], osds_per_device=1, + journal_size=None, osd_ids=[]) + filestore.MixedType(args, [], [db_dev]) -- 2.47.3