Skip to content

Commit c119a16

Browse files
murphybytesmurphybytes
authored
Fixed bugs introduced by handling multiple results in host preflights (#383)
Fixed bug caused by host preflights not handling empty when clauses, this cropped up because we now handle multiple host preflight results. Also expanded test coverage and added integration test script.
1 parent d730e6c commit c119a16

17 files changed

+250
-19
lines changed

‎.gitignore‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,3 +28,6 @@ vendor
2828

2929
dist
3030
try.sh
31+
32+
.vscode/
33+
workspace.*

‎examples/preflight/host/sample.yaml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -235,4 +235,5 @@ spec:
235235
when: "timezone == UTC"
236236
message: Timezone is UTC
237237
- fail:
238+
when: "timezone != UTC"
238239
message: Timezone is not UTC

‎examples/preflight/host/timezone.yaml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,4 +12,5 @@ spec:
1212
when: "timezone == UTC"
1313
message: Timezone is UTC
1414
- fail:
15+
when: "timezone != UTC"
1516
message: Timezone is not UTC

‎pkg/analyze/host_cpu.go‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -35,18 +35,19 @@ func (a *AnalyzeHostCPU) Analyze(getCollectedFileContents func(string) ([]byte,
3535
return nil, errors.Wrap(err, "failed to unmarshal cpu info")
3636
}
3737

38-
var coll resultCollector
38+
result := AnalyzeResult{
39+
Title: a.Title(),
40+
}
3941

4042
for _, outcome := range hostAnalyzer.Outcomes {
41-
result := &AnalyzeResult{Title: a.Title()}
4243

4344
if outcome.Fail != nil {
4445
if outcome.Fail.When == "" {
4546
result.IsFail = true
4647
result.Message = outcome.Fail.Message
4748
result.URI = outcome.Fail.URI
4849

49-
coll.push(result)
50+
return []*AnalyzeResult{&result}, nil
5051
}
5152

5253
isMatch, err := compareHostCPUConditionalToActual(outcome.Fail.When, cpuInfo.LogicalCount, cpuInfo.PhysicalCount)
@@ -59,15 +60,15 @@ func (a *AnalyzeHostCPU) Analyze(getCollectedFileContents func(string) ([]byte,
5960
result.Message = outcome.Fail.Message
6061
result.URI = outcome.Fail.URI
6162

62-
coll.push(result)
63+
return []*AnalyzeResult{&result}, nil
6364
}
6465
} else if outcome.Warn != nil {
6566
if outcome.Warn.When == "" {
6667
result.IsWarn = true
6768
result.Message = outcome.Warn.Message
6869
result.URI = outcome.Warn.URI
6970

70-
coll.push(result)
71+
return []*AnalyzeResult{&result}, nil
7172
}
7273

7374
isMatch, err := compareHostCPUConditionalToActual(outcome.Warn.When, cpuInfo.LogicalCount, cpuInfo.PhysicalCount)
@@ -80,15 +81,15 @@ func (a *AnalyzeHostCPU) Analyze(getCollectedFileContents func(string) ([]byte,
8081
result.Message = outcome.Warn.Message
8182
result.URI = outcome.Warn.URI
8283

83-
coll.push(result)
84+
return []*AnalyzeResult{&result}, nil
8485
}
8586
} else if outcome.Pass != nil {
8687
if outcome.Pass.When == "" {
8788
result.IsPass = true
8889
result.Message = outcome.Pass.Message
8990
result.URI = outcome.Pass.URI
9091

91-
coll.push(result)
92+
return []*AnalyzeResult{&result}, nil
9293
}
9394

9495
isMatch, err := compareHostCPUConditionalToActual(outcome.Pass.When, cpuInfo.LogicalCount, cpuInfo.PhysicalCount)
@@ -101,13 +102,12 @@ func (a *AnalyzeHostCPU) Analyze(getCollectedFileContents func(string) ([]byte,
101102
result.Message = outcome.Pass.Message
102103
result.URI = outcome.Pass.URI
103104

104-
coll.push(result)
105-
105+
return []*AnalyzeResult{&result}, nil
106106
}
107107
}
108108
}
109109

110-
return coll.get(a.Title()), nil
110+
return []*AnalyzeResult{&result}, nil
111111
}
112112

113113
func compareHostCPUConditionalToActual(conditional string, logicalCount int, physicalCount int) (res bool, err error) {

‎pkg/analyze/host_cpu_test.go‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,11 @@
11
package analyzer
22

33
import (
4+
"encoding/json"
45
"testing"
56

7+
troubleshootv1beta2 "github.com/replicatedhq/troubleshoot/pkg/apis/troubleshoot/v1beta2"
8+
"github.com/replicatedhq/troubleshoot/pkg/collect"
69
"github.com/stretchr/testify/assert"
710
"github.com/stretchr/testify/require"
811
)
@@ -148,3 +151,66 @@ func Test_compareHostCPUConditionalToActual(t *testing.T) {
148151
})
149152
}
150153
}
154+
155+
func TestHostCpuAnalyze(t *testing.T) {
156+
tt := []struct {
157+
name string
158+
cpuInfo collect.CPUInfo
159+
outcomes []*troubleshootv1beta2.Outcome
160+
results []*AnalyzeResult
161+
wantErr bool
162+
}{
163+
{
164+
name: "fix for passing test with empty when expr",
165+
cpuInfo: collect.CPUInfo{
166+
LogicalCount: 16,
167+
PhysicalCount: 8,
168+
},
169+
outcomes: []*troubleshootv1beta2.Outcome{
170+
{
171+
Fail: &troubleshootv1beta2.SingleOutcome{
172+
When: "logical < 8",
173+
Message: "oops",
174+
},
175+
},
176+
{
177+
Pass: &troubleshootv1beta2.SingleOutcome{
178+
When: "",
179+
Message: "it passed",
180+
},
181+
},
182+
},
183+
results: []*AnalyzeResult{
184+
{
185+
IsPass: true,
186+
Message: "it passed",
187+
Title: "Number of CPUs",
188+
},
189+
},
190+
},
191+
}
192+
193+
for _, tc := range tt {
194+
t.Run(tc.name, func(t *testing.T) {
195+
fn := func(_ string) ([]byte, error) {
196+
return json.Marshal(&tc.cpuInfo)
197+
}
198+
199+
analyzer := AnalyzeHostCPU{
200+
hostAnalyzer: &troubleshootv1beta2.CPUAnalyze{
201+
AnalyzeMeta: troubleshootv1beta2.AnalyzeMeta{
202+
CheckName: "Number of CPUs",
203+
},
204+
Outcomes: tc.outcomes,
205+
},
206+
}
207+
results, err := analyzer.Analyze(fn)
208+
if tc.wantErr {
209+
require.NotNil(t, err)
210+
return
211+
}
212+
require.Nil(t, err)
213+
require.Equal(t, tc.results, results)
214+
})
215+
}
216+
}

‎pkg/analyze/host_disk_usage.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ func (a *AnalyzeHostDiskUsage) Analyze(getCollectedFileContents func(string) ([]
5050
result.URI = outcome.Fail.URI
5151

5252
coll.push(result)
53+
continue
5354
}
5455

5556
isMatch, err := compareHostDiskUsageConditionalToActual(outcome.Fail.When, diskUsageInfo.TotalBytes, diskUsageInfo.UsedBytes)
@@ -71,6 +72,7 @@ func (a *AnalyzeHostDiskUsage) Analyze(getCollectedFileContents func(string) ([]
7172
result.URI = outcome.Warn.URI
7273

7374
coll.push(result)
75+
continue
7476
}
7577

7678
isMatch, err := compareHostDiskUsageConditionalToActual(outcome.Warn.When, diskUsageInfo.TotalBytes, diskUsageInfo.UsedBytes)
@@ -92,6 +94,7 @@ func (a *AnalyzeHostDiskUsage) Analyze(getCollectedFileContents func(string) ([]
9294
result.URI = outcome.Pass.URI
9395

9496
coll.push(result)
97+
continue
9598
}
9699

97100
isMatch, err := compareHostDiskUsageConditionalToActual(outcome.Pass.When, diskUsageInfo.TotalBytes, diskUsageInfo.UsedBytes)
@@ -106,6 +109,7 @@ func (a *AnalyzeHostDiskUsage) Analyze(getCollectedFileContents func(string) ([]
106109

107110
coll.push(result)
108111
}
112+
109113
}
110114
}
111115

‎pkg/analyze/host_disk_usage_test.go‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -362,6 +362,42 @@ func TestAnalyzeHostDiskUsage(t *testing.T) {
362362
},
363363
},
364364
},
365+
{
366+
name: "Pass with empty When and warning",
367+
diskUsageInfo: &collect.DiskUsageInfo{
368+
TotalBytes: 9 * 1024 * 1024 * 1024,
369+
UsedBytes: 1 * 1024 * 1024 * 1024,
370+
},
371+
hostAnalyzer: &troubleshootv1beta2.DiskUsageAnalyze{
372+
CollectorName: "ephemeral",
373+
Outcomes: []*troubleshootv1beta2.Outcome{
374+
{
375+
Warn: &troubleshootv1beta2.SingleOutcome{
376+
When: "available < 10Gi",
377+
Message: "/var/lib/kubelet less than 10Gi available",
378+
},
379+
},
380+
{
381+
Pass: &troubleshootv1beta2.SingleOutcome{
382+
When: "",
383+
Message: "/var/lib/kubelet passed",
384+
},
385+
},
386+
},
387+
},
388+
result: []*AnalyzeResult{
389+
{
390+
Title: "Disk Usage ephemeral",
391+
IsWarn: true,
392+
Message: "/var/lib/kubelet less than 10Gi available",
393+
},
394+
{
395+
Title: "Disk Usage ephemeral",
396+
IsPass: true,
397+
Message: "/var/lib/kubelet passed",
398+
},
399+
},
400+
},
365401
}
366402
for _, test := range tests {
367403
t.Run(test.name, func(t *testing.T) {

‎pkg/analyze/host_filesystem_performance.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ func (a *AnalyzeHostFilesystemPerformance) Analyze(getCollectedFileContents func
5757
result.URI = outcome.Fail.URI
5858

5959
coll.push(result)
60+
continue
6061
}
6162

6263
isMatch, err := compareHostFilesystemPerformanceConditionalToActual(outcome.Fail.When, fsPerf)
@@ -78,6 +79,7 @@ func (a *AnalyzeHostFilesystemPerformance) Analyze(getCollectedFileContents func
7879
result.URI = outcome.Warn.URI
7980

8081
coll.push(result)
82+
continue
8183
}
8284

8385
isMatch, err := compareHostFilesystemPerformanceConditionalToActual(outcome.Warn.When, fsPerf)
@@ -99,6 +101,7 @@ func (a *AnalyzeHostFilesystemPerformance) Analyze(getCollectedFileContents func
99101
result.URI = outcome.Pass.URI
100102

101103
coll.push(result)
104+
continue
102105
}
103106

104107
isMatch, err := compareHostFilesystemPerformanceConditionalToActual(outcome.Pass.When, fsPerf)
@@ -113,6 +116,7 @@ func (a *AnalyzeHostFilesystemPerformance) Analyze(getCollectedFileContents func
113116

114117
coll.push(result)
115118
}
119+
116120
}
117121
}
118122

‎pkg/analyze/host_http.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ func (a *AnalyzeHostHTTP) Analyze(getCollectedFileContents func(string) ([]byte,
5858
result.URI = outcome.Fail.URI
5959

6060
coll.push(result)
61+
continue
6162
}
6263

6364
isMatch, err := compareHostHTTPConditionalToActual(outcome.Fail.When, httpInfo)
@@ -79,6 +80,7 @@ func (a *AnalyzeHostHTTP) Analyze(getCollectedFileContents func(string) ([]byte,
7980
result.URI = outcome.Warn.URI
8081

8182
coll.push(result)
83+
continue
8284
}
8385

8486
isMatch, err := compareHostHTTPConditionalToActual(outcome.Warn.When, httpInfo)
@@ -100,6 +102,7 @@ func (a *AnalyzeHostHTTP) Analyze(getCollectedFileContents func(string) ([]byte,
100102
result.URI = outcome.Pass.URI
101103

102104
coll.push(result)
105+
continue
103106
}
104107

105108
isMatch, err := compareHostHTTPConditionalToActual(outcome.Pass.When, httpInfo)
@@ -114,6 +117,7 @@ func (a *AnalyzeHostHTTP) Analyze(getCollectedFileContents func(string) ([]byte,
114117

115118
coll.push(result)
116119
}
120+
117121
}
118122
}
119123

‎pkg/analyze/host_httploadbalancer.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ func (a *AnalyzeHostHTTPLoadBalancer) Analyze(getCollectedFileContents func(stri
5252
result.URI = outcome.Fail.URI
5353

5454
coll.push(result)
55+
continue
5556
}
5657

5758
if string(actual.Status) == outcome.Fail.When {
@@ -68,6 +69,7 @@ func (a *AnalyzeHostHTTPLoadBalancer) Analyze(getCollectedFileContents func(stri
6869
result.URI = outcome.Warn.URI
6970

7071
coll.push(result)
72+
continue
7173
}
7274

7375
if string(actual.Status) == outcome.Warn.When {
@@ -84,6 +86,7 @@ func (a *AnalyzeHostHTTPLoadBalancer) Analyze(getCollectedFileContents func(stri
8486
result.URI = outcome.Pass.URI
8587

8688
coll.push(result)
89+
continue
8790
}
8891

8992
if string(actual.Status) == outcome.Pass.When {
@@ -93,6 +96,7 @@ func (a *AnalyzeHostHTTPLoadBalancer) Analyze(getCollectedFileContents func(stri
9396

9497
coll.push(result)
9598
}
99+
96100
}
97101
}
98102

0 commit comments

Comments
 (0)