Advertise NFD version as an Annotation instead of a Label

Add new 'nfd.node.kubernetes.io/version' annotation for advertising the
version of NFD that created the feature labels on the node. Introduces
new 'nfd.node.kubernetes.io' namespace that is supposed to be used by
all future NFD annotations. The old
'node.alpha.kubernetes-incubator.io/node-feature-discovery.version' is
dropped in favor of the new annotation.

Annotations are better suited for this kind of metadata. NFD version
should not be used for pod scheduling, especially because all the nodes
in the cluster should normally run the same version of NFD.

main.go

@@ -35,15 +35,18 @@ const (
 	ProgramName = "node-feature-discovery"
 
 	// Namespace is the prefix for all published labels.
-	Namespace = "feature.node.kubernetes.io"
+	labelNs = "feature.node.kubernetes.io/"
+
+	// Namespace is the prefix for all published labels.
+	annotationNs = "nfd.node.kubernetes.io/"
 
 	// NodeNameEnv is the environment variable that contains this node's name.
 	NodeNameEnv = "NODE_NAME"
 )
 
 var (
-	version = "" // Must not be const, set using ldflags at build time
-	prefix  = fmt.Sprintf("%s/nfd", Namespace)
+	version     = "" // Must not be const, set using ldflags at build time
+	labelPrefix = labelNs + "nfd"
 )
 
 // package loggers
@@ -64,6 +67,9 @@ var config = NFDConfig{}
 // Labels are a Kubernetes representation of discovered features.
 type Labels map[string]string
 
