Conversation
Signed-off-by: YaoZengzeng <[email protected]>
volcano-sh-bot
added
kind/documentation
|
[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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
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.
| - Go 1.24.4 | ||
| - Kubernetes v1.24+ (cluster prerequisite) | ||
| - `k8s.io/api` + `apimachinery` + `client-go` v0.34.1 | ||
| - `sigs.k8s.io/controller-runtime` v0.22.2 | ||
| - `sigs.k8s.io/agent-sandbox` v0.1.1 (must be installed before AgentCube) | ||
| - `gin-gonic/gin` v1.10.0 | ||
| - `golang-jwt/jwt/v5` v5.2.2 | ||
| - `redis/go-redis/v9` v9.17.1 / `valkey-io/valkey-go` v1.0.69 |
There was a problem hiding this comment.
The dependency versions listed in this section appear to be incorrect. For example:
Go 1.24.4is not a valid Go version.k8s.io/api+apimachinery+client-gov0.34.1: This version seems incorrect, and it's better to list these dependencies on separate lines.sigs.k8s.io/controller-runtimev0.22.2seems incorrect.redis/go-redis/v9v9.17.1andvalkey-io/valkey-gov1.0.69also seem to be incorrect versions.
Please verify and update all dependency versions to reflect the actual versions used in this release. Accurate dependency information is critical for users.
| Related: | ||
|
|
||
| - Design Doc: [Router Proposal](../design/router-proposal.md) | ||
| - Contributors: [@volcano-sh](https://github.com/volcano-sh) |
There was a problem hiding this comment.
This Contributors section under each feature seems to have a placeholder value [@volcano-sh]. It's repeated for multiple features. It would be more informative to either list the actual contributors for each feature or remove this line if it's not applicable. A general contributors list is already present at the end of the document.
|
|
||
| **Background and Motivation:** | ||
|
|
||
| Agent sessions that complete their work or are abandoned by clients must be automatically reclaimed to avoid resource exhaustion. AgentCube implements a dual garbage collection policy enforced by background loops in both the Workload Manager and AgentD: |
There was a problem hiding this comment.
The component AgentD is mentioned here for the first time. It's not introduced in the summary or other sections, which might confuse users. Please consider adding a brief explanation of what AgentD is and its role in the AgentCube architecture in the summary or a dedicated section.
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.
| ``` | ||
| POST /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 for the invocations routes (GET is needed for things like file downloads). This section lists only POST; consider documenting both methods so users don't assume GET is unsupported.
| - **pcap-analyzer example**: `example/pcap-analyzer/` — end-to-end example agent that analyzes packet captures using code interpreter | ||
| - **Redis and ValKey backends**: pluggable session store (`pkg/store`) with implementations for both Redis and ValKey; selected via configuration | ||
| - **Prometheus metrics**: metrics exported by Router and Workload Manager for operational observability | ||
| - **Health probes**: `/healthz/live` and `/healthz/ready` on the Router for Kubernetes liveness/readiness checks |
There was a problem hiding this comment.
Router health endpoints appear to be /health/live and /health/ready (no z). The release note currently documents /healthz/live and /healthz/ready, which will 404 against the Router as implemented.
| - **Health probes**: `/healthz/live` and `/healthz/ready` on the Router for Kubernetes liveness/readiness checks | |
| - **Health probes**: `/health/live` and `/health/ready` on the Router for Kubernetes liveness/readiness checks |
| ```bash | ||
| helm install agentcube manifests/charts/base \ | ||
| --namespace agentcube-system --create-namespace \ | ||
| --set store.address=<redis-or-valkey-host>:6379 |
There was a problem hiding this comment.
The Helm values key in this install command doesn't match the chart: manifests/charts/base/values.yaml uses redis.addr (and redis.password), not store.address. As written, --set store.address=... will have no effect and the Router/Workload Manager will start without a Redis endpoint.
| --set store.address=<redis-or-valkey-host>:6379 | |
| --set redis.addr=<redis-or-valkey-host>:6379 |
| ### Dependencies | ||
|
|
||
| - Go 1.24.4 | ||
| - Kubernetes v1.24+ (cluster prerequisite) |
There was a problem hiding this comment.
The stated Kubernetes prerequisite (v1.24+) looks inconsistent with the dependency versions listed right below (e.g., k8s.io/api/apimachinery/client-go v0.34.1, which aligns with Kubernetes 1.34.x). Please verify the minimum supported cluster version for v0.1.0 and update this line accordingly to avoid misleading users.
| - Kubernetes v1.24+ (cluster prerequisite) | |
| - Kubernetes v1.34+ (cluster prerequisite) |
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #266 +/- ##
==========================================
+ Coverage 35.60% 43.32% +7.71%
==========================================
Files 29 30 +1
Lines 2533 2613 +80
==========================================
+ Hits 902 1132 +230
+ Misses 1505 1358 -147
+ 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:
|
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?: