Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Container Load Balancer - LB Rules #8218

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
30 changes: 30 additions & 0 deletions pkg/provider/azure_loadbalancer.go
Original file line number Diff line number Diff line change
Expand Up @@ -2800,6 +2800,36 @@ func (az *Cloud) getExpectedLBRules(
isIPv6 bool,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add PR desc

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated the PR description

) ([]*armnetwork.Probe, []*armnetwork.LoadBalancingRule, error) {
var expectedRules []*armnetwork.LoadBalancingRule
// If we are using Pod IP in the LB backend, we skip health probes, disable floating IP and use port.TargetPort.
if az.IsLBBackendPoolTypePodIP() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please extract this as a new func to make the main loop concise?

// Check for multi-IP families in the service spec (not allowed for BackendPool of type PodIP).
if len(service.Spec.IPFamilies) > 1 {
return nil, nil, fmt.Errorf("dual-stack services are not supported when LB backend pool type is PodIP")
Copy link
Contributor

@nilo19 nilo19 Feb 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the design doc it says "For dual stack, 2 services need to be created". Can you double check which one is the right behavior?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, that is right. Should I update the error string to say precisely that?

}
// Build rules for each service port but with floatingIP = false, no health probes.
for _, port := range service.Spec.Ports {
ruleName := az.getLoadBalancerRuleName(service, port.Protocol, port.Port, isIPv6)
transportProto, _, _, err := getProtocolsFromKubernetesProtocol(port.Protocol)
if err != nil {
return nil, nil, fmt.Errorf("failed to parse transport protocol: %w", err)
}
props, err := az.getExpectedLoadBalancingRulePropertiesForPort(service, lbFrontendIPConfigID, lbBackendPoolID, port, transportProto)
if err != nil {
return nil, nil, fmt.Errorf("error generating LB rule: %w", err)
}
// Turn off floating IP and skip health probe attachments.
props.EnableFloatingIP = ptr.To(false)
props.Probe = nil
props.BackendPort = ptr.To(int32(port.TargetPort.IntValue()))

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we won't be able to support named Port (https://kubernetes.io/docs/concepts/services-networking/service/#field-spec-ports). Better to throw a validation error.

expectedRules = append(expectedRules, &armnetwork.LoadBalancingRule{
Name: &ruleName,
Properties: props,
})
}
return nil, expectedRules, nil
}

// Otherwise, the existing flow remains unchanged
var expectedProbes []*armnetwork.Probe

// support podPresence health check when External Traffic Policy is local
Expand Down
4 changes: 4 additions & 0 deletions pkg/provider/config/azure.go
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,10 @@ func (az *Config) IsLBBackendPoolTypeNodeIP() bool {
return strings.EqualFold(az.LoadBalancerBackendPoolConfigurationType, consts.LoadBalancerBackendPoolConfigurationTypeNodeIP)
}

func (az *Config) IsLBBackendPoolTypePodIP() bool {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need a check to block podIP + non-standardV2?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If conflicted with multi-slb, need to block as well.

return strings.EqualFold(az.LoadBalancerBackendPoolConfigurationType, consts.LoadBalancerBackendPoolConfigurationTypePodIP)
}

func (az *Config) GetPutVMSSVMBatchSize() int {
return az.PutVMSSVMBatchSize
}
Expand Down