-
Notifications
You must be signed in to change notification settings - Fork 524
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
operations: Remove gossip_ring_member selector from services #1008
Conversation
de12f2a
to
6845402
Compare
Can we add test with gossip enabled into |
@pstibrany I'm not familiar with mimir-tests, but I'm for the idea of testing this. |
6845402
to
254d779
Compare
@pstibrany I added a test, which I based on test-query-sharding.jsonnet in order to get also the query scheduler, ruler and alertmanager services. Does it look alright? |
BTW, not sure why the Jsonnet test fails in CI, locally it works fine. |
Your test looks fine to me. It also shows that services don't use I'm surprised that importing |
f181599
to
f2e4c31
Compare
Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>
f2e4c31
to
8993e24
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.
Good job!
What this PR does:
In Jsonnet library, remove "gossip_ring_member" label selector for other services than the
gossip-ring
headless service.There already exists some logic to achieve this (in gossip.libsonnet), but it doesn't quite work due to Jsonnet merging (or inheritance in Jsonnet lingo) order I think :/
Which issue(s) this PR fixes:
Checklist
CHANGELOG.md
updated - the order of entries should be[CHANGE]
,[FEATURE]
,[ENHANCEMENT]
,[BUGFIX]