Merge pull request #1 from bigkraig/refactor

Working on bringing up our lint score

Author: joshrosso

awsutil/ec2.go

@@ -10,6 +10,7 @@ import (
 	"github.com/aws/aws-sdk-go/service/ec2"
 	"github.com/aws/aws-sdk-go/service/ec2/ec2iface"
 	"github.com/golang/glog"
+	"github.com/karlseguin/ccache"
 	"github.com/prometheus/client_golang/prometheus"
 )
 
@@ -18,6 +19,9 @@ type EC2 struct {
 	Svc ec2iface.EC2API
 }
 
+var ec2Cache = ccache.New(ccache.Configure())
+
+// NewEC2 returns an awsutil EC2 service
 func NewEC2(awsconfig *aws.Config) *EC2 {
 	awsSession, err := session.NewSession(awsconfig)
 	if err != nil {
@@ -47,7 +51,7 @@ func (e *EC2) GetVPCID(subnets []*string) (*string, error) {
 	}
 
 	key := fmt.Sprintf("%s-vpc", *subnets[0])
-	item := Cache.Get(key)
+	item := ec2Cache.Get(key)
 
 	if item == nil {
 		subnetInfo, err := e.Svc.DescribeSubnets(&ec2.DescribeSubnetsInput{
@@ -63,12 +67,12 @@ func (e *EC2) GetVPCID(subnets []*string) (*string, error) {
 		}
 
 		vpc = subnetInfo.Subnets[0].VpcId
-		Cache.Set(key, vpc, time.Minute*60)
+		ec2Cache.Set(key, vpc, time.Minute*60)
 
-		AWSCache.With(prometheus.Labels{"cache": "subnets", "action": "miss"}).Add(float64(1))
+		AWSCache.With(prometheus.Labels{"cache": "vpc", "action": "miss"}).Add(float64(1))
 	} else {
 		vpc = item.Value().(*string)
-		AWSCache.With(prometheus.Labels{"cache": "subnets", "action": "hit"}).Add(float64(1))
+		AWSCache.With(prometheus.Labels{"cache": "vpc", "action": "hit"}).Add(float64(1))
 	}
 
 	return vpc, nil

awsutil/route53.go

@@ -6,6 +6,7 @@ import (
 	"time"
 
 	"github.com/golang/glog"
+	"github.com/karlseguin/ccache"
 	"github.com/prometheus/client_golang/prometheus"
 
 	"github.com/aws/aws-sdk-go/aws"
@@ -21,6 +22,8 @@ type Route53 struct {
 	Svc route53iface.Route53API
 }
 
+var r53Cache = ccache.New(ccache.Configure())
+
 func NewRoute53(awsconfig *aws.Config) *Route53 {
 	awsSession, err := session.NewSession(awsconfig)
 	if err != nil {
@@ -69,7 +72,7 @@ func (r *Route53) GetZoneID(hostname *string) (*route53.HostedZone, error) {
 		return nil, err
 	}
 
-	item := Cache.Get("r53zone " + *zone)
+	item := r53Cache.Get("r53zone " + *zone)
 	if item != nil {
 		AWSCache.With(prometheus.Labels{"cache": "zone", "action": "hit"}).Add(float64(1))
 		return item.Value().(*route53.HostedZone), nil
@@ -101,7 +104,7 @@ func (r *Route53) GetZoneID(hostname *string) (*route53.HostedZone, error) {
 		zoneName := strings.TrimSuffix(*i.Name, ".")
 		if *zone == zoneName {
 			// glog.Infof("Found DNS Zone %s with ID %s", zoneName, *i.Id)
-			Cache.Set("r53zone "+*zone, i, time.Minute*60)
+			r53Cache.Set("r53zone "+*zone, i, time.Minute*60)
 			return i, nil
 		}
 	}

awsutil/util.go

@@ -1,41 +1,53 @@
 package awsutil
 
-import (
-	"github.com/karlseguin/ccache"
-	"github.com/prometheus/client_golang/prometheus"
-)
+import "github.com/prometheus/client_golang/prometheus"
 
 var (
+	// Route53svc is a pointer to the awsutil Route53 service
 	Route53svc *Route53
-	Elbv2svc   *ELBV2
-	Ec2svc     *EC2
-	AWSDebug   bool
+	// Elbv2svc is a pointer to the awsutil ELBV2 service
+	Elbv2svc *ELBV2
+	// Ec2svc is a pointer to the awsutil EC2 service
+	Ec2svc *EC2
+	// AWSDebug turns on AWS API debug logging
+	AWSDebug bool
 
+	// OnUpdateCount is a counter of the controller OnUpdate calls
 	OnUpdateCount = prometheus.NewCounter(prometheus.CounterOpts{
 		Name: "albingress_updates",
 		Help: "Number of times OnUpdate has been called.",
 	},
 	)
+
+	// ReloadCount is a counter of the controller Reload calls
 	ReloadCount = prometheus.NewCounter(prometheus.CounterOpts{
 		Name: "albingress_reloads",
 		Help: "Number of times Reload has been called.",
 	},
 	)
+
+	// AWSErrorCount is a counter of AWS errors
 	AWSErrorCount = prometheus.NewCounterVec(prometheus.CounterOpts{
 		Name: "albingress_aws_errors",
 		Help: "Number of errors from the AWS API",
 	},
 		[]string{"service", "request"},
 	)
+
+	// ManagedIngresses contains the current tally of managed ingresses
 	ManagedIngresses = prometheus.NewGauge(prometheus.GaugeOpts{
 		Name: "albingress_managed_ingresses",
 		Help: "Number of ingresses being managed",
 	})
+
+	// AWSCache contains the hits and misses to our caches
 	AWSCache = prometheus.NewCounterVec(prometheus.CounterOpts{
 		Name: "albingress_cache",
 		Help: "Number of ingresses being managed",
 	},
 		[]string{"cache", "action"})
+
+	// AWSRequest contains the requests made to the AWS API
 	AWSRequest = prometheus.NewCounterVec(prometheus.CounterOpts{
 		Name: "albingress_aws_requests",
 		Help: "Number of requests made to the AWS API",
@@ -51,5 +63,3 @@ func init() {
 	prometheus.MustRegister(AWSCache)
 	prometheus.MustRegister(AWSRequest)
 }
-
-var Cache = ccache.New(ccache.Configure())

controller/alb/listener.go

@@ -12,15 +12,17 @@ import (
 	"github.com/prometheus/client_golang/prometheus"
 )
 
+// Listener contains the relevant ID, Rules, and current/desired Listeners
 type Listener struct {
-	IngressId       *string
+	IngressID       *string
 	CurrentListener *elbv2.Listener
 	DesiredListener *elbv2.Listener
 	Rules           Rules
 	deleted         bool
 }
 
-func NewListener(annotations *config.AnnotationsT, ingressId *string) []*Listener {
+// NewListener returns a new alb.Listener based on the parameters provided.
+func NewListener(annotations *config.AnnotationsT, ingressID *string) []*Listener {
 	listeners := []*Listener{}
 
 	for _, port := range annotations.Port {
@@ -49,7 +51,7 @@ func NewListener(annotations *config.AnnotationsT, ingressId *string) []*Listene
 
 		listenerT := &Listener{
 			DesiredListener: listener,
-			IngressId:       ingressId,
+			IngressID:       ingressID,
 		}
 
 		listeners = append(listeners, listenerT)
@@ -66,21 +68,21 @@ func (l *Listener) SyncState(lb *LoadBalancer) *Listener {
 	switch {
 	// No DesiredState means Listener should be deleted.
 	case l.DesiredListener == nil:
-		log.Infof("Start Listener deletion.", *l.IngressId)
+		log.Infof("Start Listener deletion.", *l.IngressID)
 		l.delete(lb)
 
 	// No CurrentState means Listener doesn't exist in AWS and should be created.
 	case l.CurrentListener == nil:
-		log.Infof("Start Listener creation.", *l.IngressId)
+		log.Infof("Start Listener creation.", *l.IngressID)
 		l.create(lb)
 
 	// Current and Desired exist and need for modification should be evaluated.
 	case l.needsModification(l.DesiredListener):
-		log.Infof("Start Listener modification.", *l.IngressId)
+		log.Infof("Start Listener modification.", *l.IngressID)
 		l.modify(lb)
 
 	default:
-		log.Debugf("No listener modification required.", *l.IngressId)
+		log.Debugf("No listener modification required.", *l.IngressID)
 	}
 
 	return l
@@ -98,11 +100,11 @@ func (l *Listener) create(lb *LoadBalancer) error {
 	// use the Kubernetes service name attached to that.
 	for _, rule := range l.Rules {
 		if *rule.DesiredRule.IsDefault {
-			log.Infof("Located default rule. Rule: %s", *l.IngressId, log.Prettify(rule.DesiredRule))
+			log.Infof("Located default rule. Rule: %s", *l.IngressID, log.Prettify(rule.DesiredRule))
 			tgIndex := lb.TargetGroups.LookupBySvc(rule.SvcName)
 			if tgIndex < 0 {
 				log.Errorf("Failed to locate TargetGroup related to this service. Defaulting to first Target Group. SVC: %s",
-					*l.IngressId, rule.SvcName)
+					*l.IngressID, rule.SvcName)
 			} else {
 				ctg := lb.TargetGroups[tgIndex].CurrentTargetGroup
 				l.DesiredListener.DefaultActions[0].TargetGroupArn = ctg.TargetGroupArn
@@ -126,20 +128,20 @@ func (l *Listener) create(lb *LoadBalancer) error {
 	createListenerOutput, err := awsutil.Elbv2svc.Svc.CreateListener(createListenerInput)
 	if err != nil && err.(awserr.Error).Code() != "TargetGroupAssociationLimit" {
 		awsutil.AWSErrorCount.With(prometheus.Labels{"service": "ELBV2", "request": "CreateListener"}).Add(float64(1))
-		log.Errorf("Failed Listener creation. Error: %s.", *l.IngressId, err.Error())
+		log.Errorf("Failed Listener creation. Error: %s.", *l.IngressID, err.Error())
 		return err
 	} else if err != nil && err.(awserr.Error).Code() == "TargetGroupAssociationLimit" {
 		awsutil.AWSErrorCount.With(prometheus.Labels{"service": "ELBV2", "request": "CreateListener"}).Add(float64(1))
 		glog.Error("Received a TargetGroupAssociationLimit error")
 		// Something strange happening here, the original Listener doesnt have the LoadBalancerArn but a describe will return a Listener with the ARN
 		// l, _ := elbv2svc.describeListeners(lb.LoadBalancer.LoadBalancerArn)
-		log.Errorf("Failed Listener creation. Error: %s.", *l.IngressId, err.Error())
+		log.Errorf("Failed Listener creation. Error: %s.", *l.IngressID, err.Error())
 		return err
 	}
 
 	l.CurrentListener = createListenerOutput.Listeners[0]
 	log.Infof("Completed Listener creation. ARN: %s | Port: %s | Proto: %s.",
-		*l.IngressId, *l.CurrentListener.ListenerArn, *l.CurrentListener.Port, *l.CurrentListener.Protocol)
+		*l.IngressID, *l.CurrentListener.ListenerArn, *l.CurrentListener.Port, *l.CurrentListener.Protocol)
 	return nil
 }
 
@@ -150,12 +152,12 @@ func (l *Listener) modify(lb *LoadBalancer) error {
 		return l.create(lb)
 	}
 
-	glog.Infof("Modifying existing %s listener %s", *lb.Id, *l.CurrentListener.ListenerArn)
+	glog.Infof("Modifying existing %s listener %s", *lb.ID, *l.CurrentListener.ListenerArn)
 	//glog.Infof("Have %v, want %v", l.CurrentListener, l.DesiredListener)
 	glog.Info("NOT IMPLEMENTED!!!!")
 
 	log.Infof("Completed Listener modification. ARN: %s | Port: %s | Proto: %s.",
-		*l.IngressId, *l.CurrentListener.ListenerArn, *l.CurrentListener.Port, *l.CurrentListener.Protocol)
+		*l.IngressID, *l.CurrentListener.ListenerArn, *l.CurrentListener.Port, *l.CurrentListener.Protocol)
 	return nil
 }
 
@@ -174,8 +176,8 @@ func (l *Listener) delete(lb *LoadBalancer) error {
 		if awsErr.Code() == elbv2.ErrCodeListenerNotFoundException {
 			// TODO: Reorder syncs so route53 is last and this is handled in R53 resource record set syncs
 			// (relates to https://git.tm.tmcs/kubernetes/alb-ingress/issues/33)
-			log.Warnf("Listener not found during deletion attempt. It was likely already deleted when the ELBV2 (ALB) was deleted.", *l.IngressId)
-			log.Infof("Completed Listener deletion. ARN: %s", *l.IngressId, *l.CurrentListener.ListenerArn)
+			log.Warnf("Listener not found during deletion attempt. It was likely already deleted when the ELBV2 (ALB) was deleted.", *l.IngressID)
+			log.Infof("Completed Listener deletion. ARN: %s", *l.IngressID, *l.CurrentListener.ListenerArn)
 			lb.Deleted = true
 			return nil
 		}
@@ -185,7 +187,7 @@ func (l *Listener) delete(lb *LoadBalancer) error {
 	}
 
 	l.deleted = true
-	log.Infof("Completed Listener deletion. ARN: %s", *l.IngressId, *l.CurrentListener.ListenerArn)
+	log.Infof("Completed Listener deletion. ARN: %s", *l.IngressID, *l.CurrentListener.ListenerArn)
 	return nil
 }
 

controller/alb/listeners.go

@@ -4,10 +4,12 @@ import (
 	"github.com/aws/aws-sdk-go/service/elbv2"
 )
 
+// Listeners is a slice of Listener pointers
 type Listeners []*Listener
 
-func (l Listeners) Find(listener *elbv2.Listener) int {
-	for p, v := range l {
+// Find returns the position of the listener, returning -1 if unfound.
+func (ls Listeners) Find(listener *elbv2.Listener) int {
+	for p, v := range ls {
 		if !v.needsModification(listener) {
 			return p
 		}
@@ -35,8 +37,9 @@ func (ls Listeners) SyncState(lb *LoadBalancer, tgs *TargetGroups) Listeners {
 	return listeners
 }
 
-func (l Listeners) StripDesiredState() {
-	for _, listener := range l {
+// StripDesiredState removes the DesiredListener from all Listeners in the slice.
+func (ls Listeners) StripDesiredState() {
+	for _, listener := range ls {
 		listener.DesiredListener = nil
 	}
 }
@@ -47,8 +50,8 @@ func (l Listeners) StripDesiredState() {
 //
 // Additionally, since Rules are also removed its Listener is, this also calles StripDesiredState on
 // the Rules attached to each listener.
-func (l Listeners) StripCurrentState() {
-	for _, listener := range l {
+func (ls Listeners) StripCurrentState() {
+	for _, listener := range ls {
 		listener.CurrentListener = nil
 		listener.Rules.StripCurrentState()
 	}