From eb91c3aee32142f5e9b966294fcf9918e09ae0dd Mon Sep 17 00:00:00 2001 From: PietroPasotti Date: Fri, 6 Oct 2023 12:22:53 +0200 Subject: [PATCH] Fix issue with remote unit scale down (#263) * added ipa test * fixed ipa not notifying traefik on unit departed * ignore scenario error * lint --- lib/charms/traefik_k8s/v2/ingress.py | 3 +- tests/scenario/conftest.py | 6 +- tests/scenario/test_ipa.py | 126 +++++++++++++++++++++++++++ tox.ini | 4 +- 4 files changed, 135 insertions(+), 4 deletions(-) create mode 100644 tests/scenario/test_ipa.py diff --git a/lib/charms/traefik_k8s/v2/ingress.py b/lib/charms/traefik_k8s/v2/ingress.py index 0364c8ab..f2c857a6 100644 --- a/lib/charms/traefik_k8s/v2/ingress.py +++ b/lib/charms/traefik_k8s/v2/ingress.py @@ -79,7 +79,7 @@ def _on_ingress_revoked(self, event: IngressPerAppRevokedEvent): # Increment this PATCH version before using `charmcraft publish-lib` or reset # to 0 if you are raising the major API version -LIBPATCH = 6 +LIBPATCH = 7 PYDEPS = ["pydantic<2.0"] @@ -244,6 +244,7 @@ def __init__(self, charm: CharmBase, relation_name: str = DEFAULT_RELATION_NAME) observe(rel_events.relation_created, self._handle_relation) observe(rel_events.relation_joined, self._handle_relation) observe(rel_events.relation_changed, self._handle_relation) + observe(rel_events.relation_departed, self._handle_relation) observe(rel_events.relation_broken, self._handle_relation_broken) observe(charm.on.leader_elected, self._handle_upgrade_or_leader) # type: ignore observe(charm.on.upgrade_charm, self._handle_upgrade_or_leader) # type: ignore diff --git a/tests/scenario/conftest.py b/tests/scenario/conftest.py index 6ead38c2..67eeea1f 100644 --- a/tests/scenario/conftest.py +++ b/tests/scenario/conftest.py @@ -3,7 +3,7 @@ import pytest from charm import TraefikIngressCharm from ops import pebble -from scenario import Container, Context, Model, Mount +from scenario import Container, Context, ExecOutput, Model, Mount MOCK_EXTERNAL_HOSTNAME = "testhostname" @@ -52,6 +52,10 @@ def traefik_container(tmp_path): name="traefik", can_connect=True, layers={"traefik": layer}, + exec_mock={ + ("update-ca-certificates", "--fresh"): ExecOutput(), + ("/usr/bin/traefik", "version"): ExecOutput(stdout="42.42"), + }, service_status={"traefik": pebble.ServiceStatus.ACTIVE}, mounts={"opt": opt}, ) diff --git a/tests/scenario/test_ipa.py b/tests/scenario/test_ipa.py new file mode 100644 index 00000000..fbaad601 --- /dev/null +++ b/tests/scenario/test_ipa.py @@ -0,0 +1,126 @@ +from pathlib import Path +from typing import List, Tuple + +import pytest +import yaml +from scenario import Context, Relation, State + + +def create(traefik_ctx: Context, state: State): + """Create the ingress relation.""" + ingress = Relation("ingress") + return traefik_ctx.run(ingress.joined_event, state.replace(relations=[ingress])) + + +def join(traefik_ctx: Context, state: State): + """Simulate a new unit joining the ingress relation.""" + ingress = state.get_relations("ingress")[0] + state = traefik_ctx.run(ingress.joined_event, state) + remote_units_data = ingress.remote_units_data + + joining_unit_id = max(remote_units_data) + if remote_units_data[joining_unit_id]: + joining_unit_id += 1 + + remote_units_data[joining_unit_id] = { + "host": f'"neutron-{joining_unit_id}.neutron-endpoints.zaza-de71889d82db.svc.cluster.local"' + } + relations = [ + ingress.replace( + remote_app_data={ + "model": '"zaza"', + "name": '"neutron"', + "port": "9696", + "redirect-https": "false", + "scheme": '"http"', + "strip-prefix": "false", + }, + remote_units_data=remote_units_data, + ) + ] + + state = traefik_ctx.run( + state.get_relations("ingress")[0].changed_event, state.replace(relations=relations) + ) + return state + + +def depart(traefik_ctx: Context, state: State): + """Simulate a unit departing the ingress relation.""" + + def _pop(state: State): + ingress = state.get_relations("ingress")[0] + remote_units_data = ingress.remote_units_data.copy() + departing_unit_id = max(remote_units_data) + del remote_units_data[departing_unit_id] + return state.replace(relations=[ingress.replace(remote_units_data=remote_units_data)]) + + state = _pop(state) + + state = traefik_ctx.run(state.get_relations("ingress")[0].departed_event, state) + return state + + +def break_(traefik_ctx: Context, state: State): + """Simulate breaking the ingress relation.""" + for _ in state.get_relations("ingress")[0].remote_units_data: + # depart all units + depart(traefik_ctx, state) + + ingress = state.get_relations("ingress")[0] + return traefik_ctx.run( + ingress.broken_event, + state.replace(relations=[ingress.replace(remote_app_data={}, remote_units_data={})]), + ) + + +def get_configs(traefik_ctx: Context, state: State) -> Tuple[Path, List[Path]]: + """Return static and dynamic configs.""" + vfs_root = state.get_container("traefik").get_filesystem(traefik_ctx) + opt = vfs_root / "opt" / "traefik" / "juju" + etc = vfs_root / "etc" / "traefik" / "traefik.yaml" + return etc, list(opt.glob("*.yaml")) + + +def get_servers(cfg: Path): + """Return a list of servers from the traefik config.""" + cfg_yaml = yaml.safe_load(cfg.read_text()) + return cfg_yaml["http"]["services"]["juju-zaza-neutron-service"]["loadBalancer"]["servers"] + + +def test_traefik_remote_app_scaledown_from_2(traefik_ctx, traefik_container): + """Verify that on scale up and down traefik always has the right amount of servers configured. + + TODO: parametrize + """ + state = State(containers=[traefik_container]) + + with traefik_ctx.manager(traefik_container.pebble_ready_event, state) as mgr: + state = mgr.run() + static, dynamic = get_configs(traefik_ctx, state) + + assert static.exists() + assert len(dynamic) == 0 + + state = create(traefik_ctx, state) + + state = join(traefik_ctx, state) + + _, dynamic = get_configs(traefik_ctx, state) + assert len(dynamic) == 1 + assert len(get_servers(dynamic[0])) == 1 + + state = join(traefik_ctx, state) + + _, dynamic = get_configs(traefik_ctx, state) + assert len(get_servers(dynamic[0])) == 2 + + state = depart(traefik_ctx, state) + + _, dynamic = get_configs(traefik_ctx, state) + assert len(get_servers(dynamic[0])) == 1 + + with pytest.raises(IndexError): + # FIXME: solved in scenario 5.3.1 @https://github.com/canonical/ops-scenario/pull/64 + break_(traefik_ctx, state) + assert not dynamic[0].exists() diff --git a/tox.ini b/tox.ini index 0e957909..e44cc3f7 100644 --- a/tox.ini +++ b/tox.ini @@ -85,7 +85,7 @@ allowlist_externals = /usr/bin/env description = Scenario tests deps = pytest - ops-scenario >= 5.1 + ops-scenario >= 5.3 -r{toxinidir}/requirements.txt commands = pytest -v --tb native {[vars]tst_path}/scenario --log-cli-level=INFO -s {posargs} @@ -94,7 +94,7 @@ commands = description = Run interface tests deps = pytest - ops-scenario + ops-scenario>=5.3 pytest-interface-tester > 0.3 -r{toxinidir}/requirements.txt commands =