TUN-6617: Dont fallback to http2 if QUIC conn was successful.
cloudflared falls back aggressively to HTTP/2 protocol if a connection attempt with QUIC failed. This was done to ensure that machines with UDP egress disabled did not stop clients from connecting to the cloudlfare edge. This PR improves on that experience by having cloudflared remember if a QUIC connection was successful which implies UDP egress works. In this case, cloudflared does not fallback to HTTP/2 and keeps trying to connect to the edge with QUIC.
This commit is contained in:
parent
278df5478a
commit
99f39225f1
|
@ -2,6 +2,7 @@ package connection
|
|||
|
||||
import (
|
||||
"context"
|
||||
"fmt"
|
||||
"io"
|
||||
"net"
|
||||
"time"
|
||||
|
@ -21,6 +22,7 @@ type controlStream struct {
|
|||
namedTunnelProperties *NamedTunnelProperties
|
||||
connIndex uint8
|
||||
edgeAddress net.IP
|
||||
protocol Protocol
|
||||
|
||||
newRPCClientFunc RPCClientFunc
|
||||
|
||||
|
@ -51,6 +53,7 @@ func NewControlStream(
|
|||
newRPCClientFunc RPCClientFunc,
|
||||
gracefulShutdownC <-chan struct{},
|
||||
gracePeriod time.Duration,
|
||||
protocol Protocol,
|
||||
) ControlStreamHandler {
|
||||
if newRPCClientFunc == nil {
|
||||
newRPCClientFunc = newRegistrationRPCClient
|
||||
|
@ -64,6 +67,7 @@ func NewControlStream(
|
|||
edgeAddress: edgeAddress,
|
||||
gracefulShutdownC: gracefulShutdownC,
|
||||
gracePeriod: gracePeriod,
|
||||
protocol: protocol,
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -80,6 +84,9 @@ func (c *controlStream) ServeControlStream(
|
|||
rpcClient.Close()
|
||||
return err
|
||||
}
|
||||
|
||||
c.observer.logServerInfo(c.connIndex, registrationDetails.Location, c.edgeAddress, fmt.Sprintf("Connection %s registered", registrationDetails.UUID))
|
||||
c.observer.sendConnectedEvent(c.connIndex, c.protocol, registrationDetails.Location)
|
||||
c.connectedFuse.Connected()
|
||||
|
||||
// if conn index is 0 and tunnel is not remotely managed, then send local ingress rules configuration
|
||||
|
|
|
@ -5,6 +5,7 @@ type Event struct {
|
|||
Index uint8
|
||||
EventType Status
|
||||
Location string
|
||||
Protocol Protocol
|
||||
URL string
|
||||
}
|
||||
|
||||
|
|
|
@ -43,6 +43,7 @@ func newTestHTTP2Connection() (*HTTP2Connection, net.Conn) {
|
|||
nil,
|
||||
nil,
|
||||
1*time.Second,
|
||||
HTTP2,
|
||||
)
|
||||
return NewHTTP2Connection(
|
||||
cfdConn,
|
||||
|
@ -366,6 +367,7 @@ func TestServeControlStream(t *testing.T) {
|
|||
rpcClientFactory.newMockRPCClient,
|
||||
nil,
|
||||
1*time.Second,
|
||||
HTTP2,
|
||||
)
|
||||
http2Conn.controlStreamHandler = controlStream
|
||||
|
||||
|
@ -417,6 +419,7 @@ func TestFailRegistration(t *testing.T) {
|
|||
rpcClientFactory.newMockRPCClient,
|
||||
nil,
|
||||
1*time.Second,
|
||||
HTTP2,
|
||||
)
|
||||
http2Conn.controlStreamHandler = controlStream
|
||||
|
||||
|
@ -464,6 +467,7 @@ func TestGracefulShutdownHTTP2(t *testing.T) {
|
|||
rpcClientFactory.newMockRPCClient,
|
||||
shutdownC,
|
||||
1*time.Second,
|
||||
HTTP2,
|
||||
)
|
||||
|
||||
http2Conn.controlStreamHandler = controlStream
|
||||
|
|
|
@ -55,8 +55,8 @@ func (o *Observer) sendRegisteringEvent(connIndex uint8) {
|
|||
o.sendEvent(Event{Index: connIndex, EventType: RegisteringTunnel})
|
||||
}
|
||||
|
||||
func (o *Observer) sendConnectedEvent(connIndex uint8, location string) {
|
||||
o.sendEvent(Event{Index: connIndex, EventType: Connected, Location: location})
|
||||
func (o *Observer) sendConnectedEvent(connIndex uint8, protocol Protocol, location string) {
|
||||
o.sendEvent(Event{Index: connIndex, EventType: Connected, Protocol: protocol, Location: location})
|
||||
}
|
||||
|
||||
func (o *Observer) SendURL(url string) {
|
||||
|
|
|
@ -81,63 +81,63 @@ func TestQUICServer(t *testing.T) {
|
|||
},
|
||||
expectedResponse: []byte("OK"),
|
||||
},
|
||||
//{
|
||||
// desc: "test http body request streaming",
|
||||
// dest: "/slow_echo_body",
|
||||
// connectionType: quicpogs.ConnectionTypeHTTP,
|
||||
// metadata: []quicpogs.Metadata{
|
||||
// {
|
||||
// Key: "HttpHeader:Cf-Ray",
|
||||
// Val: "123123123",
|
||||
// },
|
||||
// {
|
||||
// Key: "HttpHost",
|
||||
// Val: "cf.host",
|
||||
// },
|
||||
// {
|
||||
// Key: "HttpMethod",
|
||||
// Val: "POST",
|
||||
// },
|
||||
// {
|
||||
// Key: "HttpHeader:Content-Length",
|
||||
// Val: "24",
|
||||
// },
|
||||
// },
|
||||
// message: []byte("This is the message body"),
|
||||
// expectedResponse: []byte("This is the message body"),
|
||||
//},
|
||||
//{
|
||||
// desc: "test ws proxy",
|
||||
// dest: "/ws/echo",
|
||||
// connectionType: quicpogs.ConnectionTypeWebsocket,
|
||||
// metadata: []quicpogs.Metadata{
|
||||
// {
|
||||
// Key: "HttpHeader:Cf-Cloudflared-Proxy-Connection-Upgrade",
|
||||
// Val: "Websocket",
|
||||
// },
|
||||
// {
|
||||
// Key: "HttpHeader:Another-Header",
|
||||
// Val: "Misc",
|
||||
// },
|
||||
// {
|
||||
// Key: "HttpHost",
|
||||
// Val: "cf.host",
|
||||
// },
|
||||
// {
|
||||
// Key: "HttpMethod",
|
||||
// Val: "get",
|
||||
// },
|
||||
// },
|
||||
// message: wsBuf.Bytes(),
|
||||
// expectedResponse: []byte{0x82, 0x5, 0x48, 0x65, 0x6c, 0x6c, 0x6f},
|
||||
//},
|
||||
//{
|
||||
// desc: "test tcp proxy",
|
||||
// connectionType: quicpogs.ConnectionTypeTCP,
|
||||
// metadata: []quicpogs.Metadata{},
|
||||
// message: []byte("Here is some tcp data"),
|
||||
// expectedResponse: []byte("Here is some tcp data"),
|
||||
//},
|
||||
{
|
||||
desc: "test http body request streaming",
|
||||
dest: "/slow_echo_body",
|
||||
connectionType: quicpogs.ConnectionTypeHTTP,
|
||||
metadata: []quicpogs.Metadata{
|
||||
{
|
||||
Key: "HttpHeader:Cf-Ray",
|
||||
Val: "123123123",
|
||||
},
|
||||
{
|
||||
Key: "HttpHost",
|
||||
Val: "cf.host",
|
||||
},
|
||||
{
|
||||
Key: "HttpMethod",
|
||||
Val: "POST",
|
||||
},
|
||||
{
|
||||
Key: "HttpHeader:Content-Length",
|
||||
Val: "24",
|
||||
},
|
||||
},
|
||||
message: []byte("This is the message body"),
|
||||
expectedResponse: []byte("This is the message body"),
|
||||
},
|
||||
{
|
||||
desc: "test ws proxy",
|
||||
dest: "/ws/echo",
|
||||
connectionType: quicpogs.ConnectionTypeWebsocket,
|
||||
metadata: []quicpogs.Metadata{
|
||||
{
|
||||
Key: "HttpHeader:Cf-Cloudflared-Proxy-Connection-Upgrade",
|
||||
Val: "Websocket",
|
||||
},
|
||||
{
|
||||
Key: "HttpHeader:Another-Header",
|
||||
Val: "Misc",
|
||||
},
|
||||
{
|
||||
Key: "HttpHost",
|
||||
Val: "cf.host",
|
||||
},
|
||||
{
|
||||
Key: "HttpMethod",
|
||||
Val: "get",
|
||||
},
|
||||
},
|
||||
message: wsBuf.Bytes(),
|
||||
expectedResponse: []byte{0x82, 0x5, 0x48, 0x65, 0x6c, 0x6c, 0x6f},
|
||||
},
|
||||
{
|
||||
desc: "test tcp proxy",
|
||||
connectionType: quicpogs.ConnectionTypeTCP,
|
||||
metadata: []quicpogs.Metadata{},
|
||||
message: []byte("Here is some tcp data"),
|
||||
expectedResponse: []byte("Here is some tcp data"),
|
||||
},
|
||||
}
|
||||
|
||||
for _, test := range tests {
|
||||
|
|
|
@ -117,9 +117,6 @@ func (rsc *registrationServerClient) RegisterConnection(
|
|||
|
||||
observer.metrics.regSuccess.WithLabelValues("registerConnection").Inc()
|
||||
|
||||
observer.logServerInfo(connIndex, conn.Location, edgeAddress, fmt.Sprintf("Connection %s registered", conn.UUID))
|
||||
observer.sendConnectedEvent(connIndex, conn.Location)
|
||||
|
||||
return conn, nil
|
||||
}
|
||||
|
||||
|
@ -293,9 +290,13 @@ func (h *h2muxConnection) registerNamedTunnel(
|
|||
rpcClient := h.newRPCClientFunc(ctx, stream, h.observer.log)
|
||||
defer rpcClient.Close()
|
||||
|
||||
if _, err = rpcClient.RegisterConnection(ctx, namedTunnel, connOptions, h.connIndex, nil, h.observer); err != nil {
|
||||
registrationDetails, err := rpcClient.RegisterConnection(ctx, namedTunnel, connOptions, h.connIndex, nil, h.observer)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
h.observer.logServerInfo(h.connIndex, registrationDetails.Location, nil, fmt.Sprintf("Connection %s registered", registrationDetails.UUID))
|
||||
h.observer.sendConnectedEvent(h.connIndex, H2mux, registrationDetails.Location)
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
|
|
|
@ -14,7 +14,7 @@ import (
|
|||
|
||||
func TestReadyServer_makeResponse(t *testing.T) {
|
||||
type fields struct {
|
||||
isConnected map[int]bool
|
||||
isConnected map[uint8]tunnelstate.ConnectionInfo
|
||||
}
|
||||
tests := []struct {
|
||||
name string
|
||||
|
@ -25,11 +25,11 @@ func TestReadyServer_makeResponse(t *testing.T) {
|
|||
{
|
||||
name: "One connection online => HTTP 200",
|
||||
fields: fields{
|
||||
isConnected: map[int]bool{
|
||||
0: false,
|
||||
1: false,
|
||||
2: true,
|
||||
3: false,
|
||||
isConnected: map[uint8]tunnelstate.ConnectionInfo{
|
||||
0: {IsConnected: false},
|
||||
1: {IsConnected: false},
|
||||
2: {IsConnected: true},
|
||||
3: {IsConnected: false},
|
||||
},
|
||||
},
|
||||
wantOK: true,
|
||||
|
@ -38,11 +38,11 @@ func TestReadyServer_makeResponse(t *testing.T) {
|
|||
{
|
||||
name: "No connections online => no HTTP 200",
|
||||
fields: fields{
|
||||
isConnected: map[int]bool{
|
||||
0: false,
|
||||
1: false,
|
||||
2: false,
|
||||
3: false,
|
||||
isConnected: map[uint8]tunnelstate.ConnectionInfo{
|
||||
0: {IsConnected: false},
|
||||
1: {IsConnected: false},
|
||||
2: {IsConnected: false},
|
||||
3: {IsConnected: false},
|
||||
},
|
||||
},
|
||||
wantReadyConnections: 0,
|
||||
|
|
|
@ -12,9 +12,9 @@ type ConnAwareLogger struct {
|
|||
logger *zerolog.Logger
|
||||
}
|
||||
|
||||
func NewConnAwareLogger(logger *zerolog.Logger, observer *connection.Observer) *ConnAwareLogger {
|
||||
func NewConnAwareLogger(logger *zerolog.Logger, tracker *tunnelstate.ConnTracker, observer *connection.Observer) *ConnAwareLogger {
|
||||
connAwareLogger := &ConnAwareLogger{
|
||||
tracker: tunnelstate.NewConnTracker(logger),
|
||||
tracker: tracker,
|
||||
logger: logger,
|
||||
}
|
||||
|
||||
|
|
|
@ -19,6 +19,7 @@ import (
|
|||
"github.com/cloudflare/cloudflared/retry"
|
||||
"github.com/cloudflare/cloudflared/signal"
|
||||
tunnelpogs "github.com/cloudflare/cloudflared/tunnelrpc/pogs"
|
||||
"github.com/cloudflare/cloudflared/tunnelstate"
|
||||
)
|
||||
|
||||
const (
|
||||
|
@ -88,7 +89,9 @@ func NewSupervisor(config *TunnelConfig, orchestrator *orchestration.Orchestrato
|
|||
}
|
||||
|
||||
reconnectCredentialManager := newReconnectCredentialManager(connection.MetricsNamespace, connection.TunnelSubsystem, config.HAConnections)
|
||||
log := NewConnAwareLogger(config.Log, config.Observer)
|
||||
|
||||
tracker := tunnelstate.NewConnTracker(config.Log)
|
||||
log := NewConnAwareLogger(config.Log, tracker, config.Observer)
|
||||
|
||||
var edgeAddrHandler EdgeAddrHandler
|
||||
if isStaticEdge { // static edge addresses
|
||||
|
@ -106,6 +109,7 @@ func NewSupervisor(config *TunnelConfig, orchestrator *orchestration.Orchestrato
|
|||
credentialManager: reconnectCredentialManager,
|
||||
edgeAddrs: edgeIPs,
|
||||
edgeAddrHandler: edgeAddrHandler,
|
||||
tracker: tracker,
|
||||
reconnectCh: reconnectCh,
|
||||
gracefulShutdownC: gracefulShutdownC,
|
||||
connAwareLogger: log,
|
||||
|
|
|
@ -26,6 +26,7 @@ import (
|
|||
"github.com/cloudflare/cloudflared/signal"
|
||||
"github.com/cloudflare/cloudflared/tunnelrpc"
|
||||
tunnelpogs "github.com/cloudflare/cloudflared/tunnelrpc/pogs"
|
||||
"github.com/cloudflare/cloudflared/tunnelstate"
|
||||
)
|
||||
|
||||
const (
|
||||
|
@ -191,6 +192,7 @@ type EdgeTunnelServer struct {
|
|||
edgeAddrs *edgediscovery.Edge
|
||||
reconnectCh chan ReconnectSignal
|
||||
gracefulShutdownC <-chan struct{}
|
||||
tracker *tunnelstate.ConnTracker
|
||||
|
||||
connAwareLogger *ConnAwareLogger
|
||||
}
|
||||
|
@ -273,6 +275,12 @@ func (e EdgeTunnelServer) Serve(ctx context.Context, connIndex uint8, protocolFa
|
|||
return err
|
||||
}
|
||||
|
||||
// If a single connection has connected with the current protocol, we know we know we don't have to fallback
|
||||
// to a different protocol.
|
||||
if e.tracker.HasConnectedWith(e.config.ProtocolSelector.Current()) {
|
||||
return err
|
||||
}
|
||||
|
||||
if !selectNextProtocol(
|
||||
connLog.Logger(),
|
||||
protocolFallback,
|
||||
|
@ -462,6 +470,7 @@ func serveTunnel(
|
|||
nil,
|
||||
gracefulShutdownC,
|
||||
config.GracePeriod,
|
||||
protocol,
|
||||
)
|
||||
|
||||
switch protocol {
|
||||
|
|
|
@ -10,20 +10,26 @@ import (
|
|||
|
||||
type ConnTracker struct {
|
||||
sync.RWMutex
|
||||
isConnected map[int]bool
|
||||
log *zerolog.Logger
|
||||
// int is the connection Index
|
||||
connectionInfo map[uint8]ConnectionInfo
|
||||
log *zerolog.Logger
|
||||
}
|
||||
|
||||
type ConnectionInfo struct {
|
||||
IsConnected bool
|
||||
Protocol connection.Protocol
|
||||
}
|
||||
|
||||
func NewConnTracker(log *zerolog.Logger) *ConnTracker {
|
||||
return &ConnTracker{
|
||||
isConnected: make(map[int]bool, 0),
|
||||
log: log,
|
||||
connectionInfo: make(map[uint8]ConnectionInfo, 0),
|
||||
log: log,
|
||||
}
|
||||
}
|
||||
|
||||
func MockedConnTracker(mocked map[int]bool) *ConnTracker {
|
||||
func MockedConnTracker(mocked map[uint8]ConnectionInfo) *ConnTracker {
|
||||
return &ConnTracker{
|
||||
isConnected: mocked,
|
||||
connectionInfo: mocked,
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -31,11 +37,17 @@ func (ct *ConnTracker) OnTunnelEvent(c connection.Event) {
|
|||
switch c.EventType {
|
||||
case connection.Connected:
|
||||
ct.Lock()
|
||||
ct.isConnected[int(c.Index)] = true
|
||||
ci := ConnectionInfo{
|
||||
IsConnected: true,
|
||||
Protocol: c.Protocol,
|
||||
}
|
||||
ct.connectionInfo[c.Index] = ci
|
||||
ct.Unlock()
|
||||
case connection.Disconnected, connection.Reconnecting, connection.RegisteringTunnel, connection.Unregistering:
|
||||
ct.Lock()
|
||||
ct.isConnected[int(c.Index)] = false
|
||||
ci := ct.connectionInfo[c.Index]
|
||||
ci.IsConnected = false
|
||||
ct.connectionInfo[c.Index] = ci
|
||||
ct.Unlock()
|
||||
default:
|
||||
ct.log.Error().Msgf("Unknown connection event case %v", c)
|
||||
|
@ -46,10 +58,23 @@ func (ct *ConnTracker) CountActiveConns() uint {
|
|||
ct.RLock()
|
||||
defer ct.RUnlock()
|
||||
active := uint(0)
|
||||
for _, connected := range ct.isConnected {
|
||||
if connected {
|
||||
for _, ci := range ct.connectionInfo {
|
||||
if ci.IsConnected {
|
||||
active++
|
||||
}
|
||||
}
|
||||
return active
|
||||
}
|
||||
|
||||
// HasConnectedWith checks if we've ever had a successful connection to the edge
|
||||
// with said protocol.
|
||||
func (ct *ConnTracker) HasConnectedWith(protocol connection.Protocol) bool {
|
||||
ct.RLock()
|
||||
defer ct.RUnlock()
|
||||
for _, ci := range ct.connectionInfo {
|
||||
if ci.Protocol == protocol {
|
||||
return true
|
||||
}
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
|
Loading…
Reference in New Issue