Bolster nodeExpandVolume for iSCSI for fast repeated resizes

Author: zachatnetapp

frontend/csi/node_server.go

@@ -598,7 +598,11 @@ func (p *Plugin) nodeExpandVolume(
 		return status.Errorf(codes.FailedPrecondition, "unable to read protocol info from publish info; %s", err)
 	}
 
-	var fsType, mountOptions string
+	var (
+		fsType, mountOptions     string
+		preExpandFilesystemSize  int64
+		preExpandDeviceSizeBytes int64
+	)
 
 	switch protocol {
 	case tridentconfig.File:
@@ -608,6 +612,25 @@ func (p *Plugin) nodeExpandVolume(
 		if fsType, err = filesystem.VerifyFilesystemSupport(publishInfo.FilesystemType); err != nil {
 			break
 		}
+
+		// Capture pre-expand filesystem size via statfs on the kubelet-staged mount.
+		if fsType != filesystem.Raw {
+			preExpandFilesystemSize, err = p.fs.GetFilesystemSize(ctx, stagingTargetPath)
+			if err != nil {
+				return status.Error(codes.Internal, err.Error())
+			}
+		}
+
+		// Capture pre-expand block device size before any rescan/resize work.
+		if publishInfo.DevicePath != "" {
+			preExpandDeviceSizeBytes, err = p.devices.GetDiskSize(ctx, publishInfo.DevicePath)
+			if err != nil {
+				Logc(ctx).WithFields(LogFields{
+					"devicePath": publishInfo.DevicePath,
+				}).WithError(err).Warn("Failed to read pre-expand device size; skipping device growth check.")
+			}
+		}
+
 		// We don't need to rescan mount devices for NVMe protocol backend. Automatic namespace rescanning happens
 		// everytime the NVMe controller is reset, or if the controller posts an asynchronous event indicating
 		// namespace attributes have changed.
@@ -663,19 +686,43 @@ func (p *Plugin) nodeExpandVolume(
 		}
 	}
 
+	// Read the device size after rescan/resize but before filesystem expand.
+	var postExpandDeviceSizeBytes int64
+	if publishInfo.DevicePath != "" {
+		postExpandDeviceSizeBytes, err = p.devices.GetDiskSize(ctx, publishInfo.DevicePath)
+		if err != nil {
+			Logc(ctx).WithFields(LogFields{
+				"devicePath": publishInfo.DevicePath,
+			}).WithError(err).Warn("Failed to read post-expand device size; skipping device growth check.")
+		}
+	}
+	devicesGrew := preExpandDeviceSizeBytes > 0 && postExpandDeviceSizeBytes > preExpandDeviceSizeBytes
+
 	// Expand filesystem.
 	if fsType != filesystem.Raw {
-		filesystemSize, err := p.fs.ExpandFilesystemOnNode(ctx, publishInfo, devicePath, stagingTargetPath, fsType,
-			mountOptions)
+		newFilesystemSize, err := p.fs.ExpandFilesystemOnNode(ctx, publishInfo, devicePath, stagingTargetPath, fsType,
+			mountOptions, requiredBytes)
 		if err != nil {
 			Logc(ctx).WithFields(LogFields{
 				"device":         publishInfo.DevicePath,
 				"filesystemType": fsType,
 			}).WithError(err).Error("Unable to expand filesystem.")
 			return status.Error(codes.Internal, err.Error())
 		}
+
+		if devicesGrew && newFilesystemSize <= preExpandFilesystemSize {
+			Logc(ctx).WithFields(LogFields{
+				"preExpandFilesystemSize":   preExpandFilesystemSize,
+				"newFilesystemSize":         newFilesystemSize,
+				"preExpandDeviceSizeBytes":  preExpandDeviceSizeBytes,
+				"postExpandDeviceSizeBytes": postExpandDeviceSizeBytes,
+				"requiredBytes":             requiredBytes,
+			}).Error("Filesystem did not grow despite block device growing during expand.")
+			return status.Error(codes.Internal, "filesystem size did not grow")
+		}
+
 		Logc(ctx).WithFields(LogFields{
-			"filesystemSize": filesystemSize,
+			"filesystemSize": newFilesystemSize,
 			"requiredBytes":  requiredBytes,
 		}).Debug("Filesystem size after expand.")
 	}

frontend/csi/node_server_test.go

@@ -60,11 +60,13 @@ type Filesystem interface {
 	RepairVolume(ctx context.Context, device, fstype string)
 	ExpandFilesystemOnNode(
 		ctx context.Context, publishInfo *models.VolumePublishInfo, devicePath, stagedTargetPath, fsType, mountOptions string,
+		targetSizeBytes int64,
 	) (int64, error)
 	DeleteFile(ctx context.Context, filepath, fileDescription string) (string, error)
 	GetFilesystemStats(
 		ctx context.Context, path string,
 	) (available, capacity, usage, inodes, inodesFree, inodesUsed int64, err error)
+	GetFilesystemSize(ctx context.Context, path string) (int64, error)
 	GetUnmountPath(ctx context.Context, trackingInfo *models.VolumeTrackingInfo) (string, error)
 	ScanFile(filename string) ([]byte, error)
 	ScanDir(path string) ([]os.FileInfo, error)
@@ -93,6 +95,12 @@ func NewDetailed(command tridentexec.Command, osFs afero.Fs, mount Mount) *FSCli
 	}
 }
 
