Cleanup and address review comments.

Signed-off-by: Mrunal Patel <mrunalp@gmail.com> (github: mrunalp)

console/console.go

@@ -13,15 +13,15 @@ import (
 )
 
 // Setup initializes the proper /dev/console inside the rootfs path
-func Setup(rootfs, consolePath, mountLabel string, hostRootUid int) error {
+func Setup(rootfs, consolePath, mountLabel string, hostRootUid, hostRootGid int) error {
 	oldMask := syscall.Umask(0000)
 	defer syscall.Umask(oldMask)
 
 	if err := os.Chmod(consolePath, 0600); err != nil {
 		return err
 	}
 
-	if err := os.Chown(consolePath, hostRootUid, hostRootUid); err != nil {
+	if err := os.Chown(consolePath, hostRootUid, hostRootGid); err != nil {
 		return err
 	}
 

mount/init.go

@@ -25,7 +25,7 @@ type mount struct {
 
 // InitializeMountNamespace sets up the devices, mount points, and filesystems for use inside a
 // new mount namespace.
-func InitializeMountNamespace(rootfs, console string, sysReadonly bool, hostRootUid int, mountConfig *MountConfig) error {
+func InitializeMountNamespace(rootfs, console string, sysReadonly bool, hostRootUid, hostRootGid int, mountConfig *MountConfig) error {
 	var (
 		err  error
 		flag = syscall.MS_PRIVATE
@@ -58,7 +58,7 @@ func InitializeMountNamespace(rootfs, console string, sysReadonly bool, hostRoot
 		return fmt.Errorf("create device nodes %s", err)
 	}
 
-	if err := SetupPtmx(rootfs, console, mountConfig.MountLabel, hostRootUid); err != nil {
+	if err := SetupPtmx(rootfs, console, mountConfig.MountLabel, hostRootUid, hostRootGid); err != nil {
 		return err
 	}
 

mount/ptmx.go

@@ -10,7 +10,7 @@ import (
 	"github.com/docker/libcontainer/console"
 )
 
-func SetupPtmx(rootfs, consolePath, mountLabel string, hostRootUid int) error {
+func SetupPtmx(rootfs, consolePath, mountLabel string, hostRootUid, hostRootGid int) error {
 	ptmx := filepath.Join(rootfs, "dev/ptmx")
 	if err := os.Remove(ptmx); err != nil && !os.IsNotExist(err) {
 		return err
@@ -21,7 +21,7 @@ func SetupPtmx(rootfs, consolePath, mountLabel string, hostRootUid int) error {
 	}
 
 	if consolePath != "" {
-		if err := console.Setup(rootfs, consolePath, mountLabel, hostRootUid); err != nil {
+		if err := console.Setup(rootfs, consolePath, mountLabel, hostRootUid, hostRootGid); err != nil {
 			return err
 		}
 	}

namespaces/exec.go

@@ -6,7 +6,6 @@ import (
 	"encoding/json"
 	"fmt"
 	"io"
-	"io/ioutil"
 	"log"
 	"os"
 	"os/exec"
@@ -31,11 +30,6 @@ const (
 func Exec(container *libcontainer.Config, stdin io.Reader, stdout, stderr io.Writer, console, dataPath string, args []string, createCommand CreateCommand, setupCommand SetupCommand, startCallback func()) (int, error) {
 	var err error
 
-	hostRootUid, err := GetHostRootUid(container)
-	if err != nil {
-		return -1, err
-	}
-
 	// create a pipe so that we can syncronize with the namespaced process and
 	// pass the state and configuration to the child process
 	parent, child, err := newInitPipe()
@@ -52,7 +46,6 @@ func Exec(container *libcontainer.Config, stdin io.Reader, stdout, stderr io.Wri
 	command.Stdout = stdout
 	command.Stderr = stderr
 
-	log.Println("Host Root Uid: ", hostRootUid)
 	log.Println("Starting command")
 	if err := command.Start(); err != nil {
 		child.Close()
@@ -104,24 +97,14 @@ func Exec(container *libcontainer.Config, stdin io.Reader, stdout, stderr io.Wri
 	if container.Namespaces.Contains(libcontainer.NEWUSER) {
 		log.Println("Starting setup")
 		setupCmd := setupCommand(container, console, dataPath, os.Args[0])
-		setupOut, _ := setupCmd.StderrPipe()
-		err = setupCmd.Start()
+		output, err := setupCmd.CombinedOutput()
 		if err != nil {
 			command.Process.Kill()
 			command.Wait()
 			log.Println("setup failed: %v", err)
 			return -1, err
 		}
-		out, _ := ioutil.ReadAll(setupOut)
-		log.Println("Setup output: ", string(out))
-
-		if err := setupCmd.Wait(); err != nil {
-			if _, ok := err.(*exec.ExitError); !ok {
-				command.Process.Kill()
-				command.Wait()
-				return -1, err
-			}
-		}
+		log.Println("Setup output:", output)
 		log.Println("Setup return code", setupCmd.ProcessState.Sys().(syscall.WaitStatus).ExitStatus())
 	}
 
@@ -208,23 +191,40 @@ func hostIDFromMapping(containerID int, uMap []libcontainer.IDMap) (int, bool) {
 	return -1, false
 }
 
+// Gets the root uid for the process on host which could be non-zero
+// when user namespaces are enabled.
+func GetHostRootGid(container *libcontainer.Config) (int, error) {
+	if container.Namespaces.Contains(libcontainer.NEWUSER) {
+		if container.GidMappings == nil {
+			return -1, fmt.Errorf("User namespaces enabled, but no gid mappings found.")
+		}
+		hostRootGid, found := hostIDFromMapping(0, container.GidMappings)
+		if !found {
+			return -1, fmt.Errorf("User namespaces enabled, but no root user mapping found.")
+		}
+		return hostRootGid, nil
+	}
+
+	// Return default root uid 0
+	return 0, nil
+}
+
 // Gets the root uid for the process on host which could be non-zero
 // when user namespaces are enabled.
 func GetHostRootUid(container *libcontainer.Config) (int, error) {
-	hostRootUid := 0
 	if container.Namespaces.Contains(libcontainer.NEWUSER) {
 		if container.UidMappings == nil {
 			return -1, fmt.Errorf("User namespaces enabled, but no user mappings found.")
 		}
 		hostRootUid, found := hostIDFromMapping(0, container.UidMappings)
 		if !found {
 			return -1, fmt.Errorf("User namespaces enabled, but no root user mapping found.")
-		} else {
-			return hostRootUid, nil
 		}
+		return hostRootUid, nil
 	}
 
-	return hostRootUid, nil
+	// Return default root uid 0
+	return 0, nil
 }
 
 // Converts IDMap to SysProcIDMap array and adds it to SysProcAttr.
@@ -279,9 +279,7 @@ func DefaultCreateCommand(container *libcontainer.Config, console, dataPath, ini
 	command.ExtraFiles = []*os.File{pipe}
 
 	if container.Namespaces.Contains(libcontainer.NEWUSER) {
-		if container.UidMappings != nil || container.GidMappings != nil {
-			AddUidGidMappings(command.SysProcAttr, container)
-		}
+		AddUidGidMappings(command.SysProcAttr, container)
 
 		// Default to root user when user namespaces are enabled.
 		if command.SysProcAttr.Credential == nil {

namespaces/execin.go

@@ -179,10 +179,16 @@ func SetupContainer(container *libcontainer.Config, args []string) error {
 		return fmt.Errorf("Failed to find hostRootUid")
 	}
 
+	hostRootGid, err := GetHostRootGid(container)
+	if err != nil {
+		return fmt.Errorf("Failed to find hostRootGid")
+	}
+
 	if err := mount.InitializeMountNamespace(rootfs,
 		consolePath,
 		container.RestrictSys,
 		hostRootUid,
+		hostRootGid,
 		(*mount.MountConfig)(container.MountConfig)); err != nil {
 		fmt.Println("mounting issue: %v", err)
 		return fmt.Errorf("setup mount namespace %s", err)

namespaces/init.go

@@ -49,11 +49,10 @@ func Init(container *libcontainer.Config, uncleanRootfs, consolePath string, pip
 	}()
 
 	if container.Namespaces.Contains(libcontainer.NEWUSER) {
-		err = initUserNs(container, uncleanRootfs, consolePath, pipe, args)
+		return initUserNs(container, uncleanRootfs, consolePath, pipe, args)
 	} else {
-		err = initDefault(container, uncleanRootfs, consolePath, pipe, args)
+		return initDefault(container, uncleanRootfs, consolePath, pipe, args)
 	}
-	return err
 }
 
 func initDefault(container *libcontainer.Config, uncleanRootfs, consolePath string, pipe *os.File, args []string) (err error) {
@@ -108,6 +107,7 @@ func initDefault(container *libcontainer.Config, uncleanRootfs, consolePath stri
 		consolePath,
 		container.RestrictSys,
 		0, // Default Root Uid
+		0, // Default Root Gid
 		(*mount.MountConfig)(container.MountConfig)); err != nil {
 		return fmt.Errorf("setup mount namespace %s", err)
 	}

nsinit/exec.go

@@ -129,9 +129,8 @@ func startInExistingContainer(config *libcontainer.Config, state *libcontainer.S
 // Signals sent to the current process will be forwarded to container.
 func startContainer(container *libcontainer.Config, dataPath string, args []string) (int, error) {
 	var (
-		cmd      *exec.Cmd
-		setupCmd *exec.Cmd
-		sigc     = make(chan os.Signal, 10)
+		cmd  *exec.Cmd
+		sigc = make(chan os.Signal, 10)
 	)
 
 	signal.Notify(sigc)
@@ -145,11 +144,7 @@ func startContainer(container *libcontainer.Config, dataPath string, args []stri
 	}
 
 	setupCommand := func(container *libcontainer.Config, console, dataPath, init string) *exec.Cmd {
-		setupCmd = namespaces.DefaultSetupCommand(container, console, dataPath, init)
-		if logPath != "" {
-			cmd.Env = append(cmd.Env, fmt.Sprintf("log=%s", logPath))
-		}
-		return setupCmd
+		return namespaces.DefaultSetupCommand(container, console, dataPath, init)
 	}
 
 	var (