Use APIReader client to Get() VirtualMachinePublishRequest

Unlike ConfigMaps and Secrets which we cannot cache because their quantity
and size will exceed our container's memory limit, the pub req shouldn't be
cached because of how the controller is implemented so make that explicit in
the controller.

Author: bryanv

controllers/virtualmachinepublishrequest/virtualmachinepublishrequest_controller.go

@@ -70,6 +70,7 @@ func AddToManager(ctx *context.ControllerManagerContext, mgr manager.Manager) er
 
 	r := NewReconciler(
 		mgr.GetClient(),
+		mgr.GetAPIReader(),
 		ctrl.Log.WithName("controllers").WithName(controlledTypeName),
 		record.New(mgr.GetEventRecorderFor(controllerNameLong)),
 		ctx.VMProvider,
@@ -124,12 +125,14 @@ func vmiToVMPubMapperFn(ctx *context.ControllerManagerContext, c client.Client)
 
 func NewReconciler(
 	client client.Client,
+	apiReader client.Reader,
 	logger logr.Logger,
 	recorder record.Recorder,
 	vmProvider vmprovider.VirtualMachineProviderInterface) *Reconciler {
 
 	return &Reconciler{
 		Client:     client,
+		apiReader:  apiReader,
 		Logger:     logger,
 		Recorder:   recorder,
 		VMProvider: vmProvider,
@@ -140,6 +143,7 @@ func NewReconciler(
 // Reconciler reconciles a VirtualMachinePublishRequest object.
 type Reconciler struct {
 	client.Client
+	apiReader  client.Reader
 	Logger     logr.Logger
 	Recorder   record.Recorder
 	VMProvider vmprovider.VirtualMachineProviderInterface
@@ -167,7 +171,11 @@ func requeueDelay(ctx *context.VirtualMachinePublishRequestContext) time.Duratio
 
 func (r *Reconciler) Reconcile(ctx goctx.Context, req ctrl.Request) (_ ctrl.Result, reterr error) {
 	vmPublishReq := &vmopv1alpha1.VirtualMachinePublishRequest{}
-	err := r.Get(ctx, req.NamespacedName, vmPublishReq)
+	// Get the VirtualMachinePublishRequest directly from the API server - bypassing the cache of the
+	// regular client - to avoid potentially stale objects from cache. We rely on the up-to-date Status
+	// when sending publish VM requests during reconciliation.
+	// Update() of stale object will be rejected by API server and result in unnecessary reconciles.
+	err := r.apiReader.Get(ctx, req.NamespacedName, vmPublishReq)
 	if err != nil {
 		// Delete registered metrics if the resource is not found.
 		if apiErrors.IsNotFound(err) {

controllers/virtualmachinepublishrequest/virtualmachinepublishrequest_controller_unit_test.go

@@ -66,6 +66,7 @@ func unitTestsReconcile() {
 	JustBeforeEach(func() {
 		ctx = suite.NewUnitTestContextForController(initObjects...)
 		reconciler = virtualmachinepublishrequest.NewReconciler(
+			ctx.Client,
 			ctx.Client,
 			ctx.Logger,
 			ctx.Recorder,

pkg/manager/manager.go

@@ -26,7 +26,6 @@ import (
 	netopv1alpha1 "github.com/vmware-tanzu/vm-operator/external/net-operator/api/v1alpha1"
 	cnsv1alpha1 "github.com/vmware-tanzu/vm-operator/external/vsphere-csi-driver/pkg/syncer/cnsoperator/apis/cnsnodevmattachment/v1alpha1"
 	"github.com/vmware-tanzu/vm-operator/pkg/context"
-	"github.com/vmware-tanzu/vm-operator/pkg/lib"
 	"github.com/vmware-tanzu/vm-operator/pkg/record"
 	"github.com/vmware-tanzu/vm-operator/pkg/vmprovider/providers/vsphere"
 )
@@ -58,13 +57,6 @@ func New(opts Options) (Manager, error) {
 	// the cache for ConfigMaps and Secrets so they are looked up from API sever directly.
 	cacheDisabledObjects := []client.Object{&corev1.ConfigMap{}, &corev1.Secret{}}
 
-	// Look up VirtualMachinePublishRequest resources from API server directly to avoid get stale objects from cache,
-	// We kind of rely on the up-to-date status when sending publish VM requests during reconciling VMPublishRequests.
-	// Update stale object will be rejected by API server and result in unnecessary reconciles.
-	if lib.IsWCPVMImageRegistryEnabled() {
-		cacheDisabledObjects = append(cacheDisabledObjects, &vmopv1.VirtualMachinePublishRequest{})
-	}
-
 	// Build the controller manager.
 	mgr, err := ctrlmgr.New(opts.KubeConfig, ctrlmgr.Options{
 		Scheme:                  opts.Scheme,