Skip to content

Commit

Permalink
use wait.PollImmediate
Browse files Browse the repository at this point in the history
addresses vmware-tanzu#6354 (review)

Signed-off-by: Tiger Kaovilai <[email protected]>
  • Loading branch information
kaovilai committed Mar 1, 2024
1 parent 0dbf0b0 commit cd499d1
Show file tree
Hide file tree
Showing 7 changed files with 57 additions and 14 deletions.
2 changes: 1 addition & 1 deletion changelogs/unreleased/6354-kaovilai
Original file line number Diff line number Diff line change
@@ -1 +1 @@
Implement recreate ExistingResourcePolicy to restore API
Implement recreate ExistingResourcePolicy to restore API to enable restore for immutable resources such as Pods.
16 changes: 16 additions & 0 deletions config/crd/v1/bases/velero.io_restores.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,22 @@ 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. Recreate will not recreate resources with
ownershipReferences or finalizers.
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.
Expand Down
2 changes: 1 addition & 1 deletion config/crd/v1/crds/crds.go

Large diffs are not rendered by default.

25 changes: 19 additions & 6 deletions design/Implemented/existing-resource-policy_design.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -260,4 +257,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.

Finalizers will not be supported by the recreate policy, at least for the initial implementation. Only resources that can be deleted without finalizers will be supported.


*Note:* The `recreate` option is implemented in [#6354](https://github.com/vmware-tanzu/velero/pull/6354) separate from original update policy proposal.
9 changes: 9 additions & 0 deletions pkg/apis/velero/v1/restore_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,15 @@ 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.
// Recreate will not recreate resources with ownershipReferences or finalizers.
// +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
Expand Down
5 changes: 5 additions & 0 deletions pkg/apis/velero/v1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 6 additions & 6 deletions pkg/restore/restore.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit cd499d1

Please sign in to comment.