-
Notifications
You must be signed in to change notification settings - Fork 812
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
Ruler ignore users #4074
Ruler ignore users #4074
Conversation
Signed-off-by: Peter Štibraný <[email protected]>
Signed-off-by: Peter Štibraný <[email protected]>
Signed-off-by: Peter Štibraný <[email protected]>
Signed-off-by: Peter Štibraný <[email protected]>
Signed-off-by: Peter Štibraný <[email protected]>
Signed-off-by: Peter Štibraný <[email protected]>
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.
Code looks 👌 , I just have a couple of comments on naming and wording.
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! LGTM! 🚀
Signed-off-by: Peter Štibraný <[email protected]>
Thanks for your reviews! I've now addressed all your feedback. |
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!
@@ -277,6 +277,8 @@ func TestSharding(t *testing.T) { | |||
shardingStrategy string | |||
shuffleShardSize int | |||
setupRing func(*ring.Desc) | |||
enabledUsers []string |
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.
nit: not sure if intended given I see other occurrences of the word but we're still using the Users suffix throughout this test.
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.
I would need to change entire test to replace that. Doesn't seem to be worth the effort. I've changed ruler.go and compactor.go though.
Signed-off-by: Peter Štibraný <[email protected]>
What this PR does: This PR adds
-ruler.enabled-tenants
and-ruler.disabled-tenants
options to explicitly enable or disable rules processing for specific tenants. Code is extracted from Compactor, where this support already exists.Checklist
CHANGELOG.md
updated - the order of entries should be[CHANGE]
,[FEATURE]
,[ENHANCEMENT]
,[BUGFIX]