From e18b0161ecb2d4a2f1fb1b8e74d2e397ceb92941 Mon Sep 17 00:00:00 2001 From: Tim Ebert Date: Tue, 12 Oct 2021 10:06:27 +0200 Subject: [PATCH] Correct unit tests falsely succeeding These tests were not preparing the test objects correctly: they only updated them in memory but not on the fake client. This wasn't caught until now because the fake client mimicked the real json decoder, which didn't unset fields not present on the server. Now that the fake client zeroes fields, the tests started failing (which is correct). So fix the tests. ref kubernetes-sigs/controller-runtime#1651 --- .../botanist/clusteridentity_test.go | 45 ++++++++++--------- .../kube_apiserver_service_test.go | 6 +-- .../gardener/deletion_confirmation_test.go | 4 +- 3 files changed, 31 insertions(+), 24 deletions(-) diff --git a/pkg/operation/botanist/clusteridentity_test.go b/pkg/operation/botanist/clusteridentity_test.go index 8714198b0c5..a75382124e8 100644 --- a/pkg/operation/botanist/clusteridentity_test.go +++ b/pkg/operation/botanist/clusteridentity_test.go @@ -30,7 +30,6 @@ import ( "github.com/golang/mock/gomock" . "github.com/onsi/ginkgo" - . "github.com/onsi/ginkgo/extensions/table" . "github.com/onsi/gomega" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -73,11 +72,6 @@ var _ = Describe("ClusterIdentity", func() { ctrl = gomock.NewController(GinkgoT()) clusterIdentity = mockclusteridentity.NewMockInterface(ctrl) - s := runtime.NewScheme() - Expect(corev1.AddToScheme(s)).To(Succeed()) - Expect(extensionsv1alpha1.AddToScheme(s)).NotTo(HaveOccurred()) - Expect(gardencorev1beta1.AddToScheme(s)) - shoot = &gardencorev1beta1.Shoot{ ObjectMeta: metav1.ObjectMeta{ Name: shootName, @@ -87,6 +81,13 @@ var _ = Describe("ClusterIdentity", func() { UID: shootUID, }, } + }) + + JustBeforeEach(func() { + s := runtime.NewScheme() + Expect(corev1.AddToScheme(s)).To(Succeed()) + Expect(extensionsv1alpha1.AddToScheme(s)).NotTo(HaveOccurred()) + Expect(gardencorev1beta1.AddToScheme(s)) cluster := &extensionsv1alpha1.Cluster{ ObjectMeta: metav1.ObjectMeta{ @@ -125,27 +126,31 @@ var _ = Describe("ClusterIdentity", func() { ctrl.Finish() }) - DescribeTable("#EnsureShootClusterIdentity", - func(mutator func()) { - mutator() - + Describe("#EnsureShootClusterIdentity", func() { + test := func() { Expect(botanist.EnsureShootClusterIdentity(ctx)).NotTo(HaveOccurred()) Expect(gardenClient.Get(ctx, kutil.Key(shootNamespace, shootName), shoot)).To(Succeed()) Expect(shoot.Status.ClusterIdentity).NotTo(BeNil()) Expect(*shoot.Status.ClusterIdentity).To(Equal(expectedShootClusterIdentity)) - }, - - Entry("cluster identity is nil", func() { - shoot.Status.ClusterIdentity = nil - }), - Entry("cluster idenitty already exists", func() { - shoot.Status.ClusterIdentity = pointer.String(expectedShootClusterIdentity) - }), - ) + } + + Context("cluster identity is nil", func() { + BeforeEach(func() { + shoot.Status.ClusterIdentity = nil + }) + It("should set shoot.status.clusterIdentity", test) + }) + Context("cluster identity already exists", func() { + BeforeEach(func() { + shoot.Status.ClusterIdentity = pointer.String(expectedShootClusterIdentity) + }) + It("should not touch shoot.status.clusterIdentity", test) + }) + }) Describe("#DeployClusterIdentity", func() { - BeforeEach(func() { + JustBeforeEach(func() { botanist.Shoot.GetInfo().Status.ClusterIdentity = &expectedShootClusterIdentity clusterIdentity.EXPECT().SetIdentity(expectedShootClusterIdentity) }) diff --git a/pkg/operation/botanist/component/kubeapiserverexposure/kube_apiserver_service_test.go b/pkg/operation/botanist/component/kubeapiserverexposure/kube_apiserver_service_test.go index fafc48b07d7..130951fe2a7 100644 --- a/pkg/operation/botanist/component/kubeapiserverexposure/kube_apiserver_service_test.go +++ b/pkg/operation/botanist/component/kubeapiserverexposure/kube_apiserver_service_test.go @@ -146,14 +146,14 @@ var _ = Describe("#Service", func() { It("waits for service", func() { Expect(defaultDepWaiter.Deploy(ctx)).To(Succeed()) + key := client.ObjectKeyFromObject(expected) + Expect(c.Get(ctx, key, expected)).To(Succeed()) + expected.Status = corev1.ServiceStatus{ LoadBalancer: corev1.LoadBalancerStatus{ Ingress: []corev1.LoadBalancerIngress{{IP: "3.3.3.3"}}, }, } - - key := client.ObjectKeyFromObject(expected) - Expect(c.Get(ctx, key, expected)).To(Succeed()) Expect(c.Status().Update(ctx, expected)).To(Succeed()) Expect(defaultDepWaiter.Wait(ctx)).To(Succeed()) Expect(ingressIP).To(Equal("3.3.3.3")) diff --git a/pkg/utils/gardener/deletion_confirmation_test.go b/pkg/utils/gardener/deletion_confirmation_test.go index dd815df95dc..433c7720e30 100644 --- a/pkg/utils/gardener/deletion_confirmation_test.go +++ b/pkg/utils/gardener/deletion_confirmation_test.go @@ -23,7 +23,6 @@ import ( . "github.com/gardener/gardener/pkg/utils/gardener" "github.com/gardener/gardener/pkg/utils/test" . "github.com/gardener/gardener/pkg/utils/test/matchers" - "sigs.k8s.io/controller-runtime/pkg/client/fake" "github.com/golang/mock/gomock" . "github.com/onsi/ginkgo" @@ -31,6 +30,7 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" ) var _ = Describe("DeletionConfirmation", func() { @@ -109,6 +109,8 @@ var _ = Describe("DeletionConfirmation", func() { mockNow.EXPECT().Do().Return(now.UTC()).AnyTimes() obj.SetAnnotations(map[string]string{"foo": "bar"}) + Expect(c.Update(ctx, obj)).To(Succeed()) + expectedAnnotations := map[string]string{"foo": "bar", ConfirmationDeletion: "true", v1beta1constants.GardenerTimestamp: now.UTC().String()} Expect(ConfirmDeletion(ctx, c, obj)).To(Succeed())