feat: make storage:mount upsert the existing attachment

Re-running `storage:mount <app> <entry> --container-dir <path>` against an existing `(entry, container_dir, process_type)` tuple now updates the attachment's mount-time fields in place instead of erroring with `already mounted`. Declarative tooling that wants to change `--volume-options`, `--volume-chown`, `--phase`, `--volume-subpath`, or `--volume-readonly` on an existing mount no longer has to unmount-then-remount, which briefly dropped the volume from `storage:report` and raced against any deploy that fired in the window. The legacy `host:container[:opts]` form keeps its strict `Mount path already exists.` failure.
This commit is contained in:
Jose Diaz-Gonzalez
2026-05-30 03:23:32 -04:00
parent e9160db769
commit 3bb21b6987
5 changed files with 257 additions and 1 deletions

View File

@@ -126,6 +126,43 @@ func AddAttachment(appName string, attachment *Attachment) error {
return SaveAttachments(appName, attachments)
}
// UpsertAttachment inserts an attachment, or updates the mount-time fields
// on an existing one matching the same (entry_name, container_path,
// process_type) tuple. Returns created=true when a new attachment was
// appended, created=false when an existing one was rewritten.
//
// Mount-time fields (Phases, Subpath, Readonly, VolumeOptions, VolumeChown)
// are overwritten wholesale, not merged: passing an empty Subpath or a
// false Readonly clears any previously-set value, mirroring the operator's
// "set the flags as I just typed them" intent and the same idempotent
// contract storage:create offers for entries.
func UpsertAttachment(appName string, attachment *Attachment) (bool, error) {
if err := attachment.Validate(); err != nil {
return false, err
}
attachments, err := LoadAttachments(appName)
if err != nil {
return false, err
}
for _, existing := range attachments {
if existing.EntryName == attachment.EntryName &&
existing.ContainerPath == attachment.ContainerPath &&
existing.ProcessType == attachment.ProcessType {
existing.Phases = attachment.Phases
existing.Subpath = attachment.Subpath
existing.Readonly = attachment.Readonly
existing.VolumeOptions = attachment.VolumeOptions
existing.VolumeChown = attachment.VolumeChown
return false, SaveAttachments(appName, attachments)
}
}
attachments = append(attachments, attachment)
return true, SaveAttachments(appName, attachments)
}
// RemoveAttachment removes the attachment matching the given entry and
// optional container path. If containerPath is empty and there is more
// than one match, returns an error.

View File

