From: John Mulligan Date: Thu, 19 Oct 2023 18:22:49 +0000 (-0400) Subject: cephadm: remove get_deployment_container X-Git-Tag: v19.0.0~150^2~17 X-Git-Url: http://git.apps.os.sepia.ceph.com/?a=commitdiff_plain;h=7494c56e4bb4d7114bf24ea2098259595f678826;p=ceph-ci.git cephadm: remove get_deployment_container Remove get_deployment_container replacing all calls to it with calls to to_deployment_container. Now, callers can inject modifications to the container object between calls or even have to_deployment_container update container objects that were constructed in some other way. Signed-off-by: John Mulligan --- diff --git a/src/cephadm/cephadm.py b/src/cephadm/cephadm.py index 23420c2b473..bd0a1c97f4b 100755 --- a/src/cephadm/cephadm.py +++ b/src/cephadm/cephadm.py @@ -243,7 +243,8 @@ class Ceph(ContainerDaemonForm): uid, gid = self.uid_gid(ctx) make_var_run(ctx, ctx.fsid, uid, gid) - ctr = get_deployment_container(ctx, self.identity) + ctr = get_container(ctx, self.identity) + ctr = to_deployment_container(ctx, ctr) config_json = fetch_configs(ctx) if self.identity.daemon_type == 'mon' and config_json is not None: if 'crush_location' in config_json: @@ -464,7 +465,8 @@ class SNMPGateway(ContainerDaemonForm): raise Error('config is missing destination attribute(:) of the target SNMP listener') def container(self, ctx: CephadmContext) -> CephContainer: - return get_deployment_container(ctx, self.identity) + ctr = get_container(ctx, self.identity) + return to_deployment_container(ctx, ctr) def uid_gid(self, ctx: CephadmContext) -> Tuple[int, int]: return self.uid, self.gid @@ -626,7 +628,8 @@ class Monitoring(ContainerDaemonForm): def container(self, ctx: CephadmContext) -> CephContainer: self._prevalidate(ctx) - return get_deployment_container(ctx, self.identity) + ctr = get_container(ctx, self.identity) + return to_deployment_container(ctx, ctr) def uid_gid(self, ctx: CephadmContext) -> Tuple[int, int]: return self.extract_uid_gid(ctx, self.identity.daemon_type) @@ -814,7 +817,8 @@ class NFSGanesha(ContainerDaemonForm): return 'nfs' def container(self, ctx: CephadmContext) -> CephContainer: - return get_deployment_container(ctx, self.identity) + ctr = get_container(ctx, self.identity) + return to_deployment_container(ctx, ctr) def customize_container_endpoints( self, endpoints: List[EndPoint], deployment_type: DeploymentType @@ -1023,7 +1027,9 @@ done subident = DaemonSubIdentity( self.fsid, self.daemon_type, self.daemon_id, 'tcmu' ) - tcmu_container = get_deployment_container(self.ctx, subident) + tcmu_container = to_deployment_container( + self.ctx, get_container(self.ctx, subident) + ) # TODO: Eventually we don't want to run tcmu-runner through this script. # This is intended to be a workaround backported to older releases # and should eventually be removed in at least squid onward @@ -1032,7 +1038,8 @@ done return tcmu_container def container(self, ctx: CephadmContext) -> CephContainer: - return get_deployment_container(ctx, self.identity) + ctr = get_container(ctx, self.identity) + return to_deployment_container(ctx, ctr) def config_and_keyring( self, ctx: CephadmContext @@ -1181,7 +1188,8 @@ class CephNvmeof(ContainerDaemonForm): ] def container(self, ctx: CephadmContext) -> CephContainer: - return get_deployment_container(ctx, self.identity) + ctr = get_container(ctx, self.identity) + return to_deployment_container(ctx, ctr) def uid_gid(self, ctx: CephadmContext) -> Tuple[int, int]: return 167, 167 # TODO: need to get properly the uid/gid @@ -1265,7 +1273,8 @@ class CephExporter(ContainerDaemonForm): raise Error(f'Directory does not exist. Got: {self.sock_dir}') def container(self, ctx: CephadmContext) -> CephContainer: - return get_deployment_container(ctx, self.identity) + ctr = get_container(ctx, self.identity) + return to_deployment_container(ctx, ctr) def uid_gid(self, ctx: CephadmContext) -> Tuple[int, int]: return extract_uid_gid(ctx) @@ -1378,7 +1387,8 @@ class HAproxy(ContainerDaemonForm): ] def container(self, ctx: CephadmContext) -> CephContainer: - return get_deployment_container(ctx, self.identity) + ctr = get_container(ctx, self.identity) + return to_deployment_container(ctx, ctr) ################################## @@ -1490,7 +1500,8 @@ class Keepalived(ContainerDaemonForm): return mounts def container(self, ctx: CephadmContext) -> CephContainer: - return get_deployment_container(ctx, self.identity) + ctr = get_container(ctx, self.identity) + return to_deployment_container(ctx, ctr) ################################## @@ -1546,9 +1557,8 @@ class Tracing(ContainerDaemonForm): return self._identity def container(self, ctx: CephadmContext) -> CephContainer: - # TODO(jjm) this looks to be the only container for deployment - # not using get_deployment_container. Previous oversight? - return get_container(ctx, self.identity) + ctr = get_container(ctx, self.identity) + return to_deployment_container(ctx, ctr) def uid_gid(self, ctx: CephadmContext) -> Tuple[int, int]: return 65534, 65534 @@ -1686,12 +1696,13 @@ class CustomContainer(ContainerDaemonForm): def container(self, ctx: CephadmContext) -> CephContainer: if self._container is None: - self._container = get_deployment_container( + ctr = get_container( ctx, self.identity, privileged=self.privileged, ptrace=ctx.allow_ptrace, ) + self._container = to_deployment_container(ctx, ctr) return self._container def init_containers(self, ctx: CephadmContext) -> List[InitContainer]: @@ -5096,19 +5107,6 @@ def command_registry_login(ctx: CephadmContext) -> int: ################################## -def get_deployment_container( - ctx: CephadmContext, - ident: 'DaemonIdentity', - privileged: bool = False, - ptrace: bool = False, - container_args: Optional[List[str]] = None, -) -> 'CephContainer': - # wrapper for get_container specifically for containers made during the `cephadm deploy` - # command. Adds some extra things such as extra container args and custom config files - c = get_container(ctx, ident, privileged, ptrace, container_args) - return to_deployment_container(ctx, c) - - def to_deployment_container( ctx: CephadmContext, ctr: CephContainer ) -> CephContainer: diff --git a/src/cephadm/tests/test_cephadm.py b/src/cephadm/tests/test_cephadm.py index 7e31b26307c..f933e3bc470 100644 --- a/src/cephadm/tests/test_cephadm.py +++ b/src/cephadm/tests/test_cephadm.py @@ -329,9 +329,9 @@ class TestCephAdm(object): @mock.patch('cephadm.logger') @mock.patch('cephadm.fetch_custom_config_files') @mock.patch('cephadm.get_container') - def test_get_deployment_container(self, _get_container, _get_config, _logger): + def test_to_deployment_container(self, _get_container, _get_config, _logger): """ - test get_deployment_container properly makes use of extra container args and custom conf files + test to_deployment_container properly makes use of extra container args and custom conf files """ ctx = _cephadm.CephadmContext() @@ -365,7 +365,8 @@ class TestCephAdm(object): ptrace=False, host_network=True, ) - c = _cephadm.get_deployment_container(ctx, ident) + c = _cephadm.get_container(ctx, ident) + c = _cephadm.to_deployment_container(ctx, c) assert '--pids-limit=12345' in c.container_args assert '--something' in c.container_args @@ -380,9 +381,9 @@ class TestCephAdm(object): @mock.patch('cephadm.check_unit', lambda *args, **kwargs: (None, 'running', None)) @mock.patch('cephadm.get_unit_name', lambda *args, **kwargs: 'mon-unit-name') @mock.patch('cephadm.extract_uid_gid', lambda *args, **kwargs: (0, 0)) - @mock.patch('cephadm.get_deployment_container') + @mock.patch('cephadm.get_container') @mock.patch('cephadm.apply_deploy_config_to_ctx', lambda d, c: None) - def test_mon_crush_location(self, _get_deployment_container, _migrate_sysctl, _make_var_run, _deploy_daemon, _file_lock, _logger, monkeypatch): + def test_mon_crush_location(self, _get_container, _migrate_sysctl, _make_var_run, _deploy_daemon, _file_lock, _logger, monkeypatch): """ test that crush location for mon is set if it is included in config_json """ @@ -390,6 +391,7 @@ class TestCephAdm(object): monkeypatch.setattr('cephadmlib.context_getters.fetch_configs', _fetch_configs) monkeypatch.setattr('cephadm.fetch_configs', _fetch_configs) monkeypatch.setattr('cephadm.read_configuration_source', lambda c: {}) + monkeypatch.setattr('cephadm.fetch_custom_config_files', mock.MagicMock()) ctx = _cephadm.CephadmContext() ctx.name = 'mon.test' @@ -404,7 +406,7 @@ class TestCephAdm(object): 'crush_location': 'database=a' } - _get_deployment_container.return_value = _cephadm.CephContainer.for_daemon( + _get_container.return_value = _cephadm.CephContainer.for_daemon( ctx, ident=_cephadm.DaemonIdentity( fsid='9b9d7609-f4d5-4aba-94c8-effa764d96c9',