From cd0ce64b73f934754db52a7003d01e8999b916ba Mon Sep 17 00:00:00 2001 From: cloudonly Date: Mon, 12 Oct 2026 10:47:15 +0800 Subject: [PATCH] ateapi: reject VOLUMES snapshots without supported volume mounts A template whose preferred fidelity is VOLUMES but that mounts no volume a snapshot can capture (durable-dir, or a CSI volume on the micro-VM class) fails at runtime on every pause and suspend. Reject it at template admission instead, in the custom validation that already checks volume references and the ROOTFS sandbox class. Document the requirement next to the ROOTFS rule in the API guide, including that an external volume's data stays on the volume rather than in the snapshot. --- .../internal/apivalidation/actor_template.go | 19 +++-- .../apivalidation/actor_template_test.go | 69 +++++++++++++++++++ .../controlapi/functionaltest/actor_test.go | 8 ++- docs/api-guide.md | 2 +- pkg/proto/ateapipb/ateapi.pb.go | 2 +- pkg/proto/ateapipb/ateapi.proto | 2 +- 6 files changed, 92 insertions(+), 10 deletions(-) diff --git a/cmd/ateapi/internal/apivalidation/actor_template.go b/cmd/ateapi/internal/apivalidation/actor_template.go index 2b558e74b2..f670f0782b 100644 --- a/cmd/ateapi/internal/apivalidation/actor_template.go +++ b/cmd/ateapi/internal/apivalidation/actor_template.go @@ -58,26 +58,37 @@ func ValidateActorTemplateUpdate(ctx context.Context, fldPath *field.Path, newVa // ValidateCustom_CreateActorTemplateRequest_ActorTemplate holds the rules // that span more than one field of a template: container volume mounts must -// reference volumes the template declares. +// reference volumes the template declares, and a VOLUMES preferred fidelity +// needs a mounted volume snapshots can capture. func ValidateCustom_CreateActorTemplateRequest_ActorTemplate(_ context.Context, _ operation.Operation, fldPath *field.Path, value, _ *ateapipb.ActorTemplate) field.ErrorList { var errs field.ErrorList - declared := make(map[string]bool, len(value.GetVolumes())) + declared := make(map[string]*ateapipb.Volume, len(value.GetVolumes())) for _, vol := range value.GetVolumes() { - declared[vol.GetName()] = true + declared[vol.GetName()] = vol } + hasSnapshotVolume := false for i, ctr := range value.GetContainers() { for j, mount := range ctr.GetVolumeMounts() { name := mount.GetName() if name == "" { continue // required is enforced by tags } - if !declared[name] { + vol := declared[name] + if vol == nil { errs = append(errs, field.Invalid( fldPath.Child("containers").Index(i).Child("volume_mounts").Index(j).Child("name"), name, "must reference a volume declared in the template")) + continue + } + if vol.GetDurableDir() != nil || vol.GetExternalVolumeTemplate() != nil { + hasSnapshotVolume = true } } } + fidelity := value.GetSnapshotConfig().GetPreferredFidelity() + if !hasSnapshotVolume && fidelity == ateapipb.SnapshotFidelity_SNAPSHOT_FIDELITY_VOLUMES { + errs = append(errs, field.Invalid(fldPath.Child("snapshot_config", "preferred_fidelity"), fidelity.String(), "VOLUMES snapshots require a mounted durable-dir or CSI volume")) + } return errs } diff --git a/cmd/ateapi/internal/apivalidation/actor_template_test.go b/cmd/ateapi/internal/apivalidation/actor_template_test.go index 7b4d963cee..205962cd93 100644 --- a/cmd/ateapi/internal/apivalidation/actor_template_test.go +++ b/cmd/ateapi/internal/apivalidation/actor_template_test.go @@ -26,6 +26,73 @@ import ( "k8s.io/apimachinery/pkg/util/validation/field" ) +func TestValidateCreateActorTemplateRequestVolumesFidelity(t *testing.T) { + // The CSI cases declare external volume templates, which the ExternalVolumes + // preview gate controls. + setPreviewForTest(t, "ALL") + durable := &ateapipb.Volume{Name: "data", DurableDir: &ateapipb.DurableDirVolumeSource{}} + csi := &ateapipb.Volume{Name: "csi", ExternalVolumeTemplate: &ateapipb.ExternalVolumeTemplate{ + Capacity: "1Gi", StorageClassName: "standard", + }} + image := &ateapipb.Volume{Name: "data", Image: &ateapipb.ImageVolumeSource{ + Reference: "example.com/data@sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", + }} + systemInfo := &ateapipb.Volume{Name: "system-info", SystemInfo: &ateapipb.SystemInfoVolumeSource{ + DataSources: []*ateapipb.SystemInfoDataSource{{ActorMetadata: &ateapipb.ActorMetadataDataSource{ + Items: []*ateapipb.ActorMetadataItem{{Field: ateapipb.ActorMetadataField_ACTOR_METADATA_FIELD_NAME, Path: "actor-name"}}, + }}}, + }} + for _, snapshots := range []struct { + name string + fidelity ateapipb.SnapshotFidelity + }{ + {"memory", ateapipb.SnapshotFidelity_SNAPSHOT_FIDELITY_MEMORY}, + {"volumes", ateapipb.SnapshotFidelity_SNAPSHOT_FIDELITY_VOLUMES}, + } { + for _, tt := range []struct { + name string + volume *ateapipb.Volume + extra *ateapipb.Volume + mounted bool + supportsVolumes bool + }{ + {"no volumes", nil, nil, false, false}, + {"mounted durable-dir", durable, nil, true, true}, + {"unmounted durable-dir", durable, nil, false, false}, + {"mounted CSI", csi, nil, true, true}, + {"unmounted CSI", csi, nil, false, false}, + {"mounted image", image, nil, true, false}, + {"mounted durable-dir and CSI", durable, csi, true, true}, + {"mounted systemInfo", systemInfo, nil, true, false}, + } { + t.Run(snapshots.name+"/"+tt.name, func(t *testing.T) { + tmpl := validActorTemplate(func(tmpl *ateapipb.ActorTemplate) { + tmpl.SnapshotConfig.PreferredFidelity = snapshots.fidelity + var volumes []*ateapipb.Volume + for _, v := range []*ateapipb.Volume{tt.volume, tt.extra} { + if v != nil { + volumes = append(volumes, v) + } + } + tmpl.Volumes = volumes + if tt.mounted { + for _, v := range volumes { + tmpl.Containers[0].VolumeMounts = append(tmpl.Containers[0].VolumeMounts, &ateapipb.VolumeMount{ + Name: v.GetName(), MountPath: "/" + v.GetName(), + }) + } + } + }) + var want field.ErrorList + if !tt.supportsVolumes && snapshots.fidelity == ateapipb.SnapshotFidelity_SNAPSHOT_FIDELITY_VOLUMES { + want = append(want, field.Invalid(field.NewPath("actor_template", "snapshot_config", "preferred_fidelity"), snapshots.fidelity.String(), "")) + } + assertValidateErr(t, ValidateCreateActorTemplateRequest(context.Background(), &ateapipb.CreateActorTemplateRequest{ActorTemplate: tmpl}), want) + }) + } + } +} + func TestValidateCreateActorTemplateRequest(t *testing.T) { tests := []struct { name string @@ -67,6 +134,8 @@ func TestValidateCreateActorTemplateRequest(t *testing.T) { "valid volumes fidelity", &ateapipb.CreateActorTemplateRequest{ActorTemplate: validActorTemplate(func(tmpl *ateapipb.ActorTemplate) { tmpl.SnapshotConfig.PreferredFidelity = ateapipb.SnapshotFidelity_SNAPSHOT_FIDELITY_VOLUMES + tmpl.Volumes = []*ateapipb.Volume{{Name: "data", DurableDir: &ateapipb.DurableDirVolumeSource{}}} + tmpl.Containers[0].VolumeMounts = []*ateapipb.VolumeMount{{Name: "data", MountPath: "/data"}} })}, nil, }, { diff --git a/cmd/ateapi/internal/controlapi/functionaltest/actor_test.go b/cmd/ateapi/internal/controlapi/functionaltest/actor_test.go index e7bbf484c3..fac4a20ee5 100644 --- a/cmd/ateapi/internal/controlapi/functionaltest/actor_test.go +++ b/cmd/ateapi/internal/controlapi/functionaltest/actor_test.go @@ -3256,10 +3256,12 @@ func createDataFidelityTemplate(t *testing.T, tc *testContext, ns string) *ateap PreferredFidelity: ateapipb.SnapshotFidelity_SNAPSHOT_FIDELITY_VOLUMES, }, Containers: []*ateapipb.Container{{ - Name: "main", - Image: "main@sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", - Command: []string{"/main"}, + Name: "main", + Image: "main@sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", + Command: []string{"/main"}, + VolumeMounts: []*ateapipb.VolumeMount{{Name: "data", MountPath: "/data"}}, }}, + Volumes: []*ateapipb.Volume{{Name: "data", DurableDir: &ateapipb.DurableDirVolumeSource{}}}, WorkerSelector: &ateapipb.Selector{ MatchLabels: map[string]string{poolLabelKey: ns}, }, diff --git a/docs/api-guide.md b/docs/api-guide.md index a427d5118a..e3ea63d616 100644 --- a/docs/api-guide.md +++ b/docs/api-guide.md @@ -100,7 +100,7 @@ The `ActorTemplate` defines the code, environment, and state-management policies | :--- | :--- | :--- | | `containers` | `[]Container` | **Required.** The workload definition — see [Container Fields](#container-fields) below. Each container may also declare an optional `wakeupProbe` HTTP probe — see [Container Wakeup Probe](#container-wakeup-probe-wakeupprobe). | | `workerSelector` | `*Selector` | Optional. Gates which `WorkerPool`s actors from this template may use, by matching against each pool's labels (`matchLabels`). If unset, all pools are eligible (subject to the actor's own `worker_selector`). The pool decides the sandbox class, so this is also how a template pins one: if your cluster has pools of more than one sandbox class and the workload needs a particular class, make sure the selector points only at pools of that class. An actor booting with no snapshot and no selector may land on any class, and keeps it from its first checkpoint on. | -| `snapshotConfig` | `SnapshotConfig` | **Required.** The base object-storage location snapshots are written under, the snapshot fidelity (`preferredFidelity`) every Pause and Suspend captures, and automated golden snapshot creation (`goldenSnapshotConfig.mode`). See [Snapshot Storage Layout](#snapshot-storage-layout). | +| `snapshotConfig` | `SnapshotConfig` | **Required.** The base object-storage location snapshots are written under, the snapshot fidelity (`preferredFidelity`) every Pause and Suspend captures (`SNAPSHOT_FIDELITY_VOLUMES` requires a container to mount a `durableDir` volume, or an `externalVolumeTemplate` volume whose data stays on the volume rather than in the snapshot), and automated golden snapshot creation (`goldenSnapshotConfig.mode`). See [Snapshot Storage Layout](#snapshot-storage-layout). | | `volumes` | `[]Volume` | Optional. Volumes the containers may mount, each a `durableDir`, an `externalVolumeTemplate` (see [CSI Volumes Guide](csi-volumes.md)), or a `systemInfo` volume (see [SystemInfo Volumes](#systeminfo-volumes)). Every declared volume must be mounted by at least one container. A `durableDir` is created `0777`, as Kubernetes creates an emptyDir, so a container running as any user can write it. A snapshot restores it with the mode it had, so the container's own chmod survives suspend and resume. | | `resources` | `*ResourceRequirements` | Required. Declares each actor's compute size via `limits`, which must set both `cpu` and `memory` — see [Sandbox Right-Sizing](#sandbox-right-sizing-resources). Immutable, like the rest of the template. | diff --git a/pkg/proto/ateapipb/ateapi.pb.go b/pkg/proto/ateapipb/ateapi.pb.go index b7280154ae..ff5ea009c5 100644 --- a/pkg/proto/ateapipb/ateapi.pb.go +++ b/pkg/proto/ateapipb/ateapi.pb.go @@ -4463,7 +4463,7 @@ type CreateActorTemplateRequest struct { // version, timestamps) is ignored, as are the status fields. // // +k8s:required - // +k8s:customValidation # volume_mounts must reference declared volumes + // +k8s:customValidation # volume_mounts must reference declared volumes; VOLUMES fidelity requires a snapshot-capable mounted volume ActorTemplate *ActorTemplate `protobuf:"bytes,1,opt,name=actor_template,json=actorTemplate,proto3" json:"actor_template,omitempty"` unknownFields protoimpl.UnknownFields sizeCache protoimpl.SizeCache diff --git a/pkg/proto/ateapipb/ateapi.proto b/pkg/proto/ateapipb/ateapi.proto index 7ef8c2f921..0e57d95f4b 100644 --- a/pkg/proto/ateapipb/ateapi.proto +++ b/pkg/proto/ateapipb/ateapi.proto @@ -1653,7 +1653,7 @@ message CreateActorTemplateRequest { // version, timestamps) is ignored, as are the status fields. // // +k8s:required - // +k8s:customValidation # volume_mounts must reference declared volumes + // +k8s:customValidation # volume_mounts must reference declared volumes; VOLUMES fidelity requires a snapshot-capable mounted volume ActorTemplate actor_template = 1; }