From f7b2a7b41bb2a7dacdab2037523acf136e13e964 Mon Sep 17 00:00:00 2001
From: Jason Kulatunga <jason@thesparktree.com>
Date: Sat, 22 Aug 2020 17:27:44 -0700
Subject: [PATCH] make sure results from failing smartctl execution is still
 sent to API.

---
 collector/pkg/collector/base.go       |  4 +-
 collector/pkg/collector/base_test.go  | 67 +++++++++++++++++++++++++++
 collector/pkg/collector/metrics.go    | 18 ++++---
 webapp/backend/pkg/version/version.go |  2 +-
 4 files changed, 81 insertions(+), 10 deletions(-)
 create mode 100644 collector/pkg/collector/base_test.go

diff --git a/collector/pkg/collector/base.go b/collector/pkg/collector/base.go
index 6a2f8f3..cd379e0 100644
--- a/collector/pkg/collector/base.go
+++ b/collector/pkg/collector/base.go
@@ -21,7 +21,7 @@ type BaseCollector struct {
 	logger *logrus.Entry
 }
 
-func (c *BaseCollector) detectStorageDevices() ([]models.Device, error) {
+func (c *BaseCollector) DetectStorageDevices() ([]models.Device, error) {
 
 	block, err := ghw.Block()
 	if err != nil {
@@ -113,7 +113,7 @@ func (c *BaseCollector) postJson(url string, body interface{}, target interface{
 	return json.NewDecoder(r.Body).Decode(target)
 }
 
-func (c *BaseCollector) execCmd(cmdName string, cmdArgs []string, workingDir string, environ []string) (string, error) {
+func (c *BaseCollector) ExecCmd(cmdName string, cmdArgs []string, workingDir string, environ []string) (string, error) {
 
 	cmd := exec.Command(cmdName, cmdArgs...)
 	var stdBuffer bytes.Buffer
diff --git a/collector/pkg/collector/base_test.go b/collector/pkg/collector/base_test.go
new file mode 100644
index 0000000..def6dff
--- /dev/null
+++ b/collector/pkg/collector/base_test.go
@@ -0,0 +1,67 @@
+package collector_test
+
+import (
+	"github.com/analogj/scrutiny/collector/pkg/collector"
+	"github.com/stretchr/testify/require"
+	"os/exec"
+	"testing"
+)
+
+func TestExecCmd(t *testing.T) {
+	t.Parallel()
+
+	//setup
+	bc := collector.BaseCollector{}
+
+	//test
+	result, err := bc.ExecCmd("echo", []string{"hello world"}, "", nil)
+
+	//assert
+	require.NoError(t, err)
+	require.Equal(t, "hello world\n", result)
+}
+
+func TestExecCmd_Date(t *testing.T) {
+	t.Parallel()
+
+	//setup
+	bc := collector.BaseCollector{}
+
+	//test
+	_, err := bc.ExecCmd("date", []string{}, "", nil)
+
+	//assert
+	require.NoError(t, err)
+}
+
+//
+//func TestExecCmd_Error(t *testing.T) {
+//	t.Parallel()
+//
+//	//setup
+//	bc := collector.BaseCollector {}
+//
+//	//test
+//	_, err := bc.ExecCmd("smartctl", []string{"-a", "/dev/doesnotexist"}, "", nil)
+//
+//	//assert
+//	exitError, castOk := err.(*exec.ExitError);
+//	require.True(t, castOk)
+//	require.Equal(t, 1, exitError.ExitCode())
+//
+//}
+//
+
+func TestExecCmd_InvalidCommand(t *testing.T) {
+	t.Parallel()
+
+	//setup
+	bc := collector.BaseCollector{}
+
+	//test
+	_, err := bc.ExecCmd("invalid_binary", []string{}, "", nil)
+
+	//assert
+	_, castOk := err.(*exec.ExitError)
+	require.False(t, castOk)
+}
diff --git a/collector/pkg/collector/metrics.go b/collector/pkg/collector/metrics.go
index 56ea24b..7bd812b 100644
--- a/collector/pkg/collector/metrics.go
+++ b/collector/pkg/collector/metrics.go
@@ -43,7 +43,7 @@ func (mc *MetricsCollector) Run() error {
 	apiEndpoint.Path = "/api/devices/register"
 
 	deviceRespWrapper := new(models.DeviceWrapper)
-	detectedStorageDevices, err := mc.detectStorageDevices()
+	detectedStorageDevices, err := mc.DetectStorageDevices()
 	if err != nil {
 		return err
 	}
@@ -92,14 +92,18 @@ func (mc *MetricsCollector) Collect(wg *sync.WaitGroup, deviceWWN string, device
 	defer wg.Done()
 	mc.logger.Infof("Collecting smartctl results for %s\n", deviceName)
 
-	result, err := mc.execCmd("smartctl", []string{"-a", "-j", fmt.Sprintf("/dev/%s", deviceName)}, "", nil)
+	result, err := mc.ExecCmd("smartctl", []string{"-a", "-j", fmt.Sprintf("/dev/%s", deviceName)}, "", nil)
 	resultBytes := []byte(result)
 	if err != nil {
-		mc.logger.Errorf("error while retrieving data from smartctl %s\n", deviceName)
-		mc.logger.Errorf("ERROR MESSAGE: %v", err)
-		mc.logger.Errorf("RESULT: %v", result)
-		// TODO: error while retrieving data from smartctl.
-		// TODO: we should pass this data on to scrutiny API for recording.
+		if exitError, ok := err.(*exec.ExitError); ok {
+			// smartctl command exited with an error, we should still push the data to the API server
+			mc.logger.Errorf("smartctl returned an error code (%d) while processing %s\n", exitError.ExitCode(), deviceName)
+			mc.Publish(deviceWWN, resultBytes)
+		} else {
+			mc.logger.Errorf("error while attempting to execute smartctl: %s\n", deviceName)
+			mc.logger.Errorf("ERROR MESSAGE: %v", err)
+			mc.logger.Errorf("IGNORING RESULT: %v", result)
+		}
 		return
 	} else {
 		//successful run, pass the results directly to webapp backend for parsing and processing.
diff --git a/webapp/backend/pkg/version/version.go b/webapp/backend/pkg/version/version.go
index 4ce3f31..1963faf 100644
--- a/webapp/backend/pkg/version/version.go
+++ b/webapp/backend/pkg/version/version.go
@@ -2,4 +2,4 @@ package version
 
 // VERSION is the app-global version string, which will be replaced with a
 // new value during packaging
-const VERSION = "0.1.2"
+const VERSION = "0.1.3"