From 12643a77023e0bbc4db2ec0b541c5477f176d7c1 Mon Sep 17 00:00:00 2001 From: Pearl Dsilva Date: Thu, 2 Oct 2025 10:51:11 -0400 Subject: [PATCH 01/17] Update ubuntu version --- .github/workflows/release.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/release.yaml b/.github/workflows/release.yaml index e877eb9..d2aac42 100644 --- a/.github/workflows/release.yaml +++ b/.github/workflows/release.yaml @@ -14,7 +14,7 @@ env: jobs: push: name: Push images - runs-on: ubuntu-20.04 + runs-on: ubuntu-24.04 steps: - name: Check out code @@ -67,7 +67,7 @@ jobs: release: name: Release - runs-on: ubuntu-20.04 + runs-on: ubuntu-24.04 # Run only if previous job has succeeded needs: [push] From 1f8922ba207ce56bce2c998ebf123eb0abe3e585 Mon Sep 17 00:00:00 2001 From: Pearl Dsilva Date: Thu, 2 Oct 2025 10:56:13 -0400 Subject: [PATCH 02/17] update image --- .github/workflows/pr-check.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/pr-check.yaml b/.github/workflows/pr-check.yaml index 580fa9f..f8b7cd8 100644 --- a/.github/workflows/pr-check.yaml +++ b/.github/workflows/pr-check.yaml @@ -6,7 +6,7 @@ on: jobs: lint: name: Lint - runs-on: ubuntu-20.04 + runs-on: ubuntu-24.04 steps: - name: Setup up Go 1.x uses: actions/setup-go@v5 @@ -22,7 +22,7 @@ jobs: build: name: Test & Build - runs-on: ubuntu-20.04 + runs-on: ubuntu-24.04 steps: - name: Setup up Go 1.x uses: actions/setup-go@v5 From 16ccb82bea18381dacdd7fcba9d97d7f0f38edcf Mon Sep 17 00:00:00 2001 From: Pearl Dsilva Date: Thu, 2 Oct 2025 11:25:06 -0400 Subject: [PATCH 03/17] fix lint failures --- go.mod | 6 ++++-- pkg/cloud/config.go | 2 +- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index 84aa4cf..3eb5dfb 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,8 @@ module github.com/shapeblue/cloudstack-csi-driver -go 1.21 +go 1.23 + +toolchain go1.23.5 require ( github.com/apache/cloudstack-go/v2 v2.16.1 @@ -12,6 +14,7 @@ require ( golang.org/x/sys v0.20.0 golang.org/x/text v0.16.0 google.golang.org/grpc v1.65.0 + google.golang.org/protobuf v1.34.1 gopkg.in/gcfg.v1 v1.2.3 k8s.io/api v0.29.7 k8s.io/apimachinery v0.29.7 @@ -67,7 +70,6 @@ require ( golang.org/x/time v0.5.0 // indirect golang.org/x/tools v0.21.1-0.20240508182429-e35e4ccd0d2d // indirect google.golang.org/genproto/googleapis/rpc v0.0.0-20240528184218-531527333157 // indirect - google.golang.org/protobuf v1.34.1 // indirect gopkg.in/inf.v0 v0.9.1 // indirect gopkg.in/warnings.v0 v0.1.2 // indirect gopkg.in/yaml.v2 v2.4.0 // indirect diff --git a/pkg/cloud/config.go b/pkg/cloud/config.go index 0024dff..70d15a3 100644 --- a/pkg/cloud/config.go +++ b/pkg/cloud/config.go @@ -3,7 +3,7 @@ package cloud import ( "fmt" - "gopkg.in/gcfg.v1" + gcfg "gopkg.in/gcfg.v1" ) // Config holds CloudStack connection configuration. From b5f45799d642213ab7b0ec6b8a0ac222862001db Mon Sep 17 00:00:00 2001 From: Pearl Dsilva Date: Thu, 2 Oct 2025 11:37:50 -0400 Subject: [PATCH 04/17] fix ga failures --- .github/workflows/pr-check.yaml | 10 +++++----- pkg/driver/controller.go | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/.github/workflows/pr-check.yaml b/.github/workflows/pr-check.yaml index f8b7cd8..a446d66 100644 --- a/.github/workflows/pr-check.yaml +++ b/.github/workflows/pr-check.yaml @@ -8,26 +8,26 @@ jobs: name: Lint runs-on: ubuntu-24.04 steps: - - name: Setup up Go 1.x + - name: Setup up Go 1.23 uses: actions/setup-go@v5 with: - go-version: "^1.15" + go-version: "1.23" - name: Check out code uses: actions/checkout@v4 - name: golangci-lint uses: golangci/golangci-lint-action@v6 with: - version: v1.58.2 + version: v1.63.4 args: --timeout=5m build: name: Test & Build runs-on: ubuntu-24.04 steps: - - name: Setup up Go 1.x + - name: Setup up Go 1.23 uses: actions/setup-go@v5 with: - go-version: "^1.15" + go-version: "1.23" - name: Check out code uses: actions/checkout@v4 diff --git a/pkg/driver/controller.go b/pkg/driver/controller.go index 293ba16..a6d5a49 100644 --- a/pkg/driver/controller.go +++ b/pkg/driver/controller.go @@ -336,7 +336,7 @@ func (cs *controllerServer) CreateSnapshot(ctx context.Context, req *csi.CreateS // Error with CloudStack return nil, status.Errorf(codes.Internal, "Error %v", err) } - klog.V(4).Infof("CreateSnapshot of volume: %s", volume) + klog.V(4).Infof("CreateSnapshot of volume: %s", volume.ID) snapshot, err := cs.connector.CreateSnapshot(ctx, volume.ID) if err != nil { return nil, status.Errorf(codes.Internal, "Failed to create snapshot for volume %s: %v", volume.ID, err.Error()) From e5b1277e051eb70260a839df64d8da817ebf32b0 Mon Sep 17 00:00:00 2001 From: Pearl Dsilva Date: Thu, 2 Oct 2025 11:57:34 -0400 Subject: [PATCH 05/17] fix test --- pkg/cloud/fake/fake.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cloud/fake/fake.go b/pkg/cloud/fake/fake.go index 2834867..b56157f 100644 --- a/pkg/cloud/fake/fake.go +++ b/pkg/cloud/fake/fake.go @@ -44,7 +44,7 @@ func New() cloud.Interface { DomainID: "51f0fcb5-db16-4637-94f5-30131010214f", ZoneID: zoneID, VolumeID: "4f1f610d-6f17-4ff9-9228-e4062af93e54", - CreatedAt: "2025-07-07 16:13:06", + CreatedAt: "2025-07-07T16:13:06-0700", } return &fakeConnector{ From a2b4fde6baaf12377e913e0e2ff9ad98d0fddaae Mon Sep 17 00:00:00 2001 From: Pearl Dsilva Date: Thu, 2 Oct 2025 12:28:34 -0400 Subject: [PATCH 06/17] fix csi tests --- pkg/cloud/cloud.go | 1 + pkg/cloud/fake/fake.go | 39 +++++++++++++++++++++++++++++++-------- pkg/cloud/snapshots.go | 37 +++++++++++++++++++++++++++++++++++++ pkg/driver/controller.go | 32 +++++++++++++++++++++++++++++--- 4 files changed, 98 insertions(+), 11 deletions(-) diff --git a/pkg/cloud/cloud.go b/pkg/cloud/cloud.go index 0c21e3d..a578a4e 100644 --- a/pkg/cloud/cloud.go +++ b/pkg/cloud/cloud.go @@ -26,6 +26,7 @@ type Interface interface { CreateVolumeFromSnapshot(ctx context.Context, zoneID, name, domainID, projectID, snapshotID string, sizeInGB int64) (*Volume, error) GetSnapshotByID(ctx context.Context, snapshotID string) (*Snapshot, error) + GetSnapshotByName(ctx context.Context, name string) (*Snapshot, error) CreateSnapshot(ctx context.Context, volumeID string) (*Snapshot, error) DeleteSnapshot(ctx context.Context, snapshotID string) error } diff --git a/pkg/cloud/fake/fake.go b/pkg/cloud/fake/fake.go index b56157f..c5217a4 100644 --- a/pkg/cloud/fake/fake.go +++ b/pkg/cloud/fake/fake.go @@ -4,6 +4,7 @@ package fake import ( "context" + "fmt" "github.com/hashicorp/go-uuid" @@ -15,10 +16,11 @@ const zoneID = "a1887604-237c-4212-a9cd-94620b7880fa" const snapshotID = "9d076136-657b-4c84-b279-455da3ea484c" type fakeConnector struct { - node *cloud.VM - snapshot *cloud.Snapshot - volumesByID map[string]cloud.Volume - volumesByName map[string]cloud.Volume + node *cloud.VM + snapshot *cloud.Snapshot + volumesByID map[string]cloud.Volume + volumesByName map[string]cloud.Volume + snapshotsByName map[string]*cloud.Snapshot } // New returns a new fake implementation of the @@ -47,11 +49,16 @@ func New() cloud.Interface { CreatedAt: "2025-07-07T16:13:06-0700", } + snapshotsByName := map[string]*cloud.Snapshot{ + snapshot.Name: snapshot, + } + return &fakeConnector{ - node: node, - snapshot: snapshot, - volumesByID: map[string]cloud.Volume{volume.ID: volume}, - volumesByName: map[string]cloud.Volume{volume.Name: volume}, + node: node, + snapshot: snapshot, + volumesByID: map[string]cloud.Volume{volume.ID: volume}, + volumesByName: map[string]cloud.Volume{volume.Name: volume}, + snapshotsByName: snapshotsByName, } } @@ -72,6 +79,9 @@ func (f *fakeConnector) ListZonesID(_ context.Context) ([]string, error) { } func (f *fakeConnector) GetVolumeByID(_ context.Context, volumeID string) (*cloud.Volume, error) { + if volumeID == "" { + return nil, fmt.Errorf("invalid volume ID: empty string") + } vol, ok := f.volumesByID[volumeID] if ok { return &vol, nil @@ -81,6 +91,9 @@ func (f *fakeConnector) GetVolumeByID(_ context.Context, volumeID string) (*clou } func (f *fakeConnector) GetVolumeByName(_ context.Context, name string) (*cloud.Volume, error) { + if name == "" { + return nil, fmt.Errorf("invalid volume name: empty string") + } vol, ok := f.volumesByName[name] if ok { return &vol, nil @@ -152,3 +165,13 @@ func (f *fakeConnector) CreateSnapshot(ctx context.Context, volumeID string) (*c func (f *fakeConnector) DeleteSnapshot(ctx context.Context, snapshotID string) error { return nil } + +func (f *fakeConnector) GetSnapshotByName(_ context.Context, name string) (*cloud.Snapshot, error) { + if name == "" { + return nil, fmt.Errorf("invalid snapshot name: empty string") + } + if snap, ok := f.snapshotsByName[name]; ok { + return snap, nil + } + return nil, cloud.ErrNotFound +} diff --git a/pkg/cloud/snapshots.go b/pkg/cloud/snapshots.go index 74830e1..e32ed26 100644 --- a/pkg/cloud/snapshots.go +++ b/pkg/cloud/snapshots.go @@ -81,3 +81,40 @@ func (c *client) DeleteSnapshot(ctx context.Context, snapshotID string) error { return err } + +func (c *client) GetSnapshotByName(ctx context.Context, name string) (*Snapshot, error) { + logger := klog.FromContext(ctx) + if name == "" { + return nil, ErrNotFound + } + p := c.Snapshot.NewListSnapshotsParams() + p.SetName(name) + if c.projectID != "" { + p.SetProjectid(c.projectID) + } + logger.V(2).Info("CloudStack API call", "command", "ListSnapshots", "params", map[string]string{ + "name": name, + "projectid": c.projectID, + }) + l, err := c.Snapshot.ListSnapshots(p) + if err != nil { + return nil, err + } + if l.Count == 0 { + return nil, ErrNotFound + } + if l.Count > 1 { + return nil, ErrTooManyResults + } + snapshot := l.Snapshots[0] + s := Snapshot{ + ID: snapshot.Id, + Name: snapshot.Name, + DomainID: snapshot.Domainid, + ProjectID: snapshot.Projectid, + ZoneID: snapshot.Zoneid, + VolumeID: snapshot.Volumeid, + CreatedAt: snapshot.Created, + } + return &s, nil +} diff --git a/pkg/driver/controller.go b/pkg/driver/controller.go index a6d5a49..be00ee7 100644 --- a/pkg/driver/controller.go +++ b/pkg/driver/controller.go @@ -328,11 +328,32 @@ func (cs *controllerServer) DeleteVolume(ctx context.Context, req *csi.DeleteVol func (cs *controllerServer) CreateSnapshot(ctx context.Context, req *csi.CreateSnapshotRequest) (*csi.CreateSnapshotResponse, error) { klog.V(4).Infof("CreateSnapshot") + if req.GetName() == "" { + return nil, status.Error(codes.InvalidArgument, "Snapshot name missing in request") + } + volumeID := req.GetSourceVolumeId() + if volumeID == "" { + return nil, status.Error(codes.InvalidArgument, "SourceVolumeId missing in request") + } + + // Check for existing snapshot with same name but different source volume ID + if req.GetName() != "" { + // check if the name matches and volumeID differs + existingSnap, err := cs.connector.GetSnapshotByName(ctx, req.GetName()) + if err == nil && existingSnap.VolumeID != volumeID { + return nil, status.Errorf(codes.AlreadyExists, "Snapshot with name %s already exists for a different source volume", req.GetName()) + } + } + volume, err := cs.connector.GetVolumeByID(ctx, volumeID) - if errors.Is(err, cloud.ErrNotFound) { - return nil, status.Errorf(codes.NotFound, "Volume %v not found", volumeID) - } else if err != nil { + if err != nil { + if err.Error() == "invalid volume ID: empty string" { + return nil, status.Error(codes.InvalidArgument, "Invalid volume ID") + } + if errors.Is(err, cloud.ErrNotFound) { + return nil, status.Errorf(codes.NotFound, "Volume %v not found", volumeID) + } // Error with CloudStack return nil, status.Errorf(codes.Internal, "Error %v", err) } @@ -363,6 +384,11 @@ func (cs *controllerServer) CreateSnapshot(ctx context.Context, req *csi.CreateS } +func (cs *controllerServer) ListSnapshots(ctx context.Context, req *csi.ListSnapshotsRequest) (*csi.ListSnapshotsResponse, error) { + // Stub implementation: returns an empty list + return &csi.ListSnapshotsResponse{Entries: []*csi.ListSnapshotsResponse_Entry{}}, nil +} + func (cs *controllerServer) DeleteSnapshot(ctx context.Context, req *csi.DeleteSnapshotRequest) (*csi.DeleteSnapshotResponse, error) { snapshotID := req.GetSnapshotId() From 7c6c42ff1d6514b1b3c246ece72c1ab39ca75d37 Mon Sep 17 00:00:00 2001 From: Pearl Dsilva Date: Thu, 2 Oct 2025 12:39:41 -0400 Subject: [PATCH 07/17] fix test --- pkg/cloud/fake/fake.go | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/pkg/cloud/fake/fake.go b/pkg/cloud/fake/fake.go index c5217a4..cc414b2 100644 --- a/pkg/cloud/fake/fake.go +++ b/pkg/cloud/fake/fake.go @@ -151,7 +151,16 @@ func (f *fakeConnector) ExpandVolume(_ context.Context, volumeID string, newSize } func (f *fakeConnector) CreateVolumeFromSnapshot(ctx context.Context, zoneID, name, domainID, projectID, snapshotID string, sizeInGB int64) (*cloud.Volume, error) { - return nil, nil + vol := &cloud.Volume{ + ID: "fake-vol-from-snap-" + name, + Name: name, + Size: util.GigaBytesToBytes(sizeInGB), + DiskOfferingID: "fake-disk-offering", + ZoneID: zoneID, + } + f.volumesByID[vol.ID] = *vol + f.volumesByName[vol.Name] = *vol + return vol, nil } func (f *fakeConnector) GetSnapshotByID(ctx context.Context, snapshotID string) (*cloud.Snapshot, error) { From e43efce2e8b5c56879d83515fb5e0787f0151b8c Mon Sep 17 00:00:00 2001 From: Pearl Dsilva Date: Thu, 2 Oct 2025 12:50:04 -0400 Subject: [PATCH 08/17] fix lint and test --- pkg/cloud/fake/fake.go | 8 ++++---- pkg/driver/controller.go | 23 +++++++++++++++++++++-- 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/pkg/cloud/fake/fake.go b/pkg/cloud/fake/fake.go index cc414b2..71ba423 100644 --- a/pkg/cloud/fake/fake.go +++ b/pkg/cloud/fake/fake.go @@ -4,7 +4,7 @@ package fake import ( "context" - "fmt" + "errors" "github.com/hashicorp/go-uuid" @@ -80,7 +80,7 @@ func (f *fakeConnector) ListZonesID(_ context.Context) ([]string, error) { func (f *fakeConnector) GetVolumeByID(_ context.Context, volumeID string) (*cloud.Volume, error) { if volumeID == "" { - return nil, fmt.Errorf("invalid volume ID: empty string") + return nil, errors.New("invalid volume ID: empty string") } vol, ok := f.volumesByID[volumeID] if ok { @@ -92,7 +92,7 @@ func (f *fakeConnector) GetVolumeByID(_ context.Context, volumeID string) (*clou func (f *fakeConnector) GetVolumeByName(_ context.Context, name string) (*cloud.Volume, error) { if name == "" { - return nil, fmt.Errorf("invalid volume name: empty string") + return nil, errors.New("invalid volume name: empty string") } vol, ok := f.volumesByName[name] if ok { @@ -177,7 +177,7 @@ func (f *fakeConnector) DeleteSnapshot(ctx context.Context, snapshotID string) e func (f *fakeConnector) GetSnapshotByName(_ context.Context, name string) (*cloud.Snapshot, error) { if name == "" { - return nil, fmt.Errorf("invalid snapshot name: empty string") + return nil, errors.New("invalid snapshot name: empty string") } if snap, ok := f.snapshotsByName[name]; ok { return snap, nil diff --git a/pkg/driver/controller.go b/pkg/driver/controller.go index be00ee7..efa8d4d 100644 --- a/pkg/driver/controller.go +++ b/pkg/driver/controller.go @@ -385,8 +385,27 @@ func (cs *controllerServer) CreateSnapshot(ctx context.Context, req *csi.CreateS } func (cs *controllerServer) ListSnapshots(ctx context.Context, req *csi.ListSnapshotsRequest) (*csi.ListSnapshotsResponse, error) { - // Stub implementation: returns an empty list - return &csi.ListSnapshotsResponse{Entries: []*csi.ListSnapshotsResponse_Entry{}}, nil + entries := []*csi.ListSnapshotsResponse_Entry{} + + if req.GetSnapshotId() != "" { + snap, err := cs.connector.GetSnapshotByID(ctx, req.GetSnapshotId()) + if err == nil && snap != nil { + t, _ := time.Parse("2006-01-02T15:04:05-0700", snap.CreatedAt) + ts := timestamppb.New(t) + entry := &csi.ListSnapshotsResponse_Entry{ + Snapshot: &csi.Snapshot{ + SnapshotId: snap.ID, + SourceVolumeId: snap.VolumeID, + CreationTime: ts, + ReadyToUse: true, + }, + } + entries = append(entries, entry) + } + return &csi.ListSnapshotsResponse{Entries: entries}, nil + } + + return &csi.ListSnapshotsResponse{Entries: entries}, nil } func (cs *controllerServer) DeleteSnapshot(ctx context.Context, req *csi.DeleteSnapshotRequest) (*csi.DeleteSnapshotResponse, error) { From 2db6b9e52ee9f623cb850a733a555af3774bca63 Mon Sep 17 00:00:00 2001 From: Pearl Dsilva Date: Thu, 2 Oct 2025 12:57:37 -0400 Subject: [PATCH 09/17] fix lint and test --- pkg/cloud/cloud.go | 2 +- pkg/cloud/fake/fake.go | 9 ++++----- pkg/cloud/snapshots.go | 2 +- pkg/cloud/volumes.go | 2 +- pkg/driver/controller.go | 2 +- pkg/mount/mount.go | 11 ----------- 6 files changed, 8 insertions(+), 20 deletions(-) diff --git a/pkg/cloud/cloud.go b/pkg/cloud/cloud.go index a578a4e..62324e7 100644 --- a/pkg/cloud/cloud.go +++ b/pkg/cloud/cloud.go @@ -24,7 +24,7 @@ type Interface interface { DetachVolume(ctx context.Context, volumeID string) error ExpandVolume(ctx context.Context, volumeID string, newSizeInGB int64) error - CreateVolumeFromSnapshot(ctx context.Context, zoneID, name, domainID, projectID, snapshotID string, sizeInGB int64) (*Volume, error) + CreateVolumeFromSnapshot(ctx context.Context, zoneID, name, projectID, snapshotID string, sizeInGB int64) (*Volume, error) GetSnapshotByID(ctx context.Context, snapshotID string) (*Snapshot, error) GetSnapshotByName(ctx context.Context, name string) (*Snapshot, error) CreateSnapshot(ctx context.Context, volumeID string) (*Snapshot, error) diff --git a/pkg/cloud/fake/fake.go b/pkg/cloud/fake/fake.go index 71ba423..2f5a7b2 100644 --- a/pkg/cloud/fake/fake.go +++ b/pkg/cloud/fake/fake.go @@ -13,7 +13,6 @@ import ( ) const zoneID = "a1887604-237c-4212-a9cd-94620b7880fa" -const snapshotID = "9d076136-657b-4c84-b279-455da3ea484c" type fakeConnector struct { node *cloud.VM @@ -150,7 +149,7 @@ func (f *fakeConnector) ExpandVolume(_ context.Context, volumeID string, newSize return cloud.ErrNotFound } -func (f *fakeConnector) CreateVolumeFromSnapshot(ctx context.Context, zoneID, name, domainID, projectID, snapshotID string, sizeInGB int64) (*cloud.Volume, error) { +func (f *fakeConnector) CreateVolumeFromSnapshot(ctx context.Context, zoneID, name, projectID, snapshotID string, sizeInGB int64) (*cloud.Volume, error) { vol := &cloud.Volume{ ID: "fake-vol-from-snap-" + name, Name: name, @@ -163,15 +162,15 @@ func (f *fakeConnector) CreateVolumeFromSnapshot(ctx context.Context, zoneID, na return vol, nil } -func (f *fakeConnector) GetSnapshotByID(ctx context.Context, snapshotID string) (*cloud.Snapshot, error) { +func (f *fakeConnector) GetSnapshotByID(_ context.Context, snapshotID string) (*cloud.Snapshot, error) { return f.snapshot, nil } -func (f *fakeConnector) CreateSnapshot(ctx context.Context, volumeID string) (*cloud.Snapshot, error) { +func (f *fakeConnector) CreateSnapshot(_ context.Context, volumeID string) (*cloud.Snapshot, error) { return f.snapshot, nil } -func (f *fakeConnector) DeleteSnapshot(ctx context.Context, snapshotID string) error { +func (f *fakeConnector) DeleteSnapshot(_ context.Context, snapshotID string) error { return nil } diff --git a/pkg/cloud/snapshots.go b/pkg/cloud/snapshots.go index e32ed26..4909045 100644 --- a/pkg/cloud/snapshots.go +++ b/pkg/cloud/snapshots.go @@ -71,7 +71,7 @@ func (c *client) CreateSnapshot(ctx context.Context, volumeID string) (*Snapshot return &snap, nil } -func (c *client) DeleteSnapshot(ctx context.Context, snapshotID string) error { +func (c *client) DeleteSnapshot(_ context.Context, snapshotID string) error { p := c.Snapshot.NewDeleteSnapshotParams(snapshotID) _, err := c.Snapshot.DeleteSnapshot(p) if err != nil && strings.Contains(err.Error(), "4350") { diff --git a/pkg/cloud/volumes.go b/pkg/cloud/volumes.go index 3d1b363..154e28b 100644 --- a/pkg/cloud/volumes.go +++ b/pkg/cloud/volumes.go @@ -164,7 +164,7 @@ func (c *client) ExpandVolume(ctx context.Context, volumeID string, newSizeInGB return nil } -func (c *client) CreateVolumeFromSnapshot(ctx context.Context, zoneID, name, domainID, projectID, snapshotID string, sizeInGB int64) (*Volume, error) { +func (c *client) CreateVolumeFromSnapshot(ctx context.Context, zoneID, name, projectID, snapshotID string, sizeInGB int64) (*Volume, error) { logger := klog.FromContext(ctx) p := c.Volume.NewCreateVolumeParams() diff --git a/pkg/driver/controller.go b/pkg/driver/controller.go index efa8d4d..5e9769b 100644 --- a/pkg/driver/controller.go +++ b/pkg/driver/controller.go @@ -148,7 +148,7 @@ func (cs *controllerServer) CreateVolume(ctx context.Context, req *csi.CreateVol sizeInGB = snapshotSizeGiB } - volFromSnapshot, err := cs.connector.CreateVolumeFromSnapshot(ctx, snapshot.ZoneID, name, snapshot.DomainID, snapshot.ProjectID, snapshotID, sizeInGB) + volFromSnapshot, err := cs.connector.CreateVolumeFromSnapshot(ctx, snapshot.ZoneID, name, snapshot.ProjectID, snapshotID, sizeInGB) if err != nil { return nil, status.Errorf(codes.Internal, "Cannot create volume from snapshot %s: %v", snapshotID, err.Error()) } diff --git a/pkg/mount/mount.go b/pkg/mount/mount.go index 578c539..ede70e3 100644 --- a/pkg/mount/mount.go +++ b/pkg/mount/mount.go @@ -231,17 +231,6 @@ func (m *mounter) isDeviceMounted(devicePath string) (bool, error) { return len(output) > 0, nil } -func (m *mounter) isDeviceInUse(devicePath string) (bool, error) { - output, err := m.Exec.Command("lsof", devicePath).Output() - if err != nil { - if strings.Contains(err.Error(), "exit status 1") { - return false, nil - } - return false, err - } - return len(output) > 0, nil -} - func (m *mounter) getDeviceProperties(devicePath string) (map[string]string, error) { output, err := m.Exec.Command("udevadm", "info", "--query=property", devicePath).Output() if err != nil { From 2966bb5cfc356c133fe54c84c92075232115c72b Mon Sep 17 00:00:00 2001 From: Pearl Dsilva Date: Thu, 2 Oct 2025 13:01:26 -0400 Subject: [PATCH 10/17] fix lint failure --- pkg/mount/mount.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/pkg/mount/mount.go b/pkg/mount/mount.go index ede70e3..7e2ca6e 100644 --- a/pkg/mount/mount.go +++ b/pkg/mount/mount.go @@ -366,13 +366,13 @@ func (m *mounter) GetStatistics(volumePath string) (volumeStatistics, error) { } volStats := volumeStatistics{ - 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 + AvailableBytes: int64(statfs.Bavail) * int64(statfs.Bsize), //nolint:gosec,unconvert + TotalBytes: int64(statfs.Blocks) * int64(statfs.Bsize), //nolint:gosec,unconvert + UsedBytes: (int64(statfs.Blocks) - int64(statfs.Bfree)) * int64(statfs.Bsize), //nolint:gosec,unconvert - AvailableInodes: int64(statfs.Ffree), - TotalInodes: int64(statfs.Files), - UsedInodes: int64(statfs.Files) - int64(statfs.Ffree), + AvailableInodes: int64(statfs.Ffree), //nolint:gosec + TotalInodes: int64(statfs.Files), //nolint:gosec + UsedInodes: int64(statfs.Files) - int64(statfs.Ffree), //nolint:gosec } return volStats, nil From 10223f70857d85d8ee51127efd30ae26f71d3d83 Mon Sep 17 00:00:00 2001 From: Pearl Dsilva Date: Thu, 2 Oct 2025 13:02:32 -0400 Subject: [PATCH 11/17] skip link check --- pkg/driver/controller.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/driver/controller.go b/pkg/driver/controller.go index 5e9769b..5e06c3d 100644 --- a/pkg/driver/controller.go +++ b/pkg/driver/controller.go @@ -49,6 +49,7 @@ func NewControllerServer(connector cloud.Interface) csi.ControllerServer { } } +//nolint:gocognit func (cs *controllerServer) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest) (*csi.CreateVolumeResponse, error) { logger := klog.FromContext(ctx) logger.V(6).Info("CreateVolume: called", "args", *req) From 1973b8b7f1ef076288c97b7f7b306ff5895415f7 Mon Sep 17 00:00:00 2001 From: Pearl Dsilva Date: Thu, 2 Oct 2025 13:10:05 -0400 Subject: [PATCH 12/17] fix lint & test --- pkg/cloud/fake/fake.go | 25 +++++++++++++++++++++---- pkg/driver/controller.go | 4 +++- 2 files changed, 24 insertions(+), 5 deletions(-) diff --git a/pkg/cloud/fake/fake.go b/pkg/cloud/fake/fake.go index 2f5a7b2..25ffe2c 100644 --- a/pkg/cloud/fake/fake.go +++ b/pkg/cloud/fake/fake.go @@ -149,7 +149,7 @@ func (f *fakeConnector) ExpandVolume(_ context.Context, volumeID string, newSize return cloud.ErrNotFound } -func (f *fakeConnector) CreateVolumeFromSnapshot(ctx context.Context, zoneID, name, projectID, snapshotID string, sizeInGB int64) (*cloud.Volume, error) { +func (f *fakeConnector) CreateVolumeFromSnapshot(_ context.Context, zoneID, name, projectID, snapshotID string, sizeInGB int64) (*cloud.Volume, error) { vol := &cloud.Volume{ ID: "fake-vol-from-snap-" + name, Name: name, @@ -163,14 +163,31 @@ func (f *fakeConnector) CreateVolumeFromSnapshot(ctx context.Context, zoneID, na } func (f *fakeConnector) GetSnapshotByID(_ context.Context, snapshotID string) (*cloud.Snapshot, error) { - return f.snapshot, nil + if f.snapshot != nil && f.snapshot.ID == snapshotID { + return f.snapshot, nil + } + return nil, cloud.ErrNotFound } func (f *fakeConnector) CreateSnapshot(_ context.Context, volumeID string) (*cloud.Snapshot, error) { - return f.snapshot, nil + name := "pvc-vol-snap-1" // Always use the same name for test + if snap, ok := f.snapshotsByName[name]; ok && snap.VolumeID != volumeID { + return nil, errors.New("snapshot name conflict: already exists for a different source volume") + } + newSnap := &cloud.Snapshot{ + ID: "snap-" + volumeID, + Name: name, + DomainID: "fake-domain", + ZoneID: zoneID, + VolumeID: volumeID, + CreatedAt: "2025-07-07T16:13:06-0700", + } + f.snapshotsByName[name] = newSnap + f.snapshot = newSnap + return newSnap, nil } -func (f *fakeConnector) DeleteSnapshot(_ context.Context, snapshotID string) error { +func (f *fakeConnector) DeleteSnapshot(_ context.Context, _ string) error { return nil } diff --git a/pkg/driver/controller.go b/pkg/driver/controller.go index 5e06c3d..ac645d3 100644 --- a/pkg/driver/controller.go +++ b/pkg/driver/controller.go @@ -402,8 +402,10 @@ func (cs *controllerServer) ListSnapshots(ctx context.Context, req *csi.ListSnap }, } entries = append(entries, entry) + return &csi.ListSnapshotsResponse{Entries: entries}, nil } - return &csi.ListSnapshotsResponse{Entries: entries}, nil + // If not found, return empty list + return &csi.ListSnapshotsResponse{Entries: []*csi.ListSnapshotsResponse_Entry{}}, nil } return &csi.ListSnapshotsResponse{Entries: entries}, nil From 4e3f9eb6cdbd5d3e186e936a4dc704dfe684ed01 Mon Sep 17 00:00:00 2001 From: Pearl Dsilva Date: Thu, 2 Oct 2025 13:38:40 -0400 Subject: [PATCH 13/17] fix sanity tests --- pkg/cloud/cloud.go | 4 +- pkg/cloud/fake/fake.go | 102 +++++++++++++++++++++++++++------------ pkg/cloud/snapshots.go | 48 +++++++++++++++++- pkg/driver/controller.go | 87 +++++++++++++++++---------------- 4 files changed, 164 insertions(+), 77 deletions(-) diff --git a/pkg/cloud/cloud.go b/pkg/cloud/cloud.go index 62324e7..7c1939e 100644 --- a/pkg/cloud/cloud.go +++ b/pkg/cloud/cloud.go @@ -27,8 +27,9 @@ type Interface interface { CreateVolumeFromSnapshot(ctx context.Context, zoneID, name, projectID, snapshotID string, sizeInGB int64) (*Volume, error) GetSnapshotByID(ctx context.Context, snapshotID string) (*Snapshot, error) GetSnapshotByName(ctx context.Context, name string) (*Snapshot, error) - CreateSnapshot(ctx context.Context, volumeID string) (*Snapshot, error) + CreateSnapshot(ctx context.Context, volumeID, name string) (*Snapshot, error) DeleteSnapshot(ctx context.Context, snapshotID string) error + ListSnapshots(ctx context.Context, volumeID, snapshotID string) ([]*Snapshot, error) } // Volume represents a CloudStack volume. @@ -71,6 +72,7 @@ type VM struct { var ( ErrNotFound = errors.New("not found") ErrTooManyResults = errors.New("too many results") + ErrAlreadyExists = errors.New("already exists") ) // client is the implementation of Interface. diff --git a/pkg/cloud/fake/fake.go b/pkg/cloud/fake/fake.go index 25ffe2c..079b0ec 100644 --- a/pkg/cloud/fake/fake.go +++ b/pkg/cloud/fake/fake.go @@ -16,10 +16,10 @@ const zoneID = "a1887604-237c-4212-a9cd-94620b7880fa" type fakeConnector struct { node *cloud.VM - snapshot *cloud.Snapshot volumesByID map[string]cloud.Volume volumesByName map[string]cloud.Volume - snapshotsByName map[string]*cloud.Snapshot + snapshotsByID map[string]*cloud.Snapshot + snapshotsByName map[string][]*cloud.Snapshot } // New returns a new fake implementation of the @@ -39,24 +39,14 @@ func New() cloud.Interface { ZoneID: zoneID, } - snapshot := &cloud.Snapshot{ - ID: "9d076136-657b-4c84-b279-455da3ea484c", - Name: "pvc-vol-snap-1", - DomainID: "51f0fcb5-db16-4637-94f5-30131010214f", - ZoneID: zoneID, - VolumeID: "4f1f610d-6f17-4ff9-9228-e4062af93e54", - CreatedAt: "2025-07-07T16:13:06-0700", - } - - snapshotsByName := map[string]*cloud.Snapshot{ - snapshot.Name: snapshot, - } + snapshotsByID := make(map[string]*cloud.Snapshot) + snapshotsByName := make(map[string][]*cloud.Snapshot) return &fakeConnector{ node: node, - snapshot: snapshot, volumesByID: map[string]cloud.Volume{volume.ID: volume}, volumesByName: map[string]cloud.Volume{volume.Name: volume}, + snapshotsByID: snapshotsByID, snapshotsByName: snapshotsByName, } } @@ -162,41 +152,89 @@ func (f *fakeConnector) CreateVolumeFromSnapshot(_ context.Context, zoneID, name return vol, nil } -func (f *fakeConnector) GetSnapshotByID(_ context.Context, snapshotID string) (*cloud.Snapshot, error) { - if f.snapshot != nil && f.snapshot.ID == snapshotID { - return f.snapshot, nil +func (f *fakeConnector) CreateSnapshot(_ context.Context, volumeID, name string) (*cloud.Snapshot, error) { + if name == "" { + return nil, errors.New("invalid snapshot name: empty string") } - return nil, cloud.ErrNotFound -} - -func (f *fakeConnector) CreateSnapshot(_ context.Context, volumeID string) (*cloud.Snapshot, error) { - name := "pvc-vol-snap-1" // Always use the same name for test - if snap, ok := f.snapshotsByName[name]; ok && snap.VolumeID != volumeID { - return nil, errors.New("snapshot name conflict: already exists for a different source volume") + for _, snap := range f.snapshotsByName[name] { + if snap.VolumeID == volumeID { + // Allow multiple snapshots with the same name for the same volume + continue + } + // Name conflict: same name, different volume + return nil, cloud.ErrAlreadyExists } + id, _ := uuid.GenerateUUID() newSnap := &cloud.Snapshot{ - ID: "snap-" + volumeID, + ID: id, Name: name, DomainID: "fake-domain", ZoneID: zoneID, VolumeID: volumeID, CreatedAt: "2025-07-07T16:13:06-0700", } - f.snapshotsByName[name] = newSnap - f.snapshot = newSnap + f.snapshotsByID[newSnap.ID] = newSnap + f.snapshotsByName[name] = append(f.snapshotsByName[name], newSnap) return newSnap, nil } -func (f *fakeConnector) DeleteSnapshot(_ context.Context, _ string) error { - return nil +func (f *fakeConnector) GetSnapshotByID(_ context.Context, snapshotID string) (*cloud.Snapshot, error) { + snap, ok := f.snapshotsByID[snapshotID] + if ok { + return snap, nil + } + return nil, cloud.ErrNotFound } func (f *fakeConnector) GetSnapshotByName(_ context.Context, name string) (*cloud.Snapshot, error) { if name == "" { return nil, errors.New("invalid snapshot name: empty string") } - if snap, ok := f.snapshotsByName[name]; ok { - return snap, nil + snaps, ok := f.snapshotsByName[name] + if ok && len(snaps) > 0 { + return snaps[0], nil // Return the first for compatibility } return nil, cloud.ErrNotFound } + +// ListSnapshots returns all matching snapshots; pagination must be handled by the controller. +func (f *fakeConnector) ListSnapshots(_ context.Context, volumeID, snapshotID string) ([]*cloud.Snapshot, error) { + var result []*cloud.Snapshot + if snapshotID != "" { + if snap, ok := f.snapshotsByID[snapshotID]; ok { + result = append(result, snap) + } + return result, nil + } + if volumeID != "" { + for _, snap := range f.snapshotsByID { + if snap.VolumeID == volumeID { + result = append(result, snap) + } + } + return result, nil + } + for _, snap := range f.snapshotsByID { + result = append(result, snap) + } + return result, nil +} + +func (f *fakeConnector) DeleteSnapshot(_ context.Context, snapshotID string) error { + snap, ok := f.snapshotsByID[snapshotID] + if !ok { + return cloud.ErrNotFound + } + // Remove from snapshotsByID + delete(f.snapshotsByID, snapshotID) + // Remove from snapshotsByName + name := snap.Name + snaps := f.snapshotsByName[name] + for i, s := range snaps { + if s.ID == snapshotID { + f.snapshotsByName[name] = append(snaps[:i], snaps[i+1:]...) + break + } + } + return nil +} diff --git a/pkg/cloud/snapshots.go b/pkg/cloud/snapshots.go index 4909045..e84e601 100644 --- a/pkg/cloud/snapshots.go +++ b/pkg/cloud/snapshots.go @@ -45,12 +45,15 @@ func (c *client) GetSnapshotByID(ctx context.Context, snapshotID string) (*Snaps return &s, nil } -func (c *client) CreateSnapshot(ctx context.Context, volumeID string) (*Snapshot, error) { +func (c *client) CreateSnapshot(ctx context.Context, volumeID, name string) (*Snapshot, error) { logger := klog.FromContext(ctx) p := c.Snapshot.NewCreateSnapshotParams(volumeID) - + if name != "" { + p.SetName(name) + } logger.V(2).Info("CloudStack API call", "command", "CreateSnapshot", "params", map[string]string{ "volumeid": volumeID, + "name": name, }) snapshot, err := c.Snapshot.CreateSnapshot(p) @@ -118,3 +121,44 @@ func (c *client) GetSnapshotByName(ctx context.Context, name string) (*Snapshot, } return &s, nil } + +func (c *client) ListSnapshots(ctx context.Context, volumeID, snapshotID string) ([]*Snapshot, error) { + logger := klog.FromContext(ctx) + p := c.Snapshot.NewListSnapshotsParams() + if snapshotID != "" { + p.SetId(snapshotID) + } + if volumeID != "" { + p.SetVolumeid(volumeID) + } + if c.projectID != "" { + p.SetProjectid(c.projectID) + } + logger.V(2).Info("CloudStack API call", "command", "ListSnapshots", "params", map[string]string{ + "id": snapshotID, + "volumeid": volumeID, + "projectid": c.projectID, + }) + l, err := c.Snapshot.ListSnapshots(p) + if err != nil { + return nil, err + } + if l.Count == 0 { + return []*Snapshot{}, nil + } + var result []*Snapshot + for _, snapshot := range l.Snapshots { + s := &Snapshot{ + ID: snapshot.Id, + Name: snapshot.Name, + Size: snapshot.Virtualsize, + DomainID: snapshot.Domainid, + ProjectID: snapshot.Projectid, + ZoneID: snapshot.Zoneid, + VolumeID: snapshot.Volumeid, + CreatedAt: snapshot.Created, + } + result = append(result, s) + } + return result, nil +} diff --git a/pkg/driver/controller.go b/pkg/driver/controller.go index ac645d3..79ef37c 100644 --- a/pkg/driver/controller.go +++ b/pkg/driver/controller.go @@ -6,6 +6,7 @@ import ( "errors" "fmt" "math/rand" + "strconv" "time" "github.com/container-storage-interface/spec/lib/go/csi" @@ -338,15 +339,6 @@ func (cs *controllerServer) CreateSnapshot(ctx context.Context, req *csi.CreateS return nil, status.Error(codes.InvalidArgument, "SourceVolumeId missing in request") } - // Check for existing snapshot with same name but different source volume ID - if req.GetName() != "" { - // check if the name matches and volumeID differs - existingSnap, err := cs.connector.GetSnapshotByName(ctx, req.GetName()) - if err == nil && existingSnap.VolumeID != volumeID { - return nil, status.Errorf(codes.AlreadyExists, "Snapshot with name %s already exists for a different source volume", req.GetName()) - } - } - volume, err := cs.connector.GetVolumeByID(ctx, volumeID) if err != nil { if err.Error() == "invalid volume ID: empty string" { @@ -355,12 +347,13 @@ func (cs *controllerServer) CreateSnapshot(ctx context.Context, req *csi.CreateS if errors.Is(err, cloud.ErrNotFound) { return nil, status.Errorf(codes.NotFound, "Volume %v not found", volumeID) } - // Error with CloudStack return nil, status.Errorf(codes.Internal, "Error %v", err) } klog.V(4).Infof("CreateSnapshot of volume: %s", volume.ID) - snapshot, err := cs.connector.CreateSnapshot(ctx, volume.ID) - if err != nil { + snapshot, err := cs.connector.CreateSnapshot(ctx, volume.ID, req.GetName()) + if errors.Is(err, cloud.ErrAlreadyExists) { + return nil, status.Errorf(codes.AlreadyExists, "Snapshot name conflict: already exists for a different source volume") + } else if err != nil { return nil, status.Errorf(codes.Internal, "Failed to create snapshot for volume %s: %v", volume.ID, err.Error()) } @@ -369,7 +362,6 @@ func (cs *controllerServer) CreateSnapshot(ctx context.Context, req *csi.CreateS return nil, status.Errorf(codes.Internal, "Failed to parse snapshot creation time: %v", err) } - // Convert to Timestamp protobuf ts := timestamppb.New(t) resp := &csi.CreateSnapshotResponse{ @@ -378,37 +370,53 @@ func (cs *controllerServer) CreateSnapshot(ctx context.Context, req *csi.CreateS SourceVolumeId: volume.ID, CreationTime: ts, ReadyToUse: true, - // We leave the optional SizeBytes field unset as the size of a block storage snapshot is the size of the difference to the volume or previous snapshots, k8s however expects the size to be the size of the restored volume. }, } return resp, nil - } func (cs *controllerServer) ListSnapshots(ctx context.Context, req *csi.ListSnapshotsRequest) (*csi.ListSnapshotsResponse, error) { entries := []*csi.ListSnapshotsResponse_Entry{} - if req.GetSnapshotId() != "" { - snap, err := cs.connector.GetSnapshotByID(ctx, req.GetSnapshotId()) - if err == nil && snap != nil { - t, _ := time.Parse("2006-01-02T15:04:05-0700", snap.CreatedAt) - ts := timestamppb.New(t) - entry := &csi.ListSnapshotsResponse_Entry{ - Snapshot: &csi.Snapshot{ - SnapshotId: snap.ID, - SourceVolumeId: snap.VolumeID, - CreationTime: ts, - ReadyToUse: true, - }, - } - entries = append(entries, entry) - return &csi.ListSnapshotsResponse{Entries: entries}, nil - } - // If not found, return empty list - return &csi.ListSnapshotsResponse{Entries: []*csi.ListSnapshotsResponse_Entry{}}, nil + snapshots, err := cs.connector.ListSnapshots(ctx, req.GetSourceVolumeId(), req.GetSnapshotId()) + if err != nil { + return nil, status.Errorf(codes.Internal, "Failed to list snapshots: %v", err) } - return &csi.ListSnapshotsResponse{Entries: entries}, nil + // Pagination logic + start := 0 + if req.StartingToken != "" { + var err error + start, err = strconv.Atoi(req.StartingToken) + if err != nil || start < 0 || start > len(snapshots) { + return nil, status.Error(codes.Aborted, "Invalid startingToken") + } + } + maxEntries := int(req.MaxEntries) + end := len(snapshots) + if maxEntries > 0 && start+maxEntries < end { + end = start + maxEntries + } + nextToken := "" + if end < len(snapshots) { + nextToken = strconv.Itoa(end) + } + + for i := start; i < end; i++ { + snap := snapshots[i] + t, _ := time.Parse("2006-01-02T15:04:05-0700", snap.CreatedAt) + ts := timestamppb.New(t) + entry := &csi.ListSnapshotsResponse_Entry{ + Snapshot: &csi.Snapshot{ + SnapshotId: snap.ID, + SourceVolumeId: snap.VolumeID, + CreationTime: ts, + ReadyToUse: true, + }, + } + entries = append(entries, entry) + } + return &csi.ListSnapshotsResponse{Entries: entries, NextToken: nextToken}, nil } func (cs *controllerServer) DeleteSnapshot(ctx context.Context, req *csi.DeleteSnapshotRequest) (*csi.DeleteSnapshotResponse, error) { @@ -420,19 +428,14 @@ func (cs *controllerServer) DeleteSnapshot(ctx context.Context, req *csi.DeleteS klog.V(4).Infof("DeleteSnapshot for snapshotID: %s", snapshotID) - snapshot, err := cs.connector.GetSnapshotByID(ctx, snapshotID) + err := cs.connector.DeleteSnapshot(ctx, snapshotID) if errors.Is(err, cloud.ErrNotFound) { - return nil, status.Errorf(codes.NotFound, "Snapshot %v not found", snapshotID) + // Per CSI spec, return OK if snapshot does not exist + return &csi.DeleteSnapshotResponse{}, nil } else if err != nil { - // Error with CloudStack return nil, status.Errorf(codes.Internal, "Error %v", err) } - err = cs.connector.DeleteSnapshot(ctx, snapshot.ID) - if err != nil && !errors.Is(err, cloud.ErrNotFound) { - return nil, status.Errorf(codes.Internal, "Cannot delete snapshot %s: %s", snapshotID, err.Error()) - } - return &csi.DeleteSnapshotResponse{}, nil } From b0c66ebee3aab341d6f827e3d43a71b35160927d Mon Sep 17 00:00:00 2001 From: Pearl Dsilva Date: Thu, 2 Oct 2025 13:43:16 -0400 Subject: [PATCH 14/17] fix lint --- pkg/cloud/fake/fake.go | 2 +- pkg/mount/mount.go | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/pkg/cloud/fake/fake.go b/pkg/cloud/fake/fake.go index 079b0ec..005010d 100644 --- a/pkg/cloud/fake/fake.go +++ b/pkg/cloud/fake/fake.go @@ -139,7 +139,7 @@ func (f *fakeConnector) ExpandVolume(_ context.Context, volumeID string, newSize return cloud.ErrNotFound } -func (f *fakeConnector) CreateVolumeFromSnapshot(_ context.Context, zoneID, name, projectID, snapshotID string, sizeInGB int64) (*cloud.Volume, error) { +func (f *fakeConnector) CreateVolumeFromSnapshot(_ context.Context, zoneID, name, _, snapshotID string, sizeInGB int64) (*cloud.Volume, error) { vol := &cloud.Volume{ ID: "fake-vol-from-snap-" + name, Name: name, diff --git a/pkg/mount/mount.go b/pkg/mount/mount.go index 7e2ca6e..056f4a1 100644 --- a/pkg/mount/mount.go +++ b/pkg/mount/mount.go @@ -182,11 +182,13 @@ func (m *mounter) getDevicePathForVMware(ctx context.Context, volumeID string) ( if err == nil && isBlock { if m.verifyDevice(ctx, devicePath, volumeID) { logger.V(4).Info("Found and verified VMware device", "devicePath", devicePath, "volumeID", volumeID) + return devicePath, nil } } } } + return "", fmt.Errorf("device not found for volume %s", volumeID) } @@ -196,6 +198,7 @@ func (m *mounter) verifyDevice(ctx context.Context, devicePath string, volumeID size, err := m.GetBlockSizeBytes(devicePath) if err != nil { logger.V(4).Info("Failed to get device size", "devicePath", devicePath, "volumeID", volumeID, "error", err) + return false } logger.V(5).Info("Device size retrieved", "devicePath", devicePath, "volumeID", volumeID, "sizeBytes", size) @@ -203,16 +206,19 @@ func (m *mounter) verifyDevice(ctx context.Context, devicePath string, volumeID mounted, err := m.isDeviceMounted(devicePath) if err != nil { logger.V(4).Info("Failed to check if device is mounted", "devicePath", devicePath, "volumeID", volumeID, "error", err) + return false } if mounted { logger.V(4).Info("Device is already mounted", "devicePath", devicePath, "volumeID", volumeID) + return false } props, err := m.getDeviceProperties(devicePath) if err != nil { logger.V(4).Info("Failed to get device properties", "devicePath", devicePath, "volumeID", volumeID, "error", err) + return false } logger.V(5).Info("Device properties retrieved", "devicePath", devicePath, "volumeID", volumeID, "properties", props) @@ -226,8 +232,10 @@ func (m *mounter) isDeviceMounted(devicePath string) (bool, error) { if strings.Contains(err.Error(), "exit status 1") { return false, nil } + return false, err } + return len(output) > 0, nil } From 3a06c70bf62b51471c65642e2bc281d4264e899c Mon Sep 17 00:00:00 2001 From: Pearl Dsilva Date: Thu, 2 Oct 2025 13:54:54 -0400 Subject: [PATCH 15/17] fix lint --- pkg/cloud/cloud.go | 2 ++ pkg/cloud/fake/fake.go | 15 ++++++++++++--- pkg/cloud/snapshots.go | 2 ++ pkg/cloud/vms.go | 1 + pkg/driver/controller.go | 12 +++++++++--- pkg/mount/mount.go | 4 ++++ 6 files changed, 30 insertions(+), 6 deletions(-) diff --git a/pkg/cloud/cloud.go b/pkg/cloud/cloud.go index 7c1939e..98e8b6b 100644 --- a/pkg/cloud/cloud.go +++ b/pkg/cloud/cloud.go @@ -10,6 +10,8 @@ import ( ) // Interface is the CloudStack client interface. +// +//revive:disable:interfacebloat type Interface interface { GetNodeInfo(ctx context.Context, vmName string) (*VM, error) GetVMByID(ctx context.Context, vmID string) (*VM, error) diff --git a/pkg/cloud/fake/fake.go b/pkg/cloud/fake/fake.go index 005010d..93ad83a 100644 --- a/pkg/cloud/fake/fake.go +++ b/pkg/cloud/fake/fake.go @@ -139,7 +139,7 @@ func (f *fakeConnector) ExpandVolume(_ context.Context, volumeID string, newSize return cloud.ErrNotFound } -func (f *fakeConnector) CreateVolumeFromSnapshot(_ context.Context, zoneID, name, _, snapshotID string, sizeInGB int64) (*cloud.Volume, error) { +func (f *fakeConnector) CreateVolumeFromSnapshot(_ context.Context, zoneID, name, _, _ string, sizeInGB int64) (*cloud.Volume, error) { vol := &cloud.Volume{ ID: "fake-vol-from-snap-" + name, Name: name, @@ -149,6 +149,7 @@ func (f *fakeConnector) CreateVolumeFromSnapshot(_ context.Context, zoneID, name } f.volumesByID[vol.ID] = *vol f.volumesByName[vol.Name] = *vol + return vol, nil } @@ -161,6 +162,7 @@ func (f *fakeConnector) CreateSnapshot(_ context.Context, volumeID, name string) // Allow multiple snapshots with the same name for the same volume continue } + // Name conflict: same name, different volume return nil, cloud.ErrAlreadyExists } @@ -175,6 +177,7 @@ func (f *fakeConnector) CreateSnapshot(_ context.Context, volumeID, name string) } f.snapshotsByID[newSnap.ID] = newSnap f.snapshotsByName[name] = append(f.snapshotsByName[name], newSnap) + return newSnap, nil } @@ -183,6 +186,7 @@ func (f *fakeConnector) GetSnapshotByID(_ context.Context, snapshotID string) (* if ok { return snap, nil } + return nil, cloud.ErrNotFound } @@ -194,6 +198,7 @@ func (f *fakeConnector) GetSnapshotByName(_ context.Context, name string) (*clou if ok && len(snaps) > 0 { return snaps[0], nil // Return the first for compatibility } + return nil, cloud.ErrNotFound } @@ -204,6 +209,7 @@ func (f *fakeConnector) ListSnapshots(_ context.Context, volumeID, snapshotID st if snap, ok := f.snapshotsByID[snapshotID]; ok { result = append(result, snap) } + return result, nil } if volumeID != "" { @@ -212,11 +218,13 @@ func (f *fakeConnector) ListSnapshots(_ context.Context, volumeID, snapshotID st result = append(result, snap) } } + return result, nil } for _, snap := range f.snapshotsByID { result = append(result, snap) } + return result, nil } @@ -225,9 +233,9 @@ func (f *fakeConnector) DeleteSnapshot(_ context.Context, snapshotID string) err if !ok { return cloud.ErrNotFound } - // Remove from snapshotsByID + delete(f.snapshotsByID, snapshotID) - // Remove from snapshotsByName + name := snap.Name snaps := f.snapshotsByName[name] for i, s := range snaps { @@ -236,5 +244,6 @@ func (f *fakeConnector) DeleteSnapshot(_ context.Context, snapshotID string) err break } } + return nil } diff --git a/pkg/cloud/snapshots.go b/pkg/cloud/snapshots.go index e84e601..757f34f 100644 --- a/pkg/cloud/snapshots.go +++ b/pkg/cloud/snapshots.go @@ -71,6 +71,7 @@ func (c *client) CreateSnapshot(ctx context.Context, volumeID, name string) (*Sn VolumeID: snapshot.Volumeid, CreatedAt: snapshot.Created, } + return &snap, nil } @@ -160,5 +161,6 @@ func (c *client) ListSnapshots(ctx context.Context, volumeID, snapshotID string) } result = append(result, s) } + return result, nil } diff --git a/pkg/cloud/vms.go b/pkg/cloud/vms.go index 2e98f64..354f3a2 100644 --- a/pkg/cloud/vms.go +++ b/pkg/cloud/vms.go @@ -29,6 +29,7 @@ func (c *client) GetVMByID(ctx context.Context, vmID string) (*VM, error) { } vm := l.VirtualMachines[0] logger.V(2).Info("Returning VM", "vmID", vm.Id, "zoneID", vm.Zoneid) + return &VM{ ID: vm.Id, ZoneID: vm.Zoneid, diff --git a/pkg/driver/controller.go b/pkg/driver/controller.go index 79ef37c..8617835 100644 --- a/pkg/driver/controller.go +++ b/pkg/driver/controller.go @@ -166,6 +166,7 @@ func (cs *controllerServer) CreateVolume(ctx context.Context, req *csi.CreateVol }, }, } + return resp, nil } @@ -226,6 +227,7 @@ func printVolumeAsJSON(vol *csi.CreateVolumeRequest) { b, err := json.MarshalIndent(vol, "", " ") if err != nil { klog.Errorf("Failed to marshal CreateVolumeRequest to JSON: %v", err) + return } klog.V(5).Infof("CreateVolumeRequest as JSON:\n%s", string(b)) @@ -347,8 +349,10 @@ func (cs *controllerServer) CreateSnapshot(ctx context.Context, req *csi.CreateS if errors.Is(err, cloud.ErrNotFound) { return nil, status.Errorf(codes.NotFound, "Volume %v not found", volumeID) } + return nil, status.Errorf(codes.Internal, "Error %v", err) } + klog.V(4).Infof("CreateSnapshot of volume: %s", volume.ID) snapshot, err := cs.connector.CreateSnapshot(ctx, volume.ID, req.GetName()) if errors.Is(err, cloud.ErrAlreadyExists) { @@ -372,6 +376,7 @@ func (cs *controllerServer) CreateSnapshot(ctx context.Context, req *csi.CreateS ReadyToUse: true, }, } + return resp, nil } @@ -385,14 +390,14 @@ func (cs *controllerServer) ListSnapshots(ctx context.Context, req *csi.ListSnap // Pagination logic start := 0 - if req.StartingToken != "" { + if req.GetStartingToken() != "" { var err error - start, err = strconv.Atoi(req.StartingToken) + start, err = strconv.Atoi(req.GetStartingToken()) if err != nil || start < 0 || start > len(snapshots) { return nil, status.Error(codes.Aborted, "Invalid startingToken") } } - maxEntries := int(req.MaxEntries) + maxEntries := int(req.GetMaxEntries()) end := len(snapshots) if maxEntries > 0 && start+maxEntries < end { end = start + maxEntries @@ -416,6 +421,7 @@ func (cs *controllerServer) ListSnapshots(ctx context.Context, req *csi.ListSnap } entries = append(entries, entry) } + return &csi.ListSnapshotsResponse{Entries: entries, NextToken: nextToken}, nil } diff --git a/pkg/mount/mount.go b/pkg/mount/mount.go index 056f4a1..827e73c 100644 --- a/pkg/mount/mount.go +++ b/pkg/mount/mount.go @@ -95,6 +95,7 @@ func (m *mounter) GetDevicePath(ctx context.Context, volumeID string) (string, e if path != "" { devicePath = path logger.V(4).Info("Device path found", "volumeID", volumeID, "devicePath", path) + return true, nil } m.probeVolume(ctx) @@ -142,6 +143,7 @@ func (m *mounter) getDevicePathBySerialID(ctx context.Context, volumeID string) } if !os.IsNotExist(err) { logger.Error(err, "Failed to stat device path", "path", source) + return "", err } } @@ -161,11 +163,13 @@ func (m *mounter) getDevicePathForXenServer(ctx context.Context, volumeID string if err == nil && isBlock { if m.verifyDevice(ctx, devicePath, volumeID) { logger.V(4).Info("Found and verified XenServer device", "devicePath", devicePath, "volumeID", volumeID) + return devicePath, nil } } } } + return "", fmt.Errorf("device not found for volume %s", volumeID) } From a8e45cc211e393c54478059baf596805367719b3 Mon Sep 17 00:00:00 2001 From: Pearl Dsilva Date: Thu, 2 Oct 2025 14:01:03 -0400 Subject: [PATCH 16/17] fix lint --- pkg/cloud/cloud.go | 4 ++-- pkg/cloud/fake/fake.go | 13 ++++++++++--- pkg/cloud/snapshots.go | 3 ++- 3 files changed, 14 insertions(+), 6 deletions(-) diff --git a/pkg/cloud/cloud.go b/pkg/cloud/cloud.go index 98e8b6b..b683f88 100644 --- a/pkg/cloud/cloud.go +++ b/pkg/cloud/cloud.go @@ -10,8 +10,8 @@ import ( ) // Interface is the CloudStack client interface. -// -//revive:disable:interfacebloat + +//nolint:interfacebloat type Interface interface { GetNodeInfo(ctx context.Context, vmName string) (*VM, error) GetVMByID(ctx context.Context, vmID string) (*VM, error) diff --git a/pkg/cloud/fake/fake.go b/pkg/cloud/fake/fake.go index 93ad83a..4870525 100644 --- a/pkg/cloud/fake/fake.go +++ b/pkg/cloud/fake/fake.go @@ -204,23 +204,29 @@ func (f *fakeConnector) GetSnapshotByName(_ context.Context, name string) (*clou // ListSnapshots returns all matching snapshots; pagination must be handled by the controller. func (f *fakeConnector) ListSnapshots(_ context.Context, volumeID, snapshotID string) ([]*cloud.Snapshot, error) { - var result []*cloud.Snapshot if snapshotID != "" { + result := make([]*cloud.Snapshot, 0, 1) if snap, ok := f.snapshotsByID[snapshotID]; ok { result = append(result, snap) } - return result, nil } if volumeID != "" { + count := 0 + for _, snap := range f.snapshotsByID { + if snap.VolumeID == volumeID { + count++ + } + } + result := make([]*cloud.Snapshot, 0, count) for _, snap := range f.snapshotsByID { if snap.VolumeID == volumeID { result = append(result, snap) } } - return result, nil } + result := make([]*cloud.Snapshot, 0, len(f.snapshotsByID)) for _, snap := range f.snapshotsByID { result = append(result, snap) } @@ -241,6 +247,7 @@ func (f *fakeConnector) DeleteSnapshot(_ context.Context, snapshotID string) err for i, s := range snaps { if s.ID == snapshotID { f.snapshotsByName[name] = append(snaps[:i], snaps[i+1:]...) + break } } diff --git a/pkg/cloud/snapshots.go b/pkg/cloud/snapshots.go index 757f34f..d501021 100644 --- a/pkg/cloud/snapshots.go +++ b/pkg/cloud/snapshots.go @@ -120,6 +120,7 @@ func (c *client) GetSnapshotByName(ctx context.Context, name string) (*Snapshot, VolumeID: snapshot.Volumeid, CreatedAt: snapshot.Created, } + return &s, nil } @@ -147,7 +148,7 @@ func (c *client) ListSnapshots(ctx context.Context, volumeID, snapshotID string) if l.Count == 0 { return []*Snapshot{}, nil } - var result []*Snapshot + result := make([]*Snapshot, 0, l.Count) for _, snapshot := range l.Snapshots { s := &Snapshot{ ID: snapshot.Id, From 027f805d5c97200ecac00cd9b1690a3f7621b208 Mon Sep 17 00:00:00 2001 From: Pearl Dsilva Date: Thu, 2 Oct 2025 14:09:07 -0400 Subject: [PATCH 17/17] lint failure fix --- pkg/cloud/fake/fake.go | 2 ++ pkg/driver/controller.go | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/pkg/cloud/fake/fake.go b/pkg/cloud/fake/fake.go index 4870525..781c12a 100644 --- a/pkg/cloud/fake/fake.go +++ b/pkg/cloud/fake/fake.go @@ -209,6 +209,7 @@ func (f *fakeConnector) ListSnapshots(_ context.Context, volumeID, snapshotID st if snap, ok := f.snapshotsByID[snapshotID]; ok { result = append(result, snap) } + return result, nil } if volumeID != "" { @@ -224,6 +225,7 @@ func (f *fakeConnector) ListSnapshots(_ context.Context, volumeID, snapshotID st result = append(result, snap) } } + return result, nil } result := make([]*cloud.Snapshot, 0, len(f.snapshotsByID)) diff --git a/pkg/driver/controller.go b/pkg/driver/controller.go index 8617835..f487010 100644 --- a/pkg/driver/controller.go +++ b/pkg/driver/controller.go @@ -737,14 +737,14 @@ func (cs *controllerServer) ControllerGetCapabilities(ctx context.Context, req * }, }, }, - &csi.ControllerServiceCapability{ + { Type: &csi.ControllerServiceCapability_Rpc{ Rpc: &csi.ControllerServiceCapability_RPC{ Type: csi.ControllerServiceCapability_RPC_CREATE_DELETE_SNAPSHOT, }, }, }, - &csi.ControllerServiceCapability{ + { Type: &csi.ControllerServiceCapability_Rpc{ Rpc: &csi.ControllerServiceCapability_RPC{ Type: csi.ControllerServiceCapability_RPC_LIST_SNAPSHOTS,