Fix parallel dind's (#235)

* Working support for earthy-in-earthly (eine).

* Add test for parallel loads. Fix race condition in dind creation in parallel.

* Performance improvement: reuse solves if they are for the same targets or docker tags.

* Move the dockerd wrapper into an .sh file.

* Fix dockerd start lock being held for too long.

Co-authored-by: Vlad A. Ionescu <vladaionescu@users.noreply.github.com>

builder/builder.go

@@ -182,7 +182,7 @@ func (b *Builder) buildCommon(ctx context.Context, mts *earthfile2llb.MultiTarge
 			existingValue, alreadyExists := localDirs[key]
 			if alreadyExists && existingValue != value {
 				return "", nil, fmt.Errorf(
-					"Inconsistent local dirs. For dir entry %s found both %s and %s",
+					"inconsistent local dirs. For dir entry %s found both %s and %s",
 					key, value, existingValue)
 			}
 			localDirs[key] = value

buildkitd/Earthfile

@@ -17,6 +17,7 @@ buildkitd:
 
     COPY ../+shellrepeater/shellrepeater /usr/bin/shellrepeater
     COPY ../+debugger/earth_debugger /usr/bin/earth_debugger
+    COPY ./dockerd-wrapper.sh /var/earthly/dockerd-wrapper.sh
 
     ENV EARTHLY_RESET_TMP_DIR=false
     ENV EARTHLY_TMP_DIR=/tmp/earthly

buildkitd/buildkitd.go

@@ -204,11 +204,20 @@ func Start(ctx context.Context, image string, settings Settings, reset bool) err
 		"-e", fmt.Sprintf("ENABLE_LOOP_DEVICE=%t", !settings.DisableLoopDevice),
 		"-e", fmt.Sprintf("FORCE_LOOP_DEVICE=%t", !settings.DisableLoopDevice),
 		"-e", fmt.Sprintf("BUILDKIT_DEBUG=%t", settings.Debug),
-		"-p", fmt.Sprintf("127.0.0.1:%d:5000", settings.DebuggerPort),
 		"--label", fmt.Sprintf("dev.earthly.settingshash=%s", settingsHash),
 		"--name", ContainerName,
 		"--privileged",
 	}
+	if os.Getenv("EARTHLY_WITH_DOCKER") == "1" {
+		// Add /sys/fs/cgroup if it's earthly-in-earthly.
+		args = append(args, "-v", "/sys/fs/cgroup:/sys/fs/cgroup")
+	} else {
+		// Debugger only supported in top-most earthly.
+		// TODO: Main reason for this is port clash. This could be improved in the future,
+		//       if needed.
+		args = append(args,
+			"-p", fmt.Sprintf("127.0.0.1:%d:5000", settings.DebuggerPort))
+	}
 	// Apply some buildkitd-related settings.
 	if settings.CacheSizeMb > 0 {
 		args = append(args,

buildkitd/dockerd-wrapper.sh

@@ -0,0 +1,69 @@
+#!/bin/sh
+
+set -e
+
+if [ -z "$EARTHLY_DOCKERD_DATA_ROOT" ]; then
+    echo "EARTHLY_DOCKERD_DATA_ROOT not set"
+    exit 1
+fi
+
+function start_dockerd() {
+    mkdir -p "$EARTHLY_DOCKERD_DATA_ROOT"
+    dockerd --data-root="$EARTHLY_DOCKERD_DATA_ROOT" &>/var/log/docker.log &
+    let i=1
+    timeout=30
+    while ! docker ps &>/dev/null; do
+        sleep 1
+        if [ "$i" -gt "$timeout" ]; then
+            # Print dockerd logs on start failure.
+            cat /var/log/docker.log
+            exit 1
+        fi
+        let i+=1
+    done
+}
+
+function stop_dockerd() {
+    dockerd_pid="$(cat /var/run/docker.pid)"
+    timeout=10
+    if [ -n "$dockerd_pid" ]; then
+        kill "$dockerd_pid" &>/dev/null
+        let i=1
+        while kill -0 "$dockerd_pid" &>/dev/null; do
+            sleep 1
+            if [ "$i" -gt "$timeout" ]; then
+                kill -9 "$dockerd_pid" &>/dev/null || true
+            fi
+            let i+=1
+        done
+    fi
+    # Wipe dockerd data when done.
+    rm -rf "$EARTHLY_DOCKERD_DATA_ROOT"
+}
+
+function load_images() {
+    if [ -n "$EARTHLY_DOCKER_LOAD_IMAGES" ]; then
+        echo "Loading images..."
+        for img in $EARTHLY_DOCKER_LOAD_IMAGES; do
+            docker load -i "$img" || (stop_dockerd; exit 1)
+        done
+        echo "...done"
+    fi
+}
+
+export EARTHLY_WITH_DOCKER=1
+
+# Lock the creation of the docker daemon - only one daemon can be started at a time
+# (dockerd race conditions in handling networking setup).
+(
+    flock -x 200
+    start_dockerd
+    flock -u 200
+) 200>/var/earthly/dind/lock
+load_images
+set +e
+"$@"
+exit_code="$?"
+set -e
+stop_dockerd
+exit "$exit_code"

earthfile2llb/converter.go

@@ -49,6 +49,7 @@ type Converter struct {
 	artifactBuilderFun ArtifactBuilderFun
 	cleanCollection    *cleanup.Collection
 	nextArgIndex       int
+	solveCache         map[string]llb.State
 }
 
 // NewConverter constructs a new converter for a given earth target.
@@ -81,6 +82,7 @@ func NewConverter(ctx context.Context, target domain.Target, bc *buildcontext.Da
 		dockerBuilderFun:   opt.DockerBuilderFun,
 		artifactBuilderFun: opt.ArtifactBuilderFun,
 		cleanCollection:    opt.CleanCollection,
+		solveCache:         opt.SolveCache,
 	}, nil
 }
 
@@ -483,7 +485,9 @@ func (c *Converter) Build(ctx context.Context, fullTargetName string, buildArgs
 			DockerBuilderFun: c.dockerBuilderFun,
 			CleanCollection:  c.cleanCollection,
 			VisitedStates:    c.mts.VisitedStates,
-			VarCollection:    newVarCollection})
+			VarCollection:    newVarCollection,
+			SolveCache:       c.solveCache,
+		})
 	if err != nil {
 		return nil, errors.Wrapf(err, "earthfile2llb for %s", fullTargetName)
 	}

earthfile2llb/earthfile2llb.go

@@ -13,6 +13,7 @@ import (
 	"github.com/earthly/earthly/earthfile2llb/parser"
 	"github.com/earthly/earthly/earthfile2llb/variables"
 	"github.com/earthly/earthly/logging"
+	"github.com/moby/buildkit/client/llb"
 	"github.com/pkg/errors"
 )
 
@@ -35,6 +36,8 @@ type ConvertOpt struct {
 	VisitedStates map[string][]*SingleTargetStates
 	// VarCollection is a collection of build args used for overriding args in the build.
 	VarCollection *variables.Collection
+	// A cache for image solves. depTargetInputHash -> context containing image.tar.
+	SolveCache map[string]llb.State
 }
 
 // DockerBuilderFun is a function able to build a target into a docker tar file.
@@ -45,6 +48,9 @@ type ArtifactBuilderFun = func(ctx context.Context, mts *MultiTargetStates, arti
 
 // Earthfile2LLB parses a earthfile and executes the statements for a given target.
 func Earthfile2LLB(ctx context.Context, target domain.Target, opt ConvertOpt) (mts *MultiTargetStates, err error) {
+	if opt.SolveCache == nil {
+		opt.SolveCache = make(map[string]llb.State)
+	}
 	if opt.VisitedStates == nil {
 		opt.VisitedStates = make(map[string][]*SingleTargetStates)
 	}

earthfile2llb/withdockerrun.go

@@ -5,19 +5,23 @@ import (
 	"crypto/sha256"
 	"encoding/hex"
 	"fmt"
+	"hash/fnv"
 	"io/ioutil"
 	"os"
 	"path"
 	"strings"
 
 	"github.com/earthly/earthly/dockertar"
 	"github.com/earthly/earthly/domain"
+	"github.com/earthly/earthly/earthfile2llb/dedup"
 	"github.com/earthly/earthly/llbutil"
 	"github.com/earthly/earthly/logging"
 	"github.com/moby/buildkit/client/llb"
 	"github.com/pkg/errors"
 )
 
+const dockerdWrapperPath = "/var/earthly/dockerd-wrapper.sh"
+
 type withDockerRun struct {
 	c        *Converter
 	tarLoads []llb.State
@@ -64,12 +68,18 @@ func (wdr *withDockerRun) Run(ctx context.Context, args []string, opt WithDocker
 	runOpts = append(runOpts, mountRunOpts...)
 	runOpts = append(runOpts, llb.AddMount(
 		"/var/earthly/dind", llb.Scratch(), llb.HostBind(), llb.SourcePath("/tmp/earthly/dind")))
-	var loadCmds []string
+	runOpts = append(runOpts, llb.AddMount(
+		dockerdWrapperPath, llb.Scratch(), llb.HostBind(), llb.SourcePath(dockerdWrapperPath)))
+	// This seems to make earthly-in-earthly work
+	// (and docker run --privileged, together with -v /sys/fs/cgroup:/sys/fs/cgroup),
+	// however, it breaks regular cases.
+	//runOpts = append(runOpts, llb.AddMount(
+	//"/sys/fs/cgroup", llb.Scratch(), llb.HostBind(), llb.SourcePath("/sys/fs/cgroup")))
+	var tarPaths []string
 	for index, tarContext := range wdr.tarLoads {
 		loadDir := fmt.Sprintf("/var/earthly/load-%d", index)
 		runOpts = append(runOpts, llb.AddMount(loadDir, tarContext, llb.Readonly))
-		loadTar := path.Join(loadDir, "image.tar")
-		loadCmds = append(loadCmds, fmt.Sprintf("docker load -i %s", loadTar))
+		tarPaths = append(tarPaths, path.Join(loadDir, "image.tar"))
 	}
 
 	finalArgs := args
@@ -92,7 +102,7 @@ func (wdr *withDockerRun) Run(ctx context.Context, args []string, opt WithDocker
 	if err != nil {
 		return errors.Wrap(err, "compute dind id")
 	}
-	shellWrap := makeWithDockerdWrapFun(dindID, loadCmds)
+	shellWrap := makeWithDockerdWrapFun(dindID, tarPaths)
 	return wdr.c.internalRun(ctx, finalArgs, opt.Secrets, opt.WithShell, shellWrap, false, runStr, runOpts...)
 }
 
@@ -101,7 +111,7 @@ func (wdr *withDockerRun) pull(ctx context.Context, dockerTag string) error {
 	logging.GetLogger(ctx).With("dockerTag", dockerTag).Info("Applying DOCKER PULL")
 	state, image, _, err := wdr.c.internalFromClassical(
 		ctx, dockerTag,
-		llb.WithCustomNamef("%sDOCKER PULL %s", wdr.c.vertexPrefix(), dockerTag),
+		llb.WithCustomNamef("%sDOCKER PULL %s", wdr.vertexPrefix(dockerTag), dockerTag),
 	)
 	if err != nil {
 		return err
@@ -110,6 +120,9 @@ func (wdr *withDockerRun) pull(ctx context.Context, dockerTag string) error {
 		FinalStates: &SingleTargetStates{
 			SideEffectsState: state,
 			SideEffectsImage: image,
+			TargetInput: dedup.TargetInput{
+				TargetCanonical: fmt.Sprintf("+@docker-pull:%s", dockerTag),
+			},
 			SaveImages: []SaveImage{
 				{
 					State:     state,
@@ -121,7 +134,7 @@ func (wdr *withDockerRun) pull(ctx context.Context, dockerTag string) error {
 	}
 	return wdr.solveImage(
 		ctx, mts, dockerTag, dockerTag,
-		llb.WithCustomNamef("%sDOCKER LOAD (PULL %s)", wdr.c.vertexPrefix(), dockerTag))
+		llb.WithCustomNamef("%sDOCKER LOAD (PULL %s)", wdr.vertexPrefix(dockerTag), dockerTag))
 }
 
 func (wdr *withDockerRun) load(ctx context.Context, opt DockerLoadOpt) error {
@@ -142,10 +155,25 @@ func (wdr *withDockerRun) load(ctx context.Context, opt DockerLoadOpt) error {
 	return wdr.solveImage(
 		ctx, mts, depTarget.String(), dockerTag,
 		llb.WithCustomNamef(
-			"%sDOCKER LOAD %s %s", wdr.c.vertexPrefix(), depTarget.String(), dockerTag))
+			"%sDOCKER LOAD %s %s", wdr.vertexPrefix(depTarget.String()), depTarget.String(), dockerTag))
+}
+
+func (wdr *withDockerRun) vertexPrefix(id string) string {
+	h := fnv.New32a()
+	h.Write([]byte(id))
+	return fmt.Sprintf("[%s %d] ", id, h.Sum32())
 }
 
 func (wdr *withDockerRun) solveImage(ctx context.Context, mts *MultiTargetStates, opName string, dockerTag string, opts ...llb.RunOption) error {
+	solveID, err := mts.FinalStates.TargetInput.Hash()
+	if err != nil {
+		return errors.Wrap(err, "target input hash")
+	}
+	tarContext, found := wdr.c.solveCache[solveID]
+	if found {
+		wdr.tarLoads = append(wdr.tarLoads, tarContext)
+		return nil
+	}
 	// Use a builder to create docker .tar file, mount it via a local build context,
 	// then docker load it within the current side effects state.
 	outDir, err := ioutil.TempDir("/tmp", "earthly-docker-load")
@@ -171,75 +199,33 @@ func (wdr *withDockerRun) solveImage(ctx context.Context, mts *MultiTargetStates
 	sha256SessionIDKey := sha256.Sum256([]byte(sessionIDKey))
 	sessionID := hex.EncodeToString(sha256SessionIDKey[:])
 	// Add the tar to the local context.
-	tarContext := llb.Local(
-		opName,
+	tarContext = llb.Local(
+		solveID,
 		llb.SharedKeyHint(opName),
 		llb.SessionID(sessionID),
 		llb.Platform(llbutil.TargetPlatform),
 		llb.WithCustomNamef("[internal] docker tar context %s %s", opName, sessionID),
 	)
 	wdr.tarLoads = append(wdr.tarLoads, tarContext)
-	wdr.c.mts.FinalStates.LocalDirs[opName] = outDir
+	wdr.c.mts.FinalStates.LocalDirs[solveID] = outDir
+	wdr.c.solveCache[solveID] = tarContext
 	return nil
 }
 
-func makeWithDockerdWrapFun(dindID string, loadCmds []string) shellWrapFun {
+func makeWithDockerdWrapFun(dindID string, tarPaths []string) shellWrapFun {
+	dockerRoot := path.Join("/var/earthly/dind", dindID)
+	params := []string{
+		fmt.Sprintf("EARTHLY_DOCKERD_DATA_ROOT=\"%s\"", dockerRoot),
+		fmt.Sprintf("EARTHLY_DOCKER_LOAD_IMAGES=\"%s\"", strings.Join(tarPaths, " ")),
+	}
 	return func(args []string, envVars []string, isWithShell bool, withDebugger bool) []string {
 		return []string{
 			"/bin/sh", "-c",
 			fmt.Sprintf(
-				"/bin/sh <<EOF\n%s\nEOF",
-				dockerdWrapCmds(args, envVars, isWithShell, withDebugger, dindID, loadCmds)),
+				"%s %s %s",
+				strings.Join(params, " "),
+				dockerdWrapperPath,
+				strWithEnvVars(args, envVars, isWithShell, withDebugger)),
 		}
 	}
 }
-
-func dockerdWrapCmds(args []string, envVars []string, isWithShell bool, withDebugger bool, dindID string, loadCmds []string) string {
-	dockerRoot := path.Join("/var/earthly/dind", dindID)
-	var cmds []string
-	cmds = append(cmds, "#!/bin/sh")
-	cmds = append(cmds, startDockerdCmds(dockerRoot)...)
-	cmds = append(cmds, loadCmds...)
-	cmds = append(cmds, strWithEnvVars(args, envVars, isWithShell, withDebugger))
-	cmds = append(cmds, "exit_code=\"\\$?\"")
-	cmds = append(cmds, stopDockerdCmds(dockerRoot)...)
-	cmds = append(cmds, "exit \"\\$exit_code\"")
-	return strings.Join(cmds, "\n")
-}
-
-func startDockerdCmds(dockerRoot string) []string {
-	return []string{
-		// Uncomment this line for debugging.
-		// "set -x",
-		fmt.Sprintf("mkdir -p %s", dockerRoot),
-		fmt.Sprintf("dockerd-entrypoint.sh dockerd --data-root=%s &>/var/log/docker.log &", dockerRoot),
-		"dockerd_pid=\"\\$!\"",
-		"let i=1",
-		"while ! docker ps &>/dev/null ; do",
-		"sleep 1",
-		"if [ \"\\$i\" -gt \"30\" ] ; then",
-		// Print logs on dockerd start failure.
-		"cat /var/log/docker.log",
-		"exit 1",
-		"fi",
-		"let i+=1",
-		"done",
-	}
-}
-
-func stopDockerdCmds(dockerRoot string) []string {
-	return []string{
-		"kill \"\\$dockerd_pid\" &>/dev/null",
-		"let i=1",
-		"while kill -0 \"\\$dockerd_pid\" &>/dev/null ; do",
-		"sleep 1",
-		"let i+=1",
-		"if [ \"\\$i\" -gt \"10\" ]; then",
-		"kill -9 \"\\$dockerd_pid\" &>/dev/null",
-		"sleep 1",
-		"fi",
-		// Wipe the WITH DOCKER docker data after each run.
-		fmt.Sprintf("rm -rf %s", dockerRoot),
-		"done",
-	}
-}