Skip to content

Commit

Permalink
Make updatepath restore path index on return (networkservicemesh#602)
Browse files Browse the repository at this point in the history 
* Make updatepath restore path index on return

Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>

* Remove update path segment from updatetoken

Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>
Signed-off-by: Sergey Ershov <sergey.ershov@xored.com>
  • Loading branch information
Vladimir Popov authored and Sergey Ershov committed Dec 20, 2020
1 parent 42dc9b3 commit de6c66c
Show file tree
Hide file tree
Showing 10 changed files with 218 additions and 128 deletions.
21 changes: 0 additions & 21 deletions pkg/networkservice/common/interpose/server_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,6 @@ func TestInterposeServer(t *testing.T) {
client := next.NewNetworkServiceClient(
updatepath.NewClient("client"),
adapters.NewServerToClient(next.NewNetworkServiceServer(
// actual `connect.NewServer()` should not update `request.Connection.Path.PathIndex`
new(restorePathServer),
updatepath.NewServer("nsmgr"),
clienturl.NewServer(&nseURL),
interposeServer,
Expand All @@ -66,11 +64,9 @@ func TestInterposeServer(t *testing.T) {
}),
)),
adapters.NewServerToClient(next.NewNetworkServiceServer(
new(restorePathServer),
updatepath.NewServer("interpose-nse"),
)),
adapters.NewServerToClient(next.NewNetworkServiceServer(
new(restorePathServer),
updatepath.NewServer("nsmgr"),
clienturl.NewServer(&nseURL),
interposeServer,
Expand All @@ -81,7 +77,6 @@ func TestInterposeServer(t *testing.T) {
}),
)),
adapters.NewServerToClient(next.NewNetworkServiceServer(
new(restorePathServer),
updatepath.NewServer("endpoint"),
touchServer,
)),
Expand Down Expand Up @@ -119,22 +114,6 @@ func TestInterposeServer(t *testing.T) {
require.True(t, touchServer.touched)
}

type restorePathServer struct{}

func (s *restorePathServer) Request(ctx context.Context, request *networkservice.NetworkServiceRequest) (*networkservice.Connection, error) {
pathIndex := request.Connection.Path.Index
conn, err := next.Server(ctx).Request(ctx, request)
request.Connection.Path.Index = pathIndex
return conn, err
}

func (s *restorePathServer) Close(ctx context.Context, conn *networkservice.Connection) (*empty.Empty, error) {
pathIndex := conn.Path.Index
_, err := next.Server(ctx).Close(ctx, conn)
conn.Path.Index = pathIndex
return &empty.Empty{}, err
}

type touchServer struct {
touched bool
}
Expand Down
15 changes: 12 additions & 3 deletions pkg/networkservice/common/updatepath/client.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,15 +44,24 @@ func (i *updatePathClient) Request(ctx context.Context, request *networkservice.
request.Connection = &networkservice.Connection{}
}

request.Connection, err = updatePath(request.Connection, i.name)
var index uint32
request.Connection, index, err = updatePath(request.Connection, i.name)
if err != nil {
return nil, err
}
return next.Client(ctx).Request(ctx, request, opts...)

conn, err = next.Client(ctx).Request(ctx, request, opts...)
if err != nil {
return nil, err
}

conn.Path.Index = index

return conn, err
}

