feat(agent): add container list handler #16346

johnstcn · 2025-01-30T15:06:58Z

This PR adds an API endpoint to coderd /api/v2/workspaceagents/:id/containers that allows listing containers visible to the agent. Optional filtering by labels is supported.

A later PR will consume this endpoint and use it to expose this information to users.

Under the hood, this operates in a very similar manner to the listening-ports endpoint -- coderd dials the workspace agent and hits an agent endpoint exposed over tailnet.

This initial implementation only supports listing containers using the Docker CLI. Support for other data sources may be added at a future date.

Sample response:

{
  "containers": [
    {
      "created_at": "2025-02-05T12:53:18.943217568Z",
      "id": "1f119acf43202781f7699534fe3613ce2764369148738a16970fb124233a7c58",
      "name": "affectionate_black",
      "image": "busybox:latest",
      "labels": {

      },
      "running": false,
      "ports": [],
      "status": "exited with code 127: failed to create task for container: failed to create shim task: OCI runtime create failed: runc create failed: unable to start container process: exec: \"doesnotexist\": executable file not found in $PATH: unknown",
      "volumes": {

      }
    },
    {
      "created_at": "2025-02-05T12:49:16.182445827Z",
      "id": "972e129e48bcf3420113f0d8d1570d5c3dbb5045ad46cc688c692c76e7cf9382",
      "name": "eloquent_cartwright",
      "image": "busybox:latest",
      "labels": {

      },
      "running": true,
      "ports": [],
      "status": "running",
      "volumes": {
        "testing": "/testing"
      }
    }
  }
]

Sample error response:

{
  "message": "Internal error fetching containers.",
  "detail": "GET http://[fd7a:115c:a1e0:4ca7:9e0e:688f:a0d8:ac55]:4/api/v0/containers: unexpected status code 500: Could not get containers.\n\tError: get containers: run docker ps: exit status 1"
}

johnstcn · 2025-02-05T11:14:59Z

coderd/util/maps/maps.go

+func Subset[T, U comparable](a, b map[T]U) bool {
+	for ka, va := range a {
+		if vb, ok := b[ka]; !ok {
+			return false
+		} else if va != vb {
+			return false
+		}
+	}
+	return true
+}


review: I couldn't find a stdlib function for this, so rolled my own. I'd love to get rid of this.

agent/containers_dockercli.go

johnstcn · 2025-02-05T16:20:34Z

agent/containers_dockercli.go

+	}
+}
+
+// convertDockerVolume converts a Docker volume string to a host path and


Again, if there's pre-existing code I'd be happy to use this.

johnstcn · 2025-02-05T16:21:26Z

codersdk/workspaceagents.go

+	// Running is true if the container is currently running.
+	Running bool `json:"running"`
+	// Ports includes ports exposed by the container.
+	Ports []WorkspaceAgentListeningPort `json:"ports"`


I'm re-using this struct, as any exposed port should also function the same way as a "listening port".

codersdk/workspaceagents.go

dannykopping

Great work 👍
Left mostly small comments

agent/api.go

agent/containers.go

agent/containers_dockercli.go

coderd/workspaceagents.go

dannykopping · 2025-02-06T04:31:32Z

coderd/workspaceagents.go

+		return
+	}
+
+	// Filter in-place by labels


Curious why we don't pass the label filtering down to where the data is collected?
docker can filter by label

I found that co-locating filtering and caching gets difficult.

That's a good point Cian, although I'd question the premise of caching at this stage, did you find it was a requirement to have any form of responsive UI? Is docker ps too slow?

It would be easier to just run multiple docker ps commands in parallel if there are multiple requests inbound.

Eh, perhaps. The caching logic isn't overly complex, and I was mainly copying the pattern from the listening-ports handler. I can remove it if you feel strongly about it, but I suspect it'll be needed -- especially if someone is running lots of containers in their workspace.

Alright, I simply raised it since a little slow is probably fine for the first deliverable, and caching prematurely may solve or hide the wrong problem. I leave it up to you though.

coderd/workspaceagents_test.go

codersdk/workspaceagents.go

mafredri

Nice work, Cian! Looking past nits and suggestions, in broad strokes I'd like to see:

Separate agent/containers package
No explicit dependency on docker for testing (e.g. skip)
Single "container" output from agent that can be consumed in a generic way in e.g. UI, so that docker can be swapped out for another backend and everything behaves the same
s/container/devcontainer/ makes sense to me, but perhaps I'm overlooking something

mafredri · 2025-02-06T09:15:35Z

agent/api.go

@@ -35,7 +35,11 @@ func (a *agent) apiHandler() http.Handler {
 		ignorePorts:   cpy,
 		cacheDuration: cacheDuration,
 	}
