-
Notifications
You must be signed in to change notification settings - Fork 500
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
cleanup of bash scripts #802
Conversation
fi | ||
|
||
GCLOUD="gcloud --project $PROJECT" | ||
if ! [[ -d .terraform ]]; then |
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.
The way the docs are written now, someone would run this before running terraform init
. https://pingcap.com/docs/v3.0/tidb-in-kubernetes/deploy/gcp-gke/
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 new approach uses terraform console
which is much more reliable. We can update the code snippet in the docs to have terraform init
before running this.
fi | ||
|
||
GCLOUD="gcloud --project $PROJECT" | ||
if ! [[ -d .terraform ]]; then | ||
echo "no .terraform directory, perhaps you need to run ''terraform init''?" >&2 |
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.
All the quoting looked a little odd to me, but no big deal.
I ran this myself. I already have the service account and it properly recognized that. |
What problem does this PR solve?
This PR resolves several bash mistakes and brings the scripts in-line with best practices, while adding some new features.
What is changed and how does it work?
Variable expansions are quotes, lowercase variable names are used, tests are cleaned up.
Check List
Tests
I manually tested a number of permutations, including changes to project, removing credentials files, etc.
Does this PR introduce a user-facing change?: