kubeflow / manifests

A repository for Kustomize manifests
Apache License 2.0
832 stars 890 forks source link

Use GitHub Actions for presubmit checks #1498

Closed swiftdiaries closed 4 years ago

swiftdiaries commented 4 years ago

The current Prow tests are too bulky for the requirements for the repo. What do folks feel about a GitHub Action replacing it?

issue-label-bot[bot] commented 4 years ago

Issue-Label Bot is automatically applying the labels:

Label Probability
kind/feature 0.69

Please mark this comment with :thumbsup: or :thumbsdown: to give our bot feedback! Links: app homepage, dashboard and code for this bot.

issue-label-bot[bot] commented 4 years ago

Issue Label Bot is not confident enough to auto-label this issue. See dashboard for more details.

PatrickXYS commented 4 years ago

I would prefer keep consistent with all the repos.

Which means if we want to use Github action for kubeflow/manifests. And we might also want to use it for kubeflow/kfctl and other components.

So my suggqestion might be keep Github action and existing Presubmit-Check together, and when they failed, we can check Github Action Results.

But I think it's a breaking change when we migrate Presubmit-Checks to Github action. So we'd better get some feedbacks from kubeflow/testing folks.

/cc @jlewi @Jeffwan @Bobgy

jlewi commented 4 years ago

@swiftdiaries

The current Prow tests are too bulky for the requirements for the repo.

Could you please explain what you mean by too bulky?

Answering the question about what to do about the tests is probably blocked based on figuring out which working group will own this repository and how that will relate to the working group owning the individual applications.

swiftdiaries commented 4 years ago

Could you please explain what you mean by too bulky?

Yeah, sure thing. So I was going off of Katib's approach to testing.

My primary intention is to increase the number of levels at which we test manifests so that we can make changes with confidence.

Answering the question about what to do about the tests is probably blocked based on figuring out which working group will own this repository and how that will relate to the working group owning the individual applications.

I agree. We should identify clear OWNERS and a WG for the repo.

jlewi commented 4 years ago

Manifests has unittests. https://github.com/kubeflow/testing/tree/master/tests

And these run just fine under prow/tekton. If the WG that owns manifests prefers to use GitHub actions or Travis that sounds good to me.

I think we need to figure out which WG owns manifests before we make any changes to the infra so we know who the deciders are.

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in one week if no further activity occurs. Thank you for your contributions.

PatrickXYS commented 4 years ago

I think manifest will still use prow for testing.

/close

k8s-ci-robot commented 4 years ago

@PatrickXYS: Closing this issue.

In response to [this](https://github.com/kubeflow/manifests/issues/1498#issuecomment-732579638): >I think manifest will still use prow for testing. > >/close Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.