Merge pull request #605 from marquiz/devel/feature-source-config-flag

nfd-worker: add core.featureSources config option

cmd/nfd-worker/main.go

@@ -80,6 +80,8 @@ func parseArgs(flags *flag.FlagSet, osArgs ...string) *worker.Args {
 		switch f.Name {
 		case "no-publish":
 			args.Overrides.NoPublish = overrides.NoPublish
+		case "feature-sources":
+			args.Overrides.FeatureSources = overrides.FeatureSources
 		case "label-sources":
 			args.Overrides.LabelSources = overrides.LabelSources
 		case "label-whitelist":
@@ -123,12 +125,16 @@ func initFlags(flagset *flag.FlagSet) (*worker.Args, *worker.ConfigOverrideArgs)
 	// Flags overlapping with config file options
 	overrides := &worker.ConfigOverrideArgs{
 		LabelWhiteList: &utils.RegexpVal{},
+		FeatureSources: &utils.StringSliceVal{},
 		LabelSources:   &utils.StringSliceVal{},
 	}
 	overrides.NoPublish = flagset.Bool("no-publish", false,
 		"Do not publish discovered features, disable connection to nfd-master.")
+	flagset.Var(overrides.FeatureSources, "feature-sources",
+		"Comma separated list of feature sources. Special value 'all' enables all sources. "+
+			"Prefix the source name with '-' to disable it.")
 	flagset.Var(overrides.LabelSources, "label-sources",
-		"Comma separated list of label sources. Special value 'all' enables all feature sources. "+
+		"Comma separated list of label sources. Special value 'all' enables all sources. "+
 			"Prefix the source name with '-' to disable it.")
 	flagset.Var(overrides.LabelWhiteList, "label-whitelist",
 		"Regular expression to filter label names to publish to the Kubernetes API server. "+

cmd/nfd-worker/main_test.go

@@ -38,6 +38,7 @@ func TestParseArgs(t *testing.T) {
 				So(args.Overrides.NoPublish, ShouldBeNil)
 				So(args.Overrides.LabelWhiteList, ShouldBeNil)
 				So(args.Overrides.SleepInterval, ShouldBeNil)
+				So(args.Overrides.FeatureSources, ShouldBeNil)
 				So(args.Overrides.LabelSources, ShouldBeNil)
 			})
 		})
@@ -46,6 +47,7 @@ func TestParseArgs(t *testing.T) {
 			args := parseArgs(flags,
 				"-no-publish",
 				"-label-whitelist=.*rdt.*",
+				"-feature-sources=cpu",
 				"-label-sources=fake1,fake2,fake3",
 				"-sleep-interval=30s")
 
@@ -53,6 +55,7 @@ func TestParseArgs(t *testing.T) {
 				So(args.Oneshot, ShouldBeFalse)
 				So(*args.Overrides.NoPublish, ShouldBeTrue)
 				So(*args.Overrides.SleepInterval, ShouldEqual, 30*time.Second)
+				So(*args.Overrides.FeatureSources, ShouldResemble, utils.StringSliceVal{"cpu"})
 				So(*args.Overrides.LabelSources, ShouldResemble, utils.StringSliceVal{"fake1", "fake2", "fake3"})
 				So(args.Overrides.LabelWhiteList.Regexp.String(), ShouldResemble, ".*rdt.*")
 			})

deployment/components/worker-config/nfd-worker.conf.example

@@ -2,6 +2,7 @@
 #  labelWhiteList:
 #  noPublish: false
 #  sleepInterval: 60s
+#  featureSources: [all]
 #  labelSources: [all]
 #  klog:
 #    addDirHeader: false

deployment/helm/node-feature-discovery/values.yaml

@@ -91,6 +91,7 @@ worker:
     #  labelWhiteList:
     #  noPublish: false
     #  sleepInterval: 60s
+    #  featureSources: [all]
     #  labelSources: [all]
     #  klog:
     #    addDirHeader: false

docs/advanced/worker-commandline-reference.md

@@ -136,6 +136,28 @@ Example:
 nfd-worker -server-name-override=localhost
 ```
 
+### -feature-sources
+
+The `-feature-sources` flag specifies a comma-separated list of enabled feature
+sources. A special value `all` enables all sources. Prefixing a source name
+with `-` indicates that the source will be disabled instead - this is only
+meaningful when used in conjunction with `all`. This command line flag allows
+completely disabling the feature detection so that neither standard feature
+labels are generated nor the raw feature data is available for custom rule
+processing.  Consider using the `core.featureSources` config file option,
+instead, allowing dynamic configurability.
+
+Note: This flag takes precedence over the `core.featureSources` configuration
+file option.
+
+Default: all
+
+Example:
+
+```bash
+nfd-worker -feature-sources=all,-pci
+```
+
 ### -label-sources
 
 The `-label-sources` flag specifies a comma-separated list of enabled label

docs/advanced/worker-configuration-reference.md

@@ -43,12 +43,44 @@ core:
   sleepInterval: 60s
 ```
 
+### core.featureSources
+
+`core.featureSources` specifies the list of enabled feature sources. A special
+value `all` enables all sources. Prefixing a source name with `-` indicates
+that the source will be disabled instead - this is only meaningful when used in
+conjunction with `all`. This option allows completely disabling the feature
+detection so that neither standard feature labels are generated nor the raw
+feature data is available for custom rule processing.
+
+Default: `[all]`
+
+Example:
+
+```yaml
+core:
+  # Enable all but cpu and local sources
+  featureSources:
+    - "all"
+    - "-cpu"
+    - "-local"
+```
+
+```yaml
+core:
+  # Enable only cpu and local sources
+  featureSources:
+    - "cpu"
+    - "local"
+```
+
 ### core.labelSources
 
 `core.labelSources` specifies the list of enabled label sources. A special
 value `all` enables all sources. Prefixing a source name with `-` indicates
 that the source will be disabled instead - this is only meaningful when used in
-conjunction with `all`.
+conjunction with `all`. This configuration option affects the generation of
+node labels but not the actual discovery of the underlying feature data that is
+used e.g. in custom/`NodeFeatureRule` rules.
 
 Note: Overridden by the `-label-sources` and `-sources` command line flags and
 the `core.sources` configurations option (if any of them is specified).

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

