-
Notifications
You must be signed in to change notification settings - Fork 512
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
Non-ingesters should not fail on its config validation #7990
Conversation
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.
Added a comment about test, but in general LGTM.
0b312bf
to
a5f39a6
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.
LGTM! There's a conflict in the changelog. Can you resolve that and I can merge the PR?
Thanks for the cleanup btw :)
pkg/mimir/mimir.go
Outdated
if c.isAnyModuleEnabled(Ingester, Write, All) || !errors.Is(err, ingester.ErrSpreadMinimizingValidation) { | ||
return errors.Wrap(err, "invalid ingester config") | ||
} | ||
level.Warn(log).Log("ingester config is invalid; moving on because the \"ingester\" module is not in this process's targets", "err", err.Error()) |
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'd opt for a debug log here. It shouldn't be anything to worry about. WDYT?
Signed-off-by: Vladimir Varankin <vladimir.varankin@grafana.com>
Signed-off-by: Vladimir Varankin <vladimir.varankin@grafana.com>
a5f39a6
to
b9ebb43
Compare
…g validation Signed-off-by: Vladimir Varankin <vladimir.varankin@grafana.com>
Signed-off-by: Vladimir Varankin <vladimir.varankin@grafana.com>
Signed-off-by: Vladimir Varankin <vladimir.varankin@grafana.com>
b9ebb43
to
2a36e8e
Compare
What this PR does
These changes fix an issue with mimir's config validation, where non-ingester modules fail on its config validation, when
-ingester.ring.token-generation-strategy=spread-minimizing
is set.Which issue(s) this PR fixes or relates to
Fixes #7822
Checklist
CHANGELOG.md
updated - the order of entries should be[CHANGE]
,[FEATURE]
,[ENHANCEMENT]
,[BUGFIX]
.about-versioning.md
updated with experimental features.