From 1356b0a1605d19d95fe1620b2c699a7199f56b38 Mon Sep 17 00:00:00 2001 From: Artsiom Koltun Date: Fri, 19 Jan 2024 12:52:31 +0100 Subject: [PATCH 1/5] feat(storage/nvme): add create backend controller cmd Signed-off-by: Artsiom Koltun --- cmd/storage/backend/backend.go | 42 ++++++++++++++++++ cmd/storage/backend/nvme_controller.go | 61 ++++++++++++++++++++++++++ cmd/storage/storage.go | 2 + storage/backend/client.go | 38 ++++++++++++++++ storage/backend/nvme_controller.go | 37 ++++++++++++++++ 5 files changed, 180 insertions(+) create mode 100644 cmd/storage/backend/backend.go create mode 100644 cmd/storage/backend/nvme_controller.go create mode 100644 storage/backend/client.go create mode 100644 storage/backend/nvme_controller.go diff --git a/cmd/storage/backend/backend.go b/cmd/storage/backend/backend.go new file mode 100644 index 0000000..d7ceb80 --- /dev/null +++ b/cmd/storage/backend/backend.go @@ -0,0 +1,42 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright (C) 2024 Intel Corporation + +// Package backend implements the CLI commands for storage backend +package backend + +import "github.com/spf13/cobra" + +// NewCreateCommand creates a new command to create backend resources +func NewCreateCommand() *cobra.Command { + cmd := &cobra.Command{ + Use: "backend", + Aliases: []string{"b"}, + Short: "Creates backend resource", + Args: cobra.NoArgs, + Run: func(c *cobra.Command, args []string) { + err := c.Help() + cobra.CheckErr(err) + }, + } + + cmd.AddCommand(newCreateNvmeCommand()) + + return cmd +} + +func newCreateNvmeCommand() *cobra.Command { + cmd := &cobra.Command{ + Use: "nvme", + Aliases: []string{"n"}, + Short: "Creates nvme resource", + Args: cobra.NoArgs, + Run: func(c *cobra.Command, args []string) { + err := c.Help() + cobra.CheckErr(err) + }, + } + + cmd.AddCommand(newCreateNvmeControllerCommand()) + + return cmd +} diff --git a/cmd/storage/backend/nvme_controller.go b/cmd/storage/backend/nvme_controller.go new file mode 100644 index 0000000..fdf1daa --- /dev/null +++ b/cmd/storage/backend/nvme_controller.go @@ -0,0 +1,61 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright (C) 2024 Intel Corporation + +// Package backend implements the CLI commands for storage backend +package backend + +import ( + "context" + "fmt" + "strings" + + "github.com/opiproject/godpu/cmd/storage/common" + backendclient "github.com/opiproject/godpu/storage/backend" + pb "github.com/opiproject/opi-api/storage/v1alpha1/gen/go" + "github.com/spf13/cobra" +) + +func newCreateNvmeControllerCommand() *cobra.Command { + id := "" + multipath := "" + cmd := &cobra.Command{ + Use: "controller", + Aliases: []string{"c"}, + Short: "Creates nvme controller representing an external nvme device", + Args: cobra.NoArgs, + Run: func(c *cobra.Command, args []string) { + addr, err := c.Flags().GetString(common.AddrCmdLineArg) + cobra.CheckErr(err) + + timeout, err := c.Flags().GetDuration(common.TimeoutCmdLineArg) + cobra.CheckErr(err) + + client, err := backendclient.New(addr) + cobra.CheckErr(err) + + ctx, cancel := context.WithTimeout(context.Background(), timeout) + defer cancel() + + allowedModes := map[string]pb.NvmeMultipath{ + "disable": pb.NvmeMultipath_NVME_MULTIPATH_DISABLE, + "failover": pb.NvmeMultipath_NVME_MULTIPATH_FAILOVER, + "multipath": pb.NvmeMultipath_NVME_MULTIPATH_MULTIPATH, + } + + mode, ok := allowedModes[strings.ToLower(multipath)] + if !ok { + cobra.CheckErr(fmt.Errorf("not allowed multipath mode: '%s'", multipath)) + } + + response, err := client.CreateNvmeController(ctx, id, mode) + cobra.CheckErr(err) + + common.PrintResponse(response.Name) + }, + } + + cmd.Flags().StringVar(&id, "id", "", "id for created resource. Assigned by server if omitted.") + cmd.Flags().StringVar(&multipath, "multipath", "disable", "multipath mode (disable, failover, enable)") + + return cmd +} diff --git a/cmd/storage/storage.go b/cmd/storage/storage.go index ba79777..def4e7a 100644 --- a/cmd/storage/storage.go +++ b/cmd/storage/storage.go @@ -7,6 +7,7 @@ package storage import ( "time" + "github.com/opiproject/godpu/cmd/storage/backend" "github.com/opiproject/godpu/cmd/storage/common" "github.com/opiproject/godpu/cmd/storage/frontend" "github.com/spf13/cobra" @@ -49,6 +50,7 @@ func newStorageCreateCommand() *cobra.Command { } cmd.AddCommand(frontend.NewCreateCommand()) + cmd.AddCommand(backend.NewCreateCommand()) return cmd } diff --git a/storage/backend/client.go b/storage/backend/client.go new file mode 100644 index 0000000..52107bf --- /dev/null +++ b/storage/backend/client.go @@ -0,0 +1,38 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright (C) 2024 Intel Corporation + +// Package backend implements the go library for OPI backend storage +package backend + +import ( + grpcOpi "github.com/opiproject/godpu/grpc" + pb "github.com/opiproject/opi-api/storage/v1alpha1/gen/go" + "google.golang.org/grpc" +) + +// CreateNvmeClient defines the function type used to retrieve NvmeRemoteControllerServiceClient +type CreateNvmeClient func(cc grpc.ClientConnInterface) pb.NvmeRemoteControllerServiceClient + +// Client is used for managing storage devices on OPI server +type Client struct { + connector grpcOpi.Connector + createNvmeClient CreateNvmeClient +} + +// New creates a new instance of Client +func New(addr string) (*Client, error) { + connector, err := grpcOpi.New(addr) + if err != nil { + return nil, err + } + + return NewWithConnector(connector) +} + +// NewWithConnector creates a new instance of Client with provided connector +func NewWithConnector(connector grpcOpi.Connector) (*Client, error) { + return &Client{ + connector: connector, + createNvmeClient: pb.NewNvmeRemoteControllerServiceClient, + }, nil +} diff --git a/storage/backend/nvme_controller.go b/storage/backend/nvme_controller.go new file mode 100644 index 0000000..65d23df --- /dev/null +++ b/storage/backend/nvme_controller.go @@ -0,0 +1,37 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright (C) 2024 Intel Corporation + +// Package backend implements the go library for OPI backend storage +package backend + +import ( + "context" + + pb "github.com/opiproject/opi-api/storage/v1alpha1/gen/go" +) + +// CreateNvmeController creates an nvme controller representing +// an external nvme device +func (c *Client) CreateNvmeController( + ctx context.Context, + id string, + multipath pb.NvmeMultipath, +) (*pb.NvmeRemoteController, error) { + conn, connClose, err := c.connector.NewConn() + if err != nil { + return nil, err + } + defer connClose() + + client := c.createNvmeClient(conn) + response, err := client.CreateNvmeRemoteController( + ctx, + &pb.CreateNvmeRemoteControllerRequest{ + NvmeRemoteControllerId: id, + NvmeRemoteController: &pb.NvmeRemoteController{ + Multipath: multipath, + }, + }) + + return response, err +} From e1886cb0b57374dfa7d562fd129ac4ea9ea16421 Mon Sep 17 00:00:00 2001 From: Artsiom Koltun Date: Fri, 19 Jan 2024 12:55:36 +0100 Subject: [PATCH 2/5] feat(storage/nvme): add delete backend controller cmd Signed-off-by: Artsiom Koltun --- cmd/storage/backend/backend.go | 35 ++++++++++++++++++++++++++ cmd/storage/backend/nvme_controller.go | 34 +++++++++++++++++++++++++ cmd/storage/storage.go | 1 + storage/backend/nvme_controller.go | 24 ++++++++++++++++++ 4 files changed, 94 insertions(+) diff --git a/cmd/storage/backend/backend.go b/cmd/storage/backend/backend.go index d7ceb80..0b45030 100644 --- a/cmd/storage/backend/backend.go +++ b/cmd/storage/backend/backend.go @@ -40,3 +40,38 @@ func newCreateNvmeCommand() *cobra.Command { return cmd } + +// NewDeleteCommand creates a new command to delete backend resources +func NewDeleteCommand() *cobra.Command { + cmd := &cobra.Command{ + Use: "backend", + Aliases: []string{"b"}, + Short: "Deletes backend resource", + Args: cobra.NoArgs, + Run: func(c *cobra.Command, args []string) { + err := c.Help() + cobra.CheckErr(err) + }, + } + + cmd.AddCommand(newDeleteNvmeCommand()) + + return cmd +} + +func newDeleteNvmeCommand() *cobra.Command { + cmd := &cobra.Command{ + Use: "nvme", + Aliases: []string{"n"}, + Short: "Deletes nvme resource", + Args: cobra.NoArgs, + Run: func(c *cobra.Command, args []string) { + err := c.Help() + cobra.CheckErr(err) + }, + } + + cmd.AddCommand(newDeleteNvmeControllerCommand()) + + return cmd +} diff --git a/cmd/storage/backend/nvme_controller.go b/cmd/storage/backend/nvme_controller.go index fdf1daa..2a71346 100644 --- a/cmd/storage/backend/nvme_controller.go +++ b/cmd/storage/backend/nvme_controller.go @@ -59,3 +59,37 @@ func newCreateNvmeControllerCommand() *cobra.Command { return cmd } + +func newDeleteNvmeControllerCommand() *cobra.Command { + name := "" + allowMissing := false + cmd := &cobra.Command{ + Use: "controller", + Aliases: []string{"c"}, + Short: "Deletes nvme controller representing an external nvme device", + Args: cobra.NoArgs, + Run: func(c *cobra.Command, args []string) { + addr, err := c.Flags().GetString(common.AddrCmdLineArg) + cobra.CheckErr(err) + + timeout, err := c.Flags().GetDuration(common.TimeoutCmdLineArg) + cobra.CheckErr(err) + + client, err := backendclient.New(addr) + cobra.CheckErr(err) + + ctx, cancel := context.WithTimeout(context.Background(), timeout) + defer cancel() + + err = client.DeleteNvmeController(ctx, name, allowMissing) + cobra.CheckErr(err) + }, + } + + cmd.Flags().StringVar(&name, "name", "", "name of deleted remote controller") + cmd.Flags().BoolVar(&allowMissing, "allowMissing", false, "cmd succeeds if attempts to delete a resource that is not present") + + cobra.CheckErr(cmd.MarkFlagRequired("name")) + + return cmd +} diff --git a/cmd/storage/storage.go b/cmd/storage/storage.go index def4e7a..dfc1ed3 100644 --- a/cmd/storage/storage.go +++ b/cmd/storage/storage.go @@ -68,6 +68,7 @@ func newStorageDeleteCommand() *cobra.Command { } cmd.AddCommand(frontend.NewDeleteCommand()) + cmd.AddCommand(backend.NewDeleteCommand()) return cmd } diff --git a/storage/backend/nvme_controller.go b/storage/backend/nvme_controller.go index 65d23df..79d0c2e 100644 --- a/storage/backend/nvme_controller.go +++ b/storage/backend/nvme_controller.go @@ -35,3 +35,27 @@ func (c *Client) CreateNvmeController( return response, err } + +// DeleteNvmeController deletes an nvme controller representing +// an external nvme device +func (c *Client) DeleteNvmeController( + ctx context.Context, + name string, + allowMissing bool, +) error { + conn, connClose, err := c.connector.NewConn() + if err != nil { + return err + } + defer connClose() + + client := c.createNvmeClient(conn) + _, err = client.DeleteNvmeRemoteController( + ctx, + &pb.DeleteNvmeRemoteControllerRequest{ + Name: name, + AllowMissing: allowMissing, + }) + + return err +} From 79adddc915b66699fb660d38d5429645150ee237 Mon Sep 17 00:00:00 2001 From: Artsiom Koltun Date: Fri, 19 Jan 2024 13:05:36 +0100 Subject: [PATCH 3/5] test(storage): add backend client tests Signed-off-by: Artsiom Koltun --- storage/backend/client_test.go | 40 ++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) create mode 100644 storage/backend/client_test.go diff --git a/storage/backend/client_test.go b/storage/backend/client_test.go new file mode 100644 index 0000000..385826a --- /dev/null +++ b/storage/backend/client_test.go @@ -0,0 +1,40 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright (C) 2024 Intel Corporation + +// Package backend implements the go library for OPI backend storage +package backend + +import ( + "testing" +) + +func TestNewClient(t *testing.T) { + tests := map[string]struct { + address string + wantErr bool + wantClient bool + }{ + "empty address": { + address: "", + wantErr: true, + wantClient: false, + }, + "non-empty address": { + address: "localhost:50051", + wantErr: false, + wantClient: true, + }, + } + + for testName, tt := range tests { + t.Run(testName, func(t *testing.T) { + client, err := New(tt.address) + if (err != nil) == !tt.wantErr { + t.Errorf("expected err: %v, received: %v", tt.wantErr, err) + } + if (client != nil) == !tt.wantClient { + t.Errorf("expected client: %v, received: %v", tt.wantClient, client) + } + }) + } +} From e9462555193b41a67d70a8353ff65ceb106542ba Mon Sep 17 00:00:00 2001 From: Artsiom Koltun Date: Fri, 19 Jan 2024 13:21:58 +0100 Subject: [PATCH 4/5] test(storage/nvme): add client create backend controller tests Signed-off-by: Artsiom Koltun --- storage/backend/client.go | 14 +++- storage/backend/nvme_controller_test.go | 104 ++++++++++++++++++++++++ 2 files changed, 114 insertions(+), 4 deletions(-) create mode 100644 storage/backend/nvme_controller_test.go diff --git a/storage/backend/client.go b/storage/backend/client.go index 52107bf..f4e155f 100644 --- a/storage/backend/client.go +++ b/storage/backend/client.go @@ -26,13 +26,19 @@ func New(addr string) (*Client, error) { return nil, err } - return NewWithConnector(connector) + return NewWithArgs( + connector, + pb.NewNvmeRemoteControllerServiceClient, + ) } -// NewWithConnector creates a new instance of Client with provided connector -func NewWithConnector(connector grpcOpi.Connector) (*Client, error) { +// NewWithArgs creates a new instance of Client with non-default members +func NewWithArgs( + connector grpcOpi.Connector, + createNvmeClient CreateNvmeClient, +) (*Client, error) { return &Client{ connector: connector, - createNvmeClient: pb.NewNvmeRemoteControllerServiceClient, + createNvmeClient: createNvmeClient, }, nil } diff --git a/storage/backend/nvme_controller_test.go b/storage/backend/nvme_controller_test.go new file mode 100644 index 0000000..e3d7ee4 --- /dev/null +++ b/storage/backend/nvme_controller_test.go @@ -0,0 +1,104 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright (C) 2024 Intel Corporation + +// Package backend implements the go library for OPI backend storage +package backend + +import ( + "context" + "errors" + "testing" + "time" + + "github.com/opiproject/godpu/mocks" + pb "github.com/opiproject/opi-api/storage/v1alpha1/gen/go" + "github.com/stretchr/testify/require" + "google.golang.org/grpc" + "google.golang.org/protobuf/proto" +) + +func TestCreateNvmeController(t *testing.T) { + testControllerID := "remotenvme0" + testController := &pb.NvmeRemoteController{ + Multipath: pb.NvmeMultipath_NVME_MULTIPATH_FAILOVER, + } + + tests := map[string]struct { + giveClientErr error + giveConnectorErr error + wantErr error + wantRequest *pb.CreateNvmeRemoteControllerRequest + wantResponse *pb.NvmeRemoteController + wantConnClosed bool + }{ + "successful call": { + giveConnectorErr: nil, + giveClientErr: nil, + wantErr: nil, + wantRequest: &pb.CreateNvmeRemoteControllerRequest{ + NvmeRemoteControllerId: testControllerID, + NvmeRemoteController: proto.Clone(testController).(*pb.NvmeRemoteController), + }, + wantResponse: proto.Clone(testController).(*pb.NvmeRemoteController), + wantConnClosed: true, + }, + "client err": { + giveConnectorErr: nil, + giveClientErr: errors.New("Some client error"), + wantErr: errors.New("Some client error"), + wantRequest: &pb.CreateNvmeRemoteControllerRequest{ + NvmeRemoteControllerId: testControllerID, + NvmeRemoteController: proto.Clone(testController).(*pb.NvmeRemoteController), + }, + wantResponse: nil, + wantConnClosed: true, + }, + "connector err": { + giveConnectorErr: errors.New("Some conn error"), + giveClientErr: nil, + wantErr: errors.New("Some conn error"), + wantRequest: nil, + wantResponse: nil, + wantConnClosed: false, + }, + } + + for testName, tt := range tests { + t.Run(testName, func(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), time.Second) + defer cancel() + + mockClient := mocks.NewNvmeRemoteControllerServiceClient(t) + if tt.wantRequest != nil { + toReturn := proto.Clone(tt.wantResponse).(*pb.NvmeRemoteController) + mockClient.EXPECT().CreateNvmeRemoteController(ctx, tt.wantRequest). + Return(toReturn, tt.giveClientErr) + } + + connClosed := false + mockConn := mocks.NewConnector(t) + mockConn.EXPECT().NewConn().Return( + &grpc.ClientConn{}, + func() { connClosed = true }, + tt.giveConnectorErr, + ) + + c, _ := NewWithArgs( + mockConn, + func(grpc.ClientConnInterface) pb.NvmeRemoteControllerServiceClient { + return mockClient + }, + ) + + response, err := c.CreateNvmeController( + ctx, + testControllerID, + pb.NvmeMultipath_NVME_MULTIPATH_FAILOVER, + ) + + require.Equal(t, tt.wantErr, err) + require.True(t, proto.Equal(response, tt.wantResponse)) + require.Equal(t, tt.wantConnClosed, connClosed) + }) + } +} From 3c01831ab0e715e3d8594d80a436325dd8bbf955 Mon Sep 17 00:00:00 2001 From: Artsiom Koltun Date: Fri, 19 Jan 2024 13:22:49 +0100 Subject: [PATCH 5/5] test(storage/nvme): add client delete backend controller tests Signed-off-by: Artsiom Koltun --- storage/backend/nvme_controller_test.go | 71 +++++++++++++++++++++++++ 1 file changed, 71 insertions(+) diff --git a/storage/backend/nvme_controller_test.go b/storage/backend/nvme_controller_test.go index e3d7ee4..56be15a 100644 --- a/storage/backend/nvme_controller_test.go +++ b/storage/backend/nvme_controller_test.go @@ -15,6 +15,7 @@ import ( "github.com/stretchr/testify/require" "google.golang.org/grpc" "google.golang.org/protobuf/proto" + "google.golang.org/protobuf/types/known/emptypb" ) func TestCreateNvmeController(t *testing.T) { @@ -102,3 +103,73 @@ func TestCreateNvmeController(t *testing.T) { }) } } + +func TestDeleteNvmeController(t *testing.T) { + testControllerName := "remotenvme0" + testRequest := &pb.DeleteNvmeRemoteControllerRequest{ + Name: testControllerName, + AllowMissing: true, + } + tests := map[string]struct { + giveClientErr error + giveConnectorErr error + wantErr error + wantRequest *pb.DeleteNvmeRemoteControllerRequest + wantConnClosed bool + }{ + "successful call": { + giveConnectorErr: nil, + giveClientErr: nil, + wantErr: nil, + wantRequest: proto.Clone(testRequest).(*pb.DeleteNvmeRemoteControllerRequest), + wantConnClosed: true, + }, + "client err": { + giveConnectorErr: nil, + giveClientErr: errors.New("Some client error"), + wantErr: errors.New("Some client error"), + wantRequest: proto.Clone(testRequest).(*pb.DeleteNvmeRemoteControllerRequest), + wantConnClosed: true, + }, + "connector err": { + giveConnectorErr: errors.New("Some conn error"), + giveClientErr: nil, + wantErr: errors.New("Some conn error"), + wantRequest: nil, + wantConnClosed: false, + }, + } + + for testName, tt := range tests { + t.Run(testName, func(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), time.Second) + defer cancel() + + mockClient := mocks.NewNvmeRemoteControllerServiceClient(t) + if tt.wantRequest != nil { + mockClient.EXPECT().DeleteNvmeRemoteController(ctx, tt.wantRequest). + Return(&emptypb.Empty{}, tt.giveClientErr) + } + + connClosed := false + mockConn := mocks.NewConnector(t) + mockConn.EXPECT().NewConn().Return( + &grpc.ClientConn{}, + func() { connClosed = true }, + tt.giveConnectorErr, + ) + + c, _ := NewWithArgs( + mockConn, + func(grpc.ClientConnInterface) pb.NvmeRemoteControllerServiceClient { + return mockClient + }, + ) + + err := c.DeleteNvmeController(ctx, testControllerName, true) + + require.Equal(t, tt.wantErr, err) + require.Equal(t, tt.wantConnClosed, connClosed) + }) + } +}