[Bug bash] Make sure each release creates sandbox config to enable rollback

README.md

@@ -274,6 +274,7 @@ We provide several sample applications demonstrating Agent Substrate's capabilit
 * [Request Parking](docs/request-parking.md): How the router parks requests through transient worker-pool saturation.
 * [Sandbox Asset Cache](docs/sandbox-asset-cache.md): How a node caches the sandbox binaries actors run on, decides which runtimes it needs, and evicts to a budget.
 * [Upgrade Runbook](docs/upgrade.md): Upgrade a running install to a newer supported release, and roll back if needed.
+* [SandboxConfig Upgrade](docs/sandboxconfig-upgrade.md): Move pools to a release's SandboxConfigs during an upgrade, and back on rollback.
 * [Node Storage](docs/node-storage.md): How to give snapshot I/O its own disk.
 * [Security Guide](docs/security.md): Security overview, security principles, and threat modeling.
 * [Benchmarking Guide](benchmarking/README.md): Locust-based load tests, monitoring stack, and the orchestrated benchmark harness.

api/infra/k8s/ate/v1/sandboxconfig_types.go

@@ -212,6 +212,25 @@ type SandboxConfigList struct {
 	Items           []SandboxConfig `json:"items"`
 }
 
+// ClassDefaultSandboxConfig returns the default SandboxConfig of class among
+// configs, or nil if there is none. It is the newest config of the class with
+// SandboxConfigClassDefaultAnnotation set to "true" that is not being deleted.
+// Of two created in the same second, the one whose name sorts first wins.
+func ClassDefaultSandboxConfig(configs []SandboxConfig, class SandboxClass) *SandboxConfig {
+	var best *SandboxConfig
+	for i := range configs {
+		sc := &configs[i]
+		if sc.Spec.SandboxClass != class || sc.Annotations[SandboxConfigClassDefaultAnnotation] != "true" || !sc.DeletionTimestamp.IsZero() {
+			continue
+		}
+		if best == nil || best.CreationTimestamp.Before(&sc.CreationTimestamp) ||
+			(sc.CreationTimestamp.Equal(&best.CreationTimestamp) && sc.Name < best.Name) {
+			best = sc
+		}
+	}
+	return best
+}
+
 func init() {
 	SchemeBuilder.Register(&SandboxConfig{}, &SandboxConfigList{})
 }

api/infra/k8s/ate/v1/sandboxconfig_validation_test.go

@@ -18,6 +18,7 @@ import (
 	"context"
 	"fmt"
 	"os"
+	"path/filepath"
 	"strings"
 	"testing"
 	"time"
@@ -136,6 +137,136 @@ func applyVAP(t *testing.T, ctx context.Context) {
 	}
 }
 
