Skip to content

Conversation

pracucci
Copy link
Contributor

What this PR does:
Earlier this week I was testing the alertmanager in the local setup and I've learned the hard way that ruler->alertmanger notification doesn't work in the local dev setup and that if you configure -alertmanager.web.external-url with an ending / the routing doesn't work as expected.

This PR fixes the local dev env and adds a validation rule on -alertmanager.web.external-url.

Which issue(s) this PR fixes:
N/A

Checklist

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

Signed-off-by: Marco Pracucci <[email protected]>
@pstibrany
Copy link
Contributor

I've learned the hard way that ruler->alertmanger notification doesn't work in the local dev setup and that if you configure -alertmanager.web.external-url with an ending /

Instead of erroring out, can we fix the problem with /? Either by removing it from the path, or using path.Join when appending. Why does having / in external-url fail in the first place?

@pracucci
Copy link
Contributor Author

Why does having / in external-url fail in the first place?

I don't have time to investigate it (I consider it low priority). I see two options: accept the validation as a signal of a misconfiguration or I can roll it back and we just keep the fix for the dev env.

@pstibrany
Copy link
Contributor

I see two options: accept the validation as a signal of a misconfiguration or I can roll it back and we just keep the fix for the dev env.

Validation is better than rollback. Approved. We can improve it when there is more time.

@pracucci pracucci merged commit 8851245 into cortexproject:master Apr 16, 2021
@pracucci pracucci deleted the fix-alertmanager-local-dev-and-config-validation branch April 16, 2021 14:30
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.

2 participants