Skip to content

Commit 7553814

Browse files
EItanyaCopilot
EItanya
and
Copilot
committed
design: expected final ateapi.proto for decoupled substrate resources
Draft the target ateapi surface for issue #368 (ActorTemplate, WorkerPool, SandboxConfig, WorkerPoolGrant moved out of Kubernetes CRDs), following the draft API style guide (#351), plus design rationale and CRD migration notes. For review only: codegen and server implementation intentionally lag the proto. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by-:
1 parent 35cc760 commit 7553814

3 files changed

Lines changed: 936 additions & 159 deletions

File tree

‎docs/design/ateapi-crd-migration.md‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
# Migrating substrate-plane resources from CRDs to ateapi (issue #368)
2+
3+
Companion to [ateapi-resource-api.md](ateapi-resource-api.md), which defines
4+
the target API. This doc covers the transition: what changes relative to the
5+
Kubernetes CRDs and the POC proto (`poc-decouple-upstream`), and the
6+
migration path.
7+
8+
## Why the POC shape changes
9+
10+
The POC copied the CRD Go types field-for-field to move fast. That imported
11+
Kubernetes idioms that don't pull their weight in a native API:
12+
13+
| # | Leak | Where | Problem |
14+
|---|------|-------|---------|
15+
| 1 | `deployment_atespace` | WorkerPoolSpec | It's a **Kubernetes namespace** (the projector uses it verbatim as the Deployment namespace), mislabeled with a substrate term. Renamed `kubernetes.namespace`. |
16+
| 2 | `WorkerPoolPodTemplate` + Toleration/NodeAffinity/ResourceRequirements at top level | WorkerPool | corev1 mirrored into the public API without a boundary marking it k8s-specific. Quarantined into `KubernetesPlacement`. |
17+
| 3 | `EnvVarSource`/`SecretKeySelector` | Container env | corev1's three-field env shape; invalid states representable (value + value_from). Now a `oneof`. |
18+
| 4 | `LabelSelector.match_expressions` | template worker_selector | Set-based operators copied but only equality is implemented or meaningful today. Dropped. |
19+
| 5 | `Condition` list + string `phase` | ActorTemplateStatus | KRM condition machinery; `observed_generation` has no generation to observe. Replaced by a state enum. |
20+
| 6 | Stringly-typed enums (`on_pause: "Full"`) | SnapshotsConfig | CEL-validated strings instead of proto enums. Now `Scope` enum. |
21+
| 7 | Update RPCs with unspecified FieldMask semantics | templates, pools, configs | The POC marked mask semantics TODO. Now defined (top-level paths), and templates lose Update entirely. |
22+
| 8 | Grant identity `(atespace, name)` with free-form name | WorkerPoolGrant | The scheduler looks up grants by `(atespace, pool)`; the POC allows multiple grants for the same pair, making revocation ambiguous. The POC's own `GetWorkerPoolGrant` is documented "by atespace and worker pool" but keyed by name. Now: Create enforces at most one grant per (atespace, pool). |
23+
24+
## Behavior deltas vs the POC
25+
26+
1. **No UpdateActorTemplate** — POC has it; the target API drops it
27+
(immutable spec).
28+
2. **Grant uniqueness** — Create rejects a second grant for the same
29+
(atespace, worker_pool) pair with `ALREADY_EXISTS`; the store gains a
30+
`(atespace, pool)` index so the scheduler check stays a point lookup.
31+
3. **Delete returns the resource** — POC returned `Empty` for
32+
templates/pools/configs/grants and an empty message for actors.
33+
4. **Required update_mask** — Update without a mask becomes
34+
`INVALID_ARGUMENT`; POC treated masks as TODO/full-replace.
35+
5. **Actor RPC shapes** — bare-resource returns, `ObjectRef` identity, and
36+
the legacy `actor_template_namespace/name` create path is gone. The
37+
package rename (`ateapi` → `ateapi.v1alpha1`) makes every message a new
38+
proto type, so this is a clean break: field numbers are assigned fresh,
39+
no `reserved` scar tissue, and migration is a coordinated client cutover
40+
rather than in-place field evolution. (The POC has no external users.)
41+
6. **Referential integrity on delete** — template↔actor, pool↔grant,
42+
pool↔assigned-actor, config↔pool checks return `FAILED_PRECONDITION`;
43+
the POC checks only atespace emptiness.
44+
7. **Watch SYNCED event** — new; consumers can await cache completeness.
45+
46+
## CRD validations that move into handlers
47+
48+
Validation currently expressed as CEL/OpenAPI on the CRDs moves into
49+
Create (and Update, where applicable) handlers:
50+
51+
- DNS-1123 label names (atespace, name — per style guide §2.3)
52+
- template spec immutability (`self == oldSelf` → no Update method)
53+
- container images pinned by digest
54+
- volume mount-path rules
55+
- `snapshots_config.on_commit` ⊆ `on_pause`
56+
- at most one `default: true` SandboxConfig
57+
58+
## Migration path
59+
60+
1. Land the target proto as `ateapi.v1alpha1` alongside the POC `ateapi`
61+
package; generate both.
62+
2. Port store keys/values: add the grant `(atespace, pool)` uniqueness
63+
index; template records gain immutability enforcement.
64+
3. Move CRD validation into handlers (table above); delete the CRDs and the
65+
projector's CRD-watching path.
66+
4. Cut clients (atectl, atelet caches, demos) over to `v1alpha1`; delete
67+
the POC package.

