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.

Sign up for GitHub

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

Jump to bottom

feat: support migrate from NIC-based to IP-based backend pool by migration API by setting "enableMigrateToIPBasedBackendPoolAPI": true #3972

Merged
merged 1 commit into from Jun 5, 2023
Merged

feat: support migrate from NIC-based to IP-based backend pool by migration API by setting "enableMigrateToIPBasedBackendPoolAPI": true #3972

merged 1 commit into from Jun 5, 2023

Conversation

nilo19
Copy link
Contributor

@nilo19 nilo19 commented May 25, 2023 •
edited

What type of PR is this?

/kind feature

What this PR does / why we need it:

Support migrate from NIC-based to IP-based backend pool by migration API by setting "enableMigrateToIpBasedBackendPoolAPI": true. By using this API, there will be no downtime during the migration.

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?

feat: Support migrate from NIC-based to IP-based backend pool by migration API by setting `"enableMigrateToIpBasedBackendPoolAPI": true`. By using this API, there will be no downtime during the migration.

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

@k8s-ci-robot 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 May 25, 2023
@netlify
Copy link

netlify bot commented May 25, 2023 •
edited

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

Name Link
🔨 Latest commit d97617f
🔍 Latest deploy log https://app.netlify.com/sites/kubernetes-sigs-cloud-provide-azure/deploys/64741722a51f790008bbf261

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: nilo19

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:

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 k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels May 25, 2023
@nilo19 nilo19 force-pushed the feat/migration-api branch 4 times, most recently from cdd7146 to 04d4ed4 Compare May 25, 2023 08:22
@nilo19 nilo19 changed the title feat: support migrate from NIC-based to IP-based backend pool by migration API by setting "enableMigrateToIpBasedBackendPoolAPI": true feat: support migrate from NIC-based to IP-based backend pool by migration API by setting "enableMigrateToIPBasedBackendPoolAPI": true May 25, 2023
feiskyer
feiskyer reviewed May 26, 2023
View reviewed changes
pkg/azureclients/loadbalancerclient/azure_loadbalancerclient.go Outdated
response, rerr := c.armClient.GetResourceWithExpandQuery(ctx, resourceID, expand)
defer c.armClient.CloseResponse(ctx, response)
if rerr != nil {
klog.V(5).Infof("Received error in %s: resourceID: %s, error: %s", "loadbalancer.get.request", resourceID, rerr.Error())
Copy link
Member

@feiskyer feiskyer May 26, 2023 •
edited

Choose a reason for hiding this comment

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

nit: loadbalancer.backendpool.get.request

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

feiskyer
feiskyer reviewed May 26, 2023
View reviewed changes
pkg/azureclients/loadbalancerclient/azure_loadbalancerclient.go Outdated
azure.WithErrorUnlessStatusCode(http.StatusOK),
autorest.ByUnmarshallingJSON(&result))
if err != nil {
klog.V(5).Infof("Received error in %s: resourceID: %s, error: %s", "loadbalancer.get.respond", resourceID, err)
Copy link
Member

Choose a reason for hiding this comment

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

loadbalancer.backendpool.get.respond

@nilo19 nilo19 force-pushed the feat/migration-api branch 2 times, most recently from be76293 to 81a34d5 Compare May 26, 2023 14:20
jwtty
jwtty reviewed May 29, 2023
View reviewed changes
pkg/azureclients/loadbalancerclient/azure_loadbalancerclient.go Outdated
// Report errors if the client is throttled.
if c.RetryAfterReader.After(time.Now()) {
mc.ThrottledCount()
rerr := retry.GetThrottlingError("LBGet", "client throttled", c.RetryAfterReader)
Copy link
Member

Choose a reason for hiding this comment

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

nit: LBBackendPoolGet

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@nilo19
feat: support migrate from NIC-based to IP-based backend pool by migr…
d97617f 
…ation API by setting `"enableMigrateToIpBasedBackendPoolAPI": true`
@nilo19
Copy link
Contributor Author

nilo19 commented May 29, 2023

/retest

2 similar comments
@nilo19
Copy link
Contributor Author

nilo19 commented May 29, 2023

/retest

@nilo19
Copy link
Contributor Author

nilo19 commented Jun 2, 2023

/retest

@MartinForReal
Copy link
Contributor

LGTM

feiskyer
feiskyer reviewed Jun 5, 2023
View reviewed changes
Copy link
Member

@feiskyer feiskyer left a comment

Choose a reason for hiding this comment

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

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 5, 2023
@k8s-ci-robot k8s-ci-robot merged commit 700ed37 into kubernetes-sigs:master Jun 5, 2023
21 checks passed
@nilo19 nilo19 deleted the feat/migration-api branch June 5, 2023 07:47
@nilo19
Copy link
Contributor Author

nilo19 commented Jun 6, 2023

/cherrypick release-1.27

@k8s-infra-cherrypick-robot
Copy link

@nilo19: #3972 failed to apply on top of branch "release-1.27":

Applying: feat: support migrate from NIC-based to IP-based backend pool by migration API by setting `"enableMigrateToIpBasedBackendPoolAPI": true`
Using index info to reconstruct a base tree...
M	pkg/provider/azure.go
M	pkg/provider/azure_backoff.go
M	pkg/provider/azure_backoff_test.go
M	pkg/provider/azure_loadbalancer.go
M	pkg/provider/azure_loadbalancer_backendpool.go
M	pkg/provider/azure_loadbalancer_test.go
M	pkg/provider/azure_mock_vmsets.go
M	pkg/provider/azure_test.go
M	pkg/provider/azure_utils.go
M	pkg/provider/azure_vmss.go
M	site/content/en/topics/loadbalancer.md
Falling back to patching base and 3-way merge...
Auto-merging site/content/en/topics/loadbalancer.md
Auto-merging pkg/provider/azure_vmss.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_mock_vmsets.go
Auto-merging pkg/provider/azure_loadbalancer_test.go
Auto-merging pkg/provider/azure_loadbalancer_backendpool.go
Auto-merging pkg/provider/azure_loadbalancer.go
Auto-merging pkg/provider/azure_backoff_test.go
Auto-merging pkg/provider/azure_backoff.go
Auto-merging pkg/provider/azure.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 feat: support migrate from NIC-based to IP-based backend pool by migration API by setting `"enableMigrateToIpBasedBackendPoolAPI": true`
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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@feiskyer feiskyer feiskyer left review comments

@MartinForReal MartinForReal MartinForReal left review comments

@jwtty jwtty jwtty left review comments

@andyzhangx andyzhangx Awaiting requested review from andyzhangx

Assignees

@feiskyer feiskyer

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/feature Categorizes issue or PR as related to a new feature. lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@nilo19 @k8s-ci-robot @MartinForReal @k8s-infra-cherrypick-robot @feiskyer @jwtty