client-go: Can not use patch to remove fields via fake client

Hi client-go experts,

I’m having trouble adding unit test around patching resource via the fake client (fake.NewSimpleClientset()). Specifically patch works just fine until the test tries to delete something from the object (e.g. removing a finalizer). Patch to add or update field is working as expected.

--- FAIL: TestPatchFinalizer (0.00s)
    --- PASS: TestPatchFinalizer/no-op_remove_finalizer (0.00s)
    --- FAIL: TestPatchFinalizer/remove_finalizer (0.00s)
        service_controller_test.go:1136: Service hasFinalizer = true, want false
    --- PASS: TestPatchFinalizer/no-op_add_finalizer (0.00s)
    --- PASS: TestPatchFinalizer/add_finalizer (0.00s)

We are currently calling utils in https://github.com/kubernetes/kubernetes/blob/master/pkg/controller/service/patch.go. It pretty much follows what the other controller uses (e.g. pod, node).

The util codes use strategicpatch to create a two way merge patch. And I confirmed the generated patch contains merge directive.

{"metadata":{"$deleteFromPrimitiveList/finalizers":["service.kubernetes.io/load-balancer-cleanup"],"$setElementOrder/finalizers":["bar"],"finalizers":["bar"]}}

From the fake implementation I saw it is merely calling StrategicMergePatch. Hence I was baffled why this doesn’t work: https://github.com/kubernetes/client-go/blob/f0c6576981f26757c12f4757060d2a036951b67c/testing/fixture.go#L164-L171

Would appreciate if anyone can help take a look. Thanks!

Ref https://github.com/kubernetes/kubernetes/pull/78262#issuecomment-495802117.

cc @mengqiy

About this issue

  • Original URL
  • State: closed
  • Created 5 years ago
  • Comments: 22 (12 by maintainers)

Most upvoted comments

After debugging the code a bit, it seems there is a bug in the fake client 😕

https://github.com/kubernetes/client-go/blob/f0c6576981f26757c12f4757060d2a036951b67c/testing/fixture.go#L132 does the decoding and obj has the finalizer.

https://github.com/kubernetes/client-go/blob/f0c6576981f26757c12f4757060d2a036951b67c/testing/fixture.go#L165 actually merge the patch correctly.

The bug is in https://github.com/kubernetes/client-go/blob/f0c6576981f26757c12f4757060d2a036951b67c/testing/fixture.go#L169 When you decode mergedByte into the non-empty obj, finalizer stay around.

I guess the fix is to use an empty obj for decoding.