Drop chart OH_*_KIND env now baked into the enterprise image (#14811)#719
Conversation
Bumps enterprise-server to sha-48cc27a (OpenHands#14811, which adds ENV OH_LLM_MODEL_KIND / OH_APP_CONVERSATION_INFO_KIND to the image) and removes the matching values from _env.yaml. The chart values were byte-identical to the image defaults, so this is a no-op at runtime and makes it impossible for the chart and image injector kinds to skew.
|
✅ Review complete. This review was performed through OpenHands Cloud Automation. You can log in and view the conversation here. |
all-hands-bot
left a comment
There was a problem hiding this comment.
Code Review
🟢 Good taste — Elegant, minimal change. The Helm chart no longer needs to set environment variables that are now embedded directly in the enterprise image, eliminating a potential source of version drift between chart and image.
Summary
This PR removes two chart-managed environment variables (OH_APP_CONVERSATION_INFO_KIND and OH_LLM_MODEL_KIND) and updates the corresponding enterprise image tag. The environment variables are now baked into the image itself, making the two always stay in sync.
Files Reviewed
| File | Change | Assessment |
|---|---|---|
charts/openhands/Chart.yaml |
Version bump 0.7.52 → 0.7.53 | ✅ Standard practice |
charts/openhands/templates/_env.yaml |
Removed 2 env vars + added explanation comment | ✅ Clean removal |
replicated/openhands.yaml |
Image tag sha-480c094 → sha-48cc27a | ✅ Required for the new image |
Observations
-
The comment added is appropriate here: Unlike typical "narrate the diff" comments, this one documents a design invariant — that the chart and image must stay in sync. This is useful for future maintainers.
-
Replicated config and Helm chart are in sync: Both files reference the same enterprise image (just in different deployment paths). The image tag update confirms the new image has been built and tested.
-
No breaking change: Existing deployments are unaffected since the env vars being removed were previously set by the chart; removing them just means the values come from the image instead.
[RISK ASSESSMENT]
- [Overall PR]
⚠️ Risk Assessment: 🟢 LOW
This is a low-risk infrastructure change. The environment variables being removed are simply relocated to the image layer. As long as the new enterprise image (sha-48cc27a) has these values correctly set, there is no impact on existing deployments.
VERDICT:
✅ Worth merging — Clean, focused change that reduces configuration complexity and eliminates potential version drift between chart and image.
KEY INSIGHT:
Embedding configuration like OH_APP_CONVERSATION_INFO_KIND directly in the Docker image is the right approach — it guarantees the image always runs with the correct injector class without relying on Helm chart versions to be updated in sync.
This review was generated by an AI agent (OpenHands) on behalf of the user through OpenHands Automation. View conversation
What
replicated/openhands.yamlfromsha-480c094→sha-48cc27a(= fix: default enterprise injector kinds in the image, not just the chart OpenHands#14811, currentmainHEAD).OH_APP_CONVERSATION_INFO_KINDandOH_LLM_MODEL_KINDfromcharts/openhands/templates/_env.yaml.openhandschart0.7.52→0.7.53(chart file changed).Why
OpenHands#14811 bakes those two injector-kind defaults into the enterprise image's Dockerfile (
ENV OH_LLM_MODEL_KIND=…,ENV OH_APP_CONVERSATION_INFO_KIND=…). With an image that carries them, the chart no longer needs to set them — and shouldn't, so the two can never skew (the failure mode behind #717's crashloop, where the chart referenced an injector class the pinned image didn't have).The chart values were byte-for-byte identical to the image's
ENVdefaults, so this is a no-op at runtime — the same value just comes from the image instead of the chart.Ordering / safety
The image bump and the line removal are in the same commit on purpose:
sha-480c094predates #14811 and has no baked-in defaults, so dropping the lines against the old image would leave the injector kinds unset. Pinning the #14811 image first (here, atomically) keeps every rendered state correct. Thesha-48cc27aimage build is already green.🤖 Generated with Claude Code