From 6547dc6a4dfaf2400613178bac9af98e0cce0ae1 Mon Sep 17 00:00:00 2001 From: Andreas Fritzler Date: Thu, 23 Mar 2023 16:53:48 +0100 Subject: [PATCH] Refactor Node driver tests (#237) --- .github/workflows/clean.yml | 1 + Makefile | 7 +- hack/check.sh | 3 +- ...update-gomock => fix-mountwrapper-mock.sh} | 2 +- pkg/driver/constants.go | 4 +- pkg/driver/controller.go | 4 +- pkg/driver/driver.go | 15 +-- pkg/driver/node.go | 16 +-- pkg/driver/node_test.go | 124 +++++++++--------- .../mount/mock_mountutils_unix.go} | 8 +- .../mount/mountutils_unix.go} | 20 +-- .../os/mock_osutils_unix.go} | 6 +- .../osutils.go => utils/os/osutils_unix.go} | 7 +- pkg/{util => utils}/util.go | 2 +- 14 files changed, 109 insertions(+), 110 deletions(-) rename hack/{update-gomock => fix-mountwrapper-mock.sh} (90%) rename pkg/{driver/mocks/mountutils.go => utils/mount/mock_mountutils_unix.go} (98%) rename pkg/{driver/mountutils/mountutils.go => utils/mount/mountutils_unix.go} (74%) rename pkg/{driver/mocks/osutils.go => utils/os/mock_osutils_unix.go} (96%) rename pkg/{driver/osutils/osutils.go => utils/os/osutils_unix.go} (89%) rename pkg/{util => utils}/util.go (99%) diff --git a/.github/workflows/clean.yml b/.github/workflows/clean.yml index fecf04f..a8af939 100644 --- a/.github/workflows/clean.yml +++ b/.github/workflows/clean.yml @@ -17,4 +17,5 @@ jobs: with: go-version: '1.20' - run: go mod tidy + - run: make generate-mocks - run: hack/check.sh diff --git a/Makefile b/Makefile index 937c8a6..0ded29b 100755 --- a/Makefile +++ b/Makefile @@ -132,7 +132,7 @@ fmt: goimports ## Run goimports against code. vet: ## Run go vet against code. go vet ./... -test: generate-mocks update-mock fmt vet envtest ## Run tests. +test: generate-mocks fmt vet envtest ## Run tests. KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) -p path)" go test ./... -coverprofile cover.out .PHONY: add-license @@ -164,12 +164,9 @@ $(GOIMPORTS): $(LOCALBIN) .PHONY: generate-mocks generate-mocks: mockgen ## Generate code (mocks etc.). MOCKGEN=$(MOCKGEN) go generate ./... + ./hack/fix-mountwrapper-mock.sh .PHONY: mockgen mockgen: $(MOCKGEN) $(MOCKGEN): $(LOCALBIN) test -s $(LOCALBIN)/mockgen || GOBIN=$(LOCALBIN) go install github.com/golang/mock/mockgen@$(MOCKGEN_VERSION) - -.PHONY: update-mock -update-mock: ## Run update-gomock script to update mountutils mock file - ./hack/update-gomock \ No newline at end of file diff --git a/hack/check.sh b/hack/check.sh index ce2f8be..7ab439d 100755 --- a/hack/check.sh +++ b/hack/check.sh @@ -2,7 +2,8 @@ if ! git diff --name-only --exit-code; then echo "#################################################################################" - echo "Project is dirty. Make sure you run 'go mod tidy' before committing your changes." + echo "Project is dirty. Make sure you run 'make generate-mocks && go mod tidy'" + echo "before committing your changes." echo "#################################################################################" exit 1 fi diff --git a/hack/update-gomock b/hack/fix-mountwrapper-mock.sh similarity index 90% rename from hack/update-gomock rename to hack/fix-mountwrapper-mock.sh index 0095465..b4be308 100755 --- a/hack/update-gomock +++ b/hack/fix-mountwrapper-mock.sh @@ -4,4 +4,4 @@ set -euo pipefail # Fixes "MountWrapper Type cannot implement 'MountWrapper' as it has a non-exported method and is defined in a different package" # See https://github.com/kubernetes/mount-utils/commit/a20fcfb15a701977d086330b47b7efad51eb608e for context. -sed -i '/type MockMountWrapper struct {/a \\mount.Interface' pkg/driver/mocks/mountutils.go +sed -i '/type MockMountWrapper struct {/a \\mount.Interface' pkg/utils/mount/mock_mountutils_unix.go diff --git a/pkg/driver/constants.go b/pkg/driver/constants.go index 7bc6c1a..1e33f5e 100644 --- a/pkg/driver/constants.go +++ b/pkg/driver/constants.go @@ -15,13 +15,13 @@ package driver import ( - "github.com/onmetal/onmetal-csi-driver/pkg/util" + "github.com/onmetal/onmetal-csi-driver/pkg/utils" "sigs.k8s.io/controller-runtime/pkg/client" ) const ( // DefaultVolumeSize represents the default volume size. - DefaultVolumeSize int64 = 10 * util.GiB + DefaultVolumeSize int64 = 10 * utils.GiB // ParameterType is the name of the type parameter ParameterType = "type" diff --git a/pkg/driver/controller.go b/pkg/driver/controller.go index 7e2398e..9baa0cc 100755 --- a/pkg/driver/controller.go +++ b/pkg/driver/controller.go @@ -23,7 +23,7 @@ import ( computev1alpha1 "github.com/onmetal/onmetal-api/api/compute/v1alpha1" corev1alpha1 "github.com/onmetal/onmetal-api/api/core/v1alpha1" storagev1alpha1 "github.com/onmetal/onmetal-api/api/storage/v1alpha1" - "github.com/onmetal/onmetal-csi-driver/pkg/util" + "github.com/onmetal/onmetal-csi-driver/pkg/utils" "golang.org/x/exp/slices" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" @@ -337,7 +337,7 @@ func getVolSizeBytes(req *csi.CreateVolumeRequest) (int64, error) { if capRange == nil { volSizeBytes = DefaultVolumeSize } else { - volSizeBytes = util.RoundUpBytes(capRange.GetRequiredBytes()) + volSizeBytes = utils.RoundUpBytes(capRange.GetRequiredBytes()) maxVolSize := capRange.GetLimitBytes() if maxVolSize > 0 && maxVolSize < volSizeBytes { return 0, status.Error(codes.InvalidArgument, "after round-up, volume size exceeds the limit specified") diff --git a/pkg/driver/driver.go b/pkg/driver/driver.go index 909b68b..8a87fcf 100755 --- a/pkg/driver/driver.go +++ b/pkg/driver/driver.go @@ -22,16 +22,15 @@ import ( "github.com/container-storage-interface/spec/lib/go/csi" "github.com/dell/gocsi" "github.com/onmetal/onmetal-csi-driver/cmd/options" - "github.com/onmetal/onmetal-csi-driver/pkg/driver/mountutils" - "github.com/onmetal/onmetal-csi-driver/pkg/driver/osutils" - + "github.com/onmetal/onmetal-csi-driver/pkg/utils/mount" + "github.com/onmetal/onmetal-csi-driver/pkg/utils/os" "k8s.io/klog/v2" "sigs.k8s.io/controller-runtime/pkg/client" ) type driver struct { - mount mountutils.MountWrapper - os osutils.OSWrapper + mounter mount.MountWrapper + os os.OSWrapper targetClient client.Client onmetalClient client.Client config *options.Config @@ -48,7 +47,7 @@ type Driver interface { func NewDriver(config *options.Config, targetClient, onMetalClient client.Client) Driver { klog.InfoS("Driver Information", "Driver", CSIDriverName, "Version", "dev") - nodeMounter, err := mountutils.NewNodeMounter() + nodeMounter, err := mount.NewNodeMounter() if err != nil { panic(fmt.Errorf("error creating node mounter: %w", err)) } @@ -56,8 +55,8 @@ func NewDriver(config *options.Config, targetClient, onMetalClient client.Client config: config, targetClient: targetClient, onmetalClient: onMetalClient, - mount: nodeMounter, - os: osutils.OsOps{}, + mounter: nodeMounter, + os: os.OsOps{}, } } diff --git a/pkg/driver/node.go b/pkg/driver/node.go index b3af5e4..fddb7ed 100755 --- a/pkg/driver/node.go +++ b/pkg/driver/node.go @@ -40,7 +40,7 @@ func (d *driver) NodeStageVolume(_ context.Context, req *csi.NodeStageVolumeRequ targetPath := req.GetStagingTargetPath() klog.InfoS("Validate mount point", "MountPoint", targetPath) - notMnt, err := d.mount.IsLikelyNotMountPoint(targetPath) + notMnt, err := d.mounter.IsLikelyNotMountPoint(targetPath) if err != nil && !d.os.IsNotExist(err) { return nil, status.Errorf(codes.Internal, "Failed to verify mount point %s: %v", devicePath, err) } @@ -62,7 +62,7 @@ func (d *driver) NodeStageVolume(_ context.Context, req *csi.NodeStageVolumeRequ } options = append(options, mountOptions...) klog.InfoS("Format and mount the volume") - if err = d.mount.FormatAndMount(devicePath, targetPath, fstype, options); err != nil { + if err = d.mounter.FormatAndMount(devicePath, targetPath, fstype, options); err != nil { return nil, status.Errorf(codes.Internal, "Failed to mount volume %s [%s] to %s: %v", devicePath, fstype, targetPath, err) } klog.InfoS("Staged volume on node", "Volume", req.GetVolumeId()) @@ -110,7 +110,7 @@ func (d *driver) NodePublishVolume(_ context.Context, req *csi.NodePublishVolume fstype = "ext4" } - notMnt, err := d.mount.IsLikelyNotMountPoint(targetMountPath) + notMnt, err := d.mounter.IsLikelyNotMountPoint(targetMountPath) if err != nil && !d.os.IsNotExist(err) { return nil, status.Errorf(codes.Internal, "Mount directory %s does not exist: %v", targetMountPath, err) } @@ -123,7 +123,7 @@ func (d *driver) NodePublishVolume(_ context.Context, req *csi.NodePublishVolume return nil, fmt.Errorf("failed to create mount directory %s: %w", targetMountPath, err) } } - if err := d.mount.Mount(stagePath, targetMountPath, fstype, mountOptions); err != nil { + if err := d.mounter.Mount(stagePath, targetMountPath, fstype, mountOptions); err != nil { return nil, status.Errorf(codes.Internal, "Could not mount %q at %q: %v", stagePath, targetMountPath, err) } } @@ -150,7 +150,7 @@ func (d *driver) NodeUnstageVolume(_ context.Context, req *csi.NodeUnstageVolume if devicePath == "" { return nil, status.Error(codes.Internal, "Device path not set") } - if err := d.mount.Unmount(stagePath); err != nil { + if err := d.mounter.Unmount(stagePath); err != nil { return nil, status.Errorf(codes.Internal, "Failed to unmount stating target path %s: %v", stagePath, err) } klog.InfoS("Remove stagingTargetPath directory after unmount") @@ -178,13 +178,13 @@ func (d *driver) NodeUnpublishVolume(_ context.Context, req *csi.NodeUnpublishVo return nil, status.Errorf(codes.Internal, "Unable to stat %s: %v", targetPath, err) } - notMnt, err := d.mount.IsLikelyNotMountPoint(targetPath) + notMnt, err := d.mounter.IsLikelyNotMountPoint(targetPath) if err != nil && !d.os.IsNotExist(err) { return nil, status.Errorf(codes.Internal, "Mount point %s does not exist: %v", targetPath, err) } if !notMnt { - err = d.mount.Unmount(targetPath) + err = d.mounter.Unmount(targetPath) if err != nil { return nil, status.Errorf(codes.Internal, "Failed not unmount %s: %v", targetPath, err) } @@ -247,7 +247,7 @@ func (d *driver) NodeGetCapabilities(_ context.Context, _ *csi.NodeGetCapabiliti } func (d *driver) GetMountDeviceName(mountPath string) (device string, err error) { - mountPoints, err := d.mount.List() + mountPoints, err := d.mounter.List() if err != nil { return device, err } diff --git a/pkg/driver/node_test.go b/pkg/driver/node_test.go index aad26be..d258efa 100644 --- a/pkg/driver/node_test.go +++ b/pkg/driver/node_test.go @@ -19,14 +19,15 @@ import ( "os" "github.com/container-storage-interface/spec/lib/go/csi" - gomock "github.com/golang/mock/gomock" + "github.com/golang/mock/gomock" testutils "github.com/onmetal/onmetal-api/utils/testing" - mocks "github.com/onmetal/onmetal-csi-driver/pkg/driver/mocks" + mountutils "github.com/onmetal/onmetal-csi-driver/pkg/utils/mount" + osutils "github.com/onmetal/onmetal-csi-driver/pkg/utils/os" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" - mount "k8s.io/mount-utils" + "k8s.io/mount-utils" ) var _ = Describe("Node", func() { @@ -34,21 +35,24 @@ var _ = Describe("Node", func() { _, drv := SetupTest(ctx) var ( - ctrl *gomock.Controller - mockMountUtil *mocks.MockMountWrapper - mockOSUtil *mocks.MockOSWrapper - volumeId string - devicePath string - targetPath string - fstype string + ctrl *gomock.Controller + mockMounter *mountutils.MockMountWrapper + mockOS *osutils.MockOSWrapper + volumeId string + devicePath string + targetPath string + fstype string ) BeforeEach(func() { ctrl = gomock.NewController(GinkgoT()) - mockMountUtil = mocks.NewMockMountWrapper(ctrl) - mockOSUtil = mocks.NewMockOSWrapper(ctrl) - drv.mount = mockMountUtil - drv.os = mockOSUtil + mockMounter = mountutils.NewMockMountWrapper(ctrl) + mockOS = osutils.NewMockOSWrapper(ctrl) + + // inject mock mounter and os wrapper + drv.mounter = mockMounter + drv.os = mockOS + volumeId = "test-volume-id" devicePath = "/dev/sdb" targetPath = "/target/path" @@ -86,14 +90,14 @@ var _ = Describe("Node", func() { }) It("should fail if the volume is already mounted", func() { - mockMountUtil.EXPECT().IsLikelyNotMountPoint(targetPath).Return(false, nil) + mockMounter.EXPECT().IsLikelyNotMountPoint(targetPath).Return(false, nil) _, err := drv.NodeStageVolume(ctx, req) Expect(err).NotTo(BeNil()) }) It("should fail if the mount point validation fails", func() { - mockMountUtil.EXPECT().IsLikelyNotMountPoint(targetPath).Return(false, errors.New("failed to validate mount point")) - mockOSUtil.EXPECT().IsNotExist(gomock.Any()).Return(false) + mockMounter.EXPECT().IsLikelyNotMountPoint(targetPath).Return(false, errors.New("failed to validate mount point")) + mockOS.EXPECT().IsNotExist(gomock.Any()).Return(false) _, err := drv.NodeStageVolume(ctx, req) Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("Failed to verify mount point")) @@ -103,8 +107,8 @@ var _ = Describe("Node", func() { }) It("should fail if the target directory creation fails", func() { - mockMountUtil.EXPECT().IsLikelyNotMountPoint(targetPath).Return(true, nil) - mockOSUtil.EXPECT().MkdirAll(targetPath, os.FileMode(0750)).Return(errors.New("failed to create target directory")) + mockMounter.EXPECT().IsLikelyNotMountPoint(targetPath).Return(true, nil) + mockOS.EXPECT().MkdirAll(targetPath, os.FileMode(0750)).Return(errors.New("failed to create target directory")) _, err := drv.NodeStageVolume(ctx, req) Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("Failed to create target directory")) @@ -115,9 +119,9 @@ var _ = Describe("Node", func() { }) It("should fail if the mount operation fails", func() { - mockMountUtil.EXPECT().IsLikelyNotMountPoint(targetPath).Return(true, nil) - mockOSUtil.EXPECT().MkdirAll(targetPath, os.FileMode(0750)).Return(nil) - mockMountUtil.EXPECT().FormatAndMount(devicePath, targetPath, fstype, mountOptions).Return(errors.New("failed to mount volume")) + mockMounter.EXPECT().IsLikelyNotMountPoint(targetPath).Return(true, nil) + mockOS.EXPECT().MkdirAll(targetPath, os.FileMode(0750)).Return(nil) + mockMounter.EXPECT().FormatAndMount(devicePath, targetPath, fstype, mountOptions).Return(errors.New("failed to mount volume")) _, err := drv.NodeStageVolume(ctx, req) Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("Failed to mount volume")) @@ -127,9 +131,9 @@ var _ = Describe("Node", func() { }) It("should stage the volume", func() { - mockMountUtil.EXPECT().IsLikelyNotMountPoint(targetPath).Return(true, nil) - mockOSUtil.EXPECT().MkdirAll(targetPath, os.FileMode(0750)).Return(nil) - mockMountUtil.EXPECT().FormatAndMount(devicePath, targetPath, fstype, mountOptions).Return(nil) + mockMounter.EXPECT().IsLikelyNotMountPoint(targetPath).Return(true, nil) + mockOS.EXPECT().MkdirAll(targetPath, os.FileMode(0750)).Return(nil) + mockMounter.EXPECT().FormatAndMount(devicePath, targetPath, fstype, mountOptions).Return(nil) _, err := drv.NodeStageVolume(ctx, req) Expect(err).NotTo(HaveOccurred()) }) @@ -209,8 +213,8 @@ var _ = Describe("Node", func() { }) It("should fail if the mount point validation fails", func() { - mockMountUtil.EXPECT().IsLikelyNotMountPoint(targetPath).Return(false, errors.New("failed to validate mount point")) - mockOSUtil.EXPECT().IsNotExist(gomock.Any()).Return(false) + mockMounter.EXPECT().IsLikelyNotMountPoint(targetPath).Return(false, errors.New("failed to validate mount point")) + mockOS.EXPECT().IsNotExist(gomock.Any()).Return(false) _, err := drv.NodePublishVolume(ctx, req) Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("does not exist")) @@ -220,17 +224,17 @@ var _ = Describe("Node", func() { }) It("should publish volume on node if mount point already exist", func() { - mockMountUtil.EXPECT().IsLikelyNotMountPoint(targetPath).Return(false, nil) + mockMounter.EXPECT().IsLikelyNotMountPoint(targetPath).Return(false, nil) _, err := drv.NodePublishVolume(ctx, req) Expect(err).NotTo(HaveOccurred()) }) It("should publish volume on node if mount directory does not exist", func() { - mockMountUtil.EXPECT().IsLikelyNotMountPoint(targetPath).Return(true, errors.New("file does not exist")) - mockOSUtil.EXPECT().IsNotExist(errors.New("file does not exist")).Return(true) - mockOSUtil.EXPECT().IsNotExist(errors.New("file does not exist")).Return(true) - mockOSUtil.EXPECT().MkdirAll(targetPath, os.FileMode(os.FileMode(0750))).Return(nil) - mockMountUtil.EXPECT().Mount(stagingTargetPath, targetPath, fstype, mountOptions).Return(nil) + mockMounter.EXPECT().IsLikelyNotMountPoint(targetPath).Return(true, errors.New("file does not exist")) + mockOS.EXPECT().IsNotExist(errors.New("file does not exist")).Return(true) + mockOS.EXPECT().IsNotExist(errors.New("file does not exist")).Return(true) + mockOS.EXPECT().MkdirAll(targetPath, os.FileMode(os.FileMode(0750))).Return(nil) + mockMounter.EXPECT().Mount(stagingTargetPath, targetPath, fstype, mountOptions).Return(nil) _, err := drv.NodePublishVolume(ctx, req) Expect(err).NotTo(HaveOccurred()) }) @@ -261,7 +265,7 @@ var _ = Describe("Node", func() { }) It("should fail if the list mounted filesystems operation fails", func() { - mockMountUtil.EXPECT().List().Return(nil, errors.New("error")) + mockMounter.EXPECT().List().Return(nil, errors.New("error")) _, err := drv.NodeUnstageVolume(ctx, req) Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("Failed to get device path")) @@ -271,8 +275,8 @@ var _ = Describe("Node", func() { }) It("should fail if the unmount operation fails", func() { - mockMountUtil.EXPECT().List().Return([]mount.MountPoint{{Device: "/dev/sda1", Path: stagingTargetPath}}, nil) - mockMountUtil.EXPECT().Unmount(stagingTargetPath).Return(errors.New("error")) + mockMounter.EXPECT().List().Return([]mount.MountPoint{{Device: "/dev/sda1", Path: stagingTargetPath}}, nil) + mockMounter.EXPECT().Unmount(stagingTargetPath).Return(errors.New("error")) _, err := drv.NodeUnstageVolume(ctx, req) Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("Failed to unmount")) @@ -282,9 +286,9 @@ var _ = Describe("Node", func() { }) It("should fail if the remove mount directory operation fails", func() { - mockMountUtil.EXPECT().List().Return([]mount.MountPoint{{Device: "/dev/sda1", Path: stagingTargetPath}}, nil) - mockMountUtil.EXPECT().Unmount(stagingTargetPath).Return(nil) - mockOSUtil.EXPECT().RemoveAll(stagingTargetPath).Return(errors.New("error")) + mockMounter.EXPECT().List().Return([]mount.MountPoint{{Device: "/dev/sda1", Path: stagingTargetPath}}, nil) + mockMounter.EXPECT().Unmount(stagingTargetPath).Return(nil) + mockOS.EXPECT().RemoveAll(stagingTargetPath).Return(errors.New("error")) _, err := drv.NodeUnstageVolume(ctx, req) Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("Failed to remove mount directory")) @@ -294,9 +298,9 @@ var _ = Describe("Node", func() { }) It("should unstage the volume", func() { - mockMountUtil.EXPECT().List().Return([]mount.MountPoint{{Device: "/dev/sda1", Path: stagingTargetPath}}, nil) - mockMountUtil.EXPECT().Unmount(stagingTargetPath).Return(nil) - mockOSUtil.EXPECT().RemoveAll(stagingTargetPath).Return(nil) + mockMounter.EXPECT().List().Return([]mount.MountPoint{{Device: "/dev/sda1", Path: stagingTargetPath}}, nil) + mockMounter.EXPECT().Unmount(stagingTargetPath).Return(nil) + mockOS.EXPECT().RemoveAll(stagingTargetPath).Return(nil) _, err := drv.NodeUnstageVolume(ctx, req) Expect(err).NotTo(HaveOccurred()) }) @@ -335,7 +339,7 @@ var _ = Describe("Node", func() { }) It("should fail if the stat operation fails", func() { - mockOSUtil.EXPECT().Stat(targetPath).Return(nil, errors.New("error")) + mockOS.EXPECT().Stat(targetPath).Return(nil, errors.New("error")) _, err := drv.NodeUnpublishVolume(ctx, req) Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("Unable to stat")) @@ -345,9 +349,9 @@ var _ = Describe("Node", func() { }) It("should fail if the mount point validation fails", func() { - mockOSUtil.EXPECT().Stat(targetPath).Return(nil, nil) - mockMountUtil.EXPECT().IsLikelyNotMountPoint(targetPath).Return(false, errors.New("failed to validate mount point")) - mockOSUtil.EXPECT().IsNotExist(gomock.Any()).Return(false) + mockOS.EXPECT().Stat(targetPath).Return(nil, nil) + mockMounter.EXPECT().IsLikelyNotMountPoint(targetPath).Return(false, errors.New("failed to validate mount point")) + mockOS.EXPECT().IsNotExist(gomock.Any()).Return(false) _, err := drv.NodeUnpublishVolume(ctx, req) Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("does not exist")) @@ -357,10 +361,10 @@ var _ = Describe("Node", func() { }) It("should fail if the unmount operation fails", func() { - mockOSUtil.EXPECT().Stat(targetPath).Return(nil, nil) - mockMountUtil.EXPECT().IsLikelyNotMountPoint(targetPath).Return(false, errors.New("error")) - mockOSUtil.EXPECT().IsNotExist(errors.New("error")).Return(true) - mockMountUtil.EXPECT().Unmount(targetPath).Return(errors.New("error")) + mockOS.EXPECT().Stat(targetPath).Return(nil, nil) + mockMounter.EXPECT().IsLikelyNotMountPoint(targetPath).Return(false, errors.New("error")) + mockOS.EXPECT().IsNotExist(errors.New("error")).Return(true) + mockMounter.EXPECT().Unmount(targetPath).Return(errors.New("error")) _, err := drv.NodeUnpublishVolume(ctx, req) Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("Failed not unmount")) @@ -370,11 +374,11 @@ var _ = Describe("Node", func() { }) It("should fail if the remove mount directory operation fails", func() { - mockOSUtil.EXPECT().Stat(targetPath).Return(nil, nil) - mockMountUtil.EXPECT().IsLikelyNotMountPoint(targetPath).Return(false, errors.New("error")) - mockOSUtil.EXPECT().IsNotExist(errors.New("error")).Return(true) - mockMountUtil.EXPECT().Unmount(targetPath).Return(nil) - mockOSUtil.EXPECT().RemoveAll(targetPath).Return(errors.New("error")) + mockOS.EXPECT().Stat(targetPath).Return(nil, nil) + mockMounter.EXPECT().IsLikelyNotMountPoint(targetPath).Return(false, errors.New("error")) + mockOS.EXPECT().IsNotExist(errors.New("error")).Return(true) + mockMounter.EXPECT().Unmount(targetPath).Return(nil) + mockOS.EXPECT().RemoveAll(targetPath).Return(errors.New("error")) _, err := drv.NodeUnpublishVolume(ctx, req) Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("Failed to remove mount directory")) @@ -384,11 +388,11 @@ var _ = Describe("Node", func() { }) It("should unpublish volume from node", func() { - mockOSUtil.EXPECT().Stat(targetPath).Return(nil, nil) - mockMountUtil.EXPECT().IsLikelyNotMountPoint(targetPath).Return(false, errors.New("error")) - mockOSUtil.EXPECT().IsNotExist(errors.New("error")).Return(true) - mockMountUtil.EXPECT().Unmount(targetPath).Return(nil) - mockOSUtil.EXPECT().RemoveAll(targetPath).Return(nil) + mockOS.EXPECT().Stat(targetPath).Return(nil, nil) + mockMounter.EXPECT().IsLikelyNotMountPoint(targetPath).Return(false, errors.New("error")) + mockOS.EXPECT().IsNotExist(errors.New("error")).Return(true) + mockMounter.EXPECT().Unmount(targetPath).Return(nil) + mockOS.EXPECT().RemoveAll(targetPath).Return(nil) _, err := drv.NodeUnpublishVolume(ctx, req) Expect(err).NotTo(HaveOccurred()) }) diff --git a/pkg/driver/mocks/mountutils.go b/pkg/utils/mount/mock_mountutils_unix.go similarity index 98% rename from pkg/driver/mocks/mountutils.go rename to pkg/utils/mount/mock_mountutils_unix.go index 95c3d70..28a211d 100644 --- a/pkg/driver/mocks/mountutils.go +++ b/pkg/utils/mount/mock_mountutils_unix.go @@ -1,8 +1,8 @@ // Code generated by MockGen. DO NOT EDIT. -// Source: ../../../pkg/driver/mountutils/mountutils.go +// Source: mountutils_unix.go -// Package mocks is a generated GoMock package. -package mocks +// Package mount is a generated GoMock package. +package mount import ( reflect "reflect" @@ -13,7 +13,7 @@ import ( // MockMountWrapper is a mock of MountWrapper interface. type MockMountWrapper struct { - mount.Interface +mount.Interface ctrl *gomock.Controller recorder *MockMountWrapperMockRecorder } diff --git a/pkg/driver/mountutils/mountutils.go b/pkg/utils/mount/mountutils_unix.go similarity index 74% rename from pkg/driver/mountutils/mountutils.go rename to pkg/utils/mount/mountutils_unix.go index 487eb26..bb9e838 100644 --- a/pkg/driver/mountutils/mountutils.go +++ b/pkg/utils/mount/mountutils_unix.go @@ -1,3 +1,6 @@ +//go:build linux || darwin +// +build linux darwin + // Copyright 2023 OnMetal authors // // Licensed under the Apache License, Version 2.0 (the "License"); @@ -12,14 +15,14 @@ // See the License for the specific language governing permissions and // limitations under the License. -package mountutils +package mount import ( mount "k8s.io/mount-utils" utilexec "k8s.io/utils/exec" ) -//go:generate $MOCKGEN -package mocks -destination=../../../pkg/driver/mocks/mountutils.go -source ../../../pkg/driver/mountutils/mountutils.go +//go:generate $MOCKGEN -package mount -destination=mock_mountutils_unix.go -source mountutils_unix.go // MountWrapper is the interface implemented by NodeMounter. A mix & match of // functions defined in upstream libraries. (FormatAndMount from struct @@ -36,17 +39,8 @@ type NodeMounter struct { } func NewNodeMounter() (MountWrapper, error) { - safeMounter, err := newSafeMounter() - if err != nil { - return nil, err - } - return &NodeMounter{safeMounter}, nil -} - -// newSafeMounter returns a SafeFormatAndMount -func newSafeMounter() (*mount.SafeFormatAndMount, error) { - return &mount.SafeFormatAndMount{ + return &NodeMounter{SafeFormatAndMount: &mount.SafeFormatAndMount{ Interface: mount.New(""), Exec: utilexec.New(), - }, nil + }}, nil } diff --git a/pkg/driver/mocks/osutils.go b/pkg/utils/os/mock_osutils_unix.go similarity index 96% rename from pkg/driver/mocks/osutils.go rename to pkg/utils/os/mock_osutils_unix.go index 482b639..979acbc 100644 --- a/pkg/driver/mocks/osutils.go +++ b/pkg/utils/os/mock_osutils_unix.go @@ -1,8 +1,8 @@ // Code generated by MockGen. DO NOT EDIT. -// Source: ../../../pkg/driver/osutils/osutils.go +// Source: osutils_unix.go -// Package mocks is a generated GoMock package. -package mocks +// Package os is a generated GoMock package. +package os import ( os "os" diff --git a/pkg/driver/osutils/osutils.go b/pkg/utils/os/osutils_unix.go similarity index 89% rename from pkg/driver/osutils/osutils.go rename to pkg/utils/os/osutils_unix.go index a671fb4..40d47af 100644 --- a/pkg/driver/osutils/osutils.go +++ b/pkg/utils/os/osutils_unix.go @@ -1,3 +1,6 @@ +//go:build linux || darwin +// +build linux darwin + // Copyright 2023 OnMetal authors // // Licensed under the Apache License, Version 2.0 (the "License"); @@ -12,13 +15,13 @@ // See the License for the specific language governing permissions and // limitations under the License. -package osutils +package os import ( "os" ) -//go:generate $MOCKGEN -package mocks -destination=../../../pkg/driver/mocks/osutils.go -source ../../../pkg/driver/osutils/osutils.go +//go:generate $MOCKGEN -package os -destination=mock_osutils_unix.go -source osutils_unix.go // OSWrapper is the interface having os package methods implemented by OsOps. // Defined it explicitly so that it can be mocked. diff --git a/pkg/util/util.go b/pkg/utils/util.go similarity index 99% rename from pkg/util/util.go rename to pkg/utils/util.go index 7f0df32..0ebdb4c 100644 --- a/pkg/util/util.go +++ b/pkg/utils/util.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package util +package utils const ( GiB = 1024 * 1024 * 1024