[Bug bash] Make sure each release creates sandbox config to enable rollback by zoez7 · Pull Request #3254 · agent-substrate/substrate

Zoe Zhao (zoez7) · 2026-10-26T22:57:54Z

Fixes #3253, where each release updated the one class default SandboxConfig in place, so after an upgrade the cluster had no previous config to roll back to.

ate-setup deploy sandboxconfig and deploy ate-system render manifests/ate-install/sandboxconfig-gvisor.yaml.tmpl into substrate-<version>-gvisor. --add-default-annotation (default true) adds the class-default annotation with a separate merge patch, so a later apply without the flag leaves it in place. If a newer config of the class already has the annotation, that one stays the default and the installer warns.
hack/install-microvm-deps.sh --install does the same for substrate-<version>-microvm.
install-microvm-deps.sh --delete is gone. Like the gVisor configs, microVM configs go with the SandboxConfig CRD. ate-setup delete all, delete benchmarks and the benchmark orchestrator no longer call it.

MicroVM assets still live at fixed paths in the bucket, so a microVM release that changes them cannot keep the previous version fetchable. docs/upgrade.md says so. Moving them to per-version paths is a separate change.

AI use: AI-assisted with Claude Code. I directed the design and reviewed the diff. A separate agent session reviewed it adversarially, and I validated its findings and fixed the confirmed ones.

On upgrade, pools without a configRef move from gvisor-default to substrate-<release>-gvisor without rolling their workers. gvisor-default stays until deleted. Scripts that call hack/install-microvm-deps.sh --delete must drop the call.

Tests pass
Docs updated

Haven Xia (HavenXia)

#2092 deletes the entirehack/install-microvm-deps.sh and moves microVM staging into Go, with per-version asset paths.

We might want to land #2092 first and create the microVM config in Go instead of bash (which seems to be the general direction for GA)

