Upgrade agent sandbox v0.5.2#442
Conversation
|
[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 upgrades the agent-sandbox dependency from v0.4.6 to v0.5.2, migrating the codebase from the v1alpha1 to the v1beta1 API. This includes updating controllers, helpers, and tests to align with the new API structures, updating documentation and scripts to use the consolidated sandbox-with-extensions.yaml manifest, and adding a proposal for Sandbox Resource Pool Management. The review feedback points out critical compilation errors in codeinterpreter_controller.go and workload_builder_test.go where PodTemplate is accessed directly instead of through SandboxBlueprint, a potential runtime panic in codeinterpreter_controller.go due to an unsafe pointer dereference, and a CLI flag syntax error in the getting-started documentation.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #442 +/- ##
==========================================
+ Coverage 58.41% 59.94% +1.53%
==========================================
Files 36 36
Lines 3463 3593 +130
==========================================
+ Hits 2023 2154 +131
+ Misses 1231 1219 -12
- Partials 209 220 +11
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
3dc6e0f to
c772a36
Compare
There was a problem hiding this comment.
Pull request overview
Upgrades AgentCube’s integration with kubernetes-sigs/agent-sandbox to v0.5.2, migrating internal references from the v1alpha1 API to v1beta1 across the workload-manager and E2E coverage, and refreshing docs/manifests to use the consolidated sandbox-with-extensions.yaml.
Changes:
- Migrate agent-sandbox API usage from
v1alpha1tov1beta1across controllers/handlers, tests, and E2E validation. - Update installation/E2E workflows to apply the single
sandbox-with-extensions.yamlmanifest. - Bump Kubernetes/controller-runtime and related Go dependencies; add a new sandbox pool management design proposal doc.
Reviewed changes
Copilot reviewed 15 out of 16 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| test/e2e/run_e2e.sh | Switch E2E install to sandbox-with-extensions.yaml. |
| test/e2e/e2e_test.go | Update E2E tests to use agent-sandbox v1beta1 types. |
| pkg/workloadmanager/workload_builder.go | Build Sandbox/SandboxClaim objects using v1beta1 schemas/fields. |
| pkg/workloadmanager/workload_builder_test.go | Adjust tests for SandboxClaim spec field rename (TemplateRef → WarmPoolRef). |
| pkg/workloadmanager/sandbox_helper.go | Update helpers to accept v1beta1 Sandbox types/conditions. |
| pkg/workloadmanager/sandbox_helper_test.go | Update helper tests to v1beta1 types/condition constants. |
| pkg/workloadmanager/sandbox_controller.go | Update controller watch/reconcile type to v1beta1 Sandbox. |
| pkg/workloadmanager/k8s_client.go | Update dynamic-client create/get helpers to v1beta1 Sandbox/SandboxClaim types. |
| pkg/workloadmanager/handlers.go | Update handler plumbing and annotation constants to v1beta1. |
| pkg/workloadmanager/handlers_test.go | Update handler tests for v1beta1 types/constants (fixtures still need alignment). |
| pkg/workloadmanager/codeinterpreter_controller.go | Update CodeInterpreter reconciliation logic to v1beta1 extension resources. |
| pkg/workloadmanager/codeinterpreter_controller_test.go | Update controller tests to v1beta1 template/warm-pool schemas. |
| go.sum | Dependency checksum updates for the upgrade. |
| go.mod | Bump agent-sandbox, k8s libs, controller-runtime, and related deps. |
| docs/proposals/sandbox-pool-management/README.md | Add new design proposal (sandbox resource pool management). |
| docs/getting-started.md | Update install/upgrade instructions for agent-sandbox v0.5.2. |
| cmd/workload-manager/main.go | Register v1beta1 schemes and update controller For() type. |
Comments suppressed due to low confidence (1)
pkg/workloadmanager/handlers_test.go:323
- This test constructs a v1beta1 SandboxClaim object but sets TypeMeta.APIVersion to v1alpha1. That inconsistency can break unstructured conversion / fake dynamic client behavior and makes the test exercise a different API version than the production code.
claim := &extensionsv1beta1.SandboxClaim{
TypeMeta: metav1.TypeMeta{
APIVersion: "extensions.agents.x-k8s.io/v1alpha1",
Kind: types.SandboxClaimsKind,
},
…spec path, nil check on Replicas, and tests
…and scheme.Builder deprecations
… for v1beta1 semantics and clarify Helm upgrades in docs
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 21 out of 27 changed files in this pull request and generated 4 comments.
Files not reviewed (5)
- client-go/clientset/versioned/fake/clientset_generated.go: Generated file
- client-go/informers/externalversions/factory.go: Generated file
- client-go/informers/externalversions/internalinterfaces/factory_interfaces.go: Generated file
- client-go/informers/externalversions/runtime/v1alpha1/agentruntime.go: Generated file
- client-go/informers/externalversions/runtime/v1alpha1/codeinterpreter.go: Generated file
Signed-off-by: Safiya <[email protected]>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 21 out of 27 changed files in this pull request and generated 3 comments.
Files not reviewed (5)
- client-go/clientset/versioned/fake/clientset_generated.go: Generated file
- client-go/informers/externalversions/factory.go: Generated file
- client-go/informers/externalversions/internalinterfaces/factory_interfaces.go: Generated file
- client-go/informers/externalversions/runtime/v1alpha1/agentruntime.go: Generated file
- client-go/informers/externalversions/runtime/v1alpha1/codeinterpreter.go: Generated file
| // WaitForCacheSync blocks until all started informers' caches were synced | ||
| // or the stop channel gets closed. | ||
| // | ||
| // Contextual logging: WaitForCacheSync should be used instead of WaitForCacheSync in code which supports contextual logging. It also returns a more useful result. |
Co-authored-by: Copilot Autofix powered by AI <[email protected]> Signed-off-by: Safiya <[email protected]>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 21 out of 27 changed files in this pull request and generated 2 comments.
Files not reviewed (5)
- client-go/clientset/versioned/fake/clientset_generated.go: Generated file
- client-go/informers/externalversions/factory.go: Generated file
- client-go/informers/externalversions/internalinterfaces/factory_interfaces.go: Generated file
- client-go/informers/externalversions/runtime/v1alpha1/agentruntime.go: Generated file
- client-go/informers/externalversions/runtime/v1alpha1/codeinterpreter.go: Generated file
| // Always prefer using an informer factory to get a shared informer instead of getting an independent | ||
| // one. This reduces memory footprint and number of connections to the server. | ||
| func NewAgentRuntimeInformerWithOptions(client versioned.Interface, namespace string, options internalinterfaces.InformerOptions) cache.SharedIndexInformer { | ||
| gvr := schema.GroupVersionResource{Group: "runtime", Version: "v1alpha1", Resource: "agentruntimes"} |
| // WaitForCacheSync blocks until all started informers' caches were synced | ||
| // or the stop channel gets closed. | ||
| // | ||
| // Contextual logging: WaitForCacheSync should be used instead of WaitForCacheSync in code which supports contextual logging. It also returns a more useful result. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 21 out of 27 changed files in this pull request and generated 2 comments.
Files not reviewed (5)
- client-go/clientset/versioned/fake/clientset_generated.go: Generated file
- client-go/informers/externalversions/factory.go: Generated file
- client-go/informers/externalversions/internalinterfaces/factory_interfaces.go: Generated file
- client-go/informers/externalversions/runtime/v1alpha1/agentruntime.go: Generated file
- client-go/informers/externalversions/runtime/v1alpha1/codeinterpreter.go: Generated file
Comments suppressed due to low confidence (1)
client-go/clientset/versioned/fake/clientset_generated.go:90
- The fake clientset method is named
IsWatchListSemanticsUnSupported, which doesn't follow Go naming conventions ("Unsupported") and may not satisfy the optional reflector interface check if it expectsIsWatchListSemanticsUnsupported. Align the method (and its doc comment) to the canonical spelling to ensure watchlist semantics are correctly disabled in tests.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 21 out of 27 changed files in this pull request and generated 3 comments.
Files not reviewed (5)
- client-go/clientset/versioned/fake/clientset_generated.go: Generated file
- client-go/informers/externalversions/factory.go: Generated file
- client-go/informers/externalversions/internalinterfaces/factory_interfaces.go: Generated file
- client-go/informers/externalversions/runtime/v1alpha1/agentruntime.go: Generated file
- client-go/informers/externalversions/runtime/v1alpha1/codeinterpreter.go: Generated file
| // Always prefer using an informer factory to get a shared informer instead of getting an independent | ||
| // one. This reduces memory footprint and number of connections to the server. | ||
| func NewCodeInterpreterInformerWithOptions(client versioned.Interface, namespace string, options internalinterfaces.InformerOptions) cache.SharedIndexInformer { | ||
| gvr := schema.GroupVersionResource{Group: "runtime", Version: "v1alpha1", Resource: "codeinterpreters"} |
| // WaitForCacheSync blocks until all started informers' caches were synced | ||
| // or the stop channel gets closed. | ||
| // | ||
| // Contextual logging: WaitForCacheSync should be used instead of WaitForCacheSync in code which supports contextual logging. It also returns a more useful result. |
Which issue(s) this PR fixes:
Fixes #438
Will resume my work tomorrow...