nfd-master: parse kubeconfig even with NoPublish set

Don't try to be too smart when kubeconfig is needed. In practice, the
nfd-master really doesn't work anymore (with the NodeFeature API
enabled) without a kubeconfig set. This patch fixes crashes happening
when NoPublish is enabled, e.g. in listing all nodes in the nfd api
handler and in getting single node objects in the node updater pool.

This patch changes the kubeconfig parsing to happen at the creation of
the nfd-master instance. We don't need to do that at reconfigure time as
none of the dynamic config options affect it. Unit tests are adjusted,
accordingly.

Author: marquiz

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

@@ -40,7 +40,6 @@ import (
 	fakecorev1client "k8s.io/client-go/kubernetes/typed/core/v1/fake"
 	clienttesting "k8s.io/client-go/testing"
 	"k8s.io/client-go/tools/cache"
-	"k8s.io/klog/v2"
 
 	nfdv1alpha1 "sigs.k8s.io/node-feature-discovery/pkg/apis/nfd/v1alpha1"
 	"sigs.k8s.io/node-feature-discovery/pkg/features"
@@ -115,6 +114,7 @@ func newFakeMaster(opts ...NfdMasterOption) *nfdMaster {
 	defaultOpts := []NfdMasterOption{
 		withNodeName(testNodeName),
 		withConfig(&NFDConfig{}),
+		WithKubernetesClient(fakeclient.NewSimpleClientset()),
 	}
 	m, err := NewNfdMaster(append(defaultOpts, opts...)...)
 	if err != nil {
@@ -664,6 +664,10 @@ leaderElection:
 
 func TestDynamicConfig(t *testing.T) {
 	Convey("When running nfd-master", t, func() {
+		// Add feature gates as running nfd-master depends on that
+		err := features.NFDMutableFeatureGate.Add(features.DefaultNFDFeatureGates)
+		So(err, ShouldBeNil)
+
 		tmpDir, err := os.MkdirTemp("", "*.nfd-test")
 		So(err, ShouldBeNil)
 		defer os.RemoveAll(tmpDir)
@@ -674,7 +678,7 @@ func TestDynamicConfig(t *testing.T) {
 		So(err, ShouldBeNil)
 
 		// Create config file
-		configFile := filepath.Join(configDir, "master.conf")
+		configFile := filepath.Clean(filepath.Join(configDir, "master.conf"))
 
 		writeConfig := func(data string) {
 			f, err := os.Create(configFile)
@@ -685,25 +689,22 @@ func TestDynamicConfig(t *testing.T) {
 			So(err, ShouldBeNil)
 		}
 		writeConfig(`
+klog:
+  v: "4"
 extraLabelNs: ["added.ns.io"]
 `)
 
-		noPublish := true
-		// Add FeatureGates flag
-		if err := features.NFDMutableFeatureGate.Add(features.DefaultNFDFeatureGates); err != nil {
-			klog.ErrorS(err, "failed to add default feature gates")
-			os.Exit(1)
-		}
-		_ = features.NFDMutableFeatureGate.OverrideDefault(features.NodeFeatureAPI, false)
-		master := newFakeMaster(WithArgs(&Args{
-			ConfigFile: configFile,
-			Overrides: ConfigOverrideArgs{
-				NoPublish: &noPublish,
-			},
-		}))
+		master := newFakeMaster(
+			WithArgs(&Args{ConfigFile: configFile}),
+			WithKubernetesClient(fakeclient.NewSimpleClientset(newTestNode())))
 
 		Convey("config file updates should take effect", func() {
-			go func() { _ = master.Run() }()
+			go func() {
+				Convey("nfd-master should exit gracefully", t, func() {
+					err = master.Run()
+					So(err, ShouldBeNil)
+				})
+			}()
 			defer master.Stop()
 			// Check initial config
 			time.Sleep(10 * time.Second)

pkg/nfd-master/nfd-master.go

@@ -194,6 +194,19 @@ func NewNfdMaster(opts ...NfdMasterOption) (NfdMaster, error) {
 		nfd.configFilePath = filepath.Clean(nfd.args.ConfigFile)
 	}
 
+	// k8sClient might've been set via opts by tests
+	if nfd.k8sClient == nil {
+		kubeconfig, err := utils.GetKubeconfig(nfd.args.Kubeconfig)
+		if err != nil {
+			return nfd, err
+		}
+		cli, err := k8sclient.NewForConfig(kubeconfig)
+		if err != nil {
+			return nfd, err
+		}
+		nfd.k8sClient = cli
+	}
+
 	nfd.nodeUpdaterPool = newNodeUpdaterPool(nfd)
 
 	return nfd, nil
@@ -770,10 +783,6 @@ func (m *nfdMaster) nfdAPIUpdateOneNode(node *corev1.Node) error {
 		return objs[i].Namespace < objs[j].Namespace
 	})
 
-	if m.config.NoPublish {
-		return nil
-	}
-
 	klog.V(1).InfoS("processing of node initiated by NodeFeature API", "nodeName", node.Name)
 
 	features := nfdv1alpha1.NewNodeFeatureSpec()
@@ -875,6 +884,11 @@ func (m *nfdMaster) refreshNodeFeatures(node *corev1.Node, labels map[string]str
 		taints = filterTaints(crTaints)
 	}
 
+	if m.config.NoPublish {
+		klog.V(1).InfoS("node update skipped, NoPublish=true", "nodeName", node.Name)
+		return nil
+	}
+
 	err := m.updateNodeObject(node, labels, annotations, extendedResources, taints)
 	if err != nil {
 		klog.ErrorS(err, "failed to update node", "nodeName", node.Name)
@@ -1250,18 +1264,6 @@ func (m *nfdMaster) configure(filepath string, overrides string) error {
 		return err
 	}
 
-	if !c.NoPublish {
-		kubeconfig, err := utils.GetKubeconfig(m.args.Kubeconfig)
-		if err != nil {
-			return err
-		}
-		cli, err := k8sclient.NewForConfig(kubeconfig)
-		if err != nil {
-			return err
-		}
-		m.k8sClient = cli
-	}
-
 	// Pre-process DenyLabelNS into 2 lists: one for normal ns, and the other for wildcard ns
 	normalDeniedNs, wildcardDeniedNs := preProcessDeniedNamespaces(c.DenyLabelNs)
 	m.deniedNs.normal = normalDeniedNs

pkg/nfd-master/nfd-master_test.go

@@ -20,6 +20,7 @@ import (
 	"testing"
 
 	. "github.com/smartystreets/goconvey/convey"
+	fakeclient "k8s.io/client-go/kubernetes/fake"
 	m "sigs.k8s.io/node-feature-discovery/pkg/nfd-master"
 )
 
@@ -36,7 +37,13 @@ func TestNewNfdMaster(t *testing.T) {
 			})
 		})
 		Convey("When -config is supplied", func() {
-			_, err := m.NewNfdMaster(m.WithArgs(&m.Args{CertFile: "crt", KeyFile: "key", CaFile: "ca", ConfigFile: "master-config.yaml"}))
+			_, err := m.NewNfdMaster(
+				m.WithArgs(&m.Args{
+					CertFile:   "crt",
+					KeyFile:    "key",
+					CaFile:     "ca",
+					ConfigFile: "master-config.yaml"}),
+				m.WithKubernetesClient(fakeclient.NewSimpleClientset()))
 			Convey("An error should not be returned", func() {
 				So(err, ShouldBeNil)
 			})

pkg/nfd-worker/nfd-worker_test.go

@@ -24,6 +24,7 @@ import (
 	"time"
 
 	. "github.com/smartystreets/goconvey/convey"
+	fakeclient "k8s.io/client-go/kubernetes/fake"
 	"k8s.io/klog/v2"
 
 	"sigs.k8s.io/node-feature-discovery/pkg/features"
@@ -52,7 +53,9 @@ func setupTest(args *master.Args) testContext {
 		os.Exit(1)
 	}
 	_ = features.NFDMutableFeatureGate.OverrideDefault(features.NodeFeatureAPI, false)
-	m, err := master.NewNfdMaster(master.WithArgs(args))
+	m, err := master.NewNfdMaster(
+		master.WithArgs(args),
+		master.WithKubernetesClient(fakeclient.NewSimpleClientset()))
 	if err != nil {
 		fmt.Printf("Test setup failed: %v\n", err)
 		os.Exit(1)