Skip to content

Commit

Permalink
Avoid plugin framework importers from needing cloud provider imports
Browse files Browse the repository at this point in the history
Signed-off-by: Tiger Kaovilai <[email protected]>
  • Loading branch information
kaovilai committed Sep 11, 2024
1 parent 7c9b7c1 commit 1457428
Show file tree
Hide file tree
Showing 15 changed files with 217 additions and 98 deletions.
1 change: 1 addition & 0 deletions changelogs/unreleased/8208-kaovilai
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Reduces indirect imports for plugin/framework importers
5 changes: 3 additions & 2 deletions pkg/cmd/server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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(

Check warning on line 473 in pkg/cmd/server/server.go

View check run for this annotation

Codecov / codecov/patch

pkg/cmd/server/server.go#L473

Added line #L473 was not covered by tests
s.namespace,
s.mgr.GetClient(),
s.repoLocker,
Expand Down
18 changes: 10 additions & 8 deletions pkg/controller/backup_deletion_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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
Expand All @@ -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,
Expand Down Expand Up @@ -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 {
Expand All @@ -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
Expand All @@ -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)
Expand Down Expand Up @@ -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{
Expand All @@ -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 {

Check warning on line 640 in pkg/controller/backup_deletion_controller.go

View check run for this annotation

Codecov / codecov/patch

pkg/controller/backup_deletion_controller.go#L640

Added line #L640 was not covered by tests
var errs []error
for volumeNamespace, snapshots := range directSnapshots {
batchForget := []string{}
Expand Down
30 changes: 16 additions & 14 deletions pkg/controller/backup_deletion_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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",
Expand All @@ -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",
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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
}
Expand All @@ -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
}{
{
Expand All @@ -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",
Expand All @@ -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",
Expand All @@ -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",
Expand Down
12 changes: 6 additions & 6 deletions pkg/controller/backup_repository_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,13 +35,13 @@ 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/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 (
Expand All @@ -56,11 +56,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,
Expand Down Expand Up @@ -293,7 +293,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)
}

Expand Down
4 changes: 2 additions & 2 deletions pkg/controller/backup_repository_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions pkg/datamover/dataupload_delete_action.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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{

Check warning on line 55 in pkg/datamover/dataupload_delete_action.go

View check run for this annotation

Codecov / codecov/patch

pkg/datamover/dataupload_delete_action.go#L55

Added line #L55 was not covered by tests
VolumeNamespace: du.Spec.SourceNamespace,
BackupStorageLocation: bak.Spec.StorageLocation,
SnapshotID: du.Status.SnapshotID,
Expand Down
55 changes: 55 additions & 0 deletions pkg/install/import_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
package install

import (
"os/exec"
"path/filepath"
"regexp"
"runtime"
"strings"
"testing"

"github.com/stretchr/testify/require"
)

// test that this framework 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 "pkg/install"
// go list -f {{.Deps}} ./pkg/install/
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|gcp|azure")
require.NoError(t, err)
// depsArr :=[]string{}
cloudProviderDeps := []string{}
for _, dep := range strings.Split(deps, "\n") {
if !k8sio.MatchString(dep) {
// depsArr = append(depsArr, dep)
if cloudProvider.MatchString(dep) {
cloudProviderDeps = append(cloudProviderDeps, dep)
}
}
}
require.Empty(t, cloudProviderDeps)
}
Loading

0 comments on commit 1457428

Please sign in to comment.