mirror of
https://github.com/mulgadc/spinifex.git
synced 2026-10-20 11:21:49 +00:00
Julian Sommer
126 lines
9.8 KiB
Markdown
126 lines
9.8 KiB
Markdown
# EC2 AttachVolume
|
|
|
|
Hot-plug an EBS volume to a running instance via QMP, implementing the AWS `ec2 attach-volume` API.
|
|
|
|
## Request Flow
|
|
|
|
```
|
|
AWS SDK (attach-volume)
|
|
-> Gateway (port 9999): parse AWS query params, validate input
|
|
-> NATS ec2.cmd.{instanceId}: per-instance topic routing
|
|
-> Daemon handleEC2Events: owns the instance's QEMU process
|
|
1. Validate volume is "available" (Predastore/S3 lookup)
|
|
2. ebs.mount via NATS -> viperblock starts NBD server
|
|
3. QMP blockdev-add -> register NBD block device in QEMU
|
|
4. QMP device_add -> attach as virtio-blk-pci to guest
|
|
5. Update instance metadata + volume metadata
|
|
<- NATS response: ec2.VolumeAttachment JSON
|
|
<- Gateway: wrap in XML
|
|
<- AWS SDK: VolumeAttachment response
|
|
```
|
|
|
|
## Why Per-Instance NATS Routing
|
|
|
|
AttachVolume uses `ec2.cmd.{instanceId}` instead of a generic `ec2.AttachVolume` topic. This is because the command must reach the specific daemon process that owns the QEMU instance. Each daemon subscribes to `ec2.cmd.{instanceId}` for every instance it manages. The same pattern is used by Start, Stop, and Terminate. If no daemon owns the instance, NATS returns `ErrNoResponders`, which the gateway maps to `InvalidInstanceID.NotFound`.
|
|
|
|
Other volume operations (CreateVolume, DeleteVolume, DescribeVolumes) use generic topics like `ec2.CreateVolume` because they don't require access to a specific QEMU process.
|
|
|
|
## Files Changed
|
|
|
|
### Wire Format: `hive/qmp/qmp.go`
|
|
|
|
Added `AttachVolume bool` to `Attributes` and a new `AttachVolumeData` struct carried as an optional pointer on `Command`. The pointer-with-omitempty pattern means the field is absent from JSON for non-attach commands, keeping the wire format clean. The `Attributes` struct uses boolean flags for dispatch (matching the existing Start/Stop/Terminate pattern).
|
|
|
|
### Gateway Handler: `hive/gateway/ec2/volume/AttachVolume.go`
|
|
|
|
Validates `VolumeId` and `InstanceId` are present, constructs a `qmp.Command` with `AttachVolume: true` and `AttachVolumeData`, sends it to `ec2.cmd.{instanceId}` via NATS request-response (30s timeout). The `Device` field is optional per the AWS API spec.
|
|
|
|
NATS errors are differentiated: `ErrNoResponders` returns `InvalidInstanceID.NotFound` (no daemon owns this instance), while timeouts and connection errors return `InternalError` to avoid misleading users about non-existent instances when the real problem is infrastructure.
|
|
|
|
### Gateway Switch: `hive/gateway/ec2.go`
|
|
|
|
Added `"AttachVolume"` case to the EC2 action switch statement.
|
|
|
|
### Daemon Handler: `hive/daemon/daemon_handlers.go`
|
|
|
|
The core logic, placed as the first branch in `handleEC2Events` (before Start/Stop/Terminate). Implements a three-phase commit with reverse-order rollback:
|
|
|
|
**Phase 1 - EBS Mount:** Sends `ebs.mount` via NATS to viperblock, which starts an NBD server and returns a URI (unix socket or TCP). If this fails, no cleanup is needed since nothing was mounted.
|
|
|
|
**Phase 2 - QMP blockdev-add:** Registers the NBD block device in QEMU. Uses `nbd-{volumeId}` as the node-name for predictable identification during future detach. The `export` field is set to `""` per viperblock convention (default export). If this fails, Phase 1 is rolled back via `ebs.unmount`.
|
|
|
|
**Phase 3 - QMP device_add:** Attaches the block device to the guest as `virtio-blk-pci` with id `vdisk-{volumeId}`. If this fails, Phase 2 is rolled back via `blockdev-del`, then Phase 1 via `ebs.unmount`. The `blockdev-del` rollback error is checked: if it fails, the EBS unmount is skipped to avoid tearing down storage that QEMU still references (which could crash the VM).
|
|
|
|
After all three phases succeed, the handler updates in-memory state (`EBSRequests`, `BlockDeviceMappings`), persists to Predastore (volume metadata) and JetStream (instance state), then responds with `ec2.VolumeAttachment` JSON.
|
|
|
|
### Volume Service: `hive/handlers/ec2/volume/service_impl.go`
|
|
|
|
`getVolumeConfig` was renamed to `GetVolumeConfig` (exported directly instead of using a wrapper) so the daemon can validate that a volume exists and check its state before attempting attachment.
|
|
|
|
`UpdateVolumeState` was extended with a `deviceName` parameter rather than creating a separate `UpdateVolumeAttachment` method. The existing callers pass `""` for deviceName since boot volumes don't track device names this way. This avoids near-duplicate methods.
|
|
|
|
### Volume Service Interface: `hive/handlers/ec2/volume/service.go`
|
|
|
|
The `VolumeService` interface is kept to AWS-facing operations only (CreateVolume, DescribeVolumes, ModifyVolume, DeleteVolume). `GetVolumeConfig` and `UpdateVolumeState` are methods on the concrete `VolumeServiceImpl` type, not the interface, because they are internal operations used by the daemon rather than AWS API endpoints. The daemon holds a `*VolumeServiceImpl` directly so it can call these methods without polluting the interface that the gateway-side `NATSVolumeService` also implements.
|
|
|
|
### Device Name Assignment: `hive/daemon/daemon.go`
|
|
|
|
`nextAvailableDevice` scans both `EBSRequests` and `BlockDeviceMappings` to find the next available `/dev/sd[f-p]` slot. Both collections are checked because there's a window during attachment where a volume appears in `EBSRequests` but not yet in `BlockDeviceMappings`. Returns `""` when all 11 slots are exhausted, which the handler maps to `AttachmentLimitExceeded`.
|
|
|
|
The `/dev/sd[f-p]` range follows the AWS convention. The guest OS sees these as `/dev/vd*` when using virtio drivers, but the metadata device name is the `/dev/sd*` form per AWS API compatibility.
|
|
|
|
### Rollback Helper: `hive/daemon/daemon.go`
|
|
|
|
`rollbackEBSMount` sends `ebs.unmount` via NATS and verifies the response: checks both the `Error` field and the `Mounted` boolean. Rollback failures are logged at Error level but not propagated to callers since rollback is best-effort cleanup during an already-failed operation. This is a deliberate tradeoff: propagating rollback errors would complicate the handler without changing the user-visible outcome (the attach already failed).
|
|
|
|
### Config: `hive/config/config.go`
|
|
|
|
Added `DeviceName` field to `EBSRequest`. This tracks which `/dev/sd*` device name is assigned to a hot-plugged volume. Boot-time volumes don't use this field since their device assignment is handled by QEMU command-line args.
|
|
|
|
### NBD URI Parsing: `hive/utils/utils.go`
|
|
|
|
`ParseNBDURI` is the inverse of the existing `FormatNBDSocketURI`/`FormatNBDTCPURI` functions. It extracts the components needed for QMP's `blockdev-add` server argument:
|
|
- `nbd:unix:/path/to/socket.sock` -> `type=unix, path=/path/to/socket.sock`
|
|
- `nbd://host:port` -> `type=inet, host=host, port=port`
|
|
|
|
## Design Decisions
|
|
|
|
**Why QMP hot-plug instead of restarting the VM?** Hot-plug via `blockdev-add` + `device_add` attaches the volume without downtime. This matches AWS behavior where `attach-volume` works on running instances immediately.
|
|
|
|
**Why `nbd-{volumeId}` and `vdisk-{volumeId}` naming?** These are predictable identifiers derived from the volume ID. This makes future `detach-volume` straightforward: the daemon can construct the node-name and device-id from the volume ID without maintaining a separate mapping.
|
|
|
|
**Why `export: ""`?** Viperblock's NBD server uses a blank export name as the default. This is a viperblock convention, not a universal NBD convention.
|
|
|
|
**Why rollback skips EBS unmount when blockdev-del fails?** If `blockdev-del` fails, QEMU still has a reference to the NBD backend. Unmounting the EBS volume would tear down the NBD server while QEMU still tries to do I/O through it, potentially crashing or hanging the VM. Leaving the mount in place is the safer failure mode: the volume is leaked but the VM stays healthy.
|
|
|
|
**Why `DeleteOnTermination` defaults to false?** This matches AWS behavior: hot-attached volumes default to `DeleteOnTermination: false`. Boot volumes set this to true. Users can change it via `modify-instance-attribute`.
|
|
|
|
## Error Codes
|
|
|
|
| Condition | AWS Error Code |
|
|
|-----------|---------------|
|
|
| Missing VolumeId/InstanceId | `InvalidParameterValue` |
|
|
| Volume not found in Predastore | `InvalidVolume.NotFound` |
|
|
| Volume already in-use | `VolumeInUse` |
|
|
| Instance not found (no NATS responder) | `InvalidInstanceID.NotFound` |
|
|
| Instance not running | `IncorrectInstanceState` |
|
|
| All device slots full | `AttachmentLimitExceeded` |
|
|
| Internal failures (mount, QMP, etc.) | `InternalError` |
|
|
|
|
## Testing
|
|
|
|
**Gateway validation** (`AttachVolume_test.go`): Table-driven tests for nil input, empty input, missing fields, empty strings, valid inputs with/without device. Follows the pattern from `CreateVolume_test.go`.
|
|
|
|
**Device name assignment** (`daemon_test.go`): 6 scenarios covering empty instance, existing block device mappings, existing EBS requests, all devices used, nil instance, mixed sources.
|
|
|
|
**Handler error paths** (`daemon_test.go`): Tests for missing attach volume data, instance not running, and volume not found. Uses a real embedded NATS server (not mocks). The happy path is not currently testable because `volumeService` is a concrete type — testing the full flow would require introducing interface-based dependency injection for the volume service and QMP client.
|
|
|
|
**NBD URI parsing** (`utils_test.go`): 8 cases covering both formats, empty paths, missing ports, invalid ports, unsupported formats.
|
|
|
|
## Known Limitations
|
|
|
|
- No validation of user-supplied device names against the `/dev/sd[f-p]` range or conflict checking.
|
|
- TOCTOU gap between checking instance status/volume state and performing the actual QMP operations. A concurrent stop could race with an attach.
|
|
- The `Attributes` struct uses boolean flags for mutually exclusive operations. A string-based action field would be structurally safer but is a breaking wire-format change.
|
|
- Volume metadata and JetStream state persistence failures after a successful attach are logged but do not cause the response to indicate failure, since the volume is already physically attached to the VM.
|