From 4262b475360419eb6679e92f7207ad5b56ea7125 Mon Sep 17 00:00:00 2001 From: Scott Seago Date: Wed, 31 Aug 2022 13:48:53 -0400 Subject: [PATCH] plugin/clientmgmt refactoring for BackupItemAction v1 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 --- changelogs/unreleased/5271-sseago | 1 + pkg/cmd/server/server.go | 5 +- .../v1}/restartable_backup_item_action.go | 52 ++++-- .../restartable_backup_item_action_test.go | 27 +-- .../backupitemaction/v1/shared_test.go | 156 ++++++++++++++++++ pkg/plugin/clientmgmt/manager.go | 51 +++--- pkg/plugin/clientmgmt/manager_test.go | 96 +++++------ .../{ => process}/client_builder.go | 2 +- .../{ => process}/client_builder_test.go | 2 +- .../{ => process}/logrus_adapter.go | 2 +- .../{ => process}/logrus_adapter_test.go | 2 +- .../clientmgmt/{ => process}/process.go | 14 +- .../clientmgmt/{ => process}/process_test.go | 16 +- .../clientmgmt/{ => process}/registry.go | 28 ++-- .../clientmgmt/{ => process}/registry_test.go | 2 +- .../{ => process}/restartable_process.go | 56 +++---- .../clientmgmt/restartable_delegate_test.go | 11 +- .../restartable_delete_item_action.go | 15 +- .../restartable_delete_item_action_test.go | 19 ++- .../restartable_item_snapshotter.go | 18 +- .../restartable_item_snapshotter_test.go | 22 +-- .../clientmgmt/restartable_object_store.go | 19 ++- .../restartable_object_store_test.go | 31 ++-- .../restartable_restore_item_action.go | 17 +- .../restartable_restore_item_action_test.go | 19 ++- .../restartable_volume_snapshotter.go | 21 +-- .../restartable_volume_snapshotter_test.go | 31 ++-- pkg/plugin/mocks/process_factory.go | 20 ++- 28 files changed, 479 insertions(+), 276 deletions(-) create mode 100644 changelogs/unreleased/5271-sseago rename pkg/plugin/clientmgmt/{ => backupitemaction/v1}/restartable_backup_item_action.go (57%) rename pkg/plugin/clientmgmt/{ => backupitemaction/v1}/restartable_backup_item_action_test.go (82%) create mode 100644 pkg/plugin/clientmgmt/backupitemaction/v1/shared_test.go rename pkg/plugin/clientmgmt/{ => process}/client_builder.go (99%) rename pkg/plugin/clientmgmt/{ => process}/client_builder_test.go (99%) rename pkg/plugin/clientmgmt/{ => process}/logrus_adapter.go (99%) rename pkg/plugin/clientmgmt/{ => process}/logrus_adapter_test.go (99%) rename pkg/plugin/clientmgmt/{ => process}/process.go (93%) rename pkg/plugin/clientmgmt/{ => process}/process_test.go (92%) rename pkg/plugin/clientmgmt/{ => process}/registry.go (91%) rename pkg/plugin/clientmgmt/{ => process}/registry_test.go (99%) rename pkg/plugin/clientmgmt/{ => process}/restartable_process.go (75%) diff --git a/changelogs/unreleased/5271-sseago b/changelogs/unreleased/5271-sseago new file mode 100644 index 000000000..339b81b69 --- /dev/null +++ b/changelogs/unreleased/5271-sseago @@ -0,0 +1 @@ + plugin/clientmgmt refactoring for BackupItemAction v1 diff --git a/pkg/cmd/server/server.go b/pkg/cmd/server/server.go index 482e91731..3d09f7213 100644 --- a/pkg/cmd/server/server.go +++ b/pkg/cmd/server/server.go @@ -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" @@ -250,7 +251,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 @@ -295,7 +296,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 } diff --git a/pkg/plugin/clientmgmt/restartable_backup_item_action.go b/pkg/plugin/clientmgmt/backupitemaction/v1/restartable_backup_item_action.go similarity index 57% rename from pkg/plugin/clientmgmt/restartable_backup_item_action.go rename to pkg/plugin/clientmgmt/backupitemaction/v1/restartable_backup_item_action.go index f9ff2faa3..3317be280 100644 --- a/pkg/plugin/clientmgmt/restartable_backup_item_action.go +++ b/pkg/plugin/clientmgmt/backupitemaction/v1/restartable_backup_item_action.go @@ -14,40 +14,60 @@ 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" ) -// restartableBackupItemAction is a backup item action for a given implementation (such as "pod"). It is associated with +// 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 +} + +func AdaptedBackupItemActions() []AdaptedBackupItemAction { + return []AdaptedBackupItemAction{ + { + Kind: framework.PluginKindBackupItemAction, + GetRestartable: 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, +// NewRestartableBackupItemAction returns a new RestartableBackupItemAction. +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 } @@ -61,8 +81,8 @@ 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 } @@ -70,7 +90,7 @@ func (r *restartableBackupItemAction) getDelegate() (biav1.BackupItemAction, err } // 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 @@ -80,7 +100,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 diff --git a/pkg/plugin/clientmgmt/restartable_backup_item_action_test.go b/pkg/plugin/clientmgmt/backupitemaction/v1/restartable_backup_item_action_test.go similarity index 82% rename from pkg/plugin/clientmgmt/restartable_backup_item_action_test.go rename to pkg/plugin/clientmgmt/backupitemaction/v1/restartable_backup_item_action_test.go index 7f528e73a..947a0f150 100644 --- a/pkg/plugin/clientmgmt/restartable_backup_item_action_test.go +++ b/pkg/plugin/clientmgmt/backupitemaction/v1/restartable_backup_item_action_test.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package clientmgmt +package v1 import ( "testing" @@ -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" ) @@ -60,10 +61,10 @@ 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) + r := NewRestartableBackupItemAction(name, p) a, err := r.getBackupItemAction() if tc.expectedError != "" { assert.EqualError(t, err, tc.expectedError) @@ -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) + 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) @@ -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 { diff --git a/pkg/plugin/clientmgmt/backupitemaction/v1/shared_test.go b/pkg/plugin/clientmgmt/backupitemaction/v1/shared_test.go new file mode 100644 index 000000000..a612c764c --- /dev/null +++ b/pkg/plugin/clientmgmt/backupitemaction/v1/shared_test.go @@ -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) + }) + } +} diff --git a/pkg/plugin/clientmgmt/manager.go b/pkg/plugin/clientmgmt/manager.go index 476608138..e2b55951c 100644 --- a/pkg/plugin/clientmgmt/manager.go +++ b/pkg/plugin/clientmgmt/manager.go @@ -17,11 +17,15 @@ limitations under the License. package clientmgmt import ( + "errors" + "fmt" "strings" "sync" "github.com/sirupsen/logrus" + 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/velero" biav1 "github.com/vmware-tanzu/velero/pkg/plugin/velero/backupitemaction/v1" @@ -36,7 +40,7 @@ type Manager interface { // GetVolumeSnapshotter returns the VolumeSnapshotter plugin for name. GetVolumeSnapshotter(name string) (velero.VolumeSnapshotter, error) - // GetBackupItemActions returns all backup item action plugins. + // GetBackupItemActions returns all v1 backup item action plugins. GetBackupItemActions() ([]biav1.BackupItemAction, error) // GetBackupItemAction returns the backup item action plugin for name. @@ -68,25 +72,25 @@ type Manager interface { type manager struct { logger logrus.FieldLogger logLevel logrus.Level - registry Registry + registry process.Registry - restartableProcessFactory RestartableProcessFactory + restartableProcessFactory process.RestartableProcessFactory // lock guards restartableProcesses lock sync.Mutex - restartableProcesses map[string]RestartableProcess + restartableProcesses map[string]process.RestartableProcess } // NewManager constructs a manager for getting plugins. -func NewManager(logger logrus.FieldLogger, level logrus.Level, registry Registry) Manager { +func NewManager(logger logrus.FieldLogger, level logrus.Level, registry process.Registry) Manager { return &manager{ logger: logger, logLevel: level, registry: registry, - restartableProcessFactory: newRestartableProcessFactory(), + restartableProcessFactory: process.NewRestartableProcessFactory(), - restartableProcesses: make(map[string]RestartableProcess), + restartableProcesses: make(map[string]process.RestartableProcess), } } @@ -94,7 +98,7 @@ func (m *manager) CleanupClients() { m.lock.Lock() for _, restartableProcess := range m.restartableProcesses { - restartableProcess.stop() + restartableProcess.Stop() } m.lock.Unlock() @@ -102,7 +106,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) (RestartableProcess, error) { +func (m *manager) getRestartableProcess(kind framework.PluginKind, name string) (process.RestartableProcess, error) { m.lock.Lock() defer m.lock.Unlock() @@ -127,7 +131,7 @@ func (m *manager) getRestartableProcess(kind framework.PluginKind, name string) logger.Debug("creating new restartable plugin process") - restartableProcess, err = m.restartableProcessFactory.newRestartableProcess(info.Command, m.logger, m.logLevel) + restartableProcess, err = m.restartableProcessFactory.NewRestartableProcess(info.Command, m.logger, m.logLevel) if err != nil { return nil, err } @@ -146,7 +150,7 @@ func (m *manager) GetObjectStore(name string) (velero.ObjectStore, error) { return nil, err } - r := newRestartableObjectStore(name, restartableProcess) + r := NewRestartableObjectStore(name, restartableProcess) return r, nil } @@ -160,7 +164,7 @@ func (m *manager) GetVolumeSnapshotter(name string) (velero.VolumeSnapshotter, e return nil, err } - r := newRestartableVolumeSnapshotter(name, restartableProcess) + r := NewRestartableVolumeSnapshotter(name, restartableProcess) return r, nil } @@ -189,13 +193,18 @@ func (m *manager) GetBackupItemActions() ([]biav1.BackupItemAction, error) { func (m *manager) GetBackupItemAction(name string) (biav1.BackupItemAction, error) { name = sanitizeName(name) - restartableProcess, err := m.getRestartableProcess(framework.PluginKindBackupItemAction, name) - if err != nil { - return nil, err + for _, adaptedBackupItemAction := range biav1cli.AdaptedBackupItemActions() { + restartableProcess, err := m.getRestartableProcess(adaptedBackupItemAction.Kind, name) + // Check if plugin was not found + if errors.Is(err, &process.PluginNotFoundError{}) { + continue + } + if err != nil { + return nil, err + } + return adaptedBackupItemAction.GetRestartable(name, restartableProcess), nil } - - r := newRestartableBackupItemAction(name, restartableProcess) - return r, nil + return nil, fmt.Errorf("unable to get valid BackupItemAction for %q", name) } // GetRestoreItemActions returns all restore item actions as restartableRestoreItemActions. @@ -227,7 +236,7 @@ func (m *manager) GetRestoreItemAction(name string) (velero.RestoreItemAction, e return nil, err } - r := newRestartableRestoreItemAction(name, restartableProcess) + r := NewRestartableRestoreItemAction(name, restartableProcess) return r, nil } @@ -260,7 +269,7 @@ func (m *manager) GetDeleteItemAction(name string) (velero.DeleteItemAction, err return nil, err } - r := newRestartableDeleteItemAction(name, restartableProcess) + r := NewRestartableDeleteItemAction(name, restartableProcess) return r, nil } @@ -272,7 +281,7 @@ func (m *manager) GetItemSnapshotter(name string) (isv1.ItemSnapshotter, error) return nil, err } - r := newRestartableItemSnapshotter(name, restartableProcess) + r := NewRestartableItemSnapshotter(name, restartableProcess) return r, nil } diff --git a/pkg/plugin/clientmgmt/manager_test.go b/pkg/plugin/clientmgmt/manager_test.go index cc36bc653..64b8bf4ed 100644 --- a/pkg/plugin/clientmgmt/manager_test.go +++ b/pkg/plugin/clientmgmt/manager_test.go @@ -26,6 +26,8 @@ import ( "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" + 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/test" ) @@ -72,11 +74,11 @@ type mockRestartableProcessFactory struct { mock.Mock } -func (f *mockRestartableProcessFactory) newRestartableProcess(command string, logger logrus.FieldLogger, logLevel logrus.Level) (RestartableProcess, error) { +func (f *mockRestartableProcessFactory) NewRestartableProcess(command string, logger logrus.FieldLogger, logLevel logrus.Level) (process.RestartableProcess, error) { args := f.Called(command, logger, logLevel) - var rp RestartableProcess + var rp process.RestartableProcess if args.Get(0) != nil { - rp = args.Get(0).(RestartableProcess) + rp = args.Get(0).(process.RestartableProcess) } return rp, args.Error(1) } @@ -85,26 +87,26 @@ type mockRestartableProcess struct { mock.Mock } -func (rp *mockRestartableProcess) addReinitializer(key kindAndName, r 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 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() } @@ -135,7 +137,7 @@ func TestGetRestartableProcess(t *testing.T) { Name: pluginName, } registry.On("Get", pluginKind, pluginName).Return(podID, nil) - factory.On("newRestartableProcess", podID.Command, logger, logLevel).Return(nil, errors.Errorf("factory")).Once() + factory.On("NewRestartableProcess", podID.Command, logger, logLevel).Return(nil, errors.Errorf("factory")).Once() rp, err = m.getRestartableProcess(pluginKind, pluginName) assert.Nil(t, rp) assert.EqualError(t, err, "factory") @@ -143,7 +145,7 @@ func TestGetRestartableProcess(t *testing.T) { // Test 3: registry ok, factory ok restartableProcess := &mockRestartableProcess{} defer restartableProcess.AssertExpectations(t) - factory.On("newRestartableProcess", podID.Command, logger, logLevel).Return(restartableProcess, nil).Once() + factory.On("NewRestartableProcess", podID.Command, logger, logLevel).Return(restartableProcess, nil).Once() rp, err = m.getRestartableProcess(pluginKind, pluginName) require.NoError(t, err) assert.Equal(t, restartableProcess, rp) @@ -166,7 +168,7 @@ func TestCleanupClients(t *testing.T) { for i := 0; i < 5; i++ { rp := &mockRestartableProcess{} defer rp.AssertExpectations(t) - rp.On("stop") + rp.On("Stop") m.restartableProcesses[fmt.Sprintf("rp%d", i)] = rp } @@ -180,9 +182,9 @@ func TestGetObjectStore(t *testing.T) { func(m Manager, name string) (interface{}, error) { return m.GetObjectStore(name) }, - func(name string, sharedPluginProcess RestartableProcess) interface{} { + func(name string, sharedPluginProcess process.RestartableProcess) interface{} { return &restartableObjectStore{ - key: kindAndName{kind: framework.PluginKindObjectStore, name: name}, + key: process.KindAndName{Kind: framework.PluginKindObjectStore, Name: name}, sharedPluginProcess: sharedPluginProcess, } }, @@ -197,9 +199,9 @@ func TestGetVolumeSnapshotter(t *testing.T) { func(m Manager, name string) (interface{}, error) { return m.GetVolumeSnapshotter(name) }, - func(name string, sharedPluginProcess RestartableProcess) interface{} { + func(name string, sharedPluginProcess process.RestartableProcess) interface{} { return &restartableVolumeSnapshotter{ - key: kindAndName{kind: framework.PluginKindVolumeSnapshotter, name: name}, + key: process.KindAndName{Kind: framework.PluginKindVolumeSnapshotter, Name: name}, sharedPluginProcess: sharedPluginProcess, } }, @@ -214,10 +216,10 @@ func TestGetBackupItemAction(t *testing.T) { func(m Manager, name string) (interface{}, error) { return m.GetBackupItemAction(name) }, - func(name string, sharedPluginProcess RestartableProcess) interface{} { - return &restartableBackupItemAction{ - key: kindAndName{kind: framework.PluginKindBackupItemAction, name: name}, - sharedPluginProcess: sharedPluginProcess, + func(name string, sharedPluginProcess process.RestartableProcess) interface{} { + return &biav1cli.RestartableBackupItemAction{ + Key: process.KindAndName{Kind: framework.PluginKindBackupItemAction, Name: name}, + SharedPluginProcess: sharedPluginProcess, } }, false, @@ -231,9 +233,9 @@ func TestGetRestoreItemAction(t *testing.T) { func(m Manager, name string) (interface{}, error) { return m.GetRestoreItemAction(name) }, - func(name string, sharedPluginProcess RestartableProcess) interface{} { + func(name string, sharedPluginProcess process.RestartableProcess) interface{} { return &restartableRestoreItemAction{ - key: kindAndName{kind: framework.PluginKindRestoreItemAction, name: name}, + key: process.KindAndName{Kind: framework.PluginKindRestoreItemAction, Name: name}, sharedPluginProcess: sharedPluginProcess, } }, @@ -246,7 +248,7 @@ func getPluginTest( kind framework.PluginKind, name string, getPluginFunc func(m Manager, name string) (interface{}, error), - expectedResultFunc func(name string, sharedPluginProcess RestartableProcess) interface{}, + expectedResultFunc func(name string, sharedPluginProcess process.RestartableProcess) interface{}, reinitializable bool, ) { logger := test.NewLogger() @@ -273,18 +275,18 @@ func getPluginTest( defer restartableProcess.AssertExpectations(t) // Test 1: error getting restartable process - factory.On("newRestartableProcess", pluginID.Command, logger, logLevel).Return(nil, errors.Errorf("newRestartableProcess")).Once() + factory.On("NewRestartableProcess", pluginID.Command, logger, logLevel).Return(nil, errors.Errorf("NewRestartableProcess")).Once() actual, err := getPluginFunc(m, pluginName) assert.Nil(t, actual) - assert.EqualError(t, err, "newRestartableProcess") + assert.EqualError(t, err, "NewRestartableProcess") // Test 2: happy path - factory.On("newRestartableProcess", pluginID.Command, logger, logLevel).Return(restartableProcess, nil).Once() + factory.On("NewRestartableProcess", pluginID.Command, logger, logLevel).Return(restartableProcess, nil).Once() expected := expectedResultFunc(name, restartableProcess) if reinitializable { - key := kindAndName{kind: pluginID.Kind, name: pluginID.Name} - restartableProcess.On("addReinitializer", key, expected) + key := process.KindAndName{Kind: pluginID.Kind, Name: pluginID.Name} + restartableProcess.On("AddReinitializer", key, expected) } actual, err = getPluginFunc(m, pluginName) @@ -306,8 +308,8 @@ func TestGetBackupItemActions(t *testing.T) { { name: "Error getting restartable process", names: []string{"velero.io/a", "velero.io/b", "velero.io/c"}, - newRestartableProcessError: errors.Errorf("newRestartableProcess"), - expectedError: "newRestartableProcess", + newRestartableProcessError: errors.Errorf("NewRestartableProcess"), + expectedError: "NewRestartableProcess", }, { name: "Happy path", @@ -349,20 +351,20 @@ func TestGetBackupItemActions(t *testing.T) { restartableProcess := &mockRestartableProcess{} defer restartableProcess.AssertExpectations(t) - expected := &restartableBackupItemAction{ - key: kindAndName{kind: pluginKind, name: pluginName}, - sharedPluginProcess: restartableProcess, + expected := &biav1cli.RestartableBackupItemAction{ + Key: process.KindAndName{Kind: pluginKind, Name: pluginName}, + SharedPluginProcess: restartableProcess, } if tc.newRestartableProcessError != nil { // Test 1: error getting restartable process - factory.On("newRestartableProcess", pluginID.Command, logger, logLevel).Return(nil, errors.Errorf("newRestartableProcess")).Once() + factory.On("NewRestartableProcess", pluginID.Command, logger, logLevel).Return(nil, errors.Errorf("NewRestartableProcess")).Once() break } // Test 2: happy path if i == 0 { - factory.On("newRestartableProcess", pluginID.Command, logger, logLevel).Return(restartableProcess, nil).Once() + factory.On("NewRestartableProcess", pluginID.Command, logger, logLevel).Return(restartableProcess, nil).Once() } expectedActions = append(expectedActions, expected) @@ -371,7 +373,7 @@ func TestGetBackupItemActions(t *testing.T) { backupItemActions, err := m.GetBackupItemActions() if tc.newRestartableProcessError != nil { assert.Nil(t, backupItemActions) - assert.EqualError(t, err, "newRestartableProcess") + assert.EqualError(t, err, "NewRestartableProcess") } else { require.NoError(t, err) var actual []interface{} @@ -398,8 +400,8 @@ func TestGetRestoreItemActions(t *testing.T) { { name: "Error getting restartable process", names: []string{"velero.io/a", "velero.io/b", "velero.io/c"}, - newRestartableProcessError: errors.Errorf("newRestartableProcess"), - expectedError: "newRestartableProcess", + newRestartableProcessError: errors.Errorf("NewRestartableProcess"), + expectedError: "NewRestartableProcess", }, { name: "Happy path", @@ -442,19 +444,19 @@ func TestGetRestoreItemActions(t *testing.T) { defer restartableProcess.AssertExpectations(t) expected := &restartableRestoreItemAction{ - key: kindAndName{kind: pluginKind, name: pluginName}, + key: process.KindAndName{Kind: pluginKind, Name: pluginName}, sharedPluginProcess: restartableProcess, } if tc.newRestartableProcessError != nil { // Test 1: error getting restartable process - factory.On("newRestartableProcess", pluginID.Command, logger, logLevel).Return(nil, errors.Errorf("newRestartableProcess")).Once() + factory.On("NewRestartableProcess", pluginID.Command, logger, logLevel).Return(nil, errors.Errorf("NewRestartableProcess")).Once() break } // Test 2: happy path if i == 0 { - factory.On("newRestartableProcess", pluginID.Command, logger, logLevel).Return(restartableProcess, nil).Once() + factory.On("NewRestartableProcess", pluginID.Command, logger, logLevel).Return(restartableProcess, nil).Once() } expectedActions = append(expectedActions, expected) @@ -463,7 +465,7 @@ func TestGetRestoreItemActions(t *testing.T) { restoreItemActions, err := m.GetRestoreItemActions() if tc.newRestartableProcessError != nil { assert.Nil(t, restoreItemActions) - assert.EqualError(t, err, "newRestartableProcess") + assert.EqualError(t, err, "NewRestartableProcess") } else { require.NoError(t, err) var actual []interface{} @@ -483,9 +485,9 @@ func TestGetDeleteItemAction(t *testing.T) { func(m Manager, name string) (interface{}, error) { return m.GetDeleteItemAction(name) }, - func(name string, sharedPluginProcess RestartableProcess) interface{} { + func(name string, sharedPluginProcess process.RestartableProcess) interface{} { return &restartableDeleteItemAction{ - key: kindAndName{kind: framework.PluginKindDeleteItemAction, name: name}, + key: process.KindAndName{Kind: framework.PluginKindDeleteItemAction, Name: name}, sharedPluginProcess: sharedPluginProcess, } }, @@ -541,19 +543,19 @@ func TestGetDeleteItemActions(t *testing.T) { defer restartableProcess.AssertExpectations(t) expected := &restartableRestoreItemAction{ - key: kindAndName{kind: pluginKind, name: pluginName}, + key: process.KindAndName{Kind: pluginKind, Name: pluginName}, sharedPluginProcess: restartableProcess, } if tc.newRestartableProcessError != nil { // Test 1: error getting restartable process - factory.On("newRestartableProcess", pluginID.Command, logger, logLevel).Return(nil, errors.Errorf("newRestartableProcess")).Once() + factory.On("NewRestartableProcess", pluginID.Command, logger, logLevel).Return(nil, errors.Errorf("NewRestartableProcess")).Once() break } // Test 2: happy path if i == 0 { - factory.On("newRestartableProcess", pluginID.Command, logger, logLevel).Return(restartableProcess, nil).Once() + factory.On("NewRestartableProcess", pluginID.Command, logger, logLevel).Return(restartableProcess, nil).Once() } expectedActions = append(expectedActions, expected) @@ -562,7 +564,7 @@ func TestGetDeleteItemActions(t *testing.T) { deleteItemActions, err := m.GetDeleteItemActions() if tc.newRestartableProcessError != nil { assert.Nil(t, deleteItemActions) - assert.EqualError(t, err, "newRestartableProcess") + assert.EqualError(t, err, "NewRestartableProcess") } else { require.NoError(t, err) var actual []interface{} diff --git a/pkg/plugin/clientmgmt/client_builder.go b/pkg/plugin/clientmgmt/process/client_builder.go similarity index 99% rename from pkg/plugin/clientmgmt/client_builder.go rename to pkg/plugin/clientmgmt/process/client_builder.go index 76e3b1985..0d3948807 100644 --- a/pkg/plugin/clientmgmt/client_builder.go +++ b/pkg/plugin/clientmgmt/process/client_builder.go @@ -15,7 +15,7 @@ limitations under the License. */ // Package clientmgmt contains the plugin client for Velero. -package clientmgmt +package process import ( "os" diff --git a/pkg/plugin/clientmgmt/client_builder_test.go b/pkg/plugin/clientmgmt/process/client_builder_test.go similarity index 99% rename from pkg/plugin/clientmgmt/client_builder_test.go rename to pkg/plugin/clientmgmt/process/client_builder_test.go index e922c3d00..1b9c77145 100644 --- a/pkg/plugin/clientmgmt/client_builder_test.go +++ b/pkg/plugin/clientmgmt/process/client_builder_test.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package clientmgmt +package process import ( "os" diff --git a/pkg/plugin/clientmgmt/logrus_adapter.go b/pkg/plugin/clientmgmt/process/logrus_adapter.go similarity index 99% rename from pkg/plugin/clientmgmt/logrus_adapter.go rename to pkg/plugin/clientmgmt/process/logrus_adapter.go index 3bb55d464..cb73da789 100644 --- a/pkg/plugin/clientmgmt/logrus_adapter.go +++ b/pkg/plugin/clientmgmt/process/logrus_adapter.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package clientmgmt +package process import ( "fmt" diff --git a/pkg/plugin/clientmgmt/logrus_adapter_test.go b/pkg/plugin/clientmgmt/process/logrus_adapter_test.go similarity index 99% rename from pkg/plugin/clientmgmt/logrus_adapter_test.go rename to pkg/plugin/clientmgmt/process/logrus_adapter_test.go index ccfdfa828..dae7f8ce2 100644 --- a/pkg/plugin/clientmgmt/logrus_adapter_test.go +++ b/pkg/plugin/clientmgmt/process/logrus_adapter_test.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 clientmgmt +package process import ( "testing" diff --git a/pkg/plugin/clientmgmt/process.go b/pkg/plugin/clientmgmt/process/process.go similarity index 93% rename from pkg/plugin/clientmgmt/process.go rename to pkg/plugin/clientmgmt/process/process.go index f2fc8ae36..66033768d 100644 --- a/pkg/plugin/clientmgmt/process.go +++ b/pkg/plugin/clientmgmt/process/process.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package clientmgmt +package process import ( "strings" @@ -42,7 +42,7 @@ func (pf *processFactory) newProcess(command string, logger logrus.FieldLogger, } type Process interface { - dispense(key kindAndName) (interface{}, error) + dispense(key KindAndName) (interface{}, error) exited() bool kill() } @@ -124,21 +124,21 @@ func removeFeaturesFlag(args []string) []string { return commandArgs } -func (r *process) dispense(key kindAndName) (interface{}, error) { +func (r *process) dispense(key KindAndName) (interface{}, error) { // This calls GRPCClient(clientConn) on the plugin instance registered for key.name. - dispensed, err := r.protocolClient.Dispense(key.kind.String()) + dispensed, err := r.protocolClient.Dispense(key.Kind.String()) if err != nil { return nil, errors.WithStack(err) } // Currently all plugins except for PluginLister dispense clientDispenser instances. if clientDispenser, ok := dispensed.(framework.ClientDispenser); ok { - if key.name == "" { - return nil, errors.Errorf("%s plugin requested but name is missing", key.kind.String()) + if key.Name == "" { + return nil, errors.Errorf("%s plugin requested but name is missing", key.Kind.String()) } // Get the instance that implements our plugin interface (e.g. ObjectStore) that is a gRPC-based // client - dispensed = clientDispenser.ClientFor(key.name) + dispensed = clientDispenser.ClientFor(key.Name) } return dispensed, nil diff --git a/pkg/plugin/clientmgmt/process_test.go b/pkg/plugin/clientmgmt/process/process_test.go similarity index 92% rename from pkg/plugin/clientmgmt/process_test.go rename to pkg/plugin/clientmgmt/process/process_test.go index ac82ade87..a44254dbd 100644 --- a/pkg/plugin/clientmgmt/process_test.go +++ b/pkg/plugin/clientmgmt/process/process_test.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 clientmgmt +package process import ( "testing" @@ -94,20 +94,20 @@ func TestDispense(t *testing.T) { var client interface{} - key := kindAndName{} + key := KindAndName{} if tc.clientDispenser { - key.kind = framework.PluginKindObjectStore - protocolClient.On("Dispense", key.kind.String()).Return(clientDispenser, tc.dispenseError) + key.Kind = framework.PluginKindObjectStore + protocolClient.On("Dispense", key.Kind.String()).Return(clientDispenser, tc.dispenseError) if !tc.missingKeyName { - key.name = "aws" + key.Name = "aws" client = &framework.BackupItemActionGRPCClient{} - clientDispenser.On("ClientFor", key.name).Return(client) + clientDispenser.On("ClientFor", key.Name).Return(client) } } else { - key.kind = framework.PluginKindPluginLister + key.Kind = framework.PluginKindPluginLister client = &framework.PluginListerGRPCClient{} - protocolClient.On("Dispense", key.kind.String()).Return(client, tc.dispenseError) + protocolClient.On("Dispense", key.Kind.String()).Return(client, tc.dispenseError) } dispensed, err := p.dispense(key) diff --git a/pkg/plugin/clientmgmt/registry.go b/pkg/plugin/clientmgmt/process/registry.go similarity index 91% rename from pkg/plugin/clientmgmt/registry.go rename to pkg/plugin/clientmgmt/process/registry.go index fe8a7f4b4..2e51ea2a2 100644 --- a/pkg/plugin/clientmgmt/registry.go +++ b/pkg/plugin/clientmgmt/process/registry.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package clientmgmt +package process import ( "fmt" @@ -38,10 +38,10 @@ type Registry interface { Get(kind framework.PluginKind, name string) (framework.PluginIdentifier, error) } -// kindAndName is a convenience struct that combines a PluginKind and a name. -type kindAndName struct { - kind framework.PluginKind - name string +// KindAndName is a convenience struct that combines a PluginKind and a name. +type KindAndName struct { + Kind framework.PluginKind + Name string } // registry implements Registry. @@ -53,7 +53,7 @@ type registry struct { processFactory ProcessFactory fs filesystem.Interface - pluginsByID map[kindAndName]framework.PluginIdentifier + pluginsByID map[KindAndName]framework.PluginIdentifier pluginsByKind map[framework.PluginKind][]framework.PluginIdentifier } @@ -66,7 +66,7 @@ func NewRegistry(dir string, logger logrus.FieldLogger, logLevel logrus.Level) R processFactory: newProcessFactory(), fs: filesystem.NewFileSystem(), - pluginsByID: make(map[kindAndName]framework.PluginIdentifier), + pluginsByID: make(map[KindAndName]framework.PluginIdentifier), pluginsByKind: make(map[framework.PluginKind][]framework.PluginIdentifier), } } @@ -117,7 +117,7 @@ func (r *registry) List(kind framework.PluginKind) []framework.PluginIdentifier // Get returns info about a plugin with the given name and kind, or an // error if one cannot be found. func (r *registry) Get(kind framework.PluginKind, name string) (framework.PluginIdentifier, error) { - p, found := r.pluginsByID[kindAndName{kind: kind, name: name}] + p, found := r.pluginsByID[KindAndName{Kind: kind, Name: name}] if !found { return framework.PluginIdentifier{}, newPluginNotFoundError(kind, name) } @@ -182,7 +182,7 @@ func (r *registry) listPlugins(command string) ([]framework.PluginIdentifier, er } defer process.kill() - plugin, err := process.dispense(kindAndName{kind: framework.PluginKindPluginLister}) + plugin, err := process.dispense(KindAndName{Kind: framework.PluginKindPluginLister}) if err != nil { return nil, err } @@ -197,7 +197,7 @@ func (r *registry) listPlugins(command string) ([]framework.PluginIdentifier, er // register registers a PluginIdentifier with the registry. func (r *registry) register(id framework.PluginIdentifier) error { - key := kindAndName{kind: id.Kind, name: id.Name} + key := KindAndName{Kind: id.Kind, Name: id.Name} if existing, found := r.pluginsByID[key]; found { return newDuplicatePluginRegistrationError(existing, id) } @@ -214,20 +214,20 @@ func (r *registry) register(id framework.PluginIdentifier) error { } // pluginNotFoundError indicates a plugin could not be located for kind and name. -type pluginNotFoundError struct { +type PluginNotFoundError struct { kind framework.PluginKind name string } // newPluginNotFoundError returns a new pluginNotFoundError for kind and name. -func newPluginNotFoundError(kind framework.PluginKind, name string) *pluginNotFoundError { - return &pluginNotFoundError{ +func newPluginNotFoundError(kind framework.PluginKind, name string) *PluginNotFoundError { + return &PluginNotFoundError{ kind: kind, name: name, } } -func (e *pluginNotFoundError) Error() string { +func (e *PluginNotFoundError) Error() string { return fmt.Sprintf("unable to locate %v plugin named %s", e.kind, e.name) } diff --git a/pkg/plugin/clientmgmt/registry_test.go b/pkg/plugin/clientmgmt/process/registry_test.go similarity index 99% rename from pkg/plugin/clientmgmt/registry_test.go rename to pkg/plugin/clientmgmt/process/registry_test.go index 45bbcbb89..cd0614f85 100644 --- a/pkg/plugin/clientmgmt/registry_test.go +++ b/pkg/plugin/clientmgmt/process/registry_test.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 clientmgmt +package process import ( "os" diff --git a/pkg/plugin/clientmgmt/restartable_process.go b/pkg/plugin/clientmgmt/process/restartable_process.go similarity index 75% rename from pkg/plugin/clientmgmt/restartable_process.go rename to pkg/plugin/clientmgmt/process/restartable_process.go index 54211d8d1..21ed81022 100644 --- a/pkg/plugin/clientmgmt/restartable_process.go +++ b/pkg/plugin/clientmgmt/process/restartable_process.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package clientmgmt +package process import ( "sync" @@ -24,26 +24,26 @@ import ( ) type RestartableProcessFactory interface { - newRestartableProcess(command string, logger logrus.FieldLogger, logLevel logrus.Level) (RestartableProcess, error) + NewRestartableProcess(command string, logger logrus.FieldLogger, logLevel logrus.Level) (RestartableProcess, error) } type restartableProcessFactory struct { } -func newRestartableProcessFactory() RestartableProcessFactory { +func NewRestartableProcessFactory() RestartableProcessFactory { return &restartableProcessFactory{} } -func (rpf *restartableProcessFactory) newRestartableProcess(command string, logger logrus.FieldLogger, logLevel logrus.Level) (RestartableProcess, error) { +func (rpf *restartableProcessFactory) NewRestartableProcess(command string, logger logrus.FieldLogger, logLevel logrus.Level) (RestartableProcess, error) { return newRestartableProcess(command, logger, logLevel) } type RestartableProcess interface { - addReinitializer(key kindAndName, r reinitializer) - reset() error - resetIfNeeded() error - getByKindAndName(key kindAndName) (interface{}, error) - stop() + AddReinitializer(key KindAndName, r Reinitializer) + Reset() error + ResetIfNeeded() error + GetByKindAndName(key KindAndName) (interface{}, error) + Stop() } // restartableProcess encapsulates the lifecycle for all plugins contained in a single executable file. It is able @@ -57,15 +57,15 @@ type restartableProcess struct { // lock guards all of the fields below lock sync.RWMutex process Process - plugins map[kindAndName]interface{} - reinitializers map[kindAndName]reinitializer + plugins map[KindAndName]interface{} + reinitializers map[KindAndName]Reinitializer resetFailures int } // reinitializer is capable of reinitializing a restartable plugin instance using the newly dispensed plugin. -type reinitializer interface { +type Reinitializer interface { // reinitialize reinitializes a restartable plugin instance using the newly dispensed plugin. - reinitialize(dispensed interface{}) error + Reinitialize(dispensed interface{}) error } // newRestartableProcess creates a new restartableProcess for the given command and options. @@ -74,26 +74,26 @@ func newRestartableProcess(command string, logger logrus.FieldLogger, logLevel l command: command, logger: logger, logLevel: logLevel, - plugins: make(map[kindAndName]interface{}), - reinitializers: make(map[kindAndName]reinitializer), + plugins: make(map[KindAndName]interface{}), + reinitializers: make(map[KindAndName]Reinitializer), } // This launches the process - err := p.reset() + err := p.Reset() return p, err } -// addReinitializer registers the reinitializer r for key. -func (p *restartableProcess) addReinitializer(key kindAndName, r reinitializer) { +// AddReinitializer registers the reinitializer r for key. +func (p *restartableProcess) AddReinitializer(key KindAndName, r Reinitializer) { p.lock.Lock() defer p.lock.Unlock() p.reinitializers[key] = r } -// reset acquires the lock and calls resetLH. -func (p *restartableProcess) reset() error { +// Reset acquires the lock and calls resetLH. +func (p *restartableProcess) Reset() error { p.lock.Lock() defer p.lock.Unlock() @@ -118,7 +118,7 @@ func (p *restartableProcess) resetLH() error { // Redispense any previously dispensed plugins, reinitializing if necessary. // Start by creating a new map to hold the newly dispensed plugins. - newPlugins := make(map[kindAndName]interface{}) + newPlugins := make(map[KindAndName]interface{}) for key := range p.plugins { // Re-dispense dispensed, err := p.process.dispense(key) @@ -131,7 +131,7 @@ func (p *restartableProcess) resetLH() error { // Reinitialize if r, found := p.reinitializers[key]; found { - if err := r.reinitialize(dispensed); err != nil { + if err := r.Reinitialize(dispensed); err != nil { p.resetFailures++ return err } @@ -146,8 +146,8 @@ func (p *restartableProcess) resetLH() error { return nil } -// resetIfNeeded checks if the plugin process has exited and resets p if it has. -func (p *restartableProcess) resetIfNeeded() error { +// ResetIfNeeded checks if the plugin process has exited and resets p if it has. +func (p *restartableProcess) ResetIfNeeded() error { p.lock.Lock() defer p.lock.Unlock() @@ -159,8 +159,8 @@ func (p *restartableProcess) resetIfNeeded() error { return nil } -// getByKindAndName acquires the lock and calls getByKindAndNameLH. -func (p *restartableProcess) getByKindAndName(key kindAndName) (interface{}, error) { +// GetByKindAndName acquires the lock and calls getByKindAndNameLH. +func (p *restartableProcess) GetByKindAndName(key KindAndName) (interface{}, error) { p.lock.Lock() defer p.lock.Unlock() @@ -169,7 +169,7 @@ func (p *restartableProcess) getByKindAndName(key kindAndName) (interface{}, err // getByKindAndNameLH returns the dispensed plugin for key. If the plugin hasn't been dispensed before, it dispenses a // new one. -func (p *restartableProcess) getByKindAndNameLH(key kindAndName) (interface{}, error) { +func (p *restartableProcess) getByKindAndNameLH(key KindAndName) (interface{}, error) { dispensed, found := p.plugins[key] if found { return dispensed, nil @@ -184,7 +184,7 @@ func (p *restartableProcess) getByKindAndNameLH(key kindAndName) (interface{}, e } // stop terminates the plugin process. -func (p *restartableProcess) stop() { +func (p *restartableProcess) Stop() { p.lock.Lock() p.process.kill() p.lock.Unlock() diff --git a/pkg/plugin/clientmgmt/restartable_delegate_test.go b/pkg/plugin/clientmgmt/restartable_delegate_test.go index 3e3366c88..c71d3e276 100644 --- a/pkg/plugin/clientmgmt/restartable_delegate_test.go +++ b/pkg/plugin/clientmgmt/restartable_delegate_test.go @@ -24,6 +24,7 @@ import ( "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" ) @@ -43,7 +44,7 @@ type mockable interface { func runRestartableDelegateTests( t *testing.T, kind framework.PluginKind, - newRestartable func(key kindAndName, p RestartableProcess) interface{}, + newRestartable func(key process.KindAndName, p process.RestartableProcess) interface{}, newMock func() mockable, tests ...restartableDelegateTest, ) { @@ -54,9 +55,9 @@ func runRestartableDelegateTests( defer p.AssertExpectations(t) // getDelegate error - p.On("resetIfNeeded").Return(errors.Errorf("reset error")).Once() + p.On("ResetIfNeeded").Return(errors.Errorf("reset error")).Once() name := "delegateName" - key := kindAndName{kind: kind, name: name} + key := process.KindAndName{Kind: kind, Name: name} r := newRestartable(key, p) // Get the method we're going to call using reflection @@ -107,13 +108,13 @@ func runRestartableDelegateTests( checkOutputs(tc.expectedErrorOutputs, actual) // Invoke delegate, make sure all returned values are passed through - p.On("resetIfNeeded").Return(nil) + p.On("ResetIfNeeded").Return(nil) delegate := newMock() delegate.Test(t) defer delegate.AssertExpectations(t) - p.On("getByKindAndName", key).Return(delegate, nil) + p.On("GetByKindAndName", key).Return(delegate, nil) // Set up the mocked method in the delegate delegate.On(tc.function, tc.inputs...).Return(tc.expectedDelegateOutputs...) diff --git a/pkg/plugin/clientmgmt/restartable_delete_item_action.go b/pkg/plugin/clientmgmt/restartable_delete_item_action.go index 266500c7a..977203aae 100644 --- a/pkg/plugin/clientmgmt/restartable_delete_item_action.go +++ b/pkg/plugin/clientmgmt/restartable_delete_item_action.go @@ -19,6 +19,7 @@ package clientmgmt import ( "github.com/pkg/errors" + "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" ) @@ -28,15 +29,15 @@ import ( // call, the restartableDeleteItemAction 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 restartableDeleteItemAction struct { - key kindAndName - sharedPluginProcess RestartableProcess + key process.KindAndName + sharedPluginProcess process.RestartableProcess config map[string]string } -// newRestartableDeleteItemAction returns a new restartableDeleteItemAction. -func newRestartableDeleteItemAction(name string, sharedPluginProcess RestartableProcess) *restartableDeleteItemAction { +// NewRestartableDeleteItemAction returns a new restartableDeleteItemAction. +func NewRestartableDeleteItemAction(name string, sharedPluginProcess process.RestartableProcess) *restartableDeleteItemAction { r := &restartableDeleteItemAction{ - key: kindAndName{kind: framework.PluginKindDeleteItemAction, name: name}, + key: process.KindAndName{Kind: framework.PluginKindDeleteItemAction, Name: name}, sharedPluginProcess: sharedPluginProcess, } return r @@ -45,7 +46,7 @@ func newRestartableDeleteItemAction(name string, sharedPluginProcess Restartable // getDeleteItemAction returns the delete item action for this restartableDeleteItemAction. It does *not* restart the // plugin process. func (r *restartableDeleteItemAction) getDeleteItemAction() (velero.DeleteItemAction, error) { - plugin, err := r.sharedPluginProcess.getByKindAndName(r.key) + plugin, err := r.sharedPluginProcess.GetByKindAndName(r.key) if err != nil { return nil, err } @@ -60,7 +61,7 @@ func (r *restartableDeleteItemAction) getDeleteItemAction() (velero.DeleteItemAc // getDelegate restarts the plugin process (if needed) and returns the delete item action for this restartableDeleteItemAction. func (r *restartableDeleteItemAction) getDelegate() (velero.DeleteItemAction, error) { - if err := r.sharedPluginProcess.resetIfNeeded(); err != nil { + if err := r.sharedPluginProcess.ResetIfNeeded(); err != nil { return nil, err } diff --git a/pkg/plugin/clientmgmt/restartable_delete_item_action_test.go b/pkg/plugin/clientmgmt/restartable_delete_item_action_test.go index 641959e17..65fb9646c 100644 --- a/pkg/plugin/clientmgmt/restartable_delete_item_action_test.go +++ b/pkg/plugin/clientmgmt/restartable_delete_item_action_test.go @@ -25,6 +25,7 @@ import ( "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" 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" "github.com/vmware-tanzu/velero/pkg/plugin/velero/mocks" @@ -59,10 +60,10 @@ func TestRestartableGetDeleteItemAction(t *testing.T) { defer p.AssertExpectations(t) name := "pod" - key := kindAndName{kind: framework.PluginKindDeleteItemAction, name: name} - p.On("getByKindAndName", key).Return(tc.plugin, tc.getError) + key := process.KindAndName{Kind: framework.PluginKindDeleteItemAction, Name: name} + p.On("GetByKindAndName", key).Return(tc.plugin, tc.getError) - r := newRestartableDeleteItemAction(name, p) + r := NewRestartableDeleteItemAction(name, p) a, err := r.getDeleteItemAction() if tc.expectedError != "" { assert.EqualError(t, err, tc.expectedError) @@ -80,19 +81,19 @@ func TestRestartableDeleteItemActionGetDelegate(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 := newRestartableDeleteItemAction(name, p) + r := NewRestartableDeleteItemAction(name, p) a, err := r.getDelegate() assert.Nil(t, a) assert.EqualError(t, err, "reset error") // Happy path // Currently broken since this mocks out the restore item action interface - p.On("resetIfNeeded").Return(nil) + p.On("ResetIfNeeded").Return(nil) expected := new(mocks.DeleteItemAction) - key := kindAndName{kind: framework.PluginKindDeleteItemAction, name: name} - p.On("getByKindAndName", key).Return(expected, nil) + key := process.KindAndName{Kind: framework.PluginKindDeleteItemAction, Name: name} + p.On("GetByKindAndName", key).Return(expected, nil) a, err = r.getDelegate() assert.NoError(t, err) @@ -116,7 +117,7 @@ func TestRestartableDeleteItemActionDelegatedFunctions(t *testing.T) { runRestartableDelegateTests( t, framework.PluginKindDeleteItemAction, - func(key kindAndName, p RestartableProcess) interface{} { + func(key process.KindAndName, p process.RestartableProcess) interface{} { return &restartableDeleteItemAction{ key: key, sharedPluginProcess: p, diff --git a/pkg/plugin/clientmgmt/restartable_item_snapshotter.go b/pkg/plugin/clientmgmt/restartable_item_snapshotter.go index e211bcf28..d2743789a 100644 --- a/pkg/plugin/clientmgmt/restartable_item_snapshotter.go +++ b/pkg/plugin/clientmgmt/restartable_item_snapshotter.go @@ -21,21 +21,21 @@ import ( "github.com/pkg/errors" - isv1 "github.com/vmware-tanzu/velero/pkg/plugin/velero/item_snapshotter/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" + isv1 "github.com/vmware-tanzu/velero/pkg/plugin/velero/item_snapshotter/v1" ) type restartableItemSnapshotter struct { - key kindAndName - sharedPluginProcess RestartableProcess + key process.KindAndName + sharedPluginProcess process.RestartableProcess } -// newRestartableItemSnapshotter returns a new newRestartableItemSnapshotter. -func newRestartableItemSnapshotter(name string, sharedPluginProcess RestartableProcess) *restartableItemSnapshotter { +// NewRestartableItemSnapshotter returns a new restartableItemSnapshotter. +func NewRestartableItemSnapshotter(name string, sharedPluginProcess process.RestartableProcess) *restartableItemSnapshotter { r := &restartableItemSnapshotter{ - key: kindAndName{kind: framework.PluginKindItemSnapshotter, name: name}, + key: process.KindAndName{Kind: framework.PluginKindItemSnapshotter, Name: name}, sharedPluginProcess: sharedPluginProcess, } return r @@ -44,7 +44,7 @@ func newRestartableItemSnapshotter(name string, sharedPluginProcess RestartableP // getItemSnapshotter returns the item snapshotter for this restartableItemSnapshotter. It does *not* restart the // plugin process. func (r *restartableItemSnapshotter) getItemSnapshotter() (isv1.ItemSnapshotter, error) { - plugin, err := r.sharedPluginProcess.getByKindAndName(r.key) + plugin, err := r.sharedPluginProcess.GetByKindAndName(r.key) if err != nil { return nil, err } @@ -59,7 +59,7 @@ func (r *restartableItemSnapshotter) getItemSnapshotter() (isv1.ItemSnapshotter, // getDelegate restarts the plugin process (if needed) and returns the item snapshotter for this restartableItemSnapshotter. func (r *restartableItemSnapshotter) getDelegate() (isv1.ItemSnapshotter, error) { - if err := r.sharedPluginProcess.resetIfNeeded(); err != nil { + if err := r.sharedPluginProcess.ResetIfNeeded(); err != nil { return nil, err } diff --git a/pkg/plugin/clientmgmt/restartable_item_snapshotter_test.go b/pkg/plugin/clientmgmt/restartable_item_snapshotter_test.go index 30b2aab43..70d4a934a 100644 --- a/pkg/plugin/clientmgmt/restartable_item_snapshotter_test.go +++ b/pkg/plugin/clientmgmt/restartable_item_snapshotter_test.go @@ -21,8 +21,6 @@ import ( "testing" "time" - isv1 "github.com/vmware-tanzu/velero/pkg/plugin/velero/item_snapshotter/v1" - "github.com/pkg/errors" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -32,8 +30,10 @@ import ( v1 "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" "github.com/vmware-tanzu/velero/pkg/plugin/velero/item_snapshotter/v1/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" + isv1 "github.com/vmware-tanzu/velero/pkg/plugin/velero/item_snapshotter/v1" ) func TestRestartableGetItemSnapshotter(t *testing.T) { @@ -65,10 +65,10 @@ func TestRestartableGetItemSnapshotter(t *testing.T) { defer p.AssertExpectations(t) name := "pvc" - key := kindAndName{kind: framework.PluginKindItemSnapshotter, name: name} - p.On("getByKindAndName", key).Return(tc.plugin, tc.getError) + key := process.KindAndName{Kind: framework.PluginKindItemSnapshotter, Name: name} + p.On("GetByKindAndName", key).Return(tc.plugin, tc.getError) - r := newRestartableItemSnapshotter(name, p) + r := NewRestartableItemSnapshotter(name, p) a, err := r.getItemSnapshotter() if tc.expectedError != "" { assert.EqualError(t, err, tc.expectedError) @@ -86,18 +86,18 @@ func TestRestartableItemSnapshotterGetDelegate(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 := "pvc" - r := newRestartableItemSnapshotter(name, p) + r := NewRestartableItemSnapshotter(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.ItemSnapshotter) - key := kindAndName{kind: framework.PluginKindItemSnapshotter, name: name} - p.On("getByKindAndName", key).Return(expected, nil) + key := process.KindAndName{Kind: framework.PluginKindItemSnapshotter, Name: name} + p.On("GetByKindAndName", key).Return(expected, nil) a, err = r.getDelegate() assert.NoError(t, err) @@ -178,7 +178,7 @@ func TestRestartableItemSnasphotterDelegatedFunctions(t *testing.T) { runRestartableDelegateTests( t, framework.PluginKindItemSnapshotter, - func(key kindAndName, p RestartableProcess) interface{} { + func(key process.KindAndName, p process.RestartableProcess) interface{} { return &restartableItemSnapshotter{ key: key, sharedPluginProcess: p, diff --git a/pkg/plugin/clientmgmt/restartable_object_store.go b/pkg/plugin/clientmgmt/restartable_object_store.go index 95fbc75c5..5be111340 100644 --- a/pkg/plugin/clientmgmt/restartable_object_store.go +++ b/pkg/plugin/clientmgmt/restartable_object_store.go @@ -22,6 +22,7 @@ import ( "github.com/pkg/errors" + "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" ) @@ -31,29 +32,29 @@ import ( // call, the restartableObjectStore 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 restartableObjectStore struct { - key kindAndName - sharedPluginProcess RestartableProcess + key process.KindAndName + sharedPluginProcess process.RestartableProcess // config contains the data used to initialize the plugin. It is used to reinitialize the plugin in the event its // sharedPluginProcess gets restarted. config map[string]string } -// newRestartableObjectStore returns a new restartableObjectStore. -func newRestartableObjectStore(name string, sharedPluginProcess RestartableProcess) *restartableObjectStore { - key := kindAndName{kind: framework.PluginKindObjectStore, name: name} +// NewRestartableObjectStore returns a new restartableObjectStore. +func NewRestartableObjectStore(name string, sharedPluginProcess process.RestartableProcess) *restartableObjectStore { + key := process.KindAndName{Kind: framework.PluginKindObjectStore, Name: name} r := &restartableObjectStore{ key: key, sharedPluginProcess: sharedPluginProcess, } // Register our reinitializer so we can reinitialize after a restart with r.config. - sharedPluginProcess.addReinitializer(key, r) + sharedPluginProcess.AddReinitializer(key, r) return r } // reinitialize reinitializes a re-dispensed plugin using the initial data passed to Init(). -func (r *restartableObjectStore) reinitialize(dispensed interface{}) error { +func (r *restartableObjectStore) Reinitialize(dispensed interface{}) error { objectStore, ok := dispensed.(velero.ObjectStore) if !ok { return errors.Errorf("%T is not a ObjectStore!", dispensed) @@ -65,7 +66,7 @@ func (r *restartableObjectStore) reinitialize(dispensed interface{}) error { // getObjectStore returns the object store for this restartableObjectStore. It does *not* restart the // plugin process. func (r *restartableObjectStore) getObjectStore() (velero.ObjectStore, error) { - plugin, err := r.sharedPluginProcess.getByKindAndName(r.key) + plugin, err := r.sharedPluginProcess.GetByKindAndName(r.key) if err != nil { return nil, err } @@ -80,7 +81,7 @@ func (r *restartableObjectStore) getObjectStore() (velero.ObjectStore, error) { // getDelegate restarts the plugin process (if needed) and returns the object store for this restartableObjectStore. func (r *restartableObjectStore) getDelegate() (velero.ObjectStore, error) { - if err := r.sharedPluginProcess.resetIfNeeded(); err != nil { + if err := r.sharedPluginProcess.ResetIfNeeded(); err != nil { return nil, err } diff --git a/pkg/plugin/clientmgmt/restartable_object_store_test.go b/pkg/plugin/clientmgmt/restartable_object_store_test.go index 57a64e8b2..b4e94fccc 100644 --- a/pkg/plugin/clientmgmt/restartable_object_store_test.go +++ b/pkg/plugin/clientmgmt/restartable_object_store_test.go @@ -26,6 +26,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/vmware-tanzu/velero/pkg/plugin/clientmgmt/process" "github.com/vmware-tanzu/velero/pkg/plugin/framework" providermocks "github.com/vmware-tanzu/velero/pkg/plugin/velero/mocks" ) @@ -60,8 +61,8 @@ func TestRestartableGetObjectStore(t *testing.T) { defer p.AssertExpectations(t) name := "aws" - key := kindAndName{kind: framework.PluginKindObjectStore, name: name} - p.On("getByKindAndName", key).Return(tc.plugin, tc.getError) + key := process.KindAndName{Kind: framework.PluginKindObjectStore, Name: name} + p.On("GetByKindAndName", key).Return(tc.plugin, tc.getError) r := &restartableObjectStore{ key: key, @@ -85,7 +86,7 @@ func TestRestartableObjectStoreReinitialize(t *testing.T) { defer p.AssertExpectations(t) name := "aws" - key := kindAndName{kind: framework.PluginKindObjectStore, name: name} + key := process.KindAndName{Kind: framework.PluginKindObjectStore, Name: name} r := &restartableObjectStore{ key: key, sharedPluginProcess: p, @@ -94,7 +95,7 @@ func TestRestartableObjectStoreReinitialize(t *testing.T) { }, } - err := r.reinitialize(3) + err := r.Reinitialize(3) assert.EqualError(t, err, "int is not a ObjectStore!") objectStore := new(providermocks.ObjectStore) @@ -102,11 +103,11 @@ func TestRestartableObjectStoreReinitialize(t *testing.T) { defer objectStore.AssertExpectations(t) objectStore.On("Init", r.config).Return(errors.Errorf("init error")).Once() - err = r.reinitialize(objectStore) + err = r.Reinitialize(objectStore) assert.EqualError(t, err, "init error") objectStore.On("Init", r.config).Return(nil) - err = r.reinitialize(objectStore) + err = r.Reinitialize(objectStore) assert.NoError(t, err) } @@ -116,9 +117,9 @@ func TestRestartableObjectStoreGetDelegate(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 := "aws" - key := kindAndName{kind: framework.PluginKindObjectStore, name: name} + key := process.KindAndName{Kind: framework.PluginKindObjectStore, Name: name} r := &restartableObjectStore{ key: key, sharedPluginProcess: p, @@ -128,11 +129,11 @@ func TestRestartableObjectStoreGetDelegate(t *testing.T) { assert.EqualError(t, err, "reset error") // Happy path - p.On("resetIfNeeded").Return(nil) + p.On("ResetIfNeeded").Return(nil) objectStore := new(providermocks.ObjectStore) objectStore.Test(t) defer objectStore.AssertExpectations(t) - p.On("getByKindAndName", key).Return(objectStore, nil) + p.On("GetByKindAndName", key).Return(objectStore, nil) a, err = r.getDelegate() assert.NoError(t, err) @@ -146,24 +147,24 @@ func TestRestartableObjectStoreInit(t *testing.T) { // getObjectStore error name := "aws" - key := kindAndName{kind: framework.PluginKindObjectStore, name: name} + key := process.KindAndName{Kind: framework.PluginKindObjectStore, Name: name} r := &restartableObjectStore{ key: key, sharedPluginProcess: p, } - p.On("getByKindAndName", key).Return(nil, errors.Errorf("getByKindAndName error")).Once() + p.On("GetByKindAndName", key).Return(nil, errors.Errorf("GetByKindAndName error")).Once() config := map[string]string{ "color": "blue", } err := r.Init(config) - assert.EqualError(t, err, "getByKindAndName error") + assert.EqualError(t, err, "GetByKindAndName error") // Delegate returns error objectStore := new(providermocks.ObjectStore) objectStore.Test(t) defer objectStore.AssertExpectations(t) - p.On("getByKindAndName", key).Return(objectStore, nil) + p.On("GetByKindAndName", key).Return(objectStore, nil) objectStore.On("Init", config).Return(errors.Errorf("Init error")).Once() err = r.Init(config) @@ -187,7 +188,7 @@ func TestRestartableObjectStoreDelegatedFunctions(t *testing.T) { runRestartableDelegateTests( t, framework.PluginKindObjectStore, - func(key kindAndName, p RestartableProcess) interface{} { + func(key process.KindAndName, p process.RestartableProcess) interface{} { return &restartableObjectStore{ key: key, sharedPluginProcess: p, diff --git a/pkg/plugin/clientmgmt/restartable_restore_item_action.go b/pkg/plugin/clientmgmt/restartable_restore_item_action.go index 0f55ebfde..c4e6911b4 100644 --- a/pkg/plugin/clientmgmt/restartable_restore_item_action.go +++ b/pkg/plugin/clientmgmt/restartable_restore_item_action.go @@ -19,24 +19,25 @@ package clientmgmt import ( "github.com/pkg/errors" + "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" ) -// restartableRestoreItemAction is a restore item action for a given implementation (such as "pod"). It is associated with +// RestartableRestoreItemAction is a restore 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 restartableRestoreItemAction 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 restartableRestoreItemAction struct { - key kindAndName - sharedPluginProcess RestartableProcess + key process.KindAndName + sharedPluginProcess process.RestartableProcess config map[string]string } -// newRestartableRestoreItemAction returns a new restartableRestoreItemAction. -func newRestartableRestoreItemAction(name string, sharedPluginProcess RestartableProcess) *restartableRestoreItemAction { +// NewRestartableRestoreItemAction returns a new RestartableRestoreItemAction. +func NewRestartableRestoreItemAction(name string, sharedPluginProcess process.RestartableProcess) *restartableRestoreItemAction { r := &restartableRestoreItemAction{ - key: kindAndName{kind: framework.PluginKindRestoreItemAction, name: name}, + key: process.KindAndName{Kind: framework.PluginKindRestoreItemAction, Name: name}, sharedPluginProcess: sharedPluginProcess, } return r @@ -45,7 +46,7 @@ func newRestartableRestoreItemAction(name string, sharedPluginProcess Restartabl // getRestoreItemAction returns the restore item action for this restartableRestoreItemAction. It does *not* restart the // plugin process. func (r *restartableRestoreItemAction) getRestoreItemAction() (velero.RestoreItemAction, error) { - plugin, err := r.sharedPluginProcess.getByKindAndName(r.key) + plugin, err := r.sharedPluginProcess.GetByKindAndName(r.key) if err != nil { return nil, err } @@ -60,7 +61,7 @@ func (r *restartableRestoreItemAction) getRestoreItemAction() (velero.RestoreIte // getDelegate restarts the plugin process (if needed) and returns the restore item action for this restartableRestoreItemAction. func (r *restartableRestoreItemAction) getDelegate() (velero.RestoreItemAction, error) { - if err := r.sharedPluginProcess.resetIfNeeded(); err != nil { + if err := r.sharedPluginProcess.ResetIfNeeded(); err != nil { return nil, err } diff --git a/pkg/plugin/clientmgmt/restartable_restore_item_action_test.go b/pkg/plugin/clientmgmt/restartable_restore_item_action_test.go index 8a37cdb4a..3d4fd52e9 100644 --- a/pkg/plugin/clientmgmt/restartable_restore_item_action_test.go +++ b/pkg/plugin/clientmgmt/restartable_restore_item_action_test.go @@ -25,6 +25,7 @@ import ( "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" 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" "github.com/vmware-tanzu/velero/pkg/plugin/velero" "github.com/vmware-tanzu/velero/pkg/restore/mocks" @@ -59,10 +60,10 @@ func TestRestartableGetRestoreItemAction(t *testing.T) { defer p.AssertExpectations(t) name := "pod" - key := kindAndName{kind: framework.PluginKindRestoreItemAction, name: name} - p.On("getByKindAndName", key).Return(tc.plugin, tc.getError) + key := process.KindAndName{Kind: framework.PluginKindRestoreItemAction, Name: name} + p.On("GetByKindAndName", key).Return(tc.plugin, tc.getError) - r := newRestartableRestoreItemAction(name, p) + r := NewRestartableRestoreItemAction(name, p) a, err := r.getRestoreItemAction() if tc.expectedError != "" { assert.EqualError(t, err, tc.expectedError) @@ -80,18 +81,18 @@ func TestRestartableRestoreItemActionGetDelegate(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 := newRestartableRestoreItemAction(name, p) + r := NewRestartableRestoreItemAction(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.PluginKindRestoreItemAction, name: name} - p.On("getByKindAndName", key).Return(expected, nil) + key := process.KindAndName{Kind: framework.PluginKindRestoreItemAction, Name: name} + p.On("GetByKindAndName", key).Return(expected, nil) a, err = r.getDelegate() assert.NoError(t, err) @@ -122,7 +123,7 @@ func TestRestartableRestoreItemActionDelegatedFunctions(t *testing.T) { runRestartableDelegateTests( t, framework.PluginKindRestoreItemAction, - func(key kindAndName, p RestartableProcess) interface{} { + func(key process.KindAndName, p process.RestartableProcess) interface{} { return &restartableRestoreItemAction{ key: key, sharedPluginProcess: p, diff --git a/pkg/plugin/clientmgmt/restartable_volume_snapshotter.go b/pkg/plugin/clientmgmt/restartable_volume_snapshotter.go index 5e46bf229..f8a1a66b0 100644 --- a/pkg/plugin/clientmgmt/restartable_volume_snapshotter.go +++ b/pkg/plugin/clientmgmt/restartable_volume_snapshotter.go @@ -20,36 +20,37 @@ import ( "github.com/pkg/errors" "k8s.io/apimachinery/pkg/runtime" + "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" ) -// restartableVolumeSnapshotter is a volume snapshotter for a given implementation (such as "aws"). It is associated with +// RestartableVolumeSnapshotter is a volume snapshotter for a given implementation (such as "aws"). It is associated with // a restartableProcess, which may be shared and used to run multiple plugins. At the beginning of each method // call, the restartableVolumeSnapshotter 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 restartableVolumeSnapshotter struct { - key kindAndName - sharedPluginProcess RestartableProcess + key process.KindAndName + sharedPluginProcess process.RestartableProcess config map[string]string } -// newRestartableVolumeSnapshotter returns a new restartableVolumeSnapshotter. -func newRestartableVolumeSnapshotter(name string, sharedPluginProcess RestartableProcess) *restartableVolumeSnapshotter { - key := kindAndName{kind: framework.PluginKindVolumeSnapshotter, name: name} +// NewRestartableVolumeSnapshotter returns a new RestartableVolumeSnapshotter. +func NewRestartableVolumeSnapshotter(name string, sharedPluginProcess process.RestartableProcess) *restartableVolumeSnapshotter { + key := process.KindAndName{Kind: framework.PluginKindVolumeSnapshotter, Name: name} r := &restartableVolumeSnapshotter{ key: key, sharedPluginProcess: sharedPluginProcess, } // Register our reinitializer so we can reinitialize after a restart with r.config. - sharedPluginProcess.addReinitializer(key, r) + sharedPluginProcess.AddReinitializer(key, r) return r } // reinitialize reinitializes a re-dispensed plugin using the initial data passed to Init(). -func (r *restartableVolumeSnapshotter) reinitialize(dispensed interface{}) error { +func (r *restartableVolumeSnapshotter) Reinitialize(dispensed interface{}) error { volumeSnapshotter, ok := dispensed.(velero.VolumeSnapshotter) if !ok { return errors.Errorf("%T is not a VolumeSnapshotter!", dispensed) @@ -60,7 +61,7 @@ func (r *restartableVolumeSnapshotter) reinitialize(dispensed interface{}) error // getVolumeSnapshotter returns the volume snapshotter for this restartableVolumeSnapshotter. It does *not* restart the // plugin process. func (r *restartableVolumeSnapshotter) getVolumeSnapshotter() (velero.VolumeSnapshotter, error) { - plugin, err := r.sharedPluginProcess.getByKindAndName(r.key) + plugin, err := r.sharedPluginProcess.GetByKindAndName(r.key) if err != nil { return nil, err } @@ -75,7 +76,7 @@ func (r *restartableVolumeSnapshotter) getVolumeSnapshotter() (velero.VolumeSnap // getDelegate restarts the plugin process (if needed) and returns the volume snapshotter for this restartableVolumeSnapshotter. func (r *restartableVolumeSnapshotter) getDelegate() (velero.VolumeSnapshotter, error) { - if err := r.sharedPluginProcess.resetIfNeeded(); err != nil { + if err := r.sharedPluginProcess.ResetIfNeeded(); err != nil { return nil, err } diff --git a/pkg/plugin/clientmgmt/restartable_volume_snapshotter_test.go b/pkg/plugin/clientmgmt/restartable_volume_snapshotter_test.go index 762153e05..8d198ec96 100644 --- a/pkg/plugin/clientmgmt/restartable_volume_snapshotter_test.go +++ b/pkg/plugin/clientmgmt/restartable_volume_snapshotter_test.go @@ -25,6 +25,7 @@ import ( "github.com/stretchr/testify/require" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "github.com/vmware-tanzu/velero/pkg/plugin/clientmgmt/process" "github.com/vmware-tanzu/velero/pkg/plugin/framework" providermocks "github.com/vmware-tanzu/velero/pkg/plugin/velero/mocks" ) @@ -59,8 +60,8 @@ func TestRestartableGetVolumeSnapshotter(t *testing.T) { defer p.AssertExpectations(t) name := "aws" - key := kindAndName{kind: framework.PluginKindVolumeSnapshotter, name: name} - p.On("getByKindAndName", key).Return(tc.plugin, tc.getError) + key := process.KindAndName{Kind: framework.PluginKindVolumeSnapshotter, Name: name} + p.On("GetByKindAndName", key).Return(tc.plugin, tc.getError) r := &restartableVolumeSnapshotter{ key: key, @@ -84,7 +85,7 @@ func TestRestartableVolumeSnapshotterReinitialize(t *testing.T) { defer p.AssertExpectations(t) name := "aws" - key := kindAndName{kind: framework.PluginKindVolumeSnapshotter, name: name} + key := process.KindAndName{Kind: framework.PluginKindVolumeSnapshotter, Name: name} r := &restartableVolumeSnapshotter{ key: key, sharedPluginProcess: p, @@ -93,7 +94,7 @@ func TestRestartableVolumeSnapshotterReinitialize(t *testing.T) { }, } - err := r.reinitialize(3) + err := r.Reinitialize(3) assert.EqualError(t, err, "int is not a VolumeSnapshotter!") volumeSnapshotter := new(providermocks.VolumeSnapshotter) @@ -101,11 +102,11 @@ func TestRestartableVolumeSnapshotterReinitialize(t *testing.T) { defer volumeSnapshotter.AssertExpectations(t) volumeSnapshotter.On("Init", r.config).Return(errors.Errorf("init error")).Once() - err = r.reinitialize(volumeSnapshotter) + err = r.Reinitialize(volumeSnapshotter) assert.EqualError(t, err, "init error") volumeSnapshotter.On("Init", r.config).Return(nil) - err = r.reinitialize(volumeSnapshotter) + err = r.Reinitialize(volumeSnapshotter) assert.NoError(t, err) } @@ -115,9 +116,9 @@ func TestRestartableVolumeSnapshotterGetDelegate(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 := "aws" - key := kindAndName{kind: framework.PluginKindVolumeSnapshotter, name: name} + key := process.KindAndName{Kind: framework.PluginKindVolumeSnapshotter, Name: name} r := &restartableVolumeSnapshotter{ key: key, sharedPluginProcess: p, @@ -127,11 +128,11 @@ func TestRestartableVolumeSnapshotterGetDelegate(t *testing.T) { assert.EqualError(t, err, "reset error") // Happy path - p.On("resetIfNeeded").Return(nil) + p.On("ResetIfNeeded").Return(nil) volumeSnapshotter := new(providermocks.VolumeSnapshotter) volumeSnapshotter.Test(t) defer volumeSnapshotter.AssertExpectations(t) - p.On("getByKindAndName", key).Return(volumeSnapshotter, nil) + p.On("GetByKindAndName", key).Return(volumeSnapshotter, nil) a, err = r.getDelegate() assert.NoError(t, err) @@ -145,24 +146,24 @@ func TestRestartableVolumeSnapshotterInit(t *testing.T) { // getVolumeSnapshottererror name := "aws" - key := kindAndName{kind: framework.PluginKindVolumeSnapshotter, name: name} + key := process.KindAndName{Kind: framework.PluginKindVolumeSnapshotter, Name: name} r := &restartableVolumeSnapshotter{ key: key, sharedPluginProcess: p, } - p.On("getByKindAndName", key).Return(nil, errors.Errorf("getByKindAndName error")).Once() + p.On("GetByKindAndName", key).Return(nil, errors.Errorf("GetByKindAndName error")).Once() config := map[string]string{ "color": "blue", } err := r.Init(config) - assert.EqualError(t, err, "getByKindAndName error") + assert.EqualError(t, err, "GetByKindAndName error") // Delegate returns error volumeSnapshotter := new(providermocks.VolumeSnapshotter) volumeSnapshotter.Test(t) defer volumeSnapshotter.AssertExpectations(t) - p.On("getByKindAndName", key).Return(volumeSnapshotter, nil) + p.On("GetByKindAndName", key).Return(volumeSnapshotter, nil) volumeSnapshotter.On("Init", config).Return(errors.Errorf("Init error")).Once() err = r.Init(config) @@ -198,7 +199,7 @@ func TestRestartableVolumeSnapshotterDelegatedFunctions(t *testing.T) { runRestartableDelegateTests( t, framework.PluginKindVolumeSnapshotter, - func(key kindAndName, p RestartableProcess) interface{} { + func(key process.KindAndName, p process.RestartableProcess) interface{} { return &restartableVolumeSnapshotter{ key: key, sharedPluginProcess: p, diff --git a/pkg/plugin/mocks/process_factory.go b/pkg/plugin/mocks/process_factory.go index 22abc81fc..586e31121 100644 --- a/pkg/plugin/mocks/process_factory.go +++ b/pkg/plugin/mocks/process_factory.go @@ -1,5 +1,5 @@ /* -Copyright 2018 the Velero contributors. +Copyright 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. @@ -14,11 +14,15 @@ See the License for the specific language governing permissions and limitations under the License. */ // Code generated by mockery v1.0.0. DO NOT EDIT. + package mocks -import logrus "github.com/sirupsen/logrus" -import mock "github.com/stretchr/testify/mock" -import "github.com/vmware-tanzu/velero/pkg/plugin/clientmgmt" +import ( + logrus "github.com/sirupsen/logrus" + mock "github.com/stretchr/testify/mock" + + process "github.com/vmware-tanzu/velero/pkg/plugin/clientmgmt/process" +) // ProcessFactory is an autogenerated mock type for the ProcessFactory type type ProcessFactory struct { @@ -26,15 +30,15 @@ type ProcessFactory struct { } // newProcess provides a mock function with given fields: command, logger, logLevel -func (_m *ProcessFactory) newProcess(command string, logger logrus.FieldLogger, logLevel logrus.Level) (clientmgmt.Process, error) { +func (_m *ProcessFactory) newProcess(command string, logger logrus.FieldLogger, logLevel logrus.Level) (process.Process, error) { ret := _m.Called(command, logger, logLevel) - var r0 clientmgmt.Process - if rf, ok := ret.Get(0).(func(string, logrus.FieldLogger, logrus.Level) clientmgmt.Process); ok { + var r0 process.Process + if rf, ok := ret.Get(0).(func(string, logrus.FieldLogger, logrus.Level) process.Process); ok { r0 = rf(command, logger, logLevel) } else { if ret.Get(0) != nil { - r0 = ret.Get(0).(clientmgmt.Process) + r0 = ret.Get(0).(process.Process) } }