Fix other unhandled error cases

pkg/checks/cpu.go

@@ -7,14 +7,17 @@ import (
 	"github.com/conjurinc/conjur-preflight/pkg/framework"
 )
 
+// Cpu collects inspection information on the host machines CPU cores and
+// architecture
 type Cpu struct {
 }
 
-// Describe provides a textual description of what this check gathers info on
+// Describe provides a textual description of the info this check gathers
 func (*Cpu) Describe() string {
 	return "CPU"
 }
 
+// Run executes the CPU inspection checks
 func (cpu *Cpu) Run() <-chan []framework.CheckResult {
 	future := make(chan []framework.CheckResult)
 

pkg/checks/cpu_test.go

@@ -1,16 +1,15 @@
-package checks_test
+package checks
 
 import (
 	"regexp"
 	"testing"
 
-	"github.com/conjurinc/conjur-preflight/pkg/checks"
 	"github.com/conjurinc/conjur-preflight/pkg/framework"
 	"github.com/stretchr/testify/assert"
 )
 
 func TestCpuRun(t *testing.T) {
-	testCheck := &checks.Cpu{}
+	testCheck := &Cpu{}
 	resultChan := testCheck.Run()
 	results := <-resultChan
 

pkg/checks/disk/iops.go

@@ -23,6 +23,8 @@ type IopsCheck struct {
 	fioNewJob func(string, []string) fio.Executable
 }
 
+var getWorkingDirectory func() (string, error) = os.Getwd
+
 // NewIopsCheck instantiates an Iops check with the default dependencies
 func NewIopsCheck(debug bool) *IopsCheck {
 	return &IopsCheck{
@@ -91,7 +93,11 @@ func fioReadIopsResult(job *fio.JobResult) framework.CheckResult {
 	}
 
 	// Format title
-	path, _ := os.Getwd()
+	path, err := getWorkingDirectory()
+	if err != nil {
+		log.Debug("Unable to get working directory: %s", err)
+		path = "working directory"
+	}
 	titleStr := fmt.Sprintf("FIO - Read IOPs (%s)", path)
 
 	// Format value
@@ -119,7 +125,11 @@ func fioWriteIopsResult(job *fio.JobResult) framework.CheckResult {
 	}
 
 	// Format title
-	path, _ := os.Getwd()
+	path, err := getWorkingDirectory()
+	if err != nil {
+		log.Debug("Unable to get working directory: %s", err)
+		path = "working directory"
+	}
 	titleStr := fmt.Sprintf("FIO - Write IOPs (%s)", path)
 
 	// Format value

pkg/checks/disk/iops_test.go

@@ -94,6 +94,32 @@ func TestIopsWithNoJobs(t *testing.T) {
 	assert.Equal(t, "No job results returned by 'fio'", results[0].Message)
 }
 
+func TestIopsWithWorkingDirectoryError(t *testing.T) {
+	// Double the working directory function to simulate it failing with an error
+	originalWorkingDirectoryFunc := getWorkingDirectory
+	getWorkingDirectory = failedWorkingDir
+	defer func() {
+		getWorkingDirectory = originalWorkingDirectoryFunc
+	}()
+
+	testCheck := &IopsCheck{
+		debug:     true,
+		fioNewJob: newSuccessfulIopsFioJob,
+	}
+	resultChan := testCheck.Run()
+	results := <-resultChan
+
+	assert.Equal(
+		t,
+		2,
+		len(results),
+		"There are read and write IOPs results present",
+	)
+
+	assertReadIopsResult(t, results[0], framework.STATUS_INFO)
+	assertWriteIopsResult(t, results[1], framework.STATUS_INFO)
+}
+
 func assertReadIopsResult(
 	t *testing.T,
 	result framework.CheckResult,

pkg/checks/disk/latency.go

@@ -2,10 +2,10 @@ package disk
 
 import (
 	"fmt"
-	"os"
 
 	"github.com/conjurinc/conjur-preflight/pkg/checks/disk/fio"
 	"github.com/conjurinc/conjur-preflight/pkg/framework"
+	"github.com/conjurinc/conjur-preflight/pkg/log"
 )
 
 // LatencyCheck is a pre-flight check to report the read, write, and sync
@@ -90,7 +90,11 @@ func fioReadLatencyResult(jobResult *fio.JobResult) framework.CheckResult {
 		status = framework.STATUS_WARN
 	}
 
-	path, _ := os.Getwd()
+	path, err := getWorkingDirectory()
+	if err != nil {
+		log.Debug("Unable to get working directory: %s", err)
+		path = "working directory"
+	}
 
 	return framework.CheckResult{
 		Title:  fmt.Sprintf("FIO - Read Latency (99%%, %s)", path),
@@ -110,7 +114,11 @@ func fioWriteLatencyResult(jobResult *fio.JobResult) framework.CheckResult {
 		status = framework.STATUS_WARN
 	}
 
-	path, _ := os.Getwd()
+	path, err := getWorkingDirectory()
+	if err != nil {
+		log.Debug("Unable to get working directory: %s", err)
+		path = "working directory"
+	}
 
 	return framework.CheckResult{
 		Title:  fmt.Sprintf("FIO - Write Latency (99%%, %s)", path),
@@ -130,7 +138,11 @@ func fioSyncLatencyResult(jobResult *fio.JobResult) framework.CheckResult {
 		status = framework.STATUS_WARN
 	}
 
-	path, _ := os.Getwd()
+	path, err := getWorkingDirectory()
+	if err != nil {
+		log.Debug("Unable to get working directory: %s", err)
+		path = "working directory"
+	}
 
 	return framework.CheckResult{
 		Title:  fmt.Sprintf("FIO - Sync Latency (99%%, %s)", path),

pkg/checks/disk/latency_test.go

@@ -3,6 +3,7 @@
 package disk
 
 import (
+	"errors"
 	"regexp"
 	"testing"
 
@@ -71,6 +72,27 @@ func TestLatencyWithNoJobs(t *testing.T) {
 	assert.Equal(t, "No job results returned by 'fio'", results[0].Message)
 }
 
+func TestLatencyWithWorkingDirectoryError(t *testing.T) {
+	// Double the working directory function to simulate it failing with an error
+	originalWorkingDirectoryFunc := getWorkingDirectory
+	getWorkingDirectory = failedWorkingDir
+	defer func() {
+		getWorkingDirectory = originalWorkingDirectoryFunc
+	}()
+
+	testCheck := &LatencyCheck{
+		fioNewJob: newSuccessfulLatencyFioJob,
+	}
+	resultChan := testCheck.Run()
+	results := <-resultChan
+
+	assert.Equal(t, 3, len(results), "There are disk latency results present")
+
+	assertReadLatencyResult(t, results[0], framework.STATUS_INFO)
+	assertWriteLatencyResult(t, results[1], framework.STATUS_INFO)
+	assertSyncLatencyResult(t, results[2], framework.STATUS_INFO)
+}
+
 func assertReadLatencyResult(
 	t *testing.T,
 	result framework.CheckResult,
@@ -187,3 +209,7 @@ func newPoorLatencyPerformanceFioJob(
 		},
 	}
 }
+
+func failedWorkingDir() (string, error) {
+	return "", errors.New("working directory error")
+}

pkg/checks/disk/space.go

@@ -9,19 +9,14 @@ import (
 	"github.com/shirou/gopsutil/v3/disk"
 )
 
+// Aliasing our external dependencies like this allows to swap them out for
+// testing
+var getPartitions func(all bool) ([]disk.PartitionStat, error) = disk.Partitions
+var getUsage func(path string) (*disk.UsageStat, error) = disk.Usage
+
 // SpaceCheck reports on the available partitions and devices on the current
 // machine, as well as their available disk space.
-type SpaceCheck struct {
-	partitionsFunc func(all bool) ([]disk.PartitionStat, error)
-	usageFunc      func(path string) (*disk.UsageStat, error)
-}
-
-func NewSpaceCheck() *SpaceCheck {
-	return &SpaceCheck{
-		partitionsFunc: disk.Partitions,
-		usageFunc:      disk.Usage,
-	}
-}
+type SpaceCheck struct{}
 
 // Describe provides a textual description of what this check gathers info on
 func (*SpaceCheck) Describe() string {
@@ -33,7 +28,7 @@ func (spaceCheck *SpaceCheck) Run() <-chan []framework.CheckResult {
 	future := make(chan []framework.CheckResult)
 
 	go func() {
-		partitions, err := spaceCheck.partitionsFunc(true)
+		partitions, err := getPartitions(true)
 		// If we can't list the partitions, we exit early with the failure message
 		if err != nil {
 			log.Debug("Unable to list disk partitions: %s", err)
@@ -51,7 +46,7 @@ func (spaceCheck *SpaceCheck) Run() <-chan []framework.CheckResult {
 		results := []framework.CheckResult{}
 
 		for _, partition := range partitions {
-			usage, err := spaceCheck.usageFunc(partition.Mountpoint)
+			usage, err := getUsage(partition.Mountpoint)
 			if err != nil {
 				log.Debug(
 					"Unable to collect disk usage for '%s': %s",

pkg/checks/disk/space_test.go

@@ -11,7 +11,7 @@ import (
 )
 
 func TestSpaceCheck(t *testing.T) {
-	testCheck := NewSpaceCheck()
+	testCheck := &SpaceCheck{}
 	resultChan := testCheck.Run()
 	results := <-resultChan
 
@@ -35,10 +35,14 @@ func TestSpaceCheck(t *testing.T) {
 }
 
 func TestPartitionListError(t *testing.T) {
-	testCheck := &SpaceCheck{
-		partitionsFunc: failedPartitionsFunc,
-		usageFunc:      disk.Usage,
-	}
+	// Double the usage function to simulate an error
+	originalPartitionsFunc := getPartitions
+	getPartitions = failedPartitionsFunc
+	defer func() {
+		getPartitions = originalPartitionsFunc
+	}()
+
+	testCheck := &SpaceCheck{}
 	resultChan := testCheck.Run()
 	results := <-resultChan
 
@@ -50,10 +54,14 @@ func TestPartitionListError(t *testing.T) {
 }
 
 func TestDiskUsageError(t *testing.T) {
-	testCheck := &SpaceCheck{
-		partitionsFunc: disk.Partitions,
-		usageFunc:      failedUsageFunc,
-	}
+	// Double the usage function to simulate an error
+	originalUsageFunc := getUsage
+	getUsage = failedUsageFunc
+	defer func() {
+		getUsage = originalUsageFunc
+	}()
+
+	testCheck := &SpaceCheck{}
 	resultChan := testCheck.Run()
 	results := <-resultChan
 

pkg/checks/follower_test.go

@@ -1,15 +1,14 @@
-package checks_test
+package checks
 
 import (
 	"testing"
 
-	"github.com/conjurinc/conjur-preflight/pkg/checks"
 	"github.com/stretchr/testify/assert"
 )
 
 func TestFollowerRun(t *testing.T) {
 	t.Setenv("MASTER_HOSTNAME", "http://example.com")
-	testCheck := &checks.Follower{}
+	testCheck := &Follower{}
 	resultChan := testCheck.Run()
 	results := <-resultChan
 

pkg/checks/host.go

@@ -5,23 +5,40 @@ import (
 	"time"
 
 	"github.com/conjurinc/conjur-preflight/pkg/framework"
+	"github.com/conjurinc/conjur-preflight/pkg/log"
 	"github.com/hako/durafmt"
 	"github.com/shirou/gopsutil/v3/host"
 )
 
-type Host struct {
-}
+var getHostInfo func() (*host.InfoStat, error) = host.Info
+
+// Host collects inspection information on the host machine's metadata, such
+// as the operating system
+type Host struct{}
 
-// Describe provides a textual description of what this check gathers info on
+// Describe provides a textual description of what this check gathers
 func (*Host) Describe() string {
 	return "operating system"
 }
 
-func (*Host) Run() <-chan []framework.CheckResult {
+// Run executes the Host inspection checks
+func (host *Host) Run() <-chan []framework.CheckResult {
 	future := make(chan []framework.CheckResult)
 
 	go func() {
-		hostInfo, _ := host.Info()
+		hostInfo, err := getHostInfo()
+		if err != nil {
+			log.Debug("Unable to inspect host info: %s", err)
+			future <- []framework.CheckResult{
+				{
+					Title:  "Error",
+					Status: framework.STATUS_ERROR,
+					Value:  fmt.Sprintf("%s", err),
+				},
+			}
+
+			return
+		}
 
 		future <- []framework.CheckResult{
 			hostnameResult(hostInfo),