Skip to content

Commit 5ba5364

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request #2132 from laozc/cluster-cache
🌱 Using ClusterCacheTracker instead of remote.NewClusterClient
2 parents 678ac99 + 071401f commit 5ba5364

File tree

6 files changed

+158
-32
lines changed

6 files changed

+158
-32
lines changed

‎controllers/controllers_suite_test.go

Lines changed: 33 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,9 @@ import (
2828
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
2929
"k8s.io/client-go/kubernetes/scheme"
3030
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
31+
"sigs.k8s.io/cluster-api/controllers/remote"
3132
ctrl "sigs.k8s.io/controller-runtime"
33+
"sigs.k8s.io/controller-runtime/pkg/client"
3234
"sigs.k8s.io/controller-runtime/pkg/controller"
3335

3436
infrav1 "sigs.k8s.io/cluster-api-provider-vsphere/apis/v1beta1"
@@ -63,15 +65,43 @@ func setup() {
6365

6466
testEnv = helpers.NewTestEnvironment()
6567

68+
secretCachingClient, err := client.New(testEnv.Manager.GetConfig(), client.Options{
69+
HTTPClient: testEnv.Manager.GetHTTPClient(),
70+
Cache: &client.CacheOptions{
71+
Reader: testEnv.Manager.GetCache(),
72+
},
73+
})
74+
if err != nil {
75+
panic("unable to create secret caching client")
76+
}
77+
78+
tracker, err := remote.NewClusterCacheTracker(
79+
testEnv.Manager,
80+
remote.ClusterCacheTrackerOptions{
81+
SecretCachingClient: secretCachingClient,
82+
ControllerName: "testenv-manager",
83+
},
84+
)
85+
if err != nil {
86+
panic(fmt.Sprintf("unable to setup ClusterCacheTracker: %v", err))
87+
}
88+
6689
controllerOpts := controller.Options{MaxConcurrentReconciles: 10}
6790

91+
if err := (&remote.ClusterCacheReconciler{
92+
Client: testEnv.Manager.GetClient(),
93+
Tracker: tracker,
94+
}).SetupWithManager(ctx, testEnv.Manager, controllerOpts); err != nil {
95+
panic(fmt.Sprintf("unable to create ClusterCacheReconciler controller: %v", err))
96+
}
97+
6898
if err := AddClusterControllerToManager(testEnv.GetContext(), testEnv.Manager, &infrav1.VSphereCluster{}, controllerOpts); err != nil {
6999
panic(fmt.Sprintf("unable to setup VsphereCluster controller: %v", err))
70100
}
71101
if err := AddMachineControllerToManager(testEnv.GetContext(), testEnv.Manager, &infrav1.VSphereMachine{}, controllerOpts); err != nil {
72102
panic(fmt.Sprintf("unable to setup VsphereMachine controller: %v", err))
73103
}
74-
if err := AddVMControllerToManager(testEnv.GetContext(), testEnv.Manager, controllerOpts); err != nil {
104+
if err := AddVMControllerToManager(testEnv.GetContext(), testEnv.Manager, tracker, controllerOpts); err != nil {
75105
panic(fmt.Sprintf("unable to setup VsphereVM controller: %v", err))
76106
}
77107
if err := AddVsphereClusterIdentityControllerToManager(testEnv.GetContext(), testEnv.Manager, controllerOpts); err != nil {
@@ -80,10 +110,10 @@ func setup() {
80110
if err := AddVSphereDeploymentZoneControllerToManager(testEnv.GetContext(), testEnv.Manager, controllerOpts); err != nil {
81111
panic(fmt.Sprintf("unable to setup VSphereDeploymentZone controller: %v", err))
82112
}
83-
if err := AddServiceAccountProviderControllerToManager(testEnv.GetContext(), testEnv.Manager, controllerOpts); err != nil {
113+
if err := AddServiceAccountProviderControllerToManager(testEnv.GetContext(), testEnv.Manager, tracker, controllerOpts); err != nil {
84114
panic(fmt.Sprintf("unable to setup ServiceAccount controller: %v", err))
85115
}
86-
if err := AddServiceDiscoveryControllerToManager(testEnv.GetContext(), testEnv.Manager, controllerOpts); err != nil {
116+
if err := AddServiceDiscoveryControllerToManager(testEnv.GetContext(), testEnv.Manager, tracker, controllerOpts); err != nil {
87117
panic(fmt.Sprintf("unable to setup SvcDiscovery controller: %v", err))
88118
}
89119

‎controllers/serviceaccount_controller.go

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ const (
6666
)
6767

6868
// AddServiceAccountProviderControllerToManager adds this controller to the provided manager.
69-
func AddServiceAccountProviderControllerToManager(ctx *context.ControllerManagerContext, mgr manager.Manager, options controller.Options) error {
69+
func AddServiceAccountProviderControllerToManager(ctx *context.ControllerManagerContext, mgr manager.Manager, tracker *remote.ClusterCacheTracker, options controller.Options) error {
7070
var (
7171
controlledType = &vmwarev1.ProviderServiceAccount{}
7272
controlledTypeName = reflect.TypeOf(controlledType).Elem().Name()
@@ -82,8 +82,8 @@ func AddServiceAccountProviderControllerToManager(ctx *context.ControllerManager
8282
Logger: ctx.Logger.WithName(controllerNameShort),
8383
}
8484
r := ServiceAccountReconciler{
85-
ControllerContext: controllerContext,
86-
remoteClientGetter: remote.NewClusterClient,
85+
ControllerContext: controllerContext,
86+
remoteClusterCacheTracker: tracker,
8787
}
8888

8989
clusterToInfraFn := clusterToSupervisorInfrastructureMapFunc(ctx)
@@ -134,7 +134,7 @@ func clusterToSupervisorInfrastructureMapFunc(managerContext *context.Controller
134134
type ServiceAccountReconciler struct {
135135
*context.ControllerContext
136136

137-
remoteClientGetter remote.ClusterClientGetter
137+
remoteClusterCacheTracker *remote.ClusterCacheTracker
138138
}
139139

140140
func (r ServiceAccountReconciler) Reconcile(_ goctx.Context, req reconcile.Request) (_ reconcile.Result, reterr error) {
@@ -202,8 +202,12 @@ func (r ServiceAccountReconciler) Reconcile(_ goctx.Context, req reconcile.Reque
202202
// then just return a no-op and wait for the next sync. This will occur when
203203
// the Cluster's status is updated with a reference to the secret that has
204204
// the Kubeconfig data used to access the target cluster.
205-
guestClient, err := r.remoteClientGetter(clusterContext, ProviderServiceAccountControllerName, clusterContext.Client, client.ObjectKeyFromObject(cluster))
205+
guestClient, err := r.remoteClusterCacheTracker.GetClient(clusterContext, client.ObjectKeyFromObject(cluster))
206206
if err != nil {
207+
if errors.Is(err, remote.ErrClusterLocked) {
208+
r.Logger.V(5).Info("Requeuing because another worker has the lock on the ClusterCacheTracker")
209+
return ctrl.Result{Requeue: true}, nil
210+
}
207211
clusterContext.Logger.Info("The control plane is not ready yet", "err", err)
208212
return reconcile.Result{RequeueAfter: clusterNotReadyRequeueTime}, nil
209213
}

‎controllers/servicediscovery_controller.go

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ const (
7272
// +kubebuilder:rbac:groups="",resources=configmaps/status,verbs=get
7373

7474
// AddServiceDiscoveryControllerToManager adds the ServiceDiscovery controller to the provided manager.
75-
func AddServiceDiscoveryControllerToManager(ctx *context.ControllerManagerContext, mgr manager.Manager, options controller.Options) error {
75+
func AddServiceDiscoveryControllerToManager(ctx *context.ControllerManagerContext, mgr manager.Manager, tracker *remote.ClusterCacheTracker, options controller.Options) error {
7676
var (
7777
controllerNameShort = ServiceDiscoveryControllerName
7878
controllerNameLong = fmt.Sprintf("%s/%s/%s", ctx.Namespace, ctx.Name, ServiceDiscoveryControllerName)
@@ -84,8 +84,8 @@ func AddServiceDiscoveryControllerToManager(ctx *context.ControllerManagerContex
8484
Logger: ctx.Logger.WithName(controllerNameShort),
8585
}
8686
r := serviceDiscoveryReconciler{
87-
ControllerContext: controllerContext,
88-
remoteClientGetter: remote.NewClusterClient,
87+
ControllerContext: controllerContext,
88+
remoteClusterCacheTracker: tracker,
8989
}
9090

9191
configMapCache, err := cache.New(mgr.GetConfig(), cache.Options{
@@ -128,7 +128,7 @@ func AddServiceDiscoveryControllerToManager(ctx *context.ControllerManagerContex
128128
type serviceDiscoveryReconciler struct {
129129
*context.ControllerContext
130130

131-
remoteClientGetter remote.ClusterClientGetter
131+
remoteClusterCacheTracker *remote.ClusterCacheTracker
132132
}
133133

134134
func (r serviceDiscoveryReconciler) Reconcile(_ goctx.Context, req reconcile.Request) (_ reconcile.Result, reterr error) {
@@ -189,8 +189,12 @@ func (r serviceDiscoveryReconciler) Reconcile(_ goctx.Context, req reconcile.Req
189189

190190
// We cannot proceed until we are able to access the target cluster. Until
191191
// then just return a no-op and wait for the next sync.
192-
guestClient, err := r.remoteClientGetter(clusterContext, ServiceDiscoveryControllerName, clusterContext.Client, client.ObjectKeyFromObject(cluster))
192+
guestClient, err := r.remoteClusterCacheTracker.GetClient(clusterContext, client.ObjectKeyFromObject(cluster))
193193
if err != nil {
194+
if errors.Is(err, remote.ErrClusterLocked) {
195+
r.Logger.V(5).Info("Requeuing because another worker has the lock on the ClusterCacheTracker")
196+
return ctrl.Result{Requeue: true}, nil
197+
}
194198
logger.Info("The control plane is not ready yet", "err", err)
195199
return reconcile.Result{RequeueAfter: clusterNotReadyRequeueTime}, nil
196200
}

‎controllers/vspherevm_controller.go

Lines changed: 21 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ import (
7070
// AddVMControllerToManager adds the VM controller to the provided manager.
7171
//
7272
//nolint:forcetypeassert
73-
func AddVMControllerToManager(ctx *context.ControllerManagerContext, mgr manager.Manager, options controller.Options) error {
73+
func AddVMControllerToManager(ctx *context.ControllerManagerContext, mgr manager.Manager, tracker *remote.ClusterCacheTracker, options controller.Options) error {
7474
var (
7575
controlledType = &infrav1.VSphereVM{}
7676
controlledTypeName = reflect.TypeOf(controlledType).Elem().Name()
@@ -88,8 +88,9 @@ func AddVMControllerToManager(ctx *context.ControllerManagerContext, mgr manager
8888
Logger: ctx.Logger.WithName(controllerNameShort),
8989
}
9090
r := vmReconciler{
91-
ControllerContext: controllerContext,
92-
VMService: &govmomi.VMService{},
91+
ControllerContext: controllerContext,
92+
VMService: &govmomi.VMService{},
93+
remoteClusterCacheTracker: tracker,
9394
}
9495
controller, err := ctrl.NewControllerManagedBy(mgr).
9596
// Watch the controlled, infrastructure resource.
@@ -158,7 +159,8 @@ func AddVMControllerToManager(ctx *context.ControllerManagerContext, mgr manager
158159
type vmReconciler struct {
159160
*context.ControllerContext
160161

161-
VMService services.VirtualMachineService
162+
VMService services.VirtualMachineService
163+
remoteClusterCacheTracker *remote.ClusterCacheTracker
162164
}
163165

164166
// Reconcile ensures the back-end state reflects the Kubernetes resource state intent.
@@ -344,9 +346,14 @@ func (r vmReconciler) reconcileDelete(ctx *context.VMContext) (reconcile.Result,
344346
}
345347

346348
// Attempt to delete the node corresponding to the vsphere VM
347-
if err := r.deleteNode(ctx, vm.Name); err != nil {
349+
result, err = r.deleteNode(ctx, vm.Name)
350+
if err != nil {
348351
r.Logger.V(6).Info("unable to delete node", "err", err)
349352
}
353+
if !result.IsZero() {
354+
// a non-zero value means we need to requeue the request before proceed.
355+
return result, nil
356+
}
350357

351358
if err := r.deleteIPAddressClaims(ctx); err != nil {
352359
return reconcile.Result{}, err
@@ -362,15 +369,19 @@ func (r vmReconciler) reconcileDelete(ctx *context.VMContext) (reconcile.Result,
362369
// This is necessary since CAPI does not the nodeRef field on the owner Machine object
363370
// until the node moves to Ready state. Hence, on Machine deletion it is unable to delete
364371
// the kubernetes node corresponding to the VM.
365-
func (r vmReconciler) deleteNode(ctx *context.VMContext, name string) error {
372+
func (r vmReconciler) deleteNode(ctx *context.VMContext, name string) (reconcile.Result, error) {
366373
// Fetching the cluster object from the VSphereVM object to create a remote client to the cluster
367374
cluster, err := clusterutilv1.GetClusterFromMetadata(r.ControllerContext, r.Client, ctx.VSphereVM.ObjectMeta)
368375
if err != nil {
369-
return err
376+
return ctrl.Result{}, err
370377
}
371-
clusterClient, err := remote.NewClusterClient(ctx, r.ControllerContext.Name, r.Client, ctrlclient.ObjectKeyFromObject(cluster))
378+
clusterClient, err := r.remoteClusterCacheTracker.GetClient(ctx, ctrlclient.ObjectKeyFromObject(cluster))
372379
if err != nil {
373-
return err
380+
if errors.Is(err, remote.ErrClusterLocked) {
381+
r.Logger.V(5).Info("Requeuing because another worker has the lock on the ClusterCacheTracker")
382+
return ctrl.Result{Requeue: true}, nil
383+
}
384+
return ctrl.Result{}, err
374385
}
375386

376387
// Attempt to delete the corresponding node
@@ -379,7 +390,7 @@ func (r vmReconciler) deleteNode(ctx *context.VMContext, name string) error {
379390
Name: name,
380391
},
381392
}
382-
return clusterClient.Delete(ctx, node)
393+
return ctrl.Result{}, clusterClient.Delete(ctx, node)
383394
}
384395

385396
func (r vmReconciler) reconcileNormal(ctx *context.VMContext) (reconcile.Result, error) {

‎controllers/vspherevm_controller_test.go

Lines changed: 35 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,11 +30,13 @@ import (
3030
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
3131
apirecord "k8s.io/client-go/tools/record"
3232
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
33+
"sigs.k8s.io/cluster-api/controllers/remote"
3334
ipamv1 "sigs.k8s.io/cluster-api/exp/ipam/api/v1alpha1"
3435
"sigs.k8s.io/cluster-api/util"
3536
"sigs.k8s.io/cluster-api/util/conditions"
3637
ctrl "sigs.k8s.io/controller-runtime"
3738
"sigs.k8s.io/controller-runtime/pkg/client"
39+
"sigs.k8s.io/controller-runtime/pkg/controller"
3840
"sigs.k8s.io/controller-runtime/pkg/log"
3941
"sigs.k8s.io/controller-runtime/pkg/reconcile"
4042

@@ -73,6 +75,36 @@ func TestReconcileNormal_WaitingForIPAddrAllocation(t *testing.T) {
7375
}
7476
defer simr.Destroy()
7577

78+
secretCachingClient, err := client.New(testEnv.Manager.GetConfig(), client.Options{
79+
HTTPClient: testEnv.Manager.GetHTTPClient(),
80+
Cache: &client.CacheOptions{
81+
Reader: testEnv.Manager.GetCache(),
82+
},
83+
})
84+
if err != nil {
85+
panic("unable to create secret caching client")
86+
}
87+
88+
tracker, err := remote.NewClusterCacheTracker(
89+
testEnv.Manager,
90+
remote.ClusterCacheTrackerOptions{
91+
SecretCachingClient: secretCachingClient,
92+
ControllerName: "testvspherevm-manager",
93+
},
94+
)
95+
if err != nil {
96+
t.Fatalf("unable to setup ClusterCacheTracker: %v", err)
97+
}
98+
99+
controllerOpts := controller.Options{MaxConcurrentReconciles: 10}
100+
101+
if err := (&remote.ClusterCacheReconciler{
102+
Client: testEnv.Manager.GetClient(),
103+
Tracker: tracker,
104+
}).SetupWithManager(ctx, testEnv.Manager, controllerOpts); err != nil {
105+
panic(fmt.Sprintf("unable to create ClusterCacheReconciler controller: %v", err))
106+
}
107+
76108
create := func(netSpec infrav1.NetworkSpec) func() {
77109
return func() {
78110
vsphereCluster = &infrav1.VSphereCluster{
@@ -177,8 +209,9 @@ func TestReconcileNormal_WaitingForIPAddrAllocation(t *testing.T) {
177209
Logger: log.Log,
178210
}
179211
return vmReconciler{
180-
ControllerContext: controllerContext,
181-
VMService: vmService,
212+
ControllerContext: controllerContext,
213+
VMService: vmService,
214+
remoteClusterCacheTracker: tracker,
182215
}
183216
}
184217

0 commit comments

Comments
 (0)