[don't merge/just review] operator: Add token storage for ingester + make sure PDBs are maxUnavail 1#1
[don't merge/just review] operator: Add token storage for ingester + make sure PDBs are maxUnavail 1#1saswatamcode wants to merge 1 commit into
Conversation
…ail 1 Signed-off-by: Saswata Mukherjee <saswataminsta@yahoo.com>
|
@saswatamcode are we running the tests as part of ci? it appears not? |
xperimental
left a comment
xperimental
left a comment
There was a problem hiding this comment.
Gave it a quick spin in a test cluster and it seems to do the things we expect to 👍
Some small code-style comments.
I'm looking forward to seeing what the impact is 🙂
| MatchLabels: l, | ||
| }, | ||
| MinAvailable: &mu, | ||
| MaxUnavailable: &maxUnavailable, |
There was a problem hiding this comment.
I think this can be "modernized" to this (ptr is already used in this file):
| MaxUnavailable: &maxUnavailable, | |
| MaxUnavailable: ptr.To(intstr.FromInt32(1)), |
| MatchLabels: l, | ||
| }, | ||
| MinAvailable: &minAvailable, | ||
| MaxUnavailable: &maxUnavailable, |
There was a problem hiding this comment.
| MaxUnavailable: &maxUnavailable, | |
| MaxUnavailable: ptr.To(intstr.FromInt32(1)), |
| 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.
(optional: This is less a comment for the purpose of adding this option for a test and more for when we later add it to the operator permanently)
We had moved "all" of the configuration to the config file at one point and, for now, I think I would move this option to the configuration file as well.
Having everything in the configuration file has its own set of advantages/disadvantages, which I have recently been thinking about again with some new insight, but I would like to make it a conscious decision.
For testing the impact of the option on running the ingesters it's fine where it is though 👍
3 participants
What this PR does / why we need it:
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 PR