fix for pod condition type too long

go.mod

@@ -2,7 +2,7 @@ module github.com/kubernetes-sigs/aws-alb-ingress-controller
 
 require (
 	github.com/appscode/jsonpatch v0.0.0-20190108182946-7c0e3b262f30 // indirect
-	github.com/aws/aws-k8s-tester/e2e/tester v0.0.0-20191205114352-5bcf20ecd383 // indirect
+	github.com/aws/aws-k8s-tester/e2e/tester v0.0.0-20200603230341-7330f5b419cc // indirect
 	github.com/aws/aws-sdk-go v1.27.3
 	github.com/blang/semver v3.5.1+incompatible
 	github.com/go-logr/glogr v0.1.0
@@ -11,6 +11,7 @@ require (
 	github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b
 	github.com/golang/mock v1.2.0
 	github.com/golangci/golangci-lint v1.21.0 // indirect
+	github.com/google/go-cmp v0.4.1
 	github.com/google/gofuzz v0.0.0-20170612174753-24818f796faf // indirect
 	github.com/googleapis/gnostic v0.2.0 // indirect
 	github.com/gregjones/httpcache v0.0.0-20190212212710-3befbb6ad0cc // indirect

go.sum

@@ -11,14 +11,11 @@ github.com/alecthomas/units v0.0.0-20151022065526-2efee857e7cf/go.mod h1:ybxpYRF
 github.com/appscode/jsonpatch v0.0.0-20190108182946-7c0e3b262f30 h1:Kn3rqvbUFqSepE2OqVu0Pn1CbDw9IuMlONapol0zuwk=
 github.com/appscode/jsonpatch v0.0.0-20190108182946-7c0e3b262f30/go.mod h1:4AJxUpXUhv4N+ziTvIcWWXgeorXpxPZOfk9HdEVr96M=
 github.com/armon/consul-api v0.0.0-20180202201655-eb2c6b5be1b6/go.mod h1:grANhF5doyWs3UAsr3K4I6qtAmlQcZDesFNEHPZAzj8=
-github.com/aws/aws-k8s-tester v0.4.5-0.20191203073900-e5a28058b567 h1:tzabayjAiOaNO1nEoy+Fj9JHs1X3+LYqrRHbJRfVwqg=
-github.com/aws/aws-k8s-tester v0.5.1-0.20191205114352-5bcf20ecd383 h1:d6+rwKyAFMAyb7az2fq4sfDhCB1ykCgLkpogwDepns0=
-github.com/aws/aws-k8s-tester/e2e/tester v0.0.0-20190907061006-260b0e114d90 h1:FRpHLOVjM/FO/sl84ilNQWATtRd1FR6uk7UUs8MUl5Y=
-github.com/aws/aws-k8s-tester/e2e/tester v0.0.0-20190907061006-260b0e114d90/go.mod h1:xCa3ZGICI7/IqtJdYjEsM3QL9vwlLHxgwSA/MD09Zgo=
-github.com/aws/aws-k8s-tester/e2e/tester v0.0.0-20191203073900-e5a28058b567 h1:6zSO87AbCbY3doj1VEaqIs9Z81QxzNX0LWF7o2P6liU=
-github.com/aws/aws-k8s-tester/e2e/tester v0.0.0-20191203073900-e5a28058b567/go.mod h1:DJkIJa8BA0LCJWjl01jsEl8vm/fjei0uWJFzZol0KUI=
+github.com/aws/aws-k8s-tester v1.3.0 h1:J3v0DeWvx6PtK01fzAlWE2+/OLWm7h7KW7nxYo+fY5w=
 github.com/aws/aws-k8s-tester/e2e/tester v0.0.0-20191205114352-5bcf20ecd383 h1:0ZF830kkP4LvV7mEEm4c4nI4UM1LRqVntW2jPfC3GFc=
 github.com/aws/aws-k8s-tester/e2e/tester v0.0.0-20191205114352-5bcf20ecd383/go.mod h1:DJkIJa8BA0LCJWjl01jsEl8vm/fjei0uWJFzZol0KUI=
+github.com/aws/aws-k8s-tester/e2e/tester v0.0.0-20200603230341-7330f5b419cc h1:k3ULeEufqKLvL/b+8A6XXE6mb7cTot7WnWfKS24ZcbM=
+github.com/aws/aws-k8s-tester/e2e/tester v0.0.0-20200603230341-7330f5b419cc/go.mod h1:DJkIJa8BA0LCJWjl01jsEl8vm/fjei0uWJFzZol0KUI=
 github.com/aws/aws-sdk-go v1.15.39/go.mod h1:mFuSZ37Z9YOHbQEwBWztmVzqXrEkub65tZoCYDt7FT0=
 github.com/aws/aws-sdk-go v1.23.21 h1:eVJT2C99cAjZlBY8+CJovf6AwrSANzAcYNuxdCB+SPk=
 github.com/aws/aws-sdk-go v1.23.21/go.mod h1:KmX6BPdI08NWTb3/sm4ZGu5ShLoqVDhKgpiN924inxo=
@@ -143,7 +140,10 @@ github.com/golangci/unconvert v0.0.0-20180507085042-28b1c447d1f4/go.mod h1:Izgrg
 github.com/google/btree v0.0.0-20180813153112-4030bb1f1f0c/go.mod h1:lNA+9X1NB3Zf8V7Ke586lFgjr2dZNuvo3lPJSGZ5JPQ=
 github.com/google/btree v1.0.0 h1:0udJVsspx3VBr5FwtLhQQtuAsVc79tTq0ocGIPAU6qo=
 github.com/google/btree v1.0.0/go.mod h1:lNA+9X1NB3Zf8V7Ke586lFgjr2dZNuvo3lPJSGZ5JPQ=
+github.com/google/go-cmp v0.2.0 h1:+dTQ8DZQJz0Mb/HjFlkptS1FeQ4cWSnN941F8aEG4SQ=
 github.com/google/go-cmp v0.2.0/go.mod h1:oXzfMopK8JAjlY9xF4vHSVASa0yLyX7SntLO5aqRK0M=
+github.com/google/go-cmp v0.4.1 h1:/exdXoGamhu5ONeUJH0deniYLWYvQwW66yvlfiiKTu0=
+github.com/google/go-cmp v0.4.1/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
 github.com/google/gofuzz v0.0.0-20170612174753-24818f796faf h1:+RRA9JqSOZFfKrOeqr2z77+8R2RKyh8PG66dcu1V0ck=
 github.com/google/gofuzz v0.0.0-20170612174753-24818f796faf/go.mod h1:HP5RmnzzSNb993RKQDq4+1A4ia9nllfqcQFTQJedwGI=
 github.com/google/renameio v0.1.0/go.mod h1:KWCgfxg9yswjAJkECMjeO8J8rahYeXnNhOm40UhjYkI=
@@ -392,6 +392,8 @@ golang.org/x/tools v0.0.0-20190930201159-7c411dea38b0/go.mod h1:b+2E5dAYhXwXZwtn
 golang.org/x/tools v0.0.0-20191010075000-0337d82405ff h1:XdBG6es/oFDr1HwaxkxgVve7NB281QhxgK/i4voubFs=
 golang.org/x/tools v0.0.0-20191010075000-0337d82405ff/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo=
 golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
+golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543 h1:E7g+9GITq07hpfrRu66IVDexMakfv52eLZ2CXBWiKr4=
+golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
 google.golang.org/appengine v1.1.0/go.mod h1:EbEs0AVv82hx2wNQdGPgUI5lhzA/G0D9YwlJXL52JkM=
 google.golang.org/appengine v1.4.0 h1:/wp5JvzpHIxhs/dumFmF7BXTf3Z+dd4uXta4kVyO508=
 google.golang.org/appengine v1.4.0/go.mod h1:xpcJRLb0r/rnEns0DIKYYv+WjYCduHsrkT7/EB5XEv4=

internal/alb/lb/loadbalancer.go

@@ -402,10 +402,6 @@ func subnetIsUsable(new *ec2.Subnet, existing []*ec2.Subnet) bool {
 			return false
 		}
 	}
-
-	if aws.Int64Value(new.AvailableIpAddressCount) < 8 {
-		return false
-	}
-
-	return true
+
+	return aws.Int64Value(new.AvailableIpAddressCount) >= 8
 }

