Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Process all nodes and treat empty status as unknown #937

Merged
merged 1 commit into from
Oct 3, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 13 additions & 6 deletions pkg/client/results/processing.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,24 +97,31 @@ func aggregateStatus(items ...Item) string {
return StatusUnknown
}

unknownFound := false
failedFound, unknownFound := false, false
for i := range items {
// Branches should just aggregate their leaves and return the result.
if len(items[i].Items) > 0 {
items[i].Status = aggregateStatus(items[i].Items...)
}

// Any failures immediately fail the parent.
if items[i].Status == StatusFailed {
return StatusFailed
// Empty status should be updated to unknown.
if items[i].Status == "" {
items[i].Status = StatusUnknown
}

if items[i].Status == StatusUnknown {
switch items[i].Status {
case StatusFailed:
failedFound = true
case StatusUnknown:
unknownFound = true
default:
}
}

if unknownFound {
// Only return once all processing is completed; otherwise other leaves don't get resolved.
if failedFound {
return StatusFailed
} else if unknownFound {
return StatusUnknown
}

Expand Down
69 changes: 69 additions & 0 deletions pkg/client/results/processing_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -420,6 +420,75 @@ func TestAggregateStatus(t *testing.T) {
},
},
expected: StatusUnknown,
}, {
desc: "Leaf nodes with empty status get changed to unknown",
input: []Item{
{
Name: "unknown node",
Items: []Item{
{
Name: "first leaf no status",
},
},
},
},
expectedItems: []Item{
{
Name: "unknown node",
Status: StatusUnknown,
Items: []Item{
{
Name: "first leaf no status",
Status: StatusUnknown,
},
},
},
},
expected: StatusUnknown,
}, {
desc: "Processes all nodes even after seeing first failure",
input: []Item{
{
Name: "DS plugin",
Items: []Item{
{
Name: "node1",
Items: []Item{
{Name: "foo", Status: StatusFailed},
},
},
{
Name: "node2",
Items: []Item{
{Name: "foo", Status: StatusFailed},
},
},
},
},
},
expectedItems: []Item{
{
Name: "DS plugin",
Status: StatusFailed,
Items: []Item{
{
Name: "node1",
Status: StatusFailed,
Items: []Item{
{Name: "foo", Status: StatusFailed},
},
},
{
Name: "node2",
Status: StatusFailed,
Items: []Item{
{Name: "foo", Status: StatusFailed},
},
},
},
},
},
expected: StatusFailed,
},
}

Expand Down