Ability to reconfigure NLB target group health check

docs/deploy/configurations.md

@@ -158,4 +158,4 @@ They are a set of kye=value pairs that describe AWS load balance controller feat
 | EnableServiceController               | string                          | true           | Toggles support for `Service` type resources. |
 | EnableIPTargetType                    | string                          | true           | Used to toggle support for target-type `ip` across `Ingress` and `Service` type resources. |
 | SubnetsClusterTagCheck                | string                          | true           | Enable or disable the check for `kubernetes.io/cluster/${cluster-name}` during subnet auto-discovery |
-| NLBHealthCheckTimeout                 | string                          | true           | Enable or disable the use of `service.beta.kubernetes.io/aws-load-balancer-healthcheck-timeout` for `Service` type resources (NLB) |
+| NLBHealthCheckAdvancedConfiguration   | string                          | true           | Enable or disable advanced health check configuration for NLB, for example health check timeout |

docs/guide/service/annotations.md

@@ -39,6 +39,7 @@
 | [service.beta.kubernetes.io/aws-load-balancer-healthcheck-unhealthy-threshold](#healthcheck-unhealthy-threshold) | integer | 3                         |                                                        |
 | [service.beta.kubernetes.io/aws-load-balancer-healthcheck-timeout](#healthcheck-timeout)         | integer                 | 10                        |                                                        |
 | [service.beta.kubernetes.io/aws-load-balancer-healthcheck-interval](#healthcheck-interval)       | integer                 | 10                        |                                                        |
+| [service.beta.kubernetes.io/aws-load-balancer-healthcheck-success-codes](#healthcheck-success-codes)       | string        | 200-399                   |                                                        |
 | [service.beta.kubernetes.io/aws-load-balancer-eip-allocations](#eip-allocations)                 | stringList              |                           | internet-facing lb only. Length must match the number of subnets|
 | [service.beta.kubernetes.io/aws-load-balancer-private-ipv4-addresses](#private-ipv4-addresses)   | stringList              |                           | internal lb only. Length must match the number of subnets |
 | [service.beta.kubernetes.io/aws-load-balancer-ipv6-addresses](#ipv6-addresses)                   | stringList              |                           | dualstack lb only. Length must match the number of subnets |
@@ -341,6 +342,12 @@ Health check on target groups can be configured with following annotations:
         service.beta.kubernetes.io/aws-load-balancer-healthcheck-interval: "10"
         ```
 
+- <a name="healthcheck-success-codes">`service.beta.kubernetes.io/aws-load-balancer-healthcheck-success-codes`</a> specifies the http success codes for the health check in case of http/https protocol.
+
+    !!!example
+        ```
+        service.beta.kubernetes.io/aws-load-balancer-healthcheck-success-codes: "200-399"
+        ```
 
 - <a name="healthcheck-timeout">`service.beta.kubernetes.io/aws-load-balancer-healthcheck-timeout`</a> specifies the target group health check timeout. The target has to respond within the timeout for a successful health check.
 

pkg/annotations/constants.go

@@ -73,6 +73,7 @@ const (
 	SvcLBSuffixHCProtocol                    = "aws-load-balancer-healthcheck-protocol"
 	SvcLBSuffixHCPort                        = "aws-load-balancer-healthcheck-port"
 	SvcLBSuffixHCPath                        = "aws-load-balancer-healthcheck-path"
+	SvcLBSuffixHCSuccessCodes                = "aws-load-balancer-healthcheck-success-codes"
 	SvcLBSuffixTargetGroupAttributes         = "aws-load-balancer-target-group-attributes"
 	SvcLBSuffixSubnets                       = "aws-load-balancer-subnets"
 	SvcLBSuffixEIPAllocations                = "aws-load-balancer-eip-allocations"

pkg/config/feature_gates.go

@@ -11,14 +11,14 @@ import (
 type Feature string
 
 const (
-	ListenerRulesTagging        Feature = "ListenerRulesTagging"
-	WeightedTargetGroups        Feature = "WeightedTargetGroups"
-	ServiceTypeLoadBalancerOnly Feature = "ServiceTypeLoadBalancerOnly"
-	EndpointsFailOpen           Feature = "EndpointsFailOpen"
-	EnableServiceController     Feature = "EnableServiceController"
-	EnableIPTargetType          Feature = "EnableIPTargetType"
-	SubnetsClusterTagCheck      Feature = "SubnetsClusterTagCheck"
-	NLBHealthCheckTimeout       Feature = "NLBHealthCheckTimeout"
+	ListenerRulesTagging         Feature = "ListenerRulesTagging"
+	WeightedTargetGroups         Feature = "WeightedTargetGroups"
+	ServiceTypeLoadBalancerOnly  Feature = "ServiceTypeLoadBalancerOnly"
+	EndpointsFailOpen            Feature = "EndpointsFailOpen"
+	EnableServiceController      Feature = "EnableServiceController"
+	EnableIPTargetType           Feature = "EnableIPTargetType"
+	SubnetsClusterTagCheck       Feature = "SubnetsClusterTagCheck"
+	NLBHealthCheckAdvancedConfig Feature = "NLBHealthCheckAdvancedConfig"
 )
 
 type FeatureGates interface {
@@ -46,14 +46,14 @@ type defaultFeatureGates struct {
 func NewFeatureGates() FeatureGates {
 	return &defaultFeatureGates{
 		featureState: map[Feature]bool{
-			ListenerRulesTagging:        true,
-			WeightedTargetGroups:        true,
-			ServiceTypeLoadBalancerOnly: false,
-			EndpointsFailOpen:           false,
-			EnableServiceController:     true,
-			EnableIPTargetType:          true,
-			SubnetsClusterTagCheck:      true,
-			NLBHealthCheckTimeout:       true,
+			ListenerRulesTagging:         true,
+			WeightedTargetGroups:         true,
+			ServiceTypeLoadBalancerOnly:  false,
+			EndpointsFailOpen:            false,
+			EnableServiceController:      true,
+			EnableIPTargetType:           true,
+			SubnetsClusterTagCheck:       true,
+			NLBHealthCheckAdvancedConfig: true,
 		},
 	}
 }

pkg/deploy/elbv2/target_group_synthesizer.go

@@ -7,19 +7,21 @@ import (
 	"github.com/pkg/errors"
 	"k8s.io/apimachinery/pkg/util/sets"
 	"sigs.k8s.io/aws-load-balancer-controller/pkg/aws/services"
+	"sigs.k8s.io/aws-load-balancer-controller/pkg/config"
 	"sigs.k8s.io/aws-load-balancer-controller/pkg/deploy/tracking"
 	"sigs.k8s.io/aws-load-balancer-controller/pkg/model/core"
 	elbv2model "sigs.k8s.io/aws-load-balancer-controller/pkg/model/elbv2"
 )
 
 // NewTargetGroupSynthesizer constructs targetGroupSynthesizer
 func NewTargetGroupSynthesizer(elbv2Client services.ELBV2, trackingProvider tracking.Provider, taggingManager TaggingManager,
-	tgManager TargetGroupManager, logger logr.Logger, stack core.Stack) *targetGroupSynthesizer {
+	tgManager TargetGroupManager, logger logr.Logger, featureGates config.FeatureGates, stack core.Stack) *targetGroupSynthesizer {
 	return &targetGroupSynthesizer{
 		elbv2Client:      elbv2Client,
 		trackingProvider: trackingProvider,
 		taggingManager:   taggingManager,
 		tgManager:        tgManager,
+		featureGates:     featureGates,
 		logger:           logger,
 		stack:            stack,
 		unmatchedSDKTGs:  nil,
@@ -32,6 +34,7 @@ type targetGroupSynthesizer struct {
 	trackingProvider tracking.Provider
 	taggingManager   TaggingManager
 	tgManager        TargetGroupManager
+	featureGates     config.FeatureGates
 	logger           logr.Logger
 
 	stack           core.Stack
@@ -45,7 +48,8 @@ func (s *targetGroupSynthesizer) Synthesize(ctx context.Context) error {
 	if err != nil {
 		return err
 	}
-	matchedResAndSDKTGs, unmatchedResTGs, unmatchedSDKTGs, err := matchResAndSDKTargetGroups(resTGs, sdkTGs, s.trackingProvider.ResourceIDTagKey())
+	matchedResAndSDKTGs, unmatchedResTGs, unmatchedSDKTGs, err := matchResAndSDKTargetGroups(resTGs, sdkTGs,
+		s.trackingProvider.ResourceIDTagKey(), s.featureGates)
 	if err != nil {
 		return err
 	}
@@ -95,7 +99,7 @@ type resAndSDKTargetGroupPair struct {
 }
 
 func matchResAndSDKTargetGroups(resTGs []*elbv2model.TargetGroup, sdkTGs []TargetGroupWithTags,
-	resourceIDTagKey string) ([]resAndSDKTargetGroupPair, []*elbv2model.TargetGroup, []TargetGroupWithTags, error) {
+	resourceIDTagKey string, featureGates config.FeatureGates) ([]resAndSDKTargetGroupPair, []*elbv2model.TargetGroup, []TargetGroupWithTags, error) {
 	var matchedResAndSDKTGs []resAndSDKTargetGroupPair
 	var unmatchedResTGs []*elbv2model.TargetGroup
 	var unmatchedSDKTGs []TargetGroupWithTags
@@ -113,7 +117,7 @@ func matchResAndSDKTargetGroups(resTGs []*elbv2model.TargetGroup, sdkTGs []Targe
 		sdkTGs := sdkTGsByID[resID]
 		foundMatch := false
 		for _, sdkTG := range sdkTGs {
-			if isSDKTargetGroupRequiresReplacement(sdkTG, resTG) {
+			if isSDKTargetGroupRequiresReplacement(sdkTG, resTG, featureGates) {
 				unmatchedSDKTGs = append(unmatchedSDKTGs, sdkTG)
 				continue
 			}
@@ -158,7 +162,7 @@ func mapSDKTargetGroupByResourceID(sdkTGs []TargetGroupWithTags, resourceIDTagKe
 }
 
 // isSDKTargetGroupRequiresReplacement checks whether a sdk TargetGroup requires replacement to fulfill a TargetGroup resource.
-func isSDKTargetGroupRequiresReplacement(sdkTG TargetGroupWithTags, resTG *elbv2model.TargetGroup) bool {
+func isSDKTargetGroupRequiresReplacement(sdkTG TargetGroupWithTags, resTG *elbv2model.TargetGroup, featureGates config.FeatureGates) bool {
 	if string(resTG.Spec.TargetType) != awssdk.StringValue(sdkTG.TargetGroup.TargetType) {
 		return true
 	}
@@ -171,12 +175,12 @@ func isSDKTargetGroupRequiresReplacement(sdkTG TargetGroupWithTags, resTG *elbv2
 		}
 	}
 
-	return isSDKTargetGroupRequiresReplacementDueToNLBHealthCheck(sdkTG, resTG)
+	return isSDKTargetGroupRequiresReplacementDueToNLBHealthCheck(sdkTG, resTG, featureGates)
 }
 
 // most of the healthCheck settings for NLB targetGroups cannot be changed for now.
-func isSDKTargetGroupRequiresReplacementDueToNLBHealthCheck(sdkTG TargetGroupWithTags, resTG *elbv2model.TargetGroup) bool {
-	if resTG.Spec.HealthCheckConfig == nil {
+func isSDKTargetGroupRequiresReplacementDueToNLBHealthCheck(sdkTG TargetGroupWithTags, resTG *elbv2model.TargetGroup, featureGates config.FeatureGates) bool {
+	if resTG.Spec.HealthCheckConfig == nil || featureGates.Enabled(config.NLBHealthCheckAdvancedConfig) {
 		return false
 	}
 	if resTG.Spec.Protocol != elbv2model.ProtocolTCP && resTG.Spec.Protocol != elbv2model.ProtocolUDP &&

pkg/deploy/elbv2/target_group_synthesizer_test.go

@@ -6,6 +6,7 @@ import (
 	"github.com/pkg/errors"
 	"github.com/stretchr/testify/assert"
 	"k8s.io/apimachinery/pkg/util/intstr"
+	"sigs.k8s.io/aws-load-balancer-controller/pkg/config"
 	coremodel "sigs.k8s.io/aws-load-balancer-controller/pkg/model/core"
 	elbv2model "sigs.k8s.io/aws-load-balancer-controller/pkg/model/elbv2"
 	"testing"
@@ -283,7 +284,8 @@ func Test_matchResAndSDKTargetGroups(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			got, got1, got2, err := matchResAndSDKTargetGroups(tt.args.resTGs, tt.args.sdkTGs, tt.args.resourceIDTagKey)
+			featureGates := config.NewFeatureGates()
+			got, got1, got2, err := matchResAndSDKTargetGroups(tt.args.resTGs, tt.args.sdkTGs, tt.args.resourceIDTagKey, featureGates)
 			if tt.wantErr != nil {
 				assert.EqualError(t, err, tt.wantErr.Error())
 			} else {
@@ -619,7 +621,7 @@ func Test_isSDKTargetGroupRequiresReplacement(t *testing.T) {
 			want: true,
 		},
 		{
-			name: "healthCheck change need replacement",
+			name: "healthCheck change needs no replacement for protocol change",
 			args: args{
 				sdkTG: TargetGroupWithTags{
 					TargetGroup: &elbv2sdk.TargetGroup{
@@ -653,12 +655,13 @@ func Test_isSDKTargetGroupRequiresReplacement(t *testing.T) {
 					},
 				},
 			},
-			want: true,
+			want: false,
 		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			got := isSDKTargetGroupRequiresReplacement(tt.args.sdkTG, tt.args.resTG)
+			featureGates := config.NewFeatureGates()
+			got := isSDKTargetGroupRequiresReplacement(tt.args.sdkTG, tt.args.resTG, featureGates)
 			assert.Equal(t, tt.want, got)
 		})
 	}
@@ -668,8 +671,9 @@ func Test_isSDKTargetGroupRequiresReplacementDueToNLBHealthCheck(t *testing.T) {
 	port8080 := intstr.FromInt(8080)
 	protocolHTTP := elbv2model.ProtocolHTTP
 	type args struct {
-		sdkTG TargetGroupWithTags
-		resTG *elbv2model.TargetGroup
+		sdkTG                               TargetGroupWithTags
+		resTG                               *elbv2model.TargetGroup
+		disableAdvancedNLBHealthCheckConfig bool
 	}
 	tests := []struct {
 		name string
@@ -714,7 +718,7 @@ func Test_isSDKTargetGroupRequiresReplacementDueToNLBHealthCheck(t *testing.T) {
 			want: false,
 		},
 		{
-			name: "NLB TargetGroup healthCheck cannot change protocol",
+			name: "NLB TargetGroup healthCheck cannot change protocol without advanced config",
 			args: args{
 				sdkTG: TargetGroupWithTags{
 					TargetGroup: &elbv2sdk.TargetGroup{
@@ -747,6 +751,7 @@ func Test_isSDKTargetGroupRequiresReplacementDueToNLBHealthCheck(t *testing.T) {
 						},
 					},
 				},
+				disableAdvancedNLBHealthCheckConfig: true,
 			},
 			want: true,
 		},
@@ -784,6 +789,7 @@ func Test_isSDKTargetGroupRequiresReplacementDueToNLBHealthCheck(t *testing.T) {
 						},
 					},
 				},
+				disableAdvancedNLBHealthCheckConfig: true,
 			},
 			want: true,
 		},
@@ -821,6 +827,7 @@ func Test_isSDKTargetGroupRequiresReplacementDueToNLBHealthCheck(t *testing.T) {
 						},
 					},
 				},
+				disableAdvancedNLBHealthCheckConfig: true,
 			},
 			want: true,
 		},
@@ -858,6 +865,7 @@ func Test_isSDKTargetGroupRequiresReplacementDueToNLBHealthCheck(t *testing.T) {
 						},
 					},
 				},
+				disableAdvancedNLBHealthCheckConfig: true,
 			},
 			want: true,
 		},
@@ -975,7 +983,11 @@ func Test_isSDKTargetGroupRequiresReplacementDueToNLBHealthCheck(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			got := isSDKTargetGroupRequiresReplacementDueToNLBHealthCheck(tt.args.sdkTG, tt.args.resTG)
+			featureGates := config.NewFeatureGates()
+			if tt.args.disableAdvancedNLBHealthCheckConfig {
+				featureGates.Disable(config.NLBHealthCheckAdvancedConfig)
+			}
+			got := isSDKTargetGroupRequiresReplacementDueToNLBHealthCheck(tt.args.sdkTG, tt.args.resTG, featureGates)
 			assert.Equal(t, tt.want, got)
 		})
 	}

pkg/deploy/stack_deployer.go

@@ -47,6 +47,7 @@ func NewDefaultStackDeployer(cloud aws.Cloud, k8sClient client.Client,
 		wafv2WebACLAssociationManager:       wafv2.NewDefaultWebACLAssociationManager(cloud.WAFv2(), logger),
 		wafRegionalWebACLAssociationManager: wafregional.NewDefaultWebACLAssociationManager(cloud.WAFRegional(), logger),
 		shieldProtectionManager:             shield.NewDefaultProtectionManager(cloud.Shield(), logger),
+		featureGates:                        config.FeatureGates,
 		vpcID:                               cloud.VpcID(),
 		logger:                              logger,
 	}
@@ -71,6 +72,7 @@ type defaultStackDeployer struct {
 	wafv2WebACLAssociationManager       wafv2.WebACLAssociationManager
 	wafRegionalWebACLAssociationManager wafregional.WebACLAssociationManager
 	shieldProtectionManager             shield.ProtectionManager
+	featureGates                        config.FeatureGates
 	vpcID                               string
 
 	logger logr.Logger
@@ -85,7 +87,7 @@ type ResourceSynthesizer interface {
 func (d *defaultStackDeployer) Deploy(ctx context.Context, stack core.Stack) error {
 	synthesizers := []ResourceSynthesizer{
 		ec2.NewSecurityGroupSynthesizer(d.cloud.EC2(), d.trackingProvider, d.ec2TaggingManager, d.ec2SGManager, d.vpcID, d.logger, stack),
-		elbv2.NewTargetGroupSynthesizer(d.cloud.ELBV2(), d.trackingProvider, d.elbv2TaggingManager, d.elbv2TGManager, d.logger, stack),
+		elbv2.NewTargetGroupSynthesizer(d.cloud.ELBV2(), d.trackingProvider, d.elbv2TaggingManager, d.elbv2TGManager, d.logger, d.featureGates, stack),
 		elbv2.NewLoadBalancerSynthesizer(d.cloud.ELBV2(), d.trackingProvider, d.elbv2TaggingManager, d.elbv2LBManager, d.logger, stack),
 		elbv2.NewListenerSynthesizer(d.cloud.ELBV2(), d.elbv2TaggingManager, d.elbv2LSManager, d.logger, stack),
 		elbv2.NewListenerRuleSynthesizer(d.cloud.ELBV2(), d.elbv2TaggingManager, d.elbv2LRManager, d.logger, stack),