From 030ff51443d80e33e13abee99075444e048b86de Mon Sep 17 00:00:00 2001 From: Luca Bello <36242061+lucabello@users.noreply.github.com> Date: Fri, 23 Aug 2024 09:30:50 +0200 Subject: [PATCH] fix: use external_url in alert source (#635) * fix: use external_url in alert source * tox fmt * try to fix unit tests * fixed test * tox fmt * change pytest.mark.xfail to pytest.mark.skip * tox fmt * trigger ci * add updated pip to requirements * add all build deps --------- Co-authored-by: Pietro Pasotti --- charmcraft.yaml | 6 +++++- src/charm.py | 4 ++-- .../test_prometheus_scrape_multiunit.py | 13 +++++++------ .../test_remote_write_grafana_agent.py | 2 +- tests/interface/conftest.py | 3 ++- tests/scenario/conftest.py | 3 ++- tests/scenario/test_server_scheme.py | 2 +- tests/unit/test_charm.py | 18 +++++++++++++++--- tests/unit/test_charm_status.py | 3 ++- tests/unit/test_prometheus_client.py | 1 + tests/unit/test_remote_write.py | 3 ++- tests/unit/test_tls.py | 3 ++- tox.ini | 1 + 13 files changed, 43 insertions(+), 19 deletions(-) diff --git a/charmcraft.yaml b/charmcraft.yaml index 36637d50..6d19916b 100644 --- a/charmcraft.yaml +++ b/charmcraft.yaml @@ -12,7 +12,10 @@ parts: charm: build-packages: - git + - rustc + - cargo charm-binary-python-packages: + - pip>=24 - jsonschema - cryptography - pyyaml @@ -20,7 +23,8 @@ parts: - ops - wheel==0.37.1 - setuptools==45.2.0 - - pydantic>=2 + - pydantic + - pydantic-core cos-tool: plugin: dump source: . diff --git a/src/charm.py b/src/charm.py index ccee97eb..7ed1abfe 100755 --- a/src/charm.py +++ b/src/charm.py @@ -60,6 +60,7 @@ ) from ops.pebble import Error as PebbleError from ops.pebble import ExecError, Layer + from prometheus_client import Prometheus from utils import convert_k8s_quantity_to_legacy_binary_gigabytes @@ -725,8 +726,7 @@ def _generate_command(self) -> str: # For stripPrefix middleware to work correctly, we need to set web.external-url and # web.route-prefix in a particular way. # https://github.com/prometheus/prometheus/issues/1191 - route_prefix = urlparse(self.external_url).path.strip("/") - external_url = f"{self.internal_url.rstrip('/')}/{route_prefix}".rstrip("/") + external_url = self.external_url.rstrip("/") args.append(f"--web.external-url={external_url}") args.append("--web.route-prefix=/") diff --git a/tests/integration/test_prometheus_scrape_multiunit.py b/tests/integration/test_prometheus_scrape_multiunit.py index 61fab7b3..3a59276c 100644 --- a/tests/integration/test_prometheus_scrape_multiunit.py +++ b/tests/integration/test_prometheus_scrape_multiunit.py @@ -48,13 +48,14 @@ idle_period = 90 +@pytest.mark.skip(reason="xfail") async def test_setup_env(ops_test: OpsTest): await ops_test.model.set_config( {"logging-config": "=WARNING; unit=DEBUG", "update-status-hook-interval": "60m"} ) -@pytest.mark.xfail +@pytest.mark.skip(reason="xfail") async def test_prometheus_scrape_relation_with_prometheus_tester( ops_test: OpsTest, prometheus_charm, prometheus_tester_charm ): @@ -179,7 +180,7 @@ async def test_prometheus_scrape_relation_with_prometheus_tester( ) -@pytest.mark.xfail +@pytest.mark.skip(reason="xfail") async def test_upgrade_prometheus(ops_test: OpsTest, prometheus_charm): """Upgrade prometheus and confirm all is still green (see also test_upgrade_charm.py).""" # GIVEN an existing "up" timeseries @@ -221,7 +222,7 @@ async def test_upgrade_prometheus(ops_test: OpsTest, prometheus_charm): assert all(up_before[i] <= up_after[i] for i in range(num_units)) -@pytest.mark.xfail +@pytest.mark.skip(reason="xfail") async def test_rescale_prometheus(ops_test: OpsTest): # GitHub runner doesn't have enough resources to deploy 3 unit with the default "requests", and # the unit fails to schedule. Setting a low limit, so it is able to schedule. @@ -265,7 +266,7 @@ async def test_rescale_prometheus(ops_test: OpsTest): ) -@pytest.mark.xfail +@pytest.mark.skip(reason="xfail") async def test_rescale_tester(ops_test: OpsTest): # WHEN testers are scaled up num_additional_units = 1 @@ -306,7 +307,7 @@ async def test_rescale_tester(ops_test: OpsTest): ) -@pytest.mark.xfail +@pytest.mark.skip(reason="xfail") async def test_upgrade_prometheus_while_rescaling_testers(ops_test: OpsTest, prometheus_charm): """Upgrade prometheus and rescale testers at the same time (without waiting for idle).""" # WHEN prometheus is upgraded at the same time that the testers are scaled up @@ -369,7 +370,7 @@ async def test_upgrade_prometheus_while_rescaling_testers(ops_test: OpsTest, pro ) -@pytest.mark.xfail +@pytest.mark.skip(reason="xfail") async def test_rescale_prometheus_while_upgrading_testers( ops_test: OpsTest, prometheus_tester_charm ): diff --git a/tests/integration/test_remote_write_grafana_agent.py b/tests/integration/test_remote_write_grafana_agent.py index 9d73ada5..a742805e 100644 --- a/tests/integration/test_remote_write_grafana_agent.py +++ b/tests/integration/test_remote_write_grafana_agent.py @@ -150,7 +150,7 @@ async def test_check_data_persist_on_kubectl_delete_pod(ops_test, prometheus_cha assert num_head_chunks_before <= num_head_chunks_after -@pytest.mark.xfail +@pytest.mark.skip(reason="xfail") async def test_check_data_not_persist_on_scale_0(ops_test, prometheus_charm): prometheus_app_name = "prometheus" diff --git a/tests/interface/conftest.py b/tests/interface/conftest.py index 3a7ab9ac..f2bbbac4 100644 --- a/tests/interface/conftest.py +++ b/tests/interface/conftest.py @@ -5,10 +5,11 @@ from unittest.mock import patch import pytest -from charm import PrometheusCharm from interface_tester import InterfaceTester from scenario import Container, ExecOutput, State +from charm import PrometheusCharm + def tautology(*_, **__) -> bool: return True diff --git a/tests/scenario/conftest.py b/tests/scenario/conftest.py index 2b3c68ab..10927833 100644 --- a/tests/scenario/conftest.py +++ b/tests/scenario/conftest.py @@ -4,9 +4,10 @@ from unittest.mock import patch import pytest -from charm import PrometheusCharm from scenario import Context +from charm import PrometheusCharm + def tautology(*_, **__) -> bool: return True diff --git a/tests/scenario/test_server_scheme.py b/tests/scenario/test_server_scheme.py index 9b6d2442..c70db15b 100644 --- a/tests/scenario/test_server_scheme.py +++ b/tests/scenario/test_server_scheme.py @@ -49,7 +49,7 @@ def test_initial_state_has_http_scheme_in_pebble_layer(self, context, initial_st command = container.layers["prometheus"].services["prometheus"].command assert f"--web.external-url=http://{fqdn}:9090" in command - @pytest.mark.xfail + @pytest.mark.skip(reason="xfail") def test_pebble_layer_scheme_becomes_https_if_tls_relation_added( self, context, initial_state, fqdn ): diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index 9df7ae06..19741d2c 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -10,11 +10,12 @@ import ops import yaml -from charm import PROMETHEUS_CONFIG, PrometheusCharm from helpers import cli_arg, k8s_resource_multipatch, prom_multipatch from ops.model import ActiveStatus, BlockedStatus, MaintenanceStatus from ops.testing import Harness +from charm import PROMETHEUS_CONFIG, PrometheusCharm + ops.testing.SIMULATE_CAN_CONNECT = True logger = logging.getLogger(__name__) @@ -96,9 +97,20 @@ def test_ingress_relation_set(self): rel_id = self.harness.add_relation("ingress", "traefik-ingress") self.harness.add_relation_unit(rel_id, "traefik-ingress/0") + with patch( + "charms.observability_libs.v0.kubernetes_compute_resources_patch.KubernetesComputeResourcesPatch.is_ready", + new=lambda _: True, + ): + self.harness.update_relation_data( + rel_id, + "traefik-ingress", + key_values={ + "ingress": yaml.safe_dump({"prometheus-k8s/0": {"url": "http://test:80"}}) + }, + ) + plan = self.harness.get_container_pebble_plan("prometheus") - fqdn = socket.getfqdn() - self.assertEqual(cli_arg(plan, "--web.external-url"), f"http://{fqdn}:9090") + self.assertEqual(cli_arg(plan, "--web.external-url"), "http://test:80") @k8s_resource_multipatch @patch("lightkube.core.client.GenericSyncClient") diff --git a/tests/unit/test_charm_status.py b/tests/unit/test_charm_status.py index e0517e67..98ca1a63 100644 --- a/tests/unit/test_charm_status.py +++ b/tests/unit/test_charm_status.py @@ -8,12 +8,13 @@ from unittest.mock import Mock, patch import ops -from charm import PrometheusCharm from helpers import k8s_resource_multipatch, patch_network_get, prom_multipatch from ops.model import ActiveStatus, BlockedStatus from ops.pebble import Change, ChangeError, ChangeID from ops.testing import Harness +from charm import PrometheusCharm + ops.testing.SIMULATE_CAN_CONNECT = True logger = logging.getLogger(__name__) diff --git a/tests/unit/test_prometheus_client.py b/tests/unit/test_prometheus_client.py index 03b1008c..5d033690 100644 --- a/tests/unit/test_prometheus_client.py +++ b/tests/unit/test_prometheus_client.py @@ -4,6 +4,7 @@ import unittest import responses + from prometheus_client import Prometheus diff --git a/tests/unit/test_remote_write.py b/tests/unit/test_remote_write.py index 3a1fc31d..e21d4244 100644 --- a/tests/unit/test_remote_write.py +++ b/tests/unit/test_remote_write.py @@ -5,7 +5,6 @@ import unittest from unittest.mock import patch -from charm import Prometheus, PrometheusCharm from charms.prometheus_k8s.v1.prometheus_remote_write import ( DEFAULT_RELATION_NAME as RELATION_NAME, ) @@ -27,6 +26,8 @@ from ops.model import ActiveStatus from ops.testing import Harness +from charm import Prometheus, PrometheusCharm + METADATA = f""" name: consumer-tester requires: diff --git a/tests/unit/test_tls.py b/tests/unit/test_tls.py index 1d11cd0b..22fe5650 100644 --- a/tests/unit/test_tls.py +++ b/tests/unit/test_tls.py @@ -4,7 +4,6 @@ import unittest from unittest.mock import patch -from charm import Prometheus, PrometheusCharm from helpers import ( k8s_resource_multipatch, patch_network_get, @@ -12,6 +11,8 @@ ) from ops.testing import Harness +from charm import Prometheus, PrometheusCharm + @prom_multipatch class TestTls(unittest.TestCase): diff --git a/tox.ini b/tox.ini index b79429ca..bfb30d1c 100644 --- a/tox.ini +++ b/tox.ini @@ -68,6 +68,7 @@ deps = cosl fs pytest + pytest-asyncio coverage[toml] responses==0.20.0 -r{toxinidir}/requirements.txt