@@ -217,3 +217,129 @@ func TestListAppMountEntriesPhaseFilter(t *testing.T) {
Expect(runRows).To(HaveLen(1))
Expect(runRows[0].EntryName).To(Equal("demo-run-only"))
}
func TestUpsertAttachmentInsert(t *testing.T) {
RegisterTestingT(t)
withTempLibRoot(t)
created, err := UpsertAttachment("demo", &Attachment{
EntryName: "demo-data",
ContainerPath: "/data",
Phases: []string{PhaseDeploy, PhaseRun},
ProcessType: DefaultProcessType,
VolumeOptions: "Z",
})
Expect(err).NotTo(HaveOccurred())
Expect(created).To(BeTrue())
attachments, err := LoadAttachments("demo")
Expect(err).NotTo(HaveOccurred())
Expect(attachments).To(HaveLen(1))
Expect(attachments[0].VolumeOptions).To(Equal("Z"))
}
func TestUpsertAttachmentUpdatesInPlace(t *testing.T) {
RegisterTestingT(t)
withTempLibRoot(t)
base := &Attachment{
EntryName: "demo-data",
ContainerPath: "/data",
Phases: []string{PhaseDeploy, PhaseRun},
ProcessType: DefaultProcessType,
VolumeOptions: "Z",
Subpath: "uploads",
Readonly: false,
VolumeChown: "herokuish",
}
created, err := UpsertAttachment("demo", base)
Expect(err).NotTo(HaveOccurred())
Expect(created).To(BeTrue())
// Same identity, new mount-time fields - update in place.
created, err = UpsertAttachment("demo", &Attachment{
EntryName: "demo-data",
ContainerPath: "/data",
Phases: []string{PhaseDeploy, PhaseRun},
ProcessType: DefaultProcessType,
VolumeOptions: "noexec,nosuid",
Readonly: true,
})
Expect(err).NotTo(HaveOccurred())
Expect(created).To(BeFalse())
attachments, err := LoadAttachments("demo")
Expect(err).NotTo(HaveOccurred())
Expect(attachments).To(HaveLen(1))
Expect(attachments[0].VolumeOptions).To(Equal("noexec,nosuid"))
Expect(attachments[0].Readonly).To(BeTrue())
// Wholesale rewrite: Subpath and VolumeChown cleared since the second
// call didn't supply them.
Expect(attachments[0].Subpath).To(BeEmpty())
Expect(attachments[0].VolumeChown).To(BeEmpty())
}
func TestUpsertAttachmentDistinctIdentity(t *testing.T) {
RegisterTestingT(t)
withTempLibRoot(t)
created, err := UpsertAttachment("demo", &Attachment{
EntryName: "demo-data",
ContainerPath: "/data",
Phases: []string{PhaseDeploy},
ProcessType: DefaultProcessType,
})
Expect(err).NotTo(HaveOccurred())
Expect(created).To(BeTrue())
// Different container path on the same entry is a distinct attachment.
created, err = UpsertAttachment("demo", &Attachment{
EntryName: "demo-data",
ContainerPath: "/other",
Phases: []string{PhaseDeploy},
ProcessType: DefaultProcessType,
})
Expect(err).NotTo(HaveOccurred())
Expect(created).To(BeTrue())
// Different process type on the same (entry, container_path) is also distinct.
created, err = UpsertAttachment("demo", &Attachment{
EntryName: "demo-data",
ContainerPath: "/data",
Phases: []string{PhaseDeploy},
ProcessType: "web",
})
Expect(err).NotTo(HaveOccurred())
Expect(created).To(BeTrue())
attachments, err := LoadAttachments("demo")
Expect(err).NotTo(HaveOccurred())
Expect(attachments).To(HaveLen(3))
}
func TestUpsertAttachmentValidationFailureLeavesStoreUntouched(t *testing.T) {
RegisterTestingT(t)
withTempLibRoot(t)
created, err := UpsertAttachment("demo", &Attachment{
EntryName: "demo-data",
ContainerPath: "/data",
Phases: []string{PhaseDeploy},
ProcessType: DefaultProcessType,
})
Expect(err).NotTo(HaveOccurred())
Expect(created).To(BeTrue())
// Invalid attachment (empty ContainerPath) fails validation and the
// existing list stays exactly as it was.
_, err = UpsertAttachment("demo", &Attachment{
EntryName: "demo-data",
Phases: []string{PhaseDeploy},
})
Expect(err).To(HaveOccurred())
attachments, err := LoadAttachments("demo")
Expect(err).NotTo(HaveOccurred())
Expect(attachments).To(HaveLen(1))
Expect(attachments[0].ContainerPath).To(Equal("/data"))
}

View File

@@ -193,7 +193,21 @@ func CommandMount(input CommandMountInput) error {
VolumeChown: input.VolumeChown,
}
return AddAttachment(input.AppName, attachment)
// Idempotent contract: same (entry, container_dir, process_type)
// tuple updates the mount-time fields in place rather than appending
// a duplicate attachment. Lets declarative tooling change
// --volume-options / --volume-chown / --phase / etc. without an
// unmount-then-remount dance that briefly drops the volume.
created, err := UpsertAttachment(input.AppName, attachment)
if err != nil {
return err
}
if created {
common.LogInfo1(fmt.Sprintf("Storage entry %s mounted at %s on %s", entry.Name, input.ContainerDir, input.AppName))
} else {
common.LogInfo1(fmt.Sprintf("Storage entry %s mount at %s on %s updated", entry.Name, input.ContainerDir, input.AppName))
}
return nil
}
// CommandUnmountInput captures the flags accepted by storage:unmount when