Conversation
Signed-off-by: YaoZengzeng <yaozengzeng@huawei.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
There was a problem hiding this comment.
Code Review
This pull request introduces the initial release notes for AgentCube v0.1.0, detailing the project's architecture, core features like session-based microVM routing and the PicoD runtime daemon, and its integration with Kubernetes via new CRDs. Review feedback identifies several issues in the documentation that require correction: multiple dependency versions (including Go and Kubernetes components) appear to be incorrect or non-existent, placeholder contributor tags are used repeatedly across feature sections, and the 'AgentD' component is mentioned without being previously defined or explained.
There was a problem hiding this comment.
Pull request overview
Adds an initial draft of the AgentCube v0.1.0 release notes to support the v0.1.0 release checklist in #263.
Changes:
- Introduces
docs/release-notes/v0.1.0.mdwith a v0.1.0 summary, feature highlights, API overview, dependencies, and install/usage snippets. - Documents Router/session flow, CRDs (
AgentRuntime,CodeInterpreter), PicoD, warm pools, and JWT auth chain. - Includes contributor acknowledgements and a placeholder “Full Changelog” section.
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #266 +/- ##
==========================================
+ Coverage 35.60% 43.37% +7.76%
==========================================
Files 29 30 +1
Lines 2533 2610 +77
==========================================
+ Hits 902 1132 +230
+ Misses 1505 1355 -150
+ Partials 126 123 -3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Signed-off-by: YaoZengzeng <yaozengzeng@huawei.com>
|
@hzxuzhonghu ptal |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Yao Zengzeng <yaozengzeng@huawei.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Yao Zengzeng <yaozengzeng@huawei.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Yao Zengzeng <yaozengzeng@huawei.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Yao Zengzeng <yaozengzeng@huawei.com>
| - **File upload / write** (`POST /api/files`): supports multipart form-data and JSON/base64 content for workspace-scoped file creation and updates | ||
| - **File download / read** (`GET /api/files/*path`): streams files from the sandbox workspace using path-addressed operations |
There was a problem hiding this comment.
The PicoD endpoint paths documented here don't match the implementation. PicoD serves authenticated APIs under the /api prefix (e.g., POST /api/execute, POST /api/files, GET /api/files, GET /api/files/*path), not /execute or /files/upload//files/download. Updating these paths (and aligning the described operations with the actual handlers) will prevent readers from calling non-existent endpoints.
| - **File upload / write** (`POST /api/files`): supports multipart form-data and JSON/base64 content for workspace-scoped file creation and updates | |
| - **File download / read** (`GET /api/files/*path`): streams files from the sandbox workspace using path-addressed operations | |
| - **File create / write** (`POST /api/files`): supports multipart form-data and JSON/base64 content for workspace-scoped file creation and updates | |
| - **File listing** (`GET /api/files`): returns workspace-scoped file and directory listings | |
| - **File read / download** (`GET /api/files/*path`): streams files from the sandbox workspace using path-addressed operations |
| ``` | ||
| POST /v1/namespaces/{namespace}/agent-runtimes/{name}/invocations/*path | ||
| GET /v1/namespaces/{namespace}/agent-runtimes/{name}/invocations/*path | ||
| POST /v1/namespaces/{namespace}/code-interpreters/{name}/invocations/*path |
There was a problem hiding this comment.
The Router supports both GET and POST on the .../invocations/*path routes (GET is used for file downloads), but this section lists only POST. Consider documenting both methods (or noting that GET is supported) to avoid confusion when users try to download files through the Router.
| Sandbox pods are ephemeral and may be replaced at any time; embedding a shared secret in cluster config is fragile and hard to rotate. AgentCube establishes an RSA-based trust chain: the Router generates an RSA-2048 key pair at startup, stores the public key in a Kubernetes Secret (`picod-router-identity`), and the Workload Manager injects it as `PICOD_AUTH_PUBLIC_KEY` into every sandbox pod. The Router signs short-lived (5-minute) RS256 JWTs for every proxied request. PicoD verifies these tokens entirely in-process — no network round-trip, no shared database. | ||
|
|
||
| Key Capabilities: | ||
|
|
||
| - RSA-2048 key pair auto-generated at Router startup | ||
| - Public key distributed via `picod-router-identity` Kubernetes Secret | ||
| - Workload Manager injects public key into sandbox env at pod creation time |
There was a problem hiding this comment.
This description implies the Workload Manager injects PICOD_AUTH_PUBLIC_KEY into every sandbox pod. In the current implementation, the public key is injected for CodeInterpreter sandboxes only when spec.authMode is not none (default picod); AgentRuntime sandboxes are built from the provided PodSpec and are not automatically patched with this env var. Please tighten the wording to reflect the conditional injection behavior.
| Sandbox pods are ephemeral and may be replaced at any time; embedding a shared secret in cluster config is fragile and hard to rotate. AgentCube establishes an RSA-based trust chain: the Router generates an RSA-2048 key pair at startup, stores the public key in a Kubernetes Secret (`picod-router-identity`), and the Workload Manager injects it as `PICOD_AUTH_PUBLIC_KEY` into every sandbox pod. The Router signs short-lived (5-minute) RS256 JWTs for every proxied request. PicoD verifies these tokens entirely in-process — no network round-trip, no shared database. | |
| Key Capabilities: | |
| - RSA-2048 key pair auto-generated at Router startup | |
| - Public key distributed via `picod-router-identity` Kubernetes Secret | |
| - Workload Manager injects public key into sandbox env at pod creation time | |
| Sandbox pods are ephemeral and may be replaced at any time; embedding a shared secret in cluster config is fragile and hard to rotate. AgentCube establishes an RSA-based trust chain: the Router generates an RSA-2048 key pair at startup, stores the public key in a Kubernetes Secret (`picod-router-identity`), and the Workload Manager injects it as `PICOD_AUTH_PUBLIC_KEY` for `CodeInterpreter` sandboxes when authentication is enabled (the default is `picod`; `none` disables injection). The Router signs short-lived (5-minute) RS256 JWTs for every proxied request. PicoD verifies these tokens entirely in-process — no network round-trip, no shared database. | |
| Key Capabilities: | |
| - RSA-2048 key pair auto-generated at Router startup | |
| - Public key distributed via `picod-router-identity` Kubernetes Secret | |
| - Workload Manager injects public key into `CodeInterpreter` sandbox env when `spec.authMode` is not `none` |
| **Background and Motivation:** | ||
|
|
||
| Creating a microVM sandbox from scratch on every session request incurs a cold-start penalty that is unacceptable for interactive workloads. AgentCube introduces a warm pool mechanism: the Workload Manager pre-creates a configurable number of idle `Sandbox` pods and keeps them ready. When an invocation arrives, the Router claims a pre-warmed pod via a `SandboxClaim` CR instead of waiting for a new pod to start. The pool is automatically replenished after each claim. | ||
|
|
||
| Key Capabilities: | ||
|
|
||
| - `spec.warmPoolSize` on `CodeInterpreter` controls pool depth |
There was a problem hiding this comment.
This section says the Router claims a pre-warmed pod via a SandboxClaim CR. In the current flow, the Router requests sandbox allocation from the Workload Manager, and the Workload Manager decides whether to create a SandboxClaim (warm pool) or a Sandbox (cold start). Updating the wording to attribute the SandboxClaim usage to the Workload Manager will better match the actual architecture.
| namespace: default | ||
| spec: | ||
| template: | ||
| image: python:3.12-slim |
There was a problem hiding this comment.
PicoD is not injected so /api/execute will not work
| helm install agentcube manifests/charts/base \ | ||
| --namespace agentcube-system --create-namespace \ | ||
| --set redis.addr=<redis-or-valkey-host>:6379 |
hzxuzhonghu
left a comment
There was a problem hiding this comment.
Thanks for putting together the v0.1.0 release notes! I did a quick check of the technical claims against the codebase — most are accurate, but I spotted one issue in the install example.
Bug: command field must be an array, not a string
The "Invoke the interpreter" snippet in the Upgrade Instructions section sends command as a single string:
-d '{"command": "python3 -c \"print(1+1)\""}'However, ExecuteRequest.Command is defined as []string in pkg/picod/execute.go:
Command []string `json:"command" binding:"required"` // The command and its arguments to execute. The first element is the executable.Sending a JSON string will cause a 400 binding error. The correct form is:
curl -X POST \
http://<router-host>/v1/namespaces/default/code-interpreters/my-interpreter/invocations/api/execute \
-H "Content-Type: application/json" \
-d '{"command": ["python3", "-c", "print(1+1)"]}'Everything else I checked matches the code:
- Helm value
redis.addr✓ - JWT expiry 5 minutes (
jwtExpiration = 5 * time.Minuteinpkg/router/jwt.go) ✓ - Secret name
picod-router-identity✓ sigs.k8s.io/agent-sandbox v0.1.1prerequisite ✓- 32 MB body limit ✓
Please fix the curl example before merging.
hzxuzhonghu
left a comment
There was a problem hiding this comment.
Reading through this note, i feel this is more like a proposal, especially the L55 section
What type of PR is this?
/kind documentation
What this PR does / why we need it:
Which issue(s) this PR fixes:
Fixes part of #263
Special notes for your reviewer:
Does this PR introduce a user-facing change?: