refactor(controller): refactor common control in sandbox controller#496
refactor(controller): refactor common control in sandbox controller#496Liquorice-Ma wants to merge 1 commit into
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 |
|
Welcome @13910796706! It looks like this is your first PR to openkruise/agents 🎉 |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #496 +/- ##
==========================================
- Coverage 78.37% 78.34% -0.04%
==========================================
Files 162 162
Lines 11744 11787 +43
==========================================
+ Hits 9204 9234 +30
- Misses 2186 2195 +9
- Partials 354 358 +4
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:
|
|
@13910796706: PR needs rebase. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
Ⅰ. Describe what this PR does
Refactor sandbox controller core to extract shared helper functions from
commonControl, enabling reuse by different control implementations.Key changes:
ensureSandboxUpdatedCommon,ensureSandboxUpgradedCommon,ensureSandboxTerminatedCommonas shared orchestration functions with dependency injection via args structs (UpgradeOrchestratorArgs,RecreateUpgradeArgs,UpgradeActionArgs)createPodCommonwith configurablegeneratePodFuncandtolerateAlreadyExistsparameterperformRecreateUpgradeCommonandexecuteUpgradeActionCommonfor upgrade lifecycle reusesetReadyConditionFalseForPauseandperformPostResumeInitas shared pause/resume helpersSandboxFinalizerandPodConditionContainersPaused/Resumedconstants fromcorepackage topkg/utilsto avoid circular dependenciessyncSandboxStatusFromPodto aggregate all container failure messages instead of only keeping the last oneEnsureSandboxResumedwhenrCondis nilThe existing
commonControlmethods now delegate to these shared functions, preserving full backward compatibility. No behavioral changes for existing users.Ⅱ. Does this pull request fix one issue?
NONE
Ⅲ. Describe how to verify it
go test ./pkg/controller/sandbox/core/...Run unit tests for the full sandbox controller package:
go test ./pkg/controller/sandbox/...
Verify full project builds successfully:
go build ./...
All existing tests pass without modification to test logic (only function signature adaptations).
Ⅳ. Special notes for reviews
This is a pure refactoring PR — all commonControl public methods retain their original signatures and behavior.
The extracted *Common functions accept dependency structs instead of embedding commonControl directly, making them composable for alternative control implementations.
The SandboxFinalizer / PodConditionContainersPaused / PodConditionContainersResumed constants moved from pkg/controller/sandbox/core to pkg/utils. All call sites (sandbox_controller.go, pod_event_handler.go, and their tests) have been updated accordingly.
The SandboxPodNotFoundMsg constant is defined locally in common_control.go for now; it can be relocated once additional consumers exist.
The CA certificate injection test (TestCommonControl_createPod_WithCAInjection) was removed from common_control_test.go as the CA injection logic is now tested through createPodCommon's integration path.