diff --git a/pkg/alb/lb/loadbalancer.go b/pkg/alb/lb/loadbalancer.go index 002b7c550..3cd3e1cb9 100644 --- a/pkg/alb/lb/loadbalancer.go +++ b/pkg/alb/lb/loadbalancer.go @@ -187,7 +187,7 @@ func NewCurrentLoadBalancer(o *NewCurrentLoadBalancerOptions) (newLoadBalancer * attrs, err := albelbv2.ELBV2svc.DescribeLoadBalancerAttributesFiltered(o.LoadBalancer.LoadBalancerArn) if err != nil { - return newLoadBalancer, fmt.Errorf("Failed to retrieve attributes from ALB in AWS. Error: %s", err.Error()) + return newLoadBalancer, fmt.Errorf("Failed to retrieve attributes from ELBV2 in AWS. Error: %s", err.Error()) } var managedSG *string @@ -542,7 +542,7 @@ func (l *LoadBalancer) modify(rOpts *ReconcileOptions) error { Subnets: util.AvailabilityZones(l.lb.desired.AvailabilityZones).AsSubnets(), }); err != nil { rOpts.Eventf(api.EventTypeWarning, "ERROR", "%s subnet modification failed: %s", *l.lb.current.LoadBalancerName, err.Error()) - return fmt.Errorf("Failure Setting ALB Subnets: %s", err) + return fmt.Errorf("Failed setting ELBV2 subnets: %s", err) } l.lb.current.AvailabilityZones = l.lb.desired.AvailabilityZones rOpts.Eventf(api.EventTypeNormal, "MODIFY", "%s subnets modified", *l.lb.current.LoadBalancerName) @@ -556,7 +556,7 @@ func (l *LoadBalancer) modify(rOpts *ReconcileOptions) error { IpAddressType: l.lb.desired.IpAddressType, }); err != nil { rOpts.Eventf(api.EventTypeNormal, "ERROR", "%s ip address type modification failed: %s", *l.lb.current.LoadBalancerName, err.Error()) - return fmt.Errorf("Failure Setting ALB IpAddressType: %s", err) + return fmt.Errorf("Failed setting ELBV2 IP address type: %s", err) } l.lb.current.IpAddressType = l.lb.desired.IpAddressType rOpts.Eventf(api.EventTypeNormal, "MODIFY", "%s ip address type modified", *l.lb.current.LoadBalancerName) @@ -581,7 +581,7 @@ func (l *LoadBalancer) modify(rOpts *ReconcileOptions) error { Attributes: l.attributes.desired, }); err != nil { rOpts.Eventf(api.EventTypeWarning, "ERROR", "%s attributes modification failed: %s", *l.lb.current.LoadBalancerName, err.Error()) - return fmt.Errorf("Failure modifying attributes: %s", err) + return fmt.Errorf("Failed modifying attributes: %s", err) } l.attributes.current = l.attributes.desired rOpts.Eventf(api.EventTypeNormal, "MODIFY", "%s attributes modified", *l.lb.current.LoadBalancerName) @@ -591,7 +591,7 @@ func (l *LoadBalancer) modify(rOpts *ReconcileOptions) error { l.logger.Infof("Associating %v Web ACL.", l.options.desired.webACLId) if _, err := albwaf.WAFRegionalsvc.Associate(l.lb.current.LoadBalancerArn, l.options.desired.webACLId); err != nil { rOpts.Eventf(api.EventTypeWarning, "ERROR", "%s Web ACL (%s) association failed: %s", *l.lb.current.LoadBalancerName, *l.options.desired.webACLId, err.Error()) - return fmt.Errorf("Failure associating Web ACL: %s", err.Error()) + return fmt.Errorf("Failed associating Web ACL: %s", err.Error()) } else { l.options.current.webACLId = l.options.desired.webACLId rOpts.Eventf(api.EventTypeNormal, "MODIFY", "Web ACL association updated to %s", *l.options.desired.webACLId) @@ -602,7 +602,7 @@ func (l *LoadBalancer) modify(rOpts *ReconcileOptions) error { l.logger.Infof("Disassociating Web ACL.") if _, err := albwaf.WAFRegionalsvc.Disassociate(l.lb.current.LoadBalancerArn); err != nil { rOpts.Eventf(api.EventTypeWarning, "ERROR", "%s Web ACL disassociation failed: %s", *l.lb.current.LoadBalancerName, err.Error()) - return fmt.Errorf("Failure removing Web ACL association: %s", err.Error()) + return fmt.Errorf("Failed removing Web ACL association: %s", err.Error()) } else { l.options.current.webACLId = l.options.desired.webACLId rOpts.Eventf(api.EventTypeNormal, "MODIFY", "Web ACL disassociated") @@ -634,8 +634,7 @@ func (l *LoadBalancer) delete(rOpts *ReconcileOptions) error { if l.options.current.webACLId != nil { if _, err := albwaf.WAFRegionalsvc.Disassociate(l.lb.current.LoadBalancerArn); err != nil { rOpts.Eventf(api.EventTypeWarning, "ERROR", "Error disassociating Web ACL for %s: %s", *l.lb.current.LoadBalancerName, err.Error()) - l.logger.Errorf("Failed disassociation of ELBV2 Web ACL: %s.", err.Error()) - return err + return fmt.Errorf("Failed disassociation of ELBV2 Web ACL: %s.", err.Error()) } } @@ -645,8 +644,7 @@ func (l *LoadBalancer) delete(rOpts *ReconcileOptions) error { if _, err := albelbv2.ELBV2svc.DeleteLoadBalancer(in); err != nil { rOpts.Eventf(api.EventTypeWarning, "ERROR", "Error deleting %s: %s", *l.lb.current.LoadBalancerName, err.Error()) - l.logger.Errorf("Failed deletion of ELBV2: %s.", err.Error()) - return err + return fmt.Errorf("Failed deletion of ELBV2: %s.", err.Error()) } // if the alb controller was managing a SG we must: @@ -658,7 +656,7 @@ func (l *LoadBalancer) delete(rOpts *ReconcileOptions) error { if l.options.current.managedSG != nil { if err := albec2.EC2svc.DisassociateSGFromInstanceIfNeeded(l.targetgroups[0].CurrentTargets(), l.options.current.managedInstanceSG); err != nil { rOpts.Eventf(api.EventTypeWarning, "WARN", "Failed disassociating sgs from instances: %s", err.Error()) - l.logger.Warnf("Failed in deletion of managed SG: %s.", err.Error()) + return fmt.Errorf("Failed disassociating managed SG: %s.", err.Error()) } if err := attemptSGDeletion(l.options.current.managedInstanceSG); err != nil { rOpts.Eventf(api.EventTypeWarning, "WARN", "Failed deleting %s: %s", *l.options.current.managedInstanceSG, err.Error()) diff --git a/pkg/alb/ls/listener.go b/pkg/alb/ls/listener.go index 6c011b105..791ea61ed 100644 --- a/pkg/alb/ls/listener.go +++ b/pkg/alb/ls/listener.go @@ -1,26 +1,33 @@ package ls import ( + "fmt" + "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/elbv2" "github.com/kubernetes-sigs/aws-alb-ingress-controller/pkg/alb/rs" + "github.com/kubernetes-sigs/aws-alb-ingress-controller/pkg/alb/tg" "github.com/kubernetes-sigs/aws-alb-ingress-controller/pkg/annotations" "github.com/kubernetes-sigs/aws-alb-ingress-controller/pkg/aws/albelbv2" "github.com/kubernetes-sigs/aws-alb-ingress-controller/pkg/util/log" util "github.com/kubernetes-sigs/aws-alb-ingress-controller/pkg/util/types" api "k8s.io/api/core/v1" + extensions "k8s.io/api/extensions/v1beta1" ) type NewDesiredListenerOptions struct { - Port annotations.PortData - CertificateArn *string - Logger *log.Logger - SslPolicy *string + ExistingListener *Listener + Port annotations.PortData + CertificateArn *string + Logger *log.Logger + SslPolicy *string + IngressRules []extensions.IngressRule + IgnoreHostHeader bool } // NewDesiredListener returns a new listener.Listener based on the parameters provided. -func NewDesiredListener(o *NewDesiredListenerOptions) *Listener { - listener := &elbv2.Listener{ +func NewDesiredListener(o *NewDesiredListenerOptions) (*Listener, error) { + l := &elbv2.Listener{ Port: aws.Int64(o.Port.Port), Protocol: aws.String("HTTP"), DefaultActions: []*elbv2.Action{ @@ -31,35 +38,72 @@ func NewDesiredListener(o *NewDesiredListenerOptions) *Listener { } if o.CertificateArn != nil && o.Port.Scheme == "HTTPS" { - listener.Certificates = []*elbv2.Certificate{ + l.Certificates = []*elbv2.Certificate{ {CertificateArn: o.CertificateArn}, } - listener.Protocol = aws.String("HTTPS") + l.Protocol = aws.String("HTTPS") } if o.SslPolicy != nil && o.Port.Scheme == "HTTPS" { - listener.SslPolicy = o.SslPolicy + l.SslPolicy = o.SslPolicy } - listenerT := &Listener{ - ls: ls{desired: listener}, + listener := &Listener{ + ls: ls{desired: l}, logger: o.Logger, } - return listenerT + if o.ExistingListener != nil { + listener.rules = o.ExistingListener.rules + } + + var p int + for _, rule := range o.IngressRules { + var err error + + listener.rules, p, err = rs.NewDesiredRules(&rs.NewDesiredRulesOptions{ + Priority: p, + Logger: o.Logger, + ListenerRules: listener.rules, + Rule: &rule, + IgnoreHostHeader: o.IgnoreHostHeader, + }) + if err != nil { + return nil, err + } + } + + if o.ExistingListener != nil { + o.ExistingListener.ls.desired = listener.ls.desired + o.ExistingListener.rules = listener.rules + return o.ExistingListener, nil + } + + return listener, nil } type NewCurrentListenerOptions struct { - Listener *elbv2.Listener - Logger *log.Logger + Listener *elbv2.Listener + Logger *log.Logger + TargetGroups tg.TargetGroups } // NewCurrentListener returns a new listener.Listener based on an elbv2.Listener. -func NewCurrentListener(o *NewCurrentListenerOptions) *Listener { +func NewCurrentListener(o *NewCurrentListenerOptions) (*Listener, error) { + rules, err := rs.NewCurrentRules(&rs.NewCurrentRulesOptions{ + ListenerArn: o.Listener.ListenerArn, + Logger: o.Logger, + TargetGroups: o.TargetGroups, + }) + if err != nil { + return nil, err + } + return &Listener{ ls: ls{current: o.Listener}, logger: o.Logger, - } + rules: rules, + }, nil } // Reconcile compares the current and desired state of this Listener instance. Comparison @@ -89,16 +133,22 @@ func (l *Listener) Reconcile(rOpts *ReconcileOptions) error { *l.ls.current.Protocol) case l.needsModification(rOpts): // current and desired diff; needs mod - l.logger.Infof("Start Listener modification.") if err := l.modify(rOpts); err != nil { return err } rOpts.Eventf(api.EventTypeNormal, "MODIFY", "%v listener modified", *l.ls.current.Port) - l.logger.Infof("Completed Listener modification. ARN: %s | Port: %v | Proto: %s.", - *l.ls.current.ListenerArn, *l.ls.current.Port, *l.ls.current.Protocol) + } - default: - // l.logger.Debugf("No listener modification required.") + if l.ls.current != nil { + if rs, err := l.rules.Reconcile(&rs.ReconcileOptions{ + Eventf: rOpts.Eventf, + ListenerArn: l.ls.current.ListenerArn, + TargetGroups: rOpts.TargetGroups, + }); err != nil { + return err + } else { + l.rules = rs + } } return nil @@ -132,8 +182,7 @@ func (l *Listener) create(rOpts *ReconcileOptions) error { o, err := albelbv2.ELBV2svc.CreateListener(in) if err != nil { rOpts.Eventf(api.EventTypeWarning, "ERROR", "Error creating %v listener: %s", *desired.Port, err.Error()) - l.logger.Errorf("Failed Listener creation: %s.", err.Error()) - return err + return fmt.Errorf("Failed Listener creation: %s.", err.Error()) } l.ls.current = o.Listeners[0] @@ -160,9 +209,7 @@ func (l *Listener) modify(rOpts *ReconcileOptions) error { o, err := albelbv2.ELBV2svc.ModifyListener(in) if err != nil { rOpts.Eventf(api.EventTypeWarning, "ERROR", "Error modifying %v listener: %s", *desired.Port, err.Error()) - l.logger.Errorf("Failed Listener modification: %s.", err.Error()) - l.logger.Debugf("Payload: %s", log.Prettify(in)) - return err + return fmt.Errorf("Failed Listener modification: %s", err.Error()) } l.ls.current = o.Listeners[0] @@ -173,9 +220,7 @@ func (l *Listener) modify(rOpts *ReconcileOptions) error { func (l *Listener) delete(rOpts *ReconcileOptions) error { if err := albelbv2.ELBV2svc.RemoveListener(l.ls.current.ListenerArn); err != nil { rOpts.Eventf(api.EventTypeWarning, "ERROR", "Error deleting %v listener: %s", *l.ls.current.Port, err.Error()) - l.logger.Errorf("Failed Listener deletion. ARN: %s: %s", - *l.ls.current.ListenerArn, err.Error()) - return err + return fmt.Errorf("Failed Listener deletion. ARN: %s: %s", *l.ls.current.ListenerArn, err.Error()) } l.deleted = true diff --git a/pkg/alb/ls/listener_test.go b/pkg/alb/ls/listener_test.go index 95a8bb8c0..56c1f7270 100644 --- a/pkg/alb/ls/listener_test.go +++ b/pkg/alb/ls/listener_test.go @@ -78,7 +78,7 @@ func TestNewHTTPListener(t *testing.T) { Logger: logr, } - l := NewDesiredListener(o) + l, _ := NewDesiredListener(o) desiredProto := "HTTP" if o.CertificateArn != nil { @@ -105,7 +105,7 @@ func TestNewHTTPSListener(t *testing.T) { Logger: logr, } - l := NewDesiredListener(o) + l, _ := NewDesiredListener(o) desiredProto := "HTTP" if o.CertificateArn != nil { diff --git a/pkg/alb/ls/listeners.go b/pkg/alb/ls/listeners.go index 083f3ebb5..8967441a8 100644 --- a/pkg/alb/ls/listeners.go +++ b/pkg/alb/ls/listeners.go @@ -1,17 +1,12 @@ package ls import ( - "fmt" - - "github.com/aws/aws-sdk-go/aws/awsutil" "github.com/aws/aws-sdk-go/service/elbv2" extensions "k8s.io/api/extensions/v1beta1" - "github.com/kubernetes-sigs/aws-alb-ingress-controller/pkg/alb/rs" "github.com/kubernetes-sigs/aws-alb-ingress-controller/pkg/alb/tg" "github.com/kubernetes-sigs/aws-alb-ingress-controller/pkg/annotations" - "github.com/kubernetes-sigs/aws-alb-ingress-controller/pkg/aws/albelbv2" "github.com/kubernetes-sigs/aws-alb-ingress-controller/pkg/util/log" ) @@ -24,18 +19,6 @@ func (ls Listeners) Reconcile(rOpts *ReconcileOptions) (Listeners, error) { return nil, err } - if l.ls.current != nil { - rsOpts := &rs.ReconcileOptions{ - Eventf: rOpts.Eventf, - ListenerArn: l.ls.current.ListenerArn, - TargetGroups: rOpts.TargetGroups, - } - if rs, err := l.rules.Reconcile(rsOpts); err != nil { - return nil, err - } else { - l.rules = rs - } - } if !l.deleted { output = append(output, l) } @@ -74,33 +57,15 @@ func NewCurrentListeners(o *NewCurrentListenersOptions) (Listeners, error) { var output Listeners for _, l := range o.Listeners { - o.Logger.Infof("Fetching Rules for Listener %s", *l.ListenerArn) - rules, err := albelbv2.ELBV2svc.DescribeRules(&elbv2.DescribeRulesInput{ListenerArn: l.ListenerArn}) + newListener, err := NewCurrentListener(&NewCurrentListenerOptions{ + Listener: l, + Logger: o.Logger, + TargetGroups: o.TargetGroups, + }) if err != nil { return nil, err } - newListener := NewCurrentListener(&NewCurrentListenerOptions{ - Listener: l, - Logger: o.Logger, - }) - - for _, r := range rules.Rules { - // TODO LOOKUP svcName based on TG - i, tg := o.TargetGroups.FindCurrentByARN(*r.Actions[0].TargetGroupArn) - if i < 0 { - return nil, fmt.Errorf("failed to find a target group associated with a rule. This should not be possible. Rule: %s", awsutil.Prettify(r.RuleArn)) - } - o.Logger.Debugf("Assembling rule for: %s", log.Prettify(r.Conditions)) - newRule := rs.NewCurrentRule(&rs.NewCurrentRuleOptions{ - SvcName: tg.SvcName, - Rule: r, - Logger: o.Logger, - }) - - newListener.rules = append(newListener.rules, newRule) - } - output = append(output, newListener) } return output, nil @@ -132,37 +97,23 @@ func NewDesiredListeners(o *NewDesiredListenersOptions) (Listeners, error) { } } - newListener := NewDesiredListener(&NewDesiredListenerOptions{ - Port: port, - CertificateArn: o.Annotations.CertificateArn, - Logger: o.Logger, - SslPolicy: o.Annotations.SslPolicy, + newListener, err := NewDesiredListener(&NewDesiredListenerOptions{ + Port: port, + CertificateArn: o.Annotations.CertificateArn, + Logger: o.Logger, + SslPolicy: o.Annotations.SslPolicy, + IngressRules: o.IngressRules, + IgnoreHostHeader: *o.Annotations.IgnoreHostHeader, + ExistingListener: thisListener, }) - - if thisListener != nil { - thisListener.ls.desired = newListener.ls.desired - newListener = thisListener + if err != nil { + return nil, err } - var p int - for _, rule := range o.IngressRules { - var err error - - newListener.rules, p, err = rs.NewDesiredRules(&rs.NewDesiredRulesOptions{ - Priority: p, - Logger: o.Logger, - ListenerRules: newListener.rules, - Rule: &rule, - IgnoreHostHeader: *o.Annotations.IgnoreHostHeader, - }) - if err != nil { - return nil, err - } - } output = append(output, newListener) } - // Generate a listener for each existing known port that is not longer annotated + // Generate a listener for each existing known port that is no longer annotated // representing it needs to be deleted for _, l := range o.ExistingListeners { exists := false diff --git a/pkg/alb/rs/rule.go b/pkg/alb/rs/rule.go index 29d804695..6ffc40d1e 100644 --- a/pkg/alb/rs/rule.go +++ b/pkg/alb/rs/rule.go @@ -117,14 +117,10 @@ func (r *Rule) Reconcile(rOpts *ReconcileOptions) error { log.Prettify(r.rs.current.Conditions)) case r.needsModification(): // diff between current and desired, modify rule - r.logger.Infof("Start Rule modification.") if err := r.modify(rOpts); err != nil { return err } rOpts.Eventf(api.EventTypeNormal, "MODIFY", "%s rule modified", *r.rs.current.Priority) - r.logger.Infof("Completed Rule modification. Rule Priority: %s | Condition: %s", - log.Prettify(r.rs.current.Priority), - log.Prettify(r.rs.current.Conditions)) default: // r.logger.Debugf("No rule modification required.") @@ -159,9 +155,7 @@ func (r *Rule) create(rOpts *ReconcileOptions) error { o, err := albelbv2.ELBV2svc.CreateRule(in) if err != nil { rOpts.Eventf(api.EventTypeWarning, "ERROR", "Error creating %v rule: %s", *in.Priority, err.Error()) - r.logger.Errorf("Failed Rule creation. Rule: %s | Error: %s", - log.Prettify(r.rs.desired), err.Error()) - return err + return fmt.Errorf("Failed Rule creation. Rule: %s | Error: %s", log.Prettify(r.rs.desired), err.Error()) } r.rs.current = o.Rules[0] r.svcname.current = r.svcname.desired @@ -181,8 +175,7 @@ func (r *Rule) modify(rOpts *ReconcileOptions) error { if err != nil { msg := fmt.Sprintf("Error modifying rule %s: %s", *r.rs.current.RuleArn, err.Error()) rOpts.Eventf(api.EventTypeWarning, "ERROR", msg) - r.logger.Errorf(msg) - return err + return fmt.Errorf(msg) } if len(o.Rules) > 0 { r.rs.current = o.Rules[0] @@ -209,8 +202,7 @@ func (r *Rule) delete(rOpts *ReconcileOptions) error { in := &elbv2.DeleteRuleInput{RuleArn: r.rs.current.RuleArn} if _, err := albelbv2.ELBV2svc.DeleteRule(in); err != nil { rOpts.Eventf(api.EventTypeWarning, "ERROR", "Error deleting %s rule: %s", *r.rs.current.Priority, err.Error()) - r.logger.Infof("Failed Rule deletion. Error: %s", err.Error()) - return err + return fmt.Errorf("Failed Rule deletion. Error: %s", err.Error()) } r.deleted = true diff --git a/pkg/alb/rs/rules.go b/pkg/alb/rs/rules.go index 97e99f43a..d0c29e1a8 100644 --- a/pkg/alb/rs/rules.go +++ b/pkg/alb/rs/rules.go @@ -4,13 +4,49 @@ import ( "fmt" "github.com/aws/aws-sdk-go/aws/awsutil" + "github.com/aws/aws-sdk-go/service/elbv2" extensions "k8s.io/api/extensions/v1beta1" "github.com/kubernetes-sigs/aws-alb-ingress-controller/pkg/alb/tg" + "github.com/kubernetes-sigs/aws-alb-ingress-controller/pkg/aws/albelbv2" "github.com/kubernetes-sigs/aws-alb-ingress-controller/pkg/util/log" ) +type NewCurrentRulesOptions struct { + ListenerArn *string + Logger *log.Logger + TargetGroups tg.TargetGroups +} + +// NewCurrentRules +func NewCurrentRules(o *NewCurrentRulesOptions) (Rules, error) { + var rs Rules + + o.Logger.Infof("Fetching Rules for Listener %s", *o.ListenerArn) + rules, err := albelbv2.ELBV2svc.DescribeRules(&elbv2.DescribeRulesInput{ListenerArn: o.ListenerArn}) + if err != nil { + return nil, err + } + + for _, r := range rules.Rules { + // TODO LOOKUP svcName based on TG + i, tg := o.TargetGroups.FindCurrentByARN(*r.Actions[0].TargetGroupArn) + if i < 0 { + return nil, fmt.Errorf("failed to find a target group associated with a rule. This should not be possible. Rule: %s", awsutil.Prettify(r.RuleArn)) + } + + newRule := NewCurrentRule(&NewCurrentRuleOptions{ + SvcName: tg.SvcName, + Rule: r, + Logger: o.Logger, + }) + + rs = append(rs, newRule) + } + return rs, nil +} + type NewDesiredRulesOptions struct { Priority int Logger *log.Logger diff --git a/pkg/alb/tg/targetgroup.go b/pkg/alb/tg/targetgroup.go index 48db89d37..3e810a9b1 100644 --- a/pkg/alb/tg/targetgroup.go +++ b/pkg/alb/tg/targetgroup.go @@ -161,16 +161,10 @@ func (t *TargetGroup) Reconcile(rOpts *ReconcileOptions) error { default: // Current and Desired exist and need for modification should be evaluated. if mods := t.needsModification(); mods != 0 { - t.logger.Infof("Start TargetGroup modification.") if err := t.modify(mods, rOpts); err != nil { return err } rOpts.Eventf(api.EventTypeNormal, "CREATE", "%s target group modified", t.ID) - t.logger.Infof("Succeeded TargetGroup modification. ARN: %s | Name: %s.", - *t.tg.current.TargetGroupArn, - *t.tg.current.TargetGroupName) - } else { - // t.logger.Debugf("No TargetGroup modification required.") } } @@ -199,38 +193,36 @@ func (t *TargetGroup) create(rOpts *ReconcileOptions) error { o, err := albelbv2.ELBV2svc.CreateTargetGroup(in) if err != nil { rOpts.Eventf(api.EventTypeWarning, "ERROR", "Error creating target group %s: %s", t.ID, err.Error()) - t.logger.Infof("Failed TargetGroup creation: %s.", err.Error()) - return err + return fmt.Errorf("Failed TargetGroup creation: %s.", err.Error()) } t.tg.current = o.TargetGroups[0] // Add tags if err = albelbv2.ELBV2svc.UpdateTags(t.CurrentARN(), t.tags.current, t.tags.desired); err != nil { rOpts.Eventf(api.EventTypeWarning, "ERROR", "Error tagging target group %s: %s", t.ID, err.Error()) - t.logger.Infof("Failed TargetGroup creation. Unable to add tags: %s.", err.Error()) - return err + return fmt.Errorf("Failed TargetGroup creation. Unable to add tags: %s.", err.Error()) } t.tags.current = t.tags.desired // Register Targets if err = t.registerTargets(t.targets.desired, rOpts); err != nil { rOpts.Eventf(api.EventTypeWarning, "ERROR", "Error registering targets to target group %s: %s", t.ID, err.Error()) - t.logger.Infof("Failed TargetGroup creation. Unable to register targets: %s.", err.Error()) - return err + return fmt.Errorf("Failed TargetGroup creation. Unable to register targets: %s.", err.Error()) } - // Add TargetGroup attributes - attributes := &elbv2.ModifyTargetGroupAttributesInput{ - Attributes: t.attributes.desired, - TargetGroupArn: t.CurrentARN(), - } + if len(t.attributes.desired) > 0 { + // Add TargetGroup attributes + attributes := &elbv2.ModifyTargetGroupAttributesInput{ + Attributes: t.attributes.desired, + TargetGroupArn: t.CurrentARN(), + } - if _, err := albelbv2.ELBV2svc.ModifyTargetGroupAttributes(attributes); err != nil { - rOpts.Eventf(api.EventTypeWarning, "ERROR", "Error adding attributes to target group %s: %s", t.ID, err.Error()) - t.logger.Infof("Failed TargetGroup creation. Unable to add target group attributes: %s.", err.Error()) - return err + if _, err := albelbv2.ELBV2svc.ModifyTargetGroupAttributes(attributes); err != nil { + rOpts.Eventf(api.EventTypeWarning, "ERROR", "Error adding attributes to target group %s: %s", t.ID, err.Error()) + return fmt.Errorf("Failed TargetGroup creation. Unable to add target group attributes: %s.", err.Error()) + } + t.attributes.current = t.attributes.desired } - t.attributes.current = t.attributes.desired return nil } @@ -395,6 +387,8 @@ func (t *TargetGroup) registerTargets(additions util.AWSStringSlice, rOpts *Reco } if _, err := albelbv2.ELBV2svc.RegisterTargets(in); err != nil { + // Flush the cached health of the TG so that on the next iteration it will get fresh data, these change often + albelbv2.ELBV2svc.CacheDelete(albelbv2.DescribeTargetGroupTargetsForArnCache, *t.CurrentARN()) return err } diff --git a/pkg/albingress/albingress.go b/pkg/albingress/albingress.go index c3bcfa99a..45b92e71a 100644 --- a/pkg/albingress/albingress.go +++ b/pkg/albingress/albingress.go @@ -235,11 +235,9 @@ func (a *ALBIngress) Hostnames() ([]api.LoadBalancerIngress, error) { } if len(hostnames) == 0 { - a.logger.Errorf("No ALB hostnames for ingress") return nil, fmt.Errorf("No ALB hostnames for ingress") } - // a.logger.Debugf("Ingress library requested hostname list and we returned %s", *a.loadBalancer.Hostname()) return hostnames, nil } diff --git a/pkg/aws/albelbv2/elbv2.go b/pkg/aws/albelbv2/elbv2.go index d9fdce9f8..054f3c2a4 100644 --- a/pkg/aws/albelbv2/elbv2.go +++ b/pkg/aws/albelbv2/elbv2.go @@ -28,10 +28,13 @@ const ( deleteTargetGroupReattemptSleep int = 10 // Maximum attempts should be made to delete a target group deleteTargetGroupReattemptMax int = 10 + + DescribeTargetGroupTargetsForArnCache string = "ELBV2-DescribeTargetGroupTargetsForArn" ) type ELBV2API interface { elbv2iface.ELBV2API + CacheDelete(string, string) ClusterLoadBalancers(*albrgt.Resources) ([]*elbv2.LoadBalancer, error) ClusterTargetGroups(*albrgt.Resources) (map[string][]*elbv2.TargetGroup, error) UpdateTags(arn *string, old util.ELBv2Tags, new util.ELBv2Tags) error @@ -311,9 +314,15 @@ func (e *ELBV2) DescribeListenersForLoadBalancer(loadBalancerArn *string) ([]*el return listeners, nil } +// CacheDelete deletes an item from the cache +func (e *ELBV2) CacheDelete(cache, key string) { + cacheKey := cache + "." + key + e.cache.Delete(cacheKey) +} + // DescribeTargetGroupTargetsForArn looks up target group targets by an ARN. func (e *ELBV2) DescribeTargetGroupTargetsForArn(arn *string, targets ...[]*elbv2.TargetDescription) (result util.AWSStringSlice, err error) { - cache := "ELBV2-DescribeTargetGroupTargetsForArn" + cache := DescribeTargetGroupTargetsForArnCache key := cache + "." + *arn item := e.cache.Get(key)