Add support for service containers (#1949)

* Support services (#42) Removed createSimpleContainerName and AutoRemove flag Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com> Co-authored-by: Jason Song <i@wolfogre.com> Reviewed-on: https://gitea.com/gitea/act/pulls/42 Reviewed-by: Jason Song <i@wolfogre.com> Co-authored-by: Zettat123 <zettat123@gmail.com> Co-committed-by: Zettat123 <zettat123@gmail.com> * Support services options (#45) Reviewed-on: https://gitea.com/gitea/act/pulls/45 Reviewed-by: Lunny Xiao <xiaolunwen@gmail.com> Co-authored-by: Zettat123 <zettat123@gmail.com> Co-committed-by: Zettat123 <zettat123@gmail.com> * Support intepolation for `env` of `services` (#47) Reviewed-on: https://gitea.com/gitea/act/pulls/47 Reviewed-by: Lunny Xiao <xiaolunwen@gmail.com> Co-authored-by: Zettat123 <zettat123@gmail.com> Co-committed-by: Zettat123 <zettat123@gmail.com> * 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 <i@wolfogre.com> Reviewed-on: https://gitea.com/gitea/act/pulls/51 Reviewed-by: Jason Song <i@wolfogre.com> Reviewed-by: Lunny Xiao <xiaolunwen@gmail.com> Co-authored-by: Zettat123 <zettat123@gmail.com> Co-committed-by: Zettat123 <zettat123@gmail.com> * Add ContainerMaxLifetime and ContainerNetworkMode options from: https://gitea.com/gitea/act/commit/b9c20dcaa43899cb3bb327619d447248303170e0 * 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 `<custom-network>`. Because, the network created by `act` is also user defined network.), will also specify alias by `--network-alias`. The alias of service is `<service-id>`. So we can be access service container by `<service-id>:<port>` 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 <container-name>` 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 <i@wolfogre.com> Reviewed-on: https://gitea.com/gitea/act/pulls/56 Reviewed-by: Jason Song <i@wolfogre.com> Co-authored-by: sillyguodong <gedong_1994@163.com> Co-committed-by: sillyguodong <gedong_1994@163.com> * 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.<job_id>.container.volumes](https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#jobsjob_idcontainervolumes) - [jobs.<job_id>.services.<service_id>.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` - `<container-name>` and `<container-name>-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 <i@wolfogre.com> Reviewed-on: https://gitea.com/gitea/act/pulls/60 Reviewed-by: Jason Song <i@wolfogre.com> Co-authored-by: Zettat123 <zettat123@gmail.com> Co-committed-by: Zettat123 <zettat123@gmail.com> * Remove ContainerMaxLifetime; fix lint * Remove unused ValidVolumes * Remove ConnectToNetwork * Add docker stubs * Close docker clients to prevent file descriptor leaks * 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 <zettat123@gmail.com> Co-committed-by: Zettat123 <zettat123@gmail.com> * Move service container and network cleanup to rc.cleanUpJobContainer * Add --network flag; default to host if not using service containers or set explicitly * Correctly close executor to prevent fd leak * Revert to tail instead of full path * fix network duplication * backport networkingConfig for aliaes * don't hardcode netMode host * Convert services test to table driven tests * Add failing tests for services * Expose service container ports onto the host * Set container network mode in artifacts server test to host mode * Log container network mode when creating/starting a container * fix: Correctly handle ContainerNetworkMode * fix: missing service container network * 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. * Remove networks only if no active endpoints exist * Ensure job containers are stopped before starting a new job * fix: go build -tags WITHOUT_DOCKER --------- Co-authored-by: Zettat123 <zettat123@gmail.com> Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com> Co-authored-by: Jason Song <i@wolfogre.com> Co-authored-by: sillyguodong <gedong_1994@163.com> Co-authored-by: ChristopherHX <christopher.homberger@web.de> Co-authored-by: ZauberNerd <zaubernerd@zaubernerd.de>
nektos · Oct 19, 2023 · ceeb6c1 · ceeb6c1
1 parent ace4cd4
commit ceeb6c1
Show file tree

Hide file tree

Showing 14 changed files with 469 additions and 100 deletions.
diff --git 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
@@ -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/container/container_types.go b/pkg/container/container_types.go
@@ -4,28 +4,32 @@ import (
 	"context"
 	"io"
 
+	"github.com/docker/go-connections/nat"
 	"github.com/nektos/act/pkg/common"
 )
 
 // 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
+	ExposedPorts   nat.PortSet
+	PortBindings   nat.PortMap
 }
 
 // FileEntry is a file to copy to a container

diff --git a/pkg/container/docker_network.go b/pkg/container/docker_network.go
@@ -0,0 +1,79 @@
+//go:build !(WITHOUT_DOCKER || !(linux || darwin || windows))
+
+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
+		}
+		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",
+		})
+		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
+		}
+		defer cli.Close()
+
+		// 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 {
+				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)
+				}
+			}
+		}
+
+		return err
+	}
+}
diff --git 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"
@@ -66,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(),
@@ -78,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(),
@@ -346,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)
 		}
 	}
 
@@ -391,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)
 
@@ -430,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)
 
@@ -445,7 +448,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)
 		}

diff --git 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/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
+	}
+}
diff --git 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,12 @@ 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 container for job %s", rc.JobName)
+			if err = info.stopContainer()(ctx); err != nil {
+				logger.Errorf("Error while stop job container: %v", err)
+			}
 		}
 		setJobResult(ctx, info, rc, jobError == nil)
 		setJobOutputs(ctx, rc)