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

coadler · web-flow · commit e0f7f011d816 · 2023-04-03T16:14:48.000-05:00
diff --git a/coderd/healthcheck/derp.go b/coderd/healthcheck/derp.go
@@ -171,7 +171,14 @@ func (r *DERPNodeReport) Run(ctx context.Context) error {
 	r.doExchangeMessage(ctx)
 	r.doSTUNTest(ctx)
 
-	if !r.CanExchangeMessages || r.UsesWebsocket || r.STUN.Error != nil {
+	// We can't exchange messages with the node,
+	if (!r.CanExchangeMessages && !r.Node.STUNOnly) ||
+		// A node may use websockets because `Upgrade: DERP` may be blocked on
+		// the load balancer. This is unhealthy because websockets are slower
+		// than the regular DERP protocol.
+		r.UsesWebsocket ||
+		// The node was marked as STUN compatible but the STUN test failed.
+		r.STUN.Error != nil {
 		r.Healthy = false
 	}
 	return nil
diff --git a/coderd/healthcheck/derp_test.go b/coderd/healthcheck/derp_test.go
@@ -186,6 +186,49 @@ func TestDERP(t *testing.T) {
 			}
 		}
 	})
+
+	t.Run("OK/STUNOnly", func(t *testing.T) {
+		t.Parallel()
+
+		var (
+			ctx    = context.Background()
+			report = healthcheck.DERPReport{}
+			opts   = &healthcheck.DERPReportOptions{
+				DERPMap: &tailcfg.DERPMap{Regions: map[int]*tailcfg.DERPRegion{
+					1: {
+						EmbeddedRelay: true,
+						RegionID:      999,
+						Nodes: []*tailcfg.DERPNode{{
+							Name:             "999stun0",
+							RegionID:         999,
+							HostName:         "stun.l.google.com",
+							STUNPort:         19302,
+							STUNOnly:         true,
+							InsecureForTests: true,
+							ForceHTTP:        true,
+						}},
+					},
+				}},
+			}
+		)
+
+		err := report.Run(ctx, opts)
+		require.NoError(t, err)
+
+		assert.True(t, report.Healthy)
+		for _, region := range report.Regions {
+			assert.True(t, region.Healthy)
+			for _, node := range region.NodeReports {
+				assert.True(t, node.Healthy)
+				assert.False(t, node.CanExchangeMessages)
+				assert.Len(t, node.ClientLogs, 0)
+
+				assert.True(t, node.STUN.Enabled)
+				assert.True(t, node.STUN.CanSTUN)
+				assert.NoError(t, node.STUN.Error)
+			}
+		}
+	})
 }
 
 func tsDERPMap(ctx context.Context, t testing.TB) *tailcfg.DERPMap {
