Skip to content

Commit

Permalink
🐛 fix: fix pagination on ListSnapshots
Browse files Browse the repository at this point in the history
  • Loading branch information
@jfbus
jfbus committed Jan 30, 2025
1 parent 0d11af3 commit f706b63
Show file tree
Hide file tree
Showing 2 changed files with 284 additions and 19 deletions.
25 changes: 15 additions & 10 deletions pkg/cloud/cloud.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import (
"github.com/outscale-dev/osc-bsu-csi-driver/pkg/util"
"k8s.io/apimachinery/pkg/util/wait"
"k8s.io/klog/v2"
"k8s.io/utils/ptr"
)

// AWS volume types
Expand Down Expand Up @@ -864,28 +865,32 @@ func (c *cloud) GetSnapshotByID(ctx context.Context, snapshotID string) (snapsho
return c.oscSnapshotResponseToStruct(oscsnapshot), nil
}

const maxResultsLimit = 1000

// ListSnapshots retrieves AWS EBS snapshots for an optionally specified volume ID. If maxResults is set, it will return up to maxResults snapshots. If there are more snapshots than maxResults,
// a next token value will be returned to the client as well. They can use this token with subsequent calls to retrieve the next page of results. If maxResults is not set (0),
// there will be no restriction up to 1000 results (https://docs.aws.amazon.com/sdk-for-go/api/service/ec2/#DescribeSnapshotsInput).
// Pagination not supported
func (c *cloud) ListSnapshots(ctx context.Context, volumeID string, maxResults int64, nextToken string) (listSnapshotsResponse ListSnapshotsResponse, err error) {
klog.Infof("Debug ListSnapshots : %+v, %+v, %+v\n", volumeID, maxResults, nextToken)

request := osc.ReadSnapshotsRequest{
Filters: &osc.FiltersSnapshot{
VolumeIds: &[]string{},
},
if maxResults > maxResultsLimit {
maxResults = maxResultsLimit
}

req := osc.ReadSnapshotsRequest{}
if maxResults > 0 {
req.ResultsPerPage = ptr.To(int32(maxResults))
}
if nextToken != "" {
req.NextPageToken = &nextToken
}
if len(volumeID) != 0 {
request = osc.ReadSnapshotsRequest{
Filters: &osc.FiltersSnapshot{
VolumeIds: &[]string{volumeID},
},
req.Filters = &osc.FiltersSnapshot{
VolumeIds: &[]string{volumeID},
}
}

oscSnapshotsResponse, err := c.listSnapshots(ctx, request)
oscSnapshotsResponse, err := c.listSnapshots(ctx, req)
if err != nil {
return ListSnapshotsResponse{}, err
}
Expand Down
278 changes: 269 additions & 9 deletions pkg/cloud/cloud_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,12 @@ import (

"github.com/aws/aws-sdk-go/aws/awserr"
"github.com/golang/mock/gomock"

osc "github.com/outscale/osc-sdk-go/v2"

dm "github.com/outscale-dev/osc-bsu-csi-driver/pkg/cloud/devicemanager"
"github.com/outscale-dev/osc-bsu-csi-driver/pkg/cloud/mocks"
"github.com/outscale-dev/osc-bsu-csi-driver/pkg/util"
osc "github.com/outscale/osc-sdk-go/v2"
"github.com/stretchr/testify/require"
"k8s.io/utils/ptr"
)

const (
Expand Down Expand Up @@ -764,7 +764,7 @@ func TestListSnapshots(t *testing.T) {
testFunc func(t *testing.T)
}{
{
name: "success: normal",
name: "success without token and maxitems",
testFunc: func(t *testing.T) {
expSnapshots := []Snapshot{
{
Expand Down Expand Up @@ -800,13 +800,273 @@ func TestListSnapshots(t *testing.T) {
c := newCloud(mockOscInterface)

ctx := context.Background()
fmt.Printf("Read snapshot :\n")
mockOscInterface.EXPECT().ReadSnapshots(gomock.Eq(ctx), gomock.Any()).Return(osc.ReadSnapshotsResponse{Snapshots: &oscsnapshot}, nil, nil)
fmt.Printf("End Read snapshot :\n")
mockOscInterface.EXPECT().ReadSnapshots(gomock.Eq(ctx), gomock.Eq(osc.ReadSnapshotsRequest{})).Return(osc.ReadSnapshotsResponse{Snapshots: &oscsnapshot}, nil, nil)
_, err := c.ListSnapshots(ctx, "", 0, "")
if err != nil {
t.Fatalf("ListSnapshots() failed: expected no error, got: %v", err)
require.NoError(t, err)
},
},
{
name: "success with max entries",
testFunc: func(t *testing.T) {
expSnapshots := []Snapshot{
{
SourceVolumeID: "snap-test-volume1",
SnapshotID: "snap-test-name1",
},
{
SourceVolumeID: "snap-test-volume2",
SnapshotID: "snap-test-name2",
},
}
state := "completed"
volumeIds := []string{
"snap-test-volume1",
"snap-test-volume2",
}
oscsnapshot := []osc.Snapshot{
{
SnapshotId: &expSnapshots[0].SnapshotID,
VolumeId: &volumeIds[0],
State: &state,
},
{
SnapshotId: &expSnapshots[1].SnapshotID,
VolumeId: &volumeIds[1],
State: &state,
},
}

mockCtrl := gomock.NewController(t)
defer mockCtrl.Finish()
mockOscInterface := mocks.NewMockOscInterface(mockCtrl)
c := newCloud(mockOscInterface)

ctx := context.Background()
mockOscInterface.EXPECT().ReadSnapshots(gomock.Eq(ctx), gomock.Eq(osc.ReadSnapshotsRequest{
ResultsPerPage: ptr.To(int32(10)),
})).Return(osc.ReadSnapshotsResponse{Snapshots: &oscsnapshot}, nil, nil)
_, err := c.ListSnapshots(ctx, "", 10, "")
require.NoError(t, err)
},
},
{
name: "success with max entries over OAPI max (1000)",
testFunc: func(t *testing.T) {
expSnapshots := []Snapshot{
{
SourceVolumeID: "snap-test-volume1",
SnapshotID: "snap-test-name1",
},
{
SourceVolumeID: "snap-test-volume2",
SnapshotID: "snap-test-name2",
},
}
state := "completed"
volumeIds := []string{
"snap-test-volume1",
"snap-test-volume2",
}
oscsnapshot := []osc.Snapshot{
{
SnapshotId: &expSnapshots[0].SnapshotID,
VolumeId: &volumeIds[0],
State: &state,
},
{
SnapshotId: &expSnapshots[1].SnapshotID,
VolumeId: &volumeIds[1],
State: &state,
},
}

mockCtrl := gomock.NewController(t)
defer mockCtrl.Finish()
mockOscInterface := mocks.NewMockOscInterface(mockCtrl)
c := newCloud(mockOscInterface)

ctx := context.Background()
mockOscInterface.EXPECT().ReadSnapshots(gomock.Eq(ctx), gomock.Eq(osc.ReadSnapshotsRequest{
ResultsPerPage: ptr.To(int32(1000)),
})).Return(osc.ReadSnapshotsResponse{Snapshots: &oscsnapshot}, nil, nil)
_, err := c.ListSnapshots(ctx, "", 2000, "")
require.NoError(t, err)
},
},
{
name: "success with next token",
testFunc: func(t *testing.T) {
expSnapshots := []Snapshot{
{
SourceVolumeID: "snap-test-volume1",
SnapshotID: "snap-test-name1",
},
{
SourceVolumeID: "snap-test-volume2",
SnapshotID: "snap-test-name2",
},
}
state := "completed"
volumeIds := []string{
"snap-test-volume1",
"snap-test-volume2",
}
oscsnapshot := []osc.Snapshot{
{
SnapshotId: &expSnapshots[0].SnapshotID,
VolumeId: &volumeIds[0],
State: &state,
},
{
SnapshotId: &expSnapshots[1].SnapshotID,
VolumeId: &volumeIds[1],
State: &state,
},
}

mockCtrl := gomock.NewController(t)
defer mockCtrl.Finish()
mockOscInterface := mocks.NewMockOscInterface(mockCtrl)
c := newCloud(mockOscInterface)

ctx := context.Background()
mockOscInterface.EXPECT().ReadSnapshots(gomock.Eq(ctx), gomock.Eq(osc.ReadSnapshotsRequest{
NextPageToken: ptr.To("foo"),
})).Return(osc.ReadSnapshotsResponse{Snapshots: &oscsnapshot}, nil, nil)
_, err := c.ListSnapshots(ctx, "", 0, "foo")
require.NoError(t, err)
},
},
{
name: "success with max entries",
testFunc: func(t *testing.T) {
expSnapshots := []Snapshot{
{
SourceVolumeID: "snap-test-volume1",
SnapshotID: "snap-test-name1",
},
{
SourceVolumeID: "snap-test-volume2",
SnapshotID: "snap-test-name2",
},
}
state := "completed"
volumeIds := []string{
"snap-test-volume1",
"snap-test-volume2",
}
oscsnapshot := []osc.Snapshot{
{
SnapshotId: &expSnapshots[0].SnapshotID,
VolumeId: &volumeIds[0],
State: &state,
},
{
SnapshotId: &expSnapshots[1].SnapshotID,
VolumeId: &volumeIds[1],
State: &state,
},
}

mockCtrl := gomock.NewController(t)
defer mockCtrl.Finish()
mockOscInterface := mocks.NewMockOscInterface(mockCtrl)
c := newCloud(mockOscInterface)

ctx := context.Background()
mockOscInterface.EXPECT().ReadSnapshots(gomock.Eq(ctx), gomock.Eq(osc.ReadSnapshotsRequest{
ResultsPerPage: ptr.To(int32(10)),
})).Return(osc.ReadSnapshotsResponse{Snapshots: &oscsnapshot}, nil, nil)
_, err := c.ListSnapshots(ctx, "", 10, "")
require.NoError(t, err)
},
},
{
name: "success with max entries over OAPI max (1000)",
testFunc: func(t *testing.T) {
expSnapshots := []Snapshot{
{
SourceVolumeID: "snap-test-volume1",
SnapshotID: "snap-test-name1",
},
{
SourceVolumeID: "snap-test-volume2",
SnapshotID: "snap-test-name2",
},
}
state := "completed"
volumeIds := []string{
"snap-test-volume1",
"snap-test-volume2",
}
oscsnapshot := []osc.Snapshot{
{
SnapshotId: &expSnapshots[0].SnapshotID,
VolumeId: &volumeIds[0],
State: &state,
},
{
SnapshotId: &expSnapshots[1].SnapshotID,
VolumeId: &volumeIds[1],
State: &state,
},
}

mockCtrl := gomock.NewController(t)
defer mockCtrl.Finish()
mockOscInterface := mocks.NewMockOscInterface(mockCtrl)
c := newCloud(mockOscInterface)

ctx := context.Background()
mockOscInterface.EXPECT().ReadSnapshots(gomock.Eq(ctx), gomock.Eq(osc.ReadSnapshotsRequest{
ResultsPerPage: ptr.To(int32(1000)),
})).Return(osc.ReadSnapshotsResponse{Snapshots: &oscsnapshot}, nil, nil)
_, err := c.ListSnapshots(ctx, "", 2000, "")
require.NoError(t, err)
},
},
{
name: "success with next token",
testFunc: func(t *testing.T) {
expSnapshots := []Snapshot{
{
SourceVolumeID: "snap-test-volume1",
SnapshotID: "snap-test-name1",
},
{
SourceVolumeID: "snap-test-volume2",
SnapshotID: "snap-test-name2",
},
}
state := "completed"
volumeIds := []string{
"snap-test-volume1",
"snap-test-volume2",
}
oscsnapshot := []osc.Snapshot{
{
SnapshotId: &expSnapshots[0].SnapshotID,
VolumeId: &volumeIds[0],
State: &state,
},
{
SnapshotId: &expSnapshots[1].SnapshotID,
VolumeId: &volumeIds[1],
State: &state,
},
}

mockCtrl := gomock.NewController(t)
defer mockCtrl.Finish()
mockOscInterface := mocks.NewMockOscInterface(mockCtrl)
c := newCloud(mockOscInterface)

ctx := context.Background()
mockOscInterface.EXPECT().ReadSnapshots(gomock.Eq(ctx), gomock.Eq(osc.ReadSnapshotsRequest{
NextPageToken: ptr.To("foo"),
})).Return(osc.ReadSnapshotsResponse{Snapshots: &oscsnapshot}, nil, nil)
_, err := c.ListSnapshots(ctx, "", 0, "foo")
require.NoError(t, err)
},
},
{
Expand Down

0 comments on commit f706b63

Please sign in to comment.