Skip to content

Commit 4e3f9eb

Browse files
Pearl1594Pearl1594
committed
fix sanity tests
1 parent 1973b8b commit 4e3f9eb

File tree

4 files changed

+164
-77
lines changed

4 files changed

+164
-77
lines changed

‎pkg/cloud/cloud.go‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,9 @@ type Interface interface {
2727
CreateVolumeFromSnapshot(ctx context.Context, zoneID, name, projectID, snapshotID string, sizeInGB int64) (*Volume, error)
2828
GetSnapshotByID(ctx context.Context, snapshotID string) (*Snapshot, error)
2929
GetSnapshotByName(ctx context.Context, name string) (*Snapshot, error)
30-
CreateSnapshot(ctx context.Context, volumeID string) (*Snapshot, error)
30+
CreateSnapshot(ctx context.Context, volumeID, name string) (*Snapshot, error)
3131
DeleteSnapshot(ctx context.Context, snapshotID string) error
32+
ListSnapshots(ctx context.Context, volumeID, snapshotID string) ([]*Snapshot, error)
3233
}
3334

3435
// Volume represents a CloudStack volume.
@@ -71,6 +72,7 @@ type VM struct {
7172
var (
7273
ErrNotFound = errors.New("not found")
7374
ErrTooManyResults = errors.New("too many results")
75+
ErrAlreadyExists = errors.New("already exists")
7476
)
7577

7678
// client is the implementation of Interface.

‎pkg/cloud/fake/fake.go‎

Lines changed: 70 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,10 @@ const zoneID = "a1887604-237c-4212-a9cd-94620b7880fa"
1616

1717
type fakeConnector struct {
1818
node *cloud.VM
19-
snapshot *cloud.Snapshot
2019
volumesByID map[string]cloud.Volume
2120
volumesByName map[string]cloud.Volume
22-
snapshotsByName map[string]*cloud.Snapshot
21+
snapshotsByID map[string]*cloud.Snapshot
22+
snapshotsByName map[string][]*cloud.Snapshot
2323
}
2424

2525
// New returns a new fake implementation of the
@@ -39,24 +39,14 @@ func New() cloud.Interface {
3939
ZoneID: zoneID,
4040
}
4141

42-
snapshot := &cloud.Snapshot{
43-
ID: "9d076136-657b-4c84-b279-455da3ea484c",
44-
Name: "pvc-vol-snap-1",
45-
DomainID: "51f0fcb5-db16-4637-94f5-30131010214f",
46-
ZoneID: zoneID,
47-
VolumeID: "4f1f610d-6f17-4ff9-9228-e4062af93e54",
48-
CreatedAt: "2025-07-07T16:13:06-0700",
49-
}
50-
51-
snapshotsByName := map[string]*cloud.Snapshot{
52-
snapshot.Name: snapshot,
53-
}
42+
snapshotsByID := make(map[string]*cloud.Snapshot)
43+
snapshotsByName := make(map[string][]*cloud.Snapshot)
5444

5545
return &fakeConnector{
5646
node: node,
57-
snapshot: snapshot,
5847
volumesByID: map[string]cloud.Volume{volume.ID: volume},
5948
volumesByName: map[string]cloud.Volume{volume.Name: volume},
49+
snapshotsByID: snapshotsByID,
6050
snapshotsByName: snapshotsByName,
6151
}
6252
}
@@ -162,41 +152,89 @@ func (f *fakeConnector) CreateVolumeFromSnapshot(_ context.Context, zoneID, name
162152
return vol, nil
163153
}
164154

