Skip to content

Commit a18eb9d

Browse files
mafredrimafredri
authored
feat(site): allow recreating devcontainers and showing dirty status (coder#18049)
This change allows showing the devcontainer dirty status in the UI as well as a recreate button to update the devcontainer. Closes coder#16424
1 parent 23d1423 commit a18eb9d

File tree

14 files changed

+275
-46
lines changed

14 files changed

+275
-46
lines changed

‎agent/agent_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2226,7 +2226,7 @@ func TestAgent_DevcontainerRecreate(t *testing.T) {
22262226
// devcontainer, we do it in a goroutine so we can process logs
22272227
// concurrently.
22282228
go func(container codersdk.WorkspaceAgentContainer) {
2229-
err := conn.RecreateDevcontainer(ctx, container.ID)
2229+
_, err := conn.RecreateDevcontainer(ctx, container.ID)
22302230
assert.NoError(t, err, "recreate devcontainer should succeed")
22312231
}(container)
22322232

‎agent/agentcontainers/api.go

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -403,6 +403,7 @@ func (api *API) processUpdatedContainersLocked(ctx context.Context, updated code
403403
// Check if the container is running and update the known devcontainers.
404404
for i := range updated.Containers {
405405
container := &updated.Containers[i] // Grab a reference to the container to allow mutating it.
406+
container.DevcontainerStatus = "" // Reset the status for the container (updated later).
406407
container.DevcontainerDirty = false // Reset dirty state for the container (updated later).
407408

408409
workspaceFolder := container.Labels[DevcontainerLocalFolderLabel]
@@ -465,16 +466,25 @@ func (api *API) processUpdatedContainersLocked(ctx context.Context, updated code
465466
for _, dc := range api.knownDevcontainers {
466467
switch {
467468
case dc.Status == codersdk.WorkspaceAgentDevcontainerStatusStarting:
469+
if dc.Container != nil {
470+
dc.Container.DevcontainerStatus = dc.Status
471+
dc.Container.DevcontainerDirty = dc.Dirty
472+
}
468473
continue // This state is handled by the recreation routine.
469474

470475
case dc.Status == codersdk.WorkspaceAgentDevcontainerStatusError && (dc.Container == nil || dc.Container.CreatedAt.Before(api.recreateErrorTimes[dc.WorkspaceFolder])):
476+
if dc.Container != nil {
477+
dc.Container.DevcontainerStatus = dc.Status
478+
dc.Container.DevcontainerDirty = dc.Dirty
479+
}
471480
continue // The devcontainer needs to be recreated.
472481

473482
case dc.Container != nil:
474483
dc.Status = codersdk.WorkspaceAgentDevcontainerStatusStopped
475484
if dc.Container.Running {
476485
dc.Status = codersdk.WorkspaceAgentDevcontainerStatusRunning
477486
}
487+
dc.Container.DevcontainerStatus = dc.Status
478488

479489
dc.Dirty = false
480490
if lastModified, hasModTime := api.configFileModifiedTimes[dc.ConfigPath]; hasModTime && dc.Container.CreatedAt.Before(lastModified) {
@@ -608,6 +618,9 @@ func (api *API) handleDevcontainerRecreate(w http.ResponseWriter, r *http.Reques
608618
// Update the status so that we don't try to recreate the
609619
// devcontainer multiple times in parallel.
610620
dc.Status = codersdk.WorkspaceAgentDevcontainerStatusStarting
621+
if dc.Container != nil {
622+
dc.Container.DevcontainerStatus = dc.Status
623+
}
611624
api.knownDevcontainers[dc.WorkspaceFolder] = dc
612625
api.recreateWg.Add(1)
613626
go api.recreateDevcontainer(dc, configPath)
@@ -680,6 +693,9 @@ func (api *API) recreateDevcontainer(dc codersdk.WorkspaceAgentDevcontainer, con
680693
api.mu.Lock()
681694
dc = api.knownDevcontainers[dc.WorkspaceFolder]
682695
dc.Status = codersdk.WorkspaceAgentDevcontainerStatusError
696+
if dc.Container != nil {
697+
dc.Container.DevcontainerStatus = dc.Status
698+
}
683699
api.knownDevcontainers[dc.WorkspaceFolder] = dc
684700
api.recreateErrorTimes[dc.WorkspaceFolder] = api.clock.Now("recreate", "errorTimes")
685701
api.mu.Unlock()
@@ -695,10 +711,12 @@ func (api *API) recreateDevcontainer(dc codersdk.WorkspaceAgentDevcontainer, con
695711
// allows the update routine to update the devcontainer status, but
696712
// to minimize the time between API consistency, we guess the status
697713
// based on the container state.
698-
if dc.Container != nil && dc.Container.Running {
699-
dc.Status = codersdk.WorkspaceAgentDevcontainerStatusRunning
700-
} else {
701-
dc.Status = codersdk.WorkspaceAgentDevcontainerStatusStopped
714+
dc.Status = codersdk.WorkspaceAgentDevcontainerStatusStopped
715+
if dc.Container != nil {
716+
if dc.Container.Running {
717+
dc.Status = codersdk.WorkspaceAgentDevcontainerStatusRunning
718+
}
719+
dc.Container.DevcontainerStatus = dc.Status
702720
}
703721
dc.Dirty = false
704722
api.recreateSuccessTimes[dc.WorkspaceFolder] = api.clock.Now("recreate", "successTimes")

‎agent/agentcontainers/api_test.go

Lines changed: 31 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -477,6 +477,8 @@ func TestAPI(t *testing.T) {
477477
require.NoError(t, err, "unmarshal response failed")
478478
require.Len(t, resp.Devcontainers, 1, "expected one devcontainer in response")
479479
assert.Equal(t, codersdk.WorkspaceAgentDevcontainerStatusStarting, resp.Devcontainers[0].Status, "devcontainer is not starting")
480+
require.NotNil(t, resp.Devcontainers[0].Container, "devcontainer should have container reference")
481+
assert.Equal(t, codersdk.WorkspaceAgentDevcontainerStatusStarting, resp.Devcontainers[0].Container.DevcontainerStatus, "container dc status is not starting")
480482

481483
// Allow the devcontainer CLI to continue the up process.
482484
close(tt.devcontainerCLI.continueUp)
@@ -503,6 +505,8 @@ func TestAPI(t *testing.T) {
503505
require.NoError(t, err, "unmarshal response failed after error")
504506
require.Len(t, resp.Devcontainers, 1, "expected one devcontainer in response after error")
505507
assert.Equal(t, codersdk.WorkspaceAgentDevcontainerStatusError, resp.Devcontainers[0].Status, "devcontainer is not in an error state after up failure")
508+
require.NotNil(t, resp.Devcontainers[0].Container, "devcontainer should have container reference after up failure")
509+
assert.Equal(t, codersdk.WorkspaceAgentDevcontainerStatusError, resp.Devcontainers[0].Container.DevcontainerStatus, "container dc status is not error after up failure")
506510
return
507511
}
508512

@@ -525,7 +529,9 @@ func TestAPI(t *testing.T) {
525529
err = json.NewDecoder(rec.Body).Decode(&resp)
526530
require.NoError(t, err, "unmarshal response failed after recreation")
527531
require.Len(t, resp.Devcontainers, 1, "expected one devcontainer in response after recreation")
528-
assert.Equal(t, codersdk.WorkspaceAgentDevcontainerStatusRunning, resp.Devcontainers[0].Status, "devcontainer is not stopped after recreation")
532+
assert.Equal(t, codersdk.WorkspaceAgentDevcontainerStatusRunning, resp.Devcontainers[0].Status, "devcontainer is not running after recreation")
533+
require.NotNil(t, resp.Devcontainers[0].Container, "devcontainer should have container reference after recreation")
534+
assert.Equal(t, codersdk.WorkspaceAgentDevcontainerStatusRunning, resp.Devcontainers[0].Container.DevcontainerStatus, "container dc status is not running after recreation")
529535
})
530536
}
531537
})
@@ -620,6 +626,7 @@ func TestAPI(t *testing.T) {
620626
assert.Equal(t, codersdk.WorkspaceAgentDevcontainerStatusRunning, dc.Status)
621627
require.NotNil(t, dc.Container)
622628
assert.Equal(t, "runtime-container-1", dc.Container.ID)
629+
assert.Equal(t, codersdk.WorkspaceAgentDevcontainerStatusRunning, dc.Container.DevcontainerStatus)
623630
},
624631
},
625632
{
@@ -660,12 +667,14 @@ func TestAPI(t *testing.T) {
660667
assert.Equal(t, codersdk.WorkspaceAgentDevcontainerStatusStopped, known2.Status)
661668
assert.Equal(t, codersdk.WorkspaceAgentDevcontainerStatusRunning, runtime1.Status)
662669

663-
require.NotNil(t, known1.Container)
664670
assert.Nil(t, known2.Container)
665-
require.NotNil(t, runtime1.Container)
666671

672+
require.NotNil(t, known1.Container)
667673
assert.Equal(t, "known-container-1", known1.Container.ID)
674+
assert.Equal(t, codersdk.WorkspaceAgentDevcontainerStatusRunning, known1.Container.DevcontainerStatus)
675+
require.NotNil(t, runtime1.Container)
668676
assert.Equal(t, "runtime-container-1", runtime1.Container.ID)
677+
assert.Equal(t, codersdk.WorkspaceAgentDevcontainerStatusRunning, runtime1.Container.DevcontainerStatus)
669678
},
670679
},
671680
{
@@ -704,10 +713,12 @@ func TestAPI(t *testing.T) {
704713
assert.Equal(t, codersdk.WorkspaceAgentDevcontainerStatusStopped, nonRunning.Status)
705714

706715
require.NotNil(t, running.Container, "running container should have container reference")
707-
require.NotNil(t, nonRunning.Container, "non-running container should have container reference")
708-
709716
assert.Equal(t, "running-container", running.Container.ID)
717+
assert.Equal(t, codersdk.WorkspaceAgentDevcontainerStatusRunning, running.Container.DevcontainerStatus)
718+
719+
require.NotNil(t, nonRunning.Container, "non-running container should have container reference")
710720
assert.Equal(t, "non-running-container", nonRunning.Container.ID)
721+
assert.Equal(t, codersdk.WorkspaceAgentDevcontainerStatusStopped, nonRunning.Container.DevcontainerStatus)
711722
},
712723
},
713724
{
@@ -743,6 +754,7 @@ func TestAPI(t *testing.T) {
743754
assert.NotEmpty(t, dc2.ConfigPath)
744755
require.NotNil(t, dc2.Container)
745756
assert.Equal(t, "known-container-2", dc2.Container.ID)
757+
assert.Equal(t, codersdk.WorkspaceAgentDevcontainerStatusRunning, dc2.Container.DevcontainerStatus)
746758
},
747759
},
748760
{
@@ -811,9 +823,14 @@ func TestAPI(t *testing.T) {
811823

812824
logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}).Leveled(slog.LevelDebug)
813825

826+
mClock := quartz.NewMock(t)
827+
mClock.Set(time.Now()).MustWait(testutil.Context(t, testutil.WaitShort))
828+
tickerTrap := mClock.Trap().TickerFunc("updaterLoop")
829+
814830
// Setup router with the handler under test.
815831
r := chi.NewRouter()
816832
apiOptions := []agentcontainers.Option{
833+
agentcontainers.WithClock(mClock),
817834
agentcontainers.WithLister(tt.lister),
818835
agentcontainers.WithWatcher(watcher.NewNoop()),
819836
}
@@ -838,6 +855,15 @@ func TestAPI(t *testing.T) {
838855

839856
ctx := testutil.Context(t, testutil.WaitShort)
840857

858+
// Make sure the ticker function has been registered
859+
// before advancing the clock.
860+
tickerTrap.MustWait(ctx).MustRelease(ctx)
861+
tickerTrap.Close()
862+
863+
// Advance the clock to run the updater loop.
864+
_, aw := mClock.AdvanceNext()
865+
aw.MustWait(ctx)
866+
841867
req := httptest.NewRequest(http.MethodGet, "/devcontainers", nil).
842868
WithContext(ctx)
843869
rec := httptest.NewRecorder()

‎coderd/apidoc/docs.go

Lines changed: 23 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/apidoc/swagger.json

Lines changed: 18 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/workspaceagents.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -956,7 +956,7 @@ func (api *API) workspaceAgentRecreateDevcontainer(rw http.ResponseWriter, r *ht
956956
}
957957
defer release()
958958

959-
err = agentConn.RecreateDevcontainer(ctx, container)
959+
m, err := agentConn.RecreateDevcontainer(ctx, container)
960960
if err != nil {
961961
if errors.Is(err, context.Canceled) {
962962
httpapi.Write(ctx, rw, http.StatusRequestTimeout, codersdk.Response{
@@ -977,7 +977,7 @@ func (api *API) workspaceAgentRecreateDevcontainer(rw http.ResponseWriter, r *ht
977977
return
978978
}
979979

980-
httpapi.Write(ctx, rw, http.StatusNoContent, nil)
980+
httpapi.Write(ctx, rw, http.StatusAccepted, m)
981981
}
982982

983983
// @Summary Get connection info for workspace agent

‎coderd/workspaceagents_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1483,7 +1483,7 @@ func TestWorkspaceAgentRecreateDevcontainer(t *testing.T) {
14831483

14841484
ctx := testutil.Context(t, testutil.WaitLong)
14851485

1486-
err := client.WorkspaceAgentRecreateDevcontainer(ctx, agentID, devContainer.ID)
1486+
_, err := client.WorkspaceAgentRecreateDevcontainer(ctx, agentID, devContainer.ID)
14871487
if wantStatus > 0 {
14881488
cerr, ok := codersdk.AsError(err)
14891489
require.True(t, ok, "expected error to be a coder error")

‎codersdk/workspaceagents.go

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -450,6 +450,10 @@ type WorkspaceAgentContainer struct {
450450
// Volumes is a map of "things" mounted into the container. Again, this
451451
// is somewhat implementation-dependent.
452452
Volumes map[string]string `json:"volumes"`
453+
// DevcontainerStatus is the status of the devcontainer, if this
454+
// container is a devcontainer. This is used to determine if the
455+
// devcontainer is running, stopped, starting, or in an error state.
456+
DevcontainerStatus WorkspaceAgentDevcontainerStatus `json:"devcontainer_status,omitempty"`
453457
// DevcontainerDirty is true if the devcontainer configuration has changed
454458
// since the container was created. This is used to determine if the
455459
// container needs to be rebuilt.
@@ -518,16 +522,20 @@ func (c *Client) WorkspaceAgentListContainers(ctx context.Context, agentID uuid.
518522
}
519523

520524
// WorkspaceAgentRecreateDevcontainer recreates the devcontainer with the given ID.
521-
func (c *Client) WorkspaceAgentRecreateDevcontainer(ctx context.Context, agentID uuid.UUID, containerIDOrName string) error {
525+
func (c *Client) WorkspaceAgentRecreateDevcontainer(ctx context.Context, agentID uuid.UUID, containerIDOrName string) (Response, error) {
522526
res, err := c.Request(ctx, http.MethodPost, fmt.Sprintf("/api/v2/workspaceagents/%s/containers/devcontainers/container/%s/recreate", agentID, containerIDOrName), nil)
523527
if err != nil {
524-
return err
528+
return Response{}, err
525529
}
526530
defer res.Body.Close()
527-
if res.StatusCode != http.StatusNoContent {
528-
return ReadBodyAsError(res)
531+
if res.StatusCode != http.StatusAccepted {
532+
return Response{}, ReadBodyAsError(res)
529533
}
530-
return nil
534+
var m Response
535+
if err := json.NewDecoder(res.Body).Decode(&m); err != nil {
536+
return Response{}, xerrors.Errorf("decode response body: %w", err)
537+
}
538+
return m, nil
531539
}
532540

533541
//nolint:revive // Follow is a control flag on the server as well.

‎codersdk/workspacesdk/agentconn.go

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -389,18 +389,22 @@ func (c *AgentConn) ListContainers(ctx context.Context) (codersdk.WorkspaceAgent
389389

390390
// RecreateDevcontainer recreates a devcontainer with the given container.
391391
// This is a blocking call and will wait for the container to be recreated.
392-
func (c *AgentConn) RecreateDevcontainer(ctx context.Context, containerIDOrName string) error {
392+
func (c *AgentConn) RecreateDevcontainer(ctx context.Context, containerIDOrName string) (codersdk.Response, error) {
393393
ctx, span := tracing.StartSpan(ctx)
394394
defer span.End()
395395
res, err := c.apiRequest(ctx, http.MethodPost, "/api/v0/containers/devcontainers/container/"+containerIDOrName+"/recreate", nil)
396396
if err != nil {
397-
return xerrors.Errorf("do request: %w", err)
397+
return codersdk.Response{}, xerrors.Errorf("do request: %w", err)
398398
}
399399
defer res.Body.Close()
400400
if res.StatusCode != http.StatusAccepted {
401-
return codersdk.ReadBodyAsError(res)
401+
return codersdk.Response{}, codersdk.ReadBodyAsError(res)
402402
}
403-
return nil
403+
var m codersdk.Response
404+
if err := json.NewDecoder(res.Body).Decode(&m); err != nil {
405+
return codersdk.Response{}, xerrors.Errorf("decode response body: %w", err)
406+
}
407+
return m, nil
404408
}
405409

406410
// apiRequest makes a request to the workspace agent's HTTP API server.

‎docs/reference/api/agents.md

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)