UPSTREAM: 3887: default to kube-proxy health probes for Service type=…

…LoadBalancer, externalTrafficPolicy=Cluster

pkg/consts/consts.go

@@ -474,8 +474,10 @@ const (
 	HealthProbeParamsProtocol HealthProbeParams = "protocol"
 
 	// HealthProbeParamsPort determines the probe port for the health probe params.
-	// It always takes priority over the NodePort of the spec.ports in a Service
-	HealthProbeParamsPort HealthProbeParams = "port"
+	// It always takes priority over the NodePort of the spec.ports in a Service.
+	// If not set, the kube-proxy health port (10256) will be configured by default.
+	HealthProbeParamsPort         HealthProbeParams = "port"
+	HealthProbeDefaultRequestPort int32             = 10256
 
 	// HealthProbeParamsProbeInterval determines the probe interval of the load balancer health probe.
 	// The minimum probe interval is 5 seconds and the default value is 5. The total duration of all intervals cannot exceed 120 seconds.
@@ -491,7 +493,7 @@ const (
 	// This is only useful for the HTTP and HTTPS, and would be ignored when using TCP. If not set,
 	// `/healthz` would be configured by default.
 	HealthProbeParamsRequestPath  HealthProbeParams = "request-path"
-	HealthProbeDefaultRequestPath string            = "/"
+	HealthProbeDefaultRequestPath string            = "/healthz"
 )
 
 type HealthProbeParams string

pkg/provider/azure_loadbalancer_healthprobe.go

@@ -19,6 +19,7 @@ package provider
 import (
 	"fmt"
 	"strconv"
+
 	"strings"
 
 	"github.com/Azure/azure-sdk-for-go/services/network/mgmt/2022-07-01/network"
@@ -64,7 +65,62 @@ func (az *Cloud) buildHealthProbeRulesForPort(serviceManifest *v1.Service, port
 	// order - Specific Override
 	// port_ annotation
 	// global annotation
+
+	// Select Protocol
+	//
+	var protocol *string
+
+	// 1. Look up port-specific override
+	protocol, err = consts.GetHealthProbeConfigOfPortFromK8sSvcAnnotation(serviceManifest.Annotations, port.Port, consts.HealthProbeParamsProtocol)
+	if err != nil {
+		return nil, fmt.Errorf("failed to parse annotation %s: %w", consts.BuildHealthProbeAnnotationKeyForPort(port.Port, consts.HealthProbeParamsProtocol), err)
+	}
+
+	// 2. If not specified, look up from AppProtocol
+	// Note - this order is to remain compatible with previous versions
+	if protocol == nil {
+		protocol = port.AppProtocol
+	}
+
+	// 3. If protocol is still nil, check the global annotation
+	if protocol == nil {
+		protocol, err = consts.GetAttributeValueInSvcAnnotation(serviceManifest.Annotations, consts.ServiceAnnotationLoadBalancerHealthProbeProtocol)
+		if err != nil {
+			return nil, fmt.Errorf("failed to parse annotation %s: %w", consts.ServiceAnnotationLoadBalancerHealthProbeProtocol, err)
+		}
+	}
+
+	// 4. Finally, if protocol is still nil, default to HTTP
+	if protocol == nil {
+		protocol = pointer.String(string(network.ProtocolHTTP))
+	}
+
+	*protocol = strings.TrimSpace(*protocol)
+	switch {
+	case strings.EqualFold(*protocol, string(network.ProtocolTCP)):
+		properties.Protocol = network.ProbeProtocolTCP
+		properties.Port = &port.NodePort
+	case strings.EqualFold(*protocol, string(network.ProtocolHTTPS)):
+		//HTTPS probe is only supported in standard loadbalancer
+		//For backward compatibility,when unsupported protocol is used, fall back to tcp protocol in basic lb mode instead
+		if !az.useStandardLoadBalancer() {
+			properties.Protocol = network.ProbeProtocolTCP
+			properties.Port = &port.NodePort
+		} else {
+			properties.Protocol = network.ProbeProtocolHTTPS
+		}
+	case strings.EqualFold(*protocol, string(network.ProtocolHTTP)):
+		properties.Protocol = network.ProbeProtocolHTTP
+	default:
+		//For backward compatibility,when unsupported protocol is used, fall back to tcp protocol in basic lb mode instead
+		properties.Protocol = network.ProbeProtocolTCP
+		properties.Port = &port.NodePort
+	}
+
 	// Lookup or Override Health Probe Port
+	if properties.Port == nil {
+		properties.Port = pointer.Int32Ptr(consts.HealthProbeDefaultRequestPort)
+	}
 
 	probePort, err := consts.GetHealthProbeConfigOfPortFromK8sSvcAnnotation(serviceManifest.Annotations, port.Port, consts.HealthProbeParamsPort, func(s *string) error {
 		if s == nil {
@@ -189,11 +245,63 @@ func (az *Cloud) buildHealthProbeRulesForPort(serviceManifest *v1.Service, port
 		}
 		properties.RequestPath = path
 	}
+	// get number of probes
+	var numOfProbeValidator = func(val *int32) error {
+		//minimum number of unhealthy responses is 2. ref: https://docs.microsoft.com/en-us/rest/api/load-balancer/load-balancers/create-or-update#probe
+		const (
+			MinimumNumOfProbe = 2
+		)
+		if *val < MinimumNumOfProbe {
+			return fmt.Errorf("the minimum value of %s is %d", consts.HealthProbeParamsNumOfProbe, MinimumNumOfProbe)
+		}
+		return nil
+	}
+	numberOfProbes, err := consts.GetInt32HealthProbeConfigOfPortFromK8sSvcAnnotation(serviceManifest.Annotations, port.Port, consts.HealthProbeParamsNumOfProbe, numOfProbeValidator)
+	if err != nil {
+		return nil, fmt.Errorf("failed to parse annotation %s: %w", consts.BuildHealthProbeAnnotationKeyForPort(port.Port, consts.HealthProbeParamsNumOfProbe), err)
+	}
+	if numberOfProbes == nil {
+		if numberOfProbes, err = consts.Getint32ValueFromK8sSvcAnnotation(serviceManifest.Annotations, consts.ServiceAnnotationLoadBalancerHealthProbeNumOfProbe, numOfProbeValidator); err != nil {
+			return nil, fmt.Errorf("failed to parse annotation %s: %w", consts.ServiceAnnotationLoadBalancerHealthProbeNumOfProbe, err)
+		}
+	}
+
+	// if numberOfProbes is not set, set it to default instead ref: https://docs.microsoft.com/en-us/rest/api/load-balancer/load-balancers/create-or-update#probe
+	if numberOfProbes == nil {
+		numberOfProbes = pointer.Int32(consts.HealthProbeDefaultNumOfProbe)
+	}
 
-	properties.IntervalInSeconds, properties.ProbeThreshold, err = az.getHealthProbeConfigProbeIntervalAndNumOfProbe(serviceManifest, port.Port)
+	// get probe interval in seconds
+	var probeIntervalValidator = func(val *int32) error {
+		//minimum probe interval in seconds is 5. ref: https://docs.microsoft.com/en-us/rest/api/load-balancer/load-balancers/create-or-update#probe
+		const (
+			MinimumProbeIntervalInSecond = 5
+		)
+		if *val < 5 {
+			return fmt.Errorf("the minimum value of %s is %d", consts.HealthProbeParamsProbeInterval, MinimumProbeIntervalInSecond)
+		}
+		return nil
+	}
+	probeInterval, err := consts.GetInt32HealthProbeConfigOfPortFromK8sSvcAnnotation(serviceManifest.Annotations, port.Port, consts.HealthProbeParamsProbeInterval, probeIntervalValidator)
 	if err != nil {
-		return nil, fmt.Errorf("failed to parse health probe config for port %d: %w", port.Port, err)
+		return nil, fmt.Errorf("failed to parse annotation %s:%w", consts.BuildHealthProbeAnnotationKeyForPort(port.Port, consts.HealthProbeParamsProbeInterval), err)
+	}
+	if probeInterval == nil {
+		if probeInterval, err = consts.Getint32ValueFromK8sSvcAnnotation(serviceManifest.Annotations, consts.ServiceAnnotationLoadBalancerHealthProbeInterval, probeIntervalValidator); err != nil {
+			return nil, fmt.Errorf("failed to parse annotation %s: %w", consts.ServiceAnnotationLoadBalancerHealthProbeInterval, err)
+		}
+	}
+	// if probeInterval is not set, set it to default instead ref: https://docs.microsoft.com/en-us/rest/api/load-balancer/load-balancers/create-or-update#probe
+	if probeInterval == nil {
+		probeInterval = pointer.Int32(consts.HealthProbeDefaultProbeInterval)
+	}
+
+	// total probe should be less than 120 seconds ref: https://docs.microsoft.com/en-us/rest/api/load-balancer/load-balancers/create-or-update#probe
+	if (*probeInterval)*(*numberOfProbes) >= 120 {
+		return nil, fmt.Errorf("total probe should be less than 120, please adjust interval and number of probe accordingly")
 	}
+	properties.IntervalInSeconds = probeInterval
+	properties.ProbeThreshold = numberOfProbes
 	probe := &network.Probe{
 		Name:                  &lbrule,
 		ProbePropertiesFormat: properties,

pkg/provider/azure_loadbalancer_healthprobe_test.go

@@ -57,8 +57,8 @@ func getTestProbe(protocol, path string, interval, servicePort, probePort, numOf
 }
 
 // getDefaultTestProbes returns dualStack probes.
-func getDefaultTestProbes(protocol, path string) map[bool][]network.Probe {
-	return getTestProbes(protocol, path, pointer.Int32(5), pointer.Int32(80), pointer.Int32(10080), pointer.Int32(2))
+func getDefaultTestProbes(protocol, path string, port int32) map[bool][]network.Probe {
+	return getTestProbes(protocol, path, pointer.Int32(5), pointer.Int32(80), pointer.Int32(port), pointer.Int32(2))
 }
 
 func TestFindProbe(t *testing.T) {