fix: docker proxy probe for podman and containerized runners (#1231)

Podman creates a missing bind source instead of rejecting it, so the Docker proxy probe passed even when the daemon could not see the runner's files. Jobs of Podman runners in a container got an empty directory at `/var/run/docker.sock` (https://gitea.com/gitea/runner/issues/1193#issuecomment-1700501).

The probe now binds the directory and stats its marker through the created container. A runner in a container stats the marker through its own container, so it no longer creates probe containers, which cost up to 2s per job. `make test-dind TARGET=podman` runs the probe against Podman in CI.

Reviewed-on: https://gitea.com/gitea/runner/pulls/1231
Reviewed-by: bircni <bircni@icloud.com>
Co-authored-by: silverwind <me@silverwind.io>

.gitea/workflows/test.yml

@@ -53,6 +53,8 @@ jobs:
       # after `make test` so the images it needs are already present on the host daemon.
       - name: test against dind image
         run: make test-dind
+      - name: test against podman
+        run: make test-dind TARGET=podman
       - name: coverage report
         run: |
           make coverage-report

Makefile

@@ -8,6 +8,11 @@ XGO_PACKAGE ?= src.techknowlogick.com/xgo@v1.9.0 # renovate: datasource=go
 XGO_VERSION := go-1.27.x
 GXZ_PACKAGE ?= github.com/ulikunitz/xz/cmd/gxz@v0.5.16 # renovate: datasource=go
 
+PODMAN_TEST_IMAGE ?= quay.io/podman/stable:v5.8.4@sha256:d64648d15311e3997df7c747da47cd359f6175655e8b236ca09b32624935c568 # renovate: datasource=docker
+E2E_JOB_IMAGE ?= node:24-bookworm@sha256:6dac556d980b7f0e5498d08f08cee0ca67798b4ad6c23964a9214920e67758d0 # renovate: datasource=docker
+SERVICE_IMAGE ?= nginx:1.31.4-alpine@sha256:db35bfc6b2951e7f8a72db5db120288c127ffaeeb4a6d4b95a26fead017d5913 # renovate: datasource=docker
+E2E_GITEA_IMAGE ?= gitea/gitea:main-nightly
+
 LINUX_ARCHS ?= linux/amd64,linux/arm64
 DARWIN_ARCHS ?= darwin-12/amd64,darwin-12/arm64
 WINDOWS_ARCHS ?= windows/amd64
@@ -158,12 +163,9 @@ coverage-report: ## turn coverage.txt from `make test` into .tmp/coverage.md
 	@echo "Wrote .tmp/coverage.md"
 
 .PHONY: test-dind
-test-dind: ## run the daemon-facing tests against the built dind image (TARGET=dind|dind-rootless)
-	@./scripts/test-dind.sh $(TARGET)
+test-dind: ## run the daemon-facing tests against the built dind image or podman (TARGET=dind|dind-rootless|podman)
+	@PODMAN_TEST_IMAGE=$(PODMAN_TEST_IMAGE) ./scripts/test-dind.sh $(TARGET)
 
-E2E_JOB_IMAGE ?= node:24-bookworm@sha256:6dac556d980b7f0e5498d08f08cee0ca67798b4ad6c23964a9214920e67758d0 # renovate: datasource=docker
-SERVICE_IMAGE ?= nginx:1.31.4-alpine@sha256:db35bfc6b2951e7f8a72db5db120288c127ffaeeb4a6d4b95a26fead017d5913 # renovate: datasource=docker
-E2E_GITEA_IMAGE ?= gitea/gitea:main-nightly
 E2E_CONCURRENCY ?= 8
 
 .PHONY: test-e2e

act/container/container_types.go

@@ -9,6 +9,8 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"path"
+	"strings"
 	"sync"
 	"sync/atomic"
 
@@ -107,6 +109,16 @@ type Info struct {
 	Mounts       map[string]string // container path to its source on the daemon
 }
 
+// DaemonPath maps target to its daemon path through the nearest mount with a source, empty without one.
+func (info *Info) DaemonPath(target string) string {
+	for dir := target; dir != "/" && dir != "."; dir = path.Dir(dir) {
+		if source := info.Mounts[dir]; source != "" {
+			return path.Join(source, strings.TrimPrefix(target, dir))
+		}
+	}
+	return ""
+}
+
 // Container for managing docker run containers
 type Container interface {
 	Create(capAdd, capDrop []string) common.Executor

act/container/docker_proxy.go

@@ -66,15 +66,11 @@ func NewDockerProxy(ctx context.Context, job string) *DockerProxy {
 	if info, err := os.Stat(daemonSocket); err != nil || info.Mode()&os.ModeSocket == 0 {
 		return nil
 	}
-	dir, err := filepath.Abs(os.TempDir())
-	if err != nil {
-		common.Logger(ctx).Infof("docker proxy probe failed, jobs get the daemon socket directly: %v", err)
-		return nil
-	}
-	daemonDir := dir
-	seen, err := daemonSeesDir(probeCtx, cli, dir, daemonDir)
-	if err == nil && !seen {
-		if dir, daemonDir = runnerContainerWorkdir(probeCtx, cli); daemonDir != "" {
+	dir, daemonDir := runnerContainerWorkdir(probeCtx, cli)
+	seen := daemonDir != ""
+	if !seen {
+		if dir, err = filepath.Abs(os.TempDir()); err == nil {
+			daemonDir = dir
 			seen, err = daemonSeesDir(probeCtx, cli, dir, daemonDir)
 		}
 	}
@@ -109,18 +105,16 @@ func runnerContainerWorkdir(ctx context.Context, cli client.APIClient) (workdir,
 	if err != nil {
 		return "", ""
 	}
-	destination := ""
-	for _, point := range self.Container.Mounts {
-		if rel, err := filepath.Rel(point.Destination, workdir); err == nil && filepath.IsLocal(rel) && len(point.Destination) > len(destination) {
-			destination, daemonDir = point.Destination, filepath.Join(point.Source, rel)
-		}
+	if daemonDir = containerInfoFromInspect(self.Container).DaemonPath(workdir); daemonDir == "" {
+		return "", ""
+	}
+	if seen, _ := containerSeesMarker(ctx, cli, workdir, self.Container.ID, workdir); !seen { // the hostname may name another container
+		return "", ""
 	}
 	return workdir, daemonDir
 }
 
-// daemonSeesDir reports whether the daemon opens the files the runner writes in dir by their path in daemonDir,
-// which is what a job's proxy socket mounted from there needs.
-func daemonSeesDir(ctx context.Context, cli client.APIClient, dir, daemonDir string) (bool, error) {
+func containerSeesMarker(ctx context.Context, cli client.APIClient, dir, id, containerDir string) (bool, error) {
 	marker, err := os.CreateTemp(dir, "gitea-runner-probe-")
 	if err != nil {
 		return false, err
@@ -133,6 +127,16 @@ func daemonSeesDir(ctx context.Context, cli client.APIClient, dir, daemonDir str
 	if err := marker.Close(); err != nil {
 		return false, err
 	}
+	_, err = cli.ContainerStatPath(ctx, id, client.ContainerStatPathOptions{Path: path.Join(containerDir, filepath.Base(marker.Name()))})
+	if cerrdefs.IsNotFound(err) {
+		return false, nil
+	}
+	return err == nil, err
+}
+
+// daemonSeesDir reports whether the daemon opens the files the runner writes in dir by their path in daemonDir,
+// which is what a job's proxy socket mounted from there needs.
+func daemonSeesDir(ctx context.Context, cli client.APIClient, dir, daemonDir string) (bool, error) {
 	images, err := cli.ImageList(ctx, client.ImageListOptions{})
 	if err != nil {
 		return false, err
@@ -143,7 +147,7 @@ func daemonSeesDir(ctx context.Context, cli client.APIClient, dir, daemonDir str
 	created, err := cli.ContainerCreate(ctx, client.ContainerCreateOptions{
 		Config: &container.Config{Image: images.Items[0].ID, Cmd: []string{"true"}},
 		HostConfig: &container.HostConfig{Mounts: []mount.Mount{
-			{Type: mount.TypeBind, Source: filepath.Join(daemonDir, filepath.Base(marker.Name())), Target: "/gitea-runner-probe", ReadOnly: true},
+			{Type: mount.TypeBind, Source: daemonDir, Target: "/gitea-runner-probe", ReadOnly: true}, // not the marker itself, podman creates a missing bind source where docker rejects it
 		}},
 	})
 	if cerrdefs.IsInvalidArgument(err) {
@@ -152,12 +156,13 @@ func daemonSeesDir(ctx context.Context, cli client.APIClient, dir, daemonDir str
 	if err != nil {
 		return false, err
 	}
+	seen, err := containerSeesMarker(ctx, cli, dir, created.ID, "/gitea-runner-probe")
 	cleanupCtx, cancel := context.WithTimeout(context.WithoutCancel(ctx), dockerProxyProbeTimeout)
 	defer cancel()
-	if _, err := cli.ContainerRemove(cleanupCtx, created.ID, client.ContainerRemoveOptions{Force: true, RemoveVolumes: true}); err != nil {
-		return false, fmt.Errorf("removing the docker proxy probe container failed: %w", err)
+	if _, removeErr := cli.ContainerRemove(cleanupCtx, created.ID, client.ContainerRemoveOptions{Force: true, RemoveVolumes: true}); removeErr != nil {
+		return false, fmt.Errorf("removing the docker proxy probe container failed: %w", removeErr)
 	}
-	return true, nil
+	return seen, err
 }
 
 // StartDockerProxy serves a job's docker socket in dir, labelling what the job creates through it.

act/container/docker_proxy_test.go

@@ -321,6 +321,9 @@ func TestDockerProxyWithDaemon(t *testing.T) {
 	seen, err := daemonSeesDir(ctx, direct, dir, dir)
 	require.NoError(t, err)
 	t.Logf("daemon sees the runner's filesystem: %v", seen)
+	seen, err = daemonSeesDir(ctx, direct, dir, shortTempDir(t))
+	require.NoError(t, err)
+	assert.False(t, seen)
 
 	job := "proxy-test-" + strconv.FormatInt(time.Now().UnixNano(), 36)
 	proxy, err := StartDockerProxy(daemonSocketPath(t, direct), dir, job)
@@ -425,6 +428,12 @@ type probeClient struct {
 	mobyclient.APIClient
 	create func(mobyclient.ContainerCreateOptions) (mobyclient.ContainerCreateResult, error)
 	remove func(context.Context, string, mobyclient.ContainerRemoveOptions) (mobyclient.ContainerRemoveResult, error)
+	stat   func(string) error
+	mounts []container.MountPoint
+}
+
+func (c *probeClient) ContainerInspect(context.Context, string, mobyclient.ContainerInspectOptions) (mobyclient.ContainerInspectResult, error) {
+	return mobyclient.ContainerInspectResult{Container: container.InspectResponse{Mounts: c.mounts}}, nil
 }
 
 func (c *probeClient) ImageList(context.Context, mobyclient.ImageListOptions) (mobyclient.ImageListResult, error) {
@@ -439,43 +448,47 @@ func (c *probeClient) ContainerRemove(ctx context.Context, id string, opts mobyc
 	return c.remove(ctx, id, opts)
 }
 
+func (c *probeClient) ContainerStatPath(_ context.Context, _ string, opts mobyclient.ContainerStatPathOptions) (mobyclient.ContainerStatPathResult, error) {
+	return mobyclient.ContainerStatPathResult{}, c.stat(opts.Path)
+}
+
 func TestDaemonSeesDir(t *testing.T) {
 	dir := t.TempDir()
 	markers := make(map[string]bool)
 	for _, testCase := range []struct {
 		name      string
-		private   bool
+		daemonDir string
 		removeErr error
+		seen      bool
+		removed   bool
 	}{
-		{name: "cleanup after cancellation"},
-		{name: "private filesystem with stale marker", private: true},
-		{name: "cleanup error after cancellation", removeErr: errors.New("cleanup failed")},
+		{name: "cleanup after cancellation", daemonDir: dir, seen: true, removed: true},
+		{name: "daemon without the directory", daemonDir: filepath.Join(dir, "missing")},
+		{name: "daemon with a different directory at that path", daemonDir: t.TempDir(), removed: true},
+		{name: "cleanup error after cancellation", daemonDir: dir, removeErr: errors.New("cleanup failed"), removed: true},
 	} {
 		t.Run(testCase.name, func(t *testing.T) {
 			ctx, cancel := context.WithCancel(t.Context())
 			defer cancel()
-			daemonDir := dir
-			if testCase.private {
-				daemonDir = t.TempDir()
-				require.NoError(t, os.WriteFile(filepath.Join(daemonDir, "probe"), []byte("stale"), 0o600))
-			}
 			removed := false
 			cli := &probeClient{
 				create: func(opts mobyclient.ContainerCreateOptions) (mobyclient.ContainerCreateResult, error) {
-					require.Len(t, opts.HostConfig.Mounts, 1)
-					marker := opts.HostConfig.Mounts[0].Source
-					assert.Equal(t, mount.Mount{Type: mount.TypeBind, Source: marker, Target: "/gitea-runner-probe", ReadOnly: true}, opts.HostConfig.Mounts[0])
-					assert.False(t, markers[marker])
-					markers[marker] = true
-					info, err := os.Stat(marker)
-					require.NoError(t, err)
-					assert.True(t, info.Mode().IsRegular())
+					assert.Equal(t, []mount.Mount{{Type: mount.TypeBind, Source: testCase.daemonDir, Target: "/gitea-runner-probe", ReadOnly: true}}, opts.HostConfig.Mounts)
 					cancel()
-					if _, err := os.Stat(filepath.Join(daemonDir, filepath.Base(marker))); errors.Is(err, os.ErrNotExist) {
+					if _, err := os.Stat(testCase.daemonDir); err != nil {
 						return mobyclient.ContainerCreateResult{}, cerrdefs.ErrInvalidArgument
 					}
 					return mobyclient.ContainerCreateResult{ID: "probe"}, nil
 				},
+				stat: func(path string) error {
+					marker := strings.TrimPrefix(path, "/gitea-runner-probe/")
+					assert.False(t, markers[marker])
+					markers[marker] = true
+					if _, err := os.Stat(filepath.Join(testCase.daemonDir, marker)); err != nil {
+						return cerrdefs.ErrNotFound
+					}
+					return nil
+				},
 				remove: func(ctx context.Context, id string, opts mobyclient.ContainerRemoveOptions) (mobyclient.ContainerRemoveResult, error) {
 					removed = true
 					assert.Equal(t, "probe", id)
@@ -486,13 +499,45 @@ func TestDaemonSeesDir(t *testing.T) {
 					return mobyclient.ContainerRemoveResult{}, testCase.removeErr
 				},
 			}
-			seen, err := daemonSeesDir(ctx, cli, dir, dir)
+			seen, err := daemonSeesDir(ctx, cli, dir, testCase.daemonDir)
 			require.ErrorIs(t, err, testCase.removeErr)
-			assert.Equal(t, !testCase.private && testCase.removeErr == nil, seen)
-			assert.Equal(t, !testCase.private, removed)
+			assert.Equal(t, testCase.seen, seen)
+			assert.Equal(t, testCase.removed, removed)
 			entries, err := os.ReadDir(dir)
 			require.NoError(t, err)
 			assert.Empty(t, entries)
 		})
 	}
 }
+
+func TestRunnerContainerWorkdir(t *testing.T) {
+	workdir := t.TempDir()
+	t.Chdir(workdir)
+	volume := container.MountPoint{Type: mount.TypeVolume, Source: "/daemon/_data", Destination: filepath.Dir(workdir)}
+	for _, testCase := range []struct {
+		name      string
+		mount     container.MountPoint
+		foreign   bool
+		daemonDir string
+	}{
+		{name: "volume above the working directory", mount: volume, daemonDir: filepath.Join(volume.Source, filepath.Base(workdir))},
+		{name: "tmpfs", mount: container.MountPoint{Type: mount.TypeTmpfs, Destination: volume.Destination}},
+		{name: "hostname of another container", mount: volume, foreign: true},
+	} {
+		cli := &probeClient{
+			mounts: []container.MountPoint{testCase.mount},
+			stat: func(path string) error {
+				if _, err := os.Stat(path); err != nil || testCase.foreign {
+					return cerrdefs.ErrNotFound
+				}
+				return nil
+			},
+		}
+		dir, daemonDir := runnerContainerWorkdir(t.Context(), cli)
+		assert.Equal(t, testCase.daemonDir, daemonDir, testCase.name)
+		assert.Equal(t, testCase.daemonDir != "", dir == workdir, testCase.name)
+	}
+	entries, err := os.ReadDir(workdir)
+	require.NoError(t, err)
+	assert.Empty(t, entries)
+}

act/runner/run_context.go

@@ -18,7 +18,6 @@ import (
 	maps0 "maps"
 	"net"
 	"os"
-	"path"
 	"path/filepath"
 	"regexp"
 	"runtime"
@@ -995,12 +994,8 @@ func (rc *RunContext) captureJobContainerInfo() common.Executor {
 		if rc.dockerProxy != nil {
 			rc.dockerProxy.SetMounts(info.Mounts)
 		}
-		workspace := rc.githubWorkspace()
-		for dir := workspace; dir != "/" && dir != "."; dir = path.Dir(dir) {
-			if source := info.Mounts[dir]; source != "" {
-				rc.Env["GITEA_DOCKER_WORKSPACE"] = path.Join(source, strings.TrimPrefix(workspace, dir))
-				break
-			}
+		if source := info.DaemonPath(rc.githubWorkspace()); source != "" {
+			rc.Env["GITEA_DOCKER_WORKSPACE"] = source
 		}
 		return nil
 	}

scripts/test-dind.sh

@@ -8,9 +8,10 @@
 # It is deliberately generic: point it at any package/test to exercise the dind daemon.
 #
 # Usage: scripts/test-dind.sh [target] [-- go-test-args...]
-#   target:        dind (default) or dind-rootless
+#   target:        dind (default), dind-rootless, or podman to run PODMAN_TEST_IMAGE's API service instead
 #   go-test-args:  passed verbatim to `go test`. Defaults cover image env extraction,
-#                  symlink copying and a mounted Docker job using cached images.
+#                  symlink copying and a mounted Docker job using cached images, or the
+#                  Docker proxy probe for podman.
 #
 # Env:
 #   DIND_TEST_PORT     host port for the daemon (default 32375)
@@ -20,18 +21,13 @@ set -euo pipefail
 
 target="dind"
 case "${1:-}" in
-  dind|dind-rootless) target="$1"; shift ;;
+  dind|dind-rootless|podman) target="$1"; shift ;;
 esac
 [ "${1:-}" = "--" ] && shift
 default_tests=false
-if [ $# -eq 0 ]; then
-  default_tests=true
-  set -- -count=1 -race -run '^TestDocker$' ./act/container/
-fi
 
 port="${DIND_TEST_PORT:-32375}"
 name="gitea-runner-dind-test-$$"
-image="${DIND_TEST_IMAGE:-gitea-runner-${target}:dind-test}"
 # The host daemon endpoint, captured before DOCKER_HOST is pointed at the fresh dind daemon.
 host_docker="${DOCKER_HOST:-$(docker context inspect --format '{{.Endpoints.docker.Host}}')}"
 test_dir=""
@@ -44,14 +40,25 @@ cleanup() {
 }
 trap cleanup EXIT
 
-if [ -z "${DIND_TEST_IMAGE:-}" ]; then
-  echo "==> Building ${target} image"
-  docker build --target "$target" -t "$image" .
+if [ "$target" = podman ]; then
+  image="${PODMAN_TEST_IMAGE:?}"
+  daemon_args=("$image" podman system service --time=0 tcp://0.0.0.0:2375)
+  [ $# -gt 0 ] || set -- -count=1 -race -run '^TestDockerProxyWithDaemon$' ./act/container/
+else
+  image="${DIND_TEST_IMAGE:-gitea-runner-${target}:dind-test}"
+  if [ -z "${DIND_TEST_IMAGE:-}" ]; then
+    echo "==> Building ${target} image"
+    docker build --target "$target" -t "$image" .
+  fi
+  # Override the image entrypoint (s6) and run only dockerd, exposed over insecure TCP.
+  # We are testing the daemon the image ships, not the runner supervision tree.
+  daemon_args=(-e DOCKER_TLS_CERTDIR= --entrypoint dockerd-entrypoint.sh "$image" --host=tcp://0.0.0.0:2375)
+  if [ $# -eq 0 ]; then
+    default_tests=true
+    set -- -count=1 -race -run '^TestDocker$' ./act/container/
+  fi
 fi
 
-# Override the image entrypoint (s6) and run only dockerd, exposed over insecure TCP.
-# We are testing the daemon the image ships, not the runner supervision tree.
-#
 # How the test process reaches the daemon depends on where it runs:
 #   - plain host: publish 2375 on loopback and connect to 127.0.0.1.
 #   - inside a container (CI), the daemon is a sibling container, so its published port is on
@@ -66,8 +73,8 @@ if [ -n "$self_container" ]; then
 fi
 
 # The two cases differ only in how the daemon is exposed and addressed; everything else
-# (privileged, name, TLS-off entrypoint, image, --host) is shared, so collect just the
-# differing run args and the resulting DOCKER_HOST here.
+# (privileged, name, daemon_args) is shared, so collect just the differing run args and the
+# resulting DOCKER_HOST here.
 if [ -n "$self_network" ]; then
   echo "==> Starting ${target} daemon on network ${self_network} (reached as ${name}:2375)"
   run_args=(--network "$self_network")
@@ -79,10 +86,7 @@ else
 fi
 # Create the dind container on the host daemon first, then repoint DOCKER_HOST at it: exporting
 # DOCKER_HOST before `docker run` would make this `docker run` target the not-yet-existent dind.
-docker -H "$host_docker" run -d --privileged --name "$name" "${run_args[@]}" \
-  -e DOCKER_TLS_CERTDIR= \
-  --entrypoint dockerd-entrypoint.sh \
-  "$image" --host=tcp://0.0.0.0:2375 >/dev/null
+docker -H "$host_docker" run -d --privileged --name "$name" "${run_args[@]}" "${daemon_args[@]}" >/dev/null
 export DOCKER_HOST="$daemon_host"
 
 echo "==> Waiting for daemon"
@@ -109,7 +113,7 @@ for img in $preload; do
   fi
 done
 
-echo "==> Running tests against dind daemon"
+echo "==> Running tests against ${target} daemon"
 go test "$@"
 
 if [ "$default_tests" = true ]; then