From 3c513a43a9d953ccbc92eb1c6b94fa3a703e5b7c Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Thu, 28 Mar 2024 22:59:32 +1300 Subject: [PATCH 01/18] Add support for Pebble notices. --- README.md | 39 +++++++++++++ pyproject.toml | 2 +- scenario/__init__.py | 2 + scenario/consistency_checker.py | 8 +-- scenario/mocking.py | 6 ++ scenario/runtime.py | 19 +++++- scenario/state.py | 97 +++++++++++++++++++++++++++++++ tests/test_consistency_checker.py | 10 ++++ tests/test_e2e/test_deferred.py | 22 ++++++- tests/test_e2e/test_event.py | 2 + tests/test_e2e/test_pebble.py | 22 ++++++- tests/test_e2e/test_state.py | 1 + 12 files changed, 222 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index 4f8b2a81..fbbc9042 100644 --- a/README.md +++ b/README.md @@ -718,6 +718,45 @@ def test_pebble_exec(): ) ``` +### Pebble Notices + +Pebble can generate notices, which Juju will detect, and wake up the charm to +let it know that something has happened in the container. The most common +use-case is Pebble custom notices, which is a mechanism for the workload +application to trigger a charm event. + +When the charm is notified, there might be a queue of existing notices, or just +the one that has triggered the event: + +```python +import ops +import scenario + +class MyCharm(ops.CharmBase): + def __init__(self, framework): + super().__init__(framework) + framework.observe(self.on["cont"].pebble_custom_notice, self._on_notice) + + def _on_notice(self, event): + event.notice.key # == "example.com/c" + for notice in self.unit.get_container("cont").get_notices(): + ... + +ctx = scenario.Context(MyCharm, meta={"name": "foo", "containers": "cont": {}}) +notices = [ + scenario.PebbleNotice(key="example.com/a", occurences=10), + scenario.PebbleNotice(key="example.com/b", last_data={"bar": "baz"}), + scenario.PebbleNotice(key="example.com/c"), +] +cont = scenario.Container(notices=notices) +ctx.run(cont.custom_notice_event, scenario.State(containers=[cont])) +``` + +Note that the `custom_notice_event` is accessed via the container, not the notice, +and is always for the last notice in the list. An `ops.pebble.Notice` does not +know which container it is in, but an `ops.PebbleCustomNoticeEvent` does know +which container did the notifying. + # Storage If your charm defines `storage` in its metadata, you can use `scenario.state.Storage` to instruct Scenario to make (mocked) filesystem storage available to the charm at runtime. diff --git a/pyproject.toml b/pyproject.toml index dc5a4b13..5c46c574 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -18,7 +18,7 @@ license.text = "Apache-2.0" keywords = ["juju", "test"] dependencies = [ - "ops>=2.6", + "ops>=2.10", "PyYAML>=6.0.1", ] readme = "README.md" diff --git a/scenario/__init__.py b/scenario/__init__.py index b8424819..7162772e 100644 --- a/scenario/__init__.py +++ b/scenario/__init__.py @@ -13,6 +13,7 @@ Model, Mount, Network, + PebbleNotice, PeerRelation, Port, Relation, @@ -41,6 +42,7 @@ "ExecOutput", "Mount", "Container", + "PebbleNotice", "Address", "BindAddress", "Network", diff --git a/scenario/consistency_checker.py b/scenario/consistency_checker.py index ea9ad489..843f3ed2 100644 --- a/scenario/consistency_checker.py +++ b/scenario/consistency_checker.py @@ -517,10 +517,10 @@ def check_containers_consistency( # it's fine if you have containers in meta that are not in state.containers (yet), but it's # not fine if: - # - you're processing a pebble-ready event and that container is not in state.containers or + # - you're processing a Pebble event and that container is not in state.containers or # meta.containers if event._is_workload_event: - evt_container_name = event.name[: -len("-pebble-ready")] + evt_container_name = event.name.split("_pebble_")[0] if evt_container_name not in meta_containers: errors.append( f"the event being processed concerns container {evt_container_name!r}, but a " @@ -529,8 +529,8 @@ def check_containers_consistency( if evt_container_name not in state_containers: errors.append( f"the event being processed concerns container {evt_container_name!r}, but a " - f"container with that name is not present in the state. It's odd, but consistent, " - f"if it cannot connect; but it should at least be there.", + f"container with that name is not present in the state. It's odd, but " + f"consistent, if it cannot connect; but it should at least be there.", ) # - a container in state.containers is not in meta.containers diff --git a/scenario/mocking.py b/scenario/mocking.py index d4c7aab3..f19fcaf8 100644 --- a/scenario/mocking.py +++ b/scenario/mocking.py @@ -663,6 +663,12 @@ def __init__( self._root = container_root + # load any existing notices from the state + self._notices: Dict[Tuple[str, str], pebble.Notice] = {} + for container in state.containers: + for notice in container.notices: + self._notices[str(notice.type), notice.key] = notice._to_pebble_notice() + def get_plan(self) -> pebble.Plan: return self._container.plan diff --git a/scenario/runtime.py b/scenario/runtime.py index a6aebab0..acafbe25 100644 --- a/scenario/runtime.py +++ b/scenario/runtime.py @@ -17,7 +17,12 @@ from scenario.capture_events import capture_events from scenario.logger import logger as scenario_logger from scenario.ops_main_mock import NoObserverError -from scenario.state import DeferredEvent, PeerRelation, StoredState +from scenario.state import ( + PEBBLE_CUSTOM_NOTICE_EVENT_SUFFIX, + DeferredEvent, + PeerRelation, + StoredState, +) if TYPE_CHECKING: # pragma: no cover from ops.testing import CharmType @@ -248,6 +253,18 @@ def _get_event_env(self, state: "State", event: "Event", charm_root: Path): if container := event.container: env.update({"JUJU_WORKLOAD_NAME": container.name}) + if event.name.endswith(PEBBLE_CUSTOM_NOTICE_EVENT_SUFFIX): + if not event.container or not event.container.notices: + raise RuntimeError("Pebble notice with no container or notice.") + notice = event.container.notices[-1] + env.update( + { + "JUJU_NOTICE_ID": notice.id, + "JUJU_NOTICE_TYPE": str(notice.type), + "JUJU_NOTICE_KEY": notice.key, + }, + ) + if storage := event.storage: env.update({"JUJU_STORAGE_ID": f"{storage.name}/{storage.index}"}) diff --git a/scenario/state.py b/scenario/state.py index 8d14e493..a79dbeb3 100644 --- a/scenario/state.py +++ b/scenario/state.py @@ -87,6 +87,7 @@ "collect_unit_status", } PEBBLE_READY_EVENT_SUFFIX = "_pebble_ready" +PEBBLE_CUSTOM_NOTICE_EVENT_SUFFIX = "_pebble_custom_notice" RELATION_EVENTS_SUFFIX = { "_relation_changed", "_relation_broken", @@ -609,6 +610,72 @@ class Mount(_DCBase): src: Union[str, Path] +def _now_utc(): + return datetime.datetime.now(tz=datetime.timezone.utc) + + +# Ideally, this would be a subclass of pebble.Notice, but we want to make it +# easier to use in tests by providing sensible default values, but there's no +# default key value, so that would need to be first, and that's not the case +# in pebble.Notice, so it's easier to just be explicit and repetitive here. +@dataclasses.dataclass(frozen=True) +class PebbleNotice(_DCBase): + key: str + """The notice key, a string that differentiates notices of this type. + + This is in the format ``example.com/path``. + """ + + id: str = dataclasses.field(default_factory=lambda: str(uuid4())) + """Unique ID for this notice.""" + + user_id: Optional[int] = None + """UID of the user who may view this notice (None means notice is public).""" + + type: Union[pebble.NoticeType, str] = pebble.NoticeType.CUSTOM + """Type of the notice.""" + + first_occurred: datetime.datetime = dataclasses.field(default_factory=_now_utc) + """The first time one of these notices (type and key combination) occurs.""" + + last_occurred: datetime.datetime = dataclasses.field(default_factory=_now_utc) + """The last time one of these notices occurred.""" + + last_repeated: datetime.datetime = dataclasses.field(default_factory=_now_utc) + """The time this notice was last repeated. + + See Pebble's `Notices documentation `_ + for an explanation of what "repeated" means. + """ + + occurrences: int = 1 + """The number of times one of these notices has occurred.""" + + last_data: Dict[str, str] = dataclasses.field(default_factory=dict) + """Additional data captured from the last occurrence of one of these notices.""" + + repeat_after: Optional[datetime.timedelta] = None + """Minimum time after one of these was last repeated before Pebble will repeat it again.""" + + expire_after: Optional[datetime.timedelta] = None + """How long since one of these last occurred until Pebble will drop the notice.""" + + def _to_pebble_notice(self) -> pebble.Notice: + return pebble.Notice( + id=self.id, + user_id=self.user_id, + type=self.type, + key=self.key, + first_occurred=self.first_occurred, + last_occurred=self.last_occurred, + last_repeated=self.last_repeated, + occurrences=self.occurrences, + last_data=self.last_data, + repeat_after=self.repeat_after, + expire_after=self.expire_after, + ) + + @dataclasses.dataclass(frozen=True) class Container(_DCBase): name: str @@ -646,6 +713,8 @@ class Container(_DCBase): exec_mock: _ExecMock = dataclasses.field(default_factory=dict) + notices: List[PebbleNotice] = dataclasses.field(default_factory=list) + def _render_services(self): # copied over from ops.testing._TestingPebbleClient._render_services() services = {} # type: Dict[str, pebble.Service] @@ -713,6 +782,21 @@ def pebble_ready_event(self): ) return Event(path=normalize_name(self.name + "-pebble-ready"), container=self) + @property + def custom_notice_event(self): + """Sugar to generate a -pebble-custom-notice event for the latest notice.""" + if not self.notices: + raise RuntimeError("This container does not have any notices.") + if not self.can_connect: + logger.warning( + "you **can** fire pebble-custom-notice while the container cannot connect, " + "but that's most likely not what you want.", + ) + return Event( + path=normalize_name(self.name + "-pebble-custom-notice"), + container=self, + ) + _RawStatusLiteral = Literal[ "waiting", @@ -1191,6 +1275,8 @@ def _get_suffix_and_type(s: str) -> Tuple[str, _EventType]: # Whether the event name indicates that this is a workload event. if s.endswith(PEBBLE_READY_EVENT_SUFFIX): return PEBBLE_READY_EVENT_SUFFIX, _EventType.workload + if s.endswith(PEBBLE_CUSTOM_NOTICE_EVENT_SUFFIX): + return PEBBLE_CUSTOM_NOTICE_EVENT_SUFFIX, _EventType.workload if s in BUILTIN_EVENTS: return "", _EventType.builtin @@ -1397,6 +1483,17 @@ def deferred(self, handler: Callable, event_id: int = 1) -> DeferredEvent: snapshot_data = { "container_name": container.name, } + if self._path.suffix == PEBBLE_CUSTOM_NOTICE_EVENT_SUFFIX: + if not container.notices: + raise RuntimeError("Container has no notices.") + notice = container.notices[-1] + snapshot_data.update( + { + "notice_id": notice.id, + "notice_key": notice.key, + "notice_type": str(notice.type), + }, + ) elif self._is_relation_event: # this is a RelationEvent. diff --git a/tests/test_consistency_checker.py b/tests/test_consistency_checker.py index ef92e6d9..55b54ea8 100644 --- a/tests/test_consistency_checker.py +++ b/tests/test_consistency_checker.py @@ -60,6 +60,16 @@ def test_workload_event_without_container(): Event("foo-pebble-ready", container=Container("foo")), _CharmSpec(MyCharm, {"containers": {"foo": {}}}), ) + assert_inconsistent( + State(), + Event("foo-pebble-custom-notice", container=Container("foo")), + _CharmSpec(MyCharm, {}), + ) + assert_consistent( + State(containers=[Container("foo")]), + Event("foo-pebble-custom-notice", container=Container("foo")), + _CharmSpec(MyCharm, {"containers": {"foo": {}}}), + ) def test_container_meta_mismatch(): diff --git a/tests/test_e2e/test_deferred.py b/tests/test_e2e/test_deferred.py index b084f6ff..4461cd3b 100644 --- a/tests/test_e2e/test_deferred.py +++ b/tests/test_e2e/test_deferred.py @@ -12,7 +12,14 @@ from ops.framework import Framework from scenario import Context -from scenario.state import Container, DeferredEvent, Relation, State, deferred +from scenario.state import ( + Container, + DeferredEvent, + PebbleNotice, + Relation, + State, + deferred, +) from tests.helpers import trigger CHARM_CALLED = 0 @@ -97,6 +104,19 @@ def test_deferred_workload_evt(mycharm): assert asdict(evt2) == asdict(evt1) +def test_deferred_notice_evt(mycharm): + notice = PebbleNotice(key="example.com/bar") + ctr = Container("foo", notices=[notice]) + evt1 = ctr.custom_notice_event.deferred(handler=mycharm._on_event) + evt2 = deferred( + event="foo_pebble_custom_notice", + handler=mycharm._on_event, + container=ctr, + ) + + assert asdict(evt2) == asdict(evt1) + + def test_deferred_relation_event(mycharm): mycharm.defer_next = 2 diff --git a/tests/test_e2e/test_event.py b/tests/test_e2e/test_event.py index 0dd50077..07c8d30a 100644 --- a/tests/test_e2e/test_event.py +++ b/tests/test_e2e/test_event.py @@ -17,6 +17,8 @@ ("foo_bar_baz_storage_detaching", _EventType.storage), ("foo_pebble_ready", _EventType.workload), ("foo_bar_baz_pebble_ready", _EventType.workload), + ("foo_pebble_custom_notice", _EventType.workload), + ("foo_bar_baz_pebble_custom_notice", _EventType.workload), ("secret_removed", _EventType.secret), ("pre_commit", _EventType.framework), ("commit", _EventType.framework), diff --git a/tests/test_e2e/test_pebble.py b/tests/test_e2e/test_pebble.py index b87b350d..ec536e0e 100644 --- a/tests/test_e2e/test_pebble.py +++ b/tests/test_e2e/test_pebble.py @@ -8,7 +8,7 @@ from ops.pebble import ExecError, ServiceStartup, ServiceStatus from scenario import Context -from scenario.state import Container, ExecOutput, Mount, Port, State +from scenario.state import Container, ExecOutput, Mount, PebbleNotice, Port, State from tests.helpers import trigger @@ -365,3 +365,23 @@ def test_exec_wait_output_error(charm_cls): proc = container.exec(["foo"]) with pytest.raises(ExecError): proc.wait_output() + + +def test_pebble_custom_notice(charm_cls): + notices = [ + PebbleNotice(key="example.com/foo"), + PebbleNotice(key="example.com/bar", last_data={"a": "b"}), + PebbleNotice(key="example.com/baz", occurrences=42), + ] + cont = Container( + name="foo", + can_connect=True, + notices=notices, + ) + + state = State(containers=[cont]) + with Context(charm_cls, meta={"name": "foo", "containers": {"foo": {}}}).manager( + cont.custom_notice_event, state + ) as mgr: + container = mgr.charm.unit.get_container("foo") + assert container.get_notices() == [n._to_pebble_notice() for n in notices] diff --git a/tests/test_e2e/test_state.py b/tests/test_e2e/test_state.py index d92e5b12..838426ae 100644 --- a/tests/test_e2e/test_state.py +++ b/tests/test_e2e/test_state.py @@ -19,6 +19,7 @@ "storage_detaching", "action", "pebble_ready", + "pebble_custom_notice", } From db76e53e625ce75699ceb1b84b4ae7f53e16c9da Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Thu, 4 Apr 2024 11:58:39 +1300 Subject: [PATCH 02/18] Adjust examples in doc. --- scenario/state.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/scenario/state.py b/scenario/state.py index a79dbeb3..8ffb8ced 100644 --- a/scenario/state.py +++ b/scenario/state.py @@ -623,7 +623,8 @@ class PebbleNotice(_DCBase): key: str """The notice key, a string that differentiates notices of this type. - This is in the format ``example.com/path``. + This is in the format ``domain/path``; for example: + ``canonical.com/postgresql/backup`` or ``example.com/mycharm/notice``. """ id: str = dataclasses.field(default_factory=lambda: str(uuid4())) From 653e631e689bde776a52e3ede0b14553d619a84b Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Thu, 4 Apr 2024 12:03:58 +1300 Subject: [PATCH 03/18] Rename Notice.id to Notice.notice_id. --- scenario/runtime.py | 2 +- scenario/state.py | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/scenario/runtime.py b/scenario/runtime.py index acafbe25..d3d5bd84 100644 --- a/scenario/runtime.py +++ b/scenario/runtime.py @@ -259,7 +259,7 @@ def _get_event_env(self, state: "State", event: "Event", charm_root: Path): notice = event.container.notices[-1] env.update( { - "JUJU_NOTICE_ID": notice.id, + "JUJU_NOTICE_ID": notice.notice_id, "JUJU_NOTICE_TYPE": str(notice.type), "JUJU_NOTICE_KEY": notice.key, }, diff --git a/scenario/state.py b/scenario/state.py index 8ffb8ced..634773f1 100644 --- a/scenario/state.py +++ b/scenario/state.py @@ -627,7 +627,7 @@ class PebbleNotice(_DCBase): ``canonical.com/postgresql/backup`` or ``example.com/mycharm/notice``. """ - id: str = dataclasses.field(default_factory=lambda: str(uuid4())) + notice_id: str = dataclasses.field(default_factory=lambda: str(uuid4())) """Unique ID for this notice.""" user_id: Optional[int] = None @@ -663,7 +663,7 @@ class PebbleNotice(_DCBase): def _to_pebble_notice(self) -> pebble.Notice: return pebble.Notice( - id=self.id, + id=self.notice_id, user_id=self.user_id, type=self.type, key=self.key, @@ -1490,7 +1490,7 @@ def deferred(self, handler: Callable, event_id: int = 1) -> DeferredEvent: notice = container.notices[-1] snapshot_data.update( { - "notice_id": notice.id, + "notice_id": notice.notice_id, "notice_key": notice.key, "notice_type": str(notice.type), }, From b9daa695b7ada9b8731fbdc41ef5a6dff7c49de9 Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Thu, 4 Apr 2024 13:00:36 +1300 Subject: [PATCH 04/18] Go back to PebbleNotice.id. --- scenario/runtime.py | 2 +- scenario/state.py | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/scenario/runtime.py b/scenario/runtime.py index d3d5bd84..acafbe25 100644 --- a/scenario/runtime.py +++ b/scenario/runtime.py @@ -259,7 +259,7 @@ def _get_event_env(self, state: "State", event: "Event", charm_root: Path): notice = event.container.notices[-1] env.update( { - "JUJU_NOTICE_ID": notice.notice_id, + "JUJU_NOTICE_ID": notice.id, "JUJU_NOTICE_TYPE": str(notice.type), "JUJU_NOTICE_KEY": notice.key, }, diff --git a/scenario/state.py b/scenario/state.py index 634773f1..8ffb8ced 100644 --- a/scenario/state.py +++ b/scenario/state.py @@ -627,7 +627,7 @@ class PebbleNotice(_DCBase): ``canonical.com/postgresql/backup`` or ``example.com/mycharm/notice``. """ - notice_id: str = dataclasses.field(default_factory=lambda: str(uuid4())) + id: str = dataclasses.field(default_factory=lambda: str(uuid4())) """Unique ID for this notice.""" user_id: Optional[int] = None @@ -663,7 +663,7 @@ class PebbleNotice(_DCBase): def _to_pebble_notice(self) -> pebble.Notice: return pebble.Notice( - id=self.notice_id, + id=self.id, user_id=self.user_id, type=self.type, key=self.key, @@ -1490,7 +1490,7 @@ def deferred(self, handler: Callable, event_id: int = 1) -> DeferredEvent: notice = container.notices[-1] snapshot_data.update( { - "notice_id": notice.notice_id, + "notice_id": notice.id, "notice_key": notice.key, "notice_type": str(notice.type), }, From 38f7071608883f8f8be1d137fc38f6b93e5f388c Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Thu, 4 Apr 2024 13:50:43 +1300 Subject: [PATCH 05/18] Adjustments per code review. --- README.md | 16 ++++++++++-- scenario/consistency_checker.py | 6 +++++ scenario/mocking.py | 2 +- scenario/runtime.py | 12 ++------- scenario/state.py | 46 +++++++++++++++++++++------------ tests/test_e2e/test_deferred.py | 3 ++- tests/test_e2e/test_pebble.py | 7 +++-- 7 files changed, 58 insertions(+), 34 deletions(-) diff --git a/README.md b/README.md index 7959113b..f5053b6d 100644 --- a/README.md +++ b/README.md @@ -745,13 +745,25 @@ notices = [ scenario.PebbleNotice(key="example.com/c"), ] cont = scenario.Container(notices=notices) -ctx.run(cont.custom_notice_event, scenario.State(containers=[cont])) +ctx.run(cont.notice_event, scenario.State(containers=[cont])) ``` Note that the `custom_notice_event` is accessed via the container, not the notice, and is always for the last notice in the list. An `ops.pebble.Notice` does not know which container it is in, but an `ops.PebbleCustomNoticeEvent` does know -which container did the notifying. +which container did the notifying. If you need to generate an event for a different +notice, you can override the notice: + +```python +ctx = scenario.Context(MyCharm, meta={"name": "foo", "containers": "cont": {}}) +notices = [ + scenario.PebbleNotice(key="example.com/a", occurences=10), + scenario.PebbleNotice(key="example.com/b", last_data={"bar": "baz"}), + scenario.PebbleNotice(key="example.com/c"), +] +cont = scenario.Container(notices=notices) +ctx.run(cont.notice_event(notice=notices[0]), scenario.State(containers=[cont])) +``` ## Storage diff --git a/scenario/consistency_checker.py b/scenario/consistency_checker.py index 843f3ed2..7803ce5e 100644 --- a/scenario/consistency_checker.py +++ b/scenario/consistency_checker.py @@ -513,6 +513,7 @@ def check_containers_consistency( meta = charm_spec.meta meta_containers = list(map(normalize_name, meta.get("containers", {}))) state_containers = [normalize_name(c.name) for c in state.containers] + all_notices = [notice.id for c in state.containers for notice in c.notices] errors = [] # it's fine if you have containers in meta that are not in state.containers (yet), but it's @@ -532,6 +533,11 @@ def check_containers_consistency( f"container with that name is not present in the state. It's odd, but " f"consistent, if it cannot connect; but it should at least be there.", ) + if event.notice and event.notice.id not in all_notices: + errors.append( + f"the event being processed concerns notice {event.notice!r}, but that " + "notice is not in any of the containers present in the state.", + ) # - a container in state.containers is not in meta.containers if diff := (set(state_containers).difference(set(meta_containers))): diff --git a/scenario/mocking.py b/scenario/mocking.py index f19fcaf8..6e1f9d1e 100644 --- a/scenario/mocking.py +++ b/scenario/mocking.py @@ -667,7 +667,7 @@ def __init__( self._notices: Dict[Tuple[str, str], pebble.Notice] = {} for container in state.containers: for notice in container.notices: - self._notices[str(notice.type), notice.key] = notice._to_pebble_notice() + self._notices[str(notice.type), notice.key] = notice._to_ops_notice() def get_plan(self) -> pebble.Plan: return self._container.plan diff --git a/scenario/runtime.py b/scenario/runtime.py index acafbe25..1ded1b4f 100644 --- a/scenario/runtime.py +++ b/scenario/runtime.py @@ -17,12 +17,7 @@ from scenario.capture_events import capture_events from scenario.logger import logger as scenario_logger from scenario.ops_main_mock import NoObserverError -from scenario.state import ( - PEBBLE_CUSTOM_NOTICE_EVENT_SUFFIX, - DeferredEvent, - PeerRelation, - StoredState, -) +from scenario.state import DeferredEvent, PeerRelation, StoredState if TYPE_CHECKING: # pragma: no cover from ops.testing import CharmType @@ -253,10 +248,7 @@ def _get_event_env(self, state: "State", event: "Event", charm_root: Path): if container := event.container: env.update({"JUJU_WORKLOAD_NAME": container.name}) - if event.name.endswith(PEBBLE_CUSTOM_NOTICE_EVENT_SUFFIX): - if not event.container or not event.container.notices: - raise RuntimeError("Pebble notice with no container or notice.") - notice = event.container.notices[-1] + if notice := event.notice: env.update( { "JUJU_NOTICE_ID": notice.id, diff --git a/scenario/state.py b/scenario/state.py index 8ffb8ced..5ab9810b 100644 --- a/scenario/state.py +++ b/scenario/state.py @@ -614,10 +614,17 @@ def _now_utc(): return datetime.datetime.now(tz=datetime.timezone.utc) -# Ideally, this would be a subclass of pebble.Notice, but we want to make it -# easier to use in tests by providing sensible default values, but there's no -# default key value, so that would need to be first, and that's not the case -# in pebble.Notice, so it's easier to just be explicit and repetitive here. +_next_notice_id_counter = 1 + + +def next_notice_id(update=True): + global _next_notice_id_counter + cur = _next_notice_id_counter + if update: + _next_notice_id_counter += 1 + return str(cur) + + @dataclasses.dataclass(frozen=True) class PebbleNotice(_DCBase): key: str @@ -627,7 +634,7 @@ class PebbleNotice(_DCBase): ``canonical.com/postgresql/backup`` or ``example.com/mycharm/notice``. """ - id: str = dataclasses.field(default_factory=lambda: str(uuid4())) + id: str = dataclasses.field(default_factory=next_notice_id) """Unique ID for this notice.""" user_id: Optional[int] = None @@ -661,7 +668,7 @@ class PebbleNotice(_DCBase): expire_after: Optional[datetime.timedelta] = None """How long since one of these last occurred until Pebble will drop the notice.""" - def _to_pebble_notice(self) -> pebble.Notice: + def _to_ops_notice(self) -> pebble.Notice: return pebble.Notice( id=self.id, user_id=self.user_id, @@ -784,18 +791,24 @@ def pebble_ready_event(self): return Event(path=normalize_name(self.name + "-pebble-ready"), container=self) @property - def custom_notice_event(self): + def notice_event(self): """Sugar to generate a -pebble-custom-notice event for the latest notice.""" if not self.notices: raise RuntimeError("This container does not have any notices.") + # We assume this event is about the most recent notice. + notice = self.notices[-1] + if notice.type != pebble.NoticeType.CUSTOM: + raise RuntimeError("Scenario only knows about custom notices at this time.") + suffix = PEBBLE_CUSTOM_NOTICE_EVENT_SUFFIX.replace("_", "-") if not self.can_connect: logger.warning( "you **can** fire pebble-custom-notice while the container cannot connect, " "but that's most likely not what you want.", ) return Event( - path=normalize_name(self.name + "-pebble-custom-notice"), + path=normalize_name(self.name + suffix), container=self, + notice=notice, ) @@ -1304,6 +1317,9 @@ class Event(_DCBase): # if this is a workload (container) event, the container it refers to container: Optional[Container] = None + # if this is a Pebble notice event, the notice it refers to + notice: Optional[PebbleNotice] = None + # if this is an action event, the Action instance action: Optional["Action"] = None @@ -1484,15 +1500,12 @@ def deferred(self, handler: Callable, event_id: int = 1) -> DeferredEvent: snapshot_data = { "container_name": container.name, } - if self._path.suffix == PEBBLE_CUSTOM_NOTICE_EVENT_SUFFIX: - if not container.notices: - raise RuntimeError("Container has no notices.") - notice = container.notices[-1] + if self.notice: snapshot_data.update( { - "notice_id": notice.id, - "notice_key": notice.key, - "notice_type": str(notice.type), + "notice_id": self.notice.id, + "notice_key": self.notice.key, + "notice_type": str(self.notice.type), }, ) @@ -1558,8 +1571,9 @@ def deferred( event_id: int = 1, relation: Optional["Relation"] = None, container: Optional["Container"] = None, + notice: Optional["PebbleNotice"] = None, ): """Construct a DeferredEvent from an Event or an event name.""" if isinstance(event, str): - event = Event(event, relation=relation, container=container) + event = Event(event, relation=relation, container=container, notice=notice) return event.deferred(handler=handler, event_id=event_id) diff --git a/tests/test_e2e/test_deferred.py b/tests/test_e2e/test_deferred.py index 4461cd3b..d520cdaa 100644 --- a/tests/test_e2e/test_deferred.py +++ b/tests/test_e2e/test_deferred.py @@ -107,11 +107,12 @@ def test_deferred_workload_evt(mycharm): def test_deferred_notice_evt(mycharm): notice = PebbleNotice(key="example.com/bar") ctr = Container("foo", notices=[notice]) - evt1 = ctr.custom_notice_event.deferred(handler=mycharm._on_event) + evt1 = ctr.notice_event.deferred(handler=mycharm._on_event) evt2 = deferred( event="foo_pebble_custom_notice", handler=mycharm._on_event, container=ctr, + notice=notice, ) assert asdict(evt2) == asdict(evt1) diff --git a/tests/test_e2e/test_pebble.py b/tests/test_e2e/test_pebble.py index ec536e0e..edc341d9 100644 --- a/tests/test_e2e/test_pebble.py +++ b/tests/test_e2e/test_pebble.py @@ -380,8 +380,7 @@ def test_pebble_custom_notice(charm_cls): ) state = State(containers=[cont]) - with Context(charm_cls, meta={"name": "foo", "containers": {"foo": {}}}).manager( - cont.custom_notice_event, state - ) as mgr: + ctx = Context(charm_cls, meta={"name": "foo", "containers": {"foo": {}}}) + with ctx.manager(cont.notice_event, state) as mgr: container = mgr.charm.unit.get_container("foo") - assert container.get_notices() == [n._to_pebble_notice() for n in notices] + assert container.get_notices() == [n._to_ops_notice() for n in notices] From 8071e5ff6ba500afb84e72359f97bf0dac3f4a57 Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Thu, 4 Apr 2024 19:42:27 +1300 Subject: [PATCH 06/18] Update to more generic name. --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index f5053b6d..508a6743 100644 --- a/README.md +++ b/README.md @@ -731,7 +731,7 @@ import scenario class MyCharm(ops.CharmBase): def __init__(self, framework): super().__init__(framework) - framework.observe(self.on["cont"].pebble_custom_notice, self._on_notice) + framework.observe(self.on["cont"].pebble_notice, self._on_notice) def _on_notice(self, event): event.notice.key # == "example.com/c" @@ -748,7 +748,7 @@ cont = scenario.Container(notices=notices) ctx.run(cont.notice_event, scenario.State(containers=[cont])) ``` -Note that the `custom_notice_event` is accessed via the container, not the notice, +Note that the `custom_event` is accessed via the container, not the notice, and is always for the last notice in the list. An `ops.pebble.Notice` does not know which container it is in, but an `ops.PebbleCustomNoticeEvent` does know which container did the notifying. If you need to generate an event for a different From 82da90682102c6dc1e5aa33d07fd68737f77a3c0 Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Thu, 4 Apr 2024 19:44:59 +1300 Subject: [PATCH 07/18] Notice IDs are unique, so use a set not a list for __contains__. --- scenario/consistency_checker.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scenario/consistency_checker.py b/scenario/consistency_checker.py index 7803ce5e..9c5b2a83 100644 --- a/scenario/consistency_checker.py +++ b/scenario/consistency_checker.py @@ -513,7 +513,7 @@ def check_containers_consistency( meta = charm_spec.meta meta_containers = list(map(normalize_name, meta.get("containers", {}))) state_containers = [normalize_name(c.name) for c in state.containers] - all_notices = [notice.id for c in state.containers for notice in c.notices] + all_notices = {notice.id for c in state.containers for notice in c.notices} errors = [] # it's fine if you have containers in meta that are not in state.containers (yet), but it's From 050ebac7fd98abb4f694c7d3712e342dba12a69f Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Thu, 4 Apr 2024 19:55:26 +1300 Subject: [PATCH 08/18] Expand the consistent check tests to match new checks. --- tests/test_consistency_checker.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/tests/test_consistency_checker.py b/tests/test_consistency_checker.py index 55b54ea8..e9b1d46e 100644 --- a/tests/test_consistency_checker.py +++ b/tests/test_consistency_checker.py @@ -9,6 +9,7 @@ Container, Event, Network, + PebbleNotice, PeerRelation, Relation, Secret, @@ -65,9 +66,15 @@ def test_workload_event_without_container(): Event("foo-pebble-custom-notice", container=Container("foo")), _CharmSpec(MyCharm, {}), ) + notice = PebbleNotice("example.com/foo") assert_consistent( + State(containers=[Container("foo", notices=[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")), + Event("foo-pebble-custom-notice", container=Container("foo"), notice=notice), _CharmSpec(MyCharm, {"containers": {"foo": {}}}), ) From 8161af7408dd8bb674b17dfab8392f5b85df716a Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Wed, 10 Apr 2024 22:25:35 +1200 Subject: [PATCH 09/18] Update README.md Co-authored-by: PietroPasotti --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 508a6743..8e44c056 100644 --- a/README.md +++ b/README.md @@ -738,7 +738,7 @@ class MyCharm(ops.CharmBase): for notice in self.unit.get_container("cont").get_notices(): ... -ctx = scenario.Context(MyCharm, meta={"name": "foo", "containers": "cont": {}}) +ctx = scenario.Context(MyCharm, meta={"name": "foo", "containers": {"my-container": {}}}) notices = [ scenario.PebbleNotice(key="example.com/a", occurences=10), scenario.PebbleNotice(key="example.com/b", last_data={"bar": "baz"}), From 318273ea74a55b2f0212c8b26d28533abef58dd2 Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Fri, 12 Apr 2024 16:59:32 +1200 Subject: [PATCH 10/18] Update README.md --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 8e44c056..ee877e8d 100644 --- a/README.md +++ b/README.md @@ -731,7 +731,7 @@ import scenario class MyCharm(ops.CharmBase): def __init__(self, framework): super().__init__(framework) - framework.observe(self.on["cont"].pebble_notice, self._on_notice) + framework.observe(self.on["cont"].pebble_custom_notice, self._on_notice) def _on_notice(self, event): event.notice.key # == "example.com/c" From 52c2c93c0463b25b3711cc02457cebea0a10d342 Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Fri, 5 Apr 2024 17:05:18 +1300 Subject: [PATCH 11/18] Rename PebbleNotice to Notice. --- README.md | 12 ++++++------ scenario/__init__.py | 4 ++-- scenario/state.py | 8 ++++---- tests/test_consistency_checker.py | 4 ++-- tests/test_e2e/test_deferred.py | 11 ++--------- tests/test_e2e/test_pebble.py | 8 ++++---- 6 files changed, 20 insertions(+), 27 deletions(-) diff --git a/README.md b/README.md index ee877e8d..ae68a417 100644 --- a/README.md +++ b/README.md @@ -740,9 +740,9 @@ class MyCharm(ops.CharmBase): ctx = scenario.Context(MyCharm, meta={"name": "foo", "containers": {"my-container": {}}}) notices = [ - scenario.PebbleNotice(key="example.com/a", occurences=10), - scenario.PebbleNotice(key="example.com/b", last_data={"bar": "baz"}), - scenario.PebbleNotice(key="example.com/c"), + scenario.Notice(key="example.com/a", occurences=10), + scenario.Notice(key="example.com/b", last_data={"bar": "baz"}), + scenario.Notice(key="example.com/c"), ] cont = scenario.Container(notices=notices) ctx.run(cont.notice_event, scenario.State(containers=[cont])) @@ -757,9 +757,9 @@ notice, you can override the notice: ```python ctx = scenario.Context(MyCharm, meta={"name": "foo", "containers": "cont": {}}) notices = [ - scenario.PebbleNotice(key="example.com/a", occurences=10), - scenario.PebbleNotice(key="example.com/b", last_data={"bar": "baz"}), - scenario.PebbleNotice(key="example.com/c"), + scenario.Notice(key="example.com/a", occurences=10), + scenario.Notice(key="example.com/b", last_data={"bar": "baz"}), + scenario.Notice(key="example.com/c"), ] cont = scenario.Container(notices=notices) ctx.run(cont.notice_event(notice=notices[0]), scenario.State(containers=[cont])) diff --git a/scenario/__init__.py b/scenario/__init__.py index 7162772e..78224506 100644 --- a/scenario/__init__.py +++ b/scenario/__init__.py @@ -13,7 +13,7 @@ Model, Mount, Network, - PebbleNotice, + Notice, PeerRelation, Port, Relation, @@ -42,7 +42,7 @@ "ExecOutput", "Mount", "Container", - "PebbleNotice", + "Notice", "Address", "BindAddress", "Network", diff --git a/scenario/state.py b/scenario/state.py index 5ab9810b..62dc11bd 100644 --- a/scenario/state.py +++ b/scenario/state.py @@ -626,7 +626,7 @@ def next_notice_id(update=True): @dataclasses.dataclass(frozen=True) -class PebbleNotice(_DCBase): +class Notice(_DCBase): key: str """The notice key, a string that differentiates notices of this type. @@ -721,7 +721,7 @@ class Container(_DCBase): exec_mock: _ExecMock = dataclasses.field(default_factory=dict) - notices: List[PebbleNotice] = dataclasses.field(default_factory=list) + notices: List[Notice] = dataclasses.field(default_factory=list) def _render_services(self): # copied over from ops.testing._TestingPebbleClient._render_services() @@ -1318,7 +1318,7 @@ class Event(_DCBase): container: Optional[Container] = None # if this is a Pebble notice event, the notice it refers to - notice: Optional[PebbleNotice] = None + notice: Optional[Notice] = None # if this is an action event, the Action instance action: Optional["Action"] = None @@ -1571,7 +1571,7 @@ def deferred( event_id: int = 1, relation: Optional["Relation"] = None, container: Optional["Container"] = None, - notice: Optional["PebbleNotice"] = None, + notice: Optional["Notice"] = None, ): """Construct a DeferredEvent from an Event or an event name.""" if isinstance(event, str): diff --git a/tests/test_consistency_checker.py b/tests/test_consistency_checker.py index e9b1d46e..bbdecd27 100644 --- a/tests/test_consistency_checker.py +++ b/tests/test_consistency_checker.py @@ -9,7 +9,7 @@ Container, Event, Network, - PebbleNotice, + Notice, PeerRelation, Relation, Secret, @@ -66,7 +66,7 @@ def test_workload_event_without_container(): Event("foo-pebble-custom-notice", container=Container("foo")), _CharmSpec(MyCharm, {}), ) - notice = PebbleNotice("example.com/foo") + notice = Notice("example.com/foo") assert_consistent( State(containers=[Container("foo", notices=[notice])]), Event("foo-pebble-custom-notice", container=Container("foo"), notice=notice), diff --git a/tests/test_e2e/test_deferred.py b/tests/test_e2e/test_deferred.py index d520cdaa..20a16720 100644 --- a/tests/test_e2e/test_deferred.py +++ b/tests/test_e2e/test_deferred.py @@ -12,14 +12,7 @@ from ops.framework import Framework from scenario import Context -from scenario.state import ( - Container, - DeferredEvent, - PebbleNotice, - Relation, - State, - deferred, -) +from scenario.state import Container, DeferredEvent, Notice, Relation, State, deferred from tests.helpers import trigger CHARM_CALLED = 0 @@ -105,7 +98,7 @@ def test_deferred_workload_evt(mycharm): def test_deferred_notice_evt(mycharm): - notice = PebbleNotice(key="example.com/bar") + notice = Notice(key="example.com/bar") ctr = Container("foo", notices=[notice]) evt1 = ctr.notice_event.deferred(handler=mycharm._on_event) evt2 = deferred( diff --git a/tests/test_e2e/test_pebble.py b/tests/test_e2e/test_pebble.py index edc341d9..73791d07 100644 --- a/tests/test_e2e/test_pebble.py +++ b/tests/test_e2e/test_pebble.py @@ -8,7 +8,7 @@ from ops.pebble import ExecError, ServiceStartup, ServiceStatus from scenario import Context -from scenario.state import Container, ExecOutput, Mount, PebbleNotice, Port, State +from scenario.state import Container, ExecOutput, Mount, Notice, Port, State from tests.helpers import trigger @@ -369,9 +369,9 @@ def test_exec_wait_output_error(charm_cls): def test_pebble_custom_notice(charm_cls): notices = [ - PebbleNotice(key="example.com/foo"), - PebbleNotice(key="example.com/bar", last_data={"a": "b"}), - PebbleNotice(key="example.com/baz", occurrences=42), + Notice(key="example.com/foo"), + Notice(key="example.com/bar", last_data={"a": "b"}), + Notice(key="example.com/baz", occurrences=42), ] cont = Container( name="foo", From 1c37b67f85aa03d12965f62d90dad9fe3f0a7703 Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Tue, 16 Apr 2024 12:21:56 +1200 Subject: [PATCH 12/18] Use a simpler name. --- scenario/mocking.py | 2 +- scenario/state.py | 2 +- tests/test_e2e/test_pebble.py | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/scenario/mocking.py b/scenario/mocking.py index 6e1f9d1e..22b487b0 100644 --- a/scenario/mocking.py +++ b/scenario/mocking.py @@ -667,7 +667,7 @@ def __init__( self._notices: Dict[Tuple[str, str], pebble.Notice] = {} for container in state.containers: for notice in container.notices: - self._notices[str(notice.type), notice.key] = notice._to_ops_notice() + self._notices[str(notice.type), notice.key] = notice._to_ops() def get_plan(self) -> pebble.Plan: return self._container.plan diff --git a/scenario/state.py b/scenario/state.py index 62dc11bd..7108c212 100644 --- a/scenario/state.py +++ b/scenario/state.py @@ -668,7 +668,7 @@ class Notice(_DCBase): expire_after: Optional[datetime.timedelta] = None """How long since one of these last occurred until Pebble will drop the notice.""" - def _to_ops_notice(self) -> pebble.Notice: + def _to_ops(self) -> pebble.Notice: return pebble.Notice( id=self.id, user_id=self.user_id, diff --git a/tests/test_e2e/test_pebble.py b/tests/test_e2e/test_pebble.py index 73791d07..04dd75f1 100644 --- a/tests/test_e2e/test_pebble.py +++ b/tests/test_e2e/test_pebble.py @@ -383,4 +383,4 @@ def test_pebble_custom_notice(charm_cls): ctx = Context(charm_cls, meta={"name": "foo", "containers": {"foo": {}}}) with ctx.manager(cont.notice_event, state) as mgr: container = mgr.charm.unit.get_container("foo") - assert container.get_notices() == [n._to_ops_notice() for n in notices] + assert container.get_notices() == [n._to_ops() for n in notices] From f9928d0692aa9e0b164fbd76ede8e77aaf807f48 Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Tue, 16 Apr 2024 14:11:22 +1200 Subject: [PATCH 13/18] str(x) only works for enum.StrEnum, not enum.Enum. --- scenario/mocking.py | 6 +++++- scenario/runtime.py | 7 ++++++- scenario/state.py | 6 +++++- 3 files changed, 16 insertions(+), 3 deletions(-) diff --git a/scenario/mocking.py b/scenario/mocking.py index 22b487b0..7fc8f4c9 100644 --- a/scenario/mocking.py +++ b/scenario/mocking.py @@ -667,7 +667,11 @@ def __init__( self._notices: Dict[Tuple[str, str], pebble.Notice] = {} for container in state.containers: for notice in container.notices: - self._notices[str(notice.type), notice.key] = notice._to_ops() + if hasattr(notice.type, "value"): + notice_type = cast(pebble.NoticeType, notice.type).value + else: + notice_type = str(notice.type) + self._notices[notice_type, notice.key] = notice._to_ops() def get_plan(self) -> pebble.Plan: return self._container.plan diff --git a/scenario/runtime.py b/scenario/runtime.py index 1ded1b4f..3cb67f0b 100644 --- a/scenario/runtime.py +++ b/scenario/runtime.py @@ -11,6 +11,7 @@ from typing import TYPE_CHECKING, Dict, List, Optional, Type, Union import yaml +from ops import pebble from ops.framework import _event_regex from ops.storage import NoSnapshotError, SQLiteStorage @@ -249,10 +250,14 @@ def _get_event_env(self, state: "State", event: "Event", charm_root: Path): env.update({"JUJU_WORKLOAD_NAME": container.name}) if notice := event.notice: + if hasattr(notice.type, "value"): + notice_type = typing.cast(pebble.NoticeType, notice.type).value + else: + notice_type = str(notice.type) env.update( { "JUJU_NOTICE_ID": notice.id, - "JUJU_NOTICE_TYPE": str(notice.type), + "JUJU_NOTICE_TYPE": notice_type, "JUJU_NOTICE_KEY": notice.key, }, ) diff --git a/scenario/state.py b/scenario/state.py index 7108c212..f898c9bc 100644 --- a/scenario/state.py +++ b/scenario/state.py @@ -1501,11 +1501,15 @@ def deferred(self, handler: Callable, event_id: int = 1) -> DeferredEvent: "container_name": container.name, } if self.notice: + if hasattr(self.notice.type, "value"): + notice_type = cast(pebble.NoticeType, self.notice.type).value + else: + notice_type = str(self.notice.type) snapshot_data.update( { "notice_id": self.notice.id, "notice_key": self.notice.key, - "notice_type": str(self.notice.type), + "notice_type": notice_type, }, ) From a35cc2c7a15e5003d2ca5d272b8c6452b817f902 Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Tue, 16 Apr 2024 14:11:42 +1200 Subject: [PATCH 14/18] Add tests that verify the notice in the event works as expected. --- tests/test_e2e/test_pebble.py | 63 ++++++++++++++++++++++++++++++++--- 1 file changed, 59 insertions(+), 4 deletions(-) diff --git a/tests/test_e2e/test_pebble.py b/tests/test_e2e/test_pebble.py index 04dd75f1..a750a070 100644 --- a/tests/test_e2e/test_pebble.py +++ b/tests/test_e2e/test_pebble.py @@ -1,8 +1,9 @@ +import datetime import tempfile from pathlib import Path import pytest -from ops import pebble +from ops import PebbleCustomNoticeEvent, pebble from ops.charm import CharmBase from ops.framework import Framework from ops.pebble import ExecError, ServiceStartup, ServiceStatus @@ -373,14 +374,68 @@ def test_pebble_custom_notice(charm_cls): Notice(key="example.com/bar", last_data={"a": "b"}), Notice(key="example.com/baz", occurrences=42), ] - cont = Container( + container = Container( name="foo", can_connect=True, notices=notices, ) - state = State(containers=[cont]) + state = State(containers=[container]) ctx = Context(charm_cls, meta={"name": "foo", "containers": {"foo": {}}}) - with ctx.manager(cont.notice_event, state) as mgr: + with ctx.manager(container.notice_event, state) as mgr: container = mgr.charm.unit.get_container("foo") assert container.get_notices() == [n._to_ops() for n in notices] + + +def test_pebble_custom_notice_in_charm(): + key = "example.com/test/charm" + data = {"foo": "bar"} + user_id = 100 + first_occurred = datetime.datetime(1979, 1, 25, 11, 0, 0) + last_occured = datetime.datetime(2006, 8, 28, 13, 28, 0) + last_repeated = datetime.datetime(2023, 9, 4, 9, 0, 0) + occurrences = 42 + repeat_after = datetime.timedelta(days=7) + expire_after = datetime.timedelta(days=365) + + class MyCharm(CharmBase): + def __init__(self, framework): + super().__init__(framework) + framework.observe(self.on.foo_pebble_custom_notice, self._on_custom_notice) + + def _on_custom_notice(self, event: PebbleCustomNoticeEvent): + notice = event.notice + assert notice.type == pebble.NoticeType.CUSTOM + assert notice.key == key + assert notice.last_data == data + assert notice.user_id == user_id + assert notice.first_occurred == first_occurred + assert notice.last_occurred == last_occured + assert notice.last_repeated == last_repeated + assert notice.occurrences == occurrences + assert notice.repeat_after == repeat_after + assert notice.expire_after == expire_after + + notices = [ + Notice("example.com/test/other"), + Notice(key, last_data={"foo": "baz"}), + Notice( + key, + last_data=data, + user_id=user_id, + first_occurred=first_occurred, + last_occurred=last_occured, + last_repeated=last_repeated, + occurrences=occurrences, + repeat_after=repeat_after, + expire_after=expire_after, + ), + ] + container = Container( + name="foo", + can_connect=True, + notices=notices, + ) + state = State(containers=[container]) + ctx = Context(MyCharm, meta={"name": "foo", "containers": {"foo": {}}}) + ctx.run(container.notice_event, state) From 0749de2a1c18af9bc8877a592d5cab4af3700a3e Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Fri, 24 May 2024 12:40:16 +1200 Subject: [PATCH 15/18] Adjust the way that the notice event is selected. --- README.md | 19 +------------ scenario/state.py | 50 ++++++++++++++++++++------------- tests/test_e2e/test_deferred.py | 2 +- tests/test_e2e/test_pebble.py | 6 ++-- 4 files changed, 36 insertions(+), 41 deletions(-) diff --git a/README.md b/README.md index ae68a417..e470f29d 100644 --- a/README.md +++ b/README.md @@ -745,24 +745,7 @@ notices = [ scenario.Notice(key="example.com/c"), ] cont = scenario.Container(notices=notices) -ctx.run(cont.notice_event, scenario.State(containers=[cont])) -``` - -Note that the `custom_event` is accessed via the container, not the notice, -and is always for the last notice in the list. An `ops.pebble.Notice` does not -know which container it is in, but an `ops.PebbleCustomNoticeEvent` does know -which container did the notifying. If you need to generate an event for a different -notice, you can override the notice: - -```python -ctx = scenario.Context(MyCharm, meta={"name": "foo", "containers": "cont": {}}) -notices = [ - scenario.Notice(key="example.com/a", occurences=10), - scenario.Notice(key="example.com/b", last_data={"bar": "baz"}), - scenario.Notice(key="example.com/c"), -] -cont = scenario.Container(notices=notices) -ctx.run(cont.notice_event(notice=notices[0]), scenario.State(containers=[cont])) +ctx.run(container.get_notice("example.com/c").event, scenario.State(containers=[cont])) ``` ## Storage diff --git a/scenario/state.py b/scenario/state.py index f898c9bc..c7e056b5 100644 --- a/scenario/state.py +++ b/scenario/state.py @@ -684,6 +684,22 @@ def _to_ops(self) -> pebble.Notice: ) +@dataclasses.dataclass(frozen=True) +class BoundNotice(_DCBase): + notice: Notice + container: "Container" + + @property + def event(self): + """Sugar to generate a -pebble-custom-notice event for this notice.""" + suffix = PEBBLE_CUSTOM_NOTICE_EVENT_SUFFIX.replace("_", "-") + return Event( + path=normalize_name(self.container.name + suffix), + container=self.container, + notice=self.notice, + ) + + @dataclasses.dataclass(frozen=True) class Container(_DCBase): name: str @@ -790,25 +806,21 @@ def pebble_ready_event(self): ) return Event(path=normalize_name(self.name + "-pebble-ready"), container=self) - @property - def notice_event(self): - """Sugar to generate a -pebble-custom-notice event for the latest notice.""" - if not self.notices: - raise RuntimeError("This container does not have any notices.") - # We assume this event is about the most recent notice. - notice = self.notices[-1] - if notice.type != pebble.NoticeType.CUSTOM: - raise RuntimeError("Scenario only knows about custom notices at this time.") - suffix = PEBBLE_CUSTOM_NOTICE_EVENT_SUFFIX.replace("_", "-") - if not self.can_connect: - logger.warning( - "you **can** fire pebble-custom-notice while the container cannot connect, " - "but that's most likely not what you want.", - ) - return Event( - path=normalize_name(self.name + suffix), - container=self, - notice=notice, + def get_notice( + self, + key: str, + notice_type: pebble.NoticeType = pebble.NoticeType.CUSTOM, + ) -> BoundNotice: + """Get a Pebble notice by key and type. + + Raises: + KeyError: if the notice is not found. + """ + for notice in self.notices: + if notice.key == key and notice.type == notice_type: + return BoundNotice(notice, self) + raise KeyError( + f"{self.name} does not have a notice with key {key} and type {notice_type}", ) diff --git a/tests/test_e2e/test_deferred.py b/tests/test_e2e/test_deferred.py index 20a16720..a96100bc 100644 --- a/tests/test_e2e/test_deferred.py +++ b/tests/test_e2e/test_deferred.py @@ -100,7 +100,7 @@ def test_deferred_workload_evt(mycharm): def test_deferred_notice_evt(mycharm): notice = Notice(key="example.com/bar") ctr = Container("foo", notices=[notice]) - evt1 = ctr.notice_event.deferred(handler=mycharm._on_event) + evt1 = ctr.get_notice("example.com/bar").event.deferred(handler=mycharm._on_event) evt2 = deferred( event="foo_pebble_custom_notice", handler=mycharm._on_event, diff --git a/tests/test_e2e/test_pebble.py b/tests/test_e2e/test_pebble.py index a750a070..e5c16bf7 100644 --- a/tests/test_e2e/test_pebble.py +++ b/tests/test_e2e/test_pebble.py @@ -382,7 +382,7 @@ def test_pebble_custom_notice(charm_cls): state = State(containers=[container]) ctx = Context(charm_cls, meta={"name": "foo", "containers": {"foo": {}}}) - with ctx.manager(container.notice_event, state) as mgr: + with ctx.manager(container.get_notice("example.com/baz").event, state) as mgr: container = mgr.charm.unit.get_container("foo") assert container.get_notices() == [n._to_ops() for n in notices] @@ -418,7 +418,7 @@ def _on_custom_notice(self, event: PebbleCustomNoticeEvent): notices = [ Notice("example.com/test/other"), - Notice(key, last_data={"foo": "baz"}), + Notice("example.org/test/charm", last_data={"foo": "baz"}), Notice( key, last_data=data, @@ -438,4 +438,4 @@ def _on_custom_notice(self, event: PebbleCustomNoticeEvent): ) state = State(containers=[container]) ctx = Context(MyCharm, meta={"name": "foo", "containers": {"foo": {}}}) - ctx.run(container.notice_event, state) + ctx.run(container.get_notice(key).event, state) From 7ba3d3dd85beffe533c6e343a02912c8fb5c0078 Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Fri, 7 Jun 2024 18:22:14 +1200 Subject: [PATCH 16/18] Make BoundNotice private. --- scenario/state.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/scenario/state.py b/scenario/state.py index c7e056b5..ee2bd10e 100644 --- a/scenario/state.py +++ b/scenario/state.py @@ -685,7 +685,7 @@ def _to_ops(self) -> pebble.Notice: @dataclasses.dataclass(frozen=True) -class BoundNotice(_DCBase): +class _BoundNotice(_DCBase): notice: Notice container: "Container" @@ -810,7 +810,7 @@ def get_notice( self, key: str, notice_type: pebble.NoticeType = pebble.NoticeType.CUSTOM, - ) -> BoundNotice: + ) -> _BoundNotice: """Get a Pebble notice by key and type. Raises: @@ -818,7 +818,7 @@ def get_notice( """ for notice in self.notices: if notice.key == key and notice.type == notice_type: - return BoundNotice(notice, self) + return _BoundNotice(notice, self) raise KeyError( f"{self.name} does not have a notice with key {key} and type {notice_type}", ) From f52d5d971894ce9492e37892a4b52a2d0bbfcbc3 Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Tue, 11 Jun 2024 19:25:53 +1200 Subject: [PATCH 17/18] Add version bump. --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 5c46c574..5907ae71 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -8,7 +8,7 @@ build-backend = "setuptools.build_meta" [project] name = "ops-scenario" -version = "6.0.3" +version = "6.1.0" authors = [ { name = "Pietro Pasotti", email = "pietro.pasotti@canonical.com" } From cb6cbb25615851c84ac2955319582a68de35e2ed Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Tue, 11 Jun 2024 19:28:02 +1200 Subject: [PATCH 18/18] Avoid unnecessary conversions, per review. --- .pre-commit-config.yaml | 3 +-- scenario/state.py | 4 ++-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index f5fcd9e8..0f8dfeca 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -13,10 +13,9 @@ repos: - id: end-of-file-fixer - id: trailing-whitespace - repo: https://github.com/asottile/add-trailing-comma - rev: v2.4.0 + rev: v3.1.0 hooks: - id: add-trailing-comma - args: [--py36-plus] - repo: https://github.com/asottile/pyupgrade rev: v3.3.1 hooks: diff --git a/scenario/state.py b/scenario/state.py index ee2bd10e..e290eaa4 100644 --- a/scenario/state.py +++ b/scenario/state.py @@ -692,9 +692,9 @@ class _BoundNotice(_DCBase): @property def event(self): """Sugar to generate a -pebble-custom-notice event for this notice.""" - suffix = PEBBLE_CUSTOM_NOTICE_EVENT_SUFFIX.replace("_", "-") + suffix = PEBBLE_CUSTOM_NOTICE_EVENT_SUFFIX return Event( - path=normalize_name(self.container.name + suffix), + path=normalize_name(self.container.name) + suffix, container=self.container, notice=self.notice, )