From c896292d5841c10c87eac3948d71f9eec31fe5e5 Mon Sep 17 00:00:00 2001 From: Sage Weil Date: Fri, 12 Feb 2021 15:04:57 -0500 Subject: [PATCH] Revert "Merge pull request #37764 from mgfritch/cephadm-no-container-init" This reverts commit f635555fe76322336a1e0e8cc3c199562642ad0e, reversing changes made to d4d3d17b23fcf6d8808a96b992f56924f495b174. This PR seems to be (indirectly?) responsible for https://tracker.ceph.com/issues/49237 Also, it was causing the rados.py task's follow-up step to wait for snap trimming to fail: it would time out a 'ceph osd dump --format=json' command. :/ Signed-off-by: Sage Weil --- doc/man/8/cephadm.rst | 11 +++++---- src/cephadm/cephadm | 40 +++++++------------------------- src/pybind/mgr/cephadm/module.py | 4 ++-- src/pybind/mgr/cephadm/serve.py | 11 +++------ 4 files changed, 20 insertions(+), 46 deletions(-) diff --git a/doc/man/8/cephadm.rst b/doc/man/8/cephadm.rst index db0c0698c37..7a247348592 100644 --- a/doc/man/8/cephadm.rst +++ b/doc/man/8/cephadm.rst @@ -12,7 +12,7 @@ Synopsis | **cephadm**** [-h] [--image IMAGE] [--docker] [--data-dir DATA_DIR] | [--log-dir LOG_DIR] [--logrotate-dir LOGROTATE_DIR] | [--unit-dir UNIT_DIR] [--verbose] [--timeout TIMEOUT] -| [--retry RETRY] [--no-container-init] +| [--retry RETRY] | {version,pull,inspect-image,ls,list-networks,adopt,rm-daemon,rm-cluster,run,shell,enter,ceph-volume,unit,logs,bootstrap,deploy,check-host,prepare-host,add-repo,rm-repo,install} | ... @@ -28,6 +28,7 @@ Synopsis | **cephadm** **adopt** [-h] --name NAME --style STYLE [--cluster CLUSTER] | [--legacy-dir LEGACY_DIR] [--config-json CONFIG_JSON] | [--skip-firewalld] [--skip-pull] +| [--container-init] | **cephadm** **rm-daemon** [-h] --name NAME --fsid FSID [--force] | [--force-delete-data] @@ -77,6 +78,7 @@ Synopsis | [--registry-username REGISTRY_USERNAME] | [--registry-password REGISTRY_PASSWORD] | [--registry-json REGISTRY_JSON] +| [--container-init] @@ -84,6 +86,7 @@ Synopsis | [--config-json CONFIG_JSON] [--keyring KEYRING] | [--key KEY] [--osd-fsid OSD_FSID] [--skip-firewalld] | [--tcp-ports TCP_PORTS] [--reconfig] [--allow-ptrace] +| [--container-init] | **cephadm** **check-host** [-h] [--expect-hostname EXPECT_HOSTNAME] @@ -154,10 +157,6 @@ Options max number of retries (default: 10) -.. option:: --no-container-init - - do not run podman/docker with `--init` (default: False) - Commands ======== @@ -239,6 +238,7 @@ Arguments: * [--registry-username REGISTRY_USERNAME] username of account to login to on custom registry * [--registry-password REGISTRY_PASSWORD] password of account to login to on custom registry * [--registry-json REGISTRY_JSON] JSON file containing registry login info (see registry-login command documentation) +* [--container-init] Run podman/docker with `--init` ceph-volume @@ -289,6 +289,7 @@ Arguments: * [--tcp-ports List of tcp ports to open in the host firewall * [--reconfig] Reconfigure a previously deployed daemon * [--allow-ptrace] Allow SYS_PTRACE on daemon container +* [--container-init] Run podman/docker with `--init` enter diff --git a/src/cephadm/cephadm b/src/cephadm/cephadm index f42ec370eaf..cc9e2c4ffc3 100755 --- a/src/cephadm/cephadm +++ b/src/cephadm/cephadm @@ -10,7 +10,6 @@ LOGROTATE_DIR = '/etc/logrotate.d' UNIT_DIR = '/etc/systemd/system' LOG_DIR_MODE = 0o770 DATA_DIR_MODE = 0o700 -CONTAINER_INIT=True CONTAINER_PREFERENCE = ['podman', 'docker'] # prefer podman to docker MIN_PODMAN_VERSION = (2, 0, 2) CUSTOM_PS1 = r'[ceph: \u@\h \W]\$ ' @@ -108,7 +107,6 @@ class BaseConfig: self.retry: int = DEFAULT_RETRY self.env: List[str] = [] - self.container_init: bool = CONTAINER_INIT self.container_path: str = "" def set_from_args(self, args: argparse.Namespace): @@ -2392,6 +2390,7 @@ def get_container(ctx: CephadmContext, envs=envs, privileged=privileged, ptrace=ptrace, + init=ctx.container_init, host_network=host_network, ) @@ -2914,7 +2913,7 @@ class CephContainer: privileged: bool = False, ptrace: bool = False, bind_mounts: Optional[List[List[str]]] = None, - init: Optional[bool] = None, + init: bool = False, host_network: bool = True, ) -> None: self.ctx = ctx @@ -2928,7 +2927,7 @@ class CephContainer: self.privileged = privileged self.ptrace = ptrace self.bind_mounts = bind_mounts if bind_mounts else [] - self.init = init if init else ctx.container_init + self.init = init self.host_network = host_network def run_cmd(self) -> List[str]: @@ -3004,8 +3003,6 @@ class CephContainer: # let OSD etc read block devs that haven't been chowned '--group-add=disk', ]) - if self.init: - cmd_args.append('--init') if self.envs: for env in self.envs: envs.extend(['-e', env]) @@ -3844,7 +3841,8 @@ def command_bootstrap(ctx): cli(['config', 'set', 'mgr', 'mgr/cephadm/registry_username', ctx.registry_username, '--force']) cli(['config', 'set', 'mgr', 'mgr/cephadm/registry_password', ctx.registry_password, '--force']) - cli(['config', 'set', 'mgr', 'mgr/cephadm/container_init', str(ctx.container_init), '--force']) + if ctx.container_init: + cli(['config', 'set', 'mgr', 'mgr/cephadm/container_init', str(ctx.container_init), '--force']) if ctx.with_exporter: cli(['config-key', 'set', 'mgr/cephadm/exporter_enabled', 'true']) @@ -3867,6 +3865,7 @@ def command_bootstrap(ctx): logger.info('Deploying cephadm exporter service with default placement...') cli(['orch', 'apply', 'cephadm-exporter']) + if not ctx.skip_dashboard: prepare_dashboard(ctx, uid, gid, cli, wait_for_mgr_restart) @@ -7057,11 +7056,6 @@ def _get_parser(): action='append', default=[], help='set environment variable') - parser.add_argument( - '--no-container-init', - action='store_true', - default=not CONTAINER_INIT, - help='Do not run podman/docker with `--init`') subparsers = parser.add_subparsers(help='sub-command') @@ -7130,8 +7124,7 @@ def _get_parser(): parser_adopt.add_argument( '--container-init', action='store_true', - default=CONTAINER_INIT, - help=argparse.SUPPRESS) + help='Run podman/docker with `--init`') parser_rm_daemon = subparsers.add_parser( 'rm-daemon', help='remove daemon instance') @@ -7425,8 +7418,7 @@ def _get_parser(): parser_bootstrap.add_argument( '--container-init', action='store_true', - default=CONTAINER_INIT, - help=argparse.SUPPRESS) + help='Run podman/docker with `--init`') parser_bootstrap.add_argument( '--with-exporter', action='store_true', @@ -7484,8 +7476,7 @@ def _get_parser(): parser_deploy.add_argument( '--container-init', action='store_true', - default=CONTAINER_INIT, - help=argparse.SUPPRESS) + help='Run podman/docker with `--init`') parser_check_host = subparsers.add_parser( 'check-host', help='check host configuration') @@ -7605,22 +7596,9 @@ def _get_parser(): def _parse_args(av): parser = _get_parser() - args = parser.parse_args(av) if 'command' in args and args.command and args.command[0] == "--": args.command.pop(0) - - # workaround argparse to deprecate the subparser `--container-init` flag - # container_init and no_container_init must always be mutually exclusive - container_init_args = ('--container-init', '--no-container-init') - if set(container_init_args).issubset(av): - parser.error('argument %s: not allowed with argument %s' % (container_init_args)) - elif '--container-init' in av: - args.no_container_init = not args.container_init - else: - args.container_init = not args.no_container_init - assert args.container_init is not args.no_container_init - return args diff --git a/src/pybind/mgr/cephadm/module.py b/src/pybind/mgr/cephadm/module.py index e659b231ead..3f01f6a63a3 100644 --- a/src/pybind/mgr/cephadm/module.py +++ b/src/pybind/mgr/cephadm/module.py @@ -255,7 +255,7 @@ class CephadmOrchestrator(orchestrator.Orchestrator, MgrModule, Option( 'container_init', type='bool', - default=True, + default=False, desc='Run podman/docker with `--init`' ), Option( @@ -342,7 +342,7 @@ class CephadmOrchestrator(orchestrator.Orchestrator, MgrModule, self.warn_on_stray_daemons = True self.warn_on_failed_host_check = True self.allow_ptrace = False - self.container_init = True + self.container_init = False self.prometheus_alerts_path = '' self.migration_current: Optional[int] = None self.config_dashboard = True diff --git a/src/pybind/mgr/cephadm/serve.py b/src/pybind/mgr/cephadm/serve.py index d39038f5c43..0c630e8a3fd 100644 --- a/src/pybind/mgr/cephadm/serve.py +++ b/src/pybind/mgr/cephadm/serve.py @@ -944,27 +944,22 @@ class CephadmServe: final_args = [] - # global args if env_vars: for env_var_pair in env_vars: final_args.extend(['--env', env_var_pair]) if image: final_args.extend(['--image', image]) - - if not self.mgr.container_init: - final_args += ['--no-container-init'] - - # subcommand final_args.append(command) - # subcommand args if not no_fsid: final_args += ['--fsid', self.mgr._cluster_fsid] + if self.mgr.container_init: + final_args += ['--container-init'] + final_args += args - # exec self.log.debug('args: %s' % (' '.join(final_args))) if self.mgr.mode == 'root': if stdin: -- 2.39.5