vsphere: Cache REST API sessions to prevent excessive vCenter logouts

This commit fixes a critical issue where the machine-api-operator was
creating and destroying vCenter REST API sessions on every machine
reconciliation, causing excessive login/logout cycles that pollute
vCenter audit logs and create unnecessary session churn.

Root Cause:
The WithRestClient() and WithCachingTagsManager() wrapper functions
were creating new REST sessions, performing operations, and immediately
logging out on every invocation. With hundreds of machines reconciling
periodically, this created a constant stream of login/logout events.

Solution (inspired by cluster-api-provider-vsphere):
- Add TagManager field to Session struct to cache REST client
- Initialize and cache REST client during session creation (GetOrCreate)
- Validate both SOAP and REST session health before reusing cached sessions
- Add GetCachingTagsManager() helper for direct access to cached tag manager
- Update reconcileRegionAndZoneLabels() to use cached tag manager
- Update reconcileTags() to use cached tag manager
- Deprecate WithRestClient() and WithCachingTagsManager() for backward compatibility

Key Changes:
1. pkg/controller/vsphere/session/session.go:
   - Added TagManager *tags.Manager field to Session struct
   - Modified GetOrCreate() to create and cache REST client once
   - Added dual session validation (SOAP + REST) before reusing sessions
   - Added GetCachingTagsManager() method for direct access
   - Deprecated old wrapper functions with migration guidance

2. pkg/controller/vsphere/reconciler.go:
   - Updated reconcileRegionAndZoneLabels() to use GetCachingTagsManager()
   - Updated reconcileTags() to use GetCachingTagsManager()
   - Eliminated callback pattern in favor of direct access

Impact:
- Eliminates excessive vCenter login/logout cycles
- Reduces vCenter session churn from O(reconciliations) to O(1) per MAPI instance
- Improves performance by removing authentication overhead on every tag operation
- REST session now lives as long as SOAP session (until invalidation)

Reference Implementation:
https://github.com/kubernetes-sigs/cluster-api-provider-vsphere/blob/main/pkg/session/session.go

Backward Compatibility:
The deprecated wrapper functions are maintained with warning logs to support
existing test code. All production code paths now use the new pattern.

Fixes: Excessive vCenter logout events reported by customers
Signed-off-by: Claude Code Assistant <[email protected]>

Author: jcpowermac

pkg/controller/vsphere/reconciler.go

