Results aggregation should consider the errors directory #936
Conversation
| // which should have the nodes as subdirectories. It returns an item for each node processed and an error | ||
| // only if it couldn't open the original directory. Any errors while processing a specific node are logged | ||
| // but not returned. | ||
| func processNodesWithProcessor(p plugin.Interface, baseDir, dir string, processor postProcessor, selector fileSelector) ([]Item, error) { |
There was a problem hiding this comment.
This method is just code moved code, not new code. Extracted from the processPluginWithProcessor method.
pkg/client/results/processing.go
Outdated
| if err != nil { | ||
| return Item{}, err | ||
| logrus.Warningf("Error processing results entries for plugin %v: %v", p.GetName(), err) |
There was a problem hiding this comment.
TODO: rather than log here, I want to return a slice of errors and just return them all and log at the parent if we want. Awkward to have lower level methods not pass up their errors.
I really just wanted to continue processing after errors so on my first pass I added logging rather than returning.
Codecov Report
@@ Coverage Diff @@
## master #936 +/- ##
==========================================
+ Coverage 47.72% 47.87% +0.15%
==========================================
Files 76 76
Lines 5184 5226 +42
==========================================
+ Hits 2474 2502 +28
- Misses 2562 2570 +8
- Partials 148 154 +6
Continue to review full report at Codecov.
|
pkg/client/results/testdata/mockResults/plugins/ds-errors-02/ds-errors-02.golden.json
Outdated
Show resolved
Hide resolved
Whenever the aggregator itself deems that a plugin has failed, (e.g. pods didnt start, timeouts, etc) then it writes its results into an errors directory. This should be considered when doing results aggregation so that they can be resulted as failures. Otherwise, the plugin is just reported as having 0 results and having an unknown status even though Sonobuoy may have been the one to mark it as failure internally. Related to #883 Fixes #897 Signed-off-by: John Schnake <[email protected]>
|
Green tests, reasonable coverage. Going to just merge and move forward. Can fixup if desired but this is a blocker for the #883 which is a slightly larger change and I want ready for review soon. |
What this PR does / why we need it:
Whenever the aggregator itself deems that a plugin has failed,
(e.g. pods didnt start, timeouts, etc) then it writes its results
into an errors directory. This should be considered when doing
results aggregation so that they can be resulted as failures.
Otherwise, the plugin is just reported as having 0 results and
having an unknown status even though Sonobuoy may have been the one
to mark it as failure internally.
Which issue(s) this PR fixes
Related to #883
Fixes #897
Special notes for your reviewer:
Release note: