vmcluster: support storage node pools

[AndrewChubatiuk]

fixes #741

Adds pools to VMCluster, allowing to run multiple isolated groups of vmstorage nodes (and optionally dedicated vminsert nodes) within a single cluster. Each pool is queried by vmselect as a named storage group, enabling multi-retention.

Each entry in spec.pools has a name, an optional vmstorage override (merges onto the top-level spec.vmstorage), and an optional vminsert override (merges onto the top-level spec.vminsert).
When any pool is defined, the top-level vmstorage StatefulSet is not deployed; pools replace it entirely.
The top-level vminsert Deployment is deployed as a shared insert only when no pool defines its own dedicated vminsert.
vmstorage.retentionPeriod inside a pool overrides the cluster-level spec.retentionPeriod for that pool, enabling per-pool retention.

Pool resources are named vm<component>-<cluster>-<pool> (e.g. vmstorage-prod-hot).
An app.kubernetes.io/pool=<name> label is added to both the StatefulSet/Deployment selector and pod template labels, keeping per-pool selectors disjoint.

[cubic-dev-ai]

3 issues found across 9 files

<sub>Reply with feedback, questions, or to request a fix.

Fix all with cubic | Re-trigger cubic</sub>

[cubic-dev-ai]

1 issue found across 10 files

<sub>Reply with feedback, questions, or to request a fix.

Fix all with cubic | Re-trigger cubic</sub>

[cubic-dev-ai]

2 issues found across 17 files

<sub>Reply with feedback, questions, or to request a fix.

Fix all with cubic | Re-trigger cubic</sub>

[cubic-dev-ai]

No issues found across 17 files

_{Re-trigger cubic}

[AndrewChubatiuk]

@vrutkovs need your feedback on this

[vrutkovs]

I'd like to have this merged after release is cut, but in general it looks okay

[AndrewChubatiuk]

sure, no rush, just wanted to get some feedback

[cubic-dev-ai]

2 issues found across 33 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/v1beta1/vmextra_types.go">

<violation number="1" location="api/operator/v1beta1/vmextra_types.go:1910">
P2: When a pool explicitly disables an inherited scalar option such as `hostNetwork: false` or sets `minReadySeconds: 0`, `MergeDeep` drops that override during JSON marshaling because these fields use `omitempty`, so the pool continues using the top-level value. Preserve field presence when merging (or model overrideable scalar fields as pointers) so explicit zero values override inherited defaults.</violation>
</file>

<file name="api/operator/v1beta1/vmcluster_types.go">

<violation number="1" location="api/operator/v1beta1/vmcluster_types.go:1454">
P1: Pool-only VMStorage or VMInsert specs bypass `addVMClusterDefaults`, which initializes ports, images, and common parameters only for top-level fields. A pool without a corresponding top-level component can therefore generate empty ports and unset runtime defaults; default each resolved pool view.</violation>
</file>

<sub>Tip: instead of fixing issues one by one fix them all with cubic
Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic</sub>

[cubic-dev-ai]

1 existing issue remains and 5 new issues found across 33 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/v1beta1/vmalert_types.go">

<violation number="1" location="api/operator/v1beta1/vmalert_types.go:522">
P2: Changing `VMAlert.AsURL` to return an error breaks source compatibility for existing Go callers. Preserve the one-result API and use a separate error-returning adapter or method for VMAuth's pool-aware URL resolution.</violation>
</file>

<file name="api/operator/v1beta1/vlogs_types.go">

<violation number="1" location="api/operator/v1beta1/vlogs_types.go:302">
P2: Changing the exported `VLogs.AsURL` signature breaks downstream Go callers at compile time, while this deprecated resource has no internal caller requiring the error return. Preserve the existing `string` signature, or add a separately named method if an error-returning API is needed.</violation>
</file>

<file name="internal/controller/operator/factory/vmcluster/vmcluster_pools.go">

<violation number="1" location="internal/controller/operator/factory/vmcluster/vmcluster_pools.go:33">
P1: When pools are added to an existing cluster while top-level component specs remain set, this branch stops reconciling the shared resources but cleanup still preserves them. Base vmstorage and vminsert can continue running and receiving traffic; make orphan cleanup use the effective pool mode before keeping top-level names.</violation>
</file>

<file name="api/operator/v1beta1/vmcluster_types.go">

<violation number="1" location="api/operator/v1beta1/vmcluster_types.go:1452">
P2: These annotations make pool component specs preserve unknown fields instead of using their typed schemas. The CRD consequently omits admission validation for fields such as `retentionPeriod`, enum log settings, and nested HPA settings; remove the schemaless annotations.</violation>
</file>

<file name="api/operator/v1beta1/vmsingle_types.go">

<violation number="1" location="api/operator/v1beta1/vmsingle_types.go:388">
P3: VMSingle.AsURL always returns a nil error, so the `if err != nil { return "" }` branch in GetRemoteWriteURL is unreachable dead code. Same pattern was added in VLSingle.GetRemoteWriteURL. Drop the unreachable guard.</violation>
</file>