+// removeVAPBinding deletes the shipped policy's binding and waits until the
+// apiserver stops enforcing the policy, confirmed by a sentinel that must be
+// admitted.
+func removeVAPBinding(t *testing.T, ctx context.Context) {
+	t.Helper()
+	binding := &unstructured.Unstructured{}
+	binding.SetAPIVersion("admissionregistration.k8s.io/v1")
+	binding.SetKind("ValidatingAdmissionPolicyBinding")
+	binding.SetName("sandboxconfig-assets")
+	if err := client.IgnoreNotFound(k8sClient.Delete(ctx, binding)); err != nil {
+		t.Fatalf("delete binding: %v", err)
+	}
+	i := 0
+	err := wait.PollUntilContextTimeout(ctx, 200*time.Millisecond, 30*time.Second, true, func(ctx context.Context) (bool, error) {
+		i++
+		sc := sandboxConfig(fmt.Sprintf("vap-cooldown-%d", i), SandboxClassGvisor,
+			map[string]map[string]AssetFile{"amd64": {"other": otherAsset()}})
+		if err := k8sClient.Create(ctx, sc); err != nil {
+			return false, nil
+		}
+		return true, k8sClient.Delete(ctx, sc)
+	})
+	if err != nil {
+		t.Fatalf("VAP did not become inactive: %v", err)
+	}
+}
+
+// A config that an earlier, looser policy admitted can still be updated, as
+// long as the update adds no version and changes no version's assets.
+func TestSandboxConfigUpdateUnderStricterPolicy(t *testing.T) {
+	ctx := t.Context()
+	runscAssets := func() map[string]map[string]AssetFile {
+		return map[string]map[string]AssetFile{"amd64": {"runsc": otherAsset()}}
+	}
+	// The config before the update. Its versions v1 and v0 have a bare runsc
+	// asset, which the current policy rejects.
+	base := func(name string) *SandboxConfig {
+		sc := withVersions(sandboxConfig(name, SandboxClassGvisor, runscAssets()), sandboxVersion("v0", runscAssets()))
+		sc.Finalizers = []string{"test.ate.dev/hold"}
+		sc.Annotations = map[string]string{SandboxConfigClassDefaultAnnotation: "true"}
+		return sc
+	}
+
+	tests := []struct {
+		name   string
+		update func(sc *SandboxConfig)
+		errMsg string // empty when the update is allowed
+	}{{
+		name:   "remove a finalizer",
+		update: func(sc *SandboxConfig) { sc.Finalizers = nil },
+	}, {
+		name:   "remove the class-default annotation",
+		update: func(sc *SandboxConfig) { sc.Annotations = nil },
+	}, {
+		name:   "disable a version",
+		update: func(sc *SandboxConfig) { sc.Spec.Versions[1].State = ptr.To(SandboxVersionStateDisabled) },
+	}, {
+		name:   "delete a version",
+		update: func(sc *SandboxConfig) { sc.Spec.Versions = sc.Spec.Versions[:1] },
+	}, {
+		name:   "change defaultVersion",
+		update: func(sc *SandboxConfig) { withDefaultVersion(sc, "v0") },
+	}, {
+		name:   "add a valid version",
+		update: func(sc *SandboxConfig) { withVersions(sc, sandboxVersion("v2", gvisorAssets())) },
+	}, {
+		name:   "add an invalid version",
+		update: func(sc *SandboxConfig) { withVersions(sc, sandboxVersion("v2", runscAssets())) },
+		errMsg: gvisorAssetMsg,
+	}, {
+		name: "change a version's assets",
+		update: func(sc *SandboxConfig) {
+			sc.Spec.Versions[1].Assets["arm64"] = map[string]AssetFile{"runsc": otherAsset()}
+		},
+		errMsg: gvisorAssetMsg,
+	}, {
+		name:   "change sandboxClass",
+		update: func(sc *SandboxConfig) { sc.Spec.SandboxClass = SandboxClassMicroVM },
+		errMsg: "microvm SandboxConfig must define",
+	}}
+
+	// Create the configs while no policy is enforced, as an earlier release
+	// might have.
+	removeVAPBinding(t, ctx)
+	configs := make([]*SandboxConfig, len(tests))
+	for i := range tests {
+		configs[i] = base(fmt.Sprintf("stricter-policy-%d", i))
+		if err := k8sClient.Create(ctx, configs[i]); err != nil {
+			t.Fatalf("Create() %s: %v", configs[i].Name, err)
+		}
+	}
+	applyVAP(t, ctx)
+
+	for i, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			sc := configs[i]
+			t.Cleanup(func() {
+				ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
+				defer cancel()
+				latest := &SandboxConfig{}
+				if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(sc), latest); err != nil {
+					t.Errorf("cleanup: get SandboxConfig %s: %v", sc.Name, err)
+					return
+				}
+				latest.Finalizers = nil
+				if err := k8sClient.Update(ctx, latest); err != nil {
+					t.Errorf("cleanup: remove finalizer from SandboxConfig %s: %v", sc.Name, err)
+				}
+				if err := client.IgnoreNotFound(k8sClient.Delete(ctx, latest)); err != nil {
+					t.Errorf("cleanup: delete SandboxConfig %s: %v", sc.Name, err)
+				}
+			})
+			tt.update(sc)
+			err := k8sClient.Update(ctx, sc)
+			if tt.errMsg == "" {
+				if err != nil {
+					t.Fatalf("Update() unexpected error: %v", err)
+				}
+				return
+			}
+			if err == nil {
+				t.Fatalf("Update() succeeded, want denied")
+			}
+			if !strings.Contains(err.Error(), tt.errMsg) {
+				t.Errorf("Update() error = %q, want it to contain %q", err.Error(), tt.errMsg)
+			}
+		})
+	}
+}
+
 func TestSandboxConfigValidation(t *testing.T) {
 	ctx := t.Context()
 	applyVAP(t, ctx)
@@ -344,28 +475,30 @@ func TestSandboxConfigUpdateRemovingDefaultVersion(t *testing.T) {
 	}
 }
 
