Avoid needlessly VC client logout when creds haven't changed

Only logout an existing VC client when the creds have actually
changed. Previously, when the Infra Secret controller first
starts it would have always logged the VC client if present.
While this race has been present for a long time, the addition
of the VM Watcher Service made this more likely now since that
will usually create a VC client before the Infra secret has
started.

Move the VC creds secret into a config param. Previously, the
Infra Secret controller was hardcoding the secret name, while
we'd otherwise obtain the name from the VMOP ConfigMap. This
secret name hasn't changed since day 1 so just move it into
our config.

In the fake VM provider, add gets of the config from the context
since we now expect and require the config to be there.

controllers/contentlibrary/utils/controller_builder_test.go

@@ -48,7 +48,7 @@ var _ = Describe("AddToManager",
 		)
 
 		BeforeEach(func() {
-			parentCtx = pkgcfg.NewContext()
+			parentCtx = pkgcfg.NewContextWithDefaultConfig()
 		})
 
 		JustBeforeEach(func() {

controllers/infra/secret/infra_secret_controller.go

@@ -10,30 +10,24 @@ import (
 
 	"github.com/go-logr/logr"
 	corev1 "k8s.io/api/core/v1"
-	"sigs.k8s.io/controller-runtime/pkg/client"
+	ctrl "sigs.k8s.io/controller-runtime"
+	ctrlclient "sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller"
 	"sigs.k8s.io/controller-runtime/pkg/event"
 	"sigs.k8s.io/controller-runtime/pkg/handler"
+	"sigs.k8s.io/controller-runtime/pkg/manager"
 	"sigs.k8s.io/controller-runtime/pkg/predicate"
 	"sigs.k8s.io/controller-runtime/pkg/source"
 
-	ctrl "sigs.k8s.io/controller-runtime"
-	"sigs.k8s.io/controller-runtime/pkg/manager"
-
 	pkgcfg "github.com/vmware-tanzu/vm-operator/pkg/config"
 	pkgctx "github.com/vmware-tanzu/vm-operator/pkg/context"
 	pkgmgr "github.com/vmware-tanzu/vm-operator/pkg/manager"
 	"github.com/vmware-tanzu/vm-operator/pkg/record"
 	kubeutil "github.com/vmware-tanzu/vm-operator/pkg/util/kube"
 )
 
-const (
-	// VcCredsSecretName is the credential secret that stores the VM operator service provider user credentials.
-	VcCredsSecretName = "wcp-vmop-sa-vc-auth" //nolint:gosec
-)
-
 type provider interface {
-	ResetVcClient(ctx context.Context)
+	UpdateVcCreds(ctx context.Context, data map[string][]byte) error
 }
 
 // AddToManager adds this package's controller to the provided manager.
@@ -45,25 +39,34 @@ func AddToManager(ctx *pkgctx.ControllerManagerContext, mgr manager.Manager) err
 		controllerNameLong  = fmt.Sprintf("%s/%s/%s", ctx.Namespace, ctx.Name, controllerNameShort)
 	)
 
+	vcCredsKey := ctrlclient.ObjectKey{
+		Name:      pkgcfg.FromContext(ctx).VCCredsSecretName,
+		Namespace: ctx.Namespace,
+	}
+
+	// This controller only watches for Secrets in the pod namespace.
+	cache, err := pkgmgr.NewNamespacedCacheForObject(
+		mgr,
+		&ctx.SyncPeriod,
+		controlledType,
+		vcCredsKey.Namespace)
+	if err != nil {
+		return err
+	}
+
 	r := NewReconciler(
 		ctx,
-		mgr.GetClient(),
+		cache,
 		ctrl.Log.WithName("controllers").WithName(controllerName),
 		record.New(mgr.GetEventRecorderFor(controllerNameLong)),
-		ctx.Namespace,
 		ctx.VMProvider,
+		vcCredsKey,
 	)
 
-	c, err := controller.New(controllerName, mgr, controller.Options{Reconciler: r})
-	if err != nil {
-		return err
-	}
-
-	cache, err := pkgmgr.NewNamespacedCacheForObject(
-		mgr,
-		&ctx.SyncPeriod,
-		controlledType,
-		r.vmOpNamespace)
+	c, err := controller.New(controllerName, mgr, controller.Options{
+		Reconciler:              r,
+		MaxConcurrentReconciles: 1,
+	})
 	if err != nil {
 		return err
 	}
@@ -74,10 +77,10 @@ func AddToManager(ctx *pkgctx.ControllerManagerContext, mgr manager.Manager) err
 		&handler.TypedEnqueueRequestForObject[*corev1.Secret]{},
 		predicate.TypedFuncs[*corev1.Secret]{
 			CreateFunc: func(e event.TypedCreateEvent[*corev1.Secret]) bool {
-				return e.Object.GetName() == VcCredsSecretName
+				return e.Object.GetName() == vcCredsKey.Name
 			},
 			UpdateFunc: func(e event.TypedUpdateEvent[*corev1.Secret]) bool {
-				return e.ObjectOld.GetName() == VcCredsSecretName
+				return e.ObjectOld.GetName() == vcCredsKey.Name
 			},
 			DeleteFunc: func(e event.TypedDeleteEvent[*corev1.Secret]) bool {
 				return false
@@ -88,50 +91,54 @@ func AddToManager(ctx *pkgctx.ControllerManagerContext, mgr manager.Manager) err
 		},
 		kubeutil.TypedResourceVersionChangedPredicate[*corev1.Secret]{},
 	))
