Repository: zalando-incubator/kube-ingress-aws-controller
Pull request #453 (2021-11-18 11:33:33)

adding config option for ALB & NLB healthy threshold count
Healthy threshold: The number of consecutive health checks successes required before considering an unhealthy target healthy

This configuration is required since the default of "5" time the interval, which is minimum of 6s, sums up to 30s that is too long delay in our configurations.

Signed-off-by: Samuel Lang <gh@lang-sam.de>

universam1: @szuecs please review, thanks
dhohengassner: @universam1 nice addition - thanks for adding :+1: 
w-reichert: Good idea, I like that addition, too. 👍
universam1: @szuecs Not sure what to do about the coverage decrease.

Tested this change in our clusters, works as expected
szuecs: @universam1 it's no an issue if coverage drops <1%. It's more a hint we should make it better in the future.
szuecs: :+1: 

Patch:
diff --git a/aws/adapter.go b/aws/adapter.go
index 80de0c1..2cb9b4e 100644
--- a/aws/adapter.go
+++ b/aws/adapter.go
@@ -43,6 +43,9 @@ type Adapter struct {
 	healthCheckPort             uint
 	healthCheckInterval         time.Duration
 	healthCheckTimeout          time.Duration
+	albHealthyThresholdCount    uint
+	albUnhealthyThresholdCount  uint
+	nlbHealthyThresholdCount    uint
 	targetPort                  uint
 	albHTTPTargetPort           uint
 	nlbHTTPTargetPort           uint
@@ -86,16 +89,19 @@ type manifest struct {
 type configProviderFunc func() client.ConfigProvider
 
 const (
-	DefaultHealthCheckPath           = "/kube-system/healthz"
-	DefaultHealthCheckPort           = 9999
-	DefaultTargetPort                = 9999
-	DefaultHealthCheckInterval       = 10 * time.Second
-	DefaultHealthCheckTimeout        = 5 * time.Second
-	DefaultCertificateUpdateInterval = 30 * time.Minute
-	DefaultCreationTimeout           = 5 * time.Minute
-	DefaultIdleConnectionTimeout     = 1 * time.Minute
-	DefaultDeregistrationTimeout     = 5 * time.Minute
-	DefaultControllerID              = "kube-ingress-aws-controller"
+	DefaultHealthCheckPath            = "/kube-system/healthz"
+	DefaultHealthCheckPort            = 9999
+	DefaultTargetPort                 = 9999
+	DefaultHealthCheckInterval        = 10 * time.Second
+	DefaultHealthCheckTimeout         = 5 * time.Second
+	DefaultAlbHealthyThresholdCount   = 5
+	DefaultAlbUnhealthyThresholdCount = 2
+	DefaultNlbHealthyThresholdCount   = 3
+	DefaultCertificateUpdateInterval  = 30 * time.Minute
+	DefaultCreationTimeout            = 5 * time.Minute
+	DefaultIdleConnectionTimeout      = 1 * time.Minute
+	DefaultDeregistrationTimeout      = 5 * time.Minute
+	DefaultControllerID               = "kube-ingress-aws-controller"
 	// DefaultMaxCertsPerALB defines the maximum number of certificates per
 	// ALB. AWS limit is 25 but one space is needed to work around
 	// CloudFormation bug:
@@ -192,30 +198,33 @@ func newConfigProvider(debug, disableInstrumentedHttpClient bool) client.ConfigP
 func NewAdapter(clusterID, newControllerID, vpcID string, debug, disableInstrumentedHttpClient bool) (adapter *Adapter, err error) {
 	p := newConfigProvider(debug, disableInstrumentedHttpClient)
 	adapter = &Adapter{
-		ec2:                 ec2.New(p),
-		elbv2:               elbv2.New(p),
-		ec2metadata:         ec2metadata.New(p),
-		autoscaling:         autoscaling.New(p),
-		acm:                 acm.New(p),
-		iam:                 iam.New(p),
-		cloudformation:      cloudformation.New(p),
-		healthCheckPath:     DefaultHealthCheckPath,
-		healthCheckPort:     DefaultHealthCheckPort,
-		targetPort:          DefaultTargetPort,
-		healthCheckInterval: DefaultHealthCheckInterval,
-		healthCheckTimeout:  DefaultHealthCheckTimeout,
-		creationTimeout:     DefaultCreationTimeout,
-		ec2Details:          make(map[string]*instanceDetails),
-		singleInstances:     make(map[string]*instanceDetails),
-		obsoleteInstances:   make([]string, 0),
-		controllerID:        newControllerID,
-		sslPolicy:           DefaultSslPolicy,
-		ipAddressType:       DefaultIpAddressType,
-		albLogsS3Bucket:     DefaultAlbS3LogsBucket,
-		albLogsS3Prefix:     DefaultAlbS3LogsPrefix,
-		nlbCrossZone:        DefaultNLBCrossZone,
-		nlbHTTPEnabled:      DefaultNLBHTTPEnabled,
-		customFilter:        DefaultCustomFilter,
+		ec2:                        ec2.New(p),
+		elbv2:                      elbv2.New(p),
+		ec2metadata:                ec2metadata.New(p),
+		autoscaling:                autoscaling.New(p),
+		acm:                        acm.New(p),
+		iam:                        iam.New(p),
+		cloudformation:             cloudformation.New(p),
+		healthCheckPath:            DefaultHealthCheckPath,
+		healthCheckPort:            DefaultHealthCheckPort,
+		targetPort:                 DefaultTargetPort,
+		healthCheckInterval:        DefaultHealthCheckInterval,
+		healthCheckTimeout:         DefaultHealthCheckTimeout,
+		albHealthyThresholdCount:   DefaultAlbHealthyThresholdCount,
+		albUnhealthyThresholdCount: DefaultAlbUnhealthyThresholdCount,
+		nlbHealthyThresholdCount:   DefaultNlbHealthyThresholdCount,
+		creationTimeout:            DefaultCreationTimeout,
+		ec2Details:                 make(map[string]*instanceDetails),
+		singleInstances:            make(map[string]*instanceDetails),
+		obsoleteInstances:          make([]string, 0),
+		controllerID:               newControllerID,
+		sslPolicy:                  DefaultSslPolicy,
+		ipAddressType:              DefaultIpAddressType,
+		albLogsS3Bucket:            DefaultAlbS3LogsBucket,
+		albLogsS3Prefix:            DefaultAlbS3LogsPrefix,
+		nlbCrossZone:               DefaultNLBCrossZone,
+		nlbHTTPEnabled:             DefaultNLBHTTPEnabled,
+		customFilter:               DefaultCustomFilter,
 	}
 
 	adapter.manifest, err = buildManifest(adapter, clusterID, vpcID)
@@ -248,6 +257,27 @@ func (a *Adapter) WithHealthCheckPort(port uint) *Adapter {
 	return a
 }
 
+// WithAlbHealthyThresholdCount returns the receiver adapter after changing the healthy threshold count that will be used by
+// the resources created by the adapter
+func (a *Adapter) WithAlbHealthyThresholdCount(count uint) *Adapter {
+	a.albHealthyThresholdCount = count
+	return a
+}
+
+// WithAlbUnhealthyThresholdCount returns the receiver adapter after changing the unhealthy threshold count that will be used by
+// the resources created by the adapter
+func (a *Adapter) WithAlbUnhealthyThresholdCount(count uint) *Adapter {
+	a.albUnhealthyThresholdCount = count
+	return a
+}
+
+// WithNlbHealthyThresholdCount returns the receiver adapter after changing the healthy threshold count that will be used by
+// the resources created by the adapter
+func (a *Adapter) WithNlbHealthyThresholdCount(count uint) *Adapter {
+	a.nlbHealthyThresholdCount = count
+	return a
+}
+
 // WithTargetPort returns the receiver adapter after changing the target port that will be used by
 // the resources created by the adapter
 func (a *Adapter) WithTargetPort(port uint) *Adapter {
@@ -611,6 +641,9 @@ func (a *Adapter) CreateStack(certificateARNs []string, scheme, securityGroup, o
 			interval: a.healthCheckInterval,
 			timeout:  a.healthCheckTimeout,
 		},
+		albHealthyThresholdCount:          a.albHealthyThresholdCount,
+		albUnhealthyThresholdCount:        a.albUnhealthyThresholdCount,
+		nlbHealthyThresholdCount:          a.nlbHealthyThresholdCount,
 		targetPort:                        a.targetPort,
 		targetHTTPS:                       a.targetHTTPS,
 		httpDisabled:                      a.httpDisabled(loadBalancerType),
@@ -663,6 +696,9 @@ func (a *Adapter) UpdateStack(stackName string, certificateARNs map[string]time.
 			interval: a.healthCheckInterval,
 			timeout:  a.healthCheckTimeout,
 		},
+		albHealthyThresholdCount:          a.albHealthyThresholdCount,
+		albUnhealthyThresholdCount:        a.albUnhealthyThresholdCount,
+		nlbHealthyThresholdCount:          a.nlbHealthyThresholdCount,
 		targetPort:                        a.targetPort,
 		targetHTTPS:                       a.targetHTTPS,
 		httpDisabled:                      a.httpDisabled(loadBalancerType),
diff --git a/aws/cf.go b/aws/cf.go
index b27ce1e..058883d 100644
--- a/aws/cf.go
+++ b/aws/cf.go
@@ -137,6 +137,9 @@ type stackSpec struct {
 	clusterID                         string
 	vpcID                             string
 	healthCheck                       *healthCheck
+	albHealthyThresholdCount          uint
+	albUnhealthyThresholdCount        uint
+	nlbHealthyThresholdCount          uint
 	targetPort                        uint
 	targetHTTPS                       bool
 	httpDisabled                      bool
diff --git a/aws/cf_template.go b/aws/cf_template.go
index b1adbf7..c5b0af0 100644
--- a/aws/cf_template.go
+++ b/aws/cf_template.go
@@ -446,9 +446,12 @@ func generateDenyInternalTrafficRule(listenerName string, rulePriority int64, in
 func newTargetGroup(spec *stackSpec, targetPortParameter string) *cloudformation.ElasticLoadBalancingV2TargetGroup {
 	protocol := "HTTP"
 	healthCheckProtocol := "HTTP"
+	healthyThresholdCount, unhealthyThresholdCount := spec.albHealthyThresholdCount, spec.albUnhealthyThresholdCount
 	if spec.loadbalancerType == LoadBalancerTypeNetwork {
 		protocol = "TCP"
 		healthCheckProtocol = "HTTP"
+		// For NLBs the healthy and unhealthy threshold count value must be equal
+		healthyThresholdCount, unhealthyThresholdCount = spec.nlbHealthyThresholdCount, spec.nlbHealthyThresholdCount
 	} else if spec.targetHTTPS {
 		protocol = "HTTPS"
 		healthCheckProtocol = "HTTPS"
@@ -465,6 +468,8 @@ func newTargetGroup(spec *stackSpec, targetPortParameter string) *cloudformation
 		HealthCheckPath:            cloudformation.Ref(parameterTargetGroupHealthCheckPathParameter).String(),
 		HealthCheckPort:            cloudformation.Ref(parameterTargetGroupHealthCheckPortParameter).String(),
 		HealthCheckProtocol:        cloudformation.String(healthCheckProtocol),
+		HealthyThresholdCount:      cloudformation.Integer(int64(healthyThresholdCount)),
+		UnhealthyThresholdCount:    cloudformation.Integer(int64(unhealthyThresholdCount)),
 		Port:                       cloudformation.Ref(targetPortParameter).Integer(),
 		Protocol:                   cloudformation.String(protocol),
 		VPCID:                      cloudformation.Ref(parameterTargetGroupVPCIDParameter).String(),
diff --git a/controller.go b/controller.go
index b50ddda..1235125 100644
--- a/controller.go
+++ b/controller.go
@@ -43,6 +43,9 @@ var (
 	healthCheckPort               uint
 	healthCheckInterval           time.Duration
 	healthCheckTimeout            time.Duration
+	albHealthyThresholdCount      uint
+	albUnhealthyThresholdCount    uint
+	nlbHealthyThresholdCount      uint
 	targetPort                    uint
 	albHTTPTargetPort             uint
 	nlbHTTPTargetPort             uint
@@ -221,6 +224,12 @@ func loadSettings() error {
 		Default(aws.DefaultHealthCheckInterval.String()).DurationVar(&healthCheckInterval)
 	kingpin.Flag("health-check-timeout", "sets the health check timeout for the created target groups. The flag accepts a value acceptable to time.ParseDuration").
 		Default(aws.DefaultHealthCheckTimeout.String()).DurationVar(&healthCheckTimeout)
+	kingpin.Flag("alb-healthy-threshold-count", "The number of consecutive successful health checks required before considering an unhealthy target healthy. The range is 2–10. (ALB only)").
+		Default(strconv.FormatUint(aws.DefaultAlbHealthyThresholdCount, 10)).UintVar(&albHealthyThresholdCount)
+	kingpin.Flag("alb-unhealthy-threshold-count", "The number of consecutive failed health checks required before considering a target unhealthy. The range is 2–10. (ALB only)").
+		Default(strconv.FormatUint(aws.DefaultAlbUnhealthyThresholdCount, 10)).UintVar(&albUnhealthyThresholdCount)
+	kingpin.Flag("nlb-healthy-threshold-count", "The number of consecutive successful or failed health checks required before considering a target healthy or unhealthy. The range is 2–10. (NLB only)").
+		Default(strconv.FormatUint(aws.DefaultNlbHealthyThresholdCount, 10)).UintVar(&nlbHealthyThresholdCount)
 	kingpin.Flag("idle-connection-timeout", "sets the idle connection timeout of all ALBs. The flag accepts a value acceptable to time.ParseDuration and are between 1s and 4000s.").
 		Default(aws.DefaultIdleConnectionTimeout.String()).DurationVar(&idleConnectionTimeout)
 	kingpin.Flag("deregistration-delay-timeout", "sets the deregistration delay timeout of all target groups.  The flag accepts a value acceptable to time.ParseDuration that is between 1s and 3600s.").
@@ -286,6 +295,12 @@ func loadSettings() error {
 		return fmt.Errorf("invalid health check port: %d. please use a valid TCP port", healthCheckPort)
 	}
 
+	for _, v := range []uint{albHealthyThresholdCount, albUnhealthyThresholdCount, nlbHealthyThresholdCount} {
+		if v < 2 || v > 10 {
+			return fmt.Errorf("invalid (un)healthy threshold: %d. must be between 2 and 10", v)
+		}
+	}
+
 	if targetPort == 0 || targetPort > 65535 {
 		return fmt.Errorf("invalid target port: %d. please use a valid TCP port", targetPort)
 	}
@@ -370,6 +385,9 @@ func main() {
 		WithHealthCheckPort(healthCheckPort).
 		WithHealthCheckInterval(healthCheckInterval).
 		WithHealthCheckTimeout(healthCheckTimeout).
+		WithAlbHealthyThresholdCount(albHealthyThresholdCount).
+		WithAlbUnhealthyThresholdCount(albUnhealthyThresholdCount).
+		WithNlbHealthyThresholdCount(nlbHealthyThresholdCount).
 		WithTargetPort(targetPort).
 		WithALBHTTPTargetPort(albHTTPTargetPort).
 		WithNLBHTTPTargetPort(nlbHTTPTargetPort).


Test patch:
diff --git a/aws/adapter_test.go b/aws/adapter_test.go
index db2c47e..eec2b71 100644
--- a/aws/adapter_test.go
+++ b/aws/adapter_test.go
@@ -927,3 +927,23 @@ func TestWithTargetHTTPS(t *testing.T) {
 		require.Equal(t, true, b.targetHTTPS)
 	})
 }
+
+func TestWithxlbHealthyThresholdCount(t *testing.T) {
+	t.Run("WithAlbHealthyThresholdCount sets the albHealthyThresholdCount property", func(t *testing.T) {
+		a := Adapter{}
+		b := a.WithAlbHealthyThresholdCount(2)
+		require.Equal(t, uint(2), b.albHealthyThresholdCount)
+	})
+
+	t.Run("WithAlbUnhealthyThresholdCount sets the albUnhealthyThresholdCount property", func(t *testing.T) {
+		a := Adapter{}
+		b := a.WithAlbUnhealthyThresholdCount(3)
+		require.Equal(t, uint(3), b.albUnhealthyThresholdCount)
+	})
+
+	t.Run("WithNlbHealthyThresholdCount sets the nlbHealthyThresholdCount property", func(t *testing.T) {
+		a := Adapter{}
+		b := a.WithNlbHealthyThresholdCount(4)
+		require.Equal(t, uint(4), b.nlbHealthyThresholdCount)
+	})
+}
diff --git a/aws/cf_template_test.go b/aws/cf_template_test.go
index d0e5181..addf883 100644
--- a/aws/cf_template_test.go
+++ b/aws/cf_template_test.go
@@ -585,6 +585,35 @@ func TestGenerateTemplate(t *testing.T) {
 				validateTargetGroupOutput(t, template, "TG", "TargetGroupARN")
 			},
 		},
+		{
+			name: "For Albs sets Healthy and Unhealthy Threshold Count individually",
+			spec: &stackSpec{
+				loadbalancerType:           LoadBalancerTypeApplication,
+				albHealthyThresholdCount:   7,
+				albUnhealthyThresholdCount: 3,
+			},
+			validate: func(t *testing.T, template *cloudformation.Template) {
+				tg := template.Resources["TG"].Properties.(*cloudformation.ElasticLoadBalancingV2TargetGroup)
+				require.Equal(t, cloudformation.Integer(7), tg.HealthyThresholdCount)
+				require.Equal(t, cloudformation.Integer(3), tg.UnhealthyThresholdCount)
+			},
+		},
+		{
+			name: "For Nlbs sets Healthy and Unhealthy Threshold Count equally and ignores ALB settings",
+			spec: &stackSpec{
+				loadbalancerType:           LoadBalancerTypeNetwork,
+				nlbHealthyThresholdCount:   4,
+				albHealthyThresholdCount:   7,
+				albUnhealthyThresholdCount: 3,
+			},
+			validate: func(t *testing.T, template *cloudformation.Template) {
+				tg := template.Resources["TG"].Properties.(*cloudformation.ElasticLoadBalancingV2TargetGroup)
+				require.Equal(t, cloudformation.Integer(4), tg.HealthyThresholdCount)
+				require.Equal(t, cloudformation.Integer(4), tg.UnhealthyThresholdCount)
+				require.NotEqual(t, cloudformation.Integer(7), tg.HealthyThresholdCount)
+				require.NotEqual(t, cloudformation.Integer(3), tg.UnhealthyThresholdCount)
+			},
+		},
 	} {
 		t.Run(test.name, func(t *testing.T) {
 			generated, err := generateTemplate(test.spec)