Skip to content
This repository has been archived by the owner on Aug 17, 2023. It is now read-only.

Make kubeflow/kfctl source of truth for kfctl development; turn down kfctl in kubeflow/kubeflow #7

Closed
jlewi opened this issue Aug 14, 2019 · 22 comments

Comments

@jlewi
Copy link
Contributor

jlewi commented Aug 14, 2019

Opening this issue to track migrating development of kfctl from kubeflow/kubeflow to kubeflow/kfctl

@kkasravi What are the next steps here? Are we waiting on E2E tests to be migrated to kubeflow/kfctl?

/assign @kkasravi

@jlewi jlewi added this to To Do in 0.7.0 via automation Aug 14, 2019
@jlewi jlewi added this to To do in kfctl-1.0 via automation Aug 14, 2019
@kkasravi
Copy link
Contributor

Yes, I need to add the tests and fix the error you noted where we are actually running e2e on Kubeflow/kubeflow

@jlewi jlewi moved this from To do to sprint backlog in kfctl-1.0 Aug 26, 2019
@jlewi
Copy link
Contributor Author

jlewi commented Aug 26, 2019

@kkasravi Any update on this? When do you think we should migrate development over from kubeflow/kubeflow to kubeflow/kfctl?

It would be great to do this well in advance of trying to cut 0.7.

@jlewi
Copy link
Contributor Author

jlewi commented Sep 4, 2019

@kkasravi any update?

@kkasravi
Copy link
Contributor

kkasravi commented Sep 4, 2019

@jlewi

There is are several pending PRs that we should merge

#17 I'm working on - should be ready today sometime
#11 is dependent on #17
#16 can be merged

after the above PRs get merged we should switch over.

@jlewi
Copy link
Contributor Author

jlewi commented Sep 4, 2019

Fantastic. Once we start switching over to kubeflow/kfctl; I think we'll need to update the kubeflow/manifests tests?

@kkasravi
Copy link
Contributor

kkasravi commented Sep 4, 2019

@jlewi yep - we'll need to

  • remove kfctl tests in kubeflow/kubeflow that are redundant.
  • update manifests/tests/workflows/

@swiftdiaries
Copy link
Member

/cc @swiftdiaries

@jlewi
Copy link
Contributor Author

jlewi commented Sep 9, 2019

@kkasravi It looks like #16 and #17 have been merged. Does this mean we are ready to start moving development to kubeflow/kfctl?

@kkasravi
Copy link
Contributor

kkasravi commented Sep 9, 2019

Yes.

I’ve also moved config over to manifests.
We should probably switch over to that in the same timeframe. I’ll ping you on that PR later today.

@jlewi
Copy link
Contributor Author

jlewi commented Sep 10, 2019

@kkasravi Do you want to send a note to kubeflow-discuss so we can get a heads up to all folks working on kfctl who might have changes in flight? Do we also want to try to identify any PRs in flight that will be impacted?

@jlewi
Copy link
Contributor Author

jlewi commented Sep 10, 2019

/cc @swiftdiaries @gabrielwen

@kkasravi
Copy link
Contributor

@jlewi
Copy link
Contributor Author

jlewi commented Sep 12, 2019

Thanks @kkasravi so can you provide some detail here about the plan

  1. When are we going to switch to make kubeflow/kfctl the source of truth?
  2. Are we planning on deleting kubeflow/kubeflow/bootstrap to try to make it clear to folks that source of truth is now kubeflow/kfctl
  3. Is there going to be some sort of transition period during which PRs could be submitted to both repositories?
    • Could we generate patches from kubeflow/kubeflow and then apply them in kubeflow/kfctl?

A cursory glance shows a number of open PRs still on kubeflow/kubeflow

kubeflow/kubeflow#4110 fix kfctl init
kubeflow/kubeflow#4106 reconcile
kubeflow/kubeflow#4092 fix readme
....

Is there a way to identify all PRs that are modifying files in that directory?

@jlewi
Copy link
Contributor Author

jlewi commented Sep 16, 2019

Here's a recipe for patching commits to kubeflow/kubeflow into kfctl

  1. On kubeflow/kubeflow generate a patch for the specified commit e.g

    git format-patch -1 ${COMMIT-SHA}
    
    • This will output a patch file
  2. Now apply the patch to kubeflow/kfctl

    git apply --reject -p2 ${PATCH_FILE}
    
    • The reject option means git will skip any patches that can't be applied cleanly

      • This will probably happen for changes to imports
      • Reject files will be spit out
  3. Manually fix any remaining merges and then open a PR.

@jlewi
Copy link
Contributor Author

jlewi commented Sep 16, 2019

Here's a list of PRs with kfctl in the name

hub pr list | grep kfctl
   #4115  Support Kubeflow upgrades in kfctl (pt 1)   cla: yes   do-not-merge/hold   size/XL 
   #4092  update kfctl readme   cla: yes   needs-ok-to-test   size/XS 
   #4040  adds apply -f to kfctl   cla: yes   size/L 
   #4017  sync kfctl_gcp_iap.yaml with v0.6.2 config   cla: yes   lgtm   size/M 
   #3949  builds, pushes kfctl to GCS as part of pre and post submits   cla: yes   size/M 

@jlewi
Copy link
Contributor Author

jlewi commented Sep 16, 2019

