nfd-worker: add core config

Allows dynamic (re-)configuration of most nfd-worker options. The goal
is to have most configuration parameters specified in the configuration
file and deprecate most of the command line flags. The priority is
intended to be such that command line flags override whatever is
specified in the configuration file. Thus, specifying something on the
command line effectively disables dynamic configurability of that
parameter.

This patch adds core.noPublish config file option to demonstrate how the
new mechanism is supposed to work. The --no-publish command line flag
takes precedence over this config file option.

cmd/nfd-worker/main.go

@@ -119,7 +119,6 @@ func argsParse(argv []string) (worker.Args, error) {
 	args.CertFile = arguments["--cert-file"].(string)
 	args.ConfigFile = arguments["--config"].(string)
 	args.KeyFile = arguments["--key-file"].(string)
-	args.NoPublish = arguments["--no-publish"].(bool)
 	args.Options = arguments["--options"].(string)
 	args.Server = arguments["--server"].(string)
 	args.ServerNameOverride = arguments["--server-name-override"].(string)
@@ -130,5 +129,12 @@ func argsParse(argv []string) (worker.Args, error) {
 	if err != nil {
 		return args, fmt.Errorf("invalid --sleep-interval specified: %s", err.Error())
 	}
+
+	// Parse deprecated/override args
+	if arguments["--no-publish"].(bool) {
+		b := true
+		args.NoPublish = &b
+	}
+
 	return args, nil
 }

cmd/nfd-worker/main_test.go

@@ -32,7 +32,7 @@ func TestArgsParse(t *testing.T) {
 
 			Convey("noPublish is set and args.sources is set to the default value", func() {
 				So(args.SleepInterval, ShouldEqual, 60*time.Second)
-				So(args.NoPublish, ShouldBeTrue)
+				So(*args.NoPublish, ShouldBeTrue)
 				So(args.Oneshot, ShouldBeTrue)
 				So(args.Sources, ShouldResemble, allSources)
 				So(len(args.LabelWhiteList), ShouldEqual, 0)
@@ -45,7 +45,7 @@ func TestArgsParse(t *testing.T) {
 
 			Convey("args.sources is set to appropriate values", func() {
 				So(args.SleepInterval, ShouldEqual, 30*time.Second)
-				So(args.NoPublish, ShouldBeFalse)
+				So(args.NoPublish, ShouldBeNil)
 				So(args.Oneshot, ShouldBeFalse)
 				So(args.Sources, ShouldResemble, []string{"fake1", "fake2", "fake3"})
 				So(len(args.LabelWhiteList), ShouldEqual, 0)
@@ -57,7 +57,7 @@ func TestArgsParse(t *testing.T) {
 			args, err := argsParse([]string{"--label-whitelist=.*rdt.*"})
 
 			Convey("args.labelWhiteList is set to appropriate value and args.sources is set to default value", func() {
-				So(args.NoPublish, ShouldBeFalse)
+				So(args.NoPublish, ShouldBeNil)
 				So(args.Sources, ShouldResemble, allSources)
 				So(args.LabelWhiteList, ShouldResemble, ".*rdt.*")
 				So(err, ShouldBeNil)
@@ -68,7 +68,7 @@ func TestArgsParse(t *testing.T) {
 			args, err := argsParse([]string{"--no-publish", "--sources=fake1,fake2,fake3", "--ca-file=ca", "--cert-file=crt", "--key-file=key"})
 
 			Convey("--no-publish is set and args.sources is set to appropriate values", func() {
-				So(args.NoPublish, ShouldBeTrue)
+				So(*args.NoPublish, ShouldBeTrue)
 				So(args.CaFile, ShouldEqual, "ca")
 				So(args.CertFile, ShouldEqual, "crt")
 				So(args.KeyFile, ShouldEqual, "key")

nfd-daemonset-combined.yaml.template

@@ -142,6 +142,8 @@ metadata:
   namespace: node-feature-discovery
 data:
   nfd-worker.conf: | ### <NFD-WORKER-CONF-START-DO-NOT-REMOVE>
+    #core:
+    #  noPublish: false
     #sources:
     #  cpu:
     #    cpuid:

nfd-worker-daemonset.yaml.template

@@ -102,6 +102,8 @@ metadata:
   namespace: node-feature-discovery
 data:
   nfd-worker.conf: | ### <NFD-WORKER-CONF-START-DO-NOT-REMOVE>
+    #core:
+    #  noPublish: false
     #sources:
     #  cpu:
     #    cpuid:

nfd-worker-job.yaml.template

@@ -112,6 +112,8 @@ metadata:
   namespace: node-feature-discovery
 data:
   nfd-worker.conf: | ### <NFD-WORKER-CONF-START-DO-NOT-REMOVE>
+    #core:
+    #  noPublish: false
     #sources:
     #  cpu:
     #    cpuid:

nfd-worker.conf.example

@@ -1,3 +1,5 @@
+#core:
+#  noPublish: false
 #sources:
 #  cpu:
 #    cpuid:

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

@@ -169,6 +169,7 @@ func TestNewNfdWorker(t *testing.T) {
 				So(err, ShouldBeNil)
 			})
 			worker := w.(*nfdWorker)
+			worker.configure("", "")
 			Convey("no sources should be enabled and the whitelist regexp should be empty", func() {
 				So(len(worker.sources), ShouldEqual, 0)
 				So(worker.labelWhiteList, ShouldResemble, emptyRegexp)
@@ -183,6 +184,7 @@ func TestNewNfdWorker(t *testing.T) {
 				So(err, ShouldBeNil)
 			})
 			worker := w.(*nfdWorker)
+			worker.configure("", "")
 			Convey("proper sources should be enabled", func() {
 				So(len(worker.sources), ShouldEqual, 1)
 				So(worker.sources[0], ShouldHaveSameTypeAs, &fake.Source{})
@@ -194,6 +196,7 @@ func TestNewNfdWorker(t *testing.T) {
 			args := Args{LabelWhiteList: "*"}
 			w, err := NewNfdWorker(args)
 			worker := w.(*nfdWorker)
+			worker.configure("", "")
 			Convey("an error should be returned", func() {
 				So(len(worker.sources), ShouldEqual, 0)
 				So(worker.labelWhiteList, ShouldBeNil)
@@ -208,6 +211,7 @@ func TestNewNfdWorker(t *testing.T) {
 				So(err, ShouldBeNil)
 			})
 			worker := w.(*nfdWorker)
+			worker.configure("", "")
 			expectRegexp := regexp.MustCompile(".*rdt.*")
 			Convey("proper labelWhiteList regexp should be produced", func() {
 				So(len(worker.sources), ShouldEqual, 0)

pkg/nfd-worker/nfd-worker.go

@@ -61,9 +61,14 @@ var (
 
 // Global config
 type NFDConfig struct {
+	Core    coreConfig
 	Sources sourcesConfig
 }
 
+type coreConfig struct {
+	NoPublish bool
+}
+
 type sourcesConfig map[string]source.Config
 
 // Labels are a Kubernetes representation of discovered features.
@@ -76,13 +81,14 @@ type Args struct {
 	CertFile           string
 	KeyFile            string
 	ConfigFile         string
-	NoPublish          bool
 	Options            string
 	Oneshot            bool
 	Server             string
 	ServerNameOverride string
 	SleepInterval      time.Duration
 	Sources            []string
+	// Deprecated options that should be set via the config file
+	NoPublish *bool
 }
 
 type NfdWorker interface {
@@ -93,7 +99,8 @@ type nfdWorker struct {
 	args           Args
 	clientConn     *grpc.ClientConn
 	client         pb.LabelerClient
-	config         NFDConfig
+	configFilePath string
+	config         *NFDConfig
 	sources        []source.FeatureSource
 	labelWhiteList *regexp.Regexp
 }
@@ -102,9 +109,14 @@ type nfdWorker struct {
 func NewNfdWorker(args Args) (NfdWorker, error) {
 	nfd := &nfdWorker{
 		args:    args,
+		config:  &NFDConfig{},
 		sources: []source.FeatureSource{},
 	}
 
+	if args.ConfigFile != "" {
+		nfd.configFilePath = filepath.Clean(args.ConfigFile)
+	}
+
 	if args.SleepInterval > 0 && args.SleepInterval < time.Second {
 		stderrLogger.Printf("WARNING: too short sleep-intervall specified (%s), forcing to 1s", args.SleepInterval.String())
 		args.SleepInterval = time.Second
@@ -214,27 +226,32 @@ func addConfigWatch(path string) (*fsnotify.Watcher, map[string]struct{}, error)
 	return w, paths, nil
 }
 
+func newDefaultConfig() *NFDConfig {
+	return &NFDConfig{
+		Core: coreConfig{},
+	}
+}
+
 // Run NfdWorker client. Returns if a fatal error is encountered, or, after
 // one request if OneShot is set to 'true' in the worker args.
 func (w *nfdWorker) Run() error {
 	stdoutLogger.Printf("Node Feature Discovery Worker %s", version.Get())
 	stdoutLogger.Printf("NodeName: '%s'", nodeName)
 
+	// Create watcher for config file and read initial configuration
+	configWatch, paths, err := addConfigWatch(w.configFilePath)
+	if err != nil {
+		return err
+	}
+	w.configure(w.configFilePath, w.args.Options)
+
 	// Connect to NFD master
-	err := w.connect()
+	err = w.connect()
 	if err != nil {
 		return fmt.Errorf("failed to connect: %v", err)
 	}
 	defer w.disconnect()
 
-	// Create watcher for config file and read initial configuration
-	configFilePath := filepath.Clean(w.args.ConfigFile)
-	configWatch, paths, err := addConfigWatch(configFilePath)
-	if err != nil {
-		return (err)
-	}
-	w.configure(configFilePath, w.args.Options)
-
 	labelTrigger := time.After(0)
 	var configTrigger <-chan time.Time
 	for {
@@ -270,7 +287,7 @@ func (w *nfdWorker) Run() error {
 				if err := configWatch.Close(); err != nil {
 					stderrLogger.Printf("WARNING: failed to close fsnotify watcher: %v", err)
 				}
-				configWatch, paths, err = addConfigWatch(configFilePath)
+				configWatch, paths, err = addConfigWatch(w.configFilePath)
 				if err != nil {
 					return err
 				}
@@ -285,8 +302,15 @@ func (w *nfdWorker) Run() error {
 			stderrLogger.Printf("ERROR: config file watcher error: %v", e)
 
 		case <-configTrigger:
-			w.configure(configFilePath, w.args.Options)
-
+			w.configure(w.configFilePath, w.args.Options)
+			// Manage connection to master
+			if w.config.Core.NoPublish {
+				w.disconnect()
+			} else if w.clientConn == nil {
+				if err := w.connect(); err != nil {
+					return err
+				}
+			}
 			// Always re-label after a re-config event. This way the new config
 			// comes into effect even if the sleep interval is long (or infinite)
 			labelTrigger = time.After(0)
@@ -297,7 +321,7 @@ func (w *nfdWorker) Run() error {
 // connect creates a client connection to the NFD master
 func (w *nfdWorker) connect() error {
 	// Return a dummy connection in case of dry-run
-	if w.args.NoPublish {
+	if w.config.Core.NoPublish {
 		return nil
 	}
 
@@ -357,7 +381,8 @@ func (w *nfdWorker) disconnect() {
 // Parse configuration options
 func (w *nfdWorker) configure(filepath string, overrides string) {
 	// Create a new default config
-	c := NFDConfig{Sources: make(map[string]source.Config, len(w.sources))}
+	c := newDefaultConfig()
+	c.Sources = make(map[string]source.Config, len(w.sources))
 	for _, s := range w.sources {
 		c.Sources[s.Name()] = s.NewConfig()
 	}
@@ -367,7 +392,7 @@ func (w *nfdWorker) configure(filepath string, overrides string) {
 	if err != nil {
 		stderrLogger.Printf("Failed to read config file: %s", err)
 	} else {
-		err = yaml.Unmarshal(data, &c)
+		err = yaml.Unmarshal(data, c)
 		if err != nil {
 			stderrLogger.Printf("Failed to parse config file: %s", err)
 		} else {
@@ -376,11 +401,15 @@ func (w *nfdWorker) configure(filepath string, overrides string) {
 	}
 
 	// Parse config overrides
-	err = yaml.Unmarshal([]byte(overrides), &c)
+	err = yaml.Unmarshal([]byte(overrides), c)
 	if err != nil {
 		stderrLogger.Printf("Failed to parse --options: %s", err)
 	}
 
+	if w.args.NoPublish != nil {
+		c.Core.NoPublish = *w.args.NoPublish
+	}
+
 	w.config = c
 
 	// (Re-)configure all sources