From 8770fdaee361e38791f4980101a73db8e03eaca7 Mon Sep 17 00:00:00 2001 From: Guillaume Abrioux Date: Tue, 9 May 2023 18:49:10 +0200 Subject: [PATCH] Revert "ceph-volume: assign seastore as object store backend when deplying crimson-osd using LVM with cephadm" Signed-off-by: Guillaume Abrioux --- src/ceph-volume/ceph_volume/activate/main.py | 12 ---- .../ceph_volume/devices/lvm/activate.py | 52 --------------- .../ceph_volume/devices/lvm/batch.py | 10 +-- .../ceph_volume/devices/lvm/common.py | 13 +--- .../ceph_volume/devices/lvm/create.py | 2 +- .../ceph_volume/devices/lvm/prepare.py | 57 +---------------- src/ceph-volume/ceph_volume/util/prepare.py | 63 ------------------- src/cephadm/cephadm.py | 9 --- src/pybind/mgr/cephadm/serve.py | 27 ++------ src/pybind/mgr/cephadm/tests/test_cephadm.py | 2 +- .../ceph/deployment/drive_group.py | 4 +- .../ceph/deployment/translate.py | 5 -- 12 files changed, 13 insertions(+), 243 deletions(-) diff --git a/src/ceph-volume/ceph_volume/activate/main.py b/src/ceph-volume/ceph_volume/activate/main.py index e13def81d21b9..1cef038b62fe7 100644 --- a/src/ceph-volume/ceph_volume/activate/main.py +++ b/src/ceph-volume/ceph_volume/activate/main.py @@ -29,16 +29,6 @@ class Activate(object): '--osd-uuid', help='OSD UUID to activate' ) - parser.add_argument( - '--bluestore', - action='store_true', - help='force bluestore objectstore activation', - ) - parser.add_argument( - '--seastore', - action='store_true', - help='force seastore objectstore activation', - ) parser.add_argument( '--no-systemd', dest='no_systemd', @@ -71,8 +61,6 @@ class Activate(object): argparse.Namespace( osd_id=self.args.osd_id, osd_fsid=self.args.osd_uuid, - seastore=self.args.seastore, - bluestore=self.args.bluestore, no_tmpfs=self.args.no_tmpfs, no_systemd=self.args.no_systemd, ) diff --git a/src/ceph-volume/ceph_volume/devices/lvm/activate.py b/src/ceph-volume/ceph_volume/devices/lvm/activate.py index aec269c036913..53ed6aa47918e 100644 --- a/src/ceph-volume/ceph_volume/devices/lvm/activate.py +++ b/src/ceph-volume/ceph_volume/devices/lvm/activate.py @@ -218,51 +218,6 @@ def activate_bluestore(osd_lvs, no_systemd=False, no_tmpfs=False): systemctl.start_osd(osd_id) terminal.success("ceph-volume lvm activate successful for osd ID: %s" % osd_id) -def activate_seastore(osd_lvs, no_systemd=False, no_tmpfs=False): - for lv in osd_lvs: - if lv.tags.get('ceph.type') == 'block': - osd_block_lv = lv - break - else: - raise RuntimeError('could not find a seastore OSD to activate') - - is_encrypted = osd_block_lv.tags.get('ceph.encrypted', '0') == '1' - dmcrypt_secret = None - osd_id = osd_block_lv.tags['ceph.osd_id'] - conf.cluster = osd_block_lv.tags['ceph.cluster_name'] - osd_fsid = osd_block_lv.tags['ceph.osd_fsid'] - configuration.load_ceph_conf_path(osd_block_lv.tags['ceph.cluster_name']) - configuration.load() - - # mount on tmpfs the osd directory - osd_path = '/var/lib/ceph/osd/%s-%s' % (conf.cluster, osd_id) - if not system.path_is_mounted(osd_path): - # mkdir -p and mount as tmpfs - prepare_utils.create_osd_path(osd_id, tmpfs=not no_tmpfs) - # encryption is handled here, before priming the OSD dir - if is_encrypted: - osd_lv_path = '/dev/mapper/%s' % osd_block_lv.lv_uuid - lockbox_secret = osd_block_lv.tags['ceph.cephx_lockbox_secret'] - encryption_utils.write_lockbox_keyring(osd_id, osd_fsid, lockbox_secret) - dmcrypt_secret = encryption_utils.get_dmcrypt_key(osd_id, osd_fsid) - encryption_utils.luks_open(dmcrypt_secret, osd_block_lv.lv_path, osd_block_lv.lv_uuid) - else: - osd_lv_path = osd_block_lv.lv_path - - # always re-do the symlink regardless if it exists, so that the block - # devices that may have changed can be mapped correctly every time - process.run(['ln', '-snf', osd_lv_path, os.path.join(osd_path, 'block')]) - system.chown(os.path.join(osd_path, 'block')) - if no_systemd is False: - # enable the ceph-volume unit for this OSD - systemctl.enable_volume(osd_id, osd_fsid, 'lvm') - - # enable the OSD - systemctl.enable_osd(osd_id) - - # start the OSD - systemctl.start_osd(osd_id) - terminal.success("ceph-volume lvm activate successful for osd ID: %s" % osd_id) class Activate(object): @@ -342,8 +297,6 @@ class Activate(object): # explicit filestore/bluestore flags take precedence if getattr(args, 'bluestore', False): activate_bluestore(lvs, args.no_systemd, getattr(args, 'no_tmpfs', False)) - elif getattr(args, 'seastore', False): - activate_seastore(lvs, args.no_systemd, getattr(args, 'no_tmpfs', False)) elif getattr(args, 'filestore', False): activate_filestore(lvs, args.no_systemd) elif any('ceph.block_device' in lv.tags for lv in lvs): @@ -395,11 +348,6 @@ class Activate(object): action='store_true', help='force bluestore objectstore activation', ) - parser.add_argument( - '--seastore', - action='store_true', - help='force seastore objectstore activation', - ) parser.add_argument( '--filestore', action='store_true', diff --git a/src/ceph-volume/ceph_volume/devices/lvm/batch.py b/src/ceph-volume/ceph_volume/devices/lvm/batch.py index 2511d3a611b9e..9ed2bf2fccf43 100644 --- a/src/ceph-volume/ceph_volume/devices/lvm/batch.py +++ b/src/ceph-volume/ceph_volume/devices/lvm/batch.py @@ -246,11 +246,6 @@ class Batch(object): action='store_true', help='bluestore objectstore (default)', ) - parser.add_argument( - '--seastore', - action='store_true', - help='seastore objectstore (defualt)', - ) parser.add_argument( '--filestore', action='store_true', @@ -425,7 +420,7 @@ class Batch(object): # Default to bluestore here since defaulting it in add_argument may # cause both to be True - if not self.args.bluestore and not self.args.filestore and not self.args.seastore: + if not self.args.bluestore and not self.args.filestore: self.args.bluestore = True if (self.args.auto and not self.args.db_devices and not @@ -458,7 +453,6 @@ class Batch(object): defaults = common.get_default_args() global_args = [ 'bluestore', - 'seastore', 'filestore', 'dmcrypt', 'crush_device_class', @@ -479,8 +473,6 @@ class Batch(object): if args.bluestore: plan = self.get_deployment_layout(args, args.devices, args.db_devices, args.wal_devices) - elif args.seastore: - plan = self.get_deployment_layout(args, args.devices) elif args.filestore: plan = self.get_deployment_layout(args, args.devices, args.journal_devices) return plan diff --git a/src/ceph-volume/ceph_volume/devices/lvm/common.py b/src/ceph-volume/ceph_volume/devices/lvm/common.py index 4b1762e229586..edc8e1cbce117 100644 --- a/src/ceph-volume/ceph_volume/devices/lvm/common.py +++ b/src/ceph-volume/ceph_volume/devices/lvm/common.py @@ -126,13 +126,6 @@ bluestore_args = { }, } -seastore_args = { - '--seastore': { - 'action': 'store_true', - 'help': 'Use the seastore objectstore', - }, -} - filestore_args = { '--filestore': { 'action': 'store_true', @@ -159,7 +152,7 @@ def get_default_args(): defaults = {} def format_name(name): return name.strip('-').replace('-', '_').replace('.', '_') - for argset in (common_args, filestore_args, bluestore_args, seastore_args): + for argset in (common_args, filestore_args, bluestore_args): defaults.update({format_name(name): val.get('default', None) for name, val in argset.items()}) return defaults @@ -177,7 +170,6 @@ def common_parser(prog, description): filestore_group = parser.add_argument_group('filestore') bluestore_group = parser.add_argument_group('bluestore') - seastore_group = parser.add_argument_group('seastore') for name, kwargs in common_args.items(): parser.add_argument(name, **kwargs) @@ -188,9 +180,6 @@ def common_parser(prog, description): for name, kwargs in filestore_args.items(): filestore_group.add_argument(name, **kwargs) - for name, kwargs in seastore_args.items(): - seastore_group.add_argument(name, **kwargs) - # Do not parse args, so that consumers can do something before the args get # parsed triggering argparse behavior return parser diff --git a/src/ceph-volume/ceph_volume/devices/lvm/create.py b/src/ceph-volume/ceph_volume/devices/lvm/create.py index 29175fbfc6735..af2cd96c0845e 100644 --- a/src/ceph-volume/ceph_volume/devices/lvm/create.py +++ b/src/ceph-volume/ceph_volume/devices/lvm/create.py @@ -68,7 +68,7 @@ class Create(object): if len(self.argv) == 0: print(sub_command_help) return - exclude_group_options(parser, groups=['filestore', 'bluestore', 'seastore'], argv=self.argv) + exclude_group_options(parser, groups=['filestore', 'bluestore'], argv=self.argv) args = parser.parse_args(self.argv) # Default to bluestore here since defaulting it in add_argument may # cause both to be True diff --git a/src/ceph-volume/ceph_volume/devices/lvm/prepare.py b/src/ceph-volume/ceph_volume/devices/lvm/prepare.py index 3c53399cd68ac..2f715fdba122c 100644 --- a/src/ceph-volume/ceph_volume/devices/lvm/prepare.py +++ b/src/ceph-volume/ceph_volume/devices/lvm/prepare.py @@ -119,33 +119,6 @@ def prepare_bluestore(block, wal, db, secrets, tags, osd_id, fsid): db=db ) -def prepare_seastore(block, secrets, tags, osd_id, fsid): - """ - :param block: The name of the logical volume for the seastore data - :param secrets: A dict with the secrets needed to create the osd (e.g. cephx) - :param id_: The OSD id - :param fsid: The OSD fsid, also known as the OSD UUID - """ - cephx_secret = secrets.get('cephx_secret', prepare_utils.create_key()) - # encryption-only operations - if secrets.get('dmcrypt_key'): - key = secrets['dmcrypt_key'] - block = prepare_dmcrypt(key, block, 'block', tags) - - # create the directory - prepare_utils.create_osd_path(osd_id, tmpfs=True) - # symlink the block - prepare_utils.link_block(block, osd_id) - # get the latest monmap - prepare_utils.get_monmap(osd_id) - # write the OSD keyring if it doesn't exist already - prepare_utils.write_keyring(osd_id, cephx_secret) - # prepare the osd filesystem - prepare_utils.osd_mkfs_seastore( - osd_id, fsid, - keyring=cephx_secret, - ) - class Prepare(object): @@ -420,32 +393,6 @@ class Prepare(object): self.osd_id, osd_fsid, ) - elif self.args.seastore: - try: - vg_name, lv_name = self.args.data.split('/') - block_lv = api.get_single_lv(filters={'lv_name': lv_name, - 'vg_name': vg_name}) - except ValueError: - block_lv = None - - if not block_lv: - block_lv = self.prepare_data_device('block', osd_fsid) - - tags['ceph.block_device'] = block_lv.lv_path - tags['ceph.block_uuid'] = block_lv.lv_uuid - tags['ceph.cephx_lockbox_secret'] = cephx_lockbox_secret - tags['ceph.encrypted'] = encrypted - tags['ceph.vdo'] = api.is_vdo(block_lv.lv_path) - tags['ceph.type'] = 'block' - block_lv.set_tags(tags) - - prepare_seastore( - block_lv.lv_path, - secrets, - tags, - self.osd_id, - osd_fsid, - ) def main(self): sub_command_help = dedent(""" @@ -480,7 +427,7 @@ class Prepare(object): if len(self.argv) == 0: print(sub_command_help) return - exclude_group_options(parser, argv=self.argv, groups=['filestore', 'bluestore', 'seastore']) + exclude_group_options(parser, argv=self.argv, groups=['filestore', 'bluestore']) self.args = parser.parse_args(self.argv) # the unfortunate mix of one superset for both filestore and bluestore # makes this validation cumbersome @@ -489,6 +436,6 @@ class Prepare(object): raise SystemExit('--journal is required when using --filestore') # Default to bluestore here since defaulting it in add_argument may # cause both to be True - if not self.args.bluestore and not self.args.filestore and not self.args.seastore: + if not self.args.bluestore and not self.args.filestore: self.args.bluestore = True self.safe_prepare() diff --git a/src/ceph-volume/ceph_volume/util/prepare.py b/src/ceph-volume/ceph_volume/util/prepare.py index 6d4fc88373634..ff7427eedd207 100644 --- a/src/ceph-volume/ceph_volume/util/prepare.py +++ b/src/ceph-volume/ceph_volume/util/prepare.py @@ -484,69 +484,6 @@ def osd_mkfs_bluestore(osd_id, fsid, keyring=None, wal=False, db=False): raise RuntimeError('Command failed with exit code %s: %s' % (returncode, ' '.join(command))) -def osd_mkfs_seastore(osd_id, fsid, keyring=None): - """ - Create the files for the OSD to function. A normal call will look like: - - ceph-osd --cluster ceph --mkfs --mkkey -i 0 \ - --monmap /var/lib/ceph/osd/ceph-0/activate.monmap \ - --osd-data /var/lib/ceph/osd/ceph-0 \ - --osd-uuid 8d208665-89ae-4733-8888-5d3bfbeeec6c \ - --keyring /var/lib/ceph/osd/ceph-0/keyring \ - --setuser ceph --setgroup ceph - - In some cases it is required to use the keyring, when it is passed in as - a keyword argument it is used as part of the ceph-osd command - """ - path = '/var/lib/ceph/osd/%s-%s/' % (conf.cluster, osd_id) - monmap = os.path.join(path, 'activate.monmap') - - system.chown(path) - - base_command = [ - 'ceph-osd', - '--cluster', conf.cluster, - '--osd-objectstore', 'seastore', - '--mkfs', - '--key', "/var/lib/ceph/osd/ceph-%s/keyring" % osd_id, - '-i', osd_id, - '--monmap', monmap, - ] - - supplementary_command = [ - '--osd-data', path, - '--osd-uuid', fsid, - '--setuser', 'ceph', - '--setgroup', 'ceph' - ] - - if keyring is not None: - base_command.extend(['--keyfile', '-']) - - if get_osdspec_affinity(): - base_command.extend(['--osdspec-affinity', get_osdspec_affinity()]) - - command = base_command + supplementary_command - - """ - When running in containers the --mkfs on raw device sometimes fails - to acquire a lock through flock() on the device because systemd-udevd holds one temporarily. - See KernelDevice.cc and _lock() to understand how ceph-osd acquires the lock. - Because this is really transient, we retry up to 5 times and wait for 1 sec in-between - """ - for retry in range(5): - _, _, returncode = process.call(command, stdin=keyring, terminal_verbose=True, show_command=True) - if returncode == 0: - break - else: - if returncode == errno.EWOULDBLOCK: - time.sleep(1) - logger.info('disk is held by another process, trying to mkfs again... (%s/5 attempt)' % retry) - continue - else: - raise RuntimeError('Command failed with exit code %s: %s' % (returncode, ' '.join(command))) - - def osd_mkfs_filestore(osd_id, fsid, keyring): """ Create the files for the OSD to function. A normal call will look like: diff --git a/src/cephadm/cephadm.py b/src/cephadm/cephadm.py index aa8d2bbaf1e91..f23ab08326ad4 100755 --- a/src/cephadm/cephadm.py +++ b/src/cephadm/cephadm.py @@ -3585,13 +3585,8 @@ def deploy_daemon_units( '--no-systemd', ] else: - osd_type = 'bluestore' - if 'objectstore' in ctx and ctx.objectstore: - osd_type = ctx.objectstore - cmd = [ 'activate', - '--' + osd_type, '--osd-id', str(daemon_id), '--osd-uuid', osd_fsid, '--no-systemd', @@ -9857,10 +9852,6 @@ def _get_parser(): '--meta-json', help='JSON dict of additional metadata' ) - parser_deploy.add_argument( - '--objectstore', - help='Set object store' - ) parser_deploy.add_argument( '--extra-container-args', action='append', diff --git a/src/pybind/mgr/cephadm/serve.py b/src/pybind/mgr/cephadm/serve.py index 14cc06cb5ff9e..b4467947ae052 100644 --- a/src/pybind/mgr/cephadm/serve.py +++ b/src/pybind/mgr/cephadm/serve.py @@ -1226,11 +1226,11 @@ class CephadmServe: ports: List[int] = daemon_spec.ports if daemon_spec.ports else [] if daemon_spec.daemon_type == 'container': - custom_container_spec = cast(CustomContainerSpec, - self.mgr.spec_store[daemon_spec.service_name].spec) - image = custom_container_spec.image - if custom_container_spec.ports: - ports.extend(custom_container_spec.ports) + spec = cast(CustomContainerSpec, + self.mgr.spec_store[daemon_spec.service_name].spec) + image = spec.image + if spec.ports: + ports.extend(spec.ports) # TCP port to open in the host firewall if len(ports) > 0: @@ -1246,23 +1246,6 @@ class CephadmServe: if not osd_uuid: raise OrchestratorError('osd.%s not in osdmap' % daemon_spec.daemon_id) daemon_spec.extra_args.extend(['--osd-fsid', osd_uuid]) - if daemon_spec.service_name in self.mgr.spec_store: - osd_spec = cast(DriveGroupSpec, self.mgr.spec_store[daemon_spec.service_name].spec) - objectstore = osd_spec.objectstore - if objectstore: - daemon_spec.extra_args.extend(['--objectstore', objectstore]) - final_conf = daemon_spec.final_config['config'] - objectstore_str = '\n\tosd_objectstore = ' + objectstore - index = 0 - if final_conf.find("[osd]") != -1: - index = final_conf.index("[osd]") + 5 - elif final_conf.find("[global]") != -1: - index = final_conf.index("[global]") + 8 - if index != 0: - final_conf = final_conf[:index] + objectstore_str + final_conf[index:] - daemon_spec.final_config['config'] = final_conf - else: - daemon_spec.extra_args.extend(['--objectstore', 'bluestore']) if reconfig: daemon_spec.extra_args.append('--reconfig') diff --git a/src/pybind/mgr/cephadm/tests/test_cephadm.py b/src/pybind/mgr/cephadm/tests/test_cephadm.py index 67deaf441da20..364d4db692f52 100644 --- a/src/pybind/mgr/cephadm/tests/test_cephadm.py +++ b/src/pybind/mgr/cephadm/tests/test_cephadm.py @@ -121,7 +121,7 @@ def with_osd_daemon(cephadm_module: CephadmOrchestrator, _run_cephadm, host: str ['--', 'lvm', 'list', '--format', 'json'], no_fsid=False, error_ok=False, image='', log_output=True), mock.call(host, f'osd.{osd_id}', 'deploy', ['--name', f'osd.{osd_id}', '--meta-json', mock.ANY, - '--config-json', '-', '--osd-fsid', 'uuid', '--objectstore', 'bluestore'], + '--config-json', '-', '--osd-fsid', 'uuid'], stdin=mock.ANY, image=''), mock.call(host, 'osd', 'ceph-volume', ['--', 'raw', 'list', '--format', 'json'], no_fsid=False, error_ok=False, image='', log_output=True), diff --git a/src/python-common/ceph/deployment/drive_group.py b/src/python-common/ceph/deployment/drive_group.py index bfd54b1dbe091..7ac9759523dd8 100644 --- a/src/python-common/ceph/deployment/drive_group.py +++ b/src/python-common/ceph/deployment/drive_group.py @@ -339,10 +339,10 @@ class DriveGroupSpec(ServiceSpec): self.service_id, "`all` is only allowed for data_devices") - if self.objectstore not in ['bluestore', 'seastore']: + if self.objectstore not in ('bluestore'): raise DriveGroupValidationError(self.service_id, f"{self.objectstore} is not supported. Must be " - f"one of bluestore, seastore") + f"one of ('bluestore')") if self.block_wal_size is not None and type(self.block_wal_size) not in [int, str]: raise DriveGroupValidationError( diff --git a/src/python-common/ceph/deployment/translate.py b/src/python-common/ceph/deployment/translate.py index 8f2e330533866..86243b8aefda3 100644 --- a/src/python-common/ceph/deployment/translate.py +++ b/src/python-common/ceph/deployment/translate.py @@ -163,11 +163,6 @@ class to_ceph_volume(object): cmd += " --crush-device-class {}".format(d) cmds.append(cmd) - elif self.spec.objectstore == 'seastore': - cmd = "lvm batch --no-auto {}".format(" ".join(data_devices)) - cmd += " --seastore" - cmds.append(cmd) - for i in range(len(cmds)): if self.spec.encrypted: cmds[i] += " --dmcrypt" -- 2.39.5