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

feat: support change-update notice and pebble-change-updated event #1170

Merged
merged 5 commits into from
Apr 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
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 @@ -66,6 +66,7 @@
'LeaderSettingsChangedEvent',
'MetadataLinks',
'PayloadMeta',
'PebbleChangeUpdatedEvent',
'PebbleCustomNoticeEvent',
'PebbleNoticeEvent',
'PebbleReadyEvent',
Expand Down Expand Up @@ -208,6 +209,7 @@
LeaderSettingsChangedEvent,
MetadataLinks,
PayloadMeta,
PebbleChangeUpdatedEvent,
PebbleCustomNoticeEvent,
PebbleNoticeEvent,
PebbleReadyEvent,
Expand Down
14 changes: 13 additions & 1 deletion ops/charm.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
cast,
)

from ops import model
from ops import model, pebble
from ops._private import yaml
from ops.framework import (
EventBase,
Expand Down Expand Up @@ -808,6 +808,15 @@ class PebbleCustomNoticeEvent(PebbleNoticeEvent):
"""Event triggered when a Pebble notice of type "custom" is created or repeats."""


class PebbleChangeUpdatedEvent(PebbleNoticeEvent):
benhoyt marked this conversation as resolved.
Show resolved Hide resolved
"""Event triggered when a Pebble notice of type "change-update" is created or repeats."""

def get_change(self) -> pebble.Change:
"""Get the Pebble change associated with this event."""
change_id = pebble.ChangeID(self.notice.key)
return self.workload.pebble.get_change(change_id)


class SecretEvent(HookEvent):
"""Base class for all secret events."""

Expand Down Expand Up @@ -1187,6 +1196,9 @@ def __init__(self, framework: Framework):
container_name = container_name.replace('-', '_')
self.on.define_event(f"{container_name}_pebble_ready", PebbleReadyEvent)
self.on.define_event(f"{container_name}_pebble_custom_notice", PebbleCustomNoticeEvent)
self.on.define_event(
f"{container_name}_pebble_change_updated",
PebbleChangeUpdatedEvent)

@property
def app(self) -> model.Application:
Expand Down
10 changes: 10 additions & 0 deletions ops/pebble.py
Original file line number Diff line number Diff line change
Expand Up @@ -1314,7 +1314,17 @@ def __repr__(self):
class NoticeType(enum.Enum):
"""Enum of notice types."""

CHANGE_UPDATE = 'change-update'
"""Recorded whenever a change is updated, that is, when it is first
spawned or its status was updated. The key for change-update notices is
the change ID.
"""

CUSTOM = 'custom'
"""A custom notice reported via the Pebble client API or ``pebble notify``.
The key and data fields are provided by the user. The key must be in
the format ``example.com/path`` to ensure well-namespaced notice keys.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we validate this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, we validate it (in the Pebble server), so the workload will get an error if it tries to add a custom notice (pebble notify) with an invalid key. The API does "reasonable effort" validation with a regex.

"""


class NoticesUsers(enum.Enum):
Expand Down
13 changes: 7 additions & 6 deletions ops/testing.py
Original file line number Diff line number Diff line change
Expand Up @@ -1166,8 +1166,13 @@ def pebble_notify(self, container_name: str, key: str, *,

id, new_or_repeated = client._notify(type, key, data=data, repeat_after=repeat_after)

if self._charm is not None and type == pebble.NoticeType.CUSTOM and new_or_repeated:
self.charm.on[container_name].pebble_custom_notice.emit(container, id, type.value, key)
if self._charm is not None and new_or_repeated:
if type == pebble.NoticeType.CUSTOM:
self.charm.on[container_name].pebble_custom_notice.emit(
container, id, type.value, key)
elif type == pebble.NoticeType.CHANGE_UPDATE:
self.charm.on[container_name].pebble_change_updated.emit(
container, id, type.value, key)

return id

Expand Down Expand Up @@ -3362,10 +3367,6 @@ def _notify(self, type: pebble.NoticeType, key: str, *,

Return a tuple of (notice_id, new_or_repeated).
"""
if type != pebble.NoticeType.CUSTOM:
message = f'invalid type "{type.value}" (can only add "custom" notices)'
raise self._api_error(400, message)

# The shape of the code below is taken from State.AddNotice in Pebble.
now = datetime.datetime.now(tz=datetime.timezone.utc)

Expand Down
10 changes: 10 additions & 0 deletions test/charms/test_main/src/charm.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ def __init__(self, *args: typing.Any):
on_collect_metrics=[],
on_test_pebble_ready=[],
on_test_pebble_custom_notice=[],
on_test_pebble_change_updated=[],

on_log_critical_action=[],
on_log_error_action=[],
Expand Down Expand Up @@ -92,6 +93,8 @@ def __init__(self, *args: typing.Any):
self.framework.observe(self.on.test_pebble_ready, self._on_test_pebble_ready)
self.framework.observe(self.on.test_pebble_custom_notice,
self._on_test_pebble_custom_notice)
self.framework.observe(self.on.test_pebble_change_updated,
self._on_test_pebble_change_updated)

self.framework.observe(self.on.secret_remove, self._on_secret_remove)
self.framework.observe(self.on.secret_rotate, self._on_secret_rotate)
Expand Down Expand Up @@ -197,6 +200,13 @@ def _on_test_pebble_custom_notice(self, event: ops.PebbleCustomNoticeEvent):
self._stored.observed_event_types.append(type(event).__name__)
self._stored.test_pebble_custom_notice_data = event.snapshot()

def _on_test_pebble_change_updated(self, event: ops.PebbleChangeUpdatedEvent):
assert event.workload is not None
assert isinstance(event.notice, ops.LazyNotice)
self._stored.on_test_pebble_change_updated.append(type(event).__name__)
self._stored.observed_event_types.append(type(event).__name__)
self._stored.test_pebble_change_updated_data = event.snapshot()

def _on_start_action(self, event: ops.ActionEvent):
assert event.handle.kind == 'start_action', (
'event action name cannot be different from the one being handled')
Expand Down
33 changes: 32 additions & 1 deletion test/test_charm.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,14 @@
import tempfile
import typing
import unittest
import unittest.mock
from pathlib import Path

import yaml

import ops
import ops.charm
from ops import pebble
from ops.model import ModelError, _ModelBackend
from ops.storage import SQLiteStorage

Expand Down Expand Up @@ -354,7 +356,19 @@ def _on_stor4_attach(self, event: ops.StorageAttachedEvent):
'StorageAttachedEvent',
])

def test_workload_events(self):
@unittest.mock.patch('ops.pebble.Client.get_change')
def test_workload_events(self, mock_get_change: unittest.mock.MagicMock):
def get_change(change_id: pebble.ChangeID) -> pebble.Change:
return pebble.Change.from_dict({
'id': pebble.ChangeID(change_id),
'kind': 'exec',
'ready': False,
'spawn-time': '2021-01-28T14:37:02.247202105+13:00',
'status': 'Doing',
'summary': 'Exec command "foo"',
})

mock_get_change.side_effect = get_change

class MyCharm(ops.CharmBase):
def __init__(self, *args: typing.Any):
Expand All @@ -369,13 +383,23 @@ def __init__(self, *args: typing.Any):
self.on[workload].pebble_custom_notice,
self.on_any_pebble_custom_notice,
)
self.framework.observe(
self.on[workload].pebble_change_updated,
self.on_any_pebble_change_updated,
)

def on_any_pebble_ready(self, event: ops.PebbleReadyEvent):
self.seen.append(type(event).__name__)

def on_any_pebble_custom_notice(self, event: ops.PebbleCustomNoticeEvent):
self.seen.append(type(event).__name__)

def on_any_pebble_change_updated(self, event: ops.PebbleChangeUpdatedEvent):
self.seen.append(type(event).__name__)
change = event.get_change()
assert change.id == event.notice.key
assert change.kind == 'exec'

# language=YAML
self.meta = ops.CharmMeta.from_yaml(metadata='''
name: my-charm
Expand All @@ -399,11 +423,18 @@ def on_any_pebble_custom_notice(self, event: ops.PebbleCustomNoticeEvent):
charm.on['containerb'].pebble_custom_notice.emit(
charm.framework.model.unit.get_container('containerb'), '2', 'custom', 'y')

charm.on['container-a'].pebble_change_updated.emit(
charm.framework.model.unit.get_container('container-a'), '1', 'change-update', '42')
charm.on['containerb'].pebble_change_updated.emit(
charm.framework.model.unit.get_container('containerb'), '2', 'change-update', '42')

self.assertEqual(charm.seen, [
'PebbleReadyEvent',
'PebbleReadyEvent',
'PebbleCustomNoticeEvent',
'PebbleCustomNoticeEvent',
'PebbleChangeUpdatedEvent',
'PebbleChangeUpdatedEvent',
])

def test_relations_meta(self):
Expand Down
10 changes: 10 additions & 0 deletions test/test_main.py
Original file line number Diff line number Diff line change
Expand Up @@ -608,6 +608,16 @@ def test_multiple_events_handled(self):
'notice_id': '123',
'notice_type': 'custom',
'notice_key': 'example.com/a'},
), (
EventSpec(ops.PebbleChangeUpdatedEvent, 'test_pebble_change_updated',
workload_name='test',
notice_id='456',
notice_type='change-update',
notice_key='42'),
{'container_name': 'test',
'notice_id': '456',
'notice_type': 'change-update',
'notice_key': '42'},
), (
EventSpec(ops.SecretChangedEvent, 'secret_changed',
secret_id='secret:12345',
Expand Down
45 changes: 44 additions & 1 deletion test/test_testing.py
Original file line number Diff line number Diff line change
Expand Up @@ -1107,7 +1107,7 @@ def test_config_secret_option(self):
secret_id = harness.add_model_secret('mycharm', {'key': 'value'})
harness.update_config(key_values={'a': secret_id})
self.assertEqual(harness.charm.changes,
[{'name': 'config-changed', 'data': {'a': secret_id}}])
[{'name': 'config-changed', 'data': {'a': secret_id}}])

def test_no_config_option_type(self):
with self.assertRaises(RuntimeError):
Expand Down Expand Up @@ -3223,6 +3223,8 @@ def observe_container_events(self, container_name: str):
self.framework.observe(self.on[container_name].pebble_ready, self._on_pebble_ready)
self.framework.observe(self.on[container_name].pebble_custom_notice,
self._on_pebble_custom_notice)
self.framework.observe(self.on[container_name].pebble_change_updated,
self._on_pebble_change_updated)

def _on_pebble_ready(self, event: ops.PebbleReadyEvent):
self.changes.append({
Expand All @@ -3241,6 +3243,17 @@ def _on_pebble_custom_notice(self, event: ops.PebbleCustomNoticeEvent):
'notice_key': event.notice.key,
})

def _on_pebble_change_updated(self, event: ops.PebbleChangeUpdatedEvent):
type_str = (event.notice.type.value if isinstance(event.notice.type, pebble.NoticeType)
else event.notice.type)
self.changes.append({
'name': 'pebble-change-updated',
'container': event.workload.name,
'notice_id': event.notice.id,
'notice_type': type_str,
'notice_key': event.notice.key,
})


def get_public_methods(obj: object):
"""Get the public attributes of obj to compare to another object."""
Expand Down Expand Up @@ -5764,6 +5777,36 @@ def _on_pebble_custom_notice(self, event: ops.PebbleCustomNoticeEvent):
self.assertNotEqual(id, '')
self.assertEqual(num_notices, 0)

def test_notify_change_update(self):
harness = ops.testing.Harness(ContainerEventCharm, meta="""
name: notifier
containers:
foo:
resource: foo-image
""")
self.addCleanup(harness.cleanup)
harness.begin()
harness.charm.observe_container_events('foo')

id1 = harness.pebble_notify('foo', '42', type=pebble.NoticeType.CHANGE_UPDATE)
id2 = harness.pebble_notify('foo', '43', type=pebble.NoticeType.CHANGE_UPDATE)
self.assertNotEqual(id1, id2)

expected_changes = [{
'name': 'pebble-change-updated',
'container': 'foo',
'notice_id': id1,
'notice_type': 'change-update',
'notice_key': '42',
}, {
'name': 'pebble-change-updated',
'container': 'foo',
'notice_id': id2,
'notice_type': 'change-update',
'notice_key': '43',
}]
self.assertEqual(harness.charm.changes, expected_changes)


class PebbleNoticesMixin:
client: ops.pebble.Client
Expand Down