From 094501916155c183775e4e941d55a4d9ca5f38a0 Mon Sep 17 00:00:00 2001 From: Markus Lehtonen Date: Tue, 2 Mar 2021 10:33:45 +0200 Subject: [PATCH] source/kernel: implement FeatureSource Separate feature discovery and creation of feature labels in the kernel source. Move kernelutils from source/internal back to the source/kernel package. Change the kconfig custom rule to rely on the FeatureSource interface (GetFeatures()) of the kernel source. Also, add minimalist unit test. --- source/custom/rules/kconfig_rule.go | 24 ++-- .../kernel_kconfig.go => kernel/kconfig.go} | 8 +- source/kernel/kernel.go | 105 +++++++++--------- .../kernel_test.go} | 26 +++-- source/kernel/version.go | 61 ++++++++++ source/source_test.go | 1 + 6 files changed, 144 insertions(+), 81 deletions(-) rename source/{internal/kernelutils/kernel_kconfig.go => kernel/kconfig.go} (96%) rename source/{internal/kernelutils/kernel_version.go => kernel/kernel_test.go} (57%) create mode 100644 source/kernel/version.go diff --git a/source/custom/rules/kconfig_rule.go b/source/custom/rules/kconfig_rule.go index 9ef65c480..d7cd3d9ce 100644 --- a/source/custom/rules/kconfig_rule.go +++ b/source/custom/rules/kconfig_rule.go @@ -18,9 +18,11 @@ package rules import ( "encoding/json" + "fmt" "strings" - "sigs.k8s.io/node-feature-discovery/source/internal/kernelutils" + "sigs.k8s.io/node-feature-discovery/source" + "sigs.k8s.io/node-feature-discovery/source/kernel" ) // KconfigRule implements Rule @@ -31,11 +33,14 @@ type kconfig struct { Value string } -var kConfigs map[string]string - func (kconfigs *KconfigRule) Match() (bool, error) { + options, ok := source.GetFeatureSource("kernel").GetFeatures().Values[kernel.ConfigFeature] + if !ok { + return false, fmt.Errorf("kernel config options not available") + } + for _, f := range *kconfigs { - if v, ok := kConfigs[f.Name]; !ok || f.Value != v { + if v, ok := options.Elements[f.Name]; !ok || f.Value != v { return false, nil } } @@ -57,14 +62,3 @@ func (c *kconfig) UnmarshalJSON(data []byte) error { } return nil } - -func init() { - kConfigs = make(map[string]string) - - kconfig, err := kernelutils.ParseKconfig("") - if err == nil { - for k, v := range kconfig { - kConfigs[k] = v - } - } -} diff --git a/source/internal/kernelutils/kernel_kconfig.go b/source/kernel/kconfig.go similarity index 96% rename from source/internal/kernelutils/kernel_kconfig.go rename to source/kernel/kconfig.go index 98451abf7..b77d304b5 100644 --- a/source/internal/kernelutils/kernel_kconfig.go +++ b/source/kernel/kconfig.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package kernelutils +package kernel import ( "bytes" @@ -26,9 +26,9 @@ import ( "regexp" "strings" + "k8s.io/apimachinery/pkg/util/validation" "k8s.io/klog/v2" - "k8s.io/apimachinery/pkg/util/validation" "sigs.k8s.io/node-feature-discovery/source" ) @@ -52,13 +52,13 @@ func readKconfigGzip(filename string) ([]byte, error) { } // ParseKconfig reads kconfig and return a map -func ParseKconfig(configPath string) (map[string]string, error) { +func parseKconfig(configPath string) (map[string]string, error) { kconfig := map[string]string{} raw := []byte(nil) var err error var searchPaths []string - kVer, err := GetKernelVersion() + kVer, err := getVersion() if err != nil { searchPaths = []string{ "/proc/config.gz", diff --git a/source/kernel/kernel.go b/source/kernel/kernel.go index 5e0de7a4a..24a8824a1 100644 --- a/source/kernel/kernel.go +++ b/source/kernel/kernel.go @@ -17,17 +17,23 @@ limitations under the License. package kernel import ( - "regexp" - "strings" + "strconv" "k8s.io/klog/v2" + "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/internal/kernelutils" ) const Name = "kernel" +const ( + ConfigFeature = "config" + SelinuxFeature = "selinux" + VersionFeature = "version" +) + // Configuration file options type Config struct { KconfigFile string @@ -47,14 +53,16 @@ func newDefaultConfig() *Config { } } -// kernelSource implements the LabelSource and ConfigurableSource interfaces. +// kernelSource implements the FeatureSource, LabelSource and ConfigurableSource interfaces. type kernelSource struct { - config *Config + config *Config + features *feature.DomainFeatures } // Singleton source instance var ( - src kernelSource + src = kernelSource{config: newDefaultConfig()} + _ source.FeatureSource = &src _ source.LabelSource = &src _ source.ConfigurableSource = &src ) @@ -82,69 +90,62 @@ func (s *kernelSource) Priority() int { return 0 } // GetLabels method of the LabelSource interface func (s *kernelSource) GetLabels() (source.FeatureLabels, error) { - features := source.FeatureLabels{} + labels := source.FeatureLabels{} + features := s.GetFeatures() - // Read kernel version - version, err := parseVersion() - if err != nil { - klog.Errorf("failed to get kernel version: %s", err) - } else { - for key := range version { - features["version."+key] = version[key] - } - } - - // Read kconfig - kconfig, err := kernelutils.ParseKconfig(s.config.KconfigFile) - if err != nil { - klog.Errorf("failed to read kconfig: %s", err) + for k, v := range features.Values[VersionFeature].Elements { + labels[VersionFeature+"."+k] = v } // Check flags for _, opt := range s.config.ConfigOpts { - if val, ok := kconfig[opt]; ok { - features["config."+opt] = val + if val, ok := features.Values[ConfigFeature].Elements[opt]; ok { + labels[ConfigFeature+"."+opt] = val } } - selinux, err := SelinuxEnabled() - if err != nil { - klog.Warning(err) - } else if selinux { - features["selinux.enabled"] = true + for k, v := range features.Values[SelinuxFeature].Elements { + labels[SelinuxFeature+"."+k] = v } - return features, nil + return labels, nil } -// Read and parse kernel version -func parseVersion() (map[string]string, error) { - version := map[string]string{} +// Discover method of the FeatureSource interface +func (s *kernelSource) Discover() error { + s.features = feature.NewDomainFeatures() - full, err := kernelutils.GetKernelVersion() - if err != nil { - return nil, err + // Read kernel version + if version, err := parseVersion(); err != nil { + klog.Errorf("failed to get kernel version: %s", err) + } else { + s.features.Values[VersionFeature] = feature.NewValueFeatures(version) } - // Replace forbidden symbols - fullRegex := regexp.MustCompile("[^-A-Za-z0-9_.]") - full = fullRegex.ReplaceAllString(full, "_") - // Label values must start and end with an alphanumeric - full = strings.Trim(full, "-_.") - - version["full"] = full - - // Regexp for parsing version components - re := regexp.MustCompile(`^(?P\d+)(\.(?P\d+))?(\.(?P\d+))?(-.*)?$`) - if m := re.FindStringSubmatch(full); m != nil { - for i, name := range re.SubexpNames() { - if i != 0 && name != "" { - version[name] = m[i] - } - } + // Read kconfig + if kconfig, err := parseKconfig(s.config.KconfigFile); err != nil { + klog.Errorf("failed to read kconfig: %s", err) + } else { + s.features.Values[ConfigFeature] = feature.NewValueFeatures(kconfig) } - return version, nil + if selinux, err := SelinuxEnabled(); err != nil { + klog.Warning(err) + } else { + s.features.Values[SelinuxFeature] = feature.NewValueFeatures(nil) + s.features.Values[SelinuxFeature].Elements["enabled"] = strconv.FormatBool(selinux) + } + + utils.KlogDump(3, "discovered kernel features:", " ", s.features) + + return nil +} + +func (s *kernelSource) GetFeatures() *feature.DomainFeatures { + if s.features == nil { + s.features = feature.NewDomainFeatures() + } + return s.features } func init() { diff --git a/source/internal/kernelutils/kernel_version.go b/source/kernel/kernel_test.go similarity index 57% rename from source/internal/kernelutils/kernel_version.go rename to source/kernel/kernel_test.go index 2da110e0d..eef3064d6 100644 --- a/source/internal/kernelutils/kernel_version.go +++ b/source/kernel/kernel_test.go @@ -1,5 +1,5 @@ /* -Copyright 2018-2020 The Kubernetes Authors. +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. @@ -14,17 +14,23 @@ See the License for the specific language governing permissions and limitations under the License. */ -package kernelutils +package kernel import ( - "io/ioutil" - "strings" + "testing" + + "github.com/stretchr/testify/assert" + "sigs.k8s.io/node-feature-discovery/pkg/api/feature" ) -func GetKernelVersion() (string, error) { - unameRaw, err := ioutil.ReadFile("/proc/sys/kernel/osrelease") - if err != nil { - return "", err - } - return strings.TrimSpace(string(unameRaw)), nil +func TestKernelSource(t *testing.T) { + assert.Equal(t, src.Name(), Name) + + // Check that GetLabels works with empty features + src.features = feature.NewDomainFeatures() + l, err := src.GetLabels() + + assert.Nil(t, err, err) + assert.Empty(t, l) + } diff --git a/source/kernel/version.go b/source/kernel/version.go new file mode 100644 index 000000000..4cac1eaf8 --- /dev/null +++ b/source/kernel/version.go @@ -0,0 +1,61 @@ +/* +Copyright 2018-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 kernel + +import ( + "io/ioutil" + "regexp" + "strings" +) + +// Read and parse kernel version +func parseVersion() (map[string]string, error) { + version := map[string]string{} + + full, err := getVersion() + if err != nil { + return nil, err + } + + // Replace forbidden symbols + fullRegex := regexp.MustCompile("[^-A-Za-z0-9_.]") + full = fullRegex.ReplaceAllString(full, "_") + // Label values must start and end with an alphanumeric + full = strings.Trim(full, "-_.") + + version["full"] = full + + // Regexp for parsing version components + re := regexp.MustCompile(`^(?P\d+)(\.(?P\d+))?(\.(?P\d+))?(-.*)?$`) + if m := re.FindStringSubmatch(full); m != nil { + for i, name := range re.SubexpNames() { + if i != 0 && name != "" { + version[name] = m[i] + } + } + } + + return version, nil +} + +func getVersion() (string, error) { + unameRaw, err := ioutil.ReadFile("/proc/sys/kernel/osrelease") + if err != nil { + return "", err + } + return strings.TrimSpace(string(unameRaw)), nil +} diff --git a/source/source_test.go b/source/source_test.go index fb1da297e..1d4c11006 100644 --- a/source/source_test.go +++ b/source/source_test.go @@ -65,6 +65,7 @@ func TestConfigurableSources(t *testing.T) { func TestFeatureSources(t *testing.T) { sources := source.GetAllFeatureSources() + assert.NotZero(t, len(sources)) for n, s := range sources { msg := fmt.Sprintf("testing FeatureSource %q failed", n)