refactor(controllers): clean up deprecated v1alpha1 references and no…#1064
refactor(controllers): clean up deprecated v1alpha1 references and no…#1064SHRUTI6991 wants to merge 1 commit into
Conversation
…ne warm pool fallback.
✅ Deploy Preview for agent-sandbox ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
kubernetes-prow
Bot
added
the
cncf-cla: yes
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
💤 Files with no reviewable changes (1)
📝 WalkthroughWalkthroughMoves ChangesScheme registration and poolName fix
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
kubernetes-prow
Bot
added
the
size/S
![coderabbitai[bot]](https://cdn.statically.io/img/avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
This PR removes remaining deprecated api/v1alpha1 wiring from the core controllers package (while still registering it in the controller manager for webhook conversion support) and simplifies warm-pool name handling during Sandbox adoption.
Changes:
- Removed
v1alpha1import andAddToSchemeregistration fromcontrollers/sandbox_controller.go’s shared scheme initialization. - Registered
api/v1alpha1in the controller manager (cmd/agent-sandbox-controller/main.go) to keep conversion webhook decoding working. - Simplified warm pool name resolution in
SandboxClaimReconciler.adoptSandboxFromCandidatesby removing the"none"fallback and usinggetWarmPoolName(...)directly.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| extensions/controllers/sandboxclaim_controller.go | Removes redundant "none" fallback for warm pool name during adoption logging/metrics. |
| controllers/sandbox_controller.go | Decouples the controllers package scheme from deprecated v1alpha1 types. |
| cmd/agent-sandbox-controller/main.go | Ensures the manager scheme still registers v1alpha1 for conversion webhook support. |
|
/lgtm |
kubernetes-prow
Bot
added
the
lgtm
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: aditya-shantanu, dongjiang1989, SHRUTI6991 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 |
janetkuo
left a comment
janetkuo
left a comment
There was a problem hiding this comment.
This lgtm, but would be good to fix the log level
| } | ||
|
|
||
| poolName := getWarmPoolName(adopted) | ||
| logger.Info("Attempting sandbox adoption", "sandbox candidate", adopted.Name, "warm pool", poolName, "claim", claim.Name) |
There was a problem hiding this comment.
While you're at it, update this to use V(4) for workflow checkpoints. Reserve logger.Info for major lifecycle events and state transitions to avoid spammy logs
Description
This PR cleans up the remaining deprecated
v1alpha1schema references from the core Sandbox controller and removes the redundant"none"warm pool name fallback.Changes
v1alpha1schema imports and registration fromsandbox_controller.go. Registeredv1alpha1directly in the manager to preserve conversion webhooks."none"Warm Pool Fallback: Simplified pool name resolution insideadoptSandboxFromCandidatesinsandboxclaim_controller.goto directly callgetWarmPoolName(adopted).Verification
go test -v ./controllers/... ./extensions/controllers/...Summary by CodeRabbit
"none"in adoption tracking and logs.