-
 }
 
 func NewReconciler(
 	ctx context.Context,
-	client client.Client,
+	secretReader ctrlclient.Reader,
 	logger logr.Logger,
 	recorder record.Recorder,
-	vmOpNamespace string,
-	provider provider) *Reconciler {
+	provider provider,
+	vcCredsKey ctrlclient.ObjectKey) *Reconciler {
+
 	return &Reconciler{
-		Context:       ctx,
-		Client:        client,
-		Logger:        logger,
-		Recorder:      recorder,
-		vmOpNamespace: vmOpNamespace,
-		provider:      provider,
+		Context:      ctx,
+		SecretReader: secretReader,
+		Logger:       logger,
+		Recorder:     recorder,
+		provider:     provider,
+		vcCredsKey:   vcCredsKey,
 	}
 }
 
 type Reconciler struct {
-	client.Client
-	Context       context.Context
-	Logger        logr.Logger
-	Recorder      record.Recorder
-	vmOpNamespace string
-	provider      provider
+	Context      context.Context
+	SecretReader ctrlclient.Reader
+	Logger       logr.Logger
+	Recorder     record.Recorder
+	provider     provider
+	vcCredsKey   ctrlclient.ObjectKey
 }
 
 // +kubebuilder:rbac:groups="",resources=secrets,verbs=get;list;watch
 
 func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
 	ctx = pkgcfg.JoinContext(ctx, r.Context)
 
-	if req.Name == VcCredsSecretName && req.Namespace == r.vmOpNamespace {
-		r.reconcileVcCreds(ctx, req)
-		return ctrl.Result{}, nil
+	if req.NamespacedName == r.vcCredsKey {
+		return ctrl.Result{}, r.reconcileVcCreds(ctx)
 	}
 
 	r.Logger.Error(nil, "Reconciling unexpected object", "req", req.NamespacedName)
 	return ctrl.Result{}, nil
 }
 
