Skip to content
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

mimir-distributed: allow components to override their container image #10340

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

narqo
Copy link
Contributor

@narqo narqo commented Jan 3, 2025

What this PR does

The PR updates the Helm chart to allow an individual component to override its container image reference via the component's local values. E.g.

# values.yaml

ingester:
  image:
    repository: registry.example/mimir-ingester
    tag: blah-blah

The order in which the values take precedence is:

  1. take the component's local image or tag if defined
  2. take the image defined in the enterprise section if enterprise is enabled
  3. take the globally-defined image otherwise

Which issue(s) this PR fixes or relates to

Fixes #10237

Checklist

  • Tests updated.
  • Documentation added.
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX].
  • about-versioning.md updated with experimental features.

@narqo narqo requested a review from a team as a code owner January 3, 2025 12:34
narqo added 3 commits January 3, 2025 13:35
Signed-off-by: Vladimir Varankin <[email protected]>
Signed-off-by: Vladimir Varankin <[email protected]>
Signed-off-by: Vladimir Varankin <[email protected]>
@narqo narqo force-pushed the vldmr/helm-component-image-override branch from a650f8a to d559783 Compare January 3, 2025 12:35
Copy link
Contributor

@dimitarvdimitrov dimitarvdimitrov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you also include the keys in the values.yaml? Since we still want to use the top-level image, then you can make the new <component>.image be a comment. Something like the TSC examples

# -- topologySpreadConstraints allows to customize the default topologySpreadConstraints. This can be either a single dict as shown below or a slice of topologySpreadConstraints.
# labelSelector is taken from the constraint itself (if it exists) or is generated by the chart using the same selectors as for services.
topologySpreadConstraints:
maxSkew: 1
topologyKey: kubernetes.io/hostname
whenUnsatisfiable: ScheduleAnyway
# minDomains: 1
# nodeAffinityPolicy: Honor
# nodeTaintsPolicy: Honor
# matchLabelKeys:
# - pod-template-hash

that's basically the only documentation we have of helm chart features (plus the changelog, which is barely documentation)

@@ -30,6 +30,7 @@ Entries should include a reference to the Pull Request that introduced the chang
## main / unreleased

* [CHANGE] Memcached: Update to Memcached 1.6.34. #10318
* [ENHANCEMENT] Add support for individual components to override their container image. #10340
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i've been trying to make the changelog enough for anyone to get the gist of a change and know how to use it. Can you give an example of overriding the image for a component? Something like ingester.image.tag and ingester.image.repository. Or explain that there's an image object under all component objects now

Build mimir image reference based on whether enterprise features are requested. The component local values always take precedence.
Params:
ctx = . context
component = component name (optional)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

componentSectionFromName errors out if the component doesn't exist, so i think component isn't optional

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow override image of individual components in Helm chart
2 participants