diff --git a/controllers/serviceaccount_controller.go b/controllers/serviceaccount_controller.go index d73dacf7e7..98394233f6 100644 --- a/controllers/serviceaccount_controller.go +++ b/controllers/serviceaccount_controller.go @@ -22,6 +22,7 @@ import ( "os" "reflect" "strconv" + "strings" "github.com/pkg/errors" corev1 "k8s.io/api/core/v1" @@ -31,6 +32,7 @@ import ( "k8s.io/apimachinery/pkg/types" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" "sigs.k8s.io/cluster-api/controllers/remote" + clusterutilv1 "sigs.k8s.io/cluster-api/util" "sigs.k8s.io/cluster-api/util/conditions" "sigs.k8s.io/cluster-api/util/patch" ctrl "sigs.k8s.io/controller-runtime" @@ -55,16 +57,19 @@ import ( // +kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=roles;rolebindings,verbs=get;list;watch;create;update;patch;delete const ( - controllerName = "provider-serviceaccount-controller" - kindProviderServiceAccount = "ProviderServiceAccount" - systemServiceAccountPrefix = "system.serviceaccount" + // ProviderServiceAccountControllerName defines the controller used when creating clients. + ProviderServiceAccountControllerName = "provider-serviceaccount-controller" + kindProviderServiceAccount = "ProviderServiceAccount" + systemServiceAccountPrefix = "system.serviceaccount" ) // AddServiceAccountProviderControllerToManager adds this controller to the provided manager. func AddServiceAccountProviderControllerToManager(ctx *context.ControllerManagerContext, mgr manager.Manager) error { var ( - controlledType = &vmwarev1.ProviderServiceAccount{} - controllerNameShort = reflect.TypeOf(controlledType).Elem().Name() + controlledType = &vmwarev1.ProviderServiceAccount{} + controlledTypeName = reflect.TypeOf(controlledType).Elem().Name() + + controllerNameShort = fmt.Sprintf("%s-controller", strings.ToLower(controlledTypeName)) controllerNameLong = fmt.Sprintf("%s/%s/%s", ctx.Namespace, ctx.Name, controllerNameShort) ) @@ -75,56 +80,31 @@ func AddServiceAccountProviderControllerToManager(ctx *context.ControllerManager Logger: ctx.Logger.WithName(controllerNameShort), } r := ServiceAccountReconciler{ - ControllerContext: controllerContext, + ControllerContext: controllerContext, + remoteClientGetter: remote.NewClusterClient, } return ctrl.NewControllerManagedBy(mgr).For(controlledType). // Watch a ProviderServiceAccount Watches( - &source.Kind{Type: &vmwarev1.ProviderServiceAccount{}}, &handler.EnqueueRequestForObject{}). + &source.Kind{Type: &vmwarev1.ProviderServiceAccount{}}, + handler.EnqueueRequestsFromMapFunc(r.providerServiceAccountToVSphereCluster), + ). Watches( &source.Kind{Type: &corev1.ServiceAccount{}}, - handler.EnqueueRequestsFromMapFunc(requestMapper{ctx}.Map), + handler.EnqueueRequestsFromMapFunc(r.serviceAccountToVSphereCluster), ). Complete(r) } -type requestMapper struct { - ctx *context.ControllerManagerContext -} - -func (d requestMapper) Map(o client.Object) []reconcile.Request { - // If the watched object is owned by this providerserviceaccount controller, then - // lookup the vsphere cluster that owns the providerserviceaccount object that needs to be queued. We do this because - // this controller is effectively a vsphere controller which reconciles it's dependent providerserviceaccounts. - ownerRef := metav1.GetControllerOf(o) - if ownerRef != nil && ownerRef.Kind == kindProviderServiceAccount { - key := types.NamespacedName{Namespace: o.GetNamespace(), Name: ownerRef.Name} - return getVSphereCluster(d.ctx, key) - } - return nil -} - -func getVSphereCluster(ctx *context.ControllerManagerContext, pSvcAccountKey types.NamespacedName) []reconcile.Request { - pSvcAccount := &vmwarev1.ProviderServiceAccount{} - if err := ctx.Client.Get(ctx, pSvcAccountKey, pSvcAccount); err != nil { - return nil - } - - vsphereClusterRef := pSvcAccount.Spec.Ref - if vsphereClusterRef == nil || vsphereClusterRef.Name == "" { - return nil - } - key := client.ObjectKey{Namespace: pSvcAccount.Namespace, Name: vsphereClusterRef.Name} - return []reconcile.Request{{NamespacedName: key}} -} - func NewServiceAccountReconciler() builder.Reconciler { return ServiceAccountReconciler{} } type ServiceAccountReconciler struct { *context.ControllerContext + + remoteClientGetter remote.ClusterClientGetter } func (r ServiceAccountReconciler) Reconcile(ctx goctx.Context, req reconcile.Request) (_ reconcile.Result, reterr error) { @@ -175,11 +155,17 @@ func (r ServiceAccountReconciler) Reconcile(ctx goctx.Context, req reconcile.Req return r.ReconcileDelete(clusterContext) } + cluster, err := clusterutilv1.GetClusterFromMetadata(r, r.Client, vsphereCluster.ObjectMeta) + if err != nil { + r.Logger.Info("unable to get capi cluster from vsphereCluster", "err", err) + return reconcile.Result{}, nil + } + // We cannot proceed until we are able to access the target cluster. Until // then just return a no-op and wait for the next sync. This will occur when // the Cluster's status is updated with a reference to the secret that has // the Kubeconfig data used to access the target cluster. - guestClient, err := remote.NewClusterClient(clusterContext, controllerName, clusterContext.Client, clusterKey) + guestClient, err := r.remoteClientGetter(clusterContext, ProviderServiceAccountControllerName, clusterContext.Client, client.ObjectKeyFromObject(cluster)) if err != nil { clusterContext.Logger.Info("The control plane is not ready yet", "err", err) return reconcile.Result{RequeueAfter: clusterNotReadyRequeueTime}, nil @@ -502,3 +488,41 @@ func GetCMNamespaceName() types.NamespacedName { Name: os.Getenv("SERVICE_ACCOUNTS_CM_NAME"), } } + +// serviceAccountToVSphereCluster is a mapper function used to enqueue reconcile.Request objects. +// From the watched object owned by this controller, it creates reconcile.Request object +// for the vmwarev1.VSphereCluster object that owns the watched object. +func (r ServiceAccountReconciler) serviceAccountToVSphereCluster(o client.Object) []reconcile.Request { + // We do this because this controller is effectively a vSphereCluster controller that reconciles its + // dependent ProviderServiceAccount objects. + ownerRef := metav1.GetControllerOf(o) + if ownerRef != nil && ownerRef.Kind == kindProviderServiceAccount { + key := types.NamespacedName{Namespace: o.GetNamespace(), Name: ownerRef.Name} + pSvcAccount := &vmwarev1.ProviderServiceAccount{} + if err := r.Client.Get(r.Context, key, pSvcAccount); err != nil { + return nil + } + return toVSphereClusterRequest(pSvcAccount) + } + return nil +} + +// providerServiceAccountToVSphereCluster is a mapper function used to enqueue reconcile.Request objects. +func (r ServiceAccountReconciler) providerServiceAccountToVSphereCluster(o client.Object) []reconcile.Request { + providerServiceAccount, ok := o.(*vmwarev1.ProviderServiceAccount) + if !ok { + return nil + } + + return toVSphereClusterRequest(providerServiceAccount) +} + +func toVSphereClusterRequest(providerServiceAccount *vmwarev1.ProviderServiceAccount) []reconcile.Request { + vsphereClusterRef := providerServiceAccount.Spec.Ref + if vsphereClusterRef == nil || vsphereClusterRef.Name == "" { + return nil + } + return []reconcile.Request{ + {NamespacedName: client.ObjectKey{Namespace: providerServiceAccount.Namespace, Name: vsphereClusterRef.Name}}, + } +} diff --git a/controllers/serviceaccount_controller_intg_test.go b/controllers/serviceaccount_controller_intg_test.go index 12b4cc8491..808250c1d4 100644 --- a/controllers/serviceaccount_controller_intg_test.go +++ b/controllers/serviceaccount_controller_intg_test.go @@ -17,26 +17,30 @@ limitations under the License. package controllers import ( - "context" + "fmt" "os" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" + "github.com/pkg/errors" corev1 "k8s.io/api/core/v1" + rbacv1 "k8s.io/api/rbac/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" + "sigs.k8s.io/cluster-api/util/conditions" + "sigs.k8s.io/controller-runtime/pkg/client" vmwarev1 "sigs.k8s.io/cluster-api-provider-vsphere/apis/vmware/v1beta1" "sigs.k8s.io/cluster-api-provider-vsphere/pkg/builder" ) -var _ = Describe("ServiceAccount controller integration tests", func() { +var _ = Describe("ProviderServiceAccount controller integration tests", func() { var ( intCtx *builder.IntegrationTestContext ) BeforeEach(func() { - ServiceAccountProviderTestsuite.SetIntegrationTestClient(testEnv.Manager.GetClient()) - intCtx = ServiceAccountProviderTestsuite.NewIntegrationTestContextWithClusters(context.Background(), testEnv.Manager.GetClient(), true) + intCtx = ServiceAccountProviderTestsuite.NewIntegrationTestContextWithClusters(ctx, testEnv.Manager.GetClient()) testSystemSvcAcctCM := "test-system-svc-acct-cm" cfgMap := getSystemServiceAccountsConfigMap(intCtx.VSphereCluster.Namespace, testSystemSvcAcctCM) Expect(intCtx.Client.Create(intCtx, cfgMap)).To(Succeed()) @@ -46,7 +50,6 @@ var _ = Describe("ServiceAccount controller integration tests", func() { AfterEach(func() { intCtx.AfterEach() - intCtx = nil }) Describe("When the ProviderServiceAccount is created", func() { @@ -55,13 +58,13 @@ var _ = Describe("ServiceAccount controller integration tests", func() { targetNSObj *corev1.Namespace ) BeforeEach(func() { - pSvcAccount = getTestProviderServiceAccount(intCtx.Namespace, testProviderSvcAccountName, intCtx.VSphereCluster) + pSvcAccount = getTestProviderServiceAccount(intCtx.Namespace, intCtx.VSphereCluster) createTestResource(intCtx, intCtx.Client, pSvcAccount) - assertEventuallyExistsInNamespace(intCtx, intCtx.Client, intCtx.Namespace, testProviderSvcAccountName, pSvcAccount) + assertEventuallyExistsInNamespace(intCtx, 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. + // Deleting the provider service account is not strictly required as the context itself + // gets teared down but keeping it for clarity. deleteTestResource(intCtx, intCtx.Client, pSvcAccount) }) @@ -71,11 +74,32 @@ var _ = Describe("ServiceAccount controller integration tests", func() { // to create a secret containing the bearer token, cert etc for a service account. We need to // simulate the job of the token controller by waiting for the service account creation and then updating it // with a prototype secret. - assertServiceAccountAndUpdateSecret(intCtx, intCtx.Client, intCtx.Namespace, testProviderSvcAccountName) + assertServiceAccountAndUpdateSecret(intCtx, intCtx.Client, intCtx.Namespace, pSvcAccount.GetName()) }) + + It("should create the role and role binding", func() { + Eventually(func() error { + role := &rbacv1.Role{} + key := client.ObjectKeyFromObject(pSvcAccount) + return intCtx.Client.Get(ctx, key, role) + }).Should(Succeed()) + + Eventually(func() error { + roleBinding := &rbacv1.RoleBinding{} + key := client.ObjectKeyFromObject(pSvcAccount) + if err := intCtx.Client.Get(ctx, key, roleBinding); err != nil { + return err + } + if roleBinding.RoleRef.Name != pSvcAccount.GetName() || len(roleBinding.Subjects) != 1 { + return errors.Errorf("roleBinding %s/%s is incorrect", roleBinding.GetNamespace(), roleBinding.GetName()) + } + return nil + }).Should(Succeed()) + }) + It("Should reconcile", func() { By("Creating the target secret in the target namespace") - assertTargetSecret(intCtx, intCtx.GuestClient, testTargetNS, testTargetSecret) + assertTargetSecret(intCtx, intCtx.GuestClient, pSvcAccount.Spec.TargetNamespace, testTargetSecret) }) }) @@ -83,19 +107,74 @@ var _ = Describe("ServiceAccount controller integration tests", func() { BeforeEach(func() { targetNSObj = &corev1.Namespace{ ObjectMeta: metav1.ObjectMeta{ - Name: testTargetNS, + Name: pSvcAccount.Spec.TargetNamespace, }, } Expect(intCtx.GuestClient.Create(intCtx, targetNSObj)).To(Succeed()) - createTargetSecretWithInvalidToken(intCtx, intCtx.GuestClient, testTargetNS) - assertServiceAccountAndUpdateSecret(intCtx, intCtx.Client, intCtx.Namespace, testSvcAccountName) + createTargetSecretWithInvalidToken(intCtx, intCtx.GuestClient, pSvcAccount.Spec.TargetNamespace) + assertServiceAccountAndUpdateSecret(intCtx, intCtx.Client, intCtx.Namespace, pSvcAccount.GetName()) }) AfterEach(func() { deleteTestResource(intCtx, intCtx.GuestClient, targetNSObj) }) It("Should reconcile", func() { By("Updating the target secret in the target namespace") - assertTargetSecret(intCtx, intCtx.GuestClient, testTargetNS, testTargetSecret) + assertTargetSecret(intCtx, intCtx.GuestClient, pSvcAccount.Spec.TargetNamespace, testTargetSecret) + }) + }) + }) + + Context("With non-existent Cluster object", func() { + It("cannot reconcile the ProviderServiceAccount object", func() { + By("Deleting the CAPI cluster object", func() { + clusterName, ok := intCtx.VSphereCluster.GetLabels()[clusterv1.ClusterLabelName] + Expect(ok).To(BeTrue()) + cluster := &clusterv1.Cluster{} + key := client.ObjectKey{Namespace: intCtx.Namespace, Name: clusterName} + Expect(intCtx.Client.Get(intCtx, key, cluster)).To(Succeed()) + Expect(intCtx.Client.Delete(intCtx, cluster)).To(Succeed()) + }) + + By("Creating the ProviderServiceAccount", func() { + pSvcAccount := getTestProviderServiceAccount(intCtx.Namespace, intCtx.VSphereCluster) + createTestResource(intCtx, intCtx.Client, pSvcAccount) + assertEventuallyExistsInNamespace(intCtx, intCtx.Client, intCtx.Namespace, pSvcAccount.GetName(), pSvcAccount) + }) + + By("ProviderServiceAccountsReady Condition is not set", func() { + vsphereCluster := &vmwarev1.VSphereCluster{} + key := client.ObjectKey{Namespace: intCtx.Namespace, Name: intCtx.VSphereCluster.GetName()} + Expect(intCtx.Client.Get(intCtx, key, vsphereCluster)).To(Succeed()) + Expect(conditions.Has(vsphereCluster, vmwarev1.ProviderServiceAccountsReadyCondition)).To(BeFalse()) + }) + }) + }) + + Context("With non-existent Cluster credentials secret", func() { + It("cannot reconcile the ProviderServiceAccount object", func() { + By("Deleting the CAPI kubeconfig secret object", func() { + clusterName, ok := intCtx.VSphereCluster.GetLabels()[clusterv1.ClusterLabelName] + Expect(ok).To(BeTrue()) + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: intCtx.Namespace, + Name: fmt.Sprintf("%s-kubeconfig", clusterName), + }, + } + Expect(intCtx.Client.Delete(intCtx, secret)).To(Succeed()) + }) + + By("Creating the ProviderServiceAccount", func() { + pSvcAccount := getTestProviderServiceAccount(intCtx.Namespace, intCtx.VSphereCluster) + createTestResource(intCtx, intCtx.Client, pSvcAccount) + assertEventuallyExistsInNamespace(intCtx, intCtx.Client, intCtx.Namespace, pSvcAccount.GetName(), pSvcAccount) + }) + + By("ProviderServiceAccountsReady Condition is not set", func() { + vsphereCluster := &vmwarev1.VSphereCluster{} + key := client.ObjectKey{Namespace: intCtx.Namespace, Name: intCtx.VSphereCluster.GetName()} + Expect(intCtx.Client.Get(intCtx, key, vsphereCluster)).To(Succeed()) + Expect(conditions.Has(vsphereCluster, vmwarev1.ProviderServiceAccountsReadyCondition)).To(BeFalse()) }) }) }) diff --git a/controllers/serviceaccount_controller_suite_test.go b/controllers/serviceaccount_controller_suite_test.go index 08172e99aa..11b8593cfc 100644 --- a/controllers/serviceaccount_controller_suite_test.go +++ b/controllers/serviceaccount_controller_suite_test.go @@ -39,15 +39,11 @@ import ( var ServiceAccountProviderTestsuite = builder.NewTestSuiteForController(NewServiceAccountReconciler) const ( - testNS = "test-pvcsi-system" testProviderSvcAccountName = "test-pvcsi" testTargetNS = "test-pvcsi-system" testTargetSecret = "test-pvcsi-secret" // nolint:gosec - testSvcAccountName = testProviderSvcAccountName testSvcAccountSecretName = testProviderSvcAccountName + "-token-abcdef" - testRoleName = testProviderSvcAccountName - testRoleBindingName = testProviderSvcAccountName testSystemSvcAcctNs = "test-system-svc-acct-namespace" testSystemSvcAcctCM = "test-system-svc-acct-cm" @@ -161,7 +157,7 @@ func assertRoleBinding(_ *builder.UnitTestContextForController, ctrlClient clien Expect(len(roleBindingList.Items)).To(Equal(1)) Expect(roleBindingList.Items[0].Name).To(Equal(name)) Expect(roleBindingList.Items[0].RoleRef).To(Equal(rbacv1.RoleRef{ - Name: testRoleName, + Name: name, Kind: "Role", APIGroup: rbacv1.GroupName, })) @@ -200,12 +196,17 @@ func getTestTargetSecretWithValidToken(namespace string) *corev1.Secret { } } -func getTestProviderServiceAccount(namespace, name string, vSphereCluster *vmwarev1.VSphereCluster) *vmwarev1.ProviderServiceAccount { +func getTestProviderServiceAccount(namespace string, vSphereCluster *vmwarev1.VSphereCluster, randomize ...bool) *vmwarev1.ProviderServiceAccount { + objectMeta := metav1.ObjectMeta{ + Namespace: namespace, + } + if len(randomize) > 0 && !randomize[0] { + objectMeta.Name = vSphereCluster.GetName() + } else { + objectMeta.GenerateName = vSphereCluster.GetName() + } pSvcAccount := &vmwarev1.ProviderServiceAccount{ - ObjectMeta: metav1.ObjectMeta{ - Name: name, - Namespace: namespace, - }, + ObjectMeta: objectMeta, Spec: vmwarev1.ProviderServiceAccountSpec{ Rules: []rbacv1.PolicyRule{ { @@ -292,7 +293,7 @@ func getTestRoleBindingWithInvalidRoleRef(namespace, name string) *rbacv1.RoleBi { Kind: "ServiceAccount", APIGroup: "", - Name: testProviderSvcAccountName, + Name: name, Namespace: namespace}, }, } diff --git a/controllers/serviceaccount_controller_unit_test.go b/controllers/serviceaccount_controller_unit_test.go index 058c838e36..d3c6e097c2 100644 --- a/controllers/serviceaccount_controller_unit_test.go +++ b/controllers/serviceaccount_controller_unit_test.go @@ -22,6 +22,7 @@ import ( . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" corev1 "k8s.io/api/core/v1" + capiutil "sigs.k8s.io/cluster-api/util" "sigs.k8s.io/controller-runtime/pkg/client" vmwarev1 "sigs.k8s.io/cluster-api-provider-vsphere/apis/vmware/v1beta1" @@ -36,41 +37,43 @@ func unitTestsReconcileNormal() { ctx *builder.UnitTestContextForController vsphereCluster *vmwarev1.VSphereCluster initObjects []client.Object + namespace string ) JustBeforeEach(func() { // Note: The service account provider requires a reference to the vSphereCluster hence the need to create // a fake vSphereCluster in the test and pass it to during context setup. - ctx = ServiceAccountProviderTestsuite.NewUnitTestContextForControllerWithVSphereCluster(vsphereCluster, false, initObjects...) + ctx = ServiceAccountProviderTestsuite.NewUnitTestContextForControllerWithVSphereCluster(namespace, vsphereCluster, false, initObjects...) }) AfterEach(func() { ctx = nil }) Context("When no provider service account is available", func() { + namespace = capiutil.RandomString(6) It("Should reconcile", func() { By("Not creating any entities") - assertNoEntities(ctx, ctx.Client, testNS) + assertNoEntities(ctx, ctx.Client, namespace) assertProviderServiceAccountsCondition(ctx.VSphereCluster, corev1.ConditionTrue, "", "", "") }) }) Describe("When the ProviderServiceAccount is created", func() { BeforeEach(func() { - obj := fake.NewVSphereCluster() + namespace = capiutil.RandomString(6) + obj := fake.NewVSphereCluster(namespace) vsphereCluster = &obj - vsphereCluster.Namespace = testNS _ = os.Setenv("SERVICE_ACCOUNTS_CM_NAMESPACE", testSystemSvcAcctNs) _ = os.Setenv("SERVICE_ACCOUNTS_CM_NAME", testSystemSvcAcctCM) initObjects = []client.Object{ getSystemServiceAccountsConfigMap(testSystemSvcAcctNs, testSystemSvcAcctCM), - getTestProviderServiceAccount(testNS, testProviderSvcAccountName, vsphereCluster), + getTestProviderServiceAccount(namespace, vsphereCluster, false), } }) Context("When serviceaccount secret is created", func() { It("Should reconcile", func() { assertTargetNamespace(ctx, ctx.GuestClient, testTargetNS, false) - updateServiceAccountSecretAndReconcileNormal(ctx) + updateServiceAccountSecretAndReconcileNormal(ctx, vsphereCluster) assertTargetNamespace(ctx, ctx.GuestClient, testTargetNS, true) By("Creating the target secret in the target namespace") assertTargetSecret(ctx, ctx.GuestClient, testTargetNS, testTargetSecret) @@ -81,7 +84,7 @@ func unitTestsReconcileNormal() { It("Should reconcile", func() { // This is to simulate an outdated token that will be replaced when the serviceaccount secret is created. createTargetSecretWithInvalidToken(ctx, ctx.GuestClient, testTargetNS) - updateServiceAccountSecretAndReconcileNormal(ctx) + updateServiceAccountSecretAndReconcileNormal(ctx, vsphereCluster) By("Updating the target secret in the target namespace") assertTargetSecret(ctx, ctx.GuestClient, testTargetNS, testTargetSecret) assertProviderServiceAccountsCondition(ctx.VSphereCluster, corev1.ConditionTrue, "", "", "") @@ -89,19 +92,19 @@ func unitTestsReconcileNormal() { }) Context("When invalid role exists", func() { BeforeEach(func() { - initObjects = append(initObjects, getTestRoleWithGetPod(testNS, testRoleName)) + initObjects = append(initObjects, getTestRoleWithGetPod(namespace, vsphereCluster.GetName())) }) It("Should update role", func() { - assertRoleWithGetPVC(ctx, ctx.Client, testNS, testRoleName) + assertRoleWithGetPVC(ctx, ctx.Client, namespace, vsphereCluster.GetName()) assertProviderServiceAccountsCondition(ctx.VSphereCluster, corev1.ConditionTrue, "", "", "") }) }) Context("When invalid rolebinding exists", func() { BeforeEach(func() { - initObjects = append(initObjects, getTestRoleBindingWithInvalidRoleRef(testNS, testRoleBindingName)) + initObjects = append(initObjects, getTestRoleBindingWithInvalidRoleRef(namespace, vsphereCluster.GetName())) }) It("Should update rolebinding", func() { - assertRoleBinding(ctx, ctx.Client, testNS, testRoleBindingName) + assertRoleBinding(ctx, ctx.Client, namespace, vsphereCluster.GetName()) assertProviderServiceAccountsCondition(ctx.VSphereCluster, corev1.ConditionTrue, "", "", "") }) }) @@ -110,7 +113,7 @@ func unitTestsReconcileNormal() { // Updates the service account secret similar to how a token controller would act upon a service account // and then re-invokes reconcileNormal. -func updateServiceAccountSecretAndReconcileNormal(ctx *builder.UnitTestContextForController) { - assertServiceAccountAndUpdateSecret(ctx, ctx.Client, testNS, testSvcAccountName) +func updateServiceAccountSecretAndReconcileNormal(ctx *builder.UnitTestContextForController, object client.Object) { + assertServiceAccountAndUpdateSecret(ctx, ctx.Client, object.GetNamespace(), object.GetName()) Expect(ctx.ReconcileNormal()).Should(Succeed()) } diff --git a/controllers/servicediscovery_controller.go b/controllers/servicediscovery_controller.go index 8b37691fa6..7ff2e23f16 100644 --- a/controllers/servicediscovery_controller.go +++ b/controllers/servicediscovery_controller.go @@ -35,6 +35,7 @@ import ( bootstrapapi "k8s.io/cluster-bootstrap/token/api" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" "sigs.k8s.io/cluster-api/controllers/remote" + clusterutilv1 "sigs.k8s.io/cluster-api/util" "sigs.k8s.io/cluster-api/util/conditions" "sigs.k8s.io/cluster-api/util/patch" ctrl "sigs.k8s.io/controller-runtime" @@ -53,8 +54,8 @@ import ( ) const ( - clusterNotReadyRequeueTime = time.Minute * 2 - serviceDiscoverControllerName = "svcdiscovery-controller" + clusterNotReadyRequeueTime = time.Minute * 2 + ServiceDiscoveryControllerName = "servicediscovery-controller" supervisorLoadBalancerSvcNamespace = "kube-system" supervisorLoadBalancerSvcName = "kube-apiserver-lb-svc" @@ -72,8 +73,8 @@ const ( // AddServiceDiscoveryControllerToManager adds the ServiceDiscovery controller to the provided manager. func AddServiceDiscoveryControllerToManager(ctx *context.ControllerManagerContext, mgr manager.Manager) error { var ( - controllerNameShort = serviceDiscoverControllerName - controllerNameLong = fmt.Sprintf("%s/%s/%s", ctx.Namespace, ctx.Name, serviceDiscoverControllerName) + controllerNameShort = ServiceDiscoveryControllerName + controllerNameLong = fmt.Sprintf("%s/%s/%s", ctx.Namespace, ctx.Name, ServiceDiscoveryControllerName) ) controllerContext := &context.ControllerContext{ ControllerManagerContext: ctx, @@ -83,7 +84,8 @@ func AddServiceDiscoveryControllerToManager(ctx *context.ControllerManagerContex } vsphereCluster := &vmwarev1.VSphereCluster{} r := serviceDiscoveryReconciler{ - ControllerContext: controllerContext, + ControllerContext: controllerContext, + remoteClientGetter: remote.NewClusterClient, } configMapCache, err := cache.New(mgr.GetConfig(), cache.Options{ @@ -104,11 +106,11 @@ func AddServiceDiscoveryControllerToManager(ctx *context.ControllerManagerContex return ctrl.NewControllerManagedBy(mgr).For(&vmwarev1.VSphereCluster{}). Watches( &source.Kind{Type: &corev1.Service{}}, - handler.EnqueueRequestsFromMapFunc(svcMapper{ctx: controllerContext.ControllerManagerContext}.Map), + handler.EnqueueRequestsFromMapFunc(r.serviceToClusters), ). Watches( src, - handler.EnqueueRequestsFromMapFunc(configMapMapper{ctx: controllerContext.ControllerManagerContext}.Map), + handler.EnqueueRequestsFromMapFunc(r.configMapToClusters), ). // watch the CAPI cluster Watches( @@ -125,6 +127,8 @@ func newServiceDiscoveryReconciler() builder.Reconciler { type serviceDiscoveryReconciler struct { *context.ControllerContext + + remoteClientGetter remote.ClusterClientGetter } func (r serviceDiscoveryReconciler) Reconcile(ctx goctx.Context, req reconcile.Request) (_ reconcile.Result, reterr error) { @@ -133,10 +137,9 @@ func (r serviceDiscoveryReconciler) Reconcile(ctx goctx.Context, req reconcile.R // Get the vspherecluster for this request. vsphereCluster := &vmwarev1.VSphereCluster{} - clusterKey := client.ObjectKey{Namespace: req.Namespace, Name: req.Name} - if err := r.Client.Get(r, clusterKey, vsphereCluster); err != nil { + if err := r.Client.Get(r, req.NamespacedName, vsphereCluster); err != nil { if apierrors.IsNotFound(err) { - logger.Info("Cluster not found, won't reconcile", "cluster", clusterKey) + logger.Info("Cluster not found, won't reconcile", "key", req.NamespacedName) return reconcile.Result{}, nil } return reconcile.Result{}, err @@ -178,11 +181,15 @@ func (r serviceDiscoveryReconciler) Reconcile(ctx goctx.Context, req reconcile.R return reconcile.Result{}, nil } + cluster, err := clusterutilv1.GetClusterFromMetadata(r, r.Client, vsphereCluster.ObjectMeta) + if err != nil { + logger.Info("unable to get capi cluster from vsphereCluster", "err", err) + return reconcile.Result{RequeueAfter: clusterNotReadyRequeueTime}, nil + } + // We cannot proceed until we are able to access the target cluster. Until - // then just return a no-op and wait for the next sync. This will occur when - // the Cluster's status is updated with a reference to the secret that has - // the Kubeconfig data used to access the target cluster. - guestClient, err := remote.NewClusterClient(clusterContext, serviceDiscoverControllerName, clusterContext.Client, clusterKey) + // then just return a no-op and wait for the next sync. + guestClient, err := r.remoteClientGetter(clusterContext, ServiceDiscoveryControllerName, clusterContext.Client, client.ObjectKeyFromObject(cluster)) if err != nil { logger.Info("The control plane is not ready yet", "err", err) return reconcile.Result{RequeueAfter: clusterNotReadyRequeueTime}, nil @@ -195,44 +202,6 @@ func (r serviceDiscoveryReconciler) Reconcile(ctx goctx.Context, req reconcile.R }) } -type svcMapper struct { - ctx *context.ControllerManagerContext -} - -func (d svcMapper) Map(o client.Object) []reconcile.Request { - // We are only interested in the LB-type Service for the supervisor apiserver. - if o.GetNamespace() != vmwarev1.SupervisorLoadBalancerSvcNamespace || o.GetName() != vmwarev1.SupervisorLoadBalancerSvcName { - return nil - } - return allClustersRequests(d.ctx) -} - -type configMapMapper struct { - ctx *context.ControllerManagerContext -} - -func (d configMapMapper) Map(o client.Object) []reconcile.Request { - // We are only interested in the cluster-info configmap for the supervisor apiserver. - if o.GetNamespace() != metav1.NamespacePublic || o.GetName() != bootstrapapi.ConfigMapClusterInfo { - return nil - } - return allClustersRequests(d.ctx) -} - -func allClustersRequests(ctx *context.ControllerManagerContext) []reconcile.Request { - clustersList := &vmwarev1.VSphereClusterList{} - if err := ctx.Client.List(ctx, clustersList, &client.ListOptions{}); err != nil { - return nil - } - - requests := make([]reconcile.Request, 0, len(clustersList.Items)) - for _, cluster := range clustersList.Items { - key := client.ObjectKey{Namespace: cluster.Namespace, Name: cluster.Name} - requests = append(requests, reconcile.Request{NamespacedName: key}) - } - return requests -} - func (r serviceDiscoveryReconciler) ReconcileNormal(ctx *vmwarecontext.GuestClusterContext) (reconcile.Result, error) { ctx.Logger.V(4).Info("Reconciling Service Discovery", "cluster", ctx.VSphereCluster.Name) if err := r.reconcileSupervisorHeadlessService(ctx); err != nil { @@ -427,3 +396,38 @@ func getClusterFromKubeConfig(config *clientcmdapi.Config) *clientcmdapi.Cluster } return nil } + +// serviceToClusters is a mapper function used to enqueue reconcile.Requests +// It watches for Service objects of type LoadBalancer for the supervisor api-server. +func (r serviceDiscoveryReconciler) serviceToClusters(o client.Object) []reconcile.Request { + if o.GetNamespace() != vmwarev1.SupervisorLoadBalancerSvcNamespace || o.GetName() != vmwarev1.SupervisorLoadBalancerSvcName { + return nil + } + return allClustersRequests(r.Context, r.Client) +} + +// configMapToClusters is a mapper function used to enqueue reconcile.Requests +// It watches for cluster-info configmaps for the supervisor api-server. +func (r serviceDiscoveryReconciler) configMapToClusters(o client.Object) []reconcile.Request { + if o.GetNamespace() != metav1.NamespacePublic || o.GetName() != bootstrapapi.ConfigMapClusterInfo { + return nil + } + return allClustersRequests(r.Context, r.Client) +} + +func allClustersRequests(ctx goctx.Context, c client.Client) []reconcile.Request { + vsphereClusterList := &vmwarev1.VSphereClusterList{} + if err := c.List(ctx, vsphereClusterList, &client.ListOptions{}); err != nil { + return nil + } + + requests := make([]reconcile.Request, 0, len(vsphereClusterList.Items)) + for _, vSphereCluster := range vsphereClusterList.Items { + key := client.ObjectKey{ + Namespace: vSphereCluster.GetNamespace(), + Name: vSphereCluster.GetName(), + } + requests = append(requests, reconcile.Request{NamespacedName: key}) + } + return requests +} diff --git a/controllers/svcdiscovery_controller_intg_test.go b/controllers/svcdiscovery_controller_intg_test.go index e81ee75099..770924e86d 100644 --- a/controllers/svcdiscovery_controller_intg_test.go +++ b/controllers/svcdiscovery_controller_intg_test.go @@ -17,8 +17,6 @@ limitations under the License. package controllers import ( - goctx "context" - . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" corev1 "k8s.io/api/core/v1" @@ -33,12 +31,10 @@ var _ = Describe("Service Discovery controller integration tests", func() { initObjects []client.Object ) BeforeEach(func() { - serviceDiscoveryTestSuite.SetIntegrationTestClient(testEnv.Manager.GetClient()) - intCtx = serviceDiscoveryTestSuite.NewIntegrationTestContextWithClusters(goctx.Background(), testEnv.Manager.GetClient(), true) + intCtx = serviceDiscoveryTestSuite.NewIntegrationTestContextWithClusters(ctx, testEnv.Manager.GetClient()) }) AfterEach(func() { intCtx.AfterEach() - intCtx = nil }) Context("When VIP is available", func() { diff --git a/controllers/svcdiscovery_controller_unit_test.go b/controllers/svcdiscovery_controller_unit_test.go index 913c76e75c..f687aa64e2 100644 --- a/controllers/svcdiscovery_controller_unit_test.go +++ b/controllers/svcdiscovery_controller_unit_test.go @@ -22,21 +22,26 @@ import ( corev1 "k8s.io/api/core/v1" bootstrapapi "k8s.io/cluster-bootstrap/token/api" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" + capiutil "sigs.k8s.io/cluster-api/util" "sigs.k8s.io/controller-runtime/pkg/client" vmwarev1b1 "sigs.k8s.io/cluster-api-provider-vsphere/apis/vmware/v1beta1" "sigs.k8s.io/cluster-api-provider-vsphere/pkg/builder" + "sigs.k8s.io/cluster-api-provider-vsphere/pkg/context/fake" ) var _ = Describe("ServiceDiscoveryReconciler ReconcileNormal", serviceDiscoveryUnitTestsReconcileNormal) func serviceDiscoveryUnitTestsReconcileNormal() { var ( - ctx *builder.UnitTestContextForController - initObjects []client.Object + ctx *builder.UnitTestContextForController + vsphereCluster vmwarev1b1.VSphereCluster + initObjects []client.Object ) + namespace := capiutil.RandomString(6) JustBeforeEach(func() { - ctx = serviceDiscoveryTestSuite.NewUnitTestContextForController(initObjects...) + vsphereCluster = fake.NewVSphereCluster(namespace) + ctx = serviceDiscoveryTestSuite.NewUnitTestContextForController(namespace, &vsphereCluster, initObjects...) }) JustAfterEach(func() { ctx = nil diff --git a/pkg/builder/flags.go b/pkg/builder/flags.go index f209b26c54..deb26de5ab 100644 --- a/pkg/builder/flags.go +++ b/pkg/builder/flags.go @@ -32,10 +32,6 @@ type TestFlags struct { // flag. // Defaults to false. IntegrationTestsEnabled bool - - // unitTestsEnabled is set to true with the -enable-unit-tests flag. - // Defaults to true. - UnitTestsEnabled bool } var flags TestFlags @@ -60,7 +56,6 @@ func init() { i++ } cmdLine.BoolVar(&flags.IntegrationTestsEnabled, "enable-integration-tests", false, "Enables integration tests") - cmdLine.BoolVar(&flags.UnitTestsEnabled, "enable-unit-tests", true, "Enables unit tests") _ = cmdLine.Parse(args) // We still need to add the flags to the default flagset, because otherwise diff --git a/pkg/builder/intg_test_context.go b/pkg/builder/intg_test_context.go index afe6022c20..ea8aafda20 100644 --- a/pkg/builder/intg_test_context.go +++ b/pkg/builder/intg_test_context.go @@ -13,6 +13,7 @@ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. */ + package builder import ( @@ -30,7 +31,7 @@ import ( "k8s.io/client-go/tools/clientcmd" "k8s.io/client-go/tools/clientcmd/api" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" - "sigs.k8s.io/cluster-api/util/patch" + capiutil "sigs.k8s.io/cluster-api/util" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/envtest" @@ -44,21 +45,16 @@ type IntegrationTestContext struct { Client client.Client GuestClient client.Client Namespace string - Cluster *clusterv1.Cluster - ClusterKey client.ObjectKey VSphereCluster *vmwarev1.VSphereCluster VSphereClusterKey client.ObjectKey envTest *envtest.Environment suite *TestSuite - PatchHelper *patch.Helper } func (*IntegrationTestContext) GetLogger() logr.Logger { return logr.Discard() } -var boolTrue = true - // AfterEach should be invoked by ginko.AfterEach to stop the guest cluster's // API server. func (ctx *IntegrationTestContext) AfterEach() { @@ -76,19 +72,21 @@ func (ctx *IntegrationTestContext) AfterEach() { } } -// NewIntegrationTestContext should be invoked by ginkgo.BeforeEach +// NewIntegrationTestContextWithClusters should be invoked by ginkgo.BeforeEach. // -// This function creates a VSphereCluster with a generated name, but stops -// short of generating a CAPI cluster so that it will work when the VSphere Cluster -// controller is also deployed. +// This function creates a VSphereCluster with a generated name as well as a +// CAPI Cluster with the same name. The function also creates a test environment +// and starts its API server to serve as the control plane endpoint for the +// guest cluster. +// +// This function returns a IntegrationTest context. // -// This function returns a TestSuite context // The resources created by this function may be cleaned up by calling AfterEach // with the IntegrationTestContext returned by this function. -func (s *TestSuite) NewIntegrationTestContext(goctx context.Context, integrationTestClient client.Client) *IntegrationTestContext { +func (s *TestSuite) NewIntegrationTestContextWithClusters(goctx context.Context, integrationTestClient client.Client) *IntegrationTestContext { ctx := &IntegrationTestContext{ Context: goctx, - Client: s.integrationTestClient, + Client: integrationTestClient, suite: s, } @@ -103,160 +101,133 @@ func (s *TestSuite) NewIntegrationTestContext(goctx context.Context, integration ctx.Namespace = namespace.Name }) + vsphereClusterName := capiutil.RandomString(6) + cluster := createCluster(goctx, integrationTestClient, ctx.Namespace, vsphereClusterName) + By("Create a vsphere cluster and wait for it to exist", func() { - ctx.VSphereCluster = &vmwarev1.VSphereCluster{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: ctx.Namespace, - Name: "test-pvcsi", + ctx.VSphereCluster = createVSphereCluster(goctx, integrationTestClient, ctx.Namespace, vsphereClusterName, cluster.GetName()) + ctx.VSphereClusterKey = client.ObjectKeyFromObject(ctx.VSphereCluster) + }) + + var config *rest.Config + By("Creating guest cluster control plane", func() { + // Initialize a test environment to simulate the control plane of the guest cluster. + var err error + envTest := &envtest.Environment{ + // Add some form of CRD so the CRD object is registered in the + // scheme... + CRDDirectoryPaths: []string{ + filepath.Join(s.flags.RootDir, "config", "default", "crd"), + filepath.Join(s.flags.RootDir, "config", "supervisor", "crd"), }, } - Expect(ctx.Client.Create(s, ctx.VSphereCluster)).To(Succeed()) - ctx.VSphereClusterKey = client.ObjectKey{Namespace: ctx.VSphereCluster.Namespace, Name: ctx.VSphereCluster.Name} - Eventually(func() error { - return ctx.Client.Get(s, ctx.VSphereClusterKey, ctx.VSphereCluster) - }).Should(Succeed()) + envTest.ControlPlane.GetAPIServer().Configure().Set("allow-privileged", "true") + config, err = envTest.Start() + Expect(err).ShouldNot(HaveOccurred()) + Expect(config).ShouldNot(BeNil()) - ph, err := patch.NewHelper(ctx.VSphereCluster, ctx.Client) - Expect(err).To(BeNil()) - ctx.PatchHelper = ph - }) + ctx.GuestClient, err = client.New(config, client.Options{}) + Expect(err).ShouldNot(HaveOccurred()) + Expect(ctx.GuestClient).ShouldNot(BeNil()) - By("Creating a extensions ca", func() { + ctx.envTest = envTest + }) + By("Create the kubeconfig secret for the cluster", func() { + buf, err := writeKubeConfig(config) + Expect(err).ToNot(HaveOccurred()) secret := &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ - Name: ctx.VSphereCluster.Name + "-extensions-ca", Namespace: ctx.Namespace, + Name: fmt.Sprintf("%s-kubeconfig", cluster.Name), + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: cluster.APIVersion, + Kind: cluster.Kind, + Name: cluster.Name, + UID: cluster.UID, + }, + }, }, Data: map[string][]byte{ - "ca.crt": []byte("test-ca"), - "tls.crt": []byte("test-tls.crt"), - "tls.key": []byte("test-tls.key"), + "value": buf, }, - Type: corev1.SecretTypeTLS, } - Expect(ctx.Client.Create(s, secret)).To(Succeed()) - secretKey := client.ObjectKey{Namespace: secret.Namespace, Name: secret.Name} + Expect(integrationTestClient.Create(s, secret)).To(Succeed()) Eventually(func() error { - return ctx.Client.Get(s, secretKey, secret) + return integrationTestClient.Get(s, client.ObjectKeyFromObject(secret), secret) }).Should(Succeed()) }) + return ctx } -// NewIntegrationTestContextWithClusters should be invoked by ginkgo.BeforeEach. -// -// This function creates a VSphereCluster with a generated name as well as a -// CAPI Cluster with the same name. The function also creates a test environment -// and starts its API server to serve as the control plane endpoint for the -// guest cluster. -// -// This function returns a IntegrationTest context. -// -// The resources created by this function may be cleaned up by calling AfterEach -// with the IntegrationTestContext returned by this function. -func (s *TestSuite) NewIntegrationTestContextWithClusters(goctx context.Context, integrationTestClient client.Client, simulateControlPlane bool) *IntegrationTestContext { - ctx := s.NewIntegrationTestContext(goctx, integrationTestClient) - s.SetIntegrationTestClient(integrationTestClient) - By("Create the CAPI Cluster and wait for it to exist", func() { - cluster := &clusterv1.Cluster{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: ctx.VSphereCluster.Namespace, - Name: ctx.VSphereCluster.Name, - OwnerReferences: []metav1.OwnerReference{ - { - APIVersion: ctx.VSphereCluster.APIVersion, - Kind: ctx.VSphereCluster.Kind, - Name: ctx.VSphereCluster.Name, - UID: ctx.VSphereCluster.UID, - BlockOwnerDeletion: &boolTrue, - Controller: &boolTrue, - }, - }, - }, - Spec: clusterv1.ClusterSpec{ - ClusterNetwork: &clusterv1.ClusterNetwork{ - Pods: &clusterv1.NetworkRanges{ - CIDRBlocks: []string{"1.0.0.0/16"}, - }, - Services: &clusterv1.NetworkRanges{ - CIDRBlocks: []string{"2.0.0.0/16"}, - }, +func createCluster(ctx context.Context, integrationTestClient client.Client, namespace, name string) *clusterv1.Cluster { + By("Create the CAPI Cluster and wait for it to exist") + cluster := &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: namespace, + GenerateName: name, + }, + Spec: clusterv1.ClusterSpec{ + ClusterNetwork: &clusterv1.ClusterNetwork{ + Pods: &clusterv1.NetworkRanges{ + CIDRBlocks: []string{"1.0.0.0/16"}, }, - InfrastructureRef: &corev1.ObjectReference{ - Name: ctx.VSphereCluster.Name, - Namespace: ctx.VSphereCluster.Namespace, + Services: &clusterv1.NetworkRanges{ + CIDRBlocks: []string{"2.0.0.0/16"}, }, }, + InfrastructureRef: &corev1.ObjectReference{ + Name: name, + Namespace: namespace, + }, + }, + } + Expect(integrationTestClient.Create(ctx, cluster)).To(Succeed()) + Eventually(func() error { + return integrationTestClient.Get(ctx, client.ObjectKeyFromObject(cluster), cluster) + }).Should(Succeed()) + return cluster +} + +func createVSphereCluster(ctx context.Context, integrationTestClient client.Client, namespace, name, capiClusterName string) *vmwarev1.VSphereCluster { + vsphereCluster := &vmwarev1.VSphereCluster{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: namespace, + Name: name, + Labels: map[string]string{clusterv1.ClusterLabelName: capiClusterName}, + }, + } + Expect(integrationTestClient.Create(ctx, vsphereCluster)).To(Succeed()) + Eventually(func() error { + return integrationTestClient.Get(ctx, client.ObjectKeyFromObject(vsphereCluster), vsphereCluster) + }).Should(Succeed()) + + // TODO: remove if not needed + By("Creating a extensions ca", func() { + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: vsphereCluster.Name + "-extensions-ca", + Namespace: namespace, + }, + Data: map[string][]byte{ + "ca.crt": []byte("test-ca"), + "tls.crt": []byte("test-tls.crt"), + "tls.key": []byte("test-tls.key"), + }, + Type: corev1.SecretTypeTLS, } - Expect(ctx.Client.Create(s, cluster)).To(Succeed()) - clusterKey := client.ObjectKey{Namespace: cluster.Namespace, Name: cluster.Name} + Expect(integrationTestClient.Create(ctx, secret)).To(Succeed()) + secretKey := client.ObjectKey{Namespace: secret.Namespace, Name: secret.Name} Eventually(func() error { - return ctx.Client.Get(s, clusterKey, cluster) + return integrationTestClient.Get(ctx, secretKey, secret) }).Should(Succeed()) - - ctx.Cluster = cluster - ctx.ClusterKey = clusterKey }) - - if simulateControlPlane { - var config *rest.Config - By("Creating guest cluster control plane", func() { - // Initialize a test environment to simulate the control plane of the - // guest cluster. - var err error - ctx.envTest = &envtest.Environment{ - //KubeAPIServerFlags: append([]string{"--allow-privileged=true"}, envtest.DefaultKubeAPIServerFlags...), - // Add some form of CRD so the CRD object is registered in the - // scheme... - CRDDirectoryPaths: []string{ - filepath.Join(s.flags.RootDir, "config", "default", "crd"), - filepath.Join(s.flags.RootDir, "config", "supervisor", "crd"), - }, - } - ctx.envTest.ControlPlane.GetAPIServer().Configure().Set("allow-privileged", "true") - config, err = ctx.envTest.Start() - Expect(err).ShouldNot(HaveOccurred()) - Expect(config).ShouldNot(BeNil()) - - ctx.GuestClient, err = client.New(config, client.Options{}) - Expect(err).ShouldNot(HaveOccurred()) - Expect(ctx.GuestClient).ShouldNot(BeNil()) - }) - - By("Create the kubeconfig secret for the cluster", func() { - buf, err := WriteKubeConfig(config) - Expect(err).ToNot(HaveOccurred()) - secret := &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: ctx.Cluster.Namespace, - Name: fmt.Sprintf("%s-kubeconfig", ctx.Cluster.Name), - OwnerReferences: []metav1.OwnerReference{ - { - APIVersion: ctx.Cluster.APIVersion, - Kind: ctx.Cluster.Kind, - Name: ctx.Cluster.Name, - UID: ctx.Cluster.UID, - }, - }, - }, - Data: map[string][]byte{ - "value": buf, - }, - } - Expect(ctx.Client.Create(s, secret)).To(Succeed()) - secretKey := client.ObjectKey{Namespace: secret.Namespace, Name: secret.Name} - Eventually(func() error { - return ctx.Client.Get(s, secretKey, secret) - }).Should(Succeed()) - }) - } - - return ctx + return vsphereCluster } -// WriteKubeConfig writes an existing *rest.Config out as the typical -// KubeConfig YAML data. -func WriteKubeConfig(config *rest.Config) ([]byte, error) { +// writeKubeConfig writes an existing *rest.Config out as the typical kubeconfig YAML data. +func writeKubeConfig(config *rest.Config) ([]byte, error) { return clientcmd.Write(api.Config{ Clusters: map[string]*api.Cluster{ config.ServerName: { diff --git a/pkg/builder/test_suite.go b/pkg/builder/test_suite.go index d25caf7a93..6b03dc49a6 100644 --- a/pkg/builder/test_suite.go +++ b/pkg/builder/test_suite.go @@ -14,7 +14,6 @@ See the License for the specific language governing permissions and limitations under the License. */ -//nolint:unused package builder import ( @@ -27,7 +26,6 @@ import ( . "github.com/onsi/gomega" "k8s.io/apimachinery/pkg/runtime" - "k8s.io/client-go/rest" "k8s.io/klog/v2" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/envtest" @@ -42,18 +40,8 @@ type TestSuite struct { goctx.Context integrationTestClient client.Client envTest envtest.Environment - config *rest.Config flags TestFlags newReconcilerFn NewReconcilerFunc - webhookName string -} - -func (s *TestSuite) isWebhookTest() bool { - return s.webhookName != "" -} - -func (s *TestSuite) GetEnvTestConfg() *rest.Config { - return s.config } type Reconciler interface { @@ -71,13 +59,6 @@ func NewTestSuiteForController(newReconcilerFn NewReconcilerFunc) *TestSuite { Context: goctx.Background(), } testSuite.init(newReconcilerFn) - - if testSuite.flags.UnitTestsEnabled { - if newReconcilerFn == nil { - panic("newReconcilerFn is nil") - } - } - return testSuite } @@ -126,35 +107,21 @@ func findModuleDir(module string) string { // suite's reconciler. // // Returns nil if unit testing is disabled. -func (s *TestSuite) NewUnitTestContextForController(initObjects ...client.Object) *UnitTestContextForController { - return s.NewUnitTestContextForControllerWithVSphereCluster(nil, false, initObjects...) -} - -// NewUnitTestContextForControllerWithPrototypeCluster returns a new unit test -// context with a prototype cluster for this suite's reconciler. This prototype cluster -// helps controllers that do not wish to invoke the full TanzuKubernetesCluster -// spec reconciliation. -// -// Returns nil if unit testing is disabled. -func (s *TestSuite) NewUnitTestContextForControllerWithPrototypeCluster(initObjects ...client.Object) *UnitTestContextForController { - return s.NewUnitTestContextForControllerWithVSphereCluster(nil, true, initObjects...) +func (s *TestSuite) NewUnitTestContextForController(namespace string, vsphereCluster *vmwarev1.VSphereCluster, initObjects ...client.Object) *UnitTestContextForController { + return s.NewUnitTestContextForControllerWithVSphereCluster(namespace, vsphereCluster, false, initObjects...) } // NewUnitTestContextForControllerWithVSphereCluster returns a new unit test context for this // suite's reconciler initialized with the given vspherecluster. // // Returns nil if unit testing is disabled. -func (s *TestSuite) NewUnitTestContextForControllerWithVSphereCluster(vsphereCluster *vmwarev1.VSphereCluster, prototypeCluster bool, initObjects ...client.Object) *UnitTestContextForController { - if s.flags.UnitTestsEnabled { - ctx := NewUnitTestContextForController(s.newReconcilerFn, vsphereCluster, prototypeCluster, initObjects, nil) - reconcileNormalAndExpectSuccess(ctx) - // Update the VSphereCluster and its status in the fake client. - Expect(ctx.Client.Update(ctx, ctx.VSphereCluster)).To(Succeed()) - Expect(ctx.Client.Status().Update(ctx, ctx.VSphereCluster)).To(Succeed()) - - return ctx - } - return nil +func (s *TestSuite) NewUnitTestContextForControllerWithVSphereCluster(namespace string, vsphereCluster *vmwarev1.VSphereCluster, prototypeCluster bool, initObjects ...client.Object) *UnitTestContextForController { + ctx := NewUnitTestContextForController(s.newReconcilerFn, namespace, vsphereCluster, prototypeCluster, initObjects, nil) + reconcileNormalAndExpectSuccess(ctx) + // Update the VSphereCluster and its status in the fake client. + Expect(ctx.Client.Update(ctx, ctx.VSphereCluster)).To(Succeed()) + Expect(ctx.Client.Status().Update(ctx, ctx.VSphereCluster)).To(Succeed()) + return ctx } func reconcileNormalAndExpectSuccess(ctx *UnitTestContextForController) { diff --git a/pkg/builder/unit_test_context.go b/pkg/builder/unit_test_context.go index 2477a25705..1db98fbbed 100644 --- a/pkg/builder/unit_test_context.go +++ b/pkg/builder/unit_test_context.go @@ -39,20 +39,20 @@ type UnitTestContextForController struct { VirtualMachineImage *vmoprv1.VirtualMachineImage - // reconciler is the builder.Reconciler being unit tested. + // Reconciler is the builder.Reconciler being unit tested. Reconciler Reconciler } // NewUnitTestContextForController returns a new UnitTestContextForController // with an optional prototype cluster for unit testing controllers that do not // invoke the VSphereCluster spec controller. -func NewUnitTestContextForController(newReconcilerFn NewReconcilerFunc, vSphereCluster *vmwarev1.VSphereCluster, +func NewUnitTestContextForController(newReconcilerFn NewReconcilerFunc, namespace string, vSphereCluster *vmwarev1.VSphereCluster, prototypeCluster bool, initObjects, gcInitObjects []client.Object) *UnitTestContextForController { reconciler := newReconcilerFn() ctx := &UnitTestContextForController{ GuestClusterContext: fake.NewGuestClusterContext(fake.NewVmwareClusterContext( fake.NewControllerContext( - fake.NewControllerManagerContext(initObjects...)), vSphereCluster), prototypeCluster, gcInitObjects...), + fake.NewControllerManagerContext(initObjects...)), namespace, vSphereCluster), prototypeCluster, gcInitObjects...), Reconciler: reconciler, } ctx.Key = client.ObjectKey{Namespace: ctx.VSphereCluster.Namespace, Name: ctx.VSphereCluster.Name} diff --git a/pkg/context/fake/fake_util.go b/pkg/context/fake/fake_util.go index fe04680c33..84de1b1300 100644 --- a/pkg/context/fake/fake_util.go +++ b/pkg/context/fake/fake_util.go @@ -23,10 +23,10 @@ import ( vmwarev1 "sigs.k8s.io/cluster-api-provider-vsphere/apis/vmware/v1beta1" ) -func NewVSphereCluster() vmwarev1.VSphereCluster { +func NewVSphereCluster(namespace string) vmwarev1.VSphereCluster { return vmwarev1.VSphereCluster{ ObjectMeta: metav1.ObjectMeta{ - Namespace: Namespace, + Namespace: namespace, Name: VSphereClusterName, UID: VSphereClusterUUID, }, diff --git a/pkg/context/fake/fake_vmware_cluster_context.go b/pkg/context/fake/fake_vmware_cluster_context.go index 8c8a4b23b2..ae1954f271 100644 --- a/pkg/context/fake/fake_vmware_cluster_context.go +++ b/pkg/context/fake/fake_vmware_cluster_context.go @@ -24,11 +24,11 @@ import ( // NewVmwareClusterContext returns a fake ClusterContext for unit testing // reconcilers with a fake client. -func NewVmwareClusterContext(ctx *context.ControllerContext, vsphereCluster *infrav1.VSphereCluster) *vmware.ClusterContext { +func NewVmwareClusterContext(ctx *context.ControllerContext, namespace string, vsphereCluster *infrav1.VSphereCluster) *vmware.ClusterContext { // Create the cluster resources. cluster := newClusterV1() if vsphereCluster == nil { - v := NewVSphereCluster() + v := NewVSphereCluster(namespace) vsphereCluster = &v }