Skip to content

Commit 993f2ad

Browse files
vivakeramvivakeram
authored
Clone export policy fix for ontap-nas
1 parent 33fa90d commit 993f2ad

File tree

3 files changed

+136
-2
lines changed

3 files changed

+136
-2
lines changed

‎core/orchestrator_core.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2339,6 +2339,7 @@ func (o *TridentOrchestrator) cloneVolumeInitial(
23392339

23402340
// Clone the source config, as most of its attributes will apply to the clone
23412341
cloneConfig := sourceVolume.Config.ConstructClone()
2342+
23422343
internalName := ""
23432344
internalID := ""
23442345

@@ -2373,6 +2374,8 @@ func (o *TridentOrchestrator) cloneVolumeInitial(
23732374
cloneConfig.Namespace = volumeConfig.Namespace
23742375
cloneConfig.RequestName = volumeConfig.RequestName
23752376
cloneConfig.SkipRecoveryQueue = volumeConfig.SkipRecoveryQueue
2377+
// Empty out the export policy. It will be set in the backend driver.
2378+
cloneConfig.ExportPolicy = ""
23762379

23772380
// If it's from snapshot, we need the LUKS passphrases value from the snapshot
23782381
isLUKS, err := strconv.ParseBool(cloneConfig.LUKSEncryption)

‎storage_drivers/ontap/ontap_nas.go‎

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -553,6 +553,30 @@ func (d *NASStorageDriver) CreateClone(
553553
return err
554554
}
555555

556+
// Cloned volume in ontap by default will use the same export policy as the source volume.
557+
// We need to make sure the correct export policy is set on the clone.
558+
exportPolicy := collection.GetV(opts, "exportPolicy", storagePool.InternalAttributes()[ExportPolicy])
559+
560+
// only create the empty policy if autoExportPolicy = true
561+
if d.Config.AutoExportPolicy {
562+
// set empty export policy on flexVol creation
563+
exportPolicy = getEmptyExportPolicyName(*d.Config.StoragePrefix)
564+
565+
err = ensureExportPolicyExists(ctx, exportPolicy, d.API)
566+
if err != nil {
567+
return fmt.Errorf("error ensuring export policy exists: %v", err)
568+
}
569+
}
570+
571+
// Set export policy on the new cloned volume
572+
err = d.API.VolumeModifyExportPolicy(ctx, cloneVolConfig.InternalName, exportPolicy)
573+
if err != nil {
574+
Logc(ctx).WithError(err).Errorf("Error setting volume %s to empty export policy.", cloneVolConfig.InternalName)
575+
return err
576+
}
577+
578+
cloneVolConfig.ExportPolicy = exportPolicy
579+
556580
// TODO: enable secure SMB share for clone volumes
557581
if d.Config.NASType == sa.SMB {
558582
if err := d.EnsureSMBShare(ctx, cloneVolConfig.InternalName, "/"+cloneVolConfig.InternalName, cloneVolConfig.SMBShareACL, false); err != nil {
@@ -898,6 +922,7 @@ func (d *NASStorageDriver) setVolToEmptyPolicy(ctx context.Context, volName stri
898922
if err != nil {
899923
return err
900924
}
925+
901926
if !exists {
902927
Logc(ctx).WithField("exportPolicy", emptyExportPolicy).
903928
Debug("Export policy not found, attempting to create it.")
@@ -906,11 +931,14 @@ func (d *NASStorageDriver) setVolToEmptyPolicy(ctx context.Context, volName stri
906931
return err
907932
}
908933
}
934+
909935
err = d.API.VolumeModifyExportPolicy(ctx, volName, emptyExportPolicy)
910936
if err != nil {
911937
Logc(ctx).WithError(err).Errorf("Error setting volume %s to empty export policy.", volName)
938+
return err
912939
}
913-
return err
940+
941+
return nil
914942
}
915943

916944
func (d *NASStorageDriver) publishFlexVolShare(

‎storage_drivers/ontap/ontap_nas_test.go‎

Lines changed: 104 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -861,6 +861,7 @@ func TestOntapNasStorageDriverVolumeClone(t *testing.T) {
861861
pool1 := storage.NewStoragePool(nil, "pool1")
862862
pool1.SetInternalAttributes(map[string]string{
863863
"tieringPolicy": "none",
864+
"exportPolicy": "default",
864865
})
865866
pool1.Attributes()["labels"] = sa.NewLabelOffer(map[string]string{
866867
"type": "clone",
@@ -899,6 +900,7 @@ func TestOntapNasStorageDriverVolumeClone(t *testing.T) {
899900
mockAPI.EXPECT().VolumeSetComment(ctx, volConfig.InternalName, volConfig.InternalName, "{\"provisioning\":{\"type\":\"clone\"}}").
900901
Return(nil)
901902
mockAPI.EXPECT().VolumeMount(ctx, volConfig.InternalName, "/"+volConfig.InternalName).Return(nil)
903+
mockAPI.EXPECT().VolumeModifyExportPolicy(ctx, volConfig.InternalName, "default").Return(nil)
902904

903905
if test.NasType == sa.SMB {
904906
driver.Config.NASType = sa.SMB
@@ -1024,6 +1026,7 @@ func TestOntapNasStorageDriverVolumeClone_NameTemplateStoragePoolUnset(t *testin
10241026
})
10251027
pool1.InternalAttributes()[NameTemplate] = "{{.config.StorageDriverName}}_{{.labels.Cluster}}_{{.volume.Namespace}}_{{.volume." +
10261028
"RequestName}}"
1029+
pool1.InternalAttributes()[ExportPolicy] = "default"
10271030
driver.physicalPools = map[string]storage.Pool{"pool1": pool1}
10281031
driver.Config.SplitOnClone = "false"
10291032
driver.Config.Labels = pool1.GetLabels(ctx, "")
@@ -1039,8 +1042,104 @@ func TestOntapNasStorageDriverVolumeClone_NameTemplateStoragePoolUnset(t *testin
10391042
mockAPI.EXPECT().VolumeSetComment(ctx, volConfig.InternalName, volConfig.InternalName, "{\"provisioning\":{\"type\":\"clone\"}}").
10401043
Return(nil)
10411044
mockAPI.EXPECT().VolumeMount(ctx, volConfig.InternalName, "/"+volConfig.InternalName).Return(nil)
1045+
mockAPI.EXPECT().VolumeModifyExportPolicy(ctx, volConfig.InternalName, "default").Return(nil)
10421046

1043-
result := driver.CreateClone(ctx, nil, volConfig, nil)
1047+
result := driver.CreateClone(ctx, nil, volConfig, pool1)
1048+
1049+
assert.NoError(t, result)
1050+
}
1051+
1052+
func TestOntapNasStorageDriverVolumeClone_AutoExportPolicy_On(t *testing.T) {
1053+
mockAPI, driver := newMockOntapNASDriver(t)
1054+
volConfig := &storage.VolumeConfig{
1055+
Size: "1g",
1056+
Encryption: "false",
1057+
FileSystem: "nfs",
1058+
}
1059+
1060+
flexVol := api.Volume{
1061+
Name: "flexvol",
1062+
Comment: "flexvol",
1063+
}
1064+
1065+
pool1 := storage.NewStoragePool(nil, "pool1")
1066+
pool1.SetInternalAttributes(map[string]string{
1067+
"tieringPolicy": "none",
1068+
})
1069+
pool1.Attributes()["labels"] = sa.NewLabelOffer(map[string]string{
1070+
"type": "clone",
1071+
})
1072+
pool1.InternalAttributes()[ExportPolicy] = "<automatic>"
1073+
1074+
driver.physicalPools = map[string]storage.Pool{"pool1": pool1}
1075+
driver.Config.SplitOnClone = "false"
1076+
driver.Config.Labels = pool1.GetLabels(ctx, "")
1077+
driver.Config.AutoExportPolicy = true
1078+
prefix := "trident-"
1079+
driver.Config.StoragePrefix = &prefix
1080+
1081+
mockAPI.EXPECT().SVMName().AnyTimes().Return("SVM1")
1082+
mockAPI.EXPECT().VolumeInfo(ctx, volConfig.CloneSourceVolumeInternal).Return(&flexVol, nil)
1083+
mockAPI.EXPECT().VolumeExists(ctx, "").Return(false, nil)
1084+
mockAPI.EXPECT().VolumeSnapshotCreate(ctx, gomock.Any(), gomock.Any()).Return(nil)
1085+
mockAPI.EXPECT().VolumeCloneCreate(ctx, volConfig.InternalName, volConfig.CloneSourceVolumeInternal,
1086+
gomock.Any(), false).Return(nil)
1087+
mockAPI.EXPECT().VolumeWaitForStates(ctx, volConfig.InternalName, gomock.Any(), gomock.Any(),
1088+
maxFlexvolCloneWait).Return("online", nil)
1089+
mockAPI.EXPECT().VolumeSetComment(ctx, volConfig.InternalName, volConfig.InternalName, "{\"provisioning\":{\"type\":\"clone\"}}").
1090+
Return(nil)
1091+
mockAPI.EXPECT().VolumeMount(ctx, volConfig.InternalName, "/"+volConfig.InternalName).Return(nil)
1092+
mockAPI.EXPECT().ExportPolicyCreate(ctx, "trident-empty").Return(nil)
1093+
mockAPI.EXPECT().VolumeModifyExportPolicy(ctx, volConfig.InternalName, "trident-empty").Return(nil)
1094+
1095+
result := driver.CreateClone(ctx, nil, volConfig, pool1)
1096+
1097+
assert.NoError(t, result)
1098+
}
1099+
1100+
func TestOntapNasStorageDriverVolumeClone_AutoExportPolicy_Off(t *testing.T) {
1101+
mockAPI, driver := newMockOntapNASDriver(t)
1102+
volConfig := &storage.VolumeConfig{
1103+
Size: "1g",
1104+
Encryption: "false",
1105+
FileSystem: "nfs",
1106+
}
1107+
1108+
flexVol := api.Volume{
1109+
Name: "flexvol",
1110+
Comment: "flexvol",
1111+
}
1112+
1113+
pool1 := storage.NewStoragePool(nil, "pool1")
1114+
pool1.SetInternalAttributes(map[string]string{
1115+
"tieringPolicy": "none",
1116+
})
1117+
pool1.Attributes()["labels"] = sa.NewLabelOffer(map[string]string{
1118+
"type": "clone",
1119+
})
1120+
pool1.InternalAttributes()[ExportPolicy] = "default"
1121+
1122+
driver.physicalPools = map[string]storage.Pool{"pool1": pool1}
1123+
driver.Config.SplitOnClone = "false"
1124+
driver.Config.Labels = pool1.GetLabels(ctx, "")
1125+
driver.Config.AutoExportPolicy = false
1126+
prefix := "trident-"
1127+
driver.Config.StoragePrefix = &prefix
1128+
1129+
mockAPI.EXPECT().SVMName().AnyTimes().Return("SVM1")
1130+
mockAPI.EXPECT().VolumeInfo(ctx, volConfig.CloneSourceVolumeInternal).Return(&flexVol, nil)
1131+
mockAPI.EXPECT().VolumeExists(ctx, "").Return(false, nil)
1132+
mockAPI.EXPECT().VolumeSnapshotCreate(ctx, gomock.Any(), gomock.Any()).Return(nil)
1133+
mockAPI.EXPECT().VolumeCloneCreate(ctx, volConfig.InternalName, volConfig.CloneSourceVolumeInternal,
1134+
gomock.Any(), false).Return(nil)
1135+
mockAPI.EXPECT().VolumeWaitForStates(ctx, volConfig.InternalName, gomock.Any(), gomock.Any(),
1136+
maxFlexvolCloneWait).Return("online", nil)
1137+
mockAPI.EXPECT().VolumeSetComment(ctx, volConfig.InternalName, volConfig.InternalName, "{\"provisioning\":{\"type\":\"clone\"}}").
1138+
Return(nil)
1139+
mockAPI.EXPECT().VolumeMount(ctx, volConfig.InternalName, "/"+volConfig.InternalName).Return(nil)
1140+
mockAPI.EXPECT().VolumeModifyExportPolicy(ctx, volConfig.InternalName, "default").Return(nil)
1141+
1142+
result := driver.CreateClone(ctx, nil, volConfig, pool1)
10441143

10451144
assert.NoError(t, result)
10461145
}
@@ -1157,6 +1256,7 @@ func TestOntapNasStorageDriverVolumeClone_NameTemplate(t *testing.T) {
11571256
pool1.SetAttributes(map[string]sa.Offer{
11581257
sa.Labels: sa.NewLabelOffer(driver.Config.Labels),
11591258
})
1259+
pool1.InternalAttributes()[ExportPolicy] = "default"
11601260
driver.physicalPools = map[string]storage.Pool{"pool1": pool1}
11611261

11621262
mockAPI.EXPECT().SVMName().AnyTimes().Return("SVM1")
@@ -1169,6 +1269,7 @@ func TestOntapNasStorageDriverVolumeClone_NameTemplate(t *testing.T) {
11691269
mockAPI.EXPECT().VolumeSetComment(ctx, volConfig.InternalName, volConfig.InternalName, test.expectedLabel).
11701270
Return(nil)
11711271
mockAPI.EXPECT().VolumeMount(ctx, volConfig.InternalName, "/"+volConfig.InternalName).Return(nil)
1272+
mockAPI.EXPECT().VolumeModifyExportPolicy(ctx, volConfig.InternalName, "default").Return(nil)
11721273

11731274
result := driver.CreateClone(ctx, nil, volConfig, pool1)
11741275

@@ -1329,6 +1430,7 @@ func TestOntapNasStorageDriverVolumeClone_SMBShareCreateFail(t *testing.T) {
13291430
pool1 := storage.NewStoragePool(nil, "pool1")
13301431
pool1.SetInternalAttributes(map[string]string{
13311432
"tieringPolicy": "none",
1433+
"exportPolicy": "default",
13321434
})
13331435
pool1.Attributes()["labels"] = sa.NewLabelOffer(map[string]string{
13341436
"type": "clone",
@@ -1361,6 +1463,7 @@ func TestOntapNasStorageDriverVolumeClone_SMBShareCreateFail(t *testing.T) {
13611463
mockAPI.EXPECT().SMBShareExists(ctx, volConfig.InternalName).Return(false, nil)
13621464
mockAPI.EXPECT().SMBShareCreate(ctx, volConfig.InternalName,
13631465
"/"+volConfig.InternalName).Return(fmt.Errorf("cannot create volume"))
1466+
mockAPI.EXPECT().VolumeModifyExportPolicy(ctx, volConfig.InternalName, "default").Return(nil)
13641467

13651468
result := driver.CreateClone(ctx, nil, volConfig, pool1)
13661469

0 commit comments

Comments
 (0)