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,13 +47,15 @@ func TestParseArgs(t *testing.T) {
 			args := parseArgs(flags,
 				"-no-publish",
 				"-label-whitelist=.*rdt.*",
+				"-feature-sources=cpu",
 				"-label-sources=fake1,fake2,fake3",
 				"-sleep-interval=30s")
 
 			Convey("args.sources is set to appropriate values", func() {
 				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,22 +315,27 @@ 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)
 			})
 			worker := w.(*nfdWorker)
 			So(worker.configure("", ""), ShouldBeNil)
 			Convey("proper sources should be enabled", func() {
-				So(len(worker.enabledSources), ShouldEqual, 1)
-				So(worker.enabledSources[0].Name(), ShouldEqual, "fake")
+				So(len(worker.featureSources), ShouldEqual, 1)
+				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,20 +388,26 @@ 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)
 			})
 		})
 		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)
 			})