source/usb: implement FeatureSource

Separate feature discovery and creation of feature labels in the usb
source.

Move usb_utils from source/internal to the source/usb package. Change
the implementation of the UsbID custom rule to utilize the FeatureSource
interface of the usb source.

Also, add minimalist unit test.

source/custom/rules/usb_id_rule.go

@@ -18,7 +18,10 @@ package rules
 
 import (
 	"fmt"
-	usbutils "sigs.k8s.io/node-feature-discovery/source/internal"
+
+	"sigs.k8s.io/node-feature-discovery/pkg/api/feature"
+	"sigs.k8s.io/node-feature-discovery/source"
+	"sigs.k8s.io/node-feature-discovery/source/usb"
 )
 
 // Rule that matches on the following USB device attributes: <class, vendor, device>
@@ -38,44 +41,39 @@ type UsbIDRule struct {
 
 // Match USB devices on provided USB device attributes
 func (r *UsbIDRule) Match() (bool, error) {
-	devAttr := map[string]bool{}
-	for _, attr := range []string{"class", "vendor", "device", "serial"} {
-		devAttr[attr] = true
-	}
-	allDevs, err := usbutils.DetectUsb(devAttr)
-	if err != nil {
-		return false, fmt.Errorf("failed to detect USB devices: %s", err.Error())
+	devs, ok := source.GetFeatureSource("usb").GetFeatures().Instances[usb.DeviceFeature]
+	if !ok {
+		return false, fmt.Errorf("usb device information not available")
 	}
 
-	for _, classDevs := range allDevs {
-		for _, dev := range classDevs {
-			// match rule on a single device
-			if r.matchDevOnRule(dev) {
-				return true, nil
-			}
+	for _, dev := range devs.Elements {
+		// match rule on a single device
+		if r.matchDevOnRule(dev) {
+			return true, nil
 		}
 	}
 	return false, nil
 }
 
-func (r *UsbIDRule) matchDevOnRule(dev usbutils.UsbDeviceInfo) bool {
+func (r *UsbIDRule) matchDevOnRule(dev feature.InstanceFeature) bool {
 	if len(r.Class) == 0 && len(r.Vendor) == 0 && len(r.Device) == 0 {
 		return false
 	}
 
-	if len(r.Class) > 0 && !in(dev["class"], r.Class) {
+	attrs := dev.Attributes
+	if len(r.Class) > 0 && !in(attrs["class"], r.Class) {
 		return false
 	}
 
-	if len(r.Vendor) > 0 && !in(dev["vendor"], r.Vendor) {
+	if len(r.Vendor) > 0 && !in(attrs["vendor"], r.Vendor) {
 		return false
 	}
 
-	if len(r.Device) > 0 && !in(dev["device"], r.Device) {
+	if len(r.Device) > 0 && !in(attrs["device"], r.Device) {
 		return false
 	}
 
-	if len(r.Serial) > 0 && !in(dev["serial"], r.Serial) {
+	if len(r.Serial) > 0 && !in(attrs["serial"], r.Serial) {
 		return false
 	}
 

source/usb/usb.go

@@ -22,12 +22,15 @@ import (
 
 	"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"
-	usbutils "sigs.k8s.io/node-feature-discovery/source/internal"
 )
 
 const Name = "usb"
 
+const DeviceFeature = "device"
+
 type Config struct {
 	DeviceClassWhitelist []string `json:"deviceClassWhitelist,omitempty"`
 	DeviceLabelFields    []string `json:"deviceLabelFields,omitempty"`
@@ -46,12 +49,14 @@ func newDefaultConfig() *Config {
 
 // usbSource implements the LabelSource and ConfigurableSource interfaces.
 type usbSource struct {
-	config *Config
+	config   *Config
+	features *feature.DomainFeatures
 }
 
 // Singleton source instance
 var (
-	src usbSource
+	src                           = usbSource{config: newDefaultConfig()}
+	_   source.FeatureSource      = &src
 	_   source.LabelSource        = &src
 	_   source.ConfigurableSource = &src
 )
@@ -80,7 +85,8 @@ func (s *usbSource) Priority() int { return 0 }
 
 // GetLabels method of the LabelSource interface
 func (s *usbSource) GetLabels() (source.FeatureLabels, error) {
-	features := source.FeatureLabels{}
+	labels := source.FeatureLabels{}
+	features := s.GetFeatures()
 
 	// Construct a device label format, a sorted list of valid attributes
 	deviceLabelFields := []string{}
@@ -89,7 +95,7 @@ func (s *usbSource) GetLabels() (source.FeatureLabels, error) {
 		configLabelFields[field] = true
 	}
 
-	for _, attr := range usbutils.DefaultUsbDevAttrs {
+	for _, attr := range devAttrs {
 		if _, ok := configLabelFields[attr]; ok {
 			deviceLabelFields = append(deviceLabelFields, attr)
 			delete(configLabelFields, attr)
@@ -100,42 +106,55 @@ func (s *usbSource) GetLabels() (source.FeatureLabels, error) {
 		for key := range configLabelFields {
 			keys = append(keys, key)
 		}
-		klog.Warningf("invalid fields '%v' in deviceLabelFields, ignoring...", keys)
+		klog.Warningf("invalid fields (%s) in deviceLabelFields, ignoring...", strings.Join(keys, ", "))
 	}
 	if len(deviceLabelFields) == 0 {
 		klog.Warningf("no valid fields in deviceLabelFields defined, using the defaults")
 		deviceLabelFields = []string{"vendor", "device"}
 	}
 
-	// Read configured or default labels. Attributes set to 'true' are considered must-have.
-	deviceAttrs := map[string]bool{}
-	for _, label := range deviceLabelFields {
-		deviceAttrs[label] = true
-	}
-
-	devs, err := usbutils.DetectUsb(deviceAttrs)
-	if err != nil {
-		return nil, fmt.Errorf("failed to detect USB devices: %s", err.Error())
-	}
-
 	// Iterate over all device classes
-	for class, classDevs := range devs {
+	for _, dev := range features.Instances[DeviceFeature].Elements {
+		attrs := dev.Attributes
+		class := attrs["class"]
 		for _, white := range s.config.DeviceClassWhitelist {
-			if strings.HasPrefix(class, strings.ToLower(white)) {
-				for _, dev := range classDevs {
-					devLabel := ""
-					for i, attr := range deviceLabelFields {
-						devLabel += dev[attr]
-						if i < len(deviceLabelFields)-1 {
-							devLabel += "_"
-						}
+			if strings.HasPrefix(string(class), strings.ToLower(white)) {
+				devLabel := ""
+				for i, attr := range deviceLabelFields {
+					devLabel += attrs[attr]
+					if i < len(deviceLabelFields)-1 {
+						devLabel += "_"
 					}
-					features[devLabel+".present"] = true
 				}
+				labels[devLabel+".present"] = true
+				break
 			}
 		}
 	}
-	return features, nil
+	return labels, nil
+}
+
+// Discover method of the FeatureSource interface
+func (s *usbSource) Discover() error {
+	s.features = feature.NewDomainFeatures()
+
+	devs, err := detectUsb()
+	if err != nil {
+		return fmt.Errorf("failed to detect USB devices: %s", err.Error())
+	}
+	s.features.Instances[DeviceFeature] = feature.NewInstanceFeatures(devs)
+
+	utils.KlogDump(3, "discovered usb features:", "  ", s.features)
+
+	return nil
+}
+
+// GetFeatures method of the FeatureSource Interface
+func (s *usbSource) GetFeatures() *feature.DomainFeatures {
+	if s.features == nil {
+		s.features = feature.NewDomainFeatures()
+	}
+	return s.features
 }
 
 func init() {

source/usb/usb_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 usb
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"sigs.k8s.io/node-feature-discovery/pkg/api/feature"
+)
+
+func TestUsbSource(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/usb/utils.go

@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package busutils
+package usb
 
 import (
 	"fmt"
@@ -24,12 +24,11 @@ import (
 	"strings"
 
 	"k8s.io/klog/v2"
+
+	"sigs.k8s.io/node-feature-discovery/pkg/api/feature"
 )
 
-type UsbDeviceInfo map[string]string
-type UsbClassMap map[string]UsbDeviceInfo
-
-var DefaultUsbDevAttrs = []string{"class", "vendor", "device", "serial"}
+var devAttrs = []string{"class", "vendor", "device", "serial"}
 
 // The USB device sysfs files do not have terribly user friendly names, map
 // these for consistency with the PCI matcher.
@@ -58,26 +57,26 @@ func readSingleUsbAttribute(devPath string, attrName string) (string, error) {
 }
 
 // Read information of one USB device
-func readUsbDevInfo(devPath string, deviceAttrSpec map[string]bool) (UsbClassMap, error) {
-	classmap := UsbClassMap{}
-	info := UsbDeviceInfo{}
+func readUsbDevInfo(devPath string) ([]feature.InstanceFeature, error) {
+	instances := make([]feature.InstanceFeature, 0)
+	attrs := make(map[string]string)
 
-	for attr := range deviceAttrSpec {
+	for _, attr := range devAttrs {
 		attrVal, _ := readSingleUsbAttribute(devPath, attr)
 		if len(attrVal) > 0 {
-			info[attr] = attrVal
+			attrs[attr] = attrVal
 		}
 	}
 
 	// USB devices encode their class information either at the device or the interface level. If the device class
 	// is set, return as-is.
-	if info["class"] != "00" {
-		classmap[info["class"]] = info
+	if attrs["class"] != "00" {
+		instances = append(instances, *feature.NewInstanceFeature(attrs))
 	} else {
 		// Otherwise, if a 00 is presented at the device level, descend to the interface level.
 		interfaces, err := filepath.Glob(devPath + "/*/bInterfaceClass")
 		if err != nil {
-			return classmap, err
+			return nil, err
 		}
 
 		// A device may, notably, have multiple interfaces with mixed classes, so we create a unique device for each
@@ -86,54 +85,43 @@ func readUsbDevInfo(devPath string, deviceAttrSpec map[string]bool) (UsbClassMap
 			// Determine the interface class
 			attrVal, err := readSingleUsbSysfsAttribute(intf)
 			if err != nil {
-				return classmap, err
+				return nil, err
 			}
 
-			attr := UsbDeviceInfo{}
-			for k, v := range info {
-				attr[k] = v
+			subdevAttrs := make(map[string]string, len(attrs))
+			for k, v := range attrs {
+				subdevAttrs[k] = v
 			}
+			subdevAttrs["class"] = attrVal
 
-			attr["class"] = attrVal
-			classmap[attrVal] = attr
+			instances = append(instances, *feature.NewInstanceFeature(subdevAttrs))
 		}
 	}
 
-	return classmap, nil
+	return instances, nil
 }
 
-// List available USB devices and retrieve device attributes.
-// deviceAttrSpec is a map which specifies which attributes to retrieve.
-// a false value for a specific attribute marks the attribute as optional.
-// a true value for a specific attribute marks the attribute as mandatory.
-// "class" attribute is considered mandatory.
-// DetectUsb() will fail if the retrieval of a mandatory attribute fails.
-func DetectUsb(deviceAttrSpec map[string]bool) (map[string][]UsbDeviceInfo, error) {
+// detectUsb detects available USB devices and retrieves their device attributes.
+func detectUsb() ([]feature.InstanceFeature, error) {
 	// Unlike PCI, the USB sysfs interface includes entries not just for
 	// devices. We work around this by globbing anything that includes a
 	// valid product ID.
-	const devicePath = "/sys/bus/usb/devices/*/idProduct"
-	devInfo := make(map[string][]UsbDeviceInfo)
-
-	devices, err := filepath.Glob(devicePath)
+	const devPathGlob = "/sys/bus/usb/devices/*/idProduct"
+	devPaths, err := filepath.Glob(devPathGlob)
 	if err != nil {
 		return nil, err
 	}
 
-	// "class" is a mandatory attribute, inject it to spec if needed.
-	deviceAttrSpec["class"] = true
-
 	// Iterate over devices
-	for _, device := range devices {
-		devMap, err := readUsbDevInfo(filepath.Dir(device), deviceAttrSpec)
+	devInfo := make([]feature.InstanceFeature, 0)
+	for _, devPath := range devPaths {
+		devs, err := readUsbDevInfo(filepath.Dir(devPath))
 		if err != nil {
 			klog.Error(err)
 			continue
 		}
 
-		for class, info := range devMap {
-			devInfo[class] = append(devInfo[class], info)
-		}
+		devInfo = append(devInfo, devs...)
 	}
 
 	return devInfo, nil