From 4723dcb5cddb544b7d6d0740495ee6696a6a5cd9 Mon Sep 17 00:00:00 2001 From: Artem Belov Date: Thu, 25 Feb 2021 14:23:42 +0700 Subject: [PATCH 01/21] Heal Signed-off-by: Artem Belov --- pkg/networkservice/chains/client/client.go | 12 +- pkg/networkservice/chains/nsmgr/server.go | 6 +- .../chains/nsmgr/server_heal_test.go | 364 +++++++++++++++ .../chains/nsmgr/server_test.go | 39 ++ .../chains/nsmgrproxy/server.go | 15 +- pkg/networkservice/common/clienturl/client.go | 14 +- pkg/networkservice/common/clienturl/const.go | 21 + .../common/discover/match_selector.go | 15 +- pkg/networkservice/common/heal/client.go | 247 +---------- .../common/heal/client_connection_map.gen.go | 73 +++ .../common/heal/connection_map.gen.go | 73 +++ pkg/networkservice/common/heal/gen.go | 29 ++ pkg/networkservice/common/heal/server.go | 419 ++++++++++++++++++ .../heal/{client_test.go => server_test.go} | 9 +- pkg/networkservice/common/interpose/server.go | 14 +- .../common/monitor/connection_map.gen.go | 73 +++ pkg/networkservice/common/monitor/gen.go | 25 ++ pkg/networkservice/common/monitor/server.go | 39 +- pkg/networkservice/common/timeout/server.go | 12 +- pkg/registry/common/clienturl/nse_client.go | 25 +- pkg/tools/grpcutils/errors.go | 38 ++ pkg/tools/sandbox/builder.go | 82 +++- pkg/tools/sandbox/node.go | 25 +- pkg/tools/sandbox/types.go | 15 +- pkg/tools/sandbox/utils.go | 8 + 25 files changed, 1394 insertions(+), 298 deletions(-) create mode 100644 pkg/networkservice/chains/nsmgr/server_heal_test.go create mode 100644 pkg/networkservice/common/clienturl/const.go create mode 100644 pkg/networkservice/common/heal/client_connection_map.gen.go create mode 100644 pkg/networkservice/common/heal/connection_map.gen.go create mode 100644 pkg/networkservice/common/heal/gen.go create mode 100644 pkg/networkservice/common/heal/server.go rename pkg/networkservice/common/heal/{client_test.go => server_test.go} (94%) create mode 100644 pkg/networkservice/common/monitor/connection_map.gen.go create mode 100644 pkg/networkservice/common/monitor/gen.go create mode 100644 pkg/tools/grpcutils/errors.go diff --git a/pkg/networkservice/chains/client/client.go b/pkg/networkservice/chains/client/client.go index c9f344b70..be2b9fcb0 100644 --- a/pkg/networkservice/chains/client/client.go +++ b/pkg/networkservice/chains/client/client.go @@ -1,5 +1,7 @@ // Copyright (c) 2020-2021 Cisco Systems, Inc. // +// Copyright (c) 2021 Doc.ai and/or its affiliates. +// // SPDX-License-Identifier: Apache-2.0 // // Licensed under the Apache License, Version 2.0 (the "License"); @@ -19,7 +21,6 @@ package client import ( "context" - "google.golang.org/grpc" "github.com/google/uuid" @@ -37,18 +38,18 @@ import ( type clientOptions struct { name string - onHeal *networkservice.NetworkServiceClient additionalFunctionality []networkservice.NetworkServiceClient authorizeClient networkservice.NetworkServiceClient + registerClientFunc heal.RegisterClientFunc } // Option modifies default client chain values. type Option func(c *clientOptions) // WithHeal sets heal for the client. -func WithHeal(onHeal *networkservice.NetworkServiceClient) Option { +func WithRegisterHealClientFunc(registerClientFunc heal.RegisterClientFunc) Option { return Option(func(c *clientOptions) { - c.onHeal = onHeal + c.registerClientFunc = registerClientFunc }) } @@ -84,7 +85,6 @@ func NewClient(ctx context.Context, cc grpc.ClientConnInterface, clientOpts ...O var opts = &clientOptions{ name: "client-" + uuid.New().String(), authorizeClient: null.NewClient(), - onHeal: &rv, } for _, opt := range clientOpts { opt(opts) @@ -94,7 +94,7 @@ func NewClient(ctx context.Context, cc grpc.ClientConnInterface, clientOpts ...O append([]networkservice.NetworkServiceClient{ updatepath.NewClient(opts.name), serialize.NewClient(), - heal.NewClient(ctx, networkservice.NewMonitorConnectionClient(cc), opts.onHeal), + heal.NewClient(ctx, networkservice.NewMonitorConnectionClient(cc), opts.registerClientFunc), refresh.NewClient(ctx), metadata.NewClient(), }, opts.additionalFunctionality...), diff --git a/pkg/networkservice/chains/nsmgr/server.go b/pkg/networkservice/chains/nsmgr/server.go index 8be2fb775..fd1b3d879 100644 --- a/pkg/networkservice/chains/nsmgr/server.go +++ b/pkg/networkservice/chains/nsmgr/server.go @@ -34,6 +34,7 @@ import ( "github.com/networkservicemesh/sdk/pkg/networkservice/common/discover" "github.com/networkservicemesh/sdk/pkg/networkservice/common/excludedprefixes" "github.com/networkservicemesh/sdk/pkg/networkservice/common/filtermechanisms" + "github.com/networkservicemesh/sdk/pkg/networkservice/common/heal" "github.com/networkservicemesh/sdk/pkg/networkservice/common/interpose" "github.com/networkservicemesh/sdk/pkg/networkservice/common/mechanisms/recvfd" "github.com/networkservicemesh/sdk/pkg/networkservice/common/mechanisms/sendfd" @@ -102,6 +103,8 @@ func NewServer(ctx context.Context, nsmRegistration *registryapi.NetworkServiceE nsClient := registryadapter.NetworkServiceServerToClient(nsRegistry) + healServer, registerHealClient := heal.NewServer(ctx, addressof.NetworkServiceClient(adapters.NewServerToClient(rv))) + // Construct Endpoint rv.Endpoint = endpoint.NewServer(ctx, nsmRegistration.Name, @@ -113,10 +116,11 @@ func NewServer(ctx context.Context, nsmRegistration *registryapi.NetworkServiceE recvfd.NewServer(), // Receive any files passed interpose.NewServer(&interposeRegistryServer), filtermechanisms.NewServer(&urlsRegistryServer), + healServer, connect.NewServer(ctx, client.NewClientFactory( client.WithName(nsmRegistration.Name), - client.WithHeal(addressof.NetworkServiceClient(adapters.NewServerToClient(rv))), + client.WithRegisterHealClientFunc(registerHealClient), client.WithAdditionalFunctionality( recvfd.NewClient(), sendfd.NewClient(), diff --git a/pkg/networkservice/chains/nsmgr/server_heal_test.go b/pkg/networkservice/chains/nsmgr/server_heal_test.go new file mode 100644 index 000000000..90ceaa174 --- /dev/null +++ b/pkg/networkservice/chains/nsmgr/server_heal_test.go @@ -0,0 +1,364 @@ +// Copyright (c) 2020-2021 Doc.ai 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 define a tests for NSMGR chain element. +package nsmgr_test + +import ( + "context" + "sync/atomic" + "testing" + "time" + + "github.com/golang/protobuf/ptypes" + "github.com/networkservicemesh/api/pkg/api/networkservice" + "github.com/networkservicemesh/api/pkg/api/networkservice/mechanisms/cls" + kernelmech "github.com/networkservicemesh/api/pkg/api/networkservice/mechanisms/kernel" + "github.com/networkservicemesh/api/pkg/api/registry" + "github.com/stretchr/testify/require" + "go.uber.org/goleak" + + "github.com/networkservicemesh/sdk/pkg/tools/sandbox" +) + +const ( + tick = 10 * time.Millisecond + timeout = 10 * time.Second +) + +func TestNSMGR_HealEndpoint(t *testing.T) { + defer goleak.VerifyNone(t, goleak.IgnoreCurrent()) + ctx, cancel := context.WithTimeout(context.Background(), timeout) + + defer cancel() + domain := sandbox.NewBuilder(t). + SetNodesCount(2). + SetRegistryProxySupplier(nil). + SetContext(ctx). + Build() + defer domain.Cleanup() + + expireTime, err := ptypes.TimestampProto(time.Now().Add(time.Second)) + require.NoError(t, err) + + nseReg := ®istry.NetworkServiceEndpoint{ + Name: "final-endpoint", + NetworkServiceNames: []string{"my-service-remote"}, + ExpirationTime: expireTime, + } + + counter := &counterServer{} + nseCtx, nseCtxCancel := context.WithTimeout(context.Background(), time.Second) + defer nseCtxCancel() + + _, err = domain.Nodes[0].NewEndpoint(nseCtx, nseReg, sandbox.GenerateExpiringToken(time.Second), counter) + require.NoError(t, err) + + request := &networkservice.NetworkServiceRequest{ + MechanismPreferences: []*networkservice.Mechanism{ + {Cls: cls.LOCAL, Type: kernelmech.MECHANISM}, + }, + Connection: &networkservice.Connection{ + Id: "1", + NetworkService: "my-service-remote", + Context: &networkservice.ConnectionContext{}, + }, + } + + nsc := domain.Nodes[1].NewClient(ctx, sandbox.GenerateTestToken) + + conn, err := nsc.Request(ctx, request.Clone()) + require.NoError(t, err) + require.NotNil(t, conn) + require.Equal(t, 1, counter.UniqueRequests()) + require.Equal(t, 8, len(conn.Path.PathSegments)) + + nseReg2 := ®istry.NetworkServiceEndpoint{ + Name: "final-endpoint2", + NetworkServiceNames: []string{"my-service-remote"}, + } + _, err = domain.Nodes[0].NewEndpoint(ctx, nseReg2, sandbox.GenerateTestToken, counter) + require.NoError(t, err) + + nseCtxCancel() + + // Wait NSE expired and reconnecting to the new NSE + require.Eventually(t, checkRequests(2, counter.UniqueRequests), timeout, tick) + + // Close. + e, err := nsc.Close(ctx, conn) + require.NoError(t, err) + require.NotNil(t, e) + require.Equal(t, 2, counter.UniqueRequests()) + require.Equal(t, 1, counter.UniqueCloses()) +} + +func TestNSMGR_HealLocalForwarder(t *testing.T) { + forwarderCtx, forwarderCtxCancel := context.WithTimeout(context.Background(), time.Second) + defer forwarderCtxCancel() + + customConfig := []*sandbox.NodeConfig{ + nil, + { + ForwarderCtx: forwarderCtx, + ForwarderGenerateTokenFunc: sandbox.GenerateExpiringToken(time.Second), + }, + } + + testNSMGRHealForwarder(t, 1, customConfig, forwarderCtxCancel) +} + +func TestNSMGR_HealRemoteForwarder(t *testing.T) { + forwarderCtx, forwarderCtxCancel := context.WithTimeout(context.Background(), time.Second) + defer forwarderCtxCancel() + + customConfig := []*sandbox.NodeConfig{ + { + ForwarderCtx: forwarderCtx, + ForwarderGenerateTokenFunc: sandbox.GenerateExpiringToken(time.Second), + }, + } + + testNSMGRHealForwarder(t, 0, customConfig, forwarderCtxCancel) +} + +func testNSMGRHealForwarder(t *testing.T, nodeNum int, customConfig []*sandbox.NodeConfig, forwarderCtxCancel context.CancelFunc) { + defer goleak.VerifyNone(t, goleak.IgnoreCurrent()) + ctx, cancel := context.WithTimeout(context.Background(), time.Second*100) + defer cancel() + + builder := sandbox.NewBuilder(t) + domain := builder. + SetNodesCount(2). + SetRegistryProxySupplier(nil). + SetContext(ctx). + SetCustomConfig(customConfig). + Build() + defer domain.Cleanup() + + nseReg := ®istry.NetworkServiceEndpoint{ + Name: "final-endpoint", + NetworkServiceNames: []string{"my-service-remote"}, + } + + counter := &counterServer{} + _, err := domain.Nodes[0].NewEndpoint(ctx, nseReg, sandbox.GenerateTestToken, counter) + require.NoError(t, err) + + request := &networkservice.NetworkServiceRequest{ + MechanismPreferences: []*networkservice.Mechanism{ + {Cls: cls.LOCAL, Type: kernelmech.MECHANISM}, + }, + Connection: &networkservice.Connection{ + Id: "1", + NetworkService: "my-service-remote", + Context: &networkservice.ConnectionContext{}, + }, + } + + nsc := domain.Nodes[1].NewClient(ctx, sandbox.GenerateTestToken) + + conn, err := nsc.Request(ctx, request.Clone()) + require.NoError(t, err) + require.NotNil(t, conn) + require.Equal(t, 1, counter.UniqueRequests()) + require.Equal(t, 8, len(conn.Path.PathSegments)) + + nseReg.Name = "cross-nse-restored" + forwarderReg := ®istry.NetworkServiceEndpoint{ + Name: "forwarder-restored", + } + domain.Nodes[nodeNum].NewForwarder(ctx, forwarderReg, sandbox.GenerateTestToken) + + forwarderCtxCancel() + + // Wait Cross NSE expired and reconnecting through the new Cross NSE + require.Eventually(t, checkRequests(2, counter.UniqueRequests), timeout, tick) + + // Close. + e, err := nsc.Close(ctx, conn) + require.NoError(t, err) + require.NotNil(t, e) + require.Equal(t, 2, counter.UniqueRequests()) + require.Equal(t, 2, counter.UniqueCloses()) +} + +func TestNSMGR_HealRemoteNSMgrRestored(t *testing.T) { + nsmgrCtx, nsmgrCtxCancel := context.WithTimeout(context.Background(), time.Second) + defer nsmgrCtxCancel() + + customConfig := []*sandbox.NodeConfig{ + { + NsmgrCtx: nsmgrCtx, + NsmgrGenerateTokenFunc: sandbox.GenerateExpiringToken(time.Second), + ForwarderCtx: nsmgrCtx, + ForwarderGenerateTokenFunc: sandbox.GenerateExpiringToken(time.Second), + }, + } + + testNSMGRHealNSMgr(t, 0, customConfig, nsmgrCtxCancel) +} + +func testNSMGRHealNSMgr(t *testing.T, nodeNum int, customConfig []*sandbox.NodeConfig, nsmgrCtxCancel context.CancelFunc) { + defer goleak.VerifyNone(t, goleak.IgnoreCurrent()) + ctx, cancel := context.WithTimeout(context.Background(), time.Second*10) + defer cancel() + + builder := sandbox.NewBuilder(t) + domain := builder. + SetNodesCount(2). + SetRegistryProxySupplier(nil). + SetContext(ctx). + SetCustomConfig(customConfig). + Build() + defer domain.Cleanup() + + nseReg := ®istry.NetworkServiceEndpoint{ + Name: "final-endpoint", + NetworkServiceNames: []string{"my-service-remote"}, + } + + counter := &counterServer{} + nse, err := domain.Nodes[0].NewEndpoint(ctx, nseReg, sandbox.GenerateTestToken, counter) + require.NoError(t, err) + + request := &networkservice.NetworkServiceRequest{ + MechanismPreferences: []*networkservice.Mechanism{ + {Cls: cls.LOCAL, Type: kernelmech.MECHANISM}, + }, + Connection: &networkservice.Connection{ + Id: "1", + NetworkService: "my-service-remote", + Context: &networkservice.ConnectionContext{}, + }, + } + + nsc := domain.Nodes[1].NewClient(ctx, sandbox.GenerateTestToken) + + conn, err := nsc.Request(ctx, request.Clone()) + require.NoError(t, err) + require.NotNil(t, conn) + require.Equal(t, 1, counter.UniqueRequests()) + require.Equal(t, 8, len(conn.Path.PathSegments)) + + nsmgrCtxCancel() + require.Equal(t, int32(1), atomic.LoadInt32(&counter.Requests)) + require.Equal(t, int32(0), atomic.LoadInt32(&counter.Closes)) + + restoredNSMgrEntry, restoredNSMgrResources := builder.NewNSMgr(ctx, domain.Nodes[nodeNum].NSMgr.URL.Host, domain.Registry.URL, sandbox.GenerateTestToken) + domain.Nodes[nodeNum].NSMgr = restoredNSMgrEntry + domain.AddResources(restoredNSMgrResources) + + forwarderReg := ®istry.NetworkServiceEndpoint{ + Name: "forwarder-restored", + } + domain.Nodes[nodeNum].NewForwarder(ctx, forwarderReg, sandbox.GenerateTestToken) + + nseReg.Url = nse.URL.String() + err = domain.Nodes[nodeNum].RegisterEndpoint(ctx, nseReg, domain.Nodes[nodeNum].EndpointRegistryClient) + require.NoError(t, err) + + // Wait Cross NSE expired and reconnecting through the new Cross NSE + require.Eventually(t, checkRequests(2, counter.UniqueRequests), timeout, tick) + + // Close. + closes := atomic.LoadInt32(&counter.Closes) + e, err := nsc.Close(ctx, conn) + require.NoError(t, err) + require.NotNil(t, e) + require.Equal(t, int32(2), atomic.LoadInt32(&counter.Requests)) + require.Equal(t, closes+1, atomic.LoadInt32(&counter.Closes)) +} + +func TestNSMGR_HealRemoteNSMgr(t *testing.T) { + defer goleak.VerifyNone(t, goleak.IgnoreCurrent()) + + nsmgrCtx, nsmgrCtxCancel := context.WithTimeout(context.Background(), time.Second) + defer nsmgrCtxCancel() + customConfig := []*sandbox.NodeConfig{ + { + NsmgrCtx: nsmgrCtx, + NsmgrGenerateTokenFunc: sandbox.GenerateExpiringToken(time.Second), + ForwarderCtx: nsmgrCtx, + ForwarderGenerateTokenFunc: sandbox.GenerateExpiringToken(time.Second), + }, + } + + ctx, cancel := context.WithTimeout(context.Background(), time.Second*10) + defer cancel() + + builder := sandbox.NewBuilder(t) + domain := builder. + SetNodesCount(3). + SetRegistryProxySupplier(nil). + SetContext(ctx). + SetCustomConfig(customConfig). + Build() + defer domain.Cleanup() + + nseReg := ®istry.NetworkServiceEndpoint{ + Name: "final-endpoint", + NetworkServiceNames: []string{"my-service-remote"}, + } + + counter := &counterServer{} + _, err := domain.Nodes[0].NewEndpoint(ctx, nseReg, sandbox.GenerateTestToken, counter) + require.NoError(t, err) + + request := &networkservice.NetworkServiceRequest{ + MechanismPreferences: []*networkservice.Mechanism{ + {Cls: cls.LOCAL, Type: kernelmech.MECHANISM}, + }, + Connection: &networkservice.Connection{ + Id: "1", + NetworkService: "my-service-remote", + Context: &networkservice.ConnectionContext{}, + }, + } + + nsc := domain.Nodes[1].NewClient(ctx, sandbox.GenerateTestToken) + + conn, err := nsc.Request(ctx, request.Clone()) + require.NoError(t, err) + require.NotNil(t, conn) + require.Equal(t, 1, counter.UniqueRequests()) + require.Equal(t, 8, len(conn.Path.PathSegments)) + + nsmgrCtxCancel() + + nseReg2 := ®istry.NetworkServiceEndpoint{ + Name: "final-endpoint-2", + NetworkServiceNames: []string{"my-service-remote"}, + } + _, err = domain.Nodes[2].NewEndpoint(ctx, nseReg2, sandbox.GenerateTestToken, counter) + require.NoError(t, err) + + // Wait Cross NSE expired and reconnecting through the new Cross NSE + require.Eventually(t, checkRequests(2, counter.UniqueRequests), timeout, tick) + + // Close. + e, err := nsc.Close(ctx, conn) + require.NoError(t, err) + require.NotNil(t, e) + require.Equal(t, 2, counter.UniqueRequests()) + require.Equal(t, 2, counter.UniqueCloses()) +} + +func checkRequests(requestsRequired int, requestsDone func() int) func() bool { + return func() bool { + return requestsRequired == requestsDone() + } +} diff --git a/pkg/networkservice/chains/nsmgr/server_test.go b/pkg/networkservice/chains/nsmgr/server_test.go index d1e2ebbe7..aef54a7e3 100644 --- a/pkg/networkservice/chains/nsmgr/server_test.go +++ b/pkg/networkservice/chains/nsmgr/server_test.go @@ -722,18 +722,57 @@ func (c *passThroughClient) Close(ctx context.Context, conn *networkservice.Conn type counterServer struct { Requests, Closes int32 + requests map[string]int32 + closes map[string]int32 + mu sync.Mutex } func (c *counterServer) Request(ctx context.Context, request *networkservice.NetworkServiceRequest) (*networkservice.Connection, error) { + c.mu.Lock() + defer c.mu.Unlock() + atomic.AddInt32(&c.Requests, 1) + if c.requests == nil { + c.requests = make(map[string]int32) + } + c.requests[request.GetConnection().GetId()]++ + return next.Server(ctx).Request(ctx, request) } func (c *counterServer) Close(ctx context.Context, connection *networkservice.Connection) (*empty.Empty, error) { + c.mu.Lock() + defer c.mu.Unlock() + atomic.AddInt32(&c.Closes, 1) + if c.closes == nil { + c.closes = make(map[string]int32) + } + c.closes[connection.GetId()]++ + return next.Server(ctx).Close(ctx, connection) } +func (c *counterServer) UniqueRequests() int { + c.mu.Lock() + defer c.mu.Unlock() + + if c.requests == nil { + return 0 + } + return len(c.requests) +} + +func (c *counterServer) UniqueCloses() int { + c.mu.Lock() + defer c.mu.Unlock() + + if c.closes == nil { + return 0 + } + return len(c.closes) +} + type restartingEndpoint struct { startTime time.Time } diff --git a/pkg/networkservice/chains/nsmgrproxy/server.go b/pkg/networkservice/chains/nsmgrproxy/server.go index d2c8a5279..068895579 100644 --- a/pkg/networkservice/chains/nsmgrproxy/server.go +++ b/pkg/networkservice/chains/nsmgrproxy/server.go @@ -28,24 +28,37 @@ import ( "github.com/networkservicemesh/sdk/pkg/networkservice/chains/endpoint" "github.com/networkservicemesh/sdk/pkg/networkservice/common/connect" "github.com/networkservicemesh/sdk/pkg/networkservice/common/externalips" + "github.com/networkservicemesh/sdk/pkg/networkservice/common/heal" "github.com/networkservicemesh/sdk/pkg/networkservice/common/interdomainurl" "github.com/networkservicemesh/sdk/pkg/networkservice/common/swapip" + "github.com/networkservicemesh/sdk/pkg/networkservice/core/adapters" + "github.com/networkservicemesh/sdk/pkg/tools/addressof" "github.com/networkservicemesh/sdk/pkg/tools/token" ) // NewServer creates new proxy NSMgr + func NewServer(ctx context.Context, name string, authorizeServer networkservice.NetworkServiceServer, tokenGenerator token.GeneratorFunc, options ...grpc.DialOption) endpoint.Endpoint { - return endpoint.NewServer(ctx, + type nsmgrProxyServer struct { + endpoint.Endpoint + } + + rv := nsmgrProxyServer{} + + healServer, _ := heal.NewServer(ctx, addressof.NetworkServiceClient(adapters.NewServerToClient(rv))) + rv.Endpoint = endpoint.NewServer(ctx, name, authorizeServer, tokenGenerator, interdomainurl.NewServer(), externalips.NewServer(ctx), swapip.NewServer(), + healServer, connect.NewServer( ctx, client.NewClientFactory(client.WithName(name)), options..., ), ) + return rv } diff --git a/pkg/networkservice/common/clienturl/client.go b/pkg/networkservice/common/clienturl/client.go index 7b1fcbf98..0c012d8a1 100644 --- a/pkg/networkservice/common/clienturl/client.go +++ b/pkg/networkservice/common/clienturl/client.go @@ -22,15 +22,15 @@ import ( "context" "sync" - "github.com/networkservicemesh/sdk/pkg/networkservice/chains/client" - "github.com/networkservicemesh/sdk/pkg/tools/clienturlctx" - "github.com/golang/protobuf/ptypes/empty" - "github.com/networkservicemesh/api/pkg/api/networkservice" "github.com/pkg/errors" "google.golang.org/grpc" "google.golang.org/grpc/connectivity" + "github.com/networkservicemesh/api/pkg/api/networkservice" + + "github.com/networkservicemesh/sdk/pkg/networkservice/chains/client" + "github.com/networkservicemesh/sdk/pkg/tools/clienturlctx" "github.com/networkservicemesh/sdk/pkg/tools/grpcutils" ) @@ -76,8 +76,12 @@ func (u *clientURLClient) init() error { u.dialErr = errors.New("cannot dial nil clienturl.ClientURL(ctx)") return } + + ctx, cancel := context.WithTimeout(u.ctx, dialTimeout) + defer cancel() + var cc *grpc.ClientConn - cc, u.dialErr = grpc.DialContext(u.ctx, grpcutils.URLToTarget(clientURL), u.dialOptions...) + cc, u.dialErr = grpc.DialContext(ctx, grpcutils.URLToTarget(clientURL), u.dialOptions...) if u.dialErr != nil { return } diff --git a/pkg/networkservice/common/clienturl/const.go b/pkg/networkservice/common/clienturl/const.go new file mode 100644 index 000000000..9a831e619 --- /dev/null +++ b/pkg/networkservice/common/clienturl/const.go @@ -0,0 +1,21 @@ +// Copyright (c) 2021 Doc.ai 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 clienturl + +import "time" + +const dialTimeout = 100 * time.Millisecond diff --git a/pkg/networkservice/common/discover/match_selector.go b/pkg/networkservice/common/discover/match_selector.go index df8ac5ed9..69f699617 100644 --- a/pkg/networkservice/common/discover/match_selector.go +++ b/pkg/networkservice/common/discover/match_selector.go @@ -1,5 +1,7 @@ // Copyright (c) 2018-2020 VMware, Inc. // +// Copyright (c) 2021 Doc.ai and/or its affiliates. +// // SPDX-License-Identifier: Apache-2.0 // // Licensed under the Apache License, Version 2.0 (the "License"); @@ -19,6 +21,7 @@ package discover import ( "bytes" "text/template" + "time" "github.com/networkservicemesh/api/pkg/api/registry" ) @@ -40,6 +43,13 @@ func isSubset(a, b, nsLabels map[string]string) bool { } func matchEndpoint(nsLabels map[string]string, ns *registry.NetworkService, networkServiceEndpoints ...*registry.NetworkServiceEndpoint) []*registry.NetworkServiceEndpoint { + validNetworkServiceEndpoints := make([]*registry.NetworkServiceEndpoint, 0) + for _, nse := range networkServiceEndpoints { + if nse.GetExpirationTime() == nil || nse.GetExpirationTime().AsTime().After(time.Now()) { + validNetworkServiceEndpoints = append(validNetworkServiceEndpoints, nse) + } + } + // Iterate through the matches for _, match := range ns.GetMatches() { // All match source selector labels should be present in the requested labels map @@ -50,7 +60,7 @@ func matchEndpoint(nsLabels map[string]string, ns *registry.NetworkService, netw // Check all Destinations in that match for _, destination := range match.GetRoutes() { // Each NSE should be matched against that destination - for _, nse := range networkServiceEndpoints { + for _, nse := range validNetworkServiceEndpoints { if isSubset(nse.GetNetworkServiceLabels()[ns.Name].Labels, destination.GetDestinationSelector(), nsLabels) { nseCandidates = append(nseCandidates, nse) } @@ -58,7 +68,8 @@ func matchEndpoint(nsLabels map[string]string, ns *registry.NetworkService, netw } return nseCandidates } - return networkServiceEndpoints + + return validNetworkServiceEndpoints } // ProcessLabels generates matches based on destination label selectors that specify templating. diff --git a/pkg/networkservice/common/heal/client.go b/pkg/networkservice/common/heal/client.go index 9603f77c6..afebb9565 100644 --- a/pkg/networkservice/common/heal/client.go +++ b/pkg/networkservice/common/heal/client.go @@ -1,5 +1,3 @@ -// Copyright (c) 2020 Cisco Systems, Inc. -// // Copyright (c) 2021 Doc.ai and/or its affiliates. // // SPDX-License-Identifier: Apache-2.0 @@ -21,250 +19,41 @@ package heal import ( "context" - "runtime" - "github.com/golang/protobuf/ptypes" "github.com/golang/protobuf/ptypes/empty" - "github.com/pkg/errors" "google.golang.org/grpc" "github.com/networkservicemesh/api/pkg/api/networkservice" - "github.com/edwarnicke/serialize" - - "github.com/networkservicemesh/sdk/pkg/tools/addressof" - "github.com/networkservicemesh/sdk/pkg/networkservice/core/next" ) +// RegisterClientFunc - required to inform heal server about new client connection and assign it to the connection ID +type RegisterClientFunc func(context.Context, *networkservice.Connection, networkservice.MonitorConnectionClient) + type healClient struct { - ctx context.Context - client networkservice.MonitorConnectionClient - onHeal *networkservice.NetworkServiceClient - cancelHealMap map[string]func() <-chan error - cancelHealMapExecutor serialize.Executor + ctx context.Context + cc networkservice.MonitorConnectionClient + registerClient RegisterClientFunc } -// NewClient - creates a new networkservice.NetworkServiceClient chain element that implements the healing algorithm -// - ctx - context for the lifecycle of the *Client* itself. Cancel when discarding the client. -// - client - networkservice.MonitorConnectionClient that can be used to call MonitorConnection against the endpoint -// - onHeal - *networkservice.NetworkServiceClient. Since networkservice.NetworkServiceClient is an interface -// (and thus a pointer) *networkservice.NetworkServiceClient is a double pointer. Meaning it -// points to a place that points to a place that implements networkservice.NetworkServiceClient -// This is done because when we use heal.NewClient as part of a chain, we may not *have* -// a pointer to this -// client used 'onHeal'. If we detect we need to heal, onHeal.Request is used to heal. -// If onHeal is nil, then we simply set onHeal to this client chain element -// If we are part of a larger chain or a server, we should pass the resulting chain into -// this constructor before we actually have a pointer to it. -// If onHeal nil, onHeal will be pointed to the returned networkservice.NetworkServiceClient -func NewClient(ctx context.Context, client networkservice.MonitorConnectionClient, onHeal *networkservice.NetworkServiceClient) networkservice.NetworkServiceClient { - rv := &healClient{ - ctx: ctx, - client: client, - onHeal: onHeal, - cancelHealMap: make(map[string]func() <-chan error), +// NewClient - creates a new networkservice.NetworkServiceClient chain element that inform healServer about new client connection +func NewClient(ctx context.Context, cc networkservice.MonitorConnectionClient, registerClient RegisterClientFunc) networkservice.NetworkServiceClient { + return &healClient{ + ctx: ctx, + cc: cc, + registerClient: registerClient, } - - if rv.onHeal == nil { - rv.onHeal = addressof.NetworkServiceClient(rv) - } - - return rv } -func (f *healClient) Request(ctx context.Context, request *networkservice.NetworkServiceRequest, opts ...grpc.CallOption) (*networkservice.Connection, error) { +func (u *healClient) Request(ctx context.Context, request *networkservice.NetworkServiceRequest, opts ...grpc.CallOption) (*networkservice.Connection, error) { conn, err := next.Client(ctx).Request(ctx, request, opts...) - if err != nil { - return nil, err - } - err = f.startHeal(request.Clone().SetRequestConnection(conn.Clone()), opts...) - if err != nil { - return nil, err - } - return conn, nil -} - -func (f *healClient) Close(ctx context.Context, conn *networkservice.Connection, opts ...grpc.CallOption) (*empty.Empty, error) { - f.stopHeal(conn) - rv, err := next.Client(ctx).Close(ctx, conn, opts...) - if err != nil { - return nil, err - } - return rv, nil -} - -func (f *healClient) stopHeal(conn *networkservice.Connection) { - var cancelHeal func() <-chan error - <-f.cancelHealMapExecutor.AsyncExec(func() { - cancelHeal = f.cancelHealMap[conn.GetId()] - }) - <-cancelHeal() -} - -// startHeal - start a healAsNeeded using the request as the request for re-request if healing is needed. -func (f *healClient) startHeal(request *networkservice.NetworkServiceRequest, opts ...grpc.CallOption) error { - errCh := make(chan error, 1) - go f.healAsNeeded(request, errCh, opts...) - return <-errCh -} - -// healAsNeeded - heal the connection found in request. Will immediately do a monitor to make sure the server has the -// expected connection and it is sane, returning an error via errCh if there is an issue, and nil via errCh if there is -// not. You will only *ever* receive one real error via the errCh. errCh will be closed when healAsNeeded is finished -// allowing it to double as a 'done' channel we can use when we stopHealing in f.Close. -// healAsNeeded will then continue to monitor the servers opinions about the state of the connection until either -// expireTime has passed or stopHeal is called (as in Close) or a different pathSegment is found via monitoring -// indicating that a later Request has occurred and in doing so created its own healAsNeeded and so we can stop this one -func (f *healClient) healAsNeeded(request *networkservice.NetworkServiceRequest, errCh chan error, opts ...grpc.CallOption) { - // When we are done, close the errCh - defer close(errCh) - - pathSegment := request.GetConnection().GetNextPathSegment() - - // Make sure we have a valid expireTime to work with - expireTime, err := ptypes.Timestamp(pathSegment.GetExpires()) - if err != nil { - errCh <- errors.Wrapf(err, "error converting pathSegment.GetExpires() to time.Time: %+v", pathSegment.GetExpires()) - return - } - - // Set the ctx Deadline to expireTime based on the heal servers context - ctx, cancel := context.WithDeadline(f.ctx, expireTime) - defer cancel() - id := request.GetConnection().GetId() - f.cancelHealMapExecutor.AsyncExec(func() { - if cancel, ok := f.cancelHealMap[id]; ok { - go cancel() // TODO - what to do with the errCh here? - } - f.cancelHealMap[id] = func() <-chan error { - cancel() - return errCh - } - }) - - // Monitor the pathSegment for the first time, so we can pass back an error - // if we can't confirm via monitor the other side has the expected state - recv, err := f.initialMonitorSegment(ctx, pathSegment) - if err != nil { - errCh <- errors.Wrapf(err, "error calling MonitorConnection_MonitorConnectionsClient.Recv to get initial confirmation server has connection: %+v", request.GetConnection()) - return - } - - // Tell the caller all is well by sending them a nil err so the call can continue - errCh <- nil - - // Start looping over events - for { - select { - case <-ctx.Done(): - return - default: - } - event, err := recv.Recv() - if err != nil { - // If we get an error, try to get a new recv ... if that fails, loop around and try again until - // we succeed or the ctx is canceled or expires - newRecv, newRecvErr := f.client.MonitorConnections(ctx, &networkservice.MonitorScopeSelector{ - PathSegments: []*networkservice.PathSegment{ - pathSegment, - }, - }) - if newRecvErr == nil { - recv = newRecv - } - runtime.Gosched() - continue - } - select { - case <-ctx.Done(): - return - default: - } - if err := f.processEvent(ctx, request, event, opts...); err != nil { - if err != nil { - return - } - } - } -} - -// initialMonitorSegment - monitors for pathSegment and returns a recv and an error if the server does not have -// a record for the connection matching our expectations -func (f *healClient) initialMonitorSegment(ctx context.Context, pathSegment *networkservice.PathSegment) (networkservice.MonitorConnection_MonitorConnectionsClient, error) { - // If pathSegment is nil, the server is very very screwed up - if pathSegment == nil { - return nil, errors.New("pathSegment for server connection must not be nil") + if err == nil && u.registerClient != nil { + u.registerClient(u.ctx, conn, u.cc) } - - // Monitor *just* this connection - recv, err := f.client.MonitorConnections(ctx, &networkservice.MonitorScopeSelector{ - PathSegments: []*networkservice.PathSegment{ - pathSegment, - }, - }) - if err != nil { - return nil, errors.Wrap(err, "error when attempting to MonitorConnections") - } - - // Get an initial event to make sure we have the expected connection - event, err := recv.Recv() - if err != nil { - return nil, err - } - // If we didn't get an event something very bad has happened - if event.Connections == nil || event.Connections[pathSegment.GetId()] == nil { - return nil, errors.Errorf("connection with id %s not found in MonitorConnections event as expected: event: %+v", pathSegment.Id, event) - } - // If its not *our* connection something's gone wrong like a later Request succeeding - if !pathSegment.Equal(event.GetConnections()[pathSegment.GetId()].GetCurrentPathSegment()) { - return nil, errors.Errorf("server reports a different connection for this id, pathSegments do not match. Expected: %+v Received %+v", pathSegment, event.GetConnections()[pathSegment.GetId()].GetCurrentPathSegment()) - } - return recv, nil + return conn, err } -// processEvent - process event, calling (*f.OnHeal).Request(ctx,request,opts...) if the server does not have our connection. -// returns a non-nil error if the event is such that we should no longer to continue to attempt to heal. -func (f *healClient) processEvent(ctx context.Context, request *networkservice.NetworkServiceRequest, event *networkservice.ConnectionEvent, opts ...grpc.CallOption) error { - pathSegment := request.GetConnection().GetNextPathSegment() - switch event.GetType() { - case networkservice.ConnectionEventType_UPDATE: - // We should never receive an UPDATE that isn't ours, but in case we do... - if event.Connections == nil || event.Connections[pathSegment.GetId()] == nil { - break - } - fallthrough - case networkservice.ConnectionEventType_INITIAL_STATE_TRANSFER: - if event.Connections != nil && event.Connections[pathSegment.GetId()] != nil { - // If the server has a pathSegment for this Connection.Id, but its not the one we - // got back from it... we should fail, as different Request came after ours successfully - if !pathSegment.Equal(event.GetConnections()[pathSegment.GetId()].GetCurrentPathSegment()) { - return errors.Errorf("server has a different pathSegment than was returned to this call.") - } - break - } - fallthrough - case networkservice.ConnectionEventType_DELETE: - if event.Connections != nil && event.Connections[pathSegment.Id] != nil && pathSegment.Equal(event.GetConnections()[pathSegment.GetId()].GetCurrentPathSegment()) { - _, err := (*f.onHeal).Request(ctx, request, opts...) - for err != nil { - // Note: ctx here has deadline set to the expireTime of the pathSegment... so there is a finite stop point - // to trying to heal. Additionally, a Close on the connection will trigger a cancel on ctx and - // wait for errCh to finish *before* calling Close down the line... so we won't accidentally - // recreate a closed connection. - runtime.Gosched() - select { - case <-ctx.Done(): - return nil - default: - } - _, err := (*f.onHeal).Request(ctx, request, opts...) - if err != nil { - return err - } - } - return nil - } - } - return nil +func (u *healClient) Close(ctx context.Context, conn *networkservice.Connection, opts ...grpc.CallOption) (*empty.Empty, error) { + return next.Client(ctx).Close(ctx, conn, opts...) } diff --git a/pkg/networkservice/common/heal/client_connection_map.gen.go b/pkg/networkservice/common/heal/client_connection_map.gen.go new file mode 100644 index 000000000..95ab0a288 --- /dev/null +++ b/pkg/networkservice/common/heal/client_connection_map.gen.go @@ -0,0 +1,73 @@ +// Code generated by "-output client_connection_map.gen.go -type clientConnMap -output client_connection_map.gen.go -type clientConnMap"; DO NOT EDIT. +package heal + +import ( + "sync" // Used by sync.Map. +) + +// Generate code that will fail if the constants change value. +func _() { + // An "cannot convert clientConnMap literal (type clientConnMap) to type sync.Map" compiler error signifies that the base type have changed. + // Re-run the go-syncmap command to generate them again. + _ = (sync.Map)(clientConnMap{}) +} + +var _nil_clientConnMap_clientConnInfo_value = func() (val clientConnInfo) { return }() + +// Load returns the value stored in the map for a key, or nil if no +// value is present. +// The ok result indicates whether value was found in the map. +func (m *clientConnMap) Load(key string) (clientConnInfo, bool) { + value, ok := (*sync.Map)(m).Load(key) + if value == nil { + return _nil_clientConnMap_clientConnInfo_value, ok + } + return value.(clientConnInfo), ok +} + +// Store sets the value for a key. +func (m *clientConnMap) Store(key string, value clientConnInfo) { + (*sync.Map)(m).Store(key, value) +} + +// LoadOrStore returns the existing value for the key if present. +// Otherwise, it stores and returns the given value. +// The loaded result is true if the value was loaded, false if stored. +func (m *clientConnMap) LoadOrStore(key string, value clientConnInfo) (clientConnInfo, bool) { + actual, loaded := (*sync.Map)(m).LoadOrStore(key, value) + if actual == nil { + return _nil_clientConnMap_clientConnInfo_value, loaded + } + return actual.(clientConnInfo), loaded +} + +// LoadAndDelete deletes the value for a key, returning the previous value if any. +// The loaded result reports whether the key was present. +func (m *clientConnMap) LoadAndDelete(key string) (value clientConnInfo, loaded bool) { + actual, loaded := (*sync.Map)(m).LoadAndDelete(key) + if actual == nil { + return _nil_clientConnMap_clientConnInfo_value, loaded + } + return actual.(clientConnInfo), loaded +} + +// Delete deletes the value for a key. +func (m *clientConnMap) Delete(key string) { + (*sync.Map)(m).Delete(key) +} + +// Range calls f sequentially for each key and value present in the map. +// If f returns false, range stops the iteration. +// +// Range does not necessarily correspond to any consistent snapshot of the Map's +// contents: no key will be visited more than once, but if the value for any key +// is stored or deleted concurrently, Range may reflect any mapping for that key +// from any point during the Range call. +// +// Range may be O(N) with the number of elements in the map even if f returns +// false after a constant number of calls. +func (m *clientConnMap) Range(f func(key string, value clientConnInfo) bool) { + (*sync.Map)(m).Range(func(key, value interface{}) bool { + return f(key.(string), value.(clientConnInfo)) + }) +} diff --git a/pkg/networkservice/common/heal/connection_map.gen.go b/pkg/networkservice/common/heal/connection_map.gen.go new file mode 100644 index 000000000..8cb97a2e8 --- /dev/null +++ b/pkg/networkservice/common/heal/connection_map.gen.go @@ -0,0 +1,73 @@ +// Code generated by "-output connection_map.gen.go -type connectionMap -output connection_map.gen.go -type connectionMap"; DO NOT EDIT. +package heal + +import ( + "sync" // Used by sync.Map. +) + +// Generate code that will fail if the constants change value. +func _() { + // An "cannot convert connectionMap literal (type connectionMap) to type sync.Map" compiler error signifies that the base type have changed. + // Re-run the go-syncmap command to generate them again. + _ = (sync.Map)(connectionMap{}) +} + +var _nil_connectionMap_connection_value = func() (val connection) { return }() + +// Load returns the value stored in the map for a key, or nil if no +// value is present. +// The ok result indicates whether value was found in the map. +func (m *connectionMap) Load(key string) (connection, bool) { + value, ok := (*sync.Map)(m).Load(key) + if value == nil { + return _nil_connectionMap_connection_value, ok + } + return value.(connection), ok +} + +// Store sets the value for a key. +func (m *connectionMap) Store(key string, value connection) { + (*sync.Map)(m).Store(key, value) +} + +// LoadOrStore returns the existing value for the key if present. +// Otherwise, it stores and returns the given value. +// The loaded result is true if the value was loaded, false if stored. +func (m *connectionMap) LoadOrStore(key string, value connection) (connection, bool) { + actual, loaded := (*sync.Map)(m).LoadOrStore(key, value) + if actual == nil { + return _nil_connectionMap_connection_value, loaded + } + return actual.(connection), loaded +} + +// LoadAndDelete deletes the value for a key, returning the previous value if any. +// The loaded result reports whether the key was present. +func (m *connectionMap) LoadAndDelete(key string) (value connection, loaded bool) { + actual, loaded := (*sync.Map)(m).LoadAndDelete(key) + if actual == nil { + return _nil_connectionMap_connection_value, loaded + } + return actual.(connection), loaded +} + +// Delete deletes the value for a key. +func (m *connectionMap) Delete(key string) { + (*sync.Map)(m).Delete(key) +} + +// Range calls f sequentially for each key and value present in the map. +// If f returns false, range stops the iteration. +// +// Range does not necessarily correspond to any consistent snapshot of the Map's +// contents: no key will be visited more than once, but if the value for any key +// is stored or deleted concurrently, Range may reflect any mapping for that key +// from any point during the Range call. +// +// Range may be O(N) with the number of elements in the map even if f returns +// false after a constant number of calls. +func (m *connectionMap) Range(f func(key string, value connection) bool) { + (*sync.Map)(m).Range(func(key, value interface{}) bool { + return f(key.(string), value.(connection)) + }) +} diff --git a/pkg/networkservice/common/heal/gen.go b/pkg/networkservice/common/heal/gen.go new file mode 100644 index 000000000..2e012277b --- /dev/null +++ b/pkg/networkservice/common/heal/gen.go @@ -0,0 +1,29 @@ +// Copyright (c) 2020 Cisco and/or its affiliates. +// +// Copyright (c) 2021 Doc.ai 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 heal + +import ( + "sync" +) + +//go:generate go-syncmap -output connection_map.gen.go -type connectionMap +//go:generate go-syncmap -output client_connection_map.gen.go -type clientConnMap + +type connectionMap sync.Map +type clientConnMap sync.Map diff --git a/pkg/networkservice/common/heal/server.go b/pkg/networkservice/common/heal/server.go new file mode 100644 index 000000000..91b7ed8d9 --- /dev/null +++ b/pkg/networkservice/common/heal/server.go @@ -0,0 +1,419 @@ +// Copyright (c) 2020 Cisco Systems, Inc. +// +// Copyright (c) 2021 Doc.ai 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 heal provides a chain element that carries out proper nsm healing from client to endpoint +package heal + +import ( + "context" + "github.com/networkservicemesh/sdk/pkg/tools/log" + "runtime" + "time" + + "github.com/edwarnicke/serialize" + "github.com/golang/protobuf/ptypes" + "github.com/golang/protobuf/ptypes/empty" + "github.com/pkg/errors" + "google.golang.org/grpc" + + "github.com/networkservicemesh/api/pkg/api/networkservice" + + "github.com/networkservicemesh/sdk/pkg/networkservice/common/discover" + "github.com/networkservicemesh/sdk/pkg/networkservice/core/adapters" + "github.com/networkservicemesh/sdk/pkg/networkservice/core/next" + "github.com/networkservicemesh/sdk/pkg/tools/addressof" +) + +type ctxWrapper struct { + ctx context.Context + cancel func() +} + +type clientConnInfo struct { + ctx context.Context + cc networkservice.MonitorConnectionClient +} + +type connection struct { + *networkservice.Connection +} + +type healServer struct { + ctx context.Context + clients clientConnMap + onHeal *networkservice.NetworkServiceClient + cancelHealMap map[string]*ctxWrapper + cancelHealMapExecutor serialize.Executor + conns connectionMap +} + +// NewServer - creates a new networkservice.NetworkServiceServer chain element that implements the healing algorithm +// - ctx - context for the lifecycle of the *Client* itself. Cancel when discarding the client. +// - client - networkservice.MonitorConnectionClient that can be used to call MonitorConnection against the endpoint +// - onHeal - *networkservice.NetworkServiceClient. Since networkservice.NetworkServiceClient is an interface +// (and thus a pointer) *networkservice.NetworkServiceClient is a double pointer. Meaning it +// points to a place that points to a place that implements networkservice.NetworkServiceClient +// This is done because when we use heal.NewClient as part of a chain, we may not *have* +// a pointer to this +// client used 'onHeal'. If we detect we need to heal, onHeal.Request is used to heal. +// If onHeal is nil, then we simply set onHeal to this client chain element +// If we are part of a larger chain or a server, we should pass the resulting chain into +// this constructor before we actually have a pointer to it. +// If onHeal nil, onHeal will be pointed to the returned networkservice.NetworkServiceClient +func NewServer(ctx context.Context, onHeal *networkservice.NetworkServiceClient) (networkservice.NetworkServiceServer, RegisterClientFunc) { + rv := &healServer{ + ctx: ctx, + onHeal: onHeal, + cancelHealMap: make(map[string]*ctxWrapper), + } + + if rv.onHeal == nil { + rv.onHeal = addressof.NetworkServiceClient(adapters.NewServerToClient(rv)) + } + + return rv, rv.RegisterClient +} + +func (f *healServer) Request(ctx context.Context, request *networkservice.NetworkServiceRequest) (*networkservice.Connection, error) { + // Replace path within connection to the healed one + f.replaceConnectionPath(request.GetConnection()) + + conn, err := next.Server(ctx).Request(ctx, request) + if err != nil { + return nil, err + } + + // Check heal client is in the chain and connection was added + if _, ok := f.clients.Load(conn.GetId()); !ok { + log.FromContext(ctx).WithField("healServer", "Request").Errorf("Heal server ignored connection %s: heal client is not active", conn.GetId()) + return conn, nil + } + + err = f.startHeal(ctx, request.Clone().SetRequestConnection(conn.Clone())) + if err != nil { + return nil, err + } + + f.conns.Store(conn.GetId(), connection{conn.Clone()}) + + return conn, nil +} + +func (f *healServer) Close(ctx context.Context, conn *networkservice.Connection) (*empty.Empty, error) { + // Replace path within connection to the healed one + f.replaceConnectionPath(conn) + + f.stopHeal(conn) + f.clients.Delete(conn.Id) + + rv, err := next.Server(ctx).Close(ctx, conn) + if err != nil { + return nil, err + } + return rv, nil +} + +func (f *healServer) RegisterClient(ctx context.Context, conn *networkservice.Connection, cc networkservice.MonitorConnectionClient) { + f.clients.Store(conn.Id, clientConnInfo{ + ctx: ctx, + cc: cc, + }) +} + +func (f *healServer) stopHeal(conn *networkservice.Connection) { + var cancelHeal func() + <-f.cancelHealMapExecutor.AsyncExec(func() { + if cancelHealEntry, ok := f.cancelHealMap[conn.GetId()]; ok { + cancelHeal = cancelHealEntry.cancel + delete(f.cancelHealMap, conn.GetId()) + } + }) + if cancelHeal != nil { + cancelHeal() + } + f.conns.Delete(conn.GetId()) +} + +// startHeal - start a healAsNeeded using the request as the request for re-request if healing is needed. +func (f *healServer) startHeal(ctx context.Context, request *networkservice.NetworkServiceRequest, opts ...grpc.CallOption) error { + errCh := make(chan error, 1) + go f.healAsNeeded(ctx, request, errCh, opts...) + return <-errCh +} + +// healAsNeeded - heal the connection found in request. Will immediately do a monitor to make sure the server has the +// expected connection and it is sane, returning an error via errCh if there is an issue, and nil via errCh if there is +// not. You will only *ever* receive one real error via the errCh. errCh will be closed when healAsNeeded is finished +// allowing it to double as a 'done' channel we can use when we stopHealing in f.Close. +// healAsNeeded will then continue to monitor the servers opinions about the state of the connection until either +// expireTime has passed or stopHeal is called (as in Close) or a different pathSegment is found via monitoring +// indicating that a later Request has occurred and in doing so created its own healAsNeeded and so we can stop this one +func (f *healServer) healAsNeeded(baseCtx context.Context, request *networkservice.NetworkServiceRequest, errCh chan error, opts ...grpc.CallOption) { + // When we are done, close the errCh + defer close(errCh) + + pathSegment := request.GetConnection().GetNextPathSegment() + + // Make sure we have a valid expireTime to work with + expireTime, err := ptypes.Timestamp(pathSegment.GetExpires()) + if err != nil { + errCh <- errors.Wrapf(err, "error converting pathSegment.GetExpires() to time.Time: %+v", pathSegment.GetExpires()) + return + } + + // Set the ctx Deadline to expireTime based on the heal servers context + ctx, cancel := context.WithDeadline(f.ctx, expireTime) + defer cancel() + id := request.GetConnection().GetId() + <-f.cancelHealMapExecutor.AsyncExec(func() { + if entry, ok := f.cancelHealMap[id]; ok { + go entry.cancel() // TODO - what to do with the errCh here? + } + f.cancelHealMap[id] = &ctxWrapper{ + ctx: ctx, + cancel: cancel, + } + }) + + // Monitor the pathSegment for the first time, so we can pass back an error + // if we can't confirm via monitor the other side has the expected state + recv, err := f.initialMonitorSegment(ctx, request.GetConnection(), time.Until(expireTime)) + if err != nil { + errCh <- errors.Wrapf(err, "error calling MonitorConnection_MonitorConnectionsClient.Recv to get initial confirmation server has connection: %+v", request.GetConnection()) + return + } + + // Tell the caller all is well by sending them a nil err so the call can continue + errCh <- nil + + // Start looping over events + for { + if ctx.Err() != nil { + return + } + event, err := recv.Recv() + if err != nil { + deadline := time.Now().Add(time.Minute) + if deadline.After(expireTime) { + deadline = expireTime + } + newRecv, newRecvErr := f.initialMonitorSegment(ctx, request.GetConnection(), time.Until(deadline)) + if newRecvErr == nil { + recv = newRecv + } else { + f.restoreConnection(ctx, baseCtx, request, opts...) + return + } + runtime.Gosched() + continue + } + if ctx.Err() != nil || f.ctx.Err() != nil { + return + } + if err := f.processEvent(baseCtx, request, event, opts...); err != nil { + return + } + } +} + +// initialMonitorSegment - monitors for pathSegment and returns a recv and an error if the server does not have +// a record for the connection matching our expectations +func (f *healServer) initialMonitorSegment(ctx context.Context, conn *networkservice.Connection, timeout time.Duration) (networkservice.MonitorConnection_MonitorConnectionsClient, error) { + errCh := make(chan error, 1) + var recv networkservice.MonitorConnection_MonitorConnectionsClient + pathSegment := conn.GetNextPathSegment() + + // nolint:govet + initialCtx, cancel := context.WithCancel(ctx) + + go func() { + // If pathSegment is nil, the server is very very screwed up + if pathSegment == nil { + errCh <- errors.New("pathSegment for server connection must not be nil") + return + } + + // Get connection client by connection + client, ok := f.clients.Load(conn.GetId()) + if !ok { + errCh <- errors.Errorf("error when attempting to MonitorConnections") + return + } + + // Monitor *just* this connection + var err error + recv, err = client.cc.MonitorConnections(initialCtx, &networkservice.MonitorScopeSelector{ + PathSegments: []*networkservice.PathSegment{ + pathSegment, + }, + }) + if err != nil { + errCh <- errors.Wrap(err, "error when attempting to MonitorConnections") + return + } + + // Get an initial event to make sure we have the expected connection + event, err := recv.Recv() + if err != nil { + errCh <- err + return + } + // If we didn't get an event something very bad has happened + if event.Connections == nil || event.Connections[pathSegment.GetId()] == nil { + errCh <- errors.Errorf("connection with id %s not found in MonitorConnections event as expected: event: %+v", pathSegment.Id, event) + return + } + // If its not *our* connection something's gone wrong like a later Request succeeding + if !pathSegment.Equal(event.GetConnections()[pathSegment.GetId()].GetCurrentPathSegment()) { + errCh <- errors.Errorf("server reports a different connection for this id, pathSegments do not match. Expected: %+v Received %+v", pathSegment, event.GetConnections()[pathSegment.GetId()].GetCurrentPathSegment()) + return + } + errCh <- nil + }() + + select { + case err := <-errCh: + // nolint:govet + return recv, err + case <-time.After(timeout): + cancel() + err := <-errCh + return recv, err + } +} + +// processEvent - process event, calling (*f.OnHeal).Request(ctx,request,opts...) if the server does not have our connection. +// returns a non-nil error if the event is such that we should no longer to continue to attempt to heal. +func (f *healServer) processEvent(baseCtx context.Context, request *networkservice.NetworkServiceRequest, event *networkservice.ConnectionEvent, opts ...grpc.CallOption) error { + pathSegment := request.GetConnection().GetNextPathSegment() + + switch event.GetType() { + case networkservice.ConnectionEventType_UPDATE: + // We should never receive an UPDATE that isn't ours, but in case we do... + if event.Connections == nil || event.Connections[pathSegment.GetId()] == nil { + break + } + + fallthrough + case networkservice.ConnectionEventType_INITIAL_STATE_TRANSFER: + if event.Connections != nil && event.Connections[pathSegment.GetId()] != nil { + // If the server has a pathSegment for this Connection.Id, but its not the one we + // got back from it... we should fail, as different Request came after ours successfully + if !pathSegment.Equal(event.GetConnections()[pathSegment.GetId()].GetCurrentPathSegment()) { + return errors.Errorf("server has a different pathSegment than was returned to this call.") + } + break + } + fallthrough + case networkservice.ConnectionEventType_DELETE: + if event.Connections != nil && event.Connections[pathSegment.GetId()] != nil { + f.processHeal(baseCtx, request, opts...) + } + } + return nil +} + +func (f *healServer) restoreConnection(ctx, baseCtx context.Context, request *networkservice.NetworkServiceRequest, opts ...grpc.CallOption) { + id := request.GetConnection().GetId() + + var healMapCtx context.Context + <-f.cancelHealMapExecutor.AsyncExec(func() { + if entry, ok := f.cancelHealMap[id]; ok { + healMapCtx = entry.ctx + } + }) + if healMapCtx != ctx || f.ctx.Err() != nil { + return + } + + // Make sure we have a valid expireTime to work with + expires := request.GetConnection().GetNextPathSegment().GetExpires() + expireTime, err := ptypes.Timestamp(expires) + if err != nil { + return + } + + deadline := time.Now().Add(time.Minute) + if deadline.After(expireTime) { + deadline = expireTime + } + requestCtx, requestCancel := context.WithDeadline(f.ctx, deadline) + defer requestCancel() + + for f.ctx.Err() == nil { + _, err = (*f.onHeal).Request(requestCtx, request.Clone(), opts...) + if healMapCtx != ctx { + return + } + if time.Now().After(deadline) || err == nil { + break + } + } + + if err != nil { + f.processHeal(baseCtx, request.Clone(), opts...) + } +} + +func (f *healServer) processHeal(baseCtx context.Context, request *networkservice.NetworkServiceRequest, opts ...grpc.CallOption) { + logEntry := log.FromContext(f.ctx).WithField("healServer", "processHeal") + conn := request.GetConnection() + + var err error + candidates := discover.Candidates(baseCtx) + if candidates != nil || conn.GetPath().GetIndex() == 0 { + logEntry.Infof("Starting heal process for %s", conn.GetId()) + + healCtx, healCancel := context.WithCancel(f.ctx) + defer healCancel() + + reRequest := request.Clone() + reRequest.GetConnection().NetworkServiceEndpointName = "" + path := reRequest.GetConnection().Path + reRequest.GetConnection().Path.PathSegments = path.PathSegments[0 : path.Index+1] + + for f.ctx.Err() == nil { + _, err = (*f.onHeal).Request(healCtx, reRequest, opts...) + if err != nil { + logEntry.Errorf("Failed to heal connection %s: %v", conn.GetId(), err) + } else { + logEntry.Infof("Finished heal process for %s", conn.GetId()) + break + } + } + } else { + // Huge timeout is not required to close connection on a current path segment + closeCtx, closeCancel := context.WithTimeout(f.ctx, 1*time.Second) + defer closeCancel() + + _, err := (*f.onHeal).Close(closeCtx, request.GetConnection().Clone(), opts...) + if err != nil { + logEntry.Errorf("Failed to close connection %s: %v", request.GetConnection().GetId(), err) + } + } +} + +func (f *healServer) replaceConnectionPath(conn *networkservice.Connection) { + path := conn.GetPath() + if path != nil && int(path.Index) < len(path.PathSegments)-1 { + if storedConn, ok := f.conns.Load(conn.GetId()); ok { + path.PathSegments = path.PathSegments[:path.Index+1] + path.PathSegments = append(path.PathSegments, storedConn.Path.PathSegments[path.Index+1:]...) + } + } +} diff --git a/pkg/networkservice/common/heal/client_test.go b/pkg/networkservice/common/heal/server_test.go similarity index 94% rename from pkg/networkservice/common/heal/client_test.go rename to pkg/networkservice/common/heal/server_test.go index 15abd84d4..6965f88b5 100644 --- a/pkg/networkservice/common/heal/client_test.go +++ b/pkg/networkservice/common/heal/server_test.go @@ -78,9 +78,11 @@ func TestHealClient_Request(t *testing.T) { monitor.NewServer(ctx, &monitorServer), updatetoken.NewServer(sandbox.GenerateTestToken), ) + healServer, registerClient := heal.NewServer(ctx, addressof.NetworkServiceClient(onHeal)) client := chain.NewNetworkServiceClient( updatepath.NewClient("testClient"), - heal.NewClient(ctx, adapters.NewMonitorServerToClient(monitorServer), addressof.NetworkServiceClient(onHeal)), + adapters.NewServerToClient(healServer), + heal.NewClient(ctx, adapters.NewMonitorServerToClient(monitorServer), registerClient), adapters.NewServerToClient(updatetoken.NewServer(sandbox.GenerateTestToken)), adapters.NewServerToClient(server), ) @@ -138,9 +140,12 @@ func TestHealClient_EmptyInit(t *testing.T) { ctx, cancelFunc := context.WithCancel(context.Background()) defer cancelFunc() + + healServer, registerClient := heal.NewServer(ctx, addressof.NetworkServiceClient(onHeal)) client := chain.NewNetworkServiceClient( updatepath.NewClient("testClient"), - heal.NewClient(ctx, eventchannel.NewMonitorConnectionClient(eventCh), addressof.NetworkServiceClient(onHeal)), + adapters.NewServerToClient(healServer), + heal.NewClient(ctx, eventchannel.NewMonitorConnectionClient(eventCh), registerClient), adapters.NewServerToClient(updatetoken.NewServer(sandbox.GenerateTestToken)), updatepath.NewClient("testServer"), eventTrigger, diff --git a/pkg/networkservice/common/interpose/server.go b/pkg/networkservice/common/interpose/server.go index f84d26477..bbf092fc8 100644 --- a/pkg/networkservice/common/interpose/server.go +++ b/pkg/networkservice/common/interpose/server.go @@ -71,13 +71,13 @@ func (l *interposeServer) Request(ctx context.Context, request *networkservice.N return nil, errors.Errorf("path segment doesn't have a client or cross connect nse identity") } // We need to find an Id from path to match active connection request. - activeConnID := l.getConnectionID(conn) + activeConnID, fromPath := l.getConnectionID(conn) // We came from client, so select cross nse and go to it. clientURL := clienturlctx.ClientURL(ctx) connInfo, ok := l.activeConnection.Load(activeConnID) - if ok { + if ok && (fromPath || int(conn.Path.Index) < len(conn.Path.PathSegments)-1) { if connID != activeConnID { l.activeConnection.Store(connID, connInfo) } @@ -91,7 +91,7 @@ func (l *interposeServer) Request(ctx context.Context, request *networkservice.N crossCTX := clienturlctx.WithClientURL(ctx, crossNSEURL) // Store client connection and selected cross connection URL. - connInfo, _ = l.activeConnection.LoadOrStore(activeConnID, connectionInfo{ + l.activeConnection.Store(activeConnID, connectionInfo{ clientConnID: activeConnID, endpointURL: clientURL, interposeNSEURL: crossNSEURL, @@ -127,18 +127,18 @@ func (l *interposeServer) Request(ctx context.Context, request *networkservice.N return next.Server(crossCTX).Request(crossCTX, request) } -func (l *interposeServer) getConnectionID(conn *networkservice.Connection) string { +func (l *interposeServer) getConnectionID(conn *networkservice.Connection) (string, bool) { id := conn.Id - for i := conn.GetPath().GetIndex(); i > 0; i-- { + for i := conn.GetPath().GetIndex() - 1; i > 0; i-- { activeConnID := conn.GetPath().GetPathSegments()[i].Id if connInfo, ok := l.activeConnection.Load(activeConnID); ok { if activeConnID == connInfo.clientConnID { - id = activeConnID + return activeConnID, true } break } } - return id + return id, false } func (l *interposeServer) Close(ctx context.Context, conn *networkservice.Connection) (*empty.Empty, error) { diff --git a/pkg/networkservice/common/monitor/connection_map.gen.go b/pkg/networkservice/common/monitor/connection_map.gen.go new file mode 100644 index 000000000..75faf7e92 --- /dev/null +++ b/pkg/networkservice/common/monitor/connection_map.gen.go @@ -0,0 +1,73 @@ +// Code generated by "-output connection_map.gen.go -type connectionMap -output connection_map.gen.go -type connectionMap"; DO NOT EDIT. +package monitor + +import ( + "sync" // Used by sync.Map. +) + +// Generate code that will fail if the constants change value. +func _() { + // An "cannot convert connectionMap literal (type connectionMap) to type sync.Map" compiler error signifies that the base type have changed. + // Re-run the go-syncmap command to generate them again. + _ = (sync.Map)(connectionMap{}) +} + +var _nil_connectionMap_connection_value = func() (val connection) { return }() + +// Load returns the value stored in the map for a key, or nil if no +// value is present. +// The ok result indicates whether value was found in the map. +func (m *connectionMap) Load(key string) (connection, bool) { + value, ok := (*sync.Map)(m).Load(key) + if value == nil { + return _nil_connectionMap_connection_value, ok + } + return value.(connection), ok +} + +// Store sets the value for a key. +func (m *connectionMap) Store(key string, value connection) { + (*sync.Map)(m).Store(key, value) +} + +// LoadOrStore returns the existing value for the key if present. +// Otherwise, it stores and returns the given value. +// The loaded result is true if the value was loaded, false if stored. +func (m *connectionMap) LoadOrStore(key string, value connection) (connection, bool) { + actual, loaded := (*sync.Map)(m).LoadOrStore(key, value) + if actual == nil { + return _nil_connectionMap_connection_value, loaded + } + return actual.(connection), loaded +} + +// LoadAndDelete deletes the value for a key, returning the previous value if any. +// The loaded result reports whether the key was present. +func (m *connectionMap) LoadAndDelete(key string) (value connection, loaded bool) { + actual, loaded := (*sync.Map)(m).LoadAndDelete(key) + if actual == nil { + return _nil_connectionMap_connection_value, loaded + } + return actual.(connection), loaded +} + +// Delete deletes the value for a key. +func (m *connectionMap) Delete(key string) { + (*sync.Map)(m).Delete(key) +} + +// Range calls f sequentially for each key and value present in the map. +// If f returns false, range stops the iteration. +// +// Range does not necessarily correspond to any consistent snapshot of the Map's +// contents: no key will be visited more than once, but if the value for any key +// is stored or deleted concurrently, Range may reflect any mapping for that key +// from any point during the Range call. +// +// Range may be O(N) with the number of elements in the map even if f returns +// false after a constant number of calls. +func (m *connectionMap) Range(f func(key string, value connection) bool) { + (*sync.Map)(m).Range(func(key, value interface{}) bool { + return f(key.(string), value.(connection)) + }) +} diff --git a/pkg/networkservice/common/monitor/gen.go b/pkg/networkservice/common/monitor/gen.go new file mode 100644 index 000000000..c85d96234 --- /dev/null +++ b/pkg/networkservice/common/monitor/gen.go @@ -0,0 +1,25 @@ +// Copyright (c) 2020-2021 Doc.ai 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 monitor + +import ( + "sync" +) + +//go:generate go-syncmap -output connection_map.gen.go -type connectionMap + +type connectionMap sync.Map diff --git a/pkg/networkservice/common/monitor/server.go b/pkg/networkservice/common/monitor/server.go index 2bdad2917..2b365520b 100644 --- a/pkg/networkservice/common/monitor/server.go +++ b/pkg/networkservice/common/monitor/server.go @@ -33,8 +33,12 @@ import ( "github.com/networkservicemesh/sdk/pkg/tools/log" ) +type connection struct { + *networkservice.Connection +} + type monitorServer struct { - connections map[string]*networkservice.Connection + connections connectionMap monitors []networkservice.MonitorConnection_MonitorConnectionsServer executor serialize.Executor ctx context.Context @@ -51,9 +55,8 @@ type monitorServer struct { // ctx - context for lifecycle management func NewServer(ctx context.Context, monitorServerPtr *networkservice.MonitorConnectionServer) networkservice.NetworkServiceServer { rv := &monitorServer{ - ctx: ctx, - connections: make(map[string]*networkservice.Connection), - monitors: nil, // Intentionally nil + ctx: ctx, + monitors: nil, // Intentionally nil } *monitorServerPtr = rv return rv @@ -63,11 +66,16 @@ func (m *monitorServer) MonitorConnections(selector *networkservice.MonitorScope m.executor.AsyncExec(func() { monitor := newMonitorFilter(selector, srv) m.monitors = append(m.monitors, monitor) - + connections := make(map[string]*networkservice.Connection) + for _, ps := range selector.PathSegments { + if conn, ok := m.connections.Load(ps.GetId()); ok { + connections[ps.GetId()] = conn.Connection + } + } // Send initial transfer of all data available _ = monitor.Send(&networkservice.ConnectionEvent{ Type: networkservice.ConnectionEventType_INITIAL_STATE_TRANSFER, - Connections: m.connections, + Connections: connections, }) }) select { @@ -82,7 +90,8 @@ func (m *monitorServer) Request(ctx context.Context, request *networkservice.Net eventConn := conn.Clone() if err == nil { m.executor.AsyncExec(func() { - m.connections[eventConn.GetId()] = eventConn + m.connections.Store(eventConn.GetId(), connection{eventConn}) + // Send update event event := &networkservice.ConnectionEvent{ Type: networkservice.ConnectionEventType_UPDATE, @@ -97,14 +106,21 @@ func (m *monitorServer) Request(ctx context.Context, request *networkservice.Net } func (m *monitorServer) Close(ctx context.Context, conn *networkservice.Connection) (*empty.Empty, error) { - eventConn := conn.Clone() - _, closeErr := next.Server(ctx).Close(ctx, conn) + // Use more recent version of the connection + eventConn, ok := m.connections.Load(conn.GetId()) + if !ok { + eventConn = connection{conn} + } + + _, closeErr := next.Server(ctx).Close(ctx, eventConn.Connection) + // Remove connection object we have and send DELETE m.executor.AsyncExec(func() { - delete(m.connections, eventConn.GetId()) + m.connections.Delete(eventConn.GetId()) + event := &networkservice.ConnectionEvent{ Type: networkservice.ConnectionEventType_DELETE, - Connections: map[string]*networkservice.Connection{eventConn.GetId(): eventConn}, + Connections: map[string]*networkservice.Connection{eventConn.GetId(): eventConn.Connection}, } if err := m.send(ctx, event); err != nil { log.FromContext(ctx).Errorf("Error during sending event: %v", err) @@ -126,6 +142,7 @@ func (m *monitorServer) send(ctx context.Context, event *networkservice.Connecti newMonitors = append(newMonitors, filter) } } + m.monitors = newMonitors return err } diff --git a/pkg/networkservice/common/timeout/server.go b/pkg/networkservice/common/timeout/server.go index 072085dcd..7fc25072f 100644 --- a/pkg/networkservice/common/timeout/server.go +++ b/pkg/networkservice/common/timeout/server.go @@ -54,6 +54,11 @@ func (t *timeoutServer) Request(ctx context.Context, request *networkservice.Net connID := request.GetConnection().GetId() + conn, err := next.Server(ctx).Request(ctx, request) + if err != nil { + return nil, err + } + if timer, ok := t.timers.LoadAndDelete(connID); ok { if !timer.Stop() { // Even if we failed to stop the timer, we should execute. It does mean that the timeout action @@ -65,12 +70,7 @@ func (t *timeoutServer) Request(ctx context.Context, request *networkservice.Net } } - conn, err := next.Server(ctx).Request(ctx, request) - if err != nil { - return nil, err - } - - timer, err := t.createTimer(ctx, conn) + timer, err := t.createTimer(ctx, conn.Clone()) if err != nil { if _, closeErr := next.Server(ctx).Close(ctx, conn); closeErr != nil { err = errors.Wrapf(err, "error attempting to close failed connection %v: %+v", connID, closeErr) diff --git a/pkg/registry/common/clienturl/nse_client.go b/pkg/registry/common/clienturl/nse_client.go index e7601749f..006d56c73 100644 --- a/pkg/registry/common/clienturl/nse_client.go +++ b/pkg/registry/common/clienturl/nse_client.go @@ -1,4 +1,4 @@ -// Copyright (c) 2020 Doc.ai and/or its affiliates. +// Copyright (c) 2020-2021 Doc.ai and/or its affiliates. // // SPDX-License-Identifier: Apache-2.0 // @@ -20,15 +20,15 @@ import ( "context" "sync" - "github.com/networkservicemesh/sdk/pkg/tools/clienturlctx" - - "github.com/networkservicemesh/api/pkg/api/registry" - "github.com/golang/protobuf/ptypes/empty" "github.com/pkg/errors" "google.golang.org/grpc" + "google.golang.org/grpc/connectivity" + + "github.com/networkservicemesh/api/pkg/api/registry" "github.com/networkservicemesh/sdk/pkg/registry/core/next" + "github.com/networkservicemesh/sdk/pkg/tools/clienturlctx" "github.com/networkservicemesh/sdk/pkg/tools/grpcutils" ) @@ -99,11 +99,22 @@ func (u *nseRegistryURLClient) init() error { if u.dialErr != nil { 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 } diff --git a/pkg/tools/grpcutils/errors.go b/pkg/tools/grpcutils/errors.go new file mode 100644 index 000000000..ba6403276 --- /dev/null +++ b/pkg/tools/grpcutils/errors.go @@ -0,0 +1,38 @@ +// Copyright (c) 2021 Doc.ai 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 grpcutils + +import ( + "errors" + + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" +) + +// UnwrapCode searches grpc error status Code within error +func UnwrapCode(err error) codes.Code { + if err == nil { + return codes.OK + } + for err != nil { + if code := status.Code(err); code != codes.Unknown { + return code + } + err = errors.Unwrap(err) + } + return codes.Unknown +} diff --git a/pkg/tools/sandbox/builder.go b/pkg/tools/sandbox/builder.go index 63a8a4d8f..22c5a92f0 100644 --- a/pkg/tools/sandbox/builder.go +++ b/pkg/tools/sandbox/builder.go @@ -47,6 +47,7 @@ type Builder struct { require *require.Assertions resources []context.CancelFunc nodesCount int + nodesConfig []*NodeConfig DNSDomainName string Resolver dnsresolve.Resolver supplyNSMgr SupplyNSMgrFunc @@ -100,7 +101,7 @@ func (b *Builder) Build() *Domain { } for i := 0; i < b.nodesCount; i++ { - domain.Nodes = append(domain.Nodes, b.newNode(ctx, domain.Registry.URL)) + domain.Nodes = append(domain.Nodes, b.newNode(ctx, domain.Registry.URL, b.nodesConfig[i])) } domain.resources, b.resources = b.resources, nil @@ -108,15 +109,62 @@ func (b *Builder) Build() *Domain { return domain } -// SetContext sets context for all chains +// SetContext sets default context for all chains func (b *Builder) SetContext(ctx context.Context) *Builder { b.ctx = ctx + b.SetCustomConfig([]*NodeConfig{}) + return b +} + +// SetCustomConfig sets custom configuration for nodes +func (b *Builder) SetCustomConfig(config []*NodeConfig) *Builder { + oldConfig := b.nodesConfig + b.nodesConfig = nil + + for i := 0; i < b.nodesCount; i++ { + nodeConfig := &NodeConfig{} + if i < len(config) && config[i] != nil { + *nodeConfig = *oldConfig[i] + } + + customConfig := &NodeConfig{} + if i < len(config) && config[i] != nil { + *customConfig = *config[i] + } + + if customConfig.NsmgrCtx != nil { + nodeConfig.NsmgrCtx = customConfig.NsmgrCtx + } else if nodeConfig.NsmgrCtx == nil { + nodeConfig.NsmgrCtx = b.ctx + } + + if customConfig.NsmgrGenerateTokenFunc != nil { + nodeConfig.NsmgrGenerateTokenFunc = customConfig.NsmgrGenerateTokenFunc + } else if nodeConfig.NsmgrGenerateTokenFunc == nil { + nodeConfig.NsmgrGenerateTokenFunc = b.generateTokenFunc + } + + if customConfig.ForwarderCtx != nil { + nodeConfig.ForwarderCtx = customConfig.ForwarderCtx + } else if nodeConfig.ForwarderCtx == nil { + nodeConfig.ForwarderCtx = b.ctx + } + + if customConfig.ForwarderGenerateTokenFunc != nil { + nodeConfig.ForwarderGenerateTokenFunc = customConfig.ForwarderGenerateTokenFunc + } else if nodeConfig.ForwarderGenerateTokenFunc == nil { + nodeConfig.ForwarderGenerateTokenFunc = b.generateTokenFunc + } + + b.nodesConfig = append(b.nodesConfig, nodeConfig) + } return b } // SetNodesCount sets nodes count func (b *Builder) SetNodesCount(nodesCount int) *Builder { b.nodesCount = nodesCount + b.SetCustomConfig([]*NodeConfig{}) return b } @@ -198,7 +246,17 @@ func (b *Builder) newNSMgrProxy(ctx context.Context) *EndpointEntry { } } -func (b *Builder) newNSMgr(ctx context.Context, registryURL *url.URL) *NSMgrEntry { +// NewNSMgr - starts new Network Service Manager +func (b *Builder) NewNSMgr(ctx context.Context, address string, registryURL *url.URL, generateTokenFunc token.GeneratorFunc) (entry *NSMgrEntry, resources []context.CancelFunc) { + nsmgrCtx, nsmgrCancel := context.WithCancel(ctx) + b.resources = append(b.resources, nsmgrCancel) + + entry = b.newNSMgr(nsmgrCtx, address, registryURL, generateTokenFunc) + resources, b.resources = b.resources, nil + return +} + +func (b *Builder) newNSMgr(ctx context.Context, address string, registryURL *url.URL, generateTokenFunc token.GeneratorFunc) *NSMgrEntry { if b.supplyNSMgr == nil { panic("nodes without managers are not supported") } @@ -206,7 +264,7 @@ func (b *Builder) newNSMgr(ctx context.Context, registryURL *url.URL) *NSMgrEntr if registryURL != nil { registryCC = b.dialContext(ctx, registryURL) } - listener, err := net.Listen("tcp", "127.0.0.1:0") + listener, err := net.Listen("tcp", address) b.require.NoError(err) serveURL := grpcutils.AddressToURL(listener.Addr()) b.require.NoError(listener.Close()) @@ -216,7 +274,7 @@ func (b *Builder) newNSMgr(ctx context.Context, registryURL *url.URL) *NSMgrEntr Url: serveURL.String(), } - mgr := b.supplyNSMgr(ctx, nsmgrReg, authorize.NewServer(authorize.Any()), b.generateTokenFunc, registryCC, DefaultDialOptions(b.generateTokenFunc)...) + mgr := b.supplyNSMgr(ctx, nsmgrReg, authorize.NewServer(authorize.Any()), generateTokenFunc, registryCC, DefaultDialOptions(generateTokenFunc)...) serve(ctx, serveURL, mgr.Register) log.FromContext(ctx).Infof("%v listen on: %v", nsmgrReg.Name, serveURL) @@ -271,9 +329,9 @@ func (b *Builder) newRegistry(ctx context.Context, proxyRegistryURL *url.URL) *R } } -func (b *Builder) newNode(ctx context.Context, registryURL *url.URL) *Node { - nsmgrEntry := b.newNSMgr(ctx, registryURL) - nsmgrCC := b.dialContext(ctx, nsmgrEntry.URL) +func (b *Builder) newNode(ctx context.Context, registryURL *url.URL, nodeConfig *NodeConfig) *Node { + nsmgrEntry := b.newNSMgr(nodeConfig.NsmgrCtx, "127.0.0.1:0", registryURL, nodeConfig.NsmgrGenerateTokenFunc) + nsmgrCC := b.dialContext(nodeConfig.NsmgrCtx, nsmgrEntry.URL) node := &Node{ ctx: b.ctx, @@ -284,18 +342,18 @@ func (b *Builder) newNode(ctx context.Context, registryURL *url.URL) *Node { } if b.setupNode != nil { - b.setupNode(ctx, node) + b.setupNode(ctx, node, nodeConfig) } return node } func defaultSetupNode(t *testing.T) SetupNodeFunc { - return func(ctx context.Context, node *Node) { + return func(ctx context.Context, node *Node, nodeConfig *NodeConfig) { nseReg := ®istryapi.NetworkServiceEndpoint{ - Name: uuid.New().String(), + Name: "forwarder-" + uuid.New().String(), } - _, err := node.NewForwarder(ctx, nseReg, GenerateTestToken) + _, err := node.NewForwarder(nodeConfig.ForwarderCtx, nseReg, nodeConfig.ForwarderGenerateTokenFunc) require.NoError(t, err) } } diff --git a/pkg/tools/sandbox/node.go b/pkg/tools/sandbox/node.go index f981e1873..b0a42f20e 100644 --- a/pkg/tools/sandbox/node.go +++ b/pkg/tools/sandbox/node.go @@ -18,12 +18,13 @@ package sandbox import ( "context" + "github.com/networkservicemesh/api/pkg/api/networkservice/payload" + "github.com/networkservicemesh/sdk/pkg/networkservice/common/heal" "net/url" "google.golang.org/grpc" "github.com/networkservicemesh/api/pkg/api/networkservice" - "github.com/networkservicemesh/api/pkg/api/networkservice/payload" registryapi "github.com/networkservicemesh/api/pkg/api/registry" "github.com/networkservicemesh/sdk/pkg/networkservice/chains/client" @@ -55,13 +56,14 @@ func (n *Node) NewForwarder( additionalFunctionality ...networkservice.NetworkServiceServer, ) (*EndpointEntry, error) { ep := new(EndpointEntry) + healServer, registerHealClient := heal.NewServer(ctx, addressof.NetworkServiceClient(adapters.NewServerToClient(ep))) additionalFunctionality = append(additionalFunctionality, clienturl.NewServer(n.NSMgr.URL), + healServer, connect.NewServer(ctx, client.NewCrossConnectClientFactory( client.WithName(nse.Name), - // What to call onHeal - client.WithHeal(addressof.NetworkServiceClient(adapters.NewServerToClient(ep)))), + client.WithRegisterHealClientFunc(registerHealClient)), DefaultDialOptions(generatorFunc)..., ), ) @@ -116,27 +118,34 @@ func (n *Node) newEndpoint( nse.Url = u.String() // 3. Register with the node registry client + n.RegisterEndpoint(ctx, nse, registryClient) + + log.FromContext(ctx).Infof("Started listen endpoint %s on %s.", nse.Name, u.String()) + + return &EndpointEntry{Endpoint: ep, URL: u}, nil +} +// RegisterEndpoint +func (n *Node) RegisterEndpoint(ctx context.Context, nse *registryapi.NetworkServiceEndpoint, registryClient registryapi.NetworkServiceEndpointRegistryClient) error { + var err error for _, nsName := range nse.NetworkServiceNames { if _, err = n.NSRegistryClient.Register(ctx, ®istryapi.NetworkService{ Name: nsName, Payload: payload.IP, }); err != nil { - return nil, err + return err } } var reg *registryapi.NetworkServiceEndpoint if reg, err = registryClient.Register(ctx, nse); err != nil { - return nil, err + return err } nse.Name = reg.Name nse.ExpirationTime = reg.ExpirationTime - log.FromContext(ctx).Infof("Started listen endpoint %s on %s.", nse.Name, u.String()) - - return &EndpointEntry{Endpoint: ep, URL: u}, nil + return nil } // NewClient starts a new client and connects it to the node NSMgr diff --git a/pkg/tools/sandbox/types.go b/pkg/tools/sandbox/types.go index 9b60f5d46..fe3343749 100644 --- a/pkg/tools/sandbox/types.go +++ b/pkg/tools/sandbox/types.go @@ -45,7 +45,7 @@ type SupplyRegistryFunc func(ctx context.Context, expiryDuration time.Duration, type SupplyRegistryProxyFunc func(ctx context.Context, dnsResolver dnsresolve.Resolver, handlingDNSDomain string, proxyNSMgrURL *url.URL, options ...grpc.DialOption) registry.Registry // SetupNodeFunc setups each node on Builder.Build() stage -type SetupNodeFunc func(ctx context.Context, node *Node) +type SetupNodeFunc func(ctx context.Context, node *Node, config *NodeConfig) // RegistryEntry is pair of registry.Registry and url.URL type RegistryEntry struct { @@ -76,6 +76,19 @@ type Domain struct { resources []context.CancelFunc } +// NodeConfig keeps custom node configuration parameters +type NodeConfig struct { + NsmgrCtx context.Context + NsmgrGenerateTokenFunc token.GeneratorFunc + ForwarderCtx context.Context + ForwarderGenerateTokenFunc token.GeneratorFunc +} + +// AddResources appends resources to the Domain to close it later +func (d *Domain) AddResources(resources []context.CancelFunc) { + d.resources = append(d.resources, resources...) +} + // Cleanup frees all resources related to the domain func (d *Domain) Cleanup() { for _, r := range d.resources { diff --git a/pkg/tools/sandbox/utils.go b/pkg/tools/sandbox/utils.go index e43314ab1..64407ea5c 100644 --- a/pkg/tools/sandbox/utils.go +++ b/pkg/tools/sandbox/utils.go @@ -75,6 +75,14 @@ func GenerateTestToken(_ credentials.AuthInfo) (tokenValue string, expireTime ti return "TestToken", time.Now().Add(time.Hour).Local(), nil } +// GenerateExpiringToken returns a token generator with the specified expiration duration. +func GenerateExpiringToken(duration time.Duration) token.GeneratorFunc { + value := fmt.Sprintf("TestToken-%s", duration) + return func(_ credentials.AuthInfo) (tokenValue string, expireTime time.Time, err error) { + return value, time.Now().Add(duration).Local(), nil + } +} + // NewCrossConnectClientFactory is a client.NewCrossConnectClientFactory with some fields preset for testing func NewCrossConnectClientFactory(generatorFunc token.GeneratorFunc, additionalFunctionality ...networkservice.NetworkServiceClient) client.Factory { return client.NewCrossConnectClientFactory( From f82e0461e86b0c79382b904de0489f61f06b5430 Mon Sep 17 00:00:00 2001 From: Artem Belov Date: Thu, 25 Feb 2021 15:06:04 +0700 Subject: [PATCH 02/21] Fix linter issues Signed-off-by: Artem Belov --- pkg/networkservice/chains/client/client.go | 5 +++-- .../chains/nsmgr/server_heal_test.go | 18 ++++++++++-------- pkg/networkservice/chains/nsmgrproxy/server.go | 9 ++++----- pkg/networkservice/common/heal/server.go | 2 +- pkg/tools/sandbox/node.go | 11 +++++++---- 5 files changed, 25 insertions(+), 20 deletions(-) diff --git a/pkg/networkservice/chains/client/client.go b/pkg/networkservice/chains/client/client.go index be2b9fcb0..a5eeee335 100644 --- a/pkg/networkservice/chains/client/client.go +++ b/pkg/networkservice/chains/client/client.go @@ -21,9 +21,10 @@ package client import ( "context" - "google.golang.org/grpc" "github.com/google/uuid" + "google.golang.org/grpc" + "github.com/networkservicemesh/api/pkg/api/networkservice" "github.com/networkservicemesh/sdk/pkg/networkservice/common/heal" @@ -46,7 +47,7 @@ type clientOptions struct { // Option modifies default client chain values. type Option func(c *clientOptions) -// WithHeal sets heal for the client. +// WithRegisterHealClientFunc sets server's register client method func WithRegisterHealClientFunc(registerClientFunc heal.RegisterClientFunc) Option { return Option(func(c *clientOptions) { c.registerClientFunc = registerClientFunc diff --git a/pkg/networkservice/chains/nsmgr/server_heal_test.go b/pkg/networkservice/chains/nsmgr/server_heal_test.go index 90ceaa174..cab0aeb5f 100644 --- a/pkg/networkservice/chains/nsmgr/server_heal_test.go +++ b/pkg/networkservice/chains/nsmgr/server_heal_test.go @@ -96,7 +96,7 @@ func TestNSMGR_HealEndpoint(t *testing.T) { nseCtxCancel() // Wait NSE expired and reconnecting to the new NSE - require.Eventually(t, checkRequests(2, counter.UniqueRequests), timeout, tick) + require.Eventually(t, checkSecondRequestsReceived(counter.UniqueRequests), timeout, tick) // Close. e, err := nsc.Close(ctx, conn) @@ -181,12 +181,13 @@ func testNSMGRHealForwarder(t *testing.T, nodeNum int, customConfig []*sandbox.N forwarderReg := ®istry.NetworkServiceEndpoint{ Name: "forwarder-restored", } - domain.Nodes[nodeNum].NewForwarder(ctx, forwarderReg, sandbox.GenerateTestToken) + _, err = domain.Nodes[nodeNum].NewForwarder(ctx, forwarderReg, sandbox.GenerateTestToken) + require.NoError(t, err) forwarderCtxCancel() // Wait Cross NSE expired and reconnecting through the new Cross NSE - require.Eventually(t, checkRequests(2, counter.UniqueRequests), timeout, tick) + require.Eventually(t, checkSecondRequestsReceived(counter.UniqueRequests), timeout, tick) // Close. e, err := nsc.Close(ctx, conn) @@ -265,14 +266,15 @@ func testNSMGRHealNSMgr(t *testing.T, nodeNum int, customConfig []*sandbox.NodeC forwarderReg := ®istry.NetworkServiceEndpoint{ Name: "forwarder-restored", } - domain.Nodes[nodeNum].NewForwarder(ctx, forwarderReg, sandbox.GenerateTestToken) + _, err = domain.Nodes[nodeNum].NewForwarder(ctx, forwarderReg, sandbox.GenerateTestToken) + require.NoError(t, err) nseReg.Url = nse.URL.String() err = domain.Nodes[nodeNum].RegisterEndpoint(ctx, nseReg, domain.Nodes[nodeNum].EndpointRegistryClient) require.NoError(t, err) // Wait Cross NSE expired and reconnecting through the new Cross NSE - require.Eventually(t, checkRequests(2, counter.UniqueRequests), timeout, tick) + require.Eventually(t, checkSecondRequestsReceived(counter.UniqueRequests), timeout, tick) // Close. closes := atomic.LoadInt32(&counter.Closes) @@ -347,7 +349,7 @@ func TestNSMGR_HealRemoteNSMgr(t *testing.T) { require.NoError(t, err) // Wait Cross NSE expired and reconnecting through the new Cross NSE - require.Eventually(t, checkRequests(2, counter.UniqueRequests), timeout, tick) + require.Eventually(t, checkSecondRequestsReceived(counter.UniqueRequests), timeout, tick) // Close. e, err := nsc.Close(ctx, conn) @@ -357,8 +359,8 @@ func TestNSMGR_HealRemoteNSMgr(t *testing.T) { require.Equal(t, 2, counter.UniqueCloses()) } -func checkRequests(requestsRequired int, requestsDone func() int) func() bool { +func checkSecondRequestsReceived(requestsDone func() int) func() bool { return func() bool { - return requestsRequired == requestsDone() + return requestsDone() == 2 } } diff --git a/pkg/networkservice/chains/nsmgrproxy/server.go b/pkg/networkservice/chains/nsmgrproxy/server.go index 068895579..34eaa3ffc 100644 --- a/pkg/networkservice/chains/nsmgrproxy/server.go +++ b/pkg/networkservice/chains/nsmgrproxy/server.go @@ -37,16 +37,15 @@ import ( ) // NewServer creates new proxy NSMgr - func NewServer(ctx context.Context, name string, authorizeServer networkservice.NetworkServiceServer, tokenGenerator token.GeneratorFunc, options ...grpc.DialOption) endpoint.Endpoint { - type nsmgrProxyServer struct { + type nsmgrProxyServer struct { endpoint.Endpoint } - rv := nsmgrProxyServer{} + rv := nsmgrProxyServer{} - healServer, _ := heal.NewServer(ctx, addressof.NetworkServiceClient(adapters.NewServerToClient(rv))) - rv.Endpoint = endpoint.NewServer(ctx, + healServer, _ := heal.NewServer(ctx, addressof.NetworkServiceClient(adapters.NewServerToClient(rv))) + rv.Endpoint = endpoint.NewServer(ctx, name, authorizeServer, tokenGenerator, diff --git a/pkg/networkservice/common/heal/server.go b/pkg/networkservice/common/heal/server.go index 91b7ed8d9..fe7673fb1 100644 --- a/pkg/networkservice/common/heal/server.go +++ b/pkg/networkservice/common/heal/server.go @@ -21,7 +21,6 @@ package heal import ( "context" - "github.com/networkservicemesh/sdk/pkg/tools/log" "runtime" "time" @@ -37,6 +36,7 @@ import ( "github.com/networkservicemesh/sdk/pkg/networkservice/core/adapters" "github.com/networkservicemesh/sdk/pkg/networkservice/core/next" "github.com/networkservicemesh/sdk/pkg/tools/addressof" + "github.com/networkservicemesh/sdk/pkg/tools/log" ) type ctxWrapper struct { diff --git a/pkg/tools/sandbox/node.go b/pkg/tools/sandbox/node.go index b0a42f20e..5cfdb5d8f 100644 --- a/pkg/tools/sandbox/node.go +++ b/pkg/tools/sandbox/node.go @@ -18,13 +18,12 @@ package sandbox import ( "context" - "github.com/networkservicemesh/api/pkg/api/networkservice/payload" - "github.com/networkservicemesh/sdk/pkg/networkservice/common/heal" "net/url" "google.golang.org/grpc" "github.com/networkservicemesh/api/pkg/api/networkservice" + "github.com/networkservicemesh/api/pkg/api/networkservice/payload" registryapi "github.com/networkservicemesh/api/pkg/api/registry" "github.com/networkservicemesh/sdk/pkg/networkservice/chains/client" @@ -32,6 +31,7 @@ import ( "github.com/networkservicemesh/sdk/pkg/networkservice/common/authorize" "github.com/networkservicemesh/sdk/pkg/networkservice/common/clienturl" "github.com/networkservicemesh/sdk/pkg/networkservice/common/connect" + "github.com/networkservicemesh/sdk/pkg/networkservice/common/heal" "github.com/networkservicemesh/sdk/pkg/networkservice/core/adapters" "github.com/networkservicemesh/sdk/pkg/tools/addressof" "github.com/networkservicemesh/sdk/pkg/tools/grpcutils" @@ -118,14 +118,17 @@ func (n *Node) newEndpoint( nse.Url = u.String() // 3. Register with the node registry client - n.RegisterEndpoint(ctx, nse, registryClient) + err = n.RegisterEndpoint(ctx, nse, registryClient) + if err != nil { + return nil, err + } log.FromContext(ctx).Infof("Started listen endpoint %s on %s.", nse.Name, u.String()) return &EndpointEntry{Endpoint: ep, URL: u}, nil } -// RegisterEndpoint +// RegisterEndpoint - registers endpoint in the registry client func (n *Node) RegisterEndpoint(ctx context.Context, nse *registryapi.NetworkServiceEndpoint, registryClient registryapi.NetworkServiceEndpointRegistryClient) error { var err error for _, nsName := range nse.NetworkServiceNames { From 2f6b4b949220be2f4e5551d7a7463dd6f14a6fe8 Mon Sep 17 00:00:00 2001 From: Artem Belov Date: Thu, 25 Feb 2021 15:09:13 +0700 Subject: [PATCH 03/21] Skip test depends on restoring connection Signed-off-by: Artem Belov --- pkg/networkservice/chains/nsmgr/server_heal_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/networkservice/chains/nsmgr/server_heal_test.go b/pkg/networkservice/chains/nsmgr/server_heal_test.go index cab0aeb5f..ef724ae42 100644 --- a/pkg/networkservice/chains/nsmgr/server_heal_test.go +++ b/pkg/networkservice/chains/nsmgr/server_heal_test.go @@ -198,6 +198,8 @@ func testNSMGRHealForwarder(t *testing.T, nodeNum int, customConfig []*sandbox.N } func TestNSMGR_HealRemoteNSMgrRestored(t *testing.T) { + t.Skip("Depends on https://github.com/networkservicemesh/sdk/pull/703") + nsmgrCtx, nsmgrCtxCancel := context.WithTimeout(context.Background(), time.Second) defer nsmgrCtxCancel() From 8ab29d27978d17707169b1f7f8d95649f1fa7625 Mon Sep 17 00:00:00 2001 From: Artem Belov Date: Fri, 26 Feb 2021 13:10:55 +0700 Subject: [PATCH 04/21] Fix goroutine race Signed-off-by: Artem Belov --- .../common/monitor/connection_map.gen.go | 73 ------------------- pkg/networkservice/common/monitor/gen.go | 25 ------- pkg/networkservice/common/monitor/server.go | 38 +++++----- 3 files changed, 20 insertions(+), 116 deletions(-) delete mode 100644 pkg/networkservice/common/monitor/connection_map.gen.go delete mode 100644 pkg/networkservice/common/monitor/gen.go diff --git a/pkg/networkservice/common/monitor/connection_map.gen.go b/pkg/networkservice/common/monitor/connection_map.gen.go deleted file mode 100644 index 75faf7e92..000000000 --- a/pkg/networkservice/common/monitor/connection_map.gen.go +++ /dev/null @@ -1,73 +0,0 @@ -// Code generated by "-output connection_map.gen.go -type connectionMap -output connection_map.gen.go -type connectionMap"; DO NOT EDIT. -package monitor - -import ( - "sync" // Used by sync.Map. -) - -// Generate code that will fail if the constants change value. -func _() { - // An "cannot convert connectionMap literal (type connectionMap) to type sync.Map" compiler error signifies that the base type have changed. - // Re-run the go-syncmap command to generate them again. - _ = (sync.Map)(connectionMap{}) -} - -var _nil_connectionMap_connection_value = func() (val connection) { return }() - -// Load returns the value stored in the map for a key, or nil if no -// value is present. -// The ok result indicates whether value was found in the map. -func (m *connectionMap) Load(key string) (connection, bool) { - value, ok := (*sync.Map)(m).Load(key) - if value == nil { - return _nil_connectionMap_connection_value, ok - } - return value.(connection), ok -} - -// Store sets the value for a key. -func (m *connectionMap) Store(key string, value connection) { - (*sync.Map)(m).Store(key, value) -} - -// LoadOrStore returns the existing value for the key if present. -// Otherwise, it stores and returns the given value. -// The loaded result is true if the value was loaded, false if stored. -func (m *connectionMap) LoadOrStore(key string, value connection) (connection, bool) { - actual, loaded := (*sync.Map)(m).LoadOrStore(key, value) - if actual == nil { - return _nil_connectionMap_connection_value, loaded - } - return actual.(connection), loaded -} - -// LoadAndDelete deletes the value for a key, returning the previous value if any. -// The loaded result reports whether the key was present. -func (m *connectionMap) LoadAndDelete(key string) (value connection, loaded bool) { - actual, loaded := (*sync.Map)(m).LoadAndDelete(key) - if actual == nil { - return _nil_connectionMap_connection_value, loaded - } - return actual.(connection), loaded -} - -// Delete deletes the value for a key. -func (m *connectionMap) Delete(key string) { - (*sync.Map)(m).Delete(key) -} - -// Range calls f sequentially for each key and value present in the map. -// If f returns false, range stops the iteration. -// -// Range does not necessarily correspond to any consistent snapshot of the Map's -// contents: no key will be visited more than once, but if the value for any key -// is stored or deleted concurrently, Range may reflect any mapping for that key -// from any point during the Range call. -// -// Range may be O(N) with the number of elements in the map even if f returns -// false after a constant number of calls. -func (m *connectionMap) Range(f func(key string, value connection) bool) { - (*sync.Map)(m).Range(func(key, value interface{}) bool { - return f(key.(string), value.(connection)) - }) -} diff --git a/pkg/networkservice/common/monitor/gen.go b/pkg/networkservice/common/monitor/gen.go deleted file mode 100644 index c85d96234..000000000 --- a/pkg/networkservice/common/monitor/gen.go +++ /dev/null @@ -1,25 +0,0 @@ -// Copyright (c) 2020-2021 Doc.ai 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 monitor - -import ( - "sync" -) - -//go:generate go-syncmap -output connection_map.gen.go -type connectionMap - -type connectionMap sync.Map diff --git a/pkg/networkservice/common/monitor/server.go b/pkg/networkservice/common/monitor/server.go index 2b365520b..2ba414a21 100644 --- a/pkg/networkservice/common/monitor/server.go +++ b/pkg/networkservice/common/monitor/server.go @@ -33,12 +33,8 @@ import ( "github.com/networkservicemesh/sdk/pkg/tools/log" ) -type connection struct { - *networkservice.Connection -} - type monitorServer struct { - connections connectionMap + connections map[string]*networkservice.Connection monitors []networkservice.MonitorConnection_MonitorConnectionsServer executor serialize.Executor ctx context.Context @@ -55,8 +51,9 @@ type monitorServer struct { // ctx - context for lifecycle management func NewServer(ctx context.Context, monitorServerPtr *networkservice.MonitorConnectionServer) networkservice.NetworkServiceServer { rv := &monitorServer{ - ctx: ctx, - monitors: nil, // Intentionally nil + ctx: ctx, + connections: make(map[string]*networkservice.Connection), + monitors: nil, // Intentionally nil } *monitorServerPtr = rv return rv @@ -68,8 +65,8 @@ func (m *monitorServer) MonitorConnections(selector *networkservice.MonitorScope m.monitors = append(m.monitors, monitor) connections := make(map[string]*networkservice.Connection) for _, ps := range selector.PathSegments { - if conn, ok := m.connections.Load(ps.GetId()); ok { - connections[ps.GetId()] = conn.Connection + if conn, ok := m.connections[ps.GetId()]; ok { + connections[ps.GetId()] = conn } } // Send initial transfer of all data available @@ -90,7 +87,7 @@ func (m *monitorServer) Request(ctx context.Context, request *networkservice.Net eventConn := conn.Clone() if err == nil { m.executor.AsyncExec(func() { - m.connections.Store(eventConn.GetId(), connection{eventConn}) + m.connections[eventConn.GetId()] = eventConn // Send update event event := &networkservice.ConnectionEvent{ @@ -106,21 +103,26 @@ func (m *monitorServer) Request(ctx context.Context, request *networkservice.Net } func (m *monitorServer) Close(ctx context.Context, conn *networkservice.Connection) (*empty.Empty, error) { - // Use more recent version of the connection - eventConn, ok := m.connections.Load(conn.GetId()) - if !ok { - eventConn = connection{conn} - } + var eventConn *networkservice.Connection + + <-m.executor.AsyncExec(func() { + // Use more recent version of the connection + var ok bool + eventConn, ok = m.connections[conn.GetId()] + if !ok { + eventConn = conn.Clone() + } + }) - _, closeErr := next.Server(ctx).Close(ctx, eventConn.Connection) + _, closeErr := next.Server(ctx).Close(ctx, eventConn) // Remove connection object we have and send DELETE m.executor.AsyncExec(func() { - m.connections.Delete(eventConn.GetId()) + delete(m.connections, eventConn.GetId()) event := &networkservice.ConnectionEvent{ Type: networkservice.ConnectionEventType_DELETE, - Connections: map[string]*networkservice.Connection{eventConn.GetId(): eventConn.Connection}, + Connections: map[string]*networkservice.Connection{eventConn.GetId(): eventConn}, } if err := m.send(ctx, event); err != nil { log.FromContext(ctx).Errorf("Error during sending event: %v", err) From a5d577869563a77bffc6d7e4f97e5aaaba627888 Mon Sep 17 00:00:00 2001 From: Artem Belov Date: Fri, 26 Feb 2021 17:31:02 +0700 Subject: [PATCH 05/21] Remove request updating from monitor Signed-off-by: Artem Belov --- pkg/networkservice/common/monitor/server.go | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/pkg/networkservice/common/monitor/server.go b/pkg/networkservice/common/monitor/server.go index 2ba414a21..959eb8517 100644 --- a/pkg/networkservice/common/monitor/server.go +++ b/pkg/networkservice/common/monitor/server.go @@ -103,20 +103,10 @@ func (m *monitorServer) Request(ctx context.Context, request *networkservice.Net } func (m *monitorServer) Close(ctx context.Context, conn *networkservice.Connection) (*empty.Empty, error) { - var eventConn *networkservice.Connection - - <-m.executor.AsyncExec(func() { - // Use more recent version of the connection - var ok bool - eventConn, ok = m.connections[conn.GetId()] - if !ok { - eventConn = conn.Clone() - } - }) - - _, closeErr := next.Server(ctx).Close(ctx, eventConn) + _, closeErr := next.Server(ctx).Close(ctx, conn) // Remove connection object we have and send DELETE + eventConn := conn.Clone() m.executor.AsyncExec(func() { delete(m.connections, eventConn.GetId()) From 063706dab2de04296c9880ed240e857b34954ff1 Mon Sep 17 00:00:00 2001 From: Artem Belov Date: Tue, 9 Mar 2021 14:51:02 +0700 Subject: [PATCH 06/21] Localbypass Client Signed-off-by: Artem Belov --- pkg/networkservice/chains/nsmgr/server.go | 2 + pkg/registry/common/localbypass/client.go | 61 +++++++++++++ .../common/localbypass/client_test.go | 89 +++++++++++++++++++ pkg/registry/common/localbypass/const_test.go | 22 +++++ .../common/localbypass/find_client.go | 38 ++++++++ .../common/localbypass/server_test.go | 5 -- 6 files changed, 212 insertions(+), 5 deletions(-) create mode 100644 pkg/registry/common/localbypass/client.go create mode 100644 pkg/registry/common/localbypass/client_test.go create mode 100644 pkg/registry/common/localbypass/const_test.go create mode 100644 pkg/registry/common/localbypass/find_client.go diff --git a/pkg/networkservice/chains/nsmgr/server.go b/pkg/networkservice/chains/nsmgr/server.go index b25e8d4cc..aecbad2d4 100644 --- a/pkg/networkservice/chains/nsmgr/server.go +++ b/pkg/networkservice/chains/nsmgr/server.go @@ -93,9 +93,11 @@ func NewServer(ctx context.Context, nsmRegistration *registryapi.NetworkServiceE } localBypassRegistryServer := localbypass.NewNetworkServiceEndpointRegistryServer(nsmRegistration.Url) + localBypassRegistryClient := localbypass.NewNetworkServiceEndpointRegistryClient(nsmRegistration.Url) nseClient := next.NewNetworkServiceEndpointRegistryClient( querycache.NewClient(ctx), + localBypassRegistryClient, registryadapter.NetworkServiceEndpointServerToClient(next.NewNetworkServiceEndpointRegistryServer( localBypassRegistryServer, nseRegistry)), diff --git a/pkg/registry/common/localbypass/client.go b/pkg/registry/common/localbypass/client.go new file mode 100644 index 000000000..abb8fdc04 --- /dev/null +++ b/pkg/registry/common/localbypass/client.go @@ -0,0 +1,61 @@ +// Copyright (c) 2020-2021 Doc.ai 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 localbypass implements a chain element to set NSMgr URL to endpoints on registration and set back endpoints +// URLs on find +package localbypass + +import ( + "context" + "github.com/golang/protobuf/ptypes/empty" + "github.com/networkservicemesh/api/pkg/api/registry" + "google.golang.org/grpc" + + "github.com/networkservicemesh/sdk/pkg/registry/core/next" + "github.com/networkservicemesh/sdk/pkg/tools/stringurl" +) + +type localBypassNSEClient struct { + url string + nseURLs stringurl.Map +} + +// NewNetworkServiceEndpointRegistryClient creates new instance of NetworkServiceEndpointRegistryClient which +// checks endpoints URLs on find +func NewNetworkServiceEndpointRegistryClient(u string) registry.NetworkServiceEndpointRegistryClient { + return &localBypassNSEClient{ + url: u, + } +} + +func (rc *localBypassNSEClient) Register(ctx context.Context, in *registry.NetworkServiceEndpoint, opts ...grpc.CallOption) (*registry.NetworkServiceEndpoint, error) { + return next.NetworkServiceEndpointRegistryClient(ctx).Register(ctx, in, opts...) +} + +func (rc *localBypassNSEClient) Find(ctx context.Context, in *registry.NetworkServiceEndpointQuery, opts ...grpc.CallOption) (registry.NetworkServiceEndpointRegistry_FindClient, error) { + client, err := next.NetworkServiceEndpointRegistryClient(ctx).Find(ctx, in, opts...) + if err != nil { + return client, err + } + return &localBypassNSEFindClient{ + url: rc.url, + NetworkServiceEndpointRegistry_FindClient: client, + }, nil +} + +func (rc *localBypassNSEClient) Unregister(ctx context.Context, in *registry.NetworkServiceEndpoint, opts ...grpc.CallOption) (*empty.Empty, error) { + return next.NetworkServiceEndpointRegistryClient(ctx).Unregister(ctx, in, opts...) +} diff --git a/pkg/registry/common/localbypass/client_test.go b/pkg/registry/common/localbypass/client_test.go new file mode 100644 index 000000000..03fcdbe2b --- /dev/null +++ b/pkg/registry/common/localbypass/client_test.go @@ -0,0 +1,89 @@ +// Copyright (c) 2021 Doc.ai 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 localbypass_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/registry/common/localbypass" + "github.com/networkservicemesh/sdk/pkg/registry/common/memory" + "github.com/networkservicemesh/sdk/pkg/registry/core/adapters" + "github.com/networkservicemesh/sdk/pkg/registry/core/next" +) + +func TestLocalBypassNSEClient_NSEUrl(t *testing.T) { + defer goleak.VerifyNone(t, goleak.IgnoreCurrent()) + + server := next.NewNetworkServiceEndpointRegistryClient( + localbypass.NewNetworkServiceEndpointRegistryClient(nsmgrURL), + adapters.NetworkServiceEndpointServerToClient(next.NewNetworkServiceEndpointRegistryServer( + memory.NewNetworkServiceEndpointRegistryServer(), + )), + ) + + // 1. Register + nse, err := server.Register(context.Background(), ®istry.NetworkServiceEndpoint{ + Name: "nse-1", + Url: nseURL, + }) + require.NoError(t, err) + require.Equal(t, nseURL, nse.Url) + + // 2. Find + stream, err := server.Find(context.Background(), ®istry.NetworkServiceEndpointQuery{ + NetworkServiceEndpoint: new(registry.NetworkServiceEndpoint), + }) + require.NoError(t, err) + + findNSE, err := stream.Recv() + require.NoError(t, err) + require.Equal(t, nseURL, findNSE.Url) +} + +func TestLocalBypassNSEClient_NSMgrUrl(t *testing.T) { + defer goleak.VerifyNone(t, goleak.IgnoreCurrent()) + + server := next.NewNetworkServiceEndpointRegistryClient( + localbypass.NewNetworkServiceEndpointRegistryClient(nsmgrURL), + adapters.NetworkServiceEndpointServerToClient(next.NewNetworkServiceEndpointRegistryServer( + memory.NewNetworkServiceEndpointRegistryServer(), + )), + ) + + // 1. Register + nse, err := server.Register(context.Background(), ®istry.NetworkServiceEndpoint{ + Name: "nse-1", + Url: nsmgrURL, + }) + require.NoError(t, err) + require.Equal(t, nsmgrURL, nse.Url) + + // 2. Find + stream, err := server.Find(context.Background(), ®istry.NetworkServiceEndpointQuery{ + NetworkServiceEndpoint: new(registry.NetworkServiceEndpoint), + }) + require.NoError(t, err) + + _, err = stream.Recv() + require.Error(t, err) +} diff --git a/pkg/registry/common/localbypass/const_test.go b/pkg/registry/common/localbypass/const_test.go new file mode 100644 index 000000000..1b2945afa --- /dev/null +++ b/pkg/registry/common/localbypass/const_test.go @@ -0,0 +1,22 @@ +// Copyright (c) 2021 Doc.ai 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 localbypass_test + +const ( + nsmgrURL = "tcp://0.0.0.0" + nseURL = "tcp://1.1.1.1" +) diff --git a/pkg/registry/common/localbypass/find_client.go b/pkg/registry/common/localbypass/find_client.go new file mode 100644 index 000000000..82c66c8ec --- /dev/null +++ b/pkg/registry/common/localbypass/find_client.go @@ -0,0 +1,38 @@ +// Copyright (c) 2020-2021 Doc.ai 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 localbypass + +import ( + "errors" + "github.com/networkservicemesh/api/pkg/api/registry" +) + +type localBypassNSEFindClient struct { + url string + registry.NetworkServiceEndpointRegistry_FindClient +} + +func (s *localBypassNSEFindClient) Recv() (*registry.NetworkServiceEndpoint, error) { + nse, err := s.NetworkServiceEndpointRegistry_FindClient.Recv() + if err != nil { + return nse, err + } + if nse.Url == s.url { + return nil, errors.New("NSMgr found unregistered endpoint") + } + return nse, err +} diff --git a/pkg/registry/common/localbypass/server_test.go b/pkg/registry/common/localbypass/server_test.go index e865fb511..9150db32b 100644 --- a/pkg/registry/common/localbypass/server_test.go +++ b/pkg/registry/common/localbypass/server_test.go @@ -31,11 +31,6 @@ import ( "github.com/networkservicemesh/sdk/pkg/registry/core/next" ) -const ( - nsmgrURL = "tcp://0.0.0.0" - nseURL = "tcp://1.1.1.1" -) - func TestLocalBypassNSEServer(t *testing.T) { defer goleak.VerifyNone(t, goleak.IgnoreCurrent()) From aced21e51cf3a2727f1621876912b91d00772ce3 Mon Sep 17 00:00:00 2001 From: Artem Belov Date: Tue, 9 Mar 2021 14:51:43 +0700 Subject: [PATCH 07/21] Restore last valid connection by id Signed-off-by: Artem Belov --- pkg/networkservice/common/monitor/server.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/pkg/networkservice/common/monitor/server.go b/pkg/networkservice/common/monitor/server.go index 959eb8517..d899c754b 100644 --- a/pkg/networkservice/common/monitor/server.go +++ b/pkg/networkservice/common/monitor/server.go @@ -103,6 +103,12 @@ func (m *monitorServer) Request(ctx context.Context, request *networkservice.Net } func (m *monitorServer) Close(ctx context.Context, conn *networkservice.Connection) (*empty.Empty, error) { + <-m.executor.AsyncExec(func() { + if c, ok := m.connections[conn.GetId()]; ok { + conn = c.Clone() + } + }) + _, closeErr := next.Server(ctx).Close(ctx, conn) // Remove connection object we have and send DELETE From d37f992c991f0d92fe9acdc9379ee3849ae86909 Mon Sep 17 00:00:00 2001 From: Artem Belov Date: Tue, 9 Mar 2021 17:01:55 +0700 Subject: [PATCH 08/21] Revert "Localbypass Client" This reverts commit 063706dab2de04296c9880ed240e857b34954ff1. Signed-off-by: Artem Belov --- pkg/networkservice/chains/nsmgr/server.go | 2 - pkg/registry/common/localbypass/client.go | 61 ------------- .../common/localbypass/client_test.go | 89 ------------------- pkg/registry/common/localbypass/const_test.go | 22 ----- .../common/localbypass/find_client.go | 38 -------- .../common/localbypass/server_test.go | 5 ++ 6 files changed, 5 insertions(+), 212 deletions(-) delete mode 100644 pkg/registry/common/localbypass/client.go delete mode 100644 pkg/registry/common/localbypass/client_test.go delete mode 100644 pkg/registry/common/localbypass/const_test.go delete mode 100644 pkg/registry/common/localbypass/find_client.go diff --git a/pkg/networkservice/chains/nsmgr/server.go b/pkg/networkservice/chains/nsmgr/server.go index aecbad2d4..b25e8d4cc 100644 --- a/pkg/networkservice/chains/nsmgr/server.go +++ b/pkg/networkservice/chains/nsmgr/server.go @@ -93,11 +93,9 @@ func NewServer(ctx context.Context, nsmRegistration *registryapi.NetworkServiceE } localBypassRegistryServer := localbypass.NewNetworkServiceEndpointRegistryServer(nsmRegistration.Url) - localBypassRegistryClient := localbypass.NewNetworkServiceEndpointRegistryClient(nsmRegistration.Url) nseClient := next.NewNetworkServiceEndpointRegistryClient( querycache.NewClient(ctx), - localBypassRegistryClient, registryadapter.NetworkServiceEndpointServerToClient(next.NewNetworkServiceEndpointRegistryServer( localBypassRegistryServer, nseRegistry)), diff --git a/pkg/registry/common/localbypass/client.go b/pkg/registry/common/localbypass/client.go deleted file mode 100644 index abb8fdc04..000000000 --- a/pkg/registry/common/localbypass/client.go +++ /dev/null @@ -1,61 +0,0 @@ -// Copyright (c) 2020-2021 Doc.ai 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 localbypass implements a chain element to set NSMgr URL to endpoints on registration and set back endpoints -// URLs on find -package localbypass - -import ( - "context" - "github.com/golang/protobuf/ptypes/empty" - "github.com/networkservicemesh/api/pkg/api/registry" - "google.golang.org/grpc" - - "github.com/networkservicemesh/sdk/pkg/registry/core/next" - "github.com/networkservicemesh/sdk/pkg/tools/stringurl" -) - -type localBypassNSEClient struct { - url string - nseURLs stringurl.Map -} - -// NewNetworkServiceEndpointRegistryClient creates new instance of NetworkServiceEndpointRegistryClient which -// checks endpoints URLs on find -func NewNetworkServiceEndpointRegistryClient(u string) registry.NetworkServiceEndpointRegistryClient { - return &localBypassNSEClient{ - url: u, - } -} - -func (rc *localBypassNSEClient) Register(ctx context.Context, in *registry.NetworkServiceEndpoint, opts ...grpc.CallOption) (*registry.NetworkServiceEndpoint, error) { - return next.NetworkServiceEndpointRegistryClient(ctx).Register(ctx, in, opts...) -} - -func (rc *localBypassNSEClient) Find(ctx context.Context, in *registry.NetworkServiceEndpointQuery, opts ...grpc.CallOption) (registry.NetworkServiceEndpointRegistry_FindClient, error) { - client, err := next.NetworkServiceEndpointRegistryClient(ctx).Find(ctx, in, opts...) - if err != nil { - return client, err - } - return &localBypassNSEFindClient{ - url: rc.url, - NetworkServiceEndpointRegistry_FindClient: client, - }, nil -} - -func (rc *localBypassNSEClient) Unregister(ctx context.Context, in *registry.NetworkServiceEndpoint, opts ...grpc.CallOption) (*empty.Empty, error) { - return next.NetworkServiceEndpointRegistryClient(ctx).Unregister(ctx, in, opts...) -} diff --git a/pkg/registry/common/localbypass/client_test.go b/pkg/registry/common/localbypass/client_test.go deleted file mode 100644 index 03fcdbe2b..000000000 --- a/pkg/registry/common/localbypass/client_test.go +++ /dev/null @@ -1,89 +0,0 @@ -// Copyright (c) 2021 Doc.ai 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 localbypass_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/registry/common/localbypass" - "github.com/networkservicemesh/sdk/pkg/registry/common/memory" - "github.com/networkservicemesh/sdk/pkg/registry/core/adapters" - "github.com/networkservicemesh/sdk/pkg/registry/core/next" -) - -func TestLocalBypassNSEClient_NSEUrl(t *testing.T) { - defer goleak.VerifyNone(t, goleak.IgnoreCurrent()) - - server := next.NewNetworkServiceEndpointRegistryClient( - localbypass.NewNetworkServiceEndpointRegistryClient(nsmgrURL), - adapters.NetworkServiceEndpointServerToClient(next.NewNetworkServiceEndpointRegistryServer( - memory.NewNetworkServiceEndpointRegistryServer(), - )), - ) - - // 1. Register - nse, err := server.Register(context.Background(), ®istry.NetworkServiceEndpoint{ - Name: "nse-1", - Url: nseURL, - }) - require.NoError(t, err) - require.Equal(t, nseURL, nse.Url) - - // 2. Find - stream, err := server.Find(context.Background(), ®istry.NetworkServiceEndpointQuery{ - NetworkServiceEndpoint: new(registry.NetworkServiceEndpoint), - }) - require.NoError(t, err) - - findNSE, err := stream.Recv() - require.NoError(t, err) - require.Equal(t, nseURL, findNSE.Url) -} - -func TestLocalBypassNSEClient_NSMgrUrl(t *testing.T) { - defer goleak.VerifyNone(t, goleak.IgnoreCurrent()) - - server := next.NewNetworkServiceEndpointRegistryClient( - localbypass.NewNetworkServiceEndpointRegistryClient(nsmgrURL), - adapters.NetworkServiceEndpointServerToClient(next.NewNetworkServiceEndpointRegistryServer( - memory.NewNetworkServiceEndpointRegistryServer(), - )), - ) - - // 1. Register - nse, err := server.Register(context.Background(), ®istry.NetworkServiceEndpoint{ - Name: "nse-1", - Url: nsmgrURL, - }) - require.NoError(t, err) - require.Equal(t, nsmgrURL, nse.Url) - - // 2. Find - stream, err := server.Find(context.Background(), ®istry.NetworkServiceEndpointQuery{ - NetworkServiceEndpoint: new(registry.NetworkServiceEndpoint), - }) - require.NoError(t, err) - - _, err = stream.Recv() - require.Error(t, err) -} diff --git a/pkg/registry/common/localbypass/const_test.go b/pkg/registry/common/localbypass/const_test.go deleted file mode 100644 index 1b2945afa..000000000 --- a/pkg/registry/common/localbypass/const_test.go +++ /dev/null @@ -1,22 +0,0 @@ -// Copyright (c) 2021 Doc.ai 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 localbypass_test - -const ( - nsmgrURL = "tcp://0.0.0.0" - nseURL = "tcp://1.1.1.1" -) diff --git a/pkg/registry/common/localbypass/find_client.go b/pkg/registry/common/localbypass/find_client.go deleted file mode 100644 index 82c66c8ec..000000000 --- a/pkg/registry/common/localbypass/find_client.go +++ /dev/null @@ -1,38 +0,0 @@ -// Copyright (c) 2020-2021 Doc.ai 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 localbypass - -import ( - "errors" - "github.com/networkservicemesh/api/pkg/api/registry" -) - -type localBypassNSEFindClient struct { - url string - registry.NetworkServiceEndpointRegistry_FindClient -} - -func (s *localBypassNSEFindClient) Recv() (*registry.NetworkServiceEndpoint, error) { - nse, err := s.NetworkServiceEndpointRegistry_FindClient.Recv() - if err != nil { - return nse, err - } - if nse.Url == s.url { - return nil, errors.New("NSMgr found unregistered endpoint") - } - return nse, err -} diff --git a/pkg/registry/common/localbypass/server_test.go b/pkg/registry/common/localbypass/server_test.go index 9150db32b..e865fb511 100644 --- a/pkg/registry/common/localbypass/server_test.go +++ b/pkg/registry/common/localbypass/server_test.go @@ -31,6 +31,11 @@ import ( "github.com/networkservicemesh/sdk/pkg/registry/core/next" ) +const ( + nsmgrURL = "tcp://0.0.0.0" + nseURL = "tcp://1.1.1.1" +) + func TestLocalBypassNSEServer(t *testing.T) { defer goleak.VerifyNone(t, goleak.IgnoreCurrent()) From ade908c8b8bd0b1ffcf46b4c323dc4235918531d Mon Sep 17 00:00:00 2001 From: Artem Belov Date: Tue, 9 Mar 2021 17:06:11 +0700 Subject: [PATCH 09/21] Fix stuck on unregistered endpoint Signed-off-by: Artem Belov --- pkg/registry/common/localbypass/find_server.go | 9 ++++++++- pkg/registry/common/localbypass/server.go | 3 ++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/pkg/registry/common/localbypass/find_server.go b/pkg/registry/common/localbypass/find_server.go index ba4a0c630..3e6b32a95 100644 --- a/pkg/registry/common/localbypass/find_server.go +++ b/pkg/registry/common/localbypass/find_server.go @@ -17,13 +17,16 @@ package localbypass import ( + "errors" + "github.com/networkservicemesh/api/pkg/api/registry" "github.com/networkservicemesh/sdk/pkg/tools/stringurl" ) type localBypassNSEFindServer struct { - nseURLs *stringurl.Map + nseURLs *stringurl.Map + nsmgrUrl string registry.NetworkServiceEndpointRegistry_FindServer } @@ -32,5 +35,9 @@ func (s *localBypassNSEFindServer) Send(endpoint *registry.NetworkServiceEndpoin if u, ok := s.nseURLs.Load(endpoint.Name); ok { endpoint.Url = u.String() } + if endpoint.Url == s.nsmgrUrl { + return errors.New("NSMgr found unregistered endpoint") + } + return s.NetworkServiceEndpointRegistry_FindServer.Send(endpoint) } diff --git a/pkg/registry/common/localbypass/server.go b/pkg/registry/common/localbypass/server.go index 89b84cda8..2e3bdd812 100644 --- a/pkg/registry/common/localbypass/server.go +++ b/pkg/registry/common/localbypass/server.go @@ -73,7 +73,8 @@ func (s *localBypassNSEServer) Find(query *registry.NetworkServiceEndpointQuery, func (s *localBypassNSEServer) findServer(server registry.NetworkServiceEndpointRegistry_FindServer) registry.NetworkServiceEndpointRegistry_FindServer { return &localBypassNSEFindServer{ - nseURLs: &s.nseURLs, + nseURLs: &s.nseURLs, + nsmgrUrl: s.url, NetworkServiceEndpointRegistry_FindServer: server, } } From a59fd48679f145be90332cf81c630dda074775d7 Mon Sep 17 00:00:00 2001 From: Artem Belov Date: Tue, 9 Mar 2021 17:06:55 +0700 Subject: [PATCH 10/21] Enable NSMgr restored test Signed-off-by: Artem Belov --- pkg/networkservice/chains/nsmgr/server_heal_test.go | 2 -- pkg/registry/common/localbypass/find_server.go | 4 ++-- pkg/registry/common/localbypass/server.go | 2 +- 3 files changed, 3 insertions(+), 5 deletions(-) diff --git a/pkg/networkservice/chains/nsmgr/server_heal_test.go b/pkg/networkservice/chains/nsmgr/server_heal_test.go index ef724ae42..cab0aeb5f 100644 --- a/pkg/networkservice/chains/nsmgr/server_heal_test.go +++ b/pkg/networkservice/chains/nsmgr/server_heal_test.go @@ -198,8 +198,6 @@ func testNSMGRHealForwarder(t *testing.T, nodeNum int, customConfig []*sandbox.N } func TestNSMGR_HealRemoteNSMgrRestored(t *testing.T) { - t.Skip("Depends on https://github.com/networkservicemesh/sdk/pull/703") - nsmgrCtx, nsmgrCtxCancel := context.WithTimeout(context.Background(), time.Second) defer nsmgrCtxCancel() diff --git a/pkg/registry/common/localbypass/find_server.go b/pkg/registry/common/localbypass/find_server.go index 3e6b32a95..9376243c7 100644 --- a/pkg/registry/common/localbypass/find_server.go +++ b/pkg/registry/common/localbypass/find_server.go @@ -26,7 +26,7 @@ import ( type localBypassNSEFindServer struct { nseURLs *stringurl.Map - nsmgrUrl string + nsmgrURL string registry.NetworkServiceEndpointRegistry_FindServer } @@ -35,7 +35,7 @@ func (s *localBypassNSEFindServer) Send(endpoint *registry.NetworkServiceEndpoin if u, ok := s.nseURLs.Load(endpoint.Name); ok { endpoint.Url = u.String() } - if endpoint.Url == s.nsmgrUrl { + if endpoint.Url == s.nsmgrURL { return errors.New("NSMgr found unregistered endpoint") } diff --git a/pkg/registry/common/localbypass/server.go b/pkg/registry/common/localbypass/server.go index 2e3bdd812..ffed14f12 100644 --- a/pkg/registry/common/localbypass/server.go +++ b/pkg/registry/common/localbypass/server.go @@ -74,7 +74,7 @@ func (s *localBypassNSEServer) Find(query *registry.NetworkServiceEndpointQuery, func (s *localBypassNSEServer) findServer(server registry.NetworkServiceEndpointRegistry_FindServer) registry.NetworkServiceEndpointRegistry_FindServer { return &localBypassNSEFindServer{ nseURLs: &s.nseURLs, - nsmgrUrl: s.url, + nsmgrURL: s.url, NetworkServiceEndpointRegistry_FindServer: server, } } From 752c43d7b545cfcecb02895aee463ae91cdc7c69 Mon Sep 17 00:00:00 2001 From: Artem Belov Date: Tue, 9 Mar 2021 20:26:47 +0700 Subject: [PATCH 11/21] Fix success request waiting for windows Signed-off-by: Artem Belov --- pkg/networkservice/chains/nsmgr/server_heal_test.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pkg/networkservice/chains/nsmgr/server_heal_test.go b/pkg/networkservice/chains/nsmgr/server_heal_test.go index cab0aeb5f..9dd393294 100644 --- a/pkg/networkservice/chains/nsmgr/server_heal_test.go +++ b/pkg/networkservice/chains/nsmgr/server_heal_test.go @@ -274,7 +274,9 @@ func testNSMGRHealNSMgr(t *testing.T, nodeNum int, customConfig []*sandbox.NodeC require.NoError(t, err) // Wait Cross NSE expired and reconnecting through the new Cross NSE - require.Eventually(t, checkSecondRequestsReceived(counter.UniqueRequests), timeout, tick) + require.Eventually(t, checkSecondRequestsReceived(func() int { + return int(atomic.LoadInt32(&counter.Requests)) + }), timeout, tick) // Close. closes := atomic.LoadInt32(&counter.Closes) From c4fa1e2f95ab648c2c16ebddf128a5fcfcbcb4f5 Mon Sep 17 00:00:00 2001 From: Artem Belov Date: Tue, 9 Mar 2021 22:34:08 +0700 Subject: [PATCH 12/21] Fix testing registry clients Signed-off-by: Artem Belov --- .../chains/nsmgr/server_heal_test.go | 5 +++- pkg/tools/sandbox/builder.go | 24 +++++++++++++------ 2 files changed, 21 insertions(+), 8 deletions(-) diff --git a/pkg/networkservice/chains/nsmgr/server_heal_test.go b/pkg/networkservice/chains/nsmgr/server_heal_test.go index 9dd393294..ae209341c 100644 --- a/pkg/networkservice/chains/nsmgr/server_heal_test.go +++ b/pkg/networkservice/chains/nsmgr/server_heal_test.go @@ -19,6 +19,7 @@ package nsmgr_test import ( "context" + "runtime" "sync/atomic" "testing" "time" @@ -259,7 +260,9 @@ func testNSMGRHealNSMgr(t *testing.T, nodeNum int, customConfig []*sandbox.NodeC require.Equal(t, int32(1), atomic.LoadInt32(&counter.Requests)) require.Equal(t, int32(0), atomic.LoadInt32(&counter.Closes)) - restoredNSMgrEntry, restoredNSMgrResources := builder.NewNSMgr(ctx, domain.Nodes[nodeNum].NSMgr.URL.Host, domain.Registry.URL, sandbox.GenerateTestToken) + runtime.Gosched() + + restoredNSMgrEntry, restoredNSMgrResources := builder.NewNSMgr(ctx, domain.Nodes[nodeNum], domain.Nodes[nodeNum].NSMgr.URL.Host, domain.Registry.URL, sandbox.GenerateTestToken) domain.Nodes[nodeNum].NSMgr = restoredNSMgrEntry domain.AddResources(restoredNSMgrResources) diff --git a/pkg/tools/sandbox/builder.go b/pkg/tools/sandbox/builder.go index 22c5a92f0..c8073e68d 100644 --- a/pkg/tools/sandbox/builder.go +++ b/pkg/tools/sandbox/builder.go @@ -247,11 +247,14 @@ func (b *Builder) newNSMgrProxy(ctx context.Context) *EndpointEntry { } // NewNSMgr - starts new Network Service Manager -func (b *Builder) NewNSMgr(ctx context.Context, address string, registryURL *url.URL, generateTokenFunc token.GeneratorFunc) (entry *NSMgrEntry, resources []context.CancelFunc) { +func (b *Builder) NewNSMgr(ctx context.Context, node *Node, address string, registryURL *url.URL, generateTokenFunc token.GeneratorFunc) (entry *NSMgrEntry, resources []context.CancelFunc) { nsmgrCtx, nsmgrCancel := context.WithCancel(ctx) b.resources = append(b.resources, nsmgrCancel) entry = b.newNSMgr(nsmgrCtx, address, registryURL, generateTokenFunc) + + b.SetupRegistryClients(nsmgrCtx, node) + resources, b.resources = b.resources, nil return } @@ -331,16 +334,14 @@ func (b *Builder) newRegistry(ctx context.Context, proxyRegistryURL *url.URL) *R func (b *Builder) newNode(ctx context.Context, registryURL *url.URL, nodeConfig *NodeConfig) *Node { nsmgrEntry := b.newNSMgr(nodeConfig.NsmgrCtx, "127.0.0.1:0", registryURL, nodeConfig.NsmgrGenerateTokenFunc) - nsmgrCC := b.dialContext(nodeConfig.NsmgrCtx, nsmgrEntry.URL) node := &Node{ - ctx: b.ctx, - NSMgr: nsmgrEntry, - ForwarderRegistryClient: client.NewNetworkServiceEndpointRegistryInterposeClient(ctx, nsmgrCC), - EndpointRegistryClient: client.NewNetworkServiceEndpointRegistryClient(ctx, nsmgrCC), - NSRegistryClient: client.NewNetworkServiceRegistryClient(nsmgrCC), + ctx: b.ctx, + NSMgr: nsmgrEntry, } + b.SetupRegistryClients(nodeConfig.NsmgrCtx, node) + if b.setupNode != nil { b.setupNode(ctx, node, nodeConfig) } @@ -348,6 +349,15 @@ func (b *Builder) newNode(ctx context.Context, registryURL *url.URL, nodeConfig return node } +// SetupRegistryClients - creates Network Service Registry Clients +func (b *Builder) SetupRegistryClients(ctx context.Context, node *Node) { + nsmgrCC := b.dialContext(ctx, node.NSMgr.URL) + + node.ForwarderRegistryClient = client.NewNetworkServiceEndpointRegistryInterposeClient(ctx, nsmgrCC) + node.EndpointRegistryClient = client.NewNetworkServiceEndpointRegistryClient(ctx, nsmgrCC) + node.NSRegistryClient = client.NewNetworkServiceRegistryClient(nsmgrCC) +} + func defaultSetupNode(t *testing.T) SetupNodeFunc { return func(ctx context.Context, node *Node, nodeConfig *NodeConfig) { nseReg := ®istryapi.NetworkServiceEndpoint{ From bae43f8e2914ed3b2f35b704e4a3eb276fb79629 Mon Sep 17 00:00:00 2001 From: Artem Belov Date: Wed, 17 Mar 2021 11:15:27 +0700 Subject: [PATCH 13/21] Remove registry client from RegisterEndpoint arguments Signed-off-by: Artem Belov --- pkg/networkservice/chains/nsmgr/server_heal_test.go | 2 +- pkg/tools/sandbox/node.go | 8 ++++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/pkg/networkservice/chains/nsmgr/server_heal_test.go b/pkg/networkservice/chains/nsmgr/server_heal_test.go index ae209341c..d88fdf743 100644 --- a/pkg/networkservice/chains/nsmgr/server_heal_test.go +++ b/pkg/networkservice/chains/nsmgr/server_heal_test.go @@ -273,7 +273,7 @@ func testNSMGRHealNSMgr(t *testing.T, nodeNum int, customConfig []*sandbox.NodeC require.NoError(t, err) nseReg.Url = nse.URL.String() - err = domain.Nodes[nodeNum].RegisterEndpoint(ctx, nseReg, domain.Nodes[nodeNum].EndpointRegistryClient) + err = domain.Nodes[nodeNum].RegisterEndpoint(ctx, nseReg) require.NoError(t, err) // Wait Cross NSE expired and reconnecting through the new Cross NSE diff --git a/pkg/tools/sandbox/node.go b/pkg/tools/sandbox/node.go index 5cfdb5d8f..0c2b5dad7 100644 --- a/pkg/tools/sandbox/node.go +++ b/pkg/tools/sandbox/node.go @@ -118,7 +118,7 @@ func (n *Node) newEndpoint( nse.Url = u.String() // 3. Register with the node registry client - err = n.RegisterEndpoint(ctx, nse, registryClient) + err = n.registerEndpoint(ctx, nse, registryClient) if err != nil { return nil, err } @@ -129,7 +129,11 @@ func (n *Node) newEndpoint( } // RegisterEndpoint - registers endpoint in the registry client -func (n *Node) RegisterEndpoint(ctx context.Context, nse *registryapi.NetworkServiceEndpoint, registryClient registryapi.NetworkServiceEndpointRegistryClient) error { +func (n *Node) RegisterEndpoint(ctx context.Context, nse *registryapi.NetworkServiceEndpoint) error { + return n.registerEndpoint(ctx, nse, n.EndpointRegistryClient) +} + +func (n *Node) registerEndpoint(ctx context.Context, nse *registryapi.NetworkServiceEndpoint, registryClient registryapi.NetworkServiceEndpointRegistryClient) error { var err error for _, nsName := range nse.NetworkServiceNames { if _, err = n.NSRegistryClient.Register(ctx, ®istryapi.NetworkService{ From 8840056bdafd30b5172e4d6ab6badb19ab4228ae Mon Sep 17 00:00:00 2001 From: Artem Belov Date: Wed, 17 Mar 2021 11:17:05 +0700 Subject: [PATCH 14/21] Use timestamppb.New instead of Signed-off-by: Artem Belov --- pkg/networkservice/chains/nsmgr/server_heal_test.go | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/pkg/networkservice/chains/nsmgr/server_heal_test.go b/pkg/networkservice/chains/nsmgr/server_heal_test.go index d88fdf743..17ff462c5 100644 --- a/pkg/networkservice/chains/nsmgr/server_heal_test.go +++ b/pkg/networkservice/chains/nsmgr/server_heal_test.go @@ -24,13 +24,14 @@ import ( "testing" "time" - "github.com/golang/protobuf/ptypes" + "github.com/stretchr/testify/require" + "go.uber.org/goleak" + "google.golang.org/protobuf/types/known/timestamppb" + "github.com/networkservicemesh/api/pkg/api/networkservice" "github.com/networkservicemesh/api/pkg/api/networkservice/mechanisms/cls" kernelmech "github.com/networkservicemesh/api/pkg/api/networkservice/mechanisms/kernel" "github.com/networkservicemesh/api/pkg/api/registry" - "github.com/stretchr/testify/require" - "go.uber.org/goleak" "github.com/networkservicemesh/sdk/pkg/tools/sandbox" ) @@ -52,9 +53,7 @@ func TestNSMGR_HealEndpoint(t *testing.T) { Build() defer domain.Cleanup() - expireTime, err := ptypes.TimestampProto(time.Now().Add(time.Second)) - require.NoError(t, err) - + expireTime := timestamppb.New(time.Now().Add(time.Second)) nseReg := ®istry.NetworkServiceEndpoint{ Name: "final-endpoint", NetworkServiceNames: []string{"my-service-remote"}, @@ -65,7 +64,7 @@ func TestNSMGR_HealEndpoint(t *testing.T) { nseCtx, nseCtxCancel := context.WithTimeout(context.Background(), time.Second) defer nseCtxCancel() - _, err = domain.Nodes[0].NewEndpoint(nseCtx, nseReg, sandbox.GenerateExpiringToken(time.Second), counter) + _, err := domain.Nodes[0].NewEndpoint(nseCtx, nseReg, sandbox.GenerateExpiringToken(time.Second), counter) require.NoError(t, err) request := &networkservice.NetworkServiceRequest{ From ad98d5b4ec4e2a5084afef1992a2541496adad81 Mon Sep 17 00:00:00 2001 From: Artem Belov Date: Wed, 17 Mar 2021 12:01:48 +0700 Subject: [PATCH 15/21] Fix missing candidates on refresh Signed-off-by: Artem Belov --- pkg/networkservice/common/heal/server.go | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/pkg/networkservice/common/heal/server.go b/pkg/networkservice/common/heal/server.go index fe7673fb1..76e11a500 100644 --- a/pkg/networkservice/common/heal/server.go +++ b/pkg/networkservice/common/heal/server.go @@ -51,6 +51,7 @@ type clientConnInfo struct { type connection struct { *networkservice.Connection + ctx context.Context } type healServer struct { @@ -104,12 +105,17 @@ func (f *healServer) Request(ctx context.Context, request *networkservice.Networ return conn, nil } + ctx = f.applyStoredCandidates(ctx, conn) + err = f.startHeal(ctx, request.Clone().SetRequestConnection(conn.Clone())) if err != nil { return nil, err } - f.conns.Store(conn.GetId(), connection{conn.Clone()}) + f.conns.Store(conn.GetId(), connection{ + Connection: conn.Clone(), + ctx: ctx, + }) return conn, nil } @@ -417,3 +423,16 @@ func (f *healServer) replaceConnectionPath(conn *networkservice.Connection) { } } } + +func (f *healServer) applyStoredCandidates(ctx context.Context, conn *networkservice.Connection) context.Context { + if candidates := discover.Candidates(ctx); candidates != nil && len(candidates.Endpoints) > 0 { + return ctx + } + + if info, ok := f.conns.Load(conn.Id); ok { + if candidates := discover.Candidates(info.ctx); candidates != nil { + ctx = discover.WithCandidates(ctx, candidates.Endpoints, candidates.NetworkService) + } + } + return ctx +} From 7c1875233ef5f0ab164e00145ad792eced6ece00 Mon Sep 17 00:00:00 2001 From: Artem Belov Date: Wed, 17 Mar 2021 17:13:35 +0700 Subject: [PATCH 16/21] Add restored pods testing Signed-off-by: Artem Belov --- .../chains/nsmgr/server_heal_test.go | 100 +++++++++++++++--- pkg/tools/sandbox/node.go | 3 +- 2 files changed, 89 insertions(+), 14 deletions(-) diff --git a/pkg/networkservice/chains/nsmgr/server_heal_test.go b/pkg/networkservice/chains/nsmgr/server_heal_test.go index 17ff462c5..0dd400d5b 100644 --- a/pkg/networkservice/chains/nsmgr/server_heal_test.go +++ b/pkg/networkservice/chains/nsmgr/server_heal_test.go @@ -19,7 +19,7 @@ package nsmgr_test import ( "context" - "runtime" + "net" "sync/atomic" "testing" "time" @@ -42,6 +42,14 @@ const ( ) func TestNSMGR_HealEndpoint(t *testing.T) { + testNSMGRHealEndpoint(t, false) +} + +func TestNSMGR_HealEndpointRestored(t *testing.T) { + testNSMGRHealEndpoint(t, true) +} + +func testNSMGRHealEndpoint(t *testing.T, restored bool) { defer goleak.VerifyNone(t, goleak.IgnoreCurrent()) ctx, cancel := context.WithTimeout(context.Background(), timeout) @@ -64,7 +72,7 @@ func TestNSMGR_HealEndpoint(t *testing.T) { nseCtx, nseCtxCancel := context.WithTimeout(context.Background(), time.Second) defer nseCtxCancel() - _, err := domain.Nodes[0].NewEndpoint(nseCtx, nseReg, sandbox.GenerateExpiringToken(time.Second), counter) + nse, err := domain.Nodes[0].NewEndpoint(nseCtx, nseReg, sandbox.GenerateExpiringToken(time.Second), counter) require.NoError(t, err) request := &networkservice.NetworkServiceRequest{ @@ -86,15 +94,21 @@ func TestNSMGR_HealEndpoint(t *testing.T) { require.Equal(t, 1, counter.UniqueRequests()) require.Equal(t, 8, len(conn.Path.PathSegments)) + nseCtxCancel() + + // Wait grpc unblock the port + require.Eventually(t, checkURLFree(nse.URL.Host), timeout, tick) + nseReg2 := ®istry.NetworkServiceEndpoint{ Name: "final-endpoint2", NetworkServiceNames: []string{"my-service-remote"}, } + if restored { + nseReg2.Url = nse.URL.String() + } _, err = domain.Nodes[0].NewEndpoint(ctx, nseReg2, sandbox.GenerateTestToken, counter) require.NoError(t, err) - nseCtxCancel() - // Wait NSE expired and reconnecting to the new NSE require.Eventually(t, checkSecondRequestsReceived(counter.UniqueRequests), timeout, tick) @@ -118,7 +132,22 @@ func TestNSMGR_HealLocalForwarder(t *testing.T) { }, } - testNSMGRHealForwarder(t, 1, customConfig, forwarderCtxCancel) + testNSMGRHealForwarder(t, 1, false, customConfig, forwarderCtxCancel) +} + +func TestNSMGR_HealLocalForwarderRestored(t *testing.T) { + forwarderCtx, forwarderCtxCancel := context.WithTimeout(context.Background(), time.Second) + defer forwarderCtxCancel() + + customConfig := []*sandbox.NodeConfig{ + nil, + { + ForwarderCtx: forwarderCtx, + ForwarderGenerateTokenFunc: sandbox.GenerateExpiringToken(time.Second), + }, + } + + testNSMGRHealForwarder(t, 1, true, customConfig, forwarderCtxCancel) } func TestNSMGR_HealRemoteForwarder(t *testing.T) { @@ -132,10 +161,24 @@ func TestNSMGR_HealRemoteForwarder(t *testing.T) { }, } - testNSMGRHealForwarder(t, 0, customConfig, forwarderCtxCancel) + testNSMGRHealForwarder(t, 0, false, customConfig, forwarderCtxCancel) +} + +func TestNSMGR_HealRemoteForwarderRestored(t *testing.T) { + forwarderCtx, forwarderCtxCancel := context.WithTimeout(context.Background(), time.Second) + defer forwarderCtxCancel() + + customConfig := []*sandbox.NodeConfig{ + { + ForwarderCtx: forwarderCtx, + ForwarderGenerateTokenFunc: sandbox.GenerateExpiringToken(time.Second), + }, + } + + testNSMGRHealForwarder(t, 0, true, customConfig, forwarderCtxCancel) } -func testNSMGRHealForwarder(t *testing.T, nodeNum int, customConfig []*sandbox.NodeConfig, forwarderCtxCancel context.CancelFunc) { +func testNSMGRHealForwarder(t *testing.T, nodeNum int, restored bool, customConfig []*sandbox.NodeConfig, forwarderCtxCancel context.CancelFunc) { defer goleak.VerifyNone(t, goleak.IgnoreCurrent()) ctx, cancel := context.WithTimeout(context.Background(), time.Second*100) defer cancel() @@ -177,24 +220,44 @@ func testNSMGRHealForwarder(t *testing.T, nodeNum int, customConfig []*sandbox.N require.Equal(t, 1, counter.UniqueRequests()) require.Equal(t, 8, len(conn.Path.PathSegments)) + forwarderCtxCancel() + + // Wait grpc unblock the port + require.GreaterOrEqual(t, 1, len(domain.Nodes[nodeNum].Forwarder)) + require.Eventually(t, checkURLFree(domain.Nodes[nodeNum].Forwarder[0].URL.Host), timeout, tick) + nseReg.Name = "cross-nse-restored" forwarderReg := ®istry.NetworkServiceEndpoint{ Name: "forwarder-restored", } + if restored { + forwarderReg.Url = domain.Nodes[nodeNum].Forwarder[0].URL.String() + } _, err = domain.Nodes[nodeNum].NewForwarder(ctx, forwarderReg, sandbox.GenerateTestToken) require.NoError(t, err) - forwarderCtxCancel() - // Wait Cross NSE expired and reconnecting through the new Cross NSE - require.Eventually(t, checkSecondRequestsReceived(counter.UniqueRequests), timeout, tick) + if restored { + require.Eventually(t, checkSecondRequestsReceived(func() int { + return int(atomic.LoadInt32(&counter.Requests)) + }), timeout, tick) + } else { + require.Eventually(t, checkSecondRequestsReceived(counter.UniqueRequests), timeout, tick) + } // Close. + closes := atomic.LoadInt32(&counter.Closes) e, err := nsc.Close(ctx, conn) require.NoError(t, err) require.NotNil(t, e) - require.Equal(t, 2, counter.UniqueRequests()) - require.Equal(t, 2, counter.UniqueCloses()) + + if restored { + require.Equal(t, int32(2), atomic.LoadInt32(&counter.Requests)) + require.Equal(t, closes+1, atomic.LoadInt32(&counter.Closes)) + } else { + require.Equal(t, 2, counter.UniqueRequests()) + require.Equal(t, 2, counter.UniqueCloses()) + } } func TestNSMGR_HealRemoteNSMgrRestored(t *testing.T) { @@ -259,7 +322,7 @@ func testNSMGRHealNSMgr(t *testing.T, nodeNum int, customConfig []*sandbox.NodeC require.Equal(t, int32(1), atomic.LoadInt32(&counter.Requests)) require.Equal(t, int32(0), atomic.LoadInt32(&counter.Closes)) - runtime.Gosched() + require.Eventually(t, checkURLFree(domain.Nodes[nodeNum].NSMgr.URL.Host), timeout, tick) restoredNSMgrEntry, restoredNSMgrResources := builder.NewNSMgr(ctx, domain.Nodes[nodeNum], domain.Nodes[nodeNum].NSMgr.URL.Host, domain.Registry.URL, sandbox.GenerateTestToken) domain.Nodes[nodeNum].NSMgr = restoredNSMgrEntry @@ -368,3 +431,14 @@ func checkSecondRequestsReceived(requestsDone func() int) func() bool { return requestsDone() == 2 } } + +func checkURLFree(url string) func() bool { + return func() bool { + ln, err := net.Listen("tcp", url) + if err != nil { + return false + } + err = ln.Close() + return err == nil + } +} diff --git a/pkg/tools/sandbox/node.go b/pkg/tools/sandbox/node.go index 0c2b5dad7..26c8d7420 100644 --- a/pkg/tools/sandbox/node.go +++ b/pkg/tools/sandbox/node.go @@ -43,6 +43,7 @@ import ( type Node struct { ctx context.Context NSMgr *NSMgrEntry + Forwarder []*EndpointEntry ForwarderRegistryClient registryapi.NetworkServiceEndpointRegistryClient EndpointRegistryClient registryapi.NetworkServiceEndpointRegistryClient NSRegistryClient registryapi.NetworkServiceRegistryClient @@ -73,7 +74,7 @@ func (n *Node) NewForwarder( return nil, err } *ep = *entry - + n.Forwarder = append(n.Forwarder, ep) return ep, nil } From 920c1d8bdebbca1785b85237b096f59df84788b8 Mon Sep 17 00:00:00 2001 From: Artem Belov Date: Thu, 18 Mar 2021 12:12:19 +0700 Subject: [PATCH 17/21] Revert localbypass fix; Skip NSMgr restored heal Signed-off-by: Artem Belov --- pkg/networkservice/chains/nsmgr/server_heal_test.go | 2 ++ pkg/registry/common/localbypass/find_server.go | 9 +-------- pkg/registry/common/localbypass/server.go | 3 +-- 3 files changed, 4 insertions(+), 10 deletions(-) diff --git a/pkg/networkservice/chains/nsmgr/server_heal_test.go b/pkg/networkservice/chains/nsmgr/server_heal_test.go index 0dd400d5b..c6af09585 100644 --- a/pkg/networkservice/chains/nsmgr/server_heal_test.go +++ b/pkg/networkservice/chains/nsmgr/server_heal_test.go @@ -261,6 +261,8 @@ func testNSMGRHealForwarder(t *testing.T, nodeNum int, restored bool, customConf } func TestNSMGR_HealRemoteNSMgrRestored(t *testing.T) { + t.Skip("https://github.com/networkservicemesh/sdk/issues/770") + nsmgrCtx, nsmgrCtxCancel := context.WithTimeout(context.Background(), time.Second) defer nsmgrCtxCancel() diff --git a/pkg/registry/common/localbypass/find_server.go b/pkg/registry/common/localbypass/find_server.go index 9376243c7..ba4a0c630 100644 --- a/pkg/registry/common/localbypass/find_server.go +++ b/pkg/registry/common/localbypass/find_server.go @@ -17,16 +17,13 @@ package localbypass import ( - "errors" - "github.com/networkservicemesh/api/pkg/api/registry" "github.com/networkservicemesh/sdk/pkg/tools/stringurl" ) type localBypassNSEFindServer struct { - nseURLs *stringurl.Map - nsmgrURL string + nseURLs *stringurl.Map registry.NetworkServiceEndpointRegistry_FindServer } @@ -35,9 +32,5 @@ func (s *localBypassNSEFindServer) Send(endpoint *registry.NetworkServiceEndpoin if u, ok := s.nseURLs.Load(endpoint.Name); ok { endpoint.Url = u.String() } - if endpoint.Url == s.nsmgrURL { - return errors.New("NSMgr found unregistered endpoint") - } - return s.NetworkServiceEndpointRegistry_FindServer.Send(endpoint) } diff --git a/pkg/registry/common/localbypass/server.go b/pkg/registry/common/localbypass/server.go index ffed14f12..89b84cda8 100644 --- a/pkg/registry/common/localbypass/server.go +++ b/pkg/registry/common/localbypass/server.go @@ -73,8 +73,7 @@ func (s *localBypassNSEServer) Find(query *registry.NetworkServiceEndpointQuery, func (s *localBypassNSEServer) findServer(server registry.NetworkServiceEndpointRegistry_FindServer) registry.NetworkServiceEndpointRegistry_FindServer { return &localBypassNSEFindServer{ - nseURLs: &s.nseURLs, - nsmgrURL: s.url, + nseURLs: &s.nseURLs, NetworkServiceEndpointRegistry_FindServer: server, } } From 70891b76dfa7aa8e69a605d29cfcbb44b0c54560 Mon Sep 17 00:00:00 2001 From: Artem Belov Date: Thu, 18 Mar 2021 16:45:07 +0700 Subject: [PATCH 18/21] Fix data race Signed-off-by: Artem Belov --- pkg/registry/common/querycache/nse_client.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/registry/common/querycache/nse_client.go b/pkg/registry/common/querycache/nse_client.go index 37aef2b19..b35481a6f 100644 --- a/pkg/registry/common/querycache/nse_client.go +++ b/pkg/registry/common/querycache/nse_client.go @@ -79,7 +79,7 @@ func (q *queryCacheNSEClient) findInCache(ctx context.Context, key string) (regi } resultCh := make(chan *registry.NetworkServiceEndpoint, 1) - resultCh <- nse + resultCh <- nse.Clone() close(resultCh) return streamchannel.NewNetworkServiceEndpointFindClient(ctx, resultCh), true From 6e1807c26f6eeaf3e50e71d929dd2329b3d5a1c2 Mon Sep 17 00:00:00 2001 From: Artem Belov Date: Mon, 22 Mar 2021 11:32:50 +0700 Subject: [PATCH 19/21] Fix refresh after heal request; Add refresh after heal testing Signed-off-by: Artem Belov --- .../chains/nsmgr/server_heal_test.go | 29 ++++++++++++++++++- pkg/networkservice/common/heal/server.go | 1 + 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/pkg/networkservice/chains/nsmgr/server_heal_test.go b/pkg/networkservice/chains/nsmgr/server_heal_test.go index c6af09585..81029d8c5 100644 --- a/pkg/networkservice/chains/nsmgr/server_heal_test.go +++ b/pkg/networkservice/chains/nsmgr/server_heal_test.go @@ -111,6 +111,12 @@ func testNSMGRHealEndpoint(t *testing.T, restored bool) { // Wait NSE expired and reconnecting to the new NSE require.Eventually(t, checkSecondRequestsReceived(counter.UniqueRequests), timeout, tick) + require.Equal(t, 2, counter.UniqueRequests()) + + // Check refresh + request.Connection = conn + _, err = nsc.Request(ctx, request.Clone()) + require.NoError(t, err) // Close. e, err := nsc.Close(ctx, conn) @@ -244,6 +250,16 @@ func testNSMGRHealForwarder(t *testing.T, nodeNum int, restored bool, customConf } else { require.Eventually(t, checkSecondRequestsReceived(counter.UniqueRequests), timeout, tick) } + if restored { + require.Equal(t, int32(2), atomic.LoadInt32(&counter.Requests)) + } else { + require.Equal(t, 2, counter.UniqueRequests()) + } + + // Check refresh + request.Connection = conn + _, err = nsc.Request(ctx, request.Clone()) + require.NoError(t, err) // Close. closes := atomic.LoadInt32(&counter.Closes) @@ -252,7 +268,7 @@ func testNSMGRHealForwarder(t *testing.T, nodeNum int, restored bool, customConf require.NotNil(t, e) if restored { - require.Equal(t, int32(2), atomic.LoadInt32(&counter.Requests)) + require.Equal(t, int32(3), atomic.LoadInt32(&counter.Requests)) require.Equal(t, closes+1, atomic.LoadInt32(&counter.Closes)) } else { require.Equal(t, 2, counter.UniqueRequests()) @@ -345,6 +361,11 @@ func testNSMGRHealNSMgr(t *testing.T, nodeNum int, customConfig []*sandbox.NodeC return int(atomic.LoadInt32(&counter.Requests)) }), timeout, tick) + // Check refresh + request.Connection = conn + _, err = nsc.Request(ctx, request.Clone()) + require.NoError(t, err) + // Close. closes := atomic.LoadInt32(&counter.Closes) e, err := nsc.Close(ctx, conn) @@ -419,6 +440,12 @@ func TestNSMGR_HealRemoteNSMgr(t *testing.T) { // Wait Cross NSE expired and reconnecting through the new Cross NSE require.Eventually(t, checkSecondRequestsReceived(counter.UniqueRequests), timeout, tick) + require.Equal(t, 2, counter.UniqueRequests()) + + // Check refresh + request.Connection = conn + _, err = nsc.Request(ctx, request.Clone()) + require.NoError(t, err) // Close. e, err := nsc.Close(ctx, conn) diff --git a/pkg/networkservice/common/heal/server.go b/pkg/networkservice/common/heal/server.go index 76e11a500..22a851bb9 100644 --- a/pkg/networkservice/common/heal/server.go +++ b/pkg/networkservice/common/heal/server.go @@ -420,6 +420,7 @@ func (f *healServer) replaceConnectionPath(conn *networkservice.Connection) { if storedConn, ok := f.conns.Load(conn.GetId()); ok { path.PathSegments = path.PathSegments[:path.Index+1] path.PathSegments = append(path.PathSegments, storedConn.Path.PathSegments[path.Index+1:]...) + conn.NetworkServiceEndpointName = storedConn.NetworkServiceEndpointName } } } From 30dde036856d6291d0d2deab893be6721992b431 Mon Sep 17 00:00:00 2001 From: Artem Belov Date: Mon, 22 Mar 2021 12:28:24 +0700 Subject: [PATCH 20/21] Close error chan on initial monitor done Signed-off-by: Artem Belov --- pkg/networkservice/common/heal/server.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/pkg/networkservice/common/heal/server.go b/pkg/networkservice/common/heal/server.go index 22a851bb9..96c97a994 100644 --- a/pkg/networkservice/common/heal/server.go +++ b/pkg/networkservice/common/heal/server.go @@ -170,9 +170,6 @@ func (f *healServer) startHeal(ctx context.Context, request *networkservice.Netw // expireTime has passed or stopHeal is called (as in Close) or a different pathSegment is found via monitoring // indicating that a later Request has occurred and in doing so created its own healAsNeeded and so we can stop this one func (f *healServer) healAsNeeded(baseCtx context.Context, request *networkservice.NetworkServiceRequest, errCh chan error, opts ...grpc.CallOption) { - // When we are done, close the errCh - defer close(errCh) - pathSegment := request.GetConnection().GetNextPathSegment() // Make sure we have a valid expireTime to work with @@ -205,7 +202,7 @@ func (f *healServer) healAsNeeded(baseCtx context.Context, request *networkservi } // Tell the caller all is well by sending them a nil err so the call can continue - errCh <- nil + close(errCh) // Start looping over events for { From ed6ea144601f9f8b41882c5da4683b2069cb1c8a Mon Sep 17 00:00:00 2001 From: Artem Belov Date: Tue, 23 Mar 2021 14:04:38 +0700 Subject: [PATCH 21/21] Pass register heal client func via context Signed-off-by: Artem Belov --- pkg/networkservice/chains/client/client.go | 10 +---- pkg/networkservice/chains/nsmgr/server.go | 5 +-- .../chains/nsmgrproxy/server.go | 3 +- .../common/discover/match_selector.go | 2 +- pkg/networkservice/common/heal/client.go | 18 ++++---- pkg/networkservice/common/heal/context.go | 41 +++++++++++++++++++ pkg/networkservice/common/heal/server.go | 5 ++- pkg/networkservice/common/heal/server_test.go | 8 ++-- pkg/tools/sandbox/node.go | 5 +-- 9 files changed, 63 insertions(+), 34 deletions(-) create mode 100644 pkg/networkservice/common/heal/context.go diff --git a/pkg/networkservice/chains/client/client.go b/pkg/networkservice/chains/client/client.go index a5eeee335..8c1f434e8 100644 --- a/pkg/networkservice/chains/client/client.go +++ b/pkg/networkservice/chains/client/client.go @@ -41,19 +41,11 @@ type clientOptions struct { name string additionalFunctionality []networkservice.NetworkServiceClient authorizeClient networkservice.NetworkServiceClient - registerClientFunc heal.RegisterClientFunc } // Option modifies default client chain values. type Option func(c *clientOptions) -// WithRegisterHealClientFunc sets server's register client method -func WithRegisterHealClientFunc(registerClientFunc heal.RegisterClientFunc) Option { - return Option(func(c *clientOptions) { - c.registerClientFunc = registerClientFunc - }) -} - // WithName sets name for the client. func WithName(name string) Option { return Option(func(c *clientOptions) { @@ -95,7 +87,7 @@ func NewClient(ctx context.Context, cc grpc.ClientConnInterface, clientOpts ...O append([]networkservice.NetworkServiceClient{ updatepath.NewClient(opts.name), serialize.NewClient(), - heal.NewClient(ctx, networkservice.NewMonitorConnectionClient(cc), opts.registerClientFunc), + heal.NewClient(ctx, networkservice.NewMonitorConnectionClient(cc)), refresh.NewClient(ctx), metadata.NewClient(), }, opts.additionalFunctionality...), diff --git a/pkg/networkservice/chains/nsmgr/server.go b/pkg/networkservice/chains/nsmgr/server.go index 0ef7f8fe8..fb69ed2ae 100644 --- a/pkg/networkservice/chains/nsmgr/server.go +++ b/pkg/networkservice/chains/nsmgr/server.go @@ -112,8 +112,6 @@ func NewServer(ctx context.Context, nsmRegistration *registryapi.NetworkServiceE nsClient := registryadapter.NetworkServiceServerToClient(nsRegistry) - healServer, registerHealClient := heal.NewServer(ctx, addressof.NetworkServiceClient(adapters.NewServerToClient(rv))) - // Construct Endpoint rv.Endpoint = endpoint.NewServer(ctx, tokenGenerator, endpoint.WithName(nsmRegistration.Name), @@ -125,11 +123,10 @@ func NewServer(ctx context.Context, nsmRegistration *registryapi.NetworkServiceE recvfd.NewServer(), // Receive any files passed interpose.NewServer(&interposeRegistryServer), filtermechanisms.NewServer(&urlsRegistryServer), - healServer, + heal.NewServer(ctx, addressof.NetworkServiceClient(adapters.NewServerToClient(rv))), connect.NewServer(ctx, client.NewClientFactory( client.WithName(nsmRegistration.Name), - client.WithRegisterHealClientFunc(registerHealClient), client.WithAdditionalFunctionality( recvfd.NewClient(), sendfd.NewClient(), diff --git a/pkg/networkservice/chains/nsmgrproxy/server.go b/pkg/networkservice/chains/nsmgrproxy/server.go index b612e5cde..f1fe737fa 100644 --- a/pkg/networkservice/chains/nsmgrproxy/server.go +++ b/pkg/networkservice/chains/nsmgrproxy/server.go @@ -78,7 +78,6 @@ func NewServer(ctx context.Context, tokenGenerator token.GeneratorFunc, options endpoint.Endpoint } rv := nsmgrProxyServer{} - healServer, _ := heal.NewServer(ctx, addressof.NetworkServiceClient(adapters.NewServerToClient(rv))) opts := &serverOptions{ name: "nsmgr-proxy-" + uuid.New().String(), @@ -95,7 +94,7 @@ func NewServer(ctx context.Context, tokenGenerator token.GeneratorFunc, options interdomainurl.NewServer(), externalips.NewServer(ctx), swapip.NewServer(), - healServer, + heal.NewServer(ctx, addressof.NetworkServiceClient(adapters.NewServerToClient(rv))), connect.NewServer(ctx, client.NewClientFactory(client.WithName(opts.name)), connect.WithDialOptions(opts.dialOptions...), diff --git a/pkg/networkservice/common/discover/match_selector.go b/pkg/networkservice/common/discover/match_selector.go index 69f699617..9c354da73 100644 --- a/pkg/networkservice/common/discover/match_selector.go +++ b/pkg/networkservice/common/discover/match_selector.go @@ -43,7 +43,7 @@ func isSubset(a, b, nsLabels map[string]string) bool { } func matchEndpoint(nsLabels map[string]string, ns *registry.NetworkService, networkServiceEndpoints ...*registry.NetworkServiceEndpoint) []*registry.NetworkServiceEndpoint { - validNetworkServiceEndpoints := make([]*registry.NetworkServiceEndpoint, 0) + var validNetworkServiceEndpoints []*registry.NetworkServiceEndpoint for _, nse := range networkServiceEndpoints { if nse.GetExpirationTime() == nil || nse.GetExpirationTime().AsTime().After(time.Now()) { validNetworkServiceEndpoints = append(validNetworkServiceEndpoints, nse) diff --git a/pkg/networkservice/common/heal/client.go b/pkg/networkservice/common/heal/client.go index afebb9565..59c6ba8fd 100644 --- a/pkg/networkservice/common/heal/client.go +++ b/pkg/networkservice/common/heal/client.go @@ -32,24 +32,24 @@ import ( type RegisterClientFunc func(context.Context, *networkservice.Connection, networkservice.MonitorConnectionClient) type healClient struct { - ctx context.Context - cc networkservice.MonitorConnectionClient - registerClient RegisterClientFunc + ctx context.Context + cc networkservice.MonitorConnectionClient } // NewClient - creates a new networkservice.NetworkServiceClient chain element that inform healServer about new client connection -func NewClient(ctx context.Context, cc networkservice.MonitorConnectionClient, registerClient RegisterClientFunc) networkservice.NetworkServiceClient { +func NewClient(ctx context.Context, cc networkservice.MonitorConnectionClient) networkservice.NetworkServiceClient { return &healClient{ - ctx: ctx, - cc: cc, - registerClient: registerClient, + ctx: ctx, + cc: cc, } } func (u *healClient) Request(ctx context.Context, request *networkservice.NetworkServiceRequest, opts ...grpc.CallOption) (*networkservice.Connection, error) { conn, err := next.Client(ctx).Request(ctx, request, opts...) - if err == nil && u.registerClient != nil { - u.registerClient(u.ctx, conn, u.cc) + + registerClient := registerClientFunc(ctx) + if err == nil && registerClient != nil { + registerClient(u.ctx, conn, u.cc) } return conn, err } diff --git a/pkg/networkservice/common/heal/context.go b/pkg/networkservice/common/heal/context.go new file mode 100644 index 000000000..dfc9a4b81 --- /dev/null +++ b/pkg/networkservice/common/heal/context.go @@ -0,0 +1,41 @@ +// Copyright (c) 2021 Doc.ai 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 heal + +import ( + "context" +) + +const ( + registerClientFuncKey contextKeyType = "RegisterFunc" +) + +type contextKeyType string + +func withRegisterClientFunc(parent context.Context, registerClientFunc RegisterClientFunc) context.Context { + if parent == nil { + panic("cannot create context from nil parent") + } + return context.WithValue(parent, registerClientFuncKey, registerClientFunc) +} + +func registerClientFunc(ctx context.Context) RegisterClientFunc { + if rv, ok := ctx.Value(registerClientFuncKey).(RegisterClientFunc); ok { + return rv + } + return nil +} diff --git a/pkg/networkservice/common/heal/server.go b/pkg/networkservice/common/heal/server.go index 96c97a994..4755856c6 100644 --- a/pkg/networkservice/common/heal/server.go +++ b/pkg/networkservice/common/heal/server.go @@ -76,7 +76,7 @@ type healServer struct { // If we are part of a larger chain or a server, we should pass the resulting chain into // this constructor before we actually have a pointer to it. // If onHeal nil, onHeal will be pointed to the returned networkservice.NetworkServiceClient -func NewServer(ctx context.Context, onHeal *networkservice.NetworkServiceClient) (networkservice.NetworkServiceServer, RegisterClientFunc) { +func NewServer(ctx context.Context, onHeal *networkservice.NetworkServiceClient) networkservice.NetworkServiceServer { rv := &healServer{ ctx: ctx, onHeal: onHeal, @@ -87,13 +87,14 @@ func NewServer(ctx context.Context, onHeal *networkservice.NetworkServiceClient) rv.onHeal = addressof.NetworkServiceClient(adapters.NewServerToClient(rv)) } - return rv, rv.RegisterClient + return rv } func (f *healServer) Request(ctx context.Context, request *networkservice.NetworkServiceRequest) (*networkservice.Connection, error) { // Replace path within connection to the healed one f.replaceConnectionPath(request.GetConnection()) + ctx = withRegisterClientFunc(ctx, f.RegisterClient) conn, err := next.Server(ctx).Request(ctx, request) if err != nil { return nil, err diff --git a/pkg/networkservice/common/heal/server_test.go b/pkg/networkservice/common/heal/server_test.go index 6965f88b5..351c8ebca 100644 --- a/pkg/networkservice/common/heal/server_test.go +++ b/pkg/networkservice/common/heal/server_test.go @@ -78,11 +78,11 @@ func TestHealClient_Request(t *testing.T) { monitor.NewServer(ctx, &monitorServer), updatetoken.NewServer(sandbox.GenerateTestToken), ) - healServer, registerClient := heal.NewServer(ctx, addressof.NetworkServiceClient(onHeal)) + healServer := heal.NewServer(ctx, addressof.NetworkServiceClient(onHeal)) client := chain.NewNetworkServiceClient( updatepath.NewClient("testClient"), adapters.NewServerToClient(healServer), - heal.NewClient(ctx, adapters.NewMonitorServerToClient(monitorServer), registerClient), + heal.NewClient(ctx, adapters.NewMonitorServerToClient(monitorServer)), adapters.NewServerToClient(updatetoken.NewServer(sandbox.GenerateTestToken)), adapters.NewServerToClient(server), ) @@ -141,11 +141,11 @@ func TestHealClient_EmptyInit(t *testing.T) { ctx, cancelFunc := context.WithCancel(context.Background()) defer cancelFunc() - healServer, registerClient := heal.NewServer(ctx, addressof.NetworkServiceClient(onHeal)) + healServer := heal.NewServer(ctx, addressof.NetworkServiceClient(onHeal)) client := chain.NewNetworkServiceClient( updatepath.NewClient("testClient"), adapters.NewServerToClient(healServer), - heal.NewClient(ctx, eventchannel.NewMonitorConnectionClient(eventCh), registerClient), + heal.NewClient(ctx, eventchannel.NewMonitorConnectionClient(eventCh)), adapters.NewServerToClient(updatetoken.NewServer(sandbox.GenerateTestToken)), updatepath.NewClient("testServer"), eventTrigger, diff --git a/pkg/tools/sandbox/node.go b/pkg/tools/sandbox/node.go index c53dfbe1a..6b4e008dd 100644 --- a/pkg/tools/sandbox/node.go +++ b/pkg/tools/sandbox/node.go @@ -57,14 +57,13 @@ func (n *Node) NewForwarder( additionalFunctionality ...networkservice.NetworkServiceServer, ) (*EndpointEntry, error) { ep := new(EndpointEntry) - healServer, registerHealClient := heal.NewServer(ctx, addressof.NetworkServiceClient(adapters.NewServerToClient(ep))) additionalFunctionality = append(additionalFunctionality, clienturl.NewServer(n.NSMgr.URL), - healServer, + heal.NewServer(ctx, addressof.NetworkServiceClient(adapters.NewServerToClient(ep))), connect.NewServer(ctx, client.NewCrossConnectClientFactory( client.WithName(nse.Name), - client.WithRegisterHealClientFunc(registerHealClient)), + ), connect.WithDialOptions(DefaultDialOptions(generatorFunc)...), ), )