From c7615523d103052de591ecf8c09751d81265b1af Mon Sep 17 00:00:00 2001 From: "James O. D. Hunt" Date: Wed, 27 Sep 2017 11:20:49 +0100 Subject: [PATCH 1/2] cc-check: Always run all tests Change "cc-check" so that it always runs all commands before erroring. Previously, it would fail on first error, but this could mask the real reason for failure. For example, on a system with nesting and vhost disabled, "cc-check" would fail with a nesting error. But the nesting check is not strictly required (#281) whereas vhost definitely is which lead to confusing results for users. "cc-check" will now run all tests and display an error message at the end if any errors were found in the set of tests run. Fixes #638. Signed-off-by: James O. D. Hunt --- cc-check.go | 77 +++++++++++++++++++++++++++++++++++------------- cc-check_test.go | 70 ++++++++++++++++++++++++++++++------------- 2 files changed, 106 insertions(+), 41 deletions(-) diff --git a/cc-check.go b/cc-check.go index 7989a9e5..870ab330 100644 --- a/cc-check.go +++ b/cc-check.go @@ -15,7 +15,6 @@ package main import ( - "errors" "fmt" "os/exec" "path/filepath" @@ -38,6 +37,7 @@ const ( moduleParamDir = "parameters" cpuFlagsTag = "flags" successMessage = "System is capable of running " + project + failMessage = "System is not capable of running " + project ) // variables rather than consts to allow tests to modify them @@ -83,7 +83,7 @@ var requiredKernelModules = map[string]kernelModule{ }, } -// return details of the first CPU +// getCPUInfo returns details of the first CPU func getCPUInfo(cpuInfoFile string) (string, error) { text, err := getFileContents(cpuInfoFile) if err != nil { @@ -137,38 +137,51 @@ func haveKernelModule(module string) bool { return err == nil } -func checkCPU(tag, cpuinfo string, attribs map[string]string) error { +// checkCPU checks all required CPU attributes modules and returns a count of +// the number of CPU attribute errors (all of which are logged by this +// function). Only fatal errors result in an error return. +func checkCPU(tag, cpuinfo string, attribs map[string]string) (count uint32, err error) { if cpuinfo == "" { - return fmt.Errorf("Need cpuinfo") + return 0, fmt.Errorf("Need cpuinfo") } for attrib, desc := range attribs { found := findAnchoredString(cpuinfo, attrib) if !found { - return fmt.Errorf("CPU does not have required %v: %q (%s)", tag, desc, attrib) + ccLog.Errorf("CPU does not have required %v: %q (%s)", tag, desc, attrib) + count++ + continue + } + ccLog.Infof("Found CPU %v %q (%s)", tag, desc, attrib) } - return nil + return count, nil } -func checkCPUFlags(cpuflags string, required map[string]string) error { + +func checkCPUFlags(cpuflags string, required map[string]string) (uint32, error) { return checkCPU("flag", cpuflags, required) } -func checkCPUAttribs(cpuinfo string, attribs map[string]string) error { +func checkCPUAttribs(cpuinfo string, attribs map[string]string) (uint32, error) { return checkCPU("attribute", cpuinfo, attribs) } -func checkKernelModules(modules map[string]kernelModule) error { +// checkKernelModules checks all required kernel modules modules and returns a count of +// the number of module errors (all of which are logged by this +// function). Only fatal errors result in an error return. +func checkKernelModules(modules map[string]kernelModule) (count uint32, err error) { onVMM, err := vc.RunningOnVMM(procCPUInfo) if err != nil { - return err + return 0, err } for module, details := range modules { if !haveKernelModule(module) { - return fmt.Errorf("kernel module %q (%s) not found", module, details.desc) + ccLog.Errorf("kernel module %q (%s) not found", module, details.desc) + count++ + continue } ccLog.Infof("Found kernel module %q (%s)", details.desc, module) @@ -177,7 +190,7 @@ func checkKernelModules(modules map[string]kernelModule) error { path := filepath.Join(sysModuleDir, module, moduleParamDir, param) value, err := getFileContents(path) if err != nil { - return err + return 0, err } value = strings.TrimRight(value, "\n\r") @@ -192,14 +205,15 @@ func checkKernelModules(modules map[string]kernelModule) error { continue } - return errors.New(msg) + ccLog.Error(msg) + count++ } ccLog.Infof("Kernel module %q parameter %q has correct value", details.desc, param) } } - return nil + return count, nil } // hostIsClearContainersCapable determines if the system is capable of @@ -210,20 +224,41 @@ func hostIsClearContainersCapable(cpuinfoFile string) error { return err } - if err = checkCPUAttribs(cpuinfo, requiredCPUAttribs); err != nil { - return err - } - cpuFlags := getCPUFlags(cpuinfo) if cpuFlags == "" { return fmt.Errorf("Cannot find CPU flags") } - if err = checkCPUFlags(cpuFlags, requiredCPUFlags); err != nil { + // Keep a track of the error count, but don't error until all tests + // have been performed! + errorCount := uint32(0) + + count, err := checkCPUAttribs(cpuinfo, requiredCPUAttribs) + if err != nil { return err } - return checkKernelModules(requiredKernelModules) + errorCount += count + + count, err = checkCPUFlags(cpuFlags, requiredCPUFlags) + if err != nil { + return err + } + + errorCount += count + + count, err = checkKernelModules(requiredKernelModules) + if err != nil { + return err + } + + errorCount += count + + if errorCount == 0 { + return nil + } + + return fmt.Errorf("ERROR: %s", failMessage) } var ccCheckCLICommand = cli.Command{ @@ -232,7 +267,7 @@ var ccCheckCLICommand = cli.Command{ Action: func(context *cli.Context) error { err := hostIsClearContainersCapable(procCPUInfo) if err != nil { - return fmt.Errorf("ERROR: %v", err) + return err } ccLog.Info("") diff --git a/cc-check_test.go b/cc-check_test.go index 4ca1d6ec..9bdfee4e 100644 --- a/cc-check_test.go +++ b/cc-check_test.go @@ -22,7 +22,6 @@ import ( "path" "path/filepath" "regexp" - "strings" "testing" "github.com/stretchr/testify/assert" @@ -156,6 +155,7 @@ func TestCheckCheckCPUFlags(t *testing.T) { cpuflags string required map[string]string expectError bool + expectCount uint32 } data := []testData{ @@ -163,6 +163,7 @@ func TestCheckCheckCPUFlags(t *testing.T) { "", map[string]string{}, true, + 0, }, { "", @@ -170,6 +171,7 @@ func TestCheckCheckCPUFlags(t *testing.T) { "a": "A flag", }, true, + 0, }, { "", @@ -178,6 +180,7 @@ func TestCheckCheckCPUFlags(t *testing.T) { "b": "B flag", }, true, + 0, }, { "a b c", @@ -185,16 +188,29 @@ func TestCheckCheckCPUFlags(t *testing.T) { "b": "B flag", }, false, + 0, + }, + { + "a b c", + map[string]string{ + "x": "X flag", + "y": "Y flag", + "z": "Z flag", + }, + false, + 3, }, } for _, d := range data { - err := checkCPUFlags(d.cpuflags, d.required) + count, err := checkCPUFlags(d.cpuflags, d.required) if d.expectError { - assert.Error(err) + assert.Error(err, "%+v", d) } else { - assert.NoError(err) + assert.NoError(err, "%+v", d) } + + assert.Equal(d.expectCount, count, "%+v", d) } } @@ -205,6 +221,7 @@ func TestCheckCheckCPUAttribs(t *testing.T) { cpuinfo string required map[string]string expectError bool + expectCount uint32 } data := []testData{ @@ -212,6 +229,7 @@ func TestCheckCheckCPUAttribs(t *testing.T) { "", map[string]string{}, true, + 0, }, { "", @@ -219,6 +237,7 @@ func TestCheckCheckCPUAttribs(t *testing.T) { "a": "", }, true, + 0, }, { "a: b", @@ -226,6 +245,7 @@ func TestCheckCheckCPUAttribs(t *testing.T) { "b": "B attribute", }, false, + 0, }, { "a: b\nc: d\ne: f", @@ -233,6 +253,7 @@ func TestCheckCheckCPUAttribs(t *testing.T) { "b": "B attribute", }, false, + 0, }, { "a: b\n", @@ -241,7 +262,8 @@ func TestCheckCheckCPUAttribs(t *testing.T) { "c": "C attribute", "d": "D attribute", }, - true, + false, + 2, }, { "a: b\nc: d\ne: f", @@ -251,16 +273,19 @@ func TestCheckCheckCPUAttribs(t *testing.T) { "f": "F attribute", }, false, + 0, }, } for _, d := range data { - err := checkCPUAttribs(d.cpuinfo, d.required) + count, err := checkCPUAttribs(d.cpuinfo, d.required) if d.expectError { - assert.Error(err) + assert.Error(err, "%+v", d) } else { - assert.NoError(err) + assert.NoError(err, "%+v", d) } + + assert.Equal(d.expectCount, count, "%+v", d) } } @@ -356,13 +381,15 @@ func TestCheckCheckKernelModules(t *testing.T) { }, } - err = checkKernelModules(map[string]kernelModule{}) + count, err := checkKernelModules(map[string]kernelModule{}) // No required modules means no error assert.NoError(err) + assert.Equal(count, uint32(0)) - err = checkKernelModules(testData) + count, err = checkKernelModules(testData) + assert.NoError(err) // No modules exist - assert.Error(err) + assert.Equal(count, uint32(2)) for module, details := range testData { path := filepath.Join(sysModuleDir, module) @@ -386,8 +413,9 @@ func TestCheckCheckKernelModules(t *testing.T) { } } - err = checkKernelModules(testData) + count, err = checkKernelModules(testData) assert.NoError(err) + assert.Equal(count, uint32(0)) } func TestCheckCheckKernelModulesNoUnrestrictedGuest(t *testing.T) { @@ -440,7 +468,7 @@ func TestCheckCheckKernelModulesNoUnrestrictedGuest(t *testing.T) { vendor := "GenuineIntel" flags := "vmx lm sse4_1" - err = checkKernelModules(requiredModules) + _, err = checkKernelModules(requiredModules) // no cpuInfoFile yet assert.Error(err) @@ -449,11 +477,11 @@ func TestCheckCheckKernelModulesNoUnrestrictedGuest(t *testing.T) { createModules(assert, cpuInfoFile, actualModuleData) - err = checkKernelModules(requiredModules) + count, err := checkKernelModules(requiredModules) + assert.NoError(err) // fails due to unrestricted_guest not being available - assert.Error(err) - assert.True(strings.Contains(err.Error(), "unrestricted_guest")) + assert.Equal(count, uint32(1)) // pretend test is running under a hypervisor flags += " hypervisor" @@ -473,10 +501,11 @@ func TestCheckCheckKernelModulesNoUnrestrictedGuest(t *testing.T) { ccLog.Out = buf - err = checkKernelModules(requiredModules) + count, err = checkKernelModules(requiredModules) // no error now because running under a hypervisor assert.NoError(err) + assert.Equal(count, uint32(0)) re := regexp.MustCompile(`\bwarning\b.*\bunrestricted_guest\b`) matches := re.FindAllStringSubmatch(buf.String(), -1) @@ -530,7 +559,7 @@ func TestCheckCheckKernelModulesUnreadableFile(t *testing.T) { err = os.Chmod(modParamFile, 0000) assert.NoError(err) - err = checkKernelModules(testData) + _, err = checkKernelModules(testData) assert.Error(err) } @@ -573,8 +602,9 @@ func TestCheckCheckKernelModulesInvalidFileContents(t *testing.T) { err = createFile(modParamFile, "burp") assert.NoError(err) - err = checkKernelModules(testData) - assert.Error(err) + count, err := checkKernelModules(testData) + assert.NoError(err) + assert.Equal(count, uint32(1)) } func createModules(assert *assert.Assertions, cpuInfoFile string, moduleData []testModuleData) { From 413e2ed246fb17c80494d4ee7f3ec4c324c2164e Mon Sep 17 00:00:00 2001 From: "James O. D. Hunt" Date: Wed, 27 Sep 2017 14:34:14 +0100 Subject: [PATCH 2/2] cc-check: Only warn if nesting not available KVM nesting support is not strictly necessary, so warn rather than erroring if not available. Fixes #281. Signed-off-by: James O. D. Hunt --- cc-check.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/cc-check.go b/cc-check.go index 870ab330..908c97b4 100644 --- a/cc-check.go +++ b/cc-check.go @@ -205,6 +205,11 @@ func checkKernelModules(modules map[string]kernelModule) (count uint32, err erro continue } + if param == "nested" { + ccLog.Warn(msg) + continue + } + ccLog.Error(msg) count++ }