Merge pull request #1198 from marquiz/devel/nfd-api-controller

nfd-master: refactor api-controller object handling

pkg/nfd-master/nfd-api-controller.go

@@ -17,8 +17,10 @@ limitations under the License.
 package nfdmaster
 
 import (
+	"fmt"
 	"time"
 
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
 	restclient "k8s.io/client-go/rest"
 	"k8s.io/client-go/tools/cache"
@@ -61,19 +63,19 @@ func newNfdController(config *restclient.Config, nfdApiControllerOptions nfdApiC
 		featureInformer := informerFactory.Nfd().V1alpha1().NodeFeatures()
 		if _, err := featureInformer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{
 			AddFunc: func(obj interface{}) {
-				key, _ := cache.MetaNamespaceKeyFunc(obj)
-				klog.V(2).Infof("NodeFeature %v added", key)
-				c.updateOneNode(obj)
+				nfr := obj.(*nfdv1alpha1.NodeFeature)
+				klog.V(2).Infof("NodeFeature %s/%s added", nfr.Namespace, nfr.Name)
+				c.updateOneNode("NodeFeature", nfr)
 			},
 			UpdateFunc: func(oldObj, newObj interface{}) {
-				key, _ := cache.MetaNamespaceKeyFunc(newObj)
-				klog.V(2).Infof("NodeFeature %v updated", key)
-				c.updateOneNode(newObj)
+				nfr := newObj.(*nfdv1alpha1.NodeFeature)
+				klog.V(2).Infof("NodeFeature %s/%s updated", nfr.Namespace, nfr.Name)
+				c.updateOneNode("NodeFeature", nfr)
 			},
 			DeleteFunc: func(obj interface{}) {
-				key, _ := cache.MetaNamespaceKeyFunc(obj)
-				klog.V(2).Infof("NodeFeature %v deleted", key)
-				c.updateOneNode(obj)
+				nfr := obj.(*nfdv1alpha1.NodeFeature)
+				klog.V(2).Infof("NodeFeature %s/%s deleted", nfr.Namespace, nfr.Name)
+				c.updateOneNode("NodeFeature", nfr)
 			},
 		}); err != nil {
 			return nil, err
@@ -128,26 +130,24 @@ func (c *nfdController) stop() {
 	}
 }
 
-func (c *nfdController) updateOneNode(obj interface{}) {
-	o, ok := obj.(*nfdv1alpha1.NodeFeature)
-	if !ok {
-		klog.Errorf("not a NodeFeature object (but of type %T): %v", obj, obj)
+func (c *nfdController) updateOneNode(typ string, obj metav1.Object) {
+	nodeName, err := getNodeNameForObj(obj)
+	if err != nil {
+		klog.Errorf("failed to determine node name for %s %s/%s: %v", typ, obj.GetNamespace(), obj.GetName(), err)
 		return
 	}
+	c.updateOneNodeChan <- nodeName
+}
 
-	nodeName, ok := o.Labels[nfdv1alpha1.NodeFeatureObjNodeNameLabel]
+func getNodeNameForObj(obj metav1.Object) (string, error) {
+	nodeName, ok := obj.GetLabels()[nfdv1alpha1.NodeFeatureObjNodeNameLabel]
 	if !ok {
-		klog.Errorf("no node name for NodeFeature object %s/%s: %q label is missing",
-			o.Namespace, o.Name, nfdv1alpha1.NodeFeatureObjNodeNameLabel)
-		return
+		return "", fmt.Errorf("%q label is missing", nfdv1alpha1.NodeFeatureObjNodeNameLabel)
 	}
 	if nodeName == "" {
-		klog.Errorf("no node name for NodeFeature object %s/%s: %q label is empty",
-			o.Namespace, o.Name, nfdv1alpha1.NodeFeatureObjNodeNameLabel)
-		return
+		return "", fmt.Errorf("%q label is empty", nfdv1alpha1.NodeFeatureObjNodeNameLabel)
 	}
-
-	c.updateOneNodeChan <- nodeName
+	return nodeName, nil
 }
 
 func (c *nfdController) updateAllNodes() {

pkg/nfd-master/nfd-api-controller_test.go

@@ -0,0 +1,44 @@
+/*
+Copyright 2023 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package nfdmaster
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	nfdv1alpha1 "sigs.k8s.io/node-feature-discovery/pkg/apis/nfd/v1alpha1"
+)
+
+func TestGetNodeNameForObj(t *testing.T) {
+	// Test missing label
+	obj := &nfdv1alpha1.NodeFeature{ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "bar"}}
+	_, err := getNodeNameForObj(obj)
+	assert.Error(t, err)
+
+	// Test empty label
+	obj.SetLabels(map[string]string{nfdv1alpha1.NodeFeatureObjNodeNameLabel: ""})
+	_, err = getNodeNameForObj(obj)
+	assert.Error(t, err)
+
+	// Test proper node name
+	obj.SetLabels(map[string]string{nfdv1alpha1.NodeFeatureObjNodeNameLabel: "node-1"})
+	n, err := getNodeNameForObj(obj)
+	assert.Nil(t, err)
+	assert.Equal(t, n, "node-1")
+}