Removed a lot of repeated log messages, since we pass the reconcile errors up the stack and report at the end, these are duplicated

bigkraig · bigkraig · commit cac3133368dc · 2018-06-28T17:57:18.000-07:00
diff --git 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
@@ -1,6 +1,8 @@
 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"
@@ -89,16 +91,12 @@ 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.")
 	}
 
 	return nil
@@ -132,8 +130,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,8 +157,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 fmt.Errorf("Failed Listener modification: %s.", err.Error())
 		return err
 	}
 	l.ls.current = o.Listeners[0]
@@ -173,9 +169,8 @@ 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",
+		return fmt.Errorf("Failed Listener deletion. ARN: %s: %s",
 			*l.ls.current.ListenerArn, err.Error())
-		return err
 	}
 
 	l.deleted = true
diff --git 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/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,24 +193,21 @@ 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
@@ -227,8 +218,7 @@ func (t *TargetGroup) create(rOpts *ReconcileOptions) error {
 
 	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
+		return fmt.Errorf("Failed TargetGroup creation. Unable to add target group attributes: %s.", err.Error())
 	}
 	t.attributes.current = t.attributes.desired
 
diff --git 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
 }
 

Original file line number	Diff line number	Diff line change
`@@ -235,11 +235,9 @@ func (a *ALBIngress) Hostnames() ([]api.LoadBalancerIngress, error) {`
`235`	`235`	`}`
`236`	`236`
`237`	`237`	`if len(hostnames) == 0 {`
`238`		`- a.logger.Errorf("No ALB hostnames for ingress")`
`239`	`238`	`return nil, fmt.Errorf("No ALB hostnames for ingress")`
`240`	`239`	`}`
`241`	`240`
`242`		`- // a.logger.Debugf("Ingress library requested hostname list and we returned %s", *a.loadBalancer.Hostname())`
`243`	`241`	`return hostnames, nil`
`244`	`242`	`}`
`245`	`243`