fix(OCPBUGS-77530): remove pause mechanism from backup/restore plugins

Remove the pause/unpause logic for HostedClusters, NodePools and CAPI
resources (clusters, machines) during backup and restore operations.

The pause mechanism was causing a leak where resources could get stuck
in a paused state when a HostedCluster deletion races with an OADP
backup. The HyperShift nodepool controller's deletion path returns
early before reaching the pause/unpause logic, leaving resources with
spec.pausedUntil set indefinitely.

Changes:
- Remove pauseAll/unPauseAll methods and progress-based pause logic
  from BackupPlugin
- Remove UpdateHostedCluster, UpdateNodepools, WaitForPausedPropagated
  and updateHypershiftResource functions from common utils
- Remove CAPI paused annotation (cluster.x-k8s.io/paused) from backup
  and restore flows
- Remove pause audit annotations (paused-by, paused-at) and related
  constants
- Remove shouldLogProgress throttling (only used for pause logging)
- Clean up BackupPluginValidator by removing HCPaused/NPaused fields
- Remove all related tests

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Juan Manuel Parrilla Madrid <jparrill@redhat.com>

Author: jparrill

pkg/common/types.go

@@ -3,7 +3,6 @@ package common
 import "time"
 
 const (
-	CAPIPausedAnnotationName    string = "cluster.x-k8s.io/paused"
 	CommonBackupAnnotationName  string = "hypershift.openshift.io/common-backup-plugin"
 	CommonRestoreAnnotationName string = "hypershift.openshift.io/common-restore-plugin"
 	FSBackupLabelName           string = "hypershift.openshift.io/fsbackup"
@@ -15,22 +14,15 @@ const (
 	PluginConfigMapName string = "hypershift-oadp-plugin-config"
 
 	// Default values for the backup plugin.
-	defaultPVBackupTimeout      time.Duration = 30 * time.Minute
-	defaultPVBackupCheckPace    time.Duration = 10 * time.Second
-	defaultWaitForPausedTimeout time.Duration = 2 * time.Minute
-	defaultWaitForTimeout       time.Duration = 5 * time.Minute
+	defaultPVBackupTimeout   time.Duration = 30 * time.Minute
+	defaultPVBackupCheckPace time.Duration = 10 * time.Second
 
 	DefaultK8sSAFilePath string = "/var/run/secrets/kubernetes.io/serviceaccount"
 	KubevirtRHCOSLabel   string = "hypershift.openshift.io/is-kubevirt-rhcos"
 
 	// Integration with Hypershift, more info here: https://github.com/openshift/hypershift/pull/6195
 	HostedClusterRestoredFromBackupAnnotation string = "hypershift.openshift.io/restored-from-backup"
 
-	// OADP Plugin audit annotations for pause operations
-	OADPPausedByAnnotation   string = "oadp.openshift.io/paused-by"
-	OADPPausedAtAnnotation   string = "oadp.openshift.io/paused-at"
-	HypershiftOADPPluginName string = "hypershift-oadp-plugin"
-
 	// hypershift/cluster-api kinds
 	HostedClusterKind         string = "HostedCluster"
 	HostedControlPlaneKind    string = "HostedControlPlane"

pkg/common/utils.go

@@ -24,7 +24,6 @@ import (
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/wait"
 	"k8s.io/client-go/rest"
-	"k8s.io/utils/ptr"
 	cr "sigs.k8s.io/controller-runtime"
 	crclient "sigs.k8s.io/controller-runtime/pkg/client"
 )
@@ -449,239 +448,6 @@ func WaitForPodVolumeBackup(ctx context.Context, c crclient.Client, log logrus.F
 	return true, nil
 }
 
-// WaitForPausedPropagated waits for the HostedControlPlane (HCP) associated with the given HostedCluster (HC)
-// to be paused. It polls the status of the HCP at regular intervals until the HCP is found to be paused or the
-// specified timeout is reached.
-func WaitForPausedPropagated(ctx context.Context, c crclient.Client, log logrus.FieldLogger, hc *hyperv1.HostedCluster, timeout time.Duration, paused string) error {
-	if timeout == 0 {
-		timeout = defaultWaitForPausedTimeout
-	}
-	hcpNamespace := GetHCPNamespace(hc.Name, hc.Namespace)
-
-	log = log.WithFields(logrus.Fields{
-		"namespace": hc.Namespace,
-		"name":      hc.Name,
-	})
-
-	waitCtx, cancel := context.WithTimeout(ctx, timeout)
-	defer cancel()
-
-	err := wait.PollUntilContextCancel(waitCtx, 5*time.Second, true, func(ctx context.Context) (bool, error) {
-		hcp := &hyperv1.HostedControlPlane{}
-		if err := c.Get(ctx, types.NamespacedName{Name: hc.Name, Namespace: hcpNamespace}, hcp); err != nil {
-			if apierrors.IsNotFound(err) {
-				return true, nil
-			}
-			log.Info(err, "failed to get HostedControlPlane")
-			return false, err
-		}
-		log.Infof("waiting for HCP to set PausedUntil to %s", paused)
-
-		// For unpause (paused == ""), we expect PausedUntil to be nil
-		// For pause (paused != ""), we expect PausedUntil to equal paused
-		if paused == "" {
-			if hcp.Spec.PausedUntil == nil {
-				log.Debug("HostedControlPlane is unpaused (PausedUntil is nil)")
-				return true, nil
-			}
-		} else {
-			if hcp.Spec.PausedUntil != nil && *hcp.Spec.PausedUntil == paused {
-				log.Debug("HostedControlPlane is paused")
-				return true, nil
-			}
-		}
-
-		return false, nil
-	})
-
-	if err != nil {
-		log.Errorf("giving up, HCP was not updated in the expected timeout: %v", err)
-		return err
-	}
-
-	return err
-}
-
-// updateHypershiftResource updates Hypershift resources (HostedCluster or NodePool) with pause functionality and audit annotations.
-// This unified function handles both resource types and includes atomic annotation updates.
-func updateHypershiftResource(
-	ctx context.Context,
-	c crclient.Client,
-	log logrus.FieldLogger,
-	paused string,
-	namespaces []string,
-	resourceType string,
-	listObjFactory func() crclient.ObjectList,
-	itemsExtractor func(crclient.ObjectList) []crclient.Object,
-	pausedUntilGetter func(crclient.Object) *string,
-	pausedUntilSetter func(crclient.Object, *string),
-	postUpdateHook func(context.Context, crclient.Client, logrus.FieldLogger, crclient.Object, time.Duration, string) error,
-) error {
-	// Get the appropriate list object
-	listObj := listObjFactory()
-
-	// Search through namespaces for resources
-	var items []crclient.Object
-	for _, ns := range namespaces {
-		if err := c.List(ctx, listObj, crclient.InNamespace(ns)); err != nil {
-			return fmt.Errorf("failed to list %s in namespace %s: %w", resourceType, ns, err)
-		}
-
-		items = itemsExtractor(listObj)
-		if len(items) > 0 {
-			log.Debugf("found %s in namespace %s", resourceType, ns)
-			break
-		}
-	}
-	for _, item := range items {
-		// Create a retry loop with improved backoff for better conflict resolution
-		backoff := wait.Backoff{
-			Steps:    10,                     // Increased from 5 to 10
-			Duration: 500 * time.Millisecond, // Reduced initial duration for faster retries
-			Factor:   2.0,
-			Jitter:   0.1,
-			Cap:      30 * time.Second, // Cap maximum wait time to 30 seconds
-		}
-
-		err := wait.ExponentialBackoff(backoff, func() (bool, error) {
-			// Get the latest version of the resource
-			current := item.DeepCopyObject().(crclient.Object)
-			if err := c.Get(ctx, crclient.ObjectKeyFromObject(item), current); err != nil {
-				return false, err
-			}
-
-			// Check if update is needed
-			currentPausedUntil := pausedUntilGetter(current)
-			var needsUpdate bool
-			if paused == "" {
-				// For unpause: we need to set pausedUntil to nil, so update needed if it's currently not nil
-				needsUpdate = currentPausedUntil != nil
-			} else {
-				// For pause: we need to set pausedUntil to the paused value
-				needsUpdate = currentPausedUntil == nil || *currentPausedUntil != paused
-			}
-
-			if needsUpdate {
-				switch paused {
-				case "":
-					log.Infof("setting PauseUntil to nil (unpause) in %s %s", resourceType, current.GetName())
-					pausedUntilSetter(current, nil)
-					removePauseAuditAnnotations(current)
-				case "true":
-					log.Infof("setting PauseUntil to %s in %s %s", paused, resourceType, current.GetName())
-					pausedUntilSetter(current, ptr.To(paused))
-					addPauseAuditAnnotations(current)
-				}
-
-				// Perform the atomic update
-				if err := c.Update(ctx, current); err != nil {
-					if apierrors.IsConflict(err) {
-						log.Infof("Conflict detected updating %s %s, retrying...", resourceType, current.GetName())
-						return false, nil
-					}
-					return false, err
-				}
-
-				// Call post-update hook if provided (for HostedCluster propagation validation)
-				if postUpdateHook != nil {
-					if err := postUpdateHook(ctx, c, log, current, defaultWaitForPausedTimeout, paused); err != nil {
-						return false, err
-					}
-				}
-
-				log.Infof("Successfully set %s %s PausedUntil to %s", resourceType, current.GetName(), paused)
-			} else {
-				log.Debugf("%s %s already has PausedUntil set to %s, no update needed", resourceType, current.GetName(), paused)
-			}
-
-			return true, nil
-		})
-
-		if err != nil {
-			return fmt.Errorf("failed to update %s %s after retries: %w", resourceType, item.GetName(), err)
-		}
-	}
-
-	return nil
-}
-
-// UpdateHostedCluster updates the HostedCluster's necessary fields in the specified namespaces.
-func UpdateHostedCluster(ctx context.Context, c crclient.Client, log logrus.FieldLogger, paused string, namespaces []string) error {
-	return updateHypershiftResource(
-		ctx, c, log, paused, namespaces, "HostedCluster",
-
-		// listObjFactory - creates a new HostedClusterList
-		func() crclient.ObjectList {
-			return &hyperv1.HostedClusterList{Items: []hyperv1.HostedCluster{}}
-		},
-
-		// itemsExtractor - extracts HostedCluster items as crclient.Object slice
-		func(list crclient.ObjectList) []crclient.Object {
-			hcList := list.(*hyperv1.HostedClusterList)
-			items := make([]crclient.Object, len(hcList.Items))
-			for i := range hcList.Items {
-				items[i] = &hcList.Items[i]
-			}
-			return items
-		},
-
-		// pausedUntilGetter - gets PausedUntil from HostedCluster
-		func(obj crclient.Object) *string {
-			hc := obj.(*hyperv1.HostedCluster)
-			return hc.Spec.PausedUntil
-		},
-
-		// pausedUntilSetter - sets PausedUntil on HostedCluster
-		func(obj crclient.Object, paused *string) {
-			hc := obj.(*hyperv1.HostedCluster)
-			hc.Spec.PausedUntil = paused
-		},
-
-		// postUpdateHook - validates propagation for HostedClusters
-		func(ctx context.Context, c crclient.Client, log logrus.FieldLogger, obj crclient.Object, timeout time.Duration, paused string) error {
-			hc := obj.(*hyperv1.HostedCluster)
-			log.Debug("checking paused state propagation")
-			return WaitForPausedPropagated(ctx, c, log, hc, timeout, paused)
-		},
-	)
-}
-
-// UpdateNodepools updates the NodePool's necessary fields in the specified namespaces.
-func UpdateNodepools(ctx context.Context, c crclient.Client, log logrus.FieldLogger, paused string, namespaces []string) error {
-	return updateHypershiftResource(
-		ctx, c, log, paused, namespaces, "NodePool",
-
-		// listObjFactory - creates a new NodePoolList
-		func() crclient.ObjectList {
-			return &hyperv1.NodePoolList{}
-		},
-
-		// itemsExtractor - extracts NodePool items as crclient.Object slice
-		func(list crclient.ObjectList) []crclient.Object {
-			npList := list.(*hyperv1.NodePoolList)
-			items := make([]crclient.Object, len(npList.Items))
-			for i := range npList.Items {
-				items[i] = &npList.Items[i]
-			}
-			return items
-		},
-
-		// pausedUntilGetter - gets PausedUntil from NodePool
-		func(obj crclient.Object) *string {
-			np := obj.(*hyperv1.NodePool)
-			return np.Spec.PausedUntil
-		},
-
-		// pausedUntilSetter - sets PausedUntil on NodePool
-		func(obj crclient.Object, paused *string) {
-			np := obj.(*hyperv1.NodePool)
-			np.Spec.PausedUntil = paused
-		},
-
-		// postUpdateHook - NodePools don't need propagation validation
-		nil,
-	)
-}
 
 // GetCurrentNamespace reads the namespace from the Kubernetes service account
 // token file and returns it as a string. The file is expected to be located at
@@ -730,20 +496,6 @@ func RemoveAnnotation(metadata metav1.Object, key string) {
 	metadata.SetAnnotations(annotations)
 }
 
-// addPauseAuditAnnotations adds OADP audit annotations when pausing a resource.
-// It adds both the paused-by and paused-at annotations atomically.
-func addPauseAuditAnnotations(obj metav1.Object) {
-	timestamp := time.Now().Format(time.RFC3339)
-	AddAnnotation(obj, OADPPausedByAnnotation, HypershiftOADPPluginName)
-	AddAnnotation(obj, OADPPausedAtAnnotation, timestamp)
-}
-
-// removePauseAuditAnnotations removes OADP audit annotations when unpausing a resource.
-// It removes both the paused-by and paused-at annotations.
-func removePauseAuditAnnotations(obj metav1.Object) {
-	RemoveAnnotation(obj, OADPPausedByAnnotation)
-	RemoveAnnotation(obj, OADPPausedAtAnnotation)
-}
 
 // AddLabel adds a label with the specified key and value to the given metadata object.
 // If the metadata object does not have any labels, a new map is created to store the label.