-// TestShippedSandboxConfigManifests guards the SandboxConfigs the install ships
-// against the CRD schema and the shipped ValidatingAdmissionPolicy.
+// TestShippedSandboxConfigManifests guards the SandboxConfig templates the
+// install renders against the CRD schema and the shipped
+// ValidatingAdmissionPolicy.
 func TestShippedSandboxConfigManifests(t *testing.T) {
 	ctx := t.Context()
 	applyVAP(t, ctx)
 
 	for _, path := range []string{
-		"../../../../../manifests/ate-install/sandboxconfig-gvisor.yaml",
+		"../../../../../manifests/ate-install/sandboxconfig-gvisor.yaml.tmpl",
 		"../../../../../manifests/microvm/sandboxconfig-microvm.yaml.tmpl",
 	} {
 		t.Run(path, func(t *testing.T) {
 			raw, err := os.ReadFile(path)
 			if err != nil {
 				t.Fatalf("read manifest: %v", err)
 			}
-			raw = []byte(strings.NewReplacer("${BUCKET_SCHEME}", "s3", "${BUCKET_NAME}", "test-bucket", "${ASSET_BUCKET_NAME}", "test-bucket").Replace(string(raw)))
+			raw = []byte(strings.NewReplacer("${SANDBOXCONFIG_NAME}", "test-"+filepath.Base(path), "${SUBSTRATE_VERSION}", "v0.4.2", "${BUCKET_SCHEME}", "s3", "${ASSET_BUCKET_NAME}", "test-bucket").Replace(string(raw)))
 			sc := &SandboxConfig{}
 			if err := yaml.UnmarshalStrict(raw, sc); err != nil {
 				t.Fatalf("decode manifest: %v", err)
 			}
-			if sc.Annotations[SandboxConfigClassDefaultAnnotation] != "true" {
-				t.Errorf("manifest is not annotated %s=true", SandboxConfigClassDefaultAnnotation)
+			// The installers add the annotation when they create the config.
+			if _, ok := sc.Annotations[SandboxConfigClassDefaultAnnotation]; ok {
+				t.Errorf("template sets %s; the installers add it", SandboxConfigClassDefaultAnnotation)
 			}
 			if err := k8sClient.Create(ctx, sc); err != nil {
 				t.Fatalf("Create() unexpected error: %v", err)

benchmarking/automation/README.md

@@ -32,7 +32,8 @@ To run the cold resume tests from your workstation instead, see
    Envoy ingress router. For a `microvm` test the orchestrator also
    runs `hack/install-microvm-deps.sh --install` between the two, which
    stages kata + cloud-hypervisor + virtiofsd assets to the cluster's object
-   store bucket and applies the cluster-wide `microvm` SandboxConfig.
+   store bucket and creates this release's microVM SandboxConfig as the
+   class default.
    For a `nighthawk-ingress` test the orchestrator additionally patches the
    `atenet-router` Deployment right after `deploy_substrate`: `envoyCpu`
    is the benchmark's independent variable and the shipped manifest sets
@@ -49,7 +50,8 @@ To run the cold resume tests from your workstation instead, see
      variable (a JSON object) if set. A `--metadata` in the test's own
      `flags` is merged in too, its keys winning on conflict.
    - Polls until complete/failed/timeout; tails logs; deletes the Job.
-   - Tears down workloads + microVM deps (if any) + substrate.
+   - Tears down workloads + substrate. Deleting substrate also removes the
+     microVM SandboxConfigs with their CRD.
    - If not the last test, redeploys them so the next run starts clean.
 
 ## Choosing a sandbox class

benchmarking/automation/orchestrator.py

@@ -421,19 +421,13 @@ def teardown_substrate() -> None:
 
 
 def install_microvm_deps() -> None:
-    """Stage kata/cloud-hypervisor assets and apply the cluster-wide
-    microvm SandboxConfig. Required before a microvm WorkerPool can run
-    actors; must run after deploy_substrate() (which installs the CRDs)."""
+    """Stage kata/cloud-hypervisor assets and create this release's microVM
+    SandboxConfig as the class default. Required before a microVM WorkerPool
+    can run actors; must run after deploy_substrate() (which installs the
+    CRDs)."""
     run(["hack/install-microvm-deps.sh", "--install"])
 
 
-def teardown_microvm_deps() -> None:
-    """Remove the microvm SandboxConfig. Must run before
-    teardown_substrate(), which deletes the SandboxConfig CRD (and would
-    prevent this from succeeding via kubectl)."""
-    run_no_check(["hack/install-microvm-deps.sh", "--delete"])
-
-
 def deploy_workloads(
     worker_count: int = 1,
     sandbox_class: str = "gvisor",
@@ -632,11 +626,9 @@ def main() -> None:
             # fire that crashed mid-test (or any other process that left
             # state behind) would otherwise leak its substrate + workloads
             # into this run. All teardowns use --ignore-not-found, so
-            # this is cheap on a clean cluster. Order matters:
-            # microvm-deps deletes a SandboxConfig CR, which requires the
-            # SandboxConfig CRD that teardown_substrate removes.
+            # this is cheap on a clean cluster. teardown_substrate also
+            # removes the microVM SandboxConfigs with their CRD.
             teardown_workloads()
-            teardown_microvm_deps()
             teardown_substrate()
 
             sandbox_class = test.get("sandboxClass", "gvisor")
@@ -678,10 +670,7 @@ def main() -> None:
             finally:
                 # Always tear down, even if deploy or run failed, so the
                 # next test (and the next CronJob fire) starts clean.
-                # microvm-deps must go before substrate for the same reason
-                # as above.
                 teardown_workloads()
-                teardown_microvm_deps()
                 teardown_substrate()
             duration = time.time() - start_time
             results.append((test["name"], status, duration, failure_msg))

cmd/ate-setup/commands.md

@@ -113,6 +113,12 @@ apiserver, the controller, atenet, and atelet. It creates every `create`
 resource below on the way, so those subcommands are only needed to redo one on
 a running cluster.
 
+`deploy ate-system` and `deploy sandboxconfig` create this build's gVisor
+SandboxConfig with the `sandboxconfig.ate.dev/is-class-default` annotation. It
+is named `substrate-<hash>-gvisor`, hashed from the rendered config. Pass
+`--create-with-default-annotation=false` to create it without the annotation.
+An existing config is left as it is, annotations included.
+
 ## Publish
 
 - `publish worker-images`

cmd/ate-setup/internal/cmd/deploy.go

@@ -84,6 +84,9 @@ Shape the install with the global --atenet-dataplane, --cluster-size,
 		if err != nil {
 			return err
 		}
+		if opts.CreateSandboxConfigWithDefaultAnnotation, err = cmd.Flags().GetBool(createWithDefaultAnnotationFlag); err != nil {
+			return err
+		}
 		return env.DeployAteSystem(cmd.Context(), opts)
 	},
 }
@@ -136,12 +139,36 @@ var deployPodCertControllerCmd = &cobra.Command{
 	},
 }
 
+// createWithDefaultAnnotationFlag names the flag that leaves the
+// class-default annotation off this build's gVisor SandboxConfig when false.
+// It is a plain flag rather than a recorded setting, because it applies only
+// to the run that creates the config.
+const createWithDefaultAnnotationFlag = "create-with-default-annotation"
+
+const createWithDefaultAnnotationUsage = "Add the sandboxconfig.ate.dev/is-class-default annotation to this build's gVisor SandboxConfig if this run creates it. An existing config is left as it is"
+
 var deploySandboxConfigCmd = &cobra.Command{
 	Use:   "sandboxconfig",
-	Short: "Deploy the SandboxConfig admission policy and the default gVisor SandboxConfig only",
-	Args:  cobra.NoArgs,
+	Short: "Deploy the SandboxConfig admission policy and this build's gVisor SandboxConfig only",
+	Long: `Deploy the SandboxConfig admission policy and create this build's gVisor
+SandboxConfig from manifests/ate-install/sandboxconfig-gvisor.yaml.tmpl. It is
+named substrate-<hash>-gvisor, hashed from the rendered config, so builds with
+the same config share one, and labeled
+sandboxconfig.ate.dev/created-by-version with the version that creates it.
+
+When this run creates the config, it gets the
+sandboxconfig.ate.dev/is-class-default annotation, unless
+--create-with-default-annotation=false. An existing config is left as it is,
+annotations included. Change its annotation with kubectl annotate. Every
+gVisor WorkerPool without a configRef follows the newest config with the
+annotation, so a warning names a newer one if it exists.`,
+	Args: cobra.NoArgs,
 	RunE: func(cmd *cobra.Command, _ []string) error {
-		return env.DeploySandboxConfig(cmd.Context())
+		create, err := cmd.Flags().GetBool(createWithDefaultAnnotationFlag)
+		if err != nil {
+			return err
+		}
+		return env.DeploySandboxConfig(cmd.Context(), create)
 	},
 }
 
@@ -181,4 +208,6 @@ func init() {
 	})
 	config.BindCommandFlags(deployAteSystemPath, deployAteSystemCmd.Flags())
 	deployAteSystemCmd.Flags().Lookup("setup-csi").NoOptDefVal = "none"
+	deployAteSystemCmd.Flags().Bool(createWithDefaultAnnotationFlag, true, createWithDefaultAnnotationUsage)
+	deploySandboxConfigCmd.Flags().Bool(createWithDefaultAnnotationFlag, true, createWithDefaultAnnotationUsage)
 }

cmd/ate-setup/internal/demos/countermicrovm/countermicrovm.go

@@ -14,7 +14,7 @@
 
 // Package countermicrovm installs the micro-VM variant of the counter demo.
 //
-// It needs the cluster-wide `microvm` SandboxConfig that
+// It needs the microVM class default SandboxConfig that
 // hack/install-microvm-deps.sh --install creates. It has no external-volume
 // option: the CSI path is covered by demo-counter.
 package countermicrovm

cmd/ate-setup/internal/demos/egressmicrovm/egressmicrovm.go

@@ -14,7 +14,7 @@
 
 // Package egressmicrovm installs the micro-VM variant of the egress demo.
 //
-// It needs the cluster-wide `microvm` SandboxConfig that
+// It needs the microVM class default SandboxConfig that
 // hack/install-microvm-deps.sh --install creates. It is a demo of its own
 // rather than a flag on demo-egress so that it appears in help and in the
 // `delete all` sweep, and so that both can be installed side by side: the

cmd/ate-setup/internal/images/release.go

@@ -48,7 +48,8 @@ const RevisionLabel = "org.opencontainers.image.revision"
 const VersionLabel = "org.opencontainers.image.version"
 
 // releaseTag matches a release tag, vMAJOR.MINOR.PATCH, or a release candidate,
-// vMAJOR.MINOR.PATCH-PRERELEASE. hack/release/draft-release.sh accepts the same.
+// vMAJOR.MINOR.PATCH-PRERELEASE. hack/release/draft-release.sh accepts the
+// same.
 var releaseTag = regexp.MustCompile(`^v[0-9]+\.[0-9]+\.[0-9]+(-[0-9A-Za-z.-]+)?$`)
 
 // CheckReleaseVersion returns an error unless version is a release tag.