Repository navigation
ate-env-api: create the default atespace at startup - #78
Tomer Glottmann (tomergee) merged 2 commits into
Conversation
…be registered on a fresh cluster
a2d5e9a to
d3066d9
Compare
Tomer Glottmann (tomergee)
left a comment
There was a problem hiding this comment.
Approve after two small changes. The change does what the description says. EnsureAtespace tolerates AlreadyExists, thus two replicas that start together do not conflict, and TestEnsureAtespace covers the idempotent path. The branch sits on main after #77 with no drift.
Blocking
None.
Before merge
-
cmd/ate-env-api/main.go:41— the call usescontext.Background()with no deadline. If the control plane hangs, startup blocks for ever. The server does not listen, the readiness probe never passes, and no log line explains the state. Use a context with a timeout, for example 30 seconds. Keeplog.Fatalfon error: the pod restarts with backoff until Substrate answers. -
internal/ate/client.go:215—EnsureAtespacecallsCreateAtespacefirst. Under Substrate authorization (--experimental-enable-authz, off by default today), an identity that can use the atespace but cannot create one getsPermissionDenied. The API then refuses to start although the atespace exists. CallGetAtespacefirst andCreateAtespaceonly onNotFound. The fake control server implements both RPCs, thus the test can cover the new order. -
README.md:62— the quickstart does not say that the API creates the atespace. Add one sentence to step 2: register templates after theate-env-apipod isRunning, or runkubectl-ate create atespace ate-envby hand. Without this, a reader who registers the template first sees the failure this PR removes.
Nits
-
cmd/ate-env-api/main.go:41andinternal/apiservice/server.go:46—ate.DefaultAtespaceandapiservice.DefaultAtespaceare two constants with the same value. This PR adds a third user of the first. Make one an alias of the other. -
cmd/ate-env-api/main.go:43— log one line when the atespace is ready. Operators can then correlate startup with the first template registration. -
The branch name mentions a Substrate pin bump that is not in the diff. Rename the branch or ignore; nothing in the change depends on it.
…one default constant, README note
|
Addressed all five: 30 s timeout with the fatal kept, so a hung control plane restarts the pod with backoff; EnsureAtespace now calls GetAtespace first and only creates on NotFound, with a test where the fake denies CreateAtespace (existing atespace passes, missing one fails loudly); apiservice.DefaultAtespace is an alias of ate.DefaultAtespace; one startup log line when the atespace is ready; README step 2 says to register templates after the API pod is Running or create the atespace by hand. Leaving the branch name as is since it is a cosmetic leftover from the superseded version of this PR. |
cbe7d28
into
agent-substrate:main
…ting check (cherry picked from commit 86eebcc)
* Publish release images and a CLI that knows them Nothing published images for this repo: the Makefile pushed to a personal registry, releases carried no assets, and the manifest command's error message pointed at "images published by the latest release" that did not exist. Every user, including the quickstart, had to build and push ate-env-api and ate-env-guest first. A release workflow now runs on every v* tag (and by hand for an existing tag): ko builds both images for linux/amd64 and linux/arm64, pushes them to ghcr.io/<org>/<repo>/ate-env-api and ate-env-guest tagged with the release and latest, signs them with cosign keyless signing bound to the workflow's identity, then builds the ate-env CLI for linux and darwin on amd64 and arm64 with the two digests and the version baked in, uploads the binaries, an images_<tag>.txt and checksums as release assets, and writes an Images section with the digests and the verify command into the release notes. The CLI gains build-time defaults: a release binary's `manifest --api-image` and `manifest template --guest-image` default to the images published with it, `ate-env --version` prints the tag, and a source build keeps the flags required with messages that say why. The worker image stays a required flag, since ateom-gvisor comes from the Substrate repo and must match the Substrate version on the cluster; the message says so. `make build-cli` reproduces a release-style binary locally. A ci workflow runs the Go build, vet and tests, the license-header check, and the Python client tests on pull requests and main; the repo had no checks before. The guest's bash base image is pinned by digest in .ko.yaml so a published guest is reproducible. docs/release.md describes the artifacts, pinning, verification, cutting and republishing a release, and the one-time organization setup for GitHub Container Registry; the README quickstart no longer needs image flags with a release binary. * workflows: pin actions by SHA, scope permissions, keep expressions out of shell What the organization's workflow security scan (zizmor) requires, and what a release pipeline should do anyway: - every action pinned by commit SHA, with the version in a trailing comment (each SHA checked against its tag) - checkout with persist-credentials off - top-level permissions empty; the release job asks for contents, packages and id-token itself - no expression interpolated into a run block: inputs and step outputs reach the shell through env - Go module cache off for the release build, so a release cannot consume a cache another run seeded - concurrency: a new push supersedes running CI; one publish per tag at a time docs/release.md explains the pinning convention and how to bump a pin. zizmor reports no findings on both workflows. * Makefile: images prints the digests instead of editing the README The quickstart no longer pins this repo's images, release binaries carry the defaults, so the regex rewrite had nothing to match and printed a misleading "updated README" line. Build, push, print. * gofmt internal/ate: #78 landed with space-indented code and no formatting check * ci: fail on unformatted Go (make verify-gofmt) * ate-env-guest: stop a sidecar's whole process group on shutdown runSidecar used exec.CommandContext, whose cancel kills only the direct child. A sidecar that forks (a wrapper script, which RUNTIMES.md recommends for quoted arguments) died alone while its child kept the output pipes open, and cmd.Wait blocked until that child exited. The guest's shutdown hung for as long as the orphan lived. TestRunSidecarRestartsAndStopsWithContext caught this on Linux and not on macOS: `sh -c "sleep 60"` is dash on Linux, which forks sleep, and bash on macOS, which execs it. First seen in the ci run on #71 (https://github.com/agent-substrate/env/actions/runs/38440900526), the first Linux run of this code. Each sidecar now runs in its own process group. On cancel the group gets SIGTERM, cmd.WaitDelay (sidecarStopGrace, 5 s) bounds the wait for the pipes, and the group gets SIGKILL for anything still alive. Verified in a golang:1.27 container, the test 3/3 and the guest packages green, and on macOS. (cherry picked from commit 93c5fb2) * workflows: pin the runner image to ubuntu-24.04 ubuntu-latest migrates to Ubuntu 26.04 from 2026-10-19 (actions/runner-images#14748). A release pipeline must not change its toolchain image underneath a tag; pin the image and bump it on purpose. * ate-env-guest: finish the sidecar stop sequence and cover it on every platform Review follow-ups on the process-group fix: - main waits for the supervisors after Serve returns (bounded by sidecarStopGrace plus a second), so the SIGTERM, grace and SIGKILL sequence runs before the process exits. Inside an actor the guest is PID 1 and the container ends with it anyway; outside one, this is what stops the sidecars. - The old process group is SIGKILLed before a restart as well, so a restarted sidecar does not compete with its own orphans for ports. - A stop is logged. - The test forks explicitly (`sleep 60 & wait`): a bare `sh -c "sleep 60"` execs on macOS and did not exercise the group kill there. The bound is derived from sidecarStopGrace and explained: the forked sleep holds the pipe, so returning within it proves the child died. (cherry picked from commit 922a95b) * Address review: name the worker-image publish command, anchor the cosign identity, strip CR before editing notes - The --worker-image message, the README quickstart and docs/release.md name `go run ./cmd/ate-setup publish worker-images` (which pushes the ateom images and prints their digest-pinned refs) instead of "build it with ko", and warn that a registry's :latest is whatever was built last and a stale worker image registers workers with no capacity. - The cosign identity regexp in the release notes and the docs is anchored to this repository's release workflow at a refs/tags/ ref, not to any identity under the repository. - The republish path strips carriage returns from the release body before looking for the previous "## Images" block; with CRLF from the API the block was appended twice instead of replaced. * ate-env-guest: one stop sequence for sidecar process groups, with step logs Review follow-up on #79. stopGroup is now the only way a sidecar's group is taken down: SIGTERM, up to sidecarStopGrace for the group to disappear, then SIGKILL for what is left. Both paths use it: after a cancel, where it finishes what cmd.Cancel started, and before a restart, where it clears what a crashed sidecar left behind so the new one does not compete with its own orphans for ports, pipes or lock files. Each step is logged: SIGTERM sent, group exited, still alive after the grace and SIGKILL sent, or already gone (ESRCH counts as success). main's shutdown wait covers both grace periods. The test asserts the logged sequence. (cherry picked from commit 8eb8274)
A fresh cluster has no atespace, so registering an ActorTemplate fails until
someone runs
kubectl-ate create atespace ate-envby hand. ate-env-api nowensures its default atespace exists at startup, before the first request.
Sits on top of the substrate bump to f03327bd. Verified on GKE at substrate
2e3814a0: template registers and bakes a golden on a cluster that had no
atespace.