diff --git a/docs/loadbalancer-annotations.md b/docs/loadbalancer-annotations.md index 64c02c0..19a7cab 100644 --- a/docs/loadbalancer-annotations.md +++ b/docs/loadbalancer-annotations.md @@ -239,3 +239,18 @@ This annotation is ignored when `service.beta.kubernetes.io/scw-loadbalancer-ext The possible formats are: - ``: will attach a single Private Network to the LB. - `,`: will attach the two Private Networks to the LB. + +### `service.beta.kubernetes.io/scw-loadbalancer-health-check-from-service` + +This is the annotation to configure the load balancer backend to use the service's `healthCheckNodePort` for health checks. +When enabled for a port, the health check will use the `healthCheckNodePort` from the service specification instead of the regular `NodePort`. +This is particularly useful when the service has `externalTrafficPolicy: Local`, which automatically allocates a `healthCheckNodePort` for health checking. +The possible values are `false`, `true` or `*` for all ports or a comma delimited list of the service ports (for instance `80,443`). + +**Important:** When this annotation is enabled, the health check configuration is overridden with the following settings: +- **Protocol:** HTTP +- **Method:** GET +- **URI:** `/healthz` +- **Expected Code:** 200 + +This configuration is specifically designed to work with Kubernetes' standard health check endpoint. All other health check type annotations (such as `service.beta.kubernetes.io/scw-loadbalancer-health-check-type`, `service.beta.kubernetes.io/scw-loadbalancer-health-check-http-uri`, etc.) are ignored for the ports where this annotation is enabled. diff --git a/scaleway/loadbalancers.go b/scaleway/loadbalancers.go index 59c5ed1..ab2b0fc 100644 --- a/scaleway/loadbalancers.go +++ b/scaleway/loadbalancers.go @@ -994,6 +994,10 @@ func isPortInRange(r string, p int32) (bool, error) { if err != nil { return false, err } + // Validate port is within valid range (1-65535) + if intPort < 1 || intPort > 65535 { + return false, fmt.Errorf("port %d is outside valid range (1-65535)", intPort) + } if int64(p) == intPort { return true, nil } @@ -1158,8 +1162,74 @@ func servicePortToBackend(service *v1.Service, loadbalancer *scwlb.LB, port v1.S return nil, err } - healthCheck := &scwlb.HealthCheck{ - Port: port.NodePort, + healthCheck, err := getNativeHealthCheck(service, port.Port) + if err != nil { + return nil, err + } + + if healthCheck == nil { + healthCheck = &scwlb.HealthCheck{ + Port: port.NodePort, + } + + healthCheckType, err := getHealthCheckType(service, port.NodePort) + if err != nil { + return nil, err + } + + switch healthCheckType { + case "mysql": + hc, err := getMysqlHealthCheck(service, port.NodePort) + if err != nil { + return nil, err + } + healthCheck.MysqlConfig = hc + case "ldap": + hc, err := getLdapHealthCheck(service, port.NodePort) + if err != nil { + return nil, err + } + healthCheck.LdapConfig = hc + case "redis": + hc, err := getRedisHealthCheck(service, port.NodePort) + if err != nil { + return nil, err + } + healthCheck.RedisConfig = hc + case "pgsql": + hc, err := getPgsqlHealthCheck(service, port.NodePort) + if err != nil { + return nil, err + } + healthCheck.PgsqlConfig = hc + case "tcp": + hc, err := getTCPHealthCheck(service, port.NodePort) + if err != nil { + return nil, err + } + healthCheck.TCPConfig = hc + case "http": + hc, err := getHTTPHealthCheck(service, port.NodePort) + if err != nil { + return nil, err + } + healthCheck.HTTPConfig = hc + case "https": + hc, err := getHTTPSHealthCheck(service, port.NodePort) + if err != nil { + return nil, err + } + healthCheck.HTTPSConfig = hc + default: + klog.Errorf("wrong value for healthCheckType") + return nil, errLoadBalancerInvalidAnnotation + } + + healthCheckSendProxy, err := getHealthCheckSendProxy(service) + if err != nil { + return nil, err + } + healthCheck.CheckSendProxy = healthCheckSendProxy } healthCheckDelay, err := getHealthCheckDelay(service) @@ -1186,65 +1256,6 @@ func servicePortToBackend(service *v1.Service, loadbalancer *scwlb.LB, port v1.S } healthCheck.TransientCheckDelay = healthCheckTransientCheckDelay - healthCheckSendProxy, err := getHealthCheckSendProxy(service) - if err != nil { - return nil, err - } - healthCheck.CheckSendProxy = healthCheckSendProxy - - healthCheckType, err := getHealthCheckType(service, port.NodePort) - if err != nil { - return nil, err - } - - switch healthCheckType { - case "mysql": - hc, err := getMysqlHealthCheck(service, port.NodePort) - if err != nil { - return nil, err - } - healthCheck.MysqlConfig = hc - case "ldap": - hc, err := getLdapHealthCheck(service, port.NodePort) - if err != nil { - return nil, err - } - healthCheck.LdapConfig = hc - case "redis": - hc, err := getRedisHealthCheck(service, port.NodePort) - if err != nil { - return nil, err - } - healthCheck.RedisConfig = hc - case "pgsql": - hc, err := getPgsqlHealthCheck(service, port.NodePort) - if err != nil { - return nil, err - } - healthCheck.PgsqlConfig = hc - case "tcp": - hc, err := getTCPHealthCheck(service, port.NodePort) - if err != nil { - return nil, err - } - healthCheck.TCPConfig = hc - case "http": - hc, err := getHTTPHealthCheck(service, port.NodePort) - if err != nil { - return nil, err - } - healthCheck.HTTPConfig = hc - case "https": - hc, err := getHTTPSHealthCheck(service, port.NodePort) - if err != nil { - return nil, err - } - healthCheck.HTTPSConfig = hc - default: - klog.Errorf("wrong value for healthCheckType") - return nil, errLoadBalancerInvalidAnnotation - } - backend := &scwlb.Backend{ Name: fmt.Sprintf("%s_tcp_%d", string(service.UID), port.NodePort), Pool: nodeIPs, diff --git a/scaleway/loadbalancers_annotations.go b/scaleway/loadbalancers_annotations.go index d10944b..fc5d660 100644 --- a/scaleway/loadbalancers_annotations.go +++ b/scaleway/loadbalancers_annotations.go @@ -236,6 +236,12 @@ const ( // - "": will attach a single Private Network to the LB. // - ",": will attach the two Private Networks to the LB. serviceAnnotationPrivateNetworkIDs = "service.beta.kubernetes.io/scw-loadbalancer-pn-ids" + + // serviceAnnotationLoadBalancerHealthCheckFromService is the annotation to use healthCheckNodePort from the service + // The possible values are "false", "true" or "*" for all ports or a comma delimited list of the service port + // (for instance "80,443"). When enabled for a port, the health check will use the service's healthCheckNodePort + // instead of the regular NodePort, overriding any other health check configuration. + serviceAnnotationLoadBalancerHealthCheckFromService = "service.beta.kubernetes.io/scw-loadbalancer-health-check-from-service" ) func getLoadBalancerID(service *v1.Service) (scw.Zone, string, error) { @@ -1057,3 +1063,41 @@ func getEnableHTTP3(service *v1.Service) (bool, error) { } return strconv.ParseBool(enableHTTP3) } + +// getNativeHealthCheck returns the healthCheckNodePort config if the feature is enabled. +// Returns nil if standard legacy logic should be used. +func getNativeHealthCheck(service *v1.Service, targetPort int32) (*scwlb.HealthCheck, error) { + annotationValue := service.Annotations[serviceAnnotationLoadBalancerHealthCheckFromService] + isEnabled, err := isPortInRange(annotationValue, targetPort) + if err != nil { + return nil, fmt.Errorf("invalid health check annotation: %w", err) + } + + if !isEnabled { + return nil, nil + } + + // If the user requested the feature but K8s hasn't assigned the port yet (usually means + // externalTrafficPolicy is not set to Local), we must fall back to avoid blackholing traffic. + hcNodePort := service.Spec.HealthCheckNodePort + if hcNodePort == 0 { + klog.Warningf("Annotation '%s' is active for %s/%s, but HealthCheckNodePort is 0. "+ + "Ensure 'externalTrafficPolicy: Local'. Falling back to standard NodePort.", + serviceAnnotationLoadBalancerHealthCheckFromService, service.Namespace, service.Name) + return nil, nil + } + + // Validate healthCheckNodePort is within valid range + if hcNodePort < 1 || hcNodePort > 65535 { + return nil, fmt.Errorf("invalid healthCheckNodePort %d: port must be in range 1-65535", hcNodePort) + } + + return &scwlb.HealthCheck{ + Port: hcNodePort, + HTTPConfig: &scwlb.HealthCheckHTTPConfig{ + Method: "GET", + Code: scw.Int32Ptr(200), + URI: "/healthz", + }, + }, nil +} diff --git a/scaleway/loadbalancers_test.go b/scaleway/loadbalancers_test.go index 7110673..268674a 100644 --- a/scaleway/loadbalancers_test.go +++ b/scaleway/loadbalancers_test.go @@ -1984,3 +1984,498 @@ func Test_ipMode(t *testing.T) { }) } } + +func TestGetHealthCheckFromService(t *testing.T) { + testCases := []struct { + name string + service *v1.Service + port int32 + wantEnabled bool + wantErr bool + wantErrContains string + }{ + { + name: "annotation not set", + service: &v1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{}, + }, + }, + port: 80, + wantEnabled: false, + wantErr: false, + }, + { + name: "annotation set to true", + service: &v1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + serviceAnnotationLoadBalancerHealthCheckFromService: "true", + }, + }, + }, + port: 80, + wantEnabled: true, + wantErr: false, + }, + { + name: "annotation set to false", + service: &v1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + serviceAnnotationLoadBalancerHealthCheckFromService: "false", + }, + }, + }, + port: 80, + wantEnabled: false, + wantErr: false, + }, + { + name: "annotation set to * (all ports)", + service: &v1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + serviceAnnotationLoadBalancerHealthCheckFromService: "*", + }, + }, + }, + port: 443, + wantEnabled: true, + wantErr: false, + }, + { + name: "annotation set to specific port - match", + service: &v1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + serviceAnnotationLoadBalancerHealthCheckFromService: "80", + }, + }, + }, + port: 80, + wantEnabled: true, + wantErr: false, + }, + { + name: "annotation set to specific port - no match", + service: &v1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + serviceAnnotationLoadBalancerHealthCheckFromService: "80", + }, + }, + }, + port: 443, + wantEnabled: false, + wantErr: false, + }, + { + name: "annotation set to comma-separated ports - match first", + service: &v1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + serviceAnnotationLoadBalancerHealthCheckFromService: "80,443", + }, + }, + }, + port: 80, + wantEnabled: true, + wantErr: false, + }, + { + name: "annotation set to comma-separated ports - match second", + service: &v1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + serviceAnnotationLoadBalancerHealthCheckFromService: "80,443", + }, + }, + }, + port: 443, + wantEnabled: true, + wantErr: false, + }, + { + name: "annotation set to comma-separated ports - no match", + service: &v1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + serviceAnnotationLoadBalancerHealthCheckFromService: "80,443", + }, + }, + }, + port: 8080, + wantEnabled: false, + wantErr: false, + }, + { + name: "annotation with invalid value", + service: &v1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + serviceAnnotationLoadBalancerHealthCheckFromService: "invalid", + }, + }, + }, + port: 80, + wantEnabled: false, + wantErr: true, + wantErrContains: "invalid health check annotation", + }, + { + name: "annotation with negative port", + service: &v1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + serviceAnnotationLoadBalancerHealthCheckFromService: "-1", + }, + }, + }, + port: 80, + wantEnabled: false, + wantErr: true, + wantErrContains: "outside valid range", + }, + { + name: "annotation with port 0", + service: &v1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + serviceAnnotationLoadBalancerHealthCheckFromService: "0", + }, + }, + }, + port: 80, + wantEnabled: false, + wantErr: true, + wantErrContains: "outside valid range", + }, + { + name: "annotation with port > 65535", + service: &v1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + serviceAnnotationLoadBalancerHealthCheckFromService: "99999", + }, + }, + }, + port: 80, + wantEnabled: false, + wantErr: true, + wantErrContains: "outside valid range", + }, + { + name: "annotation with mixed valid and invalid ports", + service: &v1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + serviceAnnotationLoadBalancerHealthCheckFromService: "80,-1,443", + }, + }, + }, + port: 443, + wantEnabled: false, + wantErr: true, + wantErrContains: "outside valid range", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + // Set healthCheckNodePort to test annotation parsing + tc.service.Spec.HealthCheckNodePort = 30100 + + hc, err := getNativeHealthCheck(tc.service, tc.port) + + // Check error cases + if (err != nil) != tc.wantErr { + t.Errorf("getNativeHealthCheck() error = %v, wantErr %v", err, tc.wantErr) + return + } + + if tc.wantErr && err != nil && tc.wantErrContains != "" { + if !strings.Contains(err.Error(), tc.wantErrContains) { + t.Errorf("getNativeHealthCheck() error = %q, want error containing %q", err.Error(), tc.wantErrContains) + } + } + + // Check native health check is enabled/disabled as expected + isEnabled := (hc != nil) + if isEnabled != tc.wantEnabled { + t.Errorf("getNativeHealthCheck() returned healthCheck = %v (enabled=%v), want enabled=%v", hc, isEnabled, tc.wantEnabled) + } + }) + } +} + +func TestServicePortToBackendWithHealthCheckFromService(t *testing.T) { + const nonDefaultHealthCheckDelay = "2s" + const nonDefaultHealthCheckTimeout = "2s" + const nonDefaultHealthCheckMaxRetries = "2" + const nonDefaultHealthCheckTransientCheckDelay = "2s" + + testCases := []struct { + name string + service *v1.Service + port v1.ServicePort + wantNativeHC bool + wantHealthPort int32 + wantErr bool + wantErrContains string + }{ + { + name: "native health check enabled", + service: &v1.Service{ + ObjectMeta: metav1.ObjectMeta{ + UID: "test-uid", + Annotations: map[string]string{ + serviceAnnotationLoadBalancerHealthCheckFromService: "true", + }, + }, + Spec: v1.ServiceSpec{ + HealthCheckNodePort: 30100, + Ports: []v1.ServicePort{ + {Port: 80, NodePort: 30000}, + }, + }, + }, + port: v1.ServicePort{ + Port: 80, + NodePort: 30000, + }, + wantNativeHC: true, + wantHealthPort: 30100, + }, + { + name: "native health check disabled - uses classic", + service: &v1.Service{ + ObjectMeta: metav1.ObjectMeta{ + UID: "test-uid", + Annotations: map[string]string{ + serviceAnnotationLoadBalancerHealthCheckType: "http", + serviceAnnotationLoadBalancerHealthCheckHTTPURI: "/custom", + }, + }, + Spec: v1.ServiceSpec{ + HealthCheckNodePort: 30100, + Ports: []v1.ServicePort{ + {Port: 80, NodePort: 30000}, + }, + }, + }, + port: v1.ServicePort{ + Port: 80, + NodePort: 30000, + }, + wantNativeHC: false, + wantHealthPort: 30000, // uses NodePort + }, + { + name: "native enabled but healthCheckNodePort is 0 - falls back to classic", + service: &v1.Service{ + ObjectMeta: metav1.ObjectMeta{ + UID: "test-uid", + Annotations: map[string]string{ + serviceAnnotationLoadBalancerHealthCheckFromService: "true", + serviceAnnotationLoadBalancerHealthCheckType: "tcp", + }, + }, + Spec: v1.ServiceSpec{ + HealthCheckNodePort: 0, + Ports: []v1.ServicePort{ + {Port: 80, NodePort: 30000}, + }, + }, + }, + port: v1.ServicePort{ + Port: 80, + NodePort: 30000, + }, + wantNativeHC: false, + wantHealthPort: 30000, + }, + { + name: "invalid healthCheckNodePort - negative", + service: &v1.Service{ + ObjectMeta: metav1.ObjectMeta{ + UID: "test-uid", + Annotations: map[string]string{ + serviceAnnotationLoadBalancerHealthCheckFromService: "true", + }, + }, + Spec: v1.ServiceSpec{ + HealthCheckNodePort: -1, + Ports: []v1.ServicePort{ + {Port: 80, NodePort: 30000}, + }, + }, + }, + port: v1.ServicePort{ + Port: 80, + NodePort: 30000, + }, + wantErr: true, + wantErrContains: "invalid healthCheckNodePort", + }, + { + name: "invalid healthCheckNodePort - too high", + service: &v1.Service{ + ObjectMeta: metav1.ObjectMeta{ + UID: "test-uid", + Annotations: map[string]string{ + serviceAnnotationLoadBalancerHealthCheckFromService: "true", + }, + }, + Spec: v1.ServiceSpec{ + HealthCheckNodePort: 99999, + Ports: []v1.ServicePort{ + {Port: 80, NodePort: 30000}, + }, + }, + }, + port: v1.ServicePort{ + Port: 80, + NodePort: 30000, + }, + wantErr: true, + wantErrContains: "invalid healthCheckNodePort", + }, + { + name: "native health check enabled - keep health check config", + service: &v1.Service{ + ObjectMeta: metav1.ObjectMeta{ + UID: "test-uid", + Annotations: map[string]string{ + serviceAnnotationLoadBalancerHealthCheckFromService: "true", + serviceAnnotationLoadBalancerHealthCheckDelay: nonDefaultHealthCheckDelay, + serviceAnnotationLoadBalancerHealthCheckTimeout: nonDefaultHealthCheckTimeout, + serviceAnnotationLoadBalancerHealthCheckMaxRetries: nonDefaultHealthCheckMaxRetries, + serviceAnnotationLoadBalancerHealthTransientCheckDelay: nonDefaultHealthCheckTransientCheckDelay, + }, + }, + Spec: v1.ServiceSpec{ + HealthCheckNodePort: 30100, + Ports: []v1.ServicePort{ + {Port: 80, NodePort: 30000}, + }, + }, + }, + port: v1.ServicePort{ + Port: 80, + NodePort: 30000, + }, + wantNativeHC: true, + wantHealthPort: 30100, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + backend, err := servicePortToBackend(tc.service, &scwlb.LB{ID: "test-lb"}, tc.port, []string{"10.0.0.1"}) + + // Check error cases + if tc.wantErr { + if err == nil { + t.Fatal("Expected error but got nil") + } + if tc.wantErrContains != "" && !strings.Contains(err.Error(), tc.wantErrContains) { + t.Errorf("Expected error containing %q, got %q", tc.wantErrContains, err.Error()) + } + return + } + + // Check success cases + if err != nil { + t.Fatalf("servicePortToBackend() error = %v", err) + } + + if backend == nil { + t.Fatal("servicePortToBackend() returned nil backend") + } + + if backend.HealthCheck == nil { + t.Fatal("backend.HealthCheck is nil") + } + + // Verify health check port + if backend.HealthCheck.Port != tc.wantHealthPort { + t.Errorf("HealthCheck.Port = %v, want %v", backend.HealthCheck.Port, tc.wantHealthPort) + } + + // Verify native health check configuration + if tc.wantNativeHC { + if backend.HealthCheck.HTTPConfig == nil { + t.Error("Expected HTTPConfig for native health check, got nil") + } else { + wantHTTPConfig := &scwlb.HealthCheckHTTPConfig{ + Method: "GET", + Code: scw.Int32Ptr(200), + URI: "/healthz", + } + if !reflect.DeepEqual(backend.HealthCheck.HTTPConfig, wantHTTPConfig) { + t.Errorf("HealthCheck.HTTPConfig = %v, want %v", backend.HealthCheck.HTTPConfig, wantHTTPConfig) + } + notAlteredDelay, err := getHealthCheckDelay(tc.service) + if err != nil { + t.Errorf("error getting health check delay %q", err.Error()) + } + if *backend.HealthCheck.CheckDelay != notAlteredDelay { + t.Errorf("HealthCheck.CheckDelay = %v, want %v", backend.HealthCheck.CheckDelay, notAlteredDelay) + } + notAlteredTimeout, err := getHealthCheckTimeout(tc.service) + if err != nil { + t.Errorf("error getting health check timeout %q", err.Error()) + } + if *backend.HealthCheck.CheckTimeout != notAlteredTimeout { + t.Errorf("HealthCheck.CheckTimeout = %v, want %v", backend.HealthCheck.CheckTimeout, notAlteredTimeout) + } + notAlteredMaxRetries, err := getHealthCheckMaxRetries(tc.service) + if err != nil { + t.Errorf("error getting health check maxRetries %q", err.Error()) + } + if !reflect.DeepEqual(backend.HealthCheck.CheckMaxRetries, notAlteredMaxRetries) { + t.Errorf("HealthCheck.CheckMaxRetries = %v, want %v", backend.HealthCheck.CheckMaxRetries, notAlteredMaxRetries) + } + notAlteredTranscientCheckDelay, err := getHealthCheckTransientCheckDelay(tc.service) + if err != nil { + t.Errorf("error getting health check transcientCheckDelay %q", err.Error()) + } + if !reflect.DeepEqual(backend.HealthCheck.TransientCheckDelay, notAlteredTranscientCheckDelay) { + t.Errorf("HealthCheck.CheckMaxRetries = %v, want %v", backend.HealthCheck.TransientCheckDelay, notAlteredTranscientCheckDelay) + } + if tc.wantNativeHC { + if backend.HealthCheck.CheckSendProxy { + t.Errorf("HealthCheck.CheckSendProxy should be false when native kubernetes health check is used") + } + } + } + } + + // Exactly one health check config must be set + configs := []struct { + name string + set bool + }{ + {"HTTPConfig", backend.HealthCheck.HTTPConfig != nil}, + {"HTTPSConfig", backend.HealthCheck.HTTPSConfig != nil}, + {"TCPConfig", backend.HealthCheck.TCPConfig != nil}, + {"MysqlConfig", backend.HealthCheck.MysqlConfig != nil}, + {"PgsqlConfig", backend.HealthCheck.PgsqlConfig != nil}, + {"RedisConfig", backend.HealthCheck.RedisConfig != nil}, + {"LdapConfig", backend.HealthCheck.LdapConfig != nil}, + } + + var setConfigs []string + for _, cfg := range configs { + if cfg.set { + setConfigs = append(setConfigs, cfg.name) + } + } + + if len(setConfigs) != 1 { + t.Errorf("Expected exactly 1 health check config to be set, got %d: %v", len(setConfigs), setConfigs) + } + }) + } +}