Skip to content

Commit

Permalink
fix: connectClient freezes on request to dead server (#681)
Browse files Browse the repository at this point in the history
* fix: connectClient freezes on request to dead server

Signed-off-by: Denis Tingaikin <denis.tingajkin@xored.com>

* make tests more strict

Signed-off-by: Denis Tingaikin <denis.tingajkin@xored.com>
  • Loading branch information
denis-tingaikin authored Jan 28, 2021
1 parent 70aa662 commit 1759d4b
Show file tree
Hide file tree
Showing 2 changed files with 70 additions and 3 deletions.
54 changes: 54 additions & 0 deletions pkg/networkservice/chains/nsmgr/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,8 @@ func TestNSMGR_SelectsRestartingEndpoint(t *testing.T) {
require.NoError(t, err)
require.NotNil(t, conn)
require.Equal(t, 5, len(conn.Path.PathSegments))

require.NoError(t, ctx.Err())
}

func TestNSMGR_RemoteUsecase_BusyEndpoints(t *testing.T) {
Expand Down Expand Up @@ -274,6 +276,58 @@ func TestNSMGR_RemoteUsecase(t *testing.T) {
require.Equal(t, int32(1), atomic.LoadInt32(&counter.Closes))
}

func TestNSMGR_ConnectToDeadNSE(t *testing.T) {
defer goleak.VerifyNone(t, goleak.IgnoreCurrent())
ctx, cancel := context.WithTimeout(context.Background(), time.Second*5)
defer cancel()

domain := sandbox.NewBuilder(t).
SetNodesCount(1).
SetContext(ctx).
SetRegistryProxySupplier(nil).
Build()
defer domain.Cleanup()

nseReg := &registry.NetworkServiceEndpoint{
Name: "final-endpoint",
NetworkServiceNames: []string{"my-service-remote"},
}

counter := &counterServer{}

nseCtx, killNse := context.WithCancel(ctx)
_, err := sandbox.NewEndpoint(nseCtx, nseReg, sandbox.GenerateTestToken, domain.Nodes[0].NSMgr, counter)
require.NoError(t, err)

nsc := sandbox.NewClient(ctx, sandbox.GenerateTestToken, domain.Nodes[0].NSMgr.URL)

request := &networkservice.NetworkServiceRequest{
MechanismPreferences: []*networkservice.Mechanism{
{Cls: cls.LOCAL, Type: kernelmech.MECHANISM},
},
Connection: &networkservice.Connection{
Id: "1",
NetworkService: "my-service-remote",
Context: &networkservice.ConnectionContext{},
},
}

conn, err := nsc.Request(ctx, request.Clone())
require.NoError(t, err)
require.NotNil(t, conn)
require.Equal(t, int32(1), atomic.LoadInt32(&counter.Requests))
require.Equal(t, 5, len(conn.Path.PathSegments))

killNse()
// Simulate refresh from client.
refreshRequest := request.Clone()
refreshRequest.Connection = conn.Clone()

_, err = nsc.Request(ctx, refreshRequest)
require.Error(t, err)
require.NoError(t, ctx.Err())
}

func TestNSMGR_LocalUsecase(t *testing.T) {
defer goleak.VerifyNone(t, goleak.IgnoreCurrent())
ctx, cancel := context.WithTimeout(context.Background(), time.Second*5)
Expand Down
19 changes: 16 additions & 3 deletions pkg/networkservice/common/clienturl/client.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
// Copyright (c) 2020 Cisco and/or its affiliates.
// Copyright (c) 2020-2021 Doc.ai and/or its affiliates.
//
// Copyright (c) 2020-2021 Cisco and/or its affiliates.
//
// SPDX-License-Identifier: Apache-2.0
//
Expand Down Expand Up @@ -26,6 +28,7 @@ import (
"github.com/networkservicemesh/api/pkg/api/networkservice"
"github.com/pkg/errors"
"google.golang.org/grpc"
"google.golang.org/grpc/connectivity"

"github.com/networkservicemesh/sdk/pkg/tools/grpcutils"
)
Expand Down Expand Up @@ -78,9 +81,19 @@ func (u *clientURLClient) init() error {
return
}
u.client = u.clientFactory(u.ctx, cc)

go func() {
<-u.ctx.Done()
_ = cc.Close()
defer func() {
_ = cc.Close()
}()
for cc.WaitForStateChange(u.ctx, cc.GetState()) {
switch cc.GetState() {
case connectivity.Connecting, connectivity.Idle, connectivity.Ready:
continue
default:
return
}
}
}()
})
return u.dialErr
Expand Down

0 comments on commit 1759d4b

Please sign in to comment.