nfd-worker: add core.sources config option

Add a config file option for controlling the enabled feature sources,
aimed at replacing the --sources command line flag which is now marked
as deprecated. The command line flag takes precedence over the config
file option.

cmd/nfd-worker/main.go

@@ -93,7 +93,8 @@ func argsParse(argv []string) (worker.Args, error) {
                               [Default: ]
   --sources=<sources>         Comma separated list of feature sources. Special
                               value 'all' enables all feature sources.
-                              [Default: all]
+                              (DEPRECATED: This parameter should be set via the
+                              config file)
   --no-publish                Do not publish discovered features to the
                               cluster-local Kubernetes API server.
   --label-whitelist=<pattern> Regular expression to filter label names to
@@ -125,7 +126,6 @@ func argsParse(argv []string) (worker.Args, error) {
 	args.Options = arguments["--options"].(string)
 	args.Server = arguments["--server"].(string)
 	args.ServerNameOverride = arguments["--server-name-override"].(string)
-	args.Sources = strings.Split(arguments["--sources"].(string), ",")
 	args.Oneshot = arguments["--oneshot"].(bool)
 
 	// Parse deprecated/override args
@@ -150,5 +150,10 @@ func argsParse(argv []string) (worker.Args, error) {
 			args.SleepInterval = &s
 		}
 	}
+	if v := arguments["--sources"]; v != nil {
+		fmt.Println(v)
+		s := strings.Split(v.(string), ",")
+		args.Sources = &s
+	}
 	return args, nil
 }

cmd/nfd-worker/main_test.go

@@ -23,8 +23,6 @@ import (
 	. "github.com/smartystreets/goconvey/convey"
 )
 
-var allSources = []string{"all"}
-
 func TestArgsParse(t *testing.T) {
 	Convey("When parsing command line arguments", t, func() {
 		Convey("When --no-publish and --oneshot flags are passed", func() {
@@ -34,7 +32,7 @@ func TestArgsParse(t *testing.T) {
 				So(args.SleepInterval, ShouldEqual, nil)
 				So(*args.NoPublish, ShouldBeTrue)
 				So(args.Oneshot, ShouldBeTrue)
-				So(args.Sources, ShouldResemble, allSources)
+				So(args.Sources, ShouldBeNil)
 				So(args.LabelWhiteList, ShouldBeNil)
 				So(err, ShouldBeNil)
 			})
@@ -47,7 +45,7 @@ func TestArgsParse(t *testing.T) {
 				So(*args.SleepInterval, ShouldEqual, 30*time.Second)
 				So(args.NoPublish, ShouldBeNil)
 				So(args.Oneshot, ShouldBeFalse)
-				So(args.Sources, ShouldResemble, []string{"fake1", "fake2", "fake3"})
+				So(*args.Sources, ShouldResemble, []string{"fake1", "fake2", "fake3"})
 				So(args.LabelWhiteList, ShouldBeNil)
 				So(err, ShouldBeNil)
 			})
@@ -58,7 +56,7 @@ func TestArgsParse(t *testing.T) {
 
 			Convey("args.labelWhiteList is set to appropriate value and args.sources is set to default value", func() {
 				So(args.NoPublish, ShouldBeNil)
-				So(args.Sources, ShouldResemble, allSources)
+				So(args.Sources, ShouldBeNil)
 				So(args.LabelWhiteList.String(), ShouldResemble, ".*rdt.*")
 				So(err, ShouldBeNil)
 			})
@@ -72,7 +70,7 @@ func TestArgsParse(t *testing.T) {
 				So(args.CaFile, ShouldEqual, "ca")
 				So(args.CertFile, ShouldEqual, "crt")
 				So(args.KeyFile, ShouldEqual, "key")
-				So(args.Sources, ShouldResemble, []string{"fake1", "fake2", "fake3"})
+				So(*args.Sources, ShouldResemble, []string{"fake1", "fake2", "fake3"})
 				So(args.LabelWhiteList, ShouldBeNil)
 				So(err, ShouldBeNil)
 			})

docs/advanced/worker-commandline-reference.md

@@ -141,6 +141,9 @@ nfd-worker --server-name-override=localhost
 The `--sources` flag specifies a comma-separated list of enabled feature
 sources. A special value `all` enables all feature sources.
 
+Note: This flag takes precedence over the `core.sources` configuration
+file option.
+
 Default: all
 
 Example:
@@ -149,6 +152,9 @@ Example:
 nfd-worker --sources=kernel,system,local
 ```
 
+**DEPRECATED**: you should use the `core.sources` option in the
+configuration file, instead.
+
 ### --no-publish
 
 The `--no-publish` flag disables all communication with the nfd-master, making

nfd-daemonset-combined.yaml.template

@@ -146,6 +146,7 @@ data:
     #  labelWhiteList:
     #  noPublish: false
     #  sleepInterval: 60s
+    #  sources: [all]
     #sources:
     #  cpu:
     #    cpuid:

nfd-worker-daemonset.yaml.template

@@ -106,6 +106,7 @@ data:
     #  labelWhiteList:
     #  noPublish: false
     #  sleepInterval: 60s
+    #  sources: [all]
     #sources:
     #  cpu:
     #    cpuid:

nfd-worker-job.yaml.template

@@ -116,6 +116,7 @@ data:
     #  labelWhiteList:
     #  noPublish: false
     #  sleepInterval: 60s
+    #  sources: [all]
     #sources:
     #  cpu:
     #    cpuid:

nfd-worker.conf.example

@@ -2,6 +2,7 @@
 #  labelWhiteList:
 #  noPublish: false
 #  sleepInterval: 60s
+#  sources: [all]
 #sources:
 #  cpu:
 #    cpuid:

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

@@ -95,7 +95,7 @@ func makeFakeFeatures(names []string) (source.Features, Labels) {
 }
 
 func (w *nfdWorker) getSource(name string) source.FeatureSource {
-	for _, s := range w.sources {
+	for _, s := range w.realSources {
 		if s.Name() == name {
 			return s
 		}
@@ -105,7 +105,7 @@ func (w *nfdWorker) getSource(name string) source.FeatureSource {
 
 func TestConfigParse(t *testing.T) {
 	Convey("When parsing configuration", t, func() {
-		w, err := NewNfdWorker(Args{Sources: []string{"cpu", "kernel", "pci"}})
+		w, err := NewNfdWorker(Args{Sources: &[]string{"cpu", "kernel", "pci"}})
 		So(err, ShouldBeNil)
 		worker := w.(*nfdWorker)
 		Convey("and a non-accessible file and some overrides are specified", func() {
@@ -171,14 +171,14 @@ func TestNewNfdWorker(t *testing.T) {
 			})
 			worker := w.(*nfdWorker)
 			worker.configure("", "")
-			Convey("no sources should be enabled and the whitelist regexp should be empty", func() {
+			Convey("all sources should be enabled and the whitelist regexp should be empty", func() {
-				So(len(worker.sources), ShouldEqual, 0)
+				So(len(worker.enabledSources), ShouldEqual, len(worker.realSources))
 				So(worker.config.Core.LabelWhiteList, ShouldResemble, emptyRegexp)
 			})
 		})
 
 		Convey("with non-empty Sources arg specified", func() {
-			args := Args{Sources: []string{"fake"}}
+			args := Args{Sources: &[]string{"fake"}}
 			w, err := NewNfdWorker(args)
 			Convey("no error should be returned", func() {
 				So(err, ShouldBeNil)
@@ -186,8 +186,8 @@ func TestNewNfdWorker(t *testing.T) {
 			worker := w.(*nfdWorker)
 			worker.configure("", "")
 			Convey("proper sources should be enabled", func() {
-				So(len(worker.sources), ShouldEqual, 1)
+				So(len(worker.enabledSources), ShouldEqual, 1)
-				So(worker.sources[0], ShouldHaveSameTypeAs, &fake.Source{})
+				So(worker.enabledSources[0], ShouldHaveSameTypeAs, &fake.Source{})
 				So(worker.config.Core.LabelWhiteList, ShouldResemble, emptyRegexp)
 			})
 		})
@@ -202,7 +202,6 @@ func TestNewNfdWorker(t *testing.T) {
 			worker.configure("", "")
 			expectRegexp := regex{*regexp.MustCompile(".*rdt.*")}
 			Convey("proper labelWhiteList regexp should be produced", func() {
-				So(len(worker.sources), ShouldEqual, 0)
 				So(worker.config.Core.LabelWhiteList, ShouldResemble, expectRegexp)
 			})
 		})

pkg/nfd-worker/nfd-worker.go

@@ -68,6 +68,7 @@ type NFDConfig struct {
 type coreConfig struct {
 	LabelWhiteList regex
 	NoPublish      bool
+	Sources        []string
 	SleepInterval  duration
 }
 
@@ -86,11 +87,11 @@ type Args struct {
 	Oneshot            bool
 	Server             string
 	ServerNameOverride string
-	Sources            []string
 	// Deprecated options that should be set via the config file
 	LabelWhiteList *regexp.Regexp
 	NoPublish      *bool
 	SleepInterval  *time.Duration
+	Sources        *[]string
 }
 
 type NfdWorker interface {
@@ -103,7 +104,9 @@ type nfdWorker struct {
 	client         pb.LabelerClient
 	configFilePath string
 	config         *NFDConfig
-	sources        []source.FeatureSource
+	realSources    []source.FeatureSource
+	testSources    []source.FeatureSource
+	enabledSources []source.FeatureSource
 }
 
 type regex struct {
@@ -117,9 +120,27 @@ type duration struct {
 // Create new NfdWorker instance.
 func NewNfdWorker(args Args) (NfdWorker, error) {
 	nfd := &nfdWorker{
-		args:    args,
+		args:   args,
-		config:  &NFDConfig{},
+		config: &NFDConfig{},
-		sources: []source.FeatureSource{},
+		realSources: []source.FeatureSource{
+			&cpu.Source{},
+			&iommu.Source{},
+			&kernel.Source{},
+			&memory.Source{},
+			&network.Source{},
+			&pci.Source{},
+			&storage.Source{},
+			&system.Source{},
+			&usb.Source{},
+			&custom.Source{},
+			// local needs to be the last source so that it is able to override
+			// labels from other sources
+			&local.Source{},
+		},
+		testSources: []source.FeatureSource{
+			&fake.Source{},
+			&panicfake.Source{},
+		},
 	}
 
 	if args.ConfigFile != "" {
@@ -139,53 +160,6 @@ func NewNfdWorker(args Args) (NfdWorker, error) {
 		}
 	}
 
-	// Figure out active sources
-	allSources := []source.FeatureSource{
-		&cpu.Source{},
-		&iommu.Source{},
-		&kernel.Source{},
-		&memory.Source{},
-		&network.Source{},
-		&pci.Source{},
-		&storage.Source{},
-		&system.Source{},
-		&usb.Source{},
-		&custom.Source{},
-		// local needs to be the last source so that it is able to override
-		// labels from other sources
-		&local.Source{},
-	}
-
-	// Determine enabled feature
-	if len(args.Sources) == 1 && args.Sources[0] == "all" {
-		nfd.sources = allSources
-	} else {
-		// Add fake source which is only meant for testing. It will be enabled
-		// only if listed explicitly.
-		allSources = append(allSources, &fake.Source{})
-		allSources = append(allSources, &panicfake.Source{})
-
-		sourceWhiteList := map[string]struct{}{}
-		for _, s := range args.Sources {
-			sourceWhiteList[strings.TrimSpace(s)] = struct{}{}
-		}
-
-		nfd.sources = []source.FeatureSource{}
-		for _, s := range allSources {
-			if _, enabled := sourceWhiteList[s.Name()]; enabled {
-				nfd.sources = append(nfd.sources, s)
-				delete(sourceWhiteList, s.Name())
-			}
-		}
-		if len(sourceWhiteList) > 0 {
-			names := make([]string, 0, len(sourceWhiteList))
-			for n := range sourceWhiteList {
-				names = append(names, n)
-			}
-			stderrLogger.Printf("WARNING: skipping unknown source(s) %q specified in --sources", strings.Join(names, ", "))
-		}
-	}
-
 	return nfd, nil
 }
 
@@ -228,6 +202,7 @@ func newDefaultConfig() *NFDConfig {
 		Core: coreConfig{
 			LabelWhiteList: regex{*regexp.MustCompile("")},
 			SleepInterval:  duration{60 * time.Second},
+			Sources:        []string{"all"},
 		},
 	}
 }
@@ -258,7 +233,7 @@ func (w *nfdWorker) Run() error {
 		select {
 		case <-labelTrigger:
 			// Get the set of feature labels.
-			labels := createFeatureLabels(w.sources, w.config.Core.LabelWhiteList)
+			labels := createFeatureLabels(w.enabledSources, w.config.Core.LabelWhiteList)
 
 			// Update the node with the feature labels.
 			if w.client != nil {
@@ -386,12 +361,47 @@ func (c *coreConfig) sanitize() {
 	}
 }
 
+func (w *nfdWorker) configureCore(c coreConfig) {
+	// Determine enabled feature sourcds
+	sourceList := map[string]struct{}{}
+	all := false
+	for _, s := range c.Sources {
+		if s == "all" {
+			all = true
+			continue
+		}
+		sourceList[strings.TrimSpace(s)] = struct{}{}
+	}
+
+	w.enabledSources = []source.FeatureSource{}
+	for _, s := range w.realSources {
+		if _, enabled := sourceList[s.Name()]; all || enabled {
+			w.enabledSources = append(w.enabledSources, s)
+			delete(sourceList, s.Name())
+		}
+	}
+	for _, s := range w.testSources {
+		if _, enabled := sourceList[s.Name()]; enabled {
+			w.enabledSources = append(w.enabledSources, s)
+			delete(sourceList, s.Name())
+		}
+	}
+	if len(sourceList) > 0 {
+		names := make([]string, 0, len(sourceList))
+		for n := range sourceList {
+			names = append(names, n)
+		}
+		stderrLogger.Printf("WARNING: skipping unknown source(s) %q specified in core.sources (or --sources)", strings.Join(names, ", "))
+	}
+}
+
 // Parse configuration options
 func (w *nfdWorker) configure(filepath string, overrides string) {
 	// Create a new default config
 	c := newDefaultConfig()
-	c.Sources = make(map[string]source.Config, len(w.sources))
+	allSources := append(w.realSources, w.testSources...)
-	for _, s := range w.sources {
+	c.Sources = make(map[string]source.Config, len(allSources))
+	for _, s := range allSources {
 		c.Sources[s.Name()] = s.NewConfig()
 	}
 
@@ -423,13 +433,18 @@ func (w *nfdWorker) configure(filepath string, overrides string) {
 	if w.args.SleepInterval != nil {
 		c.Core.SleepInterval = duration{*w.args.SleepInterval}
 	}
+	if w.args.Sources != nil {
+		c.Core.Sources = *w.args.Sources
+	}
 
 	c.Core.sanitize()
 
 	w.config = c
 
-	// (Re-)configure all sources
+	w.configureCore(c.Core)
-	for _, s := range w.sources {
+
+	// (Re-)configure all "real" sources, test sources are not configurable
+	for _, s := range allSources {
 		s.SetConfig(c.Sources[s.Name()])
 	}
 }

pkg/nfd-worker/nfd-worker_test.go

@@ -90,7 +90,7 @@ func TestRun(t *testing.T) {
 	defer teardownTest(ctx)
 	Convey("When running nfd-worker against nfd-master", t, func() {
 		Convey("When publishing features from fake source", func() {
-			worker, _ := w.NewNfdWorker(w.Args{Oneshot: true, Sources: []string{"fake"}, Server: "localhost:8192"})
+			worker, _ := w.NewNfdWorker(w.Args{Oneshot: true, Sources: &[]string{"fake"}, Server: "localhost:8192"})
 			err := worker.Run()
 			Convey("No error should be returned", func() {
 				So(err, ShouldBeNil)
@@ -115,7 +115,7 @@ func TestRunTls(t *testing.T) {
 				CertFile:           data.FilePath("nfd-test-worker.crt"),
 				KeyFile:            data.FilePath("nfd-test-worker.key"),
 				Oneshot:            true,
-				Sources:            []string{"fake"},
+				Sources:            &[]string{"fake"},
 				Server:             "localhost:8192",
 				ServerNameOverride: "nfd-test-master"}
 			worker, _ := w.NewNfdWorker(workerArgs)