Decouple daemon and container to mount and unmount filesystems.

Side effects: - Decouple daemon and container to start containers. - Decouple daemon and container to copy files. Signed-off-by: David Calavera <[email protected]>
jawher · Nov 4, 2015 · 3a49765 · 3a49765
1 parent 1c94f5f
commit 3a49765
Show file tree

Hide file tree

Showing 17 changed files with 188 additions and 176 deletions.
diff --git a/builder/builder.go b/builder/builder.go
@@ -130,6 +130,12 @@ type Docker interface {
 	Release(sessionID string, activeImages []string)
 	// Kill stops the container execution abruptly.
 	Kill(c *daemon.Container) error
+	// Mount mounts the root filesystem for the container.
+	Mount(c *daemon.Container) error
+	// Unmount unmounts the root filesystem for the container.
+	Unmount(c *daemon.Container) error
+	// Start starts a new container
+	Start(c *daemon.Container) error
 }
 
 // ImageCache abstracts an image cache store.

diff --git a/builder/dockerfile/dispatchers.go b/builder/dockerfile/dispatchers.go
@@ -399,8 +399,8 @@ func run(b *Builder, args []string, attributes map[string]bool, original string)
 
 	// Ensure that we keep the container mounted until the commit
 	// to avoid unmounting and then mounting directly again
-	c.Mount()
-	defer c.Unmount()
+	b.docker.Mount(c)
+	defer b.docker.Unmount(c)
 
 	err = b.run(c)
 	if err != nil {

diff --git a/builder/dockerfile/internals.go b/builder/dockerfile/internals.go
@@ -67,10 +67,10 @@ func (b *Builder) commit(id string, autoCmd *stringutils.StrSlice, comment strin
 		}
 		id = container.ID
 
-		if err := container.Mount(); err != nil {
+		if err := b.docker.Mount(container); err != nil {
 			return err
 		}
-		defer container.Unmount()
+		defer b.docker.Unmount(container)
 	}
 
 	container, err := b.docker.Container(id)
@@ -201,7 +201,7 @@ func (b *Builder) runContextCommand(args []string, allowRemote bool, allowLocalD
 	if err != nil {
 		return err
 	}
-	defer container.Unmount()
+	defer b.docker.Unmount(container)
 	b.tmpContainers[container.ID] = struct{}{}
 
 	comment := fmt.Sprintf("%s %s in %s", cmdName, origPaths, dest)
@@ -524,7 +524,7 @@ func (b *Builder) create() (*daemon.Container, error) {
 	if err != nil {
 		return nil, err
 	}
-	defer c.Unmount()
+	defer b.docker.Unmount(c)
 	for _, warning := range warnings {
 		fmt.Fprintf(b.Stdout, " ---> [Warning] %s\n", warning)
 	}
@@ -549,7 +549,7 @@ func (b *Builder) run(c *daemon.Container) error {
 	}
 
 	//start the container
-	if err := c.Start(); err != nil {
+	if err := b.docker.Start(c); err != nil {
 		return err
 	}
 

diff --git a/daemon/archive.go b/daemon/archive.go
@@ -30,7 +30,7 @@ func (daemon *Daemon) ContainerCopy(name string, res string) (io.ReadCloser, err
 		res = res[1:]
 	}
 
-	return container.copy(res)
+	return daemon.containerCopy(container, res)
 }
 
 // ContainerStatPath stats the filesystem resource at the specified path in the
@@ -41,7 +41,7 @@ func (daemon *Daemon) ContainerStatPath(name string, path string) (stat *types.C
 		return nil, err
 	}
 
-	return container.StatPath(path)
+	return daemon.containerStatPath(container, path)
 }
 
 // ContainerArchivePath creates an archive of the filesystem resource at the
@@ -53,7 +53,7 @@ func (daemon *Daemon) ContainerArchivePath(name string, path string) (content io
 		return nil, nil, err
 	}
 
-	return container.ArchivePath(path)
+	return daemon.containerArchivePath(container, path)
 }
 
 // ContainerExtractToDir extracts the given archive to the specified location
@@ -68,7 +68,7 @@ func (daemon *Daemon) ContainerExtractToDir(name, path string, noOverwriteDirNon
 		return err
 	}
 
-	return container.ExtractToDir(path, noOverwriteDirNonDir, content)
+	return daemon.containerExtractToDir(container, path, noOverwriteDirNonDir, content)
 }
 
 // resolvePath resolves the given path in the container to a resource on the
@@ -131,16 +131,16 @@ func (container *Container) statPath(resolvedPath, absPath string) (stat *types.
 	}, nil
 }
 
-// StatPath stats the filesystem resource at the specified path in this
+// containerStatPath stats the filesystem resource at the specified path in this
 // container. Returns stat info about the resource.
