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 interface for retrieving alerting config from charmed AlertManager #14

Merged
merged 1 commit into from
Jun 2, 2020

Conversation

relaxdiego
Copy link
Collaborator

@relaxdiego relaxdiego commented May 28, 2020

This commit allows the charm to retrieve a configuration string from the
alertmanager charm. The configuration string is a JSON string that this
charm converts to a dict that is then inserted to another dict that
ultimately gets serialized into a YAML-formatted prometheus config file.

While this charm is agnostic to the contents of the JSON string, in
AlertManager's current iteration, it uses the kube_sd_configs method
so that Prometheus can quickly detect changes in the number of
AlertManager pods without waiting for any Juju events.

@relaxdiego relaxdiego marked this pull request as draft May 28, 2020 01:35
@relaxdiego

This comment has been minimized.

@relaxdiego relaxdiego force-pushed the add-alerting branch 3 times, most recently from 086920f to 689f5db Compare May 29, 2020 05:56
@relaxdiego relaxdiego requested a review from exceptorr May 29, 2020 05:57
@relaxdiego relaxdiego marked this pull request as ready for review May 29, 2020 05:57
Copy link
Contributor

@exceptorr exceptorr left a comment

Choose a reason for hiding this comment

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

Hey @relaxdiego - great job! Just a few comments to discuss.

src/charm.py Outdated Show resolved Hide resolved
src/charm.py Outdated Show resolved Hide resolved
src/charm.py Outdated Show resolved Hide resolved
src/charm.py Outdated Show resolved Hide resolved
src/charm.py Outdated Show resolved Hide resolved
src/domain.py Outdated Show resolved Hide resolved
src/interface_alertmanager.py Outdated Show resolved Hide resolved
test/charm_test.py Show resolved Hide resolved
@relaxdiego
Copy link
Collaborator Author

Thanks for the review @exceptorr! Will address them now.

@relaxdiego relaxdiego force-pushed the add-alerting branch 5 times, most recently from 3a780f5 to 634d3a6 Compare June 2, 2020 03:00
This commit allows the charm to retrieve a configuration string from the
alertmanager charm. The configuration string is a JSON string that this
charm converts to a dict that is then inserted to another dict that
ultimately gets serialized into a YAML-formatted prometheus config file.

While this charm is agnostic to the contents of the JSON string, in
AlertManager's current iteration, it uses the `kube_sd_configs` method
so that Prometheus can quickly detect changes in the number of
AlertManager pods without waiting for any Juju events.
@relaxdiego relaxdiego changed the title Add AlertManager Support Add interface for retrieving alerting config from charmed AlertManager Jun 2, 2020
@relaxdiego relaxdiego requested a review from exceptorr June 2, 2020 05:13
Copy link
Contributor

@exceptorr exceptorr left a comment

Choose a reason for hiding this comment

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

Thanks, looks much better now! A few comments before we can land this - please, see inline.

test/charm_test.py Show resolved Hide resolved
test/interface_alertmanager_test.py Show resolved Hide resolved
@exceptorr exceptorr merged commit 8ef5661 into master Jun 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants