From d1d8de944e84ec33218c88a0d648510cfe2e792d Mon Sep 17 00:00:00 2001 From: Markus Lehtonen Date: Tue, 1 Dec 2020 14:27:47 +0200 Subject: [PATCH] nfd-worker: add core.sleepInterval config option Add a new config file option for (dynamically) controlling the sleep interval. At the same time, deprecate the --sleep-interval command line flag. The command line flag takes precedence over the config file option. --- cmd/nfd-worker/main.go | 18 +++--- cmd/nfd-worker/main_test.go | 4 +- docs/advanced/worker-commandline-reference.md | 7 +++ nfd-daemonset-combined.yaml.template | 1 + nfd-worker-daemonset.yaml.template | 1 + nfd-worker-job.yaml.template | 1 + nfd-worker.conf.example | 1 + pkg/nfd-worker/nfd-worker.go | 58 +++++++++++++++---- 8 files changed, 71 insertions(+), 20 deletions(-) diff --git a/cmd/nfd-worker/main.go b/cmd/nfd-worker/main.go index e3798a038..8a7ded05e 100644 --- a/cmd/nfd-worker/main.go +++ b/cmd/nfd-worker/main.go @@ -103,7 +103,9 @@ func argsParse(argv []string) (worker.Args, error) { --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,7 +116,6 @@ 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) @@ -125,16 +126,19 @@ func argsParse(argv []string) (worker.Args, error) { 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 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 + } + } return args, nil } diff --git a/cmd/nfd-worker/main_test.go b/cmd/nfd-worker/main_test.go index 0dbf47853..18e1298c4 100644 --- a/cmd/nfd-worker/main_test.go +++ b/cmd/nfd-worker/main_test.go @@ -31,7 +31,7 @@ func TestArgsParse(t *testing.T) { 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.SleepInterval, ShouldEqual, nil) So(*args.NoPublish, ShouldBeTrue) So(args.Oneshot, ShouldBeTrue) So(args.Sources, ShouldResemble, allSources) @@ -44,7 +44,7 @@ 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.SleepInterval, ShouldEqual, 30*time.Second) So(args.NoPublish, ShouldBeNil) So(args.Oneshot, ShouldBeFalse) So(args.Sources, ShouldResemble, []string{"fake1", "fake2", "fake3"}) diff --git a/docs/advanced/worker-commandline-reference.md b/docs/advanced/worker-commandline-reference.md index ea191eb72..10b083c0c 100644 --- a/docs/advanced/worker-commandline-reference.md +++ b/docs/advanced/worker-commandline-reference.md @@ -199,6 +199,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 +209,7 @@ Example: ```bash nfd-worker --sleep-interval=1h ``` + +**DEPRECATED**: you should use the `core.sleepInterval` option in the +configuration file, instead. + diff --git a/nfd-daemonset-combined.yaml.template b/nfd-daemonset-combined.yaml.template index a350caba5..0a4c61740 100644 --- a/nfd-daemonset-combined.yaml.template +++ b/nfd-daemonset-combined.yaml.template @@ -144,6 +144,7 @@ data: nfd-worker.conf: | ### #core: # noPublish: false + # sleepInterval: 60s #sources: # cpu: # cpuid: diff --git a/nfd-worker-daemonset.yaml.template b/nfd-worker-daemonset.yaml.template index 5b2c5e2de..659687d8a 100644 --- a/nfd-worker-daemonset.yaml.template +++ b/nfd-worker-daemonset.yaml.template @@ -104,6 +104,7 @@ data: nfd-worker.conf: | ### #core: # noPublish: false + # sleepInterval: 60s #sources: # cpu: # cpuid: diff --git a/nfd-worker-job.yaml.template b/nfd-worker-job.yaml.template index 56d2044bf..77fa98b41 100644 --- a/nfd-worker-job.yaml.template +++ b/nfd-worker-job.yaml.template @@ -114,6 +114,7 @@ data: nfd-worker.conf: | ### #core: # noPublish: false + # sleepInterval: 60s #sources: # cpu: # cpuid: diff --git a/nfd-worker.conf.example b/nfd-worker.conf.example index 22577e33f..b741e78f1 100644 --- a/nfd-worker.conf.example +++ b/nfd-worker.conf.example @@ -1,5 +1,6 @@ #core: # noPublish: false +# sleepInterval: 60s #sources: # cpu: # cpuid: diff --git a/pkg/nfd-worker/nfd-worker.go b/pkg/nfd-worker/nfd-worker.go index 5d880c45e..48ebc4eda 100644 --- a/pkg/nfd-worker/nfd-worker.go +++ b/pkg/nfd-worker/nfd-worker.go @@ -66,7 +66,8 @@ type NFDConfig struct { } type coreConfig struct { - NoPublish bool + NoPublish bool + SleepInterval duration } type sourcesConfig map[string]source.Config @@ -85,10 +86,10 @@ type Args struct { Oneshot bool Server string ServerNameOverride string - SleepInterval time.Duration Sources []string // Deprecated options that should be set via the config file - NoPublish *bool + NoPublish *bool + SleepInterval *time.Duration } type NfdWorker interface { @@ -105,6 +106,10 @@ type nfdWorker struct { labelWhiteList *regexp.Regexp } +type duration struct { + time.Duration +} + // Create new NfdWorker instance. func NewNfdWorker(args Args) (NfdWorker, error) { nfd := &nfdWorker{ @@ -117,11 +122,6 @@ func NewNfdWorker(args Args) (NfdWorker, error) { nfd.configFilePath = filepath.Clean(args.ConfigFile) } - 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 - } - // Check TLS related args if args.CertFile != "" || args.KeyFile != "" || args.CaFile != "" { if args.CertFile == "" { @@ -228,7 +228,9 @@ func addConfigWatch(path string) (*fsnotify.Watcher, map[string]struct{}, error) func newDefaultConfig() *NFDConfig { return &NFDConfig{ - Core: coreConfig{}, + Core: coreConfig{ + SleepInterval: duration{60 * time.Second}, + }, } } @@ -272,8 +274,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: @@ -378,6 +380,14 @@ 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} + } +} + // Parse configuration options func (w *nfdWorker) configure(filepath string, overrides string) { // Create a new default config @@ -409,6 +419,11 @@ func (w *nfdWorker) configure(filepath string, overrides string) { if w.args.NoPublish != nil { c.Core.NoPublish = *w.args.NoPublish } + if w.args.SleepInterval != nil { + c.Core.SleepInterval = duration{*w.args.SleepInterval} + } + + c.Core.sanitize() w.config = c @@ -527,6 +542,27 @@ func advertiseFeatureLabels(client pb.LabelerClient, labels Labels) error { 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