From 4f6ce26226b3bd19c4bcd37ed2210dd808abce13 Mon Sep 17 00:00:00 2001 From: Boris Glimcher Date: Mon, 10 Apr 2023 22:04:31 +0300 Subject: [PATCH 01/11] List: generate NextPageToken If API returned partial result: - generate new NextPageToken based on UUID - return the token in the responce - save the token to re-use later on - Empty NextPageToken indicates end of results list Token expiration is not implemeted yet. Signed-off-by: Boris Glimcher --- go.mod | 1 + go.sum | 2 ++ pkg/backend/aio.go | 6 +++++- pkg/backend/aio_test.go | 4 ++++ pkg/backend/backend.go | 6 ++++-- pkg/backend/null.go | 6 +++++- pkg/backend/null_test.go | 4 ++++ pkg/backend/nvme.go | 6 +++++- pkg/backend/nvme_test.go | 4 ++++ pkg/frontend/blk.go | 6 +++++- pkg/frontend/blk_test.go | 4 ++++ pkg/frontend/frontend.go | 8 +++++--- pkg/frontend/nvme.go | 15 ++++++++++++--- pkg/frontend/nvme_test.go | 8 ++++++++ pkg/frontend/scsi.go | 11 +++++++++-- pkg/middleend/middleend.go | 12 +++++++++--- pkg/middleend/middleend_test.go | 4 ++++ 17 files changed, 90 insertions(+), 17 deletions(-) diff --git a/go.mod b/go.mod index efb6baba..ad6d3498 100644 --- a/go.mod +++ b/go.mod @@ -4,6 +4,7 @@ go 1.19 require ( github.com/digitalocean/go-qemu v0.0.0-20221209210016-f035778c97f7 + github.com/google/uuid v1.3.0 github.com/opiproject/opi-api v0.0.0-20230404182329-b6f178ec8cfa github.com/ulule/deepcopier v0.0.0-20200430083143-45decc6639b6 google.golang.org/grpc v1.54.0 diff --git a/go.sum b/go.sum index 960b6b31..d573d23f 100644 --- a/go.sum +++ b/go.sum @@ -8,6 +8,8 @@ github.com/golang/protobuf v1.5.2 h1:ROPKBNFfQgOUMifHyP+KYbvpjbdoFNs+aK7DXlji0Tw github.com/golang/protobuf v1.5.2/go.mod h1:XVQd3VNwM+JqD3oG2Ue2ip4fOMUkwXdXDdiuN0vRsmY= github.com/google/go-cmp v0.5.5/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= github.com/google/go-cmp v0.5.9 h1:O2Tfq5qg4qc4AmwVlvv0oLiVAGB7enBSJ2x2DqQFi38= +github.com/google/uuid v1.3.0 h1:t6JiXgmwXMjEs8VusXIJk2BXHsn+wx8BZdTaoZ5fu7I= +github.com/google/uuid v1.3.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= github.com/opiproject/opi-api v0.0.0-20230111150933-e4b3480e8ee9 h1:Y/0Ku5yIfnLEnLa9jzYCK/7l85CHQVefs0PnaXFX+v0= github.com/opiproject/opi-api v0.0.0-20230111150933-e4b3480e8ee9/go.mod h1:92pv4ulvvPMuxCJ9ND3aYbmBfEMLx0VCjpkiR7ZTqPY= github.com/opiproject/opi-api v0.0.0-20230123165122-10e47bafd42b h1:Ho6qkBoU1vfqERZbih+Hmn7Y1lsx14YtOiqMl4lzdF4= diff --git a/pkg/backend/aio.go b/pkg/backend/aio.go index b52ac389..accaee11 100644 --- a/pkg/backend/aio.go +++ b/pkg/backend/aio.go @@ -14,6 +14,7 @@ import ( pb "github.com/opiproject/opi-api/storage/v1alpha1/gen/go" "github.com/opiproject/opi-spdk-bridge/pkg/models" + "github.com/google/uuid" "github.com/ulule/deepcopier" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" @@ -148,16 +149,19 @@ func (s *Server) ListAioControllers(_ context.Context, in *pb.ListAioControllers return nil, err } log.Printf("Received from SPDK: %v", result) + var token string if in.PageSize > 0 && int(in.PageSize) < len(result) { log.Printf("Limiting result to: %d", in.PageSize) result = result[:in.PageSize] + token = uuid.New().String() + s.Pagination[token] = int(in.PageSize) } Blobarray := make([]*pb.AioController, len(result)) for i := range result { r := &result[i] Blobarray[i] = &pb.AioController{Handle: &pc.ObjectKey{Value: r.Name}, BlockSize: r.BlockSize, BlocksCount: r.NumBlocks} } - return &pb.ListAioControllersResponse{AioControllers: Blobarray}, nil + return &pb.ListAioControllersResponse{AioControllers: Blobarray, NextPageToken: token}, nil } // GetAioController gets an Aio controller diff --git a/pkg/backend/aio_test.go b/pkg/backend/aio_test.go index 98992f74..2cd2de2d 100644 --- a/pkg/backend/aio_test.go +++ b/pkg/backend/aio_test.go @@ -362,6 +362,10 @@ func TestBackEnd_ListAioControllers(t *testing.T) { if !reflect.DeepEqual(response.AioControllers, tt.out) { t.Error("response: expected", tt.out, "received", response.AioControllers) } + // Empty NextPageToken indicates end of results list + if tt.size != 1 && response.NextPageToken != "" { + t.Error("response: expected", tt, "received", response) + } } if err != nil { diff --git a/pkg/backend/backend.go b/pkg/backend/backend.go index d24f3423..7cd7e818 100644 --- a/pkg/backend/backend.go +++ b/pkg/backend/backend.go @@ -26,8 +26,9 @@ type Server struct { pb.UnimplementedNullDebugServiceServer pb.UnimplementedAioControllerServiceServer - rpc server.JSONRPC - Volumes VolumeParameters + rpc server.JSONRPC + Volumes VolumeParameters + Pagination map[string]int } // NewServer creates initialized instance of BackEnd server communicating @@ -40,5 +41,6 @@ func NewServer(jsonRPC server.JSONRPC) *Server { NullVolumes: make(map[string]*pb.NullDebug), NvmeVolumes: make(map[string]*pb.NVMfRemoteController), }, + Pagination: make(map[string]int), } } diff --git a/pkg/backend/null.go b/pkg/backend/null.go index e8f06c7a..3e3c5e6b 100644 --- a/pkg/backend/null.go +++ b/pkg/backend/null.go @@ -14,6 +14,7 @@ import ( pb "github.com/opiproject/opi-api/storage/v1alpha1/gen/go" "github.com/opiproject/opi-spdk-bridge/pkg/models" + "github.com/google/uuid" "github.com/ulule/deepcopier" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" @@ -148,16 +149,19 @@ func (s *Server) ListNullDebugs(_ context.Context, in *pb.ListNullDebugsRequest) return nil, err } log.Printf("Received from SPDK: %v", result) + var token string if in.PageSize > 0 && int(in.PageSize) < len(result) { log.Printf("Limiting result to: %d", in.PageSize) result = result[:in.PageSize] + token = uuid.New().String() + s.Pagination[token] = int(in.PageSize) } Blobarray := make([]*pb.NullDebug, len(result)) for i := range result { r := &result[i] Blobarray[i] = &pb.NullDebug{Handle: &pc.ObjectKey{Value: r.Name}, Uuid: &pc.Uuid{Value: r.UUID}, BlockSize: r.BlockSize, BlocksCount: r.NumBlocks} } - return &pb.ListNullDebugsResponse{NullDebugs: Blobarray}, nil + return &pb.ListNullDebugsResponse{NullDebugs: Blobarray, NextPageToken: token}, nil } // GetNullDebug gets a a Null Debug instance diff --git a/pkg/backend/null_test.go b/pkg/backend/null_test.go index ee994774..a1f5b574 100644 --- a/pkg/backend/null_test.go +++ b/pkg/backend/null_test.go @@ -366,6 +366,10 @@ func TestBackEnd_ListNullDebugs(t *testing.T) { if !reflect.DeepEqual(response.NullDebugs, tt.out) { t.Error("response: expected", tt.out, "received", response.NullDebugs) } + // Empty NextPageToken indicates end of results list + if tt.size != 1 && response.NextPageToken != "" { + t.Error("response: expected", tt, "received", response) + } } if err != nil { diff --git a/pkg/backend/nvme.go b/pkg/backend/nvme.go index 667cf181..dd991d6d 100644 --- a/pkg/backend/nvme.go +++ b/pkg/backend/nvme.go @@ -15,6 +15,7 @@ import ( pb "github.com/opiproject/opi-api/storage/v1alpha1/gen/go" "github.com/opiproject/opi-spdk-bridge/pkg/models" + "github.com/google/uuid" "github.com/ulule/deepcopier" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" @@ -110,9 +111,12 @@ func (s *Server) ListNVMfRemoteControllers(_ context.Context, in *pb.ListNVMfRem return nil, err } log.Printf("Received from SPDK: %v", result) + var token string if in.PageSize > 0 && int(in.PageSize) < len(result) { log.Printf("Limiting result to: %d", in.PageSize) result = result[:in.PageSize] + token = uuid.New().String() + s.Pagination[token] = int(in.PageSize) } Blobarray := make([]*pb.NVMfRemoteController, len(result)) for i := range result { @@ -128,7 +132,7 @@ func (s *Server) ListNVMfRemoteControllers(_ context.Context, in *pb.ListNVMfRem Trsvcid: port, } } - return &pb.ListNVMfRemoteControllersResponse{NvMfRemoteControllers: Blobarray}, nil + return &pb.ListNVMfRemoteControllersResponse{NvMfRemoteControllers: Blobarray, NextPageToken: token}, nil } // GetNVMfRemoteController gets an NVMf remote controller diff --git a/pkg/backend/nvme_test.go b/pkg/backend/nvme_test.go index d5140ac1..ca03376b 100644 --- a/pkg/backend/nvme_test.go +++ b/pkg/backend/nvme_test.go @@ -315,6 +315,10 @@ func TestBackEnd_ListNVMfRemoteControllers(t *testing.T) { if !reflect.DeepEqual(response.NvMfRemoteControllers, tt.out) { t.Error("response: expected", tt.out, "received", response.NvMfRemoteControllers) } + // Empty NextPageToken indicates end of results list + if tt.size != 1 && response.NextPageToken != "" { + t.Error("response: expected", tt, "received", response) + } } if err != nil { diff --git a/pkg/frontend/blk.go b/pkg/frontend/blk.go index 8cb47902..f64b0d5f 100644 --- a/pkg/frontend/blk.go +++ b/pkg/frontend/blk.go @@ -10,6 +10,7 @@ import ( "fmt" "log" + "github.com/google/uuid" pc "github.com/opiproject/opi-api/common/v1/gen/go" pb "github.com/opiproject/opi-api/storage/v1alpha1/gen/go" "github.com/opiproject/opi-spdk-bridge/pkg/models" @@ -107,9 +108,12 @@ func (s *Server) ListVirtioBlks(_ context.Context, in *pb.ListVirtioBlksRequest) return nil, err } log.Printf("Received from SPDK: %v", result) + var token string if in.PageSize > 0 && int(in.PageSize) < len(result) { log.Printf("Limiting result to: %d", in.PageSize) result = result[:in.PageSize] + token = uuid.New().String() + s.Pagination[token] = int(in.PageSize) } Blobarray := make([]*pb.VirtioBlk, len(result)) for i := range result { @@ -119,7 +123,7 @@ func (s *Server) ListVirtioBlks(_ context.Context, in *pb.ListVirtioBlksRequest) PcieId: &pb.PciEndpoint{PhysicalFunction: 1}, VolumeId: &pc.ObjectKey{Value: "TBD"}} } - return &pb.ListVirtioBlksResponse{VirtioBlks: Blobarray}, nil + return &pb.ListVirtioBlksResponse{VirtioBlks: Blobarray, NextPageToken: token}, nil } // GetVirtioBlk gets a Virtio block device diff --git a/pkg/frontend/blk_test.go b/pkg/frontend/blk_test.go index 4a01e52e..ce7e722f 100644 --- a/pkg/frontend/blk_test.go +++ b/pkg/frontend/blk_test.go @@ -269,6 +269,10 @@ func TestFrontEnd_ListVirtioBlks(t *testing.T) { if !reflect.DeepEqual(response.VirtioBlks, tt.out) { t.Error("response: expected", tt.out, "received", response.VirtioBlks) } + // Empty NextPageToken indicates end of results list + if tt.size != 1 && response.NextPageToken != "" { + t.Error("response: expected", tt, "received", response) + } } if err != nil { diff --git a/pkg/frontend/frontend.go b/pkg/frontend/frontend.go index d99a2a18..000689f2 100644 --- a/pkg/frontend/frontend.go +++ b/pkg/frontend/frontend.go @@ -40,9 +40,10 @@ type Server struct { pb.UnimplementedFrontendVirtioBlkServiceServer pb.UnimplementedFrontendVirtioScsiServiceServer - rpc server.JSONRPC - Nvme NvmeParameters - Virt VirtioParameters + rpc server.JSONRPC + Nvme NvmeParameters + Virt VirtioParameters + Pagination map[string]int } // NewServer creates initialized instance of FrontEnd server communicating @@ -61,6 +62,7 @@ func NewServer(jsonRPC server.JSONRPC) *Server { ScsiCtrls: make(map[string]*pb.VirtioScsiController), ScsiLuns: make(map[string]*pb.VirtioScsiLun), }, + Pagination: make(map[string]int), } } diff --git a/pkg/frontend/nvme.go b/pkg/frontend/nvme.go index 6177f339..8d45222f 100644 --- a/pkg/frontend/nvme.go +++ b/pkg/frontend/nvme.go @@ -15,6 +15,7 @@ import ( pb "github.com/opiproject/opi-api/storage/v1alpha1/gen/go" "github.com/opiproject/opi-spdk-bridge/pkg/models" + "github.com/google/uuid" "github.com/ulule/deepcopier" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" @@ -172,16 +173,19 @@ func (s *Server) ListNVMeSubsystems(_ context.Context, in *pb.ListNVMeSubsystems return nil, err } log.Printf("Received from SPDK: %v", result) + var token string if in.PageSize > 0 && int(in.PageSize) < len(result) { log.Printf("Limiting result to: %d", in.PageSize) result = result[:in.PageSize] + token = uuid.New().String() + s.Pagination[token] = int(in.PageSize) } Blobarray := make([]*pb.NVMeSubsystem, len(result)) for i := range result { r := &result[i] Blobarray[i] = &pb.NVMeSubsystem{Spec: &pb.NVMeSubsystemSpec{Nqn: r.Nqn, SerialNumber: r.SerialNumber, ModelNumber: r.ModelNumber}} } - return &pb.ListNVMeSubsystemsResponse{NvMeSubsystems: Blobarray}, nil + return &pb.ListNVMeSubsystemsResponse{NvMeSubsystems: Blobarray, NextPageToken: token}, nil } // GetNVMeSubsystem gets NVMe Subsystems @@ -325,7 +329,9 @@ func (s *Server) ListNVMeControllers(_ context.Context, in *pb.ListNVMeControlle for _, controller := range s.Nvme.Controllers { Blobarray = append(Blobarray, controller) } - return &pb.ListNVMeControllersResponse{NvMeControllers: Blobarray}, nil + token := uuid.New().String() + s.Pagination[token] = int(in.PageSize) + return &pb.ListNVMeControllersResponse{NvMeControllers: Blobarray, NextPageToken: token}, nil } // GetNVMeController gets an NVMe controller @@ -474,6 +480,7 @@ func (s *Server) ListNVMeNamespaces(_ context.Context, in *pb.ListNVMeNamespaces return nil, err } log.Printf("Received from SPDK: %v", result) + var token string Blobarray := []*pb.NVMeNamespace{} for i := range result { rr := &result[i] @@ -481,6 +488,8 @@ func (s *Server) ListNVMeNamespaces(_ context.Context, in *pb.ListNVMeNamespaces if in.PageSize > 0 && int(in.PageSize) < len(result) { log.Printf("Limiting result to: %d", in.PageSize) rr.Namespaces = rr.Namespaces[:in.PageSize] + token = uuid.New().String() + s.Pagination[token] = int(in.PageSize) } for j := range rr.Namespaces { r := &rr.Namespaces[j] @@ -489,7 +498,7 @@ func (s *Server) ListNVMeNamespaces(_ context.Context, in *pb.ListNVMeNamespaces } } if len(Blobarray) > 0 { - return &pb.ListNVMeNamespacesResponse{NvMeNamespaces: Blobarray}, nil + return &pb.ListNVMeNamespacesResponse{NvMeNamespaces: Blobarray, NextPageToken: token}, nil } msg := fmt.Sprintf("Could not find any namespaces for NQN: %s", nqn) diff --git a/pkg/frontend/nvme_test.go b/pkg/frontend/nvme_test.go index 40716f64..c7b47fc4 100644 --- a/pkg/frontend/nvme_test.go +++ b/pkg/frontend/nvme_test.go @@ -357,6 +357,10 @@ func TestFrontEnd_ListNVMeSubsystem(t *testing.T) { if !reflect.DeepEqual(response.NvMeSubsystems, tt.out) { t.Error("response: expected", tt.out, "received", response.NvMeSubsystems) } + // Empty NextPageToken indicates end of results list + if tt.size != 1 && response.NextPageToken != "" { + t.Error("response: expected", tt, "received", response) + } } if err != nil { @@ -1316,6 +1320,10 @@ func TestFrontEnd_ListNVMeNamespaces(t *testing.T) { if !reflect.DeepEqual(response.NvMeNamespaces, tt.out) { t.Error("response: expected", tt.out, "received", response.NvMeNamespaces) } + // Empty NextPageToken indicates end of results list + if tt.size != 1 && response.NextPageToken != "" { + t.Error("response: expected", tt, "received", response) + } } if err != nil { diff --git a/pkg/frontend/scsi.go b/pkg/frontend/scsi.go index 7c02d239..3e7258ab 100644 --- a/pkg/frontend/scsi.go +++ b/pkg/frontend/scsi.go @@ -14,6 +14,7 @@ import ( pb "github.com/opiproject/opi-api/storage/v1alpha1/gen/go" "github.com/opiproject/opi-spdk-bridge/pkg/models" + "github.com/google/uuid" "github.com/ulule/deepcopier" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" @@ -85,16 +86,19 @@ func (s *Server) ListVirtioScsiControllers(_ context.Context, in *pb.ListVirtioS return nil, err } log.Printf("Received from SPDK: %v", result) + var token string if in.PageSize > 0 && int(in.PageSize) < len(result) { log.Printf("Limiting result to: %d", in.PageSize) result = result[:in.PageSize] + token = uuid.New().String() + s.Pagination[token] = int(in.PageSize) } Blobarray := make([]*pb.VirtioScsiController, len(result)) for i := range result { r := &result[i] Blobarray[i] = &pb.VirtioScsiController{Id: &pc.ObjectKey{Value: r.Ctrlr}} } - return &pb.ListVirtioScsiControllersResponse{VirtioScsiControllers: Blobarray}, nil + return &pb.ListVirtioScsiControllersResponse{VirtioScsiControllers: Blobarray, NextPageToken: token}, nil } // GetVirtioScsiController gets a Virtio SCSI controller @@ -190,16 +194,19 @@ func (s *Server) ListVirtioScsiLuns(_ context.Context, in *pb.ListVirtioScsiLuns return nil, err } log.Printf("Received from SPDK: %v", result) + var token string if in.PageSize > 0 && int(in.PageSize) < len(result) { log.Printf("Limiting result to: %d", in.PageSize) result = result[:in.PageSize] + token = uuid.New().String() + s.Pagination[token] = int(in.PageSize) } Blobarray := make([]*pb.VirtioScsiLun, len(result)) for i := range result { r := &result[i] Blobarray[i] = &pb.VirtioScsiLun{VolumeId: &pc.ObjectKey{Value: r.Ctrlr}} } - return &pb.ListVirtioScsiLunsResponse{VirtioScsiLuns: Blobarray}, nil + return &pb.ListVirtioScsiLunsResponse{VirtioScsiLuns: Blobarray, NextPageToken: token}, nil } // GetVirtioScsiLun gets a Virtio SCSI LUN diff --git a/pkg/middleend/middleend.go b/pkg/middleend/middleend.go index f8f677fb..a4527672 100644 --- a/pkg/middleend/middleend.go +++ b/pkg/middleend/middleend.go @@ -17,6 +17,7 @@ import ( "github.com/opiproject/opi-spdk-bridge/pkg/models" "github.com/opiproject/opi-spdk-bridge/pkg/server" + "github.com/google/uuid" "github.com/ulule/deepcopier" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" @@ -27,14 +28,16 @@ import ( type Server struct { pb.UnimplementedMiddleendServiceServer - rpc server.JSONRPC + rpc server.JSONRPC + Pagination map[string]int } // NewServer creates initialized instance of MiddleEnd server communicating // with provided jsonRPC func NewServer(jsonRPC server.JSONRPC) *Server { return &Server{ - rpc: jsonRPC, + rpc: jsonRPC, + Pagination: make(map[string]int), } } @@ -236,16 +239,19 @@ func (s *Server) ListEncryptedVolumes(_ context.Context, in *pb.ListEncryptedVol return nil, err } log.Printf("Received from SPDK: %v", result) + var token string if in.PageSize > 0 && int(in.PageSize) < len(result) { log.Printf("Limiting result to: %d", in.PageSize) result = result[:in.PageSize] + token = uuid.New().String() + s.Pagination[token] = int(in.PageSize) } Blobarray := make([]*pb.EncryptedVolume, len(result)) for i := range result { r := &result[i] Blobarray[i] = &pb.EncryptedVolume{EncryptedVolumeId: &pc.ObjectKey{Value: r.Name}} } - return &pb.ListEncryptedVolumesResponse{EncryptedVolumes: Blobarray}, nil + return &pb.ListEncryptedVolumesResponse{EncryptedVolumes: Blobarray, NextPageToken: token}, nil } // GetEncryptedVolume gets an encrypted volume diff --git a/pkg/middleend/middleend_test.go b/pkg/middleend/middleend_test.go index a17b18e5..8aa65ea6 100644 --- a/pkg/middleend/middleend_test.go +++ b/pkg/middleend/middleend_test.go @@ -527,6 +527,10 @@ func TestMiddleEnd_ListEncryptedVolumes(t *testing.T) { if !reflect.DeepEqual(response.EncryptedVolumes, tt.out) { t.Error("response: expected", tt.out, "received", response.EncryptedVolumes) } + // Empty NextPageToken indicates end of results list + if tt.size != 1 && response.NextPageToken != "" { + t.Error("response: expected", tt, "received", response) + } } if err != nil { From bd54e62a33fa7fd2c63443b590c70ce1f43e81b7 Mon Sep 17 00:00:00 2001 From: Boris Glimcher Date: Mon, 10 Apr 2023 23:56:42 +0300 Subject: [PATCH 02/11] List: support PageToken in request Signed-off-by: Boris Glimcher --- pkg/backend/aio.go | 16 +++++++++++++--- pkg/backend/null.go | 16 +++++++++++++--- pkg/backend/nvme.go | 16 +++++++++++++--- pkg/frontend/blk.go | 16 +++++++++++++--- pkg/frontend/nvme.go | 32 ++++++++++++++++++++++++++------ pkg/frontend/scsi.go | 32 ++++++++++++++++++++++++++------ pkg/middleend/middleend.go | 16 +++++++++++++--- 7 files changed, 117 insertions(+), 27 deletions(-) diff --git a/pkg/backend/aio.go b/pkg/backend/aio.go index accaee11..12875f9c 100644 --- a/pkg/backend/aio.go +++ b/pkg/backend/aio.go @@ -142,6 +142,16 @@ func (s *Server) ListAioControllers(_ context.Context, in *pb.ListAioControllers log.Printf("error: %v", err) return nil, err } + offset := 0 + if in.PageToken != "" { + offset, ok := s.Pagination[in.PageToken] + if !ok { + err := status.Errorf(codes.NotFound, "unable to find pagination token %s", in.PageToken) + log.Printf("error: %v", err) + return nil, err + } + log.Printf("Found offset %d from pagination token: %s", offset, in.PageToken) + } var result []models.BdevGetBdevsResult err := s.rpc.Call("bdev_get_bdevs", nil, &result) if err != nil { @@ -151,10 +161,10 @@ func (s *Server) ListAioControllers(_ context.Context, in *pb.ListAioControllers log.Printf("Received from SPDK: %v", result) var token string if in.PageSize > 0 && int(in.PageSize) < len(result) { - log.Printf("Limiting result to: %d", in.PageSize) - result = result[:in.PageSize] + log.Printf("Limiting result to %d:%d", offset, in.PageSize) + result = result[offset:in.PageSize] token = uuid.New().String() - s.Pagination[token] = int(in.PageSize) + s.Pagination[token] = offset + int(in.PageSize) } Blobarray := make([]*pb.AioController, len(result)) for i := range result { diff --git a/pkg/backend/null.go b/pkg/backend/null.go index 3e3c5e6b..3b712778 100644 --- a/pkg/backend/null.go +++ b/pkg/backend/null.go @@ -142,6 +142,16 @@ func (s *Server) ListNullDebugs(_ context.Context, in *pb.ListNullDebugsRequest) log.Printf("error: %v", err) return nil, err } + offset := 0 + if in.PageToken != "" { + offset, ok := s.Pagination[in.PageToken] + if !ok { + err := status.Errorf(codes.NotFound, "unable to find pagination token %s", in.PageToken) + log.Printf("error: %v", err) + return nil, err + } + log.Printf("Found offset %d from pagination token: %s", offset, in.PageToken) + } var result []models.BdevGetBdevsResult err := s.rpc.Call("bdev_get_bdevs", nil, &result) if err != nil { @@ -151,10 +161,10 @@ func (s *Server) ListNullDebugs(_ context.Context, in *pb.ListNullDebugsRequest) log.Printf("Received from SPDK: %v", result) var token string if in.PageSize > 0 && int(in.PageSize) < len(result) { - log.Printf("Limiting result to: %d", in.PageSize) - result = result[:in.PageSize] + log.Printf("Limiting result to %d:%d", offset, in.PageSize) + result = result[offset:in.PageSize] token = uuid.New().String() - s.Pagination[token] = int(in.PageSize) + s.Pagination[token] = offset + int(in.PageSize) } Blobarray := make([]*pb.NullDebug, len(result)) for i := range result { diff --git a/pkg/backend/nvme.go b/pkg/backend/nvme.go index dd991d6d..9138c9d0 100644 --- a/pkg/backend/nvme.go +++ b/pkg/backend/nvme.go @@ -104,6 +104,16 @@ func (s *Server) ListNVMfRemoteControllers(_ context.Context, in *pb.ListNVMfRem log.Printf("error: %v", err) return nil, err } + offset := 0 + if in.PageToken != "" { + offset, ok := s.Pagination[in.PageToken] + if !ok { + err := status.Errorf(codes.NotFound, "unable to find pagination token %s", in.PageToken) + log.Printf("error: %v", err) + return nil, err + } + log.Printf("Found offset %d from pagination token: %s", offset, in.PageToken) + } var result []models.BdevNvmeGetControllerResult err := s.rpc.Call("bdev_nvme_get_controllers", nil, &result) if err != nil { @@ -113,10 +123,10 @@ func (s *Server) ListNVMfRemoteControllers(_ context.Context, in *pb.ListNVMfRem log.Printf("Received from SPDK: %v", result) var token string if in.PageSize > 0 && int(in.PageSize) < len(result) { - log.Printf("Limiting result to: %d", in.PageSize) - result = result[:in.PageSize] + log.Printf("Limiting result to %d:%d", offset, in.PageSize) + result = result[offset:in.PageSize] token = uuid.New().String() - s.Pagination[token] = int(in.PageSize) + s.Pagination[token] = offset + int(in.PageSize) } Blobarray := make([]*pb.NVMfRemoteController, len(result)) for i := range result { diff --git a/pkg/frontend/blk.go b/pkg/frontend/blk.go index f64b0d5f..0f001baa 100644 --- a/pkg/frontend/blk.go +++ b/pkg/frontend/blk.go @@ -101,6 +101,16 @@ func (s *Server) ListVirtioBlks(_ context.Context, in *pb.ListVirtioBlksRequest) log.Printf("error: %v", err) return nil, err } + offset := 0 + if in.PageToken != "" { + offset, ok := s.Pagination[in.PageToken] + if !ok { + err := status.Errorf(codes.NotFound, "unable to find pagination token %s", in.PageToken) + log.Printf("error: %v", err) + return nil, err + } + log.Printf("Found offset %d from pagination token: %s", offset, in.PageToken) + } var result []models.VhostGetControllersResult err := s.rpc.Call("vhost_get_controllers", nil, &result) if err != nil { @@ -110,10 +120,10 @@ func (s *Server) ListVirtioBlks(_ context.Context, in *pb.ListVirtioBlksRequest) log.Printf("Received from SPDK: %v", result) var token string if in.PageSize > 0 && int(in.PageSize) < len(result) { - log.Printf("Limiting result to: %d", in.PageSize) - result = result[:in.PageSize] + log.Printf("Limiting result to %d:%d", offset, in.PageSize) + result = result[offset:in.PageSize] token = uuid.New().String() - s.Pagination[token] = int(in.PageSize) + s.Pagination[token] = offset + int(in.PageSize) } Blobarray := make([]*pb.VirtioBlk, len(result)) for i := range result { diff --git a/pkg/frontend/nvme.go b/pkg/frontend/nvme.go index 8d45222f..6fd07b0b 100644 --- a/pkg/frontend/nvme.go +++ b/pkg/frontend/nvme.go @@ -166,6 +166,16 @@ func (s *Server) ListNVMeSubsystems(_ context.Context, in *pb.ListNVMeSubsystems log.Printf("error: %v", err) return nil, err } + offset := 0 + if in.PageToken != "" { + offset, ok := s.Pagination[in.PageToken] + if !ok { + err := status.Errorf(codes.NotFound, "unable to find pagination token %s", in.PageToken) + log.Printf("error: %v", err) + return nil, err + } + log.Printf("Found offset %d from pagination token: %s", offset, in.PageToken) + } var result []models.NvmfGetSubsystemsResult err := s.rpc.Call("nvmf_get_subsystems", nil, &result) if err != nil { @@ -175,10 +185,10 @@ func (s *Server) ListNVMeSubsystems(_ context.Context, in *pb.ListNVMeSubsystems log.Printf("Received from SPDK: %v", result) var token string if in.PageSize > 0 && int(in.PageSize) < len(result) { - log.Printf("Limiting result to: %d", in.PageSize) - result = result[:in.PageSize] + log.Printf("Limiting result to %d:%d", offset, in.PageSize) + result = result[offset:in.PageSize] token = uuid.New().String() - s.Pagination[token] = int(in.PageSize) + s.Pagination[token] = offset + int(in.PageSize) } Blobarray := make([]*pb.NVMeSubsystem, len(result)) for i := range result { @@ -463,6 +473,16 @@ func (s *Server) ListNVMeNamespaces(_ context.Context, in *pb.ListNVMeNamespaces log.Printf("error: %v", err) return nil, err } + offset := 0 + if in.PageToken != "" { + offset, ok := s.Pagination[in.PageToken] + if !ok { + err := status.Errorf(codes.NotFound, "unable to find pagination token %s", in.PageToken) + log.Printf("error: %v", err) + return nil, err + } + log.Printf("Found offset %d from pagination token: %s", offset, in.PageToken) + } nqn := "" if in.Parent != "" { subsys, ok := s.Nvme.Subsystems[in.Parent] @@ -486,10 +506,10 @@ func (s *Server) ListNVMeNamespaces(_ context.Context, in *pb.ListNVMeNamespaces rr := &result[i] if rr.Nqn == nqn || nqn == "" { if in.PageSize > 0 && int(in.PageSize) < len(result) { - log.Printf("Limiting result to: %d", in.PageSize) - rr.Namespaces = rr.Namespaces[:in.PageSize] + log.Printf("Limiting result to %d:%d", offset, in.PageSize) + rr.Namespaces = rr.Namespaces[offset:in.PageSize] token = uuid.New().String() - s.Pagination[token] = int(in.PageSize) + s.Pagination[token] = offset + int(in.PageSize) } for j := range rr.Namespaces { r := &rr.Namespaces[j] diff --git a/pkg/frontend/scsi.go b/pkg/frontend/scsi.go index 3e7258ab..c478b3b0 100644 --- a/pkg/frontend/scsi.go +++ b/pkg/frontend/scsi.go @@ -79,6 +79,16 @@ func (s *Server) ListVirtioScsiControllers(_ context.Context, in *pb.ListVirtioS log.Printf("error: %v", err) return nil, err } + offset := 0 + if in.PageToken != "" { + offset, ok := s.Pagination[in.PageToken] + if !ok { + err := status.Errorf(codes.NotFound, "unable to find pagination token %s", in.PageToken) + log.Printf("error: %v", err) + return nil, err + } + log.Printf("Found offset %d from pagination token: %s", offset, in.PageToken) + } var result []models.VhostGetControllersResult err := s.rpc.Call("vhost_get_controllers", nil, &result) if err != nil { @@ -88,10 +98,10 @@ func (s *Server) ListVirtioScsiControllers(_ context.Context, in *pb.ListVirtioS log.Printf("Received from SPDK: %v", result) var token string if in.PageSize > 0 && int(in.PageSize) < len(result) { - log.Printf("Limiting result to: %d", in.PageSize) - result = result[:in.PageSize] + log.Printf("Limiting result to %d:%d", offset, in.PageSize) + result = result[offset:in.PageSize] token = uuid.New().String() - s.Pagination[token] = int(in.PageSize) + s.Pagination[token] = offset + int(in.PageSize) } Blobarray := make([]*pb.VirtioScsiController, len(result)) for i := range result { @@ -187,6 +197,16 @@ func (s *Server) ListVirtioScsiLuns(_ context.Context, in *pb.ListVirtioScsiLuns log.Printf("error: %v", err) return nil, err } + offset := 0 + if in.PageToken != "" { + offset, ok := s.Pagination[in.PageToken] + if !ok { + err := status.Errorf(codes.NotFound, "unable to find pagination token %s", in.PageToken) + log.Printf("error: %v", err) + return nil, err + } + log.Printf("Found offset %d from pagination token: %s", offset, in.PageToken) + } var result []models.VhostGetControllersResult err := s.rpc.Call("vhost_get_controllers", nil, &result) if err != nil { @@ -196,10 +216,10 @@ func (s *Server) ListVirtioScsiLuns(_ context.Context, in *pb.ListVirtioScsiLuns log.Printf("Received from SPDK: %v", result) var token string if in.PageSize > 0 && int(in.PageSize) < len(result) { - log.Printf("Limiting result to: %d", in.PageSize) - result = result[:in.PageSize] + log.Printf("Limiting result to %d:%d", offset, in.PageSize) + result = result[offset:in.PageSize] token = uuid.New().String() - s.Pagination[token] = int(in.PageSize) + s.Pagination[token] = offset + int(in.PageSize) } Blobarray := make([]*pb.VirtioScsiLun, len(result)) for i := range result { diff --git a/pkg/middleend/middleend.go b/pkg/middleend/middleend.go index a4527672..adb43a02 100644 --- a/pkg/middleend/middleend.go +++ b/pkg/middleend/middleend.go @@ -232,6 +232,16 @@ func (s *Server) ListEncryptedVolumes(_ context.Context, in *pb.ListEncryptedVol log.Printf("error: %v", err) return nil, err } + offset := 0 + if in.PageToken != "" { + offset, ok := s.Pagination[in.PageToken] + if !ok { + err := status.Errorf(codes.NotFound, "unable to find pagination token %s", in.PageToken) + log.Printf("error: %v", err) + return nil, err + } + log.Printf("Found offset %d from pagination token: %s", offset, in.PageToken) + } var result []models.BdevGetBdevsResult err := s.rpc.Call("bdev_get_bdevs", nil, &result) if err != nil { @@ -241,10 +251,10 @@ func (s *Server) ListEncryptedVolumes(_ context.Context, in *pb.ListEncryptedVol log.Printf("Received from SPDK: %v", result) var token string if in.PageSize > 0 && int(in.PageSize) < len(result) { - log.Printf("Limiting result to: %d", in.PageSize) - result = result[:in.PageSize] + log.Printf("Limiting result to %d:%d", offset, in.PageSize) + result = result[offset:in.PageSize] token = uuid.New().String() - s.Pagination[token] = int(in.PageSize) + s.Pagination[token] = offset + int(in.PageSize) } Blobarray := make([]*pb.EncryptedVolume, len(result)) for i := range result { From 523b9c1f40a196da444d5306792ac031039e85ad Mon Sep 17 00:00:00 2001 From: Boris Glimcher Date: Tue, 11 Apr 2023 00:30:22 +0300 Subject: [PATCH 03/11] tests: add List PageToken Signed-off-by: Boris Glimcher --- pkg/backend/aio.go | 3 +- pkg/backend/aio_test.go | 24 +++++++++++++++- pkg/backend/null.go | 3 +- pkg/backend/null_test.go | 24 +++++++++++++++- pkg/backend/nvme.go | 3 +- pkg/backend/nvme_test.go | 24 +++++++++++++++- pkg/frontend/blk.go | 3 +- pkg/frontend/blk_test.go | 23 +++++++++++++++- pkg/frontend/nvme.go | 6 ++-- pkg/frontend/nvme_test.go | 49 +++++++++++++++++++++++++++++++-- pkg/frontend/scsi.go | 6 ++-- pkg/middleend/middleend.go | 3 +- pkg/middleend/middleend_test.go | 24 +++++++++++++++- 13 files changed, 178 insertions(+), 17 deletions(-) diff --git a/pkg/backend/aio.go b/pkg/backend/aio.go index 12875f9c..b5715301 100644 --- a/pkg/backend/aio.go +++ b/pkg/backend/aio.go @@ -144,7 +144,8 @@ func (s *Server) ListAioControllers(_ context.Context, in *pb.ListAioControllers } offset := 0 if in.PageToken != "" { - offset, ok := s.Pagination[in.PageToken] + var ok bool + offset, ok = s.Pagination[in.PageToken] if !ok { err := status.Errorf(codes.NotFound, "unable to find pagination token %s", in.PageToken) log.Printf("error: %v", err) diff --git a/pkg/backend/aio_test.go b/pkg/backend/aio_test.go index 2cd2de2d..cfd00216 100644 --- a/pkg/backend/aio_test.go +++ b/pkg/backend/aio_test.go @@ -238,6 +238,7 @@ func TestBackEnd_ListAioControllers(t *testing.T) { errMsg string start bool size int32 + token string }{ "valid request with invalid SPDK response": { "volume-test", @@ -247,6 +248,7 @@ func TestBackEnd_ListAioControllers(t *testing.T) { fmt.Sprintf("Could not find any namespaces for NQN: %v", "nqn.2022-09.io.spdk:opi3"), true, 0, + "", }, "valid request with invalid marshal SPDK response": { "volume-test", @@ -256,6 +258,7 @@ func TestBackEnd_ListAioControllers(t *testing.T) { fmt.Sprintf("bdev_get_bdevs: %v", "json: cannot unmarshal bool into Go value of type []models.BdevGetBdevsResult"), true, 0, + "", }, "valid request with empty SPDK response": { "volume-test", @@ -265,6 +268,7 @@ func TestBackEnd_ListAioControllers(t *testing.T) { fmt.Sprintf("bdev_get_bdevs: %v", "EOF"), true, 0, + "", }, "valid request with ID mismatch SPDK response": { "volume-test", @@ -274,6 +278,7 @@ func TestBackEnd_ListAioControllers(t *testing.T) { fmt.Sprintf("bdev_get_bdevs: %v", "json response ID mismatch"), true, 0, + "", }, "valid request with error code from SPDK response": { "volume-test", @@ -283,6 +288,7 @@ func TestBackEnd_ListAioControllers(t *testing.T) { fmt.Sprintf("bdev_get_bdevs: %v", "json response error: myopierr"), true, 0, + "", }, "valid request with valid SPDK response": { "volume-test", @@ -303,6 +309,7 @@ func TestBackEnd_ListAioControllers(t *testing.T) { "", true, 0, + "", }, "pagination overflow": { "volume-test", @@ -323,6 +330,7 @@ func TestBackEnd_ListAioControllers(t *testing.T) { "", true, 1000, + "", }, "pagination negative": { "volume-test", @@ -332,6 +340,17 @@ func TestBackEnd_ListAioControllers(t *testing.T) { "negative PageSize is not allowed", false, -10, + "", + }, + "pagination error": { + "volume-test", + nil, + []string{}, + codes.NotFound, + fmt.Sprintf("unable to find pagination token %s", "unknown-pagination-token"), + false, + 0, + "unknown-pagination-token", }, "pagination": { "volume-test", @@ -347,6 +366,7 @@ func TestBackEnd_ListAioControllers(t *testing.T) { "", true, 1, + "", }, } @@ -356,7 +376,9 @@ func TestBackEnd_ListAioControllers(t *testing.T) { testEnv := createTestEnvironment(tt.start, tt.spdk) defer testEnv.Close() - request := &pb.ListAioControllersRequest{Parent: tt.in, PageSize: tt.size} + testEnv.opiSpdkServer.Pagination["existing-pagination-token"] = 1 + + request := &pb.ListAioControllersRequest{Parent: tt.in, PageSize: tt.size, PageToken: tt.token} response, err := testEnv.client.ListAioControllers(testEnv.ctx, request) if response != nil { if !reflect.DeepEqual(response.AioControllers, tt.out) { diff --git a/pkg/backend/null.go b/pkg/backend/null.go index 3b712778..4484a03c 100644 --- a/pkg/backend/null.go +++ b/pkg/backend/null.go @@ -144,7 +144,8 @@ func (s *Server) ListNullDebugs(_ context.Context, in *pb.ListNullDebugsRequest) } offset := 0 if in.PageToken != "" { - offset, ok := s.Pagination[in.PageToken] + var ok bool + offset, ok = s.Pagination[in.PageToken] if !ok { err := status.Errorf(codes.NotFound, "unable to find pagination token %s", in.PageToken) log.Printf("error: %v", err) diff --git a/pkg/backend/null_test.go b/pkg/backend/null_test.go index a1f5b574..6c7cc430 100644 --- a/pkg/backend/null_test.go +++ b/pkg/backend/null_test.go @@ -237,6 +237,7 @@ func TestBackEnd_ListNullDebugs(t *testing.T) { errMsg string start bool size int32 + token string }{ "valid request with invalid SPDK response": { "volume-test", @@ -246,6 +247,7 @@ func TestBackEnd_ListNullDebugs(t *testing.T) { fmt.Sprintf("Could not find any namespaces for NQN: %v", "nqn.2022-09.io.spdk:opi3"), true, 0, + "", }, "valid request with invalid marshal SPDK response": { "volume-test", @@ -255,6 +257,7 @@ func TestBackEnd_ListNullDebugs(t *testing.T) { fmt.Sprintf("bdev_get_bdevs: %v", "json: cannot unmarshal bool into Go value of type []models.BdevGetBdevsResult"), true, 0, + "", }, "valid request with empty SPDK response": { "volume-test", @@ -264,6 +267,7 @@ func TestBackEnd_ListNullDebugs(t *testing.T) { fmt.Sprintf("bdev_get_bdevs: %v", "EOF"), true, 0, + "", }, "valid request with ID mismatch SPDK response": { "volume-test", @@ -273,6 +277,7 @@ func TestBackEnd_ListNullDebugs(t *testing.T) { fmt.Sprintf("bdev_get_bdevs: %v", "json response ID mismatch"), true, 0, + "", }, "valid request with error code from SPDK response": { "volume-test", @@ -282,6 +287,7 @@ func TestBackEnd_ListNullDebugs(t *testing.T) { fmt.Sprintf("bdev_get_bdevs: %v", "json response error: myopierr"), true, 0, + "", }, "valid request with valid SPDK response": { "volume-test", @@ -304,6 +310,7 @@ func TestBackEnd_ListNullDebugs(t *testing.T) { "", true, 0, + "", }, "pagination overflow": { "volume-test", @@ -326,6 +333,7 @@ func TestBackEnd_ListNullDebugs(t *testing.T) { "", true, 1000, + "", }, "pagination negative": { "volume-test", @@ -335,6 +343,17 @@ func TestBackEnd_ListNullDebugs(t *testing.T) { "negative PageSize is not allowed", false, -10, + "", + }, + "pagination error": { + "volume-test", + nil, + []string{}, + codes.NotFound, + fmt.Sprintf("unable to find pagination token %s", "unknown-pagination-token"), + false, + 0, + "unknown-pagination-token", }, "pagination": { "volume-test", @@ -351,6 +370,7 @@ func TestBackEnd_ListNullDebugs(t *testing.T) { "", true, 1, + "", }, } @@ -360,7 +380,9 @@ func TestBackEnd_ListNullDebugs(t *testing.T) { testEnv := createTestEnvironment(tt.start, tt.spdk) defer testEnv.Close() - request := &pb.ListNullDebugsRequest{Parent: tt.in, PageSize: tt.size} + testEnv.opiSpdkServer.Pagination["existing-pagination-token"] = 1 + + request := &pb.ListNullDebugsRequest{Parent: tt.in, PageSize: tt.size, PageToken: tt.token} response, err := testEnv.client.ListNullDebugs(testEnv.ctx, request) if response != nil { if !reflect.DeepEqual(response.NullDebugs, tt.out) { diff --git a/pkg/backend/nvme.go b/pkg/backend/nvme.go index 9138c9d0..5ef196b9 100644 --- a/pkg/backend/nvme.go +++ b/pkg/backend/nvme.go @@ -106,7 +106,8 @@ func (s *Server) ListNVMfRemoteControllers(_ context.Context, in *pb.ListNVMfRem } offset := 0 if in.PageToken != "" { - offset, ok := s.Pagination[in.PageToken] + var ok bool + offset, ok = s.Pagination[in.PageToken] if !ok { err := status.Errorf(codes.NotFound, "unable to find pagination token %s", in.PageToken) log.Printf("error: %v", err) diff --git a/pkg/backend/nvme_test.go b/pkg/backend/nvme_test.go index ca03376b..b26aee7e 100644 --- a/pkg/backend/nvme_test.go +++ b/pkg/backend/nvme_test.go @@ -171,6 +171,7 @@ func TestBackEnd_ListNVMfRemoteControllers(t *testing.T) { errMsg string start bool size int32 + token string }{ "valid request with invalid SPDK response": { "volume-test", @@ -180,6 +181,7 @@ func TestBackEnd_ListNVMfRemoteControllers(t *testing.T) { fmt.Sprintf("Could not find any namespaces for NQN: %v", "nqn.2022-09.io.spdk:opi3"), true, 0, + "", }, "valid request with invalid marshal SPDK response": { "volume-test", @@ -189,6 +191,7 @@ func TestBackEnd_ListNVMfRemoteControllers(t *testing.T) { fmt.Sprintf("bdev_nvme_get_controllers: %v", "json: cannot unmarshal bool into Go value of type []models.BdevNvmeGetControllerResult"), true, 0, + "", }, "valid request with empty SPDK response": { "volume-test", @@ -198,6 +201,7 @@ func TestBackEnd_ListNVMfRemoteControllers(t *testing.T) { fmt.Sprintf("bdev_nvme_get_controllers: %v", "EOF"), true, 0, + "", }, "valid request with ID mismatch SPDK response": { "volume-test", @@ -207,6 +211,7 @@ func TestBackEnd_ListNVMfRemoteControllers(t *testing.T) { fmt.Sprintf("bdev_nvme_get_controllers: %v", "json response ID mismatch"), true, 0, + "", }, "valid request with error code from SPDK response": { "volume-test", @@ -216,6 +221,7 @@ func TestBackEnd_ListNVMfRemoteControllers(t *testing.T) { fmt.Sprintf("bdev_nvme_get_controllers: %v", "json response error: myopierr"), true, 0, + "", }, "valid request with valid SPDK response": { "volume-test", @@ -244,6 +250,7 @@ func TestBackEnd_ListNVMfRemoteControllers(t *testing.T) { "", true, 0, + "", }, "pagination overflow": { "volume-test", @@ -272,6 +279,7 @@ func TestBackEnd_ListNVMfRemoteControllers(t *testing.T) { "", true, 1000, + "", }, "pagination negative": { "volume-test", @@ -281,6 +289,17 @@ func TestBackEnd_ListNVMfRemoteControllers(t *testing.T) { "negative PageSize is not allowed", false, -10, + "", + }, + "pagination error": { + "volume-test", + nil, + []string{}, + codes.NotFound, + fmt.Sprintf("unable to find pagination token %s", "unknown-pagination-token"), + false, + 0, + "unknown-pagination-token", }, "pagination": { "volume-test", @@ -300,6 +319,7 @@ func TestBackEnd_ListNVMfRemoteControllers(t *testing.T) { "", true, 1, + "", }, } @@ -309,7 +329,9 @@ func TestBackEnd_ListNVMfRemoteControllers(t *testing.T) { testEnv := createTestEnvironment(tt.start, tt.spdk) defer testEnv.Close() - request := &pb.ListNVMfRemoteControllersRequest{Parent: tt.in, PageSize: tt.size} + testEnv.opiSpdkServer.Pagination["existing-pagination-token"] = 1 + + request := &pb.ListNVMfRemoteControllersRequest{Parent: tt.in, PageSize: tt.size, PageToken: tt.token} response, err := testEnv.client.ListNVMfRemoteControllers(testEnv.ctx, request) if response != nil { if !reflect.DeepEqual(response.NvMfRemoteControllers, tt.out) { diff --git a/pkg/frontend/blk.go b/pkg/frontend/blk.go index 0f001baa..3979f6f6 100644 --- a/pkg/frontend/blk.go +++ b/pkg/frontend/blk.go @@ -103,7 +103,8 @@ func (s *Server) ListVirtioBlks(_ context.Context, in *pb.ListVirtioBlksRequest) } offset := 0 if in.PageToken != "" { - offset, ok := s.Pagination[in.PageToken] + var ok bool + offset, ok = s.Pagination[in.PageToken] if !ok { err := status.Errorf(codes.NotFound, "unable to find pagination token %s", in.PageToken) log.Printf("error: %v", err) diff --git a/pkg/frontend/blk_test.go b/pkg/frontend/blk_test.go index ce7e722f..30abb424 100644 --- a/pkg/frontend/blk_test.go +++ b/pkg/frontend/blk_test.go @@ -143,6 +143,7 @@ func TestFrontEnd_ListVirtioBlks(t *testing.T) { errMsg string start bool size int32 + token string }{ "valid request with invalid SPDK response": { "subsystem-test", @@ -152,6 +153,7 @@ func TestFrontEnd_ListVirtioBlks(t *testing.T) { fmt.Sprintf("Could not create NQN: %v", "nqn.2022-09.io.spdk:opi3"), true, 0, + "", }, "valid request with empty SPDK response": { "subsystem-test", @@ -161,6 +163,7 @@ func TestFrontEnd_ListVirtioBlks(t *testing.T) { fmt.Sprintf("vhost_get_controllers: %v", "EOF"), true, 0, + "", }, "valid request with ID mismatch SPDK response": { "subsystem-test", @@ -170,6 +173,7 @@ func TestFrontEnd_ListVirtioBlks(t *testing.T) { fmt.Sprintf("vhost_get_controllers: %v", "json response ID mismatch"), true, 0, + "", }, "valid request with error code from SPDK response": { "subsystem-test", @@ -179,6 +183,7 @@ func TestFrontEnd_ListVirtioBlks(t *testing.T) { fmt.Sprintf("vhost_get_controllers: %v", "json response error: myopierr"), true, 0, + "", }, "valid request with valid SPDK response": { "subsystem-test", @@ -204,6 +209,7 @@ func TestFrontEnd_ListVirtioBlks(t *testing.T) { "", true, 0, + "", }, "pagination overflow": { "subsystem-test", @@ -229,6 +235,7 @@ func TestFrontEnd_ListVirtioBlks(t *testing.T) { "", true, 1000, + "", }, "pagination negative": { "volume-test", @@ -238,6 +245,17 @@ func TestFrontEnd_ListVirtioBlks(t *testing.T) { "negative PageSize is not allowed", false, -10, + "", + }, + "pagination error": { + "volume-test", + nil, + []string{}, + codes.NotFound, + fmt.Sprintf("unable to find pagination token %s", "unknown-pagination-token"), + false, + 0, + "unknown-pagination-token", }, "pagination": { "subsystem-test", @@ -253,6 +271,7 @@ func TestFrontEnd_ListVirtioBlks(t *testing.T) { "", true, 1, + "", }, } @@ -262,7 +281,9 @@ func TestFrontEnd_ListVirtioBlks(t *testing.T) { testEnv := createTestEnvironment(tt.start, tt.spdk) defer testEnv.Close() - request := &pb.ListVirtioBlksRequest{Parent: tt.in, PageSize: tt.size} + testEnv.opiSpdkServer.Pagination["existing-pagination-token"] = 1 + + request := &pb.ListVirtioBlksRequest{Parent: tt.in, PageSize: tt.size, PageToken: tt.token} response, err := testEnv.client.ListVirtioBlks(testEnv.ctx, request) if response != nil { diff --git a/pkg/frontend/nvme.go b/pkg/frontend/nvme.go index 6fd07b0b..3451c6ee 100644 --- a/pkg/frontend/nvme.go +++ b/pkg/frontend/nvme.go @@ -168,7 +168,8 @@ func (s *Server) ListNVMeSubsystems(_ context.Context, in *pb.ListNVMeSubsystems } offset := 0 if in.PageToken != "" { - offset, ok := s.Pagination[in.PageToken] + var ok bool + offset, ok = s.Pagination[in.PageToken] if !ok { err := status.Errorf(codes.NotFound, "unable to find pagination token %s", in.PageToken) log.Printf("error: %v", err) @@ -475,7 +476,8 @@ func (s *Server) ListNVMeNamespaces(_ context.Context, in *pb.ListNVMeNamespaces } offset := 0 if in.PageToken != "" { - offset, ok := s.Pagination[in.PageToken] + var ok bool + offset, ok = s.Pagination[in.PageToken] if !ok { err := status.Errorf(codes.NotFound, "unable to find pagination token %s", in.PageToken) log.Printf("error: %v", err) diff --git a/pkg/frontend/nvme_test.go b/pkg/frontend/nvme_test.go index c7b47fc4..75192142 100644 --- a/pkg/frontend/nvme_test.go +++ b/pkg/frontend/nvme_test.go @@ -254,6 +254,7 @@ func TestFrontEnd_ListNVMeSubsystem(t *testing.T) { errMsg string start bool size int32 + token string }{ "valid request with invalid SPDK response": { nil, @@ -262,6 +263,7 @@ func TestFrontEnd_ListNVMeSubsystem(t *testing.T) { fmt.Sprintf("Could not create NQN: %v", "nqn.2022-09.io.spdk:opi3"), true, 0, + "", }, "valid request with empty SPDK response": { nil, @@ -270,6 +272,7 @@ func TestFrontEnd_ListNVMeSubsystem(t *testing.T) { fmt.Sprintf("nvmf_get_subsystems: %v", "EOF"), true, 0, + "", }, "valid request with ID mismatch SPDK response": { nil, @@ -278,6 +281,7 @@ func TestFrontEnd_ListNVMeSubsystem(t *testing.T) { fmt.Sprintf("nvmf_get_subsystems: %v", "json response ID mismatch"), true, 0, + "", }, "valid request with error code from SPDK response": { nil, @@ -286,6 +290,7 @@ func TestFrontEnd_ListNVMeSubsystem(t *testing.T) { fmt.Sprintf("nvmf_get_subsystems: %v", "json response error: myopierr"), true, 0, + "", }, "valid request with valid SPDK response": { []*pb.NVMeSubsystem{ @@ -317,6 +322,7 @@ func TestFrontEnd_ListNVMeSubsystem(t *testing.T) { "", true, 0, + "", }, "pagination negative": { nil, @@ -325,6 +331,16 @@ func TestFrontEnd_ListNVMeSubsystem(t *testing.T) { "negative PageSize is not allowed", false, -10, + "", + }, + "pagination error": { + nil, + []string{}, + codes.NotFound, + fmt.Sprintf("unable to find pagination token %s", "unknown-pagination-token"), + false, + 0, + "unknown-pagination-token", }, "pagination": { []*pb.NVMeSubsystem{ @@ -342,6 +358,7 @@ func TestFrontEnd_ListNVMeSubsystem(t *testing.T) { "", true, 1, + "", }, } @@ -351,7 +368,9 @@ func TestFrontEnd_ListNVMeSubsystem(t *testing.T) { testEnv := createTestEnvironment(tt.start, tt.spdk) defer testEnv.Close() - request := &pb.ListNVMeSubsystemsRequest{PageSize: tt.size} + testEnv.opiSpdkServer.Pagination["existing-pagination-token"] = 1 + + request := &pb.ListNVMeSubsystemsRequest{PageSize: tt.size, PageToken: tt.token} response, err := testEnv.client.ListNVMeSubsystems(testEnv.ctx, request) if response != nil { if !reflect.DeepEqual(response.NvMeSubsystems, tt.out) { @@ -792,6 +811,7 @@ func TestFrontEnd_ListNVMeControllers(t *testing.T) { errMsg string start bool size int32 + token string }{ "valid request without SPDK": { "subsystem-test", @@ -813,6 +833,7 @@ func TestFrontEnd_ListNVMeControllers(t *testing.T) { "", false, 0, + "", }, } @@ -824,7 +845,7 @@ func TestFrontEnd_ListNVMeControllers(t *testing.T) { testEnv.opiSpdkServer.Nvme.Subsystems[testSubsystem.Spec.Id.Value] = &testSubsystem testEnv.opiSpdkServer.Nvme.Controllers[testController.Spec.Id.Value] = &testController - request := &pb.ListNVMeControllersRequest{Parent: tt.in, PageSize: tt.size} + request := &pb.ListNVMeControllersRequest{Parent: tt.in, PageSize: tt.size, PageToken: tt.token} response, err := testEnv.client.ListNVMeControllers(testEnv.ctx, request) if response != nil { if !reflect.DeepEqual(response.NvMeControllers, tt.out) { @@ -1200,6 +1221,7 @@ func TestFrontEnd_ListNVMeNamespaces(t *testing.T) { errMsg string start bool size int32 + token string }{ "valid request with invalid SPDK response": { "subsystem-test", @@ -1209,6 +1231,7 @@ func TestFrontEnd_ListNVMeNamespaces(t *testing.T) { fmt.Sprintf("Could not find any namespaces for NQN: %v", "nqn.2022-09.io.spdk:opi3"), true, 0, + "", }, "valid request with invalid marshal SPDK response": { "subsystem-test", @@ -1218,6 +1241,7 @@ func TestFrontEnd_ListNVMeNamespaces(t *testing.T) { fmt.Sprintf("nvmf_get_subsystems: %v", "json: cannot unmarshal bool into Go value of type []models.NvmfGetSubsystemsResult"), true, 0, + "", }, "valid request with empty SPDK response": { "subsystem-test", @@ -1227,6 +1251,7 @@ func TestFrontEnd_ListNVMeNamespaces(t *testing.T) { fmt.Sprintf("nvmf_get_subsystems: %v", "EOF"), true, 0, + "", }, "valid request with ID mismatch SPDK response": { "subsystem-test", @@ -1236,6 +1261,7 @@ func TestFrontEnd_ListNVMeNamespaces(t *testing.T) { fmt.Sprintf("nvmf_get_subsystems: %v", "json response ID mismatch"), true, 0, + "", }, "valid request with error code from SPDK response": { "subsystem-test", @@ -1245,6 +1271,7 @@ func TestFrontEnd_ListNVMeNamespaces(t *testing.T) { fmt.Sprintf("nvmf_get_subsystems: %v", "json response error: myopierr"), true, 0, + "", }, "valid request with valid SPDK response": { "subsystem-test", @@ -1258,6 +1285,7 @@ func TestFrontEnd_ListNVMeNamespaces(t *testing.T) { "", true, 0, + "", }, "pagination overflow": { "subsystem-test", @@ -1271,6 +1299,7 @@ func TestFrontEnd_ListNVMeNamespaces(t *testing.T) { "", true, 1000, + "", }, "pagination negative": { "volume-test", @@ -1280,6 +1309,17 @@ func TestFrontEnd_ListNVMeNamespaces(t *testing.T) { "negative PageSize is not allowed", false, -10, + "", + }, + "pagination error": { + "volume-test", + nil, + []string{}, + codes.NotFound, + fmt.Sprintf("unable to find pagination token %s", "unknown-pagination-token"), + false, + 0, + "unknown-pagination-token", }, "pagination": { "subsystem-test", @@ -1291,6 +1331,7 @@ func TestFrontEnd_ListNVMeNamespaces(t *testing.T) { "", true, 1, + "", }, "valid request with unknown key": { "unknown-namespace-id", @@ -1300,6 +1341,7 @@ func TestFrontEnd_ListNVMeNamespaces(t *testing.T) { fmt.Sprintf("unable to find subsystem %v", "unknown-namespace-id"), false, 0, + "", }, } @@ -1313,8 +1355,9 @@ func TestFrontEnd_ListNVMeNamespaces(t *testing.T) { testEnv.opiSpdkServer.Nvme.Namespaces["ns0"] = &testNamespaces[0] testEnv.opiSpdkServer.Nvme.Namespaces["ns1"] = &testNamespaces[1] testEnv.opiSpdkServer.Nvme.Namespaces["ns2"] = &testNamespaces[2] + testEnv.opiSpdkServer.Pagination["existing-pagination-token"] = 1 - request := &pb.ListNVMeNamespacesRequest{Parent: tt.in, PageSize: tt.size} + request := &pb.ListNVMeNamespacesRequest{Parent: tt.in, PageSize: tt.size, PageToken: tt.token} response, err := testEnv.client.ListNVMeNamespaces(testEnv.ctx, request) if response != nil { if !reflect.DeepEqual(response.NvMeNamespaces, tt.out) { diff --git a/pkg/frontend/scsi.go b/pkg/frontend/scsi.go index c478b3b0..4e986ae3 100644 --- a/pkg/frontend/scsi.go +++ b/pkg/frontend/scsi.go @@ -81,7 +81,8 @@ func (s *Server) ListVirtioScsiControllers(_ context.Context, in *pb.ListVirtioS } offset := 0 if in.PageToken != "" { - offset, ok := s.Pagination[in.PageToken] + var ok bool + offset, ok = s.Pagination[in.PageToken] if !ok { err := status.Errorf(codes.NotFound, "unable to find pagination token %s", in.PageToken) log.Printf("error: %v", err) @@ -199,7 +200,8 @@ func (s *Server) ListVirtioScsiLuns(_ context.Context, in *pb.ListVirtioScsiLuns } offset := 0 if in.PageToken != "" { - offset, ok := s.Pagination[in.PageToken] + var ok bool + offset, ok = s.Pagination[in.PageToken] if !ok { err := status.Errorf(codes.NotFound, "unable to find pagination token %s", in.PageToken) log.Printf("error: %v", err) diff --git a/pkg/middleend/middleend.go b/pkg/middleend/middleend.go index adb43a02..61c8d802 100644 --- a/pkg/middleend/middleend.go +++ b/pkg/middleend/middleend.go @@ -234,7 +234,8 @@ func (s *Server) ListEncryptedVolumes(_ context.Context, in *pb.ListEncryptedVol } offset := 0 if in.PageToken != "" { - offset, ok := s.Pagination[in.PageToken] + var ok bool + offset, ok = s.Pagination[in.PageToken] if !ok { err := status.Errorf(codes.NotFound, "unable to find pagination token %s", in.PageToken) log.Printf("error: %v", err) diff --git a/pkg/middleend/middleend_test.go b/pkg/middleend/middleend_test.go index 8aa65ea6..3cf5c318 100644 --- a/pkg/middleend/middleend_test.go +++ b/pkg/middleend/middleend_test.go @@ -413,6 +413,7 @@ func TestMiddleEnd_ListEncryptedVolumes(t *testing.T) { errMsg string start bool size int32 + token string }{ "valid request with invalid SPDK response": { "volume-test", @@ -422,6 +423,7 @@ func TestMiddleEnd_ListEncryptedVolumes(t *testing.T) { fmt.Sprintf("Could not find any namespaces for NQN: %v", "nqn.2022-09.io.spdk:opi3"), true, 0, + "", }, "valid request with invalid marshal SPDK response": { "volume-test", @@ -431,6 +433,7 @@ func TestMiddleEnd_ListEncryptedVolumes(t *testing.T) { fmt.Sprintf("bdev_get_bdevs: %v", "json: cannot unmarshal bool into Go value of type []models.BdevGetBdevsResult"), true, 0, + "", }, "valid request with empty SPDK response": { "volume-test", @@ -440,6 +443,7 @@ func TestMiddleEnd_ListEncryptedVolumes(t *testing.T) { fmt.Sprintf("bdev_get_bdevs: %v", "EOF"), true, 0, + "", }, "valid request with ID mismatch SPDK response": { "volume-test", @@ -449,6 +453,7 @@ func TestMiddleEnd_ListEncryptedVolumes(t *testing.T) { fmt.Sprintf("bdev_get_bdevs: %v", "json response ID mismatch"), true, 0, + "", }, "valid request with error code from SPDK response": { "volume-test", @@ -458,6 +463,7 @@ func TestMiddleEnd_ListEncryptedVolumes(t *testing.T) { fmt.Sprintf("bdev_get_bdevs: %v", "json response error: myopierr"), true, 0, + "", }, "valid request with valid SPDK response": { "volume-test", @@ -474,6 +480,7 @@ func TestMiddleEnd_ListEncryptedVolumes(t *testing.T) { "", true, 0, + "", }, "pagination overflow": { "volume-test", @@ -490,6 +497,7 @@ func TestMiddleEnd_ListEncryptedVolumes(t *testing.T) { "", true, 1000, + "", }, "pagination negative": { "volume-test", @@ -499,6 +507,17 @@ func TestMiddleEnd_ListEncryptedVolumes(t *testing.T) { "negative PageSize is not allowed", false, -10, + "", + }, + "pagination error": { + "volume-test", + nil, + []string{}, + codes.NotFound, + fmt.Sprintf("unable to find pagination token %s", "unknown-pagination-token"), + false, + 0, + "unknown-pagination-token", }, "pagination": { "volume-test", @@ -512,6 +531,7 @@ func TestMiddleEnd_ListEncryptedVolumes(t *testing.T) { "", true, 1, + "", }, } @@ -521,7 +541,9 @@ func TestMiddleEnd_ListEncryptedVolumes(t *testing.T) { testEnv := createTestEnvironment(tt.start, tt.spdk) defer testEnv.Close() - request := &pb.ListEncryptedVolumesRequest{Parent: tt.in, PageSize: tt.size} + testEnv.opiSpdkServer.Pagination["existing-pagination-token"] = 1 + + request := &pb.ListEncryptedVolumesRequest{Parent: tt.in, PageSize: tt.size, PageToken: tt.token} response, err := testEnv.client.ListEncryptedVolumes(testEnv.ctx, request) if response != nil { if !reflect.DeepEqual(response.EncryptedVolumes, tt.out) { From 052229b1afa693ab2112cfd07db75cb0e6af1328 Mon Sep 17 00:00:00 2001 From: Boris Glimcher Date: Tue, 11 Apr 2023 15:03:34 +0300 Subject: [PATCH 04/11] List: limit in.PageSize to 50 by default According to Google AIP, if PageSize is zero means not provided by user, reasonable default value should be used Signed-off-by: Boris Glimcher --- pkg/backend/aio.go | 12 ++++++++---- pkg/backend/null.go | 12 ++++++++---- pkg/backend/nvme.go | 12 ++++++++---- pkg/frontend/blk.go | 12 ++++++++---- pkg/frontend/nvme.go | 24 ++++++++++++++++-------- pkg/frontend/scsi.go | 24 ++++++++++++++++-------- pkg/middleend/middleend.go | 12 ++++++++---- 7 files changed, 72 insertions(+), 36 deletions(-) diff --git a/pkg/backend/aio.go b/pkg/backend/aio.go index b5715301..bef272ca 100644 --- a/pkg/backend/aio.go +++ b/pkg/backend/aio.go @@ -142,6 +142,10 @@ func (s *Server) ListAioControllers(_ context.Context, in *pb.ListAioControllers log.Printf("error: %v", err) return nil, err } + size := 50 + if in.PageSize > 0 { + size = int(in.PageSize) + } offset := 0 if in.PageToken != "" { var ok bool @@ -161,11 +165,11 @@ func (s *Server) ListAioControllers(_ context.Context, in *pb.ListAioControllers } log.Printf("Received from SPDK: %v", result) var token string - if in.PageSize > 0 && int(in.PageSize) < len(result) { - log.Printf("Limiting result to %d:%d", offset, in.PageSize) - result = result[offset:in.PageSize] + if size < len(result) { + log.Printf("Limiting result to %d:%d", offset, size) + result = result[offset:size] token = uuid.New().String() - s.Pagination[token] = offset + int(in.PageSize) + s.Pagination[token] = offset + size } Blobarray := make([]*pb.AioController, len(result)) for i := range result { diff --git a/pkg/backend/null.go b/pkg/backend/null.go index 4484a03c..9a487bb2 100644 --- a/pkg/backend/null.go +++ b/pkg/backend/null.go @@ -142,6 +142,10 @@ func (s *Server) ListNullDebugs(_ context.Context, in *pb.ListNullDebugsRequest) log.Printf("error: %v", err) return nil, err } + size := 50 + if in.PageSize > 0 { + size = int(in.PageSize) + } offset := 0 if in.PageToken != "" { var ok bool @@ -161,11 +165,11 @@ func (s *Server) ListNullDebugs(_ context.Context, in *pb.ListNullDebugsRequest) } log.Printf("Received from SPDK: %v", result) var token string - if in.PageSize > 0 && int(in.PageSize) < len(result) { - log.Printf("Limiting result to %d:%d", offset, in.PageSize) - result = result[offset:in.PageSize] + if size < len(result) { + log.Printf("Limiting result to %d:%d", offset, size) + result = result[offset:size] token = uuid.New().String() - s.Pagination[token] = offset + int(in.PageSize) + s.Pagination[token] = offset + size } Blobarray := make([]*pb.NullDebug, len(result)) for i := range result { diff --git a/pkg/backend/nvme.go b/pkg/backend/nvme.go index 5ef196b9..10e75e57 100644 --- a/pkg/backend/nvme.go +++ b/pkg/backend/nvme.go @@ -104,6 +104,10 @@ func (s *Server) ListNVMfRemoteControllers(_ context.Context, in *pb.ListNVMfRem log.Printf("error: %v", err) return nil, err } + size := 50 + if in.PageSize > 0 { + size = int(in.PageSize) + } offset := 0 if in.PageToken != "" { var ok bool @@ -123,11 +127,11 @@ func (s *Server) ListNVMfRemoteControllers(_ context.Context, in *pb.ListNVMfRem } log.Printf("Received from SPDK: %v", result) var token string - if in.PageSize > 0 && int(in.PageSize) < len(result) { - log.Printf("Limiting result to %d:%d", offset, in.PageSize) - result = result[offset:in.PageSize] + if size < len(result) { + log.Printf("Limiting result to %d:%d", offset, size) + result = result[offset:size] token = uuid.New().String() - s.Pagination[token] = offset + int(in.PageSize) + s.Pagination[token] = offset + size } Blobarray := make([]*pb.NVMfRemoteController, len(result)) for i := range result { diff --git a/pkg/frontend/blk.go b/pkg/frontend/blk.go index 3979f6f6..6a899c9d 100644 --- a/pkg/frontend/blk.go +++ b/pkg/frontend/blk.go @@ -101,6 +101,10 @@ func (s *Server) ListVirtioBlks(_ context.Context, in *pb.ListVirtioBlksRequest) log.Printf("error: %v", err) return nil, err } + size := 50 + if in.PageSize > 0 { + size = int(in.PageSize) + } offset := 0 if in.PageToken != "" { var ok bool @@ -120,11 +124,11 @@ func (s *Server) ListVirtioBlks(_ context.Context, in *pb.ListVirtioBlksRequest) } log.Printf("Received from SPDK: %v", result) var token string - if in.PageSize > 0 && int(in.PageSize) < len(result) { - log.Printf("Limiting result to %d:%d", offset, in.PageSize) - result = result[offset:in.PageSize] + if size < len(result) { + log.Printf("Limiting result to %d:%d", offset, size) + result = result[offset:size] token = uuid.New().String() - s.Pagination[token] = offset + int(in.PageSize) + s.Pagination[token] = offset + size } Blobarray := make([]*pb.VirtioBlk, len(result)) for i := range result { diff --git a/pkg/frontend/nvme.go b/pkg/frontend/nvme.go index 3451c6ee..820ed862 100644 --- a/pkg/frontend/nvme.go +++ b/pkg/frontend/nvme.go @@ -166,6 +166,10 @@ func (s *Server) ListNVMeSubsystems(_ context.Context, in *pb.ListNVMeSubsystems log.Printf("error: %v", err) return nil, err } + size := 50 + if in.PageSize > 0 { + size = int(in.PageSize) + } offset := 0 if in.PageToken != "" { var ok bool @@ -185,11 +189,11 @@ func (s *Server) ListNVMeSubsystems(_ context.Context, in *pb.ListNVMeSubsystems } log.Printf("Received from SPDK: %v", result) var token string - if in.PageSize > 0 && int(in.PageSize) < len(result) { - log.Printf("Limiting result to %d:%d", offset, in.PageSize) - result = result[offset:in.PageSize] + if size < len(result) { + log.Printf("Limiting result to %d:%d", offset, size) + result = result[offset:size] token = uuid.New().String() - s.Pagination[token] = offset + int(in.PageSize) + s.Pagination[token] = offset + size } Blobarray := make([]*pb.NVMeSubsystem, len(result)) for i := range result { @@ -474,6 +478,10 @@ func (s *Server) ListNVMeNamespaces(_ context.Context, in *pb.ListNVMeNamespaces log.Printf("error: %v", err) return nil, err } + size := 50 + if in.PageSize > 0 { + size = int(in.PageSize) + } offset := 0 if in.PageToken != "" { var ok bool @@ -507,11 +515,11 @@ func (s *Server) ListNVMeNamespaces(_ context.Context, in *pb.ListNVMeNamespaces for i := range result { rr := &result[i] if rr.Nqn == nqn || nqn == "" { - if in.PageSize > 0 && int(in.PageSize) < len(result) { - log.Printf("Limiting result to %d:%d", offset, in.PageSize) - rr.Namespaces = rr.Namespaces[offset:in.PageSize] + if size < len(result) { + log.Printf("Limiting result to %d:%d", offset, size) + rr.Namespaces = rr.Namespaces[offset:size] token = uuid.New().String() - s.Pagination[token] = offset + int(in.PageSize) + s.Pagination[token] = offset + size } for j := range rr.Namespaces { r := &rr.Namespaces[j] diff --git a/pkg/frontend/scsi.go b/pkg/frontend/scsi.go index 4e986ae3..5fb35f2d 100644 --- a/pkg/frontend/scsi.go +++ b/pkg/frontend/scsi.go @@ -79,6 +79,10 @@ func (s *Server) ListVirtioScsiControllers(_ context.Context, in *pb.ListVirtioS log.Printf("error: %v", err) return nil, err } + size := 50 + if in.PageSize > 0 { + size = int(in.PageSize) + } offset := 0 if in.PageToken != "" { var ok bool @@ -98,11 +102,11 @@ func (s *Server) ListVirtioScsiControllers(_ context.Context, in *pb.ListVirtioS } log.Printf("Received from SPDK: %v", result) var token string - if in.PageSize > 0 && int(in.PageSize) < len(result) { - log.Printf("Limiting result to %d:%d", offset, in.PageSize) - result = result[offset:in.PageSize] + if size < len(result) { + log.Printf("Limiting result to %d:%d", offset, size) + result = result[offset:size] token = uuid.New().String() - s.Pagination[token] = offset + int(in.PageSize) + s.Pagination[token] = offset + size } Blobarray := make([]*pb.VirtioScsiController, len(result)) for i := range result { @@ -198,6 +202,10 @@ func (s *Server) ListVirtioScsiLuns(_ context.Context, in *pb.ListVirtioScsiLuns log.Printf("error: %v", err) return nil, err } + size := 50 + if in.PageSize > 0 { + size = int(in.PageSize) + } offset := 0 if in.PageToken != "" { var ok bool @@ -217,11 +225,11 @@ func (s *Server) ListVirtioScsiLuns(_ context.Context, in *pb.ListVirtioScsiLuns } log.Printf("Received from SPDK: %v", result) var token string - if in.PageSize > 0 && int(in.PageSize) < len(result) { - log.Printf("Limiting result to %d:%d", offset, in.PageSize) - result = result[offset:in.PageSize] + if size < len(result) { + log.Printf("Limiting result to %d:%d", offset, size) + result = result[offset:size] token = uuid.New().String() - s.Pagination[token] = offset + int(in.PageSize) + s.Pagination[token] = offset + size } Blobarray := make([]*pb.VirtioScsiLun, len(result)) for i := range result { diff --git a/pkg/middleend/middleend.go b/pkg/middleend/middleend.go index 61c8d802..dd596714 100644 --- a/pkg/middleend/middleend.go +++ b/pkg/middleend/middleend.go @@ -232,6 +232,10 @@ func (s *Server) ListEncryptedVolumes(_ context.Context, in *pb.ListEncryptedVol log.Printf("error: %v", err) return nil, err } + size := 50 + if in.PageSize > 0 { + size = int(in.PageSize) + } offset := 0 if in.PageToken != "" { var ok bool @@ -251,11 +255,11 @@ func (s *Server) ListEncryptedVolumes(_ context.Context, in *pb.ListEncryptedVol } log.Printf("Received from SPDK: %v", result) var token string - if in.PageSize > 0 && int(in.PageSize) < len(result) { - log.Printf("Limiting result to %d:%d", offset, in.PageSize) - result = result[offset:in.PageSize] + if size < len(result) { + log.Printf("Limiting result to %d:%d", offset, size) + result = result[offset:size] token = uuid.New().String() - s.Pagination[token] = offset + int(in.PageSize) + s.Pagination[token] = offset + size } Blobarray := make([]*pb.EncryptedVolume, len(result)) for i := range result { From 3b7f67a3b85ab9bfdbd4d31d1994e8d2eccd1925 Mon Sep 17 00:00:00 2001 From: Boris Glimcher Date: Wed, 12 Apr 2023 03:38:58 +0300 Subject: [PATCH 05/11] List: limit in.PageSize to 250 by max According to Google AIP, max PageSize value shoould be limited to a reasonable max value Signed-off-by: Boris Glimcher --- pkg/backend/aio.go | 3 ++- pkg/backend/null.go | 3 ++- pkg/backend/nvme.go | 3 ++- pkg/frontend/blk.go | 3 ++- pkg/frontend/nvme.go | 5 +++-- pkg/frontend/scsi.go | 5 +++-- pkg/middleend/middleend.go | 3 ++- 7 files changed, 16 insertions(+), 9 deletions(-) diff --git a/pkg/backend/aio.go b/pkg/backend/aio.go index bef272ca..d4da4c90 100644 --- a/pkg/backend/aio.go +++ b/pkg/backend/aio.go @@ -9,6 +9,7 @@ import ( "context" "fmt" "log" + "math" pc "github.com/opiproject/opi-api/common/v1/gen/go" pb "github.com/opiproject/opi-api/storage/v1alpha1/gen/go" @@ -144,7 +145,7 @@ func (s *Server) ListAioControllers(_ context.Context, in *pb.ListAioControllers } size := 50 if in.PageSize > 0 { - size = int(in.PageSize) + size = int(math.Min(float64(in.PageSize), 250)) } offset := 0 if in.PageToken != "" { diff --git a/pkg/backend/null.go b/pkg/backend/null.go index 9a487bb2..b83d6d83 100644 --- a/pkg/backend/null.go +++ b/pkg/backend/null.go @@ -9,6 +9,7 @@ import ( "context" "fmt" "log" + "math" pc "github.com/opiproject/opi-api/common/v1/gen/go" pb "github.com/opiproject/opi-api/storage/v1alpha1/gen/go" @@ -144,7 +145,7 @@ func (s *Server) ListNullDebugs(_ context.Context, in *pb.ListNullDebugsRequest) } size := 50 if in.PageSize > 0 { - size = int(in.PageSize) + size = int(math.Min(float64(in.PageSize), 250)) } offset := 0 if in.PageToken != "" { diff --git a/pkg/backend/nvme.go b/pkg/backend/nvme.go index 10e75e57..067e9fd8 100644 --- a/pkg/backend/nvme.go +++ b/pkg/backend/nvme.go @@ -8,6 +8,7 @@ import ( "context" "fmt" "log" + "math" "strconv" "strings" @@ -106,7 +107,7 @@ func (s *Server) ListNVMfRemoteControllers(_ context.Context, in *pb.ListNVMfRem } size := 50 if in.PageSize > 0 { - size = int(in.PageSize) + size = int(math.Min(float64(in.PageSize), 250)) } offset := 0 if in.PageToken != "" { diff --git a/pkg/frontend/blk.go b/pkg/frontend/blk.go index 6a899c9d..98b49a1b 100644 --- a/pkg/frontend/blk.go +++ b/pkg/frontend/blk.go @@ -9,6 +9,7 @@ import ( "context" "fmt" "log" + "math" "github.com/google/uuid" pc "github.com/opiproject/opi-api/common/v1/gen/go" @@ -103,7 +104,7 @@ func (s *Server) ListVirtioBlks(_ context.Context, in *pb.ListVirtioBlksRequest) } size := 50 if in.PageSize > 0 { - size = int(in.PageSize) + size = int(math.Min(float64(in.PageSize), 250)) } offset := 0 if in.PageToken != "" { diff --git a/pkg/frontend/nvme.go b/pkg/frontend/nvme.go index 820ed862..a2fe65ef 100644 --- a/pkg/frontend/nvme.go +++ b/pkg/frontend/nvme.go @@ -9,6 +9,7 @@ import ( "context" "fmt" "log" + "math" "net" pc "github.com/opiproject/opi-api/common/v1/gen/go" @@ -168,7 +169,7 @@ func (s *Server) ListNVMeSubsystems(_ context.Context, in *pb.ListNVMeSubsystems } size := 50 if in.PageSize > 0 { - size = int(in.PageSize) + size = int(math.Min(float64(in.PageSize), 250)) } offset := 0 if in.PageToken != "" { @@ -480,7 +481,7 @@ func (s *Server) ListNVMeNamespaces(_ context.Context, in *pb.ListNVMeNamespaces } size := 50 if in.PageSize > 0 { - size = int(in.PageSize) + size = int(math.Min(float64(in.PageSize), 250)) } offset := 0 if in.PageToken != "" { diff --git a/pkg/frontend/scsi.go b/pkg/frontend/scsi.go index 5fb35f2d..6dc43878 100644 --- a/pkg/frontend/scsi.go +++ b/pkg/frontend/scsi.go @@ -9,6 +9,7 @@ import ( "context" "fmt" "log" + "math" pc "github.com/opiproject/opi-api/common/v1/gen/go" pb "github.com/opiproject/opi-api/storage/v1alpha1/gen/go" @@ -81,7 +82,7 @@ func (s *Server) ListVirtioScsiControllers(_ context.Context, in *pb.ListVirtioS } size := 50 if in.PageSize > 0 { - size = int(in.PageSize) + size = int(math.Min(float64(in.PageSize), 250)) } offset := 0 if in.PageToken != "" { @@ -204,7 +205,7 @@ func (s *Server) ListVirtioScsiLuns(_ context.Context, in *pb.ListVirtioScsiLuns } size := 50 if in.PageSize > 0 { - size = int(in.PageSize) + size = int(math.Min(float64(in.PageSize), 250)) } offset := 0 if in.PageToken != "" { diff --git a/pkg/middleend/middleend.go b/pkg/middleend/middleend.go index dd596714..112c3547 100644 --- a/pkg/middleend/middleend.go +++ b/pkg/middleend/middleend.go @@ -9,6 +9,7 @@ import ( "context" "fmt" "log" + "math" "regexp" "strings" @@ -234,7 +235,7 @@ func (s *Server) ListEncryptedVolumes(_ context.Context, in *pb.ListEncryptedVol } size := 50 if in.PageSize > 0 { - size = int(in.PageSize) + size = int(math.Min(float64(in.PageSize), 250)) } offset := 0 if in.PageToken != "" { From fe587f72117698180c702a9d8353fe52393a4965 Mon Sep 17 00:00:00 2001 From: Boris Glimcher Date: Wed, 12 Apr 2023 04:33:27 +0300 Subject: [PATCH 06/11] List: remove code duplication introduce new ExtractPagination function Signed-off-by: Boris Glimcher --- pkg/backend/aio.go | 25 ++++---------------- pkg/backend/null.go | 25 ++++---------------- pkg/backend/nvme.go | 25 ++++---------------- pkg/frontend/blk.go | 24 ++++--------------- pkg/frontend/nvme.go | 48 +++++++------------------------------- pkg/frontend/scsi.go | 48 +++++++------------------------------- pkg/middleend/middleend.go | 24 ++++--------------- pkg/server/utils.go | 28 ++++++++++++++++++++++ 8 files changed, 69 insertions(+), 178 deletions(-) diff --git a/pkg/backend/aio.go b/pkg/backend/aio.go index d4da4c90..bdf38f98 100644 --- a/pkg/backend/aio.go +++ b/pkg/backend/aio.go @@ -9,11 +9,11 @@ import ( "context" "fmt" "log" - "math" pc "github.com/opiproject/opi-api/common/v1/gen/go" pb "github.com/opiproject/opi-api/storage/v1alpha1/gen/go" "github.com/opiproject/opi-spdk-bridge/pkg/models" + "github.com/opiproject/opi-spdk-bridge/pkg/server" "github.com/google/uuid" "github.com/ulule/deepcopier" @@ -138,25 +138,10 @@ func (s *Server) UpdateAioController(_ context.Context, in *pb.UpdateAioControll // ListAioControllers lists Aio controllers func (s *Server) ListAioControllers(_ context.Context, in *pb.ListAioControllersRequest) (*pb.ListAioControllersResponse, error) { log.Printf("ListAioControllers: Received from client: %v", in) - if in.PageSize < 0 { - err := status.Error(codes.InvalidArgument, "negative PageSize is not allowed") - log.Printf("error: %v", err) - return nil, err - } - size := 50 - if in.PageSize > 0 { - size = int(math.Min(float64(in.PageSize), 250)) - } - offset := 0 - if in.PageToken != "" { - var ok bool - offset, ok = s.Pagination[in.PageToken] - if !ok { - err := status.Errorf(codes.NotFound, "unable to find pagination token %s", in.PageToken) - log.Printf("error: %v", err) - return nil, err - } - log.Printf("Found offset %d from pagination token: %s", offset, in.PageToken) + size, offset, perr := server.ExtractPagination(in.PageSize, in.PageToken, s.Pagination) + if perr != nil { + log.Printf("error: %v", perr) + return nil, perr } var result []models.BdevGetBdevsResult err := s.rpc.Call("bdev_get_bdevs", nil, &result) diff --git a/pkg/backend/null.go b/pkg/backend/null.go index b83d6d83..4028cb16 100644 --- a/pkg/backend/null.go +++ b/pkg/backend/null.go @@ -9,11 +9,11 @@ import ( "context" "fmt" "log" - "math" pc "github.com/opiproject/opi-api/common/v1/gen/go" pb "github.com/opiproject/opi-api/storage/v1alpha1/gen/go" "github.com/opiproject/opi-spdk-bridge/pkg/models" + "github.com/opiproject/opi-spdk-bridge/pkg/server" "github.com/google/uuid" "github.com/ulule/deepcopier" @@ -138,25 +138,10 @@ func (s *Server) UpdateNullDebug(_ context.Context, in *pb.UpdateNullDebugReques // ListNullDebugs lists Null Debug instances func (s *Server) ListNullDebugs(_ context.Context, in *pb.ListNullDebugsRequest) (*pb.ListNullDebugsResponse, error) { log.Printf("ListNullDebugs: Received from client: %v", in) - if in.PageSize < 0 { - err := status.Error(codes.InvalidArgument, "negative PageSize is not allowed") - log.Printf("error: %v", err) - return nil, err - } - size := 50 - if in.PageSize > 0 { - size = int(math.Min(float64(in.PageSize), 250)) - } - offset := 0 - if in.PageToken != "" { - var ok bool - offset, ok = s.Pagination[in.PageToken] - if !ok { - err := status.Errorf(codes.NotFound, "unable to find pagination token %s", in.PageToken) - log.Printf("error: %v", err) - return nil, err - } - log.Printf("Found offset %d from pagination token: %s", offset, in.PageToken) + size, offset, perr := server.ExtractPagination(in.PageSize, in.PageToken, s.Pagination) + if perr != nil { + log.Printf("error: %v", perr) + return nil, perr } var result []models.BdevGetBdevsResult err := s.rpc.Call("bdev_get_bdevs", nil, &result) diff --git a/pkg/backend/nvme.go b/pkg/backend/nvme.go index 067e9fd8..0f28344c 100644 --- a/pkg/backend/nvme.go +++ b/pkg/backend/nvme.go @@ -8,13 +8,13 @@ import ( "context" "fmt" "log" - "math" "strconv" "strings" pc "github.com/opiproject/opi-api/common/v1/gen/go" pb "github.com/opiproject/opi-api/storage/v1alpha1/gen/go" "github.com/opiproject/opi-spdk-bridge/pkg/models" + "github.com/opiproject/opi-spdk-bridge/pkg/server" "github.com/google/uuid" "github.com/ulule/deepcopier" @@ -100,25 +100,10 @@ func (s *Server) NVMfRemoteControllerReset(_ context.Context, in *pb.NVMfRemoteC // ListNVMfRemoteControllers lists an NVMf remote controllers func (s *Server) ListNVMfRemoteControllers(_ context.Context, in *pb.ListNVMfRemoteControllersRequest) (*pb.ListNVMfRemoteControllersResponse, error) { log.Printf("ListNVMfRemoteControllers: Received from client: %v", in) - if in.PageSize < 0 { - err := status.Error(codes.InvalidArgument, "negative PageSize is not allowed") - log.Printf("error: %v", err) - return nil, err - } - size := 50 - if in.PageSize > 0 { - size = int(math.Min(float64(in.PageSize), 250)) - } - offset := 0 - if in.PageToken != "" { - var ok bool - offset, ok = s.Pagination[in.PageToken] - if !ok { - err := status.Errorf(codes.NotFound, "unable to find pagination token %s", in.PageToken) - log.Printf("error: %v", err) - return nil, err - } - log.Printf("Found offset %d from pagination token: %s", offset, in.PageToken) + size, offset, perr := server.ExtractPagination(in.PageSize, in.PageToken, s.Pagination) + if perr != nil { + log.Printf("error: %v", perr) + return nil, perr } var result []models.BdevNvmeGetControllerResult err := s.rpc.Call("bdev_nvme_get_controllers", nil, &result) diff --git a/pkg/frontend/blk.go b/pkg/frontend/blk.go index 98b49a1b..5a4fda85 100644 --- a/pkg/frontend/blk.go +++ b/pkg/frontend/blk.go @@ -9,7 +9,6 @@ import ( "context" "fmt" "log" - "math" "github.com/google/uuid" pc "github.com/opiproject/opi-api/common/v1/gen/go" @@ -97,25 +96,10 @@ func (s *Server) UpdateVirtioBlk(_ context.Context, in *pb.UpdateVirtioBlkReques // ListVirtioBlks lists Virtio block devices func (s *Server) ListVirtioBlks(_ context.Context, in *pb.ListVirtioBlksRequest) (*pb.ListVirtioBlksResponse, error) { log.Printf("ListVirtioBlks: Received from client: %v", in) - if in.PageSize < 0 { - err := status.Error(codes.InvalidArgument, "negative PageSize is not allowed") - log.Printf("error: %v", err) - return nil, err - } - size := 50 - if in.PageSize > 0 { - size = int(math.Min(float64(in.PageSize), 250)) - } - offset := 0 - if in.PageToken != "" { - var ok bool - offset, ok = s.Pagination[in.PageToken] - if !ok { - err := status.Errorf(codes.NotFound, "unable to find pagination token %s", in.PageToken) - log.Printf("error: %v", err) - return nil, err - } - log.Printf("Found offset %d from pagination token: %s", offset, in.PageToken) + size, offset, perr := server.ExtractPagination(in.PageSize, in.PageToken, s.Pagination) + if perr != nil { + log.Printf("error: %v", perr) + return nil, perr } var result []models.VhostGetControllersResult err := s.rpc.Call("vhost_get_controllers", nil, &result) diff --git a/pkg/frontend/nvme.go b/pkg/frontend/nvme.go index a2fe65ef..496bd1ca 100644 --- a/pkg/frontend/nvme.go +++ b/pkg/frontend/nvme.go @@ -9,12 +9,12 @@ import ( "context" "fmt" "log" - "math" "net" pc "github.com/opiproject/opi-api/common/v1/gen/go" pb "github.com/opiproject/opi-api/storage/v1alpha1/gen/go" "github.com/opiproject/opi-spdk-bridge/pkg/models" + "github.com/opiproject/opi-spdk-bridge/pkg/server" "github.com/google/uuid" "github.com/ulule/deepcopier" @@ -162,25 +162,10 @@ func (s *Server) UpdateNVMeSubsystem(_ context.Context, in *pb.UpdateNVMeSubsyst // ListNVMeSubsystems lists NVMe Subsystems func (s *Server) ListNVMeSubsystems(_ context.Context, in *pb.ListNVMeSubsystemsRequest) (*pb.ListNVMeSubsystemsResponse, error) { log.Printf("ListNVMeSubsystems: Received from client: %v", in) - if in.PageSize < 0 { - err := status.Error(codes.InvalidArgument, "negative PageSize is not allowed") - log.Printf("error: %v", err) - return nil, err - } - size := 50 - if in.PageSize > 0 { - size = int(math.Min(float64(in.PageSize), 250)) - } - offset := 0 - if in.PageToken != "" { - var ok bool - offset, ok = s.Pagination[in.PageToken] - if !ok { - err := status.Errorf(codes.NotFound, "unable to find pagination token %s", in.PageToken) - log.Printf("error: %v", err) - return nil, err - } - log.Printf("Found offset %d from pagination token: %s", offset, in.PageToken) + size, offset, perr := server.ExtractPagination(in.PageSize, in.PageToken, s.Pagination) + if perr != nil { + log.Printf("error: %v", perr) + return nil, perr } var result []models.NvmfGetSubsystemsResult err := s.rpc.Call("nvmf_get_subsystems", nil, &result) @@ -474,25 +459,10 @@ func (s *Server) UpdateNVMeNamespace(_ context.Context, in *pb.UpdateNVMeNamespa // ListNVMeNamespaces lists NVMe namespaces func (s *Server) ListNVMeNamespaces(_ context.Context, in *pb.ListNVMeNamespacesRequest) (*pb.ListNVMeNamespacesResponse, error) { log.Printf("ListNVMeNamespaces: Received from client: %v", in) - if in.PageSize < 0 { - err := status.Error(codes.InvalidArgument, "negative PageSize is not allowed") - log.Printf("error: %v", err) - return nil, err - } - size := 50 - if in.PageSize > 0 { - size = int(math.Min(float64(in.PageSize), 250)) - } - offset := 0 - if in.PageToken != "" { - var ok bool - offset, ok = s.Pagination[in.PageToken] - if !ok { - err := status.Errorf(codes.NotFound, "unable to find pagination token %s", in.PageToken) - log.Printf("error: %v", err) - return nil, err - } - log.Printf("Found offset %d from pagination token: %s", offset, in.PageToken) + size, offset, perr := server.ExtractPagination(in.PageSize, in.PageToken, s.Pagination) + if perr != nil { + log.Printf("error: %v", perr) + return nil, perr } nqn := "" if in.Parent != "" { diff --git a/pkg/frontend/scsi.go b/pkg/frontend/scsi.go index 6dc43878..a5a7d83d 100644 --- a/pkg/frontend/scsi.go +++ b/pkg/frontend/scsi.go @@ -9,11 +9,11 @@ import ( "context" "fmt" "log" - "math" pc "github.com/opiproject/opi-api/common/v1/gen/go" pb "github.com/opiproject/opi-api/storage/v1alpha1/gen/go" "github.com/opiproject/opi-spdk-bridge/pkg/models" + "github.com/opiproject/opi-spdk-bridge/pkg/server" "github.com/google/uuid" "github.com/ulule/deepcopier" @@ -75,25 +75,10 @@ func (s *Server) UpdateVirtioScsiController(_ context.Context, in *pb.UpdateVirt // ListVirtioScsiControllers lists Virtio SCSI controllers func (s *Server) ListVirtioScsiControllers(_ context.Context, in *pb.ListVirtioScsiControllersRequest) (*pb.ListVirtioScsiControllersResponse, error) { log.Printf("ListVirtioScsiControllers: Received from client: %v", in) - if in.PageSize < 0 { - err := status.Error(codes.InvalidArgument, "negative PageSize is not allowed") - log.Printf("error: %v", err) - return nil, err - } - size := 50 - if in.PageSize > 0 { - size = int(math.Min(float64(in.PageSize), 250)) - } - offset := 0 - if in.PageToken != "" { - var ok bool - offset, ok = s.Pagination[in.PageToken] - if !ok { - err := status.Errorf(codes.NotFound, "unable to find pagination token %s", in.PageToken) - log.Printf("error: %v", err) - return nil, err - } - log.Printf("Found offset %d from pagination token: %s", offset, in.PageToken) + size, offset, perr := server.ExtractPagination(in.PageSize, in.PageToken, s.Pagination) + if perr != nil { + log.Printf("error: %v", perr) + return nil, perr } var result []models.VhostGetControllersResult err := s.rpc.Call("vhost_get_controllers", nil, &result) @@ -198,25 +183,10 @@ func (s *Server) UpdateVirtioScsiLun(_ context.Context, in *pb.UpdateVirtioScsiL // ListVirtioScsiLuns lists Virtio SCSI LUNs func (s *Server) ListVirtioScsiLuns(_ context.Context, in *pb.ListVirtioScsiLunsRequest) (*pb.ListVirtioScsiLunsResponse, error) { log.Printf("ListVirtioScsiLuns: Received from client: %v", in) - if in.PageSize < 0 { - err := status.Error(codes.InvalidArgument, "negative PageSize is not allowed") - log.Printf("error: %v", err) - return nil, err - } - size := 50 - if in.PageSize > 0 { - size = int(math.Min(float64(in.PageSize), 250)) - } - offset := 0 - if in.PageToken != "" { - var ok bool - offset, ok = s.Pagination[in.PageToken] - if !ok { - err := status.Errorf(codes.NotFound, "unable to find pagination token %s", in.PageToken) - log.Printf("error: %v", err) - return nil, err - } - log.Printf("Found offset %d from pagination token: %s", offset, in.PageToken) + size, offset, perr := server.ExtractPagination(in.PageSize, in.PageToken, s.Pagination) + if perr != nil { + log.Printf("error: %v", perr) + return nil, perr } var result []models.VhostGetControllersResult err := s.rpc.Call("vhost_get_controllers", nil, &result) diff --git a/pkg/middleend/middleend.go b/pkg/middleend/middleend.go index 112c3547..e7957c44 100644 --- a/pkg/middleend/middleend.go +++ b/pkg/middleend/middleend.go @@ -9,7 +9,6 @@ import ( "context" "fmt" "log" - "math" "regexp" "strings" @@ -228,25 +227,10 @@ func (s *Server) UpdateEncryptedVolume(_ context.Context, in *pb.UpdateEncrypted // ListEncryptedVolumes lists encrypted volumes func (s *Server) ListEncryptedVolumes(_ context.Context, in *pb.ListEncryptedVolumesRequest) (*pb.ListEncryptedVolumesResponse, error) { log.Printf("ListEncryptedVolumes: Received from client: %v", in) - if in.PageSize < 0 { - err := status.Error(codes.InvalidArgument, "negative PageSize is not allowed") - log.Printf("error: %v", err) - return nil, err - } - size := 50 - if in.PageSize > 0 { - size = int(math.Min(float64(in.PageSize), 250)) - } - offset := 0 - if in.PageToken != "" { - var ok bool - offset, ok = s.Pagination[in.PageToken] - if !ok { - err := status.Errorf(codes.NotFound, "unable to find pagination token %s", in.PageToken) - log.Printf("error: %v", err) - return nil, err - } - log.Printf("Found offset %d from pagination token: %s", offset, in.PageToken) + size, offset, perr := server.ExtractPagination(in.PageSize, in.PageToken, s.Pagination) + if perr != nil { + log.Printf("error: %v", perr) + return nil, perr } var result []models.BdevGetBdevsResult err := s.rpc.Call("bdev_get_bdevs", nil, &result) diff --git a/pkg/server/utils.go b/pkg/server/utils.go index 6be366a1..8722ce9b 100644 --- a/pkg/server/utils.go +++ b/pkg/server/utils.go @@ -9,6 +9,7 @@ import ( "crypto/rand" "fmt" "log" + "math" "math/big" "net" "os" @@ -16,8 +17,35 @@ import ( "strings" "google.golang.org/grpc" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" ) +// ExtractPagination is a helper function for List pagination to fetch PageSize and PageToken +func ExtractPagination(PageSize int32, PageToken string, Pagination map[string]int) (int, int, error) { + if PageSize < 0 { + err := status.Error(codes.InvalidArgument, "negative PageSize is not allowed") + log.Printf("error: %v", err) + return -1, -1, err + } + size := 50 + if PageSize > 0 { + size = int(math.Min(float64(PageSize), 250)) + } + offset := 0 + if PageToken != "" { + var ok bool + offset, ok = Pagination[PageToken] + if !ok { + err := status.Errorf(codes.NotFound, "unable to find pagination token %s", PageToken) + log.Printf("error: %v", err) + return -1, -1, err + } + log.Printf("Found offset %d from pagination token: %s", offset, PageToken) + } + return size, offset, nil +} + // CreateTestSpdkServer creates a mock spdk server for testing func CreateTestSpdkServer(socket string, startSpdkServer bool, spdkResponses []string) (net.Listener, JSONRPC) { jsonRPC := NewSpdkJSONRPC(socket).(*spdkJSONRPC) From 1eff2f78262fbddbc2a8349967339e2a7dbbeea5 Mon Sep 17 00:00:00 2001 From: Boris Glimcher Date: Wed, 12 Apr 2023 05:08:07 +0300 Subject: [PATCH 07/11] List: remove code duplication introduce new LimitPagination function Signed-off-by: Boris Glimcher --- pkg/backend/aio.go | 8 ++++---- pkg/backend/null.go | 8 ++++---- pkg/backend/nvme.go | 8 ++++---- pkg/frontend/blk.go | 8 ++++---- pkg/frontend/nvme.go | 17 +++++++++-------- pkg/frontend/scsi.go | 16 ++++++++-------- pkg/middleend/middleend.go | 8 ++++---- pkg/server/utils.go | 32 ++++++++++++++++++++++---------- 8 files changed, 59 insertions(+), 46 deletions(-) diff --git a/pkg/backend/aio.go b/pkg/backend/aio.go index bdf38f98..db582f44 100644 --- a/pkg/backend/aio.go +++ b/pkg/backend/aio.go @@ -150,10 +150,10 @@ func (s *Server) ListAioControllers(_ context.Context, in *pb.ListAioControllers return nil, err } log.Printf("Received from SPDK: %v", result) - var token string - if size < len(result) { - log.Printf("Limiting result to %d:%d", offset, size) - result = result[offset:size] + token := "" + log.Printf("Limiting result len(%d) to [%d:%d]", len(result), offset, size) + result, hasMoreElements := server.LimitPagination(result, offset, size) + if hasMoreElements { token = uuid.New().String() s.Pagination[token] = offset + size } diff --git a/pkg/backend/null.go b/pkg/backend/null.go index 4028cb16..d9425db0 100644 --- a/pkg/backend/null.go +++ b/pkg/backend/null.go @@ -150,10 +150,10 @@ func (s *Server) ListNullDebugs(_ context.Context, in *pb.ListNullDebugsRequest) return nil, err } log.Printf("Received from SPDK: %v", result) - var token string - if size < len(result) { - log.Printf("Limiting result to %d:%d", offset, size) - result = result[offset:size] + token := "" + log.Printf("Limiting result len(%d) to [%d:%d]", len(result), offset, size) + result, hasMoreElements := server.LimitPagination(result, offset, size) + if hasMoreElements { token = uuid.New().String() s.Pagination[token] = offset + size } diff --git a/pkg/backend/nvme.go b/pkg/backend/nvme.go index 0f28344c..c0c2d0f0 100644 --- a/pkg/backend/nvme.go +++ b/pkg/backend/nvme.go @@ -112,10 +112,10 @@ func (s *Server) ListNVMfRemoteControllers(_ context.Context, in *pb.ListNVMfRem return nil, err } log.Printf("Received from SPDK: %v", result) - var token string - if size < len(result) { - log.Printf("Limiting result to %d:%d", offset, size) - result = result[offset:size] + token := "" + log.Printf("Limiting result len(%d) to [%d:%d]", len(result), offset, size) + result, hasMoreElements := server.LimitPagination(result, offset, size) + if hasMoreElements { token = uuid.New().String() s.Pagination[token] = offset + size } diff --git a/pkg/frontend/blk.go b/pkg/frontend/blk.go index 5a4fda85..20b2c4cb 100644 --- a/pkg/frontend/blk.go +++ b/pkg/frontend/blk.go @@ -108,10 +108,10 @@ func (s *Server) ListVirtioBlks(_ context.Context, in *pb.ListVirtioBlksRequest) return nil, err } log.Printf("Received from SPDK: %v", result) - var token string - if size < len(result) { - log.Printf("Limiting result to %d:%d", offset, size) - result = result[offset:size] + token := "" + log.Printf("Limiting result len(%d) to [%d:%d]", len(result), offset, size) + result, hasMoreElements := server.LimitPagination(result, offset, size) + if hasMoreElements { token = uuid.New().String() s.Pagination[token] = offset + size } diff --git a/pkg/frontend/nvme.go b/pkg/frontend/nvme.go index 496bd1ca..68d22338 100644 --- a/pkg/frontend/nvme.go +++ b/pkg/frontend/nvme.go @@ -174,10 +174,10 @@ func (s *Server) ListNVMeSubsystems(_ context.Context, in *pb.ListNVMeSubsystems return nil, err } log.Printf("Received from SPDK: %v", result) - var token string - if size < len(result) { - log.Printf("Limiting result to %d:%d", offset, size) - result = result[offset:size] + token := "" + log.Printf("Limiting result len(%d) to [%d:%d]", len(result), offset, size) + result, hasMoreElements := server.LimitPagination(result, offset, size) + if hasMoreElements { token = uuid.New().String() s.Pagination[token] = offset + size } @@ -481,14 +481,15 @@ func (s *Server) ListNVMeNamespaces(_ context.Context, in *pb.ListNVMeNamespaces return nil, err } log.Printf("Received from SPDK: %v", result) - var token string + token := "" Blobarray := []*pb.NVMeNamespace{} for i := range result { rr := &result[i] if rr.Nqn == nqn || nqn == "" { - if size < len(result) { - log.Printf("Limiting result to %d:%d", offset, size) - rr.Namespaces = rr.Namespaces[offset:size] + log.Printf("Limiting result len(%d) to [%d:%d]", len(result), offset, size) + hasMoreElements := false + rr.Namespaces, hasMoreElements = server.LimitPagination(rr.Namespaces, offset, size) + if hasMoreElements { token = uuid.New().String() s.Pagination[token] = offset + size } diff --git a/pkg/frontend/scsi.go b/pkg/frontend/scsi.go index a5a7d83d..d3c2431e 100644 --- a/pkg/frontend/scsi.go +++ b/pkg/frontend/scsi.go @@ -87,10 +87,10 @@ func (s *Server) ListVirtioScsiControllers(_ context.Context, in *pb.ListVirtioS return nil, err } log.Printf("Received from SPDK: %v", result) - var token string - if size < len(result) { - log.Printf("Limiting result to %d:%d", offset, size) - result = result[offset:size] + token := "" + log.Printf("Limiting result len(%d) to [%d:%d]", len(result), offset, size) + result, hasMoreElements := server.LimitPagination(result, offset, size) + if hasMoreElements { token = uuid.New().String() s.Pagination[token] = offset + size } @@ -195,10 +195,10 @@ func (s *Server) ListVirtioScsiLuns(_ context.Context, in *pb.ListVirtioScsiLuns return nil, err } log.Printf("Received from SPDK: %v", result) - var token string - if size < len(result) { - log.Printf("Limiting result to %d:%d", offset, size) - result = result[offset:size] + token := "" + log.Printf("Limiting result len(%d) to [%d:%d]", len(result), offset, size) + result, hasMoreElements := server.LimitPagination(result, offset, size) + if hasMoreElements { token = uuid.New().String() s.Pagination[token] = offset + size } diff --git a/pkg/middleend/middleend.go b/pkg/middleend/middleend.go index e7957c44..dcc94c7d 100644 --- a/pkg/middleend/middleend.go +++ b/pkg/middleend/middleend.go @@ -239,10 +239,10 @@ func (s *Server) ListEncryptedVolumes(_ context.Context, in *pb.ListEncryptedVol return nil, err } log.Printf("Received from SPDK: %v", result) - var token string - if size < len(result) { - log.Printf("Limiting result to %d:%d", offset, size) - result = result[offset:size] + token := "" + log.Printf("Limiting result len(%d) to [%d:%d]", len(result), offset, size) + result, hasMoreElements := server.LimitPagination(result, offset, size) + if hasMoreElements { token = uuid.New().String() s.Pagination[token] = offset + size } diff --git a/pkg/server/utils.go b/pkg/server/utils.go index 8722ce9b..99dc5974 100644 --- a/pkg/server/utils.go +++ b/pkg/server/utils.go @@ -22,30 +22,42 @@ import ( ) // ExtractPagination is a helper function for List pagination to fetch PageSize and PageToken -func ExtractPagination(PageSize int32, PageToken string, Pagination map[string]int) (int, int, error) { - if PageSize < 0 { +func ExtractPagination(pageSize int32, pageToken string, pagination map[string]int) (int, int, error) { + if pageSize < 0 { err := status.Error(codes.InvalidArgument, "negative PageSize is not allowed") - log.Printf("error: %v", err) return -1, -1, err } + // pick reasonable default and max sizes size := 50 - if PageSize > 0 { - size = int(math.Min(float64(PageSize), 250)) + if pageSize > 0 { + size = int(math.Min(float64(pageSize), 250)) } + // fetch offset from the database using opaque token offset := 0 - if PageToken != "" { + if pageToken != "" { var ok bool - offset, ok = Pagination[PageToken] + offset, ok = pagination[pageToken] if !ok { - err := status.Errorf(codes.NotFound, "unable to find pagination token %s", PageToken) - log.Printf("error: %v", err) + err := status.Errorf(codes.NotFound, "unable to find pagination token %s", pageToken) return -1, -1, err } - log.Printf("Found offset %d from pagination token: %s", offset, PageToken) + log.Printf("Found offset %d from pagination token: %s", offset, pageToken) } return size, offset, nil } +// LimitPagination is a helper function for slice the result by offset and size +func LimitPagination[T any](result []T, offset int, size int) ([]T, bool) { + end := offset + size + hasMoreElements := false + if end < len(result) { + hasMoreElements = true + } else { + end = len(result) + } + return result[offset:end], hasMoreElements +} + // CreateTestSpdkServer creates a mock spdk server for testing func CreateTestSpdkServer(socket string, startSpdkServer bool, spdkResponses []string) (net.Listener, JSONRPC) { jsonRPC := NewSpdkJSONRPC(socket).(*spdkJSONRPC) From cd7b36d60f76ffd6038e593b755064a649866b31 Mon Sep 17 00:00:00 2001 From: Boris Glimcher Date: Wed, 12 Apr 2023 15:17:10 +0300 Subject: [PATCH 08/11] List: use named returned values for pagination Code review comments Signed-off-by: Boris Glimcher --- pkg/server/utils.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/server/utils.go b/pkg/server/utils.go index 99dc5974..fcea4260 100644 --- a/pkg/server/utils.go +++ b/pkg/server/utils.go @@ -22,18 +22,18 @@ import ( ) // ExtractPagination is a helper function for List pagination to fetch PageSize and PageToken -func ExtractPagination(pageSize int32, pageToken string, pagination map[string]int) (int, int, error) { +func ExtractPagination(pageSize int32, pageToken string, pagination map[string]int) (size int, offset int, err error) { if pageSize < 0 { err := status.Error(codes.InvalidArgument, "negative PageSize is not allowed") return -1, -1, err } // pick reasonable default and max sizes - size := 50 + size = 50 if pageSize > 0 { size = int(math.Min(float64(pageSize), 250)) } // fetch offset from the database using opaque token - offset := 0 + offset = 0 if pageToken != "" { var ok bool offset, ok = pagination[pageToken] From c8a9ed8a335b1bd99744a28dfe3685e34c8e08a9 Mon Sep 17 00:00:00 2001 From: Boris Glimcher Date: Wed, 12 Apr 2023 15:21:30 +0300 Subject: [PATCH 09/11] List: use explicit maxPageSize and defaultPageSize consts Signed-off-by: Boris Glimcher --- pkg/server/utils.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/pkg/server/utils.go b/pkg/server/utils.go index fcea4260..d208c459 100644 --- a/pkg/server/utils.go +++ b/pkg/server/utils.go @@ -23,14 +23,18 @@ import ( // ExtractPagination is a helper function for List pagination to fetch PageSize and PageToken func ExtractPagination(pageSize int32, pageToken string, pagination map[string]int) (size int, offset int, err error) { + const ( + maxPageSize = 250 + defaultPageSize = 50 + ) if pageSize < 0 { err := status.Error(codes.InvalidArgument, "negative PageSize is not allowed") return -1, -1, err } // pick reasonable default and max sizes - size = 50 + size = defaultPageSize if pageSize > 0 { - size = int(math.Min(float64(pageSize), 250)) + size = int(math.Min(float64(pageSize), maxPageSize)) } // fetch offset from the database using opaque token offset = 0 From 19db8b7e9adf3d5b4f3910dee807b878f3b7609f Mon Sep 17 00:00:00 2001 From: Boris Glimcher Date: Wed, 12 Apr 2023 15:28:50 +0300 Subject: [PATCH 10/11] List: get rid of ugly math and casting, use explicit switch Code review comments Signed-off-by: Boris Glimcher --- pkg/server/utils.go | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/pkg/server/utils.go b/pkg/server/utils.go index d208c459..145a47cf 100644 --- a/pkg/server/utils.go +++ b/pkg/server/utils.go @@ -9,7 +9,6 @@ import ( "crypto/rand" "fmt" "log" - "math" "math/big" "net" "os" @@ -27,14 +26,15 @@ func ExtractPagination(pageSize int32, pageToken string, pagination map[string]i maxPageSize = 250 defaultPageSize = 50 ) - if pageSize < 0 { - err := status.Error(codes.InvalidArgument, "negative PageSize is not allowed") - return -1, -1, err - } - // pick reasonable default and max sizes - size = defaultPageSize - if pageSize > 0 { - size = int(math.Min(float64(pageSize), maxPageSize)) + switch { + case pageSize < 0: + return -1, -1, status.Error(codes.InvalidArgument, "negative PageSize is not allowed") + case pageSize == 0: + size = defaultPageSize + case pageSize > maxPageSize: + size = maxPageSize + default: + size = int(pageSize) } // fetch offset from the database using opaque token offset = 0 @@ -42,8 +42,7 @@ func ExtractPagination(pageSize int32, pageToken string, pagination map[string]i var ok bool offset, ok = pagination[pageToken] if !ok { - err := status.Errorf(codes.NotFound, "unable to find pagination token %s", pageToken) - return -1, -1, err + return -1, -1, status.Errorf(codes.NotFound, "unable to find pagination token %s", pageToken) } log.Printf("Found offset %d from pagination token: %s", offset, pageToken) } From 997e2a6f00609ccd7a72b9c15ad3a2157d81f4c4 Mon Sep 17 00:00:00 2001 From: Boris Glimcher Date: Wed, 12 Apr 2023 16:19:09 +0300 Subject: [PATCH 11/11] code review: improve error message Signed-off-by: Boris Glimcher --- pkg/backend/aio_test.go | 2 +- pkg/backend/null_test.go | 2 +- pkg/backend/nvme_test.go | 2 +- pkg/frontend/blk_test.go | 2 +- pkg/frontend/nvme_test.go | 4 ++-- pkg/middleend/middleend_test.go | 2 +- 6 files changed, 7 insertions(+), 7 deletions(-) diff --git a/pkg/backend/aio_test.go b/pkg/backend/aio_test.go index cfd00216..e59a318f 100644 --- a/pkg/backend/aio_test.go +++ b/pkg/backend/aio_test.go @@ -386,7 +386,7 @@ func TestBackEnd_ListAioControllers(t *testing.T) { } // Empty NextPageToken indicates end of results list if tt.size != 1 && response.NextPageToken != "" { - t.Error("response: expected", tt, "received", response) + t.Error("Expected end of results, receieved non-empty next page token", response.NextPageToken) } } diff --git a/pkg/backend/null_test.go b/pkg/backend/null_test.go index 6c7cc430..07a7e024 100644 --- a/pkg/backend/null_test.go +++ b/pkg/backend/null_test.go @@ -390,7 +390,7 @@ func TestBackEnd_ListNullDebugs(t *testing.T) { } // Empty NextPageToken indicates end of results list if tt.size != 1 && response.NextPageToken != "" { - t.Error("response: expected", tt, "received", response) + t.Error("Expected end of results, receieved non-empty next page token", response.NextPageToken) } } diff --git a/pkg/backend/nvme_test.go b/pkg/backend/nvme_test.go index b26aee7e..d034684f 100644 --- a/pkg/backend/nvme_test.go +++ b/pkg/backend/nvme_test.go @@ -339,7 +339,7 @@ func TestBackEnd_ListNVMfRemoteControllers(t *testing.T) { } // Empty NextPageToken indicates end of results list if tt.size != 1 && response.NextPageToken != "" { - t.Error("response: expected", tt, "received", response) + t.Error("Expected end of results, receieved non-empty next page token", response.NextPageToken) } } diff --git a/pkg/frontend/blk_test.go b/pkg/frontend/blk_test.go index 30abb424..9ca46149 100644 --- a/pkg/frontend/blk_test.go +++ b/pkg/frontend/blk_test.go @@ -292,7 +292,7 @@ func TestFrontEnd_ListVirtioBlks(t *testing.T) { } // Empty NextPageToken indicates end of results list if tt.size != 1 && response.NextPageToken != "" { - t.Error("response: expected", tt, "received", response) + t.Error("Expected end of results, receieved non-empty next page token", response.NextPageToken) } } diff --git a/pkg/frontend/nvme_test.go b/pkg/frontend/nvme_test.go index 75192142..8c542782 100644 --- a/pkg/frontend/nvme_test.go +++ b/pkg/frontend/nvme_test.go @@ -378,7 +378,7 @@ func TestFrontEnd_ListNVMeSubsystem(t *testing.T) { } // Empty NextPageToken indicates end of results list if tt.size != 1 && response.NextPageToken != "" { - t.Error("response: expected", tt, "received", response) + t.Error("Expected end of results, receieved non-empty next page token", response.NextPageToken) } } @@ -1365,7 +1365,7 @@ func TestFrontEnd_ListNVMeNamespaces(t *testing.T) { } // Empty NextPageToken indicates end of results list if tt.size != 1 && response.NextPageToken != "" { - t.Error("response: expected", tt, "received", response) + t.Error("Expected end of results, receieved non-empty next page token", response.NextPageToken) } } diff --git a/pkg/middleend/middleend_test.go b/pkg/middleend/middleend_test.go index 3cf5c318..02a9cca0 100644 --- a/pkg/middleend/middleend_test.go +++ b/pkg/middleend/middleend_test.go @@ -551,7 +551,7 @@ func TestMiddleEnd_ListEncryptedVolumes(t *testing.T) { } // Empty NextPageToken indicates end of results list if tt.size != 1 && response.NextPageToken != "" { - t.Error("response: expected", tt, "received", response) + t.Error("Expected end of results, receieved non-empty next page token", response.NextPageToken) } }