[qfix] Fix metadata cleanup (#1038)

* Fix metadata cleanup Signed-off-by: Vladimir Popov <vladimir.popov@xored.com> * Suggested rename for variables Signed-off-by: Ed Warnicke <hagbard@gmail.com> Co-authored-by: Ed Warnicke <hagbard@gmail.com>
networkservicemesh · Jul 25, 2021 · 92b2824 · 92b2824
1 parent eb8d4f4
commit 92b2824
Show file tree

Hide file tree

Showing 3 changed files with 41 additions and 2 deletions.
diff --git a/pkg/networkservice/utils/metadata/client.go b/pkg/networkservice/utils/metadata/client.go
@@ -40,10 +40,13 @@ func NewClient() networkservice.NetworkServiceClient {
 
 func (m *metaDataClient) Request(ctx context.Context, request *networkservice.NetworkServiceRequest, opts ...grpc.CallOption) (*networkservice.Connection, error) {
 	connID := request.GetConnection().GetId()
+	_, isEstablished := m.Map.Load(connID)
 
 	conn, err := next.Client(ctx).Request(store(ctx, connID, &m.Map), request, opts...)
 	if err != nil {
-		del(ctx, connID, &m.Map)
+		if !isEstablished {
+			del(ctx, connID, &m.Map)
+		}
 		return nil, err
 	}
 

diff --git a/pkg/networkservice/utils/metadata/meta_data_test.go b/pkg/networkservice/utils/metadata/meta_data_test.go
@@ -112,6 +112,39 @@ var samples = []*sample{
 			require.NoError(t, err)
 		},
 	},
+	{
+		name: "Refresh failed",
+		test: func(t *testing.T, server networkservice.NetworkServiceServer, isClient bool) {
+			conn := &networkservice.Connection{
+				Id: "id",
+			}
+
+			chainServer := next.NewNetworkServiceServer(
+				server,
+				checkcontext.NewServer(t, func(_ *testing.T, ctx context.Context) {
+					metadata.Map(ctx, isClient).Store(testKey, 0)
+				}),
+				injecterror.NewServer(
+					injecterror.WithRequestErrorTimes(1),
+				),
+			)
+			conn, err := chainServer.Request(context.Background(), testRequest(conn))
+			require.NoError(t, err)
+
+			_, err = chainServer.Request(context.Background(), testRequest(conn))
+			require.Error(t, err)
+
+			chainServer = next.NewNetworkServiceServer(
+				server,
+				checkcontext.NewServer(t, func(t *testing.T, ctx context.Context) {
+					_, ok := metadata.Map(ctx, isClient).Load(testKey)
+					require.True(t, ok)
+				}),
+			)
+			_, err = chainServer.Request(context.Background(), testRequest(conn))
+			require.NoError(t, err)
+		},
+	},
 	{
 		name: "Close",
 		test: func(t *testing.T, server networkservice.NetworkServiceServer, isClient bool) {

diff --git a/pkg/networkservice/utils/metadata/server.go b/pkg/networkservice/utils/metadata/server.go
@@ -39,10 +39,13 @@ func NewServer() networkservice.NetworkServiceServer {
 
 func (m *metadataServer) Request(ctx context.Context, request *networkservice.NetworkServiceRequest) (*networkservice.Connection, error) {
 	connID := request.GetConnection().GetId()
+	_, isEstablished := m.Map.Load(connID)
 
 	conn, err := next.Server(ctx).Request(store(ctx, connID, &m.Map), request)
 	if err != nil {
-		del(ctx, connID, &m.Map)
+		if !isEstablished {
+			del(ctx, connID, &m.Map)
+		}
 		return nil, err
 	}