Merge pull request #3126 from sbueringer/pr-improve-tests

 🐛 Fix unit tests, improve debuggability

controllers/controllers_suite_test.go

@@ -53,6 +53,7 @@ func TestControllers(t *testing.T) {
 
 var (
 	testEnv *helpers.TestEnvironment
+	tracker *remote.ClusterCacheTracker
 	ctx     = ctrl.SetupSignalHandler()
 )
 
@@ -80,7 +81,7 @@ func setup() {
 		panic("unable to create secret caching client")
 	}
 
-	tracker, err := remote.NewClusterCacheTracker(
+	tracker, err = remote.NewClusterCacheTracker(
 		testEnv.Manager,
 		remote.ClusterCacheTrackerOptions{
 			SecretCachingClient: secretCachingClient,

controllers/serviceaccount_controller.go

@@ -20,6 +20,7 @@ import (
 	"context"
 	"fmt"
 	"reflect"
+	"strings"
 	"time"
 
 	"github.com/pkg/errors"
@@ -242,6 +243,12 @@ func (r *ServiceAccountReconciler) reconcileNormal(ctx context.Context, guestClu
 func (r *ServiceAccountReconciler) ensureProviderServiceAccounts(ctx context.Context, guestClusterCtx *vmwarecontext.GuestClusterContext, pSvcAccounts []vmwarev1.ProviderServiceAccount) error {
 	log := ctrl.LoggerFrom(ctx)
 
+	pSvcAccountNames := []string{}
+	for _, pSvcAccount := range pSvcAccounts {
+		pSvcAccountNames = append(pSvcAccountNames, pSvcAccount.Name)
+	}
+	log.V(5).Info(fmt.Sprintf("Reconcile ProviderServiceAccounts: %v", strings.Join(pSvcAccountNames, ",")))
+
 	for i, pSvcAccount := range pSvcAccounts {
 		// Note: We have to use := here to not overwrite log & ctx outside the for loop.
 		log := log.WithValues("ProviderServiceAccount", klog.KRef(pSvcAccount.Namespace, pSvcAccount.Name))

controllers/serviceaccount_controller_intg_test.go

@@ -59,14 +59,21 @@ var _ = Describe("ProviderServiceAccount controller integration tests", func() {
 				helpers.CreateAndWait(ctx, intCtx.Client, intCtx.KubeconfigSecret)
 			})
 
+			By("Verifying that the guest cluster client works")
+			guestClient, err := tracker.GetClient(ctx, client.ObjectKeyFromObject(intCtx.Cluster))
+			Expect(err).ToNot(HaveOccurred())
+			// Note: Create a Service informer, so the test later doesn't fail if this doesn't work.
+			Expect(guestClient.List(ctx, &corev1.ServiceList{}, client.InNamespace(metav1.NamespaceDefault))).To(Succeed())
+
 			pSvcAccount = getTestProviderServiceAccount(intCtx.Namespace, intCtx.VSphereCluster)
 			createTestResource(ctx, intCtx.Client, pSvcAccount)
 			assertEventuallyExistsInNamespace(ctx, intCtx.Client, intCtx.Namespace, pSvcAccount.GetName(), pSvcAccount)
 		})
 		AfterEach(func() {
-			// Deleting the provider service account is not strictly required as the context itself
-			// gets teared down but keeping it for clarity.
 			deleteTestResource(ctx, intCtx.Client, pSvcAccount)
+			deleteTestResource(ctx, intCtx.Client, intCtx.VSphereCluster)
+			deleteTestResource(ctx, intCtx.Client, intCtx.Cluster)
+			deleteTestResource(ctx, intCtx.Client, intCtx.KubeconfigSecret)
 		})
 
 		Context("When serviceaccount secret is created", func() {

controllers/servicediscovery_controller_intg_test.go

@@ -20,6 +20,7 @@ import (
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	helpers "sigs.k8s.io/cluster-api-provider-vsphere/internal/test/helpers/vmware"
@@ -37,8 +38,17 @@ var _ = Describe("Service Discovery controller integration tests", func() {
 			helpers.CreateAndWait(ctx, intCtx.Client, intCtx.VSphereCluster)
 			helpers.CreateAndWait(ctx, intCtx.Client, intCtx.KubeconfigSecret)
 		})
+
+		By("Verifying that the guest cluster client works")
+		guestClient, err := tracker.GetClient(ctx, client.ObjectKeyFromObject(intCtx.Cluster))
+		Expect(err).ToNot(HaveOccurred())
+		// Note: Create a Service informer, so the test later doesn't fail if this doesn't work.
+		Expect(guestClient.List(ctx, &corev1.ServiceList{}, client.InNamespace(metav1.NamespaceDefault))).To(Succeed())
 	})
 	AfterEach(func() {
+		deleteTestResource(ctx, intCtx.Client, intCtx.VSphereCluster)
+		deleteTestResource(ctx, intCtx.Client, intCtx.Cluster)
+		deleteTestResource(ctx, intCtx.Client, intCtx.KubeconfigSecret)
 		intCtx.AfterEach()
 	})
 

controllers/vspherecluster_reconciler_test.go

@@ -358,7 +358,11 @@ var _ = Describe("VIM based VSphere ClusterReconciler", func() {
 		})
 
 		AfterEach(func() {
-			Expect(testEnv.CleanupAndWait(ctx, instance, zoneOne, capiCluster, namespace)).To(Succeed())
+			// Note: Make sure VSphereCluster is deleted before the Cluster is deleted.
+			// Otherwise reconcileDelete in VSphereCluster reconciler will fail because the Cluster cannot be found.
+			Expect(testEnv.CleanupAndWait(ctx, instance, zoneOne)).To(Succeed())
+			Expect(testEnv.CleanupAndWait(ctx, capiCluster)).To(Succeed())
+			Expect(testEnv.CleanupAndWait(ctx, namespace)).To(Succeed())
 		})
 
 		It("should reconcile a cluster", func() {

internal/test/helpers/envtest.go

@@ -42,6 +42,8 @@ import (
 	"k8s.io/apimachinery/pkg/util/wait"
 	clientgoscheme "k8s.io/client-go/kubernetes/scheme"
 	"k8s.io/client-go/rest"
+	"k8s.io/component-base/logs"
+	logsv1 "k8s.io/component-base/logs/api/v1"
 	"k8s.io/klog/v2"
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
 	"sigs.k8s.io/cluster-api/util/kubeconfig"
@@ -60,7 +62,14 @@ import (
 )
 
 func init() {
+	// Set log level 5 as default for testing (default for prod is 2).
+	logOptions := logs.NewOptions()
+	logOptions.Verbosity = 5
+	if err := logsv1.ValidateAndApply(logOptions, nil); err != nil {
+		panic(err)
+	}
 	ctrl.SetLogger(klog.Background())
+
 	// add logger for ginkgo
 	klog.SetOutput(ginkgo.GinkgoWriter)
 }