Skip to content

vlcluster,vtcluster: do not ignore extraStorageNode, when default storage is not enabled#1911

Merged
vrutkovs merged 3 commits intomasterfrom
fix-extra-storage-node-when-no-storage
Mar 30, 2026
Merged

vlcluster,vtcluster: do not ignore extraStorageNode, when default storage is not enabled#1911
vrutkovs merged 3 commits intomasterfrom
fix-extra-storage-node-when-no-storage

Conversation

@AndrewChubatiuk
Copy link
Copy Markdown
Contributor

@AndrewChubatiuk AndrewChubatiuk commented Mar 2, 2026

fixes #1910


Summary by cubic

Fixes VLSelect and VTSelect ignoring ExtraStorageNodes when default storage is disabled, so select pods connect to the provided storage nodes. Also validates and rejects duplicate storageNode addresses across defaults and extras (fixes #1910).

  • Bug Fixes
    • Build -storageNode flag and collect node IDs outside the default-storage check.
    • Append -storageNode only when nodes exist; always include ExtraStorageNodes even without default storage.
    • Validate uniqueness of storageNode addresses from default storage (when enabled), ExtraArgs["storageNode"], and ExtraStorageNodes.

Written for commit de3e677. Summary will update on new commits.

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.

No issues found across 3 files

Copy link
Copy Markdown
Member

@Haleygo Haleygo left a comment

Choose a reason for hiding this comment

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

Nit: should we forbid using extraStorageNodes and extraArgs.storageNode together? A user might accidentally set both and get duplicate settings.

}
if len(cr.Spec.VLSelect.ExtraStorageNodes) > 0 {
for i, node := range cr.Spec.VLSelect.ExtraStorageNodes {
idx := i + len(storageNodeIds)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Perhaps we should deduplicate the list? Users may accidentally set the same node twice.

Also, needs tests

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

throwing error in case of duplicates instead since also per storage auth can be added and deduplication can break an order

@AndrewChubatiuk AndrewChubatiuk force-pushed the fix-extra-storage-node-when-no-storage branch from 281ae15 to f0dfa99 Compare March 3, 2026 08:15
Signed-off-by: Vadim Rutkovsky <vadim@vrutkovs.eu>
Signed-off-by: Vadim Rutkovsky <vadim@vrutkovs.eu>
@vrutkovs vrutkovs merged commit bc5f74d into master Mar 30, 2026
7 checks passed
@vrutkovs vrutkovs deleted the fix-extra-storage-node-when-no-storage branch March 30, 2026 08:04
AndrewChubatiuk added a commit that referenced this pull request Mar 31, 2026
…rage is not enabled (#1911)

Co-authored-by: Vadim Rutkovsky <vadim@vrutkovs.eu>
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.

VLCluster: extraStorageNodes silently ignored without in-cluster vlstorage

3 participants