Skip to content

Commit

Permalink
fix: use external_url in alert source (#635)
Browse files Browse the repository at this point in the history
* 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 <[email protected]>
  • Loading branch information
lucabello and PietroPasotti authored Aug 23, 2024
1 parent 76e8081 commit 030ff51
Show file tree
Hide file tree
Showing 13 changed files with 43 additions and 19 deletions.
6 changes: 5 additions & 1 deletion charmcraft.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,19 @@ parts:
charm:
build-packages:
- git
- rustc
- cargo
charm-binary-python-packages:
- pip>=24
- jsonschema
- cryptography
- pyyaml
- requests
- ops
- wheel==0.37.1
- setuptools==45.2.0
- pydantic>=2
- pydantic
- pydantic-core
cos-tool:
plugin: dump
source: .
Expand Down
4 changes: 2 additions & 2 deletions src/charm.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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=/")

Expand Down
13 changes: 7 additions & 6 deletions tests/integration/test_prometheus_scrape_multiunit.py
Original file line number Diff line number Diff line change
Expand Up @@ -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": "<root>=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
):
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand 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
):
Expand Down
2 changes: 1 addition & 1 deletion tests/integration/test_remote_write_grafana_agent.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down
3 changes: 2 additions & 1 deletion tests/interface/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion tests/scenario/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion tests/scenario/test_server_scheme.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
):
Expand Down
18 changes: 15 additions & 3 deletions tests/unit/test_charm.py
Original file line number Diff line number Diff line change
Expand Up @@ -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__)

Expand Down Expand Up @@ -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")
Expand Down
3 changes: 2 additions & 1 deletion tests/unit/test_charm_status.py
Original file line number Diff line number Diff line change
Expand Up @@ -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__)

Expand Down
1 change: 1 addition & 0 deletions tests/unit/test_prometheus_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import unittest

import responses

from prometheus_client import Prometheus


Expand Down
3 changes: 2 additions & 1 deletion tests/unit/test_remote_write.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)
Expand All @@ -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:
Expand Down
3 changes: 2 additions & 1 deletion tests/unit/test_tls.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,15 @@
import unittest
from unittest.mock import patch

from charm import Prometheus, PrometheusCharm
from helpers import (
k8s_resource_multipatch,
patch_network_get,
prom_multipatch,
)
from ops.testing import Harness

from charm import Prometheus, PrometheusCharm


@prom_multipatch
class TestTls(unittest.TestCase):
Expand Down
1 change: 1 addition & 0 deletions tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ deps =
cosl
fs
pytest
pytest-asyncio
coverage[toml]
responses==0.20.0
-r{toxinidir}/requirements.txt
Expand Down

0 comments on commit 030ff51

Please sign in to comment.