batches: check that docker is running when running src batch (#844)

* check docker version before batch

* remove debug statement

* create wrapper for fastcontext

* move executeFastCommand into context.go

* cleanup

Author: BolajiOlajide

cmd/src/batch_common.go

@@ -268,9 +268,13 @@ func executeBatchSpec(ctx context.Context, ui ui.ExecUI, opts executeBatchSpecOp
 		return err
 	}
 
-	// In the past, we checked `docker version`, but now we retrieve the number
-	// of CPUs, since we need that anyway and it performs the same check (is
-	// Docker working _at all_?).
+	// In the past, we relied on `getBatchParallelism` to ascertain if docker is running,
+	// however, we don't always check for the number of CPUs (especially when the -j parallelis)
+	// flag is passed. This is a more explicit check to confirm docker is working.
+	if err := docker.CheckVersion(ctx); err != nil {
+		return err
+	}
+
 	parallelism, err := getBatchParallelism(ctx, opts.flags.parallelism)
 	if err != nil {
 		return err
@@ -547,7 +551,7 @@ func parseBatchSpec(ctx context.Context, file string, svc *service.Service, isRe
 func checkExecutable(cmd string, args ...string) error {
 	if err := exec.Command(cmd, args...).Run(); err != nil {
 		return fmt.Errorf(
-			"failed to execute \"%s %s\":\n\t%s\n\n'src batch' require %q to be available.",
+			"failed to execute \"%s %s\":\n\t%s\n\n'src batch' requires %q to be available.",
 			cmd,
 			strings.Join(args, " "),
 			err,

internal/batches/docker/context.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"fmt"
 	"os"
+	"os/exec"
 	"sync"
 	"time"
 
@@ -94,3 +95,20 @@ func fastCommandTimeout() (time.Duration, error) {
 
 	return fastCommandTimeoutData.timeout, fastCommandTimeoutData.err
 }
+
+// executeFastCommand creates a fastCommandContext used to execute docker commands
+// with a timeout for docker commands that are supposed to be fast (e.g docker info).
+func executeFastCommand(ctx context.Context, args ...string) ([]byte, error) {
+	dctx, cancel, err := withFastCommandContext(ctx)
+	if err != nil {
+		return nil, err
+	}
+	defer cancel()
+
+	out, err := exec.CommandContext(dctx, "docker", args...).CombinedOutput()
+	if errors.IsDeadlineExceeded(err) || errors.IsDeadlineExceeded(dctx.Err()) {
+		return nil, newFastCommandTimeoutError(dctx, args...)
+	}
+
+	return out, err
+}

internal/batches/docker/version.go

@@ -0,0 +1,21 @@
+package docker
+
+import (
+	"context"
+	"fmt"
+)
+
+// CheckVersion is used to check if docker is running. We use this method instead of
+// checkExecutable (https://sourcegraph.com/github.com/sourcegraph/src-cli@main/-/blob/cmd/src/batch_common.go?L547%3A6=&popover=pinned)
+// to prevent a case where docker commands take too long and results in `src-cli` freezing for some users.
+func CheckVersion(ctx context.Context) error {
+	_, err := executeFastCommand(ctx, "version")
+	if err != nil {
+		return fmt.Errorf(
+			"failed to execute \"docker version\":\n\t%s\n\n'src batch' requires \"docker\" to be available.",
+			err,
+		)
+	}
+
+	return nil
+}