Haven Xia (HavenXia) · 2026-10-27T05:36:27Z

 ```bash
-go run ./cmd/ate-setup deploy sandboxconfig --config ~/ate-upgrade/old.yaml
+ROLLBACK=substrate-$NEW_RELEASE-gvisor-rollback
+sed "s/\${SANDBOXCONFIG_NAME}/$ROLLBACK/" manifests/ate-install/sandboxconfig-gvisor.yaml.tmpl | kubectl apply -f -


as we said in chat this should be changing the annotation like kubectl annotate sandboxconfig substrate-$NEW_RELEASE-gvisor sandboxconfig.ate.dev/is-class-default-?

Done in 0649891.

Benjamin Elder (BenTheElder)

I think the overall direction makes sense but there are some issues currently.

Benjamin Elder (BenTheElder) · 2026-10-28T06:21:03Z

+	// Enforce per-class SandboxConfig asset requirements. This is applied
+	// before any SandboxConfig so the config below is validated too.
+	if err := e.Kube.ApplyPath(ctx, e.Cfg.Manifest("sandboxconfig-validation.yaml")); err != nil {


With older releases' configs now staying in the cluster, I think the asset policy becomes a problem. It checks every version on every UPDATE, so once a release tightens it (like #2798 dropping the bare runsc asset), every update to an older release's config is denied, including atecontroller removing the protection finalizer, so deleting that config later hangs in Terminating. The rollback guide's "copy the version into the old config" step gets rejected too.

Before this, the one config was re-applied to match the new policy. Can we make the policy skip versions that are unchanged from oldObject?

Fixed in c9fe37e. I updated the VAP to check changedVersions only, so older specs can still be updated.

Benjamin Elder (BenTheElder) · 2026-10-28T06:21:03Z

 #### Roll back step 4

-Tell actor owners to stop using API fields new in this release, because the old API server rejects them. Then:
+Tell actor owners to stop using API fields new in this release, because the old API server rejects them. Roll back the SandboxConfigs as in [Roll back](sandboxconfig-upgrade.md#roll-back) in the SandboxConfig upgrade guide. Then:


deploy sandboxconfig was the only thing that applies sandboxconfig-validation.yaml, so dropping it here leaves the new release's admission policy in place after a rollback. Should we keep running it from the old checkout, after removing the annotation from the new configs? For the old config it's a no-op re-apply.

The user might have updated the old config since the last time they install substrate. I updated the script to leave an existing config as is, so applying an old config is a no-op if it exists. I updated the playbook to restore the old admission policy.

Deleting the new configs makes the newest other annotated config the class default. If there was not an existing class-wide default in the cluster, the user will need to run deploy sandboxconfig from the old checkout to create it.

Benjamin Elder (BenTheElder) · 2026-10-28T06:21:03Z

 	})
 	config.BindCommandFlags(deployAteSystemPath, deployAteSystemCmd.Flags())
 	deployAteSystemCmd.Flags().Lookup("setup-csi").NoOptDefVal = "none"
+	deployAteSystemCmd.Flags().Bool(addDefaultAnnotationFlag, true, addDefaultAnnotationUsage)


Defaulting to true and deciding it again on every run means a later deploy ate-system for something unrelated, without the flag, annotates the config and moves every gVisor pool without a configRef. That's why the runbook has to warn about it in two places.

What if we only add the annotation when we create the config? Then a rerun of the same release leaves whatever the admin chose, and the flag only matters on the first apply.

If we only add the annotation when creating the config, we need to make sure deploy sandboxconfig keep adding the annotation. Otherwise a sandboxConfig rollback (from v2 to v1) won't add the default annotation back to old SandboxConfig.

I'm thinking of

If the user didn't specify the flag at all, and there is already an existing config, we do not modify the annotation.

Otherwise, if the user didn't specify the flag at all, and there is no existing config, we create one and add the annotation.

If there is already a config, and user explicitly passes in --add-default-annotation=false|true, we'll remove/add the annotation to the existing config.

Thinking about it again, the 3 above isn't right, because even if the config exists, it may not be the same content that will be applied by this change.

Fixed in 3a7653f and c9fe37e. The installers now add the annotation only when they create the config, and the flag is --create-with-default-annotation (default true). Rollback handles both cases. If the old config is gone, deploy sandboxconfig recreates it with the annotation. If it exists without the annotation, the rollback guide tells users to run kubectl annotate sandboxconfig substrate-$OLD_RELEASE-gvisor sandboxconfig.ate.dev/is-class-default=true.

Benjamin Elder (BenTheElder) · 2026-10-28T06:21:03Z

+// It fails when checkClassDefault does.
+func (e *Env) checkSandboxConfig(ctx context.Context) (string, error) {
+	class := atev1.SandboxClassGvisor
+	name, err := sandboxConfigName(e.SubstrateVersion(), class)


The name is only as unique as the version. Building from source that's git describe --dirty, so every commit deployed to a dev cluster creates another class default config that moves every pool and never goes away (and no doc says when an old one is safe to delete). With --image-repo and a moving tag like main it's the opposite, every install updates the same config in place and there's nothing to roll back to.

Did we consider naming it from a hash of the rendered spec, at least for non-release builds? Or ...?

I changed it to name from the hash for both release and non-release builds. The caveat is that SandboxConfigs are mutable, so the hash only represents what a substrate release intended to apply, and becomes outdated if the user mutates it. However I think this is a good thing, if the user has modified config-hash1, when next time we want to apply content matching config-hash1 again, the users modification will stay (because the script will only create a config if it doesn't yet exist)

Benjamin Elder (BenTheElder) · 2026-10-28T06:21:03Z

+// newerSandboxConfig reports whether a takes precedence over b as a class
+// default: it was created later, or in the same second with a smaller name.
+// atecontroller applies the same rule.
+func newerSandboxConfig(a, b *atev1.SandboxConfig) bool {
+	if !a.CreationTimestamp.Equal(&b.CreationTimestamp) {
+		return b.CreationTimestamp.Before(&a.CreationTimestamp)
+	}
+	return a.Name < b.Name
+}


non-blocking: this is a copy of atecontroller's newerSandboxConfig (cmd/atecontroller/internal/controllers/workerpool_sandboxconfig.go:127), and check_configs in hack/install-microvm-deps.sh is a third copy in bash. If the controller's rule changes, these warnings quietly disagree with what pools actually resolve. Can we move the class default resolution into api/infra/k8s/ate/v1 and call it from both?

Fixed in 93edee0. The rule now lives in atev1.ClassDefaultSandboxConfig, and atecontroller, ate-setup and the e2e preflight call it. check_configs in hack/install-microvm-deps.sh is still a bash copy until #2092 moves microVM staging to Go.

Benjamin Elder (BenTheElder) · 2026-10-28T06:21:03Z

+	// The warning is about the cluster before the apply.
+	if _, err := checkClassDefault(configs, name, class); err != nil {


non-blocking: the warning is dropped here, despite the comment. DeployAteSystem also calls this up front and then DeploySandboxConfig calls it again, so one deploy runs git describe twice and lists every SandboxConfig three times. Can we compute the name once and pass it down?

Fixed in 98c62dc and 3a7653f. DeployAteSystem calls checkSandboxConfig once and passes the result to deploySandboxConfig. A deploy now runs git describe once and lists the SandboxConfigs twice, before and after the create. I removed the warning before the apply along with its comment. The check after the create still warns.

Benjamin Elder (BenTheElder) · 2026-10-28T06:21:03Z

  # (rootfs/) and durable-dir volumes (fs/) as separate bundles, and to run the
  # sandbox with no root container (google/gvisor#15551).
-  - name: v5
+  - name: gvisor-5


why rename v5 to gvisor-5? It doesn't break restore, atelet matches on the asset digests, but it isn't needed for the per-release names and the comment above says not to edit listed versions.

Reverted this change.

Haven Xia (HavenXia) · 2026-10-28T20:59:50Z

+	if err != nil {
+		return err
+	}
+	if err := e.Kube.ApplyBytes(ctx, manifest); err != nil {


if someone sets a version to Disabled after a CVE, the next deploy ate-system of the same release puts it back to Enabled.

Should we skip a config that already exists?

Updated this PR to name the sandbox config from the hash. The caveat here is that SandboxConfigs are mutable, so the hash only represents what a substrate release intended to apply, and becomes outdated if the user mutates it. However I think this is a good thing - if the user has modified config-hash1 by setting a version Disabled, when next time we want to apply content matching config-hash1 again, the users modification will stay, because now we will not modify an existing config.

Haven Xia (HavenXia) · 2026-10-28T21:11:56Z

+
+Roll back the SandboxConfigs first in the runbook's [Roll back step 4](upgrade.md#roll-back-step-4), from the old checkout.
+
+Actors suspended on a version the old release's config does not list resume without their memory. To keep restoring snapshots taken on a version new in this release, copy that version's entry from the new release's config into the old one before you go on.


This .. introducing a new gvisor version in this release works with the old workers?

Keeping it saves the memory of new actors suspended on (gvisor-v2), but after steps 3 and 2 roll back, the old ateworker restores them with it.

But I don't think there's a policy that ateworker v.N can worker with sandboxConfig v.N+1. And it's hard to support this policy in development. Maybe we should just eat the pain that these actors will return to cold boot.

Benjamin Elder (@BenTheElder) WDYT

I agree. Removed the step. The rollback guide and the version skew policy now say an actor suspended on a new version the old config doesn't list will cold boot.

Every upgrade applied gvisor-default in place, so the previous release's config was gone after an upgrade. ate-setup now renders sandboxconfig-gvisor.yaml.tmpl into substrate-<version>-gvisor, and --add-default-annotation (default true) adds the class-default annotation. hack/install-microvm-deps.sh does the same for substrate-<version>-microvm. atecontroller uses the newest annotated config of a class. Both installers warn when the config gets the annotation but a newer annotated config exists, and when no config of the class has the annotation. They refuse a config that is being deleted. deploy ate-system checks this before it changes the cluster. install-microvm-deps.sh no longer deletes configs. Like the gVisor configs, they go with the SandboxConfig CRD. Signed-off-by: zoezhao <zoezhao@google.com>

Update the upgrade, rollback and version skew docs for the SandboxConfig each release creates. A rollback creates a new config from the old release's template and makes it the class default. Signed-off-by: zoezhao <zoezhao@google.com>

The class-default warning now reads the cluster after the apply, so it sees the config's real creation time and annotations. ate-setup passes VERSION to the scripts it runs, so the microVM config gets the same version as the gVisor one with prebuilt images. The upgrade runbook passes --add-default-annotation=false to step 4e as well, gives install-microvm-deps.sh the bucket and version, makes the rollback commands safe to rerun, and covers upgrading again after a rollback. Prose spells microVM. Signed-off-by: zoezhao <zoezhao@google.com>

docs/sandboxconfig-upgrade.md covers creating a release's SandboxConfigs, holding pools back with --add-default-annotation=false, and rollback. Step 4d of the runbook keeps the default command and links to it. The old release's config keeps its class-default annotation through the upgrade. Rollback now removes the annotation from the new release's config, so the old one is the class default again, instead of creating a separate rollback config. Signed-off-by: zoezhao <zoezhao@google.com>

atecontroller, ate-setup and the e2e preflight each picked the class default SandboxConfig with their own copy of the rule. ClassDefaultSandboxConfig now holds it, and all three call it. Signed-off-by: zoezhao <zoezhao@google.com>

Naming every build's SandboxConfig after its version gave each dev commit a new class default, and gave a moving tag like main one config updated in place. Only a release tag now names the config after the version. Any other build gets substrate-dev-<hash>-<class>, hashed from the rendered config, so builds with the same config share one. ate-setup and install-microvm-deps.sh create the config only when it does not exist, so a rerun keeps edits such as a disabled version. They leave the annotations of an existing config alone too. --add-default-annotation becomes --create-with-default-annotation. It defaults to true and applies only to a config the run creates. deploy ate-system computes the config once and passes it to deploySandboxConfig. The templates go back to the version names on main. Signed-off-by: zoezhao <zoezhao@google.com>

…licy Configs of earlier releases now stay in the cluster. The policy checked every version on every update, so once a release tightened it, an older config could not be updated, and atecontroller could not remove its finalizer. On update the policy now checks only versions that are new or whose assets changed, and every version when sandboxClass changes. Rollback deletes the new release's configs instead of removing their class-default annotation, so upgrading again creates them anew. It then restores the old release's policy once the deletes finish. The guide also says when to delete earlier configs, and no longer copies versions into the old config. Signed-off-by: zoezhao <zoezhao@google.com>

A release tag named its config after the version, and only other builds used the hash. Telling the two apart meant parsing git describe output. Every build and release now gets substrate-<hash>-<class>, so a release that does not change a template reuses the existing config, and pools do not move. The upgrade can no longer predict the new config's name, and it may create none. So a created config is labeled sandboxconfig.ate.dev/created-by-version with the Substrate version, and rollback deletes the configs labeled with the new release. The label is filled in after hashing, so it does not change the name. ate-setup and install-microvm-deps.sh fail early when the version is not a valid label value. Signed-off-by: zoezhao <zoezhao@google.com>

Zoe Zhao (zoez7) force-pushed the sbx-config-upgrade branch 2 times, most recently from 0007396 to 95dbed2 Compare October 27, 2026 00:15

Zoe Zhao (zoez7) marked this pull request as ready for review October 27, 2026 00:38

Zoe Zhao (zoez7) requested review from Benjamin Elder (BenTheElder) and Haven Xia (HavenXia) October 27, 2026 00:38

Haven Xia (HavenXia) reviewed Oct 27, 2026

View reviewed changes

Zoe Zhao (zoez7) force-pushed the sbx-config-upgrade branch from 95dbed2 to a24c3ac Compare October 27, 2026 14:47

Zoe Zhao (zoez7) added this to the M3 milestone Oct 27, 2026

Zoe Zhao (zoez7) assigned Benjamin Elder (BenTheElder) Oct 27, 2026

Tim Hockin (thockin) added prio/P0 Highest priority / required for next milestone milestone-ack labels Oct 27, 2026

Bowei Du (bowei) added area/dev-infra area/node kind/bug Something isn't working / bugfixes labels Oct 27, 2026

Benjamin Elder (BenTheElder) reviewed Oct 28, 2026

View reviewed changes

Benjamin Elder (BenTheElder) mentioned this pull request Oct 28, 2026

ate-setup: port microvm-deps installation from shell to Go #2092

Open

Zoe Zhao (zoez7) force-pushed the sbx-config-upgrade branch from 0649891 to 842f824 Compare October 28, 2026 19:21

Haven Xia (HavenXia) reviewed Oct 28, 2026

View reviewed changes

Zoe Zhao (zoez7) force-pushed the sbx-config-upgrade branch 2 times, most recently from c9fe37e to d1571da Compare October 29, 2026 00:35

Zoe Zhao (zoez7) added 8 commits October 28, 2026 17:50

docs: describe per-release SandboxConfigs

10608d3

Update the upgrade, rollback and version skew docs for the SandboxConfig each release creates. A rollback creates a new config from the old release's template and makes it the class default. Signed-off-by: zoezhao <zoezhao@google.com>

atev1: share the class default rule

cc6bb9b

atecontroller, ate-setup and the e2e preflight each picked the class default SandboxConfig with their own copy of the rule. ClassDefaultSandboxConfig now holds it, and all three call it. Signed-off-by: zoezhao <zoezhao@google.com>

Zoe Zhao (zoez7) force-pushed the sbx-config-upgrade branch from d1571da to c631e51 Compare October 29, 2026 00:51

		// The warning is about the cluster before the apply.
		if _, err := checkClassDefault(configs, name, class); err != nil {


		Roll back the SandboxConfigs first in the runbook's [Roll back step 4](upgrade.md#roll-back-step-4), from the old checkout.

		Actors suspended on a version the old release's config does not list resume without their memory. To keep restoring snapshots taken on a version new in this release, copy that version's entry from the new release's config into the old one before you go on.

Conversation

Zoe Zhao (zoez7) commented Oct 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Haven Xia (HavenXia) left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Benjamin Elder (BenTheElder) left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Zoe Zhao (zoez7) Oct 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Zoe Zhao (zoez7) Oct 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Zoe Zhao (zoez7) commented Oct 26, 2026 •

edited

Loading

Zoe Zhao (zoez7) Oct 28, 2026 •

edited

Loading

Zoe Zhao (zoez7) Oct 28, 2026 •

edited

Loading