Fix connect (networkservicemesh#585)

* Added translateMechanism chain element Signed-off-by: Tigran Manasyan <tigran.manasyan@xored.com> * Rework connect.NewServer Signed-off-by: Vladimir Popov <vladimir.popov@xored.com> * Add RefcountMap.LoadUnsafe() Signed-off-by: Vladimir Popov <vladimir.popov@xored.com> * Fix connect Signed-off-by: Vladimir Popov <vladimir.popov@xored.com> * Move translation client implementations to the related places Signed-off-by: Vladimir Popov <vladimir.popov@xored.com> * Add new clients, rework connect Signed-off-by: Vladimir Popov <vladimir.popov@xored.com> * Rename translation to mechanismtranslation Signed-off-by: Vladimir Popov <vladimir.popov@xored.com> * Make RefcountMap.Delete() return bool Signed-off-by: Vladimir Popov <vladimir.popov@xored.com> Co-authored-by: Tigran Manasyan <tigran.manasyan@xored.com> Signed-off-by: Sergey Ershov <sergey.ershov@xored.com>
illbegood · Dec 20, 2020 · 228c248 · 228c248
1 parent f98513f
commit 228c248
Show file tree

Hide file tree

Showing 20 changed files with 976 additions and 491 deletions.
diff --git a/.golangci.yml b/.golangci.yml
@@ -253,3 +253,7 @@ issues:
       linters:
         - gocyclo
       text: "processEvent"
+    - path: pkg/networkservice/common/connect/server_test.go
+      linters:
+        - funlen
+      text: "Function 'TestConnectServer_Request'"
diff --git a/pkg/networkservice/chains/client/README.md b/pkg/networkservice/chains/client/README.md
@@ -0,0 +1,51 @@
+# Functional requirements
+
+There are some common chain elements that we expect to have in every client chain to make NSM working. Instead of that,
+there are few different scenarios when we need to create a client chain to initiate NSM request:
+1. Client to NSMgr - simple case when there is an application requesting some L2/L3 connection from the NSMgr.
+    * no incoming L2/L3 request - client itself is a request generator
+    * complete chain
+    ```
+    Client   --Request-->   NSMgr
+       |                      |
+       |---L2/L3 connection---|
+       |                      |
+    ```
+2. Server to endpoint client - we already have application running as a NSM endpoint receiving request to L2/L3
+connection, but it also needs to request some other L2/L3 connection from some other endpoint.
+    * there is an incoming L2/L3 request - we need to generate an outgoing L2/L3 request, but the connection we return
+      is an incoming connection
+    * part of some server chain - we need to add `clientConnection` and request next elements
+    ```
+    ...                 Endpoint  --Request-->  Endpoint
+     |                      |                      |
+     |---L2/L3 connection---|---L2/L3 connection---|
+     |                      |                      |
+    ```
+3. Proxy to endpoint client - we already have application running as a NSM server, but it doesn't provide L2/L3
+connection, it simply passes the request to some other endpoint.
+    * there is an incoming L2/L3 request but we simply forward it
+    * part of some server chain - we need to add `clientConnection` and request next elements
+    ```
+    ...                   Proxy   --Request-->  Endpoint
+     |                      |                      |
+     |---------------L2/L3 connection--------------|
+     |                      |                      |
+    ```
+
+# Implementation
+
+## client.NewClient(..., grpcCC, ...additionalFunctionality)
+
+It is a solution for the (1.) case. Client appends `additionalFunctionality` to the default client chain and passes
+incoming request to the NSMgr over the `grpcCC`.
+
+## client.NewCrossConnectClientFactory(..., ...additionalFunctionality)
+
+It is a solution for the (2.) case. We create a new GRPC client on each new client URL received from the incoming request.
+It can be used in `connect.NewServer` so `clientConnection` will be processed correctly.
+
+## client.NewClientFactory(..., ...additionalFunctionality)
+
+It is a solution for the (3.) case. We create a new GRPC client on each new client URL received, but process like (1.).
+It can be used in `connect.NewServer` so `clientConnection` will be processed correctly.
diff --git a/pkg/networkservice/chains/client/client.go b/pkg/networkservice/chains/client/client.go
@@ -20,23 +20,22 @@ package client
 import (
 	"context"
 
-	"github.com/networkservicemesh/sdk/pkg/networkservice/common/updatetoken"
-	"github.com/networkservicemesh/sdk/pkg/networkservice/utils/inject/injectpeer"
-
 	"github.com/networkservicemesh/api/pkg/api/networkservice"
 	"google.golang.org/grpc"
 
 	"github.com/networkservicemesh/sdk/pkg/networkservice/common/authorize"
 	"github.com/networkservicemesh/sdk/pkg/networkservice/common/heal"
+	"github.com/networkservicemesh/sdk/pkg/networkservice/common/mechanismtranslation"
 	"github.com/networkservicemesh/sdk/pkg/networkservice/common/refresh"
 	"github.com/networkservicemesh/sdk/pkg/networkservice/common/updatepath"
+	"github.com/networkservicemesh/sdk/pkg/networkservice/common/updatetoken"
 	"github.com/networkservicemesh/sdk/pkg/networkservice/core/chain"
+	"github.com/networkservicemesh/sdk/pkg/networkservice/utils/inject/injectpeer"
 
 	"github.com/networkservicemesh/sdk/pkg/tools/token"
 )
 
-// NewClient - returns a NetworkServiceMesh client as a chain of the standard Client pieces plus whatever
-//             additional functionality is specified
+// NewClient - returns a (1.) case NSM client.
 //             - ctx    - context for the lifecycle of the *Client* itself.  Cancel when discarding the client.
 //             - name   - name of the NetworkServiceMeshClient
 //             - onHeal - *networkservice.NetworkServiceClient.  Since networkservice.NetworkServiceClient is an interface
@@ -71,20 +70,17 @@ func NewClient(ctx context.Context, name string, onHeal *networkservice.NetworkS
 	return rv
 }
 
-// NewClientFactory - returns a func(cc grpc.ClientConnInterface)that returns a  standard Client pieces plus whatever
-//                    additional functionality is specified
-//                    - name - name of the NetworkServiceMeshClient
-//                    - 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
-//                    - additionalFunctionality - any additional NetworkServiceClient chain elements to be included in the chain
+// NewCrossConnectClientFactory - returns a (2.) case func(cc grpc.ClientConnInterface) NSM client factory.
+func NewCrossConnectClientFactory(name string, onHeal *networkservice.NetworkServiceClient, tokenGenerator token.GeneratorFunc, additionalFunctionality ...networkservice.NetworkServiceClient) func(ctx context.Context, cc grpc.ClientConnInterface) networkservice.NetworkServiceClient {
+	return func(ctx context.Context, cc grpc.ClientConnInterface) networkservice.NetworkServiceClient {
+		return chain.NewNetworkServiceClient(
+			mechanismtranslation.NewClient(),
+			NewClient(ctx, name, onHeal, tokenGenerator, cc, additionalFunctionality...),
+		)
+	}
+}
+
+// NewClientFactory - returns a (3.) case func(cc grpc.ClientConnInterface) NSM client factory.
 func NewClientFactory(name string, onHeal *networkservice.NetworkServiceClient, tokenGenerator token.GeneratorFunc, additionalFunctionality ...networkservice.NetworkServiceClient) func(ctx context.Context, cc grpc.ClientConnInterface) networkservice.NetworkServiceClient {
 	return func(ctx context.Context, cc grpc.ClientConnInterface) networkservice.NetworkServiceClient {
 		return NewClient(ctx, name, onHeal, tokenGenerator, cc, additionalFunctionality...)

diff --git a/pkg/networkservice/chains/nsmgr/server.go b/pkg/networkservice/chains/nsmgr/server.go
@@ -22,6 +22,7 @@ package nsmgr
 import (
 	"context"
 
+	"github.com/networkservicemesh/sdk/pkg/networkservice/chains/client"
 	"github.com/networkservicemesh/sdk/pkg/registry/common/querycache"
 	"github.com/networkservicemesh/sdk/pkg/registry/core/next"
 
@@ -43,7 +44,6 @@ import (
 	"github.com/networkservicemesh/sdk/pkg/registry/core/nextwrap"
 	"github.com/networkservicemesh/sdk/pkg/registry/memory"
 
-	"github.com/networkservicemesh/sdk/pkg/networkservice/chains/client"
 	"github.com/networkservicemesh/sdk/pkg/networkservice/chains/endpoint"
 	"github.com/networkservicemesh/sdk/pkg/networkservice/common/connect"
 	"github.com/networkservicemesh/sdk/pkg/networkservice/common/discover"
@@ -114,11 +114,10 @@ func NewServer(ctx context.Context, nsmRegistration *registryapi.NetworkServiceE
 		newRecvFD(), // Receive any files passed
 		interpose.NewServer(&interposeRegistry),
 		filtermechanisms.NewServer(&urlsRegistryServer),
-		connect.NewServer(
-			ctx,
-			client.NewClientFactory(nsmRegistration.Name,
-				addressof.NetworkServiceClient(
-					adapters.NewServerToClient(rv)),
+		connect.NewServer(ctx,
+			client.NewClientFactory(
+				nsmRegistration.Name,
+				addressof.NetworkServiceClient(adapters.NewServerToClient(rv)),
 				tokenGenerator,
 				newSendFDClient(), // Send passed files.
 			),