code review feedback

Signed-off-by: Steve Kriss <krisss@vmware.com>

pkg/cloudprovider/aws/block_store.go

@@ -266,7 +266,7 @@ func (b *blockStore) GetVolumeID(unstructuredPV runtime.Unstructured) (string, e
 	}
 
 	if pv.Spec.AWSElasticBlockStore.VolumeID == "" {
-		return "", errors.New(".spec.awsElasticBlockStore.volumeID not found")
+		return "", errors.New("spec.awsElasticBlockStore.volumeID not found")
 	}
 
 	return ebsVolumeIDRegex.FindString(pv.Spec.AWSElasticBlockStore.VolumeID), nil
@@ -279,7 +279,7 @@ func (b *blockStore) SetVolumeID(unstructuredPV runtime.Unstructured, volumeID s
 	}
 
 	if pv.Spec.AWSElasticBlockStore == nil {
-		return nil, errors.New(".spec.awsElasticBlockStore not found")
+		return nil, errors.New("spec.awsElasticBlockStore not found")
 	}
 
 	pv.Spec.AWSElasticBlockStore.VolumeID = volumeID

pkg/cloudprovider/azure/block_store.go

@@ -347,7 +347,7 @@ func (b *blockStore) GetVolumeID(unstructuredPV runtime.Unstructured) (string, e
 	}
 
 	if pv.Spec.AzureDisk.DiskName == "" {
-		return "", errors.New(".spec.azureDisk.diskName not found")
+		return "", errors.New("spec.azureDisk.diskName not found")
 	}
 
 	return pv.Spec.AzureDisk.DiskName, nil
@@ -360,7 +360,7 @@ func (b *blockStore) SetVolumeID(unstructuredPV runtime.Unstructured, volumeID s
 	}
 
 	if pv.Spec.AzureDisk == nil {
-		return nil, errors.New(".spec.azureDisk not found")
+		return nil, errors.New("spec.azureDisk not found")
 	}
 
 	pv.Spec.AzureDisk.DiskName = volumeID

pkg/cloudprovider/gcp/block_store.go

@@ -304,7 +304,7 @@ func (b *blockStore) GetVolumeID(unstructuredPV runtime.Unstructured) (string, e
 	}
 
 	if pv.Spec.GCEPersistentDisk.PDName == "" {
-		return "", errors.New(".spec.gcePersistentDisk.pdName not found")
+		return "", errors.New("spec.gcePersistentDisk.pdName not found")
 	}
 
 	return pv.Spec.GCEPersistentDisk.PDName, nil
@@ -317,7 +317,7 @@ func (b *blockStore) SetVolumeID(unstructuredPV runtime.Unstructured, volumeID s
 	}
 
 	if pv.Spec.GCEPersistentDisk == nil {
-		return nil, errors.New(".spec.gcePersistentDisk not found")
+		return nil, errors.New("spec.gcePersistentDisk not found")
 	}
 
 	pv.Spec.GCEPersistentDisk.PDName = volumeID

pkg/controller/restore_controller_test.go

@@ -458,7 +458,7 @@ func TestProcessRestore(t *testing.T) {
 					res.Status.Phase = api.RestorePhase(phase)
 
 					backupName, found, err := unstructured.NestedString(patchMap, "spec", "backupName")
-					if err == nil && found {
+					if found {
 						res.Spec.BackupName = backupName
 					}
 

pkg/restore/pod_action.go

@@ -51,15 +51,6 @@ func (a *podAction) Execute(obj runtime.Unstructured, restore *api.Restore) (run
 	pod.Spec.NodeName = ""
 	pod.Spec.Priority = nil
 
-	// if there are no volumes, then there can't be any volume mounts, so we're done.
-	if len(pod.Spec.Volumes) == 0 {
-		res, err := runtime.DefaultUnstructuredConverter.ToUnstructured(pod)
-		if err != nil {
-			return nil, nil, errors.WithStack(err)
-		}
-		return &unstructured.Unstructured{Object: res}, nil, nil
-	}
-
 	serviceAccountTokenPrefix := pod.Spec.ServiceAccountName + "-token-"
 
 	var preservedVolumes []v1.Volume

pkg/restore/restore.go

@@ -1120,6 +1120,9 @@ func (r *pvRestorer) executePVAction(obj *unstructured.Unstructured) (*unstructu
 		return nil, errors.New("PersistentVolume is missing its name")
 	}
 
+	// It's simpler to just access the spec through the unstructured object than to convert
+	// to structured and back here, especially since the SetVolumeID(...) call below needs
+	// the unstructured representation (and does a conversion internally).
 	res, ok := obj.Object["spec"]
 	if !ok {
 		return nil, errors.New("spec not found")

pkg/restore/service_action.go

@@ -22,7 +22,6 @@ import (
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
 	corev1api "k8s.io/api/core/v1"
-	"k8s.io/apimachinery/pkg/api/meta"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/util/sets"
@@ -68,26 +67,6 @@ func (a *serviceAction) Execute(obj runtime.Unstructured, restore *api.Restore)
 	return &unstructured.Unstructured{Object: res}, nil, nil
 }
 
-func getPreservedPorts(obj runtime.Unstructured) (map[string]bool, error) {
-	preservedPorts := map[string]bool{}
-	metadata, err := meta.Accessor(obj)
-	if err != nil {
-		return nil, errors.WithStack(err)
-	}
-	if lac, ok := metadata.GetAnnotations()[annotationLastAppliedConfig]; ok {
-		var svc corev1api.Service
-		if err := json.Unmarshal([]byte(lac), &svc); err != nil {
-			return nil, errors.WithStack(err)
-		}
-		for _, port := range svc.Spec.Ports {
-			if port.NodePort > 0 {
-				preservedPorts[port.Name] = true
-			}
-		}
-	}
-	return preservedPorts, nil
-}
-
 func deleteNodePorts(service *corev1api.Service) error {
 	if service.Spec.Type == corev1api.ServiceTypeExternalName {
 		return nil