Merge pull request #1341 from AhmedGrati/chore-common-klog-handling

chore: introduce a commong klog handling for cmd/nfd-*

cmd/nfd-master/main.go

@@ -20,10 +20,10 @@ import (
 	"flag"
 	"fmt"
 	"os"
-	"strings"
 	"time"
 
 	"k8s.io/klog/v2"
+	klogutils "sigs.k8s.io/node-feature-discovery/pkg/utils/klog"
 
 	master "sigs.k8s.io/node-feature-discovery/pkg/nfd-master"
 	"sigs.k8s.io/node-feature-discovery/pkg/utils"
@@ -154,7 +154,7 @@ func initFlags(flagset *flag.FlagSet) (*master.Args, *master.ConfigOverrideArgs)
 	flagset.BoolVar(&args.EnableLeaderElection, "enable-leader-election", false,
 		"Enables a leader election. Enable this when running more than one replica on nfd master.")
 
-	initKlogFlags(flagset, args)
+	args.Klog = klogutils.InitKlogFlags(flagset)
 
 	overrides := &master.ConfigOverrideArgs{
 		LabelWhiteList: &utils.RegexpVal{},
@@ -184,24 +184,3 @@ func initFlags(flagset *flag.FlagSet) (*master.Args, *master.ConfigOverrideArgs)
 
 	return args, overrides
 }
-
-func initKlogFlags(flagset *flag.FlagSet, args *master.Args) {
-	args.Klog = make(map[string]*utils.KlogFlagVal)
-
-	flags := flag.NewFlagSet("klog flags", flag.ContinueOnError)
-	//flags.SetOutput(ioutil.Discard)
-	klog.InitFlags(flags)
-	flags.VisitAll(func(f *flag.Flag) {
-		name := klogConfigOptName(f.Name)
-		args.Klog[name] = utils.NewKlogFlagVal(f)
-		flagset.Var(args.Klog[name], f.Name, f.Usage)
-	})
-}
-
-func klogConfigOptName(flagName string) string {
-	split := strings.Split(flagName, "_")
-	for i, v := range split[1:] {
-		split[i+1] = strings.ToUpper(v[0:1]) + v[1:]
-	}
-	return strings.Join(split, "")
-}

cmd/nfd-worker/main.go

@@ -20,9 +20,9 @@ import (
 	"flag"
 	"fmt"
 	"os"
-	"strings"
 
 	"k8s.io/klog/v2"
+	klogutils "sigs.k8s.io/node-feature-discovery/pkg/utils/klog"
 
 	worker "sigs.k8s.io/node-feature-discovery/pkg/nfd-worker"
 	"sigs.k8s.io/node-feature-discovery/pkg/utils"
@@ -143,7 +143,7 @@ func initFlags(flagset *flag.FlagSet) (*worker.Args, *worker.ConfigOverrideArgs)
 		"Hostname expected from server certificate, useful in testing."+
 			" DEPRECATED: will be removed in a future release along with the deprecated gRPC API.")
 
-	initKlogFlags(flagset, args)
+	args.Klog = klogutils.InitKlogFlags(flagset)
 
 	// Flags overlapping with config file options
 	overrides := &worker.ConfigOverrideArgs{
@@ -161,24 +161,3 @@ func initFlags(flagset *flag.FlagSet) (*worker.Args, *worker.ConfigOverrideArgs)
 
 	return args, overrides
 }
-
-func initKlogFlags(flagset *flag.FlagSet, args *worker.Args) {
-	args.Klog = make(map[string]*utils.KlogFlagVal)
-
-	flags := flag.NewFlagSet("klog flags", flag.ContinueOnError)
-	//flags.SetOutput(ioutil.Discard)
-	klog.InitFlags(flags)
-	flags.VisitAll(func(f *flag.Flag) {
-		name := klogConfigOptName(f.Name)
-		args.Klog[name] = utils.NewKlogFlagVal(f)
-		flagset.Var(args.Klog[name], f.Name, f.Usage)
-	})
-}
-
-func klogConfigOptName(flagName string) string {
-	split := strings.Split(flagName, "_")
-	for i, v := range split[1:] {
-		split[i+1] = strings.ToUpper(v[0:1]) + v[1:]
-	}
-	return strings.Join(split, "")
-}

cmd/nfd-worker/main_test.go

@@ -55,19 +55,3 @@ func TestParseArgs(t *testing.T) {
 		})
 	})
 }
-
-func TestKlogConfigOptName(t *testing.T) {
-	Convey("When converting names of klog command line flags", t, func() {
-		tcs := map[string]string{
-			"":                    "",
-			"a":                   "a",
-			"an_arg":              "anArg",
-			"arg_with_many_parts": "argWithManyParts",
-		}
-		Convey("resulting config option names should be as expected", func() {
-			for input, expected := range tcs {
-				So(klogConfigOptName(input), ShouldEqual, expected)
-			}
-		})
-	})
-}

pkg/nfd-master/nfd-master.go

@@ -48,6 +48,8 @@ import (
 	"k8s.io/client-go/tools/leaderelection/resourcelock"
 	"k8s.io/klog/v2"
 	controller "k8s.io/kubernetes/pkg/controller"
+	klogutils "sigs.k8s.io/node-feature-discovery/pkg/utils/klog"
+
 	taintutils "k8s.io/kubernetes/pkg/util/taints"
 	"sigs.k8s.io/yaml"
 
@@ -78,7 +80,7 @@ type NFDConfig struct {
 	ResyncPeriod      utils.DurationVal
 	LeaderElection    LeaderElectionConfig
 	NfdApiParallelism int
-	Klog              map[string]string
+	Klog              klogutils.KlogConfigOpts
 }
 
 // LeaderElectionConfig contains the configuration for leader election
@@ -1169,28 +1171,6 @@ func (m *nfdMaster) createExtendedResourcePatches(n *corev1.Node, extendedResour
 	return patches
 }
 
-func (m *nfdMaster) configureKlog(c *NFDConfig) error {
-	// Handle klog
-	for k, a := range m.args.Klog {
-		if !a.IsSetFromCmdline() {
-			v, ok := c.Klog[k]
-			if !ok {
-				v = a.DefValue()
-			}
-			if err := a.SetFromConfig(v); err != nil {
-				return fmt.Errorf("failed to set logger option klog.%s = %v: %v", k, v, err)
-			}
-		}
-	}
-	for k := range c.Klog {
-		if _, ok := m.args.Klog[k]; !ok {
-			klog.InfoS("unknown logger option in config", "optionName", k)
-		}
-	}
-
-	return nil
-}
-
 // Parse configuration options
 func (m *nfdMaster) configure(filepath string, overrides string) error {
 	// Create a new default config
@@ -1250,7 +1230,7 @@ func (m *nfdMaster) configure(filepath string, overrides string) error {
 
 	m.config = c
 
-	if err := m.configureKlog(c); err != nil {
+	if err := klogutils.MergeKlogConfiguration(m.args.Klog, c.Klog); err != nil {
 		return err
 	}
 

pkg/nfd-worker/nfd-worker.go

@@ -35,6 +35,7 @@ import (
 	"k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/util/validation"
 	"k8s.io/klog/v2"
+	klogutils "sigs.k8s.io/node-feature-discovery/pkg/utils/klog"
 	"sigs.k8s.io/yaml"
 
 	apiequality "k8s.io/apimachinery/pkg/api/equality"
@@ -74,7 +75,7 @@ type NFDConfig struct {
 }
 
 type coreConfig struct {
-	Klog           map[string]string
+	Klog           klogutils.KlogConfigOpts
 	LabelWhiteList utils.RegexpVal
 	NoPublish      bool
 	FeatureSources []string
@@ -377,21 +378,9 @@ func (c *coreConfig) sanitize() {
 
 func (w *nfdWorker) configureCore(c coreConfig) error {
 	// Handle klog
-	for k, a := range w.args.Klog {
-		if !a.IsSetFromCmdline() {
-			v, ok := c.Klog[k]
-			if !ok {
-				v = a.DefValue()
-			}
-			if err := a.SetFromConfig(v); err != nil {
-				return fmt.Errorf("failed to set logger option klog.%s = %v: %v", k, v, err)
-			}
-		}
-	}
-	for k := range c.Klog {
-		if _, ok := w.args.Klog[k]; !ok {
-			klog.InfoS("unknown logger option in config", "optionName", k)
-		}
+	err := klogutils.MergeKlogConfiguration(w.args.Klog, c.Klog)
+	if err != nil {
+		return err
 	}
 
 	// Determine enabled feature sources

pkg/utils/klog/klog.go

@@ -0,0 +1,74 @@
+/*
+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 klog
+
+import (
+	"flag"
+	"fmt"
+	"strings"
+
+	"k8s.io/klog/v2"
+	"sigs.k8s.io/node-feature-discovery/pkg/utils"
+)
+
+// KlogConfigOpts defines klog configuration options
+type KlogConfigOpts map[string]string
+
+// InitKlogFlags function is responsible for initializing klog flags.
+func InitKlogFlags(flagset *flag.FlagSet) map[string]*utils.KlogFlagVal {
+	klogFlags := make(map[string]*utils.KlogFlagVal)
+
+	flags := flag.NewFlagSet("klog flags", flag.ContinueOnError)
+	klog.InitFlags(flags)
+	flags.VisitAll(func(f *flag.Flag) {
+		name := klogConfigOptName(f.Name)
+		klogFlags[name] = utils.NewKlogFlagVal(f)
+		flagset.Var(klogFlags[name], f.Name, f.Usage)
+	})
+
+	return klogFlags
+}
+
+// MergeKlogConfiguration merges klog command line flags to klog configuration file options
+func MergeKlogConfiguration(klogArgs map[string]*utils.KlogFlagVal, klogConfig KlogConfigOpts) error {
+	for k, a := range klogArgs {
+		if !a.IsSetFromCmdline() {
+			v, ok := klogConfig[k]
+			if !ok {
+				v = a.DefValue()
+			}
+			if err := a.SetFromConfig(v); err != nil {
+				return fmt.Errorf("failed to set logger option klog.%s = %v: %v", k, v, err)
+			}
+		}
+	}
+	for k := range klogConfig {
+		if _, ok := klogArgs[k]; !ok {
+			klog.InfoS("unknown logger option in config", "optionName", k)
+		}
+	}
+
+	return nil
+}
+
+func klogConfigOptName(flagName string) string {
+	split := strings.Split(flagName, "_")
+	for i, v := range split[1:] {
+		split[i+1] = strings.ToUpper(v[0:1]) + v[1:]
+	}
+	return strings.Join(split, "")
+}

cmd/nfd-master/main_test.go → pkg/utils/klog/klog_test.go

@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package main
+package klog
 
 import (
 	"testing"