Split kfctl_go_test.py into separate python functions for building and deploying kubeflow

[jlewi]

trafficstars

Right now we have a single python file https://github.com/kubeflow/kubeflow/blob/master/testing/kfctl/kfctl_go_test.py

That does two things

1. Builds kfctl
2. Deploys kfctl

We will probably want to split those into two separate python functions to make it more composable.

One use case for composability is to create an E2E test for upgradability (#35). In that E2E test we will want to build kfctl once but invoke kfctl twice; once to deploy and once to do the upgrade.

Another use case is to create E2E tests for other platforms configurations. For example we want an E2E test for installing Kubeflow on an existing cluster (kubeflow/kubeflow#3496).

In that case we need to provision a Kubernetes cluster before deploying Kubeflow; e.g. using kops.

Related to: #35 E2E test for kfctl upgrade

/cc @yanniszark @Jeffwan

[nrchakradhar]

@jlewi I can take a shot at this. I need to spend some time to understand the flow before attempting changes. If its not urgent, I can try to submit a PR. My current understanding is as below:

• Create kfctl_go_build_test.py and kfctl_go_deploy_test.py.

• Move the build and deploy code to the appropriate files from

https://github.com/kubeflow/kubeflow/blob/2f15d8dbe85f0081652c1e5363f6a8c547a3e5fc/testing/kfctl/kfctl_go_test.py#L53

3. Modify

https://github.com/kubeflow/kubeflow/blob/2f15d8dbe85f0081652c1e5363f6a8c547a3e5fc/testing/workflows/components/kfctl_go_test.jsonnet#L219

to include separate steps for build and deploy.

In #35, there is mention about "minimal py_func". Does this mean that create two py_funcs one for build and another for deploy in kfctl_go_test.py and change the kfctl_go_test.jsonnet to only invoke specific py_func in the workflow.

I see that each step calls only one test and not individual py_func from test files.

[nrchakradhar]

@jlewi Should I start a PR in the kfctl repo or in the kubeflow repo. I understand that currently there is duplication in these repos. Based on your advice, I will start.

[jlewi]

@nrchakradhar I'll need this next week so if you could take a stab at this that would be great.

Create kfctl_go_build_test.py and kfctl_go_deploy_test.py

Yes. Although it may be easier to fork kfctl_go_build_test.py into a new script kfctl_build_test.py this way we can check in those files without having to update the E2E test.

Updating the E2E test to use the new scripts is blocked on kubeflow/kubeflow#4148 which replaces kfctl_go_test.jsonnet with a python function that builds the Argo workflow.

So we'll want to get that PR merged first and then modify the resulting python code to use the new scripts.

[nrchakradhar]

@jlewi I started with the first refactor with PR#4170. PTAL If this is fine, I will take up further modifications after merging this. Does that sound reasonable?

[jlewi]

@nrchakradhar sounds great. I LGTM'd your first PR this morning.

[jlewi]

@nrchakradhar How's it going splitting up kfctl_go_test.py into two separate steps?

We can split the code into two separate function/modules now. But lets wait until after kubeflow/kubeflow#4148 is merged before updating the workflow to run two steps instead of 1.

[nrchakradhar]

@jlewi I am https://github.com/kubeflow/kubeflow/pull/4148 following your PR, I will start today with changes and try to submit a PR.

[nrchakradhar]

@jlewi I just created a PR https://github.com/kubeflow/kubeflow/pull/4187 to separate build and deploy. PTAL.