Merge pull request vmware-tanzu#704 from bryanv/bryanv/volume-use-ctr…

…l-client-cache-pvcs 🐛 Use cached Get() of PVCs in Volume controller
aruneshpa · Sep 10, 2024 · fdc2d21 · fdc2d21
2 parents 9652755 + a7b9c08
commit fdc2d21
Show file tree

Hide file tree

Showing 2 changed files with 1 addition and 27 deletions.
diff --git a/controllers/virtualmachine/volume/volume_controller.go b/controllers/virtualmachine/volume/volume_controller.go
@@ -70,7 +70,6 @@ func AddToManager(ctx *pkgctx.ControllerManagerContext, mgr manager.Manager) err
 	r := NewReconciler(
 		ctx,
 		mgr.GetClient(),
-		mgr.GetAPIReader(),
 		ctrl.Log.WithName("controllers").WithName("volume"),
 		record.New(mgr.GetEventRecorderFor(controllerNameLong)),
 		ctx.VMProvider,
@@ -177,14 +176,12 @@ func AddToManager(ctx *pkgctx.ControllerManagerContext, mgr manager.Manager) err
 func NewReconciler(
 	ctx context.Context,
 	client client.Client,
-	reader client.Reader,
 	logger logr.Logger,
 	recorder record.Recorder,
 	vmProvider providers.VirtualMachineProviderInterface) *Reconciler {
 	return &Reconciler{
 		Context:    ctx,
 		Client:     client,
-		reader:     reader,
 		logger:     logger,
 		recorder:   recorder,
 		VMProvider: vmProvider,
@@ -195,7 +192,6 @@ var _ reconcile.Reconciler = &Reconciler{}
 
 type Reconciler struct {
 	client.Client
-	reader     client.Reader
 	Context    context.Context
 	logger     logr.Logger
 	recorder   record.Recorder
@@ -666,7 +662,7 @@ func (r *Reconciler) processAttachments(
 			if volume.PersistentVolumeClaim.ClaimName == attachment.Spec.VolumeName {
 				volumeStatus := attachmentToVolumeStatus(volume.Name, attachment)
 				volumeStatus.Used = existingManagedVolUsage[volume.Name]
-				if err := updateVolumeStatusWithLimit(ctx, r.reader, *volume.PersistentVolumeClaim, &volumeStatus); err != nil {
+				if err := updateVolumeStatusWithLimit(ctx, r.Client, *volume.PersistentVolumeClaim, &volumeStatus); err != nil {
 					ctx.Logger.Error(err, "failed to get volume status limit")
 				}
 				volumeStatuses = append(volumeStatuses, volumeStatus)

diff --git a/controllers/virtualmachine/volume/volume_controller_unit_test.go b/controllers/virtualmachine/volume/volume_controller_unit_test.go
@@ -45,24 +45,6 @@ func (f *testFailClient) Create(ctx context.Context, obj client.Object, opts ...
 	return apierrors.NewForbidden(schema.GroupResource{}, "", errors.New("insufficient quota for creating PVC"))
 }
 
-type noPVCReader struct {
-	client.Client
-}
-
-func (c *noPVCReader) Get(ctx context.Context, key client.ObjectKey, obj client.Object, opts ...client.GetOption) error {
-	if _, ok := obj.(*corev1.PersistentVolumeClaim); ok {
-		panic("Don't Get() PersistentVolumeClaim with this Client!")
-	}
-	return c.Client.Get(ctx, key, obj, opts...)
-}
-
-func (c *noPVCReader) List(ctx context.Context, list client.ObjectList, opts ...client.ListOption) error {
-	if _, ok := list.(*corev1.PersistentVolumeClaimList); ok {
-		panic("Don't List() PersistentVolumeClaim with this Client!")
-	}
-	return c.Client.List(ctx, list, opts...)
-}
-
 func unitTests() {
 	Describe(
 		"Reconcile",
@@ -164,12 +146,8 @@ func unitTestsReconcile() {
 				}).
 			Build()
 
-		// Any PVC Get/List should not use this client so we don't start an informer.
-		noPVCClient := &noPVCReader{ctx.Client}
-
 		reconciler = volume.NewReconciler(
 			ctx,
-			noPVCClient,
 			ctx.Client,
 			ctx.Logger,
 			ctx.Recorder,