Skip to content

Commit a4ca9d8

Browse files
bigkraigbigkraig
authored
Merge pull request #442 from kubernetes-sigs/logging-cleanup
Logging cleanup, cache purging, and a small refactor
2 parents d97f48d + b1de5bc commit a4ca9d8

File tree

9 files changed

+166
-143
lines changed

9 files changed

+166
-143
lines changed

‎pkg/alb/lb/loadbalancer.go

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -187,7 +187,7 @@ func NewCurrentLoadBalancer(o *NewCurrentLoadBalancerOptions) (newLoadBalancer *
187187

188188
attrs, err := albelbv2.ELBV2svc.DescribeLoadBalancerAttributesFiltered(o.LoadBalancer.LoadBalancerArn)
189189
if err != nil {
190-
return newLoadBalancer, fmt.Errorf("Failed to retrieve attributes from ALB in AWS. Error: %s", err.Error())
190+
return newLoadBalancer, fmt.Errorf("Failed to retrieve attributes from ELBV2 in AWS. Error: %s", err.Error())
191191
}
192192

193193
var managedSG *string
@@ -542,7 +542,7 @@ func (l *LoadBalancer) modify(rOpts *ReconcileOptions) error {
542542
Subnets: util.AvailabilityZones(l.lb.desired.AvailabilityZones).AsSubnets(),
543543
}); err != nil {
544544
rOpts.Eventf(api.EventTypeWarning, "ERROR", "%s subnet modification failed: %s", *l.lb.current.LoadBalancerName, err.Error())
545-
return fmt.Errorf("Failure Setting ALB Subnets: %s", err)
545+
return fmt.Errorf("Failed setting ELBV2 subnets: %s", err)
546546
}
547547
l.lb.current.AvailabilityZones = l.lb.desired.AvailabilityZones
548548
rOpts.Eventf(api.EventTypeNormal, "MODIFY", "%s subnets modified", *l.lb.current.LoadBalancerName)
@@ -556,7 +556,7 @@ func (l *LoadBalancer) modify(rOpts *ReconcileOptions) error {
556556
IpAddressType: l.lb.desired.IpAddressType,
557557
}); err != nil {
558558
rOpts.Eventf(api.EventTypeNormal, "ERROR", "%s ip address type modification failed: %s", *l.lb.current.LoadBalancerName, err.Error())
559-
return fmt.Errorf("Failure Setting ALB IpAddressType: %s", err)
559+
return fmt.Errorf("Failed setting ELBV2 IP address type: %s", err)
560560
}
561561
l.lb.current.IpAddressType = l.lb.desired.IpAddressType
562562
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 {
581581
Attributes: l.attributes.desired,
582582
}); err != nil {
583583
rOpts.Eventf(api.EventTypeWarning, "ERROR", "%s attributes modification failed: %s", *l.lb.current.LoadBalancerName, err.Error())
584-
return fmt.Errorf("Failure modifying attributes: %s", err)
584+
return fmt.Errorf("Failed modifying attributes: %s", err)
585585
}
586586
l.attributes.current = l.attributes.desired
587587
rOpts.Eventf(api.EventTypeNormal, "MODIFY", "%s attributes modified", *l.lb.current.LoadBalancerName)
@@ -591,7 +591,7 @@ func (l *LoadBalancer) modify(rOpts *ReconcileOptions) error {
591591
l.logger.Infof("Associating %v Web ACL.", l.options.desired.webACLId)
592592
if _, err := albwaf.WAFRegionalsvc.Associate(l.lb.current.LoadBalancerArn, l.options.desired.webACLId); err != nil {
593593
rOpts.Eventf(api.EventTypeWarning, "ERROR", "%s Web ACL (%s) association failed: %s", *l.lb.current.LoadBalancerName, *l.options.desired.webACLId, err.Error())
594-
return fmt.Errorf("Failure associating Web ACL: %s", err.Error())
594+
return fmt.Errorf("Failed associating Web ACL: %s", err.Error())
595595
} else {
596596
l.options.current.webACLId = l.options.desired.webACLId
597597
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 {
602602
l.logger.Infof("Disassociating Web ACL.")
603603
if _, err := albwaf.WAFRegionalsvc.Disassociate(l.lb.current.LoadBalancerArn); err != nil {
604604
rOpts.Eventf(api.EventTypeWarning, "ERROR", "%s Web ACL disassociation failed: %s", *l.lb.current.LoadBalancerName, err.Error())
605-
return fmt.Errorf("Failure removing Web ACL association: %s", err.Error())
605+
return fmt.Errorf("Failed removing Web ACL association: %s", err.Error())
606606
} else {
607607
l.options.current.webACLId = l.options.desired.webACLId
608608
rOpts.Eventf(api.EventTypeNormal, "MODIFY", "Web ACL disassociated")
@@ -634,8 +634,7 @@ func (l *LoadBalancer) delete(rOpts *ReconcileOptions) error {
634634
if l.options.current.webACLId != nil {
635635
if _, err := albwaf.WAFRegionalsvc.Disassociate(l.lb.current.LoadBalancerArn); err != nil {
636636
rOpts.Eventf(api.EventTypeWarning, "ERROR", "Error disassociating Web ACL for %s: %s", *l.lb.current.LoadBalancerName, err.Error())
637-
l.logger.Errorf("Failed disassociation of ELBV2 Web ACL: %s.", err.Error())
638-
return err
637+
return fmt.Errorf("Failed disassociation of ELBV2 Web ACL: %s.", err.Error())
639638
}
640639
}
641640

@@ -645,8 +644,7 @@ func (l *LoadBalancer) delete(rOpts *ReconcileOptions) error {
645644

646645
if _, err := albelbv2.ELBV2svc.DeleteLoadBalancer(in); err != nil {
647646
rOpts.Eventf(api.EventTypeWarning, "ERROR", "Error deleting %s: %s", *l.lb.current.LoadBalancerName, err.Error())
648-
l.logger.Errorf("Failed deletion of ELBV2: %s.", err.Error())
649-
return err
647+
return fmt.Errorf("Failed deletion of ELBV2: %s.", err.Error())
650648
}
651649

652650
// if the alb controller was managing a SG we must:
@@ -658,7 +656,7 @@ func (l *LoadBalancer) delete(rOpts *ReconcileOptions) error {
658656
if l.options.current.managedSG != nil {
659657
if err := albec2.EC2svc.DisassociateSGFromInstanceIfNeeded(l.targetgroups[0].CurrentTargets(), l.options.current.managedInstanceSG); err != nil {
660658
rOpts.Eventf(api.EventTypeWarning, "WARN", "Failed disassociating sgs from instances: %s", err.Error())
661-
l.logger.Warnf("Failed in deletion of managed SG: %s.", err.Error())
659+
return fmt.Errorf("Failed disassociating managed SG: %s.", err.Error())
662660
}
663661
if err := attemptSGDeletion(l.options.current.managedInstanceSG); err != nil {
664662
rOpts.Eventf(api.EventTypeWarning, "WARN", "Failed deleting %s: %s", *l.options.current.managedInstanceSG, err.Error())

‎pkg/alb/ls/listener.go

Lines changed: 74 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,26 +1,33 @@
11
package ls
22

33
import (
4+
"fmt"
5+
46
"github.com/aws/aws-sdk-go/aws"
57
"github.com/aws/aws-sdk-go/service/elbv2"
68
"github.com/kubernetes-sigs/aws-alb-ingress-controller/pkg/alb/rs"
9+
"github.com/kubernetes-sigs/aws-alb-ingress-controller/pkg/alb/tg"
710
"github.com/kubernetes-sigs/aws-alb-ingress-controller/pkg/annotations"
811
"github.com/kubernetes-sigs/aws-alb-ingress-controller/pkg/aws/albelbv2"
912
"github.com/kubernetes-sigs/aws-alb-ingress-controller/pkg/util/log"
1013
util "github.com/kubernetes-sigs/aws-alb-ingress-controller/pkg/util/types"
1114
api "k8s.io/api/core/v1"
15+
extensions "k8s.io/api/extensions/v1beta1"
1216
)
1317

1418
type NewDesiredListenerOptions struct {
15-
Port annotations.PortData
16-
CertificateArn *string
17-
Logger *log.Logger
18-
SslPolicy *string
19+
ExistingListener *Listener
20+
Port annotations.PortData
21+
CertificateArn *string
22+
Logger *log.Logger
23+
SslPolicy *string
24+
IngressRules []extensions.IngressRule
25+
IgnoreHostHeader bool
1926
}
2027

2128
// NewDesiredListener returns a new listener.Listener based on the parameters provided.
22-
func NewDesiredListener(o *NewDesiredListenerOptions) *Listener {
23-
listener := &elbv2.Listener{
29+
func NewDesiredListener(o *NewDesiredListenerOptions) (*Listener, error) {
30+
l := &elbv2.Listener{
2431
Port: aws.Int64(o.Port.Port),
2532
Protocol: aws.String("HTTP"),
2633
DefaultActions: []*elbv2.Action{
@@ -31,35 +38,72 @@ func NewDesiredListener(o *NewDesiredListenerOptions) *Listener {
3138
}
3239

3340
if o.CertificateArn != nil && o.Port.Scheme == "HTTPS" {
34-
listener.Certificates = []*elbv2.Certificate{
41+
l.Certificates = []*elbv2.Certificate{
3542
{CertificateArn: o.CertificateArn},
3643
}
37-
listener.Protocol = aws.String("HTTPS")
44+
l.Protocol = aws.String("HTTPS")
3845
}
3946

4047
if o.SslPolicy != nil && o.Port.Scheme == "HTTPS" {
41-
listener.SslPolicy = o.SslPolicy
48+
l.SslPolicy = o.SslPolicy
4249
}
4350

44-
listenerT := &Listener{
45-
ls: ls{desired: listener},
51+
listener := &Listener{
52+
ls: ls{desired: l},
4653
logger: o.Logger,
4754
}
4855

49-
return listenerT
56+
if o.ExistingListener != nil {
57+
listener.rules = o.ExistingListener.rules
58+
}
59+
60+
var p int
61+
for _, rule := range o.IngressRules {
62+
var err error
63+
64+
listener.rules, p, err = rs.NewDesiredRules(&rs.NewDesiredRulesOptions{
65+
Priority: p,
66+
Logger: o.Logger,
67+
ListenerRules: listener.rules,
68+
Rule: &rule,
69+
IgnoreHostHeader: o.IgnoreHostHeader,
70+
})
71+
if err != nil {
72+
return nil, err
73+
}
74+
}
75+
76+
if o.ExistingListener != nil {
77+
o.ExistingListener.ls.desired = listener.ls.desired
78+
o.ExistingListener.rules = listener.rules
79+
return o.ExistingListener, nil
80+
}
81+
82+
return listener, nil
5083
}
5184

5285
type NewCurrentListenerOptions struct {
53-
Listener *elbv2.Listener
54-
Logger *log.Logger
86+
Listener *elbv2.Listener
87+
Logger *log.Logger
88+
TargetGroups tg.TargetGroups
5589
}
5690

5791
// NewCurrentListener returns a new listener.Listener based on an elbv2.Listener.
58-
func NewCurrentListener(o *NewCurrentListenerOptions) *Listener {
92+
func NewCurrentListener(o *NewCurrentListenerOptions) (*Listener, error) {
93+
rules, err := rs.NewCurrentRules(&rs.NewCurrentRulesOptions{
94+
ListenerArn: o.Listener.ListenerArn,
95+
Logger: o.Logger,
96+
TargetGroups: o.TargetGroups,
97+
})
98+
if err != nil {
99+
return nil, err
100+
}
101+
59102
return &Listener{
60103
ls: ls{current: o.Listener},
61104
logger: o.Logger,
62-
}
105+
rules: rules,
106+
}, nil
63107
}
64108

65109
// Reconcile compares the current and desired state of this Listener instance. Comparison
@@ -89,16 +133,22 @@ func (l *Listener) Reconcile(rOpts *ReconcileOptions) error {
89133
*l.ls.current.Protocol)
90134

91135
case l.needsModification(rOpts): // current and desired diff; needs mod
92-
l.logger.Infof("Start Listener modification.")
93136
if err := l.modify(rOpts); err != nil {
94137
return err
95138
}
96139
rOpts.Eventf(api.EventTypeNormal, "MODIFY", "%v listener modified", *l.ls.current.Port)
97-
l.logger.Infof("Completed Listener modification. ARN: %s | Port: %v | Proto: %s.",
98-
*l.ls.current.ListenerArn, *l.ls.current.Port, *l.ls.current.Protocol)
140+
}
99141

100-
default:
101-
// l.logger.Debugf("No listener modification required.")
142+
if l.ls.current != nil {
143+
if rs, err := l.rules.Reconcile(&rs.ReconcileOptions{
144+
Eventf: rOpts.Eventf,
145+
ListenerArn: l.ls.current.ListenerArn,
146+
TargetGroups: rOpts.TargetGroups,
147+
}); err != nil {
148+
return err
149+
} else {
150+
l.rules = rs
151+
}
102152
}
103153

104154
return nil
@@ -132,8 +182,7 @@ func (l *Listener) create(rOpts *ReconcileOptions) error {
132182
o, err := albelbv2.ELBV2svc.CreateListener(in)
133183
if err != nil {
134184
rOpts.Eventf(api.EventTypeWarning, "ERROR", "Error creating %v listener: %s", *desired.Port, err.Error())
135-
l.logger.Errorf("Failed Listener creation: %s.", err.Error())
136-
return err
185+
return fmt.Errorf("Failed Listener creation: %s.", err.Error())
137186
}
138187

139188
l.ls.current = o.Listeners[0]
@@ -160,9 +209,7 @@ func (l *Listener) modify(rOpts *ReconcileOptions) error {
160209
o, err := albelbv2.ELBV2svc.ModifyListener(in)
161210
if err != nil {
162211
rOpts.Eventf(api.EventTypeWarning, "ERROR", "Error modifying %v listener: %s", *desired.Port, err.Error())
163-
l.logger.Errorf("Failed Listener modification: %s.", err.Error())
164-
l.logger.Debugf("Payload: %s", log.Prettify(in))
165-
return err
212+
return fmt.Errorf("Failed Listener modification: %s", err.Error())
166213
}
167214
l.ls.current = o.Listeners[0]
168215

@@ -173,9 +220,7 @@ func (l *Listener) modify(rOpts *ReconcileOptions) error {
173220
func (l *Listener) delete(rOpts *ReconcileOptions) error {
174221
if err := albelbv2.ELBV2svc.RemoveListener(l.ls.current.ListenerArn); err != nil {
175222
rOpts.Eventf(api.EventTypeWarning, "ERROR", "Error deleting %v listener: %s", *l.ls.current.Port, err.Error())
176-
l.logger.Errorf("Failed Listener deletion. ARN: %s: %s",
177-
*l.ls.current.ListenerArn, err.Error())
178-
return err
223+
return fmt.Errorf("Failed Listener deletion. ARN: %s: %s", *l.ls.current.ListenerArn, err.Error())
179224
}
180225

181226
l.deleted = true

‎pkg/alb/ls/listener_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ func TestNewHTTPListener(t *testing.T) {
7878
Logger: logr,
7979
}
8080

81-
l := NewDesiredListener(o)
81+
l, _ := NewDesiredListener(o)
8282

8383
desiredProto := "HTTP"
8484
if o.CertificateArn != nil {
@@ -105,7 +105,7 @@ func TestNewHTTPSListener(t *testing.T) {
105105
Logger: logr,
106106
}
107107

108-
l := NewDesiredListener(o)
108+
l, _ := NewDesiredListener(o)
109109

110110
desiredProto := "HTTP"
111111
if o.CertificateArn != nil {

0 commit comments

Comments
 (0)