Skip to content
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.

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

bump test e2e timeout to 100m #3039

Closed
wants to merge 1 commit into from
Closed

Conversation

mkenigs
Copy link
Contributor

@mkenigs mkenigs commented Mar 28, 2022

e2e-gcp-op has been failing on a lot of PRs, and while continuing to
increase the timeout long term is not the solution, it seems like
bumping the timeout for now is better than mashing retest until we fix
it

e2e-gcp-op has been failing on a lot of PRs, and while continuing to
increase the timeout long term is not the solution, it seems like
bumping the timeout for now is better than mashing retest until we fix
it
@openshift-ci openshift-ci bot requested review from jkyros and sinnykumari March 28, 2022 15:50
@kikisdeliveryservice
Copy link
Contributor

Why is the test timing out? Is this a functional issue or were other tests added? If the latter there might be a bug. When did this start?
Til further info:
/hold

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 28, 2022
@mkenigs
Copy link
Contributor Author

mkenigs commented Mar 28, 2022

Not sure. At least as early as #2976 (comment)

@kikisdeliveryservice
Copy link
Contributor

Not sure. At least as early as #2976 (comment)

I think we're more interested in when it first started - was it a PR or did something else change?

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Mar 28, 2022

@mkenigs: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/e2e-aws ba532ba link true /test e2e-aws

Full PR test history. Your PR dashboard.

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.

@cgwalters
Copy link
Member

I agree it'd be good to investigate, but I think merging this PR as is is better than the status quo of wasting CI resources on retesting until PRs eventually make it in.

@cgwalters
Copy link
Member

/approve
Just from my PoV, but not doing lgtm or lifting hold, that's team consensus.

This PR was 85.6 minutes for reference. But yeah, looking at e.g. this job from the release-4.10 branch the run as at 78 minutes.

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Mar 29, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cgwalters, mkenigs

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 29, 2022
@cgwalters
Copy link
Member

Just eyeballing it, some differences:

4.11: --- PASS: TestKubeletConfigMaxPods (650.27s)
4.10: --- PASS: TestKubeletConfigMaxPods (480.11s)

4.11: --- PASS: TestKernelArguments (489.49s)
4.10: --- PASS: TestKernelArguments (295.44s)

Computing how much variance there is here and digging into why is another task...

@kikisdeliveryservice
Copy link
Contributor

gcp-op is mainly green so it's not really blocking merges. We have before just extended test times and later on realized there was indeed a bug. Since this isn't blocking prs from merging (other jobs are doing that) would prefer to know when this started and root cause instead of just extending time.

Those times Colin points out above are very very not trivial. =/

@mkenigs mkenigs closed this May 10, 2022
cgwalters added a commit to yuqi-zhang/machine-config-operator that referenced this pull request May 17, 2022
This is a repeat of openshift#3039
yuqi-zhang pushed a commit to yuqi-zhang/machine-config-operator that referenced this pull request May 18, 2022
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. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants