Skip to content

Revert 1.25 again #27499

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
openshift-merge-robot merged 3 commits into from
Oct 28, 2022
Merged

Revert 1.25 again #27499

openshift-merge-robot merged 3 commits into from
Oct 28, 2022

Conversation

@stbenjam
Copy link
Member

@stbenjam stbenjam commented Oct 27, 2022 •
edited
Loading

This reverts #27491, #27490 and #27498 ; tracked by TRT-643

Per OpenShift policy, we are reverting this breaking change to get CI and/or nightly payloads flowing again. Note we had to revert 2 PR's, as #27498 is dependent on #27490.

We have noticed that tests are now taking 2x-4x longer to run sometimes, but this is causing random payload rejections. We are ending up in a situation where around 30% of runs end up exceeding the timeout for the test step. We do not know why. Spot checking tests during office hours, we noticed there are tests that typically take 5-6 seconds taking 1-2 minutes.

This PR was previously reverted, we tested the CI blocking jobs twice, and merged. Immediately after merging we saw this problem again:

image

To get this back in, we're going to unrevert this, and run some payload jobs on it to do further debugging.

@openshift-ci openshift-ci bot added e2e-images-update Related to images used by e2e tests vendor-update Touching vendor dir or related files labels Oct 27, 2022
@stbenjam
Copy link
Member Author

stbenjam commented Oct 27, 2022

/payload 4.12 nightly blocking

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Oct 27, 2022

@stbenjam: trigger 6 job(s) of type blocking for the nightly release of OCP 4.12

  • periodic-ci-openshift-release-master-nightly-4.12-e2e-aws-sdn-upgrade
  • periodic-ci-openshift-release-master-ci-4.12-e2e-azure-ovn-upgrade
  • periodic-ci-openshift-release-master-ci-4.12-upgrade-from-stable-4.11-e2e-gcp-ovn-upgrade
  • periodic-ci-openshift-release-master-nightly-4.12-e2e-aws-sdn-serial
  • periodic-ci-openshift-release-master-nightly-4.12-e2e-metal-ipi-ovn-ipv6
  • periodic-ci-openshift-release-master-nightly-4.12-e2e-metal-ipi-sdn-bm

See details on https://pr-payload-tests.ci.openshift.org/runs/ci/199ab2d0-5629-11ed-82ea-6b3cd6c180ec-0

@DennisPeriquet
Copy link
Contributor

DennisPeriquet commented Oct 27, 2022

/hold
/lgtm

@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 Oct 27, 2022
@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Oct 27, 2022
soltysh
soltysh approved these changes Oct 27, 2022
View reviewed changes
Copy link
Contributor

@soltysh soltysh left a comment

Choose a reason for hiding this comment

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

/approve

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 27, 2022
@kikisdeliveryservice
Copy link

kikisdeliveryservice commented Oct 27, 2022

/payload 4.12 ci blocking

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Oct 27, 2022

@kikisdeliveryservice: trigger 4 job(s) of type blocking for the ci release of OCP 4.12

  • periodic-ci-openshift-release-master-ci-4.12-e2e-aws-ovn-upgrade
  • periodic-ci-openshift-release-master-ci-4.12-upgrade-from-stable-4.11-e2e-aws-ovn-upgrade
  • periodic-ci-openshift-release-master-ci-4.12-upgrade-from-stable-4.11-e2e-azure-sdn-upgrade
  • periodic-ci-openshift-release-master-ci-4.12-e2e-aws-sdn-serial

See details on https://pr-payload-tests.ci.openshift.org/runs/ci/9397cb70-564e-11ed-8e04-43fd59826c90-0

@stbenjam
Copy link
Member Author

stbenjam commented Oct 28, 2022

In the runs that completed, there were no cases of the ~5 hour runs, going to remove the hold. Many of the aggregated pods got deleted as usual, I reported to test platform again.

/hold cancel
/retest-required

@openshift-ci openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 28, 2022
@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 28, 2022
@stbenjam
Revert "Merge pull request openshift#27491 from ingvagabund/mirror-sc…
6a8ff38 
…c-with-service-account"

This reverts commit 519b7eb, reversing
changes made to 86a216a.
@stbenjam
Revert "Merge pull request openshift#27498 from ingvagabund/merge-pab…
cf82ce5 
…los-PRs"

This reverts commit 86a216a, reversing
changes made to a9dc0d5.
@stbenjam
Revert "Merge pull request openshift#27490 from soltysh/rebase-1.25.0"
1ec0eb3 
This reverts commit de3cdf6, reversing
changes made to bd2b0b1.
@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Oct 28, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Oct 28, 2022

New changes are detected. LGTM label has been removed.

@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 28, 2022
@stbenjam
Copy link
Member Author

stbenjam commented Oct 28, 2022

