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
Create a deployment script for gke and minikube #1111
Conversation
scripts/gke/deploy.sh
Outdated
KUBEFLOW_REPO=$(cd "${SCRIPT_DIR}/../.."; pwd) | ||
check_install gcloud | ||
check_install kubectl | ||
check_install ks |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might want to check ks version since only new one support local registry.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is non-trivial to do in bash. Added a TODO. We will mention in the docs that the requirements are ks > 0.11
/retest |
1 similar comment
/retest |
* Move away from using bootstrapper by doing everything locally * Try to use gcloud to infer as much as possible * Delete k8s resources from DM configs * Require prerequisite of gcloud, kubectl and ks Docs update Update
/retest |
Can we move the clone into the deploy script? So the experience is something like
|
This is fantastic. Thank you. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 0 of 7 files reviewed, 3 unresolved discussions (waiting on @kunmingg, @ankushagarwal, and @jlewi)
scripts/gke/deploy.sh, line 49 at r3 (raw file):
CONFIG_FILE=${CONFIG_FILE:-"cluster-kubeflow.yaml"} PROJECT_NUMBER=`gcloud projects describe ${PROJECT} --format='value(project_number)'` SA_EMAIL=${DEPLOYMENT_NAME}-admin@${PROJECT}.iam.gserviceaccount.com
nit: ADMIN_EMAIL not SA_email
scripts/gke/deploy.sh, line 108 at r3 (raw file):
o
not -> no
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 0 of 7 files reviewed, 2 unresolved discussions (waiting on @kunmingg and @jlewi)
scripts/gke/deploy.sh, line 108 at r3 (raw file):
Previously, jlewi (Jeremy Lewi) wrote…
o
not -> no
Done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 0 of 7 files reviewed, 2 unresolved discussions (waiting on @kunmingg and @ankushagarwal)
scripts/gke/deploy.sh, line 15 at r4 (raw file):
KUBEFLOW_VERSION=${KUBEFLOW_VERSION:-"master"} rm -rf "${KUBEFLOW_REPO}"
How about checking if KUBEFLOW_REPO exists and if it does don't clone it?
ks registry add kubeflow "${KUBEFLOW_REPO}/kubeflow" | ||
|
||
# Install all required packages | ||
ks pkg install kubeflow/core |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we make this a function so that other deploy scripts could use it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is fine for now because different deploy scripts will have different pkgs / components to install. It's easy to add plain ks commands in the individual deploy scripts.
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: 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 |
* Create a deployment script for gke and minikube * Move away from using bootstrapper by doing everything locally * Try to use gcloud to infer as much as possible * Delete k8s resources from DM configs * Require prerequisite of gcloud, kubectl and ks Docs update Update * Address comments * Check if kubeflow repo exists
TODO(ankushagarwal): Add integration test for this.
TODO(ankushagarwal): Delete https://github.com/kubeflow/kubeflow/tree/master/docs/gke/configs
Fixes #1068
After this PR the getting started experience for kubeflow on GKE:
We can get rid of
CLIENT_ID and CLIENT_SECRET
step soonAfter this the getting started experience for kubeflow on GKE:
This change is