Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
AndrewChubatiuk wants to merge 1 commit into
base: master
Choose a base branch
from
Open

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 36 additions & 17 deletions api/operator/v1/vlcluster_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"k8s.io/apimachinery/pkg/api/equality"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/labels"
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/utils/ptr"

vmv1beta1 "github.com/VictoriaMetrics/operator/api/operator/v1beta1"
Expand Down Expand Up @@ -686,23 +687,6 @@ func (cr *VLCluster) Validate() error {
if vmv1beta1.MustSkipCRValidation(cr) {
return nil
}
if cr.Spec.VLSelect != nil {
vms := cr.Spec.VLSelect
name := cr.PrefixedName(vmv1beta1.ClusterComponentSelect)
if vms.ServiceSpec != nil && vms.ServiceSpec.Name == name {
return fmt.Errorf(".serviceSpec.Name cannot be equal to prefixed name=%q", name)
}
if vms.HPA != nil {
if err := vms.HPA.Validate(); err != nil {
return err
}
}
if vms.VPA != nil {
if err := vms.VPA.Validate(); err != nil {
return err
}
}
}
if cr.Spec.VLInsert != nil {
vli := cr.Spec.VLInsert
name := cr.PrefixedName(vmv1beta1.ClusterComponentInsert)
Expand All @@ -720,6 +704,7 @@ func (cr *VLCluster) Validate() error {
}
}
}
storageNodes := sets.New[string]()
if cr.Spec.VLStorage != nil {
vls := cr.Spec.VLStorage
name := cr.PrefixedName(vmv1beta1.ClusterComponentStorage)
Expand All @@ -735,6 +720,40 @@ func (cr *VLCluster) Validate() error {
}
}
}
if cr.Spec.VLSelect != nil {
vms := cr.Spec.VLSelect
name := cr.PrefixedName(vmv1beta1.ClusterComponentSelect)
if vms.ServiceSpec != nil && vms.ServiceSpec.Name == name {
return fmt.Errorf(".serviceSpec.Name cannot be equal to prefixed name=%q", name)
}
if vms.HPA != nil {
if err := vms.HPA.Validate(); err != nil {
return err
}
}
if vms.VPA != nil {
if err := vms.VPA.Validate(); err != nil {
return err
}
}
if nodes, ok := cr.Spec.VLSelect.ExtraArgs["storageNode"]; ok {
for _, node := range strings.Split(nodes, ",") {
node = strings.TrimSpace(node)
if storageNodes.Has(node) {
return fmt.Errorf("encountered storageNode=%s multiple times, please make all storage node addresses are unique", node)
} else {
storageNodes.Insert(node)
}
}
}
for _, node := range cr.Spec.VLSelect.ExtraStorageNodes {
if storageNodes.Has(node.Addr) {
return fmt.Errorf("encountered storageNode=%s multiple times, please make all storage node addresses are unique", node.Addr)
} else {
storageNodes.Insert(node.Addr)
}
}
}
if cr.Spec.RequestsLoadBalancer.Enabled {
rlb := cr.Spec.RequestsLoadBalancer.Spec
name := cr.PrefixedName(vmv1beta1.ClusterComponentBalancer)
Expand Down
54 changes: 37 additions & 17 deletions api/operator/v1/vtcluster_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"k8s.io/apimachinery/pkg/api/equality"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/labels"
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/utils/ptr"

vmv1beta1 "github.com/VictoriaMetrics/operator/api/operator/v1beta1"
Expand Down Expand Up @@ -596,23 +597,6 @@ func (cr *VTCluster) Validate() error {
if vmv1beta1.MustSkipCRValidation(cr) {
return nil
}
if cr.Spec.Select != nil {
vms := cr.Spec.Select
name := cr.PrefixedName(vmv1beta1.ClusterComponentSelect)
if vms.ServiceSpec != nil && vms.ServiceSpec.Name == name {
return fmt.Errorf(".serviceSpec.Name cannot be equal to prefixed name=%q", name)
}
if vms.HPA != nil {
if err := vms.HPA.Validate(); err != nil {
return err
}
}
if vms.VPA != nil {
if err := vms.VPA.Validate(); err != nil {
return err
}
}
}
if cr.Spec.Insert != nil {
vti := cr.Spec.Insert
name := cr.PrefixedName(vmv1beta1.ClusterComponentInsert)
Expand All @@ -630,7 +614,9 @@ func (cr *VTCluster) Validate() error {
}
}
}
storageNodes := sets.New[string]()
if cr.Spec.Storage != nil {
storageNodes.Insert(cr.AsURL(vmv1beta1.ClusterComponentStorage))
vts := cr.Spec.Storage
name := cr.PrefixedName(vmv1beta1.ClusterComponentStorage)
if vts.ServiceSpec != nil && vts.ServiceSpec.Name == name {
Expand All @@ -645,6 +631,40 @@ func (cr *VTCluster) Validate() error {
}
}
}
if cr.Spec.Select != nil {
vms := cr.Spec.Select
name := cr.PrefixedName(vmv1beta1.ClusterComponentSelect)
if vms.ServiceSpec != nil && vms.ServiceSpec.Name == name {
return fmt.Errorf(".serviceSpec.Name cannot be equal to prefixed name=%q", name)
}
if vms.HPA != nil {
if err := vms.HPA.Validate(); err != nil {
return err
}
}
if vms.VPA != nil {
if err := vms.VPA.Validate(); err != nil {
return err
}
}
if nodes, ok := cr.Spec.Select.ExtraArgs["storageNode"]; ok {
for _, node := range strings.Split(nodes, ",") {
node = strings.TrimSpace(node)
if storageNodes.Has(node) {
return fmt.Errorf("encountered storageNode=%s multiple times, please make all storage node addresses are unique", node)
} else {
storageNodes.Insert(node)
}
}
}
for _, node := range cr.Spec.Select.ExtraStorageNodes {
if storageNodes.Has(node.Addr) {
return fmt.Errorf("encountered storageNode=%s multiple times, please make all storage node addresses are unique", node.Addr)
} else {
storageNodes.Insert(node.Addr)
}
}
}
if cr.Spec.RequestsLoadBalancer.Enabled {
rlb := cr.Spec.RequestsLoadBalancer.Spec
name := cr.PrefixedName(vmv1beta1.ClusterComponentBalancer)
Expand Down
2 changes: 1 addition & 1 deletion docs/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ aliases:

* BUGFIX: [vmoperator](https://docs.victoriametrics.com/operator/): VMPodScrape for VLAgent and VMAgent now uses the correct port; previously it used the wrong port and could cause scrape failures. See [#1887](https://github.com/VictoriaMetrics/operator/issues/1887).
* BUGFIX: [vmdistributed](https://docs.victoriametrics.com/operator/resources/vmdistributed/): updated VMAuth config consolidating all VMSelects into a single read and all VMClusters into a single write backend

* BUGFIX: [vmoperator](https://docs.victoriametrics.com/operator/): remove unneeded finalizer from core K8s resources. See [#835](https://github.com/VictoriaMetrics/operator/issues/835).
* BUGFIX: [vlcluster](https://docs.victoriametrics.com/operator/resources/vlcluster/) and [vtcluster](https://docs.victoriametrics.com/operator/resources/vtcluster/): do not ignore ExtraStorageNodes for select, when default storage is disabled. See [#1910](https://github.com/VictoriaMetrics/operator/issues/1910).

## [v0.68.1](https://github.com/VictoriaMetrics/operator/releases/tag/v0.68.1)
**Release date:** 23 February 2026
Expand Down
18 changes: 10 additions & 8 deletions internal/controller/operator/factory/vlcluster/vlselect.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -233,20 +233,22 @@ func buildVLSelectPodSpec(cr *vmv1.VLCluster) (*corev1.PodTemplateSpec, error) {
args = append(args, fmt.Sprintf("-loggerFormat=%s", cr.Spec.VLSelect.LogFormat))
}

storageNodeFlag := build.NewFlag("-storageNode", "")
storageNodeIds := cr.AvailableStorageNodeIDs("select")
if cr.Spec.VLStorage != nil && cr.Spec.VLStorage.ReplicaCount != nil {
// TODO: check TLS
storageNodeFlag := build.NewFlag("-storageNode", "")
storageNodeIds := cr.AvailableStorageNodeIDs("select")
for idx, i := range storageNodeIds {
storageNodeFlag.Add(build.PodDNSAddress(cr.PrefixedName(vmv1beta1.ClusterComponentStorage), i, cr.Namespace, cr.Spec.VLStorage.Port, cr.Spec.ClusterDomainName), idx)
}
if len(cr.Spec.VLSelect.ExtraStorageNodes) > 0 {
for i, node := range cr.Spec.VLSelect.ExtraStorageNodes {
idx := i + len(storageNodeIds)
storageNodeFlag.Add(node.Addr, idx)
}
}
if len(cr.Spec.VLSelect.ExtraStorageNodes) > 0 {
for i, node := range cr.Spec.VLSelect.ExtraStorageNodes {
idx := i + len(storageNodeIds)
Copy link
Collaborator

@vrutkovs vrutkovs Mar 3, 2026

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

@AndrewChubatiuk AndrewChubatiuk Mar 3, 2026

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

storageNodeFlag.Add(node.Addr, idx)
}
totalNodes := len(cr.Spec.VLSelect.ExtraStorageNodes) + len(storageNodeIds)
}
totalNodes := len(cr.Spec.VLSelect.ExtraStorageNodes) + len(storageNodeIds)
if totalNodes > 0 {
args = build.AppendFlagsToArgs(args, totalNodes, storageNodeFlag)
}

Expand Down
18 changes: 10 additions & 8 deletions internal/controller/operator/factory/vtcluster/select.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -232,20 +232,22 @@ func buildVTSelectPodSpec(cr *vmv1.VTCluster) (*corev1.PodTemplateSpec, error) {
args = append(args, fmt.Sprintf("-loggerFormat=%s", cr.Spec.Select.LogFormat))
}

storageNodeFlag := build.NewFlag("-storageNode", "")
storageNodeIds := cr.AvailableStorageNodeIDs("select")
if cr.Spec.Storage != nil && cr.Spec.Storage.ReplicaCount != nil {
// TODO: check TLS
storageNodeFlag := build.NewFlag("-storageNode", "")
storageNodeIds := cr.AvailableStorageNodeIDs("select")
for idx, i := range storageNodeIds {
storageNodeFlag.Add(build.PodDNSAddress(cr.PrefixedName(vmv1beta1.ClusterComponentStorage), i, cr.Namespace, cr.Spec.Storage.Port, cr.Spec.ClusterDomainName), idx)
}
if len(cr.Spec.Select.ExtraStorageNodes) > 0 {
for i, node := range cr.Spec.Select.ExtraStorageNodes {
idx := i + len(storageNodeIds)
storageNodeFlag.Add(node.Addr, idx)
}
}
if len(cr.Spec.Select.ExtraStorageNodes) > 0 {
for i, node := range cr.Spec.Select.ExtraStorageNodes {
idx := i + len(storageNodeIds)
storageNodeFlag.Add(node.Addr, idx)
}
totalNodes := len(cr.Spec.Select.ExtraStorageNodes) + len(storageNodeIds)
}
totalNodes := len(cr.Spec.Select.ExtraStorageNodes) + len(storageNodeIds)
if totalNodes > 0 {
args = build.AppendFlagsToArgs(args, totalNodes, storageNodeFlag)
}

Expand Down