fix(helm): default image tag to chart appVersion - #29371
Conversation
|
|
Greptile SummaryThis PR fixes
Confidence Score: 5/5Safe to merge — changes are limited to Helm chart defaults and test files, with no impact on Python code or runtime behavior. Explicit image.tag overrides are untouched. The template change is a one-line fix in two files with a clear before/after, and the new tests confirm the regression is covered. The only friction is the hardcoded version string in the test assertions, which will require a manual edit the next time appVersion advances — a minor maintenance point, not a defect. The two test files (deployment_tests.yaml and migrations-job_tests.yaml) contain hardcoded version strings that will need updating on every future appVersion bump.
|
| Filename | Overview |
|---|---|
| deploy/charts/litellm-helm/Chart.yaml | Bumps appVersion from v1.80.12 to v1.85.1 to match the latest stable release tag published in the container registry. |
| deploy/charts/litellm-helm/templates/deployment.yaml | Removes the main- prefix from the default image tag fallback; now uses .Chart.AppVersion directly, matching how stable release images are actually tagged. |
| deploy/charts/litellm-helm/templates/migrations-job.yaml | Same main- prefix removal as deployment.yaml, keeping image tag logic consistent between the Deployment and the migrations Job. |
| deploy/charts/litellm-helm/tests/deployment_tests.yaml | Adds a unit test asserting the default image renders as the appVersion tag without a :main- prefix; image value is hardcoded to v1.85.1, so it will break if appVersion is bumped without updating this test. |
| deploy/charts/litellm-helm/tests/migrations-job_tests.yaml | Same new test as deployment_tests.yaml but for the migrations Job; carries the same hardcoded version coupling concern. |
| deploy/charts/litellm-helm/values.yaml | Updates the commented-out example tag override from main-latest to latest to match the new default behavior and avoid confusing users. |
Reviews (1): Last reviewed commit: "fix helm default image tag" | Re-trigger Greptile
| - equal: | ||
| path: spec.template.spec.containers[0].image | ||
| value: ghcr.io/berriai/litellm-database:v1.85.1 | ||
| - notMatchRegex: |
There was a problem hiding this comment.
Hardcoded version in
equal assertion will break on next appVersion bump
The equal check pins the full image string to ghcr.io/berriai/litellm-database:v1.85.1. When Chart.yaml's appVersion is next incremented, this assertion will fail even though the template logic is correct, requiring a manual test-file update. Since the goal is to prove that the tag equals the chart's appVersion (not a fixed string), the notMatchRegex: :main- check already covers the regression. The same pattern is repeated in migrations-job_tests.yaml.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
@yuneng-berri Can you take a look? |
|
Quick CI triage: the remaining red I tried to rerun the failed job, but GitHub requires repo admin rights for that run. |
Problem
Default Helm installs from the OSS branch render image refs like
ghcr.io/berriai/litellm-database:main-<appVersion>. Recent stable releases publish the plain app version tag, notmain-<version>, so the default Deployment and migrations Job can hitImagePullBackOffas reported in #29348.Fix
.Chart.AppVersioninstead ofmain-<appVersion>.appVersionfrom the stalev1.80.12tov1.85.1and the example override tolatest.image.tagpath and assert the rendered image does not use a:main-prefix.This mirrors the main-branch fix from #28710, but applies it to
litellm_oss_branchwhere external PRs are expected to target.Test
make test-unit-helm-> 55 passedhelm lint ./deploy/charts/litellm-helm-> passedgit diff --check-> passedNote: with Helm 4 locally, I had to install the existing pinned
helm-unittestplugin with--verify=false; after that, the repo'smake test-unit-helmcommand ran successfully.Risk
Low. This changes only the Helm chart default image tag path and adds chart tests. Explicit
image.tagoverrides still render unchanged.