Skip to content

Commit

Permalink
Merge pull request #13 from vmware-tanzu/main
Browse files Browse the repository at this point in the history
Fork Sync: Update from parent repository
  • Loading branch information
kaovilai authored Oct 17, 2022
2 parents a7a74d5 + ad7e3ab commit 53129a9
Show file tree
Hide file tree
Showing 61 changed files with 509 additions and 502 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/pr-codespell.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
6 changes: 3 additions & 3 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions changelogs/unreleased/5432-lyndon
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Rename Velero pod volume restore init helper from "velero-restic-restore-helper" to "velero-restore-helper"
1 change: 1 addition & 0 deletions changelogs/unreleased/5444-lyndon
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Remove irrational "Restic" names in Velero code after the PVBR refactor
1 change: 1 addition & 0 deletions changelogs/unreleased/5446-allenxu404
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Change subcommand `velero restic repo` to `velero repo`
6 changes: 3 additions & 3 deletions design/Implemented/backup-resources-order.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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.
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 4 additions & 4 deletions internal/hook/item_hook_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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:]
}
Expand Down
20 changes: 10 additions & 10 deletions internal/hook/item_hook_handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
},
},
},
Expand All @@ -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").
Expand Down Expand Up @@ -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").
Expand All @@ -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").
Expand Down
6 changes: 3 additions & 3 deletions internal/velero/images.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}
4 changes: 2 additions & 2 deletions internal/velero/images_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
2 changes: 1 addition & 1 deletion pkg/apis/velero/v1/pod_volume_operation_progress.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ limitations under the License.
package v1

