From 7d0173f70fe5857df43c679ae7ce5af9c5fcfd8b Mon Sep 17 00:00:00 2001 From: hackercat Date: Tue, 4 May 2021 16:55:45 +0000 Subject: [PATCH] fix: rework services --- pkg/container/docker_network.go | 56 ++++++++++++- pkg/container/docker_run.go | 106 ++++++++++++++----------- pkg/runner/run_context.go | 94 ++++++++++++++-------- pkg/runner/runner.go | 1 + pkg/runner/runner_test.go | 2 +- pkg/runner/testdata/services/push.yaml | 1 - 6 files changed, 174 insertions(+), 86 deletions(-) mode change 100755 => 100644 pkg/runner/run_context.go diff --git a/pkg/container/docker_network.go b/pkg/container/docker_network.go index d7be3dacc46..f7e6dff8e39 100644 --- a/pkg/container/docker_network.go +++ b/pkg/container/docker_network.go @@ -2,20 +2,30 @@ package container import ( "context" + "fmt" "github.com/docker/docker/api/types" + // "github.com/docker/docker/api/types/network" "github.com/nektos/act/pkg/common" ) -func NewDockerNetworkCreateExecutor(name string) common.Executor { +func NewDockerNetworkCreateExecutor(name, subnet, ipRange, gateway string) common.Executor { return func(ctx context.Context) error { + if common.Dryrun(ctx) { + return nil + } + cli, err := GetDockerClient(ctx) if err != nil { return err } - _, err = cli.NetworkCreate(ctx, name, types.NetworkCreate{}) - if err != nil { + if exists := DockerNetworkExists(ctx, name); exists { + return nil + // return fmt.Errorf("network '%s' already exists", name) + } + + if _, err = cli.NetworkCreate(ctx, name, types.NetworkCreate{}); err != nil { return err } @@ -25,12 +35,50 @@ func NewDockerNetworkCreateExecutor(name string) common.Executor { func NewDockerNetworkRemoveExecutor(name string) common.Executor { return func(ctx context.Context) error { + if common.Dryrun(ctx) { + return nil + } + cli, err := GetDockerClient(ctx) if err != nil { return err } - cli.NetworkRemove(ctx, name) + err = cli.NetworkRemove(ctx, name) + if err != nil { + return err + } + return nil } } + +func DockerNetworkExists(ctx context.Context, name string) bool { + if exists, err := dockerNetworkExists(ctx, name); err != nil && !exists || err == nil && !exists { + return false + } else if exists && err == nil || exists && err != nil { + return true + } + return false +} + +func dockerNetworkExists(ctx context.Context, name string) (bool, error) { + log := common.Logger(ctx) + + cli, err := GetDockerClient(ctx) + if err != nil { + log.Debug(err) + return false, err + } + + _, err = cli.NetworkInspect(ctx, name, types.NetworkInspectOptions{}) + if err != nil { + if err.Error() == fmt.Sprintf("Error: No such %s: %s", "network", name) { + log.Debug(err) + return false, err + } + return false, err + } + + return true, nil +} diff --git a/pkg/container/docker_run.go b/pkg/container/docker_run.go index a6de6d61649..4aba3dd3bc0 100644 --- a/pkg/container/docker_run.go +++ b/pkg/container/docker_run.go @@ -39,6 +39,7 @@ type NewContainerInput struct { Image string Username string Password string + Hostname string Entrypoint []string Cmd []string WorkingDir string @@ -74,6 +75,7 @@ type Container interface { UpdateFromEnv(srcPath string, env *map[string]string) common.Executor UpdateFromPath(env *map[string]string) common.Executor Remove() common.Executor + SetContainerNetworkMode(mode string) common.Executor } // NewContainer creates a reference to a container @@ -102,63 +104,73 @@ func supportsContainerImagePlatform(cli *client.Client) bool { return constraint.Check(sv) } +func (cr *containerReference) SetContainerNetworkMode(mode string) common.Executor { + return common.NewPipelineExecutor( + common.NewDebugExecutor("Changed network mode for container '%s' from '%s' to '%s'", cr.input.Name, cr.input.NetworkMode, mode), + func(ctx context.Context) error { + cr.input.NetworkMode = mode + return nil + }, + ) +} + 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), - ).IfNot(common.Dryrun), - ) + return common.NewPipelineExecutor( + common.NewInfoExecutor("%sdocker network connect %s %s", logPrefix, name, cr.input.Name), + common.NewPipelineExecutor( + cr.connect(), + cr.find(), + cr.connectToNetwork(name), + ).IfNot(common.Dryrun), + ) } 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). - Then( - common.NewPipelineExecutor( - cr.connect(), - cr.find(), - cr.create(capAdd, capDrop), - ).IfNot(common.Dryrun), - ) + return common.NewPipelineExecutor( + 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), + common.NewPipelineExecutor( + cr.connect(), + cr.find(), + cr.create(capAdd, capDrop), + ).IfNot(common.Dryrun), + ) } 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). - Then( - common.NewPipelineExecutor( - cr.connect(), - cr.find(), - cr.attach().IfBool(attach), - cr.start(), - cr.wait().IfBool(attach), - ).IfNot(common.Dryrun), - ) + return common.NewPipelineExecutor( + 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), + common.NewPipelineExecutor( + cr.connect(), + cr.find(), + cr.attach().IfBool(attach), + cr.start(), + cr.wait().IfBool(attach), + ).IfNot(common.Dryrun), + ) } func (cr *containerReference) Pull(forcePull bool) common.Executor { - return common. - NewInfoExecutor("%sdocker pull image=%s platform=%s username=%s forcePull=%t", logPrefix, cr.input.Image, cr.input.Platform, cr.input.Username, forcePull). - Then( - NewDockerPullExecutor(NewDockerPullExecutorInput{ - Image: cr.input.Image, - ForcePull: forcePull, - Platform: cr.input.Platform, - Username: cr.input.Username, - Password: cr.input.Password, - }), - ) + return common.NewPipelineExecutor( + common.NewInfoExecutor("%sdocker pull image=%s platform=%s username=%s forcePull=%t", logPrefix, cr.input.Image, cr.input.Platform, cr.input.Username, forcePull), + NewDockerPullExecutor(NewDockerPullExecutorInput{ + Image: cr.input.Image, + ForcePull: forcePull, + Platform: cr.input.Platform, + Username: cr.input.Username, + Password: cr.input.Password, + }), + ) } func (cr *containerReference) Copy(destPath string, files ...*FileEntry) common.Executor { return common.NewPipelineExecutor( - cr.connect(), - cr.find(), - cr.copyContent(destPath, files...), - ).IfNot(common.Dryrun) + common.NewInfoExecutor("%sdocker cp ... destPath=%s", logPrefix, destPath), + common.NewPipelineExecutor( + cr.connect(), + cr.find(), + cr.copyContent(destPath, files...), + ).IfNot(common.Dryrun), + ) } func (cr *containerReference) CopyDir(destPath string, srcPath string, useGitIgnore bool) common.Executor { @@ -184,7 +196,7 @@ func (cr *containerReference) UpdateFromPath(env *map[string]string) common.Exec func (cr *containerReference) Exec(command []string, env map[string]string, user, workdir string) common.Executor { return common.NewPipelineExecutor( - common.NewInfoExecutor("%sdocker exec cmd=[%s] user=%s workdir=%s", logPrefix, strings.Join(command, " "), user, workdir), + common.NewInfoExecutor("%sdocker exec cmd=%v user=%s workdir=%s", logPrefix, command, user, workdir), cr.connect(), cr.find(), cr.exec(command, env, user, workdir), @@ -239,7 +251,7 @@ func GetDockerClient(ctx context.Context) (*client.Client, error) { func (cr *containerReference) connectToNetwork(name string) common.Executor { return func(ctx context.Context) error { - return cr.cli.NetworkConnect(ctx, name, cr.input.Name, nil) + return cr.cli.NetworkConnect(ctx, name, cr.id, nil) } } @@ -314,14 +326,17 @@ func (cr *containerReference) create(capAdd []string, capDrop []string) common.E input := cr.input config := &container.Config{ + Hostname: input.Hostname, Image: input.Image, WorkingDir: input.WorkingDir, Env: input.Env, Tty: isTerminal, } + if len(input.Cmd) > 0 { config.Cmd = input.Cmd } + if len(input.Entrypoint) > 0 { config.Entrypoint = input.Entrypoint } @@ -360,6 +375,7 @@ func (cr *containerReference) create(capAdd []string, capDrop []string) common.E if err != nil { return errors.WithStack(err) } + logger.Debugf("Created container name=%s id=%v from image %v (platform: %s)", input.Name, resp.ID, input.Image, input.Platform) logger.Debugf("ENV ==> %v", input.Env) diff --git a/pkg/runner/run_context.go b/pkg/runner/run_context.go old mode 100755 new mode 100644 index a3cd1ac0b5f..8311037d4f9 --- a/pkg/runner/run_context.go +++ b/pkg/runner/run_context.go @@ -95,6 +95,11 @@ func (rc *RunContext) GetBindsAndMounts() ([]string, map[string]string) { return binds, mounts } +// JobNetworkName returns formatted network name used in main container and its services +func (rc *RunContext) JobNetworkName() string { + return createNetworkName("act", rc.Run.Workflow.Name, rc.Run.JobID, rc.Name) +} + func (rc *RunContext) startJobContainer() common.Executor { image := rc.platformImage() @@ -111,6 +116,7 @@ func (rc *RunContext) startJobContainer() common.Executor { common.Logger(ctx).Infof("\U0001f680 Start image=%s", image) name := rc.jobContainerName() + networkName := rc.JobNetworkName() envList := make([]string, 0) @@ -120,33 +126,36 @@ func (rc *RunContext) startJobContainer() common.Executor { 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)) - } + if rc.Run.Job().Services != nil { + // 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)) + } - mnt := mounts - mnt[name] = filepath.Dir(rc.Config.ContainerWorkdir()) - - c := container.NewContainer(&container.NewContainerInput{ - Name: name, - WorkingDir: rc.Config.ContainerWorkdir(), - Image: spec.Image, - Username: rc.Config.Secrets["DOCKER_USERNAME"], - Password: rc.Config.Secrets["DOCKER_PASSWORD"], - Env: mergedEnv, - Mounts: mnt, - // NetworkMode: "host", - Binds: binds, - Stdout: logWriter, - Stderr: logWriter, - Privileged: rc.Config.Privileged, - UsernsMode: rc.Config.UsernsMode, - Platform: rc.Config.ContainerArchitecture, - }) - rc.ServiceContainers = append(rc.ServiceContainers, c) + mnt := mounts + mnt[name] = filepath.Dir(rc.Config.ContainerWorkdir()) + + c := container.NewContainer(&container.NewContainerInput{ + Name: name, + Hostname: name, + WorkingDir: rc.Config.ContainerWorkdir(), + Image: spec.Image, + Username: rc.Config.Secrets["DOCKER_USERNAME"], + Password: rc.Config.Secrets["DOCKER_PASSWORD"], + Env: mergedEnv, + Mounts: mnt, + //NetworkMode: "bridge", + Binds: binds, + Stdout: logWriter, + Stderr: logWriter, + Privileged: rc.Config.Privileged, + UsernsMode: rc.Config.UsernsMode, + Platform: rc.Config.ContainerArchitecture, + }) + rc.ServiceContainers = append(rc.ServiceContainers, c) + } } rc.JobContainer = container.NewContainer(&container.NewContainerInput{ @@ -157,6 +166,7 @@ func (rc *RunContext) startJobContainer() common.Executor { Username: rc.Config.Secrets["DOCKER_USERNAME"], Password: rc.Config.Secrets["DOCKER_PASSWORD"], Name: name, + Hostname: name, Env: envList, Mounts: mounts, // NetworkMode: "host", @@ -168,6 +178,10 @@ func (rc *RunContext) startJobContainer() common.Executor { Platform: rc.Config.ContainerArchitecture, }) + if rc.Run.Job().Services == nil { + rc.JobContainer.SetContainerNetworkMode("host") + } + var copyWorkspace bool var copyToPath string if !rc.Config.BindWorkdir { @@ -175,17 +189,20 @@ func (rc *RunContext) startJobContainer() common.Executor { copyToPath = filepath.Join(rc.Config.ContainerWorkdir(), copyToPath) } - networkName := fmt.Sprintf("act-%s-network", rc.Run.JobID) return common.NewPipelineExecutor( rc.JobContainer.Pull(rc.Config.ForcePull), rc.stopServiceContainers(), rc.stopJobContainer(), - rc.removeNetwork(networkName), - rc.createNetwork(networkName), - rc.startServiceContainers(networkName), + common.NewPipelineExecutor( + // rc.removeNetwork(networkName).IfBool(container.DockerNetworkExists(ctx, networkName)), + rc.createNetwork(networkName, "", "", "").IfBool(!container.DockerNetworkExists(ctx, networkName)), + rc.startServiceContainers(networkName), + ).IfBool(rc.Run.Job().Services != nil), rc.JobContainer.Create(rc.Config.ContainerCapAdd, rc.Config.ContainerCapDrop), rc.JobContainer.Start(false), - rc.JobContainer.ConnectToNetwork(networkName), + common.NewPipelineExecutor( + rc.JobContainer.ConnectToNetwork(networkName), + ).IfBool(rc.Run.Job().Services != nil), rc.JobContainer.UpdateFromEnv("/etc/environment", &rc.Env), rc.JobContainer.Exec([]string{"mkdir", "-m", "0777", "-p", ActPath}, rc.Env, "root", ""), rc.JobContainer.CopyDir(copyToPath, rc.Config.Workdir+string(filepath.Separator)+".", rc.Config.UseGitIgnore).IfBool(copyWorkspace), @@ -206,9 +223,9 @@ func (rc *RunContext) startJobContainer() common.Executor { } } -func (rc *RunContext) createNetwork(name string) common.Executor { +func (rc *RunContext) createNetwork(name, subnet, ipRange, gateway string) common.Executor { return func(ctx context.Context) error { - return container.NewDockerNetworkCreateExecutor(name)(ctx) + return container.NewDockerNetworkCreateExecutor(name, subnet, ipRange, gateway)(ctx) } } @@ -228,8 +245,10 @@ func (rc *RunContext) execJobContainer(cmd []string, env map[string]string, user func (rc *RunContext) stopJobContainer() common.Executor { return func(ctx context.Context) error { if rc.JobContainer != nil && !rc.Config.ReuseContainers { - return rc.JobContainer.Remove(). - Then(container.NewDockerVolumeRemoveExecutor(rc.jobContainerName(), false))(ctx) + return common.NewPipelineExecutor( + rc.stopServiceContainers(), + rc.JobContainer.Remove().Then(container.NewDockerVolumeRemoveExecutor(rc.jobContainerName(), false)), + )(ctx) } return nil } @@ -244,6 +263,7 @@ func (rc *RunContext) startServiceContainers(networkName string) common.Executor c.Create([]string{}, []string{}), c.Start(false), c.ConnectToNetwork(networkName), + c.Start(false), )) } return common.NewParallelExecutor(execs...)(ctx) @@ -494,6 +514,10 @@ func createContainerName(parts ...string) string { return strings.ReplaceAll(strings.Trim(strings.Join(name, "-"), "-"), "--", "-") } +func createNetworkName(parts ...string) string { + return strings.ReplaceAll(strings.Trim(strings.Join(parts, "_"), "-"), "--", "-") +} + func trimToLen(s string, l int) string { if l < 0 { l = 0 diff --git a/pkg/runner/runner.go b/pkg/runner/runner.go index f34f9e80a33..13d57d0c10c 100644 --- a/pkg/runner/runner.go +++ b/pkg/runner/runner.go @@ -28,6 +28,7 @@ type Config struct { 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 + ReuseNetworks bool // reuse networks ForcePull bool // force pulling of the image, even if already present LogOutput bool // log the output from docker run Env map[string]string // env for containers diff --git a/pkg/runner/runner_test.go b/pkg/runner/runner_test.go index c02a4962cbc..4b3fce1c922 100644 --- a/pkg/runner/runner_test.go +++ b/pkg/runner/runner_test.go @@ -183,7 +183,7 @@ func TestRunWithService(t *testing.T) { ctx := context.Background() platforms := map[string]string{ - "ubuntu-latest": "node:12.20.1-buster-slim", + "ubuntu-latest": baseImage, } workflowPath := "services" diff --git a/pkg/runner/testdata/services/push.yaml b/pkg/runner/testdata/services/push.yaml index f6ca7bc4c50..8ffd2b1c105 100644 --- a/pkg/runner/testdata/services/push.yaml +++ b/pkg/runner/testdata/services/push.yaml @@ -2,7 +2,6 @@ name: services on: push jobs: services: - name: Reproduction of failing Services interpolation runs-on: ubuntu-latest services: postgres: