From 99162a9c9b811a47f0a08650a42cf71c595c4899 Mon Sep 17 00:00:00 2001 From: Redouane Kachach Date: Fri, 14 Oct 2022 12:01:47 +0200 Subject: [PATCH] mgr/cephadm: making grafana protocol configurable Fixes: https://tracker.ceph.com/issues/57816 Signed-off-by: Redouane Kachach --- doc/cephadm/services/monitoring.rst | 4 +++- src/pybind/mgr/cephadm/module.py | 6 ++++++ src/pybind/mgr/cephadm/services/monitoring.py | 4 +++- .../templates/services/grafana/grafana.ini.j2 | 2 +- src/pybind/mgr/cephadm/tests/test_spec.py | 3 ++- .../ceph/deployment/service_spec.py | 8 ++++++++ .../ceph/tests/test_service_spec.py | 20 ++++++++++++++++++- 7 files changed, 42 insertions(+), 5 deletions(-) diff --git a/doc/cephadm/services/monitoring.rst b/doc/cephadm/services/monitoring.rst index 5a0bdb385ed9b..4ffde25d5fa02 100644 --- a/doc/cephadm/services/monitoring.rst +++ b/doc/cephadm/services/monitoring.rst @@ -110,7 +110,8 @@ These two services are not deployed by default in a Ceph cluster. To enable the Networks and Ports ~~~~~~~~~~~~~~~~~~ -All monitoring services can have the network and port they bind to configured with a yaml service specification +All monitoring services can have the network and port they bind to configured with a yaml service specification. By default +cephadm will use ``https`` protocol when configuring Grafana daemons unless the user explicitly sets the protocol to ``http``. example spec file: @@ -124,6 +125,7 @@ example spec file: - 192.169.142.0/24 spec: port: 4200 + protocol: http Using custom images ~~~~~~~~~~~~~~~~~~~ diff --git a/src/pybind/mgr/cephadm/module.py b/src/pybind/mgr/cephadm/module.py index a226e808502cd..a22184ac13b5c 100644 --- a/src/pybind/mgr/cephadm/module.py +++ b/src/pybind/mgr/cephadm/module.py @@ -2488,6 +2488,12 @@ Then run the following: deps.append(self.get_active_mgr().name()) deps.append(str(self.get_module_option_ex('prometheus', 'server_port', 9283))) deps.append(str(self.service_discovery_port)) + # prometheus yaml configuration file (generated by prometheus.yml.j2) contains + # a scrape_configs section for each service type. This should be included only + # when at least one daemon of the corresponding service is running. Therefore, + # an explicit dependency is added for each service-type to force a reconfig + # whenever the number of daemons for those service-type changes from 0 to greater + # than zero and vice versa. deps += [s for s in ['node-exporter', 'alertmanager', 'ingress'] if self.cache.get_daemons_by_service(s)] else: need = { diff --git a/src/pybind/mgr/cephadm/services/monitoring.py b/src/pybind/mgr/cephadm/services/monitoring.py index d04605752fd28..52cc34a0113e3 100644 --- a/src/pybind/mgr/cephadm/services/monitoring.py +++ b/src/pybind/mgr/cephadm/services/monitoring.py @@ -57,6 +57,7 @@ class GrafanaService(CephadmService): 'services/grafana/grafana.ini.j2', { 'initial_admin_password': spec.initial_admin_password, 'http_port': daemon_spec.ports[0] if daemon_spec.ports else self.DEFAULT_SERVICE_PORT, + 'protocol': spec.protocol, 'http_addr': daemon_spec.ip if daemon_spec.ip else '' }) @@ -141,7 +142,8 @@ class GrafanaService(CephadmService): assert dd.hostname is not None addr = dd.ip if dd.ip else self._inventory_get_fqdn(dd.hostname) port = dd.ports[0] if dd.ports else self.DEFAULT_SERVICE_PORT - service_url = build_url(scheme='https', host=addr, port=port) + spec = cast(GrafanaSpec, self.mgr.spec_store[dd.service_name()].spec) + service_url = build_url(scheme=spec.protocol, host=addr, port=port) self._set_service_url_on_dashboard( 'Grafana', 'dashboard get-grafana-api-url', diff --git a/src/pybind/mgr/cephadm/templates/services/grafana/grafana.ini.j2 b/src/pybind/mgr/cephadm/templates/services/grafana/grafana.ini.j2 index e7e81d89a4b6f..9d74ccd039c30 100644 --- a/src/pybind/mgr/cephadm/templates/services/grafana/grafana.ini.j2 +++ b/src/pybind/mgr/cephadm/templates/services/grafana/grafana.ini.j2 @@ -7,7 +7,7 @@ org_role = 'Viewer' [server] domain = 'bootstrap.storage.lab' - protocol = https + protocol = {{ protocol }} cert_file = /etc/grafana/certs/cert_file cert_key = /etc/grafana/certs/cert_key http_port = {{ http_port }} diff --git a/src/pybind/mgr/cephadm/tests/test_spec.py b/src/pybind/mgr/cephadm/tests/test_spec.py index c89d3b6c1ca64..5e2a479d9f513 100644 --- a/src/pybind/mgr/cephadm/tests/test_spec.py +++ b/src/pybind/mgr/cephadm/tests/test_spec.py @@ -30,7 +30,8 @@ from orchestrator import DaemonDescription, OrchestratorError "placement": { "count": 1 }, - "service_type": "grafana" + "service_type": "grafana", + "protocol": "https" }, { "placement": { diff --git a/src/python-common/ceph/deployment/service_spec.py b/src/python-common/ceph/deployment/service_spec.py index 16db5ed7cc4e4..482a6cd4f65cd 100644 --- a/src/python-common/ceph/deployment/service_spec.py +++ b/src/python-common/ceph/deployment/service_spec.py @@ -1244,6 +1244,7 @@ class GrafanaSpec(MonitoringSpec): config: Optional[Dict[str, str]] = None, networks: Optional[List[str]] = None, port: Optional[int] = None, + protocol: Optional[str] = 'https', initial_admin_password: Optional[str] = None, extra_container_args: Optional[List[str]] = None, custom_configs: Optional[List[CustomConfig]] = None, @@ -1256,6 +1257,13 @@ class GrafanaSpec(MonitoringSpec): extra_container_args=extra_container_args, custom_configs=custom_configs) self.initial_admin_password = initial_admin_password + self.protocol = protocol + + def validate(self) -> None: + super(GrafanaSpec, self).validate() + if self.protocol not in ['http', 'https']: + err_msg = f"Invalid protocol '{self.protocol}'. Valid values are: 'http', 'https'." + raise SpecValidationError(err_msg) yaml.add_representer(GrafanaSpec, ServiceSpec.yaml_representer) diff --git a/src/python-common/ceph/tests/test_service_spec.py b/src/python-common/ceph/tests/test_service_spec.py index d3fb4329668bb..ac281865156ab 100644 --- a/src/python-common/ceph/tests/test_service_spec.py +++ b/src/python-common/ceph/tests/test_service_spec.py @@ -8,7 +8,7 @@ import pytest from ceph.deployment.service_spec import HostPlacementSpec, PlacementSpec, \ ServiceSpec, RGWSpec, NFSServiceSpec, IscsiServiceSpec, AlertManagerSpec, \ - CustomContainerSpec + CustomContainerSpec, GrafanaSpec from ceph.deployment.drive_group import DriveGroupSpec from ceph.deployment.hostspec import SpecValidationError @@ -44,6 +44,22 @@ def test_parse_host_placement_specs(test_input, expected, require_network): assert ret == HostPlacementSpec.from_json(ret.to_json()) +@pytest.mark.parametrize( + "spec, raise_exception, msg", + [ + (GrafanaSpec(protocol=''), True, '^Invalid protocol'), + (GrafanaSpec(protocol='invalid'), True, '^Invalid protocol'), + (GrafanaSpec(protocol='-http'), True, '^Invalid protocol'), + (GrafanaSpec(protocol='-https'), True, '^Invalid protocol'), + (GrafanaSpec(protocol='http'), False, ''), + (GrafanaSpec(protocol='https'), False, ''), + ]) +def test_apply_grafana(spec: GrafanaSpec, raise_exception: bool, msg: str): + if raise_exception: + with pytest.raises(SpecValidationError, match=msg): + spec.validate() + else: + spec.validate() @pytest.mark.parametrize( @@ -250,12 +266,14 @@ service_type: grafana service_name: grafana spec: port: 1234 + protocol: https --- service_type: grafana service_name: grafana spec: initial_admin_password: secure port: 1234 + protocol: https --- service_type: ingress service_id: rgw.foo -- 2.39.5