From df02f369a3c045ddfac8ec480993194fe8291c9f 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 | 55 ++++++++++++- pkg/container/docker_run.go | 108 ++++++++++++++----------- pkg/runner/run_context.go | 97 +++++++++++++--------- pkg/runner/runner.go | 1 + pkg/runner/runner_test.go | 2 +- pkg/runner/testdata/services/push.yaml | 1 - 6 files changed, 175 insertions(+), 89 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..54eaac45552 100644 --- a/pkg/container/docker_network.go +++ b/pkg/container/docker_network.go @@ -2,20 +2,29 @@ package container import ( "context" + "fmt" "github.com/docker/docker/api/types" "github.com/nektos/act/pkg/common" ) -func NewDockerNetworkCreateExecutor(name string) common.Executor { +func NewDockerNetworkCreateExecutor(name string, config types.NetworkCreate) 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 +34,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 { + log := common.Logger(ctx) + if _, exists, err := GetDockerNetwork(ctx, name); !exists { + log.Error(err) + return false + } + return true +} + +func GetDockerNetwork(ctx context.Context, name string) (types.NetworkResource, bool, error) { + log := common.Logger(ctx) + + cli, err := GetDockerClient(ctx) + if err != nil { + log.Debug(err) + return types.NetworkResource{}, false, err + } + + res, err := cli.NetworkInspect(ctx, name, types.NetworkInspectOptions{}) + if err != nil { + if err.Error() == fmt.Sprintf("Error: No such network: %s", name) { + log.Error(err) + return types.NetworkResource{}, false, err + } + return types.NetworkResource{}, false, err + } + + return res, true, nil +} diff --git a/pkg/container/docker_run.go b/pkg/container/docker_run.go index d163228a5d3..15081785a5e 100644 --- a/pkg/container/docker_run.go +++ b/pkg/container/docker_run.go @@ -40,6 +40,7 @@ type NewContainerInput struct { Image string Username string Password string + Hostname string Entrypoint []string Cmd []string WorkingDir string @@ -53,7 +54,6 @@ type NewContainerInput struct { Privileged bool UsernsMode string Platform string - Hostname string } // FileEntry is a file to copy to a container @@ -78,6 +78,7 @@ type Container interface { UpdateFromPath(env *map[string]string) common.Executor Remove() common.Executor Close() common.Executor + SetContainerNetworkMode(mode string) common.Executor } // NewContainer creates a reference to a container @@ -106,63 +107,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 { @@ -192,7 +203,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), @@ -247,7 +258,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) } } @@ -332,15 +343,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, - Hostname: input.Hostname, } + if len(input.Cmd) > 0 { config.Cmd = input.Cmd } + if len(input.Entrypoint) > 0 { config.Entrypoint = input.Entrypoint } @@ -379,6 +392,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 6a505b7426b..b3f0908ec1e --- a/pkg/runner/run_context.go +++ b/pkg/runner/run_context.go @@ -11,6 +11,7 @@ import ( "runtime" "strings" + "github.com/docker/docker/api/types" "github.com/google/shlex" "github.com/spf13/pflag" @@ -99,6 +100,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() hostname := rc.hostname() @@ -116,6 +122,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) @@ -125,33 +132,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{ @@ -162,6 +172,7 @@ func (rc *RunContext) startJobContainer() common.Executor { Username: rc.Config.Secrets["DOCKER_USERNAME"], Password: rc.Config.Secrets["DOCKER_PASSWORD"], Name: name, + Hostname: hostname, Env: envList, Mounts: mounts, // NetworkMode: "host", @@ -171,9 +182,12 @@ func (rc *RunContext) startJobContainer() common.Executor { Privileged: rc.Config.Privileged, UsernsMode: rc.Config.UsernsMode, Platform: rc.Config.ContainerArchitecture, - Hostname: hostname, }) + if rc.Run.Job().Services == nil { + rc.JobContainer.SetContainerNetworkMode("host") + } + var copyWorkspace bool var copyToPath string if !rc.Config.BindWorkdir { @@ -181,18 +195,21 @@ 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, types.NetworkCreate{}).IfBool(!container.DockerNetworkExists(ctx, networkName)), + rc.startServiceContainers(networkName), + ).IfBool(rc.Run.Job().Services != nil || rc.Run.Job().Container() != nil), rc.JobContainer.Create(rc.Config.ContainerCapAdd, rc.Config.ContainerCapDrop), rc.JobContainer.Start(false), rc.JobContainer.UpdateFromImageEnv(&rc.Env), - rc.JobContainer.ConnectToNetwork(networkName), + common.NewPipelineExecutor( + rc.JobContainer.ConnectToNetwork(networkName), + ).IfBool(rc.Run.Job().Services != nil || rc.Run.Job().Container() != 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), @@ -213,9 +230,9 @@ func (rc *RunContext) startJobContainer() common.Executor { } } -func (rc *RunContext) createNetwork(name string) common.Executor { +func (rc *RunContext) createNetwork(name string, config types.NetworkCreate) common.Executor { return func(ctx context.Context) error { - return container.NewDockerNetworkCreateExecutor(name)(ctx) + return container.NewDockerNetworkCreateExecutor(name, config)(ctx) } } @@ -235,8 +252,11 @@ 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)), + rc.removeNetwork(rc.JobNetworkName()).IfBool((rc.Run.Job().Services != nil || rc.Run.Job().Container() != nil) && !rc.Config.ReuseNetworks && container.DockerNetworkExists(ctx, rc.JobNetworkName())), + )(ctx) } return nil } @@ -251,6 +271,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) @@ -528,6 +549,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 343a4b728ea..1839d082aba 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 ad10a1a6391..ad7dda6f01a 100644 --- a/pkg/runner/runner_test.go +++ b/pkg/runner/runner_test.go @@ -185,7 +185,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: