Merge pull request #1655 from marquiz/devel/master-no-publish

nfd-master: parse kubeconfig even with NoPublish set

Author: k8s-ci-robot

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)