-func (r *Reconciler) reconcileVcCreds(ctx context.Context, req ctrl.Request) {
-	r.Logger.Info("Reconciling updated VM Operator credentials", "secret", req.NamespacedName)
-	r.provider.ResetVcClient(ctx)
+func (r *Reconciler) reconcileVcCreds(ctx context.Context) error {
+	secret := corev1.Secret{}
+	if err := r.SecretReader.Get(ctx, r.vcCredsKey, &secret); err != nil {
+		return ctrlclient.IgnoreNotFound(err)
+	}
+
+	r.Logger.Info("Reconciling updated VM Operator credentials", "secret", r.vcCredsKey)
+	return r.provider.UpdateVcCreds(ctx, secret.Data)
 }

controllers/infra/secret/infra_secret_controller_intg_test.go

@@ -15,7 +15,7 @@ import (
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
-	"github.com/vmware-tanzu/vm-operator/controllers/infra/secret"
+	pkgcfg "github.com/vmware-tanzu/vm-operator/pkg/config"
 	"github.com/vmware-tanzu/vm-operator/pkg/constants/testlabels"
 	"github.com/vmware-tanzu/vm-operator/test/builder"
 )
@@ -57,15 +57,16 @@ func intgTestsReconcile() {
 			obj = &corev1.Secret{
 				ObjectMeta: metav1.ObjectMeta{
 					Namespace: ctx.PodNamespace,
-					Name:      secret.VcCredsSecretName,
+					Name:      pkgcfg.FromContext(suite.Context).VCCredsSecretName,
 				},
 			}
 		})
 
 		JustBeforeEach(func() {
 			provider.Lock()
-			provider.ResetVcClientFn = func(_ context.Context) {
+			provider.UpdateVcCredsFn = func(_ context.Context, _ map[string][]byte) error {
 				atomic.AddInt32(&called, 1)
+				return nil
 			}
 			provider.Unlock()
 			Expect(ctx.Client.Create(ctx, obj)).To(Succeed())
@@ -121,7 +122,7 @@ func intgTestsReconcile() {
 				})
 				It("should not be reconciled", func() {
 					Consistently(func() int32 {
-						// NOTE: ResetVcClient() won't be called during the reconcile because the
+						// NOTE: UpdateVcCreds() won't be called during the reconcile because the
 						// obj namespace won't match the pod's namespace. It is bad news if you see
 						// "Reconciling unexpected object" in the logs.
 						return atomic.LoadInt32(&called)

controllers/infra/secret/infra_secret_controller_suite_test.go

@@ -12,14 +12,16 @@ import (
 	ctrlmgr "sigs.k8s.io/controller-runtime/pkg/manager"
 
 	"github.com/vmware-tanzu/vm-operator/controllers/infra/secret"
+	pkgcfg "github.com/vmware-tanzu/vm-operator/pkg/config"
 	pkgctx "github.com/vmware-tanzu/vm-operator/pkg/context"
 	providerfake "github.com/vmware-tanzu/vm-operator/pkg/providers/fake"
 	"github.com/vmware-tanzu/vm-operator/test/builder"
 )
 
 var provider = providerfake.NewVMProvider()
 
-var suite = builder.NewTestSuiteForController(
+var suite = builder.NewTestSuiteForControllerWithContext(
+	pkgcfg.NewContextWithDefaultConfig(),
 	secret.AddToManager,
 	func(ctx *pkgctx.ControllerManagerContext, _ ctrlmgr.Manager) error {
 		ctx.VMProvider = provider

pkg/config/config.go

@@ -143,6 +143,12 @@ type Config struct {
 	//
 	// Defaults to "direct".
 	FastDeployMode string
+
+	// VCCredsSecretName is the name of the secret in the pod namespace that
+	// contains the VC credentials.
+	//
+	// Defaults to "wcp-vmop-sa-vc-auth".
+	VCCredsSecretName string
 }
 
 // GetMaxDeployThreadsOnProvider returns MaxDeployThreadsOnProvider if it is >0

pkg/config/default.go

@@ -43,6 +43,7 @@ func Default() Config {
 		AsyncCreateEnabled:           true,
 		MemStatsPeriod:               10 * time.Minute,
 		FastDeployMode:               pkgconst.FastDeployModeDirect,
+		VCCredsSecretName:            pkgconst.VCCredsSecretName,
 		CreateVMRequeueDelay:         10 * time.Second,
 		PoweredOnVMHasIPRequeueDelay: 10 * time.Second,
 		SyncImageRequeueDelay:        10 * time.Second,

pkg/config/env.go

@@ -33,6 +33,7 @@ func FromEnv() Config {
 	setBool(env.AsyncCreateEnabled, &config.AsyncCreateEnabled)
 	setDuration(env.MemStatsPeriod, &config.MemStatsPeriod)
 	setString(env.FastDeployMode, &config.FastDeployMode)
+	setString(env.VCCredsSecretName, &config.VCCredsSecretName)
 
 	setDuration(env.InstanceStoragePVPlacementFailedTTL, &config.InstanceStorage.PVPlacementFailedTTL)
 	setFloat64(env.InstanceStorageJitterMaxFactor, &config.InstanceStorage.JitterMaxFactor)

pkg/config/env/env.go

@@ -29,6 +29,7 @@ const (
 	AsyncSignalEnabled
 	AsyncCreateEnabled
 	FastDeployMode
+	VCCredsSecretName
 	InstanceStoragePVPlacementFailedTTL
 	InstanceStorageJitterMaxFactor
 	InstanceStorageSeedRequeueDuration
@@ -119,6 +120,8 @@ func (n VarName) String() string {
 		return "ASYNC_CREATE_ENABLED"
 	case FastDeployMode:
 		return "FAST_DEPLOY_MODE"
+	case VCCredsSecretName:
+		return "VC_CREDS_SECRET_NAME"
 	case InstanceStoragePVPlacementFailedTTL:
 		return "INSTANCE_STORAGE_PV_PLACEMENT_FAILED_TTL"
 	case InstanceStorageJitterMaxFactor:

pkg/config/env_test.go

@@ -81,6 +81,7 @@ var _ = Describe(
 					Expect(os.Setenv("ASYNC_SIGNAL_ENABLED", "false")).To(Succeed())
 					Expect(os.Setenv("ASYNC_CREATE_ENABLED", "false")).To(Succeed())
 					Expect(os.Setenv("FAST_DEPLOY_MODE", pkgconst.FastDeployModeLinked)).To(Succeed())
+					Expect(os.Setenv("VC_CREDS_SECRET_NAME", pkgconst.VCCredsSecretName)).To(Succeed())
 					Expect(os.Setenv("LEADER_ELECTION_ID", "115")).To(Succeed())
 					Expect(os.Setenv("POD_NAME", "116")).To(Succeed())
 					Expect(os.Setenv("POD_NAMESPACE", "117")).To(Succeed())
@@ -133,6 +134,7 @@ var _ = Describe(
 						AsyncSignalEnabled:           false,
 						AsyncCreateEnabled:           false,
 						FastDeployMode:               pkgconst.FastDeployModeLinked,
+						VCCredsSecretName:            pkgconst.VCCredsSecretName,
 						LeaderElectionID:             "115",
 						PodName:                      "116",
 						PodNamespace:                 "117",

pkg/constants/constants.go

@@ -73,4 +73,8 @@ const (
 	// FastDeployModeLinked is a fast deploy mode. See FastDeployAnnotationKey
 	// for more information.
 	FastDeployModeLinked = "linked"
+
+	// VCCredsSecretName is the name of the secret in the pod namespace
+	// that contains the VC credentials.
+	VCCredsSecretName = "wcp-vmop-sa-vc-auth" //nolint:gosec
 )