-
Notifications
You must be signed in to change notification settings - Fork 927
fix(agent): start devcontainers through agentcontainers package #18471
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
Changes from 24 commits
Commits
Show all changes
25 commits
Select commit
Hold shift + click to select a range
978c871
fix(agent): start devcontainers through agentcontainers package
DanielleMaywood fe99bd6
chore: appease formatter
DanielleMaywood aeae6e2
chore: fix test, appease linter
DanielleMaywood 916f7e8
chore: feedback
DanielleMaywood 53e256b
Merge branch 'main' into dm-devcontainer-log-spam
DanielleMaywood 91bb43a
chore: re-add script timings
DanielleMaywood 81fe11d
fix: change how containerAPI is stored
DanielleMaywood 5c70a8c
Merge branch 'main' into dm-devcontainer-log-spam
DanielleMaywood 8437ca4
chore: appease linter
DanielleMaywood 2c6a2b1
chore: ensure the last log line is printed
DanielleMaywood a512ad4
chore: fix typo
DanielleMaywood c50dc6e
chore: OOPS
DanielleMaywood 4d40ef2
chore: 1 -> 2
DanielleMaywood ce32e2e
chore: add a status to the timings
DanielleMaywood 32ac48a
chore: initialize containerapi even earlier
DanielleMaywood 738b755
chore: only enable when devcontainers are enabled
DanielleMaywood 3714fec
chore: simplify things a little
DanielleMaywood 996d440
chore: recreate -> create with argument
DanielleMaywood ae5dd1e
chore: ensure we close and init
DanielleMaywood 9d76cf6
chore: appease linter
DanielleMaywood 7285c39
chore: mock ReadConfig any time
DanielleMaywood 3fce51c
chore: feedback
DanielleMaywood 54aa84a
chore: feedback
DanielleMaywood 3d08d0e
chore: only set status if not set, and run create in test
DanielleMaywood fa479fc
chore: feedback on poor error message
DanielleMaywood File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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 | ||||
---|---|---|---|---|---|---|
|
@@ -207,6 +207,10 @@ func WithDevcontainers(devcontainers []codersdk.WorkspaceAgentDevcontainer, scri | |||||
api.devcontainerNames = make(map[string]bool, len(devcontainers)) | ||||||
api.devcontainerLogSourceIDs = make(map[string]uuid.UUID) | ||||||
for _, dc := range devcontainers { | ||||||
if dc.Status == "" { | ||||||
dc.Status = codersdk.WorkspaceAgentDevcontainerStatusStarting | ||||||
} | ||||||
|
||||||
api.knownDevcontainers[dc.WorkspaceFolder] = dc | ||||||
api.devcontainerNames[dc.Name] = true | ||||||
for _, script := range scripts { | ||||||
|
@@ -265,8 +269,6 @@ func NewAPI(logger slog.Logger, options ...Option) *API { | |||||
api := &API{ | ||||||
ctx: ctx, | ||||||
cancel: cancel, | ||||||
watcherDone: make(chan struct{}), | ||||||
updaterDone: make(chan struct{}), | ||||||
initialUpdateDone: make(chan struct{}), | ||||||
updateTrigger: make(chan chan error), | ||||||
updateInterval: defaultUpdateInterval, | ||||||
|
@@ -315,10 +317,28 @@ func NewAPI(logger slog.Logger, options ...Option) *API { | |||||
api.subAgentClient.Store(&c) | ||||||
} | ||||||
|
||||||
return api | ||||||
} | ||||||
|
||||||
// Init applies a final set of options to the API and then | ||||||
// begins the watcherLoop and updaterLoop. This function | ||||||
// must only be called once. | ||||||
func (api *API) Init(opts ...Option) { | ||||||
DanielleMaywood marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
api.mu.Lock() | ||||||
defer api.mu.Unlock() | ||||||
if api.closed { | ||||||
return | ||||||
} | ||||||
|
||||||
for _, opt := range opts { | ||||||
DanielleMaywood marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
opt(api) | ||||||
} | ||||||
|
||||||
api.watcherDone = make(chan struct{}) | ||||||
api.updaterDone = make(chan struct{}) | ||||||
|
||||||
go api.watcherLoop() | ||||||
go api.updaterLoop() | ||||||
|
||||||
return api | ||||||
} | ||||||
|
||||||
func (api *API) watcherLoop() { | ||||||
|
@@ -909,8 +929,9 @@ func (api *API) handleDevcontainerRecreate(w http.ResponseWriter, r *http.Reques | |||||
dc.Status = codersdk.WorkspaceAgentDevcontainerStatusStarting | ||||||
dc.Container = nil | ||||||
api.knownDevcontainers[dc.WorkspaceFolder] = dc | ||||||
api.asyncWg.Add(1) | ||||||
go api.recreateDevcontainer(dc, configPath) | ||||||
go func() { | ||||||
_ = api.CreateDevcontainer(dc.WorkspaceFolder, configPath, WithRemoveExistingContainer()) | ||||||
}() | ||||||
|
||||||
api.mu.Unlock() | ||||||
|
||||||
|
@@ -920,15 +941,29 @@ func (api *API) handleDevcontainerRecreate(w http.ResponseWriter, r *http.Reques | |||||
}) | ||||||
} | ||||||
|
||||||
// recreateDevcontainer should run in its own goroutine and is responsible for | ||||||
// createDevcontainer should run in its own goroutine and is responsible for | ||||||
// recreating a devcontainer based on the provided devcontainer configuration. | ||||||
// It updates the devcontainer status and logs the process. The configPath is | ||||||
// passed as a parameter for the odd chance that the container being recreated | ||||||
// has a different config file than the one stored in the devcontainer state. | ||||||
// The devcontainer state must be set to starting and the asyncWg must be | ||||||
// incremented before calling this function. | ||||||
func (api *API) recreateDevcontainer(dc codersdk.WorkspaceAgentDevcontainer, configPath string) { | ||||||
func (api *API) CreateDevcontainer(workspaceFolder, configPath string, opts ...DevcontainerCLIUpOptions) error { | ||||||
api.mu.Lock() | ||||||
if api.closed { | ||||||
api.mu.Unlock() | ||||||
return nil | ||||||
} | ||||||
|
||||||
dc, found := api.knownDevcontainers[workspaceFolder] | ||||||
if !found { | ||||||
api.mu.Unlock() | ||||||
return xerrors.Errorf("no devcontainer found") | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Minor: Just a slightly different signal to not suggest there isn't a single devcontainer. |
||||||
} | ||||||
DanielleMaywood marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
|
||||||
api.asyncWg.Add(1) | ||||||
defer api.asyncWg.Done() | ||||||
api.mu.Unlock() | ||||||
|
||||||
var ( | ||||||
err error | ||||||
|
@@ -969,12 +1004,15 @@ func (api *API) recreateDevcontainer(dc codersdk.WorkspaceAgentDevcontainer, con | |||||
|
||||||
logger.Debug(ctx, "starting devcontainer recreation") | ||||||
|
||||||
_, err = api.dccli.Up(ctx, dc.WorkspaceFolder, configPath, WithUpOutput(infoW, errW), WithRemoveExistingContainer()) | ||||||
upOptions := []DevcontainerCLIUpOptions{WithUpOutput(infoW, errW)} | ||||||
upOptions = append(upOptions, opts...) | ||||||
|
||||||
_, err = api.dccli.Up(ctx, dc.WorkspaceFolder, configPath, upOptions...) | ||||||
if err != nil { | ||||||
// No need to log if the API is closing (context canceled), as this | ||||||
// is expected behavior when the API is shutting down. | ||||||
if !errors.Is(err, context.Canceled) { | ||||||
logger.Error(ctx, "devcontainer recreation failed", slog.Error(err)) | ||||||
logger.Error(ctx, "devcontainer creation failed", slog.Error(err)) | ||||||
} | ||||||
|
||||||
api.mu.Lock() | ||||||
|
@@ -983,10 +1021,11 @@ func (api *API) recreateDevcontainer(dc codersdk.WorkspaceAgentDevcontainer, con | |||||
api.knownDevcontainers[dc.WorkspaceFolder] = dc | ||||||
api.recreateErrorTimes[dc.WorkspaceFolder] = api.clock.Now("agentcontainers", "recreate", "errorTimes") | ||||||
api.mu.Unlock() | ||||||
return | ||||||
|
||||||
return xerrors.Errorf("start devcontainer: %w", err) | ||||||
} | ||||||
|
||||||
logger.Info(ctx, "devcontainer recreated successfully") | ||||||
logger.Info(ctx, "devcontainer created successfully") | ||||||
|
||||||
api.mu.Lock() | ||||||
dc = api.knownDevcontainers[dc.WorkspaceFolder] | ||||||
|
@@ -1009,8 +1048,11 @@ func (api *API) recreateDevcontainer(dc codersdk.WorkspaceAgentDevcontainer, con | |||||
// Ensure an immediate refresh to accurately reflect the | ||||||
// devcontainer state after recreation. | ||||||
if err := api.RefreshContainers(ctx); err != nil { | ||||||
logger.Error(ctx, "failed to trigger immediate refresh after devcontainer recreation", slog.Error(err)) | ||||||
logger.Error(ctx, "failed to trigger immediate refresh after devcontainer creation", slog.Error(err)) | ||||||
return xerrors.Errorf("refresh containers: %w", err) | ||||||
} | ||||||
|
||||||
return nil | ||||||
} | ||||||
|
||||||
// markDevcontainerDirty finds the devcontainer with the given config file path | ||||||
|
@@ -1609,8 +1651,12 @@ func (api *API) Close() error { | |||||
err := api.watcher.Close() | ||||||
|
||||||
// Wait for loops to finish. | ||||||
<-api.watcherDone | ||||||
<-api.updaterDone | ||||||
if api.watcherDone != nil { | ||||||
<-api.watcherDone | ||||||
} | ||||||
if api.updaterDone != nil { | ||||||
<-api.updaterDone | ||||||
} | ||||||
|
||||||
// Wait for all async tasks to complete. | ||||||
api.asyncWg.Wait() | ||||||
|
Oops, something went wrong.
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.