diff --git a/components/engine/api/swagger.yaml b/components/engine/api/swagger.yaml index ca9d29e021..1cf310d689 100644 --- a/components/engine/api/swagger.yaml +++ b/components/engine/api/swagger.yaml @@ -1082,6 +1082,7 @@ definitions: type: "object" additionalProperties: type: "array" + x-nullable: true items: $ref: "#/definitions/PortBinding" example: @@ -1106,7 +1107,6 @@ definitions: PortBinding represents a binding between a host IP address and a host port. type: "object" - x-nullable: true properties: HostIp: description: "Host IP address that the container's port is mapped to." @@ -5351,7 +5351,7 @@ paths: /containers/{id}/resize: post: summary: "Resize a container TTY" - description: "Resize the TTY for a container. You must restart the container for the resize to take effect." + description: "Resize the TTY for a container." operationId: "ContainerResize" consumes: - "application/octet-stream" @@ -6105,12 +6105,17 @@ paths: in: "query" description: "If “1”, “true”, or “True” then it will be an error if unpacking the given content would cause an existing directory to be replaced with a non-directory and vice versa." type: "string" + - name: "copyUIDGID" + in: "query" + description: "If “1”, “true”, then it will copy UID/GID maps to the dest file or dir" + type: "string" - name: "inputStream" in: "body" required: true description: "The input stream must be a tar archive compressed with one of the following algorithms: identity (no compression), gzip, bzip2, xz." schema: type: "string" + format: "binary" tags: ["Container"] /containers/prune: post: @@ -8932,7 +8937,9 @@ paths: type: "string" RemoteAddrs: description: "Addresses of manager nodes already participating in the swarm." - type: "string" + type: "array" + items: + type: "string" JoinToken: description: "Secret token for joining this swarm." type: "string" diff --git a/components/engine/container/container_unix.go b/components/engine/container/container_unix.go index 1a184555ee..6d402be3a0 100644 --- a/components/engine/container/container_unix.go +++ b/components/engine/container/container_unix.go @@ -175,8 +175,8 @@ func (container *Container) HasMountFor(path string) bool { return false } -// UnmountIpcMount uses the provided unmount function to unmount shm if it was mounted -func (container *Container) UnmountIpcMount(unmount func(pth string) error) error { +// UnmountIpcMount unmounts shm if it was mounted +func (container *Container) UnmountIpcMount() error { if container.HasMountFor("/dev/shm") { return nil } @@ -190,10 +190,8 @@ func (container *Container) UnmountIpcMount(unmount func(pth string) error) erro if shmPath == "" { return nil } - if err = unmount(shmPath); err != nil && !os.IsNotExist(err) { - if mounted, mErr := mount.Mounted(shmPath); mounted || mErr != nil { - return errors.Wrapf(err, "umount %s", shmPath) - } + if err = mount.Unmount(shmPath); err != nil && !os.IsNotExist(err) { + return err } return nil } @@ -383,7 +381,8 @@ func (container *Container) DetachAndUnmount(volumeEventLog func(name, action st for _, mountPath := range mountPaths { if err := mount.Unmount(mountPath); err != nil { - logrus.Warnf("%s unmountVolumes: Failed to do lazy umount fo volume '%s': %v", container.ID, mountPath, err) + logrus.WithError(err).WithField("container", container.ID). + Warn("Unable to unmount") } } return container.UnmountVolumes(volumeEventLog) diff --git a/components/engine/container/container_windows.go b/components/engine/container/container_windows.go index b5bdb5bc34..090db12c20 100644 --- a/components/engine/container/container_windows.go +++ b/components/engine/container/container_windows.go @@ -22,7 +22,7 @@ const ( // UnmountIpcMount unmounts Ipc related mounts. // This is a NOOP on windows. -func (container *Container) UnmountIpcMount(unmount func(pth string) error) error { +func (container *Container) UnmountIpcMount() error { return nil } diff --git a/components/engine/daemon/cluster/nodes.go b/components/engine/daemon/cluster/nodes.go index 3c073b0bac..dffd7556f0 100644 --- a/components/engine/daemon/cluster/nodes.go +++ b/components/engine/daemon/cluster/nodes.go @@ -8,6 +8,7 @@ import ( "github.com/docker/docker/daemon/cluster/convert" "github.com/docker/docker/errdefs" swarmapi "github.com/docker/swarmkit/api" + "google.golang.org/grpc" ) // GetNodes returns a list of all nodes known to a cluster. @@ -30,7 +31,9 @@ func (c *Cluster) GetNodes(options apitypes.NodeListOptions) ([]types.Node, erro r, err := state.controlClient.ListNodes( ctx, - &swarmapi.ListNodesRequest{Filters: filters}) + &swarmapi.ListNodesRequest{Filters: filters}, + grpc.MaxCallRecvMsgSize(defaultRecvSizeForListResponse), + ) if err != nil { return nil, err } diff --git a/components/engine/daemon/cluster/secrets.go b/components/engine/daemon/cluster/secrets.go index c6fd842081..6f652eb54d 100644 --- a/components/engine/daemon/cluster/secrets.go +++ b/components/engine/daemon/cluster/secrets.go @@ -7,6 +7,7 @@ import ( types "github.com/docker/docker/api/types/swarm" "github.com/docker/docker/daemon/cluster/convert" swarmapi "github.com/docker/swarmkit/api" + "google.golang.org/grpc" ) // GetSecret returns a secret from a managed swarm cluster @@ -44,7 +45,9 @@ func (c *Cluster) GetSecrets(options apitypes.SecretListOptions) ([]types.Secret defer cancel() r, err := state.controlClient.ListSecrets(ctx, - &swarmapi.ListSecretsRequest{Filters: filters}) + &swarmapi.ListSecretsRequest{Filters: filters}, + grpc.MaxCallRecvMsgSize(defaultRecvSizeForListResponse), + ) if err != nil { return nil, err } diff --git a/components/engine/daemon/container_operations_unix.go b/components/engine/daemon/container_operations_unix.go index c0aab72342..03b01b8061 100644 --- a/components/engine/daemon/container_operations_unix.go +++ b/components/engine/daemon/container_operations_unix.go @@ -351,10 +351,6 @@ func killProcessDirectly(cntr *container.Container) error { return nil } -func detachMounted(path string) error { - return unix.Unmount(path, unix.MNT_DETACH) -} - func isLinkable(child *container.Container) bool { // A container is linkable only if it belongs to the default network _, ok := child.NetworkSettings.Networks[runconfig.DefaultDaemonNetworkMode().NetworkName()] diff --git a/components/engine/daemon/container_operations_windows.go b/components/engine/daemon/container_operations_windows.go index 349d3a1566..10bfd53d6e 100644 --- a/components/engine/daemon/container_operations_windows.go +++ b/components/engine/daemon/container_operations_windows.go @@ -78,10 +78,6 @@ func (daemon *Daemon) mountVolumes(container *container.Container) error { return nil } -func detachMounted(path string) error { - return nil -} - func (daemon *Daemon) setupSecretDir(c *container.Container) (setupErr error) { if len(c.SecretReferences) == 0 { return nil diff --git a/components/engine/daemon/daemon_unix.go b/components/engine/daemon/daemon_unix.go index 5234201c82..8b59b5278a 100644 --- a/components/engine/daemon/daemon_unix.go +++ b/components/engine/daemon/daemon_unix.go @@ -174,8 +174,8 @@ func getBlkioWeightDevices(config containertypes.Resources) ([]specs.LinuxWeight } weight := weightDevice.Weight d := specs.LinuxWeightDevice{Weight: &weight} - d.Major = int64(stat.Rdev / 256) - d.Minor = int64(stat.Rdev % 256) + d.Major = int64(unix.Major(stat.Rdev)) + d.Minor = int64(unix.Minor(stat.Rdev)) blkioWeightDevices = append(blkioWeightDevices, d) } @@ -245,8 +245,8 @@ func getBlkioThrottleDevices(devs []*blkiodev.ThrottleDevice) ([]specs.LinuxThro return nil, err } d := specs.LinuxThrottleDevice{Rate: d.Rate} - d.Major = int64(stat.Rdev / 256) - d.Minor = int64(stat.Rdev % 256) + d.Major = int64(unix.Major(stat.Rdev)) + d.Minor = int64(unix.Minor(stat.Rdev)) throttleDevices = append(throttleDevices, d) } diff --git a/components/engine/daemon/daemon_unix_test.go b/components/engine/daemon/daemon_unix_test.go index 36c6030988..9ea2560d97 100644 --- a/components/engine/daemon/daemon_unix_test.go +++ b/components/engine/daemon/daemon_unix_test.go @@ -6,11 +6,16 @@ import ( "errors" "io/ioutil" "os" + "path/filepath" "testing" + "github.com/docker/docker/api/types/blkiodev" containertypes "github.com/docker/docker/api/types/container" "github.com/docker/docker/container" "github.com/docker/docker/daemon/config" + "golang.org/x/sys/unix" + "gotest.tools/assert" + is "gotest.tools/assert/cmp" ) type fakeContainerGetter struct { @@ -266,3 +271,61 @@ func TestNetworkOptions(t *testing.T) { t.Fatal("Expected networkOptions error, got nil") } } + +const ( + // prepare major 0x1FD(509 in decimal) and minor 0x130(304) + DEVNO = 0x11FD30 + MAJOR = 509 + MINOR = 304 + WEIGHT = 1024 +) + +func deviceTypeMock(t *testing.T, testAndCheck func(string)) { + if os.Getuid() != 0 { + t.Skip("root required") // for mknod + } + + t.Parallel() + + tempDir, err := ioutil.TempDir("", "tempDevDir"+t.Name()) + assert.NilError(t, err, "create temp file") + tempFile := filepath.Join(tempDir, "dev") + + defer os.RemoveAll(tempDir) + + if err = unix.Mknod(tempFile, unix.S_IFCHR, DEVNO); err != nil { + t.Fatalf("mknod error %s(%x): %v", tempFile, DEVNO, err) + } + + testAndCheck(tempFile) +} + +func TestGetBlkioWeightDevices(t *testing.T) { + deviceTypeMock(t, func(tempFile string) { + mockResource := containertypes.Resources{ + BlkioWeightDevice: []*blkiodev.WeightDevice{{Path: tempFile, Weight: WEIGHT}}, + } + + weightDevs, err := getBlkioWeightDevices(mockResource) + + assert.NilError(t, err, "getBlkioWeightDevices") + assert.Check(t, is.Len(weightDevs, 1), "getBlkioWeightDevices") + assert.Check(t, weightDevs[0].Major == MAJOR, "get major device type") + assert.Check(t, weightDevs[0].Minor == MINOR, "get minor device type") + assert.Check(t, *weightDevs[0].Weight == WEIGHT, "get device weight") + }) +} + +func TestGetBlkioThrottleDevices(t *testing.T) { + deviceTypeMock(t, func(tempFile string) { + mockDevs := []*blkiodev.ThrottleDevice{{Path: tempFile, Rate: WEIGHT}} + + retDevs, err := getBlkioThrottleDevices(mockDevs) + + assert.NilError(t, err, "getBlkioThrottleDevices") + assert.Check(t, is.Len(retDevs, 1), "getBlkioThrottleDevices") + assert.Check(t, retDevs[0].Major == MAJOR, "get major device type") + assert.Check(t, retDevs[0].Minor == MINOR, "get minor device type") + assert.Check(t, retDevs[0].Rate == WEIGHT, "get device rate") + }) +} diff --git a/components/engine/daemon/graphdriver/aufs/aufs.go b/components/engine/daemon/graphdriver/aufs/aufs.go index 114aa9a615..bbd19a82b0 100644 --- a/components/engine/daemon/graphdriver/aufs/aufs.go +++ b/components/engine/daemon/graphdriver/aufs/aufs.go @@ -43,7 +43,7 @@ import ( "github.com/docker/docker/pkg/directory" "github.com/docker/docker/pkg/idtools" "github.com/docker/docker/pkg/locker" - mountpk "github.com/docker/docker/pkg/mount" + "github.com/docker/docker/pkg/mount" "github.com/docker/docker/pkg/system" rsystem "github.com/opencontainers/runc/libcontainer/system" "github.com/opencontainers/selinux/go-selinux/label" @@ -72,7 +72,6 @@ func init() { // Driver contains information about the filesystem mounted. type Driver struct { - sync.Mutex root string uidMaps []idtools.IDMap gidMaps []idtools.IDMap @@ -81,6 +80,7 @@ type Driver struct { pathCache map[string]string naiveDiff graphdriver.DiffDriver locker *locker.Locker + mntL sync.Mutex } // Init returns a new AUFS driver. @@ -327,11 +327,11 @@ func (a *Driver) Remove(id string) error { break } - if err != unix.EBUSY { - return errors.Wrapf(err, "aufs: unmount error: %s", mountpoint) + if errors.Cause(err) != unix.EBUSY { + return errors.Wrap(err, "aufs: unmount error") } if retries >= 5 { - return errors.Wrapf(err, "aufs: unmount error after retries: %s", mountpoint) + return errors.Wrap(err, "aufs: unmount error after retries") } // If unmount returns EBUSY, it could be a transient error. Sleep and retry. retries++ @@ -437,7 +437,7 @@ func (a *Driver) Put(id string) error { err := a.unmount(m) if err != nil { - logger.Debugf("Failed to unmount %s aufs: %v", id, err) + logger.WithError(err).WithField("method", "Put()").Warn() } return err } @@ -547,9 +547,6 @@ func (a *Driver) getParentLayerPaths(id string) ([]string, error) { } func (a *Driver) mount(id string, target string, mountLabel string, layers []string) error { - a.Lock() - defer a.Unlock() - // If the id is mounted or we get an error return if mounted, err := a.mounted(target); err != nil || mounted { return err @@ -564,9 +561,6 @@ func (a *Driver) mount(id string, target string, mountLabel string, layers []str } func (a *Driver) unmount(mountPath string) error { - a.Lock() - defer a.Unlock() - if mounted, err := a.mounted(mountPath); err != nil || !mounted { return err } @@ -579,32 +573,29 @@ func (a *Driver) mounted(mountpoint string) (bool, error) { // Cleanup aufs and unmount all mountpoints func (a *Driver) Cleanup() error { - var dirs []string - if err := filepath.Walk(a.mntPath(), func(path string, info os.FileInfo, err error) error { - if err != nil { - return err - } - if !info.IsDir() { - return nil - } - dirs = append(dirs, path) - return nil - }); err != nil { - return err + dir := a.mntPath() + files, err := ioutil.ReadDir(dir) + if err != nil { + return errors.Wrap(err, "aufs readdir error") } + for _, f := range files { + if !f.IsDir() { + continue + } + + m := path.Join(dir, f.Name()) - for _, m := range dirs { if err := a.unmount(m); err != nil { - logger.Debugf("error unmounting %s: %s", m, err) + logger.WithError(err).WithField("method", "Cleanup()").Warn() } } - return mountpk.RecursiveUnmount(a.root) + return mount.RecursiveUnmount(a.root) } func (a *Driver) aufsMount(ro []string, rw, target, mountLabel string) (err error) { defer func() { if err != nil { - Unmount(target) + mount.Unmount(target) } }() @@ -632,14 +623,29 @@ func (a *Driver) aufsMount(ro []string, rw, target, mountLabel string) (err erro opts += ",dirperm1" } data := label.FormatMountLabel(fmt.Sprintf("%s,%s", string(b[:bp]), opts), mountLabel) - if err = mount("none", target, "aufs", 0, data); err != nil { + a.mntL.Lock() + err = unix.Mount("none", target, "aufs", 0, data) + a.mntL.Unlock() + if err != nil { + err = errors.Wrap(err, "mount target="+target+" data="+data) return } - for ; index < len(ro); index++ { - layer := fmt.Sprintf(":%s=ro+wh", ro[index]) - data := label.FormatMountLabel(fmt.Sprintf("append%s", layer), mountLabel) - if err = mount("none", target, "aufs", unix.MS_REMOUNT, data); err != nil { + for index < len(ro) { + bp = 0 + for ; index < len(ro); index++ { + layer := fmt.Sprintf("append:%s=ro+wh,", ro[index]) + if bp+len(layer) > len(b) { + break + } + bp += copy(b[bp:], layer) + } + data := label.FormatMountLabel(string(b[:bp]), mountLabel) + a.mntL.Lock() + err = unix.Mount("none", target, "aufs", unix.MS_REMOUNT, data) + a.mntL.Unlock() + if err != nil { + err = errors.Wrap(err, "mount target="+target+" flags=MS_REMOUNT data="+data) return } } @@ -666,7 +672,7 @@ func useDirperm() bool { defer os.RemoveAll(union) opts := fmt.Sprintf("br:%s,dirperm1,xino=/dev/shm/aufs.xino", base) - if err := mount("none", union, "aufs", 0, opts); err != nil { + if err := unix.Mount("none", union, "aufs", 0, opts); err != nil { return } enableDirperm = true diff --git a/components/engine/daemon/graphdriver/aufs/mount.go b/components/engine/daemon/graphdriver/aufs/mount.go index 9f5510380c..fc20a5eca6 100644 --- a/components/engine/daemon/graphdriver/aufs/mount.go +++ b/components/engine/daemon/graphdriver/aufs/mount.go @@ -4,14 +4,38 @@ package aufs // import "github.com/docker/docker/daemon/graphdriver/aufs" import ( "os/exec" + "syscall" - "golang.org/x/sys/unix" + "github.com/docker/docker/pkg/mount" ) // Unmount the target specified. func Unmount(target string) error { - if err := exec.Command("auplink", target, "flush").Run(); err != nil { - logger.WithError(err).Warnf("Couldn't run auplink before unmount %s", target) + const ( + EINVAL = 22 // if auplink returns this, + retries = 3 // retry a few times + ) + + for i := 0; ; i++ { + out, err := exec.Command("auplink", target, "flush").CombinedOutput() + if err == nil { + break + } + rc := 0 + if exiterr, ok := err.(*exec.ExitError); ok { + if status, ok := exiterr.Sys().(syscall.WaitStatus); ok { + rc = status.ExitStatus() + } + } + if i >= retries || rc != EINVAL { + logger.WithError(err).WithField("method", "Unmount").Warnf("auplink flush failed: %s", out) + break + } + // auplink failed to find target in /proc/self/mounts because + // kernel can't guarantee continuity while reading from it + // while mounts table is being changed + logger.Debugf("auplink flush error (retrying %d/%d): %s", i+1, retries, out) } - return unix.Unmount(target, 0) + + return mount.Unmount(target) } diff --git a/components/engine/daemon/graphdriver/aufs/mount_linux.go b/components/engine/daemon/graphdriver/aufs/mount_linux.go deleted file mode 100644 index 8d5ad8f32d..0000000000 --- a/components/engine/daemon/graphdriver/aufs/mount_linux.go +++ /dev/null @@ -1,7 +0,0 @@ -package aufs // import "github.com/docker/docker/daemon/graphdriver/aufs" - -import "golang.org/x/sys/unix" - -func mount(source string, target string, fstype string, flags uintptr, data string) error { - return unix.Mount(source, target, fstype, flags, data) -} diff --git a/components/engine/daemon/graphdriver/aufs/mount_unsupported.go b/components/engine/daemon/graphdriver/aufs/mount_unsupported.go deleted file mode 100644 index cf7f58c29e..0000000000 --- a/components/engine/daemon/graphdriver/aufs/mount_unsupported.go +++ /dev/null @@ -1,12 +0,0 @@ -// +build !linux - -package aufs // import "github.com/docker/docker/daemon/graphdriver/aufs" - -import "errors" - -// MsRemount declared to specify a non-linux system mount. -const MsRemount = 0 - -func mount(source string, target string, fstype string, flags uintptr, data string) (err error) { - return errors.New("mount is not implemented on this platform") -} diff --git a/components/engine/daemon/graphdriver/btrfs/btrfs.go b/components/engine/daemon/graphdriver/btrfs/btrfs.go index 7ce7edef36..fcaedc6eab 100644 --- a/components/engine/daemon/graphdriver/btrfs/btrfs.go +++ b/components/engine/daemon/graphdriver/btrfs/btrfs.go @@ -178,7 +178,7 @@ func (d *Driver) Cleanup() error { } if umountErr != nil { - return errors.Wrapf(umountErr, "error unmounting %s", d.home) + return umountErr } return nil diff --git a/components/engine/daemon/graphdriver/devmapper/deviceset.go b/components/engine/daemon/graphdriver/devmapper/deviceset.go index 5dc01d71d9..c41b50c119 100644 --- a/components/engine/daemon/graphdriver/devmapper/deviceset.go +++ b/components/engine/daemon/graphdriver/devmapper/deviceset.go @@ -1200,7 +1200,7 @@ func (devices *DeviceSet) growFS(info *devInfo) error { options = joinMountOptions(options, devices.mountOptions) if err := mount.Mount(info.DevName(), fsMountPoint, devices.BaseDeviceFilesystem, options); err != nil { - return fmt.Errorf("Error mounting '%s' on '%s' (fstype='%s' options='%s'): %s\n%v", info.DevName(), fsMountPoint, devices.BaseDeviceFilesystem, options, err, string(dmesg.Dmesg(256))) + return errors.Wrapf(err, "Failed to mount; dmesg: %s", string(dmesg.Dmesg(256))) } defer unix.Unmount(fsMountPoint, unix.MNT_DETACH) @@ -2381,7 +2381,7 @@ func (devices *DeviceSet) MountDevice(hash, path, mountLabel string) error { options = joinMountOptions(options, label.FormatMountLabel("", mountLabel)) if err := mount.Mount(info.DevName(), path, fstype, options); err != nil { - return fmt.Errorf("devmapper: Error mounting '%s' on '%s' (fstype='%s' options='%s'): %s\n%v", info.DevName(), path, fstype, options, err, string(dmesg.Dmesg(256))) + return errors.Wrapf(err, "Failed to mount; dmesg: %s", string(dmesg.Dmesg(256))) } if fstype == "xfs" && devices.xfsNospaceRetries != "" { diff --git a/components/engine/daemon/graphdriver/devmapper/driver.go b/components/engine/daemon/graphdriver/devmapper/driver.go index 899b1f8670..a42a03ba90 100644 --- a/components/engine/daemon/graphdriver/devmapper/driver.go +++ b/components/engine/daemon/graphdriver/devmapper/driver.go @@ -16,7 +16,6 @@ import ( "github.com/docker/docker/pkg/locker" "github.com/docker/docker/pkg/mount" "github.com/docker/go-units" - "github.com/pkg/errors" "github.com/sirupsen/logrus" "golang.org/x/sys/unix" ) @@ -129,11 +128,7 @@ func (d *Driver) Cleanup() error { return err } - if umountErr != nil { - return errors.Wrapf(umountErr, "error unmounting %s", d.home) - } - - return nil + return umountErr } // CreateReadWrite creates a layer that is writable for use as a container diff --git a/components/engine/daemon/graphdriver/windows/windows.go b/components/engine/daemon/graphdriver/windows/windows.go index 52b0c6d845..699e434f4c 100644 --- a/components/engine/daemon/graphdriver/windows/windows.go +++ b/components/engine/daemon/graphdriver/windows/windows.go @@ -338,11 +338,14 @@ func (d *Driver) Remove(id string) error { // If permission denied, it's possible that the scratch is still mounted, an // artifact after a hard daemon crash for example. Worth a shot to try detaching it // before retrying the rename. - if detachErr := vhd.DetachVhd(filepath.Join(layerPath, "sandbox.vhdx")); detachErr != nil { - return errors.Wrapf(err, "failed to detach VHD: %s", detachErr) - } - if renameErr := os.Rename(layerPath, tmpLayerPath); renameErr != nil && !os.IsNotExist(renameErr) { - return errors.Wrapf(err, "second rename attempt following detach failed: %s", renameErr) + sandbox := filepath.Join(layerPath, "sandbox.vhdx") + if _, statErr := os.Stat(sandbox); statErr == nil { + if detachErr := vhd.DetachVhd(sandbox); detachErr != nil { + return errors.Wrapf(err, "failed to detach VHD: %s", detachErr) + } + if renameErr := os.Rename(layerPath, tmpLayerPath); renameErr != nil && !os.IsNotExist(renameErr) { + return errors.Wrapf(err, "second rename attempt following detach failed: %s", renameErr) + } } } if err := hcsshim.DestroyLayer(d.info, tmpID); err != nil { diff --git a/components/engine/daemon/graphdriver/zfs/zfs.go b/components/engine/daemon/graphdriver/zfs/zfs.go index 8a798778d2..c83446cf8f 100644 --- a/components/engine/daemon/graphdriver/zfs/zfs.go +++ b/components/engine/daemon/graphdriver/zfs/zfs.go @@ -19,6 +19,7 @@ import ( "github.com/docker/docker/pkg/parsers" "github.com/mistifyio/go-zfs" "github.com/opencontainers/selinux/go-selinux/label" + "github.com/pkg/errors" "github.com/sirupsen/logrus" "golang.org/x/sys/unix" ) @@ -390,7 +391,7 @@ func (d *Driver) Get(id, mountLabel string) (_ containerfs.ContainerFS, retErr e } if err := mount.Mount(filesystem, mountpoint, "zfs", options); err != nil { - return nil, fmt.Errorf("error creating zfs mount of %s to %s: %v", filesystem, mountpoint, err) + return nil, errors.Wrap(err, "error creating zfs mount") } // this could be our first mount after creation of the filesystem, and the root dir may still have root diff --git a/components/engine/daemon/start.go b/components/engine/daemon/start.go index e2265a4fae..e2416efe5e 100644 --- a/components/engine/daemon/start.go +++ b/components/engine/daemon/start.go @@ -229,7 +229,7 @@ func (daemon *Daemon) containerStart(container *container.Container, checkpoint func (daemon *Daemon) Cleanup(container *container.Container) { daemon.releaseNetwork(container) - if err := container.UnmountIpcMount(detachMounted); err != nil { + if err := container.UnmountIpcMount(); err != nil { logrus.Warnf("%s cleanup: failed to unmount IPC: %s", container.ID, err) } diff --git a/components/engine/docs/api/version-history.md b/components/engine/docs/api/version-history.md index 6f0083e90e..d98c16943a 100644 --- a/components/engine/docs/api/version-history.md +++ b/components/engine/docs/api/version-history.md @@ -159,6 +159,7 @@ keywords: "API, Docker, rcli, REST, documentation" * `GET /events` now supports service, node and secret events which are emitted when users create, update and remove service, node and secret * `GET /events` now supports network remove event which is emitted when users remove a swarm scoped network * `GET /events` now supports a filter type `scope` in which supported value could be swarm and local +* `PUT /containers/(name)/archive` now accepts a `copyUIDGID` parameter to allow copy UID/GID maps to dest file or dir. ## v1.29 API changes diff --git a/components/engine/hack/ci/windows.ps1 b/components/engine/hack/ci/windows.ps1 index c2c937f4cb..6d87f3256b 100644 --- a/components/engine/hack/ci/windows.ps1 +++ b/components/engine/hack/ci/windows.ps1 @@ -119,6 +119,7 @@ $FinallyColour="Cyan" #$env:INTEGRATION_IN_CONTAINER="yes" #$env:WINDOWS_BASE_IMAGE="" #$env:SKIP_COPY_GO="yes" +#$env:INTEGRATION_TESTFLAGS="-test.v" Function Nuke-Everything { $ErrorActionPreference = 'SilentlyContinue' @@ -409,7 +410,7 @@ Try { # Redirect to a temporary location. $TEMPORIG=$env:TEMP $env:TEMP="$env:TESTRUN_DRIVE`:\$env:TESTRUN_SUBDIR\CI-$COMMITHASH" - $env:LOCALAPPDATA="$TEMP\localappdata" + $env:LOCALAPPDATA="$env:TEMP\localappdata" $errorActionPreference='Stop' New-Item -ItemType Directory "$env:TEMP" -ErrorAction SilentlyContinue | Out-Null New-Item -ItemType Directory "$env:TEMP\userprofile" -ErrorAction SilentlyContinue | Out-Null @@ -825,18 +826,32 @@ Try { docker ` "`$env`:PATH`='c`:\target;'+`$env:PATH`; `$env:DOCKER_HOST`='tcp`://'+(ipconfig | select -last 1).Substring(39)+'`:2357'; c:\target\runIntegrationCLI.ps1" | Out-Host } ) } else { - Write-Host -ForegroundColor Green "INFO: Integration tests being run from the host:" - Set-Location "$env:SOURCES_DRIVE`:\$env:SOURCES_SUBDIR\src\github.com\docker\docker\integration-cli" $env:DOCKER_HOST=$DASHH_CUT $env:PATH="$env:TEMP\binary;$env:PATH;" # Force to use the test binaries, not the host ones. - Write-Host -ForegroundColor Green "INFO: $c" Write-Host -ForegroundColor Green "INFO: DOCKER_HOST at $DASHH_CUT" + + $ErrorActionPreference = "SilentlyContinue" + Write-Host -ForegroundColor Cyan "INFO: Integration API tests being run from the host:" + if (!($env:INTEGRATION_TESTFLAGS)) { + $env:INTEGRATION_TESTFLAGS = "-test.v" + } + Set-Location "$env:SOURCES_DRIVE`:\$env:SOURCES_SUBDIR\src\github.com\docker\docker" + $start=(Get-Date); Invoke-Expression ".\hack\make.ps1 -TestIntegration"; $Duration=New-Timespan -Start $start -End (Get-Date) + $ErrorActionPreference = "Stop" + if (-not($LastExitCode -eq 0)) { + Throw "ERROR: Integration API tests failed at $(Get-Date). Duration`:$Duration" + } + + $ErrorActionPreference = "SilentlyContinue" + Write-Host -ForegroundColor Green "INFO: Integration CLI tests being run from the host:" + Write-Host -ForegroundColor Green "INFO: $c" + Set-Location "$env:SOURCES_DRIVE`:\$env:SOURCES_SUBDIR\src\github.com\docker\docker\integration-cli" # Explicit to not use measure-command otherwise don't get output as it goes $start=(Get-Date); Invoke-Expression $c; $Duration=New-Timespan -Start $start -End (Get-Date) } $ErrorActionPreference = "Stop" if (-not($LastExitCode -eq 0)) { - Throw "ERROR: Integration tests failed at $(Get-Date). Duration`:$Duration" + Throw "ERROR: Integration CLI tests failed at $(Get-Date). Duration`:$Duration" } Write-Host -ForegroundColor Green "INFO: Integration tests ended at $(Get-Date). Duration`:$Duration" } else { diff --git a/components/engine/hack/make.ps1 b/components/engine/hack/make.ps1 index 6221e364d6..ab2a978e76 100644 --- a/components/engine/hack/make.ps1 +++ b/components/engine/hack/make.ps1 @@ -60,6 +60,9 @@ .PARAMETER TestUnit Runs unit tests. +.PARAMETER TestIntegration + Runs integration tests. + .PARAMETER All Runs everything this script knows about that can run in a container. @@ -84,6 +87,7 @@ param( [Parameter(Mandatory=$False)][switch]$PkgImports, [Parameter(Mandatory=$False)][switch]$GoFormat, [Parameter(Mandatory=$False)][switch]$TestUnit, + [Parameter(Mandatory=$False)][switch]$TestIntegration, [Parameter(Mandatory=$False)][switch]$All ) @@ -320,6 +324,39 @@ Function Run-UnitTests() { if ($LASTEXITCODE -ne 0) { Throw "Unit tests failed" } } +# Run the integration tests +Function Run-IntegrationTests() { + $env:DOCKER_INTEGRATION_DAEMON_DEST = $root + "\bundles\tmp" + $dirs = Get-ChildItem -Path integration -Directory -Recurse + $integration_api_dirs = @() + ForEach($dir in $dirs) { + $RelativePath = "." + $dir.FullName -replace "$($PWD.Path -replace "\\","\\")","" + If ($RelativePath -notmatch '(^.\\integration($|\\internal)|\\testdata)') { + $integration_api_dirs += $dir + Write-Host "Building test suite binary $RelativePath" + go test -c -o "$RelativePath\test.exe" $RelativePath + } + } + + ForEach($dir in $integration_api_dirs) { + Set-Location $dir.FullName + Write-Host "Running $($PWD.Path)" + $pinfo = New-Object System.Diagnostics.ProcessStartInfo + $pinfo.FileName = "$($PWD.Path)\test.exe" + $pinfo.RedirectStandardError = $true + $pinfo.UseShellExecute = $false + $pinfo.Arguments = $env:INTEGRATION_TESTFLAGS + $p = New-Object System.Diagnostics.Process + $p.StartInfo = $pinfo + $p.Start() | Out-Null + $p.WaitForExit() + $err = $p.StandardError.ReadToEnd() + if (($LASTEXITCODE -ne 0) -and ($err -notlike "*warning: no tests to run*")) { + Throw "Integration tests failed: $err" + } + } +} + # Start of main code. Try { Write-Host -ForegroundColor Cyan "INFO: make.ps1 starting at $(Get-Date)" @@ -331,13 +368,13 @@ Try { # Handle the "-All" shortcut to turn on all things we can handle. # Note we expressly only include the items which can run in a container - the validations tests cannot # as they require the .git directory which is excluded from the image by .dockerignore - if ($All) { $Client=$True; $Daemon=$True; $TestUnit=$True } + if ($All) { $Client=$True; $Daemon=$True; $TestUnit=$True; } # Handle the "-Binary" shortcut to build both client and daemon. if ($Binary) { $Client = $True; $Daemon = $True } # Default to building the daemon if not asked for anything explicitly. - if (-not($Client) -and -not($Daemon) -and -not($DCO) -and -not($PkgImports) -and -not($GoFormat) -and -not($TestUnit)) { $Daemon=$True } + if (-not($Client) -and -not($Daemon) -and -not($DCO) -and -not($PkgImports) -and -not($GoFormat) -and -not($TestUnit) -and -not($TestIntegration)) { $Daemon=$True } # Verify git is installed if ($(Get-Command git -ErrorAction SilentlyContinue) -eq $nil) { Throw "Git does not appear to be installed" } @@ -351,6 +388,8 @@ Try { # Get the version of docker (eg 17.04.0-dev) $dockerVersion="0.0.0-dev" + # Overwrite dockerVersion if VERSION Environment variable is available + if (Test-Path Env:\VERSION) { $dockerVersion=$env:VERSION } # Give a warning if we are not running in a container and are building binaries or running unit tests. # Not relevant for validation tests as these are fine to run outside of a container. @@ -423,6 +462,9 @@ Try { # Run unit tests if ($TestUnit) { Run-UnitTests } + # Run integration tests + if ($TestIntegration) { Run-IntegrationTests } + # Gratuitous ASCII art. if ($Daemon -or $Client) { Write-Host diff --git a/components/engine/image/rootfs.go b/components/engine/image/rootfs.go index 84843e10c6..f73a0660fa 100644 --- a/components/engine/image/rootfs.go +++ b/components/engine/image/rootfs.go @@ -38,7 +38,8 @@ func (r *RootFS) Append(id layer.DiffID) { func (r *RootFS) Clone() *RootFS { newRoot := NewRootFS() newRoot.Type = r.Type - newRoot.DiffIDs = append(r.DiffIDs) + newRoot.DiffIDs = make([]layer.DiffID, len(r.DiffIDs)) + copy(newRoot.DiffIDs, r.DiffIDs) return newRoot } diff --git a/components/engine/integration-cli/docker_cli_daemon_test.go b/components/engine/integration-cli/docker_cli_daemon_test.go index d1a709b736..96cd2dc78a 100644 --- a/components/engine/integration-cli/docker_cli_daemon_test.go +++ b/components/engine/integration-cli/docker_cli_daemon_test.go @@ -1777,7 +1777,7 @@ func (s *DockerDaemonSuite) TestDaemonNoSpaceLeftOnDeviceError(c *check.C) { dockerCmd(c, "run", "--rm", "-v", testDir+":/test", "busybox", "sh", "-c", "dd of=/test/testfs.img bs=1M seek=3 count=0") icmd.RunCommand("mkfs.ext4", "-F", filepath.Join(testDir, "testfs.img")).Assert(c, icmd.Success) - dockerCmd(c, "run", "--privileged", "--rm", "-v", testDir+":/test:shared", "busybox", "sh", "-c", "mkdir -p /test/test-mount/vfs && mount -n /test/testfs.img /test/test-mount/vfs") + dockerCmd(c, "run", "--privileged", "--rm", "-v", testDir+":/test:shared", "busybox", "sh", "-c", "mkdir -p /test/test-mount/vfs && mount -n -t ext4 /test/testfs.img /test/test-mount/vfs") defer mount.Unmount(filepath.Join(testDir, "test-mount")) s.d.Start(c, "--storage-driver", "vfs", "--data-root", filepath.Join(testDir, "test-mount")) diff --git a/components/engine/integration-cli/docker_cli_ps_test.go b/components/engine/integration-cli/docker_cli_ps_test.go index f34cd0f8a8..cab75f4696 100644 --- a/components/engine/integration-cli/docker_cli_ps_test.go +++ b/components/engine/integration-cli/docker_cli_ps_test.go @@ -439,15 +439,11 @@ func (s *DockerSuite) TestPsListContainersFilterLabel(c *check.C) { func (s *DockerSuite) TestPsListContainersFilterExited(c *check.C) { runSleepingContainer(c, "--name=sleep") - dockerCmd(c, "run", "--name", "zero1", "busybox", "true") - firstZero := getIDByName(c, "zero1") - - dockerCmd(c, "run", "--name", "zero2", "busybox", "true") - secondZero := getIDByName(c, "zero2") + firstZero, _ := dockerCmd(c, "run", "-d", "busybox", "true") + secondZero, _ := dockerCmd(c, "run", "-d", "busybox", "true") out, _, err := dockerCmdWithError("run", "--name", "nonzero1", "busybox", "false") c.Assert(err, checker.NotNil, check.Commentf("Should fail.", out, err)) - firstNonZero := getIDByName(c, "nonzero1") out, _, err = dockerCmdWithError("run", "--name", "nonzero2", "busybox", "false") @@ -456,17 +452,16 @@ func (s *DockerSuite) TestPsListContainersFilterExited(c *check.C) { // filter containers by exited=0 out, _ = dockerCmd(c, "ps", "-a", "-q", "--no-trunc", "--filter=exited=0") - ids := strings.Split(strings.TrimSpace(out), "\n") - c.Assert(ids, checker.HasLen, 2, check.Commentf("Should be 2 zero exited containers got %d: %s", len(ids), out)) - c.Assert(ids[0], checker.Equals, secondZero, check.Commentf("First in list should be %q, got %q", secondZero, ids[0])) - c.Assert(ids[1], checker.Equals, firstZero, check.Commentf("Second in list should be %q, got %q", firstZero, ids[1])) + c.Assert(out, checker.Contains, strings.TrimSpace(firstZero)) + c.Assert(out, checker.Contains, strings.TrimSpace(secondZero)) + c.Assert(out, checker.Not(checker.Contains), strings.TrimSpace(firstNonZero)) + c.Assert(out, checker.Not(checker.Contains), strings.TrimSpace(secondNonZero)) out, _ = dockerCmd(c, "ps", "-a", "-q", "--no-trunc", "--filter=exited=1") - ids = strings.Split(strings.TrimSpace(out), "\n") - c.Assert(ids, checker.HasLen, 2, check.Commentf("Should be 2 zero exited containers got %d", len(ids))) - c.Assert(ids[0], checker.Equals, secondNonZero, check.Commentf("First in list should be %q, got %q", secondNonZero, ids[0])) - c.Assert(ids[1], checker.Equals, firstNonZero, check.Commentf("Second in list should be %q, got %q", firstNonZero, ids[1])) - + c.Assert(out, checker.Contains, strings.TrimSpace(firstNonZero)) + c.Assert(out, checker.Contains, strings.TrimSpace(secondNonZero)) + c.Assert(out, checker.Not(checker.Contains), strings.TrimSpace(firstZero)) + c.Assert(out, checker.Not(checker.Contains), strings.TrimSpace(secondZero)) } func (s *DockerSuite) TestPsRightTagName(c *check.C) { diff --git a/components/engine/integration-cli/docker_cli_run_unix_test.go b/components/engine/integration-cli/docker_cli_run_unix_test.go index cd3f5e356b..46dff49161 100644 --- a/components/engine/integration-cli/docker_cli_run_unix_test.go +++ b/components/engine/integration-cli/docker_cli_run_unix_test.go @@ -1544,6 +1544,10 @@ func (s *DockerDaemonSuite) TestRunWithDaemonDefaultSeccompProfile(c *check.C) { { "name": "chmod", "action": "SCMP_ACT_ERRNO" + }, + { + "name": "fchmodat", + "action": "SCMP_ACT_ERRNO" } ] }` diff --git a/components/engine/integration-cli/docker_cli_search_test.go b/components/engine/integration-cli/docker_cli_search_test.go index 2f811d4b96..95ad9ce1b2 100644 --- a/components/engine/integration-cli/docker_cli_search_test.go +++ b/components/engine/integration-cli/docker_cli_search_test.go @@ -44,54 +44,6 @@ func (s *DockerSuite) TestSearchStarsOptionWithWrongParameter(c *check.C) { assert.Assert(c, strings.Contains(out, "invalid syntax"), "couldn't find the invalid value warning") } -func (s *DockerSuite) TestSearchCmdOptions(c *check.C) { - testRequires(c, Network, DaemonIsLinux) - - out, _ := dockerCmd(c, "search", "--help") - assert.Assert(c, strings.Contains(out, "Usage:\tdocker search [OPTIONS] TERM")) - - outSearchCmd, _ := dockerCmd(c, "search", "busybox") - outSearchCmdNotrunc, _ := dockerCmd(c, "search", "--no-trunc=true", "busybox") - - assert.Assert(c, len(outSearchCmd) <= len(outSearchCmdNotrunc), "The no-trunc option can't take effect.") - - outSearchCmdautomated, _ := dockerCmd(c, "search", "--filter", "is-automated=true", "busybox") //The busybox is a busybox base image, not an AUTOMATED image. - outSearchCmdautomatedSlice := strings.Split(outSearchCmdautomated, "\n") - for i := range outSearchCmdautomatedSlice { - assert.Assert(c, !strings.HasPrefix(outSearchCmdautomatedSlice[i], "busybox "), "The busybox is not an AUTOMATED image: %s", outSearchCmdautomated) - } - - outSearchCmdNotOfficial, _ := dockerCmd(c, "search", "--filter", "is-official=false", "busybox") //The busybox is a busybox base image, official image. - outSearchCmdNotOfficialSlice := strings.Split(outSearchCmdNotOfficial, "\n") - for i := range outSearchCmdNotOfficialSlice { - assert.Assert(c, !strings.HasPrefix(outSearchCmdNotOfficialSlice[i], "busybox "), "The busybox is not an OFFICIAL image: %s", outSearchCmdNotOfficial) - } - - outSearchCmdOfficial, _ := dockerCmd(c, "search", "--filter", "is-official=true", "busybox") //The busybox is a busybox base image, official image. - outSearchCmdOfficialSlice := strings.Split(outSearchCmdOfficial, "\n") - assert.Equal(c, len(outSearchCmdOfficialSlice), 3) // 1 header, 1 line, 1 carriage return - assert.Assert(c, strings.HasPrefix(outSearchCmdOfficialSlice[1], "busybox "), "The busybox is an OFFICIAL image: %s", outSearchCmdOfficial) - - outSearchCmdStars, _ := dockerCmd(c, "search", "--filter", "stars=2", "busybox") - assert.Assert(c, strings.Count(outSearchCmdStars, "[OK]") <= strings.Count(outSearchCmd, "[OK]"), "The quantity of images with stars should be less than that of all images: %s", outSearchCmdStars) - - dockerCmd(c, "search", "--filter", "is-automated=true", "--filter", "stars=2", "--no-trunc=true", "busybox") - - // --automated deprecated since Docker 1.13 - outSearchCmdautomated1, _ := dockerCmd(c, "search", "--automated=true", "busybox") //The busybox is a busybox base image, not an AUTOMATED image. - outSearchCmdautomatedSlice1 := strings.Split(outSearchCmdautomated1, "\n") - for i := range outSearchCmdautomatedSlice1 { - assert.Assert(c, !strings.HasPrefix(outSearchCmdautomatedSlice1[i], "busybox "), "The busybox is not an AUTOMATED image: %s", outSearchCmdautomated) - } - - // -s --stars deprecated since Docker 1.13 - outSearchCmdStars1, _ := dockerCmd(c, "search", "--stars=2", "busybox") - assert.Assert(c, strings.Count(outSearchCmdStars1, "[OK]") <= strings.Count(outSearchCmd, "[OK]"), "The quantity of images with stars should be less than that of all images: %s", outSearchCmdStars1) - - // -s --stars deprecated since Docker 1.13 - dockerCmd(c, "search", "--stars=2", "--automated=true", "--no-trunc=true", "busybox") -} - // search for repos which start with "ubuntu-" on the central registry func (s *DockerSuite) TestSearchOnCentralRegistryWithDash(c *check.C) { testRequires(c, Network, DaemonIsLinux) diff --git a/components/engine/integration/internal/container/container.go b/components/engine/integration/internal/container/container.go index 20ad774242..85e6a24fe7 100644 --- a/components/engine/integration/internal/container/container.go +++ b/components/engine/integration/internal/container/container.go @@ -2,6 +2,7 @@ package container import ( "context" + "runtime" "testing" "github.com/docker/docker/api/types" @@ -24,10 +25,14 @@ type TestContainerConfig struct { // nolint: golint func Create(t *testing.T, ctx context.Context, client client.APIClient, ops ...func(*TestContainerConfig)) string { // nolint: golint t.Helper() + cmd := []string{"top"} + if runtime.GOOS == "windows" { + cmd = []string{"sleep", "240"} + } config := &TestContainerConfig{ Config: &container.Config{ Image: "busybox", - Cmd: []string{"top"}, + Cmd: cmd, }, HostConfig: &container.HostConfig{}, NetworkingConfig: &network.NetworkingConfig{}, diff --git a/components/engine/integration/system/info_test.go b/components/engine/integration/system/info_test.go index 80058523a3..8130361988 100644 --- a/components/engine/integration/system/info_test.go +++ b/components/engine/integration/system/info_test.go @@ -44,6 +44,7 @@ func TestInfoAPI(t *testing.T) { } func TestInfoAPIWarnings(t *testing.T) { + skip.If(t, testEnv.IsRemoteDaemon, "cannot run daemon when remote daemon") skip.If(t, testEnv.DaemonInfo.OSType == "windows", "FIXME") d := daemon.New(t) c := d.NewClientT(t) diff --git a/components/engine/integration/volume/volume_test.go b/components/engine/integration/volume/volume_test.go index 4ee109e675..3b2babad19 100644 --- a/components/engine/integration/volume/volume_test.go +++ b/components/engine/integration/volume/volume_test.go @@ -26,6 +26,10 @@ func TestVolumesCreateAndList(t *testing.T) { ctx := context.Background() name := t.Name() + // Windows file system is case insensitive + if testEnv.OSType == "windows" { + name = strings.ToLower(name) + } vol, err := client.VolumeCreate(ctx, volumetypes.VolumeCreateBody{ Name: name, }) diff --git a/components/engine/internal/test/daemon/daemon.go b/components/engine/internal/test/daemon/daemon.go index c0b5c483f9..61fbdb4e74 100644 --- a/components/engine/internal/test/daemon/daemon.go +++ b/components/engine/internal/test/daemon/daemon.go @@ -268,8 +268,11 @@ func (d *Daemon) StartWithLogFile(out *os.File, providedArgs ...string) error { wait := make(chan error) go func() { - wait <- d.cmd.Wait() + ret := d.cmd.Wait() d.log.Logf("[%s] exiting daemon", d.id) + // If we send before logging, we might accidentally log _after_ the test is done. + // As of Go 1.12, this incurs a panic instead of silently being dropped. + wait <- ret close(wait) }() diff --git a/components/engine/layer/layer_store.go b/components/engine/layer/layer_store.go index 1601465c04..81730e9d92 100644 --- a/components/engine/layer/layer_store.go +++ b/components/engine/layer/layer_store.go @@ -10,6 +10,7 @@ import ( "github.com/docker/distribution" "github.com/docker/docker/daemon/graphdriver" "github.com/docker/docker/pkg/idtools" + "github.com/docker/docker/pkg/locker" "github.com/docker/docker/pkg/plugingetter" "github.com/docker/docker/pkg/stringid" "github.com/docker/docker/pkg/system" @@ -36,7 +37,11 @@ type layerStore struct { mounts map[string]*mountedLayer mountL sync.Mutex - os string + + // protect *RWLayer() methods from operating on the same name/id + locker *locker.Locker + + os string } // StoreOptions are the options used to create a new Store instance @@ -92,6 +97,7 @@ func newStoreFromGraphDriver(root string, driver graphdriver.Driver, os string) driver: driver, layerMap: map[ChainID]*roLayer{}, mounts: map[string]*mountedLayer{}, + locker: locker.New(), useTarSplit: !caps.ReproducesExactDiffs, os: os, } @@ -189,6 +195,8 @@ func (ls *layerStore) loadLayer(layer ChainID) (*roLayer, error) { } func (ls *layerStore) loadMount(mount string) error { + ls.mountL.Lock() + defer ls.mountL.Unlock() if _, ok := ls.mounts[mount]; ok { return nil } @@ -477,7 +485,7 @@ func (ls *layerStore) Release(l Layer) ([]Metadata, error) { return ls.releaseLayer(layer) } -func (ls *layerStore) CreateRWLayer(name string, parent ChainID, opts *CreateRWLayerOpts) (RWLayer, error) { +func (ls *layerStore) CreateRWLayer(name string, parent ChainID, opts *CreateRWLayerOpts) (_ RWLayer, err error) { var ( storageOpt map[string]string initFunc MountInit @@ -490,14 +498,16 @@ func (ls *layerStore) CreateRWLayer(name string, parent ChainID, opts *CreateRWL initFunc = opts.InitFunc } + ls.locker.Lock(name) + defer ls.locker.Unlock(name) + ls.mountL.Lock() - defer ls.mountL.Unlock() - m, ok := ls.mounts[name] + _, ok := ls.mounts[name] + ls.mountL.Unlock() if ok { return nil, ErrMountNameConflict } - var err error var pid string var p *roLayer if string(parent) != "" { @@ -517,7 +527,7 @@ func (ls *layerStore) CreateRWLayer(name string, parent ChainID, opts *CreateRWL }() } - m = &mountedLayer{ + m := &mountedLayer{ name: name, parent: p, mountID: ls.mountID(name), @@ -528,7 +538,7 @@ func (ls *layerStore) CreateRWLayer(name string, parent ChainID, opts *CreateRWL if initFunc != nil { pid, err = ls.initMount(m.mountID, pid, mountLabel, initFunc, storageOpt) if err != nil { - return nil, err + return } m.initID = pid } @@ -538,20 +548,23 @@ func (ls *layerStore) CreateRWLayer(name string, parent ChainID, opts *CreateRWL } if err = ls.driver.CreateReadWrite(m.mountID, pid, createOpts); err != nil { - return nil, err + return } if err = ls.saveMount(m); err != nil { - return nil, err + return } return m.getReference(), nil } func (ls *layerStore) GetRWLayer(id string) (RWLayer, error) { + ls.locker.Lock(id) + defer ls.locker.Unlock(id) + ls.mountL.Lock() - defer ls.mountL.Unlock() - mount, ok := ls.mounts[id] - if !ok { + mount := ls.mounts[id] + ls.mountL.Unlock() + if mount == nil { return nil, ErrMountDoesNotExist } @@ -560,9 +573,10 @@ func (ls *layerStore) GetRWLayer(id string) (RWLayer, error) { func (ls *layerStore) GetMountID(id string) (string, error) { ls.mountL.Lock() - defer ls.mountL.Unlock() - mount, ok := ls.mounts[id] - if !ok { + mount := ls.mounts[id] + ls.mountL.Unlock() + + if mount == nil { return "", ErrMountDoesNotExist } logrus.Debugf("GetMountID id: %s -> mountID: %s", id, mount.mountID) @@ -571,10 +585,14 @@ func (ls *layerStore) GetMountID(id string) (string, error) { } func (ls *layerStore) ReleaseRWLayer(l RWLayer) ([]Metadata, error) { + name := l.Name() + ls.locker.Lock(name) + defer ls.locker.Unlock(name) + ls.mountL.Lock() - defer ls.mountL.Unlock() - m, ok := ls.mounts[l.Name()] - if !ok { + m := ls.mounts[name] + ls.mountL.Unlock() + if m == nil { return []Metadata{}, nil } @@ -606,7 +624,9 @@ func (ls *layerStore) ReleaseRWLayer(l RWLayer) ([]Metadata, error) { return nil, err } - delete(ls.mounts, m.Name()) + ls.mountL.Lock() + delete(ls.mounts, name) + ls.mountL.Unlock() ls.layerL.Lock() defer ls.layerL.Unlock() @@ -634,7 +654,9 @@ func (ls *layerStore) saveMount(mount *mountedLayer) error { } } + ls.mountL.Lock() ls.mounts[mount.name] = mount + ls.mountL.Unlock() return nil } diff --git a/components/engine/layer/migration.go b/components/engine/layer/migration.go index 2668ea96bb..12500694f0 100644 --- a/components/engine/layer/migration.go +++ b/components/engine/layer/migration.go @@ -3,7 +3,6 @@ package layer // import "github.com/docker/docker/layer" import ( "compress/gzip" "errors" - "fmt" "io" "os" @@ -13,64 +12,6 @@ import ( "github.com/vbatts/tar-split/tar/storage" ) -// CreateRWLayerByGraphID creates a RWLayer in the layer store using -// the provided name with the given graphID. To get the RWLayer -// after migration the layer may be retrieved by the given name. -func (ls *layerStore) CreateRWLayerByGraphID(name, graphID string, parent ChainID) (err error) { - ls.mountL.Lock() - defer ls.mountL.Unlock() - m, ok := ls.mounts[name] - if ok { - if m.parent.chainID != parent { - return errors.New("name conflict, mismatched parent") - } - if m.mountID != graphID { - return errors.New("mount already exists") - } - - return nil - } - - if !ls.driver.Exists(graphID) { - return fmt.Errorf("graph ID does not exist: %q", graphID) - } - - var p *roLayer - if string(parent) != "" { - p = ls.get(parent) - if p == nil { - return ErrLayerDoesNotExist - } - - // Release parent chain if error - defer func() { - if err != nil { - ls.layerL.Lock() - ls.releaseLayer(p) - ls.layerL.Unlock() - } - }() - } - - // TODO: Ensure graphID has correct parent - - m = &mountedLayer{ - name: name, - parent: p, - mountID: graphID, - layerStore: ls, - references: map[RWLayer]*referencedRWLayer{}, - } - - // Check for existing init layer - initID := fmt.Sprintf("%s-init", graphID) - if ls.driver.Exists(initID) { - m.initID = initID - } - - return ls.saveMount(m) -} - func (ls *layerStore) ChecksumForGraphID(id, parent, oldTarDataPath, newTarDataPath string) (diffID DiffID, size int64, err error) { defer func() { if err != nil { diff --git a/components/engine/layer/migration_test.go b/components/engine/layer/migration_test.go index 923166371c..2b5c3301f8 100644 --- a/components/engine/layer/migration_test.go +++ b/components/engine/layer/migration_test.go @@ -3,7 +3,6 @@ package layer // import "github.com/docker/docker/layer" import ( "bytes" "compress/gzip" - "fmt" "io" "io/ioutil" "os" @@ -12,7 +11,6 @@ import ( "testing" "github.com/docker/docker/daemon/graphdriver" - "github.com/docker/docker/pkg/archive" "github.com/docker/docker/pkg/stringid" "github.com/vbatts/tar-split/tar/asm" "github.com/vbatts/tar-split/tar/storage" @@ -269,161 +267,3 @@ func TestLayerMigrationNoTarsplit(t *testing.T) { assertMetadata(t, metadata, createMetadata(layer2a)) } - -func TestMountMigration(t *testing.T) { - // TODO Windows: Figure out why this is failing (obvious - paths... needs porting) - if runtime.GOOS == "windows" { - t.Skip("Failing on Windows") - } - ls, _, cleanup := newTestStore(t) - defer cleanup() - - baseFiles := []FileApplier{ - newTestFile("/root/.bashrc", []byte("# Boring configuration"), 0644), - newTestFile("/etc/profile", []byte("# Base configuration"), 0644), - } - initFiles := []FileApplier{ - newTestFile("/etc/hosts", []byte{}, 0644), - newTestFile("/etc/resolv.conf", []byte{}, 0644), - } - mountFiles := []FileApplier{ - newTestFile("/etc/hosts", []byte("localhost 127.0.0.1"), 0644), - newTestFile("/root/.bashrc", []byte("# Updated configuration"), 0644), - newTestFile("/root/testfile1.txt", []byte("nothing valuable"), 0644), - } - - initTar, err := tarFromFiles(initFiles...) - if err != nil { - t.Fatal(err) - } - - mountTar, err := tarFromFiles(mountFiles...) - if err != nil { - t.Fatal(err) - } - - graph := ls.(*layerStore).driver - - layer1, err := createLayer(ls, "", initWithFiles(baseFiles...)) - if err != nil { - t.Fatal(err) - } - - graphID1 := layer1.(*referencedCacheLayer).cacheID - - containerID := stringid.GenerateRandomID() - containerInit := fmt.Sprintf("%s-init", containerID) - - if err := graph.Create(containerInit, graphID1, nil); err != nil { - t.Fatal(err) - } - if _, err := graph.ApplyDiff(containerInit, graphID1, bytes.NewReader(initTar)); err != nil { - t.Fatal(err) - } - - if err := graph.Create(containerID, containerInit, nil); err != nil { - t.Fatal(err) - } - if _, err := graph.ApplyDiff(containerID, containerInit, bytes.NewReader(mountTar)); err != nil { - t.Fatal(err) - } - - if err := ls.(*layerStore).CreateRWLayerByGraphID("migration-mount", containerID, layer1.ChainID()); err != nil { - t.Fatal(err) - } - - rwLayer1, err := ls.GetRWLayer("migration-mount") - if err != nil { - t.Fatal(err) - } - - if _, err := rwLayer1.Mount(""); err != nil { - t.Fatal(err) - } - - changes, err := rwLayer1.Changes() - if err != nil { - t.Fatal(err) - } - - if expected := 5; len(changes) != expected { - t.Logf("Changes %#v", changes) - t.Fatalf("Wrong number of changes %d, expected %d", len(changes), expected) - } - - sortChanges(changes) - - assertChange(t, changes[0], archive.Change{ - Path: "/etc", - Kind: archive.ChangeModify, - }) - assertChange(t, changes[1], archive.Change{ - Path: "/etc/hosts", - Kind: archive.ChangeModify, - }) - assertChange(t, changes[2], archive.Change{ - Path: "/root", - Kind: archive.ChangeModify, - }) - assertChange(t, changes[3], archive.Change{ - Path: "/root/.bashrc", - Kind: archive.ChangeModify, - }) - assertChange(t, changes[4], archive.Change{ - Path: "/root/testfile1.txt", - Kind: archive.ChangeAdd, - }) - - if _, err := ls.CreateRWLayer("migration-mount", layer1.ChainID(), nil); err == nil { - t.Fatal("Expected error creating mount with same name") - } else if err != ErrMountNameConflict { - t.Fatal(err) - } - - rwLayer2, err := ls.GetRWLayer("migration-mount") - if err != nil { - t.Fatal(err) - } - - if getMountLayer(rwLayer1) != getMountLayer(rwLayer2) { - t.Fatal("Expected same layer from get with same name as from migrate") - } - - if _, err := rwLayer2.Mount(""); err != nil { - t.Fatal(err) - } - - if _, err := rwLayer2.Mount(""); err != nil { - t.Fatal(err) - } - - if metadata, err := ls.Release(layer1); err != nil { - t.Fatal(err) - } else if len(metadata) > 0 { - t.Fatalf("Expected no layers to be deleted, deleted %#v", metadata) - } - - if err := rwLayer1.Unmount(); err != nil { - t.Fatal(err) - } - - if _, err := ls.ReleaseRWLayer(rwLayer1); err != nil { - t.Fatal(err) - } - - if err := rwLayer2.Unmount(); err != nil { - t.Fatal(err) - } - if err := rwLayer2.Unmount(); err != nil { - t.Fatal(err) - } - metadata, err := ls.ReleaseRWLayer(rwLayer2) - if err != nil { - t.Fatal(err) - } - if len(metadata) == 0 { - t.Fatal("Expected base layer to be deleted when deleting mount") - } - - assertMetadata(t, metadata, createMetadata(layer1)) -} diff --git a/components/engine/layer/mounted_layer.go b/components/engine/layer/mounted_layer.go index d6858c662c..c5d9e0e488 100644 --- a/components/engine/layer/mounted_layer.go +++ b/components/engine/layer/mounted_layer.go @@ -2,6 +2,7 @@ package layer // import "github.com/docker/docker/layer" import ( "io" + "sync" "github.com/docker/docker/pkg/archive" "github.com/docker/docker/pkg/containerfs" @@ -15,6 +16,7 @@ type mountedLayer struct { path string layerStore *layerStore + sync.Mutex references map[RWLayer]*referencedRWLayer } @@ -62,16 +64,24 @@ func (ml *mountedLayer) getReference() RWLayer { ref := &referencedRWLayer{ mountedLayer: ml, } + ml.Lock() ml.references[ref] = ref + ml.Unlock() return ref } func (ml *mountedLayer) hasReferences() bool { - return len(ml.references) > 0 + ml.Lock() + ret := len(ml.references) > 0 + ml.Unlock() + + return ret } func (ml *mountedLayer) deleteReference(ref RWLayer) error { + ml.Lock() + defer ml.Unlock() if _, ok := ml.references[ref]; !ok { return ErrLayerNotRetained } @@ -81,7 +91,9 @@ func (ml *mountedLayer) deleteReference(ref RWLayer) error { func (ml *mountedLayer) retakeReference(r RWLayer) { if ref, ok := r.(*referencedRWLayer); ok { + ml.Lock() ml.references[ref] = ref + ml.Unlock() } } diff --git a/components/engine/pkg/mount/mount.go b/components/engine/pkg/mount/mount.go index 874aff6545..4afd63c427 100644 --- a/components/engine/pkg/mount/mount.go +++ b/components/engine/pkg/mount/mount.go @@ -2,12 +2,46 @@ package mount // import "github.com/docker/docker/pkg/mount" import ( "sort" + "strconv" "strings" - "syscall" "github.com/sirupsen/logrus" ) +// mountError records an error from mount or unmount operation +type mountError struct { + op string + source, target string + flags uintptr + data string + err error +} + +func (e *mountError) Error() string { + out := e.op + " " + + if e.source != "" { + out += e.source + ":" + e.target + } else { + out += e.target + } + + if e.flags != uintptr(0) { + out += ", flags: 0x" + strconv.FormatUint(uint64(e.flags), 16) + } + if e.data != "" { + out += ", data: " + e.data + } + + out += ": " + e.err.Error() + return out +} + +// Cause returns the underlying cause of the error +func (e *mountError) Cause() error { + return e.err +} + // FilterFunc is a type defining a callback function // to filter out unwanted entries. It takes a pointer // to an Info struct (not fully populated, currently @@ -89,12 +123,7 @@ func ForceMount(device, target, mType, options string) error { // Unmount lazily unmounts a filesystem on supported platforms, otherwise // does a normal unmount. func Unmount(target string) error { - err := unmount(target, mntDetach) - if err == syscall.EINVAL { - // ignore "not mounted" error - err = nil - } - return err + return unmount(target, mntDetach) } // RecursiveUnmount unmounts the target and all mounts underneath, starting with @@ -114,25 +143,14 @@ func RecursiveUnmount(target string) error { logrus.Debugf("Trying to unmount %s", m.Mountpoint) err = unmount(m.Mountpoint, mntDetach) if err != nil { - // If the error is EINVAL either this whole package is wrong (invalid flags passed to unmount(2)) or this is - // not a mountpoint (which is ok in this case). - // Meanwhile calling `Mounted()` is very expensive. - // - // We've purposefully used `syscall.EINVAL` here instead of `unix.EINVAL` to avoid platform branching - // Since `EINVAL` is defined for both Windows and Linux in the `syscall` package (and other platforms), - // this is nicer than defining a custom value that we can refer to in each platform file. - if err == syscall.EINVAL { - continue - } - if i == len(mounts)-1 { + if i == len(mounts)-1 { // last mount if mounted, e := Mounted(m.Mountpoint); e != nil || mounted { return err } - continue + } else { + // This is some submount, we can ignore this error for now, the final unmount will fail if this is a real problem + logrus.WithError(err).Warnf("Failed to unmount submount %s", m.Mountpoint) } - // This is some submount, we can ignore this error for now, the final unmount will fail if this is a real problem - logrus.WithError(err).Warnf("Failed to unmount submount %s", m.Mountpoint) - continue } logrus.Debugf("Unmounted %s", m.Mountpoint) diff --git a/components/engine/pkg/mount/mounter_freebsd.go b/components/engine/pkg/mount/mounter_freebsd.go index b6ab83a230..09ad360608 100644 --- a/components/engine/pkg/mount/mounter_freebsd.go +++ b/components/engine/pkg/mount/mounter_freebsd.go @@ -11,11 +11,9 @@ package mount // import "github.com/docker/docker/pkg/mount" import "C" import ( - "fmt" "strings" + "syscall" "unsafe" - - "golang.org/x/sys/unix" ) func allocateIOVecs(options []string) []C.struct_iovec { @@ -49,12 +47,13 @@ func mount(device, target, mType string, flag uintptr, data string) error { } if errno := C.nmount(&rawOptions[0], C.uint(len(options)), C.int(flag)); errno != 0 { - reason := C.GoString(C.strerror(*C.__error())) - return fmt.Errorf("Failed to call nmount: %s", reason) + return &mountError{ + op: "mount", + source: device, + target: target, + flags: flag, + err: syscall.Errno(errno), + } } return nil } - -func unmount(target string, flag int) error { - return unix.Unmount(target, flag) -} diff --git a/components/engine/pkg/mount/mounter_linux.go b/components/engine/pkg/mount/mounter_linux.go index 631daf10a5..48837adde5 100644 --- a/components/engine/pkg/mount/mounter_linux.go +++ b/components/engine/pkg/mount/mounter_linux.go @@ -33,25 +33,42 @@ func mount(device, target, mType string, flags uintptr, data string) error { // Initial call applying all non-propagation flags for mount // or remount with changed data if err := unix.Mount(device, target, mType, oflags, data); err != nil { - return err + return &mountError{ + op: "mount", + source: device, + target: target, + flags: oflags, + data: data, + err: err, + } } } if flags&ptypes != 0 { // Change the propagation type. if err := unix.Mount("", target, "", flags&pflags, ""); err != nil { + return &mountError{ + op: "remount", + target: target, + flags: flags & pflags, + err: err, + } return err } } if oflags&broflags == broflags { // Remount the bind to apply read only. - return unix.Mount("", target, "", oflags|unix.MS_REMOUNT, "") + if err := unix.Mount("", target, "", oflags|unix.MS_REMOUNT, ""); err != nil { + return &mountError{ + op: "remount-ro", + target: target, + flags: oflags | unix.MS_REMOUNT, + err: err, + } + + } } return nil } - -func unmount(target string, flag int) error { - return unix.Unmount(target, flag) -} diff --git a/components/engine/pkg/mount/sharedsubtree_linux_test.go b/components/engine/pkg/mount/sharedsubtree_linux_test.go index 019514491f..7a37f66098 100644 --- a/components/engine/pkg/mount/sharedsubtree_linux_test.go +++ b/components/engine/pkg/mount/sharedsubtree_linux_test.go @@ -7,6 +7,7 @@ import ( "path" "testing" + "github.com/pkg/errors" "golang.org/x/sys/unix" ) @@ -326,7 +327,7 @@ func TestSubtreeUnbindable(t *testing.T) { }() // then attempt to mount it to target. It should fail - if err := Mount(sourceDir, targetDir, "none", "bind,rw"); err != nil && err != unix.EINVAL { + if err := Mount(sourceDir, targetDir, "none", "bind,rw"); err != nil && errors.Cause(err) != unix.EINVAL { t.Fatal(err) } else if err == nil { t.Fatalf("%q should not have been bindable", sourceDir) diff --git a/components/engine/pkg/mount/unmount_unix.go b/components/engine/pkg/mount/unmount_unix.go new file mode 100644 index 0000000000..4be4276851 --- /dev/null +++ b/components/engine/pkg/mount/unmount_unix.go @@ -0,0 +1,22 @@ +// +build !windows + +package mount // import "github.com/docker/docker/pkg/mount" + +import "golang.org/x/sys/unix" + +func unmount(target string, flags int) error { + err := unix.Unmount(target, flags) + if err == nil || err == unix.EINVAL { + // Ignore "not mounted" error here. Note the same error + // can be returned if flags are invalid, so this code + // assumes that the flags value is always correct. + return nil + } + + return &mountError{ + op: "umount", + target: target, + flags: uintptr(flags), + err: err, + } +} diff --git a/components/engine/plugin/manager_linux.go b/components/engine/plugin/manager_linux.go index df1fe5b73d..86ada8d02f 100644 --- a/components/engine/plugin/manager_linux.go +++ b/components/engine/plugin/manager_linux.go @@ -61,7 +61,7 @@ func (pm *Manager) enable(p *v2.Plugin, c *controller, force bool) error { if err := pm.executor.Create(p.GetID(), *spec, stdout, stderr); err != nil { if p.PluginObj.Config.PropagatedMount != "" { if err := mount.Unmount(propRoot); err != nil { - logrus.Warnf("Could not unmount %s: %v", propRoot, err) + logrus.WithField("plugin", p.Name()).WithError(err).Warn("Failed to unmount vplugin propagated mount root") } } return errors.WithStack(err) diff --git a/components/engine/volume/local/local.go b/components/engine/volume/local/local.go index 7190de9ed6..d3119cb2ff 100644 --- a/components/engine/volume/local/local.go +++ b/components/engine/volume/local/local.go @@ -344,7 +344,7 @@ func (v *localVolume) unmount() error { if v.opts != nil { if err := mount.Unmount(v.path); err != nil { if mounted, mErr := mount.Mounted(v.path); mounted || mErr != nil { - return errdefs.System(errors.Wrapf(err, "error while unmounting volume path '%s'", v.path)) + return errdefs.System(err) } } v.active.mounted = false diff --git a/components/engine/volume/local/local_unix.go b/components/engine/volume/local/local_unix.go index b1c68b931b..5ee2ed894b 100644 --- a/components/engine/volume/local/local_unix.go +++ b/components/engine/volume/local/local_unix.go @@ -86,7 +86,7 @@ func (v *localVolume) mount() error { } } err := mount.Mount(v.opts.MountDevice, v.path, v.opts.MountType, mountOpts) - return errors.Wrapf(err, "error while mounting volume with options: %s", v.opts) + return errors.Wrap(err, "failed to mount local volume") } func (v *localVolume) CreatedAt() (time.Time, error) {