Skip to content

Commit

Permalink
Merge pull request #442 from kubernetes-sigs/logging-cleanup
Browse files Browse the repository at this point in the history 
Logging cleanup, cache purging, and a small refactor
  • Loading branch information
@bigkraig
bigkraig authored Jun 29, 2018
2 parents 39cde7f + 28a5741 commit 96fdc19
Show file tree
Hide file tree
Showing 9 changed files with 166 additions and 143 deletions.
20 changes: 9 additions & 11 deletions pkg/alb/lb/loadbalancer.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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")
Expand Down Expand Up @@ -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())
}
}

Expand All @@ -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:
Expand All @@ -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())
Expand Down
103 changes: 74 additions & 29 deletions pkg/alb/ls/listener.go
View file
Original file line number Diff line number Diff line change
@@ -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{
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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]
Expand All @@ -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]

Expand All @@ -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
Expand Down
4 changes: 2 additions & 2 deletions pkg/alb/ls/listener_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ func TestNewHTTPListener(t *testing.T) {
Logger: logr,
}

l := NewDesiredListener(o)
l, _ := NewDesiredListener(o)

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

l := NewDesiredListener(o)
l, _ := NewDesiredListener(o)

desiredProto := "HTTP"
if o.CertificateArn != nil {
Expand Down
Loading

0 comments on commit 96fdc19

Please sign in to comment.