From 827163ba0b502c723e1421d2d1abed1366b24c03 Mon Sep 17 00:00:00 2001 From: shrutiyam-glitch Date: Fri, 11 Sep 2026 08:20:57 -0700 Subject: [PATCH 1/3] Allow mounting the same volume at multiple paths --- .../internal/controlapi/actor_template.go | 10 +- .../controlapi/actor_template_test.go | 6 +- .../controlapi/zz_generated.validation.go | 4 +- .../suites/imagevolume/imagevolume_test.go | 144 +++++++++++++++++- pkg/proto/ateapipb/ateapi.pb.go | 11 +- pkg/proto/ateapipb/ateapi.proto | 15 +- 6 files changed, 151 insertions(+), 39 deletions(-) diff --git a/cmd/ateapi/internal/controlapi/actor_template.go b/cmd/ateapi/internal/controlapi/actor_template.go index 707b2572a1..0e5df426fa 100644 --- a/cmd/ateapi/internal/controlapi/actor_template.go +++ b/cmd/ateapi/internal/controlapi/actor_template.go @@ -421,20 +421,15 @@ func actorTemplateObjectRef(actor *ateapipb.Actor) *ateapipb.ObjectRef { return &ateapipb.ObjectRef{Atespace: ref.GetAtespace(), Name: ref.GetName()} } -// ValidateCustom_Container_VolumeMounts rejects two mounts at the same path -// within one container. The list is keyed by volume name (one mount per -// volume), so path uniqueness cannot come from the list-map key +// ValidateCustom_Container_VolumeMounts rejects mounts that nest under one +// another. func ValidateCustom_Container_VolumeMounts(_ context.Context, _ operation.Operation, fldPath *field.Path, value, _ []*ateapipb.VolumeMount) field.ErrorList { var errs field.ErrorList - seen := make(map[string]bool, len(value)) for i, m := range value { path := m.GetMountPath() if path == "" { continue // required is enforced by tags } - if seen[path] { - errs = append(errs, field.Duplicate(fldPath.Index(i).Child("mount_path"), path)) - } // Nested mounts are unsupported (volumes cannot mount onto // other volumes). for j := 0; j < i; j++ { @@ -447,7 +442,6 @@ func ValidateCustom_Container_VolumeMounts(_ context.Context, _ operation.Operat fmt.Sprintf("must not nest under or over another mount (%q)", prior))) } } - seen[path] = true } return errs } diff --git a/cmd/ateapi/internal/controlapi/actor_template_test.go b/cmd/ateapi/internal/controlapi/actor_template_test.go index dabcc6fa69..9ee1ac2e75 100644 --- a/cmd/ateapi/internal/controlapi/actor_template_test.go +++ b/cmd/ateapi/internal/controlapi/actor_template_test.go @@ -567,15 +567,13 @@ func TestValidateActorTemplate(t *testing.T) { }, want: field.ErrorList{field.Duplicate(field.NewPath("containers").Index(0).Child("env").Index(1), nil)}, }, { - // One mount per volume for now; see the TODO on volume_mounts. - name: "same volume mounted twice is rejected", + name: "the same volume mounted at two paths is allowed", mutate: func(tmpl *ateapipb.ActorTemplate) { tmpl.Containers[0].VolumeMounts = []*ateapipb.VolumeMount{ {Name: "data", MountPath: "/var/data"}, {Name: "data", MountPath: "/mnt/data"}, } }, - want: field.ErrorList{field.Duplicate(field.NewPath("containers").Index(0).Child("volume_mounts").Index(1), nil)}, }, { name: "two volumes at the same path are rejected", mutate: func(tmpl *ateapipb.ActorTemplate) { @@ -584,7 +582,7 @@ func TestValidateActorTemplate(t *testing.T) { {Name: "other", MountPath: "/var/data"}, } }, - want: field.ErrorList{field.Duplicate(field.NewPath("containers").Index(0).Child("volume_mounts").Index(1).Child("mount_path"), nil)}, + want: field.ErrorList{field.Duplicate(field.NewPath("containers").Index(0).Child("volume_mounts").Index(1), nil)}, }, { name: "nested mount paths are rejected", mutate: func(tmpl *ateapipb.ActorTemplate) { diff --git a/cmd/ateapi/internal/controlapi/zz_generated.validation.go b/cmd/ateapi/internal/controlapi/zz_generated.validation.go index 3bbc3dbc9e..ad9c93274d 100644 --- a/cmd/ateapi/internal/controlapi/zz_generated.validation.go +++ b/cmd/ateapi/internal/controlapi/zz_generated.validation.go @@ -1411,12 +1411,12 @@ func Validate_Container( } // lists with map semantics require unique keys if e := validate.PtrSliceUnique(ctx, op, fldPath, obj, oldObj, - func(a *ateapipb.VolumeMount, b *ateapipb.VolumeMount) bool { return a.Name == b.Name }); len(e) != 0 { + func(a *ateapipb.VolumeMount, b *ateapipb.VolumeMount) bool { return a.MountPath == b.MountPath }); len(e) != 0 { errs = append(errs, e...) } // iterate the list and call the type's validation function if e := validate.EachPtrSliceVal(ctx, op, fldPath, obj, oldObj, - func(a *ateapipb.VolumeMount, b *ateapipb.VolumeMount) bool { return a.Name == b.Name }, ateDeepEqual, Validate_VolumeMount); len(e) != 0 { + func(a *ateapipb.VolumeMount, b *ateapipb.VolumeMount) bool { return a.MountPath == b.MountPath }, ateDeepEqual, Validate_VolumeMount); len(e) != 0 { errs = append(errs, e...) } return diff --git a/internal/e2e/suites/imagevolume/imagevolume_test.go b/internal/e2e/suites/imagevolume/imagevolume_test.go index 350ef24bb9..509b879e11 100644 --- a/internal/e2e/suites/imagevolume/imagevolume_test.go +++ b/internal/e2e/suites/imagevolume/imagevolume_test.go @@ -12,7 +12,8 @@ // See the License for the specific language governing permissions and // limitations under the License. -// Package imagevolume exercises image volumes against a live cluster. +// Package imagevolume exercises image volumes, and the volume mount semantics +// they share with durable dirs and external volumes, against a live cluster. package imagevolume import ( @@ -38,6 +39,7 @@ import ( "github.com/google/go-containerregistry/pkg/v1/mutate" "github.com/google/go-containerregistry/pkg/v1/remote" "github.com/google/go-containerregistry/pkg/v1/tarball" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) const ( @@ -45,6 +47,25 @@ const ( // mountPath must not collide with anything the probe's own image ships. mountPath = "/mnt/ate-image-volume" + // mountPathAlias is a second mount of the same image volume: one volume + // may be mounted at multiple paths. + mountPathAlias = "/mnt/ate-image-volume-alias" + + // The scratch durable-dir volume is mounted at both of these paths; a + // write through one must be readable through the other. + scratchPathA = "/mnt/ate-scratch-a" + scratchPathB = "/mnt/ate-scratch-b" + + // The external (CSI-backed) volume is mounted at both of these paths. + // Unlike the durable dir it is provisioned per actor and detached on + // suspend, so it must come back attached once and visible at both. + extVolume = "external" + extPathA = "/mnt/ate-external-a" + extPathB = "/mnt/ate-external-b" + extCapacity = "1Gi" + + // probeWrittenContent is the fixed string the probe's /writefile writes. + probeWrittenContent = "written by probe" payloadName = "payload.txt" payloadContent = "delivered by an image volume" @@ -128,11 +149,26 @@ func buildFixtureImage(t *testing.T, repo string) string { return fmt.Sprintf("%s@%s", tag.Context().Name(), digest) } +// storageClassOrEmpty returns the configured StorageClass if the cluster has +// one, and "" if it does not. The CSI driver is optional, so a missing class +// drops the external volume from the template instead of failing every test +// in the suite. +func storageClassOrEmpty(ctx context.Context, t *testing.T, clients *e2e.Clients) string { + t.Helper() + + if _, err := clients.K8s.StorageV1().StorageClasses().Get(ctx, e2e.StorageClass, metav1.GetOptions{}); err != nil { + t.Logf("StorageClass %q not found (%v); the external-volume case will be skipped", e2e.StorageClass, err) + return "" + } + return e2e.StorageClass +} + // createTemplate builds a probe ActorTemplate with the fixture attached as an // image volume, copying the resolved runtime from the shared probe template. // The template's name is suffixed per test run: it lives in the suite's -// shared atespace, which outlives the per-test k8s namespace. -func createTemplate(ctx context.Context, t *testing.T, clients *e2e.Clients, ns *e2e.Namespace, fixtureImage string) *ateapipb.ActorTemplate { +// shared atespace, which outlives the per-test k8s namespace. A non-empty +// storageClass adds the external volume. +func createTemplate(ctx context.Context, t *testing.T, clients *e2e.Clients, ns *e2e.Namespace, fixtureImage, storageClass string) *ateapipb.ActorTemplate { t.Helper() env, err := e2e.CheckEnv("BUCKET_NAME") @@ -162,11 +198,35 @@ func createTemplate(ctx context.Context, t *testing.T, clients *e2e.Clients, ns StorageLocation: fmt.Sprintf("gs://%s/%s/", env["BUCKET_NAME"], ns.Name), }, Modify: func(tmpl *ateapipb.ActorTemplate) { + // Every volume is mounted at two paths, covering the read-only + // (image), writable (durable-dir) and per-actor provisioned + // (external) multi-path cases. tmpl.Containers[0].VolumeMounts = append(tmpl.Containers[0].VolumeMounts, - &ateapipb.VolumeMount{Name: "fixture", MountPath: mountPath}) + &ateapipb.VolumeMount{Name: "fixture", MountPath: mountPath}, + &ateapipb.VolumeMount{Name: "fixture", MountPath: mountPathAlias}, + &ateapipb.VolumeMount{Name: "scratch", MountPath: scratchPathA}, + &ateapipb.VolumeMount{Name: "scratch", MountPath: scratchPathB}) + tmpl.Volumes = append(tmpl.Volumes, + &ateapipb.Volume{ + Name: "fixture", + Image: &ateapipb.ImageVolumeSource{Reference: fixtureImage}, + }, + &ateapipb.Volume{ + Name: "scratch", + DurableDir: &ateapipb.DurableDirVolumeSource{}, + }) + if storageClass == "" { + return + } + tmpl.Containers[0].VolumeMounts = append(tmpl.Containers[0].VolumeMounts, + &ateapipb.VolumeMount{Name: extVolume, MountPath: extPathA}, + &ateapipb.VolumeMount{Name: extVolume, MountPath: extPathB}) tmpl.Volumes = append(tmpl.Volumes, &ateapipb.Volume{ - Name: "fixture", - Image: &ateapipb.ImageVolumeSource{Reference: fixtureImage}, + Name: extVolume, + ExternalVolumeTemplate: &ateapipb.ExternalVolumeTemplate{ + Capacity: extCapacity, + StorageClassName: storageClass, + }, }) }, }) @@ -205,7 +265,8 @@ func TestImageVolume(t *testing.T) { fixtureImage := buildFixtureImage(t, repo) t.Logf("fixture image: %s", fixtureImage) - tmpl := createTemplate(ctx, t, clients, ns, fixtureImage) + storageClass := storageClassOrEmpty(ctx, t, clients) + tmpl := createTemplate(ctx, t, clients, ns, fixtureImage, storageClass) actorRef := resources.ActorRef{Atespace: atespace, Name: "iv-" + ns.Name} if _, err := clients.SubstrateAPI.CreateActor(ctx, &ateapipb.CreateActorRequest{ @@ -268,6 +329,45 @@ func TestImageVolume(t *testing.T) { } }) + t.Run("SameImageVolumeAtTwoPaths", func(t *testing.T) { + got := probeJSON(ctx, t, router, actorRef, "/readfile?path="+mountPathAlias+"/"+payloadName) + if got["error"] != "" { + t.Fatalf("reading %s through the alias mount: %s", payloadName, got["error"]) + } + if got["content"] != payloadContent { + t.Errorf("content through alias mount = %q, want %q", got["content"], payloadContent) + } + }) + + t.Run("SameDurableVolumeAtTwoPathsSharesWrites", func(t *testing.T) { + if got := probeJSON(ctx, t, router, actorRef, "/writefile?path="+scratchPathA+"/multi.txt"); got["error"] != "" { + t.Fatalf("writing through %s: %s", scratchPathA, got["error"]) + } + got := probeJSON(ctx, t, router, actorRef, "/readfile?path="+scratchPathB+"/multi.txt") + if got["error"] != "" { + t.Fatalf("reading through %s what was written through %s: %s", scratchPathB, scratchPathA, got["error"]) + } + if got["content"] != probeWrittenContent { + t.Errorf("content through second mount = %q, want %q", got["content"], probeWrittenContent) + } + }) + + t.Run("SameExternalVolumeAtTwoPathsSharesWrites", func(t *testing.T) { + if storageClass == "" { + t.Skipf("StorageClass %q is not installed", e2e.StorageClass) + } + if got := probeJSON(ctx, t, router, actorRef, "/writefile?path="+extPathA+"/multi.txt"); got["error"] != "" { + t.Fatalf("writing through %s: %s", extPathA, got["error"]) + } + got := probeJSON(ctx, t, router, actorRef, "/readfile?path="+extPathB+"/multi.txt") + if got["error"] != "" { + t.Fatalf("reading through %s what was written through %s: %s", extPathB, extPathA, got["error"]) + } + if got["content"] != probeWrittenContent { + t.Errorf("content through second mount = %q, want %q", got["content"], probeWrittenContent) + } + }) + t.Run("SurvivesSuspendResume", func(t *testing.T) { if _, err := clients.SubstrateAPI.SuspendActor(ctx, &ateapipb.SuspendActorRequest{Actor: actorRef.ToObjectRef()}); err != nil { t.Fatalf("SuspendActor: %v", err) @@ -283,5 +383,35 @@ func TestImageVolume(t *testing.T) { if got["content"] != payloadContent { t.Errorf("content after resume = %q, want %q", got["content"], payloadContent) } + + // Restore must re-establish all mounts and preserve shared writes across them. + got = probeJSON(resumeCtx, t, router, actorRef, "/readfile?path="+mountPathAlias+"/"+payloadName) + if got["error"] != "" { + t.Fatalf("reading %s through the alias mount after resume: %s", payloadName, got["error"]) + } + if got["content"] != payloadContent { + t.Errorf("content through alias mount after resume = %q, want %q", got["content"], payloadContent) + } + + got = probeJSON(resumeCtx, t, router, actorRef, "/readfile?path="+scratchPathB+"/multi.txt") + if got["error"] != "" { + t.Fatalf("reading %s/multi.txt after resume: %s", scratchPathB, got["error"]) + } + if got["content"] != probeWrittenContent { + t.Errorf("content through second mount after resume = %q, want %q", got["content"], probeWrittenContent) + } + + if storageClass == "" { + return + } + // Suspend detached the external volume; the resume must reattach it + // once and restore both of its mounts. + got = probeJSON(resumeCtx, t, router, actorRef, "/readfile?path="+extPathB+"/multi.txt") + if got["error"] != "" { + t.Fatalf("reading %s/multi.txt after resume: %s", extPathB, got["error"]) + } + if got["content"] != probeWrittenContent { + t.Errorf("external content through second mount after resume = %q, want %q", got["content"], probeWrittenContent) + } }) } diff --git a/pkg/proto/ateapipb/ateapi.pb.go b/pkg/proto/ateapipb/ateapi.pb.go index 8f04416d2e..fb5e9061a5 100644 --- a/pkg/proto/ateapipb/ateapi.pb.go +++ b/pkg/proto/ateapipb/ateapi.pb.go @@ -1506,8 +1506,6 @@ type ActorStatus struct { // They are deleted when the actor is deleted. Each template volume // appears at most once. // - // TODO: consider a literal map keyed by volume_name instead of a list-map. - // // +k8s:optional // +k8s:maxItems=32 # matches the template's volumes bound // +k8s:listType=map @@ -2627,15 +2625,14 @@ type Container struct { // // +k8s:optional Readyz *ContainerReadyz `protobuf:"bytes,6,opt,name=readyz,proto3" json:"readyz,omitempty"` - // TODO: Kubernetes permits mounting a single volume at multiple paths - // (which requires keying by mountPath). We restrict it to one mount per - // volume (keyed by name). + // Keyed by mount_path: each path hosts exactly one mount, while a volume + // may be mounted at multiple paths. // // +k8s:optional // +k8s:maxItems=32 // +k8s:listType=map - // +k8s:listMapKey=name - // +k8s:customValidation # mount_path must be unique within the container + // +k8s:listMapKey=mount_path + // +k8s:customValidation # mounts must not nest VolumeMounts []*VolumeMount `protobuf:"bytes,7,rep,name=volume_mounts,json=volumeMounts,proto3" json:"volume_mounts,omitempty"` // security_context adjusts the container's security settings. Unset leaves // the default capability set. diff --git a/pkg/proto/ateapipb/ateapi.proto b/pkg/proto/ateapipb/ateapi.proto index 00a3d35b3b..2eccad140d 100644 --- a/pkg/proto/ateapipb/ateapi.proto +++ b/pkg/proto/ateapipb/ateapi.proto @@ -551,8 +551,6 @@ message ActorStatus { // They are deleted when the actor is deleted. Each template volume // appears at most once. // - // TODO: consider a literal map keyed by volume_name instead of a list-map. - // // +k8s:optional // +k8s:maxItems=32 # matches the template's volumes bound // +k8s:listType=map @@ -937,15 +935,14 @@ message Container { // +k8s:optional ContainerReadyz readyz = 6; - // TODO: Kubernetes permits mounting a single volume at multiple paths - // (which requires keying by mountPath). We restrict it to one mount per - // volume (keyed by name). + // Keyed by mount_path: each path hosts exactly one mount, while a volume + // may be mounted at multiple paths. // // +k8s:optional // +k8s:maxItems=32 // +k8s:listType=map - // +k8s:listMapKey=name - // +k8s:customValidation # mount_path must be unique within the container + // +k8s:listMapKey=mount_path + // +k8s:customValidation # mounts must not nest repeated VolumeMount volume_mounts = 7; // security_context adjusts the container's security settings. Unset leaves @@ -1599,10 +1596,6 @@ message ListWorkersResponse { string next_page_token = 2; } -// TODO: Workers are still created, updated, and deleted by writing directly to -// the store from the WorkerPoolSyncer and the actor workflows; migrating those -// callers onto the RPCs below lands in a follow-up change. - message GetWorkerRequest { // The Worker to fetch. atespace is always empty; Workers are global-scoped. // From b43f8908a65eaec1c536894fa65dc0cd2202919b Mon Sep 17 00:00:00 2001 From: shrutiyam-glitch Date: Fri, 11 Sep 2026 15:46:58 -0700 Subject: [PATCH 2/3] Extract e2e volume probe helpers and verify both mount paths --- .../suites/imagevolume/imagevolume_test.go | 154 ++++++++---------- 1 file changed, 70 insertions(+), 84 deletions(-) diff --git a/internal/e2e/suites/imagevolume/imagevolume_test.go b/internal/e2e/suites/imagevolume/imagevolume_test.go index 509b879e11..3ae7c6e964 100644 --- a/internal/e2e/suites/imagevolume/imagevolume_test.go +++ b/internal/e2e/suites/imagevolume/imagevolume_test.go @@ -253,6 +253,59 @@ func probeJSON(ctx context.Context, t *testing.T, router *e2e.RouterClient, acto return out } +// requireContent fails unless path holds want. Used to verify a mount +// delivers what the volume behind it should. +func requireContent(ctx context.Context, t *testing.T, router *e2e.RouterClient, actorRef resources.ActorRef, path, want string) { + t.Helper() + + got := probeJSON(ctx, t, router, actorRef, "/readfile?path="+path) + if got["error"] != "" { + t.Fatalf("reading %s: %s", path, got["error"]) + } + if got["content"] != want { + t.Errorf("content at %s = %q, want %q", path, got["content"], want) + } +} + +// requireContentAtBoth fails unless both paths hold want, so both mounts must +// be live and backed by the same volume. +func requireContentAtBoth(ctx context.Context, t *testing.T, router *e2e.RouterClient, actorRef resources.ActorRef, pathA, pathB, want string) { + t.Helper() + + requireContent(ctx, t, router, actorRef, pathA, want) + requireContent(ctx, t, router, actorRef, pathB, want) +} + +// requireUnreadable fails if path can be read. +func requireUnreadable(ctx context.Context, t *testing.T, router *e2e.RouterClient, actorRef resources.ActorRef, path string) { + t.Helper() + + if got := probeJSON(ctx, t, router, actorRef, "/readfile?path="+path); got["error"] == "" { + t.Errorf("%s is readable (%q), want it hidden", path, got["content"]) + } +} + +// requireWriteRejected fails if path can be written. +func requireWriteRejected(ctx context.Context, t *testing.T, router *e2e.RouterClient, actorRef resources.ActorRef, path string) { + t.Helper() + + if got := probeJSON(ctx, t, router, actorRef, "/writefile?path="+path); got["error"] == "" { + t.Errorf("write to %s succeeded, want it rejected as read-only", path) + } +} + +// requireSharedWrite writes through writePath and requires the content at both +// paths: writePath confirms the write persisted, aliasPath that the two mounts +// reach the same volume. +func requireSharedWrite(ctx context.Context, t *testing.T, router *e2e.RouterClient, actorRef resources.ActorRef, writePath, aliasPath string) { + t.Helper() + + if got := probeJSON(ctx, t, router, actorRef, "/writefile?path="+writePath); got["error"] != "" { + t.Fatalf("writing %s: %s", writePath, got["error"]) + } + requireContentAtBoth(ctx, t, router, actorRef, writePath, aliasPath, probeWrittenContent) +} + func TestImageVolume(t *testing.T) { repo := os.Getenv("KO_DOCKER_REPO") if repo == "" { @@ -296,76 +349,34 @@ func TestImageVolume(t *testing.T) { payloadPath := mountPath + "/" + payloadName t.Run("DeliversImageContents", func(t *testing.T) { - got := probeJSON(ctx, t, router, actorRef, "/readfile?path="+payloadPath) - if got["error"] != "" { - t.Fatalf("reading %s: %s", payloadPath, got["error"]) - } - if got["content"] != payloadContent { - t.Errorf("content = %q, want %q", got["content"], payloadContent) - } + requireContent(ctx, t, router, actorRef, payloadPath, payloadContent) }) t.Run("UpperLayerWins", func(t *testing.T) { - got := probeJSON(ctx, t, router, actorRef, "/readfile?path="+mountPath+"/"+shadowedName) - if got["error"] != "" { - t.Fatalf("reading %s: %s", shadowedName, got["error"]) - } - if got["content"] != shadowedContent { - t.Errorf("content = %q, want %q from the upper layer", got["content"], shadowedContent) - } + requireContent(ctx, t, router, actorRef, mountPath+"/"+shadowedName, shadowedContent) }) t.Run("WhiteoutHidesLowerLayerFile", func(t *testing.T) { - got := probeJSON(ctx, t, router, actorRef, "/readfile?path="+mountPath+"/"+deletedName) - if got["error"] == "" { - t.Errorf("%s is readable (%q), want it hidden by the whiteout", deletedName, got["content"]) - } + requireUnreadable(ctx, t, router, actorRef, mountPath+"/"+deletedName) }) t.Run("MountIsReadOnly", func(t *testing.T) { - got := probeJSON(ctx, t, router, actorRef, "/writefile?path="+mountPath+"/should-not-exist") - if got["error"] == "" { - t.Errorf("write to the image volume succeeded, want it rejected as read-only") - } + requireWriteRejected(ctx, t, router, actorRef, mountPath+"/should-not-exist") }) t.Run("SameImageVolumeAtTwoPaths", func(t *testing.T) { - got := probeJSON(ctx, t, router, actorRef, "/readfile?path="+mountPathAlias+"/"+payloadName) - if got["error"] != "" { - t.Fatalf("reading %s through the alias mount: %s", payloadName, got["error"]) - } - if got["content"] != payloadContent { - t.Errorf("content through alias mount = %q, want %q", got["content"], payloadContent) - } + requireContentAtBoth(ctx, t, router, actorRef, payloadPath, mountPathAlias+"/"+payloadName, payloadContent) }) t.Run("SameDurableVolumeAtTwoPathsSharesWrites", func(t *testing.T) { - if got := probeJSON(ctx, t, router, actorRef, "/writefile?path="+scratchPathA+"/multi.txt"); got["error"] != "" { - t.Fatalf("writing through %s: %s", scratchPathA, got["error"]) - } - got := probeJSON(ctx, t, router, actorRef, "/readfile?path="+scratchPathB+"/multi.txt") - if got["error"] != "" { - t.Fatalf("reading through %s what was written through %s: %s", scratchPathB, scratchPathA, got["error"]) - } - if got["content"] != probeWrittenContent { - t.Errorf("content through second mount = %q, want %q", got["content"], probeWrittenContent) - } + requireSharedWrite(ctx, t, router, actorRef, scratchPathA+"/multi.txt", scratchPathB+"/multi.txt") }) t.Run("SameExternalVolumeAtTwoPathsSharesWrites", func(t *testing.T) { if storageClass == "" { t.Skipf("StorageClass %q is not installed", e2e.StorageClass) } - if got := probeJSON(ctx, t, router, actorRef, "/writefile?path="+extPathA+"/multi.txt"); got["error"] != "" { - t.Fatalf("writing through %s: %s", extPathA, got["error"]) - } - got := probeJSON(ctx, t, router, actorRef, "/readfile?path="+extPathB+"/multi.txt") - if got["error"] != "" { - t.Fatalf("reading through %s what was written through %s: %s", extPathB, extPathA, got["error"]) - } - if got["content"] != probeWrittenContent { - t.Errorf("content through second mount = %q, want %q", got["content"], probeWrittenContent) - } + requireSharedWrite(ctx, t, router, actorRef, extPathA+"/multi.txt", extPathB+"/multi.txt") }) t.Run("SurvivesSuspendResume", func(t *testing.T) { @@ -373,45 +384,20 @@ func TestImageVolume(t *testing.T) { t.Fatalf("SuspendActor: %v", err) } - // No explicit resume: routing to the actor is what wakes it. + // No explicit resume: routing to the actor is what wakes it. Restore + // must re-establish all mounts and preserve shared writes across them. resumeCtx, cancel := context.WithTimeout(ctx, 2*time.Minute) defer cancel() - got := probeJSON(resumeCtx, t, router, actorRef, "/readfile?path="+payloadPath) - if got["error"] != "" { - t.Fatalf("reading %s after resume: %s", payloadPath, got["error"]) - } - if got["content"] != payloadContent { - t.Errorf("content after resume = %q, want %q", got["content"], payloadContent) - } - - // Restore must re-establish all mounts and preserve shared writes across them. - got = probeJSON(resumeCtx, t, router, actorRef, "/readfile?path="+mountPathAlias+"/"+payloadName) - if got["error"] != "" { - t.Fatalf("reading %s through the alias mount after resume: %s", payloadName, got["error"]) - } - if got["content"] != payloadContent { - t.Errorf("content through alias mount after resume = %q, want %q", got["content"], payloadContent) - } - - got = probeJSON(resumeCtx, t, router, actorRef, "/readfile?path="+scratchPathB+"/multi.txt") - if got["error"] != "" { - t.Fatalf("reading %s/multi.txt after resume: %s", scratchPathB, got["error"]) - } - if got["content"] != probeWrittenContent { - t.Errorf("content through second mount after resume = %q, want %q", got["content"], probeWrittenContent) - } + requireContentAtBoth(resumeCtx, t, router, actorRef, payloadPath, mountPathAlias+"/"+payloadName, payloadContent) + requireContentAtBoth(resumeCtx, t, router, actorRef, scratchPathA+"/multi.txt", scratchPathB+"/multi.txt", probeWrittenContent) - if storageClass == "" { - return - } // Suspend detached the external volume; the resume must reattach it // once and restore both of its mounts. - got = probeJSON(resumeCtx, t, router, actorRef, "/readfile?path="+extPathB+"/multi.txt") - if got["error"] != "" { - t.Fatalf("reading %s/multi.txt after resume: %s", extPathB, got["error"]) - } - if got["content"] != probeWrittenContent { - t.Errorf("external content through second mount after resume = %q, want %q", got["content"], probeWrittenContent) - } + t.Run("ExternalVolumeReattached", func(t *testing.T) { + if storageClass == "" { + t.Skipf("StorageClass %q is not installed", e2e.StorageClass) + } + requireContentAtBoth(resumeCtx, t, router, actorRef, extPathA+"/multi.txt", extPathB+"/multi.txt", probeWrittenContent) + }) }) } From d1fabac09f6c0adba3afad65f3d792791018386a Mon Sep 17 00:00:00 2001 From: shrutiyam-glitch Date: Fri, 11 Sep 2026 16:52:30 -0700 Subject: [PATCH 3/3] Rename the imagevolume e2e suite to combinedvolumes --- .../combinedvolumes_test.go} | 21 ++++++++++--------- .../testmain_test.go | 2 +- 2 files changed, 12 insertions(+), 11 deletions(-) rename internal/e2e/suites/{imagevolume/imagevolume_test.go => combinedvolumes/combinedvolumes_test.go} (95%) rename internal/e2e/suites/{imagevolume => combinedvolumes}/testmain_test.go (96%) diff --git a/internal/e2e/suites/imagevolume/imagevolume_test.go b/internal/e2e/suites/combinedvolumes/combinedvolumes_test.go similarity index 95% rename from internal/e2e/suites/imagevolume/imagevolume_test.go rename to internal/e2e/suites/combinedvolumes/combinedvolumes_test.go index 3ae7c6e964..c21ee1191d 100644 --- a/internal/e2e/suites/imagevolume/imagevolume_test.go +++ b/internal/e2e/suites/combinedvolumes/combinedvolumes_test.go @@ -12,9 +12,10 @@ // See the License for the specific language governing permissions and // limitations under the License. -// Package imagevolume exercises image volumes, and the volume mount semantics -// they share with durable dirs and external volumes, against a live cluster. -package imagevolume +// Package combinedvolumes exercises the three volume sources -- image volumes, +// durable dirs, and external volumes -- mounted together on a single actor, +// along with the mount semantics they share, against a live cluster. +package combinedvolumes import ( "archive/tar" @@ -43,7 +44,7 @@ import ( ) const ( - atespace = "imagevolume" + atespace = "combinedvolumes" // mountPath must not collide with anything the probe's own image ships. mountPath = "/mnt/ate-image-volume" @@ -130,7 +131,7 @@ func buildFixtureImage(t *testing.T, repo string) string { // A unique tag per run: some registries refuse to overwrite an existing // tag. The returned reference is digest-pinned, so the tag itself is // throwaway. - ref := fmt.Sprintf("%s/e2e-imagevolume-fixture:%d", strings.TrimSuffix(repo, "/"), time.Now().UnixNano()) + ref := fmt.Sprintf("%s/e2e-combinedvolumes-fixture:%d", strings.TrimSuffix(repo, "/"), time.Now().UnixNano()) tag, err := name.ParseReference(ref, name.Insecure) if err != nil { t.Fatalf("parsing %q: %v", ref, err) @@ -177,13 +178,13 @@ func createTemplate(ctx context.Context, t *testing.T, clients *e2e.Clients, ns } // The probe supplies this suite's container image and resolved runtime. - probeAtespace, _ := e2e.DeployProbe(t, env["BUCKET_NAME"], "imagevolume") + probeAtespace, _ := e2e.DeployProbe(t, env["BUCKET_NAME"], "combinedvolumes") src := e2e.SubstrateFixture{ Atespace: probeAtespace, Name: probeName, PoolNamespace: probeAtespace, PoolName: probeName, - DeployWith: "the imagevolume suite's own DeployProbe", + DeployWith: "the combinedvolumes suite's own DeployProbe", } return e2e.CreateSubstrateTemplateFrom(ctx, t, clients, ns.Name, src, e2e.SubstrateTemplateOptions{ @@ -193,7 +194,7 @@ func createTemplate(ctx context.Context, t *testing.T, clients *e2e.Clients, ns PoolReplicas: 2, // The pool is labeled uniquely to this namespace so the cluster-wide // scheduler cannot hand its workers to another suite's actors. - Labels: map[string]string{"imagevolume": ns.Name}, + Labels: map[string]string{"combinedvolumes": ns.Name}, SnapshotsConfig: &ateapipb.SnapshotsConfig{ StorageLocation: fmt.Sprintf("gs://%s/%s/", env["BUCKET_NAME"], ns.Name), }, @@ -306,7 +307,7 @@ func requireSharedWrite(ctx context.Context, t *testing.T, router *e2e.RouterCli requireContentAtBoth(ctx, t, router, actorRef, writePath, aliasPath, probeWrittenContent) } -func TestImageVolume(t *testing.T) { +func TestCombinedVolumes(t *testing.T) { repo := os.Getenv("KO_DOCKER_REPO") if repo == "" { t.Skip("KO_DOCKER_REPO is unset; it names the registry both this host and the cluster can reach") @@ -321,7 +322,7 @@ func TestImageVolume(t *testing.T) { storageClass := storageClassOrEmpty(ctx, t, clients) tmpl := createTemplate(ctx, t, clients, ns, fixtureImage, storageClass) - actorRef := resources.ActorRef{Atespace: atespace, Name: "iv-" + ns.Name} + actorRef := resources.ActorRef{Atespace: atespace, Name: "cv-" + ns.Name} if _, err := clients.SubstrateAPI.CreateActor(ctx, &ateapipb.CreateActorRequest{ Actor: &ateapipb.Actor{ Metadata: &ateapipb.ResourceMetadata{Atespace: actorRef.Atespace, Name: actorRef.Name}, diff --git a/internal/e2e/suites/imagevolume/testmain_test.go b/internal/e2e/suites/combinedvolumes/testmain_test.go similarity index 96% rename from internal/e2e/suites/imagevolume/testmain_test.go rename to internal/e2e/suites/combinedvolumes/testmain_test.go index cda783977c..0d283d557a 100644 --- a/internal/e2e/suites/imagevolume/testmain_test.go +++ b/internal/e2e/suites/combinedvolumes/testmain_test.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package imagevolume +package combinedvolumes import ( "os"