<sub>Requires human review: Auto-approval blocked because this review re-detected 7 unresolved issues already reported by Cubic.
Tip: instead of fixing issues one by one fix them all with cubic
Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic</sub>

[cubic-dev-ai]

2 existing issues remain and no new issues found across 27 files (changes from recent commits).

<sub>Requires human review: Auto-approval blocked because this review re-detected 4 unresolved issues already reported by Cubic.
Tip: Review your code locally with the cubic CLI to iterate faster.

Fix all with cubic | Re-trigger cubic</sub>

[cubic-dev-ai]

1 issue found across 27 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="internal/controller/operator/factory/vmauth/vmusers_config_test.go">

<violation number="1" location="internal/controller/operator/factory/vmauth/vmusers_config_test.go:3369">
P2: This new test case omits the objURLs field, so genUserCfg is called with a nil map and genURLMaps returns "cannot find crdRef target" for the pooled key, making assert.NoError fail. Add an objURLs entry mapping "VMCluster/vminsert/default/pooled/hot" to the expected URL.</violation>
</file>

<sub>Tip: Review your code locally with the cubic CLI to iterate faster.

Fix all with cubic | Re-trigger cubic</sub>

[cubic-dev-ai]

1 existing issue remains and no new issues found across 41 files (changes from recent commits).

<sub>Requires human review: Auto-approval blocked because this review re-detected 3 unresolved issues already reported by Cubic.
Tip: Review your code locally with the cubic CLI to iterate faster.

Fix all with cubic | Re-trigger cubic</sub>

[cubic-dev-ai]

2 issues found across 41 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="internal/controller/operator/factory/vmcluster/vmcluster_pools.go">

<violation number="1" location="internal/controller/operator/factory/vmcluster/vmcluster_pools.go:148">
P3: `storageNodeIDs` duplicates the existing `availableStorageNodeIDsFor` implementation, so future routing changes can diverge between validation and generated arguments. Reuse a shared helper instead of maintaining this second copy, and preserve the unsupported-kind guard.</violation>
</file>

<file name="internal/controller/operator/factory/vmauth/vmusers_config_test.go">

<violation number="1" location="internal/controller/operator/factory/vmauth/vmusers_config_test.go:3362">
P3: This test case appears twice in the file: the 'VMUser targeting a specific pool's vminsert' block (cluster 'pooled', pools hot/cold, pool 'hot' target) is present as unchanged context and is also inserted again in this same diff. Remove the duplicate so the redundant identical case isn't run twice.</violation>
</file>

<sub>Tip: instead of fixing issues one by one fix them all with cubic
Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic</sub>

[cubic-dev-ai]

1 existing issue remains and no new issues found across 41 files (changes from recent commits).

<sub>Requires human review: Auto-approval blocked because this review re-detected 1 unresolved issue already reported by Cubic.
Tip: Review your code locally with the cubic CLI to iterate faster.

Fix all with cubic | Re-trigger cubic</sub>

[cubic-dev-ai]

2 existing issues remain and 1 new issue found across 41 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="internal/controller/operator/factory/vmcluster/vmcluster_pools.go">

<violation number="1" location="internal/controller/operator/factory/vmcluster/vmcluster_pools.go:47">
P1: When a pool gains a dedicated vminsert, `prevView` is non-nil but its `Spec.VMInsert` is nil, so configuring a pool PDB or HPA makes reconciliation panic while the helper dereferences the missing previous component. Pass a nil previous view for each absent component, or add component-specific previous views before calling these helpers.</violation>
</file>

<sub>Requires human review: Auto-approval blocked because this review re-detected 2 unresolved issues already reported by Cubic.
Tip: Review your code locally with the cubic CLI to iterate faster.

Fix all with cubic | Re-trigger cubic</sub>

[cubic-dev-ai]

2 existing issues remain and 1 new issue found across 41 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/v1alpha1/vmdistributed_types.go">

<violation number="1" location="api/operator/v1alpha1/vmdistributed_types.go:188">
P2: When a `VMDistributed` uses the new `spec.pools` field in an inline VMCluster spec, strict parsing rejects the CR because this type excludes `Pools`. Keep the full VMCluster spec here and merge the pool list explicitly so distributed clusters can use storage pools too.</violation>
</file>

<sub>Requires human review: Auto-approval blocked because this review re-detected 2 unresolved issues already reported by Cubic.
Tip: Review your code locally with the cubic CLI to iterate faster.

Fix all with cubic | Re-trigger cubic</sub>

[cubic-dev-ai]

2 existing issues remain and 1 new issue found across 41 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="internal/controller/operator/factory/vmcluster/vmcluster.go">