+	ch := &containersHandler{


At this level, calling this devcontainers rather than containers would make more sense to me. The idea being that we process devcontainer related stuff on the agent and expose it in a neat package to coderd and don't need logic over there to handle different types.

I might be overlooking something, though, so feel free to dismiss if there's a good reason for the naming.

agent/containers.go

mafredri · 2025-02-06T10:07:43Z

coderd/workspaceagents.go

+		return
+	}
+
+	// Filter in-place by labels


That's a good point Cian, although I'd question the premise of caching at this stage, did you find it was a requirement to have any form of responsive UI? Is docker ps too slow?

It would be easier to just run multiple docker ps commands in parallel if there are multiple requests inbound.

coderd/workspaceagents_test.go

codersdk/workspaceagents.go

agent/agentcontainers/containers_dockercli.go

johnstcn · 2025-02-06T18:51:32Z

@dannykopping @mafredri I think I'm ready for round 2 now. I still need to use agentexec.Execer instead but I think I've addressed all of your comments.

mafredri

This PR is looking quite good! Just a few minor things and while testing this PR I realized we don't include mounts in the output. I think that could be relevant, like volumes, but I'll defer to you whether or not we include those.

We should still make the agentexec changes, but other than that, approved!

agent/agentcontainers/containers.go

agent/agentcontainers/containers_dockercli.go

dannykopping

LGTM

Adds an API endpoint to coderd `/api/v2/workspaceagents/:id/containers` that allows listing containers visible to the agent. This initial implementation only supports listing containers using the Docker CLI. Support for other data sources may be added at a future date.

johnstcn self-assigned this Jan 30, 2025

johnstcn force-pushed the cj/agent-containers-list branch 4 times, most recently from a17ca58 to 798f1a3 Compare February 4, 2025 21:38

johnstcn commented Feb 5, 2025

View reviewed changes

johnstcn force-pushed the cj/agent-containers-list branch from 7a55cf7 to a56462e Compare February 5, 2025 15:58

johnstcn marked this pull request as ready for review February 5, 2025 16:15

johnstcn requested review from mafredri and dannykopping February 5, 2025 16:17

johnstcn commented Feb 5, 2025

View reviewed changes

dannykopping reviewed Feb 6, 2025

View reviewed changes

mafredri reviewed Feb 6, 2025

View reviewed changes

johnstcn commented Feb 6, 2025

View reviewed changes

agent/agentcontainers/containers_dockercli.go Outdated Show resolved Hide resolved

johnstcn commented Feb 6, 2025

View reviewed changes

agent/agentcontainers/containers_dockercli.go Outdated Show resolved Hide resolved

mafredri approved these changes Feb 7, 2025

View reviewed changes

agent/agentcontainers/containers.go Outdated Show resolved Hide resolved

agent/agentcontainers/containers.go Outdated Show resolved Hide resolved

agent/agentcontainers/containers.go Outdated Show resolved Hide resolved

agent/agentcontainers/containers_dockercli.go Show resolved Hide resolved

dannykopping approved these changes Feb 10, 2025

View reviewed changes

johnstcn added 12 commits February 10, 2025 11:06

Apply suggestions from code review

b3fed45

lint

9342c69

address code review comments

9fe2409

move to agentcontainers package

fe17940

remove docker dependency from tests

ca25f69

update .gitattributes

7f0b970

handle r.Context() canceled

ab1469f

move mock to its own package

51e5976

init lockCh in New(), remove sync.Once

b53552d

use agentexec

1dbcf0f

correctly detect ports and volumes

8b081ac

johnstcn force-pushed the cj/agent-containers-list branch from 2c87b7e to 8b081ac Compare February 10, 2025 11:06

ci: add go tools to build-dylib

37ebbeb

johnstcn merged commit 31b1ff7 into main Feb 10, 2025
35 checks passed

johnstcn deleted the cj/agent-containers-list branch February 10, 2025 11:29

github-actions bot locked and limited conversation to collaborators Feb 10, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(agent): add container list handler #16346

feat(agent): add container list handler #16346

johnstcn commented Jan 30, 2025 •

edited

Loading

johnstcn Feb 5, 2025

johnstcn Feb 5, 2025

johnstcn Feb 5, 2025

dannykopping left a comment

dannykopping Feb 6, 2025

johnstcn Feb 6, 2025

mafredri Feb 6, 2025

johnstcn Feb 6, 2025

mafredri Feb 6, 2025

mafredri left a comment

mafredri Feb 6, 2025

mafredri Feb 6, 2025

johnstcn commented Feb 6, 2025

mafredri left a comment

dannykopping left a comment

feat(agent): add container list handler #16346

feat(agent): add container list handler #16346

Conversation

johnstcn commented Jan 30, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dannykopping left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mafredri left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

johnstcn commented Feb 6, 2025

mafredri left a comment

Choose a reason for hiding this comment

dannykopping left a comment

Choose a reason for hiding this comment

johnstcn commented Jan 30, 2025 •

edited

Loading