From defb8aa856ec72996efbd15b64d7350c4c5b6908 Mon Sep 17 00:00:00 2001 From: Shubheksha Jalan Date: Wed, 31 Oct 2018 00:44:40 +0530 Subject: [PATCH] remove code that checks directly for a backup from restore controller Signed-off-by: Shubheksha Jalan --- pkg/controller/backup_sync_controller.go | 21 +++++++++ pkg/controller/restore_controller.go | 55 +---------------------- pkg/controller/restore_controller_test.go | 2 +- 3 files changed, 24 insertions(+), 54 deletions(-) diff --git a/pkg/controller/backup_sync_controller.go b/pkg/controller/backup_sync_controller.go index f9727acb4..4f1ceadae 100644 --- a/pkg/controller/backup_sync_controller.go +++ b/pkg/controller/backup_sync_controller.go @@ -121,6 +121,27 @@ func shouldSync(location *arkv1api.BackupStorageLocation, now time.Time, backupS return false, "" } +// orderedBackupLocations returns a new slice with the default backup location first (if it exists), +// followed by the rest of the locations in no particular order. +func orderedBackupLocations(locations []*arkv1api.BackupStorageLocation, defaultLocationName string) []*arkv1api.BackupStorageLocation { + var result []*arkv1api.BackupStorageLocation + + for i := range locations { + if locations[i].Name == defaultLocationName { + // put the default location first + result = append(result, locations[i]) + // append everything before the default + result = append(result, locations[:i]...) + // append everything after the default + result = append(result, locations[i+1:]...) + + return result + } + } + + return locations +} + func (c *backupSyncController) run() { c.logger.Info("Checking for backup storage locations to sync into cluster") diff --git a/pkg/controller/restore_controller.go b/pkg/controller/restore_controller.go index fdb1a4aa5..b7826c432 100644 --- a/pkg/controller/restore_controller.go +++ b/pkg/controller/restore_controller.go @@ -28,7 +28,6 @@ import ( jsonpatch "github.com/evanphx/json-patch" "github.com/pkg/errors" "github.com/sirupsen/logrus" - apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/sets" @@ -397,17 +396,11 @@ func mostRecentCompletedBackup(backups []*api.Backup) *api.Backup { } // fetchBackupInfo checks the backup lister for a backup that matches the given name. If it doesn't -// find it, it tries to retrieve it from one of the backup storage locations. +// find it, it returns an error. func (c *restoreController) fetchBackupInfo(backupName string, pluginManager plugin.Manager) (backupInfo, error) { backup, err := c.backupLister.Backups(c.namespace).Get(backupName) if err != nil { - if !apierrors.IsNotFound(err) { - return backupInfo{}, errors.WithStack(err) - } - - log := c.logger.WithField("backupName", backupName) - log.Debug("Backup not found in backupLister, checking each backup location directly, starting with default...") - return c.fetchFromBackupStorage(backupName, pluginManager) + return backupInfo{}, err } location, err := c.backupLocationLister.BackupStorageLocations(c.namespace).Get(backup.Spec.StorageLocation) @@ -426,50 +419,6 @@ func (c *restoreController) fetchBackupInfo(backupName string, pluginManager plu }, nil } -// fetchFromBackupStorage checks each backup storage location, starting with the default, -// looking for a backup that matches the given backup name. -func (c *restoreController) fetchFromBackupStorage(backupName string, pluginManager plugin.Manager) (backupInfo, error) { - locations, err := c.backupLocationLister.BackupStorageLocations(c.namespace).List(labels.Everything()) - if err != nil { - return backupInfo{}, errors.WithStack(err) - } - - orderedLocations := orderedBackupLocations(locations, c.defaultBackupLocation) - - log := c.logger.WithField("backupName", backupName) - for _, location := range orderedLocations { - info, err := c.backupInfoForLocation(location, backupName, pluginManager) - if err != nil { - log.WithField("locationName", location.Name).WithError(err).Error("Unable to fetch backup from object storage location") - continue - } - return info, nil - } - - return backupInfo{}, errors.New("not able to fetch from backup storage") -} - -// orderedBackupLocations returns a new slice with the default backup location first (if it exists), -// followed by the rest of the locations in no particular order. -func orderedBackupLocations(locations []*api.BackupStorageLocation, defaultLocationName string) []*api.BackupStorageLocation { - var result []*api.BackupStorageLocation - - for i := range locations { - if locations[i].Name == defaultLocationName { - // put the default location first - result = append(result, locations[i]) - // append everything before the default - result = append(result, locations[:i]...) - // append everything after the default - result = append(result, locations[i+1:]...) - - return result - } - } - - return locations -} - func (c *restoreController) backupInfoForLocation(location *api.BackupStorageLocation, backupName string, pluginManager plugin.Manager) (backupInfo, error) { backupStore, err := persistence.NewObjectBackupStore(location, pluginManager, c.logger) if err != nil { diff --git a/pkg/controller/restore_controller_test.go b/pkg/controller/restore_controller_test.go index 06fcb2e60..606c34123 100644 --- a/pkg/controller/restore_controller_test.go +++ b/pkg/controller/restore_controller_test.go @@ -287,7 +287,7 @@ func TestProcessRestore(t *testing.T) { restore: NewRestore("foo", "bar", "backup-1", "ns-1", "*", api.RestorePhaseNew).Restore, expectedErr: false, expectedPhase: string(api.RestorePhaseFailedValidation), - expectedValidationErrors: []string{"Error retrieving backup: not able to fetch from backup storage"}, + expectedValidationErrors: []string{"Error retrieving backup: backup.ark.heptio.com \"backup-1\" not found"}, backupStoreGetBackupMetadataErr: errors.New("no backup here"), }, {