Container Load Balancer - BackendPool #8176

georgeedward2000 · 2025-01-28T10:10:51Z

What type of PR is this?

/kind feature

What this PR does / why we need it:

This PR introduces the implementation of the backend pool interface for pod backend pools. The key changes are as follows:

Backend Pool Interface implementation: A new implementation is introduced to manage pod IPs in the backend pool. It includes functions to ensure hosts in the pool, clean up VM sets, reconcile backend pools, and get backend private IPs.
Instantiation of the Backend Pool implementation when the CLB is desired.
Unit tests

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-01-28T10:10:56Z

❌ - login: @georgeedward2000 / name: George Edward Nechitoaia . The commit (5e0729d, e3510f3, 9a5c9fd, e830609, aa360af, 6de9646, 807b4d6, 7737e3b, b74ed14, 0d329fb, eae85c9, a0170f1, b2624b4, 55bfee0, 34c9c32) 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-01-28T10:11:02Z

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.

kartickmsft · 2025-02-04T11:45:08Z

pkg/provider/azure_loadbalancer_backendpool.go

+func (bpi *backendPoolTypePodIP) CleanupVMSetFromBackendPoolByCondition(_ context.Context, _ *armnetwork.LoadBalancer, _ *v1.Service, _ []*v1.Node, _ string, _ func(string) bool) (*armnetwork.LoadBalancer, error) {
+	return nil, errors.New("CleanupVMSetFromBackendPoolByCondition is not applicable for pod IP backend pool")
+}
+


