diff --git a/deployment/components/worker-config/nfd-worker.conf.example b/deployment/components/worker-config/nfd-worker.conf.example index 37466c40de..c07c4147d7 100644 --- a/deployment/components/worker-config/nfd-worker.conf.example +++ b/deployment/components/worker-config/nfd-worker.conf.example @@ -107,3 +107,56 @@ # value: customValue # matchOn: # - nodename: ["worker-0", "my-.*-node"] +# +# # The following feature demonstrates the capabilities of the matchFeatures +# - name: "my.ng.feature" +# labels: +# my-ng-feature: "true" +# # matchFeatures implements a logical AND over all matcher terms in the +# # list (i.e. all of the terms, or per-feature matchers, must match) +# matchFeatures: +# - feature: cpu.cpuid +# matchExpressions: +# AVX512F: {op: Exists} +# - feature: cpu.cstate +# matchExpressions: +# enabled: {op: IsTrue} +# - feature: cpu.pstate +# matchExpressions: +# no_turbo: {op: IsFalse} +# scaling_governor: {op: In, value: ["performance"]} +# - feature: cpu.rdt +# matchExpressions: +# RDTL3CA: {op: Exists} +# - feature: cpu.sst +# matchExpressions: +# bf.enabled: {op: IsTrue} +# - feature: cpu.topology +# matchExpressions: +# hardware_multithreading: {op: IsFalse} +# +# - feature: kernel.config +# matchExpressions: +# X86: {op: Exists} +# LSM: {op: InRegexp, value: ["apparmor"]} +# - feature: kernel.loadedmodule +# matchExpressions: +# e1000e: {op: Exists} +# - feature: kernel.selinux +# matchExpressions: +# enabled: {op: IsFalse} +# - feature: kernel.version +# matchExpressions: +# major: {op: In, value: ["5"]} +# minor: {op: Gt, value: ["10"]} +# +# - feature: system.osrelease +# matchExpressions: +# ID: {op: In, value: ["fedora", "centos"]} +# - feature: system.name +# matchExpressions: +# nodename: {op: InRegexp, value: ["^worker-X"]} +# +# - feature: local.label +# matchExpressions: +# custom-feature-knob: {op: Gt, value: ["100"]} diff --git a/deployment/helm/node-feature-discovery/values.yaml b/deployment/helm/node-feature-discovery/values.yaml index 349afdcb2d..0b7253fc55 100644 --- a/deployment/helm/node-feature-discovery/values.yaml +++ b/deployment/helm/node-feature-discovery/values.yaml @@ -193,6 +193,59 @@ worker: # value: customValue # matchOn: # - nodename: ["worker-0", "my-.*-node"] + # + # # The following feature demonstrates the capabilities of the matchFeatures + # - name: "my.ng.feature" + # labels: + # my-ng-feature: "true" + # # matchFeatures implements a logical AND over all matcher terms in the + # # list (i.e. all of the terms, or per-feature matchers, must match) + # matchFeatures: + # - feature: cpu.cpuid + # matchExpressions: + # AVX512F: {op: Exists} + # - feature: cpu.cstate + # matchExpressions: + # enabled: {op: IsTrue} + # - feature: cpu.pstate + # matchExpressions: + # no_turbo: {op: IsFalse} + # scaling_governor: {op: In, value: ["performance"]} + # - feature: cpu.rdt + # matchExpressions: + # RDTL3CA: {op: Exists} + # - feature: cpu.sst + # matchExpressions: + # bf.enabled: {op: IsTrue} + # - feature: cpu.topology + # matchExpressions: + # hardware_multithreading: {op: IsFalse} + # + # - feature: kernel.config + # matchExpressions: + # X86: {op: Exists} + # LSM: {op: InRegexp, value: ["apparmor"]} + # - feature: kernel.loadedmodule + # matchExpressions: + # e1000e: {op: Exists} + # - feature: kernel.selinux + # matchExpressions: + # enabled: {op: IsFalse} + # - feature: kernel.version + # matchExpressions: + # major: {op: In, value: ["5"]} + # minor: {op: Gt, value: ["10"]} + # + # - feature: system.osrelease + # matchExpressions: + # ID: {op: In, value: ["fedora", "centos"]} + # - feature: system.name + # matchExpressions: + # nodename: {op: InRegexp, value: ["^worker-X"]} + # + # - feature: local.label + # matchExpressions: + # custom-feature-knob: {op: Gt, value: ["100"]} ### podSecurityContext: {} diff --git a/source/custom/custom.go b/source/custom/custom.go index 427f4a5cb7..af8b11832f 100644 --- a/source/custom/custom.go +++ b/source/custom/custom.go @@ -17,19 +17,25 @@ limitations under the License. package custom import ( + "encoding/json" + "fmt" "reflect" + "strings" "k8s.io/klog/v2" + "sigs.k8s.io/yaml" + "sigs.k8s.io/node-feature-discovery/pkg/api/feature" "sigs.k8s.io/node-feature-discovery/pkg/utils" "sigs.k8s.io/node-feature-discovery/source" + "sigs.k8s.io/node-feature-discovery/source/custom/expression" "sigs.k8s.io/node-feature-discovery/source/custom/rules" ) const Name = "custom" -// Custom Features Configurations -type MatchRule struct { +// LegacyMatcher contains the legacy custom rules. +type LegacyMatcher struct { PciID *rules.PciIDRule `json:"pciId,omitempty"` UsbID *rules.UsbIDRule `json:"usbId,omitempty"` LoadedKMod *rules.LoadedKModRule `json:"loadedKMod,omitempty"` @@ -38,13 +44,31 @@ type MatchRule struct { Nodename *rules.NodenameRule `json:"nodename,omitempty"` } -type FeatureSpec struct { - Name string `json:"name"` - Value *string `json:"value,omitempty"` - MatchOn []MatchRule `json:"matchOn"` +type LegacyRule struct { + Name string `json:"name"` + Value *string `json:"value,omitempty"` + MatchOn []LegacyMatcher `json:"matchOn"` } -type config []FeatureSpec +type Rule struct { + Name string `json:"name"` + Labels map[string]string `json:"labels"` + MatchFeatures FeatureMatcher `json:"matchFeatures"` +} + +type FeatureMatcher []FeatureMatcherTerm + +type FeatureMatcherTerm struct { + Feature string + MatchExpressions expression.MatchExpressionSet +} + +type config []CustomRule + +type CustomRule struct { + *LegacyRule + *Rule +} // newDefaultConfig returns a new config with pre-populated defaults func newDefaultConfig() *config { @@ -91,64 +115,183 @@ func (s *customSource) Priority() int { return 10 } // GetLabels method of the LabelSource interface func (s *customSource) GetLabels() (source.FeatureLabels, error) { - features := source.FeatureLabels{} + // Get raw features from all sources + domainFeatures := make(map[string]*feature.DomainFeatures) + for n, s := range source.GetAllFeatureSources() { + domainFeatures[n] = s.GetFeatures() + } + + labels := source.FeatureLabels{} allFeatureConfig := append(getStaticFeatureConfig(), *s.config...) allFeatureConfig = append(allFeatureConfig, getDirectoryFeatureConfig()...) utils.KlogDump(2, "custom features configuration:", " ", allFeatureConfig) // Iterate over features - for _, customFeature := range allFeatureConfig { - featureExist, err := s.discoverFeature(customFeature) + for _, rule := range allFeatureConfig { + ruleOut, err := rule.execute(domainFeatures) if err != nil { - klog.Errorf("failed to discover feature: %q: %s", customFeature.Name, err.Error()) + klog.Error(err) continue } - if featureExist { - var value interface{} = true - if customFeature.Value != nil { - value = *customFeature.Value - } - features[customFeature.Name] = value + + for n, v := range ruleOut { + labels[n] = v } } - return features, nil + return labels, nil } // Process a single feature by Matching on the defined rules. -// A feature is present if all defined Rules in a MatchRule return a match. -func (s *customSource) discoverFeature(feature FeatureSpec) (bool, error) { - for _, matchRules := range feature.MatchOn { - - allRules := []legacyRule{ - matchRules.PciID, - matchRules.UsbID, - matchRules.LoadedKMod, - matchRules.CpuID, - matchRules.Kconfig, - matchRules.Nodename, +func (r *CustomRule) execute(features map[string]*feature.DomainFeatures) (map[string]string, error) { + if r.LegacyRule != nil { + ruleOut, err := r.LegacyRule.execute(features) + if err != nil { + return nil, fmt.Errorf("failed to execute legacy rule %s: %w", r.LegacyRule.Name, err) } + return ruleOut, err + } - // return true, nil if all rules match - matchRules := func(rules []legacyRule) (bool, error) { - for _, rule := range rules { - if reflect.ValueOf(rule).IsNil() { - continue - } - if match, err := rule.Match(); err != nil { - return false, err - } else if !match { - return false, nil - } + if r.Rule != nil { + ruleOut, err := r.Rule.execute(features) + if err != nil { + return nil, fmt.Errorf("failed to execute rule %s: %w", r.Rule.Name, err) + } + return ruleOut, err + } + + return nil, fmt.Errorf("BUG: an empty rule, this really should not happen") +} + +// Process a single feature by Matching on the defined rules. +func (r *LegacyRule) execute(features map[string]*feature.DomainFeatures) (map[string]string, error) { + if len(r.MatchOn) > 0 { + // Logical OR over the legacy rules + matched := false + for _, matcher := range r.MatchOn { + if m, err := matcher.match(); err != nil { + return nil, err + } else if m { + matched = true + break } - return true, nil + } + if !matched { + return nil, nil + } + } + + value := "true" + if r.Value != nil { + value = *r.Value + } + return map[string]string{r.Name: value}, nil +} + +func (r *Rule) execute(features map[string]*feature.DomainFeatures) (map[string]string, error) { + if len(r.MatchFeatures) > 0 { + if m, err := r.MatchFeatures.match(features); err != nil { + return nil, err + } else if !m { + return nil, nil + } + } + + labels := make(map[string]string, len(r.Labels)) + for k, v := range r.Labels { + labels[k] = v + } + + return labels, nil +} + +func (m *FeatureMatcher) match(features map[string]*feature.DomainFeatures) (bool, error) { + // Logical AND over the terms + for _, term := range *m { + split := strings.SplitN(term.Feature, ".", 2) + if len(split) != 2 { + return false, fmt.Errorf("invalid selector %q: must be .", term.Feature) + } + domain := split[0] + // Ignore case + featureName := strings.ToLower(split[1]) + + domainFeatures, ok := features[domain] + if !ok { + return false, fmt.Errorf("unknown feature source/domain %q", domain) } - if match, err := matchRules(allRules); err != nil { + var m bool + var err error + if f, ok := domainFeatures.Keys[featureName]; ok { + m, err = term.MatchExpressions.MatchKeys(f.Elements) + } else if f, ok := domainFeatures.Values[featureName]; ok { + m, err = term.MatchExpressions.MatchValues(f.Elements) + } else if f, ok := domainFeatures.Instances[featureName]; ok { + m, err = term.MatchExpressions.MatchInstances(f.Elements) + } else { + return false, fmt.Errorf("%q feature of source/domain %q not available", featureName, domain) + } + + if err != nil { return false, err - } else if match { - return true, nil + } else if !m { + return false, nil } } - return false, nil + return true, nil +} + +func (m *LegacyMatcher) match() (bool, error) { + allRules := []legacyRule{ + m.PciID, + m.UsbID, + m.LoadedKMod, + m.CpuID, + m.Kconfig, + m.Nodename, + } + + // return true, nil if all rules match + matchRules := func(rules []legacyRule) (bool, error) { + for _, rule := range rules { + if reflect.ValueOf(rule).IsNil() { + continue + } + if match, err := rule.Match(); err != nil { + return false, err + } else if !match { + return false, nil + } + } + return true, nil + } + + return matchRules(allRules) +} + +// UnmarshalJSON implements the Unmarshaler interface from "encoding/json" +func (c *CustomRule) UnmarshalJSON(data []byte) error { + // Do a raw parse to determine if this is a legacy rule + raw := map[string]json.RawMessage{} + err := yaml.Unmarshal(data, &raw) + if err != nil { + return err + } + + for k := range raw { + if strings.ToLower(k) == "matchon" { + return yaml.Unmarshal(data, &c.LegacyRule) + } + } + + return yaml.Unmarshal(data, &c.Rule) +} + +// MarshalJSON implements the Marshaler interface from "encoding/json" +func (c *CustomRule) MarshalJSON() ([]byte, error) { + if c.LegacyRule != nil { + return json.Marshal(c.LegacyRule) + } + return json.Marshal(c.Rule) } func init() { diff --git a/source/custom/custom_test.go b/source/custom/custom_test.go new file mode 100644 index 0000000000..13881d8479 --- /dev/null +++ b/source/custom/custom_test.go @@ -0,0 +1,153 @@ +/* +Copyright 2021 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package custom + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "sigs.k8s.io/node-feature-discovery/pkg/api/feature" + "sigs.k8s.io/node-feature-discovery/source/custom/expression" +) + +func TestRule(t *testing.T) { + f := map[string]*feature.DomainFeatures{} + r1 := Rule{Labels: map[string]string{"label-1": "", "label-2": "true"}} + r2 := Rule{ + Labels: map[string]string{"label-1": "label-val-1"}, + MatchFeatures: FeatureMatcher{ + FeatureMatcherTerm{ + Feature: "domain-1.kf-1", + MatchExpressions: expression.MatchExpressionSet{"key-1": expression.MustCreateMatchExpression(expression.MatchExists)}, + }, + }, + } + + // Test totally empty features + m, err := r1.execute(f) + assert.Nilf(t, err, "unexpected error: %v", err) + assert.Equal(t, r1.Labels, m, "empty matcher should have matched empty features") + + _, err = r2.execute(f) + assert.Error(t, err, "matching agains a missing domain should have returned an error") + + // Test empty domain + d := feature.NewDomainFeatures() + f["domain-1"] = d + + m, err = r1.execute(f) + assert.Nilf(t, err, "unexpected error: %v", err) + assert.Equal(t, r1.Labels, m, "empty matcher should have matched empty features") + + _, err = r2.execute(f) + assert.Error(t, err, "matching agains a missing feature type should have returned an error") + + // Test empty feature sets + d.Keys["kf-1"] = feature.NewKeyFeatures() + d.Values["vf-1"] = feature.NewValueFeatures(nil) + d.Instances["if-1"] = feature.NewInstanceFeatures(nil) + + m, err = r1.execute(f) + assert.Nilf(t, err, "unexpected error: %v", err) + assert.Equal(t, r1.Labels, m, "empty matcher should have matched empty features") + + m, err = r2.execute(f) + assert.Nilf(t, err, "unexpected error: %v", err) + assert.Nil(t, m, "unexpected match") + + // Test non-empty feature sets + d.Keys["kf-1"].Elements["key-x"] = feature.Nil{} + d.Values["vf-1"].Elements["key-1"] = "val-x" + d.Instances["if-1"] = feature.NewInstanceFeatures([]feature.InstanceFeature{ + *feature.NewInstanceFeature(map[string]string{"attr-1": "val-x"})}) + + m, err = r1.execute(f) + assert.Nilf(t, err, "unexpected error: %v", err) + assert.Equal(t, r1.Labels, m, "empty matcher should have matched empty features") + + // Match "key" features + m, err = r2.execute(f) + assert.Nilf(t, err, "unexpected error: %v", err) + assert.Nil(t, m, "keys should not have matched") + + d.Keys["kf-1"].Elements["key-1"] = feature.Nil{} + m, err = r2.execute(f) + assert.Nilf(t, err, "unexpected error: %v", err) + assert.Equal(t, r2.Labels, m, "keys should have matched") + + // Match "value" features + r3 := Rule{ + Labels: map[string]string{"label-3": "label-val-3", "empty": ""}, + MatchFeatures: FeatureMatcher{ + FeatureMatcherTerm{ + Feature: "domain-1.vf-1", + MatchExpressions: expression.MatchExpressionSet{"key-1": expression.MustCreateMatchExpression(expression.MatchIn, "val-1")}, + }, + }, + } + m, err = r3.execute(f) + assert.Nilf(t, err, "unexpected error: %v", err) + assert.Nil(t, m, "values should not have matched") + + d.Values["vf-1"].Elements["key-1"] = "val-1" + m, err = r3.execute(f) + assert.Nilf(t, err, "unexpected error: %v", err) + assert.Equal(t, r3.Labels, m, "values should have matched") + + // Match "instance" features + r4 := Rule{ + Labels: map[string]string{"label-4": "label-val-4"}, + MatchFeatures: FeatureMatcher{ + FeatureMatcherTerm{ + Feature: "domain-1.if-1", + MatchExpressions: expression.MatchExpressionSet{"attr-1": expression.MustCreateMatchExpression(expression.MatchIn, "val-1")}, + }, + }, + } + m, err = r4.execute(f) + assert.Nilf(t, err, "unexpected error: %v", err) + assert.Nil(t, m, "instances should not have matched") + + d.Instances["if-1"].Elements[0].Attributes["attr-1"] = "val-1" + m, err = r4.execute(f) + assert.Nilf(t, err, "unexpected error: %v", err) + assert.Equal(t, r4.Labels, m, "instances should have matched") + + // Test multiple feature matchers + r5 := Rule{ + Labels: map[string]string{"label-5": "label-val-5"}, + MatchFeatures: FeatureMatcher{ + FeatureMatcherTerm{ + Feature: "domain-1.vf-1", + MatchExpressions: expression.MatchExpressionSet{"key-1": expression.MustCreateMatchExpression(expression.MatchIn, "val-x")}, + }, + FeatureMatcherTerm{ + Feature: "domain-1.if-1", + MatchExpressions: expression.MatchExpressionSet{"attr-1": expression.MustCreateMatchExpression(expression.MatchIn, "val-1")}, + }, + }, + } + m, err = r5.execute(f) + assert.Nilf(t, err, "unexpected error: %v", err) + assert.Nil(t, m, "instances should not have matched") + + r5.MatchFeatures[0].MatchExpressions["key-1"] = expression.MustCreateMatchExpression(expression.MatchIn, "val-1") + m, err = r5.execute(f) + assert.Nilf(t, err, "unexpected error: %v", err) + assert.Equal(t, r5.Labels, m, "instances should have matched") + +} diff --git a/source/custom/directory_features.go b/source/custom/directory_features.go index d2a725ac6b..2df415e3a8 100644 --- a/source/custom/directory_features.go +++ b/source/custom/directory_features.go @@ -31,14 +31,14 @@ const Directory = "/etc/kubernetes/node-feature-discovery/custom.d" // getDirectoryFeatureConfig returns features configured in the "/etc/kubernetes/node-feature-discovery/custom.d" // host directory and its 1st level subdirectories, which can be populated e.g. by ConfigMaps -func getDirectoryFeatureConfig() []FeatureSpec { +func getDirectoryFeatureConfig() []CustomRule { features := readDir(Directory, true) klog.V(1).Infof("all configmap based custom feature specs: %+v", features) return features } -func readDir(dirName string, recursive bool) []FeatureSpec { - features := make([]FeatureSpec, 0) +func readDir(dirName string, recursive bool) []CustomRule { + features := make([]CustomRule, 0) klog.V(1).Infof("getting files in %s", dirName) files, err := ioutil.ReadDir(dirName) @@ -76,7 +76,7 @@ func readDir(dirName string, recursive bool) []FeatureSpec { } klog.V(2).Infof("custom config rules raw: %s", string(bytes)) - config := &[]FeatureSpec{} + config := &[]CustomRule{} err = yaml.UnmarshalStrict(bytes, config) if err != nil { klog.Errorf("could not parse custom config file %q, %v", fileName, err) diff --git a/source/custom/static_features.go b/source/custom/static_features.go index 3d632e4791..d0c4cd96e6 100644 --- a/source/custom/static_features.go +++ b/source/custom/static_features.go @@ -23,28 +23,32 @@ import ( // getStaticFeatures returns statically configured custom features to discover // e.g RMDA related features. NFD configuration file may extend these custom features by adding rules. -func getStaticFeatureConfig() []FeatureSpec { - return []FeatureSpec{ +func getStaticFeatureConfig() []CustomRule { + return []CustomRule{ { - Name: "rdma.capable", - MatchOn: []MatchRule{ - { - PciID: &rules.PciIDRule{ - MatchExpressionSet: expression.MatchExpressionSet{ - "vendor": expression.MustCreateMatchExpression(expression.MatchIn, "15b3"), + LegacyRule: &LegacyRule{ + Name: "rdma.capable", + MatchOn: []LegacyMatcher{ + { + PciID: &rules.PciIDRule{ + MatchExpressionSet: expression.MatchExpressionSet{ + "vendor": expression.MustCreateMatchExpression(expression.MatchIn, "15b3"), + }, }, }, }, }, }, { - Name: "rdma.available", - MatchOn: []MatchRule{ - { - LoadedKMod: &rules.LoadedKModRule{ - MatchExpressionSet: expression.MatchExpressionSet{ - "ib_uverbs": expression.MustCreateMatchExpression(expression.MatchExists), - "rdma_ucm": expression.MustCreateMatchExpression(expression.MatchExists), + LegacyRule: &LegacyRule{ + Name: "rdma.available", + MatchOn: []LegacyMatcher{ + { + LoadedKMod: &rules.LoadedKModRule{ + MatchExpressionSet: expression.MatchExpressionSet{ + "ib_uverbs": expression.MustCreateMatchExpression(expression.MatchExists), + "rdma_ucm": expression.MustCreateMatchExpression(expression.MatchExists), + }, }, }, },