From: John Mulligan Date: Fri, 29 Sep 2023 17:42:33 +0000 (-0400) Subject: cephadm: move podman service args selection to Podman class X-Git-Tag: v19.0.0~177^2 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=19f5249029fdb3e14bac0c462db422469c680cd6;p=ceph.git cephadm: move podman service args selection to Podman class Move code that chooses what additional args should be added to a service container under podman to the Podman class. This continues the effort to separate and encapsulate code and associate it with general types best suited to them. Signed-off-by: John Mulligan --- diff --git a/src/cephadm/cephadm.py b/src/cephadm/cephadm.py index 175ecad7defc1..bb16d1c8c224f 100755 --- a/src/cephadm/cephadm.py +++ b/src/cephadm/cephadm.py @@ -2820,28 +2820,12 @@ def get_container( def _update_container_args_for_podman( ctx: CephadmContext, ident: DaemonIdentity, container_args: List[str] ) -> None: - # if using podman, set -d, --conmon-pidfile & --cidfile flags - # so service can have Type=Forking - if isinstance(ctx.container_engine, Podman): - runtime_dir = '/run' - service_name = f'{ident.unit_name}.service' - container_args.extend([ - '-d', '--log-driver', 'journald', - '--conmon-pidfile', - f'{runtime_dir}/{service_name}-pid', - '--cidfile', - f'{runtime_dir}/{service_name}-cid', - ]) - if ctx.container_engine.supports_split_cgroups and not ctx.no_cgroups_split: - container_args.append('--cgroups=split') - # if /etc/hosts doesn't exist, we can be confident - # users aren't using it for host name resolution - # and adding --no-hosts avoids bugs created in certain daemons - # by modifications podman makes to /etc/hosts - # https://tracker.ceph.com/issues/58532 - # https://tracker.ceph.com/issues/57018 - if not os.path.exists('/etc/hosts'): - container_args.extend(['--no-hosts']) + if not isinstance(ctx.container_engine, Podman): + return + service_name = f'{ident.unit_name}.service' + container_args.extend( + ctx.container_engine.service_args(ctx, service_name) + ) def extract_uid_gid(ctx, img='', file_path='/var/lib/ceph'): diff --git a/src/cephadm/cephadmlib/container_engines.py b/src/cephadm/cephadmlib/container_engines.py index dc7834de36acd..99d64ff015cba 100644 --- a/src/cephadm/cephadmlib/container_engines.py +++ b/src/cephadm/cephadmlib/container_engines.py @@ -45,6 +45,39 @@ class Podman(ContainerEngine): """Return true if this version of podman supports split cgroups.""" return self.version >= CGROUPS_SPLIT_PODMAN_VERSION + def service_args( + self, ctx: CephadmContext, service_name: str + ) -> List[str]: + """Return a list of arguments that should be added to the engine's run + command when starting a long-term service (aka daemon) container. + """ + args = [] + # if using podman, set -d, --conmon-pidfile & --cidfile flags + # so service can have Type=Forking + runtime_dir = '/run' + args.extend( + [ + '-d', + '--log-driver', + 'journald', + '--conmon-pidfile', + f'{runtime_dir}/{service_name}-pid', + '--cidfile', + f'{runtime_dir}/{service_name}-cid', + ] + ) + if self.supports_split_cgroups and not ctx.no_cgroups_split: + args.append('--cgroups=split') + # if /etc/hosts doesn't exist, we can be confident + # users aren't using it for host name resolution + # and adding --no-hosts avoids bugs created in certain daemons + # by modifications podman makes to /etc/hosts + # https://tracker.ceph.com/issues/58532 + # https://tracker.ceph.com/issues/57018 + if not os.path.exists('/etc/hosts'): + args.append('--no-hosts') + return args + class Docker(ContainerEngine): EXE = 'docker' diff --git a/src/cephadm/tests/fixtures.py b/src/cephadm/tests/fixtures.py index 940703733db7a..f84a2d7ba2051 100644 --- a/src/cephadm/tests/fixtures.py +++ b/src/cephadm/tests/fixtures.py @@ -36,6 +36,7 @@ def mock_podman(): # supports_split_cgroups attribute: # https://docs.python.org/3/library/unittest.mock.html#unittest.mock.Mock type(podman).supports_split_cgroups = Podman.supports_split_cgroups + type(podman).service_args = Podman.service_args return podman