Skip to content

Commit a2d452f

Browse files
vrutkovsvrutkovs
committed
chore: convert more code to use k8s sets (#2088)
1 parent aadb46d commit a2d452f

19 files changed

Lines changed: 216 additions & 83 deletions

File tree

‎api/operator/v1/cluster_types_test.go‎

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
package v1
2+
3+
import (
4+
"testing"
5+
6+
"github.com/stretchr/testify/assert"
7+
"k8s.io/utils/ptr"
8+
9+
vmv1beta1 "github.com/VictoriaMetrics/operator/api/operator/v1beta1"
10+
)
11+
12+
func TestVTCluster_AvailableStorageNodeIDs(t *testing.T) {
13+
f := func(cr *VTCluster, requestsType string, want []int32) {
14+
t.Helper()
15+
assert.Equal(t, want, cr.AvailableStorageNodeIDs(requestsType))
16+
}
17+
18+
cr := &VTCluster{
19+
Spec: VTClusterSpec{
20+
Storage: &VTStorage{
21+
CommonAppsParams: vmv1beta1.CommonAppsParams{
22+
ReplicaCount: ptr.To(int32(5)),
23+
},
24+
MaintenanceSelectNodeIDs: []int32{1, 3},
25+
MaintenanceInsertNodeIDs: []int32{0, 4},
26+
},
27+
},
28+
}
29+
30+
// select excludes maintenance nodes
31+
f(cr, "select", []int32{0, 2, 4})
32+
33+
// insert excludes maintenance nodes
34+
f(cr, "insert", []int32{1, 2, 3})
35+
36+
// no maintenance nodes
37+
f(&VTCluster{
38+
Spec: VTClusterSpec{
39+
Storage: &VTStorage{
40+
CommonAppsParams: vmv1beta1.CommonAppsParams{ReplicaCount: ptr.To(int32(3))},
41+
},
42+
},
43+
}, "select", []int32{0, 1, 2})
44+
}
45+
46+
func TestVLCluster_AvailableStorageNodeIDs(t *testing.T) {
47+
f := func(cr *VLCluster, requestsType string, want []int32) {
48+
t.Helper()
49+
assert.Equal(t, want, cr.AvailableStorageNodeIDs(requestsType))
50+
}
51+
52+
cr := &VLCluster{
53+
Spec: VLClusterSpec{
54+
VLStorage: &VLStorage{
55+
CommonAppsParams: vmv1beta1.CommonAppsParams{
56+
ReplicaCount: ptr.To(int32(5)),
57+
},
58+
MaintenanceSelectNodeIDs: []int32{1, 3},
59+
MaintenanceInsertNodeIDs: []int32{0, 4},
60+
},
61+
},
62+
}
63+
64+
// select excludes maintenance nodes
65+
f(cr, "select", []int32{0, 2, 4})
66+
67+
// insert excludes maintenance nodes
68+
f(cr, "insert", []int32{1, 2, 3})
69+
70+
// no maintenance nodes
71+
f(&VLCluster{
72+
Spec: VLClusterSpec{
73+
VLStorage: &VLStorage{
74+
CommonAppsParams: vmv1beta1.CommonAppsParams{ReplicaCount: ptr.To(int32(3))},
75+
},
76+
},
77+
}, "select", []int32{0, 1, 2})
78+
}

‎api/operator/v1/vlcluster_types.go‎

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -781,21 +781,17 @@ func (cr *VLCluster) AvailableStorageNodeIDs(requestsType string) []int32 {
781781
if cr.Spec.VLStorage == nil || cr.Spec.VLStorage.ReplicaCount == nil {
782782
return result
783783
}
784-
maintenanceNodes := make(map[int32]struct{})
784+
maintenanceNodes := sets.New[int32]()
785785
switch requestsType {
786786
case "select":
787-
for _, i := range cr.Spec.VLStorage.MaintenanceSelectNodeIDs {
788-
maintenanceNodes[i] = struct{}{}
789-
}
787+
maintenanceNodes.Insert(cr.Spec.VLStorage.MaintenanceSelectNodeIDs...)
790788
case "insert":
791-
for _, i := range cr.Spec.VLStorage.MaintenanceInsertNodeIDs {
792-
maintenanceNodes[i] = struct{}{}
793-
}
789+
maintenanceNodes.Insert(cr.Spec.VLStorage.MaintenanceInsertNodeIDs...)
794790
default:
795791
panic("BUG unsupported requestsType: " + requestsType)
796792
}
797793
for i := int32(0); i < *cr.Spec.VLStorage.ReplicaCount; i++ {
798-
if _, ok := maintenanceNodes[i]; ok {
794+
if maintenanceNodes.Has(i) {
799795
continue
800796
}
801797
result = append(result, i)

‎api/operator/v1/vtcluster_types.go‎

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -692,21 +692,17 @@ func (cr *VTCluster) AvailableStorageNodeIDs(requestsType string) []int32 {
692692
if cr.Spec.Storage == nil || cr.Spec.Storage.ReplicaCount == nil {
693693
return result
694694
}
695-
maintenanceNodes := make(map[int32]struct{})
695+
maintenanceNodes := sets.New[int32]()
696696
switch requestsType {
697697
case "select":
698-
for _, i := range cr.Spec.Storage.MaintenanceSelectNodeIDs {
699-
maintenanceNodes[i] = struct{}{}
700-
}
698+
maintenanceNodes.Insert(cr.Spec.Storage.MaintenanceSelectNodeIDs...)
701699
case "insert":
702-
for _, i := range cr.Spec.Storage.MaintenanceInsertNodeIDs {
703-
maintenanceNodes[i] = struct{}{}
704-
}
700+
maintenanceNodes.Insert(cr.Spec.Storage.MaintenanceInsertNodeIDs...)
705701
default:
706702
panic("BUG unsupported requestsType: " + requestsType)
707703
}
708704
for i := int32(0); i < *cr.Spec.Storage.ReplicaCount; i++ {
709-
if _, ok := maintenanceNodes[i]; ok {
705+
if maintenanceNodes.Has(i) {
710706
continue
711707
}
712708
result = append(result, i)

‎api/operator/v1alpha1/vmdistributed_types.go‎

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import (
2424
appsv1 "k8s.io/api/apps/v1"
2525
corev1 "k8s.io/api/core/v1"
2626
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
27+
"k8s.io/apimachinery/pkg/util/sets"
2728
"k8s.io/utils/ptr"
2829
"sigs.k8s.io/controller-runtime/pkg/client"
2930

@@ -433,9 +434,9 @@ func (cr *VMDistributedSpec) UnmarshalJSON(src []byte) error {
433434

434435
// Validate validates the VMDistributed resource
435436
func (cr *VMDistributed) Validate() error {
436-
zones := make(map[string]struct{})
437-
clusters := make(map[string]struct{})
438-
agents := make(map[string]struct{})
437+
zones := sets.New[string]()
438+
clusters := sets.New[string]()
439+
agents := sets.New[string]()
439440
spec := cr.Spec
440441
hasCommonVMInsert := cr.Spec.ZoneCommon.VMCluster.Spec.VMInsert != nil
441442
hasCommonVMSelect := cr.Spec.ZoneCommon.VMCluster.Spec.VMSelect != nil
@@ -444,23 +445,23 @@ func (cr *VMDistributed) Validate() error {
444445
if len(zone.Name) == 0 {
445446
return fmt.Errorf("spec.zones[%d].name is required", i)
446447
}
447-
if _, ok := zones[zone.Name]; ok {
448+
if zones.Has(zone.Name) {
448449
return fmt.Errorf("spec.zones[%d].name=%s is duplicated, zone names must be unique", i, zone.Name)
449450
}
450-
zones[zone.Name] = struct{}{}
451+
zones.Insert(zone.Name)
451452
clusterName := zone.VMClusterName(cr)
452453
agentName := zone.VMAgentName(cr)
453454
if len(clusterName) > 0 {
454-
if _, ok := clusters[clusterName]; ok {
455+
if clusters.Has(clusterName) {
455456
return fmt.Errorf("spec.zones[%d].vmcluster.name=%s is already added in a different zone", i, clusterName)
456457
}
457-
clusters[clusterName] = struct{}{}
458+
clusters.Insert(clusterName)
458459
}
459460
if len(agentName) > 0 {
460-
if _, ok := agents[agentName]; ok {
461+
if agents.Has(agentName) {
461462
return fmt.Errorf("spec.zones[%d].vmagent.name=%s is already added in a different zone", i, agentName)
462463
}
463-
agents[agentName] = struct{}{}
464+
agents.Insert(agentName)
464465
}
465466
if zone.VMAgent.Spec.StatefulMode {
466467
if zone.VMAgent.Spec.StatefulRollingUpdateStrategyBehavior != nil {

‎api/operator/v1beta1/vmagent_types.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -203,6 +203,7 @@ func (cr *VMAgent) Validate() error {
203203
if err := sc.validate(); err != nil {
204204
return fmt.Errorf("incorrect relabeling for scrapeClass=%q: %w", sc.Name, err)
205205
}
206+
scrapeClassNames.Insert(sc.Name)
206207
}
207208
return nil
208209
}

‎api/operator/v1beta1/vmagent_types_test.go‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,28 @@ func TestVMAgent_Validate(t *testing.T) {
5454
},
5555
}, false)
5656

57+
// duplicate scrape class names
58+
f(VMAgentSpec{
59+
RemoteWrite: []VMAgentRemoteWriteSpec{{URL: "http://some-rw"}},
60+
CommonScrapeParams: CommonScrapeParams{
61+
ScrapeClasses: []ScrapeClass{
62+
{Name: "class-a"},
63+
{Name: "class-a"},
64+
},
65+
},
66+
}, true)
67+
68+
// multiple default scrape classes
69+
f(VMAgentSpec{
70+
RemoteWrite: []VMAgentRemoteWriteSpec{{URL: "http://some-rw"}},
71+
CommonScrapeParams: CommonScrapeParams{
72+
ScrapeClasses: []ScrapeClass{
73+
{Name: "class-a", Default: ptr.To(true)},
74+
{Name: "class-b", Default: ptr.To(true)},
75+
},
76+
},
77+
}, true)
78+
5779
// relabeling with if array
5880
f(VMAgentSpec{
5981
RemoteWrite: []VMAgentRemoteWriteSpec{{URL: "http://some-rw"}},

‎api/operator/v1beta1/vmalertmanagerconfig_types.go‎

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ import (
3636
corev1 "k8s.io/api/core/v1"
3737
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
3838
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
39+
"k8s.io/apimachinery/pkg/util/sets"
3940
)
4041

4142
// VMAlertmanagerConfigSpec defines configuration for VMAlertmanagerConfig
@@ -121,12 +122,12 @@ func (r *VMAlertmanagerConfig) Validate() error {
121122
if MustSkipCRValidation(r) {
122123
return nil
123124
}
124-
receivers := make(map[string]struct{})
125+
receivers := sets.New[string]()
125126
for idx, recv := range r.Spec.Receivers {
126-
if _, ok := receivers[recv.Name]; ok {
127+
if receivers.Has(recv.Name) {
127128
return fmt.Errorf("notification config name %q is not unique", recv.Name)
128129
}
129-
receivers[recv.Name] = struct{}{}
130+
receivers.Insert(recv.Name)
130131
if err := validateReceiver(recv); err != nil {
131132
return fmt.Errorf("receiver at idx=%d is invalid: %w", idx, err)
132133
}
@@ -1506,17 +1507,17 @@ func parseTime(in string) (mins int, err error) {
15061507
return mins, nil
15071508
}
15081509

1509-
func validateTimeIntervals(timeIntervals []TimeIntervals) (map[string]struct{}, error) {
1510-
timeIntervalNames := make(map[string]struct{}, len(timeIntervals))
1510+
func validateTimeIntervals(timeIntervals []TimeIntervals) (sets.Set[string], error) {
1511+
timeIntervalNames := sets.New[string]()
15111512

15121513
for idx, ti := range timeIntervals {
15131514
if err := validateTimeIntervalsEntry(&ti); err != nil {
15141515
return nil, fmt.Errorf("time interval at idx=%d is invalid: %w", idx, err)
15151516
}
1516-
if _, ok := timeIntervalNames[ti.Name]; ok {
1517+
if timeIntervalNames.Has(ti.Name) {
15171518
return nil, fmt.Errorf("time interval at idx=%d is not unique with name=%q", idx, ti.Name)
15181519
}
1519-
timeIntervalNames[ti.Name] = struct{}{}
1520+
timeIntervalNames.Insert(ti.Name)
15201521
}
15211522
return timeIntervalNames, nil
15221523
}
@@ -1527,21 +1528,21 @@ var opsgenieTypeMatcher = regexp.MustCompile(opsgenieValidTypesRe)
15271528

15281529
// checkRouteReceiver returns an error if a node in the routing tree
15291530
// references a receiver not in the given map.
1530-
func checkRouteReceiver(r *SubRoute, receivers map[string]struct{}, tiNames map[string]struct{}) error {
1531+
func checkRouteReceiver(r *SubRoute, receivers sets.Set[string], tiNames sets.Set[string]) error {
15311532
for _, ti := range r.ActiveTimeIntervals {
1532-
if _, ok := tiNames[ti]; !ok {
1533+
if !tiNames.Has(ti) {
15331534
return fmt.Errorf("undefined time interval %q used in route", ti)
15341535
}
15351536
}
15361537
for _, ti := range r.MuteTimeIntervals {
1537-
if _, ok := tiNames[ti]; !ok {
1538+
if !tiNames.Has(ti) {
15381539
return fmt.Errorf("undefined time interval %q used in route", ti)
15391540
}
15401541
}
15411542
if r.Receiver == "" {
15421543
return nil
15431544
}
1544-
if _, ok := receivers[r.Receiver]; !ok {
1545+
if !receivers.Has(r.Receiver) {
15451546
return fmt.Errorf("undefined receiver %q used in route", r.Receiver)
15461547
}
15471548
for idx, sr := range r.Routes {

‎api/operator/v1beta1/vmcluster_types.go‎

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
corev1 "k8s.io/api/core/v1"
1010
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1111
"k8s.io/apimachinery/pkg/labels"
12+
"k8s.io/apimachinery/pkg/util/sets"
1213
"k8s.io/utils/ptr"
1314
"sigs.k8s.io/controller-runtime/pkg/client"
1415
)
@@ -719,21 +720,17 @@ func (cr *VMCluster) AvailableStorageNodeIDs(requestsType string) []int32 {
719720
if cr.Spec.VMStorage == nil || cr.Spec.VMStorage.ReplicaCount == nil {
720721
return result
721722
}
722-
maintenanceNodes := make(map[int32]struct{})
723+
maintenanceNodes := sets.New[int32]()
723724
switch requestsType {
724725
case "select":
725-
for _, i := range cr.Spec.VMStorage.MaintenanceSelectNodeIDs {
726-
maintenanceNodes[i] = struct{}{}
727-
}
726+
maintenanceNodes.Insert(cr.Spec.VMStorage.MaintenanceSelectNodeIDs...)
728727
case "insert":
729-
for _, i := range cr.Spec.VMStorage.MaintenanceInsertNodeIDs {
730-
maintenanceNodes[i] = struct{}{}
731-
}
728+
maintenanceNodes.Insert(cr.Spec.VMStorage.MaintenanceInsertNodeIDs...)
732729
default:
733730
panic("BUG unsupported requestsType: " + requestsType)
734731
}
735732
for i := int32(0); i < *cr.Spec.VMStorage.ReplicaCount; i++ {
736-
if _, ok := maintenanceNodes[i]; ok {
733+
if maintenanceNodes.Has(i) {
737734
continue
738735
}
739736
result = append(result, i)

‎api/operator/v1beta1/vmcluster_types_test.go‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"testing"
55

66
"github.com/stretchr/testify/assert"
7+
"k8s.io/utils/ptr"
78
)
89

910
func TestVMBackup_SnapshotDeletePathWithFlags(t *testing.T) {
@@ -86,3 +87,37 @@ func TestVMBackup_SnapshotCreatePathWithFlags(t *testing.T) {
8687
want: "http://localhost:8429/prefix/custom/snapshot/create?authKey=some-auth-key",
8788
})
8889
}
90+
91+
func TestVMCluster_AvailableStorageNodeIDs(t *testing.T) {
92+
f := func(cr *VMCluster, requestsType string, want []int32) {
93+
t.Helper()
94+
assert.Equal(t, want, cr.AvailableStorageNodeIDs(requestsType))
95+
}
96+
97+
cr := &VMCluster{
98+
Spec: VMClusterSpec{
99+
VMStorage: &VMStorage{
100+
CommonAppsParams: CommonAppsParams{
101+
ReplicaCount: ptr.To(int32(5)),
102+
},
103+
MaintenanceSelectNodeIDs: []int32{1, 3},
104+
MaintenanceInsertNodeIDs: []int32{0, 4},
105+
},
106+
},
107+
}
108+
109+
// select excludes maintenance nodes
110+
f(cr, "select", []int32{0, 2, 4})
111+
112+
// insert excludes maintenance nodes
113+
f(cr, "insert", []int32{1, 2, 3})
114+
115+
// no maintenance nodes
116+
f(&VMCluster{
117+
Spec: VMClusterSpec{
118+
VMStorage: &VMStorage{
119+
CommonAppsParams: CommonAppsParams{ReplicaCount: ptr.To(int32(3))},
120+
},
121+
},
122+
}, "select", []int32{0, 1, 2})
123+
}

0 commit comments

Comments
 (0)