✨ Document and test client-only install pattern (controller.enabled=false) - #747
Conversation
GPU Pre-flight Check ✅GPUs are available for e2e-openshift tests. Proceeding with deployment.
|
There was a problem hiding this comment.
Pull request overview
This PR documents and tests the controller.enabled=false workflow for adding models to an existing WVA controller without reinstalling the full stack. It addresses asks 2 and 3 from issue #746 by providing clear documentation and comprehensive Helm template tests.
Changes:
- Adds "Adding Models to an Existing Controller" section to the multi-controller isolation guide with step-by-step instructions and resource breakdowns
- Introduces 4 Helm template tests validating client-only install behavior, full install resources, minimal installs, and controllerInstance label propagation
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| docs/user-guide/multi-controller-isolation.md | Documents controller-only install pattern with three-step workflow (install controller, add model A, add model B) and lists resources included/excluded in each mode |
| test/chart/client_only_install_test.go | Adds 4 test cases using helm template to verify resource generation for client-only vs full installs and controllerInstance label behavior |
| --create-namespace \ | ||
| --set controller.enabled=true \ | ||
| --set va.enabled=false \ | ||
| --set hpa.enabled=false |
There was a problem hiding this comment.
The controller-only install example should explicitly set vllmService.enabled=false to avoid creating unnecessary vLLM Service and ServiceMonitor resources with default model names. Since this step intends to install only the controller infrastructure without any workload-specific resources, the vLLM monitoring resources should also be disabled.
| --set hpa.enabled=false | |
| --set hpa.enabled=false \ | |
| --set vllmService.enabled=false |
| @@ -0,0 +1,146 @@ | |||
| package chart_test | |||
There was a problem hiding this comment.
Missing Apache 2.0 license header. All Go files in this repository include a copyright and license header. Add the standard Apache 2.0 license header with "Copyright 2025." at the top of the file, consistent with other test files in the codebase.
There was a problem hiding this comment.
bad AI. It's not missing, and you missed the fact that we are in 2026
GPU Pre-flight Check ✅GPUs are available for e2e-openshift tests. Proceeding with deployment.
|
GPU Pre-flight Check ✅GPUs are available for e2e-openshift tests. Proceeding with deployment.
|
| {"leader-election", "leader election RBAC should be excluded"}, | ||
| {"controller-manager", "controller manager resources should be excluded"}, | ||
| {"prometheus-ca", "prometheus CA configmaps should be excluded"}, | ||
| {"wva-deployment", "controller deployment template should be excluded"}, |
There was a problem hiding this comment.
The marker "wva-deployment" doesn't appear anywhere in the Helm templates and therefore isn't a useful check. The deployment is already caught by the "controller-manager" marker on line 82, which appears in the deployment name, labels, and associated resources. Consider removing this redundant check or replacing it with a more specific marker if there's a particular resource you want to verify is excluded.
| {"wva-deployment", "controller deployment template should be excluded"}, |
|
👋 @asm582 — all CI checks green. Could you approve when you get a chance? Thanks! |
GPU Pre-flight Check ✅GPUs are available for e2e-openshift tests. Proceeding with deployment.
|
|
👋 @asm582 — Copilot comments addressed, all CI checks green (3/3 on e2e-openshift). Ready for your approval when you get a chance. Thanks! |
|
/unhold |
The merge-base changed after approval.
4187af5 to
4a92361
Compare
GPU Pre-flight Check ✅GPUs are available for e2e-openshift tests. Proceeding with deployment.
|
| fi | ||
| fi | ||
|
|
||
There was a problem hiding this comment.
please remove extra blank space, everywhere in this file
…alse) Add multi-controller isolation docs and Helm template tests for client-only install workflow. Signed-off-by: Andy Anderson <andy@clubanderson.com> Signed-off-by: Andrew Anderson <andy@clubanderson.com>
4a92361 to
6ad2cb5
Compare
GPU Pre-flight Check ❌Insufficient GPUs to run e2e-openshift tests. Re-run with
|
Pull request was closed
GPU Pre-flight Check ❌Insufficient GPUs to run e2e-openshift tests. Re-run with
|
GPU Pre-flight Check ❌Insufficient GPUs to run e2e-openshift tests. Re-run with
|
GPU Pre-flight Check ❌Insufficient GPUs to run e2e-openshift tests. Re-run with
|
GPU Pre-flight Check ❌Insufficient GPUs to run e2e-openshift tests. Re-run with
|
GPU Pre-flight Check ✅GPUs are available for e2e-openshift tests. Proceeding with deployment.
|
GPU Pre-flight Check ✅GPUs are available for e2e-openshift tests. Proceeding with deployment.
|
GPU Pre-flight Check ✅GPUs are available for e2e-openshift tests. Proceeding with deployment.
|
…alse) (llm-d#747) Add multi-controller isolation docs and Helm template tests for client-only install workflow. Signed-off-by: Andy Anderson <andy@clubanderson.com> Signed-off-by: Andrew Anderson <andy@clubanderson.com>
Summary
controller.enabled=falseworkflow for adding models to an existing WVA controllerFixes #746 (asks 2 and 3). Ask 1 (ConfigMap collision fix) is in #745.
Documentation (ask #2)
Adds a new "Adding Models to an Existing Controller" section to the multi-controller isolation guide with:
controller.enabled=falsecontrollerInstancealignment between controller and model installsTests (ask #3)
Adds
test/chart/client_only_install_test.gowith 4 test cases usinghelm template:TestClientOnlyInstallcontroller.enabled=falseproduces VA, HPA, Service, ServiceMonitor but excludes controller Deployment, ServiceAccount, leader election RBAC, ConfigMapsTestFullInstallcontroller.enabled=trueincludes all controller infrastructureTestClientOnlyNoVATestClientOnlyControllerInstancecontrollerInstancelabel propagates to VA labels and HPA metric selectors in client-only modeRun locally:
Test plan