Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix nvme block resize #366

Merged
merged 4 commits into from
Oct 25, 2024
Merged

Fix nvme block resize #366

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9b4fa1e
Fix nvme block resize
AkshaySainiDell Oct 25, 2024
5662664
Add mock for GetNVMeControllerwq
AkshaySainiDell Oct 25, 2024
25230af
Fix multipath resize error
AkshaySainiDell Oct 25, 2024
08b603d
Merge branch 'main' into bugfix-1534-nvme-block-resize
AkshaySainiDell Oct 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@ require (
github.com/dell/dell-csi-extensions/volumeGroupSnapshot v1.6.0
github.com/dell/gobrick v1.11.5-0.20241014113235-06cfe9928229
github.com/dell/gocsi v1.11.0
github.com/dell/gofsutil v1.16.2-0.20241024070128-65a51f273f8a
github.com/dell/gofsutil v1.16.2-0.20241025084148-087a7378a075
github.com/dell/goiscsi v1.9.0
github.com/dell/gonvme v1.8.1
github.com/dell/gonvme v1.8.2-0.20241025082321-e7b104b7df7c
github.com/dell/gopowerstore v1.15.2-0.20241008144633-6990d1b86e5b
github.com/fsnotify/fsnotify v1.7.0
github.com/go-openapi/strfmt v0.23.0
Expand Down
8 changes: 4 additions & 4 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,12 +131,12 @@ github.com/dell/gobrick v1.11.5-0.20241014113235-06cfe9928229 h1:jer/8oHRkUe3Kgc
github.com/dell/gobrick v1.11.5-0.20241014113235-06cfe9928229/go.mod h1:+qnDjbft08dV0s5BWG/R6KUFnS3nxJRqfALV1ficbWI=
github.com/dell/gocsi v1.11.0 h1:P84VOPd1V55JQjx4tfd/6QOlVQRQkYUqmGqbzPKeyUQ=
github.com/dell/gocsi v1.11.0/go.mod h1:LzGAsEIjBxVXJuabzsG3/MsdCOczxDE1IWOBxzXIUhw=
github.com/dell/gofsutil v1.16.2-0.20241024070128-65a51f273f8a h1:20B30iO7h5cv/DtC5Hts3iPxMOsrL/V+uHiAyQRfnlY=
github.com/dell/gofsutil v1.16.2-0.20241024070128-65a51f273f8a/go.mod h1:PN2hWl/pVLQiTsFR0X1x+GfhfOrfW8pGgH5xGcGMeFs=
github.com/dell/gofsutil v1.16.2-0.20241025084148-087a7378a075 h1:c1Hk/MW7pPP/9OnzNl1PceeI8QkQaqRsCzu0OegTtR4=
github.com/dell/gofsutil v1.16.2-0.20241025084148-087a7378a075/go.mod h1:PN2hWl/pVLQiTsFR0X1x+GfhfOrfW8pGgH5xGcGMeFs=
github.com/dell/goiscsi v1.9.0 h1:VvMHbAO4vk80oc/TAbQPYlxysscCqVBW78GyPoUxgik=
github.com/dell/goiscsi v1.9.0/go.mod h1:NI/W/0O1UrMW2zVdMxy4z395Jn0r7utH6RQDFSZiFyQ=
github.com/dell/gonvme v1.8.1 h1:46M5lPqj7+Xjen+qxooRN9cx/+uJG4xtK9TpwduWDgE=
github.com/dell/gonvme v1.8.1/go.mod h1:ajbuF+fswq+ty2tRTG5FN4ecIMJsG7aDu/bkMynTKAs=
github.com/dell/gonvme v1.8.2-0.20241025082321-e7b104b7df7c h1:ftk2SoZT0lHeIHrDrfK9XWblcXoL2NIjQU+Ux6Qld+w=
github.com/dell/gonvme v1.8.2-0.20241025082321-e7b104b7df7c/go.mod h1:5IgWNLcuffHzuXSa6YH3APHiET/hROouuj3mg7GPoqQ=
github.com/dell/gopowerstore v1.15.2-0.20241008144633-6990d1b86e5b h1:wVpvFdhc4FIl7w73fm9aTGtE3iTfoO5Ag7/WQUNdHUU=
github.com/dell/gopowerstore v1.15.2-0.20241008144633-6990d1b86e5b/go.mod h1:EQEsDdulHtvpDIe2tMCW/2swr1enisuzW4Zmw6c2SKc=
github.com/dgrijalva/jwt-go v3.2.0+incompatible/go.mod h1:E3ru+11k8xSBh+hMPgOLZmtrrCbhqsmaPHjLKYnJCaQ=
Expand Down
28 changes: 28 additions & 0 deletions mocks/UtilInterface.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions pkg/common/fs/fs.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ type UtilInterface interface {
ResizeMultipath(ctx context.Context, deviceName string) error
FindFSType(ctx context.Context, mountpoint string) (fsType string, err error)
GetMpathNameFromDevice(ctx context.Context, device string) (string, error)
GetNVMeController(device string) (string, error)
}

// Fs implementation of FsInterface that uses default os/file calls
Expand Down
1 change: 1 addition & 0 deletions pkg/node/base.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ const (
powerStoreMaxNodeNameLength = 64
blockVolumePathMarker = "/csi/volumeDevices/publish/"
sysBlock = "/sys/block/"
dev = "/dev/"
defaultNodeNamePrefix = "csi-node"
defaultNodeChrootPath = "/noderoot"

Expand Down
29 changes: 23 additions & 6 deletions pkg/node/node.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1022,12 +1022,29 @@ func (s *Service) nodeExpandRawBlockVolume(ctx context.Context, volumeWWN string
if len(deviceNames) > 0 {
var devName string
for _, deviceName := range deviceNames {
devicePath := sysBlock + deviceName
log.Infof("Rescanning unmounted (raw block) device %s to expand size", deviceName)
err = s.Fs.GetUtil().DeviceRescan(context.Background(), devicePath)
if err != nil {
log.Errorf("Failed to rescan device (%s) with error (%s)", devicePath, err.Error())
return nil, status.Error(codes.Internal, err.Error())
if strings.HasPrefix(deviceName, "nvme") {
nvmeControllerDevice, err := s.Fs.GetUtil().GetNVMeController(deviceName)
if err != nil {
log.Errorf("Failed to rescan device (%s) with error (%s)", deviceName, err.Error())
return nil, status.Error(codes.Internal, err.Error())
Comment on lines +1026 to +1029
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why would the GetNVMeController() call fail? Is bailing out the proper handling? Could anything else be done? The error message is inaccurate and can cause problems during troubleshooting. Is it better to indicate that there was a failure to get the device controller?

}
if nvmeControllerDevice != "" {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So there seems to be a place where nvmeControllerDevice can be empty but GetNVMeController() returns no error. Is this code still then correct?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For PowerMax, we have two devices: nvme0c0n1 and nvme0n1.
image

Here, nvme0c0n1 is a controller, but nvme0n1 is not.

The GetNVMeController function processes each device as follows:

  1. nvme0c0n1 - Successfully returns the controller nvme0.
  2. nvme0n1 - print logger Not a valid nvme controller device: nvme0n1 and return empty nvmeControllerDevice

devicePath := dev + nvmeControllerDevice
log.Infof("Rescanning unmounted (raw block) device %s to expand size", devicePath)
err = s.nvmeLib.DeviceRescan(devicePath)
if err != nil {
log.Errorf("Failed to rescan device (%s) with error (%s)", devicePath, err.Error())
return nil, status.Error(codes.Internal, err.Error())
}
}
} else {
devicePath := sysBlock + deviceName
log.Infof("Rescanning unmounted (raw block) device %s to expand size", deviceName)
err = s.Fs.GetUtil().DeviceRescan(context.Background(), devicePath)
if err != nil {
log.Errorf("Failed to rescan device (%s) with error (%s)", devicePath, err.Error())
return nil, status.Error(codes.Internal, err.Error())
}
}
devName = deviceName
}
Expand Down