Merge pull request #570 from marquiz/devel/errors

Better error reporting of kernel and cpu feature sources

source/cpu/cpu.go

@@ -147,10 +147,10 @@ func (s *Source) Discover() (source.Features, error) {
 	}
 
 	// Detect cstate configuration
-	cstate, err := detectCstate()
+	cstate, ok, err := detectCstate()
 	if err != nil {
-		klog.Errorf("failed to detect cstate: %v", err)
+		klog.Error(err)
-	} else {
+	} else if ok {
 		features["cstate.enabled"] = cstate
 	}
 

source/cpu/cstate.go

@@ -19,33 +19,49 @@ package cpu
 import (
 	"fmt"
 	"io/ioutil"
+	"os"
+	"path/filepath"
 	"strconv"
 	"strings"
 
+	"k8s.io/klog/v2"
+
 	"sigs.k8s.io/node-feature-discovery/source"
 )
 
 // Discover if c-states are enabled
-func detectCstate() (bool, error) {
+func detectCstate() (bool, bool, error) {
-	// When the intel_idle driver is in use (default), check setting of max_cstates
+	// Check that sysfs is available
-	driver, err := ioutil.ReadFile(source.SysfsDir.Path("devices/system/cpu/cpuidle/current_driver"))
+	sysfsBase := source.SysfsDir.Path("devices/system/cpu")
-	if err != nil {
+	if _, err := os.Stat(sysfsBase); err != nil {
-		return false, fmt.Errorf("cannot get driver for cpuidle: %s", err.Error())
+		return false, false, 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
 	}
 
-	if strings.TrimSpace(string(driver)) != "intel_idle" {
+	// 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)
+	}
+
+	if d := strings.TrimSpace(string(driver)); d != "intel_idle" {
 		// Currently only checking intel_idle driver for cstates
-		return false, fmt.Errorf("intel_idle driver is not in use: %s", string(driver))
+		klog.V(1).Infof("intel_idle driver is not in use (%s is active)", d)
+		return false, false, nil
 	}
 
 	data, err := ioutil.ReadFile(source.SysfsDir.Path("module/intel_idle/parameters/max_cstate"))
 	if err != nil {
-		return false, fmt.Errorf("cannot determine cstate from max_cstates: %s", err.Error())
+		return false, false, fmt.Errorf("cannot determine cstate from max_cstates: %w", err)
 	}
 	cstates, err := strconv.Atoi(strings.TrimSpace(string(data)))
 	if err != nil {
-		return false, fmt.Errorf("non-integer value of cstates: %s", err.Error())
+		return false, false, fmt.Errorf("non-integer value of cstates: %w", err)
 	}
 
-	return cstates > 0, nil
+	return cstates > 0, true, nil
 }

source/cpu/pstate.go

@@ -19,6 +19,8 @@ package cpu
 import (
 	"fmt"
 	"io/ioutil"
+	"os"
+	"path/filepath"
 	"runtime"
 	"strings"
 
@@ -35,10 +37,21 @@ func detectPstate() (map[string]string, error) {
 		return nil, nil
 	}
 
+	// Check that sysfs is available
+	sysfsBase := source.SysfsDir.Path("devices/system/cpu")
+	if _, err := os.Stat(sysfsBase); err != nil {
+		return nil, fmt.Errorf("unable to detect pstate status: %w", err)
+	}
+	pstateDir := filepath.Join(sysfsBase, "intel_pstate")
+	if _, err := os.Stat(pstateDir); os.IsNotExist(err) {
+		klog.V(1).Info("intel pstate driver not enabled")
+		return nil, nil
+	}
+
 	// Get global pstate status
-	data, err := ioutil.ReadFile(source.SysfsDir.Path("devices/system/cpu/intel_pstate/status"))
+	data, err := ioutil.ReadFile(filepath.Join(pstateDir, "status"))
 	if err != nil {
-		return nil, fmt.Errorf("could not read pstate status: %s", err.Error())
+		return nil, fmt.Errorf("could not read pstate status: %w", err)
 	}
 	status := strings.TrimSpace(string(data))
 	if status == "off" {
@@ -49,7 +62,7 @@ func detectPstate() (map[string]string, error) {
 	features := map[string]string{"status": status}
 
 	// Check turbo boost
-	bytes, err := ioutil.ReadFile(source.SysfsDir.Path("devices/system/cpu/intel_pstate/no_turbo"))
+	bytes, err := ioutil.ReadFile(filepath.Join(pstateDir, "no_turbo"))
 	if err != nil {
 		klog.Errorf("can't detect whether turbo boost is enabled: %s", err.Error())
 	} else {
@@ -65,7 +78,8 @@ func detectPstate() (map[string]string, error) {
 	}
 
 	// Determine scaling governor that is being used
-	policies, err := ioutil.ReadDir(source.SysfsDir.Path("devices/system/cpu/cpufreq"))
+	cpufreqDir := filepath.Join(sysfsBase, "cpufreq")
+	policies, err := ioutil.ReadDir(cpufreqDir)
 	if err != nil {
 		klog.Errorf("failed to read cpufreq directory: %s", err.Error())
 		return features, nil
@@ -74,7 +88,7 @@ func detectPstate() (map[string]string, error) {
 	scaling := ""
 	for _, policy := range policies {
 		// Ensure at least one cpu is using this policy
-		cpus, err := ioutil.ReadFile(source.SysfsDir.Path("devices/system/cpu/cpufreq", policy.Name(), "affected_cpus"))
+		cpus, err := ioutil.ReadFile(filepath.Join(cpufreqDir, policy.Name(), "affected_cpus"))
 		if err != nil {
 			klog.Errorf("could not read cpufreq policy %s affected_cpus", policy.Name())
 			continue
@@ -84,7 +98,7 @@ func detectPstate() (map[string]string, error) {
 			continue
 		}
 
-		data, err := ioutil.ReadFile(source.SysfsDir.Path("devices/system/cpu/cpufreq", policy.Name(), "scaling_governor"))
+		data, err := ioutil.ReadFile(filepath.Join(cpufreqDir, policy.Name(), "scaling_governor"))
 		if err != nil {
 			klog.Errorf("could not read cpufreq policy %s scaling_governor", policy.Name())
 			continue

source/kernel/selinux.go

@@ -19,15 +19,30 @@ package kernel
 import (
 	"fmt"
 	"io/ioutil"
+	"os"
+	"path/filepath"
+
+	"k8s.io/klog/v2"
 
 	"sigs.k8s.io/node-feature-discovery/source"
 )
 
 // Detect if selinux has been enabled in the kernel
 func SelinuxEnabled() (bool, error) {
-	status, err := ioutil.ReadFile(source.SysfsDir.Path("fs/selinux/enforce"))
+	sysfsBase := source.SysfsDir.Path("fs")
+	if _, err := os.Stat(sysfsBase); err != nil {
+		return false, fmt.Errorf("unable to detect selinux status: %w", err)
+	}
+
+	selinuxBase := filepath.Join(sysfsBase, "selinux")
+	if _, err := os.Stat(selinuxBase); os.IsNotExist(err) {
+		klog.V(1).Info("selinux not available on the system")
+		return false, nil
+	}
+
+	status, err := ioutil.ReadFile(filepath.Join(selinuxBase, "enforce"))
 	if err != nil {
-		return false, fmt.Errorf("failed to detect the status of selinux, please check if the system supports selinux and make sure /sys on the host is mounted into the container: %s", err.Error())
+		return false, fmt.Errorf("failed to detect the status of selinux: %w", err)
 	}
 	if status[0] == byte('1') {
 		// selinux is enabled.