mirror of
https://github.com/netbirdio/netbird.git
synced 2026-04-15 23:06:38 +00:00
* [client] Fix flow client Receive retry loop not stopping after Close Use backoff.Permanent for canceled gRPC errors so Receive returns immediately instead of retrying until context deadline when the connection is already closed. Add TestNewClient_PermanentClose to verify the behavior. The connectivity.Shutdown check was meaningless because when the connection is shut down, c.realClient.Events(ctx, grpc.WaitForReady(true)) on the nex line already fails with codes.Canceled — which is now handled as a permanent error. The explicit state check was just duplicating what gRPC already reports through its normal error path. * [client] remove WaitForReady from stream open call grpc.WaitForReady(true) parks the RPC call internally until the connection reaches READY, only unblocking on ctx cancellation. This means the external backoff.Retry loop in Receive() never gets control back during a connection outage — it cannot tick, log, or apply its retry intervals while WaitForReady is blocking. Removing it restores fail-fast behaviour: Events() returns immediately with codes.Unavailable when the connection is not ready, which is exactly what the backoff loop expects. The backoff becomes the single authority over retry timing and cadence, as originally intended. * [client] Add connection recreation and improve flow client error handling Store gRPC dial options on the client to enable connection recreation on Internal errors (RST_STREAM/PROTOCOL_ERROR). Treat Unauthenticated, PermissionDenied, and Unimplemented as permanent failures. Unify mutex usage and add reconnection logging for better observability. * [client] Remove Unauthenticated, PermissionDenied, and Unimplemented from permanent error handling * [client] Fix error handling in Receive to properly re-establish stream and improve reconnection messaging * Fix test * [client] Add graceful shutdown handling and test for concurrent Close during Receive Prevent reconnection attempts after client closure by tracking a `closed` flag. Use `backoff.Permanent` for errors caused by operations on a closed client. Add a test to ensure `Close` does not block when `Receive` is actively running. * [client] Fix connection swap to properly close old gRPC connection Close the old `gRPC.ClientConn` after successfully swapping to a new connection during reconnection. * [client] Reset backoff * [client] Ensure stream closure on error during initialization * [client] Add test for handling server-side stream closure and reconnection Introduce `TestReceive_ServerClosesStream` to verify the client's ability to recover and process acknowledgments after the server closes the stream. Enhance test server with a controlled stream closure mechanism. * [client] Add protocol error simulation and enhance reconnection test Introduce `connTrackListener` to simulate HTTP/2 RST_STREAM with PROTOCOL_ERROR for testing. Refactor and rename `TestReceive_ServerClosesStream` to `TestReceive_ProtocolErrorStreamReconnect` to verify client recovery on protocol errors. * [client] Update Close error message in test for clarity * [client] Fine-tune the tests * [client] Adjust connection tracking in reconnection test * [client] Wait for Events handler to exit in RST_STREAM reconnection test Ensure the old `Events` handler exits fully before proceeding in the reconnection test to avoid dropped acknowledgments on a broken stream. Add a `handlerDone` channel to synchronize handler exits. * [client] Prevent panic on nil connection during Close * [client] Refactor connection handling to use explicit target tracking Introduce `target` field to store the gRPC connection target directly, simplifying reconnections and ensuring consistent connection reuse logic. * [client] Rename `isCancellation` to `isContextDone` and extend handling for `DeadlineExceeded` Refactor error handling to include `DeadlineExceeded` scenarios alongside `Canceled`. Update related condition checks for consistency. * [client] Add connection generation tracking to prevent stale reconnections Introduce `connGen` to track connection generations and ensure that stale `recreateConnection` calls do not override newer connections. Update stream establishment and reconnection logic to incorporate generation validation. * [client] Add backoff reset condition to prevent short-lived retry cycles Refine backoff reset logic to ensure it only occurs for sufficiently long-lived stream connections, avoiding interference with `MaxElapsedTime`. * [client] Introduce `minHealthyDuration` to refine backoff reset logic Add `minHealthyDuration` constant to ensure stream retries only reset the backoff timer if the stream survives beyond a minimum duration. Prevents unhealthy, short-lived streams from interfering with `MaxElapsedTime`. * [client] IPv6 friendly connection parsedURL.Hostname() strips IPv6 brackets. For http://[::1]:443, this turns it into ::1:443, which is not a valid host:port target for gRPC. Additionally, fmt.Sprintf("%s:%s", hostname, port) produces a trailing colon when the URL has no explicit port—http://example.com becomes example.com:. Both cases break the initial dial and reconnect paths. Use parsedURL.Host directly instead. * [client] Add `handlerStarted` channel to synchronize stream establishment in tests Introduce `handlerStarted` channel in the test server to signal when the server-side handler begins, ensuring robust synchronization between client and server during stream establishment. Update relevant test cases to wait for this signal before proceeding. * [client] Replace `receivedAcks` map with atomic counter and improve stream establishment sync in tests Refactor acknowledgment tracking in tests to use an `atomic.Int32` counter instead of a map. Replace fixed sleep with robust synchronization by waiting on `handlerStarted` signal for stream establishment. * [client] Extract `handleReceiveError` to simplify receive logic Refactor error handling in `receive` to a dedicated `handleReceiveError` method. Streamlines the main logic and isolates error recovery, including backoff reset and connection recreation. * [client] recreate gRPC ClientConn on every retry to prevent dual backoff The flow client had two competing retry loops: our custom exponential backoff and gRPC's internal subchannel reconnection. When establishStream failed, the same ClientConn was reused, allowing gRPC's internal backoff state to accumulate and control dial timing independently. Changes: - Consolidate error handling into handleRetryableError, which now handles context cancellation, permanent errors, backoff reset, and connection recreation in a single path - Call recreateConnection on every retryable error so each retry gets a fresh ClientConn with no internal backoff state - Remove connGen tracking since Receive is sequential and protected by a new receiving guard against concurrent calls - Reduce RandomizationFactor from 1 to 0.5 to avoid near-zero backoff intervals
313 lines
8.0 KiB
Go
313 lines
8.0 KiB
Go
package client
|
|
|
|
import (
|
|
"context"
|
|
"crypto/tls"
|
|
"crypto/x509"
|
|
"errors"
|
|
"fmt"
|
|
"net/url"
|
|
"sync"
|
|
"time"
|
|
|
|
"github.com/cenkalti/backoff/v4"
|
|
log "github.com/sirupsen/logrus"
|
|
"google.golang.org/grpc"
|
|
"google.golang.org/grpc/codes"
|
|
"google.golang.org/grpc/credentials"
|
|
"google.golang.org/grpc/credentials/insecure"
|
|
"google.golang.org/grpc/keepalive"
|
|
"google.golang.org/grpc/status"
|
|
|
|
nbgrpc "github.com/netbirdio/netbird/client/grpc"
|
|
"github.com/netbirdio/netbird/flow/proto"
|
|
"github.com/netbirdio/netbird/util/embeddedroots"
|
|
"github.com/netbirdio/netbird/util/wsproxy"
|
|
)
|
|
|
|
var ErrClientClosed = errors.New("client is closed")
|
|
|
|
// minHealthyDuration is the minimum time a stream must survive before a failure
|
|
// resets the backoff timer. Streams that fail faster are considered unhealthy and
|
|
// should not reset backoff, so that MaxElapsedTime can eventually stop retries.
|
|
const minHealthyDuration = 5 * time.Second
|
|
|
|
type GRPCClient struct {
|
|
realClient proto.FlowServiceClient
|
|
clientConn *grpc.ClientConn
|
|
stream proto.FlowService_EventsClient
|
|
target string
|
|
opts []grpc.DialOption
|
|
closed bool // prevent creating conn in the middle of the Close
|
|
receiving bool // prevent concurrent Receive calls
|
|
mu sync.Mutex // protects clientConn, realClient, stream, closed, and receiving
|
|
}
|
|
|
|
func NewClient(addr, payload, signature string, interval time.Duration) (*GRPCClient, error) {
|
|
parsedURL, err := url.Parse(addr)
|
|
if err != nil {
|
|
return nil, fmt.Errorf("parsing url: %w", err)
|
|
}
|
|
var opts []grpc.DialOption
|
|
tlsEnabled := parsedURL.Scheme == "https"
|
|
if tlsEnabled {
|
|
certPool, err := x509.SystemCertPool()
|
|
if err != nil || certPool == nil {
|
|
log.Debugf("System cert pool not available; falling back to embedded cert, error: %v", err)
|
|
certPool = embeddedroots.Get()
|
|
}
|
|
|
|
opts = append(opts, grpc.WithTransportCredentials(credentials.NewTLS(&tls.Config{
|
|
RootCAs: certPool,
|
|
})))
|
|
} else {
|
|
opts = append(opts, grpc.WithTransportCredentials(insecure.NewCredentials()))
|
|
}
|
|
|
|
opts = append(opts,
|
|
nbgrpc.WithCustomDialer(tlsEnabled, wsproxy.FlowComponent),
|
|
grpc.WithIdleTimeout(interval*2),
|
|
grpc.WithKeepaliveParams(keepalive.ClientParameters{
|
|
Time: 30 * time.Second,
|
|
Timeout: 10 * time.Second,
|
|
}),
|
|
withAuthToken(payload, signature),
|
|
grpc.WithDefaultServiceConfig(`{"healthCheckConfig": {"serviceName": ""}}`),
|
|
)
|
|
|
|
target := parsedURL.Host
|
|
conn, err := grpc.NewClient(target, opts...)
|
|
if err != nil {
|
|
return nil, fmt.Errorf("creating new grpc client: %w", err)
|
|
}
|
|
|
|
return &GRPCClient{
|
|
realClient: proto.NewFlowServiceClient(conn),
|
|
clientConn: conn,
|
|
target: target,
|
|
opts: opts,
|
|
}, nil
|
|
}
|
|
|
|
func (c *GRPCClient) Close() error {
|
|
c.mu.Lock()
|
|
c.closed = true
|
|
c.stream = nil
|
|
conn := c.clientConn
|
|
c.clientConn = nil
|
|
c.mu.Unlock()
|
|
|
|
if conn == nil {
|
|
return nil
|
|
}
|
|
|
|
if err := conn.Close(); err != nil && !errors.Is(err, context.Canceled) {
|
|
return fmt.Errorf("close client connection: %w", err)
|
|
}
|
|
|
|
return nil
|
|
}
|
|
|
|
func (c *GRPCClient) Send(event *proto.FlowEvent) error {
|
|
c.mu.Lock()
|
|
stream := c.stream
|
|
c.mu.Unlock()
|
|
|
|
if stream == nil {
|
|
return errors.New("stream not initialized")
|
|
}
|
|
|
|
if err := stream.Send(event); err != nil {
|
|
return fmt.Errorf("send flow event: %w", err)
|
|
}
|
|
|
|
return nil
|
|
}
|
|
|
|
func (c *GRPCClient) Receive(ctx context.Context, interval time.Duration, msgHandler func(msg *proto.FlowEventAck) error) error {
|
|
c.mu.Lock()
|
|
if c.receiving {
|
|
c.mu.Unlock()
|
|
return errors.New("concurrent Receive calls are not supported")
|
|
}
|
|
c.receiving = true
|
|
c.mu.Unlock()
|
|
defer func() {
|
|
c.mu.Lock()
|
|
c.receiving = false
|
|
c.mu.Unlock()
|
|
}()
|
|
|
|
backOff := defaultBackoff(ctx, interval)
|
|
operation := func() error {
|
|
stream, err := c.establishStream(ctx)
|
|
if err != nil {
|
|
log.Errorf("failed to establish flow stream, retrying: %v", err)
|
|
return c.handleRetryableError(err, time.Time{}, backOff)
|
|
}
|
|
|
|
streamStart := time.Now()
|
|
|
|
if err := c.receive(stream, msgHandler); err != nil {
|
|
log.Errorf("receive failed: %v", err)
|
|
return c.handleRetryableError(err, streamStart, backOff)
|
|
}
|
|
return nil
|
|
}
|
|
|
|
if err := backoff.Retry(operation, backOff); err != nil {
|
|
return fmt.Errorf("receive failed permanently: %w", err)
|
|
}
|
|
|
|
return nil
|
|
}
|
|
|
|
// handleRetryableError resets the backoff timer if the stream was healthy long
|
|
// enough and recreates the underlying ClientConn so that gRPC's internal
|
|
// subchannel backoff does not accumulate and compete with our own retry timer.
|
|
// A zero streamStart means the stream was never established.
|
|
func (c *GRPCClient) handleRetryableError(err error, streamStart time.Time, backOff backoff.BackOff) error {
|
|
if isContextDone(err) {
|
|
return backoff.Permanent(err)
|
|
}
|
|
|
|
var permErr *backoff.PermanentError
|
|
if errors.As(err, &permErr) {
|
|
return err
|
|
}
|
|
|
|
// Reset the backoff so the next retry starts with a short delay instead of
|
|
// continuing the already-elapsed timer. Only do this if the stream was healthy
|
|
// long enough; short-lived connect/drop cycles must not defeat MaxElapsedTime.
|
|
if !streamStart.IsZero() && time.Since(streamStart) >= minHealthyDuration {
|
|
backOff.Reset()
|
|
}
|
|
|
|
if recreateErr := c.recreateConnection(); recreateErr != nil {
|
|
log.Errorf("recreate connection: %v", recreateErr)
|
|
return recreateErr
|
|
}
|
|
|
|
log.Infof("connection recreated, retrying stream")
|
|
return fmt.Errorf("retrying after error: %w", err)
|
|
}
|
|
|
|
func (c *GRPCClient) recreateConnection() error {
|
|
c.mu.Lock()
|
|
if c.closed {
|
|
c.mu.Unlock()
|
|
return backoff.Permanent(ErrClientClosed)
|
|
}
|
|
|
|
conn, err := grpc.NewClient(c.target, c.opts...)
|
|
if err != nil {
|
|
c.mu.Unlock()
|
|
return fmt.Errorf("create new connection: %w", err)
|
|
}
|
|
|
|
old := c.clientConn
|
|
c.clientConn = conn
|
|
c.realClient = proto.NewFlowServiceClient(conn)
|
|
c.stream = nil
|
|
c.mu.Unlock()
|
|
|
|
_ = old.Close()
|
|
|
|
return nil
|
|
}
|
|
|
|
func (c *GRPCClient) establishStream(ctx context.Context) (proto.FlowService_EventsClient, error) {
|
|
c.mu.Lock()
|
|
if c.closed {
|
|
c.mu.Unlock()
|
|
return nil, backoff.Permanent(ErrClientClosed)
|
|
}
|
|
cl := c.realClient
|
|
c.mu.Unlock()
|
|
|
|
// open stream outside the lock — blocking operation
|
|
stream, err := cl.Events(ctx)
|
|
if err != nil {
|
|
return nil, fmt.Errorf("create event stream: %w", err)
|
|
}
|
|
streamReady := false
|
|
defer func() {
|
|
if !streamReady {
|
|
_ = stream.CloseSend()
|
|
}
|
|
}()
|
|
|
|
if err = stream.Send(&proto.FlowEvent{IsInitiator: true}); err != nil {
|
|
return nil, fmt.Errorf("send initiator: %w", err)
|
|
}
|
|
|
|
if err = checkHeader(stream); err != nil {
|
|
return nil, fmt.Errorf("check header: %w", err)
|
|
}
|
|
|
|
c.mu.Lock()
|
|
if c.closed {
|
|
c.mu.Unlock()
|
|
return nil, backoff.Permanent(ErrClientClosed)
|
|
}
|
|
c.stream = stream
|
|
c.mu.Unlock()
|
|
streamReady = true
|
|
|
|
return stream, nil
|
|
}
|
|
|
|
func (c *GRPCClient) receive(stream proto.FlowService_EventsClient, msgHandler func(msg *proto.FlowEventAck) error) error {
|
|
for {
|
|
msg, err := stream.Recv()
|
|
if err != nil {
|
|
return err
|
|
}
|
|
|
|
if msg.IsInitiator {
|
|
log.Tracef("received initiator message from flow receiver")
|
|
continue
|
|
}
|
|
|
|
if err := msgHandler(msg); err != nil {
|
|
return fmt.Errorf("handle message: %w", err)
|
|
}
|
|
}
|
|
}
|
|
|
|
func checkHeader(stream proto.FlowService_EventsClient) error {
|
|
header, err := stream.Header()
|
|
if err != nil {
|
|
log.Errorf("waiting for flow receiver header: %s", err)
|
|
return fmt.Errorf("wait for header: %w", err)
|
|
}
|
|
|
|
if len(header) == 0 {
|
|
log.Error("flow receiver sent no headers")
|
|
return fmt.Errorf("should have headers")
|
|
}
|
|
return nil
|
|
}
|
|
|
|
func defaultBackoff(ctx context.Context, interval time.Duration) backoff.BackOff {
|
|
return backoff.WithContext(&backoff.ExponentialBackOff{
|
|
InitialInterval: 800 * time.Millisecond,
|
|
RandomizationFactor: 0.5,
|
|
Multiplier: 1.7,
|
|
MaxInterval: interval / 2,
|
|
MaxElapsedTime: 3 * 30 * 24 * time.Hour, // 3 months
|
|
Stop: backoff.Stop,
|
|
Clock: backoff.SystemClock,
|
|
}, ctx)
|
|
}
|
|
|
|
func isContextDone(err error) bool {
|
|
if errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) {
|
|
return true
|
|
}
|
|
if s, ok := status.FromError(err); ok {
|
|
return s.Code() == codes.Canceled || s.Code() == codes.DeadlineExceeded
|
|
}
|
|
return false
|
|
}
|