Skip to content

Commit

Permalink
mgr/cephadm: fix issue with missing prometheus alerts
Browse files Browse the repository at this point in the history
Files passed as configuration to the cephadm binary had not been created
and mapped to the container, if those files weren't included in the
required files section inside cephadm. This prevented optional file
includes in the configuration.

The configuration file for the Prometheus default alerts is not
mandatory and hence wasn't included in the required files section, still it
needs to be added to the container by cephadm.

This change enables optional files to be included in the configuration
for monitoring components, so that those files are created and mapped
within the container.

Note that a `required_files` variable has been removed at one position
in these changes, though it wasn't used to ensure that required files
were included in the configuration at that point anyway. The test which
ensures that all required files are passed is somewhere else.

Fixes: https://tracker.ceph.com/issues/49856

Signed-off-by: Patrick Seidensal <[email protected]>
  • Loading branch information
p-se committed Mar 17, 2021
1 parent ba56112 commit 38f9846
Show file tree
Hide file tree
Showing 2 changed files with 57 additions and 4 deletions.
11 changes: 7 additions & 4 deletions src/cephadm/cephadm
Original file line number Diff line number Diff line change
Expand Up @@ -2117,7 +2117,6 @@ def create_daemon_dirs(ctx, fsid, daemon_type, daemon_id, uid, gid,

if daemon_type in Monitoring.components.keys():
config_json: Dict[str, Any] = get_parm(ctx.config_json)
required_files = Monitoring.components[daemon_type].get('config-json-files', list())

# Set up directories specific to the monitoring component
config_dir = ''
Expand Down Expand Up @@ -2145,10 +2144,14 @@ def create_daemon_dirs(ctx, fsid, daemon_type, daemon_id, uid, gid,
makedirs(os.path.join(data_dir_root, config_dir, 'data'), uid, gid, 0o755)

# populate the config directory for the component from the config-json
for fname in required_files:
if 'files' in config_json: # type: ignore
if 'files' in config_json:
for fname in config_json['files']:
content = dict_get_join(config_json['files'], fname)
with open(os.path.join(data_dir_root, config_dir, fname), 'w') as f:
if os.path.isabs(fname):
fpath = os.path.join(data_dir_root, fname.lstrip(os.path.sep))
else:
fpath = os.path.join(data_dir_root, config_dir, fname)
with open(fpath, 'w') as f:
os.fchown(f.fileno(), uid, gid)
os.fchmod(f.fileno(), 0o600)
f.write(content)
Expand Down
50 changes: 50 additions & 0 deletions src/cephadm/tests/test_cephadm.py
Original file line number Diff line number Diff line change
Expand Up @@ -861,3 +861,53 @@ def test_get_version_node_exporter(self, _call):
_call.return_value = '', '{}, version 0.16.1'.format(daemon_type.replace('-', '_')), 0
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):
"""
Ensures the required and optional files given in the configuration are
created and mapped correctly inside the container. Tests absolute and
relative file paths given in the configuration.
"""

fsid = 'aaf5a720-13fe-4a3b-82b9-2d99b7fd9704'
daemon_type = 'prometheus'
uid, gid = 50, 50
daemon_id = 'home'
ctx = mock.Mock()
ctx.data_dir = '/somedir'
files = {
'files': {
'prometheus.yml': 'foo',
'/etc/prometheus/alerting/ceph_alerts.yml': 'bar'
}
}
get_parm.return_value = files

cd.create_daemon_dirs(ctx,
fsid,
daemon_type,
daemon_id,
uid,
gid,
config=None,
keyring=None)

prefix = '{data_dir}/{fsid}/{daemon_type}.{daemon_id}'.format(
data_dir=ctx.data_dir,
fsid=fsid,
daemon_type=daemon_type,
daemon_id=daemon_id
)
assert _open.call_args_list == [
call('{}/etc/prometheus/prometheus.yml'.format(prefix), 'w'),
call('{}/etc/prometheus/alerting/ceph_alerts.yml'.format(prefix), 'w'),
]
assert call().__enter__().write('foo') in _open.mock_calls
assert call().__enter__().write('bar') in _open.mock_calls

0 comments on commit 38f9846

Please sign in to comment.