From 146cf928c4a0ea05ff93261c0dbcaa7cc65186cc Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Mon, 14 Apr 2025 09:53:21 +0000 Subject: [PATCH 1/4] feat(agent/agentcontainers): add devcontainers list endpoint This change allows listing both predefined and runtime-detected devcontainers, as well as showing whether or not the devcontainer is running and which container represents it. Fixes coder/internal#478 --- agent/agentcontainers/api.go | 131 +++++++++++- agent/agentcontainers/api_test.go | 342 +++++++++++++++++++++++++++++- agent/api.go | 15 +- codersdk/workspaceagents.go | 10 + 4 files changed, 482 insertions(+), 16 deletions(-) diff --git a/agent/agentcontainers/api.go b/agent/agentcontainers/api.go index 81354457d0730..f7126f7a5f6bf 100644 --- a/agent/agentcontainers/api.go +++ b/agent/agentcontainers/api.go @@ -3,11 +3,16 @@ package agentcontainers import ( "context" "errors" + "fmt" "net/http" + "path" "slices" + "strings" + "sync" "time" "github.com/go-chi/chi/v5" + "github.com/google/uuid" "golang.org/x/xerrors" "cdr.dev/slog" @@ -31,11 +36,15 @@ type API struct { dccli DevcontainerCLI clock quartz.Clock - // lockCh protects the below fields. We use a channel instead of a mutex so we - // can handle cancellation properly. + // lockCh protects the below fields. We use a channel instead of a + // mutex so we can handle cancellation properly. lockCh chan struct{} containers codersdk.WorkspaceAgentListContainersResponse mtime time.Time + + devcontainersMu sync.Mutex // Protects following. + devcontainerNames map[string]struct{} // Track devcontainer names to avoid duplicates. + knownDevcontainers []codersdk.WorkspaceAgentDevcontainer // Track predefined and runtime-detected devcontainers. } // Option is a functional option for API. @@ -55,12 +64,29 @@ func WithDevcontainerCLI(dccli DevcontainerCLI) Option { } } +// WithDevcontainers sets the known devcontainers and scripts for the +// API. This allows allows the API to be aware of devcontainers defined +// in the workspace agent manifest. +func WithDevcontainers(devcontainers []codersdk.WorkspaceAgentDevcontainer) Option { + return func(api *API) { + if len(devcontainers) > 0 { + api.knownDevcontainers = slices.Clone(devcontainers) + api.devcontainerNames = make(map[string]struct{}, len(devcontainers)) + for _, devcontainer := range devcontainers { + api.devcontainerNames[devcontainer.Name] = struct{}{} + } + } + } +} + // NewAPI returns a new API with the given options applied. func NewAPI(logger slog.Logger, options ...Option) *API { api := &API{ - clock: quartz.NewReal(), - cacheDuration: defaultGetContainersCacheDuration, - lockCh: make(chan struct{}, 1), + clock: quartz.NewReal(), + cacheDuration: defaultGetContainersCacheDuration, + lockCh: make(chan struct{}, 1), + devcontainerNames: make(map[string]struct{}), + knownDevcontainers: []codersdk.WorkspaceAgentDevcontainer{}, } for _, opt := range options { opt(api) @@ -79,6 +105,7 @@ func NewAPI(logger slog.Logger, options ...Option) *API { func (api *API) Routes() http.Handler { r := chi.NewRouter() r.Get("/", api.handleList) + r.Get("/devcontainers", api.handleListDevcontainers) r.Post("/{id}/recreate", api.handleRecreate) return r } @@ -121,12 +148,11 @@ func (api *API) getContainers(ctx context.Context) (codersdk.WorkspaceAgentListC select { case <-ctx.Done(): return codersdk.WorkspaceAgentListContainersResponse{}, ctx.Err() - default: - api.lockCh <- struct{}{} + case api.lockCh <- struct{}{}: + defer func() { + <-api.lockCh + }() } - defer func() { - <-api.lockCh - }() now := api.clock.Now() if now.Sub(api.mtime) < api.cacheDuration { @@ -142,6 +168,57 @@ func (api *API) getContainers(ctx context.Context) (codersdk.WorkspaceAgentListC api.containers = updated api.mtime = now + api.devcontainersMu.Lock() + defer api.devcontainersMu.Unlock() + + // Reset all known devcontainers to not running. + for i := range api.knownDevcontainers { + api.knownDevcontainers[i].Running = false + api.knownDevcontainers[i].Container = nil + } + + // Check if the container is running and update the known devcontainers. + for _, container := range updated.Containers { + workspaceFolder := container.Labels[DevcontainerLocalFolderLabel] + if workspaceFolder != "" { + // Check if this is already in our known list. + if knownIndex := slices.IndexFunc(api.knownDevcontainers, func(dc codersdk.WorkspaceAgentDevcontainer) bool { + return dc.WorkspaceFolder == workspaceFolder + }); knownIndex != -1 { + // Update existing entry with runtime information. + if api.knownDevcontainers[knownIndex].ConfigPath == "" { + api.knownDevcontainers[knownIndex].ConfigPath = container.Labels[DevcontainerConfigFileLabel] + } + api.knownDevcontainers[knownIndex].Running = container.Running + api.knownDevcontainers[knownIndex].Container = &container + continue + } + + // If not in our known list, add as a runtime detected entry. + // Parse name from folder + name := path.Base(workspaceFolder) + if _, ok := api.devcontainerNames[name]; ok { + // Try to find a unique name by appending a number. + for i := 2; ; i++ { + newName := fmt.Sprintf("%s-%d", name, i) + if _, ok := api.devcontainerNames[newName]; !ok { + name = newName + break + } + } + } + api.devcontainerNames[name] = struct{}{} + api.knownDevcontainers = append(api.knownDevcontainers, codersdk.WorkspaceAgentDevcontainer{ + ID: uuid.New(), + Name: name, + WorkspaceFolder: workspaceFolder, + ConfigPath: container.Labels[DevcontainerConfigFileLabel], + Running: container.Running, + Container: &container, + }) + } + } + return copyListContainersResponse(api.containers), nil } @@ -158,7 +235,7 @@ func (api *API) handleRecreate(w http.ResponseWriter, r *http.Request) { return } - containers, err := api.cl.List(ctx) + containers, err := api.getContainers(ctx) if err != nil { httpapi.Write(ctx, w, http.StatusInternalServerError, codersdk.Response{ Message: "Could not list containers", @@ -203,3 +280,35 @@ func (api *API) handleRecreate(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusNoContent) } + +// handleListDevcontainers handles the HTTP request to list known devcontainers. +func (api *API) handleListDevcontainers(w http.ResponseWriter, r *http.Request) { + ctx := r.Context() + + // Run getContainers to detect the latest devcontainers and their state. + _, err := api.getContainers(ctx) + if err != nil { + httpapi.Write(ctx, w, http.StatusInternalServerError, codersdk.Response{ + Message: "Could not list containers", + Detail: err.Error(), + }) + return + } + + api.devcontainersMu.Lock() + devcontainers := slices.Clone(api.knownDevcontainers) + api.devcontainersMu.Unlock() + + slices.SortFunc(devcontainers, func(a, b codersdk.WorkspaceAgentDevcontainer) int { + if cmp := strings.Compare(a.WorkspaceFolder, b.WorkspaceFolder); cmp != 0 { + return cmp + } + return strings.Compare(a.ConfigPath, b.ConfigPath) + }) + + response := codersdk.WorkspaceAgentDevcontainersResponse{ + Devcontainers: devcontainers, + } + + httpapi.Write(ctx, w, http.StatusOK, response) +} diff --git a/agent/agentcontainers/api_test.go b/agent/agentcontainers/api_test.go index 76a88f4fc1da4..7b7ea22d2bfb2 100644 --- a/agent/agentcontainers/api_test.go +++ b/agent/agentcontainers/api_test.go @@ -2,11 +2,13 @@ package agentcontainers_test import ( "context" + "encoding/json" "net/http" "net/http/httptest" "testing" "github.com/go-chi/chi/v5" + "github.com/google/uuid" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "golang.org/x/xerrors" @@ -151,10 +153,10 @@ func TestAPI(t *testing.T) { agentcontainers.WithLister(tt.lister), agentcontainers.WithDevcontainerCLI(tt.devcontainerCLI), ) - r.Mount("/containers", api.Routes()) + r.Mount("/", api.Routes()) // Simulate HTTP request to the recreate endpoint. - req := httptest.NewRequest(http.MethodPost, "/containers/"+tt.containerID+"/recreate", nil) + req := httptest.NewRequest(http.MethodPost, "/"+tt.containerID+"/recreate", nil) rec := httptest.NewRecorder() r.ServeHTTP(rec, req) @@ -168,4 +170,340 @@ func TestAPI(t *testing.T) { }) } }) + + t.Run("List devcontainers", func(t *testing.T) { + t.Parallel() + + knownDevcontainerID1 := uuid.New() + knownDevcontainerID2 := uuid.New() + + knownDevcontainers := []codersdk.WorkspaceAgentDevcontainer{ + { + ID: knownDevcontainerID1, + Name: "known-devcontainer-1", + WorkspaceFolder: "/workspace/known1", + ConfigPath: "/workspace/known1/.devcontainer/devcontainer.json", + }, + { + ID: knownDevcontainerID2, + Name: "known-devcontainer-2", + WorkspaceFolder: "/workspace/known2", + // No config path intentionally + }, + } + + tests := []struct { + name string + lister *fakeLister + knownDevcontainers []codersdk.WorkspaceAgentDevcontainer + wantStatus int + wantCount int + verify func(t *testing.T, devcontainers []codersdk.WorkspaceAgentDevcontainer) + }{ + { + name: "List error", + lister: &fakeLister{ + err: xerrors.New("list error"), + }, + wantStatus: http.StatusInternalServerError, + }, + { + name: "Empty containers", + lister: &fakeLister{}, + wantStatus: http.StatusOK, + wantCount: 0, + }, + { + name: "Only known devcontainers, no containers", + lister: &fakeLister{ + containers: codersdk.WorkspaceAgentListContainersResponse{ + Containers: []codersdk.WorkspaceAgentContainer{}, + }, + }, + knownDevcontainers: knownDevcontainers, + wantStatus: http.StatusOK, + wantCount: 2, + verify: func(t *testing.T, devcontainers []codersdk.WorkspaceAgentDevcontainer) { + for _, dc := range devcontainers { + assert.False(t, dc.Running, "devcontainer should not be running") + assert.Nil(t, dc.Container, "devcontainer should not have container reference") + } + }, + }, + { + name: "Runtime-detected devcontainer", + lister: &fakeLister{ + containers: codersdk.WorkspaceAgentListContainersResponse{ + Containers: []codersdk.WorkspaceAgentContainer{ + { + ID: "runtime-container-1", + FriendlyName: "runtime-container-1", + Running: true, + Labels: map[string]string{ + agentcontainers.DevcontainerLocalFolderLabel: "/workspace/runtime1", + agentcontainers.DevcontainerConfigFileLabel: "/workspace/runtime1/.devcontainer/devcontainer.json", + }, + }, + { + ID: "not-a-devcontainer", + FriendlyName: "not-a-devcontainer", + Running: true, + Labels: map[string]string{}, + }, + }, + }, + }, + wantStatus: http.StatusOK, + wantCount: 1, + verify: func(t *testing.T, devcontainers []codersdk.WorkspaceAgentDevcontainer) { + dc := devcontainers[0] + assert.Equal(t, "/workspace/runtime1", dc.WorkspaceFolder) + assert.True(t, dc.Running) + require.NotNil(t, dc.Container) + assert.Equal(t, "runtime-container-1", dc.Container.ID) + }, + }, + { + name: "Mixed known and runtime-detected devcontainers", + lister: &fakeLister{ + containers: codersdk.WorkspaceAgentListContainersResponse{ + Containers: []codersdk.WorkspaceAgentContainer{ + { + ID: "known-container-1", + FriendlyName: "known-container-1", + Running: true, + Labels: map[string]string{ + agentcontainers.DevcontainerLocalFolderLabel: "/workspace/known1", + agentcontainers.DevcontainerConfigFileLabel: "/workspace/known1/.devcontainer/devcontainer.json", + }, + }, + { + ID: "runtime-container-1", + FriendlyName: "runtime-container-1", + Running: true, + Labels: map[string]string{ + agentcontainers.DevcontainerLocalFolderLabel: "/workspace/runtime1", + agentcontainers.DevcontainerConfigFileLabel: "/workspace/runtime1/.devcontainer/devcontainer.json", + }, + }, + }, + }, + }, + knownDevcontainers: knownDevcontainers, + wantStatus: http.StatusOK, + wantCount: 3, // 2 known + 1 runtime + verify: func(t *testing.T, devcontainers []codersdk.WorkspaceAgentDevcontainer) { + // Find and verify devcontainers by workspace folder + known1 := mustFindDevcontainerByPath(t, devcontainers, "/workspace/known1") + known2 := mustFindDevcontainerByPath(t, devcontainers, "/workspace/known2") + runtime1 := mustFindDevcontainerByPath(t, devcontainers, "/workspace/runtime1") + + assert.True(t, known1.Running) + assert.False(t, known2.Running) + assert.True(t, runtime1.Running) + + require.NotNil(t, known1.Container) + assert.Nil(t, known2.Container) + require.NotNil(t, runtime1.Container) + + assert.Equal(t, "known-container-1", known1.Container.ID) + assert.Equal(t, "runtime-container-1", runtime1.Container.ID) + }, + }, + { + name: "Both running and non-running containers have container references", + lister: &fakeLister{ + containers: codersdk.WorkspaceAgentListContainersResponse{ + Containers: []codersdk.WorkspaceAgentContainer{ + { + ID: "running-container", + FriendlyName: "running-container", + Running: true, + Labels: map[string]string{ + agentcontainers.DevcontainerLocalFolderLabel: "/workspace/running", + agentcontainers.DevcontainerConfigFileLabel: "/workspace/running/.devcontainer/devcontainer.json", + }, + }, + { + ID: "non-running-container", + FriendlyName: "non-running-container", + Running: false, + Labels: map[string]string{ + agentcontainers.DevcontainerLocalFolderLabel: "/workspace/non-running", + agentcontainers.DevcontainerConfigFileLabel: "/workspace/non-running/.devcontainer/devcontainer.json", + }, + }, + }, + }, + }, + wantStatus: http.StatusOK, + wantCount: 2, + verify: func(t *testing.T, devcontainers []codersdk.WorkspaceAgentDevcontainer) { + running := mustFindDevcontainerByPath(t, devcontainers, "/workspace/running") + nonRunning := mustFindDevcontainerByPath(t, devcontainers, "/workspace/non-running") + + assert.True(t, running.Running) + assert.False(t, nonRunning.Running) + + require.NotNil(t, running.Container, "running container should have container reference") + require.NotNil(t, nonRunning.Container, "non-running container should have container reference") + + assert.Equal(t, "running-container", running.Container.ID) + assert.Equal(t, "non-running-container", nonRunning.Container.ID) + }, + }, + { + name: "Config path update", + lister: &fakeLister{ + containers: codersdk.WorkspaceAgentListContainersResponse{ + Containers: []codersdk.WorkspaceAgentContainer{ + { + ID: "known-container-2", + FriendlyName: "known-container-2", + Running: true, + Labels: map[string]string{ + agentcontainers.DevcontainerLocalFolderLabel: "/workspace/known2", + agentcontainers.DevcontainerConfigFileLabel: "/workspace/known2/.devcontainer/devcontainer.json", + }, + }, + }, + }, + }, + knownDevcontainers: knownDevcontainers, + wantStatus: http.StatusOK, + wantCount: 2, + verify: func(t *testing.T, devcontainers []codersdk.WorkspaceAgentDevcontainer) { + // Find devcontainer with ID matching knownDevcontainerID2 + var dc2 *codersdk.WorkspaceAgentDevcontainer + for i := range devcontainers { + if devcontainers[i].ID == knownDevcontainerID2 { + dc2 = &devcontainers[i] + break + } + } + require.NotNil(t, dc2, "missing devcontainer with ID %s", knownDevcontainerID2) + assert.True(t, dc2.Running) + assert.NotEmpty(t, dc2.ConfigPath) + require.NotNil(t, dc2.Container) + assert.Equal(t, "known-container-2", dc2.Container.ID) + }, + }, + { + name: "Name generation and uniqueness", + lister: &fakeLister{ + containers: codersdk.WorkspaceAgentListContainersResponse{ + Containers: []codersdk.WorkspaceAgentContainer{ + { + ID: "project1-container", + FriendlyName: "project1-container", + Running: true, + Labels: map[string]string{ + agentcontainers.DevcontainerLocalFolderLabel: "/workspace/project", + agentcontainers.DevcontainerConfigFileLabel: "/workspace/project/.devcontainer/devcontainer.json", + }, + }, + { + ID: "project2-container", + FriendlyName: "project2-container", + Running: true, + Labels: map[string]string{ + agentcontainers.DevcontainerLocalFolderLabel: "/home/user/project", + agentcontainers.DevcontainerConfigFileLabel: "/home/user/project/.devcontainer/devcontainer.json", + }, + }, + { + ID: "project3-container", + FriendlyName: "project3-container", + Running: true, + Labels: map[string]string{ + agentcontainers.DevcontainerLocalFolderLabel: "/var/lib/project", + agentcontainers.DevcontainerConfigFileLabel: "/var/lib/project/.devcontainer/devcontainer.json", + }, + }, + }, + }, + }, + knownDevcontainers: []codersdk.WorkspaceAgentDevcontainer{ + { + ID: uuid.New(), + Name: "project", // This will cause uniqueness conflicts. + WorkspaceFolder: "/usr/local/project", + ConfigPath: "/usr/local/project/.devcontainer/devcontainer.json", + }, + }, + wantStatus: http.StatusOK, + wantCount: 4, // 1 known + 3 runtime + verify: func(t *testing.T, devcontainers []codersdk.WorkspaceAgentDevcontainer) { + names := make(map[string]int) + for _, dc := range devcontainers { + names[dc.Name]++ + assert.NotEmpty(t, dc.Name, "devcontainer name should not be empty") + } + + for name, count := range names { + assert.Equal(t, 1, count, "name '%s' appears %d times, should be unique", name, count) + } + assert.Len(t, names, 4, "should have four unique devcontainer names") + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + logger := slogtest.Make(t, nil).Leveled(slog.LevelDebug) + + // Setup router with the handler under test. + r := chi.NewRouter() + apiOptions := []agentcontainers.Option{ + agentcontainers.WithLister(tt.lister), + } + + if len(tt.knownDevcontainers) > 0 { + apiOptions = append(apiOptions, agentcontainers.WithDevcontainers(tt.knownDevcontainers)) + } + + api := agentcontainers.NewAPI(logger, apiOptions...) + r.Mount("/", api.Routes()) + + req := httptest.NewRequest(http.MethodGet, "/devcontainers", nil) + rec := httptest.NewRecorder() + r.ServeHTTP(rec, req) + + // Check the response status code. + require.Equal(t, tt.wantStatus, rec.Code, "status code mismatch") + if tt.wantStatus != http.StatusOK { + return + } + + var response codersdk.WorkspaceAgentDevcontainersResponse + err := json.NewDecoder(rec.Body).Decode(&response) + require.NoError(t, err, "unmarshal response failed") + + // Verify the number of devcontainers in the response. + assert.Len(t, response.Devcontainers, tt.wantCount, "wrong number of devcontainers") + + // Run custom verification if provided + if tt.verify != nil && len(response.Devcontainers) > 0 { + tt.verify(t, response.Devcontainers) + } + }) + } + }) +} + +// mustFindDevcontainerByPath returns the devcontainer with the given workspace +// folder path. It fails the test if no matching devcontainer is found. +func mustFindDevcontainerByPath(t *testing.T, devcontainers []codersdk.WorkspaceAgentDevcontainer, path string) codersdk.WorkspaceAgentDevcontainer { + t.Helper() + + for i := range devcontainers { + if devcontainers[i].WorkspaceFolder == path { + return devcontainers[i] + } + } + + require.Failf(t, "no devcontainer found with workspace folder %q", path) + return codersdk.WorkspaceAgentDevcontainer{} // Unreachable, but required for compilation } diff --git a/agent/api.go b/agent/api.go index bb357d1b87da2..0813deb77a146 100644 --- a/agent/api.go +++ b/agent/api.go @@ -37,10 +37,19 @@ func (a *agent) apiHandler() http.Handler { cacheDuration: cacheDuration, } - containerAPI := agentcontainers.NewAPI( - a.logger.Named("containers"), + containerAPIOpts := []agentcontainers.Option{ agentcontainers.WithLister(a.lister), - ) + } + if a.experimentalDevcontainersEnabled { + manifest := a.manifest.Load() + if manifest != nil && len(manifest.Devcontainers) > 0 { + containerAPIOpts = append( + containerAPIOpts, + agentcontainers.WithDevcontainers(manifest.Devcontainers), + ) + } + } + containerAPI := agentcontainers.NewAPI(a.logger.Named("containers"), containerAPIOpts...) promHandler := PrometheusMetricsHandler(a.prometheusRegistry, a.logger) diff --git a/codersdk/workspaceagents.go b/codersdk/workspaceagents.go index ef770712c340a..6a72de5ae4ff3 100644 --- a/codersdk/workspaceagents.go +++ b/codersdk/workspaceagents.go @@ -392,6 +392,12 @@ func (c *Client) WorkspaceAgentListeningPorts(ctx context.Context, agentID uuid. return listeningPorts, json.NewDecoder(res.Body).Decode(&listeningPorts) } +// WorkspaceAgentDevcontainersResponse is the response to the devcontainers +// request. +type WorkspaceAgentDevcontainersResponse struct { + Devcontainers []WorkspaceAgentDevcontainer `json:"devcontainers"` +} + // WorkspaceAgentDevcontainer defines the location of a devcontainer // configuration in a workspace that is visible to the workspace agent. type WorkspaceAgentDevcontainer struct { @@ -399,6 +405,10 @@ type WorkspaceAgentDevcontainer struct { Name string `json:"name"` WorkspaceFolder string `json:"workspace_folder"` ConfigPath string `json:"config_path,omitempty"` + + // Additional runtime fields. + Running bool `json:"running"` + Container *WorkspaceAgentContainer `json:"container,omitempty"` } // WorkspaceAgentContainer describes a devcontainer of some sort From a34e202b1ccd4caa4deb2b8acceb5ce5fb7aa692 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Mon, 14 Apr 2025 13:56:53 +0000 Subject: [PATCH 2/4] fix comments --- agent/agentcontainers/api.go | 7 +++---- agent/agentcontainers/api_test.go | 6 ++---- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/agent/agentcontainers/api.go b/agent/agentcontainers/api.go index f7126f7a5f6bf..5cb86ec925f6b 100644 --- a/agent/agentcontainers/api.go +++ b/agent/agentcontainers/api.go @@ -64,9 +64,9 @@ func WithDevcontainerCLI(dccli DevcontainerCLI) Option { } } -// WithDevcontainers sets the known devcontainers and scripts for the -// API. This allows allows the API to be aware of devcontainers defined -// in the workspace agent manifest. +// WithDevcontainers sets the known devcontainers for the API. This +// allows the API to be aware of devcontainers defined in the workspace +// agent manifest. func WithDevcontainers(devcontainers []codersdk.WorkspaceAgentDevcontainer) Option { return func(api *API) { if len(devcontainers) > 0 { @@ -195,7 +195,6 @@ func (api *API) getContainers(ctx context.Context) (codersdk.WorkspaceAgentListC } // If not in our known list, add as a runtime detected entry. - // Parse name from folder name := path.Base(workspaceFolder) if _, ok := api.devcontainerNames[name]; ok { // Try to find a unique name by appending a number. diff --git a/agent/agentcontainers/api_test.go b/agent/agentcontainers/api_test.go index 7b7ea22d2bfb2..6f2fe5ce84919 100644 --- a/agent/agentcontainers/api_test.go +++ b/agent/agentcontainers/api_test.go @@ -188,7 +188,7 @@ func TestAPI(t *testing.T) { ID: knownDevcontainerID2, Name: "known-devcontainer-2", WorkspaceFolder: "/workspace/known2", - // No config path intentionally + // No config path intentionally. }, } @@ -293,7 +293,6 @@ func TestAPI(t *testing.T) { wantStatus: http.StatusOK, wantCount: 3, // 2 known + 1 runtime verify: func(t *testing.T, devcontainers []codersdk.WorkspaceAgentDevcontainer) { - // Find and verify devcontainers by workspace folder known1 := mustFindDevcontainerByPath(t, devcontainers, "/workspace/known1") known2 := mustFindDevcontainerByPath(t, devcontainers, "/workspace/known2") runtime1 := mustFindDevcontainerByPath(t, devcontainers, "/workspace/runtime1") @@ -373,7 +372,6 @@ func TestAPI(t *testing.T) { wantStatus: http.StatusOK, wantCount: 2, verify: func(t *testing.T, devcontainers []codersdk.WorkspaceAgentDevcontainer) { - // Find devcontainer with ID matching knownDevcontainerID2 var dc2 *codersdk.WorkspaceAgentDevcontainer for i := range devcontainers { if devcontainers[i].ID == knownDevcontainerID2 { @@ -484,7 +482,7 @@ func TestAPI(t *testing.T) { // Verify the number of devcontainers in the response. assert.Len(t, response.Devcontainers, tt.wantCount, "wrong number of devcontainers") - // Run custom verification if provided + // Run custom verification if provided. if tt.verify != nil && len(response.Devcontainers) > 0 { tt.verify(t, response.Devcontainers) } From 359799489b318b5b25a301604dc5e535774527e2 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Mon, 14 Apr 2025 14:03:12 +0000 Subject: [PATCH 3/4] make gen --- site/src/api/typesGenerated.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 3f3d8f92c27e5..465be3b6f0973 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -3234,6 +3234,13 @@ export interface WorkspaceAgentDevcontainer { readonly name: string; readonly workspace_folder: string; readonly config_path?: string; + readonly running: boolean; + readonly container?: WorkspaceAgentContainer; +} + +// From codersdk/workspaceagents.go +export interface WorkspaceAgentDevcontainersResponse { + readonly devcontainers: readonly WorkspaceAgentDevcontainer[]; } // From codersdk/workspaceagents.go From 9ecb6b2999b0c62b8547ff77239ca7eab3b40684 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Mon, 14 Apr 2025 15:22:09 +0000 Subject: [PATCH 4/4] switch to lockCh --- agent/agentcontainers/api.go | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/agent/agentcontainers/api.go b/agent/agentcontainers/api.go index 5cb86ec925f6b..9a028e565b6ca 100644 --- a/agent/agentcontainers/api.go +++ b/agent/agentcontainers/api.go @@ -8,7 +8,6 @@ import ( "path" "slices" "strings" - "sync" "time" "github.com/go-chi/chi/v5" @@ -38,11 +37,9 @@ type API struct { // lockCh protects the below fields. We use a channel instead of a // mutex so we can handle cancellation properly. - lockCh chan struct{} - containers codersdk.WorkspaceAgentListContainersResponse - mtime time.Time - - devcontainersMu sync.Mutex // Protects following. + lockCh chan struct{} + containers codersdk.WorkspaceAgentListContainersResponse + mtime time.Time devcontainerNames map[string]struct{} // Track devcontainer names to avoid duplicates. knownDevcontainers []codersdk.WorkspaceAgentDevcontainer // Track predefined and runtime-detected devcontainers. } @@ -168,9 +165,6 @@ func (api *API) getContainers(ctx context.Context) (codersdk.WorkspaceAgentListC api.containers = updated api.mtime = now - api.devcontainersMu.Lock() - defer api.devcontainersMu.Unlock() - // Reset all known devcontainers to not running. for i := range api.knownDevcontainers { api.knownDevcontainers[i].Running = false @@ -294,9 +288,13 @@ func (api *API) handleListDevcontainers(w http.ResponseWriter, r *http.Request) return } - api.devcontainersMu.Lock() + select { + case <-ctx.Done(): + return + case api.lockCh <- struct{}{}: + } devcontainers := slices.Clone(api.knownDevcontainers) - api.devcontainersMu.Unlock() + <-api.lockCh slices.SortFunc(devcontainers, func(a, b codersdk.WorkspaceAgentDevcontainer) int { if cmp := strings.Compare(a.WorkspaceFolder, b.WorkspaceFolder); cmp != 0 {