From 4a99b771a4a59671728e072bb27270bba8cb78c8 Mon Sep 17 00:00:00 2001 From: Michael Fritch Date: Thu, 8 Jul 2021 19:35:42 -0600 Subject: [PATCH 1/2] cephadm: use CephadmContext rather than MagicMock MagicMock hides attribute errors: ``` self = , name = 'config_json' def __getattr__(self, name: str) -> Any: if '_conf' in self.__dict__ and hasattr(self._conf, name): return getattr(self._conf, name) elif '_args' in self.__dict__ and hasattr(self._args, name): return getattr(self._args, name) else: > return super().__getattribute__(name) E AttributeError: 'CephadmContext' object has no attribute 'config_json' ``` Signed-off-by: Michael Fritch --- src/cephadm/cephadm | 4 +++- src/cephadm/tests/test_cephadm.py | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/cephadm/cephadm b/src/cephadm/cephadm index 4b8aff238f4..4c02704c088 100755 --- a/src/cephadm/cephadm +++ b/src/cephadm/cephadm @@ -2196,7 +2196,9 @@ def create_daemon_dirs(ctx, fsid, daemon_type, daemon_id, uid, gid, f.write(keyring) if daemon_type in Monitoring.components.keys(): - config_json: Dict[str, Any] = get_parm(ctx.config_json) + config_json: Dict[str, Any] = dict() + if 'config_json' in ctx: + config_json = get_parm(ctx.config_json) # Set up directories specific to the monitoring component config_dir = '' diff --git a/src/cephadm/tests/test_cephadm.py b/src/cephadm/tests/test_cephadm.py index cbd1e1cea8c..f664ac4c482 100644 --- a/src/cephadm/tests/test_cephadm.py +++ b/src/cephadm/tests/test_cephadm.py @@ -995,7 +995,7 @@ class TestMonitoring(object): daemon_type = 'prometheus' uid, gid = 50, 50 daemon_id = 'home' - ctx = mock.Mock() + ctx = cd.CephadmContext() ctx.data_dir = '/somedir' files = { 'files': { From f853ce7e9a52b4abd0f6626ca13886bd0e2e36a6 Mon Sep 17 00:00:00 2001 From: Michael Fritch Date: Fri, 9 Jul 2021 13:35:52 -0600 Subject: [PATCH 2/2] cephadm: use pyfakefs during `test_create_daemon_dirs_prometheus` convert test to use the `cephadm_fs` fixture Signed-off-by: Michael Fritch --- src/cephadm/tests/test_cephadm.py | 34 ++++++++++++++----------------- 1 file changed, 15 insertions(+), 19 deletions(-) diff --git a/src/cephadm/tests/test_cephadm.py b/src/cephadm/tests/test_cephadm.py index f664ac4c482..66e6e3b4140 100644 --- a/src/cephadm/tests/test_cephadm.py +++ b/src/cephadm/tests/test_cephadm.py @@ -1,6 +1,7 @@ # type: ignore import errno +import json import mock import os import pytest @@ -977,14 +978,7 @@ class TestMonitoring(object): version = cd.Monitoring.get_version(ctx, 'container_id', daemon_type) assert version == '0.16.1' - @mock.patch('cephadm.os.fchown') - @mock.patch('cephadm.get_parm') - @mock.patch('cephadm.makedirs') - @mock.patch('cephadm.open') - @mock.patch('cephadm.make_log_dir') - @mock.patch('cephadm.make_data_dir') - def test_create_daemon_dirs_prometheus(self, make_data_dir, make_log_dir, _open, makedirs, - get_parm, fchown): + def test_create_daemon_dirs_prometheus(self, cephadm_fs): """ Ensures the required and optional files given in the configuration are created and mapped correctly inside the container. Tests absolute and @@ -997,13 +991,12 @@ class TestMonitoring(object): daemon_id = 'home' ctx = cd.CephadmContext() ctx.data_dir = '/somedir' - files = { + ctx.config_json = json.dumps({ 'files': { 'prometheus.yml': 'foo', '/etc/prometheus/alerting/ceph_alerts.yml': 'bar' } - } - get_parm.return_value = files + }) cd.create_daemon_dirs(ctx, fsid, @@ -1020,14 +1013,17 @@ class TestMonitoring(object): daemon_type=daemon_type, daemon_id=daemon_id ) - assert _open.call_args_list == [ - mock.call('{}/etc/prometheus/prometheus.yml'.format(prefix), 'w', - encoding='utf-8'), - mock.call('{}/etc/prometheus/alerting/ceph_alerts.yml'.format(prefix), 'w', - encoding='utf-8'), - ] - assert mock.call().__enter__().write('foo') in _open.mock_calls - assert mock.call().__enter__().write('bar') in _open.mock_calls + + expected = { + 'etc/prometheus/prometheus.yml': 'foo', + 'etc/prometheus/alerting/ceph_alerts.yml': 'bar', + } + + for file,content in expected.items(): + file = os.path.join(prefix, file) + assert os.path.exists(file) + with open(file) as f: + assert f.read() == content class TestBootstrap(object):