Implement Rename() for ontap-nas-economy and ontap-san-economy drivers

storage_drivers/ontap/ontap_nas_qtree.go

@@ -799,6 +799,9 @@ func (d *NASQtreeStorageDriver) Import(
 	return nil
 }
 
+// Rename renames a qtree in place. newName may also be a "flexvol/qtree" path (the
+// format Import() stores as ImportOriginalName) to revert a failed import; in that
+// case the FlexVol is renamed back too, but only if it holds no other qtrees.
 func (d *NASQtreeStorageDriver) Rename(ctx context.Context, name, newName string) error {
 	fields := LogFields{
 		"Method":  "Rename",
@@ -809,7 +812,75 @@ func (d *NASQtreeStorageDriver) Rename(ctx context.Context, name, newName string
 	Logd(ctx, d.Name(), d.Config.DebugTraceFlags["method"]).WithFields(fields).Trace(">>>> Rename")
 	defer Logd(ctx, d.Name(), d.Config.DebugTraceFlags["method"]).WithFields(fields).Trace("<<<< Rename")
 
-	return errors.New("rename is not implemented")
+	volumePattern, qtreeName, err := d.SetVolumePatternToFindQtree(ctx, "", name, d.FlexvolNamePrefix())
+	if err != nil {
+		return err
+	}
+
+	exists, flexvol, err := d.API.QtreeExists(ctx, qtreeName, volumePattern)
+	if err != nil {
+		return fmt.Errorf("error checking for existing qtree %s: %v", name, err)
+	}
+	if !exists {
+		return errors.NotFoundError("qtree %s not found", name)
+	}
+
+	d.flexvolLocks.Lock(flexvol)
+	defer d.flexvolLocks.Unlock(flexvol)
+
+	targetFlexvol := flexvol
+	targetQtreeName := newName
+	if pathElements := strings.Split(newName, "/"); len(pathElements) == 2 {
+		targetFlexvol = pathElements[0]
+		targetQtreeName = pathElements[1]
+	}
+
+	currentPath := fmt.Sprintf("/vol/%s/%s", flexvol, qtreeName)
+	targetPath := fmt.Sprintf("/vol/%s/%s", flexvol, targetQtreeName)
+
+	if err := d.API.QtreeRename(ctx, currentPath, targetPath); err != nil {
+		return fmt.Errorf("error renaming qtree %s to %s: %v", name, targetQtreeName, err)
+	}
+
+	if targetFlexvol == flexvol {
+		return nil
+	}
+
+	// The caller also wants the FlexVol renamed back. Only do so if this FlexVol is
+	// exclusive to the qtree we just renamed.
+	count, err := d.API.QtreeCount(ctx, flexvol)
+	if err != nil {
+		Logc(ctx).WithError(err).WithField("flexvol", flexvol).
+			Warn("Could not verify FlexVol holds only this qtree; leaving FlexVol name unchanged.")
+		return nil
+	}
+	if count != 1 {
+		Logc(ctx).WithFields(LogFields{
+			"flexvol":    flexvol,
+			"qtreeCount": count,
+		}).Warn("FlexVol holds other qtrees; leaving FlexVol name unchanged to avoid orphaning them.")
+		return nil
+	}
+
+	flexvolExists, err := d.API.VolumeExists(ctx, targetFlexvol)
+	if err != nil {
+		return fmt.Errorf("error checking for existing FlexVol %s: %v", targetFlexvol, err)
+	}
+	if flexvolExists {
+		Logc(ctx).WithField("flexvol", targetFlexvol).
+			Warn("Target FlexVol name is already in use; leaving FlexVol name unchanged.")
+		return nil
+	}
+
+	if err := d.API.VolumeRename(ctx, flexvol, targetFlexvol); err != nil {
+		// Roll back the qtree rename so we don't leave a half-reverted state.
+		if renameErr := d.API.QtreeRename(ctx, targetPath, currentPath); renameErr != nil {
+			Logc(ctx).WithError(renameErr).Warn("Failed to restore qtree name after FlexVol rename failure.")
+		}
+		return fmt.Errorf("error renaming FlexVol %s to %s: %v", flexvol, targetFlexvol, err)
+	}
+
+	return nil
 }
 
 // Destroy the volume

storage_drivers/ontap/ontap_nas_qtree_test.go

@@ -1203,10 +1203,117 @@ func TestImport_QtreeRenameFails(t *testing.T) {
 	assert.Contains(t, err.Error(), "rename failed")
 }
 
-func TestRename_NotSupported(t *testing.T) {
-	_, driver := newMockOntapNasQtreeDriver(t)
-	result := driver.Rename(ctx, "", "")
-	assert.Error(t, result, "Expected error in Rename, got nil")
+func TestRename_Success_PlainName(t *testing.T) {
+	mockAPI, driver := newMockOntapNasQtreeDriver(t)
+	flexvol := "userVol1"
+
+	mockAPI.EXPECT().QtreeExists(ctx, "old-qtree", gomock.Any()).Return(true, flexvol, nil)
+	mockAPI.EXPECT().QtreeRename(ctx, "/vol/"+flexvol+"/old-qtree", "/vol/"+flexvol+"/new-qtree").Return(nil)
+
+	err := driver.Rename(ctx, "old-qtree", "new-qtree")
+	assert.NoError(t, err)
+}
+
+func TestRename_QtreeNotFound(t *testing.T) {
+	mockAPI, driver := newMockOntapNasQtreeDriver(t)
+
+	mockAPI.EXPECT().QtreeExists(ctx, "missing-qtree", gomock.Any()).Return(false, "", nil)
+
+	err := driver.Rename(ctx, "missing-qtree", "new-qtree")
+	assert.Error(t, err)
+}
+
+func TestRename_QtreeExistsCheckFails(t *testing.T) {
+	mockAPI, driver := newMockOntapNasQtreeDriver(t)
+
+	mockAPI.EXPECT().QtreeExists(ctx, "qtree1", gomock.Any()).Return(false, "", fmt.Errorf("api error"))
+
+	err := driver.Rename(ctx, "qtree1", "new-qtree")
+	assert.Error(t, err)
+}
+
+func TestRename_QtreeRenameFails(t *testing.T) {
+	mockAPI, driver := newMockOntapNasQtreeDriver(t)
+	flexvol := "userVol1"
+
+	mockAPI.EXPECT().QtreeExists(ctx, "old-qtree", gomock.Any()).Return(true, flexvol, nil)
+	mockAPI.EXPECT().QtreeRename(ctx, "/vol/"+flexvol+"/old-qtree", "/vol/"+flexvol+"/new-qtree").
+		Return(fmt.Errorf("rename blocked"))
+
+	err := driver.Rename(ctx, "old-qtree", "new-qtree")
+	assert.Error(t, err)
+}
+
+// TestRename_ImportCleanup_RevertsFlexVol asserts that when newName is a "flexvol/qtree" path (as
+// used by import-failure cleanup via ImportOriginalName) and the current FlexVol holds only the
+// qtree being reverted, both the qtree and the FlexVol are renamed back to their original names.
+func TestRename_ImportCleanup_RevertsFlexVol(t *testing.T) {
+	mockAPI, driver := newMockOntapNasQtreeDriver(t)
+	currentFlexvol := "trident_qtree_pool_abc123"
+	originalFlexvol := "userVol1"
+
+	mockAPI.EXPECT().QtreeExists(ctx, "pvc-123", gomock.Any()).Return(true, currentFlexvol, nil)
+	mockAPI.EXPECT().QtreeRename(ctx,
+		"/vol/"+currentFlexvol+"/pvc-123", "/vol/"+currentFlexvol+"/old-qtree").Return(nil)
+	mockAPI.EXPECT().QtreeCount(ctx, currentFlexvol).Return(1, nil)
+	mockAPI.EXPECT().VolumeExists(ctx, originalFlexvol).Return(false, nil)
+	mockAPI.EXPECT().VolumeRename(ctx, currentFlexvol, originalFlexvol).Return(nil)
+
+	err := driver.Rename(ctx, "pvc-123", originalFlexvol+"/old-qtree")
+	assert.NoError(t, err)
+}
+
+// TestRename_ImportCleanup_SharedFlexVolNotReverted asserts that the FlexVol is left alone when it
+// holds other qtrees besides the one being reverted, so those other qtrees aren't orphaned.
+func TestRename_ImportCleanup_SharedFlexVolNotReverted(t *testing.T) {
+	mockAPI, driver := newMockOntapNasQtreeDriver(t)
+	currentFlexvol := "trident_qtree_pool_abc123"
+	originalFlexvol := "userVol1"
+
+	mockAPI.EXPECT().QtreeExists(ctx, "pvc-123", gomock.Any()).Return(true, currentFlexvol, nil)
+	mockAPI.EXPECT().QtreeRename(ctx,
+		"/vol/"+currentFlexvol+"/pvc-123", "/vol/"+currentFlexvol+"/old-qtree").Return(nil)
+	mockAPI.EXPECT().QtreeCount(ctx, currentFlexvol).Return(2, nil)
+
+	err := driver.Rename(ctx, "pvc-123", originalFlexvol+"/old-qtree")
+	assert.NoError(t, err)
+}
+
+// TestRename_ImportCleanup_TargetFlexVolAlreadyExists asserts that the FlexVol rename is skipped
+// when a FlexVol with the target name already exists, to avoid a collision.
+func TestRename_ImportCleanup_TargetFlexVolAlreadyExists(t *testing.T) {
+	mockAPI, driver := newMockOntapNasQtreeDriver(t)
+	currentFlexvol := "trident_qtree_pool_abc123"
+	originalFlexvol := "userVol1"
+
+	mockAPI.EXPECT().QtreeExists(ctx, "pvc-123", gomock.Any()).Return(true, currentFlexvol, nil)
+	mockAPI.EXPECT().QtreeRename(ctx,
+		"/vol/"+currentFlexvol+"/pvc-123", "/vol/"+currentFlexvol+"/old-qtree").Return(nil)
+	mockAPI.EXPECT().QtreeCount(ctx, currentFlexvol).Return(1, nil)
+	mockAPI.EXPECT().VolumeExists(ctx, originalFlexvol).Return(true, nil)
+
+	err := driver.Rename(ctx, "pvc-123", originalFlexvol+"/old-qtree")
+	assert.NoError(t, err)
+}
+
+// TestRename_ImportCleanup_FlexVolRenameFailsRollsBackQtree asserts that if the FlexVol rename
+// fails, the qtree rename is rolled back so the driver doesn't leave a half-reverted state.
+func TestRename_ImportCleanup_FlexVolRenameFailsRollsBackQtree(t *testing.T) {
+	mockAPI, driver := newMockOntapNasQtreeDriver(t)
+	currentFlexvol := "trident_qtree_pool_abc123"
+	originalFlexvol := "userVol1"
+
+	mockAPI.EXPECT().QtreeExists(ctx, "pvc-123", gomock.Any()).Return(true, currentFlexvol, nil)
+	mockAPI.EXPECT().QtreeRename(ctx,
+		"/vol/"+currentFlexvol+"/pvc-123", "/vol/"+currentFlexvol+"/old-qtree").Return(nil)
+	mockAPI.EXPECT().QtreeCount(ctx, currentFlexvol).Return(1, nil)
+	mockAPI.EXPECT().VolumeExists(ctx, originalFlexvol).Return(false, nil)
+	mockAPI.EXPECT().VolumeRename(ctx, currentFlexvol, originalFlexvol).Return(fmt.Errorf("volume rename blocked"))
+	mockAPI.EXPECT().QtreeRename(ctx,
+		"/vol/"+currentFlexvol+"/old-qtree", "/vol/"+currentFlexvol+"/pvc-123").Return(nil)
+
+	err := driver.Rename(ctx, "pvc-123", originalFlexvol+"/old-qtree")
+	assert.Error(t, err)
 }
 
 func TestDestroy_Success(t *testing.T) {

storage_drivers/ontap/ontap_san_economy.go

@@ -1193,6 +1193,9 @@ func (d *SANEconomyStorageDriver) Import(
 	return nil
 }
 
+// Rename renames a LUN in place. newName may also be a "flexvol/LUN" path (the
+// format Import() stores as ImportOriginalName) to revert a failed import; in that
+// case the FlexVol is renamed back too, but only if it holds no other LUNs.
 func (d *SANEconomyStorageDriver) Rename(ctx context.Context, name, newName string) error {
 	fields := LogFields{
 		"Method":  "Rename",
@@ -1203,7 +1206,70 @@ func (d *SANEconomyStorageDriver) Rename(ctx context.Context, name, newName stri
 	Logd(ctx, d.Name(), d.Config.DebugTraceFlags["method"]).WithFields(fields).Trace(">>>> Rename")
 	defer Logd(ctx, d.Name(), d.Config.DebugTraceFlags["method"]).WithFields(fields).Trace("<<<< Rename")
 
-	return errors.New("rename is not implemented")
+	exists, bucketVol, err := d.LUNExists(ctx, name, "", d.FlexvolNamePrefix())
+	if err != nil {
+		return fmt.Errorf("error checking for existing LUN %s: %v", name, err)
+	}
+	if !exists {
+		return errors.NotFoundError("LUN %s not found", name)
+	}
+
+	lockedFlexvol := d.lockFlexvol(bucketVol)
+	defer lockedFlexvol.Unlock()
+
+	targetBucketVol := bucketVol
+	targetLUNName := newName
+	if pathElements := strings.Split(newName, "/"); len(pathElements) == 2 {
+		targetBucketVol = pathElements[0]
+		targetLUNName = pathElements[1]
+	}
+
+	currentPath := GetLUNPathEconomy(bucketVol, name)
+	targetPath := GetLUNPathEconomy(bucketVol, targetLUNName)
+
+	if err := d.API.LunRename(ctx, currentPath, targetPath); err != nil {
+		return fmt.Errorf("error renaming LUN %s to %s: %v", name, targetLUNName, err)
+	}
+
+	if targetBucketVol == bucketVol {
+		return nil
+	}
+
+	// The caller also wants the FlexVol renamed back. Only do so if this FlexVol is
+	// exclusive to the LUN we just renamed.
+	luns, err := d.listFlexvolLUNs(ctx, bucketVol)
+	if err != nil {
+		Logc(ctx).WithError(err).WithField("flexvol", bucketVol).
+			Warn("Could not verify FlexVol holds only this LUN; leaving FlexVol name unchanged.")
+		return nil
+	}
+	if len(luns) != 1 {
+		Logc(ctx).WithFields(LogFields{
+			"flexvol":  bucketVol,
+			"lunCount": len(luns),
+		}).Warn("FlexVol holds other LUNs; leaving FlexVol name unchanged to avoid orphaning them.")
+		return nil
+	}
+
+	flexvolExists, err := d.API.VolumeExists(ctx, targetBucketVol)
+	if err != nil {
+		return fmt.Errorf("error checking for existing FlexVol %s: %v", targetBucketVol, err)
+	}
+	if flexvolExists {
+		Logc(ctx).WithField("flexvol", targetBucketVol).
+			Warn("Target FlexVol name is already in use; leaving FlexVol name unchanged.")
+		return nil
+	}
+
+	if err := d.API.VolumeRename(ctx, bucketVol, targetBucketVol); err != nil {
+		// Roll back the LUN rename so we don't leave a half-reverted state.
+		if renameErr := d.API.LunRename(ctx, targetPath, currentPath); renameErr != nil {
+			Logc(ctx).WithError(renameErr).Warn("Failed to restore LUN name after FlexVol rename failure.")
+		}
+		return fmt.Errorf("error renaming FlexVol %s to %s: %v", bucketVol, targetBucketVol, err)
+	}
+
+	return nil
 }
 
 // Destroy the LUN