internal/alb/tg/targethealth.go

@@ -2,14 +2,14 @@ package tg
 
 import (
 	"context"
-	"fmt"
 	"time"
 
 	"github.com/aws/aws-sdk-go/service/elbv2"
 	"github.com/kubernetes-sigs/aws-alb-ingress-controller/internal/albctx"
 	"github.com/kubernetes-sigs/aws-alb-ingress-controller/internal/aws"
 	"github.com/kubernetes-sigs/aws-alb-ingress-controller/internal/ingress/annotations/healthcheck"
 	"github.com/kubernetes-sigs/aws-alb-ingress-controller/internal/ingress/backend"
+	backendpkg "github.com/kubernetes-sigs/aws-alb-ingress-controller/internal/ingress/backend"
 	"github.com/kubernetes-sigs/aws-alb-ingress-controller/internal/ingress/controller/store"
 	"github.com/kubernetes-sigs/aws-alb-ingress-controller/internal/k8s"
 	api "k8s.io/api/core/v1"
@@ -75,9 +75,11 @@ type targetHealthController struct {
 
 // SyncTargetsForReconciliation starts a go routine for reconciling pod condition statuses for the given targets in the background until they are healthy in the target group
 func (c *targetHealthController) SyncTargetsForReconciliation(ctx context.Context, t *Targets, desiredTargets []*elbv2.TargetDescription) error {
-	conditionType := podConditionTypeForIngressBackend(t.Ingress, t.Backend)
-
-	targetsToReconcile, err := c.filterTargetsNeedingReconciliation(conditionType, t, desiredTargets)
+	readinessConditionTypes := []api.PodConditionType{
+		backendpkg.PodReadinessGateConditionType(t.Ingress, t.Backend),
+		backendpkg.AnyLBTGReadyConditionType,
+	}
+	targetsToReconcile, err := c.filterTargetsNeedingReconciliation(t, desiredTargets, readinessConditionTypes...)
 	if err != nil {
 		return err
 	}
@@ -102,7 +104,7 @@ func (c *targetHealthController) SyncTargetsForReconciliation(ctx context.Contex
 		c.tgWatches[t.TgArn] = tgWatch
 
 		// start watching target health in target group and updating pod condition status
-		go c.reconcilePodConditionsLoop(ctx, t.TgArn, conditionType, tgWatch)
+		go c.reconcilePodConditionsLoop(ctx, t.TgArn, tgWatch, readinessConditionTypes...)
 	}
 	tgWatch.interval <- c.ingressTargetHealthReconciliationInterval(t.Backend.ServiceName, t.Ingress)
 	tgWatch.targetsToReconcile <- targetsToReconcile
@@ -126,16 +128,25 @@ func (c *targetHealthController) RemovePodConditions(ctx context.Context, t *Tar
 		return err
 	}
 
-	conditionType := podConditionTypeForIngressBackend(t.Ingress, t.Backend)
-
+	readinessConditionTypes := []api.PodConditionType{
+		backendpkg.PodReadinessGateConditionType(t.Ingress, t.Backend),
+		backendpkg.AnyLBTGReadyConditionType,
+	}
 	for _, pod := range pods {
-		if pod != nil {
-			if i, cond := podConditionForReadinessGate(pod, conditionType); cond != nil {
+		if pod == nil {
+			continue
+		}
+		needUpdates := false
+		for _, conditionType := range readinessConditionTypes {
+			i, cond := backendpkg.PodConditionForReadinessGate(pod, conditionType)
+			if cond != nil {
 				pod.Status.Conditions = append(pod.Status.Conditions[:i], pod.Status.Conditions[i+1:]...)
-				err := c.client.Status().Update(ctx, pod)
-				if err != nil && !k8serrors.IsNotFound(err) {
-					return err
-				}
+				needUpdates = true
+			}
+		}
+		if needUpdates {
+			if err := c.client.Status().Update(ctx, pod); err != nil && !k8serrors.IsNotFound(err) {
+				return err
 			}
 		}
 	}
@@ -144,7 +155,7 @@ func (c *targetHealthController) RemovePodConditions(ctx context.Context, t *Tar
 }
 
 // Background loop which keeps reconciling pod condition statuses for the given target groups until the given context is cancelled.
-func (c *targetHealthController) reconcilePodConditionsLoop(ctx context.Context, tgArn string, conditionType api.PodConditionType, tgWatch *targetGroupWatch) {
+func (c *targetHealthController) reconcilePodConditionsLoop(ctx context.Context, tgArn string, tgWatch *targetGroupWatch, readinessConditionTypes ...api.PodConditionType) {
 	logger := albctx.GetLogger(ctx)
 	logger.Infof("Starting reconciliation of pod condition status for target group: %v", tgArn)
 
@@ -162,7 +173,7 @@ func (c *targetHealthController) reconcilePodConditionsLoop(ctx context.Context,
 		case targetsToReconcile = <-tgWatch.targetsToReconcile: // update targets
 
 		case <-reconcile:
-			notReadyTargets, err := c.reconcilePodConditions(ctx, tgArn, conditionType, tgWatch.ingress, tgWatch.backend, targetsToReconcile)
+			notReadyTargets, err := c.reconcilePodConditions(ctx, tgArn, tgWatch.ingress, tgWatch.backend, targetsToReconcile, readinessConditionTypes...)
 			if err == nil {
 				targetsToReconcile = notReadyTargets
 			} else {
@@ -175,11 +186,10 @@ func (c *targetHealthController) reconcilePodConditionsLoop(ctx context.Context,
 			return
 		}
 	}
-
 }
 
 // For each given pod, checks for the health status of the corresponding target in the target group and adds/updates a pod condition that can be used for pod readiness gates.
-func (c *targetHealthController) reconcilePodConditions(ctx context.Context, tgArn string, conditionType api.PodConditionType, ingress *extensions.Ingress, backend *extensions.IngressBackend, targetsToReconcile []*elbv2.TargetDescription) ([]*elbv2.TargetDescription, error) {
+func (c *targetHealthController) reconcilePodConditions(ctx context.Context, tgArn string, ingress *extensions.Ingress, backend *extensions.IngressBackend, targetsToReconcile []*elbv2.TargetDescription, readinessConditionTypes ...api.PodConditionType) ([]*elbv2.TargetDescription, error) {
 	var notReadyTargets []*elbv2.TargetDescription
 
 	in := &elbv2.DescribeTargetHealthInput{
@@ -206,85 +216,64 @@ func (c *targetHealthController) reconcilePodConditions(ctx context.Context, tgA
 			continue
 		}
 		targetHealth, ok := targetsHealth[pod.Status.PodIP]
-		if ok && podHasReadinessGate(pod, conditionType) {
-			if aws.StringValue(targetHealth.State) != elbv2.TargetHealthStateEnumHealthy {
-				notReadyTargets = append(notReadyTargets, target)
-			}
-			if err := c.reconcilePodCondition(ctx, conditionType, pod, targetHealth, true); err != nil {
-				return notReadyTargets, err
-			}
+		if !ok {
+			continue
 		}
-	}
-	return notReadyTargets, nil
-}
-
-// Creates or updates the condition status for the given pod with the given target health.
-func (c *targetHealthController) reconcilePodCondition(ctx context.Context, conditionType api.PodConditionType, pod *api.Pod, targetHealth *elbv2.TargetHealth, updateTimes bool) error {
-	conditionStatus := podConditionStatusFromTargetHealth(targetHealth)
 
-	// check if condition already exists
-	now := metav1.Now()
-	i, cond := podConditionForReadinessGate(pod, conditionType)
-	if cond == nil {
-		// new condition
-		targetHealthCondition := api.PodCondition{
-			Type:    conditionType,
-			Status:  conditionStatus,
-			Reason:  aws.StringValue(targetHealth.Reason),
-			Message: aws.StringValue(targetHealth.Description),
+		needsContinueProbe := false
+		needsUpdate := false
+		for _, conditionType := range readinessConditionTypes {
+			if backendpkg.PodHasReadinessGate(pod, conditionType) {
+				if readinessConditionBecomeHealthy := updatePodReadinessCondition(pod, targetHealth, conditionType); !readinessConditionBecomeHealthy {
+					needsContinueProbe = true
+				}
+				needsUpdate = true
+			}
 		}
-		if updateTimes {
-			targetHealthCondition.LastProbeTime = now
-			targetHealthCondition.LastTransitionTime = now
+		if needsContinueProbe {
+			notReadyTargets = append(notReadyTargets, target)
 		}
-		pod.Status.Conditions = append(pod.Status.Conditions, targetHealthCondition)
-	} else {
-		// update condition
-		if updateTimes {
-			cond.LastProbeTime = now
-			if cond.Status != conditionStatus {
-				cond.LastTransitionTime = now
+		if needsUpdate {
+			if err := c.client.Status().Update(ctx, pod); err != nil && !k8serrors.IsNotFound(err) {
+				return nil, err
 			}
 		}
-		cond.Status = conditionStatus
-		cond.Reason = aws.StringValue(targetHealth.Reason)
-		cond.Message = aws.StringValue(targetHealth.Description)
-		pod.Status.Conditions[i] = *cond
 	}
-
-	// pod will always be updated (at least to update `LastProbeTime`);
-	// this will trigger another invocation of `Reconcile`, which will remove this pod from the list of pods to reconcile if its health status is ok
-	err := c.client.Status().Update(ctx, pod)
-	if err != nil {
-		return err
-	}
-
-	return nil
+	return notReadyTargets, nil
 }
 
 // From the given targets, only returns the ones that have a readiness gate for the given ingress/service and whose pod conditions actually need to be reconciled.
-func (c *targetHealthController) filterTargetsNeedingReconciliation(conditionType api.PodConditionType, t *Targets, desiredTargets []*elbv2.TargetDescription) ([]*elbv2.TargetDescription, error) {
-	targetsToReconcile := []*elbv2.TargetDescription{}
+func (c *targetHealthController) filterTargetsNeedingReconciliation(t *Targets, desiredTargets []*elbv2.TargetDescription, readinessConditionTypes ...api.PodConditionType) ([]*elbv2.TargetDescription, error) {
 	if len(desiredTargets) == 0 {
-		return targetsToReconcile, nil
+		return nil, nil
 	}
-
 	// find the pods that correspond to the targets
 	pods, err := c.endpointResolver.ReverseResolve(t.Ingress, t.Backend, desiredTargets)
 	if err != nil {
-		return targetsToReconcile, err
+		return nil, err
 	}
 
+	var targetsToReconcile []*elbv2.TargetDescription
 	// filter out targets whose pods don't have the `readinessGate` for this target group or whose pod condition status is already `True`
 	for i, target := range desiredTargets {
 		pod := pods[i]
-		if pod != nil && podHasReadinessGate(pod, conditionType) {
-			if _, cond := podConditionForReadinessGate(pod, conditionType); cond == nil || cond.Status != api.ConditionTrue {
-				targetsToReconcile = append(targetsToReconcile, target)
+		if pod == nil {
+			continue
+		}
+
+		foundAnyUnreadyReadinessGate := false
+		for _, conditionType := range readinessConditionTypes {
+			if backendpkg.PodHasReadinessGate(pod, conditionType) {
+				if _, cond := backendpkg.PodConditionForReadinessGate(pod, conditionType); cond == nil || cond.Status != api.ConditionTrue {
+					foundAnyUnreadyReadinessGate = true
+					break
+				}
 			}
 		}
+		if foundAnyUnreadyReadinessGate {
+			targetsToReconcile = append(targetsToReconcile, target)
+		}
 	}
-
 	return targetsToReconcile, nil
 }
 
@@ -301,33 +290,37 @@ func (c *targetHealthController) ingressTargetHealthReconciliationInterval(servi
 	return healthcheck.DefaultIntervalSeconds
 }
 
-// PodConditionTypeForIngressBackend returns the PodConditionType that is associated with the given ingress and backend
-func podConditionTypeForIngressBackend(ingress *extensions.Ingress, backend *extensions.IngressBackend) api.PodConditionType {
-	return api.PodConditionType(fmt.Sprintf(
-		"target-health.alb.ingress.k8s.aws/%s_%s_%s",
-		ingress.Name,
-		backend.ServiceName,
-		backend.ServicePort.String(),
-	))
-}
+// updatePodReadinessCondition will creates or updates the condition status for the given pod with the given target health
+// returns whether pod readinessGate becomes healthy
+func updatePodReadinessCondition(pod *api.Pod, targetHealth *elbv2.TargetHealth, conditionType api.PodConditionType) bool {
+	conditionStatus := podConditionStatusFromTargetHealth(targetHealth)
 
-// PodHasReadinessGate returns true if the given pod has a readinessGate with the given conditionType
-func podHasReadinessGate(pod *api.Pod, conditionType api.PodConditionType) bool {
-	for _, rg := range pod.Spec.ReadinessGates {
-		if rg.ConditionType == conditionType {
-			return true
+	// check if condition already exists
+	now := metav1.Now()
+	i, cond := backendpkg.PodConditionForReadinessGate(pod, conditionType)
+	if cond == nil {
+		// new condition
+		targetHealthCondition := api.PodCondition{
+			Type:    conditionType,
+			Status:  conditionStatus,
+			Reason:  aws.StringValue(targetHealth.Reason),
+			Message: aws.StringValue(targetHealth.Description),
 		}
-	}
-	return false
-}
-
-func podConditionForReadinessGate(pod *api.Pod, conditionType api.PodConditionType) (int, *api.PodCondition) {
-	for i, condition := range pod.Status.Conditions {
-		if condition.Type == conditionType {
-			return i, &condition
+		targetHealthCondition.LastProbeTime = now
+		targetHealthCondition.LastTransitionTime = now
+		pod.Status.Conditions = append(pod.Status.Conditions, targetHealthCondition)
+	} else {
+		cond.LastProbeTime = now
+		if cond.Status != conditionStatus {
+			cond.LastTransitionTime = now
 		}
+		cond.Status = conditionStatus
+		cond.Reason = aws.StringValue(targetHealth.Reason)
+		cond.Message = aws.StringValue(targetHealth.Description)
+		pod.Status.Conditions[i] = *cond
 	}
-	return -1, nil
+
+	return conditionStatus == api.ConditionTrue
 }
 
 func podConditionStatusFromTargetHealth(targetHealth *elbv2.TargetHealth) api.ConditionStatus {