@@ -107,15 +107,19 @@ func TestConfigParse(t *testing.T) {
 
 			Convey("core overrides should be in effect", func() {
 				So(worker.config.Core.LabelSources, ShouldResemble, []string{"fake"})
+				So(worker.config.Core.FeatureSources, ShouldResemble, []string{"all"})
 				So(worker.config.Core.NoPublish, ShouldBeTrue)
 			})
 		})
 		Convey("and a non-accessible file, but core cmdline flags and some overrides are specified", func() {
-			worker.args = Args{Overrides: ConfigOverrideArgs{LabelSources: &utils.StringSliceVal{"cpu", "kernel", "pci"}}}
+			worker.args = Args{Overrides: ConfigOverrideArgs{
+				LabelSources:   &utils.StringSliceVal{"cpu", "kernel", "pci"},
+				FeatureSources: &utils.StringSliceVal{"cpu"}}}
 			So(worker.configure("non-existing-file", overrides), ShouldBeNil)
 
 			Convey("core cmdline flags should be in effect instead overrides", func() {
 				So(worker.config.Core.LabelSources, ShouldResemble, []string{"cpu", "kernel", "pci"})
+				So(worker.config.Core.FeatureSources, ShouldResemble, []string{"cpu"})
 			})
 			Convey("overrides should take effect", func() {
 				So(worker.config.Core.NoPublish, ShouldBeTrue)
@@ -131,6 +135,7 @@ func TestConfigParse(t *testing.T) {
 		_, err = f.WriteString(`
 core:
   noPublish: false
+  featureSources: ["memory", "storage"]
   sources: ["system"]
   labelWhiteList: "foo"
   sleepInterval: "10s"
@@ -151,6 +156,7 @@ sources:
 			Convey("specified configuration should take effect", func() {
 				// Verify core config
 				So(worker.config.Core.NoPublish, ShouldBeFalse)
+				So(worker.config.Core.FeatureSources, ShouldResemble, []string{"memory", "storage"})
 				So(worker.config.Core.LabelSources, ShouldResemble, []string{"cpu", "kernel", "pci"}) // from cmdline
 				So(worker.config.Core.LabelWhiteList.String(), ShouldEqual, "foo")
 				So(worker.config.Core.SleepInterval.Duration, ShouldEqual, 10*time.Second)
@@ -173,6 +179,7 @@ sources:
 			Convey("overrides should take precedence over the config file", func() {
 				// Verify core config
 				So(worker.config.Core.NoPublish, ShouldBeTrue)
+				So(worker.config.Core.FeatureSources, ShouldResemble, []string{"memory", "storage"})
 				So(worker.config.Core.LabelSources, ShouldResemble, []string{"fake"}) // from overrides
 				So(worker.config.Core.LabelWhiteList.String(), ShouldEqual, "foo")
 				So(worker.config.Core.SleepInterval.Duration, ShouldEqual, 15*time.Second) // from cmdline
@@ -308,13 +315,16 @@ func TestNewNfdWorker(t *testing.T) {
 			worker := w.(*nfdWorker)
 			So(worker.configure("", ""), ShouldBeNil)
 			Convey("all sources should be enabled and the whitelist regexp should be empty", func() {
-				So(len(worker.enabledSources), ShouldEqual, len(source.GetAllLabelSources())-1)
+				So(len(worker.featureSources), ShouldEqual, len(source.GetAllFeatureSources()))
+				So(len(worker.labelSources), ShouldEqual, len(source.GetAllLabelSources())-1)
 				So(worker.config.Core.LabelWhiteList, ShouldResemble, emptyRegexp)
 			})
 		})
 
 		Convey("with non-empty Sources arg specified", func() {
-			args := &Args{Overrides: ConfigOverrideArgs{LabelSources: &utils.StringSliceVal{"fake"}}}
+			args := &Args{Overrides: ConfigOverrideArgs{
+				LabelSources:   &utils.StringSliceVal{"fake"},
+				FeatureSources: &utils.StringSliceVal{"cpu"}}}
 			w, err := NewNfdWorker(args)
 			Convey("no error should be returned", func() {
 				So(err, ShouldBeNil)
@@ -322,8 +332,10 @@ func TestNewNfdWorker(t *testing.T) {
 			worker := w.(*nfdWorker)
 			So(worker.configure("", ""), ShouldBeNil)
 			Convey("proper sources should be enabled", func() {
-				So(len(worker.enabledSources), ShouldEqual, 1)
+				So(len(worker.featureSources), ShouldEqual, 1)
-				So(worker.enabledSources[0].Name(), ShouldEqual, "fake")
+				So(worker.featureSources[0].Name(), ShouldEqual, "cpu")
+				So(len(worker.labelSources), ShouldEqual, 1)
+				So(worker.labelSources[0].Name(), ShouldEqual, "fake")
 				So(worker.config.Core.LabelWhiteList, ShouldResemble, emptyRegexp)
 			})
 		})
@@ -376,12 +388,18 @@ func TestCreateFeatureLabels(t *testing.T) {
 
 func TestAdvertiseFeatureLabels(t *testing.T) {
 	Convey("When advertising labels", t, func() {
+		w, err := NewNfdWorker(&Args{})
+		So(err, ShouldBeNil)
+		worker := w.(*nfdWorker)
+
 		mockClient := &labeler.MockLabelerClient{}
+		worker.client = mockClient
+
 		labels := map[string]string{"feature-1": "value-1"}
 
 		Convey("Correct labeling request is sent", func() {
 			mockClient.On("SetLabels", mock.AnythingOfType("*context.timerCtx"), mock.AnythingOfType("*labeler.SetLabelsRequest")).Return(&labeler.SetLabelsReply{}, nil)
-			err := advertiseFeatureLabels(mockClient, labels)
+			err := worker.advertiseFeatureLabels(labels)
 			Convey("There should be no error", func() {
 				So(err, ShouldBeNil)
 			})
@@ -389,7 +407,7 @@ func TestAdvertiseFeatureLabels(t *testing.T) {
 		Convey("Labeling request fails", func() {
 			mockErr := errors.New("mock-error")
 			mockClient.On("SetLabels", mock.AnythingOfType("*context.timerCtx"), mock.AnythingOfType("*labeler.SetLabelsRequest")).Return(&labeler.SetLabelsReply{}, mockErr)
-			err := advertiseFeatureLabels(mockClient, labels)
+			err := worker.advertiseFeatureLabels(labels)
 			Convey("An error should be returned", func() {
 				So(err, ShouldEqual, mockErr)
 			})

pkg/nfd-client/worker/nfd-worker.go

@@ -64,6 +64,7 @@ type coreConfig struct {
 	Klog           map[string]string
 	LabelWhiteList utils.RegexpVal
 	NoPublish      bool
+	FeatureSources []string
 	Sources        *[]string
 	LabelSources   []string
 	SleepInterval  duration
@@ -93,6 +94,7 @@ type ConfigOverrideArgs struct {
 	// Deprecated
 	LabelWhiteList *utils.RegexpVal
 	SleepInterval  *time.Duration
+	FeatureSources *utils.StringSliceVal
 	LabelSources   *utils.StringSliceVal
 }
 
@@ -105,7 +107,8 @@ type nfdWorker struct {
 	configFilePath string
 	config         *NFDConfig
 	stop           chan struct{} // channel for signaling stop
-	enabledSources []source.LabelSource
+	featureSources []source.FeatureSource
+	labelSources   []source.LabelSource
 }
 
 type duration struct {
@@ -139,6 +142,7 @@ func newDefaultConfig() *NFDConfig {
 		Core: coreConfig{
 			LabelWhiteList: utils.RegexpVal{Regexp: *regexp.MustCompile("")},
 			SleepInterval:  duration{60 * time.Second},
+			FeatureSources: []string{"all"},
 			LabelSources:   []string{"all"},
 			Klog:           make(map[string]string),
 		},
@@ -178,19 +182,19 @@ func (w *nfdWorker) Run() error {
 		select {
 		case <-labelTrigger:
 			// Run feature discovery
-			for n, s := range source.GetAllFeatureSources() {
+			for _, s := range w.featureSources {
-				klog.V(2).Infof("running discovery for %q source", n)
+				klog.V(2).Infof("running discovery for %q source", s.Name())
 				if err := s.Discover(); err != nil {
-					klog.Errorf("feature discovery of %q source failed: %v", n, err)
+					klog.Errorf("feature discovery of %q source failed: %v", s.Name(), err)
 				}
 			}
 
 			// Get the set of feature labels.
-			labels := createFeatureLabels(w.enabledSources, w.config.Core.LabelWhiteList.Regexp)
+			labels := createFeatureLabels(w.labelSources, w.config.Core.LabelWhiteList.Regexp)
 
 			// Update the node with the feature labels.
 			if w.client != nil {
-				err := advertiseFeatureLabels(w.client, labels)
+				err := w.advertiseFeatureLabels(labels)
 				if err != nil {
 					return fmt.Errorf("failed to advertise labels: %s", err.Error())
 				}
@@ -294,12 +298,47 @@ func (w *nfdWorker) configureCore(c coreConfig) error {
 	}
 
 	// Determine enabled feature sources
-	enabled := make(map[string]source.LabelSource)
+	featureSources := make(map[string]source.FeatureSource)
+	for _, name := range c.FeatureSources {
+		if name == "all" {
+			for n, s := range source.GetAllFeatureSources() {
+				if ts, ok := s.(source.TestSource); !ok || !ts.IsTestSource() {
+					featureSources[n] = s
+				}
+			}
+		} else {
+			disable := false
+			strippedName := name
+			if strings.HasPrefix(name, "-") {
+				strippedName = name[1:]
+				disable = true
+			}
+			if s := source.GetFeatureSource(strippedName); s != nil {
+				if !disable {
+					featureSources[name] = s
+				} else {
+					delete(featureSources, strippedName)
+				}
+			} else {
+				klog.Warningf("skipping unknown feature source %q specified in core.featureSources", name)
+			}
+		}
+	}
+
+	w.featureSources = make([]source.FeatureSource, 0, len(featureSources))
+	for _, s := range featureSources {
+		w.featureSources = append(w.featureSources, s)
+	}
+
+	sort.Slice(w.featureSources, func(i, j int) bool { return w.featureSources[i].Name() < w.featureSources[j].Name() })
+
+	// Determine enabled label sources
+	labelSources := make(map[string]source.LabelSource)
 	for _, name := range c.LabelSources {
 		if name == "all" {
 			for n, s := range source.GetAllLabelSources() {
 				if ts, ok := s.(source.TestSource); !ok || !ts.IsTestSource() {
-					enabled[n] = s
+					labelSources[n] = s
 				}
 			}
 		} else {
@@ -311,9 +350,9 @@ func (w *nfdWorker) configureCore(c coreConfig) error {
 			}
 			if s := source.GetLabelSource(strippedName); s != nil {
 				if !disable {
-					enabled[strippedName] = s
+					labelSources[name] = s
 				} else {
-					delete(enabled, strippedName)
+					delete(labelSources, strippedName)
 				}
 			} else {
 				klog.Warningf("skipping unknown source %q specified in core.sources (or -sources)", name)
@@ -321,22 +360,28 @@ func (w *nfdWorker) configureCore(c coreConfig) error {
 		}
 	}
 
-	w.enabledSources = make([]source.LabelSource, 0, len(enabled))
+	w.labelSources = make([]source.LabelSource, 0, len(labelSources))
-	for _, s := range enabled {
+	for _, s := range labelSources {
-		w.enabledSources = append(w.enabledSources, s)
+		w.labelSources = append(w.labelSources, s)
 	}
 
-	sort.Slice(w.enabledSources, func(i, j int) bool {
+	sort.Slice(w.labelSources, func(i, j int) bool {
-		iP, jP := w.enabledSources[i].Priority(), w.enabledSources[j].Priority()
+		iP, jP := w.labelSources[i].Priority(), w.labelSources[j].Priority()
 		if iP != jP {
 			return iP < jP
 		}
-		return w.enabledSources[i].Name() < w.enabledSources[j].Name()
+		return w.labelSources[i].Name() < w.labelSources[j].Name()
 	})
 
 	if klog.V(1).Enabled() {
-		n := make([]string, len(w.enabledSources))
+		n := make([]string, len(w.featureSources))
-		for i, s := range w.enabledSources {
+		for i, s := range w.featureSources {
+			n[i] = s.Name()
+		}
+		klog.Infof("enabled feature sources: %s", strings.Join(n, ", "))
+
+		n = make([]string, len(w.labelSources))
+		for i, s := range w.labelSources {
 			n[i] = s.Name()
 		}
 		klog.Infof("enabled label sources: %s", strings.Join(n, ", "))
@@ -393,6 +438,9 @@ func (w *nfdWorker) configure(filepath string, overrides string) error {
 	if w.args.Overrides.SleepInterval != nil {
 		c.Core.SleepInterval = duration{*w.args.Overrides.SleepInterval}
 	}
+	if w.args.Overrides.FeatureSources != nil {
+		c.Core.FeatureSources = *w.args.Overrides.FeatureSources
+	}
 	if w.args.Overrides.LabelSources != nil {
 		c.Core.LabelSources = *w.args.Overrides.LabelSources
 	}
@@ -510,7 +558,7 @@ func getFeatures() map[string]*feature.DomainFeatures {
 
 // advertiseFeatureLabels advertises the feature labels to a Kubernetes node
 // via the NFD server.
-func advertiseFeatureLabels(client pb.LabelerClient, labels Labels) error {
+func (w *nfdWorker) advertiseFeatureLabels(labels Labels) error {
 	ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
 	defer cancel()
 
@@ -520,7 +568,7 @@ func advertiseFeatureLabels(client pb.LabelerClient, labels Labels) error {
 		Features:   getFeatures(),
 		NfdVersion: version.Get(),
 		NodeName:   nfdclient.NodeName()}
-	_, err := client.SetLabels(ctx, &labelReq)
+	_, err := w.client.SetLabels(ctx, &labelReq)
 	if err != nil {
 		klog.Errorf("failed to set node labels: %v", err)
 		return err