Skip to content

Commit bf65d99

Browse files
committed
Address review comments: add documentation and TODOs
- Add documentation for CONFIG_MAP_NAME env var in prometheus.go explaining how it's set by Helm chart for multi-instance support - Add TODO for scaleTargetRef.name CRD requirement in variant.go noting the check should be enforced at schema level Related issues: - llm-d#454: Consider hardware type when inferring target deployment - llm-d#455: Replace pod filtering with app-level readiness probes
1 parent 42dd954 commit bf65d99

File tree

2 files changed

+9
-1
lines changed

2 files changed

+9
-1
lines changed

internal/config/prometheus.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,12 @@ const (
2020
DefaultConfigMapName = "workload-variant-autoscaler-variantautoscaling-config"
2121
)
2222

23-
// GetConfigMapName returns the ConfigMap name from environment variable or default
23+
// GetConfigMapName returns the ConfigMap name from environment variable or default.
24+
// The CONFIG_MAP_NAME environment variable is set by the Helm chart during deployment
25+
// (see charts/workload-variant-autoscaler/templates/manager/wva-deployment-controller-manager.yaml).
26+
// Each WVA deployment gets its own uniquely-named ConfigMap based on the Helm release name,
27+
// allowing multiple WVA instances to coexist in the same cluster without conflicts.
28+
// The Helm template sets this to: {{ include "workload-variant-autoscaler.fullname" . }}-variantautoscaling-config
2429
func GetConfigMapName() string {
2530
if name := os.Getenv("CONFIG_MAP_NAME"); name != "" {
2631
return name

internal/utils/variant.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,9 @@ func filterVariantsByDeployment(ctx context.Context, client client.Client, filte
101101
}
102102

103103
// Skip VAs without scaleTargetRef (required to know which deployment to look up)
104+
// TODO: Remove this check once scaleTargetRef.name is made a required field in the CRD.
105+
// This defensive check exists because the CRD currently allows empty scaleTargetRef,
106+
// but it should be enforced at the schema level instead.
104107
if va.Spec.ScaleTargetRef.Name == "" {
105108
ctrl.LoggerFrom(ctx).V(logging.DEBUG).Info("Skipping VA without scaleTargetRef", "namespace", va.Namespace, "name", va.Name)
106109
continue

0 commit comments

Comments
 (0)