TagStatus.source_actor_uid is unused, we can add this back if we ever
need it.
#1168
We were also missing some of the DV for TagStatus fields:
* Added maxLength=2048 requirement for ExternalSnapshot.snapshot_uri
(matching in_progress_snapshot_uri)
* Added validation for actor_template_uid
* Added validation for storage_location, matching what we have in
SnapshotsConfig.storage_location
https://github.com/agent-substrate/substrate/issues/1378
We don't have a backwards compatibility requirement yet, so let's clean
this up for now.
- [x] Tests pass
- [x] Appropriate changes to documentation are included in the PR
This is part of #1266 , opening now for discussion.
Stacked on https://github.com/agent-substrate/substrate/pull/1682 which
was slightly orthogonal.
This is loosely based on the mini proposal by @howardjohn as discussed
in the community meeting, and feedback from @bowei @EItanya
@LiorLieberman @aojea.
https://docs.google.com/document/d/1TycfQ3iiEpbI3rveMIj0S2PpPuLecb8I5R--yTpt9Ig/edit?resourcekey=0-kJbtEZ-KGzuL5eCjHDvBhg&tab=t.0#heading=h.ga9bfaf55ptk
Roughly:
1. Actor sandboxes each get their own netns.
- gVisor grabs all interfaces in the netns, and tap currently requires
running something like slipr, so for now we do two netns + a veth when
gVisor.
3. In the netns we directly intercept TCP => atunnel for general
traffic.
4. In the netns we serve a trivial TCP+UDP DNS relay to the pod
resolution.
- In the future we can insert policy here.
6. We consistently inject a modified resolv.conf instead of
bind-mounting it (gVisor) across both runtimes.
7. Readiness probe dialing happens in the actor netns.
Every actor gets the same fixed guest IP as before, which is only
visible to the actor.
All inbound/outbound traffic comes from atunnel / the DNS relay.
The actor no longer has any direct use of the pod interface, so we can
begin to consider ateom using the network itself.
When we add the rest of multi-actor changes, this greatly simplifies
thing.
Full multi-actor requires further changes, but this diff is already
large (suggest reading commit by commit) and can stand-alone. I'll file
more stacked changes when we've got consensus on this one.
The name `readyz` is not very descriptive. We also want to avoid
`readinessProbe` to prevent confusion with the Kubernetes concept, which
represents continuous traffic gating. Renaming this field to
`wakeupProbe` clarifies its actual behavior, and the fact that it only
operates when the actor is woken up.
Part of #1378
# Description
**Substrate assumes the canonical install layout, and every deviation
fails closed.** The namespace `ate-system`, the Services `api` /
`atenet-router`, and the ServiceAccounts `atelet` / `atenet-router` are
compiled-in constants. An install in a per-developer namespace, or under
a deployment that prefixes resource names, breaks — and no failure
points at the naming.
This series makes the namespace, the Service names, and the
ServiceAccount names configurable. Every option defaults to the
canonical value. A canonical install is byte-for-byte unaffected.
| Hardcoded assumption | Failure | What it looks like instead |
|---|---|---|
| atelet's namespace in ateapi's SPIFFE check | ateapi rejects every
atelet | an mTLS handshake failure, not a naming error |
| atelet's namespace in the worker's broker check | no actor obtains a
certificate | `credential broker is not atelet` |
| NetworkPolicy's ingress namespace | the CNI drops every request to the
pool | a silent network fault |
| ateapi Service name and namespace in the client | the client cannot
authenticate | `services "api" not found`, then `invalid bearer token` |
| Resource names in `hack/install-ate.sh` and the e2e harness |
authorities land in the wrong namespace | suites die in preflight |
**A SPIFFE ID breaks on two axes.** It names a namespace and a
ServiceAccount. Both were constants, and the failure surfaces as a
rejected peer, not a missing object.
## Design notes
- **ate-controller passes the worker-side identities.** ateom already
took `--atunnel-client-identity` as a flag but relied on its default. A
new `--atunnel-broker-identity` flag alone would be inert: correct only
where the constant was already correct. The controller knows the control
plane's namespace, so it supplies both.
- **ateapi takes the whole expected identity, not a namespace.** The
dialer and `ateletauth` used the namespace only to build one string. One
place decides how atelet's identity is spelled.
- **`ateletauth` deduplicates without taking the dependency it was
avoiding.** Its constants are duplicated rather than imported so the
package does not depend on `controlapi` for three strings. `ateletdial`
would make a third copy, so the strings move to
`internal/installdefaults` instead — a leaf package of constants that
imports only the standard library, so consuming it does not reintroduce
the coupling the duplication was there to prevent.
- **The client reads environment variables, not flags.** It runs outside
the cluster. It has no downward API and nothing to discover from.
- **Namespaces come from the downward API, not flags.** Every supported
topology co-locates these components. Only Service and ServiceAccount
names, which a deployment may legitimately rename, get flags.
- **`hack/install-ate.sh` and the e2e harness are in scope.** A
relocated install cannot be bootstrapped or exercised without them. The
script refuses `ATE_NAMESPACE` or `ATE_API_SERVICE_NAME` overrides on
the manifest-applying subcommands, which would half-install:
`manifests/ate-install/` names `ate-system` and `api` literally.
## Why this is one PR
The series is stacked, not parallel. Commit 1 creates
`internal/installdefaults` and seeds seven imports; commit 2 adds
fourteen more; every later commit builds on those constants. Split into
separate PRs, each blocks on the previous merging and none reviews
independently.
The guard test cannot land first. Applied to `main` it fails on eleven
hardcoded literals across `ateletauth`, `controlapi/informer.go`,
`networkpolicy_controller.go`, both `ateom` mains, `ateclient`,
`ateletdial` and three e2e files. It is green only because the preceding
commits removed them.
A partial series still fails closed. Each commit's audit turned up more
places assuming `ate-system`, so landing commits 1-3 without 4-5 leaves
a relocated install broken later and less legibly than before.
`CONTRIBUTING.md` covers this case: when the intermediate steps are not
useful on their own, keep the change as one PR split into commits at
logical break points. Review commit by commit, and preserve the commits
on merge. If you would still rather split, the one defensible cut is by
axis — relocation (commits 1-3) then renaming (commits 4-5) with the
guard test rebased on top.
## Rebase notes (2026-09-15)
The series is rebased onto `main` at `d0d85c39`, ninety-two commits on
from the base the PR first carried:
- `main` moved actor JWT and certificate minting out of
`cmd/ateapi/internal/actoridentity` into `controlapi`, deleting the
package and dropping its inline atelet authorization check in favor of
the unified authorizer. This series previously threaded the expected
atelet identity into that check; the check no longer exists, so that
adaptation is gone. `ateletauth` survives with one caller,
`workerservice.SetWorkerCapacity`, which still takes the configured
identity.
- The same refactor replaced `BrokerConfig.ExpectedActorUID` with an
`ActorAtespace` / `ActorName` / `ActorUID` triple. `AteletSPIFFEID` is
additive and still required by `ateletdial.TLSConfig`.
- `ateletauth` (ateapi's side) and `ateletdial` (the worker's side) each
re-declared the hardcoded SPIFFE ID. Both take the expected identity as
a parameter.
- `main` deleted the atenet DNS subsystem. The series no longer touches
it, and `installdefaults` carries no DNS Service name.
- The controller passes `--atunnel-broker-identity` only when it differs
from the canonical default. An ateom old enough to predate the flag
exits on it, and `docs/upgrade.md` keeps such a pool serving during a
rolling upgrade. A relocated install gets the flag and necessarily runs
an image that accepts it.
- `main` added `internal/e2e/collector_metrics.go` (agentgateway CI
support), which addresses the router through the package-level
`routerNamespace` and `routerService` that this series replaces. Its two
call sites become `SystemNamespace()` and
`ResourceName("atenet-router")`, matching `router_client.go` and
`statusz.go`. A reviewer diffing against the older base sees those call
sites move; nothing else in that file changes.
- `main` added `cmd/credential-provider/kubernetes-secrets` (`#1335`),
whose `injectorSPIFFEID` constant is the mTLS peer check on the only
caller permitted to read Secrets. The comparison is an exact string
match, so a renamed install rejects every fetch at TLS. It becomes
`--injector-identity`, defaulting to
`installdefaults.EgressSPIFFEID(SystemNamespace)`, alongside a new
`EgressServiceAccount` constant and helper. The flag name follows
`--atunnel-client-identity`; the manifest beside it still names
`ate-system`, so a renaming deployment configures both.
- `main` replaced the dialer's worker indexer with
`DialForAteletOnNode`, dropped `WorkerPodInformer` from `controlapi`,
and removed `kataConfig` from ateom-microvm's `NewService`. The series
adapts to each narrower signature and keeps only its own added
parameter.
- The guard test's allowlist entry for the two inert `ate-system`
literals follows the code from `actoridentity.go` to
`controlapi/actor.go`. The refactor re-introduced exactly the class of
constant this series removes, so the guard earns its place.
# Testing
- `go test -race ./...` passes. `make verify` passes boilerplate,
codegen, go-modules, gofmt, golangci-lint, kube-api-linter, licenses,
metrics and postgresql-migrations. `proto-fmt` needs `clang-format`,
which is absent locally; this series touches no `.proto` files.
- Every commit builds and vets individually, via `git rebase -x 'go
build ./... && go vet ./...'`.
- On a fresh kind cluster, all twelve e2e suites pass: `capabilities`,
`combinedvolumes`, `demo`, `egressauthz`, `egressmitm`, `example`,
`identity`, `metrics`, `networking`, `networkpolicy`, `parking`,
`sizing`. The `demo`, `metrics`, `networkpolicy`, `parking` and
`networking` suites need the `--deploy-demo-counter` and
`--deploy-demo-egress` fixtures, and the MITM path needs
`--deploy-demo-egress-mitm`; without them they fail on `actor template
not found`, which reads as a control-plane fault rather than a missing
fixture.
- With the sdsmint egress gateway (`--deploy-atenet
--experimental-use-sdsmint`, `E2E_EGRESS_MITM=1`),
`TestActorEgressMITMTrust` and `TestActorEgressHTTPSByHostnameMITM` both
pass, and the `networking` suite is green at 25 passed. The two modes
are mutually exclusive by design:
`TestActorEgressHTTPSByHostnamePassthrough` covers the plain gateway and
skips under MITM, and the two MITM tests skip without it. Both modes
were run, so every case executed in one of them.
- The new unit tests use a relocated namespace and renamed
ServiceAccounts. Reintroducing each hardcoding makes them fail.
One e2e caveat that predates this series and misleads: the `identity`
suite calls `ReplaceEgressTrustPool`, which takes over the shared
`egress-mitm-ca-pool` Secret, overwrites it and registers no cleanup.
Any MITM test running afterwards fails with `certificate signed by
unknown authority`, which reads as a broken interception path rather
than a mutated fixture. Reinstall the gateway before running
`egressmitm`.
# Additional Notes
**Credential contents stay canonical.** `controlapi/actor.go` still
mints credentials naming `api.ate-system.svc`: the JWT issuer and the
certificate's Issuer CN. Relying parties validate these, so changing
them is a compatibility decision, not a lookup fix. The code carries a
TODO to make the issuer a globally unique, OIDC-compliant name.
Closes#1802.
## What this does
Deletes the `ate.scheduler.eligible_workers` histogram. Nothing replaces
it in this PR.
## Why
**It is expensive on the resume path.** `Schedule` built a second slice
of the matching workers that scheduling never read, walked it again, and
repeated `HasRoom` for each entry. `HasRoom` parses resource quantity
strings up to three times for each worker. The metric roughly doubled
the per-worker arithmetic of a placement. The filter loop is now one
pass over the fleet and builds one slice.
**A metric cannot answer the question it was built for.** #564 wanted to
tell a full fleet apart from an empty intersection of the constraints,
such as a cordoned node behind a node requirement. That needs the
selectors, the node requirement and the actor.
`docs/metrics/substrate.yaml` bars all three from a metric label and
sends them to logs and spans, so no shape of this instrument reaches the
answer. A log record at the rejection is the replacement the issue
names, and it is not in this PR.
## Also removed
`ate.scheduling.constraint` goes with the instrument. No other signal
used the attribute.
## Scope of the change
- `cmd/ateapi/internal/scheduling/metrics.go` deleted, along with the
`WithMeter` option and the second filter pass in `Schedule`.
- The instrument and the attribute removed from
`docs/metrics/registry/metrics.yaml`. The `pool-keys-paired` exception
in `docs/metrics/substrate.yaml` dropped, because the empty pool pair
was this instrument's alone.
- `docs/observability.md`: the table row and the two label notes
removed.
- The e2e collector check and the label assertions for the instrument
removed.
- `TestSchedule_EligibleWorkersMetric` removed. The behaviors it covered
(draining workers, a sandbox class mismatch, busy workers) are already
in the `TestSchedule` table.
## Testing
`go test ./cmd/... ./internal/...` passes. `make verify` passes except
`hack/verify/metrics.sh`, which needs Weaver or Docker. Neither is
available on this machine, so CI is what checks the registry.
Label the four fields that carry credentials or user-supplied secrets
with the standard protobuf debug_redact option:
- ateapi EnvVar.value actor env var values
- ateapi MintActorJWTResponse.actor_jwt a bearer token
- atelet EnvEntry.value actor env var values
- credprovider FetchSecretResponse.opaque_bytes a fetched credential
The option is descriptor metadata only. It does not change the wire
format, the generated Go or Python API, or any runtime behaviour on its
own; Go's protobuf library does not act on it. It exists so that log
redaction can find sensitive fields by asking the schema instead of
matching field names, and so that new sensitive fields are labelled in
the same line that defines them. The interceptor that logs RPC bodies
today still clears only fields named "env"; making it honour this label
is a separate change.
Generated files are refreshed with hack/update/codegen.sh; the only
generated changes are the embedded descriptor bytes.
First step to fix#1743
> It's a good idea to open an issue first for discussion.
- [ ] Tests pass
- [ ] Appropriate changes to documentation are included in the PR
The exemptions file is generated / maintained by the `apitool` so we
want to be strict and fail if it's not sorted (the tool uses
deterministic ordering already).
This is to prevent cases where entries are manually edited and
subsequent calls to `apitool validate --update` generate spurious
changes (like what happened in commit 1e56e66b).
Also update the exemptions to make sure the entries are properly sorted.
* Fixes some minor inconsistencies
* Deletes the shell scripts (at least some of them)
* Creates a temporary shim in the old shell to call the new ate-setup
command.
Part of #1550
This PR adds `kubectl ate get` and `kubectl ate create` support for
`egress-policy`.
- `get` prints a table by default, or a bare JSON/YAML document with
`-o`.
- `create` consumes that same document. `-f -` reads stdin.
- Omitted `metadata` is filled from `--atespace` and the fixed name
`default`; a manifest naming another atespace is rejected before any
RPC.
- `get` accepts exactly one actor for now; a follow-up adds several
actors and a list document.
Recommend reviewing the four commits one at a time:
- printer and manifest decoder with a round-trip test,
- `get` command,
- `create` command,
- then the README updates on their own.
- [x] Tests pass:
- unit tests and `make verify`
- https://github.com/ygao-g/substrate/pull/33 against this head on a
local kind cluster;
- Also tested the new `test-egress.sh` step on a local kind cluster;
- [x] Appropriate changes to documentation are included in the PR.
🤖 This PR was developed with AI assistance. I have reviewed and tested
all changes.
Fixes#1474
`atenet.router.route.duration` carries two labels that the router filled
in wrongly on a failure path.
## `ate.router.resume` said "none" for a failed resume
`none` means "the resume found the actor already running" — the warm
route. The router also gave `none` to every failed resume, every
canceled caller, and every caller shed by a full parking lot. Those
failures landed in the warm-route series, so they did not show in the
cold-start rate and they polluted the warm-route latency.
A gRPC code cannot recover the fact. A canceled leader's flight outlives
its request and keeps restoring the actor, and a `DeadlineExceeded` can
land in the middle of a restore. So the PR does not classify the error.
It reserves `none`, `triggered` and `joined` for a resume that
completed, and adds a fourth value, `unknown`, for every resume that did
not:
| Case | Before | After |
|---|---|---|
| Actor already running | `none` | `none` |
| Leader completed a cold activation | `triggered` | `triggered` |
| Joiner waited on a completed cold activation | `joined` | `joined` |
| Resume failed (leader and joiners) | `none` | `unknown` |
| Caller canceled before the flight finished | `none` | `unknown` |
| Caller shed by a full parking lot | `none` | `unknown` |
| Egress, which never resumes an actor | `none` | `unknown` |
## `ate.template.*` was the empty string on a failure
The registry marks `ate.template.atespace` and `ate.template.name`
required on this metric, but the router sent an empty string when it
failed before it resolved a template.
`ateattr.NormalizeTemplateDimension` now sends the constant `"unknown"`
instead. The constant does not come from the request, thus it adds no
caller-controlled label value — the cardinality rule in
`docs/metrics/substrate.yaml` still holds, and the PR records the
exception there.
## How to read the change on a dashboard
A query that already splits by `ate.router.resume` gains an `unknown`
series and loses the failures that used to hide inside `none`. A `none`
rate reads lower after the change and a cold-start rate is unaffected.
Do not read the time in the `unknown` series as an activation time.
## Documentation
- `docs/metrics/registry/metrics.yaml`: the `unknown` member of
`ate.router.resume`, the `"unknown"` fallback on the two template
labels, and a note on the metric.
- `docs/metrics/substrate.yaml`: the cardinality-rule exception.
- `docs/observability.md`: the label list for the metric.
## Tests
- `resumer_test.go`: a failed flight gives `unknown` to the leader and
to all nine joiners; a canceled caller gives `unknown`; a shed caller
and a shed joiner give `unknown`.
- `metrics_test.go`: empty template dimensions record as `"unknown"`;
`Result.resume()` defaults to `unknown`.
- `ateattr_test.go`: `NormalizeTemplateDimension` and the four
`RouterResume*` values.
- [x] Tests pass
- [x] Appropriate changes to documentation are included in the PR
Kata 4.1.0 bundles virtiofsd 1.14.0 (required by Substrate). This
simplifies the dev process on arm64 because it removes the need to build
virtiofsd from source.
Verified on an arm64 KVM host (Lima + kind).
Fixes#1695
> It's a good idea to open an issue first for discussion.
- [x] Tests pass
- [x] Appropriate changes to documentation are included in the PR
The Kubernetes default strategy (`maxSurge=25%, maxUnavailable=25%`) can
stall small pools when a workerpool edit and get k8s deployment rolling:
below 4 replic, as `maxUnavailable` rounds to 0, the surge pod needs to
land first, and if existing pods already take all spaces of their node,
the new one never lands and the rollout is stuck.
Use `maxSurge` 0 with `maxUnavailable` 10%, which Kubernetes will
evaluate as removing remove least one pod first so it will never block.
Also modify `progressDeadlineSeconds`to be 3600s so a batch waiting out
the drain window (currently 30min) is not reported as a failed rollout.
- [ ] Tests pass
- [ ] Appropriate changes to documentation are included in the PR
Add an example gRPC credential-provider plugin which reads from k8s
secrets for the egress
credential-injection path.
- It resolves `ate-secret://k8s.io/default/<namespace>/<secret>/<key>`
URIs to Kubernetes Secret values, so Substrate never stores secrets — it
only
brokers a read that the provider is authorized to perform.
- It is the only
component in the injection path with Kubernetes secerts access; the
egress gateway and
its injector never read Secrets directly.
### What's included
- **`cmd/credential-provider/kubernetes-secrets`** — the gRPC service:
- Parses and validates `ate-secret://` URIs, resolving the requested
Secret
(with single-key fallback when the URI omits a key).
- Enforces an **atespace→namespace authorization policy**
(default-deny),
derived from the caller's attested actor SPIFFE ID.
- Serves over **mutual TLS**, requiring the caller's client cert to
chain to
the trust bundle *and* carry the egress injector's SPIFFE SAN.
- **Manifests** (`manifests/egress-credential-injection/`) — Deployment,
Service, ServiceAccount + RBAC, a sample namespace-policy ConfigMap, and
a
sample Secret.
- Renames the default provider address/service from `credprovider` to
`k8s-credential-provider` across `ate-setup`, install scripts, and the
egress
injection overlay.
### atespace → namespace authorization
Beyond the mTLS check that only the egress injector may call the
provider, each
request is authorized against a **default-deny atespace→namespace
policy**. The
provider derives the requesting actor's atespace from its attested
SPIFFE ID and
resolves a Secret only if that atespace is explicitly granted access to
the URI's
namespace.
Fixes#1692
Adds a SWE-Perf benchmarking workload to the benchmarking suite,
exercising
actor suspend/resume against a real SWE-bench task image
(`astropy-7336`)
rather than a synthetic load.
The workload runs a recorded agent trajectory in configurable cycles,
suspending and resuming the actor between each, so the benchmark
measures
lifecycle cost under a realistic in-sandbox server.
### What's here
| Area | Change |
|---|---|
| `internal/benchmarking/boomer/sweperf/` | New `SweperfUser` boomer
user class (+ tests) |
| `internal/benchmarking/boomer/dynconfig/` | `sweperf_template`,
`sweperf_total_steps`, `sweperf_num_cycles` knobs |
| `benchmarking/locust/common/sweperf_config.py` | `--sweperf-*` Locust
flags |
| `benchmarking/locust/tests/sweperf.py` | Stub user class so the master
attributes boomer's stats rows |
| `benchmarking/workloads/manifests/` | `swebench-astropy-7336`
ActorTemplate |
Defaults: 21 trace steps partitioned into 4 cycles.
### Verification
- `hack/verify-all.sh` — pass
- `go test -race ./internal/benchmarking/... ./cmd/benchmarking/...` —
pass
- End-to-end on a GKE dev cluster (gVisor), 1 user:
```
state: running | users: 1 | fail_ratio: 0.0
NAME REQ FAIL AVG_ms
CreateActor 1 0 1
CreateAtespace 1 0 1
ResumeActor 20 0 495
SuspendActor 20 0 947
Workload_Cycle_1 5 0 1996
Workload_Cycle_2 5 0 964
Workload_Cycle_3 5 0 4877
Workload_Cycle_4 5 0 3022
```
164 requests total, 0 failures, 0 errors.
The ActorTemplate image is currently pinned to a personal Artifact
Registry
repo; a shared public registry for these SWE-Perf task images is
planned.
- [x] Tests pass
- [ ] Appropriate changes to documentation are included in the PR
Today, `ateom` fetches the `kata-config` asset (configuration-clh.toml)
to read only 3 values `default_memory`, `default_vcpus` and
`kernel_params`. The first 2 are redundant because (1) the values never
change and (2) ateom has the same defaults. Only `kernel_params` is
relevant for the `kata-agent`; its value also changed once in the Kata
project history.
`ateom` now owns all three values, which also allows us to tune them
specifically for Substrate.
Verified e2e with a GKE cluster.
Fixes#1693
> It's a good idea to open an issue first for discussion.
- [x] Tests pass
- [x] Appropriate changes to documentation are included in the PR
Record which egress protocols are allowed, which are allowed only under
policy controls, and which are blocked, along with the data path each
one takes. Having this written down gives users a single place to check
what Substrate lets an actor reach, and gives us a checklist to
implement and test against before GA.
Point readers at the issue tracker so that requests for traffic we do
not yet support arrive with a use case attached.
Address #1339
> It's a good idea to open an issue first for discussion.
- [x] Tests pass
- [x] Appropriate changes to documentation are included in the PR
An actor lifecycle record went to stdout and to OTLP in two separate
calls. The attributes were shared, but the severity was not, so the slog
level sat at the call site and the OTel severity sat on the `Event`.
Nothing made a caller write both copies either, so a new record could
reach stdout only and no test would notice.
Since this PR `actorevent.Log` now writes both copies. The level comes
from `Event.Severity`, so it is stored once. The OTLP only `Emit` and
the exported body constants are gone, so the dual write is the only way
out of the package.
Also normalizes `ate.actor.operation.name` on the crash event, which the
state change event already did.
Fixes#1744
Testing
- Unit tests for the level mapping, the dual write, and the registry
check.
- End to end on a fresh kind cluster. Both event names arrive with the
right severity (9 and 17), the right attributes, and trace context on
the record fields. The stdout copies match record for record.
- [x] Tests pass
- [ ] Appropriate changes to documentation are included in the PR
This is a skill I've been iterating on, first with a few coworkers' PRs
only, and then progressively more broadly. It gives pretty decent
results with frontier models, but I think we need to continue refining
it.
The basic idea is that this creates or updates draft PR comments to add
AI findings, categorized by severity, and *clearly denoted* as coming
from an agent. It is instructed to bias towards brevity as these models
tend to write massive comments. That still needs more tweaking.
Reviewers are expected to leverage this skill to suggest draft comments
which they then review and delete as needed, and finally submit an
overall review with their own top-level comments.
It does NOT do things like "summarize what the PR does" ... PR authors
already do this, and adding that information to the PR thread is not
helpful. Anyone interested can ask their agent to summarize the PR for
them locally.
What it does do, is report possible bugs, for a reviewer to vet and then
post as they see fit.
NOTE: **this should be squash merged**
> It's a good idea to open an issue first for discussion.
- [x] Tests pass
- [x] Appropriate changes to documentation are included in the PR
* (Prefactor): Extract defaulting logic into
`cmd/ateapi/internal/defaults`.
* When reading resources from DB, perform a default backfill.
* Change defaults for readyz probes.
Part of #1593
The tag deletion logic is getting too complex, so let's move it to a
workflow, following what we do for other resources.
#1510#664
- [x] Tests pass
- [x] Appropriate changes to documentation are included in the PR
Part of #1677 — the graceful-shutdown half; the atelet-side janitor for
ungraceful exits is tracked there.
Nothing removes `ateoms/<podUID>/` when a worker pod goes away, so every
pool rollout leaks one directory (holding a dead `ateom.sock`) per
replaced worker — and since the stats sweep enumerates that directory as
its discovery registry (#961), each leak costs a dial+probe per sweep,
forever.
## Change
A `defer os.RemoveAll(ateomDir)` in both runtime mains, registered right
after the `MkdirAll` that creates the directory. ~7 lines each plus the
comment carrying the safety argument.
**Why this is safe:**
* **Same-pod container restart:** kubelet serializes containers — the
replacement is not started until this process, and therefore this defer,
has exited. The remove cannot race the successor's `MkdirAll`.
* **Shutdown ordering:** the SIGTERM path drains, `GracefulStop`s,
`Serve` returns, `do` returns — the defer runs after the listener is
already closed, so nothing can be dialing the socket it removes.
* **Boot-failure paths:** error returns before `Listen` also run the
defer; the pod restarts and re-creates the directory. Harmless.
**What it deliberately does not cover:** SIGKILL after the grace period,
OOM kills, node crashes — defers don't run there by nature. That residue
is the janitor's job (#1677); this change shrinks the janitor's caseload
to exactly those, since rollouts — the dominant growth driver —
terminate gracefully.
## Testing
Both runtime mains are untested boot plumbing (no unit seam exists for
`do()`); verified by linux builds and test-compiles of both packages.
**Live-verified on the ate-dev GKE cluster** (gVisor pool; the micro-VM
change is the identical lines):
* Baseline census of one node found **11 ateom directories with exactly
1 live pod** — ten stale leftovers from prior rollouts, the leak in the
wild.
* Rolled the pool onto an image built from this branch. The outgoing
pre-change pod (`445af5aa…`) **leaked its directory** on termination,
reproducing the bug side-by-side with the fix.
* Gracefully deleted a pod running this branch (`5618282c…`): its
directory **was removed** on shutdown, while the pre-change pod's leaked
directory remained in the same census — before/after behavior on the
same node, same listing.
Working on https://github.com/agent-substrate/substrate/issues/1563
## Summary
Initializes an embedded OpenFGA server within `ateapi` backed by
PostgreSQL and updates the top-level authorization scope.
## Key Changes
- **Rename scope to `global`**: Renamed `type cluster` and
`parent_cluster` to `type global` and `parent_global`, adding
`can_set_policy` and `can_get_policy` to `global`.
- **Embedded OpenFGA server (`internal/authz/server.go`)**:
- Compiles `model.fga` into OpenFGA's protobuf representation.
- Runs OpenFGA PostgreSQL schema migrations (`goose_db_version`).
- Serializes startup via `pg_advisory_lock` to avoid multi-replica
races.
- Connects OpenFGA's PostgreSQL adapter with a new dedicated
`*pgxpool.Pool`.
- Idempotently creates or reuses the `"substrate"` store and
authorization model.
- **Service wiring (`cmd/ateapi`)**: Exposes `Pool()` on
`*atepg.Persistence` and initializes `authz.NewServer` on bootstrap.
## Verification
- `openfga model test`: 103/103 checks passing (`model_test.fga.yaml`).
- `go test -mod=mod ./internal/authz/...`: Passes against PostgreSQL
(migrations, tuple writes, permission checks, and restart idempotency).
As we're getting closer to GA, it's important to start documenting the
network contracts, especially where we want things to be pluggable. This
PR takes a stab at documenting the contract between atunnel and the
egress PEP. It doesn't have a policy section since that's evolving, but
this aims to be a good first step.
/cc @EItanya @thockin @bowei @LiorLieberman
- [X] Tests pass
- [X] Appropriate changes to documentation are included in the PR
---------
Signed-off-by: Keith Mattix II <keithmattix2@gmail.com>
#### Summary
This PR introduces the `RevertActor` RPC for actor lifecycle management.
It includes the API definition, the corresponding workflow execution
logic, observability metrics, and updates to the authorization model to
support reverting actors.
Fixes#1556
Docs updated in #1711
#### Commit-wise Changes
**1. Add RevertActor RPC (`1145d99`)**
* Introduces the new `RevertActor` RPC to the API definitions.
* Updates the corresponding protobuf bindings (affecting files like
`ateapi.pb.go` and `ateapi_pb2.py`).
**2. Add the revertActor workflow and observability metrics
(`dcb1fae`)**
* Implements the core `revertActor` workflow logic, designed to be
idempotent and re-enterable. It progresses through the following steps:
* **Mark Reverting:** Validates that the actor is in a revertable state
(`RUNNING`, `PAUSED`, or `CRASHED`) and transitions its state to
`REVERTING`.
* **Discard Worker:** Safely tears down the execution environment by
terminating the workload, detaching volumes, and releasing the assigned
worker.
* **Collect In-Progress Snapshot:** Cleans up external object storage by
deleting any objects a previous suspend operation was partway through
writing.
* **Finalize:** Commits the actor to `SUSPENDED` and strips all
node-local and in-progress state pointers (clearing `WorkerAssignment`,
`LocalSnapshotInfo`, etc.), returning the actor to its untouched
external snapshot.
* Instruments the workflow with lifecycle operation metrics (e.g.,
updating `ate.actor.lifecycle.operation.duration` to track `revert`
operations).
* *Note/TODO:* Currently, when reverting a paused actor, the workflow
drops the pointer to the node-local state but does *not* actually prune
the local checkpoint bytes from the node (this is tracked in #641).
**3. Add `can_revert` to the authorization model (`2fc53ae`)**
* Adds the `can_revert` permission to the auth model, mirroring the
shape of `can_suspend` (editor tier of the parent atespace, plus a
direct grant so a machine identity can revert the actor it drives
without holding an atespace role).
**4. Serve RevertActor and add the CLI verb (`f04fab4`)**
* Wires the `Control.RevertActor` service method to the workflow
(replacing the generated stub that previously answered `Unimplemented`).
* Adds the `"ate revert actor"` CLI command, making the feature usable
end-to-end.
* Implements `Terminate` for the fake atelet. This was necessary because
reverting an actor from the `RUNNING` state is the first path to reach
this call in functional tests (previously, delete tests skipped this
step as they ran against actors with no worker assignment).
**5. Add a manual verify script for RevertActor (`8022db3`)**
* Adds a script to manually exercise `RevertActor` against a real
control plane, since unit and functional tests only run against a fake
atelet.
* Tests reverting from `CRASHED`, `RUNNING`, and `PAUSED` states, and
verifies that attempting to revert a `SUSPENDED` actor is properly
rejected.
* Simulates a crash by deleting the worker pod the actor runs on to
verify the workflow can handle the absence of a worker to terminate.
- [ ] Tests pass
- [ ] Appropriate changes to documentation are included in the PR
## What this does
ateapi writes a record every time an actor changes state. Until now
those records only went to the pod's stdout, and nothing reads stdout.
This sends the same records to a collector as OTLP log events.
Actor name and uid cannot be metric labels (too many values), and traces
are sampled at 1%. So these records are the only way to answer "what
state is this actor in, and since when".
## Changes
- `serverboot.InitLogging` sets up a LoggerProvider, next to the
existing tracer and meter ones.
- New `internal/actorevent` package builds the log records.
- ateapi emits at the two places that already write the stdout records.
- Two event names: `ate.actor.state_changed` and `ate.actor.crashed`.
- Both names are registered in `docs/metrics/registry/events.yaml`, so
`make verify` checks them.
- kind gets a logs pipeline and a count connector. The e2e suite reads
the counts back.
- Docs updated. `otel-collector.md` said substrate has no
LoggerProvider, which is no longer true.
## Opt-in
`OTEL_LOGS_EXPORTER` defaults to `none`. Only the kind overlay sets it
to `otlp`. The base ConfigMap is untouched, so no deployed environment
changes when this merges.
## Notes on the design
- **No slog bridge.** Only two call sites emit these records, so
emitting twice costs two lines. A bridge would also send every ateapi
log over the wire, could not set the event name, and would loop, because
SDK export errors are logged through slog.
- **Batching processor, not the simple one.** These records sit on the
actor resume path. A processor that exports inside the emit call would
add a blocking gRPC call there, so a slow collector would become control
plane latency.
- **Two event names, not one per state.** `ate.actor.state` already says
which transition happened. A crash gets its own name because it carries
two extra attributes and a higher severity.
- **Both copies are kept on purpose.** No collector in this repo reads
pod stdout, so nothing is duplicated today. `kubectl logs` keeps
working. If a filelog agent is ever added, drop one of the two. The
escape hatch is written down in `docs/metrics/substrate.yaml`.
## Dependencies
Adds `otel/log`, `otel/sdk/log` and `otlploggrpc`, all pinned at
v0.20.0. That is the release that matches the pinned `otel v1.44.0`.
v0.21.0 would pull the core modules to v1.45.0, which this change does
not need. The logs API has a
v1.47.0 release candidate upstream, so it is on its way to stable.
## Testing
- Unit tests for the exporter resolver, the record builder, and both
ateapi emit sites.
- The record builder test checks the attribute set matches what the
event name declares, in both directions.
- The ateapi tests check the OTLP record carries the same attributes as
the stdout record.
- Ran end to end on kind. Records arrive with the right event name,
severity, attributes, and with trace context on the record's own fields
rather than as attributes.
- Checked the off state too. With `OTEL_LOGS_EXPORTER` removed, the
collector receives no log records and stdout is unchanged.
- [x] Tests pass
- [x] Appropriate changes to documentation are included in the PR
Dialing by resolving the worker pod is problematic, because in some
cases (e.g. during Worker Pod deletion), we need to dial to atelet (to
properly call `Terminate`), but the worker is gone.
A much simpler model is to always dial by resolving the IP of the atelet
on a given node.
As the number of resources in the Substrate API grows, having one file
for each (resource x verb) is becoming hard to manage. We recently
started consolidating all commands for a each resource in a single file
(see tag.go), similar to how we organize RPC handlers in the
ate-apiserver `controlapi` package.
This PR re-organizes the resources that are sill spread across multiple
files.
The startup registered-worker scan read its backoff from a package-level
var that tests overwrote. Syncers started by earlier tests outlive them
and keep reading that var, so the write races the read and `go test
-race` fails, in whichever test happens to do the write.
Hold the schedule on the syncer instead, so a test only ever shrinks the
backoff of the syncer it owns.
Fixes a flake I encountered on one of my other PRs. I didn't see an open
issue or fix but this is pretty straightforward.
The test released the blocked fetch as soon as the first caller reached
the client, then required exactly one fetch for eight callers. A caller
that had passed the cache miss but not yet reached the flight starts its
own fetch once the first one completes, so a slow runner failed the test
on scheduling alone.
Run it in a synctest bubble instead: synctest.Wait returns only once
every caller is parked on the fetch, so releasing it cannot race them.
Fixes a test flake I encountered on an unrelated PR.
The microvm demo is unnecessarily upsized vs the gvisor counter demo,
and we create larger pools than we need.
CI is relatively tight on resources, and over-sizing uVM actors also
costs boot time, snapshot size etc.
The suite oversubscribes one worker with two actors, but built its pool
from the counter demo, so whether the worker filled after one actor
depended on the runtime's per-worker actor limit rather than on anything
the suite declared.
Declare both halves in a probe fixture instead, the way the sized probe
does: a one-worker pool whose limits are one of its own actors. The
fixture also drops the suite's dependency on the counter demo's sizing,
and /whoami lets the served request assert which actor answered.
Pre-req for #1266
Nested virtualization exposes `/dev/kvm`, which microVMs need. The new
`--enable-nested-virtualization` flag turns it on for the node pool
`setup-gcp` creates. It defaults to on, so pass
`--enable-nested-virtualization=false` for a cluster that does not need
it.
Also, rename the env var `GVISOR_NODE_MACHINE_TYPE` to
`NODE_MACHINE_TYPE` to be consistent with other env vars and to avoid
confusion for microVMs. The old env var still works with a warning.
- [x] Tests pass
- [x] Appropriate changes to documentation are included in the PR
A per-test WorkerPool copied the replicas, image, and sandbox class of
the installed pool but not its pod template. Its workers declared no
resources and so advertised no compute capacity. That did not matter
while a worker held a single actor, because placement compared actor
counts.
Copy the resources too. The parking suite needs them: a request parks
when the worker is full.
Related to #1266
When we start advertising multiple actor support, this becomes a
problem. Breaking out this small fix.
Step 1 of #1640 (adapting the actor resource-telemetry stack to
multi-actor workers, #1266): the discovery read's wire shape breaks in
place, ahead of the runtime work, while the capacity cap still holds
every list to at most one entry.
## Contract
`GetActiveWorkloadStatsResponse` becomes:
```proto
message GetActiveWorkloadStatsResponse {
repeated WorkloadStatsSample samples = 1;
}
```
The `oneof {sample | no_sample_reason}` and the `NoSampleReason` enum
are deleted outright — no `reserved` tombstones, per the accepted-skew
stance below.
* **Available** = the empty list. "Available" stopped being a *reason*;
it is the absence of workloads.
* **Not measurable yet** (boot, restore, teardown, a lifecycle
transition underneath the lock-free read) = a **pending entry**: full
attribution + sandbox class with `source = STATS_SOURCE_UNSPECIFIED`,
whose "not measured" meaning `WorkloadStatsSample` already defines. It
became a per-workload statement because with N workloads it can't be a
response-level one — and it closes an old gap: a workload that dies
during boot is attributable to a blind caller instead of anonymous.
* The keyed `GetWorkloadStats` is untouched: uid-addressed, per-actor
codes correct at any occupancy.
## Consumers
The atelet sweep folds every entry of every response; nothing in it
assumes at most one. Pending entries contribute nothing anywhere — not
to the aggregates (`sampled_actors` keeps meaning "measured"), not to
the CPU delta baselines, and no usage event — exactly matching the old
no-sample behavior, so metrics and events are byte-identical before and
after this change on a capacity-1 fleet.
## Version skew: broken
There is no dual population, no fallback decode, no rollout gate (per
the decision recorded on #1640). Given that this is prior to GA, I
recommend making the breaking change in favor of code simplicity..
## Testing
* Both runtimes: available → empty list; mid-boot → one pending entry
asserted in full (attribution, class, `observed_at` set);
transition-mid-read → pending entry for the new occupant / empty when
the slot cleared; the discovery sample stays byte-identical to the keyed
read's against the same fixture.
* atelet: pending entries skipped for aggregates, baselines, and events;
multi-entry folding exercised by the loop shape.
* `verify-all.sh` clean end to end (includes proto regen check and the
metrics registry).
Part of #1640, toward #1266.
When a worker pod crashes or is deleted out-of-band, `ateapi`
transitions the actor to `CRASHED` and clears `worker_assignment`
without calling `atelet.Terminate`. If the actor is later recovered and
scheduled onto a new worker pod on the same node,
`systemInfoVolumeRefresher.Register` panicked on the leftover entry in
r.actors.
Supersede any existing registration for the same actor UID by marking
the previous entry stale under its mutex and replacing it in r.actors.
Temporary fix for #1710
- [X] Tests pass
- [X] Appropriate changes to documentation are included in the PR
Benchmarking: multi-actor glutton VUs, live window, resume retries,
client-side latency
Glutton actors per VU. Each VU creates --actors-per-user actors on
startup and cycles through them round-robin, so one goroutine can drive
many mostly-idle actors. runner.py forwards the flag to boomer-glutton.
A crashed actor stays crashed for the run (ateapi never rehabilitates
it), so it is marked on the first Aborted "crashed" error, skipped from
then on, and counted in a CrashCount stat.
Live window. --min/--max-live-time (default 0-0) set how long an actor
stays resumed between its first ping and the suspend. Up to
--max-pings-per-wake pings (default 1) run inside it, spaced 0.2-1.0s
apart. The wait window (--min/--max-wait-time) remains the gap between
one actor's suspend and the VU's next resume, and every return path
sleeps it, so a failing startUser, resume, or crashed actor does not
spin on boomer's zero-delay re-entry. With the defaults the cycle is
resume, ping, suspend, wait: the same shape as before.
Resume retries. ResumeActor retries ateapi's transient "concurrent
update conflict" Aborted up to five times with a 50ms backoff, inside
the timed call, so the conflict no longer shows up as a failure.
Client-side latency. Every gRPC row in the locust stats now reports
client wall clock, which covers retries, queueing, and the network. The
server's elapsed-time trailer stays on the trace span only.
Worker robustness. The router HTTP client keeps up to 10000 idle
connections per host so each VU reuses its connection across wakes. A
failed dynconfig fetch after the first successful one keeps the last
fetched values instead of exiting the worker.
Orchestrator. util.run logs the duration of each shell command.
Fixes #<issue_number_goes_here>
> It's a good idea to open an issue first for discussion.
- [ ] Tests pass
- [ ] Appropriate changes to documentation are included in the PR
Issue : #1665
## Problem
Once a container doesn't exists on worker anymore, ateom can never tear
the actor down again. `runsc state` and `runsc delete -force` both
fatal-exit 128 on a container runsc has no record of (`loading
container: file does not exist`; the `-force` not-found tolerance sits
after the spec fetch that fails first).
`cleanupContainers` returned that error, `TerminateWorkload` failed, and
ateapi skipped releasing the worker. The actor stayed `DELETING` and its
share of the worker's capacity stayed booked; every retry hit the same
128 at the `state` check.
The record goes missing in two observed ways:
- The first `runsc delete` after the sandbox is lost removes the record
and still exits 128, because gVisor's `Destroy` runs every step and
reports errors at the end. Seen when the ateom container was OOM-killed
and restarted: the sandbox and its rootfs mounts were gone, the records
were not, and the delete failed on the missing filestore after erasing
the record.
- A request deadline or cancellation landing mid-`runsc delete` SIGKILLs
runsc partway through destroying the container.
## Change
`cleanupContainers` now treats "runsc has no record of it" as already
destroyed:
- When `runsc state` errors, it runs `runsc list` and skips the
container if it is not listed. `runsc list` loads no container, so it
never fatal-exits on a missing one, which distinguishes "already gone"
from a real failure without matching on runsc's error text. It runs only
on the error path; the success path is unchanged.
- `stopContainers` and `cleanupContainers` run detached from the request
context with their own 30s timeout, as the post-checkpoint resume
already does, so a caller deadline can no longer SIGKILL runsc
mid-delete. Both `TerminateWorkload` and `CheckpointWorkload`'s
post-checkpoint cleanup go through this path.
Remove mounted and unmounted leftovers before namespace creation. Reject
invalid names and avoid following symlinks during unmount. Existing
namespace handles remain valid.
Tiny pre-req for #1266
Also considering anonymous namespaces or per-activation IDs but eh we
can revisit that later.
Adds **egress credential injection**: on the sdsmint egress gateway's
decrypted MITM leg, when an actor's `EgressPolicy` rule matches and
carries an `inject_static_headers` effect, the gateway resolves the
referenced credential from a **credential provider** and sets it as a
request header (e.g. `Authorization: Bearer <token>`) before
re-originating upstream.
This implements the `applyEffects` in the egress handler. Injector runs
as part of the existing egress ext_proc handler that already
fetches/caches/evaluates each actor's `EgressPolicy`, so the MITM leg
gains injection without a second ext_proc hop.
### How it works
- New `CredentialProvider.FetchSecret` plugin API
(`pkg/proto/credproviderpb`): the gateway calls it with the credential
URI and the actor's attested SPIFFE identity; the provider authenticates
the gateway (mTLS) and resolves the secret.
- The egress handler dials the provider over mTLS
(`egress.DialProvider`) and injects on matched, allowed requests
(`egress.applyEffects`).
- Behavior:
- **TLS MITM leg + provider configured** → inject the credential
(overwriting any actor-set header).
- **Cleartext leg, or no provider configured** → skip injection and pass
the request through (never put a secret on a cleartext wire; don't block
allowed egress).
- **Attempted but failed** (unfetchable secret, empty/malformed secret,
credential URI of an unserved provider class) → fail closed.
## Installation
One flag stands up and wires the whole stack:
```
hack/install-ate.sh --deploy-ate-system --experimental-egress-credential-injection
```
`--experimental-egress-credential-injection` deploys credprovider and
the injector, then re-wires the sdsmint egress gateway to route through
the injector. It implies `--experimental-use-sdsmint`
### New install flags
Two flags configure which credential provider the injector targets:
| Flag | Purpose | Default |
|---|---|---|
| `--credential-provider-name` | Provider class, as a `ate-secret://`
prefix; a policy URI of any other class is refused |
`ate-secret://kubernetes.io` |
| `--credential-provider-address` | Where the injector dials the
provider | `credprovider.ate-system.svc:50051` |
## Why
`TestActorIdentity_AfterRestore_IsOwnID_NotGolden` flaked twice on
microVM, once on `main` (run 35011781948) and once on #1667 (run
35023632017, attempt 1):
```
identity_test.go:197: after suspend/resume: /run/ate/trust-bundle.pem = "", want the rotated sanitized bundle ... (probe read error: "open /run/ate/trust-bundle.pem: input/output error; ")
```
## Root cause
Race condition when the trust bundle is rotated in the test:
1. atelet rewrites the bundle by rename. The rotation replaces the file
at /run/ate/trust-bundle.pem with a new inode. The old inode is unlinked
but the guest still references it from its earlier reads.
2. The rename can land during the checkpoint. atelet only stops the
refresher after the ateom checkpoint returns, so nothing prevents the
rewrite from happening between the guest's last lookup and the snapshot.
3. virtiofsd cannot migrate an unlinked inode. In find-paths mode it has
to find a path for every inode the guest holds. The old bundle no longer
has one, so guest-error mode marks it faulty on restore, which means EIO
instead of a failed restore.
4. The guest still trusts its cached dentry. Guest jiffies do not
advance while the VM is paused, so the dentry cached just before suspend
is still within its timeout right after resume. The first open goes to
the faulty inode and gets EIO. Once the dentry expires, a fresh lookup
finds the regenerated file and reads work again.
The other files under `/run/ate` weren't rewritten in that window, which
is why only the trust assertion fails.
## Fix
The live-refresh phase of the same test already polls with
`waitForTrust` for up to two minutes. Do the same after resume. The test
still asserts that the resumed actor sees the rotated bundle whichever
side of the suspend the rewrite lands on. It no longer requires that on
the first read.
## Testing
`go vet ./internal/e2e/suites/identity/` passes.
Ran this 30 times on my own machine to verify it de-flakes the test
using:
```
E2E_SANDBOX_CLASS=microvm hack/run-e2e-kind.sh ./internal/e2e/suites/identity \
-count=30 -timeout 3h -run 'TestActorIdentity_AfterRestore_IsOwnID_NotGolden'
```
To build a snapshot URI, we need the external storage location (which is
stored in the actor template), actor atespace and UID (which are stored
in the actor resource). If an actorTemplate was deleted, we'd leak the
in-progress snapshot, because the storage information was gone.
Fixes#1608
- [x] Tests pass
- [ ] Appropriate changes to documentation are included in the PR
We shouldn't have a bunch of different gRPC services for atelet to
provide services to ateoms. Combine the two that currently exist into
one (AteomSupport).
#1522 added a resyncInterval parameter to NewActorTemplateReconciler and
#1523 added this call site. They landed without seeing each other, so
`main` doesn't compile.
SEt the resyncInterval to 7s, to match what we have in other tests
- [x] Tests pass
- [x] Appropriate changes to documentation are included in the PR
Fixes#1507
Golden snapshots currently remain owned by the temporary golden actor,
so another resume/suspend cycle or actor deletion can collect a snapshot
still referenced by its template. The controller now copies the warmed
snapshot into a published tag, deletes the golden actor, and records the
tag reference on the template. Interrupted tag creation and cleanup
remain retryable; template deletion cleans up both resources.
`CreateActor` resolves an explicit `sourceTag` or the template's golden
tag into the actor's initial snapshot. Actors created before the golden
tag is ready retain their cold-boot behavior. The golden tag uses the
template UID as its name in `ate-golden`. The proto replaces
`golden_snapshot` with `golden_tag` at field 1, without backward
compatibility.
This PR is based directly on `main` and does not depend on #1521.
Follow-up recommendation: move the create → resume → wait → suspend →
tag → delete sequence into a golden-template workflow using the existing
workflow conventions. The reconciler now coordinates multiple
recoverable steps; it could retain scheduling and retries while
delegating that sequence to the workflow. This refactor is outside this
PR.
- [x] Tests pass
- [x] Appropriate changes to documentation are included in the PR
Validation: full `env -u NO_COLOR make verify` passed after rebasing
onto `main`. After the final proto field-number change, bindings were
regenerated and the control API unit/functional tests plus proto-format
and Go-format checks passed.
---------
Signed-off-by: Eitan Yarmush <eitan.yarmush@solo.io>