From aea7418656ee081c76cb4a78733ace31f50ad37b Mon Sep 17 00:00:00 2001 From: Artsiom Koltun Date: Tue, 23 May 2023 15:30:55 +0200 Subject: [PATCH 1/7] Consolidate pagination into dedicated structs. Signed-off-by: Artsiom Koltun --- pkg/server/pagination.go | 110 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 110 insertions(+) create mode 100644 pkg/server/pagination.go diff --git a/pkg/server/pagination.go b/pkg/server/pagination.go new file mode 100644 index 00000000..ecd31c9b --- /dev/null +++ b/pkg/server/pagination.go @@ -0,0 +1,110 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright (c) 2022-2023 Dell Inc, or its subsidiaries. +// Copyright (C) 2023 Intel Corporation + +// Package server implements the server +package server + +import ( + "fmt" + "log" + + "github.com/google/uuid" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" +) + +// Pagination is type used to track pagination for List calls +type Pagination map[string]int + +// NewPagination creates a new Pagination instance +func NewPagination() Pagination { + return make(Pagination) +} + +// PageToken returns page token describing an existing token or a new one based +// on provided request parameters +func (p Pagination) PageToken(size int32, token string) (PageToken, error) { + const ( + maxPageSize = 250 + defaultPageSize = 50 + ) + switch { + case size < 0: + return PageToken{}, status.Error(codes.InvalidArgument, "negative PageSize is not allowed") + case size == 0: + size = defaultPageSize + case size > maxPageSize: + size = maxPageSize + } + + offset := 0 + if token != "" { + var ok bool + offset, ok = p[token] + if !ok { + return PageToken{}, status.Errorf(codes.NotFound, "unable to find pagination token %s", token) + } + log.Printf("Found offset %d from pagination token: %s", offset, token) + } + + end, err := addInt(offset, int(size)) + if err != nil { + return PageToken{}, status.Errorf(codes.InvalidArgument, "Invalid page argument") + } + + return PageToken{ + offset: offset, + end: end, + pagination: p, + }, nil +} + +// PageToken describes an existing token or a new one based +// on provided request parameters +type PageToken struct { + offset int + end int + pagination Pagination +} + +// LimitToPage returns a list for response based on provided page token +func LimitToPage[T any](pt PageToken, list []T) Page[T] { + listSize := len(list) + nextToken := "" + offset := pt.offset + end := pt.end + if offset >= listSize { + log.Printf("Offset %v is greater than list size %v", offset, listSize) + return Page[T]{} + } + + if end < listSize { + nextToken = uuid.NewString() + pt.pagination[nextToken] = end + } else { + end = listSize + } + + log.Printf("Limiting result len(%d) to [%d:%d]", len(list), offset, end) + return Page[T]{ + List: list[offset:end], + NextToken: nextToken, + } +} + +// Page describes results to send in response based on pagination +type Page[T any] struct { + List []T + NextToken string +} + +func addInt(a, b int) (int, error) { + r := a + b + if a > 0 && b > 0 && r < 0 { + return 0, fmt.Errorf("integer overflow for %v and %v", a, b) + } else if a < 0 && b < 0 && r > 0 { + return 0, fmt.Errorf("integer underflow for %v and %v", a, b) + } + return r, nil +} From 63192b1b20be5f004b8d28157c604365dc7d215e Mon Sep 17 00:00:00 2001 From: Artsiom Koltun Date: Tue, 23 May 2023 16:01:59 +0200 Subject: [PATCH 2/7] Add pagination struct tests. Signed-off-by: Artsiom Koltun --- pkg/server/pagination_test.go | 150 ++++++++++++++++++++++++++++++++++ 1 file changed, 150 insertions(+) create mode 100644 pkg/server/pagination_test.go diff --git a/pkg/server/pagination_test.go b/pkg/server/pagination_test.go new file mode 100644 index 00000000..c5f83bbf --- /dev/null +++ b/pkg/server/pagination_test.go @@ -0,0 +1,150 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright (C) 2023 Intel Corporation + +// Package server implements the server +package server + +import ( + "bytes" + "math" + "testing" +) + +func TestServer_PageToken(t *testing.T) { + existingToken := "existing-token" + tests := map[string]struct { + size int32 + token string + existingTokenOffset int + expectErr bool + expectOffset int + expectEnd int + }{ + "negative size": { + size: -1, + token: existingToken, + existingTokenOffset: 0, + expectErr: true, + expectOffset: 0, + expectEnd: 0, + }, + "non-existing token": { + size: 0, + token: "non-existing-token", + existingTokenOffset: 0, + expectErr: true, + expectOffset: 0, + expectEnd: 0, + }, + "offset in db overflow": { + size: 0, + token: existingToken, + existingTokenOffset: math.MaxInt, + expectErr: true, + expectOffset: 0, + expectEnd: 0, + }, + "empty token": { + size: 0, + token: "", + existingTokenOffset: 0, + expectErr: false, + expectOffset: 0, + expectEnd: 50, + }, + "existing-token": { + size: 5, + token: existingToken, + existingTokenOffset: 5, + expectErr: false, + expectOffset: 5, + expectEnd: 10, + }, + "non-zero size": { + size: 5, + token: existingToken, + existingTokenOffset: 0, + expectErr: false, + expectOffset: 0, + expectEnd: 5, + }, + "size more than allowed": { + size: 1000, + token: existingToken, + existingTokenOffset: 0, + expectErr: false, + expectOffset: 0, + expectEnd: 250, + }, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + pagination := NewPagination() + pagination["existing-token"] = test.existingTokenOffset + + pageToken, err := pagination.PageToken(test.size, test.token) + if (err != nil) != test.expectErr { + t.Error("Expect error", test.expectErr, "received", err) + } + if !test.expectErr { + if pageToken.offset != test.expectOffset { + t.Error("Expect offset", test.expectOffset, "received", pageToken.offset) + } + if pageToken.end != test.expectEnd { + t.Error("Expect page end", test.expectEnd, "received", pageToken.end) + } + } + }) + } +} + +func TestServer_LimitToPage(t *testing.T) { + tests := map[string]struct { + list []byte + existingTokenOffset int + expectList []byte + expectEmptyToken bool + }{ + "list fits into page": { + list: []byte{1, 2, 3}, + existingTokenOffset: 1, + expectList: []byte{2, 3}, + expectEmptyToken: true, + }, + "list fits into page, but less then requested size": { + list: []byte{1, 2, 3}, + existingTokenOffset: 2, + expectList: []byte{3}, + expectEmptyToken: true, + }, + "list does not fit into page": { + list: []byte{1, 2, 3, 4, 5}, + existingTokenOffset: 1, + expectList: []byte{2, 3}, + expectEmptyToken: false, + }, + "existing token offset out of list": { + list: []byte{1, 2, 3}, + existingTokenOffset: 10, + expectList: []byte{}, + expectEmptyToken: true, + }, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + pagination := NewPagination() + pagination["existing-token"] = test.existingTokenOffset + pageToken, _ := pagination.PageToken(2, "existing-token") + + page := LimitToPage(pageToken, test.list) + if !bytes.Equal(page.List, test.expectList) { + t.Error("Expect", test.expectList, "received", page.List) + } + if (page.NextToken == "") != test.expectEmptyToken { + t.Error("Expect empty token", test.expectEmptyToken, "received", page.NextToken) + } + }) + } +} From 743b1ffa2c94f19910e272eb53fd54634b8288d7 Mon Sep 17 00:00:00 2001 From: Artsiom Koltun Date: Tue, 23 May 2023 15:32:01 +0200 Subject: [PATCH 3/7] Use pagination structs in backend. Signed-off-by: Artsiom Koltun --- pkg/backend/aio.go | 12 +++--------- pkg/backend/backend.go | 5 +++-- pkg/backend/null.go | 12 +++--------- pkg/backend/nvme.go | 12 +++--------- 4 files changed, 12 insertions(+), 29 deletions(-) diff --git a/pkg/backend/aio.go b/pkg/backend/aio.go index 86817f06..816a6cf4 100644 --- a/pkg/backend/aio.go +++ b/pkg/backend/aio.go @@ -152,7 +152,7 @@ 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) - size, offset, perr := server.ExtractPagination(in.PageSize, in.PageToken, s.Pagination) + pageToken, perr := s.Pagination.PageToken(in.PageSize, in.PageToken) if perr != nil { log.Printf("error: %v", perr) return nil, perr @@ -164,20 +164,14 @@ func (s *Server) ListAioControllers(_ context.Context, in *pb.ListAioControllers return nil, err } log.Printf("Received from SPDK: %v", result) - 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 - } 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} } sortAioControllers(Blobarray) - return &pb.ListAioControllersResponse{AioControllers: Blobarray, NextPageToken: token}, nil + page := server.LimitToPage(pageToken, Blobarray) + return &pb.ListAioControllersResponse{AioControllers: page.List, NextPageToken: page.NextToken}, nil } // GetAioController gets an Aio controller diff --git a/pkg/backend/backend.go b/pkg/backend/backend.go index 9072439b..3ba3e5e1 100644 --- a/pkg/backend/backend.go +++ b/pkg/backend/backend.go @@ -8,6 +8,7 @@ package backend import ( "github.com/opiproject/gospdk/spdk" pb "github.com/opiproject/opi-api/storage/v1alpha1/gen/go" + "github.com/opiproject/opi-spdk-bridge/pkg/server" ) // TODO: can we combine all of volume types into a single list? @@ -28,7 +29,7 @@ type Server struct { rpc spdk.JSONRPC Volumes VolumeParameters - Pagination map[string]int + Pagination server.Pagination } // NewServer creates initialized instance of BackEnd server communicating @@ -41,6 +42,6 @@ func NewServer(jsonRPC spdk.JSONRPC) *Server { NullVolumes: make(map[string]*pb.NullDebug), NvmeVolumes: make(map[string]*pb.NVMfRemoteController), }, - Pagination: make(map[string]int), + Pagination: server.NewPagination(), } } diff --git a/pkg/backend/null.go b/pkg/backend/null.go index 98599f95..fe5bb746 100644 --- a/pkg/backend/null.go +++ b/pkg/backend/null.go @@ -152,7 +152,7 @@ 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) - size, offset, perr := server.ExtractPagination(in.PageSize, in.PageToken, s.Pagination) + pageToken, perr := s.Pagination.PageToken(in.PageSize, in.PageToken) if perr != nil { log.Printf("error: %v", perr) return nil, perr @@ -164,20 +164,14 @@ func (s *Server) ListNullDebugs(_ context.Context, in *pb.ListNullDebugsRequest) return nil, err } log.Printf("Received from SPDK: %v", result) - 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 - } 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} } sortNullDebugs(Blobarray) - return &pb.ListNullDebugsResponse{NullDebugs: Blobarray, NextPageToken: token}, nil + page := server.LimitToPage(pageToken, Blobarray) + return &pb.ListNullDebugsResponse{NullDebugs: page.List, NextPageToken: page.NextToken}, nil } // GetNullDebug gets a a Null Debug instance diff --git a/pkg/backend/nvme.go b/pkg/backend/nvme.go index 3ad76165..194cdfab 100644 --- a/pkg/backend/nvme.go +++ b/pkg/backend/nvme.go @@ -114,7 +114,7 @@ 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) - size, offset, perr := server.ExtractPagination(in.PageSize, in.PageToken, s.Pagination) + pageToken, perr := s.Pagination.PageToken(in.PageSize, in.PageToken) if perr != nil { log.Printf("error: %v", perr) return nil, perr @@ -126,13 +126,6 @@ func (s *Server) ListNVMfRemoteControllers(_ context.Context, in *pb.ListNVMfRem return nil, err } log.Printf("Received from SPDK: %v", result) - 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 - } Blobarray := make([]*pb.NVMfRemoteController, len(result)) for i := range result { r := &result[i] @@ -148,7 +141,8 @@ func (s *Server) ListNVMfRemoteControllers(_ context.Context, in *pb.ListNVMfRem } } sortNVMfRemoteControllers(Blobarray) - return &pb.ListNVMfRemoteControllersResponse{NvMfRemoteControllers: Blobarray, NextPageToken: token}, nil + page := server.LimitToPage(pageToken, Blobarray) + return &pb.ListNVMfRemoteControllersResponse{NvMfRemoteControllers: page.List, NextPageToken: page.NextToken}, nil } // GetNVMfRemoteController gets an NVMf remote controller From c5c6c2208f9f240275e472cd7030a2519e6769cd Mon Sep 17 00:00:00 2001 From: Artsiom Koltun Date: Tue, 23 May 2023 16:16:07 +0200 Subject: [PATCH 4/7] Use pagination structs in middleend. Signed-off-by: Artsiom Koltun --- pkg/middleend/encryption.go | 13 +++---------- pkg/middleend/middleend.go | 5 +++-- pkg/middleend/qos.go | 14 +++----------- 3 files changed, 9 insertions(+), 23 deletions(-) diff --git a/pkg/middleend/encryption.go b/pkg/middleend/encryption.go index ec966e83..0950edc9 100644 --- a/pkg/middleend/encryption.go +++ b/pkg/middleend/encryption.go @@ -207,7 +207,7 @@ 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) - size, offset, perr := server.ExtractPagination(in.PageSize, in.PageToken, s.Pagination) + pageToken, perr := s.Pagination.PageToken(in.PageSize, in.PageToken) if perr != nil { log.Printf("error: %v", perr) return nil, perr @@ -219,21 +219,14 @@ func (s *Server) ListEncryptedVolumes(_ context.Context, in *pb.ListEncryptedVol return nil, err } log.Printf("Received from SPDK: %v", result) - 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 - } Blobarray := make([]*pb.EncryptedVolume, len(result)) for i := range result { r := &result[i] Blobarray[i] = &pb.EncryptedVolume{EncryptedVolumeId: &pc.ObjectKey{Value: r.Name}} } sortEncryptedVolumes(Blobarray) - - return &pb.ListEncryptedVolumesResponse{EncryptedVolumes: Blobarray, NextPageToken: token}, nil + page := server.LimitToPage(pageToken, Blobarray) + return &pb.ListEncryptedVolumesResponse{EncryptedVolumes: page.List, NextPageToken: page.NextToken}, nil } // GetEncryptedVolume gets an encrypted volume diff --git a/pkg/middleend/middleend.go b/pkg/middleend/middleend.go index 04e56325..063b0eb4 100644 --- a/pkg/middleend/middleend.go +++ b/pkg/middleend/middleend.go @@ -8,6 +8,7 @@ package middleend import ( "github.com/opiproject/gospdk/spdk" pb "github.com/opiproject/opi-api/storage/v1alpha1/gen/go" + "github.com/opiproject/opi-spdk-bridge/pkg/server" ) // VolumeParameters contains MiddleEnd volume related structures @@ -22,7 +23,7 @@ type Server struct { rpc spdk.JSONRPC volumes VolumeParameters - Pagination map[string]int + Pagination server.Pagination } // NewServer creates initialized instance of MiddleEnd server communicating @@ -33,6 +34,6 @@ func NewServer(jsonRPC spdk.JSONRPC) *Server { volumes: VolumeParameters{ qosVolumes: make(map[string]*pb.QosVolume), }, - Pagination: make(map[string]int), + Pagination: server.NewPagination(), } } diff --git a/pkg/middleend/qos.go b/pkg/middleend/qos.go index df523999..dab2cd20 100644 --- a/pkg/middleend/qos.go +++ b/pkg/middleend/qos.go @@ -108,7 +108,7 @@ func (s *Server) UpdateQosVolume(_ context.Context, in *pb.UpdateQosVolumeReques func (s *Server) ListQosVolumes(_ context.Context, in *pb.ListQosVolumesRequest) (*pb.ListQosVolumesResponse, error) { log.Printf("ListQosVolume: Received from client: %v", in) - size, offset, err := server.ExtractPagination(in.PageSize, in.PageToken, s.Pagination) + pageToken, err := s.Pagination.PageToken(in.PageSize, in.PageToken) if err != nil { log.Printf("error: %v", err) return nil, err @@ -119,16 +119,8 @@ func (s *Server) ListQosVolumes(_ context.Context, in *pb.ListQosVolumesRequest) volumes = append(volumes, proto.Clone(qosVolume).(*pb.QosVolume)) } sortQosVolumes(volumes) - - token := "" - log.Printf("Limiting result len(%d) to [%d:%d]", len(volumes), offset, size) - volumes, hasMoreElements := server.LimitPagination(volumes, offset, size) - if hasMoreElements { - token = uuid.New().String() - s.Pagination[token] = offset + size - } - - return &pb.ListQosVolumesResponse{QosVolumes: volumes, NextPageToken: token}, nil + page := server.LimitToPage(pageToken, volumes) + return &pb.ListQosVolumesResponse{QosVolumes: page.List, NextPageToken: page.NextToken}, nil } // GetQosVolume gets a QoS volume From 0d147ab7a2a7ce1799e8c383dbedf14b8bc90868 Mon Sep 17 00:00:00 2001 From: Artsiom Koltun Date: Wed, 24 May 2023 09:23:33 +0200 Subject: [PATCH 5/7] Use pagination structs in frontend Signed-off-by: Artsiom Koltun --- pkg/frontend/blk.go | 13 +++---------- pkg/frontend/blk_test.go | 8 ++++++-- pkg/frontend/frontend.go | 5 +++-- pkg/frontend/nvme.go | 35 +++++++++++++---------------------- pkg/frontend/scsi.go | 24 ++++++------------------ 5 files changed, 31 insertions(+), 54 deletions(-) diff --git a/pkg/frontend/blk.go b/pkg/frontend/blk.go index ad59e780..07821211 100644 --- a/pkg/frontend/blk.go +++ b/pkg/frontend/blk.go @@ -110,7 +110,7 @@ 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) - size, offset, perr := server.ExtractPagination(in.PageSize, in.PageToken, s.Pagination) + pageToken, perr := s.Pagination.PageToken(in.PageSize, in.PageToken) if perr != nil { log.Printf("error: %v", perr) return nil, perr @@ -122,13 +122,6 @@ func (s *Server) ListVirtioBlks(_ context.Context, in *pb.ListVirtioBlksRequest) return nil, err } log.Printf("Received from SPDK: %v", result) - 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 - } Blobarray := make([]*pb.VirtioBlk, len(result)) for i := range result { r := &result[i] @@ -138,8 +131,8 @@ func (s *Server) ListVirtioBlks(_ context.Context, in *pb.ListVirtioBlksRequest) VolumeId: &pc.ObjectKey{Value: "TBD"}} } sortVirtioBlks(Blobarray) - - return &pb.ListVirtioBlksResponse{VirtioBlks: Blobarray, NextPageToken: token}, nil + page := server.LimitToPage(pageToken, Blobarray) + return &pb.ListVirtioBlksResponse{VirtioBlks: page.List, NextPageToken: page.NextToken}, nil } // GetVirtioBlk gets a Virtio block device diff --git a/pkg/frontend/blk_test.go b/pkg/frontend/blk_test.go index 43d1f8ec..a484cd69 100644 --- a/pkg/frontend/blk_test.go +++ b/pkg/frontend/blk_test.go @@ -285,12 +285,16 @@ func TestFrontEnd_ListVirtioBlks(t *testing.T) { "subsystem-test", []*pb.VirtioBlk{ { - Id: &pc.ObjectKey{Value: "virtio-blk-42"}, + Id: &pc.ObjectKey{Value: "VblkEmu0pf2"}, PcieId: &pb.PciEndpoint{PhysicalFunction: int32(1)}, VolumeId: &pc.ObjectKey{Value: "TBD"}, }, }, - []string{`{"jsonrpc":"2.0","id":%d,"result":[{"ctrlr":"VblkEmu0pf0","emulation_manager":"mlx5_0","type":"virtio_blk","pci_index":0,"pci_bdf":"ca:00.4"},{"ctrlr":"virtio-blk-42","emulation_manager":"mlx5_0","type":"virtio_blk","pci_index":0,"pci_bdf":"ca:00.4"},{"ctrlr":"VblkEmu0pf2","emulation_manager":"mlx5_0","type":"virtio_blk","pci_index":0,"pci_bdf":"ca:00.4"}],"error":{"code":0,"message":""}}`}, + []string{`{"jsonrpc":"2.0","id":%d,"result":[ + {"ctrlr":"VblkEmu0pf0","emulation_manager":"mlx5_0","type":"virtio_blk","pci_index":0,"pci_bdf":"ca:00.4"}, + {"ctrlr":"virtio-blk-42","emulation_manager":"mlx5_0","type":"virtio_blk","pci_index":0,"pci_bdf":"ca:00.4"}, + {"ctrlr":"VblkEmu0pf2","emulation_manager":"mlx5_0","type":"virtio_blk","pci_index":0,"pci_bdf":"ca:00.4"} + ],"error":{"code":0,"message":""}}`}, codes.OK, "", true, diff --git a/pkg/frontend/frontend.go b/pkg/frontend/frontend.go index c41c8c72..7c7c7af2 100644 --- a/pkg/frontend/frontend.go +++ b/pkg/frontend/frontend.go @@ -10,6 +10,7 @@ import ( "github.com/opiproject/gospdk/spdk" pb "github.com/opiproject/opi-api/storage/v1alpha1/gen/go" + "github.com/opiproject/opi-spdk-bridge/pkg/server" ) // SubsystemListener interface is used to provide SPDK call params to create/delete @@ -42,7 +43,7 @@ type Server struct { rpc spdk.JSONRPC Nvme NvmeParameters Virt VirtioParameters - Pagination map[string]int + Pagination server.Pagination } // NewServer creates initialized instance of FrontEnd server communicating @@ -61,7 +62,7 @@ func NewServer(jsonRPC spdk.JSONRPC) *Server { ScsiCtrls: make(map[string]*pb.VirtioScsiController), ScsiLuns: make(map[string]*pb.VirtioScsiLun), }, - Pagination: make(map[string]int), + Pagination: server.NewPagination(), } } diff --git a/pkg/frontend/nvme.go b/pkg/frontend/nvme.go index bd081b83..2de6ac5c 100644 --- a/pkg/frontend/nvme.go +++ b/pkg/frontend/nvme.go @@ -188,7 +188,7 @@ 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) - size, offset, perr := server.ExtractPagination(in.PageSize, in.PageToken, s.Pagination) + pageToken, perr := s.Pagination.PageToken(in.PageSize, in.PageToken) if perr != nil { log.Printf("error: %v", perr) return nil, perr @@ -200,20 +200,14 @@ func (s *Server) ListNVMeSubsystems(_ context.Context, in *pb.ListNVMeSubsystems return nil, err } log.Printf("Received from SPDK: %v", result) - 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 - } 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}} } sortNVMeSubsystems(Blobarray) - return &pb.ListNVMeSubsystemsResponse{NvMeSubsystems: Blobarray, NextPageToken: token}, nil + page := server.LimitToPage(pageToken, Blobarray) + return &pb.ListNVMeSubsystemsResponse{NvMeSubsystems: page.List, NextPageToken: page.NextToken}, nil } // GetNVMeSubsystem gets NVMe Subsystems @@ -360,14 +354,18 @@ func (s *Server) UpdateNVMeController(_ context.Context, in *pb.UpdateNVMeContro // ListNVMeControllers lists NVMe controllers func (s *Server) ListNVMeControllers(_ context.Context, in *pb.ListNVMeControllersRequest) (*pb.ListNVMeControllersResponse, error) { log.Printf("Received from client: %v", in.Parent) + pageToken, perr := s.Pagination.PageToken(in.PageSize, in.PageToken) + if perr != nil { + log.Printf("error: %v", perr) + return nil, perr + } Blobarray := []*pb.NVMeController{} for _, controller := range s.Nvme.Controllers { Blobarray = append(Blobarray, controller) } sortNVMeControllers(Blobarray) - token := uuid.New().String() - s.Pagination[token] = int(in.PageSize) - return &pb.ListNVMeControllersResponse{NvMeControllers: Blobarray, NextPageToken: token}, nil + page := server.LimitToPage(pageToken, Blobarray) + return &pb.ListNVMeControllersResponse{NvMeControllers: page.List, NextPageToken: page.NextToken}, nil } // GetNVMeController gets an NVMe controller @@ -501,7 +499,7 @@ 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) - size, offset, perr := server.ExtractPagination(in.PageSize, in.PageToken, s.Pagination) + pageToken, perr := s.Pagination.PageToken(in.PageSize, in.PageToken) if perr != nil { log.Printf("error: %v", perr) return nil, perr @@ -523,18 +521,10 @@ func (s *Server) ListNVMeNamespaces(_ context.Context, in *pb.ListNVMeNamespaces return nil, err } log.Printf("Received from SPDK: %v", result) - token := "" Blobarray := []*pb.NVMeNamespace{} for i := range result { rr := &result[i] if rr.Nqn == nqn || nqn == "" { - 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 - } for j := range rr.Namespaces { r := &rr.Namespaces[j] Blobarray = append(Blobarray, &pb.NVMeNamespace{Spec: &pb.NVMeNamespaceSpec{HostNsid: int32(r.Nsid)}}) @@ -543,7 +533,8 @@ func (s *Server) ListNVMeNamespaces(_ context.Context, in *pb.ListNVMeNamespaces } if len(Blobarray) > 0 { sortNVMeNamespaces(Blobarray) - return &pb.ListNVMeNamespacesResponse{NvMeNamespaces: Blobarray, NextPageToken: token}, nil + page := server.LimitToPage(pageToken, Blobarray) + return &pb.ListNVMeNamespacesResponse{NvMeNamespaces: page.List, NextPageToken: page.NextToken}, nil } msg := fmt.Sprintf("Could not find any namespaces for NQN: %s", nqn) diff --git a/pkg/frontend/scsi.go b/pkg/frontend/scsi.go index 2b85aacc..f64ccb06 100644 --- a/pkg/frontend/scsi.go +++ b/pkg/frontend/scsi.go @@ -108,7 +108,7 @@ 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) - size, offset, perr := server.ExtractPagination(in.PageSize, in.PageToken, s.Pagination) + pageToken, perr := s.Pagination.PageToken(in.PageSize, in.PageToken) if perr != nil { log.Printf("error: %v", perr) return nil, perr @@ -120,20 +120,14 @@ func (s *Server) ListVirtioScsiControllers(_ context.Context, in *pb.ListVirtioS return nil, err } log.Printf("Received from SPDK: %v", result) - 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 - } Blobarray := make([]*pb.VirtioScsiController, len(result)) for i := range result { r := &result[i] Blobarray[i] = &pb.VirtioScsiController{Id: &pc.ObjectKey{Value: r.Ctrlr}} } sortScsiControllers(Blobarray) - return &pb.ListVirtioScsiControllersResponse{VirtioScsiControllers: Blobarray, NextPageToken: token}, nil + page := server.LimitToPage(pageToken, Blobarray) + return &pb.ListVirtioScsiControllersResponse{VirtioScsiControllers: page.List, NextPageToken: page.NextToken}, nil } // GetVirtioScsiController gets a Virtio SCSI controller @@ -249,7 +243,7 @@ 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) - size, offset, perr := server.ExtractPagination(in.PageSize, in.PageToken, s.Pagination) + pageToken, perr := s.Pagination.PageToken(in.PageSize, in.PageToken) if perr != nil { log.Printf("error: %v", perr) return nil, perr @@ -261,19 +255,13 @@ func (s *Server) ListVirtioScsiLuns(_ context.Context, in *pb.ListVirtioScsiLuns return nil, err } log.Printf("Received from SPDK: %v", result) - 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 - } 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, NextPageToken: token}, nil + page := server.LimitToPage(pageToken, Blobarray) + return &pb.ListVirtioScsiLunsResponse{VirtioScsiLuns: page.List, NextPageToken: page.NextToken}, nil } // GetVirtioScsiLun gets a Virtio SCSI LUN From f1f743cf2cf49d7df7c2d29ba3e850bdd409c3a2 Mon Sep 17 00:00:00 2001 From: Artsiom Koltun Date: Wed, 24 May 2023 09:27:11 +0200 Subject: [PATCH 6/7] Add TODO to remove old pagination functions. Signed-off-by: Artsiom Koltun --- pkg/server/utils.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/server/utils.go b/pkg/server/utils.go index 7d3fec27..bc518b7f 100644 --- a/pkg/server/utils.go +++ b/pkg/server/utils.go @@ -23,6 +23,7 @@ import ( ) // ExtractPagination is a helper function for List pagination to fetch PageSize and PageToken +// TODO: remove when not used in other bridges func ExtractPagination(pageSize int32, pageToken string, pagination map[string]int) (size int, offset int, err error) { const ( maxPageSize = 250 @@ -52,6 +53,7 @@ func ExtractPagination(pageSize int32, pageToken string, pagination map[string]i } // LimitPagination is a helper function for slice the result by offset and size +// TODO: remove when not used in other bridges func LimitPagination[T any](result []T, offset int, size int) ([]T, bool) { end := offset + size hasMoreElements := false From 5996f445f9e6bc98dd84b8c6f97635464a0f2529 Mon Sep 17 00:00:00 2001 From: Artsiom Koltun Date: Wed, 24 May 2023 09:45:07 +0200 Subject: [PATCH 7/7] Add sort of scsi luns in list call. Signed-off-by: Artsiom Koltun --- pkg/frontend/scsi.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/pkg/frontend/scsi.go b/pkg/frontend/scsi.go index f64ccb06..f86a1bb2 100644 --- a/pkg/frontend/scsi.go +++ b/pkg/frontend/scsi.go @@ -29,6 +29,12 @@ func sortScsiControllers(controllers []*pb.VirtioScsiController) { }) } +func sortScsiLuns(controllers []*pb.VirtioScsiLun) { + sort.Slice(controllers, func(i int, j int) bool { + return controllers[i].Id.Value < controllers[j].Id.Value + }) +} + // CreateVirtioScsiController creates a Virtio SCSI controller func (s *Server) CreateVirtioScsiController(_ context.Context, in *pb.CreateVirtioScsiControllerRequest) (*pb.VirtioScsiController, error) { log.Printf("CreateVirtioScsiController: Received from client: %v", in) @@ -260,6 +266,7 @@ func (s *Server) ListVirtioScsiLuns(_ context.Context, in *pb.ListVirtioScsiLuns r := &result[i] Blobarray[i] = &pb.VirtioScsiLun{VolumeId: &pc.ObjectKey{Value: r.Ctrlr}} } + sortScsiLuns(Blobarray) page := server.LimitToPage(pageToken, Blobarray) return &pb.ListVirtioScsiLunsResponse{VirtioScsiLuns: page.List, NextPageToken: page.NextToken}, nil }