Skip to content

[vm|vl|vt]cluster: properly generate storageNode command line flags when HPA enabled on a storage#2119

Open
AndrewChubatiuk wants to merge 3 commits intomasterfrom
vl-hpa-fix
Open

[vm|vl|vt]cluster: properly generate storageNode command line flags when HPA enabled on a storage#2119
AndrewChubatiuk wants to merge 3 commits intomasterfrom
vl-hpa-fix

Conversation

@AndrewChubatiuk
Copy link
Copy Markdown
Contributor

@AndrewChubatiuk AndrewChubatiuk commented Apr 29, 2026

fixes #2117


Summary by cubic

Fixes incorrect -storageNode flag generation when storage is scaled by HPA in vmcluster, vlcluster, and vtcluster. Insert/select now compute storage nodes from the live replica count so routing stays correct during scaling. See #2117.

  • Bug Fixes
    • When HPA is enabled, copy current StatefulSet/Deployment replicas into Spec.*.ReplicaCount (storage, select, insert).
    • Switch AvailableStorageNodeIDs to a typed component and derive node count from ReplicaCount or HPA.MinReplicas (default 1), respecting maintenance filters.
    • Always build -storageNode flags for insert/select from the computed storage node IDs.

Written for commit 8f1c937. Summary will update on new commits. Review in cubic

Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

3 issues found across 11 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="api/operator/v1/vlcluster_types.go">

<violation number="1" location="api/operator/v1/vlcluster_types.go:792">
P1: Handle HPA configs with omitted `minReplicas`; otherwise `AvailableStorageNodeIDs` returns no nodes and generates empty storage targets.</violation>
</file>

<file name="api/operator/v1/vtcluster_types.go">

<violation number="1" location="api/operator/v1/vtcluster_types.go:703">
P1: When HPA is set but `minReplicas` is omitted, `replicaCount` remains 0 and no storage nodes are generated. Default this path to 1 replica (Kubernetes HPA default) so `-storageNode` flags are still built.</violation>
</file>

<file name="docs/CHANGELOG.md">

<violation number="1" location="docs/CHANGELOG.md:21">
P2: Custom agent: **Changelog Review Agent**

Changelog entry uses implementation-detail wording and misses explicit before/after user-visible explanation required by the changelog rule.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.

Comment thread api/operator/v1/vlcluster_types.go Outdated
Comment thread api/operator/v1/vtcluster_types.go Outdated
Comment thread docs/CHANGELOG.md Outdated
Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com>
Signed-off-by: Andrii Chubatiuk <andrew.chubatiuk@gmail.com>
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 3 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="api/operator/v1/vlcluster_types.go">

<violation number="1" location="api/operator/v1/vlcluster_types.go:796">
P2: Avoid local defaulting of optional HPA replica fields here; set this default in centralized CR defaulting instead so all controllers/use-sites observe the same value.

(Based on your team's feedback about relying on centralized CR defaulting for optional pointer fields.) [FEEDBACK_USED]</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.

Comment thread api/operator/v1/vlcluster_types.go Outdated
if cr.Spec.VLStorage.HPA.MinReplicas != nil {
replicaCount = *cr.Spec.VLStorage.HPA.MinReplicas
} else {
replicaCount = 1
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot Apr 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Avoid local defaulting of optional HPA replica fields here; set this default in centralized CR defaulting instead so all controllers/use-sites observe the same value.

(Based on your team's feedback about relying on centralized CR defaulting for optional pointer fields.)

View Feedback

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At api/operator/v1/vlcluster_types.go, line 796:

<comment>Avoid local defaulting of optional HPA replica fields here; set this default in centralized CR defaulting instead so all controllers/use-sites observe the same value.

(Based on your team's feedback about relying on centralized CR defaulting for optional pointer fields.) </comment>

<file context>
@@ -789,8 +789,12 @@ func (cr *VLCluster) AvailableStorageNodeIDs(kind vmv1beta1.ClusterComponent) []
+		if cr.Spec.VLStorage.HPA.MinReplicas != nil {
+			replicaCount = *cr.Spec.VLStorage.HPA.MinReplicas
+		} else {
+			replicaCount = 1
+		}
 	}
</file context>
Fix with Cubic

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: VLCluster deploys in single mode

2 participants