Skip to content

feat: reinitialize agents when a prebuilt workspace is claimed #17475

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 49 commits into from
May 14, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
49 commits
Select commit Hold shift + click to select a range
c09c9b9
WIP: agent reinitialization
SasSwart Apr 21, 2025
476fe71
fix assignment to nil map
SasSwart Apr 21, 2025
8c8bca6
fix: ensure prebuilt workspace agent tokens are reused when a prebuil…
SasSwart Apr 23, 2025
7ce4eea
test agent reinitialization
SasSwart Apr 24, 2025
52ac64e
remove defunct metric
SasSwart Apr 24, 2025
362db7c
Remove todo
SasSwart Apr 25, 2025
dcc7379
test that we trigger workspace agent reinitialization under the right…
SasSwart Apr 28, 2025
ff66b3f
slight improvements to a test
SasSwart Apr 28, 2025
efff5d9
review notes to improve legibility
SasSwart Apr 28, 2025
cebd5db
add an integration test for prebuilt workspace agent reinitialization
SasSwart Apr 29, 2025
2679138
Merge remote-tracking branch 'origin/main' into jjs/prebuilds-agent-r…
SasSwart Apr 29, 2025
9feebef
enable the premium license in a prebuilds integration test
SasSwart Apr 29, 2025
b117b5c
encapsulate WaitForReinitLoop for easier testing
SasSwart Apr 30, 2025
a22b414
introduce unit testable abstraction layers
SasSwart Apr 30, 2025
9bbd2c7
test workspace claim pubsub
SasSwart May 1, 2025
5804201
add tests for agent reinitialization
SasSwart May 1, 2025
7e8dcee
review notes
SasSwart May 1, 2025
725f97b
Merge remote-tracking branch 'origin/main' into jjs/prebuilds-agent-r…
SasSwart May 1, 2025
a9b1567
make fmt lint
SasSwart May 1, 2025
21ee970
remove go mod replace
SasSwart May 1, 2025
e54d7e7
remove defunct logging
SasSwart May 1, 2025
2799858
update dependency on terraform-provider-coder
SasSwart May 2, 2025
1d93003
update dependency on terraform-provider-coder
SasSwart May 2, 2025
763fc12
go mod tidy
SasSwart May 2, 2025
0f879c7
make -B gen
SasSwart May 2, 2025
61784c9
dont require ids to InsertPresetParameters
SasSwart May 2, 2025
604eb27
dont require ids to InsertPresetParameters
SasSwart May 2, 2025
bf4d2cf
fix: set the running agent token
dannykopping May 2, 2025
38b4f0d
fix: use http client without timeout like we do in connectRPCVersion
dannykopping May 5, 2025
20df538
review notes
SasSwart May 6, 2025
4bb3b68
Merge remote-tracking branch 'origin/main' into jjs/prebuilds-agent-r…
SasSwart May 7, 2025
83972db
bump provisionerd proto version
SasSwart May 7, 2025
146b158
fix: fetch the previous agent when we need its token for prebuilt wor…
SasSwart May 12, 2025
5eb16cd
Merge remote-tracking branch 'origin/main' into jjs/prebuilds-agent-r…
SasSwart May 12, 2025
730d803
make -B lint
SasSwart May 12, 2025
150adc0
Test GetWorkspaceAgentsByBuildID
SasSwart May 12, 2025
b4ecf10
Rename GetWorkspaceAgentsByWorkspaceAndBuildNumber
SasSwart May 12, 2025
3fa3edf
make gen
SasSwart May 12, 2025
7e45919
fix a race condition
SasSwart May 12, 2025
a632508
Merge remote-tracking branch 'origin/main' into jjs/prebuilds-agent-r…
SasSwart May 12, 2025
72125ec
Merge remote-tracking branch 'origin/main' into jjs/prebuilds-agent-r…
SasSwart May 13, 2025
b65eea7
fix provisionerdserver test for prebuild claims
SasSwart May 13, 2025
e1339f3
fix race conditions
SasSwart May 13, 2025
c1a8ba6
Merge remote-tracking branch 'origin/main' into jjs/prebuilds-agent-r…
SasSwart May 13, 2025
5363dcc
Make TestReinitializeAgent more robust
SasSwart May 13, 2025
7ad9b6d
fix tests
SasSwart May 14, 2025
394571d
make -B gen
SasSwart May 14, 2025
890747b
remove a potential race in reinitialization testing in TestCompleteJob
SasSwart May 14, 2025
b3870db
fix a potential race in TestReinit
SasSwart May 14, 2025
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
8 changes: 7 additions & 1 deletion agent/agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -368,9 +368,11 @@ func (a *agent) runLoop() {
if ctx.Err() != nil {
// Context canceled errors may come from websocket pings, so we
// don't want to use `errors.Is(err, context.Canceled)` here.
a.logger.Warn(ctx, "runLoop exited with error", slog.Error(ctx.Err()))
return
}
if a.isClosed() {
a.logger.Warn(ctx, "runLoop exited because agent is closed")
return
}
if errors.Is(err, io.EOF) {
Expand Down Expand Up @@ -1051,7 +1053,11 @@ func (a *agent) run() (retErr error) {
return a.statsReporter.reportLoop(ctx, aAPI)
})

return connMan.wait()
err = connMan.wait()
if err != nil {
a.logger.Info(context.Background(), "connection manager errored", slog.Error(err))
}
return err
}

// handleManifest returns a function that fetches and processes the manifest
Expand Down
114 changes: 70 additions & 44 deletions cli/agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ import (
"cdr.dev/slog/sloggers/sloghuman"
"cdr.dev/slog/sloggers/slogjson"
"cdr.dev/slog/sloggers/slogstackdriver"
"github.com/coder/serpent"

"github.com/coder/coder/v2/agent"
"github.com/coder/coder/v2/agent/agentexec"
"github.com/coder/coder/v2/agent/agentssh"
Expand All @@ -33,7 +35,6 @@ import (
"github.com/coder/coder/v2/cli/clilog"
"github.com/coder/coder/v2/codersdk"
"github.com/coder/coder/v2/codersdk/agentsdk"
"github.com/coder/serpent"
)

func (r *RootCmd) workspaceAgent() *serpent.Command {
Expand Down Expand Up @@ -63,8 +64,10 @@ func (r *RootCmd) workspaceAgent() *serpent.Command {
// This command isn't useful to manually execute.
Hidden: true,
Handler: func(inv *serpent.Invocation) error {
ctx, cancel := context.WithCancel(inv.Context())
defer cancel()
ctx, cancel := context.WithCancelCause(inv.Context())
defer func() {
cancel(xerrors.New("agent exited"))
}()

var (
ignorePorts = map[int]string{}
Expand Down Expand Up @@ -281,7 +284,6 @@ func (r *RootCmd) workspaceAgent() *serpent.Command {
return xerrors.Errorf("add executable to $PATH: %w", err)
}

prometheusRegistry := prometheus.NewRegistry()
subsystemsRaw := inv.Environ.Get(agent.EnvAgentSubsystem)
subsystems := []codersdk.AgentSubsystem{}
for _, s := range strings.Split(subsystemsRaw, ",") {
Expand Down Expand Up @@ -325,46 +327,70 @@ func (r *RootCmd) workspaceAgent() *serpent.Command {
logger.Info(ctx, "agent devcontainer detection not enabled")
}

agnt := agent.New(agent.Options{
Client: client,
Logger: logger,
LogDir: logDir,
ScriptDataDir: scriptDataDir,
// #nosec G115 - Safe conversion as tailnet listen port is within uint16 range (0-65535)
TailnetListenPort: uint16(tailnetListenPort),
ExchangeToken: func(ctx context.Context) (string, error) {
if exchangeToken == nil {
return client.SDK.SessionToken(), nil
}
resp, err := exchangeToken(ctx)
if err != nil {
return "", err
}
client.SetSessionToken(resp.SessionToken)
return resp.SessionToken, nil
},
EnvironmentVariables: environmentVariables,
IgnorePorts: ignorePorts,
SSHMaxTimeout: sshMaxTimeout,
Subsystems: subsystems,

PrometheusRegistry: prometheusRegistry,
BlockFileTransfer: blockFileTransfer,
Execer: execer,
SubAgent: subAgent,

ExperimentalDevcontainersEnabled: experimentalDevcontainersEnabled,
})

promHandler := agent.PrometheusMetricsHandler(prometheusRegistry, logger)
prometheusSrvClose := ServeHandler(ctx, logger, promHandler, prometheusAddress, "prometheus")
defer prometheusSrvClose()

debugSrvClose := ServeHandler(ctx, logger, agnt.HTTPDebug(), debugAddress, "debug")
defer debugSrvClose()

<-ctx.Done()
return agnt.Close()
reinitEvents := agentsdk.WaitForReinitLoop(ctx, logger, client)

var (
lastErr error
mustExit bool
)
for {
prometheusRegistry := prometheus.NewRegistry()

agnt := agent.New(agent.Options{
Client: client,
Logger: logger,
LogDir: logDir,
ScriptDataDir: scriptDataDir,
// #nosec G115 - Safe conversion as tailnet listen port is within uint16 range (0-65535)
TailnetListenPort: uint16(tailnetListenPort),
ExchangeToken: func(ctx context.Context) (string, error) {
if exchangeToken == nil {
return client.SDK.SessionToken(), nil
}
resp, err := exchangeToken(ctx)
if err != nil {
return "", err
}
client.SetSessionToken(resp.SessionToken)
return resp.SessionToken, nil
},
EnvironmentVariables: environmentVariables,
IgnorePorts: ignorePorts,
SSHMaxTimeout: sshMaxTimeout,
Subsystems: subsystems,

PrometheusRegistry: prometheusRegistry,
BlockFileTransfer: blockFileTransfer,
Execer: execer,
SubAgent: subAgent,
ExperimentalDevcontainersEnabled: experimentalDevcontainersEnabled,
})

promHandler := agent.PrometheusMetricsHandler(prometheusRegistry, logger)
prometheusSrvClose := ServeHandler(ctx, logger, promHandler, prometheusAddress, "prometheus")

debugSrvClose := ServeHandler(ctx, logger, agnt.HTTPDebug(), debugAddress, "debug")

select {
case <-ctx.Done():
logger.Info(ctx, "agent shutting down", slog.Error(context.Cause(ctx)))
mustExit = true
case event := <-reinitEvents:
logger.Info(ctx, "agent received instruction to reinitialize",
slog.F("workspace_id", event.WorkspaceID), slog.F("reason", event.Reason))
}

lastErr = agnt.Close()
debugSrvClose()
prometheusSrvClose()

if mustExit {
break
}

logger.Info(ctx, "agent reinitializing")
}
return lastErr
},
}

Expand Down
45 changes: 45 additions & 0 deletions coderd/apidoc/docs.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

37 changes: 37 additions & 0 deletions coderd/apidoc/swagger.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 3 additions & 1 deletion coderd/coderd.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ import (
"sync/atomic"
"time"

"github.com/coder/coder/v2/coderd/prebuilds"

"github.com/andybalholm/brotli"
"github.com/go-chi/chi/v5"
"github.com/go-chi/chi/v5/middleware"
Expand Down Expand Up @@ -46,7 +48,6 @@ import (
"github.com/coder/coder/v2/coderd/entitlements"
"github.com/coder/coder/v2/coderd/files"
"github.com/coder/coder/v2/coderd/idpsync"
"github.com/coder/coder/v2/coderd/prebuilds"
"github.com/coder/coder/v2/coderd/runtimeconfig"
"github.com/coder/coder/v2/coderd/webpush"

Expand Down Expand Up @@ -1299,6 +1300,7 @@ func New(options *Options) *API {
r.Get("/external-auth", api.workspaceAgentsExternalAuth)
r.Get("/gitsshkey", api.agentGitSSHKey)
r.Post("/log-source", api.workspaceAgentPostLogSource)
r.Get("/reinit", api.workspaceAgentReinit)
})
r.Route("/{workspaceagent}", func(r chi.Router) {
r.Use(
Expand Down
63 changes: 63 additions & 0 deletions coderd/coderdtest/coderdtest.go
Original file line number Diff line number Diff line change
Expand Up @@ -1105,6 +1105,69 @@ func (w WorkspaceAgentWaiter) MatchResources(m func([]codersdk.WorkspaceResource
return w
}

// WaitForAgentFn represents a boolean assertion to be made against each agent
// that a given WorkspaceAgentWaited knows about. Each WaitForAgentFn should apply
// the check to a single agent, but it should be named for plural, because `func (w WorkspaceAgentWaiter) WaitFor`
// applies the check to all agents that it is aware of. This ensures that the public API of the waiter
// reads correctly. For example:
//
// waiter := coderdtest.NewWorkspaceAgentWaiter(t, client, r.Workspace.ID)
// waiter.WaitFor(coderdtest.AgentsReady)
type WaitForAgentFn func(agent codersdk.WorkspaceAgent) bool

// AgentsReady checks that the latest lifecycle state of an agent is "Ready".
func AgentsReady(agent codersdk.WorkspaceAgent) bool {
return agent.LifecycleState == codersdk.WorkspaceAgentLifecycleReady
}

// AgentsNotReady checks that the latest lifecycle state of an agent is anything except "Ready".
func AgentsNotReady(agent codersdk.WorkspaceAgent) bool {
return !AgentsReady(agent)
}

func (w WorkspaceAgentWaiter) WaitFor(criteria ...WaitForAgentFn) {
w.t.Helper()

agentNamesMap := make(map[string]struct{}, len(w.agentNames))
for _, name := range w.agentNames {
agentNamesMap[name] = struct{}{}
}

ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
defer cancel()

w.t.Logf("waiting for workspace agents (workspace %s)", w.workspaceID)
require.Eventually(w.t, func() bool {
var err error
workspace, err := w.client.Workspace(ctx, w.workspaceID)
if err != nil {
return false
}
if workspace.LatestBuild.Job.CompletedAt == nil {
return false
}
if workspace.LatestBuild.Job.CompletedAt.IsZero() {
return false
}

for _, resource := range workspace.LatestBuild.Resources {
for _, agent := range resource.Agents {
if len(w.agentNames) > 0 {
if _, ok := agentNamesMap[agent.Name]; !ok {
continue
}
}
for _, criterium := range criteria {
if !criterium(agent) {
return false
}
}
}
}
return true
}, testutil.WaitLong, testutil.IntervalMedium)
}

// Wait waits for the agent(s) to connect and fails the test if they do not within testutil.WaitLong
func (w WorkspaceAgentWaiter) Wait() []codersdk.WorkspaceResource {
w.t.Helper()
Expand Down
9 changes: 9 additions & 0 deletions coderd/database/dbauthz/dbauthz.go
Original file line number Diff line number Diff line change
Expand Up @@ -3020,6 +3020,15 @@ func (q *querier) GetWorkspaceAgentsByResourceIDs(ctx context.Context, ids []uui
return q.db.GetWorkspaceAgentsByResourceIDs(ctx, ids)
}

func (q *querier) GetWorkspaceAgentsByWorkspaceAndBuildNumber(ctx context.Context, arg database.GetWorkspaceAgentsByWorkspaceAndBuildNumberParams) ([]database.WorkspaceAgent, error) {
_, err := q.GetWorkspaceByID(ctx, arg.WorkspaceID)
if err != nil {
return nil, err
}

return q.db.GetWorkspaceAgentsByWorkspaceAndBuildNumber(ctx, arg)
}

func (q *querier) GetWorkspaceAgentsCreatedAfter(ctx context.Context, createdAt time.Time) ([]database.WorkspaceAgent, error) {
if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceSystem); err != nil {
return nil, err
Expand Down
Loading
Loading