‎docs/design/ateapi-resource-api.md‎

Lines changed: 140 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,140 @@
1+
# ateapi resource API (issue #368)
2+
3+
Proto: [`pkg/proto/ateapipb/ateapi.proto`](../../pkg/proto/ateapipb/ateapi.proto)
4+
5+
This is the proposed API surface once ActorTemplate, WorkerPool,
6+
SandboxConfig and WorkerPoolGrant move out of Kubernetes CRDs into the
7+
substrate API ([#368](https://github.com/agent-substrate/substrate/issues/368)).
8+
It builds on [decoupling-planes.md](https://github.com/agent-substrate/substrate/blob/poc-decouple-upstream/docs/design/decoupling-planes.md)
9+
(on the POC branch), which settled the
10+
resource model (global pools + per-atespace grants); this doc covers the API
11+
shape. The transition from the CRDs/POC is covered separately in
12+
[ateapi-crd-migration.md](ateapi-crd-migration.md).
13+
14+
The API follows the draft
15+
[API style guide](https://github.com/agent-substrate/substrate/pull/351);
16+
deliberate divergences are listed at the [end](#style-guide-conformance).
17+
18+
## Resource model
19+
20+
| Resource | Scope | Service | Mutable? | Notes |
21+
|---|---|---|---|---|
22+
| Atespace | global | Control | no | isolation boundary |
23+
| ActorTemplate | atespace | Control | no (immutable spec) | + Watch |
24+
| Actor | atespace | Control | `worker_selector` only | + Suspend/Pause/Resume |
25+
| WorkerPool | global | Admin | yes | + Watch; labels are the selector match target |
26+
| SandboxConfig | global | Admin | yes | de-facto registry of sandbox classes |
27+
| WorkerPoolGrant | atespace | Admin | no | at most one grant per (atespace, pool) |
28+
| Worker | — | Admin | — | debug projection, ListWorkers only |
29+
30+
All managed resources carry `ResourceMetadata meta = 1` (atespace, name,
31+
uid, version, create_time, update_time, labels) and use the standard method
32+
shapes from the style guide: Get/Create/Update return the bare resource,
33+
Delete returns the deleted resource, List paginates with
34+
`page_size`/`page_token`, identity travels as `ObjectRef{atespace, name}`,
35+
and optimistic concurrency uses `meta.version` (int64, `ABORTED` on
36+
mismatch, 0 skips).
37+
38+
## Design decisions
39+
40+
**D1. Versioned package `ateapi.v1alpha1`.** gRPC method paths embed the
41+
package, so the package name is the API-versioning mechanism.
42+
43+
**D2. Two services with distinct audiences.** `Control` is the tenant
44+
surface (atespaces, templates, actors); `Admin` is the platform surface
45+
(pools, sandbox configs, grants, debug). Watch RPCs live next to their
46+
resource. The split gives a future authz story a natural boundary.
47+
48+
**D3. ActorTemplate spec is immutable — no UpdateActorTemplate.** Golden
49+
snapshots are only valid for the exact spec they were taken from, so
50+
in-place mutation is meaningless. Template changes = create a new template
51+
and migrate actors. Delete returns `FAILED_PRECONDITION` while any actor
52+
references the template.
53+
54+
**D4. At most one WorkerPoolGrant per (atespace, worker pool).** Grants are
55+
fully conventional resources (caller-named, standard methods), but Create
56+
enforces uniqueness on the pair — a duplicate grant for the same pool
57+
returns `ALREADY_EXISTS` regardless of its name (same pattern as the
58+
single-default SandboxConfig rule). This keeps revocation unambiguous and
59+
lets the scheduler's `(atespace, pool)` check stay a point lookup via a
60+
server-side index. Grants are immutable; future policy fields (quotas)
61+
would add Update.
62+
63+
**D5. One Kubernetes pocket: `WorkerPoolSpec.kubernetes`.** Everything
64+
k8s-specific about materializing a pool — namespace, node_selector,
65+
tolerations, node_affinity, resources, priority_class_name — lives in a
66+
single `KubernetesPlacement` message. Everything outside it is
67+
plane-neutral; a future non-k8s substrate adds a sibling placement message.
68+
69+
**D6. Referential integrity on delete.** Deletes that would orphan
70+
dependents return `FAILED_PRECONDITION`: atespace↔actors/templates,
71+
template↔actors, pool↔grants and pool↔assigned actors, config↔pools.
72+
73+
**D7. Status is a closed enum, not phase strings or conditions.**
74+
`ActorTemplateStatus.state` ∈ {PENDING, SNAPSHOTTING, READY, FAILED} plus
75+
`golden_actor_id`, `golden_snapshot`, `error_message`. Likewise
76+
`SnapshotsConfig` scopes are a `Scope` enum (FULL/DATA).
77+
78+
**D8. Equality-only `Selector{match_labels}`** for template and per-actor
79+
worker selectors, matched against `WorkerPool.meta.labels`. The scheduler
80+
implements equality matching only; set-based expressions can be added as a
81+
new field if a consumer appears.
82+
83+
**D9. Env vars are a `oneof {value | secret}`** with a plane-neutral
84+
`SecretRef{name, key, optional}` — value-vs-secret is exclusive by
85+
construction, and no k8s types leak into the container spec.
86+
87+
**D10. `sandbox_class` is an open string** on templates and pools.
88+
SandboxConfigs are the class registry; discoverability comes from
89+
ListSandboxConfigs, not a proto enum.
90+
91+
**D11. Update masks accept top-level paths only.** `update_mask` is
92+
required (per the style guide); paths one level below the resource root
93+
(`spec.replicas`, `spec.kubernetes`, `meta.labels`) are accepted, and
94+
naming a message or map field replaces it wholesale. Unknown, immutable, or
95+
deeper paths → `INVALID_ARGUMENT`. Servers may widen to deeper paths later
96+
without breaking clients.
97+
98+
**D12. Watch = snapshot, SYNCED, then incremental events.** Streams
99+
(templates, pools) deliver an initial snapshot, a `SYNCED` marker, then
100+
at-least-once CREATED/UPDATED/DELETED events carrying full resource state.
101+
No resume tokens: consumers are in-cluster caches and re-syncing is cheap;
102+
`meta.version` lets them discard stale events.
103+
104+
**D13. Worker stays a debug projection.** No `ResourceMetadata`, no CRUD.
105+
Making Worker a managed resource would promise lifecycle semantics the
106+
system doesn't offer — workers are pool-managed.
107+
108+
**D14. Actor is a managed resource like the rest.** `meta` carries its
109+
identity; `actor_template` and `worker_pool` are `ObjectRef`s; lifecycle
110+
verbs (Suspend/Pause/Resume) are custom methods returning the Actor. The
111+
`ateom_pod_*` fields remain, marked output-only infrastructure debugging.
112+
113+
**D15. Atespace and SessionIdentity are unchanged** beyond conforming to
114+
`meta` and standard method shapes. Redesigning them is a non-goal of #368.
115+
116+
## Style-guide conformance
117+
118+
The surface follows PR #351 (identity model, `ResourceMetadata`,
119+
`ObjectRef`, standard method shapes, required `update_mask`, `version`
120+
concurrency). Four deliberate divergences, feedback filed on PR #351:
121+
122+
1. **`labels` in `ResourceMetadata`** — the guide's meta has no labels
123+
field, but scheduling requires pool labels as a selector match target.
124+
2. **Immutable resources may omit Update** (D3, D4) — the guide implies all
125+
five standard methods everywhere.
126+
3. **Top-level-only mask paths** (D11) — the guide requires the mask but
127+
leaves path granularity undefined.
128+
4. **Empty `atespace` = list across all atespaces** — the guide reads as if
129+
`atespace` were required on List; admin/debug flows need cross-atespace
130+
listing.
131+
132+
## Open questions
133+
134+
- **Secrets plane**: `SecretRef` resolves against the worker's environment
135+
(a k8s Secret today). Freeze that rule, or design secret distribution
136+
before v1beta1?
137+
- **Pagination defaults**: pick a server default page size (proposal: 500,
138+
max 1000) and document it once, centrally.
139+
- **Watch scope**: watches are global today (atelet caches everything).
140+
Add an optional `atespace` filter now, or when a consumer needs it?

0 commit comments

Comments
 (0)