feat(engine): orchestrator external_ref on boxes (CRI uid analog)#747
Merged
Merged
Conversation
…r's name-slot workaround Mirror CRI's PodSandboxMetadata.uid: an orchestrator can attach an opaque external_ref to a box at create time. The engine never interprets it, rejects duplicates at create, and resolves lookups by id, name, external_ref, then id prefix. The runner now attaches the control-plane box id as external_ref instead of squatting the name slot, freeing name for a future user-facing box name. - engine: BoxOptions.external_ref (serde default, json-persisted; no DB migration — lookup_box already scans configs), create-time uniqueness check, BoxInfo exposes it (omitted when unset). - C SDK: boxlite_options_set_external_ref + CBoxInfo.external_ref (freed on drop; leak-count test updated 4 -> 5 strings), header regenerated. - Go SDK: WithExternalRef option, BoxInfo.ExternalRef. - Node SDK: field defaulted to None (orchestrator-facing knob, not exposed in JS options yet). - CLI: inspect shows ExternalRef only when present; list unchanged. - runner: WithName(dto.Id) -> WithExternalRef(dto.Id); recovery via runtime.Get(dto.Id) now resolves through the external_ref match. Tests: engine lib 735/735 (new lookup_box_by_external_ref test verified two-sided: fails without the lookup clause, passes with it); boxlite-c 55/55; CLI/node/python compile; go vet clean on sdks/go and runner pkg (final cgo link needs prebuilt libboxlite.a, absent here).
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
law-chain-hot
added a commit
that referenced
this pull request
Jun 12, 2026
## Summary Stacked on #744. Implements the identity follow-up discussed on #735 (see the "Identity model note" comment there): the engine gains a CRI-style orchestrator reference, and the runner stops squatting the engine's `name` slot to carry the control-plane box id. ```text before after ────── ───── runner: WithName(cloudId) runner: WithExternalRef(cloudId) engine: name = cloudId (squatted) engine: external_ref = cloudId name unavailable for users name free for a future user-facing box name ``` Industry precedent (verified sources): - CRI `PodSandboxMetadata.uid` — the orchestrator's id travels as a dedicated metadata field; the runtime mints its own id ([cri-api api.proto](https://github.com/kubernetes/cri-api/blob/master/pkg/apis/runtime/v1/api.proto), `message PodSandboxMetadata`) - ECS agent attaches `com.amazonaws.ecs.task-arn` as a docker label ([docker_task_engine.go](https://github.com/aws/amazon-ecs-agent/blob/master/agent/engine/docker_task_engine.go)) - The caller-supplied-id pattern (OCI runtime spec `create <container-id>`) applies to stateless executors (runc/firecracker), not to stateful daemons like this engine — hence a reference field, not id injection. ## What changed - **engine**: `BoxOptions.external_ref` (opaque, json-persisted via the existing config blob — no DB migration; `lookup_box` already scans configs). Duplicate refs are rejected at create. Lookup order: exact id → name → external_ref → id prefix. `BoxInfo` exposes it, omitted when unset. - **C SDK**: `boxlite_options_set_external_ref`, `CBoxInfo.external_ref` (NULL when unset, freed on drop — the leak-count test now expects 5 inner CStrings), header regenerated by the crate build. - **Go SDK**: `WithExternalRef`, `BoxInfo.ExternalRef`. - **Node SDK**: field filled with `None` (orchestrator-facing knob; not exposed in JS options yet). - **CLI**: `inspect` shows `ExternalRef` only when present; `list` columns unchanged (nothing empty is rendered). - **runner**: `WithName(dto.Id)` → `WithExternalRef(dto.Id)`; restart recovery (`runtime.Get(dto.Id)`) resolves through the external_ref match. ## Verification - engine lib tests: 735/735; the new `lookup_box_by_external_ref` test was verified two-sided (fails with the lookup clause reverted, passes restored) - boxlite-c: 55/55 - CLI / Node / Python crates compile; `go vet` clean on `sdks/go` and `apps/runner/pkg/boxlite` (final cgo link needs the prebuilt `libboxlite.a`, absent in this worktree) ## Merge order #743 → #735 → #744 → this PR. Self-contained: reverting is closing this PR.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Stacked on #744. Implements the identity follow-up discussed on #735 (see the "Identity model note" comment there): the engine gains a CRI-style orchestrator reference, and the runner stops squatting the engine's
nameslot to carry the control-plane box id.Industry precedent (verified sources):
PodSandboxMetadata.uid— the orchestrator's id travels as a dedicated metadata field; the runtime mints its own id (cri-api api.proto,message PodSandboxMetadata)com.amazonaws.ecs.task-arnas a docker label (docker_task_engine.go)create <container-id>) applies to stateless executors (runc/firecracker), not to stateful daemons like this engine — hence a reference field, not id injection.What changed
BoxOptions.external_ref(opaque, json-persisted via the existing config blob — no DB migration;lookup_boxalready scans configs). Duplicate refs are rejected at create. Lookup order: exact id → name → external_ref → id prefix.BoxInfoexposes it, omitted when unset.boxlite_options_set_external_ref,CBoxInfo.external_ref(NULL when unset, freed on drop — the leak-count test now expects 5 inner CStrings), header regenerated by the crate build.WithExternalRef,BoxInfo.ExternalRef.None(orchestrator-facing knob; not exposed in JS options yet).inspectshowsExternalRefonly when present;listcolumns unchanged (nothing empty is rendered).WithName(dto.Id)→WithExternalRef(dto.Id); restart recovery (runtime.Get(dto.Id)) resolves through the external_ref match.Verification
lookup_box_by_external_reftest was verified two-sided (fails with the lookup clause reverted, passes restored)go vetclean onsdks/goandapps/runner/pkg/boxlite(final cgo link needs the prebuiltlibboxlite.a, absent in this worktree)Merge order
#743 → #735 → #744 → this PR. Self-contained: reverting is closing this PR.