Merge pull request #94546 from kishorj/nlb_custom_healthcheck

AWS NLB/ELB health check config based on service annotations

staging/src/k8s.io/legacy-cloud-providers/aws/aws.go

@@ -187,9 +187,25 @@ const ServiceAnnotationLoadBalancerBEProtocol = "service.beta.kubernetes.io/aws-
 // For example: "Key1=Val1,Key2=Val2,KeyNoVal1=,KeyNoVal2"
 const ServiceAnnotationLoadBalancerAdditionalTags = "service.beta.kubernetes.io/aws-load-balancer-additional-resource-tags"
 
+// ServiceAnnotationLoadBalancerHealthCheckProtocol is the annotation used on the service to
+// specify the protocol used for the ELB health check. Supported values are TCP, HTTP, HTTPS
+// Default is TCP if externalTrafficPolicy is Cluster, HTTP if externalTrafficPolicy is Local
+const ServiceAnnotationLoadBalancerHealthCheckProtocol = "service.beta.kubernetes.io/aws-load-balancer-healthcheck-protocol"
+
+// ServiceAnnotationLoadBalancerHealthCheckPort is the annotation used on the service to
+// specify the port used for ELB health check.
+// Default is traffic-port if externalTrafficPolicy is Cluster, healthCheckNodePort if externalTrafficPolicy is Local
+const ServiceAnnotationLoadBalancerHealthCheckPort = "service.beta.kubernetes.io/aws-load-balancer-healthcheck-port"
+
+// ServiceAnnotationLoadBalancerHealthCheckPath is the annotation used on the service to
+// specify the path for the ELB health check when the health check protocol is HTTP/HTTPS
+// Defaults to /healthz if externalTrafficPolicy is Local, / otherwise
+const ServiceAnnotationLoadBalancerHealthCheckPath = "service.beta.kubernetes.io/aws-load-balancer-healthcheck-path"
+
 // ServiceAnnotationLoadBalancerHCHealthyThreshold is the annotation used on
 // the service to specify the number of successive successful health checks
-// required for a backend to be considered healthy for traffic.
+// required for a backend to be considered healthy for traffic. For NLB, healthy-threshold
+// and unhealthy-threshold must be equal.
 const ServiceAnnotationLoadBalancerHCHealthyThreshold = "service.beta.kubernetes.io/aws-load-balancer-healthcheck-healthy-threshold"
 
 // ServiceAnnotationLoadBalancerHCUnhealthyThreshold is the annotation used
@@ -3686,6 +3702,91 @@ func (c *Cloud) getSubnetCidrs(subnetIDs []string) ([]string, error) {
 	return cidrs, nil
 }
 
+func parseStringAnnotation(annotations map[string]string, annotation string, value *string) bool {
+	if v, ok := annotations[annotation]; ok {
+		*value = v
+		return true
+	}
+	return false
+}
+
+func parseInt64Annotation(annotations map[string]string, annotation string, value *int64) (bool, error) {
+	if v, ok := annotations[annotation]; ok {
+		parsed, err := strconv.ParseInt(v, 10, 0)
+		if err != nil {
+			return true, fmt.Errorf("failed to parse annotation %v=%v", annotation, v)
+		}
+		*value = parsed
+		return true, nil
+	}
+	return false, nil
+}
+
+func (c *Cloud) buildNLBHealthCheckConfiguration(svc *v1.Service) (healthCheckConfig, error) {
+	hc := healthCheckConfig{
+		Port:               defaultHealthCheckPort,
+		Path:               defaultHealthCheckPath,
+		Protocol:           elbv2.ProtocolEnumTcp,
+		Interval:           defaultNlbHealthCheckInterval,
+		Timeout:            defaultNlbHealthCheckTimeout,
+		HealthyThreshold:   defaultNlbHealthCheckThreshold,
+		UnhealthyThreshold: defaultNlbHealthCheckThreshold,
+	}
+	if svc.Spec.ExternalTrafficPolicy == v1.ServiceExternalTrafficPolicyTypeLocal {
+		path, port := servicehelpers.GetServiceHealthCheckPathPort(svc)
+		hc = healthCheckConfig{
+			Port:               strconv.Itoa(int(port)),
+			Path:               path,
+			Protocol:           elbv2.ProtocolEnumHttp,
+			Interval:           10,
+			Timeout:            10,
+			HealthyThreshold:   2,
+			UnhealthyThreshold: 2,
+		}
+	}
+	if parseStringAnnotation(svc.Annotations, ServiceAnnotationLoadBalancerHealthCheckProtocol, &hc.Protocol) {
+		hc.Protocol = strings.ToUpper(hc.Protocol)
+	}
+	switch hc.Protocol {
+	case elbv2.ProtocolEnumHttp, elbv2.ProtocolEnumHttps:
+		parseStringAnnotation(svc.Annotations, ServiceAnnotationLoadBalancerHealthCheckPath, &hc.Path)
+	case elbv2.ProtocolEnumTcp:
+		hc.Path = ""
+	default:
+		return healthCheckConfig{}, fmt.Errorf("Unsupported health check protocol %v", hc.Protocol)
+	}
+
+	parseStringAnnotation(svc.Annotations, ServiceAnnotationLoadBalancerHealthCheckPort, &hc.Port)
+
+	if _, err := parseInt64Annotation(svc.Annotations, ServiceAnnotationLoadBalancerHCInterval, &hc.Interval); err != nil {
+		return healthCheckConfig{}, err
+	}
+	if _, err := parseInt64Annotation(svc.Annotations, ServiceAnnotationLoadBalancerHCTimeout, &hc.Timeout); err != nil {
+		return healthCheckConfig{}, err
+	}
+	if _, err := parseInt64Annotation(svc.Annotations, ServiceAnnotationLoadBalancerHCHealthyThreshold, &hc.HealthyThreshold); err != nil {
+		return healthCheckConfig{}, err
+	}
+	if _, err := parseInt64Annotation(svc.Annotations, ServiceAnnotationLoadBalancerHCUnhealthyThreshold, &hc.UnhealthyThreshold); err != nil {
+		return healthCheckConfig{}, err
+	}
+
+	if hc.HealthyThreshold != hc.UnhealthyThreshold {
+		return healthCheckConfig{}, fmt.Errorf("Health check healthy threshold and unhealthy threshold must be equal")
+	}
+
+	if hc.Interval != 10 && hc.Interval != 30 {
+		return healthCheckConfig{}, fmt.Errorf("Invalid health check interval '%v', must be either 10 or 30", hc.Interval)
+	}
+
+	if hc.Port != defaultHealthCheckPort {
+		if _, err := strconv.ParseInt(hc.Port, 10, 0); err != nil {
+			return healthCheckConfig{}, fmt.Errorf("Invalid health check port '%v'", hc.Port)
+		}
+	}
+	return hc, nil
+}
+
 // EnsureLoadBalancer implements LoadBalancer.EnsureLoadBalancer
 func (c *Cloud) EnsureLoadBalancer(ctx context.Context, clusterName string, apiService *v1.Service, nodes []*v1.Node) (*v1.LoadBalancerStatus, error) {
 	annotations := apiService.Annotations
@@ -3724,11 +3825,10 @@ func (c *Cloud) EnsureLoadBalancer(ctx context.Context, clusterName string, apiS
 				FrontendProtocol: string(port.Protocol),
 				TrafficPort:      int64(port.NodePort),
 				TrafficProtocol:  string(port.Protocol),
-
+			}
-				// if externalTrafficPolicy == "Local", we'll override the
+			var err error
-				// health check later
+			if portMapping.HealthCheckConfig, err = c.buildNLBHealthCheckConfiguration(apiService); err != nil {
-				HealthCheckPort:     int64(port.NodePort),
+				return nil, err
-				HealthCheckProtocol: elbv2.ProtocolEnumTcp,
 			}
 
 			certificateARN := annotations[ServiceAnnotationLoadBalancerCertificate]
@@ -3776,15 +3876,6 @@ func (c *Cloud) EnsureLoadBalancer(ctx context.Context, clusterName string, apiS
 	}
 
 	if isNLB(annotations) {
-
-		if path, healthCheckNodePort := servicehelpers.GetServiceHealthCheckPathPort(apiService); path != "" {
-			for i := range v2Mappings {
-				v2Mappings[i].HealthCheckPort = int64(healthCheckNodePort)
-				v2Mappings[i].HealthCheckPath = path
-				v2Mappings[i].HealthCheckProtocol = elbv2.ProtocolEnumHttp
-			}
-		}
-
 		// Find the subnets that the ELB will live in
 		subnetIDs, err := c.findELBSubnets(internalELB)
 		if err != nil {
@@ -4041,23 +4132,26 @@ func (c *Cloud) EnsureLoadBalancer(ctx context.Context, clusterName string, apiS
 		}
 	}
 
+	// We only configure a TCP health-check on the first port
+	var tcpHealthCheckPort int32
+	for _, listener := range listeners {
+		if listener.InstancePort == nil {
+			continue
+		}
+		tcpHealthCheckPort = int32(*listener.InstancePort)
+		break
+	}
 	if path, healthCheckNodePort := servicehelpers.GetServiceHealthCheckPathPort(apiService); path != "" {
 		klog.V(4).Infof("service %v (%v) needs health checks on :%d%s)", apiService.Name, loadBalancerName, healthCheckNodePort, path)
+		if annotations[ServiceAnnotationLoadBalancerHealthCheckPort] == defaultHealthCheckPort {
+			healthCheckNodePort = tcpHealthCheckPort
+		}
 		err = c.ensureLoadBalancerHealthCheck(loadBalancer, "HTTP", healthCheckNodePort, path, annotations)
 		if err != nil {
 			return nil, fmt.Errorf("Failed to ensure health check for localized service %v on node port %v: %q", loadBalancerName, healthCheckNodePort, err)
 		}
 	} else {
 		klog.V(4).Infof("service %v does not need custom health checks", apiService.Name)
-		// We only configure a TCP health-check on the first port
-		var tcpHealthCheckPort int32
-		for _, listener := range listeners {
-			if listener.InstancePort == nil {
-				continue
-			}
-			tcpHealthCheckPort = int32(*listener.InstancePort)
-			break
-		}
 		annotationProtocol := strings.ToLower(annotations[ServiceAnnotationLoadBalancerBEProtocol])
 		var hcProtocol string
 		if annotationProtocol == "https" || annotationProtocol == "ssl" {

staging/src/k8s.io/legacy-cloud-providers/aws/aws_loadbalancer.go

@@ -56,10 +56,15 @@ const (
 
 var (
 	// Defaults for ELB Healthcheck
-	defaultHCHealthyThreshold   = int64(2)
+	defaultElbHCHealthyThreshold   = int64(2)
-	defaultHCUnhealthyThreshold = int64(6)
+	defaultElbHCUnhealthyThreshold = int64(6)
-	defaultHCTimeout            = int64(5)
+	defaultElbHCTimeout            = int64(5)
-	defaultHCInterval           = int64(10)
+	defaultElbHCInterval           = int64(10)
+	defaultNlbHealthCheckInterval  = int64(30)
+	defaultNlbHealthCheckTimeout   = int64(10)
+	defaultNlbHealthCheckThreshold = int64(3)
+	defaultHealthCheckPort         = "traffic-port"
+	defaultHealthCheckPath         = "/"
 )
 
 func isNLB(annotations map[string]string) bool {
@@ -76,6 +81,16 @@ func isLBExternal(annotations map[string]string) bool {
 	return false
 }
 
+type healthCheckConfig struct {
+	Port               string
+	Path               string
+	Protocol           string
+	Interval           int64
+	Timeout            int64
+	HealthyThreshold   int64
+	UnhealthyThreshold int64
+}
+
 type nlbPortMapping struct {
 	FrontendPort     int64
 	FrontendProtocol string
@@ -83,12 +98,9 @@ type nlbPortMapping struct {
 	TrafficPort     int64
 	TrafficProtocol string
 
-	HealthCheckPort     int64
-	HealthCheckPath     string
-	HealthCheckProtocol string
-
 	SSLCertificateARN string
 	SSLPolicy         string
+	HealthCheckConfig healthCheckConfig
 }
 
 // getKeyValuePropertiesFromAnnotation converts the comma separated list of key-value
@@ -219,7 +231,6 @@ func (c *Cloud) ensureLoadBalancerv2(namespacedName types.NamespacedName, loadBa
 				frontendPort := mapping.FrontendPort
 				frontendProtocol := mapping.FrontendProtocol
 				nodePort := mapping.TrafficPort
-
 				// modifications
 				if listener, ok := actual[frontendPort][frontendProtocol]; ok {
 					listenerNeedsModification := false
@@ -248,10 +259,17 @@ func (c *Cloud) ensureLoadBalancerv2(namespacedName types.NamespacedName, loadBa
 						}
 					}
 
-					// recreate targetGroup if trafficPort or protocol changed
+					// recreate targetGroup if trafficPort, protocol or HealthCheckProtocol changed
+					healthCheckModified := false
 					targetGroupRecreated := false
 					targetGroup, ok := nodePortTargetGroup[nodePort]
-					if !ok || aws.StringValue(targetGroup.Protocol) != mapping.TrafficProtocol {
+
+					if targetGroup != nil && (!strings.EqualFold(mapping.HealthCheckConfig.Protocol, aws.StringValue(targetGroup.HealthCheckProtocol)) ||
+						mapping.HealthCheckConfig.Interval != aws.Int64Value(targetGroup.HealthCheckIntervalSeconds)) {
+						healthCheckModified = true
+					}
+
+					if !ok || aws.StringValue(targetGroup.Protocol) != mapping.TrafficProtocol || healthCheckModified {
 						// create new target group
 						targetGroup, err = c.ensureTargetGroup(
 							nil,
@@ -467,7 +485,7 @@ var invalidELBV2NameRegex = regexp.MustCompile("[^[:alnum:]]")
 // buildTargetGroupName will build unique name for targetGroup of service & port.
 // the name is in format k8s-{namespace:8}-{name:8}-{uuid:10} (chosen to benefit most common use cases).
 // Note: nodePort & targetProtocol & targetType are included since they cannot be modified on existing targetGroup.
-func (c *Cloud) buildTargetGroupName(serviceName types.NamespacedName, servicePort int64, nodePort int64, targetProtocol string, targetType string) string {
+func (c *Cloud) buildTargetGroupName(serviceName types.NamespacedName, servicePort int64, nodePort int64, targetProtocol string, targetType string, mapping nlbPortMapping) string {
 	hasher := sha1.New()
 	_, _ = hasher.Write([]byte(c.tagging.clusterID()))
 	_, _ = hasher.Write([]byte(serviceName.Namespace))
@@ -476,6 +494,8 @@ func (c *Cloud) buildTargetGroupName(serviceName types.NamespacedName, servicePo
 	_, _ = hasher.Write([]byte(strconv.FormatInt(nodePort, 10)))
 	_, _ = hasher.Write([]byte(targetProtocol))
 	_, _ = hasher.Write([]byte(targetType))
+	_, _ = hasher.Write([]byte(mapping.HealthCheckConfig.Protocol))
+	_, _ = hasher.Write([]byte(strconv.FormatInt(mapping.HealthCheckConfig.Interval, 10)))
 	tgUUID := hex.EncodeToString(hasher.Sum(nil))
 
 	sanitizedNamespace := invalidELBV2NameRegex.ReplaceAllString(serviceName.Namespace, "")
@@ -542,7 +562,7 @@ func (c *Cloud) ensureTargetGroup(targetGroup *elbv2.TargetGroup, serviceName ty
 	dirty := false
 	if targetGroup == nil {
 		targetType := "instance"
-		name := c.buildTargetGroupName(serviceName, mapping.FrontendPort, mapping.TrafficPort, mapping.TrafficProtocol, targetType)
+		name := c.buildTargetGroupName(serviceName, mapping.FrontendPort, mapping.TrafficPort, mapping.TrafficProtocol, targetType, mapping)
 		klog.Infof("Creating load balancer target group for %v with name: %s", serviceName, name)
 		input := &elbv2.CreateTargetGroupInput{
 			VpcId:                      aws.String(vpcID),
@@ -550,26 +570,16 @@ func (c *Cloud) ensureTargetGroup(targetGroup *elbv2.TargetGroup, serviceName ty
 			Port:                       aws.Int64(mapping.TrafficPort),
 			Protocol:                   aws.String(mapping.TrafficProtocol),
 			TargetType:                 aws.String(targetType),
-			HealthCheckIntervalSeconds: aws.Int64(30),
+			HealthCheckIntervalSeconds: aws.Int64(mapping.HealthCheckConfig.Interval),
-			HealthCheckPort:            aws.String("traffic-port"),
+			HealthCheckPort:            aws.String(mapping.HealthCheckConfig.Port),
-			HealthCheckProtocol:        aws.String("TCP"),
+			HealthCheckProtocol:        aws.String(mapping.HealthCheckConfig.Protocol),
-			HealthyThresholdCount:      aws.Int64(3),
+			HealthyThresholdCount:      aws.Int64(mapping.HealthCheckConfig.HealthyThreshold),
-			UnhealthyThresholdCount:    aws.Int64(3),
+			UnhealthyThresholdCount:    aws.Int64(mapping.HealthCheckConfig.UnhealthyThreshold),
+			// HealthCheckTimeoutSeconds:  Currently not configurable, 6 seconds for HTTP, 10 for TCP/HTTPS
 		}
 
-		input.HealthCheckProtocol = aws.String(mapping.HealthCheckProtocol)
+		if mapping.HealthCheckConfig.Protocol != elbv2.ProtocolEnumTcp {
-		if mapping.HealthCheckProtocol != elbv2.ProtocolEnumTcp {
+			input.HealthCheckPath = aws.String(mapping.HealthCheckConfig.Path)
-			input.HealthCheckPath = aws.String(mapping.HealthCheckPath)
-		}
-
-		// Account for externalTrafficPolicy = "Local"
-		if mapping.HealthCheckPort != mapping.TrafficPort {
-			input.HealthCheckPort = aws.String(strconv.Itoa(int(mapping.HealthCheckPort)))
-			// Local traffic should have more aggressive health checking by default.
-			// Min allowed by NLB is 10 seconds, and 2 threshold count
-			input.HealthCheckIntervalSeconds = aws.Int64(10)
-			input.HealthyThresholdCount = aws.Int64(2)
-			input.UnhealthyThresholdCount = aws.Int64(2)
 		}
 
 		result, err := c.elbv2.CreateTargetGroup(input)
@@ -686,18 +696,20 @@ func (c *Cloud) ensureTargetGroup(targetGroup *elbv2.TargetGroup, serviceName ty
 		input := &elbv2.ModifyTargetGroupInput{
 			TargetGroupArn: targetGroup.TargetGroupArn,
 		}
-
+		if mapping.HealthCheckConfig.Port != aws.StringValue(targetGroup.HealthCheckPort) {
-		if aws.StringValue(targetGroup.HealthCheckProtocol) != mapping.HealthCheckProtocol {
+			input.HealthCheckPort = aws.String(mapping.HealthCheckConfig.Port)
-			input.HealthCheckProtocol = aws.String(mapping.HealthCheckProtocol)
 			dirtyHealthCheck = true
 		}
-		if aws.StringValue(targetGroup.HealthCheckPort) != strconv.Itoa(int(mapping.HealthCheckPort)) {
+		if mapping.HealthCheckConfig.HealthyThreshold != aws.Int64Value(targetGroup.HealthyThresholdCount) {
-			input.HealthCheckPort = aws.String(strconv.Itoa(int(mapping.HealthCheckPort)))
 			dirtyHealthCheck = true
+			input.HealthyThresholdCount = aws.Int64(mapping.HealthCheckConfig.HealthyThreshold)
+			input.UnhealthyThresholdCount = aws.Int64(mapping.HealthCheckConfig.UnhealthyThreshold)
 		}
-		if mapping.HealthCheckPath != "" && mapping.HealthCheckProtocol != elbv2.ProtocolEnumTcp {
+		if !strings.EqualFold(mapping.HealthCheckConfig.Protocol, elbv2.ProtocolEnumTcp) {
-			input.HealthCheckPath = aws.String(mapping.HealthCheckPath)
+			if mapping.HealthCheckConfig.Path != aws.StringValue(input.HealthCheckPath) {
-			dirtyHealthCheck = true
+				input.HealthCheckPath = aws.String(mapping.HealthCheckConfig.Path)
+				dirtyHealthCheck = true
+			}
 		}
 
 		if dirtyHealthCheck {
@@ -766,7 +778,14 @@ func (c *Cloud) updateInstanceSecurityGroupsForNLB(lbName string, instances map[
 		healthCheckPorts := sets.Int64{}
 		for _, port := range portMappings {
 			clientPorts.Insert(port.TrafficPort)
-			healthCheckPorts.Insert(port.HealthCheckPort)
+			hcPort := port.TrafficPort
+			if port.HealthCheckConfig.Port != defaultHealthCheckPort {
+				var err error
+				if hcPort, err = strconv.ParseInt(port.HealthCheckConfig.Port, 10, 0); err != nil {
+					return fmt.Errorf("Invalid health check port %v", port.HealthCheckConfig.Port)
+				}
+			}
+			healthCheckPorts.Insert(hcPort)
 			if port.TrafficProtocol == string(v1.ProtocolUDP) {
 				clientProtocol = "udp"
 			}
@@ -1284,19 +1303,19 @@ func (c *Cloud) getExpectedHealthCheck(target string, annotations map[string]str
 		return &i64, nil
 	}
 	var err error
-	healthcheck.HealthyThreshold, err = getOrDefault(ServiceAnnotationLoadBalancerHCHealthyThreshold, defaultHCHealthyThreshold)
+	healthcheck.HealthyThreshold, err = getOrDefault(ServiceAnnotationLoadBalancerHCHealthyThreshold, defaultElbHCHealthyThreshold)
 	if err != nil {
 		return nil, err
 	}
-	healthcheck.UnhealthyThreshold, err = getOrDefault(ServiceAnnotationLoadBalancerHCUnhealthyThreshold, defaultHCUnhealthyThreshold)
+	healthcheck.UnhealthyThreshold, err = getOrDefault(ServiceAnnotationLoadBalancerHCUnhealthyThreshold, defaultElbHCUnhealthyThreshold)
 	if err != nil {
 		return nil, err
 	}
-	healthcheck.Timeout, err = getOrDefault(ServiceAnnotationLoadBalancerHCTimeout, defaultHCTimeout)
+	healthcheck.Timeout, err = getOrDefault(ServiceAnnotationLoadBalancerHCTimeout, defaultElbHCTimeout)
 	if err != nil {
 		return nil, err
 	}
-	healthcheck.Interval, err = getOrDefault(ServiceAnnotationLoadBalancerHCInterval, defaultHCInterval)
+	healthcheck.Interval, err = getOrDefault(ServiceAnnotationLoadBalancerHCInterval, defaultElbHCInterval)
 	if err != nil {
 		return nil, err
 	}
@@ -1311,6 +1330,28 @@ func (c *Cloud) ensureLoadBalancerHealthCheck(loadBalancer *elb.LoadBalancerDesc
 	name := aws.StringValue(loadBalancer.LoadBalancerName)
 
 	actual := loadBalancer.HealthCheck
+	// Override healthcheck protocol, port and path based on annotations
+	if s, ok := annotations[ServiceAnnotationLoadBalancerHealthCheckProtocol]; ok {
+		protocol = s
+	}
+	if s, ok := annotations[ServiceAnnotationLoadBalancerHealthCheckPort]; ok && s != defaultHealthCheckPort {
+		p, err := strconv.ParseInt(s, 10, 0)
+		if err != nil {
+			return err
+		}
+		port = int32(p)
+	}
+	switch strings.ToUpper(protocol) {
+	case "HTTP", "HTTPS":
+		if path == "" {
+			path = defaultHealthCheckPath
+		}
+		if s := annotations[ServiceAnnotationLoadBalancerHealthCheckPath]; s != "" {
+			path = s
+		}
+	default:
+		path = ""
+	}
 	expectedTarget := protocol + ":" + strconv.FormatInt(int64(port), 10) + path
 	expected, err := c.getExpectedHealthCheck(expectedTarget, annotations)
 	if err != nil {

staging/src/k8s.io/legacy-cloud-providers/aws/aws_loadbalancer_test.go

@@ -344,6 +344,7 @@ func TestBuildTargetGroupName(t *testing.T) {
 		nodePort       int64
 		targetProtocol string
 		targetType     string
+		nlbConfig      nlbPortMapping
 	}
 	tests := []struct {
 		name      string
@@ -360,8 +361,9 @@ func TestBuildTargetGroupName(t *testing.T) {
 				nodePort:       8080,
 				targetProtocol: "TCP",
 				targetType:     "instance",
+				nlbConfig:      nlbPortMapping{},
 			},
-			want: "k8s-default-servicea-0aeb5b75af",
+			want: "k8s-default-servicea-7fa2e07508",
 		},
 		{
 			name:      "base case & clusterID changed",
@@ -372,8 +374,9 @@ func TestBuildTargetGroupName(t *testing.T) {
 				nodePort:       8080,
 				targetProtocol: "TCP",
 				targetType:     "instance",
+				nlbConfig:      nlbPortMapping{},
 			},
-			want: "k8s-default-servicea-5d3a0a69a8",
+			want: "k8s-default-servicea-719ee635da",
 		},
 		{
 			name:      "base case & serviceNamespace changed",
@@ -384,8 +387,9 @@ func TestBuildTargetGroupName(t *testing.T) {
 				nodePort:       8080,
 				targetProtocol: "TCP",
 				targetType:     "instance",
+				nlbConfig:      nlbPortMapping{},
 			},
-			want: "k8s-another-servicea-f3a3263315",
+			want: "k8s-another-servicea-f66e09847d",
 		},
 		{
 			name:      "base case & serviceName changed",
@@ -396,8 +400,9 @@ func TestBuildTargetGroupName(t *testing.T) {
 				nodePort:       8080,
 				targetProtocol: "TCP",
 				targetType:     "instance",
+				nlbConfig:      nlbPortMapping{},
 			},
-			want: "k8s-default-serviceb-9a3c03b25e",
+			want: "k8s-default-serviceb-196c19c881",
 		},
 		{
 			name:      "base case & servicePort changed",
@@ -408,8 +413,9 @@ func TestBuildTargetGroupName(t *testing.T) {
 				nodePort:       8080,
 				targetProtocol: "TCP",
 				targetType:     "instance",
+				nlbConfig:      nlbPortMapping{},
 			},
-			want: "k8s-default-servicea-6e07474ff4",
+			want: "k8s-default-servicea-06876706cb",
 		},
 		{
 			name:      "base case & nodePort changed",
@@ -420,8 +426,9 @@ func TestBuildTargetGroupName(t *testing.T) {
 				nodePort:       9090,
 				targetProtocol: "TCP",
 				targetType:     "instance",
+				nlbConfig:      nlbPortMapping{},
 			},
-			want: "k8s-default-servicea-6cb2d0201c",
+			want: "k8s-default-servicea-119f844ec0",
 		},
 		{
 			name:      "base case & targetProtocol changed",
@@ -432,8 +439,9 @@ func TestBuildTargetGroupName(t *testing.T) {
 				nodePort:       8080,
 				targetProtocol: "UDP",
 				targetType:     "instance",
+				nlbConfig:      nlbPortMapping{},
 			},
-			want: "k8s-default-servicea-70495e628e",
+			want: "k8s-default-servicea-3868761686",
 		},
 		{
 			name:      "base case & targetType changed",
@@ -444,8 +452,27 @@ func TestBuildTargetGroupName(t *testing.T) {
 				nodePort:       8080,
 				targetProtocol: "TCP",
 				targetType:     "ip",
+				nlbConfig:      nlbPortMapping{},
 			},
-			want: "k8s-default-servicea-fff6dd8028",
+			want: "k8s-default-servicea-0fa31f4b0f",
+		},
+		{
+			name:      "custom healthcheck config",
+			clusterID: "cluster-a",
+			args: args{
+				serviceName:    types.NamespacedName{Namespace: "default", Name: "service-a"},
+				servicePort:    80,
+				nodePort:       8080,
+				targetProtocol: "TCP",
+				targetType:     "ip",
+				nlbConfig: nlbPortMapping{
+					HealthCheckConfig: healthCheckConfig{
+						Protocol: "HTTP",
+						Interval: 10,
+					},
+				},
+			},
+			want: "k8s-default-servicea-4028e49618",
 		},
 	}
 	for _, tt := range tests {
@@ -453,7 +480,7 @@ func TestBuildTargetGroupName(t *testing.T) {
 			c := &Cloud{
 				tagging: awsTagging{ClusterID: tt.clusterID},
 			}
-			if got := c.buildTargetGroupName(tt.args.serviceName, tt.args.servicePort, tt.args.nodePort, tt.args.targetProtocol, tt.args.targetType); got != tt.want {
+			if got := c.buildTargetGroupName(tt.args.serviceName, tt.args.servicePort, tt.args.nodePort, tt.args.targetProtocol, tt.args.targetType, tt.args.nlbConfig); got != tt.want {
 				assert.Equal(t, tt.want, got)
 			}
 		})

staging/src/k8s.io/legacy-cloud-providers/aws/aws_test.go

@@ -1721,16 +1721,135 @@ func TestAddLoadBalancerTags(t *testing.T) {
 
 func TestEnsureLoadBalancerHealthCheck(t *testing.T) {
 	tests := []struct {
-		name                string
+		name        string
-		annotations         map[string]string
+		annotations map[string]string
-		overriddenFieldName string
+		want        elb.HealthCheck
-		overriddenValue     int64
 	}{
-		{"falls back to HC defaults", map[string]string{}, "", int64(0)},
+		{
-		{"healthy threshold override", map[string]string{ServiceAnnotationLoadBalancerHCHealthyThreshold: "7"}, "HealthyThreshold", int64(7)},
+			name:        "falls back to HC defaults",
-		{"unhealthy threshold override", map[string]string{ServiceAnnotationLoadBalancerHCUnhealthyThreshold: "7"}, "UnhealthyThreshold", int64(7)},
+			annotations: map[string]string{},
-		{"timeout override", map[string]string{ServiceAnnotationLoadBalancerHCTimeout: "7"}, "Timeout", int64(7)},
+			want: elb.HealthCheck{
-		{"interval override", map[string]string{ServiceAnnotationLoadBalancerHCInterval: "7"}, "Interval", int64(7)},
+				HealthyThreshold:   aws.Int64(2),
+				UnhealthyThreshold: aws.Int64(6),
+				Timeout:            aws.Int64(5),
+				Interval:           aws.Int64(10),
+				Target:             aws.String("TCP:8080"),
+			},
+		},
+		{
+			name:        "healthy threshold override",
+			annotations: map[string]string{ServiceAnnotationLoadBalancerHCHealthyThreshold: "7"},
+			want: elb.HealthCheck{
+				HealthyThreshold:   aws.Int64(7),
+				UnhealthyThreshold: aws.Int64(6),
+				Timeout:            aws.Int64(5),
+				Interval:           aws.Int64(10),
+				Target:             aws.String("TCP:8080"),
+			},
+		},
+		{
+			name:        "unhealthy threshold override",
+			annotations: map[string]string{ServiceAnnotationLoadBalancerHCUnhealthyThreshold: "7"},
+			want: elb.HealthCheck{
+				HealthyThreshold:   aws.Int64(2),
+				UnhealthyThreshold: aws.Int64(7),
+				Timeout:            aws.Int64(5),
+				Interval:           aws.Int64(10),
+				Target:             aws.String("TCP:8080"),
+			},
+		},
+		{
+			name:        "timeout override",
+			annotations: map[string]string{ServiceAnnotationLoadBalancerHCTimeout: "7"},
+			want: elb.HealthCheck{
+				HealthyThreshold:   aws.Int64(2),
+				UnhealthyThreshold: aws.Int64(6),
+				Timeout:            aws.Int64(7),
+				Interval:           aws.Int64(10),
+				Target:             aws.String("TCP:8080"),
+			},
+		},
+		{
+			name:        "interval override",
+			annotations: map[string]string{ServiceAnnotationLoadBalancerHCInterval: "7"},
+			want: elb.HealthCheck{
+				HealthyThreshold:   aws.Int64(2),
+				UnhealthyThreshold: aws.Int64(6),
+				Timeout:            aws.Int64(5),
+				Interval:           aws.Int64(7),
+				Target:             aws.String("TCP:8080"),
+			},
+		},
+		{
+			name: "healthcheck port override",
+			annotations: map[string]string{
+				ServiceAnnotationLoadBalancerHealthCheckPort: "2122",
+			},
+			want: elb.HealthCheck{
+				HealthyThreshold:   aws.Int64(2),
+				UnhealthyThreshold: aws.Int64(6),
+				Timeout:            aws.Int64(5),
+				Interval:           aws.Int64(10),
+				Target:             aws.String("TCP:2122"),
+			},
+		},
+		{
+			name: "healthcheck protocol override",
+			annotations: map[string]string{
+				ServiceAnnotationLoadBalancerHealthCheckProtocol: "HTTP",
+			},
+			want: elb.HealthCheck{
+				HealthyThreshold:   aws.Int64(2),
+				UnhealthyThreshold: aws.Int64(6),
+				Timeout:            aws.Int64(5),
+				Interval:           aws.Int64(10),
+				Target:             aws.String("HTTP:8080/"),
+			},
+		},
+		{
+			name: "healthcheck protocol, port, path override",
+			annotations: map[string]string{
+				ServiceAnnotationLoadBalancerHealthCheckProtocol: "HTTPS",
+				ServiceAnnotationLoadBalancerHealthCheckPath:     "/healthz",
+				ServiceAnnotationLoadBalancerHealthCheckPort:     "31224",
+			},
+			want: elb.HealthCheck{
+				HealthyThreshold:   aws.Int64(2),
+				UnhealthyThreshold: aws.Int64(6),
+				Timeout:            aws.Int64(5),
+				Interval:           aws.Int64(10),
+				Target:             aws.String("HTTPS:31224/healthz"),
+			},
+		},
+		{
+			name: "healthcheck protocol SSL",
+			annotations: map[string]string{
+				ServiceAnnotationLoadBalancerHealthCheckProtocol: "SSL",
+				ServiceAnnotationLoadBalancerHealthCheckPath:     "/healthz",
+				ServiceAnnotationLoadBalancerHealthCheckPort:     "3124",
+			},
+			want: elb.HealthCheck{
+				HealthyThreshold:   aws.Int64(2),
+				UnhealthyThreshold: aws.Int64(6),
+				Timeout:            aws.Int64(5),
+				Interval:           aws.Int64(10),
+				Target:             aws.String("SSL:3124"),
+			},
+		},
+		{
+			name: "healthcheck port annotation traffic-port",
+			annotations: map[string]string{
+				ServiceAnnotationLoadBalancerHealthCheckProtocol: "TCP",
+				ServiceAnnotationLoadBalancerHealthCheckPort:     "traffic-port",
+			},
+			want: elb.HealthCheck{
+				HealthyThreshold:   aws.Int64(2),
+				UnhealthyThreshold: aws.Int64(6),
+				Timeout:            aws.Int64(5),
+				Interval:           aws.Int64(10),
+				Target:             aws.String("TCP:8080"),
+			},
+		},
 	}
 	lbName := "myLB"
 	// this HC will always differ from the expected HC and thus it is expected an
@@ -1741,8 +1860,8 @@ func TestEnsureLoadBalancerHealthCheck(t *testing.T) {
 	defaultUnhealthyThreshold := int64(6)
 	defaultTimeout := int64(5)
 	defaultInterval := int64(10)
-	protocol, path, port := "tcp", "", int32(8080)
+	protocol, path, port := "TCP", "", int32(8080)
-	target := "tcp:8080"
+	target := "TCP:8080"
 	defaultHC := &elb.HealthCheck{
 		HealthyThreshold:   &defaultHealthyThreshold,
 		UnhealthyThreshold: &defaultUnhealthyThreshold,
@@ -1755,11 +1874,7 @@ func TestEnsureLoadBalancerHealthCheck(t *testing.T) {
 			awsServices := newMockedFakeAWSServices(TestClusterID)
 			c, err := newAWSCloud(CloudConfig{}, awsServices)
 			assert.Nil(t, err, "Error building aws cloud: %v", err)
-			expectedHC := *defaultHC
+			expectedHC := test.want
-			if test.overriddenFieldName != "" { // cater for test case with no overrides
-				value := reflect.ValueOf(&test.overriddenValue)
-				reflect.ValueOf(&expectedHC).Elem().FieldByName(test.overriddenFieldName).Set(value)
-			}
 			awsServices.elb.(*MockedFakeELB).expectConfigureHealthCheck(&lbName, &expectedHC, nil)
 
 			err = c.ensureLoadBalancerHealthCheck(elbDesc, protocol, port, path, test.annotations)
@@ -2104,10 +2219,17 @@ func (m *MockedFakeELBV2) CreateTargetGroup(request *elbv2.CreateTargetGroupInpu
 		rand.Uint32())
 
 	newTG := &elbv2.TargetGroup{
-		TargetGroupArn:  aws.String(arn),
+		TargetGroupArn:             aws.String(arn),
-		TargetGroupName: request.Name,
+		TargetGroupName:            request.Name,
-		Port:            request.Port,
+		Port:                       request.Port,
-		Protocol:        request.Protocol,
+		Protocol:                   request.Protocol,
+		HealthCheckProtocol:        request.HealthCheckProtocol,
+		HealthCheckPath:            request.HealthCheckPath,
+		HealthCheckPort:            request.HealthCheckPort,
+		HealthCheckTimeoutSeconds:  request.HealthCheckTimeoutSeconds,
+		HealthCheckIntervalSeconds: request.HealthCheckIntervalSeconds,
+		HealthyThresholdCount:      request.HealthyThresholdCount,
+		UnhealthyThresholdCount:    request.UnhealthyThresholdCount,
 	}
 
 	m.TargetGroups = append(m.TargetGroups, newTG)
@@ -2528,6 +2650,15 @@ func TestNLBNodeRegistration(t *testing.T) {
 			t.Errorf("Expected 3 nodes registered with target group, saw %d", len(instances))
 		}
 	}
+
+	fauxService.Annotations[ServiceAnnotationLoadBalancerHealthCheckProtocol] = "http"
+	tgARN := aws.StringValue(awsServices.elbv2.(*MockedFakeELBV2).Listeners[0].DefaultActions[0].TargetGroupArn)
+	_, err = c.EnsureLoadBalancer(context.TODO(), TestClusterName, fauxService, nodes)
+	if err != nil {
+		t.Errorf("EnsureLoadBalancer returned an error: %v", err)
+	}
+	assert.Equal(t, 1, len(awsServices.elbv2.(*MockedFakeELBV2).Listeners))
+	assert.NotEqual(t, tgARN, aws.StringValue(awsServices.elbv2.(*MockedFakeELBV2).Listeners[0].DefaultActions[0].TargetGroupArn))
 }
 
 func makeNamedNode(s *FakeAWSServices, offset int, name string) *v1.Node {
@@ -2643,3 +2774,279 @@ func TestCloud_sortELBSecurityGroupList(t *testing.T) {
 		})
 	}
 }
+
+func TestCloud_buildNLBHealthCheckConfiguration(t *testing.T) {
+	tests := []struct {
+		name        string
+		annotations map[string]string
+		service     *v1.Service
+		want        healthCheckConfig
+		wantError   bool
+	}{
+		{
+			name:        "default cluster",
+			annotations: map[string]string{},
+			service: &v1.Service{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "test-svc",
+					UID:  "UID",
+				},
+				Spec: v1.ServiceSpec{
+					Ports: []v1.ServicePort{
+						{
+							Name:       "http",
+							Protocol:   v1.ProtocolTCP,
+							Port:       8080,
+							TargetPort: intstr.FromInt(8880),
+							NodePort:   32205,
+						},
+					},
+				},
+			},
+			want: healthCheckConfig{
+				Port:               "traffic-port",
+				Protocol:           elbv2.ProtocolEnumTcp,
+				Interval:           30,
+				Timeout:            10,
+				HealthyThreshold:   3,
+				UnhealthyThreshold: 3,
+			},
+			wantError: false,
+		},
+		{
+			name:        "default local",
+			annotations: map[string]string{},
+			service: &v1.Service{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "test-svc",
+					UID:  "UID",
+				},
+				Spec: v1.ServiceSpec{
+					Type: v1.ServiceTypeLoadBalancer,
+					Ports: []v1.ServicePort{
+						{
+							Name:       "http",
+							Protocol:   v1.ProtocolTCP,
+							Port:       8080,
+							TargetPort: intstr.FromInt(8880),
+							NodePort:   32205,
+						},
+					},
+					ExternalTrafficPolicy: v1.ServiceExternalTrafficPolicyTypeLocal,
+					HealthCheckNodePort:   32213,
+				},
+			},
+			want: healthCheckConfig{
+				Port:               "32213",
+				Path:               "/healthz",
+				Protocol:           elbv2.ProtocolEnumHttp,
+				Interval:           10,
+				Timeout:            10,
+				HealthyThreshold:   2,
+				UnhealthyThreshold: 2,
+			},
+			wantError: false,
+		},
+		{
+			name: "with TCP healthcheck",
+			service: &v1.Service{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "test-svc",
+					UID:  "UID",
+					Annotations: map[string]string{
+						ServiceAnnotationLoadBalancerHealthCheckProtocol:  "TCP",
+						ServiceAnnotationLoadBalancerHealthCheckPort:      "8001",
+						ServiceAnnotationLoadBalancerHealthCheckPath:      "/healthz",
+						ServiceAnnotationLoadBalancerHCHealthyThreshold:   "4",
+						ServiceAnnotationLoadBalancerHCUnhealthyThreshold: "4",
+						ServiceAnnotationLoadBalancerHCInterval:           "10",
+						ServiceAnnotationLoadBalancerHCTimeout:            "5",
+					},
+				},
+				Spec: v1.ServiceSpec{
+					Type: v1.ServiceTypeLoadBalancer,
+					Ports: []v1.ServicePort{
+						{
+							Name:       "http",
+							Protocol:   v1.ProtocolTCP,
+							Port:       8080,
+							TargetPort: intstr.FromInt(8880),
+							NodePort:   32205,
+						},
+					},
+					ExternalTrafficPolicy: v1.ServiceExternalTrafficPolicyTypeLocal,
+					HealthCheckNodePort:   32213,
+				},
+			},
+			want: healthCheckConfig{
+				Interval:           10,
+				Timeout:            5,
+				Protocol:           "TCP",
+				Port:               "8001",
+				HealthyThreshold:   4,
+				UnhealthyThreshold: 4,
+			},
+			wantError: false,
+		},
+		{
+			name: "with HTTP healthcheck",
+			service: &v1.Service{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "test-svc",
+					UID:  "UID",
+					Annotations: map[string]string{
+						ServiceAnnotationLoadBalancerHealthCheckProtocol:  "HTTP",
+						ServiceAnnotationLoadBalancerHealthCheckPort:      "41233",
+						ServiceAnnotationLoadBalancerHealthCheckPath:      "/healthz",
+						ServiceAnnotationLoadBalancerHCHealthyThreshold:   "5",
+						ServiceAnnotationLoadBalancerHCUnhealthyThreshold: "5",
+						ServiceAnnotationLoadBalancerHCInterval:           "30",
+						ServiceAnnotationLoadBalancerHCTimeout:            "6",
+					},
+				},
+				Spec: v1.ServiceSpec{
+					Ports: []v1.ServicePort{
+						{
+							Name:       "http",
+							Protocol:   v1.ProtocolTCP,
+							Port:       8080,
+							TargetPort: intstr.FromInt(8880),
+							NodePort:   32205,
+						},
+					},
+				},
+			},
+			want: healthCheckConfig{
+				Interval:           30,
+				Timeout:            6,
+				Protocol:           "HTTP",
+				Port:               "41233",
+				Path:               "/healthz",
+				HealthyThreshold:   5,
+				UnhealthyThreshold: 5,
+			},
+			wantError: false,
+		},
+		{
+			name: "HTTP healthcheck default path",
+			service: &v1.Service{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "test-svc",
+					UID:  "UID",
+					Annotations: map[string]string{
+						ServiceAnnotationLoadBalancerHealthCheckProtocol: "Http",
+					},
+				},
+				Spec: v1.ServiceSpec{
+					Ports: []v1.ServicePort{
+						{
+							Name:       "http",
+							Protocol:   v1.ProtocolTCP,
+							Port:       8080,
+							TargetPort: intstr.FromInt(8880),
+							NodePort:   32205,
+						},
+					},
+				},
+			},
+			want: healthCheckConfig{
+				Interval:           30,
+				Timeout:            10,
+				Protocol:           "HTTP",
+				Path:               "/",
+				Port:               "traffic-port",
+				HealthyThreshold:   3,
+				UnhealthyThreshold: 3,
+			},
+			wantError: false,
+		},
+		{
+			name: "invalid interval",
+			service: &v1.Service{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "test-svc",
+					UID:  "UID",
+					Annotations: map[string]string{
+						ServiceAnnotationLoadBalancerHCInterval: "23",
+					},
+				},
+				Spec: v1.ServiceSpec{
+					Ports: []v1.ServicePort{
+						{
+							Name:       "http",
+							Protocol:   v1.ProtocolTCP,
+							Port:       8080,
+							TargetPort: intstr.FromInt(8880),
+							NodePort:   32205,
+						},
+					},
+				},
+			},
+			want:      healthCheckConfig{},
+			wantError: true,
+		},
+		{
+			name: "invalid timeout",
+			service: &v1.Service{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "test-svc",
+					UID:  "UID",
+					Annotations: map[string]string{
+						ServiceAnnotationLoadBalancerHCTimeout: "non-numeric",
+					},
+				},
+				Spec: v1.ServiceSpec{
+					Ports: []v1.ServicePort{
+						{
+							Name:       "http",
+							Protocol:   v1.ProtocolTCP,
+							Port:       8080,
+							TargetPort: intstr.FromInt(8880),
+							NodePort:   32205,
+						},
+					},
+				},
+			},
+			want:      healthCheckConfig{},
+			wantError: true,
+		},
+		{
+			name: "mismatch healthy and unhealthy targets",
+			service: &v1.Service{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "test-svc",
+					UID:  "UID",
+					Annotations: map[string]string{
+						ServiceAnnotationLoadBalancerHCHealthyThreshold:   "7",
+						ServiceAnnotationLoadBalancerHCUnhealthyThreshold: "5",
+					},
+				},
+				Spec: v1.ServiceSpec{
+					Ports: []v1.ServicePort{
+						{
+							Name:       "http",
+							Protocol:   v1.ProtocolTCP,
+							Port:       8080,
+							TargetPort: intstr.FromInt(8880),
+							NodePort:   32205,
+						},
+					},
+				},
+			},
+			want:      healthCheckConfig{},
+			wantError: true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			c := &Cloud{}
+			hc, err := c.buildNLBHealthCheckConfiguration(tt.service)
+			if !tt.wantError {
+				assert.Equal(t, tt.want, hc)
+			} else {
+				assert.NotNil(t, err)
+			}
+		})
+	}
+}