From: Jan Fajerski Date: Wed, 28 Nov 2018 11:20:20 +0000 (+0100) Subject: ceph-volume batch: refactor filtered_devices and strategy retrieval X-Git-Tag: v14.1.0~243^2~8 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=dc2c4c597249d61ea6f48d37dbe02d8b97e3d55d;p=ceph.git ceph-volume batch: refactor filtered_devices and strategy retrieval Keep filtered_devices as a member variable of the batch object and pass this to the report functionality instead of having the report functions retrieve filtered_devices. Also keep selected strategy as a batch member variable to avoid re-retrival in e.g. the report methods. Signed-off-by: Jan Fajerski --- diff --git a/src/ceph-volume/ceph_volume/devices/lvm/batch.py b/src/ceph-volume/ceph_volume/devices/lvm/batch.py index 4d14181c37e9..2da6aab81661 100644 --- a/src/ceph-volume/ceph_volume/devices/lvm/batch.py +++ b/src/ceph-volume/ceph_volume/devices/lvm/batch.py @@ -98,10 +98,10 @@ def filter_devices(args): unused_devices = [device for device in args.devices if not device.used_by_ceph] # only data devices, journals can be reused used_devices = [device.abspath for device in args.devices if device.used_by_ceph] - args.filtered_devices = {} + filtered_devices = {} if used_devices: for device in used_devices: - args.filtered_devices[device] = {"reasons": ["Used by ceph as a data device already"]} + filtered_devices[device] = {"reasons": ["Used by ceph as a data device already"]} logger.info("Ignoring devices already used by ceph: %s" % ", ".join(used_devices)) if len(unused_devices) == 1: last_device = unused_devices[0] @@ -109,11 +109,11 @@ def filter_devices(args): reason = "Used by ceph as a %s already and there are no devices left for data/block" % ( last_device.lvs[0].tags.get("ceph.type"), ) - args.filtered_devices[last_device.abspath] = {"reasons": [reason]} + filtered_devices[last_device.abspath] = {"reasons": [reason]} logger.info(reason + ": %s" % last_device.abspath) unused_devices = [] - return unused_devices + return unused_devices, filtered_devices class Batch(object): @@ -165,7 +165,7 @@ class Batch(object): help='Devices to provision OSDs wal volumes', ) parser.add_argument( - '--db-devices', + '--journal-devices', nargs='*', type=arg_validators.ValidDevice(), default=[], @@ -241,6 +241,7 @@ class Batch(object): help='Only prepare all OSDs, do not activate', ) self.args = parser.parse_args(argv) + self.parser = parser def get_devices(self): # remove devices with partitions @@ -255,29 +256,27 @@ class Batch(object): ) def report(self): - strategy = self._get_strategy() if self.args.format == 'pretty': - strategy.report_pretty() + self.strategy.report_pretty(self.filtered_devices) elif self.args.format == 'json': - strategy.report_json() + self.strategy.report_json(self.filtered_devices) else: raise RuntimeError('report format must be "pretty" or "json"') def execute(self): - strategy = self._get_strategy() if not self.args.yes: - strategy.report_pretty() + self.strategy.report_pretty(self.filtered_devices) terminal.info('The above OSDs would be created if the operation continues') if not prompt_bool('do you want to proceed? (yes/no)'): devices = ','.join([device.abspath for device in self.args.devices]) terminal.error('aborting OSD provisioning for %s' % devices) raise SystemExit(0) - strategy.execute() + self.strategy.execute() def _get_strategy(self): - strategy = get_strategy(args, self.args.devices) - unused_devices = filter_devices(self.args) + strategy = get_strategy(self.args, self.args.devices) + unused_devices, self.filtered_devices = filter_devices(self.args) if not unused_devices and not self.args.format == 'json': # report nothing changed mlogger.info("All devices are already used by ceph. No OSDs will be created.") @@ -288,12 +287,12 @@ class Batch(object): mlogger.error("Aborting because strategy changed from %s to %s after filtering" % (strategy.type(), new_strategy.type())) raise SystemExit(1) - return strategy(unused_devices, self.args) + self.strategy = strategy.with_auto_devices(unused_devices, self.args) @decorators.needs_root def main(self): if not self.args.devices: - return parser.print_help() + return self.parser.print_help() # Default to bluestore here since defaulting it in add_argument may # cause both to be True @@ -303,12 +302,56 @@ class Batch(object): if (self.args.no_auto or self.args.db_devices or self.args.journal_devices or self.args.wal_devices): - self.get_explicit_strategy() + self._get_explicit_strategy() else: - if self.args.report: - self.report() - else: - self.execute() + self._get_strategy() - def get_explicit_strategy(self): - raise NotImplementedError() + if self.args.report: + self.report() + else: + self.execute() + + def _get_explicit_strategy(self): + # TODO assert that none of the device lists overlap? + self._filter_devices() + if self.args.bluestore: + if self.db_usable: + self.strategy = strategies.bluestore.MixedType( + self.usable, + self.db_usable, + [], + self.args) + else: + self.strategy = strategies.bluestore.SingleType( + self.usable, + self.db_usable, + [], + self.args) + else: + if self.journal_usable: + self.strategy = strategies.filestore.MixedType( + self.usable, + self.journal_usable, + self.args) + else: + self.strategy = strategies.filestore.SingleType( + self.usable, + self.journal_usable, + self.args) + + + def _filter_devices(self): + # filter devices by their available property. + # TODO: Some devices are rejected in the argparser already. maybe it + # makes sense to unifiy this + used_reason = {"reasons": ["Used by ceph as a data device already"]} + self.filtered_devices = {} + for dev_list in ['', 'db_', 'wal_', 'journal_']: + dev_list_prop = '{}devices'.format(dev_list) + if hasattr(self.args, dev_list_prop): + usable_dev_list_prop = '{}usable'.format(dev_list) + 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) + if d.used_by_ceph}) 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 f9d8b8095141..f10ffd6e0fbe 100644 --- a/src/ceph-volume/ceph_volume/devices/lvm/strategies/bluestore.py +++ b/src/ceph-volume/ceph_volume/devices/lvm/strategies/bluestore.py @@ -15,23 +15,23 @@ class SingleType(Strategy): Support for all SSDs, or all HDDS """ - def __init__(self, block_devs, db_devs, wal_devs, args): - super(SingleType, self).__init__(block_devs, db_devs, wal_devs, args) + def __init__(self, block_devs, args): + super(SingleType, self).__init__(block_devs, [], [], args) self.validate_compute() @classmethod def with_auto_devices(cls, devices, args): - block_devs, wal_devs = cls.split_devices_rotational(devices) - return cls(block_devs, wal_devs, [], args) + #SingleType only deploys standalone OSDs + return cls(devices, args) @staticmethod def type(): return "bluestore.SingleType" - def report_pretty(self): + def report_pretty(self, filtered_devices): string = "" - if self.args.filtered_devices: - string += templates.filtered_devices(self.args.filtered_devices) + if filtered_devices: + string += templates.filtered_devices(filtered_devices) string += templates.total_osds.format( total_osds=self.total_osds, ) @@ -55,7 +55,7 @@ class SingleType(Strategy): """ # validate minimum size for all devices validators.minimum_device_size( - self.devices, osds_per_device=self.osds_per_device + self.block_devs, osds_per_device=self.osds_per_device ) # make sure that data devices do not have any LVs @@ -68,20 +68,8 @@ class SingleType(Strategy): """ osds = self.computed['osds'] for device in self.block_devs: - for hdd in range(self.osds_per_device): - osd = {'data': {}, 'block.db': {}} - osd['data']['path'] = device.abspath - osd['data']['size'] = device.lvm_size.b / self.osds_per_device - osd['data']['parts'] = self.osds_per_device - osd['data']['percentage'] = 100 / self.osds_per_device - osd['data']['human_readable_size'] = str( - disk.Size(b=device.lvm_size.b) / self.osds_per_device - ) - osds.append(osd) - - for device in self.wal_devs: extents = lvm.sizing(device.lvm_size.b, parts=self.osds_per_device) - for ssd in range(self.osds_per_device): + for _i in range(self.osds_per_device): osd = {'data': {}, 'block.db': {}} osd['data']['path'] = device.abspath osd['data']['size'] = extents['sizes'] @@ -152,13 +140,13 @@ class MixedType(MixedStrategy): else: return prepare.get_block_db_size(lv_format=False) or disk.Size(b=0) - def report_pretty(self): + def report_pretty(self, filtered_devices): vg_extents = lvm.sizing(self.total_available_db_space.b, parts=self.dbs_needed) db_size = str(disk.Size(b=(vg_extents['sizes']))) string = "" - if self.args.filtered_devices: - string += templates.filtered_devices(self.args.filtered_devices) + if filtered_devices: + string += templates.filtered_devices(filtered_devices) string += templates.total_osds.format( total_osds=len(self.block_devs) * self.osds_per_device ) diff --git a/src/ceph-volume/ceph_volume/devices/lvm/strategies/filestore.py b/src/ceph-volume/ceph_volume/devices/lvm/strategies/filestore.py index 5cede7e10bbd..681f55d2cdd4 100644 --- a/src/ceph-volume/ceph_volume/devices/lvm/strategies/filestore.py +++ b/src/ceph-volume/ceph_volume/devices/lvm/strategies/filestore.py @@ -28,24 +28,23 @@ class SingleType(Strategy): """ - def __init__(self, block_devs, db_devs, args): - super(SingleType, self).__init__(block_devs, db_devs, [], args) + def __init__(self, block_devs, args): + super(SingleType, self).__init__(block_devs, [], [], args) self.journal_size = get_journal_size(args) self.validate_compute() @classmethod def with_auto_devices(cls, devices, args): - block_devs, wal_devs = cls.split_devices_rotational(devices) - return cls(block_devs, wal_devs, args) + return cls(devices, args) @staticmethod def type(): return "filestore.SingleType" - def report_pretty(self): + def report_pretty(self, filtered_devices): string = "" - if self.args.filtered_devices: - string += templates.filtered_devices(self.args.filtered_devices) + if filtered_devices: + string += templates.filtered_devices(filtered_devices) string += templates.total_osds.format( total_osds=self.total_osds ) @@ -74,17 +73,12 @@ class SingleType(Strategy): met, raise an error if the provided devices would not work """ # validate minimum size for all devices - validators.minimum_device_size(self.devices, osds_per_device=self.osds_per_device) + validators.minimum_device_size(self.block_devs, osds_per_device=self.osds_per_device) # validate collocation - if self.block_devs: - validators.minimum_device_collocated_size( - self.block_devs, self.journal_size, osds_per_device=self.osds_per_device - ) - else: - validators.minimum_device_collocated_size( - self.wal_devs, self.journal_size, osds_per_device=self.osds_per_device - ) + validators.minimum_device_collocated_size( + self.block_devs, self.journal_size, osds_per_device=self.osds_per_device + ) # make sure that data devices do not have any LVs validators.no_lvm_membership(self.block_devs) @@ -95,9 +89,8 @@ class SingleType(Strategy): a dictionary with the result """ # chose whichever is the one group we have to compute against - devices = self.block_devs or self.wal_devs osds = self.computed['osds'] - for device in devices: + for device in self.block_devs: for osd in range(self.osds_per_device): device_size = disk.Size(b=device.lvm_size.b) osd_size = device_size / self.osds_per_device @@ -174,8 +167,8 @@ class MixedType(MixedStrategy): """ - def __init__(self, block_devs, db_devs, args): - super(MixedType, self).__init__(block_devs, db_devs, [], args) + def __init__(self, block_devs, journal_devs, args): + super(MixedType, self).__init__(block_devs, journal_devs, [], args) self.blank_ssds = [] self.journals_needed = len(self.block_devs) * self.osds_per_device self.journal_size = get_journal_size(args) @@ -184,17 +177,17 @@ class MixedType(MixedStrategy): @classmethod def with_auto_devices(cls, devices, args): - block_devs, db_devs = cls.split_devices_rotational(devices) - return cls(block_devs, db_devs, args) + block_devs, journal_devs = cls.split_devices_rotational(devices) + return cls(block_devs, journal_devs, args) @staticmethod def type(): return "filestore.MixedType" - def report_pretty(self): + def report_pretty(self, filtered_devices): string = "" - if self.args.filtered_devices: - string += templates.filtered_devices(self.args.filtered_devices) + if filtered_devices: + string += templates.filtered_devices(filtered_devices) string += templates.total_osds.format( total_osds=self.total_osds ) diff --git a/src/ceph-volume/ceph_volume/devices/lvm/strategies/strategies.py b/src/ceph-volume/ceph_volume/devices/lvm/strategies/strategies.py index 4c5dd9d4999c..272eb107d7e9 100644 --- a/src/ceph-volume/ceph_volume/devices/lvm/strategies/strategies.py +++ b/src/ceph-volume/ceph_volume/devices/lvm/strategies/strategies.py @@ -8,7 +8,8 @@ class Strategy(object): self.devices = block_devs + wal_devs + db_devs self.block_devs = block_devs self.db_devs = db_devs - self.computed = {'osds': [], 'vgs': [], 'filtered_devices': args.filtered_devices} + self.wal_devs = wal_devs + self.computed = {'osds': [], 'vgs': []} @staticmethod def split_devices_rotational(devices): @@ -24,15 +25,15 @@ class Strategy(object): else: self.computed["changed"] = False - def report_json(self): + def report_json(self, filtered_devices): + # add filtered devices to report + report = self.computed.copy() + report['filtered_devices'] = filtered_devices print(json.dumps(self.computed, indent=4, sort_keys=True)) @property def total_osds(self): - if self.block_devs: - return len(self.block_devs) * self.osds_per_device - else: - return len(self.wal_devs) * self.osds_per_device + return len(self.block_devs) * self.osds_per_device # protect against base class instantiation and incomplete implementations. # We could also use the abc module and implement this as an 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 d0e8586d3033..7ad77ab1ef09 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 @@ -61,9 +61,9 @@ class TestFilterDevices(object): def test_filter_used_device(self, factory): device1 = factory(used_by_ceph=True, abspath="/dev/sda") args = factory(devices=[device1], filtered_devices={}) - result = batch.filter_devices(args) + result, filtered_devices = batch.filter_devices(args) assert not result - assert device1.abspath in args.filtered_devices + assert device1.abspath in filtered_devices def test_has_unused_devices(self, factory): device1 = factory( @@ -73,9 +73,9 @@ class TestFilterDevices(object): is_lvm_member=False ) args = factory(devices=[device1], filtered_devices={}) - result = batch.filter_devices(args) + result, filtered_devices = batch.filter_devices(args) assert device1 in result - assert not args.filtered_devices + assert not filtered_devices def test_filter_device_used_as_a_journal(self, factory): hdd1 = factory( @@ -93,9 +93,9 @@ class TestFilterDevices(object): lvs=[lv], ) args = factory(devices=[hdd1, ssd1], filtered_devices={}) - result = batch.filter_devices(args) + result, filtered_devices = batch.filter_devices(args) assert not result - assert ssd1.abspath in args.filtered_devices + assert ssd1.abspath in filtered_devices def test_last_device_is_not_filtered(self, factory): hdd1 = factory( @@ -111,6 +111,6 @@ class TestFilterDevices(object): is_lvm_member=False, ) args = factory(devices=[hdd1, ssd1], filtered_devices={}) - result = batch.filter_devices(args) + result, filtered_devices = batch.filter_devices(args) assert result - assert len(args.filtered_devices) == 1 + assert len(filtered_devices) == 1