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: Add new hook types for Pebble check events (OP046) #432

Merged
merged 1 commit into from
Jul 9, 2024

Conversation

tonyandrewmeyer
Copy link
Contributor

@tonyandrewmeyer tonyandrewmeyer commented Jul 3, 2024

Add new hook types for the upcoming Pebble check events work. See OP046. New hook names are:

  • <container>-pebble-check-failed
  • <container>-pebble-check-recovered

This also removes the pebble-change-updated hook, since adding support for pebble-change-updated-notice events was reverted (pre-release) from Juju and there are no current plans to add it back (see OP045).

This is required for the main Juju PR adding check event support.

@jujubot
Copy link
Contributor

jujubot commented Jul 3, 2024

Thanks for opening a pull request! Please follow the instructions here to ensure your pull request is ready for review. Then, a maintainer will review your patch.

@hpidcock @anvial

1 similar comment
@jujubot
Copy link
Contributor

jujubot commented Jul 3, 2024

Thanks for opening a pull request! Please follow the instructions here to ensure your pull request is ready for review. Then, a maintainer will review your patch.

@hpidcock @anvial

See [OP046](https://docs.google.com/document/d/13ItH8l5ZSmmv9WqZXpqjV8EifZU5e3zxINiNLubnC68/edit). New hook names are:

* `<container>-pebble-check-failed`
* `<container>-pebble-check-recovered`
Copy link
Member

@SimonRichardson SimonRichardson left a comment

Choose a reason for hiding this comment

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

LGTM. This will need to be brought into 4.0 branch directly once this has landed. We're sunsetting most of the repositories in the github juju namespace.

See: https://github.com/juju/juju/tree/main/internal/charm for the new location.

PS. Any prior work will need to be moved as well, just in case.

@SimonRichardson
Copy link
Member

PR to add deprecation warning #433

@hpidcock hpidcock merged commit c09ff56 into juju:v12 Jul 9, 2024
@hpidcock hpidcock mentioned this pull request Jul 9, 2024
@tonyandrewmeyer tonyandrewmeyer deleted the pebble-check-hooks branch July 9, 2024 10:49
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants