Skip to content

Commit 19f5249

Browse files
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 <[email protected]>
1 parent 621e108 commit 19f5249

File tree

3 files changed

+40
-22
lines changed

3 files changed

+40
-22
lines changed

src/cephadm/cephadm.py

Lines changed: 6 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -2820,28 +2820,12 @@ def get_container(
28202820
def _update_container_args_for_podman(
28212821
ctx: CephadmContext, ident: DaemonIdentity, container_args: List[str]
28222822
) -> None:
2823-
# if using podman, set -d, --conmon-pidfile & --cidfile flags
2824-
# so service can have Type=Forking
2825-
if isinstance(ctx.container_engine, Podman):
2826-
runtime_dir = '/run'
2827-
service_name = f'{ident.unit_name}.service'
2828-
container_args.extend([
2829-
'-d', '--log-driver', 'journald',
2830-
'--conmon-pidfile',
2831-
f'{runtime_dir}/{service_name}-pid',
2832-
'--cidfile',
2833-
f'{runtime_dir}/{service_name}-cid',
2834-
])
2835-
if ctx.container_engine.supports_split_cgroups and not ctx.no_cgroups_split:
2836-
container_args.append('--cgroups=split')
2837-
# if /etc/hosts doesn't exist, we can be confident
2838-
# users aren't using it for host name resolution
2839-
# and adding --no-hosts avoids bugs created in certain daemons
2840-
# by modifications podman makes to /etc/hosts
2841-
# https://tracker.ceph.com/issues/58532
2842-
# https://tracker.ceph.com/issues/57018
2843-
if not os.path.exists('/etc/hosts'):
2844-
container_args.extend(['--no-hosts'])
2823+
if not isinstance(ctx.container_engine, Podman):
2824+
return
2825+
service_name = f'{ident.unit_name}.service'
2826+
container_args.extend(
2827+
ctx.container_engine.service_args(ctx, service_name)
2828+
)
28452829

28462830

28472831
def extract_uid_gid(ctx, img='', file_path='/var/lib/ceph'):

src/cephadm/cephadmlib/container_engines.py

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,39 @@ def supports_split_cgroups(self) -> bool:
4545
"""Return true if this version of podman supports split cgroups."""
4646
return self.version >= CGROUPS_SPLIT_PODMAN_VERSION
4747

48+
def service_args(
49+
self, ctx: CephadmContext, service_name: str
50+
) -> List[str]:
51+
"""Return a list of arguments that should be added to the engine's run
52+
command when starting a long-term service (aka daemon) container.
53+
"""
54+
args = []
55+
# if using podman, set -d, --conmon-pidfile & --cidfile flags
56+
# so service can have Type=Forking
57+
runtime_dir = '/run'
58+
args.extend(
59+
[
60+
'-d',
61+
'--log-driver',
62+
'journald',
63+
'--conmon-pidfile',
64+
f'{runtime_dir}/{service_name}-pid',
65+
'--cidfile',
66+
f'{runtime_dir}/{service_name}-cid',
67+
]
68+
)
69+
if self.supports_split_cgroups and not ctx.no_cgroups_split:
70+
args.append('--cgroups=split')
71+
# if /etc/hosts doesn't exist, we can be confident
72+
# users aren't using it for host name resolution
73+
# and adding --no-hosts avoids bugs created in certain daemons
74+
# by modifications podman makes to /etc/hosts
75+
# https://tracker.ceph.com/issues/58532
76+
# https://tracker.ceph.com/issues/57018
77+
if not os.path.exists('/etc/hosts'):
78+
args.append('--no-hosts')
79+
return args
80+
4881

4982
class Docker(ContainerEngine):
5083
EXE = 'docker'

src/cephadm/tests/fixtures.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ def mock_podman():
3636
# supports_split_cgroups attribute:
3737
# https://docs.python.org/3/library/unittest.mock.html#unittest.mock.Mock
3838
type(podman).supports_split_cgroups = Podman.supports_split_cgroups
39+
type(podman).service_args = Podman.service_args
3940
return podman
4041

4142

0 commit comments

Comments
 (0)