diff --git a/staging/src/k8s.io/legacy-cloud-providers/aws/aws.go b/staging/src/k8s.io/legacy-cloud-providers/aws/aws.go index 06e3e8efe64..e48de1aab08 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/aws/aws.go +++ b/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 - // health check later - HealthCheckPort: int64(port.NodePort), - HealthCheckProtocol: elbv2.ProtocolEnumTcp, + } + var err error + if portMapping.HealthCheckConfig, err = c.buildNLBHealthCheckConfiguration(apiService); err != nil { + return nil, err } 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" { diff --git a/staging/src/k8s.io/legacy-cloud-providers/aws/aws_loadbalancer.go b/staging/src/k8s.io/legacy-cloud-providers/aws/aws_loadbalancer.go index 8c45fe6a521..fb7523c960b 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/aws/aws_loadbalancer.go +++ b/staging/src/k8s.io/legacy-cloud-providers/aws/aws_loadbalancer.go @@ -56,10 +56,15 @@ const ( var ( // Defaults for ELB Healthcheck - defaultHCHealthyThreshold = int64(2) - defaultHCUnhealthyThreshold = int64(6) - defaultHCTimeout = int64(5) - defaultHCInterval = int64(10) + defaultElbHCHealthyThreshold = int64(2) + defaultElbHCUnhealthyThreshold = int64(6) + defaultElbHCTimeout = int64(5) + 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), - HealthCheckPort: aws.String("traffic-port"), - HealthCheckProtocol: aws.String("TCP"), - HealthyThresholdCount: aws.Int64(3), - UnhealthyThresholdCount: aws.Int64(3), + HealthCheckIntervalSeconds: aws.Int64(mapping.HealthCheckConfig.Interval), + HealthCheckPort: aws.String(mapping.HealthCheckConfig.Port), + HealthCheckProtocol: aws.String(mapping.HealthCheckConfig.Protocol), + HealthyThresholdCount: aws.Int64(mapping.HealthCheckConfig.HealthyThreshold), + 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.HealthCheckProtocol != elbv2.ProtocolEnumTcp { - 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) + if mapping.HealthCheckConfig.Protocol != elbv2.ProtocolEnumTcp { + input.HealthCheckPath = aws.String(mapping.HealthCheckConfig.Path) } 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 aws.StringValue(targetGroup.HealthCheckProtocol) != mapping.HealthCheckProtocol { - input.HealthCheckProtocol = aws.String(mapping.HealthCheckProtocol) + if mapping.HealthCheckConfig.Port != aws.StringValue(targetGroup.HealthCheckPort) { + input.HealthCheckPort = aws.String(mapping.HealthCheckConfig.Port) dirtyHealthCheck = true } - if aws.StringValue(targetGroup.HealthCheckPort) != strconv.Itoa(int(mapping.HealthCheckPort)) { - input.HealthCheckPort = aws.String(strconv.Itoa(int(mapping.HealthCheckPort))) + if mapping.HealthCheckConfig.HealthyThreshold != aws.Int64Value(targetGroup.HealthyThresholdCount) { dirtyHealthCheck = true + input.HealthyThresholdCount = aws.Int64(mapping.HealthCheckConfig.HealthyThreshold) + input.UnhealthyThresholdCount = aws.Int64(mapping.HealthCheckConfig.UnhealthyThreshold) } - if mapping.HealthCheckPath != "" && mapping.HealthCheckProtocol != elbv2.ProtocolEnumTcp { - input.HealthCheckPath = aws.String(mapping.HealthCheckPath) - dirtyHealthCheck = true + if !strings.EqualFold(mapping.HealthCheckConfig.Protocol, elbv2.ProtocolEnumTcp) { + if mapping.HealthCheckConfig.Path != aws.StringValue(input.HealthCheckPath) { + 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 { diff --git a/staging/src/k8s.io/legacy-cloud-providers/aws/aws_loadbalancer_test.go b/staging/src/k8s.io/legacy-cloud-providers/aws/aws_loadbalancer_test.go index df63d9b21f0..5ec1d1b3219 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/aws/aws_loadbalancer_test.go +++ b/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) } }) diff --git a/staging/src/k8s.io/legacy-cloud-providers/aws/aws_test.go b/staging/src/k8s.io/legacy-cloud-providers/aws/aws_test.go index 69564f6357c..0d7d61066e1 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/aws/aws_test.go +++ b/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 - annotations map[string]string - overriddenFieldName string - overriddenValue int64 + name string + annotations map[string]string + want elb.HealthCheck }{ - {"falls back to HC defaults", map[string]string{}, "", int64(0)}, - {"healthy threshold override", map[string]string{ServiceAnnotationLoadBalancerHCHealthyThreshold: "7"}, "HealthyThreshold", int64(7)}, - {"unhealthy threshold override", map[string]string{ServiceAnnotationLoadBalancerHCUnhealthyThreshold: "7"}, "UnhealthyThreshold", int64(7)}, - {"timeout override", map[string]string{ServiceAnnotationLoadBalancerHCTimeout: "7"}, "Timeout", int64(7)}, - {"interval override", map[string]string{ServiceAnnotationLoadBalancerHCInterval: "7"}, "Interval", int64(7)}, + { + name: "falls back to HC defaults", + annotations: map[string]string{}, + want: elb.HealthCheck{ + 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) - target := "tcp:8080" + protocol, path, port := "TCP", "", int32(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 - if test.overriddenFieldName != "" { // cater for test case with no overrides - value := reflect.ValueOf(&test.overriddenValue) - reflect.ValueOf(&expectedHC).Elem().FieldByName(test.overriddenFieldName).Set(value) - } + expectedHC := test.want 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), - TargetGroupName: request.Name, - Port: request.Port, - Protocol: request.Protocol, + TargetGroupArn: aws.String(arn), + TargetGroupName: request.Name, + Port: request.Port, + 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) + } + }) + } +}