Skip to content

Commit 29ee4fe

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request #1426 from marquiz/devel/e2e-annotations-validation
test/e2e: stricter validation of node annotations
2 parents 6144f3d + ddec2e2 commit 29ee4fe

File tree

2 files changed

+46
-47
lines changed

2 files changed

+46
-47
lines changed

‎test/e2e/gomega.go

Lines changed: 10 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -44,10 +44,9 @@ func eventuallyNonControlPlaneNodes(ctx context.Context, cli clientset.Interface
4444

4545
// MatchLabels returns a specialized Gomega matcher for checking if a list of
4646
// nodes are labeled as expected.
47-
func MatchLabels(expectedNew map[string]k8sLabels, oldNodes []corev1.Node, ignoreUnexpected bool) gomegatypes.GomegaMatcher {
47+
func MatchLabels(expectedNew map[string]k8sLabels, oldNodes []corev1.Node) gomegatypes.GomegaMatcher {
4848
matcher := &nodeIterablePropertyMatcher[k8sLabels]{
49-
propertyName: "labels",
50-
ignoreUnexpected: ignoreUnexpected,
49+
propertyName: "labels",
5150
matchFunc: func(newNode, oldNode corev1.Node, expected k8sLabels) ([]string, []string, []string) {
5251
expectedAll := maps.Clone(oldNode.Labels)
5352
maps.Copy(expectedAll, expected)
@@ -64,10 +63,9 @@ func MatchLabels(expectedNew map[string]k8sLabels, oldNodes []corev1.Node, ignor
6463

6564
// MatchAnnotations returns a specialized Gomega matcher for checking if a list of
6665
// nodes are annotated as expected.
67-
func MatchAnnotations(expectedNew map[string]k8sAnnotations, oldNodes []corev1.Node, ignoreUnexpected bool) gomegatypes.GomegaMatcher {
66+
func MatchAnnotations(expectedNew map[string]k8sAnnotations, oldNodes []corev1.Node) gomegatypes.GomegaMatcher {
6867
matcher := &nodeIterablePropertyMatcher[k8sAnnotations]{
69-
propertyName: "annotations",
70-
ignoreUnexpected: ignoreUnexpected,
68+
propertyName: "annotations",
7169
matchFunc: func(newNode, oldNode corev1.Node, expected k8sAnnotations) ([]string, []string, []string) {
7270
expectedAll := maps.Clone(oldNode.Annotations)
7371
maps.Copy(expectedAll, expected)
@@ -84,10 +82,9 @@ func MatchAnnotations(expectedNew map[string]k8sAnnotations, oldNodes []corev1.N
8482

8583
// MatchCapacity returns a specialized Gomega matcher for checking if a list of
8684
// nodes have resource capacity as expected.
87-
func MatchCapacity(expectedNew map[string]corev1.ResourceList, oldNodes []corev1.Node, ignoreUnexpected bool) gomegatypes.GomegaMatcher {
85+
func MatchCapacity(expectedNew map[string]corev1.ResourceList, oldNodes []corev1.Node) gomegatypes.GomegaMatcher {
8886
matcher := &nodeIterablePropertyMatcher[corev1.ResourceList]{
89-
propertyName: "resource capacity",
90-
ignoreUnexpected: ignoreUnexpected,
87+
propertyName: "resource capacity",
9188
matchFunc: func(newNode, oldNode corev1.Node, expected corev1.ResourceList) ([]string, []string, []string) {
9289
expectedAll := oldNode.Status.DeepCopy().Capacity
9390
maps.Copy(expectedAll, expected)
@@ -104,10 +101,9 @@ func MatchCapacity(expectedNew map[string]corev1.ResourceList, oldNodes []corev1
104101

105102
// MatchTaints returns a specialized Gomega matcher for checking if a list of
106103
// nodes are tainted as expected.
107-
func MatchTaints(expectedNew map[string][]corev1.Taint, oldNodes []corev1.Node, ignoreUnexpected bool) gomegatypes.GomegaMatcher {
104+
func MatchTaints(expectedNew map[string][]corev1.Taint, oldNodes []corev1.Node) gomegatypes.GomegaMatcher {
108105
matcher := &nodeIterablePropertyMatcher[[]corev1.Taint]{
109-
propertyName: "taints",
110-
ignoreUnexpected: ignoreUnexpected,
106+
propertyName: "taints",
111107
matchFunc: func(newNode, oldNode corev1.Node, expected []corev1.Taint) (missing, invalid, unexpected []string) {
112108
expectedAll := oldNode.Spec.DeepCopy().Taints
113109
expectedAll = append(expectedAll, expected...)
@@ -203,9 +199,8 @@ func (m *nodeListPropertyMatcher[T]) NegatedFailureMessage(actual interface{}) s
203199
// nodeIterablePropertyMatcher is a nodePropertyMatcher for matching iterable
204200
// elements such as maps or lists.
205201
type nodeIterablePropertyMatcher[T any] struct {
206-
propertyName string
207-
ignoreUnexpected bool
208-
matchFunc func(newNode, oldNode corev1.Node, expected T) ([]string, []string, []string)
202+
propertyName string
203+
matchFunc func(newNode, oldNode corev1.Node, expected T) ([]string, []string, []string)
209204

210205
// TODO remove nolint when golangci-lint is able to cope with generics
211206
node *corev1.Node //nolint:unused
@@ -222,9 +217,6 @@ func (m *nodeIterablePropertyMatcher[T]) match(newNode, oldNode corev1.Node, exp
222217
m.node = &newNode
223218
m.missing, m.invalidValue, m.unexpected = m.matchFunc(newNode, oldNode, expected)
224219

225-
if m.ignoreUnexpected {
226-
m.unexpected = nil
227-
}
228220
return len(m.missing) == 0 && len(m.invalidValue) == 0 && len(m.unexpected) == 0
229221
}
230222

‎test/e2e/node_feature_discovery_test.go

Lines changed: 36 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -294,7 +294,7 @@ var _ = SIGDescribe("NFD master and worker", func() {
294294
},
295295
"*": {},
296296
}
297-
eventuallyNonControlPlaneNodes(ctx, f.ClientSet).Should(MatchLabels(expectedLabels, nodes, false))
297+
eventuallyNonControlPlaneNodes(ctx, f.ClientSet).Should(MatchLabels(expectedLabels, nodes))
298298

299299
checkNodeFeatureObject(ctx, workerPod.Spec.NodeName)
300300

@@ -478,7 +478,7 @@ var _ = SIGDescribe("NFD master and worker", func() {
478478
},
479479
"*": {},
480480
}
481-
eventuallyNonControlPlaneNodes(ctx, f.ClientSet).Should(MatchLabels(expectedLabels, nodes, false))
481+
eventuallyNonControlPlaneNodes(ctx, f.ClientSet).Should(MatchLabels(expectedLabels, nodes))
482482

483483
By("Deleting nfd-worker daemonset")
484484
err = f.ClientSet.AppsV1().DaemonSets(f.Namespace.Name).Delete(ctx, workerDS.Name, metav1.DeleteOptions{})
@@ -523,14 +523,14 @@ var _ = SIGDescribe("NFD master and worker", func() {
523523
},
524524
"*": {},
525525
}
526-
eventuallyNonControlPlaneNodes(ctx, f.ClientSet).Should(MatchLabels(expectedLabels, nodes, false))
526+
eventuallyNonControlPlaneNodes(ctx, f.ClientSet).Should(MatchLabels(expectedLabels, nodes))
527527
By("Deleting NodeFeature object")
528528
err = nfdClient.NfdV1alpha1().NodeFeatures(f.Namespace.Name).Delete(ctx, nodeFeatures[0], metav1.DeleteOptions{})
529529
Expect(err).NotTo(HaveOccurred())
530530

531531
By("Verifying node labels from NodeFeature object were removed")
532532
expectedLabels[targetNodeName] = k8sLabels{}
533-
eventuallyNonControlPlaneNodes(ctx, f.ClientSet).Should(MatchLabels(expectedLabels, nodes, false))
533+
eventuallyNonControlPlaneNodes(ctx, f.ClientSet).Should(MatchLabels(expectedLabels, nodes))
534534

535535
By("Creating nfd-worker daemonset")
536536
podSpecOpts := createPodSpecOpts(
@@ -552,7 +552,7 @@ var _ = SIGDescribe("NFD master and worker", func() {
552552
nfdv1alpha1.FeatureLabelNs + "/fake-fakefeature3": "true",
553553
},
554554
}
555-
eventuallyNonControlPlaneNodes(ctx, f.ClientSet).Should(MatchLabels(expectedLabels, nodes, false))
555+
eventuallyNonControlPlaneNodes(ctx, f.ClientSet).Should(MatchLabels(expectedLabels, nodes))
556556

557557
By("Re-creating NodeFeature object")
558558
_, err = testutils.CreateOrUpdateNodeFeaturesFromFile(ctx, nfdClient, "nodefeature-1.yaml", f.Namespace.Name, targetNodeName)
@@ -566,7 +566,7 @@ var _ = SIGDescribe("NFD master and worker", func() {
566566
nfdv1alpha1.FeatureLabelNs + "/fake-fakefeature2": "true",
567567
nfdv1alpha1.FeatureLabelNs + "/fake-fakefeature3": "overridden",
568568
}
569-
eventuallyNonControlPlaneNodes(ctx, f.ClientSet).Should(MatchLabels(expectedLabels, nodes, false))
569+
eventuallyNonControlPlaneNodes(ctx, f.ClientSet).Should(MatchLabels(expectedLabels, nodes))
570570

571571
By("Creating extra namespace")
572572
extraNs, err := f.CreateNamespace(ctx, "node-feature-discvery-extra-ns", nil)
@@ -579,7 +579,7 @@ var _ = SIGDescribe("NFD master and worker", func() {
579579
By("Verifying node labels from NodeFeature object #2 are created")
580580
expectedLabels[targetNodeName][nfdv1alpha1.FeatureLabelNs+"/e2e-nodefeature-test-1"] = "overridden-from-obj-2"
581581
expectedLabels[targetNodeName][nfdv1alpha1.FeatureLabelNs+"/e2e-nodefeature-test-3"] = "obj-2"
582-
eventuallyNonControlPlaneNodes(ctx, f.ClientSet).Should(MatchLabels(expectedLabels, nodes, false))
582+
eventuallyNonControlPlaneNodes(ctx, f.ClientSet).Should(MatchLabels(expectedLabels, nodes))
583583

584584
By("Deleting NodeFeature object from the extra namespace")
585585
err = nfdClient.NfdV1alpha1().NodeFeatures(extraNs.Name).Delete(ctx, nodeFeatures[0], metav1.DeleteOptions{})
@@ -588,7 +588,7 @@ var _ = SIGDescribe("NFD master and worker", func() {
588588
By("Verifying node labels from NodeFeature object were removed")
589589
expectedLabels[targetNodeName][nfdv1alpha1.FeatureLabelNs+"/e2e-nodefeature-test-1"] = "obj-1"
590590
delete(expectedLabels[targetNodeName], nfdv1alpha1.FeatureLabelNs+"/e2e-nodefeature-test-3")
591-
eventuallyNonControlPlaneNodes(ctx, f.ClientSet).Should(MatchLabels(expectedLabels, nodes, false))
591+
eventuallyNonControlPlaneNodes(ctx, f.ClientSet).Should(MatchLabels(expectedLabels, nodes))
592592
})
593593

594594
It("denied labels should not be created by the NodeFeature object", func(ctx context.Context) {
@@ -615,14 +615,14 @@ var _ = SIGDescribe("NFD master and worker", func() {
615615
"custom.vendor.io/e2e-nodefeature-test-3": "vendor-ns",
616616
},
617617
}
618-
eventuallyNonControlPlaneNodes(ctx, f.ClientSet).Should(MatchLabels(expectedLabels, nodes, false))
618+
eventuallyNonControlPlaneNodes(ctx, f.ClientSet).Should(MatchLabels(expectedLabels, nodes))
619619

620620
By("Deleting NodeFeature object")
621621
err = nfdClient.NfdV1alpha1().NodeFeatures(f.Namespace.Name).Delete(ctx, nodeFeatures[0], metav1.DeleteOptions{})
622622
Expect(err).NotTo(HaveOccurred())
623623

624624
expectedLabels[targetNodeName] = k8sLabels{}
625-
eventuallyNonControlPlaneNodes(ctx, f.ClientSet).Should(MatchLabels(expectedLabels, nodes, false))
625+
eventuallyNonControlPlaneNodes(ctx, f.ClientSet).Should(MatchLabels(expectedLabels, nodes))
626626
})
627627
})
628628

@@ -698,7 +698,7 @@ core:
698698
Expect(testutils.CreateNodeFeatureRulesFromFile(ctx, nfdClient, "nodefeaturerule-1.yaml")).NotTo(HaveOccurred())
699699

700700
By("Verifying node labels from NodeFeatureRules #1")
701-
eventuallyNonControlPlaneNodes(ctx, f.ClientSet).Should(MatchLabels(expectedLabels, nodes, false))
701+
eventuallyNonControlPlaneNodes(ctx, f.ClientSet).Should(MatchLabels(expectedLabels, nodes))
702702

703703
By("Creating NodeFeatureRules #2")
704704
Expect(testutils.CreateNodeFeatureRulesFromFile(ctx, nfdClient, "nodefeaturerule-2.yaml")).NotTo(HaveOccurred())
@@ -708,9 +708,14 @@ core:
708708
expectedLabels["*"][nfdv1alpha1.FeatureLabelNs+"/e2e-template-test-1-instance_1"] = "found"
709709
expectedLabels["*"][nfdv1alpha1.FeatureLabelNs+"/e2e-template-test-1-instance_2"] = "found"
710710
expectedLabels["*"][nfdv1alpha1.FeatureLabelNs+"/dynamic-label"] = "true"
711+
expectedAnnotations := map[string]k8sAnnotations{
712+
"*": {
713+
"nfd.node.kubernetes.io/feature-labels": "dynamic-label,e2e-attribute-test-1,e2e-flag-test-1,e2e-instance-test-1,e2e-matchany-test-1,e2e-template-test-1-instance_1,e2e-template-test-1-instance_2"},
714+
}
711715

712716
By("Verifying node labels from NodeFeatureRules #1 and #2")
713-
eventuallyNonControlPlaneNodes(ctx, f.ClientSet).Should(MatchLabels(expectedLabels, nodes, false))
717+
eventuallyNonControlPlaneNodes(ctx, f.ClientSet).Should(MatchLabels(expectedLabels, nodes))
718+
eventuallyNonControlPlaneNodes(ctx, f.ClientSet).Should(MatchAnnotations(expectedAnnotations, nodes))
714719

715720
// Add features from NodeFeatureRule #3
716721
By("Creating NodeFeatureRules #3")
@@ -736,12 +741,10 @@ core:
736741
},
737742
},
738743
}
739-
expectedAnnotations := map[string]k8sAnnotations{
740-
"*": {
741-
"nfd.node.kubernetes.io/taints": "feature.node.kubernetes.io/fake-special-node=exists:PreferNoSchedule,feature.node.kubernetes.io/fake-dedicated-node=true:NoExecute,feature.node.kubernetes.io/performance-optimized-node=true:NoExecute"},
742-
}
743-
eventuallyNonControlPlaneNodes(ctx, f.ClientSet).Should(MatchTaints(expectedTaints, nodes, false))
744-
eventuallyNonControlPlaneNodes(ctx, f.ClientSet).Should(MatchAnnotations(expectedAnnotations, nodes, true))
744+
expectedAnnotations["*"]["nfd.node.kubernetes.io/taints"] = "feature.node.kubernetes.io/fake-special-node=exists:PreferNoSchedule,feature.node.kubernetes.io/fake-dedicated-node=true:NoExecute,feature.node.kubernetes.io/performance-optimized-node=true:NoExecute"
745+
746+
eventuallyNonControlPlaneNodes(ctx, f.ClientSet).Should(MatchTaints(expectedTaints, nodes))
747+
eventuallyNonControlPlaneNodes(ctx, f.ClientSet).Should(MatchAnnotations(expectedAnnotations, nodes))
745748

746749
By("Re-applying NodeFeatureRules #3 with updated taints")
747750
Expect(testutils.UpdateNodeFeatureRulesFromFile(ctx, nfdClient, "nodefeaturerule-3-updated.yaml")).NotTo(HaveOccurred())
@@ -760,16 +763,18 @@ core:
760763
expectedAnnotations["*"]["nfd.node.kubernetes.io/taints"] = "feature.node.kubernetes.io/fake-special-node=exists:PreferNoSchedule,feature.node.kubernetes.io/foo=true:NoExecute"
761764

762765
By("Verifying updated node taints and annotation from NodeFeatureRules #3")
763-
eventuallyNonControlPlaneNodes(ctx, f.ClientSet).Should(MatchTaints(expectedTaints, nodes, false))
764-
eventuallyNonControlPlaneNodes(ctx, f.ClientSet).Should(MatchAnnotations(expectedAnnotations, nodes, true))
766+
eventuallyNonControlPlaneNodes(ctx, f.ClientSet).Should(MatchTaints(expectedTaints, nodes))
767+
eventuallyNonControlPlaneNodes(ctx, f.ClientSet).Should(MatchAnnotations(expectedAnnotations, nodes))
765768

766769
By("Deleting NodeFeatureRule object")
767770
err = nfdClient.NfdV1alpha1().NodeFeatureRules().Delete(ctx, "e2e-test-3", metav1.DeleteOptions{})
768771
Expect(err).NotTo(HaveOccurred())
769772
expectedTaints["*"] = []corev1.Taint{}
770-
eventuallyNonControlPlaneNodes(ctx, f.ClientSet).Should(MatchTaints(expectedTaints, nodes, false))
773+
delete(expectedAnnotations["*"], "nfd.node.kubernetes.io/taints")
774+
eventuallyNonControlPlaneNodes(ctx, f.ClientSet).Should(MatchTaints(expectedTaints, nodes))
775+
eventuallyNonControlPlaneNodes(ctx, f.ClientSet).Should(MatchAnnotations(expectedAnnotations, nodes))
771776

772-
expectedAnnotations["*"] = k8sAnnotations{"nfd.node.kubernetes.io/extended-resources": "nons,vendor.feature.node.kubernetes.io/static,vendor.io/dynamic"}
777+
expectedAnnotations["*"]["nfd.node.kubernetes.io/extended-resources"] = "nons,vendor.feature.node.kubernetes.io/static,vendor.io/dynamic"
773778

774779
expectedCapacity := map[string]corev1.ResourceList{
775780
"*": {
@@ -783,18 +788,20 @@ core:
783788
Expect(testutils.CreateNodeFeatureRulesFromFile(ctx, nfdClient, "nodefeaturerule-4.yaml")).NotTo(HaveOccurred())
784789

785790
By("Verifying node annotations from NodeFeatureRules #4")
786-
eventuallyNonControlPlaneNodes(ctx, f.ClientSet).Should(MatchAnnotations(expectedAnnotations, nodes, true))
791+
eventuallyNonControlPlaneNodes(ctx, f.ClientSet).Should(MatchAnnotations(expectedAnnotations, nodes))
787792

788793
By("Verfiying node status capacity from NodeFeatureRules #4")
789-
eventuallyNonControlPlaneNodes(ctx, f.ClientSet).Should(MatchCapacity(expectedCapacity, nodes, false))
794+
eventuallyNonControlPlaneNodes(ctx, f.ClientSet).Should(MatchCapacity(expectedCapacity, nodes))
790795

791796
By("Deleting NodeFeatureRule object")
792797
err = nfdClient.NfdV1alpha1().NodeFeatureRules().Delete(ctx, "e2e-extened-resource-test", metav1.DeleteOptions{})
793798
Expect(err).NotTo(HaveOccurred())
794799

795800
By("Verfiying node status capacity from NodeFeatureRules #4")
796801
expectedCapacity = map[string]corev1.ResourceList{"*": {}}
797-
eventuallyNonControlPlaneNodes(ctx, f.ClientSet).Should(MatchCapacity(expectedCapacity, nodes, false))
802+
delete(expectedAnnotations["*"], "nfd.node.kubernetes.io/extended-resources")
803+
eventuallyNonControlPlaneNodes(ctx, f.ClientSet).Should(MatchCapacity(expectedCapacity, nodes))
804+
eventuallyNonControlPlaneNodes(ctx, f.ClientSet).Should(MatchAnnotations(expectedAnnotations, nodes))
798805

799806
By("Deleting nfd-worker daemonset")
800807
err = f.ClientSet.AppsV1().DaemonSets(f.Namespace.Name).Delete(ctx, workerDS.Name, metav1.DeleteOptions{})
@@ -838,7 +845,7 @@ denyLabelNs: ["*.denied.ns","random.unwanted.ns"]
838845
"custom.vendor.io/e2e-nodefeature-test-3": "vendor-ns",
839846
},
840847
}
841-
eventuallyNonControlPlaneNodes(ctx, f.ClientSet).Should(MatchLabels(expectedLabels, nodes, false))
848+
eventuallyNonControlPlaneNodes(ctx, f.ClientSet).Should(MatchLabels(expectedLabels, nodes))
842849
By("Deleting NodeFeature object")
843850
err = nfdClient.NfdV1alpha1().NodeFeatures(f.Namespace.Name).Delete(ctx, nodeFeatures[0], metav1.DeleteOptions{})
844851
Expect(err).NotTo(HaveOccurred())
@@ -859,7 +866,7 @@ denyLabelNs: []
859866
"random.unwanted.ns/e2e-nodefeature-test-2": "unwanted-ns",
860867
},
861868
}
862-
eventuallyNonControlPlaneNodes(ctx, f.ClientSet).Should(MatchLabels(expectedLabels, nodes, false))
869+
eventuallyNonControlPlaneNodes(ctx, f.ClientSet).Should(MatchLabels(expectedLabels, nodes))
863870
})
864871
})
865872

@@ -900,7 +907,7 @@ resyncPeriod: "1s"
900907
},
901908
"*": {},
902909
}
903-
eventuallyNonControlPlaneNodes(ctx, f.ClientSet).Should(MatchLabels(expectedLabels, nodes, false))
910+
eventuallyNonControlPlaneNodes(ctx, f.ClientSet).Should(MatchLabels(expectedLabels, nodes))
904911

905912
patches, err := json.Marshal(
906913
[]apihelper.JsonPatch{
@@ -917,7 +924,7 @@ resyncPeriod: "1s"
917924
_, err = f.ClientSet.CoreV1().Nodes().Patch(ctx, targetNodeName, types.JSONPatchType, patches, metav1.PatchOptions{})
918925
Expect(err).NotTo(HaveOccurred())
919926

920-
eventuallyNonControlPlaneNodes(ctx, f.ClientSet).Should(MatchLabels(expectedLabels, nodes, false))
927+
eventuallyNonControlPlaneNodes(ctx, f.ClientSet).Should(MatchLabels(expectedLabels, nodes))
921928

922929
By("Deleting NodeFeature object")
923930
err = nfdClient.NfdV1alpha1().NodeFeatures(f.Namespace.Name).Delete(ctx, nodeFeatures[0], metav1.DeleteOptions{})

0 commit comments

Comments
 (0)