Skip to content

Commit

Permalink
Move the code in pkg/restic/common.go to the proper package
Browse files Browse the repository at this point in the history
Move the code in pkg/restic/common.go to the proper package

Fixes vmware-tanzu#5243

Signed-off-by: Wenkai Yin(尹文开) <[email protected]>
  • Loading branch information
ywk253100 committed Aug 29, 2022
1 parent 3e30a3d commit 4a5647a
Show file tree
Hide file tree
Showing 13 changed files with 258 additions and 265 deletions.
4 changes: 2 additions & 2 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/restic"
"github.com/vmware-tanzu/velero/pkg/podvolume"
"github.com/vmware-tanzu/velero/pkg/util/collections"
"github.com/vmware-tanzu/velero/pkg/util/kube"
)
Expand Down Expand Up @@ -126,7 +126,7 @@ func (i *InitContainerRestoreHookHandler) HandleRestoreHooks(
// 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
// it before running any other init container.
if len(pod.Spec.InitContainers) > 0 && pod.Spec.InitContainers[0].Name == restic.InitContainer {
if len(pod.Spec.InitContainers) > 0 && pod.Spec.InitContainers[0].Name == podvolume.InitContainer {
initContainers = append(initContainers, pod.Spec.InitContainers[0])
pod.Spec.InitContainers = pod.Spec.InitContainers[1:]
}
Expand Down
12 changes: 10 additions & 2 deletions pkg/cmd/cli/restore/describe.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import (
"github.com/vmware-tanzu/velero/pkg/client"
"github.com/vmware-tanzu/velero/pkg/cmd"
"github.com/vmware-tanzu/velero/pkg/cmd/util/output"
"github.com/vmware-tanzu/velero/pkg/restic"
"github.com/vmware-tanzu/velero/pkg/label"
)

func NewDescribeCommand(f client.Factory, use string) *cobra.Command {
Expand Down Expand Up @@ -69,7 +69,7 @@ func NewDescribeCommand(f client.Factory, use string) *cobra.Command {

first := true
for _, restore := range restores.Items {
opts := restic.NewPodVolumeRestoreListOptions(restore.Name)
opts := newPodVolumeRestoreListOptions(restore.Name)
podvolumeRestoreList, err := veleroClient.VeleroV1().PodVolumeRestores(f.Namespace()).List(context.TODO(), opts)
if err != nil {
fmt.Fprintf(os.Stderr, "error getting PodVolumeRestores for restore %s: %v\n", restore.Name, err)
Expand All @@ -94,3 +94,11 @@ func NewDescribeCommand(f client.Factory, use string) *cobra.Command {

return c
}

// newPodVolumeRestoreListOptions creates a ListOptions with a label selector configured to
// find PodVolumeRestores for the restore identified by name.
func newPodVolumeRestoreListOptions(name string) metav1.ListOptions {
return metav1.ListOptions{
LabelSelector: fmt.Sprintf("%s=%s", velerov1api.RestoreNameLabel, label.GetValidName(name)),
}
}
5 changes: 4 additions & 1 deletion pkg/cmd/server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,9 @@ const (
// defaultCredentialsDirectory is the path on disk where credential
// files will be written to
defaultCredentialsDirectory = "/tmp/credentials"

// daemonSet is the name of the Velero restic daemonset.
daemonSet = "restic"
)

type serverConfig struct {
Expand Down Expand Up @@ -529,7 +532,7 @@ var defaultRestorePriorities = []string{

func (s *server) initRestic() error {
// warn if restic daemonset does not exist
if _, err := s.kubeClient.AppsV1().DaemonSets(s.namespace).Get(s.ctx, restic.DaemonSet, metav1.GetOptions{}); apierrors.IsNotFound(err) {
if _, err := s.kubeClient.AppsV1().DaemonSets(s.namespace).Get(s.ctx, daemonSet, metav1.GetOptions{}); apierrors.IsNotFound(err) {
s.logger.Warn("Velero restic daemonset not found; restic backups/restores will not work until it's created")
} else if err != nil {
s.logger.WithError(errors.WithStack(err)).Warn("Error checking for existence of velero restic daemonset")
Expand Down
33 changes: 31 additions & 2 deletions pkg/controller/backup_deletion_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ import (
"github.com/vmware-tanzu/velero/pkg/plugin/clientmgmt"
"github.com/vmware-tanzu/velero/pkg/plugin/velero"
"github.com/vmware-tanzu/velero/pkg/repository"
"github.com/vmware-tanzu/velero/pkg/restic"
"github.com/vmware-tanzu/velero/pkg/util/filesystem"
"github.com/vmware-tanzu/velero/pkg/util/kube"

Expand Down Expand Up @@ -440,7 +439,7 @@ func (r *backupDeletionReconciler) deleteResticSnapshots(ctx context.Context, ba
return nil
}

snapshots, err := restic.GetSnapshotsInBackup(ctx, backup, r.Client)
snapshots, err := getSnapshotsInBackup(ctx, backup, r.Client)
if err != nil {
return []error{err}
}
Expand Down Expand Up @@ -491,3 +490,33 @@ func (r *backupDeletionReconciler) patchBackup(ctx context.Context, backup *vele
}
return backup, nil
}

// getSnapshotsInBackup returns a list of all restic snapshot ids associated with
// a given Velero backup.
func getSnapshotsInBackup(ctx context.Context, backup *velerov1api.Backup, kbClient client.Client) ([]repository.SnapshotIdentifier, error) {
podVolumeBackups := &velerov1api.PodVolumeBackupList{}
options := &client.ListOptions{
LabelSelector: labels.Set(map[string]string{
velerov1api.BackupNameLabel: label.GetValidName(backup.Name),
}).AsSelector(),
}

err := kbClient.List(ctx, podVolumeBackups, options)
if err != nil {
return nil, errors.WithStack(err)
}

var res []repository.SnapshotIdentifier
for _, item := range podVolumeBackups.Items {
if item.Status.SnapshotID == "" {
continue
}
res = append(res, repository.SnapshotIdentifier{
VolumeNamespace: item.Spec.Pod.Namespace,
BackupStorageLocation: backup.Spec.StorageLocation,
SnapshotID: item.Status.SnapshotID,
})
}

return res, nil
}
172 changes: 172 additions & 0 deletions pkg/controller/backup_deletion_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package controller
import (
"bytes"
"fmt"
"sort"
"time"

"context"
Expand All @@ -32,6 +33,7 @@ import (

"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require"
corev1api "k8s.io/api/core/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
Expand All @@ -52,6 +54,7 @@ import (
persistencemocks "github.com/vmware-tanzu/velero/pkg/persistence/mocks"
"github.com/vmware-tanzu/velero/pkg/plugin/clientmgmt"
pluginmocks "github.com/vmware-tanzu/velero/pkg/plugin/mocks"
"github.com/vmware-tanzu/velero/pkg/repository"
velerotest "github.com/vmware-tanzu/velero/pkg/test"
)

Expand Down Expand Up @@ -692,3 +695,172 @@ func TestBackupDeletionControllerReconcile(t *testing.T) {

})
}

func TestGetSnapshotsInBackup(t *testing.T) {
tests := []struct {
name string
podVolumeBackups []velerov1api.PodVolumeBackup
expected []repository.SnapshotIdentifier
longBackupNameEnabled bool
}{
{
name: "no pod volume backups",
podVolumeBackups: nil,
expected: nil,
},
{
name: "no pod volume backups with matching label",
podVolumeBackups: []velerov1api.PodVolumeBackup{
{
ObjectMeta: metav1.ObjectMeta{Name: "foo", Labels: map[string]string{velerov1api.BackupNameLabel: "non-matching-backup-1"}},
Spec: velerov1api.PodVolumeBackupSpec{
Pod: corev1api.ObjectReference{Name: "pod-1", Namespace: "ns-1"},
},
Status: velerov1api.PodVolumeBackupStatus{SnapshotID: "snap-1"},
},
{
ObjectMeta: metav1.ObjectMeta{Name: "bar", Labels: map[string]string{velerov1api.BackupNameLabel: "non-matching-backup-2"}},
Spec: velerov1api.PodVolumeBackupSpec{
Pod: corev1api.ObjectReference{Name: "pod-2", Namespace: "ns-2"},
},
Status: velerov1api.PodVolumeBackupStatus{SnapshotID: "snap-2"},
},
},
expected: nil,
},
{
name: "some pod volume backups with matching label",
podVolumeBackups: []velerov1api.PodVolumeBackup{
{
ObjectMeta: metav1.ObjectMeta{Name: "foo", Labels: map[string]string{velerov1api.BackupNameLabel: "non-matching-backup-1"}},
Spec: velerov1api.PodVolumeBackupSpec{
Pod: corev1api.ObjectReference{Name: "pod-1", Namespace: "ns-1"},
},
Status: velerov1api.PodVolumeBackupStatus{SnapshotID: "snap-1"},
},
{
ObjectMeta: metav1.ObjectMeta{Name: "bar", Labels: map[string]string{velerov1api.BackupNameLabel: "non-matching-backup-2"}},
Spec: velerov1api.PodVolumeBackupSpec{
Pod: corev1api.ObjectReference{Name: "pod-2", Namespace: "ns-2"},
},
Status: velerov1api.PodVolumeBackupStatus{SnapshotID: "snap-2"},
},
{
ObjectMeta: metav1.ObjectMeta{Name: "completed-pvb", Labels: map[string]string{velerov1api.BackupNameLabel: "backup-1"}},
Spec: velerov1api.PodVolumeBackupSpec{
Pod: corev1api.ObjectReference{Name: "pod-1", Namespace: "ns-1"},
},
Status: velerov1api.PodVolumeBackupStatus{SnapshotID: "snap-3"},
},
{
ObjectMeta: metav1.ObjectMeta{Name: "completed-pvb-2", Labels: map[string]string{velerov1api.BackupNameLabel: "backup-1"}},
Spec: velerov1api.PodVolumeBackupSpec{
Pod: corev1api.ObjectReference{Name: "pod-1", Namespace: "ns-1"},
},
Status: velerov1api.PodVolumeBackupStatus{SnapshotID: "snap-4"},
},
{
ObjectMeta: metav1.ObjectMeta{Name: "incomplete-or-failed-pvb", Labels: map[string]string{velerov1api.BackupNameLabel: "backup-1"}},
Spec: velerov1api.PodVolumeBackupSpec{
Pod: corev1api.ObjectReference{Name: "pod-1", Namespace: "ns-2"},
},
Status: velerov1api.PodVolumeBackupStatus{SnapshotID: ""},
},
},
expected: []repository.SnapshotIdentifier{
{
VolumeNamespace: "ns-1",
SnapshotID: "snap-3",
},
{
VolumeNamespace: "ns-1",
SnapshotID: "snap-4",
},
},
},
{
name: "some pod volume backups with matching label and backup name greater than 63 chars",
longBackupNameEnabled: true,
podVolumeBackups: []velerov1api.PodVolumeBackup{
{
ObjectMeta: metav1.ObjectMeta{Name: "foo", Labels: map[string]string{velerov1api.BackupNameLabel: "non-matching-backup-1"}},
Spec: velerov1api.PodVolumeBackupSpec{
Pod: corev1api.ObjectReference{Name: "pod-1", Namespace: "ns-1"},
},
Status: velerov1api.PodVolumeBackupStatus{SnapshotID: "snap-1"},
},
{
ObjectMeta: metav1.ObjectMeta{Name: "bar", Labels: map[string]string{velerov1api.BackupNameLabel: "non-matching-backup-2"}},
Spec: velerov1api.PodVolumeBackupSpec{
Pod: corev1api.ObjectReference{Name: "pod-2", Namespace: "ns-2"},
},
Status: velerov1api.PodVolumeBackupStatus{SnapshotID: "snap-2"},
},
{
ObjectMeta: metav1.ObjectMeta{Name: "completed-pvb", Labels: map[string]string{velerov1api.BackupNameLabel: "the-really-long-backup-name-that-is-much-more-than-63-cha6ca4bc"}},
Spec: velerov1api.PodVolumeBackupSpec{
Pod: corev1api.ObjectReference{Name: "pod-1", Namespace: "ns-1"},
},
Status: velerov1api.PodVolumeBackupStatus{SnapshotID: "snap-3"},
},
{
ObjectMeta: metav1.ObjectMeta{Name: "completed-pvb-2", Labels: map[string]string{velerov1api.BackupNameLabel: "backup-1"}},
Spec: velerov1api.PodVolumeBackupSpec{
Pod: corev1api.ObjectReference{Name: "pod-1", Namespace: "ns-1"},
},
Status: velerov1api.PodVolumeBackupStatus{SnapshotID: "snap-4"},
},
{
ObjectMeta: metav1.ObjectMeta{Name: "incomplete-or-failed-pvb", Labels: map[string]string{velerov1api.BackupNameLabel: "backup-1"}},
Spec: velerov1api.PodVolumeBackupSpec{
Pod: corev1api.ObjectReference{Name: "pod-1", Namespace: "ns-2"},
},
Status: velerov1api.PodVolumeBackupStatus{SnapshotID: ""},
},
},
expected: []repository.SnapshotIdentifier{
{
VolumeNamespace: "ns-1",
SnapshotID: "snap-3",
},
},
},
}

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
var (
clientBuilder = velerotest.NewFakeControllerRuntimeClientBuilder(t)
veleroBackup = &velerov1api.Backup{}
)

veleroBackup.Name = "backup-1"

if test.longBackupNameEnabled {
veleroBackup.Name = "the-really-long-backup-name-that-is-much-more-than-63-characters"
}
clientBuilder.WithLists(&velerov1api.PodVolumeBackupList{
Items: test.podVolumeBackups,
})

res, err := getSnapshotsInBackup(context.TODO(), veleroBackup, clientBuilder.Build())
assert.NoError(t, err)

// sort to ensure good compare of slices
less := func(snapshots []repository.SnapshotIdentifier) func(i, j int) bool {
return func(i, j int) bool {
if snapshots[i].VolumeNamespace == snapshots[j].VolumeNamespace {
return snapshots[i].SnapshotID < snapshots[j].SnapshotID
}
return snapshots[i].VolumeNamespace < snapshots[j].VolumeNamespace
}

}

sort.Slice(test.expected, less(test.expected))
sort.Slice(res, less(res))

assert.Equal(t, test.expected, res)
})
}
}
5 changes: 3 additions & 2 deletions pkg/controller/pod_volume_restore_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import (

"github.com/vmware-tanzu/velero/internal/credentials"
velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1"
"github.com/vmware-tanzu/velero/pkg/podvolume"
repokey "github.com/vmware-tanzu/velero/pkg/repository/keys"
"github.com/vmware-tanzu/velero/pkg/restic"
"github.com/vmware-tanzu/velero/pkg/uploader"
Expand Down Expand Up @@ -106,7 +107,7 @@ func (c *PodVolumeRestoreReconciler) Reconcile(ctx context.Context, req ctrl.Req
resticInitContainerIndex := getResticInitContainerIndex(pod)
if resticInitContainerIndex > 0 {
log.Warnf(`Init containers before the %s container may cause issues
if they interfere with volumes being restored: %s index %d`, restic.InitContainer, restic.InitContainer, resticInitContainerIndex)
if they interfere with volumes being restored: %s index %d`, podvolume.InitContainer, podvolume.InitContainer, resticInitContainerIndex)
}

log.Info("Restore starting")
Expand Down Expand Up @@ -216,7 +217,7 @@ func isResticInitContainerRunning(pod *corev1api.Pod) bool {
func getResticInitContainerIndex(pod *corev1api.Pod) int {
// Restic wait container can be anywhere in the list of init containers so locate it.
for i, initContainer := range pod.Spec.InitContainers {
if initContainer.Name == restic.InitContainer {
if initContainer.Name == podvolume.InitContainer {
return i
}
}
Expand Down
Loading

0 comments on commit 4a5647a

Please sign in to comment.