diff --git a/cmd/nfd-worker/main.go b/cmd/nfd-worker/main.go index 2b28475a2..01f93c4f9 100644 --- a/cmd/nfd-worker/main.go +++ b/cmd/nfd-worker/main.go @@ -19,6 +19,7 @@ package main import ( "fmt" "log" + "regexp" "strings" "time" @@ -92,18 +93,22 @@ func argsParse(argv []string) (worker.Args, error) { [Default: ] --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= Regular expression to filter label names to publish to the Kubernetes API server. NB: the label namespace is omitted i.e. the filter is only applied to the name part after '/'. - [Default: ] + (DEPRECATED: This parameter should be set via the + config file) --oneshot Label once and exit. --sleep-interval= Time to sleep between re-labeling. Non-positive value implies no re-labeling (i.e. infinite - sleep). [Default: 60s]`, + sleep). + (DEPRECATED: This parameter should be set via the + config file)`, ProgramName, ProgramName, ProgramName, @@ -114,21 +119,41 @@ func argsParse(argv []string) (worker.Args, error) { fmt.Sprintf("%s %s", ProgramName, version.Get())) // Parse argument values as usable types. - var err error args.CaFile = arguments["--ca-file"].(string) 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) - args.Sources = strings.Split(arguments["--sources"].(string), ",") - args.LabelWhiteList = arguments["--label-whitelist"].(string) args.Oneshot = arguments["--oneshot"].(bool) - args.SleepInterval, err = time.ParseDuration(arguments["--sleep-interval"].(string)) - if err != nil { - return args, fmt.Errorf("invalid --sleep-interval specified: %s", err.Error()) + + // Parse deprecated/override args + if v := arguments["--label-whitelist"]; v != nil { + s := v.(string) + // Compile labelWhiteList regex + if r, err := regexp.Compile(s); err != nil { + return args, fmt.Errorf("error parsing --label-whitelist regex (%s): %v", s, err) + } else { + args.LabelWhiteList = r + } + } + if arguments["--no-publish"].(bool) { + b := true + args.NoPublish = &b + } + if v := arguments["--sleep-interval"]; v != nil { + log.Printf("WARNING: --sleep-interval is deprecated, use 'core.sleepInterval' option in the config file, instead") + if s, err := time.ParseDuration(v.(string)); err != nil { + return args, fmt.Errorf("invalid --sleep-interval specified: %s", err.Error()) + } else { + args.SleepInterval = &s + } + } + if v := arguments["--sources"]; v != nil { + fmt.Println(v) + s := strings.Split(v.(string), ",") + args.Sources = &s } return args, nil } diff --git a/cmd/nfd-worker/main_test.go b/cmd/nfd-worker/main_test.go index f6cf3e539..d6b222bc5 100644 --- a/cmd/nfd-worker/main_test.go +++ b/cmd/nfd-worker/main_test.go @@ -23,19 +23,17 @@ 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() { args, err := argsParse([]string{"--no-publish", "--oneshot"}) 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.SleepInterval, ShouldEqual, nil) + So(*args.NoPublish, ShouldBeTrue) So(args.Oneshot, ShouldBeTrue) - So(args.Sources, ShouldResemble, allSources) - So(len(args.LabelWhiteList), ShouldEqual, 0) + So(args.Sources, ShouldBeNil) + So(args.LabelWhiteList, ShouldBeNil) So(err, ShouldBeNil) }) }) @@ -44,11 +42,11 @@ func TestArgsParse(t *testing.T) { args, err := argsParse([]string{"--sources=fake1,fake2,fake3", "--sleep-interval=30s"}) Convey("args.sources is set to appropriate values", func() { - So(args.SleepInterval, ShouldEqual, 30*time.Second) - So(args.NoPublish, ShouldBeFalse) + So(*args.SleepInterval, ShouldEqual, 30*time.Second) + So(args.NoPublish, ShouldBeNil) So(args.Oneshot, ShouldBeFalse) - So(args.Sources, ShouldResemble, []string{"fake1", "fake2", "fake3"}) - So(len(args.LabelWhiteList), ShouldEqual, 0) + So(*args.Sources, ShouldResemble, []string{"fake1", "fake2", "fake3"}) + So(args.LabelWhiteList, ShouldBeNil) So(err, ShouldBeNil) }) }) @@ -57,9 +55,9 @@ 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.Sources, ShouldResemble, allSources) - So(args.LabelWhiteList, ShouldResemble, ".*rdt.*") + So(args.NoPublish, ShouldBeNil) + So(args.Sources, ShouldBeNil) + So(args.LabelWhiteList.String(), ShouldResemble, ".*rdt.*") So(err, ShouldBeNil) }) }) @@ -68,12 +66,12 @@ 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") - So(args.Sources, ShouldResemble, []string{"fake1", "fake2", "fake3"}) - So(len(args.LabelWhiteList), ShouldEqual, 0) + So(*args.Sources, ShouldResemble, []string{"fake1", "fake2", "fake3"}) + So(args.LabelWhiteList, ShouldBeNil) So(err, ShouldBeNil) }) }) diff --git a/docs/advanced/worker-commandline-reference.md b/docs/advanced/worker-commandline-reference.md index ea191eb72..bbfed916b 100644 --- a/docs/advanced/worker-commandline-reference.md +++ b/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 @@ -172,6 +178,9 @@ expression in order to be published. Note: The regular expression is only matches against the "basename" part of the label, i.e. to the part of the name after '/'. The label namespace is omitted. +Note: This flag takes precedence over the `core.labelWhiteList` configuration +file option. + Default: *empty* Example: @@ -180,6 +189,9 @@ Example: nfd-worker --label-whitelist='.*cpuid\.' ``` +**DEPRECATED**: you should use the `core.labelWhiteList` option in the +configuration file, instead. + ### --oneshot The `--oneshot` flag causes nfd-worker to exit after one pass of feature @@ -199,6 +211,9 @@ The `--sleep-interval` specifies the interval between feature re-detection (and node re-labeling). A non-positive value implies infinite sleep interval, i.e. no re-detection or re-labeling is done. +Note: This flag takes precedence over the `core.sleepInterval` configuration +file option. + Default: 60s Example: @@ -206,3 +221,7 @@ Example: ```bash nfd-worker --sleep-interval=1h ``` + +**DEPRECATED**: you should use the `core.sleepInterval` option in the +configuration file, instead. + diff --git a/docs/advanced/worker-configuration-reference.md b/docs/advanced/worker-configuration-reference.md index 3129418db..27dd98187 100644 --- a/docs/advanced/worker-configuration-reference.md +++ b/docs/advanced/worker-configuration-reference.md @@ -2,7 +2,6 @@ title: "Worker Config Reference" layout: default sort: 4 -published: false --- # NFD-Worker Configuration File Reference @@ -11,12 +10,108 @@ published: false ## Table of contents {: .no_toc .text-delta } +***WORK IN PROGRESS*** + 1. TOC {:toc} --- -***WORK IN PROGRESS*** +See the +[sample configuration file](https://github.com/kubernetes-sigs/node-feature-discovery/blob/{{ site.release }}/nfd-worker.conf.example) +for a full example configuration. -This section is a reference to all the configuration settings in the worker -config file. +## core + +The `core` section contains common configuration settings that are not specific +to any particular feature source. + +### core.sleepInterval + +`core.sleepInterval` specifies the interval between consecutive passes of +feature (re-)detection, and thus also the interval between node re-labeling. A +non-positive value implies infinite sleep interval, i.e. no re-detection or +re-labeling is done. + +Note: Overridden by the deprecated `--sleep-interval` command line flag (if +specified). + +Default: `60s` + +Example: + +```yaml +core: + sleepInterval: 60s +``` + +### core.sources + +`core.sources` specifies the list of enabled feature sources. A special value +`all` enables all feature sources. + +Note: Overridden by the deprecated `--sources` command line flag (if +specified). + +Default: `[all]` + +Example: + +```yaml +core: + sources: + - system + - custom +``` + +### core.labelWhiteList + +`core.labelWhiteList` specifies a regular expression for filtering feature +labels based on the label name. Non-matching labels are not published. + +Note: The regular expression is only matches against the "basename" part of the +label, i.e. to the part of the name after '/'. The label prefix (or namespace) +is omitted. + +Note: Overridden by the deprecated `--label-whitelist` command line flag (if +specified). + +Default: `null` + +Example: + +```yaml +core: + labelWhiteList: '^cpu-cpuid' +``` + +### core.noPublish + +Setting `core.noPublish` to `true` disables all communication with the +nfd-master. It is effectively a "dry-run" flag: nfd-worker runs feature +detection normally, but no labeling requests are sent to nfd-master. + +Note: Overridden by the `--no-publish` command line flag (if specified). + +Default: `false` + +Example: + +```yaml +core: + noPublish: true +``` + +## sources + +The `sources` section contains feature source specific configuration parameters. + +### sources.cpu + +### sources.kernel + +### soures.pci + +### sources.usb + +### sources.custom diff --git a/docs/get-started/deployment-and-usage.md b/docs/get-started/deployment-and-usage.md index f2a88a0c3..d4ec9dcc7 100644 --- a/docs/get-started/deployment-and-usage.md +++ b/docs/get-started/deployment-and-usage.md @@ -164,12 +164,11 @@ each nfd-worker requires a individual node-specific TLS certificate. ## Configuration -NFD-Worker supports a configuration file. The default location is -`/etc/kubernetes/node-feature-discovery/nfd-worker.conf`, but, -this can be changed by specifying the`--config` command line flag. -Configuration file is re-read on each labeling pass (determined by -`--sleep-interval`) which makes run-time re-configuration of nfd-worker -possible. +NFD-Worker supports dynamic configuration through a configuration file. The +default location is `/etc/kubernetes/node-feature-discovery/nfd-worker.conf`, +but, this can be changed by specifying the`--config` command line flag. +Configuration file is re-read whenever it is modified which makes run-time +re-configuration of nfd-worker straightforward. Worker configuration file is read inside the container, and thus, Volumes and VolumeMounts are needed to make your configuration available for NFD. The @@ -183,6 +182,9 @@ mount it inside the nfd-worker containers. Configuration can be edited with: kubectl -n ${NFD_NS} edit configmap nfd-worker-conf ``` +See +[nfd-worker configuration file reference](../advanced/worker-configuration-reference.md) +for more details. The (empty-by-default) [example config](https://github.com/kubernetes-sigs/node-feature-discovery/blob/{{ site.release }}/nfd-worker.conf.example) contains all available configuration options and can be used as a reference diff --git a/nfd-daemonset-combined.yaml.template b/nfd-daemonset-combined.yaml.template index 1e0f1611a..5dcc3c2a0 100644 --- a/nfd-daemonset-combined.yaml.template +++ b/nfd-daemonset-combined.yaml.template @@ -142,6 +142,11 @@ metadata: namespace: node-feature-discovery data: nfd-worker.conf: | ### + #core: + # labelWhiteList: + # noPublish: false + # sleepInterval: 60s + # sources: [all] #sources: # cpu: # cpuid: diff --git a/nfd-worker-daemonset.yaml.template b/nfd-worker-daemonset.yaml.template index 788b596bd..6f700e107 100644 --- a/nfd-worker-daemonset.yaml.template +++ b/nfd-worker-daemonset.yaml.template @@ -102,6 +102,11 @@ metadata: namespace: node-feature-discovery data: nfd-worker.conf: | ### + #core: + # labelWhiteList: + # noPublish: false + # sleepInterval: 60s + # sources: [all] #sources: # cpu: # cpuid: diff --git a/nfd-worker-job.yaml.template b/nfd-worker-job.yaml.template index 8f752727b..bb19c023e 100644 --- a/nfd-worker-job.yaml.template +++ b/nfd-worker-job.yaml.template @@ -112,6 +112,11 @@ metadata: namespace: node-feature-discovery data: nfd-worker.conf: | ### + #core: + # labelWhiteList: + # noPublish: false + # sleepInterval: 60s + # sources: [all] #sources: # cpu: # cpuid: diff --git a/nfd-worker.conf.example b/nfd-worker.conf.example index 6197b199b..8f9447c1a 100644 --- a/nfd-worker.conf.example +++ b/nfd-worker.conf.example @@ -1,3 +1,8 @@ +#core: +# labelWhiteList: +# noPublish: false +# sleepInterval: 60s +# sources: [all] #sources: # cpu: # cpuid: diff --git a/pkg/nfd-worker/nfd-worker-internal_test.go b/pkg/nfd-worker/nfd-worker-internal_test.go index 436ce4053..eaeffa122 100644 --- a/pkg/nfd-worker/nfd-worker-internal_test.go +++ b/pkg/nfd-worker/nfd-worker-internal_test.go @@ -20,13 +20,17 @@ import ( "fmt" "io/ioutil" "os" + "path/filepath" "regexp" "strings" "testing" + "time" + //"github.com/onsi/gomega" . "github.com/smartystreets/goconvey/convey" "github.com/stretchr/testify/mock" "github.com/vektra/errors" + "sigs.k8s.io/node-feature-discovery/pkg/labeler" "sigs.k8s.io/node-feature-discovery/source" "sigs.k8s.io/node-feature-discovery/source/cpu" @@ -49,7 +53,7 @@ func TestDiscoveryWithMockSources(t *testing.T) { fakeFeatureSource := source.FeatureSource(mockFeatureSource) - labelWhiteList := regexp.MustCompile("^test") + labelWhiteList := regex{*regexp.MustCompile("^test")} Convey("When I successfully get the labels from the mock source", func() { mockFeatureSource.On("Name").Return(fakeFeatureSourceName) @@ -95,7 +99,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,14 +109,29 @@ 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{}) So(err, ShouldBeNil) worker := w.(*nfdWorker) - Convey("and a non-accessible file and some overrides are specified", func() { - overrides := `{"sources": {"cpu": {"cpuid": {"attributeBlacklist": ["foo","bar"]}}}}` - worker.configure("non-existing-file", overrides) + overrides := `{"core": {"sources": ["fake"],"noPublish": true},"sources": {"cpu": {"cpuid": {"attributeBlacklist": ["foo","bar"]}}}}` + Convey("and no core cmdline flags have been specified", func() { + So(worker.configure("non-existing-file", overrides), ShouldBeNil) + + Convey("core overrides should be in effect", func() { + So(worker.config.Core.Sources, ShouldResemble, []string{"fake"}) + 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{Sources: &[]string{"cpu", "kernel", "pci"}} + So(worker.configure("non-existing-file", overrides), ShouldBeNil) + + Convey("core cmdline flags should be in effect instead overrides", func() { + So(worker.config.Core.Sources, ShouldResemble, []string{"cpu", "kernel", "pci"}) + }) Convey("overrides should take effect", func() { + So(worker.config.Core.NoPublish, ShouldBeTrue) + c := worker.getSource("cpu").GetConfig().(*cpu.Config) So(c.Cpuid.AttributeBlacklist, ShouldResemble, []string{"foo", "bar"}) }) @@ -121,7 +140,13 @@ func TestConfigParse(t *testing.T) { f, err := ioutil.TempFile("", "nfd-test-") defer os.Remove(f.Name()) So(err, ShouldBeNil) - _, err = f.WriteString(`sources: + _, err = f.WriteString(` +core: + noPublish: false + sources: ["system"] + labelWhiteList: "foo" + sleepInterval: "10s" +sources: kernel: configOpts: - "DMI" @@ -132,9 +157,17 @@ func TestConfigParse(t *testing.T) { So(err, ShouldBeNil) Convey("and a proper config file is specified", func() { - worker.configure(f.Name(), "") + worker.args = Args{Sources: &[]string{"cpu", "kernel", "pci"}} + So(worker.configure(f.Name(), ""), ShouldBeNil) Convey("specified configuration should take effect", func() { + // Verify core config + So(worker.config.Core.NoPublish, ShouldBeFalse) + So(worker.config.Core.Sources, 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) + + // Verify feature source config So(err, ShouldBeNil) c := worker.getSource("kernel").GetConfig() So(c.(*kernel.Config).ConfigOpts, ShouldResemble, []string{"DMI"}) @@ -144,10 +177,19 @@ func TestConfigParse(t *testing.T) { }) Convey("and a proper config file and overrides are given", func() { - overrides := `{"sources": {"pci": {"deviceClassWhitelist": ["03"]}}}` - worker.configure(f.Name(), overrides) + sleepIntervalArg := 15 * time.Second + worker.args = Args{SleepInterval: &sleepIntervalArg} + overrides := `{"core": {"sources": ["fake"],"noPublish": true},"sources": {"pci": {"deviceClassWhitelist": ["03"]}}}` + So(worker.configure(f.Name(), overrides), ShouldBeNil) Convey("overrides should take precedence over the config file", func() { + // Verify core config + So(worker.config.Core.NoPublish, ShouldBeTrue) + So(worker.config.Core.Sources, 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 + + // Verify feature source config So(err, ShouldBeNil) c := worker.getSource("kernel").GetConfig() So(c.(*kernel.Config).ConfigOpts, ShouldResemble, []string{"DMI"}) @@ -158,60 +200,157 @@ func TestConfigParse(t *testing.T) { }) } +func TestDynamicConfig(t *testing.T) { + Convey("When running nfd-worker", t, func() { + tmpDir, err := ioutil.TempDir("", "*.nfd-test") + So(err, ShouldBeNil) + defer os.RemoveAll(tmpDir) + + // Create (temporary) dir for config + configDir := filepath.Join(tmpDir, "subdir-1", "subdir-2", "worker.conf") + err = os.MkdirAll(configDir, 0755) + So(err, ShouldBeNil) + + // Create config file + configFile := filepath.Join(configDir, "worker.conf") + + writeConfig := func(data string) { + f, err := os.Create(configFile) + So(err, ShouldBeNil) + _, err = f.WriteString(data) + So(err, ShouldBeNil) + err = f.Close() + So(err, ShouldBeNil) + + } + writeConfig(` +core: + labelWhiteList: "fake" +`) + + noPublish := true + w, err := NewNfdWorker(Args{ + ConfigFile: configFile, + Sources: &[]string{"fake"}, + NoPublish: &noPublish, + SleepInterval: new(time.Duration), + }) + So(err, ShouldBeNil) + worker := w.(*nfdWorker) + + Convey("config file updates should take effect", func() { + go func() { _ = w.Run() }() + defer w.Stop() + + // Check initial config + So(func() interface{} { return worker.config.Core.LabelWhiteList.String() }, + withTimeout, 2*time.Second, ShouldEqual, "fake") + + // Update config and verify the effect + writeConfig(` +core: + labelWhiteList: "foo" +`) + So(func() interface{} { return worker.config.Core.LabelWhiteList.String() }, + withTimeout, 2*time.Second, ShouldEqual, "foo") + + // Removing config file should get back our defaults + err = os.RemoveAll(tmpDir) + So(err, ShouldBeNil) + So(func() interface{} { return worker.config.Core.LabelWhiteList.String() }, + withTimeout, 2*time.Second, ShouldEqual, "") + + // Re-creating config dir and file should change the config + err = os.MkdirAll(configDir, 0755) + So(err, ShouldBeNil) + writeConfig(` +core: + labelWhiteList: "bar" +`) + So(func() interface{} { return worker.config.Core.LabelWhiteList.String() }, + withTimeout, 2*time.Second, ShouldEqual, "bar") + }) + }) +} + +// withTimeout is a custom assertion for polling a value asynchronously +// actual is a function for getting the actual value +// expected[0] is a time.Duration value specifying the timeout +// expected[1] is the "real" assertion function to be called +// expected[2:] are the arguments for the "real" assertion function +func withTimeout(actual interface{}, expected ...interface{}) string { + getter, ok := actual.(func() interface{}) + if !ok { + return "not getterFunc" + } + t, ok := expected[0].(time.Duration) + if !ok { + return "not time.Duration" + } + f, ok := expected[1].(func(interface{}, ...interface{}) string) + if !ok { + return "not an assert func" + } + + timeout := time.After(t) + for { + result := f(getter(), expected[2:]...) + if result == "" { + return "" + } + select { + case <-timeout: + return result + case <-time.After(10 * time.Millisecond): + } + } +} + func TestNewNfdWorker(t *testing.T) { Convey("When creating new NfdWorker instance", t, func() { + emptyRegexp := regex{*regexp.MustCompile("")} + Convey("without any args specified", func() { args := Args{} - emptyRegexp, _ := regexp.Compile("") w, err := NewNfdWorker(args) Convey("no error should be returned", func() { So(err, ShouldBeNil) }) worker := w.(*nfdWorker) - 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) + So(worker.configure("", ""), ShouldBeNil) + Convey("all sources should be enabled and the whitelist regexp should be empty", func() { + 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"}} - emptyRegexp, _ := regexp.Compile("") + args := Args{Sources: &[]string{"fake"}} 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.sources), ShouldEqual, 1) - So(worker.sources[0], ShouldHaveSameTypeAs, &fake.Source{}) - So(worker.labelWhiteList, ShouldResemble, emptyRegexp) - }) - }) - - Convey("with invalid LabelWhiteList arg specified", func() { - args := Args{LabelWhiteList: "*"} - w, err := NewNfdWorker(args) - worker := w.(*nfdWorker) - Convey("an error should be returned", func() { - So(len(worker.sources), ShouldEqual, 0) - So(worker.labelWhiteList, ShouldBeNil) - So(err, ShouldNotBeNil) + So(len(worker.enabledSources), ShouldEqual, 1) + So(worker.enabledSources[0], ShouldHaveSameTypeAs, &fake.Source{}) + So(worker.config.Core.LabelWhiteList, ShouldResemble, emptyRegexp) }) }) Convey("with valid LabelWhiteListStr arg specified", func() { - args := Args{LabelWhiteList: ".*rdt.*"} + args := Args{LabelWhiteList: regexp.MustCompile(".*rdt.*")} w, err := NewNfdWorker(args) Convey("no error should be returned", func() { So(err, ShouldBeNil) }) worker := w.(*nfdWorker) - expectRegexp := regexp.MustCompile(".*rdt.*") + So(worker.configure("", ""), ShouldBeNil) + expectRegexp := regex{*regexp.MustCompile(".*rdt.*")} Convey("proper labelWhiteList regexp should be produced", func() { - So(len(worker.sources), ShouldEqual, 0) - So(worker.labelWhiteList, ShouldResemble, expectRegexp) + So(worker.config.Core.LabelWhiteList, ShouldResemble, expectRegexp) }) }) }) @@ -220,8 +359,9 @@ func TestNewNfdWorker(t *testing.T) { func TestCreateFeatureLabels(t *testing.T) { Convey("When creating feature labels from the configured sources", t, func() { Convey("When fake feature source is configured", func() { - emptyLabelWL, _ := regexp.Compile("") + emptyLabelWL := regex{*regexp.MustCompile("")} fakeFeatureSource := source.FeatureSource(new(fake.Source)) + fakeFeatureSource.SetConfig(fakeFeatureSource.NewConfig()) sources := []source.FeatureSource{} sources = append(sources, fakeFeatureSource) labels := createFeatureLabels(sources, emptyLabelWL) @@ -234,11 +374,11 @@ func TestCreateFeatureLabels(t *testing.T) { }) }) Convey("When fake feature source is configured with a whitelist that doesn't match", func() { - emptyLabelWL, _ := regexp.Compile(".*rdt.*") + labelWL := regex{*regexp.MustCompile(".*rdt.*")} fakeFeatureSource := source.FeatureSource(new(fake.Source)) sources := []source.FeatureSource{} sources = append(sources, fakeFeatureSource) - labels := createFeatureLabels(sources, emptyLabelWL) + labels := createFeatureLabels(sources, labelWL) Convey("fake labels are not returned", func() { So(len(labels), ShouldEqual, 0) @@ -254,7 +394,7 @@ func TestGetFeatureLabels(t *testing.T) { Convey("When I get feature labels and panic occurs during discovery of a feature source", t, func() { fakePanicFeatureSource := source.FeatureSource(new(panicfake.Source)) - returnedLabels, err := getFeatureLabels(fakePanicFeatureSource, regexp.MustCompile("")) + returnedLabels, err := getFeatureLabels(fakePanicFeatureSource, regex{*regexp.MustCompile("")}) Convey("No label is returned", func() { So(len(returnedLabels), ShouldEqual, 0) }) diff --git a/pkg/nfd-worker/nfd-worker.go b/pkg/nfd-worker/nfd-worker.go index 527fefbd8..454eede3d 100644 --- a/pkg/nfd-worker/nfd-worker.go +++ b/pkg/nfd-worker/nfd-worker.go @@ -61,9 +61,17 @@ var ( // Global config type NFDConfig struct { + Core coreConfig Sources sourcesConfig } +type coreConfig struct { + LabelWhiteList regex + NoPublish bool + Sources []string + SleepInterval duration +} + type sourcesConfig map[string]source.Config // Labels are a Kubernetes representation of discovered features. @@ -71,43 +79,75 @@ type Labels map[string]string // Command line arguments type Args struct { - LabelWhiteList string CaFile string 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 + LabelWhiteList *regexp.Regexp + NoPublish *bool + SleepInterval *time.Duration + Sources *[]string } type NfdWorker interface { Run() error + Stop() } type nfdWorker struct { args Args clientConn *grpc.ClientConn client pb.LabelerClient - config NFDConfig - sources []source.FeatureSource - labelWhiteList *regexp.Regexp + configFilePath string + config *NFDConfig + realSources []source.FeatureSource + stop chan struct{} // channel for signaling stop + testSources []source.FeatureSource + enabledSources []source.FeatureSource +} + +type regex struct { + regexp.Regexp +} + +type duration struct { + time.Duration } // Create new NfdWorker instance. func NewNfdWorker(args Args) (NfdWorker, error) { nfd := &nfdWorker{ - args: args, - sources: []source.FeatureSource{}, + args: args, + config: &NFDConfig{}, + 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{}, + }, + stop: make(chan struct{}, 1), } - 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 + if args.ConfigFile != "" { + nfd.configFilePath = filepath.Clean(args.ConfigFile) } // Check TLS related args @@ -123,60 +163,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, ", ")) - } - } - - // Compile labelWhiteList regex - var err error - nfd.labelWhiteList, err = regexp.Compile(args.LabelWhiteList) - if err != nil { - return nfd, fmt.Errorf("error parsing label whitelist regex (%s): %s", args.LabelWhiteList, err) - } - return nfd, nil } @@ -214,34 +200,45 @@ func addConfigWatch(path string) (*fsnotify.Watcher, map[string]struct{}, error) return w, paths, nil } +func newDefaultConfig() *NFDConfig { + return &NFDConfig{ + Core: coreConfig{ + LabelWhiteList: regex{*regexp.MustCompile("")}, + SleepInterval: duration{60 * time.Second}, + Sources: []string{"all"}, + }, + } +} + // 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 + } + if err := w.configure(w.configFilePath, w.args.Options); err != nil { + return err + } + // 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 { select { case <-labelTrigger: // Get the set of feature labels. - labels := createFeatureLabels(w.sources, w.labelWhiteList) + labels := createFeatureLabels(w.enabledSources, w.config.Core.LabelWhiteList) // Update the node with the feature labels. if w.client != nil { @@ -255,8 +252,8 @@ func (w *nfdWorker) Run() error { return nil } - if w.args.SleepInterval > 0 { - labelTrigger = time.After(w.args.SleepInterval) + if w.config.Core.SleepInterval.Duration > 0 { + labelTrigger = time.After(w.config.Core.SleepInterval.Duration) } case e := <-configWatch.Events: @@ -270,7 +267,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,19 +282,41 @@ func (w *nfdWorker) Run() error { stderrLogger.Printf("ERROR: config file watcher error: %v", e) case <-configTrigger: - w.configure(configFilePath, w.args.Options) - + if err := w.configure(w.configFilePath, w.args.Options); err != nil { + return err + } + // 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) + + case <-w.stop: + stdoutLogger.Printf("shutting down nfd-worker") + configWatch.Close() + return nil } } } +// Stop NfdWorker +func (w *nfdWorker) Stop() { + select { + case w.stop <- struct{}{}: + default: + } +} + // 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 } @@ -354,44 +373,111 @@ func (w *nfdWorker) disconnect() { w.client = nil } +func (c *coreConfig) sanitize() { + if c.SleepInterval.Duration > 0 && c.SleepInterval.Duration < time.Second { + stderrLogger.Printf("WARNING: too short sleep-intervall specified (%s), forcing to 1s", + c.SleepInterval.Duration.String()) + c.SleepInterval = duration{time.Second} + } +} + +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) { +func (w *nfdWorker) configure(filepath string, overrides string) error { // Create a new default config - c := NFDConfig{Sources: make(map[string]source.Config, len(w.sources))} - for _, s := range w.sources { + c := newDefaultConfig() + allSources := append(w.realSources, w.testSources...) + c.Sources = make(map[string]source.Config, len(allSources)) + for _, s := range allSources { c.Sources[s.Name()] = s.NewConfig() } // Try to read and parse config file - data, err := ioutil.ReadFile(filepath) - if err != nil { - stderrLogger.Printf("Failed to read config file: %s", err) - } else { - err = yaml.Unmarshal(data, &c) + if filepath != "" { + data, err := ioutil.ReadFile(filepath) if err != nil { - stderrLogger.Printf("Failed to parse config file: %s", err) + if os.IsNotExist(err) { + stderrLogger.Printf("config file %q not found, using defaults", filepath) + } else { + return fmt.Errorf("error reading config file: %s", err) + } } else { + err = yaml.Unmarshal(data, c) + if err != nil { + return fmt.Errorf("Failed to parse config file: %s", err) + } stdoutLogger.Printf("Configuration successfully loaded from %q", filepath) } } // Parse config overrides - err = yaml.Unmarshal([]byte(overrides), &c) - if err != nil { - stderrLogger.Printf("Failed to parse --options: %s", err) + if err := yaml.Unmarshal([]byte(overrides), c); err != nil { + return fmt.Errorf("Failed to parse --options: %s", err) } + if w.args.LabelWhiteList != nil { + c.Core.LabelWhiteList = regex{*w.args.LabelWhiteList} + } + if w.args.NoPublish != nil { + c.Core.NoPublish = *w.args.NoPublish + } + 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 - for _, s := range w.sources { + w.configureCore(c.Core) + + // (Re-)configure all "real" sources, test sources are not configurable + for _, s := range allSources { s.SetConfig(c.Sources[s.Name()]) } + + return nil } // createFeatureLabels returns the set of feature labels from the enabled // sources and the whitelist argument. -func createFeatureLabels(sources []source.FeatureSource, labelWhiteList *regexp.Regexp) (labels Labels) { +func createFeatureLabels(sources []source.FeatureSource, labelWhiteList regex) (labels Labels) { labels = Labels{} // Do feature discovery from all configured sources. @@ -414,7 +500,7 @@ func createFeatureLabels(sources []source.FeatureSource, labelWhiteList *regexp. // getFeatureLabels returns node labels for features discovered by the // supplied source. -func getFeatureLabels(source source.FeatureSource, labelWhiteList *regexp.Regexp) (labels Labels, err error) { +func getFeatureLabels(source source.FeatureSource, labelWhiteList regex) (labels Labels, err error) { defer func() { if r := recover(); r != nil { stderrLogger.Printf("panic occurred during discovery of source [%s]: %v", source.Name(), r) @@ -498,6 +584,46 @@ func advertiseFeatureLabels(client pb.LabelerClient, labels Labels) error { return nil } +// UnmarshalJSON implements the Unmarshaler interface from "encoding/json" +func (r *regex) UnmarshalJSON(data []byte) error { + var v interface{} + if err := json.Unmarshal(data, &v); err != nil { + return err + } + switch val := v.(type) { + case string: + if rr, err := regexp.Compile(string(val)); err != nil { + return err + } else { + *r = regex{*rr} + } + default: + return fmt.Errorf("invalid regexp %s", data) + } + return nil +} + +// UnmarshalJSON implements the Unmarshaler interface from "encoding/json" +func (d *duration) UnmarshalJSON(data []byte) error { + var v interface{} + if err := json.Unmarshal(data, &v); err != nil { + return err + } + switch val := v.(type) { + case float64: + d.Duration = time.Duration(val) + case string: + var err error + d.Duration, err = time.ParseDuration(val) + if err != nil { + return err + } + default: + return fmt.Errorf("invalid duration %s", data) + } + return nil +} + // UnmarshalJSON implements the Unmarshaler interface from "encoding/json" func (c *sourcesConfig) UnmarshalJSON(data []byte) error { // First do a raw parse to get the per-source data diff --git a/pkg/nfd-worker/nfd-worker_test.go b/pkg/nfd-worker/nfd-worker_test.go index 5cc8e598c..2707a98bb 100644 --- a/pkg/nfd-worker/nfd-worker_test.go +++ b/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) diff --git a/source/fake/fake.go b/source/fake/fake.go index 63326c2e0..4b9d56c8b 100644 --- a/source/fake/fake.go +++ b/source/fake/fake.go @@ -16,22 +16,51 @@ limitations under the License. package fake -import "sigs.k8s.io/node-feature-discovery/source" +import ( + "fmt" + + "sigs.k8s.io/node-feature-discovery/source" +) + +// Configuration file options +type Config struct { + Labels map[string]string `json:"labels"` +} + +// newDefaultConfig returns a new config with defaults values +func newDefaultConfig() *Config { + return &Config{ + Labels: map[string]string{ + "fakefeature1": "true", + "fakefeature2": "true", + "fakefeature3": "true", + }, + } +} // Source implements FeatureSource. -type Source struct{} +type Source struct { + config *Config +} // Name returns an identifier string for this feature source. func (s Source) Name() string { return "fake" } // NewConfig method of the FeatureSource interface -func (s *Source) NewConfig() source.Config { return nil } +func (s *Source) NewConfig() source.Config { return newDefaultConfig() } // GetConfig method of the FeatureSource interface -func (s *Source) GetConfig() source.Config { return nil } +func (s *Source) GetConfig() source.Config { return s.config } // SetConfig method of the FeatureSource interface -func (s *Source) SetConfig(source.Config) {} +func (s *Source) SetConfig(conf source.Config) { + switch v := conf.(type) { + case *Config: + s.config = v + default: + panic(fmt.Sprintf("invalid config type: %T", conf)) + } +} // Configure method of the FeatureSource interface func (s Source) Configure([]byte) error { return nil } @@ -39,10 +68,9 @@ func (s Source) Configure([]byte) error { return nil } // Discover returns feature names for some fake features. func (s Source) Discover() (source.Features, error) { // Adding three fake features. - features := source.Features{ - "fakefeature1": true, - "fakefeature2": true, - "fakefeature3": true, + features := make(source.Features, len(s.config.Labels)) + for k, v := range s.config.Labels { + features[k] = v } return features, nil