From 3f9c2dc789afc3c0a464500148e211d94bdf3fab Mon Sep 17 00:00:00 2001 From: Tiger Kaovilai Date: Thu, 12 Sep 2024 22:21:51 -0400 Subject: [PATCH] Reduces ~140 indirect imports for plugin/framework importers (#8208) * Avoid plugin framework importers from needing cloud provider imports Signed-off-by: Tiger Kaovilai --- changelogs/unreleased/8208-kaovilai | 1 + pkg/cmd/server/config/config.go | 4 +- pkg/cmd/server/server.go | 5 +- pkg/controller/backup_deletion_controller.go | 18 ++++--- .../backup_deletion_controller_test.go | 30 ++++++----- .../backup_repository_controller.go | 12 ++--- .../backup_repository_controller_test.go | 4 +- pkg/datamover/dataupload_delete_action.go | 4 +- pkg/install/import_test.go | 53 ++++++++++++++++++ pkg/plugin/framework/import_test.go | 53 ++++++++++++++++++ pkg/podvolume/backupper.go | 3 +- pkg/podvolume/configs/configs.go | 11 ++++ pkg/podvolume/util.go | 18 ++----- pkg/repository/maintenance.go | 16 +++--- pkg/repository/maintenance_test.go | 12 ++--- pkg/repository/{ => manager}/manager.go | 54 +++++-------------- pkg/repository/{ => manager}/manager_test.go | 9 ++-- pkg/repository/types/snapshotidentifier.go | 30 +++++++++++ pkg/restore/restore.go | 3 +- 19 files changed, 230 insertions(+), 110 deletions(-) create mode 100644 changelogs/unreleased/8208-kaovilai create mode 100644 pkg/install/import_test.go create mode 100644 pkg/plugin/framework/import_test.go create mode 100644 pkg/podvolume/configs/configs.go rename pkg/repository/{ => manager}/manager.go (90%) rename pkg/repository/{ => manager}/manager_test.go (96%) create mode 100644 pkg/repository/types/snapshotidentifier.go diff --git a/changelogs/unreleased/8208-kaovilai b/changelogs/unreleased/8208-kaovilai new file mode 100644 index 0000000000..4cb1106eb2 --- /dev/null +++ b/changelogs/unreleased/8208-kaovilai @@ -0,0 +1 @@ +Reduces indirect imports for plugin/framework importers diff --git a/pkg/cmd/server/config/config.go b/pkg/cmd/server/config/config.go index 032cfdd763..ac9fe69df5 100644 --- a/pkg/cmd/server/config/config.go +++ b/pkg/cmd/server/config/config.go @@ -10,7 +10,7 @@ import ( "github.com/vmware-tanzu/velero/pkg/cmd/util/flag" "github.com/vmware-tanzu/velero/pkg/constant" - "github.com/vmware-tanzu/velero/pkg/podvolume" + podvolumeconfigs "github.com/vmware-tanzu/velero/pkg/podvolume/configs" "github.com/vmware-tanzu/velero/pkg/types" "github.com/vmware-tanzu/velero/pkg/uploader" "github.com/vmware-tanzu/velero/pkg/util/kube" @@ -198,7 +198,7 @@ func GetDefaultConfig() *Config { ResourceTerminatingTimeout: defaultResourceTerminatingTimeout, LogLevel: logging.LogLevelFlag(logrus.InfoLevel), LogFormat: logging.NewFormatFlag(), - DefaultVolumesToFsBackup: podvolume.DefaultVolumesToFsBackup, + DefaultVolumesToFsBackup: podvolumeconfigs.DefaultVolumesToFsBackup, UploaderType: uploader.ResticType, MaxConcurrentK8SConnections: defaultMaxConcurrentK8SConnections, DefaultSnapshotMoveData: false, diff --git a/pkg/cmd/server/server.go b/pkg/cmd/server/server.go index 55b4229b2f..a697e4d01b 100644 --- a/pkg/cmd/server/server.go +++ b/pkg/cmd/server/server.go @@ -78,6 +78,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" + repomanager "github.com/vmware-tanzu/velero/pkg/repository/manager" "github.com/vmware-tanzu/velero/pkg/restore" "github.com/vmware-tanzu/velero/pkg/uploader" "github.com/vmware-tanzu/velero/pkg/util/filesystem" @@ -147,7 +148,7 @@ type server struct { logger logrus.FieldLogger logLevel logrus.Level pluginRegistry process.Registry - repoManager repository.Manager + repoManager repomanager.Manager repoLocker *repository.RepoLocker repoEnsurer *repository.Ensurer metrics *metrics.ServerMetrics @@ -469,7 +470,7 @@ func (s *server) initRepoManager() error { s.repoLocker = repository.NewRepoLocker() s.repoEnsurer = repository.NewEnsurer(s.mgr.GetClient(), s.logger, s.config.ResourceTimeout) - s.repoManager = repository.NewManager( + s.repoManager = repomanager.NewManager( s.namespace, s.mgr.GetClient(), s.repoLocker, diff --git a/pkg/controller/backup_deletion_controller.go b/pkg/controller/backup_deletion_controller.go index cace160dc9..e76ff5cbbe 100644 --- a/pkg/controller/backup_deletion_controller.go +++ b/pkg/controller/backup_deletion_controller.go @@ -49,6 +49,8 @@ import ( vsv1 "github.com/vmware-tanzu/velero/pkg/plugin/velero/volumesnapshotter/v1" "github.com/vmware-tanzu/velero/pkg/podvolume" "github.com/vmware-tanzu/velero/pkg/repository" + repomanager "github.com/vmware-tanzu/velero/pkg/repository/manager" + repotypes "github.com/vmware-tanzu/velero/pkg/repository/types" "github.com/vmware-tanzu/velero/pkg/util/boolptr" "github.com/vmware-tanzu/velero/pkg/util/filesystem" "github.com/vmware-tanzu/velero/pkg/util/kube" @@ -62,7 +64,7 @@ type backupDeletionReconciler struct { client.Client logger logrus.FieldLogger backupTracker BackupTracker - repoMgr repository.Manager + repoMgr repomanager.Manager metrics *metrics.ServerMetrics clock clock.Clock discoveryHelper discovery.Helper @@ -77,7 +79,7 @@ func NewBackupDeletionReconciler( logger logrus.FieldLogger, client client.Client, backupTracker BackupTracker, - repoMgr repository.Manager, + repoMgr repomanager.Manager, metrics *metrics.ServerMetrics, helper discovery.Helper, newPluginManager func(logrus.FieldLogger) clientmgmt.Manager, @@ -524,7 +526,7 @@ func (r *backupDeletionReconciler) deleteMovedSnapshots(ctx context.Context, bac return []error{errors.Wrapf(err, "failed to retrieve config for snapshot info")} } var errs []error - directSnapshots := map[string][]repository.SnapshotIdentifier{} + directSnapshots := map[string][]repotypes.SnapshotIdentifier{} for i := range list.Items { cm := list.Items[i] if cm.Data == nil || len(cm.Data) == 0 { @@ -538,7 +540,7 @@ func (r *backupDeletionReconciler) deleteMovedSnapshots(ctx context.Context, bac continue } - snapshot := repository.SnapshotIdentifier{} + snapshot := repotypes.SnapshotIdentifier{} if err := json.Unmarshal(b, &snapshot); err != nil { errs = append(errs, errors.Wrapf(err, "failed to unmarshal snapshot info")) continue @@ -550,7 +552,7 @@ func (r *backupDeletionReconciler) deleteMovedSnapshots(ctx context.Context, bac } if directSnapshots[snapshot.VolumeNamespace] == nil { - directSnapshots[snapshot.VolumeNamespace] = []repository.SnapshotIdentifier{} + directSnapshots[snapshot.VolumeNamespace] = []repotypes.SnapshotIdentifier{} } directSnapshots[snapshot.VolumeNamespace] = append(directSnapshots[snapshot.VolumeNamespace], snapshot) @@ -618,7 +620,7 @@ func (r *backupDeletionReconciler) patchBackup(ctx context.Context, backup *vele // getSnapshotsInBackup returns a list of all pod volume snapshot ids associated with // a given Velero backup. -func getSnapshotsInBackup(ctx context.Context, backup *velerov1api.Backup, kbClient client.Client) (map[string][]repository.SnapshotIdentifier, error) { +func getSnapshotsInBackup(ctx context.Context, backup *velerov1api.Backup, kbClient client.Client) (map[string][]repotypes.SnapshotIdentifier, error) { podVolumeBackups := &velerov1api.PodVolumeBackupList{} options := &client.ListOptions{ LabelSelector: labels.Set(map[string]string{ @@ -634,8 +636,8 @@ func getSnapshotsInBackup(ctx context.Context, backup *velerov1api.Backup, kbCli return podvolume.GetSnapshotIdentifier(podVolumeBackups), nil } -func batchDeleteSnapshots(ctx context.Context, repoEnsurer *repository.Ensurer, repoMgr repository.Manager, - directSnapshots map[string][]repository.SnapshotIdentifier, backup *velerov1api.Backup, logger logrus.FieldLogger) []error { +func batchDeleteSnapshots(ctx context.Context, repoEnsurer *repository.Ensurer, repoMgr repomanager.Manager, + directSnapshots map[string][]repotypes.SnapshotIdentifier, backup *velerov1api.Backup, logger logrus.FieldLogger) []error { var errs []error for volumeNamespace, snapshots := range directSnapshots { batchForget := []string{} diff --git a/pkg/controller/backup_deletion_controller_test.go b/pkg/controller/backup_deletion_controller_test.go index c1c5b70187..426ee000ce 100644 --- a/pkg/controller/backup_deletion_controller_test.go +++ b/pkg/controller/backup_deletion_controller_test.go @@ -55,7 +55,9 @@ import ( "github.com/vmware-tanzu/velero/pkg/plugin/velero" "github.com/vmware-tanzu/velero/pkg/plugin/velero/mocks" "github.com/vmware-tanzu/velero/pkg/repository" + repomanager "github.com/vmware-tanzu/velero/pkg/repository/manager" repomocks "github.com/vmware-tanzu/velero/pkg/repository/mocks" + repotypes "github.com/vmware-tanzu/velero/pkg/repository/types" velerotest "github.com/vmware-tanzu/velero/pkg/test" ) @@ -698,13 +700,13 @@ func TestGetSnapshotsInBackup(t *testing.T) { tests := []struct { name string podVolumeBackups []velerov1api.PodVolumeBackup - expected map[string][]repository.SnapshotIdentifier + expected map[string][]repotypes.SnapshotIdentifier longBackupNameEnabled bool }{ { name: "no pod volume backups", podVolumeBackups: nil, - expected: map[string][]repository.SnapshotIdentifier{}, + expected: map[string][]repotypes.SnapshotIdentifier{}, }, { name: "no pod volume backups with matching label", @@ -724,7 +726,7 @@ func TestGetSnapshotsInBackup(t *testing.T) { Status: velerov1api.PodVolumeBackupStatus{SnapshotID: "snap-2"}, }, }, - expected: map[string][]repository.SnapshotIdentifier{}, + expected: map[string][]repotypes.SnapshotIdentifier{}, }, { name: "some pod volume backups with matching label", @@ -765,7 +767,7 @@ func TestGetSnapshotsInBackup(t *testing.T) { Status: velerov1api.PodVolumeBackupStatus{SnapshotID: ""}, }, }, - expected: map[string][]repository.SnapshotIdentifier{ + expected: map[string][]repotypes.SnapshotIdentifier{ "ns-1": { { VolumeNamespace: "ns-1", @@ -820,7 +822,7 @@ func TestGetSnapshotsInBackup(t *testing.T) { Status: velerov1api.PodVolumeBackupStatus{SnapshotID: ""}, }, }, - expected: map[string][]repository.SnapshotIdentifier{ + expected: map[string][]repotypes.SnapshotIdentifier{ "ns-1": { { VolumeNamespace: "ns-1", @@ -856,18 +858,18 @@ func TestGetSnapshotsInBackup(t *testing.T) { } } -func batchDeleteSucceed(ctx context.Context, repoEnsurer *repository.Ensurer, repoMgr repository.Manager, directSnapshots map[string][]repository.SnapshotIdentifier, backup *velerov1api.Backup, logger logrus.FieldLogger) []error { +func batchDeleteSucceed(ctx context.Context, repoEnsurer *repository.Ensurer, repoMgr repomanager.Manager, directSnapshots map[string][]repotypes.SnapshotIdentifier, backup *velerov1api.Backup, logger logrus.FieldLogger) []error { return nil } -func batchDeleteFail(ctx context.Context, repoEnsurer *repository.Ensurer, repoMgr repository.Manager, directSnapshots map[string][]repository.SnapshotIdentifier, backup *velerov1api.Backup, logger logrus.FieldLogger) []error { +func batchDeleteFail(ctx context.Context, repoEnsurer *repository.Ensurer, repoMgr repomanager.Manager, directSnapshots map[string][]repotypes.SnapshotIdentifier, backup *velerov1api.Backup, logger logrus.FieldLogger) []error { return []error{ errors.New("fake-delete-1"), errors.New("fake-delete-2"), } } -func generateSnapshotData(snapshot *repository.SnapshotIdentifier) (map[string]string, error) { +func generateSnapshotData(snapshot *repotypes.SnapshotIdentifier) (map[string]string, error) { if snapshot == nil { return nil, nil } @@ -888,10 +890,10 @@ func generateSnapshotData(snapshot *repository.SnapshotIdentifier) (map[string]s func TestDeleteMovedSnapshots(t *testing.T) { tests := []struct { name string - repoMgr repository.Manager + repoMgr repomanager.Manager batchDeleteSucceed bool backupName string - snapshots []*repository.SnapshotIdentifier + snapshots []*repotypes.SnapshotIdentifier expected []string }{ { @@ -905,14 +907,14 @@ func TestDeleteMovedSnapshots(t *testing.T) { name: "bad cm info", repoMgr: repomocks.NewManager(t), backupName: "backup-01", - snapshots: []*repository.SnapshotIdentifier{nil}, + snapshots: []*repotypes.SnapshotIdentifier{nil}, expected: []string{"no snapshot info in config"}, }, { name: "invalid snapshots", repoMgr: repomocks.NewManager(t), backupName: "backup-01", - snapshots: []*repository.SnapshotIdentifier{ + snapshots: []*repotypes.SnapshotIdentifier{ { RepositoryType: "repo-1", VolumeNamespace: "ns-1", @@ -937,7 +939,7 @@ func TestDeleteMovedSnapshots(t *testing.T) { name: "batch delete succeed", repoMgr: repomocks.NewManager(t), backupName: "backup-01", - snapshots: []*repository.SnapshotIdentifier{ + snapshots: []*repotypes.SnapshotIdentifier{ { SnapshotID: "snapshot-1", @@ -952,7 +954,7 @@ func TestDeleteMovedSnapshots(t *testing.T) { name: "batch delete fail", repoMgr: repomocks.NewManager(t), backupName: "backup-01", - snapshots: []*repository.SnapshotIdentifier{ + snapshots: []*repotypes.SnapshotIdentifier{ { RepositoryType: "repo-1", VolumeNamespace: "ns-1", diff --git a/pkg/controller/backup_repository_controller.go b/pkg/controller/backup_repository_controller.go index 6c2b21a9bc..fbe193095c 100644 --- a/pkg/controller/backup_repository_controller.go +++ b/pkg/controller/backup_repository_controller.go @@ -35,14 +35,14 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/reconcile" + corev1api "k8s.io/api/core/v1" + velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" "github.com/vmware-tanzu/velero/pkg/constant" "github.com/vmware-tanzu/velero/pkg/label" - "github.com/vmware-tanzu/velero/pkg/repository" repoconfig "github.com/vmware-tanzu/velero/pkg/repository/config" + repomanager "github.com/vmware-tanzu/velero/pkg/repository/manager" "github.com/vmware-tanzu/velero/pkg/util/kube" - - corev1api "k8s.io/api/core/v1" ) const ( @@ -57,11 +57,11 @@ type BackupRepoReconciler struct { clock clocks.WithTickerAndDelayedExecution maintenanceFrequency time.Duration backupRepoConfig string - repositoryManager repository.Manager + repositoryManager repomanager.Manager } func NewBackupRepoReconciler(namespace string, logger logrus.FieldLogger, client client.Client, - maintenanceFrequency time.Duration, backupRepoConfig string, repositoryManager repository.Manager) *BackupRepoReconciler { + maintenanceFrequency time.Duration, backupRepoConfig string, repositoryManager repomanager.Manager) *BackupRepoReconciler { c := &BackupRepoReconciler{ client, namespace, @@ -294,7 +294,7 @@ func (r *BackupRepoReconciler) getRepositoryMaintenanceFrequency(req *velerov1ap // ensureRepo calls repo manager's PrepareRepo to ensure the repo is ready for use. // An error is returned if the repository can't be connected to or initialized. -func ensureRepo(repo *velerov1api.BackupRepository, repoManager repository.Manager) error { +func ensureRepo(repo *velerov1api.BackupRepository, repoManager repomanager.Manager) error { return repoManager.PrepareRepo(repo) } diff --git a/pkg/controller/backup_repository_controller_test.go b/pkg/controller/backup_repository_controller_test.go index 873d2ce18f..3dab5efa74 100644 --- a/pkg/controller/backup_repository_controller_test.go +++ b/pkg/controller/backup_repository_controller_test.go @@ -28,8 +28,8 @@ import ( ctrl "sigs.k8s.io/controller-runtime" velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" - "github.com/vmware-tanzu/velero/pkg/repository" repomokes "github.com/vmware-tanzu/velero/pkg/repository/mocks" + repotypes "github.com/vmware-tanzu/velero/pkg/repository/types" velerotest "github.com/vmware-tanzu/velero/pkg/test" clientFake "sigs.k8s.io/controller-runtime/pkg/client/fake" @@ -210,7 +210,7 @@ func TestBackupRepoReconcile(t *testing.T) { func TestGetRepositoryMaintenanceFrequency(t *testing.T) { tests := []struct { name string - mgr repository.Manager + mgr repotypes.SnapshotIdentifier repo *velerov1api.BackupRepository freqReturn time.Duration freqError error diff --git a/pkg/datamover/dataupload_delete_action.go b/pkg/datamover/dataupload_delete_action.go index 18501719d0..1c09a20a2d 100644 --- a/pkg/datamover/dataupload_delete_action.go +++ b/pkg/datamover/dataupload_delete_action.go @@ -15,7 +15,7 @@ import ( velerov1 "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" velerov2alpha1 "github.com/vmware-tanzu/velero/pkg/apis/velero/v2alpha1" "github.com/vmware-tanzu/velero/pkg/plugin/velero" - "github.com/vmware-tanzu/velero/pkg/repository" + repotypes "github.com/vmware-tanzu/velero/pkg/repository/types" ) type DataUploadDeleteAction struct { @@ -52,7 +52,7 @@ func genConfigmap(bak *velerov1.Backup, du velerov2alpha1.DataUpload) *corev1api if !IsBuiltInUploader(du.Spec.DataMover) || du.Status.SnapshotID == "" { return nil } - snapshot := repository.SnapshotIdentifier{ + snapshot := repotypes.SnapshotIdentifier{ VolumeNamespace: du.Spec.SourceNamespace, BackupStorageLocation: bak.Spec.StorageLocation, SnapshotID: du.Status.SnapshotID, diff --git a/pkg/install/import_test.go b/pkg/install/import_test.go new file mode 100644 index 0000000000..ae5e8b41df --- /dev/null +++ b/pkg/install/import_test.go @@ -0,0 +1,53 @@ +package install + +import ( + "os/exec" + "path/filepath" + "regexp" + "runtime" + "strings" + "testing" + + "github.com/stretchr/testify/require" +) + +// test that this package do not import cloud provider + +// Prevent https://github.com/vmware-tanzu/velero/issues/8207 and https://github.com/vmware-tanzu/velero/issues/8157 +func TestPkgImportNoCloudProvider(t *testing.T) { + _, filename, _, ok := runtime.Caller(0) + if !ok { + t.Fatalf("No caller information") + } + t.Logf("Current test file path: %s", filename) + t.Logf("Current test directory: %s", filepath.Dir(filename)) // should be this package name + // go list -f {{.Deps}} ./ + cmd := exec.Command( + "go", + "list", + "-f", + "{{.Deps}}", + ".", + ) + // set cmd.Dir to this package even if executed from different dir + cmd.Dir = filepath.Dir(filename) + output, err := cmd.Output() + require.NoError(t, err) + // split dep by line, replace space with newline + deps := strings.ReplaceAll(string(output), " ", "\n") + require.NotEmpty(t, deps) + // ignore k8s.io + k8sio, err := regexp.Compile("^k8s.io") + require.NoError(t, err) + cloudProvider, err := regexp.Compile("aws|cloud.google.com|azure") + require.NoError(t, err) + cloudProviderDeps := []string{} + for _, dep := range strings.Split(deps, "\n") { + if !k8sio.MatchString(dep) { + if cloudProvider.MatchString(dep) { + cloudProviderDeps = append(cloudProviderDeps, dep) + } + } + } + require.Empty(t, cloudProviderDeps) +} diff --git a/pkg/plugin/framework/import_test.go b/pkg/plugin/framework/import_test.go new file mode 100644 index 0000000000..29be811227 --- /dev/null +++ b/pkg/plugin/framework/import_test.go @@ -0,0 +1,53 @@ +package framework + +import ( + "os/exec" + "path/filepath" + "regexp" + "runtime" + "strings" + "testing" + + "github.com/stretchr/testify/require" +) + +// test that this package do not import cloud provider + +// Prevent https://github.com/vmware-tanzu/velero/issues/8207 and https://github.com/vmware-tanzu/velero/issues/8157 +func TestPkgImportNoCloudProvider(t *testing.T) { + _, filename, _, ok := runtime.Caller(0) + if !ok { + t.Fatalf("No caller information") + } + t.Logf("Current test file path: %s", filename) + t.Logf("Current test directory: %s", filepath.Dir(filename)) // should be this package name + // go list -f {{.Deps}} ./ + cmd := exec.Command( + "go", + "list", + "-f", + "{{.Deps}}", + ".", + ) + // set cmd.Dir to this package even if executed from different dir + cmd.Dir = filepath.Dir(filename) + output, err := cmd.Output() + require.NoError(t, err) + // split dep by line, replace space with newline + deps := strings.ReplaceAll(string(output), " ", "\n") + require.NotEmpty(t, deps) + // ignore k8s.io + k8sio, err := regexp.Compile("^k8s.io") + require.NoError(t, err) + cloudProvider, err := regexp.Compile("aws|cloud.google.com|azure") + require.NoError(t, err) + cloudProviderDeps := []string{} + for _, dep := range strings.Split(deps, "\n") { + if !k8sio.MatchString(dep) { + if cloudProvider.MatchString(dep) { + cloudProviderDeps = append(cloudProviderDeps, dep) + } + } + } + require.Empty(t, cloudProviderDeps) +} diff --git a/pkg/podvolume/backupper.go b/pkg/podvolume/backupper.go index 5576f4e407..0a0c63eff1 100644 --- a/pkg/podvolume/backupper.go +++ b/pkg/podvolume/backupper.go @@ -35,6 +35,7 @@ import ( veleroclient "github.com/vmware-tanzu/velero/pkg/client" "github.com/vmware-tanzu/velero/pkg/label" "github.com/vmware-tanzu/velero/pkg/nodeagent" + "github.com/vmware-tanzu/velero/pkg/podvolume/configs" "github.com/vmware-tanzu/velero/pkg/repository" "github.com/vmware-tanzu/velero/pkg/uploader" uploaderutil "github.com/vmware-tanzu/velero/pkg/uploader/util" @@ -419,7 +420,7 @@ func newPodVolumeBackup(backup *velerov1api.Backup, pod *corev1api.Pod, volume c // this annotation is used in pkg/restore to identify if a PVC // has a pod volume backup. pvb.Annotations = map[string]string{ - PVCNameAnnotation: pvc.Name, + configs.PVCNameAnnotation: pvc.Name, } // this label is used by the pod volume backup controller to tell diff --git a/pkg/podvolume/configs/configs.go b/pkg/podvolume/configs/configs.go new file mode 100644 index 0000000000..17d57671c9 --- /dev/null +++ b/pkg/podvolume/configs/configs.go @@ -0,0 +1,11 @@ +package configs + +const ( + // PVCNameAnnotation is the key for the annotation added to + // pod volume backups when they're for a PVC. + PVCNameAnnotation = "velero.io/pvc-name" + + // DefaultVolumesToFsBackup specifies whether pod volume backup should be used, by default, to + // take backup of all pod volumes. + DefaultVolumesToFsBackup = false +) diff --git a/pkg/podvolume/util.go b/pkg/podvolume/util.go index b1dfcbe65b..e9bb3075cc 100644 --- a/pkg/podvolume/util.go +++ b/pkg/podvolume/util.go @@ -23,23 +23,15 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" - "github.com/vmware-tanzu/velero/pkg/repository" + repotypes "github.com/vmware-tanzu/velero/pkg/repository/types" "github.com/vmware-tanzu/velero/pkg/uploader" ) const ( - // PVCNameAnnotation is the key for the annotation added to - // pod volume backups when they're for a PVC. - PVCNameAnnotation = "velero.io/pvc-name" - // Deprecated. // // TODO(2.0): remove podAnnotationPrefix = "snapshot.velero.io/" - - // DefaultVolumesToFsBackup specifies whether pod volume backup should be used, by default, to - // take backup of all pod volumes. - DefaultVolumesToFsBackup = false ) // volumeBackupInfo describes the backup info of a volume backed up by PodVolumeBackups @@ -122,20 +114,20 @@ func getVolumeBackupInfoForPod(podVolumeBackups []*velerov1api.PodVolumeBackup, } // GetSnapshotIdentifier returns the snapshots represented by SnapshotIdentifier for the given PVBs -func GetSnapshotIdentifier(podVolumeBackups *velerov1api.PodVolumeBackupList) map[string][]repository.SnapshotIdentifier { - res := map[string][]repository.SnapshotIdentifier{} +func GetSnapshotIdentifier(podVolumeBackups *velerov1api.PodVolumeBackupList) map[string][]repotypes.SnapshotIdentifier { + res := map[string][]repotypes.SnapshotIdentifier{} for _, item := range podVolumeBackups.Items { if item.Status.SnapshotID == "" { continue } if res[item.Spec.Pod.Namespace] == nil { - res[item.Spec.Pod.Namespace] = []repository.SnapshotIdentifier{} + res[item.Spec.Pod.Namespace] = []repotypes.SnapshotIdentifier{} } snapshots := res[item.Spec.Pod.Namespace] - snapshots = append(snapshots, repository.SnapshotIdentifier{ + snapshots = append(snapshots, repotypes.SnapshotIdentifier{ VolumeNamespace: item.Spec.Pod.Namespace, BackupStorageLocation: item.Spec.BackupStorageLocation, SnapshotID: item.Status.SnapshotID, diff --git a/pkg/repository/maintenance.go b/pkg/repository/maintenance.go index 3df82f8fb9..1a137cba90 100644 --- a/pkg/repository/maintenance.go +++ b/pkg/repository/maintenance.go @@ -50,7 +50,7 @@ type JobConfigs struct { PodResources *kube.PodResources `json:"podResources,omitempty"` } -func generateJobName(repo string) string { +func GenerateJobName(repo string) string { millisecond := time.Now().UTC().UnixMilli() // millisecond jobName := fmt.Sprintf("%s-maintain-job-%d", repo, millisecond) @@ -61,8 +61,8 @@ func generateJobName(repo string) string { return jobName } -// deleteOldMaintenanceJobs deletes old maintenance jobs and keeps the latest N jobs -func deleteOldMaintenanceJobs(cli client.Client, repo string, keep int) error { +// DeleteOldMaintenanceJobs deletes old maintenance jobs and keeps the latest N jobs +func DeleteOldMaintenanceJobs(cli client.Client, repo string, keep int) error { // Get the maintenance job list by label jobList := &batchv1.JobList{} err := cli.List(context.TODO(), jobList, client.MatchingLabels(map[string]string{RepositoryNameLabel: repo})) @@ -86,7 +86,7 @@ func deleteOldMaintenanceJobs(cli client.Client, repo string, keep int) error { return nil } -func waitForJobComplete(ctx context.Context, client client.Client, job *batchv1.Job) error { +func WaitForJobComplete(ctx context.Context, client client.Client, job *batchv1.Job) error { return wait.PollUntilContextCancel(ctx, 1, true, func(ctx context.Context) (bool, error) { err := client.Get(ctx, types.NamespacedName{Namespace: job.Namespace, Name: job.Name}, job) if err != nil && !apierrors.IsNotFound(err) { @@ -104,7 +104,7 @@ func waitForJobComplete(ctx context.Context, client client.Client, job *batchv1. }) } -func getMaintenanceResultFromJob(cli client.Client, job *batchv1.Job) (string, error) { +func GetMaintenanceResultFromJob(cli client.Client, job *batchv1.Job) (string, error) { // Get the maintenance job related pod by label selector podList := &v1.PodList{} err := cli.List(context.TODO(), podList, client.InNamespace(job.Namespace), client.MatchingLabels(map[string]string{"job-name": job.Name})) @@ -120,7 +120,7 @@ func getMaintenanceResultFromJob(cli client.Client, job *batchv1.Job) (string, e return podList.Items[0].Status.ContainerStatuses[0].State.Terminated.Message, nil } -func getLatestMaintenanceJob(cli client.Client, ns string) (*batchv1.Job, error) { +func GetLatestMaintenanceJob(cli client.Client, ns string) (*batchv1.Job, error) { // Get the maintenance job list by label jobList := &batchv1.JobList{} err := cli.List(context.TODO(), jobList, &client.ListOptions{ @@ -145,7 +145,7 @@ func getLatestMaintenanceJob(cli client.Client, ns string) (*batchv1.Job, error) return &jobList.Items[0], nil } -// getMaintenanceJobConfig is called to get the Maintenance Job Config for the +// GetMaintenanceJobConfig is called to get the Maintenance Job Config for the // BackupRepository specified by the repo parameter. // // Params: @@ -156,7 +156,7 @@ func getLatestMaintenanceJob(cli client.Client, ns string) (*batchv1.Job, error) // veleroNamespace: the Velero-installed namespace. It's used to retrieve the BackupRepository. // repoMaintenanceJobConfig: the repository maintenance job ConfigMap name. // repo: the BackupRepository needs to run the maintenance Job. -func getMaintenanceJobConfig( +func GetMaintenanceJobConfig( ctx context.Context, client client.Client, logger logrus.FieldLogger, diff --git a/pkg/repository/maintenance_test.go b/pkg/repository/maintenance_test.go index f7850d5d6d..1ef4c2c89b 100644 --- a/pkg/repository/maintenance_test.go +++ b/pkg/repository/maintenance_test.go @@ -56,7 +56,7 @@ func TestGenerateJobName1(t *testing.T) { for _, tc := range testCases { t.Run(tc.repo, func(t *testing.T) { // Call the function to test - jobName := generateJobName(tc.repo) + jobName := GenerateJobName(tc.repo) // Check if the generated job name starts with the expected prefix if !strings.HasPrefix(jobName, tc.expectedStart) { @@ -108,7 +108,7 @@ func TestDeleteOldMaintenanceJobs(t *testing.T) { cli := fake.NewClientBuilder().WithScheme(scheme).WithObjects(objs...).Build() // Call the function - err := deleteOldMaintenanceJobs(cli, repo, keep) + err := DeleteOldMaintenanceJobs(cli, repo, keep) assert.NoError(t, err) // Get the remaining jobs @@ -167,7 +167,7 @@ func TestWaitForJobComplete(t *testing.T) { // Create a fake Kubernetes client cli := fake.NewClientBuilder().WithObjects(job).Build() // Call the function - err := waitForJobComplete(context.Background(), cli, job) + err := WaitForJobComplete(context.Background(), cli, job) // Check if the error matches the expectation if tc.expectError { @@ -212,7 +212,7 @@ func TestGetMaintenanceResultFromJob(t *testing.T) { cli := fake.NewClientBuilder().WithObjects(job, pod).Build() // Call the function - result, err := getMaintenanceResultFromJob(cli, job) + result, err := GetMaintenanceResultFromJob(cli, job) // Check if the result and error match the expectation assert.NoError(t, err) @@ -256,7 +256,7 @@ func TestGetLatestMaintenanceJob(t *testing.T) { cli := fake.NewClientBuilder().WithScheme(scheme).WithObjects(objs...).Build() // Call the function - job, err := getLatestMaintenanceJob(cli, "default") + job, err := GetLatestMaintenanceJob(cli, "default") assert.NoError(t, err) // We expect the returned job to be the newer job @@ -419,7 +419,7 @@ func TestGetMaintenanceJobConfig(t *testing.T) { fakeClient = velerotest.NewFakeControllerRuntimeClient(t) } - jobConfig, err := getMaintenanceJobConfig( + jobConfig, err := GetMaintenanceJobConfig( ctx, fakeClient, logger, diff --git a/pkg/repository/manager.go b/pkg/repository/manager/manager.go similarity index 90% rename from pkg/repository/manager.go rename to pkg/repository/manager/manager.go index 1135a9b99f..cc4276b730 100644 --- a/pkg/repository/manager.go +++ b/pkg/repository/manager/manager.go @@ -32,6 +32,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/repository" "github.com/vmware-tanzu/velero/pkg/repository/provider" "github.com/vmware-tanzu/velero/pkg/util/filesystem" "github.com/vmware-tanzu/velero/pkg/util/kube" @@ -39,35 +40,6 @@ import ( veleroutil "github.com/vmware-tanzu/velero/pkg/util/velero" ) -// SnapshotIdentifier uniquely identifies a snapshot -// taken by Velero. -type SnapshotIdentifier struct { - // VolumeNamespace is the namespace of the pod/volume that - // the snapshot is for. - VolumeNamespace string `json:"volumeNamespace"` - - // BackupStorageLocation is the backup's storage location - // name. - BackupStorageLocation string `json:"backupStorageLocation"` - - // SnapshotID is the short ID of the snapshot. - SnapshotID string `json:"snapshotID"` - - // RepositoryType is the type of the repository where the - // snapshot is stored - RepositoryType string `json:"repositoryType"` - - // Source is the source of the data saved in the repo by the snapshot - Source string `json:"source"` - - // UploaderType is the type of uploader which saved the snapshot data - UploaderType string `json:"uploaderType"` - - // RepoIdentifier is the identifier of the repository where the - // snapshot is stored - RepoIdentifier string `json:"repoIdentifier"` -} - // Manager manages backup repositories. type Manager interface { // InitRepo initializes a repo with the specified name and identifier. @@ -105,8 +77,8 @@ type manager struct { // client is the Velero controller manager's client. // It's limited to resources in the Velero namespace. client client.Client - repoLocker *RepoLocker - repoEnsurer *Ensurer + repoLocker *repository.RepoLocker + repoEnsurer *repository.Ensurer fileSystem filesystem.Interface repoMaintenanceJobConfig string podResources kube.PodResources @@ -120,8 +92,8 @@ type manager struct { func NewManager( namespace string, client client.Client, - repoLocker *RepoLocker, - repoEnsurer *Ensurer, + repoLocker *repository.RepoLocker, + repoEnsurer *repository.Ensurer, credentialFileStore credentials.FileStore, credentialSecretStore credentials.SecretStore, repoMaintenanceJobConfig string, @@ -216,7 +188,7 @@ func (m *manager) PruneRepo(repo *velerov1api.BackupRepository) error { "repo UID": param.BackupRepo.UID, }) - job, err := getLatestMaintenanceJob(m.client, m.namespace) + job, err := repository.GetLatestMaintenanceJob(m.client, m.namespace) if err != nil { return errors.WithStack(err) } @@ -226,7 +198,7 @@ func (m *manager) PruneRepo(repo *velerov1api.BackupRepository) error { return nil } - jobConfig, err := getMaintenanceJobConfig( + jobConfig, err := repository.GetMaintenanceJobConfig( context.Background(), m.client, m.log, @@ -259,7 +231,7 @@ func (m *manager) PruneRepo(repo *velerov1api.BackupRepository) error { log.Debug("Creating maintenance job") defer func() { - if err := deleteOldMaintenanceJobs( + if err := repository.DeleteOldMaintenanceJobs( m.client, param.BackupRepo.Name, m.keepLatestMaintenanceJobs, @@ -269,12 +241,12 @@ func (m *manager) PruneRepo(repo *velerov1api.BackupRepository) error { }() var jobErr error - if err := waitForJobComplete(context.TODO(), m.client, maintenanceJob); err != nil { + if err := repository.WaitForJobComplete(context.TODO(), m.client, maintenanceJob); err != nil { log.WithError(err).Error("Error to wait for maintenance job complete") jobErr = err // we won't return here for job may failed by maintenance failure, we want return the actual error } - result, err := getMaintenanceResultFromJob(m.client, maintenanceJob) + result, err := repository.GetMaintenanceResultFromJob(m.client, maintenanceJob) if err != nil { return errors.Wrap(err, "error to get maintenance job result") } @@ -383,7 +355,7 @@ func (m *manager) assembleRepoParam(repo *velerov1api.BackupRepository) (provide } func (m *manager) buildMaintenanceJob( - config *JobConfigs, + config *repository.JobConfigs, param provider.RepoParam, ) (*batchv1.Job, error) { // Get the Velero server deployment @@ -435,10 +407,10 @@ func (m *manager) buildMaintenanceJob( // build the maintenance job job := &batchv1.Job{ ObjectMeta: metav1.ObjectMeta{ - Name: generateJobName(param.BackupRepo.Name), + Name: repository.GenerateJobName(param.BackupRepo.Name), Namespace: param.BackupRepo.Namespace, Labels: map[string]string{ - RepositoryNameLabel: param.BackupRepo.Name, + repository.RepositoryNameLabel: param.BackupRepo.Name, }, }, Spec: batchv1.JobSpec{ diff --git a/pkg/repository/manager_test.go b/pkg/repository/manager/manager_test.go similarity index 96% rename from pkg/repository/manager_test.go rename to pkg/repository/manager/manager_test.go index 534a689631..ace4406c39 100644 --- a/pkg/repository/manager_test.go +++ b/pkg/repository/manager/manager_test.go @@ -33,6 +33,7 @@ import ( velerov1 "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" + "github.com/vmware-tanzu/velero/pkg/repository" "github.com/vmware-tanzu/velero/pkg/repository/provider" "github.com/vmware-tanzu/velero/pkg/util/kube" "github.com/vmware-tanzu/velero/pkg/util/logging" @@ -63,7 +64,7 @@ func TestGetRepositoryProvider(t *testing.T) { func TestBuildMaintenanceJob(t *testing.T) { testCases := []struct { name string - m *JobConfigs + m *repository.JobConfigs deploy *appsv1.Deployment logLevel logrus.Level logFormat *logging.FormatFlag @@ -72,7 +73,7 @@ func TestBuildMaintenanceJob(t *testing.T) { }{ { name: "Valid maintenance job", - m: &JobConfigs{ + m: &repository.JobConfigs{ PodResources: &kube.PodResources{ CPURequest: "100m", MemoryRequest: "128Mi", @@ -105,7 +106,7 @@ func TestBuildMaintenanceJob(t *testing.T) { }, { name: "Error getting Velero server deployment", - m: &JobConfigs{ + m: &repository.JobConfigs{ PodResources: &kube.PodResources{ CPURequest: "100m", MemoryRequest: "128Mi", @@ -179,7 +180,7 @@ func TestBuildMaintenanceJob(t *testing.T) { assert.NotNil(t, job) assert.Contains(t, job.Name, tc.expectedJobName) assert.Equal(t, param.BackupRepo.Namespace, job.Namespace) - assert.Equal(t, param.BackupRepo.Name, job.Labels[RepositoryNameLabel]) + assert.Equal(t, param.BackupRepo.Name, job.Labels[repository.RepositoryNameLabel]) // Check container assert.Len(t, job.Spec.Template.Spec.Containers, 1) diff --git a/pkg/repository/types/snapshotidentifier.go b/pkg/repository/types/snapshotidentifier.go new file mode 100644 index 0000000000..578d3100e4 --- /dev/null +++ b/pkg/repository/types/snapshotidentifier.go @@ -0,0 +1,30 @@ +package types + +// SnapshotIdentifier uniquely identifies a snapshot +// taken by Velero. +type SnapshotIdentifier struct { + // VolumeNamespace is the namespace of the pod/volume that + // the snapshot is for. + VolumeNamespace string `json:"volumeNamespace"` + + // BackupStorageLocation is the backup's storage location + // name. + BackupStorageLocation string `json:"backupStorageLocation"` + + // SnapshotID is the short ID of the snapshot. + SnapshotID string `json:"snapshotID"` + + // RepositoryType is the type of the repository where the + // snapshot is stored + RepositoryType string `json:"repositoryType"` + + // Source is the source of the data saved in the repo by the snapshot + Source string `json:"source"` + + // UploaderType is the type of uploader which saved the snapshot data + UploaderType string `json:"uploaderType"` + + // RepoIdentifier is the identifier of the repository where the + // snapshot is stored + RepoIdentifier string `json:"repoIdentifier"` +} diff --git a/pkg/restore/restore.go b/pkg/restore/restore.go index a58d8d8d3c..9ae0b0f663 100644 --- a/pkg/restore/restore.go +++ b/pkg/restore/restore.go @@ -68,6 +68,7 @@ import ( vsv1 "github.com/vmware-tanzu/velero/pkg/plugin/velero/volumesnapshotter/v1" "github.com/vmware-tanzu/velero/pkg/podexec" "github.com/vmware-tanzu/velero/pkg/podvolume" + "github.com/vmware-tanzu/velero/pkg/podvolume/configs" "github.com/vmware-tanzu/velero/pkg/types" "github.com/vmware-tanzu/velero/pkg/util/boolptr" "github.com/vmware-tanzu/velero/pkg/util/collections" @@ -2025,7 +2026,7 @@ func hasPodVolumeBackup(unstructuredPV *unstructured.Unstructured, ctx *restoreC var found bool for _, pvb := range ctx.podVolumeBackups { - if pvb.Spec.Pod.Namespace == pv.Spec.ClaimRef.Namespace && pvb.GetAnnotations()[podvolume.PVCNameAnnotation] == pv.Spec.ClaimRef.Name { + if pvb.Spec.Pod.Namespace == pv.Spec.ClaimRef.Namespace && pvb.GetAnnotations()[configs.PVCNameAnnotation] == pv.Spec.ClaimRef.Name { found = true break }