Skip to content

Commit 2fc3064

Browse files
deansheatherdeansheather
authored
chore: add tests for app ID copy in app healths (#12088)
1 parent 06254a1 commit 2fc3064

File tree

4 files changed

+79
-23
lines changed

4 files changed

+79
-23
lines changed

‎agent/agenttest/client.go

+15-8
Original file line numberDiff line numberDiff line change
@@ -197,9 +197,10 @@ type FakeAgentAPI struct {
197197
t testing.TB
198198
logger slog.Logger
199199

200-
manifest *agentproto.Manifest
201-
startupCh chan *agentproto.Startup
202-
statsCh chan *agentproto.Stats
200+
manifest *agentproto.Manifest
201+
startupCh chan *agentproto.Startup
202+
statsCh chan *agentproto.Stats
203+
appHealthCh chan *agentproto.BatchUpdateAppHealthRequest
203204

204205
getServiceBannerFunc func() (codersdk.ServiceBannerConfig, error)
205206
}
@@ -244,9 +245,14 @@ func (*FakeAgentAPI) UpdateLifecycle(context.Context, *agentproto.UpdateLifecycl
244245

245246
func (f *FakeAgentAPI) BatchUpdateAppHealths(ctx context.Context, req *agentproto.BatchUpdateAppHealthRequest) (*agentproto.BatchUpdateAppHealthResponse, error) {
246247
f.logger.Debug(ctx, "batch update app health", slog.F("req", req))
248+
f.appHealthCh <- req
247249
return &agentproto.BatchUpdateAppHealthResponse{}, nil
248250
}
249251

252+
func (f *FakeAgentAPI) AppHealthCh() <-chan *agentproto.BatchUpdateAppHealthRequest {
253+
return f.appHealthCh
254+
}
255+
250256
func (f *FakeAgentAPI) UpdateStartup(_ context.Context, req *agentproto.UpdateStartupRequest) (*agentproto.Startup, error) {
251257
f.startupCh <- req.GetStartup()
252258
return req.GetStartup(), nil
@@ -264,10 +270,11 @@ func (*FakeAgentAPI) BatchCreateLogs(context.Context, *agentproto.BatchCreateLog
264270

265271
func NewFakeAgentAPI(t testing.TB, logger slog.Logger, manifest *agentproto.Manifest, statsCh chan *agentproto.Stats) *FakeAgentAPI {
266272
return &FakeAgentAPI{
267-
t: t,
268-
logger: logger.Named("FakeAgentAPI"),
269-
manifest: manifest,
270-
statsCh: statsCh,
271-
startupCh: make(chan *agentproto.Startup, 100),
273+
t: t,
274+
logger: logger.Named("FakeAgentAPI"),
275+
manifest: manifest,
276+
statsCh: statsCh,
277+
startupCh: make(chan *agentproto.Startup, 100),
278+
appHealthCh: make(chan *agentproto.BatchUpdateAppHealthRequest, 100),
272279
}
273280
}

‎agent/apphealth_test.go

+56-14
Original file line numberDiff line numberDiff line change
@@ -4,16 +4,21 @@ import (
44
"context"
55
"net/http"
66
"net/http/httptest"
7+
"strings"
78
"sync"
89
"sync/atomic"
910
"testing"
1011
"time"
1112

13+
"github.com/google/uuid"
14+
"github.com/stretchr/testify/assert"
1215
"github.com/stretchr/testify/require"
1316

1417
"cdr.dev/slog"
1518
"cdr.dev/slog/sloggers/slogtest"
1619
"github.com/coder/coder/v2/agent"
20+
"github.com/coder/coder/v2/agent/agenttest"
21+
"github.com/coder/coder/v2/agent/proto"
1722
"github.com/coder/coder/v2/coderd/httpapi"
1823
"github.com/coder/coder/v2/codersdk"
1924
"github.com/coder/coder/v2/codersdk/agentsdk"
@@ -40,12 +45,23 @@ func TestAppHealth_Healthy(t *testing.T) {
4045
},
4146
Health: codersdk.WorkspaceAppHealthInitializing,
4247
},
48+
{
49+
Slug: "app3",
50+
Healthcheck: codersdk.Healthcheck{
51+
Interval: 2,
52+
Threshold: 1,
53+
},
54+
Health: codersdk.WorkspaceAppHealthInitializing,
55+
},
4356
}
4457
handlers := []http.Handler{
4558
nil,
4659
http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
4760
httpapi.Write(r.Context(), w, http.StatusOK, nil)
4861
}),
62+
http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
63+
httpapi.Write(r.Context(), w, http.StatusOK, nil)
64+
}),
4965
}
5066
getApps, closeFn := setupAppReporter(ctx, t, apps, handlers)
5167
defer closeFn()
@@ -58,7 +74,7 @@ func TestAppHealth_Healthy(t *testing.T) {
5874
return false
5975
}
6076

