From 66fe4a5ed0d93f3ad906f4dc3369d34e8ac1205f Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Tue, 2 Jul 2024 23:03:42 +1200 Subject: [PATCH] Fix merging. Most particularly, the relation_id to id change was merged in main (mistakenly) and then unmerged, so the rebasing undid it, so that is redone. Also, the basic changes for Pebble notices and Cloud Configuration are done in here, although those need to be double-checked to make sure they make sense with the updated API. --- README.md | 4 ++-- scenario/consistency_checker.py | 10 +++++----- scenario/mocking.py | 6 ++---- scenario/ops_main_mock.py | 2 +- scenario/runtime.py | 2 +- scenario/state.py | 11 ++++++----- tests/test_consistency_checker.py | 22 ++++++++-------------- tests/test_e2e/test_cloud_spec.py | 6 +++--- tests/test_e2e/test_deferred.py | 2 +- tests/test_e2e/test_network.py | 4 ++-- tests/test_e2e/test_pebble.py | 2 +- tests/test_e2e/test_play_assertions.py | 2 +- tests/test_e2e/test_relations.py | 2 +- tests/test_e2e/test_secrets.py | 8 +++++--- 14 files changed, 39 insertions(+), 44 deletions(-) diff --git a/README.md b/README.md index 065055fd..f7a794e3 100644 --- a/README.md +++ b/README.md @@ -439,7 +439,7 @@ needs to set up the process that will run `ops.main` with the right environment ### Working with relation IDs -Every time you instantiate `Relation` (or peer, or subordinate), the new instance will be given a unique `relation_id`. +Every time you instantiate `Relation` (or peer, or subordinate), the new instance will be given a unique `id`. To inspect the ID the next relation instance will have, you can call `scenario.state.next_relation_id`. ```python @@ -447,7 +447,7 @@ import scenario.state next_id = scenario.state.next_relation_id(update=False) rel = scenario.Relation('foo') -assert rel.relation_id == next_id +assert rel.id == next_id ``` This can be handy when using `replace` to create new relations, to avoid relation ID conflicts: diff --git a/scenario/consistency_checker.py b/scenario/consistency_checker.py index befbbeed..87d9da80 100644 --- a/scenario/consistency_checker.py +++ b/scenario/consistency_checker.py @@ -125,7 +125,7 @@ def check_event_consistency( state: "State", **_kwargs, # noqa: U101 ) -> Results: - """Check the internal consistency of the Event data structure. + """Check the internal consistency of the _Event data structure. For example, it checks that a relation event has a relation instance, and that the relation endpoint name matches the event prefix. @@ -514,13 +514,13 @@ def _get_relations(r): expected_sub = relation_meta.get("scope", "") == "container" relations = _get_relations(endpoint) for relation in relations: - if relation.relation_id in seen_ids: + if relation.id in seen_ids: errors.append( - f"duplicate relation ID: {relation.relation_id} is claimed " + f"duplicate relation ID: {relation.id} is claimed " f"by multiple Relation instances", ) - seen_ids.add(relation.relation_id) + seen_ids.add(relation.id) is_sub = isinstance(relation, SubordinateRelation) if is_sub and not expected_sub: errors.append( @@ -604,7 +604,7 @@ def check_containers_consistency( def check_cloudspec_consistency( *, state: "State", - event: "Event", + event: "_Event", charm_spec: "_CharmSpec", **_kwargs, # noqa: U101 ) -> Results: diff --git a/scenario/mocking.py b/scenario/mocking.py index ad74ef91..8885e420 100644 --- a/scenario/mocking.py +++ b/scenario/mocking.py @@ -163,7 +163,7 @@ def _get_relation_by_id( ) -> Union["Relation", "SubordinateRelation", "PeerRelation"]: try: return next( - filter(lambda r: r.relation_id == rel_id, self._state.relations), + filter(lambda r: r.id == rel_id, self._state.relations), ) except StopIteration: raise RelationNotFoundError() @@ -245,9 +245,7 @@ def status_get(self, *, is_app: bool = False): def relation_ids(self, relation_name): return [ - rel.relation_id - for rel in self._state.relations - if rel.endpoint == relation_name + rel.id for rel in self._state.relations if rel.endpoint == relation_name ] def relation_list(self, relation_id: int) -> Tuple[str, ...]: diff --git a/scenario/ops_main_mock.py b/scenario/ops_main_mock.py index dc167160..433b880f 100644 --- a/scenario/ops_main_mock.py +++ b/scenario/ops_main_mock.py @@ -118,7 +118,7 @@ def setup_framework( # If we are in a RelationBroken event, we want to know which relation is # broken within the model, not only in the event's `.relation` attribute. broken_relation_id = ( - event.relation.relation_id # type: ignore + event.relation.id # type: ignore if event.name.endswith("_relation_broken") else None ) diff --git a/scenario/runtime.py b/scenario/runtime.py index 114e66a9..97a7c773 100644 --- a/scenario/runtime.py +++ b/scenario/runtime.py @@ -211,7 +211,7 @@ def _get_event_env(self, state: "State", event: "_Event", charm_root: Path): env.update( { "JUJU_RELATION": relation.endpoint, - "JUJU_RELATION_ID": str(relation.relation_id), + "JUJU_RELATION_ID": str(relation.id), "JUJU_REMOTE_APP": remote_app_name, }, ) diff --git a/scenario/state.py b/scenario/state.py index 7929a539..b2c6fe76 100644 --- a/scenario/state.py +++ b/scenario/state.py @@ -340,7 +340,7 @@ class _RelationBase: """Interface name. Must match the interface name attached to this endpoint in metadata.yaml. If left empty, it will be automatically derived from metadata.yaml.""" - relation_id: int = dataclasses.field(default_factory=next_relation_id) + id: int = dataclasses.field(default_factory=next_relation_id) """Juju relation ID. Every new Relation instance gets a unique one, if there's trouble, override.""" @@ -575,7 +575,7 @@ def next_notice_id(update=True): @dataclasses.dataclass(frozen=True) -class Notice(_DCBase): +class Notice: key: str """The notice key, a string that differentiates notices of this type. @@ -634,7 +634,7 @@ def _to_ops(self) -> pebble.Notice: @dataclasses.dataclass(frozen=True) -class _BoundNotice(_DCBase): +class _BoundNotice: notice: Notice container: "Container" @@ -642,7 +642,7 @@ class _BoundNotice(_DCBase): def event(self): """Sugar to generate a -pebble-custom-notice event for this notice.""" suffix = PEBBLE_CUSTOM_NOTICE_EVENT_SUFFIX - return Event( + return _Event( path=normalize_name(self.container.name) + suffix, container=self.container, notice=self.notice, @@ -762,6 +762,7 @@ def get_notice( f"{self.name} does not have a notice with key {key} and type {notice_type}", ) + _RawStatusLiteral = Literal[ "waiting", "blocked", @@ -1367,7 +1368,7 @@ def deferred(self, handler: Callable, event_id: int = 1) -> DeferredEvent: snapshot_data = { "relation_name": relation.endpoint, - "relation_id": relation.relation_id, + "relation_id": relation.id, "app_name": remote_app, "unit_name": f"{remote_app}/{self.relation_remote_unit_id}", } diff --git a/tests/test_consistency_checker.py b/tests/test_consistency_checker.py index 217a68d9..6a955be7 100644 --- a/tests/test_consistency_checker.py +++ b/tests/test_consistency_checker.py @@ -69,18 +69,18 @@ def test_workload_event_without_container(): ) assert_inconsistent( State(), - Event("foo-pebble-custom-notice", container=Container("foo")), + _Event("foo-pebble-custom-notice", container=Container("foo")), _CharmSpec(MyCharm, {}), ) notice = Notice("example.com/foo") assert_consistent( State(containers=[Container("foo", notices=[notice])]), - Event("foo-pebble-custom-notice", container=Container("foo"), notice=notice), + _Event("foo-pebble-custom-notice", container=Container("foo"), notice=notice), _CharmSpec(MyCharm, {"containers": {"foo": {}}}), ) assert_inconsistent( State(containers=[Container("foo")]), - Event("foo-pebble-custom-notice", container=Container("foo"), notice=notice), + _Event("foo-pebble-custom-notice", container=Container("foo"), notice=notice), _CharmSpec(MyCharm, {"containers": {"foo": {}}}), ) @@ -459,9 +459,7 @@ def test_action_params_type(ptype, good, bad): def test_duplicate_relation_ids(): assert_inconsistent( - State( - relations=[Relation("foo", id=1), Relation("bar", id=1)] - ), + State(relations=[Relation("foo", id=1), Relation("bar", id=1)]), _Event("start"), _CharmSpec( MyCharm, @@ -474,17 +472,13 @@ def test_duplicate_relation_ids(): def test_relation_without_endpoint(): assert_inconsistent( - State( - relations=[Relation("foo", id=1), Relation("bar", id=1)] - ), + State(relations=[Relation("foo", id=1), Relation("bar", id=1)]), _Event("start"), _CharmSpec(MyCharm, meta={"name": "charlemagne"}), ) assert_consistent( - State( - relations=[Relation("foo", id=1), Relation("bar", id=2)] - ), + State(relations=[Relation("foo", id=1), Relation("bar", id=2)]), _Event("start"), _CharmSpec( MyCharm, @@ -658,7 +652,7 @@ def test_cloudspec_consistency(): assert_consistent( State(model=Model(name="lxd-model", type="lxd", cloud_spec=cloud_spec)), - Event("start"), + _Event("start"), _CharmSpec( MyCharm, meta={"name": "MyVMCharm"}, @@ -667,7 +661,7 @@ def test_cloudspec_consistency(): assert_inconsistent( State(model=Model(name="k8s-model", type="kubernetes", cloud_spec=cloud_spec)), - Event("start"), + _Event("start"), _CharmSpec( MyCharm, meta={"name": "MyK8sCharm"}, diff --git a/tests/test_e2e/test_cloud_spec.py b/tests/test_e2e/test_cloud_spec.py index 8ce413f8..1834b3da 100644 --- a/tests/test_e2e/test_cloud_spec.py +++ b/tests/test_e2e/test_cloud_spec.py @@ -47,14 +47,14 @@ def test_get_cloud_spec(): name="lxd-model", type="lxd", cloud_spec=scenario_cloud_spec ), ) - with ctx.manager("start", state=state) as mgr: + with ctx.manager(ctx.on.start(), state=state) as mgr: assert mgr.charm.model.get_cloud_spec() == expected_cloud_spec def test_get_cloud_spec_error(): ctx = scenario.Context(MyCharm, meta={"name": "foo"}) state = scenario.State(model=scenario.Model(name="lxd-model", type="lxd")) - with ctx.manager("start", state) as mgr: + with ctx.manager(ctx.on.start(), state) as mgr: with pytest.raises(ops.ModelError): mgr.charm.model.get_cloud_spec() @@ -65,6 +65,6 @@ def test_get_cloud_spec_untrusted(): state = scenario.State( model=scenario.Model(name="lxd-model", type="lxd", cloud_spec=cloud_spec), ) - with ctx.manager("start", state) as mgr: + with ctx.manager(ctx.on.start(), state) as mgr: with pytest.raises(ops.ModelError): mgr.charm.model.get_cloud_spec() diff --git a/tests/test_e2e/test_deferred.py b/tests/test_e2e/test_deferred.py index 8645c77b..fccb326c 100644 --- a/tests/test_e2e/test_deferred.py +++ b/tests/test_e2e/test_deferred.py @@ -169,7 +169,7 @@ def test_deferred_relation_event_from_relation(mycharm): assert out.deferred[0].name == "foo_relation_changed" assert out.deferred[0].snapshot_data == { "relation_name": rel.endpoint, - "relation_id": rel.relation_id, + "relation_id": rel.id, "app_name": "remote", "unit_name": "remote/1", } diff --git a/tests/test_e2e/test_network.py b/tests/test_e2e/test_network.py index 8856277b..440a01be 100644 --- a/tests/test_e2e/test_network.py +++ b/tests/test_e2e/test_network.py @@ -48,7 +48,7 @@ def test_ip_get(mycharm): interface="foo", remote_app_name="remote", endpoint="metrics-endpoint", - relation_id=1, + id=1, ), ], networks={"foo": Network.default(private_address="4.4.4.4")}, @@ -110,7 +110,7 @@ def test_no_relation_error(mycharm): interface="foo", remote_app_name="remote", endpoint="metrics-endpoint", - relation_id=1, + id=1, ), ], networks={"bar": Network.default()}, diff --git a/tests/test_e2e/test_pebble.py b/tests/test_e2e/test_pebble.py index bf83c6c6..a9223120 100644 --- a/tests/test_e2e/test_pebble.py +++ b/tests/test_e2e/test_pebble.py @@ -1,5 +1,5 @@ -import datetime import dataclasses +import datetime import tempfile from pathlib import Path diff --git a/tests/test_e2e/test_play_assertions.py b/tests/test_e2e/test_play_assertions.py index a5166db7..7fe07899 100644 --- a/tests/test_e2e/test_play_assertions.py +++ b/tests/test_e2e/test_play_assertions.py @@ -104,7 +104,7 @@ def check_relation_data(charm): Relation( endpoint="relation_test", interface="azdrubales", - relation_id=1, + id=1, remote_app_name="karlos", remote_app_data={"yaba": "doodle"}, remote_units_data={0: {"foo": "bar"}, 1: {"baz": "qux"}}, diff --git a/tests/test_e2e/test_relations.py b/tests/test_e2e/test_relations.py index f3447cbe..e72f754c 100644 --- a/tests/test_e2e/test_relations.py +++ b/tests/test_e2e/test_relations.py @@ -407,7 +407,7 @@ def test_relation_ids(): initial_id = _next_relation_id_counter for i in range(10): rel = Relation("foo") - assert rel.relation_id == initial_id + i + assert rel.id == initial_id + i def test_broken_relation_not_in_model_relations(mycharm): diff --git a/tests/test_e2e/test_secrets.py b/tests/test_e2e/test_secrets.py index 318817a1..97e1f3b2 100644 --- a/tests/test_e2e/test_secrets.py +++ b/tests/test_e2e/test_secrets.py @@ -195,6 +195,9 @@ def test_set_legacy_behaviour(mycharm): ) secret.set_content(rev2) + # We need to get the secret again, because ops caches the content in + # the object. + secret: ops_Secret = charm.model.get_secret(label="mylabel") assert ( secret.get_content() == secret.peek_content() @@ -204,6 +207,7 @@ def test_set_legacy_behaviour(mycharm): secret.set_content(rev3) state_out = mgr.run() + secret: ops_Secret = charm.model.get_secret(label="mylabel") assert ( secret.get_content() == secret.peek_content() @@ -505,9 +509,7 @@ def __init__(self, *args): state = State( leader=True, relations=[ - Relation( - "bar", remote_app_name=relation_remote_app, relation_id=relation_id - ) + Relation("bar", remote_app_name=relation_remote_app, id=relation_id) ], )