Container Load Balancer - LB Rules #8218

georgeedward2000 · 2025-02-04T08:21:48Z

What type of PR is this?

/kind feature

What this PR does / why we need it:

This PR introduces updates to LB rules and probes for the Container Load Balancer. The key changes are as follows:

Floating IP Disabled: The floatingIP property in the LB Rule settings is set to false.
Health Probes: Modifications have been made to ensure that health probes are not selected or set up.
Pod Target Port: The target port is now utilized in the LB Rule settings, ensuring proper routing of traffic to the intended port.

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Added the ability to set the type of backend pool to PodIP. 
This action allows the Load balancer attached to the service to route traffic directly to the containers, 
instead of routing to AKS nodes and then AKS node routing it to the PODs hosting the service.

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

linux-foundation-easycla · 2025-02-04T08:21:53Z

❌ - login: @georgeedward2000 / name: George Edward Nechitoaia . The commit (29bb3c8, e9693c6, 2a45c63, 1027221, e1eb35d, f33f04e, f52242e, f51f58c, 8f489e7, 81d5568) is not authorized under a signed CLA. Please click here to be authorized. For further assistance with EasyCLA, please submit a support request ticket.

k8s-ci-robot · 2025-02-04T08:21:54Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: georgeedward2000
Once this PR has been reviewed and has the lgtm label, please assign lzhecheng for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

k8s-ci-robot · 2025-02-04T08:21:59Z

Hi @georgeedward2000. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

david-kow · 2025-02-04T12:04:49Z

pkg/provider/azure_loadbalancer.go

