Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add forwarder reuse #1232

Merged
merged 69 commits into from
Mar 14, 2022
Merged
Show file tree
Hide file tree
Changes from 68 commits
Commits
Show all changes
69 commits
Select commit Hold shift + click to select a range
c5e7b2b
add forwarder matching
NikitaSkrynnik Feb 22, 2022
158bf44
use path index instead of magic number
NikitaSkrynnik Feb 22, 2022
23c70b9
minor fixes for tests
NikitaSkrynnik Feb 22, 2022
5054a60
fix linter issue
NikitaSkrynnik Feb 22, 2022
2d84a44
make samples
NikitaSkrynnik Feb 22, 2022
4b4acb2
fix linter issues
NikitaSkrynnik Feb 22, 2022
f6f782d
fix heal tests
NikitaSkrynnik Feb 22, 2022
5f2925c
change iter count
NikitaSkrynnik Feb 22, 2022
fcc66a8
fix timeout value
NikitaSkrynnik Feb 22, 2022
a82b946
change iter count again
NikitaSkrynnik Feb 22, 2022
a0ffe05
change count
NikitaSkrynnik Feb 24, 2022
0fc5ace
increase timeout
NikitaSkrynnik Feb 24, 2022
5565160
apply suggestions
NikitaSkrynnik Feb 24, 2022
8ffcdf8
add debug info
NikitaSkrynnik Feb 24, 2022
208be86
forwarder health check after nsmgr restart
NikitaSkrynnik Feb 24, 2022
55e2195
disable traces
NikitaSkrynnik Feb 24, 2022
195ddfe
cleanup
NikitaSkrynnik Feb 24, 2022
ee38b2b
rework health check
NikitaSkrynnik Feb 24, 2022
47a46f4
enable tracing
NikitaSkrynnik Feb 24, 2022
b60cf58
disable tracing
NikitaSkrynnik Feb 24, 2022
a18802b
increase timeout
NikitaSkrynnik Feb 24, 2022
40d35dc
rework forwarder health check again
NikitaSkrynnik Feb 24, 2022
5d360a2
add simple sleep
NikitaSkrynnik Feb 25, 2022
f14fbee
disable tracing
NikitaSkrynnik Feb 25, 2022
d472110
increase sleep time
NikitaSkrynnik Feb 25, 2022
65667d1
add healing
NikitaSkrynnik Feb 25, 2022
90e510b
tracing
NikitaSkrynnik Feb 25, 2022
9403600
add delete
NikitaSkrynnik Feb 25, 2022
b7500d1
disable race
NikitaSkrynnik Feb 25, 2022
757e328
revert 'add delete' commit
NikitaSkrynnik Feb 25, 2022
1c05aac
enable tracing
NikitaSkrynnik Feb 25, 2022
1c1a40f
add client conn delete
NikitaSkrynnik Feb 25, 2022
fbccb29
delete registry restart
NikitaSkrynnik Feb 25, 2022
4b9cec6
test with forwarder health check
NikitaSkrynnik Feb 25, 2022
8fd4ac4
remove health check + increase sleep
NikitaSkrynnik Feb 25, 2022
5efebe9
change iter count
NikitaSkrynnik Feb 25, 2022
f742e7f
add forwarder registration
NikitaSkrynnik Feb 28, 2022
e5c6519
fix linter issue
NikitaSkrynnik Feb 28, 2022
b6c027f
enable all tests
NikitaSkrynnik Feb 28, 2022
65666e5
test with health check again
NikitaSkrynnik Feb 28, 2022
cf605e2
run only one test
NikitaSkrynnik Feb 28, 2022
f65e776
add forwarder register
NikitaSkrynnik Feb 28, 2022
5ba1a92
test with clientconn delete again
NikitaSkrynnik Feb 28, 2022
69a8dbc
revert last commit
NikitaSkrynnik Feb 28, 2022
a619cdc
increase timeout
NikitaSkrynnik Feb 28, 2022
94aa4cd
add log
NikitaSkrynnik Feb 28, 2022
d5a5bc1
test with forwarder register again
NikitaSkrynnik Feb 28, 2022
8dbd566
increase forwarder count
NikitaSkrynnik Feb 28, 2022
46e5b59
add race flag + disable tracing
NikitaSkrynnik Feb 28, 2022
388295e
move test to heal_test.go file
NikitaSkrynnik Feb 28, 2022
4f70571
add clientconn.Delete(ctx) again
NikitaSkrynnik Feb 28, 2022
d333978
test only local case
NikitaSkrynnik Feb 28, 2022
d52e9a2
test with registry restart
NikitaSkrynnik Feb 28, 2022
415daf3
cleanup
NikitaSkrynnik Feb 28, 2022
fd0fd61
test retry patch
NikitaSkrynnik Feb 28, 2022
6ecf906
add logs
NikitaSkrynnik Feb 28, 2022
42dadd3
wake CI
NikitaSkrynnik Feb 28, 2022
1eecafb
test with 100 ms
NikitaSkrynnik Feb 28, 2022
1040bfd
test with 1s
NikitaSkrynnik Feb 28, 2022
47cff32
test without retry
NikitaSkrynnik Mar 1, 2022
654a609
add manual forwarder heal
NikitaSkrynnik Mar 1, 2022
8353758
add remote test
NikitaSkrynnik Mar 1, 2022
c87bfc0
reduce tryTimeout
NikitaSkrynnik Mar 1, 2022
e1daf89
add nse reregistration
NikitaSkrynnik Mar 1, 2022
8c6fc9d
cleanup
NikitaSkrynnik Mar 1, 2022
6d28137
fix linter issue
NikitaSkrynnik Mar 1, 2022
3d4fc28
enable all tests
NikitaSkrynnik Mar 1, 2022
4c2855b
fix linter issue
NikitaSkrynnik Mar 1, 2022
c83bf8e
apply suggestions
NikitaSkrynnik Mar 2, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
114 changes: 114 additions & 0 deletions pkg/networkservice/chains/nsmgr/heal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package nsmgr_test

