-
-
Notifications
You must be signed in to change notification settings - Fork 322
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
AWS NLB - controllers #136
Conversation
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.
Thanks. I explored this in November, but there were bugs in the AWS provider causing issues. What is the minimum version of the AWS provider you've evaluated this against?
} | ||
} | ||
|
||
resource "aws_lb_listener" "apiserver-443" { |
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.
Resource naming convention would be apiserver-https if you look at other places in the project.
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'll provide a change soon'ish. probably tomorrow morning.
resource "aws_lb" "apiserver" { | ||
name = "${var.cluster_name}-apiserver" | ||
load_balancer_type = "network" | ||
subnets = ["${aws_subnet.public.*.id}"] |
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.
You've removed the security group
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.
Disregard, I see network load balancer doesn't allow it.
|
||
# Kubelet HTTP health check | ||
health_check { | ||
target = "SSL:443" | ||
protocol = "TCP" |
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.
Intentional change?
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.
Yes. Required to get the healthcheck working with nlb's.
} | ||
|
||
resource "aws_lb_target_group_attachment" "apiserver" { |
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.
"apiservers" or better yet "controllers" would match the established patterns
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'll provide a fix soon changing it to controllers.
In answer to your question:
My current setup is.
|
* Renames lb. Changes suffix from 443 to https. * Renames target-group-attachment to "controllers".
The new minimum version for the terraform-provider-aws plugin is 1.7.0, as established in #141. You won't have to worry about it after all, since that's already made its way to master. |
Squashed and merged with a few minor tweaks in ceb5555. Thanks! |
Controller load balancer set to an AWS NLB
Testing
deployed/destroyed/applied all addons.
ref: #130