use wait.PollImmediate

addresses vmware-tanzu#6354 (review)

Signed-off-by: Tiger Kaovilai <[email protected]>

changelogs/unreleased/6354-kaovilai

@@ -1 +1 @@
-Implement recreate ExistingResourcePolicy to restore API
+Implement recreate ExistingResourcePolicy to restore API to enable restore for immutable resources such as Pods.

config/crd/v1/bases/velero.io_restores.yaml

@@ -62,6 +62,23 @@ spec:
                 - recreate
                 nullable: true
                 type: string
+              existingResourcePolicyRecreateResources:
+                description: ExistingResourcePolicyRecreateResources is a slice of
+                  resource names where recreate ExistingResourcePolicy will apply.
+                  Resoruce names can be in the format of "group/version/resource"
+                  or "resource" If empty, a default list of resources (Pod, PV, Secret,
+                  ConfigMap) will be used. Resources with finalizers without velero.io/recreate-remove-finalizer=true
+                  annotation will not be recreated. Recreate will not recreate resources
+                  with ownershipReferences.
+                example:
+                - pods
+                - persistentvolumes
+                - secrets
+                - configmaps
+                items:
+                  type: string
+                nullable: true
+                type: array
               hooks:
                 description: Hooks represent custom behaviors that should be executed
                   during or post restore.

design/Implemented/existing-resource-policy_design.md

@@ -20,7 +20,6 @@ Related issue: https://github.com/vmware-tanzu/velero/issues/4066
 
 ## Non Goals
 - Change existing restore workflow for `ServiceAccount` objects
-- Add support for `ExistingResourcePolicy` as `recreate` for Kubernetes resources. (Future scope feature)
 
 ## Unrelated Proposals (Completely different functionalities than the one proposed in the design)
 - Add support for `ExistingResourcePolicy` to restore API for Non-Kubernetes resources.
@@ -55,11 +54,9 @@ skip restoration.
    - Changed resources: Velero will first try to patch the changed resource, Now if the patch:
        - succeeds: Then the in-cluster resource gets updated with the labels as well as the resource diff
        - fails: Velero adds a restore warning and tries to just update the backup/restore labels on the resource, if the labels patch also fails then we add restore error.
-3. `recreate`:  Similar to `update` but if resource already exists and is immutable (such as Pods), then Velero attempt to recreate the resource by deleting it first.
+3. `recreate`:  Similar to `update` but if resource already exists and is immutable (such as Pods), then Velero attempt to recreate the resource by deleting it first. More details below.
 
