Import SAN-Economy volume

* Import SAN-Economy volume * making san-economy import as ACP gated * Convert GetVolumeExternal to GetVolumeForImport --------- Co-authored-by: Clinton Knight <[email protected]>
NetApp · Jun 1, 2024 · 86cd9c9 · 86cd9c9
1 parent 0cd9a33
commit 86cd9c9
Show file tree

Hide file tree

Showing 39 changed files with 885 additions and 429 deletions.
diff --git a/acp/config.go b/acp/config.go
@@ -9,8 +9,9 @@ const (
 
 	// Feature constants.
 
-	FeatureSnapshotRestore      = "SnapshotRestore"
-	FeatureSnapshotMirrorUpdate = "SnapshotMirrorUpdate"
-	FeatureReadOnlyClone        = "ReadOnlyClone"
-	FeatureInflightEncryption   = "InflightEncryption"
+	FeatureSnapshotRestore        = "SnapshotRestore"
+	FeatureSnapshotMirrorUpdate   = "SnapshotMirrorUpdate"
+	FeatureReadOnlyClone          = "ReadOnlyClone"
+	FeatureInflightEncryption     = "InflightEncryption"
+	FeatureSANEconomyVolumeImport = "SANEconomyVolumeImport"
 )
diff --git a/core/orchestrator_core.go b/core/orchestrator_core.go
@@ -2516,27 +2516,28 @@ func (o *TridentOrchestrator) cloneVolumeRetry(
 	return o.addVolumeFinish(ctx, txn, vol, backend, pool)
 }
 
-// GetVolumeExternal is used by volume import so it doesn't check core's o.volumes to see if the
+// GetVolumeForImport is used by volume import so it doesn't check core's o.volumes to see if the
 // volume exists or not. Instead it asks the driver if the volume exists before requesting
