Skip to content

Commit cb54adf

Browse files
authored
feat(client): add Patch and PatchOnConflict (#100)
* feat(client): add Patch and PatchOnConflict - Implement Patch and PatchOnConflict utility functions - Replace UpdateOnConflict with PatchOnConflict in finalizer operations - Add comprehensive test suite for patch operations * fix lint * fix lint
1 parent 6043805 commit cb54adf

File tree

5 files changed

+638
-85
lines changed

5 files changed

+638
-85
lines changed

client/finalizer.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ func AddFinalizerAndUpdate(ctx context.Context, c client.Client, obj client.Obje
2727
if controllerutil.ContainsFinalizer(obj, finalizer) {
2828
return nil
2929
}
30-
_, err := UpdateOnConflict(ctx, c, c, obj, func(obj client.Object) error {
30+
_, err := PatchOnConflict(ctx, c, c, obj, func(obj client.Object) error {
3131
controllerutil.AddFinalizer(obj, finalizer)
3232
return nil
3333
})
@@ -38,7 +38,7 @@ func RemoveFinalizerAndUpdate(ctx context.Context, c client.Client, obj client.O
3838
if !controllerutil.ContainsFinalizer(obj, finalizer) {
3939
return nil
4040
}
41-
_, err := UpdateOnConflict(ctx, c, c, obj, func(obj client.Object) error {
41+
_, err := PatchOnConflict(ctx, c, c, obj, func(obj client.Object) error {
4242
controllerutil.RemoveFinalizer(obj, finalizer)
4343
return nil
4444
})

client/finalizer_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ import (
3131
func TestAddFinalizerAndUpdate(t *testing.T) {
3232
c := fake.NewClientBuilder().WithScheme(scheme.Scheme).Build()
3333

34-
pod := newTestPod()
34+
pod := newTestPod("test-pod")
3535
err := c.Create(context.Background(), pod)
3636
require.NoError(t, err)
3737

@@ -62,7 +62,7 @@ func TestAddFinalizerAndUpdate(t *testing.T) {
6262
func TestRemoveFinalizerAndUpdate(t *testing.T) {
6363
c := fake.NewClientBuilder().WithScheme(scheme.Scheme).Build()
6464

65-
pod := newTestPod()
65+
pod := newTestPod("test-pod")
6666
err := c.Create(context.Background(), pod)
6767
require.NoError(t, err)
6868

@@ -95,7 +95,7 @@ func TestRemoveFinalizerAndUpdate(t *testing.T) {
9595
func TestRemoveFinalizerAndDelete(t *testing.T) {
9696
c := fake.NewClientBuilder().WithScheme(scheme.Scheme).Build()
9797

98-
pod := newTestPod()
98+
pod := newTestPod("test-pod")
9999
err := c.Create(context.Background(), pod)
100100
require.NoError(t, err)
101101

client/suit_test.go

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
/*
2+
Copyright 2025 The KusionStack Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package client
18+
19+
import (
20+
"testing"
21+
22+
"github.com/stretchr/testify/suite"
23+
)
24+
25+
// In order for 'go test' to run this suite, we need to create
26+
// a normal test function and pass our suite to suite.Run
27+
func TestClientTestSuite(t *testing.T) {
28+
suite.Run(t, new(clientTestSuite))
29+
}

client/util.go

Lines changed: 134 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -37,43 +37,48 @@ type UpdateWriter interface {
3737
Update(ctx context.Context, obj client.Object, opts ...client.UpdateOption) error
3838
}
3939

40-
// UpdateOnConflict attempts to update a resource while avoiding conflicts that may arise from concurrent modifications.
41-
// It utilizes the mutateFn function to apply changes to the original obj and then attempts an update using the writer,
42-
// which can be either client.Writer or client.StatusWriter.
43-
// In case of an update failure due to a conflict, UpdateOnConflict will retrieve the latest version of the object using
44-
// the reader and attempt the update again.
45-
// The retry mechanism adheres to the retry.DefaultBackoff policy.
40+
// UpdateOnConflict attempts to update a resource while avoiding conflicts that may arise
41+
// from concurrent modifications. It utilizes the mutateFn function to apply changes to the
42+
// input obj and then attempts an update using the writer, which can be either client.Writer
43+
// or client.StatusWriter.
44+
//
45+
// In case of an update failure due to a conflict, UpdateOnConflict will retrieve the latest version
46+
// of the object using the reader and attempt the update again. The retry mechanism adheres to the
47+
// retry.DefaultBackoff policy.
48+
//
49+
// NOTE: It must be ensured that the input object has not been modified in any way after being obtained
50+
// from cache or apiserver.
4651
func UpdateOnConflict[T client.Object](
4752
ctx context.Context,
4853
reader client.Reader,
4954
writer UpdateWriter,
50-
original T,
55+
inputObj T,
5156
mutateFn MutateFn[T],
5257
) (changed bool, err error) {
53-
key := client.ObjectKeyFromObject(original)
58+
key := client.ObjectKeyFromObject(inputObj)
5459
first := true
5560

5661
err = retry.RetryOnConflict(retry.DefaultBackoff, func() error {
5762
if !first {
5863
// refresh object
59-
if innerErr := reader.Get(ctx, key, original); innerErr != nil {
64+
if innerErr := reader.Get(ctx, key, inputObj); innerErr != nil {
6065
return innerErr
6166
}
6267
} else {
6368
first = false
6469
}
6570

66-
existing := original.DeepCopyObject()
67-
if innerErr := mutate(mutateFn, key, original); innerErr != nil {
71+
original := inputObj.DeepCopyObject()
72+
if innerErr := mutate(mutateFn, key, inputObj); innerErr != nil {
6873
return innerErr
6974
}
7075

71-
if equality.Semantic.DeepEqual(existing, original) {
76+
if equality.Semantic.DeepEqual(original, inputObj) {
7277
// nothing changed, skip update
7378
return nil
7479
}
7580

76-
if innerErr := writer.Update(ctx, original); innerErr != nil {
81+
if innerErr := writer.Update(ctx, inputObj); innerErr != nil {
7782
return innerErr
7883
}
7984

@@ -106,24 +111,24 @@ func CreateOrUpdateOnConflict[T client.Object](
106111
ctx context.Context,
107112
reader client.Reader,
108113
writer client.Writer,
109-
original T,
114+
inputObj T,
110115
f MutateFn[T],
111116
) (controllerutil.OperationResult, error) {
112-
key := client.ObjectKeyFromObject(original)
113-
if err := reader.Get(ctx, key, original); err != nil {
117+
key := client.ObjectKeyFromObject(inputObj)
118+
if err := reader.Get(ctx, key, inputObj); err != nil {
114119
if !apierrors.IsNotFound(err) {
115120
return controllerutil.OperationResultNone, err
116121
}
117122
// not found, create it
118-
if err := mutate(f, key, original); err != nil {
123+
if err := mutate(f, key, inputObj); err != nil {
119124
return controllerutil.OperationResultNone, err
120125
}
121-
if err := writer.Create(ctx, original); err != nil {
126+
if err := writer.Create(ctx, inputObj); err != nil {
122127
return controllerutil.OperationResultNone, err
123128
}
124129
return controllerutil.OperationResultCreated, nil
125130
}
126-
changed, err := UpdateOnConflict(ctx, reader, writer, original, f)
131+
changed, err := UpdateOnConflict(ctx, reader, writer, inputObj, f)
127132
if err != nil {
128133
return controllerutil.OperationResultNone, err
129134
}
@@ -132,3 +137,113 @@ func CreateOrUpdateOnConflict[T client.Object](
132137
}
133138
return controllerutil.OperationResultUpdated, nil
134139
}
140+
141+
type PatchWriter interface {
142+
// Patch patches the given obj in the Kubernetes cluster. obj must be a
143+
// struct pointer so that obj can be updated with the content returned by the Server.
144+
Patch(ctx context.Context, obj client.Object, patch client.Patch, opts ...client.PatchOption) error
145+
}
146+
147+
// Patch applies changes to a Kubernetes object using the merge patch strategy.
148+
// It creates a merge patch from the original object, applies the mutateFn to modify the object,
149+
// and then applies the patch if there are actual changes.
150+
// Returns true if changes were applied, false otherwise.
151+
func Patch[T client.Object](
152+
ctx context.Context,
153+
writer PatchWriter,
154+
inputObj T,
155+
mutateFn MutateFn[T],
156+
) (changed bool, err error) {
157+
original := inputObj.DeepCopyObject().(client.Object)
158+
159+
mergePatch := client.MergeFrom(original)
160+
161+
// modify inputObj object
162+
err = mutateFn(inputObj)
163+
if err != nil {
164+
return false, err
165+
}
166+
167+
// calculate patch data
168+
patchData, err := mergePatch.Data(inputObj)
169+
if err != nil {
170+
return false, err
171+
}
172+
173+
if len(patchData) == 0 || string(patchData) == "{}" {
174+
// nothing changed, skip update
175+
return false, nil
176+
}
177+
178+
err = writer.Patch(ctx, inputObj, mergePatch)
179+
if err != nil {
180+
return false, err
181+
}
182+
183+
// patched
184+
return true, nil
185+
}
186+
187+
// PatchOnConflict attempts to patch a resource while avoiding conflicts that may arise
188+
// from concurrent modifications. It utilizes the mutateFn function to apply changes to the
189+
// input obj and then attempts an mergePatch using the writer, which can be either client.Writer
190+
// or client.StatusWriter.
191+
//
192+
// In case of an update failure due to a conflict, PatchOnConflict will retrieve the latest version
193+
// of the object using the reader and attempt the update again. The retry mechanism adheres to the
194+
// retry.DefaultBackoff policy.
195+
//
196+
// NOTE: It must be ensured that the input object has not been modified in any way after being obtained
197+
// from cache or apiserver.
198+
func PatchOnConflict[T client.Object](
199+
ctx context.Context,
200+
reader client.Reader,
201+
writer PatchWriter,
202+
inputObj T,
203+
mutateFn MutateFn[T],
204+
) (changed bool, err error) {
205+
key := client.ObjectKeyFromObject(inputObj)
206+
first := true
207+
208+
err = retry.RetryOnConflict(retry.DefaultBackoff, func() error {
209+
if !first {
210+
// refresh object
211+
if innerErr := reader.Get(ctx, key, inputObj); innerErr != nil {
212+
return innerErr
213+
}
214+
} else {
215+
first = false
216+
}
217+
218+
original := inputObj.DeepCopyObject().(client.Object)
219+
220+
// modify inputObj object
221+
if innerErr := mutateFn(inputObj); innerErr != nil {
222+
return innerErr
223+
}
224+
225+
mergePatch := client.MergeFrom(original)
226+
227+
// calculate patch data
228+
patchData, err := mergePatch.Data(inputObj)
229+
if err != nil {
230+
return err
231+
}
232+
233+
if len(patchData) == 0 || string(patchData) == "{}" {
234+
// nothing changed, skip update
235+
return nil
236+
}
237+
238+
// use resourceVersion as optimisticLock
239+
mergePatchWithOptimisticLock := client.MergeFromWithOptions(original, client.MergeFromWithOptimisticLock{})
240+
if innerErr := writer.Patch(ctx, inputObj, mergePatchWithOptimisticLock); innerErr != nil {
241+
return innerErr
242+
}
243+
244+
changed = true
245+
return nil
246+
})
247+
248+
return changed, err
249+
}

0 commit comments

Comments
 (0)