-func (container *Container) StatPath(path string) (stat *types.ContainerPathStat, err error) {
+func (daemon *Daemon) containerStatPath(container *Container, path string) (stat *types.ContainerPathStat, err error) {
 	container.Lock()
 	defer container.Unlock()
 
-	if err = container.Mount(); err != nil {
+	if err = daemon.Mount(container); err != nil {
 		return nil, err
 	}
-	defer container.Unmount()
+	defer daemon.Unmount(container)
 
 	err = container.mountVolumes()
 	defer container.unmountVolumes(true)
@@ -156,10 +156,10 @@ func (container *Container) StatPath(path string) (stat *types.ContainerPathStat
 	return container.statPath(resolvedPath, absPath)
 }
 
-// ArchivePath creates an archive of the filesystem resource at the specified
+// containerArchivePath creates an archive of the filesystem resource at the specified
 // path in this container. Returns a tar archive of the resource and stat info
 // about the resource.
-func (container *Container) ArchivePath(path string) (content io.ReadCloser, stat *types.ContainerPathStat, err error) {
+func (daemon *Daemon) containerArchivePath(container *Container, path string) (content io.ReadCloser, stat *types.ContainerPathStat, err error) {
 	container.Lock()
 
 	defer func() {
@@ -171,7 +171,7 @@ func (container *Container) ArchivePath(path string) (content io.ReadCloser, sta
 		}
 	}()
 
-	if err = container.Mount(); err != nil {
+	if err = daemon.Mount(container); err != nil {
 		return nil, nil, err
 	}
 
@@ -180,7 +180,7 @@ func (container *Container) ArchivePath(path string) (content io.ReadCloser, sta
 			// unmount any volumes
 			container.unmountVolumes(true)
 			// unmount the container's rootfs
-			container.Unmount()
+			daemon.Unmount(container)
 		}
 	}()
 
@@ -214,7 +214,7 @@ func (container *Container) ArchivePath(path string) (content io.ReadCloser, sta
 	content = ioutils.NewReadCloserWrapper(data, func() error {
 		err := data.Close()
 		container.unmountVolumes(true)
-		container.Unmount()
+		daemon.Unmount(container)
 		container.Unlock()
 		return err
 	})
@@ -224,20 +224,20 @@ func (container *Container) ArchivePath(path string) (content io.ReadCloser, sta
 	return content, stat, nil
 }
 
-// ExtractToDir extracts the given tar archive to the specified location in the
+// containerExtractToDir extracts the given tar archive to the specified location in the
 // filesystem of this container. The given path must be of a directory in the
 // container. If it is not, the error will be ErrExtractPointNotDirectory. If
 // noOverwriteDirNonDir is 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.
-func (container *Container) ExtractToDir(path string, noOverwriteDirNonDir bool, content io.Reader) (err error) {
+func (daemon *Daemon) containerExtractToDir(container *Container, path string, noOverwriteDirNonDir bool, content io.Reader) (err error) {
 	container.Lock()
 	defer container.Unlock()
 
-	if err = container.Mount(); err != nil {
+	if err = daemon.Mount(container); err != nil {
 		return err
 	}
-	defer container.Unmount()
+	defer daemon.Unmount(container)
 
 	err = container.mountVolumes()
 	defer container.unmountVolumes(true)

diff --git a/daemon/container.go b/daemon/container.go
@@ -225,76 +225,6 @@ func (container *Container) getRootResourcePath(path string) (string, error) {
 	return symlink.FollowSymlinkInScope(filepath.Join(container.root, cleanPath), container.root)
 }
 
-// Start prepares the container to run by setting up everything the
-// container needs, such as storage and networking, as well as links
-// between containers. The container is left waiting for a signal to
-// begin running.
-func (container *Container) Start() (err error) {
-	container.Lock()
-	defer container.Unlock()
-
-	if container.Running {
-		return nil
-	}
-
-	if container.removalInProgress || container.Dead {
-		return derr.ErrorCodeContainerBeingRemoved
-	}
-
-	// if we encounter an error during start we need to ensure that any other
-	// setup has been cleaned up properly
-	defer func() {
-		if err != nil {
-			container.setError(err)
-			// if no one else has set it, make sure we don't leave it at zero
-			if container.ExitCode == 0 {
-				container.ExitCode = 128
-			}
-			container.toDisk()
-			container.cleanup()
-			container.logEvent("die")
-		}
-	}()
-
-	if err := container.conditionalMountOnStart(); err != nil {
-		return err
-	}
-
-	// Make sure NetworkMode has an acceptable value. We do this to ensure
-	// backwards API compatibility.
-	container.hostConfig = runconfig.SetDefaultNetModeIfBlank(container.hostConfig)
-
-	if err := container.initializeNetworking(); err != nil {
-		return err
-	}
-	linkedEnv, err := container.setupLinkedContainers()
-	if err != nil {
-		return err
-	}
-	if err := container.setupWorkingDirectory(); err != nil {
-		return err
-	}
-	env := container.createDaemonEnvironment(linkedEnv)
-	if err := populateCommand(container, env); err != nil {
-		return err
-	}
-
-	if !container.hostConfig.IpcMode.IsContainer() && !container.hostConfig.IpcMode.IsHost() {
-		if err := container.setupIpcDirs(); err != nil {
-			return err
-		}
-	}
-
-	mounts, err := container.setupMounts()
-	if err != nil {
-		return err
-	}
-	mounts = append(mounts, container.ipcMounts()...)
-
-	container.command.Mounts = mounts
-	return container.waitForStart()
-}
-
 // streamConfig.StdinPipe returns a WriteCloser which can be used to feed data
 // to the standard input of the container's active process.
 // Container.StdoutPipe and Container.StderrPipe each return a ReadCloser
@@ -326,7 +256,7 @@ func (container *Container) cleanup() {
 
 	container.unmountIpcMounts(detachMounted)
 
-	container.conditionalUnmountOnCleanup()
+	container.daemon.conditionalUnmountOnCleanup(container)
 
 	for _, eConfig := range container.execCommands.s {
 		container.daemon.unregisterExecCommand(eConfig)
@@ -356,11 +286,6 @@ func (container *Container) Resize(h, w int) error {
 	return nil
 }
 
-// Mount sets container.basefs
-func (container *Container) Mount() error {
-	return container.daemon.Mount(container)
-}
-
 func (container *Container) changes() ([]archive.Change, error) {
 	container.Lock()
 	defer container.Unlock()
@@ -374,12 +299,6 @@ func (container *Container) getImage() (*image.Image, error) {
 	return container.daemon.graph.Get(container.ImageID)
 }
 
-// Unmount asks the daemon to release the layered filesystems that are
-// mounted by the container.
-func (container *Container) Unmount() error {
-	return container.daemon.unmount(container)
-}
-
 func (container *Container) hostConfigPath() (string, error) {
 	return container.getRootResourcePath("hostconfig.json")
 }
@@ -401,7 +320,7 @@ func validateID(id string) error {
 	return nil
 }
 
-func (container *Container) copy(resource string) (rc io.ReadCloser, err error) {
+func (daemon *Daemon) containerCopy(container *Container, resource string) (rc io.ReadCloser, err error) {
 	container.Lock()
 
 	defer func() {
@@ -413,7 +332,7 @@ func (container *Container) copy(resource string) (rc io.ReadCloser, err error)
 		}
 	}()
 
-	if err := container.Mount(); err != nil {
+	if err := daemon.Mount(container); err != nil {
 		return nil, err
 	}
 
@@ -422,7 +341,7 @@ func (container *Container) copy(resource string) (rc io.ReadCloser, err error)
 			// unmount any volumes
 			container.unmountVolumes(true)
 			// unmount the container's rootfs
-			container.Unmount()
+			daemon.Unmount(container)
 		}
 	}()
 
@@ -458,11 +377,11 @@ func (container *Container) copy(resource string) (rc io.ReadCloser, err error)
 	reader := ioutils.NewReadCloserWrapper(archive, func() error {
 		err := archive.Close()
 		container.unmountVolumes(true)
-		container.Unmount()
+		daemon.Unmount(container)
 		container.Unlock()
 		return err
 	})
-	container.logEvent("copy")
+	daemon.logContainerEvent(container, "copy")
 	return reader, nil
 }
 

diff --git a/daemon/container_unix.go b/daemon/container_unix.go
@@ -379,24 +379,23 @@ func mergeDevices(defaultDevices, userDevices []*configs.Device) []*configs.Devi
 	return append(devs, userDevices...)
 }
 
-// GetSize returns the real size & virtual size of the container.
-func (container *Container) getSize() (int64, int64) {
+// getSize returns the real size & virtual size of the container.
+func (daemon *Daemon) getSize(container *Container) (int64, int64) {
 	var (
 		sizeRw, sizeRootfs int64
 		err                error
-		driver             = container.daemon.driver
 	)
 
-	if err := container.Mount(); err != nil {
+	if err := daemon.Mount(container); err != nil {
 		logrus.Errorf("Failed to compute size of container rootfs %s: %s", container.ID, err)
 		return sizeRw, sizeRootfs
 	}
-	defer container.Unmount()
+	defer daemon.Unmount(container)
 
 	initID := fmt.Sprintf("%s-init", container.ID)
-	sizeRw, err = driver.DiffSize(container.ID, initID)
+	sizeRw, err = daemon.driver.DiffSize(container.ID, initID)
 	if err != nil {
-		logrus.Errorf("Driver %s couldn't return diff size of container %s: %s", driver, container.ID, err)
+		logrus.Errorf("Driver %s couldn't return diff size of container %s: %s", daemon.driver, container.ID, err)
 		// FIXME: GetSize should return an error. Not changing it now in case
 		// there is a side-effect.
 		sizeRw = -1
@@ -1444,20 +1443,3 @@ func (container *Container) ipcMounts() []execdriver.Mount {
 func detachMounted(path string) error {
 	return syscall.Unmount(path, syscall.MNT_DETACH)
 }
-
-// conditionalMountOnStart is a platform specific helper function during the
-// container start to call mount.
-func (container *Container) conditionalMountOnStart() error {
-	if err := container.Mount(); err != nil {
-		return err
-	}
-	return nil
-}
-
-// conditionalUnmountOnCleanup is a platform specific helper function called
-// during the cleanup of a container to unmount.
-func (container *Container) conditionalUnmountOnCleanup() {
-	if err := container.Unmount(); err != nil {
-		logrus.Errorf("%v: Failed to umount filesystem: %v", container.ID, err)
-	}
-}