Replace AttachDisk/DetachDisk with a single UpdateDisks RPC - #444
Merged
Conversation
PR #441 added two dedicated cloud-agent RPCs (AttachDisk, DetachDisk) to the VirtualMachineAgent service. Since no consumer implements or calls them yet (the wssdcloudagent handler is a follow-up), collapse them into a single UpdateDisks RPC whose VirtualMachineDiskRequest carries a VirtualMachineDiskOperation (ATTACH/DETACH) selector. - Single-direction per call matches the CSI driver, which attaches/detaches exactly one disk per ControllerPublish/UnpublishVolume call. - operation field defaults to UNSPECIFIED (proto3 zero) so a caller that omits it is rejected rather than silently attaching/detaching. - Preserves the #441 intent: operate only on the passed disks, never recreate the VM (unlike Invoke/CreateOrUpdate). Regenerated moc_cloudagent_virtualmachine.pb.go with protoc 3.11.4 + protoc-gen-go v1.3.2 (matching the repo toolchain). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4a902de7-e15e-494c-b590-1f9e94a5fadd
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
Contributor
Author
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
madhanrm
approved these changes
Jul 22, 2026
rjaini
approved these changes
Jul 22, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Replace the two dedicated cloud-agent RPCs
AttachDiskandDetachDisk(added in #441) with a singleUpdateDisksRPC on theVirtualMachineAgentservice. The direction is selected by a newVirtualMachineDiskOperationenum (ATTACH/DETACH) carried onVirtualMachineDiskRequest.Why
#441 introduced
AttachDisk/DetachDiskas two RPCs, but no consumer implements or calls them yet (the wssdcloudagent handler is a follow-up), so this is a safe time to consolidate the surface:ControllerPublishVolume/ControllerUnpublishVolume(each maps to a singleDiskAttach/DiskDetach). There is no caller that needs to attach and detach in one atomic request, so a two-list "attach these + detach those" API would be unused complexity. One RPC + an operation selector covers every real caller with a smaller surface.operationdefaults toUNSPECIFIED(proto3 zero value). The handler rejectsUNSPECIFIED, so a caller that forgets to set the direction gets an error instead of a silent attach/detach.Invoke/CreateOrUpdate, which can resurrect a concurrently-deleted VM).Changes
rpc/cloudagent/compute/virtualmachine/moc_cloudagent_virtualmachine.proto: added theVirtualMachineDiskOperationenum andoperationfield; replaced theAttachDisk/DetachDiskRPCs with a singleUpdateDisks.rpc/cloudagent/compute/moc_cloudagent_virtualmachine.pb.go: regenerated with protoc 3.11.4 + protoc-gen-go v1.3.2 (matching the repo toolchain).Testing
go build ./...andgo vet ./rpc/cloudagent/compute/pass.Follow-up: microsoft/wssdcloudagent will add the
UpdateDiskshandler and bump itsgithub.com/microsoft/mocdependency to the merged commit of this PR.Relates to #441.