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 support for Pebble change-update notice #17118

Merged
merged 1 commit into from
Apr 8, 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
31 changes: 31 additions & 0 deletions testcharms/charms/pebble-notices/src/charm.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,24 @@ def __init__(self, *args):
self.framework.observe(self.on["redis"].pebble_ready, self._on_pebble_ready)
self.framework.observe(self.on["redis"].pebble_custom_notice, self._on_custom_notice)

# TODO(benhoyt): update to use pebble_change_updated once ops supports that:
# https://github.com/canonical/operator/pull/1170
import os
import pathlib

dispatch_path = pathlib.Path(os.environ.get("JUJU_DISPATCH_PATH", ""))
event_name = dispatch_path.name.replace("-", "_")
logger.info(f"__init__: path={dispatch_path} event={event_name}")
if event_name == "redis_pebble_change_updated":
event = ops.PebbleNoticeEvent(
None,
self.unit.get_container(os.environ["JUJU_WORKLOAD_NAME"]),
os.environ["JUJU_NOTICE_ID"],
os.environ["JUJU_NOTICE_TYPE"],
os.environ["JUJU_NOTICE_KEY"],
)
self._on_change_updated(event)

def _on_pebble_ready(self, event):
self.unit.status = ops.ActiveStatus()

Expand All @@ -28,6 +46,19 @@ def _on_custom_notice(self, event):
# Don't include the (arbitrary) ID in the status message
self.unit.status = ops.MaintenanceStatus(f"notice type={notice_type} key={notice_key}")

def _on_change_updated(self, event):
notice_id = event.notice.id
notice_type = (
event.notice.type if isinstance(event.notice.type, str) else event.notice.type.value
)
notice_key = event.notice.key
logger.info(f"_on_change_updated: id={notice_id} type={notice_type} key={notice_key}")

change = event.workload.pebble.get_change(notice_key)
self.unit.status = ops.MaintenanceStatus(
f"notice type={notice_type} kind={change.kind} status={change.status}"
)


if __name__ == "__main__":
ops.main(PebbleNoticesCharm)
68 changes: 68 additions & 0 deletions testcharms/charms/pebble-notices/tests/unit/test_charm.py
Original file line number Diff line number Diff line change
@@ -1,15 +1,18 @@
import unittest
import unittest.mock

import ops
import ops.testing
from charm import PebbleNoticesCharm
from ops import pebble


class TestCharm(unittest.TestCase):
def setUp(self):
self.harness = ops.testing.Harness(PebbleNoticesCharm)
self.addCleanup(self.harness.cleanup)
self.harness.begin()
self._next_notice_id = 1

def test_pebble_ready(self):
self.harness.container_pebble_ready("redis")
Expand All @@ -27,3 +30,68 @@ def test_custom_notice(self):
self.harness.model.unit.status,
ops.MaintenanceStatus("notice type=custom key=ubuntu.com/bar/buzz"),
)

@unittest.mock.patch("ops.testing._TestingPebbleClient.get_change")
def test_change_updated(self, mock_get_change):
# TODO(benhoyt): update to use pebble_change_updated once ops supports that:
# https://github.com/canonical/operator/pull/1170

import os

os.environ["JUJU_DISPATCH_PATH"] = "hooks/redis-pebble-change-updated"
self.addCleanup(os.environ.__delitem__, "JUJU_DISPATCH_PATH")
self.addCleanup(os.environ.__delitem__, "JUJU_NOTICE_ID")
self.addCleanup(os.environ.__delitem__, "JUJU_NOTICE_TYPE")
self.addCleanup(os.environ.__delitem__, "JUJU_NOTICE_KEY")

mock_get_change.return_value = pebble.Change.from_dict(
{
"id": "1",
"kind": "exec",
"summary": "",
"status": "Doing",
"ready": False,
"spawn-time": "2021-01-28T14:37:02.247202105+13:00",
}
)
self._pebble_notify_change_updated("redis", "123")
self.assertEqual(
self.harness.model.unit.status,
ops.MaintenanceStatus("notice type=change-update kind=exec status=Doing"),
)
mock_get_change.assert_called_once_with("123")

mock_get_change.reset_mock()
mock_get_change.return_value = pebble.Change.from_dict(
{
"id": "2",
"kind": "changeroo",
"summary": "",
"status": "Done",
"ready": True,
"spawn-time": "2024-01-28T14:37:02.247202105+13:00",
"ready-time": "2024-01-28T14:37:04.291517768+13:00",
}
)
self._pebble_notify_change_updated("redis", "42")
self.assertEqual(
self.harness.model.unit.status,
ops.MaintenanceStatus("notice type=change-update kind=changeroo status=Done"),
)
mock_get_change.assert_called_once_with("42")

def _pebble_notify_change_updated(self, container_name, notice_key):
import os

os.environ["JUJU_NOTICE_ID"] = notice_id = str(self._next_notice_id)
self._next_notice_id += 1
os.environ["JUJU_NOTICE_TYPE"] = notice_type = "change-update"
os.environ["JUJU_NOTICE_KEY"] = notice_key
event = ops.PebbleNoticeEvent(
None,
self.harness.model.unit.get_container(container_name),
notice_id,
notice_type,
notice_key,
)
self.harness.charm._on_change_updated(event)
21 changes: 21 additions & 0 deletions tests/suites/sidecar/sidecar.sh
Original file line number Diff line number Diff line change
Expand Up @@ -89,3 +89,24 @@ test_pebble_notices() {
# Clean up model
destroy_model "${model_name}"
}

test_pebble_change_updated() {
echo

# Ensure that a valid Juju controller exists
model_name="controller-model-sidecar"
file="${TEST_DIR}/test-${model_name}.log"
ensure "${model_name}" "${file}"

# Deploy Pebble Notices test application
juju deploy juju-qa-pebble-notices
wait_for "active" '.applications["juju-qa-pebble-notices"].units["juju-qa-pebble-notices/0"]["workload-status"].current'

# Check that charm is responding correctly to a change-update notice
juju ssh --container redis juju-qa-pebble-notices/0 /charm/bin/pebble exec -- echo foo
wait_for "maintenance" '.applications["juju-qa-pebble-notices"].units["juju-qa-pebble-notices/0"]["workload-status"].current'
wait_for "notice type=change-update kind=exec status=Done" '.applications["juju-qa-pebble-notices"].units["juju-qa-pebble-notices/0"]["workload-status"].message'

# Clean up model
destroy_model "${model_name}"
}
1 change: 1 addition & 0 deletions tests/suites/sidecar/task.sh
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ test_sidecar() {
test_deploy_and_remove_application
test_deploy_and_force_remove_application
test_pebble_notices
test_pebble_change_updated
;;
*)
echo "==> TEST SKIPPED: sidecar charm tests, not a k8s provider"
Expand Down
9 changes: 9 additions & 0 deletions worker/uniter/container/workload.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ const (
// ReadyEvent is triggered when the container/pebble starts up.
ReadyEvent WorkloadEventType = iota
CustomNoticeEvent
ChangeUpdatedEvent
)

// WorkloadEvent contains information about the event type and data associated with
Expand Down Expand Up @@ -197,6 +198,14 @@ func (r *workloadHookResolver) NextOp(
NoticeType: evt.NoticeType,
NoticeKey: evt.NoticeKey,
})
case ChangeUpdatedEvent:
op, err = opFactory.NewRunHook(hook.Info{
Kind: hooks.PebbleChangeUpdated,
WorkloadName: evt.WorkloadName,
NoticeID: evt.NoticeID,
NoticeType: evt.NoticeType,
NoticeKey: evt.NoticeKey,
})
case ReadyEvent:
op, err = opFactory.NewRunHook(hook.Info{
Kind: hooks.PebbleReady,
Expand Down
43 changes: 43 additions & 0 deletions worker/uniter/container/workload_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,3 +131,46 @@ func (s *workloadSuite) TestWorkloadCustomNoticeHook(c *gc.C) {
NoticeKey: "example.com/foo",
})
}