<violation number="1" location="internal/controller/operator/factory/vmcluster/vmcluster.go:891">
P1: When two or more pools use the shared vminsert, this loop emits multiple `-storageNode` flags instead of one combined value. Aggregate all pool addresses into a single comma-separated `-storageNode` flag so vminsert does not drop earlier pools.</violation>
</file>

<sub>Requires human review: Auto-approval blocked because this review re-detected 2 unresolved issues already reported by Cubic.
Tip: Review your code locally with the cubic CLI to iterate faster.

Fix all with cubic | Re-trigger cubic</sub>

[cubic-dev-ai]

2 existing issues remain and no new issues found across 41 files (changes from recent commits).

<sub>Requires human review: Auto-approval blocked because this review re-detected 2 unresolved issues already reported by Cubic.
Tip: Review your code locally with the cubic CLI to iterate faster.

Fix all with cubic | Re-trigger cubic</sub>

[cubic-dev-ai]

1 existing issue remains and 2 new issues found across 41 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="internal/controller/operator/factory/vmcluster/vmcluster.go">

<violation number="1" location="internal/controller/operator/factory/vmcluster/vmcluster.go:1698">
P1: When multiple pools define dedicated vminserts and request load balancing is enabled, vmauth sends all insert traffic through one shared internal Service, while each pool reconcile overwrites that Service's selector; only one pool receives traffic. Generate pool-specific backends and services for the load balancer, or reject this unsupported combination explicitly.</violation>
</file>

<file name="internal/controller/operator/factory/vmdistributed/zone.go">

<violation number="1" location="internal/controller/operator/factory/vmdistributed/zone.go:227">
P2: This line silently carries forward the live cluster's pools (prevClusterSpec.Pools) instead of using whatever the merged spec produced, which is easy to mistake for a bug. The zone spec is a VMClusterSpecBase (see VMDistributedZoneCommon/ZoneCommon VMCluster.Spec and the DeepEqual against vmv1beta1.VMClusterSpecBase{} in vmdistributed_types.go), so it cannot express Pools at all — pools can only be created by manually editing the operator-managed VMCluster after first reconcile, and this line is what keeps them from being wiped on every reconcile. Add a comment explaining this preservation intent, otherwise a future cleanup that reverts it to `vmCluster.Spec = vmv1beta1.VMClusterSpec{VMClusterSpecBase: *vmClusterSpec}` (or trims the field) will silently delete every user-configured pool on each zone reconcile. Consider also documenting that VMDistributed cannot manage pools through its CR spec.</violation>
</file>

<sub>Requires human review: Auto-approval blocked because this review re-detected 1 unresolved issue already reported by Cubic.
Tip: Review your code locally with the cubic CLI to iterate faster.

Fix all with cubic | Re-trigger cubic</sub>

[cubic-dev-ai]

2 existing issues remain and no new issues found across 42 files (changes from recent commits).

<sub>Requires human review: Auto-approval blocked because this review re-detected 2 unresolved issues already reported by Cubic.
Tip: Review your code locally with the cubic CLI to iterate faster.

Fix all with cubic | Re-trigger cubic</sub>

[cubic-dev-ai]

3 existing issues remain and 1 new issue found across 42 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="test/e2e/vmcluster_test.go">

<violation number="1" location="test/e2e/vmcluster_test.go:497">
P2: With pools defined, all vmstorage nodes are registered under named groups (pool.Name) and none under the default/global group, so a vmselect query to `/select/0/prometheus/...` targets an empty storage group and will not return the inserted metric. Additionally the select assertion only checks that the HTTP request returns 200, not that the metric is actually present, so a broken pool read path would not be caught. Query the pool group explicitly (e.g. `/select/0/hot/...`) and assert the response body contains the metric name to genuinely validate the pool round-trip.</violation>
</file>

<sub>Requires human review: Auto-approval blocked because this review re-detected 3 unresolved issues already reported by Cubic.
Tip: Review your code locally with the cubic CLI to iterate faster.

Fix all with cubic | Re-trigger cubic</sub>

[cubic-dev-ai]

2 existing issues remain and no new issues found across 43 files (changes from recent commits).

<sub>Requires human review: Auto-approval blocked because this review re-detected 2 unresolved issues already reported by Cubic.
Tip: Review your code locally with the cubic CLI to iterate faster.

Fix all with cubic | Re-trigger cubic</sub>

[cubic-dev-ai]

2 existing issues remain and 1 new issue found across 43 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/vtcluster_types.go">

<violation number="1" location="api/operator/v1/vtcluster_types.go:803">
P2: Changing exported `VTCluster.AsURL` from `string` to `(string, error)` breaks source compatibility for Go callers outside this repository. Preserve the existing method signature and add a separate error-returning helper for the vmauth adapter.</violation>
</file>

<sub>Requires human review: Auto-approval blocked because this review re-detected 2 unresolved issues already reported by Cubic.
Tip: Review your code locally with the cubic CLI to iterate faster.

Fix all with cubic | Re-trigger cubic</sub>