Print pod logs for tests #19504
Print pod logs for tests #19504
Conversation
openshift-ci-robot
added
the
size/XS
|
PTAL @openshift/sig-networking |
openshift-ci-robot
added
sig/networking
size/S
| e2e.Failf("Error getting container logs: %s", logErr) | ||
| } | ||
|
|
||
| err = errors.New(err.Error() + logs) |
There was a problem hiding this comment.
hm... what does that end up looking like? (Don't you at least need a space or a newline between the original error and the logs?)
You could alternatively call e2e.Logf() to print the logs separately from returning the error...
There was a problem hiding this comment.
I think it might be useful to have the container log added to the error string.
as of #19130 we do |
imcsk8
force-pushed
the
multicast-pod-logs
branch
from
a842eeb to
9e1fc6b
Compare
| e2e.Failf("Error getting container logs: %s", logErr) | ||
| } | ||
|
|
||
| err = errors.New(err.Error() + "\nContainer Logs:\n" + logs) |
There was a problem hiding this comment.
This expects caller to log the error, why not log the podLogs here itself just like what we do in other places?
Prefer e2e.Failf() or e2e.LogF(), check https://github.com/openshift/origin/pull/19130/files for reference.
There was a problem hiding this comment.
This will log more details for test case:
[Area:Networking] multicast when using one of the plugins 'redhat/openshift-ovs-multitenant, redhat/openshift-ovs-networkpolicy' should allow multicast traffic in namespaces where it is enabled [Suite:openshift/conformance/parallel] 5m10s
Don't we need similar code for this below test case?
NetworkPolicy when using a plugin that implements NetworkPolicy should enforce policy based on PodSelector [Feature:OSNetworkPolicy] [Suite:openshift/conformance/parallel] 1m12s
There was a problem hiding this comment.
It appears to be addressed in #19180
There was a problem hiding this comment.
Don't we need similar code for this below test case?
as I said above, it's already there (it just postdates the rhbz bug)
There was a problem hiding this comment.
(It's also possible that the NetworkPolicy flake was fixed by #19130)
|
Note that the multicast flake that inspired this is #19218 and there is a suggestion there about fixing it |
imcsk8
force-pushed
the
multicast-pod-logs
branch
from
9e1fc6b to
128fb10
Compare
openshift-ci-robot
added
size/XS
imcsk8
force-pushed
the
multicast-pod-logs
branch
from
128fb10 to
bbee8fc
Compare
| } | ||
| e2e.Failf("Could not launch pod %s\nPod logs:\n%s", podName, logs) | ||
| } | ||
|
|
||
| return podIP, err |
There was a problem hiding this comment.
Since error is already handled in the above block, this return is same as return podIP, nil (e2e.Failf in Line176 panics with useful info)
Either remove returning error from launchTestMulticastPod method or change Line176 to e2e.LogF()
In order to help debugging problems print the pod log if the multicast test fails while launching the pod. Add error check for pod logs Fix framework reference Format error message Change pod logging to the general pattern Fix typo Change pod error logs
imcsk8
force-pushed
the
multicast-pod-logs
branch
from
bbee8fc to
481af8f
Compare
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: imcsk8, knobunc The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci-robot
added
the
approved
|
/retest |
1 similar comment
|
/retest |
In order to help debugging problems print the pod log if the multicast test fails while launching the pod.
NOTE: ¿Should we look for a generic solution for printing pod logs in tests?
Fixes partially: rhbz#1557487