From cbc838d8ad0f4d71cd0d6f414dfbc904394586fd Mon Sep 17 00:00:00 2001 From: Zettat123 Date: Wed, 19 Apr 2023 11:23:28 +0800 Subject: [PATCH 01/31] Support services (#42) Removed createSimpleContainerName and AutoRemove flag Co-authored-by: Lunny Xiao Co-authored-by: Jason Song Reviewed-on: https://gitea.com/gitea/act/pulls/42 Reviewed-by: Jason Song Co-authored-by: Zettat123 Co-committed-by: Zettat123 --- pkg/container/container_types.go | 36 +++++----- pkg/container/docker_network.go | 38 +++++++++++ pkg/container/docker_run.go | 21 ++++++ pkg/container/host_environment.go | 6 ++ pkg/runner/job_executor.go | 15 +++++ pkg/runner/run_context.go | 93 ++++++++++++++++++++++++++ pkg/runner/runner_test.go | 37 ++++++++++ pkg/runner/testdata/services/push.yaml | 26 +++++++ 8 files changed, 255 insertions(+), 17 deletions(-) create mode 100644 pkg/container/docker_network.go create mode 100644 pkg/runner/testdata/services/push.yaml diff --git a/pkg/container/container_types.go b/pkg/container/container_types.go index 767beb520a3..0036b9a8a5f 100644 --- a/pkg/container/container_types.go +++ b/pkg/container/container_types.go @@ -9,23 +9,24 @@ import ( // NewContainerInput the input for the New function type NewContainerInput struct { - Image string - Username string - Password string - Entrypoint []string - Cmd []string - WorkingDir string - Env []string - Binds []string - Mounts map[string]string - Name string - Stdout io.Writer - Stderr io.Writer - NetworkMode string - Privileged bool - UsernsMode string - Platform string - Options string + Image string + Username string + Password string + Entrypoint []string + Cmd []string + WorkingDir string + Env []string + Binds []string + Mounts map[string]string + Name string + Stdout io.Writer + Stderr io.Writer + NetworkMode string + Privileged bool + UsernsMode string + Platform string + Options string + NetworkAliases []string } // FileEntry is a file to copy to a container @@ -38,6 +39,7 @@ type FileEntry struct { // Container for managing docker run containers type Container interface { Create(capAdd []string, capDrop []string) common.Executor + ConnectToNetwork(name string) common.Executor Copy(destPath string, files ...*FileEntry) common.Executor CopyTarStream(ctx context.Context, destPath string, tarStream io.Reader) error CopyDir(destPath string, srcPath string, useGitIgnore bool) common.Executor diff --git a/pkg/container/docker_network.go b/pkg/container/docker_network.go new file mode 100644 index 00000000000..76394a93df8 --- /dev/null +++ b/pkg/container/docker_network.go @@ -0,0 +1,38 @@ +package container + +import ( + "context" + + "github.com/docker/docker/api/types" + "github.com/nektos/act/pkg/common" +) + +func NewDockerNetworkCreateExecutor(name string) common.Executor { + return func(ctx context.Context) error { + cli, err := GetDockerClient(ctx) + if err != nil { + return err + } + + _, err = cli.NetworkCreate(ctx, name, types.NetworkCreate{ + Driver: "bridge", + Scope: "local", + }) + if err != nil { + return err + } + + return nil + } +} + +func NewDockerNetworkRemoveExecutor(name string) common.Executor { + return func(ctx context.Context) error { + cli, err := GetDockerClient(ctx) + if err != nil { + return err + } + + return cli.NetworkRemove(ctx, name) + } +} diff --git a/pkg/container/docker_run.go b/pkg/container/docker_run.go index cf58aee902d..6957d5769ac 100644 --- a/pkg/container/docker_run.go +++ b/pkg/container/docker_run.go @@ -16,6 +16,8 @@ import ( "strconv" "strings" + networktypes "github.com/docker/docker/api/types/network" + "github.com/go-git/go-billy/v5/helper/polyfill" "github.com/go-git/go-billy/v5/osfs" "github.com/go-git/go-git/v5/plumbing/format/gitignore" @@ -46,6 +48,25 @@ func NewContainer(input *NewContainerInput) ExecutionsEnvironment { return cr } +func (cr *containerReference) ConnectToNetwork(name string) common.Executor { + return common. + NewDebugExecutor("%sdocker network connect %s %s", logPrefix, name, cr.input.Name). + Then( + common.NewPipelineExecutor( + cr.connect(), + cr.connectToNetwork(name, cr.input.NetworkAliases), + ).IfNot(common.Dryrun), + ) +} + +func (cr *containerReference) connectToNetwork(name string, aliases []string) common.Executor { + return func(ctx context.Context) error { + return cr.cli.NetworkConnect(ctx, name, cr.input.Name, &networktypes.EndpointSettings{ + Aliases: aliases, + }) + } +} + // supportsContainerImagePlatform returns true if the underlying Docker server // API version is 1.41 and beyond func supportsContainerImagePlatform(ctx context.Context, cli client.APIClient) bool { diff --git a/pkg/container/host_environment.go b/pkg/container/host_environment.go index 91dae4c57fe..a131f81418b 100644 --- a/pkg/container/host_environment.go +++ b/pkg/container/host_environment.go @@ -40,6 +40,12 @@ func (e *HostEnvironment) Create(_ []string, _ []string) common.Executor { } } +func (e *HostEnvironment) ConnectToNetwork(name string) common.Executor { + return func(ctx context.Context) error { + return nil + } +} + func (e *HostEnvironment) Close() common.Executor { return func(ctx context.Context) error { return nil diff --git a/pkg/runner/job_executor.go b/pkg/runner/job_executor.go index 3f2e41e2b9f..4686b122ec7 100644 --- a/pkg/runner/job_executor.go +++ b/pkg/runner/job_executor.go @@ -102,6 +102,21 @@ func newJobExecutor(info jobInfo, sf stepFactory, rc *RunContext) common.Executo ctx, cancel := context.WithTimeout(common.WithLogger(context.Background(), common.Logger(ctx)), time.Minute) defer cancel() err = info.stopContainer()(ctx) //nolint:contextcheck + + logger := common.Logger(ctx) + logger.Infof("Cleaning up services for job %s", rc.JobName) + if err := rc.stopServiceContainers()(ctx); err != nil { + logger.Errorf("Error while cleaning services: %v", err) + } + + logger.Infof("Cleaning up container for job %s", rc.JobName) + err = info.stopContainer()(ctx) + + logger.Infof("Cleaning up network for job %s", rc.JobName) + networkName := fmt.Sprintf("%s-network", rc.jobContainerName()) + if err := rc.removeNetwork(networkName)(ctx); err != nil { + logger.Errorf("Error while cleaning network: %v", err) + } } setJobResult(ctx, info, rc, jobError == nil) setJobOutputs(ctx, rc) diff --git a/pkg/runner/run_context.go b/pkg/runner/run_context.go index 37f4767190d..624ee8ec0d6 100644 --- a/pkg/runner/run_context.go +++ b/pkg/runner/run_context.go @@ -17,6 +17,7 @@ import ( "runtime" "strings" + "github.com/docker/docker/errdefs" "github.com/opencontainers/selinux/go-selinux" "github.com/nektos/act/pkg/common" @@ -40,6 +41,7 @@ type RunContext struct { IntraActionState map[string]map[string]string ExprEval ExpressionEvaluator JobContainer container.ExecutionsEnvironment + ServiceContainers []container.ExecutionsEnvironment OutputMappings map[MappableOutput]MappableOutput JobName string ActionPath string @@ -259,6 +261,37 @@ func (rc *RunContext) startJobContainer() common.Executor { ext := container.LinuxContainerEnvironmentExtensions{} binds, mounts := rc.GetBindsAndMounts() + // add service containers + for name, spec := range rc.Run.Job().Services { + mergedEnv := envList + for k, v := range spec.Env { + mergedEnv = append(mergedEnv, fmt.Sprintf("%s=%s", k, v)) + } + serviceContainerName := createContainerName(rc.jobContainerName(), name) + c := container.NewContainer(&container.NewContainerInput{ + Name: serviceContainerName, + WorkingDir: ext.ToContainerPath(rc.Config.Workdir), + Image: spec.Image, + Username: username, + Password: password, + Env: mergedEnv, + Mounts: map[string]string{ + // TODO merge volumes + name: ext.ToContainerPath(rc.Config.Workdir), + "act-toolcache": "/toolcache", + "act-actions": "/actions", + }, + Binds: binds, + Stdout: logWriter, + Stderr: logWriter, + Privileged: rc.Config.Privileged, + UsernsMode: rc.Config.UsernsMode, + Platform: rc.Config.ContainerArchitecture, + NetworkAliases: []string{name}, + }) + rc.ServiceContainers = append(rc.ServiceContainers, c) + } + rc.cleanUpJobContainer = func(ctx context.Context) error { if rc.JobContainer != nil && !rc.Config.ReuseContainers { return rc.JobContainer.Remove(). @@ -291,11 +324,24 @@ func (rc *RunContext) startJobContainer() common.Executor { return errors.New("Failed to create job container") } + networkName := fmt.Sprintf("%s-network", rc.jobContainerName()) return common.NewPipelineExecutor( + rc.pullServicesImages(rc.Config.ForcePull), rc.JobContainer.Pull(rc.Config.ForcePull), + rc.stopServiceContainers(), rc.stopJobContainer(), + func(ctx context.Context) error { + err := rc.removeNetwork(networkName)(ctx) + if errdefs.IsNotFound(err) { + return nil + } + return err + }, + rc.createNetwork(networkName), + rc.startServiceContainers(networkName), rc.JobContainer.Create(rc.Config.ContainerCapAdd, rc.Config.ContainerCapDrop), rc.JobContainer.Start(false), + rc.JobContainer.ConnectToNetwork(networkName), rc.JobContainer.Copy(rc.JobContainer.GetActPath()+"/", &container.FileEntry{ Name: "workflow/event.json", Mode: 0o644, @@ -309,6 +355,18 @@ func (rc *RunContext) startJobContainer() common.Executor { } } +func (rc *RunContext) createNetwork(name string) common.Executor { + return func(ctx context.Context) error { + return container.NewDockerNetworkCreateExecutor(name)(ctx) + } +} + +func (rc *RunContext) removeNetwork(name string) common.Executor { + return func(ctx context.Context) error { + return container.NewDockerNetworkRemoveExecutor(name)(ctx) + } +} + func (rc *RunContext) execJobContainer(cmd []string, env map[string]string, user, workdir string) common.Executor { return func(ctx context.Context) error { return rc.JobContainer.Exec(cmd, env, user, workdir)(ctx) @@ -379,6 +437,41 @@ func (rc *RunContext) stopJobContainer() common.Executor { } } +func (rc *RunContext) pullServicesImages(forcePull bool) common.Executor { + return func(ctx context.Context) error { + execs := []common.Executor{} + for _, c := range rc.ServiceContainers { + execs = append(execs, c.Pull(forcePull)) + } + return common.NewParallelExecutor(len(execs), execs...)(ctx) + } +} + +func (rc *RunContext) startServiceContainers(networkName string) common.Executor { + return func(ctx context.Context) error { + execs := []common.Executor{} + for _, c := range rc.ServiceContainers { + execs = append(execs, common.NewPipelineExecutor( + c.Pull(false), + c.Create(rc.Config.ContainerCapAdd, rc.Config.ContainerCapDrop), + c.Start(false), + c.ConnectToNetwork(networkName), + )) + } + return common.NewParallelExecutor(len(execs), execs...)(ctx) + } +} + +func (rc *RunContext) stopServiceContainers() common.Executor { + return func(ctx context.Context) error { + execs := []common.Executor{} + for _, c := range rc.ServiceContainers { + execs = append(execs, c.Remove()) + } + return common.NewParallelExecutor(len(execs), execs...)(ctx) + } +} + // Prepare the mounts and binds for the worker // ActionCacheDir is for rc diff --git a/pkg/runner/runner_test.go b/pkg/runner/runner_test.go index 4034e96e34c..b1de915703e 100644 --- a/pkg/runner/runner_test.go +++ b/pkg/runner/runner_test.go @@ -548,6 +548,43 @@ func TestRunEventSecrets(t *testing.T) { tjfi.runTest(context.Background(), t, &Config{Secrets: secrets, Env: env}) } +func TestRunWithService(t *testing.T) { + if testing.Short() { + t.Skip("skipping integration test") + } + + log.SetLevel(log.DebugLevel) + ctx := context.Background() + + platforms := map[string]string{ + "ubuntu-latest": "node:12.20.1-buster-slim", + } + + workflowPath := "services" + eventName := "push" + + workdir, err := filepath.Abs("testdata") + assert.NoError(t, err, workflowPath) + + runnerConfig := &Config{ + Workdir: workdir, + EventName: eventName, + Platforms: platforms, + ReuseContainers: false, + } + runner, err := New(runnerConfig) + assert.NoError(t, err, workflowPath) + + planner, err := model.NewWorkflowPlanner(fmt.Sprintf("testdata/%s", workflowPath), true) + assert.NoError(t, err, workflowPath) + + plan, err := planner.PlanEvent(eventName) + assert.NoError(t, err, workflowPath) + + err = runner.NewPlanExecutor(plan)(ctx) + assert.NoError(t, err, workflowPath) +} + func TestRunActionInputs(t *testing.T) { if testing.Short() { t.Skip("skipping integration test") diff --git a/pkg/runner/testdata/services/push.yaml b/pkg/runner/testdata/services/push.yaml new file mode 100644 index 00000000000..f6ca7bc4c50 --- /dev/null +++ b/pkg/runner/testdata/services/push.yaml @@ -0,0 +1,26 @@ +name: services +on: push +jobs: + services: + name: Reproduction of failing Services interpolation + runs-on: ubuntu-latest + services: + postgres: + image: postgres:12 + env: + POSTGRES_USER: runner + POSTGRES_PASSWORD: mysecretdbpass + POSTGRES_DB: mydb + options: >- + --health-cmd pg_isready + --health-interval 10s + --health-timeout 5s + --health-retries 5 + ports: + - 5432:5432 + steps: + - name: Echo the Postgres service ID / Network / Ports + run: | + echo "id: ${{ job.services.postgres.id }}" + echo "network: ${{ job.services.postgres.network }}" + echo "ports: ${{ job.services.postgres.ports }}" From 5cc4bd063ed631dde114ec08aae11260ec64ec14 Mon Sep 17 00:00:00 2001 From: Zettat123 Date: Wed, 19 Apr 2023 21:53:57 +0800 Subject: [PATCH 02/31] Support services options (#45) Reviewed-on: https://gitea.com/gitea/act/pulls/45 Reviewed-by: Lunny Xiao Co-authored-by: Zettat123 Co-committed-by: Zettat123 --- pkg/runner/run_context.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/runner/run_context.go b/pkg/runner/run_context.go index 624ee8ec0d6..ca5291e6dda 100644 --- a/pkg/runner/run_context.go +++ b/pkg/runner/run_context.go @@ -287,6 +287,7 @@ func (rc *RunContext) startJobContainer() common.Executor { Privileged: rc.Config.Privileged, UsernsMode: rc.Config.UsernsMode, Platform: rc.Config.ContainerArchitecture, + Options: spec.Options, NetworkAliases: []string{name}, }) rc.ServiceContainers = append(rc.ServiceContainers, c) From ca623cef53706c7e048ebba2e12f8119dc37a5b9 Mon Sep 17 00:00:00 2001 From: Zettat123 Date: Thu, 20 Apr 2023 16:24:31 +0800 Subject: [PATCH 03/31] Support intepolation for `env` of `services` (#47) Reviewed-on: https://gitea.com/gitea/act/pulls/47 Reviewed-by: Lunny Xiao Co-authored-by: Zettat123 Co-committed-by: Zettat123 --- pkg/runner/run_context.go | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/pkg/runner/run_context.go b/pkg/runner/run_context.go index ca5291e6dda..5cf7dca6b57 100644 --- a/pkg/runner/run_context.go +++ b/pkg/runner/run_context.go @@ -263,9 +263,13 @@ func (rc *RunContext) startJobContainer() common.Executor { // add service containers for name, spec := range rc.Run.Job().Services { - mergedEnv := envList + interpolatedEnvs := make(map[string]string, len(spec.Env)) for k, v := range spec.Env { - mergedEnv = append(mergedEnv, fmt.Sprintf("%s=%s", k, v)) + interpolatedEnvs[k] = rc.ExprEval.Interpolate(ctx, v) + } + envs := make([]string, 0, len(interpolatedEnvs)) + for k, v := range interpolatedEnvs { + envs = append(envs, fmt.Sprintf("%s=%s", k, v)) } serviceContainerName := createContainerName(rc.jobContainerName(), name) c := container.NewContainer(&container.NewContainerInput{ @@ -274,7 +278,7 @@ func (rc *RunContext) startJobContainer() common.Executor { Image: spec.Image, Username: username, Password: password, - Env: mergedEnv, + Env: envs, Mounts: map[string]string{ // TODO merge volumes name: ext.ToContainerPath(rc.Config.Workdir), From 7a09a4835d85cf902d9418ad92adf74a041a215b Mon Sep 17 00:00:00 2001 From: Zettat123 Date: Tue, 25 Apr 2023 14:45:39 +0800 Subject: [PATCH 04/31] Support services `credentials` (#51) If a service's image is from a container registry requires authentication, `act_runner` will need `credentials` to pull the image, see [documentation](https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#jobsjob_idservicesservice_idcredentials). Currently, `act_runner` incorrectly uses the `credentials` of `containers` to pull services' images and the `credentials` of services won't be used, see the related code: https://gitea.com/gitea/act/src/commit/0c1f2edb996a87ee17dcf3cfa7259c04be02abd7/pkg/runner/run_context.go#L228-L269 Co-authored-by: Jason Song Reviewed-on: https://gitea.com/gitea/act/pulls/51 Reviewed-by: Jason Song Reviewed-by: Lunny Xiao Co-authored-by: Zettat123 Co-committed-by: Zettat123 --- pkg/runner/run_context.go | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/pkg/runner/run_context.go b/pkg/runner/run_context.go index 5cf7dca6b57..df673236de0 100644 --- a/pkg/runner/run_context.go +++ b/pkg/runner/run_context.go @@ -271,6 +271,10 @@ func (rc *RunContext) startJobContainer() common.Executor { for k, v := range interpolatedEnvs { envs = append(envs, fmt.Sprintf("%s=%s", k, v)) } + username, password, err = rc.handleServiceCredentials(ctx, spec.Credentials) + if err != nil { + return fmt.Errorf("failed to handle service %s credentials: %w", name, err) + } serviceContainerName := createContainerName(rc.jobContainerName(), name) c := container.NewContainer(&container.NewContainerInput{ Name: serviceContainerName, @@ -949,3 +953,26 @@ func (rc *RunContext) handleCredentials(ctx context.Context) (string, string, er return username, password, nil } + +func (rc *RunContext) handleServiceCredentials(ctx context.Context, creds map[string]string) (username, password string, err error) { + if creds == nil { + return + } + if len(creds) != 2 { + err = fmt.Errorf("invalid property count for key 'credentials:'") + return + } + + ee := rc.NewExpressionEvaluator(ctx) + if username = ee.Interpolate(ctx, creds["username"]); username == "" { + err = fmt.Errorf("failed to interpolate credentials.username") + return + } + + if password = ee.Interpolate(ctx, creds["password"]); password == "" { + err = fmt.Errorf("failed to interpolate credentials.password") + return + } + + return +} From 92f9cc7a969b112cbbaef95db0ad176ba2a249bf Mon Sep 17 00:00:00 2001 From: Sam Foo Date: Sun, 6 Aug 2023 17:13:52 -0700 Subject: [PATCH 05/31] Add ContainerMaxLifetime and ContainerNetworkMode options from: https://gitea.com/gitea/act/commit/b9c20dcaa43899cb3bb327619d447248303170e0 --- pkg/runner/run_context.go | 5 +++-- pkg/runner/runner.go | 3 +++ 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/pkg/runner/run_context.go b/pkg/runner/run_context.go index df673236de0..4a9b5e58a67 100644 --- a/pkg/runner/run_context.go +++ b/pkg/runner/run_context.go @@ -16,6 +16,7 @@ import ( "regexp" "runtime" "strings" + "time" "github.com/docker/docker/errdefs" "github.com/opencontainers/selinux/go-selinux" @@ -312,7 +313,7 @@ func (rc *RunContext) startJobContainer() common.Executor { rc.JobContainer = container.NewContainer(&container.NewContainerInput{ Cmd: nil, - Entrypoint: []string{"tail", "-f", "/dev/null"}, + Entrypoint: []string{"/bin/sleep", fmt.Sprint(rc.Config.ContainerMaxLifetime.Round(time.Second).Seconds())}, WorkingDir: ext.ToContainerPath(rc.Config.Workdir), Image: image, Username: username, @@ -320,7 +321,7 @@ func (rc *RunContext) startJobContainer() common.Executor { Name: name, Env: envList, Mounts: mounts, - NetworkMode: "host", + NetworkMode: rc.Config.ContainerNetworkMode, Binds: binds, Stdout: logWriter, Stderr: logWriter, diff --git a/pkg/runner/runner.go b/pkg/runner/runner.go index 09c17319e0d..5daf861ba02 100644 --- a/pkg/runner/runner.go +++ b/pkg/runner/runner.go @@ -6,6 +6,7 @@ import ( "fmt" "os" "runtime" + "time" log "github.com/sirupsen/logrus" @@ -58,6 +59,8 @@ type Config struct { ReplaceGheActionWithGithubCom []string // Use actions from GitHub Enterprise instance to GitHub ReplaceGheActionTokenWithGithubCom string // Token of private action repo on GitHub. Matrix map[string]map[string]bool // Matrix config to run + ContainerMaxLifetime time.Duration // the max lifetime of job containers + ContainerNetworkMode string // the network mode of job containers } type caller struct { From bcfbd2858320640750ae16a82d7a33641151da2d Mon Sep 17 00:00:00 2001 From: sillyguodong Date: Tue, 16 May 2023 14:03:55 +0800 Subject: [PATCH 06/31] Fix container network issue (#56) Follow: https://gitea.com/gitea/act_runner/pulls/184 Close https://gitea.com/gitea/act_runner/issues/177 - `act` create new networks only if the value of `NeedCreateNetwork` is true, and remove these networks at last. `NeedCreateNetwork` is passed by `act_runner`. 'NeedCreateNetwork' is true only if `container.network` in the configuration file of the `act_runner` is empty. - In the `docker create` phase, specify the network to which containers will connect. Because, if not specify , container will connect to `bridge` network which is created automatically by Docker. - If the network is user defined network ( the value of `container.network` is empty or ``. Because, the network created by `act` is also user defined network.), will also specify alias by `--network-alias`. The alias of service is ``. So we can be access service container by `:` in the steps of job. - Won't try to `docker network connect ` network after `docker start` any more. - Because on the one hand, `docker network connect` applies only to user defined networks, if try to `docker network connect host ` will return error. - On the other hand, we just specify network in the stage of `docker create`, the same effect can be achieved. - Won't try to remove containers and networks berfore the stage of `docker start`, because the name of these containers and netwoks won't be repeat. Co-authored-by: Jason Song Reviewed-on: https://gitea.com/gitea/act/pulls/56 Reviewed-by: Jason Song Co-authored-by: sillyguodong Co-committed-by: sillyguodong --- pkg/runner/job_executor.go | 17 +++++--- pkg/runner/run_context.go | 76 ++++++++++++++++++----------------- pkg/runner/runner.go | 81 +++++++++++++++++++------------------- 3 files changed, 92 insertions(+), 82 deletions(-) diff --git a/pkg/runner/job_executor.go b/pkg/runner/job_executor.go index 4686b122ec7..4d0aee432ae 100644 --- a/pkg/runner/job_executor.go +++ b/pkg/runner/job_executor.go @@ -110,12 +110,17 @@ func newJobExecutor(info jobInfo, sf stepFactory, rc *RunContext) common.Executo } logger.Infof("Cleaning up container for job %s", rc.JobName) - err = info.stopContainer()(ctx) - - logger.Infof("Cleaning up network for job %s", rc.JobName) - networkName := fmt.Sprintf("%s-network", rc.jobContainerName()) - if err := rc.removeNetwork(networkName)(ctx); err != nil { - logger.Errorf("Error while cleaning network: %v", err) + if err = info.stopContainer()(ctx); err != nil { + logger.Errorf("Error while stop job container: %v", err) + } + if rc.Config.ContainerNetworkMode == "" { + // if the value of `ContainerNetworkMode` is empty string, + // it means that the network to which containers are connecting is created by `act_runner`, + // so, we should remove the network at last. + logger.Infof("Cleaning up network for job %s, and network name is: %s", rc.JobName, rc.networkName()) + if err := rc.removeNetwork(rc.networkName())(ctx); err != nil { + logger.Errorf("Error while cleaning network: %v", err) + } } } setJobResult(ctx, info, rc, jobError == nil) diff --git a/pkg/runner/run_context.go b/pkg/runner/run_context.go index 4a9b5e58a67..b427e822c1d 100644 --- a/pkg/runner/run_context.go +++ b/pkg/runner/run_context.go @@ -18,7 +18,6 @@ import ( "strings" "time" - "github.com/docker/docker/errdefs" "github.com/opencontainers/selinux/go-selinux" "github.com/nektos/act/pkg/common" @@ -90,6 +89,12 @@ func (rc *RunContext) jobContainerName() string { return createContainerName("act", rc.String()) } +// networkName return the name of the network which will be created by `act` automatically for job, +// only create network if `rc.Config.ContainerNetworkMode` is empty string. +func (rc *RunContext) networkName() string { + return fmt.Sprintf("%s-network", rc.jobContainerName()) +} + func getDockerDaemonSocketMountPath(daemonPath string) string { if protoIndex := strings.Index(daemonPath, "://"); protoIndex != -1 { scheme := daemonPath[:protoIndex] @@ -262,8 +267,17 @@ func (rc *RunContext) startJobContainer() common.Executor { ext := container.LinuxContainerEnvironmentExtensions{} binds, mounts := rc.GetBindsAndMounts() + // specify the network to which the container will connect when `docker create` stage. (like execute command line: docker create --network ) + networkName := string(rc.Config.ContainerNetworkMode) + if networkName == "" { + // if networkName is empty string, will create a new network for the containers. + // and it will be removed after at last. + networkName = rc.networkName() + } + // add service containers - for name, spec := range rc.Run.Job().Services { + for serviceId, spec := range rc.Run.Job().Services { + // interpolate env interpolatedEnvs := make(map[string]string, len(spec.Env)) for k, v := range spec.Env { interpolatedEnvs[k] = rc.ExprEval.Interpolate(ctx, v) @@ -274,9 +288,9 @@ func (rc *RunContext) startJobContainer() common.Executor { } username, password, err = rc.handleServiceCredentials(ctx, spec.Credentials) if err != nil { - return fmt.Errorf("failed to handle service %s credentials: %w", name, err) + return fmt.Errorf("failed to handle service %s credentials: %w", serviceId, err) } - serviceContainerName := createContainerName(rc.jobContainerName(), name) + serviceContainerName := createContainerName(rc.jobContainerName(), serviceId) c := container.NewContainer(&container.NewContainerInput{ Name: serviceContainerName, WorkingDir: ext.ToContainerPath(rc.Config.Workdir), @@ -286,7 +300,7 @@ func (rc *RunContext) startJobContainer() common.Executor { Env: envs, Mounts: map[string]string{ // TODO merge volumes - name: ext.ToContainerPath(rc.Config.Workdir), + serviceId: ext.ToContainerPath(rc.Config.Workdir), "act-toolcache": "/toolcache", "act-actions": "/actions", }, @@ -297,7 +311,8 @@ func (rc *RunContext) startJobContainer() common.Executor { UsernsMode: rc.Config.UsernsMode, Platform: rc.Config.ContainerArchitecture, Options: spec.Options, - NetworkAliases: []string{name}, + NetworkMode: networkName, + NetworkAliases: []string{serviceId}, }) rc.ServiceContainers = append(rc.ServiceContainers, c) } @@ -312,46 +327,36 @@ func (rc *RunContext) startJobContainer() common.Executor { } rc.JobContainer = container.NewContainer(&container.NewContainerInput{ - Cmd: nil, - Entrypoint: []string{"/bin/sleep", fmt.Sprint(rc.Config.ContainerMaxLifetime.Round(time.Second).Seconds())}, - WorkingDir: ext.ToContainerPath(rc.Config.Workdir), - Image: image, - Username: username, - Password: password, - Name: name, - Env: envList, - Mounts: mounts, - NetworkMode: rc.Config.ContainerNetworkMode, - Binds: binds, - Stdout: logWriter, - Stderr: logWriter, - Privileged: rc.Config.Privileged, - UsernsMode: rc.Config.UsernsMode, - Platform: rc.Config.ContainerArchitecture, - Options: rc.options(ctx), + Cmd: nil, + Entrypoint: []string{"/bin/sleep", fmt.Sprint(rc.Config.ContainerMaxLifetime.Round(time.Second).Seconds())}, + WorkingDir: ext.ToContainerPath(rc.Config.Workdir), + Image: image, + Username: username, + Password: password, + Name: name, + Env: envList, + Mounts: mounts, + NetworkMode: networkName, + NetworkAliases: []string{rc.Name}, + Binds: binds, + Stdout: logWriter, + Stderr: logWriter, + Privileged: rc.Config.Privileged, + UsernsMode: rc.Config.UsernsMode, + Platform: rc.Config.ContainerArchitecture, + Options: rc.options(ctx), }) if rc.JobContainer == nil { return errors.New("Failed to create job container") } - networkName := fmt.Sprintf("%s-network", rc.jobContainerName()) return common.NewPipelineExecutor( rc.pullServicesImages(rc.Config.ForcePull), rc.JobContainer.Pull(rc.Config.ForcePull), - rc.stopServiceContainers(), - rc.stopJobContainer(), - func(ctx context.Context) error { - err := rc.removeNetwork(networkName)(ctx) - if errdefs.IsNotFound(err) { - return nil - } - return err - }, - rc.createNetwork(networkName), + rc.createNetwork(networkName).IfBool(rc.Config.ContainerNetworkMode == ""), // if the value of `ContainerNetworkMode` is empty string, then will create a new network for containers. rc.startServiceContainers(networkName), rc.JobContainer.Create(rc.Config.ContainerCapAdd, rc.Config.ContainerCapDrop), rc.JobContainer.Start(false), - rc.JobContainer.ConnectToNetwork(networkName), rc.JobContainer.Copy(rc.JobContainer.GetActPath()+"/", &container.FileEntry{ Name: "workflow/event.json", Mode: 0o644, @@ -465,7 +470,6 @@ func (rc *RunContext) startServiceContainers(networkName string) common.Executor c.Pull(false), c.Create(rc.Config.ContainerCapAdd, rc.Config.ContainerCapDrop), c.Start(false), - c.ConnectToNetwork(networkName), )) } return common.NewParallelExecutor(len(execs), execs...)(ctx) diff --git a/pkg/runner/runner.go b/pkg/runner/runner.go index 5daf861ba02..c63f6b866d9 100644 --- a/pkg/runner/runner.go +++ b/pkg/runner/runner.go @@ -10,6 +10,7 @@ import ( log "github.com/sirupsen/logrus" + docker_container "github.com/docker/docker/api/types/container" "github.com/nektos/act/pkg/common" "github.com/nektos/act/pkg/model" ) @@ -21,46 +22,46 @@ type Runner interface { // Config contains the config for a new runner type Config struct { - Actor string // the user that triggered the event - Workdir string // path to working directory - ActionCacheDir string // path used for caching action contents - BindWorkdir bool // bind the workdir to the job container - EventName string // name of event to run - EventPath string // path to JSON file to use for event.json in containers - DefaultBranch string // name of the main branch for this repository - ReuseContainers bool // reuse containers to maintain state - ForcePull bool // force pulling of the image, even if already present - ForceRebuild bool // force rebuilding local docker image action - LogOutput bool // log the output from docker run - JSONLogger bool // use json or text logger - LogPrefixJobID bool // switches from the full job name to the job id - Env map[string]string // env for containers - Inputs map[string]string // manually passed action inputs - Secrets map[string]string // list of secrets - Vars map[string]string // list of vars - Token string // GitHub token - InsecureSecrets bool // switch hiding output when printing to terminal - Platforms map[string]string // list of platforms - Privileged bool // use privileged mode - UsernsMode string // user namespace to use - ContainerArchitecture string // Desired OS/architecture platform for running containers - ContainerDaemonSocket string // Path to Docker daemon socket - ContainerOptions string // Options for the job container - UseGitIgnore bool // controls if paths in .gitignore should not be copied into container, default true - GitHubInstance string // GitHub instance to use, default "github.com" - ContainerCapAdd []string // list of kernel capabilities to add to the containers - ContainerCapDrop []string // list of kernel capabilities to remove from the containers - AutoRemove bool // controls if the container is automatically removed upon workflow completion - ArtifactServerPath string // the path where the artifact server stores uploads - ArtifactServerAddr string // the address the artifact server binds to - ArtifactServerPort string // the port the artifact server binds to - NoSkipCheckout bool // do not skip actions/checkout - RemoteName string // remote name in local git repo config - ReplaceGheActionWithGithubCom []string // Use actions from GitHub Enterprise instance to GitHub - ReplaceGheActionTokenWithGithubCom string // Token of private action repo on GitHub. - Matrix map[string]map[string]bool // Matrix config to run - ContainerMaxLifetime time.Duration // the max lifetime of job containers - ContainerNetworkMode string // the network mode of job containers + Actor string // the user that triggered the event + Workdir string // path to working directory + ActionCacheDir string // path used for caching action contents + BindWorkdir bool // bind the workdir to the job container + EventName string // name of event to run + EventPath string // path to JSON file to use for event.json in containers + DefaultBranch string // name of the main branch for this repository + ReuseContainers bool // reuse containers to maintain state + ForcePull bool // force pulling of the image, even if already present + ForceRebuild bool // force rebuilding local docker image action + LogOutput bool // log the output from docker run + JSONLogger bool // use json or text logger + LogPrefixJobID bool // switches from the full job name to the job id + Env map[string]string // env for containers + Inputs map[string]string // manually passed action inputs + Secrets map[string]string // list of secrets + Vars map[string]string // list of vars + Token string // GitHub token + InsecureSecrets bool // switch hiding output when printing to terminal + Platforms map[string]string // list of platforms + Privileged bool // use privileged mode + UsernsMode string // user namespace to use + ContainerArchitecture string // Desired OS/architecture platform for running containers + ContainerDaemonSocket string // Path to Docker daemon socket + ContainerOptions string // Options for the job container + UseGitIgnore bool // controls if paths in .gitignore should not be copied into container, default true + GitHubInstance string // GitHub instance to use, default "github.com" + ContainerCapAdd []string // list of kernel capabilities to add to the containers + ContainerCapDrop []string // list of kernel capabilities to remove from the containers + AutoRemove bool // controls if the container is automatically removed upon workflow completion + ArtifactServerPath string // the path where the artifact server stores uploads + ArtifactServerAddr string // the address the artifact server binds to + ArtifactServerPort string // the port the artifact server binds to + NoSkipCheckout bool // do not skip actions/checkout + RemoteName string // remote name in local git repo config + ReplaceGheActionWithGithubCom []string // Use actions from GitHub Enterprise instance to GitHub + ReplaceGheActionTokenWithGithubCom string // Token of private action repo on GitHub. + Matrix map[string]map[string]bool // Matrix config to run + ContainerMaxLifetime time.Duration // the max lifetime of job containers + ContainerNetworkMode docker_container.NetworkMode // the network mode of job containers (the value of --network) } type caller struct { From a168f8f07ad610dbaa66c812454bb218af7f8a90 Mon Sep 17 00:00:00 2001 From: Zettat123 Date: Mon, 5 Jun 2023 09:21:59 +0000 Subject: [PATCH 07/31] Check volumes (#60) This PR adds a `ValidVolumes` config. Users can specify the volumes (including bind mounts) that can be mounted to containers by this config. Options related to volumes: - [jobs..container.volumes](https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#jobsjob_idcontainervolumes) - [jobs..services..volumes](https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#jobsjob_idservicesservice_idvolumes) In addition, volumes specified by `options` will also be checked. Currently, the following default volumes (see https://gitea.com/gitea/act/src/commit/a72822b3f83d3e68ffc697101b713b7badf57e2f/pkg/runner/run_context.go#L116-L166) will be added to `ValidVolumes`: - `act-toolcache` - `` and `-env` - `/var/run/docker.sock` (We need to add a new configuration to control whether the docker daemon can be mounted) Co-authored-by: Jason Song Reviewed-on: https://gitea.com/gitea/act/pulls/60 Reviewed-by: Jason Song Co-authored-by: Zettat123 Co-committed-by: Zettat123 --- pkg/container/container_types.go | 1 + pkg/runner/run_context.go | 51 ++++++++++++++++++++++++-------- pkg/runner/runner.go | 1 + 3 files changed, 40 insertions(+), 13 deletions(-) diff --git a/pkg/container/container_types.go b/pkg/container/container_types.go index 0036b9a8a5f..317a5150bf4 100644 --- a/pkg/container/container_types.go +++ b/pkg/container/container_types.go @@ -27,6 +27,7 @@ type NewContainerInput struct { Platform string Options string NetworkAliases []string + ValidVolumes []string } // FileEntry is a file to copy to a container diff --git a/pkg/runner/run_context.go b/pkg/runner/run_context.go index b427e822c1d..03e6326e0e2 100644 --- a/pkg/runner/run_context.go +++ b/pkg/runner/run_context.go @@ -290,21 +290,17 @@ func (rc *RunContext) startJobContainer() common.Executor { if err != nil { return fmt.Errorf("failed to handle service %s credentials: %w", serviceId, err) } + serviceBinds, serviceMounts := rc.GetServiceBindsAndMounts(spec.Volumes) serviceContainerName := createContainerName(rc.jobContainerName(), serviceId) c := container.NewContainer(&container.NewContainerInput{ - Name: serviceContainerName, - WorkingDir: ext.ToContainerPath(rc.Config.Workdir), - Image: spec.Image, - Username: username, - Password: password, - Env: envs, - Mounts: map[string]string{ - // TODO merge volumes - serviceId: ext.ToContainerPath(rc.Config.Workdir), - "act-toolcache": "/toolcache", - "act-actions": "/actions", - }, - Binds: binds, + Name: serviceContainerName, + WorkingDir: ext.ToContainerPath(rc.Config.Workdir), + Image: spec.Image, + Username: username, + Password: password, + Env: envs, + Mounts: serviceMounts, + Binds: serviceBinds, Stdout: logWriter, Stderr: logWriter, Privileged: rc.Config.Privileged, @@ -313,6 +309,7 @@ func (rc *RunContext) startJobContainer() common.Executor { Options: spec.Options, NetworkMode: networkName, NetworkAliases: []string{serviceId}, + ValidVolumes: rc.Config.ValidVolumes, }) rc.ServiceContainers = append(rc.ServiceContainers, c) } @@ -345,6 +342,7 @@ func (rc *RunContext) startJobContainer() common.Executor { UsernsMode: rc.Config.UsernsMode, Platform: rc.Config.ContainerArchitecture, Options: rc.options(ctx), + ValidVolumes: rc.Config.ValidVolumes, }) if rc.JobContainer == nil { return errors.New("Failed to create job container") @@ -981,3 +979,30 @@ func (rc *RunContext) handleServiceCredentials(ctx context.Context, creds map[st return } + +// GetServiceBindsAndMounts returns the binds and mounts for the service container, resolving paths as appopriate +func (rc *RunContext) GetServiceBindsAndMounts(svcVolumes []string) ([]string, map[string]string) { + if rc.Config.ContainerDaemonSocket == "" { + rc.Config.ContainerDaemonSocket = "/var/run/docker.sock" + } + binds := []string{} + if rc.Config.ContainerDaemonSocket != "-" { + daemonPath := getDockerDaemonSocketMountPath(rc.Config.ContainerDaemonSocket) + binds = append(binds, fmt.Sprintf("%s:%s", daemonPath, "/var/run/docker.sock")) + } + + mounts := map[string]string{} + + for _, v := range svcVolumes { + if !strings.Contains(v, ":") || filepath.IsAbs(v) { + // Bind anonymous volume or host file. + binds = append(binds, v) + } else { + // Mount existing volume. + paths := strings.SplitN(v, ":", 2) + mounts[paths[0]] = paths[1] + } + } + + return binds, mounts +} diff --git a/pkg/runner/runner.go b/pkg/runner/runner.go index c63f6b866d9..2271ffce2f8 100644 --- a/pkg/runner/runner.go +++ b/pkg/runner/runner.go @@ -62,6 +62,7 @@ type Config struct { Matrix map[string]map[string]bool // Matrix config to run ContainerMaxLifetime time.Duration // the max lifetime of job containers ContainerNetworkMode docker_container.NetworkMode // the network mode of job containers (the value of --network) + ValidVolumes []string // only volumes (and bind mounts) in this slice can be mounted on the job container or service containers } type caller struct { From 78e461384b687842212d0c5c7af095b35b69757b Mon Sep 17 00:00:00 2001 From: Sam Foo Date: Sun, 6 Aug 2023 18:34:56 -0700 Subject: [PATCH 08/31] Remove ContainerMaxLifetime; fix lint --- pkg/container/host_environment.go | 2 +- pkg/runner/job_executor.go | 4 ++-- pkg/runner/run_context.go | 16 +++++++--------- pkg/runner/runner.go | 5 +---- 4 files changed, 11 insertions(+), 16 deletions(-) diff --git a/pkg/container/host_environment.go b/pkg/container/host_environment.go index a131f81418b..fb1393bd86c 100644 --- a/pkg/container/host_environment.go +++ b/pkg/container/host_environment.go @@ -40,7 +40,7 @@ func (e *HostEnvironment) Create(_ []string, _ []string) common.Executor { } } -func (e *HostEnvironment) ConnectToNetwork(name string) common.Executor { +func (e *HostEnvironment) ConnectToNetwork(_ string) common.Executor { return func(ctx context.Context) error { return nil } diff --git a/pkg/runner/job_executor.go b/pkg/runner/job_executor.go index 4d0aee432ae..960fe1fbbef 100644 --- a/pkg/runner/job_executor.go +++ b/pkg/runner/job_executor.go @@ -19,6 +19,7 @@ type jobInfo interface { result(result string) } +//nolint:contextcheck,gocyclo func newJobExecutor(info jobInfo, sf stepFactory, rc *RunContext) common.Executor { steps := make([]common.Executor, 0) preSteps := make([]common.Executor, 0) @@ -87,7 +88,7 @@ func newJobExecutor(info jobInfo, sf stepFactory, rc *RunContext) common.Executo postExec := useStepLogger(rc, stepModel, stepStagePost, step.post()) if postExecutor != nil { - // run the post exector in reverse order + // run the post executor in reverse order postExecutor = postExec.Finally(postExecutor) } else { postExecutor = postExec @@ -101,7 +102,6 @@ func newJobExecutor(info jobInfo, sf stepFactory, rc *RunContext) common.Executo // always allow 1 min for stopping and removing the runner, even if we were cancelled ctx, cancel := context.WithTimeout(common.WithLogger(context.Background(), common.Logger(ctx)), time.Minute) defer cancel() - err = info.stopContainer()(ctx) //nolint:contextcheck logger := common.Logger(ctx) logger.Infof("Cleaning up services for job %s", rc.JobName) diff --git a/pkg/runner/run_context.go b/pkg/runner/run_context.go index 03e6326e0e2..84ed53d66ca 100644 --- a/pkg/runner/run_context.go +++ b/pkg/runner/run_context.go @@ -16,14 +16,12 @@ import ( "regexp" "runtime" "strings" - "time" - - "github.com/opencontainers/selinux/go-selinux" "github.com/nektos/act/pkg/common" "github.com/nektos/act/pkg/container" "github.com/nektos/act/pkg/exprparser" "github.com/nektos/act/pkg/model" + "github.com/opencontainers/selinux/go-selinux" ) // RunContext contains info about current job @@ -276,7 +274,7 @@ func (rc *RunContext) startJobContainer() common.Executor { } // add service containers - for serviceId, spec := range rc.Run.Job().Services { + for serviceID, spec := range rc.Run.Job().Services { // interpolate env interpolatedEnvs := make(map[string]string, len(spec.Env)) for k, v := range spec.Env { @@ -288,10 +286,10 @@ func (rc *RunContext) startJobContainer() common.Executor { } username, password, err = rc.handleServiceCredentials(ctx, spec.Credentials) if err != nil { - return fmt.Errorf("failed to handle service %s credentials: %w", serviceId, err) + return fmt.Errorf("failed to handle service %s credentials: %w", serviceID, err) } serviceBinds, serviceMounts := rc.GetServiceBindsAndMounts(spec.Volumes) - serviceContainerName := createContainerName(rc.jobContainerName(), serviceId) + serviceContainerName := createContainerName(rc.jobContainerName(), serviceID) c := container.NewContainer(&container.NewContainerInput{ Name: serviceContainerName, WorkingDir: ext.ToContainerPath(rc.Config.Workdir), @@ -308,7 +306,7 @@ func (rc *RunContext) startJobContainer() common.Executor { Platform: rc.Config.ContainerArchitecture, Options: spec.Options, NetworkMode: networkName, - NetworkAliases: []string{serviceId}, + NetworkAliases: []string{serviceID}, ValidVolumes: rc.Config.ValidVolumes, }) rc.ServiceContainers = append(rc.ServiceContainers, c) @@ -325,7 +323,7 @@ func (rc *RunContext) startJobContainer() common.Executor { rc.JobContainer = container.NewContainer(&container.NewContainerInput{ Cmd: nil, - Entrypoint: []string{"/bin/sleep", fmt.Sprint(rc.Config.ContainerMaxLifetime.Round(time.Second).Seconds())}, + Entrypoint: []string{"/usr/bin/tail", "-f", "/dev/null"}, WorkingDir: ext.ToContainerPath(rc.Config.Workdir), Image: image, Username: username, @@ -460,7 +458,7 @@ func (rc *RunContext) pullServicesImages(forcePull bool) common.Executor { } } -func (rc *RunContext) startServiceContainers(networkName string) common.Executor { +func (rc *RunContext) startServiceContainers(_ string) common.Executor { return func(ctx context.Context) error { execs := []common.Executor{} for _, c := range rc.ServiceContainers { diff --git a/pkg/runner/runner.go b/pkg/runner/runner.go index 2271ffce2f8..75b29f5e767 100644 --- a/pkg/runner/runner.go +++ b/pkg/runner/runner.go @@ -6,13 +6,11 @@ import ( "fmt" "os" "runtime" - "time" - - log "github.com/sirupsen/logrus" docker_container "github.com/docker/docker/api/types/container" "github.com/nektos/act/pkg/common" "github.com/nektos/act/pkg/model" + log "github.com/sirupsen/logrus" ) // Runner provides capabilities to run GitHub actions @@ -60,7 +58,6 @@ type Config struct { ReplaceGheActionWithGithubCom []string // Use actions from GitHub Enterprise instance to GitHub ReplaceGheActionTokenWithGithubCom string // Token of private action repo on GitHub. Matrix map[string]map[string]bool // Matrix config to run - ContainerMaxLifetime time.Duration // the max lifetime of job containers ContainerNetworkMode docker_container.NetworkMode // the network mode of job containers (the value of --network) ValidVolumes []string // only volumes (and bind mounts) in this slice can be mounted on the job container or service containers } From 2cfb56b995227fb61f31409639ffb6b6891b8f74 Mon Sep 17 00:00:00 2001 From: Sam Foo Date: Sun, 6 Aug 2023 21:20:21 -0700 Subject: [PATCH 09/31] Remove unused ValidVolumes --- pkg/artifacts/server_test.go | 1 + pkg/container/container_types.go | 1 - pkg/runner/run_context.go | 2 -- pkg/runner/runner.go | 1 - 4 files changed, 1 insertion(+), 4 deletions(-) diff --git a/pkg/artifacts/server_test.go b/pkg/artifacts/server_test.go index aeeb0598a4d..42d0aa32bcf 100644 --- a/pkg/artifacts/server_test.go +++ b/pkg/artifacts/server_test.go @@ -292,6 +292,7 @@ func runTestJobFile(ctx context.Context, t *testing.T, tjfi TestJobFileInfo) { ArtifactServerPath: artifactsPath, ArtifactServerAddr: artifactsAddr, ArtifactServerPort: artifactsPort, + ContainerNetworkMode: "host", } runner, err := runner.New(runnerConfig) diff --git a/pkg/container/container_types.go b/pkg/container/container_types.go index 317a5150bf4..0036b9a8a5f 100644 --- a/pkg/container/container_types.go +++ b/pkg/container/container_types.go @@ -27,7 +27,6 @@ type NewContainerInput struct { Platform string Options string NetworkAliases []string - ValidVolumes []string } // FileEntry is a file to copy to a container diff --git a/pkg/runner/run_context.go b/pkg/runner/run_context.go index 84ed53d66ca..34e34058fab 100644 --- a/pkg/runner/run_context.go +++ b/pkg/runner/run_context.go @@ -307,7 +307,6 @@ func (rc *RunContext) startJobContainer() common.Executor { Options: spec.Options, NetworkMode: networkName, NetworkAliases: []string{serviceID}, - ValidVolumes: rc.Config.ValidVolumes, }) rc.ServiceContainers = append(rc.ServiceContainers, c) } @@ -340,7 +339,6 @@ func (rc *RunContext) startJobContainer() common.Executor { UsernsMode: rc.Config.UsernsMode, Platform: rc.Config.ContainerArchitecture, Options: rc.options(ctx), - ValidVolumes: rc.Config.ValidVolumes, }) if rc.JobContainer == nil { return errors.New("Failed to create job container") diff --git a/pkg/runner/runner.go b/pkg/runner/runner.go index 75b29f5e767..e1d646eb119 100644 --- a/pkg/runner/runner.go +++ b/pkg/runner/runner.go @@ -59,7 +59,6 @@ type Config struct { ReplaceGheActionTokenWithGithubCom string // Token of private action repo on GitHub. Matrix map[string]map[string]bool // Matrix config to run ContainerNetworkMode docker_container.NetworkMode // the network mode of job containers (the value of --network) - ValidVolumes []string // only volumes (and bind mounts) in this slice can be mounted on the job container or service containers } type caller struct { From 973a5d8f54d3648adc88c29c4e9333552760bc4d Mon Sep 17 00:00:00 2001 From: Sam Foo Date: Sat, 12 Aug 2023 09:51:03 -0700 Subject: [PATCH 10/31] Remove ConnectToNetwork --- pkg/container/container_types.go | 1 - pkg/container/docker_run.go | 21 --------------------- pkg/container/host_environment.go | 6 ------ 3 files changed, 28 deletions(-) diff --git a/pkg/container/container_types.go b/pkg/container/container_types.go index 0036b9a8a5f..b7e8445dfc3 100644 --- a/pkg/container/container_types.go +++ b/pkg/container/container_types.go @@ -39,7 +39,6 @@ type FileEntry struct { // Container for managing docker run containers type Container interface { Create(capAdd []string, capDrop []string) common.Executor - ConnectToNetwork(name string) common.Executor Copy(destPath string, files ...*FileEntry) common.Executor CopyTarStream(ctx context.Context, destPath string, tarStream io.Reader) error CopyDir(destPath string, srcPath string, useGitIgnore bool) common.Executor diff --git a/pkg/container/docker_run.go b/pkg/container/docker_run.go index 6957d5769ac..cf58aee902d 100644 --- a/pkg/container/docker_run.go +++ b/pkg/container/docker_run.go @@ -16,8 +16,6 @@ import ( "strconv" "strings" - networktypes "github.com/docker/docker/api/types/network" - "github.com/go-git/go-billy/v5/helper/polyfill" "github.com/go-git/go-billy/v5/osfs" "github.com/go-git/go-git/v5/plumbing/format/gitignore" @@ -48,25 +46,6 @@ func NewContainer(input *NewContainerInput) ExecutionsEnvironment { return cr } -func (cr *containerReference) ConnectToNetwork(name string) common.Executor { - return common. - NewDebugExecutor("%sdocker network connect %s %s", logPrefix, name, cr.input.Name). - Then( - common.NewPipelineExecutor( - cr.connect(), - cr.connectToNetwork(name, cr.input.NetworkAliases), - ).IfNot(common.Dryrun), - ) -} - -func (cr *containerReference) connectToNetwork(name string, aliases []string) common.Executor { - return func(ctx context.Context) error { - return cr.cli.NetworkConnect(ctx, name, cr.input.Name, &networktypes.EndpointSettings{ - Aliases: aliases, - }) - } -} - // supportsContainerImagePlatform returns true if the underlying Docker server // API version is 1.41 and beyond func supportsContainerImagePlatform(ctx context.Context, cli client.APIClient) bool { diff --git a/pkg/container/host_environment.go b/pkg/container/host_environment.go index fb1393bd86c..91dae4c57fe 100644 --- a/pkg/container/host_environment.go +++ b/pkg/container/host_environment.go @@ -40,12 +40,6 @@ func (e *HostEnvironment) Create(_ []string, _ []string) common.Executor { } } -func (e *HostEnvironment) ConnectToNetwork(_ string) common.Executor { - return func(ctx context.Context) error { - return nil - } -} - func (e *HostEnvironment) Close() common.Executor { return func(ctx context.Context) error { return nil From 197d19d38c8e165355f5afa37bdca2e7bb5f341a Mon Sep 17 00:00:00 2001 From: Sam Foo Date: Sat, 12 Aug 2023 10:39:20 -0700 Subject: [PATCH 11/31] Add docker stubs --- pkg/container/docker_stub.go | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/pkg/container/docker_stub.go b/pkg/container/docker_stub.go index b28c90de613..36f530eecc5 100644 --- a/pkg/container/docker_stub.go +++ b/pkg/container/docker_stub.go @@ -55,3 +55,15 @@ func NewDockerVolumeRemoveExecutor(volume string, force bool) common.Executor { return nil } } + +func NewDockerNetworkCreateExecutor(name string) common.Executor { + return func(ctx context.Context) error { + return nil + } +} + +func NewDockerNetworkRemoveExecutor(name string) common.Executor { + return func(ctx context.Context) error { + return nil + } +} From 83c1304af6f9d5f7e9e45725a208a1a62c5de2bf Mon Sep 17 00:00:00 2001 From: Sam Foo Date: Sat, 12 Aug 2023 10:57:26 -0700 Subject: [PATCH 12/31] Close docker clients to prevent file descriptor leaks --- pkg/container/docker_network.go | 2 ++ pkg/container/docker_run_test.go | 1 + pkg/runner/run_context.go | 1 + 3 files changed, 4 insertions(+) diff --git a/pkg/container/docker_network.go b/pkg/container/docker_network.go index 76394a93df8..049bc21b699 100644 --- a/pkg/container/docker_network.go +++ b/pkg/container/docker_network.go @@ -13,6 +13,7 @@ func NewDockerNetworkCreateExecutor(name string) common.Executor { if err != nil { return err } + defer cli.Close() _, err = cli.NetworkCreate(ctx, name, types.NetworkCreate{ Driver: "bridge", @@ -32,6 +33,7 @@ func NewDockerNetworkRemoveExecutor(name string) common.Executor { if err != nil { return err } + defer cli.Close() return cli.NetworkRemove(ctx, name) } diff --git a/pkg/container/docker_run_test.go b/pkg/container/docker_run_test.go index 8309df6ca80..96bda59214e 100644 --- a/pkg/container/docker_run_test.go +++ b/pkg/container/docker_run_test.go @@ -19,6 +19,7 @@ func TestDocker(t *testing.T) { ctx := context.Background() client, err := GetDockerClient(ctx) assert.NoError(t, err) + defer client.Close() dockerBuild := NewDockerBuildExecutor(NewDockerBuildExecutorInput{ ContextDir: "testdata", diff --git a/pkg/runner/run_context.go b/pkg/runner/run_context.go index 34e34058fab..7da574e07cb 100644 --- a/pkg/runner/run_context.go +++ b/pkg/runner/run_context.go @@ -475,6 +475,7 @@ func (rc *RunContext) stopServiceContainers() common.Executor { execs := []common.Executor{} for _, c := range rc.ServiceContainers { execs = append(execs, c.Remove()) + c.Close() } return common.NewParallelExecutor(len(execs), execs...)(ctx) } From fee22cd43861257d8a8973c51080a456b8251988 Mon Sep 17 00:00:00 2001 From: Zettat123 Date: Wed, 28 Jun 2023 02:27:12 +0000 Subject: [PATCH 13/31] Fix the error when removing network in self-hosted mode (#69) Fixes https://gitea.com/gitea/act_runner/issues/255 Reviewed-on: https://gitea.com/gitea/act/pulls/69 Co-authored-by: Zettat123 Co-committed-by: Zettat123 --- pkg/runner/job_executor.go | 6 ++++-- pkg/runner/run_context.go | 14 +------------- 2 files changed, 5 insertions(+), 15 deletions(-) diff --git a/pkg/runner/job_executor.go b/pkg/runner/job_executor.go index 960fe1fbbef..e877ddb2405 100644 --- a/pkg/runner/job_executor.go +++ b/pkg/runner/job_executor.go @@ -6,6 +6,7 @@ import ( "time" "github.com/nektos/act/pkg/common" + "github.com/nektos/act/pkg/container" "github.com/nektos/act/pkg/model" ) @@ -113,12 +114,13 @@ func newJobExecutor(info jobInfo, sf stepFactory, rc *RunContext) common.Executo if err = info.stopContainer()(ctx); err != nil { logger.Errorf("Error while stop job container: %v", err) } - if rc.Config.ContainerNetworkMode == "" { + if !rc.IsHostEnv(ctx) && rc.Config.ContainerNetworkMode == "" { + // clean network in docker mode only // if the value of `ContainerNetworkMode` is empty string, // it means that the network to which containers are connecting is created by `act_runner`, // so, we should remove the network at last. logger.Infof("Cleaning up network for job %s, and network name is: %s", rc.JobName, rc.networkName()) - if err := rc.removeNetwork(rc.networkName())(ctx); err != nil { + if err := container.NewDockerNetworkRemoveExecutor(rc.networkName())(ctx); err != nil { logger.Errorf("Error while cleaning network: %v", err) } } diff --git a/pkg/runner/run_context.go b/pkg/runner/run_context.go index 7da574e07cb..0db9ff5a16e 100644 --- a/pkg/runner/run_context.go +++ b/pkg/runner/run_context.go @@ -347,7 +347,7 @@ func (rc *RunContext) startJobContainer() common.Executor { return common.NewPipelineExecutor( rc.pullServicesImages(rc.Config.ForcePull), rc.JobContainer.Pull(rc.Config.ForcePull), - rc.createNetwork(networkName).IfBool(rc.Config.ContainerNetworkMode == ""), // if the value of `ContainerNetworkMode` is empty string, then will create a new network for containers. + container.NewDockerNetworkCreateExecutor(networkName).IfBool(!rc.IsHostEnv(ctx) && rc.Config.ContainerNetworkMode == ""), // if the value of `ContainerNetworkMode` is empty string, then will create a new network for containers. rc.startServiceContainers(networkName), rc.JobContainer.Create(rc.Config.ContainerCapAdd, rc.Config.ContainerCapDrop), rc.JobContainer.Start(false), @@ -364,18 +364,6 @@ func (rc *RunContext) startJobContainer() common.Executor { } } -func (rc *RunContext) createNetwork(name string) common.Executor { - return func(ctx context.Context) error { - return container.NewDockerNetworkCreateExecutor(name)(ctx) - } -} - -func (rc *RunContext) removeNetwork(name string) common.Executor { - return func(ctx context.Context) error { - return container.NewDockerNetworkRemoveExecutor(name)(ctx) - } -} - func (rc *RunContext) execJobContainer(cmd []string, env map[string]string, user, workdir string) common.Executor { return func(ctx context.Context) error { return rc.JobContainer.Exec(cmd, env, user, workdir)(ctx) From b84f8315084a927d2671dc7122214538064e4970 Mon Sep 17 00:00:00 2001 From: Sam Foo Date: Sat, 12 Aug 2023 12:38:13 -0700 Subject: [PATCH 14/31] Move service container and network cleanup to rc.cleanUpJobContainer --- pkg/runner/job_executor.go | 16 ---------------- pkg/runner/run_context.go | 16 ++++++++++++++++ 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/pkg/runner/job_executor.go b/pkg/runner/job_executor.go index e877ddb2405..148c9ff5de1 100644 --- a/pkg/runner/job_executor.go +++ b/pkg/runner/job_executor.go @@ -6,7 +6,6 @@ import ( "time" "github.com/nektos/act/pkg/common" - "github.com/nektos/act/pkg/container" "github.com/nektos/act/pkg/model" ) @@ -105,25 +104,10 @@ func newJobExecutor(info jobInfo, sf stepFactory, rc *RunContext) common.Executo defer cancel() logger := common.Logger(ctx) - logger.Infof("Cleaning up services for job %s", rc.JobName) - if err := rc.stopServiceContainers()(ctx); err != nil { - logger.Errorf("Error while cleaning services: %v", err) - } - logger.Infof("Cleaning up container for job %s", rc.JobName) if err = info.stopContainer()(ctx); err != nil { logger.Errorf("Error while stop job container: %v", err) } - if !rc.IsHostEnv(ctx) && rc.Config.ContainerNetworkMode == "" { - // clean network in docker mode only - // if the value of `ContainerNetworkMode` is empty string, - // it means that the network to which containers are connecting is created by `act_runner`, - // so, we should remove the network at last. - logger.Infof("Cleaning up network for job %s, and network name is: %s", rc.JobName, rc.networkName()) - if err := container.NewDockerNetworkRemoveExecutor(rc.networkName())(ctx); err != nil { - logger.Errorf("Error while cleaning network: %v", err) - } - } } setJobResult(ctx, info, rc, jobError == nil) setJobOutputs(ctx, rc) diff --git a/pkg/runner/run_context.go b/pkg/runner/run_context.go index 0db9ff5a16e..5e00de8e14a 100644 --- a/pkg/runner/run_context.go +++ b/pkg/runner/run_context.go @@ -312,6 +312,22 @@ func (rc *RunContext) startJobContainer() common.Executor { } rc.cleanUpJobContainer = func(ctx context.Context) error { + if len(rc.ServiceContainers) > 0 { + logger.Infof("Cleaning up services for job %s", rc.JobName) + if err := rc.stopServiceContainers()(ctx); err != nil { + logger.Errorf("Error while cleaning services: %v", err) + } + if !rc.IsHostEnv(ctx) && rc.Config.ContainerNetworkMode == "" { + // clean network in docker mode only + // if the value of `ContainerNetworkMode` is empty string, + // it means that the network to which containers are connecting is created by `act_runner`, + // so, we should remove the network at last. + logger.Infof("Cleaning up network for job %s, and network name is: %s", rc.JobName, rc.networkName()) + if err := container.NewDockerNetworkRemoveExecutor(rc.networkName())(ctx); err != nil { + logger.Errorf("Error while cleaning network: %v", err) + } + } + } if rc.JobContainer != nil && !rc.Config.ReuseContainers { return rc.JobContainer.Remove(). Then(container.NewDockerVolumeRemoveExecutor(rc.jobContainerName(), false)). From d0aad7fa18ce66c069486dfe75a8d8c9e33fbbbb Mon Sep 17 00:00:00 2001 From: Sam Foo Date: Sat, 12 Aug 2023 16:40:47 -0700 Subject: [PATCH 15/31] Add --network flag; default to host if not using service containers or set explicitly --- cmd/input.go | 1 + cmd/root.go | 3 +++ pkg/artifacts/server_test.go | 1 - pkg/runner/run_context.go | 25 ++++++++++++++----------- 4 files changed, 18 insertions(+), 12 deletions(-) diff --git a/cmd/input.go b/cmd/input.go index 1ae27930b65..f2f8edcdc28 100644 --- a/cmd/input.go +++ b/cmd/input.go @@ -56,6 +56,7 @@ type Input struct { matrix []string actionCachePath string logPrefixJobID bool + networkName string } func (i *Input) resolve(path string) string { diff --git a/cmd/root.go b/cmd/root.go index 43a5ae4bf62..29b0de116e5 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -14,6 +14,7 @@ import ( "github.com/AlecAivazis/survey/v2" "github.com/adrg/xdg" "github.com/andreaskoch/go-fswatch" + docker_container "github.com/docker/docker/api/types/container" "github.com/joho/godotenv" gitignore "github.com/sabhiram/go-gitignore" log "github.com/sirupsen/logrus" @@ -96,6 +97,7 @@ func Execute(ctx context.Context, version string) { rootCmd.PersistentFlags().StringVarP(&input.cacheServerAddr, "cache-server-addr", "", common.GetOutboundIP().String(), "Defines the address to which the cache server binds.") rootCmd.PersistentFlags().Uint16VarP(&input.cacheServerPort, "cache-server-port", "", 0, "Defines the port where the artifact server listens. 0 means a randomly available port.") rootCmd.PersistentFlags().StringVarP(&input.actionCachePath, "action-cache-path", "", filepath.Join(CacheHomeDir, "act"), "Defines the path where the actions get cached and host workspaces created.") + rootCmd.PersistentFlags().StringVarP(&input.networkName, "network", "", "host", "Sets a docker network name. Defaults to host.") rootCmd.SetArgs(args()) if err := rootCmd.Execute(); err != nil { @@ -612,6 +614,7 @@ func newRunCommand(ctx context.Context, input *Input) func(*cobra.Command, []str ReplaceGheActionWithGithubCom: input.replaceGheActionWithGithubCom, ReplaceGheActionTokenWithGithubCom: input.replaceGheActionTokenWithGithubCom, Matrix: matrixes, + ContainerNetworkMode: docker_container.NetworkMode(input.networkName), } r, err := runner.New(config) if err != nil { diff --git a/pkg/artifacts/server_test.go b/pkg/artifacts/server_test.go index 42d0aa32bcf..aeeb0598a4d 100644 --- a/pkg/artifacts/server_test.go +++ b/pkg/artifacts/server_test.go @@ -292,7 +292,6 @@ func runTestJobFile(ctx context.Context, t *testing.T, tjfi TestJobFileInfo) { ArtifactServerPath: artifactsPath, ArtifactServerAddr: artifactsAddr, ArtifactServerPort: artifactsPort, - ContainerNetworkMode: "host", } runner, err := runner.New(runnerConfig) diff --git a/pkg/runner/run_context.go b/pkg/runner/run_context.go index 5e00de8e14a..d878fd8ea35 100644 --- a/pkg/runner/run_context.go +++ b/pkg/runner/run_context.go @@ -88,9 +88,15 @@ func (rc *RunContext) jobContainerName() string { } // networkName return the name of the network which will be created by `act` automatically for job, -// only create network if `rc.Config.ContainerNetworkMode` is empty string. +// only create network if using a service container func (rc *RunContext) networkName() string { - return fmt.Sprintf("%s-network", rc.jobContainerName()) + if rc.Config.ContainerNetworkMode == "" { + return "host" + } + if len(rc.Run.Job().Services) > 0 { + return fmt.Sprintf("%s-%s-network", rc.jobContainerName(), rc.Run.JobID) + } + return string(rc.Config.ContainerNetworkMode) } func getDockerDaemonSocketMountPath(daemonPath string) string { @@ -266,12 +272,9 @@ func (rc *RunContext) startJobContainer() common.Executor { binds, mounts := rc.GetBindsAndMounts() // specify the network to which the container will connect when `docker create` stage. (like execute command line: docker create --network ) - networkName := string(rc.Config.ContainerNetworkMode) - if networkName == "" { - // if networkName is empty string, will create a new network for the containers. - // and it will be removed after at last. - networkName = rc.networkName() - } + // if using service containers, will create a new network for the containers. + // and it will be removed after at last. + networkName := rc.networkName() // add service containers for serviceID, spec := range rc.Run.Job().Services { @@ -317,9 +320,9 @@ func (rc *RunContext) startJobContainer() common.Executor { if err := rc.stopServiceContainers()(ctx); err != nil { logger.Errorf("Error while cleaning services: %v", err) } - if !rc.IsHostEnv(ctx) && rc.Config.ContainerNetworkMode == "" { + if !rc.IsHostEnv(ctx) { // clean network in docker mode only - // if the value of `ContainerNetworkMode` is empty string, + // if using service containers // it means that the network to which containers are connecting is created by `act_runner`, // so, we should remove the network at last. logger.Infof("Cleaning up network for job %s, and network name is: %s", rc.JobName, rc.networkName()) @@ -363,7 +366,7 @@ func (rc *RunContext) startJobContainer() common.Executor { return common.NewPipelineExecutor( rc.pullServicesImages(rc.Config.ForcePull), rc.JobContainer.Pull(rc.Config.ForcePull), - container.NewDockerNetworkCreateExecutor(networkName).IfBool(!rc.IsHostEnv(ctx) && rc.Config.ContainerNetworkMode == ""), // if the value of `ContainerNetworkMode` is empty string, then will create a new network for containers. + container.NewDockerNetworkCreateExecutor(networkName).IfBool(!rc.IsHostEnv(ctx) && networkName != "host"), rc.startServiceContainers(networkName), rc.JobContainer.Create(rc.Config.ContainerCapAdd, rc.Config.ContainerCapDrop), rc.JobContainer.Start(false), From 3711e0d047ba7a01099a18009c048c56d1b68a92 Mon Sep 17 00:00:00 2001 From: Sam Foo Date: Sun, 13 Aug 2023 11:55:46 -0700 Subject: [PATCH 16/31] Correctly close executor to prevent fd leak --- pkg/runner/run_context.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pkg/runner/run_context.go b/pkg/runner/run_context.go index d878fd8ea35..29e376366ca 100644 --- a/pkg/runner/run_context.go +++ b/pkg/runner/run_context.go @@ -481,8 +481,7 @@ func (rc *RunContext) stopServiceContainers() common.Executor { return func(ctx context.Context) error { execs := []common.Executor{} for _, c := range rc.ServiceContainers { - execs = append(execs, c.Remove()) - c.Close() + execs = append(execs, c.Remove().Finally(c.Close())) } return common.NewParallelExecutor(len(execs), execs...)(ctx) } From 7480591b9f8eeccea153228551308e99ab149143 Mon Sep 17 00:00:00 2001 From: Sam Foo Date: Mon, 14 Aug 2023 11:46:03 -0700 Subject: [PATCH 17/31] Revert to tail instead of full path --- pkg/runner/run_context.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/runner/run_context.go b/pkg/runner/run_context.go index 29e376366ca..756fb9cc5cb 100644 --- a/pkg/runner/run_context.go +++ b/pkg/runner/run_context.go @@ -341,7 +341,7 @@ func (rc *RunContext) startJobContainer() common.Executor { rc.JobContainer = container.NewContainer(&container.NewContainerInput{ Cmd: nil, - Entrypoint: []string{"/usr/bin/tail", "-f", "/dev/null"}, + Entrypoint: []string{"tail", "-f", "/dev/null"}, WorkingDir: ext.ToContainerPath(rc.Config.Workdir), Image: image, Username: username, From f3b795377839368ac97ffcd15661a049d97b861e Mon Sep 17 00:00:00 2001 From: ChristopherHX Date: Tue, 22 Aug 2023 15:45:35 +0000 Subject: [PATCH 18/31] fix network duplication --- pkg/container/docker_network.go | 30 +++++++++++++++++++++++++- pkg/runner/run_context.go | 37 ++++++++++++++++++--------------- 2 files changed, 49 insertions(+), 18 deletions(-) diff --git a/pkg/container/docker_network.go b/pkg/container/docker_network.go index 049bc21b699..5812aa9e082 100644 --- a/pkg/container/docker_network.go +++ b/pkg/container/docker_network.go @@ -15,6 +15,19 @@ func NewDockerNetworkCreateExecutor(name string) common.Executor { } defer cli.Close() + // Only create the network if it doesn't exist + networks, err := cli.NetworkList(ctx, types.NetworkListOptions{}) + if err != nil { + return err + } + common.Logger(ctx).Debugf("%v", networks) + for _, network := range networks { + if network.Name == name { + common.Logger(ctx).Debugf("Network %v exists", name) + return nil + } + } + _, err = cli.NetworkCreate(ctx, name, types.NetworkCreate{ Driver: "bridge", Scope: "local", @@ -35,6 +48,21 @@ func NewDockerNetworkRemoveExecutor(name string) common.Executor { } defer cli.Close() - return cli.NetworkRemove(ctx, name) + // Make shure that all network of the specified name are removed + // cli.NetworkRemove refuses to remove a network if there are duplicates + networks, err := cli.NetworkList(ctx, types.NetworkListOptions{}) + if err != nil { + return err + } + common.Logger(ctx).Debugf("%v", networks) + for _, network := range networks { + if network.Name == name { + if err = cli.NetworkRemove(ctx, network.ID); err != nil { + common.Logger(ctx).Debugf("%v", err) + } + } + } + + return err } } diff --git a/pkg/runner/run_context.go b/pkg/runner/run_context.go index 756fb9cc5cb..ffd756eef76 100644 --- a/pkg/runner/run_context.go +++ b/pkg/runner/run_context.go @@ -315,26 +315,29 @@ func (rc *RunContext) startJobContainer() common.Executor { } rc.cleanUpJobContainer = func(ctx context.Context) error { - if len(rc.ServiceContainers) > 0 { - logger.Infof("Cleaning up services for job %s", rc.JobName) - if err := rc.stopServiceContainers()(ctx); err != nil { - logger.Errorf("Error while cleaning services: %v", err) - } - if !rc.IsHostEnv(ctx) { - // clean network in docker mode only - // if using service containers - // it means that the network to which containers are connecting is created by `act_runner`, - // so, we should remove the network at last. - logger.Infof("Cleaning up network for job %s, and network name is: %s", rc.JobName, rc.networkName()) - if err := container.NewDockerNetworkRemoveExecutor(rc.networkName())(ctx); err != nil { - logger.Errorf("Error while cleaning network: %v", err) - } - } - } if rc.JobContainer != nil && !rc.Config.ReuseContainers { return rc.JobContainer.Remove(). Then(container.NewDockerVolumeRemoveExecutor(rc.jobContainerName(), false)). - Then(container.NewDockerVolumeRemoveExecutor(rc.jobContainerName()+"-env", false))(ctx) + Then(container.NewDockerVolumeRemoveExecutor(rc.jobContainerName()+"-env", false)). + Then(func(ctx context.Context) error { + if len(rc.ServiceContainers) > 0 { + logger.Infof("Cleaning up services for job %s", rc.JobName) + if err := rc.stopServiceContainers()(ctx); err != nil { + logger.Errorf("Error while cleaning services: %v", err) + } + if !rc.IsHostEnv(ctx) { + // clean network in docker mode only + // if using service containers + // it means that the network to which containers are connecting is created by `act_runner`, + // so, we should remove the network at last. + logger.Infof("Cleaning up network for job %s, and network name is: %s", rc.JobName, rc.networkName()) + if err := container.NewDockerNetworkRemoveExecutor(rc.networkName())(ctx); err != nil { + logger.Errorf("Error while cleaning network: %v", err) + } + } + } + return nil + })(ctx) } return nil } From e90c5438dc84952546bc0977ac906cf7ecbb6200 Mon Sep 17 00:00:00 2001 From: ChristopherHX Date: Tue, 22 Aug 2023 15:56:58 +0000 Subject: [PATCH 19/31] backport networkingConfig for aliaes --- pkg/container/docker_run.go | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/pkg/container/docker_run.go b/pkg/container/docker_run.go index cf58aee902d..b6478cc86e6 100644 --- a/pkg/container/docker_run.go +++ b/pkg/container/docker_run.go @@ -29,6 +29,7 @@ import ( "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/container" "github.com/docker/docker/api/types/mount" + "github.com/docker/docker/api/types/network" "github.com/docker/docker/client" "github.com/docker/docker/pkg/stdcopy" specs "github.com/opencontainers/image-spec/specs-go/v1" @@ -445,7 +446,22 @@ func (cr *containerReference) create(capAdd []string, capDrop []string) common.E return err } - resp, err := cr.cli.ContainerCreate(ctx, config, hostConfig, nil, platSpecs, input.Name) + var networkingConfig *network.NetworkingConfig + logger.Debugf("input.NetworkAliases ==> %v", input.NetworkAliases) + if hostConfig.NetworkMode.IsUserDefined() && len(input.NetworkAliases) > 0 { + endpointConfig := &network.EndpointSettings{ + Aliases: input.NetworkAliases, + } + networkingConfig = &network.NetworkingConfig{ + EndpointsConfig: map[string]*network.EndpointSettings{ + input.NetworkMode: endpointConfig, + }, + } + } else { + logger.Debugf("not a use defined config??") + } + + resp, err := cr.cli.ContainerCreate(ctx, config, hostConfig, networkingConfig, platSpecs, input.Name) if err != nil { return fmt.Errorf("failed to create container: '%w'", err) } From 779ad2af2f25c4ecf2d5b705e2eeba203ccedccb Mon Sep 17 00:00:00 2001 From: ChristopherHX Date: Tue, 22 Aug 2023 16:38:22 +0000 Subject: [PATCH 20/31] don't hardcode netMode host --- pkg/container/docker_run.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/container/docker_run.go b/pkg/container/docker_run.go index b6478cc86e6..c6edd4b6238 100644 --- a/pkg/container/docker_run.go +++ b/pkg/container/docker_run.go @@ -347,8 +347,8 @@ func (cr *containerReference) mergeContainerConfigs(ctx context.Context, config } if len(copts.netMode.Value()) == 0 { - if err = copts.netMode.Set("host"); err != nil { - return nil, nil, fmt.Errorf("Cannot parse networkmode=host. This is an internal error and should not happen: '%w'", err) + if err = copts.netMode.Set(cr.input.NetworkMode); err != nil { + return nil, nil, fmt.Errorf("Cannot parse networkmode=%s. This is an internal error and should not happen: '%w'", cr.input.NetworkMode, err) } } From 1e8ce808debf5eddb3041de171f00fffa969e41b Mon Sep 17 00:00:00 2001 From: ZauberNerd Date: Sun, 3 Sep 2023 12:14:59 +0200 Subject: [PATCH 21/31] Convert services test to table driven tests --- pkg/runner/runner_test.go | 41 ++++----------------------------------- 1 file changed, 4 insertions(+), 37 deletions(-) diff --git a/pkg/runner/runner_test.go b/pkg/runner/runner_test.go index b1de915703e..fba7528bc45 100644 --- a/pkg/runner/runner_test.go +++ b/pkg/runner/runner_test.go @@ -187,6 +187,7 @@ func (j *TestJobFileInfo) runTest(ctx context.Context, t *testing.T, cfg *Config GitHubInstance: "github.com", ContainerArchitecture: cfg.ContainerArchitecture, Matrix: cfg.Matrix, + ContainerNetworkMode: "host", } runner, err := New(runnerConfig) @@ -301,6 +302,9 @@ func TestRunEvent(t *testing.T) { {workdir, "set-env-step-env-override", "push", "", platforms, secrets}, {workdir, "set-env-new-env-file-per-step", "push", "", platforms, secrets}, {workdir, "no-panic-on-invalid-composite-action", "push", "jobs failed due to invalid action", platforms, secrets}, + + // services + {workdir, "services", "push", "", platforms, secrets}, } for _, table := range tables { @@ -548,43 +552,6 @@ func TestRunEventSecrets(t *testing.T) { tjfi.runTest(context.Background(), t, &Config{Secrets: secrets, Env: env}) } -func TestRunWithService(t *testing.T) { - if testing.Short() { - t.Skip("skipping integration test") - } - - log.SetLevel(log.DebugLevel) - ctx := context.Background() - - platforms := map[string]string{ - "ubuntu-latest": "node:12.20.1-buster-slim", - } - - workflowPath := "services" - eventName := "push" - - workdir, err := filepath.Abs("testdata") - assert.NoError(t, err, workflowPath) - - runnerConfig := &Config{ - Workdir: workdir, - EventName: eventName, - Platforms: platforms, - ReuseContainers: false, - } - runner, err := New(runnerConfig) - assert.NoError(t, err, workflowPath) - - planner, err := model.NewWorkflowPlanner(fmt.Sprintf("testdata/%s", workflowPath), true) - assert.NoError(t, err, workflowPath) - - plan, err := planner.PlanEvent(eventName) - assert.NoError(t, err, workflowPath) - - err = runner.NewPlanExecutor(plan)(ctx) - assert.NoError(t, err, workflowPath) -} - func TestRunActionInputs(t *testing.T) { if testing.Short() { t.Skip("skipping integration test") From 0f0639ce57aba391ca8af23b00c2dbbdcd5102b4 Mon Sep 17 00:00:00 2001 From: ZauberNerd Date: Sun, 3 Sep 2023 12:15:32 +0200 Subject: [PATCH 22/31] Add failing tests for services --- pkg/runner/runner_test.go | 2 ++ .../testdata/services-host-network/push.yml | 14 ++++++++++++++ .../testdata/services-with-container/push.yml | 16 ++++++++++++++++ 3 files changed, 32 insertions(+) create mode 100644 pkg/runner/testdata/services-host-network/push.yml create mode 100644 pkg/runner/testdata/services-with-container/push.yml diff --git a/pkg/runner/runner_test.go b/pkg/runner/runner_test.go index fba7528bc45..b0619de46fc 100644 --- a/pkg/runner/runner_test.go +++ b/pkg/runner/runner_test.go @@ -305,6 +305,8 @@ func TestRunEvent(t *testing.T) { // services {workdir, "services", "push", "", platforms, secrets}, + {workdir, "services-host-network", "push", "", platforms, secrets}, + {workdir, "services-with-container", "push", "", platforms, secrets}, } for _, table := range tables { diff --git a/pkg/runner/testdata/services-host-network/push.yml b/pkg/runner/testdata/services-host-network/push.yml new file mode 100644 index 00000000000..8d0eb2947bf --- /dev/null +++ b/pkg/runner/testdata/services-host-network/push.yml @@ -0,0 +1,14 @@ +name: services-host-network +on: push +jobs: + services-host-network: + runs-on: ubuntu-latest + services: + nginx: + image: "nginx:latest" + ports: + - "8080:80" + steps: + - run: apt-get -qq update && apt-get -yqq install --no-install-recommends curl net-tools + - run: netstat -tlpen + - run: curl -v http://localhost:8080 diff --git a/pkg/runner/testdata/services-with-container/push.yml b/pkg/runner/testdata/services-with-container/push.yml new file mode 100644 index 00000000000..b37e5dcd421 --- /dev/null +++ b/pkg/runner/testdata/services-with-container/push.yml @@ -0,0 +1,16 @@ +name: services-with-containers +on: push +jobs: + services-with-containers: + runs-on: ubuntu-latest + # https://docs.github.com/en/actions/using-containerized-services/about-service-containers#running-jobs-in-a-container + container: + image: "ubuntu:latest" + services: + nginx: + image: "nginx:latest" + ports: + - "8080:80" + steps: + - run: apt-get -qq update && apt-get -yqq install --no-install-recommends curl + - run: curl -v http://nginx:80 From 5bad75c32f4bb1e573dbdcf6d3413b412e91cc1a Mon Sep 17 00:00:00 2001 From: ZauberNerd Date: Sun, 3 Sep 2023 12:16:51 +0200 Subject: [PATCH 23/31] Expose service container ports onto the host --- pkg/container/container_types.go | 3 +++ pkg/container/docker_run.go | 24 +++++++++++++----------- pkg/runner/run_context.go | 17 ++++++++++++++++- 3 files changed, 32 insertions(+), 12 deletions(-) diff --git a/pkg/container/container_types.go b/pkg/container/container_types.go index b7e8445dfc3..37d293a3817 100644 --- a/pkg/container/container_types.go +++ b/pkg/container/container_types.go @@ -4,6 +4,7 @@ import ( "context" "io" + "github.com/docker/go-connections/nat" "github.com/nektos/act/pkg/common" ) @@ -27,6 +28,8 @@ type NewContainerInput struct { Platform string Options string NetworkAliases []string + ExposedPorts nat.PortSet + PortBindings nat.PortMap } // FileEntry is a file to copy to a container diff --git a/pkg/container/docker_run.go b/pkg/container/docker_run.go index c6edd4b6238..b3e33145fc9 100644 --- a/pkg/container/docker_run.go +++ b/pkg/container/docker_run.go @@ -392,10 +392,11 @@ func (cr *containerReference) create(capAdd []string, capDrop []string) common.E input := cr.input config := &container.Config{ - Image: input.Image, - WorkingDir: input.WorkingDir, - Env: input.Env, - Tty: isTerminal, + Image: input.Image, + WorkingDir: input.WorkingDir, + Env: input.Env, + ExposedPorts: input.ExposedPorts, + Tty: isTerminal, } logger.Debugf("Common container.Config ==> %+v", config) @@ -431,13 +432,14 @@ func (cr *containerReference) create(capAdd []string, capDrop []string) common.E } hostConfig := &container.HostConfig{ - CapAdd: capAdd, - CapDrop: capDrop, - Binds: input.Binds, - Mounts: mounts, - NetworkMode: container.NetworkMode(input.NetworkMode), - Privileged: input.Privileged, - UsernsMode: container.UsernsMode(input.UsernsMode), + CapAdd: capAdd, + CapDrop: capDrop, + Binds: input.Binds, + Mounts: mounts, + NetworkMode: container.NetworkMode(input.NetworkMode), + Privileged: input.Privileged, + UsernsMode: container.UsernsMode(input.UsernsMode), + PortBindings: input.PortBindings, } logger.Debugf("Common container.HostConfig ==> %+v", hostConfig) diff --git a/pkg/runner/run_context.go b/pkg/runner/run_context.go index ffd756eef76..7e96c8fff16 100644 --- a/pkg/runner/run_context.go +++ b/pkg/runner/run_context.go @@ -17,6 +17,7 @@ import ( "runtime" "strings" + "github.com/docker/go-connections/nat" "github.com/nektos/act/pkg/common" "github.com/nektos/act/pkg/container" "github.com/nektos/act/pkg/exprparser" @@ -238,6 +239,7 @@ func (rc *RunContext) startHostEnvironment() common.Executor { } } +//nolint:gocyclo func (rc *RunContext) startJobContainer() common.Executor { return func(ctx context.Context) error { logger := common.Logger(ctx) @@ -292,6 +294,12 @@ func (rc *RunContext) startJobContainer() common.Executor { return fmt.Errorf("failed to handle service %s credentials: %w", serviceID, err) } serviceBinds, serviceMounts := rc.GetServiceBindsAndMounts(spec.Volumes) + + exposedPorts, portBindings, err := nat.ParsePortSpecs(spec.Ports) + if err != nil { + return fmt.Errorf("failed to parse service %s ports: %w", serviceID, err) + } + serviceContainerName := createContainerName(rc.jobContainerName(), serviceID) c := container.NewContainer(&container.NewContainerInput{ Name: serviceContainerName, @@ -310,6 +318,8 @@ func (rc *RunContext) startJobContainer() common.Executor { Options: spec.Options, NetworkMode: networkName, NetworkAliases: []string{serviceID}, + ExposedPorts: exposedPorts, + PortBindings: portBindings, }) rc.ServiceContainers = append(rc.ServiceContainers, c) } @@ -342,6 +352,11 @@ func (rc *RunContext) startJobContainer() common.Executor { return nil } + jobContainerNetwork := rc.Config.ContainerNetworkMode.NetworkName() + if rc.Run.Job().Container() != nil { + jobContainerNetwork = networkName + } + rc.JobContainer = container.NewContainer(&container.NewContainerInput{ Cmd: nil, Entrypoint: []string{"tail", "-f", "/dev/null"}, @@ -352,7 +367,7 @@ func (rc *RunContext) startJobContainer() common.Executor { Name: name, Env: envList, Mounts: mounts, - NetworkMode: networkName, + NetworkMode: jobContainerNetwork, NetworkAliases: []string{rc.Name}, Binds: binds, Stdout: logWriter, From 5f5f3cfbe70d7b0424abab19124c836671fcc8bd Mon Sep 17 00:00:00 2001 From: ZauberNerd Date: Sun, 3 Sep 2023 16:46:45 +0200 Subject: [PATCH 24/31] Set container network mode in artifacts server test to host mode --- pkg/artifacts/server_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/artifacts/server_test.go b/pkg/artifacts/server_test.go index aeeb0598a4d..42d0aa32bcf 100644 --- a/pkg/artifacts/server_test.go +++ b/pkg/artifacts/server_test.go @@ -292,6 +292,7 @@ func runTestJobFile(ctx context.Context, t *testing.T, tjfi TestJobFileInfo) { ArtifactServerPath: artifactsPath, ArtifactServerAddr: artifactsAddr, ArtifactServerPort: artifactsPort, + ContainerNetworkMode: "host", } runner, err := runner.New(runnerConfig) From 10ab2db0e2ea80d83a73c91699a647227d6e0e7d Mon Sep 17 00:00:00 2001 From: ZauberNerd Date: Sun, 3 Sep 2023 16:47:16 +0200 Subject: [PATCH 25/31] Log container network mode when creating/starting a container --- pkg/container/docker_run.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/container/docker_run.go b/pkg/container/docker_run.go index b3e33145fc9..d24c4ac33f8 100644 --- a/pkg/container/docker_run.go +++ b/pkg/container/docker_run.go @@ -67,7 +67,7 @@ func supportsContainerImagePlatform(ctx context.Context, cli client.APIClient) b func (cr *containerReference) Create(capAdd []string, capDrop []string) common.Executor { return common. - NewInfoExecutor("%sdocker create image=%s platform=%s entrypoint=%+q cmd=%+q", logPrefix, cr.input.Image, cr.input.Platform, cr.input.Entrypoint, cr.input.Cmd). + NewInfoExecutor("%sdocker create image=%s platform=%s entrypoint=%+q cmd=%+q network=%+q", logPrefix, cr.input.Image, cr.input.Platform, cr.input.Entrypoint, cr.input.Cmd, cr.input.NetworkMode). Then( common.NewPipelineExecutor( cr.connect(), @@ -79,7 +79,7 @@ func (cr *containerReference) Create(capAdd []string, capDrop []string) common.E func (cr *containerReference) Start(attach bool) common.Executor { return common. - NewInfoExecutor("%sdocker run image=%s platform=%s entrypoint=%+q cmd=%+q", logPrefix, cr.input.Image, cr.input.Platform, cr.input.Entrypoint, cr.input.Cmd). + NewInfoExecutor("%sdocker run image=%s platform=%s entrypoint=%+q cmd=%+q network=%+q", logPrefix, cr.input.Image, cr.input.Platform, cr.input.Entrypoint, cr.input.Cmd, cr.input.NetworkMode). Then( common.NewPipelineExecutor( cr.connect(), From 5ce86a6768d2827ac944f949fc6532ce1e52df68 Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Mon, 4 Sep 2023 18:15:09 +0200 Subject: [PATCH 26/31] fix: Correctly handle ContainerNetworkMode --- pkg/artifacts/server_test.go | 1 - pkg/runner/run_context.go | 24 +++++++++++++----------- pkg/runner/runner_test.go | 1 - 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/pkg/artifacts/server_test.go b/pkg/artifacts/server_test.go index 42d0aa32bcf..aeeb0598a4d 100644 --- a/pkg/artifacts/server_test.go +++ b/pkg/artifacts/server_test.go @@ -292,7 +292,6 @@ func runTestJobFile(ctx context.Context, t *testing.T, tjfi TestJobFileInfo) { ArtifactServerPath: artifactsPath, ArtifactServerAddr: artifactsAddr, ArtifactServerPort: artifactsPort, - ContainerNetworkMode: "host", } runner, err := runner.New(runnerConfig) diff --git a/pkg/runner/run_context.go b/pkg/runner/run_context.go index 7e96c8fff16..b484160fb27 100644 --- a/pkg/runner/run_context.go +++ b/pkg/runner/run_context.go @@ -90,14 +90,14 @@ func (rc *RunContext) jobContainerName() string { // networkName return the name of the network which will be created by `act` automatically for job, // only create network if using a service container -func (rc *RunContext) networkName() string { +func (rc *RunContext) networkName() (string, bool) { if rc.Config.ContainerNetworkMode == "" { - return "host" + return "host", false } if len(rc.Run.Job().Services) > 0 { - return fmt.Sprintf("%s-%s-network", rc.jobContainerName(), rc.Run.JobID) + return fmt.Sprintf("%s-%s-network", rc.jobContainerName(), rc.Run.JobID), true } - return string(rc.Config.ContainerNetworkMode) + return string(rc.Config.ContainerNetworkMode), false } func getDockerDaemonSocketMountPath(daemonPath string) string { @@ -276,7 +276,7 @@ func (rc *RunContext) startJobContainer() common.Executor { // specify the network to which the container will connect when `docker create` stage. (like execute command line: docker create --network ) // if using service containers, will create a new network for the containers. // and it will be removed after at last. - networkName := rc.networkName() + networkName, createAndDeleteNetwork := rc.networkName() // add service containers for serviceID, spec := range rc.Run.Job().Services { @@ -335,13 +335,13 @@ func (rc *RunContext) startJobContainer() common.Executor { if err := rc.stopServiceContainers()(ctx); err != nil { logger.Errorf("Error while cleaning services: %v", err) } - if !rc.IsHostEnv(ctx) { - // clean network in docker mode only + if createAndDeleteNetwork { + // clean network if it has been created by act // if using service containers // it means that the network to which containers are connecting is created by `act_runner`, // so, we should remove the network at last. - logger.Infof("Cleaning up network for job %s, and network name is: %s", rc.JobName, rc.networkName()) - if err := container.NewDockerNetworkRemoveExecutor(rc.networkName())(ctx); err != nil { + logger.Infof("Cleaning up network for job %s, and network name is: %s", rc.JobName, networkName) + if err := container.NewDockerNetworkRemoveExecutor(networkName)(ctx); err != nil { logger.Errorf("Error while cleaning network: %v", err) } } @@ -353,8 +353,10 @@ func (rc *RunContext) startJobContainer() common.Executor { } jobContainerNetwork := rc.Config.ContainerNetworkMode.NetworkName() - if rc.Run.Job().Container() != nil { + if rc.containerImage(ctx) != "" { jobContainerNetwork = networkName + } else if jobContainerNetwork == "" { + jobContainerNetwork = "host" } rc.JobContainer = container.NewContainer(&container.NewContainerInput{ @@ -384,7 +386,7 @@ func (rc *RunContext) startJobContainer() common.Executor { return common.NewPipelineExecutor( rc.pullServicesImages(rc.Config.ForcePull), rc.JobContainer.Pull(rc.Config.ForcePull), - container.NewDockerNetworkCreateExecutor(networkName).IfBool(!rc.IsHostEnv(ctx) && networkName != "host"), + container.NewDockerNetworkCreateExecutor(networkName).IfBool(createAndDeleteNetwork), rc.startServiceContainers(networkName), rc.JobContainer.Create(rc.Config.ContainerCapAdd, rc.Config.ContainerCapDrop), rc.JobContainer.Start(false), diff --git a/pkg/runner/runner_test.go b/pkg/runner/runner_test.go index b0619de46fc..6d6943ba995 100644 --- a/pkg/runner/runner_test.go +++ b/pkg/runner/runner_test.go @@ -187,7 +187,6 @@ func (j *TestJobFileInfo) runTest(ctx context.Context, t *testing.T, cfg *Config GitHubInstance: "github.com", ContainerArchitecture: cfg.ContainerArchitecture, Matrix: cfg.Matrix, - ContainerNetworkMode: "host", } runner, err := New(runnerConfig) From ed086f2bdb5d4ab459da157f2718a0787994d200 Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Mon, 4 Sep 2023 20:34:18 +0200 Subject: [PATCH 27/31] fix: missing service container network --- pkg/runner/run_context.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/runner/run_context.go b/pkg/runner/run_context.go index b484160fb27..6bdf431d741 100644 --- a/pkg/runner/run_context.go +++ b/pkg/runner/run_context.go @@ -91,12 +91,12 @@ func (rc *RunContext) jobContainerName() string { // networkName return the name of the network which will be created by `act` automatically for job, // only create network if using a service container func (rc *RunContext) networkName() (string, bool) { - if rc.Config.ContainerNetworkMode == "" { - return "host", false - } if len(rc.Run.Job().Services) > 0 { return fmt.Sprintf("%s-%s-network", rc.jobContainerName(), rc.Run.JobID), true } + if rc.Config.ContainerNetworkMode == "" { + return "host", false + } return string(rc.Config.ContainerNetworkMode), false } From 8e5b4f92da00e6c47816808eb69cc809934ce478 Mon Sep 17 00:00:00 2001 From: ZauberNerd Date: Wed, 11 Oct 2023 18:50:51 +0200 Subject: [PATCH 28/31] Always remove service containers Although we usually keep containers running if the workflow errored (unless `--rm` is given) in order to facilitate debugging and we have a flag (`--reuse`) to always keep containers running in order to speed up repeated `act` invocations, I believe that these should only apply to job containers and not service containers, because changing the network settings on a service container requires re-creating it anyway. --- pkg/runner/run_context.go | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/pkg/runner/run_context.go b/pkg/runner/run_context.go index 6bdf431d741..e8af5d2629c 100644 --- a/pkg/runner/run_context.go +++ b/pkg/runner/run_context.go @@ -325,10 +325,14 @@ func (rc *RunContext) startJobContainer() common.Executor { } rc.cleanUpJobContainer = func(ctx context.Context) error { - if rc.JobContainer != nil && !rc.Config.ReuseContainers { - return rc.JobContainer.Remove(). - Then(container.NewDockerVolumeRemoveExecutor(rc.jobContainerName(), false)). - Then(container.NewDockerVolumeRemoveExecutor(rc.jobContainerName()+"-env", false)). + reuseJobContainer := func(ctx context.Context) bool { + return rc.Config.ReuseContainers + } + + if rc.JobContainer != nil { + return rc.JobContainer.Remove().IfNot(reuseJobContainer). + Then(container.NewDockerVolumeRemoveExecutor(rc.jobContainerName(), false)).IfNot(reuseJobContainer). + Then(container.NewDockerVolumeRemoveExecutor(rc.jobContainerName()+"-env", false)).IfNot(reuseJobContainer). Then(func(ctx context.Context) error { if len(rc.ServiceContainers) > 0 { logger.Infof("Cleaning up services for job %s", rc.JobName) @@ -463,10 +467,10 @@ func (rc *RunContext) UpdateExtraPath(ctx context.Context, githubEnvPath string) return nil } -// stopJobContainer removes the job container (if it exists) and its volume (if it exists) if !rc.Config.ReuseContainers +// stopJobContainer removes the job container (if it exists) and its volume (if it exists) func (rc *RunContext) stopJobContainer() common.Executor { return func(ctx context.Context) error { - if rc.cleanUpJobContainer != nil && !rc.Config.ReuseContainers { + if rc.cleanUpJobContainer != nil { return rc.cleanUpJobContainer(ctx) } return nil From 092a41cec07de8f894fe680d61396e60b4da2132 Mon Sep 17 00:00:00 2001 From: ZauberNerd Date: Wed, 11 Oct 2023 19:01:40 +0200 Subject: [PATCH 29/31] Remove networks only if no active endpoints exist --- pkg/container/docker_network.go | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/pkg/container/docker_network.go b/pkg/container/docker_network.go index 5812aa9e082..6914439afac 100644 --- a/pkg/container/docker_network.go +++ b/pkg/container/docker_network.go @@ -57,8 +57,17 @@ func NewDockerNetworkRemoveExecutor(name string) common.Executor { common.Logger(ctx).Debugf("%v", networks) for _, network := range networks { if network.Name == name { - if err = cli.NetworkRemove(ctx, network.ID); err != nil { - common.Logger(ctx).Debugf("%v", err) + result, err := cli.NetworkInspect(ctx, network.ID, types.NetworkInspectOptions{}) + if err != nil { + return err + } + + if len(result.Containers) == 0 { + if err = cli.NetworkRemove(ctx, network.ID); 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) } } } From 41b68ebea3f05746a80ef35e62bb8521fe5a6f0d Mon Sep 17 00:00:00 2001 From: ZauberNerd Date: Fri, 13 Oct 2023 14:44:43 +0200 Subject: [PATCH 30/31] Ensure job containers are stopped before starting a new job --- pkg/runner/run_context.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/runner/run_context.go b/pkg/runner/run_context.go index e8af5d2629c..f42170ca4e9 100644 --- a/pkg/runner/run_context.go +++ b/pkg/runner/run_context.go @@ -390,6 +390,7 @@ func (rc *RunContext) startJobContainer() common.Executor { return common.NewPipelineExecutor( rc.pullServicesImages(rc.Config.ForcePull), rc.JobContainer.Pull(rc.Config.ForcePull), + rc.stopJobContainer(), container.NewDockerNetworkCreateExecutor(networkName).IfBool(createAndDeleteNetwork), rc.startServiceContainers(networkName), rc.JobContainer.Create(rc.Config.ContainerCapAdd, rc.Config.ContainerCapDrop), From 0f701ecaa9e6851a8c00f79a76560e4e3ddc30eb Mon Sep 17 00:00:00 2001 From: ChristopherHX Date: Tue, 17 Oct 2023 14:11:19 +0200 Subject: [PATCH 31/31] fix: go build -tags WITHOUT_DOCKER --- pkg/container/docker_network.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/container/docker_network.go b/pkg/container/docker_network.go index 6914439afac..8a7528ab300 100644 --- a/pkg/container/docker_network.go +++ b/pkg/container/docker_network.go @@ -1,3 +1,5 @@ +//go:build !(WITHOUT_DOCKER || !(linux || darwin || windows)) + package container import (