From bb7b0a3c33853e2e016e7c6b63ee56b075e30946 Mon Sep 17 00:00:00 2001 From: Nolan Brubaker Date: Thu, 4 Jan 2018 11:05:09 -0500 Subject: [PATCH] Invalidate restores based on missing backups When creating a restore based on a backup that doesn't exist, the restore should be marked as invalid and the error clearly communicated so the user understands why the restore wasn't made. Previously, the restore was left as in progress with an error attached. Since restores are CRDs and must be updated via a controller, there's currently not a way to give the client immediate errors. Signed-off-by: Nolan Brubaker --- pkg/controller/restore_controller.go | 2 ++ pkg/controller/restore_controller_test.go | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/pkg/controller/restore_controller.go b/pkg/controller/restore_controller.go index 8524943fb..b8e1a4853 100644 --- a/pkg/controller/restore_controller.go +++ b/pkg/controller/restore_controller.go @@ -294,6 +294,8 @@ func (controller *restoreController) getValidationErrors(itm *api.Restore) []str if itm.Spec.BackupName == "" { validationErrors = append(validationErrors, "BackupName must be non-empty and correspond to the name of a backup in object storage.") + } else if _, err := controller.fetchBackup(controller.bucket, itm.Spec.BackupName); err != nil { + validationErrors = append(validationErrors, fmt.Sprintf("Error retrieving backup: %v", err)) } includedResources := sets.NewString(itm.Spec.IncludedResources...) diff --git a/pkg/controller/restore_controller_test.go b/pkg/controller/restore_controller_test.go index 40ff8c2bc..4c64a189c 100644 --- a/pkg/controller/restore_controller_test.go +++ b/pkg/controller/restore_controller_test.go @@ -182,8 +182,8 @@ func TestProcessRestore(t *testing.T) { name: "restore with non-existent backup name fails", restore: arktest.NewTestRestore("foo", "bar", api.RestorePhaseNew).WithBackup("backup-1").WithIncludedNamespace("ns-1").Restore, expectedErr: false, - expectedPhase: string(api.RestorePhaseInProgress), - expectedRestoreErrors: 1, + expectedPhase: string(api.RestorePhaseFailedValidation), + expectedValidationErrors: []string{"Error retrieving backup: no backup here"}, backupServiceGetBackupError: errors.New("no backup here"), }, {