feat: Add support for point-to-multipoint forwarders in NSM #1122
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -119,7 +119,8 @@ func Test_ShouldCorrectlyAddForwardersWithSameNames(t *testing.T) { | |
| require.NoError(t, err) | ||
|
|
||
| forwarderReg := ®istry.NetworkServiceEndpoint{ | ||
| Name: "forwarder", | ||
| NetworkServiceNames: []string{"forwarder"}, | ||
|
There was a problem hiding this comment.
There was a problem hiding this comment.
I don't quite follow this, could you say more? There was a problem hiding this comment. Sure, we can add something like: type setForwarderNetworkServiceNameNSEClient struct{}
func (n *setNetworkServiceNamesNSEClient) Register(ctx context.Context, in *registry.NetworkServiceEndpoint, opts ...grpc.CallOption) (*registry.NetworkServiceEndpoint, error) {
in.NetworkServiceNames = []string{"forwarder"}
...This client can be used by forwarders on registration. There was a problem hiding this comment. Ah... that's interesting... but how does it handle the case of that being overriden for some reason by env variables in the forwarder? Better question... where is the 'forwarder' string encoded in the NSMgr ? There was a problem hiding this comment.
See at discovercrossnse chain element. There was a problem hiding this comment. if you are fine with this, then I'll add the option as you suggested. There was a problem hiding this comment. Forwarders should already be able to overload the NS name via ENV variable... do they not? There was a problem hiding this comment. Yes, but all forwarders and nsmgrs on the cluster should use the same network service name. So constant in api can simplify it There was a problem hiding this comment. Mostly, I like your suggestion, but just shared another possible way |
||
| } | ||
|
|
||
| // 1. Add forwarders | ||
|
|
@@ -253,3 +254,78 @@ func Test_ShouldParseNetworkServiceLabelsTemplate(t *testing.T) { | |
| _, err = nse.Unregister(ctx, nseReg) | ||
| require.NoError(t, err) | ||
| } | ||
|
|
||
| func Test_UsecasePoint2MultiPoint(t *testing.T) { | ||
|
There was a problem hiding this comment. @edwarnicke Is this test look valid? |
||
| t.Cleanup(func() { goleak.VerifyNone(t) }) | ||
|
|
||
| ctx, cancel := context.WithTimeout(context.Background(), time.Second*5) | ||
| defer cancel() | ||
|
|
||
| domain := sandbox.NewBuilder(ctx, t). | ||
| SetNodesCount(1). | ||
| SetRegistryProxySupplier(nil). | ||
| SetNodeSetup(func(ctx context.Context, node *sandbox.Node, _ int) { | ||
| node.NewNSMgr(ctx, "nsmgr", nil, sandbox.GenerateTestToken, nsmgr.NewServer) | ||
| }). | ||
| SetRegistryExpiryDuration(sandbox.RegistryExpiryDuration). | ||
| Build() | ||
|
|
||
| domain.Nodes[0].NewForwarder(ctx, ®istry.NetworkServiceEndpoint{ | ||
| Name: "p2mp forwarder", | ||
| NetworkServiceNames: []string{"forwarder"}, | ||
| NetworkServiceLabels: map[string]*registry.NetworkServiceLabels{ | ||
| "forwarder": { | ||
| Labels: map[string]string{ | ||
| "p2mp": "true", | ||
| }, | ||
| }, | ||
| }, | ||
| }, sandbox.GenerateTestToken) | ||
|
|
||
| domain.Nodes[0].NewForwarder(ctx, ®istry.NetworkServiceEndpoint{ | ||
| Name: "p2p forwarder", | ||
| NetworkServiceNames: []string{"forwarder"}, | ||
| NetworkServiceLabels: map[string]*registry.NetworkServiceLabels{ | ||
| "forwarder": { | ||
| Labels: map[string]string{ | ||
| "p2p": "true", | ||
| }, | ||
| }, | ||
| }, | ||
| }, sandbox.GenerateTestToken) | ||
|
|
||
| domain.Nodes[0].NewForwarder(ctx, ®istry.NetworkServiceEndpoint{ | ||
| Name: "special forwarder", | ||
| NetworkServiceNames: []string{"forwarder"}, | ||
| NetworkServiceLabels: map[string]*registry.NetworkServiceLabels{ | ||
| "forwarder": { | ||
| Labels: map[string]string{ | ||
| "special": "true", | ||
| }, | ||
| }, | ||
| }, | ||
| }, sandbox.GenerateTestToken) | ||
|
|
||
| nsRegistryClient := domain.NewNSRegistryClient(ctx, sandbox.GenerateTestToken) | ||
|
|
||
| nsReg, err := nsRegistryClient.Register(ctx, defaultRegistryService()) | ||
| require.NoError(t, err) | ||
|
|
||
| nseReg := defaultRegistryEndpoint(nsReg.Name) | ||
|
|
||
| domain.Nodes[0].NewEndpoint(ctx, nseReg, sandbox.GenerateTestToken) | ||
|
|
||
| nsc := domain.Nodes[0].NewClient(ctx, sandbox.GenerateTestToken) | ||
|
|
||
| request := defaultRequest(nsReg.Name) | ||
|
|
||
| request.GetConnection().Labels = map[string]string{ | ||
| "p2mp": "true", | ||
| } | ||
|
|
||
| conn, err := nsc.Request(ctx, request.Clone()) | ||
| require.NoError(t, err) | ||
| require.NotNil(t, conn) | ||
| require.Equal(t, 4, len(conn.Path.PathSegments)) | ||
| require.Equal(t, "p2mp forwarder", conn.GetPath().GetPathSegments()[2].Name) | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: now nsmgr has its own memory registry cache by default.