Merge pull request #5312 from sseago/riav1-refactor

RestoreItemAction v1 refactoring for plugin api versioning

changelogs/unreleased/5312-sseago

@@ -0,0 +1 @@
+RestoreItemAction v1 refactoring for plugin api versioning

pkg/plugin/clientmgmt/backupitemaction/v1/shared_test.go → internal/restartabletest/restartable_delegate.go

@@ -13,7 +13,7 @@ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 See the License for the specific language governing permissions and
 limitations under the License.
 */
-package v1
+package restartabletest
 
 import (
 	"reflect"
@@ -28,56 +28,56 @@ import (
 	"github.com/vmware-tanzu/velero/pkg/plugin/framework/common"
 )
 
-type mockRestartableProcess struct {
+type MockRestartableProcess struct {
 	mock.Mock
 }
 
-func (rp *mockRestartableProcess) AddReinitializer(key process.KindAndName, r process.Reinitializer) {
+func (rp *MockRestartableProcess) AddReinitializer(key process.KindAndName, r process.Reinitializer) {
 	rp.Called(key, r)
 }
 
-func (rp *mockRestartableProcess) Reset() error {
+func (rp *MockRestartableProcess) Reset() error {
 	args := rp.Called()
 	return args.Error(0)
 }
 
-func (rp *mockRestartableProcess) ResetIfNeeded() error {
+func (rp *MockRestartableProcess) ResetIfNeeded() error {
 	args := rp.Called()
 	return args.Error(0)
 }
 
-func (rp *mockRestartableProcess) GetByKindAndName(key process.KindAndName) (interface{}, error) {
+func (rp *MockRestartableProcess) GetByKindAndName(key process.KindAndName) (interface{}, error) {
 	args := rp.Called(key)
 	return args.Get(0), args.Error(1)
 }
 
-func (rp *mockRestartableProcess) Stop() {
+func (rp *MockRestartableProcess) Stop() {
 	rp.Called()
 }
 
-type restartableDelegateTest struct {
-	function                string
-	inputs                  []interface{}
-	expectedErrorOutputs    []interface{}
-	expectedDelegateOutputs []interface{}
+type RestartableDelegateTest struct {
+	Function                string
+	Inputs                  []interface{}
+	ExpectedErrorOutputs    []interface{}
+	ExpectedDelegateOutputs []interface{}
 }
 
-type mockable interface {
+type Mockable interface {
 	Test(t mock.TestingT)
 	On(method string, args ...interface{}) *mock.Call
 	AssertExpectations(t mock.TestingT) bool
 }
 
-func runRestartableDelegateTests(
+func RunRestartableDelegateTests(
 	t *testing.T,
 	kind common.PluginKind,
 	newRestartable func(key process.KindAndName, p process.RestartableProcess) interface{},
-	newMock func() mockable,
-	tests ...restartableDelegateTest,
+	newMock func() Mockable,
+	tests ...RestartableDelegateTest,
 ) {
 	for _, tc := range tests {
-		t.Run(tc.function, func(t *testing.T) {
-			p := new(mockRestartableProcess)
+		t.Run(tc.Function, func(t *testing.T) {
+			p := new(MockRestartableProcess)
 			p.Test(t)
 			defer p.AssertExpectations(t)
 
@@ -88,19 +88,19 @@ func runRestartableDelegateTests(
 			r := newRestartable(key, p)
 
 			// Get the method we're going to call using reflection
-			method := reflect.ValueOf(r).MethodByName(tc.function)
+			method := reflect.ValueOf(r).MethodByName(tc.Function)
 			require.NotEmpty(t, method)
 
 			// Convert the test case inputs ([]interface{}) to []reflect.Value
 			var inputValues []reflect.Value
-			for i := range tc.inputs {
-				inputValues = append(inputValues, reflect.ValueOf(tc.inputs[i]))
+			for i := range tc.Inputs {
+				inputValues = append(inputValues, reflect.ValueOf(tc.Inputs[i]))
 			}
 
 			// Invoke the method being tested
 			actual := method.Call(inputValues)
 
-			// This function asserts that the actual outputs match the expected outputs
+			// This Function asserts that the actual outputs match the expected outputs
 			checkOutputs := func(expected []interface{}, actual []reflect.Value) {
 				require.Equal(t, len(expected), len(actual))
 
@@ -118,7 +118,7 @@ func runRestartableDelegateTests(
 						continue
 					}
 
-					// If function returns nil as struct return type, we cannot just
+					// If Function returns nil as struct return type, we cannot just
 					// compare the interface to nil as its type will not be nil,
 					// only the value will be
 					if expected[i] == nil && reflect.ValueOf(a).Kind() == reflect.Ptr {
@@ -132,7 +132,7 @@ func runRestartableDelegateTests(
 			}
 
 			// Make sure we get what we expected when getDelegate returned an error
-			checkOutputs(tc.expectedErrorOutputs, actual)
+			checkOutputs(tc.ExpectedErrorOutputs, actual)
 
 			// Invoke delegate, make sure all returned values are passed through
 			p.On("ResetIfNeeded").Return(nil)
@@ -144,13 +144,13 @@ func runRestartableDelegateTests(
 			p.On("GetByKindAndName", key).Return(delegate, nil)
 
 			// Set up the mocked method in the delegate
-			delegate.On(tc.function, tc.inputs...).Return(tc.expectedDelegateOutputs...)
+			delegate.On(tc.Function, tc.Inputs...).Return(tc.ExpectedDelegateOutputs...)
 
 			// Invoke the method being tested
 			actual = method.Call(inputValues)
 
 			// Make sure we get what we expected when invoking the delegate
-			checkOutputs(tc.expectedDelegateOutputs, actual)
+			checkOutputs(tc.ExpectedDelegateOutputs, actual)
 		})
 	}
 }

pkg/controller/restore_controller_test.go

@@ -46,8 +46,8 @@ import (
 	"github.com/vmware-tanzu/velero/pkg/plugin/clientmgmt"
 	"github.com/vmware-tanzu/velero/pkg/plugin/framework"
 	pluginmocks "github.com/vmware-tanzu/velero/pkg/plugin/mocks"
-	"github.com/vmware-tanzu/velero/pkg/plugin/velero"
 	isv1 "github.com/vmware-tanzu/velero/pkg/plugin/velero/item_snapshotter/v1"
+	riav1 "github.com/vmware-tanzu/velero/pkg/plugin/velero/restoreitemaction/v1"
 	pkgrestore "github.com/vmware-tanzu/velero/pkg/restore"
 	velerotest "github.com/vmware-tanzu/velero/pkg/test"
 	"github.com/vmware-tanzu/velero/pkg/util/logging"
@@ -861,7 +861,7 @@ type fakeRestorer struct {
 
 func (r *fakeRestorer) Restore(
 	info pkgrestore.Request,
-	actions []velero.RestoreItemAction,
+	actions []riav1.RestoreItemAction,
 	snapshotLocationLister listers.VolumeSnapshotLocationLister,
 	volumeSnapshotterGetter pkgrestore.VolumeSnapshotterGetter,
 ) (pkgrestore.Result, pkgrestore.Result) {

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

@@ -25,6 +25,7 @@ import (
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 
+	"github.com/vmware-tanzu/velero/internal/restartabletest"
 	v1 "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/common"
@@ -57,7 +58,7 @@ func TestRestartableGetBackupItemAction(t *testing.T) {
 
 	for _, tc := range tests {
 		t.Run(tc.name, func(t *testing.T) {
-			p := new(mockRestartableProcess)
+			p := new(restartabletest.MockRestartableProcess)
 			defer p.AssertExpectations(t)
 
 			name := "pod"
@@ -78,7 +79,7 @@ func TestRestartableGetBackupItemAction(t *testing.T) {
 }
 
 func TestRestartableBackupItemActionGetDelegate(t *testing.T) {
-	p := new(mockRestartableProcess)
+	p := new(restartabletest.MockRestartableProcess)
 	defer p.AssertExpectations(t)
 
 	// Reset error
@@ -121,7 +122,7 @@ func TestRestartableBackupItemActionDelegatedFunctions(t *testing.T) {
 		},
 	}
 
-	runRestartableDelegateTests(
+	restartabletest.RunRestartableDelegateTests(
 		t,
 		common.PluginKindBackupItemAction,
 		func(key process.KindAndName, p process.RestartableProcess) interface{} {
@@ -130,20 +131,20 @@ func TestRestartableBackupItemActionDelegatedFunctions(t *testing.T) {
 				SharedPluginProcess: p,
 			}
 		},
-		func() mockable {
+		func() restartabletest.Mockable {
 			return new(mocks.BackupItemAction)
 		},
-		restartableDelegateTest{
-			function:                "AppliesTo",
-			inputs:                  []interface{}{},
-			expectedErrorOutputs:    []interface{}{velero.ResourceSelector{}, errors.Errorf("reset error")},
-			expectedDelegateOutputs: []interface{}{velero.ResourceSelector{IncludedNamespaces: []string{"a"}}, errors.Errorf("delegate error")},
+		restartabletest.RestartableDelegateTest{
+			Function:                "AppliesTo",
+			Inputs:                  []interface{}{},
+			ExpectedErrorOutputs:    []interface{}{velero.ResourceSelector{}, errors.Errorf("reset error")},
+			ExpectedDelegateOutputs: []interface{}{velero.ResourceSelector{IncludedNamespaces: []string{"a"}}, errors.Errorf("delegate error")},
 		},
-		restartableDelegateTest{
-			function:                "Execute",
-			inputs:                  []interface{}{pv, b},
-			expectedErrorOutputs:    []interface{}{nil, ([]velero.ResourceIdentifier)(nil), errors.Errorf("reset error")},
-			expectedDelegateOutputs: []interface{}{pvToReturn, additionalItems, errors.Errorf("delegate error")},
+		restartabletest.RestartableDelegateTest{
+			Function:                "Execute",
+			Inputs:                  []interface{}{pv, b},
+			ExpectedErrorOutputs:    []interface{}{nil, ([]velero.ResourceIdentifier)(nil), errors.Errorf("reset error")},
+			ExpectedDelegateOutputs: []interface{}{pvToReturn, additionalItems, errors.Errorf("delegate error")},
 		},
 	)
 }

pkg/plugin/clientmgmt/manager.go

@@ -26,10 +26,12 @@ 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"
 	"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"
 )
 
 // Manager manages the lifecycles of plugins.
@@ -47,10 +49,10 @@ type Manager interface {
 	GetBackupItemAction(name string) (biav1.BackupItemAction, error)
 
 	// GetRestoreItemActions returns all restore item action plugins.
-	GetRestoreItemActions() ([]velero.RestoreItemAction, error)
+	GetRestoreItemActions() ([]riav1.RestoreItemAction, error)
 
 	// GetRestoreItemAction returns the restore item action plugin for name.
-	GetRestoreItemAction(name string) (velero.RestoreItemAction, error)
+	GetRestoreItemAction(name string) (riav1.RestoreItemAction, error)
 
 	// GetDeleteItemActions returns all delete item action plugins.
 	GetDeleteItemActions() ([]velero.DeleteItemAction, error)
@@ -211,10 +213,10 @@ func (m *manager) GetBackupItemAction(name string) (biav1.BackupItemAction, erro
 }
 
 // GetRestoreItemActions returns all restore item actions as restartableRestoreItemActions.
-func (m *manager) GetRestoreItemActions() ([]velero.RestoreItemAction, error) {
+func (m *manager) GetRestoreItemActions() ([]riav1.RestoreItemAction, error) {
 	list := m.registry.List(common.PluginKindRestoreItemAction)
 
-	actions := make([]velero.RestoreItemAction, 0, len(list))
+	actions := make([]riav1.RestoreItemAction, 0, len(list))
 
 	for i := range list {
 		id := list[i]
@@ -231,16 +233,21 @@ func (m *manager) GetRestoreItemActions() ([]velero.RestoreItemAction, error) {
 }
 
 // GetRestoreItemAction returns a restartableRestoreItemAction for name.
-func (m *manager) GetRestoreItemAction(name string) (velero.RestoreItemAction, error) {
+func (m *manager) GetRestoreItemAction(name string) (riav1.RestoreItemAction, error) {
 	name = sanitizeName(name)
 
-	restartableProcess, err := m.getRestartableProcess(common.PluginKindRestoreItemAction, name)
-	if err != nil {
-		return nil, err
+	for _, adaptedRestoreItemAction := range riav1cli.AdaptedRestoreItemActions() {
+		restartableProcess, err := m.getRestartableProcess(adaptedRestoreItemAction.Kind, name)
+		// Check if plugin was not found
+		if errors.As(err, &pluginNotFoundErrType) {
+			continue
+		}
+		if err != nil {
+			return nil, err
+		}
+		return adaptedRestoreItemAction.GetRestartable(name, restartableProcess), nil
 	}
-
-	r := NewRestartableRestoreItemAction(name, restartableProcess)
-	return r, nil
+	return nil, fmt.Errorf("unable to get valid RestoreItemAction for %q", name)
 }
 
 // GetDeleteItemActions returns all delete item actions as restartableDeleteItemActions.