Skip to content

Commit 41ab580

Browse files
author
zhihao jian
committed
change workload controller to use patch instead of update
Signed-off-by: zhihao jian <zhihao.jian@shopee.com> fix test use MergeFrom func to patch data
1 parent deaa5f3 commit 41ab580

File tree

4 files changed

+36
-6
lines changed

4 files changed

+36
-6
lines changed

pkg/controller/deployment/controller.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,7 @@ func newReconciler(mgr manager.Manager) (reconcile.Reconciler, error) {
107107
// Deployment controller factory
108108
factory := &controllerFactory{
109109
client: genericClient.KubeClient,
110+
runtimeClient: mgr.GetClient(),
110111
eventBroadcaster: eventBroadcaster,
111112
eventRecorder: recorder,
112113
dLister: dLister,
@@ -268,6 +269,7 @@ func (f *controllerFactory) NewController(deployment *appsv1.Deployment) *Deploy
268269

269270
return &DeploymentController{
270271
client: f.client,
272+
runtimeClient: f.runtimeClient,
271273
eventBroadcaster: f.eventBroadcaster,
272274
eventRecorder: f.eventRecorder,
273275
dLister: f.dLister,

pkg/controller/deployment/deployment_controller.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ import (
3939

4040
rolloutsv1alpha1 "github.com/openkruise/rollouts/api/v1alpha1"
4141
deploymentutil "github.com/openkruise/rollouts/pkg/controller/deployment/util"
42+
"sigs.k8s.io/controller-runtime/pkg/client"
4243
)
4344

4445
const (
@@ -68,6 +69,8 @@ type DeploymentController struct {
6869

6970
// we will use this strategy to replace spec.strategy of deployment
7071
strategy rolloutsv1alpha1.DeploymentStrategy
72+
73+
runtimeClient client.Client
7174
}
7275

7376
// getReplicaSetsForDeployment uses ControllerRefManager to reconcile

pkg/controller/deployment/sync.go

Lines changed: 31 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import (
3030
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
3131
"k8s.io/klog/v2"
3232
"k8s.io/utils/integer"
33+
"sigs.k8s.io/controller-runtime/pkg/client"
3334

3435
rolloutsv1alpha1 "github.com/openkruise/rollouts/api/v1alpha1"
3536
deploymentutil "github.com/openkruise/rollouts/pkg/controller/deployment/util"
@@ -118,7 +119,14 @@ func (dc *DeploymentController) getNewReplicaSet(ctx context.Context, d *apps.De
118119
minReadySecondsNeedsUpdate := rsCopy.Spec.MinReadySeconds != d.Spec.MinReadySeconds
119120
if annotationsUpdated || minReadySecondsNeedsUpdate {
120121
rsCopy.Spec.MinReadySeconds = d.Spec.MinReadySeconds
121-
return dc.client.AppsV1().ReplicaSets(rsCopy.ObjectMeta.Namespace).Update(ctx, rsCopy, metav1.UpdateOptions{})
122+
123+
// Use controller-runtime's MergeFrom for more concise and resilient patching
124+
// This automatically handles strategic merge patches and is more maintainable
125+
err := dc.runtimeClient.Patch(ctx, rsCopy, client.MergeFrom(existingNewRS))
126+
if err != nil {
127+
return nil, err
128+
}
129+
return rsCopy, nil
122130
}
123131

124132
// Should use the revision in existingNewRS's annotation, since it set by before
@@ -410,11 +418,29 @@ func (dc *DeploymentController) scaleReplicaSet(ctx context.Context, rs *apps.Re
410418
var err error
411419
if sizeNeedsUpdate || annotationsNeedUpdate {
412420
oldScale := *(rs.Spec.Replicas)
421+
422+
// Create a copy to modify without affecting the original
413423
rsCopy := rs.DeepCopy()
414-
*(rsCopy.Spec.Replicas) = newScale
415-
deploymentutil.SetReplicasAnnotations(rsCopy, *(deployment.Spec.Replicas), *(deployment.Spec.Replicas)+deploymentutil.MaxSurge(deployment, &dc.strategy))
416-
rs, err = dc.client.AppsV1().ReplicaSets(rsCopy.Namespace).Update(ctx, rsCopy, metav1.UpdateOptions{})
417-
if err == nil && sizeNeedsUpdate {
424+
425+
if sizeNeedsUpdate {
426+
rsCopy.Spec.Replicas = &newScale
427+
}
428+
if annotationsNeedUpdate {
429+
// Set the annotations that need to be updated
430+
desiredReplicas := *(deployment.Spec.Replicas)
431+
maxReplicas := *(deployment.Spec.Replicas) + deploymentutil.MaxSurge(deployment, &dc.strategy)
432+
deploymentutil.SetReplicasAnnotations(rsCopy, desiredReplicas, maxReplicas)
433+
}
434+
435+
// Use controller-runtime's MergeFrom for more concise and resilient patching
436+
// This automatically handles strategic merge patches and is more maintainable
437+
err = dc.runtimeClient.Patch(ctx, rsCopy, client.MergeFrom(rs))
438+
if err != nil {
439+
return scaled, rs, err
440+
}
441+
442+
rs = rsCopy
443+
if sizeNeedsUpdate {
418444
scaled = true
419445
dc.eventRecorder.Eventf(deployment, v1.EventTypeNormal, "ScalingReplicaSet", "Scaled %s replica set %s to %d from %d", scalingOperation, rs.Name, newScale, oldScale)
420446
}

pkg/util/patch/patch_utils_test.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,5 +41,4 @@ func TestCommonPatch(t *testing.T) {
4141
if !reflect.DeepEqual(patchReq.String(), expectedPatchBody) {
4242
t.Fatalf("Not equal: \n%s \n%s", expectedPatchBody, patchReq.String())
4343
}
44-
4544
}

0 commit comments

Comments
 (0)