GoogleCloudPlatform / kubeflow-distribution

Blueprints for Deploying Kubeflow on Google Cloud Platform and Anthos
Apache License 2.0
78 stars 63 forks source link

Better support for upgrading the deployment process #90

Open jlewi opened 4 years ago

jlewi commented 4 years ago

When we transitioned to blueprints for GCP we stopped using kfctl. There were a bunch of reasons that I won't go into here. A big one was that it was faster to iterate just by using a Makefile to script the various kustomize, kpt, and kubectl commands needed to describe the deployment process. The resulting Makefile's are also fairly readable and users can easily customize them.

An unresolved question is how we make it easy for users to pull in updates to the deployment recipe.

Right now a Kubeflow install is laid out as follows

my-kubeflow/
    Makefile
    instance/...
    upstream/...

To upgrade a user just re-places upstream with the latest copy and then redeploys. We'd like the same process to pull in any updates to the deployment process. For example, if we've added extra steps to hydrate or to apply we'd like that to be applied.

The problem right now is that the the deployment recipe is stored in the Makefile which is outside of upstream.

The deployment recipe is the sequence of kubectl, kpt, istioctl commands to be executed to produce the manifests and apply them.

We'd like to store the recipe in upstream so that the recipe gets updated when a user pulls in upstream.

While we could continue to use Makefile to define the recipe, a more cloud native way would be to use a YAML file listing out the commands. Tekton pipeline's and Tasks provide an existing resource that is well suited for this purpose. For example, we could easily chunk up our Makefile into tasks (e.g. hydrate and apply) that would contain steps corresponding to the commands currently invoked by the Makefile.

The downside of Tekton is that it requires Tekton and thus a K8s cluster to run.

This could be addressed by a CLI that would simply read in the Tekton resources and execute the commands by shelling out.

We could easily bake this functionality into kfctl; e.g.

kfctl apply -f deploy-kf-pipeline.yaml

@kkasravi suggested something along these lines awhile back.

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

Issue-Label Bot is automatically applying the labels:

Label Probability
platform/gcp 0.76
kind/feature 0.83
area/kfctl 0.78

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

jlewi commented 4 years ago

@animeshsingh FYI an idea for how we might re-integrate kfctl into the GCP process.

Bobgy commented 4 years ago

I worry that may be complicating things more than necessary. A local executor for tekton steps is also something new and we'd need documentation for which features we support and which not.

Did we consider just writing bash scripts for each make target and put those bash scripts in upstream?

Bobgy commented 4 years ago

For needed information, we can set up a convention of using env vars to pass to these arguments or a config.yaml in a conventional location. I think both works, I'd probably prefer a config.yaml.

Bobgy commented 4 years ago

Or maybe another approach: https://www.gnu.org/software/make/manual/html_node/Recursion.html

animeshsingh commented 4 years ago

a quick kubernetes local cluster can be created with kind.(this takes 30s) - kind create cluster.

Haven't tried Tekton on it, but no reason it shouldn't work ( have used kfserving tests in past) https://github.com/kubernetes-sigs/kind#installation-and-usage

jlewi commented 4 years ago

a quick kubernetes local cluster can be created with kind.(this takes 30s) - Running a local K8s cluster is too much complexity.

Did we consider just writing bash scripts for each make target and put those bash scripts in upstream? That's one option;

jlewi commented 4 years ago

I like @Bobgy 's suggestion to use recursive make. This would allow us to follow a similar pattern to what we are doing with kustomize in that we would have an overlay (the Make in the blueprint) where users could add custom commands and delegate to Make rules in the upstream branch. On upgrades this would preserve user customizations while pulling in any updates to the recipe.

e.g. ${MYKUBEFLOW}/Makefile:

.PHONY: apply
apply:    
    make -C upstream/gcp  apply
    <user defined commands>
jlewi commented 4 years ago

FYI @subodh101 since they have been cleaning up our Makefiles

subodh101 commented 4 years ago

I also feel @Bobgy 's suggestion is the right thing to do.

Few thoughts:

jlewi commented 4 years ago

what about the users who are using old makefile

Users would have to manually update the ${KUBEFLOW}/Makefile. This is true today. With ever release of KF if make changes to the sequence of kubectl/kpt/yq commands to deploy Kubeflow they will have to update their Makefiles manually. This is what we are trying to address with this issue.

jtfogarty commented 3 years ago

/priority p1

Bobgy commented 3 years ago

Several ways to reuse & override a makefile:

Include clause seems more lightweight, all makefile context can be shared, but there's less encapsulation either. For recursive make, I'll need to verify which working dir will it be for the upstream makefile. EDIT: verified, if using $(MAKE) -f upstream/xxx/Makefile, workdir will remain in current dir.

Bobgy commented 3 years ago

A few types of customizations people may want to do:

  1. add extra commands for a target
  2. replace all commands for a target
  3. add extra dependencies for a target
  4. replace all dependencies for a target

For recursive make, if you want to change any target that's depended on by more targets, it'll be very troublesome because you'll need to replace every target that depends on the target you changed.

For make include clause, adding extra dependencies/commands for a target can be done via https://www.gnu.org/software/make/manual/html_node/Multiple-Rules.html conveniently.

However, neither of the two solutions make it convenient to completely replace a rule (target + dependencies + commands).

Right now, I feel like the best solution is to avoid using dependencies in the upstream Makefile altogether (or only use dependencies when it doesn't make sense to override the dependencies). Replacing commands is easier via: https://stackoverflow.com/a/49804748/8745218 Or if we do not care about warnings like:

Makefile:5: warning: overriding commands for target `echo'
upstream/Makefile:2: warning: ignoring old commands for target `echo'

We can ask users just redeclare a target + command to override the command.

Therefore, in this approach, we are just using Makefile as a convenient way to write a script with multiple subcommands, so that the user can reuse and update.