From d6d130d2b42d978cb2112da3d40fa6982e5471da Mon Sep 17 00:00:00 2001 From: John Mulligan Date: Thu, 15 Jun 2023 15:54:09 -0400 Subject: [PATCH] cephadm: remove call to get_parm from fetch_configs Stop using get_parm in fetch_configs. Doing so makes clear that the two if-branches in fetch_configs are symmetric, in the handling of custom_config_files and symmetric with the behavior of fetch_custom_config_files. It also reveals that get_parm only has one remaining caller making it simpler to remove get_parm in the future. Signed-off-by: John Mulligan --- src/cephadm/cephadm.py | 9 ++++++++- src/cephadm/tests/test_cephadm.py | 6 +++--- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/src/cephadm/cephadm.py b/src/cephadm/cephadm.py index bd5bb59422b..43d1f0a88d2 100755 --- a/src/cephadm/cephadm.py +++ b/src/cephadm/cephadm.py @@ -3056,14 +3056,21 @@ def fetch_configs(ctx: CephadmContext) -> Dict[str, str]: must not be part of a deployment's configuration key-value pairs. To access custom configuration file data, use `fetch_custom_config_files`. """ + # ctx.config_blobs is *always* a dict. it is created once when + # a command is parsed/processed and stored "forever" cfg_blobs = getattr(ctx, 'config_blobs', None) if cfg_blobs: cfg_blobs = dict(cfg_blobs) cfg_blobs.pop('custom_config_files', None) return cfg_blobs + # ctx.config_json is the legacy equivalent of config_blobs. it is a + # string that either contains json or refers to a file name where + # the file contains json. cfg_json = getattr(ctx, 'config_json', None) if cfg_json: - return get_parm(cfg_json) or {} + jdata = _get_config_json(cfg_json) or {} + jdata.pop('custom_config_files', None) + return jdata return {} diff --git a/src/cephadm/tests/test_cephadm.py b/src/cephadm/tests/test_cephadm.py index ae1c18249be..ed09f91d54c 100644 --- a/src/cephadm/tests/test_cephadm.py +++ b/src/cephadm/tests/test_cephadm.py @@ -306,7 +306,7 @@ class TestCephAdm(object): @mock.patch('cephadm.logger') @mock.patch('cephadm.FileLock') @mock.patch('cephadm.deploy_daemon') - @mock.patch('cephadm.get_parm') + @mock.patch('cephadm.fetch_configs') @mock.patch('cephadm.make_var_run') @mock.patch('cephadm.migrate_sysctl_dir') @mock.patch('cephadm.check_unit', lambda *args, **kwargs: (None, 'running', None)) @@ -314,7 +314,7 @@ class TestCephAdm(object): @mock.patch('cephadm.get_deployment_container') @mock.patch('cephadm.read_configuration_source', lambda c: {}) @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, _get_parm, _deploy_daemon, _file_lock, _logger): + def test_mon_crush_location(self, _get_deployment_container, _migrate_sysctl, _make_var_run, _fetch_configs, _deploy_daemon, _file_lock, _logger): """ test that crush location for mon is set if it is included in config_json """ @@ -328,7 +328,7 @@ class TestCephAdm(object): ctx.config_json = '-' ctx.osd_fsid = '0' ctx.tcp_ports = '3300 6789' - _get_parm.return_value = { + _fetch_configs.return_value = { 'crush_location': 'database=a' } -- 2.39.5