Merge pull request vmware-tanzu#45 from bryanv/bryanv/vmpub-api-reader

Use APIReader client to Get() VirtualMachinePublishRequest

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,