Skip to content

Commit

Permalink
Adapt Patch() expectations of MockClient
Browse files Browse the repository at this point in the history 
Due to kubernetes-sigs/controller-runtime#1413,
naive gomock expectations don't work anymore. The reason is that the
`mergeFromPatch` struct has now a function attribute (`createPatch`),
and as `gomega` is using `reflect.DeepEqual` under the hood, the
comparison now fails:

> Func values are deeply equal if both are nil; otherwise they are not deeply equal.
(from https://golang.org/pkg/reflect/#DeepEqual)

Hence, we introduce our own `test.EXPECTPatch()` function.

Thanks to @timuthy for pointing this out.
  • Loading branch information
@rfranzke
rfranzke committed Mar 12, 2021
1 parent 6072fee commit ef3ca6c
Show file tree
Hide file tree
Showing 13 changed files with 70 additions and 40 deletions.
9 changes: 7 additions & 2 deletions extensions/pkg/controller/utils_test.go
View file
Expand Up @@ -23,6 +23,7 @@ import (
v1beta1constants "github.com/gardener/gardener/pkg/apis/core/v1beta1/constants"
extensionsv1alpha1 "github.com/gardener/gardener/pkg/apis/extensions/v1alpha1"
mockclient "github.com/gardener/gardener/pkg/mock/controller-runtime/client"
"github.com/gardener/gardener/pkg/utils/test"

"github.com/golang/mock/gomock"
. "github.com/onsi/ginkgo"
Expand Down Expand Up @@ -148,9 +149,11 @@ var _ = Describe("Utils", func() {
Describe("#RemoveAnnotation", func() {
It("should delete specific annotation", func() {
annotation := "test-delete-annotation-key"

annotations := make(map[string]string)
annotations[annotation] = "test-delete-annotation-value"
annotations["test-no-delete-annotation-key"] = "test-no-delete-annotation-value"

worker := &extensionsv1alpha1.Worker{
TypeMeta: metav1.TypeMeta{
Kind: "Worker",
Expand All @@ -163,9 +166,11 @@ var _ = Describe("Utils", func() {
},
}
workerWithAnnotation := worker.DeepCopyObject()
ctx := context.TODO()
expectedWorker := worker.DeepCopy()
delete(expectedWorker.Annotations, annotation)

c.EXPECT().Patch(ctx, worker, client.MergeFrom(workerWithAnnotation))
ctx := context.TODO()
test.EXPECTPatch(ctx, c, expectedWorker, workerWithAnnotation)

Expect(controller.RemoveAnnotation(ctx, c, worker, annotation)).To(Succeed())
Expect(len(worker.Annotations)).To(Equal(1))
Expand Down
2 changes: 1 addition & 1 deletion pkg/operation/botanist/component/extensions/backupentry/backupentry_test.go
View file
Expand Up @@ -242,7 +242,7 @@ var _ = Describe("#BackupEntry", func() {

mc := mockclient.NewMockClient(ctrl)
mc.EXPECT().Get(ctx, kutil.Key(name), gomock.AssignableToTypeOf(&extensionsv1alpha1.BackupEntry{})).SetArg(2, *expected)
mc.EXPECT().Patch(ctx, expectedCopy, gomock.AssignableToTypeOf(client.MergeFrom(expected)))
test.EXPECTPatch(ctx, mc, expectedCopy, expected)

defaultDepWaiter = backupentry.New(log, mc, values, time.Millisecond, 250*time.Millisecond, 500*time.Millisecond)
Expect(defaultDepWaiter.Migrate(ctx)).To(Succeed())
Expand Down
6 changes: 3 additions & 3 deletions pkg/operation/botanist/component/extensions/containerruntime/containerruntime_test.go
View file
Expand Up @@ -282,7 +282,7 @@ var _ = Describe("#ContainerRuntimee", func() {
extensions = append(extensions, gardencorev1alpha1.ExtensionResourceState{
Name: &extensionName,
Kind: extensionsv1alpha1.ContainerRuntimeResource,
State: &runtime.RawExtension{Raw: []byte("dummy state")},
State: &runtime.RawExtension{Raw: []byte(`{"dummy":"state"}`)},
})
}
}
Expand Down Expand Up @@ -317,7 +317,7 @@ var _ = Describe("#ContainerRuntimee", func() {
expected[0].Annotations[v1beta1constants.GardenerOperation] = v1beta1constants.GardenerOperationWaitForState
expectedWithState := expected[0].DeepCopy()
expectedWithState.Status = extensionsv1alpha1.ContainerRuntimeStatus{
DefaultStatus: extensionsv1alpha1.DefaultStatus{State: &runtime.RawExtension{Raw: []byte("dummy state")}},
DefaultStatus: extensionsv1alpha1.DefaultStatus{State: &runtime.RawExtension{Raw: []byte(`{"dummy":"state"}`)}},
}
expectedWithRestore := expectedWithState.DeepCopy()
expectedWithRestore.Annotations[v1beta1constants.GardenerOperation] = v1beta1constants.GardenerOperationRestore
Expand All @@ -331,7 +331,7 @@ var _ = Describe("#ContainerRuntimee", func() {
return mc
})
mc.EXPECT().Update(ctx, expectedWithState).Return(nil)
mc.EXPECT().Patch(ctx, expectedWithRestore, client.MergeFrom(expectedWithState))
test.EXPECTPatch(ctx, mc, expectedWithRestore, expectedWithState)

defaultDepWaiter = containerruntime.New(
log,
Expand Down
6 changes: 3 additions & 3 deletions pkg/operation/botanist/component/extensions/controlplane/controlplane_test.go
View file
Expand Up @@ -298,7 +298,7 @@ var _ = Describe("ControlPlane", func() {

Describe("#Restore", func() {
var (
state = &runtime.RawExtension{Raw: []byte("dummy state")}
state = &runtime.RawExtension{Raw: []byte(`{"dummy":"state"}`)}
shootState *gardencorev1alpha1.ShootState
)

Expand Down Expand Up @@ -338,7 +338,7 @@ var _ = Describe("ControlPlane", func() {
mc.EXPECT().Create(ctx, obj)
mc.EXPECT().Status().Return(mc)
mc.EXPECT().Update(ctx, expectedWithState)
mc.EXPECT().Patch(ctx, expectedWithRestore, client.MergeFrom(expectedWithState))
test.EXPECTPatch(ctx, mc, expectedWithRestore, expectedWithState)

Expect(controlplane.New(log, mc, values, time.Millisecond, 250*time.Millisecond, 500*time.Millisecond).Restore(ctx, shootState)).To(Succeed())
})
Expand Down Expand Up @@ -369,7 +369,7 @@ var _ = Describe("ControlPlane", func() {
mc.EXPECT().Create(ctx, obj)
mc.EXPECT().Status().Return(mc)
mc.EXPECT().Update(ctx, expectedWithState)
mc.EXPECT().Patch(ctx, expectedWithRestore, client.MergeFrom(expectedWithState))
test.EXPECTPatch(ctx, mc, expectedWithRestore, expectedWithState)

defaultDepWaiter = controlplane.New(log, mc, values, time.Millisecond, 250*time.Millisecond, 500*time.Millisecond)
Expect(defaultDepWaiter.Restore(ctx, shootState)).To(Succeed())
Expand Down
4 changes: 2 additions & 2 deletions pkg/operation/botanist/component/extensions/extension/extension_test.go
View file
Expand Up @@ -231,7 +231,7 @@ var _ = Describe("Extension", func() {

Describe("#Restore", func() {
var (
state = []byte("dummy state")
state = []byte(`{"dummy":"state"}`)
shootState *gardencorev1alpha1.ShootState
)

Expand Down Expand Up @@ -275,7 +275,7 @@ var _ = Describe("Extension", func() {
return mc
})
mc.EXPECT().Update(ctx, expectedWithState).Return(nil)
mc.EXPECT().Patch(ctx, expectedWithRestore, client.MergeFrom(expectedWithState))
test.EXPECTPatch(ctx, mc, expectedWithRestore, expectedWithState)

defaultDepWaiter = extension.New(
log,
Expand Down
11 changes: 5 additions & 6 deletions pkg/operation/botanist/component/extensions/infrastructure/infrastructure_test.go
View file
Expand Up @@ -88,7 +88,7 @@ var _ = Describe("#Interface", func() {
region = "europe"
sshPublicKey = []byte("secure")
providerConfig = &runtime.RawExtension{
Raw: []byte("very-provider-specific"),
Raw: []byte(`{"very":"provider-specific"}`),
}

values = &infrastructure.Values{
Expand Down Expand Up @@ -440,7 +440,7 @@ var _ = Describe("#Interface", func() {

Describe("#Restore", func() {
var (
state = &runtime.RawExtension{Raw: []byte("dummy state")}
state = &runtime.RawExtension{Raw: []byte(`{"dummy":"state"}`)}
shootState = &gardencorev1alpha1.ShootState{
Spec: gardencorev1alpha1.ShootStateSpec{
Extensions: []gardencorev1alpha1.ExtensionResourceState{
Expand Down Expand Up @@ -478,7 +478,7 @@ var _ = Describe("#Interface", func() {
mc.EXPECT().Create(ctx, obj)
mc.EXPECT().Status().Return(mc)
mc.EXPECT().Update(ctx, expectedWithState)
mc.EXPECT().Patch(ctx, expectedWithRestore, client.MergeFrom(expectedWithState))
test.EXPECTPatch(ctx, mc, expectedWithRestore, expectedWithState)

Expect(infrastructure.New(log, mc, values, time.Millisecond, 250*time.Millisecond, 500*time.Millisecond).Restore(ctx, shootState)).To(Succeed())
})
Expand All @@ -505,9 +505,8 @@ var _ = Describe("#Interface", func() {
infra.DeepCopyInto(o)
return nil
}),
c.
EXPECT().
Patch(ctx, obj, client.MergeFrom(infra)),
test.
EXPECTPatch(ctx, c, obj, infra),
)

Expect(deployWaiter.Migrate(ctx)).To(Succeed())
Expand Down
18 changes: 9 additions & 9 deletions pkg/operation/botanist/component/extensions/network/network_test.go
View file
Expand Up @@ -122,7 +122,7 @@ var _ = Describe("#Network", func() {
},
}

defaultDepWaiter = network.New(log, c, values, time.Second, 2*time.Second, 3*time.Second)
defaultDepWaiter = network.New(log, c, values, time.Millisecond, 250*time.Millisecond, 500*time.Millisecond)
})

AfterEach(func() {
Expand Down Expand Up @@ -217,7 +217,7 @@ var _ = Describe("#Network", func() {
defaultDepWaiter = network.New(log, mc, &network.Values{
Namespace: networkNs,
Name: networkName,
}, time.Second, 2*time.Second, 3*time.Second)
}, time.Millisecond, 250*time.Millisecond, 500*time.Millisecond)

err := defaultDepWaiter.Destroy(ctx)
Expect(err).To(HaveOccurred())
Expand All @@ -242,7 +242,7 @@ var _ = Describe("#Network", func() {
{
Name: &expected.Name,
Kind: extensionsv1alpha1.NetworkResource,
State: &runtime.RawExtension{Raw: []byte("dummy state")},
State: &runtime.RawExtension{Raw: []byte(`{"dummy":"state"}`)},
},
},
},
Expand All @@ -261,7 +261,7 @@ var _ = Describe("#Network", func() {

expectedWithState := expected.DeepCopy()
expectedWithState.Status = extensionsv1alpha1.NetworkStatus{
DefaultStatus: extensionsv1alpha1.DefaultStatus{State: &runtime.RawExtension{Raw: []byte("dummy state")}},
DefaultStatus: extensionsv1alpha1.DefaultStatus{State: &runtime.RawExtension{Raw: []byte(`{"dummy":"state"}`)}},
}

expectedWithRestore := expectedWithState.DeepCopy()
Expand All @@ -278,10 +278,10 @@ var _ = Describe("#Network", func() {
return mc
}),
mc.EXPECT().Update(ctx, expectedWithState).Return(nil),
mc.EXPECT().Patch(ctx, expectedWithRestore, client.MergeFrom(expectedWithState)),
test.EXPECTPatch(ctx, mc, expectedWithRestore, expectedWithState),
)

defaultDepWaiter = network.New(log, mc, values, time.Second, 2*time.Second, 3*time.Second)
defaultDepWaiter = network.New(log, mc, values, time.Millisecond, 250*time.Millisecond, 500*time.Millisecond)

Expect(defaultDepWaiter.Restore(ctx, shootState)).To(Succeed())
})
Expand All @@ -302,9 +302,9 @@ var _ = Describe("#Network", func() {
mc := mockclient.NewMockClient(ctrl)

mc.EXPECT().Get(ctx, kutil.Key(networkNs, networkName), gomock.AssignableToTypeOf(&extensionsv1alpha1.Network{})).SetArg(2, *expected)
mc.EXPECT().Patch(ctx, expectedCopy, gomock.AssignableToTypeOf(client.MergeFrom(expected)))
test.EXPECTPatch(ctx, mc, expectedCopy, expected)

defaultDepWaiter = network.New(log, mc, values, time.Second, 2*time.Second, 3*time.Second)
defaultDepWaiter = network.New(log, mc, values, time.Millisecond, 250*time.Millisecond, 500*time.Millisecond)
Expect(defaultDepWaiter.Migrate(ctx)).To(Succeed())
})

Expand All @@ -314,7 +314,7 @@ var _ = Describe("#Network", func() {
apierrors.NewNotFound(extensionsv1alpha1.Resource("Network"), expected.Name),
)

defaultDepWaiter = network.New(log, mc, values, time.Second, 2*time.Second, 3*time.Second)
defaultDepWaiter = network.New(log, mc, values, time.Millisecond, 250*time.Millisecond, 500*time.Millisecond)
Expect(defaultDepWaiter.Migrate(ctx)).To(Succeed())
})
})
Expand Down
6 changes: 3 additions & 3 deletions ...eration/botanist/component/extensions/operatingsystemconfig/operatingsystemconfig_test.go
View file
Expand Up @@ -298,8 +298,8 @@ var _ = Describe("OperatingSystemConfig", func() {

Describe("#Restore", func() {
var (
stateDownloader = []byte("dummy state downloader")
stateOriginal = []byte("dummy state original")
stateDownloader = []byte(`{"dummy":"state downloader"}`)
stateOriginal = []byte(`{"dummy":"state original"}`)
shootState *gardencorev1alpha1.ShootState
)

Expand Down Expand Up @@ -359,7 +359,7 @@ var _ = Describe("OperatingSystemConfig", func() {
mc.EXPECT().Create(ctx, expected[i])
mc.EXPECT().Status().Return(mc)
mc.EXPECT().Update(ctx, expectedWithState)
mc.EXPECT().Patch(ctx, expectedWithRestore, client.MergeFrom(expectedWithState))
test.EXPECTPatch(ctx, mc, expectedWithRestore, expectedWithState)
}

defaultDepWaiter = New(log, mc, values, time.Millisecond, 250*time.Millisecond, 500*time.Millisecond)
Expand Down
4 changes: 2 additions & 2 deletions pkg/operation/botanist/component/extensions/worker/worker_test.go
View file
Expand Up @@ -427,7 +427,7 @@ var _ = Describe("Worker", func() {

Describe("#Restore", func() {
var (
state = &runtime.RawExtension{Raw: []byte("dummy state")}
state = &runtime.RawExtension{Raw: []byte(`{"dummy":"state"}`)}
shootState = &gardencorev1alpha1.ShootState{
Spec: gardencorev1alpha1.ShootStateSpec{
Extensions: []gardencorev1alpha1.ExtensionResourceState{
Expand Down Expand Up @@ -462,7 +462,7 @@ var _ = Describe("Worker", func() {
mc.EXPECT().Create(ctx, obj)
mc.EXPECT().Status().Return(mc)
mc.EXPECT().Update(ctx, expectedWithState)
mc.EXPECT().Patch(ctx, expectedWithRestore, client.MergeFrom(expectedWithState))
test.EXPECTPatch(ctx, mc, expectedWithRestore, expectedWithState)

Expect(worker.New(log, mc, values, time.Millisecond, 250*time.Millisecond, 500*time.Millisecond).Restore(ctx, shootState)).To(Succeed())
})
Expand Down
14 changes: 7 additions & 7 deletions pkg/operation/operation_test.go
View file
Expand Up @@ -21,23 +21,23 @@ import (
gardencorev1alpha1helper "github.com/gardener/gardener/pkg/apis/core/v1alpha1/helper"
gardencorev1beta1 "github.com/gardener/gardener/pkg/apis/core/v1beta1"
fakeclientset "github.com/gardener/gardener/pkg/client/kubernetes/fake"
mock "github.com/gardener/gardener/pkg/client/kubernetes/mock"
"github.com/gardener/gardener/pkg/client/kubernetes/mock"
mockclient "github.com/gardener/gardener/pkg/mock/controller-runtime/client"
. "github.com/gardener/gardener/pkg/operation"
kutil "github.com/gardener/gardener/pkg/utils/kubernetes"
"github.com/golang/mock/gomock"
"sigs.k8s.io/controller-runtime/pkg/client"

operationseed "github.com/gardener/gardener/pkg/operation/seed"
operationshoot "github.com/gardener/gardener/pkg/operation/shoot"
kutil "github.com/gardener/gardener/pkg/utils/kubernetes"
"github.com/gardener/gardener/pkg/utils/test"

"github.com/golang/mock/gomock"
. "github.com/onsi/ginkgo"
. "github.com/onsi/ginkgo/extensions/table"
. "github.com/onsi/gomega"
"github.com/onsi/gomega/types"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
)

var _ = Describe("operation", func() {
Expand Down Expand Up @@ -169,13 +169,13 @@ var _ = Describe("operation", func() {
{
Name: "test",
Type: "test",
Data: runtime.RawExtension{Raw: []byte{}},
Data: runtime.RawExtension{Raw: []byte(`{}`)},
},
}

shootState := o.ShootState.DeepCopy()
shootState.Spec.Gardener = gardenerResourceList
k8sGardenRuntimeClient.EXPECT().Patch(context.TODO(), shootState, client.MergeFrom(o.ShootState))
test.EXPECTPatch(context.TODO(), k8sGardenRuntimeClient, shootState, o.ShootState)

Expect(o.SaveGardenerResourcesInShootState(context.TODO(), gardenerResourceList)).To(Succeed())
Expect(o.ShootState.Spec.Gardener).To(BeEquivalentTo(gardenerResourceList))
Expand Down
3 changes: 3 additions & 0 deletions pkg/utils/kubernetes/client/client.go
View file
Expand Up @@ -20,6 +20,8 @@ import (
"fmt"
"time"

"github.com/onsi/ginkgo"

"github.com/gardener/gardener/pkg/client/kubernetes"
"github.com/gardener/gardener/pkg/utils/flow"
utiltime "github.com/gardener/gardener/pkg/utils/time"
Expand Down Expand Up @@ -86,6 +88,7 @@ var defaultFinalizer = NewFinalizer()

// Finalize removes the finalizers (.meta.finalizers) of given resource.
func (f *finalizer) Finalize(ctx context.Context, c client.Client, obj client.Object) error {
defer ginkgo.GinkgoRecover()
withFinalizers := obj.DeepCopyObject()
obj.SetFinalizers(nil)
return c.Patch(ctx, obj, client.MergeFrom(withFinalizers))
Expand Down
5 changes: 3 additions & 2 deletions pkg/utils/kubernetes/client/client_test.go
View file
Expand Up @@ -24,6 +24,7 @@ import (
kutil "github.com/gardener/gardener/pkg/utils/kubernetes"
. "github.com/gardener/gardener/pkg/utils/kubernetes/client"
mockutilclient "github.com/gardener/gardener/pkg/utils/kubernetes/client/mock"
"github.com/gardener/gardener/pkg/utils/test"
mocktime "github.com/gardener/gardener/pkg/utils/time/mock"

"github.com/golang/mock/gomock"
Expand Down Expand Up @@ -132,7 +133,7 @@ var _ = Describe("Cleaner", func() {
gomock.InOrder(
c.EXPECT().Get(ctx, cm2Key, &cm2).SetArg(2, cm2WithFinalizer),
timeOps.EXPECT().Now().Return(now),
c.EXPECT().Patch(ctx, &cm2, client.MergeFrom(&cm2WithFinalizer)),
test.EXPECTPatch(ctx, c, &cm2, &cm2WithFinalizer),
)

Expect(cleaner.Clean(ctx, c, &cm2, FinalizeGracePeriodSeconds(20))).To(Succeed())
Expand Down Expand Up @@ -213,7 +214,7 @@ var _ = Describe("Cleaner", func() {
gomock.InOrder(
c.EXPECT().List(ctx, list).SetArg(1, corev1.ConfigMapList{Items: []corev1.ConfigMap{cm2WithFinalizer}}),
timeOps.EXPECT().Now().Return(now),
c.EXPECT().Patch(ctx, &cm2, client.MergeFrom(&cm2WithFinalizer)),
test.EXPECTPatch(ctx, c, &cm2, &cm2WithFinalizer),
)

Expect(cleaner.Clean(ctx, c, list, FinalizeGracePeriodSeconds(20))).To(Succeed())
Expand Down
22 changes: 22 additions & 0 deletions pkg/utils/test/test.go
View file
Expand Up @@ -15,13 +15,20 @@
package test

import (
"context"
"fmt"
"io/ioutil"
"os"
"reflect"

"github.com/golang/mock/gomock"
"github.com/onsi/ginkgo"
. "github.com/onsi/gomega"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/component-base/featuregate"
"sigs.k8s.io/controller-runtime/pkg/client"

mockclient "github.com/gardener/gardener/pkg/mock/controller-runtime/client"
)

// WithVar sets the given var to the src value and returns a function to revert to the original state.
Expand Down Expand Up @@ -173,3 +180,18 @@ func WithTempFile(dir, pattern string, content []byte, fileName *string) func()
}
}
}

// EXPECTPatch is a helper function for a GoMock call expecting a patch with the mock client.
func EXPECTPatch(ctx context.Context, c *mockclient.MockClient, obj, mergeFrom runtime.Object) *gomock.Call {
expectedPatch := client.MergeFrom(mergeFrom)
expectedData, expectedErr := expectedPatch.Data(obj)
Expect(expectedErr).To(BeNil())

return c.EXPECT().Patch(ctx, obj, gomock.Any()).DoAndReturn(func(_ context.Context, _ client.Object, patch client.Patch, _ ...client.PatchOption) error {
data, err := patch.Data(obj)
Expect(err).To(BeNil())
Expect(patch.Type()).To(Equal(expectedPatch.Type()))
Expect(data).To(Equal(expectedData))
return nil
})
}

0 comments on commit ef3ca6c

Please sign in to comment.