Skip to content

Commit 8999d57

Browse files
mtojekmtojek
authored
feat: do not fail DERP healthcheck if WebSocket is used (#10714)
1 parent 24aa223 commit 8999d57

File tree

14 files changed

+285
-80
lines changed

14 files changed

+285
-80
lines changed

‎coderd/apidoc/docs.go

Lines changed: 36 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/apidoc/swagger.json

Lines changed: 36 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/healthcheck/accessurl.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,10 @@ import (
1414

1515
// @typescript-generate AccessURLReport
1616
type AccessURLReport struct {
17+
Healthy bool `json:"healthy"`
18+
Warnings []string `json:"warnings"`
19+
1720
AccessURL string `json:"access_url"`
18-
Healthy bool `json:"healthy"`
1921
Reachable bool `json:"reachable"`
2022
StatusCode int `json:"status_code"`
2123
HealthzResponse string `json:"healthz_response"`

‎coderd/healthcheck/database.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,9 @@ const (
1616

1717
// @typescript-generate DatabaseReport
1818
type DatabaseReport struct {
19-
Healthy bool `json:"healthy"`
19+
Healthy bool `json:"healthy"`
20+
Warnings []string `json:"warnings"`
21+
2022
Reachable bool `json:"reachable"`
2123
Latency string `json:"latency"`
2224
LatencyMS int64 `json:"latency_ms"`

‎coderd/healthcheck/derphealth/derp.go

Lines changed: 25 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,14 @@ import (
2424
"github.com/coder/coder/v2/coderd/util/ptr"
2525
)
2626

27+
const (
28+
warningNodeUsesWebsocket = `Node uses WebSockets because the "Upgrade: DERP" header may be blocked on the load balancer.`
29+
)
30+
2731
// @typescript-generate Report
2832
type Report struct {
29-
Healthy bool `json:"healthy"`
33+
Healthy bool `json:"healthy"`
34+
Warnings []string `json:"warnings"`
3035

3136
Regions map[int]*RegionReport `json:"regions"`
3237

@@ -39,8 +44,9 @@ type Report struct {
3944

4045
// @typescript-generate RegionReport
4146
type RegionReport struct {
42-
mu sync.Mutex
43-
Healthy bool `json:"healthy"`
47+
mu sync.Mutex
48+
Healthy bool `json:"healthy"`
49+
Warnings []string `json:"warnings"`
4450

4551
Region *tailcfg.DERPRegion `json:"region"`
4652
NodeReports []*NodeReport `json:"node_reports"`
@@ -52,8 +58,10 @@ type NodeReport struct {
5258
mu sync.Mutex
5359
clientCounter int
5460

55-
Healthy bool `json:"healthy"`
56-
Node *tailcfg.DERPNode `json:"node"`
61+
Healthy bool `json:"healthy"`
62+
Warnings []string `json:"warnings"`
63+
64+
Node *tailcfg.DERPNode `json:"node"`
5765

5866
ServerInfo derp.ServerInfoMessage `json:"node_info"`
5967
CanExchangeMessages bool `json:"can_exchange_messages"`
@@ -108,6 +116,10 @@ func (r *Report) Run(ctx context.Context, opts *ReportOptions) {
108116
if !regionReport.Healthy {
109117
r.Healthy = false
110118
}
119+
120+
for _, w := range regionReport.Warnings {
121+
r.Warnings = append(r.Warnings, fmt.Sprintf("[%s] %s", regionReport.Region.RegionName, w))
122+
}
111123
mu.Unlock()
112124
}()
113125
}
@@ -159,6 +171,10 @@ func (r *RegionReport) Run(ctx context.Context) {
159171
if !nodeReport.Healthy {
160172
r.Healthy = false
161173
}
174+
175+
for _, w := range nodeReport.Warnings {
176+
r.Warnings = append(r.Warnings, fmt.Sprintf("[%s] %s", nodeReport.Node.Name, w))
177+
}
162178
r.mu.Unlock()
163179
}()
164180
}
@@ -208,14 +224,14 @@ func (r *NodeReport) Run(ctx context.Context) {
208224

209225
// We can't exchange messages with the node,
210226
if (!r.CanExchangeMessages && !r.Node.STUNOnly) ||
211-
// A node may use websockets because `Upgrade: DERP` may be blocked on
212-
// the load balancer. This is unhealthy because websockets are slower
213-
// than the regular DERP protocol.
214-
r.UsesWebsocket ||
215227
// The node was marked as STUN compatible but the STUN test failed.
216228
r.STUN.Error != nil {
217229
r.Healthy = false
218230
}
231+
232+
if r.UsesWebsocket {
233+
r.Warnings = append(r.Warnings, warningNodeUsesWebsocket)
234+
}
219235
}
220236

221237
func (r *NodeReport) doExchangeMessage(ctx context.Context) {

‎coderd/healthcheck/derphealth/derp_test.go

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -170,11 +170,14 @@ func TestDERP(t *testing.T) {
170170

171171
report.Run(ctx, opts)
172172

173-
assert.False(t, report.Healthy)
173+
assert.True(t, report.Healthy)
174+
assert.NotEmpty(t, report.Warnings)
174175
for _, region := range report.Regions {
175-
assert.False(t, region.Healthy)
176+
assert.True(t, region.Healthy)
177+
assert.NotEmpty(t, region.Warnings)
176178
for _, node := range region.NodeReports {
177-
assert.False(t, node.Healthy)
179+
assert.True(t, node.Healthy)
180+
assert.NotEmpty(t, node.Warnings)
178181
assert.True(t, node.CanExchangeMessages)
179182
assert.NotEmpty(t, node.RoundTripPing)
180183
assert.Len(t, node.ClientLogs, 2)

‎coderd/healthcheck/healthcheck_test.go

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,25 @@ func TestHealthcheck(t *testing.T) {
7777
},
7878
healthy: false,
7979
failingSections: []string{healthcheck.SectionDERP},
80+
}, {
81+
name: "DERPWarning",
82+
checker: &testChecker{
83+
DERPReport: derphealth.Report{
84+
Healthy: true,
85+
Warnings: []string{"foobar"},
86+
},
87+
AccessURLReport: healthcheck.AccessURLReport{
88+
Healthy: true,
89+
},
90+
WebsocketReport: healthcheck.WebsocketReport{
91+
Healthy: true,
92+
},
93+
DatabaseReport: healthcheck.DatabaseReport{
94+
Healthy: true,
95+
},
96+
},
97+
healthy: true,
98+
failingSections: nil,
8099
}, {
81100
name: "AccessURLFail",
82101
checker: &testChecker{
@@ -153,6 +172,7 @@ func TestHealthcheck(t *testing.T) {
153172
assert.Equal(t, c.healthy, report.Healthy)
154173
assert.Equal(t, c.failingSections, report.FailingSections)
155174
assert.Equal(t, c.checker.DERPReport.Healthy, report.DERP.Healthy)
175+
assert.Equal(t, c.checker.DERPReport.Warnings, report.DERP.Warnings)
156176
assert.Equal(t, c.checker.AccessURLReport.Healthy, report.AccessURL.Healthy)
157177
assert.Equal(t, c.checker.WebsocketReport.Healthy, report.Websocket.Healthy)
158178
assert.NotZero(t, report.Time)

‎coderd/healthcheck/websocket.go

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,12 @@ type WebsocketReportOptions struct {
2121

2222
// @typescript-generate WebsocketReport
2323
type WebsocketReport struct {
24-
Healthy bool `json:"healthy"`
25-
Body string `json:"body"`
26-
Code int `json:"code"`
27-
Error *string `json:"error"`
24+
Healthy bool `json:"healthy"`
25+
Warnings []string `json:"warnings"`
26+
27+
Body string `json:"body"`
28+
Code int `json:"code"`
29+
Error *string `json:"error"`
2830
}
2931

3032
func (r *WebsocketReport) Run(ctx context.Context, opts *WebsocketReportOptions) {

0 commit comments

Comments
 (0)