Merge pull request #5318 from sseago/volumesnapshotter-refactor

plugin versioning v1 refactor for VolumeSnapshotter
vmware-tanzu · Sep 23, 2022 · b7f5cbd · b7f5cbd
2 parents c81f0db + b608835
commit b7f5cbd
Show file tree

Hide file tree

Showing 16 changed files with 142 additions and 110 deletions.
diff --git a/changelogs/unreleased/5318-sseago b/changelogs/unreleased/5318-sseago
@@ -0,0 +1 @@
+plugin versioning v1 refactor for VolumeSnapshotter
diff --git a/pkg/backup/backup.go b/pkg/backup/backup.go
@@ -44,8 +44,8 @@ import (
 	velerov1client "github.com/vmware-tanzu/velero/pkg/generated/clientset/versioned/typed/velero/v1"
 	"github.com/vmware-tanzu/velero/pkg/kuberesource"
 	"github.com/vmware-tanzu/velero/pkg/plugin/framework"
-	"github.com/vmware-tanzu/velero/pkg/plugin/velero"
 	biav1 "github.com/vmware-tanzu/velero/pkg/plugin/velero/backupitemaction/v1"
+	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/util/boolptr"
@@ -165,7 +165,7 @@ func getResourceHook(hookSpec velerov1api.BackupResourceHookSpec, discoveryHelpe
 }
 
 type VolumeSnapshotterGetter interface {
-	GetVolumeSnapshotter(name string) (velero.VolumeSnapshotter, error)
+	GetVolumeSnapshotter(name string) (vsv1.VolumeSnapshotter, error)
 }
 
 // Backup backs up the items specified in the Backup, placing them in a gzip-compressed tar file

diff --git a/pkg/backup/backup_test.go b/pkg/backup/backup_test.go
@@ -48,6 +48,7 @@ import (
 	"github.com/vmware-tanzu/velero/pkg/kuberesource"
 	"github.com/vmware-tanzu/velero/pkg/plugin/velero"
 	biav1 "github.com/vmware-tanzu/velero/pkg/plugin/velero/backupitemaction/v1"
+	vsv1 "github.com/vmware-tanzu/velero/pkg/plugin/velero/volumesnapshotter/v1"
 	"github.com/vmware-tanzu/velero/pkg/podvolume"
 	"github.com/vmware-tanzu/velero/pkg/test"
 	testutil "github.com/vmware-tanzu/velero/pkg/test"
@@ -1831,10 +1832,10 @@ func TestBackupActionAdditionalItems(t *testing.T) {
 }
 
 // volumeSnapshotterGetter is a simple implementation of the VolumeSnapshotterGetter
-// interface that returns velero.VolumeSnapshotters from a map if they exist.
-type volumeSnapshotterGetter map[string]velero.VolumeSnapshotter
+// interface that returns vsv1.VolumeSnapshotters from a map if they exist.
+type volumeSnapshotterGetter map[string]vsv1.VolumeSnapshotter
 
-func (vsg volumeSnapshotterGetter) GetVolumeSnapshotter(name string) (velero.VolumeSnapshotter, error) {
+func (vsg volumeSnapshotterGetter) GetVolumeSnapshotter(name string) (vsv1.VolumeSnapshotter, error) {
 	snapshotter, ok := vsg[name]
 	if !ok {
 		return nil, errors.New("volume snapshotter not found")
@@ -1859,7 +1860,7 @@ type volumeInfo struct {
 	snapshotErr bool
 }
 
-// fakeVolumeSnapshotter is a test fake for the velero.VolumeSnapshotter interface.
+// fakeVolumeSnapshotter is a test fake for the vsv1.VolumeSnapshotter interface.
 type fakeVolumeSnapshotter struct {
 	// PVVolumeNames is a map from PV name to volume ID, used as the basis
 	// for the GetVolumeID method.
@@ -1982,7 +1983,7 @@ func TestBackupWithSnapshots(t *testing.T) {
 					builder.ForPersistentVolume("pv-1").Result(),
 				),
 			},
-			snapshotterGetter: map[string]velero.VolumeSnapshotter{
+			snapshotterGetter: map[string]vsv1.VolumeSnapshotter{
 				"default": new(fakeVolumeSnapshotter).WithVolume("pv-1", "vol-1", "", "type-1", 100, false),
 			},
 			want: []*volume.Snapshot{
@@ -2015,7 +2016,7 @@ func TestBackupWithSnapshots(t *testing.T) {
 					builder.ForPersistentVolume("pv-1").ObjectMeta(builder.WithLabels("failure-domain.beta.kubernetes.io/zone", "zone-1")).Result(),
 				),
 			},
-			snapshotterGetter: map[string]velero.VolumeSnapshotter{
+			snapshotterGetter: map[string]vsv1.VolumeSnapshotter{
 				"default": new(fakeVolumeSnapshotter).WithVolume("pv-1", "vol-1", "zone-1", "type-1", 100, false),
 			},
 			want: []*volume.Snapshot{
@@ -2049,7 +2050,7 @@ func TestBackupWithSnapshots(t *testing.T) {
 					builder.ForPersistentVolume("pv-1").ObjectMeta(builder.WithLabels("topology.kubernetes.io/zone", "zone-1")).Result(),
 				),
 			},
-			snapshotterGetter: map[string]velero.VolumeSnapshotter{
+			snapshotterGetter: map[string]vsv1.VolumeSnapshotter{
 				"default": new(fakeVolumeSnapshotter).WithVolume("pv-1", "vol-1", "zone-1", "type-1", 100, false),
 			},
 			want: []*volume.Snapshot{
@@ -2083,7 +2084,7 @@ func TestBackupWithSnapshots(t *testing.T) {
 					builder.ForPersistentVolume("pv-1").ObjectMeta(builder.WithLabelsMap(map[string]string{"failure-domain.beta.kubernetes.io/zone": "zone-1-deprecated", "topology.kubernetes.io/zone": "zone-1-ga"})).Result(),
 				),
 			},
-			snapshotterGetter: map[string]velero.VolumeSnapshotter{
+			snapshotterGetter: map[string]vsv1.VolumeSnapshotter{
 				"default": new(fakeVolumeSnapshotter).WithVolume("pv-1", "vol-1", "zone-1-ga", "type-1", 100, false),
 			},
 			want: []*volume.Snapshot{
@@ -2117,7 +2118,7 @@ func TestBackupWithSnapshots(t *testing.T) {
 					builder.ForPersistentVolume("pv-1").Result(),
 				),
 			},
-			snapshotterGetter: map[string]velero.VolumeSnapshotter{
+			snapshotterGetter: map[string]vsv1.VolumeSnapshotter{
 				"default": new(fakeVolumeSnapshotter).WithVolume("pv-1", "vol-1", "", "type-1", 100, true),
 			},
 			want: []*volume.Snapshot{
@@ -2149,7 +2150,7 @@ func TestBackupWithSnapshots(t *testing.T) {
 					builder.ForPersistentVolume("pv-1").Result(),
 				),
 			},
-			snapshotterGetter: map[string]velero.VolumeSnapshotter{
+			snapshotterGetter: map[string]vsv1.VolumeSnapshotter{
 				"default": new(fakeVolumeSnapshotter).WithVolume("pv-1", "vol-1", "", "type-1", 100, false),
 			},
 			want: nil,
@@ -2164,7 +2165,7 @@ func TestBackupWithSnapshots(t *testing.T) {
 					builder.ForPersistentVolume("pv-1").Result(),
 				),
 			},
-			snapshotterGetter: map[string]velero.VolumeSnapshotter{
+			snapshotterGetter: map[string]vsv1.VolumeSnapshotter{
 				"default": new(fakeVolumeSnapshotter).WithVolume("pv-1", "vol-1", "", "type-1", 100, false),
 			},
 			want: nil,
@@ -2182,7 +2183,7 @@ func TestBackupWithSnapshots(t *testing.T) {
 					builder.ForPersistentVolume("pv-1").Result(),
 				),
 			},
-			snapshotterGetter: map[string]velero.VolumeSnapshotter{},
+			snapshotterGetter: map[string]vsv1.VolumeSnapshotter{},
 			want:              nil,
 		},
 		{
@@ -2198,7 +2199,7 @@ func TestBackupWithSnapshots(t *testing.T) {
 					builder.ForPersistentVolume("pv-1").Result(),
 				),
 			},
-			snapshotterGetter: map[string]velero.VolumeSnapshotter{
+			snapshotterGetter: map[string]vsv1.VolumeSnapshotter{
 				"default": new(fakeVolumeSnapshotter),
 			},
 			want: nil,
@@ -2218,7 +2219,7 @@ func TestBackupWithSnapshots(t *testing.T) {
 					builder.ForPersistentVolume("pv-2").Result(),
 				),
 			},
-			snapshotterGetter: map[string]velero.VolumeSnapshotter{
+			snapshotterGetter: map[string]vsv1.VolumeSnapshotter{
 				"default": new(fakeVolumeSnapshotter).WithVolume("pv-1", "vol-1", "", "type-1", 100, false),
 				"another": new(fakeVolumeSnapshotter).WithVolume("pv-2", "vol-2", "", "type-2", 100, false),
 			},
@@ -2686,7 +2687,7 @@ func TestBackupWithRestic(t *testing.T) {
 				),
 			},
 			vsl: newSnapshotLocation("velero", "default", "default"),
-			snapshotterGetter: map[string]velero.VolumeSnapshotter{
+			snapshotterGetter: map[string]vsv1.VolumeSnapshotter{
 				"default": new(fakeVolumeSnapshotter).
 					WithVolume("pv-1", "vol-1", "", "type-1", 100, false).
 					WithVolume("pv-2", "vol-2", "", "type-1", 100, false),

diff --git a/pkg/backup/item_backupper.go b/pkg/backup/item_backupper.go
@@ -41,7 +41,7 @@ import (
 	"github.com/vmware-tanzu/velero/pkg/discovery"
 	"github.com/vmware-tanzu/velero/pkg/features"
 	"github.com/vmware-tanzu/velero/pkg/kuberesource"
-	"github.com/vmware-tanzu/velero/pkg/plugin/velero"
+	vsv1 "github.com/vmware-tanzu/velero/pkg/plugin/velero/volumesnapshotter/v1"
 	"github.com/vmware-tanzu/velero/pkg/podvolume"
 	"github.com/vmware-tanzu/velero/pkg/util/boolptr"
 	"github.com/vmware-tanzu/velero/pkg/volume"
@@ -58,7 +58,7 @@ type itemBackupper struct {
 	volumeSnapshotterGetter VolumeSnapshotterGetter
 
 	itemHookHandler                    hook.ItemHookHandler
-	snapshotLocationVolumeSnapshotters map[string]velero.VolumeSnapshotter
+	snapshotLocationVolumeSnapshotters map[string]vsv1.VolumeSnapshotter
 }
 
 const (
@@ -357,7 +357,7 @@ func (ib *itemBackupper) executeActions(
 
 // volumeSnapshotter instantiates and initializes a VolumeSnapshotter given a VolumeSnapshotLocation,
 // or returns an existing one if one's already been initialized for the location.
-func (ib *itemBackupper) volumeSnapshotter(snapshotLocation *velerov1api.VolumeSnapshotLocation) (velero.VolumeSnapshotter, error) {
+func (ib *itemBackupper) volumeSnapshotter(snapshotLocation *velerov1api.VolumeSnapshotLocation) (vsv1.VolumeSnapshotter, error) {
 	if bs, ok := ib.snapshotLocationVolumeSnapshotters[snapshotLocation.Name]; ok {
 		return bs, nil
 	}
@@ -372,7 +372,7 @@ func (ib *itemBackupper) volumeSnapshotter(snapshotLocation *velerov1api.VolumeS
 	}
 
 	if ib.snapshotLocationVolumeSnapshotters == nil {
-		ib.snapshotLocationVolumeSnapshotters = make(map[string]velero.VolumeSnapshotter)
+		ib.snapshotLocationVolumeSnapshotters = make(map[string]vsv1.VolumeSnapshotter)
 	}
 	ib.snapshotLocationVolumeSnapshotters[snapshotLocation.Name] = bs
 
@@ -447,7 +447,7 @@ func (ib *itemBackupper) takePVSnapshot(obj runtime.Unstructured, log logrus.Fie
 
 	var (
 		volumeID, location string
-		volumeSnapshotter  velero.VolumeSnapshotter
+		volumeSnapshotter  vsv1.VolumeSnapshotter
 	)
 
 	for _, snapshotLocation := range ib.backupRequest.SnapshotLocations {

diff --git a/pkg/controller/backup_deletion_controller.go b/pkg/controller/backup_deletion_controller.go
@@ -39,7 +39,7 @@ import (
 	"github.com/vmware-tanzu/velero/pkg/metrics"
 	"github.com/vmware-tanzu/velero/pkg/persistence"
 	"github.com/vmware-tanzu/velero/pkg/plugin/clientmgmt"
-	"github.com/vmware-tanzu/velero/pkg/plugin/velero"
+	vsv1 "github.com/vmware-tanzu/velero/pkg/plugin/velero/volumesnapshotter/v1"
 	"github.com/vmware-tanzu/velero/pkg/repository"
 	"github.com/vmware-tanzu/velero/pkg/util/filesystem"
 	"github.com/vmware-tanzu/velero/pkg/util/kube"
@@ -282,7 +282,7 @@ func (r *backupDeletionReconciler) Reconcile(ctx context.Context, req ctrl.Reque
 		if snapshots, err := backupStore.GetBackupVolumeSnapshots(backup.Name); err != nil {
 			errs = append(errs, errors.Wrap(err, "error getting backup's volume snapshots").Error())
 		} else {
-			volumeSnapshotters := make(map[string]velero.VolumeSnapshotter)
+			volumeSnapshotters := make(map[string]vsv1.VolumeSnapshotter)
 
 			for _, snapshot := range snapshots {
 				log.WithField("providerSnapshotID", snapshot.Status.ProviderSnapshotID).Info("Removing snapshot associated with backup")
@@ -392,7 +392,7 @@ func volumeSnapshottersForVSL(
 	namespace, vslName string,
 	client client.Client,
 	pluginManager clientmgmt.Manager,
-) (velero.VolumeSnapshotter, error) {
+) (vsv1.VolumeSnapshotter, error) {
 	vsl := &velerov1api.VolumeSnapshotLocation{}
 	if err := client.Get(ctx, types.NamespacedName{
 		Namespace: namespace,

diff --git a/pkg/plugin/clientmgmt/manager.go b/pkg/plugin/clientmgmt/manager.go
@@ -27,11 +27,13 @@ import (
 	biav1cli "github.com/vmware-tanzu/velero/pkg/plugin/clientmgmt/backupitemaction/v1"
 	"github.com/vmware-tanzu/velero/pkg/plugin/clientmgmt/process"
 	riav1cli "github.com/vmware-tanzu/velero/pkg/plugin/clientmgmt/restoreitemaction/v1"
+	vsv1cli "github.com/vmware-tanzu/velero/pkg/plugin/clientmgmt/volumesnapshotter/v1"
 	"github.com/vmware-tanzu/velero/pkg/plugin/framework/common"
 	"github.com/vmware-tanzu/velero/pkg/plugin/velero"
 	biav1 "github.com/vmware-tanzu/velero/pkg/plugin/velero/backupitemaction/v1"
 	isv1 "github.com/vmware-tanzu/velero/pkg/plugin/velero/item_snapshotter/v1"
 	riav1 "github.com/vmware-tanzu/velero/pkg/plugin/velero/restoreitemaction/v1"
+	vsv1 "github.com/vmware-tanzu/velero/pkg/plugin/velero/volumesnapshotter/v1"
 )
 
 // Manager manages the lifecycles of plugins.
@@ -40,7 +42,7 @@ type Manager interface {
 	GetObjectStore(name string) (velero.ObjectStore, error)
 
 	// GetVolumeSnapshotter returns the VolumeSnapshotter plugin for name.
-	GetVolumeSnapshotter(name string) (velero.VolumeSnapshotter, error)
+	GetVolumeSnapshotter(name string) (vsv1.VolumeSnapshotter, error)
 
 	// GetBackupItemActions returns all v1 backup item action plugins.
 	GetBackupItemActions() ([]biav1.BackupItemAction, error)
@@ -161,17 +163,21 @@ func (m *manager) GetObjectStore(name string) (velero.ObjectStore, error) {
 }
 
 // GetVolumeSnapshotter returns a restartableVolumeSnapshotter for name.
-func (m *manager) GetVolumeSnapshotter(name string) (velero.VolumeSnapshotter, error) {
+func (m *manager) GetVolumeSnapshotter(name string) (vsv1.VolumeSnapshotter, error) {
 	name = sanitizeName(name)
 
-	restartableProcess, err := m.getRestartableProcess(common.PluginKindVolumeSnapshotter, name)
-	if err != nil {
-		return nil, err
+	for _, adaptedVolumeSnapshotter := range vsv1cli.AdaptedVolumeSnapshotters() {
+		restartableProcess, err := m.getRestartableProcess(adaptedVolumeSnapshotter.Kind, name)
+		// Check if plugin was not found
+		if errors.As(err, &pluginNotFoundErrType) {
+			continue
+		}
+		if err != nil {
+			return nil, err
+		}
+		return adaptedVolumeSnapshotter.GetRestartable(name, restartableProcess), nil
 	}
-
-	r := NewRestartableVolumeSnapshotter(name, restartableProcess)
-
-	return r, nil
+	return nil, fmt.Errorf("unable to get valid VolumeSnapshotter for %q", name)
 }
 
 // GetBackupItemActions returns all backup item actions as restartableBackupItemActions.

diff --git a/pkg/plugin/clientmgmt/manager_test.go b/pkg/plugin/clientmgmt/manager_test.go
@@ -30,6 +30,7 @@ import (
 	biav1cli "github.com/vmware-tanzu/velero/pkg/plugin/clientmgmt/backupitemaction/v1"
 	"github.com/vmware-tanzu/velero/pkg/plugin/clientmgmt/process"
 	riav1cli "github.com/vmware-tanzu/velero/pkg/plugin/clientmgmt/restoreitemaction/v1"
+	vsv1cli "github.com/vmware-tanzu/velero/pkg/plugin/clientmgmt/volumesnapshotter/v1"
 	"github.com/vmware-tanzu/velero/pkg/plugin/framework"
 	"github.com/vmware-tanzu/velero/pkg/plugin/framework/common"
 	"github.com/vmware-tanzu/velero/pkg/test"
@@ -176,9 +177,9 @@ func TestGetVolumeSnapshotter(t *testing.T) {
 			return m.GetVolumeSnapshotter(name)
 		},
 		func(name string, sharedPluginProcess process.RestartableProcess) interface{} {
-			return &restartableVolumeSnapshotter{
-				key:                 process.KindAndName{Kind: common.PluginKindVolumeSnapshotter, Name: name},
-				sharedPluginProcess: sharedPluginProcess,
+			return &vsv1cli.RestartableVolumeSnapshotter{
+				Key:                 process.KindAndName{Kind: common.PluginKindVolumeSnapshotter, Name: name},
+				SharedPluginProcess: sharedPluginProcess,
 			}
 		},
 		true,
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		plugin versioning v1 refactor for VolumeSnapshotter