Merge pull request #2 from swisstxt/fix/CARGO-209

Fix CARGO-209

.github/workflows/pr-check.yaml

@@ -8,11 +8,11 @@ jobs:
     name: Lint
     runs-on: ubuntu-20.04
     steps:
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@v4
       - name: golangci-lint
-        uses: golangci/golangci-lint-action@v2
+        uses: golangci/golangci-lint-action@v3
         with:
-          version: v1.41.1
+          version: v1.54
           args: --timeout=5m
 
   build:
@@ -22,10 +22,10 @@ jobs:
       - name: Setup up Go 1.x
         uses: actions/setup-go@v2
         with:
-          go-version: "^1.15"
+          go-version: "^1.20"
 
       - name: Check out code
-        uses: actions/checkout@v2
+        uses: actions/checkout@v4
 
       - name: Cache
         uses: actions/cache@v2
@@ -40,6 +40,7 @@ jobs:
 
       - name: Run sanity tests
         run: make test-sanity
-
+        env:
+          NODE_HYPERVISOR: "test"
       - name: Build
         run: make

.github/workflows/release.yaml

@@ -18,7 +18,7 @@ jobs:
 
     steps:
       - name: Check out code
-        uses: actions/checkout@v2
+        uses: actions/checkout@v4
 
       - name: Cache
         uses: actions/cache@v2

.golangci.yml

@@ -6,11 +6,11 @@ issues:
 linters-settings:
 
   staticcheck:
-    go: "1.15"
+    go: "1.20"
     checks: [ "all" ]
 
   stylecheck:
-    go: "1.15"
+    go: "1.20"
     checks: [ "all" ]
 
   goimports:

cmd/cloudstack-csi-driver/main.go

@@ -2,8 +2,8 @@
 //
 // To get usage information:
 //
