From cd860771c28a2509668540c2ad0c7e00cfe8937f Mon Sep 17 00:00:00 2001
From: John Naulty Jr <johnnaulty@bitgo.com>
Date: Tue, 17 Dec 2019 13:23:58 -0500
Subject: [PATCH] Add error return to pvRenamer func (#2138)

Migrate logic from NewUUID function into the pvRenamer function.

PR #2133 switched to a new NewUUID function that returns an error, but
the invocation of that function needs to happen within the pvRenamer
closure. Because the new function returns an error, the pvRenamer should
return the error, the signature needs to be changed and the return
checked.

Signed-off-by: John Naulty <johnnaulty@bitgo.com>
---
 pkg/restore/restore.go      | 25 ++++++++++++++++---------
 pkg/restore/restore_test.go |  5 ++++-
 2 files changed, 20 insertions(+), 10 deletions(-)

diff --git a/pkg/restore/restore.go b/pkg/restore/restore.go
index c1baa00dd..d005d15f3 100644
--- a/pkg/restore/restore.go
+++ b/pkg/restore/restore.go
@@ -94,7 +94,7 @@ type kubernetesRestorer struct {
 	resourceTerminatingTimeout time.Duration
 	resourcePriorities         []string
 	fileSystem                 filesystem.Interface
-	pvRenamer                  func(string) string
+	pvRenamer                  func(string) (string, error)
 	logger                     logrus.FieldLogger
 }
 
@@ -167,10 +167,6 @@ func NewKubernetesRestorer(
 	resourceTerminatingTimeout time.Duration,
 	logger logrus.FieldLogger,
 ) (Restorer, error) {
-	veleroCloneUuid, err := uuid.NewV4()
-	if err != nil {
-		return nil, errors.WithStack(err)
-	}
 	return &kubernetesRestorer{
 		discoveryHelper:            discoveryHelper,
 		dynamicFactory:             dynamicFactory,
@@ -180,8 +176,15 @@ func NewKubernetesRestorer(
 		resourceTerminatingTimeout: resourceTerminatingTimeout,
 		resourcePriorities:         resourcePriorities,
 		logger:                     logger,
-		pvRenamer:                  func(string) string { return "velero-clone-" + veleroCloneUuid.String() },
-		fileSystem:                 filesystem.NewFileSystem(),
+		pvRenamer: func(string) (string, error) {
+			veleroCloneUuid, err := uuid.NewV4()
+			if err != nil {
+				return "", errors.WithStack(err)
+			}
+			veleroCloneName := "velero-clone-" + veleroCloneUuid.String()
+			return veleroCloneName, nil
+		},
+		fileSystem: filesystem.NewFileSystem(),
 	}, nil
 }
 
@@ -371,7 +374,7 @@ type context struct {
 	resourceClients            map[resourceClientKey]client.Dynamic
 	restoredItems              map[velero.ResourceIdentifier]struct{}
 	renamedPVs                 map[string]string
-	pvRenamer                  func(string) string
+	pvRenamer                  func(string) (string, error)
 }
 
 type resourceClientKey struct {
@@ -863,7 +866,11 @@ func (ctx *context) restoreItem(obj *unstructured.Unstructured, groupResource sc
 			if shouldRenamePV {
 				// give obj a new name, and record the mapping between the old and new names
 				oldName := obj.GetName()
-				newName := ctx.pvRenamer(oldName)
+				newName, err := ctx.pvRenamer(oldName)
+				if err != nil {
+					addToResult(&errs, namespace, errors.Wrapf(err, "error renaming PV"))
+					return warnings, errs
+				}
 
 				ctx.renamedPVs[oldName] = newName
 				obj.SetName(newName)
diff --git a/pkg/restore/restore_test.go b/pkg/restore/restore_test.go
index 29c668214..1c9bff12e 100644
--- a/pkg/restore/restore_test.go
+++ b/pkg/restore/restore_test.go
@@ -2237,7 +2237,10 @@ func TestRestorePersistentVolumes(t *testing.T) {
 		t.Run(tc.name, func(t *testing.T) {
 			h := newHarness(t)
 			h.restorer.resourcePriorities = []string{"persistentvolumes", "persistentvolumeclaims"}
-			h.restorer.pvRenamer = func(oldName string) string { return "renamed-" + oldName }
+			h.restorer.pvRenamer = func(oldName string) (string, error) {
+				renamed := "renamed-" + oldName
+				return renamed, nil
+			}
 
 			// set up the VolumeSnapshotLocation informer/lister and add test data to it
 			vslInformer := velerov1informers.NewSharedInformerFactory(h.VeleroClient, 0).Velero().V1().VolumeSnapshotLocations()