From a569df75794cf1eb5f504ea88faea7ceba030630 Mon Sep 17 00:00:00 2001 From: beierlm Date: Fri, 25 Aug 2023 23:03:47 +0200 Subject: [PATCH] Revert "Integrate NBI and Prometheus" This reverts commit 38f5d5834d610e011b3608fe8cc34b927775204a. Reason for revert: Grafana-k8s requires Juju 3.1 and we are not ready to change Juju versions at this time Change-Id: I90dbb6e1c2921289ecaabfa636878763f981c3e1 Signed-off-by: Mark Beierl --- installers/charm/osm-nbi/config.yaml | 3 - installers/charm/osm-nbi/metadata.yaml | 3 + installers/charm/osm-nbi/src/charm.py | 25 +++---- .../charm/osm-nbi/src/legacy_interfaces.py | 62 +++++++++++++++++ .../osm-nbi/tests/integration/test_charm.py | 67 ++++++++++--------- .../charm/osm-nbi/tests/unit/test_charm.py | 54 +++------------ 6 files changed, 117 insertions(+), 97 deletions(-) diff --git a/installers/charm/osm-nbi/config.yaml b/installers/charm/osm-nbi/config.yaml index bed55dad..d2c8c628 100644 --- a/installers/charm/osm-nbi/config.yaml +++ b/installers/charm/osm-nbi/config.yaml @@ -64,9 +64,6 @@ options: tls-secret-name: description: TLS secret name to use for ingress. type: string - prometheus-url: - description: Prometheus URL - type: string # Debug-mode options debug-mode: diff --git a/installers/charm/osm-nbi/metadata.yaml b/installers/charm/osm-nbi/metadata.yaml index b7902e0c..8a336c8e 100644 --- a/installers/charm/osm-nbi/metadata.yaml +++ b/installers/charm/osm-nbi/metadata.yaml @@ -67,6 +67,9 @@ requires: keystone: interface: keystone limit: 1 + prometheus: + interface: prometheus + limit: 1 ingress: interface: ingress limit: 1 diff --git a/installers/charm/osm-nbi/src/charm.py b/installers/charm/osm-nbi/src/charm.py index 4a0bbf16..b19beae8 100755 --- a/installers/charm/osm-nbi/src/charm.py +++ b/installers/charm/osm-nbi/src/charm.py @@ -29,7 +29,6 @@ See more: https://charmhub.io/osm import logging from typing import Any, Dict -from urllib.parse import urlparse from charms.data_platform_libs.v0.data_interfaces import DatabaseRequires from charms.kafka_k8s.v0.kafka import KafkaEvents, KafkaRequires @@ -49,7 +48,7 @@ from ops.framework import StoredState from ops.main import main from ops.model import ActiveStatus, Container -from legacy_interfaces import KeystoneClient +from legacy_interfaces import KeystoneClient, PrometheusClient HOSTPATHS = [ HostPath( @@ -87,6 +86,7 @@ class OsmNbiCharm(CharmBase): self.mongodb_client = DatabaseRequires( self, "mongodb", database_name="osm", extra_user_roles="admin" ) + self.prometheus_client = PrometheusClient(self, "prometheus") self.keystone_client = KeystoneClient(self, "keystone") self._observe_charm_events() self.container: Container = self.unit.get_container("nbi") @@ -129,7 +129,6 @@ class OsmNbiCharm(CharmBase): def _on_update_status(self, _=None) -> None: """Handler for the update-status event.""" try: - self._validate_config() self._check_relations() if self.debug_mode.started: return @@ -185,12 +184,13 @@ class OsmNbiCharm(CharmBase): self.on["kafka"].relation_broken: self._on_required_relation_broken, self.mongodb_client.on.database_created: self._on_config_changed, self.on["mongodb"].relation_broken: self._on_required_relation_broken, - self.on["keystone"].relation_changed: self._on_config_changed, - self.on["keystone"].relation_broken: self._on_required_relation_broken, # Action events self.on.get_debug_mode_information_action: self._on_get_debug_mode_information_action, self.on.nbi_relation_joined: self._update_nbi_relation, } + for relation in [self.on[rel_name] for rel_name in ["prometheus", "keystone"]]: + event_handler_mapping[relation.relation_changed] = self._on_config_changed + event_handler_mapping[relation.relation_broken] = self._on_required_relation_broken for event, handler in event_handler_mapping.items(): self.framework.observe(event, handler) @@ -208,14 +208,6 @@ class OsmNbiCharm(CharmBase): CharmError: if charm configuration is invalid. """ logger.debug("validating charm config") - url = self.config.get("prometheus-url") - if not url: - raise CharmError("need prometheus-url config") - if not self._is_valid_url(url): - raise CharmError(f"Invalid value for prometheus-url config: '{url}'") - - def _is_valid_url(self, url) -> bool: - return urlparse(url).hostname is not None def _check_relations(self) -> None: """Validate charm relations. @@ -230,6 +222,8 @@ class OsmNbiCharm(CharmBase): missing_relations.append("kafka") if not self._is_database_available(): missing_relations.append("mongodb") + if self.prometheus_client.is_missing_data_in_app(): + missing_relations.append("prometheus") if self.keystone_client.is_missing_data_in_app(): missing_relations.append("keystone") @@ -259,7 +253,6 @@ class OsmNbiCharm(CharmBase): def _get_layer(self) -> Dict[str, Any]: """Get layer for Pebble.""" - prometheus = urlparse(self.config["prometheus-url"]) return { "summary": "nbi layer", "description": "pebble config layer for nbi", @@ -290,8 +283,8 @@ class OsmNbiCharm(CharmBase): "OSMNBI_STORAGE_COLLECTION": "files", "OSMNBI_STORAGE_URI": self._get_mongodb_uri(), # Prometheus configuration - "OSMNBI_PROMETHEUS_HOST": prometheus.hostname, - "OSMNBI_PROMETHEUS_PORT": prometheus.port if prometheus.port else "", + "OSMNBI_PROMETHEUS_HOST": self.prometheus_client.hostname, + "OSMNBI_PROMETHEUS_PORT": self.prometheus_client.port, # Log configuration "OSMNBI_LOG_LEVEL": self.config["log-level"], # Authentication environments diff --git a/installers/charm/osm-nbi/src/legacy_interfaces.py b/installers/charm/osm-nbi/src/legacy_interfaces.py index 4750cffc..5deb3f5f 100644 --- a/installers/charm/osm-nbi/src/legacy_interfaces.py +++ b/installers/charm/osm-nbi/src/legacy_interfaces.py @@ -141,3 +141,65 @@ class KeystoneClient(BaseRelationClient): @property def admin_project_name(self): return self.get_data_from_app("admin_project_name") + + +class MongoClient(BaseRelationClient): + """Requires side of a Mongo Endpoint""" + + mandatory_fields_mapping = { + "reactive": ["connection_string"], + "ops": ["replica_set_uri", "replica_set_name"], + } + + def __init__(self, charm: ops.charm.CharmBase, relation_name: str): + super().__init__(charm, relation_name, mandatory_fields=[]) + + @property + def connection_string(self): + if self.is_opts(): + replica_set_uri = self.get_data_from_unit("replica_set_uri") + replica_set_name = self.get_data_from_unit("replica_set_name") + return f"{replica_set_uri}?replicaSet={replica_set_name}" + else: + return self.get_data_from_unit("connection_string") + + def is_opts(self): + return not self.is_missing_data_in_unit_ops() + + def is_missing_data_in_unit(self): + return self.is_missing_data_in_unit_ops() and self.is_missing_data_in_unit_reactive() + + def is_missing_data_in_unit_ops(self): + return not all( + [self.get_data_from_unit(field) for field in self.mandatory_fields_mapping["ops"]] + ) + + def is_missing_data_in_unit_reactive(self): + return not all( + [self.get_data_from_unit(field) for field in self.mandatory_fields_mapping["reactive"]] + ) + + +class PrometheusClient(BaseRelationClient): + """Requires side of a Prometheus Endpoint""" + + mandatory_fields = ["hostname", "port"] + + def __init__(self, charm: ops.charm.CharmBase, relation_name: str): + super().__init__(charm, relation_name, self.mandatory_fields) + + @property + def hostname(self): + return self.get_data_from_app("hostname") + + @property + def port(self): + return self.get_data_from_app("port") + + @property + def user(self): + return self.get_data_from_app("user") + + @property + def password(self): + return self.get_data_from_app("password") diff --git a/installers/charm/osm-nbi/tests/integration/test_charm.py b/installers/charm/osm-nbi/tests/integration/test_charm.py index 1528869a..85551758 100644 --- a/installers/charm/osm-nbi/tests/integration/test_charm.py +++ b/installers/charm/osm-nbi/tests/integration/test_charm.py @@ -42,15 +42,17 @@ MONGO_DB_CHARM = "mongodb-k8s" MONGO_DB_APP = "mongodb" KEYSTONE_CHARM = "osm-keystone" KEYSTONE_APP = "keystone" +PROMETHEUS_CHARM = "osm-prometheus" +PROMETHEUS_APP = "prometheus" ZOOKEEPER_CHARM = "zookeeper-k8s" ZOOKEEPER_APP = "zookeeper" INGRESS_CHARM = "nginx-ingress-integrator" INGRESS_APP = "ingress" -APPS = [KAFKA_APP, MONGO_DB_APP, MARIADB_APP, ZOOKEEPER_APP, KEYSTONE_APP, NBI_APP] +APPS = [KAFKA_APP, MONGO_DB_APP, MARIADB_APP, ZOOKEEPER_APP, KEYSTONE_APP, PROMETHEUS_APP, NBI_APP] @pytest.mark.abort_on_fail -async def test_nbi_and_other_charms_are_idle(ops_test: OpsTest): +async def test_nbi_is_deployed(ops_test: OpsTest): charm = await ops_test.build_charm(".") resources = {"nbi-image": METADATA["resources"]["nbi-image"]["upstream-source"]} @@ -62,6 +64,7 @@ async def test_nbi_and_other_charms_are_idle(ops_test: OpsTest): ops_test.model.deploy(MONGO_DB_CHARM, application_name=MONGO_DB_APP, channel="5/edge"), ops_test.model.deploy(MARIADB_CHARM, application_name=MARIADB_APP, channel="stable"), ops_test.model.deploy(ZOOKEEPER_CHARM, application_name=ZOOKEEPER_APP, channel="stable"), + ops_test.model.deploy(PROMETHEUS_CHARM, application_name=PROMETHEUS_APP, channel="stable"), ) # Keystone charm has to be deployed differently since # bug https://github.com/juju/python-libjuju/issues/766 @@ -71,31 +74,12 @@ async def test_nbi_and_other_charms_are_idle(ops_test: OpsTest): await ops_test.run(*shlex.split(cmd), check=True) async with ops_test.fast_forward(): - await ops_test.model.wait_for_idle(apps=APPS) - - -@pytest.mark.abort_on_fail -async def test_nbi_is_blocked_when_prometheus_config_is_invalid(ops_test: OpsTest): + await ops_test.model.wait_for_idle( + apps=APPS, + ) assert ops_test.model.applications[NBI_APP].status == "blocked" unit = ops_test.model.applications[NBI_APP].units[0] - assert unit.workload_status_message == "need prometheus-url config" - - await ops_test.model.applications[NBI_APP].set_config({"prometheus-url": "some_url"}) - async with ops_test.fast_forward(): - await ops_test.model.wait_for_idle(apps=[NBI_APP], status="blocked") - assert unit.workload_status_message == "Invalid value for prometheus-url config: 'some_url'" - - await ops_test.model.applications[NBI_APP].set_config( - {"prometheus-url": "http://prometheus:0231"} - ) - - -@pytest.mark.abort_on_fail -async def test_nbi_is_blocked_when_relations_are_missing(ops_test: OpsTest): - async with ops_test.fast_forward(): - await ops_test.model.wait_for_idle(apps=[NBI_APP], status="blocked") - unit = ops_test.model.applications[NBI_APP].units[0] - assert unit.workload_status_message == "need kafka, mongodb, keystone relations" + assert unit.workload_status_message == "need kafka, mongodb, prometheus, keystone relations" logger.info("Adding relations for other components") await ops_test.model.add_relation(KAFKA_APP, ZOOKEEPER_APP) @@ -106,10 +90,14 @@ async def test_nbi_is_blocked_when_relations_are_missing(ops_test: OpsTest): "{}:mongodb".format(NBI_APP), "{}:database".format(MONGO_DB_APP) ) await ops_test.model.add_relation(NBI_APP, KAFKA_APP) + await ops_test.model.add_relation(NBI_APP, PROMETHEUS_APP) await ops_test.model.add_relation(NBI_APP, KEYSTONE_APP) async with ops_test.fast_forward(): - await ops_test.model.wait_for_idle(apps=APPS, status="active") + await ops_test.model.wait_for_idle( + apps=APPS, + status="active", + ) @pytest.mark.abort_on_fail @@ -125,14 +113,18 @@ async def test_nbi_scales_up(ops_test: OpsTest): @pytest.mark.abort_on_fail -@pytest.mark.parametrize("relation_to_remove", [KAFKA_APP, MONGO_DB_APP, KEYSTONE_APP]) +@pytest.mark.parametrize( + "relation_to_remove", [KAFKA_APP, MONGO_DB_APP, PROMETHEUS_APP, KEYSTONE_APP] +) async def test_nbi_blocks_without_relation(ops_test: OpsTest, relation_to_remove): logger.info("Removing relation: %s", relation_to_remove) # mongoDB relation is named "database" local_relation = relation_to_remove if local_relation == MONGO_DB_APP: local_relation = "database" - await ops_test.model.applications[relation_to_remove].remove_relation(local_relation, NBI_APP) + await asyncio.gather( + ops_test.model.applications[relation_to_remove].remove_relation(local_relation, NBI_APP) + ) async with ops_test.fast_forward(): await ops_test.model.wait_for_idle(apps=[NBI_APP]) assert ops_test.model.applications[NBI_APP].status == "blocked" @@ -140,7 +132,10 @@ async def test_nbi_blocks_without_relation(ops_test: OpsTest, relation_to_remove assert unit.workload_status_message == f"need {relation_to_remove} relation" await ops_test.model.add_relation(NBI_APP, relation_to_remove) async with ops_test.fast_forward(): - await ops_test.model.wait_for_idle(apps=APPS, status="active") + await ops_test.model.wait_for_idle( + apps=APPS, + status="active", + ) @pytest.mark.abort_on_fail @@ -166,7 +161,10 @@ async def test_nbi_action_debug_mode_disabled(ops_test: OpsTest): async def test_nbi_action_debug_mode_enabled(ops_test: OpsTest): await ops_test.model.applications[NBI_APP].set_config({"debug-mode": "true"}) async with ops_test.fast_forward(): - await ops_test.model.wait_for_idle(apps=APPS, status="active") + await ops_test.model.wait_for_idle( + apps=APPS, + status="active", + ) logger.info("Running action 'get-debug-mode-information'") # list of units is not ordered unit_id = list( @@ -193,8 +191,13 @@ async def test_nbi_integration_ingress(ops_test: OpsTest): await ops_test.run(*shlex.split(cmd), check=True) async with ops_test.fast_forward(): - await ops_test.model.wait_for_idle(apps=APPS + [INGRESS_APP]) + await ops_test.model.wait_for_idle( + apps=APPS + [INGRESS_APP], + ) await ops_test.model.add_relation(NBI_APP, INGRESS_APP) async with ops_test.fast_forward(): - await ops_test.model.wait_for_idle(apps=APPS + [INGRESS_APP], status="active") + await ops_test.model.wait_for_idle( + apps=APPS + [INGRESS_APP], + status="active", + ) diff --git a/installers/charm/osm-nbi/tests/unit/test_charm.py b/installers/charm/osm-nbi/tests/unit/test_charm.py index af248bfc..b1604192 100644 --- a/installers/charm/osm-nbi/tests/unit/test_charm.py +++ b/installers/charm/osm-nbi/tests/unit/test_charm.py @@ -42,65 +42,21 @@ def harness(mocker: MockerFixture): harness.cleanup() -def _set_prometheus_config(harness: Harness): - harness.update_config({"prometheus-url": "http://prometheus:1234"}) - - -def test_default_config_prometheus_url_is_invalid_charm_is_blocked(harness: Harness): - harness.charm.on.config_changed.emit() - assert type(harness.charm.unit.status) == BlockedStatus - assert "need prometheus-url config" == harness.charm.unit.status.message - - def test_missing_relations(harness: Harness): - _set_prometheus_config(harness) harness.charm.on.config_changed.emit() assert type(harness.charm.unit.status) == BlockedStatus assert all( relation in harness.charm.unit.status.message - for relation in ["mongodb", "kafka", "keystone"] - ) - - -def test_prometheus_url_without_schema_blocked_status(harness: Harness): - _add_relations(harness) - harness.update_config({"prometheus-url": "foo.com"}) - assert type(harness.charm.unit.status) == BlockedStatus - assert ( - "Invalid value for prometheus-url config: 'foo.com'" in harness.charm.unit.status.message + for relation in ["mongodb", "kafka", "prometheus", "keystone"] ) -def test_prometheus_url_with_port_without_schema_blocked_status(harness: Harness): - _add_relations(harness) - harness.update_config({"prometheus-url": "foo.com:9090"}) - assert type(harness.charm.unit.status) == BlockedStatus - assert ( - "Invalid value for prometheus-url config: 'foo.com:9090'" - in harness.charm.unit.status.message - ) - - -def test_prometheus_url_without_port_is_valid(harness: Harness): - _add_relations(harness) - harness.update_config({"prometheus-url": "http://foo"}) - assert harness.charm.unit.status == ActiveStatus() - - -def test_prometheus_url_with_port_is_valid(harness: Harness): - _add_relations(harness) - harness.update_config({"prometheus-url": "http://foo:90"}) - assert harness.charm.unit.status == ActiveStatus() - - def test_ready(harness: Harness): - _set_prometheus_config(harness) _add_relations(harness) assert harness.charm.unit.status == ActiveStatus() def test_container_stops_after_relation_broken(harness: Harness): - _set_prometheus_config(harness) harness.charm.on[container_name].pebble_ready.emit(container_name) container = harness.charm.unit.get_container(container_name) relation_ids = _add_relations(harness) @@ -111,7 +67,6 @@ def test_container_stops_after_relation_broken(harness: Harness): def test_nbi_relation_joined(harness: Harness): - _set_prometheus_config(harness) harness.set_leader(True) _add_relations(harness) relation_id = harness.add_relation("nbi", "ng-ui") @@ -137,6 +92,13 @@ def _add_relations(harness: Harness): harness.add_relation_unit(relation_id, "kafka/0") harness.update_relation_data(relation_id, "kafka", {"host": "kafka", "port": "9092"}) relation_ids.append(relation_id) + # Add prometheus relation + relation_id = harness.add_relation("prometheus", "prometheus") + harness.add_relation_unit(relation_id, "prometheus/0") + harness.update_relation_data( + relation_id, "prometheus", {"hostname": "prometheus", "port": "9090"} + ) + relation_ids.append(relation_id) # Add keystone relation relation_id = harness.add_relation("keystone", "keystone") harness.add_relation_unit(relation_id, "keystone/0") -- 2.25.1