Refactored albingresses into albingress

Author: bigkraig

pkg/albingress/albingress.go

@@ -18,31 +18,6 @@ import (
 	util "github.com/kubernetes-sigs/aws-alb-ingress-controller/pkg/util/types"
 )
 
-var logger *log.Logger
-
-func init() {
-	logger = log.New("ingress")
-}
-
-// ALBIngress contains all information above the cluster, ingress resource, and AWS resources
-// needed to assemble an ALB, TargetGroup, Listener and Rules.
-type ALBIngress struct {
-	ID                    string
-	namespace             string
-	ingressName           string
-	clusterName           string
-	albNamePrefix         string
-	recorder              record.EventRecorder
-	ingress               *extensions.Ingress
-	lock                  *sync.Mutex
-	annotations           *annotations.Annotations
-	ManagedSecurityGroups util.AWSStringSlice // sgs managed by this controller rather than annotation
-	LoadBalancer          *lb.LoadBalancer
-	valid                 bool
-	logger                *log.Logger
-	Reconciled            bool
-}
-
 type NewALBIngressOptions struct {
 	Namespace     string
 	Name          string
@@ -53,26 +28,21 @@ type NewALBIngressOptions struct {
 	Reconciled    bool
 }
 
-// ID returns an ingress id based off of a namespace and name
-func ID(namespace, name string) string {
-	return fmt.Sprintf("%s/%s", namespace, name)
-}
-
 // NewALBIngress returns a minimal ALBIngress instance with a generated name that allows for lookup
 // when new ALBIngress objects are created to determine if an instance of that ALBIngress already
 // exists.
 func NewALBIngress(o *NewALBIngressOptions) *ALBIngress {
-	ingressID := ID(o.Namespace, o.Name)
+	ingressID := GenerateID(o.Namespace, o.Name)
 	return &ALBIngress{
-		ID:            ingressID,
+		id:            ingressID,
 		namespace:     o.Namespace,
 		clusterName:   o.ClusterName,
 		albNamePrefix: o.ALBNamePrefix,
 		ingressName:   o.Name,
 		lock:          new(sync.Mutex),
 		logger:        log.New(ingressID),
 		recorder:      o.Recorder,
-		Reconciled:    o.Reconciled,
+		reconciled:    o.Reconciled,
 		ingress:       o.Ingress,
 	}
 }
@@ -115,15 +85,15 @@ func NewALBIngressFromIngress(o *NewALBIngressFromIngressOptions, annotationFact
 		newIngress.ingress = o.Ingress
 		// Ensure all desired state is removed from the copied ingress. The desired state of each
 		// component will be generated later in this function.
-		newIngress.StripDesiredState()
+		newIngress.stripDesiredState()
 		newIngress.valid = false
 	}
 
 	// Load up the ingress with our current annotations.
 	newIngress.annotations, err = annotationFactory.ParseAnnotations(o.Ingress)
 	if err != nil {
 		msg := fmt.Sprintf("Error parsing annotations: %s", err.Error())
-		newIngress.Reconciled = false
+		newIngress.reconciled = false
 		newIngress.Eventf(api.EventTypeWarning, "ERROR", msg)
 		newIngress.logger.Errorf(msg)
 		return newIngress
@@ -139,15 +109,15 @@ func NewALBIngressFromIngress(o *NewALBIngressFromIngressOptions, annotationFact
 	}
 
 	// Assemble the load balancer
-	newIngress.LoadBalancer, err = lb.NewDesiredLoadBalancer(&lb.NewDesiredLoadBalancerOptions{
+	newIngress.loadBalancer, err = lb.NewDesiredLoadBalancer(&lb.NewDesiredLoadBalancerOptions{
 		ALBNamePrefix:        o.ALBNamePrefix,
 		Namespace:            o.Ingress.GetNamespace(),
-		ExistingLoadBalancer: newIngress.LoadBalancer,
+		ExistingLoadBalancer: newIngress.loadBalancer,
 		IngressName:          o.Ingress.Name,
 		IngressRules:         o.Ingress.Spec.Rules,
 		Logger:               newIngress.logger,
 		Annotations:          newIngress.annotations,
-		Tags:                 newIngress.Tags(),
+		Tags:                 newIngress.Tags(o.ClusterName),
 		GetServiceNodePort:   o.GetServiceNodePort,
 		GetNodes:             o.GetNodes,
 	})
@@ -156,7 +126,7 @@ func NewALBIngressFromIngress(o *NewALBIngressFromIngressOptions, annotationFact
 		msg := fmt.Sprintf("Error instantiating load balancer: %s", err.Error())
 		newIngress.Eventf(api.EventTypeWarning, "ERROR", msg)
 		newIngress.logger.Errorf(msg)
-		newIngress.Reconciled = false
+		newIngress.reconciled = false
 		return newIngress
 	}
 
@@ -204,7 +174,7 @@ func NewALBIngressFromAWSLoadBalancer(o *NewALBIngressFromAWSLoadBalancerOptions
 	})
 
 	// Assemble load balancer
-	ingress.LoadBalancer, err = lb.NewCurrentLoadBalancer(&lb.NewCurrentLoadBalancerOptions{
+	ingress.loadBalancer, err = lb.NewCurrentLoadBalancer(&lb.NewCurrentLoadBalancerOptions{
 		LoadBalancer:          o.LoadBalancer,
 		Tags:                  tags,
 		ALBNamePrefix:         o.ALBNamePrefix,
@@ -238,20 +208,24 @@ func (a *ALBIngress) Eventf(eventtype, reason, messageFmt string, args ...interf
 func (a *ALBIngress) Hostnames() ([]api.LoadBalancerIngress, error) {
 	var hostnames []api.LoadBalancerIngress
 
-	if a.LoadBalancer == nil {
+	if a.reconciled != true {
+		return nil, fmt.Errorf("ingress is not in sync, hostname invalid")
+	}
+
+	if a.loadBalancer == nil {
 		return hostnames, nil
 	}
 
-	if a.LoadBalancer.Hostname() != nil {
-		hostnames = append(hostnames, api.LoadBalancerIngress{Hostname: *a.LoadBalancer.Hostname()})
+	if a.loadBalancer.Hostname() != nil {
+		hostnames = append(hostnames, api.LoadBalancerIngress{Hostname: *a.loadBalancer.Hostname()})
 	}
 
 	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())
+	a.logger.Debugf("Ingress library requested hostname list and we returned %s", *a.loadBalancer.Hostname())
 	return hostnames, nil
 }
 
@@ -264,25 +238,20 @@ func (a *ALBIngress) Reconcile(rOpts *ReconcileOptions) {
 		return
 	}
 
-	errors := a.LoadBalancer.Reconcile(
+	errors := a.loadBalancer.Reconcile(
 		&lb.ReconcileOptions{
 			Eventf: rOpts.Eventf,
 		})
 	if len(errors) > 0 {
 		// marks reconciled state as false so UpdateIngressStatus won't operate
-		a.Reconciled = false
+		a.reconciled = false
 		a.logger.Errorf("Failed to reconcile state on this ingress")
 		for _, err := range errors {
 			a.logger.Errorf(" - %s", err.Error())
 		}
 	}
 	// marks reconciled state as true so that UpdateIngressStatus will operate
-	a.Reconciled = true
-}
-
-// Name returns the name of the ingress
-func (a *ALBIngress) Name() string {
-	return fmt.Sprintf("%s-%s", a.namespace, a.ingressName)
+	a.reconciled = true
 }
 
 // Namespace returns the namespace of the ingress
@@ -291,16 +260,22 @@ func (a *ALBIngress) Namespace() string {
 }
 
 // StripDesiredState strips all desired objects from an ALBIngress
-func (a *ALBIngress) StripDesiredState() {
-	if a.LoadBalancer != nil {
-		a.LoadBalancer.StripDesiredState()
+func (a *ALBIngress) stripDesiredState() {
+	if a.loadBalancer != nil {
+		a.loadBalancer.StripDesiredState()
 	}
 }
 
 // Tags returns an elbv2.Tag slice of standard tags for the ingress AWS resources
-func (a *ALBIngress) Tags() []*elbv2.Tag {
+func (a *ALBIngress) Tags(clusterName string) []*elbv2.Tag {
 	tags := a.annotations.Tags
 
+	// https://github.com/kubernetes/kubernetes/blob/master/pkg/cloudprovider/providers/aws/tags.go
+	tags = append(tags, &elbv2.Tag{
+		Key:   aws.String("kubernetes.io/cluster/" + clusterName),
+		Value: aws.String("owned"),
+	})
+
 	tags = append(tags, &elbv2.Tag{
 		Key:   aws.String("Namespace"),
 		Value: aws.String(a.namespace),
@@ -317,3 +292,12 @@ func (a *ALBIngress) Tags() []*elbv2.Tag {
 type ReconcileOptions struct {
 	Eventf func(string, string, string, ...interface{})
 }
+
+// id returns an ingress id based off of a namespace and name
+func GenerateID(namespace, name string) string {
+	return fmt.Sprintf("%s/%s", namespace, name)
+}
+
+func (a *ALBIngress) GetLoadBalancer() *lb.LoadBalancer {
+	return a.loadBalancer
+}

pkg/albingresses/albingresses.go renamed to pkg/albingress/albingresses.go

@@ -1,4 +1,4 @@
-package albingresses
+package albingress
 
 import (
 	"strconv"
@@ -12,25 +12,13 @@ import (
 	"k8s.io/client-go/tools/record"
 	"k8s.io/ingress/core/pkg/ingress/annotations/class"
 
-	"github.com/kubernetes-sigs/aws-alb-ingress-controller/pkg/albingress"
 	"github.com/kubernetes-sigs/aws-alb-ingress-controller/pkg/annotations"
 	"github.com/kubernetes-sigs/aws-alb-ingress-controller/pkg/aws/ec2"
 	albelbv2 "github.com/kubernetes-sigs/aws-alb-ingress-controller/pkg/aws/elbv2"
 	"github.com/kubernetes-sigs/aws-alb-ingress-controller/pkg/aws/waf"
-	"github.com/kubernetes-sigs/aws-alb-ingress-controller/pkg/util/log"
 	util "github.com/kubernetes-sigs/aws-alb-ingress-controller/pkg/util/types"
 )
 
-// ALBIngresses is a list of ALBIngress. It is held by the ALBController instance and evaluated
-// against to determine what should be created, deleted, and modified.
-type ALBIngresses []*albingress.ALBIngress
-
-var logger *log.Logger
-
-func init() {
-	logger = log.New("ingresses")
-}
-
 // NewALBIngressesFromIngressesOptions are the options to NewALBIngressesFromIngresses
 type NewALBIngressesFromIngressesOptions struct {
 	Recorder            record.EventRecorder
@@ -58,12 +46,12 @@ func NewALBIngressesFromIngresses(o *NewALBIngressesFromIngressesOptions, annota
 		}
 
 		// Find the existing ingress for this Kubernetes ingress (if it existed).
-		id := albingress.ID(ingResource.GetNamespace(), ingResource.Name)
+		id := GenerateID(ingResource.GetNamespace(), ingResource.Name)
 		_, existingIngress := o.ALBIngresses.FindByID(id)
 
 		// Produce a new ALBIngress instance for every ingress found. If ALBIngress returns nil, there
 		// was an issue with the ingress (e.g. bad annotations) and should not be added to the list.
-		ALBIngress := albingress.NewALBIngressFromIngress(&albingress.NewALBIngressFromIngressOptions{
+		ALBIngress := NewALBIngressFromIngress(&NewALBIngressFromIngressOptions{
 			Ingress:            ingResource,
 			ExistingIngress:    existingIngress,
 			ClusterName:        o.ClusterName,
@@ -173,7 +161,7 @@ func AssembleIngressesFromAWS(o *AssembleIngressesFromAWSOptions) ALBIngresses {
 				wafACLID = wafResult.WebACLId
 			}
 
-			albIngress, err := albingress.NewALBIngressFromAWSLoadBalancer(&albingress.NewALBIngressFromAWSLoadBalancerOptions{
+			albIngress, err := NewALBIngressFromAWSLoadBalancer(&NewALBIngressFromAWSLoadBalancerOptions{
 				LoadBalancer:          loadBalancer,
 				ALBNamePrefix:         o.ALBNamePrefix,
 				Recorder:              o.Recorder,
@@ -199,9 +187,9 @@ func AssembleIngressesFromAWS(o *AssembleIngressesFromAWSOptions) ALBIngresses {
 }
 
 // FindByID locates the ingress by the id parameter and returns its position
-func (a ALBIngresses) FindByID(id string) (int, *albingress.ALBIngress) {
+func (a ALBIngresses) FindByID(id string) (int, *ALBIngress) {
 	for p, v := range a {
-		if v.ID == id {
+		if v.id == id {
 			return p, v
 		}
 	}
@@ -216,13 +204,13 @@ func (a ALBIngresses) RemovedIngresses(newList ALBIngresses) ALBIngresses {
 	// Loop through every ingress in current (old) ingress list known to ALBController
 	for _, ingress := range a {
 		// Ingress objects not found in newList might qualify for deletion.
-		if i, _ := newList.FindByID(ingress.ID); i < 0 {
+		if i, _ := newList.FindByID(ingress.id); i < 0 {
 			// If the ALBIngress still contains a LoadBalancer, it still needs to be deleted.
 			// In this case, strip all desired state and add it to the deleteableIngress list.
 			// If the ALBIngress contains no LoadBalancer, it was previously deleted and is
 			// no longer relevant to the ALBController.
-			if ingress.LoadBalancer != nil {
-				ingress.StripDesiredState()
+			if ingress.loadBalancer != nil {
+				ingress.stripDesiredState()
 				deleteableIngress = append(deleteableIngress, ingress)
 			}
 		}

pkg/albingress/types.go

@@ -0,0 +1,41 @@
+package albingress
+
+import (
+	"sync"
+
+	"github.com/kubernetes-sigs/aws-alb-ingress-controller/pkg/alb/lb"
+	"github.com/kubernetes-sigs/aws-alb-ingress-controller/pkg/annotations"
+	"github.com/kubernetes-sigs/aws-alb-ingress-controller/pkg/util/log"
+	util "github.com/kubernetes-sigs/aws-alb-ingress-controller/pkg/util/types"
+	extensions "k8s.io/api/extensions/v1beta1"
+	"k8s.io/client-go/tools/record"
+)
+
+var logger *log.Logger
+
+func init() {
+	logger = log.New("ingress")
+}
+
+// ALBIngresses is a list of ALBIngress. It is held by the ALBController instance and evaluated
+// against to determine what should be created, deleted, and modified.
+type ALBIngresses []*ALBIngress
+
+// ALBIngress contains all information above the cluster, ingress resource, and AWS resources
+// needed to assemble an ALB, TargetGroup, Listener and Rules.
+type ALBIngress struct {
+	id                    string
+	namespace             string
+	ingressName           string
+	clusterName           string
+	albNamePrefix         string
+	recorder              record.EventRecorder
+	ingress               *extensions.Ingress
+	lock                  *sync.Mutex
+	annotations           *annotations.Annotations
+	managedSecurityGroups util.AWSStringSlice // sgs managed by this controller rather than annotation
+	loadBalancer          *lb.LoadBalancer
+	valid                 bool
+	logger                *log.Logger
+	reconciled            bool
+}