Skip to content

Add retry logic to e2e-test.sh #223

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Feb 21, 2019
Merged

Add retry logic to e2e-test.sh #223

merged 1 commit into from
Feb 21, 2019

Conversation

twiest
Copy link
Contributor

@twiest twiest commented Feb 14, 2019

No description provided.

@openshift-ci-robot openshift-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Feb 14, 2019
@twiest
Copy link
Contributor Author

twiest commented Feb 14, 2019

/test unit

@twiest twiest force-pushed the make_e2e_more_resilient branch from e443950 to 3baaa27 Compare February 14, 2019 20:57
@openshift-ci-robot openshift-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Feb 14, 2019
@twiest twiest force-pushed the make_e2e_more_resilient branch from 3baaa27 to 0827dc7 Compare February 14, 2019 21:00
@twiest
Copy link
Contributor Author

twiest commented Feb 14, 2019

/test e2e

1 similar comment
@twiest
Copy link
Contributor Author

twiest commented Feb 14, 2019

/test e2e

dgoodwin
dgoodwin reviewed Feb 15, 2019
View reviewed changes
hack/e2e-test.sh Outdated
INSTALLER_IMAGE="${INSTALLER_IMAGE}" \
OPENSHIFT_RELEASE_IMAGE="" \
TRY_INSTALL_ONCE="true" \
| oc apply -f - ; then
Copy link
Contributor

Choose a reason for hiding this comment

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

What can cause this to fail intermittently?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If the API server is down when the oc apply is run. For instance, if the certs are updated and the api server pods are rolled (they're talking about having really short cert periods for CI to test the rolling).

Or if above where CLUSTER_NAME is gathered, if that oc call fails (although now it'll retry and ultimately error out).

This is the actual line that my e2e was failing on and then @csrwng pointed out that we should add retries to the other oc commands as well.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds good thanks.

@dgoodwin
Copy link
Contributor

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Feb 15, 2019
@twiest
Copy link
Contributor Author

twiest commented Feb 15, 2019

/test e2e

2 similar comments
@twiest
Copy link
Contributor Author

twiest commented Feb 15, 2019

/test e2e

@twiest
Copy link
Contributor Author

twiest commented Feb 15, 2019

/test e2e

@twiest twiest force-pushed the make_e2e_more_resilient branch from 0827dc7 to adb91ae Compare February 18, 2019 16:31
@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Feb 18, 2019
@twiest twiest force-pushed the make_e2e_more_resilient branch 3 times, most recently from bc0cabf to eab4ff9 Compare February 18, 2019 17:05
@csrwng
Copy link
Contributor

csrwng commented Feb 18, 2019

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Feb 18, 2019
@twiest twiest force-pushed the make_e2e_more_resilient branch from eab4ff9 to 44856cb Compare February 18, 2019 19:01
@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Feb 18, 2019
@csrwng
Copy link
Contributor

csrwng commented Feb 18, 2019

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Feb 18, 2019
@csrwng
Copy link
Contributor

csrwng commented Feb 18, 2019

/test unit

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Feb 18, 2019
@twiest twiest force-pushed the make_e2e_more_resilient branch from c4f636b to 500de39 Compare February 18, 2019 20:47
@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Feb 18, 2019
@twiest twiest force-pushed the make_e2e_more_resilient branch from 500de39 to 261eada Compare February 18, 2019 20:52
@csrwng
Copy link
Contributor

csrwng commented Feb 18, 2019

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Feb 18, 2019
@csrwng
Copy link
Contributor

csrwng commented Feb 18, 2019

looks like the regular install never finished
/test e2e

@csrwng
Copy link
Contributor

csrwng commented Feb 19, 2019

/retest

1 similar comment
@csrwng
Copy link
Contributor

csrwng commented Feb 19, 2019

/retest

@csrwng
Copy link
Contributor

csrwng commented Feb 19, 2019

/test e2e

2 similar comments
@csrwng
Copy link
Contributor

csrwng commented Feb 19, 2019

/test e2e

@twiest
Copy link
Contributor Author

twiest commented Feb 19, 2019

/test e2e

@csrwng csrwng mentioned this pull request Feb 19, 2019
Closed
@wking wking mentioned this pull request Feb 19, 2019
Merged
@dgoodwin
Copy link
Contributor

/override e2e

@openshift-ci-robot
Copy link

@dgoodwin: dgoodwin unauthorized: /override is restricted to repo administrators

In response to this:

/override e2e

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.

@dgoodwin
Copy link
Contributor

/override e2e
/lgtm

@dgoodwin
Copy link
Contributor

/override ci/prow/e2e

@openshift-ci-robot
Copy link

@dgoodwin: Overrode contexts on behalf of dgoodwin: ci/prow/e2e

In response to this:

/override ci/prow/e2e

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.

@twiest twiest force-pushed the make_e2e_more_resilient branch from 261eada to 665dc5c Compare February 21, 2019 19:18
@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Feb 21, 2019
@csrwng
Copy link
Contributor

csrwng commented Feb 21, 2019

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Feb 21, 2019
@twiest
Copy link
Contributor Author

twiest commented Feb 21, 2019

/test unit

@twiest twiest force-pushed the make_e2e_more_resilient branch from 665dc5c to 3df8059 Compare February 21, 2019 19:37
@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Feb 21, 2019
@csrwng
Copy link
Contributor

csrwng commented Feb 21, 2019

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Feb 21, 2019
@openshift-merge-robot openshift-merge-robot merged commit 335c9aa into openshift:master Feb 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dgoodwin dgoodwin dgoodwin left review comments

@abutcher abutcher Awaiting requested review from abutcher

Assignees

@dgoodwin dgoodwin

@csrwng csrwng

Labels
lgtm Indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@twiest @dgoodwin @csrwng @openshift-ci-robot @openshift-merge-robot