func (s *workloadSuite) TestWorkloadChangeUpdatedHook(c *gc.C) {
events := container.NewWorkloadEvents()
expectedErr := errors.Errorf("expected error")
handler := func(err error) {
c.Assert(err, gc.Equals, expectedErr)
}
containerResolver := container.NewWorkloadHookResolver(
loggo.GetLogger("test"),
events,
events.RemoveWorkloadEvent)
localState := resolver.LocalState{
State: operation.State{
Kind: operation.Continue,
Step: operation.Pending,
},
}
remoteState := remotestate.Snapshot{
WorkloadEvents: []string{
events.AddWorkloadEvent(container.WorkloadEvent{
Type: container.ChangeUpdatedEvent,
WorkloadName: "test",
NoticeID: "123",
NoticeType: "change-update",
NoticeKey: "42",
}, handler),
},
}
opFactory := &mockOperations{}
op, err := containerResolver.NextOp(localState, remoteState, opFactory)
c.Assert(err, jc.ErrorIsNil)
c.Assert(op, gc.NotNil)
op = operation.Unwrap(op)
hookOp, ok := op.(*mockRunHookOp)
c.Assert(ok, jc.IsTrue)
c.Assert(hookOp.hookInfo, gc.DeepEquals, hook.Info{
Kind: "pebble-change-updated",
WorkloadName: "test",
NoticeID: "123",
NoticeType: "change-update",
NoticeKey: "42",
})
}
2 changes: 1 addition & 1 deletion worker/uniter/hook/hook.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ func (hi Info) Validate() error {
return errors.Errorf("%q hook has a remote unit but no application", hi.Kind)
}
return nil
case hooks.PebbleCustomNotice:
case hooks.PebbleCustomNotice, hooks.PebbleChangeUpdated:
if hi.WorkloadName == "" {
return errors.Errorf("%q hook requires a workload name", hi.Kind)
}
Expand Down
6 changes: 6 additions & 0 deletions worker/uniter/hook/hook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,12 @@ var validateTests = []struct {
}, {
hook.Info{Kind: hooks.PebbleCustomNotice, WorkloadName: "test"},
`"pebble-custom-notice" hook requires a notice ID, type, and key`,
}, {
hook.Info{Kind: hooks.PebbleChangeUpdated},
`"pebble-change-updated" hook requires a workload name`,
}, {
hook.Info{Kind: hooks.PebbleChangeUpdated, WorkloadName: "test"},
`"pebble-change-updated" hook requires a notice ID, type, and key`,
}, {
hook.Info{Kind: hooks.PreSeriesUpgrade},
`"pre-series-upgrade" hook requires a target base`,
Expand Down
2 changes: 2 additions & 0 deletions worker/uniter/pebblenotices.go
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,8 @@ func (n *pebbleNoticer) processNotice(containerName string, notice *client.Notic
switch notice.Type {
case client.CustomNotice:
eventType = container.CustomNoticeEvent
case client.ChangeUpdateNotice:
eventType = container.ChangeUpdatedEvent
default:
n.logger.Debugf("container %q: ignoring %s notice", containerName, notice.Type)
return nil
Expand Down
19 changes: 19 additions & 0 deletions worker/uniter/pebblenotices_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,25 @@ func (s *pebbleNoticerSuite) TestWaitNotices(c *gc.C) {
})
}

func (s *pebbleNoticerSuite) TestChangeUpdate(c *gc.C) {
s.setUpWorker(c, []string{"c1"})
defer workertest.CleanKill(c, s.worker)

s.clients["c1"].AddNotice(c, &client.Notice{
ID: "1",
Type: "change-update",
Key: "42",
LastRepeated: time.Now(),
})
s.waitWorkloadEvent(c, container.WorkloadEvent{
Type: container.ChangeUpdatedEvent,
WorkloadName: "c1",
NoticeID: "1",
NoticeType: "change-update",
NoticeKey: "42",
})
}

func (s *pebbleNoticerSuite) TestWaitNoticesError(c *gc.C) {
s.setUpWorker(c, []string{"c1"})
defer workertest.CleanKill(c, s.worker)
Expand Down
2 changes: 1 addition & 1 deletion worker/uniter/runner/context/contextfactory.go
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,7 @@ func (f *contextFactory) HookContext(hookInfo hook.Info) (*HookContext, error) {
ctx.workloadName = hookInfo.WorkloadName
hookName = fmt.Sprintf("%s-%s", hookInfo.WorkloadName, hookName)
switch hookInfo.Kind {
case hooks.PebbleCustomNotice:
case hooks.PebbleCustomNotice, hooks.PebbleChangeUpdated:
ctx.noticeID = hookInfo.NoticeID
ctx.noticeType = hookInfo.NoticeType
ctx.noticeKey = hookInfo.NoticeKey
Expand Down