Skip to content

Adding myself to approve ISV-operators&OLM tests#24418

Closed
jianzhangbjz wants to merge 1 commit into
openshift:masterfrom
jianzhangbjz:test-owners
Closed

Adding myself to approve ISV-operators&OLM tests#24418
jianzhangbjz wants to merge 1 commit into
openshift:masterfrom
jianzhangbjz:test-owners

Conversation

@jianzhangbjz
Copy link
Copy Markdown
Contributor

Hi, All

I'm Jian, from the OpenShift QE team. I have been working on the Ginkgo automation test work for OLM, ServiceCatalog, and ISV operators. I need this permission to push the QE upstream automation work. Could you help give approval? Thanks!

@openshift-ci-robot openshift-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Jan 20, 2020
@openshift-ci-robot
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: jianzhangbjz
To complete the pull request process, please assign smarterclayton
You can assign the PR to them by writing /assign @smarterclayton in a comment when ready.

The full list of commands accepted by this bot can be found 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

@jianzhangbjz
Copy link
Copy Markdown
Contributor Author

@shawn-hurley Could you help give approval? Thanks very much! cc: @dangeoffroy

@jianzhangbjz
Copy link
Copy Markdown
Contributor Author

/assign @shawn-hurley

@openshift-ci-robot
Copy link
Copy Markdown

@jianzhangbjz: The following test failed, say /retest to rerun all failed tests:

Test name Commit Details Rerun command
ci/prow/e2e-gcp-upgrade 1d33ed1 link /test e2e-gcp-upgrade

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.

@sttts
Copy link
Copy Markdown
Contributor

sttts commented Jan 20, 2020

Why are OLM, ServiceCatalog, and ISV operator e2e tests supposed to go here, while we are trying to federate e2e tests to their respective repositories at the same time?

/hold

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 20, 2020
@deads2k
Copy link
Copy Markdown
Contributor

deads2k commented Jan 20, 2020

I need this permission to push the QE upstream automation work. Could you help give approval? Thanks!

What is the upstream that you're talking about? If it's focused on a few areas, perhaps you could get approver rights just to the new areas that you're creating and not the entire extended test package.

/hold

@jianzhangbjz
Copy link
Copy Markdown
Contributor Author

jianzhangbjz commented Jan 20, 2020

What is the upstream that you're talking about?

@deads2k Sorry for the confusion. The upstream for QE means this Origin repo.
As you know, in before, we do the BDD test in this cucushift repo. Now(actually, from June, 2019), we start to contribute to the BDD test case to this test/extended.

@jianzhangbjz
Copy link
Copy Markdown
Contributor Author

Why are OLM, ServiceCatalog, and ISV operator e2e tests supposed to go here...

@sttts As far as I know, all of the OpenShift components tests will be contribute to Origin soon. Contribute the BDD test to Origin is a part of QE work.
Now, I'm in charge of the OLM, ServiceCatalog, ISV operator testing work, our team does this kind of contribution work first. Other teams will do it later. Actually, our team has done a lot of automation work. We need this permission to push our automation work more effectively.

@shawn-hurley
Copy link
Copy Markdown

@sttts @deads2k

The new QE process is to add certain automated tests to openshift origin tests. Not all tests should go here though.

Stable automated  Critical & High importance functional, integration, installation, upgrading testing;
Stable automated Medium importance basic functional, integration, installation, upgrading testing;

@deads2k
Copy link
Copy Markdown
Contributor

deads2k commented Jan 20, 2020

@sttts As far as I know, all of the OpenShift components tests will be contribute to Origin soon. Contribute the BDD test to Origin is a part of QE work.
Now, I'm in charge of the OLM, ServiceCatalog, ISV operator testing work, our team does this kind of contribution work first. Other teams will do it later. Actually, our team has done a lot of automation work. We need this permission to push our automation work more effectively.

I recommend that you start by trying to find reviewers familiar with the areas you're trying to test to review your changes so that you can benefit from their experience working in this code base on a regular basis. I would also suggest that you start by trying to keep your changes relatively segregated.

Trying to first get approval powers on a large swath of the repo seems to be backwards. Especially if the long term maintenance burden of these tests will fall onto those other owners. Since QE isn't likely to bring in new levels of kubernetes, this is guaranteed to be the case.