func (i *updatePathClient) Close(ctx context.Context, conn *networkservice.Connection, opts ...grpc.CallOption) (_ *empty.Empty, err error) {
conn, err = updatePath(conn, i.name)
conn, _, err = updatePath(conn, i.name)
if err != nil {
return nil, err
}
Expand Down
120 changes: 93 additions & 27 deletions pkg/networkservice/common/updatepath/client_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,16 @@ import (
"context"
"testing"

"github.com/golang/protobuf/ptypes/empty"
"github.com/stretchr/testify/require"

"github.com/networkservicemesh/sdk/pkg/networkservice/common/updatepath"
"google.golang.org/grpc"

"github.com/networkservicemesh/api/pkg/api/networkservice"
"github.com/stretchr/testify/assert"
"go.uber.org/goleak"

"github.com/networkservicemesh/sdk/pkg/networkservice/common/updatepath"
"github.com/networkservicemesh/sdk/pkg/networkservice/core/next"
"github.com/networkservicemesh/sdk/pkg/networkservice/utils/checks/checkrequest"
)

const (
Expand Down Expand Up @@ -59,84 +62,147 @@ func request(connectionID string, pathIndex uint32) *networkservice.NetworkServi

func TestNewClient_SetNewConnectionId(t *testing.T) {
defer goleak.VerifyNone(t, goleak.IgnoreCurrent())

client := updatepath.NewClient("nse-3")

conn, err := client.Request(context.Background(), request(connectionID, 1))
require.NotNil(t, conn)
require.NoError(t, err)
require.NotNil(t, conn)
require.Equal(t, 3, len(conn.Path.PathSegments))
require.Equal(t, "nse-3", conn.Path.PathSegments[2].Name)
require.NotEqual(t, conn.Id, connectionID)
}

func TestNewClient_SetNewConnectionId2(t *testing.T) {
defer goleak.VerifyNone(t)
client := updatepath.NewClient("nse-2")

client := next.NewNetworkServiceClient(
updatepath.NewClient("nse-2"),
checkrequest.NewClient(t, func(t *testing.T, request *networkservice.NetworkServiceRequest) {
require.Equal(t, 1, int(request.Connection.Path.Index))
}),
)

conn, err := client.Request(context.Background(), request(connectionID, 0))
require.NotNil(t, conn)
require.NoError(t, err)
require.Equal(t, 1, int(conn.Path.Index))
require.NotNil(t, conn)
require.Equal(t, conn.Id, pathSegmentID2)
}

func TestNewClient_SetNewConnectionIdSecondReq(t *testing.T) {
defer goleak.VerifyNone(t)

client := updatepath.NewClient("nse-1")

conn, err := client.Request(context.Background(), request(connectionID, 0))
assert.NotNil(t, conn)
assert.NoError(t, err)
assert.Equal(t, conn.Id, pathSegmentID1)
require.NoError(t, err)
require.NotNil(t, conn)
require.Equal(t, conn.Id, pathSegmentID1)

firstConnID := conn.Id

conn.GetPath().Index = 0
conn, err = client.Request(context.Background(), &networkservice.NetworkServiceRequest{
Connection: conn,
})
assert.NotNil(t, conn)
assert.NoError(t, err)
assert.Equal(t, firstConnID, conn.Id)
require.NoError(t, err)
require.NotNil(t, conn)
require.Equal(t, firstConnID, conn.Id)
}

func TestNewClient_PathSegmentNameEqualClientName(t *testing.T) {
defer goleak.VerifyNone(t, goleak.IgnoreCurrent())

client := updatepath.NewClient("nse-2")

conn, err := client.Request(context.Background(), request(connectionID, 1))
assert.NotNil(t, conn)
assert.NoError(t, err)
assert.NotEqual(t, conn.Id, connectionID)
require.NoError(t, err)
require.NotNil(t, conn)
require.NotEqual(t, conn.Id, connectionID)
}

func TestNewClient_PathSegmentIdEqualConnectionId(t *testing.T) {
defer goleak.VerifyNone(t, goleak.IgnoreCurrent())

client := updatepath.NewClient("nse-2")

conn, err := client.Request(context.Background(), request(pathSegmentID2, 1))
assert.NotNil(t, conn)
assert.NoError(t, err)
assert.Equal(t, conn.Id, pathSegmentID2)
require.NoError(t, err)
require.NotNil(t, conn)
require.Equal(t, conn.Id, pathSegmentID2)
}

func TestNewClient_PathSegmentNameAndIDEqualClientNameAndID(t *testing.T) {
defer goleak.VerifyNone(t)

client := updatepath.NewClient("nse-2")

conn, err := client.Request(context.Background(), request(pathSegmentID2, 1))
assert.NotNil(t, conn)
assert.NoError(t, err)
assert.Equal(t, conn.Id, pathSegmentID2)
require.NoError(t, err)
require.NotNil(t, conn)
require.Equal(t, conn.Id, pathSegmentID2)
}

func TestNewClient_InvalidIndex(t *testing.T) {
defer goleak.VerifyNone(t)

client := updatepath.NewClient("nse-3")

conn, err := client.Request(context.Background(), request(connectionID, 2))
require.NotNil(t, err)
require.Error(t, err)
require.Nil(t, conn)
}

func TestNewClient_NoConnection(t *testing.T) {
defer goleak.VerifyNone(t)

client := updatepath.NewClient("nse-3")

conn, err := client.Request(context.Background(), &networkservice.NetworkServiceRequest{})
assert.NotNil(t, conn)
assert.NoError(t, err)
assert.NotEqual(t, conn.Id, connectionID)
require.NoError(t, err)
require.NotNil(t, conn)
require.NotEqual(t, conn.Id, connectionID)
}

func TestNewClient_RestoreIndex(t *testing.T) {
defer goleak.VerifyNone(t, goleak.IgnoreCurrent())

client := next.NewNetworkServiceClient(
updatepath.NewClient("nse-1"),
newPathIndexClient(t, 0),
updatepath.NewClient("nse-2"),
newPathIndexClient(t, 1),
updatepath.NewClient("nse-2"),
newPathIndexClient(t, 1),
updatepath.NewClient("nse-3"),
newPathIndexClient(t, 2),
)

_, err := client.Request(context.Background(), request(connectionID, 0))
require.NoError(t, err)
}

type pathIndexClient struct {
t *testing.T
index uint32
}

func newPathIndexClient(t *testing.T, index uint32) *pathIndexClient {
return &pathIndexClient{
t: t,
index: index,
}
}

func (c *pathIndexClient) Request(ctx context.Context, request *networkservice.NetworkServiceRequest, opts ...grpc.CallOption) (*networkservice.Connection, error) {
require.Equal(c.t, c.index, request.Connection.Path.Index)
conn, err := next.Client(ctx).Request(ctx, request, opts...)
if err != nil {
return nil, err
}
require.Equal(c.t, c.index, conn.Path.Index)
return conn, nil
}

func (c *pathIndexClient) Close(ctx context.Context, conn *networkservice.Connection, opts ...grpc.CallOption) (*empty.Empty, error) {
return next.Client(ctx).Close(ctx, conn, opts...)
}
13 changes: 7 additions & 6 deletions pkg/networkservice/common/updatepath/common.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,9 @@ package updatepath

import (
"github.com/google/uuid"
"github.com/networkservicemesh/api/pkg/api/networkservice"
"github.com/pkg/errors"

"github.com/networkservicemesh/api/pkg/api/networkservice"
)

/*
Expand All @@ -32,7 +33,7 @@ import (
2.3 if path has next segment available, but next name is not equal to segmentName, will return error.
3. if Index == 0, and there is no current segment will add one.
*/
func updatePath(conn *networkservice.Connection, segmentName string) (*networkservice.Connection, error) {
func updatePath(conn *networkservice.Connection, segmentName string) (*networkservice.Connection, uint32, error) {
if conn.Path == nil {
conn.Path = &networkservice.Path{}
}
Expand All @@ -48,23 +49,23 @@ func updatePath(conn *networkservice.Connection, segmentName string) (*networkse
Name: segmentName,
Id: conn.Id,
})
return conn, nil
return conn, 0, nil
}

path := conn.GetPath()

if int(path.Index) < len(path.PathSegments) && path.PathSegments[path.Index].Name == segmentName {
// We already in current segment, just update connection Id, no need to increment
conn.Id = path.PathSegments[path.Index].Id
return conn, nil
return conn, path.Index, nil
}

// We need to move to next item
nextIndex := int(path.Index) + 1

if nextIndex > len(path.GetPathSegments()) {
// We have index > segments count
return nil, errors.Errorf("NetworkServiceRequest.Connection.Path.Index+1==%d should be less or equal len(NetworkServiceRequest.Connection.Path.PathSegement)==%d",
return nil, 0, errors.Errorf("NetworkServiceRequest.Connection.Path.Index+1==%d should be less or equal len(NetworkServiceRequest.Connection.Path.PathSegement)==%d",
nextIndex, len(path.GetPathSegments()))
}
if nextIndex < len(path.GetPathSegments()) && path.GetPathSegments()[nextIndex].Name != segmentName {
Expand All @@ -88,5 +89,5 @@ func updatePath(conn *networkservice.Connection, segmentName string) (*networkse
conn.Id = path.GetPathSegments()[conn.Path.Index].Id
}

return conn, nil
return conn, path.Index - 1, nil
}
19 changes: 15 additions & 4 deletions pkg/networkservice/common/updatepath/server.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"context"

"github.com/golang/protobuf/ptypes/empty"

"github.com/networkservicemesh/api/pkg/api/networkservice"

"github.com/networkservicemesh/sdk/pkg/networkservice/core/next"
Expand All @@ -38,19 +39,29 @@ func NewServer(name string) networkservice.NetworkServiceServer {
return &updatePathServer{name: name}
}

func (i *updatePathServer) Request(ctx context.Context, request *networkservice.NetworkServiceRequest) (_ *networkservice.Connection, err error) {
func (i *updatePathServer) Request(ctx context.Context, request *networkservice.NetworkServiceRequest) (conn *networkservice.Connection, err error) {
if request.Connection == nil {
request.Connection = &networkservice.Connection{}
}
request.Connection, err = updatePath(request.Connection, i.name)

var index uint32
request.Connection, index, err = updatePath(request.Connection, i.name)
if err != nil {
return nil, err
}

conn, err = next.Server(ctx).Request(ctx, request)
if err != nil {
return nil, err
}
return next.Server(ctx).Request(ctx, request)

conn.Path.Index = index

return conn, err
}

func (i *updatePathServer) Close(ctx context.Context, conn *networkservice.Connection) (_ *empty.Empty, err error) {
conn, err = updatePath(conn, i.name)
conn, _, err = updatePath(conn, i.name)
if err != nil {
return nil, err
}
Expand Down
Loading

0 comments on commit de6c66c

Please sign in to comment.