Skip to content

Commit

Permalink
Merge branch 'canonical:main' into allow-integer-action-params
Browse files Browse the repository at this point in the history
  • Loading branch information
tonyandrewmeyer committed Oct 19, 2023
2 parents fb307e7 + 55cb4e0 commit 29e545b
Show file tree
Hide file tree
Showing 13 changed files with 468 additions and 46 deletions.
77 changes: 74 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -500,7 +500,7 @@ 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)
```

## Containers
# Containers

When testing a kubernetes charm, you can mock container interactions. When using the null state (`State()`), there will
be no containers. So if the charm were to `self.unit.containers`, it would get back an empty dict.
Expand Down Expand Up @@ -586,7 +586,7 @@ need to associate the container with the event is that the Framework uses an env
pebble-ready event is about (it does not use the event name). Scenario needs that information, similarly, for injecting
that envvar into the charm's runtime.

### Container filesystem post-mortem
## Container filesystem post-mortem
If the charm writes files to a container (to a location you didn't Mount as a temporary folder you have access to), you will be able to inspect them using the `get_filesystem` api.

```python
Expand Down Expand Up @@ -623,7 +623,7 @@ def test_pebble_push():
assert cfg_file.read_text() == "TEST"
```

### `Container.exec` mocks
## `Container.exec` mocks

`container.exec` is a tad more complicated, but if you get to this low a level of simulation, you probably will have far
worse issues to deal with. You need to specify, for each possible command the charm might run on the container, what the
Expand Down Expand Up @@ -671,6 +671,77 @@ def test_pebble_exec():
)
```

# 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.

Using the same `get_filesystem` API as `Container`, you can access the tempdir used by Scenario to mock the filesystem root before and after the scenario runs.

```python
from scenario import Storage, Context, State
# some charm with a 'foo' filesystem-type storage defined in metadata.yaml
ctx = Context(MyCharm)
storage = Storage("foo")
# setup storage with some content
(storage.get_filesystem(ctx) / "myfile.txt").write_text("helloworld")

with ctx.manager("update-status", State(storage=[storage])) as mgr:
foo = mgr.charm.model.storages["foo"][0]
loc = foo.location
path = loc / "myfile.txt"
assert path.exists()
assert path.read_text() == "helloworld"

myfile = loc / "path.py"
myfile.write_text("helloworlds")

# post-mortem: inspect fs contents.
assert (
storage.get_filesystem(ctx) / "path.py"
).read_text() == "helloworlds"
```

Note that State only wants to know about **attached** storages. A storage which is not attached to the charm can simply be omitted from State and the charm will be none the wiser.

## Storage-add

If a charm requests adding more storage instances while handling some event, you can inspect that from the `Context.requested_storage` API.

```python
# in MyCharm._on_foo:
# the charm requests two new "foo" storage instances to be provisioned
self.model.storages.request("foo", 2)
```

From test code, you can inspect that:

```python
from scenario import Context, State

ctx = Context(MyCharm)
ctx.run('some-event-that-will-cause_on_foo-to-be-called', State())

# the charm has requested two 'foo' storages to be provisioned
assert ctx.requested_storages['foo'] == 2
```

Requesting storages has no other consequence in Scenario. In real life, this request will trigger Juju to provision the storage and execute the charm again with `foo-storage-attached`.
So a natural follow-up Scenario test suite for this case would be:

```python
from scenario import Context, State, Storage

ctx = Context(MyCharm)
foo_0 = Storage('foo')
# the charm is notified that one of the storages it has requested is ready
ctx.run(foo_0.attached_event, State(storage=[foo_0]))

foo_1 = Storage('foo')
# the charm is notified that the other storage is also ready
ctx.run(foo_1.attached_event, State(storage=[foo_0, foo_1]))
```


# Ports

Since `ops 2.6.0`, charms can invoke the `open-port`, `close-port`, and `opened-ports` hook tools to manage the ports opened on the host vm/container. Using the `State.opened_ports` api, you can:
Expand Down
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ build-backend = "setuptools.build_meta"
[project]
name = "ops-scenario"

version = "5.3.1"
version = "5.4"

