Repository navigation
Conversation
3d4777d to
1727148
Compare
Wiz Scan Summary
To detect these findings earlier in the dev lifecycle, try the Wiz Code extension for VS Code, JetBrains, or Visual Studio. |
Huy Pham (hdp617)
left a comment
Huy Pham (hdp617)
left a comment
There was a problem hiding this comment.
Instead of having deploy/delete microvm-deps, how about doing these steps as part of deploy ate-system?
I like it because we want microVM SandboxClass installed by default. Also, the staging steps are needed at the moment because we don't have the microVM assets publicly available yet. Once they are, we can adjust deploy ate-system without any user facing changes.
| from, and applies the cluster-wide `microvm` SandboxConfig. `deploy ate-system` | ||
| installs only `gvisor-default`, so this is needed before any micro-VM demo or |
There was a problem hiding this comment.
I think we want to install microvm SandboxConfig by default om deploy ate-system.
There was a problem hiding this comment.
I think this is a good change, I've updated the code to match. I also added the opt-out flag like you mentioned in the PR this is stacked upon. It also simplifies the changes to make substrate-gke match, so that's a nice plus.
There was a problem hiding this comment.
LGTM from the upgrade side: step 4d now picks up the microvm SandboxConfig through deploy sandboxconfig.
deploy ate-system runs at upgrade step 4e so it will redeploy the microvm sandboxConfig again, which is no-op but is also ok. (I am thinking about merging upgrade 4d and 4e though).
Also cc Zoe Zhao (@zoez7) as SandboxConfig will carry different name per release soon.
|
/assign |
| go install ./cmd/kubectl-ate | ||
|
|
||
| # Create the actor from the counter template. | ||
| kubectl ate create actor my-counter-1 -a ate-demo-counter --template counter |
There was a problem hiding this comment.
Again I believe this is slightly wrong for the microvm case. kubectl ate create actor my-counter-microvm-1 -a ate-demo-counter-microvm --template counter-microvm
There was a problem hiding this comment.
(same as other README.md comment - this is for the gvisor demo, adjusted the doc to make it more clear)
| defaultCloudHypervisorVersion = "v53.0" | ||
| // virtiofsdVersion is the version bundled inside kata-static for | ||
| // defaultKataVersion (not env-overridable; verified before stamping). | ||
| virtiofsdVersion = "1.14.0" |
There was a problem hiding this comment.
Ditto
hack/microvm-assets/assemble.sh:30:# to and including 4.0.0; kata 4.1.0 bundles v1.14.0, the first release carrying the
hack/microvm-assets/assemble.sh:56:VIRTIOFSD_VER="1.14.0"
bin/microvm-assets/amd64/.asset-versions:4:virtiofsd=1.14.0
There was a problem hiding this comment.
same here, .../steps/microvm.go becomes the single source of truth for the virtiofsd version from this PR. assemble.sh gets deleted and .asset-versions is a local gitignored file from build time.
|
need rebase |
a39fc51 to
1d184c3
Compare
Walter Fender (cheftako)
left a comment
Walter Fender (cheftako)
left a comment
There was a problem hiding this comment.
The port from shell to Go in cmd/ate-setup/internal/steps/microvm.go is well-implemented and maintains backwards compatibility through the flag-translation shim.
Two minor discrepancies between the PR description and implementation should be noted:
- The PR description claims Cloud Hypervisor SHA-256 verification and staging to rustfs via
docker cp+ an in-cluster Job, whereas the implementation mirrors the original scripts (logging the calculated asset hashes for manifest authoring and streaming stdin directly to theamazon/aws-clicontainer).
Walter Fender (cheftako)
left a comment
Walter Fender (cheftako)
left a comment
There was a problem hiding this comment.
Looks good.
e685e2d to
ae44cc0
Compare
Huy Pham (hdp617)
left a comment
Huy Pham (hdp617)
left a comment
There was a problem hiding this comment.
LGTM. Thank you!
ae44cc0 to
666ca16
Compare
Bowei Du (bowei)
added
area/dev-infra
area/microVM
kind/cleanup
|
need rebase |
666ca16 to
ed3e67f
Compare
|
Need rebase |
9e63f0b to
fa92200
Compare
|
LGTM from upgrade side |
fa92200 to
b105583
Compare
2b01cea to
6e5f0f1
Compare
Huy Pham (hdp617)
left a comment
Huy Pham (hdp617)
left a comment
There was a problem hiding this comment.
Thanks! Just a few agent comments.
a64a65c to
3faa286
Compare
The micro-VM prerequisites -- the guest assets and the cluster-wide `microvm` SandboxConfig -- were reachable only as a side effect of `deploy benchmarks --sandbox-class=microvm`, which also brings up the whole locust stack. Everything else that needs them, the three micro-VM demos and any WorkerPool of your own, was told to run hack/install-microvm-deps.sh by hand. Make the cluster capable of running micro-VM workers a step of its own. The script is still invoked rather than reimplemented, for the reason recorded in differences.md: it drives asset assembly and object-store staging that are out of scope for the installer. What changes is that nothing outside the steps package has to name it. The benchmark paths now go through the same steps, so the two callers cannot drift apart. Fixes agent-substrate#1826 Signed-off-by: Noah West <westnoah@google.com>
Replace the shell implementation of hack/install-microvm-deps.sh and the helper scripts under hack/microvm-assets/ (assemble.sh, stage-to-gcs.sh, stage-to-rustfs.sh) with native Go in cmd/ate-setup/internal/steps/microvm.go. DeployMicroVMDeps now resolves the target architecture and asset version stamp in Go, streams and extracts the kata-static .tar.zst archive and Cloud Hypervisor binary, verifies the bundled virtiofsd version when runnable on the host, stages the assets to the Kind rustfs bucket or GKE GCS bucket, and renders and server-side-applies the cluster-wide microvm SandboxConfig. DeleteMicroVMDeps removes the SandboxConfig by name while leaving bucket assets intact. Keep hack/install-microvm-deps.sh as a flag-translation shim calling ate-setup deploy/delete microvm-deps and remove the obsolete scripts under hack/microvm-assets/. Signed-off-by: Noah West <westnoah@google.com>
Stage the micro-VM guest assets and apply the microvm SandboxConfig as part of deploy ate-system (and deploy sandboxconfig), with a --skip-microvm-deps opt-out flag, and remove the microvm SandboxConfig during delete ate-system. Drop the standalone deploy/delete microvm-deps subcommands and the hack/install-microvm-deps.sh shim. Signed-off-by: Noah West <westnoah@google.com>
Stage both amd64 and arm64 guest runtime assets under kata-assets/<arch>/ in the object store and update manifests/microvm/sandboxconfig-microvm.yaml.tmpl to reference the per-arch URLs so mixed-architecture clusters and cross-architecture workstations do not collide on a single prefix. Verify the SHA-256 digest of each assembled or cached asset against the digests pinned in sandboxconfig-microvm.yaml.tmpl before staging, drop un-registered raw environment overrides from steps/microvm.go, and fold the remaining notes from hack/microvm-assets/README.md into docs/dev/microvm-local.md. Signed-off-by: Noah West <westnoah@google.com>
… stamping Cache the full bin/microvm-assets directory (both amd64 and arm64) in the PR workflow instead of only bin/microvm-assets/amd64. Verify SHA-256 digests inside assembleMicroVMAssets before writing .asset-versions so a corrupt download is never stamped, and drop the redundant virtiofsd --version subprocess check. Move sandboxconfig-microvm.yaml.tmpl from the single-file manifests/microvm directory into manifests/ate-install alongside the other ate-setup manifests. Signed-off-by: Noah West <westnoah@google.com>
Stage assembled micro-VM guest assets under microvm-assets/v1/<arch>/ in the object store and bin/microvm-assets/v1/<arch>/ locally instead of unversioned kata-assets/<arch>/. Versioned paths prevent future SandboxConfig bumps from overwriting assets needed to restore suspended actors. Signed-off-by: Noah West <westnoah@google.com>
DeleteMicroVMDeps still targeted ate.dev/v1alpha1 after CRDs moved to
ate.dev/v1, which kube.Client.DeleteOne ignored as a no-match error. Use
atev1.GroupVersion.WithKind("SandboxConfig") instead.
Reject BUCKET_NAME values whose <BUCKET_NAME>-assets bucket exceeds GCS's
63-character limit, matching setup-gcp's checkAssetBucketName.
Pass --skip-microvm-deps from orchestrator.substrate_args when
sandboxClass is not microvm so gVisor benchmark runs do not need write
access to <BUCKET_NAME>-assets.
Signed-off-by: Noah West <westnoah@google.com>
AGENTS.md now standardizes prose on microVM without a hyphen. Update the docs, comments, and help text introduced by this branch to match. Signed-off-by: Noah West <westnoah@google.com>
Carry over the Kata 4.3.1 v2 digests and amazon/aws-cli 2.38.5 image digest from the removed microvm-assets shell scripts. Signed-off-by: Noah West <westnoah@google.com>
…taging kube.Client.WaitJobComplete now polls through poll so transient apiserver errors retry while waiting for rustfs-bucket-init. pr-workflow.yaml runs deploy demo counter-microvm directly because deploy ate-system already deploys the control plane and stages microVM assets. assembleMicroVMAssets stages cloud-hypervisor in workDir before renaming into outDir so aborted downloads leave no partial binary behind. checkMicroVMDeps validates the asset bucket and staging CLI before DeployAteSystem, DeploySandboxConfig, or DeployMicroVMDeps mutates the cluster or downloads archives. stageMicroVMAssetsToRustfs creates custom buckets on Kind when BucketName differs from ate-snapshots, and TestDeploySkipMicroVMDepsFlag resets Cobra flags for shuffled test runs. Signed-off-by: Noah West <westnoah@google.com>
3faa286 to
838f307
Compare
| // After the bundle on Kind, which creates the rustfs Deployment and | ||
| // bucket-init Job that DeployMicroVMDeps stages guest assets into. | ||
| if err := e.DeploySandboxConfig(ctx, opts.SkipMicroVMDeps); err != nil { | ||
| return err | ||
| } |
There was a problem hiding this comment.
This moves the gVisor SandboxConfig and the whole microVM download / upload to after the bundle apply, ahead of the Cloud SQL sidecar, egress CA, credential provider and atenet-egress. If the download or upload fails (GitHub rate limit, no write access to <BUCKET_NAME>-assets, a digest mismatch), deploy exits with ate-api-server on the sidecar-free base template and no credential provider or egress gateway until a rerun succeeds.
Assembling doesn't touch the cluster, can we do it up front next to checkMicroVMDeps (the GCS upload too), and only stage into rustfs + apply the config at the end?
| if !opts.SkipMicroVMDeps { | ||
| if _, err := e.checkMicroVMDeps(); err != nil { | ||
| return err | ||
| } | ||
| } |
There was a problem hiding this comment.
With this on by default, every non-kind deploy ate-system now needs BUCKET_NAME, gcloud, write access to <BUCKET_NAME>-assets and access to github.com. The install command in the release notes (hack/release/draft-release.sh) and commands.md#installing-pre-built-images set none of those, so that install now fails here.
Installing microVM by default is fine, but we should update those in this PR to say what's needed (or pass --skip-microvm-deps).
| node, err := e.kindNodeName(ctx) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| svc, err := e.Kube.Typed.CoreV1().Services(ns).Get(ctx, "rustfs", metav1.GetOptions{}) | ||
| if err != nil { | ||
| return fmt.Errorf("while getting service %s/rustfs: %w", ns, err) | ||
| } | ||
| if svc.Spec.ClusterIP == "" || svc.Spec.ClusterIP == corev1.ClusterIPNone { | ||
| return fmt.Errorf("service %s/rustfs has no ClusterIP", ns) | ||
| } | ||
| endpoint := fmt.Sprintf("http://%s:9000", svc.Spec.ClusterIP) |
There was a problem hiding this comment.
Now that this runs on every kind install, including gVisor-only ones, two bugs from stage-to-rustfs.sh with open fixes move onto the default path:
- microvm-assets: upload the kata assets through the node that hosts rustfs #2905: the upload only works from the node that hosts rustfs (its NetworkPolicy), so on a multi-node kind cluster this times out. We should pick the rustfs pod's node.
- hack/microvm-assets: stage assets to rustfs over IPv6 #1122: on
IP_FAMILY=ipv6this isn't a valid URL, it needsnet.JoinHostPort.
| } else { | ||
| log.Infof("Assets already present in %s; skipping assemble.", outDir) | ||
| if err := verifyMicroVMAssetDigests(outDir, arch); err != nil { | ||
| return err | ||
| } |
There was a problem hiding this comment.
If the stamp matches but a file doesn't match its pinned digest (a corrupted cache, or digests that changed without a kata / CH version change), every deploy fails here until someone deletes bin/microvm-assets by hand. Can we treat a mismatch as stale and re-assemble?
| if err := verifyMicroVMAssetDigests(outDir, arch); err != nil { | ||
| return err | ||
| } | ||
| if err := os.WriteFile(stampPath, []byte(microvmAssetStamp(arch, kataVer, chVer)), 0o644); err != nil { | ||
| return fmt.Errorf("while writing %s: %w", stampPath, err) | ||
| } | ||
|
|
||
| log.Infof(">> sha256 (paste all four into the %s block in %s):", arch, microvmSandboxConfigTemplate) |
There was a problem hiding this comment.
On a kata or CH bump the verify above fails on the first changed file, so this paste message is never reached when it's needed, and each run only tells you one digest (re-downloading the tarball every time). We should report got / want for every file on a mismatch and drop the paste message?
| // and bucket-init Job on Kind. | ||
| MicroVMStageTimeout = 300 * time.Second | ||
|
|
||
| defaultMicroVMAssetVersion = "v2" |
There was a problem hiding this comment.
Nothing forces bumping v2 when the assets change. If a kata bump updates the digests and misses this (and the template URLs), staging overwrites the objects the previous release's SandboxConfig pins, and with #3254 rolling back to that config then fails atelet's sha256 check.
What if the paths were keyed by digest instead, e.g. microvm-assets/<arch>/<sha256>? Then there's no version to bump, and staging can skip objects that already exist instead of re-uploading all 8 files on every deploy.
cc Zoe Zhao (@zoez7) who is also touching this.
| microvmAssetDigests = map[string]map[string]string{ | ||
| "arm64": { | ||
| "cloud-hypervisor": "f192b510eea1c710cbc439d716bb0573c223fc463dbe3e6523788a2b7ef62850", | ||
| "virtiofsd": "4b43f4f6de6500203b01214a790314eea4182f5f110594ddb28ae2805622f06f", | ||
| "vmlinux": "dece388830720fe8bddb41552fff5f486240aeece5acc0aebc1812761e3e3f66", | ||
| "rootfs.img": "69f9a76e5a2563b7c74d1f271f3a904240f50ef405465f832021525894519f97", | ||
| }, | ||
| "amd64": { | ||
| "cloud-hypervisor": "448af3d4e59b22c2987f7df94c213ad40fb53a10d437e42b5ee6c4fce7c29ecc", | ||
| "virtiofsd": "15b2e72a78cc08a9bd8a6943e89fb69c88cb3cbeb63069efceade835342ac7d4", | ||
| "vmlinux": "f87d8359035cb7557b16a319e6431ea09b183e62268aef436bf8dc6530d786b6", | ||
| "rootfs.img": "6770db2cca5710593f27c9ac5e1d9d7aab836422695a5e5c4807496a92a0f24d", | ||
| }, | ||
| } |
There was a problem hiding this comment.
These duplicate the digests in the template, and TestMicroVMSandboxConfigTemplate only checks that each digest appears somewhere in the rendered output, so swapping the amd64 / arm64 values in the template still passes. We already render and decode the template, can we read the expected digests from it so there's one copy?
| return err | ||
| } | ||
|
|
||
| for _, arch := range microvmArches { |
There was a problem hiding this comment.
kind nodes are always the host arch (the kind profile already sets ko.defaultPlatforms to linux/<GOARCH>), so staging both arches there doubles the download on a fresh install and what we put in rustfs, for every kind user including gVisor-only. We should probably only do both arches on non-kind? (and save a lot of time ...)
| if err := e.DeleteMicroVMDeps(ctx); err != nil { | ||
| return err | ||
| } |
There was a problem hiding this comment.
nit: I think we can drop this, deleting the CRDs plus releaseSandboxConfigs below already removes every SandboxConfig (that's how gvisor-default goes on kind). On kind it also logs that the bucket assets are left in place right after rustfs was deleted.
| Usage: "Also install CSI driver (nfs, hostpath, both, none; default: none)", | ||
| }, | ||
| config.Setting{ | ||
| Key: "skipMicroVMDeps", Env: "ATE_INSTALL_SKIP_MICROVM_DEPS", Flag: "skip-microvm-deps", |
There was a problem hiding this comment.
non-blocking: "deps" is the old script's name, and this flag will outlive the staging once the assets are published. Did we consider something like --sandbox-classes=gvisor,microvm (default all)? Then the benchmark orchestrator can pass the test's class instead of appending a skip.
…ate#3354) Since agent-substrate#2718, both scripts only forward to `go run ./cmd/ate-setup`. This PR deletes them, and every caller, CI step and doc now runs `ate-setup` directly. CI passes `--kind` in place of `install-ate-kind.sh`. The kind Quickstart sets the same option once with `export ATE_INSTALL_KIND=true`, so the demo READMEs no longer mention kind or GKE. Fix agent-substrate#1503 ### Breaking Change There are some behavior changes: - `ate-setup` exits 1 on an unknown subcommand such as `deploy demo demo-counter`, instead of printing help and exiting 0, this fixes agent-substrate#3209 - `--setup-csi` no longer accepts the old shell spellings `true`, `false` or an empty value because they are returned `unknown CSI driver`, the previous params are left for compatiblity with shell script. - `hack/setup-csi-hostpath-kind.sh` is replaced by `ate-setup --kind setup csi hostpath`. ### Testing Ran every demo README as written on fresh kind and GKE clusters. Every `ate-setup` step works on both. claude-code-multiplex, parking and the counter microVM script still fail, as they did before this PR (agent-substrate#2487, agent-substrate#3287; agent-substrate#2092 rewrites the microVM script). Claude help review the list and do the replacement, I reviewed. - [x] Tests pass - [x] Docs updated --------- Signed-off-by: Haven Xia <haoyuxia@google.com>
Stacked on #1838.
Summary
Ports the microVM dependency setup (
hack/install-microvm-deps.shandhack/microvm-assets/) to Go, and folds microVM asset assembly, staging, andSandboxConfigmanagement directly intodeploy ate-system,deploy sandboxconfig, anddelete ate-system.Changes
cmd/ate-setup/internal/steps/deploy.go,delete.go,benchmarks.go):DeployMicroVMDepsinsideDeployAteSystem(after applying the system bundle so Kind's in-clusterrustfsDeployment andrustfs-bucket-initJob exist before asset staging) and insideDeploySandboxConfig.--skip-microvm-depstoate-setup deploy ate-system(and forwards it throughhack/install-ate.sh) to opt out of staging microVM assets and applyingSandboxConfig/microvm.DeleteMicroVMDepsinsideDeleteAteSystembefore CRD teardown.DeployBenchmarks/DeleteBenchmarkssincedeploy ate-systemnow installs bothgvisor-defaultandmicrovmSandboxConfigs up front.hack/install-microvm-deps.shandhack/microvm-assets/:hack/install-microvm-deps.shand the entirehack/microvm-assets/directory (assemble.sh,stage-to-gcs.sh,stage-to-rustfs.sh, andREADME.md).hack/run-microvm-demo.sh,benchmarking/automation/orchestrator.py, and.github/workflows/pr-workflow.yamlto rely ondeploy ate-system.cmd/ate-setup/internal/steps/microvm.go):.tar.zstarchive (vmlinux.container,kata-containers-ubuntu.imgasrootfs.img,virtiofsd) and Cloud Hypervisor binary in Go for bothamd64andarm64intobin/microvm-assets/{amd64,arm64}/.virtiofsdversion and checks the SHA-256 digests of all four assets (cloud-hypervisor,virtiofsd,vmlinux,rootfs.img) against the digests pinned inmanifests/microvm/sandboxconfig-microvm.yaml.tmpl(both after assembly and when reusing a cached.asset-stampdirectory).kata-assets/<arch>/<file>in Kind'srustfsbucket or GKE's GCS bucket, then renders and appliesmanifests/microvm/sandboxconfig-microvm.yaml.tmpl.manifests/microvm/sandboxconfig-microvm.yaml.tmplto use per-archkata-assets/<arch>/<file>URLs and removes stale version-number comments.hack/microvm-assets/README.mdintodocs/dev/microvm-local.mdand updatesdocs/api-guide.md,cmd/ate-setup/commands.md,cmd/ate-setup/differences.md, anddemos/counter/README.md.cmd/ate-setup/internal/steps/microvm_test.go,cmd/ate-setup/internal/cmd/deploy_test.go, andcmd/ate-setup/shim_test.go.Testing
go test -race ./cmd/ate-setup/... ./internal/e2e/...andhack/verify-all.sh.hack/install-ate-kind.sh):--deploy-ate-system --skip-microvm-depsdeploys the control plane and installs onlySandboxConfig/gvisor-default.--deploy-ate-systemassembles the microVM guest assets, stages them into the in-clusterrustfsbucket, and appliesSandboxConfig/microvmalongsideSandboxConfig/gvisor-default(reusing cached assets via.asset-stampon subsequent runs).--delete-allremovesSandboxConfig/microvmduring control-plane teardown before deleting CRDs.--deploy-demo-counterand ran./hack/run-e2e-kind.sh ./internal/e2e/suites/demo.