Skip to content

[Feat] Add global image registry override to Helm chart - #1100

Open
rohan-patnaik wants to merge 1 commit into
vllm-project:mainfrom
rohan-patnaik:feat/global-image-registry-vps785
Open

rohan-patnaik wants to merge 1 commit into
vllm-project:mainfrom
rohan-patnaik:feat/global-image-registry-vps785

Conversation

@rohan-patnaik

Copy link
Copy Markdown

Add global.imageRegistry for all chart-managed images, preserving repository paths, tags and digests. Empty values retain existing images. Document monitoring dependency support and the separate Prometheus Adapter override.

Fixes #785.

CPU validation on Linux and macOS: 5 registry tests pass (3 fail on base), Helm lint and render comparisons pass. Full Helm suite: 147 passed, 2 failed; both failures also occur on base. Schema regeneration matches. No GPU, Kubernetes deployment or image-pull tests were run.

AI-assisted implementation and independent review.

  • Applicable YAML/JSON, formatting, spelling and Markdown checks pass.
  • Contributor's DCO sign-off is preserved.
  • Title uses [Feat].

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a global image registry override feature (global.imageRegistry) to the Helm chart, allowing users to pull images from a shared registry while preserving repository paths, tags, and digests. It updates various deployment templates to use a new helper function chart.image for image resolution, and adds corresponding documentation, schema updates, and unit tests. Feedback suggests adding a defensive check in the chart.image helper to ensure both $registry and $image are present before processing, preventing potential template rendering errors when an image is empty or undefined.

Comment thread helm/templates/_helpers.tpl Outdated
Comment on lines +9 to +18
{{- if $registry -}}
{{- $parts := splitList "/" $image -}}
{{- $first := first $parts -}}
{{- if and (gt (len $parts) 1) (or (contains "." $first) (contains ":" $first) (eq $first "localhost")) -}}
{{- $image = join "/" (rest $parts) -}}
{{- end -}}
{{- printf "%s/%s" $registry $image -}}
{{- else -}}
{{- $image -}}
{{- end -}}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

To prevent potential template rendering errors (such as nil pointer or type conversion issues when calling splitList on an empty or undefined image) and to avoid generating malformed image strings like registry.example.com/ when an image is empty, we should add a defensive check to ensure both $registry and $image are present before attempting to split and rewrite the image reference.

{{- if and $registry $image -}}
  {{- $parts := splitList "/" $image -}}
  {{- $first := first $parts -}}
  {{- if and (gt (len $parts) 1) (or (contains "." $first) (contains ":" $first) (eq $first "localhost")) -}}
    {{- $image = join "/" (rest $parts) -}}
  {{- end -}}
  {{- printf "%s/%s" $registry $image -}}
{{- else -}}
  {{- $image -}}
{{- end -}}

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Fixed in d4c1f34. The helper now normalizes an absent image to an empty string and only rewrites when both registry and image are present. I added a regression using the real sidecar call path: before the fix it rendered mirror.example.com/; it now preserves the empty image. All 6 image-registry tests pass and helm lint passes. The complete 150-test chart run has 148 passes plus two unrelated existing assertion/plugin-compatibility failures in chat-template whitespace and dotted annotation paths.

AI-assisted implementation.

Signed-off-by: rohanpatnaik <rohanpatnaik1997@gmail.com>
@rohan-patnaik
rohan-patnaik force-pushed the feat/global-image-registry-vps785 branch from 5c81f6b to d4c1f34 Compare September 27, 2026 20:34

This branch has not been deployed

No deployments
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.

feature: global registry in helm chart

1 participant