refactor: update helm config to new format - #388
Merged
Conversation
evacchi
force-pushed
the
refactor-config-helm
branch
from
August 3, 2026 13:29
bbdaf4a to
b3e3a9d
Compare
8 tasks
evacchi
force-pushed
the
refactor-config-helm
branch
from
August 3, 2026 13:36
b3e3a9d to
5e54186
Compare
7 tasks
evacchi
force-pushed
the
refactor-config-helm
branch
from
August 4, 2026 06:19
7ef4c84 to
41d81b3
Compare
evacchi
marked this pull request as ready for review
August 4, 2026 06:20
evacchi
requested review from
RishabhSaini,
ahg-g,
jtechapps and
shimib
as code owners
August 4, 2026 06:20
There was a problem hiding this comment.
Pull request overview
Updates the llm-d-async Helm chart, E2E fixtures, and docs to align with the unified transport configuration surface introduced in #378, so the chart emits only --transport / --transport-config (while still translating deprecated per-backend values for compatibility).
Changes:
- Refactors the Helm chart to select/configure backends via
ap.transport+ap.transportConfig, and synthesizes transport config from deprecated values when used. - Updates Helm unit tests and E2E values/manifest fixtures to the new transport config format.
- Updates documentation and adds a Redis config-loading regression test; adds a release note entry.
Reviewed changes
Copilot reviewed 31 out of 31 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| test/e2e/yaml/llm-d-async-quota.yaml | Switches E2E manifest args to --transport / inline --transport-config. |
| test/e2e/yaml/llm-d-async-composite.yaml | Switches E2E manifest args to unified transport config (including poll interval). |
| test/e2e/helm/tier-priority.yaml | Updates Helm E2E values to ap.transport / ap.transportConfig format. |
| test/e2e/helm/short-drain.yaml | Updates Helm E2E values to unified transport surface. |
| test/e2e/helm/saturation.yaml | Updates Helm E2E values to unified transport surface; keeps metrics/redis blocks. |
| test/e2e/helm/redis-gate.yaml | Updates Helm E2E values to unified transport surface for a Redis gate case. |
| test/e2e/helm/quota.yaml | Updates Helm E2E values to unified transport surface for quota gating. |
| test/e2e/helm/prometheus-query.yaml | Updates Helm E2E values to unified transport surface for prometheus-query gating. |
| test/e2e/helm/multitenant.yaml | Updates multitenant Helm E2E values to unified transport config. |
| test/e2e/helm/mt-merge.yaml | Updates mt-merge Helm E2E values to unified transport config. |
| test/e2e/helm/integration.yaml | Updates integration Helm E2E values to unified transport config. |
| test/e2e/helm/endpoint-scrape.yaml | Updates endpoint-scrape Helm E2E values to unified transport config. |
| test/e2e/helm/composite.yaml | Updates composite gating Helm E2E values to unified transport config. |
| test/e2e/helm/budget.yaml | Updates budget gate Helm E2E values to unified transport config. |
| test/e2e/helm/benchmark.yaml | Updates benchmark Helm E2E values to unified transport config. |
| test/e2e/helm/benchmark-pool-gate.yaml | Updates benchmark pool gating Helm E2E values to unified transport config. |
| release-notes.d/unreleased/388.md | Adds release note describing the Helm chart transport-config migration and deprecations. |
| pkg/redis/options_test.go | Adds regression test ensuring REDIS_URL is a default (doesn’t override explicit config URL). |
| docs/guides/e2e-deploy/llm-d-async-values.yaml | Updates the guide’s example values to the unified transport surface (including urlSecret). |
| docs/guides/e2e-deploy.md | Updates narrative instructions to match new Helm values and secret handling. |
| charts/llm-d-async/values.yaml | Documents new ap.transport/ap.transportConfig and marks legacy blocks deprecated. |
| charts/llm-d-async/tests/validation_test.yaml | Adds Helm unittest coverage for new fail-fast transport validation. |
| charts/llm-d-async/tests/redis_secret_test.yaml | Adds Helm unittest coverage for transportConfig/legacy Redis Secret creation behavior. |
| charts/llm-d-async/tests/observability_test.yaml | Updates test defaults to satisfy new fail-fast validation via minimal transport config. |
| charts/llm-d-async/tests/deprecated_transport_test.yaml | Adds Helm unittest coverage ensuring legacy values translate to unified transport args (no retired flags). |
| charts/llm-d-async/tests/deployment_test.yaml | Updates and expands deployment unit tests around unified transport args and REDIS_URL injection. |
| charts/llm-d-async/templates/redis-secret.yaml | Refactors Redis Secret creation to support new transportConfig.urlSecret.url path. |
| charts/llm-d-async/templates/NOTES.txt | Adds install notes describing unified transport surface and migration warnings for deprecated values. |
| charts/llm-d-async/templates/ap-deployments.yaml | Implements fail-fast validation + emits unified --transport / --transport-config args only. |
| charts/llm-d-async/templates/ap-configmap.yaml | Removes Pub/Sub topics config file rendering (topics now inline in transport config). |
| charts/llm-d-async/templates/_helpers.tpl | Adds transport selection + legacy synthesis helpers and URL-secret resolution helpers. |
Suppressed comments (1)
charts/llm-d-async/templates/_helpers.tpl:188
- In llm-d-async.redisSecretKey, if a user sets ap.transportConfig.urlSecret.name with an explicitly empty key (key: ""), the helper renders an empty secretKeyRef.key, producing an invalid Deployment. Treat empty as "url" (same as the "omitted" behavior).
{{- if and .Values.ap.transport (dig "urlSecret" "url" "" $ts) -}}
url
{{- else if and .Values.ap.transport (dig "urlSecret" "name" "" $ts) -}}
{{- dig "urlSecret" "key" "url" $ts -}}
{{- else if .Values.ap.redis.url -}}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+5
to
+8
| {{- $newURL := "" -}} | ||
| {{- if .Values.ap.transport -}}{{- $newURL = dig "urlSecret" "url" "" (.Values.ap.transportConfig | default dict) -}}{{- end -}} | ||
| {{- $url := $newURL | default .Values.ap.redis.url -}} | ||
| {{- if $url }} |
Comment on lines
+154
to
158
| {{- define "llm-d-async.usingDeprecatedRedisConn" -}} | ||
| {{- if or .Values.ap.redis.url .Values.ap.redis.secretName -}} | ||
| true | ||
| {{- end -}} | ||
| {{- $out | toJson -}} | ||
| {{- end }} |
jtechapps
previously approved these changes
Aug 4, 2026
Signed-off-by: Edoardo Vacchi <evacchi@users.noreply.github.com>
Signed-off-by: Edoardo Vacchi <evacchi@users.noreply.github.com>
Signed-off-by: Edoardo Vacchi <evacchi@users.noreply.github.com>
Signed-off-by: Edoardo Vacchi <evacchi@users.noreply.github.com>
Signed-off-by: Edoardo Vacchi <evacchi@users.noreply.github.com>
evacchi
force-pushed
the
refactor-config-helm
branch
from
August 10, 2026 09:06
a2ec514 to
fd8f4be
Compare
Collaborator
Author
|
rebased/force-pushed |
jtechapps
approved these changes
Aug 10, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Updates helm charts and docs to follow the config changes in #378. Stacked, pending merge there, and also cross-testing on llm-d-batch-gateway. Relevant commit is just 2b579b0
Why is this change needed?
#378 harmonizes configuration style for llm-d-async, this propagates the change to the Helm charts. The old Helm format still works, and also the old flags work for the time being. In v+2 we will remove the deprecated formats.
How was this tested?
Checklist
git commit -s) per DCOmake test)make lint)Related Issues
Follow up to #378, closes #284
Release note