Skip to content

Commit

Permalink
✨ Fakeclient: Add apply support
Browse files Browse the repository at this point in the history
This change is a POC for adding apply patch support to the fake client.

This relies on the upstream support for this which is implemented in a
new [FieldManagedObjectTracker][0]. There are two major problems with
this for us though:
1. It requires a [type converter][1] which gets initialized with a
   [parser][2] that knows how the type look like. It doesn't look like
   it is possible to pass multiple parsers, which means the resulting
   client can only deals with the set of types the parser knows about,
   for example what is in client-go but it will not work with CRDs
2. We have some code that wants to look at the object after it was
   patched to check if its valid - Since this is implemented in the
   tracker, it doesn't look like its possible to dry run the patch

[0]: https://github.com/kubernetes/kubernetes/blob/4dc7a48ac6fb631a84e1974772bf7b8fd0bb9c59/staging/src/k8s.io/client-go/testing/fixture.go#L643
[1]: https://github.com/kubernetes/kubernetes/blob/4dc7a48ac6fb631a84e1974772bf7b8fd0bb9c59/staging/src/k8s.io/client-go/applyconfigurations/utils.go#L1739
[2]: https://github.com/kubernetes/kubernetes/blob/4dc7a48ac6fb631a84e1974772bf7b8fd0bb9c59/staging/src/k8s.io/client-go/applyconfigurations/internal/internal.go#L28
  • Loading branch information
