Skip to content

Commit

Permalink
modules/aws/vpc/vpc-public: Tag new EIPs with tectonicClusterID
Browse files Browse the repository at this point in the history
The tags I'm adding lack Name and
kubernetes.io/cluster/${var.cluster_name}, which are part of the
tagging for most other resources created in this module.  But Chance,
pointed out that Kubernetes doesn't manage EIPs, so we don't need to
set the kubernetes.io tags at all.  And naming EIPs doesn't add much
information; after setup completes you can get all the naming
information you need from the associated NAT gateway.

Setting the cluster ID makes it easier for us to reap leaked resources
after tearing down a cluster (for example, if a test which creates and
deletes a cluster is evicted before completing deletion).  And passing
extra_tags through allows us to set expiration dates and such for our
new resources, which will also help with cleanup.

Not related to this commit, but for future reference Chance elaborated
on the use of 'shared' for VPC-related resource.  Those are from
e4cd883a (modules/aws/vpc: set shared rather than owned for
VPC/Subnets, 2017-05-03, coreos/tectonic-installer#469), and Chance's
use-case is:

1. User creates a new cluster with the installer, not thinking about
   VPC reuse.
2. User creates a second cluster sharing the same VPC and subnets.

Ideally those users would create the VPCs and subnets externally with
shared tags and pass them through to the installer by ID.  Then it
would be clear that the installer did not create them, and you could
safely delete either cluster without affecting the shared resources.
But for users without the foresight to create shared VPCs, having
terraform create "shared" VPCs directly gives you similar results (as
long as you don't delete the first cluster and break everything ;).
  • Loading branch information
wking committed Jul 26, 2018
1 parent c6c6bf0 commit e297548
Showing 1 changed file with 4 additions and 0 deletions.
4 changes: 4 additions & 0 deletions modules/aws/vpc/vpc-public.tf
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,10 @@ resource "aws_eip" "nat_eip" {
count = "${min(local.new_master_az_count,local.new_worker_az_count)}"
vpc = true

tags = "${merge(map(
"tectonicClusterID", "${var.cluster_id}"
), var.extra_tags)}"

# Terraform does not declare an explicit dependency towards the internet gateway.
# this can cause the internet gateway to be deleted/detached before the EIPs.
# https://github.com/coreos/tectonic-installer/issues/1017#issuecomment-307780549
Expand Down

0 comments on commit e297548

Please sign in to comment.