-
Notifications
You must be signed in to change notification settings - Fork 820
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
Alertmanager: Remove alertmanager instead of pausing #3722
Conversation
fff6eaa
to
c7964e9
Compare
c7964e9
to
fd36a58
Compare
One more Look @pstibrany? 🙏 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
There was a problem hiding this 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>
fd36a58
to
dea78ec
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
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
CHANGELOG.md
updated - the order of entries should be[CHANGE]
,[FEATURE]
,[ENHANCEMENT]
,[BUGFIX]