From f46191938ba3d761431c5539fb360dfe45061e8a Mon Sep 17 00:00:00 2001 From: happytreees Date: Mon, 13 May 2024 14:13:25 -0400 Subject: [PATCH 1/3] fix xfs mount issues --- Dockerfile | 4 +- driver/driver.go | 3 +- driver/mounter.go | 300 ---------------------------------------------- driver/node.go | 12 +- 4 files changed, 11 insertions(+), 308 deletions(-) delete mode 100644 driver/mounter.go diff --git a/Dockerfile b/Dockerfile index e13cbdcb..40df1e50 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,7 +1,7 @@ -FROM alpine:latest +FROM alpine:3.18 RUN apk update -RUN apk add --no-cache ca-certificates e2fsprogs findmnt bind-tools e2fsprogs-extra xfsprogs blkid +RUN apk add --no-cache ca-certificates e2fsprogs findmnt bind-tools e2fsprogs-extra xfsprogs xfsprogs-extra blkid ADD csi-vultr-plugin / ENTRYPOINT ["/csi-vultr-plugin"] \ No newline at end of file diff --git a/driver/driver.go b/driver/driver.go index 90816ce1..b0b95f60 100644 --- a/driver/driver.go +++ b/driver/driver.go @@ -46,8 +46,7 @@ type VultrDriver struct { isController bool waitTimeout time.Duration - log *logrus.Entry - vMounter Mounter + log *logrus.Entry mounter *mount.SafeFormatAndMount resizer *mount.ResizeFs diff --git a/driver/mounter.go b/driver/mounter.go deleted file mode 100644 index f80b1536..00000000 --- a/driver/mounter.go +++ /dev/null @@ -1,300 +0,0 @@ -package driver - -import ( - "errors" - "fmt" - "os" - "os/exec" - "strconv" - "strings" - "syscall" - - "github.com/sirupsen/logrus" - "golang.org/x/sys/unix" -) - -const ( - runningState = "running" - blkidExitStatusNoIdentifiers = 2 - mkDirMode = 0750 -) - -// Mounter is the type interface for the mounter -type Mounter interface { - Format(source, fs string) error - IsFormatted(source string) (bool, error) - Mount(source, target, fs string, opts ...string) error - IsMounted(target string) (bool, error) - UnMount(target string) error - GetStatistics(target string) (volumeStatistics, error) - IsBlockDevice(target string) (bool, error) -} - -type volumeStatistics struct { - availableBytes, totalBytes, usedBytes int64 - availableInodes, totalInodes, usedInodes int64 -} - -type mounter struct { - log *logrus.Entry -} - -// NewMounter initializes the mounter -func NewMounter(log *logrus.Entry) *mounter { - return &mounter{log: log} -} - -func (m *mounter) Format(source, fs string) error { - if fs == "" { - return errors.New("fs type was not provided - required for formatting the volume") - } - - if source == "" { - return errors.New("source type was not provided - required for formatting the volume") - } - - mkFs := fmt.Sprintf("mkfs.%s", fs) - _, err := exec.LookPath(mkFs) - if err != nil { - if err == exec.ErrNotFound { - return fmt.Errorf("%q executable not found in $PATH", mkFs) - } - return err - } - - argument := []string{} - argument = append(argument, source) - if fs == "ext4" || fs == "ext3" { //nolint:goconst - argument = []string{"-F", source} - } - - m.log.WithFields(logrus.Fields{ - "source": source, - "fs-type": fs, - "format-cmd": mkFs, - "format-args": argument, - }).Info("Format called") - - out, err := exec.Command(mkFs, argument...).CombinedOutput() - - if err != nil { - return fmt.Errorf("formatting disk failed: %v cmd: '%s %s' output: %q", - err, mkFs, strings.Join(argument, " "), string(out)) - } - - return nil -} - -func (m *mounter) IsFormatted(source string) (bool, error) { - if source == "" { - return false, errors.New("source name was not provided") - } - - blkidCmd := "blkid" - _, err := exec.LookPath(blkidCmd) - if err != nil { - return false, fmt.Errorf("%q not found in $PATH", blkidCmd) - } - - blkidArgs := []string{source} - - m.log.WithFields(logrus.Fields{ - "format-command": blkidCmd, - "format-args": blkidArgs, - }).Info("isFormatted called") - - exitCode := 0 - cmd := exec.Command(blkidCmd, blkidArgs...) - err = cmd.Run() - if err != nil { - exitError, ok := err.(*exec.ExitError) - if !ok { - return false, fmt.Errorf("checking formatting failed: %v cmd: %q, args: %q", err, blkidCmd, blkidArgs) - } - ws := exitError.Sys().(syscall.WaitStatus) - exitCode = ws.ExitStatus() - if exitCode == blkidExitStatusNoIdentifiers { - return false, nil - } - return false, fmt.Errorf("checking formatting failed: %v cmd: %q, args: %q", err, blkidCmd, blkidArgs) - } - - return true, nil -} - -func (m *mounter) Mount(source, target, fs string, opts ...string) error { - if source == "" { - return errors.New("source type was not provided - required for mounting") - } - - if target == "" { - return errors.New("target type was not provided - required for mounting") - } - - if fs == "" { - return errors.New("fs type was not provided - required for mounting") - } - - m.log.WithFields(logrus.Fields{ - "source": source, - "target": target, - "filesystem": fs, - "options": opts, - "methods": "mount", - }).Info("Mount Called") - - mountCommand := "mount" - mountArguments := []string{} - - mountArguments = append(mountArguments, "-t", fs) - err := os.MkdirAll(target, mkDirMode) - if err != nil { - return err - } - - if len(opts) > 0 { - mountArguments = append(mountArguments, "-o", strings.Join(opts, ",")) - } - - mountArguments = append(mountArguments, source, target) - - m.log.WithFields(logrus.Fields{ - "mount command": mountCommand, - "mount arguments": mountArguments, - }).Info("mount command and arguments") - - for i, m := range mountArguments { - mountArguments[i] = strings.Trim(m, "\n") - } - - out, err := exec.Command(mountCommand, mountArguments...).CombinedOutput() - if err != nil { - return fmt.Errorf("mounting failed: %v cmd: '%s %s' output: %q", - err, mountCommand, strings.Join(mountArguments, " "), string(out)) - } - - if _, err := os.Stat(target + "/lost+found"); err == nil { - if errRmv := os.Remove(target + "/lost+found"); errRmv != nil { - m.log.WithFields(logrus.Fields{ - "error": errRmv, - }).Info("mount command - removal of lost+found error") - } - } else if os.IsNotExist(err) { - m.log.WithFields(logrus.Fields{ - "error": err, - }).Info("mount command - removal of lost+found error") - } - - return nil -} - -func (m *mounter) IsMounted(target string) (bool, error) { - if target == "" { - return false, errors.New("target path was not provided") - } - - findmntCmd := "findmnt" - _, err := exec.LookPath(findmntCmd) - if err != nil { - if err == exec.ErrNotFound { - return false, fmt.Errorf("%q not found in $PATH", findmntCmd) - } - return false, err - } - - cmdArgs := []string{"-o", "TARGET", "-T", target} - out, err := exec.Command(findmntCmd, cmdArgs...).CombinedOutput() - if err != nil { - // not an error, just nothing found. - if strings.TrimSpace(string(out)) == "" { - return false, nil - } - - return false, fmt.Errorf("checking mount failed with command %v: %v", findmntCmd, err) - } - - if len(out) == 0 { - return false, nil - } - - if strings.Contains(string(out), target) { - return true, nil - } - - return false, nil -} - -func (m *mounter) UnMount(target string) error { - umountCmd := "umount" - if target == "" { - return errors.New("target is not specified for unmounting the volume") - } - - umountArgs := []string{target} - - m.log.WithFields(logrus.Fields{ - "cmd": umountCmd, - "args": umountArgs, - }).Info("executing umount command") - - out, err := exec.Command(umountCmd, umountArgs...).CombinedOutput() - if err != nil { - return fmt.Errorf("unmounting failed: %v cmd: '%s %s' output: %q", - err, umountCmd, target, string(out)) - } - - return nil -} - -func (m *mounter) GetStatistics(target string) (volumeStatistics, error) { - isBlock, err := m.IsBlockDevice(target) - if err != nil { - return volumeStatistics{}, fmt.Errorf("failed to determine if volume %s is block device: %v", target, err) - } - - if isBlock { - output, errCommand := exec.Command("blockdev", "getsize64", target).CombinedOutput() - if errCommand != nil { - errFmt := fmt.Errorf("error when getting size of block volume at path %s: output: %s, err: %v", target, string(output), err) - return volumeStatistics{}, errFmt - } - strOut := strings.TrimSpace(string(output)) - gotSizeBytes, errParse := strconv.ParseInt(strOut, 10, 64) - if errParse != nil { - return volumeStatistics{}, fmt.Errorf("failed to parse size %s into int", strOut) - } - - return volumeStatistics{ - totalBytes: gotSizeBytes, - }, nil - } - - var statfs unix.Statfs_t - errStatfs := unix.Statfs(target, &statfs) - if errStatfs != nil { - return volumeStatistics{}, errStatfs - } - - volStats := volumeStatistics{ - // darwin arm64 on statfs.Bsize returns uint32 so we explicitly cast to int64 - availableBytes: int64(statfs.Bavail) * int64(statfs.Bsize), //nolint: unconvert - totalBytes: int64(statfs.Blocks) * int64(statfs.Bsize), //nolint: unconvert - usedBytes: (int64(statfs.Blocks) - int64(statfs.Bfree)) * int64(statfs.Bsize), //nolint: unconvert - - availableInodes: int64(statfs.Ffree), - totalInodes: int64(statfs.Files), - usedInodes: int64(statfs.Files) - int64(statfs.Ffree), - } - - return volStats, nil -} - -func (m *mounter) IsBlockDevice(devicePath string) (bool, error) { - var stat unix.Stat_t - err := unix.Stat(devicePath, &stat) - if err != nil { - return false, err - } - - return (stat.Mode & unix.S_IFMT) == unix.S_IFBLK, nil -} diff --git a/driver/node.go b/driver/node.go index 37d65d64..270badbd 100644 --- a/driver/node.go +++ b/driver/node.go @@ -19,6 +19,8 @@ const ( diskPath = "/dev/disk/by-id" diskPrefix = "virtio-" + mkDirMode = 0750 + maxVolumesPerNode = 11 volumeModeBlock = "block" @@ -65,11 +67,11 @@ func (n *VultrNodeServer) NodeStageVolume(ctx context.Context, req *csi.NodeStag source := getDeviceByPath(volumeID) target := req.StagingTargetPath mountBlk := req.VolumeCapability.GetMount() - options := mountBlk.MountFlags + // options := mountBlk.MountFlags - fsTpe := "ext4" + fsType := "ext4" if mountBlk.FsType != "" { - fsTpe = mountBlk.FsType + fsType = mountBlk.FsType } n.Driver.log.WithFields(logrus.Fields{ @@ -77,10 +79,12 @@ func (n *VultrNodeServer) NodeStageVolume(ctx context.Context, req *csi.NodeStag "target": req.StagingTargetPath, "capacity": req.VolumeCapability, }).Infof("Node Stage Volume: creating directory target %s\n", target) + err := os.MkdirAll(target, mkDirMode) if err != nil { return nil, status.Error(codes.Internal, err.Error()) } + n.Driver.log.WithFields(logrus.Fields{ "volume": req.VolumeId, "target": req.StagingTargetPath, @@ -93,7 +97,7 @@ func (n *VultrNodeServer) NodeStageVolume(ctx context.Context, req *csi.NodeStag "capacity": req.VolumeCapability, }).Info("Node Stage Volume: attempting format and mount") - if err := n.Driver.mounter.FormatAndMount(source, target, fsTpe, options); err != nil { + if err := n.Driver.mounter.FormatAndMount(source, target, fsType, nil); err != nil { return nil, status.Error(codes.Internal, err.Error()) } From 41e80b127426b56e9ad743064f565a25a7069140 Mon Sep 17 00:00:00 2001 From: happytreees Date: Mon, 13 May 2024 14:18:20 -0400 Subject: [PATCH 2/3] fix workflows --- .github/workflows/notify-pr.yml | 9 +++--- driver/driver_test.go | 49 +-------------------------------- 2 files changed, 5 insertions(+), 53 deletions(-) diff --git a/.github/workflows/notify-pr.yml b/.github/workflows/notify-pr.yml index c985559a..d35c111d 100644 --- a/.github/workflows/notify-pr.yml +++ b/.github/workflows/notify-pr.yml @@ -7,10 +7,9 @@ jobs: runs-on: ubuntu-latest name: Pull Request Notification steps: - - run: | - echo "{\"text\":\"Vultr-CSI : PR https://github.com/${{ github.repository }}/pull/${{ github.event.number }} \"}" > mattermost.json - - uses: mattermost/action-mattermost-notify@2.0.0 + - uses: mattermost/action-mattermost-notify@master with: MATTERMOST_WEBHOOK_URL: ${{ secrets.MATTERMOST_WEBHOOK_URL }} - MATTERMOST_USERNAME: ${{ secrets.MATTERMOST_USERNAME}} - MATTERMOST_ICON: ${{ secrets.MATTERMOST_ICON }} + MATTERMOST_USERNAME: ${{ secrets.MATTERMOST_USERNAME }} + MATTERMOST_ICON_URL: ${{ secrets.MATTERMOST_ICON_URL }} + TEXT: "${{ github.repository }} : PR https://github.com/${{ github.repository }}/pull/${{ github.event.number }}" \ No newline at end of file diff --git a/driver/driver_test.go b/driver/driver_test.go index 7cfa7b7b..d96fa9cd 100644 --- a/driver/driver_test.go +++ b/driver/driver_test.go @@ -51,8 +51,7 @@ func TestDriverSuite(t *testing.T) { waitTimeout: defaultTimeout, - log: log, - vMounter: NewFakeMounter(log), + log: log, } go d.Run() @@ -66,49 +65,3 @@ func TestDriverSuite(t *testing.T) { t.Errorf("driver run failed: %s", err) } } - -type fakeMounter struct { - log *logrus.Entry - mounted map[string]string -} - -func NewFakeMounter(log *logrus.Entry) *fakeMounter { - return &fakeMounter{log: log} -} - -func (f *fakeMounter) Format(source, fs string) error { - return nil -} - -func (f *fakeMounter) IsFormatted(source string) (bool, error) { - return true, nil -} - -func (f *fakeMounter) Mount(source, target, fs string, opts ...string) error { - return nil -} - -func (f *fakeMounter) IsMounted(target string) (bool, error) { - return true, nil -} - -func (f *fakeMounter) UnMount(target string) error { - delete(f.mounted, target) - return nil -} - -func (f *fakeMounter) GetStatistics(volumePath string) (volumeStatistics, error) { - return volumeStatistics{ - availableBytes: 3 * giB, - totalBytes: 10 * giB, - usedBytes: 7 * giB, - - availableInodes: 3000, - totalInodes: 10000, - usedInodes: 7000, - }, nil -} - -func (f *fakeMounter) IsBlockDevice(volumePath string) (bool, error) { - return false, nil -} From 8753aa9da5f272e0fcc05b61e50f22f08cc4d692 Mon Sep 17 00:00:00 2001 From: happytreees Date: Mon, 13 May 2024 14:21:39 -0400 Subject: [PATCH 3/3] add options back --- driver/node.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/driver/node.go b/driver/node.go index 270badbd..6b4636aa 100644 --- a/driver/node.go +++ b/driver/node.go @@ -67,7 +67,7 @@ func (n *VultrNodeServer) NodeStageVolume(ctx context.Context, req *csi.NodeStag source := getDeviceByPath(volumeID) target := req.StagingTargetPath mountBlk := req.VolumeCapability.GetMount() - // options := mountBlk.MountFlags + options := mountBlk.MountFlags fsType := "ext4" if mountBlk.FsType != "" { @@ -97,7 +97,7 @@ func (n *VultrNodeServer) NodeStageVolume(ctx context.Context, req *csi.NodeStag "capacity": req.VolumeCapability, }).Info("Node Stage Volume: attempting format and mount") - if err := n.Driver.mounter.FormatAndMount(source, target, fsType, nil); err != nil { + if err := n.Driver.mounter.FormatAndMount(source, target, fsType, options); err != nil { return nil, status.Error(codes.Internal, err.Error()) }