Skip to content

Alertmanager: Remove alertmanager instead of pausing #3722

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

Merged

Conversation

gotjosh
Copy link
Contributor

@gotjosh gotjosh commented Jan 21, 2021

What this PR does:

Completely removes a tenant's Alertmanagers when synchronising configurations.

Before, we would "pause" the Alertmanager which would never free up the resources, and leave the Alertmanager running in-memory.

I've also bundled a small change to a logger in the ruler that was double-logging the userID.

Which issue(s) this PR fixes:
NA/NA

Checklist

  • Tests updated
  • N/A Documentation added
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

@gotjosh gotjosh changed the title Remove alertmanager instead of pausing Alertmanager: Remove alertmanager instead of pausing Jan 21, 2021
@gotjosh gotjosh force-pushed the remove-alertmanager-instead-of-pausing branch from fff6eaa to c7964e9 Compare January 21, 2021 04:40
@gotjosh gotjosh force-pushed the remove-alertmanager-instead-of-pausing branch from c7964e9 to fd36a58 Compare January 27, 2021 04:33
@gotjosh gotjosh marked this pull request as ready for review January 27, 2021 04:33
@gotjosh
Copy link
Contributor Author

gotjosh commented Jan 27, 2021

One more Look @pstibrany? 🙏

Copy link
Contributor

@pstibrany pstibrany left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

Copy link
Contributor

@pracucci pracucci left a comment

Choose a reason for hiding this comment

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

Good job @gotjosh! I left a couple of comments.

Signed-off-by: gotjosh <josue@grafana.com>
Signed-off-by: gotjosh <josue@grafana.com>
Signed-off-by: gotjosh <josue@grafana.com>
Signed-off-by: gotjosh <josue@grafana.com>
Signed-off-by: gotjosh <josue@grafana.com>
@gotjosh gotjosh force-pushed the remove-alertmanager-instead-of-pausing branch from fd36a58 to dea78ec Compare January 29, 2021 15:01
Copy link
Contributor

@pracucci pracucci left a comment

Choose a reason for hiding this comment

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

Thanks!

Copy link
Contributor

@pstibrany pstibrany left a comment

Choose a reason for hiding this comment

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

👍

@pstibrany pstibrany merged commit 02f601a into cortexproject:master Feb 1, 2021
@gotjosh gotjosh deleted the remove-alertmanager-instead-of-pausing branch February 1, 2021 23:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants