Skip to content

fix!: use devcontainer ID when rebuilding a devcontainer #18604

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 2 commits into from
Jun 26, 2025
Merged

fix!: use devcontainer ID when rebuilding a devcontainer #18604

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9e789dc
fix!: use devcontainer ID when rebuilding a devcontainer
DanielleMaywood Jun 26, 2025
e2ebca9
chore: improve lookup
DanielleMaywood Jun 26, 2025
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
70 changes: 21 additions & 49 deletions agent/agentcontainers/api.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -494,8 +494,8 @@ func (api *API) Routes() http.Handler {
r.Get("/", api.handleList)
// TODO(mafredri): Simplify this route as the previous /devcontainers
// /-route was dropped. We can drop the /devcontainers prefix here too.
r.Route("/devcontainers", func(r chi.Router) {
r.Post("/container/{container}/recreate", api.handleDevcontainerRecreate)
r.Route("/devcontainers/{devcontainer}", func(r chi.Router) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This dawned on me just now, but we could take workspaceFolder here instead, which will allow starting any non-started devcontainer even if it doesn't exist. 😅 We can table this for now though as implementing it would be preemptive and more thought behind it would be good.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oooh that could be good to add in the future.

r.Post("/recreate", api.handleDevcontainerRecreate)
})

return r
Expand Down Expand Up @@ -859,68 +859,40 @@ func (api *API) getContainers() (codersdk.WorkspaceAgentListContainersResponse,
// devcontainer by referencing the container.
func (api *API) handleDevcontainerRecreate(w http.ResponseWriter, r *http.Request) {
ctx := r.Context()
containerID := chi.URLParam(r, "container")
devcontainerID := chi.URLParam(r, "devcontainer")

if containerID == "" {
if devcontainerID == "" {
httpapi.Write(ctx, w, http.StatusBadRequest, codersdk.Response{
Message: "Missing container ID or name",
Detail: "Container ID or name is required to recreate a devcontainer.",
})
return
}

containers, err := api.getContainers()
if err != nil {
httpapi.Write(ctx, w, http.StatusInternalServerError, codersdk.Response{
Message: "Could not list containers",
Detail: err.Error(),
})
return
}

containerIdx := slices.IndexFunc(containers.Containers, func(c codersdk.WorkspaceAgentContainer) bool { return c.Match(containerID) })
if containerIdx == -1 {
httpapi.Write(ctx, w, http.StatusNotFound, codersdk.Response{
Message: "Container not found",
Detail: "Container ID or name not found in the list of containers.",
})
return
}

container := containers.Containers[containerIdx]
workspaceFolder := container.Labels[DevcontainerLocalFolderLabel]
configPath := container.Labels[DevcontainerConfigFileLabel]

// Workspace folder is required to recreate a container, we don't verify
// the config path here because it's optional.
if workspaceFolder == "" {
httpapi.Write(ctx, w, http.StatusBadRequest, codersdk.Response{
Message: "Missing workspace folder label",
Detail: "The container is not a devcontainer, the container must have the workspace folder label to support recreation.",
Message: "Missing devcontainer ID",
Detail: "Devcontainer ID is required to recreate a devcontainer.",
})
return
}

api.mu.Lock()

dc, ok := api.knownDevcontainers[workspaceFolder]
switch {
case !ok:
var dc codersdk.WorkspaceAgentDevcontainer
for _, knownDC := range api.knownDevcontainers {
if knownDC.ID.String() == devcontainerID {
dc = knownDC
break
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: This could be simplified by storing dc from the for-loop rather than doing two lookups first ID then workspace folder (which we already know).

}
if dc.ID == uuid.Nil {
api.mu.Unlock()

// This case should not happen if the container is a valid devcontainer.
api.logger.Error(ctx, "devcontainer not found for workspace folder", slog.F("workspace_folder", workspaceFolder))
httpapi.Write(ctx, w, http.StatusInternalServerError, codersdk.Response{
httpapi.Write(ctx, w, http.StatusNotFound, codersdk.Response{
Message: "Devcontainer not found.",
Detail: fmt.Sprintf("Could not find devcontainer for workspace folder: %q", workspaceFolder),
Detail: fmt.Sprintf("Could not find devcontainer with ID: %q", devcontainerID),
})
return
case dc.Status == codersdk.WorkspaceAgentDevcontainerStatusStarting:
}
if dc.Status == codersdk.WorkspaceAgentDevcontainerStatusStarting {
api.mu.Unlock()

httpapi.Write(ctx, w, http.StatusConflict, codersdk.Response{
Message: "Devcontainer recreation already in progress",
Detail: fmt.Sprintf("Recreation for workspace folder %q is already underway.", dc.WorkspaceFolder),
Detail: fmt.Sprintf("Recreation for devcontainer %q is already underway.", dc.Name),
})
return
}
Expand All @@ -931,14 +903,14 @@ func (api *API) handleDevcontainerRecreate(w http.ResponseWriter, r *http.Reques
dc.Container = nil
api.knownDevcontainers[dc.WorkspaceFolder] = dc
go func() {
_ = api.CreateDevcontainer(dc.WorkspaceFolder, configPath, WithRemoveExistingContainer())
_ = api.CreateDevcontainer(dc.WorkspaceFolder, dc.ConfigPath, WithRemoveExistingContainer())
}()

api.mu.Unlock()

httpapi.Write(ctx, w, http.StatusAccepted, codersdk.Response{
Message: "Devcontainer recreation initiated",
Detail: fmt.Sprintf("Recreation process for workspace folder %q has started.", dc.WorkspaceFolder),
Detail: fmt.Sprintf("Recreation process for devcontainer %q has started.", dc.Name),
})
}

Expand Down
114 changes: 63 additions & 51 deletions agent/agentcontainers/api_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -493,78 +493,77 @@ func TestAPI(t *testing.T) {
t.Run("Recreate", func(t *testing.T) {
t.Parallel()

validContainer := codersdk.WorkspaceAgentContainer{
ID: "container-id",
FriendlyName: "container-name",
devcontainerID1 := uuid.New()
devcontainerID2 := uuid.New()
workspaceFolder1 := "/workspace/test1"
workspaceFolder2 := "/workspace/test2"
configPath1 := "/workspace/test1/.devcontainer/devcontainer.json"
configPath2 := "/workspace/test2/.devcontainer/devcontainer.json"

// Create a container that represents an existing devcontainer
devContainer1 := codersdk.WorkspaceAgentContainer{
ID: "container-1",
FriendlyName: "test-container-1",
Running: true,
Labels: map[string]string{
agentcontainers.DevcontainerLocalFolderLabel: "/workspaces",
agentcontainers.DevcontainerConfigFileLabel: "/workspace/.devcontainer/devcontainer.json",
agentcontainers.DevcontainerLocalFolderLabel: workspaceFolder1,
agentcontainers.DevcontainerConfigFileLabel: configPath1,
},
}

missingFolderContainer := codersdk.WorkspaceAgentContainer{
ID: "missing-folder-container",
FriendlyName: "missing-folder-container",
Labels: map[string]string{},
devContainer2 := codersdk.WorkspaceAgentContainer{
ID: "container-2",
FriendlyName: "test-container-2",
Running: true,
Labels: map[string]string{
agentcontainers.DevcontainerLocalFolderLabel: workspaceFolder2,
agentcontainers.DevcontainerConfigFileLabel: configPath2,
},
}

tests := []struct {
name string
containerID string
lister *fakeContainerCLI
devcontainerCLI *fakeDevcontainerCLI
wantStatus []int
wantBody []string
name string
devcontainerID string
setupDevcontainers []codersdk.WorkspaceAgentDevcontainer
lister *fakeContainerCLI
devcontainerCLI *fakeDevcontainerCLI
wantStatus []int
wantBody []string
}{
{
name: "Missing container ID",
containerID: "",
name: "Missing devcontainer ID",
devcontainerID: "",
lister: &fakeContainerCLI{},
devcontainerCLI: &fakeDevcontainerCLI{},
wantStatus: []int{http.StatusBadRequest},
wantBody: []string{"Missing container ID or name"},
wantBody: []string{"Missing devcontainer ID"},
},
{
name: "List error",
containerID: "container-id",
name: "Devcontainer not found",
devcontainerID: uuid.NewString(),
lister: &fakeContainerCLI{
listErr: xerrors.New("list error"),
},
devcontainerCLI: &fakeDevcontainerCLI{},
wantStatus: []int{http.StatusInternalServerError},
wantBody: []string{"Could not list containers"},
},
{
name: "Container not found",
containerID: "nonexistent-container",
lister: &fakeContainerCLI{
containers: codersdk.WorkspaceAgentListContainersResponse{
Containers: []codersdk.WorkspaceAgentContainer{validContainer},
},
arch: "<none>", // Unsupported architecture, don't inject subagent.
},
devcontainerCLI: &fakeDevcontainerCLI{},
wantStatus: []int{http.StatusNotFound},
wantBody: []string{"Container not found"},
wantBody: []string{"Devcontainer not found"},
},
{
name: "Missing workspace folder label",
containerID: "missing-folder-container",
lister: &fakeContainerCLI{
containers: codersdk.WorkspaceAgentListContainersResponse{
Containers: []codersdk.WorkspaceAgentContainer{missingFolderContainer},
name: "Devcontainer CLI error",
devcontainerID: devcontainerID1.String(),
setupDevcontainers: []codersdk.WorkspaceAgentDevcontainer{
{
ID: devcontainerID1,
Name: "test-devcontainer-1",
WorkspaceFolder: workspaceFolder1,
ConfigPath: configPath1,
Status: codersdk.WorkspaceAgentDevcontainerStatusRunning,
Container: &devContainer1,
},
},
devcontainerCLI: &fakeDevcontainerCLI{},
wantStatus: []int{http.StatusBadRequest},
wantBody: []string{"Missing workspace folder label"},
},
{
name: "Devcontainer CLI error",
containerID: "container-id",
lister: &fakeContainerCLI{
containers: codersdk.WorkspaceAgentListContainersResponse{
Containers: []codersdk.WorkspaceAgentContainer{validContainer},
Containers: []codersdk.WorkspaceAgentContainer{devContainer1},
},
arch: "<none>", // Unsupported architecture, don't inject subagent.
},
Expand All @@ -575,11 +574,21 @@ func TestAPI(t *testing.T) {
wantBody: []string{"Devcontainer recreation initiated", "Devcontainer recreation already in progress"},
},
{
name: "OK",
containerID: "container-id",
name: "OK",
devcontainerID: devcontainerID2.String(),
setupDevcontainers: []codersdk.WorkspaceAgentDevcontainer{
{
ID: devcontainerID2,
Name: "test-devcontainer-2",
WorkspaceFolder: workspaceFolder2,
ConfigPath: configPath2,
Status: codersdk.WorkspaceAgentDevcontainerStatusRunning,
Container: &devContainer2,
},
},
lister: &fakeContainerCLI{
containers: codersdk.WorkspaceAgentListContainersResponse{
Containers: []codersdk.WorkspaceAgentContainer{validContainer},
Containers: []codersdk.WorkspaceAgentContainer{devContainer2},
},
arch: "<none>", // Unsupported architecture, don't inject subagent.
},
Expand Down Expand Up @@ -608,13 +617,16 @@ func TestAPI(t *testing.T) {

// Setup router with the handler under test.
r := chi.NewRouter()

api := agentcontainers.NewAPI(
logger,
agentcontainers.WithClock(mClock),
agentcontainers.WithContainerCLI(tt.lister),
agentcontainers.WithDevcontainerCLI(tt.devcontainerCLI),
agentcontainers.WithWatcher(watcher.NewNoop()),
agentcontainers.WithDevcontainers(tt.setupDevcontainers, nil),
)

api.Init()
defer api.Close()
r.Mount("/", api.Routes())
Expand All @@ -626,7 +638,7 @@ func TestAPI(t *testing.T) {

for i := range tt.wantStatus {
// Simulate HTTP request to the recreate endpoint.
req := httptest.NewRequest(http.MethodPost, "/devcontainers/container/"+tt.containerID+"/recreate", nil).
req := httptest.NewRequest(http.MethodPost, "/devcontainers/"+tt.devcontainerID+"/recreate", nil).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a much more sensible URL 😍

WithContext(ctx)
rec := httptest.NewRecorder()
r.ServeHTTP(rec, req)
Expand Down
6 changes: 3 additions & 3 deletions coderd/apidoc/docs.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions coderd/apidoc/swagger.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion coderd/coderd.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1314,7 +1314,7 @@ func New(options *Options) *API {
r.Get("/listening-ports", api.workspaceAgentListeningPorts)
r.Get("/connection", api.workspaceAgentConnection)
r.Get("/containers", api.workspaceAgentListContainers)
r.Post("/containers/devcontainers/container/{container}/recreate", api.workspaceAgentRecreateDevcontainer)
r.Post("/containers/devcontainers/{devcontainer}/recreate", api.workspaceAgentRecreateDevcontainer)
r.Get("/coordinate", api.workspaceAgentClientCoordinate)

// PTY is part of workspaceAppServer.
Expand Down
14 changes: 7 additions & 7 deletions coderd/workspaceagents.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -905,19 +905,19 @@ func (api *API) workspaceAgentListContainers(rw http.ResponseWriter, r *http.Req
// @Tags Agents
// @Produce json
// @Param workspaceagent path string true "Workspace agent ID" format(uuid)
// @Param container path string true "Container ID or name"
// @Param devcontainer path string true "Devcontainer ID"
// @Success 202 {object} codersdk.Response
// @Router /workspaceagents/{workspaceagent}/containers/devcontainers/container/{container}/recreate [post]
// @Router /workspaceagents/{workspaceagent}/containers/devcontainers/{devcontainer}/recreate [post]
func (api *API) workspaceAgentRecreateDevcontainer(rw http.ResponseWriter, r *http.Request) {
ctx := r.Context()
workspaceAgent := httpmw.WorkspaceAgentParam(r)

container := chi.URLParam(r, "container")
if container == "" {
devcontainer := chi.URLParam(r, "devcontainer")
if devcontainer == "" {
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
Message: "Container ID or name is required.",
Message: "Devcontainer ID is required.",
Validations: []codersdk.ValidationError{
{Field: "container", Detail: "Container ID or name is required."},
{Field: "devcontainer", Detail: "Devcontainer ID is required."},
},
})
return
Expand Down Expand Up @@ -961,7 +961,7 @@ func (api *API) workspaceAgentRecreateDevcontainer(rw http.ResponseWriter, r *ht
}
defer release()

m, err := agentConn.RecreateDevcontainer(ctx, container)
m, err := agentConn.RecreateDevcontainer(ctx, devcontainer)
if err != nil {
if errors.Is(err, context.Canceled) {
httpapi.Write(ctx, rw, http.StatusRequestTimeout, codersdk.Response{
Expand Down
Loading
Loading