Skip to content

Commit 06de65e

Browse files
sjmiller609claude
andcommitted
Add access_mode field to attach volume request
Add an explicit AccessMode enum (ReadWriteOnce, ReadOnlyMany, ReadWriteMany) to the attach volume request. This replaces the implicit rw+rw=NFS behavior with an explicit opt-in model: - ReadWriteOnce: exclusive rw via block device (default, no change) - ReadOnlyMany: read-only, multiple instances (maps from readonly=true) - ReadWriteMany: shared rw via NFS (only mode that triggers NFS) The existing readonly field is deprecated but still works with identical semantics. Neither legacy path triggers NFS. When both fields are set, access_mode takes precedence. Validation rules enforce that different access modes cannot be mixed on the same volume (e.g., RWO + RWX = conflict). Changes: OpenAPI spec (AccessMode enum, deprecated readonly), regenerated oapi.go, domain types, volume manager attach logic, storage persistence, and 9 new tests (36 total pass). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 18e42bf commit 06de65e

File tree

6 files changed

+677
-330
lines changed

6 files changed

+677
-330
lines changed

lib/oapi/oapi.go

Lines changed: 288 additions & 267 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

lib/volumes/manager.go

Lines changed: 76 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -25,12 +25,11 @@ type Manager interface {
2525
DeleteVolume(ctx context.Context, id string) error
2626

2727
// Attachment operations (called by instance manager)
28-
// Multi-attach rules (dynamic based on current state):
29-
// - If no attachments: allow any mode (rw or ro)
30-
// - If existing attachments are ro: only allow new ro attachments
31-
// - Multiple rw attachments (ReadWriteMany): internally backed by NFS,
32-
// transparent to the caller. NFS is set up automatically when a second
33-
// rw attachment is requested.
28+
// Access mode rules:
29+
// - ReadWriteOnce: exclusive rw via block device (reject if already attached rw)
30+
// - ReadOnlyMany: read-only via block device (multiple ro attaches allowed)
31+
// - ReadWriteMany: shared rw via NFS (requires network, NFS set up automatically)
32+
// Legacy: if access_mode is unset, readonly field maps to ReadOnlyMany/ReadWriteOnce.
3433
AttachVolume(ctx context.Context, id string, req AttachVolumeRequest) error
3534
DetachVolume(ctx context.Context, volumeID string, instanceID string) error
3635

@@ -399,13 +398,13 @@ func (m *manager) DeleteVolume(ctx context.Context, id string) error {
399398
}
400399

401400
// AttachVolume marks a volume as attached to an instance.
402-
// Multi-attach rules (dynamic based on current state):
403-
// - If no attachments: allow any mode (rw or ro) via block device
404-
// - If existing attachments are all ro: only allow new ro attachments
405-
// - If existing attachment is rw (block device) and new is rw: enable NFS
406-
// for ReadWriteMany. The volume is loop-mounted on the host and exported
407-
// via NFS. The new attachment (and any subsequent ones) use NFS.
408-
// - If volume is already NFS-served: additional rw attachments use NFS
401+
// Access mode rules:
402+
// - ReadWriteOnce: exclusive rw via block device. Rejects if already attached rw.
403+
// - ReadOnlyMany: read-only via block device. Multiple ro attaches allowed.
404+
// - ReadWriteMany: shared rw via NFS. Requires NFS host (network enabled).
405+
//
406+
// Legacy readonly field: readonly=true → ReadOnlyMany, readonly=false → ReadWriteOnce.
407+
// Neither legacy path triggers NFS. Only explicit ReadWriteMany uses NFS.
409408
func (m *manager) AttachVolume(ctx context.Context, id string, req AttachVolumeRequest) error {
410409
lock := m.getVolumeLock(id)
411410
lock.Lock()
@@ -423,61 +422,88 @@ func (m *manager) AttachVolume(ctx context.Context, id string, req AttachVolumeR
423422
}
424423
}
425424

425+
mode := req.ResolveAccessMode()
426+
427+
// Log warning if both fields are set (access_mode wins)
428+
if req.AccessMode != "" && req.Readonly {
429+
fmt.Fprintf(os.Stderr, "warning: both access_mode and readonly set on attach for volume %s; access_mode takes precedence\n", id)
430+
}
431+
432+
// Classify existing attachments
433+
hasRW := false // any non-readonly block device attachment
434+
hasRO := false // any readonly attachment
435+
hasRWX := false // any ReadWriteMany (NFS) attachment
436+
for _, att := range meta.Attachments {
437+
if att.NFS {
438+
hasRWX = true
439+
} else if att.Readonly {
440+
hasRO = true
441+
} else {
442+
hasRW = true
443+
}
444+
}
445+
426446
useNFS := false
447+
readonly := false
427448

428-
if len(meta.Attachments) > 0 {
429-
hasRW := false
430-
allRO := true
431-
for _, att := range meta.Attachments {
432-
if !att.Readonly {
433-
hasRW = true
434-
allRO = false
435-
}
449+
switch mode {
450+
case AccessReadWriteOnce:
451+
// Exclusive rw via block device. Reject conflicts.
452+
if hasRW {
453+
return fmt.Errorf("cannot attach ReadWriteOnce: volume has existing read-write attachment")
454+
}
455+
if hasRO {
456+
return fmt.Errorf("cannot attach ReadWriteOnce: volume has existing read-only attachments")
457+
}
458+
if hasRWX {
459+
return fmt.Errorf("cannot attach ReadWriteOnce: volume has existing ReadWriteMany attachments")
436460
}
437461

438-
if allRO && !req.Readonly {
439-
// Existing attachments are all ro, new is rw → conflict
440-
return fmt.Errorf("cannot attach read-write: volume has existing read-only attachments")
462+
case AccessReadOnlyMany:
463+
// Read-only via block device. Reject if rw or rwx exists.
464+
if hasRW {
465+
return fmt.Errorf("cannot attach ReadOnlyMany: volume has existing read-write attachment")
441466
}
467+
if hasRWX {
468+
return fmt.Errorf("cannot attach ReadOnlyMany: volume has existing ReadWriteMany attachments")
469+
}
470+
readonly = true
442471

443-
if hasRW && req.Readonly {
444-
// Existing has rw, new is ro → conflict (rw is exclusive or NFS-only)
445-
return fmt.Errorf("cannot attach read-only: volume has existing read-write attachment")
472+
case AccessReadWriteMany:
473+
// Shared rw via NFS. Reject if non-NFS rw or ro exists.
474+
if hasRW {
475+
return fmt.Errorf("cannot attach ReadWriteMany: volume has existing ReadWriteOnce attachment")
476+
}
477+
if hasRO {
478+
return fmt.Errorf("cannot attach ReadWriteMany: volume has existing ReadOnlyMany attachments")
479+
}
480+
if m.nfsHost == "" {
481+
return fmt.Errorf("cannot attach ReadWriteMany: NFS host not configured (networking required)")
446482
}
447483

448-
if hasRW && !req.Readonly {
449-
// ReadWriteMany scenario: both existing and new want rw.
450-
// Transparently enable NFS serving.
451-
if m.nfsHost == "" {
452-
return fmt.Errorf("cannot attach read-write to multiple instances: NFS host not configured (networking required)")
484+
// Start NFS serving if not already active
485+
if meta.NFS == nil {
486+
exportPath, err := m.nfs.startServing(id)
487+
if err != nil {
488+
return fmt.Errorf("start nfs serving for ReadWriteMany: %w", err)
453489
}
454-
455-
// Start NFS serving if not already active
456-
if meta.NFS == nil {
457-
exportPath, err := m.nfs.startServing(id)
458-
if err != nil {
459-
return fmt.Errorf("start nfs serving for ReadWriteMany: %w", err)
460-
}
461-
meta.NFS = &storedNFSInfo{
462-
Host: m.nfsHost,
463-
ExportPath: exportPath,
464-
}
490+
meta.NFS = &storedNFSInfo{
491+
Host: m.nfsHost,
492+
ExportPath: exportPath,
465493
}
466-
useNFS = true
467494
}
468-
}
469-
470-
// If volume is already NFS-served, new rw attachments use NFS
471-
if meta.NFS != nil && !req.Readonly {
472495
useNFS = true
496+
497+
default:
498+
return fmt.Errorf("unsupported access mode: %s", mode)
473499
}
474500

475-
// Add new attachment
476501
meta.Attachments = append(meta.Attachments, storedAttachment{
477502
InstanceID: req.InstanceID,
478503
MountPath: req.MountPath,
479-
Readonly: req.Readonly,
504+
Readonly: readonly,
480505
NFS: useNFS,
506+
AccessMode: string(mode),
481507
})
482508

483509
return saveMetadata(m.paths, meta)

0 commit comments

Comments
 (0)