plugin/framework refactoring for BackupItemAction v1

Refactors the framework  package to implement the plugin versioning changes
needed for BIA v1 and overall package refactoring to support plugin versions
in different packages. This should be all that's needed to move on to
v2 for BackupItemAction. The remaining plugin types still need similar
refactoring to what's being done here for BIA before attempting a
v2 implementation.

Signed-off-by: Scott Seago <sseago@redhat.com>

internal/delete/delete_item_action_handler.go

@@ -48,7 +48,7 @@ type Context struct {
 func InvokeDeleteActions(ctx *Context) error {
 	var err error
 	resolver := framework.NewDeleteItemActionResolver(ctx.Actions)
-	ctx.resolvedActions, err = resolver.ResolveActions(ctx.DiscoveryHelper)
+	ctx.resolvedActions, err = resolver.ResolveActions(ctx.DiscoveryHelper, ctx.Log)
 	// No actions installed and no error means we don't have to continue;
 	// just do the backup deletion without worrying about plugins.
 	if len(ctx.resolvedActions) == 0 && err == nil {

internal/velero/serverstatusrequest.go

@@ -19,17 +19,18 @@ package velero
 import (
 	velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1"
 	"github.com/vmware-tanzu/velero/pkg/plugin/framework"
+	"github.com/vmware-tanzu/velero/pkg/plugin/framework/common"
 )
 
 type PluginLister interface {
 	// List returns all PluginIdentifiers for kind.
-	List(kind framework.PluginKind) []framework.PluginIdentifier
+	List(kind common.PluginKind) []framework.PluginIdentifier
 }
 
 // GetInstalledPluginInfo returns a list of installed plugins
 func GetInstalledPluginInfo(pluginLister PluginLister) []velerov1api.PluginInfo {
 	var plugins []velerov1api.PluginInfo
-	for _, v := range framework.AllPluginKinds() {
+	for _, v := range common.AllPluginKinds() {
 		list := pluginLister.List(v)
 		for _, plugin := range list {
 			pluginInfo := velerov1api.PluginInfo{

pkg/backup/backup.go

@@ -211,13 +211,13 @@ func (kb *kubernetesBackupper) BackupWithResolvers(log logrus.FieldLogger,
 		return err
 	}
 
-	backupRequest.ResolvedActions, err = backupItemActionResolver.ResolveActions(kb.discoveryHelper)
+	backupRequest.ResolvedActions, err = backupItemActionResolver.ResolveActions(kb.discoveryHelper, log)
 	if err != nil {
 		log.WithError(errors.WithStack(err)).Debugf("Error from backupItemActionResolver.ResolveActions")
 		return err
 	}
 
-	backupRequest.ResolvedItemSnapshotters, err = itemSnapshotterResolver.ResolveActions(kb.discoveryHelper)
+	backupRequest.ResolvedItemSnapshotters, err = itemSnapshotterResolver.ResolveActions(kb.discoveryHelper, log)
 	if err != nil {
 		log.WithError(errors.WithStack(err)).Debugf("Error from itemSnapshotterResolver.ResolveActions")
 		return err

pkg/cmd/server/plugin/plugin.go

@@ -28,6 +28,7 @@ import (
 	"github.com/vmware-tanzu/velero/pkg/client"
 	velerodiscovery "github.com/vmware-tanzu/velero/pkg/discovery"
 	veleroplugin "github.com/vmware-tanzu/velero/pkg/plugin/framework"
+	plugincommon "github.com/vmware-tanzu/velero/pkg/plugin/framework/common"
 	"github.com/vmware-tanzu/velero/pkg/restore"
 )
 
@@ -76,7 +77,7 @@ func newPodBackupItemAction(logger logrus.FieldLogger) (interface{}, error) {
 	return backup.NewPodAction(logger), nil
 }
 
-func newServiceAccountBackupItemAction(f client.Factory) veleroplugin.HandlerInitializer {
+func newServiceAccountBackupItemAction(f client.Factory) plugincommon.HandlerInitializer {
 	return func(logger logrus.FieldLogger) (interface{}, error) {
 		// TODO(ncdc): consider a k8s style WantsKubernetesClientSet initialization approach
 		clientset, err := f.KubeClient()
@@ -101,7 +102,7 @@ func newServiceAccountBackupItemAction(f client.Factory) veleroplugin.HandlerIni
 	}
 }
 
-func newRemapCRDVersionAction(f client.Factory) veleroplugin.HandlerInitializer {
+func newRemapCRDVersionAction(f client.Factory) plugincommon.HandlerInitializer {
 	return func(logger logrus.FieldLogger) (interface{}, error) {
 		config, err := f.ClientConfig()
 		if err != nil {
@@ -138,7 +139,7 @@ func newInitRestoreHookPodAction(logger logrus.FieldLogger) (interface{}, error)
 	return restore.NewInitRestoreHookPodAction(logger), nil
 }
 
-func newResticRestoreItemAction(f client.Factory) veleroplugin.HandlerInitializer {
+func newResticRestoreItemAction(f client.Factory) plugincommon.HandlerInitializer {
 	return func(logger logrus.FieldLogger) (interface{}, error) {
 		client, err := f.KubeClient()
 		if err != nil {
@@ -174,7 +175,7 @@ func newCRDV1PreserveUnknownFieldsItemAction(logger logrus.FieldLogger) (interfa
 	return restore.NewCRDV1PreserveUnknownFieldsAction(logger), nil
 }
 
-func newChangeStorageClassRestoreItemAction(f client.Factory) veleroplugin.HandlerInitializer {
+func newChangeStorageClassRestoreItemAction(f client.Factory) plugincommon.HandlerInitializer {
 	return func(logger logrus.FieldLogger) (interface{}, error) {
 		client, err := f.KubeClient()
 		if err != nil {
@@ -197,7 +198,7 @@ func newClusterRoleBindingItemAction(logger logrus.FieldLogger) (interface{}, er
 	return restore.NewClusterRoleBindingAction(logger), nil
 }
 
-func newChangePVCNodeSelectorItemAction(f client.Factory) veleroplugin.HandlerInitializer {
+func newChangePVCNodeSelectorItemAction(f client.Factory) plugincommon.HandlerInitializer {
 	return func(logger logrus.FieldLogger) (interface{}, error) {
 		client, err := f.KubeClient()
 		if err != nil {

pkg/controller/server_status_request_controller.go

@@ -33,6 +33,7 @@ import (
 	velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1"
 	"github.com/vmware-tanzu/velero/pkg/buildinfo"
 	"github.com/vmware-tanzu/velero/pkg/plugin/framework"
+	"github.com/vmware-tanzu/velero/pkg/plugin/framework/common"
 )
 
 const (
@@ -42,7 +43,7 @@ const (
 
 type PluginLister interface {
 	// List returns all PluginIdentifiers for kind.
-	List(kind framework.PluginKind) []framework.PluginIdentifier
+	List(kind common.PluginKind) []framework.PluginIdentifier
 }
 
 // serverStatusRequestReconciler reconciles a ServerStatusRequest object

pkg/controller/server_status_request_controller_test.go

@@ -36,6 +36,7 @@ import (
 	"github.com/vmware-tanzu/velero/pkg/builder"
 	"github.com/vmware-tanzu/velero/pkg/buildinfo"
 	"github.com/vmware-tanzu/velero/pkg/plugin/framework"
+	"github.com/vmware-tanzu/velero/pkg/plugin/framework/common"
 	velerotest "github.com/vmware-tanzu/velero/pkg/test"
 )
 
@@ -247,7 +248,7 @@ type fakePluginLister struct {
 	plugins []framework.PluginIdentifier
 }
 
-func (l *fakePluginLister) List(kind framework.PluginKind) []framework.PluginIdentifier {
+func (l *fakePluginLister) List(kind common.PluginKind) []framework.PluginIdentifier {
 	var plugins []framework.PluginIdentifier
 	for _, plugin := range l.plugins {
 		if plugin.Kind == kind {

pkg/plugin/clientmgmt/backupitemaction/v1/restartable_backup_item_action.go

@@ -22,14 +22,14 @@ import (
 
 	api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1"
 	"github.com/vmware-tanzu/velero/pkg/plugin/clientmgmt/process"
-	"github.com/vmware-tanzu/velero/pkg/plugin/framework"
+	"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"
 )
 
 // AdaptedBackupItemAction is a backup item action adapted to the v1 BackupItemAction API
 type AdaptedBackupItemAction struct {
-	Kind framework.PluginKind
+	Kind common.PluginKind
 
 	// Get returns a restartable BackupItemAction for the given name and process, wrapping if necessary
 	GetRestartable func(name string, restartableProcess process.RestartableProcess) biav1.BackupItemAction
@@ -38,7 +38,7 @@ type AdaptedBackupItemAction struct {
 func AdaptedBackupItemActions() []AdaptedBackupItemAction {
 	return []AdaptedBackupItemAction{
 		{
-			Kind: framework.PluginKindBackupItemAction,
+			Kind: common.PluginKindBackupItemAction,
 			GetRestartable: func(name string, restartableProcess process.RestartableProcess) biav1.BackupItemAction {
 				return NewRestartableBackupItemAction(name, restartableProcess)
 			},
@@ -58,7 +58,7 @@ type RestartableBackupItemAction struct {
 // NewRestartableBackupItemAction returns a new RestartableBackupItemAction.
 func NewRestartableBackupItemAction(name string, sharedPluginProcess process.RestartableProcess) *RestartableBackupItemAction {
 	r := &RestartableBackupItemAction{
-		Key:                 process.KindAndName{Kind: framework.PluginKindBackupItemAction, Name: name},
+		Key:                 process.KindAndName{Kind: common.PluginKindBackupItemAction, Name: name},
 		SharedPluginProcess: sharedPluginProcess,
 	}
 	return r

pkg/plugin/clientmgmt/backupitemaction/v1/restartable_backup_item_action_test.go

@@ -26,10 +26,10 @@ import (
 	"k8s.io/apimachinery/pkg/runtime/schema"
 
 	v1 "github.com/vmware-tanzu/velero/pkg/apis/velero/v1"
-	"github.com/vmware-tanzu/velero/pkg/backup/mocks"
 	"github.com/vmware-tanzu/velero/pkg/plugin/clientmgmt/process"
-	"github.com/vmware-tanzu/velero/pkg/plugin/framework"
+	"github.com/vmware-tanzu/velero/pkg/plugin/framework/common"
 	"github.com/vmware-tanzu/velero/pkg/plugin/velero"
+	mocks "github.com/vmware-tanzu/velero/pkg/plugin/velero/mocks/backupitemaction/v1"
 )
 
 func TestRestartableGetBackupItemAction(t *testing.T) {
@@ -51,7 +51,7 @@ func TestRestartableGetBackupItemAction(t *testing.T) {
 		},
 		{
 			name:   "happy path",
-			plugin: new(mocks.ItemAction),
+			plugin: new(mocks.BackupItemAction),
 		},
 	}
 
@@ -61,7 +61,7 @@ func TestRestartableGetBackupItemAction(t *testing.T) {
 			defer p.AssertExpectations(t)
 
 			name := "pod"
-			key := process.KindAndName{Kind: framework.PluginKindBackupItemAction, Name: name}
+			key := process.KindAndName{Kind: common.PluginKindBackupItemAction, Name: name}
 			p.On("GetByKindAndName", key).Return(tc.plugin, tc.getError)
 
 			r := NewRestartableBackupItemAction(name, p)
@@ -91,8 +91,8 @@ func TestRestartableBackupItemActionGetDelegate(t *testing.T) {
 
 	// Happy path
 	p.On("ResetIfNeeded").Return(nil)
-	expected := new(mocks.ItemAction)
-	key := process.KindAndName{Kind: framework.PluginKindBackupItemAction, Name: name}
+	expected := new(mocks.BackupItemAction)
+	key := process.KindAndName{Kind: common.PluginKindBackupItemAction, Name: name}
 	p.On("GetByKindAndName", key).Return(expected, nil)
 
 	a, err = r.getDelegate()
@@ -123,15 +123,15 @@ func TestRestartableBackupItemActionDelegatedFunctions(t *testing.T) {
 
 	runRestartableDelegateTests(
 		t,
-		framework.PluginKindBackupItemAction,
+		common.PluginKindBackupItemAction,
 		func(key process.KindAndName, p process.RestartableProcess) interface{} {
 			return &RestartableBackupItemAction{
 				Key:                 key,
 				SharedPluginProcess: p,
 			}
 		},
 		func() mockable {
-			return new(mocks.ItemAction)
+			return new(mocks.BackupItemAction)
 		},
 		restartableDelegateTest{
 			function:                "AppliesTo",

pkg/plugin/clientmgmt/backupitemaction/v1/shared_test.go

@@ -25,7 +25,7 @@ import (
 	"github.com/stretchr/testify/require"
 
 	"github.com/vmware-tanzu/velero/pkg/plugin/clientmgmt/process"
-	"github.com/vmware-tanzu/velero/pkg/plugin/framework"
+	"github.com/vmware-tanzu/velero/pkg/plugin/framework/common"
 )
 
 type mockRestartableProcess struct {
@@ -70,7 +70,7 @@ type mockable interface {
 
 func runRestartableDelegateTests(
 	t *testing.T,
-	kind framework.PluginKind,
+	kind common.PluginKind,
 	newRestartable func(key process.KindAndName, p process.RestartableProcess) interface{},
 	newMock func() mockable,
 	tests ...restartableDelegateTest,

pkg/plugin/clientmgmt/manager.go

@@ -26,7 +26,7 @@ import (
 
 	biav1cli "github.com/vmware-tanzu/velero/pkg/plugin/clientmgmt/backupitemaction/v1"
 	"github.com/vmware-tanzu/velero/pkg/plugin/clientmgmt/process"
-	"github.com/vmware-tanzu/velero/pkg/plugin/framework"
+	"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"
@@ -68,6 +68,9 @@ type Manager interface {
 	CleanupClients()
 }
 
+// Used checking for adapted plugin versions
+var pluginNotFoundErrType = &process.PluginNotFoundError{}
+
 // manager implements Manager.
 type manager struct {
 	logger   logrus.FieldLogger
@@ -106,7 +109,7 @@ func (m *manager) CleanupClients() {
 
 // getRestartableProcess returns a restartableProcess for a plugin identified by kind and name, creating a
 // restartableProcess if it is the first time it has been requested.
-func (m *manager) getRestartableProcess(kind framework.PluginKind, name string) (process.RestartableProcess, error) {
+func (m *manager) getRestartableProcess(kind common.PluginKind, name string) (process.RestartableProcess, error) {
 	m.lock.Lock()
 	defer m.lock.Unlock()
 
@@ -145,7 +148,7 @@ func (m *manager) getRestartableProcess(kind framework.PluginKind, name string)
 func (m *manager) GetObjectStore(name string) (velero.ObjectStore, error) {
 	name = sanitizeName(name)
 
-	restartableProcess, err := m.getRestartableProcess(framework.PluginKindObjectStore, name)
+	restartableProcess, err := m.getRestartableProcess(common.PluginKindObjectStore, name)
 	if err != nil {
 		return nil, err
 	}
@@ -159,7 +162,7 @@ func (m *manager) GetObjectStore(name string) (velero.ObjectStore, error) {
 func (m *manager) GetVolumeSnapshotter(name string) (velero.VolumeSnapshotter, error) {
 	name = sanitizeName(name)
 
-	restartableProcess, err := m.getRestartableProcess(framework.PluginKindVolumeSnapshotter, name)
+	restartableProcess, err := m.getRestartableProcess(common.PluginKindVolumeSnapshotter, name)
 	if err != nil {
 		return nil, err
 	}
@@ -171,7 +174,7 @@ func (m *manager) GetVolumeSnapshotter(name string) (velero.VolumeSnapshotter, e
 
 // GetBackupItemActions returns all backup item actions as restartableBackupItemActions.
 func (m *manager) GetBackupItemActions() ([]biav1.BackupItemAction, error) {
-	list := m.registry.List(framework.PluginKindBackupItemAction)
+	list := m.registry.List(common.PluginKindBackupItemAction)
 
 	actions := make([]biav1.BackupItemAction, 0, len(list))
 
@@ -196,7 +199,7 @@ func (m *manager) GetBackupItemAction(name string) (biav1.BackupItemAction, erro
 	for _, adaptedBackupItemAction := range biav1cli.AdaptedBackupItemActions() {
 		restartableProcess, err := m.getRestartableProcess(adaptedBackupItemAction.Kind, name)
 		// Check if plugin was not found
-		if errors.Is(err, &process.PluginNotFoundError{}) {
+		if errors.As(err, &pluginNotFoundErrType) {
 			continue
 		}
 		if err != nil {
@@ -209,7 +212,7 @@ func (m *manager) GetBackupItemAction(name string) (biav1.BackupItemAction, erro
 
 // GetRestoreItemActions returns all restore item actions as restartableRestoreItemActions.
 func (m *manager) GetRestoreItemActions() ([]velero.RestoreItemAction, error) {
-	list := m.registry.List(framework.PluginKindRestoreItemAction)
+	list := m.registry.List(common.PluginKindRestoreItemAction)
 
 	actions := make([]velero.RestoreItemAction, 0, len(list))
 
@@ -231,7 +234,7 @@ func (m *manager) GetRestoreItemActions() ([]velero.RestoreItemAction, error) {
 func (m *manager) GetRestoreItemAction(name string) (velero.RestoreItemAction, error) {
 	name = sanitizeName(name)
 
-	restartableProcess, err := m.getRestartableProcess(framework.PluginKindRestoreItemAction, name)
+	restartableProcess, err := m.getRestartableProcess(common.PluginKindRestoreItemAction, name)
 	if err != nil {
 		return nil, err
 	}
@@ -242,7 +245,7 @@ func (m *manager) GetRestoreItemAction(name string) (velero.RestoreItemAction, e
 
 // GetDeleteItemActions returns all delete item actions as restartableDeleteItemActions.
 func (m *manager) GetDeleteItemActions() ([]velero.DeleteItemAction, error) {
-	list := m.registry.List(framework.PluginKindDeleteItemAction)
+	list := m.registry.List(common.PluginKindDeleteItemAction)
 
 	actions := make([]velero.DeleteItemAction, 0, len(list))
 
@@ -264,7 +267,7 @@ func (m *manager) GetDeleteItemActions() ([]velero.DeleteItemAction, error) {
 func (m *manager) GetDeleteItemAction(name string) (velero.DeleteItemAction, error) {
 	name = sanitizeName(name)
 
-	restartableProcess, err := m.getRestartableProcess(framework.PluginKindDeleteItemAction, name)
+	restartableProcess, err := m.getRestartableProcess(common.PluginKindDeleteItemAction, name)
 	if err != nil {
 		return nil, err
 	}
@@ -276,7 +279,7 @@ func (m *manager) GetDeleteItemAction(name string) (velero.DeleteItemAction, err
 func (m *manager) GetItemSnapshotter(name string) (isv1.ItemSnapshotter, error) {
 	name = sanitizeName(name)
 
-	restartableProcess, err := m.getRestartableProcess(framework.PluginKindItemSnapshotter, name)
+	restartableProcess, err := m.getRestartableProcess(common.PluginKindItemSnapshotter, name)
 	if err != nil {
 		return nil, err
 	}
@@ -286,7 +289,7 @@ func (m *manager) GetItemSnapshotter(name string) (isv1.ItemSnapshotter, error)
 }
 
 func (m *manager) GetItemSnapshotters() ([]isv1.ItemSnapshotter, error) {
-	list := m.registry.List(framework.PluginKindItemSnapshotter)
+	list := m.registry.List(common.PluginKindItemSnapshotter)
 
 	actions := make([]isv1.ItemSnapshotter, 0, len(list))