-// the volume size. Returns the VolumeExternal representation of the volume.
-func (o *TridentOrchestrator) GetVolumeExternal(
-	ctx context.Context, volumeName, backendName string,
+// the volume size.  Accepts a backend-specific string that a driver may use to find a volume.
+// Returns the VolumeExternal representation of the volume.
+func (o *TridentOrchestrator) GetVolumeForImport(
+	ctx context.Context, volumeID, backendName string,
 ) (volExternal *storage.VolumeExternal, err error) {
 	ctx = GenerateRequestContextForLayer(ctx, LogLayerCore)
 
 	if o.bootstrapError != nil {
 		return nil, o.bootstrapError
 	}
 
-	defer recordTiming("volume_get_external", &err)()
+	defer recordTiming("volume_get_for_import", &err)()
 
 	o.mutex.Lock()
 	defer o.mutex.Unlock()
 
 	Logc(ctx).WithFields(LogFields{
-		"originalName": volumeName,
-		"backendName":  backendName,
-	}).Debug("Orchestrator#GetVolumeExternal")
+		"volumeID":    volumeID,
+		"backendName": backendName,
+	}).Debug("Orchestrator#GetVolumeForImport")
 
 	backendUUID, err := o.getBackendUUIDByBackendName(backendName)
 	if err != nil {
@@ -2547,7 +2548,7 @@ func (o *TridentOrchestrator) GetVolumeExternal(
 		return nil, errors.NotFoundError("backend %s not found", backendName)
 	}
 
-	volExternal, err = backend.GetVolumeExternal(ctx, volumeName)
+	volExternal, err = backend.GetVolumeForImport(ctx, volumeID)
 	if err != nil {
 		return nil, err
 	}
@@ -2583,8 +2584,13 @@ func (o *TridentOrchestrator) validateImportVolume(ctx context.Context, volumeCo
 	originalName := volumeConfig.ImportOriginalName
 	backendUUID := volumeConfig.ImportBackendUUID
 
-	for volumeName, volume := range o.volumes {
-		if volume.Config.InternalName == originalName && volume.BackendUUID == backendUUID {
+	extantVol, err := backend.GetVolumeForImport(ctx, originalName)
+	if err != nil {
+		return errors.NotFoundError("volume %s was not found", originalName)
+	}
+
+	for volumeName, managedVol := range o.volumes {
+		if managedVol.Config.InternalName == extantVol.Config.InternalName && managedVol.BackendUUID == backendUUID {
 			return errors.FoundError("PV %s already exists for volume %s", originalName, volumeName)
 		}
 	}
@@ -2599,10 +2605,6 @@ func (o *TridentOrchestrator) validateImportVolume(ctx context.Context, volumeCo
 			volumeConfig.StorageClass, backend.Name())
 	}
 
-	if backend.Driver().Get(ctx, originalName) != nil {
-		return errors.NotFoundError("volume %s was not found", originalName)
-	}
-
 	// Identify the resultant protocol based on the VolumeMode, AccessMode and the Protocol. Fro a valid case
 	// o.getProtocol returns a protocol that is either same as volumeConfig.Protocol or `file/block` protocol
 	// in place of `Any` Protocol.

diff --git a/core/types.go b/core/types.go
@@ -46,7 +46,7 @@ type Orchestrator interface {
 	DeleteVolume(ctx context.Context, volume string) error
 	GetVolume(ctx context.Context, volumeName string) (*storage.VolumeExternal, error)
 	GetVolumeByInternalName(ctx context.Context, volumeInternal string) (volume string, err error)
-	GetVolumeExternal(ctx context.Context, volumeName, backendName string) (*storage.VolumeExternal, error)
+	GetVolumeForImport(ctx context.Context, volumeID, backendName string) (*storage.VolumeExternal, error)
 	ImportVolume(ctx context.Context, volumeConfig *storage.VolumeConfig) (*storage.VolumeExternal, error)
 	ListVolumes(ctx context.Context) ([]*storage.VolumeExternal, error)
 	PublishVolume(ctx context.Context, volumeName string, publishInfo *utils.VolumePublishInfo) error

diff --git a/frontend/csi/controller_helpers/kubernetes/import.go b/frontend/csi/controller_helpers/kubernetes/import.go
@@ -1,4 +1,4 @@
-// Copyright 2022 NetApp, Inc. All Rights Reserved.
+// Copyright 2024 NetApp, Inc. All Rights Reserved.
 
 package kubernetes
 
@@ -11,12 +11,11 @@ import (
 	"strings"
 	"time"
 
+	"github.com/cenkalti/backoff/v4"
 	v1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
-	"github.com/cenkalti/backoff/v4"
-
 	"github.com/netapp/trident/frontend/csi"
 	. "github.com/netapp/trident/logging"
 	"github.com/netapp/trident/storage"
@@ -42,12 +41,6 @@ func (h *helper) ImportVolume(
 		return nil, fmt.Errorf("the pvcData field does not contain valid base64-encoded data: %v", err)
 	}
 
-	existingVol, err := h.orchestrator.GetVolumeByInternalName(ctx, request.InternalName)
-	if err == nil {
-		return nil, errors.FoundError("PV %s already exists for volume %s",
-			existingVol, request.InternalName)
-	}
-
 	claim := &v1.PersistentVolumeClaim{}
 	err = json.Unmarshal(jsonData, &claim)
 	if err != nil {
@@ -63,15 +56,17 @@ func (h *helper) ImportVolume(
 		"PVC_requestedSize": claim.Spec.Resources.Requests[v1.ResourceStorage],
 	}).Trace()
 
+	// Ensure the new PVC specified a storage class
 	if err = h.checkValidStorageClassReceived(ctx, claim); err != nil {
 		return nil, err
 	}
 
+	// Ensure the new PVC specified a namespace
 	if len(claim.Namespace) == 0 {
 		return nil, fmt.Errorf("a valid PVC namespace is required for volume import")
 	}
 
-	// Lookup backend ID from given name
+	// Lookup backend from given name
 	backend, err := h.orchestrator.GetBackend(ctx, request.Backend)
 	if err != nil {
 		return nil, fmt.Errorf("could not find backend %s; %v", request.Backend, err)
@@ -86,24 +81,31 @@ func (h *helper) ImportVolume(
 	claim.Annotations[AnnImportBackendUUID] = backend.BackendUUID
 	claim.Annotations[AnnStorageProvisioner] = csi.Provisioner
 
-	// Set the PVC's storage field to the actual volume data size
-	volExternal, err := h.orchestrator.GetVolumeExternal(ctx, request.InternalName, request.Backend)
+	// Find the volume on the storage system
+	extantVol, err := h.orchestrator.GetVolumeForImport(ctx, request.InternalName, request.Backend)
 	if err != nil {
 		return nil, fmt.Errorf("volume import failed to get size of volume: %v", err)
 	}
 
-	totalSize, err := strconv.ParseUint(volExternal.Config.Size, 10, 64)
+	// Ensure this volume isn't already managed
+	managedVol, err := h.orchestrator.GetVolumeByInternalName(ctx, extantVol.Config.InternalName)
+	if err == nil {
+		return nil, errors.FoundError("PV %s already exists for volume %s", managedVol, request.InternalName)
+	}
+
+	// Set the PVC's storage field to the actual volume data size
+	totalSize, err := strconv.ParseUint(extantVol.Config.Size, 10, 64)
 	if err != nil {
 		return nil, fmt.Errorf("volume import failed as volume %v has invalid size %v: %v",
-			request.InternalName, volExternal.Config.Size, err)
+			request.InternalName, extantVol.Config.Size, err)
 	}
 
 	snapshotReserve := 0
-	if volExternal.Config.SnapshotReserve != "" {
-		snapshotReserve, err = strconv.Atoi(volExternal.Config.SnapshotReserve)
+	if extantVol.Config.SnapshotReserve != "" {
+		snapshotReserve, err = strconv.Atoi(extantVol.Config.SnapshotReserve)
 		if err != nil {
 			return nil, fmt.Errorf("volume import failed as volume %v has invalid snapshot reserve %v: %v",
-				request.InternalName, volExternal.Config.SnapshotReserve, err)
+				request.InternalName, extantVol.Config.SnapshotReserve, err)
 		}
 	}
 
@@ -115,7 +117,7 @@ func (h *helper) ImportVolume(
 	claim.Spec.Resources.Requests[v1.ResourceStorage] = resource.MustParse(strconv.FormatUint(dataSize, 10))
 
 	Logc(ctx).WithFields(LogFields{
-		"total size":       volExternal.Config.Size,
+		"total size":       extantVol.Config.Size,
 		"snapshot reserve": snapshotReserve,
 		"data size":        dataSize,
 		"claimSize":        claim.Spec.Resources.Requests[v1.ResourceStorage],
@@ -144,6 +146,7 @@ func (h *helper) ImportVolume(
 	if err != nil {
 		return nil, fmt.Errorf("error getting volume %s; %v", pvName, err)
 	}
+
 	return volume, nil
 }
 
@@ -176,7 +179,7 @@ func checkAndHandleUnrecoverableError(ctx context.Context, h *helper, pvc *v1.Pe
 	var appendString string
 	if pvError != nil {
 		events, err := h.kubeClient.CoreV1().Events(pvc.Namespace).List(ctx, metav1.ListOptions{
-			FieldSelector: fmt.Sprintf("involvedObject.name=%s,involvedObject.kind=PersistentVolumeClaim", pvc.Name),
+			FieldSelector: fmt.Sprintf("involvedObject.uid=%s,involvedObject.kind=PersistentVolumeClaim", pvc.UID),
 		})
 		if err != nil {
 			return false, err

diff --git a/mocks/mock_core/mock_core.go b/mocks/mock_core/mock_core.go