import (
"context"
"fmt"
"testing"
"time"

Expand Down Expand Up @@ -445,3 +446,116 @@ func checkSecondRequestsReceived(requestsDone func() int) func() bool {
return requestsDone() >= 2
}
}

func Test_ForwarderShouldBeSelectedCorrectlyOnNSMgrRestart(t *testing.T) {
var samples = []struct {
name string
nodeNum int
pathSegmentCount int
}{
{
name: "Local",
nodeNum: 0,
pathSegmentCount: 4,
},
{
name: "Remote",
nodeNum: 1,
pathSegmentCount: 6,
},
}

for _, sample := range samples {
t.Run(sample.name, func(t *testing.T) {
// nolint:scopelint
testForwarderShouldBeSelectedCorrectlyOnNSMgrRestart(t, sample.nodeNum, sample.pathSegmentCount)
})
}
}

func testForwarderShouldBeSelectedCorrectlyOnNSMgrRestart(t *testing.T, nodeNum, pathSegmentCount int) {
t.Cleanup(func() { goleak.VerifyNone(t) })

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

domain := sandbox.NewBuilder(ctx, t).
SetNodesCount(nodeNum + 1).
SetRegistryProxySupplier(nil).
SetNSMgrProxySupplier(nil).
Build()

var expectedForwarderName string

require.Len(t, domain.Nodes[nodeNum].Forwarders, 1)
for k := range domain.Nodes[nodeNum].Forwarders {
expectedForwarderName = k
}

nsRegistryClient := domain.NewNSRegistryClient(ctx, sandbox.GenerateTestToken)

_, err := nsRegistryClient.Register(ctx, &registry.NetworkService{
Name: "my-ns",
})
require.NoError(t, err)

nseReg := &registry.NetworkServiceEndpoint{
Name: "my-nse-1",
NetworkServiceNames: []string{"my-ns"},
}

nseEntry := domain.Nodes[nodeNum].NewEndpoint(ctx, nseReg, sandbox.GenerateTestToken)

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

request := defaultRequest("my-ns")

conn, err := nsc.Request(ctx, request.Clone())
require.NoError(t, err)
require.NotNil(t, conn)
require.Equal(t, pathSegmentCount, len(conn.Path.PathSegments))
require.Equal(t, expectedForwarderName, conn.GetPath().GetPathSegments()[pathSegmentCount-2].Name)

for i := 0; i < 10; i++ {
request.Connection = conn.Clone()
conn, err = nsc.Request(ctx, request.Clone())

require.NoError(t, err)
require.Equal(t, expectedForwarderName, conn.GetPath().GetPathSegments()[pathSegmentCount-2].Name)

domain.Nodes[nodeNum].NSMgr.Restart()

domain.Nodes[nodeNum].NewForwarder(ctx, &registry.NetworkServiceEndpoint{
Name: sandbox.UniqueName(fmt.Sprintf("%v-forwarder", i)),
NetworkServiceNames: []string{"forwarder"},
NetworkServiceLabels: map[string]*registry.NetworkServiceLabels{
"forwarder": {
Labels: map[string]string{
"p2p": "true",
},
},
},
}, sandbox.GenerateTestToken)

_, err = domain.Nodes[nodeNum].NSMgr.NetworkServiceEndpointRegistryServer().Register(ctx, &registry.NetworkServiceEndpoint{
Name: expectedForwarderName,
Url: domain.Nodes[nodeNum].Forwarders[expectedForwarderName].URL.String(),
NetworkServiceNames: []string{"forwarder"},
NetworkServiceLabels: map[string]*registry.NetworkServiceLabels{
"forwarder": {
Labels: map[string]string{
"p2p": "true",
},
},
},
})
require.NoError(t, err)

_, err = domain.Nodes[nodeNum].NSMgr.NetworkServiceEndpointRegistryServer().Register(ctx, &registry.NetworkServiceEndpoint{
Name: "my-nse-1",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we reuse nseReg fields here to register NSE?

Url: nseEntry.URL.String(),
NetworkServiceNames: []string{"my-ns"},
})
require.NoError(t, err)
}
}
12 changes: 12 additions & 0 deletions pkg/networkservice/common/discoverforwarder/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,18 @@ func (d *discoverForwarderServer) Request(ctx context.Context, request *networks
return nil, errors.New("no candidates found")
}

segments := request.Connection.GetPath().GetPathSegments()
datapathForwarder := ""
if pathIndex := int(request.Connection.GetPath().Index); len(segments) > pathIndex+1 {
datapathForwarder = segments[pathIndex+1].Name
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can remove
datapathForwarder := ""
and use
datapathForwarder := segments[pathIndex+1].Name

for i, candidate := range nses {
if candidate.Name == datapathForwarder {
nses[0], nses[i] = nses[i], nses[0]
break
}
}
}

var candidatesErr = errors.New("all forwarders have failed")

// TODO: Should we consider about load balancing?
Expand Down