I've had to revert #27491 as well... it merged while this was pending.

@ingvagabund Do you have anything else pending? Please don't merge anything else until we get this revert in, thanks

@stbenjam
Copy link
Member Author

stbenjam commented Oct 28, 2022

We're returning to a previously known state, so we feel this is low risk after discussing live. Going to override CI so we can get the revert in to allow branching.

/skip
/override ci/prow/e2e-agnostic-ovn-cmd
/override ci/prow/e2e-aws-csi
/override ci/prow/e2e-aws-image-registry
/override ci/prow/e2e-aws-ovn-cgroupsv2
/override ci/prow/e2e-aws-ovn-fips
/override ci/prow/e2e-aws-ovn-serial
/override ci/prow/e2e-aws-ovn-single-node
/override ci/prow/e2e-aws-ovn-single-node-serial
/override ci/prow/e2e-aws-ovn-single-node-upgrade
/override ci/prow/e2e-gcp-builds
/override ci/prow/e2e-gcp-csi
/override ci/prow/e2e-gcp-image-ecosystem
/override ci/prow/e2e-gcp-ovn
/override ci/prow/e2e-gcp-ovn-rt-upgrade
/override ci/prow/e2e-gcp-ovn-upgrade
/override ci/prow/e2e-metal-ipi-ovn-ipv6
/override ci/prow/e2e-metal-ipi-sdn
/override ci/prow/e2e-openstack-ovn

@bparees
Copy link
Contributor

bparees commented Oct 28, 2022

/lgtm

@bparees bparees added the lgtm Indicates that a PR is ready to be merged. label Oct 28, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Oct 28, 2022

@stbenjam: Overrode contexts on behalf of stbenjam: ci/prow/e2e-agnostic-ovn-cmd, ci/prow/e2e-aws-csi, ci/prow/e2e-aws-image-registry, ci/prow/e2e-aws-ovn-cgroupsv2, ci/prow/e2e-aws-ovn-fips, ci/prow/e2e-aws-ovn-serial, ci/prow/e2e-aws-ovn-single-node, ci/prow/e2e-aws-ovn-single-node-serial, ci/prow/e2e-aws-ovn-single-node-upgrade, ci/prow/e2e-gcp-builds, ci/prow/e2e-gcp-csi, ci/prow/e2e-gcp-image-ecosystem, ci/prow/e2e-gcp-ovn, ci/prow/e2e-gcp-ovn-rt-upgrade, ci/prow/e2e-gcp-ovn-upgrade, ci/prow/e2e-metal-ipi-ovn-ipv6, ci/prow/e2e-metal-ipi-sdn, ci/prow/e2e-openstack-ovn

Details

In response to this:

We're returning to a previously known state, so we feel this is low risk after discussing live. Going to override CI so we can get the revert in to allow branching.

/skip
/override ci/prow/e2e-agnostic-ovn-cmd
/override ci/prow/e2e-aws-csi
/override ci/prow/e2e-aws-image-registry
/override ci/prow/e2e-aws-ovn-cgroupsv2
/override ci/prow/e2e-aws-ovn-fips
/override ci/prow/e2e-aws-ovn-serial
/override ci/prow/e2e-aws-ovn-single-node
/override ci/prow/e2e-aws-ovn-single-node-serial
/override ci/prow/e2e-aws-ovn-single-node-upgrade
/override ci/prow/e2e-gcp-builds
/override ci/prow/e2e-gcp-csi
/override ci/prow/e2e-gcp-image-ecosystem
/override ci/prow/e2e-gcp-ovn
/override ci/prow/e2e-gcp-ovn-rt-upgrade
/override ci/prow/e2e-gcp-ovn-upgrade
/override ci/prow/e2e-metal-ipi-ovn-ipv6
/override ci/prow/e2e-metal-ipi-sdn
/override ci/prow/e2e-openstack-ovn

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.

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Oct 28, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bparees, DennisPeriquet, soltysh, stbenjam

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

@openshift-merge-robot openshift-merge-robot merged commit 0cfbdcd into openshift:master Oct 28, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Oct 28, 2022

@stbenjam: 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.

@stbenjam stbenjam mentioned this pull request Oct 28, 2022
Closed
@ingvagabund ingvagabund mentioned this pull request Oct 30, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@adambkaplan adambkaplan Awaiting requested review from adambkaplan

@ashishkamra ashishkamra Awaiting requested review from ashishkamra

1 more reviewer

@soltysh soltysh soltysh approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@bparees bparees

@DennisPeriquet DennisPeriquet

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. e2e-images-update Related to images used by e2e tests lgtm Indicates that a PR is ready to be merged. vendor-update Touching vendor dir or related files

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@stbenjam @DennisPeriquet @kikisdeliveryservice @bparees @soltysh @openshift-merge-robot