-
Notifications
You must be signed in to change notification settings - Fork 474
GCE and general cloud deployment improvements #574
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
Conversation
lgtm |
|
||
This page shows you how to manually deploy an insecure multi-node CockroachDB cluster on Google Cloud Platform's Compute Engine. | ||
|
||
{{site.data.alerts.callout_warning}} This guide demonstrates how to deploy an <strong>insecure</strong> cluster, which we do not recommend for data in production. We'll update this page once we've improved the workflow for deploying secure clusters.{{site.data.alerts.end}} |
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.
Can you explain what plans we have for improving the workflow for deploying secure clusters? I ask since it's also relevant for deploying on top of container orchestration systems like Kubernetes.
ee3ccdf
to
933b19d
Compare
Nice, @spoiselle! Some overall comments here and specifics below.
Still need to review last half of the tutorial. Will do that soon. Review status: 0 of 4 files reviewed at latest revision, 7 unresolved discussions, some commit checks failed. cloud-deployment.md, line 16 [r2] (raw file):
deploy-cockroachdb-on-aws.md, line 11 [r2] (raw file):
Let's add "Deploy CockroachDB on Google Cloud Platform GCE" to the See Also list as well. deploy-cockroachdb-on-google-cloud-platform.md, line 30 [r2] (raw file):
Thoughts, @sploiselle? deploy-cockroachdb-on-google-cloud-platform.md, line 69 [r2] (raw file):
I think the important point is running at least 3. Probably don't need to state an upper limit. Also, can we find a way to link to https://www.cockroachlabs.com/docs/configure-replication-zones.html#nodereplica-recommendations? deploy-cockroachdb-on-google-cloud-platform.md, line 73 [r2] (raw file):
As discussed offline, I couldn't quite follow this guidance, so let's maybe be a little more verbose for the sake of clarify, e.g., "If you used a tag for your firewall rules, when you create the instance, select Management, disk, networking, SSH keys. Then on the Management tab, enter the tag in the Tags field. Also, do you have to check the Allow HTTP traffic and Allow HTTPS traffic boxes? Comments from Reviewable |
Review status: 0 of 4 files reviewed at latest revision, 14 unresolved discussions, some commit checks failed. deploy-cockroachdb-on-google-cloud-platform.md, line 80 [r2] (raw file):
To be consistent with the rest of the docs, let's start each command with the deploy-cockroachdb-on-google-cloud-platform.md, line 85 [r2] (raw file):
deploy-cockroachdb-on-google-cloud-platform.md, line 114 [r2] (raw file):
Same as above: combine into single code block. deploy-cockroachdb-on-google-cloud-platform.md, line 147 [r2] (raw file):
Same as above: combine into single code block. deploy-cockroachdb-on-google-cloud-platform.md, line 187 [r2] (raw file):
For consistency: "Use the Database". deploy-cockroachdb-on-google-cloud-platform.md, line 192 [r2] (raw file):
This is a little misleading, because you're not really creating users when granting permissions. We don't really have "user" entities right now. So let's go with something like "Grant privileges to users". deploy-cockroachdb-on-google-cloud-platform.md, line 195 [r2] (raw file):
Let's also link to the aws page. Comments from Reviewable |
933b19d
to
566f39c
Compare
Done. Review status: 0 of 5 files reviewed at latest revision, 14 unresolved discussions. cloud-deployment.md, line 16 [r2] (raw file):
|
31d9f90
to
a5ed69f
Compare
a5ed69f
to
7127398
Compare
@jseldess Persistent disks work so I was able to remove the local scratch disk SSD requirements. Barring any further edits, this is good to merge. |
Review status: 0 of 5 files reviewed at latest revision, 14 unresolved discussions. Comments from Reviewable |
Includes:
Related issues:
Closes #509
@mberhault –– Can you review?
This change is