Skip to content

Commit 798f1a3

Browse files
johnstcnjohnstcn
committed
[ci skip] add filtering by labels
1 parent 525ed1e commit 798f1a3

File tree

6 files changed

+164
-25
lines changed

6 files changed

+164
-25
lines changed

‎coderd/util/maps/maps.go

+14
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
package maps
2+
3+
// Subset returns true if all the keys of a are present
4+
// in b and have the same values.
5+
func Subset[T, U comparable](a, b map[T]U) bool {
6+
for ka, va := range a {
7+
if vb, ok := b[ka]; !ok {
8+
return false
9+
} else if va != vb {
10+
return false
11+
}
12+
}
13+
return true
14+
}

‎coderd/util/maps/maps_test.go

+63
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
package maps_test
2+
3+
import (
4+
"strconv"
5+
"testing"
6+
7+
"github.com/coder/coder/v2/coderd/util/maps"
8+
)
9+
10+
func TestSubset(t *testing.T) {
11+
t.Parallel()
12+
13+
for idx, tc := range []struct {
14+
a map[string]string
15+
b map[string]string
16+
expected bool
17+
}{
18+
{
19+
a: nil,
20+
b: nil,
21+
expected: true,
22+
},
23+
{
24+
a: map[string]string{},
25+
b: map[string]string{},
26+
expected: true,
27+
},
28+
{
29+
a: map[string]string{"a": "1", "b": "2"},
30+
b: map[string]string{"a": "1", "b": "2"},
31+
expected: true,
32+
},
33+
{
34+
a: map[string]string{"a": "1", "b": "2"},
35+
b: map[string]string{"a": "1"},
36+
expected: false,
37+
},
38+
{
39+
a: map[string]string{"a": "1"},
40+
b: map[string]string{"a": "1", "b": "2"},
41+
expected: true,
42+
},
43+
{
44+
a: map[string]string{"a": "1", "b": "2"},
45+
b: map[string]string{},
46+
expected: false,
47+
},
48+
{
49+
a: map[string]string{"a": "1", "b": "2"},
50+
b: map[string]string{"a": "1", "b": "3"},
51+
expected: false,
52+
},
53+
} {
54+
t.Run("#"+strconv.Itoa(idx), func(t *testing.T) {
55+
t.Parallel()
56+
57+
actual := maps.Subset(tc.a, tc.b)
58+
if actual != tc.expected {
59+
t.Errorf("expected %v, got %v", tc.expected, actual)
60+
}
61+
})
62+
}
63+
}

‎coderd/workspaceagents.go

