feat: include check name in notice data for perform-check and recover-check #444
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…er-check change-updated notices. Code heavily based on a suggestion from @benhoyt.
This was referenced Jul 4, 2024
benhoyt
requested changes
Jul 5, 2024
There was a problem hiding this comment.
Nice work! Some small changes requested for comments and error message.
I've also tested locally and see the check-name nicely in the state file:
{
"id": "1",
"user-id": null,
"type": "change-update",
"key": "1",
"first-occurred": "2024-07-05T03:50:52.556541656Z",
"last-occurred": "2024-07-05T03:51:46.633442924Z",
"last-repeated": "2024-07-05T03:51:46.633442924Z",
"occurrences": 4,
"last-data": {
"check-name": "chk1",
"kind": "perform-check"
},
"expire-after": "168h0m0s"
}
Co-authored-by: Ben Hoyt <[email protected]>
Ah, yes, thanks. I should have mentioned that I checked that and also locally tested that wait notices has it in the last data as well. |
benhoyt
approved these changes
Jul 5, 2024
benhoyt
changed the title
jujubot
added a commit
to juju/juju
that referenced
this pull request
Jul 16, 2024
#17655 This creates new `<container-name>-pebble-check-failed` and `<container-name>-pebble-check-failed` hooks, which are triggered by Pebble change-update notices of particular kinds & statuses, with the effect that we'll now respond to Pebble checks reaching the failure threshold, or passing after having reached the threshold, by sending a pebble-check-failed or pebble-check-recovered event to the charm. ## QA steps ### Test the hooks directly: ```shell # Pack and deploy the test charm $ juju add-model t $ cd testcharms/charms/pebble-checks $ charmcraft pack $ juju deploy ./juju-qa-pebble-checks_ubuntu-22.04-amd64.charm --resource ubuntu-image=public.ecr.aws/ubuntu/ubuntu:22.04 # The check is set to succeed if `/trigger/` exists, so make it. $ juju ssh --container ubuntu juju-qa-pebble-checks/0 mkdir /trigger/ $ juju status # unit status should settle to "active" after pebble-ready # Make the check fail $ juju ssh --container ubuntu juju-qa-pebble-checks/0 rmdir /trigger/ $ juju status # will now say status "maintenance" with message "check failed: exec-check" # Make the check pass again $ juju ssh --container ubuntu juju-qa-pebble-checks/0 mkdir /trigger/ $ juju status # will now say status "active" with message "check recovered: exec-check" ``` ### Test the shell test: `./main.sh -p k8s -c minikube sidecar test_pebble_checks` ## Documentation changes prdesc [CHARMTECH-160](https://warthogs.atlassian.net/browse/CHARMTECH-160), [CHARMTECH-161](https://warthogs.atlassian.net/browse/CHARMTECH-161), [CHARMTECH-162](https://warthogs.atlassian.net/browse/CHARMTECH-162) cover updating the juju.is (main and SDK) docs ## Links <!-- Link to all relevant specification, documentation, bug, issue or JIRA card. --> * [OP046](https://docs.google.com/document/d/13ItH8l5ZSmmv9WqZXpqjV8EifZU5e3zxINiNLubnC68/edit) * [juju/charm PR](juju/charm#432) * [ops PR](canonical/operator#1281) * [pebble PR](canonical/pebble#444) * [jhack PR](canonical/jhack#172) * [Scenario PR](canonical/ops-scenario#151) **Jira card:** [CHARMTECH-109](https://warthogs.atlassian.net/browse/CHARMTECH-109) [CHARMTECH-160]: https://warthogs.atlassian.net/browse/CHARMTECH-160?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
tonyandrewmeyer
added a commit
to canonical/operator
that referenced
this pull request
Jul 17, 2024
…1281) Two new event types are supported (actually getting these events requires Juju 3.6b2 or higher): * `PebbleChangeFailedEvent` - emitted when a Pebble check in a container hits the failure threshold * `PebbleChangeRecoveredEvent` - emitted when a Pebble check in a container passes after previously hitting the failure threshold Each event type has a single new attribute (in addition to those inherited from `WorkloadEvent`): * `info` - this is a shortcut to Pebble's `get_check` method (not otherwise accessible from the charm), but will only query Pebble if more than the `name` is requested, and will cache the result on the event instance. Two new enums are added to Pebble, to make it easier to work with these events, particularly with Harness: * `ChangeStatus` - the possible values for `ops.pebble.Change.status` (note that this is the full list, but we only expect charms to see Doing, Done, and Error for now). * `ChangeKind` - the possible values for `ops.pebble.Change.kind` (again, this is the current full list, but we only expect charms to see perform-check and recover-check at the moment). `Harness.pebble_notify` is extended to support emitting the two new events. This requires implementation knowledge (that the check events are triggered by a Pebble notice of type change-updated), but provides minimal support for testing with Harness, and will be supplemented by user-friendly testing in Scenario. The testing Pebble backend gains an implementation for `get_change` and `get_checks`. These are not used directly here, but will be used in Scenario, and it makes more sense to have the implementation in ops. This is the ops portion of [OP046](https://docs.google.com/document/d/13ItH8l5ZSmmv9WqZXpqjV8EifZU5e3zxINiNLubnC68/edit). It requires a [corresponding change to Pebble](canonical/pebble#444) and [change to Juju](juju/juju#17655).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
For notices of kind
change-updated, the notice data will include thekindfield that is standard for these notices, but also whenkindisperform-checkorrecover-check, it will also includecheck-name, set to the name of the check that is being performed or recovered.This is the Pebble portion of OP046. It is used by Juju (PR) and ops (PR).