Merge pull request #88 from canonical/extra-bindings

extra bindings and network model rework [Do not merge]

README.md

@@ -77,7 +77,7 @@ A scenario test consists of three broad steps:
     - optionally, you can use a context manager to get a hold of the charm instance and run assertions on internal APIs and the internal state of the charm and operator framework.
 
 The most basic scenario is one in which all is defaulted and barely any data is
-available. The charm has no config, no relations, no networks, no leadership, and its status is `unknown`.
+available. The charm has no config, no relations, no leadership, and its status is `unknown`.
 
 With that, we can write the simplest possible scenario test:
 
@@ -527,6 +527,26 @@ remote_unit_2_is_joining_event = relation.joined_event(remote_unit_id=2)
 remote_unit_2_is_joining_event = Event('foo-relation-changed', relation=relation, relation_remote_unit_id=2)
 ```
 
+### Networks
+
+Simplifying a bit the Juju "spaces" model, each integration endpoint a charm defines in its metadata is associated with a network. Regardless of whether there is a living relation over that endpoint, that is.  
+
+If your charm has a relation `"foo"` (defined in metadata.yaml), then the charm will be able at runtime to do `self.model.get_binding("foo").network`.
+The network you'll get by doing so is heavily defaulted (see `state.Network.default`) and good for most use-cases because the charm should typically not be concerned about what IP it gets. 
+
+On top of the relation-provided network bindings, a charm can also define some `extra-bindings` in its metadata.yaml and access them at runtime. Note that this is a deprecated feature that should not be relied upon. For completeness, we support it in Scenario.
+
+If you want to, you can override any of these relation or extra-binding associated networks with a custom one by passing it to `State.networks`.
+
+```python
+from scenario import State, Network
+state = State(networks={
+  'foo': Network.default(private_address='4.4.4.4')
+})
+```
+
+Where `foo` can either be the name of an `extra-bindings`-defined binding, or a relation endpoint.
+
 # Containers
 
 When testing a kubernetes charm, you can mock container interactions. When using the null state (`State()`), there will

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 = "[email protected]" }

scenario/consistency_checker.py

@@ -4,7 +4,6 @@
 import os
 from collections import Counter
 from collections.abc import Sequence
-from itertools import chain
 from numbers import Number
 from typing import TYPE_CHECKING, Iterable, List, NamedTuple, Tuple
 
@@ -69,6 +68,7 @@ def check_consistency(
         check_secrets_consistency,
         check_storages_consistency,
         check_relation_consistency,
+        check_network_consistency,
     ):
         results = check(
             state=state,
@@ -386,6 +386,38 @@ def check_secrets_consistency(
     return Results(errors, [])
 
 
+def check_network_consistency(
+    *,
+    state: "State",
+    event: "Event",  # noqa: U100
+    charm_spec: "_CharmSpec",
+    **_kwargs,  # noqa: U101
+) -> Results:
+    errors = []
+
+    meta_bindings = set(charm_spec.meta.get("extra-bindings", ()))
+    all_relations = charm_spec.get_all_relations()
+    non_sub_relations = {
+        endpoint
+        for endpoint, metadata in all_relations
+        if metadata.get("scope") != "container"  # mark of a sub
+    }
+
+    state_bindings = set(state.networks)
+    if diff := state_bindings.difference(meta_bindings.union(non_sub_relations)):
+        errors.append(
+            f"Some network bindings defined in State are not in metadata.yaml: {diff}.",
+        )
+
+    endpoints = {endpoint for endpoint, metadata in all_relations}
+    if collisions := endpoints.intersection(meta_bindings):
+        errors.append(
+            f"Extra bindings and integration endpoints cannot share the same name: {collisions}.",
+        )
+
+    return Results(errors, [])
+
+
 def check_relation_consistency(
     *,
     state: "State",
@@ -394,12 +426,8 @@ def check_relation_consistency(
     **_kwargs,  # noqa: U101
 ) -> 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))
+    all_relations_meta = charm_spec.get_all_relations()
 
     def _get_relations(r):
         try:

scenario/mocking.py

@@ -22,7 +22,7 @@
 from ops.testing import _TestingPebbleClient
 
 from scenario.logger import logger as scenario_logger
-from scenario.state import JujuLogLine, Mount, PeerRelation, Port, Storage
+from scenario.state import JujuLogLine, Mount, Network, PeerRelation, Port, Storage
 
 if TYPE_CHECKING:
     from scenario.context import Context
@@ -144,20 +144,21 @@ def _get_secret(self, id=None, label=None):
             # in scenario, you can create Secret(id="foo"),
             # but ops.Secret will prepend a "secret:" prefix to that ID.
             # we allow getting secret by either version.
-            try:
-                return next(
-                    filter(
-                        lambda s: canonicalize_id(s.id) == canonicalize_id(id),
-                        self._state.secrets,
-                    ),
-                )
-            except StopIteration:
-                raise SecretNotFoundError()
+            secrets = [
+                s
+                for s in self._state.secrets
+                if canonicalize_id(s.id) == canonicalize_id(id)
+            ]
+            if not secrets:
+                raise SecretNotFoundError(id)
+            return secrets[0]
+
         elif label:
-            try:
-                return next(filter(lambda s: s.label == label, self._state.secrets))
-            except StopIteration:
-                raise SecretNotFoundError()
+            secrets = [s for s in self._state.secrets if s.label == label]
+            if not secrets:
+                raise SecretNotFoundError(label)
+            return secrets[0]
+
         else:
             # if all goes well, this should never be reached. ops.model.Secret will check upon
             # instantiation that either an id or a label are set, and raise a TypeError if not.
@@ -238,14 +239,34 @@ def config_get(self):
         return state_config  # full config
 
     def network_get(self, binding_name: str, relation_id: Optional[int] = None):
-        if relation_id:
-            logger.warning("network-get -r not implemented")
-
-        relations = self._state.get_relations(binding_name)
-        if not relations:
+        # validation:
+        extra_bindings = self._charm_spec.meta.get("extra-bindings", ())
+        all_endpoints = self._charm_spec.get_all_relations()
+        non_sub_relations = {
+            name for name, meta in all_endpoints if meta.get("scope") != "container"
+        }
+
+        # - is binding_name a valid binding name?
+        if binding_name in extra_bindings:
+            logger.warning("extra-bindings is a deprecated feature")  # fyi
+
+            # - verify that if the binding is an extra binding, we're not ignoring a relation_id
+            if relation_id is not None:
+                # this should not happen
+                logger.error(
+                    "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.",
+                )
+        # - verify that the binding is a relation endpoint name, but not a subordinate one
+        elif binding_name not in non_sub_relations:
+            logger.error(
+                f"cannot get network binding for {binding_name}: is not a valid relation "
+                f"endpoint name nor an extra-binding.",
+            )
             raise RelationNotFoundError()
 
-        network = next(filter(lambda r: r.name == binding_name, self._state.networks))
+        # We look in State.networks for an override. If not given, we return a default network.
+        network = self._state.networks.get(binding_name, Network.default())
         return network.hook_tool_output_fmt()
 
     # setter methods: these can mutate the state.