Merge pull request #19 from canonical/relation-unit-and-app

Upgrade to Relation

README.md

@@ -136,6 +136,7 @@ def _on_event(self, _event):
 You can verify that the charm has followed the expected path by checking the **unit status history** like so:
 
 ```python
+from charm import MyCharm
 from ops.model import MaintenanceStatus, ActiveStatus, WaitingStatus, UnknownStatus
 from scenario import State
 
@@ -148,14 +149,15 @@ def test_statuses():
       UnknownStatus(),
       MaintenanceStatus('determining who the ruler is...'),
       WaitingStatus('checking this is right...'),
+      ActiveStatus("I am ruled"),
     ]
 ```
 
 Note that the current status is not in the **unit status history**.
 
 Also note that, unless you initialize the State with a preexisting status, the first status in the history will always be `unknown`. That is because, so far as scenario is concerned, each event is "the first event this charm has ever seen".
 
-If you want to simulate a situation in which the charm already has seen some event, and is in a status other than Unknown (the default status every charm is born with), you will have to pass the 'initial status' in State.
+If you want to simulate a situation in which the charm already has seen some event, and is in a status other than Unknown (the default status every charm is born with), you will have to pass the 'initial status' to State.
 
 ```python
 from ops.model import ActiveStatus
@@ -212,6 +214,102 @@ def test_relation_data():
 # which is very idiomatic and superbly explicit. Noice.
 ```
 
