From 6318e90128b9a1d3495b8e39e9e7742b19a00b0a Mon Sep 17 00:00:00 2001 From: Peter Rifel Date: Thu, 29 Oct 2020 12:13:36 -0500 Subject: [PATCH] Ignore changes reported by subsequent updates Usually this is an "actual.Foo = e.Foo" one-liner but we don't know which LB attached to an ASG is the API ELB so it's a bit more complicated --- pkg/model/awsmodel/autoscalinggroup.go | 20 +++++----- .../fi/cloudup/awstasks/autoscalinggroup.go | 40 +++++++++++++++++++ 2 files changed, 49 insertions(+), 11 deletions(-) diff --git a/pkg/model/awsmodel/autoscalinggroup.go b/pkg/model/awsmodel/autoscalinggroup.go index 0cc8d7e338..1587ec7733 100644 --- a/pkg/model/awsmodel/autoscalinggroup.go +++ b/pkg/model/awsmodel/autoscalinggroup.go @@ -364,25 +364,23 @@ func (b *AutoscalingGroupModelBuilder) buildAutoScalingGroupTask(c *fi.ModelBuil for _, extLB := range ig.Spec.ExternalLoadBalancers { if extLB.LoadBalancerName != nil { - t.LoadBalancers = append(t.LoadBalancers, &awstasks.LoadBalancer{ + lb := &awstasks.LoadBalancer{ Name: extLB.LoadBalancerName, LoadBalancerName: extLB.LoadBalancerName, - }) - - c.AddTask(&awstasks.LoadBalancer{ - Name: extLB.LoadBalancerName, - Shared: fi.Bool(true), - }) + Shared: fi.Bool(true), + } + t.LoadBalancers = append(t.LoadBalancers, lb) + c.AddTask(lb) } if extLB.TargetGroupARN != nil { - t.TargetGroups = append(t.TargetGroups, &awstasks.TargetGroup{Name: extLB.TargetGroupARN, ARN: extLB.TargetGroupARN}) - - c.AddTask(&awstasks.TargetGroup{ + tg := &awstasks.TargetGroup{ Name: extLB.TargetGroupARN, ARN: extLB.TargetGroupARN, Shared: fi.Bool(true), - }) + } + t.TargetGroups = append(t.TargetGroups, tg) + c.AddTask(tg) } } diff --git a/upup/pkg/fi/cloudup/awstasks/autoscalinggroup.go b/upup/pkg/fi/cloudup/awstasks/autoscalinggroup.go index 69506fb4d1..634eea0c7d 100644 --- a/upup/pkg/fi/cloudup/awstasks/autoscalinggroup.go +++ b/upup/pkg/fi/cloudup/awstasks/autoscalinggroup.go @@ -32,6 +32,7 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/autoscaling" "github.com/aws/aws-sdk-go/service/ec2" + "github.com/aws/aws-sdk-go/service/elb" "k8s.io/klog/v2" ) @@ -124,6 +125,45 @@ func (e *AutoscalingGroup) Find(c *fi.Context) (*AutoscalingGroup, error) { }) } + { + // pkg/model/awsmodel/autoscalinggroup.go doesn't know the LoadBalancerName of the API ELB task that it passes to the master ASGs, + // it only knows the LoadBalancerName of external load balancers passed through the InstanceGroupSpec. + // We lookup the LoadBalancerName for LoadBalancer tasks that don't have it set in order to attach the LB to the ASG. + // + // This means some LoadBalancer tasks have LoadBalancerName and others do not. + // When `Find`ing the ASG and recreating the LoadBalancer tasks we need them to match how the model creates them, + // but we only know the LoadBalancerNames, not the task names associated with them. + // This reuslts in spurious changes being reported during subsequent `update cluster` runs because the API ELB task is named differently + // between the kops model and the ASG's `Find`. + // + // To prevent this, we need to update the API ELB task in the ASG's LoadBalancers list. + // Because we don't know whether any given LoadBalancerName attached to an ASG is the API ELB task or not, + // we have to find the API ELB task, lookup its LoadBalancerName, and then compare that to the list of attached LoadBalancers. + var apiLBTask *LoadBalancer + var apiLBDesc *elb.LoadBalancerDescription + for _, lb := range e.LoadBalancers { + // All external ELBs have their Shared field set to true. The API ELB does not. + // Note that Shared is set by the kops model rather than AWS tags. + if !fi.BoolValue(lb.Shared) { + apiLBTask = lb + } + } + if apiLBTask != nil && len(actual.LoadBalancers) > 0 { + apiLBDesc, err = FindLoadBalancerByNameTag(c.Cloud.(awsup.AWSCloud), fi.StringValue(apiLBTask.Name)) + if err != nil { + return nil, err + } + if apiLBDesc != nil { + for i := 0; i < len(actual.LoadBalancers); i++ { + lb := actual.LoadBalancers[i] + if aws.StringValue(apiLBDesc.LoadBalancerName) == aws.StringValue(lb.Name) { + actual.LoadBalancers[i] = apiLBTask + } + } + } + } + } + for _, tg := range g.TargetGroupARNs { actual.TargetGroups = append(actual.TargetGroups, &TargetGroup{ARN: aws.String(*tg)}) }