From dc4b3ee8e21756ff2d31bddd6810637f346a1fa8 Mon Sep 17 00:00:00 2001 From: Adam King Date: Thu, 17 Nov 2022 06:42:33 -0500 Subject: [PATCH] mgr/cephadm: set --set-crush-location for mons based on crush location in spec Necessary to do this for stretch mode tiebreaker mon replacement Fixes: https://tracker.ceph.com/issues/58101 Signed-off-by: Adam King (cherry picked from commit ffec37156ffea174292ccabe6ede524333b30789) --- src/cephadm/cephadm | 13 +++++ src/cephadm/tests/test_cephadm.py | 51 +++++++++++++++++++ .../mgr/cephadm/services/cephadmservice.py | 17 ++++++- src/pybind/mgr/cephadm/tests/test_cephadm.py | 33 ++++++++++++ 4 files changed, 113 insertions(+), 1 deletion(-) diff --git a/src/cephadm/cephadm b/src/cephadm/cephadm index 4a56339dbd2a6..dc592a8fb92ae 100755 --- a/src/cephadm/cephadm +++ b/src/cephadm/cephadm @@ -5962,8 +5962,21 @@ def command_deploy(ctx): uid, gid = extract_uid_gid(ctx) make_var_run(ctx, ctx.fsid, uid, gid) + config_json: Optional[Dict[str, str]] = None + if 'config_json' in ctx and ctx.config_json: + config_json = get_parm(ctx.config_json) + c = get_deployment_container(ctx, ctx.fsid, daemon_type, daemon_id, ptrace=ctx.allow_ptrace) + + if daemon_type == 'mon' and config_json is not None: + if 'crush_location' in config_json: + c_loc = config_json['crush_location'] + # was originally "c.args.extend(['--set-crush-location', c_loc])" + # but that doesn't seem to persist in the object after it's passed + # in further function calls + c.args = c.args + ['--set-crush-location', c_loc] + deploy_daemon(ctx, ctx.fsid, daemon_type, daemon_id, c, uid, gid, config=config, keyring=keyring, osd_fsid=ctx.osd_fsid, diff --git a/src/cephadm/tests/test_cephadm.py b/src/cephadm/tests/test_cephadm.py index 1900de67e5f5a..7559402d3983b 100644 --- a/src/cephadm/tests/test_cephadm.py +++ b/src/cephadm/tests/test_cephadm.py @@ -292,6 +292,57 @@ class TestCephAdm(object): assert os.path.join('data', '9b9d7609-f4d5-4aba-94c8-effa764d96c9', 'custom_config_files', 'grafana.host1', 'testing.str') in c.volume_mounts assert c.volume_mounts[os.path.join('data', '9b9d7609-f4d5-4aba-94c8-effa764d96c9', 'custom_config_files', 'grafana.host1', 'testing.str')] == '/etc/testing.str' + @mock.patch('cephadm.logger') + @mock.patch('cephadm.FileLock') + @mock.patch('cephadm.deploy_daemon') + @mock.patch('cephadm.get_parm') + @mock.patch('cephadm.make_var_run') + @mock.patch('cephadm.migrate_sysctl_dir') + @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.get_deployment_container') + def test_mon_crush_location(self, _get_deployment_container, _migrate_sysctl, _make_var_run, _get_parm, _deploy_daemon, _file_lock, _logger): + """ + test that crush location for mon is set if it is included in config_json + """ + + ctx = cd.CephadmContext() + ctx.name = 'mon.test' + ctx.fsid = '9b9d7609-f4d5-4aba-94c8-effa764d96c9' + ctx.reconfig = False + ctx.container_engine = mock_docker() + ctx.allow_ptrace = True + ctx.config_json = '-' + ctx.osd_fsid = '0' + _get_parm.return_value = { + 'crush_location': 'database=a' + } + + _get_deployment_container.return_value = cd.CephContainer.for_daemon( + ctx, + fsid='9b9d7609-f4d5-4aba-94c8-effa764d96c9', + daemon_type='mon', + daemon_id='test', + entrypoint='', + args=[], + container_args=[], + volume_mounts={}, + bind_mounts=[], + envs=[], + privileged=False, + ptrace=False, + host_network=True, + ) + + def _crush_location_checker(ctx, fsid, daemon_type, daemon_id, container, uid, gid, **kwargs): + print(container.args) + raise Exception(' '.join(container.args)) + + _deploy_daemon.side_effect = _crush_location_checker + + with pytest.raises(Exception, match='--set-crush-location database=a'): + cd.command_deploy(ctx) + @mock.patch('cephadm.logger') @mock.patch('cephadm.get_custom_config_files') def test_write_custom_conf_files(self, _get_config, logger, cephadm_fs): diff --git a/src/pybind/mgr/cephadm/services/cephadmservice.py b/src/pybind/mgr/cephadm/services/cephadmservice.py index a6e4adae0958b..ee3ae224461a5 100644 --- a/src/pybind/mgr/cephadm/services/cephadmservice.py +++ b/src/pybind/mgr/cephadm/services/cephadmservice.py @@ -10,7 +10,7 @@ from typing import TYPE_CHECKING, List, Callable, TypeVar, \ from mgr_module import HandleCommandResult, MonCommandFailed -from ceph.deployment.service_spec import ServiceSpec, RGWSpec +from ceph.deployment.service_spec import ServiceSpec, RGWSpec, MONSpec from ceph.deployment.utils import is_ipv6, unwrap_ipv6 from mgr_util import build_url from orchestrator import OrchestratorError, DaemonDescription, DaemonDescriptionStatus @@ -642,6 +642,21 @@ class MonService(CephService): # super().post_remove(daemon) pass + def generate_config(self, daemon_spec: CephadmDaemonDeploySpec) -> Tuple[Dict[str, Any], List[str]]: + daemon_spec.final_config, daemon_spec.deps = super().generate_config(daemon_spec) + + mon_spec = cast(MONSpec, self.mgr.spec_store[daemon_spec.service_name].spec) + if mon_spec.crush_locations: + if daemon_spec.host in mon_spec.crush_locations: + # the --crush-location flag only supports a single bucker=loc pair so + # others will have to be handled later. The idea is to set the flag + # for the first bucket=loc pair in the list in order to facilitate + # replacing a tiebreaker mon (https://docs.ceph.com/en/quincy/rados/operations/stretch-mode/#other-commands) + c_loc = mon_spec.crush_locations[daemon_spec.host][0] + daemon_spec.final_config['crush_location'] = c_loc + + return daemon_spec.final_config, daemon_spec.deps + class MgrService(CephService): TYPE = 'mgr' diff --git a/src/pybind/mgr/cephadm/tests/test_cephadm.py b/src/pybind/mgr/cephadm/tests/test_cephadm.py index 1d17b267e3f35..fca9c33c43421 100644 --- a/src/pybind/mgr/cephadm/tests/test_cephadm.py +++ b/src/pybind/mgr/cephadm/tests/test_cephadm.py @@ -462,6 +462,39 @@ class TestCephadm(object): + '"keyring": "", "files": {"config": "[mon.test]\\npublic network = 127.0.0.0/8\\n"}}', image='') + @mock.patch("cephadm.serve.CephadmServe._run_cephadm") + def test_mon_crush_location_deployment(self, _run_cephadm, cephadm_module: CephadmOrchestrator): + _run_cephadm.side_effect = async_side_effect(('{}', '', 0)) + + with with_host(cephadm_module, 'test'): + cephadm_module.check_mon_command({ + 'prefix': 'config set', + 'who': 'mon', + 'name': 'public_network', + 'value': '127.0.0.0/8' + }) + + cephadm_module.cache.update_host_networks( + 'test', + { + "127.0.0.0/8": [ + "127.0.0.1" + ], + } + ) + + with with_service(cephadm_module, ServiceSpec(service_type='mon', crush_locations={'test': ['datacenter=a', 'rack=2']}), CephadmOrchestrator.apply_mon, 'test'): + _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, "extra_entrypoint_args": null}', + '--config-json', '-', + ], + stdin=('{"config": "[mon.test]\\npublic network = 127.0.0.0/8\\n", "keyring": "", ' + '"files": {"config": "[mon.test]\\npublic network = 127.0.0.0/8\\n"}, "crush_location": "datacenter=a"}'), + image='', + ) + @mock.patch("cephadm.serve.CephadmServe._run_cephadm") def test_extra_container_args(self, _run_cephadm, cephadm_module: CephadmOrchestrator): _run_cephadm.side_effect = async_side_effect(('{}', '', 0)) -- 2.39.5