Move VolumeDriver to HostConfig to make containers portable.

Signed-off-by: David Calavera <david.calavera@gmail.com>

api/server/container.go

@@ -4,7 +4,6 @@ import (
 	"fmt"
 	"io"
 	"net/http"
-	"runtime"
 	"strconv"
 	"strings"
 	"time"
@@ -20,22 +19,6 @@ import (
 	"github.com/docker/docker/runconfig"
 )
 
-func (s *Server) getContainersByName(version version.Version, w http.ResponseWriter, r *http.Request, vars map[string]string) error {
-	if vars == nil {
-		return fmt.Errorf("Missing parameter")
-	}
-
-	if version.LessThan("1.20") && runtime.GOOS != "windows" {
-		return getContainersByNameDownlevel(w, s, vars["name"])
-	}
-
-	containerJSON, err := s.daemon.ContainerInspect(vars["name"])
-	if err != nil {
-		return err
-	}
-	return writeJSON(w, http.StatusOK, containerJSON)
-}
-
 func (s *Server) getContainersJSON(version version.Version, w http.ResponseWriter, r *http.Request, vars map[string]string) error {
 	if err := parseForm(r); err != nil {
 		return err

api/server/inspect.go

@@ -0,0 +1,33 @@
+package server
+
+import (
+	"fmt"
+	"net/http"
+
+	"github.com/docker/docker/pkg/version"
+)
+
+// getContainersByName inspects containers configuration and serializes it as json.
+func (s *Server) getContainersByName(version version.Version, w http.ResponseWriter, r *http.Request, vars map[string]string) error {
+	if vars == nil {
+		return fmt.Errorf("Missing parameter")
+	}
+
+	var json interface{}
+	var err error
+
+	switch {
+	case version.LessThan("1.20"):
+		json, err = s.daemon.ContainerInspectPre120(vars["name"])
+	case version.Equal("1.20"):
+		json, err = s.daemon.ContainerInspect120(vars["name"])
+	default:
+		json, err = s.daemon.ContainerInspect(vars["name"])
+	}
+
+	if err != nil {
+		return err
+	}
+
+	return writeJSON(w, http.StatusOK, json)
+}

api/server/server_unix.go

@@ -103,16 +103,6 @@ func allocateDaemonPort(addr string) error {
 	return nil
 }
 
-// getContainersByNameDownlevel performs processing for pre 1.20 APIs. This
-// is only relevant on non-Windows daemons.
-func getContainersByNameDownlevel(w http.ResponseWriter, s *Server, namevar string) error {
-	containerJSONRaw, err := s.daemon.ContainerInspectPre120(namevar)
-	if err != nil {
-		return err
-	}
-	return writeJSON(w, http.StatusOK, containerJSONRaw)
-}
-
 // listenFD returns the specified socket activated files as a slice of
 // net.Listeners or all of the activated files if "*" is given.
 func listenFD(addr string) ([]net.Listener, error) {

api/server/server_windows.go

@@ -56,9 +56,3 @@ func (s *Server) AcceptConnections(d *daemon.Daemon) {
 func allocateDaemonPort(addr string) error {
 	return nil
 }
-
-// getContainersByNameDownlevel performs processing for pre 1.20 APIs. This
-// is only relevant on non-Windows daemons.
-func getContainersByNameDownlevel(w http.ResponseWriter, s *Server, namevar string) error {
-	return nil
-}

api/types/types.go

@@ -273,24 +273,39 @@ type ContainerJSON struct {
 	Config *runconfig.Config
 }
 
-// ContainerJSONPre120 is a backcompatibility struct along with ContainerConfig.
+// ContainerJSON120 is a backcompatibility struct along with ContainerConfig120.
+type ContainerJSON120 struct {
+	*ContainerJSONBase
+	Mounts []MountPoint
+	Config *ContainerConfig120
+}
+
+// ContainerJSONPre120 is a backcompatibility struct along with ContainerConfigPre120.
 // Note this is not used by the Windows daemon.
 type ContainerJSONPre120 struct {
 	*ContainerJSONBase
 	Volumes   map[string]string
 	VolumesRW map[string]bool
-	Config    *ContainerConfig
+	Config    *ContainerConfigPre120
 }
 
-// ContainerConfig is a backcompatibility struct used in ContainerJSONPre120
-type ContainerConfig struct {
+// ContainerConfigPre120 is a backcompatibility struct used in ContainerJSONPre120
+type ContainerConfigPre120 struct {
 	*runconfig.Config
 
 	// backward compatibility, they now live in HostConfig
-	Memory     int64
-	MemorySwap int64
-	CPUShares  int64  `json:"CpuShares"`
-	CPUSet     string `json:"CpuSet"`
+	VolumeDriver string
+	Memory       int64
+	MemorySwap   int64
+	CPUShares    int64  `json:"CpuShares"`
+	CPUSet       string `json:"CpuSet"`
+}
+
+// ContainerConfig120 is a backcompatibility struct used in ContainerJSON120
+type ContainerConfig120 struct {
+	*runconfig.Config
+	// backward compatibility, it lives now in HostConfig
+	VolumeDriver string
 }
 
 // MountPoint represents a mount point configuration inside the container.

daemon/create.go

@@ -105,7 +105,7 @@ func (daemon *Daemon) Create(config *runconfig.Config, hostConfig *runconfig.Hos
 	}
 	defer container.Unmount()
 
-	if err := createContainerPlatformSpecificSettings(container, config, img); err != nil {
+	if err := createContainerPlatformSpecificSettings(container, config, hostConfig, img); err != nil {
 		return nil, nil, err
 	}
 

daemon/create_unix.go

@@ -16,7 +16,7 @@ import (
 )
 
 // createContainerPlatformSpecificSettings performs platform specific container create functionality
-func createContainerPlatformSpecificSettings(container *Container, config *runconfig.Config, img *image.Image) error {
+func createContainerPlatformSpecificSettings(container *Container, config *runconfig.Config, hostConfig *runconfig.HostConfig, img *image.Image) error {
 	for spec := range config.Volumes {
 		var (
 			name, destination string
@@ -44,7 +44,7 @@ func createContainerPlatformSpecificSettings(container *Container, config *runco
 			return fmt.Errorf("cannot mount volume over existing file, file exists %s", path)
 		}
 
-		volumeDriver := config.VolumeDriver
+		volumeDriver := hostConfig.VolumeDriver
 		if destination != "" && img != nil {
 			if _, ok := img.ContainerConfig.Volumes[destination]; ok {
 				// check for whether bind is not specified and then set to local

daemon/create_windows.go

@@ -6,6 +6,6 @@ import (
 )
 
 // createContainerPlatformSpecificSettings performs platform specific container create functionality
-func createContainerPlatformSpecificSettings(container *Container, config *runconfig.Config, img *image.Image) error {
+func createContainerPlatformSpecificSettings(container *Container, config *runconfig.Config, hostConfig *runconfig.HostConfig, img *image.Image) error {
 	return nil
 }

daemon/inspect.go

@@ -29,6 +29,30 @@ func (daemon *Daemon) ContainerInspect(name string) (*types.ContainerJSON, error
 	return &types.ContainerJSON{base, mountPoints, container.Config}, nil
 }
 
+// ContainerInspect120 serializes the master version of a container into a json type.
+func (daemon *Daemon) ContainerInspect120(name string) (*types.ContainerJSON120, error) {
+	container, err := daemon.Get(name)
+	if err != nil {
+		return nil, err
+	}
+
+	container.Lock()
+	defer container.Unlock()
+
+	base, err := daemon.getInspectData(container)
+	if err != nil {
+		return nil, err
+	}
+
+	mountPoints := addMountPoints(container)
+	config := &types.ContainerConfig120{
+		container.Config,
+		container.hostConfig.VolumeDriver,
+	}
+
+	return &types.ContainerJSON120{base, mountPoints, config}, nil
+}
+
 func (daemon *Daemon) getInspectData(container *Container) (*types.ContainerJSONBase, error) {
 	// make a copy to play with
 	hostConfig := *container.hostConfig

daemon/inspect_unix.go

@@ -14,7 +14,7 @@ func setPlatformSpecificContainerFields(container *Container, contJSONBase *type
 	return contJSONBase
 }
 
-// ContainerInspectPre120 is for backwards compatibility with pre v1.20 clients.
+// ContainerInspectPre120 gets containers for pre 1.20 APIs.
 func (daemon *Daemon) ContainerInspectPre120(name string) (*types.ContainerJSONPre120, error) {
 	container, err := daemon.Get(name)
 	if err != nil {
@@ -36,8 +36,9 @@ func (daemon *Daemon) ContainerInspectPre120(name string) (*types.ContainerJSONP
 		volumesRW[m.Destination] = m.RW
 	}
 
-	config := &types.ContainerConfig{
+	config := &types.ContainerConfigPre120{
 		container.Config,
+		container.hostConfig.VolumeDriver,
 		container.hostConfig.Memory,
 		container.hostConfig.MemorySwap,
 		container.hostConfig.CPUShares,

daemon/inspect_windows.go

@@ -10,3 +10,8 @@ func setPlatformSpecificContainerFields(container *Container, contJSONBase *type
 func addMountPoints(container *Container) []types.MountPoint {
 	return nil
 }
+
+// ContainerInspectPre120 get containers for pre 1.20 APIs.
+func (daemon *Daemon) ContainerInspectPre120(name string) (*types.ContainerJSON, error) {
+	return daemon.ContainerInspect(name)
+}

daemon/volumes_linux_unit_test.go

@@ -5,7 +5,6 @@ package daemon
 import (
 	"testing"
 
-	"github.com/docker/docker/runconfig"
 	"github.com/docker/docker/volume"
 	"github.com/docker/docker/volume/drivers"
 )
@@ -55,8 +54,7 @@ func TestParseBindMount(t *testing.T) {
 	}
 
 	for _, c := range cases {
-		conf := &runconfig.Config{VolumeDriver: c.driver}
-		m, err := parseBindMount(c.bind, c.mountLabel, conf)
+		m, err := parseBindMount(c.bind, c.mountLabel, c.driver)
 		if c.fail {
 			if err == nil {
 				t.Fatalf("Expected error, was nil, for spec %s\n", c.bind)

daemon/volumes_unix.go

@@ -59,7 +59,7 @@ func (container *Container) setupMounts() ([]execdriver.Mount, error) {
 }
 
 // parseBindMount validates the configuration of mount information in runconfig is valid.
-func parseBindMount(spec string, mountLabel string, config *runconfig.Config) (*mountPoint, error) {
+func parseBindMount(spec, mountLabel, volumeDriver string) (*mountPoint, error) {
 	bind := &mountPoint{
 		RW: true,
 	}
@@ -87,7 +87,7 @@ func parseBindMount(spec string, mountLabel string, config *runconfig.Config) (*
 	}
 
 	if len(source) == 0 {
-		bind.Driver = config.VolumeDriver
+		bind.Driver = volumeDriver
 		if len(bind.Driver) == 0 {
 			bind.Driver = volume.DefaultDriverName
 		}
@@ -325,7 +325,7 @@ func (daemon *Daemon) registerMountPoints(container *Container, hostConfig *runc
 	// 3. Read bind mounts
 	for _, b := range hostConfig.Binds {
 		// #10618
-		bind, err := parseBindMount(b, container.MountLabel, container.Config)
+		bind, err := parseBindMount(b, container.MountLabel, hostConfig.VolumeDriver)
 		if err != nil {
 			return err
 		}

docs/reference/api/docker_remote_api.md

@@ -80,6 +80,7 @@ This section lists each version from latest to oldest.  Each listing includes a
 * `POST /volumes` to create a volume.
 * `GET /volumes/(name)` get low-level information about a volume.
 * `DELETE /volumes/(name)`remove a volume with the specified name.
+* `VolumeDriver` has been moved from config to hostConfig to make the configuration portable.
 
 
 ### v1.20 API changes

docs/reference/api/docker_remote_api_v1.21.md

@@ -196,7 +196,8 @@ Create a container
              "Ulimits": [{}],
              "LogConfig": { "Type": "json-file", "Config": {} },
              "SecurityOpt": [""],
-             "CgroupParent": ""
+             "CgroupParent": "",
+	      "VolumeDriver": ""
           }
       }
 
@@ -300,6 +301,7 @@ Json Parameters:
           Available types: `json-file`, `syslog`, `journald`, `gelf`, `none`.
           `json-file` logging driver.
     -   **CgroupParent** - Path to `cgroups` under which the container's `cgroup` is created. If the path is not absolute, the path is considered to be relative to the `cgroups` path of the init process. Cgroups are created if they do not already exist.
+    -   **VolumeDriver** - Driver that this container users to mount volumes.
 
 Query Parameters:
 
@@ -407,7 +409,8 @@ Return low-level information on the container `id`
 			},
 			"SecurityOpt": null,
 			"VolumesFrom": null,
-			"Ulimits": [{}]
+			"Ulimits": [{}],
+			"VolumeDriver": ""
 		},
 		"HostnamePath": "/var/lib/docker/containers/ba033ac4401106a3b513bc9d639eee123ad78ca3616b921167cd74b20e25ed39/hostname",
 		"HostsPath": "/var/lib/docker/containers/ba033ac4401106a3b513bc9d639eee123ad78ca3616b921167cd74b20e25ed39/hosts",

integration-cli/docker_api_inspect_test.go

@@ -48,3 +48,65 @@ func (s *DockerSuite) TestInspectApiContainerResponse(c *check.C) {
 		}
 	}
 }
+
+func (s *DockerSuite) TestInspectApiContainerVolumeDriverLegacy(c *check.C) {
+	out, _ := dockerCmd(c, "run", "-d", "busybox", "true")
+
+	cleanedContainerID := strings.TrimSpace(out)
+
+	cases := []string{"1.19", "1.20"}
+	for _, version := range cases {
+		endpoint := fmt.Sprintf("/v%s/containers/%s/json", version, cleanedContainerID)
+		status, body, err := sockRequest("GET", endpoint, nil)
+		c.Assert(status, check.Equals, http.StatusOK)
+		c.Assert(err, check.IsNil)
+
+		var inspectJSON map[string]interface{}
+		if err = json.Unmarshal(body, &inspectJSON); err != nil {
+			c.Fatalf("unable to unmarshal body for version %s: %v", version, err)
+		}
+
+		config, ok := inspectJSON["Config"]
+		if !ok {
+			c.Fatal("Unable to find 'Config'")
+		}
+		cfg := config.(map[string]interface{})
+		if _, ok := cfg["VolumeDriver"]; !ok {
+			c.Fatalf("Api version %s expected to include VolumeDriver in 'Config'", version)
+		}
+	}
+}
+
+func (s *DockerSuite) TestInspectApiContainerVolumeDriver(c *check.C) {
+	out, _ := dockerCmd(c, "run", "-d", "busybox", "true")
+
+	cleanedContainerID := strings.TrimSpace(out)
+
+	endpoint := fmt.Sprintf("/v1.21/containers/%s/json", cleanedContainerID)
+	status, body, err := sockRequest("GET", endpoint, nil)
+	c.Assert(status, check.Equals, http.StatusOK)
+	c.Assert(err, check.IsNil)
+
+	var inspectJSON map[string]interface{}
+	if err = json.Unmarshal(body, &inspectJSON); err != nil {
+		c.Fatalf("unable to unmarshal body for version 1.21: %v", err)
+	}
+
+	config, ok := inspectJSON["Config"]
+	if !ok {
+		c.Fatal("Unable to find 'Config'")
+	}
+	cfg := config.(map[string]interface{})
+	if _, ok := cfg["VolumeDriver"]; ok {
+		c.Fatal("Api version 1.21 expected to not include VolumeDriver in 'Config'")
+	}
+
+	config, ok = inspectJSON["HostConfig"]
+	if !ok {
+		c.Fatal("Unable to find 'HostConfig'")
+	}
+	cfg = config.(map[string]interface{})
+	if _, ok := cfg["VolumeDriver"]; !ok {
+		c.Fatal("Api version 1.21 expected to include VolumeDriver in 'HostConfig'")
+	}
+}

integration-cli/docker_cli_start_volume_driver_unix_test.go

@@ -253,15 +253,11 @@ func (s *DockerExternalVolumeSuite) TestStartExternalNamedVolumeDriverCheckBindL
 
 	img := "test-checkbindlocalvolume"
 
-	args := []string{"--host", s.d.sock()}
-	buildOut, err := buildImageArgs(args, img, dockerfile, true)
-	fmt.Println(buildOut)
+	_, err := buildImageWithOutInDamon(s.d.sock(), img, dockerfile, true)
+	c.Assert(err, check.IsNil)
 
 	out, err := s.d.Cmd("run", "--rm", "--name", "test-data-nobind", "-v", "external-volume-test:/tmp/external-volume-test", "--volume-driver", "test-external-volume-driver", img, "cat", "/nobindthenlocalvol/test")
-	if err != nil {
-		fmt.Println(out)
-		c.Fatal(err)
-	}
+	c.Assert(err, check.IsNil)
 
 	if !strings.Contains(out, expected) {
 		c.Fatalf("External volume mount failed. Output: %s\n", out)

integration-cli/docker_utils.go

@@ -1392,22 +1392,14 @@ func createTmpFile(c *check.C, content string) string {
 	return filename
 }
 
-func buildImageArgs(args []string, name, dockerfile string, useCache bool) (string, error) {
-	id, _, err := buildImageWithOutArgs(args, name, dockerfile, useCache)
-	return id, err
-}
-
-func buildImageWithOutArgs(args []string, name, dockerfile string, useCache bool) (string, string, error) {
+func buildImageWithOutInDamon(socket string, name, dockerfile string, useCache bool) (string, error) {
+	args := []string{"--host", socket}
 	buildCmd := buildImageCmdArgs(args, name, dockerfile, useCache)
 	out, exitCode, err := runCommandWithOutput(buildCmd)
 	if err != nil || exitCode != 0 {
-		return "", out, fmt.Errorf("failed to build the image: %s", out)
+		return out, fmt.Errorf("failed to build the image: %s, error: %v", out, err)
 	}
-	id, err := getIDByName(name)
-	if err != nil {
-		return "", out, err
-	}
-	return id, out, nil
+	return out, nil
 }
 
 func buildImageCmdArgs(args []string, name, dockerfile string, useCache bool) *exec.Cmd {

runconfig/config.go

@@ -28,7 +28,6 @@ type Config struct {
 	Cmd             *stringutils.StrSlice // Command to run when starting the container
 	Image           string                // Name of the image as it was passed by the operator (eg. could be symbolic)
 	Volumes         map[string]struct{}   // List of volumes (mounts) used for the container
-	VolumeDriver    string                // Name of the volume driver used to mount volumes
 	WorkingDir      string                // Current directory (PWD) in the command will be launched
 	Entrypoint      *stringutils.StrSlice // Entrypoint to run when starting the container
 	NetworkDisabled bool                  // Is network disabled

runconfig/config_unix.go

@@ -32,11 +32,17 @@ func (w *ContainerConfigWrapper) getHostConfig() *HostConfig {
 			w.InnerHostConfig.CpusetCpus = hc.CpusetCpus
 		}
 
+		if hc.VolumeDriver != "" && w.InnerHostConfig.VolumeDriver == "" {
+			w.InnerHostConfig.VolumeDriver = hc.VolumeDriver
+		}
+
 		hc = w.InnerHostConfig
 	}
 
-	if hc != nil && w.Cpuset != "" && hc.CpusetCpus == "" {
-		hc.CpusetCpus = w.Cpuset
+	if hc != nil {
+		if w.Cpuset != "" && hc.CpusetCpus == "" {
+			hc.CpusetCpus = w.Cpuset
+		}
 	}
 
 	// Make sure NetworkMode has an acceptable value. We do this to ensure

runconfig/hostconfig.go

@@ -251,6 +251,7 @@ type HostConfig struct {
 	LogConfig        LogConfig             // Configuration of the logs for this container
 	CgroupParent     string                // Parent cgroup.
 	ConsoleSize      [2]int                // Initial console size on Windows
+	VolumeDriver     string                // Name of the volume driver used to mount volumes
 }
 
 // DecodeHostConfig creates a HostConfig based on the specified Reader.

runconfig/parse.go

@@ -322,7 +322,6 @@ func Parse(cmd *flag.FlagSet, args []string) (*Config, *HostConfig, *flag.FlagSe
 		Entrypoint:      entrypoint,
 		WorkingDir:      *flWorkingDir,
 		Labels:          convertKVStringsToMap(labels),
-		VolumeDriver:    *flVolumeDriver,
 	}
 
 	hostConfig := &HostConfig{
@@ -362,6 +361,7 @@ func Parse(cmd *flag.FlagSet, args []string) (*Config, *HostConfig, *flag.FlagSe
 		Ulimits:          flUlimits.GetList(),
 		LogConfig:        LogConfig{Type: *flLoggingDriver, Config: loggingOpts},
 		CgroupParent:     *flCgroupParent,
+		VolumeDriver:     *flVolumeDriver,
 	}
 
 	applyExperimentalFlags(expFlags, config, hostConfig)