Skip to content

Commit

Permalink
feat(backend): replace Nvme path map with gokv.Store abstraction
Browse files Browse the repository at this point in the history 
Signed-off-by: Boris Glimcher <Boris.Glimcher@emc.com>
  • Loading branch information
@glimchb
glimchb committed Sep 27, 2023
1 parent 414ab88 commit 7c305ee
Show file tree
Hide file tree
Showing 3 changed files with 72 additions and 48 deletions.
16 changes: 2 additions & 14 deletions pkg/backend/backend.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -31,7 +23,6 @@ type Server struct {

rpc spdk.JSONRPC
store gokv.Store
Volumes VolumeParameters
Pagination map[string]int
psk psk
}
Expand All @@ -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,
Expand Down
78 changes: 56 additions & 22 deletions pkg/backend/nvme_path.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,14 +49,19 @@ func (s *Server) CreateNvmePath(_ context.Context, in *pb.CreateNvmePathRequest)
}
in.NvmePath.Name = server.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 {
fmt.Printf("Failed to interact with store: %v", err)
return nil, err
}

Check warning on line 57 in pkg/backend/nvme_path.go

View check run for this annotation

Codecov / codecov/patch

pkg/backend/nvme_path.go#L55-L57 

Added lines #L55 - L57 were not covered by tests
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 {
fmt.Printf("Failed to interact with store: %v", err)
return nil, err
Expand Down Expand Up @@ -115,7 +120,10 @@ func (s *Server) CreateNvmePath(_ context.Context, in *pb.CreateNvmePathRequest)
log.Printf("Received from SPDK: %v", result)

response := server.ProtoClone(in.NvmePath)
s.Volumes.NvmePaths[in.NvmePath.Name] = response
err = s.store.Set(in.NvmePath.Name, response)
if err != nil {
return nil, err
}

Check warning on line 126 in pkg/backend/nvme_path.go

View check run for this annotation

Codecov / codecov/patch

pkg/backend/nvme_path.go#L125-L126 

Added lines #L125 - L126 were not covered by tests
log.Printf("CreateNvmePath: Sending to client: %v", response)
return response, nil
}
Expand All @@ -128,8 +136,13 @@ func (s *Server) DeleteNvmePath(_ context.Context, in *pb.DeleteNvmePathRequest)
log.Printf("error: %v", err)
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 {
fmt.Printf("Failed to interact with store: %v", err)
return nil, err
}

Check warning on line 144 in pkg/backend/nvme_path.go

View check run for this annotation

Codecov / codecov/patch

pkg/backend/nvme_path.go#L142-L144 

Added lines #L142 - L144 were not covered by tests
if !found {
if in.AllowMissing {
return &emptypb.Empty{}, nil
}
Expand All @@ -138,7 +151,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 {
fmt.Printf("Failed to interact with store: %v", err)
return nil, err
Expand Down Expand Up @@ -171,7 +184,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
}

Check warning on line 190 in pkg/backend/nvme_path.go

View check run for this annotation

Codecov / codecov/patch

pkg/backend/nvme_path.go#L189-L190 

Added lines #L189 - L190 were not covered by tests

return &emptypb.Empty{}, nil
}
Expand All @@ -185,8 +201,13 @@ 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 {
fmt.Printf("Failed to interact with store: %v", err)
return nil, err
}

Check warning on line 209 in pkg/backend/nvme_path.go

View check run for this annotation

Codecov / codecov/patch

pkg/backend/nvme_path.go#L207-L209 

Added lines #L207 - L209 were not covered by tests
if !found {
if in.AllowMissing {
log.Printf("TODO: in case of AllowMissing, create a new resource, don;t return error")
}
Expand All @@ -202,7 +223,10 @@ func (s *Server) UpdateNvmePath(_ context.Context, in *pb.UpdateNvmePathRequest)
}
log.Printf("TODO: use resourceID=%v", resourceID)
response := server.ProtoClone(in.NvmePath)
// s.Volumes.NvmePaths[in.NvmePath.Name] = response
// err = s.store.Set(in.NvmePath.Name, response)
// if err != nil {
// return nil, err
// }

Check warning on line 229 in pkg/backend/nvme_path.go

View check run for this annotation

Codecov / codecov/patch

pkg/backend/nvme_path.go#L226-L229 

Added lines #L226 - L229 were not covered by tests
return response, nil
}

Expand Down Expand Up @@ -252,15 +276,20 @@ 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 {
fmt.Printf("Failed to interact with store: %v", err)
return nil, err
}

Check warning on line 284 in pkg/backend/nvme_path.go

View check run for this annotation

Codecov / codecov/patch

pkg/backend/nvme_path.go#L282-L284 

Added lines #L282 - L284 were not covered by tests
if !found {
err := status.Errorf(codes.NotFound, "unable to find key %s", in.Name)
log.Printf("error: %v", err)
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 {
log.Printf("error: %v", err)
return nil, err
Expand All @@ -287,16 +316,21 @@ 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 {
fmt.Printf("Failed to interact with store: %v", err)
return nil, err
}

Check warning on line 324 in pkg/backend/nvme_path.go

View check run for this annotation

Codecov / codecov/patch

pkg/backend/nvme_path.go#L322-L324 

Added lines #L322 - L324 were not covered by tests
if !found {
err := status.Errorf(codes.NotFound, "unable to find key %s", in.Name)
log.Printf("error: %v", err)
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)

Check warning on line 333 in pkg/backend/nvme_path.go

View check run for this annotation

Codecov / codecov/patch

pkg/backend/nvme_path.go#L333 

Added line #L333 was not covered by tests
if err != nil {
log.Printf("error: %v", err)
return nil, err
Expand Down Expand Up @@ -325,11 +359,11 @@ func (s *Server) opiMultipathToSpdk(multipath pb.NvmeMultipath) string {

func (s *Server) numberOfPathsForController(controllerName string) int {

Check warning on line 360 in pkg/backend/nvme_path.go

View workflow job for this annotation

GitHub Actions / call / golangci 

unused-parameter: parameter 'controllerName' seems to be unused, consider removing or renaming it as _ (revive)
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
}

Expand Down
26 changes: 14 additions & 12 deletions pkg/backend/nvme_path_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,13 @@ var (
Trsvcid: 4444,
},
}
testNvmePathWithName = pb.NvmePath{
Name: testNvmePathName,
Trtype: testNvmePath.Trtype,
Traddr: testNvmePath.Traddr,
ControllerNameRef: testNvmePath.ControllerNameRef,
Fabrics: testNvmePath.Fabrics,
}
)

func TestBackEnd_CreateNvmePath(t *testing.T) {
Expand Down Expand Up @@ -225,8 +232,7 @@ func TestBackEnd_CreateNvmePath(t *testing.T) {
testEnv.opiSpdkServer.store.Set(testNvmeCtrlName, server.ProtoClone(tt.controller))

if tt.exist {
testEnv.opiSpdkServer.Volumes.NvmePaths[testNvmePathName] = server.ProtoClone(&testNvmePath)
testEnv.opiSpdkServer.Volumes.NvmePaths[testNvmePathName].Name = testNvmePathName
testEnv.opiSpdkServer.store.Set(testNvmePathName, &testNvmePathWithName)
}
if tt.out != nil {
tt.out = server.ProtoClone(tt.out)
Expand Down Expand Up @@ -445,7 +451,7 @@ func TestBackEnd_DeleteNvmePath(t *testing.T) {
testEnv := createTestEnvironment(tt.spdk)
defer testEnv.Close()

testEnv.opiSpdkServer.Volumes.NvmePaths[testNvmePathName] = server.ProtoClone(&testNvmePath)
testEnv.opiSpdkServer.store.Set(testNvmePathName, &testNvmePathWithName)
testEnv.opiSpdkServer.store.Set(testNvmeCtrlName, &testNvmeCtrlWithName)

request := &pb.DeleteNvmePathRequest{Name: tt.in, AllowMissing: tt.missing}
Expand All @@ -470,9 +476,7 @@ func TestBackEnd_DeleteNvmePath(t *testing.T) {
}

func TestBackEnd_UpdateNvmePath(t *testing.T) {
testNvmePathWithName := server.ProtoClone(&testNvmePath)
testNvmePathWithName.Name = testNvmePathName
t.Cleanup(server.CheckTestProtoObjectsNotChanged(testNvmePathWithName)(t, t.Name()))
t.Cleanup(server.CheckTestProtoObjectsNotChanged(&testNvmePathWithName)(t, t.Name()))
t.Cleanup(checkGlobalTestProtoObjectsNotChanged(t, t.Name()))

tests := map[string]struct {
Expand All @@ -486,7 +490,7 @@ func TestBackEnd_UpdateNvmePath(t *testing.T) {
}{
"invalid fieldmask": {
mask: &fieldmaskpb.FieldMask{Paths: []string{"*", "author"}},
in: testNvmePathWithName,
in: &testNvmePathWithName,
out: nil,
spdk: []string{},
errCode: codes.Unknown,
Expand Down Expand Up @@ -634,8 +638,7 @@ func TestBackEnd_UpdateNvmePath(t *testing.T) {
testEnv := createTestEnvironment(tt.spdk)
defer testEnv.Close()

testEnv.opiSpdkServer.Volumes.NvmePaths[testNvmePathName] = server.ProtoClone(&testNvmePath)
testEnv.opiSpdkServer.Volumes.NvmePaths[testNvmePathName].Name = testNvmePathName
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)
Expand Down Expand Up @@ -948,7 +951,7 @@ func TestBackEnd_GetNvmePath(t *testing.T) {
testEnv := createTestEnvironment(tt.spdk)
defer testEnv.Close()

testEnv.opiSpdkServer.Volumes.NvmePaths[testNvmePathID] = server.ProtoClone(&testNvmePath)
testEnv.opiSpdkServer.store.Set(testNvmePathID, &testNvmePathWithName)

request := &pb.GetNvmePathRequest{Name: tt.in}
response, err := testEnv.client.GetNvmePath(testEnv.ctx, request)
Expand Down Expand Up @@ -1051,8 +1054,7 @@ func TestBackEnd_StatsNvmePath(t *testing.T) {
testEnv := createTestEnvironment(tt.spdk)
defer testEnv.Close()

testEnv.opiSpdkServer.Volumes.NvmePaths[testNvmePathID] = server.ProtoClone(&testNvmePath)
testEnv.opiSpdkServer.Volumes.NvmePaths[testNvmePathID].Name = testNvmePathName
testEnv.opiSpdkServer.store.Set(testNvmePathID, &testNvmePathWithName)

request := &pb.StatsNvmePathRequest{Name: tt.in}
response, err := testEnv.client.StatsNvmePath(testEnv.ctx, request)
Expand Down

0 comments on commit 7c305ee

Please sign in to comment.