Skip to content
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

Gke terraform and guide #493

Merged
merged 22 commits into from
May 23, 2019
Merged

Conversation

jlerche
Copy link
Contributor

@jlerche jlerche commented May 14, 2019

What problem does this PR solve?

Closes #446

What is changed and how it works?

Adds GCP terraform deploy and README

Check List

Tests

  • Manual test (add detailed scripts or steps below)

Code changes

  • Adds new Terraform script
  • Adds readme for script

Several changes were made since the last review:

  • More settings were extracted into variables
  • Workarounds added to deal with some bugs in the google Terraform provider
  • DaemonSet with startup script to increase open file descriptors on PD and TiKV nodes, also installs Linux Guest Environment
  • Outputs added to make things a bit more user friendly.

@jlerche jlerche requested review from tennix and aylei May 14, 2019 22:54
@jlerche jlerche self-assigned this May 14, 2019
@gregwebs
Copy link
Contributor

gregwebs commented May 14, 2019

We should have a firewall rule to allow SSH to the GKE nodes from the bastion. This can use the "bastion" tag

@tennix tennix requested a review from cofyc May 15, 2019 05:28
deploy/gcp/README.md Outdated Show resolved Hide resolved
deploy/gcp/main.tf Show resolved Hide resolved
deploy/gcp/main.tf Outdated Show resolved Hide resolved
deploy/gcp/outputs.tf Show resolved Hide resolved
deploy/gcp/templates/tidb-cluster-values.yaml.tpl Outdated Show resolved Hide resolved
deploy/gcp/main.tf Outdated Show resolved Hide resolved
deploy/gcp/outputs.tf Show resolved Hide resolved
deploy/gcp/README.md Outdated Show resolved Hide resolved
deploy/gcp/README.md Outdated Show resolved Hide resolved
deploy/gcp/main.tf Outdated Show resolved Hide resolved
aylei
aylei previously approved these changes May 16, 2019
Copy link
Contributor

@aylei aylei left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@tennix
Copy link
Member

tennix commented May 17, 2019

@DanielZhangQD deployed a testing cluster on GCP with this PR, the tikv pods cannot start due to small max-open-file descriptors. Besides, it's the default local ssd has poor performance without installing Linux Guest Environment

tennix
tennix previously approved these changes May 21, 2019
Copy link
Member

@tennix tennix left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

deploy/gcp/main.tf Show resolved Hide resolved
deploy/gcp/README.md Outdated Show resolved Hide resolved
@gregwebs
Copy link
Contributor

Can you mention the startup-script in the GKE related docs? Maybe move it to the manifests folder?

deploy/gcp/README.md Outdated Show resolved Hide resolved
@jlerche
Copy link
Contributor Author

jlerche commented May 22, 2019

Per slack conversation with @gregwebs, startup-script related docs changes will be done in a separate PR.

Copy link
Member

@tennix tennix left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

@DanielZhangQD DanielZhangQD left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM


Currently, there are not too many parameters exposed to be customized. However, you can modify `templates/tidb-cluster-values.yaml.tpl` before deploying. If you modify it after the cluster is created and then run `terraform apply`, it will not take effect unless the pod(s) is manually deleted.

### Customizing node pools
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
### Customizing node pools
### Removing unused instances

@tennix tennix merged commit b9889ec into pingcap:master May 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

GKE terraform and updated deployment guide.
5 participants