From: Adam King Date: Tue, 8 Nov 2022 19:09:05 +0000 (-0500) Subject: mgr/cephadm: support for extra entrypoint args X-Git-Tag: v18.1.0~488^2~2 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=db8e4462c968e1d29b0688efafad3442abb8779c;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 --- diff --git a/src/cephadm/cephadm.py b/src/cephadm/cephadm.py index 779f8eb9b83f..8013317a07c3 100755 --- a/src/cephadm/cephadm.py +++ b/src/cephadm/cephadm.py @@ -5881,6 +5881,8 @@ def get_deployment_container(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) if 'config_json' in ctx and ctx.config_json: conf_files = get_custom_config_files(ctx.config_json) mandatory_keys = ['mount_path', 'content'] @@ -9621,6 +9623,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/module.py b/src/pybind/mgr/cephadm/module.py index 94eecc9fef77..3e374a10cff1 100644 --- a/src/pybind/mgr/cephadm/module.py +++ b/src/pybind/mgr/cephadm/module.py @@ -821,6 +821,7 @@ class CephadmOrchestrator(orchestrator.Orchestrator, MgrModule, 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 = { diff --git a/src/pybind/mgr/cephadm/serve.py b/src/pybind/mgr/cephadm/serve.py index 2675f0cf5f1f..3a203fc146e4 100644 --- a/src/pybind/mgr/cephadm/serve.py +++ b/src/pybind/mgr/cephadm/serve.py @@ -981,6 +981,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: @@ -1211,13 +1217,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 daemon_spec.service_name in self.mgr.spec_store: configs = self.mgr.spec_store[daemon_spec.service_name].spec.custom_configs @@ -1243,7 +1243,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, @@ -1293,6 +1294,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 631c69dc5e52..5f763828237c 100644 --- a/src/pybind/mgr/cephadm/services/cephadmservice.py +++ b/src/pybind/mgr/cephadm/services/cephadmservice.py @@ -62,6 +62,7 @@ class CephadmDaemonDeploySpec: rank: Optional[int] = None, rank_generation: Optional[int] = None, extra_container_args: Optional[List[str]] = None, + extra_entrypoint_args: Optional[List[str]] = None, ): """ A data struction to encapsulate `cephadm deploy ... @@ -98,6 +99,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) @@ -127,6 +129,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: @@ -142,6 +145,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, ) @@ -215,6 +219,8 @@ class CephadmService(metaclass=ABCMeta): rank_generation=rank_generation, extra_container_args=spec.extra_container_args if hasattr( spec, 'extra_container_args') else None, + extra_entrypoint_args=spec.extra_entrypoint_args if hasattr( + spec, 'extra_entrypoint_args') else None, ) 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 3ae0345b88c5..9c3e820f6c54 100644 --- a/src/pybind/mgr/cephadm/tests/test_cephadm.py +++ b/src/pybind/mgr/cephadm/tests/test_cephadm.py @@ -453,7 +453,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', ], @@ -469,7 +470,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' @@ -478,6 +480,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.side_effect = async_side_effect(('{}', '', 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.side_effect = async_side_effect(('{}', '', 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") def test_custom_config(self, _run_cephadm, cephadm_module: CephadmOrchestrator): _run_cephadm.side_effect = async_side_effect(('{}', '', 0)) @@ -502,7 +553,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": null}', + '--meta-json', ('{"service_name": "crash", "ports": [], "ip": null, "deployed_by": [], "rank": null, ' + '"rank_generation": null, "extra_container_args": null, "extra_entrypoint_args": null}'), '--config-json', '-', ], stdin=stdin_str, diff --git a/src/pybind/mgr/cephadm/tests/test_services.py b/src/pybind/mgr/cephadm/tests/test_services.py index b34e7f12f61e..a06e73148aa4 100644 --- a/src/pybind/mgr/cephadm/tests/test_services.py +++ b/src/pybind/mgr/cephadm/tests/test_services.py @@ -289,7 +289,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": f"[client.iscsi.{iscsi_daemon_id}]\nkey = None\n", "files": {"iscsi-gateway.cfg": iscsi_gateway_conf}}), @@ -380,7 +380,8 @@ 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", @@ -432,7 +433,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' ], @@ -486,7 +488,8 @@ class TestMonitoring: [ '--name', 'loki.test', '--meta-json', - '{"service_name": "loki", "ports": [3100], "ip": null, "deployed_by": [], "rank": null, "rank_generation": null, "extra_container_args": null}', + ('{"service_name": "loki", "ports": [3100], "ip": null, "deployed_by": [], "rank": null, ' + '"rank_generation": null, "extra_container_args": null, "extra_entrypoint_args": null}'), '--config-json', '-', '--tcp-ports', '3100' ], @@ -527,7 +530,8 @@ class TestMonitoring: [ '--name', 'promtail.test', '--meta-json', - '{"service_name": "promtail", "ports": [9080], "ip": null, "deployed_by": [], "rank": null, "rank_generation": null, "extra_container_args": null}', + ('{"service_name": "promtail", "ports": [9080], "ip": null, "deployed_by": [], "rank": null, ' + '"rank_generation": null, "extra_container_args": null, "extra_entrypoint_args": null}'), '--config-json', '-', '--tcp-ports', '9080' ], @@ -611,7 +615,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='') @@ -691,7 +696,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' @@ -762,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' ], @@ -797,7 +804,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' ], @@ -836,7 +844,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' ], @@ -880,7 +889,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' ], @@ -1295,7 +1305,8 @@ class TestJaeger: [ '--name', 'jaeger-query.test', '--meta-json', - '{"service_name": "jaeger-query", "ports": [16686], "ip": null, "deployed_by": [], "rank": null, "rank_generation": null, "extra_container_args": null}', + ('{"service_name": "jaeger-query", "ports": [16686], "ip": null, "deployed_by": [], "rank": null, ' + '"rank_generation": null, "extra_container_args": null, "extra_entrypoint_args": null}'), '--config-json', '-', '--tcp-ports', '16686' @@ -1323,7 +1334,8 @@ class TestJaeger: [ '--name', 'elasticsearch.test', '--meta-json', - '{"service_name": "elasticsearch", "ports": [9200], "ip": null, "deployed_by": [], "rank": null, "rank_generation": null, "extra_container_args": null}', + ('{"service_name": "elasticsearch", "ports": [9200], "ip": null, "deployed_by": [], "rank": null, ' + '"rank_generation": null, "extra_container_args": null, "extra_entrypoint_args": null}'), '--config-json', '-', '--tcp-ports', '9200' @@ -1339,7 +1351,8 @@ class TestJaeger: [ '--name', 'jaeger-collector.test', '--meta-json', - '{"service_name": "jaeger-collector", "ports": [14250], "ip": null, "deployed_by": [], "rank": null, "rank_generation": null, "extra_container_args": null}', + ('{"service_name": "jaeger-collector", "ports": [14250], "ip": null, "deployed_by": [], "rank": null, ' + '"rank_generation": null, "extra_container_args": null, "extra_entrypoint_args": null}'), '--config-json', '-', '--tcp-ports', '14250' @@ -1367,7 +1380,8 @@ class TestJaeger: [ '--name', 'jaeger-collector.test', '--meta-json', - '{"service_name": "jaeger-collector", "ports": [14250], "ip": null, "deployed_by": [], "rank": null, "rank_generation": null, "extra_container_args": null}', + ('{"service_name": "jaeger-collector", "ports": [14250], "ip": null, "deployed_by": [], "rank": null, ' + '"rank_generation": null, "extra_container_args": null, "extra_entrypoint_args": null}'), '--config-json', '-', '--tcp-ports', '14250' @@ -1383,7 +1397,8 @@ class TestJaeger: [ '--name', 'jaeger-agent.test', '--meta-json', - '{"service_name": "jaeger-agent", "ports": [6799], "ip": null, "deployed_by": [], "rank": null, "rank_generation": null, "extra_container_args": null}', + ('{"service_name": "jaeger-agent", "ports": [6799], "ip": null, "deployed_by": [], "rank": null, ' + '"rank_generation": null, "extra_container_args": null, "extra_entrypoint_args": null}'), '--config-json', '-', '--tcp-ports', '6799' diff --git a/src/pybind/mgr/orchestrator/_interface.py b/src/pybind/mgr/orchestrator/_interface.py index 3b92e50f927b..6d9aab4c74e6 100644 --- a/src/pybind/mgr/orchestrator/_interface.py +++ b/src/pybind/mgr/orchestrator/_interface.py @@ -908,6 +908,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 @@ -973,6 +974,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]: