Skip to content

Commit 2970709

Browse files
deansheatherdeansheather
authored
chore: add agentapi tests (#11269)
1 parent 541154b commit 2970709

18 files changed

+2506
-159
lines changed

‎coderd/agentapi/activitybump.go

+4-3
Original file line numberDiff line numberDiff line change
@@ -41,13 +41,14 @@ func ActivityBumpWorkspace(ctx context.Context, log slog.Logger, db database.Sto
4141
// low priority operations fail first.
4242
ctx, cancel := context.WithTimeout(ctx, time.Second*15)
4343
defer cancel()
44-
if err := db.ActivityBumpWorkspace(ctx, database.ActivityBumpWorkspaceParams{
44+
err := db.ActivityBumpWorkspace(ctx, database.ActivityBumpWorkspaceParams{
4545
NextAutostart: nextAutostart.UTC(),
4646
WorkspaceID: workspaceID,
47-
}); err != nil {
47+
})
48+
if err != nil {
4849
if !xerrors.Is(err, context.Canceled) && !database.IsQueryCanceledError(err) {
4950
// Bump will fail if the context is canceled, but this is ok.
50-
log.Error(ctx, "bump failed", slog.Error(err),
51+
log.Error(ctx, "activity bump failed", slog.Error(err),
5152
slog.F("workspace_id", workspaceID),
5253
)
5354
}

‎coderd/agentapi/api.go

+20-31
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ import (
1717

1818
"cdr.dev/slog"
1919
agentproto "github.com/coder/coder/v2/agent/proto"
20-
"github.com/coder/coder/v2/coderd/batchstats"
2120
"github.com/coder/coder/v2/coderd/database"
2221
"github.com/coder/coder/v2/coderd/database/pubsub"
2322
"github.com/coder/coder/v2/coderd/externalauth"
@@ -61,19 +60,17 @@ type Options struct {
6160
DerpMapFn func() *tailcfg.DERPMap
6261
TailnetCoordinator *atomic.Pointer[tailnet.Coordinator]
6362
TemplateScheduleStore *atomic.Pointer[schedule.TemplateScheduleStore]
64-
StatsBatcher *batchstats.Batcher
63+
StatsBatcher StatsBatcher
6564
PublishWorkspaceUpdateFn func(ctx context.Context, workspaceID uuid.UUID)
6665
PublishWorkspaceAgentLogsUpdateFn func(ctx context.Context, workspaceAgentID uuid.UUID, msg agentsdk.LogsNotifyMessage)
6766

68-
AccessURL *url.URL
69-
AppHostname string
70-
AgentInactiveDisconnectTimeout time.Duration
71-
AgentFallbackTroubleshootingURL string
72-
AgentStatsRefreshInterval time.Duration
73-
DisableDirectConnections bool
74-
DerpForceWebSockets bool
75-
DerpMapUpdateFrequency time.Duration
76-
ExternalAuthConfigs []*externalauth.Config
67+
AccessURL *url.URL
68+
AppHostname string
69+
AgentStatsRefreshInterval time.Duration
70+
DisableDirectConnections bool
71+
DerpForceWebSockets bool
72+
DerpMapUpdateFrequency time.Duration
73+
ExternalAuthConfigs []*externalauth.Config
7774

7875
// Optional:
7976
// WorkspaceID avoids a future lookup to find the workspace ID by setting
@@ -90,17 +87,14 @@ func New(opts Options) *API {
9087
}
9188

9289
api.ManifestAPI = &ManifestAPI{
93-
AccessURL: opts.AccessURL,
94-
AppHostname: opts.AppHostname,
95-
AgentInactiveDisconnectTimeout: opts.AgentInactiveDisconnectTimeout,
96-
AgentFallbackTroubleshootingURL: opts.AgentFallbackTroubleshootingURL,
97-
ExternalAuthConfigs: opts.ExternalAuthConfigs,
98-
DisableDirectConnections: opts.DisableDirectConnections,
99-
DerpForceWebSockets: opts.DerpForceWebSockets,
100-
AgentFn: api.agent,
101-
Database: opts.Database,
102-
DerpMapFn: opts.DerpMapFn,
103-
TailnetCoordinator: opts.TailnetCoordinator,
90+
AccessURL: opts.AccessURL,
91+
AppHostname: opts.AppHostname,
92+
ExternalAuthConfigs: opts.ExternalAuthConfigs,
93+
DisableDirectConnections: opts.DisableDirectConnections,
94+
DerpForceWebSockets: opts.DerpForceWebSockets,
95+
AgentFn: api.agent,
96+
Database: opts.Database,
97+
DerpMapFn: opts.DerpMapFn,
10498
}
10599

106100
api.ServiceBannerAPI = &ServiceBannerAPI{
@@ -214,20 +208,15 @@ func (a *API) workspaceID(ctx context.Context, agent *database.WorkspaceAgent) (
214208
agent = &agnt
215209
}
216210

217-
resource, err := a.opts.Database.GetWorkspaceResourceByID(ctx, agent.ResourceID)
218-
if err != nil {
219-
return uuid.Nil, xerrors.Errorf("get workspace agent resource by id %q: %w", agent.ResourceID, err)
220-
}
221-
222-
build, err := a.opts.Database.GetWorkspaceBuildByJobID(ctx, resource.JobID)
211+
getWorkspaceAgentByIDRow, err := a.opts.Database.GetWorkspaceByAgentID(ctx, agent.ID)
223212
if err != nil {
224-
return uuid.Nil, xerrors.Errorf("get workspace build by job id %q: %w", resource.JobID, err)
213+
return uuid.Nil, xerrors.Errorf("get workspace by agent id %q: %w", agent.ID, err)
225214
}
226215

227216
a.mu.Lock()
228-
a.cachedWorkspaceID = build.WorkspaceID
217+
a.cachedWorkspaceID = getWorkspaceAgentByIDRow.Workspace.ID
229218
a.mu.Unlock()
230-
return build.WorkspaceID, nil
219+
return getWorkspaceAgentByIDRow.Workspace.ID, nil
231220
}
232221

233222
func (a *API) publishWorkspaceUpdate(ctx context.Context, agent *database.WorkspaceAgent) error {

‎coderd/agentapi/apps.go

+5-3
Original file line numberDiff line numberDiff line change
@@ -90,9 +90,11 @@ func (a *AppsAPI) BatchUpdateAppHealths(ctx context.Context, req *agentproto.Bat
9090
}
9191
}
9292

93-
err = a.PublishWorkspaceUpdateFn(ctx, &workspaceAgent)
94-
if err != nil {
95-
return nil, xerrors.Errorf("publish workspace update: %w", err)
93+
if a.PublishWorkspaceUpdateFn != nil && len(newApps) > 0 {
94+
err = a.PublishWorkspaceUpdateFn(ctx, &workspaceAgent)
95+
if err != nil {
96+
return nil, xerrors.Errorf("publish workspace update: %w", err)
97+
}
9698
}
9799
return &agentproto.BatchUpdateAppHealthResponse{}, nil
98100
}

‎coderd/agentapi/apps_test.go

+252
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,252 @@
1+
package agentapi_test
2+
3+
import (
4+
"context"
5+
"testing"
6+
7+
"github.com/google/uuid"
8+
"github.com/stretchr/testify/require"
9+
"go.uber.org/mock/gomock"
10+
11+
"cdr.dev/slog/sloggers/slogtest"
12+
13+
agentproto "github.com/coder/coder/v2/agent/proto"
14+
"github.com/coder/coder/v2/coderd/agentapi"
15+
"github.com/coder/coder/v2/coderd/database"
16+
"github.com/coder/coder/v2/coderd/database/dbmock"
17+
)
18+
19+
func TestBatchUpdateAppHealths(t *testing.T) {
20+
t.Parallel()
21+
22+
var (
23+
agent = database.WorkspaceAgent{
24+
ID: uuid.New(),
25+
}
26+
app1 = database.WorkspaceApp{
27+
ID: uuid.New(),
28+
AgentID: agent.ID,
29+
Slug: "code-server-1",
30+
DisplayName: "code-server 1",
31+
HealthcheckUrl: "http://localhost:3000",
32+
Health: database.WorkspaceAppHealthInitializing,
33+
}
34+
app2 = database.WorkspaceApp{
35+
ID: uuid.New(),
36+
AgentID: agent.ID,
37+
Slug: "code-server-2",
38+
DisplayName: "code-server 2",
39+
HealthcheckUrl: "http://localhost:3001",
40+
Health: database.WorkspaceAppHealthHealthy,
41+
}
42+
)
43+
44+
t.Run("OK", func(t *testing.T) {
45+
t.Parallel()
46+
47+
dbM := dbmock.NewMockStore(gomock.NewController(t))
48+
dbM.EXPECT().GetWorkspaceAppsByAgentID(gomock.Any(), agent.ID).Return([]database.WorkspaceApp{app1, app2}, nil)
49+
dbM.EXPECT().UpdateWorkspaceAppHealthByID(gomock.Any(), database.UpdateWorkspaceAppHealthByIDParams{
50+
ID: app1.ID,
51+
Health: database.WorkspaceAppHealthHealthy,
52+
}).Return(nil)
53+
dbM.EXPECT().UpdateWorkspaceAppHealthByID(gomock.Any(), database.UpdateWorkspaceAppHealthByIDParams{
54+
ID: app2.ID,
55+
Health: database.WorkspaceAppHealthUnhealthy,
56+
}).Return(nil)
57+
58+
publishCalled := false
59+
api := &agentapi.AppsAPI{
60+
AgentFn: func(context.Context) (database.WorkspaceAgent, error) {
61+
return agent, nil
62+
},
63+
Database: dbM,
64+
Log: slogtest.Make(t, nil),
65+
PublishWorkspaceUpdateFn: func(ctx context.Context, wa *database.WorkspaceAgent) error {
66+
publishCalled = true
67+
return nil
68+
},
69+
}
70+
71+
// Set one to healthy, set another to unhealthy.
72+
resp, err := api.BatchUpdateAppHealths(context.Background(), &agentproto.BatchUpdateAppHealthRequest{
73+
Updates: []*agentproto.BatchUpdateAppHealthRequest_HealthUpdate{
74+
{
75+
Id: app1.ID[:],
76+
Health: agentproto.AppHealth_HEALTHY,
77+
},
78+
{
79+
Id: app2.ID[:],
80+
Health: agentproto.AppHealth_UNHEALTHY,
81+
},
82+
},
83+
})
84+
require.NoError(t, err)
85+
require.Equal(t, &agentproto.BatchUpdateAppHealthResponse{}, resp)
86+
87+
require.True(t, publishCalled)
88+
})
89+
90+
t.Run("Unchanged", func(t *testing.T) {
91+
t.Parallel()
92+
93+
dbM := dbmock.NewMockStore(gomock.NewController(t))
94+
dbM.EXPECT().GetWorkspaceAppsByAgentID(gomock.Any(), agent.ID).Return([]database.WorkspaceApp{app1, app2}, nil)
95+
96+
publishCalled := false
97+
api := &agentapi.AppsAPI{
98+
AgentFn: func(context.Context) (database.WorkspaceAgent, error) {
99+
return agent, nil
100+
},
101+
Database: dbM,
102+
Log: slogtest.Make(t, nil),
103+
PublishWorkspaceUpdateFn: func(ctx context.Context, wa *database.WorkspaceAgent) error {
104+
publishCalled = true
105+
return nil
106+
},
107+
}
108+
109+
// Set both to their current status, neither should be updated in the
110+
// DB.
111+
resp, err := api.BatchUpdateAppHealths(context.Background(), &agentproto.BatchUpdateAppHealthRequest{
112+
Updates: []*agentproto.BatchUpdateAppHealthRequest_HealthUpdate{
113+
{
114+
Id: app1.ID[:],
115+
Health: agentproto.AppHealth_INITIALIZING,
116+
},
117+
{
118+
Id: app2.ID[:],
119+
Health: agentproto.AppHealth_HEALTHY,
120+
},
121+
},
122+
})
123+
require.NoError(t, err)
124+
require.Equal(t, &agentproto.BatchUpdateAppHealthResponse{}, resp)
125+
126+
require.False(t, publishCalled)
127+
})
128+
129+
t.Run("Empty", func(t *testing.T) {
130+
t.Parallel()
131+
132+
// No DB queries are made if there are no updates to process.
133+
dbM := dbmock.NewMockStore(gomock.NewController(t))
134+
135+
publishCalled := false
136+
api := &agentapi.AppsAPI{
137+
AgentFn: func(context.Context) (database.WorkspaceAgent, error) {
138+
return agent, nil
139+
},
140+
Database: dbM,
141+
Log: slogtest.Make(t, nil),
142+
PublishWorkspaceUpdateFn: func(ctx context.Context, wa *database.WorkspaceAgent) error {
143+
publishCalled = true
144+
return nil
145+
},
146+
}
147+
148+
// Do nothing.
149+
resp, err := api.BatchUpdateAppHealths(context.Background(), &agentproto.BatchUpdateAppHealthRequest{
150+
Updates: []*agentproto.BatchUpdateAppHealthRequest_HealthUpdate{},
151+
})
152+
require.NoError(t, err)
153+
require.Equal(t, &agentproto.BatchUpdateAppHealthResponse{}, resp)
154+
155+
require.False(t, publishCalled)
156+
})
157+
158+
t.Run("AppNoHealthcheck", func(t *testing.T) {
159+
t.Parallel()
160+
161+
app3 := database.WorkspaceApp{
162+
ID: uuid.New(),
163+
AgentID: agent.ID,
164+
Slug: "code-server-3",
165+
DisplayName: "code-server 3",
166+
}
167+
168+
dbM := dbmock.NewMockStore(gomock.NewController(t))
169+
dbM.EXPECT().GetWorkspaceAppsByAgentID(gomock.Any(), agent.ID).Return([]database.WorkspaceApp{app3}, nil)
170+
171+
api := &agentapi.AppsAPI{
172+
AgentFn: func(context.Context) (database.WorkspaceAgent, error) {
173+
return agent, nil
174+
},
175+
Database: dbM,
176+
Log: slogtest.Make(t, nil),
177+
PublishWorkspaceUpdateFn: nil,
178+
}
179+
180+
// Set app3 to healthy, should error.
181+
resp, err := api.BatchUpdateAppHealths(context.Background(), &agentproto.BatchUpdateAppHealthRequest{
182+
Updates: []*agentproto.BatchUpdateAppHealthRequest_HealthUpdate{
183+
{
184+
Id: app3.ID[:],
185+
Health: agentproto.AppHealth_HEALTHY,
186+
},
187+
},
188+
})
189+
require.Error(t, err)
190+
require.ErrorContains(t, err, "does not have healthchecks enabled")
191+
require.Nil(t, resp)
192+
})
193+
194+
t.Run("UnknownApp", func(t *testing.T) {
195+
t.Parallel()
196+
197+
dbM := dbmock.NewMockStore(gomock.NewController(t))
198+
dbM.EXPECT().GetWorkspaceAppsByAgentID(gomock.Any(), agent.ID).Return([]database.WorkspaceApp{app1, app2}, nil)
199+
200+
api := &agentapi.AppsAPI{
201+
AgentFn: func(context.Context) (database.WorkspaceAgent, error) {
202+
return agent, nil
203+
},
204+
Database: dbM,
205+
Log: slogtest.Make(t, nil),
206+
PublishWorkspaceUpdateFn: nil,
207+
}
208+
209+
// Set an unknown app to healthy, should error.
210+
id := uuid.New()
211+
resp, err := api.BatchUpdateAppHealths(context.Background(), &agentproto.BatchUpdateAppHealthRequest{
212+
Updates: []*agentproto.BatchUpdateAppHealthRequest_HealthUpdate{
213+
{
214+
Id: id[:],
215+
Health: agentproto.AppHealth_HEALTHY,
216+
},
217+
},
218+
})
219+
require.Error(t, err)
220+
require.ErrorContains(t, err, "not found")
221+
require.Nil(t, resp)
222+
})
223+
224+
t.Run("InvalidHealth", func(t *testing.T) {
225+
t.Parallel()
226+
227+
dbM := dbmock.NewMockStore(gomock.NewController(t))
228+
dbM.EXPECT().GetWorkspaceAppsByAgentID(gomock.Any(), agent.ID).Return([]database.WorkspaceApp{app1, app2}, nil)
229+
230+
api := &agentapi.AppsAPI{
231+
AgentFn: func(context.Context) (database.WorkspaceAgent, error) {
232+
return agent, nil
233+
},
234+
Database: dbM,
235+
Log: slogtest.Make(t, nil),
236+
PublishWorkspaceUpdateFn: nil,
237+
}
238+
239+
// Set an unknown app to healthy, should error.
240+
resp, err := api.BatchUpdateAppHealths(context.Background(), &agentproto.BatchUpdateAppHealthRequest{
241+
Updates: []*agentproto.BatchUpdateAppHealthRequest_HealthUpdate{
242+
{
243+
Id: app1.ID[:],
244+
Health: -999,
245+
},
246+
},
247+
})
248+
require.Error(t, err)
249+
require.ErrorContains(t, err, "unknown health status")
250+
require.Nil(t, resp)
251+
})
252+
}

0 commit comments

Comments
 (0)