The `port` and `containerPort` were removed from AgentTemplate in #201.
Also adds the `PORT` env var for agent-secret to match sandbox and
ensure it is configured explicitly.
Fixes#273
- [X] Tests pass
- [ ] Appropriate changes to documentation are included in the PR
This change removes the hard-link from templates to worker pools,
allowing pools to be selected using k8s labels.
Both actors and actor templates are augmented with a label selector
field.
When an actor is resumed, we resolve the set of *eligible* worker pools
by evaluating the intersection of both actor and template selectors.
Then we find a free worker from one of them. The overall design is
described in [1].
[1]:
https://github.com/agent-substrate/substrate/issues/47#issuecomment-4712334620
Fixes https://github.com/agent-substrate/substrate/issues/225
- [x] Tests pass
- [x] Appropriate changes to documentation are included in the PR
Tested local setup:
```
❯ kubectl exec -n ate-system valkey-cluster-0 -- cat /tmp/valkey.conf | grep cluster-announce
cluster-announce-hostname valkey-cluster-0.valkey-cluster-service.ate-system.svc
cluster-announce-tls-port 6379
cluster-announce-bus-port 16379
```
Separate from this, we should decrease the number of clusters, because
right now all of these get created:
```
❯ kubectl exec -n ate-system valkey-cluster-0 -- valkey-cli --tls --cacert /etc/valkey-ca/ca.crt --cert /run/servicedns.podcert.ate.dev/credential-bundle.pem --key /run/servicedns.podcert.ate.dev/credential-bundle.pem cluster nodes
d6ca3f519ddcf774bc8df68f7b7ab6c82cc35066 10.244.0.25:6379@16379,valkey-cluster-2.valkey-cluster-service.ate-system.svc master - 0 1781721223000 3 connected 10923-16383
6f66ad32594c1a5dc550729434996eb129112a1f 10.244.0.27:6379@16379,valkey-cluster-3.valkey-cluster-service.ate-system.svc slave d6ca3f519ddcf774bc8df68f7b7ab6c82cc35066 0 1781721224614 3 connected
c72d9e9bd165b0f800a135fbda1d8184e8111319 10.244.0.28:6379@16379,valkey-cluster-4.valkey-cluster-service.ate-system.svc slave 1c37cdd7fdcc0a261614a6e3936cc0043fe63d35 0 1781721223594 1 connected
54690be32b6647d43e5e1171e6db92ddf9515ad7 10.244.0.20:6379@16379,valkey-cluster-1.valkey-cluster-service.ate-system.svc master - 0 1781721224614 2 connected 5461-10922
6d862697f8af8750a5280409d1eb2bce9f3f8e0a 10.244.0.26:6379@16379,valkey-cluster-5.valkey-cluster-service.ate-system.svc slave 54690be32b6647d43e5e1171e6db92ddf9515ad7 0 1781721223594 2 connected
1c37cdd7fdcc0a261614a6e3936cc0043fe63d35 10.244.0.22:6379@16379,valkey-cluster-0.valkey-cluster-service.ate-system.svc myself,master - 0 0 1 connected 0-5460
```
https://github.com/agent-substrate/substrate/pull/267 is a quick fix to
decrease the pain of setting up the cluster + tearing it down. This
would fix the stale IP issue.
Signed-off-by: npolshakova <nina.polshakova@solo.io>
$(KO) != $(GO)
ko does not support any way to pass build flags directly. We have to
use GOFLAGS. Go does not fully shell-parse GOFLAGS, it just does simple
whitespace splitting and whole-value quoting.
E.g. this fails for good reason:
GOFLAGS="-ldflags=-X=foo=foo -X=bar=bar"
...becomes:
[ "go", "build", "-ldflags=-X=foo=foo", "-X=bar=bar" ]
...where `-X` is passed to `go build`
We can try:
GOFLAGS="-ldflags='-X=foo=foo -X=bar=bar'"
...but that becomes
[ "go", "build", "-ldflags='-X=foo=foo", "-X=bar=bar'" ]
...which is unfortunate.
Go does some quote handling, but:
GOFLAGS='-ldflags="-X=foo=foo -X=bar=bar"'
...still becomes:
[ "go", "build", "-ldflags=\"-X=foo=foo", "-X=bar=bar\"" ]
...which is surprising.
We can try:
GOFLAGS="-ldflags=-X=foo=foo -ldflags=-X=bar=bar"
...but go just takes the last value of that flag rather than the
accumulation.
What does work is:
GOFLAGS='"-ldflags=-X=foo=foo -X=bar=bar"'
...with the WHOLE flag quoted.
**Updated Original PR Description**
## Summary
This PR adds a small, controlled set of scheduling fields to WorkerPool
so WorkerPool-managed Pods can be placed onto appropriate Kubernetes
nodes.
Added fields:
- `nodeSelector`
- `tolerations`
- `priorityClassName`
- `nodeAffinity`
## Scope
This intentionally does not expose a full `PodTemplateSpec`, and does
not add support for full `affinity`, `podAffinity`, or
`podAntiAffinity`.
Resource requests/limits are intentionally left out of scope for this CL
while the WorkerPool resource model is still being discussed separately.
The goal is to keep the first version small while addressing the
WorkerPool-to-Kubernetes-Node scheduling gap discussed in
[#212](https://github.com/agent-substrate/substrate/issues/212).
`nodeAffinity` is included to support heterogeneous node pools where
`nodeSelector` is too limited, such as requiring or preferring one of
several equivalent accelerator/local-SSD/cache node pools, or expressing
soft preferences during node pool migration.
`priorityClassName` is included to help preserve warm WorkerPool
capacity under cluster pressure. Since WorkerPool pods are the slots
available for actor resume, higher-priority pools can protect
latency-sensitive or interactive actor workloads from displacement by
lower-priority batch workloads, and allow different WorkerPools to
represent different service classes, e.g. interactive vs. batch.
## Related issue
Part of #212.
Related to #47.
## Problem
`go test -v ./...` (the `run-tests` job) intermittently fails during
envtest **setup** — not in any test logic:
- `fork/exec .../kubebuilder-envtest/k8s/1.36.0-linux-amd64/etcd: text
file busy`
- `setup-envtest: ... unable to create file kubectl from archive for
version-platform pair 1.36.0-linux-amd64`
Three packages use envtest (`cmd/ateapi/internal/controlapi`,
`cmd/atecontroller/internal/controllers`, `pkg/api/v1alpha1`). `go test
./...` runs them as separate test binaries **in parallel**, and each
`TestMain` shells out to `setup-envtest use`. On a cold cache they race
to download, extract, and exec the **shared** kubebuilder binaries —
hence `text file busy` (exec'ing etcd while it's still being written)
and the archive-extraction error.
Seen on `main` (e.g. the #259 push run) and on PRs. It only reproduces
on a **cold** cache, which is why it's intermittent and doesn't show up
locally once the cache is warm.
## Fix
Add `internal/envtestbins.BinaryAssetsDir()`, which runs `setup-envtest
use` under a **cross-process file lock** (`flock` on a fixed file under
`os.TempDir()`, shared by the sibling test binaries). The first caller
populates the shared cache while the others wait, then they hit a warm
no-op path lookup. The duplicated setup in all three `TestMain`s now
calls this helper.
The lock scope (user/machine-global) matches the resource it guards —
the kubebuilder binary cache is itself user-global, shared even across
repo checkouts.
## Testing
Reproduced locally by moving the warm cache aside and running the three
packages concurrently: it fails without the lock and passes with it.
Full `go test ./...` green.
Test-only change; no production code touched.
Implements `PAUSED` state for issue #119.
The snapshot files are kept locally on node VM in a separate folder. At
resume time, scheduler uses a node VM hint and picks up a worker from
the same node where files were stored at suspend time.
The local file management solution is temporary and will be replaced
once @msau42 introduces a new component that is supposed to manage files
on the node VM.
- [X] Tests pass
- [X] Manual tests with counter demo
```
>kubectl ate create actor my-counter-1 --template ate-demo-counter/counter
>curl -X POST -H "Host: my-counter-1.actors.resources.substrate.ate.dev" http://localhost:8000
hello from: 169.254.17.2 | preserved memory count: 1
>curl -X POST -H "Host: my-counter-1.actors.resources.substrate.ate.dev" http://localhost:8000
hello from: 169.254.17.2 | preserved memory count: 2
>kubectl ate pause actor my-counter-1 -o json
> curl -X POST -H "Host: my-counter-1.actors.resources.substrate.ate.dev" http://localhost:8000
hello from: 169.254.17.2 | preserved memory count: 3
```
### Breaking change
This PR introduces a breaking change in the Actor proto. All existing
actor needs to be recreated, prior testing PAUSE functionality.
The one property unit tests cannot catch is identity captured in the
golden snapshot: the env-var approach passed unit tests and config.json
inspection yet returned the golden actor's ID after restore. The
identity suite restores TWO actors from one golden snapshot and asserts
each observes its own ID, and explicitly not the golden's, via the probe
actor's /whoami, reached through the atenet router the way real traffic
arrives.
- probe: minimal introspection actor for e2e suites; reports the
identity file at request time and surfaces read errors in the
response, so a failing assertion explains itself.
- RouterClient: port-forwards the atenet router service and routes on
resources.ActorDNSSuffix.
- deployProbe renders the manifest template in Go and execs the pinned
ko/kubectl directly with argument slices: no shell, nothing to quote.
KO_CONFIG_PATH must point at the repo root because ko resolves
.ko.yaml from its working directory; without it the build silently
loses defaultPlatforms and produces amd64-only images that cannot
start on arm64 nodes.
Gives an actor a reliable way to learn its own ID without parsing the
Host header. atelet populates a per-actor identity directory (currently
the single file actor-id) and bind-mounts it read-only at /run/ate into
each app container; the pause container gets none.
- A directory rather than a single-file mount, per the discussion on
#178, so future late-bound data can land as sibling files without
changing the mount shape.
- The bundle is regenerated on every resume and external bind mounts are
re-attached from the restore-time config.json, so the value stays
correct per-actor through a golden-snapshot restore; an env var (or a
file baked into the image) would be frozen at the golden actor's ID,
since it lives in the checkpointed process memory.
- actor-id is written atomically (temp file, fsync, rename, parent
fsync): the directory is visible to a possibly-running actor, so a
reader must never observe a torn value.
- The identity dir is cleared and recreated with the other per-actor
dirs in resetActorDirs.
- The in-rootfs mount point is created via os.Root, so a symlink planted
in the image cannot redirect the write outside the extracted rootfs.
Documented for actor authors in docs/api-guide.md: read it fresh on use;
raw ID, no trailing newline.
Include a verbatim copy of the k8s conventions into the skill. I tried
the skill locally and it generated a pretty decent summary of findings.
I hope this will help to catch violations to the KRM style guide as we
evolve our k8s APIs.
This follows the convention documented here:
AGENTS.md
```
**Where to put new Go code — quick rules:**
| Only used by one binary | `cmd/<binary>/internal/<pkg>` |
```
See the discussion here:
https://github.com/agent-substrate/substrate/issues/49
I just noticed that the directory structure is pretty deep. I want to
make this less deep before we get too far into the project:
* cmd/atenet/internal/app << this directory level is not needed. get rid of it.
* cmd/atenet/internal/router/* move everything but the cobra flags to internal/router
* cmd/atenet/internal/* just has the cobra flags definitions
This is moving files -- no logic change.
remote.Image was being called without remote.WithContext, so cancellation
of the workflow ctx (e.g. when ActorWorkflow.ResumeActor hits its 28s
deadline) did not stop the in-flight layer-blob HTTP requests. The
goroutines kept running until the TCP connection was torn down,
retaining partial-blob buffers in the meantime.
On a death loop of ResumeActor retries against a large image, this
amplifies atelet RSS proportionally to the loop duration — observed
peaking at ~9.4GB on a kind node where baseline atelet was ~2.4GB,
enough to OOM atelet on tight production workers.
Fixes#230.
What was happening:
1. runsc checkpoint pause succeeded.
2. After that, substrate tried to run runsc state and runsc delete for the app container.
3. But after checkpointing the root sandbox container, the runsc control server was no longer usable,
so those post-checkpoint commands failed with connection refused.
4. That made CheckpointWorkload return an error even though the actual checkpoint had already
succeeded.
5. The ActorTemplate stayed stuck in WaitGoldenActor.
The change removes the post-checkpoint state/delete calls from ateom-gvisor. After checkpoint
succeeds, it only cleans up the actor network and returns success. This matches the existing contract
in the code: atelet owns resetting the actor directories after checkpoint/upload, and it already
calls resetActorDirs(...).
Signed-off-by: Eitan Yarmush <eitan.yarmush@solo.io>
Fixes#202
atelet's gRPC server runs on an unauthenticated hostPort
(`manifests/ate-install/atelet.yaml`), so its
`Run`/`Restore`/`Checkpoint`
RPCs must treat every request field as untrusted. Several of those
fields
are turned into host filesystem paths and then deleted, created,
written,
or executed, none of them validated first. A caller that can reach the
port can smuggle a path separator or `..` through them and make atelet
operate outside the intended actor tree.
### Unvalidated inputs (each an independent escape route)
- **`actor_template_namespace` / `name` / `id`** are concatenated by
`ateompath.ActorPath` into the per-actor dir that `resetActorDirs` runs
`os.RemoveAll` / `os.MkdirAll` on.
- **`target_ateom_uid`** builds the netns path
(`/run/netns/ateom:<uid>`)
and the ateom control socket (`.../ateoms/<uid>/ateom.sock`).
- **`spec.containers[].name`** becomes an OCI bundle path component
(`os.RemoveAll` / `MkdirAll` / `config.json` write).
- **`runsc.sha256_hash`** builds `static-files/runsc-<hash>`. The
cache-hit
`os.Stat` early return hands that path to ateom to **execute** before
the
existing hex check runs, so an escaped path could execute an arbitrary
on-disk binary as the sandbox runtime.
### Where the validators live
The field rules are exported from **`internal/resources`**
(`ValidateActorRef`, `ValidateAteomUID`, `ValidateContainerNames`,
`ValidateRunscHash`, `ValidateSnapshotURIPrefix`), alongside the
existing
`ValidateActorID`. Every one of these values originates upstream in
user-supplied API/CRD data (the runsc hash and snapshot location come
from
the `ActorTemplate` spec), so the API server and controller can apply
the
same rules at admission time instead of failing at actor-activation
time.
`ValidateContainerNames` takes names rather than the atelet proto so
`resources` stays proto-free. Unifying `ValidateActorID` with
`ValidateActorRef` is deferred: it changes ate-api-server's
client-facing
error responses.
The per-RPC glue stays in `cmd/atelet`: each of
`Run`/`Checkpoint`/`Restore`
calls its own typed validator (`validateRunRequest` etc.) as the first
statement at the boundary, before any path is constructed.
### What is validated
- namespace and actor ID as DNS-1123 labels, template as a DNS-1123
subdomain, matching how these are named; additionally the combined
`<ns>:<tmpl>:<id>` path component is capped at 255 bytes, since
individually valid DNS names can join to 381 bytes, over common
filesystems' filename limit;
- `target_ateom_uid` as a DNS-1123 label (pod UIDs are UUIDs, which
qualify);
- each container name as a DNS-1123 label, additionally rejecting the
reserved name `pause` (collides with the sandbox-infra bundle and races
its concurrent writer) and duplicate names (same bundle path, mutual
corruption);
- `runsc.sha256_hash` as exactly 64 hex chars, checked in `fetchRunsc`
before the path is built and before the cache-hit early return;
- `snapshot_uri_prefix` (Checkpoint/Restore) as a parseable URI naming a
bucket, with query/fragment/userinfo rejected: object names are appended
by concatenation and the storage layer re-parses using only host and
path, so those components would silently redirect the upload/download to
a different object. The scheme is deliberately not restricted; that is
storage-backend policy.
Validation failures return `codes.InvalidArgument` with the reason.
Previously the interceptor surfaced them as `codes.Internal` with the
message masked as "internal server error".
DNS-1123 is a deliberate fit. These values already *are* Kubernetes
names,
so every legitimate value passes, while the charset forbids `/`, `.`
(for
labels), and `..`, which is exactly what enables traversal. For a
cooperating caller the change is behavior-neutral: inputs newly rejected
previously failed later or misbehaved silently.
### Testing
- Table tests for each validator in `internal/resources`, targeting the
exported functions (valid values, separators, `..`, empty, case, length,
the label-vs-subdomain distinction, the combined-length cap, the
`pause`/duplicate rules, and the URI component rejections).
- Per-request validator tests in `cmd/atelet` pin which rules each RPC
enforces, one table per validator.
- `TestRPCBoundariesReject` exercises `Run`/`Checkpoint`/`Restore`
directly
and asserts a traversal value is rejected with `InvalidArgument`, which
guards against future removal or reordering of the validation call.
- `TestFetchRunscRejectsBadHash` plants a real cache file at the path an
invalid hash resolves to and asserts `fetchRunsc` still errors. It
passes
with the current code and fails if validation is moved after the
`os.Stat` early return, so it proves the ordering rather than just that
validation runs.
- [x] Tests pass
- [x] No documentation changes needed
Fixes#211 : Create a demo that shows any worker can run any
actortemplates. Workers and actortemplates live in different namespaces.
- [x] Tests pass
- [x] Appropriate changes to documentation are included in the PR
Tested:
Deployed the demo and tested create, suspend, resume, delete actors in
both namepsaces. https://paste.googleplex.com/4999545124683776
A couple of TODOs/Discussions which I think are important to have based
on these changes.
1. Do we need a caching client for this to avoid calling the apiserver
every time?
2. Where/how do we want to resolve secrets generally? I think there are
good arguments to be made for the control-plane OR the "node" component.
In my mind this also related to:
https://github.com/agent-substrate/substrate/issues/18Fixes#15
> 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
It was using a wrong name `Actor Namespace` before, which doesn't exist
yet.
- [x] Tests pass
- [x] Appropriate changes to documentation are included in the PR
## Summary
Switch the shared atelet/ateom gVisor state directory from
`/run/ateom-gvisor` to `/var/lib/ateom-
gvisor`.
## Why
This path is not just a transient socket location. It is the shared
host/container state root for
downloaded `runsc` binaries, per-ateom sockets, OCI bundles, runsc
state, pidfiles, checkpoint state,
and restore state.
`/run` is normally volatile runtime storage and may be tmpfs-backed,
which makes it a poor fit for
large or service-owned local state. Moving this under `/var/lib` better
matches Linux filesystem
conventions and avoids treating gVisor runtime/checkpoint artifacts as
ephemeral runtime-only files.
Additionally I was running into many permissions issues with apps trying
to use various folders mounted under `/root` before this change.
## Changes
- Update `ateompath.BasePath` to `/var/lib/ateom-gvisor`.
- Update the WorkerPool controller to use `ateompath.BasePath` instead
of hard-coding the mount path.
- Update the atelet install manifest to mount the new hostPath.
## Impact
This keeps `atelet`, `ateom-gvisor`, and dynamically created WorkerPool
Deployments aligned on the
same shared state path. Without that alignment, the system can fail to
find sockets, bundles, runsc
state, or checkpoint/restore files.
This PR introduces a new OpenTelemetry histogram
`"atelet.snapshot.size"` from `atelet` over the existing
OTLP path. It records the uncompressed size in bytes of each gVisor
snapshot image written during checkpoint -- `checkpoint.img` ,
`pages.img` and `pages_meta.img` -- tagged by image `kind` and
`ActorTemplate`. This surfaces how large actor snapshots are and which
`ActorTemplate`s are expensive to checkpoint/restore.
Add a well-defined dashboard for snapshot metrics that contains 3
charts.
- snapshot image size p99 by `ActorTemplate`
- snapshot image size p50/p95/p99 (all kinds)
- snapshot QPS by `ActorTemplate`
<img width="996" height="717" alt="image"
src="https://github.com/user-attachments/assets/99bbab46-37dc-476e-bfd9-cdffeb174b4b"
/>
> 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
This PR introduces a new OpenTelemetry histogram
`"atenet.router.route.duration"` from `atenet-router` over the existing
OTLP path. It measures substrate overhead -- from envoy receiving a
request to envoy sending the request to the the resolved worker
endpoint, excluding actor compute and the response. This is the
`Substrate E2E latency` that under our control.
Add a well-defined dashboard for E2E metrics that contains 6 charts.
- Substrate routing latency — P50/P95/P99
- Substrate routing latency P99 - by stages
- Substrate routing latency P99 — by ActorTemplate
- Substrate routing QPS - by Status
- Substrate E2E (full round-trip) Latency — P50/P95/P99
- Substrate E2E (full round-trip) QPS — by Response Code
<img width="1601" height="894" alt="image"
src="https://github.com/user-attachments/assets/be6fc572-dfd8-4ef0-a0a9-a77f85f2ecf8"
/>
Fixes b/508613998
> It's a good idea to open an issue first for discussion.
- [ ] Tests pass
- [x] Appropriate changes to documentation are included in the PR
Adds a Prometheus server to the kind dev stack (in `otel-system`,
alongside the OpenTelemetry Collector and Jaeger). It scrapes component
`/metrics` via the existing `prometheus.io/scrape` annotations on
`ate-api-server` and `atelet`, scoped to the `ate-system`/`otel-system`
namespaces.
Fixes#106
- [x] Tests pass
- [x] Appropriate changes to documentation are included in the PR
Tommy McCormick