-*Note:* The `recreate` option is being implemented in [#6354](https://github.com/vmware-tanzu/velero/pull/6354) separate from original proposal.
-
-Velero will not be deleting any resources for policy `update` and `none` proposed in 
+Velero will not be deleting any resources for `update` and `none` policy proposed in
 this design but Velero will patch the resources in `update` policy option.
 
 Example:
@@ -141,22 +138,38 @@ existingResourcePolicyOverrides:
 The `existingResourcePolicy` spec field will be an `PolicyType` type field. 
 
 Restore API:
-```
+```go
 type RestoreSpec struct {
 .
 .
 .
 // ExistingResourcePolicy specifies the restore behaviour for the Kubernetes resource to be restored
+// +kubebuilder:validation:Enum=none;update;recreate
 // +optional
+// +nullable
 ExistingResourcePolicy PolicyType
 
 }
 ```
 PolicyType:
-```
+```go
 type PolicyType string
 const PolicyTypeNone PolicyType  = "none"
 const PolicyTypePatch PolicyType = "update"
+const PolicyTypePatch PolicyType = "recreate"
+```
+
+existingResourcePolicyRecreateResources will be a new field in the Restore API which will be a slice of resource names where recreate ExistingResourcePolicy will apply. Resoruce names can be in the format of "group/version/resource" or "resource". If empty, a default list of resources (Pod, PV, Secret, ConfigMap) will be used. Recreate will not recreate resources with ownershipReferences or finalizers.
+```go
+// ExistingResourcePolicyRecreateResources is a slice of resource names where recreate ExistingResourcePolicy will apply.
+// Resoruce names can be in the format of "group/version/resource" or "resource"
+// If empty, a default list of resources (Pod, PV, Secret, ConfigMap) will be used.
+// Resources with finalizers without velero.io/recreate-remove-finalizer=true annotation will not be recreated.
+// Recreate will not recreate resources with ownershipReferences.
+// +kubebuilder:example:={"pods", "persistentvolumes", "secrets", "configmaps"}
+// +optional
+// +nullable
+ExistingResourcePolicyRecreateResources []string `json:"existingResourcePolicyRecreateResources,omitempty"`
 ```
 
 ### Approach 2: Add a new spec field `existingResourcePolicyConfig` to the Restore API
@@ -260,4 +273,20 @@ Restore describer will also be updated to reflect the policy `pkg/cmd/util/outpu
 We have decided to go ahead with the implementation of Approach 1 as:
 - It is easier to implement 
 - It is also easier to scale and leaves room for improvement and the door open to expanding to approach 3
-- It also provides an option to preserve the existing velero restore workflow
+- It also provides an option to preserve the existing velero restore workflow
+
+### Recreate policy details
+Existing resource restore policy *Recreate* purpose is to help users restore existing resources that exists in cluster but differ from backup resouce, are immutable and failed to be updated due to immutability.
+
+The policy will be used to delete the existing resource and recreate it with the backup resource.
+
+The policy will be used for resources that are immutable and cannot first be updated.
+
+#### Restrictions and Limitations
+Resources with OwnerReferences will not be supported by the recreate policy, at least for the initial implementation.
+The policy will only be used on resources specified at Restore.Spec. like Pods, PVCs, PVs, etc.
+
+If an existing resource have finalizers, it would need to have annotations `velero.io/recreate-remove-finalizer=true` to be considered for recreation.
+
+
+*Note:* The `recreate` option is implemented in [#6354](https://github.com/vmware-tanzu/velero/pull/6354) separate from original update policy proposal.

pkg/apis/velero/v1/restore_types.go

@@ -115,6 +115,16 @@ type RestoreSpec struct {
 	// +nullable
 	ExistingResourcePolicy PolicyType `json:"existingResourcePolicy,omitempty"`
 
+	// ExistingResourcePolicyRecreateResources is a slice of resource names where recreate ExistingResourcePolicy will apply.
+	// Resoruce names can be in the format of "group/version/resource" or "resource"
+	// If empty, a default list of resources (Pod, PV, Secret, ConfigMap) will be used.
+	// Resources with finalizers without velero.io/recreate-remove-finalizer=true annotation will not be recreated.
+	// Recreate will not recreate resources with ownershipReferences.
+	// +kubebuilder:example:={"pods", "persistentvolumes", "secrets", "configmaps"}
+	// +optional
+	// +nullable
+	ExistingResourcePolicyRecreateResources []string `json:"existingResourcePolicyRecreateResources,omitempty"`
+
 	// ItemOperationTimeout specifies the time used to wait for RestoreItemAction operations
 	// The default value is 4 hour.
 	// +optional

pkg/controller/restore_controller.go

@@ -90,7 +90,10 @@ var nonRestorableResources = []string{
 	"backuprepositories.velero.io",
 }
 
-var ExternalResourcesFinalizer = "restores.velero.io/external-resources-finalizer"
+const (
+	ExternalResourcesFinalizer = "restores.velero.io/external-resources-finalizer"
+	RecreateRemoveFinalizerAnnotation = "velero.io/recreate-remove-finalizer"
+)
 
 type restoreReconciler struct {
 	ctx                         context.Context

pkg/restore/restore.go

@@ -2540,15 +2540,15 @@ func (ctx *restoreContext) processRecreateResourcePolicy(fromCluster, fromCluste
 		}
 	}
 	// wait up to 2 minutes until object does not exists in cluster
-	for timeStarted := time.Now(); apierrors.IsNotFound(err) || time.Now().After(timeStarted.Add(2*time.Minute)); {
+	wait.PollImmediate(5*time.Second, 2*time.Minute, func() (bool, error) {
 		_, err = resourceClient.Get(obj.GetName(), metav1.GetOptions{})
 		if !apierrors.IsNotFound(err) {
-			ctx.log.Warnf("get attempt to check object is gone failed for %s %s: %v", fromCluster.GroupVersionKind(), kube.NamespaceAndName(fromCluster), err)
-			time.Sleep(10 * time.Second)
-			continue
+			ctx.log.Infof("waiting until %s %s IsNotFound for recreate existingResourcePolicy. current err from get: %v", fromCluster.GroupVersionKind(), kube.NamespaceAndName(fromCluster), err)
+			return false, nil
 		}
-		break
-	}
+		return true, nil
+	})
+
 	// Create object from latest backup/restore)
 	obj.SetNamespace(namespace)
 	_, err = resourceClient.Create(obj)