diff --git a/pkg/model/awsmodel/api_loadbalancer.go b/pkg/model/awsmodel/api_loadbalancer.go index cca9fddd89..2e4439665c 100644 --- a/pkg/model/awsmodel/api_loadbalancer.go +++ b/pkg/model/awsmodel/api_loadbalancer.go @@ -109,14 +109,13 @@ func (b *APILoadBalancerBuilder) Build(c *fi.ModelBuilderContext) error { "443": {InstancePort: 443}, } - nlbListenerPort := "443" - nlbListeners := map[string]*awstasks.NetworkLoadBalancerListener{ - nlbListenerPort: {Port: 443}, + nlbListeners := []*awstasks.NetworkLoadBalancerListener{ + {Port: 443}, } if lbSpec.SSLCertificate != "" { listeners["443"].SSLCertificateID = lbSpec.SSLCertificate - nlbListeners["443"].SSLCertificateID = lbSpec.SSLCertificate + nlbListeners[0].SSLCertificateID = lbSpec.SSLCertificate } if lbSpec.SecurityGroupOverride != nil { @@ -138,6 +137,7 @@ func (b *APILoadBalancerBuilder) Build(c *fi.ModelBuilderContext) error { LoadBalancerName: fi.String(loadBalancerName), Subnets: elbSubnets, Listeners: nlbListeners, + TargetGroups: make([]*awstasks.TargetGroup, 0), Tags: tags, VPC: b.LinkToVPC(), @@ -196,19 +196,18 @@ func (b *APILoadBalancerBuilder) Build(c *fi.ModelBuilderContext) error { c.AddTask(clb) } else if b.APILoadBalancerClass() == kops.LoadBalancerClassNetwork { - targetGroupPort := fi.Int64(443) targetGroupName := b.NLBTargetGroupName("api") - tags := b.CloudTags(targetGroupName, false) + primaryTags := b.CloudTags(targetGroupName, false) // Override the returned name to be the expected NLB TG name - tags["Name"] = targetGroupName + primaryTags["Name"] = targetGroupName tg := &awstasks.TargetGroup{ Name: fi.String(targetGroupName), VPC: b.LinkToVPC(), - Tags: tags, + Tags: primaryTags, Protocol: fi.String("TCP"), - Port: targetGroupPort, + Port: fi.Int64(443), HealthyThreshold: fi.Int64(2), UnhealthyThreshold: fi.Int64(2), Shared: fi.Bool(false), @@ -216,7 +215,7 @@ func (b *APILoadBalancerBuilder) Build(c *fi.ModelBuilderContext) error { c.AddTask(tg) - nlb.TargetGroup = tg + nlb.TargetGroups = append(nlb.TargetGroups, tg) c.AddTask(nlb) } @@ -291,7 +290,6 @@ func (b *APILoadBalancerBuilder) Build(c *fi.ModelBuilderContext) error { for _, masterGroup := range masterGroups { t := &awstasks.SecurityGroupRule{ - // TODO: figure out how to only add this to one SG (GetSecurityGroups above returns multiple) Name: fi.String(fmt.Sprintf("https-api-elb-%s", cidr)), Lifecycle: b.SecurityLifecycle, CIDR: fi.String(cidr), @@ -304,7 +302,6 @@ func (b *APILoadBalancerBuilder) Build(c *fi.ModelBuilderContext) error { // Allow ICMP traffic required for PMTU discovery c.AddTask(&awstasks.SecurityGroupRule{ - // TODO: figure out how to only add this to one SG (GetSecurityGroups above returns multiple) Name: fi.String("icmp-pmtu-api-elb-" + cidr), Lifecycle: b.SecurityLifecycle, CIDR: fi.String(cidr), diff --git a/upup/pkg/fi/cloudup/awstasks/autoscalinggroup.go b/upup/pkg/fi/cloudup/awstasks/autoscalinggroup.go index e1575639d7..130d27c989 100644 --- a/upup/pkg/fi/cloudup/awstasks/autoscalinggroup.go +++ b/upup/pkg/fi/cloudup/awstasks/autoscalinggroup.go @@ -163,12 +163,11 @@ func (e *AutoscalingGroup) Find(c *fi.Context) (*AutoscalingGroup, error) { } if len(g.TargetGroupARNs) > 0 { - targetGroups := make([]*TargetGroup, 0) + actualTGs := make([]*TargetGroup, 0) for _, tg := range g.TargetGroupARNs { - targetGroups = append(actual.TargetGroups, &TargetGroup{ARN: aws.String(*tg)}) + actualTGs = append(actualTGs, &TargetGroup{ARN: aws.String(*tg)}) } - - targetGroups, err := ReconcileTargetGroups(c.Cloud.(awsup.AWSCloud), targetGroups, e.TargetGroups) + targetGroups, err := ReconcileTargetGroups(c.Cloud.(awsup.AWSCloud), actualTGs, e.TargetGroups) if err != nil { return nil, err } diff --git a/upup/pkg/fi/cloudup/awstasks/network_load_balancer.go b/upup/pkg/fi/cloudup/awstasks/network_load_balancer.go index 33affe89fc..2f6bd5192c 100644 --- a/upup/pkg/fi/cloudup/awstasks/network_load_balancer.go +++ b/upup/pkg/fi/cloudup/awstasks/network_load_balancer.go @@ -55,7 +55,7 @@ type NetworkLoadBalancer struct { Subnets []*Subnet - Listeners map[string]*NetworkLoadBalancerListener + Listeners []*NetworkLoadBalancerListener Scheme *string @@ -66,8 +66,8 @@ type NetworkLoadBalancer struct { Type *string - VPC *VPC - TargetGroup *TargetGroup + VPC *VPC + TargetGroups []*TargetGroup } var _ fi.CompareWithID = &NetworkLoadBalancer{} @@ -113,6 +113,16 @@ func (e *NetworkLoadBalancerListener) GetDependencies(tasks map[string]fi.Task) return nil } + +// OrderListenersByPort implements sort.Interface for []OrderTargetGroupsByPort, based on port number +type OrderListenersByPort []*NetworkLoadBalancerListener + +func (a OrderListenersByPort) Len() int { return len(a) } +func (a OrderListenersByPort) Swap(i, j int) { a[i], a[j] = a[j], a[i] } +func (a OrderListenersByPort) Less(i, j int) bool { + return a[i].Port < a[j].Port +} + //The load balancer name 'api.renamenlbcluster.k8s.local' can only contain characters that are alphanumeric characters and hyphens(-)\n\tstatus code: 400, func findNetworkLoadBalancerByLoadBalancerName(cloud awsup.AWSCloud, loadBalancerName string) (*elbv2.LoadBalancer, error) { request := &elbv2.DescribeLoadBalancersInput{ @@ -317,6 +327,7 @@ func (e *NetworkLoadBalancer) Find(c *fi.Context) (*NetworkLoadBalancer, error) actual.Scheme = lb.Scheme actual.VPC = &VPC{ID: lb.VpcId} actual.Type = lb.Type + actual.TargetGroups = make([]*TargetGroup, 0) tagMap, err := describeNetworkLoadBalancerTags(cloud, []string{*loadBalancerArn}) if err != nil { @@ -343,35 +354,30 @@ func (e *NetworkLoadBalancer) Find(c *fi.Context) (*NetworkLoadBalancer, error) return nil, fmt.Errorf("error querying for NLB listeners :%v", err) } - actual.Listeners = make(map[string]*NetworkLoadBalancerListener) + actual.Listeners = make([]*NetworkLoadBalancerListener, 0) for _, l := range response.Listeners { - loadBalancerPort := strconv.FormatInt(aws.Int64Value(l.Port), 10) - actualListener := &NetworkLoadBalancerListener{} actualListener.Port = int(aws.Int64Value(l.Port)) if len(l.Certificates) != 0 { actualListener.SSLCertificateID = aws.StringValue(l.Certificates[0].CertificateArn) // What if there is more then one certificate, can we just grab the default certificate? we don't set it as default, we only set the one. } - actual.Listeners[loadBalancerPort] = actualListener + actual.Listeners = append(actual.Listeners, actualListener) // This will need to be rearranged when we recognized multiple listeners and target groups per NLB if len(l.DefaultActions) > 0 { targetGroupARN := l.DefaultActions[0].TargetGroupArn if targetGroupARN != nil { - actual.TargetGroup = &TargetGroup{ARN: targetGroupARN} + actual.TargetGroups = append(actual.TargetGroups, &TargetGroup{ARN: targetGroupARN}) } } } - // This will be cleaned up once the NLB task supports multiple target groups - if actual.TargetGroup != nil { - actualTGs := []*TargetGroup{actual.TargetGroup} - expectedTGs := []*TargetGroup{e.TargetGroup} - targetGroups, err := ReconcileTargetGroups(c.Cloud.(awsup.AWSCloud), actualTGs, expectedTGs) + if len(actual.TargetGroups) > 0 { + targetGroups, err := ReconcileTargetGroups(c.Cloud.(awsup.AWSCloud), actual.TargetGroups, e.TargetGroups) if err != nil { return nil, err } - actual.TargetGroup = targetGroups[0] + actual.TargetGroups = targetGroups } } @@ -465,6 +471,8 @@ func (e *NetworkLoadBalancer) Run(c *fi.Context) error { func (e *NetworkLoadBalancer) Normalize() { // We need to sort our arrays consistently, so we don't get spurious changes sort.Stable(OrderSubnetsById(e.Subnets)) + sort.Stable(OrderListenersByPort(e.Listeners)) + sort.Stable(OrderTargetGroupsByPort(e.TargetGroups)) } func (s *NetworkLoadBalancer) CheckChanges(a, e, changes *NetworkLoadBalancer) error { @@ -493,12 +501,18 @@ func (_ *NetworkLoadBalancer) RenderAWS(t *awsup.AWSAPITarget, a, e, changes *Ne var loadBalancerName string var loadBalancerArn string + if len(e.Listeners) != len(e.TargetGroups) { + return fmt.Errorf("nlb listeners and target groups do not match: %v listeners vs %v target groups", len(e.Listeners), len(e.TargetGroups)) + } + if a == nil { if e.LoadBalancerName == nil { return fi.RequiredField("LoadBalancerName") } - if e.TargetGroup.ARN == nil { - return fmt.Errorf("missing required target group ARN for NLB creation %v", e.TargetGroup) + for _, tg := range e.TargetGroups { + if tg.ARN == nil { + return fmt.Errorf("missing required target group ARN for NLB creation %v", tg) + } } loadBalancerName = *e.LoadBalancerName @@ -532,8 +546,8 @@ func (_ *NetworkLoadBalancer) RenderAWS(t *awsup.AWSAPITarget, a, e, changes *Ne } { - for _, listener := range e.Listeners { - createListenerInput := listener.mapToAWS(*e.TargetGroup.ARN, loadBalancerArn) + for i, listener := range e.Listeners { + createListenerInput := listener.mapToAWS(*e.TargetGroups[i].ARN, loadBalancerArn) klog.V(2).Infof("Creating Listener for NLB") _, err := t.Cloud.ELBV2().CreateListener(createListenerInput) @@ -605,8 +619,8 @@ func (_ *NetworkLoadBalancer) RenderAWS(t *awsup.AWSAPITarget, a, e, changes *Ne } } - for _, listener := range changes.Listeners { - awsListener := listener.mapToAWS(*e.TargetGroup.ARN, loadBalancerArn) + for i, listener := range changes.Listeners { + awsListener := listener.mapToAWS(*e.TargetGroups[i].ARN, loadBalancerArn) klog.V(2).Infof("Creating Listener for NLB") _, err := t.Cloud.ELBV2().CreateListener(awsListener) @@ -674,18 +688,14 @@ func (_ *NetworkLoadBalancer) RenderTerraform(t *terraform.TerraformTarget, a, e return err } - for portStr, listener := range e.Listeners { - port, err := strconv.Atoi(portStr) - if err != nil { - return err - } + for i, listener := range e.Listeners { listenerTF := &terraformNetworkLoadBalancerListener{ LoadBalancer: e.TerraformLink(), - Port: int64(port), + Port: int64(listener.Port), DefaultAction: []terraformNetworkLoadBalancerListenerAction{ { Type: elbv2.ActionTypeEnumForward, - TargetGroupARN: e.TargetGroup.TerraformLink(), + TargetGroupARN: e.TargetGroups[i].TerraformLink(), }, }, } @@ -696,7 +706,7 @@ func (_ *NetworkLoadBalancer) RenderTerraform(t *terraform.TerraformTarget, a, e listenerTF.Protocol = elbv2.ProtocolEnumTcp } - err = t.RenderResource("aws_lb_listener", fmt.Sprintf("%v-%v", *e.Name, portStr), listenerTF) + err = t.RenderResource("aws_lb_listener", fmt.Sprintf("%v-%v", *e.Name, listener.Port), listenerTF) if err != nil { return err } @@ -753,18 +763,14 @@ func (_ *NetworkLoadBalancer) RenderCloudformation(t *cloudformation.Cloudformat return err } - for portStr, listener := range e.Listeners { - port, err := strconv.Atoi(portStr) - if err != nil { - return err - } + for i, listener := range e.Listeners { listenerCF := &cloudformationNetworkLoadBalancerListener{ LoadBalancerARN: e.CloudformationLink(), - Port: int64(port), + Port: int64(listener.Port), DefaultActions: []cloudformationNetworkLoadBalancerListenerAction{ { Type: elbv2.ActionTypeEnumForward, - TargetGroupARN: e.TargetGroup.CloudformationLink(), + TargetGroupARN: e.TargetGroups[i].CloudformationLink(), }, }, } @@ -775,7 +781,7 @@ func (_ *NetworkLoadBalancer) RenderCloudformation(t *cloudformation.Cloudformat listenerCF.Protocol = elbv2.ProtocolEnumTcp } - err = t.RenderResource("AWS::ElasticLoadBalancingV2::Listener", fmt.Sprintf("%v-%v", *e.Name, portStr), listenerCF) + err = t.RenderResource("AWS::ElasticLoadBalancingV2::Listener", fmt.Sprintf("%v-%v", *e.Name, listener.Port), listenerCF) if err != nil { return err } @@ -794,5 +800,4 @@ func (e *NetworkLoadBalancer) CloudformationAttrCanonicalHostedZoneNameID() *clo func (e *NetworkLoadBalancer) CloudformationAttrDNSName() *cloudformation.Literal { return cloudformation.GetAtt("AWS::ElasticLoadBalancingV2::LoadBalancer", *e.Name, "DNSName") - } diff --git a/upup/pkg/fi/cloudup/awstasks/targetgroup.go b/upup/pkg/fi/cloudup/awstasks/targetgroup.go index cd26112479..07d54ebcbf 100644 --- a/upup/pkg/fi/cloudup/awstasks/targetgroup.go +++ b/upup/pkg/fi/cloudup/awstasks/targetgroup.go @@ -191,6 +191,15 @@ func (_ *TargetGroup) RenderAWS(t *awsup.AWSAPITarget, a, e, changes *TargetGrou return nil } +// OrderTargetGroupsByPort implements sort.Interface for []OrderTargetGroupsByPort, based on port number +type OrderTargetGroupsByPort []*TargetGroup + +func (a OrderTargetGroupsByPort) Len() int { return len(a) } +func (a OrderTargetGroupsByPort) Swap(i, j int) { a[i], a[j] = a[j], a[i] } +func (a OrderTargetGroupsByPort) Less(i, j int) bool { + return fi.Int64Value(a[i].Port) < fi.Int64Value(a[j].Port) +} + // pkg/model/awsmodel doesn't know the ARN of the API TargetGroup tasks that it passes to the master ASGs, // it only knows the ARN of external target groups passed through the InstanceGroupSpec. // We lookup the ARN for TargetGroup tasks that don't have it set in order to attach the LB to the ASG. @@ -205,12 +214,12 @@ func (_ *TargetGroup) RenderAWS(t *awsup.AWSAPITarget, a, e, changes *TargetGrou // Because we don't know whether any given ARN attached to an ASG is an API TargetGroup task or not, // we have to find the API TargetGroup task, lookup its ARN, then compare that to the list of attached TargetGroups. func ReconcileTargetGroups(cloud awsup.AWSCloud, actual []*TargetGroup, expected []*TargetGroup) ([]*TargetGroup, error) { - var apiTGTask *TargetGroup + apiTGTasks := make([]*TargetGroup, 0) for _, tg := range expected { // All external TargetGroups have their Shared field set to true. The API TargetGroups do not. // Note that Shared is set by the kops model rather than AWS tags. if !fi.BoolValue(tg.Shared) { - apiTGTask = tg + apiTGTasks = append(apiTGTasks, tg) } } @@ -220,18 +229,17 @@ func ReconcileTargetGroups(cloud awsup.AWSCloud, actual []*TargetGroup, expected return reconciled, nil } - var apiTG *elbv2.TargetGroup - var err error - if apiTGTask != nil { - apiTG, err = FindTargetGroupByName(cloud, fi.StringValue(apiTGTask.Name)) + apiTGs := make(map[string]*TargetGroup) + for _, task := range apiTGTasks { + apiTG, err := FindTargetGroupByName(cloud, fi.StringValue(task.Name)) if err != nil { return nil, err } + apiTGs[aws.StringValue(apiTG.TargetGroupArn)] = task } - for i := 0; i < len(actual); i++ { - tg := actual[i] - if apiTG != nil && aws.StringValue(apiTG.TargetGroupArn) == aws.StringValue(tg.ARN) { - reconciled = append(reconciled, apiTGTask) + for _, tg := range actual { + if apiTask, ok := apiTGs[aws.StringValue(tg.ARN)]; ok { + reconciled = append(reconciled, apiTask) } else { reconciled = append(reconciled, tg) }