From f76cc742c16e9b6c530c59c30949968f95c78477 Mon Sep 17 00:00:00 2001 From: Pietro Pasotti Date: Wed, 29 Nov 2023 14:25:54 +0100 Subject: [PATCH] tolerate dead relations --- README.md | 2 +- pyproject.toml | 2 +- scenario/mocking.py | 39 ++++++++++++++++++- scenario/state.py | 5 ++- tests/test_consistency_checker.py | 2 +- tests/test_e2e/test_network.py | 63 +++++++++++++++++++++++-------- 6 files changed, 91 insertions(+), 22 deletions(-) diff --git a/README.md b/README.md index 677427e5..5d71117c 100644 --- a/README.md +++ b/README.md @@ -529,7 +529,7 @@ remote_unit_2_is_joining_event = Event('foo-relation-changed', relation=relation ### Networks -Each relation a charm has will +Each relation a charm has is associated with A charm can define some `extra-bindings` diff --git a/pyproject.toml b/pyproject.toml index 84b93d88..611b793c 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -8,7 +8,7 @@ build-backend = "setuptools.build_meta" [project] name = "ops-scenario" -version = "5.7.1" +version = "6.0" authors = [ { name = "Pietro Pasotti", email = "pietro.pasotti@canonical.com" } diff --git a/scenario/mocking.py b/scenario/mocking.py index a6a21f42..af7dda9d 100644 --- a/scenario/mocking.py +++ b/scenario/mocking.py @@ -5,6 +5,7 @@ import random import shutil from io import StringIO +from itertools import chain from pathlib import Path from typing import TYPE_CHECKING, Any, Dict, List, Mapping, Optional, Set, Tuple, Union @@ -240,9 +241,30 @@ def config_get(self): def network_get(self, binding_name: str, relation_id: Optional[int] = None): # is this an extra-binding-provided network? if binding_name in self._charm_spec.meta.get("extra-bindings", ()): + if relation_id is not None: + # this should not happen + raise RuntimeError( + "cannot pass relation_id to network_get if the binding name is " + "that of an extra-binding. Extra-bindings are not mapped to relation IDs.", + ) network = self._state.extra_bindings.get(binding_name, Network.default()) return network.hook_tool_output_fmt() + # is binding_name a valid relation binding name? + meta = self._charm_spec.meta + if binding_name not in set( + chain( + meta.get("peers", ()), + meta.get("requires", ()), + meta.get("provides", ()), + ), + ): + logger.error( + f"cannot get network binding for {binding_name}: is not a valid relation " + f"endpoint name nor an extra-binding.", + ) + raise RelationNotFoundError() + # Is this a network attached to a relation? relations = self._state.get_relations(binding_name) if relation_id: @@ -260,8 +282,23 @@ def network_get(self, binding_name: str, relation_id: Optional[int] = None): ) raise RelationNotFoundError() from e else: - # TODO: is this accurate? + if not relations: + logger.warning( + "Requesting the network for an endpoint with no active relations " + "will return a defaulted network.", + ) + return Network.default().hook_tool_output_fmt() + + # TODO: is this accurate? Any relation in particular? relation = relations[0] + + from scenario.state import SubordinateRelation # avoid cyclic imports + + if isinstance(relation, SubordinateRelation): + raise RuntimeError( + "Subordinate relation has no associated network binding.", + ) + return relation.network.hook_tool_output_fmt() # setter methods: these can mutate the state. diff --git a/scenario/state.py b/scenario/state.py index 398c587a..18d18d47 100644 --- a/scenario/state.py +++ b/scenario/state.py @@ -325,7 +325,8 @@ class RelationBase(_DCBase): # relation have a different network? network: Network = dataclasses.field(default=None) """Network associated with this relation. - If left empty, a default network will be assigned automatically.""" + If left empty, a default network will be assigned automatically + (except for subordinate relations).""" @property def _databags(self): @@ -355,7 +356,7 @@ def __post_init__(self): for databag in self._databags: self._validate_databag(databag) - if self.network is None: + if type(self) is not SubordinateRelation and self.network is None: object.__setattr__(self, "network", Network.default()) def _validate_databag(self, databag: dict): diff --git a/tests/test_consistency_checker.py b/tests/test_consistency_checker.py index 66415cf9..bb8d75da 100644 --- a/tests/test_consistency_checker.py +++ b/tests/test_consistency_checker.py @@ -8,6 +8,7 @@ Action, Container, Event, + Network, PeerRelation, Relation, Secret, @@ -15,7 +16,6 @@ Storage, SubordinateRelation, _CharmSpec, - Network, ) diff --git a/tests/test_e2e/test_network.py b/tests/test_e2e/test_network.py index ba707810..cdfcf530 100644 --- a/tests/test_e2e/test_network.py +++ b/tests/test_e2e/test_network.py @@ -4,7 +4,7 @@ from ops.framework import Framework from scenario import Context -from scenario.state import Network, Relation, State +from scenario.state import Network, Relation, State, SubordinateRelation from tests.helpers import trigger @@ -33,7 +33,10 @@ def test_ip_get(mycharm): mycharm, meta={ "name": "foo", - "requires": {"metrics-endpoint": {"interface": "foo"}}, + "requires": { + "metrics-endpoint": {"interface": "foo"}, + "deadnodead": {"interface": "bar"}, + }, "extra-bindings": {"foo": {}}, }, ) @@ -56,19 +59,52 @@ def test_ip_get(mycharm): rel = mgr.charm.model.get_relation("metrics-endpoint") assert str(mgr.charm.model.get_binding(rel).network.bind_address) == "1.1.1.1" + # we have a network for a binding without relations on it + assert ( + str(mgr.charm.model.get_binding("deadnodead").network.bind_address) + == "1.1.1.1" + ) + # and an extra binding assert str(mgr.charm.model.get_binding("foo").network.bind_address) == "4.4.4.4" +def test_no_sub_binding(mycharm): + ctx = Context( + mycharm, + meta={ + "name": "foo", + "requires": {"bar": {"interface": "foo", "scope": "container"}}, + }, + ) + + with ctx.manager( + "update_status", + State( + relations=[ + SubordinateRelation("bar"), + ] + ), + ) as mgr: + with pytest.raises(RuntimeError): + # sub relations have no network + mgr.charm.model.get_binding("bar").network + + def test_no_relation_error(mycharm): """Attempting to call get_binding on a non-existing relation -> RelationNotFoundError""" - mycharm._call = lambda *_: True - def fetch_unit_address(charm: CharmBase): - with pytest.raises(RelationNotFoundError): - _ = charm.model.get_binding("foo").network + ctx = Context( + mycharm, + meta={ + "name": "foo", + "requires": {"metrics-endpoint": {"interface": "foo"}}, + "extra-bindings": {"bar": {}}, + }, + ) - trigger( + with ctx.manager( + "update_status", State( relations=[ Relation( @@ -78,13 +114,8 @@ def fetch_unit_address(charm: CharmBase): relation_id=1, ), ], - extra_bindings={"foo": Network.default()}, + extra_bindings={"bar": Network.default()}, ), - "update_status", - mycharm, - meta={ - "name": "foo", - "requires": {"metrics-endpoint": {"interface": "foo"}}, - }, - post_event=fetch_unit_address, - ) + ) as mgr: + with pytest.raises(RelationNotFoundError): + net = mgr.charm.model.get_binding("foo").network