+The only mandatory argument to `Relation` (and other relation types, see below) is `endpoint`. The `interface` will be derived from the charm's `metadata.yaml`. When fully defaulted, a relation is 'empty'. There are no remote units, the remote application is called `'remote'` and only has a single unit `remote/0`, and nobody has written any data to the databags yet.
+
+That is typically the state of a relation when the first unit joins it.
+
+When you use `Relation`, you are specifying a regular (conventional) relation. But that is not the only type of relation. There are also
+peer relations and subordinate relations. While in the background the data model is the same, the data access rules and the consistency constraints on them are very different. For example, it does not make sense for a peer relation to have a different 'remote app' than its 'local app', because it's the same application.    
+
+### PeerRelation
+To declare a peer relation, you should use `scenario.state.PeerRelation`.
+The core difference with regular relations is that peer relations do not have a "remote app" (it's this app, in fact).
+So unlike `Relation`, a `PeerRelation` does not have `remote_app_name` or `remote_app_data` arguments. Also, it talks in terms of `peers`:
+- `Relation.remote_unit_ids` maps to `PeerRelation.peers_ids` 
+- `Relation.remote_units_data` maps to `PeerRelation.peers_data` 
+
+```python
+from scenario.state import PeerRelation
+
+relation = PeerRelation(
+    endpoint="peers",
+    peers_data={1: {}, 2: {}, 42: {'foo': 'bar'}},
+)
+```
+
+be mindful when using `PeerRelation` not to include **"this unit"**'s ID in `peers_data` or `peers_ids`, as that would be flagged by the Consistency Checker:
+```python
+from scenario import State, PeerRelation
+
+State(relations=[
+    PeerRelation(
+        endpoint="peers",
+        peers_data={1: {}, 2: {}, 42: {'foo': 'bar'}},
+    )]).trigger("start", ..., unit_id=1)  # invalid: this unit's id cannot be the ID of a peer.
+
+
+```
+
+### SubordinateRelation
+To declare a subordinate relation, you should use `scenario.state.SubordinateRelation`.
+The core difference with regular relations is that subordinate relations always have exactly one remote unit (there is always exactly one primary unit that this unit can see). 
+So unlike `Relation`, a `SubordinateRelation` does not have a `remote_units_data` argument. Instead, it has a `remote_unit_data` taking a single `Dict[str:str]`, and takes the primary unit ID as a separate argument.
+Also, it talks in terms of `primary`:
+- `Relation.remote_unit_ids` becomes `SubordinateRelation.primary_id` (a single ID instead of a list of IDs) 
+- `Relation.remote_units_data` becomes `SubordinateRelation.remote_unit_data` (a single databag instead of a mapping from unit IDs to databags) 
+- `Relation.remote_app_name` maps to `SubordinateRelation.primary_app_name`
+
+```python
+from scenario.state import SubordinateRelation
+
+relation = SubordinateRelation(
+    endpoint="peers",
+    remote_unit_data={"foo": "bar"},
+    primary_app_name="zookeeper",
+    primary_id=42
+)
+relation.primary_name  # "zookeeper/42"
+```
+
+
+## Triggering Relation Events
+If you want to trigger relation events, the easiest way to do so is get a hold of the Relation instance and grab the event from one of its aptly-named properties:
+
+```python
+from scenario import Relation
+relation = Relation(endpoint="foo", interface="bar")
+changed_event = relation.changed_event
+joined_event = relation.joined_event
+# ...
+```
+
+This is in fact syntactic sugar for:
+```python
+from scenario import Relation, Event
+relation = Relation(endpoint="foo", interface="bar")
+changed_event = Event('foo-relation-changed', relation=relation)
+```
+
+The reason for this construction is that the event is associated with some relation-specific metadata, that Scenario needs to set up the process that will run `ops.main` with the right environment variables.
+
+### Additional event parameters
+All relation events have some additional metadata that does not belong in the Relation object, such as, for a relation-joined event, the name of the (remote) unit that is joining the relation. That is what determines what `ops.model.Unit` you get when you get `RelationJoinedEvent().unit` in an event handler.
+
+In order to supply this parameter, you will have to **call** the event object and pass as `remote_unit_id` the id of the remote unit that the event is about.
+The reason that this parameter is not supplied to `Relation` but to relation events, is that the relation already ties 'this app' to some 'remote app' (cfr. the `Relation.remote_app_name` attr), but not to a specific unit. What remote unit this event is about is not a `State` concern but an `Event` one.  
+
+The `remote_unit_id` will default to the first ID found in the relation's `remote_unit_ids`, but if the test you are writing is close to that domain, you should probably override it and pass it manually.
+
+```python
+from scenario import Relation, Event
+relation = Relation(endpoint="foo", interface="bar")
+remote_unit_2_is_joining_event = relation.joined_event(remote_unit_id=2)
+
+# which is syntactic sugar for:
+remote_unit_2_is_joining_event = Event('foo-relation-changed', relation=relation, relation_remote_unit_id=2)
+```
+
+
 ## Containers
 
 When testing a kubernetes charm, you can mock container interactions.

pyproject.toml

@@ -8,7 +8,7 @@ build-backend = "setuptools.build_meta"
 [project]
 name = "ops-scenario"
 
-version = "2.1.3.3"
+version = "2.1.3.4"
 authors = [
     { name = "Pietro Pasotti", email = "[email protected]" }
 ]

scenario/consistency_checker.py

@@ -1,9 +1,11 @@
 import os
+from collections import Counter
+from itertools import chain
 from typing import TYPE_CHECKING, Iterable, NamedTuple, Tuple
 
 from scenario.runtime import InconsistentScenarioError
 from scenario.runtime import logger as scenario_logger
-from scenario.state import _CharmSpec, normalize_name
+from scenario.state import PeerRelation, SubordinateRelation, _CharmSpec, normalize_name
 
 if TYPE_CHECKING:
     from scenario.state import Event, State
@@ -51,6 +53,7 @@ def check_consistency(
         check_config_consistency,
         check_event_consistency,
         check_secrets_consistency,
+        check_relation_consistency,
     ):
         results = check(
             state=state, event=event, charm_spec=charm_spec, juju_version=juju_version
@@ -179,6 +182,61 @@ def check_secrets_consistency(
     return Results(errors, [])
 
 
+def check_relation_consistency(
+    *, state: "State", event: "Event", charm_spec: "_CharmSpec", **_kwargs
+) -> Results:
+    errors = []
+    nonpeer_relations_meta = chain(
+        charm_spec.meta.get("requires", {}).items(),
+        charm_spec.meta.get("provides", {}).items(),
+    )
+    peer_relations_meta = charm_spec.meta.get("peers", {}).items()
+    all_relations_meta = list(chain(nonpeer_relations_meta, peer_relations_meta))
+
+    def _get_relations(r):
+        try:
+            return state.get_relations(r)
+        except ValueError:
+            return ()
+
+    # check relation types
+    for endpoint, _ in peer_relations_meta:
+        for relation in _get_relations(endpoint):
+            if not isinstance(relation, PeerRelation):
+                errors.append(
+                    f"endpoint {endpoint} is a peer relation; "
+                    f"expecting relation to be of type PeerRelation, got {type(relation)}"
+                )
+
+    for endpoint, relation_meta in all_relations_meta:
+        expected_sub = relation_meta.get("scope", "") == "container"
+        relations = _get_relations(endpoint)
+        for relation in relations:
+            is_sub = isinstance(relation, SubordinateRelation)
+            if is_sub and not expected_sub:
+                errors.append(
+                    f"endpoint {endpoint} is not a subordinate relation; "
+                    f"expecting relation to be of type Relation, "
+                    f"got {type(relation)}"
+                )
+            if expected_sub and not is_sub:
+                errors.append(
+                    f"endpoint {endpoint} is not a subordinate relation; "
+                    f"expecting relation to be of type SubordinateRelation, "
+                    f"got {type(relation)}"
+                )
+
+    # check for duplicate endpoint names
+    seen_endpoints = set()
+    for endpoint, relation_meta in all_relations_meta:
+        if endpoint in seen_endpoints:
+            errors.append("duplicate endpoint name in metadata.")
+            break
+        seen_endpoints.add(endpoint)
+
+    return Results(errors, [])
+
+
 def check_containers_consistency(
     *, state: "State", event: "Event", charm_spec: "_CharmSpec", **_kwargs
 ) -> Results:
@@ -209,4 +267,10 @@ def check_containers_consistency(
             f"some containers declared in the state are not specified in metadata. That's not possible. "
             f"Missing from metadata: {diff}."
         )
+
+    # guard against duplicate container names
+    names = Counter(state_containers)
+    if dupes := [n for n in names if names[n] > 1]:
+        errors.append(f"Duplicate container name(s): {dupes}.")
+
     return Results(errors, [])

scenario/fs_mocks.py

@@ -0,0 +1,35 @@
+import pathlib
+from typing import Dict
+
+from ops.testing import _TestingFilesystem, _TestingStorageMount  # noqa
+
+
+# todo consider duplicating the filesystem on State.copy() to be able to diff and have true state snapshots
+class _MockStorageMount(_TestingStorageMount):
+    def __init__(self, location: pathlib.PurePosixPath, src: pathlib.Path):
+        """Creates a new simulated storage mount.
+
+        Args:
+            location: The path within simulated filesystem at which this storage will be mounted.
+            src: The temporary on-disk location where the simulated storage will live.
+        """
+        self._src = src
+        self._location = location
+
+        try:
+            # for some reason this fails if src exists, even though exists_ok=True.
+            super().__init__(location=location, src=src)
+        except FileExistsError:
+            pass
+
+
+class _MockFileSystem(_TestingFilesystem):
+    def __init__(self, mounts: Dict[str, _MockStorageMount]):
+        super().__init__()
+        self._mounts = mounts
+
+    def add_mount(self, *args, **kwargs):
+        raise NotImplementedError("Cannot mutate mounts; declare them all in State.")
+
+    def remove_mount(self, *args, **kwargs):
+        raise NotImplementedError("Cannot mutate mounts; declare them all in State.")

scenario/mocking.py

@@ -2,21 +2,29 @@
 # Copyright 2023 Canonical Ltd.
 # See LICENSE file for licensing details.
 import datetime
-import pathlib
 import random
 from io import StringIO
 from typing import TYPE_CHECKING, Dict, Optional, Tuple, Union
 
 from ops import pebble
 from ops.model import SecretInfo, SecretRotate, _ModelBackend
 from ops.pebble import Client, ExecError
-from ops.testing import _TestingFilesystem, _TestingPebbleClient, _TestingStorageMount
+from ops.testing import _TestingPebbleClient
 
 from scenario.logger import logger as scenario_logger
+from scenario.state import PeerRelation
 
 if TYPE_CHECKING:
     from scenario.state import Container as ContainerSpec
-    from scenario.state import Event, ExecOutput, State, _CharmSpec
+    from scenario.state import (
+        Event,
+        ExecOutput,
+        PeerRelation,
+        Relation,
+        State,
+        SubordinateRelation,
+        _CharmSpec,
+    )
 
 logger = scenario_logger.getChild("mocking")
 
@@ -62,7 +70,9 @@ def get_pebble(self, socket_path: str) -> "Client":
             charm_spec=self._charm_spec,
         )
 
-    def _get_relation_by_id(self, rel_id):
+    def _get_relation_by_id(
+        self, rel_id
+    ) -> Union["Relation", "SubordinateRelation", "PeerRelation"]:
         try:
             return next(
                 filter(lambda r: r.relation_id == rel_id, self._state.relations)
@@ -101,9 +111,9 @@ def relation_get(self, rel_id, obj_name, app):
             return relation.remote_app_data
         elif obj_name == self.unit_name:
             return relation.local_unit_data
-        else:
-            unit_id = obj_name.split("/")[-1]
-            return relation.remote_units_data[int(unit_id)]
+
+        unit_id = int(obj_name.split("/")[-1])
+        return relation._get_databag_for_remote(unit_id)  # noqa
 
     def is_leader(self):
         return self._state.leader
@@ -119,11 +129,14 @@ def relation_ids(self, relation_name):
             if rel.endpoint == relation_name
         ]
 
-    def relation_list(self, relation_id: int):
+    def relation_list(self, relation_id: int) -> Tuple[str]:
         relation = self._get_relation_by_id(relation_id)
+
+        if isinstance(relation, PeerRelation):
+            return tuple(f"{self.app_name}/{unit_id}" for unit_id in relation.peers_ids)
         return tuple(
-            f"{relation.remote_app_name}/{unit_id}"
-            for unit_id in relation.remote_unit_ids
+            f"{relation._remote_app_name}/{unit_id}"  # noqa
+            for unit_id in relation._remote_unit_ids  # noqa
         )
 
     def config_get(self):
@@ -317,35 +330,6 @@ def planned_units(self, *args, **kwargs):
         raise NotImplementedError("planned_units")
 
 
-class _MockStorageMount(_TestingStorageMount):
-    def __init__(self, location: pathlib.PurePosixPath, src: pathlib.Path):
-        """Creates a new simulated storage mount.
-
-        Args:
-            location: The path within simulated filesystem at which this storage will be mounted.
-            src: The temporary on-disk location where the simulated storage will live.
-        """
-        self._src = src
-        self._location = location
-        if (
-            not src.exists()
-        ):  # we need to add this guard because the directory might exist already.
-            src.mkdir(exist_ok=True, parents=True)
-
-
-# todo consider duplicating the filesystem on State.copy() to be able to diff and have true state snapshots
-class _MockFileSystem(_TestingFilesystem):
-    def __init__(self, mounts: Dict[str, _MockStorageMount]):
-        super().__init__()
-        self._mounts = mounts
-
-    def add_mount(self, *args, **kwargs):
-        raise NotImplementedError("Cannot mutate mounts; declare them all in State.")
-
-    def remove_mount(self, *args, **kwargs):
-        raise NotImplementedError("Cannot mutate mounts; declare them all in State.")
-
-
 class _MockPebbleClient(_TestingPebbleClient):
     def __init__(
         self,

scenario/ops_main_mock.py

@@ -14,7 +14,6 @@
 from ops.main import CHARM_STATE_FILE, _Dispatcher, _emit_charm_event, _get_charm_dir
 
 from scenario.logger import logger as scenario_logger
-from scenario.mocking import _MockModelBackend
 
 if TYPE_CHECKING:
     from ops.testing import CharmType
@@ -38,6 +37,9 @@ def main(
     """Set up the charm and dispatch the observed event."""
     charm_class = charm_spec.charm_type
     charm_dir = _get_charm_dir()
+
+    from scenario.mocking import _MockModelBackend
+
     model_backend = _MockModelBackend(  # pyright: reportPrivateUsage=false
         state=state, event=event, charm_spec=charm_spec
     )