mirror of
https://github.com/coder/coder.git
synced 2026-09-24 15:04:27 +08:00
fix!: use devcontainer ID when rebuilding a devcontainer (#18604)
This PR replaces the use of the **container** ID with the **devcontainer** ID. This is a breaking change. This allows rebuilding a devcontainer when there is no valid container ID.
This commit is contained in:
Danielle Maywood
@@ -496,8 +496,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) {
|
||||
r.Post("/recreate", api.handleDevcontainerRecreate)
|
||||
})
|
||||
|
||||
return r
|
||||
@@ -861,68 +861,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
|
||||
}
|
||||
}
|
||||
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
|
||||
}
|
||||
@@ -933,14 +905,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),
|
||||
})
|
||||
}
|
||||
|
||||
|
||||
@@ -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.
|
||||
},
|
||||
@@ -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.
|
||||
},
|
||||
@@ -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())
|
||||
@@ -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).
|
||||
WithContext(ctx)
|
||||
rec := httptest.NewRecorder()
|
||||
r.ServeHTTP(rec, req)
|
||||
|
||||
Reference in New Issue
Block a user