feat: implement batch-based rollout and scaling using RollingUpdateDeployment#472
feat: implement batch-based rollout and scaling using RollingUpdateDeployment#472sangheee wants to merge 4 commits intozilliztech:mainfrom
RollingUpdateDeployment#472Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: sangheee 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 |
7eee51c to
0882b89
Compare
Signed-off-by: park.sanghee <park.sanghee@navercorp.com>
0882b89 to
da386b8
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #472 +/- ##
==========================================
- Coverage 76.70% 76.64% -0.06%
==========================================
Files 66 66
Lines 6176 6196 +20
==========================================
+ Hits 4737 4749 +12
- Misses 1176 1180 +4
- Partials 263 267 +4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Thank you, @AlintaLu please help review |
There was a problem hiding this comment.
Pull request overview
This PR adds configurable Kubernetes-style rolling update parameters (maxSurge / maxUnavailable) to Milvus component specs and uses them to adjust replica changes in larger batches during two-deployment rollouts, improving rollout/scaling performance for large clusters.
Changes:
- Added
rollingUpdate(*appsv1.RollingUpdateDeployment) toComponentSpecand merged it viaMergeComponentSpec. - Updated deployment strategy resolution to accept
*v1beta1.MilvusSpecand apply per-component rollingUpdate overrides. - Updated
planScaleForRolloutto compute batch steps frommaxSurge/maxUnavailable, plus corresponding CRD and test updates.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/controllers/deployments.go | Minor logging change for deployment diffs. |
| pkg/controllers/deployment_updater.go | Updates strategy plumbing to pass *MilvusSpec into GetDeploymentStrategy. |
| pkg/controllers/deploy_ctrl_util.go | Implements batch-based rollout scaling using rollingUpdate-derived step sizes. |
| pkg/controllers/deploy_ctrl_util_test.go | Sets default rolling update strategy in test fixtures to match new rollout logic dependencies. |
| pkg/controllers/components.go | Allows per-component rollingUpdate overrides when using RollingUpdate strategy. |
| pkg/controllers/components_test.go | Adds tests for rollingUpdate merge + custom strategy behavior. |
| apis/milvus.io/v1beta1/components_types.go | Adds the rollingUpdate API field to ComponentSpec. |
| apis/milvus.io/v1beta1/zz_generated.deepcopy.go | Updates deepcopy generation for the new rollingUpdate field. |
| config/crd/bases/milvus.io_milvuses.yaml | Adds rollingUpdate schema to Milvus CRD. |
| config/crd/bases/milvus.io_milvusclusters.yaml | Adds rollingUpdate schema to MilvusCluster CRD. |
| charts/milvus-operator/templates/crds.yaml | Propagates CRD schema changes into the Helm chart CRDs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…omment Signed-off-by: park.sanghee <park.sanghee@navercorp.com>
|
Hi @AlintaLu , I've addressed all the suggestions and the PR is now ready for another look. Could you please take a look when you have a moment? Thanks! |
Hi, first of all, I would like to thank you for your positive feedback on my initial suggestions and for taking the time to review this PR.
This PR improves the rollout and scaling performance of Milvus components by implementing batch-based replica adjustment. It replaces the current hardcoded step-by-step (1, -1) approach with a configurable strategy using the standard Kubernetes
DeploymentStrategy.Changes
*appsv1.RollingUpdateDeploymenttoComponentSpec.surgeStepandunavailableStepinplanScaleForRolloutfor parallel pod replacement.Even if
podReplacementPolicyis introduced toDeploymentandtwoDeploymentModeis removed in the future, having this RollingUpdate configuration will be helpful as it can work alongside the policy to support both faster and more reliable updates.Related Issue
Fixes #470
cc. @haorenfsa