From 95dc81c5d9e27d6d75dc285fb28942159024b1c0 Mon Sep 17 00:00:00 2001
From: kayrus <kayrus@users.noreply.github.com>
Date: Tue, 18 Jul 2023 13:49:08 +0200
Subject: [PATCH] [cinder-csi-plugin]: fix pagination, avoid unnecessary memory
 allocation, add more logs (#2296)

* [cinder]: avoid unnecessary memory allocation with more logs

* Fix pagination parsing
---
 pkg/csi/cinder/controllerserver.go              | 9 +++++++--
 pkg/csi/cinder/controllerserver_test.go         | 4 +++-
 pkg/csi/cinder/openstack/openstack_snapshots.go | 4 ++--
 pkg/csi/cinder/openstack/openstack_volumes.go   | 4 ++--
 4 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/pkg/csi/cinder/controllerserver.go b/pkg/csi/cinder/controllerserver.go
index 21a72706ce..6b9674ef50 100644
--- a/pkg/csi/cinder/controllerserver.go
+++ b/pkg/csi/cinder/controllerserver.go
@@ -280,6 +280,7 @@ func (cs *controllerServer) ControllerUnpublishVolume(ctx context.Context, req *
 }
 
 func (cs *controllerServer) ListVolumes(ctx context.Context, req *csi.ListVolumesRequest) (*csi.ListVolumesResponse, error) {
+	klog.V(4).Infof("ListVolumes: called with %+#v request", req)
 
 	if req.MaxEntries < 0 {
 		return nil, status.Error(codes.InvalidArgument, fmt.Sprintf(
@@ -296,7 +297,7 @@ func (cs *controllerServer) ListVolumes(ctx context.Context, req *csi.ListVolume
 		return nil, status.Error(codes.Internal, fmt.Sprintf("ListVolumes failed with error %v", err))
 	}
 
-	var ventries []*csi.ListVolumesResponse_Entry
+	ventries := make([]*csi.ListVolumesResponse_Entry, 0, len(vlist))
 	for _, v := range vlist {
 		ventry := csi.ListVolumesResponse_Entry{
 			Volume: &csi.Volume{
@@ -306,6 +307,7 @@ func (cs *controllerServer) ListVolumes(ctx context.Context, req *csi.ListVolume
 		}
 
 		status := &csi.ListVolumesResponse_VolumeStatus{}
+		status.PublishedNodeIds = make([]string, 0, len(v.Attachments))
 		for _, attachment := range v.Attachments {
 			status.PublishedNodeIds = append(status.PublishedNodeIds, attachment.ServerID)
 		}
@@ -313,6 +315,8 @@ func (cs *controllerServer) ListVolumes(ctx context.Context, req *csi.ListVolume
 
 		ventries = append(ventries, &ventry)
 	}
+
+	klog.V(4).Infof("ListVolumes: completed with %d entries and %q next token", len(ventries), nextPageToken)
 	return &csi.ListVolumesResponse{
 		Entries:   ventries,
 		NextToken: nextPageToken,
@@ -478,7 +482,7 @@ func (cs *controllerServer) ListSnapshots(ctx context.Context, req *csi.ListSnap
 		return nil, status.Errorf(codes.Internal, "ListSnapshots failed with error %v", err)
 	}
 
-	var sentries []*csi.ListSnapshotsResponse_Entry
+	sentries := make([]*csi.ListSnapshotsResponse_Entry, 0, len(slist))
 	for _, v := range slist {
 		ctime := timestamppb.New(v.CreatedAt)
 		if err := ctime.CheckValid(); err != nil {
@@ -582,6 +586,7 @@ func (cs *controllerServer) ControllerGetVolume(ctx context.Context, req *csi.Co
 	}
 
 	status := &csi.ControllerGetVolumeResponse_VolumeStatus{}
+	status.PublishedNodeIds = make([]string, 0, len(volume.Attachments))
 	for _, attachment := range volume.Attachments {
 		status.PublishedNodeIds = append(status.PublishedNodeIds, attachment.ServerID)
 	}
diff --git a/pkg/csi/cinder/controllerserver_test.go b/pkg/csi/cinder/controllerserver_test.go
index 7032d9aad8..eb32140a4c 100644
--- a/pkg/csi/cinder/controllerserver_test.go
+++ b/pkg/csi/cinder/controllerserver_test.go
@@ -441,7 +441,9 @@ func TestListVolumes(t *testing.T) {
 					VolumeId:      FakeVol3.ID,
 					CapacityBytes: int64(FakeVol3.Size * 1024 * 1024 * 1024),
 				},
-				Status: &csi.ListVolumesResponse_VolumeStatus{},
+				Status: &csi.ListVolumesResponse_VolumeStatus{
+					PublishedNodeIds: []string{},
+				},
 			},
 		},
 		NextToken: "",
diff --git a/pkg/csi/cinder/openstack/openstack_snapshots.go b/pkg/csi/cinder/openstack/openstack_snapshots.go
index ca4fdc5b9d..00dc9b5c65 100644
--- a/pkg/csi/cinder/openstack/openstack_snapshots.go
+++ b/pkg/csi/cinder/openstack/openstack_snapshots.go
@@ -119,11 +119,11 @@ func (os *OpenStack) ListSnapshots(filters map[string]string) ([]snapshots.Snaps
 		}
 
 		if nextPageURL != "" {
-			queryParams, err := url.ParseQuery(nextPageURL)
+			pageURL, err := url.Parse(nextPageURL)
 			if err != nil {
 				return false, err
 			}
-			nextPageToken = queryParams.Get("marker")
+			nextPageToken = pageURL.Query().Get("marker")
 		}
 
 		return false, nil
diff --git a/pkg/csi/cinder/openstack/openstack_volumes.go b/pkg/csi/cinder/openstack/openstack_volumes.go
index 79372196da..083908e6b4 100644
--- a/pkg/csi/cinder/openstack/openstack_volumes.go
+++ b/pkg/csi/cinder/openstack/openstack_volumes.go
@@ -96,11 +96,11 @@ func (os *OpenStack) ListVolumes(limit int, startingToken string) ([]volumes.Vol
 		}
 
 		if nextPageURL != "" {
-			queryParams, err := url.ParseQuery(nextPageURL)
+			pageURL, err := url.Parse(nextPageURL)
 			if err != nil {
 				return false, err
 			}
-			nextPageToken = queryParams.Get("marker")
+			nextPageToken = pageURL.Query().Get("marker")
 		}
 
 		return false, nil