Skip to content

chore: add agentapi tests #11269

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Jan 26, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions coderd/agentapi/activitybump.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,13 +41,14 @@ func ActivityBumpWorkspace(ctx context.Context, log slog.Logger, db database.Sto
// low priority operations fail first.
ctx, cancel := context.WithTimeout(ctx, time.Second*15)
defer cancel()
if err := db.ActivityBumpWorkspace(ctx, database.ActivityBumpWorkspaceParams{
err := db.ActivityBumpWorkspace(ctx, database.ActivityBumpWorkspaceParams{
NextAutostart: nextAutostart.UTC(),
WorkspaceID: workspaceID,
}); err != nil {
})
if err != nil {
if !xerrors.Is(err, context.Canceled) && !database.IsQueryCanceledError(err) {
// Bump will fail if the context is canceled, but this is ok.
log.Error(ctx, "bump failed", slog.Error(err),
log.Error(ctx, "activity bump failed", slog.Error(err),
slog.F("workspace_id", workspaceID),
)
}
Expand Down
54 changes: 21 additions & 33 deletions coderd/agentapi/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,13 @@ import (

"cdr.dev/slog"
agentproto "github.com/coder/coder/v2/agent/proto"
"github.com/coder/coder/v2/coderd/batchstats"
"github.com/coder/coder/v2/coderd/database"
"github.com/coder/coder/v2/coderd/database/pubsub"
"github.com/coder/coder/v2/coderd/externalauth"
"github.com/coder/coder/v2/coderd/prometheusmetrics"
"github.com/coder/coder/v2/coderd/schedule"
"github.com/coder/coder/v2/coderd/tracing"
"github.com/coder/coder/v2/codersdk/agentsdk"
"github.com/coder/coder/v2/tailnet"
)

const AgentAPIVersionDRPC = "2.0"
Expand Down Expand Up @@ -58,21 +56,18 @@ type Options struct {
Database database.Store
Pubsub pubsub.Pubsub
DerpMapFn func() *tailcfg.DERPMap
TailnetCoordinator *atomic.Pointer[tailnet.Coordinator]
TemplateScheduleStore *atomic.Pointer[schedule.TemplateScheduleStore]
StatsBatcher *batchstats.Batcher
StatsBatcher StatsBatcher // *batchstats.Batcher
PublishWorkspaceUpdateFn func(ctx context.Context, workspaceID uuid.UUID)
PublishWorkspaceAgentLogsUpdateFn func(ctx context.Context, workspaceAgentID uuid.UUID, msg agentsdk.LogsNotifyMessage)

AccessURL *url.URL
AppHostname string
AgentInactiveDisconnectTimeout time.Duration
AgentFallbackTroubleshootingURL string
AgentStatsRefreshInterval time.Duration
DisableDirectConnections bool
DerpForceWebSockets bool
DerpMapUpdateFrequency time.Duration
ExternalAuthConfigs []*externalauth.Config
AccessURL *url.URL
AppHostname string
AgentStatsRefreshInterval time.Duration
DisableDirectConnections bool
DerpForceWebSockets bool
DerpMapUpdateFrequency time.Duration
ExternalAuthConfigs []*externalauth.Config

// Optional:
// WorkspaceID avoids a future lookup to find the workspace ID by setting
Expand All @@ -89,17 +84,15 @@ func New(opts Options) *API {
}

api.ManifestAPI = &ManifestAPI{
AccessURL: opts.AccessURL,
AppHostname: opts.AppHostname,
AgentInactiveDisconnectTimeout: opts.AgentInactiveDisconnectTimeout,
AgentFallbackTroubleshootingURL: opts.AgentFallbackTroubleshootingURL,
ExternalAuthConfigs: opts.ExternalAuthConfigs,
DisableDirectConnections: opts.DisableDirectConnections,
DerpForceWebSockets: opts.DerpForceWebSockets,
AgentFn: api.agent,
Database: opts.Database,
DerpMapFn: opts.DerpMapFn,
TailnetCoordinator: opts.TailnetCoordinator,
AccessURL: opts.AccessURL,
AppHostname: opts.AppHostname,
ExternalAuthConfigs: opts.ExternalAuthConfigs,
DisableDirectConnections: opts.DisableDirectConnections,
DerpForceWebSockets: opts.DerpForceWebSockets,
AgentFn: api.agent,
WorkspaceIDFn: api.workspaceID,
Database: opts.Database,
DerpMapFn: opts.DerpMapFn,
}

api.ServiceBannerAPI = &ServiceBannerAPI{
Expand Down Expand Up @@ -207,20 +200,15 @@ func (a *API) workspaceID(ctx context.Context, agent *database.WorkspaceAgent) (
agent = &agnt
}

resource, err := a.opts.Database.GetWorkspaceResourceByID(ctx, agent.ResourceID)
if err != nil {
return uuid.Nil, xerrors.Errorf("get workspace agent resource by id %q: %w", agent.ResourceID, err)
}

build, err := a.opts.Database.GetWorkspaceBuildByJobID(ctx, resource.JobID)
getWorkspaceAgentByIDRow, err := a.opts.Database.GetWorkspaceByAgentID(ctx, agent.ID)
if err != nil {
return uuid.Nil, xerrors.Errorf("get workspace build by job id %q: %w", resource.JobID, err)
return uuid.Nil, xerrors.Errorf("get workspace by agent id %q: %w", agent.ID, err)
}

a.mu.Lock()
a.cachedWorkspaceID = build.WorkspaceID
a.cachedWorkspaceID = getWorkspaceAgentByIDRow.Workspace.ID
a.mu.Unlock()
return build.WorkspaceID, nil
return getWorkspaceAgentByIDRow.Workspace.ID, nil
}

func (a *API) publishWorkspaceUpdate(ctx context.Context, agent *database.WorkspaceAgent) error {
Expand Down
8 changes: 5 additions & 3 deletions coderd/agentapi/apps.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,9 +90,11 @@ func (a *AppsAPI) BatchUpdateAppHealths(ctx context.Context, req *agentproto.Bat
}
}

err = a.PublishWorkspaceUpdateFn(ctx, &workspaceAgent)
if err != nil {
return nil, xerrors.Errorf("publish workspace update: %w", err)
if a.PublishWorkspaceUpdateFn != nil && len(newApps) > 0 {
err = a.PublishWorkspaceUpdateFn(ctx, &workspaceAgent)
if err != nil {
return nil, xerrors.Errorf("publish workspace update: %w", err)
}
}
return &agentproto.BatchUpdateAppHealthResponse{}, nil
}
253 changes: 253 additions & 0 deletions coderd/agentapi/apps_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,253 @@
package agentapi_test

import (
"context"
"sync/atomic"
"testing"

"github.com/golang/mock/gomock"
"github.com/google/uuid"
"github.com/stretchr/testify/require"

"cdr.dev/slog/sloggers/slogtest"

agentproto "github.com/coder/coder/v2/agent/proto"
"github.com/coder/coder/v2/coderd/agentapi"
"github.com/coder/coder/v2/coderd/database"
"github.com/coder/coder/v2/coderd/database/dbmock"
)

func TestBatchUpdateAppHealths(t *testing.T) {
t.Parallel()

var (
agent = database.WorkspaceAgent{
ID: uuid.New(),
}
app1 = database.WorkspaceApp{
ID: uuid.New(),
AgentID: agent.ID,
Slug: "code-server-1",
DisplayName: "code-server 1",
HealthcheckUrl: "http://localhost:3000",
Health: database.WorkspaceAppHealthInitializing,
}
app2 = database.WorkspaceApp{
ID: uuid.New(),
AgentID: agent.ID,
Slug: "code-server-2",
DisplayName: "code-server 2",
HealthcheckUrl: "http://localhost:3001",
Health: database.WorkspaceAppHealthHealthy,
}
)

t.Run("OK", func(t *testing.T) {
t.Parallel()

dbM := dbmock.NewMockStore(gomock.NewController(t))
dbM.EXPECT().GetWorkspaceAppsByAgentID(gomock.Any(), agent.ID).Return([]database.WorkspaceApp{app1, app2}, nil)
dbM.EXPECT().UpdateWorkspaceAppHealthByID(gomock.Any(), database.UpdateWorkspaceAppHealthByIDParams{
ID: app1.ID,
Health: database.WorkspaceAppHealthHealthy,
}).Return(nil)
dbM.EXPECT().UpdateWorkspaceAppHealthByID(gomock.Any(), database.UpdateWorkspaceAppHealthByIDParams{
ID: app2.ID,
Health: database.WorkspaceAppHealthUnhealthy,
}).Return(nil)

var publishCalled int64
api := &agentapi.AppsAPI{
AgentFn: func(context.Context) (database.WorkspaceAgent, error) {
return agent, nil
},
Database: dbM,
Log: slogtest.Make(t, nil),
PublishWorkspaceUpdateFn: func(ctx context.Context, wa *database.WorkspaceAgent) error {
atomic.AddInt64(&publishCalled, 1)
return nil
},
}

// Set both to healthy, only one should be updated in the DB.
resp, err := api.BatchUpdateAppHealths(context.Background(), &agentproto.BatchUpdateAppHealthRequest{
Updates: []*agentproto.BatchUpdateAppHealthRequest_HealthUpdate{
{
Id: app1.ID[:],
Health: agentproto.AppHealth_HEALTHY,
},
{
Id: app2.ID[:],
Health: agentproto.AppHealth_UNHEALTHY,
},
},
})
require.NoError(t, err)
require.Equal(t, &agentproto.BatchUpdateAppHealthResponse{}, resp)

require.EqualValues(t, 1, atomic.LoadInt64(&publishCalled))
})

t.Run("Unchanged", func(t *testing.T) {
t.Parallel()

dbM := dbmock.NewMockStore(gomock.NewController(t))
dbM.EXPECT().GetWorkspaceAppsByAgentID(gomock.Any(), agent.ID).Return([]database.WorkspaceApp{app1, app2}, nil)

var publishCalled int64
api := &agentapi.AppsAPI{
AgentFn: func(context.Context) (database.WorkspaceAgent, error) {
return agent, nil
},
Database: dbM,
Log: slogtest.Make(t, nil),
PublishWorkspaceUpdateFn: func(ctx context.Context, wa *database.WorkspaceAgent) error {
atomic.AddInt64(&publishCalled, 1)
return nil
},
}

// Set both to their current status, neither should be updated in the
// DB.
resp, err := api.BatchUpdateAppHealths(context.Background(), &agentproto.BatchUpdateAppHealthRequest{
Updates: []*agentproto.BatchUpdateAppHealthRequest_HealthUpdate{
{
Id: app1.ID[:],
Health: agentproto.AppHealth_INITIALIZING,
},
{
Id: app2.ID[:],
Health: agentproto.AppHealth_HEALTHY,
},
},
})
require.NoError(t, err)
require.Equal(t, &agentproto.BatchUpdateAppHealthResponse{}, resp)

require.EqualValues(t, 0, atomic.LoadInt64(&publishCalled))
})

t.Run("Empty", func(t *testing.T) {
t.Parallel()

// No DB queries are made if there are no updates to process.
dbM := dbmock.NewMockStore(gomock.NewController(t))

var publishCalled int64
api := &agentapi.AppsAPI{
AgentFn: func(context.Context) (database.WorkspaceAgent, error) {
return agent, nil
},
Database: dbM,
Log: slogtest.Make(t, nil),
PublishWorkspaceUpdateFn: func(ctx context.Context, wa *database.WorkspaceAgent) error {
atomic.AddInt64(&publishCalled, 1)
return nil
},
}

// Do nothing.
resp, err := api.BatchUpdateAppHealths(context.Background(), &agentproto.BatchUpdateAppHealthRequest{
Updates: []*agentproto.BatchUpdateAppHealthRequest_HealthUpdate{},
})
require.NoError(t, err)
require.Equal(t, &agentproto.BatchUpdateAppHealthResponse{}, resp)

require.EqualValues(t, 0, atomic.LoadInt64(&publishCalled))
})

t.Run("AppNoHealthcheck", func(t *testing.T) {
t.Parallel()

app3 := database.WorkspaceApp{
ID: uuid.New(),
AgentID: agent.ID,
Slug: "code-server-3",
DisplayName: "code-server 3",
}

dbM := dbmock.NewMockStore(gomock.NewController(t))
dbM.EXPECT().GetWorkspaceAppsByAgentID(gomock.Any(), agent.ID).Return([]database.WorkspaceApp{app3}, nil)

api := &agentapi.AppsAPI{
AgentFn: func(context.Context) (database.WorkspaceAgent, error) {
return agent, nil
},
Database: dbM,
Log: slogtest.Make(t, nil),
PublishWorkspaceUpdateFn: nil,
}

// Set app3 to healthy, should error.
resp, err := api.BatchUpdateAppHealths(context.Background(), &agentproto.BatchUpdateAppHealthRequest{
Updates: []*agentproto.BatchUpdateAppHealthRequest_HealthUpdate{
{
Id: app3.ID[:],
Health: agentproto.AppHealth_HEALTHY,
},
},
})
require.Error(t, err)
require.ErrorContains(t, err, "does not have healthchecks enabled")
require.Nil(t, resp)
})

t.Run("UnknownApp", func(t *testing.T) {
t.Parallel()

dbM := dbmock.NewMockStore(gomock.NewController(t))
dbM.EXPECT().GetWorkspaceAppsByAgentID(gomock.Any(), agent.ID).Return([]database.WorkspaceApp{app1, app2}, nil)

api := &agentapi.AppsAPI{
AgentFn: func(context.Context) (database.WorkspaceAgent, error) {
return agent, nil
},
Database: dbM,
Log: slogtest.Make(t, nil),
PublishWorkspaceUpdateFn: nil,
}

// Set an unknown app to healthy, should error.
id := uuid.New()
resp, err := api.BatchUpdateAppHealths(context.Background(), &agentproto.BatchUpdateAppHealthRequest{
Updates: []*agentproto.BatchUpdateAppHealthRequest_HealthUpdate{
{
Id: id[:],
Health: agentproto.AppHealth_HEALTHY,
},
},
})
require.Error(t, err)
require.ErrorContains(t, err, "not found")
require.Nil(t, resp)
})

t.Run("InvalidHealth", func(t *testing.T) {
t.Parallel()

dbM := dbmock.NewMockStore(gomock.NewController(t))
dbM.EXPECT().GetWorkspaceAppsByAgentID(gomock.Any(), agent.ID).Return([]database.WorkspaceApp{app1, app2}, nil)

api := &agentapi.AppsAPI{
AgentFn: func(context.Context) (database.WorkspaceAgent, error) {
return agent, nil
},
Database: dbM,
Log: slogtest.Make(t, nil),
PublishWorkspaceUpdateFn: nil,
}

// Set an unknown app to healthy, should error.
resp, err := api.BatchUpdateAppHealths(context.Background(), &agentproto.BatchUpdateAppHealthRequest{
Updates: []*agentproto.BatchUpdateAppHealthRequest_HealthUpdate{
{
Id: app1.ID[:],
Health: -999,
},
},
})
require.Error(t, err)
require.ErrorContains(t, err, "unknown health status")
require.Nil(t, resp)
})
}
Loading