Skip to content

Bug 1784334: Revert "check ServiceCatalog basic usages in OCP 4.x"#24315

Closed
shawn-hurley wants to merge 1 commit intomasterfrom
revert-23352-service-catalog
Closed

Bug 1784334: Revert "check ServiceCatalog basic usages in OCP 4.x"#24315
shawn-hurley wants to merge 1 commit intomasterfrom
revert-23352-service-catalog

Conversation

@shawn-hurley
Copy link
Copy Markdown

Reverts #23352

/cc @smarterclayton @jianzhangbjz @jmrodri

this test is causing consistent CI flakes and we should remove it.

@openshift-ci-robot openshift-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Dec 17, 2019
@shawn-hurley
Copy link
Copy Markdown
Author

Disabling because:
Service Catalog will throw alerts once it is installed.
Other tests will fail on those alerts.

this will cause flakes in 4.3 and 4.4 tests we should move these to serial tests IIUC

@jmrodri
Copy link
Copy Markdown
Contributor

jmrodri commented Dec 17, 2019

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Dec 17, 2019
@openshift-bot
Copy link
Copy Markdown
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

3 similar comments
@openshift-bot
Copy link
Copy Markdown
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Copy Markdown
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Copy Markdown
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@jianzhangbjz
Copy link
Copy Markdown
Contributor

@shawn-hurley Glad to know it. Thanks! I guess Service Catalog throw alerts only for 4.4. I was wondering if we can cherry-pick these automation tests to 4.1, 4.2, 4.3. And, in the future, can we contribute Service Catalog automation test cases to the 4.1, 4.2, 4.3 directly? No more submit PRs to the master branch.
One more question, will Service Catalog be removed from the 4.4?

@jianzhangbjz
Copy link
Copy Markdown
Contributor

/lgtm

@openshift-bot
Copy link
Copy Markdown
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@jmrodri
Copy link
Copy Markdown
Contributor

jmrodri commented Dec 18, 2019

@jianzhangbjz my understanding is you should be able to submit a PR directly to the 4.3, 4.2, & 4.1 branches directly. The tests in 4.4 should actually be looking for service catalog to be removed. The 4.3 service catalog operator does create a prometheusrule and alerts.

Copy link
Copy Markdown
Contributor

@jmrodri jmrodri left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci-robot
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jianzhangbjz, jmrodri, shawn-hurley

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-bot
Copy link
Copy Markdown
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

7 similar comments
@openshift-bot
Copy link
Copy Markdown
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Copy Markdown
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Copy Markdown
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Copy Markdown
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Copy Markdown
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Copy Markdown
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Copy Markdown
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Copy Markdown
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@jianzhangbjz
Copy link
Copy Markdown
Contributor

my understanding is you should be able to submit a PR directly to the 4.3, 4.2, & 4.1 branches directly.

@jmrodri Thanks! But, I was told the workflow should be we submit the PR to the master branch, and then, the bot can cherry-pick it to the old versions. @bparees Can I submit the PR to the old version branch directly? Thanks!

@openshift-bot
Copy link
Copy Markdown
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

3 similar comments
@openshift-bot
Copy link
Copy Markdown
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Copy Markdown
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Copy Markdown
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-ci-robot
Copy link
Copy Markdown

@shawn-hurley: The following test failed, say /retest to rerun them all:

Test name Commit Details Rerun command
ci/prow/e2e-cmd 0643a28 link /test e2e-cmd

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.

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-ci-robot openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 18, 2019
@openshift-ci-robot
Copy link
Copy Markdown

@shawn-hurley: PR needs rebase.

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.

@wking
Copy link
Copy Markdown
Member

wking commented Dec 18, 2019

Needs a rebase, @shawn-hurley

@wking
Copy link
Copy Markdown
Member

wking commented Dec 18, 2019

/retitle Bug 1784837: Revert "check ServiceCatalog basic usages in OCP 4.x"

@openshift-ci-robot openshift-ci-robot changed the title Revert "check ServiceCatalog basic usages in OCP 4.x" Bug 1784837: Revert "check ServiceCatalog basic usages in OCP 4.x" Dec 18, 2019
@openshift-ci-robot
Copy link
Copy Markdown

