From be7ddcd15a2b2c4ad1168e8fe8b462d920c7e118 Mon Sep 17 00:00:00 2001 From: silverwind Date: Wed, 14 Oct 2026 08:00:17 +0000 Subject: [PATCH] fix: give each docker proxy its own job label key (#1314) A runner nested behind another runner's docker proxy, as in gitea.com CI, had its `com.gitea.runner.job` label overwritten by the outer proxy, so its job cleanup removed nothing. This failed `TestDockerProxyWithDaemon` depending on the CI host, for example on https://gitea.com/gitea/runner/pulls/1312. Each proxy now sets `com.gitea.runner.job.` and cleans up by that key, so nested proxies keep each other's. This replaces the `com.gitea.runner.job=` label. Reviewed-on: https://gitea.com/gitea/runner/pulls/1314 Reviewed-by: bircni Co-authored-by: silverwind --- act/container/docker_proxy.go | 6 +-- act/container/docker_proxy_test.go | 45 ++++++++++--------- .../testdata/docker-proxy/action/index.js | 6 +-- act/runner/testdata/docker-proxy/push.yml | 2 +- 4 files changed, 31 insertions(+), 28 deletions(-) diff --git a/act/container/docker_proxy.go b/act/container/docker_proxy.go index fff3821d..e7b39b69 100644 --- a/act/container/docker_proxy.go +++ b/act/container/docker_proxy.go @@ -37,7 +37,7 @@ import ( ) const ( - jobLabel = "com.gitea.runner.job" + jobLabel = "com.gitea.runner.job." maxCreateBody = 8 << 20 dockerProxyProbeTimeout = 5 * time.Second @@ -302,7 +302,7 @@ func rewriteCreate(r *http.Request, job string, mounts map[string]string) error if config.Labels == nil { config.Labels = make(map[string]string) } - config.Labels[jobLabel] = job + config.Labels[jobLabel+job] = "" // per-job key, so nested proxies keep each other's if fields["Labels"], err = json.Marshal(config.Labels); err != nil { return err } @@ -560,7 +560,7 @@ func RemoveDockerJobResources(ctx context.Context, job string) error { func removeLabelled(ctx context.Context, cli client.APIClient, job string) error { logger := common.Logger(ctx) - filters := make(client.Filters).Add("label", jobLabel+"="+job) + filters := make(client.Filters).Add("label", jobLabel+job) containers, err := cli.ContainerList(ctx, client.ContainerListOptions{All: true, Filters: filters}) errs := []error{err} for _, c := range containers.Items { diff --git a/act/container/docker_proxy_test.go b/act/container/docker_proxy_test.go index 880bad60..18cb2812 100644 --- a/act/container/docker_proxy_test.go +++ b/act/container/docker_proxy_test.go @@ -146,52 +146,52 @@ func TestDockerProxy(t *testing.T) { t.Run("labels creates", func(t *testing.T) { for _, testCase := range []struct{ path, body, want string }{ { - "/v1.47/containers/create", `{"Image":"alpine","Unknown":{"enabled":true},"Labels":{"own":"1","com.gitea.runner.job":"other"}}`, - `{"Image":"alpine","Unknown":{"enabled":true},"Labels":{"own":"1","com.gitea.runner.job":"job-1"}}`, + "/v1.47/containers/create", `{"Image":"alpine","Unknown":{"enabled":true},"Labels":{"own":"1","com.gitea.runner.job.job-0":""}}`, + `{"Image":"alpine","Unknown":{"enabled":true},"Labels":{"own":"1","com.gitea.runner.job.job-0":"","com.gitea.runner.job.job-1":""}}`, }, { "/containers/create", `{"HostConfig":{"Binds":["/workspace/o/r:/src:ro","/workspace/o/r/../r/sub/:/sub","workspace/o/r:/relative","/workspace/o/rest:/rest","/workspace/o/r/tmp/x:/tmp","named:/named","/anonymous","/workspace/o/r:ro","/volumes/job/_data/x:/daemon-path"]},"DriverOpts":{"o":"bind","device":"/workspace/o/r"},"Labels":{"Foo":"1","foo":"2"}}`, - `{"HostConfig":{"Binds":["/volumes/job/_data:/src:ro","/volumes/job/_data/sub:/sub","workspace/o/r:/relative","/workspace/o/rest:/rest","/workspace/o/r/tmp/x:/tmp","named:/named","/anonymous","/workspace/o/r:ro","/volumes/job/_data/x:/daemon-path"]},"DriverOpts":{"o":"bind","device":"/workspace/o/r"},"Labels":{"Foo":"1","foo":"2","com.gitea.runner.job":"job-1"}}`, + `{"HostConfig":{"Binds":["/volumes/job/_data:/src:ro","/volumes/job/_data/sub:/sub","workspace/o/r:/relative","/workspace/o/rest:/rest","/workspace/o/r/tmp/x:/tmp","named:/named","/anonymous","/workspace/o/r:ro","/volumes/job/_data/x:/daemon-path"]},"DriverOpts":{"o":"bind","device":"/workspace/o/r"},"Labels":{"Foo":"1","foo":"2","com.gitea.runner.job.job-1":""}}`, }, { "/containers/create", `{"HostConfig":{"Mounts":[{"Type":"bind","Source":"/var/run/docker.sock","Target":"/var/run/docker.sock"},{"type":"bind","source":"/workspace/o/r/sub/","target":"/m"},{"Type":"bind","Source":"/workspace/o/r/sub/../data","Target":"/dotted"},{"Type":"bind","Source":"/workspace/o/r/../r","Target":"/escaping"},{"Type":"BIND","Source":"/workspace/o/r"},{"Type":"volume","Source":"/workspace/o/r"}]}}`, - `{"HostConfig":{"Mounts":[{"Type":"bind","Source":"/tmp/p/docker.sock","Target":"/var/run/docker.sock"},{"Type":"bind","Source":"/volumes/job/_data/sub/","target":"/m"},{"Type":"bind","Source":"/volumes/job/_data/sub/../data","Target":"/dotted"},{"Type":"bind","Source":"/volumes/job/_data","Target":"/escaping"},{"Type":"BIND","Source":"/workspace/o/r"},{"Type":"volume","Source":"/workspace/o/r"}]},"Labels":{"com.gitea.runner.job":"job-1"}}`, + `{"HostConfig":{"Mounts":[{"Type":"bind","Source":"/tmp/p/docker.sock","Target":"/var/run/docker.sock"},{"Type":"bind","Source":"/volumes/job/_data/sub/","target":"/m"},{"Type":"bind","Source":"/volumes/job/_data/sub/../data","Target":"/dotted"},{"Type":"bind","Source":"/volumes/job/_data","Target":"/escaping"},{"Type":"BIND","Source":"/workspace/o/r"},{"Type":"volume","Source":"/workspace/o/r"}]},"Labels":{"com.gitea.runner.job.job-1":""}}`, }, { "/containers/create", `{"HostConfig":{"Mounts":[{"Type":"volume","VolumeOptions":{"DriverConfig":{"Options":{"o":"bind","device":"/workspace/o/r/data"}}}},{"Type":"volume","VolumeOptions":{"DriverConfig":{"Options":{"o":"unbindable","device":"/workspace/o/r"}}}},{"Type":"volume","VolumeOptions":{"DriverConfig":{"Options":{"o":"bind,remount","device":"/workspace/o/r"}}}},{"Type":"volume","VolumeOptions":{"DriverConfig":{"Name":"plugin","Options":{"o":"bind","device":"/workspace/o/r"}}}}]}}`, - `{"HostConfig":{"Mounts":[{"Type":"volume","VolumeOptions":{"DriverConfig":{"Options":{"o":"bind","device":"/volumes/job/_data/data"}}}},{"Type":"volume","VolumeOptions":{"DriverConfig":{"Options":{"o":"unbindable","device":"/workspace/o/r"}}}},{"Type":"volume","VolumeOptions":{"DriverConfig":{"Options":{"o":"bind,remount","device":"/workspace/o/r"}}}},{"Type":"volume","VolumeOptions":{"DriverConfig":{"Name":"plugin","Options":{"o":"bind","device":"/workspace/o/r"}}}}]},"Labels":{"com.gitea.runner.job":"job-1"}}`, + `{"HostConfig":{"Mounts":[{"Type":"volume","VolumeOptions":{"DriverConfig":{"Options":{"o":"bind","device":"/volumes/job/_data/data"}}}},{"Type":"volume","VolumeOptions":{"DriverConfig":{"Options":{"o":"unbindable","device":"/workspace/o/r"}}}},{"Type":"volume","VolumeOptions":{"DriverConfig":{"Options":{"o":"bind,remount","device":"/workspace/o/r"}}}},{"Type":"volume","VolumeOptions":{"DriverConfig":{"Name":"plugin","Options":{"o":"bind","device":"/workspace/o/r"}}}}]},"Labels":{"com.gitea.runner.job.job-1":""}}`, }, { "/containers/create", `{"HostConfig":{"privileged":true,"Privileged":false,"Binds":["/workspace/o/r:/src"]},"hoſtconfig":null}`, - `{"HostConfig":{"privileged":true,"Privileged":false,"Binds":["/workspace/o/r:/src"]},"hoſtconfig":null,"Labels":{"com.gitea.runner.job":"job-1"}}`, + `{"HostConfig":{"privileged":true,"Privileged":false,"Binds":["/workspace/o/r:/src"]},"hoſtconfig":null,"Labels":{"com.gitea.runner.job.job-1":""}}`, }, { "/networks/create", `{"Name":"n"}`, - `{"Name":"n","Labels":{"com.gitea.runner.job":"job-1"}}`, + `{"Name":"n","Labels":{"com.gitea.runner.job.job-1":""}}`, }, { "/volumes/create", `{"Name":"v","labels":null,"DriverOpts":{"type":"none","o":"rbind,ro","device":"/workspace/o/r/"},"HostConfig":{"Binds":["/workspace/o/r:/src"]}}`, - `{"Name":"v","Labels":{"com.gitea.runner.job":"job-1"},"DriverOpts":{"type":"none","o":"rbind,ro","device":"/volumes/job/_data/"},"HostConfig":{"Binds":["/workspace/o/r:/src"]}}`, + `{"Name":"v","Labels":{"com.gitea.runner.job.job-1":""},"DriverOpts":{"type":"none","o":"rbind,ro","device":"/volumes/job/_data/"},"HostConfig":{"Binds":["/workspace/o/r:/src"]}}`, }, { "/volumes/create", `{"Name":"d","DriverOpts":{"o":"bind","device":1,"device":"/workspace/o/r"}}`, - `{"Name":"d","DriverOpts":{"o":"bind","device":1,"device":"/workspace/o/r"},"Labels":{"com.gitea.runner.job":"job-1"}}`, + `{"Name":"d","DriverOpts":{"o":"bind","device":1,"device":"/workspace/o/r"},"Labels":{"com.gitea.runner.job.job-1":""}}`, }, { "/volumes/create", `{"Name":"p","Driver":"plugin","DriverOpts":{"o":"bind","device":"/workspace/o/r"}}`, - `{"Name":"p","Driver":"plugin","DriverOpts":{"o":"bind","device":"/workspace/o/r"},"Labels":{"com.gitea.runner.job":"job-1"}}`, + `{"Name":"p","Driver":"plugin","DriverOpts":{"o":"bind","device":"/workspace/o/r"},"Labels":{"com.gitea.runner.job.job-1":""}}`, }, { "/volumes/create", "", - `{"Labels":{"com.gitea.runner.job":"job-1"}}`, + `{"Labels":{"com.gitea.runner.job.job-1":""}}`, }, { "/volumes/create", "null", - `{"Labels":{"com.gitea.runner.job":"job-1"}}`, + `{"Labels":{"com.gitea.runner.job.job-1":""}}`, }, { "/volumes/create", `{"Labels":{"discard":"1"},"labels":null,"LABELS":{"own":"1"},"LABELS":{"extra":"2"}}`, - `{"Labels":{"own":"1","extra":"2","com.gitea.runner.job":"job-1"}}`, + `{"Labels":{"own":"1","extra":"2","com.gitea.runner.job.job-1":""}}`, }, } { resp, err := client.Post("http://docker"+testCase.path, "application/json", strings.NewReader(testCase.body)) @@ -244,7 +244,7 @@ func TestDockerProxy(t *testing.T) { require.NoError(t, err) require.NoError(t, resp.Body.Close()) require.Equal(t, http.StatusCreated, resp.StatusCode) - assert.JSONEq(t, `{"Labels":{"com.gitea.runner.job":"job-1"}}`, string(<-bodies)) + assert.JSONEq(t, `{"Labels":{"com.gitea.runner.job.job-1":""}}`, string(<-bodies)) } }) @@ -276,7 +276,7 @@ func TestDockerProxy(t *testing.T) { require.NoError(t, err) require.NoError(t, resp.Body.Close()) require.Equal(t, http.StatusCreated, resp.StatusCode) - assert.JSONEq(t, `{"Labels":{"com.gitea.runner.job":"job-1"}}`, string(<-bodies)) + assert.JSONEq(t, `{"Labels":{"com.gitea.runner.job.job-1":""}}`, string(<-bodies)) require.NoError(t, <-closed) }) } @@ -286,7 +286,7 @@ func TestRemoveLabelledRemovesContainersNetworksAndVolumes(t *testing.T) { listFailure := errors.New("listing failed") volumeFailure := errors.New("volume removal failed") ctx := t.Context() - filters := make(mobyclient.Filters).Add("label", jobLabel+"=job-1") + filters := make(mobyclient.Filters).Add("label", jobLabel+"job-1") cli := &mockDockerClient{} cli.On("ContainerList", ctx, mobyclient.ContainerListOptions{All: true, Filters: filters}). Return(mobyclient.ContainerListResult{Items: []container.Summary{{ID: "c1", Names: []string{"/app"}}}}, nil).Once() @@ -313,6 +313,7 @@ func TestRemoveLabelledRemovesContainersNetworksAndVolumes(t *testing.T) { func TestDockerProxyWithDaemon(t *testing.T) { requireDocker(t) ctx := t.Context() + cleanupCtx := context.WithoutCancel(ctx) require.NoError(t, NewDockerPullExecutor(NewDockerPullExecutorInput{Image: "alpine"})(ctx)) direct, err := GetDockerClient(ctx) require.NoError(t, err) @@ -337,22 +338,24 @@ func TestDockerProxyWithDaemon(t *testing.T) { Config: &container.Config{Image: "alpine", Cmd: []string{"sleep", "300"}, Labels: map[string]string{"own": "1"}}, }) require.NoError(t, err) - t.Cleanup(func() { _, _ = direct.ContainerRemove(ctx, created.ID, mobyclient.ContainerRemoveOptions{Force: true}) }) + t.Cleanup(func() { + _, _ = direct.ContainerRemove(cleanupCtx, created.ID, mobyclient.ContainerRemoveOptions{Force: true}) + }) _, err = viaProxy.ContainerStart(ctx, created.ID, mobyclient.ContainerStartOptions{}) require.NoError(t, err) net, err := viaProxy.NetworkCreate(ctx, job, mobyclient.NetworkCreateOptions{}) require.NoError(t, err) - t.Cleanup(func() { _, _ = direct.NetworkRemove(ctx, net.ID, mobyclient.NetworkRemoveOptions{}) }) + t.Cleanup(func() { _, _ = direct.NetworkRemove(cleanupCtx, net.ID, mobyclient.NetworkRemoveOptions{}) }) _, err = viaProxy.VolumeCreate(ctx, mobyclient.VolumeCreateOptions{Name: job}) require.NoError(t, err) - t.Cleanup(func() { _, _ = direct.VolumeRemove(ctx, job, mobyclient.VolumeRemoveOptions{Force: true}) }) + t.Cleanup(func() { _, _ = direct.VolumeRemove(cleanupCtx, job, mobyclient.VolumeRemoveOptions{Force: true}) }) inspected, err := direct.ContainerInspect(ctx, created.ID, mobyclient.ContainerInspectOptions{}) require.NoError(t, err) - assert.Equal(t, map[string]string{"own": "1", jobLabel: job}, inspected.Container.Config.Labels) + assert.Subset(t, inspected.Container.Config.Labels, map[string]string{"own": "1", jobLabel + job: ""}) vol, err := direct.VolumeInspect(ctx, job, mobyclient.VolumeInspectOptions{}) require.NoError(t, err) - assert.Equal(t, job, vol.Volume.Labels[jobLabel]) + assert.Contains(t, vol.Volume.Labels, jobLabel+job) exec, err := viaProxy.ExecCreate(ctx, created.ID, mobyclient.ExecCreateOptions{Cmd: []string{"cat"}, AttachStdin: true, AttachStdout: true, TTY: true}) require.NoError(t, err) diff --git a/act/runner/testdata/docker-proxy/action/index.js b/act/runner/testdata/docker-proxy/action/index.js index 7442c949..175517c2 100644 --- a/act/runner/testdata/docker-proxy/action/index.js +++ b/act/runner/testdata/docker-proxy/action/index.js @@ -37,10 +37,10 @@ async function main() { await request('POST', `${api}/networks/${name}/connect`, {Container: job}); fs.appendFileSync(process.env.GITHUB_STATE, 'post=true\n'); } - const label = process.env.PROXY_TEST_MODE === 'proxy' ? job : undefined; - assert.equal(JSON.parse(await request('GET', `${api}/volumes/${name}`)).Labels?.['com.gitea.runner.job'], label); + const label = process.env.PROXY_TEST_MODE === 'proxy' ? '' : undefined; + assert.equal(JSON.parse(await request('GET', `${api}/volumes/${name}`)).Labels?.[`com.gitea.runner.job.${job}`], label); const network = JSON.parse(await request('GET', `${api}/networks/${name}`)); - assert.equal(network.Labels?.['com.gitea.runner.job'], label); + assert.equal(network.Labels?.[`com.gitea.runner.job.${job}`], label); assert(Object.values(network.Containers).some((container) => container.Name === job)); if (post) { console.log('docker proxy post verified'); diff --git a/act/runner/testdata/docker-proxy/push.yml b/act/runner/testdata/docker-proxy/push.yml index 049dc4d8..e8ad81c0 100644 --- a/act/runner/testdata/docker-proxy/push.yml +++ b/act/runner/testdata/docker-proxy/push.yml @@ -52,5 +52,5 @@ jobs: - run: | docker run --rm --user 1001 --group-add 2375 -v /var/run/docker.sock:/var/run/docker.sock -v /usr/local/bin/docker:/usr/local/bin/docker:ro -v "${GITEA_DOCKER_WORKSPACE:?}/data:/data:ro" \ "$PROXY_TEST_IMAGE" sh -c 'grep -qx from-job /data/marker && docker volume create "$0"' "$PROXY_TEST_RESOURCE-nested" - test "$(docker volume inspect -f '{{ index .Labels "com.gitea.runner.job" }}' "$PROXY_TEST_RESOURCE-nested")" = "$JOB_CONTAINER_NAME" + test "$(docker volume ls -q -f "label=com.gitea.runner.job.$JOB_CONTAINER_NAME" -f "name=$PROXY_TEST_RESOURCE-nested")" = "$PROXY_TEST_RESOURCE-nested" echo "docker binds verified"