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 <jmulligan@redhat.com>

diff --git a/src/cephadm/cephadm.py b/src/cephadm/cephadm.py
index 175ecad7defc126fbfc74f6fa6fa83ce627c13f9..bb16d1c8c224f6a64b56e8571f3083bff1353d25 100755 (executable)
--- 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 dc7834de36acd9fa49b54e304ba3d259ea0825aa..99d64ff015cbaa43470cfcb358f4147387e9f22b 100644 (file)
--- 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 940703733db7ab9f1cf85de5d54153f57c5866ee..f84a2d7ba20517aed90ddafe608abd87aa19b508 100644 (file)
--- 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