From caa520cac80dba0c2224f6b8f1f5d66c2692aa5c Mon Sep 17 00:00:00 2001 From: Danil Uzlov Date: Tue, 20 Jun 2023 18:39:20 +0700 Subject: [PATCH] add reselect test, fix old tests for reselect Signed-off-by: Danil Uzlov --- pkg/networkservice/chains/nsmgr/heal_test.go | 4 +- .../chains/nsmgr/reselect_test.go | 305 ++++++++++++++++++ .../chains/nsmgr/select_forwarder_test.go | 2 +- .../common/monitor/passthrough_test.go | 3 +- 4 files changed, 311 insertions(+), 3 deletions(-) create mode 100644 pkg/networkservice/chains/nsmgr/reselect_test.go diff --git a/pkg/networkservice/chains/nsmgr/heal_test.go b/pkg/networkservice/chains/nsmgr/heal_test.go index 89bbdf4b0..5163db074 100644 --- a/pkg/networkservice/chains/nsmgr/heal_test.go +++ b/pkg/networkservice/chains/nsmgr/heal_test.go @@ -1,5 +1,7 @@ // Copyright (c) 2020-2022 Doc.ai and/or its affiliates. // +// Copyright (c) 2023 Cisco Systems, Inc. +// // SPDX-License-Identifier: Apache-2.0 // // Licensed under the Apache License, Version 2.0 (the "License"); @@ -452,7 +454,7 @@ func testNSMGRHealNSMgr(t *testing.T, nodeNum int, restored bool) { if restored { require.Equal(t, 3, counter.Requests()) - require.Equal(t, 1, counter.Closes()) + require.Equal(t, 2, counter.Closes()) } else { require.Equal(t, 2, counter.UniqueRequests()) require.Equal(t, closes+1, counter.UniqueCloses()) diff --git a/pkg/networkservice/chains/nsmgr/reselect_test.go b/pkg/networkservice/chains/nsmgr/reselect_test.go new file mode 100644 index 000000000..c50113e8a --- /dev/null +++ b/pkg/networkservice/chains/nsmgr/reselect_test.go @@ -0,0 +1,305 @@ +// Copyright (c) 2023 Cisco and/or its affiliates. +// +// SPDX-License-Identifier: Apache-2.0 +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at: +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package nsmgr_test + +import ( + "context" + "testing" + + "github.com/stretchr/testify/require" + "go.uber.org/goleak" + + "github.com/networkservicemesh/api/pkg/api/registry" + + "github.com/networkservicemesh/sdk/pkg/networkservice/chains/client" + "github.com/networkservicemesh/sdk/pkg/networkservice/chains/nsmgr" + "github.com/networkservicemesh/sdk/pkg/networkservice/utils/count" + "github.com/networkservicemesh/sdk/pkg/tools/sandbox" +) + +// even if local NSMgr has restarted, +// we expect that all apps after it should get a Close call +// and treat new request as reselect request +func TestReselect_LocalNsmgrRestart(t *testing.T) { + var samples = []struct { + name string + nodeNum int + }{ + { + name: "Local", + nodeNum: 1, + }, + { + name: "Remote", + nodeNum: 2, + }, + } + + for _, sample := range samples { + t.Run(sample.name, func(t *testing.T) { + // nolint:scopelint + testReselectWithLocalNsmgrRestart(t, sample.nodeNum) + }) + } +} + +func testReselectWithLocalNsmgrRestart(t *testing.T, nodeNum int) { + t.Cleanup(func() { goleak.VerifyNone(t) }) + ctx, cancel := context.WithTimeout(context.Background(), timeout) + + // in this test we add counters to apps in chain + // to make sure that in each app Close call goes through the whole chain, + // without stopping on an error mid-chain + var counterFwd []*count.Server + for i := 0; i < nodeNum; i++ { + counterFwd = append(counterFwd, new(count.Server)) + } + + defer cancel() + domain := sandbox.NewBuilder(ctx, t). + SetNodesCount(nodeNum). + SetNSMgrProxySupplier(nil). + SetRegistryProxySupplier(nil). + SetNodeSetup(func(ctx context.Context, node *sandbox.Node, i int) { + node.NewNSMgr(ctx, "nsmgr", nil, sandbox.GenerateTestToken, nsmgr.NewServer) + node.NewForwarder(ctx, ®istry.NetworkServiceEndpoint{ + Name: sandbox.UniqueName("forwarder"), + NetworkServiceNames: []string{"forwarder"}, + }, sandbox.GenerateTestToken, counterFwd[i]) + }). + Build() + + nsRegistryClient := domain.NewNSRegistryClient(ctx, sandbox.GenerateTestToken) + + nsReg, err := nsRegistryClient.Register(ctx, defaultRegistryService(t.Name())) + require.NoError(t, err) + + nseReg := defaultRegistryEndpoint(nsReg.Name) + + counterNse := new(count.Server) + nse := domain.Nodes[nodeNum-1].NewEndpoint(ctx, nseReg, sandbox.GenerateTestToken, counterNse) + + request := defaultRequest(nsReg.Name) + + counterClient := new(count.Client) + nsc := domain.Nodes[0].NewClient(ctx, sandbox.GenerateTestToken, client.WithAdditionalFunctionality(counterClient)) + + conn, err := nsc.Request(ctx, request.Clone()) + require.NoError(t, err) + + domain.Nodes[0].NSMgr.Restart() + + nse.Cancel() + + nseReg2 := defaultRegistryEndpoint(nsReg.Name) + nseReg2.Name += "-2" + domain.Nodes[nodeNum-1].NewEndpoint(ctx, nseReg2, sandbox.GenerateTestToken, counterNse) + + // Wait for heal to finish successfully + require.Eventually(t, checkSecondRequestsReceived(counterNse.UniqueRequests), timeout, tick) + // Client should try to close connection before reselect + require.Equal(t, 1, counterClient.UniqueCloses()) + // Forwarder should get a Close, even though NSMgr restarted and didn't pass the Close + for i := 0; i < nodeNum; i++ { + require.Equal(t, 1, counterFwd[i].Closes()) + } + // Old NSE died, new NSE should not get a Close call + require.Equal(t, 0, counterNse.Closes()) + + // Refresh shouldn't cause Close calls + request.Connection = conn + _, err = nsc.Request(ctx, request.Clone()) + require.NoError(t, err) + require.Equal(t, 0, counterNse.Closes()) + for i := 0; i < nodeNum; i++ { + require.Equal(t, 1, counterFwd[i].Closes()) + } + + clientCloses := counterClient.Closes() + // Close should still be able to pass though the whole connection path + _, err = nsc.Close(ctx, conn) + require.NoError(t, err) + require.Equal(t, clientCloses+1, counterClient.Closes()) + require.Equal(t, 1, counterNse.Closes()) + for i := 0; i < nodeNum; i++ { + require.Equal(t, 1, counterFwd[i].UniqueCloses(), i) + require.Equal(t, 2, counterFwd[i].Closes(), i) + } +} +func TestReselect_LocalForwarderRestart(t *testing.T) { + var samples = []struct { + name string + nodeNum int + }{ + { + name: "Local", + nodeNum: 1, + }, + { + name: "Remote", + nodeNum: 2, + }, + } + + for _, sample := range samples { + t.Run(sample.name, func(t *testing.T) { + // nolint:scopelint + testReselectWithLocalForwarderRestart(t, sample.nodeNum) + }) + } +} +func testReselectWithLocalForwarderRestart(t *testing.T, nodeNum int) { + t.Cleanup(func() { goleak.VerifyNone(t) }) + ctx, cancel := context.WithTimeout(context.Background(), timeout) + + // in this test we add counters to apps in chain + // to make sure that in each app Close call goes through the whole chain, + // without stopping on an error mid-chain + var counterFwd []*count.Server + for i := 0; i < nodeNum; i++ { + counterFwd = append(counterFwd, new(count.Server)) + } + + defer cancel() + domain := sandbox.NewBuilder(ctx, t). + SetNodesCount(nodeNum). + SetNSMgrProxySupplier(nil). + SetRegistryProxySupplier(nil). + SetNodeSetup(func(ctx context.Context, node *sandbox.Node, i int) { + node.NewNSMgr(ctx, "nsmgr", nil, sandbox.GenerateTestToken, nsmgr.NewServer) + node.NewForwarder(ctx, ®istry.NetworkServiceEndpoint{ + Name: sandbox.UniqueName("forwarder"), + NetworkServiceNames: []string{"forwarder"}, + }, sandbox.GenerateTestToken, counterFwd[i]) + }). + Build() + + nsRegistryClient := domain.NewNSRegistryClient(ctx, sandbox.GenerateTestToken) + + nsReg, err := nsRegistryClient.Register(ctx, defaultRegistryService(t.Name())) + require.NoError(t, err) + + nseReg := defaultRegistryEndpoint(nsReg.Name) + + counterNse := new(count.Server) + nse := domain.Nodes[nodeNum-1].NewEndpoint(ctx, nseReg, sandbox.GenerateTestToken, counterNse) + + request := defaultRequest(nsReg.Name) + + counterClient := new(count.Client) + nsc := domain.Nodes[0].NewClient(ctx, sandbox.GenerateTestToken, client.WithAdditionalFunctionality(counterClient)) + + conn, err := nsc.Request(ctx, request.Clone()) + require.NoError(t, err) + + for _, fwd := range domain.Nodes[0].Forwarders { + fwd.Restart() + } + + nse.Cancel() + + nseReg2 := defaultRegistryEndpoint(nsReg.Name) + nseReg2.Name += "-2" + domain.Nodes[nodeNum-1].NewEndpoint(ctx, nseReg2, sandbox.GenerateTestToken, counterNse) + + // Wait for heal to finish successfully + require.Eventually(t, checkSecondRequestsReceived(counterNse.UniqueRequests), timeout, tick) + // Client should try to close connection before reselect + require.Equal(t, 1, counterClient.UniqueCloses()) + // local Forwarder has restarted, new forwarder should not get a Close call + require.Equal(t, 0, counterFwd[0].Closes()) + if nodeNum > 1 { + // remote forwarder should get Close + require.Equal(t, 1, counterFwd[1].Closes()) + } + require.Equal(t, 0, counterNse.Closes()) + + // Refresh shouldn't cause any Close calls + request.Connection = conn + _, err = nsc.Request(ctx, request.Clone()) + require.NoError(t, err) + require.Equal(t, 0, counterNse.Closes()) + require.Equal(t, 0, counterFwd[0].Closes()) + if nodeNum > 1 { + require.Equal(t, 1, counterFwd[1].Closes()) + } + + clientCloses := counterClient.Closes() + // Close should still be able to pass though the whole connection path + _, err = nsc.Close(ctx, conn) + require.NoError(t, err) + require.Equal(t, clientCloses+1, counterClient.Closes()) + require.Equal(t, 1, counterNse.Closes()) + require.Equal(t, 1, counterFwd[0].Closes()) + if nodeNum > 1 { + require.Equal(t, 2, counterFwd[1].Closes()) + } +} + +// If registry died, discover and discoverForwarder elements +// will not be able to query it to get URLs to next app +// but we still expect Close call to finish successfully +func TestReselect_Close_RegistryDied(t *testing.T) { + t.Cleanup(func() { goleak.VerifyNone(t) }) + ctx, cancel := context.WithTimeout(context.Background(), timeout) + + // in this test we add counters to apps in chain + // to make sure that in each app Close call goes through the whole chain, + // without stopping on an error mid-chain + counterFwd := new(count.Server) + + defer cancel() + domain := sandbox.NewBuilder(ctx, t). + SetNSMgrProxySupplier(nil). + SetRegistryProxySupplier(nil). + SetNSMgrSupplier(nil). + SetNodeSetup(func(ctx context.Context, node *sandbox.Node, _ int) { + node.NewNSMgr(ctx, "nsmgr", nil, sandbox.GenerateTestToken, nsmgr.NewServer) + node.NewForwarder(ctx, ®istry.NetworkServiceEndpoint{ + Name: sandbox.UniqueName("forwarder"), + NetworkServiceNames: []string{"forwarder"}, + }, sandbox.GenerateTestToken, counterFwd) + }). + Build() + + nsRegistryClient := domain.NewNSRegistryClient(ctx, sandbox.GenerateTestToken) + + nsReg, err := nsRegistryClient.Register(ctx, defaultRegistryService(t.Name())) + require.NoError(t, err) + + nseReg := defaultRegistryEndpoint(nsReg.Name) + + counterNse := new(count.Server) + domain.Nodes[0].NewEndpoint(ctx, nseReg, sandbox.GenerateTestToken, counterNse) + + request := defaultRequest(nsReg.Name) + + counterClient := new(count.Client) + nsc := domain.Nodes[0].NewClient(ctx, sandbox.GenerateTestToken, client.WithAdditionalFunctionality(counterClient)) + + conn, err := nsc.Request(ctx, request.Clone()) + require.NoError(t, err) + + domain.Registry.Cancel() + + _, err = nsc.Close(ctx, conn) + require.NoError(t, err) + + require.Equal(t, 1, counterClient.Closes()) + require.Equal(t, 1, counterFwd.Closes()) + require.Equal(t, 1, counterNse.Closes()) +} diff --git a/pkg/networkservice/chains/nsmgr/select_forwarder_test.go b/pkg/networkservice/chains/nsmgr/select_forwarder_test.go index c1395095f..b46493d5c 100644 --- a/pkg/networkservice/chains/nsmgr/select_forwarder_test.go +++ b/pkg/networkservice/chains/nsmgr/select_forwarder_test.go @@ -289,6 +289,6 @@ func Test_DiscoverForwarder_ChangeRemoteForwarderOnDeath(t *testing.T) { require.NoError(t, err) require.Equal(t, 1, counter.UniqueRequests()) require.Equal(t, 3, counter.Requests()) - require.Equal(t, 0, counter.Closes()) + require.Equal(t, 1, counter.Closes()) require.NotEqual(t, selectedFwd, conn.GetPath().GetPathSegments()[4].Name) } diff --git a/pkg/networkservice/common/monitor/passthrough_test.go b/pkg/networkservice/common/monitor/passthrough_test.go index a306bcfc0..269993639 100644 --- a/pkg/networkservice/common/monitor/passthrough_test.go +++ b/pkg/networkservice/common/monitor/passthrough_test.go @@ -1,4 +1,4 @@ -// Copyright (c) 2021-2022 Cisco and/or its affiliates. +// Copyright (c) 2021-2023 Cisco and/or its affiliates. // // SPDX-License-Identifier: Apache-2.0 // @@ -249,6 +249,7 @@ func (m *MonitorPassThroughSuite) TestServerUpdate() { expectedConn := endpointConn.Clone() expectedConn.GetPath().Index = m.conn.GetPath().GetIndex() expectedConn.Id = expectedConn.GetCurrentPathSegment().GetId() + expectedConn.State = networkservice.State_DOWN m.ValidateEvent(networkservice.ConnectionEventType_UPDATE, expectedConn) }