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 committed Oct 13, 2023
1 parent 6923f2b commit c74e86e
Show file tree
Hide file tree
Showing 32 changed files with 111 additions and 124 deletions.
4 changes: 2 additions & 2 deletions controllers/controllers_suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ func setup() {

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
56 changes: 29 additions & 27 deletions controllers/serviceaccount_controller_intg_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ limitations under the License.
package controllers

import (
"context"
"fmt"
"os"
"reflect"
Expand All @@ -41,12 +42,13 @@ import (

var _ = Describe("ProviderServiceAccount controller integration tests", func() {
var intCtx *helpers.IntegrationTestContext
ctx := context.Background()

BeforeEach(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 +64,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 +79,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 +104,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 +115,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 +136,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 +166,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 +195,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 +248,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
22 changes: 11 additions & 11 deletions controllers/servicediscovery_controller_intg_test.go
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)
})
})
})
4 changes: 2 additions & 2 deletions controllers/vspheredeploymentzone_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ func (r vsphereDeploymentZoneReconciler) Reconcile(ctx context.Context, request
PatchHelper: patchHelper,
}
defer func() {
if err := vsphereDeploymentZoneContext.Patch(); err != nil {
if err := vsphereDeploymentZoneContext.Patch(ctx); err != nil {
reterr = kerrors.NewAggregate([]error{reterr, err})
}
}()
Expand Down Expand Up @@ -233,7 +233,7 @@ func (r vsphereDeploymentZoneReconciler) getVCenterSession(ctx context.Context,
}
logger.Info("using server credentials to create the authenticated session")
params = params.WithUserInfo(creds.Username, creds.Password)
return session.GetOrCreate(r.Context,
return session.GetOrCreate(ctx,

Check warning on line 236 in controllers/vspheredeploymentzone_controller.go

View check run for this annotation

Codecov / codecov/patch

controllers/vspheredeploymentzone_controller.go#L236

Added line #L236 was not covered by tests
params)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ func TestVsphereDeploymentZoneReconciler_Reconcile_VerifyFailureDomain_ComputeCl
WithServer(simr.ServerURL().Host).
WithUserInfo(simr.Username(), simr.Password()).
WithDatacenter("*")
authSession, err := session.GetOrCreate(controllerCtx, params)
authSession, err := session.GetOrCreate(ctx, params)
g.Expect(err).NotTo(HaveOccurred())

vsphereFailureDomain := &infrav1.VSphereFailureDomain{
Expand Down
2 changes: 1 addition & 1 deletion controllers/vspheredeploymentzone_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -617,7 +617,7 @@ func TestVsphereDeploymentZone_Failed_ReconcilePlacementConstraint(t *testing.T)
mgmtContext.Password = pass

controllerCtx := fake.NewControllerContext(mgmtContext)
Expect(controllerCtx.Client.Create(controllerCtx, &infrav1.VSphereFailureDomain{
Expect(controllerCtx.Client.Create(ctx, &infrav1.VSphereFailureDomain{
ObjectMeta: metav1.ObjectMeta{
Name: "blah",
},
Expand Down
2 changes: 1 addition & 1 deletion controllers/vspherevm_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,7 @@ func (r vmReconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ ctrl.R
)

// Patch the VSphereVM resource.
if err := vmContext.Patch(); err != nil {
if err := vmContext.Patch(ctx); err != nil {
reterr = kerrors.NewAggregate([]error{reterr, err})
}
}()
Expand Down
6 changes: 3 additions & 3 deletions main.go
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,7 @@ func main() {

// Create a function that adds all the controllers and webhooks to the manager.
addToManager := func(ctx context.Context, controllerCtx *capvcontext.ControllerManagerContext, mgr ctrlmgr.Manager) error {
tracker, err := setupRemoteClusterCacheTracker(controllerCtx, mgr)
tracker, err := setupRemoteClusterCacheTracker(ctx, mgr)
if err != nil {
return perrors.Wrapf(err, "unable to create remote cluster cache tracker")
}
Expand Down Expand Up @@ -422,7 +422,7 @@ func concurrency(c int) controller.Options {
return controller.Options{MaxConcurrentReconciles: c}
}

func setupRemoteClusterCacheTracker(controllerCtx *capvcontext.ControllerManagerContext, mgr ctrlmgr.Manager) (*remote.ClusterCacheTracker, error) {
func setupRemoteClusterCacheTracker(ctx context.Context, mgr ctrlmgr.Manager) (*remote.ClusterCacheTracker, error) {
secretCachingClient, err := client.New(mgr.GetConfig(), client.Options{
HTTPClient: mgr.GetHTTPClient(),
Cache: &client.CacheOptions{
Expand Down Expand Up @@ -452,7 +452,7 @@ func setupRemoteClusterCacheTracker(controllerCtx *capvcontext.ControllerManager
Client: mgr.GetClient(),
Tracker: tracker,
WatchFilterValue: managerOpts.WatchFilterValue,
}).SetupWithManager(controllerCtx, mgr, concurrency(clusterCacheTrackerConcurrency)); err != nil {
}).SetupWithManager(ctx, mgr, concurrency(clusterCacheTrackerConcurrency)); err != nil {
return nil, perrors.Wrapf(err, "unable to create ClusterCacheReconciler controller")
}

Expand Down
6 changes: 3 additions & 3 deletions pkg/clustermodule/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import (
"github.com/blang/semver"

infrav1 "sigs.k8s.io/cluster-api-provider-vsphere/apis/v1beta1"
"sigs.k8s.io/cluster-api-provider-vsphere/pkg/context"
capvcontext "sigs.k8s.io/cluster-api-provider-vsphere/pkg/context"
)

// Compare returns whether both the cluster module slices are the same.
Expand Down Expand Up @@ -50,8 +50,8 @@ func Compare(oldMods, newMods []infrav1.ClusterModule) bool {
}

// IsClusterCompatible checks if the VCenterVersion is compatibly with CAPV. Only version 7 and over are supported.
func IsClusterCompatible(ctx *context.ClusterContext) bool {
version := ctx.VSphereCluster.Status.VCenterVersion
func IsClusterCompatible(clusterCtx *capvcontext.ClusterContext) bool {
version := clusterCtx.VSphereCluster.Status.VCenterVersion
if version == "" {
return false
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/clustermodule/util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import (
"github.com/onsi/gomega"

infrav1 "sigs.k8s.io/cluster-api-provider-vsphere/apis/v1beta1"
"sigs.k8s.io/cluster-api-provider-vsphere/pkg/context"
capvcontext "sigs.k8s.io/cluster-api-provider-vsphere/pkg/context"
)

func Test_IsCompatible(t *testing.T) {
Expand Down Expand Up @@ -65,7 +65,7 @@ func Test_IsCompatible(t *testing.T) {
}

g := gomega.NewWithT(t)
isCompatible := IsClusterCompatible(&context.ClusterContext{
isCompatible := IsClusterCompatible(&capvcontext.ClusterContext{
VSphereCluster: cluster,
})
g.Expect(isCompatible).To(gomega.Equal(tt.isCompatible))
Expand Down
3 changes: 0 additions & 3 deletions pkg/context/controller_manager_context.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ limitations under the License.
package context

import (
"context"
"sync"
"time"

Expand All @@ -33,8 +32,6 @@ import (
// ControllerManagerContext is the context of the controller that owns the
// controllers.
type ControllerManagerContext struct {
context.Context //nolint:containedctx

// Namespace is the namespace in which the resource is located responsible
// for running the controller manager.
Namespace string
Expand Down
Loading

0 comments on commit c74e86e

Please sign in to comment.