feat(agent/agentcontainers): add devcontainers list endpoint #17389
Conversation
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
| @@ -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{}{}: | |||
There was a problem hiding this comment.
Review: Drive-by bug fix.
| @@ -158,7 +234,7 @@ func (api *API) handleRecreate(w http.ResponseWriter, r *http.Request) { | |||
| return | |||
| } | |||
|
|
|||
| containers, err := api.cl.List(ctx) | |||
| containers, err := api.getContainers(ctx) | |||
There was a problem hiding this comment.
Review: Drive-by bug fix.
agent/agentcontainers/api.go
Outdated
| lockCh chan struct{} | ||
| containers codersdk.WorkspaceAgentListContainersResponse | ||
| mtime time.Time | ||
|
|
||
| devcontainersMu sync.Mutex // Protects following. |
There was a problem hiding this comment.
Do we need a separate mutex here? Can we not reuse lockCh?
There was a problem hiding this comment.
It's not as ergonomic but sure, switched to it 😄
|
|
||
| // Check if the container is running and update the known devcontainers. | ||
| for _, container := range updated.Containers { | ||
| workspaceFolder := container.Labels[DevcontainerLocalFolderLabel] |
There was a problem hiding this comment.
thought, non-blocking: Given that a WorkspaceAgentDevcontainer is a superset of a WorkspaceAgentContainer, I wonder if we could instead store the optional devcontainer-specific fields in WorkspaceAgentContainer?
There was a problem hiding this comment.
The main issue with that approach is: how do we represent a devcontainer we know about but that has no container? (I.e. either it wasn't started, or it was deleted.)
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
The intent here is to replace the currently used containers endpoint with this one as it provides more context on devcontainers, including ones that have not been started yet, or removed at some point.