-
Notifications
You must be signed in to change notification settings - Fork 176
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
Support configure alertmanager #148
Merged
Merged
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
6eee0db
support configure alertmanager
clyang82 a9f39ef
read all yaml file in /etc/thanos/rules
clyang82 23894fd
support ruleFileKey
clyang82 f08555a
Update message
clyang82 84a4bfb
separate alertmanager with rules
clyang82 e701891
Support multiple configmap
clyang82 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
This assumes that the key used in the configmap ends in
rules.yaml
, which is mysterious because it is not communicated to the user. Can we instead use the pattern of requiring arulesFileKey
in the jsonnet config and use that name instead?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.
Great comments. Thanks @squat I would like to just use
*.yaml
instead, because the configmap may contain multiple files. Is that OK?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.
The trouble is that there is no guarantee that the configmap key will even end in
.yaml
. The user could write a configmap where the keys areeach of these keys could have a valid rules file for the value, but the keys don't end in
.yaml
and thus this flag wouldn't work. File extensions are helpful hints and conventions but are not required.Similarly, what if the configmap hold multiple keys that end in
.yaml
but only ONE of them is actually a rules file? Then the ruler would be attempting to read something it can't understand, potentially causing problems.