authors = [
{ name = "Pietro Pasotti", email = "pietro.pasotti@canonical.com" }
Expand Down
2 changes: 2 additions & 0 deletions scenario/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
Secret,
State,
StateValidationError,
Storage,
StoredState,
SubordinateRelation,
deferred,
Expand All @@ -45,6 +46,7 @@
"BindAddress",
"Network",
"Port",
"Storage",
"StoredState",
"State",
"DeferredEvent",
Expand Down
59 changes: 59 additions & 0 deletions scenario/consistency_checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ def check_consistency(
check_config_consistency,
check_event_consistency,
check_secrets_consistency,
check_storages_consistency,
check_relation_consistency,
):
results = check(
Expand Down Expand Up @@ -123,6 +124,9 @@ def check_event_consistency(
if event._is_action_event:
_check_action_event(charm_spec, event, errors, warnings)

if event._is_storage_event:
_check_storage_event(charm_spec, event, errors, warnings)

return Results(errors, warnings)


Expand Down Expand Up @@ -190,6 +194,30 @@ def _check_action_event(
_check_action_param_types(charm_spec, action, errors, warnings)


def _check_storage_event(
charm_spec: _CharmSpec,
event: "Event",
errors: List[str],
warnings: List[str], # noqa: U100
):
storage = event.storage
if not storage:
errors.append(
"cannot construct a storage event without the Storage instance. "
"Please pass one.",
)
elif not event.name.startswith(normalize_name(storage.name)):
errors.append(
f"storage event should start with storage name. {event.name} does "
f"not start with {storage.name}.",
)
elif storage.name not in charm_spec.meta["storage"]:
errors.append(
f"storage event {event.name} refers to storage {storage.name} "
f"which is not declared in the charm metadata (metadata.yaml) under 'storage'.",
)


def _check_action_param_types(
charm_spec: _CharmSpec,
action: Action,
Expand Down Expand Up @@ -237,6 +265,37 @@ def _check_action_param_types(
)


def check_storages_consistency(
*,
state: "State",
charm_spec: "_CharmSpec",
**_kwargs, # noqa: U101
) -> Results:
"""Check the consistency of the state.storages with the charm_spec.metadata (metadata.yaml)."""
state_storage = state.storage
meta_storage = (charm_spec.meta or {}).get("storage", {})
errors = []

if missing := {s.name for s in state.storage}.difference(
set(meta_storage.keys()),
):
errors.append(
f"some storages passed to State were not defined in metadata.yaml: {missing}",
)

seen = []
for s in state_storage:
tag = (s.name, s.index)
if tag in seen:
errors.append(
f"duplicate storage in State: storage {s.name} with index {s.index} "
f"occurs multiple times in State.storage.",
)
seen.append(tag)

return Results(errors, [])


def check_config_consistency(
*,
state: "State",
Expand Down
9 changes: 9 additions & 0 deletions scenario/context.py
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,7 @@ def __init__(
self.unit_status_history: List["_EntityStatus"] = []
self.workload_version_history: List[str] = []
self.emitted_events: List[EventBase] = []
self.requested_storages: Dict[str, int] = {}

# set by Runtime.exec() in self._run()
self._output_state: Optional["State"] = None
Expand All @@ -263,13 +264,21 @@ def _get_container_root(self, container_name: str):
"""Get the path to a tempdir where this container's simulated root will live."""
return Path(self._tmp.name) / "containers" / container_name

def _get_storage_root(self, name: str, index: int) -> Path:
"""Get the path to a tempdir where this storage's simulated root will live."""
storage_root = Path(self._tmp.name) / "storages" / f"{name}-{index}"
# in the case of _get_container_root, _MockPebbleClient will ensure the dir exists.
storage_root.mkdir(parents=True, exist_ok=True)
return storage_root

def clear(self):
"""Cleanup side effects histories."""
self.juju_log = []
self.app_status_history = []
self.unit_status_history = []
self.workload_version_history = []
self.emitted_events = []
self.requested_storages = {}
self._action_logs = []
self._action_results = None
self._action_failure = ""
Expand Down
53 changes: 40 additions & 13 deletions scenario/mocking.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,11 @@
import shutil
from io import StringIO
from pathlib import Path
from typing import TYPE_CHECKING, Any, Dict, Optional, Set, Tuple, Union
from typing import TYPE_CHECKING, Any, Dict, List, Optional, Set, Tuple, Union

from ops import pebble
from ops.model import (
ModelError,
SecretInfo,
SecretRotate,
_format_action_result_dict,
Expand All @@ -19,7 +20,7 @@
from ops.testing import _TestingPebbleClient

from scenario.logger import logger as scenario_logger
from scenario.state import JujuLogLine, Mount, PeerRelation, Port
from scenario.state import JujuLogLine, Mount, PeerRelation, Port, Storage

if TYPE_CHECKING:
from scenario.context import Context
Expand Down Expand Up @@ -382,21 +383,47 @@ def action_get(self):
)
return action.params

# TODO:
def storage_add(self, *args, **kwargs): # noqa: U100
raise NotImplementedError("storage_add")
def storage_add(self, name: str, count: int = 1):
if "/" in name:
raise ModelError('storage name cannot contain "/"')

def resource_get(self, *args, **kwargs): # noqa: U100
raise NotImplementedError("resource_get")
self._context.requested_storages[name] = count

def storage_list(self, *args, **kwargs): # noqa: U100
raise NotImplementedError("storage_list")
def storage_list(self, name: str) -> List[int]:
return [
storage.index for storage in self._state.storage if storage.name == name
]

def storage_get(self, *args, **kwargs): # noqa: U100
raise NotImplementedError("storage_get")
def storage_get(self, storage_name_id: str, attribute: str) -> str:
if attribute != "location":
raise NotImplementedError(
f"storage-get not implemented for attribute={attribute}",
)

def planned_units(self, *args, **kwargs): # noqa: U100
raise NotImplementedError("planned_units")
name, index = storage_name_id.split("/")
index = int(index)
storages: List[Storage] = [
s for s in self._state.storage if s.name == name and s.index == index
]
if not storages:
raise RuntimeError(f"Storage with name={name} and index={index} not found.")
if len(storages) > 1:
# should not really happen: sanity check.
raise RuntimeError(
f"Multiple Storage instances with name={name} and index={index} found. "
f"Inconsistent state.",
)

storage = storages[0]
fs_path = storage.get_filesystem(self._context)
return str(fs_path)

def planned_units(self) -> int:
return self._state.planned_units

# TODO:
def resource_get(self, *args, **kwargs): # noqa: U100
raise NotImplementedError("resource_get")


class _MockPebbleClient(_TestingPebbleClient):
Expand Down
3 changes: 3 additions & 0 deletions scenario/runtime.py
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,9 @@ def _get_event_env(self, state: "State", event: "Event", charm_root: Path):
if container := event.container:
env.update({"JUJU_WORKLOAD_NAME": container.name})

if storage := event.storage:
env.update({"JUJU_STORAGE_ID": f"{storage.name}/{storage.index}"})

if secret := event.secret:
env.update(
{
Expand Down

0 comments on commit 29e545b

Please sign in to comment.