From d54a26a4f0c1253fd6f7c525191bbb97a7df0f02 Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Wed, 19 Feb 2025 12:00:33 +1300 Subject: [PATCH] feat: support starting and stopping Pebble checks, and the checks enabled field (#1560) Add support for: * The `startup` field in Pebble checks. * The `start_checks` and `stop_checks` Pebble API calls. Harness (and Scenario, mostly via re-using the Harness implementation) is adjusted to more closely simulate the Changes implementation of Pebble Checks, so that the 'if the change ID is the empty string, the check is inactive' behaviour can be simulated. A subtle bug with notices and check-infos is also fixed: previously the mocked Pebble would gather all notices and check-infos from all containers in the state, instead of only those that are in the correct container. [Pebble PR](https://github.com/canonical/pebble/pull/560) --- ops/_private/harness.py | 87 +++++++++++++++++++++++- ops/model.py | 24 +++++++ ops/pebble.py | 76 ++++++++++++++++++++- test/test_charm.py | 1 + test/test_model.py | 5 ++ test/test_pebble.py | 45 ++++++++++++- test/test_testing.py | 68 +++++++++++++++++++ testing/src/scenario/mocking.py | 77 ++++++++++++++++++++- testing/src/scenario/state.py | 34 +++++++++- testing/tests/test_e2e/test_pebble.py | 97 +++++++++++++++++++++++++++ testing/tests/test_e2e/test_state.py | 17 ++++- 11 files changed, 523 insertions(+), 8 deletions(-) diff --git a/ops/_private/harness.py b/ops/_private/harness.py index 3403aa9c9..6e18bacb8 100644 --- a/ops/_private/harness.py +++ b/ops/_private/harness.py @@ -3175,7 +3175,42 @@ def autostart_services(self, timeout: float = 30.0, delay: float = 0.1): if startup == pebble.ServiceStartup.ENABLED: self._service_status[name] = pebble.ServiceStatus.ACTIVE + def _new_perform_check(self, info: pebble.CheckInfo) -> pebble.Change: + now = datetime.datetime.now() + change = pebble.Change( + pebble.ChangeID(str(uuid.uuid4())), + pebble.ChangeKind.PERFORM_CHECK.value, + summary=info.name, + status=pebble.ChangeStatus.DOING.value, + tasks=[], + ready=False, + err=None, + spawn_time=now, + ready_time=None, + ) + info.change_id = change.id + info.status = pebble.CheckStatus.UP + info.failures = 0 + self._changes[change.id] = change + return change + def replan_services(self, timeout: float = 30.0, delay: float = 0.1): + for name, check in self._render_checks().items(): + if check.startup == pebble.CheckStartup.DISABLED: + continue + info = self._check_infos.get(name) + if info is None: + info = pebble.CheckInfo( + name=name, + level=check.level, + status=pebble.CheckStatus.UP, + failures=0, + threshold=3 if check.threshold is None else check.threshold, + startup=check.startup, + ) + self._check_infos[name] = info + if not info.change_id: + self._new_perform_check(info) return self.autostart_services(timeout, delay) def start_services( @@ -3346,12 +3381,35 @@ def add_layer( else: self._layers[label] = layer_obj + # Checks are started when the layer is added, not (only) on replan. + for name, check in layer_obj.checks.items(): + try: + info = self._check_infos[name] + except KeyError: + status = ( + pebble.CheckStatus.INACTIVE + if check.startup == pebble.CheckStartup.DISABLED + else pebble.CheckStatus.UP + ) + info = pebble.CheckInfo( + name, + level=check.level, + status=status, + failures=0, + change_id=pebble.ChangeID(''), + ) + self._check_infos[name] = info + info.level = check.level + info.threshold = 3 if check.threshold is None else check.threshold + info.startup = check.startup + if info.startup != pebble.CheckStartup.DISABLED and not info.change_id: + self._new_perform_check(info) + def _render_services(self) -> Dict[str, pebble.Service]: services: Dict[str, pebble.Service] = {} for key in sorted(self._layers.keys()): layer = self._layers[key] for name, service in layer.services.items(): - # TODO: merge existing services https://github.com/canonical/operator/issues/1112 services[name] = service return services @@ -3743,6 +3801,33 @@ def get_checks( if (level is None or level == info.level) and (names is None or info.name in names) ] + def start_checks(self, names: List[str]) -> List[str]: + self._check_connection() + started: List[str] = [] + for name in names: + if name not in self._check_infos: + raise self._api_error(404, f'cannot find check with name "{name}"') + info = self._check_infos[name] + if not info.change_id: + self._new_perform_check(info) + started.append(name) + return started + + def stop_checks(self, names: List[str]) -> List[str]: + self._check_connection() + stopped: List[str] = [] + for name in names: + if name not in self._check_infos: + raise self._api_error(404, f'cannot find check with name "{name}"') + info = self._check_infos[name] + if info.change_id: + change = self._changes[info.change_id] + change.status = pebble.ChangeStatus.ABORT.value + info.status = pebble.CheckStatus.INACTIVE + info.change_id = pebble.ChangeID('') + stopped.append(name) + return stopped + def notify( self, type: pebble.NoticeType, diff --git a/ops/model.py b/ops/model.py index 2a27ce681..9bd852d57 100644 --- a/ops/model.py +++ b/ops/model.py @@ -2503,6 +2503,30 @@ def get_check(self, check_name: str) -> pebble.CheckInfo: raise RuntimeError(f'expected 1 check, got {len(checks)}') return checks[check_name] + def start_checks(self, *check_names: str) -> List[str]: + """Start given check(s) by name. + + Returns: + A list of check names that were started. Checks that were already + running will not be included. + """ + if not check_names: + raise TypeError('start-checks expected at least 1 argument, got 0') + + return self._pebble.start_checks(check_names) + + def stop_checks(self, *check_names: str) -> List[str]: + """Stop given check(s) by name. + + Returns: + A list of check names that were stopped. Checks that were already + inactive will not be included. + """ + if not check_names: + raise TypeError('stop-checks expected at least 1 argument, got 0') + + return self._pebble.stop_checks(check_names) + @typing.overload def pull(self, path: Union[str, PurePath], *, encoding: None) -> BinaryIO: ... diff --git a/ops/pebble.py b/ops/pebble.py index f17e1cf94..958dfa13d 100644 --- a/ops/pebble.py +++ b/ops/pebble.py @@ -135,6 +135,7 @@ { 'override': str, 'level': Union['CheckLevel', str], + 'startup': Literal['', 'enabled', 'disabled'], 'period': Optional[str], 'timeout': Optional[str], 'http': Optional[HttpDict], @@ -243,6 +244,7 @@ def __enter__(self) -> typing.IO[typing.AnyStr]: ... { 'name': str, 'level': NotRequired[str], + 'startup': NotRequired[Literal['enabled', 'disabled']], 'status': str, 'failures': NotRequired[int], 'threshold': int, @@ -1103,6 +1105,7 @@ def __init__(self, name: str, raw: Optional[CheckDict] = None): except ValueError: level = dct.get('level', '') self.level = level + self.startup = CheckStartup(dct.get('startup', '')) self.period: Optional[str] = dct.get('period', '') self.timeout: Optional[str] = dct.get('timeout', '') self.threshold: Optional[int] = dct.get('threshold') @@ -1128,6 +1131,7 @@ def to_dict(self) -> CheckDict: fields = [ ('override', self.override), ('level', level), + ('startup', self.startup.value), ('period', self.period), ('timeout', self.timeout), ('threshold', self.threshold), @@ -1231,6 +1235,15 @@ class CheckStatus(enum.Enum): UP = 'up' DOWN = 'down' + INACTIVE = 'inactive' + + +class CheckStartup(enum.Enum): + """Enum of check startup options.""" + + UNSET = '' # Note that this is treated as ENABLED. + ENABLED = 'enabled' + DISABLED = 'disabled' class LogTarget: @@ -1407,12 +1420,21 @@ class CheckInfo: This can be :attr:`CheckLevel.ALIVE`, :attr:`CheckLevel.READY`, or None (level not set). """ + startup: CheckStartup + """Startup mode. + + :attr:`CheckStartup.ENABLED` means the check will be started when added, and + in a replan. :attr:`CheckStartup.DISABLED` means the check must be manually + started. + """ + status: Union[CheckStatus, str] """Status of the check. :attr:`CheckStatus.UP` means the check is healthy (the number of failures is less than the threshold), :attr:`CheckStatus.DOWN` means the check is - unhealthy (the number of failures has reached the threshold). + unhealthy (the number of failures has reached the threshold), and + :attr:`CheckStatus.INACTIVE` means the check is not running. """ failures: int @@ -1442,9 +1464,11 @@ def __init__( failures: int = 0, threshold: int = 0, change_id: Optional[ChangeID] = None, + startup: CheckStartup = CheckStartup.ENABLED, ): self.name = name self.level = level + self.startup = startup self.status = status self.failures = failures self.threshold = threshold @@ -1461,13 +1485,19 @@ def from_dict(cls, d: _CheckInfoDict) -> CheckInfo: status = CheckStatus(d['status']) except ValueError: status = d['status'] + change_id = ChangeID(d['change-id']) if 'change-id' in d else None + if not change_id and 'startup' in d: + # This is a version of Pebble that supports stopping checks, which + # means that the check is inactive if it has no change ID. + status = CheckStatus.INACTIVE return cls( name=d['name'], level=level, + startup=CheckStartup(d.get('startup', 'enabled')), status=status, failures=d.get('failures', 0), threshold=d['threshold'], - change_id=ChangeID(d['change-id']) if 'change-id' in d else None, + change_id=change_id, ) def __repr__(self): @@ -1475,6 +1505,7 @@ def __repr__(self): 'CheckInfo(' f'name={self.name!r}, ' f'level={self.level}, ' + f'startup={self.startup}, ' f'status={self.status}, ' f'failures={self.failures}, ' f'threshold={self.threshold!r}, ' @@ -2126,7 +2157,9 @@ def autostart_services(self, timeout: float = 30.0, delay: float = 0.1) -> Chang return self._services_action('autostart', [], timeout, delay) def replan_services(self, timeout: float = 30.0, delay: float = 0.1) -> ChangeID: - """Replan by (re)starting changed and startup-enabled services and wait for them to start. + """Replan by (re)starting changed and startup-enabled services and checks. + + After requesting the replan, also wait for any impacted services to start. Args: timeout: Seconds before replan change is considered timed out (float). If @@ -2335,6 +2368,19 @@ def _wait_change_using_polling( raise TimeoutError(f'timed out waiting for change {change_id} ({timeout} seconds)') + def _checks_action(self, action: str, checks: Iterable[str]) -> List[str]: + if isinstance(checks, str) or not hasattr(checks, '__iter__'): + raise TypeError(f'checks must be of type Iterable[str], not {type(checks).__name__}') + + checks = tuple(checks) + for chk in checks: + if not isinstance(chk, str): + raise TypeError(f'check names must be str, not {type(chk).__name__}') + + body = {'action': action, 'checks': checks} + resp = self._request('POST', '/v1/checks', body=body) + return resp['result']['changed'] + def add_layer(self, label: str, layer: Union[str, LayerDict, Layer], *, combine: bool = False): """Dynamically add a new layer onto the Pebble configuration layers. @@ -3052,6 +3098,30 @@ def get_checks( resp = self._request('GET', '/v1/checks', query) return [CheckInfo.from_dict(info) for info in resp['result']] + def start_checks(self, checks: Iterable[str]) -> List[str]: + """Start checks by name. + + Args: + checks: Non-empty list of checks to start. + + Returns: + Set of check names that were started. Checks that were already + running will not be included. + """ + return self._checks_action('start', checks) + + def stop_checks(self, checks: Iterable[str]) -> List[str]: + """Stop checks by name. + + Args: + checks: Non-empty list of checks to stop. + + Returns: + Set of check names that were stopped. Checks that were already + inactive will not be included. + """ + return self._checks_action('stop', checks) + def notify( self, type: NoticeType, diff --git a/test/test_charm.py b/test/test_charm.py index 8e709f5d2..934f5c32c 100644 --- a/test/test_charm.py +++ b/test/test_charm.py @@ -349,6 +349,7 @@ def mock_check_info( 'status': 'down', 'failures': 3, 'threshold': 3, + 'change-id': '1', }) ] diff --git a/test/test_model.py b/test/test_model.py index 3b0a2afbf..42f3282f5 100644 --- a/test/test_model.py +++ b/test/test_model.py @@ -1885,6 +1885,7 @@ def test_get_checks(self, container: ops.Container): 'status': 'up', 'failures': 0, 'threshold': 3, + 'change-id': '1', }), pebble.CheckInfo.from_dict({ 'name': 'c2', @@ -1892,6 +1893,7 @@ def test_get_checks(self, container: ops.Container): 'status': 'down', 'failures': 2, 'threshold': 2, + 'change-id': '2', }), ] @@ -1931,6 +1933,7 @@ def test_get_check(self, container: ops.Container): 'status': 'up', 'failures': 0, 'threshold': 3, + 'change-id': '1', }) ]) c = container.get_check('c1') @@ -1954,6 +1957,7 @@ def test_get_check(self, container: ops.Container): 'status': 'up', 'failures': 0, 'threshold': 3, + 'change-id': '1', }), pebble.CheckInfo.from_dict({ 'name': 'c2', @@ -1961,6 +1965,7 @@ def test_get_check(self, container: ops.Container): 'status': 'down', 'failures': 2, 'threshold': 2, + 'change-id': '2', }), ]) with pytest.raises(RuntimeError): diff --git a/test/test_pebble.py b/test/test_pebble.py index 602fe3134..e7cdc20e9 100644 --- a/test/test_pebble.py +++ b/test/test_pebble.py @@ -1273,9 +1273,11 @@ def test_check_status(self): assert list(pebble.CheckStatus) == [ pebble.CheckStatus.UP, pebble.CheckStatus.DOWN, + pebble.CheckStatus.INACTIVE, ] assert pebble.CheckStatus.UP.value == 'up' assert pebble.CheckStatus.DOWN.value == 'down' + assert pebble.CheckStatus.INACTIVE.value == 'inactive' def test_check_info(self): check = pebble.CheckInfo( @@ -1291,9 +1293,25 @@ def test_check_info(self): assert check.threshold == 3 assert check.change_id is None + check = pebble.CheckInfo( + name='chk1', + level=pebble.CheckLevel.READY, + startup=pebble.CheckStartup.ENABLED, + status=pebble.CheckStatus.INACTIVE, + threshold=3, + change_id=pebble.ChangeID(''), + ) + assert check.name == 'chk1' + assert check.level == pebble.CheckLevel.READY + assert check.status == pebble.CheckStatus.INACTIVE + assert check.failures == 0 + assert check.threshold == 3 + assert check.change_id == pebble.ChangeID('') + check = pebble.CheckInfo( name='chk2', level=pebble.CheckLevel.ALIVE, + startup=pebble.CheckStartup.DISABLED, status=pebble.CheckStatus.DOWN, failures=5, threshold=3, @@ -1310,6 +1328,7 @@ def test_check_info(self): 'name': 'chk3', 'status': 'up', 'threshold': 3, + 'change-id': '28', } check = pebble.CheckInfo.from_dict(d) assert check.name == 'chk3' @@ -1317,11 +1336,12 @@ def test_check_info(self): assert check.status == pebble.CheckStatus.UP assert check.failures == 0 assert check.threshold == 3 - assert check.change_id is None + assert check.change_id == pebble.ChangeID('28') check = pebble.CheckInfo.from_dict({ 'name': 'chk4', 'status': 'down', + 'startup': 'enabled', 'failures': 3, 'threshold': 3, 'change-id': '42', @@ -1333,6 +1353,21 @@ def test_check_info(self): assert check.threshold == 3 assert check.change_id == pebble.ChangeID('42') + check = pebble.CheckInfo.from_dict({ + 'name': 'chk5', + 'status': 'down', + 'startup': 'enabled', + 'failures': 3, + 'threshold': 3, + 'change-id': '', + }) + assert check.name == 'chk5' + assert check.level == pebble.CheckLevel.UNSET + assert check.status == pebble.CheckStatus.INACTIVE # Empty change-id means inactive. + assert check.failures == 3 + assert check.threshold == 3 + assert check.change_id == pebble.ChangeID('') + _bytes_generator = typing.Generator[bytes, typing.Any, typing.Any] @@ -2886,6 +2921,7 @@ def test_get_checks_all(self, client: MockClient): 'name': 'chk1', 'status': 'up', 'threshold': 2, + 'change-id': '1', }, { 'name': 'chk2', @@ -2893,6 +2929,7 @@ def test_get_checks_all(self, client: MockClient): 'status': 'down', 'failures': 5, 'threshold': 3, + 'change-id': '3', }, ], 'status': 'OK', @@ -2906,11 +2943,13 @@ def test_get_checks_all(self, client: MockClient): assert checks[0].status == pebble.CheckStatus.UP assert checks[0].failures == 0 assert checks[0].threshold == 2 + assert checks[0].change_id == pebble.ChangeID('1') assert checks[1].name == 'chk2' assert checks[1].level == pebble.CheckLevel.ALIVE assert checks[1].status == pebble.CheckStatus.DOWN assert checks[1].failures == 5 assert checks[1].threshold == 3 + assert checks[1].change_id == pebble.ChangeID('3') assert client.requests == [ ('GET', '/v1/checks', {}, None), @@ -2924,6 +2963,7 @@ def test_get_checks_filters(self, client: MockClient): 'level': 'ready', 'status': 'up', 'threshold': 3, + 'change-id': '1', }, ], 'status': 'OK', @@ -2937,6 +2977,7 @@ def test_get_checks_filters(self, client: MockClient): assert checks[0].status == pebble.CheckStatus.UP assert checks[0].failures == 0 assert checks[0].threshold == 3 + assert checks[0].change_id == pebble.ChangeID('1') assert client.requests == [ ('GET', '/v1/checks', {'level': 'ready', 'names': ['chk2']}, None), @@ -2950,6 +2991,7 @@ def test_checklevel_conversion(self, client: MockClient): 'level': 'foobar!', 'status': 'up', 'threshold': 3, + 'change-id': '8', }, ], 'status': 'OK', @@ -2963,6 +3005,7 @@ def test_checklevel_conversion(self, client: MockClient): assert checks[0].status == pebble.CheckStatus.UP assert checks[0].failures == 0 assert checks[0].threshold == 3 + assert checks[0].change_id == pebble.ChangeID('8') assert client.requests == [ ('GET', '/v1/checks', {'level': 'ready', 'names': ['chk2']}, None), diff --git a/test/test_testing.py b/test/test_testing.py index 1d636c7f8..4aa0f0b37 100644 --- a/test/test_testing.py +++ b/test/test_testing.py @@ -7027,6 +7027,74 @@ def test_get_cloud_spec_without_set_error(self, request: pytest.FixtureRequest): harness.model.get_cloud_spec() +class TestChecks: + @staticmethod + def _container_with_layer(request: pytest.FixtureRequest): + layer = pebble.Layer({ + 'checks': { + 'chk1': { + 'override': 'replace', + 'exec': {'command': 'foo'}, + }, + 'chk2': { + 'override': 'replace', + 'startup': 'enabled', + 'exec': {'command': 'foo'}, + }, + 'chk3': { + 'override': 'replace', + 'startup': 'disabled', + 'exec': {'command': 'foo'}, + }, + }, + }) + harness = ops.testing.Harness( + ops.CharmBase, + meta='name: mycharm\ncontainers:\n mycontainer:', + ) + request.addfinalizer(harness.cleanup) + harness.set_can_connect('mycontainer', True) + harness.begin() + container = harness.charm.unit.get_container('mycontainer') + container.add_layer('mylayer', layer) + return container + + def test_add_layer_with_checks(self, request: pytest.FixtureRequest): + container = self._container_with_layer(request) + chk1 = container.get_checks('chk1')['chk1'] + assert chk1.startup == pebble.CheckStartup.UNSET + assert chk1.failures == 0 + assert chk1.status == pebble.CheckStatus.UP + chk2 = container.get_checks('chk2')['chk2'] + assert chk2.startup == pebble.CheckStartup.ENABLED + assert chk2.failures == 0 + assert chk2.status == pebble.CheckStatus.UP + chk3 = container.get_checks('chk3')['chk3'] + assert chk3.startup == pebble.CheckStartup.DISABLED + assert chk3.failures == 0 + assert chk3.status == pebble.CheckStatus.INACTIVE + + def test_start_checks(self, request: pytest.FixtureRequest): + container = self._container_with_layer(request) + changed = container.start_checks('chk1', 'chk2', 'chk3') + assert changed == ['chk3'] + + def test_stop_checks(self, request: pytest.FixtureRequest): + container = self._container_with_layer(request) + changed = container.stop_checks('chk1', 'chk2', 'chk3') + assert changed == ['chk1', 'chk2'] + + def test_stop_then_start(self, request: pytest.FixtureRequest): + container = self._container_with_layer(request) + changed = container.stop_checks('chk1', 'chk2', 'chk3') + assert changed == ['chk1', 'chk2'] + changed = container.start_checks('chk1', 'chk2', 'chk3') + assert changed == ['chk1', 'chk2', 'chk3'] + for info in container.get_checks('chk1').values(): + assert info.status == pebble.CheckStatus.UP + assert info.change_id, 'Change ID should not be None or the empty string' + + @pytest.mark.skipif( not hasattr(ops.testing, 'Context'), reason='requires optional ops[testing] install' ) diff --git a/testing/src/scenario/mocking.py b/testing/src/scenario/mocking.py index 3e1bcd4f9..d60be490a 100644 --- a/testing/src/scenario/mocking.py +++ b/testing/src/scenario/mocking.py @@ -11,6 +11,7 @@ import datetime import io import shutil +import uuid from pathlib import Path from typing import ( TYPE_CHECKING, @@ -54,6 +55,7 @@ from .logger import logger as scenario_logger from .state import ( CharmType, + CheckInfo, JujuLogLine, Mount, Network, @@ -775,19 +777,92 @@ def __init__( # load any existing notices and check information from the state self._notices: Dict[Tuple[str, str], pebble.Notice] = {} self._check_infos: Dict[str, pebble.CheckInfo] = {} - for container in state.containers: + try: + container = state.get_container(self._container_name) + except KeyError: + # The container is in the metadata but not in the state - perhaps + # this is an install event, at which point the container doesn't + # exist yet. This means there will be no notices or check infos. + pass + else: for notice in container.notices: if hasattr(notice.type, "value"): notice_type = cast(pebble.NoticeType, notice.type).value else: notice_type = str(notice.type) self._notices[notice_type, notice.key] = notice._to_ops() + now = datetime.datetime.now() for check in container.check_infos: self._check_infos[check.name] = check._to_ops() + kind = ( + pebble.ChangeKind.PERFORM_CHECK.value + if check.status == pebble.CheckStatus.UP + else pebble.ChangeKind.RECOVER_CHECK.value + ) + change = pebble.Change( + pebble.ChangeID(str(uuid.uuid4())), + kind, + summary=check.name, + status=pebble.ChangeStatus.DOING.value, + tasks=[], + ready=False, + err=None, + spawn_time=now, + ready_time=now, + ) + self._changes[check.change_id] = change def get_plan(self) -> pebble.Plan: return self._container.plan + def _update_state_check_infos(self): + """Copy any new or changed check infos into the state.""" + infos: set[CheckInfo] = set() + for info in self._check_infos.values(): + if isinstance(info.level, str): + level = pebble.CheckLevel(info.level) + else: + level = info.level + if isinstance(info.status, str): + status = pebble.CheckStatus(info.status) + else: + status = info.status + check_info = CheckInfo( + name=info.name, + level=level, + startup=info.startup, + status=status, + failures=info.failures, + threshold=info.threshold, + change_id=info.change_id, + ) + infos.add(check_info) + object.__setattr__(self._container, "check_infos", frozenset(infos)) + + def replan_services(self, timeout: float = 30.0, delay: float = 0.1): + super().replan_services(timeout=timeout, delay=delay) + self._update_state_check_infos() + + def add_layer( + self, + label: str, + layer: Union[str, "pebble.LayerDict", pebble.Layer], + *, + combine: bool = False, + ): + super().add_layer(label, layer, combine=combine) + self._update_state_check_infos() + + def start_checks(self, names: List[str]) -> List[str]: + started = super().start_checks(names) + self._update_state_check_infos() + return started + + def stop_checks(self, names: List[str]) -> List[str]: + stopped = super().stop_checks(names) + self._update_state_check_infos() + return stopped + @property def _container(self) -> "ContainerSpec": container_name = self.socket_path.split("/")[-2] diff --git a/testing/src/scenario/state.py b/testing/src/scenario/state.py index da22e3fe9..6e8ff042a 100644 --- a/testing/src/scenario/state.py +++ b/testing/src/scenario/state.py @@ -865,13 +865,17 @@ class CheckInfo(_max_posargs(1)): level: pebble.CheckLevel | None = None """Level of the check.""" + startup: pebble.CheckStartup = pebble.CheckStartup.ENABLED + """Startup mode of the check.""" + status: pebble.CheckStatus = pebble.CheckStatus.UP """Status of the check. :attr:`ops.pebble.CheckStatus.UP` means the check is healthy (the number of failures is fewer than the threshold), :attr:`ops.pebble.CheckStatus.DOWN` means the check is unhealthy (the number of failures has reached the - threshold). + threshold), and :attr:`ops.pebble.CheckStatus.INACTIVE` means the check has + been stopped, so is not currently running. """ failures: int = 0 @@ -883,13 +887,33 @@ class CheckInfo(_max_posargs(1)): This is how many consecutive failures for the check to be considered 'down'. """ + change_id: pebble.ChangeID | None = None + """The ID of the Pebble Change associated with this check. + + Passing ``None`` will automatically assign a new Change ID if the status is + :attr:`ops.pebble.CheckStatus.UP` or :attr:`ops.pebble.CheckStatus.DOWN`. + """ + + def __post_init__(self): + if self.change_id is None: + if self.status == pebble.CheckStatus.INACTIVE: + change_id = "" + else: + change_id = pebble.ChangeID(_generate_new_change_id()) + object.__setattr__(self, "change_id", change_id) + + def __hash__(self) -> int: + return hash(self.name) + def _to_ops(self) -> pebble.CheckInfo: return pebble.CheckInfo( name=self.name, level=self.level, + startup=self.startup, status=self.status, failures=self.failures, threshold=self.threshold, + change_id=self.change_id, ) @@ -1010,6 +1034,7 @@ def plan(self) -> pebble.Plan: return plan for name in sorted(services.keys()): plan.services[name] = services[name] + # TODO: This should presumably also have checks and log targets. return plan @property @@ -1047,6 +1072,13 @@ def get_filesystem(self, ctx: Context) -> pathlib.Path: """ return ctx._get_container_root(self.name) + def get_check_info(self, name: str) -> CheckInfo: + """Get the check info for a check by name.""" + for check_info in self.check_infos: + if check_info.name == name: + return check_info + raise KeyError(f"check-info: {name} not found in the Container") + _RawStatusLiteral = Literal[ "waiting", diff --git a/testing/tests/test_e2e/test_pebble.py b/testing/tests/test_e2e/test_pebble.py index 884fa1366..32df78c70 100644 --- a/testing/tests/test_e2e/test_pebble.py +++ b/testing/tests/test_e2e/test_pebble.py @@ -546,3 +546,100 @@ def _on_bar_check_failed(self, event): assert foo_infos[0].status == pebble.CheckStatus.DOWN assert foo_infos[0].failures == 7 assert len(bar_infos) == 0 + + +def test_pebble_add_layer(): + class MyCharm(CharmBase): + def __init__(self, framework: Framework): + super().__init__(framework) + framework.observe(self.on.foo_pebble_ready, self._on_foo_ready) + + def _on_foo_ready(self, _): + self.unit.get_container("foo").add_layer( + "foo", + {"checks": {"chk1": {"override": "replace"}}}, + ) + + ctx = Context(MyCharm, meta={"name": "foo", "containers": {"foo": {}}}) + container = Container("foo", can_connect=True) + state_out = ctx.run( + ctx.on.pebble_ready(container), state=State(containers={container}) + ) + chk1_info = state_out.get_container("foo").get_check_info("chk1") + assert chk1_info.status == pebble.CheckStatus.UP + + +def test_pebble_start_check(): + class MyCharm(CharmBase): + def __init__(self, framework: Framework): + super().__init__(framework) + framework.observe(self.on.foo_pebble_ready, self._on_foo_ready) + framework.observe(self.on.config_changed, self._on_config_changed) + + def _on_foo_ready(self, _): + container = self.unit.get_container("foo") + container.add_layer( + "foo", + {"checks": {"chk1": {"override": "replace", "startup": "disabled"}}}, + ) + + def _on_config_changed(self, _): + container = self.unit.get_container("foo") + container.start_checks("chk1") + + ctx = Context(MyCharm, meta={"name": "foo", "containers": {"foo": {}}}) + container = Container("foo", can_connect=True) + + # Ensure that it starts as inactive. + state_out = ctx.run( + ctx.on.pebble_ready(container), state=State(containers={container}) + ) + chk1_info = state_out.get_container("foo").get_check_info("chk1") + assert chk1_info.status == pebble.CheckStatus.INACTIVE + + # Verify that start_checks works. + state_out = ctx.run(ctx.on.config_changed(), state=state_out) + chk1_info = state_out.get_container("foo").get_check_info("chk1") + assert chk1_info.status == pebble.CheckStatus.UP + + +def test_pebble_stop_check(): + class MyCharm(CharmBase): + def __init__(self, framework: Framework): + super().__init__(framework) + framework.observe(self.on.config_changed, self._on_config_changed) + + def _on_config_changed(self, _): + container = self.unit.get_container("foo") + container.stop_checks("chk1") + + ctx = Context(MyCharm, meta={"name": "foo", "containers": {"foo": {}}}) + info_in = CheckInfo("chk1", status=pebble.CheckStatus.UP) + container = Container("foo", can_connect=True, check_infos=frozenset({info_in})) + state_out = ctx.run(ctx.on.config_changed(), state=State(containers={container})) + info_out = state_out.get_container("foo").get_check_info("chk1") + assert info_out.status == pebble.CheckStatus.INACTIVE + + +def test_pebble_replan_checks(): + class MyCharm(CharmBase): + def __init__(self, framework: Framework): + super().__init__(framework) + framework.observe(self.on.config_changed, self._on_config_changed) + + def _on_config_changed(self, _): + container = self.unit.get_container("foo") + container.replan() + + ctx = Context(MyCharm, meta={"name": "foo", "containers": {"foo": {}}}) + info_in = CheckInfo("chk1", status=pebble.CheckStatus.INACTIVE) + layer = pebble.Layer({"checks": {"chk1": {"override": "replace"}}}) + container = Container( + "foo", + can_connect=True, + check_infos=frozenset({info_in}), + layers={"layer1": layer}, + ) + state_out = ctx.run(ctx.on.config_changed(), state=State(containers={container})) + info_out = state_out.get_container("foo").get_check_info("chk1") + assert info_out.status == pebble.CheckStatus.UP diff --git a/testing/tests/test_e2e/test_state.py b/testing/tests/test_e2e/test_state.py index e2ed6f4de..3f3f2e38e 100644 --- a/testing/tests/test_e2e/test_state.py +++ b/testing/tests/test_e2e/test_state.py @@ -1,7 +1,8 @@ import copy from dataclasses import asdict, replace -from typing import Type +from typing import Optional, Type +import ops import pytest from ops.charm import CharmBase, CharmEvents, CollectStatusEvent from ops.framework import EventBase, Framework @@ -11,6 +12,7 @@ _DEFAULT_JUJU_DATABAG, Address, BindAddress, + CheckInfo, Container, Model, Network, @@ -251,6 +253,19 @@ def pre_event(charm: CharmBase): } +def test_checkinfo_changeid_none(): + info = CheckInfo("foo", change_id=None) + assert info.change_id, "None should result in a random change_id" + info2 = CheckInfo("foo") # None is also the default. + assert info.change_id != info2.change_id + + +@pytest.mark.parametrize("id", ("", "28")) +def test_checkinfo_changeid(id: Optional[str]): + info = CheckInfo("foo", change_id=ops.pebble.ChangeID(id)) + assert info.change_id == ops.pebble.ChangeID(id) + + @pytest.mark.parametrize( "klass,num_args", [