Skip to content

fix(deployment): migrate workload update calls to patch#328

Open
Nazihbenbrahim wants to merge 2 commits intoopenkruise:masterfrom
Nazihbenbrahim:fix/deployment-update-to-patch
Open

fix(deployment): migrate workload update calls to patch#328
Nazihbenbrahim wants to merge 2 commits intoopenkruise:masterfrom
Nazihbenbrahim:fix/deployment-update-to-patch

Conversation

@Nazihbenbrahim
Copy link

Ⅰ. Describe what this PR does

This PR migrates the remaining workload-related Update calls to Patch in the advanced deployment controller to improve Kubernetes version-skew safety.

When Rollouts is built against older Kubernetes API types but runs against a newer cluster, full-object Update may erase fields unknown to the old client structs. Patch only sends the changed delta and avoids overwriting unknown fields.

This PR changes:

  • Replace remaining Deployment.Status().Update(...) calls with Status().Patch(...) in the advanced deployment controller
  • Replace c.Update(...) with c.Patch(...) in UpdateFinalizer
  • Keep existing behavior unchanged while switching the write path from full update to patch
  • Add tests covering:
    • status patch path
    • empty-selector sync path
    • finalizer patch behavior and idempotence

Main code changes:

  • pkg/controller/deployment/deployment_controller.go
  • pkg/controller/deployment/sync.go
  • pkg/util/workloads_utils.go

Tests added/updated:

  • pkg/controller/deployment/status_patch_test.go
  • pkg/controller/deployment/deployment_controller_test.go
  • pkg/util/workloads_utils_test.go

Ⅱ. Does this pull request fix one issue?

fixes #273

Ⅲ. Special notes for reviews

Verification commands:

gofmt -w \
  pkg/controller/deployment/deployment_controller.go \
  pkg/controller/deployment/deployment_controller_test.go \
  pkg/controller/deployment/sync.go \
  pkg/controller/deployment/status_patch_test.go \
  pkg/util/workloads_utils.go \
  pkg/util/workloads_utils_test.go

go test ./pkg/controller/deployment/... -v -count=1
go test ./pkg/util/... -v -count=1

rg -n "c\.Update\(|Status\(\)\.Update\(" pkg/controller/deployment pkg/util/workloads_utils.go

Signed-off-by: Nazih Ben Brahim <nazihbenbrahim9@gmail.com>
@kruise-bot kruise-bot requested review from FillZpp and veophi March 1, 2026 12:43
@kruise-bot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign fillzpp for approval by writing /assign @fillzpp in a comment. For more information see:The Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@kruise-bot
Copy link

Welcome @Nazihbenbrahim! It looks like this is your first PR to openkruise/rollouts 🎉

@codecov
Copy link

codecov bot commented Mar 1, 2026

Codecov Report

❌ Patch coverage is 72.72727% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 52.47%. Comparing base (8416512) to head (6ac351f).

Files with missing lines Patch % Lines
pkg/controller/deployment/sync.go 50.00% 3 Missing and 3 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #328      +/-   ##
==========================================
+ Coverage   51.38%   52.47%   +1.08%     
==========================================
  Files          66       66              
  Lines        8559     8568       +9     
==========================================
+ Hits         4398     4496      +98     
+ Misses       3575     3479      -96     
- Partials      586      593       +7     
Flag Coverage Δ
unittests 52.47% <72.72%> (+1.08%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Signed-off-by: Nazih Ben Brahim <nazihbenbrahim9@gmail.com>
@kruise-bot kruise-bot added size/XL and removed size/L labels Mar 1, 2026
@Nazihbenbrahim
Copy link
Author

@furykerry Can you please review this PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

change workload controller to use patch instead of update

2 participants