Skip to content

Commit abd502a

Browse files
timoreimanntimoreimann
committed
Improve ListSnapshot filtering tests
We provide tests on ListSnapshots for filtering by snapshot IDs and source volume IDs. However, the tests do not cover the case were snapshot listing functionality exists in general but is missing filtering capabilities: for this scenario the tests yield false positives because the fixtures set up a single snapshot only. We improve the tests by creating a number of snapshots of which a subset is expected to be found.
1 parent 60bf859 commit abd502a

File tree

1 file changed

+110
-37
lines changed

1 file changed

+110
-37
lines changed

‎pkg/sanity/controller.go

Lines changed: 110 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -1340,35 +1340,71 @@ var _ = DescribeSanity("ListSnapshots [Controller Server]", func(sc *TestContext
13401340
})
13411341

13421342
It("should return snapshots that match the specified snapshot id", func() {
1343+
// The test creates three snapshots: one that we intend to find by
1344+
// snapshot ID, and two unrelated ones that must not be returned by
1345+
// ListSnapshots.
1346+
1347+
By("creating first unrelated snapshot")
1348+
// Create volume source and afterwards the first unrelated snapshot.
1349+
volReq := MakeCreateVolumeReq(sc, "listSnapshots-volume-unrelated1")
1350+
volumeUnrelated1, err := c.CreateVolume(context.Background(), volReq)
1351+
Expect(err).NotTo(HaveOccurred())
13431352

1344-
By("creating a volume")
1345-
volReq := MakeCreateVolumeReq(sc, "listSnapshots-volume-1")
1346-
volume, err := c.CreateVolume(context.Background(), volReq)
1353+
snapshotReq := MakeCreateSnapshotReq(sc, "listSnapshots-snapshot-unrelated1", volumeUnrelated1.GetVolume().GetVolumeId())
1354+
snapshotUnrelated1, err := c.CreateSnapshot(context.Background(), snapshotReq)
13471355
Expect(err).NotTo(HaveOccurred())
13481356

1349-
By("creating a snapshot")
1350-
snapshotReq := MakeCreateSnapshotReq(sc, "listSnapshots-snapshot-1", volume.GetVolume().GetVolumeId())
1351-
snapshot, err := c.CreateSnapshot(context.Background(), snapshotReq)
1357+
By("creating target snapshot")
1358+
// Create volume source and afterwards the target snapshot.
1359+
volReq = MakeCreateVolumeReq(sc, "listSnapshots-volume-target")
1360+
volumeTarget, err := c.CreateVolume(context.Background(), volReq)
1361+
Expect(err).NotTo(HaveOccurred())
1362+
1363+
snapshotReq = MakeCreateSnapshotReq(sc, "listSnapshots-snapshot-target", volumeTarget.GetVolume().GetVolumeId())
1364+
snapshotTarget, err := c.CreateSnapshot(context.Background(), snapshotReq)
13521365
Expect(err).NotTo(HaveOccurred())
13531366

1367+
By("creating second unrelated snapshot")
1368+
// Create volume source and afterwards the second unrelated snapshot.
1369+
volReq = MakeCreateVolumeReq(sc, "listSnapshots-volume-unrelated2")
1370+
volumeUnrelated2, err := c.CreateVolume(context.Background(), volReq)
1371+
Expect(err).NotTo(HaveOccurred())
1372+
1373+
snapshotReq = MakeCreateSnapshotReq(sc, "listSnapshots-snapshot-unrelated2", volumeUnrelated2.GetVolume().GetVolumeId())
1374+
snapshotUnrelated2, err := c.CreateSnapshot(context.Background(), snapshotReq)
1375+
Expect(err).NotTo(HaveOccurred())
1376+
1377+
By("listing snapshots")
13541378
snapshots, err := c.ListSnapshots(
13551379
context.Background(),
1356-
&csi.ListSnapshotsRequest{SnapshotId: snapshot.GetSnapshot().GetSnapshotId()})
1380+
&csi.ListSnapshotsRequest{SnapshotId: snapshotTarget.GetSnapshot().GetSnapshotId()})
13571381
Expect(err).NotTo(HaveOccurred())
13581382
Expect(snapshots).NotTo(BeNil())
13591383
Expect(len(snapshots.GetEntries())).To(BeNumerically("==", 1))
13601384
verifySnapshotInfo(snapshots.GetEntries()[0].GetSnapshot())
1361-
Expect(snapshots.GetEntries()[0].GetSnapshot().GetSnapshotId()).To(Equal(snapshot.GetSnapshot().GetSnapshotId()))
1362-
1363-
By("cleaning up deleting the snapshot")
1364-
delSnapReq := MakeDeleteSnapshotReq(sc, snapshot.GetSnapshot().GetSnapshotId())
1365-
_, err = c.DeleteSnapshot(context.Background(), delSnapReq)
1366-
Expect(err).NotTo(HaveOccurred())
1385+
Expect(snapshots.GetEntries()[0].GetSnapshot().GetSnapshotId()).To(Equal(snapshotTarget.GetSnapshot().GetSnapshotId()))
1386+
1387+
By("deleting snapshots")
1388+
for _, snapshot := range []*csi.CreateSnapshotResponse{
1389+
snapshotUnrelated1,
1390+
snapshotTarget,
1391+
snapshotUnrelated2,
1392+
} {
1393+
delSnapReq := MakeDeleteSnapshotReq(sc, snapshot.GetSnapshot().GetSnapshotId())
1394+
_, err = c.DeleteSnapshot(context.Background(), delSnapReq)
1395+
Expect(err).NotTo(HaveOccurred())
1396+
}
13671397

1368-
By("cleaning up deleting the volume")
1369-
delVolReq := MakeDeleteVolumeReq(sc, volume.GetVolume().GetVolumeId())
1370-
_, err = c.DeleteVolume(context.Background(), delVolReq)
1371-
Expect(err).NotTo(HaveOccurred())
1398+
By("deleting volumes")
1399+
for _, volume := range []*csi.CreateVolumeResponse{
1400+
volumeUnrelated1,
1401+
volumeTarget,
1402+
volumeUnrelated2,
1403+
} {
1404+
delVolReq := MakeDeleteVolumeReq(sc, volume.GetVolume().GetVolumeId())
1405+
_, err = c.DeleteVolume(context.Background(), delVolReq)
1406+
Expect(err).NotTo(HaveOccurred())
1407+
}
13721408
})
13731409

