Removing check which blocks legacy ASA systems

Author: aparna0508

storage_drivers/ontap/ontap_factory.go

@@ -14,6 +14,8 @@ import (
 	"github.com/netapp/trident/storage"
 	sa "github.com/netapp/trident/storage_attribute"
 	drivers "github.com/netapp/trident/storage_drivers"
+	"github.com/netapp/trident/storage_drivers/ontap/api"
+	"github.com/netapp/trident/storage_drivers/ontap/awsapi"
 )
 
 const (
@@ -81,37 +83,60 @@ func GetStorageDriver(
 
 	// ontap-san uses additional system details to choose the needed driver
 	case config.OntapSANStorageDriverName:
-		switch driverProtocol {
-		case sa.ISCSI:
-			if API.IsSANOptimized() && API.IsDisaggregated() {
-				ontapConfig.Flags[FlagPersonality] = PersonalityASAr2 // Used by ASUP to distinguish personalities
-				storageDriver = &ASAStorageDriver{API: API, Config: *ontapConfig}
-			} else if !API.IsSANOptimized() && !API.IsDisaggregated() {
-				storageDriver = &SANStorageDriver{API: API, AWSAPI: AWSAPI, Config: *ontapConfig}
-			} else {
-				return nil, fmt.Errorf("unsupported ONTAP personality with disaggregated %t and SAN optimized %t",
-					API.IsDisaggregated(), API.IsSANOptimized())
-			}
-		case sa.FCP:
-			storageDriver = &SANStorageDriver{API: API, AWSAPI: AWSAPI, Config: *ontapConfig}
-		case sa.NVMe:
-			storageDriver = &NVMeStorageDriver{API: API, AWSAPI: AWSAPI, Config: *ontapConfig}
-		default:
-			return nil, fmt.Errorf("unsupported SAN protocol %s", driverProtocol)
+		if storageDriver, err = getSANStorageDriverBasedOnPersonality(
+			API.IsSANOptimized(), API.IsDisaggregated(),
+			driverProtocol, ontapConfig,
+			API, AWSAPI); err != nil {
+			return nil, fmt.Errorf("error initializing %s driver: %v", ontapConfig.StorageDriverName, err)
 		}
 
 	default:
 		return nil, fmt.Errorf("unsupported ONTAP driver type %s", ontapConfig.StorageDriverName)
 	}
 
 	Logc(ctx).WithFields(LogFields{
-		"disaggregated": API.IsDisaggregated(),
-		"sanOptimized":  API.IsSANOptimized(),
+		"disaggregated":  API.IsDisaggregated(),
+		"sanOptimized":   API.IsSANOptimized(),
+		"driverProtocol": driverProtocol,
 	}).Infof("ONTAP factory creating %T backend.", storageDriver)
 
 	return storageDriver, nil
 }
 
+func getSANStorageDriverBasedOnPersonality(
+	sanOptimized, disaggregated bool,
+	driverProtocol string, ontapConfig *drivers.OntapStorageDriverConfig,
+	api api.OntapAPI, awsapi awsapi.AWSAPI,
+) (storage.Driver, error) {
+	isASAr2 := false
+	if sanOptimized && disaggregated {
+		// Setup personality to be used by ASUP
+		if ontapConfig.Flags == nil {
+			ontapConfig.Flags = make(map[string]string)
+		}
+		ontapConfig.Flags[FlagPersonality] = PersonalityASAr2
+		isASAr2 = true
+	}
+
+	switch driverProtocol {
+	case sa.ISCSI:
+		if isASAr2 {
+			// ASAr2 iSCSI SAN driver
+			return &ASAStorageDriver{API: api, Config: *ontapConfig}, nil
+		} else {
+			// Unified iSCSI SAN driver
+			return &SANStorageDriver{API: api, AWSAPI: awsapi, Config: *ontapConfig}, nil
+		}
+	case sa.NVMe:
+		// Unified NVMe SAN driver
+		return &NVMeStorageDriver{API: api, AWSAPI: awsapi, Config: *ontapConfig}, nil
+	case sa.FCP:
+		return &SANStorageDriver{API: api, AWSAPI: awsapi, Config: *ontapConfig}, nil
+	default:
+		return nil, fmt.Errorf("unsupported SAN protocol %s", driverProtocol)
+	}
+}
+
 func getEmptyStorageDriver(driverName, driverProtocol string) (storage.Driver, error) {
 	var storageDriver storage.Driver
 

storage_drivers/ontap/ontap_factory_test.go

@@ -6,9 +6,12 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"go.uber.org/mock/gomock"
 
 	"github.com/netapp/trident/config"
+	mockapi "github.com/netapp/trident/mocks/mock_storage_drivers/mock_ontap"
 	sa "github.com/netapp/trident/storage_attribute"
+	drivers "github.com/netapp/trident/storage_drivers"
 )
 
 func TestGetDriverProtocol(t *testing.T) {
@@ -41,3 +44,40 @@ func TestGetDriverProtocol(t *testing.T) {
 	_, err = GetDriverProtocol(config.OntapNASStorageDriverName, `{}`)
 	assert.NoError(t, err, "Failed to get protocol type.")
 }
+
+func TestGetSANStorageDriverBasedOnPersonality(t *testing.T) {
+	ontapConfig := drivers.OntapStorageDriverConfig{
+		CommonStorageDriverConfig: &drivers.CommonStorageDriverConfig{},
+	}
+	mockCtrl := gomock.NewController(t)
+	mockAPI := mockapi.NewMockOntapAPI(mockCtrl)
+	mockAWSAPI := mockapi.NewMockAWSAPI(mockCtrl)
+
+	tests := []struct {
+		sanOptimized       bool
+		disaggregated      bool
+		driverProtocol     string
+		expectedDriverType interface{}
+		expectError        bool
+	}{
+		{true, true, sa.ISCSI, &ASAStorageDriver{}, false},
+		{true, false, sa.ISCSI, &SANStorageDriver{}, false},
+		{false, false, sa.ISCSI, &SANStorageDriver{}, false},
+		{false, false, sa.NVMe, &NVMeStorageDriver{}, false},
+		{false, false, sa.FCP, &SANStorageDriver{}, false},
+		{false, false, "unsupported", nil, true},
+	}
+
+	for _, test := range tests {
+		driver, err := getSANStorageDriverBasedOnPersonality(
+			test.sanOptimized, test.disaggregated, test.driverProtocol, &ontapConfig, mockAPI, mockAWSAPI,
+		)
+
+		if test.expectError {
+			assert.Error(t, err, "expected error but got none")
+		} else {
+			assert.NoError(t, err, "expected no error but got one")
+			assert.IsType(t, test.expectedDriverType, driver, "storage driver type does not match")
+		}
+	}
+}