@shawn-hurley: This pull request references Bugzilla bug 1784837, which is invalid:

  • expected the bug to target the "4.4.0" release, but it targets "---" instead

Comment /bugzilla refresh to re-evaluate validity if changes to the Bugzilla bug are made, or edit the title of this pull request to link to a different bug.

Details

In response to this:

Bug 1784837: Revert "check ServiceCatalog basic usages in OCP 4.x"

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-robot openshift-ci-robot added the bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. label Dec 18, 2019
@wking
Copy link
Copy Markdown
Member

wking commented Dec 18, 2019

/bugzilla refresh

@openshift-ci-robot openshift-ci-robot added the bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. label Dec 18, 2019
@openshift-ci-robot
Copy link
Copy Markdown

@wking: This pull request references Bugzilla bug 1784837, which is valid. The bug has been updated to refer to the pull request using the external bug tracker.

Details

In response to this:

/bugzilla refresh

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-robot openshift-ci-robot removed the bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. label Dec 18, 2019
@wking
Copy link
Copy Markdown
Member

wking commented Dec 18, 2019

/retitle Bug 1784334: Revert "check ServiceCatalog basic usages in OCP 4.x"

@openshift-ci-robot openshift-ci-robot changed the title Bug 1784837: Revert "check ServiceCatalog basic usages in OCP 4.x" Bug 1784334: Revert "check ServiceCatalog basic usages in OCP 4.x" Dec 18, 2019
@openshift-ci-robot
Copy link
Copy Markdown

@shawn-hurley: This pull request references Bugzilla bug 1784334, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker.

Details

In response to this:

Bug 1784334: Revert "check ServiceCatalog basic usages in OCP 4.x"

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.

@jmrodri
Copy link
Copy Markdown
Contributor

jmrodri commented Dec 18, 2019

Please close this PR in favor of PR #24323

@bparees
Copy link
Copy Markdown
Contributor

bparees commented Dec 18, 2019

@jmrodri Thanks! But, I was told the workflow should be we submit the PR to the master branch, and then, the bot can cherry-pick it to the old versions. @bparees Can I submit the PR to the old version branch directly? Thanks!

you can do it either way, but you must have merged to each prior branch before it can be merged to the next. So you need to merge into master before your PR against release-4.3 will be merged (you can open it, but it will not be merged). And then release-4.3 must merge before release-4.3, etc.

Also before you can merge to release-4.3 you need a BZ against 4.4 that is in "modified".
before you can merge to release-4.2 you need a BZ against 4.3.0 that is in "Verified"
before you can merge to release-4.1 you need a BZ against 4.2.z that is in "verified".

The patch manager+group leads will enforce those rules for release-4.3/release-4.2/release-4.1.

@jianzhangbjz
Copy link
Copy Markdown
Contributor

@bparees Thanks! Must we create a bug for the automation PR? It's a little bit of unreasonable. Do we consider a new workflow for automation?

Service Catalog will throw alerts once it is installed.
Other tests will fail on those alerts.

@shawn-hurley Service Catalog will throw alerts in 4.4 as we designed. However, these alerts lead to other tests to fail. Actually, in my opinion, the customers will encounter this scenario too. I think we should find out the root causes, not remove Service Catalog tests simply. Or am I missing something?

@bparees
Copy link
Copy Markdown
Contributor

bparees commented Dec 19, 2019

@bparees Thanks! Must we create a bug for the automation PR? It's a little bit of unreasonable. Do we consider a new workflow for automation?

we require bugs for any merge into a non-master branch. you can ask the patch-manager to override the requirement, but it's generally going to be faster to just create the bugs and follow the process than to argue for an exception.

And this should not be a normal thing (needing to merge automation/tests into non-master branches). particularly once we put all the missing automation in place.

@jianzhangbjz
Copy link
Copy Markdown
Contributor

@bparees Many thanks for your explanation! Yeah, I see.

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. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. lgtm Indicates that a PR is ready to be merged. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants