Merge pull request #7377 from allenxu404/restore-finalization-implementation

Add the finalization phase to the restore workflow

changelogs/unreleased/7377-allenxu404

@@ -0,0 +1 @@
+Add the finalization phase to the restore workflow

config/crd/v1/bases/velero.io_restores.yaml

@@ -477,6 +477,8 @@ spec:
                 - Completed
                 - PartiallyFailed
                 - Failed
+                - Finalizing
+                - FinalizingPartiallyFailed
                 type: string
               progress:
                 description: Progress contains information about the restore's execution

pkg/apis/velero/v1/restore_types.go

@@ -249,7 +249,7 @@ type InitRestoreHook struct {
 
 // RestorePhase is a string representation of the lifecycle phase
 // of a Velero restore
-// +kubebuilder:validation:Enum=New;FailedValidation;InProgress;WaitingForPluginOperations;WaitingForPluginOperationsPartiallyFailed;Completed;PartiallyFailed;Failed
+// +kubebuilder:validation:Enum=New;FailedValidation;InProgress;WaitingForPluginOperations;WaitingForPluginOperationsPartiallyFailed;Completed;PartiallyFailed;Failed;Finalizing;FinalizingPartiallyFailed
 type RestorePhase string
 
 const (
@@ -277,6 +277,19 @@ const (
 	// ongoing.  The restore is not complete yet.
 	RestorePhaseWaitingForPluginOperationsPartiallyFailed RestorePhase = "WaitingForPluginOperationsPartiallyFailed"
 
+	// RestorePhaseFinalizing means the restore of
+	// Kubernetes resources and other async plugin operations were successful and
+	// other plugin operations are now complete, but the restore is awaiting
+	// the completion of wrap-up tasks before the restore process enters terminal phase.
+	RestorePhaseFinalizing RestorePhase = "Finalizing"
+
+	// RestorePhaseFinalizingPartiallyFailed means the restore of
+	// Kubernetes resources and other async plugin operations were successful and
+	// other plugin operations are now complete, but one or more errors
+	// occurred during restore or async operation processing. The restore is awaiting
+	// the completion of wrap-up tasks before the restore process enters terminal phase.
+	RestorePhaseFinalizingPartiallyFailed RestorePhase = "FinalizingPartiallyFailed"
+
 	// RestorePhaseCompleted means the restore has run successfully
 	// without errors.
 	RestorePhaseCompleted RestorePhase = "Completed"

pkg/cmd/server/server.go

@@ -694,6 +694,7 @@ func (s *server) runControllers(defaultVolumeSnapshotLocations map[string]string
 		controller.RestoreOperations:   {},
 		controller.Schedule:            {},
 		controller.ServerStatusRequest: {},
+		controller.RestoreFinalizer:    {},
 	}
 
 	if s.config.restoreOnly {
@@ -983,6 +984,19 @@ func (s *server) runControllers(defaultVolumeSnapshotLocations map[string]string
 		}
 	}
 
+	if _, ok := enabledRuntimeControllers[controller.RestoreFinalizer]; ok {
+		if err := controller.NewRestoreFinalizerReconciler(
+			s.logger,
+			s.namespace,
+			s.mgr.GetClient(),
+			newPluginManager,
+			backupStoreGetter,
+			s.metrics,
+		).SetupWithManager(s.mgr); err != nil {
+			s.logger.Fatal(err, "unable to create controller", "controller", controller.RestoreFinalizer)
+		}
+	}
+
 	s.logger.Info("Server starting...")
 
 	if err := s.mgr.Start(s.ctx); err != nil {

pkg/controller/constants.go

@@ -32,6 +32,7 @@ const (
 	RestoreOperations     = "restore-operations"
 	Schedule              = "schedule"
 	ServerStatusRequest   = "server-status-request"
+	RestoreFinalizer      = "restore-finalizer"
 )
 
 // DisableableControllers is a list of controllers that can be disabled
@@ -48,4 +49,5 @@ var DisableableControllers = []string{
 	RestoreOperations,
 	Schedule,
 	ServerStatusRequest,
+	RestoreFinalizer,
 }

pkg/controller/restore_controller.go

@@ -645,18 +645,16 @@ func (r *restoreReconciler) runValidatedRestore(restore *api.Restore, info backu
 			r.logger.Debug("Restore WaitingForPluginOperationsPartiallyFailed")
 			restore.Status.Phase = api.RestorePhaseWaitingForPluginOperationsPartiallyFailed
 		} else {
-			r.logger.Debug("Restore partially failed")
-			restore.Status.Phase = api.RestorePhasePartiallyFailed
-			r.metrics.RegisterRestorePartialFailure(restore.Spec.ScheduleName)
+			r.logger.Debug("Restore FinalizingPartiallyFailed")
+			restore.Status.Phase = api.RestorePhaseFinalizingPartiallyFailed
 		}
 	} else {
 		if inProgressOperations {
 			r.logger.Debug("Restore WaitingForPluginOperations")
 			restore.Status.Phase = api.RestorePhaseWaitingForPluginOperations
 		} else {
-			r.logger.Debug("Restore completed")
-			restore.Status.Phase = api.RestorePhaseCompleted
-			r.metrics.RegisterRestoreSuccess(restore.Spec.ScheduleName)
+			r.logger.Debug("Restore Finalizing")
+			restore.Status.Phase = api.RestorePhaseFinalizing
 		}
 	}
 	return nil

pkg/controller/restore_finalizer_controller.go

@@ -0,0 +1,213 @@
+/*
+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.
+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 controller
+
+import (
+	"context"
+
+	"github.com/pkg/errors"
+	"github.com/sirupsen/logrus"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	ctrl "sigs.k8s.io/controller-runtime"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/utils/clock"
+
+	velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1"
+	"github.com/vmware-tanzu/velero/pkg/metrics"
+	"github.com/vmware-tanzu/velero/pkg/persistence"
+	"github.com/vmware-tanzu/velero/pkg/plugin/clientmgmt"
+	kubeutil "github.com/vmware-tanzu/velero/pkg/util/kube"
+	"github.com/vmware-tanzu/velero/pkg/util/results"
+)
+
+type restoreFinalizerReconciler struct {
+	client.Client
+	namespace         string
+	logger            logrus.FieldLogger
+	newPluginManager  func(logger logrus.FieldLogger) clientmgmt.Manager
+	backupStoreGetter persistence.ObjectBackupStoreGetter
+	metrics           *metrics.ServerMetrics
+	clock             clock.WithTickerAndDelayedExecution
+}
+
+func NewRestoreFinalizerReconciler(
+	logger logrus.FieldLogger,
+	namespace string,
+	client client.Client,
+	newPluginManager func(logrus.FieldLogger) clientmgmt.Manager,
+	backupStoreGetter persistence.ObjectBackupStoreGetter,
+	metrics *metrics.ServerMetrics,
+) *restoreFinalizerReconciler {
+	return &restoreFinalizerReconciler{
+		Client:            client,
+		logger:            logger,
+		namespace:         namespace,
+		newPluginManager:  newPluginManager,
+		backupStoreGetter: backupStoreGetter,
+		metrics:           metrics,
+		clock:             &clock.RealClock{},
+	}
+}
+
+func (r *restoreFinalizerReconciler) SetupWithManager(mgr ctrl.Manager) error {
+	return ctrl.NewControllerManagedBy(mgr).
+		For(&velerov1api.Restore{}).
+		Complete(r)
+}
+
+// +kubebuilder:rbac:groups=velero.io,resources=restores,verbs=get;list;watch;update
+// +kubebuilder:rbac:groups=velero.io,resources=restores/status,verbs=get
+func (r *restoreFinalizerReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
+	log := r.logger.WithField("restore finalizer", req.String())
+	log.Debug("restoreFinalizerReconciler getting restore")
+
+	original := &velerov1api.Restore{}
+	if err := r.Get(ctx, req.NamespacedName, original); err != nil {
+		if apierrors.IsNotFound(err) {
+			log.WithError(err).Error("restore not found")
+			return ctrl.Result{}, nil
+		}
+		return ctrl.Result{}, errors.Wrapf(err, "error getting restore %s", req.String())
+	}
+	restore := original.DeepCopy()
+	log.Debugf("restore: %s", restore.Name)
+
+	log = r.logger.WithFields(
+		logrus.Fields{
+			"restore": req.String(),
+		},
+	)
+
+	switch restore.Status.Phase {
+	case velerov1api.RestorePhaseFinalizing, velerov1api.RestorePhaseFinalizingPartiallyFailed:
+	default:
+		log.Debug("Restore is not awaiting finalization, skipping")
+		return ctrl.Result{}, nil
+	}
+
+	info, err := fetchBackupInfoInternal(r.Client, r.namespace, restore.Spec.BackupName)
+	if err != nil {
+		if apierrors.IsNotFound(err) {
+			log.WithError(err).Error("not found backup, skip")
+			if err2 := r.finishProcessing(velerov1api.RestorePhasePartiallyFailed, restore, original); err2 != nil {
+				log.WithError(err2).Error("error updating restore's final status")
+				return ctrl.Result{}, errors.Wrap(err2, "error updating restore's final status")
+			}
+			return ctrl.Result{}, nil
+		}
+		log.WithError(err).Error("error getting backup info")
+		return ctrl.Result{}, errors.Wrap(err, "error getting backup info")
+	}
+
+	pluginManager := r.newPluginManager(r.logger)
+	defer pluginManager.CleanupClients()
+	backupStore, err := r.backupStoreGetter.Get(info.location, pluginManager, r.logger)
+	if err != nil {
+		log.WithError(err).Error("error getting backup store")
+		return ctrl.Result{}, errors.Wrap(err, "error getting backup store")
+	}
+
+	finalizerCtx := &finalizerContext{log: log}
+	warnings, errs := finalizerCtx.execute()
+
+	warningCnt := len(warnings.Velero) + len(warnings.Cluster)
+	for _, w := range warnings.Namespaces {
+		warningCnt += len(w)
+	}
+	errCnt := len(errs.Velero) + len(errs.Cluster)
+	for _, e := range errs.Namespaces {
+		errCnt += len(e)
+	}
+	restore.Status.Warnings += warningCnt
+	restore.Status.Errors += errCnt
+
+	if !errs.IsEmpty() {
+		restore.Status.Phase = velerov1api.RestorePhaseFinalizingPartiallyFailed
+	}
+
+	if warningCnt > 0 || errCnt > 0 {
+		err := r.updateResults(backupStore, restore, &warnings, &errs)
+		if err != nil {
+			log.WithError(err).Error("error updating results")
+			return ctrl.Result{}, errors.Wrap(err, "error updating results")
+		}
+	}
+
+	finalPhase := velerov1api.RestorePhaseCompleted
+	if restore.Status.Phase == velerov1api.RestorePhaseFinalizingPartiallyFailed {
+		finalPhase = velerov1api.RestorePhasePartiallyFailed
+	}
+	log.Infof("Marking restore %s", finalPhase)
+
+	if err := r.finishProcessing(finalPhase, restore, original); err != nil {
+		log.WithError(err).Error("error updating restore's final status")
+		return ctrl.Result{}, errors.Wrap(err, "error updating restore's final status")
+	}
+
+	return ctrl.Result{}, nil
+}
+
+func (r *restoreFinalizerReconciler) updateResults(backupStore persistence.BackupStore, restore *velerov1api.Restore, newWarnings *results.Result, newErrs *results.Result) error {
+	originResults, err := backupStore.GetRestoreResults(restore.Name)
+	if err != nil {
+		return errors.Wrap(err, "error getting restore results")
+	}
+	warnings := originResults["warnings"]
+	errs := originResults["errors"]
+	warnings.Merge(newWarnings)
+	errs.Merge(newErrs)
+
+	m := map[string]results.Result{
+		"warnings": warnings,
+		"errors":   errs,
+	}
+	if err := putResults(restore, m, backupStore); err != nil {
+		return errors.Wrap(err, "error putting restore results")
+	}
+
+	return nil
+}
+
+func (r *restoreFinalizerReconciler) finishProcessing(restorePhase velerov1api.RestorePhase, restore *velerov1api.Restore, original *velerov1api.Restore) error {
+	if restorePhase == velerov1api.RestorePhasePartiallyFailed {
+		restore.Status.Phase = velerov1api.RestorePhasePartiallyFailed
+		r.metrics.RegisterRestorePartialFailure(restore.Spec.ScheduleName)
+	} else {
+		restore.Status.Phase = velerov1api.RestorePhaseCompleted
+		r.metrics.RegisterRestoreSuccess(restore.Spec.ScheduleName)
+	}
+	restore.Status.CompletionTimestamp = &metav1.Time{Time: r.clock.Now()}
+
+	return kubeutil.PatchResource(original, restore, r.Client)
+}
+
+// finalizerContext includes all the dependencies required by finalization tasks and
+// a function execute() to orderly implement task logic.
+type finalizerContext struct {
+	log logrus.FieldLogger
+}
+
+func (ctx *finalizerContext) execute() (results.Result, results.Result) { //nolint:unparam //temporarily ignore the lint report: result 0 is always nil (unparam)
+	warnings, errs := results.Result{}, results.Result{}
+
+	// implement finalization tasks
+	ctx.log.Debug("Starting running execute()")
+
+	return warnings, errs
+}

pkg/controller/restore_finalizer_controller_test.go

@@ -0,0 +1,204 @@
+/*
+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.
+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 controller
+
+import (
+	"context"
+	"testing"
+	"time"
+
+	"github.com/sirupsen/logrus"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/types"
+	testclocks "k8s.io/utils/clock/testing"
+	ctrl "sigs.k8s.io/controller-runtime"
+
+	"github.com/stretchr/testify/mock"
+
+	velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1"
+	"github.com/vmware-tanzu/velero/pkg/builder"
+	"github.com/vmware-tanzu/velero/pkg/metrics"
+	persistencemocks "github.com/vmware-tanzu/velero/pkg/persistence/mocks"
+	"github.com/vmware-tanzu/velero/pkg/plugin/clientmgmt"
+	pluginmocks "github.com/vmware-tanzu/velero/pkg/plugin/mocks"
+	velerotest "github.com/vmware-tanzu/velero/pkg/test"
+	"github.com/vmware-tanzu/velero/pkg/util/results"
+)
+
+func TestRestoreFinalizerReconcile(t *testing.T) {
+	defaultStorageLocation := builder.ForBackupStorageLocation("velero", "default").Provider("myCloud").Bucket("bucket").Result()
+	now, err := time.Parse(time.RFC1123Z, time.RFC1123Z)
+	require.NoError(t, err)
+	now = now.Local()
+	timestamp := metav1.NewTime(now)
+	assert.NotNil(t, timestamp)
+
+	rfrTests := []struct {
+		name                  string
+		restore               *velerov1api.Restore
+		backup                *velerov1api.Backup
+		location              *velerov1api.BackupStorageLocation
+		expectError           bool
+		expectPhase           velerov1api.RestorePhase
+		expectWarningsCnt     int
+		expectErrsCnt         int
+		statusCompare         bool
+		expectedCompletedTime *metav1.Time
+	}{
+		{
+			name:          "Restore is not awaiting finalization, skip",
+			restore:       builder.ForRestore(velerov1api.DefaultNamespace, "restore-1").Phase(velerov1api.RestorePhaseInProgress).Result(),
+			expectError:   false,
+			expectPhase:   velerov1api.RestorePhaseInProgress,
+			statusCompare: false,
+		},
+		{
+			name:                  "Upon completion of all finalization tasks in the 'FinalizingPartiallyFailed' phase, the restore process transit to the 'PartiallyFailed' phase.",
+			restore:               builder.ForRestore(velerov1api.DefaultNamespace, "restore-1").Phase(velerov1api.RestorePhaseFinalizingPartiallyFailed).Backup("backup-1").Result(),
+			backup:                defaultBackup().StorageLocation("default").Result(),
+			location:              defaultStorageLocation,
+			expectError:           false,
+			expectPhase:           velerov1api.RestorePhasePartiallyFailed,
+			statusCompare:         true,
+			expectedCompletedTime: &timestamp,
+			expectWarningsCnt:     0,
+			expectErrsCnt:         0,
+		},
+		{
+			name:                  "Upon completion of all finalization tasks in the 'Finalizing' phase, the restore process transit to the 'Completed' phase.",
+			restore:               builder.ForRestore(velerov1api.DefaultNamespace, "restore-1").Phase(velerov1api.RestorePhaseFinalizing).Backup("backup-1").Result(),
+			backup:                defaultBackup().StorageLocation("default").Result(),
+			location:              defaultStorageLocation,
+			expectError:           false,
+			expectPhase:           velerov1api.RestorePhaseCompleted,
+			statusCompare:         true,
+			expectedCompletedTime: &timestamp,
+			expectWarningsCnt:     0,
+			expectErrsCnt:         0,
+		},
+		{
+			name:        "Backup not exist",
+			restore:     builder.ForRestore(velerov1api.DefaultNamespace, "restore-1").Phase(velerov1api.RestorePhaseFinalizing).Backup("backup-2").Result(),
+			expectError: false,
+		},
+		{
+			name:          "Restore not exist",
+			restore:       builder.ForRestore("unknown", "restore-1").Phase(velerov1api.RestorePhaseFinalizing).Result(),
+			expectError:   false,
+			statusCompare: false,
+		},
+	}
+
+	for _, test := range rfrTests {
+		t.Run(test.name, func(t *testing.T) {
+			if test.restore == nil {
+				return
+			}
+
+			var (
+				fakeClient    = velerotest.NewFakeControllerRuntimeClientBuilder(t).Build()
+				logger        = velerotest.NewLogger()
+				pluginManager = &pluginmocks.Manager{}
+				backupStore   = &persistencemocks.BackupStore{}
+			)
+
+			defer func() {
+				// reset defaultStorageLocation resourceVersion
+				defaultStorageLocation.ObjectMeta.ResourceVersion = ""
+			}()
+
+			r := NewRestoreFinalizerReconciler(
+				logger,
+				velerov1api.DefaultNamespace,
+				fakeClient,
+				func(logrus.FieldLogger) clientmgmt.Manager { return pluginManager },
+				NewFakeSingleObjectBackupStoreGetter(backupStore),
+				metrics.NewServerMetrics(),
+			)
+			r.clock = testclocks.NewFakeClock(now)
+
+			if test.restore != nil && test.restore.Namespace == velerov1api.DefaultNamespace {
+				require.NoError(t, r.Client.Create(context.Background(), test.restore))
+			}
+			if test.backup != nil {
+				assert.NoError(t, r.Client.Create(context.Background(), test.backup))
+			}
+			if test.location != nil {
+				require.NoError(t, r.Client.Create(context.Background(), test.location))
+			}
+
+			if test.restore != nil {
+				pluginManager.On("GetRestoreItemActionsV2").Return(nil, nil)
+				pluginManager.On("CleanupClients")
+			}
+
+			_, err = r.Reconcile(context.Background(), ctrl.Request{NamespacedName: types.NamespacedName{
+				Namespace: test.restore.Namespace,
+				Name:      test.restore.Name,
+			}})
+
+			assert.Equal(t, test.expectError, err != nil)
+			if test.expectError {
+				return
+			}
+
+			if test.statusCompare {
+				restoreAfter := velerov1api.Restore{}
+				err = fakeClient.Get(context.TODO(), types.NamespacedName{
+					Namespace: test.restore.Namespace,
+					Name:      test.restore.Name,
+				}, &restoreAfter)
+
+				require.NoError(t, err)
+
+				assert.Equal(t, test.expectPhase, restoreAfter.Status.Phase)
+				assert.Equal(t, test.expectErrsCnt, restoreAfter.Status.Errors)
+				assert.Equal(t, test.expectWarningsCnt, restoreAfter.Status.Warnings)
+				require.True(t, test.expectedCompletedTime.Equal(restoreAfter.Status.CompletionTimestamp))
+			}
+		})
+	}
+
+}
+
+func TestUpdateResult(t *testing.T) {
+	var (
+		fakeClient    = velerotest.NewFakeControllerRuntimeClientBuilder(t).Build()
+		logger        = velerotest.NewLogger()
+		pluginManager = &pluginmocks.Manager{}
+		backupStore   = &persistencemocks.BackupStore{}
+	)
+
+	r := NewRestoreFinalizerReconciler(
+		logger,
+		velerov1api.DefaultNamespace,
+		fakeClient,
+		func(logrus.FieldLogger) clientmgmt.Manager { return pluginManager },
+		NewFakeSingleObjectBackupStoreGetter(backupStore),
+		metrics.NewServerMetrics(),
+	)
+	restore := builder.ForRestore(velerov1api.DefaultNamespace, "restore-1").Result()
+	res := map[string]results.Result{"warnings": {}, "errors": {}}
+
+	backupStore.On("GetRestoreResults", restore.Name).Return(res, nil)
+	backupStore.On("PutRestoreResults", mock.Anything, mock.Anything, mock.Anything).Return(nil)
+
+	err := r.updateResults(backupStore, restore, &results.Result{}, &results.Result{})
+	require.NoError(t, err)
+}

pkg/controller/restore_operations_controller.go

@@ -128,10 +128,8 @@ func (r *restoreOperationsReconciler) Reconcile(ctx context.Context, req ctrl.Re
 
 	info, err := r.fetchBackupInfo(restore.Spec.BackupName)
 	if err != nil {
-		log.Warnf("Cannot check progress on Restore operations because backup info is unavailable %s; marking restore PartiallyFailed", err.Error())
-		restore.Status.Phase = velerov1api.RestorePhasePartiallyFailed
-		restore.Status.CompletionTimestamp = &metav1.Time{Time: r.clock.Now()}
-		r.metrics.RegisterRestorePartialFailure(restore.Spec.ScheduleName)
+		log.Warnf("Cannot check progress on Restore operations because backup info is unavailable %s; marking restore FinalizingPartiallyFailed", err.Error())
+		restore.Status.Phase = velerov1api.RestorePhaseFinalizingPartiallyFailed
 		err2 := r.updateRestoreAndOperationsJSON(ctx, original, restore, nil, &itemoperationmap.OperationsForRestore{ErrsSinceUpdate: []string{err.Error()}}, false, false)
 		if err2 != nil {
 			log.WithError(err2).Error("error updating Restore")
@@ -178,15 +176,11 @@ func (r *restoreOperationsReconciler) Reconcile(ctx context.Context, req ctrl.Re
 	// If the only changes are incremental progress, then no write is necessary, progress can remain in memory
 	if !stillInProgress {
 		if restore.Status.Phase == velerov1api.RestorePhaseWaitingForPluginOperations {
-			log.Infof("Marking restore %s completed", restore.Name)
-			restore.Status.Phase = velerov1api.RestorePhaseCompleted
-			restore.Status.CompletionTimestamp = &metav1.Time{Time: r.clock.Now()}
-			r.metrics.RegisterRestoreSuccess(restore.Spec.ScheduleName)
+			log.Infof("Marking restore %s Finalizing", restore.Name)
+			restore.Status.Phase = velerov1api.RestorePhaseFinalizing
 		} else {
 			log.Infof("Marking restore %s FinalizingPartiallyFailed", restore.Name)
-			restore.Status.Phase = velerov1api.RestorePhasePartiallyFailed
-			restore.Status.CompletionTimestamp = &metav1.Time{Time: r.clock.Now()}
-			r.metrics.RegisterRestorePartialFailure(restore.Spec.ScheduleName)
+			restore.Status.Phase = velerov1api.RestorePhaseFinalizingPartiallyFailed
 		}
 	}
 	err = r.updateRestoreAndOperationsJSON(ctx, original, restore, backupStore, operations, changes, completionChanges)
@@ -216,8 +210,8 @@ func (r *restoreOperationsReconciler) updateRestoreAndOperationsJSON(
 	removeIfComplete := true
 	defer func() {
 		// remove local operations list if complete
-		if removeIfComplete && (restore.Status.Phase == velerov1api.RestorePhaseCompleted ||
-			restore.Status.Phase == velerov1api.RestorePhasePartiallyFailed) {
+		if removeIfComplete && (restore.Status.Phase == velerov1api.RestorePhaseFinalizing ||
+			restore.Status.Phase == velerov1api.RestorePhaseFinalizingPartiallyFailed) {
 			r.itemOperationsMap.DeleteOperationsForRestore(restore.Name)
 		} else if changes {
 			r.itemOperationsMap.PutOperationsForRestore(operations, restore.Name)
@@ -226,8 +220,8 @@ func (r *restoreOperationsReconciler) updateRestoreAndOperationsJSON(
 
 	// update restore and upload progress if errs or complete
 	if len(operations.ErrsSinceUpdate) > 0 ||
-		restore.Status.Phase == velerov1api.RestorePhaseCompleted ||
-		restore.Status.Phase == velerov1api.RestorePhasePartiallyFailed {
+		restore.Status.Phase == velerov1api.RestorePhaseFinalizing ||
+		restore.Status.Phase == velerov1api.RestorePhaseFinalizingPartiallyFailed {
 		// update file store
 		if backupStore != nil {
 			if err := r.itemOperationsMap.UploadProgressAndPutOperationsForRestore(backupStore, operations, restore.Name); err != nil {

pkg/controller/restore_operations_controller_test.go

@@ -94,7 +94,7 @@ func TestRestoreOperationsReconcile(t *testing.T) {
 			backup:            defaultBackup().StorageLocation("default").Result(),
 			backupLocation:    defaultBackupLocation,
 			operationComplete: true,
-			expectPhase:       velerov1api.RestorePhaseCompleted,
+			expectPhase:       velerov1api.RestorePhaseFinalizing,
 			restoreOperations: []*itemoperation.RestoreOperation{
 				{
 					Spec: itemoperation.RestoreOperationSpec{
@@ -157,7 +157,7 @@ func TestRestoreOperationsReconcile(t *testing.T) {
 			backupLocation:    defaultBackupLocation,
 			operationComplete: true,
 			operationErr:      "failed",
-			expectPhase:       velerov1api.RestorePhasePartiallyFailed,
+			expectPhase:       velerov1api.RestorePhaseFinalizingPartiallyFailed,
 			restoreOperations: []*itemoperation.RestoreOperation{
 				{
 					Spec: itemoperation.RestoreOperationSpec{
@@ -188,7 +188,7 @@ func TestRestoreOperationsReconcile(t *testing.T) {
 			backup:            defaultBackup().StorageLocation("default").Result(),
 			backupLocation:    defaultBackupLocation,
 			operationComplete: true,
-			expectPhase:       velerov1api.RestorePhasePartiallyFailed,
+			expectPhase:       velerov1api.RestorePhaseFinalizingPartiallyFailed,
 			restoreOperations: []*itemoperation.RestoreOperation{
 				{
 					Spec: itemoperation.RestoreOperationSpec{
@@ -251,7 +251,7 @@ func TestRestoreOperationsReconcile(t *testing.T) {
 			backupLocation:    defaultBackupLocation,
 			operationComplete: true,
 			operationErr:      "failed",
-			expectPhase:       velerov1api.RestorePhasePartiallyFailed,
+			expectPhase:       velerov1api.RestorePhaseFinalizingPartiallyFailed,
 			restoreOperations: []*itemoperation.RestoreOperation{
 				{
 					Spec: itemoperation.RestoreOperationSpec{

pkg/persistence/mocks/backup_store.go

@@ -28,6 +28,8 @@ import (
 	"github.com/vmware-tanzu/velero/pkg/persistence"
 	v1 "github.com/vmware-tanzu/velero/pkg/apis/velero/v1"
 	volume "github.com/vmware-tanzu/velero/pkg/volume"
+	"github.com/vmware-tanzu/velero/pkg/util/results"
+
 
 )
 
@@ -336,6 +338,29 @@ func (_m *BackupStore) GetBackupVolumeInfos(name string) ([]*internalVolume.Volu
 	return r0, r1
 }
 
+// GetRestoreResults provides a mock function with given fields: name
+func (_m *BackupStore) GetRestoreResults(name string) (map[string]results.Result, error) {
+	ret := _m.Called(name)
+
+	r0 := make(map[string]results.Result)
+	if rf, ok := ret.Get(0).(func(string) map[string]results.Result); ok {
+		r0 = rf(name)
+	} else {
+		if ret.Get(0) != nil {
+			r0 = ret.Get(0).(map[string]results.Result)
+		}
+	}
+
+	var r1 error
+	if rf, ok := ret.Get(1).(func(string) error); ok {
+		r1 = rf(name)
+	} else {
+		r1 = ret.Error(1)
+	}
+
+	return r0, r1
+}
+
 // IsValid provides a mock function with given fields:
 func (_m *BackupStore) IsValid() error {
 	ret := _m.Called()

pkg/persistence/object_store.go

@@ -36,6 +36,7 @@ import (
 	"github.com/vmware-tanzu/velero/pkg/itemoperation"
 	"github.com/vmware-tanzu/velero/pkg/plugin/velero"
 	"github.com/vmware-tanzu/velero/pkg/util"
+	"github.com/vmware-tanzu/velero/pkg/util/results"
 	"github.com/vmware-tanzu/velero/pkg/volume"
 )
 
@@ -75,6 +76,7 @@ type BackupStore interface {
 	GetCSIVolumeSnapshotContents(name string) ([]*snapshotv1api.VolumeSnapshotContent, error)
 	GetCSIVolumeSnapshotClasses(name string) ([]*snapshotv1api.VolumeSnapshotClass, error)
 	GetBackupVolumeInfos(name string) ([]*internalVolume.VolumeInfo, error)
+	GetRestoreResults(name string) (map[string]results.Result, error)
 
 	// BackupExists checks if the backup metadata file exists in object storage.
 	BackupExists(bucket, backupName string) (bool, error)
@@ -514,6 +516,25 @@ func (s *objectBackupStore) GetBackupVolumeInfos(name string) ([]*internalVolume
 	return volumeInfos, nil
 }
 
+func (s *objectBackupStore) GetRestoreResults(name string) (map[string]results.Result, error) {
+	results := make(map[string]results.Result)
+
+	res, err := tryGet(s.objectStore, s.bucket, s.layout.getRestoreResultsKey(name))
+	if err != nil {
+		return results, err
+	}
+	if res == nil {
+		return results, nil
+	}
+	defer res.Close()
+
+	if err := decode(res, &results); err != nil {
+		return results, err
+	}
+
+	return results, nil
+}
+
 func (s *objectBackupStore) GetBackupContents(name string) (io.ReadCloser, error) {
 	return s.objectStore.GetObject(s.bucket, s.layout.getBackupContentsKey(name))
 }

pkg/persistence/object_store_test.go

@@ -43,6 +43,7 @@ import (
 	providermocks "github.com/vmware-tanzu/velero/pkg/plugin/velero/mocks"
 	velerotest "github.com/vmware-tanzu/velero/pkg/test"
 	"github.com/vmware-tanzu/velero/pkg/util/encode"
+	"github.com/vmware-tanzu/velero/pkg/util/results"
 	"github.com/vmware-tanzu/velero/pkg/volume"
 )
 
@@ -1146,6 +1147,35 @@ func TestGetBackupVolumeInfos(t *testing.T) {
 		})
 	}
 }
+func TestGetRestoreResults(t *testing.T) {
+	harness := newObjectBackupStoreTestHarness("test-bucket", "")
+
+	// file not found should not error
+	_, err := harness.GetRestoreResults("test-restore")
+	assert.NoError(t, err)
+
+	// file containing invalid data should error
+	harness.objectStore.PutObject(harness.bucket, "restores/test-restore/restore-test-restore-results.gz", newStringReadSeeker("foo"))
+	_, err = harness.GetRestoreResults("test-restore")
+	assert.NotNil(t, err)
+
+	// file containing gzipped json data should return correctly
+	contents := map[string]results.Result{
+		"warnings": {Cluster: []string{"cluster warning"}},
+		"errors":   {Namespaces: map[string][]string{"test-ns": {"namespace error"}}},
+	}
+	obj := new(bytes.Buffer)
+	gzw := gzip.NewWriter(obj)
+
+	require.NoError(t, json.NewEncoder(gzw).Encode(contents))
+	require.NoError(t, gzw.Close())
+	require.NoError(t, harness.objectStore.PutObject(harness.bucket, "restores/test-restore/restore-test-restore-results.gz", obj))
+	res, err := harness.GetRestoreResults("test-restore")
+
+	assert.NoError(t, err)
+	assert.EqualValues(t, contents["warnings"], res["warnings"])
+	assert.EqualValues(t, contents["errors"], res["errors"])
+}
 
 func encodeToBytes(obj runtime.Object) []byte {
 	res, err := encode.Encode(obj, "json")