From 8f93ba78447b4c15e36f3f4673cb60068a099983 Mon Sep 17 00:00:00 2001 From: Balbir Thomas Date: Wed, 4 May 2022 14:39:21 +0100 Subject: [PATCH] MetricsProvider sets unit address on updated status (#268) This commit ensures that the MetricsEndpointProvider sets unit address on update status and config changed events also. This change is required because podspec charms do not have a pebble ready event and uptil now the MetricsEndpointProvider was setting IP address only in response to pebble ready event This change also ensures that the MetricsEndpointProvider does not set invalid IP addresses. --- .../prometheus_k8s/v0/prometheus_scrape.py | 37 +++++++++++++++++-- tests/unit/test_endpoint_provider.py | 16 ++++++++ 2 files changed, 49 insertions(+), 4 deletions(-) diff --git a/lib/charms/prometheus_k8s/v0/prometheus_scrape.py b/lib/charms/prometheus_k8s/v0/prometheus_scrape.py index 89afdf2e..080a7281 100644 --- a/lib/charms/prometheus_k8s/v0/prometheus_scrape.py +++ b/lib/charms/prometheus_k8s/v0/prometheus_scrape.py @@ -311,6 +311,8 @@ def _on_scrape_targets_changed(self, event): """ # noqa: W505 +import contextlib +import ipaddress import json import logging import os @@ -334,7 +336,7 @@ def _on_scrape_targets_changed(self, event): # Increment this PATCH version before using `charmcraft publish-lib` or reset # to 0 if you are raising the major API version -LIBPATCH = 18 +LIBPATCH = 19 logger = logging.getLogger(__name__) @@ -1569,6 +1571,10 @@ def __init__( self._set_unit_ip, ) + # podspec charms do not have a pebble ready event so unit ips need to be set these events + self.framework.observe(self._charm.on.update_status, self._set_unit_ip) + self.framework.observe(self._charm.on.config_changed, self._set_unit_ip) + self.framework.observe(self._charm.on.upgrade_charm, self._set_scrape_job_spec) def _set_scrape_job_spec(self, event): @@ -1611,13 +1617,36 @@ def _set_unit_ip(self, _): event is actually needed. """ for relation in self._charm.model.relations[self._relation_name]: - relation.data[self._charm.unit]["prometheus_scrape_unit_address"] = str( - self._charm.model.get_binding(relation).network.bind_address - ) + unit_ip = str(self._charm.model.get_binding(relation).network.bind_address) + + if not self._is_valid_unit_address(unit_ip): + # relation data will be updated later when a valid address becomes available + with contextlib.suppress(KeyError): + del relation.data[self._charm.unit]["prometheus_scrape_unit_address"] + del relation.data[self._charm.unit]["prometheus_scrape_unit_name"] + continue + + relation.data[self._charm.unit]["prometheus_scrape_unit_address"] = unit_ip relation.data[self._charm.unit]["prometheus_scrape_unit_name"] = str( self._charm.model.unit.name ) + def _is_valid_unit_address(self, address: str) -> bool: + """Validate a unit address. + + At present only IP address validation is supported, but + this may be extended to DNS addresses also, as needed. + + Args: + address: a string representing a unit address + """ + try: + _ = ipaddress.ip_address(address) + except ValueError: + return False + + return True + @property def _scrape_jobs(self) -> list: """Fetch list of scrape jobs. diff --git a/tests/unit/test_endpoint_provider.py b/tests/unit/test_endpoint_provider.py index a03baa9c..c9955086 100644 --- a/tests/unit/test_endpoint_provider.py +++ b/tests/unit/test_endpoint_provider.py @@ -152,6 +152,22 @@ def test_provider_unit_sets_bind_address_on_pebble_ready(self, *unused): self.assertIn("prometheus_scrape_unit_address", data) self.assertEqual(data["prometheus_scrape_unit_address"], "192.0.8.2") + @patch_network_get(private_address="192.0.8.2") + def test_provider_unit_sets_bind_address_on_update_status(self, *unused): + rel_id = self.harness.add_relation(RELATION_NAME, "provider") + self.harness.charm.on.update_status.emit() + data = self.harness.get_relation_data(rel_id, self.harness.charm.unit.name) + self.assertIn("prometheus_scrape_unit_address", data) + self.assertEqual(data["prometheus_scrape_unit_address"], "192.0.8.2") + + @patch_network_get(private_address=None) + def test_provider_does_not_set_invalid_unit_address(self, *unused): + rel_id = self.harness.add_relation(RELATION_NAME, "provider") + self.harness.container_pebble_ready("prometheus-tester") + self.harness.charm.on.update_status.emit() + data = self.harness.get_relation_data(rel_id, self.harness.charm.unit.name) + self.assertNotIn("prometheus_scrape_unit_address", data) + @patch_network_get(private_address="192.0.8.2") def test_provider_unit_sets_bind_address_on_relation_joined(self, *unused): rel_id = self.harness.add_relation(RELATION_NAME, "provider")