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 <adking@redhat.com>
(cherry picked from commit ffec37156ffea174292ccabe6ede524333b30789)

diff --git a/src/cephadm/cephadm b/src/cephadm/cephadm
index 4a56339dbd2a67bf53aefbd2a62dd94aaec2aa28..dc592a8fb92ae7daaa8da2e767335fa2155a01f3 100755 (executable)
--- 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 1900de67e5f5acfb480d00b2ed96d204e8531738..7559402d3983b6b4a9f9565d2ba8ac0d32006709 100644 (file)
--- 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 a6e4adae0958b05df2e0305534c721709c9ce6d7..ee3ae224461a580ee028bfb5bb8b3c4c9b5c5829 100644 (file)
--- 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 1d17b267e3f354661a752cc84326a093d60eb7ad..fca9c33c43421773e43dd58847c9949e33b1c158 100644 (file)
--- 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))