✨ feat: honor maxEntries in ListSnapshots

outscale · Jan 14, 2025 · ebb1bf1 · ebb1bf1
1 parent f0589e7
commit ebb1bf1
Show file tree

Hide file tree

Showing 8 changed files with 48 additions and 37 deletions.
diff --git a/Makefile b/Makefile
@@ -98,10 +98,10 @@ test-e2e-single-az-buildx:
 	docker buildx build  --load -t $(E2E_ENV) -f ./tests/e2e/docker/Dockerfile_e2eTest .
 	$(MAKE) test-e2e-single-az-run
 
-bin/mockgen: | bin
-	go get github.com/golang/mock/mockgen@latest
+bin/mockgen:
+	go install github.com/golang/mock/mockgen@latest
 
-mockgen: bin/mockgen
+mockgen:
 	./hack/update-gomock
 
 .PHONY: trivy-scan

diff --git a/hack/update-gomock b/hack/update-gomock
@@ -18,9 +18,10 @@ set -euo pipefail
 
 IMPORT_PATH=github.com/outscale/osc-bsu-csi-driver
 
-./bin/mockgen -package=mocks -destination=./pkg/cloud/mocks/mock_ec2.go ${IMPORT_PATH}/pkg/cloud EC2 
-./bin/mockgen -package=mocks -destination=./pkg/cloud/mocks/mock_ec2metadata.go ${IMPORT_PATH}/pkg/cloud EC2Metadata 
-./bin/mockgen -package=mocks -destination=./pkg/driver/mocks/mock_cloud.go ${IMPORT_PATH}/pkg/cloud Cloud
-./bin/mockgen -package=mocks -destination=./pkg/driver/mocks/mock_metadata_service.go ${IMPORT_PATH}/pkg/cloud MetadataService 
-./bin/mockgen -package=mocks -destination=./pkg/driver/mocks/mock_mounter.go ${IMPORT_PATH}/pkg/driver Mounter
+# mockgen -package=mocks -destination=./pkg/cloud/mocks/mock_ec2.go ${IMPORT_PATH}/pkg/cloud EC2 
+mockgen -package=mocks -destination=./pkg/cloud/mocks/mock_ec2metadata.go ${IMPORT_PATH}/pkg/cloud EC2Metadata 
+
+mockgen -package=mocks -destination=./pkg/driver/mocks/mock_cloud.go ${IMPORT_PATH}/pkg/cloud Cloud
+mockgen -package=mocks -destination=./pkg/driver/mocks/mock_metadata_service.go ${IMPORT_PATH}/pkg/cloud MetadataService 
+mockgen -package=mocks -destination=./pkg/driver/mocks/mock_mounter.go ${IMPORT_PATH}/pkg/driver Mounter
 
diff --git a/pkg/cloud/cloud.go b/pkg/cloud/cloud.go
@@ -29,6 +29,7 @@ import (
 	osc "github.com/outscale/osc-sdk-go/v2"
 	"k8s.io/apimachinery/pkg/util/wait"
 	"k8s.io/klog/v2"
+	"k8s.io/utils/ptr"
 )
 
 // Outscale volume types
