Refactor Node driver tests (#237)

.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

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

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

hack/update-gomock → 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

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"

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")

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,16 +47,16 @@ 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))
 	}
 	return &driver{
 		config:        config,
 		targetClient:  targetClient,
 		onmetalClient: onMetalClient,
-		mount:         nodeMounter,
-		os:            osutils.OsOps{},
+		mounter:       nodeMounter,
+		os:            os.OsOps{},
 	}
 }
 

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
 	}