Handled empty target context name when deciding to mount bindings

Testing with local and tcp hosts

Signed-off-by: Nick Adcock <nick.adcock@docker.com>

Author: zappy-shu

Gopkg.lock

@@ -44,8 +44,7 @@ required = ["github.com/wadey/gocovmerge"]
 
 [[override]]
   name = "github.com/deislabs/duffle"
-  source = "github.com/simonferquel/duffle"
-  branch = "custom-container-config"
+  branch = "master"
 
 [[constraint]]
   name = "github.com/sirupsen/logrus"

Gopkg.toml

@@ -276,7 +276,15 @@ func TestBundle(t *testing.T) {
 	assert.Assert(t, fs.Equal(tmpDir.Join("simple.dockerapp"), manifest))
 }
 
-func TestDockerAppLifecycle(t *testing.T) {
+func TestDockerAppLifecycleWithBindMounts(t *testing.T) {
+	testDockerAppLifecycle(t, true)
+}
+
+func TestDockerAppLifecycleWithoutBindMounts(t *testing.T) {
+	testDockerAppLifecycle(t, false)
+}
+
+func testDockerAppLifecycle(t *testing.T, useBindMount bool) {
 	cmd, cleanup := dockerCli.createTestCmd()
 	defer cleanup()
 
@@ -301,13 +309,18 @@ func TestDockerAppLifecycle(t *testing.T) {
 	cmd.Command = dockerCli.Command("context", "create", "swarm-context", "--docker", fmt.Sprintf(`"host=tcp://%s"`, swarm.GetAddress(t)), "--default-stack-orchestrator", "swarm")
 	icmd.RunCmd(cmd).Assert(t, icmd.Success)
 
-	// When creating a context on a Windows host we cannot use
-	// the unix socket but it's needed inside the invocation image.
-	// The workaround is to create a context with an empty host.
-	// This host will default to the unix socket inside the
-	// invocation image
-	cmd.Command = dockerCli.Command("context", "create", "swarm-target-context", "--docker", "host=", "--default-stack-orchestrator", "swarm")
-	icmd.RunCmd(cmd).Assert(t, icmd.Success)
+	if useBindMount {
+		// When creating a context on a Windows host we cannot use
+		// the unix socket but it's needed inside the invocation image.
+		// The workaround is to create a context with an empty host.
+		// This host will default to the unix socket inside the
+		// invocation image
+		cmd.Command = dockerCli.Command("context", "create", "swarm-target-context", "--docker", "host=", "--default-stack-orchestrator", "swarm")
+		icmd.RunCmd(cmd).Assert(t, icmd.Success)
+	} else {
+		cmd.Command = dockerCli.Command("context", "create", "swarm-target-context", "--docker", fmt.Sprintf(`"host=tcp://%s"`, swarm.GetPrivateAddress(t)), "--default-stack-orchestrator", "swarm")
+		icmd.RunCmd(cmd).Assert(t, icmd.Success)
+	}
 
 	// Initialize the swarm
 	cmd.Env = append(cmd.Env, "DOCKER_CONTEXT=swarm-context")

e2e/commands_test.go

@@ -97,3 +97,13 @@ func (c *Container) GetAddress(t *testing.T) string {
 	c.address = fmt.Sprintf("127.0.0.1:%v", strings.Trim(strings.Split(result.Stdout(), ":")[1], " \r\n"))
 	return c.address
 }
+
+// GetPrivateAddress returns the host:port this container listens on
+func (c *Container) GetPrivateAddress(t *testing.T) string {
+	container := c.parentContainer
+	if container == "" {
+		container = c.container
+	}
+	result := icmd.RunCommand(dockerCli.path, "inspect", container, "-f", "{{.NetworkSettings.IPAddress}}").Assert(t, icmd.Success)
+	return fmt.Sprintf("%s:%d", strings.TrimSpace(result.Stdout()), c.privatePort)
+}

e2e/helper_test.go

@@ -17,35 +17,39 @@
 	"invocationImages": [
 		{
 			"imageType": "docker",
-			"image": "simple:1.1.0-beta1-invoc"
+			"image": "simple:1.1.0-beta1-invoc",
+			"platform": {}
 		}
 	],
 	"images": {
 		"api": {
 			"imageType": "docker",
 			"image": "python:3.6",
+			"platform": {},
 			"description": "python:3.6",
 			"refs": null
 		},
 		"db": {
 			"imageType": "docker",
 			"image": "postgres:9.3",
+			"platform": {},
 			"description": "postgres:9.3",
 			"refs": null
 		},
 		"web": {
 			"imageType": "docker",
 			"image": "nginx:latest",
+			"platform": {},
 			"description": "nginx:latest",
 			"refs": null
 		}
 	},
 	"actions": {
 		"com.docker.app.inspect": {
-			"Modifies": false
+			"modifies": false
 		},
 		"com.docker.app.status": {
-			"Modifies": false
+			"modifies": false
 		}
 	},
 	"parameters": {

e2e/testdata/simple-bundle.json.golden

@@ -1,7 +1,6 @@
 package commands
 
 import (
-	"github.com/docker/cli/cli/context/docker"
 	"fmt"
 	"io/ioutil"
 	"os"
@@ -17,6 +16,8 @@ import (
 	"github.com/docker/app/internal/packager"
 	bundlestore "github.com/docker/app/internal/store"
 	"github.com/docker/cli/cli/command"
+	"github.com/docker/cli/cli/context"
+	"github.com/docker/cli/cli/context/docker"
 	"github.com/docker/cli/cli/context/store"
 	"github.com/docker/distribution/reference"
 	"github.com/docker/docker/api/types/container"
@@ -179,29 +180,36 @@ func resolveBundle(dockerCli command.Cli, name string, pullRef bool, insecureReg
 	return nil, fmt.Errorf("could not resolve bundle %q", name)
 }
 
-func requiresBindMount(targetContextName string, targetOrchestrator string, dockerCli command.Cli) (bool, error){
-	if targetOrchestrator == "kubernetes"{
+func requiresBindMount(targetContextName string, targetOrchestrator string, dockerCli command.Cli) (bool, error) {
+	if targetOrchestrator == "kubernetes" {
 		return false, nil
 	}
+
+	// TODO:smarter handling of default context required
+	if targetContextName == "" {
+		return true, nil
+	}
+
 	ctxMeta, err := dockerCli.ContextStore().GetContextMetadata(targetContextName)
-	if err != nil{
+	if err != nil {
 		return false, err
 	}
 	dockerCtx, err := command.GetDockerContext(ctxMeta)
-	if err != nil{
+	if err != nil {
 		return false, err
 	}
-	if dockerCtx.StackOrchestrator == command.OrchestratorKubernetes{
+	if dockerCtx.StackOrchestrator == command.OrchestratorKubernetes {
 		return false, nil
 	}
 	dockerEndpoint, err := docker.EndpointFromContext(ctxMeta)
-	if err != nil{
+	if err != nil {
 		return false, err
 	}
+
+	return isDockerHostLocal(dockerEndpoint), nil
+}
+
+func isDockerHostLocal(dockerEndpoint context.EndpointMetaBase) bool {
 	host := dockerEndpoint.Host
-	switch host{
-	case "", "unix:///var/run/docker.sock", "npipe:////./pipe/docker_engine":
-		return true, nil
-	}
-	return false, nil
-}
+	return host == "" || host == "unix:///var/run/docker.sock" || host == "npipe:////./pipe/docker_engine"
+}

internal/commands/cnab.go

@@ -0,0 +1,85 @@
+package commands
+
+import (
+	"testing"
+
+	"github.com/docker/cli/cli/command"
+	"github.com/docker/cli/cli/context"
+	"gotest.tools/assert"
+)
+
+func TestRequiresBindMount(t *testing.T) {
+	dockerCli, err := command.NewDockerCli()
+	assert.NilError(t, err)
+
+	testCases := []struct {
+		name               string
+		targetContextName  string
+		targetOrchestrator string
+		expectedResult     bool
+		expectedError      string
+	}{
+		{
+			name:               "kubernetes-orchestrator",
+			targetContextName:  "target-context",
+			targetOrchestrator: "kubernetes",
+			expectedResult:     false,
+			expectedError:      "",
+		},
+		{
+			name:               "no-context",
+			targetContextName:  "",
+			targetOrchestrator: "swarm",
+			expectedResult:     true,
+			expectedError:      "",
+		},
+	}
+
+	for _, testCase := range testCases {
+		t.Run(testCase.name, func(t *testing.T) {
+			result, err := requiresBindMount(testCase.targetContextName, testCase.targetOrchestrator, dockerCli)
+			if testCase.expectedError == "" {
+				assert.NilError(t, err)
+			} else {
+				assert.Error(t, err, testCase.expectedError)
+			}
+			assert.Equal(t, testCase.expectedResult, result)
+		})
+	}
+}
+
+func TestIsDockerHostLocal(t *testing.T) {
+	testCases := []struct {
+		name     string
+		host     string
+		expected bool
+	}{
+		{
+			name:     "not-local",
+			host:     "tcp://not.local.host",
+			expected: false,
+		},
+		{
+			name:     "no-endpoint",
+			host:     "",
+			expected: true,
+		},
+		{
+			name:     "docker-sock",
+			host:     "unix:///var/run/docker.sock",
+			expected: true,
+		},
+		{
+			name:     "named-pipe",
+			host:     "npipe:////./pipe/docker_engine",
+			expected: true,
+		},
+	}
+
+	for _, testCase := range testCases {
+		t.Run(testCase.name, func(t *testing.T) {
+			dockerEndpoint := context.EndpointMetaBase{Host: testCase.host}
+			assert.Equal(t, testCase.expected, isDockerHostLocal(dockerEndpoint))
+		})
+	}
+}