Commite0f7f01

authored

fix(healthcheck): ensure STUNOnly nodes aren't marked as unhealthy (#6990)

1 parenta1371db commite0f7f01Copy full SHA for e0f7f01

File tree

+51

-1

lines changed

+51

-1

lines changed

Lines changed: 8 additions & 1 deletion

Original file line number	Diff line number	Diff line change
`@@ -171,7 +171,14 @@ func (r *DERPNodeReport) Run(ctx context.Context) error {`
`171`	`171`	`r.doExchangeMessage(ctx)`
`172`	`172`	`r.doSTUNTest(ctx)`
`173`	`173`
`174`		`-if!r.CanExchangeMessages\|\|r.UsesWebsocket\|\|r.STUN.Error!=nil {`
	`174`	`+// We can't exchange messages with the node,`
	`175`	`+if (!r.CanExchangeMessages&&!r.Node.STUNOnly)\|\|`
	`176`	+// A node may use websockets because `Upgrade: DERP` may be blocked on
	`177`	`+// the load balancer. This is unhealthy because websockets are slower`
	`178`	`+// than the regular DERP protocol.`
	`179`	`+r.UsesWebsocket\|\|`
	`180`	`+// The node was marked as STUN compatible but the STUN test failed.`
	`181`	`+r.STUN.Error!=nil {`
`175`	`182`	`r.Healthy=false`
`176`	`183`	`}`
`177`	`184`	`returnnil`

Lines changed: 43 additions & 0 deletions

Original file line number	Diff line number	Diff line change
`@@ -186,6 +186,49 @@ func TestDERP(t *testing.T) {`
`186`	`186`	`}`
`187`	`187`	`}`
`188`	`188`	`})`
	`189`	`+`
	`190`	`+t.Run("OK/STUNOnly",func(t*testing.T) {`
	`191`	`+t.Parallel()`
	`192`	`+`
	`193`	`+var (`
	`194`	`+ctx=context.Background()`
	`195`	`+report= healthcheck.DERPReport{}`
	`196`	`+opts=&healthcheck.DERPReportOptions{`
	`197`	`+DERPMap:&tailcfg.DERPMap{Regions:map[int]*tailcfg.DERPRegion{`
	`198`	`+1: {`
	`199`	`+EmbeddedRelay:true,`
	`200`	`+RegionID:999,`
	`201`	`+Nodes: []*tailcfg.DERPNode{{`
	`202`	`+Name:"999stun0",`
	`203`	`+RegionID:999,`
	`204`	`+HostName:"stun.l.google.com",`
	`205`	`+STUNPort:19302,`
	`206`	`+STUNOnly:true,`
	`207`	`+InsecureForTests:true,`
	`208`	`+ForceHTTP:true,`
	`209`	`+}},`
	`210`	`+},`
	`211`	`+}},`
	`212`	`+}`
	`213`	`+)`
	`214`	`+`
	`215`	`+err:=report.Run(ctx,opts)`
	`216`	`+require.NoError(t,err)`
	`217`	`+`
	`218`	`+assert.True(t,report.Healthy)`
	`219`	`+for_,region:=rangereport.Regions {`
	`220`	`+assert.True(t,region.Healthy)`
	`221`	`+for_,node:=rangeregion.NodeReports {`
	`222`	`+assert.True(t,node.Healthy)`
	`223`	`+assert.False(t,node.CanExchangeMessages)`
	`224`	`+assert.Len(t,node.ClientLogs,0)`
	`225`	`+`
	`226`	`+assert.True(t,node.STUN.Enabled)`
	`227`	`+assert.True(t,node.STUN.CanSTUN)`
	`228`	`+assert.NoError(t,node.STUN.Error)`
	`229`	`+}`
	`230`	`+}`
	`231`	`+})`
`189`	`232`	`}`
`190`	`233`
`191`	`234`	`functsDERPMap(ctx context.Context,t testing.TB)*tailcfg.DERPMap {`

Comments

(0)