Conversation
autoscaling values per workload (main + fusillade single/split roles), replicas omitted when the HPA owns the count, and per-deployment DWCTL_DATABASE__REPLICA_GROUP so pods join the right connection-budget group. Keystore excluded: single-writer redis cannot scale horizontally. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
There was a problem hiding this comment.
🟡 Changes recommended
The new DWCTL_DATABASE__REPLICA_GROUP injection can unintentionally override the same env var when users set it via the Secret values layer (secrets.controlLayer.data), preventing secure/consistent configuration through envFrom.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds optional Horizontal Pod Autoscaler (autoscaling/v2) support for the control-layer Deployment and fusillade daemon Deployments (single + split roles), while preventing Argo CD from continuously reverting HPA-driven scaling by omitting spec.replicas when autoscaling is enabled. Also injects DWCTL_DATABASE__REPLICA_GROUP per workload and extends chart docs/tests accordingly.
Changes:
- Introduces
autoscalingvalues blocks and renders new HPA templates for control-layer and fusillade (including per-role overrides in split mode). - Omits
spec.replicasfrom autoscaled Deployments to avoid Argo CD reconciling away HPA scale events. - Injects
DWCTL_DATABASE__REPLICA_GROUPinto workloads (with “no duplicate” guards) and adds helm-unittest coverage + README updates.
File summaries
| File | Description |
|---|---|
| values.yaml | Adds autoscaling configuration blocks and documents replicas omission behavior + replica group env injection. |
| templates/hpa.yaml | New control-layer HPA template gated by autoscaling.enabled. |
| templates/fusillade/hpa.yaml | New fusillade HPA template supporting single/split mode and per-role autoscaling overlay. |
| templates/deployment.yaml | Omits spec.replicas when autoscaling is enabled; injects DWCTL_DATABASE__REPLICA_GROUP. |
| templates/fusillade/deployment.yaml | Omits spec.replicas per fusillade workload when autoscaling is enabled; injects per-workload replica group env. |
| tests/hpa_test.yaml | New unit tests validating HPA rendering and replicas omission behavior across modes/roles. |
| tests/fusillade_deployment_test.yaml | Adds tests for replica-group env injection/override behavior and split-mode replicas behavior. |
| tests/control_layer_deployment_test.yaml | Adds tests for control-layer replicas omission and replica-group env injection/override. |
| tests/configmap_test.yaml | Fixes assertion to ensure DASHBOARD_BOOTSTRAP_JS env var is absent by default. |
| README.md | Documents new autoscaling configuration and fusillade autoscaling overrides. |
Review details
- Files reviewed: 10/10 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+125
to
+128
| {{- if not (hasKey (.Values.env | default dict) "DWCTL_DATABASE__REPLICA_GROUP") }} | ||
| - name: DWCTL_DATABASE__REPLICA_GROUP | ||
| value: "control-layer" | ||
| {{- end }} |
Comment on lines
+116
to
+119
| {{- if not (hasKey $userEnv "DWCTL_DATABASE__REPLICA_GROUP") }} | ||
| - name: DWCTL_DATABASE__REPLICA_GROUP | ||
| value: {{ $component | quote }} | ||
| {{- end }} |
Comment on lines
+60
to
+61
| # This will set the replicaset count. Ignored (and omitted from the rendered | ||
| # Deployment) when autoscaling.enabled is true so the HPA owns the count. |
The control-layer side moved from a runtime-divided connection budget to pooled + direct pools with fixed direct sizes, so pods no longer need a replica group. Injecting DWCTL_DATABASE__REPLICA_GROUP would crash images that do not know the key (deny_unknown_fields). Default render is again byte-identical to 1.6.0; HPA support is unchanged. helm unittest: 128 passed, helm lint clean. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
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
autoscalingvalues block for the main deployment andfusillade.autoscalingfor the daemons, with optional per-role overrides atfusillade.split.{request,batch}.autoscaling(merged over the fusillade default, same pattern as per-rolereplicaCount).metricsandbehaviorare rawautoscaling/v2lists rendered verbatim.templates/hpa.yamlandtemplates/fusillade/hpa.yaml(one HPA per fusillade workload whose effective autoscaling is enabled; mirrors the deployment template's single/split iteration exactly).spec.replicasis omitted from a Deployment whose HPA is enabled: Argo CD reconciles continuously, so a rendered replicas field would revert every scale-up on the next sync.Compatibility
Default render is byte-identical to 1.6.0. No HPA is rendered unless enabled. (An earlier revision injected a
DWCTL_DATABASE__REPLICA_GROUPenv var for a runtime connection-budget scheme; that design was replaced by pooled + direct connection pools on the control-layer side, so the injection is gone.)Tests
helm unittest .: 128 passed.helm lintclean. README values table updated.Noticed in passing (pre-existing, not changed here): the fusillade template renders duplicate
envnames when the same key is set in more than one of the global/fusillade/per-role env layers.🤖 Generated with Claude Code