feat: [PIPE-29213]: Changes for local runner tasks being able to opt out of docker setup (#403)

* 7e34d5 Changes for local runner tasks being able to opt out of docker setup

Author: vcalasansh

engine/docker/docker.go

@@ -55,15 +55,12 @@ const (
 	running                          = "running"
 )
 
-var (
-	globalDockerClient     *client.Client
-	globalDockerClientLock sync.Mutex
-)
-
 // Opts configures the Docker engine.
 type Opts struct {
-	HidePull          bool
-	ReuseDockerClient bool
+	HidePull bool
+	// Callers can pass a non-nil client.APIClient here, and it
+	// will be used instead of creating a new docker client
+	DockerClient client.APIClient
 }
 
 // Docker implements a Docker pipeline engine.
@@ -95,20 +92,14 @@ func New(client client.APIClient, opts Opts) *Docker {
 // NewEnv returns a new Engine from the environment.
 func NewEnv(opts Opts) (*Docker, error) {
 	var cli client.APIClient
-	var err error
-	if opts.ReuseDockerClient {
-		globalDockerClientLock.Lock()
-		defer globalDockerClientLock.Unlock()
-		// Try to (re)initialize if not set
-		if globalDockerClient == nil {
-			globalDockerClient, err = client.NewClientWithOpts(client.FromEnv)
-		}
-		cli = globalDockerClient
+	if opts.DockerClient != nil {
+		cli = opts.DockerClient
 	} else {
+		var err error
 		cli, err = client.NewClientWithOpts(client.FromEnv)
-	}
-	if err != nil {
-		return nil, err
+		if err != nil {
+			return nil, err
+		}
 	}
 	return New(cli, opts), nil
 }
@@ -143,36 +134,6 @@ func (e *Docker) Setup(ctx context.Context, pipelineConfig *spec.PipelineConfig)
 	}
 
 	err := e.createNetworkWithRetries(ctx, pipelineConfig)
-
-	// launches the inernal setup steps
-	// for _, step := range pipelineConfig.Internal {
-	// 	if err := e.create(ctx, spec, step, ioutil.Discard); err != nil {
-	// 		logger.FromContext(ctx).
-	// 			WithError(err).
-	// 			WithField("container", step.ID).
-	// 			Errorln("cannot create tmate container")
-	// 		return err
-	// 	}
-	// 	if err := e.start(ctx, step.ID); err != nil {
-	// 		logger.FromContext(ctx).
-	// 			WithError(err).
-	// 			WithField("container", step.ID).
-	// 			Errorln("cannot start tmate container")
-	// 		return err
-	// 	}
-	// 	if !step.Detach {
-	// 		// the internal containers perform short-lived tasks
-	// 		// and should not require > 1 minute to execute.
-	// 		//
-	// 		// just to be on the safe side we apply a timeout to
-	// 		// ensure we never block pipeline execution because we
-	// 		// are waiting on an internal task.
-	// 		ctx, cancel := context.WithTimeout(ctx, time.Minute)
-	// 		defer cancel()
-	// 		e.wait(ctx, step.ID)
-	// 	}
-	// }
-
 	return errors.TrimExtraInfo(err)
 }
 

engine/pids/pidfile.go

@@ -150,18 +150,13 @@ func AppendPIDToFile(pid int, path string) error {
 	return nil
 }
 
-// KillProcessesFromFile forcefully terminates all processes with PIDs found in a file.
-// It takes a path to the file containing the PIDs and a timeout duration.
+// KillProcessesFromFile forcefully terminates all processes with the passed PIDs.
+// It takes a list of PIDs and a timeout duration.
 // It will return an error if it fails to kill any processes.
-// The file can be empty, and the function will return nil in that case.
+// The PID list can be empty, and the function will return nil in that case.
 // The function is goroutine-safe and will wait for all goroutines to finish
 // before returning.
-func KillProcessesFromFile(ctx context.Context, path string, timeout time.Duration) error {
-	pids, err := ReadPIDsFromFile(path)
-	if err != nil {
-		return fmt.Errorf("failed to read PIDs from file: %w", err)
-	}
-
+func KillProcesses(ctx context.Context, pids []int, timeout time.Duration) error {
 	if len(pids) == 0 {
 		fmt.Println("No valid PIDs found in file")
 		return nil

pipeline/runtime/step_executor_stateless.go

@@ -8,6 +8,7 @@ import (
 	"context"
 	"io"
 
+	"github.com/docker/docker/client"
 	"github.com/harness/lite-engine/api"
 	"github.com/harness/lite-engine/engine"
 	"github.com/harness/lite-engine/engine/docker"
@@ -39,6 +40,7 @@ func (e *StepExecutorStateless) Run(
 	ctx context.Context,
 	r *api.StartStepRequest,
 	cfg *spec.PipelineConfig,
+	dockerClient client.APIClient,
 	writer logstream.Writer,
 ) (api.VMTaskExecutionResponse, error) {
 	if r.ID == "" {
@@ -47,7 +49,7 @@ func (e *StepExecutorStateless) Run(
 
 	e.stepStatus = StepStatus{Status: Running}
 
-	state, outputs, envs, artifact, outputV2, telemetryData, optimizationState, stepErr := e.executeStep(ctx, r, cfg, writer)
+	state, outputs, envs, artifact, outputV2, telemetryData, optimizationState, stepErr := e.executeStep(ctx, r, cfg, dockerClient, writer)
 	e.stepStatus = StepStatus{Status: Complete, State: state, StepErr: stepErr, Outputs: outputs, Envs: envs,
 		Artifact: artifact, OutputV2: outputV2, OptimizationState: optimizationState, TelemetryData: telemetryData}
 	pollResponse := convertStatus(e.stepStatus)
@@ -58,11 +60,12 @@ func (e *StepExecutorStateless) executeStep( //nolint:gocritic
 	ctx context.Context,
 	r *api.StartStepRequest,
 	cfg *spec.PipelineConfig,
+	dockerClient client.APIClient,
 	writer logstream.Writer,
 ) (*runtime.State, map[string]string,
 	map[string]string, []byte, []*api.OutputV2, *types.TelemetryData, string, error) {
 	runFunc := func(ctx context.Context, step *spec.Step, output io.Writer, isDrone bool, isHosted bool) (*runtime.State, error) {
-		return engine.RunStep(ctx, engine.Opts{Opts: docker.Opts{ReuseDockerClient: true}}, step, output, cfg, isDrone, isHosted)
+		return engine.RunStep(ctx, engine.Opts{Opts: docker.Opts{DockerClient: dockerClient}}, step, output, cfg, isDrone, isHosted)
 	}
 	// Temporary: this should be removed once we have a better way of handling test intelligence.
 	tiConfig := getTiCfg(&r.TIConfig, &r.MtlsConfig)