From 9339bb9485574fcae1f95a9b36cd53e4c9fb48f4 Mon Sep 17 00:00:00 2001 From: Sudarsan Reddy Date: Wed, 16 Nov 2022 09:08:45 +0000 Subject: [PATCH] TUN-6929: Use same protocol for other connections as first one This PR changes protocol initialization of the other N connections to be the same as the one we know the initial tunnel connected with. This is so we homogenize connections and not lead to some connections being QUIC-able and the others not. There's also an improvement to the connection registered log so we know what protocol every individual connection connected with from the cloudflared side. --- connection/control.go | 2 +- supervisor/supervisor.go | 5 ++- supervisor/supervisor_test.go | 74 +++++++++++++++++++++++++++++++++++ supervisor/tunnel.go | 4 ++ 4 files changed, 82 insertions(+), 3 deletions(-) create mode 100644 supervisor/supervisor_test.go diff --git a/connection/control.go b/connection/control.go index a7fe1ac9..174db372 100644 --- a/connection/control.go +++ b/connection/control.go @@ -85,7 +85,7 @@ func (c *controlStream) ServeControlStream( return err } - c.observer.logServerInfo(c.connIndex, registrationDetails.Location, c.edgeAddress, fmt.Sprintf("Connection %s registered", registrationDetails.UUID)) + c.observer.logServerInfo(c.connIndex, registrationDetails.Location, c.edgeAddress, fmt.Sprintf("Connection %s registered with protocol: %s", registrationDetails.UUID, c.protocol)) c.observer.sendConnectedEvent(c.connIndex, c.protocol, registrationDetails.Location) c.connectedFuse.Connected() diff --git a/supervisor/supervisor.go b/supervisor/supervisor.go index 9f6a2fe2..a7a747ca 100644 --- a/supervisor/supervisor.go +++ b/supervisor/supervisor.go @@ -45,7 +45,7 @@ type Supervisor struct { config *TunnelConfig orchestrator *orchestration.Orchestrator edgeIPs *edgediscovery.Edge - edgeTunnelServer *EdgeTunnelServer + edgeTunnelServer TunnelServer tunnelErrors chan tunnelError tunnelsConnecting map[int]chan struct{} tunnelsProtocolFallback map[int]*protocolFallback @@ -285,7 +285,8 @@ func (s *Supervisor) initialize( for i := 1; i < s.config.HAConnections; i++ { s.tunnelsProtocolFallback[i] = &protocolFallback{ retry.BackoffHandler{MaxRetries: s.config.Retries, RetryForever: true}, - s.config.ProtocolSelector.Current(), + // Set the protocol we know the first tunnel connected with. + s.tunnelsProtocolFallback[0].protocol, false, } go s.startTunnel(ctx, i, s.newConnectedTunnelSignal(i)) diff --git a/supervisor/supervisor_test.go b/supervisor/supervisor_test.go new file mode 100644 index 00000000..70e0f547 --- /dev/null +++ b/supervisor/supervisor_test.go @@ -0,0 +1,74 @@ +package supervisor + +import ( + "context" + "testing" + + "github.com/rs/zerolog" + "github.com/stretchr/testify/assert" + + "github.com/cloudflare/cloudflared/connection" + "github.com/cloudflare/cloudflared/edgediscovery" + "github.com/cloudflare/cloudflared/edgediscovery/allregions" + "github.com/cloudflare/cloudflared/signal" +) + +type mockProtocolSelector struct { + protocols []connection.Protocol + index int +} + +func (m *mockProtocolSelector) Current() connection.Protocol { + return m.protocols[m.index] +} + +func (m *mockProtocolSelector) Fallback() (connection.Protocol, bool) { + m.index++ + if m.index == len(m.protocols) { + return m.protocols[len(m.protocols)-1], false + } + + return m.protocols[m.index], true +} + +type mockEdgeTunnelServer struct { + config *TunnelConfig +} + +func (m *mockEdgeTunnelServer) Serve(ctx context.Context, connIndex uint8, protocolFallback *protocolFallback, connectedSignal *signal.Signal) error { + // This is to mock the first connection falling back because of connectivity issues. + protocolFallback.protocol, _ = m.config.ProtocolSelector.Fallback() + connectedSignal.Notify() + return nil +} + +// Test to check if initialize sets all the different connections to the same protocol should the first +// tunnel fall back. +func Test_Initialize_Same_Protocol(t *testing.T) { + edgeIPs, err := edgediscovery.ResolveEdge(&zerolog.Logger{}, "us", allregions.Auto) + assert.Nil(t, err) + s := Supervisor{ + edgeIPs: edgeIPs, + config: &TunnelConfig{ + ProtocolSelector: &mockProtocolSelector{protocols: []connection.Protocol{connection.QUIC, connection.HTTP2, connection.H2mux}}, + }, + tunnelsProtocolFallback: make(map[int]*protocolFallback), + edgeTunnelServer: &mockEdgeTunnelServer{ + config: &TunnelConfig{ + ProtocolSelector: &mockProtocolSelector{protocols: []connection.Protocol{connection.QUIC, connection.HTTP2, connection.H2mux}}, + }, + }, + } + + ctx := context.Background() + connectedSignal := signal.New(make(chan struct{})) + s.initialize(ctx, connectedSignal) + + // Make sure we fell back to http2 as the mock Serve is wont to do. + assert.Equal(t, s.tunnelsProtocolFallback[0].protocol, connection.HTTP2) + + // Ensure all the protocols we set to try are the same as what the first tunnel has fallen back to. + for _, protocolFallback := range s.tunnelsProtocolFallback { + assert.Equal(t, protocolFallback.protocol, s.tunnelsProtocolFallback[0].protocol) + } +} diff --git a/supervisor/tunnel.go b/supervisor/tunnel.go index e4a0a08a..46e315e4 100644 --- a/supervisor/tunnel.go +++ b/supervisor/tunnel.go @@ -206,6 +206,10 @@ type EdgeTunnelServer struct { connAwareLogger *ConnAwareLogger } +type TunnelServer interface { + Serve(ctx context.Context, connIndex uint8, protocolFallback *protocolFallback, connectedSignal *signal.Signal) error +} + func (e *EdgeTunnelServer) Serve(ctx context.Context, connIndex uint8, protocolFallback *protocolFallback, connectedSignal *signal.Signal) error { haConnections.Inc() defer haConnections.Dec()