@@ -594,15 +594,10 @@ func (r *Reconciler) reconcileRegionAndZoneLabels(vm *virtualMachine) error {
 	regionLabel := r.vSphereConfig.Labels.Region
 	zoneLabel := r.vSphereConfig.Labels.Zone
 
-	var res map[string]string
-
-	err := r.session.WithCachingTagsManager(vm.Context, func(c *session.CachingTagsManager) error {
-		var err error
-		res, err = vm.getRegionAndZone(c, regionLabel, zoneLabel)
-
-		return err
-	})
-
+	// Use cached tag manager to avoid creating new REST sessions.
+	// This eliminates excessive vCenter login/logout cycles.
+	tagManager := r.session.GetCachingTagsManager()
+	res, err := vm.getRegionAndZone(tagManager, regionLabel, zoneLabel)
 	if err != nil {
 		return err
 	}
@@ -1599,32 +1594,29 @@ func (vm *virtualMachine) getPowerState() (types.VirtualMachinePowerState, error
 // reconcileTags ensures that the required tags are present on the virtual machine, eg the Cluster ID
 // that is used by the installer on cluster deletion to ensure ther are no leaked resources.
 func (vm *virtualMachine) reconcileTags(ctx context.Context, sessionInstance *session.Session, machine *machinev1.Machine, providerSpec *machinev1.VSphereMachineProviderSpec) error {
-	if err := sessionInstance.WithCachingTagsManager(vm.Context, func(c *session.CachingTagsManager) error {
-		klog.Infof("%v: Reconciling attached tags", machine.GetName())
-
-		clusterID := machine.Labels[machinev1.MachineClusterIDLabel]
-		tagIDs := []string{clusterID}
-		tagIDs = append(tagIDs, providerSpec.TagIDs...)
-		klog.Infof("%v: Reconciling %s tags to vm", machine.GetName(), tagIDs)
-		for _, tagID := range tagIDs {
-			attached, err := vm.checkAttachedTag(ctx, tagID, c)
-			if err != nil {
-				return err
-			}
+	// Use cached tag manager to avoid creating new REST sessions.
+	// This eliminates excessive vCenter login/logout cycles.
+	tagManager := sessionInstance.GetCachingTagsManager()
+	klog.Infof("%v: Reconciling attached tags", machine.GetName())
+
+	clusterID := machine.Labels[machinev1.MachineClusterIDLabel]
+	tagIDs := []string{clusterID}
+	tagIDs = append(tagIDs, providerSpec.TagIDs...)
+	klog.Infof("%v: Reconciling %s tags to vm", machine.GetName(), tagIDs)
+	for _, tagID := range tagIDs {
+		attached, err := vm.checkAttachedTag(ctx, tagID, tagManager)
+		if err != nil {
+			return err
+		}
 
-			if !attached {
-				klog.Infof("%v: Attaching %s tag to vm", machine.GetName(), tagID)
-				// the tag should already be created by installer or the administrator
-				if err := c.AttachTag(ctx, tagID, vm.Ref); err != nil {
-					return err
-				}
+		if !attached {
+			klog.Infof("%v: Attaching %s tag to vm", machine.GetName(), tagID)
+			// the tag should already be created by installer or the administrator
+			if err := tagManager.AttachTag(ctx, tagID, vm.Ref); err != nil {
+				return err
 			}
 		}
-		return nil
-	}); err != nil {
-		return err
 	}
-
 	return nil
 }
 

pkg/controller/vsphere/session/session.go

@@ -46,10 +46,14 @@ const (
 )
 
 // Session is a vSphere session with a configured Finder.
+// This implementation is inspired by cluster-api-provider-vsphere's session caching pattern
+// to avoid excessive vCenter login/logout cycles for REST API operations.
+// Reference: https://github.com/kubernetes-sigs/cluster-api-provider-vsphere/blob/main/pkg/session/session.go
 type Session struct {
 	*govmomi.Client
 	Finder     *find.Finder
 	Datacenter *object.Datacenter
+	TagManager *tags.Manager
 
 	username string
 	password string
@@ -80,13 +84,40 @@ func GetOrCreate(
 
 	sessionKey := server + username + datacenter
 	if session, ok := sessionCache[sessionKey]; ok {
+		// Check both SOAP and REST session validity before reusing cached session.
+		// This prevents reusing sessions where one connection type has expired.
+		// Pattern adapted from cluster-api-provider-vsphere:
+		// https://github.com/kubernetes-sigs/cluster-api-provider-vsphere/blob/main/pkg/session/session.go#L132-L149
 		sessionActive, err := session.SessionManager.SessionIsActive(ctx)
 		if err != nil {
-			klog.Errorf("Error performing session check request to vSphere: %v", err)
+			klog.Errorf("Error performing SOAP session check request to vSphere: %v", err)
 		}
-		if sessionActive {
+
+		var restSessionActive bool
+		if session.TagManager != nil {
+			restSession, err := session.TagManager.Session(ctx)
+			if err != nil {
+				klog.Errorf("Error performing REST session check request to vSphere: %v", err)
+			}
+			restSessionActive = restSession != nil
+		}
+
+		if sessionActive && restSessionActive {
+			klog.V(3).Infof("Found active cached vSphere session with valid SOAP and REST connections")
 			return &session, nil
 		}
+
+		// If either session is invalid, logout both to clean up
+		if session.TagManager != nil {
+			klog.Infof("Logging out inactive REST session")
+			if err := session.TagManager.Logout(ctx); err != nil {
+				klog.Errorf("Failed to logout REST session: %v", err)
+			}
+		}
+		klog.Infof("Logging out inactive SOAP session")
+		if err := session.Client.Logout(ctx); err != nil {
+			klog.Errorf("Failed to logout SOAP session: %v", err)
+		}
 	}
 	klog.Infof("No existing vCenter session found, creating new session")
 
@@ -127,6 +158,20 @@ func GetOrCreate(
 	session.Datacenter = dc
 	session.Finder.SetDatacenter(dc)
 
+	// Create and cache REST client for tag operations.
+	// This prevents creating a new REST session on every tag operation.
+	// Pattern adapted from cluster-api-provider-vsphere:
+	// https://github.com/kubernetes-sigs/cluster-api-provider-vsphere/blob/main/pkg/session/session.go#L196-L205
+	restClient := rest.NewClient(session.Client.Client)
+	if err := restClient.Login(ctx, url.UserPassword(username, password)); err != nil {
+		// Cleanup SOAP session on REST login failure
+		if logoutErr := client.Logout(ctx); logoutErr != nil {
+			klog.Errorf("Failed to logout SOAP session after REST login failure: %v", logoutErr)
+		}
+		return nil, fmt.Errorf("unable to login REST client to vCenter: %w", err)
+	}
+	session.TagManager = tags.NewManager(restClient)
+
 	// Cache the session.
 	sessionCache[sessionKey] = session
 
@@ -206,7 +251,21 @@ func (s *Session) GetTask(ctx context.Context, taskRef string) (*mo.Task, error)
 	return &obj, nil
 }
 
+// GetCachingTagsManager returns a CachingTagsManager that wraps the cached TagManager.
+// This replaces the previous WithCachingTagsManager pattern which created new sessions
+// on every call. The returned manager uses the session's cached REST client.
+func (s *Session) GetCachingTagsManager() *CachingTagsManager {
+	return newTagsCachingClient(s.TagManager, s.sessionKey)
+}
+
+// WithRestClient is deprecated. Use s.TagManager directly instead.
+// This function is maintained for backward compatibility but creates excessive
+// vCenter login/logout cycles. Migration path: replace callback pattern with
+// direct access to s.TagManager.
+//
+// Deprecated: Use s.TagManager for direct REST client access.
 func (s *Session) WithRestClient(ctx context.Context, f func(c *rest.Client) error) error {
+	klog.Warning("WithRestClient is deprecated and causes excessive vCenter logouts. Use s.TagManager directly instead.")
 	c := rest.NewClient(s.Client.Client)
 
 	user := url.UserPassword(s.username, s.password)
@@ -223,7 +282,14 @@ func (s *Session) WithRestClient(ctx context.Context, f func(c *rest.Client) err
 	return f(c)
 }
 
+// WithCachingTagsManager is deprecated. Use s.GetCachingTagsManager() instead.
+// This function is maintained for backward compatibility but creates excessive
+// vCenter login/logout cycles. Migration path: replace callback pattern with
+// direct call to s.GetCachingTagsManager().
+//
+// Deprecated: Use s.GetCachingTagsManager() for cached tag manager access.
 func (s *Session) WithCachingTagsManager(ctx context.Context, f func(m *CachingTagsManager) error) error {
+	klog.Warning("WithCachingTagsManager is deprecated and causes excessive vCenter logouts. Use s.GetCachingTagsManager() instead.")
 	c := rest.NewClient(s.Client.Client)
 
 	user := url.UserPassword(s.username, s.password)