@deads2k
Copy link
Copy Markdown
Contributor

deads2k commented Jan 20, 2020

@sttts @deads2k

The new QE process is to add certain automated tests to openshift origin tests. Not all tests should go here though.

@shawn-hurley Regardless of whether this is the landing spot for new tests (questionable in my opinion), The way to go about this is not to become an approver and start merging code, but rather to get the teams responsible for its maintenance to accept the change.

One of the reasons that this is highly questionable, is that of the components trying to add tests, not a single one is actually built from this repo. This repo is about building core kube components. If it were a separate, focused repo where OLM, ServiceCatalog, or ISV operator would pay the maintenance burden I'd be highly supportive of handing the approver powers to you. I'm happy to create and prime such a repo for you if you like.

Until then, I think approver rights should remain with the maintainers.

@jianzhangbjz
Copy link
Copy Markdown
Contributor Author

I recommend that you start by trying to find reviewers familiar with the areas you're trying to test to review your changes so that you can benefit from their experience working in this code base on a regular basis.

@deads2k Yes, totally agreed. By now, I have been working on the Ginkgo automation work for a long time. And, I think I'm familiar with these areas. Besides, I worked on Kubernetes before. You can see my PRs, Such as Added NVML support for GPU devices. I know this is nothing, I just want to say I'm familiar with Golang. I want to be one of the reviewers for these areas so that I can nudge the QE automation work progress. So, I submit this PR. Could you help give approval? Thanks!

I would also suggest that you start by trying to keep your changes relatively segregated.

Yes, actually, we're trying to create a new Test Suite for these tests. It won't block the core kube components test.

Especially if the long term maintenance burden of these tests will fall onto those other owners.

Yes, I think so. That's one of the reasons that I submit this PR. I'm happy to take this responsibility to maintain the QE test code.

@sttts
Copy link
Copy Markdown
Contributor

sttts commented Jan 21, 2020

Yes, actually, we're trying to create a new Test Suite for these tests. It won't block the core kube components test.

These tests must be ported to every new Kube version at the moment the core kube components are. Therefore every test here gets into the critical path of origin rebases and increases the risk to bring OpenShift to a new Kube version level.

Yes, I think so. That's one of the reasons that I submit this PR. I'm happy to take this responsibility to maintain the QE test code.

Will QE dedicate an engineer to the rebase efforts in every release to work hand-in-hand with the API&Auth team, fulltime if necessary to unblock the rebase?

I highly support the efforts to create another repository to migrate existing cross-component e2e tests to, and to add new ones. Having tests separate makes test constribution and maintenance (e.g. by QE and others) and the builds of core kube components so much easier, and let's every team to completely own their code without the need to wait for other teams for approvals. That's win-win for all teams.

I want to be one of the reviewers for these areas so that I can nudge the QE automation work progress. So, I submit this PR. Could you help give approval? Thanks!

There is a also a reviewer section in the OWNERs file. The approver list defines long-time contributors and therefore owners of the existing code base.

@jianzhangbjz
Copy link
Copy Markdown
Contributor Author

jianzhangbjz commented Jan 21, 2020

I highly support the efforts to create another repository to migrate existing cross-component e2e tests to, and to add new ones.

@sttts Thanks for your suggestions! I think so too. We can do it in other repo as long as they can be executed automatically as part of the nightly build e2e automation testing.

@openshift-bot
Copy link
Copy Markdown
Contributor

Issues go stale after 90d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle stale

@openshift-ci-robot openshift-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Apr 20, 2020
@openshift-bot
Copy link
Copy Markdown
Contributor

Stale issues rot after 30d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle rotten
/remove-lifecycle stale

@openshift-ci-robot openshift-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels May 20, 2020
@jianzhangbjz
Copy link
Copy Markdown
Contributor Author

Close it since we use https://github.com/openshift/openshift-tests repo now.

@jianzhangbjz
Copy link
Copy Markdown
Contributor Author

Address it in openshift/enhancements#183

@jianzhangbjz jianzhangbjz deleted the test-owners branch April 27, 2021 04:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants