NAS driver concurrency

Author: vivakeram

core/concurrent_core.go

@@ -3124,11 +3124,6 @@ func (o *ConcurrentTridentOrchestrator) publishVolume(ctx context.Context, volum
 	// Fill in what we already know
 	publishInfo.VolumeAccessInfo = volume.Config.AccessInfo
 	publishInfo.Nodes = results[0].Nodes
-	if err := o.reconcileNodeAccessOnBackend(ctx, backend, results[0].VolumePublications, results[0].Nodes); err != nil {
-		err = fmt.Errorf("unable to update node access rules on backend %s; %v", backend.Name(), err)
-		Logc(ctx).Error(err)
-		return err
-	}
 
 	if err := backend.PublishVolume(ctx, volume.Config, publishInfo); err != nil {
 		return err
@@ -5043,6 +5038,10 @@ func (o *ConcurrentTridentOrchestrator) DeleteNode(ctx context.Context, nodeName
 		return fmt.Errorf("failed to delete node %s in store: %v", nodeName, err)
 	}
 	deleteNode()
+
+	// TODO: enable this with periodic node reconciliation
+	// o.invalidateAllBackendNodeAccess(ctx)
+
 	return
 }
 

core/concurrent_core_test.go

@@ -10663,7 +10663,6 @@ func TestPublishVolumeConcurrentCore(t *testing.T) {
 				driver.EXPECT().GetStorageBackendSpecs(gomock.Any(), gomock.Any()).Return(nil)
 				driver.EXPECT().CreateFollowup(gomock.Any(), gomock.Any()).Return(nil)
 				driver.EXPECT().Publish(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil)
-				driver.EXPECT().ReconcileNodeAccess(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil)
 
 				fakeNode := getFakeNode("testNode")
 				addNodesToCache(t, fakeNode)

storage_drivers/ontap/api/ontap_rest.go

@@ -77,18 +77,26 @@ func (c *RestClient) ClientConfig() ClientConfig {
 }
 
 func (c *RestClient) SetSVMUUID(svmUUID string) {
+	c.m.Lock()
+	defer c.m.Unlock()
 	c.svmUUID = svmUUID
 }
 
 func (c *RestClient) SVMUUID() string {
+	c.m.RLock()
+	defer c.m.RUnlock()
 	return c.svmUUID
 }
 
 func (c *RestClient) SetSVMName(svmName string) {
+	c.m.Lock()
+	defer c.m.Unlock()
 	c.svmName = svmName
 }
 
 func (c *RestClient) SVMName() string {
+	c.m.RLock()
+	defer c.m.RUnlock()
 	return c.svmName
 }
 

storage_drivers/ontap/ontap_common.go

@@ -154,6 +154,7 @@ var (
 	smbShareDeleteACL        = map[string]string{DefaultSMBAccessControlUser: DefaultSMBAccessControlUserType}
 	lunMutex                 = locks.NewGCNamedMutex()
 	igroupMutex              = locks.NewGCNamedMutex()
+	exportPolicyMutex        = locks.NewGCNamedMutex()
 
 	duringVolCloneAfterSnapCreation1 = fiji.Register("duringVolCloneAfterSnapCreation1", "ontap_common")
 	duringVolCloneAfterSnapCreation2 = fiji.Register("duringVolCloneAfterSnapCreation2", "ontap_common")
@@ -305,9 +306,19 @@ func InitializeOntapConfig(
 }
 
 func ensureExportPolicyExists(ctx context.Context, policyName string, clientAPI api.OntapAPI) error {
+	exportPolicyMutex.Lock(policyName)
+	defer exportPolicyMutex.Unlock(policyName)
+
 	return clientAPI.ExportPolicyCreate(ctx, policyName)
 }
 
+func destroyExportPolicy(ctx context.Context, policyName string, clientAPI api.OntapAPI) error {
+	exportPolicyMutex.Lock(policyName)
+	defer exportPolicyMutex.Unlock(policyName)
+
+	return clientAPI.ExportPolicyDestroy(ctx, policyName)
+}
+
 func getExportPolicyName(backendUUID string) string {
 	return fmt.Sprintf("trident-%s", backendUUID)
 }
@@ -380,12 +391,15 @@ func ensureNodeAccessForPolicy(
 	Logc(ctx).WithFields(fields).Debug(">>>> ensureNodeAccessForPolicy")
 	defer Logc(ctx).WithFields(fields).Debug("<<<< ensureNodeAccessForPolicy")
 
+	exportPolicyMutex.Lock(policyName)
+	defer exportPolicyMutex.Unlock(policyName)
+
 	if exists, err := clientAPI.ExportPolicyExists(ctx, policyName); err != nil {
 		return err
 	} else if !exists {
 		Logc(ctx).WithField("exportPolicy", policyName).Debug("Export policy missing, will create it.")
 
-		if err = ensureExportPolicyExists(ctx, policyName, clientAPI); err != nil {
+		if err = clientAPI.ExportPolicyCreate(ctx, policyName); err != nil {
 			return err
 		}
 	}
@@ -496,6 +510,9 @@ func reconcileExportPolicyRules(
 	Logc(ctx).WithFields(fields).Debug(">>>> reconcileExportPolicyRules")
 	defer Logc(ctx).WithFields(fields).Debug("<<<< reconcileExportPolicyRules")
 
+	exportPolicyMutex.Lock(policyName)
+	defer exportPolicyMutex.Unlock(policyName)
+
 	// first grab all existing rules
 	existingRules, err := clientAPI.ExportRuleList(ctx, policyName)
 	if err != nil {
@@ -4904,6 +4921,9 @@ func removeExportPolicyRules(
 	Logc(ctx).WithFields(fields).Debug(">>>> removeExportPolicyRules")
 	defer Logc(ctx).WithFields(fields).Debug("<<<< removeExportPolicyRules")
 
+	exportPolicyMutex.Lock(exportPolicy)
+	defer exportPolicyMutex.Unlock(exportPolicy)
+
 	var removeRuleIndexes []int
 
 	nodeIPRules := make(map[string]struct{})

storage_drivers/ontap/ontap_nas.go

@@ -62,11 +62,11 @@ type NASStorageDriver struct {
 }
 
 func (d *NASStorageDriver) GetConfig() drivers.DriverConfig {
-	return &d.Config
+	return d.Config.SmartCopy()
 }
 
 func (d *NASStorageDriver) GetOntapConfig() *drivers.OntapStorageDriverConfig {
-	return &d.Config
+	return d.Config.SmartCopy()
 }
 
 func (d *NASStorageDriver) GetAPI() api.OntapAPI {
@@ -185,7 +185,7 @@ func (d *NASStorageDriver) Terminate(ctx context.Context, backendUUID string) {
 	if d.Config.AutoExportPolicy {
 		policyName := getExportPolicyName(backendUUID)
 
-		if err := d.API.ExportPolicyDestroy(ctx, policyName); err != nil {
+		if err := destroyExportPolicy(ctx, policyName, d.API); err != nil {
 			Logc(ctx).Warn(err)
 		}
 	}
@@ -965,8 +965,9 @@ func (d *NASStorageDriver) Unpublish(
 			volConfig.ExportPolicy = getEmptyExportPolicyName(*d.Config.StoragePrefix)
 
 			// Remove export policy if no rules exist
-			if err = d.API.ExportPolicyDestroy(ctx, exportPolicy); err != nil {
-				Logc(ctx).WithError(err).Errorf("Error deleting export policy %s.", exportPolicy)
+			if err = destroyExportPolicy(ctx, exportPolicy, d.API); err != nil {
+				Logc(ctx).WithError(err).Errorf("Could not delete export policy %s.", exportPolicy)
+				return err
 			}
 		}
 
@@ -986,6 +987,7 @@ func (d *NASStorageDriver) Unpublish(
 // setVolToEmptyPolicy  set export policy to the empty policy.
 func (d *NASStorageDriver) setVolToEmptyPolicy(ctx context.Context, volName string) error {
 	emptyExportPolicy := getEmptyExportPolicyName(*d.Config.StoragePrefix)
+
 	exists, err := d.API.ExportPolicyExists(ctx, emptyExportPolicy)
 	if err != nil {
 		return err
@@ -1713,6 +1715,7 @@ func (d *NASStorageDriver) reconcileNodeAccessForBackendPolicy(
 		Logc(ctx).Error(err)
 		return err
 	}
+
 	err = reconcileExportPolicyRules(ctx, policyName, desiredRules, d.API, &d.Config)
 	if err != nil {
 		err = fmt.Errorf("unabled to reconcile export policy rules; %v", err)