fix: refresh should use remote server expiration time for nse

pkg/registry/common/refresh/nse_registry_client.go

@@ -68,6 +68,7 @@ func (c *refreshNSEClient) Register(ctx context.Context, in *registry.NetworkSer
 			Nanos:   int32(expirationTime.Nanosecond()),
 		}
 	}
+	nse := in.Clone()
 	nextClient := next.NetworkServiceEndpointRegistryClient(ctx)
 	resp, err := nextClient.Register(ctx, in, opts...)
 	if err != nil {
@@ -78,7 +79,8 @@ func (c *refreshNSEClient) Register(ctx context.Context, in *registry.NetworkSer
 	}
 	ctx, cancel := context.WithCancel(c.chainContext)
 	c.nseCancels.Store(resp.Name, cancel)
-	c.startRefresh(ctx, nextClient, resp)
+	nse.ExpirationTime = resp.ExpirationTime
+	c.startRefresh(ctx, nextClient, nse)
 	return resp, err
 }
 

pkg/registry/common/refresh/nse_registry_client_test.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
 //
@@ -22,13 +22,15 @@ import (
 	"testing"
 	"time"
 
+	"github.com/networkservicemesh/sdk/pkg/registry/core/adapters"
 	"github.com/networkservicemesh/sdk/pkg/registry/core/next"
 
 	"github.com/golang/protobuf/ptypes/empty"
 	"github.com/networkservicemesh/api/pkg/api/registry"
 	"github.com/stretchr/testify/require"
 	"go.uber.org/goleak"
 	"google.golang.org/grpc"
+	"google.golang.org/protobuf/types/known/timestamppb"
 
 	"github.com/networkservicemesh/sdk/pkg/registry/common/refresh"
 )
@@ -44,7 +46,7 @@ func (t *testNSEClient) Register(ctx context.Context, in *registry.NetworkServic
 	t.Lock()
 	defer t.Unlock()
 	t.requestCount++
-	return in, nil
+	return next.NetworkServiceEndpointRegistryClient(ctx).Register(ctx, in, opts...)
 }
 
 func (t *testNSEClient) Find(ctx context.Context, in *registry.NetworkServiceEndpointQuery, opts ...grpc.CallOption) (registry.NetworkServiceEndpointRegistry_FindClient, error) {
@@ -70,33 +72,62 @@ func (c *checkExpirationTimeClient) Unregister(ctx context.Context, in *registry
 	return new(empty.Empty), nil
 }
 
+type nseExpirationServer struct {
+	expirationTime time.Duration
+}
+
+func (s *nseExpirationServer) Register(_ context.Context, in *registry.NetworkServiceEndpoint) (*registry.NetworkServiceEndpoint, error) {
+	resp := in.Clone()
+	resp.ExpirationTime = timestamppb.New(time.Now().Add(s.expirationTime))
+	return resp, nil
+}
+
+func (s *nseExpirationServer) Find(_ *registry.NetworkServiceEndpointQuery, _ registry.NetworkServiceEndpointRegistry_FindServer) error {
+	return nil
+}
+
+func (s *nseExpirationServer) Unregister(_ context.Context, _ *registry.NetworkServiceEndpoint) (*empty.Empty, error) {
+	return nil, nil
+}
+
 func TestNewNetworkServiceEndpointRegistryClient(t *testing.T) {
 	defer goleak.VerifyNone(t, goleak.IgnoreCurrent())
 	testClient := &testNSEClient{}
 
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+
 	refreshClient := next.NewNetworkServiceEndpointRegistryClient(
 		refresh.NewNetworkServiceEndpointRegistryClient(
 			refresh.WithRetryPeriod(testExpiryDuration),
-			refresh.WithDefaultExpiryDuration(testExpiryDuration)),
+			refresh.WithDefaultExpiryDuration(testExpiryDuration),
+			refresh.WithChainContext(ctx),
+		),
 		testClient)
 	_, err := refreshClient.Register(context.Background(), &registry.NetworkServiceEndpoint{
 		Name: "nse-1",
 	})
-	require.Nil(t, err)
+
+	require.NoError(t, err)
 	require.Eventually(t, func() bool {
 		testClient.Lock()
 		defer testClient.Unlock()
 		return testClient.requestCount > 0
 	}, time.Second, testExpiryDuration/4)
+
 	_, err = refreshClient.Unregister(context.Background(), &registry.NetworkServiceEndpoint{Name: "nse-1"})
-	require.Nil(t, err)
+	require.NoError(t, err)
 }
 
 func TestRefreshNSEClient_ShouldSetExpirationTime_BeforeCallNext(t *testing.T) {
 	defer goleak.VerifyNone(t, goleak.IgnoreCurrent())
+
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+
 	c := next.NewNetworkServiceEndpointRegistryClient(
 		&testNSEClient{},
-		refresh.NewNetworkServiceEndpointRegistryClient(refresh.WithDefaultExpiryDuration(time.Hour)),
+		refresh.NewNetworkServiceEndpointRegistryClient(refresh.WithDefaultExpiryDuration(time.Hour), refresh.WithChainContext(ctx)),
 		&checkExpirationTimeClient{T: t},
 	)
 
@@ -107,27 +138,51 @@ func TestRefreshNSEClient_ShouldSetExpirationTime_BeforeCallNext(t *testing.T) {
 	require.Nil(t, err)
 }
 
-func TestNewNetworkServiceEndpointRegistryClient_CalledRegisterTwice(t *testing.T) {
+func Test_RefreshNSEClient_ShouldUseExpirationFromServer(t *testing.T) {
+	c := next.NewNetworkServiceEndpointRegistryClient(
+		refresh.NewNetworkServiceEndpointRegistryClient(refresh.WithDefaultExpiryDuration(time.Hour)),
+		adapters.NetworkServiceEndpointServerToClient(&nseExpirationServer{expirationTime: time.Hour}),
+	)
+
+	resp, err := c.Register(context.Background(), &registry.NetworkServiceEndpoint{Name: "nse-1"})
+
+	require.NoError(t, err)
+	require.Greater(t, time.Until(resp.ExpirationTime.AsTime()).Minutes(), float64(50))
+}
+
+func Test_RefreshNSEClient_CalledRegisterTwice(t *testing.T) {
 	defer goleak.VerifyNone(t, goleak.IgnoreCurrent())
+
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+
 	testClient := &testNSEClient{}
+
 	refreshClient := next.NewNetworkServiceEndpointRegistryClient(
 		refresh.NewNetworkServiceEndpointRegistryClient(
 			refresh.WithRetryPeriod(testExpiryDuration),
-			refresh.WithDefaultExpiryDuration(testExpiryDuration)),
+			refresh.WithDefaultExpiryDuration(testExpiryDuration),
+			refresh.WithChainContext(ctx)),
 		testClient)
+
 	_, err := refreshClient.Register(context.Background(), &registry.NetworkServiceEndpoint{
 		Name: "nse-1",
 	})
+
 	require.Nil(t, err)
 	_, err = refreshClient.Register(context.Background(), &registry.NetworkServiceEndpoint{
 		Name: "nse-1",
 	})
+
 	require.Nil(t, err)
+
 	require.Eventually(t, func() bool {
 		testClient.Lock()
 		defer testClient.Unlock()
 		return testClient.requestCount > 0
 	}, time.Second, testExpiryDuration/4)
+
 	_, err = refreshClient.Unregister(context.Background(), &registry.NetworkServiceEndpoint{Name: "nse-1"})
+
 	require.Nil(t, err)
 }