@@ -174,7 +175,7 @@ type Cloud interface {
 	DeleteSnapshot(ctx context.Context, snapshotID string) (success bool, err error)
 	GetSnapshotByName(ctx context.Context, name string) (snapshot Snapshot, err error)
 	GetSnapshotByID(ctx context.Context, snapshotID string) (snapshot Snapshot, err error)
-	ListSnapshots(ctx context.Context, volumeID string, maxResults int64, nextToken string) (listSnapshotsResponse ListSnapshotsResponse, err error)
+	ListSnapshots(ctx context.Context, volumeID string, maxResults int32, nextToken string) (listSnapshotsResponse ListSnapshotsResponse, err error)
 }
 
 type OscInterface interface {
@@ -846,25 +847,31 @@ func (c *cloud) GetSnapshotByID(ctx context.Context, snapshotID string) (snapsho
 	return c.oscSnapshotResponseToStruct(oscsnapshot), nil
 }
 
+const maxResultsLimit = 1000
+
 // ListSnapshots retrieves Outscale BSU 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.
 // Pagination not supported
-func (c *cloud) ListSnapshots(ctx context.Context, volumeID string, maxResults int64, nextToken string) (listSnapshotsResponse ListSnapshotsResponse, err error) {
-	request := osc.ReadSnapshotsRequest{
+func (c *cloud) ListSnapshots(ctx context.Context, volumeID string, maxResults int32, nextToken string) (listSnapshotsResponse ListSnapshotsResponse, err error) {
+	if maxResults > maxResultsLimit {
+		maxResults = maxResultsLimit
+	}
+	req := osc.ReadSnapshotsRequest{
 		Filters: &osc.FiltersSnapshot{
 			VolumeIds: &[]string{},
 		},
+		ResultsPerPage: ptr.To(maxResults),
 	}
 
 	if len(volumeID) != 0 {
-		request = osc.ReadSnapshotsRequest{
+		req = osc.ReadSnapshotsRequest{
 			Filters: &osc.FiltersSnapshot{
 				VolumeIds: &[]string{volumeID},
 			},
 		}
 	}
 
-	resp, err := c.listSnapshots(ctx, request)
+	resp, err := c.listSnapshots(ctx, req)
 	if err != nil {
 		return ListSnapshotsResponse{}, err
 	}

diff --git a/pkg/cloud/mocks/mock_ec2metadata.go b/pkg/cloud/mocks/mock_ec2metadata.go
diff --git a/pkg/driver/controller.go b/pkg/driver/controller.go
@@ -486,7 +486,7 @@ func (d *controllerService) ListSnapshots(ctx context.Context, req *csi.ListSnap
 	nextToken := req.GetStartingToken()
 	maxEntries := req.GetMaxEntries()
 
-	cloudSnapshots, err := d.cloud.ListSnapshots(ctx, volumeID, int64(maxEntries), nextToken)
+	cloudSnapshots, err := d.cloud.ListSnapshots(ctx, volumeID, maxEntries, nextToken)
 	if err != nil {
 		if errors.Is(err, cloud.ErrNotFound) {
 			klog.FromContext(ctx).V(4).Info("Snapshot does not exist")

diff --git a/pkg/driver/controller_test.go b/pkg/driver/controller_test.go
@@ -1604,7 +1604,7 @@ func TestListSnapshots(t *testing.T) {
 				defer mockCtl.Finish()
 
 				mockCloud := mocks.NewMockCloud(mockCtl)
-				mockCloud.EXPECT().ListSnapshots(gomock.Eq(ctx), gomock.Eq(""), gomock.Eq(int64(0)), gomock.Eq("")).Return(mockCloudSnapshotsResponse, nil)
+				mockCloud.EXPECT().ListSnapshots(gomock.Eq(ctx), gomock.Eq(""), gomock.Eq(int32(0)), gomock.Eq("")).Return(mockCloudSnapshotsResponse, nil)
 
 				oscDriver := controllerService{
 					cloud:         mockCloud,
@@ -1630,7 +1630,7 @@ func TestListSnapshots(t *testing.T) {
 				defer mockCtl.Finish()
 
 				mockCloud := mocks.NewMockCloud(mockCtl)
-				mockCloud.EXPECT().ListSnapshots(gomock.Eq(ctx), gomock.Eq(""), gomock.Eq(int64(0)), gomock.Eq("")).Return(cloud.ListSnapshotsResponse{}, cloud.ErrNotFound)
+				mockCloud.EXPECT().ListSnapshots(gomock.Eq(ctx), gomock.Eq(""), gomock.Eq(int32(0)), gomock.Eq("")).Return(cloud.ListSnapshotsResponse{}, cloud.ErrNotFound)
 
 				oscDriver := controllerService{
 					cloud:         mockCloud,
@@ -1754,7 +1754,7 @@ func TestListSnapshots(t *testing.T) {
 				mockCtl := gomock.NewController(t)
 				defer mockCtl.Finish()
 				mockCloud := mocks.NewMockCloud(mockCtl)
-				mockCloud.EXPECT().ListSnapshots(gomock.Eq(ctx), gomock.Eq(""), gomock.Eq(int64(4)), gomock.Eq("")).Return(cloud.ListSnapshotsResponse{}, cloud.ErrInvalidMaxResults)
+				mockCloud.EXPECT().ListSnapshots(gomock.Eq(ctx), gomock.Eq(""), gomock.Eq(int32(4)), gomock.Eq("")).Return(cloud.ListSnapshotsResponse{}, cloud.ErrInvalidMaxResults)
 
 				oscDriver := controllerService{
 					cloud:         mockCloud,

diff --git a/pkg/driver/mocks/mock_cloud.go b/pkg/driver/mocks/mock_cloud.go
diff --git a/pkg/driver/sanity_test.go b/pkg/driver/sanity_test.go
@@ -104,7 +104,7 @@ type fakeCloudProvider struct {
 	// snapshots contains mapping from snapshot ID to snapshot
 	snapshots map[string]*fakeSnapshot
 	pub       map[string]string
-	tokens    map[string]int64
+	tokens    map[string]int32
 	m         *cloud.Metadata
 }
 
@@ -123,7 +123,7 @@ func newFakeCloudProvider() *fakeCloudProvider {
 		disks:     make(map[string]*fakeDisk),
 		snapshots: make(map[string]*fakeSnapshot),
 		pub:       make(map[string]string),
-		tokens:    make(map[string]int64),
+		tokens:    make(map[string]int32),
 		m: &cloud.Metadata{
 			InstanceID:       "instanceID",
 			Region:           "region",
@@ -264,7 +264,7 @@ func (c *fakeCloudProvider) GetSnapshotByID(ctx context.Context, snapshotID stri
 	return ret.Snapshot, nil
 }
 
-func (c *fakeCloudProvider) ListSnapshots(ctx context.Context, volumeID string, maxResults int64, nextToken string) (listSnapshotsResponse cloud.ListSnapshotsResponse, err error) {
+func (c *fakeCloudProvider) ListSnapshots(ctx context.Context, volumeID string, maxResults int32, nextToken string) (listSnapshotsResponse cloud.ListSnapshotsResponse, err error) {
 	var snapshots []cloud.Snapshot
 	var retToken string
 	for _, fakeSnapshot := range c.snapshots {