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

Migrate Kubeflow from k8s-prow #177

Closed
wants to merge 2 commits into from

Conversation

clarketm
Copy link
Contributor

@clarketm clarketm commented Dec 4, 2019

Copied kubeflow configuration from config and plugins and add tide and jobs.

@cjwagner
Copy link
Member

cjwagner commented Dec 4, 2019

Needed before this can merge:

Should be done soon (ideally before this merges as well):

This is all I can think of off the top of my head. Lets keep track of all the steps we take so we can leave a guide for transferring service of an org from one Prow instance to another.

@jlewi
Copy link

jlewi commented Dec 4, 2019

I invited google-oss-robot to the kubeflow org; once the invite is accepted the bot should have write access to Kubeflow repositories

  • Is admin permissions really needed? I'm giving the google-oss-robot the same permissions as k8s-ci-robot so unless something changed hopefully that's sufficient.

What's the URL to use for these webhooks?

Related to: kubernetes/test-infra#14343.

@clarketm
Copy link
Contributor Author

clarketm commented Dec 6, 2019

@cjwagner

Paradoxically, the jobs (more specifically checkconfig) needs the trigger plugin configured for kubeflow.

https://storage.googleapis.com/oss-prow/pr-logs/pull/GoogleCloudPlatform_oss-test-infra/93/pull-prow-config-validate/1201922395468730368/build-log.txt

@cjwagner
Copy link
Member

cjwagner commented Dec 6, 2019

Paradoxically, the jobs (more specifically checkconfig) needs the trigger plugin configured for kubeflow.

You can add the jobs in this PR as well or wait to enable Tide until the jobs are added (trigger > jobs > tide).

@fejta
Copy link
Contributor

fejta commented Dec 6, 2019

/hold

@jlewi
Copy link

jlewi commented Dec 10, 2019

Related to:

kubeflow/testing#475 - Kubeflow issue tracking migration to GoogleCloud Prow
kubernetes/test-infra#14343 - Migrate Kubeflow to GoogleCloudProw

@jlewi
Copy link

jlewi commented Dec 10, 2019

google-oss-robot is now a member of the kubeflow GitHub org.

/lgm

@fejta @clarketm what is blocking this PR? Does #93 need to be merged first?

@clarketm
Copy link
Contributor Author

clarketm commented Dec 11, 2019

@fejta @clarketm what is blocking this PR? Does #93 need to be merged first?

@jlewi - Apologies for the latency; I have been supporting the Istio 1.4.2 release which just went out today. I need to sync with @cjwagner to disambiguate his last comment.

You can add the jobs in this PR as well or wait to enable Tide until the jobs are added (trigger > jobs > tide).

@cjwagner - if I opt to go for the latter (i.e. wait to enable Tide) this: (trigger > jobs > tide) just means that I can omit the Tide-specific changes from this PR and reintroduce them in a PR after the jobs are merged (#93)?

Should these changes (trigger, tide, jobs) be atomic (i.e. one PR)? How can we prevent both duplicate and dropped webhooks in the transition.

@clarketm
Copy link
Contributor Author

@cjwagner
I disabled the tide configuration as requested. Is this now safe to merge?

@clarketm
Copy link
Contributor Author

clarketm commented Dec 13, 2019

@fejta / @cjwagner
I (should) have all the required PRs (I might additionally need something for testgrid?). Please verify and advise on the order and rollout strategy (either here or via this document). Additionally, I need to coordinate a time to do this with @jlewi next week.

Copy link
Contributor

@fejta fejta left a comment

Choose a reason for hiding this comment

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

/hold

@jlewi
Copy link

jlewi commented Dec 16, 2019

@clarketm is there anything blocking this PR? Are you just waiting on confirmation from me or the Kubeflow build copy (@richardsliu )?

My expectation is this PR won't actually cause any downtime so its safe to merge.
Downtime should also be fine just coordinate with @richardsliu

@googlebot
Copy link

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

@clarketm
Copy link
Contributor Author

clarketm commented Mar 5, 2020

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

Test name Commit Details Rerun command
pull-prow-config-validate 605582e link /test pull-prow-config-validate

This is a warning not an error? So I disabled it in checkconfig (#313).

@googlebot
Copy link

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

@google-oss-robot
Copy link
Collaborator

The following users are mentioned in OWNERS file(s) but are not members of the GoogleCloudPlatform org.

Once all users have been added as members of the org, you can trigger verification by writing /verify-owners in a comment.

  • abhi-g
    • prow/prowjobs/kubeflow/OWNERS
  • kunmingg
    • prow/prowjobs/kubeflow/OWNERS
  • lluunn
    • prow/prowjobs/kubeflow/OWNERS

@scottilee
Copy link

@googlebot I consent

@fejta
Copy link
Contributor

fejta commented Mar 10, 2020

I think someone from kubeflow should do this review

@scottilee
Copy link

@jlewi can you take a look?

@jlewi
Copy link

jlewi commented Mar 15, 2020

/assign @jlewi

@jlewi
Copy link

jlewi commented Mar 15, 2020

/lgtm
/approve

Thanks @clarketm . My hunch is that some of our more recent changes to our prow jobs may need to be ported over. I think we can do that after this PR is merged though.

When this PR is merged; will our tests be running simultaneously on the GCP prow instance as well as the K8s instance? Can we have some transition period during which we verify everything is working correctly on the new GCP instance before turning down the K8s instance?

@google-oss-robot
Copy link
Collaborator

@jlewi: changing LGTM is restricted to collaborators

In response to this:

/lgtm
/approve

Thanks @clarketm . My hunch is that some of our more recent changes to our prow jobs may need to be ported over. I think we can do that after this PR is merged though.

When this PR is merged; will our tests be running simultaneously on the GCP prow instance as well as the K8s instance? Can we have some transition period during which we verify everything is working correctly on the new GCP instance before turning down the K8s instance?

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.

@google-oss-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: clarketm, fejta, jlewi

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

@clarketm
Copy link
Contributor Author

Replaced by: kubernetes/test-infra#16898

@clarketm clarketm closed this Mar 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants