From 4080277fb84795a6b4b3c7241ce6ba7b2e6d5e55 Mon Sep 17 00:00:00 2001 From: Nikita Skrynnik Date: Fri, 29 Dec 2023 14:00:13 +1100 Subject: [PATCH 1/4] Add retry for routesClient + fix endless waiting in liveness check Signed-off-by: Nikita Skrynnik --- pkg/networkservice/connectioncontext/client.go | 5 ++++- .../connectioncontext/ipcontext/routes/client.go | 6 +++++- pkg/tools/heal/liveness_check.go | 1 + 3 files changed, 10 insertions(+), 2 deletions(-) diff --git a/pkg/networkservice/connectioncontext/client.go b/pkg/networkservice/connectioncontext/client.go index 8c61e78c..bdf263a3 100644 --- a/pkg/networkservice/connectioncontext/client.go +++ b/pkg/networkservice/connectioncontext/client.go @@ -20,6 +20,9 @@ package connectioncontext import ( + "time" + + "github.com/networkservicemesh/sdk/pkg/networkservice/common/retry" "github.com/networkservicemesh/sdk/pkg/networkservice/core/chain" "go.fd.io/govpp/api" @@ -55,7 +58,7 @@ import ( func NewClient(vppConn api.Connection) networkservice.NetworkServiceClient { return chain.NewNetworkServiceClient( mtu.NewClient(vppConn), - routes.NewClient(vppConn), + retry.NewClient(routes.NewClient(vppConn), retry.WithInterval(time.Millisecond*100), retry.WithTryTimeout(time.Second*2)), ipaddress.NewClient(vppConn), ) } diff --git a/pkg/networkservice/connectioncontext/ipcontext/routes/client.go b/pkg/networkservice/connectioncontext/ipcontext/routes/client.go index dc04aee9..45108494 100644 --- a/pkg/networkservice/connectioncontext/ipcontext/routes/client.go +++ b/pkg/networkservice/connectioncontext/ipcontext/routes/client.go @@ -23,6 +23,7 @@ import ( "github.com/pkg/errors" "go.fd.io/govpp/api" "google.golang.org/grpc" + "google.golang.org/protobuf/types/known/emptypb" "github.com/networkservicemesh/api/pkg/api/networkservice" "github.com/networkservicemesh/sdk/pkg/networkservice/core/next" @@ -83,6 +84,9 @@ func (r *routesClient) Request(ctx context.Context, request *networkservice.Netw } func (r *routesClient) Close(ctx context.Context, conn *networkservice.Connection, opts ...grpc.CallOption) (*empty.Empty, error) { - _ = addDel(ctx, conn, r.vppConn, metadata.IsClient(r), false) + err := addDel(ctx, conn, r.vppConn, metadata.IsClient(r), false) + if err != nil { + return &emptypb.Empty{}, err + } return next.Client(ctx).Close(ctx, conn, opts...) } diff --git a/pkg/tools/heal/liveness_check.go b/pkg/tools/heal/liveness_check.go index 87d0545a..cf8ea266 100644 --- a/pkg/tools/heal/liveness_check.go +++ b/pkg/tools/heal/liveness_check.go @@ -106,6 +106,7 @@ func doPing( for { select { case <-deadlineCtx.Done(): + responseCh <- deadlineCtx.Err() return case rawMsg := <-notifCh: if msg, ok := rawMsg.(*ping.PingFinishedEvent); ok { From ec2243d9c807db9bd795d1f16704abc8754dc669 Mon Sep 17 00:00:00 2001 From: Nikita Skrynnik Date: Fri, 29 Dec 2023 21:13:09 +1100 Subject: [PATCH 2/4] add more fixes to vpp liveness checker Signed-off-by: Nikita Skrynnik --- .../connectioncontext/client.go | 5 +- pkg/tools/heal/liveness_check.go | 53 ++++++++++--------- 2 files changed, 29 insertions(+), 29 deletions(-) diff --git a/pkg/networkservice/connectioncontext/client.go b/pkg/networkservice/connectioncontext/client.go index bdf263a3..8c61e78c 100644 --- a/pkg/networkservice/connectioncontext/client.go +++ b/pkg/networkservice/connectioncontext/client.go @@ -20,9 +20,6 @@ package connectioncontext import ( - "time" - - "github.com/networkservicemesh/sdk/pkg/networkservice/common/retry" "github.com/networkservicemesh/sdk/pkg/networkservice/core/chain" "go.fd.io/govpp/api" @@ -58,7 +55,7 @@ import ( func NewClient(vppConn api.Connection) networkservice.NetworkServiceClient { return chain.NewNetworkServiceClient( mtu.NewClient(vppConn), - retry.NewClient(routes.NewClient(vppConn), retry.WithInterval(time.Millisecond*100), retry.WithTryTimeout(time.Second*2)), + routes.NewClient(vppConn), ipaddress.NewClient(vppConn), ) } diff --git a/pkg/tools/heal/liveness_check.go b/pkg/tools/heal/liveness_check.go index cf8ea266..d0a022df 100644 --- a/pkg/tools/heal/liveness_check.go +++ b/pkg/tools/heal/liveness_check.go @@ -36,20 +36,25 @@ const ( intervalFactor = 0.85 ) -func waitForResponses(responseCh <-chan error) bool { +func waitForResponses(ctx context.Context, responseCh <-chan error) bool { respCount := cap(responseCh) success := true for { - resp, ok := <-responseCh - if !ok { + select { + case <-ctx.Done(): return false - } - if resp != nil { - success = false - } - respCount-- - if respCount == 0 { - return success + case resp, ok := <-responseCh: + if !ok { + return false + } + if resp != nil { + success = false + } + respCount-- + if respCount == 0 { + return success + } + } } } @@ -103,23 +108,21 @@ func doPing( } defer func() { _ = subscription.Unsubscribe() }() - for { - select { - case <-deadlineCtx.Done(): - responseCh <- deadlineCtx.Err() - return - case rawMsg := <-notifCh: - if msg, ok := rawMsg.(*ping.PingFinishedEvent); ok { - if msg.ReplyCount == 0 { - err = errors.New("No packets received") - logger.Errorf(err.Error()) - responseCh <- err - return - } - responseCh <- nil + select { + case <-deadlineCtx.Done(): + return + case rawMsg := <-notifCh: + if msg, ok := rawMsg.(*ping.PingFinishedEvent); ok { + if msg.ReplyCount == 0 { + err = errors.New("No packets received") + logger.Errorf(err.Error()) + responseCh <- err + return } + responseCh <- nil } } + } // VPPLivenessCheck return a liveness check function which uses VPP ping to check VPP dataplane @@ -170,6 +173,6 @@ func VPPLivenessCheck(vppConn vpphelper.Connection) func(deadlineCtx context.Con } // Waiting for all ping results. If at least one fails - return false - return waitForResponses(responseCh) + return waitForResponses(deadlineCtx, responseCh) } } From d8bfbf5e386cc9cceb203f34308c64364cc10742 Mon Sep 17 00:00:00 2001 From: Nikita Skrynnik Date: Fri, 29 Dec 2023 21:14:37 +1100 Subject: [PATCH 3/4] cleanup Signed-off-by: Nikita Skrynnik --- .../connectioncontext/ipcontext/routes/client.go | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/pkg/networkservice/connectioncontext/ipcontext/routes/client.go b/pkg/networkservice/connectioncontext/ipcontext/routes/client.go index 45108494..dc04aee9 100644 --- a/pkg/networkservice/connectioncontext/ipcontext/routes/client.go +++ b/pkg/networkservice/connectioncontext/ipcontext/routes/client.go @@ -23,7 +23,6 @@ import ( "github.com/pkg/errors" "go.fd.io/govpp/api" "google.golang.org/grpc" - "google.golang.org/protobuf/types/known/emptypb" "github.com/networkservicemesh/api/pkg/api/networkservice" "github.com/networkservicemesh/sdk/pkg/networkservice/core/next" @@ -84,9 +83,6 @@ func (r *routesClient) Request(ctx context.Context, request *networkservice.Netw } func (r *routesClient) Close(ctx context.Context, conn *networkservice.Connection, opts ...grpc.CallOption) (*empty.Empty, error) { - err := addDel(ctx, conn, r.vppConn, metadata.IsClient(r), false) - if err != nil { - return &emptypb.Empty{}, err - } + _ = addDel(ctx, conn, r.vppConn, metadata.IsClient(r), false) return next.Client(ctx).Close(ctx, conn, opts...) } From 76ca9c050a1671b15b0e071366eebbf077c67c0c Mon Sep 17 00:00:00 2001 From: Nikita Skrynnik Date: Fri, 29 Dec 2023 21:16:52 +1100 Subject: [PATCH 4/4] fix golang linter Signed-off-by: Nikita Skrynnik --- pkg/tools/heal/liveness_check.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/pkg/tools/heal/liveness_check.go b/pkg/tools/heal/liveness_check.go index d0a022df..86ca5171 100644 --- a/pkg/tools/heal/liveness_check.go +++ b/pkg/tools/heal/liveness_check.go @@ -54,7 +54,6 @@ func waitForResponses(ctx context.Context, responseCh <-chan error) bool { if respCount == 0 { return success } - } } } @@ -122,7 +121,6 @@ func doPing( responseCh <- nil } } - } // VPPLivenessCheck return a liveness check function which uses VPP ping to check VPP dataplane