-//    cloudstack-csi-driver -h
-//
+// cloudstack-csi-driver -h
+
 package main
 
 import (

go.mod

@@ -1,6 +1,6 @@
 module github.com/apalia/cloudstack-csi-driver
 
-go 1.15
+go 1.20
 
 require (
 	github.com/apache/cloudstack-go/v2 v2.9.1-0.20210727090705-0ad6453e08b8
@@ -9,15 +9,50 @@ require (
 	github.com/hashicorp/go-uuid v1.0.2
 	github.com/kubernetes-csi/csi-test/v4 v4.2.0
 	go.uber.org/zap v1.16.0
-	golang.org/x/sys v0.0.0-20201207223542-d4d67f95c62d
+	golang.org/x/sys v0.0.0-20210510120138-977fb7262007
 	golang.org/x/text v0.3.6
-	google.golang.org/genproto v0.0.0-20210726200206-e7812ac95cc0 // indirect
 	google.golang.org/grpc v1.39.0
 	gopkg.in/gcfg.v1 v1.2.3
-	gopkg.in/warnings.v0 v0.1.2 // indirect
 	k8s.io/api v0.21.3
 	k8s.io/apimachinery v0.21.3
 	k8s.io/client-go v0.21.3
 	k8s.io/mount-utils v0.21.3
 	k8s.io/utils v0.0.0-20210111153108-fddb29f9d009
 )
+
+require (
+	github.com/davecgh/go-spew v1.1.1 // indirect
+	github.com/fsnotify/fsnotify v1.4.9 // indirect
+	github.com/go-logr/logr v0.4.0 // indirect
+	github.com/gogo/protobuf v1.3.2 // indirect
+	github.com/golang/protobuf v1.5.2 // indirect
+	github.com/google/go-cmp v0.5.5 // indirect
+	github.com/google/gofuzz v1.1.0 // indirect
+	github.com/google/uuid v1.1.2 // indirect
+	github.com/googleapis/gnostic v0.4.1 // indirect
+	github.com/imdario/mergo v0.3.5 // indirect
+	github.com/json-iterator/go v1.1.10 // indirect
+	github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect
+	github.com/modern-go/reflect2 v1.0.1 // indirect
+	github.com/nxadm/tail v1.4.5 // indirect
+	github.com/onsi/ginkgo v1.14.2 // indirect
+	github.com/onsi/gomega v1.10.4 // indirect
+	github.com/spf13/pflag v1.0.5 // indirect
+	go.uber.org/atomic v1.6.0 // indirect
+	go.uber.org/multierr v1.5.0 // indirect
+	golang.org/x/net v0.0.0-20210405180319-a5a99cb37ef4 // indirect
+	golang.org/x/oauth2 v0.0.0-20200107190931-bf48bf16ab8d // indirect
+	golang.org/x/term v0.0.0-20210220032956-6a3ed077a48d // indirect
+	golang.org/x/time v0.0.0-20210220033141-f8bda1e9f3ba // indirect
+	golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1 // indirect
+	google.golang.org/appengine v1.6.5 // indirect
+	google.golang.org/genproto v0.0.0-20210726200206-e7812ac95cc0 // indirect
+	google.golang.org/protobuf v1.27.1 // indirect
+	gopkg.in/inf.v0 v0.9.1 // indirect
+	gopkg.in/tomb.v1 v1.0.0-20141024135613-dd632973f1e7 // indirect
+	gopkg.in/warnings.v0 v0.1.2 // indirect
+	gopkg.in/yaml.v2 v2.4.0 // indirect
+	k8s.io/klog/v2 v2.8.0 // indirect
+	sigs.k8s.io/structured-merge-diff/v4 v4.1.2 // indirect
+	sigs.k8s.io/yaml v1.2.0 // indirect
+)

pkg/cloud/fake/fake.go

@@ -109,3 +109,12 @@ func (f *fakeConnector) GetDomainID(ctx context.Context) (string, error) {
 func (f *fakeConnector) GetProjectID() string {
 	return "test"
 }
+
+func (f *fakeConnector) ListVolumesForVM(ctx context.Context, virtualMachineID, projectID string) ([]*cloud.Volume, error) {
+	var vols []*cloud.Volume
+	for i := range f.volumesByID {
+		v := f.volumesByID[i]
+		vols = append(vols, &v)
+	}
+	return vols, nil
+}

pkg/cloud/metadata.go

@@ -4,7 +4,6 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
-	"io/ioutil"
 	"os"
 	"strings"
 
@@ -62,7 +61,7 @@ type cloudInitV1 struct {
 func (c *client) readCloudInit(ctx context.Context, instanceFilePath string) (*cloudInitInstanceData, error) {
 	slog := ctxzap.Extract(ctx).Sugar()
 
-	b, err := ioutil.ReadFile(instanceFilePath)
+	b, err := os.ReadFile(instanceFilePath)
 	if err != nil {
 		slog.Errorf("Cannot read %s", instanceFilePath)
 		return nil, err

pkg/cloud/project.go

@@ -2,6 +2,7 @@ package cloud
 
 import (
 	"context"
+
 	"github.com/grpc-ecosystem/go-grpc-middleware/logging/zap/ctxzap"
 )
 

pkg/cloud/volumes.go

@@ -146,7 +146,7 @@ func (c *client) ListVolumesForVM(ctx context.Context, virtualMachineID, project
 	}
 
 	volumes := make([]*Volume, len(l.Volumes))
-	for i, _ := range l.Volumes {
+	for i := range l.Volumes {
 		vol := l.Volumes[i]
 		v := &Volume{
 			ID:               vol.Id,

pkg/driver/controller.go

@@ -3,10 +3,11 @@ package driver
 import (
 	"context"
 	"fmt"
-	"github.com/grpc-ecosystem/go-grpc-middleware/logging/zap/ctxzap"
 	"math/rand"
 	"sync"
 
+	"github.com/grpc-ecosystem/go-grpc-middleware/logging/zap/ctxzap"
+
 	"github.com/container-storage-interface/spec/lib/go/csi"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"

pkg/driver/node.go

@@ -389,6 +389,11 @@ func (ns *nodeServer) NodeGetVolumeStats(ctx context.Context, req *csi.NodeGetVo
 	}
 
 	ctxzap.Extract(ctx).Sugar().Debugf("NodeGetVolumeStats: for volume %s", volumeID)
+
+	if _, err := os.Stat(volumePath); err != nil && os.IsNotExist(err) {
+		return nil, status.Errorf(codes.NotFound, "Volume ID %v on Path %s not found", volumeID, volumePath)
+	}
+
 	_, err := ns.connector.GetVolumeByID(ctx, volumeID)
 	if err == cloud.ErrNotFound {
 		return nil, status.Errorf(codes.NotFound, "Volume %v not found", volumeID)

pkg/mount/mount.go

@@ -5,20 +5,23 @@ package mount
 import (
 	"context"
 	"fmt"
-	"github.com/grpc-ecosystem/go-grpc-middleware/logging/zap/ctxzap"
-	"golang.org/x/sys/unix"
-	"k8s.io/apimachinery/pkg/util/wait"
-	"k8s.io/mount-utils"
-	"k8s.io/utils/exec"
 	"os"
 	"path/filepath"
 	"strconv"
 	"strings"
 	"time"
+
+	"github.com/grpc-ecosystem/go-grpc-middleware/logging/zap/ctxzap"
+	"golang.org/x/sys/unix"
+	"k8s.io/apimachinery/pkg/util/wait"
+	"k8s.io/mount-utils"
+	"k8s.io/utils/exec"
 )
 
 const (
 	diskIDPath = "/dev/disk/by-path"
+	legacySCSI = "pci-0000:00:10.0-scsi-0:0:%s:0"
+	pvSCSI     = "pci-0000:03:00.0-scsi-0:0:%s:0"
 )
 
 // Interface defines the set of methods to allow for
@@ -64,10 +67,10 @@ func New() Interface {
 
 func (m *mounter) GetDevicePath(ctx context.Context, deviceID string, hypervisor string) (string, error) {
 
-	deviceID = CorrectDeviceId(ctx, deviceID, hypervisor)
+	deviceID = CorrectDeviceID(ctx, deviceID, hypervisor)
 
-	deviceID = fmt.Sprintf("pci-0000:00:10.0-scsi-0:0:%s:0", deviceID)
-	ctxzap.Extract(ctx).Sugar().Debugf("device path: %s/%s", diskIDPath, deviceID)
+	legacyDeviceID := fmt.Sprintf(legacySCSI, deviceID)
+	pvDeviceID := fmt.Sprintf(pvSCSI, deviceID)
 
 	backoff := wait.Backoff{
 		Duration: 1 * time.Second,
@@ -77,7 +80,8 @@ func (m *mounter) GetDevicePath(ctx context.Context, deviceID string, hypervisor
 
 	var devicePath string
 	err := wait.ExponentialBackoffWithContext(ctx, backoff, func() (bool, error) {
-		path, err := m.getDevicePathBySerialID(deviceID)
+		path, err := m.getDevicePathBySerialID(ctx, legacyDeviceID, pvDeviceID)
+
 		if err != nil {
 			return false, err
 		}
@@ -91,19 +95,19 @@ func (m *mounter) GetDevicePath(ctx context.Context, deviceID string, hypervisor
 	})
 
 	if err == wait.ErrWaitTimeout {
-		return "", fmt.Errorf("Failed to find device for the deviceID: %q within the alloted time", deviceID)
+		return "", fmt.Errorf("failed to find device for the deviceID: %q within the alloted time", deviceID)
 	} else if devicePath == "" {
-		return "", fmt.Errorf("Device path was empty for deviceID: %q", deviceID)
+		return "", fmt.Errorf("device path was empty for deviceID: %q", deviceID)
 	}
 	return devicePath, nil
 }
 
-//Corrects the device id on the node. the scsi id may not match th id which is set from te cloudstack controller
-//1. ClousStack assumes that SCSI ID 3 is always the CD-ROM and is ignoring this id.
-//https://github.com/apache/cloudstack/blob/98d42750cc21dfce5a8dd6d1880e09a621e0152e/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java#L3442
-//2. SCSI ID 7 is reserved for the Virtual SCSI Controller
-//https://docs.vmware.com/en/VMware-vSphere/6.0/com.vmware.vsphere.hostclient.doc/GUID-5872D173-A076-42FE-8D0B-9DB0EB0E7362_copy.html
-func CorrectDeviceId(ctx context.Context, deviceID, hypervisor string) string {
+// Corrects the device id on the node. the scsi id may not match th id which is set from te cloudstack controller
+// 1. ClousStack assumes that SCSI ID 3 is always the CD-ROM and is ignoring this id.
+// https://github.com/apache/cloudstack/blob/98d42750cc21dfce5a8dd6d1880e09a621e0152e/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java#L3442
+// 2. SCSI ID 7 is reserved for the Virtual SCSI Controller
+// https://docs.vmware.com/en/VMware-vSphere/6.0/com.vmware.vsphere.hostclient.doc/GUID-5872D173-A076-42FE-8D0B-9DB0EB0E7362_copy.html
+func CorrectDeviceID(ctx context.Context, deviceID, hypervisor string) string {
 	ctxzap.Extract(ctx).Sugar().Debugf("device id: '%s' (Hypervisor: %s)", deviceID, hypervisor)
 
 	if strings.ToLower(hypervisor) == "vmware" {
@@ -119,10 +123,19 @@ func CorrectDeviceId(ctx context.Context, deviceID, hypervisor string) string {
 	return deviceID
 }
 
-func (m *mounter) getDevicePathBySerialID(volumeID string) (string, error) {
-	source := filepath.Join(diskIDPath, volumeID)
+func (m *mounter) getDevicePathBySerialID(ctx context.Context, legacyPath, pvPath string) (string, error) {
+	source := filepath.Join(diskIDPath, legacyPath)
 	_, err := os.Stat(source)
+
+	//If legacy path not exists, try with new path
+	if os.IsNotExist(err) {
+		ctxzap.Extract(ctx).Sugar().Debugf("no device found with legacy path '%s', try with '%s'", legacyPath, pvPath)
+		source = filepath.Join(diskIDPath, pvPath)
+		_, err = os.Stat(source)
+	}
+
 	if err == nil {
+		ctxzap.Extract(ctx).Sugar().Debugf("device found with path '%s'", source)
 		return source, nil
 	}
 	if !os.IsNotExist(err) {
@@ -146,7 +159,7 @@ func (m *mounter) rescanScsi(ctx context.Context) {
 func (m *mounter) CleanScsi(ctx context.Context, deviceID, hypervisor string) {
 	log := ctxzap.Extract(ctx).Sugar()
 
-	deviceID = CorrectDeviceId(ctx, deviceID, hypervisor)
+	deviceID = CorrectDeviceID(ctx, deviceID, hypervisor)
 
 	devicePath := fmt.Sprintf("/sys/class/scsi_device/0:0:%s:0/device/delete", deviceID)
 	log.Debugf("removing SCSI devices on %s", devicePath)
@@ -164,19 +177,6 @@ func (m *mounter) GetDeviceName(mountPath string) (string, int, error) {
 	return mount.GetDeviceNameFromMount(m, mountPath)
 }
 
-// diskUUIDToSerial reproduces CloudStack function diskUuidToSerial
-// from https://github.com/apache/cloudstack/blob/0f3f2a0937/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java#L3000
-//
-// This is what CloudStack do *with KVM hypervisor* to translate
-// a CloudStack volume UUID to libvirt disk serial.
-func diskUUIDToSerial(uuid string) string {
-	uuidWithoutHyphen := strings.ReplaceAll(uuid, "-", "")
-	if len(uuidWithoutHyphen) < 20 {
-		return uuidWithoutHyphen
-	}
-	return uuidWithoutHyphen[:20]
-}
-
 func (*mounter) ExistsPath(filename string) (bool, error) {
 	if _, err := os.Stat(filename); os.IsNotExist(err) {
 		return false, nil
@@ -209,7 +209,7 @@ func (*mounter) MakeFile(pathname string) error {
 	return nil
 }
 
-//Copy Pasta from https://github.com/digitalocean/csi-digitalocean/blob/db266f4044178a96c5aa9e2420efae8723af75f4/driver/mounter.go
+// Copy Pasta from https://github.com/digitalocean/csi-digitalocean/blob/db266f4044178a96c5aa9e2420efae8723af75f4/driver/mounter.go
 func (m *mounter) GetStatistics(volumePath string) (volumeStatistics, error) {
 	isBlock, err := m.IsBlockDevice(volumePath)
 	if err != nil {