Repository navigation
Add a default cluster snapshot storage location with snapshotConfig.storageLocation override in preview mode - #2544
Conversation
|
Do you have a link to a design on |
|
Do you have access to this doc? Eitan Yarmush (@EItanya) |
|
Ohh I see you meant |
|
Just requested. Specifically I want to make sure that we don't remove the ability to have different buckets before changing this API. We are relying on that functionality today. |
|
I have updated the document link in my comment above, it should now be accessible link same as above |
ce59fed to
42b57d8
Compare
|
This PR is built on top of #1994 we are awaiting the base PR to stabilize and merge |
c3e7819 to
fd0a094
Compare
| message SnapshotConfig { | ||
| // storage_location is the base object-storage URI snapshots of actors on | ||
| // this version are stored under. Required. | ||
| // this version are stored under. It is required only when ate-api-server |
There was a problem hiding this comment.
I think even with the gate enabled it shouldn't be required.
You can still set it on the ateapi. The storagelocation would only override the ateapi default
There was a problem hiding this comment.
Updated this behavior.
| return "gs://" + bucket + "/", nil | ||
| } | ||
| if previewGateEnabled(gates, preview.GateSnapshotStorageLocation) { | ||
| return "", nil |
There was a problem hiding this comment.
we can still allow this to be set even if the preview gate is enabled. It can become the cluster-wide default.
There was a problem hiding this comment.
The location is returned above so it will be returned when set irrespective of the SnapshotGate. This check is more to see if we need to error here. I updated this to be more clear.
81fe063 to
411fe95
Compare
| APIAuthzBootstrapOwners: bootstrapOwners, | ||
|
|
||
| BucketName: r.String("storage.bucketName"), | ||
| SnapshotLocation: r.String("storage.snapshotLocation"), |
There was a problem hiding this comment.
🤖 should-fix 🟡 – applyKindDefaults (cmd/ate-setup/internal/config/load.go:359) sets storage.bucketName to "ate-snapshots" while ignoring the ambient environment, but leaves storage.snapshotLocation unset. Running ate-setup deploy ate-system --kind directly now fails unless --snapshot-location is also passed. If .ate-dev-env.sh was already sourced in the shell, both ate-setup --kind and hack/install-ate-kind.sh:36 (${SNAPSHOT_LOCATION:-...}) keep the cloud GCS SNAPSHOT_LOCATION while resetting BUCKET_NAME to ate-snapshots, which is the only bucket local rustfs creates.
Default storage.snapshotLocation in applyKindDefaults to "gs://" + r.String("storage.bucketName") + "/" when !namedExplicitly(r, "storage.snapshotLocation"), and set export SNAPSHOT_LOCATION="gs://${BUCKET_NAME}/" unconditionally in hack/install-ate-kind.sh:36.
There was a problem hiding this comment.
Updated the behavior as per recommendation
| } | ||
| // Snapshot location should always be set by --snapshot-location flag when not using preview mode. | ||
| if !previewGateEnabled(gates, preview.GateSnapshotStorageLocation) { | ||
| return "", errors.New("--snapshot-location must be set if not using the preview mode") |
There was a problem hiding this comment.
🤖 nit 🟢 – Saying "if not using the preview mode" does not name which gate allows omitting --snapshot-location, so a caller running --preview=ExternalVolumes gets a misleading error. Naming --preview=SnapshotStorageLocation and SNAPSHOT_LOCATION here (matching cmd/ateapi/main.go:175) makes the fix clear. Also consider running clusterSnapshotLocation up front in DeployAteSystem and DeployAteAPIServer (cmd/ate-setup/internal/steps/deploy.go:31) alongside CredentialProvider and CheckCSIDriver, since CreateAPIServerEnvVars runs only after creating the namespace, CRDs, RBAC, and CA/JWT Secrets.
| // | ||
| // +k8s:required | ||
| // +ate:preview=SnapshotStorageLocation | ||
| // +k8s:optional |
There was a problem hiding this comment.
🤖 question 🟢 – With storage_location now optional (and ignored without the gate), every field in SnapshotConfig is either optional or populated by defaults.Apply (cmd/ateapi/internal/defaults/defaults.go:25). ActorTemplate.snapshot_config (pkg/proto/ateapipb/ateapi.proto:1145) is still +k8s:required, so a template that uses all snapshot defaults still has to specify snapshotConfig: {}. Should ActorTemplate.snapshot_config also become +k8s:optional (with defaults.Apply initializing &ateapipb.SnapshotConfig{} when nil), or is requiring snapshotConfig: {} intentional?
There was a problem hiding this comment.
No, we can make this optional. Done
| // newInProgressSnapshotURI is where the snapshot an actor is currently taking is | ||
| // written: under the actor's own prefix, so the objects name their owner. | ||
| func newInProgressSnapshotURI(actorTemplate *ateapipb.ActorTemplate, actor *ateapipb.Actor, snapshotName string) (resources.SnapshotURI, error) { | ||
| func (w *ActorWorkflow) newInProgressSnapshotURI(ctx context.Context, actorTemplate *ateapipb.ActorTemplate, actor *ateapipb.Actor, snapshotName string) (resources.SnapshotURI, error) { |
There was a problem hiding this comment.
🤖 nit 🟢 – ctx is unused in newInProgressSnapshotURI and can be dropped from its signature and both call sites (cmd/ateapi/internal/controlapi/workflow_suspend.go:219 and 342).
| BucketName string | ||
|
|
||
| // SnapshotLocation is ate-api-server's --snapshot-location | ||
| // (SNAPSHOT_LOCATION). |
There was a problem hiding this comment.
Maybe mention the effects of this flag? possibly a shorter version of what you have in cmd/ate-setup/internal/config/setting.go
| return location, nil | ||
| } | ||
| // Snapshot location should always be set by --snapshot-location flag when not using preview mode. | ||
| if !previewGateEnabled(gates, preview.GateSnapshotStorageLocation) { |
There was a problem hiding this comment.
I think the name SnapshotStorageLocation is too general and does not convey what this gate actually enables. WDYT of PerTemplateSnapshotLocation?
There was a problem hiding this comment.
The name sounds good to me, updated this.
| if errs := resources.ValidateHostPort(field.NewPath("default-egress-gateway-address"), *defaultEgressGatewayAddress); len(errs) > 0 { | ||
| serverboot.Fatal(ctx, "Invalid --default-egress-gateway-address", errs.ToAggregate()) | ||
| } | ||
| if *snapshotLocation != "" { |
There was a problem hiding this comment.
Are there downsides to making --snapshot-location always required with the gate only letting a template override it?
It would simplify the logic:
- removes the removes the
FailedPreconditionpath - the extra check in
CreateActorTemplate - the gate logic in ate-setup
There was a problem hiding this comment.
If a user wants to use the snapshotConfig.storageLocation preview feature and accidentally misses the --preview flag then the command will fail with FailedPrecondition if there is no default set, with default though it will just fall back to the default location. I wonder if we should keep the explicit failure path for users so it is not failing/re-routing silently. --snapshot-location is a temporary work around till we go the final part to add snapshot specific config so that's another reason might having a non-required flag might be easier to deprecate?
There was a problem hiding this comment.
If --snapshot-location is temporary, then yeah I guess it should not be mandatory
|
| # use default bucket name for local deployment | ||
| export BUCKET_NAME="ate-snapshots" | ||
| # cluster-wide snapshot location ate-api-server is installed with | ||
| export SNAPSHOT_LOCATION="${SNAPSHOT_LOCATION:-gs://${BUCKET_NAME}/}" |
There was a problem hiding this comment.
🤖 Kind installs can pick up the developer's GCS snapshot location.
hack/ate-dev-env.sh.example now exports SNAPSHOT_LOCATION="gs://${BUCKET_NAME}/". If a developer has sourced their dev env and runs this script, ${SNAPSHOT_LOCATION:-...} keeps the shell's value, for example gs://snapshot-substrate-test-<project>/. Two lines up we hard-set BUCKET_NAME="ate-snapshots", and line 47 unsets other dev-env variables for exactly this reason, but SNAPSHOT_LOCATION slips through. ate-setup writes it into the api-server ConfigMap, and the server points at a bucket that doesn't exist in kind. The in-cluster rustfs only creates ate-snapshots (manifests/ate-install/kind/rustfs.yaml:121). The install succeeds, but every suspend and tag fails at upload time. run-e2e-kind.sh:40 and run-microvm-demo-kind.sh:42 have the same pattern.
A related problem: running go run ./cmd/ate-setup deploy on kind without these scripts fails with "--snapshot-location must be set". applyKindDefaults in cmd/ate-setup/internal/config/load.go defaults storage.bucketName to ate-snapshots and ignores the ambient value, but it gives storage.snapshotLocation no default.
I think the fix is to add the same default there:
if !namedExplicitly(r, "storage.snapshotLocation") {
r.set("storage.snapshotLocation", "gs://ate-snapshots/", OriginKindProfile)
}and then drop the SNAPSHOT_LOCATION lines from the three kind scripts. A case in load_test.go for the kind profile ignoring an ambient SNAPSHOT_LOCATION would cover it.
There was a problem hiding this comment.
Updated the logic as per recommendation
Sneha-at (Sneha-at)
changed the title
Sneha-at (Sneha-at)
changed the title
42cf9ba to
1b3bf00
Compare
| # default bucket name for local deployment (served by in-cluster rustfs) | ||
| export BUCKET_NAME="${BUCKET_NAME:-ate-snapshots}" | ||
| # cluster-wide snapshot location ate-api-server is installed with | ||
| export SNAPSHOT_LOCATION="${SNAPSHOT_LOCATION:-gs://${BUCKET_NAME}/}" |
There was a problem hiding this comment.
🤖 should-fix 🟡 – ate-setup --kind (applyKindDefaults in cmd/ate-setup/internal/config/load.go:359-366) ignores ambient BUCKET_NAME and SNAPSHOT_LOCATION and installs ate-api-server with gs://ate-snapshots/. If a developer has sourced .ate-dev-env.sh in their shell (SNAPSHOT_LOCATION="gs://snapshot-substrate-test-${PROJECT_ID}/", BUCKET_NAME="snapshot-substrate-test-${PROJECT_ID}"), ${SNAPSHOT_LOCATION:-...} and ${BUCKET_NAME:-...} here keep the cloud GCS values, causing wantSnapshotLocation in TestActorSnapshotLifecycle (internal/e2e/suites/demo/demo_test.go:828) to assert against the cloud SNAPSHOT_LOCATION while the kind cluster's ate-api-server writes under gs://ate-snapshots/.
Set both unconditionally (matching hack/install-ate-kind.sh:34):
export BUCKET_NAME="ate-snapshots"
export SNAPSHOT_LOCATION="gs://${BUCKET_NAME}/"There was a problem hiding this comment.
Updated this set both unconditionally
| // snapshot_config is optional; an omitted one takes every default. | ||
| if in.SnapshotConfig == nil { | ||
| in.SnapshotConfig = &ateapipb.SnapshotConfig{} | ||
| } |
There was a problem hiding this comment.
🤖 should-fix 🟡 – Initializing SnapshotConfig when nil belongs inside applyActorTemplateDefaults in cmd/ateapi/internal/defaults/defaults.go:50-58 rather than here before defaults.Apply(in). applyActorTemplateDefaults still returns early when t.SnapshotConfig == nil (with the stale comment "Wait for validation to catch missing required field" and test case "missing snapshot_config is left for validation" in cmd/ateapi/internal/defaults/defaults_test.go:41), which means defaults.Apply and atepg.unmarshalStored (cmd/ateapi/internal/store/atepg/atepg.go:390) do not backfill SnapshotConfig on their own.
|
|
||
| The kind scripts (`hack/install-ate-kind.sh`, `hack/run-e2e-kind.sh`, | ||
| `hack/run-microvm-demo-kind.sh`) set `SNAPSHOT_LOCATION` to | ||
| `gs://$BUCKET_NAME/` unless it is already set. On GKE, ate-setup does not |
There was a problem hiding this comment.
🤖 nit 🟢 – This paragraph still says the kind scripts (hack/install-ate-kind.sh, hack/run-e2e-kind.sh, hack/run-microvm-demo-kind.sh) set SNAPSHOT_LOCATION to gs://$BUCKET_NAME/, which is outdated now that applyKindDefaults (--kind profile) defaults storage.snapshotLocation to gs://<storage.bucketName>/ (gs://ate-snapshots/).
Also consider adding snapshotLocation: gs://ate-snapshots/ under storage: in docs/examples/substrate-install.yaml:50-52, since that example is a non-kind config and ate-setup deploy ate-system --config docs/examples/substrate-install.yaml now fails without storage.snapshotLocation.
| loadEnv(t) | ||
| t.Setenv("BUCKET_NAME", "ambient-cloud-bucket") | ||
| t.Setenv("KO_DEFAULTPLATFORMS", "linux/ambient") | ||
| t.Setenv("SNAPSHOT_LOCATION", "gs://ambient-cloud-bucket/") |
There was a problem hiding this comment.
🤖 nit 🟢 – Also add "SNAPSHOT_LOCATION" to loadEnv in cmd/ate-setup/internal/config/config_test.go:44-95 (so other config tests don't inherit an ambient SNAPSHOT_LOCATION from the developer's shell) and "storage.snapshotLocation": "gs://ate-snapshots/" to kindDefaults / TestKindRecordReplaysToTheSameInstall in cmd/ate-setup/internal/config/kinddefaults_test.go:25-30,127-132.
Bowei Du (bowei)
added
area/storage
kind/feature
Add the SnapshotStorageLocation preview gate. The ActorTemplate's snapshotConfig.storageLocation is now required only when the gate is enabled.
Snapshots are stored under the operator-set --snapshot-bucket-prefix. A template's snapshotConfig.storageLocation is honored only with the SnapshotStorageLocation preview gate enabled, falling back to the flag. ate-api-server refuses to start without the flag while the gate is off. resolveSnapshotLocation picks the location. It reads the process-wide gate rather than the per-request opt-out, so a request cannot move where an actor's snapshots live. ServiceImpl and ActorWorkflow carry the prefix, and the UpdateActor repoint check, suspend, and tag paths resolve through it. The manifest passes --snapshot-bucket-prefix=@env, which ate-api-server resolves from ATE_API_SNAPSHOT_BUCKET_PREFIX. ate-setup sets that to gs://$BUCKET_NAME/ in the ate-api-server-envvars ConfigMap, and fails if BUCKET_NAME is empty while the gate is off. Update the existing callers of NewRPCService for its new argument, and the CreateAPIServerEnvVars tests to set a bucket. The controlapi tests enable the gate in TestMain, since their workflows store snapshots under the template's storageLocation.
Cover resolveSnapshotLocation with the gate on and off, the UpdateActor repoint check with the gate off (template locations are ignored, and a missing --snapshot-bucket-prefix fails), the ATE_API_SNAPSHOT_BUCKET_PREFIX value ate-setup writes to the ate-api-server-envvars ConfigMap, and resolving --snapshot-bucket-prefix from @env. Update the storage_location validation test comment to say the field is ignored in favor of the flag when the gate is off. Check that moving an actor's snapshot location strands nothing: the next suspend releases the replaced snapshot under its old location, and deleting the actor collects the new one. The demo e2e suite asserts that actor and tag snapshots land under the template's storageLocation with the gate, and under gs://$BUCKET_NAME/ without it, so a miswired gate fails one of the two preview lanes.
Operators need to know that ate-api-server now stores snapshots under --snapshot-bucket-prefix, that ate-setup derives it from BUCKET_NAME without creating the bucket, and that a template's storageLocation is honored only with the preview gate. Moving the location strands an actor's earlier snapshots and blocks template updates, so say so where the layout and UpdateActor rules are described.
The preview stack now spells "enable every gate" as --preview=ALL instead of "*", so ate-setup's gate check and the tests use preview.All / "ALL". Two ate-setup env-var tests added on main build a config without a bucket; give them one so they reach the Postgres checks they exercise instead of stopping at the BUCKET_NAME requirement.
A request that does not send the preview opt-in header is meant to be treated as if every gate were off, but storageLocation is honored by the process-wide SnapshotStorageLocation gate, so a client that never opted in could still choose where snapshots are stored. With the gate on, validation now forbids storageLocation on requests that did not opt in; with it off the field stays ignored, so templates that set it keep working on servers without the gate. CreateActorTemplate also rejects, with FailedPrecondition, a template whose actors would have no snapshot location at all, instead of failing their first suspend. Mark the field with +ate:preview=SnapshotStorageLocation, document the opt-in rule, and cover the gate and opt-in combinations in validation, controlapi and functional tests. Also fix a functional test that still read the removed ExternalVolume status field, so the package builds.
The cluster-wide snapshot location is now a full object-storage URI, so it is not tied to GCS. ate-setup takes it from SNAPSHOT_LOCATION rather than deriving gs://$BUCKET_NAME/, and validates it like snapshotConfig.storageLocation. snapshotConfig.storageLocation is optional even with the SnapshotStorageLocation preview gate: a template that leaves it empty uses --snapshot-location.
…xemption TestActorSnapshotLifecycle reads SNAPSHOT_LOCATION to check where snapshots are stored when the PerTemplateSnapshotLocation preview gate is off. ate-setup now defaults the location for kind installs, so the variable is no longer exported to the e2e test process. Export it from run-e2e-kind.sh, matching the kind default ate-setup installs with. ActorTemplate.snapshot_config now has a doc comment, so its "documented" exemption no longer matches any finding.
… snapshot env applyActorTemplateDefaults now fills in a nil SnapshotConfig, so stored templates read back through atepg get it backfilled as well, not only templates created through the RPC handler. run-e2e-kind.sh sets BUCKET_NAME and SNAPSHOT_LOCATION unconditionally: ate-setup's kind profile ignores the ambient values and installs with gs://ate-snapshots/, so the e2e tests must expect the same location even when the developer's shell has sourced .ate-dev-env.sh. Also document the kind default in operator-install.md, add snapshotLocation to the example install config, and cover storage.snapshotLocation in the config tests.
eeb8621 to
be4bd12
Compare
… data plane (agent-substrate#2686) Adds what it takes to measure ate-api-server's and Postgres's cold resume capacity on the fake data plane from the benchmark automation, plus the first tests that use it. - **boomer: record server-side latency.** `resumepaused` records `locust_server_duration_milliseconds` from the `x-server-elapsed-us` trailer that `ateinterceptors.ServerUnaryInterceptor` sets, next to the client latency. The gap between the two is network and client time. - **Scrape boomer in automation runner Jobs.** The runner container declares port 8001 and `monitoring.yaml`'s `boomer-worker` job keeps `substrate-benchmark-runner` pods, so automated runs get the same latency histograms as manual ones. - **Shape the control plane for capacity runs.** A locust test can carry a `controlPlane:` block (`benchmarking/automation/controlplane.py`): - `apiServer:` sets ate-api-server's replicas, CPU and memory (requests = limits) and its Postgres pool size. - `postgres:` loads `pg_stat_statements` with a `postgres_exporter` sidecar that Prometheus scrapes, and passes extra Postgres settings. Placement is left to the install: capacity tests install with `--cordon-control-plane`, which keeps ate-api-server and Postgres on their own tainted pools, away from the load. The block patches the installed manifests after the deploy; a TODO in `controlplane.py` records the drift guards that shim still lacks. - **Summarize ladder runs per step, on the runner.** For a ladder run, `benchmarking/locust/ladder_steps.py` reads each step's boundaries from the runner's log, queries the in-cluster Prometheus over the step's steady window, and appends a `ladder_step_NNN` row per step to `stats.jsonl`: throughput, client and server latency, CPU and memory of ate-api-server, Postgres and fake-atelet, and Postgres connections, commits and waits. It is additive, like the server telemetry harvest it follows: a failure is logged and the run's other results are kept. - **Cold resume tests.** `tests.yaml` adds two: - `ateapi_cold_resume_pause_fake_2cpu`: a 100–1500/s cold resume ladder against one 2 CPU replica. - `ateapi_cold_resume_pause_fake_smoke`: the same setup at 50 actors and 20 → 50/s, a quick end-to-end check. Each actor books 10m CPU, since a fake Worker reports its node's allocatable as its capacity and placement must never bind. The benchmarking README explains the per-step rows. **Running them.** The target cluster needs the `ate-control-plane` and `ate-postgres` node pools `--cordon-control-plane` expects, and a node with room for the runner. On current `main` the target cluster's env also needs `SNAPSHOT_LOCATION` (agent-substrate#2544) and `ATE_INGRESS_ALLOWED_SPIFFE_IDS` (agent-substrate#2390), which every install now requires. Addresses agent-substrate#2261 - [x] Tests pass (`go test -race ./internal/benchmarking/...`, the `benchmarking/automation` and `benchmarking/locust` Python tests, `hack/verify-all.sh`) - [x] Appropriate changes to documentation are included in the PR
| // on servers without the gate. Instead, with the gate on, a request that | ||
| // did not opt in to preview features is forbidden from setting it. | ||
| func ValidateCustom_SnapshotConfig_StorageLocation(ctx context.Context, _ operation.Operation, fldPath *field.Path, value, _ *string) field.ErrorList { | ||
| if preview.IsEnabled(preview.GatePerTemplateSnapshotLocation) && preview.IsOptedOut(ctx) { |
There was a problem hiding this comment.
This should be a DV tag in the proto file - is there a reason why it is here instead? E.g. // +k8s:ifDisabled(PerTemplateSnapshotLocation)=+k8s:forbidden
From Substrate v0.6.0, ate-setup refuses to deploy unless SNAPSHOT_LOCATION names a cluster-wide snapshot location, or the PerTemplateSnapshotLocation preview gate is on (agent-substrate/substrate#2544). A v0.6.0 install stopped at "Turn on Substrate" with "--snapshot-location (SNAPSHOT_LOCATION) must be set". The shared environment now passes SNAPSHOT_LOCATION=gs://<bucket>/, the root of the install's snapshot bucket, as upstream's hack/ate-dev-env.sh.example sets it. The upgrade hand-over exports it too. Older trees ignore it. An exported SNAPSHOT_LOCATION is not honored, for the same reason BUCKET_NAME is not: setup-gcp grants the transfer account access to that bucket only. With the preview gate off, ate-api-server ignores a template's snapshotConfig.storageLocation, which the v0.6.0 demo templates still set, so the demos store under this location. This is the last of v0.6.0's pre-deploy checks for deploy ate-system. The namespace, credential provider, ingress allowlist, transfer account, CSI driver and worker-pod checks are already met. Co-Authored-By: Claude Opus 5.5 <[email protected]>
From Substrate v0.6.0, ate-setup refuses to deploy unless SNAPSHOT_LOCATION names a cluster-wide snapshot location, or the PerTemplateSnapshotLocation preview gate is on (agent-substrate/substrate#2544). A v0.6.0 install stopped at "Turn on Substrate" with "--snapshot-location (SNAPSHOT_LOCATION) must be set". The shared environment now passes SNAPSHOT_LOCATION=gs://<bucket>/, the root of the install's snapshot bucket, as upstream's hack/ate-dev-env.sh.example sets it. The upgrade hand-over exports it too. Older trees ignore it. An exported SNAPSHOT_LOCATION is not honored, for the same reason BUCKET_NAME is not: setup-gcp grants the transfer account access to that bucket only. With the preview gate off, ate-api-server ignores a template's snapshotConfig.storageLocation, which the v0.6.0 demo templates still set, so the demos store under this location. This is the last of v0.6.0's pre-deploy checks for deploy ate-system. The namespace, credential provider, ingress allowlist, transfer account, CSI driver and worker-pod checks are already met.
…-substrate#3357) Follow-up to agent-substrate#2544, from review comment agent-substrate#2544 (comment). `snapshotConfig.storageLocation` was accepted and ignored while the `PerTemplateSnapshotLocation` gate was off. A user could name a bucket, get a success response, and have snapshots written under the cluster `--snapshot-location`. ### What changed? Tested on kind with the cluster `--snapshot-location=gs://ate-snapshots/`. `<loc>` is the template's `storageLocation`. | Gate | Case | Before (`7e6476cbd`) | This PR | |---|---|---|---| | ON | with `storageLocation`, `--preview` | created, snapshot under `<loc>` | created, snapshot under `<loc>` | | ON | with `storageLocation`, no `--preview` | rejected: `Forbidden: requires a client that opts in to preview features ...` | rejected: `Forbidden` | | ON | without `storageLocation` | created, snapshot under `gs://ate-snapshots/` | same | | OFF | with `storageLocation`, `--preview` | created, but snapshot under `gs://ate-snapshots/`, not `<loc>` | rejected: `Forbidden` | | OFF | with `storageLocation`, no `--preview` | created, field ignored | rejected: `Forbidden` | | OFF | without `storageLocation` | created, snapshot under `gs://ate-snapshots/` | same | ### API validation - With preview off, a template that sets `storageLocation` is rejected with `InvalidArgument`. Before, it was accepted and silently ignored. - The old custom check is replaced by the standard ifDisabled forbidden rule that other preview fields use. ### Demos and benchmarks - No demo or benchmark template sets `storageLocation` any more. They all store snapshots under the cluster `--snapshot-location`. - `${BUCKET_NAME}` templating is gone from the demo rendering and the benchmark deploy scripts. The traffic-driver script loses `--bucket-name`. ### Install (ate-setup) - The comments and operator docs now say that the demos, benchmarks and e2e need `SNAPSHOT_LOCATION` even with the gate on. ### E2E fixtures and suites - With the gate on, the fixtures, combinedvolumes, and the demo, golden and template-update tests each set their own per-template location through `e2e.SnapshotLocation`. With it off, they leave the field out. - Templates copied from a demo also get their own location with the gate on. This brings per-template coverage back to the networkpolicy and egress suites. ### Unit tests - Validation tests now expect `Forbidden` when preview is off. Length and URI checks run with preview on. - The control API tests no longer rely on a default `storageLocation`. They set the cluster default the way nearby tests do. - The demo manifest tests and the kubectl-ate template tests fail if a shipped manifest sets `storageLocation` again. ### Docs - The API guide, operator install docs, demo READMEs, benchmarking README and the preview gate doc no longer mention bucket templating. They describe the field as preview-only. --------- Signed-off-by: Sneha Aradhey <[email protected]>
Breaking change
ate-api-server now stores snapshots under a new, operator-set
--snapshot-location(for example
gs://BUCKET/orgs://BUCKET/PREFIX/).ActorTemplate.snapshotConfig.storageLocationis now a preview field, honored only with
--preview=PerTemplateSnapshotLocation.snapshotConfigitself is now optional; an omitted one takes every default.What you must do when upgrading:
SNAPSHOT_LOCATION(--snapshot-location/storage.snapshotLocation).ate-setup passes it to ate-api-server and, unless the
PerTemplateSnapshotLocationpreview gateis enabled, fails before touching the cluster if it is unset.
BUCKET_NAMEalone is not enough.The kind profile defaults it to
gs://$BUCKET_NAME/(gs://ate-snapshots/). ate-setup does notcreate the bucket or grant access to it on GKE (
go run ./tools/setup-gcp bootstrapdoes).--snapshot-location(orATE_API_SNAPSHOT_LOCATIONwith
--snapshot-location=@env). Without it, the server refuses to start unless thePerTemplateSnapshotLocationpreview gate is enabled.storageLocation: without the gate, that value is ignored and newsnapshots and tags go under
--snapshot-location. If you rely on per-template bucketsor paths (IAM, retention, billing), enable
--preview=PerTemplateSnapshotLocationto keep theold behavior. With the gate, the field stays optional: a template that leaves it empty uses
--snapshot-location, andCreateActorTemplatefails withFailedPreconditionif neither is set.storageLocationwith the gate enabled: they must opt in to previewfeatures (
kubectl ate --preview, or theAte-Preview-Allowed: trueheader). OtherwiseCreateActorTemplatefails withInvalidArgument.location changes (by upgrading with the gate off, changing the flag, or toggling the gate),
its next suspend writes under the new location and releases the previous snapshot from the
old one. Until that suspend, changing the actor's
actor_templateis rejected, and deleting theactor collects its snapshots under the old location. If releasing the previous snapshot
fails, its objects stay under the old location and a later delete does not collect them;
ate-api-server logs a warning naming the snapshot URI. To keep the old locations, enable the
gate before upgrading.
ActorTemplate is not the right place to specify snapshot location. Bucket is an implementation detail of snapshots, workloads shouldn’t need to care about it. It is the responsibility of substrate admins to provision the buckets and give access to Substrate. This PR adds a cluster wide storage location as a building step towards the final goal of adding these details in a separate SnapshotStorageConfig.