chore: refactor network name creation

- networkName becomes ensureNetworkName and stores the results
  in the run context instead of returning it
- the getNetworkName and getNetworkCreated accessors are used
  instead of local variables

act/runner/job_executor.go

@@ -130,12 +130,8 @@ func newJobExecutor(info jobInfo, sf stepFactory, rc *RunContext) common.Executo
 				logger.Errorf("Error while stop job container %s: %v", rc.jobContainerName(), err)
 			}
 
-			if !rc.IsHostEnv(ctx) && rc.Config.ContainerNetworkMode == "" {
+			if !rc.IsHostEnv(ctx) && rc.getNetworkCreated(ctx) {
-				// clean network in docker mode only
+				networkName := rc.getNetworkName(ctx)
-				// 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.
-				networkName, _ := rc.networkName()
 				logger.Debugf("Cleaning up network %s for job %s", networkName, rc.jobContainerName())
 				if err := container.NewDockerNetworkRemoveExecutor(networkName)(ctx); err != nil {
 					logger.Errorf("Error while cleaning network %s: %v", networkName, err)

act/runner/run_context.go

@@ -52,6 +52,8 @@ type RunContext struct {
 	Masks               []string
 	cleanUpJobContainer common.Executor
 	caller              *caller // job calling this RunContext (reusable workflows)
+	networkName         string
+	networkCreated      bool
 }
 
 func (rc *RunContext) AddMask(mask string) {
@@ -97,14 +99,6 @@ func (rc *RunContext) jobContainerName() string {
 	return createSimpleContainerName(rc.Config.ContainerNamePrefix, "WORKFLOW-"+common.Sha256(rc.Run.Workflow.Name), "JOB-"+rc.Name)
 }
 
-// networkName return the name of the network which will be created by `act` automatically for job,
-func (rc *RunContext) networkName() (string, bool) {
-	if len(rc.Run.Job().Services) > 0 || rc.Config.ContainerNetworkMode == "" {
-		return fmt.Sprintf("%s-%s-network", rc.jobContainerName(), rc.Run.JobID), true
-	}
-	return string(rc.Config.ContainerNetworkMode), false
-}
-
 func getDockerDaemonSocketMountPath(daemonPath string) string {
 	if protoIndex := strings.Index(daemonPath, "://"); protoIndex != -1 {
 		scheme := daemonPath[:protoIndex]
@@ -388,15 +382,24 @@ func (rc *RunContext) startHostEnvironment() common.Executor {
 	}
 }
 
-func (rc *RunContext) getNetworkName(_ context.Context) (networkName string, createAndDeleteNetwork bool) {
+func (rc *RunContext) getNetworkCreated(ctx context.Context) bool {
-	// specify the network to which the container will connect when `docker create` stage. (like execute command line: docker create --network <networkName> <image>)
+	rc.ensureNetworkName(ctx)
-	networkName = string(rc.Config.ContainerNetworkMode)
+	return rc.networkCreated
-	if networkName == "" {
+}
-		// if networkName is empty string, will create a new network for the containers.
+
-		// and it will be removed after at last.
+func (rc *RunContext) getNetworkName(ctx context.Context) string {
-		networkName, createAndDeleteNetwork = rc.networkName()
+	rc.ensureNetworkName(ctx)
+	return rc.networkName
+}
+
+func (rc *RunContext) ensureNetworkName(ctx context.Context) {
+	if rc.networkName == "" {
+		rc.networkName = string(rc.Config.ContainerNetworkMode)
+		if len(rc.Run.Job().Services) > 0 || rc.networkName == "" {
+			rc.networkName = fmt.Sprintf("%s-%s-network", rc.jobContainerName(), rc.Run.JobID)
+			rc.networkCreated = true
+		}
 	}
-	return networkName, createAndDeleteNetwork
 }
 
 var sanitizeNetworkAliasRegex = regexp.MustCompile("[^a-z0-9-]")
@@ -445,7 +448,6 @@ func (rc *RunContext) prepareJobContainer(ctx context.Context) error {
 	ext := container.LinuxContainerEnvironmentExtensions{}
 	binds, mounts, validVolumes := rc.GetBindsAndMounts(ctx)
 
-	networkName, createAndDeleteNetwork := rc.getNetworkName(ctx)
 	// add service containers
 	for serviceID, spec := range rc.Run.Job().Services {
 		// interpolate env
@@ -498,7 +500,7 @@ func (rc *RunContext) prepareJobContainer(ctx context.Context) error {
 			Privileged:     rc.Config.Privileged,
 			UsernsMode:     rc.Config.UsernsMode,
 			Platform:       rc.Config.ContainerArchitecture,
-			NetworkMode:    networkName,
+			NetworkMode:    rc.getNetworkName(ctx),
 			NetworkAliases: []string{sanitizeNetworkAlias(ctx, serviceID)},
 			ExposedPorts:   exposedPorts,
 			PortBindings:   portBindings,
@@ -529,13 +531,9 @@ func (rc *RunContext) prepareJobContainer(ctx context.Context) error {
 							logger.Errorf("Error while cleaning services: %v", err)
 						}
 					}
-					if createAndDeleteNetwork {
+					if rc.getNetworkCreated(ctx) {
-						// clean network if it has been created by act
+						logger.Infof("Cleaning up network for job %s, and network name is: %s", rc.JobName, rc.getNetworkName(ctx))
-						// if using service containers
+						if err := container.NewDockerNetworkRemoveExecutor(rc.getNetworkName(ctx))(ctx); err != nil {
-						// 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, networkName)
-						if err := container.NewDockerNetworkRemoveExecutor(networkName)(ctx); err != nil {
 							logger.Errorf("Error while cleaning network: %v", err)
 						}
 					}
@@ -556,7 +554,7 @@ func (rc *RunContext) prepareJobContainer(ctx context.Context) error {
 		Env:            envList,
 		ToolCache:      rc.getToolCache(ctx),
 		Mounts:         mounts,
-		NetworkMode:    networkName,
+		NetworkMode:    rc.getNetworkName(ctx),
 		NetworkAliases: []string{sanitizeNetworkAlias(ctx, rc.Name)},
 		Binds:          binds,
 		Stdout:         logWriter,
@@ -581,7 +579,6 @@ func (rc *RunContext) startJobContainer() common.Executor {
 		if err := rc.prepareJobContainer(ctx); err != nil {
 			return err
 		}
-		networkName, _ := rc.getNetworkName(ctx)
 		networkConfig := network.CreateOptions{
 			Driver:     "bridge",
 			Scope:      "local",
@@ -591,8 +588,8 @@ func (rc *RunContext) startJobContainer() common.Executor {
 			rc.pullServicesImages(rc.Config.ForcePull),
 			rc.JobContainer.Pull(rc.Config.ForcePull),
 			rc.stopJobContainer(),
-			container.NewDockerNetworkCreateExecutor(networkName, &networkConfig).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(rc.getNetworkName(ctx), &networkConfig).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.startServiceContainers(rc.getNetworkName(ctx)),
 			rc.JobContainer.Create(rc.Config.ContainerCapAdd, rc.Config.ContainerCapDrop),
 			rc.JobContainer.Start(false),
 			rc.JobContainer.Copy(rc.JobContainer.GetActPath()+"/", &container.FileEntry{

act/runner/run_context_test.go

@@ -650,6 +650,88 @@ func TestRunContext_CreateSimpleContainerName(t *testing.T) {
 	}
 }
 
+func TestRunContext_ensureNetworkName(t *testing.T) {
+	t.Run("CreateNetworkForServices", func(t *testing.T) {
+		yaml := `
+on:
+  push:
+
+jobs:
+  job:
+    runs-on: docker
+    container:
+      image: some:image
+    services:
+      service1:
+        image: service1:image
+    steps:
+      - run: echo ok
+`
+		workflow, err := model.ReadWorkflow(strings.NewReader(yaml), true)
+		require.NoError(t, err)
+
+		rc := &RunContext{
+			Config: &Config{
+				ContainerNetworkMode: "host",
+			},
+			Run: &model.Run{
+				JobID:    "job",
+				Workflow: workflow,
+			},
+		}
+
+		rc.ensureNetworkName(t.Context())
+		assert.True(t, rc.getNetworkCreated(t.Context()))
+		assert.True(t, strings.HasPrefix(rc.getNetworkName(t.Context()), "WORKFLOW-"), rc.getNetworkName(t.Context()))
+	})
+
+	yaml := `
+on:
+  push:
+
+jobs:
+  job:
+    runs-on: docker
+    container:
+      image: some:image
+    steps:
+      - run: echo ok
+`
+	workflow, err := model.ReadWorkflow(strings.NewReader(yaml), true)
+	require.NoError(t, err)
+
+	run := &model.Run{
+		JobID:    "job",
+		Workflow: workflow,
+	}
+
+	t.Run("CreateNetworkIfEmptyNetworkMode", func(t *testing.T) {
+		rc := &RunContext{
+			Config: &Config{
+				ContainerNetworkMode: "",
+			},
+			Run: run,
+		}
+
+		rc.ensureNetworkName(t.Context())
+		assert.True(t, rc.getNetworkCreated(t.Context()))
+		assert.True(t, strings.HasPrefix(rc.getNetworkName(t.Context()), "WORKFLOW-"), rc.getNetworkName(t.Context()))
+	})
+
+	t.Run("FixedNetworkIfSetByNetworkMode", func(t *testing.T) {
+		rc := &RunContext{
+			Config: &Config{
+				ContainerNetworkMode: "host",
+			},
+			Run: run,
+		}
+
+		rc.ensureNetworkName(t.Context())
+		assert.False(t, rc.getNetworkCreated(t.Context()))
+		assert.Equal(t, "host", rc.getNetworkName(t.Context()))
+	})
+}
+
 func TestRunContext_SanitizeNetworkAlias(t *testing.T) {
 	same := "same"
 	assert.Equal(t, same, sanitizeNetworkAlias(t.Context(), same))