Skip to content

Commit db2c1f3

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request #2260 from mfranczy/bugfix-image-compatibility
Fix matchAny and matchFeatures processing logic for image compatibility
2 parents c3ec794 + 38adc71 commit db2c1f3

File tree

2 files changed

+287
-16
lines changed

2 files changed

+287
-16
lines changed

‎pkg/apis/nfd/nodefeaturerule/rule.go

Lines changed: 17 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -69,9 +69,10 @@ type RuleOutput struct {
6969
// Execute the rule against a set of input features.
7070
func Execute(r *nfdv1alpha1.Rule, features *nfdv1alpha1.Features, failFast bool) (RuleOutput, error) {
7171
var (
72-
matchStatus MatchStatus
73-
isMatch bool
74-
err error
72+
matchStatus MatchStatus
73+
isMatchAny bool
74+
isMatchFeature bool
75+
err error
7576
)
7677
labels := make(map[string]string)
7778
vars := make(map[string]string)
@@ -87,7 +88,7 @@ func Execute(r *nfdv1alpha1.Rule, features *nfdv1alpha1.Features, failFast bool)
8788
if matched, featureStatus, err = evaluateMatchAnyElem(&matcher, features, failFast); err != nil {
8889
return RuleOutput{}, err
8990
} else if matched {
90-
isMatch = true
91+
isMatchAny = true
9192
klog.V(4).InfoS("matchAny matched", "ruleName", r.Name, "matchedFeatures", utils.DelayedDumper(featureStatus.MatchedFeatures))
9293

9394
if r.LabelsTemplate == "" && r.VarsTemplate == "" && failFast {
@@ -108,16 +109,16 @@ func Execute(r *nfdv1alpha1.Rule, features *nfdv1alpha1.Features, failFast bool)
108109
matchStatus.MatchAny = append(matchStatus.MatchAny, featureStatus)
109110
}
110111

111-
if !isMatch && failFast {
112+
if !isMatchAny && failFast {
112113
klog.V(2).InfoS("rule did not match", "ruleName", r.Name)
113114
return RuleOutput{MatchStatus: &matchStatus}, nil
114115
}
115116
}
116117

117118
if len(r.MatchFeatures) > 0 {
118-
if isMatch, matchStatus.MatchFeatureStatus, err = evaluateFeatureMatcher(&r.MatchFeatures, features, failFast); err != nil {
119+
if isMatchFeature, matchStatus.MatchFeatureStatus, err = evaluateFeatureMatcher(&r.MatchFeatures, features, failFast); err != nil {
119120
return RuleOutput{}, err
120-
} else if !isMatch {
121+
} else if !isMatchFeature {
121122
klog.V(2).InfoS("rule did not match", "ruleName", r.Name)
122123
return RuleOutput{MatchStatus: &matchStatus}, nil
123124
} else {
@@ -133,7 +134,7 @@ func Execute(r *nfdv1alpha1.Rule, features *nfdv1alpha1.Features, failFast bool)
133134

134135
maps.Copy(labels, r.Labels)
135136
maps.Copy(vars, r.Vars)
136-
matchStatus.IsMatch = isMatch
137+
matchStatus.IsMatch = (len(r.MatchAny) == 0 || isMatchAny) && (len(r.MatchFeatures) == 0 || isMatchFeature)
137138

138139
ret := RuleOutput{
139140
Labels: labels,
@@ -157,8 +158,9 @@ type GroupRuleOutput struct {
157158
// rule matches.
158159
func ExecuteGroupRule(r *nfdv1alpha1.GroupRule, features *nfdv1alpha1.Features, failFast bool) (GroupRuleOutput, error) {
159160
var (
160-
matchStatus MatchStatus
161-
isMatch bool
161+
matchStatus MatchStatus
162+
isMatchAny bool
163+
isMatchFeature bool
162164
)
163165
vars := make(map[string]string)
164166

@@ -170,7 +172,7 @@ func ExecuteGroupRule(r *nfdv1alpha1.GroupRule, features *nfdv1alpha1.Features,
170172
if err != nil {
171173
return GroupRuleOutput{}, err
172174
} else if matched {
173-
isMatch = true
175+
isMatchAny = true
174176
klog.V(4).InfoS("matchAny matched", "ruleName", r.Name, "matchedFeatures", utils.DelayedDumper(featureStatus.MatchedFeatures))
175177

176178
if r.VarsTemplate == "" && failFast {
@@ -185,16 +187,16 @@ func ExecuteGroupRule(r *nfdv1alpha1.GroupRule, features *nfdv1alpha1.Features,
185187
}
186188
matchStatus.MatchAny = append(matchStatus.MatchAny, featureStatus)
187189
}
188-
if !isMatch && failFast {
190+
if !isMatchAny && failFast {
189191
return GroupRuleOutput{MatchStatus: &matchStatus}, nil
190192
}
191193
}
192194

193195
if len(r.MatchFeatures) > 0 {
194196
var err error
195-
if isMatch, matchStatus.MatchFeatureStatus, err = evaluateFeatureMatcher(&r.MatchFeatures, features, failFast); err != nil {
197+
if isMatchFeature, matchStatus.MatchFeatureStatus, err = evaluateFeatureMatcher(&r.MatchFeatures, features, failFast); err != nil {
196198
return GroupRuleOutput{}, err
197-
} else if !isMatch {
199+
} else if !isMatchFeature {
198200
klog.V(2).InfoS("rule did not match", "ruleName", r.Name)
199201
return GroupRuleOutput{MatchStatus: &matchStatus}, nil
200202
}
@@ -204,7 +206,7 @@ func ExecuteGroupRule(r *nfdv1alpha1.GroupRule, features *nfdv1alpha1.Features,
204206
}
205207

206208
maps.Copy(vars, r.Vars)
207-
matchStatus.IsMatch = isMatch
209+
matchStatus.IsMatch = (len(r.MatchAny) == 0 || isMatchAny) && (len(r.MatchFeatures) == 0 || isMatchFeature)
208210

209211
ret := GroupRuleOutput{
210212
Vars: vars,

‎pkg/client-nfd/compat/node-validator/node-validator_test.go

Lines changed: 270 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -311,7 +311,7 @@ func TestNodeValidator(t *testing.T) {
311311
assertOutput(ctx, spec, expectedOutput)
312312
})
313313

314-
Convey("That contains matchAny and matchFeatures in one spec", func() {
314+
Convey("That contains matchAny and matchFeatures in one spec and results in match", func() {
315315
spec := buildDefaultSpec([]v1alpha1.GroupRule{
316316
{
317317
Name: "fake_6",
@@ -404,6 +404,275 @@ func TestNodeValidator(t *testing.T) {
404404
assertOutput(ctx, spec, expectedOutput)
405405
})
406406

407+
Convey("That contains matchAny and matchFeatures in one spec and results in mismatch", func() {
408+
spec := buildDefaultSpec([]v1alpha1.GroupRule{
409+
{
410+
Name: "fake_7",
411+
MatchAny: []v1alpha1.MatchAnyElem{
412+
{
413+
MatchFeatures: v1alpha1.FeatureMatcher{
414+
{
415+
Feature: "fake.instance",
416+
MatchExpressions: &v1alpha1.MatchExpressionSet{
417+
"name": &v1alpha1.MatchExpression{Op: v1alpha1.MatchIn, Value: v1alpha1.MatchValue{"instance_1"}},
418+
},
419+
},
420+
},
421+
},
422+
{
423+
MatchFeatures: v1alpha1.FeatureMatcher{
424+
{
425+
Feature: "fake.instance",
426+
MatchExpressions: &v1alpha1.MatchExpressionSet{
427+
"name": &v1alpha1.MatchExpression{Op: v1alpha1.MatchIn, Value: v1alpha1.MatchValue{"instance_unknown"}},
428+
},
429+
},
430+
{
431+
Feature: "fake.instance",
432+
MatchExpressions: &v1alpha1.MatchExpressionSet{
433+
"id": &v1alpha1.MatchExpression{Op: v1alpha1.MatchIn, Value: v1alpha1.MatchValue{"unknown"}},
434+
},
435+
},
436+
},
437+
},
438+
},
439+
MatchFeatures: v1alpha1.FeatureMatcher{
440+
{
441+
Feature: "fake.attribute",
442+
MatchExpressions: &v1alpha1.MatchExpressionSet{
443+
"attr_unknown": &v1alpha1.MatchExpression{Op: v1alpha1.MatchIn, Value: v1alpha1.MatchValue{"true"}},
444+
},
445+
},
446+
{
447+
Feature: "fake.attribute",
448+
MatchExpressions: &v1alpha1.MatchExpressionSet{
449+
"attr_1": &v1alpha1.MatchExpression{Op: v1alpha1.MatchIn, Value: v1alpha1.MatchValue{"true"}},
450+
},
451+
},
452+
},
453+
},
454+
})
455+
456+
expectedOutput := buildDefaultExpectedOutput([]ProcessedRuleStatus{
457+
{
458+
Name: "fake_7",
459+
IsMatch: false,
460+
MatchedAny: []MatchAnyElem{
461+
{
462+
MatchedExpressions: []MatchedExpression{
463+
{
464+
Feature: "fake.instance",
465+
Name: "name",
466+
Expression: &v1alpha1.MatchExpression{Op: v1alpha1.MatchIn, Value: v1alpha1.MatchValue{"instance_1"}},
467+
MatcherType: MatchExpressionType,
468+
IsMatch: true,
469+
},
470+
},
471+
},
472+
{
473+
MatchedExpressions: []MatchedExpression{
474+
{
475+
Feature: "fake.instance",
476+
Name: "id",
477+
Expression: &v1alpha1.MatchExpression{Op: v1alpha1.MatchIn, Value: v1alpha1.MatchValue{"unknown"}},
478+
MatcherType: MatchExpressionType,
479+
IsMatch: false,
480+
},
481+
{
482+
Feature: "fake.instance",
483+
Name: "name",
484+
Expression: &v1alpha1.MatchExpression{Op: v1alpha1.MatchIn, Value: v1alpha1.MatchValue{"instance_unknown"}},
485+
MatcherType: MatchExpressionType,
486+
IsMatch: false,
487+
},
488+
},
489+
},
490+
},
491+
MatchedExpressions: []MatchedExpression{
492+
{
493+
Feature: "fake.attribute",
494+
Name: "attr_1",
495+
Expression: &v1alpha1.MatchExpression{Op: v1alpha1.MatchIn, Value: v1alpha1.MatchValue{"true"}},
496+
MatcherType: MatchExpressionType,
497+
IsMatch: true,
498+
},
499+
{
500+
Feature: "fake.attribute",
501+
Name: "attr_unknown",
502+
Expression: &v1alpha1.MatchExpression{Op: v1alpha1.MatchIn, Value: v1alpha1.MatchValue{"true"}},
503+
MatcherType: MatchExpressionType,
504+
IsMatch: false,
505+
},
506+
},
507+
},
508+
})
509+
510+
assertOutput(ctx, spec, expectedOutput)
511+
})
512+
513+
Convey("That contains flags, attrs and instances which results in mismatch", func() {
514+
spec := buildDefaultSpec([]v1alpha1.GroupRule{
515+
{
516+
Name: "fake_8",
517+
MatchFeatures: v1alpha1.FeatureMatcher{
518+
{
519+
Feature: "fake.flag",
520+
MatchExpressions: &v1alpha1.MatchExpressionSet{
521+
"flag_unknown": &v1alpha1.MatchExpression{Op: v1alpha1.MatchExists},
522+
},
523+
},
524+
{
525+
Feature: "fake.attribute",
526+
MatchExpressions: &v1alpha1.MatchExpressionSet{
527+
"attr_1": &v1alpha1.MatchExpression{Op: v1alpha1.MatchIn, Value: v1alpha1.MatchValue{"true"}},
528+
},
529+
},
530+
{
531+
Feature: "fake.instance",
532+
MatchExpressions: &v1alpha1.MatchExpressionSet{
533+
"name": &v1alpha1.MatchExpression{Op: v1alpha1.MatchIn, Value: v1alpha1.MatchValue{"instance_1"}},
534+
"attr_1": &v1alpha1.MatchExpression{Op: v1alpha1.MatchIn, Value: v1alpha1.MatchValue{"true"}},
535+
},
536+
},
537+
{
538+
Feature: "fake.instance",
539+
MatchExpressions: &v1alpha1.MatchExpressionSet{
540+
"name": &v1alpha1.MatchExpression{Op: v1alpha1.MatchIn, Value: v1alpha1.MatchValue{"instance_2"}},
541+
"attr_1": &v1alpha1.MatchExpression{Op: v1alpha1.MatchIn, Value: v1alpha1.MatchValue{"false"}},
542+
},
543+
},
544+
},
545+
},
546+
})
547+
548+
expectedOutput := buildDefaultExpectedOutput([]ProcessedRuleStatus{
549+
{
550+
Name: "fake_8",
551+
IsMatch: false,
552+
MatchedExpressions: []MatchedExpression{
553+
{
554+
Feature: "fake.attribute",
555+
Name: "attr_1",
556+
Expression: &v1alpha1.MatchExpression{Op: v1alpha1.MatchIn, Value: v1alpha1.MatchValue{"true"}},
557+
MatcherType: MatchExpressionType,
558+
IsMatch: true,
559+
},
560+
{
561+
Feature: "fake.flag",
562+
Name: "flag_unknown",
563+
Expression: &v1alpha1.MatchExpression{Op: v1alpha1.MatchExists},
564+
MatcherType: MatchExpressionType,
565+
IsMatch: false,
566+
},
567+
{
568+
Feature: "fake.instance",
569+
Name: "attr_1",
570+
Expression: &v1alpha1.MatchExpression{Op: v1alpha1.MatchIn, Value: v1alpha1.MatchValue{"false"}},
571+
MatcherType: MatchExpressionType,
572+
IsMatch: false,
573+
},
574+
{
575+
Feature: "fake.instance",
576+
Name: "attr_1",
577+
Expression: &v1alpha1.MatchExpression{Op: v1alpha1.MatchIn, Value: v1alpha1.MatchValue{"true"}},
578+
MatcherType: MatchExpressionType,
579+
IsMatch: true,
580+
},
581+
{
582+
Feature: "fake.instance",
583+
Name: "name",
584+
Expression: &v1alpha1.MatchExpression{Op: v1alpha1.MatchIn, Value: v1alpha1.MatchValue{"instance_1"}},
585+
MatcherType: MatchExpressionType,
586+
IsMatch: true,
587+
},
588+
{
589+
Feature: "fake.instance",
590+
Name: "name",
591+
Expression: &v1alpha1.MatchExpression{Op: v1alpha1.MatchIn, Value: v1alpha1.MatchValue{"instance_2"}},
592+
MatcherType: MatchExpressionType,
593+
IsMatch: true,
594+
},
595+
},
596+
},
597+
})
598+
599+
assertOutput(ctx, spec, expectedOutput)
600+
})
601+
602+
Convey("That contains flags, attrs and instances which results in match", func() {
603+
spec := buildDefaultSpec([]v1alpha1.GroupRule{
604+
{
605+
Name: "fake_9",
606+
MatchFeatures: v1alpha1.FeatureMatcher{
607+
{
608+
Feature: "fake.flag",
609+
MatchName: &v1alpha1.MatchExpression{Op: v1alpha1.MatchInRegexp, Value: v1alpha1.MatchValue{"^flag"}},
610+
},
611+
{
612+
Feature: "fake.attribute",
613+
MatchExpressions: &v1alpha1.MatchExpressionSet{
614+
"attr_1": &v1alpha1.MatchExpression{Op: v1alpha1.MatchIn, Value: v1alpha1.MatchValue{"true"}},
615+
},
616+
},
617+
{
618+
Feature: "fake.instance",
619+
MatchExpressions: &v1alpha1.MatchExpressionSet{
620+
"name": &v1alpha1.MatchExpression{Op: v1alpha1.MatchIn, Value: v1alpha1.MatchValue{"instance_1"}},
621+
},
622+
},
623+
},
624+
},
625+
})
626+
627+
expectedOutput := buildDefaultExpectedOutput([]ProcessedRuleStatus{
628+
{
629+
Name: "fake_9",
630+
IsMatch: true,
631+
MatchedExpressions: []MatchedExpression{
632+
{
633+
Feature: "fake.attribute",
634+
Name: "attr_1",
635+
Expression: &v1alpha1.MatchExpression{Op: v1alpha1.MatchIn, Value: v1alpha1.MatchValue{"true"}},
636+
MatcherType: MatchExpressionType,
637+
IsMatch: true,
638+
},
639+
{
640+
Feature: "fake.flag",
641+
Name: "",
642+
Expression: &v1alpha1.MatchExpression{Op: v1alpha1.MatchInRegexp, Value: v1alpha1.MatchValue{"^flag"}},
643+
MatcherType: MatchNameType,
644+
IsMatch: true,
645+
},
646+
{
647+
Feature: "fake.instance",
648+
Name: "name",
649+
Expression: &v1alpha1.MatchExpression{Op: v1alpha1.MatchIn, Value: v1alpha1.MatchValue{"instance_1"}},
650+
MatcherType: MatchExpressionType,
651+
IsMatch: true,
652+
},
653+
},
654+
},
655+
})
656+
657+
assertOutput(ctx, spec, expectedOutput)
658+
})
659+
660+
Convey("That contains empty spec", func() {
661+
spec := buildDefaultSpec([]v1alpha1.GroupRule{
662+
{
663+
Name: "fake_10",
664+
MatchFeatures: v1alpha1.FeatureMatcher{},
665+
},
666+
})
667+
expectedOutput := buildDefaultExpectedOutput([]ProcessedRuleStatus{
668+
{
669+
Name: "fake_10",
670+
IsMatch: true,
671+
MatchedExpressions: nil,
672+
},
673+
})
674+
assertOutput(ctx, spec, expectedOutput)
675+
})
407676
})
408677

409678
Convey("With multiple compatibility sets", t, func() {

0 commit comments

Comments
 (0)