Refactor and cleanup the intermediate container creation

This PR is trying to refactor the `probeAndCreate` and cleanup related codes based on the refactoring. Signed-off-by: Dennis Chen <[email protected]>
singholt · Jun 4, 2018 · 7f280f6 · 7f280f6
1 parent 641e2c0
commit 7f280f6
Show file tree

Hide file tree

Showing 2 changed files with 9 additions and 14 deletions.
diff --git a/builder/dockerfile/dispatchers.go b/builder/dockerfile/dispatchers.go
@@ -28,7 +28,6 @@ import (
 	"github.com/docker/docker/pkg/system"
 	"github.com/docker/go-connections/nat"
 	"github.com/pkg/errors"
-	"github.com/sirupsen/logrus"
 )
 
 // ENV foo bar
@@ -305,10 +304,12 @@ func dispatchWorkdir(d dispatchRequest, c *instructions.WorkdirCommand) error {
 
 	comment := "WORKDIR " + runConfig.WorkingDir
 	runConfigWithCommentCmd := copyRunConfig(runConfig, withCmdCommentString(comment, d.state.operatingSystem))
+
 	containerID, err := d.builder.probeAndCreate(d.state, runConfigWithCommentCmd)
 	if err != nil || containerID == "" {
 		return err
 	}
+
 	if err := d.builder.docker.ContainerCreateWorkdir(containerID); err != nil {
 		return err
 	}
@@ -350,8 +351,7 @@ func dispatchRun(d dispatchRequest, c *instructions.RunCommand) error {
 	runConfigForCacheProbe := copyRunConfig(stateRunConfig,
 		withCmd(saveCmd),
 		withEntrypointOverride(saveCmd, nil))
-	hit, err := d.builder.probeCache(d.state, runConfigForCacheProbe)
-	if err != nil || hit {
+	if hit, err := d.builder.probeCache(d.state, runConfigForCacheProbe); err != nil || hit {
 		return err
 	}
 
@@ -363,11 +363,11 @@ func dispatchRun(d dispatchRequest, c *instructions.RunCommand) error {
 	// set config as already being escaped, this prevents double escaping on windows
 	runConfig.ArgsEscaped = true
 
-	logrus.Debugf("[BUILDER] Command to be executed: %v", runConfig.Cmd)
 	cID, err := d.builder.create(runConfig)
 	if err != nil {
 		return err
 	}
+
 	if err := d.builder.containerManager.Run(d.builder.clientCtx, cID, d.builder.Stdout, d.builder.Stderr); err != nil {
 		if err, ok := err.(*statusCodeError); ok {
 			// TODO: change error type, because jsonmessage.JSONError assumes HTTP

diff --git a/builder/dockerfile/internals.go b/builder/dockerfile/internals.go
@@ -27,6 +27,7 @@ import (
 	"github.com/docker/docker/pkg/system"
 	"github.com/docker/go-connections/nat"
 	"github.com/pkg/errors"
+	"github.com/sirupsen/logrus"
 )
 
 // Archiver defines an interface for copying files from one destination to
@@ -84,12 +85,8 @@ func (b *Builder) commit(dispatchState *dispatchState, comment string) error {
 	}
 
 	runConfigWithCommentCmd := copyRunConfig(dispatchState.runConfig, withCmdComment(comment, dispatchState.operatingSystem))
-	hit, err := b.probeCache(dispatchState, runConfigWithCommentCmd)
-	if err != nil || hit {
-		return err
-	}
-	id, err := b.create(runConfigWithCommentCmd)
-	if err != nil {
+	id, err := b.probeAndCreate(dispatchState, runConfigWithCommentCmd)
+	if err != nil || id == "" {
 		return err
 	}
 
@@ -413,13 +410,11 @@ func (b *Builder) probeAndCreate(dispatchState *dispatchState, runConfig *contai
 	if hit, err := b.probeCache(dispatchState, runConfig); err != nil || hit {
 		return "", err
 	}
-	// Set a log config to override any default value set on the daemon
-	hostConfig := &container.HostConfig{LogConfig: defaultLogConfig}
-	container, err := b.containerManager.Create(runConfig, hostConfig)
-	return container.ID, err
+	return b.create(runConfig)
 }
 
 func (b *Builder) create(runConfig *container.Config) (string, error) {
+	logrus.Debugf("[BUILDER] Command to be executed: %v", runConfig.Cmd)
 	hostConfig := hostConfigFromOptions(b.options)
 	container, err := b.containerManager.Create(runConfig, hostConfig)
 	if err != nil {