Skip to content

Commit 461cc99

Browse files
authored
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
1 parent 1bdf87e commit 461cc99

File tree

3 files changed

+170
-6
lines changed

3 files changed

+170
-6
lines changed

pkg/analyze/analyzer.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ import (
1818
"k8s.io/klog/v2"
1919
)
2020

21+
const FILE_NOT_COLLECTED = "fileNotCollected"
22+
2123
type AnalyzeResult struct {
2224
IsPass bool
2325
IsFail bool

pkg/analyze/host_filesystem_performance.go

Lines changed: 31 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -32,13 +32,40 @@ func (a *AnalyzeHostFilesystemPerformance) Analyze(
3232
) ([]*AnalyzeResult, error) {
3333
hostAnalyzer := a.hostAnalyzer
3434

35+
result := &AnalyzeResult{
36+
Title: a.Title(),
37+
}
38+
3539
collectorName := hostAnalyzer.CollectorName
3640
if collectorName == "" {
3741
collectorName = "filesystemPerformance"
3842
}
3943
name := filepath.Join("host-collectors/filesystemPerformance", collectorName+".json")
4044
contents, err := getCollectedFileContents(name)
4145
if err != nil {
46+
if len(hostAnalyzer.Outcomes) >= 1 {
47+
// if the very first outcome is FILE_NOT_COLLECTED', then use that outcome
48+
// otherwise, return the error
49+
if hostAnalyzer.Outcomes[0].Fail != nil && hostAnalyzer.Outcomes[0].Fail.When == FILE_NOT_COLLECTED {
50+
result.IsFail = true
51+
result.Message = renderFSPerfOutcome(hostAnalyzer.Outcomes[0].Fail.Message, collect.FSPerfResults{})
52+
result.URI = hostAnalyzer.Outcomes[0].Fail.URI
53+
return []*AnalyzeResult{result}, nil
54+
}
55+
if hostAnalyzer.Outcomes[0].Warn != nil && hostAnalyzer.Outcomes[0].Warn.When == FILE_NOT_COLLECTED {
56+
result.IsWarn = true
57+
result.Message = renderFSPerfOutcome(hostAnalyzer.Outcomes[0].Warn.Message, collect.FSPerfResults{})
58+
result.URI = hostAnalyzer.Outcomes[0].Warn.URI
59+
return []*AnalyzeResult{result}, nil
60+
}
61+
if hostAnalyzer.Outcomes[0].Pass != nil && hostAnalyzer.Outcomes[0].Pass.When == FILE_NOT_COLLECTED {
62+
result.IsPass = true
63+
result.Message = renderFSPerfOutcome(hostAnalyzer.Outcomes[0].Pass.Message, collect.FSPerfResults{})
64+
result.URI = hostAnalyzer.Outcomes[0].Pass.URI
65+
return []*AnalyzeResult{result}, nil
66+
}
67+
}
68+
4269
return nil, errors.Wrapf(err, "failed to get collected file %s", name)
4370
}
4471

@@ -58,10 +85,6 @@ func (a *AnalyzeHostFilesystemPerformance) Analyze(
5885
return nil, errors.Wrapf(err, "failed to unmarshal filesystem performance results from %s", name)
5986
}
6087

61-
result := &AnalyzeResult{
62-
Title: a.Title(),
63-
}
64-
6588
for _, outcome := range hostAnalyzer.Outcomes {
6689

6790
if outcome.Fail != nil {
@@ -135,6 +158,10 @@ func (a *AnalyzeHostFilesystemPerformance) Analyze(
135158
}
136159

137160
func compareHostFilesystemPerformanceConditionalToActual(conditional string, fsPerf collect.FSPerfResults) (res bool, err error) {
161+
if conditional == FILE_NOT_COLLECTED {
162+
return false, nil
163+
}
164+
138165
parts := strings.Split(conditional, " ")
139166
if len(parts) != 3 {
140167
return false, fmt.Errorf("conditional must have exactly 3 parts, got %d", len(parts))

pkg/analyze/host_filesystem_performance_test.go

Lines changed: 137 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
package analyzer
22

33
import (
4+
"fmt"
45
"testing"
56

67
troubleshootv1beta2 "github.com/replicatedhq/troubleshoot/pkg/apis/troubleshoot/v1beta2"
7-
"github.com/stretchr/testify/assert"
88
"github.com/stretchr/testify/require"
99
)
1010

@@ -860,6 +860,12 @@ func TestAnalyzeHostFilesystemPerformance(t *testing.T) {
860860
hostAnalyzer: &troubleshootv1beta2.FilesystemPerformanceAnalyze{
861861
CollectorName: "file system performance",
862862
Outcomes: []*troubleshootv1beta2.Outcome{
863+
{
864+
Warn: &troubleshootv1beta2.SingleOutcome{
865+
When: "fileNotCollected",
866+
Message: "Warn when the file was not collected",
867+
},
868+
},
863869
{
864870
Pass: &troubleshootv1beta2.SingleOutcome{
865871
When: "p99 < 10ms",
@@ -954,7 +960,136 @@ func TestAnalyzeHostFilesystemPerformance(t *testing.T) {
954960
req.NoError(err)
955961
}
956962

957-
assert.Equal(t, test.result, result)
963+
req.Equal(test.result, result)
964+
})
965+
}
966+
}
967+
968+
func TestAnalyzeHostFilesystemPerformanceNoFile(t *testing.T) {
969+
tests := []struct {
970+
name string
971+
hostAnalyzer *troubleshootv1beta2.FilesystemPerformanceAnalyze
972+
result []*AnalyzeResult
973+
expectErr bool
974+
}{
975+
{
976+
name: "default behavior",
977+
hostAnalyzer: &troubleshootv1beta2.FilesystemPerformanceAnalyze{
978+
CollectorName: "irrel",
979+
Outcomes: []*troubleshootv1beta2.Outcome{
980+
{
981+
Fail: &troubleshootv1beta2.SingleOutcome{
982+
Message: "a nonexistent file should not be analyzed",
983+
},
984+
},
985+
},
986+
},
987+
expectErr: true,
988+
},
989+
{
990+
name: "fail case",
991+
hostAnalyzer: &troubleshootv1beta2.FilesystemPerformanceAnalyze{
992+
CollectorName: "irrel",
993+
Outcomes: []*troubleshootv1beta2.Outcome{
994+
{
995+
Fail: &troubleshootv1beta2.SingleOutcome{
996+
When: "fileNotCollected",
997+
Message: "the required file was not collected",
998+
},
999+
},
1000+
},
1001+
},
1002+
expectErr: false,
1003+
result: []*AnalyzeResult{
1004+
{
1005+
Title: "Filesystem Performance",
1006+
IsFail: true,
1007+
Message: "the required file was not collected",
1008+
},
1009+
},
1010+
},
1011+
{
1012+
name: "warn case",
1013+
hostAnalyzer: &troubleshootv1beta2.FilesystemPerformanceAnalyze{
1014+
CollectorName: "irrel",
1015+
Outcomes: []*troubleshootv1beta2.Outcome{
1016+
{
1017+
Warn: &troubleshootv1beta2.SingleOutcome{
1018+
When: "fileNotCollected",
1019+
Message: "the required file was not collected",
1020+
},
1021+
},
1022+
},
1023+
},
1024+
expectErr: false,
1025+
result: []*AnalyzeResult{
1026+
{
1027+
Title: "Filesystem Performance",
1028+
IsWarn: true,
1029+
Message: "the required file was not collected",
1030+
},
1031+
},
1032+
},
1033+
{
1034+
name: "pass case",
1035+
hostAnalyzer: &troubleshootv1beta2.FilesystemPerformanceAnalyze{
1036+
CollectorName: "irrel",
1037+
Outcomes: []*troubleshootv1beta2.Outcome{
1038+
{
1039+
Pass: &troubleshootv1beta2.SingleOutcome{
1040+
When: "fileNotCollected",
1041+
Message: "the required file was not collected",
1042+
},
1043+
},
1044+
},
1045+
},
1046+
expectErr: false,
1047+
result: []*AnalyzeResult{
1048+
{
1049+
Title: "Filesystem Performance",
1050+
IsPass: true,
1051+
Message: "the required file was not collected",
1052+
},
1053+
},
1054+
},
1055+
{
1056+
name: "fileNotCollected is only tested if it is the first result",
1057+
hostAnalyzer: &troubleshootv1beta2.FilesystemPerformanceAnalyze{
1058+
CollectorName: "irrel",
1059+
Outcomes: []*troubleshootv1beta2.Outcome{
1060+
{
1061+
Fail: &troubleshootv1beta2.SingleOutcome{
1062+
Message: "a nonexistent file should not be analyzed",
1063+
},
1064+
},
1065+
{
1066+
Pass: &troubleshootv1beta2.SingleOutcome{
1067+
When: "fileNotCollected",
1068+
Message: "the required file was not collected",
1069+
},
1070+
},
1071+
},
1072+
},
1073+
expectErr: true,
1074+
},
1075+
}
1076+
for _, test := range tests {
1077+
t.Run(test.name, func(t *testing.T) {
1078+
req := require.New(t)
1079+
1080+
getCollectedFileContents := func(filename string) ([]byte, error) {
1081+
return nil, fmt.Errorf("file not found")
1082+
}
1083+
1084+
a := AnalyzeHostFilesystemPerformance{test.hostAnalyzer}
1085+
result, err := a.Analyze(getCollectedFileContents, nil)
1086+
if test.expectErr {
1087+
req.Error(err)
1088+
} else {
1089+
req.NoError(err)
1090+
}
1091+
1092+
req.Equal(test.result, result)
9581093
})
9591094
}
9601095
}

0 commit comments

Comments
 (0)