Skip to content

Commit

Permalink
Merge pull request #2431 from zhanggbj/refactor_ctx_init
Browse files Browse the repository at this point in the history 
🌱 Remove context from capv customized controller context
  • Loading branch information
@k8s-ci-robot
k8s-ci-robot authored Oct 25, 2023
2 parents fda92b9 + fa689e1 commit 5211b8e
Show file tree
Hide file tree
Showing 49 changed files with 195 additions and 214 deletions.
6 changes: 3 additions & 3 deletions controllers/clustermodule_reconciler_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -410,7 +410,7 @@ func TestReconciler_Reconcile(t *testing.T) {
tt.beforeFn(md)
}
controllerCtx := fake.NewControllerContext(fake.NewControllerManagerContext(kcp, md))
clusterCtx := fake.NewClusterContext(controllerCtx)
clusterCtx := fake.NewClusterContext(ctx, controllerCtx)
clusterCtx.VSphereCluster.Spec.ClusterModules = tt.clusterModules
clusterCtx.VSphereCluster.Status = infrav1.VSphereClusterStatus{VCenterVersion: infrav1.NewVCenterVersion("7.0.0")}

Expand Down Expand Up @@ -486,7 +486,7 @@ func TestReconciler_fetchMachineOwnerObjects(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
g := gomega.NewWithT(t)
controllerCtx := fake.NewControllerContext(fake.NewControllerManagerContext(tt.initObjs...))
clusterCtx := fake.NewClusterContext(controllerCtx)
clusterCtx := fake.NewClusterContext(ctx, controllerCtx)
r := Reconciler{Client: controllerCtx.Client}
objMap, err := r.fetchMachineOwnerObjects(ctx, clusterCtx)
if tt.hasError {
Expand All @@ -511,7 +511,7 @@ func TestReconciler_fetchMachineOwnerObjects(t *testing.T) {
machineDeployment("foo", metav1.NamespaceDefault, fake.Clusterv1a2Name),
mdToBeDeleted,
))
clusterCtx := fake.NewClusterContext(controllerCtx)
clusterCtx := fake.NewClusterContext(ctx, controllerCtx)
objMap, err := Reconciler{Client: controllerCtx.Client}.fetchMachineOwnerObjects(ctx, clusterCtx)
g.Expect(err).NotTo(gomega.HaveOccurred())
g.Expect(objMap).To(gomega.HaveLen(2))
Expand Down
18 changes: 9 additions & 9 deletions controllers/controllers_suite_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,31 +93,31 @@ func setup() {
panic(fmt.Sprintf("unable to create ClusterCacheReconciler controller: %v", err))
}

if err := AddClusterControllerToManager(ctx, testEnv.GetContext(), testEnv.Manager, &infrav1.VSphereCluster{}, controllerOpts); err != nil {
if err := AddClusterControllerToManager(ctx, testEnv.GetControllerManagerContext(), testEnv.Manager, &infrav1.VSphereCluster{}, controllerOpts); err != nil {
panic(fmt.Sprintf("unable to setup VsphereCluster controller: %v", err))
}
if err := AddMachineControllerToManager(ctx, testEnv.GetContext(), testEnv.Manager, &infrav1.VSphereMachine{}, controllerOpts); err != nil {
if err := AddMachineControllerToManager(ctx, testEnv.GetControllerManagerContext(), testEnv.Manager, &infrav1.VSphereMachine{}, controllerOpts); err != nil {
panic(fmt.Sprintf("unable to setup VsphereMachine controller: %v", err))
}
if err := AddVMControllerToManager(ctx, testEnv.GetContext(), testEnv.Manager, tracker, controllerOpts); err != nil {
if err := AddVMControllerToManager(ctx, testEnv.GetControllerManagerContext(), testEnv.Manager, tracker, controllerOpts); err != nil {
panic(fmt.Sprintf("unable to setup VsphereVM controller: %v", err))
}
if err := AddVsphereClusterIdentityControllerToManager(ctx, testEnv.GetContext(), testEnv.Manager, controllerOpts); err != nil {
if err := AddVsphereClusterIdentityControllerToManager(ctx, testEnv.GetControllerManagerContext(), testEnv.Manager, controllerOpts); err != nil {
panic(fmt.Sprintf("unable to setup VSphereClusterIdentity controller: %v", err))
}
if err := AddVSphereDeploymentZoneControllerToManager(ctx, testEnv.GetContext(), testEnv.Manager, controllerOpts); err != nil {
if err := AddVSphereDeploymentZoneControllerToManager(ctx, testEnv.GetControllerManagerContext(), testEnv.Manager, controllerOpts); err != nil {
panic(fmt.Sprintf("unable to setup VSphereDeploymentZone controller: %v", err))
}
if err := AddServiceAccountProviderControllerToManager(ctx, testEnv.GetContext(), testEnv.Manager, tracker, controllerOpts); err != nil {
if err := AddServiceAccountProviderControllerToManager(ctx, testEnv.GetControllerManagerContext(), testEnv.Manager, tracker, controllerOpts); err != nil {
panic(fmt.Sprintf("unable to setup ServiceAccount controller: %v", err))
}
if err := AddServiceDiscoveryControllerToManager(ctx, testEnv.GetContext(), testEnv.Manager, tracker, controllerOpts); err != nil {
if err := AddServiceDiscoveryControllerToManager(ctx, testEnv.GetControllerManagerContext(), testEnv.Manager, tracker, controllerOpts); err != nil {
panic(fmt.Sprintf("unable to setup SvcDiscovery controller: %v", err))
}

go func() {
fmt.Println("Starting the manager")
if err := testEnv.StartManager(testEnv.GetContext()); err != nil {
if err := testEnv.StartManager(ctx); err != nil {
panic(fmt.Sprintf("failed to start the envtest manager: %v", err))
}
}()
Expand All @@ -132,7 +132,7 @@ func setup() {
Name: manager.DefaultPodNamespace,
},
}
if err := testEnv.Create(testEnv.GetContext(), ns); err != nil {
if err := testEnv.Create(ctx, ns); err != nil {
panic("unable to create controller namespace")
}
}
Expand Down
54 changes: 27 additions & 27 deletions controllers/serviceaccount_controller_intg_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ var _ = Describe("ProviderServiceAccount controller integration tests", func() {
intCtx = helpers.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())
Expect(intCtx.Client.Create(ctx, cfgMap)).To(Succeed())
_ = os.Setenv("SERVICE_ACCOUNTS_CM_NAMESPACE", intCtx.VSphereCluster.Namespace)
_ = os.Setenv("SERVICE_ACCOUNTS_CM_NAME", testSystemSvcAcctCM)
})
Expand All @@ -62,13 +62,13 @@ var _ = Describe("ProviderServiceAccount controller integration tests", func() {
)
BeforeEach(func() {
pSvcAccount = getTestProviderServiceAccount(intCtx.Namespace, intCtx.VSphereCluster)
createTestResource(intCtx, intCtx.Client, pSvcAccount)
assertEventuallyExistsInNamespace(intCtx, intCtx.Client, intCtx.Namespace, pSvcAccount.GetName(), pSvcAccount)
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(intCtx, intCtx.Client, pSvcAccount)
deleteTestResource(ctx, intCtx.Client, pSvcAccount)
})

Context("When serviceaccount secret is created", func() {
Expand All @@ -77,7 +77,7 @@ var _ = Describe("ProviderServiceAccount 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, pSvcAccount.GetName())
assertServiceAccountAndUpdateSecret(ctx, intCtx.Client, intCtx.Namespace, pSvcAccount.GetName())
})

It("should create the role and role binding", func() {
Expand All @@ -102,7 +102,7 @@ var _ = Describe("ProviderServiceAccount controller integration tests", func() {

It("Should reconcile", func() {
By("Creating the target secret in the target namespace")
assertTargetSecret(intCtx, intCtx.GuestClient, pSvcAccount.Spec.TargetNamespace, testTargetSecret)
assertTargetSecret(ctx, intCtx.GuestClient, pSvcAccount.Spec.TargetNamespace, testTargetSecret)
})
})

Expand All @@ -113,16 +113,16 @@ var _ = Describe("ProviderServiceAccount controller integration tests", func() {
Name: pSvcAccount.Spec.TargetNamespace,
},
}
Expect(intCtx.GuestClient.Create(intCtx, targetNSObj)).To(Succeed())
createTargetSecretWithInvalidToken(intCtx, intCtx.GuestClient, pSvcAccount.Spec.TargetNamespace)
assertServiceAccountAndUpdateSecret(intCtx, intCtx.Client, intCtx.Namespace, pSvcAccount.GetName())
Expect(intCtx.GuestClient.Create(ctx, targetNSObj)).To(Succeed())
createTargetSecretWithInvalidToken(ctx, intCtx.GuestClient, pSvcAccount.Spec.TargetNamespace)
assertServiceAccountAndUpdateSecret(ctx, intCtx.Client, intCtx.Namespace, pSvcAccount.GetName())
})
AfterEach(func() {
deleteTestResource(intCtx, intCtx.GuestClient, targetNSObj)
deleteTestResource(ctx, intCtx.GuestClient, targetNSObj)
})
It("Should reconcile", func() {
By("Updating the target secret in the target namespace")
assertTargetSecret(intCtx, intCtx.GuestClient, pSvcAccount.Spec.TargetNamespace, testTargetSecret)
assertTargetSecret(ctx, intCtx.GuestClient, pSvcAccount.Spec.TargetNamespace, testTargetSecret)
})
})
})
Expand All @@ -134,20 +134,20 @@ var _ = Describe("ProviderServiceAccount controller integration tests", func() {
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())
Expect(intCtx.Client.Get(ctx, key, cluster)).To(Succeed())
Expect(intCtx.Client.Delete(ctx, 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)
createTestResource(ctx, intCtx.Client, pSvcAccount)
assertEventuallyExistsInNamespace(ctx, 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(intCtx.Client.Get(ctx, key, vsphereCluster)).To(Succeed())
Expect(conditions.Has(vsphereCluster, vmwarev1.ProviderServiceAccountsReadyCondition)).To(BeFalse())
})
})
Expand All @@ -164,19 +164,19 @@ var _ = Describe("ProviderServiceAccount controller integration tests", func() {
Name: fmt.Sprintf("%s-kubeconfig", clusterName),
},
}
Expect(intCtx.Client.Delete(intCtx, secret)).To(Succeed())
Expect(intCtx.Client.Delete(ctx, 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)
createTestResource(ctx, intCtx.Client, pSvcAccount)
assertEventuallyExistsInNamespace(ctx, 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(intCtx.Client.Get(ctx, key, vsphereCluster)).To(Succeed())
Expect(conditions.Has(vsphereCluster, vmwarev1.ProviderServiceAccountsReadyCondition)).To(BeFalse())
})
})
Expand All @@ -193,7 +193,7 @@ var _ = Describe("ProviderServiceAccount controller integration tests", func() {
pSvcAccount.ObjectMeta.Annotations = map[string]string{
"cluster.x-k8s.io/paused": "true",
}
createTestResource(intCtx, intCtx.Client, pSvcAccount)
createTestResource(ctx, intCtx.Client, pSvcAccount)
oldOwnerUID := uuid.New().String()

role = &rbacv1.Role{
Expand Down Expand Up @@ -246,19 +246,19 @@ var _ = Describe("ProviderServiceAccount controller integration tests", func() {
},
}

createTestResource(intCtx, intCtx.Client, role)
createTestResource(intCtx, intCtx.Client, roleBinding)
assertEventuallyExistsInNamespace(intCtx, intCtx.Client, intCtx.Namespace, pSvcAccount.GetName(), pSvcAccount)
createTestResource(ctx, intCtx.Client, role)
createTestResource(ctx, intCtx.Client, roleBinding)
assertEventuallyExistsInNamespace(ctx, intCtx.Client, intCtx.Namespace, pSvcAccount.GetName(), pSvcAccount)
svcAccountPatcher, err := patch.NewHelper(pSvcAccount, intCtx.Client)
Expect(err).ToNot(HaveOccurred())
// Unpause the ProviderServiceAccount so we can reconcile
pSvcAccount.SetAnnotations(map[string]string{})
Expect(svcAccountPatcher.Patch(ctx, pSvcAccount)).To(Succeed())
})
AfterEach(func() {
deleteTestResource(intCtx, intCtx.Client, pSvcAccount)
deleteTestResource(intCtx, intCtx.Client, role)
deleteTestResource(intCtx, intCtx.Client, roleBinding)
deleteTestResource(ctx, intCtx.Client, pSvcAccount)
deleteTestResource(ctx, intCtx.Client, role)
deleteTestResource(ctx, intCtx.Client, roleBinding)
})

It("should fully reconciles dependent resources", func() {
Expand Down
5 changes: 2 additions & 3 deletions controllers/serviceaccount_controller_suite_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ import (
"sigs.k8s.io/controller-runtime/pkg/client"

vmwarev1 "sigs.k8s.io/cluster-api-provider-vsphere/apis/vmware/v1beta1"
helpers "sigs.k8s.io/cluster-api-provider-vsphere/test/helpers/vmware"
)

const (
Expand Down Expand Up @@ -140,12 +139,12 @@ func assertRoleWithGetPVC(ctx context.Context, ctrlClient client.Client, namespa
}))
}

func assertRoleBinding(_ *helpers.UnitTestContextForController, ctrlClient client.Client, namespace, name string) {
func assertRoleBinding(ctx context.Context, ctrlClient client.Client, namespace, name string) {
var roleBindingList rbacv1.RoleBindingList
opts := &client.ListOptions{
Namespace: namespace,
}
err := ctrlClient.List(context.TODO(), &roleBindingList, opts)
err := ctrlClient.List(ctx, &roleBindingList, opts)
Expect(err).ShouldNot(HaveOccurred())
Expect(roleBindingList.Items).To(HaveLen(1))
Expect(roleBindingList.Items[0].Name).To(Equal(name))
Expand Down
4 changes: 2 additions & 2 deletions controllers/serviceaccount_controller_unit_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ func unitTestsReconcileNormal() {
)

JustBeforeEach(func() {
controllerCtx = helpers.NewUnitTestContextForController(namespace, vsphereCluster, false, initObjects, nil)
controllerCtx = helpers.NewUnitTestContextForController(ctx, namespace, vsphereCluster, false, initObjects, nil)
// 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.
reconciler = ServiceAccountReconciler{
Expand Down Expand Up @@ -128,7 +128,7 @@ func unitTestsReconcileNormal() {
initObjects = append(initObjects, getTestRoleBindingWithInvalidRoleRef(namespace, vsphereCluster.GetName()))
})
It("Should update rolebinding", func() {
assertRoleBinding(controllerCtx, controllerCtx.ControllerContext.Client, namespace, vsphereCluster.GetName())
assertRoleBinding(ctx, controllerCtx.ControllerContext.Client, namespace, vsphereCluster.GetName())
assertProviderServiceAccountsCondition(controllerCtx.VSphereCluster, corev1.ConditionTrue, "", "", "")
})
})
Expand Down
22 changes: 11 additions & 11 deletions controllers/servicediscovery_controller_intg_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,51 +42,51 @@ var _ = Describe("Service Discovery controller integration tests", func() {
initObjects = []client.Object{
newTestSupervisorLBServiceWithIPStatus(),
}
createObjects(intCtx, intCtx.Client, initObjects)
createObjects(ctx, intCtx.Client, initObjects)
Expect(intCtx.Client.Status().Update(ctx, newTestSupervisorLBServiceWithIPStatus())).To(Succeed())
})
AfterEach(func() {
deleteObjects(intCtx, intCtx.Client, initObjects)
deleteObjects(ctx, intCtx.Client, initObjects)
})
It("Should reconcile headless svc", func() {
By("creating a service and endpoints using the VIP in the guest cluster")
headlessSvc := &corev1.Service{}
assertEventuallyExistsInNamespace(intCtx, intCtx.Client, "kube-system", "kube-apiserver-lb-svc", headlessSvc)
assertHeadlessSvcWithVIPEndpoints(intCtx, intCtx.GuestClient, supervisorHeadlessSvcNamespace, supervisorHeadlessSvcName)
assertEventuallyExistsInNamespace(ctx, intCtx.Client, "kube-system", "kube-apiserver-lb-svc", headlessSvc)
assertHeadlessSvcWithVIPEndpoints(ctx, intCtx.GuestClient, supervisorHeadlessSvcNamespace, supervisorHeadlessSvcName)
})
})

Context("When FIP is available", func() {
BeforeEach(func() {
initObjects = []client.Object{
newTestConfigMapWithHost(testSupervisorAPIServerFIP)}
createObjects(intCtx, intCtx.Client, initObjects)
createObjects(ctx, intCtx.Client, initObjects)
})
AfterEach(func() {
deleteObjects(intCtx, intCtx.Client, initObjects)
deleteObjects(ctx, intCtx.Client, initObjects)
})
It("Should reconcile headless svc", func() {
By("creating a service and endpoints using the FIP in the guest cluster")
assertHeadlessSvcWithFIPEndpoints(intCtx, intCtx.GuestClient, supervisorHeadlessSvcNamespace, supervisorHeadlessSvcName)
assertHeadlessSvcWithFIPEndpoints(ctx, intCtx.GuestClient, supervisorHeadlessSvcNamespace, supervisorHeadlessSvcName)
})
})
Context("When headless svc and endpoints already exists", func() {
BeforeEach(func() {
// Create the svc & endpoint objects in guest cluster
createObjects(intCtx, intCtx.GuestClient, newTestHeadlessSvcEndpoints())
createObjects(ctx, intCtx.GuestClient, newTestHeadlessSvcEndpoints())
// Init objects in the supervisor cluster
initObjects = []client.Object{
newTestSupervisorLBServiceWithIPStatus()}
createObjects(intCtx, intCtx.Client, initObjects)
createObjects(ctx, intCtx.Client, initObjects)
Expect(intCtx.Client.Status().Update(ctx, newTestSupervisorLBServiceWithIPStatus())).To(Succeed())
})
AfterEach(func() {
deleteObjects(intCtx, intCtx.Client, initObjects)
deleteObjects(ctx, intCtx.Client, initObjects)
// Note: No need to delete guest cluster objects as a new guest cluster testenv endpoint is created for each test.
})
It("Should reconcile headless svc", func() {
By("updating the service and endpoints using the VIP in the guest cluster")
assertHeadlessSvcWithUpdatedVIPEndpoints(intCtx, intCtx.GuestClient, supervisorHeadlessSvcNamespace, supervisorHeadlessSvcName)
assertHeadlessSvcWithUpdatedVIPEndpoints(ctx, intCtx.GuestClient, supervisorHeadlessSvcNamespace, supervisorHeadlessSvcName)
})
})
})
2 changes: 1 addition & 1 deletion controllers/servicediscovery_controller_unit_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ func serviceDiscoveryUnitTestsReconcileNormal() {
namespace := capiutil.RandomString(6)
JustBeforeEach(func() {
vsphereCluster = fake.NewVSphereCluster(namespace)
controllerCtx = helpers.NewUnitTestContextForController(namespace, &vsphereCluster, false, initObjects, nil)
controllerCtx = helpers.NewUnitTestContextForController(ctx, namespace, &vsphereCluster, false, initObjects, nil)
reconciler = serviceDiscoveryReconciler{
Client: controllerCtx.ControllerContext.Client,
Recorder: controllerCtx.ControllerContext.Recorder,
Expand Down
5 changes: 2 additions & 3 deletions controllers/vmware/vspherecluster_reconciler_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ limitations under the License.
package vmware

import (
"context"
"testing"

. "github.com/onsi/ginkgo/v2"
Expand Down Expand Up @@ -85,13 +84,13 @@ var _ = Describe("Cluster Controller Tests", func() {
// Ensure that the mechanism for reconciling clusters when a control plane machine gets an IP works
Context("Test controlPlaneMachineToCluster", func() {
It("Returns nil if there is no IP address", func() {
request := reconciler.VSphereMachineToCluster(context.Background(), vsphereMachine)
request := reconciler.VSphereMachineToCluster(ctx, vsphereMachine)
Expect(request).Should(BeNil())
})

It("Returns valid request with IP address", func() {
vsphereMachine.Status.IPAddr = testIP
request := reconciler.VSphereMachineToCluster(context.Background(), vsphereMachine)
request := reconciler.VSphereMachineToCluster(ctx, vsphereMachine)
Expect(request).ShouldNot(BeNil())
Expect(request[0].Namespace).Should(Equal(cluster.Namespace))
Expect(request[0].Name).Should(Equal(cluster.Name))
Expand Down
Loading

0 comments on commit 5211b8e

Please sign in to comment.