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 <jfajerski@suse.com>
 Conflicts:
	src/ceph-volume/ceph_volume/tests/devices/lvm/strategies/test_filestore.py
        Pick test addition and error-value change

diff --git a/src/ceph-volume/ceph_volume/devices/lvm/batch.py b/src/ceph-volume/ceph_volume/devices/lvm/batch.py
index a7d2cbea84969849765c076dade07119b1154ba7..18c947766c8ac9528bd598ffd389e190e611471d 100644 (file)
--- 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 07116661c57f79a55d446209947bdf17b57dd818..db3eff7ea3e8b0ccf5521a65cecfae8b34ec0684 100644 (file)
--- 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 9988956049bd73213ce8ba67aae57e3d62a62a10..9ee9fe4d949ec42df8dc704aec45583c27ec8bc2 100644 (file)
--- 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
@@ -50,6 +50,19 @@ class TestSingleType(object):
         assert 'Unable to use device, already a member of LVM' in str(error.value)
 
 
+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 5a012ffd6ac195ad1a2348b0091c9b730e9993b8..3cf7d2a35dbfed42960ac8d16bc6e75e3c4b7c55 100644 (file)
--- 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.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, 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])