Here's a proposal (from @abhi-g )

  • Until we are ready to cut the v0.7-branch and RC for kfctl, kubeflow/kubeflow remains the source of truth
  • When we are ready to cut the v0.7-branch
    • Perform one last sync of kubeflow/kubeflow to kubeflow/kfctl
    • Cut the v0.7-branch and build the RC from kubeflow/kfctl
  • At this point kubeflow/kfctl is the source of truth
  • Any PRs that end up being committed to kubeflow/kubeflow after that can be ported over as one offs using the git recipe I pasted above

@yanniszark @swiftdiaries @kkasravi @gabrielwen @richardsliu WDYT? Can you thumbs up this comment if this sounds good to you?

@jlewi
Copy link
Contributor Author

jlewi commented Oct 24, 2019

Downgrading to P1 because we will not block 0.7.0 on this.

@jlewi
Copy link
Contributor Author

jlewi commented Nov 6, 2019

Bump to P0. We have moved development over to kubeflow/kfctl so we need to finish getting things moved over as quickly as possible to avoid churn.

The big item is moving over the tests.

@jlewi jlewi moved this from sprint backlog to Move kfctl to kubeflow/kfctl in kfctl-1.0 Nov 6, 2019
@jlewi
Copy link
Contributor Author

jlewi commented Nov 8, 2019

@kunmingg Do we need to move over the other click to deploy binaries in
https://github.com/kubeflow/kubeflow/tree/master/bootstrap/cmd ?

jlewi pushed a commit to jlewi/testing that referenced this issue Nov 8, 2019
jlewi pushed a commit to jlewi/manifests that referenced this issue Nov 8, 2019
* The e2e test is failing because kfctl has moved from kubeflow/kubeflow
  to kubfelow/kfctl (kubeflow/kfctl#7)

* The existing E2E test is an outdated copy of the old ksonnet workflow

* To unblock presubmits we are just deleting this test

* In a subsequent PR we will add back an E2E test but avoid duplicating the
  test and just check the test out from kubeflow/kubeflow or kubeflow/kfctl

related: kubeflow#613
k8s-ci-robot pushed a commit to kubeflow/manifests that referenced this issue Nov 8, 2019
* The e2e test is failing because kfctl has moved from kubeflow/kubeflow
  to kubfelow/kfctl (kubeflow/kfctl#7)

* The existing E2E test is an outdated copy of the old ksonnet workflow

* To unblock presubmits we are just deleting this test

* In a subsequent PR we will add back an E2E test but avoid duplicating the
  test and just check the test out from kubeflow/kubeflow or kubeflow/kfctl

related: #613
k8s-ci-robot pushed a commit to kubeflow/testing that referenced this issue Nov 9, 2019
* Kubeflow autodeploy jobs need to use kubeflow/kfctl

* Related to kubeflow/kfctl#7

* Cleanup

* Use PR for manual testing.

* Unpin

* Fix lint.
zhenghuiwang pushed a commit to zhenghuiwang/manifests that referenced this issue Nov 23, 2019
* The e2e test is failing because kfctl has moved from kubeflow/kubeflow
  to kubfelow/kfctl (kubeflow/kfctl#7)

* The existing E2E test is an outdated copy of the old ksonnet workflow

* To unblock presubmits we are just deleting this test

* In a subsequent PR we will add back an E2E test but avoid duplicating the
  test and just check the test out from kubeflow/kubeflow or kubeflow/kfctl

related: kubeflow#613
k8s-ci-robot pushed a commit to kubeflow/manifests that referenced this issue Nov 25, 2019
* The e2e test is failing because kfctl has moved from kubeflow/kubeflow
  to kubfelow/kfctl (kubeflow/kfctl#7)

* The existing E2E test is an outdated copy of the old ksonnet workflow

* To unblock presubmits we are just deleting this test

* In a subsequent PR we will add back an E2E test but avoid duplicating the
  test and just check the test out from kubeflow/kubeflow or kubeflow/kfctl

related: #613
@jlewi
Copy link
Contributor Author

jlewi commented Dec 9, 2019

@kunmingg what's your thinking about the source of truth for C2D? Should we move the code in bootstrap into kubeflow/kfctl ?
https://github.com/kubeflow/kubeflow/tree/master/bootstrap

@jlewi
Copy link
Contributor Author

jlewi commented Dec 16, 2019

kubeflow/manifests#613 - is fixed so kubeflow/manifests presubmits are now running E2E tests

I think there are two remaining issues to close this out.

  1. Move kfctl py code into kubeflow/kfctl #77 - Finish migration of the python code
  2. Migrate C2D code from kubeflow/kubeflow/bootstrap into kubeflow/kfctl #141 - Migrate C2D code from kubeflow/kubeflow to kubeflow/kfctl

@jlewi
Copy link
Contributor Author

jlewi commented Jan 6, 2020

The only remaining work is moving for the C2D code. That has its own P2 bug (#141) so I'm going to go ahead and close this issue.

@jlewi jlewi closed this as completed Jan 6, 2020
0.7.0 automation moved this from To Do to Done Jan 6, 2020
kfctl-1.0 automation moved this from Move kfctl to kubeflow/kfctl to Done Jan 6, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
No open projects
0.7.0
  
Done
kfctl-1.0
  
Done
Development

No branches or pull requests

3 participants