diff --git a/cmd/nfd-worker/main.go b/cmd/nfd-worker/main.go index 8212887f3..01f93c4f9 100644 --- a/cmd/nfd-worker/main.go +++ b/cmd/nfd-worker/main.go @@ -93,7 +93,8 @@ 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 @@ -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 } diff --git a/cmd/nfd-worker/main_test.go b/cmd/nfd-worker/main_test.go index f66843354..d6b222bc5 100644 --- a/cmd/nfd-worker/main_test.go +++ b/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) }) diff --git a/docs/advanced/worker-commandline-reference.md b/docs/advanced/worker-commandline-reference.md index 6654c8bef..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 diff --git a/nfd-daemonset-combined.yaml.template b/nfd-daemonset-combined.yaml.template index 7914776a5..5dcc3c2a0 100644 --- a/nfd-daemonset-combined.yaml.template +++ b/nfd-daemonset-combined.yaml.template @@ -146,6 +146,7 @@ data: # 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 f99ba0869..6f700e107 100644 --- a/nfd-worker-daemonset.yaml.template +++ b/nfd-worker-daemonset.yaml.template @@ -106,6 +106,7 @@ data: # 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 12a3de20d..bb19c023e 100644 --- a/nfd-worker-job.yaml.template +++ b/nfd-worker-job.yaml.template @@ -116,6 +116,7 @@ data: # labelWhiteList: # noPublish: false # sleepInterval: 60s + # sources: [all] #sources: # cpu: # cpuid: diff --git a/nfd-worker.conf.example b/nfd-worker.conf.example index e9e199e18..8f9447c1a 100644 --- a/nfd-worker.conf.example +++ b/nfd-worker.conf.example @@ -2,6 +2,7 @@ # 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 948d37097..eaffc2f20 100644 --- a/pkg/nfd-worker/nfd-worker-internal_test.go +++ b/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() { - So(len(worker.sources), ShouldEqual, 0) + 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"}} + 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(worker.sources[0], ShouldHaveSameTypeAs, &fake.Source{}) + So(len(worker.enabledSources), ShouldEqual, 1) + 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) }) }) diff --git a/pkg/nfd-worker/nfd-worker.go b/pkg/nfd-worker/nfd-worker.go index 2ce83802f..ebbf59ec4 100644 --- a/pkg/nfd-worker/nfd-worker.go +++ b/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, - config: &NFDConfig{}, - 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{}, + }, } 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)) - for _, s := range w.sources { + 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() } @@ -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 - 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()]) } } 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)