165-
func (f *fakeConnector) GetSnapshotByID(_ context.Context, snapshotID string) (*cloud.Snapshot, error) {
166-
if f.snapshot != nil && f.snapshot.ID == snapshotID {
167-
return f.snapshot, nil
155+
func (f *fakeConnector) CreateSnapshot(_ context.Context, volumeID, name string) (*cloud.Snapshot, error) {
156+
if name == "" {
157+
return nil, errors.New("invalid snapshot name: empty string")
168158
}
169-
return nil, cloud.ErrNotFound
170-
}
171-
172-
func (f *fakeConnector) CreateSnapshot(_ context.Context, volumeID string) (*cloud.Snapshot, error) {
173-
name := "pvc-vol-snap-1" // Always use the same name for test
174-
if snap, ok := f.snapshotsByName[name]; ok && snap.VolumeID != volumeID {
175-
return nil, errors.New("snapshot name conflict: already exists for a different source volume")
159+
for _, snap := range f.snapshotsByName[name] {
160+
if snap.VolumeID == volumeID {
161+
// Allow multiple snapshots with the same name for the same volume
162+
continue
163+
}
164+
// Name conflict: same name, different volume
165+
return nil, cloud.ErrAlreadyExists
176166
}
167+
id, _ := uuid.GenerateUUID()
177168
newSnap := &cloud.Snapshot{
178-
ID: "snap-" + volumeID,
169+
ID: id,
179170
Name: name,
180171
DomainID: "fake-domain",
181172
ZoneID: zoneID,
182173
VolumeID: volumeID,
183174
CreatedAt: "2025-07-07T16:13:06-0700",
184175
}
185-
f.snapshotsByName[name] = newSnap
186-
f.snapshot = newSnap
176+
f.snapshotsByID[newSnap.ID] = newSnap
177+
f.snapshotsByName[name] = append(f.snapshotsByName[name], newSnap)
187178
return newSnap, nil
188179
}
189180

190-
func (f *fakeConnector) DeleteSnapshot(_ context.Context, _ string) error {
191-
return nil
181+
func (f *fakeConnector) GetSnapshotByID(_ context.Context, snapshotID string) (*cloud.Snapshot, error) {
182+
snap, ok := f.snapshotsByID[snapshotID]
183+
if ok {
184+
return snap, nil
185+
}
186+
return nil, cloud.ErrNotFound
192187
}
193188

194189
func (f *fakeConnector) GetSnapshotByName(_ context.Context, name string) (*cloud.Snapshot, error) {
195190
if name == "" {
196191
return nil, errors.New("invalid snapshot name: empty string")
197192
}
198-
if snap, ok := f.snapshotsByName[name]; ok {
199-
return snap, nil
193+
snaps, ok := f.snapshotsByName[name]
194+
if ok && len(snaps) > 0 {
195+
return snaps[0], nil // Return the first for compatibility
200196
}
201197
return nil, cloud.ErrNotFound
202198
}
199+
200+
// ListSnapshots returns all matching snapshots; pagination must be handled by the controller.
201+
func (f *fakeConnector) ListSnapshots(_ context.Context, volumeID, snapshotID string) ([]*cloud.Snapshot, error) {
202+
var result []*cloud.Snapshot
203+
if snapshotID != "" {
204+
if snap, ok := f.snapshotsByID[snapshotID]; ok {
205+
result = append(result, snap)
206+
}
207+
return result, nil
208+
}
209+
if volumeID != "" {
210+
for _, snap := range f.snapshotsByID {
211+
if snap.VolumeID == volumeID {
212+
result = append(result, snap)
213+
}
214+
}
215+
return result, nil
216+
}
217+
for _, snap := range f.snapshotsByID {
218+
result = append(result, snap)
219+
}
220+
return result, nil
221+
}
222+
223+
func (f *fakeConnector) DeleteSnapshot(_ context.Context, snapshotID string) error {
224+
snap, ok := f.snapshotsByID[snapshotID]
225+
if !ok {
226+
return cloud.ErrNotFound
227+
}
228+
// Remove from snapshotsByID
229+
delete(f.snapshotsByID, snapshotID)
230+
// Remove from snapshotsByName
231+
name := snap.Name
232+
snaps := f.snapshotsByName[name]
233+
for i, s := range snaps {
234+
if s.ID == snapshotID {
235+
f.snapshotsByName[name] = append(snaps[:i], snaps[i+1:]...)
236+
break
237+
}
238+
}
239+
return nil
240+
}

‎pkg/cloud/snapshots.go‎

Lines changed: 46 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,12 +45,15 @@ func (c *client) GetSnapshotByID(ctx context.Context, snapshotID string) (*Snaps
4545
return &s, nil
4646
}
4747

48-
func (c *client) CreateSnapshot(ctx context.Context, volumeID string) (*Snapshot, error) {
48+
func (c *client) CreateSnapshot(ctx context.Context, volumeID, name string) (*Snapshot, error) {
4949
logger := klog.FromContext(ctx)
5050
p := c.Snapshot.NewCreateSnapshotParams(volumeID)
51-
51+
if name != "" {
52+
p.SetName(name)
53+
}
5254
logger.V(2).Info("CloudStack API call", "command", "CreateSnapshot", "params", map[string]string{
5355
"volumeid": volumeID,
56+
"name": name,
5457
})
5558

5659
snapshot, err := c.Snapshot.CreateSnapshot(p)
@@ -118,3 +121,44 @@ func (c *client) GetSnapshotByName(ctx context.Context, name string) (*Snapshot,
118121
}
119122
return &s, nil
120123
}
124+
125+
func (c *client) ListSnapshots(ctx context.Context, volumeID, snapshotID string) ([]*Snapshot, error) {
126+
logger := klog.FromContext(ctx)
127+
p := c.Snapshot.NewListSnapshotsParams()
128+
if snapshotID != "" {
129+
p.SetId(snapshotID)
130+
}
131+
if volumeID != "" {
132+
p.SetVolumeid(volumeID)
133+
}
134+
if c.projectID != "" {
135+
p.SetProjectid(c.projectID)
136+
}
137+
logger.V(2).Info("CloudStack API call", "command", "ListSnapshots", "params", map[string]string{
138+
"id": snapshotID,
139+
"volumeid": volumeID,
140+
"projectid": c.projectID,
141+
})
142+
l, err := c.Snapshot.ListSnapshots(p)
143+
if err != nil {
144+
return nil, err
145+
}
146+
if l.Count == 0 {
147+
return []*Snapshot{}, nil
148+
}
149+
var result []*Snapshot
150+
for _, snapshot := range l.Snapshots {
151+
s := &Snapshot{
152+
ID: snapshot.Id,
153+
Name: snapshot.Name,
154+
Size: snapshot.Virtualsize,
155+
DomainID: snapshot.Domainid,
156+
ProjectID: snapshot.Projectid,
157+
ZoneID: snapshot.Zoneid,
158+
VolumeID: snapshot.Volumeid,
159+
CreatedAt: snapshot.Created,
160+
}
161+
result = append(result, s)
162+
}
163+
return result, nil
164+
}

‎pkg/driver/controller.go‎

Lines changed: 45 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"errors"
77
"fmt"
88
"math/rand"
9+
"strconv"
910
"time"
1011

1112
"github.com/container-storage-interface/spec/lib/go/csi"
@@ -338,15 +339,6 @@ func (cs *controllerServer) CreateSnapshot(ctx context.Context, req *csi.CreateS
338339
return nil, status.Error(codes.InvalidArgument, "SourceVolumeId missing in request")
339340
}
340341

341-
// Check for existing snapshot with same name but different source volume ID
342-
if req.GetName() != "" {
343-
// check if the name matches and volumeID differs
344-
existingSnap, err := cs.connector.GetSnapshotByName(ctx, req.GetName())
345-
if err == nil && existingSnap.VolumeID != volumeID {
346-
return nil, status.Errorf(codes.AlreadyExists, "Snapshot with name %s already exists for a different source volume", req.GetName())
347-
}
348-
}
349-
350342
volume, err := cs.connector.GetVolumeByID(ctx, volumeID)
351343
if err != nil {
352344
if err.Error() == "invalid volume ID: empty string" {
@@ -355,12 +347,13 @@ func (cs *controllerServer) CreateSnapshot(ctx context.Context, req *csi.CreateS
355347
if errors.Is(err, cloud.ErrNotFound) {
356348
return nil, status.Errorf(codes.NotFound, "Volume %v not found", volumeID)
357349
}
358-
// Error with CloudStack
359350
return nil, status.Errorf(codes.Internal, "Error %v", err)
360351
}
361352
klog.V(4).Infof("CreateSnapshot of volume: %s", volume.ID)
362-
snapshot, err := cs.connector.CreateSnapshot(ctx, volume.ID)
363-
if err != nil {
353+
snapshot, err := cs.connector.CreateSnapshot(ctx, volume.ID, req.GetName())
354+
if errors.Is(err, cloud.ErrAlreadyExists) {
355+
return nil, status.Errorf(codes.AlreadyExists, "Snapshot name conflict: already exists for a different source volume")
356+
} else if err != nil {
364357
return nil, status.Errorf(codes.Internal, "Failed to create snapshot for volume %s: %v", volume.ID, err.Error())
365358
}
366359

@@ -369,7 +362,6 @@ func (cs *controllerServer) CreateSnapshot(ctx context.Context, req *csi.CreateS
369362
return nil, status.Errorf(codes.Internal, "Failed to parse snapshot creation time: %v", err)
370363
}
371364

372-
// Convert to Timestamp protobuf
373365
ts := timestamppb.New(t)
374366

375367
resp := &csi.CreateSnapshotResponse{
@@ -378,37 +370,53 @@ func (cs *controllerServer) CreateSnapshot(ctx context.Context, req *csi.CreateS
378370
SourceVolumeId: volume.ID,
379371
CreationTime: ts,
380372
ReadyToUse: true,
381-
// 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.
382373
},
383374
}
384375
return resp, nil
385-
386376
}
387377

388378
func (cs *controllerServer) ListSnapshots(ctx context.Context, req *csi.ListSnapshotsRequest) (*csi.ListSnapshotsResponse, error) {
389379
entries := []*csi.ListSnapshotsResponse_Entry{}
390380

391-
if req.GetSnapshotId() != "" {
392-
snap, err := cs.connector.GetSnapshotByID(ctx, req.GetSnapshotId())
393-
if err == nil && snap != nil {
394-
t, _ := time.Parse("2006-01-02T15:04:05-0700", snap.CreatedAt)
395-
ts := timestamppb.New(t)
396-
entry := &csi.ListSnapshotsResponse_Entry{
397-
Snapshot: &csi.Snapshot{
398-
SnapshotId: snap.ID,
399-
SourceVolumeId: snap.VolumeID,
400-
CreationTime: ts,
401-
ReadyToUse: true,
402-
},
403-
}
404-
entries = append(entries, entry)
405-
return &csi.ListSnapshotsResponse{Entries: entries}, nil
406-
}
407-
// If not found, return empty list
408-
return &csi.ListSnapshotsResponse{Entries: []*csi.ListSnapshotsResponse_Entry{}}, nil
381+
snapshots, err := cs.connector.ListSnapshots(ctx, req.GetSourceVolumeId(), req.GetSnapshotId())
382+
if err != nil {
383+
return nil, status.Errorf(codes.Internal, "Failed to list snapshots: %v", err)
409384
}
410385

411-
return &csi.ListSnapshotsResponse{Entries: entries}, nil
386+
// Pagination logic
387+
start := 0
388+
if req.StartingToken != "" {
389+
var err error
390+
start, err = strconv.Atoi(req.StartingToken)
391+
if err != nil || start < 0 || start > len(snapshots) {
392+
return nil, status.Error(codes.Aborted, "Invalid startingToken")
393+
}
394+
}
395+
maxEntries := int(req.MaxEntries)
396+
end := len(snapshots)
397+
if maxEntries > 0 && start+maxEntries < end {
398+
end = start + maxEntries
399+
}
400+
nextToken := ""
401+
if end < len(snapshots) {
402+
nextToken = strconv.Itoa(end)
403+
}
404+
405+
for i := start; i < end; i++ {
406+
snap := snapshots[i]
407+
t, _ := time.Parse("2006-01-02T15:04:05-0700", snap.CreatedAt)
408+
ts := timestamppb.New(t)
409+
entry := &csi.ListSnapshotsResponse_Entry{
410+
Snapshot: &csi.Snapshot{
411+
SnapshotId: snap.ID,
412+
SourceVolumeId: snap.VolumeID,
413+
CreationTime: ts,
414+
ReadyToUse: true,
415+
},
416+
}
417+
entries = append(entries, entry)
418+
}
419+
return &csi.ListSnapshotsResponse{Entries: entries, NextToken: nextToken}, nil
412420
}
413421

414422
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
420428

421429
klog.V(4).Infof("DeleteSnapshot for snapshotID: %s", snapshotID)
422430

423-
snapshot, err := cs.connector.GetSnapshotByID(ctx, snapshotID)
431+
err := cs.connector.DeleteSnapshot(ctx, snapshotID)
424432
if errors.Is(err, cloud.ErrNotFound) {
425-
return nil, status.Errorf(codes.NotFound, "Snapshot %v not found", snapshotID)
433+
// Per CSI spec, return OK if snapshot does not exist
434+
return &csi.DeleteSnapshotResponse{}, nil
426435
} else if err != nil {
427-
// Error with CloudStack
428436
return nil, status.Errorf(codes.Internal, "Error %v", err)
429437
}
430438

431-
err = cs.connector.DeleteSnapshot(ctx, snapshot.ID)
432-
if err != nil && !errors.Is(err, cloud.ErrNotFound) {
433-
return nil, status.Errorf(codes.Internal, "Cannot delete snapshot %s: %s", snapshotID, err.Error())
434-
}
435-
436439
return &csi.DeleteSnapshotResponse{}, nil
437440
}
438441

0 commit comments

Comments
 (0)