Repository navigation
ate-setup: wait for the demo namespace in delete demo - #3553
Conversation
|
The changes look good. But need someone with maintainer to also review. Haven Xia (@HavenXia) |
| } | ||
| // A deploy right after the delete cannot create objects in a namespace | ||
| // that is still terminating. | ||
| for _, obj := range objs { |
There was a problem hiding this comment.
🤖 nit 🟢 – delete all calls every demo's Delete in turn, so this wait now serializes the namespace drains and delays the ate-system teardown by their sum. Deleting demos used to fire and forget, and the drains overlapped with the ate-system namespace wait. If that matters, issue all demo deletes first and wait for the namespaces afterwards, or only wait when delete demo is run on its own.
There was a problem hiding this comment.
Done: update the wait order of delete all as you suggested
8e28733 to
3c7995c
Compare
| func (e *Env) WaitDemoNamespaceDeleted(ctx context.Context, name string) error { | ||
| if e.deletingAll { | ||
| e.demoNamespaces = append(e.demoNamespaces, name) | ||
| return nil | ||
| } |
There was a problem hiding this comment.
🤖 should-fix 🟡 – A method named Wait… that silently returns without waiting, depending on a flag set elsewhere on Env, is a trap for the next caller. The flag is also left set if a demo delete fails. One field would do the job and make the mode visible at the call site: pendingNamespaces *[]string, which DeleteAll points at a local slice and clears with a defer. When it is nil the method waits inline, otherwise it appends. The doc comment can then say "waits, or records the namespace when DeleteAll has deferred the wait" and the two bool-plus-slice fields go away.
There was a problem hiding this comment.
Done: replaced the flag and the slice with pendingNamespaces *[]string, which DeleteAll points at a local slice in deleteDemos and clears with a defer.
3c7995c to
ce60063
Compare
| func (e *Env) WaitDemoNamespaceDeleted(ctx context.Context, name string) error { | ||
| if e.pendingNamespaces != nil { | ||
| *e.pendingNamespaces = append(*e.pendingNamespaces, name) | ||
| return nil | ||
| } |
There was a problem hiding this comment.
🤖 should-fix 🟡 – This is my earlier suggestion, so apologies for the churn, but it keeps the problem: the method still decides on its own whether to wait, based on a field the caller cannot see. The caller should decide. Have Deleter.Delete return the namespaces it left terminating, and let the two callers act on that: the delete demo command waits on them right away, DeleteAll collects them and waits after DeleteAteSystem. Then Env needs no field, and this method becomes a plain WaitNamespacesDeleted(ctx, names) that always waits. Only Substrate.Delete and the claude-code-multiplex wrapper implement the interface, so the change is small.
There was a problem hiding this comment.
Updated - Deleter.Delete now returns the namespaces it left terminating.
and delete demo waits on them right away, with DeleteAll collects them and waits after DeleteAteSystem, through a plain WaitNamespacesDeleted. Env has no field for it anymore.
`delete demo` returned while the demo namespace was still terminating, so a `deploy demo` right after it failed: the API server refuses new objects in a terminating namespace. Delete now waits for the namespace to be gone, as `delete ate-system` does. Signed-off-by: Haven Xia <[email protected]>
ce60063 to
13db0df
Compare
…e#3553) `delete demo` returned while the demo namespace was still terminating, so a `deploy demo` right after it failed: the API server refuses new objects in a terminating namespace. Delete now waits for the namespace to be gone, as `delete ate-system` does. Why is this needed? (Or link the issue. Open one first for features, API, install or CI changes.) Demo Fix. Fixes agent-substrate#3525 AI use: I wrote, claude polished. - [ ] Tests pass - [x] Docs updated Signed-off-by: Haven Xia <[email protected]> (cherry picked from commit e99dffb) Signed-off-by: Eitan Yarmush <[email protected]>
Bring `release-1.0` up to `main` at 5a5b192 with the 19 changes merged since the previous batch in #3536. The final tree matches that commit exactly. **Please use Rebase and merge to preserve the individual commits and author attribution.** All 19 applied cleanly in upstream order. Original authors, author dates, co-author and sign-off trailers, and `-x` source references are retained. Upgrade notes: - #3533: old gVisor MEMORY snapshots, including goldens, no longer restore. Recreate them after upgrading. MicroVM MEMORY snapshots still resume with their old mounts until recreated. `/tmp` is now always tmpfs; ROOTFS snapshots no longer preserve its contents, and image-provided files there are hidden. MicroVM `/tmp` counts against actor memory and is capped at half the guest's memory. gVisor `/dev/shm` is now a 64 MiB tmpfs. Mount a `durableDir` at either path when persistence or more space is needed. `/run` now comes from the image rootfs. - #3447: remove `--actor-jwt-jwks-file` from custom credential-exchange sidecar configs. Set `--actor-jwt-issuer-ca-file` only when the issuer uses a private CA other than the service DNS CA. - #3450: do not reuse a deleted atespace name. See the updated atespace name-reuse guidance. Includes #3505, which overlaps the open backport in #3547. GKE provisioning and teardown now live in [substrate-gke](https://github.com/ai-on-gke/substrate-gke) rather than this repository. <details> <summary>Included PRs, in cherry-pick order</summary> 1. #3500: demos/{sandbox,jupyter,multi-template}: fix tutorial prerequisites, walkthroughs, and counter volume 2. #3498: demos/counter: inline gVisor walkthrough and fix setup commands 3. #3511: e2e tests: Extend the existing tests to cover tag actor. 4. #2775: Generate API reference documentation and publish it on Substrate website 5. #3119: Document how people, pods and CI jobs should authenticate to ate-api 6. #3505: tools: remove setup-gcp and hack/teardown.sh 7. #3147: e2e: resume fidelity after SandboxConfig version edits 8. #3370: ate-setup: reconcile ate-api-server ConfigMaps and roll out on config changes 9. #1607: hack: reject breaking proto changes across each proto package 10. #3415: docs: update and add security diagrams 11. #3485: Resuming a microVM actor on the same node no longer reads its rootfs upper back from disk 12. #3447: Verify actor JWTs in the credential exchange through OIDC discovery 13. #3533: ocispec: mount tmpfs at /dev/shm, and /tmp on both runtimes 14. #3450: docs: Document that atespace names must not be reused 15. #3410: Document using more than one egress credential provider 16. #3486: atelet: register the device plugin on a new socket each time 17. #3553: ate-setup: wait for the demo namespace in delete demo 18. #3560: Fix multiple issues in claude-code-multiplex demo 19. #3517: docs: remove inaccurate early-stage architecture documentation </details> Validation: every intermediate tree matches its upstream source commit, all 19 authorship records and original attribution trailers are preserved, and `git diff --check` passes. The original 18 backport commits are unchanged. AI use: AI-assisted cherry-picking, attribution checks, and review. - [ ] Tests pass - [x] Docs updated --------- Signed-off-by: Mofi Rahman <[email protected]> Signed-off-by: Eitan Yarmush <[email protected]> Signed-off-by: Nayana Bidari <[email protected]> Signed-off-by: Julian Gutierrez Oschmann <[email protected]> Signed-off-by: Max Thompson <[email protected]> Signed-off-by: Steven Shriver <[email protected]> Signed-off-by: Grant McCloskey <[email protected]> Signed-off-by: Shrutiya <[email protected]> Signed-off-by: Haiyan Meng <[email protected]> Signed-off-by: Greg Castle <[email protected]> Signed-off-by: Benjamin Elder <[email protected]> Signed-off-by: Anish Ramasekar <[email protected]> Signed-off-by: Shruti Nair <[email protected]> Signed-off-by: Haven Xia <[email protected]> Co-authored-by: Mofi Rahman <[email protected]> Co-authored-by: nybidari <[email protected]> Co-authored-by: Julian Gutierrez Oschmann <[email protected]> Co-authored-by: Max Thompson <[email protected]> Co-authored-by: Grant McCloskey <[email protected]> Co-authored-by: Steven Shriver <[email protected]> Co-authored-by: shrutiyam-glitch <[email protected]> Co-authored-by: haiyanmeng <[email protected]> Co-authored-by: Greg Castle <[email protected]> Co-authored-by: Benjamin Elder <[email protected]> Co-authored-by: Anish Ramasekar <[email protected]> Co-authored-by: Shruti Nair <[email protected]> Co-authored-by: Haven Xia <[email protected]> Co-authored-by: Adam Wathieu <[email protected]>
`delete demo` returned while the demo namespace was still terminating, so a `deploy demo` right after it failed: the API server refuses new objects in a terminating namespace. Delete now waits for the namespace to be gone, as `delete ate-system` does. Why is this needed? (Or link the issue. Open one first for features, API, install or CI changes.) Demo Fix. Fixes #3525 AI use: I wrote, claude polished. - [ ] Tests pass - [x] Docs updated Signed-off-by: Haven Xia <[email protected]> (cherry picked from commit e99dffb) Signed-off-by: Eitan Yarmush <[email protected]>
delete demoreturned while the demo namespace was still terminating, so adeploy demoright after it failed: the API server refuses new objects in a terminating namespace. Delete now waits for the namespace to be gone, asdelete ate-systemdoes.Why is this needed? (Or link the issue. Open one first for features, API, install or CI changes.)
Demo Fix.
Fixes #3525
AI use: I wrote, claude polished.