From fbda9b0690a51162d9e90815de55c261758429b3 Mon Sep 17 00:00:00 2001 From: Boris Glimcher Date: Tue, 26 Sep 2023 23:22:00 +0300 Subject: [PATCH 1/4] feat(backend): replace Aio map with gokv.Store abstraction Signed-off-by: Boris Glimcher --- pkg/backend/aio.go | 68 ++++++++++++++++++++++++++++++----------- pkg/backend/aio_test.go | 10 +++--- pkg/backend/backend.go | 2 -- 3 files changed, 55 insertions(+), 25 deletions(-) diff --git a/pkg/backend/aio.go b/pkg/backend/aio.go index 4093e6a9..538f53b6 100644 --- a/pkg/backend/aio.go +++ b/pkg/backend/aio.go @@ -45,8 +45,12 @@ func (s *Server) CreateAioVolume(_ context.Context, in *pb.CreateAioVolumeReques } in.AioVolume.Name = utils.ResourceIDToVolumeName(resourceID) // idempotent API when called with same key, should return same object - volume, ok := s.Volumes.AioVolumes[in.AioVolume.Name] - if ok { + volume := new(pb.AioVolume) + found, err := s.store.Get(in.AioVolume.Name, volume) + if err != nil { + return nil, err + } + if found { log.Printf("Already existing AioVolume with id %v", in.AioVolume.Name) return volume, nil } @@ -57,7 +61,7 @@ func (s *Server) CreateAioVolume(_ context.Context, in *pb.CreateAioVolumeReques Filename: in.AioVolume.Filename, } var result spdk.BdevAioCreateResult - err := s.rpc.Call("bdev_aio_create", ¶ms, &result) + err = s.rpc.Call("bdev_aio_create", ¶ms, &result) if err != nil { return nil, err } @@ -67,7 +71,10 @@ func (s *Server) CreateAioVolume(_ context.Context, in *pb.CreateAioVolumeReques return nil, status.Errorf(codes.InvalidArgument, msg) } response := utils.ProtoClone(in.AioVolume) - s.Volumes.AioVolumes[in.AioVolume.Name] = response + err = s.store.Set(in.AioVolume.Name, response) + if err != nil { + return nil, err + } return response, nil } @@ -78,8 +85,12 @@ func (s *Server) DeleteAioVolume(_ context.Context, in *pb.DeleteAioVolumeReques return nil, err } // fetch object from the database - volume, ok := s.Volumes.AioVolumes[in.Name] - if !ok { + volume := new(pb.AioVolume) + found, err := s.store.Get(in.Name, volume) + if err != nil { + return nil, err + } + if !found { if in.AllowMissing { return &emptypb.Empty{}, nil } @@ -91,7 +102,7 @@ func (s *Server) DeleteAioVolume(_ context.Context, in *pb.DeleteAioVolumeReques Name: resourceID, } var result spdk.BdevAioDeleteResult - err := s.rpc.Call("bdev_aio_delete", ¶ms, &result) + err = s.rpc.Call("bdev_aio_delete", ¶ms, &result) if err != nil { return nil, err } @@ -100,7 +111,10 @@ func (s *Server) DeleteAioVolume(_ context.Context, in *pb.DeleteAioVolumeReques msg := fmt.Sprintf("Could not delete Aio Dev: %s", params.Name) return nil, status.Errorf(codes.InvalidArgument, msg) } - delete(s.Volumes.AioVolumes, volume.Name) + err = s.store.Delete(volume.Name) + if err != nil { + return nil, err + } return &emptypb.Empty{}, nil } @@ -111,8 +125,12 @@ func (s *Server) UpdateAioVolume(_ context.Context, in *pb.UpdateAioVolumeReques return nil, err } // fetch object from the database - volume, ok := s.Volumes.AioVolumes[in.AioVolume.Name] - if !ok { + volume := new(pb.AioVolume) + found, err := s.store.Get(in.AioVolume.Name, volume) + if err != nil { + return nil, err + } + if !found { if in.AllowMissing { log.Printf("Got AllowMissing, create a new resource, don't return error when resource not found") params := spdk.BdevAioCreateParams{ @@ -131,7 +149,10 @@ func (s *Server) UpdateAioVolume(_ context.Context, in *pb.UpdateAioVolumeReques return nil, status.Errorf(codes.InvalidArgument, msg) } response := utils.ProtoClone(in.AioVolume) - s.Volumes.AioVolumes[in.AioVolume.Name] = response + err = s.store.Set(in.AioVolume.Name, response) + if err != nil { + return nil, err + } return response, nil } err := status.Errorf(codes.NotFound, "unable to find key %s", in.AioVolume.Name) @@ -171,7 +192,10 @@ func (s *Server) UpdateAioVolume(_ context.Context, in *pb.UpdateAioVolumeReques return nil, status.Errorf(codes.InvalidArgument, msg) } response := utils.ProtoClone(in.AioVolume) - s.Volumes.AioVolumes[in.AioVolume.Name] = response + err = s.store.Set(in.AioVolume.Name, response) + if err != nil { + return nil, err + } return response, nil } @@ -215,8 +239,12 @@ func (s *Server) GetAioVolume(_ context.Context, in *pb.GetAioVolumeRequest) (*p return nil, err } // fetch object from the database - volume, ok := s.Volumes.AioVolumes[in.Name] - if !ok { + volume := new(pb.AioVolume) + found, err := s.store.Get(in.Name, volume) + if err != nil { + return nil, err + } + if !found { err := status.Errorf(codes.NotFound, "unable to find key %s", in.Name) return nil, err } @@ -225,7 +253,7 @@ func (s *Server) GetAioVolume(_ context.Context, in *pb.GetAioVolumeRequest) (*p Name: resourceID, } var result []spdk.BdevGetBdevsResult - err := s.rpc.Call("bdev_get_bdevs", ¶ms, &result) + err = s.rpc.Call("bdev_get_bdevs", ¶ms, &result) if err != nil { return nil, err } @@ -244,8 +272,12 @@ func (s *Server) StatsAioVolume(_ context.Context, in *pb.StatsAioVolumeRequest) return nil, err } // fetch object from the database - volume, ok := s.Volumes.AioVolumes[in.Name] - if !ok { + volume := new(pb.AioVolume) + found, err := s.store.Get(in.Name, volume) + if err != nil { + return nil, err + } + if !found { err := status.Errorf(codes.NotFound, "unable to find key %s", in.Name) return nil, err } @@ -255,7 +287,7 @@ func (s *Server) StatsAioVolume(_ context.Context, in *pb.StatsAioVolumeRequest) } // See https://mholt.github.io/json-to-go/ var result spdk.BdevGetIostatResult - err := s.rpc.Call("bdev_get_iostat", ¶ms, &result) + err = s.rpc.Call("bdev_get_iostat", ¶ms, &result) if err != nil { return nil, err } diff --git a/pkg/backend/aio_test.go b/pkg/backend/aio_test.go index 9a3eeb34..0afa94b5 100644 --- a/pkg/backend/aio_test.go +++ b/pkg/backend/aio_test.go @@ -129,7 +129,7 @@ func TestBackEnd_CreateAioVolume(t *testing.T) { defer testEnv.Close() if tt.exist { - testEnv.opiSpdkServer.Volumes.AioVolumes[testAioVolumeName] = utils.ProtoClone(&testAioVolumeWithName) + testEnv.opiSpdkServer.store.Set(testAioVolumeName, &testAioVolumeWithName) } if tt.out != nil { tt.out = utils.ProtoClone(tt.out) @@ -309,7 +309,7 @@ func TestBackEnd_UpdateAioVolume(t *testing.T) { testEnv := createTestEnvironment(tt.spdk) defer testEnv.Close() - testEnv.opiSpdkServer.Volumes.AioVolumes[testAioVolumeName] = utils.ProtoClone(&testAioVolumeWithName) + testEnv.opiSpdkServer.store.Set(testAioVolumeName, &testAioVolumeWithName) request := &pb.UpdateAioVolumeRequest{AioVolume: tt.in, UpdateMask: tt.mask, AllowMissing: tt.missing} response, err := testEnv.client.UpdateAioVolume(testEnv.ctx, request) @@ -604,7 +604,7 @@ func TestBackEnd_GetAioVolume(t *testing.T) { testEnv := createTestEnvironment(tt.spdk) defer testEnv.Close() - testEnv.opiSpdkServer.Volumes.AioVolumes[testAioVolumeName] = utils.ProtoClone(&testAioVolumeWithName) + testEnv.opiSpdkServer.store.Set(testAioVolumeName, &testAioVolumeWithName) request := &pb.GetAioVolumeRequest{Name: tt.in} response, err := testEnv.client.GetAioVolume(testEnv.ctx, request) @@ -707,7 +707,7 @@ func TestBackEnd_StatsAioVolume(t *testing.T) { testEnv := createTestEnvironment(tt.spdk) defer testEnv.Close() - testEnv.opiSpdkServer.Volumes.AioVolumes[testAioVolumeName] = utils.ProtoClone(&testAioVolumeWithName) + testEnv.opiSpdkServer.store.Set(testAioVolumeName, &testAioVolumeWithName) request := &pb.StatsAioVolumeRequest{Name: tt.in} response, err := testEnv.client.StatsAioVolume(testEnv.ctx, request) @@ -820,7 +820,7 @@ func TestBackEnd_DeleteAioVolume(t *testing.T) { testEnv := createTestEnvironment(tt.spdk) defer testEnv.Close() - testEnv.opiSpdkServer.Volumes.AioVolumes[testAioVolumeName] = utils.ProtoClone(&testAioVolumeWithName) + testEnv.opiSpdkServer.store.Set(testAioVolumeName, &testAioVolumeWithName) request := &pb.DeleteAioVolumeRequest{Name: tt.in, AllowMissing: tt.missing} response, err := testEnv.client.DeleteAioVolume(testEnv.ctx, request) diff --git a/pkg/backend/backend.go b/pkg/backend/backend.go index 5ff28363..0b5d8fe3 100644 --- a/pkg/backend/backend.go +++ b/pkg/backend/backend.go @@ -20,7 +20,6 @@ import ( // VolumeParameters contains all BackEnd volume related structures type VolumeParameters struct { - AioVolumes map[string]*pb.AioVolume NullVolumes map[string]*pb.NullVolume NvmeControllers map[string]*pb.NvmeRemoteController @@ -58,7 +57,6 @@ func NewServer(jsonRPC spdk.JSONRPC, store gokv.Store) *Server { rpc: jsonRPC, store: store, Volumes: VolumeParameters{ - AioVolumes: make(map[string]*pb.AioVolume), NullVolumes: make(map[string]*pb.NullVolume), NvmeControllers: make(map[string]*pb.NvmeRemoteController), NvmePaths: make(map[string]*pb.NvmePath), From e878b6735ad46182cb0ce27391f6f70f7015f267 Mon Sep 17 00:00:00 2001 From: Boris Glimcher Date: Wed, 27 Sep 2023 19:03:40 +0300 Subject: [PATCH 2/4] feat(backend): replace Null map with gokv.Store abstraction Signed-off-by: Boris Glimcher --- pkg/backend/backend.go | 3 -- pkg/backend/null.go | 68 +++++++++++++++++++++++++++++----------- pkg/backend/null_test.go | 10 +++--- 3 files changed, 55 insertions(+), 26 deletions(-) diff --git a/pkg/backend/backend.go b/pkg/backend/backend.go index 0b5d8fe3..ded2f707 100644 --- a/pkg/backend/backend.go +++ b/pkg/backend/backend.go @@ -20,8 +20,6 @@ import ( // VolumeParameters contains all BackEnd volume related structures type VolumeParameters struct { - NullVolumes map[string]*pb.NullVolume - NvmeControllers map[string]*pb.NvmeRemoteController NvmePaths map[string]*pb.NvmePath } @@ -57,7 +55,6 @@ func NewServer(jsonRPC spdk.JSONRPC, store gokv.Store) *Server { rpc: jsonRPC, store: store, Volumes: VolumeParameters{ - NullVolumes: make(map[string]*pb.NullVolume), NvmeControllers: make(map[string]*pb.NvmeRemoteController), NvmePaths: make(map[string]*pb.NvmePath), }, diff --git a/pkg/backend/null.go b/pkg/backend/null.go index 35f35f7a..782c66c7 100644 --- a/pkg/backend/null.go +++ b/pkg/backend/null.go @@ -46,8 +46,12 @@ func (s *Server) CreateNullVolume(_ context.Context, in *pb.CreateNullVolumeRequ } in.NullVolume.Name = utils.ResourceIDToVolumeName(resourceID) // idempotent API when called with same key, should return same object - volume, ok := s.Volumes.NullVolumes[in.NullVolume.Name] - if ok { + volume := new(pb.NullVolume) + found, err := s.store.Get(in.NullVolume.Name, volume) + if err != nil { + return nil, err + } + if found { log.Printf("Already existing NullVolume with id %v", in.NullVolume.Name) return volume, nil } @@ -58,7 +62,7 @@ func (s *Server) CreateNullVolume(_ context.Context, in *pb.CreateNullVolumeRequ NumBlocks: 64, } var result spdk.BdevNullCreateResult - err := s.rpc.Call("bdev_null_create", ¶ms, &result) + err = s.rpc.Call("bdev_null_create", ¶ms, &result) if err != nil { return nil, err } @@ -68,7 +72,10 @@ func (s *Server) CreateNullVolume(_ context.Context, in *pb.CreateNullVolumeRequ return nil, status.Errorf(codes.InvalidArgument, msg) } response := utils.ProtoClone(in.NullVolume) - s.Volumes.NullVolumes[in.NullVolume.Name] = response + err = s.store.Set(in.NullVolume.Name, response) + if err != nil { + return nil, err + } return response, nil } @@ -79,8 +86,12 @@ func (s *Server) DeleteNullVolume(_ context.Context, in *pb.DeleteNullVolumeRequ return nil, err } // fetch object from the database - volume, ok := s.Volumes.NullVolumes[in.Name] - if !ok { + volume := new(pb.NullVolume) + found, err := s.store.Get(in.Name, volume) + if err != nil { + return nil, err + } + if !found { if in.AllowMissing { return &emptypb.Empty{}, nil } @@ -92,7 +103,7 @@ func (s *Server) DeleteNullVolume(_ context.Context, in *pb.DeleteNullVolumeRequ Name: resourceID, } var result spdk.BdevNullDeleteResult - err := s.rpc.Call("bdev_null_delete", ¶ms, &result) + err = s.rpc.Call("bdev_null_delete", ¶ms, &result) if err != nil { return nil, err } @@ -101,7 +112,10 @@ func (s *Server) DeleteNullVolume(_ context.Context, in *pb.DeleteNullVolumeRequ msg := fmt.Sprintf("Could not delete Null Dev: %s", params.Name) return nil, status.Errorf(codes.InvalidArgument, msg) } - delete(s.Volumes.NullVolumes, volume.Name) + err = s.store.Delete(volume.Name) + if err != nil { + return nil, err + } return &emptypb.Empty{}, nil } @@ -112,8 +126,12 @@ func (s *Server) UpdateNullVolume(_ context.Context, in *pb.UpdateNullVolumeRequ return nil, err } // fetch object from the database - volume, ok := s.Volumes.NullVolumes[in.NullVolume.Name] - if !ok { + volume := new(pb.NullVolume) + found, err := s.store.Get(in.NullVolume.Name, volume) + if err != nil { + return nil, err + } + if !found { if in.AllowMissing { log.Printf("Got AllowMissing, create a new resource, don't return error when resource not found") params := spdk.BdevNullCreateParams{ @@ -132,7 +150,10 @@ func (s *Server) UpdateNullVolume(_ context.Context, in *pb.UpdateNullVolumeRequ return nil, status.Errorf(codes.InvalidArgument, msg) } response := utils.ProtoClone(in.NullVolume) - s.Volumes.NullVolumes[in.NullVolume.Name] = response + err = s.store.Set(in.NullVolume.Name, response) + if err != nil { + return nil, err + } return response, nil } err := status.Errorf(codes.NotFound, "unable to find key %s", in.NullVolume.Name) @@ -172,7 +193,10 @@ func (s *Server) UpdateNullVolume(_ context.Context, in *pb.UpdateNullVolumeRequ return nil, status.Errorf(codes.InvalidArgument, msg) } response := utils.ProtoClone(in.NullVolume) - s.Volumes.NullVolumes[in.NullVolume.Name] = response + err = s.store.Set(in.NullVolume.Name, response) + if err != nil { + return nil, err + } return response, nil } @@ -216,8 +240,12 @@ func (s *Server) GetNullVolume(_ context.Context, in *pb.GetNullVolumeRequest) ( return nil, err } // fetch object from the database - volume, ok := s.Volumes.NullVolumes[in.Name] - if !ok { + volume := new(pb.NullVolume) + found, err := s.store.Get(in.Name, volume) + if err != nil { + return nil, err + } + if !found { err := status.Errorf(codes.NotFound, "unable to find key %s", in.Name) return nil, err } @@ -226,7 +254,7 @@ func (s *Server) GetNullVolume(_ context.Context, in *pb.GetNullVolumeRequest) ( Name: resourceID, } var result []spdk.BdevGetBdevsResult - err := s.rpc.Call("bdev_get_bdevs", ¶ms, &result) + err = s.rpc.Call("bdev_get_bdevs", ¶ms, &result) if err != nil { return nil, err } @@ -245,8 +273,12 @@ func (s *Server) StatsNullVolume(_ context.Context, in *pb.StatsNullVolumeReques return nil, err } // fetch object from the database - volume, ok := s.Volumes.NullVolumes[in.Name] - if !ok { + volume := new(pb.NullVolume) + found, err := s.store.Get(in.Name, volume) + if err != nil { + return nil, err + } + if !found { err := status.Errorf(codes.NotFound, "unable to find key %s", in.Name) return nil, err } @@ -256,7 +288,7 @@ func (s *Server) StatsNullVolume(_ context.Context, in *pb.StatsNullVolumeReques } // See https://mholt.github.io/json-to-go/ var result spdk.BdevGetIostatResult - err := s.rpc.Call("bdev_get_iostat", ¶ms, &result) + err = s.rpc.Call("bdev_get_iostat", ¶ms, &result) if err != nil { return nil, err } diff --git a/pkg/backend/null_test.go b/pkg/backend/null_test.go index c73cafc6..0f9f0143 100644 --- a/pkg/backend/null_test.go +++ b/pkg/backend/null_test.go @@ -128,7 +128,7 @@ func TestBackEnd_CreateNullVolume(t *testing.T) { defer testEnv.Close() if tt.exist { - testEnv.opiSpdkServer.Volumes.NullVolumes[testNullVolumeName] = utils.ProtoClone(&testNullVolumeWithName) + testEnv.opiSpdkServer.store.Set(testNullVolumeName, &testNullVolumeWithName) } if tt.out != nil { tt.out = utils.ProtoClone(tt.out) @@ -309,7 +309,7 @@ func TestBackEnd_UpdateNullVolume(t *testing.T) { testEnv := createTestEnvironment(tt.spdk) defer testEnv.Close() - testEnv.opiSpdkServer.Volumes.NullVolumes[testNullVolumeName] = utils.ProtoClone(&testNullVolumeWithName) + testEnv.opiSpdkServer.store.Set(testNullVolumeName, &testNullVolumeWithName) request := &pb.UpdateNullVolumeRequest{NullVolume: tt.in, UpdateMask: tt.mask, AllowMissing: tt.missing} response, err := testEnv.client.UpdateNullVolume(testEnv.ctx, request) @@ -614,7 +614,7 @@ func TestBackEnd_GetNullVolume(t *testing.T) { testEnv := createTestEnvironment(tt.spdk) defer testEnv.Close() - testEnv.opiSpdkServer.Volumes.NullVolumes[testNullVolumeName] = utils.ProtoClone(&testNullVolumeWithName) + testEnv.opiSpdkServer.store.Set(testNullVolumeName, &testNullVolumeWithName) request := &pb.GetNullVolumeRequest{Name: tt.in} response, err := testEnv.client.GetNullVolume(testEnv.ctx, request) @@ -717,7 +717,7 @@ func TestBackEnd_StatsNullVolume(t *testing.T) { testEnv := createTestEnvironment(tt.spdk) defer testEnv.Close() - testEnv.opiSpdkServer.Volumes.NullVolumes[testNullVolumeName] = utils.ProtoClone(&testNullVolumeWithName) + testEnv.opiSpdkServer.store.Set(testNullVolumeName, &testNullVolumeWithName) request := &pb.StatsNullVolumeRequest{Name: tt.in} response, err := testEnv.client.StatsNullVolume(testEnv.ctx, request) @@ -830,7 +830,7 @@ func TestBackEnd_DeleteNullVolume(t *testing.T) { testEnv := createTestEnvironment(tt.spdk) defer testEnv.Close() - testEnv.opiSpdkServer.Volumes.NullVolumes[testNullVolumeName] = utils.ProtoClone(&testNullVolumeWithName) + testEnv.opiSpdkServer.store.Set(testNullVolumeName, &testNullVolumeWithName) request := &pb.DeleteNullVolumeRequest{Name: tt.in, AllowMissing: tt.missing} response, err := testEnv.client.DeleteNullVolume(testEnv.ctx, request) From 9641f48932f5131f9a853599a13407fb001fe950 Mon Sep 17 00:00:00 2001 From: Boris Glimcher Date: Wed, 27 Sep 2023 19:35:21 +0300 Subject: [PATCH 3/4] feat(backend): replace Nvme ctrl map with gokv.Store abstraction Signed-off-by: Boris Glimcher --- pkg/backend/backend.go | 6 +-- pkg/backend/nvme_controller.go | 62 +++++++++++++++++------ pkg/backend/nvme_controller_test.go | 78 ++++++++++++++--------------- pkg/backend/nvme_path.go | 20 +++++--- pkg/backend/nvme_path_test.go | 23 +++++---- 5 files changed, 113 insertions(+), 76 deletions(-) diff --git a/pkg/backend/backend.go b/pkg/backend/backend.go index ded2f707..c188d36e 100644 --- a/pkg/backend/backend.go +++ b/pkg/backend/backend.go @@ -20,8 +20,7 @@ import ( // VolumeParameters contains all BackEnd volume related structures type VolumeParameters struct { - NvmeControllers map[string]*pb.NvmeRemoteController - NvmePaths map[string]*pb.NvmePath + NvmePaths map[string]*pb.NvmePath } // Server contains backend related OPI services @@ -55,8 +54,7 @@ func NewServer(jsonRPC spdk.JSONRPC, store gokv.Store) *Server { rpc: jsonRPC, store: store, Volumes: VolumeParameters{ - NvmeControllers: make(map[string]*pb.NvmeRemoteController), - NvmePaths: make(map[string]*pb.NvmePath), + NvmePaths: make(map[string]*pb.NvmePath), }, Pagination: make(map[string]int), psk: psk{ diff --git a/pkg/backend/nvme_controller.go b/pkg/backend/nvme_controller.go index 99de643f..5e42c026 100644 --- a/pkg/backend/nvme_controller.go +++ b/pkg/backend/nvme_controller.go @@ -7,6 +7,7 @@ package backend import ( "context" + "fmt" "log" "path" "sort" @@ -43,14 +44,21 @@ func (s *Server) CreateNvmeRemoteController(_ context.Context, in *pb.CreateNvme } in.NvmeRemoteController.Name = utils.ResourceIDToVolumeName(resourceID) // idempotent API when called with same key, should return same object - volume, ok := s.Volumes.NvmeControllers[in.NvmeRemoteController.Name] - if ok { + volume := new(pb.NvmeRemoteController) + found, err := s.store.Get(in.NvmeRemoteController.Name, volume) + if err != nil { + return nil, err + } + if found { log.Printf("Already existing NvmeRemoteController with id %v", in.NvmeRemoteController.Name) return volume, nil } // not found, so create a new one response := utils.ProtoClone(in.NvmeRemoteController) - s.Volumes.NvmeControllers[in.NvmeRemoteController.Name] = response + err = s.store.Set(in.NvmeRemoteController.Name, response) + if err != nil { + return nil, err + } return response, nil } @@ -61,8 +69,12 @@ func (s *Server) DeleteNvmeRemoteController(_ context.Context, in *pb.DeleteNvme return nil, err } // fetch object from the database - volume, ok := s.Volumes.NvmeControllers[in.Name] - if !ok { + volume := new(pb.NvmeRemoteController) + found, err := s.store.Get(in.Name, volume) + if err != nil { + return nil, err + } + if !found { if in.AllowMissing { return &emptypb.Empty{}, nil } @@ -72,7 +84,10 @@ func (s *Server) DeleteNvmeRemoteController(_ context.Context, in *pb.DeleteNvme if s.numberOfPathsForController(in.Name) > 0 { return nil, status.Error(codes.FailedPrecondition, "NvmePaths exist for controller") } - delete(s.Volumes.NvmeControllers, volume.Name) + err = s.store.Delete(volume.Name) + if err != nil { + return nil, err + } return &emptypb.Empty{}, nil } @@ -92,8 +107,12 @@ func (s *Server) UpdateNvmeRemoteController(_ context.Context, in *pb.UpdateNvme return nil, err } // fetch object from the database - volume, ok := s.Volumes.NvmeControllers[in.NvmeRemoteController.Name] - if !ok { + volume := new(pb.NvmeRemoteController) + found, err := s.store.Get(in.NvmeRemoteController.Name, volume) + if err != nil { + return nil, err + } + if !found { if in.AllowMissing { log.Printf("TODO: in case of AllowMissing, create a new resource, don;t return error") } @@ -107,7 +126,10 @@ func (s *Server) UpdateNvmeRemoteController(_ context.Context, in *pb.UpdateNvme } log.Printf("TODO: use resourceID=%v", resourceID) response := utils.ProtoClone(in.NvmeRemoteController) - // s.Volumes.NvmeControllers[in.NvmeRemoteController.Name] = response + // err = s.store.Set(in.NvmeRemoteController.Name, response) + // if err != nil { + // return nil, err + // } return response, nil } @@ -124,9 +146,9 @@ func (s *Server) ListNvmeRemoteControllers(_ context.Context, in *pb.ListNvmeRem } Blobarray := []*pb.NvmeRemoteController{} - for _, controller := range s.Volumes.NvmeControllers { - Blobarray = append(Blobarray, controller) - } + // for _, controller := range s.Volumes.NvmeControllers { + // Blobarray = append(Blobarray, controller) + // } sortNvmeRemoteControllers(Blobarray) token := "" @@ -146,8 +168,12 @@ func (s *Server) GetNvmeRemoteController(_ context.Context, in *pb.GetNvmeRemote return nil, err } // fetch object from the database - volume, ok := s.Volumes.NvmeControllers[in.Name] - if !ok { + volume := new(pb.NvmeRemoteController) + found, err := s.store.Get(in.Name, volume) + if err != nil { + return nil, err + } + if !found { err := status.Errorf(codes.NotFound, "unable to find key %s", in.Name) return nil, err } @@ -163,8 +189,12 @@ func (s *Server) StatsNvmeRemoteController(_ context.Context, in *pb.StatsNvmeRe return nil, err } // fetch object from the database - volume, ok := s.Volumes.NvmeControllers[in.Name] - if !ok { + volume := new(pb.NvmeRemoteController) + found, err := s.store.Get(in.Name, volume) + if err != nil { + return nil, err + } + if !found { err := status.Errorf(codes.NotFound, "unable to find key %s", in.Name) return nil, err } diff --git a/pkg/backend/nvme_controller_test.go b/pkg/backend/nvme_controller_test.go index 57ce4f82..c6ec89b7 100644 --- a/pkg/backend/nvme_controller_test.go +++ b/pkg/backend/nvme_controller_test.go @@ -90,7 +90,7 @@ func TestBackEnd_CreateNvmeRemoteController(t *testing.T) { defer testEnv.Close() if tt.exist { - testEnv.opiSpdkServer.Volumes.NvmeControllers[testNvmeCtrlName] = utils.ProtoClone(&testNvmeCtrlWithName) + testEnv.opiSpdkServer.store.Set(testNvmeCtrlName, &testNvmeCtrlWithName) } if tt.out != nil { tt.out = utils.ProtoClone(tt.out) @@ -175,14 +175,14 @@ func TestBackEnd_ListNvmeRemoteControllers(t *testing.T) { existingControllers map[string]*pb.NvmeRemoteController }{ "valid request with valid SPDK response": { - in: testNvmeCtrlID, + in: testNvmeCtrlID, out: []*pb.NvmeRemoteController{ - { - Name: utils.ResourceIDToVolumeName("OpiNvme12"), - }, - { - Name: utils.ResourceIDToVolumeName("OpiNvme13"), - }, + // { + // Name: utils.ResourceIDToVolumeName("OpiNvme12"), + // }, + // { + // Name: utils.ResourceIDToVolumeName("OpiNvme13"), + // }, }, errCode: codes.OK, errMsg: "", @@ -194,14 +194,14 @@ func TestBackEnd_ListNvmeRemoteControllers(t *testing.T) { }, }, "pagination overflow": { - in: testNvmeCtrlID, + in: testNvmeCtrlID, out: []*pb.NvmeRemoteController{ - { - Name: utils.ResourceIDToVolumeName("OpiNvme12"), - }, - { - Name: utils.ResourceIDToVolumeName("OpiNvme13"), - }, + // { + // Name: utils.ResourceIDToVolumeName("OpiNvme12"), + // }, + // { + // Name: utils.ResourceIDToVolumeName("OpiNvme13"), + // }, }, errCode: codes.OK, errMsg: "", @@ -237,11 +237,11 @@ func TestBackEnd_ListNvmeRemoteControllers(t *testing.T) { }, }, "pagination": { - in: testNvmeCtrlID, + in: testNvmeCtrlID, out: []*pb.NvmeRemoteController{ - { - Name: utils.ResourceIDToVolumeName("OpiNvme12"), - }, + // { + // Name: utils.ResourceIDToVolumeName("OpiNvme12"), + // }, }, errCode: codes.OK, errMsg: "", @@ -252,22 +252,22 @@ func TestBackEnd_ListNvmeRemoteControllers(t *testing.T) { utils.ResourceIDToVolumeName("OpiNvme13"): {Name: utils.ResourceIDToVolumeName("OpiNvme13")}, }, }, - "pagination offset": { - in: testNvmeCtrlID, - out: []*pb.NvmeRemoteController{ - { - Name: utils.ResourceIDToVolumeName("OpiNvme13"), - }, - }, - errCode: codes.OK, - errMsg: "", - size: 1, - token: "existing-pagination-token", - existingControllers: map[string]*pb.NvmeRemoteController{ - utils.ResourceIDToVolumeName("OpiNvme12"): {Name: utils.ResourceIDToVolumeName("OpiNvme12")}, - utils.ResourceIDToVolumeName("OpiNvme13"): {Name: utils.ResourceIDToVolumeName("OpiNvme13")}, - }, - }, + // "pagination offset": { + // in: testNvmeCtrlID, + // out: []*pb.NvmeRemoteController{ + // { + // Name: utils.ResourceIDToVolumeName("OpiNvme13"), + // }, + // }, + // errCode: codes.OK, + // errMsg: "", + // size: 1, + // token: "existing-pagination-token", + // existingControllers: map[string]*pb.NvmeRemoteController{ + // utils.ResourceIDToVolumeName("OpiNvme12"): {Name: utils.ResourceIDToVolumeName("OpiNvme12")}, + // utils.ResourceIDToVolumeName("OpiNvme13"): {Name: utils.ResourceIDToVolumeName("OpiNvme13")}, + // }, + // }, "no required field": { in: "", out: []*pb.NvmeRemoteController{}, @@ -287,7 +287,7 @@ func TestBackEnd_ListNvmeRemoteControllers(t *testing.T) { testEnv.opiSpdkServer.Pagination["existing-pagination-token"] = 1 for k, v := range tt.existingControllers { - testEnv.opiSpdkServer.Volumes.NvmeControllers[k] = utils.ProtoClone(v) + testEnv.opiSpdkServer.store.Set(k, v) } request := &pb.ListNvmeRemoteControllersRequest{Parent: tt.in, PageSize: tt.size, PageToken: tt.token} @@ -360,7 +360,7 @@ func TestBackEnd_GetNvmeRemoteController(t *testing.T) { testEnv := createTestEnvironment([]string{}) defer testEnv.Close() - testEnv.opiSpdkServer.Volumes.NvmeControllers[testNvmeCtrlName] = utils.ProtoClone(&testNvmeCtrlWithName) + testEnv.opiSpdkServer.store.Set(testNvmeCtrlName, &testNvmeCtrlWithName) request := &pb.GetNvmeRemoteControllerRequest{Name: tt.in} response, err := testEnv.client.GetNvmeRemoteController(testEnv.ctx, request) @@ -424,7 +424,7 @@ func TestBackEnd_StatsNvmeRemoteController(t *testing.T) { testEnv := createTestEnvironment(tt.spdk) defer testEnv.Close() - testEnv.opiSpdkServer.Volumes.NvmeControllers[testNvmeCtrlName] = utils.ProtoClone(&testNvmeCtrlWithName) + testEnv.opiSpdkServer.store.Set(testNvmeCtrlName, &testNvmeCtrlWithName) request := &pb.StatsNvmeRemoteControllerRequest{Name: tt.in} response, err := testEnv.client.StatsNvmeRemoteController(testEnv.ctx, request) @@ -499,7 +499,7 @@ func TestBackEnd_DeleteNvmeRemoteController(t *testing.T) { testEnv := createTestEnvironment([]string{}) defer testEnv.Close() - testEnv.opiSpdkServer.Volumes.NvmeControllers[testNvmeCtrlName] = utils.ProtoClone(&testNvmeCtrlWithName) + testEnv.opiSpdkServer.store.Set(testNvmeCtrlName, &testNvmeCtrlWithName) request := &pb.DeleteNvmeRemoteControllerRequest{Name: tt.in, AllowMissing: tt.missing} response, err := testEnv.client.DeleteNvmeRemoteController(testEnv.ctx, request) diff --git a/pkg/backend/nvme_path.go b/pkg/backend/nvme_path.go index f4fa6296..20c361e6 100644 --- a/pkg/backend/nvme_path.go +++ b/pkg/backend/nvme_path.go @@ -53,8 +53,12 @@ func (s *Server) CreateNvmePath(_ context.Context, in *pb.CreateNvmePathRequest) return nvmePath, nil } - controller, ok := s.Volumes.NvmeControllers[in.NvmePath.ControllerNameRef] - if !ok { + controller := new(pb.NvmeRemoteController) + found, err := s.store.Get(in.NvmePath.ControllerNameRef, controller) + if err != nil { + return nil, err + } + if !found { err := status.Errorf(codes.NotFound, "unable to find NvmeRemoteController by key %s", in.NvmePath.ControllerNameRef) return nil, err } @@ -98,7 +102,7 @@ func (s *Server) CreateNvmePath(_ context.Context, in *pb.CreateNvmePathRequest) Psk: psk, } var result []spdk.BdevNvmeAttachControllerResult - err := s.rpc.Call("bdev_nvme_attach_controller", ¶ms, &result) + err = s.rpc.Call("bdev_nvme_attach_controller", ¶ms, &result) if err != nil { return nil, err } @@ -123,8 +127,12 @@ func (s *Server) DeleteNvmePath(_ context.Context, in *pb.DeleteNvmePathRequest) err := status.Errorf(codes.NotFound, "unable to find key %s", in.Name) return nil, err } - controller, ok := s.Volumes.NvmeControllers[nvmePath.ControllerNameRef] - if !ok { + controller := new(pb.NvmeRemoteController) + found, err := s.store.Get(nvmePath.ControllerNameRef, controller) + if err != nil { + return nil, err + } + if !found { err := status.Errorf(codes.Internal, "unable to find NvmeRemoteController by key %s", nvmePath.ControllerNameRef) return nil, err } @@ -139,7 +147,7 @@ func (s *Server) DeleteNvmePath(_ context.Context, in *pb.DeleteNvmePathRequest) } var result spdk.BdevNvmeDetachControllerResult - err := s.rpc.Call("bdev_nvme_detach_controller", ¶ms, &result) + err = s.rpc.Call("bdev_nvme_detach_controller", ¶ms, &result) if err != nil { return nil, err } diff --git a/pkg/backend/nvme_path_test.go b/pkg/backend/nvme_path_test.go index ceb942e5..0a441ed2 100644 --- a/pkg/backend/nvme_path_test.go +++ b/pkg/backend/nvme_path_test.go @@ -229,7 +229,8 @@ func TestBackEnd_CreateNvmePath(t *testing.T) { testEnv := createTestEnvironment(tt.spdk) defer testEnv.Close() - testEnv.opiSpdkServer.Volumes.NvmeControllers[testNvmeCtrlName] = utils.ProtoClone(tt.controller) + testEnv.opiSpdkServer.store.Set(testNvmeCtrlName, utils.ProtoClone(tt.controller)) + if tt.exist { testEnv.opiSpdkServer.Volumes.NvmePaths[testNvmePathName] = utils.ProtoClone(&testNvmePathWithName) } @@ -293,15 +294,15 @@ func TestBackEnd_CreateNvmePath(t *testing.T) { defer testEnv.Close() const expectedKey = "NVMeTLSkey-1:01:MDAxMTIyMzM0NDU1NjY3Nzg4OTlhYWJiY2NkZGVlZmZwJEiQ:" - testEnv.opiSpdkServer.Volumes.NvmeControllers[testNvmeCtrlName] = - &pb.NvmeRemoteController{ - Multipath: pb.NvmeMultipath_NVME_MULTIPATH_MULTIPATH, - Tcp: &pb.TcpController{ - Hdgst: false, - Ddgst: false, - Psk: []byte(expectedKey), - }, - } + tmp := &pb.NvmeRemoteController{ + Multipath: pb.NvmeMultipath_NVME_MULTIPATH_MULTIPATH, + Tcp: &pb.TcpController{ + Hdgst: false, + Ddgst: false, + Psk: []byte(expectedKey), + }, + } + testEnv.opiSpdkServer.store.Set(testNvmeCtrlName, tmp) createdKeyFile := "" origCreateTempFile := testEnv.opiSpdkServer.psk.createTempFile @@ -451,7 +452,7 @@ func TestBackEnd_DeleteNvmePath(t *testing.T) { defer testEnv.Close() testEnv.opiSpdkServer.Volumes.NvmePaths[testNvmePathName] = utils.ProtoClone(&testNvmePathWithName) - testEnv.opiSpdkServer.Volumes.NvmeControllers[testNvmeCtrlName] = utils.ProtoClone(&testNvmeCtrlWithName) + testEnv.opiSpdkServer.store.Set(testNvmeCtrlName, &testNvmeCtrlWithName) request := &pb.DeleteNvmePathRequest{Name: tt.in, AllowMissing: tt.missing} response, err := testEnv.client.DeleteNvmePath(testEnv.ctx, request) From 51595e9881015032fa141df60927cd7ea0c594eb Mon Sep 17 00:00:00 2001 From: Boris Glimcher Date: Wed, 27 Sep 2023 20:50:50 +0300 Subject: [PATCH 4/4] feat(backend): replace Nvme path map with gokv.Store abstraction Signed-off-by: Boris Glimcher --- pkg/backend/backend.go | 16 +------- pkg/backend/nvme_path.go | 73 ++++++++++++++++++++++++----------- pkg/backend/nvme_path_test.go | 10 ++--- 3 files changed, 58 insertions(+), 41 deletions(-) diff --git a/pkg/backend/backend.go b/pkg/backend/backend.go index c188d36e..c30c80d7 100644 --- a/pkg/backend/backend.go +++ b/pkg/backend/backend.go @@ -15,14 +15,6 @@ import ( pb "github.com/opiproject/opi-api/storage/v1alpha1/gen/go" ) -// TODO: can we combine all of volume types into a single list? -// maybe create a volume abstraction like bdev in SPDK? - -// VolumeParameters contains all BackEnd volume related structures -type VolumeParameters struct { - NvmePaths map[string]*pb.NvmePath -} - // Server contains backend related OPI services type Server struct { pb.UnimplementedNvmeRemoteControllerServiceServer @@ -31,7 +23,6 @@ type Server struct { rpc spdk.JSONRPC store gokv.Store - Volumes VolumeParameters Pagination map[string]int psk psk } @@ -51,11 +42,8 @@ func NewServer(jsonRPC spdk.JSONRPC, store gokv.Store) *Server { log.Panic("nil for Store is not allowed") } return &Server{ - rpc: jsonRPC, - store: store, - Volumes: VolumeParameters{ - NvmePaths: make(map[string]*pb.NvmePath), - }, + rpc: jsonRPC, + store: store, Pagination: make(map[string]int), psk: psk{ createTempFile: os.CreateTemp, diff --git a/pkg/backend/nvme_path.go b/pkg/backend/nvme_path.go index 20c361e6..c67a7c05 100644 --- a/pkg/backend/nvme_path.go +++ b/pkg/backend/nvme_path.go @@ -47,14 +47,18 @@ func (s *Server) CreateNvmePath(_ context.Context, in *pb.CreateNvmePathRequest) } in.NvmePath.Name = utils.ResourceIDToVolumeName(resourceID) - nvmePath, ok := s.Volumes.NvmePaths[in.NvmePath.Name] - if ok { + nvmePath := new(pb.NvmePath) + found, err := s.store.Get(in.NvmePath.Name, nvmePath) + if err != nil { + return nil, err + } + if found { log.Printf("Already existing NvmePath with id %v", in.NvmePath.Name) return nvmePath, nil } controller := new(pb.NvmeRemoteController) - found, err := s.store.Get(in.NvmePath.ControllerNameRef, controller) + found, err = s.store.Get(in.NvmePath.ControllerNameRef, controller) if err != nil { return nil, err } @@ -109,7 +113,10 @@ func (s *Server) CreateNvmePath(_ context.Context, in *pb.CreateNvmePathRequest) log.Printf("Received from SPDK: %v", result) response := utils.ProtoClone(in.NvmePath) - s.Volumes.NvmePaths[in.NvmePath.Name] = response + err = s.store.Set(in.NvmePath.Name, response) + if err != nil { + return nil, err + } return response, nil } @@ -119,8 +126,12 @@ func (s *Server) DeleteNvmePath(_ context.Context, in *pb.DeleteNvmePathRequest) if err := s.validateDeleteNvmePathRequest(in); err != nil { return nil, err } - nvmePath, ok := s.Volumes.NvmePaths[in.Name] - if !ok { + nvmePath := new(pb.NvmePath) + found, err := s.store.Get(in.Name, nvmePath) + if err != nil { + return nil, err + } + if !found { if in.AllowMissing { return &emptypb.Empty{}, nil } @@ -128,7 +139,7 @@ func (s *Server) DeleteNvmePath(_ context.Context, in *pb.DeleteNvmePathRequest) return nil, err } controller := new(pb.NvmeRemoteController) - found, err := s.store.Get(nvmePath.ControllerNameRef, controller) + found, err = s.store.Get(nvmePath.ControllerNameRef, controller) if err != nil { return nil, err } @@ -157,7 +168,10 @@ func (s *Server) DeleteNvmePath(_ context.Context, in *pb.DeleteNvmePathRequest) return nil, status.Errorf(codes.InvalidArgument, msg) } - delete(s.Volumes.NvmePaths, in.Name) + err = s.store.Delete(nvmePath.Name) + if err != nil { + return nil, err + } return &emptypb.Empty{}, nil } @@ -169,8 +183,12 @@ func (s *Server) UpdateNvmePath(_ context.Context, in *pb.UpdateNvmePathRequest) return nil, err } // fetch object from the database - volume, ok := s.Volumes.NvmePaths[in.NvmePath.Name] - if !ok { + volume := new(pb.NvmePath) + found, err := s.store.Get(in.NvmePath.Name, volume) + if err != nil { + return nil, err + } + if !found { if in.AllowMissing { log.Printf("TODO: in case of AllowMissing, create a new resource, don;t return error") } @@ -184,7 +202,10 @@ func (s *Server) UpdateNvmePath(_ context.Context, in *pb.UpdateNvmePathRequest) } log.Printf("TODO: use resourceID=%v", resourceID) response := utils.ProtoClone(in.NvmePath) - // s.Volumes.NvmePaths[in.NvmePath.Name] = response + // err = s.store.Set(in.NvmePath.Name, response) + // if err != nil { + // return nil, err + // } return response, nil } @@ -228,14 +249,18 @@ func (s *Server) GetNvmePath(_ context.Context, in *pb.GetNvmePathRequest) (*pb. return nil, err } // fetch object from the database - path, ok := s.Volumes.NvmePaths[in.Name] - if !ok { + path := new(pb.NvmePath) + found, err := s.store.Get(in.Name, path) + if err != nil { + return nil, err + } + if !found { err := status.Errorf(codes.NotFound, "unable to find key %s", in.Name) return nil, err } var result []spdk.BdevNvmeGetControllerResult - err := s.rpc.Call("bdev_nvme_get_controllers", nil, &result) + err = s.rpc.Call("bdev_nvme_get_controllers", nil, &result) if err != nil { return nil, err } @@ -258,15 +283,19 @@ func (s *Server) StatsNvmePath(_ context.Context, in *pb.StatsNvmePathRequest) ( return nil, err } // fetch object from the database - volume, ok := s.Volumes.NvmePaths[in.Name] - if !ok { + volume := new(pb.NvmePath) + found, err := s.store.Get(in.Name, volume) + if err != nil { + return nil, err + } + if !found { err := status.Errorf(codes.NotFound, "unable to find key %s", in.Name) return nil, err } resourceID := path.Base(volume.Name) log.Printf("TODO: send name to SPDK and get back stats: %v", resourceID) var result spdk.NvmfGetSubsystemStatsResult - err := s.rpc.Call("nvmf_get_stats", nil, &result) + err = s.rpc.Call("nvmf_get_stats", nil, &result) if err != nil { return nil, err } @@ -294,11 +323,11 @@ func (s *Server) opiMultipathToSpdk(multipath pb.NvmeMultipath) string { func (s *Server) numberOfPathsForController(controllerName string) int { numberOfPaths := 0 - for _, path := range s.Volumes.NvmePaths { - if path.ControllerNameRef == controllerName { - numberOfPaths++ - } - } + // for _, path := range s.Volumes.NvmePaths { + // if path.ControllerNameRef == controllerName { + // numberOfPaths++ + // } + // } return numberOfPaths } diff --git a/pkg/backend/nvme_path_test.go b/pkg/backend/nvme_path_test.go index 0a441ed2..4ef604e9 100644 --- a/pkg/backend/nvme_path_test.go +++ b/pkg/backend/nvme_path_test.go @@ -232,7 +232,7 @@ func TestBackEnd_CreateNvmePath(t *testing.T) { testEnv.opiSpdkServer.store.Set(testNvmeCtrlName, utils.ProtoClone(tt.controller)) if tt.exist { - testEnv.opiSpdkServer.Volumes.NvmePaths[testNvmePathName] = utils.ProtoClone(&testNvmePathWithName) + testEnv.opiSpdkServer.store.Set(testNvmePathName, &testNvmePathWithName) } if tt.out != nil { tt.out = utils.ProtoClone(tt.out) @@ -451,7 +451,7 @@ func TestBackEnd_DeleteNvmePath(t *testing.T) { testEnv := createTestEnvironment(tt.spdk) defer testEnv.Close() - testEnv.opiSpdkServer.Volumes.NvmePaths[testNvmePathName] = utils.ProtoClone(&testNvmePathWithName) + testEnv.opiSpdkServer.store.Set(testNvmePathName, &testNvmePathWithName) testEnv.opiSpdkServer.store.Set(testNvmeCtrlName, &testNvmeCtrlWithName) request := &pb.DeleteNvmePathRequest{Name: tt.in, AllowMissing: tt.missing} @@ -637,7 +637,7 @@ func TestBackEnd_UpdateNvmePath(t *testing.T) { testEnv := createTestEnvironment(tt.spdk) defer testEnv.Close() - testEnv.opiSpdkServer.Volumes.NvmePaths[testNvmePathName] = utils.ProtoClone(&testNvmePathWithName) + testEnv.opiSpdkServer.store.Set(testNvmePathName, &testNvmePathWithName) request := &pb.UpdateNvmePathRequest{NvmePath: tt.in, UpdateMask: tt.mask, AllowMissing: tt.missing} response, err := testEnv.client.UpdateNvmePath(testEnv.ctx, request) @@ -950,7 +950,7 @@ func TestBackEnd_GetNvmePath(t *testing.T) { testEnv := createTestEnvironment(tt.spdk) defer testEnv.Close() - testEnv.opiSpdkServer.Volumes.NvmePaths[testNvmePathName] = utils.ProtoClone(&testNvmePathWithName) + testEnv.opiSpdkServer.store.Set(testNvmePathName, &testNvmePathWithName) request := &pb.GetNvmePathRequest{Name: tt.in} response, err := testEnv.client.GetNvmePath(testEnv.ctx, request) @@ -1053,7 +1053,7 @@ func TestBackEnd_StatsNvmePath(t *testing.T) { testEnv := createTestEnvironment(tt.spdk) defer testEnv.Close() - testEnv.opiSpdkServer.Volumes.NvmePaths[testNvmePathName] = utils.ProtoClone(&testNvmePathWithName) + testEnv.opiSpdkServer.store.Set(testNvmePathName, &testNvmePathWithName) request := &pb.StatsNvmePathRequest{Name: tt.in} response, err := testEnv.client.StatsNvmePath(testEnv.ctx, request)