Increase timeout of integration tests #60458
Conversation
k8s-ci-robot
added
release-note
jennybuckley
force-pushed
the
integration-timeout
branch
from
0101ade to
9f4d2e3
Compare
jennybuckley
force-pushed
the
integration-timeout
branch
3 times, most recently
from
3f874d8 to
53f00f4
Compare
hack/jenkins/test-dockerized.sh
Outdated
| @@ -55,5 +55,10 @@ go install ./cmd/... | |||
| ./hack/install-etcd.sh | |||
|
|
|||
| make test-cmd | |||
|
|
|||
| export STORE_KUBE_TIMEOUT=${KUBE_TIMEOUT} | |||
There was a problem hiding this comment.
Use "${var_name}" in case the variable contains whitespaces.
|
Mostly lgtm. I'll leave it to @krzyzacy to apply the label. |
|
Thanks @jennybuckley |
jennybuckley
force-pushed
the
integration-timeout
branch
from
53f00f4 to
ab13325
Compare
hack/jenkins/test-dockerized.sh
Outdated
| @@ -55,5 +55,10 @@ go install ./cmd/... | |||
| ./hack/install-etcd.sh | |||
|
|
|||
| make test-cmd | |||
|
|
|||
| export STORE_KUBE_TIMEOUT="${KUBE_TIMEOUT}" | |||
| export KUBE_TIMEOUT='-timeout 600s' | |||
There was a problem hiding this comment.
This is already the default, isn't it?
https://github.com/kubernetes/kubernetes/blob/master/hack/make-rules/test-integration.sh#L34
I'm pretty sure noting else in here is using KUBE_TIMEOUT either so no reason to store it. Why don't you just remove line 47?
There was a problem hiding this comment.
"make test-integration" and "make test" are the only two places that use it. I guess that it would make sense to remove that line, but only if make test isn't called by any of the code in here
jennybuckley
force-pushed
the
integration-timeout
branch
from
ab13325 to
a1ab2b2
Compare
|
lgtm |
|
/retest |
|
ah, good catch |
k8s-ci-robot
added
the
lgtm
|
@krzyzacy it was @caesarxuchao in #59426 |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jennybuckley, krzyzacy, mikedanese, spiffxp 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 |
k8s-ci-robot
added
the
approved
|
/test all [submit-queue is verifying that this PR is safe to merge] |
|
Automatic merge from submit-queue (batch tested with PRs 60435, 60334, 60458, 59301, 60125). If you want to cherry-pick this change to another branch, please follow the instructions here. |
|
@jennybuckley: The following tests failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. 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. |
|
/retest |
|
Cherry picking as discussed in #60941 (comment) |
|
@jennybuckley could you ping the release branch manager as proper, for 1.8, 1.9 ping jpbetz, for 1.7 ping wojtek-t. |
jpbetz
added
cherry-pick-approved
What this PR does / why we need it:
In #24509 we intended to increase the timeout to 600s, but it was reverted by accident in #57521 when overriding of the value was enabled. The jenkins job should honor the default of 600s instead of continuing to override it to 300s.
This lead to flaky integration tests, specifically TestCRDDeletionCascading
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close the issue(s) when PR gets merged):Fixes #59426
Special notes for your reviewer:
Release note: