Skip to content

Commit 7523ca2

Browse files
P0lskayP0lskay
committed
Fixed "configcheck validates ClusterVectorPipeline (CVP) against all ClusterVectorAggregator instances instead of only matching ones"
1 parent 30ddb92 commit 7523ca2

5 files changed

Lines changed: 161 additions & 81 deletions

File tree

‎internal/controller/pipeline_controller.go‎

Lines changed: 31 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -24,24 +24,23 @@ import (
2424
"time"
2525

2626
"golang.org/x/sync/errgroup"
27-
"sigs.k8s.io/controller-runtime/pkg/controller"
28-
"sigs.k8s.io/controller-runtime/pkg/predicate"
29-
30-
"github.com/kaasops/vector-operator/internal/config/configcheck"
31-
"github.com/kaasops/vector-operator/internal/vector/aggregator"
32-
"github.com/kaasops/vector-operator/internal/vector/vectoragent"
33-
3427
"k8s.io/apimachinery/pkg/runtime"
3528
"k8s.io/client-go/kubernetes"
3629
ctrl "sigs.k8s.io/controller-runtime"
3730
"sigs.k8s.io/controller-runtime/pkg/client"
31+
"sigs.k8s.io/controller-runtime/pkg/controller"
3832
"sigs.k8s.io/controller-runtime/pkg/event"
3933
"sigs.k8s.io/controller-runtime/pkg/handler"
4034
"sigs.k8s.io/controller-runtime/pkg/log"
35+
"sigs.k8s.io/controller-runtime/pkg/predicate"
4136

4237
"github.com/kaasops/vector-operator/api/v1alpha1"
4338
"github.com/kaasops/vector-operator/internal/config"
39+
"github.com/kaasops/vector-operator/internal/config/configcheck"
4440
"github.com/kaasops/vector-operator/internal/pipeline"
41+
"github.com/kaasops/vector-operator/internal/utils/k8s"
42+
"github.com/kaasops/vector-operator/internal/vector/aggregator"
43+
"github.com/kaasops/vector-operator/internal/vector/vectoragent"
4544
)
4645

4746
type PipelineReconciler struct {
@@ -139,12 +138,22 @@ func (r *PipelineReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c
139138
return ctrl.Result{}, nil
140139
}
141140
pipelineCR.SetRole(pipelineVectorRole)
142-
141+
var pipelineLabels map[string]string
142+
if pipelineCR.GetLabels() != nil {
143+
pipelineLabels = pipelineCR.GetLabels()
144+
}
143145
eg := errgroup.Group{}
144146

145147
if *pipelineVectorRole == v1alpha1.VectorPipelineRoleAgent {
146148

147149
for _, vector := range vectorAgents {
150+
var selectorLabels map[string]string
151+
if vector.Spec.Selector != nil {
152+
selectorLabels = vector.Spec.Selector.MatchLabels
153+
}
154+
if !k8s.MatchLabels(selectorLabels, pipelineLabels) {
155+
continue
156+
}
148157
eg.Go(func() error {
149158
vaCtrl := vectoragent.NewController(vector, r.Client, r.Clientset)
150159
cfg, byteConfig, err := config.BuildAgentConfig(config.VectorConfigParams{
@@ -184,6 +193,13 @@ func (r *PipelineReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c
184193

185194
if pipelineCR.GetNamespace() != "" {
186195
for _, vector := range vectorAggregators {
196+
var selectorLabels map[string]string
197+
if vector.Spec.Selector != nil {
198+
selectorLabels = vector.Spec.Selector.MatchLabels
199+
}
200+
if !k8s.MatchLabels(selectorLabels, pipelineLabels) {
201+
continue
202+
}
187203
eg.Go(func() error {
188204
vaCtrl := aggregator.NewController(vector, r.Client, r.Clientset)
189205
cfg, err := config.BuildAggregatorConfig(config.VectorConfigParams{
@@ -230,6 +246,13 @@ func (r *PipelineReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c
230246
} else {
231247

232248
for _, vector := range clusterVectorAggregators {
249+
var selectorLabels map[string]string
250+
if vector.Spec.Selector != nil {
251+
selectorLabels = vector.Spec.Selector.MatchLabels
252+
}
253+
if !k8s.MatchLabels(selectorLabels, pipelineLabels) {
254+
continue
255+
}
233256
eg.Go(func() error {
234257
vaCtrl := aggregator.NewController(vector, r.Client, r.Clientset)
235258
cfg, err := config.BuildAggregatorConfig(config.VectorConfigParams{

‎internal/pipeline/pipeline.go‎

Lines changed: 3 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import (
2525
"sigs.k8s.io/controller-runtime/pkg/client"
2626

2727
"github.com/kaasops/vector-operator/api/v1alpha1"
28+
"github.com/kaasops/vector-operator/internal/utils/k8s"
2829
)
2930

3031
type Pipeline interface {
@@ -82,7 +83,7 @@ func GetValidPipelines(ctx context.Context, client client.Client, filter FilterP
8283
vp.IsValid() &&
8384
vp.GetRole() == filter.Role &&
8485
(filter.Scope == AllPipelines || vp.Namespace == filter.Namespace) &&
85-
MatchLabels(matchLabels, vp.Labels) {
86+
k8s.MatchLabels(matchLabels, vp.Labels) {
8687
validPipelines = append(validPipelines, vp.DeepCopy())
8788
}
8889
}
@@ -99,7 +100,7 @@ func GetValidPipelines(ctx context.Context, client client.Client, filter FilterP
99100
if !cvp.IsDeleted() &&
100101
cvp.IsValid() &&
101102
cvp.GetRole() == filter.Role &&
102-
MatchLabels(matchLabels, cvp.Labels) {
103+
k8s.MatchLabels(matchLabels, cvp.Labels) {
103104
validPipelines = append(validPipelines, cvp.DeepCopy())
104105
}
105106
}
@@ -148,15 +149,3 @@ func GetClusterVectorPipelines(ctx context.Context, client client.Client) ([]v1a
148149
}
149150
return cvps.Items, nil
150151
}
151-
152-
func MatchLabels(selector map[string]string, labels map[string]string) bool {
153-
if selector == nil {
154-
return true
155-
}
156-
for k, v := range selector {
157-
if labels[k] != v {
158-
return false
159-
}
160-
}
161-
return true
162-
}

‎internal/pipeline/pipeline_test.go‎

Lines changed: 0 additions & 59 deletions
This file was deleted.

‎internal/utils/k8s/label.go‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,3 +57,16 @@ func MergeLabels(dst, src map[string]string) map[string]string {
5757
}
5858
return dst
5959
}
60+
61+
// MatchLabels matches a set of Kubernetes selectors and a set of Kubernetes labels
62+
func MatchLabels(selector map[string]string, labels map[string]string) bool {
63+
if selector == nil {
64+
return true
65+
}
66+
for k, v := range selector {
67+
if labels[k] != v {
68+
return false
69+
}
70+
}
71+
return true
72+
}

‎internal/utils/k8s/label_test.go‎

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,114 @@
1+
package k8s
2+
3+
import (
4+
"reflect"
5+
"testing"
6+
)
7+
8+
func TestMatchLabels(t *testing.T) {
9+
tests := []struct {
10+
name string
11+
selector map[string]string
12+
labels map[string]string
13+
want bool
14+
}{
15+
{
16+
name: "NoSelector",
17+
selector: nil,
18+
labels: map[string]string{"label1": "value1", "label2": "value2"},
19+
want: true,
20+
},
21+
{
22+
name: "MatchingLabels",
23+
selector: map[string]string{"label1": "value1", "label2": "value2"},
24+
labels: map[string]string{"label1": "value1", "label2": "value2"},
25+
want: true,
26+
},
27+
{
28+
name: "MismatchedLabelValues",
29+
selector: map[string]string{"label1": "value1", "label2": "value2"},
30+
labels: map[string]string{"label1": "value1", "label2": "mismatch"},
31+
want: false,
32+
},
33+
{
34+
name: "ExtraLabelsInMap",
35+
selector: map[string]string{"label1": "value1"},
36+
labels: map[string]string{"label1": "value1", "label2": "value2"},
37+
want: true,
38+
},
39+
{
40+
name: "SelectorWithNoMatches",
41+
selector: map[string]string{"label1": "value1", "label2": "value2"},
42+
labels: map[string]string{"label3": "value3"},
43+
want: false,
44+
},
45+
{
46+
name: "SelectorWithNoMatches2",
47+
selector: map[string]string{"label1": "value1", "label2": "value2"},
48+
labels: map[string]string{"label1": "label1"},
49+
want: false,
50+
},
51+
}
52+
53+
for _, test := range tests {
54+
t.Run(test.name, func(t *testing.T) {
55+
if got := MatchLabels(test.selector, test.labels); got != test.want {
56+
t.Errorf("MatchLabels() = %v, want %v", got, test.want)
57+
}
58+
})
59+
}
60+
}
61+
62+
func TestMergeLabels(t *testing.T) {
63+
tests := []struct {
64+
name string
65+
sourceLabels map[string]string
66+
distLabels map[string]string
67+
want map[string]string
68+
}{
69+
{
70+
name: "EmptySource",
71+
sourceLabels: nil,
72+
distLabels: map[string]string{"label1": "value1", "label2": "value2"},
73+
want: map[string]string{"label1": "value1", "label2": "value2"},
74+
},
75+
{
76+
name: "EmptyDist",
77+
sourceLabels: map[string]string{"label1": "value1", "label2": "value2"},
78+
distLabels: nil,
79+
want: map[string]string{"label1": "value1", "label2": "value2"},
80+
},
81+
{
82+
name: "DifferentLabelValues",
83+
sourceLabels: map[string]string{"label1": "value1", "label2": "value2"},
84+
distLabels: map[string]string{"label1": "value1", "label2": "mismatch"},
85+
want: map[string]string{"label1": "value1", "label2": "mismatch"},
86+
},
87+
{
88+
name: "SameLabelValues",
89+
sourceLabels: map[string]string{"label1": "value1"},
90+
distLabels: map[string]string{"label1": "value1", "label2": "value2"},
91+
want: map[string]string{"label1": "value1", "label2": "value2"},
92+
},
93+
{
94+
name: "NewLabelValues",
95+
sourceLabels: map[string]string{"label1": "value1", "label2": "value2"},
96+
distLabels: map[string]string{"label3": "value3"},
97+
want: map[string]string{"label1": "value1", "label2": "value2", "label3": "value3"},
98+
},
99+
{
100+
name: "DifferentLabelValues2",
101+
sourceLabels: map[string]string{"label1": "value1", "label2": "value2"},
102+
distLabels: map[string]string{"label1": "label1"},
103+
want: map[string]string{"label1": "label1", "label2": "value2"},
104+
},
105+
}
106+
107+
for _, test := range tests {
108+
t.Run(test.name, func(t *testing.T) {
109+
if got := MergeLabels(test.distLabels, test.sourceLabels); !reflect.DeepEqual(got, test.want) {
110+
t.Errorf("MatchLabels() = %v, want %v", got, test.want)
111+
}
112+
})
113+
}
114+
}

0 commit comments

Comments
 (0)