From f828666224b198cfaf2bb82dfe8c47d22fc17808 Mon Sep 17 00:00:00 2001 From: Karen Almog Date: Mon, 12 Mar 2018 13:12:51 +0100 Subject: [PATCH] modules/vpc: support re-apply of terraform when AZ number changes --- modules/aws/vpc/common.tf | 69 +++++++++++++++++++++++++++++++++ modules/aws/vpc/existing-vpc.tf | 12 ------ modules/aws/vpc/variables.tf | 30 ++++---------- modules/aws/vpc/vpc-private.tf | 33 +++++++--------- modules/aws/vpc/vpc-public.tf | 31 +++++++-------- modules/aws/vpc/vpc.tf | 21 ++-------- platforms/govcloud/main.tf | 58 ++++++++------------------- platforms/govcloud/variables.tf | 4 +- steps/bootstrap/main.tf | 63 +++++++++--------------------- steps/variables.tf | 4 +- 10 files changed, 149 insertions(+), 176 deletions(-) create mode 100644 modules/aws/vpc/common.tf delete mode 100644 modules/aws/vpc/existing-vpc.tf diff --git a/modules/aws/vpc/common.tf b/modules/aws/vpc/common.tf new file mode 100644 index 0000000000..228684705f --- /dev/null +++ b/modules/aws/vpc/common.tf @@ -0,0 +1,69 @@ +# Canonical internal state definitions for this module. +# read only: only locals and data source definitions allowed. No resources or module blocks in this file +data "aws_region" "current" { + current = true +} + +// Fetch a list of available AZs +data "aws_availability_zones" "azs" {} + +// Only reference data sources which are gauranteed to exist at any time (above) in this locals{} block +locals { + // Define canonical source of truth for this + external_vpc_mode = "${var.external_vpc_id != ""}" + + // List of possible AZs for each type of subnet + new_worker_subnet_azs = ["${coalescelist(keys(var.new_worker_subnet_configs), data.aws_availability_zones.azs.names)}"] + new_master_subnet_azs = ["${coalescelist(keys(var.new_master_subnet_configs), data.aws_availability_zones.azs.names)}"] + + // How many AZs to create worker and master subnets in (always zero if external_vpc_mode) + new_worker_az_count = "${local.external_vpc_mode ? 0 : length(local.new_worker_subnet_azs)}" + new_master_az_count = "${local.external_vpc_mode ? 0 : length(local.new_master_subnet_azs)}" + + // The base set of ids needs to build rest of vpc data sources + // This is crux of dealing with existing vpc / new vpc incongruity + vpc_id = "${local.external_vpc_mode ? var.external_vpc_id : element(concat(aws_vpc.new_vpc.*.id,list("")),0)}" + + // When referencing the _ids arrays or data source arrays via count = , always use the *_count variable rather than taking the length of the list + worker_subnet_ids = ["${coalescelist(aws_subnet.worker_subnet.*.id,var.external_worker_subnet_ids)}"] + master_subnet_ids = ["${coalescelist(aws_subnet.master_subnet.*.id,var.external_master_subnet_ids)}"] + worker_subnet_count = "${local.external_vpc_mode ? length(var.external_worker_subnet_ids) : local.new_worker_az_count}" + master_subnet_count = "${local.external_vpc_mode ? length(var.external_master_subnet_ids) : local.new_master_az_count}" +} + +# all data sources should be input variable-agnostic and used as canonical source for querying "state of resources" and building outputs +# (ie: we don't want "data.aws_subnet.external-worker" and "data.aws_subnet.worker". just "data.aws_subnet.worker" used everwhere for list of worker subnets for any valid input var state) + +data "aws_vpc" "cluster_vpc" { + id = "${local.vpc_id}" +} + +data "aws_subnet" "worker" { + count = "${local.worker_subnet_count}" + id = "${local.worker_subnet_ids[count.index]}" + vpc_id = "${local.vpc_id}" +} + +data "aws_subnet" "master" { + count = "${local.master_subnet_count}" + id = "${local.master_subnet_ids[count.index]}" + vpc_id = "${local.vpc_id}" +} + +data "aws_route_table" "worker" { + count = "${local.worker_subnet_count}" + + filter = { + name = "association.subnet-id" + values = ["${list(local.worker_subnet_ids[count.index])}"] + } +} + +data "aws_route_table" "master" { + count = "${local.master_subnet_count}" + + filter = { + name = "association.subnet-id" + values = ["${list(local.master_subnet_ids[count.index])}"] + } +} diff --git a/modules/aws/vpc/existing-vpc.tf b/modules/aws/vpc/existing-vpc.tf deleted file mode 100644 index 1cd150cafd..0000000000 --- a/modules/aws/vpc/existing-vpc.tf +++ /dev/null @@ -1,12 +0,0 @@ -# These subnet data-sources import external subnets from their user-supplied subnet IDs -# whenever an external VPC is specified -# -data "aws_subnet" "external_worker" { - count = "${var.external_vpc_id == "" ? 0 : length(var.external_worker_subnets)}" - id = "${var.external_worker_subnets[count.index]}" -} - -data "aws_subnet" "external_master" { - count = "${var.external_vpc_id == "" ? 0 : length(var.external_master_subnets)}" - id = "${var.external_master_subnets[count.index]}" -} diff --git a/modules/aws/vpc/variables.tf b/modules/aws/vpc/variables.tf index 2168795716..354605d5b1 100644 --- a/modules/aws/vpc/variables.tf +++ b/modules/aws/vpc/variables.tf @@ -1,11 +1,3 @@ -variable "master_az_count" { - type = "string" -} - -variable "worker_az_count" { - type = "string" -} - variable "cidr_block" { type = "string" } @@ -26,11 +18,11 @@ variable "external_vpc_id" { type = "string" } -variable "external_master_subnets" { +variable "external_master_subnet_ids" { type = "list" } -variable "external_worker_subnets" { +variable "external_worker_subnet_ids" { type = "list" } @@ -45,20 +37,14 @@ variable "enable_etcd_sg" { default = true } -variable "master_subnets" { - type = "list" -} - -variable "worker_subnets" { - type = "list" -} - -variable "master_azs" { - type = "list" +variable "new_master_subnet_configs" { + description = "{az_name = new_subnet_cidr}: Empty map means create new subnets in all availability zones in region with generated cidrs" + type = "map" } -variable "worker_azs" { - type = "list" +variable "new_worker_subnet_configs" { + description = "{az_name = new_subnet_cidr}: Empty map means create new subnets in all availability zones in region with generated cidrs" + type = "map" } variable "private_master_endpoints" { diff --git a/modules/aws/vpc/vpc-private.tf b/modules/aws/vpc/vpc-private.tf index 315077f1ba..95c860fff7 100644 --- a/modules/aws/vpc/vpc-private.tf +++ b/modules/aws/vpc/vpc-private.tf @@ -1,16 +1,16 @@ resource "aws_route_table" "private_routes" { - count = "${var.external_vpc_id == "" ? var.worker_az_count : 0}" + count = "${local.new_worker_az_count}" vpc_id = "${data.aws_vpc.cluster_vpc.id}" tags = "${merge(map( - "Name", "${var.cluster_name}-private-${data.aws_availability_zones.azs.names[count.index]}", + "Name","${var.cluster_name}-private-${local.new_worker_subnet_azs[count.index]}", "kubernetes.io/cluster/${var.cluster_name}", "shared", "tectonicClusterID", "${var.cluster_id}" ), var.extra_tags)}" } resource "aws_route" "to_nat_gw" { - count = "${var.external_vpc_id == "" ? var.worker_az_count : 0}" + count = "${local.new_worker_az_count}" route_table_id = "${aws_route_table.private_routes.*.id[count.index]}" destination_cidr_block = "0.0.0.0/0" nat_gateway_id = "${element(aws_nat_gateway.nat_gw.*.id, count.index)}" @@ -18,29 +18,26 @@ resource "aws_route" "to_nat_gw" { } resource "aws_subnet" "worker_subnet" { - count = "${var.external_vpc_id == "" ? var.worker_az_count : 0}" + count = "${local.new_worker_az_count}" vpc_id = "${data.aws_vpc.cluster_vpc.id}" - cidr_block = "${length(var.worker_subnets) > 1 ? - "${element(var.worker_subnets, count.index)}" : - "${cidrsubnet(data.aws_vpc.cluster_vpc.cidr_block, 4, count.index + var.worker_az_count)}" - }" - - availability_zone = "${var.worker_azs[count.index]}" + cidr_block = "${lookup(var.new_worker_subnet_configs, + local.new_worker_subnet_azs[count.index], + cidrsubnet(local.new_worker_cidr_range, 3, count.index), + )}" tags = "${merge(map( - "Name", "${var.cluster_name}-worker-${ "${length(var.worker_azs)}" > 0 ? - "${var.worker_azs[count.index]}" : - "${data.aws_availability_zones.azs.names[count.index]}" }", - "kubernetes.io/cluster/${var.cluster_name}", "shared", - "kubernetes.io/role/internal-elb", "", - "tectonicClusterID", "${var.cluster_id}" - ), var.extra_tags)}" + "Name", "${var.cluster_name}-worker-${local.new_worker_subnet_azs[count.index]}", + "kubernetes.io/cluster/${var.cluster_name}","shared", + "kubernetes.io/role/internal-elb", "", + "tectonicClusterID", "${var.cluster_id}", + ), + var.extra_tags)}" } resource "aws_route_table_association" "worker_routing" { - count = "${var.external_vpc_id == "" ? var.worker_az_count : 0}" + count = "${local.new_worker_az_count}" route_table_id = "${aws_route_table.private_routes.*.id[count.index]}" subnet_id = "${aws_subnet.worker_subnet.*.id[count.index]}" } diff --git a/modules/aws/vpc/vpc-public.tf b/modules/aws/vpc/vpc-public.tf index 8f5c0d013c..8eb13ab4fb 100644 --- a/modules/aws/vpc/vpc-public.tf +++ b/modules/aws/vpc/vpc-public.tf @@ -1,5 +1,5 @@ resource "aws_internet_gateway" "igw" { - count = "${var.external_vpc_id == "" ? 1 : 0}" + count = "${local.external_vpc_mode ? 0 : 1}" vpc_id = "${data.aws_vpc.cluster_vpc.id}" tags = "${merge(map( @@ -16,52 +16,49 @@ resource "aws_route_table" "default" { tags = "${merge(map( "Name", "${var.cluster_name}-public", "kubernetes.io/cluster/${var.cluster_name}", "shared", - "tectonicClusterID", "${var.cluster_id}" + "tectonicClusterID", "${var.cluster_id}", ), var.extra_tags)}" } resource "aws_main_route_table_association" "main_vpc_routes" { - count = "${var.external_vpc_id == "" ? 1 : 0}" + count = "${local.external_vpc_mode ? 0 : 1}" vpc_id = "${data.aws_vpc.cluster_vpc.id}" route_table_id = "${aws_route_table.default.id}" } resource "aws_route" "igw_route" { - count = "${var.external_vpc_id == "" ? 1 : 0}" + count = "${local.external_vpc_mode ? 0 : 1}" destination_cidr_block = "0.0.0.0/0" route_table_id = "${aws_route_table.default.id}" gateway_id = "${aws_internet_gateway.igw.id}" } resource "aws_subnet" "master_subnet" { - count = "${var.external_vpc_id == "" ? var.master_az_count : 0}" - + count = "${local.new_master_az_count}" vpc_id = "${data.aws_vpc.cluster_vpc.id}" - cidr_block = "${length(var.master_subnets) > 1 ? - "${element(var.master_subnets, count.index)}" : - "${cidrsubnet(data.aws_vpc.cluster_vpc.cidr_block, 4, count.index)}" - }" + cidr_block = "${lookup(var.new_master_subnet_configs, + local.new_master_subnet_azs[count.index], + cidrsubnet(local.new_master_cidr_range, 3, count.index), + )}" - availability_zone = "${var.master_azs[count.index]}" + availability_zone = "${local.new_master_subnet_azs[count.index]}" tags = "${merge(map( - "Name", "${var.cluster_name}-master-${ "${length(var.master_azs)}" > 0 ? - "${var.master_azs[count.index]}" : - "${data.aws_availability_zones.azs.names[count.index]}" }", + "Name", "${var.cluster_name}-master-${local.new_master_subnet_azs[count.index]}", "kubernetes.io/cluster/${var.cluster_name}", "shared", "tectonicClusterID", "${var.cluster_id}" ), var.extra_tags)}" } resource "aws_route_table_association" "route_net" { - count = "${var.external_vpc_id == "" ? var.master_az_count : 0}" + count = "${local.new_master_az_count}" route_table_id = "${aws_route_table.default.id}" subnet_id = "${aws_subnet.master_subnet.*.id[count.index]}" } resource "aws_eip" "nat_eip" { - count = "${var.external_vpc_id == "" ? min(var.master_az_count, var.worker_az_count) : 0}" + count = "${min(local.new_master_az_count,local.new_worker_az_count)}" vpc = true # Terraform does not declare an explicit dependency towards the internet gateway. @@ -71,7 +68,7 @@ resource "aws_eip" "nat_eip" { } resource "aws_nat_gateway" "nat_gw" { - count = "${var.external_vpc_id == "" ? min(var.master_az_count, var.worker_az_count) : 0}" + count = "${min(local.new_master_az_count,local.new_worker_az_count)}" allocation_id = "${aws_eip.nat_eip.*.id[count.index]}" subnet_id = "${aws_subnet.master_subnet.*.id[count.index]}" } diff --git a/modules/aws/vpc/vpc.tf b/modules/aws/vpc/vpc.tf index 425a5db113..7f112e365b 100644 --- a/modules/aws/vpc/vpc.tf +++ b/modules/aws/vpc/vpc.tf @@ -1,4 +1,7 @@ -data "aws_availability_zones" "azs" {} +locals { + new_worker_cidr_range = "${cidrsubnet(data.aws_vpc.cluster_vpc.cidr_block,1,1)}" + new_master_cidr_range = "${cidrsubnet(data.aws_vpc.cluster_vpc.cidr_block,1,0)}" +} resource "aws_vpc" "new_vpc" { count = "${var.external_vpc_id == "" ? 1 : 0}" @@ -12,19 +15,3 @@ resource "aws_vpc" "new_vpc" { "tectonicClusterID", "${var.cluster_id}" ), var.extra_tags)}" } - -data "aws_vpc" "cluster_vpc" { - # The join() hack is required because currently the ternary operator - # evaluates the expressions on both branches of the condition before - # returning a value. When providing and external VPC, the template VPC - # resource gets a count of zero which triggers an evaluation error. - # - # This is tracked upstream: https://github.com/hashicorp/hil/issues/50 - # - id = "${var.external_vpc_id == "" ? join(" ", aws_vpc.new_vpc.*.id) : var.external_vpc_id }" -} - -locals { - master_subnet_ids = ["${split(",", var.external_vpc_id == "" ? join(",", aws_subnet.master_subnet.*.id) : join(",", data.aws_subnet.external_master.*.id))}"] - worker_subnet_ids = ["${split(",", var.external_vpc_id == "" ? join(",", aws_subnet.worker_subnet.*.id) : join(",", data.aws_subnet.external_worker.*.id))}"] -} diff --git a/platforms/govcloud/main.tf b/platforms/govcloud/main.tf index 2595973f7c..982106cc6f 100644 --- a/platforms/govcloud/main.tf +++ b/platforms/govcloud/main.tf @@ -21,50 +21,24 @@ module "container_linux" { module "vpc" { source = "../../modules/aws/vpc" - base_domain = "${var.tectonic_base_domain}" - cidr_block = "${var.tectonic_govcloud_vpc_cidr_block}" - cluster_id = "${module.tectonic.cluster_id}" - cluster_name = "${var.tectonic_cluster_name}" - custom_dns_name = "${var.tectonic_dns_name}" - enable_etcd_sg = "${length(compact(var.tectonic_etcd_servers)) == 0 ? 1 : 0}" - external_master_subnets = "${compact(var.tectonic_govcloud_external_master_subnet_ids)}" - external_vpc_id = "${var.tectonic_govcloud_external_vpc_id}" - external_worker_subnets = "${compact(var.tectonic_govcloud_external_worker_subnet_ids)}" - extra_tags = "${var.tectonic_govcloud_extra_tags}" - private_master_endpoints = true - public_master_endpoints = false + base_domain = "${var.tectonic_base_domain}" + cidr_block = "${var.tectonic_govcloud_vpc_cidr_block}" + cluster_id = "${module.tectonic.cluster_id}" + cluster_name = "${var.tectonic_cluster_name}" + custom_dns_name = "${var.tectonic_dns_name}" + enable_etcd_sg = "${length(compact(var.tectonic_etcd_servers)) == 0 ? 1 : 0}" + external_vpc_id = "${var.tectonic_govcloud_external_vpc_id}" + + external_master_subnet_ids = "${compact(var.tectonic_govcloud_external_master_subnet_ids)}" + external_worker_subnet_ids = "${compact(var.tectonic_govcloud_external_worker_subnet_ids)}" + extra_tags = "${var.tectonic_govcloud_extra_tags}" - # VPC layout settings. - # - # The following parameters control the layout of the VPC accross availability zones. - # Two modes are available: - # A. Explicitly configure a list of AZs + associated subnet CIDRs - # B. Let the module calculate subnets accross a set number of AZs - # - # To enable mode A, configure a set of AZs + CIDRs for masters and workers using the - # "tectonic_govcloud_master_custom_subnets" and "tectonic_govcloud_worker_custom_subnets" variables. - # - # To enable mode B, make sure that "tectonic_govcloud_master_custom_subnets" and "tectonic_govcloud_worker_custom_subnets" - # ARE NOT SET. + // empty map subnet_configs will have the vpc module creating subnets in all availabile AZs + new_master_subnet_configs = "${var.tectonic_govcloud_master_custom_subnets}" + new_worker_subnet_configs = "${var.tectonic_govcloud_worker_custom_subnets}" - # These counts could be deducted by length(keys(var.tectonic_govcloud_master_custom_subnets)) - # but there is a restriction on passing computed values as counts. This approach works around that. - master_az_count = "${length(keys(var.tectonic_govcloud_master_custom_subnets)) > 0 ? "${length(keys(var.tectonic_govcloud_master_custom_subnets))}" : "${length(data.aws_availability_zones.azs.names)}"}" - worker_az_count = "${length(keys(var.tectonic_govcloud_worker_custom_subnets)) > 0 ? "${length(keys(var.tectonic_govcloud_worker_custom_subnets))}" : "${length(data.aws_availability_zones.azs.names)}"}" - # The appending of the "padding" element is required as workaround since the function - # element() won't work on empty lists. See https://github.com/hashicorp/terraform/issues/11210 - master_subnets = "${concat(values(var.tectonic_govcloud_master_custom_subnets),list("padding"))}" - worker_subnets = "${concat(values(var.tectonic_govcloud_worker_custom_subnets),list("padding"))}" - # The split() / join() trick works around the limitation of ternary operator expressions - # only being able to return strings. - master_azs = "${ split("|", "${length(keys(var.tectonic_govcloud_master_custom_subnets))}" > 0 ? - join("|", keys(var.tectonic_govcloud_master_custom_subnets)) : - join("|", data.aws_availability_zones.azs.names) - )}" - worker_azs = "${ split("|", "${length(keys(var.tectonic_govcloud_worker_custom_subnets))}" > 0 ? - join("|", keys(var.tectonic_govcloud_worker_custom_subnets)) : - join("|", data.aws_availability_zones.azs.names) - )}" + private_master_endpoints = true + public_master_endpoints = false } module "etcd" { diff --git a/platforms/govcloud/variables.tf b/platforms/govcloud/variables.tf index 8918bb8b71..e7cf25eb4f 100644 --- a/platforms/govcloud/variables.tf +++ b/platforms/govcloud/variables.tf @@ -140,7 +140,7 @@ variable "tectonic_govcloud_external_master_subnet_ids" { description = < 0 ? - join("|", keys(var.tectonic_aws_master_custom_subnets)) : - join("|", data.aws_availability_zones.azs.names) - )}" - worker_azs = "${ split("|", "${length(keys(var.tectonic_aws_worker_custom_subnets))}" > 0 ? - join("|", keys(var.tectonic_aws_worker_custom_subnets)) : - join("|", data.aws_availability_zones.azs.names) - )}" } module "etcd" { diff --git a/steps/variables.tf b/steps/variables.tf index 51ce789497..75f0434cd5 100644 --- a/steps/variables.tf +++ b/steps/variables.tf @@ -151,7 +151,7 @@ variable "tectonic_aws_external_master_subnet_ids" { description = <