@@ -2800,6 +2800,36 @@ func (az *Cloud) getExpectedLBRules(
 	isIPv6 bool,
 ) ([]*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() {
+		// Check for multi-IP families in the service spec (not allowed for CLB).


rewrite to mention pod ip backend config instead of clb

update in the latest commit

david-kow · 2025-02-04T12:09:53Z

pkg/provider/azure_loadbalancer.go

@@ -2800,6 +2800,36 @@ func (az *Cloud) getExpectedLBRules(
 	isIPv6 bool,


add PR desc

updated the PR description

kartickmsft · 2025-02-07T11:59:51Z

pkg/provider/azure_loadbalancer.go

+			// 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()))


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.

feiskyer · 2025-02-18T05:26:40Z

@georgeedward2000 Thanks for the contribution. Could you sign CLA?

feiskyer · 2025-02-18T05:29:41Z

And could you use the template (the template would show when you open the PR) and fill the template contents (including release notes information together with what this PR does)?

/kind feature

feiskyer · 2025-02-18T05:45:52Z

pkg/consts/consts.go

@@ -382,8 +382,7 @@ const (
 	// LoadBalancerBackendPoolConfigurationTypeNodeIP is the lb backend pool config type node ip
 	LoadBalancerBackendPoolConfigurationTypeNodeIP = "nodeIP"
 	// LoadBalancerBackendPoolConfigurationTypePODIP is the lb backend pool config type pod ip
-	// TODO (nilo19): support pod IP in the future
-	LoadBalancerBackendPoolConfigurationTypePODIP = "podIP"
+	LoadBalancerBackendPoolConfigurationTypePodIP = "podIP"
 )


Good catch for the variable name here👍

Have we decided to use this instead of loadBalancerClass?

feiskyer

There are also a set of lint failures, could you rebase the PR to master branch?

feiskyer · 2025-02-18T05:49:59Z

pkg/provider/azure_loadbalancer.go

+		// Build rules for each service port but with floatingIP = false, no health probes.
+		for _, port := range service.Spec.Ports {
+			if port.TargetPort.Type == intstr.String {
+				return nil, nil, fmt.Errorf("named targetPort is not supported when LB backend pool type is PodIP: %s", port.TargetPort.StrVal)


are we able to make port name work?

Since, CLB directly rewrites the Destination Port in the packet after selecting the Destination POD, I think we can't support named Port specified in https://kubernetes.io/docs/concepts/services-networking/service/#field-spec-ports

nilo19 · 2025-02-20T00:24:10Z

pkg/provider/azure_loadbalancer.go

+	if az.IsLBBackendPoolTypePodIP() {
+		// 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")


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?

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

nilo19 · 2025-02-20T00:27:32Z

pkg/provider/azure_loadbalancer.go

@@ -2815,6 +2816,39 @@ func (az *Cloud) getExpectedLBRules(
 	isIPv6 bool,
 ) ([]*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() {


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

nilo19 · 2025-02-20T00:30:15Z

pkg/provider/config/azure.go

@@ -180,6 +180,10 @@ func (az *Config) IsLBBackendPoolTypeNodeIP() bool {
 	return strings.EqualFold(az.LoadBalancerBackendPoolConfigurationType, consts.LoadBalancerBackendPoolConfigurationTypeNodeIP)
 }

+func (az *Config) IsLBBackendPoolTypePodIP() bool {


Need a check to block podIP + non-standardV2?

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

nilo19 · 2025-02-20T01:00:48Z

I think for features under development, release note should be none, as they have no impact to the user for now. @feiskyer correct me if wrong.

nilo19 · 2025-02-20T03:25:56Z

/label tide/merge-method-squash

…vider-azure into clb-lb-rules

k8s-ci-robot · 2025-02-21T10:06:21Z

Adding label do-not-merge/contains-merge-commits because PR contains merge commits, which are not allowed in this repository.
Use git rebase to reapply your commits on top of the target branch. Detailed instructions for doing so can be found here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

add formation flow for clb lb rules

e9693c6

k8s-ci-robot added the do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. label Feb 4, 2025

k8s-ci-robot requested review from jwtty and lzhecheng February 4, 2025 08:21

k8s-ci-robot added cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. do-not-merge/needs-kind needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Feb 4, 2025

k8s-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Feb 4, 2025

david-kow reviewed Feb 4, 2025

View reviewed changes

comment typo

2a45c63

kartickmsft reviewed Feb 7, 2025

View reviewed changes

georgeedward2000 added 2 commits February 7, 2025 16:39

named targetPort is not supported within CLB context

e1eb35d

add unit tests

29bb3c8

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Feb 10, 2025

k8s-ci-robot added kind/feature Categorizes issue or PR as related to a new feature. and removed do-not-merge/needs-kind labels Feb 18, 2025

feiskyer reviewed Feb 18, 2025

View reviewed changes

georgeedward2000 added 4 commits February 18, 2025 12:49

add formation flow for clb lb rules

f52242e

comment typo

f51f58c

named targetPort is not supported within CLB context

1027221

add unit tests

8f489e7

georgeedward2000 force-pushed the clb-lb-rules branch from 29bb3c8 to 8f489e7 Compare February 18, 2025 10:49

k8s-ci-robot added the release-note Denotes a PR that will be considered when it comes time to generate release notes. label Feb 18, 2025

k8s-ci-robot removed the do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. label Feb 18, 2025

nilo19 reviewed Feb 20, 2025

View reviewed changes

k8s-ci-robot added the tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. label Feb 20, 2025

Merge branch 'clb-lb-rules' of https://github.com/david-kow/cloud-pro…

f33f04e

…vider-azure into clb-lb-rules

k8s-ci-robot added the do-not-merge/contains-merge-commits label Feb 21, 2025

extract function + check standardV2 sku

81d5568

georgeedward2000 mentioned this pull request Feb 21, 2025

Add support for backendpool of type PodIP #8395

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Container Load Balancer - LB Rules #8218

Container Load Balancer - LB Rules #8218

georgeedward2000 commented Feb 4, 2025 •

edited

Loading

linux-foundation-easycla bot commented Feb 4, 2025 •

edited

Loading

k8s-ci-robot commented Feb 4, 2025

k8s-ci-robot commented Feb 4, 2025

david-kow Feb 4, 2025

georgeedward2000 Feb 4, 2025

david-kow Feb 4, 2025

georgeedward2000 Feb 4, 2025

kartickmsft Feb 7, 2025

feiskyer commented Feb 18, 2025

feiskyer commented Feb 18, 2025

feiskyer Feb 18, 2025

nilo19 Feb 20, 2025

feiskyer left a comment

feiskyer Feb 18, 2025

kartickmsft Feb 19, 2025

nilo19 Feb 20, 2025 •

edited

Loading

georgeedward2000 Feb 20, 2025

nilo19 Feb 20, 2025

nilo19 Feb 20, 2025

nilo19 Feb 20, 2025

nilo19 commented Feb 20, 2025

nilo19 commented Feb 20, 2025

k8s-ci-robot commented Feb 21, 2025

		@@ -2800,6 +2800,36 @@ func (az *Cloud) getExpectedLBRules(
		isIPv6 bool,

Container Load Balancer - LB Rules #8218

Are you sure you want to change the base?

Container Load Balancer - LB Rules #8218

Conversation

georgeedward2000 commented Feb 4, 2025 • edited Loading

What type of PR is this?

What this PR does / why we need it:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

linux-foundation-easycla bot commented Feb 4, 2025 • edited Loading

k8s-ci-robot commented Feb 4, 2025

k8s-ci-robot commented Feb 4, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

feiskyer commented Feb 18, 2025

feiskyer commented Feb 18, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

feiskyer left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nilo19 Feb 20, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nilo19 commented Feb 20, 2025

nilo19 commented Feb 20, 2025

k8s-ci-robot commented Feb 21, 2025

georgeedward2000 commented Feb 4, 2025 •

edited

Loading

linux-foundation-easycla bot commented Feb 4, 2025 •

edited

Loading

nilo19 Feb 20, 2025 •

edited

Loading