Skip to content

Commit

Permalink
Merge PR #39482 into master
Browse files Browse the repository at this point in the history
* refs/pull/39482/head:
	Revert "Merge pull request #37764 from mgfritch/cephadm-no-container-init"

Reviewed-by: Josh Durgin <jdurgin@redhat.com>
  • Loading branch information
liewegas committed Feb 15, 2021
2 parents e42bbba + c896292 commit 9200b1e
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 46 deletions.
11 changes: 6 additions & 5 deletions doc/man/8/cephadm.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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}
| ...
Expand All @@ -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]
Expand Down Expand Up @@ -77,13 +78,15 @@ Synopsis
| [--registry-username REGISTRY_USERNAME]
| [--registry-password REGISTRY_PASSWORD]
| [--registry-json REGISTRY_JSON]
| [--container-init]


| **cephadm** **deploy** [-h] --name NAME --fsid FSID [--config CONFIG]
| [--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]
Expand Down Expand Up @@ -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
========
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
40 changes: 9 additions & 31 deletions src/cephadm/cephadm
Original file line number Diff line number Diff line change
Expand Up @@ -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]\$ '
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -2403,6 +2401,7 @@ def get_container(ctx: CephadmContext,
envs=envs,
privileged=privileged,
ptrace=ptrace,
init=ctx.container_init,
host_network=host_network,
)

Expand Down Expand Up @@ -2925,7 +2924,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
Expand All @@ -2939,7 +2938,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]:
Expand Down Expand Up @@ -3015,8 +3014,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])
Expand Down Expand Up @@ -3855,7 +3852,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'])
Expand All @@ -3878,6 +3876,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)

Expand Down Expand Up @@ -7078,11 +7077,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')

Expand Down Expand Up @@ -7151,8 +7145,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')
Expand Down Expand Up @@ -7446,8 +7439,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',
Expand Down Expand Up @@ -7505,8 +7497,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')
Expand Down Expand Up @@ -7626,22 +7617,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


Expand Down
4 changes: 2 additions & 2 deletions src/pybind/mgr/cephadm/module.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -342,7 +342,7 @@ def __init__(self, *args: Any, **kwargs: Any):
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
Expand Down
11 changes: 3 additions & 8 deletions src/pybind/mgr/cephadm/serve.py
Original file line number Diff line number Diff line change
Expand Up @@ -949,27 +949,22 @@ def _run_cephadm(self,

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:
Expand Down

0 comments on commit 9200b1e

Please sign in to comment.