+26-2
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ import (
3434
"github.com/coder/coder/v2/coderd/jwtutils"
3535
"github.com/coder/coder/v2/coderd/rbac"
3636
"github.com/coder/coder/v2/coderd/rbac/policy"
37+
maputil "github.com/coder/coder/v2/coderd/util/maps"
3738
"github.com/coder/coder/v2/coderd/wspubsub"
3839
"github.com/coder/coder/v2/codersdk"
3940
"github.com/coder/coder/v2/codersdk/agentsdk"
@@ -678,10 +679,28 @@ func (api *API) workspaceAgentListeningPorts(rw http.ResponseWriter, r *http.Req
678679
httpapi.Write(ctx, rw, http.StatusOK, portsResponse)
679680
}
680681

682+
// TODO: swagger summary
681683
func (api *API) workspaceAgentListContainers(rw http.ResponseWriter, r *http.Request) {
682684
ctx := r.Context()
683685
workspaceAgent := httpmw.WorkspaceAgentParam(r)
684686

687+
labelParam, ok := r.URL.Query()["label"]
688+
if !ok {
689+
labelParam = []string{}
690+
}
691+
labels := make(map[string]string)
692+
for _, label := range labelParam {
693+
kvs := strings.Split(label, "=")
694+
if len(kvs) != 2 {
695+
httpapi.Write(r.Context(), rw, http.StatusBadRequest, codersdk.Response{
696+
Message: "Invalid label format",
697+
Detail: "Labels must be in the format key=value",
698+
})
699+
return
700+
}
701+
labels[kvs[0]] = kvs[1]
702+
}
703+
685704
// If the agent is unreachable, the request will hang. Assume that if we
686705
// don't get a response after 30s that the agent is unreachable.
687706
ctx, cancel := context.WithTimeout(ctx, 30*time.Second)
@@ -721,7 +740,7 @@ func (api *API) workspaceAgentListContainers(rw http.ResponseWriter, r *http.Req
721740
defer release()
722741

723742
// Get a list of containers that the agent is able to detect
724-
containers, err := agentConn.ListContainers(ctx)
743+
cts, err := agentConn.ListContainers(ctx)
725744
if err != nil {
726745
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
727746
Message: "Internal error fetching containers.",
@@ -730,7 +749,12 @@ func (api *API) workspaceAgentListContainers(rw http.ResponseWriter, r *http.Req
730749
return
731750
}
732751

733-
httpapi.Write(ctx, rw, http.StatusOK, containers)
752+
// Filter in-place by labels
753+
filtered := slices.DeleteFunc(cts, func(ct codersdk.WorkspaceAgentContainer) bool {
754+
return !maputil.Subset(labels, ct.Labels)
755+
})
756+
757+
httpapi.Write(ctx, rw, http.StatusOK, codersdk.WorkspaceAgentListContainersResponse{Containers: filtered})
734758
}
735759

736760
// @Summary Get connection info for workspace agent

‎coderd/workspaceagents_test.go

+40-16
Original file line numberDiff line numberDiff line change
@@ -1058,14 +1058,20 @@ func TestWorkspaceAgentListeningPorts(t *testing.T) {
10581058
func TestWorkspaceAgentContainers(t *testing.T) {
10591059
t.Parallel()
10601060

1061+
if runtime.GOOS != "linux" {
1062+
t.Skip("this test creates containers, which is flaky on non-linux runners")
1063+
}
1064+
10611065
pool, err := dockertest.NewPool("")
10621066
require.NoError(t, err, "Could not connect to docker")
1063-
testLabelValue := uuid.New().String()
1067+
testLabels := map[string]string{
1068+
"com.coder.test": uuid.New().String(),
1069+
}
10641070
ct, err := pool.RunWithOptions(&dockertest.RunOptions{
10651071
Repository: "busybox",
10661072
Tag: "latest",
10671073
Cmd: []string{"sleep", "infnity"},
1068-
Labels: map[string]string{"com.coder.test": testLabelValue},
1074+
Labels: testLabels,
10691075
}, func(config *docker.HostConfig) {
10701076
config.AutoRemove = true
10711077
config.RestartPolicy = docker.RestartPolicy{Name: "no"}
@@ -1075,6 +1081,21 @@ func TestWorkspaceAgentContainers(t *testing.T) {
10751081
assert.NoError(t, pool.Purge(ct), "Could not purge resource")
10761082
})
10771083

1084+
// Start another container which we will expect to ignore.
1085+
ct2, err := pool.RunWithOptions(&dockertest.RunOptions{
1086+
Repository: "busybox",
1087+
Tag: "latest",
1088+
Cmd: []string{"sleep", "infnity"},
1089+
Labels: map[string]string{"com.coder.test": "ignoreme"},
1090+
}, func(config *docker.HostConfig) {
1091+
config.AutoRemove = true
1092+
config.RestartPolicy = docker.RestartPolicy{Name: "no"}
1093+
})
1094+
require.NoError(t, err, "Could not start second test docker container")
1095+
t.Cleanup(func() {
1096+
assert.NoError(t, pool.Purge(ct2), "Could not purge resource")
1097+
})
1098+
10781099
client, db := coderdtest.NewWithDatabase(t, &coderdtest.Options{})
10791100

10801101
user := coderdtest.CreateFirstUser(t, client)
@@ -1091,23 +1112,26 @@ func TestWorkspaceAgentContainers(t *testing.T) {
10911112
agentID := resources[0].Agents[0].ID
10921113

10931114
ctx := testutil.Context(t, testutil.WaitLong)
1094-
res, err := client.WorkspaceAgentListContainers(ctx, agentID)
1095-
require.NoError(t, err, "failed to list containers")
1096-
require.NotEmpty(t, res.Containers, "expected to find containers")
10971115

1098-
var found bool
1116+
// If we filter by testLabels, we should only get one container back.
1117+
res, err := client.WorkspaceAgentListContainers(ctx, agentID, testLabels)
1118+
require.NoError(t, err, "failed to list containers filtered by test label")
1119+
require.Len(t, res.Containers, 1, "expected exactly one container")
1120+
assert.Equal(t, ct.Container.ID, res.Containers[0].ID)
1121+
assert.Equal(t, "busybox:latest", res.Containers[0].Image)
1122+
assert.Equal(t, ct.Container.Config.Labels, res.Containers[0].Labels)
1123+
assert.Equal(t, strings.TrimPrefix(ct.Container.Name, "/"), res.Containers[0].FriendlyName)
1124+
1125+
// List all containers and ensure we get at least both (there may be more).
1126+
res, err = client.WorkspaceAgentListContainers(ctx, agentID, nil)
1127+
require.NoError(t, err, "failed to list all containers")
1128+
require.NotEmpty(t, res.Containers, "expected to find containers")
1129+
var found []string
10991130
for _, c := range res.Containers {
1100-
if c.ID == ct.Container.ID {
1101-
found = true
1102-
assert.Equal(t, ct.Container.ID, c.ID)
1103-
assert.Equal(t, "busybox:latest", c.Image)
1104-
// The container name is prefixed with a slash.
1105-
assert.Equal(t, strings.TrimPrefix(ct.Container.Name, "/"), c.FriendlyName)
1106-
assert.Equal(t, ct.Container.Config.Labels, c.Labels)
1107-
break
1108-
}
1131+
found = append(found, c.ID)
11091132
}
1110-
require.True(t, found, "expected to find container")
1133+
require.Contains(t, found, ct.Container.ID, "expected to find first container without label filter")
1134+
require.Contains(t, found, ct2.Container.ID, "expected to find first container without label filter")
11111135
}
11121136

11131137
func TestWorkspaceAgentAppHealth(t *testing.T) {

‎codersdk/workspaceagents.go

+16-2
Original file line numberDiff line numberDiff line change
@@ -411,10 +411,24 @@ type WorkspaceAgentListContainersResponse struct {
411411
Containers []WorkspaceAgentContainer `json:"containers"`
412412
}
413413

414+
// WorkspaceAgentContainersLabelFilter is a RequestOption for filtering
415+
// listing containers by labels.
416+
func WorkspaceAgentContainersLabelFilter(kvs map[string]string) RequestOption {
417+
return func(r *http.Request) {
418+
q := r.URL.Query()
419+
for k, v := range kvs {
420+
kv := fmt.Sprintf("%s=%s", k, v)
421+
q.Add("label", kv)
422+
}
423+
r.URL.RawQuery = q.Encode()
424+
}
425+
}
426+
414427
// WorkspaceAgentListContainers returns a list of containers that are currently
415428
// running on a Docker daemon accessible to the workspace agent.
416-
func (c *Client) WorkspaceAgentListContainers(ctx context.Context, agentID uuid.UUID) (WorkspaceAgentListContainersResponse, error) {
417-
res, err := c.Request(ctx, http.MethodGet, fmt.Sprintf("/api/v2/workspaceagents/%s/containers", agentID), nil)
429+
func (c *Client) WorkspaceAgentListContainers(ctx context.Context, agentID uuid.UUID, labels map[string]string) (WorkspaceAgentListContainersResponse, error) {
430+
lf := WorkspaceAgentContainersLabelFilter(labels)
431+
res, err := c.Request(ctx, http.MethodGet, fmt.Sprintf("/api/v2/workspaceagents/%s/containers", agentID), nil, lf)
418432
if err != nil {
419433
return WorkspaceAgentListContainersResponse{}, err
420434
}

‎codersdk/workspacesdk/agentconn.go

+5-5
Original file line numberDiff line numberDiff line change
@@ -337,19 +337,19 @@ func (c *AgentConn) PrometheusMetrics(ctx context.Context) ([]byte, error) {
337337
}
338338

339339
// ListContainers returns a response from the agent's containers endpoint
340-
func (c *AgentConn) ListContainers(ctx context.Context) (codersdk.WorkspaceAgentListContainersResponse, error) {
340+
func (c *AgentConn) ListContainers(ctx context.Context) ([]codersdk.WorkspaceAgentContainer, error) {
341341
ctx, span := tracing.StartSpan(ctx)
342342
defer span.End()
343343
res, err := c.apiRequest(ctx, http.MethodGet, "/api/v0/containers", nil)
344344
if err != nil {
345-
return codersdk.WorkspaceAgentListContainersResponse{}, xerrors.Errorf("do request: %w", err)
345+
return nil, xerrors.Errorf("do request: %w", err)
346346
}
347347
defer res.Body.Close()
348348
if res.StatusCode != http.StatusOK {
349-
return codersdk.WorkspaceAgentListContainersResponse{}, codersdk.ReadBodyAsError(res)
349+
return nil, codersdk.ReadBodyAsError(res)
350350
}
351-
var resp codersdk.WorkspaceAgentListContainersResponse
352-
return resp, json.NewDecoder(res.Body).Decode(&resp.Containers)
351+
var resp []codersdk.WorkspaceAgentContainer
352+
return resp, json.NewDecoder(res.Body).Decode(&resp)
353353
}
354354

355355
// apiRequest makes a request to the workspace agent's HTTP API server.

0 commit comments

Comments
 (0)