Skip to content

Make log test more resilient#1214

Merged
openshift-merge-robot merged 2 commits into
openshift:masterfrom
sebsoto:nodeLogsRetries
Aug 26, 2022
Merged

Make log test more resilient#1214
openshift-merge-robot merged 2 commits into
openshift:masterfrom
sebsoto:nodeLogsRetries

Conversation

@sebsoto
Copy link
Copy Markdown
Contributor

@sebsoto sebsoto commented Aug 25, 2022

Adds a retry for node log retrieval, and logs the command output upon
failure.

@sebsoto sebsoto changed the title Make log test more resiliant Make log test more resilient Aug 25, 2022
@sebsoto
Copy link
Copy Markdown
Contributor Author

sebsoto commented Aug 25, 2022

/approve cancel

@openshift-ci openshift-ci Bot added approved Indicates a PR has been approved by an approver from all required OWNERS files. and removed approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Aug 25, 2022
@jrvaldes
Copy link
Copy Markdown
Contributor

/lgtm

@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Aug 25, 2022
@mansikulkarni96
Copy link
Copy Markdown
Member

/lgtm

@sebsoto
Copy link
Copy Markdown
Contributor Author

sebsoto commented Aug 25, 2022

/cherry-pick release-4.11

@openshift-cherrypick-robot
Copy link
Copy Markdown

@sebsoto: once the present PR merges, I will cherry-pick it on top of release-4.11 in a new PR and assign it to you.

Details

In response to this:

/cherry-pick release-4.11

Instructions 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.

Comment thread test/e2e/logs_test.go
// Grab the optional logs for debugging purposes
for _, file := range optionalLogs {
_ = retrieveLog(node.Name, file, nodeDir)
_ = wait.PollImmediate(retry.Interval, retry.ResourceChangeTimeout, func() (bool, error) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we ignoring the error here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These logs are not guaranteed to be present. We still want to collect them if they exist

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a comment stating this. Grab the optional logs for debugging purposes does not explain why we are not looking at the error.

Copy link
Copy Markdown
Contributor

@aravindhp aravindhp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for being proactive and fixing this issue, @sebsoto

@openshift-ci openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 25, 2022
@openshift-ci-robot
Copy link
Copy Markdown

/retest-required

Remaining retests: 2 against base HEAD cc18c77 and 8 for PR HEAD 245848c in total

@openshift-ci openshift-ci Bot removed the lgtm Indicates that a PR is ready to be merged. label Aug 25, 2022
@sebsoto
Copy link
Copy Markdown
Contributor Author

sebsoto commented Aug 25, 2022

/retest

@sebsoto
Copy link
Copy Markdown
Contributor Author

sebsoto commented Aug 25, 2022

retest

@sebsoto
Copy link
Copy Markdown
Contributor Author

sebsoto commented Aug 25, 2022

/retest

Comment thread hack/run-ci-e2e-test.sh Outdated
Comment on lines +20 to +22
let retries+=1
echo "failed to get WMCO logs"
sleep 5
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to indent?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just replaced the tabs with spaces, didnt realize they were there.
This file is split between 4 and 2 spaces, but the majority of it is 2 space indenting, so chose 2 for this function.

Comment thread test/e2e/logs_test.go
// Grab the optional logs for debugging purposes
for _, file := range optionalLogs {
_ = retrieveLog(node.Name, file, nodeDir)
_ = wait.PollImmediate(retry.Interval, retry.ResourceChangeTimeout, func() (bool, error) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a comment stating this. Grab the optional logs for debugging purposes does not explain why we are not looking at the error.

@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Aug 25, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: aravindhp

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Adds a retry for node log retrieval to guard against network related
flakes, and logs the command output upon failure.
Retries the attempt but will not fail when collecting WMCO logs between
different test suites.
@jrvaldes
Copy link
Copy Markdown
Contributor

/lgtm

@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Aug 25, 2022
@sebsoto
Copy link
Copy Markdown
Contributor Author

sebsoto commented Aug 25, 2022

/retest

@sebsoto
Copy link
Copy Markdown
Contributor Author

sebsoto commented Aug 25, 2022

/test azure-e2e-operator

@openshift-ci-robot
Copy link
Copy Markdown

/retest-required

Remaining retests: 2 against base HEAD cc18c77 and 8 for PR HEAD 85a10ae in total

@sebsoto
Copy link
Copy Markdown
Contributor Author

sebsoto commented Aug 25, 2022

/retry

@openshift-ci-robot
Copy link
Copy Markdown

/retest-required

Remaining retests: 1 against base HEAD cc18c77 and 7 for PR HEAD 85a10ae in total

@sebsoto
Copy link
Copy Markdown
Contributor Author

sebsoto commented Aug 26, 2022

/retest

@openshift-ci-robot
Copy link
Copy Markdown

/retest-required

Remaining retests: 2 against base HEAD 4e6484f and 6 for PR HEAD 85a10ae in total

@openshift-ci-robot
Copy link
Copy Markdown

/retest-required

Remaining retests: 1 against base HEAD 4e6484f and 5 for PR HEAD 85a10ae in total

@openshift-ci-robot
Copy link
Copy Markdown

/retest-required

Remaining retests: 0 against base HEAD 4e6484f and 4 for PR HEAD 85a10ae in total

@sebsoto
Copy link
Copy Markdown
Contributor Author

sebsoto commented Aug 26, 2022

/retest-required

@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Aug 26, 2022

@sebsoto: all tests passed!

Full PR test history. Your PR dashboard.

Details

Instructions 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.

@openshift-cherrypick-robot
Copy link
Copy Markdown

@sebsoto: new pull request created: #1217

Details

In response to this:

/cherry-pick release-4.11

Instructions 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.

@mansikulkarni96
Copy link
Copy Markdown
Member

/cherry-pick release-4.10

@openshift-cherrypick-robot
Copy link
Copy Markdown

@mansikulkarni96: new pull request created: #1465

Details

In response to this:

/cherry-pick release-4.10

Instructions 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.

@sebsoto
Copy link
Copy Markdown
Contributor Author

sebsoto commented Mar 21, 2023

/cherry-pick release-4.9

@openshift-cherrypick-robot
Copy link
Copy Markdown

@sebsoto: new pull request created: #1508

Details

In response to this:

/cherry-pick release-4.9

Instructions 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants