Skip to content

Commit

Permalink
support customizing probe config when ETP is local
Browse files Browse the repository at this point in the history 
Signed-off-by: Fan Shang Xiang <shafan@microsoft.com>
  • Loading branch information
@MartinForReal
MartinForReal committed Sep 21, 2023
1 parent 4f8dc52 commit e0d1663
Show file tree
Hide file tree
Showing 4 changed files with 185 additions and 68 deletions.
33 changes: 18 additions & 15 deletions pkg/provider/azure_loadbalancer.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2138,6 +2138,7 @@ func (az *Cloud) getExpectedLBRules(
// healthcheck proxy server serves http requests
// https://github.com/kubernetes/kubernetes/blob/7c013c3f64db33cf19f38bb2fc8d9182e42b0b7b/pkg/proxy/healthcheck/service_health.go#L236
var nodeEndpointHealthprobe *network.Probe
var nodeEndpointHealthprobeAdded bool
if servicehelpers.NeedsHealthCheck(service) && !(consts.IsPLSEnabled(service.Annotations) && consts.IsPLSProxyProtocolEnabled(service.Annotations)) {
podPresencePath, podPresencePort := servicehelpers.GetServiceHealthCheckPathPort(service)
lbRuleName := az.getLoadBalancerRuleName(service, v1.ProtocolTCP, podPresencePort)
Expand All @@ -2156,7 +2157,6 @@ func (az *Cloud) getExpectedLBRules(
ProbeThreshold: numberOfProbes,
},
}
expectedProbes = append(expectedProbes, *nodeEndpointHealthprobe)
}

// In HA mode, lb forward traffic of all port to backend
Expand All @@ -2176,7 +2176,7 @@ func (az *Cloud) getExpectedLBRules(
if nodeEndpointHealthprobe == nil {
// use user customized health probe rule if any
for _, port := range service.Spec.Ports {
portprobe, err := az.buildHealthProbeRulesForPort(service, port, lbRuleName)
portprobe, err := az.buildHealthProbeRulesForPort(service, port, lbRuleName, nil)
if err != nil {
klog.V(2).ErrorS(err, "error occurred when buildHealthProbeRulesForPort", "service", service.Name, "namespace", service.Namespace,
"rule-name", lbRuleName, "port", port.Port)
Expand All @@ -2194,6 +2194,7 @@ func (az *Cloud) getExpectedLBRules(
props.Probe = &network.SubResource{
ID: pointer.String(az.getLoadBalancerProbeID(lbName, az.getLoadBalancerResourceGroup(), *nodeEndpointHealthprobe.Name)),
}
expectedProbes = append(expectedProbes, *nodeEndpointHealthprobe)
}

expectedRules = append(expectedRules, network.LoadBalancingRule{
Expand Down Expand Up @@ -2237,23 +2238,25 @@ func (az *Cloud) getExpectedLBRules(
"rule-name", lbRuleName, "port", port.Port)
}
if !isNoHealthProbeRule {
if nodeEndpointHealthprobe == nil {
portprobe, err := az.buildHealthProbeRulesForPort(service, port, lbRuleName)
if err != nil {
klog.V(2).ErrorS(err, "error occurred when buildHealthProbeRulesForPort", "service", service.Name, "namespace", service.Namespace,
"rule-name", lbRuleName, "port", port.Port)
return expectedProbes, expectedRules, err
}
if portprobe != nil {
props.Probe = &network.SubResource{
ID: pointer.String(az.getLoadBalancerProbeID(lbName, az.getLoadBalancerResourceGroup(), *portprobe.Name)),
}
expectedProbes = append(expectedProbes, *portprobe)
portprobe, err := az.buildHealthProbeRulesForPort(service, port, lbRuleName, nodeEndpointHealthprobe)
if err != nil {
klog.V(2).ErrorS(err, "error occurred when buildHealthProbeRulesForPort", "service", service.Name, "namespace", service.Namespace,
"rule-name", lbRuleName, "port", port.Port)
return expectedProbes, expectedRules, err
}
if portprobe != nil {
props.Probe = &network.SubResource{
ID: pointer.String(az.getLoadBalancerProbeID(lbName, *portprobe.Name)),

Check failure on line 2249 in pkg/provider/azure_loadbalancer.go

View workflow job for this annotation

GitHub Actions / Analyze (go) 

not enough arguments in call to az.getLoadBalancerProbeID
}
} else {
expectedProbes = append(expectedProbes, *portprobe)
} else if nodeEndpointHealthprobe != nil {
props.Probe = &network.SubResource{
ID: pointer.String(az.getLoadBalancerProbeID(lbName, az.getLoadBalancerResourceGroup(), *nodeEndpointHealthprobe.Name)),
}
if !nodeEndpointHealthprobeAdded {
expectedProbes = append(expectedProbes, *nodeEndpointHealthprobe)
nodeEndpointHealthprobeAdded = true
}
}
}
if consts.IsK8sServiceDisableLoadBalancerFloatingIP(service) {
Expand Down
104 changes: 53 additions & 51 deletions pkg/provider/azure_loadbalancer_healthprobe.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ import (
// buildHealthProbeRulesForPort
// for following sku: basic loadbalancer vs standard load balancer
// for following protocols: TCP HTTP HTTPS(SLB only)
func (az *Cloud) buildHealthProbeRulesForPort(serviceManifest *v1.Service, port v1.ServicePort, lbrule string) (*network.Probe, error) {
// return nil if no new probe is added
func (az *Cloud) buildHealthProbeRulesForPort(serviceManifest *v1.Service, port v1.ServicePort, lbrule string, healthCheckNodePortProbe *network.Probe) (*network.Probe, error) {
if port.Protocol == v1.ProtocolUDP || port.Protocol == v1.ProtocolSCTP {
return nil, nil
}
Expand All @@ -43,57 +44,7 @@ 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 TCP
if protocol == nil {
protocol = pointer.String(string(network.ProtocolTCP))
}

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

probePort, err := consts.GetHealthProbeConfigOfPortFromK8sSvcAnnotation(serviceManifest.Annotations, port.Port, consts.HealthProbeParamsPort, func(s *string) error {
if s == nil {
Expand Down Expand Up @@ -148,6 +99,57 @@ func (az *Cloud) buildHealthProbeRulesForPort(serviceManifest *v1.Service, port
properties.Port = pointer.Int32(int32(port))
}
}
} else if healthCheckNodePortProbe != nil {
return nil, nil
} else {
properties.Port = &port.NodePort
}
// 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 TCP
if protocol == nil {
protocol = pointer.String(string(network.ProtocolTCP))
}

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

// Select request path
Expand Down
32 changes: 32 additions & 0 deletions pkg/provider/azure_loadbalancer_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2750,6 +2750,38 @@ func TestReconcileLoadBalancerRule(t *testing.T) {
expectedProbes: probes,
expectedRules: rules1DualStack,
})
// ETP is local and port is specified in annotation
svc = getTestServiceDualStack("test1", v1.ProtocolTCP, map[string]string{
consts.ServiceAnnotationPLSCreation: "true",
consts.ServiceAnnotationPLSProxyProtocol: "true",
consts.ServiceAnnotationLoadBalancerHealthProbeProtocol: "tcp",
consts.ServiceAnnotationLoadBalancerHealthProbeRequestPath: "/broken/global/path",
consts.BuildHealthProbeAnnotationKeyForPort(80, consts.HealthProbeParamsProbeInterval): "7",
consts.BuildHealthProbeAnnotationKeyForPort(80, consts.HealthProbeParamsProtocol): "https",
consts.BuildHealthProbeAnnotationKeyForPort(80, consts.HealthProbeParamsRequestPath): "/broken/local/path",
consts.BuildHealthProbeAnnotationKeyForPort(80, consts.HealthProbeParamsNumOfProbe): "15",
consts.BuildHealthProbeAnnotationKeyForPort(80, consts.HealthProbeParamsPort): "421",
}, 80)
svc.Spec.ExternalTrafficPolicy = v1.ServiceExternalTrafficPolicyTypeLocal
svc.Spec.HealthCheckNodePort = 34567
probes = getTestProbes("Https", "/broken/local/path", pointer.Int32(7), pointer.Int32(80), pointer.Int32(421), pointer.Int32(15))
testCases = append(testCases, struct {
desc string
service v1.Service
loadBalancerSku string
probeProtocol string
probePath string
expectedProbes map[bool][]network.Probe
expectedRules map[bool][]network.LoadBalancingRule
expectedErr bool
}{
desc: "getExpectedLBRules should return expected rules when externalTrafficPolicy is local and service.beta.kubernetes.io/azure-pls-proxy-protocol is enabled",
service: svc,
loadBalancerSku: "standard",
probeProtocol: "https",
expectedProbes: probes,
expectedRules: rules1DualStack,
})
for _, test := range testCases {
t.Run(test.desc, func(t *testing.T) {
az := GetTestCloud(ctrl)
Expand Down
84 changes: 82 additions & 2 deletions tests/e2e/network/service_annotations.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ const (
serverPort = 80
alterNativeServicePort = 8080
testingPort = 81
IPV6Prefix = "IPv6"
)

var _ = Describe("Service with annotation", Label(utils.TestSuiteLabelServiceAnnotation), func() {
Expand Down Expand Up @@ -779,6 +780,85 @@ var _ = Describe("Service with annotation", Label(utils.TestSuiteLabelServiceAnn
}
})

It("should support service annotation 'service.beta.kubernetes.io/azure-load-balancer-health-probe-port' and port specific configs", func() {
By("Creating a service with health probe annotations")
annotation := map[string]string{
consts.ServiceAnnotationLoadBalancerHealthProbeNumOfProbe: "5",
consts.BuildHealthProbeAnnotationKeyForPort(serverPort, consts.HealthProbeParamsNumOfProbe): "3",
consts.ServiceAnnotationLoadBalancerHealthProbeInterval: "15",
consts.BuildHealthProbeAnnotationKeyForPort(serverPort, consts.HealthProbeParamsProbeInterval): "10",
consts.ServiceAnnotationLoadBalancerHealthProbeProtocol: "Http",
consts.ServiceAnnotationLoadBalancerHealthProbeRequestPath: "/",
consts.BuildHealthProbeAnnotationKeyForPort(serverPort, consts.HealthProbeParamsPort): "10249",
}

// create service with given annotation and wait it to expose
publicIPs := createAndExposeDefaultServiceWithAnnotation(cs, tc.IPFamily, serviceName, ns.Name, labels, annotation, ports, func(s *v1.Service) error {
s.Spec.HealthCheckNodePort = 32252
s.Spec.ExternalTrafficPolicy = v1.ServiceExternalTrafficPolicyTypeLocal
return nil
})
defer func() {
By("Cleaning up service")
err := utils.DeleteService(cs, ns.Name, serviceName)
Expect(err).NotTo(HaveOccurred())
}()
Expect(len(publicIPs)).NotTo(BeZero())
ids := []string{}
for _, publicIP := range publicIPs {
pipFrontendConfigID := getPIPFrontendConfigurationID(tc, publicIP, tc.GetResourceGroup(), "")
pipFrontendConfigIDSplit := strings.Split(pipFrontendConfigID, "/")
Expect(len(pipFrontendConfigIDSplit)).NotTo(Equal(0))
ids = append(ids, pipFrontendConfigIDSplit[len(pipFrontendConfigIDSplit)-1])
}
utils.Logf("PIP frontend config IDs %q", ids)

var lb *network.LoadBalancer
var targetProbes []*network.Probe
expectedTargetProbesCount := 1
if tc.IPFamily == utils.DualStack {
expectedTargetProbesCount = 2
}
//wait for backend update
err := wait.PollImmediate(5*time.Second, 60*time.Second, func() (bool, error) {
lb = getAzureLoadBalancerFromPIP(tc, publicIPs[0], tc.GetResourceGroup(), "")
targetProbes = []*network.Probe{}
for i := range *lb.LoadBalancerPropertiesFormat.Probes {
probe := (*lb.LoadBalancerPropertiesFormat.Probes)[i]
utils.Logf("One probe of LB is %q", *probe.Name)
probeSplit := strings.Split(*probe.Name, "-")
Expect(len(probeSplit)).NotTo(Equal(0))
probeSplitID := probeSplit[0]
if probeSplit[len(probeSplit)-1] == IPV6Prefix {
probeSplitID += "-" + probeSplit[len(probeSplit)-1]
}
for _, id := range ids {
if id == probeSplitID {
targetProbes = append(targetProbes, &probe)
}
}
}

utils.Logf("targetProbes count %d, expectedTargetProbes count %d", len(targetProbes), expectedTargetProbesCount)
return len(targetProbes) == expectedTargetProbesCount, nil
})
Expect(err).NotTo(HaveOccurred())

By("Validating health probe configs")
for _, probe := range targetProbes {
if probe.ProbeThreshold != nil {
utils.Logf("Validating health probe config numberOfProbes")
Expect(*probe.ProbeThreshold).To(Equal(int32(3)))
}
if probe.IntervalInSeconds != nil {
utils.Logf("Validating health probe config intervalInSeconds")
Expect(*probe.IntervalInSeconds).To(Equal(int32(10)))
}
utils.Logf("Validating health probe config ProbeProtocolHTTP")
Expect(probe.Protocol).To(Equal(network.ProbeProtocolHTTP))
}
})

It("should support service annotation 'service.beta.kubernetes.io/azure-load-balancer-health-probe-num-of-probe', 'service.beta.kubernetes.io/azure-load-balancer-health-probe-interval', 'service.beta.kubernetes.io/azure-load-balancer-health-probe-protocol' and port specific configs", func() {
By("Creating a service with health probe annotations")
annotation := map[string]string{
Expand Down Expand Up @@ -823,7 +903,7 @@ var _ = Describe("Service with annotation", Label(utils.TestSuiteLabelServiceAnn
probeSplit := strings.Split(*probe.Name, "-")
Expect(len(probeSplit)).NotTo(Equal(0))
probeSplitID := probeSplit[0]
if probeSplit[len(probeSplit)-1] == "IPv6" {
if probeSplit[len(probeSplit)-1] == IPV6Prefix {
probeSplitID += "-" + probeSplit[len(probeSplit)-1]
}
for _, id := range ids {
Expand Down Expand Up @@ -893,7 +973,7 @@ var _ = Describe("Service with annotation", Label(utils.TestSuiteLabelServiceAnn
probeSplit := strings.Split(*probe.Name, "-")
Expect(len(probeSplit)).NotTo(Equal(0))
probeSplitID := probeSplit[0]
if probeSplit[len(probeSplit)-1] == "IPv6" {
if probeSplit[len(probeSplit)-1] == IPV6Prefix {
probeSplitID += "-" + probeSplit[len(probeSplit)-1]
}
for _, id := range ids {
Expand Down

0 comments on commit e0d1663

Please sign in to comment.