- Notifications
You must be signed in to change notification settings - Fork1.1k
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Merged
Uh oh!
There was an error while loading.Please reload this page.
Merged
Changes from6 commits
Commits
Show all changes
49 commits Select commitHold shift + click to select a range
c09c9b9 WIP: agent reinitialization
SasSwart476fe71 fix assignment to nil map
SasSwart8c8bca6 fix: ensure prebuilt workspace agent tokens are reused when a prebuil…
SasSwart7ce4eea test agent reinitialization
SasSwart52ac64e remove defunct metric
SasSwart362db7c Remove todo
SasSwartdcc7379 test that we trigger workspace agent reinitialization under the right…
SasSwartff66b3f slight improvements to a test
SasSwartefff5d9 review notes to improve legibility
SasSwartcebd5db add an integration test for prebuilt workspace agent reinitialization
SasSwart2679138 Merge remote-tracking branch 'origin/main' into jjs/prebuilds-agent-r…
SasSwart9feebef enable the premium license in a prebuilds integration test
SasSwartb117b5c encapsulate WaitForReinitLoop for easier testing
SasSwarta22b414 introduce unit testable abstraction layers
SasSwart9bbd2c7 test workspace claim pubsub
SasSwart5804201 add tests for agent reinitialization
SasSwart7e8dcee review notes
SasSwart725f97b Merge remote-tracking branch 'origin/main' into jjs/prebuilds-agent-r…
SasSwarta9b1567 make fmt lint
SasSwart21ee970 remove go mod replace
SasSwarte54d7e7 remove defunct logging
SasSwart2799858 update dependency on terraform-provider-coder
SasSwart1d93003 update dependency on terraform-provider-coder
SasSwart763fc12 go mod tidy
SasSwart0f879c7 make -B gen
SasSwart61784c9 dont require ids to InsertPresetParameters
SasSwart604eb27 dont require ids to InsertPresetParameters
SasSwartbf4d2cf fix: set the running agent token
dannykopping38b4f0d fix: use http client without timeout like we do in connectRPCVersion
dannykopping20df538 review notes
SasSwart4bb3b68 Merge remote-tracking branch 'origin/main' into jjs/prebuilds-agent-r…
SasSwart83972db bump provisionerd proto version
SasSwart146b158 fix: fetch the previous agent when we need its token for prebuilt wor…
SasSwart5eb16cd Merge remote-tracking branch 'origin/main' into jjs/prebuilds-agent-r…
SasSwart730d803 make -B lint
SasSwart150adc0 Test GetWorkspaceAgentsByBuildID
SasSwartb4ecf10 Rename GetWorkspaceAgentsByWorkspaceAndBuildNumber
SasSwart3fa3edf make gen
SasSwart7e45919 fix a race condition
SasSwarta632508 Merge remote-tracking branch 'origin/main' into jjs/prebuilds-agent-r…
SasSwart72125ec Merge remote-tracking branch 'origin/main' into jjs/prebuilds-agent-r…
SasSwartb65eea7 fix provisionerdserver test for prebuild claims
SasSwarte1339f3 fix race conditions
SasSwartc1a8ba6 Merge remote-tracking branch 'origin/main' into jjs/prebuilds-agent-r…
SasSwart5363dcc Make TestReinitializeAgent more robust
SasSwart7ad9b6d fix tests
SasSwart394571d make -B gen
SasSwart890747b remove a potential race in reinitialization testing in TestCompleteJob
SasSwartb3870db fix a potential race in TestReinit
SasSwartFile filter
Filter by extension
Conversations
Failed to load comments.
Loading
Uh oh!
There was an error while loading.Please reload this page.
Jump to
Jump to file
Failed to load files.
Loading
Uh oh!
There was an error while loading.Please reload this page.
Diff view
Diff view
There are no files selected for viewing
12 changes: 10 additions & 2 deletionsagent/agent.go
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -36,6 +36,9 @@ import ( | ||
| "tailscale.com/util/clientmetric" | ||
| "cdr.dev/slog" | ||
| "github.com/coder/retry" | ||
SasSwart marked this conversation as resolved. OutdatedShow resolvedHide resolvedUh oh!There was an error while loading.Please reload this page. | ||
| "github.com/coder/clistat" | ||
| "github.com/coder/coder/v2/agent/agentcontainers" | ||
| "github.com/coder/coder/v2/agent/agentexec" | ||
| @@ -53,7 +56,6 @@ import ( | ||
| "github.com/coder/coder/v2/tailnet" | ||
| tailnetproto "github.com/coder/coder/v2/tailnet/proto" | ||
| "github.com/coder/quartz" | ||
| ) | ||
| const ( | ||
| @@ -365,9 +367,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) { | ||
| @@ -1048,7 +1052,11 @@ func (a *agent) run() (retErr error) { | ||
| return a.statsReporter.reportLoop(ctx, aAPI) | ||
| }) | ||
| err = connMan.wait() | ||
| if err != nil { | ||
| a.logger.Warn(context.Background(), "connection manager errored", slog.Error(err)) | ||
SasSwart marked this conversation as resolved. OutdatedShow resolvedHide resolvedUh oh!There was an error while loading.Please reload this page. | ||
| } | ||
| return err | ||
| } | ||
| // handleManifest returns a function that fetches and processes the manifest | ||
5 changes: 5 additions & 0 deletionsagent/reaper/reaper_unix.go
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
130 changes: 89 additions & 41 deletionscli/agent.go
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -19,12 +19,16 @@ import ( | ||
| "golang.org/x/xerrors" | ||
| "gopkg.in/natefinch/lumberjack.v2" | ||
| "github.com/coder/retry" | ||
| "github.com/prometheus/client_golang/prometheus" | ||
| "cdr.dev/slog" | ||
| "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/agentcontainers" | ||
| "github.com/coder/coder/v2/agent/agentexec" | ||
| @@ -34,7 +38,6 @@ import ( | ||
| "github.com/coder/coder/v2/cli/clilog" | ||
| "github.com/coder/coder/v2/codersdk" | ||
| "github.com/coder/coder/v2/codersdk/agentsdk" | ||
| ) | ||
| func (r *RootCmd) workspaceAgent() *serpent.Command { | ||
| @@ -63,8 +66,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.WithCancelCause(inv.Context()) | ||
| defer func() { | ||
| cancel(xerrors.New("defer")) | ||
SasSwart marked this conversation as resolved. OutdatedShow resolvedHide resolvedUh oh!There was an error while loading.Please reload this page. | ||
| }() | ||
| var ( | ||
| ignorePorts = map[int]string{} | ||
| @@ -281,7 +286,6 @@ func (r *RootCmd) workspaceAgent() *serpent.Command { | ||
| return xerrors.Errorf("add executable to $PATH: %w", err) | ||
| } | ||
| subsystemsRaw := inv.Environ.Get(agent.EnvAgentSubsystem) | ||
| subsystems := []codersdk.AgentSubsystem{} | ||
| for _, s := range strings.Split(subsystemsRaw, ",") { | ||
| @@ -328,46 +332,90 @@ func (r *RootCmd) workspaceAgent() *serpent.Command { | ||
| containerLister = agentcontainers.NewDocker(execer) | ||
| } | ||
| // TODO: timeout ok? | ||
| reinitCtx, reinitCancel := context.WithTimeout(context.Background(), time.Hour*24) | ||
| defer reinitCancel() | ||
| reinitEvents := make(chan agentsdk.ReinitializationResponse) | ||
| go func() { | ||
| // Retry to wait for reinit, main context cancels the retrier. | ||
| for retrier := retry.New(100*time.Millisecond, 10*time.Second); retrier.Wait(ctx); { | ||
| select { | ||
| case <-reinitCtx.Done(): | ||
| return | ||
| default: | ||
| } | ||
| err := client.WaitForReinit(reinitCtx, reinitEvents) | ||
| if err != nil { | ||
| logger.Error(ctx, "failed to wait for reinit instructions, will retry",slog.Error(err)) | ||
| } | ||
| } | ||
| }() | ||
| 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, | ||
| ContainerLister: containerLister, | ||
| 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.Warn(ctx, "agent shutting down", slog.Error(ctx.Err()), slog.F("cause", context.Cause(ctx))) | ||
| mustExit = true | ||
| case event := <-reinitEvents: | ||
| logger.Warn(ctx, "agent received instruction to reinitialize", | ||
| slog.F("message", event.Message), slog.F("reason", event.Reason)) | ||
| } | ||
| lastErr = agnt.Close() | ||
| debugSrvClose() | ||
| prometheusSrvClose() | ||
| if mustExit { | ||
| reinitCancel() | ||
| break | ||
| } | ||
| logger.Info(ctx, "agent reinitializing") | ||
| } | ||
| return lastErr | ||
| }, | ||
| } | ||
52 changes: 52 additions & 0 deletionscli/agent_test.go
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
2 changes: 2 additions & 0 deletionscoderd/agentapi/api.go
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -109,6 +109,8 @@ func New(opts Options) *API { | ||
| Database: opts.Database, | ||
| DerpMapFn: opts.DerpMapFn, | ||
| WorkspaceID: opts.WorkspaceID, | ||
| Log: opts.Log.Named("manifests"), | ||
SasSwart marked this conversation as resolved. OutdatedShow resolvedHide resolvedUh oh!There was an error while loading.Please reload this page. | ||
| Pubsub: opts.Pubsub, | ||
| } | ||
| api.AnnouncementBannerAPI = &AnnouncementBannerAPI{ | ||
6 changes: 6 additions & 0 deletionscoderd/agentapi/manifest.go
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -8,6 +8,10 @@ import ( | ||
| "strings" | ||
| "time" | ||
| "cdr.dev/slog" | ||
| "github.com/coder/coder/v2/coderd/database/pubsub" | ||
| "github.com/google/uuid" | ||
| "golang.org/x/sync/errgroup" | ||
| "golang.org/x/xerrors" | ||
| @@ -35,6 +39,8 @@ type ManifestAPI struct { | ||
| AgentFn func(context.Context) (database.WorkspaceAgent, error) | ||
| Database database.Store | ||
| DerpMapFn func() *tailcfg.DERPMap | ||
| Pubsub pubsub.Pubsub | ||
SasSwart marked this conversation as resolved. OutdatedShow resolvedHide resolvedUh oh!There was an error while loading.Please reload this page. | ||
| Log slog.Logger | ||
| } | ||
| func (a *ManifestAPI) GetManifest(ctx context.Context, _ *agentproto.GetManifestRequest) (*agentproto.Manifest, error) { | ||
45 changes: 45 additions & 0 deletionscoderd/apidoc/docs.go
Some generated files are not rendered by default. Learn more abouthow customized files appear on GitHub.
Oops, something went wrong.
Uh oh!
There was an error while loading.Please reload this page.
Oops, something went wrong.
Uh oh!
There was an error while loading.Please reload this page.
Oops, something went wrong.
Uh oh!
There was an error while loading.Please reload this page.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.