Skip to content

Commit e8f570e

Browse files
committed
fix: only setup LB discovery tags for in-use subnets (#147)
<!-- ~ Copyright 2023 StreamNative, Inc. ~ ~ 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. --> <!-- ### Contribution Checklist - Fill out the template below to describe the changes contributed by the pull request. That will give reviewers the context they need to do the review. - Each pull request should address only one issue, not mix up code from multiple issues. - Each commit in the pull request has a meaningful commit message - Once all items of the checklist are addressed, remove the above text and this checklist, leaving only the filled out template below. **(The sections below can be removed for hotfixes of typos)** --> *(If this PR fixes a github issue, please add `Fixes #<xyz>`.)* Fixes #<xyz> *(or if this PR is one task of a github issue, please add `Master Issue: #<xyz>` to link to the master issue.)* Master Issue: #<xyz> ### Motivation - Avoid unnecessary inter-zone traffic from LB - Allow disabling cross-zone load balancing when specifying zones ### Modifications - Remove duplicated tag setup in VPC module - Only setup LB discovery tags for subnet will be used by nodepool ### Verifying this change - [ ] Make sure that the change passes the CI checks. *(Please pick either of the following options)* This change is a trivial rework / code cleanup without any test coverage. *(or)* This change is already covered by existing tests, such as *(please describe tests)*. *(or)* This change added tests and can be verified as follows: *(example:)* - *Added integration tests for end-to-end deployment with large payloads (10MB)* - *Extended integration test for recovery after broker failure* ### Documentation Check the box below. Need to update docs? - [ ] `doc-required` (If you need help on updating docs, create a doc issue) - [x] `no-need-doc` (Please explain why) - [ ] `doc` (If this PR contains doc changes)
1 parent f4c4d04 commit e8f570e

File tree

4 files changed

+29
-6
lines changed

4 files changed

+29
-6
lines changed

main.tf

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -268,6 +268,7 @@ module "vpc_tags" {
268268
vpc_id = var.vpc_id
269269
public_subnet_ids = var.public_subnet_ids
270270
private_subnet_ids = var.private_subnet_ids
271+
node_pool_azs = var.node_pool_azs
271272
}
272273

273274
resource "aws_ec2_tag" "cluster_security_group" {

modules/eks-vpc-tags/main.tf

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,24 @@
1212
# See the License for the specific language governing permissions and
1313
# limitations under the License.
1414

15+
data "aws_subnet" "private_subnets" {
16+
count = length(var.private_subnet_ids)
17+
id = var.private_subnet_ids[count.index]
18+
}
19+
20+
data "aws_subnet" "public_subnets" {
21+
count = length(var.public_subnet_ids)
22+
id = var.public_subnet_ids[count.index]
23+
}
24+
1525
locals {
1626
cluster_subnet_ids = concat(var.private_subnet_ids, var.public_subnet_ids)
27+
node_pool_private_subnets = [
28+
for subnet in data.aws_subnet.private_subnets : subnet.id if(length(var.node_pool_azs) == 0 || contains(var.node_pool_azs, subnet.availability_zone))
29+
]
30+
node_pool_public_subnets = [
31+
for subnet in data.aws_subnet.public_subnets : subnet.id if(length(var.node_pool_azs) == 0 || contains(var.node_pool_azs, subnet.availability_zone))
32+
]
1733
}
1834

1935
resource "aws_ec2_tag" "vpc_tag" {
@@ -30,15 +46,15 @@ resource "aws_ec2_tag" "cluster_subnet_tag" {
3046
}
3147

3248
resource "aws_ec2_tag" "private_subnet_tag" {
33-
count = length(var.private_subnet_ids)
34-
resource_id = var.private_subnet_ids[count.index]
49+
count = length(local.node_pool_private_subnets)
50+
resource_id = local.node_pool_private_subnets[count.index]
3551
key = "kubernetes.io/role/internal-elb"
3652
value = "1"
3753
}
3854

3955
resource "aws_ec2_tag" "public_subnet_tag" {
40-
count = length(var.public_subnet_ids)
41-
resource_id = var.public_subnet_ids[count.index]
56+
count = length(local.node_pool_public_subnets)
57+
resource_id = local.node_pool_public_subnets[count.index]
4258
key = "kubernetes.io/role/elb"
4359
value = "1"
4460
}

modules/eks-vpc-tags/variables.tf

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,4 +56,10 @@ variable "vpc_id" {
5656
condition = length(var.vpc_id) > 4 && substr(var.vpc_id, 0, 4) == "vpc-"
5757
error_message = "The value for variable \"vpc_id\" must be a valid VPC id, starting with \"vpc-\"."
5858
}
59+
}
60+
61+
variable "node_pool_azs" {
62+
type = list(string)
63+
description = "A list of availability zones to use for the EKS node group. If not set, the module will use the same availability zones with the cluster."
64+
default = []
5965
}

modules/vpc/main.tf

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ resource "aws_subnet" "public" {
3939
cidr_block = cidrsubnet(var.vpc_cidr, var.public_subnet_newbits, var.public_subnet_start + count.index)
4040
availability_zone = local.azs[count.index]
4141
map_public_ip_on_launch = var.disable_nat_gateway ? true : var.public_subnet_auto_ip
42-
tags = merge({ "Vendor" = "StreamNative", "Type" = "public", "kubernetes.io/role/elb" = "1", Name = format("%s-public-sbn-%s", var.vpc_name, count.index) }, var.tags)
42+
tags = merge({ "Vendor" = "StreamNative", "Type" = "public", Name = format("%s-public-sbn-%s", var.vpc_name, count.index) }, var.tags)
4343

4444
lifecycle {
4545
ignore_changes = [tags]
@@ -52,7 +52,7 @@ resource "aws_subnet" "private" {
5252
vpc_id = aws_vpc.vpc.id
5353
cidr_block = cidrsubnet(var.vpc_cidr, var.private_subnet_newbits, var.private_subnet_start + count.index)
5454
availability_zone = local.azs[count.index]
55-
tags = merge({ "Vendor" = "StreamNative", "Type" = "private", "kubernetes.io/role/internal-elb" = "1", Name = format("%s-private-sbn-%s", var.vpc_name, count.index) }, var.tags)
55+
tags = merge({ "Vendor" = "StreamNative", "Type" = "private", Name = format("%s-private-sbn-%s", var.vpc_name, count.index) }, var.tags)
5656

5757
lifecycle {
5858
ignore_changes = [tags]

0 commit comments

Comments
 (0)