diff --git a/internal/driver/sanity_test.go b/internal/driver/sanity_test.go index bec0c08d..fb626331 100644 --- a/internal/driver/sanity_test.go +++ b/internal/driver/sanity_test.go @@ -206,14 +206,6 @@ func (s *sanityMountService) PathExists(path string) (bool, error) { return true, nil } -func (s *sanityMountService) FormatDisk(_ string, _ string) error { - return nil -} - -func (s *sanityMountService) DetectDiskFormat(_ string) (string, error) { - return "ext4", nil -} - type sanityResizeService struct{} func (s *sanityResizeService) Resize(volumePath string) error { diff --git a/internal/mock/volume.go b/internal/mock/volume.go index cc44f4ee..5a3f9a40 100644 --- a/internal/mock/volume.go +++ b/internal/mock/volume.go @@ -77,11 +77,9 @@ func (s *VolumeService) Resize(ctx context.Context, volume *csi.Volume, size int } type VolumeMountService struct { - PublishFunc func(targetPath string, devicePath string, opts volumes.MountOpts) error - UnpublishFunc func(targetPath string) error - PathExistsFunc func(path string) (bool, error) - FormatDiskFunc func(disk string, fstype string) error - DetectDiskFormatFunc func(disk string) (string, error) + PublishFunc func(targetPath string, devicePath string, opts volumes.MountOpts) error + UnpublishFunc func(targetPath string) error + PathExistsFunc func(path string) (bool, error) } func (s *VolumeMountService) Publish(targetPath string, devicePath string, opts volumes.MountOpts) error { @@ -91,13 +89,6 @@ func (s *VolumeMountService) Publish(targetPath string, devicePath string, opts return s.PublishFunc(targetPath, devicePath, opts) } -func (s *VolumeMountService) PathExists(path string) (bool, error) { - if s.PathExistsFunc == nil { - panic("not implemented") - } - return s.PathExistsFunc(path) -} - func (s *VolumeMountService) Unpublish(targetPath string) error { if s.UnpublishFunc == nil { panic("not implemented") @@ -105,18 +96,11 @@ func (s *VolumeMountService) Unpublish(targetPath string) error { return s.UnpublishFunc(targetPath) } -func (s *VolumeMountService) FormatDisk(disk string, fstype string) error { - if s.FormatDiskFunc == nil { - panic("not implemented") - } - return s.FormatDiskFunc(disk, fstype) -} - -func (s *VolumeMountService) DetectDiskFormat(disk string) (string, error) { - if s.DetectDiskFormatFunc == nil { +func (s *VolumeMountService) PathExists(path string) (bool, error) { + if s.PathExistsFunc == nil { panic("not implemented") } - return s.DetectDiskFormatFunc(disk) + return s.PathExistsFunc(path) } type VolumeResizeService struct { diff --git a/internal/volumes/mount.go b/internal/volumes/mount.go index e8ea8b7e..8d4dce5e 100644 --- a/internal/volumes/mount.go +++ b/internal/volumes/mount.go @@ -4,7 +4,6 @@ import ( "fmt" "os" "path/filepath" - "regexp" "strings" "github.com/go-kit/log" @@ -29,8 +28,6 @@ type MountService interface { Publish(targetPath string, devicePath string, opts MountOpts) error Unpublish(targetPath string) error PathExists(path string) (bool, error) - FormatDisk(disk string, fstype string) error - DetectDiskFormat(disk string) (string, error) } // LinuxMountService mounts volumes on a Linux system. @@ -100,7 +97,7 @@ func (s *LinuxMountService) Publish(targetPath string, devicePath string, opts M mountOptions = append(mountOptions, opts.Additional...) if opts.EncryptionPassphrase != "" { - existingFSType, err := s.DetectDiskFormat(devicePath) + existingFSType, err := s.mounter.GetDiskFormat(devicePath) if err != nil { return fmt.Errorf("unable to detect existing disk format of %s: %w", devicePath, err) } @@ -122,26 +119,6 @@ func (s *LinuxMountService) Publish(targetPath string, devicePath string, opts M devicePath = luksDevicePath } - // Format disk if requested (/skip formatting for block devices) - if opts.FSType != "" { - existingFSType, err := s.DetectDiskFormat(devicePath) - if err != nil { - return fmt.Errorf("unable to detect existing disk format of %s: %w", devicePath, err) - } - - if existingFSType == "" { - if opts.Readonly { - return fmt.Errorf("cannot publish unformatted disk %s in read-only mode", devicePath) - } - - if err = s.FormatDisk(devicePath, opts.FSType); err != nil { - return err - } - } else if existingFSType != opts.FSType { - return fmt.Errorf("requested %s volume, but disk %s already is formatted with %s", opts.FSType, devicePath, existingFSType) - } - } - level.Info(s.logger).Log( "msg", "publishing volume", "target-path", targetPath, @@ -153,7 +130,11 @@ func (s *LinuxMountService) Publish(targetPath string, devicePath string, opts M "encrypted", opts.EncryptionPassphrase != "", ) - return s.mounter.Mount(devicePath, targetPath, opts.FSType, mountOptions) + if opts.BlockVolume { + return s.mounter.Mount(devicePath, targetPath, opts.FSType, mountOptions) + } + + return s.mounter.FormatAndMount(devicePath, targetPath, opts.FSType, mountOptions) } func (s *LinuxMountService) Unpublish(targetPath string) error { @@ -178,71 +159,5 @@ func (s *LinuxMountService) Unpublish(targetPath string) error { } func (s *LinuxMountService) PathExists(path string) (bool, error) { - level.Debug(s.logger).Log( - "msg", "checking path existence", - "path", path, - ) - _, err := os.Stat(path) - if err == nil { - return true, nil - } - if os.IsNotExist(err) { - return false, nil - } - return false, err -} - -func (s *LinuxMountService) FormatDisk(disk string, fstype string) error { - level.Info(s.logger).Log( - "msg", "formatting disk", - "disk", disk, - "fstype", fstype, - ) - switch fstype { - case "ext4": - _, _, err := command("mkfs.ext4", "-F", "-m0", disk) - return err - case "xfs": - _, _, err := command( - "mkfs.xfs", - "-i", "nrext64=0", // Compatibility with kernel that do not support nrext64 - disk) - return err - case "btrfs": - _, _, err := command("mkfs.btrfs", disk) - return err - default: - return fmt.Errorf("unsupported disk format %s", fstype) - } -} - -// see https://github.com/kubernetes/mount-utils/blob/master/mount_linux.go -func (s *LinuxMountService) DetectDiskFormat(disk string) (string, error) { - args := []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", disk} - output, exitCode, err := command("blkid", args...) - if exitCode == 2 { - return "", nil - } - if err != nil { - return "", err - } - - fstypeRegex := regexp.MustCompile(`TYPE=(.*)`) - pttypeRegex := regexp.MustCompile(`PTTYPE=(.*)`) - fstype := "" - pttype := "" - fstypeMatch := fstypeRegex.FindStringSubmatch(output) - if fstypeMatch != nil { - fstype = fstypeMatch[1] - } - pttypeMatch := pttypeRegex.FindStringSubmatch(output) - if pttypeMatch != nil { - pttype = pttypeMatch[1] - } - - if pttype != "" { - return "", fmt.Errorf("disk %s propably contains partitions", disk) - } - - return fstype, nil + return mount.PathExists(path) } diff --git a/test/integration/volumes_test.go b/test/integration/volumes_test.go index 3288877c..c51dddc4 100644 --- a/test/integration/volumes_test.go +++ b/test/integration/volumes_test.go @@ -11,6 +11,8 @@ import ( "testing" "github.com/go-kit/log" + "k8s.io/mount-utils" + "k8s.io/utils/exec" "github.com/hetznercloud/csi-driver/internal/volumes" ) @@ -23,56 +25,70 @@ func TestVolumePublishUnpublish(t *testing.T) { tests := []struct { name string mountOpts volumes.MountOpts - prepare func(svc volumes.MountService, cs *volumes.CryptSetup, device string) error + prepare func(mounter *mount.SafeFormatAndMount, cs *volumes.CryptSetup, device string) error expectedError error }{ // Block volume not formatted { - "plain", - volumes.MountOpts{}, - nil, - nil, + name: "plain", + mountOpts: volumes.MountOpts{}, + prepare: nil, + expectedError: nil, }, { - "plain-correct-formatted", - volumes.MountOpts{}, - func(svc volumes.MountService, cs *volumes.CryptSetup, device string) error { - return svc.FormatDisk(device, "ext4") + name: "plain-correct-formatted", + mountOpts: volumes.MountOpts{}, + prepare: func(mounter *mount.SafeFormatAndMount, cs *volumes.CryptSetup, device string) error { + tmppath, err := os.MkdirTemp(os.TempDir(), "csi-driver-prepare") + if err != nil { + return err + } + + defer os.RemoveAll(tmppath) + defer mounter.Unmount(tmppath) + return mounter.FormatAndMount(device, tmppath, "ext4", nil) }, - nil, + expectedError: nil, }, { - "plain-wrong-formatted", - volumes.MountOpts{}, - func(svc volumes.MountService, cs *volumes.CryptSetup, device string) error { - return svc.FormatDisk(device, "xfs") + name: "plain-wrong-formatted", + mountOpts: volumes.MountOpts{}, + prepare: func(mounter *mount.SafeFormatAndMount, cs *volumes.CryptSetup, device string) error { + tmppath, err := os.MkdirTemp(os.TempDir(), "csi-driver-prepare") + if err != nil { + return err + } + + defer os.RemoveAll(tmppath) + defer mounter.Unmount(tmppath) + return mounter.FormatAndMount(device, tmppath, "xfs", nil) }, - fmt.Errorf("requested ext4 volume, but disk /dev-fake-plain-wrong-formatted already is formatted with xfs"), + expectedError: mount.NewMountError(mount.FilesystemMismatch, ""), }, { - "block-volume", - volumes.MountOpts{BlockVolume: true}, - nil, - nil, + name: "block-volume", + mountOpts: volumes.MountOpts{BlockVolume: true}, + prepare: nil, + expectedError: nil, }, { - "encrypted", - volumes.MountOpts{EncryptionPassphrase: "passphrase"}, - nil, - nil, + name: "encrypted", + mountOpts: volumes.MountOpts{EncryptionPassphrase: "passphrase"}, + prepare: nil, + expectedError: nil, }, { - "encrypted-correct-formatted-1", - volumes.MountOpts{EncryptionPassphrase: "passphrase"}, - func(svc volumes.MountService, cs *volumes.CryptSetup, device string) error { + name: "encrypted-correct-formatted-1", + mountOpts: volumes.MountOpts{EncryptionPassphrase: "passphrase"}, + prepare: func(mounter *mount.SafeFormatAndMount, cs *volumes.CryptSetup, device string) error { return cs.Format(device, "passphrase") }, - nil, + expectedError: nil, }, { - "encrypted-correct-formatted-2", - volumes.MountOpts{EncryptionPassphrase: "passphrase"}, - func(svc volumes.MountService, cs *volumes.CryptSetup, device string) error { + name: "encrypted-correct-formatted-2", + mountOpts: volumes.MountOpts{EncryptionPassphrase: "passphrase"}, + prepare: func(mounter *mount.SafeFormatAndMount, cs *volumes.CryptSetup, device string) error { if err := cs.Format(device, "passphrase"); err != nil { return err } @@ -85,17 +101,31 @@ func TestVolumePublishUnpublish(t *testing.T) { luksDevicePath := volumes.GenerateLUKSDevicePath(luksDeviceName) - return svc.FormatDisk(luksDevicePath, "ext4") + tmppath, err := os.MkdirTemp(os.TempDir(), "csi-driver-prepare") + if err != nil { + return err + } + + defer os.RemoveAll(tmppath) + defer mounter.Unmount(tmppath) + return mounter.FormatAndMount(luksDevicePath, tmppath, "ext4", nil) }, - nil, + expectedError: nil, }, { - "encrypted-wrong-formatted-1", - volumes.MountOpts{EncryptionPassphrase: "passphrase"}, - func(svc volumes.MountService, cs *volumes.CryptSetup, device string) error { - return svc.FormatDisk(device, "ext4") + name: "encrypted-wrong-formatted-1", + mountOpts: volumes.MountOpts{EncryptionPassphrase: "passphrase"}, + prepare: func(mounter *mount.SafeFormatAndMount, cs *volumes.CryptSetup, device string) error { + tmppath, err := os.MkdirTemp(os.TempDir(), "csi-driver-prepare") + if err != nil { + return err + } + + defer os.RemoveAll(tmppath) + defer mounter.Unmount(tmppath) + return mounter.FormatAndMount(device, tmppath, "ext4", nil) }, - fmt.Errorf("requested encrypted volume, but disk /dev-fake-encrypted-wrong-formatted-1 already is formatted with ext4"), + expectedError: fmt.Errorf("requested encrypted volume, but disk /dev-fake-encrypted-wrong-formatted-1 already is formatted with ext4"), }, } @@ -103,6 +133,10 @@ func TestVolumePublishUnpublish(t *testing.T) { t.Run(test.name, func(t *testing.T) { logger := log.NewLogfmtLogger(NewTestingWriter(t)) mountService := volumes.NewLinuxMountService(logger) + mounter := &mount.SafeFormatAndMount{ + Interface: mount.New(""), + Exec: exec.New(), + } cryptSetup := volumes.NewCryptSetup(logger) device, err := createFakeDevice("fake-"+test.name, 512) if err != nil { @@ -110,7 +144,7 @@ func TestVolumePublishUnpublish(t *testing.T) { } if test.prepare != nil { - if err := test.prepare(mountService, cryptSetup, device); err != nil { + if err := test.prepare(mounter, cryptSetup, device); err != nil { t.Fatal(err) } } @@ -123,29 +157,39 @@ func TestVolumePublishUnpublish(t *testing.T) { // Required as FS volumes require target dir, but block volumes require // target file targetPath = path.Join(targetPath, "target-path") - + defer func() { + err := mountService.Unpublish(targetPath) + if err != nil { + t.Fatal(err) + } else { + t.Logf("Unpublished targetPath %s", targetPath) + } + }() publishErr := mountService.Publish(targetPath, device, test.mountOpts) + if test.expectedError != nil { - // We expected an error if publishErr == nil { t.Fatalf("expected error %q but got no error", test.expectedError.Error()) + } + + if got, ok := publishErr.(mount.MountError); ok { + if expected, ok := test.expectedError.(mount.MountError); ok { + if got.Type != expected.Type { + t.Fatalf("Expected Mount Error %s, but got %s", expected.Type, got.Type) + } + return + } else { + t.Fatalf("Test returned MountError %s, but expected error is not of MountError", got.Type) + } } else if test.expectedError.Error() != publishErr.Error() { t.Fatal(fmt.Errorf("expected error %q but got %q", test.expectedError.Error(), publishErr.Error())) + return } - - // Makes no sense to continue verification if we got the error that we expected - _ = mountService.Unpublish(targetPath) - return } + if err != nil { t.Fatal(publishErr) } - defer func() { - err := mountService.Unpublish(targetPath) - if err != nil { - t.Fatal(err) - } - }() // Verify target exists and is of expected type fileInfo, err := os.Stat(targetPath) @@ -165,7 +209,7 @@ func TestVolumePublishUnpublish(t *testing.T) { if test.mountOpts.EncryptionPassphrase == "" { // Verify device has expected fs type // Encrypted volumes always have "crypto_LUKS" - fsType, err := mountService.DetectDiskFormat(device) + fsType, err := mounter.GetDiskFormat(device) if err != nil { t.Fatal(err) } @@ -284,54 +328,70 @@ func TestDetectDiskFormat(t *testing.T) { tests := []*struct { name string - prepare func(*volumes.LinuxMountService, string) error + prepare func(*mount.SafeFormatAndMount, string) error expectedFormat string }{ { - "empty", - nil, - "", + name: "empty", + prepare: nil, + expectedFormat: "", }, { - "ext4", - func(svc *volumes.LinuxMountService, disk string) error { - return svc.FormatDisk(disk, "ext4") + name: "ext4", + prepare: func(mounter *mount.SafeFormatAndMount, device string) error { + tmppath, err := os.MkdirTemp(os.TempDir(), "csi-driver-prepare") + if err != nil { + return err + } + + defer os.RemoveAll(tmppath) + defer mounter.Unmount(tmppath) + return mounter.FormatAndMount(device, tmppath, "ext4", nil) }, - "ext4", + expectedFormat: "ext4", }, { - "xfs", - func(svc *volumes.LinuxMountService, disk string) error { - return svc.FormatDisk(disk, "xfs") + name: "xfs", + prepare: func(mounter *mount.SafeFormatAndMount, device string) error { + tmppath, err := os.MkdirTemp(os.TempDir(), "csi-driver-prepare") + if err != nil { + return err + } + + defer os.RemoveAll(tmppath) + defer mounter.Unmount(tmppath) + return mounter.FormatAndMount(device, tmppath, "xfs", nil) }, - "xfs", + expectedFormat: "xfs", }, { - "crypto_LUKS", - func(svc *volumes.LinuxMountService, disk string) error { + name: "crypto_LUKS", + prepare: func(mounter *mount.SafeFormatAndMount, device string) error { logger := log.NewLogfmtLogger(NewTestingWriter(t)) cryptSetup := volumes.NewCryptSetup(logger) - err := cryptSetup.Format(disk, "passphrase") + err := cryptSetup.Format(device, "passphrase") return err }, - "crypto_LUKS", + expectedFormat: "crypto_LUKS", }, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { - logger := log.NewLogfmtLogger(NewTestingWriter(t)) - mountService := volumes.NewLinuxMountService(logger) + mounter := &mount.SafeFormatAndMount{ + Interface: mount.New(""), + Exec: exec.New(), + } disk, err := createFakeDevice(test.name, 512) if err != nil { t.Fatal(err) } if test.prepare != nil { - if err := test.prepare(mountService, disk); err != nil { + if err := test.prepare(mounter, disk); err != nil { t.Fatal(err) } } - format, err := mountService.DetectDiskFormat(disk) + format, err := mounter.GetDiskFormat(disk) if err != nil { t.Fatal(err) }