Rework setid to use in NSMgr chain

Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>
networkservicemesh · Feb 10, 2021 · 63d1537 · 63d1537
1 parent 9b293dd
commit 63d1537
Show file tree

Hide file tree

Showing 4 changed files with 76 additions and 43 deletions.
diff --git a/pkg/networkservice/chains/nsmgr/server.go b/pkg/networkservice/chains/nsmgr/server.go
@@ -94,10 +94,7 @@ func NewServer(ctx context.Context, nsmRegistration *registryapi.NetworkServiceE
 
 	nseRegistry := newRemoteNSEServer(registryCC)
 	if nseRegistry == nil {
-		nseRegistry = chain_registry.NewNetworkServiceEndpointRegistryServer(
-			setid.NewNetworkServiceEndpointRegistryServer(),  // If no remote registry then assign ID.
-			memory.NewNetworkServiceEndpointRegistryServer(), // Memory registry to store result inside.
-		)
+		nseRegistry = memory.NewNetworkServiceEndpointRegistryServer() // Memory registry to store result inside.
 	}
 
 	nseClient := next.NewNetworkServiceEndpointRegistryClient(
@@ -136,6 +133,7 @@ func NewServer(ctx context.Context, nsmRegistration *registryapi.NetworkServiceE
 	nseChain := chain_registry.NewNamedNetworkServiceEndpointRegistryServer(
 		nsmRegistration.Name+".NetworkServiceEndpointRegistry",
 		expire.NewNetworkServiceEndpointRegistryServer(time.Minute),
+		setid.NewNetworkServiceEndpointRegistryServer(),           // Assign ID
 		registry_recvfd.NewNetworkServiceEndpointRegistryServer(), // Allow to receive a passed files
 		urlsRegistryServer,
 		interposeRegistry,         // Store cross connect NSEs

diff --git a/pkg/networkservice/chains/nsmgr/server_test.go b/pkg/networkservice/chains/nsmgr/server_test.go
@@ -510,6 +510,63 @@ func TestNSMGR_PassThroughLocal(t *testing.T) {
 	require.Equal(t, 5*(nsesCount-1)+5, len(conn.Path.PathSegments))
 }
 
+func TestNSMGR_ShouldCorrectlyAddForwardersWithSameNames(t *testing.T) {
+	defer goleak.VerifyNone(t, goleak.IgnoreCurrent())
+
+	ctx, cancel := context.WithTimeout(context.Background(), time.Second*5)
+	defer cancel()
+
+	domain := sandbox.NewBuilder(t).
+		SetNodesCount(1).
+		SetRegistryProxySupplier(nil).
+		SetNodeSetup(nil).
+		SetContext(ctx).
+		Build()
+	defer domain.Cleanup()
+
+	forwarderReg := &registry.NetworkServiceEndpoint{
+		Name: "forwarder",
+	}
+
+	nseReg := &registry.NetworkServiceEndpoint{
+		Name:                "endpoint",
+		NetworkServiceNames: []string{"service"},
+	}
+
+	// 1. Add forwarders
+	forwarder1Reg := forwarderReg.Clone()
+	_, err := domain.Nodes[0].NewForwarder(ctx, forwarder1Reg, sandbox.GenerateTestToken)
+	require.NoError(t, err)
+
+	forwarder2Reg := forwarderReg.Clone()
+	_, err = domain.Nodes[0].NewForwarder(ctx, forwarder2Reg, sandbox.GenerateTestToken)
+	require.NoError(t, err)
+
+	forwarder3Reg := forwarderReg.Clone()
+	_, err = domain.Nodes[0].NewForwarder(ctx, forwarder3Reg, sandbox.GenerateTestToken)
+	require.NoError(t, err)
+
+	// 2. Wait for refresh
+	<-time.After(sandbox.DefaultRegistryExpiryDuration)
+
+	testNSEAndClient(ctx, t, domain, nseReg.Clone())
+
+	// 3. Delete first forwarder
+	_, err = domain.Nodes[0].ForwarderRegistryClient.Unregister(ctx, forwarder1Reg)
+	require.NoError(t, err)
+
+	testNSEAndClient(ctx, t, domain, nseReg.Clone())
+
+	// 4. Delete last forwarder
+	_, err = domain.Nodes[0].ForwarderRegistryClient.Unregister(ctx, forwarder3Reg)
+	require.NoError(t, err)
+
+	testNSEAndClient(ctx, t, domain, nseReg.Clone())
+
+	_, err = domain.Nodes[0].ForwarderRegistryClient.Unregister(ctx, forwarder2Reg)
+	require.NoError(t, err)
+}
+
 func TestNSMGR_ShouldCorrectlyAddEndpointsWithSameNames(t *testing.T) {
 	defer goleak.VerifyNone(t, goleak.IgnoreCurrent())
 

diff --git a/pkg/registry/common/setid/server.go b/pkg/registry/common/setid/server.go
@@ -39,19 +39,19 @@ func NewNetworkServiceEndpointRegistryServer() registry.NetworkServiceEndpointRe
 }
 
 func (s *setIDServer) Register(ctx context.Context, nse *registry.NetworkServiceEndpoint) (*registry.NetworkServiceEndpoint, error) {
-	name, suffix := interDomainName(nse.Name)
-	if _, ok := s.names.Load(name); !ok {
-		name = strings.Join(append(nse.NetworkServiceNames, uuid.New().String()), "-")
+	if _, ok := s.names.Load(nse.Name); !ok {
+		if nse.Name == "" {
+			nse.Name = strings.Join(nse.NetworkServiceNames, "-")
+		}
+		nse.Name = uuid.New().String() + "-" + nse.Name
 	}
-	nse.Name = name + suffix
 
 	reg, err := next.NetworkServiceEndpointRegistryServer(ctx).Register(ctx, nse)
 	if err != nil {
 		return nil, err
 	}
 
-	name, _ = interDomainName(reg.Name)
-	s.names.Store(name, struct{}{})
+	s.names.Store(reg.Name, struct{}{})
 
 	return reg, nil
 }
@@ -61,18 +61,9 @@ func (s *setIDServer) Find(query *registry.NetworkServiceEndpointQuery, server r
 }
 
 func (s *setIDServer) Unregister(ctx context.Context, nse *registry.NetworkServiceEndpoint) (*empty.Empty, error) {
-	name, _ := interDomainName(nse.Name)
-	s.names.Delete(name)
+	s.names.Delete(nse.Name)
 
 	return next.NetworkServiceEndpointRegistryServer(ctx).Unregister(ctx, nse)
 }
 
-func interDomainName(rawName string) (name, suffix string) {
-	split := strings.Split(rawName, "@")
-	if len(split) == 1 {
-		return rawName, ""
-	}
-	return split[0], "@" + strings.Join(split[1:], "@")
-}
-
 var _ registry.NetworkServiceEndpointRegistryServer = &setIDServer{}
diff --git a/pkg/registry/common/setid/server_test.go b/pkg/registry/common/setid/server_test.go
@@ -31,25 +31,23 @@ import (
 )
 
 const (
-	nseName      = "nse-1"
-	domain       = "domain.com"
-	remoteSuffix = "-remote"
+	remotePrefix = "remote-"
 )
 
-func testNSE(name string) *registry.NetworkServiceEndpoint {
+func testNSE() *registry.NetworkServiceEndpoint {
 	return &registry.NetworkServiceEndpoint{
-		Name:                name,
+		Name:                "nse-1",
 		NetworkServiceNames: []string{"ns-1"},
 	}
 }
 
 func TestSetIDServer_NewNSE(t *testing.T) {
 	server := setid.NewNetworkServiceEndpointRegistryServer()
 
-	reg1, err := server.Register(context.Background(), testNSE(nseName))
+	reg1, err := server.Register(context.Background(), testNSE())
 	require.NoError(t, err)
 
-	reg2, err := server.Register(context.Background(), testNSE(nseName))
+	reg2, err := server.Register(context.Background(), testNSE())
 	require.NoError(t, err)
 
 	require.NotEqual(t, reg1.Name, reg2.Name)
@@ -64,7 +62,7 @@ func TestSetIDServer_NewNSE(t *testing.T) {
 func TestSetIDServer_RefreshNSE(t *testing.T) {
 	server := setid.NewNetworkServiceEndpointRegistryServer()
 
-	reg, err := server.Register(context.Background(), testNSE(nseName))
+	reg, err := server.Register(context.Background(), testNSE())
 	require.NoError(t, err)
 
 	name := reg.Name
@@ -78,25 +76,13 @@ func TestSetIDServer_RefreshNSE(t *testing.T) {
 	require.NoError(t, err)
 }
 
-func TestSetIDServer_InterDomainNSE(t *testing.T) {
-	server := setid.NewNetworkServiceEndpointRegistryServer()
-
-	reg, err := server.Register(context.Background(), testNSE(nseName+"@"+domain))
-	require.NoError(t, err)
-
-	require.True(t, strings.HasSuffix(reg.Name, "@"+domain))
-
-	_, err = server.Unregister(context.Background(), reg)
-	require.NoError(t, err)
-}
-
 func TestSetIDServer_RemoteRegistry(t *testing.T) {
 	server := next.NewNetworkServiceEndpointRegistryServer(
 		setid.NewNetworkServiceEndpointRegistryServer(),
 		new(remoteRegistry),
 	)
 
-	reg, err := server.Register(context.Background(), testNSE(nseName))
+	reg, err := server.Register(context.Background(), testNSE())
 	require.NoError(t, err)
 
 	name := reg.Name
@@ -115,8 +101,9 @@ type remoteRegistry struct {
 }
 
 func (s *remoteRegistry) Register(ctx context.Context, nse *registry.NetworkServiceEndpoint) (*registry.NetworkServiceEndpoint, error) {
-	if !strings.HasSuffix(nse.Name, remoteSuffix) {
-		nse.Name += remoteSuffix
+	nse = nse.Clone()
+	if !strings.HasPrefix(nse.Name, remotePrefix) {
+		nse.Name = remotePrefix + nse.Name
 	}
 	return next.NetworkServiceEndpointRegistryServer(ctx).Register(ctx, nse)
 }