Consolidate GKE deployment script #985
Conversation
docs/gke/configs/deploy.sh
Outdated
| @@ -0,0 +1,62 @@ | |||
| #!/usr/bin/env bash | |||
|
|
|||
| set -e | |||
There was a problem hiding this comment.
set -e might be an issue because kubectl create namespace can fail if namespace is already created by bootstrapper.
There was a problem hiding this comment.
I did see that comment in the other script but it appears to be working even with setting this flag. I can add an OR flag after this command.
There was a problem hiding this comment.
I add an unset -e just before the namespace and secrets creation towards the end of the script.
| @@ -0,0 +1,62 @@ | |||
| #!/usr/bin/env bash | |||
There was a problem hiding this comment.
Can you add a comment explaining what this script does
docs/gke/configs/env-kubeflow.sh
Outdated
| # Get the project number | ||
| export PROJECT_NUMBER=`gcloud projects describe ${PROJECT} --format='value(project_number)'` | ||
| # Set OAuth2 Credentials | ||
| export CLIENT_ID= |
There was a problem hiding this comment.
I don't know that this is a good idea.
env-kubeflow.sh is something we want to be able to check into source control; but CLIENT_ID and CLIENT_SECRET are not something that should be checked in.
There was a problem hiding this comment.
I was actually considering this file as a reference for the variables needed rather than something to be checked into source control. Perhaps, we should put a warning in the documentation regarding the sensitivity of these credentials?
|
/retest |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ankushagarwal 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 |
* Consolidate GKE deployment script, update required variables with OAuth2 credentials * Restore comments from create_k8s_secrets.sh * Ignore namespace/secret creation errors * Add script title comments * Remove OAuth2 credentials, must be separately injected into environment
* Get dbUser from Env or default('root')
* Get dbUser from Env or default('root')
/cc @jlewi @kunmingg @ankushagarwal
This change is