diff --git a/pkg/csi/blockstorage/controllerserver.go b/pkg/csi/blockstorage/controllerserver.go index 02f8c1fb..ee852660 100644 --- a/pkg/csi/blockstorage/controllerserver.go +++ b/pkg/csi/blockstorage/controllerserver.go @@ -20,6 +20,7 @@ import ( "context" "errors" "fmt" + "sort" "strconv" "time" @@ -769,76 +770,107 @@ func (cs *controllerServer) ListSnapshots(ctx context.Context, req *csi.ListSnap if snapshotID != "" { snap, err := cloud.GetSnapshot(ctx, snapshotID) if err != nil { - if stackiterrors.IsNotFound(err) { - klog.V(3).Infof("Snapshot %s not found", snapshotID) - return &csi.ListSnapshotsResponse{}, nil + if !stackiterrors.IsNotFound(err) { + return nil, status.Errorf(codes.Internal, "Failed to GetSnapshot %s: %v", snapshotID, err) } - return nil, status.Errorf(codes.Internal, "Failed to GetSnapshot %s: %v", snapshotID, err) - } - ctime := timestamppb.New(*snap.CreatedAt) + backup, backupErr := cloud.GetBackup(ctx, snapshotID) + if backupErr != nil { + if stackiterrors.IsNotFound(backupErr) { + klog.V(3).Infof("Snapshot or backup %s not found", snapshotID) + return &csi.ListSnapshotsResponse{}, nil + } + return nil, status.Errorf(codes.Internal, "Failed to GetBackup %s: %v", snapshotID, backupErr) + } - entry := &csi.ListSnapshotsResponse_Entry{ - Snapshot: &csi.Snapshot{ - SizeBytes: *snap.Size * util.GIBIBYTE, - SnapshotId: *snap.Id, - SourceVolumeId: snap.VolumeId, - CreationTime: ctime, - ReadyToUse: true, - }, + entry := backupSnapshotEntry(backup) + return &csi.ListSnapshotsResponse{ + Entries: []*csi.ListSnapshotsResponse_Entry{entry}, + }, entry.Snapshot.CreationTime.CheckValid() } - entries := []*csi.ListSnapshotsResponse_Entry{entry} return &csi.ListSnapshotsResponse{ - Entries: entries, - }, ctime.CheckValid() + Entries: []*csi.ListSnapshotsResponse_Entry{snapshotEntry(snap)}, + }, timestamppb.New(*snap.CreatedAt).CheckValid() } - filters := map[string]string{} - - var slist []iaas.Snapshot - var err error - var nextPageToken string - - // Add the filters + filters := map[string]string{ + "Status": stackitclient.SnapshotReadyStatus, + } if req.GetSourceVolumeId() != "" { filters["VolumeID"] = req.GetSourceVolumeId() - } else { - filters["Limit"] = strconv.Itoa(int(req.MaxEntries)) - filters["Marker"] = req.StartingToken } - // Only retrieve snapshots that are available - filters["Status"] = stackitclient.SnapshotReadyStatus - slist, nextPageToken, err = cloud.ListSnapshots(ctx, filters) + snapshotList, _, err := cloud.ListSnapshots(ctx, filters) if err != nil { klog.Errorf("Failed to ListSnapshots: %v", err) return nil, status.Errorf(codes.Internal, "ListSnapshots failed with error %v", err) } - sentries := make([]*csi.ListSnapshotsResponse_Entry, 0, len(slist)) - for _, v := range slist { - ctime := timestamppb.New(*v.CreatedAt) - if err := ctime.CheckValid(); err != nil { - klog.Errorf("Error to convert time to timestamp: %v", err) - } - sentry := csi.ListSnapshotsResponse_Entry{ - Snapshot: &csi.Snapshot{ - SizeBytes: *v.Size * util.GIBIBYTE, - SnapshotId: *v.Id, - SourceVolumeId: v.VolumeId, - CreationTime: ctime, - ReadyToUse: true, - }, - } - sentries = append(sentries, &sentry) + backupList, err := cloud.ListBackups(ctx, filters) + if err != nil { + klog.Errorf("Failed to ListBackups: %v", err) + return nil, status.Errorf(codes.Internal, "ListBackups failed with error %v", err) + } + + entries := make([]*csi.ListSnapshotsResponse_Entry, 0, len(snapshotList)+len(backupList)) + for i := range snapshotList { + entries = append(entries, snapshotEntry(&snapshotList[i])) } + for i := range backupList { + entries = append(entries, backupSnapshotEntry(&backupList[i])) + } + + // sort by creation time and by id (fallback) + sort.Slice(entries, func(i, j int) bool { + left := entries[i].Snapshot + right := entries[j].Snapshot + if left.CreationTime.AsTime().Equal(right.CreationTime.AsTime()) { + return left.SnapshotId < right.SnapshotId + } + return left.CreationTime.AsTime().Before(right.CreationTime.AsTime()) + }) + return &csi.ListSnapshotsResponse{ - Entries: sentries, - NextToken: nextPageToken, + Entries: entries, + NextToken: "", }, nil } +func snapshotEntry(snapshot *iaas.Snapshot) *csi.ListSnapshotsResponse_Entry { + ctime := timestamppb.New(*snapshot.CreatedAt) + if err := ctime.CheckValid(); err != nil { + klog.Errorf("Error to convert time to timestamp: %v", err) + } + + return &csi.ListSnapshotsResponse_Entry{ + Snapshot: &csi.Snapshot{ + SizeBytes: *snapshot.Size * util.GIBIBYTE, + SnapshotId: *snapshot.Id, + SourceVolumeId: snapshot.VolumeId, + CreationTime: ctime, + ReadyToUse: true, + }, + } +} + +func backupSnapshotEntry(backup *iaas.Backup) *csi.ListSnapshotsResponse_Entry { + ctime := timestamppb.New(*backup.CreatedAt) + if err := ctime.CheckValid(); err != nil { + klog.Errorf("Error to convert time to timestamp: %v", err) + } + + return &csi.ListSnapshotsResponse_Entry{ + Snapshot: &csi.Snapshot{ + SizeBytes: *backup.Size * util.GIBIBYTE, + SnapshotId: *backup.Id, + SourceVolumeId: *backup.VolumeId, + CreationTime: ctime, + ReadyToUse: true, + }, + } +} + // ControllerGetCapabilities implements the default GRPC callout. // Default supports all capabilities func (cs *controllerServer) ControllerGetCapabilities(_ context.Context, _ *csi.ControllerGetCapabilitiesRequest) (*csi.ControllerGetCapabilitiesResponse, error) { diff --git a/pkg/csi/blockstorage/controllerserver_test.go b/pkg/csi/blockstorage/controllerserver_test.go index f20f1d7a..d9a9fff5 100644 --- a/pkg/csi/blockstorage/controllerserver_test.go +++ b/pkg/csi/blockstorage/controllerserver_test.go @@ -1032,32 +1032,149 @@ var _ = Describe("ControllerServer test", Ordered, func() { Expect(resp.GetEntries()).Should(Equal(expectedSnapshotListResponse)) Expect(resp.GetEntries()).To(HaveLen(1)) }) - It("should successfully list snapshots", func() { + + It("should fall back to a backup when the snapshot ID resolves to a backup", func() { + req := &csi.ListSnapshotsRequest{ + SnapshotId: "special-backup", + } + backupCreationTime := time.Now() + expectedSnapshotListResponse := []*csi.ListSnapshotsResponse_Entry{ + { + Snapshot: &csi.Snapshot{ + SnapshotId: "special-backup", + SizeBytes: 10 * util.GIBIBYTE, + CreationTime: timestamppb.New(backupCreationTime), + SourceVolumeId: "fake-volume", + ReadyToUse: true, + }, + }, + } + + iaasClient.EXPECT().GetSnapshot(gomock.Any(), "special-backup").Return(nil, &oapierror.GenericOpenAPIError{ + StatusCode: http.StatusNotFound, + }) + iaasClient.EXPECT().GetBackup(gomock.Any(), "special-backup").Return(&iaas.Backup{ + Id: new("special-backup"), + VolumeId: new("fake-volume"), + Size: new(int64(10)), + CreatedAt: new(backupCreationTime), + }, nil) + + resp, err := fakeCs.ListSnapshots(context.Background(), req) + Expect(err).To(Not(HaveOccurred())) + Expect(resp.GetEntries()).Should(Equal(expectedSnapshotListResponse)) + Expect(resp.GetEntries()).To(HaveLen(1)) + }) + + It("should return an empty response when neither snapshot nor backup can be found", func() { + req := &csi.ListSnapshotsRequest{ + SnapshotId: "missing-snapshot", + } + + iaasClient.EXPECT().GetSnapshot(gomock.Any(), "missing-snapshot").Return(nil, &oapierror.GenericOpenAPIError{ + StatusCode: http.StatusNotFound, + }) + iaasClient.EXPECT().GetBackup(gomock.Any(), "missing-snapshot").Return(nil, &oapierror.GenericOpenAPIError{ + StatusCode: http.StatusNotFound, + }) + + resp, err := fakeCs.ListSnapshots(context.Background(), req) + Expect(err).To(Not(HaveOccurred())) + Expect(resp.GetEntries()).To(BeEmpty()) + }) + + It("should successfully list snapshots and backups in deterministic order", func() { req := &csi.ListSnapshotsRequest{} - snapShotCreationTime := time.Now() + firstSnapshotTime := time.Date(2024, time.January, 1, 10, 0, 0, 0, time.UTC) + backupCreationTime := firstSnapshotTime.Add(1 * time.Hour) + secondSnapshotTime := firstSnapshotTime.Add(2 * time.Hour) expectedSnapshotListResponse := []*csi.ListSnapshotsResponse_Entry{ { Snapshot: &csi.Snapshot{ SnapshotId: "fake-snapshot", SizeBytes: 10 * util.GIBIBYTE, - CreationTime: timestamppb.New(snapShotCreationTime), + CreationTime: timestamppb.New(firstSnapshotTime), + SourceVolumeId: "something-different", + ReadyToUse: true, + }, + }, + { + Snapshot: &csi.Snapshot{ + SnapshotId: "fake-backup", + SizeBytes: 20 * util.GIBIBYTE, + CreationTime: timestamppb.New(backupCreationTime), + SourceVolumeId: "something-different", + ReadyToUse: true, + }, + }, + { + Snapshot: &csi.Snapshot{ + SnapshotId: "later-snapshot", + SizeBytes: 30 * util.GIBIBYTE, + CreationTime: timestamppb.New(secondSnapshotTime), SourceVolumeId: "something-different", ReadyToUse: true, }, }, } - iaasClient.EXPECT().ListSnapshots(gomock.Any(), gomock.Any()).Return([]iaas.Snapshot{{ - Id: new("fake-snapshot"), - VolumeId: "something-different", - Size: new(int64(10)), - CreatedAt: new(snapShotCreationTime), - }}, "", nil) + iaasClient.EXPECT().ListSnapshots(gomock.Any(), gomock.Any()).Return([]iaas.Snapshot{ + { + Id: new("later-snapshot"), + VolumeId: "something-different", + Size: new(int64(30)), + CreatedAt: new(secondSnapshotTime), + Status: new("AVAILABLE"), + }, + { + Id: new("fake-snapshot"), + VolumeId: "something-different", + Size: new(int64(10)), + CreatedAt: new(firstSnapshotTime), + Status: new("AVAILABLE"), + }, + }, "", nil) + iaasClient.EXPECT().ListBackups(gomock.Any(), gomock.Any()).Return([]iaas.Backup{ + { + Id: new("fake-backup"), + VolumeId: new("something-different"), + Size: new(int64(20)), + CreatedAt: new(backupCreationTime), + Status: new("AVAILABLE"), + }, + }, nil) + resp, err := fakeCs.ListSnapshots(context.Background(), req) Expect(err).To(Not(HaveOccurred())) Expect(resp.GetEntries()).Should(Equal(expectedSnapshotListResponse)) + Expect(resp.GetNextToken()).To(BeEmpty()) + }) + + It("should pass the source volume filter to snapshots and backups", func() { + req := &csi.ListSnapshotsRequest{ + SourceVolumeId: "volume-1", + } + + expectedFilters := map[string]string{ + "Status": stackitclient.SnapshotReadyStatus, + "VolumeID": "volume-1", + } + + iaasClient.EXPECT().ListSnapshots(gomock.Any(), gomock.Any()).DoAndReturn(func(_ context.Context, filters map[string]string) ([]iaas.Snapshot, string, error) { + Expect(filters).To(Equal(expectedFilters)) + return []iaas.Snapshot{}, "", nil + }) + iaasClient.EXPECT().ListBackups(gomock.Any(), gomock.Any()).DoAndReturn(func(_ context.Context, filters map[string]string) ([]iaas.Backup, error) { + Expect(filters).To(Equal(expectedFilters)) + return []iaas.Backup{}, nil + }) + + resp, err := fakeCs.ListSnapshots(context.Background(), req) + Expect(err).To(Not(HaveOccurred())) + Expect(resp.GetEntries()).To(BeEmpty()) }) + }) }) diff --git a/pkg/csi/blockstorage/sanity_test.go b/pkg/csi/blockstorage/sanity_test.go index bac6c348..0428447a 100644 --- a/pkg/csi/blockstorage/sanity_test.go +++ b/pkg/csi/blockstorage/sanity_test.go @@ -450,6 +450,13 @@ var _ = Describe("CSI sanity test", Ordered, func() { }) Describe("CSI sanity", func() { + BeforeEach(func() { + if CurrentSpecReport().LeafNodeText == "should return next token when a limited number of entries are requested" && + CurrentSpecReport().FullText() == "CSI sanity test Base config CSI sanity ListSnapshots [Controller Server] should return next token when a limited number of entries are requested" { + Skip("ListSnapshots pagination is intentionally not supported by this driver") + } + }) + config := sanity.NewTestConfig() config.Address = FakeEndpoint