-
Notifications
You must be signed in to change notification settings - Fork 820
Disable access to Alertmanager (user, or sending alerts), if it is not running. #3679
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
Conversation
Signed-off-by: Peter Štibraný <peter.stibrany@grafana.com>
Signed-off-by: Peter Štibraný <peter.stibrany@grafana.com>
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.
Is this enough? In addition, shouldn't we require a check to the backend before storing the fallback? Othwerwise, we could end up in a similar situation in the future. For instance, once we start sharding Alertmanager tenants if a request somehow gets routed to an Alertmanager without the tenant loaded it will override the config.
I think yes, for now.
That's a valid point, and something keep an eye on when reviewing AM sharding. I think we should also reconsider idea of uploading "empty" config to the same path as user-uploaded config. Eg. we could give empty config ".default" suffix. |
Signed-off-by: Peter Štibraný <peter.stibrany@grafana.com>
Signed-off-by: Peter Štibraný <peter.stibrany@grafana.com>
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 catch @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 modulo changelog suggestion
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!
Signed-off-by: Peter Štibraný <peter.stibrany@grafana.com>
What this PR does: This PR disables access to user's Alertmanager (by user, or services trying to send alert) until
MultitenantAlertmanager
has started and synced configuration for each user.Call to tenant's AM before
MultitenantAlertmanager
is running may result in loss of configuration for this tenant.Checklist
CHANGELOG.md
updated - the order of entries should be[CHANGE]
,[FEATURE]
,[ENHANCEMENT]
,[BUGFIX]