Skip to content

Commit

Permalink
Add port and protocol probe annotations
Browse files Browse the repository at this point in the history 
Add support for overriding health check probe protocol and port per
probe port.

Document the port and protocol probe override annotations.

Signed-off-by: Vito Sabella <vsabella@msn.com>

De-duplicate load balancer health probes by collecting them in an index
keyed by their functional characteristics (protocol, port, interval,
count, and path). Two probes with the same functional characteristics
will generate the same key, and only one of the two will ultimately be added.

Signed-off-by: Travis Raines <571832+rainest@users.noreply.github.com>
Co-authored-by: Travis Raines <571832+rainest@users.noreply.github.com>
Signed-off-by: MartinForReal <fanshangxiang@gmail.com>
  • Loading branch information
@vsabella @rainest @MartinForReal
2 people authored and MartinForReal committed Nov 25, 2022
1 parent 7cea83d commit bb6d21e
Show file tree
Hide file tree
Showing 4 changed files with 217 additions and 37 deletions.
8 changes: 8 additions & 0 deletions pkg/consts/consts.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -423,6 +423,14 @@ const CreatedByTag = "k8s-azure-created-by"
const (
HealthProbeAnnotationPrefixPattern = "service.beta.kubernetes.io/port_%d_health-probe_"

// HealthProbeParamsProtocol determines the protocol for the health probe params.
// It always takes priority over spec.appProtocol or any other specified protocol
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"

// 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.
HealthProbeParamsProbeInterval HealthProbeParams = "interval"
Expand Down
101 changes: 88 additions & 13 deletions pkg/provider/azure_loadbalancer.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2070,33 +2070,85 @@ func (az *Cloud) buildHealthProbeRulesForPort(annotations map[string]string, por

properties := &network.ProbePropertiesFormat{}
var err error
if port.AppProtocol == nil {
if port.AppProtocol, err = consts.GetAttributeValueInSvcAnnotation(annotations, consts.ServiceAnnotationLoadBalancerHealthProbeProtocol); err != nil {

// order - Specific Override
// port_ annotation
// global annotation

// Select Protocol
//
var protocol *string

// 1. Look up port-specific override
protocol, err = consts.GetHealthProbeConfigOfPortFromK8sSvcAnnotation(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(annotations, consts.ServiceAnnotationLoadBalancerHealthProbeProtocol)
if err != nil {
return nil, fmt.Errorf("failed to parse annotation %s: %w", consts.ServiceAnnotationLoadBalancerHealthProbeProtocol, err)
}
if port.AppProtocol == nil {
port.AppProtocol = to.StringPtr(string(network.ProtocolTCP))
}
}
protocol := strings.TrimSpace(*port.AppProtocol)

// 4. Finally, if protocol is still nil, default to TCP
if protocol == nil {
protocol = to.StringPtr(string(network.ProtocolTCP))
}

*protocol = strings.TrimSpace(*protocol)
switch {
case strings.EqualFold(protocol, string(network.ProtocolTCP)):
case strings.EqualFold(*protocol, string(network.ProtocolTCP)):
properties.Protocol = network.ProbeProtocolTCP
case strings.EqualFold(protocol, string(network.ProtocolHTTPS)):
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
} else {
properties.Protocol = network.ProbeProtocolHTTPS
}
case strings.EqualFold(protocol, string(network.ProtocolHTTP)):
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
}

// Lookup or Override Health Probe Port
properties.Port = &port.NodePort

// Look up port-specific override
var probePortValidator = 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 (
MinProbePort = 1
MaxProbePort = 65535
)
if *val < MinProbePort || *val > MaxProbePort {
return fmt.Errorf("the value of %s must be between %d and %d inclusive", consts.HealthProbeParamsPort, MinProbePort, MaxProbePort)
}
return nil
}

probePort, err := consts.GetInt32HealthProbeConfigOfPortFromK8sSvcAnnotation(annotations, port.Port, consts.HealthProbeParamsPort, probePortValidator)
if err != nil {
return nil, fmt.Errorf("failed to parse annotation %s: %w", consts.BuildHealthProbeAnnotationKeyForPort(port.Port, consts.HealthProbeParamsPort), err)
}

if probePort != nil {
properties.Port = probePort
}

// Select request path
if strings.EqualFold(string(properties.Protocol), string(network.ProtocolHTTPS)) || strings.EqualFold(string(properties.Protocol), string(network.ProtocolHTTP)) {
// get request path ,only used with http/https probe
path, err := consts.GetHealthProbeConfigOfPortFromK8sSvcAnnotation(annotations, port.Port, consts.HealthProbeParamsRequestPath)
Expand Down Expand Up @@ -2170,14 +2222,32 @@ func (az *Cloud) buildHealthProbeRulesForPort(annotations map[string]string, por
}
properties.IntervalInSeconds = probeInterval
properties.NumberOfProbes = numberOfProbes
properties.Port = &port.NodePort
probe := &network.Probe{
Name: &lbrule,
ProbePropertiesFormat: properties,
}
return probe, nil
}

// probeKey generates a string key containing all functional probe configuration, to allow de-duplication of like probes.
func probeKey(probe network.Probe) string {
var port, interval, number int32
var path string
if probe.Port != nil {
port = *probe.Port
}
if probe.IntervalInSeconds != nil {
interval = *probe.IntervalInSeconds
}
if probe.NumberOfProbes != nil {
number = *probe.NumberOfProbes
}
if probe.RequestPath != nil {
path = *probe.RequestPath
}
return fmt.Sprintf("%s-%d-%d-%d-%s", probe.Protocol, port, interval, number, path)
}

// buildLBRules
// for following sku: basic loadbalancer vs standard load balancer
// for following scenario: internal vs external
Expand All @@ -2189,6 +2259,7 @@ func (az *Cloud) getExpectedLBRules(

var expectedRules []network.LoadBalancingRule
var expectedProbes []network.Probe
uniqueProbes := make(map[string]network.Probe)

// support podPresence health check when External Traffic Policy is local
// take precedence over user defined probe configuration
Expand All @@ -2209,7 +2280,7 @@ func (az *Cloud) getExpectedLBRules(
NumberOfProbes: to.Int32Ptr(consts.HealthProbeDefaultNumOfProbe),
},
}
expectedProbes = append(expectedProbes, *nodeEndpointHealthprobe)
uniqueProbes[probeKey(*nodeEndpointHealthprobe)] = *nodeEndpointHealthprobe
}

// In HA mode, lb forward traffic of all port to backend
Expand All @@ -2236,7 +2307,7 @@ func (az *Cloud) getExpectedLBRules(
//ignore error because we only need one correct rule
}
if portprobe != nil {
expectedProbes = append(expectedProbes, *portprobe)
uniqueProbes[probeKey(*portprobe)] = *portprobe
props.Probe = &network.SubResource{
ID: to.StringPtr(az.getLoadBalancerProbeID(lbName, az.getLoadBalancerResourceGroup(), *portprobe.Name)),
}
Expand Down Expand Up @@ -2282,7 +2353,7 @@ func (az *Cloud) getExpectedLBRules(
return expectedProbes, expectedRules, err
}
if portprobe != nil {
expectedProbes = append(expectedProbes, *portprobe)
uniqueProbes[probeKey(*portprobe)] = *portprobe
props.Probe = &network.SubResource{
ID: to.StringPtr(az.getLoadBalancerProbeID(lbName, az.getLoadBalancerResourceGroup(), *portprobe.Name)),
}
Expand All @@ -2304,6 +2375,10 @@ func (az *Cloud) getExpectedLBRules(
}
}

for _, probe := range uniqueProbes {
expectedProbes = append(expectedProbes, probe)
}

return expectedProbes, expectedRules, nil
}

Expand Down
96 changes: 78 additions & 18 deletions pkg/provider/azure_loadbalancer_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2344,7 +2344,7 @@ func TestReconcileLoadBalancerRule(t *testing.T) {
expectedRules: getDefaultTestRules(true),
},
{
desc: "getExpectedLBRules should return error when deprecated tcp health probe annotations and protocols are added and config is not valid",
desc: "getExpectedLBRules should return error when probe interval * num > 120",
service: getTestService("test1", v1.ProtocolTCP, map[string]string{
"service.beta.kubernetes.io/port_80_health-probe_interval": "10",
"service.beta.kubernetes.io/port_80_health-probe_num-of-probe": "20",
Expand All @@ -2355,7 +2355,7 @@ func TestReconcileLoadBalancerRule(t *testing.T) {
expectedErr: true,
},
{
desc: "getExpectedLBRules should return error when deprecated tcp health probe annotations and protocols are added and config is not valid",
desc: "getExpectedLBRules should return error when probe interval * num == 120",
service: getTestService("test1", v1.ProtocolTCP, map[string]string{
"service.beta.kubernetes.io/port_80_health-probe_interval": "10",
"service.beta.kubernetes.io/port_80_health-probe_num-of-probe": "20",
Expand All @@ -2374,18 +2374,18 @@ func TestReconcileLoadBalancerRule(t *testing.T) {
loadBalancerSku: "standard",
probeProtocol: "Https",
probePath: "/healthy1",
expectedProbes: getTestProbes("Https", "/healthy1", to.Int32Ptr(20), to.Int32Ptr(10080), to.Int32Ptr(5)),
expectedProbes: getTestProbes("Https", "/healthy1", to.Int32Ptr(20), to.Int32Ptr(80), to.Int32Ptr(10080), to.Int32Ptr(5)),
expectedRules: getDefaultTestRules(true),
},
{
desc: "getExpectedLBRules should return correct rule when health probe annotations are added,default path should be /healthy",
desc: "getExpectedLBRules should return correct rule when health probe annotations are added,default path should be /",
service: getTestService("test1", v1.ProtocolTCP, map[string]string{
"service.beta.kubernetes.io/port_80_health-probe_interval": "20",
"service.beta.kubernetes.io/port_80_health-probe_num-of-probe": "5",
}, false, 80),
loadBalancerSku: "standard",
probeProtocol: "Http",
expectedProbes: getTestProbes("Http", "/", to.Int32Ptr(20), to.Int32Ptr(10080), to.Int32Ptr(5)),
expectedProbes: getTestProbes("Http", "/", to.Int32Ptr(20), to.Int32Ptr(80), to.Int32Ptr(10080), to.Int32Ptr(5)),
expectedRules: getDefaultTestRules(true),
},
{
Expand All @@ -2396,7 +2396,7 @@ func TestReconcileLoadBalancerRule(t *testing.T) {
}, false, 80),
loadBalancerSku: "standard",
probeProtocol: "Tcp",
expectedProbes: getTestProbes("Tcp", "", to.Int32Ptr(20), to.Int32Ptr(10080), to.Int32Ptr(5)),
expectedProbes: getTestProbes("Tcp", "", to.Int32Ptr(20), to.Int32Ptr(80), to.Int32Ptr(10080), to.Int32Ptr(5)),
expectedRules: getDefaultTestRules(true),
},
{
Expand Down Expand Up @@ -2448,6 +2448,60 @@ func TestReconcileLoadBalancerRule(t *testing.T) {
},
expectedProbes: getDefaultTestProbes("Tcp", ""),
},
{
desc: "getExpectedLBRules should prioritize port specific probe protocol over defaults",
service: getTestService("test1", v1.ProtocolTCP, map[string]string{
"service.beta.kubernetes.io/port_80_health-probe_protocol": "HtTp",
}, false, 80),
expectedRules: getDefaultTestRules(false),
expectedProbes: getDefaultTestProbes("Http", "/"),
},
{
desc: "getExpectedLBRules should prioritize port specific probe protocol over appProtocol",
service: getTestService("test1", v1.ProtocolTCP, map[string]string{
"service.beta.kubernetes.io/port_80_health-probe_protocol": "HtTp",
}, false, 80),
probeProtocol: "Mongodb",
expectedRules: getDefaultTestRules(false),
expectedProbes: getDefaultTestProbes("Http", "/"),
},
{
desc: "getExpectedLBRules should prioritize port specific probe protocol over deprecated annotation",
service: getTestService("test1", v1.ProtocolTCP, map[string]string{
"service.beta.kubernetes.io/port_80_health-probe_protocol": "HtTpS",
"service.beta.kubernetes.io/azure-load-balancer-health-probe-protocol": "TcP",
}, false, 80),
loadBalancerSku: "standard",
probeProtocol: "Https",
expectedRules: getDefaultTestRules(true),
expectedProbes: getDefaultTestProbes("Https", "/"),
},
{
desc: "getExpectedLBRules should default to Tcp on invalid port specific probe protocol",
service: getTestService("test1", v1.ProtocolTCP, map[string]string{
"service.beta.kubernetes.io/port_80_health-probe_protocol": "FooBar",
}, false, 80),
probeProtocol: "Http",
expectedRules: getDefaultTestRules(false),
expectedProbes: getDefaultTestProbes("Tcp", ""),
},
{
desc: "getExpectedLBRules should allow setting port specific health probe port",
service: getTestService("test1", v1.ProtocolTCP, map[string]string{
"service.beta.kubernetes.io/port_80_health-probe_protocol": "Http",
"service.beta.kubernetes.io/port_80_health-probe_port": "15012",
}, false, 80),
expectedRules: getDefaultTestRules(false),
expectedProbes: getTestProbes("Http", "/", to.Int32Ptr(5), to.Int32Ptr(80), to.Int32Ptr(15012), to.Int32Ptr(2)),
},
{
desc: "getExpectedLBRules should not include duplicate probes when overrides would create them",
service: getTestService("test1", v1.ProtocolTCP, map[string]string{
"service.beta.kubernetes.io/port_8000_health-probe_port": "10080",
}, false, 80, 8000),
expectedRules: []network.LoadBalancingRule{getTestRule(false, 80), getTestRule(false, 8000)},
expectedProbes: getTestProbes("Tcp", "/", to.Int32Ptr(5), to.Int32Ptr(8000), to.Int32Ptr(10080), to.Int32Ptr(2)),
},
}
rules := getDefaultTestRules(true)
rules[0].IdleTimeoutInMinutes = to.Int32Ptr(5)
Expand All @@ -2469,7 +2523,7 @@ func TestReconcileLoadBalancerRule(t *testing.T) {
}, false, 80),
loadBalancerSku: "standard",
probeProtocol: "Tcp",
expectedProbes: getTestProbes("Tcp", "", to.Int32Ptr(10), to.Int32Ptr(10080), to.Int32Ptr(10)),
expectedProbes: getTestProbes("Tcp", "", to.Int32Ptr(10), to.Int32Ptr(80), to.Int32Ptr(10080), to.Int32Ptr(10)),
expectedRules: rules,
})
rules1 := []network.LoadBalancingRule{
Expand All @@ -2480,14 +2534,19 @@ func TestReconcileLoadBalancerRule(t *testing.T) {
rules1[0].Probe.ID = to.StringPtr("/subscriptions/subscription/resourceGroups/rg/providers/Microsoft.Network/loadBalancers/lbname/probes/atest1-TCP-34567")
rules1[1].Probe.ID = to.StringPtr("/subscriptions/subscription/resourceGroups/rg/providers/Microsoft.Network/loadBalancers/lbname/probes/atest1-TCP-34567")
rules1[2].Probe.ID = to.StringPtr("/subscriptions/subscription/resourceGroups/rg/providers/Microsoft.Network/loadBalancers/lbname/probes/atest1-TCP-34567")

// When the service spec externalTrafficPolicy is Local all of these annotations should be ignored
svc := getTestService("test1", v1.ProtocolTCP, map[string]string{
"service.beta.kubernetes.io/port_80_health-probe_interval": "10",
"service.beta.kubernetes.io/port_80_health-probe_num-of-probe": "10",
consts.ServiceAnnotationLoadBalancerHealthProbeProtocol: "tcp",
consts.ServiceAnnotationLoadBalancerHealthProbeRequestPath: "/broken/global/path",
consts.BuildHealthProbeAnnotationKeyForPort(80, consts.HealthProbeParamsProbeInterval): "10",
consts.BuildHealthProbeAnnotationKeyForPort(80, consts.HealthProbeParamsProtocol): "https",
consts.BuildHealthProbeAnnotationKeyForPort(80, consts.HealthProbeParamsRequestPath): "/broken/local/path",
consts.BuildHealthProbeAnnotationKeyForPort(80, consts.HealthProbeParamsNumOfProbe): "10",
}, false, 80, 443, 421)
svc.Spec.ExternalTrafficPolicy = v1.ServiceExternalTrafficPolicyTypeLocal
svc.Spec.HealthCheckNodePort = 34567
probes := getTestProbes("Http", "/healthz", to.Int32Ptr(5), to.Int32Ptr(34567), to.Int32Ptr(2))
probes[0].Name = to.StringPtr("atest1-TCP-34567")
probes := getTestProbes("Http", "/healthz", to.Int32Ptr(5), to.Int32Ptr(34567), to.Int32Ptr(34567), to.Int32Ptr(2))
testCases = append(testCases, struct {
desc string
service v1.Service
Expand All @@ -2498,7 +2557,7 @@ func TestReconcileLoadBalancerRule(t *testing.T) {
expectedRules []network.LoadBalancingRule
expectedErr bool
}{
desc: "getExpectedLBRules should expected rules when externaltrafficpolicy is local",
desc: "getExpectedLBRules should expected rules when externalTrafficPolicy is local",
service: svc,
loadBalancerSku: "standard",
probeProtocol: "Http",
Expand Down Expand Up @@ -2528,18 +2587,19 @@ func TestReconcileLoadBalancerRule(t *testing.T) {
}
}
}
func getTestProbes(protocol, path string, interval, port, numOfProbe *int32) []network.Probe {

func getTestProbes(protocol, path string, interval, servicePort, probePort, numOfProbe *int32) []network.Probe {
return []network.Probe{
getTestProbe(protocol, path, interval, port, numOfProbe),
getTestProbe(protocol, path, interval, servicePort, probePort, numOfProbe),
}
}

func getTestProbe(protocol, path string, interval, port, numOfProbe *int32) network.Probe {
func getTestProbe(protocol, path string, interval, servicePort, probePort, numOfProbe *int32) network.Probe {
expectedProbes := network.Probe{
Name: to.StringPtr(fmt.Sprintf("atest1-TCP-%d", *port-10000)),
Name: to.StringPtr(fmt.Sprintf("atest1-TCP-%d", *servicePort)),
ProbePropertiesFormat: &network.ProbePropertiesFormat{
Protocol: network.ProbeProtocol(protocol),
Port: port,
Port: probePort,
IntervalInSeconds: interval,
NumberOfProbes: numOfProbe,
},
Expand All @@ -2550,7 +2610,7 @@ func getTestProbe(protocol, path string, interval, port, numOfProbe *int32) netw
return expectedProbes
}
func getDefaultTestProbes(protocol, path string) []network.Probe {
return getTestProbes(protocol, path, to.Int32Ptr(5), to.Int32Ptr(10080), to.Int32Ptr(2))
return getTestProbes(protocol, path, to.Int32Ptr(5), to.Int32Ptr(80), to.Int32Ptr(10080), to.Int32Ptr(2))
}

func getDefaultTestRules(enableTCPReset bool) []network.LoadBalancingRule {
Expand Down

0 comments on commit bb6d21e

Please sign in to comment.