Skip to content

Commit

Permalink
Remove context from capv customized controller context
Browse files Browse the repository at this point in the history 
Remove context from capv customized controller context, also get rid of containedctx
exceptions.

Signed-off-by: Gong Zhang <[email protected]>
  • Loading branch information
@zhanggbj
zhanggbj committed Oct 23, 2023
1 parent 013ce3f commit dd9842e
Show file tree
Hide file tree
Showing 49 changed files with 196 additions and 212 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 dd9842e

Please sign in to comment.