fix(operator): Ensure ingesters store tokens#21770
fix(operator): Ensure ingesters store tokens#21770saswatamcode wants to merge 1 commit intografana:mainfrom
Conversation
This commit adds ingester args for the operator, so the ingester store tokens on disk, and can restart without resharding data. Signed-off-by: Saswata Mukherjee <saswataminsta@yahoo.com>
saswatamcode
requested review from
a team,
JoaoBraveCoding,
periklis and
xperimental
as code owners
|
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is ON, but it could not run because the branch was deleted or merged before autofix could start.
Reviewed by Cursor Bugbot for commit d8a91f2. Configure here.
| fmt.Sprintf("-config.file=%s", path.Join(config.LokiConfigMountDir, config.LokiConfigFileName)), | ||
| fmt.Sprintf("-runtime-config.file=%s", path.Join(config.LokiConfigMountDir, config.LokiRuntimeConfigFileName)), | ||
| "-config.expand-env=true", | ||
| fmt.Sprintf("-ingester.tokens-file-path=%s", ingesterTokensFilePath), |
There was a problem hiding this comment.
Restarts still reshard ingesters
Medium Severity
Adding -ingester.tokens-file-path leaves ingester.unregister-on-shutdown at its default true. Clean pod restarts still remove the ingester from the ring, so rollouts reshard data even though the next process reloads the same tokens.
Reviewed by Cursor Bugbot for commit d8a91f2. Configure here.
There was a problem hiding this comment.
Don't think this is true anymore https://github.com/grafana/dskit/blob/main/ring/lifecycler.go#L708
2 participants
What this PR does / why we need it:
This commit adds ingester args for the operator, so the ingester store tokens on disk, and can restart without resharding data.
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Checklist
CONTRIBUTING.mdguide (required)docs/sources/setup/upgrade/_index.mddeprecated-config.yamlanddeleted-config.yamlfiles respectively in thetools/deprecated-config-checkerdirectory. Example PRNote
Medium Risk
Changes the ingester StatefulSet runtime args to persist ring tokens, which can affect ingester startup/resharding behavior on restarts. Low code churn, but it impacts a core component’s runtime configuration.
Overview
Ensures operator-managed ingesters persist their ring tokens by adding the
-ingester.tokens-file-pathargument to the ingester container (pointing at a newingesterTokensFilePathunder the data volume).Adds a focused unit test (
TestNewIngesterStatefulSet_HasTokensFileArg) to prevent regressions in the generated StatefulSet args.Reviewed by Cursor Bugbot for commit d8a91f2. Bugbot is set up for automated code reviews on this repo. Configure here.