fix(volumebind): create a missing read-write sub_path at the bind

FORK.md

@@ -103,6 +103,7 @@ Everything on `giantswarm` that is not in the pin (`git log v0.4.0-alpha1..giant
 | The pause image defaults to its gsoci copy: `images.pause` = `gsoci.azurecr.io/giantswarm/pause:3.10.2@sha256:f548e0e8…`, upstream's digest | every Giant Swarm installation pulls from gsoci.azurecr.io, and one whose egress admits only that registry cannot reach `registry.k8s.io`; the same digest, so the snapshots that record it restore unchanged ([#224](https://github.com/giantswarm/substrate/issues/224)) | `dd957c2e` ([#225](https://github.com/giantswarm/substrate/pull/225), rebase-merged 2026-10-09; first release 1.6.3) | **ours to keep**: a Giant Swarm registry is not upstream's default; the upstream-shaped change is the row above |
 | `resources.DeepEqual`, the unchanged-value check of the generated declarative validation, compares a slice of proto messages (a repeated message field) element by element with `proto.Equal`, a nil and an empty one being equal | it handed the slice to `reflect.DeepEqual`, which also compares each message's internal state; the RPC logger's marshal fills the size cache, so a repeated message field and its clone differed and an `+k8s:immutable` repeated field (`Actor.existing_volumes`, row below) failed every `CreateActor` with "field is immutable" | [#234](https://github.com/giantswarm/substrate/pull/234) (`fix(resources): compare repeated message fields with proto.Equal`) | none: upstream `main` has the same `DeepEqual`; to file, queued in the upstream engagement list |
 | An existing volume mounted per actor at a sub-path: an ActorTemplate declares it (`Volume.existing_volume`, field 10001, an empty marker), `CreateActor` supplies it (`Actor.existing_volumes`, field 10001, immutable; `ExistingVolume` = name, CSI driver, volume handle, access mode `READ_WRITE_MANY`/`READ_ONLY_MANY` in the shape of upstream's `VolumeAccessMode`, and the `sub_path` the actor sees as the volume's root), and `VolumeMount` gains `sub_path` and `read_only` (fields 10001, 10002), so one volume mounts at several paths. `CreateActor` refuses a reference to a volume the template does not declare as existing, a driver without a `CSIDriverConfig`, a handle no PersistentVolume of the driver holds and an access mode the PersistentVolume does not permit (ate-api-server reads PersistentVolumes: a new ClusterRole rule); the PersistentVolume's `volumeAttributes` reach the driver at resume. An unsupplied existing volume contributes neither itself nor its mounts. Substrate never creates, deletes or detaches one: pause, resume and delete only unmount and mount it; a multi-node volume is staged per target, and atelet binds each sub-path from a descriptor opened beneath the volume's root without following symbolic links (`openat2` `RESOLVE_BENEATH|RESOLVE_NO_SYMLINKS`), so a missing directory or a planted link fails the mount. An actor with existing volumes boots from its image instead of the golden snapshot and cannot be cloned from a tag. Tests: unit (validation, workload spec, CSI staging, the sub-path resolver), the `existingvolumes` e2e on kind with the CSI NFS driver (`E2E_CSI_NFS=1` in pr-workflow) | Sessions on one workspace each need their own directory of one read-write-many volume read-write and its git mirrors read-only, hundreds a day; a per-actor volume or snapshot per Session does not scale, and an external volume was created per actor, attached single-node-writer and deleted with it, with no sub-path or read-only mount | [#234](https://github.com/giantswarm/substrate/pull/234) (`feat: mount an existing volume per actor at a sub-path`) | follows agent-substrate/substrate#1637 (shared volumes across actors, open), in the shape proposed there (https://github.com/agent-substrate/substrate/issues/1637#issuecomment-6083442678), on top of #1988's access modes (open); upstream puts it behind the Preview gate of #1994 (open), which this line does not carry, so it is ungated here. [#37742](https://github.com/giantswarm/giantswarm/issues/37742) row 157. Exit: drops at the re-pin that carries #1637's API |
+| A read-write mount's missing `sub_path` directory is created at the bind: ateom's `volumebind.Prepare` makes the last component with `mkdirat` beneath the parent opened like the bind resolves a sub-path (`RESOLVE_BENEATH\|RESOLVE_NO_SYMLINKS\|RESOLVE_NO_XDEV`), owned by the user the mounting container runs as (read from its OCI bundle) and `0770` whatever the umask, every creation before any bind; a missing parent, a symbolic link in the path, a file under the name, a read-only mount, a `READ_ONLY_MANY` volume and a read-only file system keep failing with the reason. `MountDir` moves to `ocispec`. Existing-volume declarations and their mounts are exempt from the unchanged-volumes check of a template repoint, like `system_info` volumes. Tests: `TestCreateVolumeDir`, `TestProcessUser`, `TestValidateTemplateVolumesUnchanged`, `TestUpdateActor_RepointTemplate`; the `existingvolumes` e2e leaves one session directory for Substrate to create and checks its mode | a caller that assigns the directory name inside its own create call (kagent's `CreateSession` picks the Session id and mounts `sessions/<id>/` of the workspace volume) cannot make the directory before the actor exists, so every Session with a volume source failed at its first resume; a root-owned directory stays unwritable for a harness that runs as its image's `USER`; adding an existing-volume slot to a template made every actor's repoint `FailedPrecondition` ([#238](https://github.com/giantswarm/substrate/issues/238)) | [#239](https://github.com/giantswarm/substrate/pull/239) (`fix(volumebind): create a missing read-write sub_path at the bind`) | amends the row above's patch and follows agent-substrate/substrate#1637 (open) in its shape. [#37742](https://github.com/giantswarm/giantswarm/issues/37742) row 163. Exit: drops with the row above at the re-pin that carries #1637's API |
 
 Twenty-three patches change Substrate ahead of upstream — egress for an actor while it resumes, without which no
 skill-carrying agent of the platform boots, the atelet scheduling knobs, the keep policy on the CRD chart's

cmd/ateapi/internal/controlapi/actor.go

@@ -317,9 +317,9 @@ func (s *ServiceImpl) UpdateActor(ctx context.Context, actorRef resources.ActorR
 		// The repointed ref must also resolve, mirroring CreateActor's
 		// check (same non-atomicity caveat; resume re-resolves and fails
 		// cleanly), and the replacement's sandbox config and its volumes
-		// and volume mounts other than system_info must match the old
-		// template's. It must also store snapshots under the location the
-		// actor's own already live in.
+		// and volume mounts other than system_info and existing volumes
+		// must match the old template's. It must also store snapshots under
+		// the location the actor's own already live in.
 		if !proto.Equal(oldVal.GetActorTemplate(), newVal.GetActorTemplate()) {
 			if state := oldVal.GetStatus().GetState(); state != ateapipb.ActorState_ACTOR_STATE_SUSPENDED {
 				return status.Errorf(codes.FailedPrecondition,
@@ -389,15 +389,18 @@ func (s *ServiceImpl) UpdateActor(ctx context.Context, actorRef resources.ActorR
 // containers added or removed by the new template are unconstrained.
 // SystemInfo volumes and their mounts are exempt: atelet regenerates their
 // content on every Run/Restore and a snapshot holds none of it, so the new
-// template may add, remove or change them.
+// template may add, remove or change them. Existing volumes and their mounts
+// are exempt for the same reason: their data lives outside Substrate, the
+// actor supplies them at create or not at all, and an actor that supplies no
+// reference to one the new template declares gets no mount of it.
 func validateTemplateVolumesUnchanged(oldTemplate, newTemplate *ateapipb.ActorTemplate) error {
-	oldVolumes, oldSystemInfo := splitSnapshotVolumes(oldTemplate)
-	newVolumes, newSystemInfo := splitSnapshotVolumes(newTemplate)
+	oldVolumes, oldExempt := splitSnapshotVolumes(oldTemplate)
+	newVolumes, newExempt := splitSnapshotVolumes(newTemplate)
 	if !slices.EqualFunc(oldVolumes, newVolumes, func(a, b *ateapipb.Volume) bool {
 		return proto.Equal(a, b)
 	}) {
 		return status.Error(codes.FailedPrecondition,
-			"volumes differ between the current and the new actor template; volumes other than system_info must be identical to repoint an actor")
+			"volumes differ between the current and the new actor template; volumes other than system_info and existing volumes must be identical to repoint an actor")
 	}
 
 	newContainers := make(map[string]*ateapipb.Container, len(newTemplate.GetContainers()))
@@ -409,11 +412,11 @@ func validateTemplateVolumesUnchanged(oldTemplate, newTemplate *ateapipb.ActorTe
 		if !ok {
 			continue
 		}
-		if !slices.EqualFunc(snapshotVolumeMounts(oldC, oldSystemInfo), snapshotVolumeMounts(newC, newSystemInfo), func(a, b *ateapipb.VolumeMount) bool {
+		if !slices.EqualFunc(snapshotVolumeMounts(oldC, oldExempt), snapshotVolumeMounts(newC, newExempt), func(a, b *ateapipb.VolumeMount) bool {
 			return proto.Equal(a, b)
 		}) {
 			return status.Errorf(codes.FailedPrecondition,
-				"volume mounts of container %q differ between the current and the new actor template; mounts of volumes other than system_info must be identical to repoint an actor", oldC.GetName())
+				"volume mounts of container %q differ between the current and the new actor template; mounts of volumes other than system_info and existing volumes must be identical to repoint an actor", oldC.GetName())
 		}
 	}
 	return nil
@@ -453,27 +456,28 @@ func validateSnapshotLocationUnchanged(actor *ateapipb.Actor, newTemplate *ateap
 	return nil
 }
 
-// splitSnapshotVolumes returns the template's volumes other than SystemInfo
-// ones, in order, and the names of its SystemInfo volumes.
+// splitSnapshotVolumes returns the template's volumes a snapshot carries data
+// for, in order, and the names of the others: its SystemInfo and existing
+// volumes.
 func splitSnapshotVolumes(tmpl *ateapipb.ActorTemplate) ([]*ateapipb.Volume, map[string]bool) {
 	var volumes []*ateapipb.Volume
-	systemInfo := map[string]bool{}
+	exempt := map[string]bool{}
 	for _, v := range tmpl.GetVolumes() {
-		if v.GetSystemInfo() != nil {
-			systemInfo[v.GetName()] = true
+		if v.GetSystemInfo() != nil || v.GetExistingVolume() != nil {
+			exempt[v.GetName()] = true
 			continue
 		}
 		volumes = append(volumes, v)
 	}
-	return volumes, systemInfo
+	return volumes, exempt
 }
 
 // snapshotVolumeMounts returns the container's mounts, in order, without the
-// mounts of the named SystemInfo volumes.
-func snapshotVolumeMounts(c *ateapipb.Container, systemInfo map[string]bool) []*ateapipb.VolumeMount {
+// mounts of the named exempt volumes.
+func snapshotVolumeMounts(c *ateapipb.Container, exempt map[string]bool) []*ateapipb.VolumeMount {
 	var mounts []*ateapipb.VolumeMount
 	for _, m := range c.GetVolumeMounts() {
-		if !systemInfo[m.GetName()] {
+		if !exempt[m.GetName()] {
 			mounts = append(mounts, m)
 		}
 	}

cmd/ateapi/internal/controlapi/actor_test.go

@@ -242,7 +242,8 @@ func TestUpdateActor(t *testing.T) {
 // update may point a suspended actor at a different template (it takes effect
 // on the next ResumeActor), but the actor must be suspended, the new ref must
 // resolve, and the replacement's sandbox config and its volumes and volume
-// mounts other than system_info must match the old template's.
+// mounts other than system_info and existing volumes must match the old
+// template's.
 func TestUpdateActor_RepointTemplate(t *testing.T) {
 	ctx := context.Background()
 	persistence, cleanup := storetest.SetupTestStore(t)
@@ -302,6 +303,28 @@ func TestUpdateActor_RepointTemplate(t *testing.T) {
 	}); err != nil {
 		t.Fatalf("creating template tmpl-f: %v", err)
 	}
+	// tmpl-g adds an existing volume and its mount to tmpl-f's layout, which a
+	// repoint may do too: the actor supplies such a volume at create or not at
+	// all, and no snapshot carries its content.
+	if _, err := persistence.CreateActorTemplate(ctx, &ateapipb.ActorTemplate{
+		Metadata: &ateapipb.ResourceMetadata{Atespace: testAtespace, Name: "tmpl-g"},
+		Containers: []*ateapipb.Container{{
+			Name:  "main",
+			Image: "example.com/app:v1",
+			VolumeMounts: []*ateapipb.VolumeMount{
+				{Name: "data", MountPath: "/data"},
+				{Name: "egress-trust", MountPath: "/run/kagent/egress"},
+				{Name: "workspace", MountPath: "/workspace"},
+			},
+		}},
+		Volumes: []*ateapipb.Volume{dataVolume, {Name: "egress-trust", SystemInfo: &ateapipb.SystemInfoVolumeSource{
+			DataSources: []*ateapipb.SystemInfoDataSource{{TrustBundle: &ateapipb.TrustBundleDataSource{Names: []string{"egress"}, Path: "ca.crt"}}},
+		}}, {Name: "workspace", ExistingVolume: &ateapipb.ExistingVolumeSource{}}},
+		SnapshotConfig: &ateapipb.SnapshotConfig{StorageLocation: "gs://my-bucket/snapshots"},
+		SandboxConfig:  gvisorConfig,
+	}); err != nil {
+		t.Fatalf("creating template tmpl-g: %v", err)
+	}
 
 	created := storetest.MustCreateActor(t, ctx, persistence, &ateapipb.Actor{
 		Metadata:      &ateapipb.ResourceMetadata{Atespace: testAtespace, Name: testActorID},
@@ -372,6 +395,19 @@ func TestUpdateActor_RepointTemplate(t *testing.T) {
 		t.Errorf("updated actor_template.name = %q, want %q", got, want)
 	}
 
+	// Repointing at a template that only adds an existing volume and its
+	// mount succeeds.
+	updated, err = svc.UpdateActor(ctx, &ateapipb.UpdateActorRequest{Actor: &ateapipb.Actor{
+		Metadata:      updated.GetMetadata(),
+		ActorTemplate: &ateapipb.ObjectRef{Atespace: testAtespace, Name: "tmpl-g"},
+	}})
+	if err != nil {
+		t.Fatalf("UpdateActor to a template adding an existing volume failed: %v", err)
+	}
+	if got, want := updated.GetActorTemplate().GetName(), "tmpl-g"; got != want {
+		t.Errorf("updated actor_template.name = %q, want %q", got, want)
+	}
+
 	// When the old template no longer exists there is nothing left to
 	// compare the sandbox config or volume layout against, so the repoint
 	// only requires the new ref to resolve.
@@ -586,6 +622,9 @@ func TestValidateTemplateVolumesUnchanged(t *testing.T) {
 	}
 	trustMount := &ateapipb.VolumeMount{Name: "egress-trust", MountPath: "/run/kagent/egress"}
 	withTrust := []*ateapipb.Volume{dataVolume, trustBundleVolume("egress")}
+	workspaceVolume := &ateapipb.Volume{Name: "workspace", ExistingVolume: &ateapipb.ExistingVolumeSource{}}
+	workspaceMount := &ateapipb.VolumeMount{Name: "workspace", MountPath: "/workspace"}
+	withWorkspace := []*ateapipb.Volume{dataVolume, workspaceVolume}
 
 	tests := []struct {
 		name             string
@@ -683,6 +722,33 @@ func TestValidateTemplateVolumesUnchanged(t *testing.T) {
 		oldTmpl: template(oneVolume, container("main", dataMount)),
 		newTmpl: template([]*ateapipb.Volume{{Name: "data", SystemInfo: &ateapipb.SystemInfoVolumeSource{}}}, container("main", dataMount)),
 		wantErr: true,
+	}, {
+		name:    "existing volume and mount added",
+		oldTmpl: template(oneVolume, container("main", dataMount)),
+		newTmpl: template(withWorkspace, container("main", dataMount, workspaceMount)),
+	}, {
+		name:    "existing volume and mount removed",
+		oldTmpl: template(withWorkspace, container("main", workspaceMount, dataMount)),
+		newTmpl: template(oneVolume, container("main", dataMount)),
+	}, {
+		name:    "existing volume mount path and sub_path changed",
+		oldTmpl: template(withWorkspace, container("main", dataMount, workspaceMount)),
+		newTmpl: template(withWorkspace, container("main", dataMount, &ateapipb.VolumeMount{Name: "workspace", MountPath: "/work", SubPath: "src", ReadOnly: true})),
+	}, {
+		name:    "durable volume added beside an existing volume",
+		oldTmpl: template(oneVolume, container("main", dataMount)),
+		newTmpl: template([]*ateapipb.Volume{dataVolume, workspaceVolume, scratchVolume}, container("main", dataMount, workspaceMount)),
+		wantErr: true,
+	}, {
+		name:    "durable mount path changed beside an existing volume",
+		oldTmpl: template(oneVolume, container("main", dataMount)),
+		newTmpl: template(withWorkspace, container("main", &ateapipb.VolumeMount{Name: "data", MountPath: "/mnt/data"}, workspaceMount)),
+		wantErr: true,
+	}, {
+		name:    "durable volume replaced by an existing volume of the same name",
+		oldTmpl: template(oneVolume, container("main", dataMount)),
+		newTmpl: template([]*ateapipb.Volume{{Name: "data", ExistingVolume: &ateapipb.ExistingVolumeSource{}}}, container("main", dataMount)),
+		wantErr: true,
 	}}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {

cmd/ateom-gvisor/main.go

@@ -586,7 +586,7 @@ func (s *AteomService) RunWorkload(ctx context.Context, req *ateompb.RunWorkload
 	}
 	// Bind the directories of external volumes the mounts name a sub-path of
 	// or mount read-only, here for the same reason as the rootfs below.
-	if err := volumebind.Prepare(req.GetActorDirs().GetVolumesDir(), req.GetSpec().GetContainers()); err != nil {
+	if err := volumebind.Prepare(req.GetActorDirs().GetVolumesDir(), req.GetActorDirs().GetOciBundleDir(), req.GetSpec().GetContainers()); err != nil {
 		return nil, fmt.Errorf("while binding volume directories: %w", err)
 	}
 	// Create and start pause container. The bundle rootfs is composed here —
@@ -896,7 +896,7 @@ func (s *AteomService) RestoreWorkload(ctx context.Context, req *ateompb.Restore
 			return nil, fmt.Errorf("while restoring durable-dir volumes: %w", err)
 		}
 	}
-	if err := volumebind.Prepare(req.GetActorDirs().GetVolumesDir(), req.GetSpec().GetContainers()); err != nil {
+	if err := volumebind.Prepare(req.GetActorDirs().GetVolumesDir(), req.GetActorDirs().GetOciBundleDir(), req.GetSpec().GetContainers()); err != nil {
 		return nil, fmt.Errorf("while binding volume directories: %w", err)
 	}
 	// Compose the pause rootfs before create (see RunWorkload). runsc restore

cmd/ateom-microvm/run.go

@@ -700,7 +700,7 @@ func (s *AteomService) stageMergedRootfs(ctx context.Context, rr resolvedRuntime
 	if hasCsiVolumes(containers) {
 		// The directories mounts name a sub-path of, or mount read-only, are
 		// bound beside their volumes first, so the share carries them too.
-		if err := volumebind.Prepare(actorDirs.GetVolumesDir(), containers); err != nil {
+		if err := volumebind.Prepare(actorDirs.GetVolumesDir(), actorDirs.GetOciBundleDir(), containers); err != nil {
 			return nil, fmt.Errorf("while binding volume directories: %w", err)
 		}
 		if err := s.stageCsiVolumes(ctx, id, actorDirs.GetVolumesDir()); err != nil {

docs/csi-volumes.md

@@ -134,7 +134,7 @@ existingVolumes:
 ```
 
 * An existing volume's `subPath` is the directory this actor sees as the volume's root; a mount's own `subPath` is relative to it. So one template serves every actor, each in a directory of its own. One volume may be mounted at several paths.
-* The directory must exist on the volume: Substrate never creates one. It is resolved beneath the volume's root without following any symbolic link, so a link that another actor wrote on the volume fails the mount rather than redirecting it.
+* The directory is resolved beneath the volume's root without following any symbolic link, so a link that another actor wrote on the volume fails the mount rather than redirecting it. A read-write mount's directory is created when its last component is missing, mode `0770` and owned by the user the mounting container runs as, so a caller may name a directory of its own for the actor in the create request; its parent must exist. A read-only mount's directory must exist, and so must every directory of a `READ_ONLY_MANY` volume.
 * `readOnly` mounts read-only, and so does every mount of a `READ_ONLY_MANY` volume.
 * `CreateActor` refuses a reference that names no existing volume of the template, a driver without a `CSIDriverConfig`, a handle that no PersistentVolume of the driver holds, and an access mode the PersistentVolume does not permit. The PersistentVolume's `spec.csi.volumeAttributes` are passed to the driver when the volume is mounted, so it must still exist when the actor resumes.
 * An existing volume of the template that the actor does not supply contributes neither the volume nor its mounts.