Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Sonobuoy Conformance Results #204
Sonobuoy Conformance Results #204
Changes from all commits
21b3ecb
7030dec
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
this code line is unreachable
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.
this test will have a qase automatic report? if yes, we need to add in here like other suits
Check failure on line 51 in pkg/testcase/cluster.go
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.
Can probably re-use
getResults
andparseResults
func hereThere 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.
Can probably re-use
cleanupTests
func hereThere 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.
Initially some months ago I specifically didn't want to change the existing mixedOS test functionality, but good call out now.
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.
every running we are waiting to have the need to re-run failed ones?
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.
It is not a long wait to do this and this automatically resolves any flakey tests except for the cilium tests which are known/expected conformance failures at this time. It makes more sense to have sonobuoy retry any tests that do fail which someone would have to do anyways in response to conformance tests failing. This way just ensures that whomever is responding to failed conformance tests can know they were already retried in case they were flakey.
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.
yeah sure , but cant we just simply on the
get results()
parse theres
check if there is failed ones there and if yes, then we call rerun ?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.
This essentially performs identically to what you're recommending although not in the same steps, instead of handling the strings emitted by the results sonobuoy itself already parses those results which would add code complexity. I'm voting we leave the logic in sonobuoy for this type of conformance test as it's likely more conclusive.
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.
sorry i am not understanding.....
what am suggesting is only re-run if actually find failed ones
another thing is for instance, you are returning on line 106 if it is cilium right? why are you waiting a function call
rerunFailedTests()
go inside it, just to check if it is cilium and if yes , return back?makes no much sense u get it? u should check before and if its cilium dont even go inside there
u are always calling re-run but in the real usage it will be only few times needed , i get the idea "oh but if there is a failure ( which if some frequence occur ) we are already taking care of that in the logic ( because u are always re-running, execpt for cilium)
so my suggestion is , your code should do what needs to do when needs to