Skip to content

Commit

Permalink
Addressed review comments
Browse files Browse the repository at this point in the history 
Signed-off-by: Oleg Solodkov <oleg.solodkov@xored.com>
  • Loading branch information
@sol-0
sol-0 committed Dec 6, 2021
1 parent a178bfe commit 5f17250
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 95 deletions.
97 changes: 43 additions & 54 deletions pkg/networkservice/common/mechanisms/recvfd/server_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ import (
"time"

"github.com/edwarnicke/grpcfd"
"github.com/golang/protobuf/ptypes/empty"
"github.com/networkservicemesh/api/pkg/api/networkservice"
"github.com/networkservicemesh/api/pkg/api/networkservice/mechanisms/cls"
"github.com/networkservicemesh/api/pkg/api/networkservice/mechanisms/common"
Expand All @@ -43,38 +42,12 @@ import (
"github.com/networkservicemesh/sdk/pkg/networkservice/common/mechanisms/recvfd"
"github.com/networkservicemesh/sdk/pkg/networkservice/common/mechanisms/sendfd"
"github.com/networkservicemesh/sdk/pkg/networkservice/core/chain"
"github.com/networkservicemesh/sdk/pkg/networkservice/core/next"
"github.com/networkservicemesh/sdk/pkg/networkservice/utils/checks/checkcontext"
"github.com/networkservicemesh/sdk/pkg/tools/grpcfdutils"
"github.com/networkservicemesh/sdk/pkg/tools/grpcutils"
"github.com/networkservicemesh/sdk/pkg/tools/sandbox"
)

type checkRecvfdServer struct {
t *testing.T
onFileClosedCallbacks map[string]func()
}

func (n *checkRecvfdServer) Close(ctx context.Context, conn *networkservice.Connection) (*empty.Empty, error) {
return next.Server(ctx).Close(ctx, conn)
}

func (n *checkRecvfdServer) Request(ctx context.Context, request *networkservice.NetworkServiceRequest) (*networkservice.Connection, error) {
err := grpcfdutils.InjectOnFileReceivedCallback(ctx, n.onRecvFile)
require.NoError(n.t, err)

return next.Server(ctx).Request(ctx, request)
}

func (n *checkRecvfdServer) onRecvFile(fileName string, file *os.File) {
// checking a file is closed by recvfd by setting a finalizer
runtime.SetFinalizer(file, func(file *os.File) {
onFileClosedCallback, ok := n.onFileClosedCallbacks[fileName]
if ok {
onFileClosedCallback()
}
})
}

func createFile(fileName string, t *testing.T) (inodeURLStr string, fileClosedContext context.Context, cancelFunc func()) {
f, err := os.Create(fileName)
require.NoError(t, err, "Failed to create and open a file: %v", err)
Expand All @@ -87,16 +60,14 @@ func createFile(fileName string, t *testing.T) (inodeURLStr string, fileClosedCo
inodeURL, err := grpcfd.FilenameToURL(fileName)
require.NoError(t, err)

inodeURLStr = inodeURL.String()

return
return inodeURL.String(), fileClosedContext, cancelFunc
}

func startServer(ctx context.Context, t *testing.T, testServerChain *networkservice.NetworkServiceServer, serveURL *url.URL) {
var grpcServer = grpc.NewServer(grpc.Creds(grpcfd.TransportCredentials(insecure.NewCredentials())))
grpcServer := grpc.NewServer(grpc.Creds(grpcfd.TransportCredentials(insecure.NewCredentials())))
networkservice.RegisterNetworkServiceServer(grpcServer, *testServerChain)

var errCh = grpcutils.ListenAndServe(ctx, serveURL, grpcServer)
errCh := grpcutils.ListenAndServe(ctx, serveURL, grpcServer)

require.Len(t, errCh, 0)
}
Expand All @@ -119,28 +90,38 @@ func TestRecvfdClosesSingleFile(t *testing.T) {
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
defer cancel()

var dir = t.TempDir()
dir := t.TempDir()

s, err := os.Create(path.Join(dir, "test.sock"))
require.NoError(t, err)

var testFileName = path.Join(dir, t.Name()+".test")
testFileName := path.Join(dir, t.Name()+".test")

inodeURLStr, fileClosedContext, cancelFunc := createFile(testFileName, t)

onFileClosedCallbacks := map[string]func(){
inodeURLStr: cancelFunc,
}

serveURL := &url.URL{Scheme: "unix", Path: s.Name()}

var testChain = chain.NewNetworkServiceServer(
&checkRecvfdServer{
t: t,
onFileClosedCallbacks: map[string]func(){
inodeURLStr: cancelFunc,
},
},
testChain := chain.NewNetworkServiceServer(
checkcontext.NewServer(t, func(t *testing.T, c context.Context) {
injectErr := grpcfdutils.InjectOnFileReceivedCallback(c, func(fileName string, file *os.File) {
runtime.SetFinalizer(file, func(file *os.File) {
onFileClosedCallback, ok := onFileClosedCallbacks[fileName]
if ok {
onFileClosedCallback()
}
})
})

require.NoError(t, injectErr)
}),
recvfd.NewServer())

startServer(ctx, t, &testChain, serveURL)
var testClient = createClient(ctx, serveURL)
testClient := createClient(ctx, serveURL)

request := &networkservice.NetworkServiceRequest{
MechanismPreferences: []*networkservice.Mechanism{
Expand Down Expand Up @@ -169,17 +150,17 @@ func TestRecvfdClosesSingleFile(t *testing.T) {
func TestRecvfdClosesMultipleFiles(t *testing.T) {
t.Cleanup(func() { goleak.VerifyNone(t) })

ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
ctx, cancel := context.WithTimeout(context.Background(), 1*time.Second)
defer cancel()

var dir = t.TempDir()
dir := t.TempDir()

s, err := os.Create(path.Join(dir, "test.sock"))
require.NoError(t, err)

const numFiles = 3
var fileClosedContexts = make([]context.Context, numFiles)
var onFileClosedFuncs = make(map[string]func(), numFiles)
fileClosedContexts := make([]context.Context, numFiles)
onFileClosedCallbacks := make(map[string]func(), numFiles)

request := &networkservice.NetworkServiceRequest{
MechanismPreferences: make([]*networkservice.Mechanism, numFiles),
Expand All @@ -190,7 +171,7 @@ func TestRecvfdClosesMultipleFiles(t *testing.T) {
filePath = path.Join(dir, fmt.Sprintf("%s.test%d", t.Name(), i))

inodeURLStr, fileClosedContext, cancelFunc := createFile(filePath, t)
onFileClosedFuncs[inodeURLStr] = cancelFunc
onFileClosedCallbacks[inodeURLStr] = cancelFunc
fileClosedContexts[i] = fileClosedContext

request.MechanismPreferences = append(request.MechanismPreferences,
Expand All @@ -205,15 +186,23 @@ func TestRecvfdClosesMultipleFiles(t *testing.T) {

serveURL := &url.URL{Scheme: "unix", Path: s.Name(), Host: "0.0.0.0:5000"}

var testChain = chain.NewNetworkServiceServer(
&checkRecvfdServer{
t: t,
onFileClosedCallbacks: onFileClosedFuncs,
},
testChain := chain.NewNetworkServiceServer(
checkcontext.NewServer(t, func(t *testing.T, c context.Context) {
e := grpcfdutils.InjectOnFileReceivedCallback(c, func(fileName string, file *os.File) {
runtime.SetFinalizer(file, func(file *os.File) {
onFileClosedCallback, ok := onFileClosedCallbacks[fileName]
if ok {
onFileClosedCallback()
}
})
})

require.NoError(t, e)
}),
recvfd.NewServer())

startServer(ctx, t, &testChain, serveURL)
var testClient = createClient(ctx, serveURL)
testClient := createClient(ctx, serveURL)

conn, err := testClient.Request(ctx, request)
require.NoError(t, err)
Expand Down
58 changes: 17 additions & 41 deletions pkg/registry/common/recvfd/server_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ import (
"time"

"github.com/edwarnicke/grpcfd"
"github.com/golang/protobuf/ptypes/empty"
"github.com/networkservicemesh/api/pkg/api/registry"
"github.com/stretchr/testify/require"
"google.golang.org/grpc"
Expand All @@ -42,51 +41,20 @@ import (
"github.com/networkservicemesh/sdk/pkg/registry/common/sendfd"
registryserialize "github.com/networkservicemesh/sdk/pkg/registry/common/serialize"
registrychain "github.com/networkservicemesh/sdk/pkg/registry/core/chain"
"github.com/networkservicemesh/sdk/pkg/registry/core/next"
"github.com/networkservicemesh/sdk/pkg/registry/utils/checks/checkcontext"
"github.com/networkservicemesh/sdk/pkg/tools/grpcfdutils"
"github.com/networkservicemesh/sdk/pkg/tools/grpcutils"
"github.com/networkservicemesh/sdk/pkg/tools/sandbox"
"github.com/networkservicemesh/sdk/pkg/tools/token"
)

type checkNseRecvfdServer struct {
t *testing.T
onFileClosedCallbacks map[string]func()
}

func (n *checkNseRecvfdServer) Unregister(ctx context.Context, endpoint *registry.NetworkServiceEndpoint) (*empty.Empty, error) {
return next.NetworkServiceEndpointRegistryServer(ctx).Unregister(ctx, endpoint)
}

func (n *checkNseRecvfdServer) Register(ctx context.Context, endpoint *registry.NetworkServiceEndpoint) (*registry.NetworkServiceEndpoint, error) {
err := grpcfdutils.InjectOnFileReceivedCallback(ctx, n.onRecvFile)
require.NoError(n.t, err)

return next.NetworkServiceEndpointRegistryServer(ctx).Register(ctx, endpoint)
}

func (n *checkNseRecvfdServer) Find(query *registry.NetworkServiceEndpointQuery, server registry.NetworkServiceEndpointRegistry_FindServer) error {
return next.NetworkServiceEndpointRegistryServer(server.Context()).Find(query, server)
}

func (n *checkNseRecvfdServer) onRecvFile(fileName string, file *os.File) {
runtime.SetFinalizer(file, func(file *os.File) {
onFileClosedCallback, ok := n.onFileClosedCallbacks[fileName]
if ok {
onFileClosedCallback()
}
})
}

func getFileInfo(fileName string, t *testing.T) (inodeURLStr string, fileClosedContext context.Context, cancelFunc func()) {
fileClosedContext, cancelFunc = context.WithCancel(context.Background())

inodeURL, err := grpcfd.FilenameToURL(fileName)
require.NoError(t, err)

inodeURLStr = inodeURL.String()

return
return inodeURL.String(), fileClosedContext, cancelFunc
}

func startServer(ctx context.Context, t *testing.T, testRegistry registryserver.Registry, serveURL *url.URL) {
Expand All @@ -99,22 +67,30 @@ func startServer(ctx context.Context, t *testing.T, testRegistry registryserver.
}

func TestNseRecvfdServerClosesFile(t *testing.T) {
var ctx, cancel = context.WithTimeout(context.Background(), 5*time.Second)
var ctx, cancel = context.WithTimeout(context.Background(), 1*time.Second)
defer cancel()

var nsRegistry = registrychain.NewNetworkServiceRegistryServer(
registryserialize.NewNetworkServiceRegistryServer(),
memory.NewNetworkServiceRegistryServer(),
)

var checkRecvfdServer = &checkNseRecvfdServer{
t: t,
onFileClosedCallbacks: make(map[string]func()),
}
var onFileClosedCallbacks = make(map[string]func())

var nseRegistry = registrychain.NewNetworkServiceEndpointRegistryServer(
registryserialize.NewNetworkServiceEndpointRegistryServer(),
checkRecvfdServer,
checkcontext.NewNSEServer(t, func(t *testing.T, c context.Context) {
err := grpcfdutils.InjectOnFileReceivedCallback(c, func(fileName string, file *os.File) {
runtime.SetFinalizer(file, func(file *os.File) {
onFileClosedCallback, ok := onFileClosedCallbacks[fileName]
if ok {
onFileClosedCallback()
}
})
})

require.NoError(t, err)
}),
registryrecvfd.NewNetworkServiceEndpointRegistryServer(),
memory.NewNetworkServiceEndpointRegistryServer(),
)
Expand Down Expand Up @@ -150,7 +126,7 @@ func TestNseRecvfdServerClosesFile(t *testing.T) {
// setting onRecvFile after starting the server as we're re-creating a socket in grpcutils.ListenAndServe
// in registry case we're passing a socket
inodeURLStr, fileClosedContext, cancelFunc := getFileInfo(testFileName, t)
checkRecvfdServer.onFileClosedCallbacks[inodeURLStr] = cancelFunc
onFileClosedCallbacks[inodeURLStr] = cancelFunc

var testEndpoint = &registry.NetworkServiceEndpoint{
Name: "test-endpoint",
Expand Down

0 comments on commit 5f17250

Please sign in to comment.