diff --git a/storage_drivers/ontap/ontap_common_test.go b/storage_drivers/ontap/ontap_common_test.go index 597a78b9e..3402e8694 100644 --- a/storage_drivers/ontap/ontap_common_test.go +++ b/storage_drivers/ontap/ontap_common_test.go @@ -35,7 +35,7 @@ import ( // } func NewAPIResponse( - client, version, status, reason, errno string, + _, _, status, reason, errno string, ) *api.APIResponse { return api.NewAPIResponse(status, reason, errno) } @@ -1183,7 +1183,7 @@ func TestValidateStoragePools_Valid_OntapNAS(t *testing.T) { mockCtrl = gomock.NewController(t) mockAPI = mockapi.NewMockOntapAPI(mockCtrl) mockAPI.EXPECT().SVMName().AnyTimes().Return("SVM1") - storageDriverSAN := newTestOntapSANDriver(vserverAdminHost, "443", vserverAggrName, true, mockAPI) + storageDriverSAN := newTestOntapSANDriver(vserverAdminHost, "443", vserverAggrName, true, nil, mockAPI) physicalPools = map[string]storage.Pool{} virtualPools = map[string]storage.Pool{"test": getValidOntapSANPool()} storageDriver.virtualPools = virtualPools @@ -1200,7 +1200,7 @@ func TestValidateStoragePools_Valid_OntapNAS(t *testing.T) { mockCtrl = gomock.NewController(t) mockAPI = mockapi.NewMockOntapAPI(mockCtrl) mockAPI.EXPECT().SVMName().AnyTimes().Return("SVM1") - storageDriverSAN = newTestOntapSANDriver(vserverAdminHost, "443", vserverAggrName, true, mockAPI) + storageDriverSAN = newTestOntapSANDriver(vserverAdminHost, "443", vserverAggrName, true, nil, mockAPI) physicalPools = map[string]storage.Pool{} virtualPools = map[string]storage.Pool{"test": getValidOntapSANPool()} storageDriver.virtualPools = virtualPools @@ -1217,7 +1217,7 @@ func TestValidateStoragePools_Valid_OntapNAS(t *testing.T) { mockCtrl = gomock.NewController(t) mockAPI = mockapi.NewMockOntapAPI(mockCtrl) mockAPI.EXPECT().SVMName().AnyTimes().Return("SVM1") - storageDriverSAN = newTestOntapSANDriver(vserverAdminHost, "443", vserverAggrName, true, mockAPI) + storageDriverSAN = newTestOntapSANDriver(vserverAdminHost, "443", vserverAggrName, true, nil, mockAPI) physicalPools = map[string]storage.Pool{} virtualPools = map[string]storage.Pool{"test": getValidOntapSANPool()} storageDriver.virtualPools = virtualPools @@ -1233,7 +1233,7 @@ func TestValidateStoragePools_Valid_OntapNAS(t *testing.T) { mockCtrl = gomock.NewController(t) mockAPI = mockapi.NewMockOntapAPI(mockCtrl) mockAPI.EXPECT().SVMName().AnyTimes().Return("SVM1") - storageDriverSAN = newTestOntapSANDriver(vserverAdminHost, "443", vserverAggrName, true, mockAPI) + storageDriverSAN = newTestOntapSANDriver(vserverAdminHost, "443", vserverAggrName, true, nil, mockAPI) physicalPools = map[string]storage.Pool{} virtualPools = map[string]storage.Pool{"test": getValidOntapSANPool()} storageDriver.virtualPools = virtualPools @@ -1255,7 +1255,7 @@ func TestValidateStoragePools_LUKS(t *testing.T) { mockAPI := mockapi.NewMockOntapAPI(mockCtrl) mockAPI.EXPECT().SVMName().AnyTimes().Return("SVM1") - sanStorageDriver := newTestOntapSANDriver(vserverAdminHost, "443", vserverAggrName, false, mockAPI) + sanStorageDriver := newTestOntapSANDriver(vserverAdminHost, "443", vserverAggrName, false, nil, mockAPI) pool := getValidOntapSANPool() pool.InternalAttributes()[LUKSEncryption] = "true" physicalPools := map[string]storage.Pool{} @@ -1271,7 +1271,7 @@ func TestValidateStoragePools_LUKS(t *testing.T) { mockAPI = mockapi.NewMockOntapAPI(mockCtrl) mockAPI.EXPECT().SVMName().AnyTimes().Return("SVM1") - sanEcoStorageDriver := newTestOntapSanEcoDriver(vserverAdminHost, "443", vserverAggrName, false, mockAPI) + sanEcoStorageDriver := newTestOntapSanEcoDriver(vserverAdminHost, "443", vserverAggrName, false, nil, mockAPI) pool = getValidOntapSANPool() pool.InternalAttributes()[LUKSEncryption] = "true" physicalPools = map[string]storage.Pool{} @@ -1300,7 +1300,7 @@ func TestValidateStoragePools_LUKS(t *testing.T) { mockAPI = mockapi.NewMockOntapAPI(mockCtrl) mockAPI.EXPECT().SVMName().AnyTimes().Return("SVM1") - sanStorageDriver = newTestOntapSANDriver(vserverAdminHost, "443", vserverAggrName, false, mockAPI) + sanStorageDriver = newTestOntapSANDriver(vserverAdminHost, "443", vserverAggrName, false, nil, mockAPI) pool = getValidOntapSANPool() pool.InternalAttributes()[LUKSEncryption] = "invalid-not-a-bool" physicalPools = map[string]storage.Pool{} @@ -2657,11 +2657,11 @@ func TestIsDefaultAuthTypeDeny(t *testing.T) { assert.False(t, actual) } -func mockValidate(ctx context.Context) error { +func mockValidate(_ context.Context) error { return nil } -func mockValidate_Error(ctx context.Context) error { +func mockValidate_Error(_ context.Context) error { return fmt.Errorf("Error while validating") } @@ -3539,27 +3539,27 @@ func TestGoString(t *testing.T) { assert.Equal(t, expected, result) } -func mockVolumeExists(ctx context.Context, name string) (bool, error) { +func mockVolumeExists(_ context.Context, _ string) (bool, error) { return true, nil } -func mockVolumeSize(ctx context.Context, name string) (uint64, error) { +func mockVolumeSize(_ context.Context, _ string) (uint64, error) { return 1024, nil } -func mockVolumeExistsError(ctx context.Context, name string) (bool, error) { +func mockVolumeExistsError(_ context.Context, _ string) (bool, error) { return true, fmt.Errorf("VolumeExistsError") } -func mockVolumeSizeError(ctx context.Context, name string) (uint64, error) { +func mockVolumeSizeError(_ context.Context, _ string) (uint64, error) { return 1024, fmt.Errorf("VolumeSizeError") } -func mockVolumeSizeLarger(ctx context.Context, name string) (uint64, error) { +func mockVolumeSizeLarger(_ context.Context, _ string) (uint64, error) { return 10000, nil } -func mockVolumeInfo(ctx context.Context, name string) (*api.Volume, error) { +func mockVolumeInfo(_ context.Context, name string) (*api.Volume, error) { return &api.Volume{ Name: name, SnapshotPolicy: "fakePolicy", @@ -4643,7 +4643,7 @@ func TestDiscoverBackendAggrNamesCommon(t *testing.T) { mockAPI.EXPECT().GetSVMAggregateNames(ctx).Return([]string{}, nil) mockAPI.EXPECT().SVMName().AnyTimes().Return("SVM1") storageDriver := newTestOntapSANDriver(vserverAdminHost, "443", vserverAggrName, - false, mockAPI) + false, nil, mockAPI) aggr, err := discoverBackendAggrNamesCommon(ctx, storageDriver) @@ -4656,7 +4656,7 @@ func TestDiscoverBackendAggrNamesCommon(t *testing.T) { nil) mockAPI.EXPECT().SVMName().AnyTimes().Return("SVM1") storageDriver = newTestOntapSANDriver(vserverAdminHost, "443", vserverAggrName, - false, mockAPI) + false, nil, mockAPI) expected := ONTAPTEST_VSERVER_AGGR_NAME aggr, err = discoverBackendAggrNamesCommon(ctx, storageDriver) @@ -4669,7 +4669,7 @@ func TestDiscoverBackendAggrNamesCommon(t *testing.T) { mockAPI.EXPECT().GetSVMAggregateNames(ctx).Return([]string{}, nil).Return([]string{"aggr1"}, nil) mockAPI.EXPECT().SVMName().AnyTimes().Return("SVM1") storageDriver = newTestOntapSANDriver(vserverAdminHost, "443", vserverAggrName, - false, mockAPI) + false, nil, mockAPI) aggr, err = discoverBackendAggrNamesCommon(ctx, storageDriver) @@ -4677,11 +4677,11 @@ func TestDiscoverBackendAggrNamesCommon(t *testing.T) { assert.Error(t, err) } -func mockCloneSplitStart(ctx context.Context, cloneName string) error { +func mockCloneSplitStart(_ context.Context, _ string) error { return nil } -func mockCloneSplitStart_error(ctx context.Context, cloneName string) error { +func mockCloneSplitStart_error(_ context.Context, _ string) error { return fmt.Errorf("CloneSplitStart returned error") } @@ -4776,7 +4776,7 @@ func TestGetVserverAggrAttributes(t *testing.T) { mockAPI := mockapi.NewMockOntapAPI(mockCtrl) mockAPI.EXPECT().SVMName().AnyTimes().Return("SVM1") storageDriver := newTestOntapSANDriver(vserverAdminHost, "443", vserverAggrName, - false, mockAPI) + false, nil, mockAPI) pool1 := storage.NewStoragePool(nil, "dummyPool1") pool1.Attributes()[sa.BackendType] = sa.NewStringOffer("dummyBackend") @@ -4835,7 +4835,7 @@ func TestInitializeStoragePoolsCommon(t *testing.T) { mockAPI.EXPECT().SVMName().AnyTimes().Return("SVM1") storageDriver := newTestOntapSANDriver(vserverAdminHost, "443", vserverAggrName, - false, mockAPI) + false, nil, mockAPI) storageDriver.Config.Aggregate = "" pool1 := storage.NewStoragePool(nil, "dummyPool1") pool1.Attributes()[sa.Media] = sa.NewStringOffer("hdd") @@ -5147,7 +5147,7 @@ func TestNewOntapTelemetry(t *testing.T) { assert.Equal(t, storageDriver.Name(), telemetry.Plugin) } -func MockGetVolumeInfo(ctx context.Context, volName string) (volume *api.Volume, err error) { +func MockGetVolumeInfo(_ context.Context, volName string) (volume *api.Volume, err error) { volume = &api.Volume{ Name: volName, SnapshotPolicy: "fakePolicy", diff --git a/storage_drivers/ontap/ontap_nas_flexgroup.go b/storage_drivers/ontap/ontap_nas_flexgroup.go index 55a26f27b..6aa085bb5 100644 --- a/storage_drivers/ontap/ontap_nas_flexgroup.go +++ b/storage_drivers/ontap/ontap_nas_flexgroup.go @@ -21,6 +21,7 @@ import ( drivers "github.com/netapp/trident/storage_drivers" "github.com/netapp/trident/storage_drivers/ontap/api" "github.com/netapp/trident/storage_drivers/ontap/api/azgo" + "github.com/netapp/trident/storage_drivers/ontap/awsapi" "github.com/netapp/trident/utils" "github.com/netapp/trident/utils/errors" ) @@ -32,6 +33,7 @@ type NASFlexGroupStorageDriver struct { initialized bool Config drivers.OntapStorageDriverConfig API api.OntapAPI + AWSAPI awsapi.AWSAPI telemetry *Telemetry physicalPool storage.Pool @@ -87,6 +89,16 @@ func (d *NASFlexGroupStorageDriver) Initialize( } d.Config = *config + // Initialize AWS API if applicable. + // Unit tests mock the API layer, so we only use the real API interface if it doesn't already exist. + if d.AWSAPI == nil { + d.AWSAPI, err = initializeAWSDriver(ctx, config) + if err != nil { + return fmt.Errorf("error initializing %s AWS driver; %v", d.Name(), err) + } + } + + // Initialize the ONTAP API. // Unit tests mock the API layer, so we only use the real API interface if it doesn't already exist. if d.API == nil { d.API, err = InitializeOntapDriver(ctx, config) @@ -976,6 +988,16 @@ func (d *NASFlexGroupStorageDriver) Destroy(ctx context.Context, volConfig *stor Logd(ctx, d.Name(), d.Config.DebugTraceFlags["method"]).WithFields(fields).Trace(">>>> Destroy") defer Logd(ctx, d.Name(), d.Config.DebugTraceFlags["method"]).WithFields(fields).Trace("<<<< Destroy") + // If volume exists and this is FSx, try the FSx SDK first so that any backup mirror relationship + // is cleaned up. If the volume isn't found, then FSx may not know about it yet, so just try the + // underlying ONTAP delete call. Any race condition with FSx will be resolved on a retry. + if d.AWSAPI != nil { + err := destroyFSxVolume(ctx, d.AWSAPI, volConfig, &d.Config) + if err == nil || !errors.IsNotFoundError(err) { + return err + } + } + // This call is async, but we will receive an immediate error back for anything but very rare volume deletion // failures. Failures in this category are almost certainly likely to be beyond our capability to fix or even // diagnose, so we defer to the ONTAP cluster admin diff --git a/storage_drivers/ontap/ontap_nas_flexgroup_test.go b/storage_drivers/ontap/ontap_nas_flexgroup_test.go index c71f120ed..32b7901a0 100644 --- a/storage_drivers/ontap/ontap_nas_flexgroup_test.go +++ b/storage_drivers/ontap/ontap_nas_flexgroup_test.go @@ -21,11 +21,12 @@ import ( drivers "github.com/netapp/trident/storage_drivers" "github.com/netapp/trident/storage_drivers/ontap/api" "github.com/netapp/trident/storage_drivers/ontap/api/azgo" + "github.com/netapp/trident/storage_drivers/ontap/awsapi" "github.com/netapp/trident/utils" ) func newTestOntapNASFlexgroupDriver( - vserverAdminHost, vserverAdminPort, vserverAggrName string, driverContext tridentconfig.DriverContext, useREST bool, + vserverAdminHost, vserverAdminPort, vserverAggrName string, driverContext tridentconfig.DriverContext, useREST bool, fsxId *string, ) *NASFlexGroupStorageDriver { config := &drivers.OntapStorageDriverConfig{} sp := func(s string) *string { return &s } @@ -44,6 +45,11 @@ func newTestOntapNASFlexgroupDriver( config.UseREST = useREST config.FlexGroupAggregateList = []string{"aggr1", "aggr2"} + if fsxId != nil { + config.AWSConfig = &drivers.AWSConfig{} + config.AWSConfig.FSxFilesystemID = *fsxId + } + nasDriver := &NASFlexGroupStorageDriver{} nasDriver.Config = *config @@ -71,6 +77,14 @@ func newTestOntapNASFlexgroupDriver( return nasDriver } +func newMockAWSOntapNASFlexgroupDriver(t *testing.T) (*mockapi.MockOntapAPI, *mockapi.MockAWSAPI, *NASFlexGroupStorageDriver) { + mockCtrl := gomock.NewController(t) + mockAPI, driver := newMockOntapNASFlexgroupDriver(t) + mockAWSAPI := mockapi.NewMockAWSAPI(mockCtrl) + driver.AWSAPI = mockAWSAPI + return mockAPI, mockAWSAPI, driver +} + func newMockOntapNASFlexgroupDriver(t *testing.T) (*mockapi.MockOntapAPI, *NASFlexGroupStorageDriver) { mockCtrl := gomock.NewController(t) mockAPI := mockapi.NewMockOntapAPI(mockCtrl) @@ -78,8 +92,9 @@ func newMockOntapNASFlexgroupDriver(t *testing.T) (*mockapi.MockOntapAPI, *NASFl vserverAdminHost := ONTAPTEST_LOCALHOST vserverAdminPort := "0" vserverAggrName := ONTAPTEST_VSERVER_AGGR_NAME + fsxId := FSX_ID - driver := newTestOntapNASFlexgroupDriver(vserverAdminHost, vserverAdminPort, vserverAggrName, "CSI", false) + driver := newTestOntapNASFlexgroupDriver(vserverAdminHost, vserverAdminPort, vserverAggrName, "CSI", false, &fsxId) driver.API = mockAPI return mockAPI, driver } @@ -182,7 +197,7 @@ func TestOntapNasFlexgroupStorageDriverInitialize_WithTwoAuthMethods(t *testing. "clientprivatekey": "dummy-client-private-key" }` ontapNasDriver := newTestOntapNASFlexgroupDriver(vserverAdminHost, vserverAdminPort, vserverAggrName, - "CSI", false) + "CSI", false, nil) result := ontapNasDriver.Initialize(ctx, "CSI", configJSON, commonConfig, map[string]string{}, BackendUUID) @@ -219,7 +234,7 @@ func TestOntapNasFlexgroupStorageDriverInitialize_WithTwoAuthMethodsWithSecrets( "clientcertificate": "dummy-certificate", } ontapNasDriver := newTestOntapNASFlexgroupDriver(vserverAdminHost, vserverAdminPort, vserverAggrName, - "CSI", false) + "CSI", false, nil) result := ontapNasDriver.Initialize(ctx, "CSI", configJSON, commonConfig, secrets, BackendUUID) @@ -256,7 +271,7 @@ func TestOntapNasFlexgroupStorageDriverInitialize_WithTwoAuthMethodsWithConfigAn "clientcertificate": "dummy-certificate", } ontapNasDriver := newTestOntapNASFlexgroupDriver(vserverAdminHost, vserverAdminPort, vserverAggrName, - "CSI", false) + "CSI", false, nil) result := ontapNasDriver.Initialize(ctx, "CSI", configJSON, commonConfig, secrets, BackendUUID) @@ -1225,6 +1240,67 @@ func TestOntapNasFlexgroupStorageDriverVolumeCreate_FlexgroupSize(t *testing.T) } } +func TestOntapNasFlexgroupStorageDriverVolumeDestroy_FSx(t *testing.T) { + svmName := "SVM1" + volName := "testVol" + volNameInternal := volName + "Internal" + mockAPI, mockAWSAPI, driver := newMockAWSOntapNASFlexgroupDriver(t) + + volConfig := &storage.VolumeConfig{ + Size: "400g", + Name: volName, + InternalName: volNameInternal, + Encryption: "false", + FileSystem: "xfs", + } + + assert.NotNil(t, mockAPI) + + tests := []struct { + message string + nasType string + smbShare string + state string + }{ + {"Test NFS volume in FSx in available state", "nfs", "", "AVAILABLE"}, + {"Test NFS volume in FSx in deleting state", "nfs", "", "DELETING"}, + {"Test NFS volume does not exist in FSx", "nfs", "", ""}, + {"Test SMB volume does not exist in FSx", "smb", volConfig.InternalName, ""}, + } + + for _, test := range tests { + t.Run(test.message, func(t *testing.T) { + vol := awsapi.Volume{ + State: test.state, + } + isVolumeExists := vol.State != "" + mockAWSAPI.EXPECT().VolumeExists(ctx, volConfig).Return(isVolumeExists, &vol, nil) + if isVolumeExists { + mockAWSAPI.EXPECT().WaitForVolumeStates( + ctx, &vol, []string{awsapi.StateDeleted}, []string{awsapi.StateFailed}, awsapi.RetryDeleteTimeout).Return("", nil) + if vol.State == awsapi.StateAvailable { + mockAWSAPI.EXPECT().DeleteVolume(ctx, &vol).Return(nil) + } + } else { + mockAPI.EXPECT().SVMName().AnyTimes().Return(svmName) + mockAPI.EXPECT().FlexgroupDestroy(ctx, volConfig.InternalName, true).Return(nil) + if test.nasType == sa.SMB { + if test.smbShare == "" { + mockAPI.EXPECT().SMBShareExists(ctx, volConfig.InternalName).Return(true, nil) + mockAPI.EXPECT().SMBShareDestroy(ctx, volConfig.InternalName).Return(nil) + } + driver.Config.NASType = sa.SMB + driver.Config.SMBShare = test.smbShare + } + } + + result := driver.Destroy(ctx, volConfig) + + assert.NoError(t, result) + }) + } +} + func TestOntapNasFlexgroupStorageDriverVolumeDestroy(t *testing.T) { svmName := "SVM1" volName := "testVol" @@ -3020,7 +3096,7 @@ func TestOntapNasFlexgroupStorageDriverGetUpdateType(t *testing.T) { } newDriver := newTestOntapNASFlexgroupDriver(ONTAPTEST_LOCALHOST, "0", ONTAPTEST_VSERVER_AGGR_NAME, - "CSI", false) + "CSI", false, nil) newDriver.API = mockAPI prefix2 := "storage_" newDriver.Config.StoragePrefix = &prefix2 @@ -3049,14 +3125,14 @@ func TestOntapNasFlexgroupStorageDriverGetUpdateType_Failure(t *testing.T) { mockAPI, _ := newMockOntapNASFlexgroupDriver(t) mockAPI.EXPECT().SVMName().AnyTimes().Return("SVM1") - oldDriver := newTestOntapSanEcoDriver(ONTAPTEST_LOCALHOST, "0", ONTAPTEST_VSERVER_AGGR_NAME, false, mockAPI) + oldDriver := newTestOntapSanEcoDriver(ONTAPTEST_LOCALHOST, "0", ONTAPTEST_VSERVER_AGGR_NAME, false, nil, mockAPI) oldDriver.API = mockAPI prefix1 := "test_" oldDriver.Config.StoragePrefix = &prefix1 // Created a SAN driver newDriver := newTestOntapNASFlexgroupDriver(ONTAPTEST_LOCALHOST, "0", ONTAPTEST_VSERVER_AGGR_NAME, - "CSI", false) + "CSI", false, nil) newDriver.API = mockAPI prefix2 := "storage_" newDriver.Config.StoragePrefix = &prefix2 diff --git a/storage_drivers/ontap/ontap_nas_qtree.go b/storage_drivers/ontap/ontap_nas_qtree.go index 7357eeb2b..181b23035 100644 --- a/storage_drivers/ontap/ontap_nas_qtree.go +++ b/storage_drivers/ontap/ontap_nas_qtree.go @@ -22,6 +22,7 @@ import ( 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" "github.com/netapp/trident/utils" "github.com/netapp/trident/utils/crypto" "github.com/netapp/trident/utils/errors" @@ -47,6 +48,7 @@ type NASQtreeStorageDriver struct { initialized bool Config drivers.OntapStorageDriverConfig API api.OntapAPI + AWSAPI awsapi.AWSAPI telemetry *Telemetry quotaResizeMap map[string]bool flexvolNamePrefix string @@ -114,6 +116,16 @@ func (d *NASQtreeStorageDriver) Initialize( } d.Config = *config + // Initialize AWS API if applicable. + // Unit tests mock the API layer, so we only use the real API interface if it doesn't already exist. + if d.AWSAPI == nil { + d.AWSAPI, err = initializeAWSDriver(ctx, config) + if err != nil { + return fmt.Errorf("error initializing %s AWS driver; %v", d.Name(), err) + } + } + + // Initialize the ONTAP API. // Initialize ONTAP driver. Unit test uses mock driver, so initialize only if driver not already set if d.API == nil { d.API, err = InitializeOntapDriver(ctx, config) diff --git a/storage_drivers/ontap/ontap_nas_test.go b/storage_drivers/ontap/ontap_nas_test.go index e2a50bff1..b94e84554 100644 --- a/storage_drivers/ontap/ontap_nas_test.go +++ b/storage_drivers/ontap/ontap_nas_test.go @@ -1974,7 +1974,7 @@ func TestOntapNasStorageDriverGetUpdateType_Failure(t *testing.T) { mockAPI, _ := newMockOntapNASDriver(t) mockAPI.EXPECT().SVMName().AnyTimes().Return("SVM1") - oldDriver := newTestOntapSanEcoDriver(ONTAPTEST_LOCALHOST, "0", ONTAPTEST_VSERVER_AGGR_NAME, false, mockAPI) + oldDriver := newTestOntapSanEcoDriver(ONTAPTEST_LOCALHOST, "0", ONTAPTEST_VSERVER_AGGR_NAME, false, nil, mockAPI) oldDriver.API = mockAPI prefix1 := "test_" oldDriver.Config.StoragePrefix = &prefix1 diff --git a/storage_drivers/ontap/ontap_san.go b/storage_drivers/ontap/ontap_san.go index c2ce64c89..4352cb974 100644 --- a/storage_drivers/ontap/ontap_san.go +++ b/storage_drivers/ontap/ontap_san.go @@ -18,7 +18,9 @@ import ( 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" "github.com/netapp/trident/utils" + "github.com/netapp/trident/utils/errors" ) func lunPath(name string) string { @@ -31,6 +33,7 @@ type SANStorageDriver struct { Config drivers.OntapStorageDriverConfig ips []string API api.OntapAPI + AWSAPI awsapi.AWSAPI telemetry *Telemetry physicalPools map[string]storage.Pool @@ -89,6 +92,16 @@ func (d *SANStorageDriver) Initialize( } d.Config = *config + // Initialize AWS API if applicable. + // Unit tests mock the API layer, so we only use the real API interface if it doesn't already exist. + if d.AWSAPI == nil { + d.AWSAPI, err = initializeAWSDriver(ctx, config) + if err != nil { + return fmt.Errorf("error initializing %s AWS driver; %v", d.Name(), err) + } + } + + // Initialize the ONTAP API. // Unit tests mock the API layer, so we only use the real API interface if it doesn't already exist. if d.API == nil { d.API, err = InitializeOntapDriver(ctx, config) @@ -736,6 +749,16 @@ func (d *SANStorageDriver) Destroy(ctx context.Context, volConfig *storage.Volum } } + // If volume exists and this is FSx, try the FSx SDK first so that any backup mirror relationship + // is cleaned up. If the volume isn't found, then FSx may not know about it yet, so just try the + // underlying ONTAP delete call. Any race condition with FSx will be resolved on a retry. + if d.AWSAPI != nil { + err = destroyFSxVolume(ctx, d.AWSAPI, volConfig, &d.Config) + if err == nil || !errors.IsNotFoundError(err) { + return err + } + } + // If flexvol has been a snapmirror destination if err := d.API.SnapmirrorDeleteViaDestination(ctx, name, d.API.SVMName()); err != nil { if !api.IsNotFoundError(err) { diff --git a/storage_drivers/ontap/ontap_san_economy.go b/storage_drivers/ontap/ontap_san_economy.go index dfdbf06ba..ab8427847 100644 --- a/storage_drivers/ontap/ontap_san_economy.go +++ b/storage_drivers/ontap/ontap_san_economy.go @@ -18,6 +18,7 @@ import ( 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" "github.com/netapp/trident/utils" "github.com/netapp/trident/utils/crypto" "github.com/netapp/trident/utils/errors" @@ -195,6 +196,7 @@ type SANEconomyStorageDriver struct { Config drivers.OntapStorageDriverConfig ips []string API api.OntapAPI + AWSAPI awsapi.AWSAPI telemetry *Telemetry flexvolNamePrefix string helper *LUNHelper @@ -260,6 +262,16 @@ func (d *SANEconomyStorageDriver) Initialize( } d.Config = *config + // Initialize AWS API if applicable. + // Unit tests mock the API layer, so we only use the real API interface if it doesn't already exist. + if d.AWSAPI == nil { + d.AWSAPI, err = initializeAWSDriver(ctx, config) + if err != nil { + return fmt.Errorf("error initializing %s AWS driver; %v", d.Name(), err) + } + } + + // Initialize the ONTAP API. // Unit tests mock the API layer, so we only use the real API interface if it doesn't already exist. if d.API == nil { d.API, err = InitializeOntapDriver(ctx, config) @@ -956,6 +968,19 @@ func (d *SANEconomyStorageDriver) DeleteBucketIfEmpty(ctx context.Context, bucke count := len(luns) if count == 0 { + // If volume exists and this is FSx, try the FSx SDK first so that any backup mirror relationship + // is cleaned up. If the volume isn't found, then FSx may not know about it yet, so just try the + // underlying ONTAP delete call. Any race condition with FSx will be resolved on a retry. + if d.AWSAPI != nil { + volConfig := &storage.VolumeConfig{ + InternalName: bucketVol, + } + err = destroyFSxVolume(ctx, d.AWSAPI, volConfig, &d.Config) + if err == nil || !errors.IsNotFoundError(err) { + return err + } + } + // Delete the bucketVol err := d.API.VolumeDestroy(ctx, bucketVol, true) if err != nil { diff --git a/storage_drivers/ontap/ontap_san_economy_test.go b/storage_drivers/ontap/ontap_san_economy_test.go index 7d4f06266..01249701f 100644 --- a/storage_drivers/ontap/ontap_san_economy_test.go +++ b/storage_drivers/ontap/ontap_san_economy_test.go @@ -23,6 +23,7 @@ import ( 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" "github.com/netapp/trident/utils" ) @@ -169,7 +170,7 @@ func TestGetComponentsNoSnapshot(t *testing.T) { } func newTestOntapSanEcoDriver( - vserverAdminHost, vserverAdminPort, vserverAggrName string, useREST bool, apiOverride api.OntapAPI, + vserverAdminHost, vserverAdminPort, vserverAggrName string, useREST bool, fsxId *string, apiOverride api.OntapAPI, ) *SANEconomyStorageDriver { config := &drivers.OntapStorageDriverConfig{} sp := func(s string) *string { return &s } @@ -188,6 +189,11 @@ func newTestOntapSanEcoDriver( config.StoragePrefix = sp("test_") config.UseREST = useREST + if fsxId != nil { + config.AWSConfig = &drivers.AWSConfig{} + config.AWSConfig.FSxFilesystemID = *fsxId + } + sanEcoDriver := &SANEconomyStorageDriver{} sanEcoDriver.Config = *config @@ -219,6 +225,21 @@ func newTestOntapSanEcoDriver( return sanEcoDriver } +func newMockAWSOntapSanEcoDriver(t *testing.T) (*mockapi.MockOntapAPI, *mockapi.MockAWSAPI, *SANEconomyStorageDriver) { + vserverAdminHost := ONTAPTEST_LOCALHOST + vserverAdminPort := "0" + vserverAggrName := ONTAPTEST_VSERVER_AGGR_NAME + fsxId := FSX_ID + + mockCtrl := gomock.NewController(t) + mockAPI := mockapi.NewMockOntapAPI(mockCtrl) + mockAWSAPI := mockapi.NewMockAWSAPI(mockCtrl) + + driver := newTestOntapSanEcoDriver(vserverAdminHost, vserverAdminPort, vserverAggrName, false, &fsxId, mockAPI) + driver.AWSAPI = mockAWSAPI + return mockAPI, mockAWSAPI, driver +} + func newMockOntapSanEcoDriver(t *testing.T) (*mockapi.MockOntapAPI, *SANEconomyStorageDriver) { vserverAdminHost := ONTAPTEST_LOCALHOST vserverAdminPort := "0" @@ -227,7 +248,7 @@ func newMockOntapSanEcoDriver(t *testing.T) (*mockapi.MockOntapAPI, *SANEconomyS mockCtrl := gomock.NewController(t) mockAPI := mockapi.NewMockOntapAPI(mockCtrl) - driver := newTestOntapSanEcoDriver(vserverAdminHost, vserverAdminPort, vserverAggrName, false, mockAPI) + driver := newTestOntapSanEcoDriver(vserverAdminHost, vserverAdminPort, vserverAggrName, false, nil, mockAPI) return mockAPI, driver } @@ -240,7 +261,7 @@ func TestOntapSanEcoStorageDriverConfigString(t *testing.T) { mockAPI := mockapi.NewMockOntapAPI(mockCtrl) sanEcoDrivers := []SANEconomyStorageDriver{ - *newTestOntapSanEcoDriver(vserverAdminHost, vserverAdminPort, vserverAggrName, false, mockAPI), + *newTestOntapSanEcoDriver(vserverAdminHost, vserverAdminPort, vserverAggrName, false, nil, mockAPI), } sensitiveIncludeList := map[string]string{ @@ -335,7 +356,7 @@ func TestOntapSanEconomyTerminate(t *testing.T) { api.FakeIgroups[driverInfo.igroupName] = igroupsIQNMap - sanEcoStorageDriver := newTestOntapSanEcoDriver(vserverAdminHost, port, vserverAggrName, false, nil) + sanEcoStorageDriver := newTestOntapSanEcoDriver(vserverAdminHost, port, vserverAggrName, false, nil, nil) sanEcoStorageDriver.Config.IgroupName = driverInfo.igroupName sanEcoStorageDriver.telemetry = nil ontapSanDrivers = append(ontapSanDrivers, *sanEcoStorageDriver) @@ -1526,6 +1547,25 @@ func TestOntapSanEconomyVolumeDestroy_DockerContext_LunMapInfoError(t *testing.T assert.Error(t, result) } +func TestOntapSanEconomyVolumeDeleteBucketIfEmpty_Fsx(t *testing.T) { + var bucketVol string = "volumeName" + mockAPI, mockAWSAPI, d := newMockAWSOntapSanEcoDriver(t) + mockAPI.EXPECT().LunList(ctx, gomock.Any()).Times(1).Return(api.Luns{}, nil) + + vol := awsapi.Volume{ + State: "AVAILABLE", + } + volConfig := &storage.VolumeConfig{ + InternalName: bucketVol, + } + mockAWSAPI.EXPECT().VolumeExists(ctx, volConfig).Return(true, &vol, nil) + mockAWSAPI.EXPECT().WaitForVolumeStates(ctx, &vol, []string{awsapi.StateDeleted}, []string{awsapi.StateFailed}, awsapi.RetryDeleteTimeout).Return("", nil) + mockAWSAPI.EXPECT().DeleteVolume(ctx, &vol).Return(nil) + result := d.DeleteBucketIfEmpty(ctx, bucketVol) + + assert.NoError(t, result) +} + func TestOntapSanEconomyVolumeDeleteBucketIfEmpty(t *testing.T) { mockAPI, d := newMockOntapSanEcoDriver(t) @@ -1725,7 +1765,7 @@ func TestOntapSanEconomyVolumeUnpublish_LegacyVolume(t *testing.T) { mockCtrl := gomock.NewController(t) mockAPI := mockapi.NewMockOntapAPI(mockCtrl) - d := newTestOntapSanEcoDriver(ONTAPTEST_LOCALHOST, "0", ONTAPTEST_VSERVER_AGGR_NAME, true, mockAPI) + d := newTestOntapSanEcoDriver(ONTAPTEST_LOCALHOST, "0", ONTAPTEST_VSERVER_AGGR_NAME, true, nil, mockAPI) volConfig := &storage.VolumeConfig{ InternalName: "lun0", @@ -1749,7 +1789,7 @@ func TestOntapSanEconomyVolumeUnpublish_LunListFails(t *testing.T) { mockCtrl := gomock.NewController(t) mockAPI := mockapi.NewMockOntapAPI(mockCtrl) - d := newTestOntapSanEcoDriver(ONTAPTEST_LOCALHOST, "0", ONTAPTEST_VSERVER_AGGR_NAME, true, mockAPI) + d := newTestOntapSanEcoDriver(ONTAPTEST_LOCALHOST, "0", ONTAPTEST_VSERVER_AGGR_NAME, true, nil, mockAPI) d.helper = NewTestLUNHelper("", tridentconfig.ContextCSI) apiError := fmt.Errorf("api error") @@ -1779,7 +1819,7 @@ func TestOntapSanEconomyVolumeUnpublish_LUNDoesNotExist(t *testing.T) { mockCtrl := gomock.NewController(t) mockAPI := mockapi.NewMockOntapAPI(mockCtrl) - d := newTestOntapSanEcoDriver(ONTAPTEST_LOCALHOST, "0", ONTAPTEST_VSERVER_AGGR_NAME, true, mockAPI) + d := newTestOntapSanEcoDriver(ONTAPTEST_LOCALHOST, "0", ONTAPTEST_VSERVER_AGGR_NAME, true, nil, mockAPI) d.helper = NewTestLUNHelper("", tridentconfig.ContextCSI) volumeName := "lun0" volConfig := &storage.VolumeConfig{ @@ -1807,7 +1847,7 @@ func TestOntapSanEconomyVolumeUnpublish_LUNMapInfoFails(t *testing.T) { mockCtrl := gomock.NewController(t) mockAPI := mockapi.NewMockOntapAPI(mockCtrl) - d := newTestOntapSanEcoDriver(ONTAPTEST_LOCALHOST, "0", ONTAPTEST_VSERVER_AGGR_NAME, true, mockAPI) + d := newTestOntapSanEcoDriver(ONTAPTEST_LOCALHOST, "0", ONTAPTEST_VSERVER_AGGR_NAME, true, nil, mockAPI) d.helper = NewTestLUNHelper("", tridentconfig.ContextCSI) apiError := fmt.Errorf("api error") @@ -1848,7 +1888,7 @@ func TestOntapSanEconomyVolumeUnpublish_IgroupListLUNsMappedFails(t *testing.T) mockCtrl := gomock.NewController(t) mockAPI := mockapi.NewMockOntapAPI(mockCtrl) - d := newTestOntapSanEcoDriver(ONTAPTEST_LOCALHOST, "0", ONTAPTEST_VSERVER_AGGR_NAME, true, mockAPI) + d := newTestOntapSanEcoDriver(ONTAPTEST_LOCALHOST, "0", ONTAPTEST_VSERVER_AGGR_NAME, true, nil, mockAPI) d.helper = NewTestLUNHelper("", tridentconfig.ContextCSI) apiError := fmt.Errorf("api error") @@ -1891,7 +1931,7 @@ func TestOntapSanEconomyVolumeUnpublish_IgroupDestroyFails(t *testing.T) { mockCtrl := gomock.NewController(t) mockAPI := mockapi.NewMockOntapAPI(mockCtrl) - d := newTestOntapSanEcoDriver(ONTAPTEST_LOCALHOST, "0", ONTAPTEST_VSERVER_AGGR_NAME, true, mockAPI) + d := newTestOntapSanEcoDriver(ONTAPTEST_LOCALHOST, "0", ONTAPTEST_VSERVER_AGGR_NAME, true, nil, mockAPI) d.helper = NewTestLUNHelper("", tridentconfig.ContextCSI) apiError := fmt.Errorf("api error") @@ -1935,7 +1975,7 @@ func TestOntapSanEconomyVolumeUnpublishX(t *testing.T) { mockCtrl := gomock.NewController(t) mockAPI := mockapi.NewMockOntapAPI(mockCtrl) - d := newTestOntapSanEcoDriver(ONTAPTEST_LOCALHOST, "0", ONTAPTEST_VSERVER_AGGR_NAME, true, mockAPI) + d := newTestOntapSanEcoDriver(ONTAPTEST_LOCALHOST, "0", ONTAPTEST_VSERVER_AGGR_NAME, true, nil, mockAPI) d.helper = NewTestLUNHelper("", tridentconfig.ContextCSI) volumeName := "lun0" @@ -2077,7 +2117,7 @@ func TestOntapSanEconomyVolumeUnpublish(t *testing.T) { mockCtrl := gomock.NewController(t) mockAPI := mockapi.NewMockOntapAPI(mockCtrl) mockAPI.EXPECT().SVMName().AnyTimes().Return("SVM1") - d := newTestOntapSanEcoDriver(ONTAPTEST_LOCALHOST, "0", ONTAPTEST_VSERVER_AGGR_NAME, true, mockAPI) + d := newTestOntapSanEcoDriver(ONTAPTEST_LOCALHOST, "0", ONTAPTEST_VSERVER_AGGR_NAME, true, nil, mockAPI) d.API = mockAPI d.helper = NewTestLUNHelper("", tridentconfig.ContextCSI) @@ -2101,7 +2141,7 @@ func TestDriverCanSnapshot(t *testing.T) { vserverAggrName := ONTAPTEST_VSERVER_AGGR_NAME mockCtrl := gomock.NewController(t) mockAPI := mockapi.NewMockOntapAPI(mockCtrl) - d := newTestOntapSanEcoDriver(vserverAdminHost, vserverAdminPort, vserverAggrName, false, mockAPI) + d := newTestOntapSanEcoDriver(vserverAdminHost, vserverAdminPort, vserverAggrName, false, nil, mockAPI) result := d.CanSnapshot(ctx, nil, nil) @@ -3307,7 +3347,7 @@ func TestGetUpdateType_OtherChanges(t *testing.T) { mockCtrl := gomock.NewController(t) mockAPI := mockapi.NewMockOntapAPI(mockCtrl) - oldDriver := newTestOntapSanEcoDriver(ONTAPTEST_LOCALHOST, "0", ONTAPTEST_VSERVER_AGGR_NAME, true, mockAPI) + oldDriver := newTestOntapSanEcoDriver(ONTAPTEST_LOCALHOST, "0", ONTAPTEST_VSERVER_AGGR_NAME, true, nil, mockAPI) oldDriver.API = mockAPI prefix1 := "storagePrefix_" oldDriver.Config.StoragePrefix = &prefix1 @@ -3319,7 +3359,7 @@ func TestGetUpdateType_OtherChanges(t *testing.T) { oldDriver.Config.Username = "oldUser" oldDriver.Config.Password = "oldPassword" - newDriver := newTestOntapSanEcoDriver(ONTAPTEST_LOCALHOST, "0", ONTAPTEST_VSERVER_AGGR_NAME, true, mockAPI) + newDriver := newTestOntapSanEcoDriver(ONTAPTEST_LOCALHOST, "0", ONTAPTEST_VSERVER_AGGR_NAME, true, nil, mockAPI) oldDriver.API = mockAPI prefix2 := "storagePREFIX_" @@ -3348,7 +3388,7 @@ func TestGetUpdateType_Failure(t *testing.T) { mockCtrl := gomock.NewController(t) mockAPI := mockapi.NewMockOntapAPI(mockCtrl) oldDriver := newTestOntapNASDriver(ONTAPTEST_LOCALHOST, "0", ONTAPTEST_VSERVER_AGGR_NAME, "CSI", false, nil) - newDriver := newTestOntapSanEcoDriver(ONTAPTEST_LOCALHOST, "0", ONTAPTEST_VSERVER_AGGR_NAME, true, mockAPI) + newDriver := newTestOntapSanEcoDriver(ONTAPTEST_LOCALHOST, "0", ONTAPTEST_VSERVER_AGGR_NAME, true, nil, mockAPI) expectedBitmap := &roaring.Bitmap{} expectedBitmap.Add(storage.InvalidUpdate) diff --git a/storage_drivers/ontap/ontap_san_nvme.go b/storage_drivers/ontap/ontap_san_nvme.go index caf1c5ee9..14f18493e 100644 --- a/storage_drivers/ontap/ontap_san_nvme.go +++ b/storage_drivers/ontap/ontap_san_nvme.go @@ -20,6 +20,7 @@ import ( 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" "github.com/netapp/trident/utils" "github.com/netapp/trident/utils/errors" ) @@ -34,6 +35,7 @@ type NVMeStorageDriver struct { Config drivers.OntapStorageDriverConfig ips []string API api.OntapAPI + AWSAPI awsapi.AWSAPI telemetry *Telemetry physicalPools map[string]storage.Pool @@ -110,6 +112,16 @@ func (d *NVMeStorageDriver) Initialize( return fmt.Errorf("error initializing %s driver: %v", d.Name(), err) } + // Initialize AWS API if applicable. + // Unit tests mock the API layer, so we only use the real API interface if it doesn't already exist. + if d.AWSAPI == nil { + d.AWSAPI, err = initializeAWSDriver(ctx, config) + if err != nil { + return fmt.Errorf("error initializing %s AWS driver; %v", d.Name(), err) + } + } + + // Initialize the ONTAP API. // Unit tests mock the API layer, so we only use the real API interface if it doesn't already exist. if d.API == nil { if d.API, err = InitializeOntapDriver(ctx, config); err != nil { @@ -681,6 +693,16 @@ func (d *NVMeStorageDriver) Destroy(ctx context.Context, volConfig *storage.Volu Logc(ctx).Debug("No actions for Destroy for Docker.") } + // If volume exists and this is FSx, try the FSx SDK first so that any backup mirror relationship + // is cleaned up. If the volume isn't found, then FSx may not know about it yet, so just try the + // underlying ONTAP delete call. Any race condition with FSx will be resolved on a retry. + if d.AWSAPI != nil { + err = destroyFSxVolume(ctx, d.AWSAPI, volConfig, &d.Config) + if err == nil || !errors.IsNotFoundError(err) { + return err + } + } + // If flexVol has been a snapmirror destination. if err := d.API.SnapmirrorDeleteViaDestination(ctx, name, d.API.SVMName()); err != nil { if !errors.IsNotFoundError(err) { @@ -1471,7 +1493,7 @@ func (d *NVMeStorageDriver) createNVMeNamespaceCommentString(ctx context.Context } // ParseNVMeNamespaceCommentString returns the map of attributes that were stored in namespace comment field. -func (d *NVMeStorageDriver) ParseNVMeNamespaceCommentString(ctx context.Context, comment string) (map[string]string, error) { +func (d *NVMeStorageDriver) ParseNVMeNamespaceCommentString(_ context.Context, comment string) (map[string]string, error) { // Parse the comment nsComment := map[string]map[string]string{} @@ -1530,7 +1552,7 @@ func (d *NVMeStorageDriver) namespaceSize(ctx context.Context, name string) (int } // EnablePublishEnforcement sets the publishEnforcement on older NVMe volumes. -func (d *NVMeStorageDriver) EnablePublishEnforcement(ctx context.Context, volume *storage.Volume) error { +func (d *NVMeStorageDriver) EnablePublishEnforcement(_ context.Context, volume *storage.Volume) error { volume.Config.AccessInfo.PublishEnforcement = true return nil } diff --git a/storage_drivers/ontap/ontap_san_nvme_test.go b/storage_drivers/ontap/ontap_san_nvme_test.go index 3581ef4ac..3f7874a52 100644 --- a/storage_drivers/ontap/ontap_san_nvme_test.go +++ b/storage_drivers/ontap/ontap_san_nvme_test.go @@ -16,13 +16,14 @@ import ( 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" "github.com/netapp/trident/utils" "github.com/netapp/trident/utils/errors" ) var mockIPs = []string{"0.0.0.0", "1.1.1.1"} -func newNVMeDriver(apiOverride api.OntapAPI) *NVMeStorageDriver { +func newNVMeDriver(apiOverride api.OntapAPI, awsApiOverride awsapi.AWSAPI, fsxId *string) *NVMeStorageDriver { sPrefix := "test_" config := &drivers.OntapStorageDriverConfig{} @@ -38,7 +39,12 @@ func newNVMeDriver(apiOverride api.OntapAPI) *NVMeStorageDriver { StorageDriverName: "ontap-san", } - driver := &NVMeStorageDriver{Config: *config, API: apiOverride} + if fsxId != nil { + config.AWSConfig = &drivers.AWSConfig{} + config.AWSConfig.FSxFilesystemID = *fsxId + } + + driver := &NVMeStorageDriver{Config: *config, API: apiOverride, AWSAPI: awsApiOverride} driver.telemetry = &Telemetry{ Plugin: driver.Name(), SVM: config.SVM, @@ -54,15 +60,24 @@ func newNVMeDriver(apiOverride api.OntapAPI) *NVMeStorageDriver { return driver } +func newNVMeDriverAndMockApiAndAwsApi(t *testing.T) (*NVMeStorageDriver, *mockapi.MockOntapAPI, *mockapi.MockAWSAPI) { + mockCtrl := gomock.NewController(t) + mockAPI := mockapi.NewMockOntapAPI(mockCtrl) + mockAWSAPI := mockapi.NewMockAWSAPI(mockCtrl) + fsxId := FSX_ID + + return newNVMeDriver(mockAPI, mockAWSAPI, &fsxId), mockAPI, mockAWSAPI +} + func newNVMeDriverAndMockApi(t *testing.T) (*NVMeStorageDriver, *mockapi.MockOntapAPI) { mockCtrl := gomock.NewController(t) mockAPI := mockapi.NewMockOntapAPI(mockCtrl) - return newNVMeDriver(mockAPI), mockAPI + return newNVMeDriver(mockAPI, nil, nil), mockAPI } func TestNVMeBackendName(t *testing.T) { - d := newNVMeDriver(nil) + d := newNVMeDriver(nil, nil, nil) // Backend name non-empty. d.Config.BackendName = "san-nvme-backend" @@ -82,7 +97,7 @@ func TestNVMeBackendName(t *testing.T) { } func TestNVMeInitialize_ConfigUnmarshalError(t *testing.T) { - d := newNVMeDriver(nil) + d := newNVMeDriver(nil, nil, nil) commonConfig := &drivers.CommonStorageDriverConfig{ // Version: 1, StorageDriverName: "ontap-san", @@ -203,7 +218,7 @@ func TestNVMeInitialize_Success(t *testing.T) { } func TestNVMeTerminate_Success(t *testing.T) { - d := newNVMeDriver(nil) + d := newNVMeDriver(nil, nil, nil) d.telemetry = NewOntapTelemetry(ctx, d) d.Terminate(ctx, "") @@ -222,7 +237,7 @@ func TestNVMeValidate_ReplicationValidationError(t *testing.T) { } func TestNVMeValidate_StoragePoolError(t *testing.T) { - d := newNVMeDriver(nil) + d := newNVMeDriver(nil, nil, nil) pool1 := storage.NewStoragePool(nil, "pool1") pool1.InternalAttributes()[SnapshotPolicy] = "" d.virtualPools = map[string]storage.Pool{"pool1": pool1} @@ -233,7 +248,7 @@ func TestNVMeValidate_StoragePoolError(t *testing.T) { } func TestNVMeGetStorageBackendSpecs(t *testing.T) { - d := newNVMeDriver(nil) + d := newNVMeDriver(nil, nil, nil) backend := storage.StorageBackend{} backend.SetStorage(map[string]storage.Pool{}) @@ -242,7 +257,7 @@ func TestNVMeGetStorageBackendSpecs(t *testing.T) { } func TestNVMeGetStorageBackendPhysicalPoolNames(t *testing.T) { - d := newNVMeDriver(nil) + d := newNVMeDriver(nil, nil, nil) assert.Equal(t, d.GetStorageBackendPhysicalPoolNames(ctx), []string{"pool1"}, "Physical pools are different.") } @@ -272,23 +287,23 @@ func TestNVMeGetStorageBackendPools(t *testing.T) { } func TestNVMeGetVolumeOpts(t *testing.T) { - d := newNVMeDriver(nil) + d := newNVMeDriver(nil, nil, nil) volConfig := storage.VolumeConfig{} assert.NotNil(t, d.GetVolumeOpts(ctx, &volConfig, nil), "Couldn't get VolumeOpts.") } func TestNVMeGetInternalVolumeName(t *testing.T) { - d := newNVMeDriver(nil) + d := newNVMeDriver(nil, nil, nil) assert.Equal(t, d.GetInternalVolumeName(ctx, "vol1"), "test_vol1", "Got different volume.") } func TestNVMeGetProtocol(t *testing.T) { - d := newNVMeDriver(nil) + d := newNVMeDriver(nil, nil, nil) assert.Equal(t, d.GetProtocol(ctx), tridentconfig.Block, "Incorrect protocol.") } func TestNVMeStoreConfig(t *testing.T) { - d := newNVMeDriver(nil) + d := newNVMeDriver(nil, nil, nil) persistentConfig := &storage.PersistentStorageBackendConfig{} d.StoreConfig(ctx, persistentConfig) @@ -299,7 +314,7 @@ func TestNVMeStoreConfig(t *testing.T) { } func TestNVMeGetUpdateType_InvalidUpdate(t *testing.T) { - d1 := newNVMeDriver(nil) + d1 := newNVMeDriver(nil, nil, nil) _, d2 := newMockOntapNASDriver(t) bMap := d1.GetUpdateType(ctx, d2) @@ -308,8 +323,8 @@ func TestNVMeGetUpdateType_InvalidUpdate(t *testing.T) { } func TestNVMeGetUpdateType_OtherUpdates(t *testing.T) { - d1 := newNVMeDriver(nil) - d2 := newNVMeDriver(nil) + d1 := newNVMeDriver(nil, nil, nil) + d2 := newNVMeDriver(nil, nil, nil) sPrefix := "diff" d2.Config.DataLIF = "1.1.1.1" @@ -328,7 +343,7 @@ func TestNVMeGetUpdateType_OtherUpdates(t *testing.T) { } func TestNVMeGetCommonConfig(t *testing.T) { - d := newNVMeDriver(nil) + d := newNVMeDriver(nil, nil, nil) assert.Equal(t, d.GetCommonConfig(ctx), d.Config.CommonStorageDriverConfig, "Driver configuration not found.") } @@ -418,7 +433,7 @@ func TestNVMeReestablishMirror_Errors(t *testing.T) { } func TestNVMePromoteMirror_Error(t *testing.T) { - d := newNVMeDriver(nil) + d := newNVMeDriver(nil, nil, nil) promote, err := d.PromoteMirror(ctx, "", "remoteHandle", "") @@ -427,7 +442,7 @@ func TestNVMePromoteMirror_Error(t *testing.T) { } func TestNVMeGetMirrorStatus_Error(t *testing.T) { - d := newNVMeDriver(nil) + d := newNVMeDriver(nil, nil, nil) status, err := d.GetMirrorStatus(ctx, "", "remoteHandle") @@ -436,7 +451,7 @@ func TestNVMeGetMirrorStatus_Error(t *testing.T) { } func TestNVMeReleaseMirror_Error(t *testing.T) { - d := newNVMeDriver(nil) + d := newNVMeDriver(nil, nil, nil) err := d.ReleaseMirror(ctx, "") @@ -722,6 +737,49 @@ func TestNVMeDestroy_SnapMirrorAPIError(t *testing.T) { assert.ErrorContains(t, err, "snap mirror api call failed") } +func TestNVMeDestroy_VolumeDestroy_FSx(t *testing.T) { + svmName := "SVM1" + d, mAPI, mAWSAPI := newNVMeDriverAndMockApiAndAwsApi(t) + d.Config.DriverContext = tridentconfig.ContextDocker + _, volConfig, _ := getNVMeCreateArgs(d) + + tests := []struct { + message string + nasType string + smbShare string + state string + }{ + {"Test volume in FSx in available state", "nfs", "", "AVAILABLE"}, + {"Test volume in FSx in deleting state", "nfs", "", "DELETING"}, + } + + for _, test := range tests { + t.Run(test.message, func(t *testing.T) { + vol := awsapi.Volume{ + State: test.state, + } + isVolumeExists := vol.State != "" + mAPI.EXPECT().VolumeExists(ctx, volConfig.InternalName).Return(true, nil) + mAWSAPI.EXPECT().VolumeExists(ctx, volConfig).Return(isVolumeExists, &vol, nil) + if isVolumeExists { + mAWSAPI.EXPECT().WaitForVolumeStates( + ctx, &vol, []string{awsapi.StateDeleted}, []string{awsapi.StateFailed}, awsapi.RetryDeleteTimeout).Return("", nil) + if vol.State == awsapi.StateAvailable { + mAWSAPI.EXPECT().DeleteVolume(ctx, &vol).Return(nil) + } + } else { + mAPI.EXPECT().SVMName().AnyTimes().Return(svmName) + mAPI.EXPECT().SnapmirrorDeleteViaDestination(ctx, volConfig.InternalName, svmName).Return(nil) + // mockAPI.EXPECT().SnapmirrorRelease(ctx, volConfig.InternalName, svmName).Return(nil) + mAPI.EXPECT().VolumeDestroy(ctx, volConfig.InternalName, true).Return(nil) + } + result := d.Destroy(ctx, volConfig) + + assert.NoError(t, result) + }) + } +} + func TestNVMeDestroy_VolumeDestroy(t *testing.T) { d, mAPI := newNVMeDriverAndMockApi(t) d.Config.DriverContext = tridentconfig.ContextDocker @@ -835,7 +893,7 @@ func TestNVMeGetVolumeExternalWrappers_Success(t *testing.T) { } func TestNVMeCreateNVMeNamespaceCommentString(t *testing.T) { - d := newNVMeDriver(nil) + d := newNVMeDriver(nil, nil, nil) nsAttr := map[string]string{ nsAttributeFSType: "ext4", nsAttributeLUKS: "luks", @@ -859,7 +917,7 @@ func TestNVMeCreateNVMeNamespaceCommentString(t *testing.T) { } func TestNVMeParseNVMeNamespaceCommentString(t *testing.T) { - d := newNVMeDriver(nil) + d := newNVMeDriver(nil, nil, nil) nsCommentString := `{"nsAttribute":{"LUKS":"luks","com.netapp.ndvp.fstype":"ext4","driverContext":"docker"}}` @@ -902,7 +960,7 @@ func TestGetNodeSpecificSubsystemName(t *testing.T) { func TestPublish(t *testing.T) { mockCtrl := gomock.NewController(t) mock := mockapi.NewMockOntapAPI(mockCtrl) - d := newNVMeDriver(mock) + d := newNVMeDriver(mock, nil, nil) volConfig := &storage.VolumeConfig{ Name: "fakeVolName", @@ -1027,7 +1085,7 @@ func TestPublish(t *testing.T) { func TestUnpublish(t *testing.T) { mockCtrl := gomock.NewController(t) mock := mockapi.NewMockOntapAPI(mockCtrl) - d := newNVMeDriver(mock) + d := newNVMeDriver(mock, nil, nil) volConfig := &storage.VolumeConfig{ Name: "fakeVolName", @@ -1062,7 +1120,7 @@ func TestUnpublish(t *testing.T) { func TestCreatePrepare(t *testing.T) { mockCtrl := gomock.NewController(t) mock := mockapi.NewMockOntapAPI(mockCtrl) - d := newNVMeDriver(mock) + d := newNVMeDriver(mock, nil, nil) volConfig := &storage.VolumeConfig{ Name: "fakeVolName", InternalName: "fakeInternalName", @@ -1319,10 +1377,6 @@ func TestImport(t *testing.T) { _, volConfig, _ := getNVMeCreateArgs(d) originalName := "fakeOriginalName" vol := &api.Volume{Aggregates: []string{"data"}} - type error struct { - err string - message string - } ns := &api.NVMeNamespace{Name: "/vol/cloneVol1/namespace0", Size: "100", UUID: "fakeUUID"} // Test1: Error - Error getting volume info @@ -1475,7 +1529,7 @@ func TestGetBackendState(t *testing.T) { } func TestEnablePublishEnforcement(t *testing.T) { - d := newNVMeDriver(nil) + d := newNVMeDriver(nil, nil, nil) vol := storage.Volume{Config: getVolumeConfig()} assert.True(t, d.CanEnablePublishEnforcement(), "Cannot enable publish enforcement.") diff --git a/storage_drivers/ontap/ontap_san_test.go b/storage_drivers/ontap/ontap_san_test.go index 884a93b3b..840f6f1ae 100644 --- a/storage_drivers/ontap/ontap_san_test.go +++ b/storage_drivers/ontap/ontap_san_test.go @@ -21,6 +21,7 @@ import ( 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" "github.com/netapp/trident/utils" "github.com/netapp/trident/utils/errors" ) @@ -45,11 +46,25 @@ func getVolumeConfig() *storage.VolumeConfig { } } +func newMockAWSOntapSANDriver(t *testing.T) (*mockapi.MockOntapAPI, *mockapi.MockAWSAPI, *SANStorageDriver) { + mockCtrl := gomock.NewController(t) + mockAPI := mockapi.NewMockOntapAPI(mockCtrl) + mockAWSAPI := mockapi.NewMockAWSAPI(mockCtrl) + + fsxId := FSX_ID + driver := newTestOntapSANDriver(ONTAPTEST_LOCALHOST, "0", ONTAPTEST_VSERVER_AGGR_NAME, true, &fsxId, mockAPI) + driver.API = mockAPI + driver.ips = []string{"127.0.0.1"} + + driver.AWSAPI = mockAWSAPI + return mockAPI, mockAWSAPI, driver +} + func newMockOntapSANDriver(t *testing.T) (*mockapi.MockOntapAPI, *SANStorageDriver) { mockCtrl := gomock.NewController(t) mockAPI := mockapi.NewMockOntapAPI(mockCtrl) - driver := newTestOntapSANDriver(ONTAPTEST_LOCALHOST, "0", ONTAPTEST_VSERVER_AGGR_NAME, true, mockAPI) + driver := newTestOntapSANDriver(ONTAPTEST_LOCALHOST, "0", ONTAPTEST_VSERVER_AGGR_NAME, true, nil, mockAPI) driver.API = mockAPI driver.ips = []string{"127.0.0.1"} @@ -67,8 +82,8 @@ func TestOntapSanStorageDriverConfigString(t *testing.T) { mockAPI.EXPECT().SVMName().AnyTimes().Return("SVM1") ontapSanDrivers := []SANStorageDriver{ - *newTestOntapSANDriver(vserverAdminHost, vserverAdminPort, vserverAggrName, true, mockAPI), - *newTestOntapSANDriver(vserverAdminHost, vserverAdminPort, vserverAggrName, false, mockAPI), + *newTestOntapSANDriver(vserverAdminHost, vserverAdminPort, vserverAggrName, true, nil, mockAPI), + *newTestOntapSANDriver(vserverAdminHost, vserverAdminPort, vserverAggrName, false, nil, mockAPI), } sensitiveIncludeList := map[string]string{ @@ -104,7 +119,7 @@ func TestOntapSanStorageDriverConfigString(t *testing.T) { } func newTestOntapSANDriver( - vserverAdminHost, vserverAdminPort, vserverAggrName string, useREST bool, apiOverride api.OntapAPI, + vserverAdminHost, vserverAdminPort, vserverAggrName string, useREST bool, fsxId *string, apiOverride api.OntapAPI, ) *SANStorageDriver { config := &drivers.OntapStorageDriverConfig{} sp := func(s string) *string { return &s } @@ -123,6 +138,11 @@ func newTestOntapSANDriver( config.StoragePrefix = sp("test_") config.UseREST = useREST + if fsxId != nil { + config.AWSConfig = &drivers.AWSConfig{} + config.AWSConfig.FSxFilesystemID = *fsxId + } + sanDriver := &SANStorageDriver{} sanDriver.Config = *config @@ -211,7 +231,7 @@ func TestOntapSanTerminate(t *testing.T) { api.FakeIgroups[driverInfo.igroupName] = igroupsIQNMap - sanStorageDriver := newTestOntapSANDriver(vserverAdminHost, port, vserverAggrName, false, nil) + sanStorageDriver := newTestOntapSANDriver(vserverAdminHost, port, vserverAggrName, false, nil, nil) sanStorageDriver.Config.IgroupName = driverInfo.igroupName sanStorageDriver.telemetry = nil ontapSanDrivers = append(ontapSanDrivers, *sanStorageDriver) @@ -554,7 +574,7 @@ func TestOntapSanUnpublish(t *testing.T) { mockAPI.EXPECT().SVMName().AnyTimes().Return("SVM1") - d := newTestOntapSANDriver(ONTAPTEST_LOCALHOST, "0", ONTAPTEST_VSERVER_AGGR_NAME, true, mockAPI) + d := newTestOntapSANDriver(ONTAPTEST_LOCALHOST, "0", ONTAPTEST_VSERVER_AGGR_NAME, true, nil, mockAPI) d.API = mockAPI tr.mocks(mockAPI, igroupName, lunPath) @@ -575,7 +595,7 @@ func TestOntapSanVolumePublishManaged(t *testing.T) { mockAPI.EXPECT().SVMName().AnyTimes().Return("SVM1") - d := newTestOntapSANDriver(ONTAPTEST_LOCALHOST, "0", ONTAPTEST_VSERVER_AGGR_NAME, true, mockAPI) + d := newTestOntapSANDriver(ONTAPTEST_LOCALHOST, "0", ONTAPTEST_VSERVER_AGGR_NAME, true, nil, mockAPI) d.API = mockAPI d.ips = []string{"127.0.0.1"} @@ -617,7 +637,7 @@ func TestOntapSanVolumePublishUnmanaged(t *testing.T) { mockAPI.EXPECT().SVMName().AnyTimes().Return("SVM1") - d := newTestOntapSANDriver(ONTAPTEST_LOCALHOST, "0", ONTAPTEST_VSERVER_AGGR_NAME, true, mockAPI) + d := newTestOntapSANDriver(ONTAPTEST_LOCALHOST, "0", ONTAPTEST_VSERVER_AGGR_NAME, true, nil, mockAPI) d.API = mockAPI d.ips = []string{"127.0.0.1"} @@ -659,7 +679,7 @@ func TestOntapSanVolumePublishSLMError(t *testing.T) { mockAPI.EXPECT().SVMName().AnyTimes().Return("SVM1") - d := newTestOntapSANDriver(ONTAPTEST_LOCALHOST, "0", ONTAPTEST_VSERVER_AGGR_NAME, true, mockAPI) + d := newTestOntapSANDriver(ONTAPTEST_LOCALHOST, "0", ONTAPTEST_VSERVER_AGGR_NAME, true, nil, mockAPI) d.API = mockAPI d.ips = []string{"127.0.0.1"} @@ -701,7 +721,7 @@ func TestSANStorageDriverGetBackendState(t *testing.T) { mockApi.EXPECT().SVMName().AnyTimes().Return("SVM1") - mockDriver := newTestOntapSANDriver(ONTAPTEST_LOCALHOST, "0", ONTAPTEST_VSERVER_AGGR_NAME, true, mockApi) + mockDriver := newTestOntapSANDriver(ONTAPTEST_LOCALHOST, "0", ONTAPTEST_VSERVER_AGGR_NAME, true, nil, mockApi) mockDriver.API = mockApi mockApi.EXPECT().GetSVMState(ctx).Return("", fmt.Errorf("returning test error")) @@ -1729,6 +1749,49 @@ func TestOntapSanVolumeRename_fail(t *testing.T) { assert.Error(t, err, "Renamed the volume, expected to fail") } +func TestOntapSanVolumeDestroy_FSx(t *testing.T) { + svmName := "SVM1" + mockAPI, mockAWSAPI, driver := newMockAWSOntapSANDriver(t) + + volConfig := getVolumeConfig() + + tests := []struct { + message string + nasType string + smbShare string + state string + }{ + {"Test volume in FSx in available state", "nfs", "", "AVAILABLE"}, + {"Test volume in FSx in deleting state", "nfs", "", "DELETING"}, + } + + for _, test := range tests { + t.Run(test.message, func(t *testing.T) { + vol := awsapi.Volume{ + State: test.state, + } + isVolumeExists := vol.State != "" + mockAPI.EXPECT().VolumeExists(ctx, volConfig.InternalName).Return(true, nil) + mockAWSAPI.EXPECT().VolumeExists(ctx, volConfig).Return(isVolumeExists, &vol, nil) + if isVolumeExists { + mockAWSAPI.EXPECT().WaitForVolumeStates( + ctx, &vol, []string{awsapi.StateDeleted}, []string{awsapi.StateFailed}, awsapi.RetryDeleteTimeout).Return("", nil) + if vol.State == awsapi.StateAvailable { + mockAWSAPI.EXPECT().DeleteVolume(ctx, &vol).Return(nil) + } + } else { + mockAPI.EXPECT().SVMName().AnyTimes().Return(svmName) + mockAPI.EXPECT().SnapmirrorDeleteViaDestination(ctx, volConfig.InternalName, svmName).Return(nil) + mockAPI.EXPECT().SnapmirrorRelease(ctx, volConfig.InternalName, svmName).Return(nil) + mockAPI.EXPECT().VolumeDestroy(ctx, volConfig.InternalName, true).Return(nil) + } + result := driver.Destroy(ctx, volConfig) + + assert.NoError(t, result) + }) + } +} + func TestOntapSanVolumeDestroy(t *testing.T) { ctx := context.Background() @@ -2367,7 +2430,7 @@ func TestOntapSANStorageDriverGetUpdateType(t *testing.T) { } oldDriver.Config.DataLIF = "1.2.3.1" - newDriver := newTestOntapSANDriver(ONTAPTEST_LOCALHOST, "0", ONTAPTEST_VSERVER_AGGR_NAME, true, mockAPI) + newDriver := newTestOntapSANDriver(ONTAPTEST_LOCALHOST, "0", ONTAPTEST_VSERVER_AGGR_NAME, true, nil, mockAPI) newDriver.API = mockAPI prefix2 := "storage_" @@ -2399,13 +2462,13 @@ func TestOntapSANStorageDriverGetUpdateType_Failure(t *testing.T) { mockAPI, _ := newMockOntapSANDriver(t) mockAPI.EXPECT().SVMName().AnyTimes().Return("SVM1") - oldDriver := newTestOntapSanEcoDriver(ONTAPTEST_LOCALHOST, "0", ONTAPTEST_VSERVER_AGGR_NAME, false, mockAPI) + oldDriver := newTestOntapSanEcoDriver(ONTAPTEST_LOCALHOST, "0", ONTAPTEST_VSERVER_AGGR_NAME, false, nil, mockAPI) oldDriver.API = mockAPI prefix1 := "test_" oldDriver.Config.StoragePrefix = &prefix1 // Created a SAN driver - newDriver := newTestOntapSANDriver(ONTAPTEST_LOCALHOST, "0", ONTAPTEST_VSERVER_AGGR_NAME, true, mockAPI) + newDriver := newTestOntapSANDriver(ONTAPTEST_LOCALHOST, "0", ONTAPTEST_VSERVER_AGGR_NAME, true, nil, mockAPI) newDriver.API = mockAPI prefix2 := "storage_" @@ -2770,7 +2833,7 @@ func TestOntapSANStorageDriverInitialize_WithTwoAuthMethods(t *testing.T) { "clientcertificate": "dummy-certificate", "clientprivatekey": "dummy-client-private-key" }` - sanStorageDriver := newTestOntapSANDriver(vserverAdminHost, vserverAdminPort, vserverAggrName, false, nil) + sanStorageDriver := newTestOntapSANDriver(vserverAdminHost, vserverAdminPort, vserverAggrName, false, nil, nil) result := sanStorageDriver.Initialize(ctx, "CSI", configJSON, commonConfig, map[string]string{}, BackendUUID) @@ -2800,7 +2863,7 @@ func TestOntapSANStorageDriverInitialize_WithTwoAuthMethodsWithSecrets(t *testin "clientprivatekey": "dummy-client-private-key", "clientcertificate": "dummy-certificate", } - sanStorageDriver := newTestOntapSANDriver(vserverAdminHost, vserverAdminPort, vserverAggrName, false, nil) + sanStorageDriver := newTestOntapSANDriver(vserverAdminHost, vserverAdminPort, vserverAggrName, false, nil, nil) result := sanStorageDriver.Initialize(ctx, "CSI", configJSON, commonConfig, secrets, BackendUUID) @@ -2830,7 +2893,7 @@ func TestOntapSANStorageDriverInitialize_WithTwoAuthMethodsWithConfigAndSecrets( "clientprivatekey": "dummy-client-private-key", "clientcertificate": "dummy-certificate", } - sanStorageDriver := newTestOntapSANDriver(vserverAdminHost, vserverAdminPort, vserverAggrName, false, nil) + sanStorageDriver := newTestOntapSANDriver(vserverAdminHost, vserverAdminPort, vserverAggrName, false, nil, nil) result := sanStorageDriver.Initialize(ctx, "CSI", configJSON, commonConfig, secrets, BackendUUID) @@ -3651,7 +3714,7 @@ func TestOntapSanVolumeValidate_ValidateSANDriver(t *testing.T) { mockAPI := mockapi.NewMockOntapAPI(mockCtrl) mockAPI.EXPECT().SVMName().AnyTimes().Return("SVM1") - driver := newTestOntapSANDriver(ONTAPTEST_LOCALHOST, "0", ONTAPTEST_VSERVER_AGGR_NAME, true, mockAPI) + driver := newTestOntapSANDriver(ONTAPTEST_LOCALHOST, "0", ONTAPTEST_VSERVER_AGGR_NAME, true, nil, mockAPI) driver.API = mockAPI storagePrefix := "trident&#"