-
Notifications
You must be signed in to change notification settings - Fork 344
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
Add a new results format: manual #1090
Conversation
pkg/client/results/manual.go
Outdated
resultObj.Metadata, | ||
) | ||
if err != nil { | ||
return resultObj, errors.Wrap(err, "error processing junit") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not junit; just left over from copy. Update this.
pkg/client/results/processing.go
Outdated
// scale linearly with the number of nodes and may become unreasonable to show the user. | ||
// | ||
// If there is only one top level item, its status is returned. Otherwise a human readable string | ||
// is produced to show the counts of various values. E.g. "3 passed/2 failed/1 custom msg". Avoiding |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@zubron What do you think about this solution?
If we do something like this then we can summarize any custom status the user puts on the items. It seems like the only other alternative is to force them into a very narrow set of allowed values (passed/failed/skipped) which seemed to me to miss the point of allowing completely custom results from the plugin.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is a reasonable approach. You make a good point about the narrow set of allowed values, I think it will be preferable to allow custom statuses there 👍
@@ -130,6 +132,49 @@ func (i *Item) GetSubTreeByName(root string) *Item { | |||
return nil | |||
} | |||
|
|||
// manualResultsAggregation is custom logic just for aggregating results for the top level summary |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will need some good tests for this; I have some but I think it needs changes/tests to cover the daemonset case.
Not only will the top-summary need its status, but the objects right below the top level summary are the 'node summary' so we need to make sure that we arent leaving one layer blank of status values.
I've added tests to this and made some changes to the aggregation of results and which files Sonobuoy will look for during results processing. Now, Sonoboy will only process files explicitly listed in the plugin results files, or any file named For the overall status aggregation, how the results are aggregated depends on whether the plugin is a Job or DaemonSet plugin. For Job plugins, if only one results file is processed, the status in that file is used as the overall status for plugin. If multiple files are processed, then the statuses are grouped by count, and the overall status for the plugin is a human readable form of that grouping, e.g. For DaemonSet plugins, each node's status is calculated as for a Job plugin to handle the case where a DS plugin has multiple result files. However, the overall plugin status is determined based on each item from each node rather than the overall status from each node. This avoids already aggregated statuses being used within the overall aggregate status. If the same status is used for all files for all nodes then that will be the overall status as in the Job case.
This will result in the following statuses:
The documentation will need to be updated to include a guide on how to write custom results but this will be done in a follow up PR. |
Manual results allow a user to write the sonobuoy-results.yaml file largly on their own. Due to some limitations we still wrap their file with one more summary level item. When the user gave us a single top-level item, the status from that item is used for the summary. Otherwise all the top level file items are considered and a non-judegmental routine just counts the various status values and shows them as a human readable status for the top level. This helps ensure that the status value from custom output can be various pieces of info the user may want to provide and not just something like 'pass' or 'fail'. Signed-off-by: Bridget McErlean <bmcerlean@vmware.com>
What this PR does / why we need it:
Manual results allow a user to write the sonobuoy_results.yaml
file largely on their own.
Due to some limitations we still wrap their file with one more
summary level item.
When the user gave us a single top-level item, the status from that
item is used for the summary. Otherwise all the top level items
are considered and a non-judgemental routine just counts the various
status values and shows them as a human readable status for the
top level. This helps ensure that the status value from custom output
can be various pieces of info the user may want to provide and not just
something like 'pass' or 'fail'.
Release note: