Skip to content

Add UpdateDisks SDK method for safe disk attach/detach - #387

Open
Microsoft Corporation (raghavendra-nataraj) wants to merge 2 commits into
mainfrom
raghavendra-nataraj/vm-updatedisks
Open

Add UpdateDisks SDK method for safe disk attach/detach#387
Microsoft Corporation (raghavendra-nataraj) wants to merge 2 commits into
mainfrom
raghavendra-nataraj/vm-updatedisks

Conversation

@raghavendra-nataraj

Copy link
Copy Markdown
Contributor

What

Adds a new UpdateDisks method to the VirtualMachine SDK service so callers (e.g. the azhcidisk CSI driver via mocctl) can attach/detach VM data disks through the cloud-agent UpdateDisks RPC instead of the legacy read-modify-write DiskAttach/DiskDetachCreateOrUpdate path.

Why

The existing DiskAttach/DiskDetach do Get → mutate StorageProfileCreateOrUpdate. If the VM is concurrently deleted between the Get and the CreateOrUpdate, the CreateOrUpdate resurrects the deleted VM. The new path routes through the cloud-agent UpdateDisks RPC, which resolves the VM by lookup and only ever Updates it — a missing/deleted VM returns NotFound, never a recreate.

Changes

  • services/compute/compute.go: public VirtualMachineDiskOperation enum (Unspecified/Attach/Detach) mirroring the wire enum values.
  • services/compute/virtualmachine/client.go: UpdateDisks added to the Service interface + VirtualMachineClient.UpdateDisks public wrapper.
  • services/compute/virtualmachine/wssd.go: client.UpdateDisks implementation + request/response converters, reusing the existing DataDisk converter.
  • go.mod/go.sum: bump github.com/microsoft/moc v0.43.3 → v0.43.4 (contains the UpdateDisks RPC).

DiskAttach/DiskDetach are left untouched.

Dependency chain

Validation

go build ./... clean against moc v0.43.4. (Pre-existing, unrelated: a broken placementgroup_test.go and a format-string vet in virtualmachine_test.go exist on main independent of this change.)

Wrap the cloud-agent UpdateDisks RPC (moc #444, v0.43.4) so callers can
attach/detach VM data disks without the read-modify-write CreateOrUpdate
path used by DiskAttach/DiskDetach, which can recreate a VM that was
deleted concurrently.

- compute: add public VirtualMachineDiskOperation enum (Unspecified/Attach/
  Detach) mirroring the wire enum values.
- client: add UpdateDisks to the Service interface and a VirtualMachineClient
  .UpdateDisks wrapper. A missing VM returns NotFound (no recreate); only the
  supplied disks are acted on.
- wssd: implement client.UpdateDisks with getVirtualMachineDiskRequest (reuses
  getWssdVirtualMachineStorageConfigurationDataDisk) and
  getVirtualMachineFromDiskResponse.
- revendor github.com/microsoft/moc v0.43.3 -> v0.43.4 (brings grpc 1.82.1).

DiskAttach/DiskDetach are left unchanged.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 4a902de7-e15e-494c-b590-1f9e94a5fadd
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

Reimplement the existing DiskAttach/DiskDetach to delegate to the new
UpdateDisks method instead of the read-modify-write Get -> mutate ->
CreateOrUpdate loop. UpdateDisks resolves the VM and routes through the
cloud-agent Update path (never Create), so a request that races with a
delete returns NotFound instead of resurrecting the VM. Attaching an
already-attached disk and detaching an absent disk are now idempotent
no-ops (dedup/removal handled agent-side), and the stale-version retry
loop is dropped since Update handles versioning.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 4a902de7-e15e-494c-b590-1f9e94a5fadd
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants