Move pre-start hooks after container mounts

Today mounts in pre-start hooks get overriden by the default mounts. Moving the pre-start hooks to after the container mounts and before the pivot/move root gives better flexiblity in the hooks. Signed-off-by: Mrunal Patel <[email protected]>
kinvolk · Feb 23, 2016 · 2f27649 · 2f27649
1 parent 2c489ce
commit 2f27649
Show file tree

Hide file tree

Showing 5 changed files with 71 additions and 15 deletions.
diff --git a/libcontainer/generic_error.go b/libcontainer/generic_error.go
@@ -15,6 +15,8 @@ const (
 	procReady syncType = iota
 	procError
 	procRun
+	procHooks
+	procResume
 )
 
 type syncT struct {

diff --git a/libcontainer/init_linux.go b/libcontainer/init_linux.go
@@ -163,6 +163,27 @@ func syncParentReady(pipe io.ReadWriter) error {
 	return nil
 }
 
+// syncParentHooks sends to the given pipe a JSON payload which indicates that
+// the parent should execute pre-start hooks. It then waits for the parent to
+// indicate that it is cleared to resume.
+func syncParentHooks(pipe io.ReadWriter) error {
+	// Tell parent.
+	if err := utils.WriteJSON(pipe, syncT{procHooks}); err != nil {
+		return err
+	}
+	// Wait for parent to give the all-clear.
+	var procSync syncT
+	if err := json.NewDecoder(pipe).Decode(&procSync); err != nil {
+		if err == io.EOF {
+			return fmt.Errorf("parent closed synchronisation channel")
+		}
+		if procSync.Type != procResume {
+			return fmt.Errorf("invalid synchronisation flag from parent")
+		}
+	}
+	return nil
+}
+
 // joinExistingNamespaces gets all the namespace paths specified for the container and
 // does a setns on the namespace fd so that the current process joins the namespace.
 func joinExistingNamespaces(namespaces []configs.Namespace) error {

diff --git a/libcontainer/process_linux.go b/libcontainer/process_linux.go
@@ -213,16 +213,18 @@ func (p *initProcess) start() (err error) {
 			p.manager.Destroy()
 		}
 	}()
-	if p.config.Config.Hooks != nil {
-		s := configs.HookState{
-			Version: p.container.config.Version,
-			ID:      p.container.id,
-			Pid:     p.pid(),
-			Root:    p.config.Config.Rootfs,
-		}
-		for _, hook := range p.config.Config.Hooks.Prestart {
-			if err := hook.Run(s); err != nil {
-				return newSystemError(err)
+	if !p.config.Config.Namespaces.Contains(configs.NEWNS) {
+		if p.config.Config.Hooks != nil {
+			s := configs.HookState{
+				Version: p.container.config.Version,
+				ID:      p.container.id,
+				Pid:     p.pid(),
+				Root:    p.config.Config.Rootfs,
+			}
+			for _, hook := range p.config.Config.Hooks.Prestart {
+				if err := hook.Run(s); err != nil {
+					return newSystemError(err)
+				}
 			}
 		}
 	}
@@ -233,9 +235,10 @@ func (p *initProcess) start() (err error) {
 		return newSystemError(err)
 	}
 	var (
-		procSync syncT
-		sentRun  bool
-		ierr     *genericError
+		procSync   syncT
+		sentRun    bool
+		sentResume bool
+		ierr       *genericError
 	)
 
 loop:
@@ -256,6 +259,25 @@ loop:
 				return newSystemError(err)
 			}
 			sentRun = true
+		case procHooks:
+			if p.config.Config.Hooks != nil {
+				s := configs.HookState{
+					Version: p.container.config.Version,
+					ID:      p.container.id,
+					Pid:     p.pid(),
+					Root:    p.config.Config.Rootfs,
+				}
+				for _, hook := range p.config.Config.Hooks.Prestart {
+					if err := hook.Run(s); err != nil {
+						return newSystemError(err)
+					}
+				}
+			}
+			// Sync with child.
+			if err := utils.WriteJSON(p.parentPipe, syncT{procResume}); err != nil {
+				return newSystemError(err)
+			}
+			sentResume = true
 		case procError:
 			// wait for the child process to fully complete and receive an error message
 			// if one was encoutered
@@ -274,6 +296,9 @@ loop:
 	if !sentRun {
 		return newSystemError(fmt.Errorf("could not synchronise with container process"))
 	}
+	if p.config.Config.Namespaces.Contains(configs.NEWNS) && !sentResume {
+		return newSystemError(fmt.Errorf("could not synchronise after executing prestart hooks with container process"))
+	}
 	if err := syscall.Shutdown(int(p.parentPipe.Fd()), syscall.SHUT_WR); err != nil {
 		return newSystemError(err)
 	}

diff --git a/libcontainer/rootfs_linux.go b/libcontainer/rootfs_linux.go
@@ -4,6 +4,7 @@ package libcontainer
 
 import (
 	"fmt"
+	"io"
 	"io/ioutil"
 	"os"
 	"os/exec"
@@ -26,7 +27,7 @@ const defaultMountFlags = syscall.MS_NOEXEC | syscall.MS_NOSUID | syscall.MS_NOD
 
 // setupRootfs sets up the devices, mount points, and filesystems for use inside a
 // new mount namespace.
-func setupRootfs(config *configs.Config, console *linuxConsole) (err error) {
+func setupRootfs(config *configs.Config, console *linuxConsole, pipe io.ReadWriter) (err error) {
 	if err := prepareRoot(config); err != nil {
 		return newSystemError(err)
 	}
@@ -59,6 +60,13 @@ func setupRootfs(config *configs.Config, console *linuxConsole) (err error) {
 			return newSystemError(err)
 		}
 	}
+	// Signal the parent to run the pre-start hooks.
+	// The hooks are run after the mounts are setup, but before we switch to the new
+	// root, so that the old root is still available in the hooks for any mount
+	// manipulations.
+	if err := syncParentHooks(pipe); err != nil {
+		return err
+	}
 	if err := syscall.Chdir(config.Rootfs); err != nil {
 		return newSystemError(err)
 	}

diff --git a/libcontainer/standard_init_linux.go b/libcontainer/standard_init_linux.go
@@ -72,7 +72,7 @@ func (l *linuxStandardInit) Init() error {
 	label.Init()
 	// InitializeMountNamespace() can be executed only for a new mount namespace
 	if l.config.Config.Namespaces.Contains(configs.NEWNS) {
-		if err := setupRootfs(l.config.Config, console); err != nil {
+		if err := setupRootfs(l.config.Config, console, l.pipe); err != nil {
 			return err
 		}
 	}