-
Notifications
You must be signed in to change notification settings - Fork 282
Conversation
Based on the initial push none of the existing examples broke |
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.
Looking good @autero1
subnetwork = "${google_compute_subnetwork.main.self_link}" | ||
|
||
# When creating a private cluster, the 'master_ipv4_cidr_block' has to be defined and the size must be /28 | ||
master_ipv4_cidr_block = "10.5.0.0/28" |
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.
any point parameterizing this?
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.
Was thinking that also. The other examples use hardcoded values, so I went with that. Maybe parameterize with those values as defaults?
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.
Okay understood, wondering if we should do this now or ship as is. If we change, we should change all examples
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'd be tempted to create pre_launch
branch and PR we all work on. Some of the items include
- README updates
- Code documentation (comments and section within modules and examples)
- Use VPC module
- Optionally adding a deployment step in one one of the examples + exposing the service
|
||
If you want your cluster nodes to be able to access the Internet, for example pull images from external container registries, | ||
you will have to set up [Cloud NAT](https://cloud.google.com/nat/docs/overview). | ||
See [Example GKE Setup](https://cloud.google.com/nat/docs/gke-example) for further information. |
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.
Maybe one day we could include an example for this?
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 believe our VPC module has Cloud NAT. Other option could be showcasing running one of Google's containers (gcloud container images list --project google-containers
). I tested the private cluster with gcr.io/google-containers/nginx
and exposed that with a load balancer.
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.
Checked the module, and the NAT is only for the public subnet.
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.
# DEPLOY A GKE CLUSTER | ||
# This module deploys a GKE cluster, a managed, production-ready environment for deploying containerized applications. | ||
# ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | ||
|
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.
Nice 👍
Co-Authored-By: autero1 <[email protected]>
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.
LGTM. A few nits in documentation and API, but they are minor.
description = "${var.cluster_service_account_description}" | ||
} | ||
|
||
# TODO(rileykarson): Add proper VPC network config once we've made a VPC module |
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.
@rileykarson Is this still valid? I thought v0.0.1
has everything necessary, or am I missing something?
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.
@yorinasub17 nope, no longer valid. We should use @rileykarson 's module. Another thing on my pre-launch list.
region as your private cluster, can use the private endpoint. | ||
|
||
* **Public endpoint:** This is the external IP address of the master. You can disable access to the public endpoint by setting | ||
`enable_private_endpoint` to `true`. |
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 slightly confusing. E.g enable_private_endpoint
signals to me that this only controls the private endpoint, treating the public endpoint separately.
Should this be disable_public_endpoint
instead?
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 you're right... using disable_public_endpoint
feels more intuitive.
Reason for naming it like this is because gcloud
uses --enable-private-endpoint
. On the other hand the web console uses "Access master using its external IP address" -checkbox. 😄
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.
And terraform
provider uses enable_private_endpoint
😄
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.
Makes sense. I am a bit on the fence about deviating from the google APIs, but I do feel that disable_public_endpoint
is the better name...
Co-Authored-By: autero1 <[email protected]>
…oogle-gke into private_cluster
This PR adds support for GKE private clusters.