Skip to content

Commit

Permalink
Add collect-status events for multi-status handling (#954)
Browse files Browse the repository at this point in the history
This implements the `collect_app_status` and `collect_unit_status`
events that can be used by charms to allow the framework to
automatically collect status (from multiple components of the charm)
and set status at the end of every hook.

API reference under [`CollectStatusEvent`]
(https://ops--954.org.readthedocs.build/en/954/#ops.CollectStatusEvent),
though I'll also add usage examples to the new "charm status" SDK doc
that Dora is creating.

Spec and examples described in [OP037]
(https://docs.google.com/document/d/1uQNgif0GG03TdnqT4UM9BxEshXuSvCmT9TdxOHoIUkI/edit).

The implementation is very straight-forward: `model.Application` and
`model.Unit` each have a list of `_collected_statuses`, and
`CollectStatusEvent.add_status` adds to the correct one of those.
Then after every hook in `ops/main.py` we call `_evaluate_status`,
which triggers each event and sets the app or unit status to the
highest-priority status collected (if any statuses were collected).
Note that `collect_app_status` is only done on the leader unit.

To test when using `ops.testing.Harness`, use
`Harness.evaluate_status`.

This has full unit tests, including in `test_main.py`, but I've also
confirms this works on a real production deploying on Juju.
  • Loading branch information
benhoyt committed Jul 21, 2023
1 parent dd4865f commit a3b110e
Show file tree
Hide file tree
Showing 9 changed files with 400 additions and 23 deletions.
2 changes: 2 additions & 0 deletions ops/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@
'CharmEvents',
'CharmMeta',
'CollectMetricsEvent',
'CollectStatusEvent',
'ConfigChangedEvent',
'ContainerMeta',
'ContainerStorageMeta',
Expand Down Expand Up @@ -186,6 +187,7 @@
CharmEvents,
CharmMeta,
CollectMetricsEvent,
CollectStatusEvent,
ConfigChangedEvent,
ContainerMeta,
ContainerStorageMeta,
Expand Down
123 changes: 118 additions & 5 deletions ops/charm.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
"""Base objects for the Charm, events and metadata."""

import enum
import logging
import os
import pathlib
from typing import (
Expand Down Expand Up @@ -74,6 +75,9 @@
total=False)


logger = logging.getLogger(__name__)


class HookEvent(EventBase):
"""Events raised by Juju to progress a charm's lifecycle.
Expand Down Expand Up @@ -826,6 +830,83 @@ def defer(self) -> None:
'this event until you create a new revision.')


class CollectStatusEvent(EventBase):
"""Event triggered at the end of every hook to collect statuses for evaluation.
If the charm wants to provide application or unit status in a consistent
way after the end of every hook, it should observe the
:attr:`collect_app_status <CharmEvents.collect_app_status>` or
:attr:`collect_unit_status <CharmEvents.collect_unit_status>` event,
respectively.
The framework will trigger these events after the hook code runs
successfully (``collect_app_status`` will only be triggered on the leader
unit). If any statuses were added by the event handlers using
:meth:`add_status`, the framework will choose the highest-priority status
and set that as the status (application status for ``collect_app_status``,
or unit status for ``collect_unit_status``).
The order of priorities is as follows, from highest to lowest:
* error
* blocked
* maintenance
* waiting
* active
* unknown
If there are multiple statuses with the same priority, the first one added
wins (and if an event is observed multiple times, the handlers are called
in the order they were observed).
A collect-status event can be observed multiple times, and
:meth:`add_status` can be called multiple times to add multiple statuses
for evaluation. This is useful when a charm has multiple components that
each have a status. Each code path in a collect-status handler should
call ``add_status`` at least once.
Below is an example "web app" charm component that observes
``collect_unit_status`` to provide the status of the component, which
requires a "port" config option set before it can proceed::
class MyCharm(ops.CharmBase):
def __init__(self, *args):
super().__init__(*args)
self.webapp = Webapp(self)
# initialize other components
class WebApp(ops.Object):
def __init__(self, charm: ops.CharmBase):
super().__init__(charm, 'webapp')
self.framework.observe(charm.on.collect_unit_status, self._on_collect_status)
def _on_collect_status(self, event: ops.CollectStatusEvent):
if 'port' not in self.model.config:
event.add_status(ops.BlockedStatus('please set "port" config'))
return
event.add_status(ops.ActiveStatus())
.. # noqa (pydocstyle barfs on the above for unknown reasons I've spent hours on)
"""

def add_status(self, status: model.StatusBase):
"""Add a status for evaluation.
See :class:`CollectStatusEvent` for a description of how to use this.
"""
if not isinstance(status, model.StatusBase):
raise TypeError(f'status should be a StatusBase, not {type(status).__name__}')
model_ = self.framework.model
if self.handle.kind == 'collect_app_status':
if not isinstance(status, model.ActiveStatus):
logger.debug('Adding app status %s', status, stacklevel=2)
model_.app._collected_statuses.append(status)
else:
if not isinstance(status, model.ActiveStatus):
logger.debug('Adding unit status %s', status, stacklevel=2)
model_.unit._collected_statuses.append(status)


class CharmEvents(ObjectEvents):
"""Events generated by Juju pertaining to application lifecycle.
Expand Down Expand Up @@ -882,26 +963,41 @@ class CharmEvents(ObjectEvents):

leader_settings_changed = EventSource(LeaderSettingsChangedEvent)
"""DEPRECATED. Triggered when leader changes any settings (see
:class:`LeaderSettingsChangedEvent`)."""
:class:`LeaderSettingsChangedEvent`).
"""

collect_metrics = EventSource(CollectMetricsEvent)
"""Triggered by Juju to collect metrics (see :class:`CollectMetricsEvent`)."""

secret_changed = EventSource(SecretChangedEvent)
"""Triggered by Juju on the observer when the secret owner changes its contents (see
:class:`SecretChangedEvent`)."""
:class:`SecretChangedEvent`).
"""

secret_expired = EventSource(SecretExpiredEvent)
"""Triggered by Juju on the owner when a secret's expiration time elapses (see
:class:`SecretExpiredEvent`)."""
:class:`SecretExpiredEvent`).
"""

secret_rotate = EventSource(SecretRotateEvent)
"""Triggered by Juju on the owner when the secret's rotation policy elapses (see
:class:`SecretRotateEvent`)."""
:class:`SecretRotateEvent`).
"""

