Integrate NBI and Prometheus 99/13599/9
authorPatricia Reinoso <patricia.reinoso@canonical.com>
Wed, 28 Jun 2023 16:10:23 +0000 (16:10 +0000)
committerreinosop <patricia.reinoso@canonical.com>
Mon, 24 Jul 2023 09:41:21 +0000 (11:41 +0200)
Remove relation between NBI and Prometheus

Integration is done with a config parameter
prometheus-url

This parameter does not have default value.

If this parameter is not set the charm is in blocked status.

Change-Id: I312304b7c75835d0b1de5a59b21edc7f478666e9
Signed-off-by: Patricia Reinoso <patricia.reinoso@canonical.com>
installers/charm/osm-nbi/config.yaml
installers/charm/osm-nbi/metadata.yaml
installers/charm/osm-nbi/src/charm.py
installers/charm/osm-nbi/src/legacy_interfaces.py
installers/charm/osm-nbi/tests/integration/test_charm.py
installers/charm/osm-nbi/tests/unit/test_charm.py

index 85e637a..961cfff 100644 (file)
@@ -66,6 +66,9 @@ 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:
index 8a336c8..b7902e0 100644 (file)
@@ -67,9 +67,6 @@ requires:
   keystone:
     interface: keystone
     limit: 1
-  prometheus:
-    interface: prometheus
-    limit: 1
   ingress:
     interface: ingress
     limit: 1
index 8855de2..00ef425 100755 (executable)
@@ -29,6 +29,7 @@ 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
@@ -48,7 +49,7 @@ from ops.framework import StoredState
 from ops.main import main
 from ops.model import ActiveStatus, Container
 
-from legacy_interfaces import KeystoneClient, PrometheusClient
+from legacy_interfaces import KeystoneClient
 
 HOSTPATHS = [
     HostPath(
@@ -86,7 +87,6 @@ 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,6 +129,7 @@ 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
@@ -184,13 +185,12 @@ 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,6 +208,14 @@ 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.
@@ -222,8 +230,6 @@ 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")
 
@@ -253,6 +259,7 @@ 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",
@@ -282,8 +289,8 @@ class OsmNbiCharm(CharmBase):
                         "OSMNBI_STORAGE_COLLECTION": "files",
                         "OSMNBI_STORAGE_URI": self._get_mongodb_uri(),
                         # Prometheus configuration
-                        "OSMNBI_PROMETHEUS_HOST": self.prometheus_client.hostname,
-                        "OSMNBI_PROMETHEUS_PORT": self.prometheus_client.port,
+                        "OSMNBI_PROMETHEUS_HOST": prometheus.hostname,
+                        "OSMNBI_PROMETHEUS_PORT": prometheus.port if prometheus.port else "",
                         # Log configuration
                         "OSMNBI_LOG_LEVEL": self.config["log-level"],
                         # Authentication environments
index 5deb3f5..4750cff 100644 (file)
@@ -141,65 +141,3 @@ 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")
index 8555175..1528869 100644 (file)
@@ -42,17 +42,15 @@ 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, PROMETHEUS_APP, NBI_APP]
+APPS = [KAFKA_APP, MONGO_DB_APP, MARIADB_APP, ZOOKEEPER_APP, KEYSTONE_APP, NBI_APP]
 
 
 @pytest.mark.abort_on_fail
-async def test_nbi_is_deployed(ops_test: OpsTest):
+async def test_nbi_and_other_charms_are_idle(ops_test: OpsTest):
     charm = await ops_test.build_charm(".")
     resources = {"nbi-image": METADATA["resources"]["nbi-image"]["upstream-source"]}
 
@@ -64,7 +62,6 @@ async def test_nbi_is_deployed(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
@@ -74,12 +71,31 @@ async def test_nbi_is_deployed(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,
-        )
+        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):
     assert ops_test.model.applications[NBI_APP].status == "blocked"
     unit = ops_test.model.applications[NBI_APP].units[0]
-    assert unit.workload_status_message == "need kafka, mongodb, prometheus, keystone relations"
+    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"
 
     logger.info("Adding relations for other components")
     await ops_test.model.add_relation(KAFKA_APP, ZOOKEEPER_APP)
@@ -90,14 +106,10 @@ async def test_nbi_is_deployed(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
@@ -113,18 +125,14 @@ 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, PROMETHEUS_APP, KEYSTONE_APP]
-)
+@pytest.mark.parametrize("relation_to_remove", [KAFKA_APP, MONGO_DB_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 asyncio.gather(
-        ops_test.model.applications[relation_to_remove].remove_relation(local_relation, NBI_APP)
-    )
+    await 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"
@@ -132,10 +140,7 @@ 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
@@ -161,10 +166,7 @@ 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(
@@ -191,13 +193,8 @@ 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")
index b160419..af248bf 100644 (file)
@@ -42,21 +42,65 @@ 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", "prometheus", "keystone"]
+        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
     )
 
 
+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)
@@ -67,6 +111,7 @@ 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")
@@ -92,13 +137,6 @@ 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")