From 6549d6517bf9a7c79d21a86cbf36af10fcdbfbe0 Mon Sep 17 00:00:00 2001 From: David Calavera Date: Mon, 24 Aug 2015 13:57:39 -0400 Subject: [PATCH] Move VolumeDriver to HostConfig to make containers portable. Signed-off-by: David Calavera --- api/server/container.go | 17 ----- api/server/inspect.go | 33 ++++++++++ api/server/server_unix.go | 10 --- api/server/server_windows.go | 6 -- api/types/types.go | 31 +++++++--- daemon/create.go | 2 +- daemon/create_unix.go | 4 +- daemon/create_windows.go | 2 +- daemon/inspect.go | 24 +++++++ daemon/inspect_unix.go | 5 +- daemon/inspect_windows.go | 5 ++ daemon/volumes_linux_unit_test.go | 4 +- daemon/volumes_unix.go | 6 +- docs/reference/api/docker_remote_api.md | 1 + docs/reference/api/docker_remote_api_v1.21.md | 7 ++- integration-cli/docker_api_inspect_test.go | 62 +++++++++++++++++++ ...ocker_cli_start_volume_driver_unix_test.go | 10 +-- integration-cli/docker_utils.go | 16 ++--- runconfig/config.go | 1 - runconfig/config_unix.go | 10 ++- runconfig/hostconfig.go | 1 + runconfig/parse.go | 2 +- 22 files changed, 181 insertions(+), 78 deletions(-) create mode 100644 api/server/inspect.go diff --git a/api/server/container.go b/api/server/container.go index 6699b0f876..1ffa3da689 100644 --- a/api/server/container.go +++ b/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 diff --git a/api/server/inspect.go b/api/server/inspect.go new file mode 100644 index 0000000000..41f958ebb1 --- /dev/null +++ b/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) +} diff --git a/api/server/server_unix.go b/api/server/server_unix.go index 198a508c7c..2c1621f9b3 100644 --- a/api/server/server_unix.go +++ b/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) { diff --git a/api/server/server_windows.go b/api/server/server_windows.go index 51c062527e..8763ba04a8 100644 --- a/api/server/server_windows.go +++ b/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 -} diff --git a/api/types/types.go b/api/types/types.go index 66c3f82078..1984aee542 100644 --- a/api/types/types.go +++ b/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. diff --git a/daemon/create.go b/daemon/create.go index 62262792bd..ca53e5e33c 100644 --- a/daemon/create.go +++ b/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 } diff --git a/daemon/create_unix.go b/daemon/create_unix.go index b6a0edb202..2e1cc6eb7a 100644 --- a/daemon/create_unix.go +++ b/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 diff --git a/daemon/create_windows.go b/daemon/create_windows.go index 27a5d8361f..21aac13d31 100644 --- a/daemon/create_windows.go +++ b/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 } diff --git a/daemon/inspect.go b/daemon/inspect.go index 1c4a65fca7..6335de8f8f 100644 --- a/daemon/inspect.go +++ b/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 diff --git a/daemon/inspect_unix.go b/daemon/inspect_unix.go index f71caecb56..088e830fed 100644 --- a/daemon/inspect_unix.go +++ b/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, diff --git a/daemon/inspect_windows.go b/daemon/inspect_windows.go index c5baa1382c..34dceca609 100644 --- a/daemon/inspect_windows.go +++ b/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) +} diff --git a/daemon/volumes_linux_unit_test.go b/daemon/volumes_linux_unit_test.go index a2b68acc1d..9533477aaf 100644 --- a/daemon/volumes_linux_unit_test.go +++ b/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) diff --git a/daemon/volumes_unix.go b/daemon/volumes_unix.go index 0afe20ba06..80cb2d36b5 100644 --- a/daemon/volumes_unix.go +++ b/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 } diff --git a/docs/reference/api/docker_remote_api.md b/docs/reference/api/docker_remote_api.md index 6652dc2d00..f3b3e279a9 100644 --- a/docs/reference/api/docker_remote_api.md +++ b/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 diff --git a/docs/reference/api/docker_remote_api_v1.21.md b/docs/reference/api/docker_remote_api_v1.21.md index e466552af9..51a2f7901a 100644 --- a/docs/reference/api/docker_remote_api_v1.21.md +++ b/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", diff --git a/integration-cli/docker_api_inspect_test.go b/integration-cli/docker_api_inspect_test.go index 2287e7d42d..6f43538888 100644 --- a/integration-cli/docker_api_inspect_test.go +++ b/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'") + } +} diff --git a/integration-cli/docker_cli_start_volume_driver_unix_test.go b/integration-cli/docker_cli_start_volume_driver_unix_test.go index d2dae6a15a..302026d807 100644 --- a/integration-cli/docker_cli_start_volume_driver_unix_test.go +++ b/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) diff --git a/integration-cli/docker_utils.go b/integration-cli/docker_utils.go index 0f6e5e926b..1371c06dcb 100644 --- a/integration-cli/docker_utils.go +++ b/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 { diff --git a/runconfig/config.go b/runconfig/config.go index eed2cba2b4..04010954be 100644 --- a/runconfig/config.go +++ b/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 diff --git a/runconfig/config_unix.go b/runconfig/config_unix.go index 8d655eedda..63bd0a2f7a 100644 --- a/runconfig/config_unix.go +++ b/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 diff --git a/runconfig/hostconfig.go b/runconfig/hostconfig.go index 77ed0d7ed7..9746e177ac 100644 --- a/runconfig/hostconfig.go +++ b/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. diff --git a/runconfig/parse.go b/runconfig/parse.go index 90a0fecdef..8850b9f46c 100644 --- a/runconfig/parse.go +++ b/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)