allow warning when filesystem performance not collected (fio) (#1363)

* add a way to only warn when the host fs perf file was not collected * make fileNotCollected an exported constant
replicatedhq · Oct 10, 2023 · 461cc99 · 461cc99
1 parent 1bdf87e
commit 461cc99
Show file tree

Hide file tree

Showing 3 changed files with 170 additions and 6 deletions.
diff --git a/pkg/analyze/analyzer.go b/pkg/analyze/analyzer.go
@@ -18,6 +18,8 @@ import (
 	"k8s.io/klog/v2"
 )
 
+const FILE_NOT_COLLECTED = "fileNotCollected"
+
 type AnalyzeResult struct {
 	IsPass bool
 	IsFail bool

diff --git a/pkg/analyze/host_filesystem_performance.go b/pkg/analyze/host_filesystem_performance.go
@@ -32,13 +32,40 @@ func (a *AnalyzeHostFilesystemPerformance) Analyze(
 ) ([]*AnalyzeResult, error) {
 	hostAnalyzer := a.hostAnalyzer
 
+	result := &AnalyzeResult{
+		Title: a.Title(),
+	}
+
 	collectorName := hostAnalyzer.CollectorName
 	if collectorName == "" {
 		collectorName = "filesystemPerformance"
 	}
 	name := filepath.Join("host-collectors/filesystemPerformance", collectorName+".json")
 	contents, err := getCollectedFileContents(name)
 	if err != nil {
+		if len(hostAnalyzer.Outcomes) >= 1 {
+			// if the very first outcome is FILE_NOT_COLLECTED', then use that outcome
+			// otherwise, return the error
+			if hostAnalyzer.Outcomes[0].Fail != nil && hostAnalyzer.Outcomes[0].Fail.When == FILE_NOT_COLLECTED {
+				result.IsFail = true
+				result.Message = renderFSPerfOutcome(hostAnalyzer.Outcomes[0].Fail.Message, collect.FSPerfResults{})
+				result.URI = hostAnalyzer.Outcomes[0].Fail.URI
+				return []*AnalyzeResult{result}, nil
+			}
+			if hostAnalyzer.Outcomes[0].Warn != nil && hostAnalyzer.Outcomes[0].Warn.When == FILE_NOT_COLLECTED {
+				result.IsWarn = true
+				result.Message = renderFSPerfOutcome(hostAnalyzer.Outcomes[0].Warn.Message, collect.FSPerfResults{})
+				result.URI = hostAnalyzer.Outcomes[0].Warn.URI
+				return []*AnalyzeResult{result}, nil
+			}
+			if hostAnalyzer.Outcomes[0].Pass != nil && hostAnalyzer.Outcomes[0].Pass.When == FILE_NOT_COLLECTED {
+				result.IsPass = true
+				result.Message = renderFSPerfOutcome(hostAnalyzer.Outcomes[0].Pass.Message, collect.FSPerfResults{})
+				result.URI = hostAnalyzer.Outcomes[0].Pass.URI
+				return []*AnalyzeResult{result}, nil
+			}
+		}
+
 		return nil, errors.Wrapf(err, "failed to get collected file %s", name)
 	}
 
@@ -58,10 +85,6 @@ func (a *AnalyzeHostFilesystemPerformance) Analyze(
 		return nil, errors.Wrapf(err, "failed to unmarshal filesystem performance results from %s", name)
 	}
 
-	result := &AnalyzeResult{
-		Title: a.Title(),
-	}
-
 	for _, outcome := range hostAnalyzer.Outcomes {
 
 		if outcome.Fail != nil {
@@ -135,6 +158,10 @@ func (a *AnalyzeHostFilesystemPerformance) Analyze(
 }
 
 func compareHostFilesystemPerformanceConditionalToActual(conditional string, fsPerf collect.FSPerfResults) (res bool, err error) {
+	if conditional == FILE_NOT_COLLECTED {
+		return false, nil
+	}
+
 	parts := strings.Split(conditional, " ")
 	if len(parts) != 3 {
 		return false, fmt.Errorf("conditional must have exactly 3 parts, got %d", len(parts))

diff --git a/pkg/analyze/host_filesystem_performance_test.go b/pkg/analyze/host_filesystem_performance_test.go
@@ -1,10 +1,10 @@
 package analyzer
 
 import (
+	"fmt"
 	"testing"
 
 	troubleshootv1beta2 "github.com/replicatedhq/troubleshoot/pkg/apis/troubleshoot/v1beta2"
-	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
@@ -860,6 +860,12 @@ func TestAnalyzeHostFilesystemPerformance(t *testing.T) {
 			hostAnalyzer: &troubleshootv1beta2.FilesystemPerformanceAnalyze{
 				CollectorName: "file system performance",
 				Outcomes: []*troubleshootv1beta2.Outcome{
+					{
+						Warn: &troubleshootv1beta2.SingleOutcome{
+							When:    "fileNotCollected",
+							Message: "Warn when the file was not collected",
+						},
+					},
 					{
 						Pass: &troubleshootv1beta2.SingleOutcome{
 							When:    "p99 < 10ms",
@@ -954,7 +960,136 @@ func TestAnalyzeHostFilesystemPerformance(t *testing.T) {
 				req.NoError(err)
 			}
 
-			assert.Equal(t, test.result, result)
+			req.Equal(test.result, result)
+		})
+	}
+}
+
+func TestAnalyzeHostFilesystemPerformanceNoFile(t *testing.T) {
+	tests := []struct {
+		name         string
+		hostAnalyzer *troubleshootv1beta2.FilesystemPerformanceAnalyze
+		result       []*AnalyzeResult
+		expectErr    bool
+	}{
+		{
+			name: "default behavior",
+			hostAnalyzer: &troubleshootv1beta2.FilesystemPerformanceAnalyze{
+				CollectorName: "irrel",
+				Outcomes: []*troubleshootv1beta2.Outcome{
+					{
+						Fail: &troubleshootv1beta2.SingleOutcome{
+							Message: "a nonexistent file should not be analyzed",
+						},
+					},
+				},
+			},
+			expectErr: true,
+		},
+		{
+			name: "fail case",
+			hostAnalyzer: &troubleshootv1beta2.FilesystemPerformanceAnalyze{
+				CollectorName: "irrel",
+				Outcomes: []*troubleshootv1beta2.Outcome{
+					{
+						Fail: &troubleshootv1beta2.SingleOutcome{
+							When:    "fileNotCollected",
+							Message: "the required file was not collected",
+						},
+					},
+				},
+			},
+			expectErr: false,
+			result: []*AnalyzeResult{
+				{
+					Title:   "Filesystem Performance",
+					IsFail:  true,
+					Message: "the required file was not collected",
+				},
+			},
+		},
+		{
+			name: "warn case",
+			hostAnalyzer: &troubleshootv1beta2.FilesystemPerformanceAnalyze{
+				CollectorName: "irrel",
+				Outcomes: []*troubleshootv1beta2.Outcome{
+					{
+						Warn: &troubleshootv1beta2.SingleOutcome{
+							When:    "fileNotCollected",
+							Message: "the required file was not collected",
+						},
+					},
+				},
+			},
+			expectErr: false,
+			result: []*AnalyzeResult{
+				{
+					Title:   "Filesystem Performance",
+					IsWarn:  true,
+					Message: "the required file was not collected",
+				},
+			},
+		},
+		{
+			name: "pass case",
+			hostAnalyzer: &troubleshootv1beta2.FilesystemPerformanceAnalyze{
+				CollectorName: "irrel",
+				Outcomes: []*troubleshootv1beta2.Outcome{
+					{
+						Pass: &troubleshootv1beta2.SingleOutcome{
+							When:    "fileNotCollected",
+							Message: "the required file was not collected",
+						},
+					},
+				},
+			},
+			expectErr: false,
+			result: []*AnalyzeResult{
+				{
+					Title:   "Filesystem Performance",
+					IsPass:  true,
+					Message: "the required file was not collected",
+				},
+			},
+		},
+		{
+			name: "fileNotCollected is only tested if it is the first result",
+			hostAnalyzer: &troubleshootv1beta2.FilesystemPerformanceAnalyze{
+				CollectorName: "irrel",
+				Outcomes: []*troubleshootv1beta2.Outcome{
+					{
+						Fail: &troubleshootv1beta2.SingleOutcome{
+							Message: "a nonexistent file should not be analyzed",
+						},
+					},
+					{
+						Pass: &troubleshootv1beta2.SingleOutcome{
+							When:    "fileNotCollected",
+							Message: "the required file was not collected",
+						},
+					},
+				},
+			},
+			expectErr: true,
+		},
+	}
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			req := require.New(t)
+
+			getCollectedFileContents := func(filename string) ([]byte, error) {
+				return nil, fmt.Errorf("file not found")
+			}
+
+			a := AnalyzeHostFilesystemPerformance{test.hostAnalyzer}
+			result, err := a.Analyze(getCollectedFileContents, nil)
+			if test.expectErr {
+				req.Error(err)
+			} else {
+				req.NoError(err)
+			}
+
+			req.Equal(test.result, result)
 		})
 	}
 }