+// GetFilesystemSize returns the total capacity in bytes of the filesystem at the given path.
+func (f *FSClient) GetFilesystemSize(ctx context.Context, path string) (int64, error) {
+	_, capacity, _, _, _, _, err := f.GetFilesystemStats(ctx, path)
+	return capacity, err
+}
+
 // GetDFOutput returns parsed DF output
 func (f *FSClient) GetDFOutput(ctx context.Context) ([]models.DFInfo, error) {
 	GenerateRequestContextForLayer(ctx, LogLayerUtils)
@@ -254,9 +262,10 @@ func (f *FSClient) RepairVolume(ctx context.Context, device, fstype string) {
 	}
 }
 
-// ExpandFilesystemOnNode will expand the filesystem of an already expanded volume.
+// ExpandFilesystemOnNode expands the filesystem after the volume capacity step.
 func (f *FSClient) ExpandFilesystemOnNode(
 	ctx context.Context, publishInfo *models.VolumePublishInfo, devicePath, stagedTargetPath, fsType, mountOptions string,
+	targetSizeBytes int64,
 ) (int64, error) {
 	GenerateRequestContextForLayer(ctx, LogLayerUtils)
 
@@ -269,6 +278,7 @@ func (f *FSClient) ExpandFilesystemOnNode(
 		"mountOptions":      mountOptions,
 		"filesystemType":    fsType,
 		"stagingMountpoint": expansionMountPoint,
+		"targetSizeBytes":   targetSizeBytes,
 	}
 	Logc(ctx).WithFields(logFields).Debug(">>>> filesystem.ExpandFilesystemOnNode")
 	defer Logc(ctx).WithFields(logFields).Debug("<<<< filesystem.ExpandFilesystemOnNode")
@@ -289,9 +299,17 @@ func (f *FSClient) ExpandFilesystemOnNode(
 	var size int64
 	switch fsType {
 	case "xfs":
-		size, err = f.expandFilesystem(ctx, "xfs_growfs", expansionMountPoint, expansionMountPoint)
+		size, err = expandFilesystem(
+			ctx, f.command,
+			"xfs_growfs", expansionMountPoint, expansionMountPoint,
+			f.GetFilesystemSize,
+		)
 	case "ext3", "ext4":
-		size, err = f.expandFilesystem(ctx, "resize2fs", devicePath, expansionMountPoint)
+		size, err = expandFilesystem(
+			ctx, f.command,
+			"resize2fs", devicePath, expansionMountPoint,
+			f.GetFilesystemSize,
+		)
 	default:
 		err = fmt.Errorf("unsupported file system type: %s", fsType)
 	}
@@ -301,34 +319,31 @@ func (f *FSClient) ExpandFilesystemOnNode(
 	return size, err
 }
 
-func (f *FSClient) expandFilesystem(ctx context.Context, cmd, cmdArguments, tmpMountPoint string) (int64, error) {
+// expandFilesystem runs the resize tool and returns the post-expand filesystem size.
+func expandFilesystem(
+	ctx context.Context, cmd tridentexec.Command,
+	cmdName, cmdArguments, tmpMountPoint string,
+	getFilesystemSize func(context.Context, string) (int64, error),
+) (int64, error) {
 	logFields := LogFields{
-		"cmd":           cmd,
+		"cmd":           cmdName,
 		"cmdArguments":  cmdArguments,
 		"tmpMountPoint": tmpMountPoint,
 	}
 	Logc(ctx).WithFields(logFields).Debug(">>>> filesystem.expandFilesystem")
 	defer Logc(ctx).WithFields(logFields).Debug("<<<< filesystem.expandFilesystem")
 
-	preExpandSize, err := f.getFilesystemSize(ctx, tmpMountPoint)
-	if err != nil {
-		return 0, err
-	}
-	_, err = f.command.Execute(ctx, cmd, cmdArguments)
+	_, err := cmd.Execute(ctx, cmdName, cmdArguments)
 	if err != nil {
 		Logc(ctx).Errorf("Expanding filesystem failed; %s", err)
 		return 0, err
 	}
 
-	postExpandSize, err := f.getFilesystemSize(ctx, tmpMountPoint)
+	postExpandSize, err := getFilesystemSize(ctx, tmpMountPoint)
 	if err != nil {
 		return 0, err
 	}
 
-	if postExpandSize == preExpandSize {
-		Logc(ctx).Warnf("Failed to expand filesystem; size=%d", postExpandSize)
-	}
-
 	return postExpandSize, nil
 }
 

mocks/mock_utils/mock_filesystem/mock_filesystem_client.go

@@ -19,11 +19,11 @@ func (f *FSClient) GetFilesystemStats(ctx context.Context, _ string) (int64, int
 	return 0, 0, 0, 0, 0, 0, errors.UnsupportedError("GetFilesystemStats is not supported for darwin")
 }
 
-// getFilesystemSize unused stub function
-func (f *FSClient) getFilesystemSize(ctx context.Context, _ string) (int64, error) {
-	Logc(ctx).Debug(">>>> filesystem_darwin.getFilesystemSize")
-	defer Logc(ctx).Debug("<<<< filesystem_darwin.getFilesystemSize")
-	return 0, errors.UnsupportedError("getFilesystemSize is not supported for darwin")
+// getBlockDeviceSize unused stub function.
+func (f *FSClient) getBlockDeviceSize(ctx context.Context, _ string) (int64, error) {
+	Logc(ctx).Debug(">>>> filesystem_darwin.getBlockDeviceSize")
+	defer Logc(ctx).Debug("<<<< filesystem_darwin.getBlockDeviceSize")
+	return 0, errors.UnsupportedError("getBlockDeviceSize is not supported for darwin")
 }
 
 // GetDeviceFilePath returns the staging path for volume.

utils/filesystem/filesystem.go

@@ -12,16 +12,6 @@ import (
 	"github.com/netapp/trident/utils/models"
 )
 
-func TestGetFilesystemSize(t *testing.T) {
-	ctx := context.Background()
-	fsClient := New(nil)
-
-	result, err := fsClient.getFilesystemSize(ctx, "")
-	assert.Equal(t, result, int64(0), "got non-zero filesystem size")
-	assert.Error(t, err, "no error")
-	assert.True(t, errors.IsUnsupportedError(err), "not UnsupportedError")
-}
-
 func TestGetFilesystemStats(t *testing.T) {
 	ctx := context.Background()
 	fsClient := New(nil)

utils/filesystem/filesystem_darwin.go

@@ -7,7 +7,12 @@ package filesystem
 import (
 	"context"
 	"fmt"
+	"os"
+	"syscall"
 	"time"
+	"unsafe"
+
+	"golang.org/x/sys/unix"
 
 	"github.com/netapp/trident/internal/syswrap"
 	. "github.com/netapp/trident/logging"
@@ -53,15 +58,21 @@ func (f *FSClient) GetFilesystemStats(
 	return available, capacity, usage, inodes, inodesFree, inodesUsed, nil
 }
 
-// getFilesystemSize returns the size of the filesystem for the given path.
-// The caller of the func is responsible for verifying the mountPoint existence and readiness.
-func (f *FSClient) getFilesystemSize(ctx context.Context, path string) (int64, error) {
-	Logc(ctx).Debug(">>>> filesystem_linux.getFilesystemSize")
-	defer Logc(ctx).Debug("<<<< filesystem_linux.getFilesystemSize")
+// getBlockDeviceSize reports devicePath capacity via BLKGETSIZE64 (kernel-visible; may trail provisioned size slightly).
+func (f *FSClient) getBlockDeviceSize(ctx context.Context, devicePath string) (int64, error) {
+	Logc(ctx).WithField("devicePath", devicePath).Debug(">>>> filesystem_linux.getBlockDeviceSize")
+	defer Logc(ctx).WithField("devicePath", devicePath).Debug("<<<< filesystem_linux.getBlockDeviceSize")
 
-	_, size, _, _, _, _, err := f.GetFilesystemStats(ctx, path)
+	disk, err := os.Open(devicePath)
 	if err != nil {
-		return 0, err
+		return 0, fmt.Errorf("failed to open block device %s: %w", devicePath, err)
+	}
+	defer disk.Close()
+
+	var size int64
+	_, _, errno := syscall.Syscall(syscall.SYS_IOCTL, disk.Fd(), unix.BLKGETSIZE64, uintptr(unsafe.Pointer(&size)))
+	if errno != 0 {
+		return 0, fmt.Errorf("BLKGETSIZE64 ioctl failed for %s: %w", devicePath, os.NewSyscallError("ioctl", errno))
 	}
 
 	return size, nil