alvaroaleman committed Oct 14, 2024
1 parent 5af6ffa commit ac5705d
Show file tree
Hide file tree
Showing 2 changed files with 57 additions and 5 deletions.
40 changes: 35 additions & 5 deletions pkg/client/fake/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,12 +45,14 @@ import (
"k8s.io/apimachinery/pkg/labels"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/runtime/serializer"
"k8s.io/apimachinery/pkg/types"
utilrand "k8s.io/apimachinery/pkg/util/rand"
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/apimachinery/pkg/util/strategicpatch"
"k8s.io/apimachinery/pkg/util/validation/field"
"k8s.io/apimachinery/pkg/watch"
clientgoapplyconfigurations "k8s.io/client-go/applyconfigurations"
"k8s.io/client-go/kubernetes/scheme"
"k8s.io/client-go/testing"
"k8s.io/utils/ptr"
Expand Down Expand Up @@ -230,7 +232,11 @@ func (f *ClientBuilder) Build() client.WithWatch {
}

if f.objectTracker == nil {
tracker = versionedTracker{ObjectTracker: testing.NewObjectTracker(f.scheme, scheme.Codecs.UniversalDecoder()), scheme: f.scheme, withStatusSubresource: withStatusSubResource}
tracker = versionedTracker{ObjectTracker: testing.NewFieldManagedObjectTracker(
f.scheme,
serializer.NewCodecFactory(f.scheme).UniversalDecoder(),
clientgoapplyconfigurations.NewTypeConverter(f.scheme),
)}
} else {
tracker = versionedTracker{ObjectTracker: f.objectTracker, scheme: f.scheme, withStatusSubresource: withStatusSubResource}
}
Expand Down Expand Up @@ -868,6 +874,12 @@ func (c *fakeClient) patch(obj client.Object, patch client.Patch, opts ...client
if err != nil {
return err
}

// otherwise the merge logic in the tracker complains
if patch.Type() == types.ApplyPatchType {
obj.SetManagedFields(nil)
}

data, err := patch.Data(obj)
if err != nil {
return err
Expand All @@ -880,7 +892,11 @@ func (c *fakeClient) patch(obj client.Object, patch client.Patch, opts ...client

oldObj, err := c.tracker.Get(gvr, accessor.GetNamespace(), accessor.GetName())
if err != nil {
return err
if patch.Type() == types.ApplyPatchType {
oldObj = &unstructured.Unstructured{}
} else {
return err
}
}
oldAccessor, err := meta.Accessor(oldObj)
if err != nil {
Expand All @@ -895,7 +911,7 @@ func (c *fakeClient) patch(obj client.Object, patch client.Patch, opts ...client
// This ensures that the patch may be rejected if a deletionTimestamp is modified, prior
// to updating the object.
action := testing.NewPatchAction(gvr, accessor.GetNamespace(), accessor.GetName(), patch.Type(), data)
o, err := dryPatch(action, c.tracker)
o, err := dryPatch(action, c.tracker, obj)
if err != nil {
return err
}
Expand Down Expand Up @@ -954,12 +970,15 @@ func deletionTimestampEqual(newObj metav1.Object, obj metav1.Object) bool {
// This results in some code duplication, but was found to be a cleaner alternative than unmarshalling and introspecting the patch data
// and easier than refactoring the k8s client-go method upstream.
// Duplicate of upstream: https://github.com/kubernetes/client-go/blob/783d0d33626e59d55d52bfd7696b775851f92107/testing/fixture.go#L146-L194
func dryPatch(action testing.PatchActionImpl, tracker testing.ObjectTracker) (runtime.Object, error) {
func dryPatch(action testing.PatchActionImpl, tracker testing.ObjectTracker, newObj runtime.Object) (runtime.Object, error) {
ns := action.GetNamespace()
gvr := action.GetResource()

obj, err := tracker.Get(gvr, ns, action.GetName())
if err != nil {
if action.GetPatchType() == types.ApplyPatchType {
return &unstructured.Unstructured{}, nil
}
return nil, err
}

Expand Down Expand Up @@ -1005,7 +1024,18 @@ func dryPatch(action testing.PatchActionImpl, tracker testing.ObjectTracker) (ru
return nil, err
}
case types.ApplyPatchType:
return nil, errors.New("apply patches are not supported in the fake client. Follow https://github.com/kubernetes/kubernetes/issues/115598 for the current status")
// There doesn't seem to be a way to test this without actually applying it as apply is implemented in the tracker.
// We have to make sure no reader sees this and we can not handle errors resetting the obj to the original state.
defer tracker.Add(obj)

Check failure on line 1029 in pkg/client/fake/client.go

View workflow job for this annotation

GitHub Actions / lint

Error return value of `tracker.Add` is not checked (errcheck)

Check failure on line 1029 in pkg/client/fake/client.go

View workflow job for this annotation

GitHub Actions / lint

Error return value of `tracker.Add` is not checked (errcheck)
defer func() {
if err := tracker.Add(obj); err != nil {
panic(err)
}
}()
if err := tracker.Apply(gvr, newObj, ns, action.PatchOptions); err != nil {
return nil, err
}
return tracker.Get(gvr, ns, action.GetName())
default:
return nil, fmt.Errorf("%s PatchType is not supported", action.GetPatchType())
}
Expand Down
22 changes: 22 additions & 0 deletions pkg/client/fake/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2169,6 +2169,28 @@ var _ = Describe("Fake client", func() {
Expect(cl.SubResource(subResourceScale).Update(context.Background(), obj, client.WithSubResourceBody(scale)).Error()).To(Equal(expectedErr))
})

FIt("supports server-side apply", func() {
cl := NewClientBuilder().Build()
obj := &unstructured.Unstructured{}
obj.SetAPIVersion("v1")
obj.SetKind("ConfigMap")
obj.SetName("foo")
unstructured.SetNestedField(obj.Object, map[string]any{"some": "data"}, "data")

Check failure on line 2178 in pkg/client/fake/client_test.go

View workflow job for this annotation

GitHub Actions / lint

Error return value of `unstructured.SetNestedField` is not checked (errcheck)

Check failure on line 2178 in pkg/client/fake/client_test.go

View workflow job for this annotation

GitHub Actions / lint

Error return value of `unstructured.SetNestedField` is not checked (errcheck)

Expect(cl.Patch(context.Background(), obj, client.Apply)).To(Succeed())

cm := &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: "foo"}}

Expect(cl.Get(context.Background(), client.ObjectKeyFromObject(cm), cm)).To(Succeed())
Expect(cm.Data).To(Equal(map[string]string{"some": "data"}))

unstructured.SetNestedField(obj.Object, map[string]any{"other": "data"}, "data")

Check failure on line 2187 in pkg/client/fake/client_test.go

View workflow job for this annotation

GitHub Actions / lint

Error return value of `unstructured.SetNestedField` is not checked (errcheck)

Check failure on line 2187 in pkg/client/fake/client_test.go

View workflow job for this annotation

GitHub Actions / lint

Error return value of `unstructured.SetNestedField` is not checked (errcheck)
Expect(cl.Patch(context.Background(), obj, client.Apply)).To(Succeed())

Expect(cl.Get(context.Background(), client.ObjectKeyFromObject(cm), cm)).To(Succeed())
Expect(cm.Data).To(Equal(map[string]string{"other": "data"}))
})

scalableObjs := []client.Object{
&appsv1.Deployment{
ObjectMeta: metav1.ObjectMeta{
Expand Down

0 comments on commit ac5705d

Please sign in to comment.