Skip to content

fix(agent/agentcontainers): reset error at start of rebuild #18686

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.

Sign up for GitHub

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 1 commit into from
Jul 1, 2025
Merged

fix(agent/agentcontainers): reset error at start of rebuild #18686

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 1 addition & 0 deletions agent/agentcontainers/api.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -946,6 +946,7 @@ func (api *API) handleDevcontainerRecreate(w http.ResponseWriter, r *http.Reques
// devcontainer multiple times in parallel.
dc.Status = codersdk.WorkspaceAgentDevcontainerStatusStarting
dc.Container = nil
dc.Error = ""
api.knownDevcontainers[dc.WorkspaceFolder] = dc
go func() {
_ = api.CreateDevcontainer(dc.WorkspaceFolder, dc.ConfigPath, WithRemoveExistingContainer())
Expand Down
31 changes: 23 additions & 8 deletions agent/agentcontainers/api_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ func (f *fakeContainerCLI) ExecAs(ctx context.Context, name, user string, args .
type fakeDevcontainerCLI struct {
upID string
upErr error
upErrC chan error // If set, send to return err, close to return upErr.
upErrC chan func() error // If set, send to return err, close to return upErr.
execErr error
execErrC chan func(cmd string, args ...string) error // If set, send fn to return err, nil or close to return execErr.
readConfig agentcontainers.DevcontainerConfig
Expand All @@ -82,9 +82,9 @@ func (f *fakeDevcontainerCLI) Up(ctx context.Context, _, _ string, _ ...agentcon
select {
case <-ctx.Done():
return "", ctx.Err()
case err, ok := <-f.upErrC:
case fn, ok := <-f.upErrC:
if ok {
return f.upID, err
return f.upID, fn()
}
}
}
Expand Down Expand Up @@ -613,7 +613,7 @@ func TestAPI(t *testing.T) {
nowRecreateErrorTrap := mClock.Trap().Now("recreate", "errorTimes")
nowRecreateSuccessTrap := mClock.Trap().Now("recreate", "successTimes")

tt.devcontainerCLI.upErrC = make(chan error)
tt.devcontainerCLI.upErrC = make(chan func() error)

// Setup router with the handler under test.
r := chi.NewRouter()
Expand Down Expand Up @@ -1665,7 +1665,7 @@ func TestAPI(t *testing.T) {
mClock = quartz.NewMock(t)
fCCLI = &fakeContainerCLI{arch: "<none>"}
fDCCLI = &fakeDevcontainerCLI{
upErrC: make(chan error, 1),
upErrC: make(chan func() error, 1),
}
fSAC = &fakeSubAgentClient{
logger: logger.Named("fakeSubAgentClient"),
Expand Down Expand Up @@ -1717,7 +1717,7 @@ func TestAPI(t *testing.T) {

// Given: We simulate an error running `devcontainer up`
simulatedError := xerrors.New("simulated error")
testutil.RequireSend(ctx, t, fDCCLI.upErrC, simulatedError)
testutil.RequireSend(ctx, t, fDCCLI.upErrC, func() error { return simulatedError })

nowRecreateErrorTrap.MustWait(ctx).MustRelease(ctx)
nowRecreateErrorTrap.Close()
Expand All @@ -1742,7 +1742,22 @@ func TestAPI(t *testing.T) {
require.Equal(t, http.StatusAccepted, rec.Code)

// Given: We allow `devcontainer up` to succeed.
testutil.RequireSend(ctx, t, fDCCLI.upErrC, nil)
testutil.RequireSend(ctx, t, fDCCLI.upErrC, func() error {
req = httptest.NewRequest(http.MethodGet, "/", nil)
rec = httptest.NewRecorder()
r.ServeHTTP(rec, req)
require.Equal(t, http.StatusOK, rec.Code)

response = codersdk.WorkspaceAgentListContainersResponse{}
err = json.NewDecoder(rec.Body).Decode(&response)
require.NoError(t, err)

// Then: We make sure that the error has been cleared before running up.
require.Len(t, response.Devcontainers, 1)
require.Equal(t, "", response.Devcontainers[0].Error)

return nil
})

nowRecreateSuccessTrap.MustWait(ctx).MustRelease(ctx)
nowRecreateSuccessTrap.Close()
Expand All @@ -1756,7 +1771,7 @@ func TestAPI(t *testing.T) {
err = json.NewDecoder(rec.Body).Decode(&response)
require.NoError(t, err)

// Then: We expect that there will be no error associated with the devcontainer.
// Then: We also expect no error after running up..
require.Len(t, response.Devcontainers, 1)
require.Equal(t, "", response.Devcontainers[0].Error)
})
Expand Down
Loading