diff --git a/README.md b/README.md index 13514cfe..aaf48365 100644 --- a/README.md +++ b/README.md @@ -283,11 +283,12 @@ Besides `GITEA_INSTANCE_URL` and `GITEA_RUNNER_REGISTRATION_TOKEN`, the image en For a fuller container-oriented walkthrough, see [examples/docker](examples/docker/README.md). -When `container.bind_workdir` is enabled, stale task workspace directories can be cleaned while the runner is idle: -- directories older than `runner.workdir_cleanup_age` are removed (default: `24h`; set `0` to disable) -- cleanup runs every `runner.idle_cleanup_interval` (default: `10m`; set `0` to disable) +While the runner is idle it cleans up after earlier jobs: +- when `container.bind_workdir` is enabled, stale task workspace directories older than `runner.workdir_cleanup_age` are removed (default: `24h`; set `0` to disable) - only purely numeric subdirectories under `container.workdir_parent` are treated as task workspaces and may be removed - cleanup assumes `container.workdir_parent` is not shared across multiple runners +- on runners that use docker, per-job networks left behind by jobs the runner did not live to tear down are removed, identified by the `com.gitea.runner.uuid` label carrying this runner's uuid +- cleanup runs every `runner.idle_cleanup_interval` (default: `10m`; set `0` to disable), and setting either knob to `0` disables all of the above #### Post-task script (`runner.post_task_script`) diff --git a/act/container/container_types.go b/act/container/container_types.go index 61679807..4e17b516 100644 --- a/act/container/container_types.go +++ b/act/container/container_types.go @@ -89,6 +89,7 @@ type NewDockerBuildExecutorInput struct { type NewDockerNetworkCreateExecutorInput struct { EnableIPv4 *bool EnableIPv6 *bool + RunnerUUID string } // NewDockerPullExecutorInput the input for the NewDockerPullExecutor function diff --git a/act/container/docker_logger.go b/act/container/docker_logger.go index b2d7bc5c..96d59432 100644 --- a/act/container/docker_logger.go +++ b/act/container/docker_logger.go @@ -57,7 +57,7 @@ func logDockerResponse(logger logrus.FieldLogger, dockerResponse io.ReadCloser, if msg.ErrorDetail.Message != "" { writeLog(logger, isError, "%s", msg.ErrorDetail.Message) - return errors.New(msg.Error) + return errors.New(msg.ErrorDetail.Message) } if msg.Status != "" { diff --git a/act/container/docker_network.go b/act/container/docker_network.go index dd9b2960..406d1907 100644 --- a/act/container/docker_network.go +++ b/act/container/docker_network.go @@ -8,12 +8,69 @@ package container import ( "context" + "errors" + "fmt" + "strings" + "time" "gitea.com/gitea/runner/act/common" "github.com/moby/moby/client" ) +const ( + networkCreateAttempts = 3 + networkCreateRetryDelay = time.Second + + // marks the networks a runner creates for its jobs, so it can tell its own leftovers from + // those of another runner sharing the daemon + runnerUUIDLabel = "com.gitea.runner.uuid" +) + +// RemoveOrphanNetworks removes the networks this runner created for jobs whose teardown did +// not get to them: the runner died with the job, the teardown timed out, or the network still +// had an endpoint on it at the time. Each one holds a subnet of the daemon's address pool +// until it is removed. Networks created after createdBefore are left alone, so a job starting +// while this runs cannot lose the network it has created but not yet attached a container to. +func RemoveOrphanNetworks(ctx context.Context, runnerUUID string, createdBefore time.Time) error { + cli, err := GetDockerClient(ctx) + if err != nil { + return fmt.Errorf("failed to connect to the docker daemon: %w", err) + } + defer cli.Close() + + return removeOrphanNetworks(ctx, cli, runnerUUID, createdBefore) +} + +func removeOrphanNetworks(ctx context.Context, cli client.APIClient, runnerUUID string, createdBefore time.Time) error { + networks, err := cli.NetworkList(ctx, client.NetworkListOptions{ + Filters: make(client.Filters).Add("label", runnerUUIDLabel+"="+runnerUUID), + }) + if err != nil { + return err + } + + var errs []error + for _, n := range networks.Items { + result, err := cli.NetworkInspect(ctx, n.ID, client.NetworkInspectOptions{}) + if err != nil { + errs = append(errs, fmt.Errorf("failed to inspect network %s: %w", n.Name, err)) + continue + } + // the emptiness check, not the label, is what keeps a live job of another process + // sharing this registration safe + if len(result.Network.Containers) != 0 || result.Network.Created.After(createdBefore) { + continue + } + if _, err := cli.NetworkRemove(ctx, n.ID, client.NetworkRemoveOptions{}); err != nil { + errs = append(errs, fmt.Errorf("failed to remove network %s: %w", n.Name, err)) + continue + } + common.Logger(ctx).Infof("removed docker network %s left behind by an earlier job", n.Name) + } + return errors.Join(errs...) +} + func NewDockerNetworkCreateExecutor(name string, opts NewDockerNetworkCreateExecutorInput) common.Executor { return func(ctx context.Context) error { cli, err := GetDockerClient(ctx) @@ -36,18 +93,45 @@ func NewDockerNetworkCreateExecutor(name string, opts NewDockerNetworkCreateExec } } - _, err = cli.NetworkCreate(ctx, name, client.NetworkCreateOptions{ - Driver: "bridge", - Scope: "local", - EnableIPv4: opts.EnableIPv4, - EnableIPv6: opts.EnableIPv6, - }) - if err != nil { - return err + for i := range networkCreateAttempts { + if i > 0 { + common.Logger(ctx).Infof("Waiting for a free docker address pool to create network %s", name) + select { + case <-ctx.Done(): + return ctx.Err() + case <-time.After(time.Duration(i) * networkCreateRetryDelay): + } + } + if _, err = cli.NetworkCreate(ctx, name, client.NetworkCreateOptions{ + Driver: "bridge", + Scope: "local", + EnableIPv4: opts.EnableIPv4, + EnableIPv6: opts.EnableIPv6, + Labels: runnerLabels(opts.RunnerUUID), + }); err == nil { + return nil + } + if !isAddressPoolExhausted(err) { + return err + } } + return fmt.Errorf("docker has no address pool left for this job's network, lower runner.capacity or widen default-address-pools in the docker daemon config: %w", err) + } +} +func runnerLabels(runnerUUID string) map[string]string { + if runnerUUID == "" { return nil } + return map[string]string{runnerUUIDLabel: runnerUUID} +} + +// The daemon reports this as a plain invalid-parameter error, the same kind it uses for every +// malformed request, so the message is the only discriminator. +func isAddressPoolExhausted(err error) bool { + msg := err.Error() + return strings.Contains(msg, "all predefined address pools have been fully subnetted") || + strings.Contains(msg, "could not find an available, non-overlapping IPv4 address pool among the defaults") // docker 24 and older } func NewDockerNetworkRemoveExecutor(name string) common.Executor { @@ -66,6 +150,7 @@ func NewDockerNetworkRemoveExecutor(name string) common.Executor { } // For Gitea, reduce log noise // common.Logger(ctx).Debugf("%v", networks) + var errs []error for _, n := range networks.Items { if n.Name == name { result, err := cli.NetworkInspect(ctx, n.ID, client.NetworkInspectOptions{}) @@ -73,16 +158,17 @@ func NewDockerNetworkRemoveExecutor(name string) common.Executor { return err } - if len(result.Network.Containers) == 0 { - if _, err = cli.NetworkRemove(ctx, n.ID, client.NetworkRemoveOptions{}); err != nil { - common.Logger(ctx).Debugf("%v", err) - } - } else { - common.Logger(ctx).Debugf("Refusing to remove network %v because it still has active endpoints", name) + // it holds a subnet out of the daemon's pool until something reclaims it + if len(result.Network.Containers) != 0 { + common.Logger(ctx).Warnf("Refusing to remove network %s because it still has active endpoints, the idle cleanup reclaims it once they are gone", name) + continue + } + if _, err = cli.NetworkRemove(ctx, n.ID, client.NetworkRemoveOptions{}); err != nil { + errs = append(errs, fmt.Errorf("failed to remove network %s: %w", name, err)) } } } - return err + return errors.Join(errs...) } } diff --git a/act/container/docker_network_test.go b/act/container/docker_network_test.go new file mode 100644 index 00000000..5a92dd8c --- /dev/null +++ b/act/container/docker_network_test.go @@ -0,0 +1,50 @@ +// Copyright 2026 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package container + +import ( + "context" + "errors" + "testing" + "time" + + cerrdefs "github.com/containerd/errdefs" + "github.com/moby/moby/api/types/network" + mobyclient "github.com/moby/moby/client" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestIsAddressPoolExhausted(t *testing.T) { + assert.True(t, isAddressPoolExhausted(cerrdefs.ErrInvalidArgument.WithMessage("Error response from daemon: all predefined address pools have been fully subnetted"))) + assert.True(t, isAddressPoolExhausted(errors.New("could not find an available, non-overlapping IPv4 address pool among the defaults to assign to the network"))) + assert.False(t, isAddressPoolExhausted(cerrdefs.ErrInvalidArgument.WithMessage("invalid subnet 10.0.0.0/8: it overlaps with an existing network"))) +} + +// Of this runner's networks, only the ones nothing is attached to and old enough to predate +// any job now starting are the runner's to reclaim. An unexpected NetworkRemove fails the +// test on its own, since testify has no expectation to match it against. +func TestRemoveOrphanNetworks(t *testing.T) { + ctx := context.Background() + cutoff := time.Date(2026, time.April, 29, 20, 0, 0, 0, time.UTC) + client := &mockDockerClient{} + client.On("NetworkList", ctx, mobyclient.NetworkListOptions{ + Filters: make(mobyclient.Filters).Add("label", runnerUUIDLabel+"=runner-1"), + }).Return(mobyclient.NetworkListResult{Items: []network.Summary{ + {Network: network.Network{ID: "orphan"}}, + {Network: network.Network{ID: "busy"}}, + {Network: network.Network{ID: "starting"}}, + }}, nil) + client.On("NetworkInspect", ctx, "orphan", mobyclient.NetworkInspectOptions{}). + Return(mobyclient.NetworkInspectResult{}, nil) + client.On("NetworkInspect", ctx, "busy", mobyclient.NetworkInspectOptions{}). + Return(mobyclient.NetworkInspectResult{Network: network.Inspect{Containers: map[string]network.EndpointResource{"c": {}}}}, nil) + client.On("NetworkInspect", ctx, "starting", mobyclient.NetworkInspectOptions{}). + Return(mobyclient.NetworkInspectResult{Network: network.Inspect{Network: network.Network{Created: cutoff.Add(time.Second)}}}, nil) + client.On("NetworkRemove", ctx, "orphan", mobyclient.NetworkRemoveOptions{}). + Return(mobyclient.NetworkRemoveResult{}, nil) + + require.NoError(t, removeOrphanNetworks(ctx, client, "runner-1", cutoff)) + client.AssertExpectations(t) +} diff --git a/act/container/docker_pull.go b/act/container/docker_pull.go index 6bb3f6c4..3204dcc1 100644 --- a/act/container/docker_pull.go +++ b/act/container/docker_pull.go @@ -30,23 +30,19 @@ func NewDockerPullExecutor(input NewDockerPullExecutorInput) common.Executor { return nil } - pull := input.ForcePull - if !pull { + // skip the pull when the image is already here: either none was forced, or a digest + // pins the content so a forced pull could only fetch the same bytes again + if !input.ForcePull || isPinnedImage(input.Image) { imageExists, err := ImageExistsLocally(ctx, input.Image, input.Platform) logger.Debugf("Image exists? %v", imageExists) if err != nil { return fmt.Errorf("unable to determine if image already exists for image '%s' (%s): %w", input.Image, input.Platform, err) } - - if !imageExists { - pull = true + if imageExists { + return nil } } - if !pull { - return nil - } - imageRef := cleanImage(ctx, input.Image) logger.Debugf("pulling image '%v' (%s)", imageRef, input.Platform) @@ -61,22 +57,32 @@ func NewDockerPullExecutor(input NewDockerPullExecutorInput) common.Executor { return err } - reader, err := cli.ImagePull(ctx, imageRef, imagePullOptions) - - _ = logDockerResponse(logger, reader, err != nil) - if err != nil { - if imagePullOptions.RegistryAuth != "" && strings.Contains(err.Error(), "unauthorized") { - logger.Errorf("pulling image '%v' (%s) failed with credentials %s retrying without them, please check for stale docker config files", imageRef, input.Platform, err.Error()) - imagePullOptions.RegistryAuth = "" - reader, err = cli.ImagePull(ctx, imageRef, imagePullOptions) - - _ = logDockerResponse(logger, reader, err != nil) - } + // the daemon reports a failure that happens after the first progress line in the + // stream rather than on the call itself, so both have to be checked + pullOnce := func(opts client.ImagePullOptions) error { + reader, err := cli.ImagePull(ctx, imageRef, opts) + streamErr := logDockerResponse(logger, reader, err != nil) if err != nil { - return fmt.Errorf("failed to pull image '%s' (%s): %w", imageRef, input.Platform, err) + return err } + return streamErr } - return nil + + err = pullOnce(imagePullOptions) + if err != nil && imagePullOptions.RegistryAuth != "" && strings.Contains(err.Error(), "unauthorized") { + logger.Errorf("pulling image '%v' (%s) failed with credentials %s retrying without them, please check for stale docker config files", imageRef, input.Platform, err.Error()) + imagePullOptions.RegistryAuth = "" + err = pullOnce(imagePullOptions) + } + if err == nil { + return nil + } + // a registry that is down should not fail a job whose image is already here + if exists, existsErr := ImageExistsLocally(ctx, input.Image, input.Platform); existsErr == nil && exists { + logger.Warnf("could not update image '%s' (%s), continuing with the local copy: %v", imageRef, input.Platform, err) + return nil + } + return fmt.Errorf("failed to pull image '%s' (%s): %w", imageRef, input.Platform, err) } } @@ -122,6 +128,15 @@ func getImagePullOptions(ctx context.Context, input NewDockerPullExecutorInput) return imagePullOptions, nil } +func isPinnedImage(image string) bool { + ref, err := reference.ParseAnyReference(image) + if err != nil { + return false + } + _, pinned := ref.(reference.Canonical) + return pinned +} + func cleanImage(ctx context.Context, imageName string) string { ref, err := reference.ParseAnyReference(imageName) if err != nil { diff --git a/act/container/docker_pull_test.go b/act/container/docker_pull_test.go index ae2f171f..8f4a629e 100644 --- a/act/container/docker_pull_test.go +++ b/act/container/docker_pull_test.go @@ -6,11 +6,15 @@ package container import ( "context" + "io" + "strings" "testing" "github.com/docker/cli/cli/config" log "github.com/sirupsen/logrus" + "github.com/sirupsen/logrus/hooks/test" assert "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func init() { @@ -65,3 +69,21 @@ func TestGetImagePullOptions(t *testing.T) { assert.NoError(t, err, "Failed to create ImagePullOptions") //nolint:testifylint // pre-existing issue from nektos/act assert.Equal(t, "eyJ1c2VybmFtZSI6InVzZXJuYW1lIiwicGFzc3dvcmQiOiJwYXNzd29yZFxuIiwic2VydmVyYWRkcmVzcyI6Imh0dHBzOi8vaW5kZXguZG9ja2VyLmlvL3YxLyJ9", options.RegistryAuth, "RegistryAuth should be taken from local docker config") } + +// A digest-pinned image is immutable, so its local copy is always current. +func TestIsPinnedImage(t *testing.T) { + assert.True(t, isPinnedImage("alpine@sha256:28bd5fe8b56d1bd048e5babf5b10710ebe0bae67db86916198a6eec434943f8b")) + assert.False(t, isPinnedImage("alpine:latest")) +} + +// The pull path reports a failure the daemon sent mid-stream, so it must carry the reason +// whichever of the two shapes the daemon used. +func TestLogDockerResponseError(t *testing.T) { + logger, _ := test.NewNullLogger() + streamErr := func(line string) error { + return logDockerResponse(logger, io.NopCloser(strings.NewReader(line)), false) + } + require.EqualError(t, streamErr(`{"error":"toomanyrequests: rate limit exceeded"}`), "toomanyrequests: rate limit exceeded") + require.EqualError(t, streamErr(`{"errorDetail":{"message":"unexpected EOF"}}`), "unexpected EOF") + require.NoError(t, streamErr(`{"status":"Downloading"}`)) +} diff --git a/act/container/docker_run.go b/act/container/docker_run.go index 0e3a1616..c5d4e16d 100644 --- a/act/container/docker_run.go +++ b/act/container/docker_run.go @@ -386,32 +386,71 @@ func (cr *containerReference) find() common.Executor { } } -// isContainerGone reports whether a failed remove still left the container gone (NotFound or Conflict). -func isContainerGone(err error) bool { - return cerrdefs.IsNotFound(err) || cerrdefs.IsConflict(err) -} - func (cr *containerReference) remove() common.Executor { return func(ctx context.Context) error { - if cr.id == "" { + idOrName := cr.id + if idOrName == "" && cr.input != nil { + idOrName = cr.input.Name + } + if idOrName == "" { return nil } logger := common.Logger(ctx) - _, err := cr.cli.ContainerRemove(ctx, cr.id, client.ContainerRemoveOptions{ + // Kill first so removal never waits out a daemon's stop timeout: Docker kills outright + // on a forced remove, Podman sends SIGTERM and waits. Only worth it for a container + // this started, and removal can still deal with one it could not kill. + if cr.id != "" { + _, err := cr.cli.ContainerKill(ctx, cr.id, client.ContainerKillOptions{Signal: "SIGKILL"}) + if err != nil && !cerrdefs.IsConflict(err) && !cerrdefs.IsNotFound(err) { + logger.Debugf("Container %s could not be killed: %v", cr.id, err) + } + } + _, err := cr.cli.ContainerRemove(ctx, idOrName, client.ContainerRemoveOptions{ RemoveVolumes: true, Force: true, }) - if err != nil && !isContainerGone(err) { - logger.Error(fmt.Errorf("failed to remove container: %w", err)) + switch { + case cerrdefs.IsConflict(err): + // the daemon's own AutoRemove teardown is running, and it releases the volume + // references and the network endpoint only once it finishes + cr.waitForRemoval(ctx, idOrName) + case err != nil && !cerrdefs.IsNotFound(err): + logger.Error(fmt.Errorf("failed to remove container %s: %w", idOrName, err)) + return nil // keep the id, the container is still there for a later Remove() } - logger.Debugf("Removed container: %v", cr.id) + logger.Debugf("Removed container: %v", idOrName) cr.id = "" return nil } } +func (cr *containerReference) waitForRemoval(ctx context.Context, idOrName string) { + // per container, against the one minute the post-job executor allows for the whole + // cleanup, so a job with several services can spend most of that budget here + ctx, cancel := context.WithTimeout(ctx, 15*time.Second) + defer cancel() + + waitResult := cr.cli.ContainerWait(ctx, idOrName, client.ContainerWaitOptions{ + Condition: container.WaitConditionRemoved, + }) + select { + case <-waitResult.Result: + case <-waitResult.Error: + case <-ctx.Done(): + // the client delivers the result over an unbuffered channel, so leave a receiver + // behind or its goroutine parks on the send for the lifetime of the process + go func() { + select { + case <-waitResult.Result: + case <-waitResult.Error: + } + }() + common.Logger(ctx).Warnf("Timed out waiting for the daemon to remove container %s, its volumes and network may be left behind", idOrName) + } +} + func (cr *containerReference) mergeContainerConfigs(ctx context.Context, config *container.Config, hostConfig *container.HostConfig) (*container.Config, *container.HostConfig, error) { logger := common.Logger(ctx) input := cr.input diff --git a/act/container/docker_run_test.go b/act/container/docker_run_test.go index 8f362a88..cb353f4e 100644 --- a/act/container/docker_run_test.go +++ b/act/container/docker_run_test.go @@ -122,6 +122,26 @@ func (m *mockDockerClient) ContainerRemove(ctx context.Context, id string, opts return args.Get(0).(mobyclient.ContainerRemoveResult), args.Error(1) } +func (m *mockDockerClient) ContainerKill(ctx context.Context, id string, opts mobyclient.ContainerKillOptions) (mobyclient.ContainerKillResult, error) { + args := m.Called(ctx, id, opts) + return args.Get(0).(mobyclient.ContainerKillResult), args.Error(1) +} + +func (m *mockDockerClient) NetworkList(ctx context.Context, opts mobyclient.NetworkListOptions) (mobyclient.NetworkListResult, error) { + args := m.Called(ctx, opts) + return args.Get(0).(mobyclient.NetworkListResult), args.Error(1) +} + +func (m *mockDockerClient) NetworkInspect(ctx context.Context, id string, opts mobyclient.NetworkInspectOptions) (mobyclient.NetworkInspectResult, error) { + args := m.Called(ctx, id, opts) + return args.Get(0).(mobyclient.NetworkInspectResult), args.Error(1) +} + +func (m *mockDockerClient) NetworkRemove(ctx context.Context, id string, opts mobyclient.NetworkRemoveOptions) (mobyclient.NetworkRemoveResult, error) { + args := m.Called(ctx, id, opts) + return args.Get(0).(mobyclient.NetworkRemoveResult), args.Error(1) +} + type endlessReader struct { io.Reader } @@ -391,29 +411,39 @@ func TestDockerCopyTarStreamErrorInMkdir(t *testing.T) { // be logged as one. func TestRemoveIgnoresAutoRemoveRace(t *testing.T) { removeOpts := mobyclient.ContainerRemoveOptions{RemoveVolumes: true, Force: true} + killOpts := mobyclient.ContainerKillOptions{Signal: "SIGKILL"} for _, tc := range []struct { - name string - err error - wantLogs bool + name string + err error + wantWait bool + wantFailure bool }{ - {name: "removal in progress", err: cerrdefs.ErrConflict.WithMessage("removal of container abc is already in progress")}, + {name: "removal in progress", err: cerrdefs.ErrConflict.WithMessage("removal of container abc is already in progress"), wantWait: true}, {name: "already removed", err: cerrdefs.ErrNotFound.WithMessage("No such container: abc")}, {name: "removed cleanly", err: nil}, - {name: "real failure", err: errors.New("driver failed to remove root filesystem"), wantLogs: true}, + {name: "real failure", err: errors.New("driver failed to remove root filesystem"), wantFailure: true}, } { t.Run(tc.name, func(t *testing.T) { logger, hook := test.NewNullLogger() ctx := common.WithLogger(context.Background(), logger) client := &mockDockerClient{} + client.On("ContainerKill", ctx, "abc", killOpts).Return(mobyclient.ContainerKillResult{}, nil) client.On("ContainerRemove", ctx, "abc", removeOpts).Return(mobyclient.ContainerRemoveResult{}, tc.err) + if tc.wantWait { + removed := make(chan container.WaitResponse, 1) + removed <- container.WaitResponse{} + client.On("ContainerWait", mock.Anything, "abc", mobyclient.ContainerWaitOptions{Condition: container.WaitConditionRemoved}). + Return(mobyclient.ContainerWaitResult{Result: removed}) + } cr := &containerReference{id: "abc", cli: client} require.NoError(t, cr.remove()(ctx)) - assert.Empty(t, cr.id) - - if tc.wantLogs { + // a failure keeps the id, so a later Remove() can retry it + if tc.wantFailure { + assert.Equal(t, "abc", cr.id) assert.Len(t, hook.AllEntries(), 1) } else { + assert.Empty(t, cr.id) assert.Empty(t, hook.AllEntries()) } client.AssertExpectations(t) @@ -421,6 +451,20 @@ func TestRemoveIgnoresAutoRemoveRace(t *testing.T) { } } +// A container whose id was never learned, because find() could not reach the daemon or +// create() lost its reply, must still be removed rather than leaking with its network. It +// was never started here, so it is not worth a kill of its own. +func TestRemoveWithoutIDUsesName(t *testing.T) { + ctx := context.Background() + client := &mockDockerClient{} + client.On("ContainerRemove", ctx, "job-1", mobyclient.ContainerRemoveOptions{RemoveVolumes: true, Force: true}). + Return(mobyclient.ContainerRemoveResult{}, nil) + cr := &containerReference{cli: client, input: &NewContainerInput{Name: "job-1"}} + + require.NoError(t, cr.remove()(ctx)) + client.AssertExpectations(t) +} + // find() must drop a stale cached id so later Copy/Exec don't hit the // daemon with a torn-down container. func TestFindRevalidatesStaleID(t *testing.T) { diff --git a/act/container/docker_stub.go b/act/container/docker_stub.go index b146548c..bcd804d7 100644 --- a/act/container/docker_stub.go +++ b/act/container/docker_stub.go @@ -9,6 +9,7 @@ package container import ( "context" "runtime" + "time" "gitea.com/gitea/runner/act/common" @@ -72,3 +73,7 @@ func NewDockerNetworkRemoveExecutor(name string) common.Executor { return nil } } + +func RemoveOrphanNetworks(ctx context.Context, runnerUUID string, createdBefore time.Time) error { + return nil +} diff --git a/internal/app/run/runner.go b/internal/app/run/runner.go index 24eee853..f052bcd2 100644 --- a/internal/app/run/runner.go +++ b/internal/app/run/runner.go @@ -54,6 +54,7 @@ func RunnerCapabilities() []string { // Runner runs the pipeline. type Runner struct { name string + uuid string cfg *config.Config @@ -119,6 +120,7 @@ func NewRunner(cfg *config.Config, reg *config.Registration, cli client.Client) runner := &Runner{ name: reg.Name, + uuid: reg.UUID, cfg: cfg, client: cli, labels: ls, @@ -130,6 +132,9 @@ func NewRunner(cfg *config.Config, reg *config.Registration, cli client.Client) return runner } +// removeOrphanNetworks is a variable so tests can substitute one that needs no Docker daemon. +var removeOrphanNetworks = container.RemoveOrphanNetworks + // OnIdle performs lightweight maintenance during polling idle windows. // It runs synchronously on the poller goroutine; shouldRunIdleCleanup // throttles invocations to runner.idle_cleanup_interval so the impact on @@ -151,6 +156,21 @@ func (r *Runner) OnIdle(ctx context.Context) { if hostRoot := filepath.FromSlash(r.cfg.Host.WorkdirParent); hostRoot != "" { r.cleanupStaleDirs(ctx, hostRoot, isHostScratchDir) } + r.cleanupOrphanNetworks(ctx) +} + +// cleanupOrphanNetworks reclaims the per-job networks of jobs this runner did not live to +// tear down. A labelled network with no containers on it is finished with, and as for the +// directories above, a task beginning during the pass is safe because the cutoff keeps a +// network it has created but not yet attached a container to out of scope. +func (r *Runner) cleanupOrphanNetworks(ctx context.Context) { + if r.uuid == "" || !r.labels.RequireDocker() && !r.cfg.Container.RequireDocker { + return + } + cutoff := r.now().Add(-r.cfg.Runner.WorkdirCleanupAge) + if err := removeOrphanNetworks(ctx, r.uuid, cutoff); err != nil { + log.Warnf("failed to clean up networks left behind by earlier jobs: %v", err) + } } func (r *Runner) shouldRunIdleCleanup() bool { @@ -472,6 +492,9 @@ func (r *Runner) run(ctx context.Context, task *runnerv1.Task, reporter *report. ContainerNetworkCreateOptions: container.NewDockerNetworkCreateExecutorInput{ EnableIPv4: r.cfg.Container.NetworkCreateOptions.EnableIPv4, EnableIPv6: r.cfg.Container.NetworkCreateOptions.EnableIPv6, + // so a network this job leaks, if the runner dies before its teardown, can be + // told apart from one belonging to another runner on the same daemon + RunnerUUID: r.uuid, }, ContainerOptions: r.cfg.Container.Options, ContainerDaemonSocket: r.cfg.Container.DockerHost, diff --git a/internal/app/run/runner_idle_cleanup_test.go b/internal/app/run/runner_idle_cleanup_test.go index 5235ede8..0a56b505 100644 --- a/internal/app/run/runner_idle_cleanup_test.go +++ b/internal/app/run/runner_idle_cleanup_test.go @@ -297,3 +297,37 @@ func TestRunnerOnIdleSkipsWhenAlreadyCancelled(t *testing.T) { assert.DirExists(t, stale) } + +// The idle sweep reclaims the docker networks of jobs this runner did not live to tear down, +// and stays out of the way of runners that share the daemon but not the registration. +func TestRunnerOnIdleRemovesOrphanNetworks(t *testing.T) { + now := time.Date(2026, time.April, 29, 20, 0, 0, 0, time.UTC) + cfg := &config.Config{ + Container: config.Container{RequireDocker: true}, + Runner: config.Runner{ + WorkdirCleanupAge: 24 * time.Hour, + IdleCleanupInterval: time.Minute, + }, + } + + var swept []string + var sweptCutoff time.Time + origRemoveOrphanNetworks := removeOrphanNetworks + removeOrphanNetworks = func(_ context.Context, runnerUUID string, createdBefore time.Time) error { + swept = append(swept, runnerUUID) + sweptCutoff = createdBefore + return nil + } + t.Cleanup(func() { removeOrphanNetworks = origRemoveOrphanNetworks }) + + r := &Runner{uuid: "runner-1", cfg: cfg, now: func() time.Time { return now }} + r.OnIdle(context.Background()) + assert.Equal(t, []string{"runner-1"}, swept) + // a network of a job starting during the pass is younger than this and so out of scope + assert.Equal(t, now.Add(-24*time.Hour), sweptCutoff) + + // a host-only runner has no daemon to sweep + hostOnly := &Runner{uuid: "runner-2", cfg: &config.Config{Runner: cfg.Runner}, now: func() time.Time { return now }} + hostOnly.OnIdle(context.Background()) + assert.Equal(t, []string{"runner-1"}, swept) +} diff --git a/internal/pkg/config/config.example.yaml b/internal/pkg/config/config.example.yaml index 3486db29..030c5a4c 100644 --- a/internal/pkg/config/config.example.yaml +++ b/internal/pkg/config/config.example.yaml @@ -15,6 +15,9 @@ runner: # Where to store the registration result. file: .runner # Execute how many tasks concurrently at the same time. + # With `container.network` empty, each concurrent docker job takes a subnet from the + # daemon's address pool, so a high capacity can exhaust it. See `default-address-pools` + # in the docker daemon config. capacity: 1 # Extra environment variables to run jobs. envs: @@ -43,9 +46,12 @@ runner: # While idle, remove stale bind-workdir task directories and orphaned host-mode # scratch directories (left behind when a host cleanup delete stalls) older than # this duration. Setting either workdir_cleanup_age or idle_cleanup_interval to 0 - # (or any non-positive value) disables stale-directory cleanup entirely. + # (or any non-positive value) disables stale-directory cleanup entirely, along with + # the docker network cleanup below. workdir_cleanup_age: 24h - # Cadence for the idle stale-directory cleanup pass. + # Cadence for the idle cleanup pass. Besides the directories above, on runners that use + # docker it removes the per-job networks of jobs this runner did not live to tear down, + # which would otherwise hold a subnet of the daemon address pool until the host is rebuilt. idle_cleanup_interval: 10m # The base interval for periodic log flush to the Gitea instance. # Logs may be sent earlier if the buffer reaches log_report_batch_size @@ -161,7 +167,9 @@ container: # network_create_options only apply when `network` is left empty and the runner # auto-creates a per-job network that does not already exist. They have no effect # when a custom `network` name is set, because that network is used as-is and never - # created by the runner. Omit the entire block to use Docker's defaults. + # created by the runner. Omit the entire block to use Docker's defaults. An auto-created + # network is labelled com.gitea.runner.uuid=, which is how the idle + # cleanup tells its own leftovers apart from those of other runners on the same daemon. network_create_options: enable_ipv4: true # Omit to use Docker's default (IPv4 enabled). Set false to disable IPv4. enable_ipv6: false # Omit to use Docker's default (IPv6 disabled). Enabling it requires dockerd started with --ipv6. @@ -197,6 +205,9 @@ container: docker_host: "" # Pull docker image(s) even if already present. # Defaults to false when the key is omitted. + # Two exceptions: an image pinned by digest (image@sha256:...) cannot change, so it is never + # re-pulled, and a pull that fails while a copy is already on the host does not fail the job, + # which runs on that copy with a warning in its log. force_pull: false # Rebuild docker image(s) even if already present force_rebuild: false diff --git a/internal/pkg/config/config.go b/internal/pkg/config/config.go index 7e2fe29a..006edc39 100644 --- a/internal/pkg/config/config.go +++ b/internal/pkg/config/config.go @@ -42,7 +42,7 @@ type Runner struct { FetchInterval time.Duration `yaml:"fetch_interval"` // FetchInterval specifies the interval duration for fetching resources. FetchIntervalMax time.Duration `yaml:"fetch_interval_max"` // FetchIntervalMax specifies the maximum backoff interval when idle. WorkdirCleanupAge time.Duration `yaml:"workdir_cleanup_age"` // WorkdirCleanupAge removes stale bind-workdir task directories and orphaned host-mode scratch dirs older than this duration during idle cleanup. - IdleCleanupInterval time.Duration `yaml:"idle_cleanup_interval"` // IdleCleanupInterval runs stale-directory cleanup periodically while the runner is idle. Set to 0 to disable cleanup cadence. + IdleCleanupInterval time.Duration `yaml:"idle_cleanup_interval"` // IdleCleanupInterval runs the idle cleanup (stale directories and orphaned docker networks) periodically while the runner is idle. Set to 0 to disable cleanup cadence. LogReportInterval time.Duration `yaml:"log_report_interval"` // LogReportInterval specifies the base interval for periodic log flush. LogReportMaxLatency time.Duration `yaml:"log_report_max_latency"` // LogReportMaxLatency specifies the max time a log row can wait before being sent. LogReportBatchSize int `yaml:"log_report_batch_size"` // LogReportBatchSize triggers immediate log flush when buffer reaches this size. @@ -86,7 +86,7 @@ type Container struct { WorkdirParent string `yaml:"workdir_parent"` // WorkdirParent specifies the parent directory for the container's working directory. ValidVolumes []string `yaml:"valid_volumes"` // ValidVolumes specifies the volumes (including bind mounts) can be mounted to containers. DockerHost string `yaml:"docker_host"` // DockerHost specifies the Docker host. It overrides the value specified in environment variable DOCKER_HOST. - ForcePull bool `yaml:"force_pull"` // Pull docker image(s) even if already present + ForcePull bool `yaml:"force_pull"` // Pull docker image(s) even if already present, except digest-pinned ones. A pull that fails while a local copy exists is a warning, not a job failure. ForceRebuild bool `yaml:"force_rebuild"` // Rebuild docker image(s) even if already present RequireDocker bool `yaml:"require_docker"` // Always require a reachable docker daemon, even if not required by runner DockerTimeout time.Duration `yaml:"docker_timeout"` // Timeout to wait for the docker daemon to be reachable, if docker is required by require_docker or runner