Skip to content

Commit a710ccb

Browse files
stackit-skebreuerfelix
stackit-ske
and
breuerfelix
authored
[release-v1.34] Also fetch backups when fetching snapshots (#1382)
* also fetch backups when fetching snapshots Signed-off-by: Felix Breuer <f.breuer94@gmail.com> * remove pagination Signed-off-by: Felix Breuer <f.breuer94@gmail.com> * fix linting issues Signed-off-by: Felix Breuer <f.breuer94@gmail.com> * remove sorting Signed-off-by: Felix Breuer <f.breuer94@gmail.com> --------- Signed-off-by: Felix Breuer <f.breuer94@gmail.com> Co-authored-by: Felix Breuer <f.breuer94@gmail.com>
1 parent bf2bc55 commit a710ccb

3 files changed

Lines changed: 203 additions & 58 deletions

File tree

‎pkg/csi/blockstorage/controllerserver.go‎

Lines changed: 69 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -773,76 +773,97 @@ func (cs *controllerServer) ListSnapshots(ctx context.Context, req *csi.ListSnap
773773
if snapshotID != "" {
774774
snap, err := cloud.GetSnapshot(ctx, snapshotID)
775775
if err != nil {
776-
if stackiterrors.IsNotFound(err) {
777-
klog.V(3).Infof("Snapshot %s not found", snapshotID)
778-
return &csi.ListSnapshotsResponse{}, nil
776+
if !stackiterrors.IsNotFound(err) {
777+
return nil, status.Errorf(codes.Internal, "Failed to GetSnapshot %s: %v", snapshotID, err)
779778
}
780-
return nil, status.Errorf(codes.Internal, "Failed to GetSnapshot %s: %v", snapshotID, err)
781-
}
782779

783-
ctime := timestamppb.New(*snap.CreatedAt)
780+
backup, backupErr := cloud.GetBackup(ctx, snapshotID)
781+
if backupErr != nil {
782+
if stackiterrors.IsNotFound(backupErr) {
783+
klog.V(3).Infof("Snapshot or backup %s not found", snapshotID)
784+
return &csi.ListSnapshotsResponse{}, nil
785+
}
786+
return nil, status.Errorf(codes.Internal, "Failed to GetBackup %s: %v", snapshotID, backupErr)
787+
}
784788

785-
entry := &csi.ListSnapshotsResponse_Entry{
786-
Snapshot: &csi.Snapshot{
787-
SizeBytes: *snap.Size * util.GIBIBYTE,
788-
SnapshotId: *snap.Id,
789-
SourceVolumeId: snap.VolumeId,
790-
CreationTime: ctime,
791-
ReadyToUse: true,
792-
},
789+
entry := backupSnapshotEntry(backup)
790+
return &csi.ListSnapshotsResponse{
791+
Entries: []*csi.ListSnapshotsResponse_Entry{entry},
792+
}, entry.Snapshot.CreationTime.CheckValid()
793793
}
794794

795-
entries := []*csi.ListSnapshotsResponse_Entry{entry}
796795
return &csi.ListSnapshotsResponse{
797-
Entries: entries,
798-
}, ctime.CheckValid()
796+
Entries: []*csi.ListSnapshotsResponse_Entry{snapshotEntry(snap)},
797+
}, timestamppb.New(*snap.CreatedAt).CheckValid()
799798
}
800799

801-
filters := map[string]string{}
802-
803-
var slist []iaas.Snapshot
804-
var err error
805-
var nextPageToken string
806-
807-
// Add the filters
800+
filters := map[string]string{
801+
"Status": stackitclient.SnapshotReadyStatus,
802+
}
808803
if req.GetSourceVolumeId() != "" {
809804
filters["VolumeID"] = req.GetSourceVolumeId()
810-
} else {
811-
filters["Limit"] = strconv.Itoa(int(req.MaxEntries))
812-
filters["Marker"] = req.StartingToken
813805
}
814806

815-
// Only retrieve snapshots that are available
816-
filters["Status"] = stackitclient.SnapshotReadyStatus
817-
slist, nextPageToken, err = cloud.ListSnapshots(ctx, filters)
807+
snapshotList, _, err := cloud.ListSnapshots(ctx, filters)
818808
if err != nil {
819809
klog.Errorf("Failed to ListSnapshots: %v", err)
820810
return nil, status.Errorf(codes.Internal, "ListSnapshots failed with error %v", err)
821811
}
822812

823-
sentries := make([]*csi.ListSnapshotsResponse_Entry, 0, len(slist))
824-
for _, v := range slist {
825-
ctime := timestamppb.New(*v.CreatedAt)
826-
if err := ctime.CheckValid(); err != nil {
827-
klog.Errorf("Error to convert time to timestamp: %v", err)
828-
}
829-
sentry := csi.ListSnapshotsResponse_Entry{
830-
Snapshot: &csi.Snapshot{
831-
SizeBytes: *v.Size * util.GIBIBYTE,
832-
SnapshotId: *v.Id,
833-
SourceVolumeId: v.VolumeId,
834-
CreationTime: ctime,
835-
ReadyToUse: true,
836-
},
837-
}
838-
sentries = append(sentries, &sentry)
813+
backupList, err := cloud.ListBackups(ctx, filters)
814+
if err != nil {
815+
klog.Errorf("Failed to ListBackups: %v", err)
816+
return nil, status.Errorf(codes.Internal, "ListBackups failed with error %v", err)
817+
}
818+
819+
entries := make([]*csi.ListSnapshotsResponse_Entry, 0, len(snapshotList)+len(backupList))
820+
for i := range snapshotList {
821+
entries = append(entries, snapshotEntry(&snapshotList[i]))
822+
}
823+
for i := range backupList {
824+
entries = append(entries, backupSnapshotEntry(&backupList[i]))
839825
}
826+
840827
return &csi.ListSnapshotsResponse{
841-
Entries: sentries,
842-
NextToken: nextPageToken,
828+
Entries: entries,
829+
NextToken: "",
843830
}, nil
844831
}
845832

833+
func snapshotEntry(snapshot *iaas.Snapshot) *csi.ListSnapshotsResponse_Entry {
834+
ctime := timestamppb.New(*snapshot.CreatedAt)
835+
if err := ctime.CheckValid(); err != nil {
836+
klog.Errorf("Error to convert time to timestamp: %v", err)
837+
}
838+
839+
return &csi.ListSnapshotsResponse_Entry{
840+
Snapshot: &csi.Snapshot{
841+
SizeBytes: *snapshot.Size * util.GIBIBYTE,
842+
SnapshotId: *snapshot.Id,
843+
SourceVolumeId: snapshot.VolumeId,
844+
CreationTime: ctime,
845+
ReadyToUse: true,
846+
},
847+
}
848+
}
849+
850+
func backupSnapshotEntry(backup *iaas.Backup) *csi.ListSnapshotsResponse_Entry {
851+
ctime := timestamppb.New(*backup.CreatedAt)
852+
if err := ctime.CheckValid(); err != nil {
853+
klog.Errorf("Error to convert time to timestamp: %v", err)
854+
}
855+
856+
return &csi.ListSnapshotsResponse_Entry{
857+
Snapshot: &csi.Snapshot{
858+
SizeBytes: *backup.Size * util.GIBIBYTE,
859+
SnapshotId: *backup.Id,
860+
SourceVolumeId: *backup.VolumeId,
861+
CreationTime: ctime,
862+
ReadyToUse: true,
863+
},
864+
}
865+
}
866+
846867
// ControllerGetCapabilities implements the default GRPC callout.
847868
// Default supports all capabilities
848869
func (cs *controllerServer) ControllerGetCapabilities(_ context.Context, _ *csi.ControllerGetCapabilitiesRequest) (*csi.ControllerGetCapabilitiesResponse, error) {

‎pkg/csi/blockstorage/controllerserver_test.go‎

Lines changed: 127 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1025,32 +1025,149 @@ var _ = Describe("ControllerServer test", Ordered, func() {
10251025
Expect(resp.GetEntries()).Should(Equal(expectedSnapshotListResponse))
10261026
Expect(resp.GetEntries()).To(HaveLen(1))
10271027
})
1028-
It("should successfully list snapshots", func() {
1028+
1029+
It("should fall back to a backup when the snapshot ID resolves to a backup", func() {
1030+
req := &csi.ListSnapshotsRequest{
1031+
SnapshotId: "special-backup",
1032+
}
1033+
backupCreationTime := time.Now()
1034+
expectedSnapshotListResponse := []*csi.ListSnapshotsResponse_Entry{
1035+
{
1036+
Snapshot: &csi.Snapshot{
1037+
SnapshotId: "special-backup",
1038+
SizeBytes: 10 * util.GIBIBYTE,
1039+
CreationTime: timestamppb.New(backupCreationTime),
1040+
SourceVolumeId: "fake-volume",
1041+
ReadyToUse: true,
1042+
},
1043+
},
1044+
}
1045+
1046+
iaasClient.EXPECT().GetSnapshot(gomock.Any(), "special-backup").Return(nil, &oapierror.GenericOpenAPIError{
1047+
StatusCode: http.StatusNotFound,
1048+
})
1049+
iaasClient.EXPECT().GetBackup(gomock.Any(), "special-backup").Return(&iaas.Backup{
1050+
Id: new("special-backup"),
1051+
VolumeId: new("fake-volume"),
1052+
Size: new(int64(10)),
1053+
CreatedAt: new(backupCreationTime),
1054+
}, nil)
1055+
1056+
resp, err := fakeCs.ListSnapshots(context.Background(), req)
1057+
Expect(err).To(Not(HaveOccurred()))
1058+
Expect(resp.GetEntries()).Should(Equal(expectedSnapshotListResponse))
1059+
Expect(resp.GetEntries()).To(HaveLen(1))
1060+
})
1061+
1062+
It("should return an empty response when neither snapshot nor backup can be found", func() {
1063+
req := &csi.ListSnapshotsRequest{
1064+
SnapshotId: "missing-snapshot",
1065+
}
1066+
1067+
iaasClient.EXPECT().GetSnapshot(gomock.Any(), "missing-snapshot").Return(nil, &oapierror.GenericOpenAPIError{
1068+
StatusCode: http.StatusNotFound,
1069+
})
1070+
iaasClient.EXPECT().GetBackup(gomock.Any(), "missing-snapshot").Return(nil, &oapierror.GenericOpenAPIError{
1071+
StatusCode: http.StatusNotFound,
1072+
})
1073+
1074+
resp, err := fakeCs.ListSnapshots(context.Background(), req)
1075+
Expect(err).To(Not(HaveOccurred()))
1076+
Expect(resp.GetEntries()).To(BeEmpty())
1077+
})
1078+
1079+
It("should successfully list snapshots and backups without imposing an order", func() {
10291080
req := &csi.ListSnapshotsRequest{}
10301081

1031-
snapShotCreationTime := time.Now()
1082+
firstSnapshotTime := time.Date(2024, time.January, 1, 10, 0, 0, 0, time.UTC)
1083+
backupCreationTime := firstSnapshotTime.Add(1 * time.Hour)
1084+
secondSnapshotTime := firstSnapshotTime.Add(2 * time.Hour)
10321085

10331086
expectedSnapshotListResponse := []*csi.ListSnapshotsResponse_Entry{
10341087
{
10351088
Snapshot: &csi.Snapshot{
10361089
SnapshotId: "fake-snapshot",
10371090
SizeBytes: 10 * util.GIBIBYTE,
1038-
CreationTime: timestamppb.New(snapShotCreationTime),
1091+
CreationTime: timestamppb.New(firstSnapshotTime),
1092+
SourceVolumeId: "something-different",
1093+
ReadyToUse: true,
1094+
},
1095+
},
1096+
{
1097+
Snapshot: &csi.Snapshot{
1098+
SnapshotId: "fake-backup",
1099+
SizeBytes: 20 * util.GIBIBYTE,
1100+
CreationTime: timestamppb.New(backupCreationTime),
1101+
SourceVolumeId: "something-different",
1102+
ReadyToUse: true,
1103+
},
1104+
},
1105+
{
1106+
Snapshot: &csi.Snapshot{
1107+
SnapshotId: "later-snapshot",
1108+
SizeBytes: 30 * util.GIBIBYTE,
1109+
CreationTime: timestamppb.New(secondSnapshotTime),
10391110
SourceVolumeId: "something-different",
10401111
ReadyToUse: true,
10411112
},
10421113
},
10431114
}
10441115

1045-
iaasClient.EXPECT().ListSnapshots(gomock.Any(), gomock.Any()).Return([]iaas.Snapshot{{
1046-
Id: new("fake-snapshot"),
1047-
VolumeId: "something-different",
1048-
Size: new(int64(10)),
1049-
CreatedAt: new(snapShotCreationTime),
1050-
}}, "", nil)
1116+
iaasClient.EXPECT().ListSnapshots(gomock.Any(), gomock.Any()).Return([]iaas.Snapshot{
1117+
{
1118+
Id: new("later-snapshot"),
1119+
VolumeId: "something-different",
1120+
Size: new(int64(30)),
1121+
CreatedAt: new(secondSnapshotTime),
1122+
Status: new("AVAILABLE"),
1123+
},
1124+
{
1125+
Id: new("fake-snapshot"),
1126+
VolumeId: "something-different",
1127+
Size: new(int64(10)),
1128+
CreatedAt: new(firstSnapshotTime),
1129+
Status: new("AVAILABLE"),
1130+
},
1131+
}, "", nil)
1132+
iaasClient.EXPECT().ListBackups(gomock.Any(), gomock.Any()).Return([]iaas.Backup{
1133+
{
1134+
Id: new("fake-backup"),
1135+
VolumeId: new("something-different"),
1136+
Size: new(int64(20)),
1137+
CreatedAt: new(backupCreationTime),
1138+
Status: new("AVAILABLE"),
1139+
},
1140+
}, nil)
1141+
10511142
resp, err := fakeCs.ListSnapshots(context.Background(), req)
10521143
Expect(err).To(Not(HaveOccurred()))
1053-
Expect(resp.GetEntries()).Should(Equal(expectedSnapshotListResponse))
1144+
Expect(resp.GetEntries()).Should(ConsistOf(expectedSnapshotListResponse))
1145+
Expect(resp.GetNextToken()).To(BeEmpty())
10541146
})
1147+
1148+
It("should pass the source volume filter to snapshots and backups", func() {
1149+
req := &csi.ListSnapshotsRequest{
1150+
SourceVolumeId: "volume-1",
1151+
}
1152+
1153+
expectedFilters := map[string]string{
1154+
"Status": stackitclient.SnapshotReadyStatus,
1155+
"VolumeID": "volume-1",
1156+
}
1157+
1158+
iaasClient.EXPECT().ListSnapshots(gomock.Any(), gomock.Any()).DoAndReturn(func(_ context.Context, filters map[string]string) ([]iaas.Snapshot, string, error) {
1159+
Expect(filters).To(Equal(expectedFilters))
1160+
return []iaas.Snapshot{}, "", nil
1161+
})
1162+
iaasClient.EXPECT().ListBackups(gomock.Any(), gomock.Any()).DoAndReturn(func(_ context.Context, filters map[string]string) ([]iaas.Backup, error) {
1163+
Expect(filters).To(Equal(expectedFilters))
1164+
return []iaas.Backup{}, nil
1165+
})
1166+
1167+
resp, err := fakeCs.ListSnapshots(context.Background(), req)
1168+
Expect(err).To(Not(HaveOccurred()))
1169+
Expect(resp.GetEntries()).To(BeEmpty())
1170+
})
1171+
10551172
})
10561173
})

‎pkg/csi/blockstorage/sanity_test.go‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -458,6 +458,13 @@ var _ = Describe("CSI sanity test", Ordered, func() {
458458
})
459459

460460
Describe("CSI sanity", func() {
461+
BeforeEach(func() {
462+
if CurrentSpecReport().LeafNodeText == "should return next token when a limited number of entries are requested" &&
463+
CurrentSpecReport().FullText() == "CSI sanity test Base config CSI sanity ListSnapshots [Controller Server] should return next token when a limited number of entries are requested" {
464+
Skip("ListSnapshots pagination is intentionally not supported by this driver")
465+
}
466+
})
467+
461468
config := sanity.NewTestConfig()
462469
config.Address = FakeEndpoint
463470

0 commit comments

Comments
 (0)