Skip to content

Commit

Permalink
plugin/clientmgmt refactoring for BackupItemAction v1
Browse files Browse the repository at this point in the history
Refactors the clientmgmt 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>
  • Loading branch information
sseago committed Aug 31, 2022
1 parent 79056b0 commit 4f17506
Show file tree
Hide file tree
Showing 28 changed files with 466 additions and 254 deletions.
1 change: 1 addition & 0 deletions changelogs/unreleased/5271-sseago
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
plugin/clientmgmt refactoring for BackupItemAction v1
5 changes: 3 additions & 2 deletions pkg/cmd/server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,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/clientmgmt/process"
"github.com/vmware-tanzu/velero/pkg/podexec"
"github.com/vmware-tanzu/velero/pkg/restic"
"github.com/vmware-tanzu/velero/pkg/restore"
Expand Down Expand Up @@ -252,7 +253,7 @@ type server struct {
cancelFunc context.CancelFunc
logger logrus.FieldLogger
logLevel logrus.Level
pluginRegistry clientmgmt.Registry
pluginRegistry process.Registry
repoManager repository.Manager
repoLocker *repository.RepoLocker
repoEnsurer *repository.RepositoryEnsurer
Expand Down Expand Up @@ -296,7 +297,7 @@ func newServer(f client.Factory, config serverConfig, logger *logrus.Logger) (*s
return nil, err
}

pluginRegistry := clientmgmt.NewRegistry(config.pluginDir, logger, logger.Level)
pluginRegistry := process.NewRegistry(config.pluginDir, logger, logger.Level)
if err := pluginRegistry.DiscoverPlugins(); err != nil {
return nil, err
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,40 +14,69 @@ See the License for the specific language governing permissions and
limitations under the License.
*/

package clientmgmt
package v1

import (
"github.com/pkg/errors"
"k8s.io/apimachinery/pkg/runtime"

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/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

// Get returns a restartable BackupItemAction for the given name and process, wrapping if necessary
GetRestartable func(name string, restartableProcess process.RestartableProcess) biav1.BackupItemAction
}

//type v1RestartableBackpItemAction struct {
//}

//func (*v1RestartableBackpItemAction) Get(name string, restartableProcess process.RestartableProcess) *biav1.BackupItemAction {
// return newRestartableBackupItemAction(name, restartableProcess)
//}
func AdaptedBackupItemActions() []AdaptedBackupItemAction {
return []AdaptedBackupItemAction{
{
Kind: framework.PluginKindBackupItemAction,
GetRestartable: func(name string, restartableProcess process.RestartableProcess) biav1.BackupItemAction {
return newRestartableBackupItemAction(name, restartableProcess)
},
//Get: func(name string, restartableProcess process.RestartableProcess) biav1.BackupItemAction {
// return newRestartableBackupItemAction(name, restartableProcess)
//},
},
}
}

// restartableBackupItemAction is a backup item action for a given implementation (such as "pod"). It is associated with
// a restartableProcess, which may be shared and used to run multiple plugins. At the beginning of each method
// call, the restartableBackupItemAction asks its restartableProcess to restart itself if needed (e.g. if the
// process terminated for any reason), then it proceeds with the actual call.
type restartableBackupItemAction struct {
key kindAndName
sharedPluginProcess RestartableProcess
type RestartableBackupItemAction struct {
Key process.KindAndName
SharedPluginProcess process.RestartableProcess
}

// newRestartableBackupItemAction returns a new restartableBackupItemAction.
func newRestartableBackupItemAction(name string, sharedPluginProcess RestartableProcess) *restartableBackupItemAction {
r := &restartableBackupItemAction{
key: kindAndName{kind: framework.PluginKindBackupItemAction, name: name},
sharedPluginProcess: sharedPluginProcess,
func newRestartableBackupItemAction(name string, sharedPluginProcess process.RestartableProcess) *RestartableBackupItemAction {
r := &RestartableBackupItemAction{
Key: process.KindAndName{Kind: framework.PluginKindBackupItemAction, Name: name},
SharedPluginProcess: sharedPluginProcess,
}
return r
}

// getBackupItemAction returns the backup item action for this restartableBackupItemAction. It does *not* restart the
// plugin process.
func (r *restartableBackupItemAction) getBackupItemAction() (biav1.BackupItemAction, error) {
plugin, err := r.sharedPluginProcess.getByKindAndName(r.key)
func (r *RestartableBackupItemAction) getBackupItemAction() (biav1.BackupItemAction, error) {
plugin, err := r.SharedPluginProcess.GetByKindAndName(r.Key)
if err != nil {
return nil, err
}
Expand All @@ -61,16 +90,16 @@ func (r *restartableBackupItemAction) getBackupItemAction() (biav1.BackupItemAct
}

// getDelegate restarts the plugin process (if needed) and returns the backup item action for this restartableBackupItemAction.
func (r *restartableBackupItemAction) getDelegate() (biav1.BackupItemAction, error) {
if err := r.sharedPluginProcess.resetIfNeeded(); err != nil {
func (r *RestartableBackupItemAction) getDelegate() (biav1.BackupItemAction, error) {
if err := r.SharedPluginProcess.ResetIfNeeded(); err != nil {
return nil, err
}

return r.getBackupItemAction()
}

// AppliesTo restarts the plugin's process if needed, then delegates the call.
func (r *restartableBackupItemAction) AppliesTo() (velero.ResourceSelector, error) {
func (r *RestartableBackupItemAction) AppliesTo() (velero.ResourceSelector, error) {
delegate, err := r.getDelegate()
if err != nil {
return velero.ResourceSelector{}, err
Expand All @@ -80,7 +109,7 @@ func (r *restartableBackupItemAction) AppliesTo() (velero.ResourceSelector, erro
}

// Execute restarts the plugin's process if needed, then delegates the call.
func (r *restartableBackupItemAction) Execute(item runtime.Unstructured, backup *api.Backup) (runtime.Unstructured, []velero.ResourceIdentifier, error) {
func (r *RestartableBackupItemAction) Execute(item runtime.Unstructured, backup *api.Backup) (runtime.Unstructured, []velero.ResourceIdentifier, error) {
delegate, err := r.getDelegate()
if err != nil {
return nil, nil, err
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
limitations under the License.
*/

package clientmgmt
package v1

import (
"testing"
Expand All @@ -27,6 +27,7 @@ import (

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/velero"
)
Expand Down Expand Up @@ -60,8 +61,8 @@ func TestRestartableGetBackupItemAction(t *testing.T) {
defer p.AssertExpectations(t)

name := "pod"
key := kindAndName{kind: framework.PluginKindBackupItemAction, name: name}
p.On("getByKindAndName", key).Return(tc.plugin, tc.getError)
key := process.KindAndName{Kind: framework.PluginKindBackupItemAction, Name: name}
p.On("GetByKindAndName", key).Return(tc.plugin, tc.getError)

r := newRestartableBackupItemAction(name, p)
a, err := r.getBackupItemAction()
Expand All @@ -81,18 +82,18 @@ func TestRestartableBackupItemActionGetDelegate(t *testing.T) {
defer p.AssertExpectations(t)

// Reset error
p.On("resetIfNeeded").Return(errors.Errorf("reset error")).Once()
p.On("ResetIfNeeded").Return(errors.Errorf("reset error")).Once()
name := "pod"
r := newRestartableBackupItemAction(name, p)
a, err := r.getDelegate()
assert.Nil(t, a)
assert.EqualError(t, err, "reset error")

// Happy path
p.On("resetIfNeeded").Return(nil)
p.On("ResetIfNeeded").Return(nil)
expected := new(mocks.ItemAction)
key := kindAndName{kind: framework.PluginKindBackupItemAction, name: name}
p.On("getByKindAndName", key).Return(expected, nil)
key := process.KindAndName{Kind: framework.PluginKindBackupItemAction, Name: name}
p.On("GetByKindAndName", key).Return(expected, nil)

a, err = r.getDelegate()
assert.NoError(t, err)
Expand Down Expand Up @@ -123,10 +124,10 @@ func TestRestartableBackupItemActionDelegatedFunctions(t *testing.T) {
runRestartableDelegateTests(
t,
framework.PluginKindBackupItemAction,
func(key kindAndName, p RestartableProcess) interface{} {
return &restartableBackupItemAction{
key: key,
sharedPluginProcess: p,
func(key process.KindAndName, p process.RestartableProcess) interface{} {
return &RestartableBackupItemAction{
Key: key,
SharedPluginProcess: p,
}
},
func() mockable {
Expand Down
156 changes: 156 additions & 0 deletions pkg/plugin/clientmgmt/backupitemaction/v1/shared_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,156 @@
/*
Copyright 2018 the Velero contributors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
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

import (
"reflect"
"testing"

"github.com/pkg/errors"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require"

"github.com/vmware-tanzu/velero/pkg/plugin/clientmgmt/process"
"github.com/vmware-tanzu/velero/pkg/plugin/framework"
)

type mockRestartableProcess struct {
mock.Mock
}

func (rp *mockRestartableProcess) AddReinitializer(key process.KindAndName, r process.Reinitializer) {
rp.Called(key, r)
}

func (rp *mockRestartableProcess) Reset() error {
args := rp.Called()
return args.Error(0)
}

func (rp *mockRestartableProcess) ResetIfNeeded() error {
args := rp.Called()
return args.Error(0)
}

func (rp *mockRestartableProcess) GetByKindAndName(key process.KindAndName) (interface{}, error) {
args := rp.Called(key)
return args.Get(0), args.Error(1)
}

func (rp *mockRestartableProcess) Stop() {
rp.Called()
}

type restartableDelegateTest struct {
function string
inputs []interface{}
expectedErrorOutputs []interface{}
expectedDelegateOutputs []interface{}
}

type mockable interface {
Test(t mock.TestingT)
On(method string, args ...interface{}) *mock.Call
AssertExpectations(t mock.TestingT) bool
}

func runRestartableDelegateTests(
t *testing.T,
kind framework.PluginKind,
newRestartable func(key process.KindAndName, p process.RestartableProcess) interface{},
newMock func() mockable,
tests ...restartableDelegateTest,
) {
for _, tc := range tests {
t.Run(tc.function, func(t *testing.T) {
p := new(mockRestartableProcess)
p.Test(t)
defer p.AssertExpectations(t)

// getDelegate error
p.On("ResetIfNeeded").Return(errors.Errorf("reset error")).Once()
name := "delegateName"
key := process.KindAndName{Kind: kind, Name: name}
r := newRestartable(key, p)

// Get the method we're going to call using reflection
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]))
}

// Invoke the method being tested
actual := method.Call(inputValues)

// 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))

for i := range actual {
// Get the underlying value from the reflect.Value
a := actual[i].Interface()

// Check if it's an error
actualErr, actualErrOk := a.(error)
// Check if the expected output element is an error
expectedErr, expectedErrOk := expected[i].(error)
// If both are errors, use EqualError
if actualErrOk && expectedErrOk {
assert.EqualError(t, actualErr, expectedErr.Error())
continue
}

// 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 {
assert.True(t, reflect.ValueOf(a).IsNil())
continue
}

// Otherwise, use plain Equal
assert.Equal(t, expected[i], a)
}
}

// Make sure we get what we expected when getDelegate returned an error
checkOutputs(tc.expectedErrorOutputs, actual)

// Invoke delegate, make sure all returned values are passed through
p.On("ResetIfNeeded").Return(nil)

delegate := newMock()
delegate.Test(t)
defer delegate.AssertExpectations(t)

p.On("GetByKindAndName", key).Return(delegate, nil)

// Set up the mocked method in the delegate
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)
})
}
}
Loading

0 comments on commit 4f17506

Please sign in to comment.