61-
return apps[1].Health == codersdk.WorkspaceAppHealthHealthy
77+
return apps[1].Health == codersdk.WorkspaceAppHealthHealthy && apps[2].Health == codersdk.WorkspaceAppHealthHealthy
6278
}, testutil.WaitLong, testutil.IntervalSlow)
6379
}
6480

@@ -163,6 +179,12 @@ func TestAppHealth_NotSpamming(t *testing.T) {
163179

164180
func setupAppReporter(ctx context.Context, t *testing.T, apps []codersdk.WorkspaceApp, handlers []http.Handler) (agent.WorkspaceAgentApps, func()) {
165181
closers := []func(){}
182+
for i, app := range apps {
183+
if app.ID == uuid.Nil {
184+
app.ID = uuid.New()
185+
apps[i] = app
186+
}
187+
}
166188
for i, handler := range handlers {
167189
if handler == nil {
168190
continue
@@ -181,23 +203,43 @@ func setupAppReporter(ctx context.Context, t *testing.T, apps []codersdk.Workspa
181203
var newApps []codersdk.WorkspaceApp
182204
return append(newApps, apps...), nil
183205
}
184-
postWorkspaceAgentAppHealth := func(_ context.Context, req agentsdk.PostAppHealthsRequest) error {
185-
mu.Lock()
186-
for id, health := range req.Healths {
187-
for i, app := range apps {
188-
if app.ID != id {
189-
continue
206+
207+
// We don't care about manifest or stats in this test since it's not using
208+
// a full agent and these RPCs won't get called.
209+
//
210+
// We use a proper fake agent API so we can test the conversion code and the
211+
// request code as well. Before we were bypassing these by using a custom
212+
// post function.
213+
fakeAAPI := agenttest.NewFakeAgentAPI(t, slogtest.Make(t, nil), nil, nil)
214+
215+
// Process events from the channel and update the health of the apps.
216+
go func() {
217+
appHealthCh := fakeAAPI.AppHealthCh()
218+
for {
219+
select {
220+
case <-ctx.Done():
221+
return
222+
case req := <-appHealthCh:
223+
mu.Lock()
224+
for _, update := range req.Updates {
225+
updateID, err := uuid.FromBytes(update.Id)
226+
assert.NoError(t, err)
227+
updateHealth := codersdk.WorkspaceAppHealth(strings.ToLower(proto.AppHealth_name[int32(update.Health)]))
228+
229+
for i, app := range apps {
230+
if app.ID != updateID {
231+
continue
232+
}
233+
app.Health = updateHealth
234+
apps[i] = app
235+
}
190236
}
191-
app.Health = health
192-
apps[i] = app
237+
mu.Unlock()
193238
}
194239
}
195-
mu.Unlock()
196-
197-
return nil
198-
}
240+
}()
199241

200-
go agent.NewWorkspaceAppHealthReporter(slogtest.Make(t, nil).Leveled(slog.LevelDebug), apps, postWorkspaceAgentAppHealth)(ctx)
242+
go agent.NewWorkspaceAppHealthReporter(slogtest.Make(t, nil).Leveled(slog.LevelDebug), apps, agentsdk.AppHealthPoster(fakeAAPI))(ctx)
201243

202244
return workspaceAgentApps, func() {
203245
for _, closeFn := range closers {

‎codersdk/agentsdk/agentsdk.go

+6-1
Original file line numberDiff line numberDiff line change
@@ -226,7 +226,12 @@ type PostAppHealthsRequest struct {
226226
Healths map[uuid.UUID]codersdk.WorkspaceAppHealth
227227
}
228228

229-
func AppHealthPoster(aAPI proto.DRPCAgentClient) func(ctx context.Context, req PostAppHealthsRequest) error {
229+
// BatchUpdateAppHealthsClient is a partial interface of proto.DRPCAgentClient.
230+
type BatchUpdateAppHealthsClient interface {
231+
BatchUpdateAppHealths(ctx context.Context, req *proto.BatchUpdateAppHealthRequest) (*proto.BatchUpdateAppHealthResponse, error)
232+
}
233+
234+
func AppHealthPoster(aAPI BatchUpdateAppHealthsClient) func(ctx context.Context, req PostAppHealthsRequest) error {
230235
return func(ctx context.Context, req PostAppHealthsRequest) error {
231236
pReq, err := ProtoFromAppHealthsRequest(req)
232237
if err != nil {

‎codersdk/agentsdk/convert.go

+2
Original file line numberDiff line numberDiff line change
@@ -287,6 +287,8 @@ func ProtoFromAppHealthsRequest(req PostAppHealthsRequest) (*proto.BatchUpdateAp
287287
return nil, xerrors.Errorf("unknown app health: %s", h)
288288
}
289289

290+
// Copy the ID, otherwise all updates will have the same ID (the last
291+
// one in the list).
290292
var idCopy uuid.UUID
291293
copy(idCopy[:], id[:])
292294
pReq.Updates = append(pReq.Updates, &proto.BatchUpdateAppHealthRequest_HealthUpdate{

0 commit comments

Comments
 (0)