From c95ad3198cbf9894f134ff217e265dd13578b5d1 Mon Sep 17 00:00:00 2001 From: Markus Lehtonen Date: Tue, 21 Apr 2020 19:40:01 +0300 Subject: [PATCH] nfd-worker: refactor handling of enabled sources and labels Make the list of enabled sources and the label whitelist regexp members of the nfdWorker instance. Get rid of the not-that-well-defined configureParameters() function. --- pkg/nfd-worker/nfd-worker-internal_test.go | 64 ++++++------- pkg/nfd-worker/nfd-worker.go | 105 ++++++++++----------- 2 files changed, 79 insertions(+), 90 deletions(-) diff --git a/pkg/nfd-worker/nfd-worker-internal_test.go b/pkg/nfd-worker/nfd-worker-internal_test.go index 5483dbb26..28977d005 100644 --- a/pkg/nfd-worker/nfd-worker-internal_test.go +++ b/pkg/nfd-worker/nfd-worker-internal_test.go @@ -126,64 +126,60 @@ func TestConfigParse(t *testing.T) { }) } -func TestConfigureParameters(t *testing.T) { - Convey("When configuring parameters for node feature discovery", t, func() { +func TestNewNfdWorker(t *testing.T) { + Convey("When creating new NfdWorker instance", t, func() { - Convey("When no sourcesWhiteList and labelWhiteListStr are passed", func() { - sourcesWhiteList := []string{} - labelWhiteListStr := "" + Convey("without any args specified", func() { + args := Args{} emptyRegexp, _ := regexp.Compile("") - enabledSources, labelWhiteList, err := configureParameters(sourcesWhiteList, labelWhiteListStr) + worker, err := NewNfdWorker(args) - Convey("Error should not be produced", func() { + Convey("no error should be returned", func() { So(err, ShouldBeNil) }) - Convey("No sourcesWhiteList or labelWhiteList are returned", func() { - So(len(enabledSources), ShouldEqual, 0) - So(labelWhiteList, ShouldResemble, emptyRegexp) + 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) }) }) - Convey("When sourcesWhiteList is passed", func() { - sourcesWhiteList := []string{"fake"} - labelWhiteListStr := "" + Convey("with non-empty Sources arg specified", func() { + args := Args{Sources: []string{"fake"}} emptyRegexp, _ := regexp.Compile("") - enabledSources, labelWhiteList, err := configureParameters(sourcesWhiteList, labelWhiteListStr) + worker, err := NewNfdWorker(args) - Convey("Error should not be produced", func() { + Convey("no error should be returned", func() { So(err, ShouldBeNil) }) - Convey("Proper sourcesWhiteList are returned", func() { - So(len(enabledSources), ShouldEqual, 1) - So(enabledSources[0], ShouldHaveSameTypeAs, fake.Source{}) - So(labelWhiteList, ShouldResemble, emptyRegexp) + 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("When invalid labelWhiteListStr is passed", func() { - sourcesWhiteList := []string{""} - labelWhiteListStr := "*" - enabledSources, labelWhiteList, err := configureParameters(sourcesWhiteList, labelWhiteListStr) + Convey("with invalid LabelWhiteList arg specified", func() { + args := Args{LabelWhiteList: "*"} + worker, err := NewNfdWorker(args) - Convey("Error is produced", func() { - So(enabledSources, ShouldBeNil) - So(labelWhiteList, ShouldBeNil) + Convey("an error should be returned", func() { + So(len(worker.sources), ShouldEqual, 0) + So(worker.labelWhiteList, ShouldBeNil) So(err, ShouldNotBeNil) }) }) - Convey("When valid labelWhiteListStr is passed", func() { - sourcesWhiteList := []string{""} - labelWhiteListStr := ".*rdt.*" + Convey("with valid LabelWhiteListStr arg specified", func() { + args := Args{LabelWhiteList: ".*rdt.*"} + worker, err := NewNfdWorker(args) expectRegexp, err := regexp.Compile(".*rdt.*") - enabledSources, labelWhiteList, err := configureParameters(sourcesWhiteList, labelWhiteListStr) - Convey("Error should not be produced", func() { + Convey("no error should be returned", func() { So(err, ShouldBeNil) }) - Convey("Proper labelWhiteList is returned", func() { - So(len(enabledSources), ShouldEqual, 0) - So(labelWhiteList, ShouldResemble, expectRegexp) + Convey("proper labelWhiteList regexp should be produced", func() { + So(len(worker.sources), ShouldEqual, 0) + So(worker.labelWhiteList, ShouldResemble, expectRegexp) }) }) }) diff --git a/pkg/nfd-worker/nfd-worker.go b/pkg/nfd-worker/nfd-worker.go index 2468c12ac..7b97873c2 100644 --- a/pkg/nfd-worker/nfd-worker.go +++ b/pkg/nfd-worker/nfd-worker.go @@ -94,14 +94,20 @@ type NfdWorker interface { } type nfdWorker struct { - args Args - clientConn *grpc.ClientConn - client pb.LabelerClient + args Args + clientConn *grpc.ClientConn + client pb.LabelerClient + sources []source.FeatureSource + labelWhiteList *regexp.Regexp } // Create new NfdWorker instance. func NewNfdWorker(args Args) (NfdWorker, error) { - nfd := &nfdWorker{args: args} + nfd := &nfdWorker{ + args: args, + sources: []source.FeatureSource{}, + } + 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 @@ -120,6 +126,44 @@ func NewNfdWorker(args Args) (NfdWorker, error) { } } + // Figure out active sources + allSources := []source.FeatureSource{ + cpu.Source{}, + fake.Source{}, + iommu.Source{}, + kernel.Source{}, + memory.Source{}, + network.Source{}, + panicfake.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{}, + } + + 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) + } + } + + // 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 } @@ -135,12 +179,6 @@ func (w *nfdWorker) Run() error { stderrLogger.Print(err) } - // Configure the parameters for feature discovery. - enabledSources, labelWhiteList, err := configureParameters(w.args.Sources, w.args.LabelWhiteList) - if err != nil { - return fmt.Errorf("error occurred while configuring parameters: %s", err.Error()) - } - // Connect to NFD master err = w.connect() if err != nil { @@ -150,7 +188,7 @@ func (w *nfdWorker) Run() error { for { // Get the set of feature labels. - labels := createFeatureLabels(enabledSources, labelWhiteList) + labels := createFeatureLabels(w.sources, w.labelWhiteList) // Update the node with the feature labels. if w.client != nil { @@ -263,51 +301,6 @@ func configParse(filepath string, overrides string) error { return nil } -// configureParameters returns all the variables required to perform feature -// discovery based on command line arguments. -func configureParameters(sourcesWhiteList []string, labelWhiteListStr string) (enabledSources []source.FeatureSource, labelWhiteList *regexp.Regexp, err error) { - // A map for lookup - sourcesWhiteListMap := map[string]struct{}{} - for _, s := range sourcesWhiteList { - sourcesWhiteListMap[strings.TrimSpace(s)] = struct{}{} - } - - // Configure feature sources. - allSources := []source.FeatureSource{ - cpu.Source{}, - fake.Source{}, - iommu.Source{}, - kernel.Source{}, - memory.Source{}, - network.Source{}, - panicfake.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{}, - } - - enabledSources = []source.FeatureSource{} - for _, s := range allSources { - if _, enabled := sourcesWhiteListMap[s.Name()]; enabled { - enabledSources = append(enabledSources, s) - } - } - - // Compile labelWhiteList regex - labelWhiteList, err = regexp.Compile(labelWhiteListStr) - if err != nil { - stderrLogger.Printf("error parsing whitelist regex (%s): %s", labelWhiteListStr, err) - return nil, nil, err - } - - return enabledSources, labelWhiteList, 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) {