From dea9ca1da6a544b57b2308a3873db41eae11223b 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 8714198b0c51..a75382124e81 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 fafc48b07d7a..130951fe2a76 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 dd815df95dcd..433c7720e308 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())