Skip to content

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

Open
AndrewChubatiuk wants to merge 1 commit intomasterfrom
fix-extra-storage-node-when-no-storage
Open

vlcluster,vtcluster: do not ignore extraStorageNode, when default storage is not enabled#1911
AndrewChubatiuk wants to merge 1 commit intomasterfrom
fix-extra-storage-node-when-no-storage

Conversation

@AndrewChubatiuk
Copy link
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 adds validation to prevent duplicate storageNode addresses. Addresses issue #1910.

  • Bug Fixes
    • Build the -storageNode flag and collect storage node IDs outside the default-storage check.
    • Append -storageNode args only when at least one node is present; include ExtraStorageNodes even without default storage.
    • Add CR validation to reject duplicate storageNode addresses from ExtraArgs and ExtraStorageNodes.

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

Copy link
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
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
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
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
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