// PodVolumeOperationProgress represents the progress of a
// PodVolumeBackup/Restore (restic) operation
// PodVolumeBackup/Restore operation
type PodVolumeOperationProgress struct {
// +optional
TotalBytes int64 `json:"totalBytes,omitempty"`
Expand Down
62 changes: 31 additions & 31 deletions pkg/backup/backup.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,15 +71,15 @@ type Backupper interface {

// kubernetesBackupper implements Backupper.
type kubernetesBackupper struct {
backupClient velerov1client.BackupsGetter
dynamicFactory client.DynamicFactory
discoveryHelper discovery.Helper
podCommandExecutor podexec.PodCommandExecutor
resticBackupperFactory podvolume.BackupperFactory
resticTimeout time.Duration
defaultVolumesToFsBackup bool
clientPageSize int
uploaderType string
backupClient velerov1client.BackupsGetter
dynamicFactory client.DynamicFactory
discoveryHelper discovery.Helper
podCommandExecutor podexec.PodCommandExecutor
podVolumeBackupperFactory podvolume.BackupperFactory
podVolumeTimeout time.Duration
defaultVolumesToFsBackup bool
clientPageSize int
uploaderType string
}

func (i *itemKey) String() string {
Expand All @@ -102,22 +102,22 @@ func NewKubernetesBackupper(
discoveryHelper discovery.Helper,
dynamicFactory client.DynamicFactory,
podCommandExecutor podexec.PodCommandExecutor,
resticBackupperFactory podvolume.BackupperFactory,
resticTimeout time.Duration,
podVolumeBackupperFactory podvolume.BackupperFactory,
podVolumeTimeout time.Duration,
defaultVolumesToFsBackup bool,
clientPageSize int,
uploaderType string,
) (Backupper, error) {
return &kubernetesBackupper{
backupClient: backupClient,
discoveryHelper: discoveryHelper,
dynamicFactory: dynamicFactory,
podCommandExecutor: podCommandExecutor,
resticBackupperFactory: resticBackupperFactory,
resticTimeout: resticTimeout,
defaultVolumesToFsBackup: defaultVolumesToFsBackup,
clientPageSize: clientPageSize,
uploaderType: uploaderType,
backupClient: backupClient,
discoveryHelper: discoveryHelper,
dynamicFactory: dynamicFactory,
podCommandExecutor: podCommandExecutor,
podVolumeBackupperFactory: podVolumeBackupperFactory,
podVolumeTimeout: podVolumeTimeout,
defaultVolumesToFsBackup: defaultVolumesToFsBackup,
clientPageSize: clientPageSize,
uploaderType: uploaderType,
}, nil
}

Expand Down Expand Up @@ -228,7 +228,7 @@ func (kb *kubernetesBackupper) BackupWithResolvers(log logrus.FieldLogger,

backupRequest.BackedUpItems = map[itemKey]struct{}{}

podVolumeTimeout := kb.resticTimeout
podVolumeTimeout := kb.podVolumeTimeout
if val := backupRequest.Annotations[velerov1api.PodVolumeOperationTimeoutAnnotation]; val != "" {
parsed, err := time.ParseDuration(val)
if err != nil {
Expand All @@ -241,9 +241,9 @@ func (kb *kubernetesBackupper) BackupWithResolvers(log logrus.FieldLogger,
ctx, cancelFunc := context.WithTimeout(context.Background(), podVolumeTimeout)
defer cancelFunc()

var resticBackupper podvolume.Backupper
if kb.resticBackupperFactory != nil {
resticBackupper, err = kb.resticBackupperFactory.NewBackupper(ctx, backupRequest.Backup, kb.uploaderType)
var podVolumeBackupper podvolume.Backupper
if kb.podVolumeBackupperFactory != nil {
podVolumeBackupper, err = kb.podVolumeBackupperFactory.NewBackupper(ctx, backupRequest.Backup, kb.uploaderType)
if err != nil {
log.WithError(errors.WithStack(err)).Debugf("Error from NewBackupper")
return errors.WithStack(err)
Expand Down Expand Up @@ -278,13 +278,13 @@ func (kb *kubernetesBackupper) BackupWithResolvers(log logrus.FieldLogger,
}

itemBackupper := &itemBackupper{
backupRequest: backupRequest,
tarWriter: tw,
dynamicFactory: kb.dynamicFactory,
discoveryHelper: kb.discoveryHelper,
resticBackupper: resticBackupper,
resticSnapshotTracker: newPVCSnapshotTracker(),
volumeSnapshotterGetter: volumeSnapshotterGetter,
backupRequest: backupRequest,
tarWriter: tw,
dynamicFactory: kb.dynamicFactory,
discoveryHelper: kb.discoveryHelper,
podVolumeBackupper: podVolumeBackupper,
podVolumeSnapshotTracker: newPVCSnapshotTracker(),
volumeSnapshotterGetter: volumeSnapshotterGetter,
itemHookHandler: &hook.DefaultItemHookHandler{
PodCommandExecutor: kb.podCommandExecutor,
},
Expand Down
32 changes: 16 additions & 16 deletions pkg/backup/backup_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2595,17 +2595,17 @@ func TestBackupWithHooks(t *testing.T) {
}
}

type fakeResticBackupperFactory struct{}
type fakePodVolumeBackupperFactory struct{}

func (f *fakeResticBackupperFactory) NewBackupper(context.Context, *velerov1.Backup, string) (podvolume.Backupper, error) {
return &fakeResticBackupper{}, nil
func (f *fakePodVolumeBackupperFactory) NewBackupper(context.Context, *velerov1.Backup, string) (podvolume.Backupper, error) {
return &fakePodVolumeBackupper{}, nil
}

type fakeResticBackupper struct{}
type fakePodVolumeBackupper struct{}

// BackupPodVolumes returns one pod volume backup per entry in volumes, with namespace "velero"
// and name "pvb-<pod-namespace>-<pod-name>-<volume-name>".
func (b *fakeResticBackupper) BackupPodVolumes(backup *velerov1.Backup, pod *corev1.Pod, volumes []string, _ logrus.FieldLogger) ([]*velerov1.PodVolumeBackup, []error) {
func (b *fakePodVolumeBackupper) BackupPodVolumes(backup *velerov1.Backup, pod *corev1.Pod, volumes []string, _ logrus.FieldLogger) ([]*velerov1.PodVolumeBackup, []error) {
var res []*velerov1.PodVolumeBackup
for _, vol := range volumes {
pvb := builder.ForPodVolumeBackup("velero", fmt.Sprintf("pvb-%s-%s-%s", pod.Namespace, pod.Name, vol)).Result()
Expand All @@ -2615,11 +2615,11 @@ func (b *fakeResticBackupper) BackupPodVolumes(backup *velerov1.Backup, pod *cor
return res, nil
}

// TestBackupWithRestic runs backups of pods that are annotated for restic backup,
// and ensures that the restic backupper is called, that the returned PodVolumeBackups
// are added to the Request object, and that when PVCs are backed up with restic, the
// TestBackupWithPodVolume runs backups of pods that are annotated for PodVolume backup,
// and ensures that the pod volume backupper is called, that the returned PodVolumeBackups
// are added to the Request object, and that when PVCs are backed up with PodVolume, the
// claimed PVs are not also snapshotted using a VolumeSnapshotter.
func TestBackupWithRestic(t *testing.T) {
func TestBackupWithPodVolume(t *testing.T) {
tests := []struct {
name string
backup *velerov1.Backup
Expand All @@ -2629,7 +2629,7 @@ func TestBackupWithRestic(t *testing.T) {
want []*velerov1.PodVolumeBackup
}{
{
name: "a pod annotated for restic backup should result in pod volume backups being returned",
name: "a pod annotated for pod volume backup should result in pod volume backups being returned",
backup: defaultBackup().Result(),
apiResources: []*test.APIResource{
test.Pods(
Expand All @@ -2641,7 +2641,7 @@ func TestBackupWithRestic(t *testing.T) {
},
},
{
name: "when a PVC is used by two pods and annotated for restic backup on both, only one should be backed up",
name: "when a PVC is used by two pods and annotated for pod volume backup on both, only one should be backed up",
backup: defaultBackup().Result(),
apiResources: []*test.APIResource{
test.Pods(
Expand All @@ -2662,7 +2662,7 @@ func TestBackupWithRestic(t *testing.T) {
},
},
{
name: "when PVC pod volumes are backed up using restic, their claimed PVs are not also snapshotted",
name: "when PVC pod volumes are backed up using pod volume backup, their claimed PVs are not also snapshotted",
backup: defaultBackup().Result(),
apiResources: []*test.APIResource{
test.Pods(
Expand Down Expand Up @@ -2707,7 +2707,7 @@ func TestBackupWithRestic(t *testing.T) {
backupFile = bytes.NewBuffer([]byte{})
)

h.backupper.resticBackupperFactory = new(fakeResticBackupperFactory)
h.backupper.podVolumeBackupperFactory = new(fakePodVolumeBackupperFactory)

for _, resource := range tc.apiResources {
h.addItems(t, resource)
Expand Down Expand Up @@ -2786,9 +2786,9 @@ func newHarness(t *testing.T) *harness {
discoveryHelper: discoveryHelper,

// unsupported
podCommandExecutor: nil,
resticBackupperFactory: nil,
resticTimeout: 0,
podCommandExecutor: nil,
podVolumeBackupperFactory: nil,
podVolumeTimeout: 0,
},
log: log,
}
Expand Down
Loading

1 comment on commit 53129a9

@vercel
Copy link

@vercel vercel bot commented on 53129a9 Oct 17, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Successfully deployed to the following URLs:

velero – ./

velero-git-main-kaovilai.vercel.app
velero.tig.pw
velero-kaovilai.vercel.app
velero.vercel.app

Please sign in to comment.