+// Annotations are used for NFD-related node metadata
+type Annotations map[string]string
+
 // APIHelpers represents a set of API helpers for Kubernetes
 type APIHelpers interface {
 	// GetClient returns a client
@@ -82,6 +88,9 @@ type APIHelpers interface {
 	// API server using the client library.
 	AddLabels(*api.Node, Labels)
 
+	// Add annotations
+	AddAnnotations(*api.Node, Annotations)
+
 	// UpdateNode updates the node via the API server using a client.
 	UpdateNode(*k8sclient.Clientset, *api.Node) error
 }
@@ -102,6 +111,7 @@ func main() {
 	if version == "" {
 		stderrLogger.Fatalf("main.version not set! Set -ldflags \"-X main.version `git describe --tags --dirty --always`\" during build or run.")
 	}
+	stdoutLogger.Printf("Node Feature Discovery %s", version)
 
 	// Parse command-line arguments.
 	args := argsParse(nil)
@@ -276,12 +286,6 @@ func configureParameters(sourcesWhiteList []string, labelWhiteListStr string) (e
 // sources and the whitelist argument.
 func createFeatureLabels(sources []source.FeatureSource, labelWhiteList *regexp.Regexp) (labels Labels) {
 	labels = Labels{}
-	// Add the version of this discovery code as a node label
-	versionLabel := fmt.Sprintf("%s/%s.version", Namespace, ProgramName)
-	labels[versionLabel] = version
-
-	// Log version label.
-	stdoutLogger.Printf("%s = %s", versionLabel, version)
 
 	// Do feature discovery from all configured sources.
 	for _, source := range sources {
@@ -310,7 +314,10 @@ func createFeatureLabels(sources []source.FeatureSource, labelWhiteList *regexp.
 // disabled via --no-publish flag.
 func updateNodeWithFeatureLabels(helper APIHelpers, noPublish bool, labels Labels) error {
 	if !noPublish {
-		err := advertiseFeatureLabels(helper, labels)
+		// Advertise NFD version as an annotation
+		annotations := Annotations{"version": version}
+
+		err := advertiseFeatureLabels(helper, labels, annotations)
 		if err != nil {
 			stderrLogger.Printf("failed to advertise labels: %s", err.Error())
 			return err
@@ -335,14 +342,14 @@ func getFeatureLabels(source source.FeatureSource) (labels Labels, err error) {
 		return nil, err
 	}
 	for k := range features {
-		labels[fmt.Sprintf("%s-%s-%s", prefix, source.Name(), k)] = fmt.Sprintf("%v", features[k])
+		labels[fmt.Sprintf("%s-%s-%s", labelPrefix, source.Name(), k)] = fmt.Sprintf("%v", features[k])
 	}
 	return labels, nil
 }
 
 // advertiseFeatureLabels advertises the feature labels to a Kubernetes node
 // via the API server.
-func advertiseFeatureLabels(helper APIHelpers, labels Labels) error {
+func advertiseFeatureLabels(helper APIHelpers, labels Labels, annotations Annotations) error {
 	cli, err := helper.GetClient()
 	if err != nil {
 		stderrLogger.Printf("can't get kubernetes client: %s", err.Error())
@@ -357,10 +364,13 @@ func advertiseFeatureLabels(helper APIHelpers, labels Labels) error {
 	}
 
 	// Remove labels with our prefix
-	helper.RemoveLabels(node, prefix)
+	helper.RemoveLabels(node, labelPrefix)
 	// Add labels to the node object.
 	helper.AddLabels(node, labels)
 
+	// Add annotations
+	helper.AddAnnotations(node, annotations)
+
 	// Send the updated node to the apiserver.
 	err = helper.UpdateNode(cli, node)
 	if err != nil {
@@ -418,6 +428,13 @@ func (h k8sHelpers) AddLabels(n *api.Node, labels Labels) {
 	}
 }
 
+// Add Annotations to the Node object
+func (h k8sHelpers) AddAnnotations(n *api.Node, annotations Annotations) {
+	for k, v := range annotations {
+		n.Annotations[annotationNs+k] = v
+	}
+}
+
 func (h k8sHelpers) UpdateNode(c *k8sclient.Clientset, n *api.Node) error {
 	// Send the updated node to the apiserver.
 	_, err := c.Core().Nodes().Update(n)

main_test.go

@@ -25,9 +25,10 @@ func TestDiscoveryWithMockSources(t *testing.T) {
 		fakeFeatureNames := []string{"testfeature1", "testfeature2", "testfeature3"}
 		fakeFeatures := source.Features{}
 		fakeFeatureLabels := Labels{}
+		fakeAnnotations := Annotations{"version": version}
 		for _, f := range fakeFeatureNames {
 			fakeFeatures[f] = true
-			fakeFeatureLabels[fmt.Sprintf("%s-testSource-%s", prefix, f)] = "true"
+			fakeFeatureLabels[fmt.Sprintf("%s-testSource-%s", labelPrefix, f)] = "true"
 		}
 		fakeFeatureSource := source.FeatureSource(mockFeatureSource)
 
@@ -66,7 +67,8 @@ func TestDiscoveryWithMockSources(t *testing.T) {
 			mockAPIHelper.On("GetClient").Return(mockClient, nil)
 			mockAPIHelper.On("GetNode", mockClient).Return(mockNode, nil).Once()
 			mockAPIHelper.On("AddLabels", mockNode, fakeFeatureLabels).Return().Once()
-			mockAPIHelper.On("RemoveLabels", mockNode, prefix).Return().Once()
+			mockAPIHelper.On("RemoveLabels", mockNode, labelPrefix).Return().Once()
+			mockAPIHelper.On("AddAnnotations", mockNode, fakeAnnotations).Return().Once()
 			mockAPIHelper.On("UpdateNode", mockClient, mockNode).Return(nil).Once()
 			noPublish := false
 			err := updateNodeWithFeatureLabels(testHelper, noPublish, fakeFeatureLabels)
@@ -90,7 +92,7 @@ func TestDiscoveryWithMockSources(t *testing.T) {
 		Convey("When I fail to get a mock client while advertising feature labels", func() {
 			expectedError := errors.New("fake error")
 			mockAPIHelper.On("GetClient").Return(nil, expectedError)
-			err := advertiseFeatureLabels(testHelper, fakeFeatureLabels)
+			err := advertiseFeatureLabels(testHelper, fakeFeatureLabels, fakeAnnotations)
 
 			Convey("Error is produced", func() {
 				So(err, ShouldEqual, expectedError)
@@ -101,7 +103,7 @@ func TestDiscoveryWithMockSources(t *testing.T) {
 			expectedError := errors.New("fake error")
 			mockAPIHelper.On("GetClient").Return(mockClient, nil)
 			mockAPIHelper.On("GetNode", mockClient).Return(nil, expectedError).Once()
-			err := advertiseFeatureLabels(testHelper, fakeFeatureLabels)
+			err := advertiseFeatureLabels(testHelper, fakeFeatureLabels, fakeAnnotations)
 
 			Convey("Error is produced", func() {
 				So(err, ShouldEqual, expectedError)
@@ -112,10 +114,11 @@ func TestDiscoveryWithMockSources(t *testing.T) {
 			expectedError := errors.New("fake error")
 			mockAPIHelper.On("GetClient").Return(mockClient, nil)
 			mockAPIHelper.On("GetNode", mockClient).Return(mockNode, nil).Once()
-			mockAPIHelper.On("RemoveLabels", mockNode, prefix).Return().Once()
+			mockAPIHelper.On("RemoveLabels", mockNode, labelPrefix).Return().Once()
 			mockAPIHelper.On("AddLabels", mockNode, fakeFeatureLabels).Return().Once()
+			mockAPIHelper.On("AddAnnotations", mockNode, fakeAnnotations).Return().Once()
 			mockAPIHelper.On("UpdateNode", mockClient, mockNode).Return(expectedError).Once()
-			err := advertiseFeatureLabels(testHelper, fakeFeatureLabels)
+			err := advertiseFeatureLabels(testHelper, fakeFeatureLabels, fakeAnnotations)
 
 			Convey("Error is produced", func() {
 				So(err, ShouldEqual, expectedError)
@@ -281,10 +284,10 @@ func TestCreateFeatureLabels(t *testing.T) {
 			labels := createFeatureLabels(sources, emptyLabelWL)
 
 			Convey("Proper fake labels are returned", func() {
-				So(len(labels), ShouldEqual, 4)
-				So(labels, ShouldContainKey, prefix+"-fake-fakefeature1")
-				So(labels, ShouldContainKey, prefix+"-fake-fakefeature2")
-				So(labels, ShouldContainKey, prefix+"-fake-fakefeature3")
+				So(len(labels), ShouldEqual, 3)
+				So(labels, ShouldContainKey, labelPrefix+"-fake-fakefeature1")
+				So(labels, ShouldContainKey, labelPrefix+"-fake-fakefeature2")
+				So(labels, ShouldContainKey, labelPrefix+"-fake-fakefeature3")
 			})
 		})
 		Convey("When fake feature source is configured with a whitelist that doesn't match", func() {
@@ -295,10 +298,10 @@ func TestCreateFeatureLabels(t *testing.T) {
 			labels := createFeatureLabels(sources, emptyLabelWL)
 
 			Convey("fake labels are not returned", func() {
-				So(len(labels), ShouldEqual, 1)
-				So(labels, ShouldNotContainKey, prefix+"-fake-fakefeature1")
-				So(labels, ShouldNotContainKey, prefix+"-fake-fakefeature2")
-				So(labels, ShouldNotContainKey, prefix+"-fake-fakefeature3")
+				So(len(labels), ShouldEqual, 0)
+				So(labels, ShouldNotContainKey, labelPrefix+"-fake-fakefeature1")
+				So(labels, ShouldNotContainKey, labelPrefix+"-fake-fakefeature2")
+				So(labels, ShouldNotContainKey, labelPrefix+"-fake-fakefeature3")
 			})
 		})
 	})
@@ -323,7 +326,7 @@ func TestAddLabels(t *testing.T) {
 		})
 
 		Convey("They should be added to the node.Labels", func() {
-			test1 := prefix + ".test1"
+			test1 := labelPrefix + ".test1"
 			labels[test1] = "true"
 			helper.AddLabels(n, labels)
 			So(n.Labels, ShouldContainKey, test1)

mockapihelpers.go

@@ -70,6 +70,12 @@ func (_m *MockAPIHelpers) AddLabels(_a0 *api.Node, _a1 Labels) {
 	_m.Called(_a0, _a1)
 }
 
+// AddAnnotations provides a mock function with *api.Node and main.Annotations as the input arguments and
+// no return value
+func (_m *MockAPIHelpers) AddAnnotations(_a0 *api.Node, _a1 Annotations) {
+	_m.Called(_a0, _a1)
+}
+
 // UpdateNode provides a mock function with *k8sclient.Clientset and *api.Node as the input arguments and
 // error as the return value
 func (_m *MockAPIHelpers) UpdateNode(_a0 *k8sclient.Clientset, _a1 *api.Node) error {