From 9283cfc9b16637669ddac20fb66935493dac35ec Mon Sep 17 00:00:00 2001 From: sillyguodong Date: Tue, 16 May 2023 14:03:55 +0800 Subject: [PATCH] Fix container network issue (#56) Follow: https://gitea.com/gitea/act_runner/pulls/184 Close https://gitea.com/gitea/act_runner/issues/177 #### changes: - `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/container/docker_run.go | 17 +++++++- pkg/runner/job_executor.go | 17 +++++--- pkg/runner/run_context.go | 77 +++++++++++++++++++------------------ pkg/runner/runner.go | 3 +- 4 files changed, 69 insertions(+), 45 deletions(-) diff --git a/pkg/container/docker_run.go b/pkg/container/docker_run.go index 5ccbb93..82d6679 100644 --- a/pkg/container/docker_run.go +++ b/pkg/container/docker_run.go @@ -16,6 +16,7 @@ import ( "strconv" "strings" + "github.com/docker/docker/api/types/network" networktypes "github.com/docker/docker/api/types/network" "github.com/go-git/go-billy/v5/helper/polyfill" @@ -477,7 +478,21 @@ func (cr *containerReference) create(capAdd []string, capDrop []string) common.E return err } - resp, err := cr.cli.ContainerCreate(ctx, config, hostConfig, nil, platSpecs, input.Name) + // For Gitea + // network-scoped alias is supported only for containers in user defined networks + var networkingConfig *network.NetworkingConfig + 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, + }, + } + } + + resp, err := cr.cli.ContainerCreate(ctx, config, hostConfig, networkingConfig, platSpecs, input.Name) if err != nil { return fmt.Errorf("failed to create container: '%w'", err) } diff --git a/pkg/runner/job_executor.go b/pkg/runner/job_executor.go index 7dc3389..6d73bc2 100644 --- a/pkg/runner/job_executor.go +++ b/pkg/runner/job_executor.go @@ -122,12 +122,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 aa0ccfa..8909b5a 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 createSimpleContainerName(rc.Config.ContainerNamePrefix, "WORKFLOW-"+rc.Run.Workflow.Name, "JOB-"+rc.Name) } +// 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,16 @@ 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 { @@ -280,9 +293,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 := createSimpleContainerName(rc.jobContainerName(), name) + serviceContainerName := createSimpleContainerName(rc.jobContainerName(), serviceId) c := container.NewContainer(&container.NewContainerInput{ Name: serviceContainerName, WorkingDir: ext.ToContainerPath(rc.Config.Workdir), @@ -293,7 +306,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", }, @@ -305,7 +318,8 @@ func (rc *RunContext) startJobContainer() common.Executor { Platform: rc.Config.ContainerArchitecture, AutoRemove: rc.Config.AutoRemove, Options: spec.Options, - NetworkAliases: []string{name}, + NetworkMode: networkName, + NetworkAliases: []string{serviceId}, }) rc.ServiceContainers = append(rc.ServiceContainers, c) } @@ -320,47 +334,37 @@ 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), - AutoRemove: rc.Config.AutoRemove, + 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), + AutoRemove: rc.Config.AutoRemove, }) 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, @@ -474,7 +478,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 4203d00..27b332d 100644 --- a/pkg/runner/runner.go +++ b/pkg/runner/runner.go @@ -9,6 +9,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/container" "github.com/nektos/act/pkg/model" @@ -61,7 +62,7 @@ type Config struct { EventJSON string // the content of JSON file to use for event.json in containers, overrides EventPath ContainerNamePrefix string // the prefix of container name ContainerMaxLifetime time.Duration // the max lifetime of job containers - ContainerNetworkMode string // the network mode of job containers + ContainerNetworkMode docker_container.NetworkMode // the network mode of job containers (the value of --network) DefaultActionInstance string // the default actions web site PlatformPicker func(labels []string) string // platform picker, it will take precedence over Platforms if isn't nil JobLoggerLevel *log.Level // the level of job logger