Fix missing disruption data again. #27651
Conversation
Previous attempt did not work, as we were still bypassing FrameworkEventIntervals() which is needed to add the test summary, which eventually results in the AdditionalEvents json files in artifacts, which are then merged into the overall events and disruption data files. Without calling it, we still get no disruption data uploaded. Move the nil check into the ExpectNoDisruption function, and allow FrameworkEventIntervals to run as it should.
|
I am quite confused how the previous fix failed because I could see image registry disruption being gathered in the presubmits, i.e. https://gcsweb-ci.apps.ci.l2s4.p1.openshiftapps.com/gcs/origin-ci-test/pr-logs/pull/27636/pull-ci-openshift-origin-master-e2e-aws-ovn-upgrade/1612521262419021824/artifacts/e2e-aws-ovn-upgrade/openshift-e2e-test/artifacts/junit/AdditionalEvents__sig_imageregistry_Image_registry_remains_available_using_reused_connections_2023-01-09T21:59:13Z.json But none once it went live in similar jobs: https://gcsweb-ci.apps.ci.l2s4.p1.openshiftapps.com/gcs/origin-ci-test/logs/periodic-ci-openshift-release-master-ci-4.13-e2e-aws-ovn-upgrade/1613061514967650304/artifacts/e2e-aws-ovn-upgrade/openshift-e2e-test/artifacts/junit/ It must be a case where somehow we found a fallback on the presubmit to use, perhaps due to a different or missing FromRelease. Regardless I think this PR will correct it. |
openshift-ci
bot
added
the
approved
| func ExpectNoDisruptionForDuration(f *framework.Framework, testName string, allowedDisruption *time.Duration, total time.Duration, events monitorapi.Intervals, reason string) { | ||
| // This step records the test summaries data we need to result in AdditionalEvents json files in | ||
| // the openshift-e2e-test artifacts. | ||
| FrameworkEventIntervals(f, events) |
There was a problem hiding this comment.
I wonder if this line should be moved outside of the function. It can easily be missed. Your comment certainly helps.
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dgoodwin, xueqzhan The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
@dgoodwin: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
4 participants
Previous attempt did not work, as we were still bypassing
FrameworkEventIntervals() which is needed to add the test summary, which
eventually results in the AdditionalEvents json files in artifacts,
which are then merged into the overall events and disruption data files.
Without calling it, we still get no disruption data uploaded.
Move the nil check into the ExpectNoDisruption function, and allow
FrameworkEventIntervals to run as it should.
TRT-760