Add optionable arguments to NewWorker

Signed-off-by: Carlos Eduardo Arango Gutierrez <[email protected]>

Author: ArangoGutierrez

cmd/nfd-worker/main.go

@@ -73,7 +73,7 @@ func main() {
 	utils.ConfigureGrpcKlog()
 
 	// Get new NfdWorker instance
-	instance, err := worker.NewNfdWorker(args)
+	instance, err := worker.NewNfdWorker(worker.WithArgs(args))
 	if err != nil {
 		klog.ErrorS(err, "failed to initialize NfdWorker instance")
 		os.Exit(1)

pkg/nfd-worker/nfd-worker-internal_test.go

@@ -27,6 +27,7 @@ import (
 	. "github.com/smartystreets/goconvey/convey"
 	"github.com/stretchr/testify/mock"
 	"github.com/vektra/errors"
+	fakeclient "k8s.io/client-go/kubernetes/fake"
 
 	nfdv1alpha1 "sigs.k8s.io/node-feature-discovery/pkg/apis/nfd/v1alpha1"
 	"sigs.k8s.io/node-feature-discovery/pkg/labeler"
@@ -97,7 +98,8 @@ func makeFakeFeatures(names []string) (source.FeatureLabels, Labels) {
 
 func TestConfigParse(t *testing.T) {
 	Convey("When parsing configuration", t, func() {
-		w, err := NewNfdWorker(&Args{})
+		w, err := NewNfdWorker(WithArgs(&Args{}),
+			WithKubernetesClient(fakeclient.NewSimpleClientset()))
 		So(err, ShouldBeNil)
 		worker := w.(*nfdWorker)
 		overrides := `{"core": {"labelSources": ["fake"],"noPublish": true},"sources": {"cpu": {"cpuid": {"attributeBlacklist": ["foo","bar"]}}}}`
@@ -222,13 +224,13 @@ core:
 `)
 
 		noPublish := true
-		w, err := NewNfdWorker(&Args{
+		w, err := NewNfdWorker(WithArgs(&Args{
 			ConfigFile: configFile,
 			Overrides: ConfigOverrideArgs{
 				FeatureSources: &utils.StringSliceVal{"fake"},
 				LabelSources:   &utils.StringSliceVal{"fake"},
 				NoPublish:      &noPublish},
-		})
+		}), WithKubernetesClient(fakeclient.NewSimpleClientset()))
 		So(err, ShouldBeNil)
 		worker := w.(*nfdWorker)
 
@@ -307,7 +309,8 @@ func TestNewNfdWorker(t *testing.T) {
 
 		Convey("without any args specified", func() {
 			args := &Args{}
-			w, err := NewNfdWorker(args)
+			w, err := NewNfdWorker(WithArgs(args),
+				WithKubernetesClient(fakeclient.NewSimpleClientset()))
 			Convey("no error should be returned", func() {
 				So(err, ShouldBeNil)
 			})
@@ -324,7 +327,8 @@ func TestNewNfdWorker(t *testing.T) {
 			args := &Args{Overrides: ConfigOverrideArgs{
 				LabelSources:   &utils.StringSliceVal{"fake"},
 				FeatureSources: &utils.StringSliceVal{"cpu"}}}
-			w, err := NewNfdWorker(args)
+			w, err := NewNfdWorker(WithArgs(args),
+				WithKubernetesClient(fakeclient.NewSimpleClientset()))
 			Convey("no error should be returned", func() {
 				So(err, ShouldBeNil)
 			})
@@ -373,7 +377,7 @@ func TestCreateFeatureLabels(t *testing.T) {
 
 func TestAdvertiseFeatureLabels(t *testing.T) {
 	Convey("When advertising labels", t, func() {
-		w, err := NewNfdWorker(&Args{})
+		w, err := NewNfdWorker(WithArgs(&Args{}), WithKubernetesClient(fakeclient.NewSimpleClientset()))
 		So(err, ShouldBeNil)
 		worker := w.(*nfdWorker)
 

pkg/nfd-worker/nfd-worker.go

@@ -36,7 +36,7 @@ import (
 	"k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/validation"
-	"k8s.io/client-go/kubernetes"
+	k8sclient "k8s.io/client-go/kubernetes"
 	"k8s.io/klog/v2"
 	klogutils "sigs.k8s.io/node-feature-discovery/pkg/utils/klog"
 	"sigs.k8s.io/yaml"
@@ -126,42 +126,84 @@ type nfdWorker struct {
 	config              *NFDConfig
 	kubernetesNamespace string
 	grpcClient          pb.LabelerClient
-	nfdClient           *nfdclient.Clientset
-	stop                chan struct{} // channel for signaling stop
-	featureSources      []source.FeatureSource
-	labelSources        []source.LabelSource
-	ownerReference      []metav1.OwnerReference
+
+	k8sClient      k8sclient.Interface
+	nfdClient      *nfdclient.Clientset
+	stop           chan struct{} // channel for signaling stop
+	featureSources []source.FeatureSource
+	labelSources   []source.LabelSource
+	ownerReference []metav1.OwnerReference
 }
 
 // This ticker can represent infinite and normal intervals.
 type infiniteTicker struct {
 	*time.Ticker
 }
 
+// NfdWorkerOption sets properties of the NfdWorker instance.
+type NfdWorkerOption interface {
+	apply(*nfdWorker)
+}
+
+// WithArgs is used for passing settings from command line arguments.
+func WithArgs(args *Args) NfdWorkerOption {
+	return &nfdMWorkerOpt{f: func(n *nfdWorker) { n.args = *args }}
+}
+
+// WithKuberneteClient forces to use the given kubernetes client, without
+// initializing one from kubeconfig.
+func WithKubernetesClient(cli k8sclient.Interface) NfdWorkerOption {
+	return &nfdMWorkerOpt{f: func(n *nfdWorker) { n.k8sClient = cli }}
+}
+
+type nfdMWorkerOpt struct {
+	f func(*nfdWorker)
+}
+
+func (f *nfdMWorkerOpt) apply(n *nfdWorker) {
+	f.f(n)
+}
+
 // NewNfdWorker creates new NfdWorker instance.
-func NewNfdWorker(args *Args) (NfdWorker, error) {
+func NewNfdWorker(opts ...NfdWorkerOption) (NfdWorker, error) {
 	nfd := &nfdWorker{
-		args:                *args,
 		config:              &NFDConfig{},
 		kubernetesNamespace: utils.GetKubernetesNamespace(),
 		stop:                make(chan struct{}, 1),
 	}
 
+	for _, o := range opts {
+		o.apply(nfd)
+	}
+
 	// Check TLS related args
-	if args.CertFile != "" || args.KeyFile != "" || args.CaFile != "" {
-		if args.CertFile == "" {
+	if nfd.args.CertFile != "" || nfd.args.KeyFile != "" || nfd.args.CaFile != "" {
+		if nfd.args.CertFile == "" {
 			return nfd, fmt.Errorf("-cert-file needs to be specified alongside -key-file and -ca-file")
 		}
-		if args.KeyFile == "" {
+		if nfd.args.KeyFile == "" {
 			return nfd, fmt.Errorf("-key-file needs to be specified alongside -cert-file and -ca-file")
 		}
-		if args.CaFile == "" {
+		if nfd.args.CaFile == "" {
 			return nfd, fmt.Errorf("-ca-file needs to be specified alongside -cert-file and -key-file")
 		}
 	}
 
-	if args.ConfigFile != "" {
-		nfd.configFilePath = filepath.Clean(args.ConfigFile)
+	if nfd.args.ConfigFile != "" {
+		nfd.configFilePath = filepath.Clean(nfd.args.ConfigFile)
+	}
+
+	// k8sClient might've been set via opts by tests
+	if nfd.k8sClient == nil {
+		kubeconfig, err := apihelper.GetKubeconfig(nfd.args.Kubeconfig)
+		if err != nil {
+			return nfd, err
+		}
+		cli, err := k8sclient.NewForConfig(kubeconfig)
+		if err != nil {
+			return nfd, err
+		}
+		nfd.k8sClient = cli
 	}
 
 	return nfd, nil
@@ -245,32 +287,33 @@ func (w *nfdWorker) Run() error {
 	labelTrigger.Reset(w.config.Core.SleepInterval.Duration)
 	defer labelTrigger.Stop()
 
+	// Create owner ref
+	ownerReference := []metav1.OwnerReference{}
 	// Get pod owner reference
 	podName := os.Getenv("POD_NAME")
-	client, err := w.getKubeClient()
-	if err != nil {
-		return fmt.Errorf("failed to get kube client: %w", err)
-	}
-
-	selfPod, err := client.CoreV1().Pods(w.kubernetesNamespace).Get(context.TODO(), podName, metav1.GetOptions{})
-	if err != nil {
-		return fmt.Errorf("failed to get pod %q: %w", podName, err)
-	}
-
-	// Create owner ref
-	ownerReference := selfPod.OwnerReferences
 
 	// Add pod owner reference if it exists
-	podUID := os.Getenv("POD_UID")
-	if podName != "" && podUID != "" {
-		isTrue := true
-		ownerReference = append(ownerReference, metav1.OwnerReference{
-			APIVersion: "v1",
-			Kind:       "Pod",
-			Name:       podName,
-			UID:        types.UID(podUID),
-			Controller: &isTrue,
-		})
+	if podName != "" {
+		if selfPod, err := w.k8sClient.CoreV1().Pods(w.kubernetesNamespace).Get(context.TODO(), podName, metav1.GetOptions{}); err != nil {
+			klog.ErrorS(err, "failed to get self pod, cannot inherit ownerReference for NodeFeature")
+			return err
+		} else {
+			ownerReference = append(ownerReference, selfPod.OwnerReferences...)
+		}
+
+		podUID := os.Getenv("POD_UID")
+		if podUID != "" {
+			ownerReference = append(ownerReference, metav1.OwnerReference{
+				APIVersion: "v1",
+				Kind:       "Pod",
+				Name:       podName,
+				UID:        types.UID(podUID),
+			})
+		} else {
+			klog.InfoS("Cannot append POD ownerReference to NodeFeature, POD_UID not specified")
+		}
+	} else {
+		klog.InfoS("Cannot set NodeFeature owner references, POD_NAME not specified")
 	}
 
 	w.ownerReference = ownerReference
@@ -774,22 +817,6 @@ func (m *nfdWorker) getNfdClient() (*nfdclient.Clientset, error) {
 	return c, nil
 }
 
-func (m *nfdWorker) getKubeClient() (*kubernetes.Clientset, error) {
-	// creates the in-cluster config
-	kubeconfig, err := utils.GetKubeconfig(m.args.Kubeconfig)
-	if err != nil {
-		return nil, err
-	}
-
-	// creates the clientset
-	clientset, err := kubernetes.NewForConfig(kubeconfig)
-	if err != nil {
-		return nil, err
-	}
-
-	return clientset, nil
-}
-
 // UnmarshalJSON implements the Unmarshaler interface from "encoding/json"
 func (c *sourcesConfig) UnmarshalJSON(data []byte) error {
 	// First do a raw parse to get the per-source data

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"
 
 	master "sigs.k8s.io/node-feature-discovery/pkg/nfd-master"
 	worker "sigs.k8s.io/node-feature-discovery/pkg/nfd-worker"
@@ -78,9 +79,12 @@ func teardownTest(ctx testContext) {
 func TestNewNfdWorker(t *testing.T) {
 	Convey("When initializing new NfdWorker instance", t, func() {
 		Convey("When one of -cert-file, -key-file or -ca-file is missing", func() {
-			_, err := worker.NewNfdWorker(&worker.Args{CertFile: "crt", KeyFile: "key"})
-			_, err2 := worker.NewNfdWorker(&worker.Args{KeyFile: "key", CaFile: "ca"})
-			_, err3 := worker.NewNfdWorker(&worker.Args{CertFile: "crt", CaFile: "ca"})
+			_, err := worker.NewNfdWorker(worker.WithArgs(&worker.Args{CertFile: "crt", KeyFile: "key"}),
+				worker.WithKubernetesClient(fakeclient.NewSimpleClientset()))
+			_, err2 := worker.NewNfdWorker(worker.WithArgs(&worker.Args{KeyFile: "key", CaFile: "ca"}),
+				worker.WithKubernetesClient(fakeclient.NewSimpleClientset()))
+			_, err3 := worker.NewNfdWorker(worker.WithArgs(&worker.Args{CertFile: "crt", CaFile: "ca"}),
+				worker.WithKubernetesClient(fakeclient.NewSimpleClientset()))
 			Convey("An error should be returned", func() {
 				So(err, ShouldNotBeNil)
 				So(err2, ShouldNotBeNil)
@@ -100,7 +104,8 @@ func TestRun(t *testing.T) {
 				Oneshot:   true,
 				Overrides: worker.ConfigOverrideArgs{LabelSources: &utils.StringSliceVal{"fake"}},
 			}
-			fooasdf, _ := worker.NewNfdWorker(args)
+			fooasdf, _ := worker.NewNfdWorker(worker.WithArgs(args),
+				worker.WithKubernetesClient(fakeclient.NewSimpleClientset()))
 			err := fooasdf.Run()
 			Convey("No error should be returned", func() {
 				So(err, ShouldBeNil)
@@ -129,7 +134,8 @@ func TestRunTls(t *testing.T) {
 				Oneshot:            true,
 				Overrides:          worker.ConfigOverrideArgs{LabelSources: &utils.StringSliceVal{"fake"}},
 			}
-			w, _ := worker.NewNfdWorker(&workerArgs)
+			w, _ := worker.NewNfdWorker(worker.WithArgs(&workerArgs),
+				worker.WithKubernetesClient(fakeclient.NewSimpleClientset()))
 			err := w.Run()
 			Convey("No error should be returned", func() {
 				So(err, ShouldBeNil)

test/e2e/utils/rbac.go

@@ -204,6 +204,11 @@ func createRoleWorker(ctx context.Context, cs clientset.Interface, ns string) (*
 				Resources: []string{"nodefeatures"},
 				Verbs:     []string{"create", "get", "update"},
 			},
+			{
+				APIGroups: []string{""},
+				Resources: []string{"pods"},
+				Verbs:     []string{"get"},
+			},
 		},
 	}
 	return cs.RbacV1().Roles(ns).Update(ctx, cr, metav1.UpdateOptions{})