source/cpu: implement FeatureSource

Convert the cpu source to do feature discovery and creation of feature
labels separately.

Move cpuidutils from source/internal to the source/cpu package. Change
the cpuid custom rule to utilize GetFeatures of the cpu source.

Also, add minimalist unit test.

source/cpu/cpu.go

@@ -18,15 +18,26 @@ package cpu
 
 import (
 	"io/ioutil"
+	"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/cpuidutils"
 )
 
 const Name = "cpu"
 
+const (
+	CpuidFeature    = "cpuid"
+	CstateFeature   = "cstate"
+	PstateFeature   = "pstate"
+	RdtFeature      = "rdt"
+	SstFeature      = "sst"
+	TopologyFeature = "topology"
+)
+
 // Configuration file options
 type cpuidConfig struct {
 	AttributeBlacklist []string `json:"attributeBlacklist,omitempty"`
@@ -78,15 +89,17 @@ type keyFilter struct {
 	whitelist bool
 }
 
-// cpuSource implements the LabelSource and ConfigurableSource interfaces.
+// cpuSource implements the FeatureSource, LabelSource and ConfigurableSource interfaces.
 type cpuSource struct {
 	config      *Config
 	cpuidFilter *keyFilter
+	features    *feature.DomainFeatures
 }
 
 // Singleton source instance
 var (
-	src cpuSource
+	src                           = cpuSource{config: newDefaultConfig(), cpuidFilter: &keyFilter{}}
+	_   source.FeatureSource      = &src
 	_   source.LabelSource        = &src
 	_   source.ConfigurableSource = &src
 )
@@ -115,57 +128,98 @@ func (s *cpuSource) Priority() int { return 0 }
 
 // GetLabels method of the LabelSource interface
 func (s *cpuSource) GetLabels() (source.FeatureLabels, error) {
-	features := source.FeatureLabels{}
+	labels := source.FeatureLabels{}
+	features := s.GetFeatures()
 
-	// Check if hyper-threading seems to be enabled
-	found, err := haveThreadSiblings()
-	if err != nil {
-		klog.Errorf("failed to detect hyper-threading: %v", err)
-	} else if found {
-		features["hardware_multithreading"] = true
+	// CPUID
+	for f := range features.Keys[CpuidFeature].Elements {
+		if s.cpuidFilter.unmask(f) {
+			labels["cpuid."+f] = true
+		}
 	}
 
-	// Check SST-BF
-	found, err = discoverSSTBF()
-	if err != nil {
-		klog.Errorf("failed to detect SST-BF: %v", err)
-	} else if found {
-		features["power.sst_bf.enabled"] = true
+	// Cstate
+	for k, v := range features.Values[CstateFeature].Elements {
+		labels["cstate."+k] = v
 	}
 
+	// Pstate
+	for k, v := range features.Values[PstateFeature].Elements {
+		labels["pstate."+k] = v
+	}
+
+	// RDT
+	for k := range features.Keys[RdtFeature].Elements {
+		labels["rdt."+k] = true
+	}
+
+	// SST
+	for k, v := range features.Values[SstFeature].Elements {
+		labels["power.sst_"+k] = v
+	}
+
+	// Hyperthreading
+	if v, ok := features.Values[TopologyFeature].Elements["hardware_multithreading"]; ok {
+		labels["hardware_multithreading"] = v
+	}
+
+	return labels, nil
+}
+
+// Discover method of the FeatureSource Interface
+func (s *cpuSource) Discover() error {
+	s.features = feature.NewDomainFeatures()
+
 	// Detect CPUID
-	cpuidFlags := cpuidutils.GetCpuidFlags()
-	for _, f := range cpuidFlags {
-		if s.cpuidFilter.unmask(f) {
-			features["cpuid."+f] = true
-		}
+	s.features.Keys[CpuidFeature] = feature.NewKeyFeatures(getCpuidFlags()...)
+
+	// Detect cstate configuration
+	cstate, err := detectCstate()
+	if err != nil {
+		klog.Errorf("failed to detect cstate: %v", err)
+	} else {
+		s.features.Values[CstateFeature] = feature.NewValueFeatures(cstate)
 	}
 
 	// Detect pstate features
 	pstate, err := detectPstate()
 	if err != nil {
 		klog.Error(err)
-	} else {
-		for k, v := range pstate {
-			features["pstate."+k] = v
-		}
 	}
+	s.features.Values[PstateFeature] = feature.NewValueFeatures(pstate)
 
 	// Detect RDT features
-	rdt := discoverRDT()
-	for _, f := range rdt {
-		features["rdt."+f] = true
+	s.features.Keys[RdtFeature] = feature.NewKeyFeatures(discoverRDT()...)
+
+	// Detect SST features
+	s.features.Values[SstFeature] = feature.NewValueFeatures(discoverSST())
+
+	// Detect hyper-threading
+	s.features.Values[TopologyFeature] = feature.NewValueFeatures(discoverTopology())
+
+	utils.KlogDump(3, "discovered cpu features:", "  ", s.features)
+
+	return nil
+}
+
+// GetFeatures method of the FeatureSource Interface
+func (s *cpuSource) GetFeatures() *feature.DomainFeatures {
+	if s.features == nil {
+		s.features = feature.NewDomainFeatures()
+	}
+	return s.features
+}
+
+func discoverTopology() map[string]string {
+	features := make(map[string]string)
+
+	if ht, err := haveThreadSiblings(); err != nil {
+		klog.Errorf("failed to detect hyper-threading: %v", err)
+	} else {
+		features["hardware_multithreading"] = strconv.FormatBool(ht)
 	}
 
-	// Detect cstate configuration
-	cstate, ok, err := detectCstate()
-	if err != nil {
-		klog.Error(err)
-	} else if ok {
-		features["cstate.enabled"] = cstate
-	}
-
-	return features, nil
+	return features
 }
 
 // Check if any (online) CPUs have thread siblings

source/cpu/cpu_test.go

@@ -0,0 +1,36 @@
+/*
+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 cpu
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"sigs.k8s.io/node-feature-discovery/pkg/api/feature"
+)
+
+func TestCpuSource(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)
+
+}

source/cpu/cpuid_amd64.go

@@ -14,13 +14,13 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package cpuidutils
+package cpu
 
 import (
 	"github.com/klauspost/cpuid/v2"
 )
 
-// GetCpuidFlags returns feature names for all the supported CPU features.
-func GetCpuidFlags() []string {
+// getCpuidFlags returns feature names for all the supported CPU features.
+func getCpuidFlags() []string {
 	return cpuid.CPU.FeatureSet()
 }

source/cpu/cpuid_arm.go

@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package cpuidutils
+package cpu
 
 /*
 #include <sys/auxv.h>
@@ -90,7 +90,7 @@ var flagNames_arm = map[uint64]string{
 	CPU_ARM_FEATURE_CRC32:    "CRC32",
 }
 
-func GetCpuidFlags() []string {
+func getCpuidFlags() []string {
 	r := make([]string, 0, 20)
 	hwcap := uint64(C.gethwcap())
 	for i := uint(0); i < 64; i++ {

source/cpu/cpuid_arm64.go

@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package cpuidutils
+package cpu
 
 /*
 #include <sys/auxv.h>
@@ -80,7 +80,7 @@ var flagNames_arm64 = map[uint64]string{
 	CPU_ARM64_FEATURE_SVE:      "SVE",
 }
 
-func GetCpuidFlags() []string {
+func getCpuidFlags() []string {
 	r := make([]string, 0, 20)
 	hwcap := uint64(C.gethwcap())
 	for i := uint(0); i < 64; i++ {

source/cpu/cpuid_ppc64le.go

@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package cpuidutils
+package cpu
 
 /*
 #include <sys/auxv.h>
@@ -126,7 +126,7 @@ var flag2Names_ppc64le = map[uint64]string{
 	PPC_FEATURE2_HTM_NO_SUSPEND: "HTM-NO-SUSPEND",
 }
 
-func GetCpuidFlags() []string {
+func getCpuidFlags() []string {
 	r := make([]string, 0, 30)
 	hwcap := uint64(C.gethwcap())
 	hwcap2 := uint64(C.gethwcap2())

source/cpu/cpuid_s390x.go

@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package cpuidutils
+package cpu
 
 /*
 #include <sys/auxv.h>
@@ -71,7 +71,7 @@ var flagNames_s390x = map[uint64]string{
 	HWCAP_S390_DFLT:      "DFLT",
 }
 
-func GetCpuidFlags() []string {
+func getCpuidFlags() []string {
 	r := make([]string, 0, 20)
 	hwcap := uint64(C.gethwcap())
 	for i := uint(0); i < 64; i++ {

source/cpu/cstate_amd64.go

@@ -30,38 +30,42 @@ import (
 )
 
 // Discover if c-states are enabled
-func detectCstate() (bool, bool, error) {
+func detectCstate() (map[string]string, error) {
+	cstate := make(map[string]string)
+
 	// Check that sysfs is available
 	sysfsBase := source.SysfsDir.Path("devices/system/cpu")
 	if _, err := os.Stat(sysfsBase); err != nil {
-		return false, false, fmt.Errorf("unable to detect cstate status: %w", err)
+		return cstate, fmt.Errorf("unable to detect cstate status: %w", err)
 	}
 	cpuidleDir := filepath.Join(sysfsBase, "cpuidle")
 	if _, err := os.Stat(cpuidleDir); os.IsNotExist(err) {
 		klog.V(1).Info("cpuidle disabled in the kernel")
-		return false, false, nil
+		return cstate, nil
 	}
 
 	// When the intel_idle driver is in use (default), check setting of max_cstates
 	driver, err := ioutil.ReadFile(filepath.Join(cpuidleDir, "current_driver"))
 	if err != nil {
-		return false, false, fmt.Errorf("cannot get driver for cpuidle: %w", err)
+		return cstate, fmt.Errorf("cannot get driver for cpuidle: %w", err)
 	}
 
 	if d := strings.TrimSpace(string(driver)); d != "intel_idle" {
 		// Currently only checking intel_idle driver for cstates
 		klog.V(1).Infof("intel_idle driver is not in use (%s is active)", d)
-		return false, false, nil
+		return cstate, nil
 	}
 
 	data, err := ioutil.ReadFile(source.SysfsDir.Path("module/intel_idle/parameters/max_cstate"))
 	if err != nil {
-		return false, false, fmt.Errorf("cannot determine cstate from max_cstates: %w", err)
+		return cstate, fmt.Errorf("cannot determine cstate from max_cstates: %w", err)
 	}
 	cstates, err := strconv.Atoi(strings.TrimSpace(string(data)))
 	if err != nil {
-		return false, false, fmt.Errorf("non-integer value of cstates: %w", err)
+		return cstate, fmt.Errorf("non-integer value of cstates: %w", err)
+	} else {
+		cstate["enabled"] = strconv.FormatBool(cstates > 0)
 	}
 
-	return cstates > 0, true, nil
+	return cstate, nil
 }

source/cpu/cstate_stub.go

@@ -20,6 +20,6 @@ limitations under the License.
 package cpu
 
 // Discover if c-states are enabled
-func detectCstate() (bool, bool, error) {
-	return false, false, nil
+func detectCstate() (map[string]string, error) {
+	return nil, nil
 }

source/cpu/power_amd64.go

@@ -23,6 +23,8 @@ import (
 	"strconv"
 	"strings"
 
+	"k8s.io/klog/v2"
+
 	"sigs.k8s.io/node-feature-discovery/pkg/cpuid"
 	"sigs.k8s.io/node-feature-discovery/source"
 )
@@ -32,6 +34,18 @@ const (
 	LEAF_PROCESSOR_FREQUENCY_INFORMATION = 0x16
 )
 
+func discoverSST() map[string]string {
+	features := make(map[string]string)
+
+	if bf, err := discoverSSTBF(); err != nil {
+		klog.Errorf("failed to detect SST-BF: %v", err)
+	} else if bf {
+		features["bf.enabled"] = strconv.FormatBool(bf)
+	}
+
+	return features
+}
+
 func discoverSSTBF() (bool, error) {
 	// Get processor's "nominal base frequency" (in MHz) from CPUID
 	freqInfo := cpuid.Cpuid(LEAF_PROCESSOR_FREQUENCY_INFORMATION, 0)

source/custom/rules/cpuid_rule.go

@@ -1,5 +1,5 @@
 /*
-Copyright 2020 The Kubernetes Authors.
+Copyright 2020-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.
@@ -17,26 +17,25 @@ limitations under the License.
 package rules
 
 import (
-	"sigs.k8s.io/node-feature-discovery/source/internal/cpuidutils"
+	"fmt"
+
+	"sigs.k8s.io/node-feature-discovery/source"
+	"sigs.k8s.io/node-feature-discovery/source/cpu"
 )
 
-// CpuIDRule implements Rule
+// CpuIDRule implements Rule for the custom source
 type CpuIDRule []string
 
-var cpuIdFlags map[string]struct{}
-
 func (cpuids *CpuIDRule) Match() (bool, error) {
+	flags, ok := source.GetFeatureSource("cpu").GetFeatures().Keys[cpu.CpuidFeature]
+	if !ok {
+		return false, fmt.Errorf("cpuid information not available")
+	}
+
 	for _, f := range *cpuids {
-		if _, ok := cpuIdFlags[f]; !ok {
+		if _, ok := flags.Elements[f]; !ok {
 			return false, nil
 		}
 	}
 	return true, nil
 }
-
-func init() {
-	cpuIdFlags = make(map[string]struct{})
-	for _, f := range cpuidutils.GetCpuidFlags() {
-		cpuIdFlags[f] = struct{}{}
-	}
-}