ateapi: reject VOLUMES snapshots without supported volume mounts

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
 }
 

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,
 	}, {

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},
 			},

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. |
 

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;
 }