secret_remove = EventSource(SecretRemoveEvent)
"""Triggered by Juju on the owner when a secret revision can be removed (see
:class:`SecretRemoveEvent`)."""
:class:`SecretRemoveEvent`).
"""

collect_app_status = EventSource(CollectStatusEvent)
"""Triggered on the leader at the end of every hook to collect app statuses for evaluation
(see :class:`CollectStatusEvent`).
"""

collect_unit_status = EventSource(CollectStatusEvent)
"""Triggered at the end of every hook to collect unit statuses for evaluation
(see :class:`CollectStatusEvent`).
"""


class CharmBase(Object):
Expand Down Expand Up @@ -995,6 +1091,23 @@ def config(self) -> model.ConfigData:
return self.model.config


def _evaluate_status(charm: CharmBase): # pyright: ignore[reportUnusedFunction]
"""Trigger collect-status events and evaluate and set the highest-priority status.
See :class:`CollectStatusEvent` for details.
"""
if charm.framework.model._backend.is_leader():
charm.on.collect_app_status.emit()
app = charm.app
if app._collected_statuses:
app.status = model.StatusBase._get_highest_priority(app._collected_statuses)

charm.on.collect_unit_status.emit()
unit = charm.unit
if unit._collected_statuses:
unit.status = model.StatusBase._get_highest_priority(unit._collected_statuses)


class CharmMeta:
"""Object containing the metadata for the charm.
Expand Down
2 changes: 2 additions & 0 deletions ops/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -440,6 +440,8 @@ def main(charm_class: Type[ops.charm.CharmBase],

_emit_charm_event(charm, dispatcher.event_name)

ops.charm._evaluate_status(charm)

framework.commit()
finally:
framework.close()
Expand Down
27 changes: 27 additions & 0 deletions ops/model.py
Original file line number Diff line number Diff line change
Expand Up @@ -319,6 +319,7 @@ def __init__(self, name: str, meta: 'ops.charm.CharmMeta',
self._cache = cache
self._is_our_app = self.name == self._backend.app_name
self._status = None
self._collected_statuses: 'List[StatusBase]' = []

def _invalidate(self):
self._status = None
Expand All @@ -331,6 +332,10 @@ def status(self) -> 'StatusBase':
The status of remote units is always Unknown.
You can also use the :attr:`collect_app_status <CharmEvents.collect_app_status>`
event if you want to evaluate and set application status consistently
at the end of every hook.
Raises:
RuntimeError: if you try to set the status of another application, or if you try to
set the status of this application as a unit that is not the leader.
Expand Down Expand Up @@ -465,6 +470,7 @@ def __init__(self, name: str, meta: 'ops.charm.CharmMeta',
self._cache = cache
self._is_our_unit = self.name == self._backend.unit_name
self._status = None
self._collected_statuses: 'List[StatusBase]' = []

if self._is_our_unit and hasattr(meta, "containers"):
containers: _ContainerMeta_Raw = meta.containers
Expand All @@ -479,6 +485,10 @@ def status(self) -> 'StatusBase':
The status of any unit other than yourself is always Unknown.
You can also use the :attr:`collect_unit_status <CharmEvents.collect_unit_status>`
event if you want to evaluate and set unit status consistently at the
end of every hook.
Raises:
RuntimeError: if you try to set the status of a unit other than yourself.
InvalidStatusError: if you try to set the status to something other than
Expand Down Expand Up @@ -1583,6 +1593,23 @@ def register(cls, child: Type['StatusBase']):
cls._statuses[child.name] = child
return child

_priorities = {
'error': 5,
'blocked': 4,
'maintenance': 3,
'waiting': 2,
'active': 1,
# 'unknown' or any other status is handled below
}

@classmethod
def _get_highest_priority(cls, statuses: 'List[StatusBase]') -> 'StatusBase':
"""Return the highest-priority status from a list of statuses.
If there are multiple highest-priority statuses, return the first one.
"""
return max(statuses, key=lambda status: cls._priorities.get(status.name, 0))


@StatusBase.register
class UnknownStatus(StatusBase):
Expand Down
15 changes: 15 additions & 0 deletions ops/testing.py
Original file line number Diff line number Diff line change
Expand Up @@ -1484,6 +1484,21 @@ def __init__(self, *args):
container_name = container.name
return self._backend._pebble_clients[container_name]._root

def evaluate_status(self) -> None:
"""Trigger the collect-status events and set application and/or unit status.
This will always trigger ``collect_unit_status``, and set the unit status if any
statuses were added.
If running on the leader unit (:meth:`set_leader` has been called with ``True``),
this will trigger ``collect_app_status``, and set the application status if any
statuses were added.
Tests should normally call this and then assert that ``self.model.app.status``
or ``self.model.unit.status`` is the value expected.
"""
charm._evaluate_status(self.charm)


def _get_app_or_unit_name(app_or_unit: AppUnitOrName) -> str:
"""Return name of given application or unit (return strings directly)."""
Expand Down
2 changes: 1 addition & 1 deletion requirements-dev.txt
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ flake8-builtins==2.1.0
pyproject-flake8==4.0.1
pep8-naming==0.13.2
pytest==7.2.1
pyright==1.1.316
pyright==1.1.317
pytest-operator==0.23.0
coverage[toml]==7.0.5
typing_extensions==4.2.0
Expand Down
Loading

0 comments on commit a3b110e

Please sign in to comment.