From 86689efcc0b3d8e5eab03b4b0006a36027210d8d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Moritz=20Wanzenb=C3=B6ck?= Date: Fri, 4 Oct 2024 16:36:42 +0200 Subject: [PATCH] do not try to resize block volumes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Block volumes cannot be resized, so there is no point in even attempting it. An attempt would fail because it could not determine the FS one the volume, as there likely isn't any. A retry would then succeed as there is an earlier check if we need to mount at all. The fix is to rearrange the logic to only attempt to resize filesystem volumes. While at it, also remove the deprecated "IsNotMountPoint". The double negative was getting confusing in any case. Signed-off-by: Moritz Wanzenböck --- CHANGELOG.md | 5 +++++ pkg/client/linstor.go | 30 ++++++++++++++++-------------- pkg/client/mock.go | 17 ++++++++++------- pkg/driver/driver.go | 4 ++-- pkg/volume/volume.go | 2 +- 5 files changed, 34 insertions(+), 24 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 655aec6..73a9d78 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + +- Fixed an issue that caused the node publish call to attempt to resize a block volume: block volumes are now never + attempted to be resized. + ## [1.6.3] - 2024-06-28 ### Fixed diff --git a/pkg/client/linstor.go b/pkg/client/linstor.go index 3753666..7a8aeaa 100644 --- a/pkg/client/linstor.go +++ b/pkg/client/linstor.go @@ -1975,28 +1975,30 @@ func (s *Linstor) Mount(ctx context.Context, source, target, fsType string, read } } - needsMount, err := mount.IsNotMountPoint(s.mounter, target) + isMounted, err := s.mounter.IsMountPoint(target) if err != nil { return fmt.Errorf("unable to determine mount status of %s %v", target, err) } - if !needsMount { - return nil + if !isMounted { + err = s.mounter.Mount(source, target, fsType, mntOpts) + if err != nil { + return err + } } - err = s.mounter.Mount(source, target, fsType, mntOpts) - if err != nil { - return err + if block { + return nil } resizerFs := mount.NewResizeFs(s.mounter.Exec) - resize, err := resizerFs.NeedResize(source, target) + needResize, err := resizerFs.NeedResize(source, target) if err != nil { return fmt.Errorf("unable to determine if resize required: %w", err) } - if !block && resize { + if needResize { _, err := resizerFs.Resize(source, target) if err != nil { unmountErr := s.Unmount(target) @@ -2021,20 +2023,20 @@ func (s *Linstor) setDevReadWrite(ctx context.Context, srcPath string) error { return err } -// IsNotMountPoint determines if a directory is a mountpoint. +// IsMountPoint determines if a directory is a mountpoint. // -// Non-existent paths return (true, nil). -func (s *Linstor) IsNotMountPoint(target string) (bool, error) { - notMounted, err := mount.IsNotMountPoint(s.mounter, target) +// Non-existent paths return (false, nil). +func (s *Linstor) IsMountPoint(target string) (bool, error) { + mounted, err := s.mounter.IsMountPoint(target) if err != nil { if os.IsNotExist(err) { - return true, nil + return false, nil } return false, err } - return notMounted, nil + return mounted, nil } // Unmount unmounts the target. Operates locally on the machines where it is called. diff --git a/pkg/client/mock.go b/pkg/client/mock.go index bf66013..29d08bd 100644 --- a/pkg/client/mock.go +++ b/pkg/client/mock.go @@ -258,27 +258,30 @@ func (s *MockStorage) Mount(ctx context.Context, source, target, fsType string, return nil } -func (s *MockStorage) IsNotMountPoint(target string) (bool, error) { +func (s *MockStorage) IsMountPoint(target string) (bool, error) { _, err := os.Stat(target) if err != nil { if os.IsNotExist(err) { - return true, nil + return false, nil } + return false, err } - return false, nil + + return true, nil } func (s *MockStorage) Unmount(target string) error { - notMounted, err := s.IsNotMountPoint(target) + mounted, err := s.IsMountPoint(target) if err != nil { return err } - if notMounted { - return nil + + if mounted { + return os.RemoveAll(target) } - return os.RemoveAll(target) + return nil } func (s *MockStorage) GetVolumeStats(path string) (volume.VolumeStats, error) { diff --git a/pkg/driver/driver.go b/pkg/driver/driver.go index f9ea935..b4fc9ef 100644 --- a/pkg/driver/driver.go +++ b/pkg/driver/driver.go @@ -430,12 +430,12 @@ func (d Driver) NodeGetVolumeStats(ctx context.Context, req *csi.NodeGetVolumeSt return nil, missingAttr("NodeGetVolumeStats", req.GetVolumeId(), "VolumeId") } - notMounted, err := d.Mounter.IsNotMountPoint(req.GetVolumePath()) + mounted, err := d.Mounter.IsMountPoint(req.GetVolumePath()) if err != nil { return nil, status.Errorf(codes.Internal, "NodeGetVolumeStats failed for %s: failed to check if path %v is mounted: %v", req.GetVolumeId(), req.GetVolumePath(), err) } - if notMounted { + if !mounted { return nil, status.Errorf(codes.NotFound, "NodeGetVolumeStats failed for %s: path %v is not mounted", req.GetVolumeId(), req.GetVolumePath()) } diff --git a/pkg/volume/volume.go b/pkg/volume/volume.go index 3fbcfd4..f665063 100644 --- a/pkg/volume/volume.go +++ b/pkg/volume/volume.go @@ -112,7 +112,7 @@ type Querier interface { type Mounter interface { Mount(ctx context.Context, source, target, fsType string, readonly bool, mntOpts []string) error Unmount(target string) error - IsNotMountPoint(target string) (bool, error) + IsMountPoint(target string) (bool, error) } // VolumeStats provides details about filesystem usage.