Merge pull request #1394 from marquiz/devel/annotations

nfd-master: stop creating NFD version annotations

docs/get-started/introduction.md

@@ -102,16 +102,15 @@ NFD also annotates nodes it is running on:
 
 | Annotation                                                   | Description
 | ------------------------------------------------------------ | -----------
-| [<instance>.]nfd.node.kubernetes.io/master.version     | Version of the nfd-master instance running on the node. Informative use only.
-| [<instance>.]nfd.node.kubernetes.io/worker.version     | Version of the nfd-worker instance running on the node. Informative use only.
 | [<instance>.]nfd.node.kubernetes.io/feature-labels     | Comma-separated list of node labels managed by NFD. NFD uses this internally so must not be edited by users.
 | [<instance>.]nfd.node.kubernetes.io/extended-resources | Comma-separated list of node extended resources managed by NFD. NFD uses this internally so must not be edited by users.
 
 > **NOTE:** the [`-instance`](../reference/master-commandline-reference.md#instance)
 > command line flag affects the annotation names
 
-Unapplicable annotations are not created, i.e. for example master.version is
-only created on nodes running nfd-master.
+Unapplicable annotations are not created, i.e. for example
+`nfd.node.kubernetes.io/extended-resources` is only placed if some extended
+resources were created by NFD.
 
 ## Custom resources
 

pkg/apis/nfd/v1alpha1/annotations_labels.go

@@ -51,6 +51,7 @@ const (
 	FeatureLabelsAnnotation = AnnotationNs + "/feature-labels"
 
 	// MasterVersionAnnotation is the annotation that holds the version of nfd-master running on the node
+	// DEPRECATED: will not be used in NFD v0.15 or later.
 	MasterVersionAnnotation = AnnotationNs + "/master.version"
 
 	// WorkerVersionAnnotation is the annotation that holds the version of nfd-worker running on the node

pkg/nfd-master/nfd-master-internal_test.go

@@ -44,7 +44,6 @@ import (
 	nfdinformers "sigs.k8s.io/node-feature-discovery/pkg/generated/informers/externalversions"
 	"sigs.k8s.io/node-feature-discovery/pkg/labeler"
 	"sigs.k8s.io/node-feature-discovery/pkg/utils"
-	"sigs.k8s.io/node-feature-discovery/pkg/version"
 	"sigs.k8s.io/yaml"
 )
 
@@ -238,8 +237,7 @@ func TestUpdateMasterNode(t *testing.T) {
 		mockClient := &k8sclient.Clientset{}
 		mockNode := newMockNode()
 		Convey("When update operation succeeds", func() {
-			expectedPatches := []apihelper.JsonPatch{
-				apihelper.NewJsonPatch("add", "/metadata/annotations", nfdv1alpha1.AnnotationNs+"/master.version", version.Get())}
+			expectedPatches := []apihelper.JsonPatch{}
 			mockHelper.On("GetClient").Return(mockClient, nil)
 			mockHelper.On("GetNode", mockClient, mockNodeName).Return(mockNode, nil)
 			mockHelper.On("PatchNode", mockClient, mockNodeName, mock.MatchedBy(jsonPatchMatcher(expectedPatches))).Return(nil)
@@ -378,7 +376,6 @@ func TestSetLabels(t *testing.T) {
 
 		Convey("When node update succeeds", func() {
 			expectedPatches := []apihelper.JsonPatch{
-				apihelper.NewJsonPatch("add", "/metadata/annotations", nfdv1alpha1.WorkerVersionAnnotation, workerVer),
 				apihelper.NewJsonPatch("add", "/metadata/annotations", nfdv1alpha1.FeatureLabelsAnnotation, strings.Join(mockLabelNames, ",")),
 			}
 			for k, v := range mockLabels {
@@ -397,7 +394,6 @@ func TestSetLabels(t *testing.T) {
 
 		Convey("When -label-whitelist is specified", func() {
 			expectedPatches := []apihelper.JsonPatch{
-				apihelper.NewJsonPatch("add", "/metadata/annotations", nfdv1alpha1.WorkerVersionAnnotation, workerVer),
 				apihelper.NewJsonPatch("add", "/metadata/annotations", nfdv1alpha1.FeatureLabelsAnnotation, "feature-2"),
 				apihelper.NewJsonPatch("add", "/metadata/labels", nfdv1alpha1.FeatureLabelNs+"/feature-2", mockLabels["feature-2"]),
 			}
@@ -429,7 +425,6 @@ func TestSetLabels(t *testing.T) {
 				"--invalid-name--":               "valid-val",
 				"valid-name":                     "--invalid-val--"}
 			expectedPatches := []apihelper.JsonPatch{
-				apihelper.NewJsonPatch("add", "/metadata/annotations", instance+"."+nfdv1alpha1.WorkerVersionAnnotation, workerVer),
 				apihelper.NewJsonPatch("add", "/metadata/annotations",
 					instance+"."+nfdv1alpha1.FeatureLabelsAnnotation,
 					"feature-1,valid.ns/feature-2,"+vendorFeatureLabel+","+vendorProfileLabel),
@@ -457,7 +452,6 @@ func TestSetLabels(t *testing.T) {
 
 		Convey("When -resource-labels is specified", func() {
 			expectedPatches := []apihelper.JsonPatch{
-				apihelper.NewJsonPatch("add", "/metadata/annotations", nfdv1alpha1.WorkerVersionAnnotation, workerVer),
 				apihelper.NewJsonPatch("add", "/metadata/annotations", nfdv1alpha1.FeatureLabelsAnnotation, "feature-2"),
 				apihelper.NewJsonPatch("add", "/metadata/annotations", nfdv1alpha1.ExtendedResourceAnnotation, "feature-1,feature-3"),
 				apihelper.NewJsonPatch("add", "/metadata/labels", nfdv1alpha1.FeatureLabelNs+"/feature-2", mockLabels["feature-2"]),

pkg/nfd-master/nfd-master.go

@@ -479,7 +479,10 @@ func (m *nfdMaster) prune() error {
 	return nil
 }
 
-// Advertise NFD master information
+// Update annotations on the node where nfd-master is running. Currently the
+// only function is to remove the deprecated
+// "nfd.node.kubernetes.io/master.version" annotation, if it exists.
+// TODO: Drop when nfdv1alpha1.MasterVersionAnnotation is removed.
 func (m *nfdMaster) updateMasterNode() error {
 	cli, err := m.apihelper.GetClient()
 	if err != nil {
@@ -491,9 +494,9 @@ func (m *nfdMaster) updateMasterNode() error {
 	}
 
 	// Advertise NFD version as an annotation
-	p := createPatches(nil,
+	p := createPatches([]string{m.instanceAnnotation(nfdv1alpha1.MasterVersionAnnotation)},
 		node.Annotations,
-		Annotations{m.instanceAnnotation(nfdv1alpha1.MasterVersionAnnotation): version.Get()},
+		nil,
 		"/metadata/annotations")
 	err = m.apihelper.PatchNode(cli, node.Name, p)
 	if err != nil {
@@ -680,8 +683,7 @@ func (m *nfdMaster) SetLabels(c context.Context, r *pb.SetLabelsRequest) (*pb.Se
 			return &pb.SetLabelsReply{}, err
 		}
 
-		// Advertise NFD worker version as an annotation
-		annotations := Annotations{m.instanceAnnotation(nfdv1alpha1.WorkerVersionAnnotation): r.NfdVersion}
+		annotations := Annotations{}
 
 		// Create labels et al
 		if err := m.refreshNodeFeatures(cli, r.NodeName, annotations, r.GetLabels(), r.GetFeatures()); err != nil {
@@ -761,13 +763,6 @@ func (m *nfdMaster) nfdAPIUpdateOneNode(nodeName string) error {
 		}
 
 		klog.V(4).InfoS("merged nodeFeatureSpecs", "newNodeFeatureSpec", utils.DelayedDumper(features))
-
-		if objs[0].Namespace == m.namespace && objs[0].Name == nodeName {
-			// This is the one created by nfd-worker
-			if v := objs[0].Annotations[nfdv1alpha1.WorkerVersionAnnotation]; v != "" {
-				annotations[nfdv1alpha1.WorkerVersionAnnotation] = v
-			}
-		}
 	}
 
 	// Update node labels et al. This may also mean removing all NFD-owned
@@ -1067,7 +1062,10 @@ func (m *nfdMaster) updateNodeObject(cli *kubernetes.Clientset, nodeName string,
 		createPatches(
 			[]string{
 				m.instanceAnnotation(nfdv1alpha1.FeatureLabelsAnnotation),
-				m.instanceAnnotation(nfdv1alpha1.ExtendedResourceAnnotation)},
+				m.instanceAnnotation(nfdv1alpha1.ExtendedResourceAnnotation),
+				// Clean up deprecated/stale nfd version annotations
+				m.instanceAnnotation(nfdv1alpha1.MasterVersionAnnotation),
+				m.instanceAnnotation(nfdv1alpha1.WorkerVersionAnnotation)},
 			node.Annotations,
 			annotations,
 			"/metadata/annotations")...)

test/e2e/e2e-test-config.example.yaml

@@ -73,8 +73,6 @@ defaultFeatures:
     - "feature.node.kubernetes.io/system-os_release.VERSION_ID.major"
     - "feature.node.kubernetes.io/system-os_release.VERSION_ID.minor"
   annotationWhitelist:
-    - "nfd.node.kubernetes.io/master.version"
-    - "nfd.node.kubernetes.io/worker.version"
     - "nfd.node.kubernetes.io/feature-labels"
   nodes:
     - name: name-of-this-item  # name is purely informational

test/e2e/node_feature_discovery_test.go

@@ -249,16 +249,6 @@ var _ = SIGDescribe("NFD master and worker", func() {
 
 				By("Waiting for the nfd-master pod to be running")
 				Expect(e2epod.WaitTimeoutForPodRunningInNamespace(ctx, f.ClientSet, masterPod.Name, masterPod.Namespace, time.Minute)).NotTo(HaveOccurred())
-
-				By("Verifying the node where nfd-master is running")
-				// Get updated masterPod object (we want to know where it was scheduled)
-				masterPod, err = f.ClientSet.CoreV1().Pods(f.Namespace.Name).Get(ctx, masterPod.Name, metav1.GetOptions{})
-				Expect(err).NotTo(HaveOccurred())
-				// Node running nfd-master should have master version annotation
-				masterPodNode, err := f.ClientSet.CoreV1().Nodes().Get(ctx, masterPod.Spec.NodeName, metav1.GetOptions{})
-				Expect(err).NotTo(HaveOccurred())
-				Expect(masterPodNode.Annotations).To(HaveKey(nfdv1alpha1.AnnotationNs + "/master.version"))
-
 				By("Waiting for the nfd-master service to be up")
 				Expect(e2enetwork.WaitForService(ctx, f.ClientSet, f.Namespace.Name, nfdSvc.Name, true, time.Second, 10*time.Second)).NotTo(HaveOccurred())
 			})