From 19afebc53f811eb6a8e1002265ca0d7763a3ca07 Mon Sep 17 00:00:00 2001 From: silverwind Date: Mon, 14 Sep 2026 19:52:15 +0000 Subject: [PATCH] fix: send artifacts to Gitea when jobs cannot reach the cache server (#1225) Container jobs on a Docker bridge network isolated from the runner's cache server now upload artifacts to Gitea directly instead of timing out, with a job log warning on how to make the cache reachable. Fixes https://gitea.com/gitea/runner/issues/1211 Reviewed-on: https://gitea.com/gitea/runner/pulls/1225 Reviewed-by: bircni Co-authored-by: silverwind --- act/container/docker_network.go | 45 +++++++++++++++++++++++++ act/container/docker_network_test.go | 34 +++++++++++++++++++ act/container/docker_stub.go | 5 +++ e2e/gitea_fixture.go | 2 +- internal/app/run/runner.go | 37 +++++++++++++++----- internal/app/run/runner_test.go | 9 +++++ internal/pkg/config/config.example.yaml | 3 +- 7 files changed, 125 insertions(+), 10 deletions(-) diff --git a/act/container/docker_network.go b/act/container/docker_network.go index 406d1907..e6155d38 100644 --- a/act/container/docker_network.go +++ b/act/container/docker_network.go @@ -10,6 +10,7 @@ import ( "context" "errors" "fmt" + "net/netip" "strings" "time" @@ -172,3 +173,47 @@ func NewDockerNetworkRemoveExecutor(name string) common.Executor { return errors.Join(errs...) } } + +func IsolatedNetwork(ctx context.Context, addr netip.Addr, jobNetwork string) (string, error) { + cli, err := GetDockerClient(ctx) + if err != nil { + return "", err + } + defer cli.Close() + return isolatedNetwork(ctx, cli, addr, jobNetwork) +} + +func isolatedNetwork(ctx context.Context, cli client.APIClient, addr netip.Addr, jobNetwork string) (string, error) { + if jobNetwork == "host" || strings.HasPrefix(jobNetwork, "container:") { + return "", nil + } + containers, err := cli.ContainerList(ctx, client.ContainerListOptions{}) + if err != nil { + return "", err + } + var holderID string + for _, summary := range containers.Items { + if summary.NetworkSettings == nil { + continue + } + for _, endpoint := range summary.NetworkSettings.Networks { + if endpoint != nil && (endpoint.IPAddress == addr || endpoint.GlobalIPv6Address == addr) { + holderID = endpoint.NetworkID + } + } + } + if holderID == "" { + return "", nil + } + holder, err := cli.NetworkInspect(ctx, holderID, client.NetworkInspectOptions{}) + if err != nil || holder.Network.Driver != "bridge" { + return "", err + } + if jobNetwork != "" { + job, err := cli.NetworkInspect(ctx, jobNetwork, client.NetworkInspectOptions{}) + if err != nil || job.Network.ID == holderID { + return "", err + } + } + return holder.Network.Name, nil +} diff --git a/act/container/docker_network_test.go b/act/container/docker_network_test.go index cf2d72f1..04a8e6fb 100644 --- a/act/container/docker_network_test.go +++ b/act/container/docker_network_test.go @@ -6,16 +6,50 @@ package container import ( "context" "errors" + "net/netip" "testing" "time" cerrdefs "github.com/containerd/errdefs" + "github.com/moby/moby/api/types/container" "github.com/moby/moby/api/types/network" mobyclient "github.com/moby/moby/client" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) +func TestIsolatedNetwork(t *testing.T) { + ctx := context.Background() + cache, lan := netip.MustParseAddr("172.18.0.3"), netip.MustParseAddr("192.168.1.20") + client := &mockDockerClient{} + client.On("ContainerList", ctx, mobyclient.ContainerListOptions{}).Return(mobyclient.ContainerListResult{Items: []container.Summary{{}, { + NetworkSettings: &container.NetworkSettingsSummary{Networks: map[string]*network.EndpointSettings{ + "compose": {NetworkID: "c0ffee", IPAddress: cache}, + "lan": {NetworkID: "beef", IPAddress: lan}, + }}, + }}}, nil) + for ref, inspected := range map[string]network.Network{ + "c0ffee": {ID: "c0ffee", Name: "compose", Driver: "bridge"}, + "beef": {ID: "beef", Name: "lan", Driver: "macvlan"}, + "c0f": {ID: "c0ffee"}, + "jobs": {ID: "f00d"}, + } { + client.On("NetworkInspect", ctx, ref, mobyclient.NetworkInspectOptions{}). + Return(mobyclient.NetworkInspectResult{Network: network.Inspect{Network: inspected}}, nil) + } + check := func(addr netip.Addr, jobNetwork, want string) { + got, err := isolatedNetwork(ctx, client, addr, jobNetwork) + require.NoError(t, err) + assert.Equal(t, want, got, jobNetwork) + } + check(cache, "", "compose") + check(cache, "jobs", "compose") + check(cache, "c0f", "") + check(cache, "host", "") + check(lan, "", "") + check(netip.MustParseAddr("10.0.0.5"), "", "") +} + 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"))) diff --git a/act/container/docker_stub.go b/act/container/docker_stub.go index 552b43b2..45fd6962 100644 --- a/act/container/docker_stub.go +++ b/act/container/docker_stub.go @@ -9,6 +9,7 @@ package container import ( "context" "errors" + "net/netip" "runtime" "time" @@ -17,6 +18,10 @@ import ( "github.com/moby/moby/api/types/system" ) +func IsolatedNetwork(context.Context, netip.Addr, string) (string, error) { + return "", nil +} + // ImageExistsLocally returns a boolean indicating if an image with the // requested name, tag and architecture exists in the local docker image store func ImageExistsLocally(ctx context.Context, imageName, platform string) (bool, error) { diff --git a/e2e/gitea_fixture.go b/e2e/gitea_fixture.go index a5d9b93c..4f3abd82 100644 --- a/e2e/gitea_fixture.go +++ b/e2e/gitea_fixture.go @@ -360,6 +360,6 @@ func (f *GiteaFixture) Close(ctx context.Context) error { if f.id == "" { return f.cli.Close() } - _, removeErr := f.cli.ContainerRemove(ctx, f.id, mobyclient.ContainerRemoveOptions{Force: true}) + _, removeErr := f.cli.ContainerRemove(ctx, f.id, mobyclient.ContainerRemoveOptions{Force: true, RemoveVolumes: true}) return errors.Join(removeErr, f.cli.Close()) } diff --git a/internal/app/run/runner.go b/internal/app/run/runner.go index 30930ff7..7400d202 100644 --- a/internal/app/run/runner.go +++ b/internal/app/run/runner.go @@ -11,6 +11,7 @@ import ( "fmt" "maps" "net/http" + "net/netip" "net/url" "os" "path/filepath" @@ -67,6 +68,8 @@ type Runner struct { cacheHandler *artifactcache.Handler capabilities string + isolatedCacheNetwork func() string + runningTasks sync.Map runningCount atomic.Int64 lastIdleCleanupUnixNano atomic.Int64 @@ -110,7 +113,6 @@ func NewRunner(cfg *config.Config, reg *config.Registration, cli client.Client) } else { cacheHandler = handler envs["ACTIONS_CACHE_URL"] = handler.ExternalURL() + "/" - warnIfCacheUnreachable(cfg, handler.ExternalURL()) } } } @@ -135,6 +137,7 @@ func NewRunner(cfg *config.Config, reg *config.Registration, cli client.Client) now: time.Now, runHealthCheck: executeHealthCheck, } + runner.isolatedCacheNetwork = sync.OnceValue(runner.detectIsolatedCacheNetwork) return runner } @@ -477,7 +480,6 @@ func (r *Runner) run(ctx context.Context, task *runnerv1.Task, reporter *report. // is that server's responsibility to authenticate requests. revokeCache, resultsURL := r.registerCacheForTask(giteaRuntimeToken, preset.Repository, reporter) defer revokeCache() - r.setResultsService(envs, resultsURL) eventJSON, err := json.Marshal(preset.Event) if err != nil { @@ -518,6 +520,13 @@ func (r *Runner) run(ctx context.Context, task *runnerv1.Task, reporter *report. return fallbackPlatform() } + if resultsURL != "" && r.cacheIsolatedFrom(job, platformPicker) { + reporter.Logf("::warning::%s", runner.EscapeCommandData(fmt.Sprintf("jobs cannot reach the cache server at %s on docker network %q, so caching fails and artifacts go to Gitea directly, set cache.host and cache.port to an address jobs reach, or container.network to %[2]q", + r.cacheHandler.ExternalURL(), r.isolatedCacheNetwork()))) + resultsURL = "" + } + r.setResultsService(envs, resultsURL) + runnerConfig := &runner.Config{ // On Linux, Workdir will be like "///" // On Windows, Workdir will be like "\\\" @@ -847,12 +856,24 @@ func warnIgnoredCacheSecret(cfg *config.Config) { log.Warnf("%s is set but cache.external_server is not; the built-in cache server does not use a shared secret, so the value is ignored", key) } -func warnIfCacheUnreachable(cfg *config.Config, cacheURL string) { - if cfg.Cache.Host != "" || cfg.Container.Network != "" { - return +func (r *Runner) cacheIsolatedFrom(job *model.Job, pickPlatform func([]string) string) bool { + jobContainer := job.Container() + return (jobContainer != nil && jobContainer.Image != "" || pickPlatform(job.RunsOn()) != labels.SelfHostedPlatform) && r.isolatedCacheNetwork() != "" +} + +func (r *Runner) detectIsolatedCacheNetwork() string { + if r.cacheHandler == nil { + return "" } - if _, err := os.Stat("/.dockerenv"); err != nil { - return + addr, err := netip.ParseAddr(hostOf(r.cacheHandler.ExternalURL())) + if err != nil { + return "" } - log.Warnf("jobs are given %s for the cache server; if they cannot reach it, set container.network to a network this runner is on, or cache.host", cacheURL) + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + network, err := container.IsolatedNetwork(ctx, addr, r.cfg.Container.Network) + if err != nil { + log.Warnf("cannot check whether jobs reach the cache server: %v", err) + } + return network } diff --git a/internal/app/run/runner_test.go b/internal/app/run/runner_test.go index 1c40f342..8d1b3e8b 100644 --- a/internal/app/run/runner_test.go +++ b/internal/app/run/runner_test.go @@ -24,6 +24,7 @@ import ( "gitea.com/gitea/runner/internal/pkg/ver" "connectrpc.com/connect" + "gitea.dev/actionslib/pkg/model" runnerv1 "gitea.dev/actionslib/runner/v1" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" @@ -337,6 +338,14 @@ func TestNewRunnerCacheServiceV2(t *testing.T) { assert.Equal(t, resultsURL, envs["ACTIONS_RESULTS_URL"], instance) assert.Empty(t, envs[runner.CacheServiceV2Env]) } + + workflow, err := model.ReadWorkflow(strings.NewReader(`jobs: {native: {runs-on: native}, linux: {runs-on: linux}, containerized: {runs-on: native, container: alpine}, empty: {runs-on: native, container: ""}}`)) + require.NoError(t, err) + r.isolatedCacheNetwork = func() string { return "compose" } + pickPlatform := func(runsOn []string) string { return map[string]string{"native": labels.SelfHostedPlatform}[runsOn[0]] } + for job, isolated := range map[string]bool{"native": false, "linux": true, "containerized": true, "empty": false} { + assert.Equal(t, isolated, r.cacheIsolatedFrom(workflow.GetJob(job), pickPlatform), job) + } } // The v1 cache client appends its path to ACTIONS_CACHE_URL without a separator, so a configured diff --git a/internal/pkg/config/config.example.yaml b/internal/pkg/config/config.example.yaml index ee284a64..5b98964c 100644 --- a/internal/pkg/config/config.example.yaml +++ b/internal/pkg/config/config.example.yaml @@ -180,7 +180,8 @@ cache: # Serve the actions cache service v2 API. The actions that use it fall back to v1 on any host # they do not take for GitHub, so reaching it means editing that check out of their own bundle, # put back after the copy into the job. That edit is made either way, this only governs the API - # advertised. A bundle that does not match is left alone. With v2, uploads need a reachable cache. + # advertised. A bundle that does not match is left alone. With v2, artifact uploads go via the + # cache server unless container jobs cannot reach its Docker network. #v2: true # How the cache server discards entries, ignored when external_server is set since that # server applies its own. Leave a setting out for its default; 0s or 0 turns the three