Probably a comment why this function is a NOP(doesn't do anything) for POD IP based backendpool would be nice.

david-kow · 2025-02-04T10:56:08Z

pkg/consts/consts.go

@@ -381,9 +381,8 @@ const (
 	LoadBalancerBackendPoolConfigurationTypeNodeIPConfiguration = "nodeIPConfiguration"


Can we add some PR description?

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

pkg/provider/azure_local_services.go

+
+func (az *Cloud) getBackendPoolNameForCLBService(service *v1.Service) (string, error) {
+	if isDualStackService(service) {
+		return "", fmt.Errorf("dual-stack service is not supported for container load balancer")


I think we should use names hinting towards "pod ip backend". container load balancer won't mean much here

k8s-ci-robot · 2025-02-04T16:46:11Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: georgeedward2000, kartickmsft
Once this PR has been reviewed and has the lgtm label, please assign feiskyer 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

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

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

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

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

there are some lint failures either, could you rebase to master?

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

pkg/provider/azure_loadbalancer_backendpool.go

+}
+
+func (bpi *backendPoolTypePodIP) CleanupVMSetFromBackendPoolByCondition(_ context.Context, _ *armnetwork.LoadBalancer, _ *v1.Service, _ []*v1.Node, _ string, _ func(string) bool) (*armnetwork.LoadBalancer, error) {
+	return nil, errors.New("CleanupVMSetFromBackendPoolByCondition is not applicable for pod IP backend pool")


return success (return nil,nil) for no-op?

feiskyer · 2025-02-18T05:54:13Z

pkg/provider/azure_local_services.go

+
+func (az *Cloud) getBackendPoolNameForCLBService(service *v1.Service) (string, error) {
+	if isDualStackService(service) {
+		return "", fmt.Errorf("dual-stack service is not supported for container load balancer")


feiskyer · 2025-02-18T05:55:37Z

pkg/provider/azure_loadbalancer_backendpool.go

+	return &backendPoolTypePodIP{c}
+}
+
+func (bpi *backendPoolTypePodIP) CleanupVMSetFromBackendPoolByCondition(_ context.Context, _ *armnetwork.LoadBalancer, _ *v1.Service, _ []*v1.Node, _ string, _ func(string) bool) (*armnetwork.LoadBalancer, error) {


where is the lb part? we'd need first provision lb and then create/update backend pool

If you are referring to the LB changes to support POD IP as backend, then those changes are being done as part of a different PR. Here, changes corresponding to the POD IP backendpool interface is being introduced.

feiskyer · 2025-02-18T05:58:07Z

pkg/provider/azure_local_services.go

+
+	switch service.Spec.IPFamilies[0] {
+	case v1.IPv4Protocol:
+		return string(service.GetUID()), nil


would we have multiple backend. pools on the same CLB? If not, I'd suggest to use service namespace+name for better matching with service object (e.g. namespace-name and namespace-name-ipv6)

CLB will have 1 LB rule(1 FIP) with 1 backendpool. I think namespace-name-ipv6 can exceed the 80 characters limit of NRP resource name. That's why we thought of using UID.

…provider-azure into clb-backendpool

k8s-ci-robot · 2025-02-19T11:43:41Z

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.

…provider-azure into clb-backendpool

nilo19 · 2025-02-20T00:51:38Z

pkg/provider/azure_loadbalancer_backendpool.go

+	}
+
+	backendPrivateIPs := utilsets.NewString()
+	for _, bp := range lb.Properties.BackendAddressPools {


It looks similiar with nodeIP backend pool GetBackendPrivateIPs. Is is possible to reuse some logic as a separate func?

nilo19 · 2025-02-20T00:56:17Z

pkg/provider/azure_loadbalancer_backendpool.go

+	return backendPrivateIPs.UnsortedList(), nil
+}
+
+func (bpi *backendPoolTypePodIP) ReconcileBackendPools(ctx context.Context, _ string, service *v1.Service, lb *armnetwork.LoadBalancer) (bool, bool, *armnetwork.LoadBalancer, error) {


Do we need to remove unwanted nodes in this func?

Endpoint-slice update resulting in endpoint (POD) removal will trigger the removal of POD from the backendpool.
Are you referring to migration scenario?

nilo19 · 2025-02-20T00:58:12Z

Could you please create a github issue, list todo items and link to all related PRs?

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

/label tide/merge-method-squash

georgeedward2000 · 2025-02-21T12:08:09Z

Could you please create a github issue, list todo items and link to all related PRs?

#8395

georgeedward2000 added 5 commits January 9, 2025 14:43

Implement Pod BackendPool

9a5c9fd

complete implementation CLB ReconcileBackendPools

e830609

update getBackendPoolNameForCLBService

5e0729d

complete implementation CLB GetBackendPrivateIPs

e3510f3

Complete testing for Container Load Balancer BackendPool

807b4d6

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 Jan 28, 2025

k8s-ci-robot added cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. do-not-merge/needs-kind labels Jan 28, 2025

k8s-ci-robot requested review from andyzhangx and MartinForReal January 28, 2025 10:11

k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Jan 28, 2025

k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jan 28, 2025

georgeedward2000 changed the title ~~Clb backendpool~~ Container Load Balancer - BackendPool Jan 28, 2025

kartickmsft reviewed Feb 4, 2025

View reviewed changes

david-kow reviewed Feb 4, 2025

View reviewed changes

kartickmsft approved these changes Feb 4, 2025

View reviewed changes

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 5 commits February 18, 2025 12:39

Implement Pod BackendPool

eae85c9

complete implementation CLB ReconcileBackendPools

7737e3b

update getBackendPoolNameForCLBService

b74ed14

complete implementation CLB GetBackendPrivateIPs

6de9646

Complete testing for Container Load Balancer BackendPool

0d329fb

georgeedward2000 force-pushed the clb-backendpool branch from 807b4d6 to 0d329fb Compare February 18, 2025 10:39

k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Feb 18, 2025

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

a0170f1

…provider-azure into clb-backendpool

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

georgeedward2000 added 3 commits February 19, 2025 13:57

update error message + return type

b2624b4

update error message + return type

aa360af

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

55bfee0

…provider-azure into clb-backendpool

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

georgeedward2000 mentioned this pull request Feb 21, 2025

Add support for backendpool of type PodIP #8395

Open

apply DRY by extracting common code in a util function

34c9c32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Container Load Balancer - BackendPool #8176

Container Load Balancer - BackendPool #8176

georgeedward2000 commented Jan 28, 2025 •

edited

Loading

linux-foundation-easycla bot commented Jan 28, 2025 •

edited

Loading

k8s-ci-robot commented Jan 28, 2025

kartickmsft Feb 4, 2025 •

edited

Loading

david-kow Feb 4, 2025

david-kow Feb 4, 2025

feiskyer Feb 18, 2025

k8s-ci-robot commented Feb 4, 2025

feiskyer commented Feb 18, 2025

feiskyer commented Feb 18, 2025

feiskyer left a comment

feiskyer Feb 18, 2025

feiskyer Feb 18, 2025

feiskyer Feb 18, 2025

kartickmsft Feb 19, 2025

feiskyer Feb 18, 2025

kartickmsft Feb 19, 2025

k8s-ci-robot commented Feb 19, 2025

nilo19 Feb 20, 2025

nilo19 Feb 20, 2025

kartickmsft Feb 21, 2025

nilo19 commented Feb 20, 2025

nilo19 commented Feb 20, 2025

georgeedward2000 commented Feb 21, 2025

		@@ -381,9 +381,8 @@ const (
		LoadBalancerBackendPoolConfigurationTypeNodeIPConfiguration = "nodeIPConfiguration"

Container Load Balancer - BackendPool #8176

Are you sure you want to change the base?

Container Load Balancer - BackendPool #8176

Conversation

georgeedward2000 commented Jan 28, 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 Jan 28, 2025 • edited Loading

k8s-ci-robot commented Jan 28, 2025

kartickmsft Feb 4, 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

k8s-ci-robot commented Feb 4, 2025

feiskyer commented Feb 18, 2025

feiskyer commented Feb 18, 2025

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

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

k8s-ci-robot commented Feb 19, 2025

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

georgeedward2000 commented Feb 21, 2025

georgeedward2000 commented Jan 28, 2025 •

edited

Loading

linux-foundation-easycla bot commented Jan 28, 2025 •

edited

Loading

kartickmsft Feb 4, 2025 •

edited

Loading