qfix: Add support for empty routes with metadata (#1194)

* add support for empty routes Signed-off-by: denis-tingaikin <denis.tingajkin@xored.com> * make condition more strick Signed-off-by: denis-tingaikin <denis.tingajkin@xored.com>
networkservicemesh · Dec 7, 2021 · 56d8eca · 56d8eca
1 parent 23e4c36
commit 56d8eca
Show file tree

Hide file tree

Showing 3 changed files with 118 additions and 0 deletions.
diff --git a/pkg/networkservice/chains/nsmgr/single_test.go b/pkg/networkservice/chains/nsmgr/single_test.go
@@ -18,6 +18,7 @@ package nsmgr_test
 
 import (
 	"context"
+	"fmt"
 	"io/ioutil"
 	"os"
 	"path/filepath"
@@ -282,3 +283,115 @@ func Test_UsecasePoint2MultiPoint(t *testing.T) {
 	require.Equal(t, 4, len(conn.Path.PathSegments))
 	require.Equal(t, "p2p forwarder", conn.GetPath().GetPathSegments()[2].Name)
 }
+func Test_RemoteUsecase_Point2MultiPoint(t *testing.T) {
+	t.Cleanup(func() { goleak.VerifyNone(t) })
+
+	ctx, cancel := context.WithTimeout(context.Background(), time.Second*5)
+	defer cancel()
+
+	const nodeCount = 2
+
+	domain := sandbox.NewBuilder(ctx, t).
+		SetNodesCount(nodeCount).
+		SetRegistryProxySupplier(nil).
+		SetNodeSetup(func(ctx context.Context, node *sandbox.Node, _ int) {
+			node.NewNSMgr(ctx, "nsmgr", nil, sandbox.GenerateTestToken, nsmgr.NewServer)
+		}).
+		SetRegistryExpiryDuration(sandbox.RegistryExpiryDuration).
+		Build()
+
+	for i := 0; i < nodeCount; i++ {
+		domain.Nodes[i].NewForwarder(ctx, &registry.NetworkServiceEndpoint{
+			Name:                "p2mp forwarder-" + fmt.Sprint(i),
+			NetworkServiceNames: []string{"forwarder"},
+			NetworkServiceLabels: map[string]*registry.NetworkServiceLabels{
+				"forwarder": {
+					Labels: map[string]string{
+						"p2mp": "true",
+					},
+				},
+			},
+		}, sandbox.GenerateTestToken)
+
+		domain.Nodes[i].NewForwarder(ctx, &registry.NetworkServiceEndpoint{
+			Name:                "p2p forwarder-" + fmt.Sprint(i),
+			NetworkServiceNames: []string{"forwarder"},
+			NetworkServiceLabels: map[string]*registry.NetworkServiceLabels{
+				"forwarder": {
+					Labels: map[string]string{
+						"p2p": "true",
+					},
+				},
+			},
+		}, sandbox.GenerateTestToken)
+
+		domain.Nodes[i].NewForwarder(ctx, &registry.NetworkServiceEndpoint{
+			Name:                "special forwarder-" + fmt.Sprint(i),
+			NetworkServiceNames: []string{"forwarder"},
+			NetworkServiceLabels: map[string]*registry.NetworkServiceLabels{
+				"forwarder": {
+					Labels: map[string]string{
+						"special": "true",
+					},
+				},
+			},
+		}, sandbox.GenerateTestToken)
+	}
+	nsRegistryClient := domain.NewNSRegistryClient(ctx, sandbox.GenerateTestToken)
+
+	_, err := nsRegistryClient.Register(ctx, &registry.NetworkService{
+		Name: "my-ns",
+		Matches: []*registry.Match{
+			{
+				Metadata: &registry.Metadata{
+					Labels: map[string]string{
+						"p2mp": "true",
+					},
+				},
+			},
+		},
+	})
+	require.NoError(t, err)
+
+	nseReg := &registry.NetworkServiceEndpoint{
+		Name:                "my-nse-1",
+		NetworkServiceNames: []string{"my-ns"},
+	}
+
+	domain.Nodes[1].NewEndpoint(ctx, nseReg, sandbox.GenerateTestToken)
+
+	nsc := domain.Nodes[0].NewClient(ctx, sandbox.GenerateTestToken)
+
+	request := defaultRequest("my-ns")
+
+	conn, err := nsc.Request(ctx, request.Clone())
+	require.NoError(t, err)
+	require.NotNil(t, conn)
+	require.Equal(t, 6, len(conn.Path.PathSegments))
+	require.Equal(t, "p2mp forwarder-0", conn.GetPath().GetPathSegments()[2].Name)
+	require.Equal(t, "p2mp forwarder-1", conn.GetPath().GetPathSegments()[4].Name)
+
+	_, err = nsRegistryClient.Register(ctx, &registry.NetworkService{
+		Name: "my-ns",
+		Matches: []*registry.Match{
+			{
+				Metadata: &registry.Metadata{
+					Labels: map[string]string{
+						// no labels
+					},
+				},
+			},
+		},
+	})
+	require.NoError(t, err)
+
+	_, err = nsc.Close(ctx, conn)
+	require.NoError(t, err)
+
+	conn, err = nsc.Request(ctx, request.Clone())
+	require.NoError(t, err)
+	require.NotNil(t, conn)
+	require.Equal(t, 6, len(conn.Path.PathSegments))
+	require.Equal(t, "p2p forwarder-0", conn.GetPath().GetPathSegments()[2].Name)
+	require.Equal(t, "p2p forwarder-1", conn.GetPath().GetPathSegments()[4].Name)
+}
diff --git a/pkg/networkservice/common/discover/match_selector.go b/pkg/networkservice/common/discover/match_selector.go
@@ -59,6 +59,10 @@ func matchEndpoint(clockTime clock.Clock, nsLabels map[string]string, ns *regist
 			continue
 		}
 
+		if match.GetMetadata() != nil && len(match.Routes) == 0 && len(nseCandidates) == 0 {
+			break
+		}
+
 		return nseCandidates
 	}
 

diff --git a/pkg/networkservice/common/discoverforwarder/server.go b/pkg/networkservice/common/discoverforwarder/server.go
@@ -107,6 +107,7 @@ func (d *discoverForwarderServer) Request(ctx context.Context, request *networks
 				storeForwarderName(ctx, candidate.Name)
 				return resp, nil
 			}
+			logger.Errorf("forwarder=%v url=%v returned error=%v", candidate.Name, candidate.Url, err.Error())
 		}
 
 		return nil, errors.New("all forwarders failed")
-Original file line number
+Diff line change
@@ Expand Up @@
     			continue
     		}
+    		if match.GetMetadata() != nil && len(match.Routes) == 0 && len(nseCandidates) == 0 {
+    			break
+    		}
     		return nseCandidates
     	}
@@ Expand Down @@