Pod Volume Backup/Restore Refactor: Rename Init Helper (#5432)

* restore helper refactor

Signed-off-by: Lyndon-Li <[email protected]>

* resolve codespell

Signed-off-by: Lyndon-Li <[email protected]>

Signed-off-by: Lyndon-Li <[email protected]>

.github/workflows/pr-codespell.yml

@@ -15,6 +15,6 @@ jobs:
       with:
         # ignore the config/.../crd.go file as it's generated binary data that is edited elswhere.
         skip: .git,*.png,*.jpg,*.woff,*.ttf,*.gif,*.ico,./config/crd/v1beta1/crds/crds.go,./config/crd/v1/crds/crds.go,./go.sum,./LICENSE
-        ignore_words_list: iam,aks,ist,bridget,ue
+        ignore_words_list: iam,aks,ist,bridget,ue,shouldnot
         check_filenames: true
         check_hidden: true

Makefile

@@ -112,17 +112,17 @@ GOPROXY ?= https://proxy.golang.org
 # If you want to build all containers, see the 'all-containers' rule.
 all:
 	@$(MAKE) build
-	@$(MAKE) build BIN=velero-restic-restore-helper
+	@$(MAKE) build BIN=velero-restore-helper
 
 build-%:
 	@$(MAKE) --no-print-directory ARCH=$* build
-	@$(MAKE) --no-print-directory ARCH=$* build BIN=velero-restic-restore-helper
+	@$(MAKE) --no-print-directory ARCH=$* build BIN=velero-restore-helper
 
 all-build: $(addprefix build-, $(CLI_PLATFORMS))
 
 all-containers: container-builder-env
 	@$(MAKE) --no-print-directory container
-	@$(MAKE) --no-print-directory container BIN=velero-restic-restore-helper
+	@$(MAKE) --no-print-directory container BIN=velero-restore-helper
 
 local: build-dirs
 # Add DEBUG=1 to enable debug locally

changelogs/unreleased/5432-lyndon

@@ -0,0 +1 @@
+Rename Velero pod volume restore init helper from "velero-restic-restore-helper" to "velero-restore-helper"

design/Implemented/backup-resources-order.md

@@ -2,7 +2,7 @@
 This document proposes a solution that allows user to specify a backup order for resources of specific resource type.
 
 ## Background
-During backup process, user may need to back up resources of specific type in some specific order to ensure the resources were backup properly because these resources are related and ordering might be required to preserve the consistency for the apps to recover itself  from the backup image 
+During backup process, user may need to back up resources of specific type in some specific order to ensure the resources were backup properly because these resources are related and ordering might be required to preserve the consistency for the apps to recover itself �from the backup image 
 (Ex: primary-secondary database pods in a cluster).
 
 ## Goals
@@ -12,7 +12,7 @@ During backup process, user may need to back up resources of specific type in so
 - Use a plugin to backup an resources and all the sub resources.  For example use a plugin for StatefulSet and backup pods belong to the StatefulSet in specific order.  This plugin solution is not generic and requires plugin for each resource type.
 
 ## High-Level Design
-User will specify a map of resource type to list resource names (separate by semicolons).  Each name will be in the format "namespaceName/resourceName" to enable ordering accross namespaces.  Based on this map, the resources of each resource type will be sorted by the order specified in the list of resources.  If a resource instance belong to that specific type but its name is not in the order list, then it will be put behind other resources that are in the list.
+User will specify a map of resource type to list resource names (separate by semicolons).  Each name will be in the format "namespaceName/resourceName" to enable ordering across namespaces.  Based on this map, the resources of each resource type will be sorted by the order specified in the list of resources.  If a resource instance belong to that specific type but its name is not in the order list, then it will be put behind other resources that are in the list.
 
 ### Changes to BackupSpec
 Add new field to BackupSpec
@@ -36,5 +36,5 @@ Example:
 >velero backup create mybackup --ordered-resources "pod=ns1/pod1,ns1/pod2;persistentvolumeclaim=n2/slavepod,ns2/primarypod"
 
 ## Open Issues
-- In the CLI, the design proposes to use commas to separate items of a resource type and semicolon to separate key-value pairs.  This follows the convention of using commas to separate items in a list (For example: --include-namespaces ns1,ns2).  However, the syntax for map in labels and annotations use commas to seperate key-value pairs.  So it introduces some inconsistency.
+- In the CLI, the design proposes to use commas to separate items of a resource type and semicolon to separate key-value pairs.  This follows the convention of using commas to separate items in a list (For example: --include-namespaces ns1,ns2).  However, the syntax for map in labels and annotations use commas to separate key-value pairs.  So it introduces some inconsistency.
 - For pods that managed by Deployment or DaemonSet, this design may not work because the pods' name is randomly generated and if pods are restarted, they would have different names so the Backup operation may not consider the restarted pods in the sorting algorithm.  This problem will be addressed when we enhance the design to use regular expression to specify the OrderResources instead of exact match. 

design/Implemented/generating-velero-crds-with-structural-schema.md

@@ -28,7 +28,7 @@ This document proposes adding _controller-tools_ to the project to automatically
 _controller-tools_ works by reading the Go files that contain the API type definitions.
 It uses a combination of the struct fields, types, tags and comments to build the OpenAPIv3 schema for the CRDs. The tooling makes some assumptions based on conventions followed in upstream Kubernetes and the ecosystem, which involves some changes to the Velero API type definitions, especially around optional fields.
 
-In order for _controller-tools_ to read the Go files containing Velero API type defintiions, the CRDs need to be generated at build time, as these files are not available at runtime (i.e. the Go files are not accessible by the compiled binary).
+In order for _controller-tools_ to read the Go files containing Velero API type definitions, the CRDs need to be generated at build time, as these files are not available at runtime (i.e. the Go files are not accessible by the compiled binary).
 These generated CRD manifests (YAML) will then need to be available to the `pkg/install` package for it to include when installing Velero resources.
 
 ## Detailed Design

internal/hook/item_hook_handler.go

@@ -36,7 +36,7 @@ import (
 	velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1"
 	"github.com/vmware-tanzu/velero/pkg/kuberesource"
 	"github.com/vmware-tanzu/velero/pkg/podexec"
-	"github.com/vmware-tanzu/velero/pkg/podvolume"
+	"github.com/vmware-tanzu/velero/pkg/restorehelper"
 	"github.com/vmware-tanzu/velero/pkg/util/collections"
 	"github.com/vmware-tanzu/velero/pkg/util/kube"
 )
@@ -121,12 +121,12 @@ func (i *InitContainerRestoreHookHandler) HandleRestoreHooks(
 	}
 
 	initContainers := []corev1api.Container{}
-	// If this pod had pod volumes backed up using restic, then we want to the pod volumes restored prior to
+	// If this pod is backed up with data movement, then we want to the pod volumes restored prior to
 	// running the restore hook init containers. This allows the restore hook init containers to prepare the
 	// restored data to be consumed by the application container(s).
-	// So if there is a "restic-wait" init container already on the pod at index 0, we'll preserve that and run
+	// So if there is a "restore-wait" init container already on the pod at index 0, we'll preserve that and run
 	// it before running any other init container.
-	if len(pod.Spec.InitContainers) > 0 && pod.Spec.InitContainers[0].Name == podvolume.InitContainer {
+	if len(pod.Spec.InitContainers) > 0 && pod.Spec.InitContainers[0].Name == restorehelper.WaitInitContainer {
 		initContainers = append(initContainers, pod.Spec.InitContainers[0])
 		pod.Spec.InitContainers = pod.Spec.InitContainers[1:]
 	}

internal/hook/item_hook_handler_test.go

@@ -1675,16 +1675,16 @@ func TestHandleRestoreHooks(t *testing.T) {
 			},
 		},
 		{
-			name: "should preserve restic-wait init container when it is the only existing init container",
+			name: "should preserve restore-wait init container when it is the only existing init container",
 			podInput: corev1api.Pod{
 				ObjectMeta: metav1.ObjectMeta{
 					Name:      "app1",
 					Namespace: "default",
 				},
 				Spec: corev1api.PodSpec{
 					InitContainers: []corev1api.Container{
-						*builder.ForContainer("restic-wait", "bus-box").
-							Command([]string{"restic-restore"}).Result(),
+						*builder.ForContainer("restore-wait", "bus-box").
+							Command([]string{"pod-volume-restore"}).Result(),
 					},
 				},
 			},
@@ -1696,8 +1696,8 @@ func TestHandleRestoreHooks(t *testing.T) {
 				},
 				Spec: corev1api.PodSpec{
 					InitContainers: []corev1api.Container{
-						*builder.ForContainer("restic-wait", "bus-box").
-							Command([]string{"restic-restore"}).Result(),
+						*builder.ForContainer("restore-wait", "bus-box").
+							Command([]string{"pod-volume-restore"}).Result(),
 						*builder.ForContainer("restore-init-container-1", "nginx").
 							Command([]string{"a", "b", "c"}).Result(),
 						*builder.ForContainer("restore-init-container-2", "nginx").
@@ -1729,16 +1729,16 @@ func TestHandleRestoreHooks(t *testing.T) {
 		},
 
 		{
-			name: "should preserve restic-wait init container when it exits with other init containers",
+			name: "should preserve restore-wait init container when it exits with other init containers",
 			podInput: corev1api.Pod{
 				ObjectMeta: metav1.ObjectMeta{
 					Name:      "app1",
 					Namespace: "default",
 				},
 				Spec: corev1api.PodSpec{
 					InitContainers: []corev1api.Container{
-						*builder.ForContainer("restic-wait", "bus-box").
-							Command([]string{"restic-restore"}).Result(),
+						*builder.ForContainer("restore-wait", "bus-box").
+							Command([]string{"pod-volume-restore"}).Result(),
 						*builder.ForContainer("init-app-step1", "busy-box").
 							Command([]string{"init-step1"}).Result(),
 						*builder.ForContainer("init-app-step2", "busy-box").
@@ -1754,8 +1754,8 @@ func TestHandleRestoreHooks(t *testing.T) {
 				},
 				Spec: corev1api.PodSpec{
 					InitContainers: []corev1api.Container{
-						*builder.ForContainer("restic-wait", "bus-box").
-							Command([]string{"restic-restore"}).Result(),
+						*builder.ForContainer("restore-wait", "bus-box").
+							Command([]string{"pod-volume-restore"}).Result(),
 						*builder.ForContainer("restore-init-container-1", "nginx").
 							Command([]string{"a", "b", "c"}).Result(),
 						*builder.ForContainer("restore-init-container-2", "nginx").

internal/velero/images.go

@@ -44,8 +44,8 @@ func DefaultVeleroImage() string {
 	return fmt.Sprintf("%s/%s:%s", imageRegistry(), "velero", ImageTag())
 }
 
-// DefaultResticRestoreHelperImage returns the default container image to use for the restic restore helper
+// DefaultRestoreHelperImage returns the default container image to use for the restore helper
 // for this version of Velero.
-func DefaultResticRestoreHelperImage() string {
-	return fmt.Sprintf("%s/%s:%s", imageRegistry(), "velero-restic-restore-helper", ImageTag())
+func DefaultRestoreHelperImage() string {
+	return fmt.Sprintf("%s/%s:%s", imageRegistry(), "velero-restore-helper", ImageTag())
 }

internal/velero/images_test.go

@@ -135,6 +135,6 @@ func TestDefaultVeleroImage(t *testing.T) {
 	testDefaultImage(t, DefaultVeleroImage, "velero")
 }
 
-func TestDefaultResticRestoreHelperImage(t *testing.T) {
-	testDefaultImage(t, DefaultResticRestoreHelperImage, "velero-restic-restore-helper")
+func TestDefaultRestoreHelperImage(t *testing.T) {
+	testDefaultImage(t, DefaultRestoreHelperImage, "velero-restore-helper")
 }

pkg/cmd/server/plugin/plugin.go

@@ -45,7 +45,7 @@ func NewCommand(f client.Factory) *cobra.Command {
 				RegisterBackupItemAction("velero.io/service-account", newServiceAccountBackupItemAction(f)).
 				RegisterRestoreItemAction("velero.io/job", newJobRestoreItemAction).
 				RegisterRestoreItemAction("velero.io/pod", newPodRestoreItemAction).
-				RegisterRestoreItemAction("velero.io/restic", newResticRestoreItemAction(f)).
+				RegisterRestoreItemAction("velero.io/pod-volume-restore", newPodVolumeRestoreItemAction(f)).
 				RegisterRestoreItemAction("velero.io/init-restore-hook", newInitRestoreHookPodAction).
 				RegisterRestoreItemAction("velero.io/service", newServiceRestoreItemAction).
 				RegisterRestoreItemAction("velero.io/service-account", newServiceAccountRestoreItemAction).
@@ -139,7 +139,7 @@ func newInitRestoreHookPodAction(logger logrus.FieldLogger) (interface{}, error)
 	return restore.NewInitRestoreHookPodAction(logger), nil
 }
 
-func newResticRestoreItemAction(f client.Factory) plugincommon.HandlerInitializer {
+func newPodVolumeRestoreItemAction(f client.Factory) plugincommon.HandlerInitializer {
 	return func(logger logrus.FieldLogger) (interface{}, error) {
 		client, err := f.KubeClient()
 		if err != nil {
@@ -151,7 +151,7 @@ func newResticRestoreItemAction(f client.Factory) plugincommon.HandlerInitialize
 			return nil, err
 		}
 
-		return restore.NewResticRestoreAction(logger, client.CoreV1().ConfigMaps(f.Namespace()), veleroClient.VeleroV1().PodVolumeBackups(f.Namespace())), nil
+		return restore.NewPodVolumeRestoreAction(logger, client.CoreV1().ConfigMaps(f.Namespace()), veleroClient.VeleroV1().PodVolumeBackups(f.Namespace())), nil
 	}
 }
 

pkg/controller/pod_volume_restore_controller.go

@@ -42,6 +42,7 @@ import (
 	"github.com/vmware-tanzu/velero/pkg/podvolume"
 	"github.com/vmware-tanzu/velero/pkg/repository"
 	repokey "github.com/vmware-tanzu/velero/pkg/repository/keys"
+	"github.com/vmware-tanzu/velero/pkg/restorehelper"
 	"github.com/vmware-tanzu/velero/pkg/uploader"
 	"github.com/vmware-tanzu/velero/pkg/uploader/provider"
 	"github.com/vmware-tanzu/velero/pkg/util/boolptr"
@@ -105,10 +106,10 @@ func (c *PodVolumeRestoreReconciler) Reconcile(ctx context.Context, req ctrl.Req
 		return ctrl.Result{}, nil
 	}
 
-	resticInitContainerIndex := getResticInitContainerIndex(pod)
-	if resticInitContainerIndex > 0 {
+	initContainerIndex := getInitContainerIndex(pod)
+	if initContainerIndex > 0 {
 		log.Warnf(`Init containers before the %s container may cause issues
-		          if they interfere with volumes being restored: %s index %d`, podvolume.InitContainer, podvolume.InitContainer, resticInitContainerIndex)
+		          if they interfere with volumes being restored: %s index %d`, restorehelper.WaitInitContainer, restorehelper.WaitInitContainer, initContainerIndex)
 	}
 
 	log.Info("Restore starting")
@@ -162,8 +163,8 @@ func (c *PodVolumeRestoreReconciler) shouldProcess(ctx context.Context, log logr
 		return false, nil, err
 	}
 
-	if !isResticInitContainerRunning(pod) {
-		log.Debug("Pod is not running restic-wait init container, skip")
+	if !isInitContainerRunning(pod) {
+		log.Debug("Pod is not running restore-wait init container, skip")
 		return false, nil, nil
 	}
 
@@ -207,18 +208,18 @@ func isPVRNew(pvr *velerov1api.PodVolumeRestore) bool {
 	return pvr.Status.Phase == "" || pvr.Status.Phase == velerov1api.PodVolumeRestorePhaseNew
 }
 
-func isResticInitContainerRunning(pod *corev1api.Pod) bool {
-	// Restic wait container can be anywhere in the list of init containers, but must be running.
-	i := getResticInitContainerIndex(pod)
+func isInitContainerRunning(pod *corev1api.Pod) bool {
+	// Pod volume wait container can be anywhere in the list of init containers, but must be running.
+	i := getInitContainerIndex(pod)
 	return i >= 0 &&
 		len(pod.Status.InitContainerStatuses)-1 >= i &&
 		pod.Status.InitContainerStatuses[i].State.Running != nil
 }
 
-func getResticInitContainerIndex(pod *corev1api.Pod) int {
-	// Restic wait container can be anywhere in the list of init containers so locate it.
+func getInitContainerIndex(pod *corev1api.Pod) int {
+	// Pod volume wait container can be anywhere in the list of init containers so locate it.
 	for i, initContainer := range pod.Spec.InitContainers {
-		if initContainer.Name == podvolume.InitContainer {
+		if initContainer.Name == restorehelper.WaitInitContainer {
 			return i
 		}
 	}
@@ -299,7 +300,7 @@ func (c *PodVolumeRestoreReconciler) processRestore(ctx context.Context, req *ve
 	}
 
 	// Write a done file with name=<restore-uid> into the just-created .velero dir
-	// within the volume. The velero restic init container on the pod is waiting
+	// within the volume. The velero init container on the pod is waiting
 	// for this file to exist in each restored volume before completing.
 	if err := ioutil.WriteFile(filepath.Join(volumePath, ".velero", string(restoreUID)), nil, 0644); err != nil {
 		return errors.Wrap(err, "error writing done file")