[DualStack] Support FrontendIPConfig and reconcileLB() #3819

lzhecheng · 2023-04-26T07:26:37Z

What type of PR is this?

/kind feature

What this PR does / why we need it:

[DualStack] Support FrontendIPConfig and reconcileLB()

DualStack feature code
Refactor related functions and methods
Refactor and add new UTs

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?

[DualStack] Support FrontendIPConfig and reconcileLB() * DualStack feature code * Refactor related functions and methods * Refactor and add new UTs

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

netlify · 2023-04-26T07:26:43Z

✅ Deploy Preview for kubernetes-sigs-cloud-provide-azure canceled.

Name	Link
🔨 Latest commit	`822139e`
🔍 Latest deploy log	https://app.netlify.com/sites/kubernetes-sigs-cloud-provide-azure/deploys/6458b288caf46900083023a0

k8s-ci-robot · 2023-04-26T07:27:05Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: lzhecheng

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

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [lzhecheng]

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

pkg/provider/azure_loadbalancer.go

jwtty · 2023-04-27T01:00:47Z

pkg/provider/azure_loadbalancer.go

 			}
-			if !existsSubnet {
-				return false, fmt.Errorf("failed to get subnet")
+			if subnet.ID == nil {


when can this happen? when subnet is not nil but subnet.ID is nil?

I think first subnet is always not nil. Then, before subnet is az.getSubnet() for the first time, its ID is nil.

jwtty · 2023-04-27T01:52:52Z

pkg/provider/azure_utils.go

 	}
 	var fipPIPID string
 	if fip.FrontendIPConfigurationPropertiesFormat != nil && fip.FrontendIPConfigurationPropertiesFormat.PublicIPAddress != nil {
 		fipPIPID = pointer.StringDeref(fip.FrontendIPConfigurationPropertiesFormat.PublicIPAddress.ID, "")
 	}
+	pips, err := az.listPIP(az.ResourceGroup)


pips may not be in az.ResourceGroup. That can be specified from annotation.

Good catch! Updated

jwtty · 2023-05-08T05:44:42Z

pkg/provider/azure_loadbalancer.go

 	for _, existingRule := range rules {
-		if strings.EqualFold(pointer.StringDeref(existingRule.Name, ""), pointer.StringDeref(rule.Name, "")) &&
-			equalLoadBalancingRulePropertiesFormat(existingRule.LoadBalancingRulePropertiesFormat, rule.LoadBalancingRulePropertiesFormat, wantLB) {
+		// NOTICE: After dualstack support, old IPv6 cluster's Rule name doesn't have IPv6 suffix.


Do we still need this?

jwtty · 2023-05-08T05:44:55Z

pkg/provider/azure_loadbalancer.go

 	for _, existingProbe := range probes {
-		if strings.EqualFold(pointer.StringDeref(existingProbe.Name, ""), pointer.StringDeref(probe.Name, "")) &&
+		// NOTICE: After dualstack support, old IPv6 only cluster's Probe name doesn't have IPv6 suffix.


Do we still need this?

jwtty · 2023-05-08T05:45:58Z

pkg/provider/azure_loadbalancer.go

@@ -3348,6 +3453,20 @@ func equalSubResource(s *network.SubResource, t *network.SubResource) bool {
 	return strings.EqualFold(pointer.StringDeref(s.ID, ""), pointer.StringDeref(t.ID, ""))
 }

+func equalSubResourceForIPv6Only(s *network.SubResource, t *network.SubResource, isIPv6Only bool) bool {


In single stack, there will be no v6Suffix and in dual stack, there will always be v6Suffix. Do we still need this?

Thank you for reminding! Removed.

nilo19 · 2023-05-08T06:22:25Z

pkg/provider/azure_utils.go

@@ -494,3 +501,22 @@ func getResourceIDPrefix(id string) string {
 	}
 	return id[:idx]
 }
+
+// fillSubnet fills subnet value into the variable.
+// TODO: UT


nit: remove the todo

Thank you for reminding! Added a UT.

* DualStack feature code * Refactor related functions and methods * Refactor and add new UTs Signed-off-by: Zhecheng Li <zhechengli@microsoft.com>

nilo19 · 2023-05-09T03:06:04Z

/lgtm

MartinForReal · 2023-05-23T02:31:23Z

/cherrypick release-1.27

MartinForReal · 2023-05-23T02:31:29Z

/cherrypick release-1.26

MartinForReal · 2023-05-23T02:31:36Z

/cherrypick release-1.25

MartinForReal · 2023-05-23T02:31:41Z

/cherrypick release-1.24

k8s-infra-cherrypick-robot · 2023-05-23T02:32:14Z

@MartinForReal: #3819 failed to apply on top of branch "release-1.27":

Applying: Support FrontendIPConfig and reconcileLB() * DualStack feature code * Refactor related functions and methods * Refactor and add new UTs
Using index info to reconstruct a base tree...
M	pkg/provider/azure_loadbalancer.go
M	pkg/provider/azure_loadbalancer_backendpool.go
M	pkg/provider/azure_loadbalancer_test.go
M	pkg/provider/azure_standard.go
M	pkg/provider/azure_standard_test.go
M	pkg/provider/azure_test.go
M	pkg/provider/azure_utils.go
M	pkg/provider/azure_wrap.go
Falling back to patching base and 3-way merge...
Auto-merging pkg/provider/azure_wrap.go
Auto-merging pkg/provider/azure_utils.go
CONFLICT (content): Merge conflict in pkg/provider/azure_utils.go
Auto-merging pkg/provider/azure_test.go
Auto-merging pkg/provider/azure_standard_test.go
Auto-merging pkg/provider/azure_standard.go
Auto-merging pkg/provider/azure_loadbalancer_test.go
Auto-merging pkg/provider/azure_loadbalancer_backendpool.go
CONFLICT (content): Merge conflict in pkg/provider/azure_loadbalancer_backendpool.go
Auto-merging pkg/provider/azure_loadbalancer.go
CONFLICT (content): Merge conflict in pkg/provider/azure_loadbalancer.go
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 Support FrontendIPConfig and reconcileLB() * DualStack feature code * Refactor related functions and methods * Refactor and add new UTs
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

In response to this:

/cherrypick release-1.27

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/test-infra repository.

k8s-infra-cherrypick-robot · 2023-05-23T02:32:18Z

@MartinForReal: #3819 failed to apply on top of branch "release-1.26":

Applying: Support FrontendIPConfig and reconcileLB() * DualStack feature code * Refactor related functions and methods * Refactor and add new UTs
Using index info to reconstruct a base tree...
M	pkg/provider/azure_loadbalancer.go
M	pkg/provider/azure_loadbalancer_backendpool.go
M	pkg/provider/azure_loadbalancer_test.go
M	pkg/provider/azure_privatelinkservice_test.go
M	pkg/provider/azure_standard.go
M	pkg/provider/azure_standard_test.go
M	pkg/provider/azure_test.go
M	pkg/provider/azure_utils.go
M	pkg/provider/azure_utils_test.go
M	pkg/provider/azure_wrap.go
Falling back to patching base and 3-way merge...
Auto-merging pkg/provider/azure_wrap.go
Auto-merging pkg/provider/azure_utils_test.go
CONFLICT (content): Merge conflict in pkg/provider/azure_utils_test.go
Auto-merging pkg/provider/azure_utils.go
CONFLICT (content): Merge conflict in pkg/provider/azure_utils.go
Auto-merging pkg/provider/azure_test.go
CONFLICT (content): Merge conflict in pkg/provider/azure_test.go
Auto-merging pkg/provider/azure_standard_test.go
CONFLICT (content): Merge conflict in pkg/provider/azure_standard_test.go
Auto-merging pkg/provider/azure_standard.go
CONFLICT (content): Merge conflict in pkg/provider/azure_standard.go
Auto-merging pkg/provider/azure_privatelinkservice_test.go
CONFLICT (content): Merge conflict in pkg/provider/azure_privatelinkservice_test.go
Auto-merging pkg/provider/azure_loadbalancer_test.go
CONFLICT (content): Merge conflict in pkg/provider/azure_loadbalancer_test.go
Auto-merging pkg/provider/azure_loadbalancer_backendpool.go
CONFLICT (content): Merge conflict in pkg/provider/azure_loadbalancer_backendpool.go
Auto-merging pkg/provider/azure_loadbalancer.go
CONFLICT (content): Merge conflict in pkg/provider/azure_loadbalancer.go
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 Support FrontendIPConfig and reconcileLB() * DualStack feature code * Refactor related functions and methods * Refactor and add new UTs
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

In response to this:

/cherrypick release-1.26

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/test-infra repository.

k8s-infra-cherrypick-robot · 2023-05-23T02:32:22Z

@MartinForReal: #3819 failed to apply on top of branch "release-1.25":

Applying: Support FrontendIPConfig and reconcileLB() * DualStack feature code * Refactor related functions and methods * Refactor and add new UTs
Using index info to reconstruct a base tree...
M	pkg/provider/azure_loadbalancer.go
M	pkg/provider/azure_loadbalancer_backendpool.go
M	pkg/provider/azure_loadbalancer_test.go
M	pkg/provider/azure_privatelinkservice_test.go
M	pkg/provider/azure_standard.go
M	pkg/provider/azure_standard_test.go
M	pkg/provider/azure_test.go
M	pkg/provider/azure_utils.go
M	pkg/provider/azure_utils_test.go
M	pkg/provider/azure_wrap.go
Falling back to patching base and 3-way merge...
Auto-merging pkg/provider/azure_wrap.go
Auto-merging pkg/provider/azure_utils_test.go
CONFLICT (content): Merge conflict in pkg/provider/azure_utils_test.go
Auto-merging pkg/provider/azure_utils.go
CONFLICT (content): Merge conflict in pkg/provider/azure_utils.go
Auto-merging pkg/provider/azure_test.go
CONFLICT (content): Merge conflict in pkg/provider/azure_test.go
Auto-merging pkg/provider/azure_standard_test.go
CONFLICT (content): Merge conflict in pkg/provider/azure_standard_test.go
Auto-merging pkg/provider/azure_standard.go
CONFLICT (content): Merge conflict in pkg/provider/azure_standard.go
Auto-merging pkg/provider/azure_privatelinkservice_test.go
CONFLICT (content): Merge conflict in pkg/provider/azure_privatelinkservice_test.go
Auto-merging pkg/provider/azure_loadbalancer_test.go
CONFLICT (content): Merge conflict in pkg/provider/azure_loadbalancer_test.go
Auto-merging pkg/provider/azure_loadbalancer_backendpool.go
CONFLICT (content): Merge conflict in pkg/provider/azure_loadbalancer_backendpool.go
Auto-merging pkg/provider/azure_loadbalancer.go
CONFLICT (content): Merge conflict in pkg/provider/azure_loadbalancer.go
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 Support FrontendIPConfig and reconcileLB() * DualStack feature code * Refactor related functions and methods * Refactor and add new UTs
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

In response to this:

/cherrypick release-1.25

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/test-infra repository.

k8s-infra-cherrypick-robot · 2023-05-23T02:32:26Z

@MartinForReal: #3819 failed to apply on top of branch "release-1.24":

Applying: Support FrontendIPConfig and reconcileLB() * DualStack feature code * Refactor related functions and methods * Refactor and add new UTs
Using index info to reconstruct a base tree...
M	pkg/provider/azure_loadbalancer.go
M	pkg/provider/azure_loadbalancer_backendpool.go
M	pkg/provider/azure_loadbalancer_test.go
M	pkg/provider/azure_privatelinkservice_test.go
M	pkg/provider/azure_standard.go
M	pkg/provider/azure_standard_test.go
M	pkg/provider/azure_test.go
M	pkg/provider/azure_utils.go
M	pkg/provider/azure_utils_test.go
M	pkg/provider/azure_wrap.go
Falling back to patching base and 3-way merge...
Auto-merging pkg/provider/azure_wrap.go
Auto-merging pkg/provider/azure_utils_test.go
CONFLICT (content): Merge conflict in pkg/provider/azure_utils_test.go
Auto-merging pkg/provider/azure_utils.go
CONFLICT (content): Merge conflict in pkg/provider/azure_utils.go
Auto-merging pkg/provider/azure_test.go
CONFLICT (content): Merge conflict in pkg/provider/azure_test.go
Auto-merging pkg/provider/azure_standard_test.go
CONFLICT (content): Merge conflict in pkg/provider/azure_standard_test.go
Auto-merging pkg/provider/azure_standard.go
CONFLICT (content): Merge conflict in pkg/provider/azure_standard.go
Auto-merging pkg/provider/azure_privatelinkservice_test.go
CONFLICT (content): Merge conflict in pkg/provider/azure_privatelinkservice_test.go
Auto-merging pkg/provider/azure_loadbalancer_test.go
CONFLICT (content): Merge conflict in pkg/provider/azure_loadbalancer_test.go
Auto-merging pkg/provider/azure_loadbalancer_backendpool.go
CONFLICT (content): Merge conflict in pkg/provider/azure_loadbalancer_backendpool.go
Auto-merging pkg/provider/azure_loadbalancer.go
CONFLICT (content): Merge conflict in pkg/provider/azure_loadbalancer.go
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 Support FrontendIPConfig and reconcileLB() * DualStack feature code * Refactor related functions and methods * Refactor and add new UTs
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

In response to this:

/cherrypick release-1.24

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/test-infra repository.

k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. kind/feature Categorizes issue or PR as related to a new feature. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Apr 26, 2023

k8s-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Apr 26, 2023

k8s-ci-robot requested review from MartinForReal and nilo19 April 26, 2023 07:27

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 26, 2023

MartinForReal reviewed Apr 26, 2023

View reviewed changes

jwtty reviewed Apr 27, 2023

View reviewed changes

lzhecheng force-pushed the dualstack-lb-frontendipconfigs branch 2 times, most recently from cded8c5 to 9993a17 Compare May 5, 2023 10:28

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 7, 2023

lzhecheng force-pushed the dualstack-lb-frontendipconfigs branch from 9993a17 to fae6c69 Compare May 8, 2023 02:03

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 8, 2023

jwtty reviewed May 8, 2023

View reviewed changes

nilo19 reviewed May 8, 2023

View reviewed changes

lzhecheng force-pushed the dualstack-lb-frontendipconfigs branch from fae6c69 to 934f824 Compare May 8, 2023 08:17

[DualStack] Support FrontendIPConfig and reconcileLB()

822139e

* DualStack feature code * Refactor related functions and methods * Refactor and add new UTs Signed-off-by: Zhecheng Li <zhechengli@microsoft.com>

lzhecheng force-pushed the dualstack-lb-frontendipconfigs branch from 934f824 to 822139e Compare May 8, 2023 08:27

k8s-ci-robot assigned nilo19 May 9, 2023

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 9, 2023

k8s-ci-robot merged commit 5e4c55b into kubernetes-sigs:master May 9, 2023
20 checks passed

lzhecheng deleted the dualstack-lb-frontendipconfigs branch May 9, 2023 09:23

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[DualStack] Support FrontendIPConfig and reconcileLB() #3819

[DualStack] Support FrontendIPConfig and reconcileLB() #3819

lzhecheng commented Apr 26, 2023

netlify bot commented Apr 26, 2023 •

edited

k8s-ci-robot commented Apr 26, 2023

jwtty Apr 27, 2023

lzhecheng May 5, 2023

jwtty Apr 27, 2023

lzhecheng May 5, 2023

jwtty May 8, 2023

lzhecheng May 8, 2023

jwtty May 8, 2023

lzhecheng May 8, 2023

jwtty May 8, 2023

lzhecheng May 8, 2023

nilo19 May 8, 2023

lzhecheng May 8, 2023

nilo19 commented May 9, 2023

MartinForReal commented May 23, 2023

MartinForReal commented May 23, 2023

MartinForReal commented May 23, 2023

MartinForReal commented May 23, 2023

k8s-infra-cherrypick-robot commented May 23, 2023

k8s-infra-cherrypick-robot commented May 23, 2023

k8s-infra-cherrypick-robot commented May 23, 2023

k8s-infra-cherrypick-robot commented May 23, 2023

[DualStack] Support FrontendIPConfig and reconcileLB() #3819

[DualStack] Support FrontendIPConfig and reconcileLB() #3819

Conversation

lzhecheng commented Apr 26, 2023

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

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

netlify bot commented Apr 26, 2023 • edited

✅ Deploy Preview for kubernetes-sigs-cloud-provide-azure canceled.

k8s-ci-robot commented Apr 26, 2023

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

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 May 9, 2023

MartinForReal commented May 23, 2023

MartinForReal commented May 23, 2023

MartinForReal commented May 23, 2023

MartinForReal commented May 23, 2023

k8s-infra-cherrypick-robot commented May 23, 2023

k8s-infra-cherrypick-robot commented May 23, 2023

k8s-infra-cherrypick-robot commented May 23, 2023

k8s-infra-cherrypick-robot commented May 23, 2023

netlify bot commented Apr 26, 2023 •

edited