diff --git a/pkg/cmd/server/server.go b/pkg/cmd/server/server.go index 60d6c96b8..73c07bb7c 100644 --- a/pkg/cmd/server/server.go +++ b/pkg/cmd/server/server.go @@ -565,6 +565,10 @@ func (s *server) runControllers(config *api.Config, defaultBackupLocation *api.B s.metrics = metrics.NewServerMetrics() s.metrics.RegisterAllMetrics() + newPluginManager := func(logger logrus.FieldLogger) plugin.Manager { + return plugin.NewManager(logger, s.logLevel, s.pluginRegistry) + } + backupSyncController := controller.NewBackupSyncController( s.arkClient.ArkV1(), s.sharedInformerFactory.Ark().V1().Backups(), @@ -572,9 +576,8 @@ func (s *server) runControllers(config *api.Config, defaultBackupLocation *api.B s.config.backupSyncPeriod, s.namespace, s.config.defaultBackupLocation, - s.pluginRegistry, + newPluginManager, s.logger, - s.logLevel, ) wg.Add(1) go func() { @@ -604,7 +607,7 @@ func (s *server) runControllers(config *api.Config, defaultBackupLocation *api.B s.blockStore != nil, s.logger, s.logLevel, - s.pluginRegistry, + newPluginManager, backupTracker, s.sharedInformerFactory.Ark().V1().BackupStorageLocations(), s.config.defaultBackupLocation, @@ -644,7 +647,6 @@ func (s *server) runControllers(config *api.Config, defaultBackupLocation *api.B backupDeletionController := controller.NewBackupDeletionController( s.logger, - s.logLevel, s.sharedInformerFactory.Ark().V1().DeleteBackupRequests(), s.arkClient.ArkV1(), // deleteBackupRequestClient s.arkClient.ArkV1(), // backupClient @@ -655,7 +657,7 @@ func (s *server) runControllers(config *api.Config, defaultBackupLocation *api.B s.resticManager, s.sharedInformerFactory.Ark().V1().PodVolumeBackups(), s.sharedInformerFactory.Ark().V1().BackupStorageLocations(), - s.pluginRegistry, + newPluginManager, ) wg.Add(1) go func() { @@ -689,7 +691,7 @@ func (s *server) runControllers(config *api.Config, defaultBackupLocation *api.B s.blockStore != nil, s.logger, s.logLevel, - s.pluginRegistry, + newPluginManager, s.config.defaultBackupLocation, s.metrics, ) @@ -706,9 +708,8 @@ func (s *server) runControllers(config *api.Config, defaultBackupLocation *api.B s.sharedInformerFactory.Ark().V1().Restores(), s.sharedInformerFactory.Ark().V1().BackupStorageLocations(), s.sharedInformerFactory.Ark().V1().Backups(), - s.pluginRegistry, + newPluginManager, s.logger, - s.logLevel, ) wg.Add(1) go func() { diff --git a/pkg/controller/backup_controller.go b/pkg/controller/backup_controller.go index 18c487bd3..1ffd4a81a 100644 --- a/pkg/controller/backup_controller.go +++ b/pkg/controller/backup_controller.go @@ -67,14 +67,12 @@ type backupController struct { clock clock.Clock logger logrus.FieldLogger logLevel logrus.Level - pluginRegistry plugin.Registry + newPluginManager func(logrus.FieldLogger) plugin.Manager backupTracker BackupTracker backupLocationLister listers.BackupStorageLocationLister backupLocationListerSynced cache.InformerSynced defaultBackupLocation string metrics *metrics.ServerMetrics - - newPluginManager func(logger logrus.FieldLogger, logLevel logrus.Level, pluginRegistry plugin.Registry) plugin.Manager } func NewBackupController( @@ -84,7 +82,7 @@ func NewBackupController( pvProviderExists bool, logger logrus.FieldLogger, logLevel logrus.Level, - pluginRegistry plugin.Registry, + newPluginManager func(logrus.FieldLogger) plugin.Manager, backupTracker BackupTracker, backupLocationInformer informers.BackupStorageLocationInformer, defaultBackupLocation string, @@ -100,16 +98,12 @@ func NewBackupController( clock: &clock.RealClock{}, logger: logger, logLevel: logLevel, - pluginRegistry: pluginRegistry, + newPluginManager: newPluginManager, backupTracker: backupTracker, backupLocationLister: backupLocationInformer.Lister(), backupLocationListerSynced: backupLocationInformer.Informer().HasSynced, defaultBackupLocation: defaultBackupLocation, metrics: metrics, - - newPluginManager: func(logger logrus.FieldLogger, logLevel logrus.Level, pluginRegistry plugin.Registry) plugin.Manager { - return plugin.NewManager(logger, logLevel, pluginRegistry) - }, } c.syncHandler = c.processBackup @@ -387,7 +381,7 @@ func (controller *backupController) runBackup(backup *api.Backup, backupLocation log.Info("Starting backup") - pluginManager := controller.newPluginManager(log, log.Level, controller.pluginRegistry) + pluginManager := controller.newPluginManager(log) defer pluginManager.CleanupClients() backupFile, err := ioutil.TempFile("", "") diff --git a/pkg/controller/backup_controller_test.go b/pkg/controller/backup_controller_test.go index 9a41d6d29..97c6fd63a 100644 --- a/pkg/controller/backup_controller_test.go +++ b/pkg/controller/backup_controller_test.go @@ -178,7 +178,6 @@ func TestProcessBackup(t *testing.T) { backupper = &fakeBackupper{} sharedInformers = informers.NewSharedInformerFactory(client, 0) logger = logging.DefaultLogger(logrus.DebugLevel) - pluginRegistry = plugin.NewRegistry("/dir", logger, logrus.InfoLevel) clockTime, _ = time.Parse("Mon Jan 2 15:04:05 2006", "Mon Jan 2 15:04:05 2006") objectStore = &arktest.ObjectStore{} pluginManager = &pluginmocks.Manager{} @@ -194,7 +193,7 @@ func TestProcessBackup(t *testing.T) { test.allowSnapshots, logger, logrus.InfoLevel, - pluginRegistry, + func(logrus.FieldLogger) plugin.Manager { return pluginManager }, NewBackupTracker(), sharedInformers.Ark().V1().BackupStorageLocations(), "default", @@ -202,9 +201,6 @@ func TestProcessBackup(t *testing.T) { ).(*backupController) c.clock = clock.NewFakeClock(clockTime) - c.newPluginManager = func(logger logrus.FieldLogger, logLevel logrus.Level, pluginRegistry plugin.Registry) plugin.Manager { - return pluginManager - } var expiration, startTime time.Time diff --git a/pkg/controller/backup_deletion_controller.go b/pkg/controller/backup_deletion_controller.go index b4b33c6b3..3fd77d4c2 100644 --- a/pkg/controller/backup_deletion_controller.go +++ b/pkg/controller/backup_deletion_controller.go @@ -66,7 +66,6 @@ type backupDeletionController struct { // NewBackupDeletionController creates a new backup deletion controller. func NewBackupDeletionController( logger logrus.FieldLogger, - logLevel logrus.Level, deleteBackupRequestInformer informers.DeleteBackupRequestInformer, deleteBackupRequestClient arkv1client.DeleteBackupRequestsGetter, backupClient arkv1client.BackupsGetter, @@ -77,7 +76,7 @@ func NewBackupDeletionController( resticMgr restic.RepositoryManager, podvolumeBackupInformer informers.PodVolumeBackupInformer, backupLocationInformer informers.BackupStorageLocationInformer, - pluginRegistry plugin.Registry, + newPluginManager func(logrus.FieldLogger) plugin.Manager, ) Interface { c := &backupDeletionController{ genericController: newGenericController("backup-deletion", logger), @@ -94,10 +93,8 @@ func NewBackupDeletionController( // use variables to refer to these functions so they can be // replaced with fakes for testing. - deleteBackupDir: cloudprovider.DeleteBackupDir, - newPluginManager: func(logger logrus.FieldLogger) plugin.Manager { - return plugin.NewManager(logger, logLevel, pluginRegistry) - }, + newPluginManager: newPluginManager, + deleteBackupDir: cloudprovider.DeleteBackupDir, clock: &clock.RealClock{}, } diff --git a/pkg/controller/backup_deletion_controller_test.go b/pkg/controller/backup_deletion_controller_test.go index 5bd39e890..1a1b6df77 100644 --- a/pkg/controller/backup_deletion_controller_test.go +++ b/pkg/controller/backup_deletion_controller_test.go @@ -48,7 +48,6 @@ func TestBackupDeletionControllerProcessQueueItem(t *testing.T) { controller := NewBackupDeletionController( arktest.NewLogger(), - logrus.InfoLevel, sharedInformers.Ark().V1().DeleteBackupRequests(), client.ArkV1(), // deleteBackupRequestClient client.ArkV1(), // backupClient @@ -59,7 +58,7 @@ func TestBackupDeletionControllerProcessQueueItem(t *testing.T) { nil, // restic repository manager sharedInformers.Ark().V1().PodVolumeBackups(), sharedInformers.Ark().V1().BackupStorageLocations(), - nil, // pluginRegistry + nil, // new plugin manager func ).(*backupDeletionController) // Error splitting key @@ -135,7 +134,6 @@ func setupBackupDeletionControllerTest(objects ...runtime.Object) *backupDeletio objectStore: objectStore, controller: NewBackupDeletionController( arktest.NewLogger(), - logrus.InfoLevel, sharedInformers.Ark().V1().DeleteBackupRequests(), client.ArkV1(), // deleteBackupRequestClient client.ArkV1(), // backupClient @@ -146,14 +144,12 @@ func setupBackupDeletionControllerTest(objects ...runtime.Object) *backupDeletio nil, // restic repository manager sharedInformers.Ark().V1().PodVolumeBackups(), sharedInformers.Ark().V1().BackupStorageLocations(), - nil, // pluginRegistry + func(logrus.FieldLogger) plugin.Manager { return pluginManager }, ).(*backupDeletionController), req: req, } - data.controller.newPluginManager = func(_ logrus.FieldLogger) plugin.Manager { return pluginManager } - pluginManager.On("GetObjectStore", "objStoreProvider").Return(objectStore, nil) pluginManager.On("CleanupClients").Return(nil) @@ -594,7 +590,6 @@ func TestBackupDeletionControllerDeleteExpiredRequests(t *testing.T) { controller := NewBackupDeletionController( arktest.NewLogger(), - logrus.InfoLevel, sharedInformers.Ark().V1().DeleteBackupRequests(), client.ArkV1(), // deleteBackupRequestClient client.ArkV1(), // backupClient @@ -605,7 +600,7 @@ func TestBackupDeletionControllerDeleteExpiredRequests(t *testing.T) { nil, sharedInformers.Ark().V1().PodVolumeBackups(), sharedInformers.Ark().V1().BackupStorageLocations(), - nil, // pluginRegistry + nil, // new plugin manager func ).(*backupDeletionController) fakeClock := &clock.FakeClock{} diff --git a/pkg/controller/backup_sync_controller.go b/pkg/controller/backup_sync_controller.go index a19d7aea0..38904051d 100644 --- a/pkg/controller/backup_sync_controller.go +++ b/pkg/controller/backup_sync_controller.go @@ -57,9 +57,8 @@ func NewBackupSyncController( syncPeriod time.Duration, namespace string, defaultBackupLocation string, - pluginRegistry plugin.Registry, + newPluginManager func(logrus.FieldLogger) plugin.Manager, logger logrus.FieldLogger, - logLevel logrus.Level, ) Interface { if syncPeriod < time.Minute { logger.Infof("Provided backup sync period %v is too short. Setting to 1 minute", syncPeriod) @@ -74,9 +73,9 @@ func NewBackupSyncController( backupLister: backupInformer.Lister(), backupStorageLocationLister: backupStorageLocationInformer.Lister(), - newPluginManager: func(logger logrus.FieldLogger) plugin.Manager { - return plugin.NewManager(logger, logLevel, pluginRegistry) - }, + // use variables to refer to these functions so they can be + // replaced with fakes for testing. + newPluginManager: newPluginManager, listCloudBackups: cloudprovider.ListBackups, } diff --git a/pkg/controller/backup_sync_controller_test.go b/pkg/controller/backup_sync_controller_test.go index 4badb0368..a69d31c4b 100644 --- a/pkg/controller/backup_sync_controller_test.go +++ b/pkg/controller/backup_sync_controller_test.go @@ -177,14 +177,13 @@ func TestBackupSyncControllerRun(t *testing.T) { time.Duration(0), test.namespace, "", - nil, // pluginRegistry + func(logrus.FieldLogger) plugin.Manager { return pluginManager }, arktest.NewLogger(), - logrus.DebugLevel, ).(*backupSyncController) - c.newPluginManager = func(_ logrus.FieldLogger) plugin.Manager { return pluginManager } pluginManager.On("GetObjectStore", "objStoreProvider").Return(objectStore, nil) pluginManager.On("CleanupClients").Return(nil) + objectStore.On("Init", mock.Anything).Return(nil) for _, location := range test.locations { @@ -343,9 +342,8 @@ func TestDeleteOrphanedBackups(t *testing.T) { time.Duration(0), test.namespace, "", - nil, // pluginRegistry + nil, // new plugin manager func arktest.NewLogger(), - logrus.InfoLevel, ).(*backupSyncController) expectedDeleteActions := make([]core.Action, 0) diff --git a/pkg/controller/download_request_controller.go b/pkg/controller/download_request_controller.go index d06dbb6dc..7a4c025f3 100644 --- a/pkg/controller/download_request_controller.go +++ b/pkg/controller/download_request_controller.go @@ -60,9 +60,8 @@ func NewDownloadRequestController( restoreInformer informers.RestoreInformer, backupLocationInformer informers.BackupStorageLocationInformer, backupInformer informers.BackupInformer, - pluginRegistry plugin.Registry, + newPluginManager func(logrus.FieldLogger) plugin.Manager, logger logrus.FieldLogger, - logLevel logrus.Level, ) Interface { c := &downloadRequestController{ genericController: newGenericController("downloadrequest", logger), @@ -74,10 +73,8 @@ func NewDownloadRequestController( // use variables to refer to these functions so they can be // replaced with fakes for testing. - createSignedURL: cloudprovider.CreateSignedURL, - newPluginManager: func(logger logrus.FieldLogger) plugin.Manager { - return plugin.NewManager(logger, logLevel, pluginRegistry) - }, + createSignedURL: cloudprovider.CreateSignedURL, + newPluginManager: newPluginManager, clock: &clock.RealClock{}, } diff --git a/pkg/controller/download_request_controller_test.go b/pkg/controller/download_request_controller_test.go index 645fccbba..7098d9f11 100644 --- a/pkg/controller/download_request_controller_test.go +++ b/pkg/controller/download_request_controller_test.go @@ -59,9 +59,8 @@ func newDownloadRequestTestHarness(t *testing.T) *downloadRequestTestHarness { informerFactory.Ark().V1().Restores(), informerFactory.Ark().V1().BackupStorageLocations(), informerFactory.Ark().V1().Backups(), - nil, + func(logrus.FieldLogger) plugin.Manager { return pluginManager }, arktest.NewLogger(), - logrus.InfoLevel, ).(*downloadRequestController) ) @@ -70,8 +69,6 @@ func newDownloadRequestTestHarness(t *testing.T) *downloadRequestTestHarness { controller.clock = clock.NewFakeClock(clockTime) - controller.newPluginManager = func(_ logrus.FieldLogger) plugin.Manager { return pluginManager } - pluginManager.On("CleanupClients").Return() objectStore.On("Init", mock.Anything).Return(nil) diff --git a/pkg/controller/restore_controller.go b/pkg/controller/restore_controller.go index 215c51405..fead34d3a 100644 --- a/pkg/controller/restore_controller.go +++ b/pkg/controller/restore_controller.go @@ -86,7 +86,6 @@ type restoreController struct { queue workqueue.RateLimitingInterface logger logrus.FieldLogger logLevel logrus.Level - pluginRegistry plugin.Registry defaultBackupLocation string metrics *metrics.ServerMetrics @@ -94,7 +93,7 @@ type restoreController struct { downloadBackup cloudprovider.DownloadBackupFunc uploadRestoreLog cloudprovider.UploadRestoreLogFunc uploadRestoreResults cloudprovider.UploadRestoreResultsFunc - newPluginManager func(logger logrus.FieldLogger, logLevel logrus.Level, pluginRegistry plugin.Registry) plugin.Manager + newPluginManager func(logger logrus.FieldLogger) plugin.Manager } func NewRestoreController( @@ -108,10 +107,9 @@ func NewRestoreController( pvProviderExists bool, logger logrus.FieldLogger, logLevel logrus.Level, - pluginRegistry plugin.Registry, + newPluginManager func(logrus.FieldLogger) plugin.Manager, defaultBackupLocation string, metrics *metrics.ServerMetrics, - ) Interface { c := &restoreController{ namespace: namespace, @@ -128,17 +126,16 @@ func NewRestoreController( queue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "restore"), logger: logger, logLevel: logLevel, - pluginRegistry: pluginRegistry, defaultBackupLocation: defaultBackupLocation, metrics: metrics, + // use variables to refer to these functions so they can be + // replaced with fakes for testing. + newPluginManager: newPluginManager, getBackup: cloudprovider.GetBackup, downloadBackup: cloudprovider.DownloadBackup, uploadRestoreLog: cloudprovider.UploadRestoreLog, uploadRestoreResults: cloudprovider.UploadRestoreResults, - newPluginManager: func(logger logrus.FieldLogger, logLevel logrus.Level, pluginRegistry plugin.Registry) plugin.Manager { - return plugin.NewManager(logger, logLevel, pluginRegistry) - }, } c.syncHandler = c.processRestore @@ -282,7 +279,7 @@ func (c *restoreController) processRestore(key string) error { // don't modify items in the cache restore = restore.DeepCopy() - pluginManager := c.newPluginManager(logContext, logContext.Level, c.pluginRegistry) + pluginManager := c.newPluginManager(logContext) defer pluginManager.CleanupClients() actions, err := pluginManager.GetRestoreItemActions() diff --git a/pkg/controller/restore_controller_test.go b/pkg/controller/restore_controller_test.go index b67d423e2..407a30739 100644 --- a/pkg/controller/restore_controller_test.go +++ b/pkg/controller/restore_controller_test.go @@ -105,13 +105,10 @@ func TestFetchBackupInfo(t *testing.T) { false, logger, logrus.InfoLevel, - nil, //pluginRegistry + func(logrus.FieldLogger) plugin.Manager { return pluginManager }, "default", metrics.NewServerMetrics(), ).(*restoreController) - c.newPluginManager = func(logger logrus.FieldLogger, logLevel logrus.Level, pluginRegistry plugin.Registry) plugin.Manager { - return pluginManager - } if test.backupServiceError == nil { pluginManager.On("GetObjectStore", "myCloud").Return(objectStore, nil) @@ -200,13 +197,10 @@ func TestProcessRestoreSkips(t *testing.T) { false, // pvProviderExists logger, logrus.InfoLevel, - nil, // pluginRegistry + func(logrus.FieldLogger) plugin.Manager { return pluginManager }, "default", metrics.NewServerMetrics(), ).(*restoreController) - c.newPluginManager = func(logger logrus.FieldLogger, logLevel logrus.Level, pluginRegistry plugin.Registry) plugin.Manager { - return pluginManager - } if test.restore != nil { sharedInformers.Ark().V1().Restores().Informer().GetStore().Add(test.restore) @@ -427,13 +421,10 @@ func TestProcessRestore(t *testing.T) { test.allowRestoreSnapshots, logger, logrus.InfoLevel, - nil, // pluginRegistry + func(logrus.FieldLogger) plugin.Manager { return pluginManager }, "default", metrics.NewServerMetrics(), ).(*restoreController) - c.newPluginManager = func(logger logrus.FieldLogger, logLevel logrus.Level, pluginRegistry plugin.Registry) plugin.Manager { - return pluginManager - } if test.location != nil { sharedInformers.Ark().V1().BackupStorageLocations().Informer().GetStore().Add(test.location)