From f3680e74946d3a773edb118ea3f508b8237e44a8 Mon Sep 17 00:00:00 2001 From: Brian Goff Date: Tue, 16 Dec 2014 10:06:45 -0500 Subject: [PATCH] Cleanup daemon/volumes - Mount struct now called volumeMount - Merged volume creation for each volume type (volumes-from, binds, normal volumes) so this only happens in once place - Simplified container copy of volumes (for when `docker cp` is a volume) Signed-off-by: Brian Goff --- daemon/volumes.go | 327 +++++++++-------------- integration-cli/docker_cli_build_test.go | 2 +- 2 files changed, 132 insertions(+), 197 deletions(-) diff --git a/daemon/volumes.go b/daemon/volumes.go index 7c6696d65f..4d15023ba7 100644 --- a/daemon/volumes.go +++ b/daemon/volumes.go @@ -14,17 +14,14 @@ import ( "github.com/docker/docker/pkg/mount" "github.com/docker/docker/pkg/symlink" "github.com/docker/docker/pkg/system" - "github.com/docker/docker/volumes" ) -type Mount struct { - MountToPath string - container *Container - volume *volumes.Volume - Writable bool - copyData bool - from *Container - isBind bool +type volumeMount struct { + containerPath string + hostPath string + writable bool + copyData bool + from string } func (container *Container) prepareVolumes() error { @@ -33,9 +30,110 @@ func (container *Container) prepareVolumes() error { container.VolumesRW = make(map[string]bool) } + if len(container.hostConfig.VolumesFrom) > 0 && container.AppliedVolumesFrom == nil { + container.AppliedVolumesFrom = make(map[string]struct{}) + } return container.createVolumes() } +func (container *Container) createVolumes() error { + mounts := make(map[string]*volumeMount) + + // get the normal volumes + for path := range container.Config.Volumes { + path = filepath.Clean(path) + // skip if there is already a volume for this container path + if _, exists := container.Volumes[path]; exists { + continue + } + + realPath, err := container.getResourcePath(path) + if err != nil { + return err + } + if stat, err := os.Stat(realPath); err == nil { + if !stat.IsDir() { + return fmt.Errorf("can't mount to container path, file exists - %s", path) + } + } + + mnt := &volumeMount{ + containerPath: path, + writable: true, + copyData: true, + } + mounts[mnt.containerPath] = mnt + } + + // Get all the bind mounts + // track bind paths separately due to #10618 + bindPaths := make(map[string]struct{}) + for _, spec := range container.hostConfig.Binds { + mnt, err := parseBindMountSpec(spec) + if err != nil { + return err + } + + // #10618 + if _, exists := bindPaths[mnt.containerPath]; exists { + return fmt.Errorf("Duplicate volume mount %s", mnt.containerPath) + } + + bindPaths[mnt.containerPath] = struct{}{} + mounts[mnt.containerPath] = mnt + } + + // Get volumes from + for _, from := range container.hostConfig.VolumesFrom { + cID, mode, err := parseVolumesFromSpec(from) + if err != nil { + return err + } + if _, exists := container.AppliedVolumesFrom[cID]; exists { + // skip since it's already been applied + continue + } + + c, err := container.daemon.Get(cID) + if err != nil { + return fmt.Errorf("container %s not found, impossible to mount its volumes", cID) + } + + for _, mnt := range c.volumeMounts() { + mnt.writable = mnt.writable && (mode == "rw") + mnt.from = cID + mounts[mnt.containerPath] = mnt + } + } + + for _, mnt := range mounts { + containerMntPath, err := symlink.FollowSymlinkInScope(filepath.Join(container.basefs, mnt.containerPath), container.basefs) + if err != nil { + return err + } + + // Create the actual volume + v, err := container.daemon.volumes.FindOrCreateVolume(mnt.hostPath, mnt.writable) + if err != nil { + return err + } + + container.VolumesRW[mnt.containerPath] = mnt.writable + container.Volumes[mnt.containerPath] = v.Path + v.AddContainer(container.ID) + if mnt.from != "" { + container.AppliedVolumesFrom[mnt.from] = struct{}{} + } + + if mnt.writable && mnt.copyData { + // Copy whatever is in the container at the containerPath to the volume + copyExistingContents(containerMntPath, v.Path) + } + } + + return nil +} + // sortedVolumeMounts returns the list of container volume mount points sorted in lexicographic order func (container *Container) sortedVolumeMounts() []string { var mountPaths []string @@ -47,58 +145,6 @@ func (container *Container) sortedVolumeMounts() []string { return mountPaths } -func (container *Container) createVolumes() error { - mounts, err := container.parseVolumeMountConfig() - if err != nil { - return err - } - - for _, mnt := range mounts { - if err := mnt.initialize(); err != nil { - return err - } - } - - // On every start, this will apply any new `VolumesFrom` entries passed in via HostConfig, which may override volumes set in `create` - return container.applyVolumesFrom() -} - -func (m *Mount) initialize() error { - // No need to initialize anything since it's already been initialized - if hostPath, exists := m.container.Volumes[m.MountToPath]; exists { - // If this is a bind-mount/volumes-from, maybe it was passed in at start instead of create - // We need to make sure bind-mounts/volumes-from passed on start can override existing ones. - if (!m.volume.IsBindMount && !m.isBind) && m.from == nil { - return nil - } - if m.volume.Path == hostPath { - return nil - } - - // Make sure we remove these old volumes we don't actually want now. - // Ignore any errors here since this is just cleanup, maybe someone volumes-from'd this volume - if v := m.container.daemon.volumes.Get(hostPath); v != nil { - v.RemoveContainer(m.container.ID) - m.container.daemon.volumes.Delete(v.Path) - } - } - - // This is the full path to container fs + mntToPath - containerMntPath, err := symlink.FollowSymlinkInScope(filepath.Join(m.container.basefs, m.MountToPath), m.container.basefs) - if err != nil { - return err - } - m.container.VolumesRW[m.MountToPath] = m.Writable - m.container.Volumes[m.MountToPath] = m.volume.Path - m.volume.AddContainer(m.container.ID) - if m.Writable && m.copyData { - // Copy whatever is in the container at the mntToPath to the volume - copyExistingContents(containerMntPath, m.volume.Path) - } - - return nil -} - func (container *Container) VolumePaths() map[string]struct{} { var paths = make(map[string]struct{}) for _, path := range container.Volumes { @@ -139,97 +185,30 @@ func (container *Container) derefVolumes() { } } -func (container *Container) parseVolumeMountConfig() (map[string]*Mount, error) { - var mounts = make(map[string]*Mount) - // Get all the bind mounts - for _, spec := range container.hostConfig.Binds { - path, mountToPath, writable, err := parseBindMountSpec(spec) - if err != nil { - return nil, err - } - // Check if a bind mount has already been specified for the same container path - if m, exists := mounts[mountToPath]; exists { - return nil, fmt.Errorf("Duplicate volume %q: %q already in use, mounted from %q", path, mountToPath, m.volume.Path) - } - // Check if a volume already exists for this and use it - vol, err := container.daemon.volumes.FindOrCreateVolume(path, writable) - if err != nil { - return nil, err - } - mounts[mountToPath] = &Mount{ - container: container, - volume: vol, - MountToPath: mountToPath, - Writable: writable, - isBind: true, // in case the volume itself is a normal volume, but is being mounted in as a bindmount here - } - } - - // Get the rest of the volumes - for path := range container.Config.Volumes { - // Check if this is already added as a bind-mount - path = filepath.Clean(path) - if _, exists := mounts[path]; exists { - continue - } - - // Check if this has already been created - if _, exists := container.Volumes[path]; exists { - continue - } - realPath, err := container.getResourcePath(path) - if err != nil { - return nil, fmt.Errorf("failed to evaluate the absolute path of symlink") - } - if stat, err := os.Stat(realPath); err == nil { - if !stat.IsDir() { - return nil, fmt.Errorf("file exists at %s, can't create volume there", realPath) - } - } - - vol, err := container.daemon.volumes.FindOrCreateVolume("", true) - if err != nil { - return nil, err - } - mounts[path] = &Mount{ - container: container, - MountToPath: path, - volume: vol, - Writable: true, - copyData: true, - } - } - - return mounts, nil -} - -func parseBindMountSpec(spec string) (string, string, bool, error) { - var ( - path, mountToPath string - writable bool - arr = strings.Split(spec, ":") - ) +func parseBindMountSpec(spec string) (*volumeMount, error) { + arr := strings.Split(spec, ":") + mnt := &volumeMount{} switch len(arr) { case 2: - path = arr[0] - mountToPath = arr[1] - writable = true + mnt.hostPath = arr[0] + mnt.containerPath = arr[1] + mnt.writable = true case 3: - path = arr[0] - mountToPath = arr[1] - writable = validMountMode(arr[2]) && arr[2] == "rw" + mnt.hostPath = arr[0] + mnt.containerPath = arr[1] + mnt.writable = validMountMode(arr[2]) && arr[2] == "rw" default: - return "", "", false, fmt.Errorf("Invalid volume specification: %s", spec) + return nil, fmt.Errorf("Invalid volume specification: %s", spec) } - if !filepath.IsAbs(path) { - return "", "", false, fmt.Errorf("cannot bind mount volume: %s volume paths must be absolute.", path) + if !filepath.IsAbs(mnt.hostPath) { + return nil, fmt.Errorf("cannot bind mount volume: %s volume paths must be absolute.", mnt.hostPath) } - path = filepath.Clean(path) - mountToPath = filepath.Clean(mountToPath) - return path, mountToPath, writable, nil + mnt.hostPath = filepath.Clean(mnt.hostPath) + mnt.containerPath = filepath.Clean(mnt.containerPath) + return mnt, nil } func parseVolumesFromSpec(spec string) (string, string, error) { @@ -251,54 +230,6 @@ func parseVolumesFromSpec(spec string) (string, string, error) { return id, mode, nil } -func (container *Container) applyVolumesFrom() error { - volumesFrom := container.hostConfig.VolumesFrom - if len(volumesFrom) > 0 && container.AppliedVolumesFrom == nil { - container.AppliedVolumesFrom = make(map[string]struct{}) - } - - mountGroups := make(map[string][]*Mount) - - for _, spec := range volumesFrom { - id, mode, err := parseVolumesFromSpec(spec) - if err != nil { - return err - } - if _, exists := container.AppliedVolumesFrom[id]; exists { - // Don't try to apply these since they've already been applied - continue - } - - c, err := container.daemon.Get(id) - if err != nil { - return fmt.Errorf("Could not apply volumes of non-existent container %q.", id) - } - - var ( - fromMounts = c.VolumeMounts() - mounts []*Mount - ) - - for _, mnt := range fromMounts { - mnt.Writable = mnt.Writable && (mode == "rw") - mounts = append(mounts, mnt) - } - mountGroups[id] = mounts - } - - for id, mounts := range mountGroups { - for _, mnt := range mounts { - mnt.from = mnt.container - mnt.container = container - if err := mnt.initialize(); err != nil { - return err - } - } - container.AppliedVolumesFrom[id] = struct{}{} - } - return nil -} - func validMountMode(mode string) bool { validModes := map[string]bool{ "rw": true, @@ -344,13 +275,17 @@ func (container *Container) setupMounts() error { return nil } -func (container *Container) VolumeMounts() map[string]*Mount { - mounts := make(map[string]*Mount) +func (container *Container) volumeMounts() map[string]*volumeMount { + mounts := make(map[string]*volumeMount) - for mountToPath, path := range container.Volumes { - if v := container.daemon.volumes.Get(path); v != nil { - mounts[mountToPath] = &Mount{volume: v, container: container, MountToPath: mountToPath, Writable: container.VolumesRW[mountToPath]} + for containerPath, path := range container.Volumes { + v := container.daemon.volumes.Get(path) + if v == nil { + // This should never happen + logrus.Debugf("reference by container %s to non-existent volume path %s", container.ID, path) + continue } + mounts[containerPath] = &volumeMount{hostPath: path, containerPath: containerPath, writable: container.VolumesRW[containerPath]} } return mounts diff --git a/integration-cli/docker_cli_build_test.go b/integration-cli/docker_cli_build_test.go index a4ccd18ea4..df5621af18 100644 --- a/integration-cli/docker_cli_build_test.go +++ b/integration-cli/docker_cli_build_test.go @@ -4504,7 +4504,7 @@ func (s *DockerSuite) TestBuildExoticShellInterpolation(c *check.C) { _, err := buildImage(name, ` FROM busybox - + ENV SOME_VAR a.b.c RUN [ "$SOME_VAR" = 'a.b.c' ]