Skip to content

Commit

Permalink
Reduces ~140 indirect imports for plugin/framework importers (#8208)
Browse files Browse the repository at this point in the history
* Avoid plugin framework importers from needing cloud provider imports

Signed-off-by: Tiger Kaovilai <[email protected]>
  • Loading branch information
kaovilai authored Sep 13, 2024
1 parent da29146 commit 3f9c2dc
Show file tree
Hide file tree
Showing 19 changed files with 230 additions and 110 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
4 changes: 2 additions & 2 deletions pkg/cmd/server/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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,
Expand Down
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(
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 {
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,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 (
Expand All @@ -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,
Expand Down Expand Up @@ -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)
}

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{
VolumeNamespace: du.Spec.SourceNamespace,
BackupStorageLocation: bak.Spec.StorageLocation,
SnapshotID: du.Status.SnapshotID,
Expand Down
53 changes: 53 additions & 0 deletions pkg/install/import_test.go
Original file line number Diff line number Diff line change
@@ -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}} ./<path-to-this-package-dir>
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)
}
Loading

0 comments on commit 3f9c2dc

Please sign in to comment.