-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Private zonal example #310
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
Merged
morgante
merged 4 commits into
terraform-google-modules:master
from
ideasculptor:private_zonal_example
Nov 20, 2019
Merged
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,87 @@ | ||
/** | ||
* Copyright 2019 Google LLC | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); | ||
* you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
|
||
module "gcp-network" { | ||
source = "terraform-google-modules/network/google" | ||
version = "~> 1.4.0" | ||
project_id = var.project_id | ||
network_name = var.network | ||
|
||
subnets = [ | ||
{ | ||
subnet_name = var.subnetwork | ||
subnet_ip = "10.0.0.0/17" | ||
subnet_region = var.region | ||
subnet_private_access = "true" | ||
}, | ||
] | ||
|
||
secondary_ranges = { | ||
"${var.subnetwork}" = [ | ||
{ | ||
range_name = var.ip_range_pods_name | ||
ip_cidr_range = "192.168.0.0/18" | ||
}, | ||
{ | ||
range_name = var.ip_range_services_name | ||
ip_cidr_range = "192.168.64.0/18" | ||
}, | ||
] | ||
} | ||
} | ||
|
||
data "google_compute_subnetwork" "subnetwork" { | ||
name = var.subnetwork | ||
project = var.project_id | ||
region = var.region | ||
depends_on = [module.gcp-network] | ||
} | ||
|
||
module "gke" { | ||
source = "../../modules/beta-private-cluster/" | ||
project_id = var.project_id | ||
name = var.cluster_name | ||
regional = false | ||
region = var.region | ||
zones = slice(var.zones, 0, 1) | ||
|
||
// This craziness gets a plain network name from the reference link which is the | ||
// only way to force cluster creation to wait on network creation without a | ||
// depends_on link. Tests use terraform 0.12.6, which does not have regex or regexall | ||
network = reverse(split("/", data.google_compute_subnetwork.subnetwork.network))[0] | ||
|
||
subnetwork = data.google_compute_subnetwork.subnetwork.name | ||
ip_range_pods = var.ip_range_pods_name | ||
ip_range_services = var.ip_range_services_name | ||
create_service_account = true | ||
enable_private_endpoint = true | ||
enable_private_nodes = true | ||
master_ipv4_cidr_block = "172.16.0.0/28" | ||
|
||
master_authorized_networks_config = [ | ||
{ | ||
cidr_blocks = [ | ||
{ | ||
cidr_block = data.google_compute_subnetwork.subnetwork.ip_cidr_range | ||
display_name = "VPC" | ||
}, | ||
] | ||
}, | ||
] | ||
} | ||
|
||
data "google_client_config" "default" { | ||
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,59 @@ | ||
/** | ||
* Copyright 2019 Google LLC | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); | ||
* you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
|
||
output "kubernetes_endpoint" { | ||
description = "The cluster endpoint" | ||
sensitive = true | ||
value = module.gke.endpoint | ||
} | ||
|
||
output "client_token" { | ||
description = "The bearer token for auth" | ||
sensitive = true | ||
value = base64encode(data.google_client_config.default.access_token) | ||
} | ||
|
||
output "ca_certificate" { | ||
description = "The cluster ca certificate (base64 encoded)" | ||
value = module.gke.ca_certificate | ||
} | ||
|
||
output "service_account" { | ||
description = "The default service account used for running nodes." | ||
value = module.gke.service_account | ||
} | ||
|
||
output "cluster_name" { | ||
description = "Cluster name" | ||
value = module.gke.name | ||
} | ||
|
||
output "network_name" { | ||
description = "The name of the VPC being created" | ||
value = module.gcp-network.network_name | ||
} | ||
|
||
output "subnet_name" { | ||
description = "The name of the subnet being created" | ||
value = module.gcp-network.subnets_names | ||
} | ||
|
||
output "subnet_secondary_ranges" { | ||
description = "The secondary ranges associated with the subnet" | ||
value = module.gcp-network.subnets_secondary_ranges | ||
} | ||
|
||
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,58 @@ | ||
/** | ||
* Copyright 2018 Google LLC | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); | ||
* you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
|
||
// These outputs are used to test the module with kitchen-terraform | ||
// They do not need to be included in real-world uses of this module | ||
|
||
output "project_id" { | ||
value = var.project_id | ||
} | ||
|
||
output "network" { | ||
value = var.network | ||
} | ||
|
||
output "subnetwork" { | ||
value = var.subnetwork | ||
} | ||
|
||
output "location" { | ||
value = module.gke.location | ||
} | ||
|
||
output "region" { | ||
value = var.region | ||
} | ||
|
||
output "ip_range_pods_name" { | ||
description = "The secondary IP range used for pods" | ||
value = var.ip_range_pods_name | ||
} | ||
|
||
output "ip_range_services_name" { | ||
description = "The secondary IP range used for services" | ||
value = var.ip_range_services_name | ||
} | ||
|
||
output "zones" { | ||
description = "List of zones in which the cluster resides" | ||
value = module.gke.zones | ||
} | ||
|
||
output "master_kubernetes_version" { | ||
description = "The master Kubernetes version" | ||
value = module.gke.master_version | ||
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,54 @@ | ||
/** | ||
* Copyright 2019 Google LLC | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); | ||
* you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
|
||
variable "project_id" { | ||
description = "The project ID to host the cluster in" | ||
} | ||
|
||
variable "cluster_name" { | ||
description = "The name for the GKE cluster" | ||
default = "gke-on-vpc-cluster" | ||
} | ||
|
||
variable "region" { | ||
description = "The region to host the cluster in" | ||
} | ||
|
||
variable "zones" { | ||
type = list(string) | ||
description = "The zone to host the cluster in (required if is a zonal cluster)" | ||
} | ||
|
||
variable "network" { | ||
description = "The VPC network created to host the cluster in" | ||
default = "gke-network" | ||
} | ||
|
||
variable "subnetwork" { | ||
description = "The subnetwork created to host the cluster in" | ||
default = "gke-subnet" | ||
} | ||
|
||
variable "ip_range_pods_name" { | ||
description = "The secondary ip range to use for pods" | ||
default = "ip-range-pods" | ||
} | ||
|
||
variable "ip_range_services_name" { | ||
description = "The secondary ip range to use for pods" | ||
default = "ip-range-scv" | ||
} | ||
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
--- | ||
|
||
platform: linux | ||
|
||
inputs: | ||
- name: pull-request | ||
path: terraform-google-kubernetes-engine | ||
|
||
run: | ||
path: make | ||
args: ['test_integration'] | ||
dir: terraform-google-kubernetes-engine | ||
|
||
params: | ||
SUITE: "private-zonal-with-networking-local" | ||
COMPUTE_ENGINE_SERVICE_ACCOUNT: "" | ||
REGION: "us-east4" | ||
ZONES: '["us-east4-a", "us-east4-b", "us-east4-c"]' |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
/** | ||
* Copyright 2018 Google LLC | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); | ||
* you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
|
||
module "example" { | ||
source = "../../../examples/private_zonal_with_networking" | ||
|
||
project_id = var.project_id | ||
region = var.region | ||
zones = var.zones | ||
} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 we make this a reference to the
gcp-network
outputs? That might help with the dependency graph issue.Uh oh!
There was an error while loading. Please reload this page.
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 don't think so. When I used a direct dependency, it would always fail the first time, because the cluster would come up before the network was ready. Those lines were written that way specifically to eliminate that problem. I had to try a number of ways to string those dependencies together to get it to stop erroring out when creating both the network and the cluster in the same terraform operation - the problem doesn't exist when they are run separately.
I can't remember which variations my 3am brain attempted - every failed attempt is a really long test cycle, so it's entirely possible that I was addled enough from lack of sleep to have missed something obvious. If memory serves, if I used the constant cidr range as a string and just injected it into ipv4_master_networks_config, that caused dependencies in the gke module to fire before the network was ready. I'm reasonably certain my first solution would have been to grab the first value of subnets_ranges from the gcp-network module outputs, but maybe I went straight from constant value to complex hack without first trying all the obvious middle steps, though I certainly tried a few.
Because the network name is passed to the gcp-network outputs directly from an input value, dependencies in the gke module were allowed to fire with the network name before the network existed even when using the module output, since terraform's dependency graph wouldn't wait for first module to complete before starting work on resources in the second. You can't use the depends_on thing directly with modules, so I had no way to make the dependency work correctly without modifying one or both modules to manually provide a depends_on link between resources in each module and I've just spent far too much time and energy on this already.
My quick solution was to use the data source as an intermediary, but since the gke module can't accept the network name as a fully qualified link, I had to parse the network name out of that data source's attributes. That corrected the dependency ordering on creation, but breaks the destroy ordering, since the data source doesn't act as a barrier to the network module being torn down, so it gets torn down before the GKE cluster has gone away.
Uh oh!
There was an error while loading. Please reload this page.
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.
Network name should come from the network outputs though, not directly passing through the input: https://github.com/terraform-google-modules/terraform-google-network/blob/master/main.tf#L19
It definitely seems like using the output from the network module should order things correctly. BTW, in case you weren't aware, you can print the Terraform DAG: https://www.terraform.io/docs/commands/graph.html
This should be much faster to debug than actually spinning up the infra.
Uh oh!
There was an error while loading. Please reload this page.
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 speaks to the lack of real-world integration examples for the modules. On the one hand, real-world production usage will almost never bring up the network and the GKE cluster in the same terraform plan, but on the other, integration examples always will, so if those examples existed, the modules would already have handled the outputs being directly linked to inputs and failing to result in a valid dependency graph, and someone other than a user would have spotted the need for some form of computed pass-through or provided a null_resource (and corresponding consumer in other modules), which would serve as a block.
The pattern I've used in the past is for every module to include a null_resource.completion resource with an output that depends on it and a set of triggers that includes all computed resources in the dependency chain. Additionally, every module can receive an input which is used only as a trigger for a different null_resource.block resource, with every other resource in the module including a
depends_on = [null_resource.block]
. Feeding the null_resource.completion-based output of the first module to the trigger of the null_resource.block resource in the dependent module guarantees that nothing in the dependent module can execute until everything in the dependency has completed. If no value is passed through, no blockage happens.First module:
Dependent Module:
Uh oh!
There was an error while loading. Please reload this page.
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'm still not following the issue though. The linked code shows that the network output isn't directly linked to the input.
This is definitely an interesting pattern. It's quite artificial but probably a good hack to include.
Uh oh!
There was an error while loading. Please reload this page.
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.
Your constant assumption that I'm an idiot who doesn't know what he is doing, rather than someone with a great deal of experience with both terraform and infrastructure automation, encountering actual real-world problems that your simple examples and mimimal documentation just don't account for, has burned through pretty much all of the goodwill that was allowing me to spend so much of my spare time improving your product for free. Honestly, is there ANYTHING in any of my significant contributions so far that makes you think I don't read documentation before asking for assistance? Maybe don't assume that to be the case every time I raise an issue that is more significant than a typo.
I don't have the big picture of where you are going with those modules, but that's only because it hasn't been laid out anywhere public where I can see it, so far as I am aware. If you put modules out there, publicly acknowledging that they are built in-house by Google, you have to assume people are going to try to make use of them, as they surely represent something akin to common best practices. If you don't provide meaningful guidance about how to do them as such, and also refuse to respond helpfully when people encounter problems, you are going to burn down whatever community your open source efforts are trying to build.
Honestly, in the immediate wake of a nearly 72 hour unscheduled service outage, maybe consider the kind of frustrations I might be experiencing when I still can't get things to work, and take a few minutes from your obviously very busy schedule to give an assist to someone who has shown no hesitation to devote significant time and energy into giving your team a helping hand?...
Uh oh!
There was an error while loading. Please reload this page.
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.
An hour or less of a single engineer's time doesn't seem like a huge sacrifice to make in exchange for the 100 or so hours that I have devoted just to correcting problems in these modules - the vast majority of which had nothing whatsoever to do with any kind of non-standard or mistaken use.
And maybe consider that if someone who is clearly pretty darn fluent in this stack is having difficulty intuiting your intent, someone with minimal terraform or GCP experience, looking to get a leg up on their own automation efforts by exploring the official module library, is going to have a much more difficult time of it, and that maybe some documentation or assistance is in order, rather than public shaming.
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'm sorry you feel that way — I certainly think you're very skilled and appreciate all the work you've been doing. It's hugely helpful!
Agreed, I know we're not doing a good enough job here. In terms of our goals, this page is probably the best we have.
We absolutely do need to provide more and better end-to-end examples, and it's on our radar for what to build next. Sorry if my responses are too brusque thus far, I definitely recognize that need.
You're right: as you can imagine, there have been a lot of fires lately but we should still appreciate our contributors.
Uh oh!
There was an error while loading. Please reload this page.
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 can promise you, when I used a direct link between the one module and the other, it errored out on the first run (and succeeded on the 2nd, if no destroy happened first) every single time. Feel free to try it. If I'm wrong, and it was certainly hour 17 or 18 of my work day at that point, it's a simple enough change to make. I've contributed my last PR for a good long while.
Uh oh!
There was an error while loading. Please reload this page.
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.
Ok, I'll give it a shot! It's just surprising that this doesn't work considering how it's coded but I certainly believe you've tried it.