Skip VMSS VM update when no matching disks found during detach operation

[landreasyan]

What type of PR is this?

/kind improvement

What this PR does / why we need it:

This PR optimizes the VMSS DetachDisk operation by skipping unnecessary VM updates when no matching disks are found to detach. This can occur in two scenarios:

1. The VM has no data disks attached
2. The requested disk(s) to detach are not present on the VM

Previously, the code would proceed with a VM update even when no disks needed to be detached, which could lead to:

• Unnecessary API calls to Azure
• VM entering a pending state during critical failures (e.g., zone outages)
• Blocking clients from seeing successful disk operations
• Delayed recovery during outage scenarios

Changes made:

• azure_controller_vmss.go: Early return in DetachDisk() when no matching disks are found, avoiding the VM update call
• azure_controller_vmss_test.go: Added comprehensive test cases covering:
  • VM with no disks attached
  • Single non-matching disk to detach
  • Multiple non-matching disks to detach
  • Verification that VM update is skipped in these scenarios

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?

none

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

none

[k8s-ci-robot]

@landreasyan: The label(s) kind/improvement cannot be applied, because the repository doesn't have them.

Details

In response to this:

What type of PR is this?

/kind improvement

What this PR does / why we need it:

This PR optimizes the VMSS DetachDisk operation by skipping unnecessary VM updates when no matching disks are found to detach. This can occur in two scenarios:

1. The VM has no data disks attached
2. The requested disk(s) to detach are not present on the VM

Previously, the code would proceed with a VM update even when no disks needed to be detached, which could lead to:

• Unnecessary API calls to Azure
• VM entering a pending state during critical failures (e.g., zone outages)
• Blocking clients from seeing successful disk operations
• Delayed recovery during outage scenarios

Changes made:

• azure_controller_vmss.go: Early return in DetachDisk() when no matching disks are found, avoiding the VM update call
• azure_controller_vmss_test.go: Added comprehensive test cases covering:
• VM with no disks attached
• Single non-matching disk to detach
• Multiple non-matching disks to detach
• Verification that VM update is skipped in these scenarios

Which issue(s) this PR fixes:

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.:

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.

[landreasyan]

/retest

[landreasyan]

/retest

[k8s-ci-robot]

@landreasyan: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
pull-cloud-provider-azure-e2e-ccm-vmss-capz	e6e41bf	link	true	/test pull-cloud-provider-azure-e2e-ccm-vmss-capz
pull-cloud-provider-azure-e2e-ccm-vmss-shared-probe-capz	e6e41bf	link	true	/test pull-cloud-provider-azure-e2e-ccm-vmss-shared-probe-capz
pull-cloud-provider-azure-e2e-ccm-dualstack-capz	e6e41bf	link	true	/test pull-cloud-provider-azure-e2e-ccm-dualstack-capz
pull-cloud-provider-azure-e2e-ccm-dualstack-vmss-capz	e6e41bf	link	true	/test pull-cloud-provider-azure-e2e-ccm-dualstack-vmss-capz
pull-cloud-provider-azure-e2e-ccm-vmss-ip-lb-capz	e6e41bf	link	true	/test pull-cloud-provider-azure-e2e-ccm-vmss-ip-lb-capz
pull-cloud-provider-azure-e2e-ccm-capz	e6e41bf	link	true	/test pull-cloud-provider-azure-e2e-ccm-capz
pull-cloud-provider-azure-e2e-capz	e6e41bf	link	true	/test pull-cloud-provider-azure-e2e-capz
pull-cloud-provider-azure-e2e-ccm-vmss-multi-slb-capz	e6e41bf	link	true	/test pull-cloud-provider-azure-e2e-ccm-vmss-multi-slb-capz
pull-cloud-provider-azure-e2e-ccm-ipv6-capz	e6e41bf	link	true	/test pull-cloud-provider-azure-e2e-ccm-ipv6-capz
pull-cloud-provider-azure-e2e-ccm-ipv6-vmss-capz	e6e41bf	link	true	/test pull-cloud-provider-azure-e2e-ccm-ipv6-vmss-capz
pull-cloud-provider-azure-e2e-ccm-vmssflex-capz	e6e41bf	link	true	/test pull-cloud-provider-azure-e2e-ccm-vmssflex-capz

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Details

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. I understand the commands that are listed here.

[Copilot]

Pull request overview

This PR optimizes the DetachDisk operation for VMSS by preventing unnecessary VM updates when no matching disks are found to detach. This avoids wasted API calls and prevents VMs from entering pending states during critical failures.

• Early return added when no disks match the detach request, skipping the VM update entirely
• Comprehensive test coverage for three scenarios: no disks on VM, single non-matching disk, and multiple non-matching disks
• Improved logging to clearly indicate when VM updates are skipped

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
pkg/provider/azure_controller_vmss.go	Added early return logic to skip VM update when no matching disks found, restructured control flow, and improved success/error logging
pkg/provider/azure_controller_vmss_test.go	Added test infrastructure (noDisksOnVM, skipUpdate flags) and three new test cases to verify VM update is correctly skipped when no matching disks exist

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[andyzhangx]

/kind cleanup
/lgtm

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: andyzhangx, landreasyan

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [andyzhangx]

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

[landreasyan]

/cherrypick release-1.31

[k8s-infra-cherrypick-robot]

@landreasyan: #9798 failed to apply on top of branch "release-1.31":

Applying: chore: return early success on vmss detach disk
Using index info to reconstruct a base tree...
M	pkg/provider/azure_controller_vmss.go
M	pkg/provider/azure_controller_vmss_test.go
Falling back to patching base and 3-way merge...
Auto-merging pkg/provider/azure_controller_vmss_test.go
CONFLICT (content): Merge conflict in pkg/provider/azure_controller_vmss_test.go
Auto-merging pkg/provider/azure_controller_vmss.go
CONFLICT (content): Merge conflict in pkg/provider/azure_controller_vmss.go
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config set advice.mergeConflict false"
Patch failed at 0001 chore: return early success on vmss detach disk

Details

In response to this:

/cherrypick release-1.33
/cherrypick release-1.32
/cherrypick release-1.31

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.

[landreasyan]

/cherrypick release-1.32

[landreasyan]

/cherrypick release-1.33

[k8s-infra-cherrypick-robot]

@landreasyan: new pull request created: #9818

Details

In response to this:

/cherrypick release-1.32

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.

[k8s-infra-cherrypick-robot]

@landreasyan: new pull request created: #9819

Details

In response to this:

/cherrypick release-1.33

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.

[landreasyan]

/cherrypick release-1.34

[k8s-infra-cherrypick-robot]

@landreasyan: new pull request created: #9820

Details

In response to this:

/cherrypick release-1.34

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.