13741410
It("should return empty when the specified snapshot id does not exist", func() {
@@ -1381,37 +1417,74 @@ var _ = DescribeSanity("ListSnapshots [Controller Server]", func(sc *TestContext
13811417
Expect(snapshots.GetEntries()).To(BeEmpty())
13821418
})
13831419

1384-
It("should return snapshots that match the specified source volume id)", func() {
1420+
It("should return snapshots that match the specified source volume id", func() {
13851421

1386-
By("creating a volume")
1387-
volReq := MakeCreateVolumeReq(sc, "listSnapshots-volume-2")
1388-
volume, err := c.CreateVolume(context.Background(), volReq)
1422+
// The test creates three snapshots: one that we intend to find by
1423+
// source volume ID, and two unrelated ones that must not be returned by
1424+
// ListSnapshots.
1425+
1426+
By("creating first unrelated snapshot")
1427+
// Create volume source and afterwards the first unrelated snapshot.
1428+
volReq := MakeCreateVolumeReq(sc, "listSnapshots-volume-unrelated1")
1429+
volumeUnrelated1, err := c.CreateVolume(context.Background(), volReq)
13891430
Expect(err).NotTo(HaveOccurred())
13901431

1391-
By("creating a snapshot")
1392-
snapshotReq := MakeCreateSnapshotReq(sc, "listSnapshots-snapshot-2", volume.GetVolume().GetVolumeId())
1393-
snapshot, err := c.CreateSnapshot(context.Background(), snapshotReq)
1432+
snapshotReq := MakeCreateSnapshotReq(sc, "listSnapshots-snapshot-unrelated1", volumeUnrelated1.GetVolume().GetVolumeId())
1433+
snapshotUnrelated1, err := c.CreateSnapshot(context.Background(), snapshotReq)
13941434
Expect(err).NotTo(HaveOccurred())
13951435

1436+
By("creating target snapshot")
1437+
// Create volume source and afterwards the target snapshot.
1438+
volReq = MakeCreateVolumeReq(sc, "listSnapshots-volume-target")
1439+
volumeTarget, err := c.CreateVolume(context.Background(), volReq)
1440+
Expect(err).NotTo(HaveOccurred())
1441+
1442+
snapshotReq = MakeCreateSnapshotReq(sc, "listSnapshots-snapshot-target", volumeTarget.GetVolume().GetVolumeId())
1443+
snapshotTarget, err := c.CreateSnapshot(context.Background(), snapshotReq)
1444+
Expect(err).NotTo(HaveOccurred())
1445+
1446+
By("creating second unrelated snapshot")
1447+
// Create volume source and afterwards the second unrelated snapshot.
1448+
volReq = MakeCreateVolumeReq(sc, "listSnapshots-volume-unrelated2")
1449+
volumeUnrelated2, err := c.CreateVolume(context.Background(), volReq)
1450+
Expect(err).NotTo(HaveOccurred())
1451+
1452+
snapshotReq = MakeCreateSnapshotReq(sc, "listSnapshots-snapshot-unrelated2", volumeUnrelated2.GetVolume().GetVolumeId())
1453+
snapshotUnrelated2, err := c.CreateSnapshot(context.Background(), snapshotReq)
1454+
Expect(err).NotTo(HaveOccurred())
1455+
1456+
By("listing snapshots")
13961457
snapshots, err := c.ListSnapshots(
13971458
context.Background(),
1398-
&csi.ListSnapshotsRequest{SourceVolumeId: snapshot.GetSnapshot().GetSourceVolumeId()})
1459+
&csi.ListSnapshotsRequest{SourceVolumeId: snapshotTarget.GetSnapshot().GetSourceVolumeId()})
13991460
Expect(err).NotTo(HaveOccurred())
14001461
Expect(snapshots).NotTo(BeNil())
1401-
for _, snap := range snapshots.GetEntries() {
1402-
verifySnapshotInfo(snap.GetSnapshot())
1403-
Expect(snap.GetSnapshot().GetSourceVolumeId()).To(Equal(snapshot.GetSnapshot().GetSourceVolumeId()))
1462+
Expect(snapshots.GetEntries()).To(HaveLen(1))
1463+
snapshot := snapshots.GetEntries()[0].GetSnapshot()
1464+
verifySnapshotInfo(snapshot)
1465+
Expect(snapshot.GetSourceVolumeId()).To(Equal(snapshotTarget.GetSnapshot().GetSourceVolumeId()))
1466+
1467+
By("deleting snapshots")
1468+
for _, snapshot := range []*csi.CreateSnapshotResponse{
1469+
snapshotUnrelated1,
1470+
snapshotTarget,
1471+
snapshotUnrelated2,
1472+
} {
1473+
delSnapReq := MakeDeleteSnapshotReq(sc, snapshot.GetSnapshot().GetSnapshotId())
1474+
_, err = c.DeleteSnapshot(context.Background(), delSnapReq)
1475+
Expect(err).NotTo(HaveOccurred())
14041476
}
14051477

1406-
By("cleaning up deleting the snapshot")
1407-
delSnapReq := MakeDeleteSnapshotReq(sc, snapshot.GetSnapshot().GetSnapshotId())
1408-
_, err = c.DeleteSnapshot(context.Background(), delSnapReq)
1409-
Expect(err).NotTo(HaveOccurred())
1410-
1411-
By("cleaning up deleting the volume")
1412-
delVolReq := MakeDeleteVolumeReq(sc, volume.GetVolume().GetVolumeId())
1413-
_, err = c.DeleteVolume(context.Background(), delVolReq)
1414-
Expect(err).NotTo(HaveOccurred())
1478+
By("deleting volumes")
1479+
for _, volume := range []*csi.CreateVolumeResponse{
1480+
volumeUnrelated1,
1481+
volumeTarget,
1482+
volumeUnrelated2,
1483+
} {
1484+
delVolReq := MakeDeleteVolumeReq(sc, volume.GetVolume().GetVolumeId())
1485+
_, err = c.DeleteVolume(context.Background(), delVolReq)
1486+
Expect(err).NotTo(HaveOccurred())
1487+
}
14151488
})
14161489

14171490
It("should return empty when the specified source volume id does not exist", func() {

0 commit comments

Comments
 (0)