Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add collect-status events for multi-status handling #954

Merged
merged 21 commits into from
Jul 21, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
8f12f1b
Try approach to stateless multistatus that uses Framework events
benhoyt Jun 15, 2023
8773533
Got charm.collect_[app|unit]_status working
benhoyt Jun 28, 2023
88de08c
Merge branch 'main' into multistatus-poc
benhoyt Jul 10, 2023
c12c0d6
Docstrings and minor updates (just tests left)
benhoyt Jul 10, 2023
0e57f60
Oops, messed up merge conflicts
benhoyt Jul 10, 2023
e63d507
"Fix" linting
benhoyt Jul 11, 2023
5fa1522
Move _evaluate_status to charm.py; add tests
benhoyt Jul 11, 2023
47300c6
WIP fix main tests
benhoyt Jul 11, 2023
9fbafe3
Merge branch 'main' into multistatus-poc
benhoyt Jul 12, 2023
f1f38e0
Finish fixing main.py tests
benhoyt Jul 12, 2023
548eb51
Remove test_multistatus.py testing code
benhoyt Jul 12, 2023
6d58026
Fix main tests on Python 3.8 (no multi-expr with statement)
benhoyt Jul 12, 2023
c65f8a9
Merge branch 'main' into multistatus-poc
benhoyt Jul 17, 2023
3d5fcd4
Type check status arg in add_status for catching errors early
benhoyt Jul 17, 2023
d85d4cb
Bump up to latest Pyright version while we're at it
benhoyt Jul 17, 2023
3c3a4b6
Add testing Harness support for evaluating statuses
benhoyt Jul 17, 2023
694206d
Merge branch 'main' into multistatus-poc
benhoyt Jul 17, 2023
b920b3a
Make Maintenance status higher-pri than Waiting
benhoyt Jul 19, 2023
0967901
Log add_status calls (that aren't ActiveStatus)
benhoyt Jul 19, 2023
1c119ef
Flip maintenance and waiting again, as per Juju's status.DeriveStatus
benhoyt Jul 19, 2023
e389c2a
Make maintenance > waiting again, now that we've done it in Juju too
benhoyt Jul 21, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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'))
benhoyt marked this conversation as resolved.
Show resolved Hide resolved
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`).
"""
benhoyt marked this conversation as resolved.
Show resolved Hide resolved

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)
benhoyt marked this conversation as resolved.
Show resolved Hide resolved

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
benhoyt marked this conversation as resolved.
Show resolved Hide resolved
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
Loading