From: Adam King Date: Tue, 8 Nov 2022 19:09:05 +0000 (-0500) Subject: mgr/cephadm: support for extra entrypoint args X-Git-Tag: v16.2.13~81^2~2 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=24d2bc57328cd72465d3f8945d43b13981d2f1d8;p=ceph.git mgr/cephadm: support for extra entrypoint args Args specified in the service spec to be added as args for the entrypoint when we deploy the daemon Fixes: https://tracker.ceph.com/issues/57944 Signed-off-by: Adam King (cherry picked from commit db8e4462c968e1d29b0688efafad3442abb8779c) Conflicts: src/cephadm/cephadm src/pybind/mgr/cephadm/module.py src/pybind/mgr/cephadm/services/cephadmservice.py src/pybind/mgr/cephadm/tests/test_cephadm.py src/pybind/mgr/cephadm/tests/test_services.py --- diff --git a/src/cephadm/cephadm b/src/cephadm/cephadm index 2488bcceb7b..8bd026abd23 100755 --- a/src/cephadm/cephadm +++ b/src/cephadm/cephadm @@ -5033,6 +5033,8 @@ def get_container_with_extra_args(ctx: CephadmContext, c = get_container(ctx, fsid, daemon_type, daemon_id, privileged, ptrace, container_args) if 'extra_container_args' in ctx and ctx.extra_container_args: c.container_args.extend(ctx.extra_container_args) + if 'extra_entrypoint_args' in ctx and ctx.extra_entrypoint_args: + c.args.extend(ctx.extra_entrypoint_args) return c @@ -9086,6 +9088,12 @@ def _get_parser(): default=[], help='Additional container arguments to apply to deamon' ) + parser_deploy.add_argument( + '--extra-entrypoint-args', + action='append', + default=[], + help='Additional entrypoint arguments to apply to deamon' + ) parser_check_host = subparsers.add_parser( 'check-host', help='check host configuration') diff --git a/src/pybind/mgr/cephadm/serve.py b/src/pybind/mgr/cephadm/serve.py index d48f055fd93..42e25a9b962 100644 --- a/src/pybind/mgr/cephadm/serve.py +++ b/src/pybind/mgr/cephadm/serve.py @@ -336,6 +336,7 @@ class CephadmServe: sd.rank_generation = int(d['rank_generation']) if d.get( 'rank_generation') is not None else None sd.extra_container_args = d.get('extra_container_args') + sd.extra_entrypoint_args = d.get('extra_entrypoint_args') if 'state' in d: sd.status_desc = d['state'] sd.status = { @@ -903,6 +904,12 @@ class CephadmServe: self.log.info(f'Redeploying {dd.name()}, (container cli args changed) . . .') dd.extra_container_args = spec.extra_container_args action = 'redeploy' + elif spec is not None and hasattr(spec, 'extra_entrypoint_args') and dd.extra_entrypoint_args != spec.extra_entrypoint_args: + self.log.info(f'Redeploying {dd.name()}, (entrypoint args changed) . . .') + self.log.debug( + f'{dd.name()} daemon entrypoint args {dd.extra_entrypoint_args} -> {spec.extra_entrypoint_args}') + dd.extra_entrypoint_args = spec.extra_entrypoint_args + action = 'redeploy' elif self.mgr.last_monmap and \ self.mgr.last_monmap > last_config and \ dd.daemon_type in CEPH_TYPES: @@ -1133,13 +1140,7 @@ class CephadmServe: if self.mgr.allow_ptrace: daemon_spec.extra_args.append('--allow-ptrace') - try: - eca = daemon_spec.extra_container_args - if eca: - for a in eca: - daemon_spec.extra_args.append(f'--extra-container-args={a}') - except AttributeError: - eca = None + daemon_spec, extra_container_args, extra_entrypoint_args = self._setup_extra_deployment_args(daemon_spec) if self.mgr.cache.host_needs_registry_login(daemon_spec.host) and self.mgr.registry_url: self._registry_login(daemon_spec.host, @@ -1160,7 +1161,8 @@ class CephadmServe: 'deployed_by': self.mgr.get_active_mgr_digests(), 'rank': daemon_spec.rank, 'rank_generation': daemon_spec.rank_generation, - 'extra_container_args': eca + 'extra_container_args': extra_container_args, + 'extra_entrypoint_args': extra_entrypoint_args }), '--config-json', '-', ] + daemon_spec.extra_args, @@ -1201,6 +1203,29 @@ class CephadmServe: self.mgr.cephadm_services[servict_type].post_remove(dd, is_failed_deploy=True) raise + def _setup_extra_deployment_args(self, daemon_spec: CephadmDaemonDeploySpec) -> Tuple[CephadmDaemonDeploySpec, Optional[List[str]], Optional[List[str]]]: + # this function is for handling any potential user specified + # (in the service spec) extra runtime or entrypoint args for a daemon + # we are going to deploy. Effectively just adds a set of extra args to + # pass to the cephadm binary to indicate the daemon being deployed + # needs extra runtime/entrypoint args. Returns the modified daemon spec + # as well as what args were added (as those are included in unit.meta file) + try: + eca = daemon_spec.extra_container_args + if eca: + for a in eca: + daemon_spec.extra_args.append(f'--extra-container-args={a}') + except AttributeError: + eca = None + try: + eea = daemon_spec.extra_entrypoint_args + if eea: + for a in eea: + daemon_spec.extra_args.append(f'--extra-entrypoint-args={a}') + except AttributeError: + eea = None + return daemon_spec, eca, eea + def _remove_daemon(self, name: str, host: str, no_post_remove: bool = False) -> str: """ Remove a daemon diff --git a/src/pybind/mgr/cephadm/services/cephadmservice.py b/src/pybind/mgr/cephadm/services/cephadmservice.py index 92d293fcd00..48100ff582f 100644 --- a/src/pybind/mgr/cephadm/services/cephadmservice.py +++ b/src/pybind/mgr/cephadm/services/cephadmservice.py @@ -40,7 +40,9 @@ class CephadmDaemonDeploySpec: ports: Optional[List[int]] = None, rank: Optional[int] = None, rank_generation: Optional[int] = None, - extra_container_args: Optional[List[str]] = None): + extra_container_args: Optional[List[str]] = None, + extra_entrypoint_args: Optional[List[str]] = None, + ): """ A data struction to encapsulate `cephadm deploy ... """ @@ -76,6 +78,7 @@ class CephadmDaemonDeploySpec: self.rank_generation: Optional[int] = rank_generation self.extra_container_args = extra_container_args + self.extra_entrypoint_args = extra_entrypoint_args def name(self) -> str: return '%s.%s' % (self.daemon_type, self.daemon_id) @@ -102,6 +105,7 @@ class CephadmDaemonDeploySpec: rank=dd.rank, rank_generation=dd.rank_generation, extra_container_args=dd.extra_container_args, + extra_entrypoint_args=dd.extra_entrypoint_args, ) def to_daemon_description(self, status: DaemonDescriptionStatus, status_desc: str) -> DaemonDescription: @@ -117,6 +121,7 @@ class CephadmDaemonDeploySpec: rank=self.rank, rank_generation=self.rank_generation, extra_container_args=self.extra_container_args, + extra_entrypoint_args=self.extra_entrypoint_args, ) @@ -182,6 +187,10 @@ class CephadmService(metaclass=ABCMeta): eca = spec.extra_container_args except AttributeError: eca = None + try: + eea = spec.extra_entrypoint_args + except AttributeError: + eea = None return CephadmDaemonDeploySpec( host=host, daemon_id=daemon_id, @@ -193,6 +202,7 @@ class CephadmService(metaclass=ABCMeta): rank=rank, rank_generation=rank_generation, extra_container_args=eca, + extra_entrypoint_args=eea, ) def prepare_create(self, daemon_spec: CephadmDaemonDeploySpec) -> CephadmDaemonDeploySpec: diff --git a/src/pybind/mgr/cephadm/tests/test_cephadm.py b/src/pybind/mgr/cephadm/tests/test_cephadm.py index f29a31530a3..aa237e089e1 100644 --- a/src/pybind/mgr/cephadm/tests/test_cephadm.py +++ b/src/pybind/mgr/cephadm/tests/test_cephadm.py @@ -449,7 +449,8 @@ class TestCephadm(object): _run_cephadm.assert_called_with( 'test', 'mon.test', 'deploy', [ '--name', 'mon.test', - '--meta-json', '{"service_name": "mon", "ports": [], "ip": null, "deployed_by": [], "rank": null, "rank_generation": null, "extra_container_args": null}', + '--meta-json', ('{"service_name": "mon", "ports": [], "ip": null, "deployed_by": [], "rank": null, ' + '"rank_generation": null, "extra_container_args": null, "extra_entrypoint_args": null}'), '--config-json', '-', '--reconfig', ], @@ -465,7 +466,8 @@ class TestCephadm(object): _run_cephadm.assert_called_with( 'test', 'crash.test', 'deploy', [ '--name', 'crash.test', - '--meta-json', '{"service_name": "crash", "ports": [], "ip": null, "deployed_by": [], "rank": null, "rank_generation": null, "extra_container_args": ["--cpus=2", "--quiet"]}', + '--meta-json', ('{"service_name": "crash", "ports": [], "ip": null, "deployed_by": [], "rank": null, ' + '"rank_generation": null, "extra_container_args": ["--cpus=2", "--quiet"], "extra_entrypoint_args": null}'), '--config-json', '-', '--extra-container-args=--cpus=2', '--extra-container-args=--quiet' @@ -474,6 +476,55 @@ class TestCephadm(object): image='', ) + @mock.patch("cephadm.serve.CephadmServe._run_cephadm") + def test_extra_entrypoint_args(self, _run_cephadm, cephadm_module: CephadmOrchestrator): + _run_cephadm.return_value = ('{}', '', 0) + with with_host(cephadm_module, 'test'): + with with_service(cephadm_module, ServiceSpec(service_type='node-exporter', + extra_entrypoint_args=['--collector.textfile.directory=/var/lib/node_exporter/textfile_collector', '--some-other-arg']), + CephadmOrchestrator.apply_node_exporter): + _run_cephadm.assert_called_with( + 'test', 'node-exporter.test', 'deploy', [ + '--name', 'node-exporter.test', + '--meta-json', ('{"service_name": "node-exporter", "ports": [9100], "ip": null, "deployed_by": [], "rank": null, ' + '"rank_generation": null, "extra_container_args": null, "extra_entrypoint_args": ' + '["--collector.textfile.directory=/var/lib/node_exporter/textfile_collector", ' + '"--some-other-arg"]}'), + '--config-json', '-', + '--tcp-ports', '9100', + '--extra-entrypoint-args=--collector.textfile.directory=/var/lib/node_exporter/textfile_collector', + '--extra-entrypoint-args=--some-other-arg' + ], + stdin='{}', + image='', + ) + + @mock.patch("cephadm.serve.CephadmServe._run_cephadm") + def test_extra_entrypoint_and_container_args(self, _run_cephadm, cephadm_module: CephadmOrchestrator): + _run_cephadm.return_value = ('{}', '', 0) + with with_host(cephadm_module, 'test'): + with with_service(cephadm_module, ServiceSpec(service_type='node-exporter', + extra_entrypoint_args=['--collector.textfile.directory=/var/lib/node_exporter/textfile_collector', '--some-other-arg'], + extra_container_args=['--cpus=2', '--quiet']), + CephadmOrchestrator.apply_node_exporter): + _run_cephadm.assert_called_with( + 'test', 'node-exporter.test', 'deploy', [ + '--name', 'node-exporter.test', + '--meta-json', ('{"service_name": "node-exporter", "ports": [9100], "ip": null, "deployed_by": [], "rank": null, ' + '"rank_generation": null, "extra_container_args": ["--cpus=2", "--quiet"], "extra_entrypoint_args": ' + '["--collector.textfile.directory=/var/lib/node_exporter/textfile_collector", ' + '"--some-other-arg"]}'), + '--config-json', '-', + '--tcp-ports', '9100', + '--extra-container-args=--cpus=2', + '--extra-container-args=--quiet', + '--extra-entrypoint-args=--collector.textfile.directory=/var/lib/node_exporter/textfile_collector', + '--extra-entrypoint-args=--some-other-arg' + ], + stdin='{}', + image='', + ) + @mock.patch("cephadm.serve.CephadmServe._run_cephadm", _run_cephadm('{}')) def test_daemon_check_post(self, cephadm_module: CephadmOrchestrator): with with_host(cephadm_module, 'test'): diff --git a/src/pybind/mgr/cephadm/tests/test_services.py b/src/pybind/mgr/cephadm/tests/test_services.py index 8f1bc338ba1..6973ae2dbee 100644 --- a/src/pybind/mgr/cephadm/tests/test_services.py +++ b/src/pybind/mgr/cephadm/tests/test_services.py @@ -279,7 +279,7 @@ log_to_file = False""" 'deploy', [ '--name', f'iscsi.{iscsi_daemon_id}', - '--meta-json', f'{"{"}"service_name": "iscsi.{pool}", "ports": [{api_port}], "ip": null, "deployed_by": [], "rank": null, "rank_generation": null, "extra_container_args": null{"}"}', + '--meta-json', f'{"{"}"service_name": "iscsi.{pool}", "ports": [{api_port}], "ip": null, "deployed_by": [], "rank": null, "rank_generation": null, "extra_container_args": null, "extra_entrypoint_args": null{"}"}', '--config-json', '-', '--tcp-ports', '3456' ], stdin=json.dumps({"config": "", "keyring": "", "files": {"iscsi-gateway.cfg": iscsi_gateway_conf}}), @@ -331,7 +331,7 @@ class TestMonitoring: 'deploy', [ '--name', 'alertmanager.test', - '--meta-json', '{"service_name": "alertmanager", "ports": [9093, 9094], "ip": null, "deployed_by": [], "rank": null, "rank_generation": null, "extra_container_args": null}', + '--meta-json', '{"service_name": "alertmanager", "ports": [9093, 9094], "ip": null, "deployed_by": [], "rank": null, "rank_generation": null, "extra_container_args": null, "extra_entrypoint_args": null}', '--config-json', '-', '--tcp-ports', '9093 9094' ], stdin=json.dumps({"files": {"alertmanager.yml": y}, "peers": []}), @@ -355,7 +355,7 @@ class TestMonitoring: [ '--name', 'alertmanager.test', '--meta-json', - '{"service_name": "alertmanager", "ports": [9093, 9094], "ip": null, "deployed_by": [], "rank": null, "rank_generation": null, "extra_container_args": null}', + '{"service_name": "alertmanager", "ports": [9093, 9094], "ip": null, "deployed_by": [], "rank": null, "rank_generation": null, "extra_container_args": null, "extra_entrypoint_args": null}', '--config-json', '-', '--tcp-ports', '9093 9094' ], stdin=json.dumps({"files": {"alertmanager.yml": y}, "peers": []}), @@ -381,7 +381,7 @@ class TestMonitoring: [ '--name', 'alertmanager.test', '--meta-json', - '{"service_name": "alertmanager", "ports": [9093, 9094], "ip": null, "deployed_by": [], "rank": null, "rank_generation": null, "extra_container_args": null}', + '{"service_name": "alertmanager", "ports": [9093, 9094], "ip": null, "deployed_by": [], "rank": null, "rank_generation": null, "extra_container_args": null, "extra_entrypoint_args": null}', '--config-json', '-', '--tcp-ports', '9093 9094' ], stdin=json.dumps({"files": {"alertmanager.yml": y}, "peers": []}), @@ -403,7 +403,7 @@ class TestMonitoring: 'deploy', [ '--name', 'alertmanager.test', - '--meta-json', '{"service_name": "alertmanager", "ports": [9093, 9094], "ip": null, "deployed_by": [], "rank": null, "rank_generation": null, "extra_container_args": null}', + '--meta-json', '{"service_name": "alertmanager", "ports": [9093, 9094], "ip": null, "deployed_by": [], "rank": null, "rank_generation": null, "extra_container_args": null, "extra_entrypoint_args": null}', '--config-json', '-', '--tcp-ports', '9093 9094' ], stdin=json.dumps({"files": {"alertmanager.yml": y}, "peers": []}), @@ -428,7 +428,7 @@ class TestMonitoring: [ '--name', 'alertmanager.test', '--meta-json', - '{"service_name": "alertmanager", "ports": [9093, 9094], "ip": null, "deployed_by": [], "rank": null, "rank_generation": null, "extra_container_args": null}', + '{"service_name": "alertmanager", "ports": [9093, 9094], "ip": null, "deployed_by": [], "rank": null, "rank_generation": null, "extra_container_args": null, "extra_entrypoint_args": null}', '--config-json', '-', '--tcp-ports', '9093 9094' ], stdin=json.dumps({"files": {"alertmanager.yml": y}, "peers": []}), @@ -471,7 +471,8 @@ class TestMonitoring: [ '--name', 'prometheus.test', '--meta-json', - '{"service_name": "prometheus", "ports": [9095], "ip": null, "deployed_by": [], "rank": null, "rank_generation": null, "extra_container_args": null}', + ('{"service_name": "prometheus", "ports": [9095], "ip": null, "deployed_by": [], "rank": null, ' + '"rank_generation": null, "extra_container_args": null, "extra_entrypoint_args": null}'), '--config-json', '-', '--tcp-ports', '9095' ], @@ -543,7 +544,8 @@ class TestMonitoring: [ '--name', 'grafana.test', '--meta-json', - '{"service_name": "grafana", "ports": [3000], "ip": null, "deployed_by": [], "rank": null, "rank_generation": null, "extra_container_args": null}', + ('{"service_name": "grafana", "ports": [3000], "ip": null, "deployed_by": [], "rank": null, ' + '"rank_generation": null, "extra_container_args": null, "extra_entrypoint_args": null}'), '--config-json', '-', '--tcp-ports', '3000'], stdin=json.dumps({"files": files}), image='') @@ -615,7 +617,8 @@ spec: _run_cephadm.assert_called_with( 'test', 'alertmanager.test', 'deploy', [ '--name', 'alertmanager.test', - '--meta-json', '{"service_name": "alertmanager", "ports": [4200, 9094], "ip": null, "deployed_by": [], "rank": null, "rank_generation": null, "extra_container_args": null}', + '--meta-json', ('{"service_name": "alertmanager", "ports": [4200, 9094], "ip": null, "deployed_by": [], "rank": null, ' + '"rank_generation": null, "extra_container_args": null, "extra_entrypoint_args": null}'), '--config-json', '-', '--tcp-ports', '4200 9094', '--reconfig' @@ -689,7 +692,8 @@ class TestSNMPGateway: [ '--name', 'snmp-gateway.test', '--meta-json', - '{"service_name": "snmp-gateway", "ports": [9464], "ip": null, "deployed_by": [], "rank": null, "rank_generation": null, "extra_container_args": null}', + ('{"service_name": "snmp-gateway", "ports": [9464], "ip": null, "deployed_by": [], "rank": null, ' + '"rank_generation": null, "extra_container_args": null, "extra_entrypoint_args": null}'), '--config-json', '-', '--tcp-ports', '9464' ], @@ -724,7 +728,8 @@ class TestSNMPGateway: [ '--name', 'snmp-gateway.test', '--meta-json', - '{"service_name": "snmp-gateway", "ports": [9465], "ip": null, "deployed_by": [], "rank": null, "rank_generation": null, "extra_container_args": null}', + ('{"service_name": "snmp-gateway", "ports": [9465], "ip": null, "deployed_by": [], "rank": null, ' + '"rank_generation": null, "extra_container_args": null, "extra_entrypoint_args": null}'), '--config-json', '-', '--tcp-ports', '9465' ], @@ -763,7 +768,8 @@ class TestSNMPGateway: [ '--name', 'snmp-gateway.test', '--meta-json', - '{"service_name": "snmp-gateway", "ports": [9464], "ip": null, "deployed_by": [], "rank": null, "rank_generation": null, "extra_container_args": null}', + ('{"service_name": "snmp-gateway", "ports": [9464], "ip": null, "deployed_by": [], "rank": null, ' + '"rank_generation": null, "extra_container_args": null, "extra_entrypoint_args": null}'), '--config-json', '-', '--tcp-ports', '9464' ], @@ -807,7 +813,8 @@ class TestSNMPGateway: [ '--name', 'snmp-gateway.test', '--meta-json', - '{"service_name": "snmp-gateway", "ports": [9464], "ip": null, "deployed_by": [], "rank": null, "rank_generation": null, "extra_container_args": null}', + ('{"service_name": "snmp-gateway", "ports": [9464], "ip": null, "deployed_by": [], "rank": null, ' + '"rank_generation": null, "extra_container_args": null, "extra_entrypoint_args": null}'), '--config-json', '-', '--tcp-ports', '9464' ], diff --git a/src/pybind/mgr/orchestrator/_interface.py b/src/pybind/mgr/orchestrator/_interface.py index 2208a587495..148b8990968 100644 --- a/src/pybind/mgr/orchestrator/_interface.py +++ b/src/pybind/mgr/orchestrator/_interface.py @@ -861,6 +861,7 @@ class DaemonDescription(object): rank: Optional[int] = None, rank_generation: Optional[int] = None, extra_container_args: Optional[List[str]] = None, + extra_entrypoint_args: Optional[List[str]] = None, ) -> None: #: Host is at the same granularity as InventoryHost @@ -926,6 +927,7 @@ class DaemonDescription(object): self.is_active = is_active self.extra_container_args = extra_container_args + self.extra_entrypoint_args = extra_entrypoint_args @property def status(self) -> Optional[DaemonDescriptionStatus]: