Generating unique subsystem name in case of RWO volumes in ASAr2 NVMe

aparna0508 · web-flow · commit 341782579e07 · 2025-06-20T21:09:00.000+05:30
diff --git a/mocks/mock_storage_drivers/mock_ontap/mock_api.go b/mocks/mock_storage_drivers/mock_ontap/mock_api.go
diff --git a/mocks/mock_storage_drivers/mock_ontap/mock_ontap_rest_interface.go b/mocks/mock_storage_drivers/mock_ontap/mock_ontap_rest_interface.go
diff --git a/storage_drivers/ontap/api/abstraction.go b/storage_drivers/ontap/api/abstraction.go
@@ -245,7 +245,7 @@ type OntapAPI interface {
 	NVMeNamespaceList(ctx context.Context, pattern string) (NVMeNamespaces, error)
 	NVMeNamespaceGetSize(ctx context.Context, namespacePath string) (int, error)
 	NVMeNamespaceDelete(ctx context.Context, namespacePath string) error
-	NVMeSubsystemCreate(ctx context.Context, subsystemName string) (*NVMeSubsystem, error)
+	NVMeSubsystemCreate(ctx context.Context, subsystemName, comment string) (*NVMeSubsystem, error)
 	NVMeSubsystemDelete(ctx context.Context, subsysUUID string) error
 	NVMeSubsystemAddNamespace(ctx context.Context, subsystemUUID, nsUUID string) error
 	NVMeSubsystemRemoveNamespace(ctx context.Context, subsysUUID, nsUUID string) error
diff --git a/storage_drivers/ontap/api/abstraction_rest.go b/storage_drivers/ontap/api/abstraction_rest.go
@@ -3365,7 +3365,7 @@ func (d OntapAPIREST) NVMeRemoveHostFromSubsystem(ctx context.Context, hostNQN,
 }
 
 // NVMeSubsystemCreate Checks if the subsystem is already there or not. If not, creates a new one.
-func (d OntapAPIREST) NVMeSubsystemCreate(ctx context.Context, subsystemName string) (*NVMeSubsystem, error) {
+func (d OntapAPIREST) NVMeSubsystemCreate(ctx context.Context, subsystemName, comment string) (*NVMeSubsystem, error) {
 	logFields := LogFields{
 		"Method":        "SubsystemCreate",
 		"Type":          "OntapAPIREST",
@@ -3382,7 +3382,7 @@ func (d OntapAPIREST) NVMeSubsystemCreate(ctx context.Context, subsystemName str
 	}
 	if subsystem == nil {
 		Logc(ctx).Infof("Subsystem doesn't exist, creating new subsystem %v now.", subsystemName)
-		subsystem, err = d.api.NVMeSubsystemCreate(ctx, subsystemName)
+		subsystem, err = d.api.NVMeSubsystemCreate(ctx, subsystemName, comment)
 		if err != nil {
 			return nil, err
 		}
diff --git a/storage_drivers/ontap/api/abstraction_rest_test.go b/storage_drivers/ontap/api/abstraction_rest_test.go
@@ -748,6 +748,7 @@ func TestNVMeSubsystemCreate(t *testing.T) {
 
 	// case 1: No error while adding host to subsystem
 	subsystemName := "fakeSubsystem"
+	subsystemComment := "fakeSubsystemComment"
 	subsysUUID := "fakeUUID"
 	targetNQN := "fakeTargetNQN"
 	subsys := &models.NvmeSubsystem{
@@ -758,7 +759,7 @@ func TestNVMeSubsystemCreate(t *testing.T) {
 
 	mock.EXPECT().NVMeSubsystemGetByName(ctx, subsystemName, gomock.Any()).Return(subsys, nil)
 	mock.EXPECT().ClientConfig().Return(clientConfig).AnyTimes()
-	subsystem, err := oapi.NVMeSubsystemCreate(ctx, subsystemName)
+	subsystem, err := oapi.NVMeSubsystemCreate(ctx, subsystemName, subsystemComment)
 	assert.NoError(t, err)
 	assert.Equal(t, subsystem.UUID, subsysUUID, "subsystem UUID does not match")
 	assert.Equal(t, subsystem.Name, subsystemName, "subsystem name does not match")
@@ -767,31 +768,31 @@ func TestNVMeSubsystemCreate(t *testing.T) {
 	// case 2: Error getting susbsystem info from backend
 	mock.EXPECT().NVMeSubsystemGetByName(ctx, subsystemName, gomock.Any()).Return(nil, fmt.Errorf("Error getting susbsystem info"))
 	mock.EXPECT().ClientConfig().Return(clientConfig).AnyTimes()
-	_, err = oapi.NVMeSubsystemCreate(ctx, subsystemName)
+	_, err = oapi.NVMeSubsystemCreate(ctx, subsystemName, subsystemComment)
 	assert.Error(t, err)
 
 	// case 3: Subsystem not present, create a new one successfully
 	mock.EXPECT().NVMeSubsystemGetByName(ctx, subsystemName, gomock.Any()).Return(nil, nil)
-	mock.EXPECT().NVMeSubsystemCreate(ctx, subsystemName).Return(subsys, nil)
+	mock.EXPECT().NVMeSubsystemCreate(ctx, subsystemName, subsystemComment).Return(subsys, nil)
 	mock.EXPECT().ClientConfig().Return(clientConfig).AnyTimes()
-	newsubsys, err := oapi.NVMeSubsystemCreate(ctx, subsystemName)
+	newsubsys, err := oapi.NVMeSubsystemCreate(ctx, subsystemName, subsystemComment)
 	assert.NoError(t, err)
 	assert.Equal(t, newsubsys.UUID, subsysUUID, "subsystem UUID does not match")
 	assert.Equal(t, newsubsys.Name, subsystemName, "subsystem name does not match")
 	assert.Equal(t, newsubsys.NQN, targetNQN, "host does not match")
 
 	// case 4: Subsystem not present, create a new one with failure
 	mock.EXPECT().NVMeSubsystemGetByName(ctx, subsystemName, gomock.Any()).Return(nil, nil)
-	mock.EXPECT().NVMeSubsystemCreate(ctx, subsystemName).Return(nil, fmt.Errorf("Error creating susbsystem"))
+	mock.EXPECT().NVMeSubsystemCreate(ctx, subsystemName, subsystemComment).Return(nil, fmt.Errorf("Error creating susbsystem"))
 	mock.EXPECT().ClientConfig().Return(clientConfig).AnyTimes()
-	newsubsys, err = oapi.NVMeSubsystemCreate(ctx, subsystemName)
+	newsubsys, err = oapi.NVMeSubsystemCreate(ctx, subsystemName, subsystemComment)
 	assert.Error(t, err)
 
 	// case 5: Subsystem not present, create a new one but returned nil
 	mock.EXPECT().NVMeSubsystemGetByName(ctx, subsystemName, gomock.Any()).Return(nil, nil)
-	mock.EXPECT().NVMeSubsystemCreate(ctx, subsystemName).Return(nil, nil)
+	mock.EXPECT().NVMeSubsystemCreate(ctx, subsystemName, subsystemComment).Return(nil, nil)
 	mock.EXPECT().ClientConfig().Return(clientConfig).AnyTimes()
-	newsubsys, err = oapi.NVMeSubsystemCreate(ctx, subsystemName)
+	newsubsys, err = oapi.NVMeSubsystemCreate(ctx, subsystemName, subsystemComment)
 	assert.Error(t, err)
 }
 
diff --git a/storage_drivers/ontap/api/abstraction_zapi.go b/storage_drivers/ontap/api/abstraction_zapi.go
@@ -2726,7 +2726,7 @@ func (d OntapAPIZAPI) NVMeNamespaceDelete(ctx context.Context, name string) erro
 	return errors.UnsupportedError("ZAPI call is not supported yet")
 }
 
-func (d OntapAPIZAPI) NVMeSubsystemCreate(ctx context.Context, subsystemName string) (*NVMeSubsystem, error) {
+func (d OntapAPIZAPI) NVMeSubsystemCreate(ctx context.Context, subsystemName, comment string) (*NVMeSubsystem, error) {
 	return nil, errors.UnsupportedError("ZAPI call is not supported yet")
 }
 
diff --git a/storage_drivers/ontap/api/ontap_rest.go b/storage_drivers/ontap/api/ontap_rest.go
@@ -6966,7 +6966,7 @@ func (c *RestClient) NVMeSubsystemGetByName(
 }
 
 // NVMeSubsystemCreate creates a new subsystem
-func (c *RestClient) NVMeSubsystemCreate(ctx context.Context, subsystemName string) (*models.NvmeSubsystem, error) {
+func (c *RestClient) NVMeSubsystemCreate(ctx context.Context, subsystemName, comment string) (*models.NvmeSubsystem, error) {
 	params := nvme.NewNvmeSubsystemCreateParamsWithTimeout(c.httpClient.Timeout)
 	osType := "linux"
 	params.Context = ctx
@@ -6978,6 +6978,11 @@ func (c *RestClient) NVMeSubsystemCreate(ctx context.Context, subsystemName stri
 		Svm:    &models.NvmeSubsystemInlineSvm{UUID: convert.ToPtr(c.svmUUID)},
 	}
 
+	// If comment is provided, set the comment
+	if len(comment) > 0 {
+		params.Info.Comment = convert.ToPtr(comment)
+	}
+
 	subsysCreated, err := c.api.NvMe.NvmeSubsystemCreate(params, c.authInfo)
 	if err != nil {
 		return nil, err
diff --git a/storage_drivers/ontap/api/ontap_rest_interface.go b/storage_drivers/ontap/api/ontap_rest_interface.go
@@ -404,7 +404,7 @@ type RestClientInterface interface {
 	// NVMeSubsystemGetByName gets the subsystem with the specified name
 	NVMeSubsystemGetByName(ctx context.Context, subsystemName string, fields []string) (*models.NvmeSubsystem, error)
 	// NVMeSubsystemCreate creates a new subsystem
-	NVMeSubsystemCreate(ctx context.Context, subsystemName string) (*models.NvmeSubsystem, error)
+	NVMeSubsystemCreate(ctx context.Context, subsystemName, comment string) (*models.NvmeSubsystem, error)
 	// NVMeSubsystemDelete deletes a given subsystem
 	NVMeSubsystemDelete(ctx context.Context, subsysUUID string) error
 	// NVMeAddHostNqnToSubsystem adds the NQN of the host to the subsystem
diff --git a/storage_drivers/ontap/api/ontap_rest_test.go b/storage_drivers/ontap/api/ontap_rest_test.go
@@ -4075,7 +4075,7 @@ func TestOntapRest_NVMeSubsystemCreate(t *testing.T) {
 			rs := newRestClient(server.Listener.Addr().String(), server.Client())
 			assert.NotNil(t, rs)
 
-			nvmeSubsystem, err := rs.NVMeSubsystemCreate(ctx, "subsystemName")
+			nvmeSubsystem, err := rs.NVMeSubsystemCreate(ctx, "subsystemName", "comment to be set on subsystem")
 			if !test.isErrorExpected {
 				assert.NoError(t, err, "issue while creating subsystem")
 				assert.Equal(t, "subsystemName", *nvmeSubsystem.Name)
diff --git a/storage_drivers/ontap/ontap_asa_nvme.go b/storage_drivers/ontap/ontap_asa_nvme.go
@@ -44,6 +44,10 @@ type ASANVMeStorageDriver struct {
 	cloneSplitTimers *sync.Map
 }
 
+const (
+	asaNVMeSubsystemPrefix = "an"
+)
+
 func (d *ASANVMeStorageDriver) GetConfig() drivers.DriverConfig {
 	return &d.Config
 }
@@ -859,21 +863,37 @@ func (d *ASANVMeStorageDriver) Publish(
 	// When FS type is RAW, we create a new subsystem per namespace,
 	// else we use the subsystem created for that particular node
 	var ssName string
+	var completeSSName string
+
 	if volConfig.FileSystem == filesystem.Raw {
 		ssName = getNamespaceSpecificSubsystemName(name, pvName)
+		completeSSName = ssName
 	} else {
-		ssName = getNodeSpecificSubsystemName(publishInfo.HostName, publishInfo.TridentUUID)
+		// Create a unique node specific subsystem name. Use prefix to indicate ASA r2 NVMe.
+		completeSSName, ssName, err = getUniqueNodeSpecificSubsystemName(
+			publishInfo.HostName, publishInfo.TridentUUID, asaNVMeSubsystemPrefix, maximumSubsystemNameLength)
+		if err != nil {
+			return fmt.Errorf("failed to create node specific subsystem name: %w", err)
+		}
+	}
+
+	logFields := LogFields{
+		"filesystemType":        volConfig.FileSystem,
+		"hostName":              publishInfo.HostName,
+		"subsystemName":         ssName,
+		"completeSubsystemName": completeSSName,
 	}
+	Logc(ctx).WithFields(logFields).Debug("Successfully generated subsystem name for publish.")
 
 	// Checks if subsystem exists and creates a new one if not
-	subsystem, err := d.API.NVMeSubsystemCreate(ctx, ssName)
+	subsystem, err := d.API.NVMeSubsystemCreate(ctx, ssName, completeSSName)
 	if err != nil {
 		Logc(ctx).Errorf("subsystem create failed, %w", err)
 		return err
 	}
 
 	if subsystem == nil {
-		return fmt.Errorf("No subsystem returned after subsystem create")
+		return fmt.Errorf("no subsystem returned after subsystem create")
 	}
 
 	// Fill important info in publishInfo
diff --git a/storage_drivers/ontap/ontap_asa_nvme_test.go b/storage_drivers/ontap/ontap_asa_nvme_test.go
@@ -1526,7 +1526,7 @@ func TestPublishASANVMe(t *testing.T) {
 	publishInfo.HostNQN = "fakeHostNQN"
 	mockAPI.EXPECT().VolumeInfo(ctx, volConfig.InternalName).Return(flexVol, nil).Times(1)
 	mockAPI.EXPECT().NVMeNamespaceGetByName(ctx, volConfig.InternalName).Return(&nvmeNamespace, nil).Times(1)
-	mockAPI.EXPECT().NVMeSubsystemCreate(ctx, "fakeHostName-fakeUUID").Return(subsystem, nil).Times(1)
+	mockAPI.EXPECT().NVMeSubsystemCreate(ctx, gomock.Any(), gomock.Any()).Return(subsystem, nil).Times(1)
 	mockAPI.EXPECT().NVMeAddHostToSubsystem(ctx, publishInfo.HostNQN, subsystem.UUID).Return(nil).Times(1)
 	mockAPI.EXPECT().NVMeEnsureNamespaceMapped(ctx, gomock.Any(), gomock.Any()).Return(nil).Times(1)
 
@@ -1571,7 +1571,7 @@ func TestPublishASANVMe(t *testing.T) {
 	// case: Error creating subsystem in CSI Context
 	driver.Config.DriverContext = tridentconfig.ContextCSI
 	mockAPI.EXPECT().VolumeInfo(ctx, volConfig.InternalName).Return(flexVol, nil).Times(1)
-	mockAPI.EXPECT().NVMeSubsystemCreate(ctx, "fakeHostName-fakeUUID").Return(
+	mockAPI.EXPECT().NVMeSubsystemCreate(ctx, gomock.Any(), gomock.Any()).Return(
 		subsystem, fmt.Errorf("Error creating subsystem")).Times(1)
 
 	err = driver.Publish(ctx, volConfig, publishInfo)
@@ -1590,7 +1590,7 @@ func TestPublishASANVMe(t *testing.T) {
 	// case: Error while adding host nqn to subsystem
 	publishInfo.HostNQN = "fakeHostNQN"
 	mockAPI.EXPECT().VolumeInfo(ctx, volConfig.InternalName).Return(flexVol, nil).Times(1)
-	mockAPI.EXPECT().NVMeSubsystemCreate(ctx, "fakeHostName-fakeUUID").Return(subsystem, nil).Times(1)
+	mockAPI.EXPECT().NVMeSubsystemCreate(ctx, gomock.Any(), gomock.Any()).Return(subsystem, nil).Times(1)
 	mockAPI.EXPECT().NVMeAddHostToSubsystem(ctx, publishInfo.HostNQN, subsystem.UUID).Return(fmt.Errorf("Error adding host nqnq to subsystem")).Times(1)
 
 	err = driver.Publish(ctx, volConfig, publishInfo)
@@ -1599,7 +1599,7 @@ func TestPublishASANVMe(t *testing.T) {
 
 	// case: Error returned by NVMeEnsureNamespaceMapped
 	mockAPI.EXPECT().VolumeInfo(ctx, volConfig.InternalName).Return(flexVol, nil).Times(1)
-	mockAPI.EXPECT().NVMeSubsystemCreate(ctx, "fakeHostName-fakeUUID").Return(subsystem, nil).Times(1)
+	mockAPI.EXPECT().NVMeSubsystemCreate(ctx, gomock.Any(), gomock.Any()).Return(subsystem, nil).Times(1)
 	mockAPI.EXPECT().NVMeAddHostToSubsystem(ctx, publishInfo.HostNQN, subsystem.UUID).Return(nil).Times(1)
 	mockAPI.EXPECT().NVMeEnsureNamespaceMapped(ctx, gomock.Any(), gomock.Any()).Return(
 		fmt.Errorf("Error returned by NVMeEnsureNamespaceMapped")).Times(1)
@@ -1610,7 +1610,7 @@ func TestPublishASANVMe(t *testing.T) {
 
 	// case: Success
 	mockAPI.EXPECT().VolumeInfo(ctx, volConfig.InternalName).Return(flexVol, nil).Times(1)
-	mockAPI.EXPECT().NVMeSubsystemCreate(ctx, "fakeHostName-fakeUUID").Return(subsystem, nil).Times(1)
+	mockAPI.EXPECT().NVMeSubsystemCreate(ctx, gomock.Any(), gomock.Any()).Return(subsystem, nil).Times(1)
 	mockAPI.EXPECT().NVMeAddHostToSubsystem(ctx, publishInfo.HostNQN, subsystem.UUID).Return(nil).Times(1)
 	mockAPI.EXPECT().NVMeEnsureNamespaceMapped(ctx, gomock.Any(), gomock.Any()).Return(nil).Times(1)
 
@@ -1621,7 +1621,7 @@ func TestPublishASANVMe(t *testing.T) {
 	// case: Success for RAW volume
 	volConfig.FileSystem = filesystem.Raw
 	mockAPI.EXPECT().VolumeInfo(ctx, volConfig.InternalName).Return(flexVol, nil).Times(1)
-	mockAPI.EXPECT().NVMeSubsystemCreate(ctx, "s_fakeInternalName").Return(subsystem, nil).Times(1)
+	mockAPI.EXPECT().NVMeSubsystemCreate(ctx, "s_fakeInternalName", "s_fakeInternalName").Return(subsystem, nil).Times(1)
 	mockAPI.EXPECT().NVMeAddHostToSubsystem(ctx, publishInfo.HostNQN, subsystem.UUID).Return(nil).Times(1)
 	mockAPI.EXPECT().NVMeEnsureNamespaceMapped(ctx, gomock.Any(), gomock.Any()).Return(nil).Times(1)
 
diff --git a/storage_drivers/ontap/ontap_common.go b/storage_drivers/ontap/ontap_common.go
@@ -5,6 +5,8 @@ package ontap
 import (
 	"bytes"
 	"context"
+	"crypto/sha256"
+	"encoding/base64"
 	"encoding/json"
 	"fmt"
 	"html/template"
@@ -24,6 +26,7 @@ import (
 	"github.com/RoaringBitmap/roaring/v2"
 	"github.com/cenkalti/backoff/v4"
 	"github.com/google/go-cmp/cmp"
+	"github.com/google/uuid"
 
 	tridentconfig "github.com/netapp/trident/config"
 	. "github.com/netapp/trident/logging"
@@ -5063,3 +5066,48 @@ func getSMBShareNamePath(flexvol, name string, secureSMBEnabled bool) (string, s
 	}
 	return flexvol, "/" + flexvol
 }
+
+// getUniqueNodeSpecificSubsystemName generates a subsystem name combining node name and Trident UUID along with a prefix.
+// If the generated subsystem name exceeds the maximum length, it attempts to shorten the Trident UUID.
+// If it still exceeds the maximum length, it generates a hash of the subsystem name and truncates the hash if needed.
+func getUniqueNodeSpecificSubsystemName(
+	nodeName, tridentUUID, prefix string, maxSubsystemLength int,
+) (string, string, error) {
+	// Ensure node name is not empty
+	if len(nodeName) == 0 {
+		return "", "", fmt.Errorf("failed to generate susbsystem name for node %v; node name is empty", nodeName)
+	}
+
+	// Ensure Trident UUID is not empty
+	if len(tridentUUID) == 0 {
+		return "", "", fmt.Errorf("failed to generate susbsystem name for node %v; Trident UUID is empty", nodeName)
+	}
+
+	// Construct the subsystem name
+	completeSSName := fmt.Sprintf("%s_%s_%s", prefix, nodeName, tridentUUID)
+	finalSSName := completeSSName
+
+	// Ensure the final name does not exceed the maximum length
+	if len(finalSSName) > maxSubsystemLength {
+		// Try reducing character count of Trident UUID.
+		// Convert it to raw bytes and then do base64 encoding. This returns 24 character string.
+		u, err := uuid.Parse(tridentUUID)
+		if err != nil {
+			return "", "", fmt.Errorf("failed to generate susbsystem name for node %v; %w", nodeName, err)
+		}
+
+		base64Str := base64.StdEncoding.EncodeToString(u[:])
+		finalSSName = fmt.Sprintf("%s_%s_%s", prefix, nodeName, base64Str)
+
+		if len(finalSSName) > maxSubsystemLength {
+			// If even after Trident UUID reduction, the length is more than max length,
+			// then generate a hash and truncate if needed.
+			finalSSName = fmt.Sprintf("%x", sha256.Sum256([]byte(completeSSName)))
+			if len(finalSSName) > maxSubsystemLength {
+				finalSSName = finalSSName[:maxSubsystemLength]
+			}
+		}
+	}
+
+	return completeSSName, finalSSName, nil
+}
diff --git a/storage_drivers/ontap/ontap_common_test.go b/storage_drivers/ontap/ontap_common_test.go
diff --git a/storage_drivers/ontap/ontap_san_nvme.go b/storage_drivers/ontap/ontap_san_nvme.go
diff --git a/storage_drivers/ontap/ontap_san_nvme_test.go b/storage_drivers/ontap/ontap_san_nvme_test.go

Original file line number	Diff line number	Diff line change
`@@ -3365,7 +3365,7 @@ func (d OntapAPIREST) NVMeRemoveHostFromSubsystem(ctx context.Context, hostNQN,`
`3365`	`3365`	`}`
`3366`	`3366`
`3367`	`3367`	`// NVMeSubsystemCreate Checks if the subsystem is already there or not. If not, creates a new one.`
`3368`		`-func (d OntapAPIREST) NVMeSubsystemCreate(ctx context.Context, subsystemName string) (*NVMeSubsystem, error) {`
	`3368`	`+func (d OntapAPIREST) NVMeSubsystemCreate(ctx context.Context, subsystemName, comment string) (*NVMeSubsystem, error) {`
`3369`	`3369`	`logFields := LogFields{`
`3370`	`3370`	`"Method": "SubsystemCreate",`
`3371`	`3371`	`"Type": "OntapAPIREST",`
`@@ -3382,7 +3382,7 @@ func (d OntapAPIREST) NVMeSubsystemCreate(ctx context.Context, subsystemName str`
`3382`	`3382`	`}`
`3383`	`3383`	`if subsystem == nil {`
`3384`	`3384`	`Logc(ctx).Infof("Subsystem doesn't exist, creating new subsystem %v now.", subsystemName)`
`3385`		`- subsystem, err = d.api.NVMeSubsystemCreate(ctx, subsystemName)`
	`3385`	`+ subsystem, err = d.api.NVMeSubsystemCreate(ctx, subsystemName, comment)`
`3386`	`3386`	`if err != nil {`
`3387`	`3387`	`return nil, err`
`3388`	`3388`	`}`
Original file line number	Diff line number	Diff line change
`@@ -2726,7 +2726,7 @@ func (d OntapAPIZAPI) NVMeNamespaceDelete(ctx context.Context, name string) erro`
`2726`	`2726`	`return errors.UnsupportedError("ZAPI call is not supported yet")`
`2727`	`2727`	`}`
`2728`	`2728`
`2729`		`-func (d OntapAPIZAPI) NVMeSubsystemCreate(ctx context.Context, subsystemName string) (*NVMeSubsystem, error) {`
	`2729`	`+func (d OntapAPIZAPI) NVMeSubsystemCreate(ctx context.Context, subsystemName, comment string) (*NVMeSubsystem, error) {`
`2730`	`2730`	`return nil, errors.UnsupportedError("ZAPI call is not supported yet")`
`2731`	`2731`	`}`
`2732`	`2732`