Skip to content

Commit 25fb34c

Browse files
mafredrimafredri
authored
feat(agent): implement recreate for devcontainers (#17308)
This change implements an interface for running `@devcontainers/cli up` and an API endpoint on the agent for triggering recreate for a running devcontainer. A couple of limitations: 1. Synchronous HTTP request, meaning the browser might choose to time it out before it's done => no result/error (and devcontainer cli command probably gets killed via ctx cancel). 2. Logs are only written to agent logs via slog, not as a "script" in the UI. Both 1 and 2 will be improved in future refactors. Fixes coder/internal#481 Fixes coder/internal#482
1 parent 6dd1056 commit 25fb34c

17 files changed

+1338
-22
lines changed

‎.gitattributes

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
# Generated files
22
agent/agentcontainers/acmock/acmock.go linguist-generated=true
33
agent/agentcontainers/dcspec/dcspec_gen.go linguist-generated=true
4+
agent/agentcontainers/testdata/devcontainercli/*/*.log linguist-generated=true
45
coderd/apidoc/docs.go linguist-generated=true
56
docs/reference/api/*.md linguist-generated=true
67
docs/reference/cli/*.md linguist-generated=true

‎.github/workflows/typos.toml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,5 +42,6 @@ extend-exclude = [
4242
"site/src/pages/SetupPage/countries.tsx",
4343
"provisioner/terraform/testdata/**",
4444
# notifications' golden files confuse the detector because of quoted-printable encoding
45-
"coderd/notifications/testdata/**"
45+
"coderd/notifications/testdata/**",
46+
"agent/agentcontainers/testdata/devcontainercli/**"
4647
]

‎agent/agentcontainers/containers.go

Lines changed: 76 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ import (
99

1010
"golang.org/x/xerrors"
1111

12+
"github.com/go-chi/chi/v5"
13+
1214
"github.com/coder/coder/v2/coderd/httpapi"
1315
"github.com/coder/coder/v2/codersdk"
1416
"github.com/coder/quartz"
@@ -20,9 +22,10 @@ const (
2022
getContainersTimeout = 5 * time.Second
2123
)
2224

23-
type devcontainersHandler struct {
25+
type Handler struct {
2426
cacheDuration time.Duration
2527
cl Lister
28+
dccli DevcontainerCLI
2629
clock quartz.Clock
2730

2831
// lockCh protects the below fields. We use a channel instead of a mutex so we
@@ -32,20 +35,26 @@ type devcontainersHandler struct {
3235
mtime time.Time
3336
}
3437

35-
// Option is a functional option for devcontainersHandler.
36-
type Option func(*devcontainersHandler)
38+
// Option is a functional option for Handler.
39+
type Option func(*Handler)
3740

3841
// WithLister sets the agentcontainers.Lister implementation to use.
3942
// The default implementation uses the Docker CLI to list containers.
4043
func WithLister(cl Lister) Option {
41-
return func(ch *devcontainersHandler) {
44+
return func(ch *Handler) {
4245
ch.cl = cl
4346
}
4447
}
4548

46-
// New returns a new devcontainersHandler with the given options applied.
47-
func New(options ...Option) http.Handler {
48-
ch := &devcontainersHandler{
49+
func WithDevcontainerCLI(dccli DevcontainerCLI) Option {
50+
return func(ch *Handler) {
51+
ch.dccli = dccli
52+
}
53+
}
54+
55+
// New returns a new Handler with the given options applied.
56+
func New(options ...Option) *Handler {
57+
ch := &Handler{
4958
lockCh: make(chan struct{}, 1),
5059
}
5160
for _, opt := range options {
@@ -54,7 +63,7 @@ func New(options ...Option) http.Handler {
5463
return ch
5564
}
5665

57-
func (ch *devcontainersHandler) ServeHTTP(rw http.ResponseWriter, r *http.Request) {
66+
func (ch *Handler) List(rw http.ResponseWriter, r *http.Request) {
5867
select {
5968
case <-r.Context().Done():
6069
// Client went away.
@@ -80,7 +89,7 @@ func (ch *devcontainersHandler) ServeHTTP(rw http.ResponseWriter, r *http.Reques
8089
}
8190
}
8291

83-
func (ch *devcontainersHandler) getContainers(ctx context.Context) (codersdk.WorkspaceAgentListContainersResponse, error) {
92+
func (ch *Handler) getContainers(ctx context.Context) (codersdk.WorkspaceAgentListContainersResponse, error) {
8493
select {
8594
case <-ctx.Done():
8695
return codersdk.WorkspaceAgentListContainersResponse{}, ctx.Err()
@@ -149,3 +158,61 @@ var _ Lister = NoopLister{}
149158
func (NoopLister) List(_ context.Context) (codersdk.WorkspaceAgentListContainersResponse, error) {
150159
return codersdk.WorkspaceAgentListContainersResponse{}, nil
151160
}
161+
162+
func (ch *Handler) Recreate(w http.ResponseWriter, r *http.Request) {
163+
ctx := r.Context()
164+
id := chi.URLParam(r, "id")
165+
166+
if id == "" {
167+
httpapi.Write(ctx, w, http.StatusBadRequest, codersdk.Response{
168+
Message: "Missing container ID or name",
169+
Detail: "Container ID or name is required to recreate a devcontainer.",
170+
})
171+
return
172+
}
173+
174+
containers, err := ch.cl.List(ctx)
175+
if err != nil {
176+
httpapi.Write(ctx, w, http.StatusInternalServerError, codersdk.Response{
177+
Message: "Could not list containers",
178+
Detail: err.Error(),
179+
})
180+
return
181+
}
182+
183+
containerIdx := slices.IndexFunc(containers.Containers, func(c codersdk.WorkspaceAgentContainer) bool {
184+
return c.Match(id)
185+
})
186+
if containerIdx == -1 {
187+
httpapi.Write(ctx, w, http.StatusNotFound, codersdk.Response{
188+
Message: "Container not found",
189+
Detail: "Container ID or name not found in the list of containers.",
190+
})
191+
return
192+
}
193+
194+
container := containers.Containers[containerIdx]
195+
workspaceFolder := container.Labels[DevcontainerLocalFolderLabel]
196+
configPath := container.Labels[DevcontainerConfigFileLabel]
197+
198+
// Workspace folder is required to recreate a container, we don't verify
199+
// the config path here because it's optional.
200+
if workspaceFolder == "" {
201+
httpapi.Write(ctx, w, http.StatusBadRequest, codersdk.Response{
202+
Message: "Missing workspace folder label",
203+
Detail: "The workspace folder label is required to recreate a devcontainer.",
204+
})
205+
return
206+
}
207+
208+
_, err = ch.dccli.Up(ctx, workspaceFolder, configPath, WithRemoveExistingContainer())
209+
if err != nil {
210+
httpapi.Write(ctx, w, http.StatusInternalServerError, codersdk.Response{
211+
Message: "Could not recreate devcontainer",
212+
Detail: err.Error(),
213+
})
214+
return
215+
}
216+
217+
w.WriteHeader(http.StatusNoContent)
218+
}

‎agent/agentcontainers/containers_internal_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -277,7 +277,7 @@ func TestContainersHandler(t *testing.T) {
277277
ctrl = gomock.NewController(t)
278278
mockLister = acmock.NewMockLister(ctrl)
279279
now = time.Now().UTC()
280-
ch = devcontainersHandler{
280+
ch = Handler{
281281
cacheDuration: tc.cacheDur,
282282
cl: mockLister,
283283
clock: clk,

‎agent/agentcontainers/containers_test.go

Lines changed: 166 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,166 @@
1+
package agentcontainers_test
2+
3+
import (
4+
"context"
5+
"net/http"
6+
"net/http/httptest"
7+
"testing"
8+
9+
"github.com/go-chi/chi/v5"
10+
"github.com/stretchr/testify/assert"
11+
"github.com/stretchr/testify/require"
12+
"golang.org/x/xerrors"
13+
14+
"github.com/coder/coder/v2/agent/agentcontainers"
15+
"github.com/coder/coder/v2/codersdk"
16+
)
17+
18+
// fakeLister implements the agentcontainers.Lister interface for
19+
// testing.
20+
type fakeLister struct {
21+
containers codersdk.WorkspaceAgentListContainersResponse
22+
err error
23+
}
24+
25+
func (f *fakeLister) List(_ context.Context) (codersdk.WorkspaceAgentListContainersResponse, error) {
26+
return f.containers, f.err
27+
}
28+
29+
// fakeDevcontainerCLI implements the agentcontainers.DevcontainerCLI
30+
// interface for testing.
31+
type fakeDevcontainerCLI struct {
32+
id string
33+
err error
34+
}
35+
36+
func (f *fakeDevcontainerCLI) Up(_ context.Context, _, _ string, _ ...agentcontainers.DevcontainerCLIUpOptions) (string, error) {
37+
return f.id, f.err
38+
}
39+
40+
func TestHandler(t *testing.T) {
41+
t.Parallel()
42+
43+
t.Run("Recreate", func(t *testing.T) {
44+
t.Parallel()
45+
46+
validContainer := codersdk.WorkspaceAgentContainer{
47+
ID: "container-id",
48+
FriendlyName: "container-name",
49+
Labels: map[string]string{
50+
agentcontainers.DevcontainerLocalFolderLabel: "/workspace",
51+
agentcontainers.DevcontainerConfigFileLabel: "/workspace/.devcontainer/devcontainer.json",
52+
},
53+
}
54+
55+
missingFolderContainer := codersdk.WorkspaceAgentContainer{
56+
ID: "missing-folder-container",
57+
FriendlyName: "missing-folder-container",
58+
Labels: map[string]string{},
59+
}
60+
61+
tests := []struct {
62+
name string
63+
containerID string
64+
lister *fakeLister
65+
devcontainerCLI *fakeDevcontainerCLI
66+
wantStatus int
67+
wantBody string
68+
}{
69+
{
70+
name: "Missing ID",
71+
containerID: "",
72+
lister: &fakeLister{},
73+
devcontainerCLI: &fakeDevcontainerCLI{},
74+
wantStatus: http.StatusBadRequest,
75+
wantBody: "Missing container ID or name",
76+
},
77+
{
78+
name: "List error",
79+
containerID: "container-id",
80+
lister: &fakeLister{
81+
err: xerrors.New("list error"),
82+
},
83+
devcontainerCLI: &fakeDevcontainerCLI{},
84+
wantStatus: http.StatusInternalServerError,
85+
wantBody: "Could not list containers",
86+
},
87+
{
88+
name: "Container not found",
89+
containerID: "nonexistent-container",
90+
lister: &fakeLister{
91+
containers: codersdk.WorkspaceAgentListContainersResponse{
92+
Containers: []codersdk.WorkspaceAgentContainer{validContainer},
93+
},
94+
},
95+
devcontainerCLI: &fakeDevcontainerCLI{},
96+
wantStatus: http.StatusNotFound,
97+
wantBody: "Container not found",
98+
},
99+
{
100+
name: "Missing workspace folder label",
101+
containerID: "missing-folder-container",
102+
lister: &fakeLister{
103+
containers: codersdk.WorkspaceAgentListContainersResponse{
104+
Containers: []codersdk.WorkspaceAgentContainer{missingFolderContainer},
105+
},
106+
},
107+
devcontainerCLI: &fakeDevcontainerCLI{},
108+
wantStatus: http.StatusBadRequest,
109+
wantBody: "Missing workspace folder label",
110+
},
111+
{
112+
name: "Devcontainer CLI error",
113+
containerID: "container-id",
114+
lister: &fakeLister{
115+
containers: codersdk.WorkspaceAgentListContainersResponse{
116+
Containers: []codersdk.WorkspaceAgentContainer{validContainer},
117+
},
118+
},
119+
devcontainerCLI: &fakeDevcontainerCLI{
120+
err: xerrors.New("devcontainer CLI error"),
121+
},
122+
wantStatus: http.StatusInternalServerError,
123+
wantBody: "Could not recreate devcontainer",
124+
},
125+
{
126+
name: "OK",
127+
containerID: "container-id",
128+
lister: &fakeLister{
129+
containers: codersdk.WorkspaceAgentListContainersResponse{
130+
Containers: []codersdk.WorkspaceAgentContainer{validContainer},
131+
},
132+
},
133+
devcontainerCLI: &fakeDevcontainerCLI{},
134+
wantStatus: http.StatusNoContent,
135+
wantBody: "",
136+
},
137+
}
138+
139+
for _, tt := range tests {
140+
t.Run(tt.name, func(t *testing.T) {
141+
t.Parallel()
142+
143+
// Setup router with the handler under test.
144+
r := chi.NewRouter()
145+
handler := agentcontainers.New(
146+
agentcontainers.WithLister(tt.lister),
147+
agentcontainers.WithDevcontainerCLI(tt.devcontainerCLI),
148+
)
149+
r.Post("/containers/{id}/recreate", handler.Recreate)
150+
151+
// Simulate HTTP request to the recreate endpoint.
152+
req := httptest.NewRequest(http.MethodPost, "/containers/"+tt.containerID+"/recreate", nil)
153+
rec := httptest.NewRecorder()
154+
r.ServeHTTP(rec, req)
155+
156+
// Check the response status code and body.
157+
require.Equal(t, tt.wantStatus, rec.Code, "status code mismatch")
158+
if tt.wantBody != "" {
159+
assert.Contains(t, rec.Body.String(), tt.wantBody, "response body mismatch")
160+
} else if tt.wantStatus == http.StatusNoContent {
161+
assert.Empty(t, rec.Body.String(), "expected empty response body")
162+
}
163+
})
164+
}
165+
})
166+
}

‎agent/agentcontainers/devcontainer.go

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,15 @@ import (
1212
"github.com/coder/coder/v2/codersdk"
1313
)
1414

15+
const (
16+
// DevcontainerLocalFolderLabel is the label that contains the path to
17+
// the local workspace folder for a devcontainer.
18+
DevcontainerLocalFolderLabel = "devcontainer.local_folder"
19+
// DevcontainerConfigFileLabel is the label that contains the path to
20+
// the devcontainer.json configuration file.
21+
DevcontainerConfigFileLabel = "devcontainer.config_file"
22+
)
23+
1524
const devcontainerUpScriptTemplate = `
1625
if ! which devcontainer > /dev/null 2>&1; then
1726
echo "ERROR: Unable to start devcontainer, @devcontainers/cli is not installed."
@@ -52,8 +61,10 @@ ScriptLoop:
5261
}
5362

5463
func devcontainerStartupScript(dc codersdk.WorkspaceAgentDevcontainer, script codersdk.WorkspaceAgentScript) codersdk.WorkspaceAgentScript {
55-
var args []string
56-
args = append(args, fmt.Sprintf("--workspace-folder %q", dc.WorkspaceFolder))
64+
args := []string{
65+
"--log-format json",
66+
fmt.Sprintf("--workspace-folder %q", dc.WorkspaceFolder),
67+
}
5768
if dc.ConfigPath != "" {
5869
args = append(args, fmt.Sprintf("--config %q", dc.ConfigPath))
5970
}

0 commit comments

Comments
 (0)