Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commitc01d6fd

Browse files
authored
chore: refactor apphealth and tests to use clock testing library (#13576)
Refactors the apphealth subsystem and unit tests to use `clock.Clock`.Also slightly simplifies the implementation, which wrapped a function that never returned an error in a `retry.Retry`. The retry is entirely superfluous in that case, so removed.UTs used to take a few seconds to run, and now run in milliseconds or better. No sleeps, `Eventually`, or polling.Dropped the "no spamming" test since we can directly assert the number of handler calls on the mainline test case.
1 parentfe240ad commitc01d6fd

File tree

2 files changed

+203
-169
lines changed

2 files changed

+203
-169
lines changed

‎agent/apphealth.go

Lines changed: 54 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -10,14 +10,11 @@ import (
1010
"golang.org/x/xerrors"
1111

1212
"cdr.dev/slog"
13+
"github.com/coder/coder/v2/clock"
1314
"github.com/coder/coder/v2/codersdk"
1415
"github.com/coder/coder/v2/codersdk/agentsdk"
15-
"github.com/coder/retry"
1616
)
1717

18-
// WorkspaceAgentApps fetches the workspace apps.
19-
typeWorkspaceAgentAppsfunc(context.Context) ([]codersdk.WorkspaceApp,error)
20-
2118
// PostWorkspaceAgentAppHealth updates the workspace app health.
2219
typePostWorkspaceAgentAppHealthfunc(context.Context, agentsdk.PostAppHealthsRequest)error
2320

@@ -26,15 +23,26 @@ type WorkspaceAppHealthReporter func(ctx context.Context)
2623

2724
// NewWorkspaceAppHealthReporter creates a WorkspaceAppHealthReporter that reports app health to coderd.
2825
funcNewWorkspaceAppHealthReporter(logger slog.Logger,apps []codersdk.WorkspaceApp,postWorkspaceAgentAppHealthPostWorkspaceAgentAppHealth)WorkspaceAppHealthReporter {
26+
returnNewAppHealthReporterWithClock(logger,apps,postWorkspaceAgentAppHealth,clock.NewReal())
27+
}
28+
29+
// NewAppHealthReporterWithClock is only called directly by test code. Product code should call
30+
// NewAppHealthReporter.
31+
funcNewAppHealthReporterWithClock(
32+
logger slog.Logger,
33+
apps []codersdk.WorkspaceApp,
34+
postWorkspaceAgentAppHealthPostWorkspaceAgentAppHealth,
35+
clk clock.Clock,
36+
)WorkspaceAppHealthReporter {
2937
logger=logger.Named("apphealth")
3038

31-
runHealthcheckLoop:=func(ctx context.Context)error {
39+
returnfunc(ctx context.Context) {
3240
ctx,cancel:=context.WithCancel(ctx)
3341
defercancel()
3442

3543
// no need to run this loop if no apps for this workspace.
3644
iflen(apps)==0 {
37-
returnnil
45+
return
3846
}
3947

4048
hasHealthchecksEnabled:=false
@@ -49,7 +57,7 @@ func NewWorkspaceAppHealthReporter(logger slog.Logger, apps []codersdk.Workspace
4957

5058
// no need to run this loop if no health checks are configured.
5159
if!hasHealthchecksEnabled {
52-
returnnil
60+
return
5361
}
5462

5563
// run a ticker for each app health check.
@@ -61,25 +69,29 @@ func NewWorkspaceAppHealthReporter(logger slog.Logger, apps []codersdk.Workspace
6169
}
6270
app:=nextApp
6371
gofunc() {
64-
t:=time.NewTicker(time.Duration(app.Healthcheck.Interval)*time.Second)
65-
defert.Stop()
66-
67-
for {
68-
select {
69-
case<-ctx.Done():
70-
return
71-
case<-t.C:
72-
}
73-
// we set the http timeout to the healthcheck interval to prevent getting too backed up.
74-
client:=&http.Client{
75-
Timeout:time.Duration(app.Healthcheck.Interval)*time.Second,
76-
}
72+
_=clk.TickerFunc(ctx,time.Duration(app.Healthcheck.Interval)*time.Second,func()error {
73+
// We time out at the healthcheck interval to prevent getting too backed up, but
74+
// set it 1ms early so that it's not simultaneous with the next tick in testing,
75+
// which makes the test easier to understand.
76+
//
77+
// It would be idiomatic to use the http.Client.Timeout or a context.WithTimeout,
78+
// but we are passing this off to the native http library, which is not aware
79+
// of the clock library we are using. That means in testing, with a mock clock
80+
// it will compare mocked times with real times, and we will get strange results.
81+
// So, we just implement the timeout as a context we cancel with an AfterFunc
82+
reqCtx,reqCancel:=context.WithCancel(ctx)
83+
timeout:=clk.AfterFunc(
84+
time.Duration(app.Healthcheck.Interval)*time.Second-time.Millisecond,
85+
reqCancel,
86+
"timeout",app.Slug)
87+
defertimeout.Stop()
88+
7789
err:=func()error {
78-
req,err:=http.NewRequestWithContext(ctx,http.MethodGet,app.Healthcheck.URL,nil)
90+
req,err:=http.NewRequestWithContext(reqCtx,http.MethodGet,app.Healthcheck.URL,nil)
7991
iferr!=nil {
8092
returnerr
8193
}
82-
res,err:=client.Do(req)
94+
res,err:=http.DefaultClient.Do(req)
8395
iferr!=nil {
8496
returnerr
8597
}
@@ -118,54 +130,36 @@ func NewWorkspaceAppHealthReporter(logger slog.Logger, apps []codersdk.Workspace
118130
mu.Unlock()
119131
logger.Debug(ctx,"workspace app healthy",slog.F("id",app.ID.String()),slog.F("slug",app.Slug))
120132
}
121-
122-
t.Reset(time.Duration(app.Healthcheck.Interval)*time.Second)
123-
}
133+
returnnil
134+
},"healthcheck",app.Slug)
124135
}()
125136
}
126137

127138
mu.Lock()
128139
lastHealth:=copyHealth(health)
129140
mu.Unlock()
130-
reportTicker:=time.NewTicker(time.Second)
131-
deferreportTicker.Stop()
132-
// every second we check if the health values of the apps have changed
133-
// and if there is a change we will report the new values.
134-
for {
135-
select {
136-
case<-ctx.Done():
141+
reportTicker:=clk.TickerFunc(ctx,time.Second,func()error {
142+
mu.RLock()
143+
changed:=healthChanged(lastHealth,health)
144+
mu.RUnlock()
145+
if!changed {
137146
returnnil
138-
case<-reportTicker.C:
139-
mu.RLock()
140-
changed:=healthChanged(lastHealth,health)
141-
mu.RUnlock()
142-
if!changed {
143-
continue
144-
}
145-
146-
mu.Lock()
147-
lastHealth=copyHealth(health)
148-
mu.Unlock()
149-
err:=postWorkspaceAgentAppHealth(ctx, agentsdk.PostAppHealthsRequest{
150-
Healths:lastHealth,
151-
})
152-
iferr!=nil {
153-
logger.Error(ctx,"failed to report workspace app health",slog.Error(err))
154-
}else {
155-
logger.Debug(ctx,"sent workspace app health",slog.F("health",lastHealth))
156-
}
157147
}
158-
}
159-
}
160148

161-
returnfunc(ctx context.Context) {
162-
forr:=retry.New(time.Second,30*time.Second);r.Wait(ctx); {
163-
err:=runHealthcheckLoop(ctx)
164-
iferr==nil||xerrors.Is(err,context.Canceled)||xerrors.Is(err,context.DeadlineExceeded) {
165-
return
149+
mu.Lock()
150+
lastHealth=copyHealth(health)
151+
mu.Unlock()
152+
err:=postWorkspaceAgentAppHealth(ctx, agentsdk.PostAppHealthsRequest{
153+
Healths:lastHealth,
154+
})
155+
iferr!=nil {
156+
logger.Error(ctx,"failed to report workspace app health",slog.Error(err))
157+
}else {
158+
logger.Debug(ctx,"sent workspace app health",slog.F("health",lastHealth))
166159
}
167-
logger.Error(ctx,"failed running workspace app reporter",slog.Error(err))
168-
}
160+
returnnil
161+
},"report")
162+
_=reportTicker.Wait()// only possible error is context done
169163
}
170164
}
171165

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp