feat!: serve the actions cache from the runner (#1229)

Make cache requests go through the runner and remove outbound traffic from the cache server. The runner forwards them to `cache.external_server` when one is set, and attaches itself to each job network so jobs can reach it.

Benefits:

1. Jobs need no http proxy rules for the cache server, which `@actions/http-client` mishandles for CIDR ranges in `no_proxy` (https://github.com/actions/toolkit/pull/2495).
2. Only runners reach a shared cache server, which needs no route to Gitea.
3. A runner container sharing the host Docker socket now caches, where jobs previously could not reach it.

Breaks:

1. Jobs that reach `external_server` but not their runner, such as a remote `DOCKER_HOST`. The built-in cache has always needed that reachability, so both modes now agree. Set `cache.host` and `cache.port` to an address those jobs reach.
2. Jobs on `container.network: bridge`, where Docker rejects the alias, so the runner cannot attach and their cache fails as it does today. Leave `container.network` empty, or name a user-defined network.

*Written by Claude.*

---------

Co-authored-by: bircni <[email protected]>
Reviewed-on: https://gitea.com/gitea/runner/pulls/1229
Reviewed-by: bircni <[email protected]>
Co-authored-by: silverwind <[email protected]>
This commit is contained in:
silverwindandbircni authored and bircni committed 2026-09-24 18:56:07 +00:00
1 parent 95daa50d9b
commit ace8653105
17 files changed
+428 -252

No files matched your search

+67 -21
View File
@@ -17,6 +17,7 @@ import (
"io"
"net"
"net/http"
"net/http/httputil"
"net/url"
"os"
"path/filepath"
@@ -57,10 +58,9 @@ type JobCredential struct {
Repo string `json:"repo"`
// Results is the instance whose artifact service this server forwards for the job, and
// InsecureTLS how the runner reaches it; see results.go. The tags are the wire format a
// remote runner registers with.
Results string `json:"results"`
InsecureTLS bool `json:"insecure_tls"`
// InsecureTLS how the runner reaches it; see results.go.
Results string `json:"-"`
InsecureTLS bool `json:"-"`
// PublicURL is this server as a reverse proxy makes the job reach it, not the listen address.
PublicURL string `json:"public_url"`
@@ -117,6 +117,7 @@ type Options struct {
Dir string
OutboundIP string
Port uint16
Upstream string
// InternalSecret, when non-empty, enables a control-plane API at
// /_internal/{register,revoke} that lets a remote runner pre-register the
@@ -129,7 +130,7 @@ type Options struct {
Logger logrus.FieldLogger
}
// StartHandler opens the on-disk cache store and starts the HTTP server.
// StartHandler starts an embedded cache or forwards cache requests to Upstream.
func StartHandler(opts Options) (*Handler, error) {
dir, logger := opts.Dir, opts.Logger
h := &Handler{
@@ -147,6 +148,25 @@ func StartHandler(opts Options) (*Handler, error) {
logger = logger.WithField("module", "artifactcache")
h.logger = logger
if opts.OutboundIP != "" {
h.outboundIP = opts.OutboundIP
} else if ip := common.GetOutboundIP(); ip == nil {
return nil, errors.New("unable to determine outbound IP address")
} else {
h.outboundIP = ip.String()
}
if opts.Upstream != "" {
handler, err := h.cacheForwarder(opts.Upstream)
if err != nil {
return nil, err
}
if err := h.serve(opts.Port, handler); err != nil {
return nil, err
}
return h, nil
}
if dir == "" {
home, err := os.UserHomeDir()
if err != nil {
@@ -166,14 +186,6 @@ func StartHandler(opts Options) (*Handler, error) {
}
h.storage = storage
if opts.OutboundIP != "" {
h.outboundIP = opts.OutboundIP
} else if ip := common.GetOutboundIP(); ip == nil {
return nil, errors.New("unable to determine outbound IP address")
} else {
h.outboundIP = ip.String()
}
secret, err := loadOrCreateSecret(dir)
if err != nil {
return nil, err
@@ -201,34 +213,69 @@ func StartHandler(opts Options) (*Handler, error) {
h.gcCache()
if err := h.serve(opts.Port, router); err != nil {
return nil, err
}
return h, nil
}
func (h *Handler) serve(port uint16, handler http.Handler) error {
// Listen on all interfaces. Binding to outboundIP only would give no real
// security benefit (it is the LAN/internet-facing address either way) and
// can break Docker Desktop variants where the host's outbound IP is not
// routable from inside the container network. Authentication is enforced
// by the bearer middleware and per-repo scoping, not by reachability.
listener, err := net.Listen("tcp", fmt.Sprintf(":%d", opts.Port))
listener, err := net.Listen("tcp", fmt.Sprintf(":%d", port))
if err != nil {
return nil, err
return err
}
addr, ok := listener.Addr().(*net.TCPAddr)
if !ok {
listener.Close()
return nil, fmt.Errorf("cache server listens on %T, want a TCP address", listener.Addr())
return fmt.Errorf("cache server listens on %T, want a TCP address", listener.Addr())
}
h.port = addr.Port
server := &http.Server{
ReadHeaderTimeout: 2 * time.Second,
Handler: router,
Handler: handler,
}
go func() {
if err := server.Serve(listener); err != nil && errors.Is(err, net.ErrClosed) {
logger.Errorf("http serve: %v", err)
h.logger.Errorf("http serve: %v", err)
}
}()
h.listener = listener
h.server = server
return h, nil
return nil
}
func (h *Handler) cacheForwarder(upstream string) (http.Handler, error) {
target, err := url.Parse(strings.TrimRight(upstream, "/"))
if err != nil {
return nil, fmt.Errorf("parse cache upstream: %w", err)
}
transport, _ := http.DefaultTransport.(*http.Transport)
transport = transport.Clone()
transport.MaxIdleConnsPerHost = 64 // parallel block uploads would otherwise redial past the default 2
proxy := &httputil.ReverseProxy{
Transport: transport,
Rewrite: func(r *httputil.ProxyRequest) {
r.SetURL(target)
r.Out.URL.RawQuery = r.In.URL.RawQuery // Rewrite drops unparsable query params, and signed URLs must pass verbatim.
},
ErrorHandler: func(w http.ResponseWriter, r *http.Request, err error) {
h.logger.Warnf("cache forward %s %s: %v", r.Method, r.URL.Path, err)
h.twirpError(w, r, twirpUnavailable, errors.New("cache upstream is unavailable"))
},
}
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if strings.HasPrefix(r.URL.Path, apiPath+"/") || strings.HasPrefix(r.URL.Path, cacheServiceV2Path+"/") {
proxy.ServeHTTP(w, r)
} else {
h.forwardOrNotFound(w, r)
}
}), nil
}
func (h *Handler) ExternalURL() string {
@@ -708,8 +755,7 @@ func (h *Handler) internalRegister(w http.ResponseWriter, r *http.Request, _ htt
return
}
h.RegisterJob(body.Token, body.JobCredential)
// A server too old to forward answers without this, which is how the caller knows.
h.responseJSON(w, r, http.StatusOK, map[string]any{"results_url": h.ResultsURL(body.JobCredential)})
h.responseJSON(w, r, http.StatusOK)
}
// POST /_internal/revoke
+54 -1
View File
@@ -12,6 +12,7 @@ import (
"fmt"
"io"
"net/http"
"net/http/httptest"
"os"
"path/filepath"
"strings"
@@ -56,6 +57,55 @@ func TestHandler_ExternalURL(t *testing.T) {
assert.Equal(t, "http://[2001:db8::1]:8080", (&Handler{outboundIP: "2001:db8::1", port: 8080}).ExternalURL())
}
func TestHandler_Upstream(t *testing.T) {
var upstream *httptest.Server
upstream = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
assert.Equal(t, http.MethodPut, r.Method)
assert.Equal(t, "/cache%20server"+blobPath+"/%34%32", r.URL.EscapedPath())
assert.Equal(t, "sig=opaque%2Fvalue&raw=a;b", r.URL.RawQuery)
assert.Equal(t, strings.TrimPrefix(upstream.URL, "http://"), r.Host)
assert.Equal(t, "Bearer unregistered", r.Header.Get("Authorization"))
_, err := io.Copy(w, r.Body)
assert.NoError(t, err)
}))
t.Cleanup(upstream.Close)
dir := filepath.Join(t.TempDir(), "artifactcache")
handler, err := StartHandler(Options{
Dir: dir,
OutboundIP: "127.0.0.1",
Upstream: upstream.URL + "/cache%20server//",
})
require.NoError(t, err)
t.Cleanup(func() { require.NoError(t, handler.Close()) })
req, err := http.NewRequestWithContext(t.Context(), http.MethodPut,
handler.ExternalURL()+blobPath+"/%34%32?sig=opaque%2Fvalue&raw=a;b", strings.NewReader("cache content"))
require.NoError(t, err)
req.Header.Set("Authorization", "Bearer unregistered")
resp, err := http.DefaultClient.Do(req)
require.NoError(t, err)
forwarded, err := io.ReadAll(resp.Body)
resp.Body.Close()
require.NoError(t, err)
assert.Equal(t, "cache content", string(forwarded))
resp, err = http.Post(handler.ExternalURL()+internalPath+"/register", "application/json", nil)
require.NoError(t, err)
resp.Body.Close()
assert.Equal(t, http.StatusNotFound, resp.StatusCode)
upstream.Close()
resp, err = http.Post(handler.ExternalURL()+cacheServiceV2Path+"/CreateCacheEntry", "application/json", strings.NewReader(`{}`))
require.NoError(t, err)
var failure map[string]string
err = json.UnmarshalRead(resp.Body, &failure)
resp.Body.Close()
require.NoError(t, err)
assert.Equal(t, http.StatusServiceUnavailable, resp.StatusCode)
assert.Equal(t, map[string]string{"code": "unavailable", "msg": "cache upstream is unavailable"}, failure)
assert.NoDirExists(t, dir)
}
func TestHandler(t *testing.T) {
dir := filepath.Join(t.TempDir(), "artifactcache")
handler, err := StartHandler(Options{Dir: dir})
@@ -1522,7 +1572,10 @@ func TestHandler_InternalAPI_AuthAndUsage(t *testing.T) {
}
assert.Equal(t, http.StatusUnauthorized, probe("via-internal-api"))
assert.Equal(t, http.StatusOK, post("/_internal/register", secret, `{"token":"via-internal-api","repo":"owner/repo"}`))
assert.Equal(t, http.StatusOK, post("/_internal/register", secret, `{"token":"via-internal-api","repo":"owner/repo","public_url":"http://runner.example","results":"https://gitea.example","insecure_tls":true}`))
cred, ok := handler.lookupCredential("via-internal-api")
require.True(t, ok)
assert.Equal(t, JobCredential{Repo: testRepo, PublicURL: "http://runner.example"}, cred)
assert.NotEqual(t, http.StatusUnauthorized, probe("via-internal-api"))
assert.Equal(t, http.StatusOK, post("/_internal/revoke", secret, `{"token":"via-internal-api"}`))
assert.Equal(t, http.StatusUnauthorized, probe("via-internal-api"))
+3
View File
@@ -40,6 +40,7 @@ const (
blobUploadURLTTL = time.Hour
twirpInternal = "internal"
twirpUnavailable = "unavailable"
twirpUnauthenticated = "unauthenticated"
)
@@ -261,6 +262,8 @@ func (h *Handler) twirpError(w http.ResponseWriter, r *http.Request, code string
switch code {
case twirpInternal:
status = http.StatusInternalServerError
case twirpUnavailable:
status = http.StatusServiceUnavailable
case twirpUnauthenticated:
status = http.StatusUnauthorized
}
+1 -7
View File
@@ -17,9 +17,6 @@ import (
// Gitea implements only the artifact half of it. Forwarding that half from here makes this origin
// the whole service, so ACTIONS_RESULTS_URL can point at it truthfully, which is what the clients
// this runner cannot patch need, docker buildx among them.
//
// The instance to forward to travels with the job registration rather than with configuration, so
// a cache server shared between runners serves each of their instances.
const artifactServicePath = "/twirp/github.actions.results.api.v1.ArtifactService/"
// forwardOrNotFound is the router's fallback: the artifact service of the instance the job
@@ -81,9 +78,6 @@ func (h *Handler) mustProxy(r *http.Request, cred JobCredential, target *url.URL
return true
}
scheme := r.Header.Get("X-Forwarded-Proto")
if base, err := url.Parse(cred.PublicURL); err == nil && base.Scheme != "" {
scheme = base.Scheme
}
if scheme == "" {
scheme = "http"
}
@@ -98,4 +92,4 @@ func mergeQuery(target, request string) string {
}
// insecureTransport is shared, because a transport per request would pool no connections.
var insecureTransport = &http.Transport{TLSClientConfig: &tls.Config{InsecureSkipVerify: true}} //nolint:gosec // the runner reaches its instance on the operator's say-so
var insecureTransport = &http.Transport{Proxy: http.ProxyFromEnvironment, TLSClientConfig: &tls.Config{InsecureSkipVerify: true}} //nolint:gosec // the runner reaches its instance on the operator's say-so
+6 -6
View File
@@ -54,11 +54,11 @@ func TestFrontResultsService(t *testing.T) {
assert.Contains(t, body["msg"], "bearer")
assert.NotEmpty(t, body["msg"])
defer handler.RegisterJob(token, JobCredential{Repo: "owner/repo", Results: gitea.URL, PublicURL: "https://cache.example"})()
defer handler.RegisterJob(token, JobCredential{Repo: "owner/repo", Results: selfSigned.URL, InsecureTLS: true})()
status, _ = post(rpc)
require.Equal(t, http.StatusOK, status, "a registered https public URL onto an http instance must proxy, not redirect")
assert.Equal(t, strings.TrimPrefix(gitea.URL, "http://"), gotHost, "Gitea must see the host it mints its URLs from")
require.Equal(t, http.StatusOK, status, "an http job reaching an https instance must proxy")
assert.Equal(t, strings.TrimPrefix(selfSigned.URL, "https://"), gotHost, "Gitea must see the host it mints its URLs from")
assert.Empty(t, gotProto, "a forwarded scheme would make an https Gitea mint http URLs")
assert.Equal(t, rpc, gotPath)
@@ -87,15 +87,15 @@ func TestFrontResultsService(t *testing.T) {
},
{
name: "a trusted https instance is reached by the job itself",
cred: JobCredential{Results: selfSigned.URL, PublicURL: "https://cache.example"}, want: http.StatusTemporaryRedirect,
cred: JobCredential{Results: selfSigned.URL}, forwardedProto: "https", want: http.StatusTemporaryRedirect,
},
{
name: "the job does not share this server's disregard for the certificate",
cred: JobCredential{Results: selfSigned.URL, PublicURL: "https://cache.example", InsecureTLS: true}, want: http.StatusOK,
cred: JobCredential{Results: selfSigned.URL, InsecureTLS: true}, forwardedProto: "https", want: http.StatusOK,
},
{name: "an http job is not redirected to an https instance", cred: JobCredential{Results: selfSigned.URL}, want: http.StatusInternalServerError},
{
name: "a runner too old to send its public URL leaves the terminator to say so",
name: "a forwarded https scheme onto an http instance is proxied",
cred: JobCredential{Results: gitea.URL}, forwardedProto: "https", want: http.StatusOK,
},
{
+67 -9
View File
@@ -16,6 +16,8 @@ import (
"gitea.com/gitea/runner/act/common"
cerrdefs "github.com/containerd/errdefs"
"github.com/moby/moby/api/types/network"
"github.com/moby/moby/client"
)
@@ -33,17 +35,17 @@ const (
// 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 {
func RemoveOrphanNetworks(ctx context.Context, runnerUUID, detach 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)
return removeOrphanNetworks(ctx, cli, runnerUUID, detach, createdBefore)
}
func removeOrphanNetworks(ctx context.Context, cli client.APIClient, runnerUUID string, createdBefore time.Time) error {
func removeOrphanNetworks(ctx context.Context, cli client.APIClient, runnerUUID, detach string, createdBefore time.Time) error {
networks, err := cli.NetworkList(ctx, client.NetworkListOptions{
Filters: make(client.Filters).Add("label", runnerUUIDLabel+"="+runnerUUID),
})
@@ -52,6 +54,7 @@ func removeOrphanNetworks(ctx context.Context, cli client.APIClient, runnerUUID
}
var errs []error
networks:
for _, n := range networks.Items {
result, err := cli.NetworkInspect(ctx, n.ID, client.NetworkInspectOptions{})
if err != nil {
@@ -60,9 +63,18 @@ func removeOrphanNetworks(ctx context.Context, cli client.APIClient, runnerUUID
}
// 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) {
if len(result.Network.Containers) > 1 || result.Network.Created.After(createdBefore) {
continue
}
for containerID := range result.Network.Containers {
if detach == "" || !strings.HasPrefix(containerID, detach) {
continue networks
}
if err := disconnectNetwork(ctx, cli, n.ID, containerID); err != nil {
errs = append(errs, fmt.Errorf("failed to disconnect container from network %s: %w", n.Name, err))
continue networks
}
}
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
@@ -135,6 +147,50 @@ func isAddressPoolExhausted(err error) bool {
strings.Contains(msg, "could not find an available, non-overlapping IPv4 address pool among the defaults") // docker 24 and older
}
func NewDockerNetworkConnectExecutor(networkName, container string) common.Executor {
return func(ctx context.Context) error {
cli, err := GetDockerClient(ctx)
if err != nil {
return err
}
defer cli.Close()
if err := connectNetwork(ctx, cli, networkName, container); err != nil {
common.Logger(ctx).Warnf("cannot connect %s to network %s, so jobs on it cannot reach the cache: %v", container, networkName, err)
}
return nil
}
}
func connectNetwork(ctx context.Context, cli client.APIClient, networkName, container string) error {
_, err := cli.NetworkConnect(ctx, networkName, client.NetworkConnectOptions{
Container: container,
EndpointConfig: &network.EndpointSettings{Aliases: []string{container}},
})
if err != nil && (strings.Contains(err.Error(), "already exists in network") || strings.Contains(err.Error(), "already connected")) {
return nil
}
return err
}
func NewDockerNetworkDisconnectExecutor(networkName, container string) common.Executor {
return func(ctx context.Context) error {
cli, err := GetDockerClient(ctx)
if err != nil {
return err
}
defer cli.Close()
return disconnectNetwork(ctx, cli, networkName, container)
}
}
func disconnectNetwork(ctx context.Context, cli client.APIClient, networkName, container string) error {
_, err := cli.NetworkDisconnect(ctx, networkName, client.NetworkDisconnectOptions{Container: container, Force: true})
if cerrdefs.IsNotFound(err) || (err != nil && strings.Contains(err.Error(), "is not connected")) {
return nil
}
return err
}
func NewDockerNetworkRemoveExecutor(name string) common.Executor {
return func(ctx context.Context) error {
cli, err := GetDockerClient(ctx)
@@ -174,16 +230,17 @@ func NewDockerNetworkRemoveExecutor(name string) common.Executor {
}
}
func IsolatedNetwork(ctx context.Context, addr netip.Addr, jobNetwork string) (string, error) {
// IsolatedCacheContainer returns the container holding addr, when jobs on jobNetwork cannot reach it there.
func IsolatedCacheContainer(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)
return isolatedCacheContainer(ctx, cli, addr, jobNetwork)
}
func isolatedNetwork(ctx context.Context, cli client.APIClient, addr netip.Addr, jobNetwork string) (string, error) {
func isolatedCacheContainer(ctx context.Context, cli client.APIClient, addr netip.Addr, jobNetwork string) (string, error) {
if jobNetwork == "host" || strings.HasPrefix(jobNetwork, "container:") {
return "", nil
}
@@ -191,7 +248,7 @@ func isolatedNetwork(ctx context.Context, cli client.APIClient, addr netip.Addr,
if err != nil {
return "", err
}
var holderID string
var holderID, holderContainer string
for _, summary := range containers.Items {
if summary.NetworkSettings == nil {
continue
@@ -199,6 +256,7 @@ func isolatedNetwork(ctx context.Context, cli client.APIClient, addr netip.Addr,
for _, endpoint := range summary.NetworkSettings.Networks {
if endpoint != nil && (endpoint.IPAddress == addr || endpoint.GlobalIPv6Address == addr) {
holderID = endpoint.NetworkID
holderContainer = summary.ID[:12]
}
}
}
@@ -215,5 +273,5 @@ func isolatedNetwork(ctx context.Context, cli client.APIClient, addr netip.Addr,
return "", err
}
}
return holder.Network.Name, nil
return holderContainer, nil
}
+58 -9
View File
@@ -18,11 +18,12 @@ import (
"github.com/stretchr/testify/require"
)
func TestIsolatedNetwork(t *testing.T) {
func TestIsolatedCacheContainer(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{{}, {
ID: "0123456789abcdef",
NetworkSettings: &container.NetworkSettingsSummary{Networks: map[string]*network.EndpointSettings{
"compose": {NetworkID: "c0ffee", IPAddress: cache},
"lan": {NetworkID: "beef", IPAddress: lan},
@@ -38,12 +39,12 @@ func TestIsolatedNetwork(t *testing.T) {
Return(mobyclient.NetworkInspectResult{Network: network.Inspect{Network: inspected}}, nil)
}
check := func(addr netip.Addr, jobNetwork, want string) {
got, err := isolatedNetwork(ctx, client, addr, jobNetwork)
got, err := isolatedCacheContainer(ctx, client, addr, jobNetwork)
require.NoError(t, err)
assert.Equal(t, want, got, jobNetwork)
}
check(cache, "", "compose")
check(cache, "jobs", "compose")
check(cache, "", "0123456789ab")
check(cache, "jobs", "0123456789ab")
check(cache, "c0f", "")
check(cache, "host", "")
check(lan, "", "")
@@ -56,9 +57,44 @@ func TestIsAddressPoolExhausted(t *testing.T) {
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 (m *mockDockerClient) NetworkConnect(ctx context.Context, id string, opts mobyclient.NetworkConnectOptions) (mobyclient.NetworkConnectResult, error) {
return mobyclient.NetworkConnectResult{}, m.Called(ctx, id, opts).Error(0)
}
func (m *mockDockerClient) NetworkDisconnect(ctx context.Context, id string, opts mobyclient.NetworkDisconnectOptions) (mobyclient.NetworkDisconnectResult, error) {
return mobyclient.NetworkDisconnectResult{}, m.Called(ctx, id, opts).Error(0)
}
func TestConnectAndDisconnectNetwork(t *testing.T) {
ctx := context.Background()
connect := func(answer error) error {
client := &mockDockerClient{}
client.On("NetworkConnect", ctx, "jobs", mobyclient.NetworkConnectOptions{
Container: "0123456789ab",
EndpointConfig: &network.EndpointSettings{Aliases: []string{"0123456789ab"}},
}).Return(answer).Once()
return connectNetwork(ctx, client, "jobs", "0123456789ab")
}
require.NoError(t, connect(nil))
require.NoError(t, connect(errors.New("endpoint with name runner already exists in network jobs")))
require.NoError(t, connect(errors.New("container runner already connected to network jobs")))
require.ErrorIs(t, connect(context.Canceled), context.Canceled)
disconnect := func(answer error) error {
client := &mockDockerClient{}
client.On("NetworkDisconnect", ctx, "jobs", mobyclient.NetworkDisconnectOptions{
Container: "0123456789ab", Force: true,
}).Return(answer).Once()
return disconnectNetwork(ctx, client, "jobs", "0123456789ab")
}
require.NoError(t, disconnect(cerrdefs.ErrNotFound))
require.NoError(t, disconnect(errors.New("container runner is not connected to the network jobs")))
require.ErrorIs(t, disconnect(context.Canceled), context.Canceled)
}
// Of this runner's networks, only the ones nothing but its own cache container 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)
@@ -69,16 +105,29 @@ func TestRemoveOrphanNetworks(t *testing.T) {
{ID: "orphan"},
{ID: "busy"},
{ID: "starting"},
{ID: "cached"},
{ID: "shared"},
}}, 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)
Return(mobyclient.NetworkInspectResult{Network: network.Inspect{
Network: network.Network{Created: cutoff.Add(time.Second)},
Containers: map[string]network.EndpointResource{"0123456789abcdef": {}},
}}, nil)
client.On("NetworkInspect", ctx, "cached", mobyclient.NetworkInspectOptions{}).
Return(mobyclient.NetworkInspectResult{Network: network.Inspect{Containers: map[string]network.EndpointResource{"0123456789abcdef": {}}}}, nil)
client.On("NetworkInspect", ctx, "shared", mobyclient.NetworkInspectOptions{}).
Return(mobyclient.NetworkInspectResult{Network: network.Inspect{Containers: map[string]network.EndpointResource{"0123456789abcdef": {}, "job": {}}}}, nil)
client.On("NetworkRemove", ctx, "orphan", mobyclient.NetworkRemoveOptions{}).
Return(mobyclient.NetworkRemoveResult{}, nil)
client.On("NetworkDisconnect", ctx, "cached", mobyclient.NetworkDisconnectOptions{Container: "0123456789abcdef", Force: true}).
Return(nil).Once()
client.On("NetworkRemove", ctx, "cached", mobyclient.NetworkRemoveOptions{}).
Return(mobyclient.NetworkRemoveResult{}, nil).Once()
require.NoError(t, removeOrphanNetworks(ctx, client, "runner-1", cutoff))
require.NoError(t, removeOrphanNetworks(ctx, client, "runner-1", "0123456789ab", cutoff))
client.AssertExpectations(t)
}
+14 -2
View File
@@ -18,7 +18,7 @@ import (
"github.com/moby/moby/api/types/system"
)
func IsolatedNetwork(context.Context, netip.Addr, string) (string, error) {
func IsolatedCacheContainer(context.Context, netip.Addr, string) (string, error) {
return "", nil
}
@@ -91,7 +91,19 @@ func NewDockerNetworkRemoveExecutor(name string) common.Executor {
}
}
func RemoveOrphanNetworks(ctx context.Context, runnerUUID string, createdBefore time.Time) error {
func NewDockerNetworkConnectExecutor(_, _ string) common.Executor {
return func(_ context.Context) error {
return nil
}
}
func NewDockerNetworkDisconnectExecutor(_, _ string) common.Executor {
return func(_ context.Context) error {
return nil
}
}
func RemoveOrphanNetworks(_ context.Context, _, _ string, _ time.Time) error {
return nil
}
+4
View File
@@ -610,6 +610,8 @@ func (rc *RunContext) startJobContainer() common.Executor {
return common.NewPipelineExecutor(
container.NewDockerNetworkCreateExecutor(networkName, rc.Config.ContainerNetworkCreateOptions).
IfBool(createAndDeleteNetwork),
container.NewDockerNetworkConnectExecutor(networkName, rc.Config.CacheContainer).
IfBool(rc.Config.CacheContainer != ""),
rc.startServiceContainers(),
rc.reportUnstartedServices(),
func(ctx context.Context) error { return rc.createJobVolumes(ctx, containerInput.Mounts) },
@@ -659,6 +661,8 @@ func (rc *RunContext) cleanupJobResources(networkName string, createAndDeleteNet
}
if createAndDeleteNetwork {
logger.Infof("Cleaning up network for job %s, and network name is: %s", rc.JobName, networkName)
errs = append(errs, container.NewDockerNetworkDisconnectExecutor(networkName, rc.Config.CacheContainer).
IfBool(rc.Config.CacheContainer != "")(ctx))
errs = append(errs, container.NewDockerNetworkRemoveExecutor(networkName)(ctx))
}
return errors.Join(errs...)
+2 -2
View File
@@ -743,7 +743,7 @@ func TestCleanupJobResourcesContinuesAfterFailure(t *testing.T) {
service.On("Remove").Return(func(context.Context) error { return removeError }).Once()
service.On("Close").Return(func(context.Context) error { return closeError }).Once()
rc := &RunContext{
Config: &Config{},
Config: &Config{CacheContainer: "cache-container"},
Run: &model.Run{Workflow: &model.Workflow{Name: "wf"}, JobID: "job"},
JobContainer: job,
serviceContainers: []*serviceContainer{{name: "svc", container: service}},
@@ -794,7 +794,7 @@ func TestCleanupJobResourcesContinuesAfterFailure(t *testing.T) {
if name == "canceled" {
require.ErrorIs(t, err, context.Canceled)
} else {
for _, operation := range []string{"DELETE /volumes/" + rc.jobContainerName(), "DELETE /volumes/" + rc.jobContainerName() + "-env", "GET /networks"} {
for _, operation := range []string{"DELETE /volumes/" + rc.jobContainerName(), "DELETE /volumes/" + rc.jobContainerName() + "-env", "POST /networks/job-network/disconnect", "GET /networks"} {
require.ErrorContains(t, err, operation)
}
assert.Equal(t, 2, volumeRemovals)
+1
View File
@@ -57,6 +57,7 @@ type Config struct {
DisableActEnv bool // do not inject the ACT=true environment variable into jobs
ContainerNetworkMode docker_container.NetworkMode // the network mode of job containers (the value of --network)
ContainerNetworkCreateOptions container.NewDockerNetworkCreateExecutorInput // the default network create options
CacheContainer string // let jobs reach an otherwise isolated cache
ProxyEnv map[string]string // the proxy variables the job runs with, also given to service containers and image builds
NoActionPatch bool // run actions exactly as published, applying no compatibility patches, see patch_actions.go