Merge pull request openshift#54 from crosbymichael/report-child-error

Report child error to parent

namespaces/exec.go

@@ -32,6 +32,7 @@ func Exec(container *libcontainer.Config, term Terminal, rootfs, dataPath string
 	if err != nil {
 		return -1, err
 	}
+	defer syncPipe.Close()
 
 	if container.Tty {
 		master, console, err = system.CreateMasterAndConsole()
@@ -52,6 +53,9 @@ func Exec(container *libcontainer.Config, term Terminal, rootfs, dataPath string
 		return -1, err
 	}
 
+	// Now we passed the pipe to the child, close our side
+	syncPipe.CloseChild()
+
 	started, err := system.GetProcessStartTime(command.Process.Pid)
 	if err != nil {
 		return -1, err
@@ -90,7 +94,11 @@ func Exec(container *libcontainer.Config, term Terminal, rootfs, dataPath string
 	defer libcontainer.DeleteState(dataPath)
 
 	// Sync with child
-	syncPipe.Close()
+	if err := syncPipe.ReadFromChild(); err != nil {
+		command.Process.Kill()
+		command.Wait()
+		return -1, err
+	}
 
 	if startCallback != nil {
 		startCallback()

namespaces/init.go

@@ -27,7 +27,13 @@ import (
 // Move this to libcontainer package.
 // Init is the init process that first runs inside a new namespace to setup mounts, users, networking,
 // and other options required for the new container.
-func Init(container *libcontainer.Config, uncleanRootfs, consolePath string, syncPipe *SyncPipe, args []string) error {
+func Init(container *libcontainer.Config, uncleanRootfs, consolePath string, syncPipe *SyncPipe, args []string) (err error) {
+	defer func() {
+		if err != nil {
+			syncPipe.ReportChildError(err)
+		}
+	}()
+
 	rootfs, err := utils.ResolveRootfs(uncleanRootfs)
 	if err != nil {
 		return err
@@ -42,10 +48,8 @@ func Init(container *libcontainer.Config, uncleanRootfs, consolePath string, syn
 	// We always read this as it is a way to sync with the parent as well
 	networkState, err := syncPipe.ReadFromParent()
 	if err != nil {
-		syncPipe.Close()
 		return err
 	}
-	syncPipe.Close()
 
 	if consolePath != "" {
 		if err := console.OpenAndDup(consolePath); err != nil {

namespaces/sync_pipe.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"io/ioutil"
 	"os"
+	"syscall"
 
 	"github.com/docker/libcontainer/network"
 )
@@ -16,24 +17,17 @@ type SyncPipe struct {
 	parent, child *os.File
 }
 
-func NewSyncPipe() (s *SyncPipe, err error) {
-	s = &SyncPipe{}
-	s.child, s.parent, err = os.Pipe()
-	if err != nil {
-		return nil, err
-	}
-	return s, nil
-}
-
-func NewSyncPipeFromFd(parendFd, childFd uintptr) (*SyncPipe, error) {
+func NewSyncPipeFromFd(parentFd, childFd uintptr) (*SyncPipe, error) {
 	s := &SyncPipe{}
-	if parendFd > 0 {
-		s.parent = os.NewFile(parendFd, "parendPipe")
+
+	if parentFd > 0 {
+		s.parent = os.NewFile(parentFd, "parentPipe")
 	} else if childFd > 0 {
 		s.child = os.NewFile(childFd, "childPipe")
 	} else {
 		return nil, fmt.Errorf("no valid sync pipe fd specified")
 	}
+
 	return s, nil
 }
 
@@ -50,7 +44,22 @@ func (s *SyncPipe) SendToChild(networkState *network.NetworkState) error {
 	if err != nil {
 		return err
 	}
+
 	s.parent.Write(data)
+
+	return syscall.Shutdown(int(s.parent.Fd()), syscall.SHUT_WR)
+}
+
+func (s *SyncPipe) ReadFromChild() error {
+	data, err := ioutil.ReadAll(s.parent)
+	if err != nil {
+		return err
+	}
+
+	if len(data) > 0 {
+		return fmt.Errorf("%s", data)
+	}
+
 	return nil
 }
 
@@ -66,15 +75,28 @@ func (s *SyncPipe) ReadFromParent() (*network.NetworkState, error) {
 		}
 	}
 	return networkState, nil
+}
 
+func (s *SyncPipe) ReportChildError(err error) {
+	s.child.Write([]byte(err.Error()))
+	s.CloseChild()
 }
 
 func (s *SyncPipe) Close() error {
 	if s.parent != nil {
 		s.parent.Close()
 	}
+
 	if s.child != nil {
 		s.child.Close()
 	}
+
 	return nil
 }
+
+func (s *SyncPipe) CloseChild() {
+	if s.child != nil {
+		s.child.Close()
+		s.child = nil
+	}
+}

namespaces/sync_pipe_linux.go

@@ -0,0 +1,20 @@
+package namespaces
+
+import (
+	"os"
+	"syscall"
+)
+
+func NewSyncPipe() (s *SyncPipe, err error) {
+	s = &SyncPipe{}
+
+	fds, err := syscall.Socketpair(syscall.AF_LOCAL, syscall.SOCK_STREAM|syscall.SOCK_CLOEXEC, 0)
+	if err != nil {
+		return nil, err
+	}
+
+	s.child = os.NewFile(uintptr(fds[0]), "child syncpipe")
+	s.parent = os.NewFile(uintptr(fds[1]), "parent syncpipe")
+
+	return s, nil
+}

namespaces/sync_pipe_test.go

@@ -0,0 +1,61 @@
+package namespaces
+
+import (
+	"fmt"
+	"testing"
+
+	"github.com/docker/libcontainer/network"
+)
+
+func TestSendErrorFromChild(t *testing.T) {
+	pipe, err := NewSyncPipe()
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer func() {
+		if err := pipe.Close(); err != nil {
+			t.Fatal(err)
+		}
+	}()
+
+	expected := "something bad happened"
+
+	pipe.ReportChildError(fmt.Errorf(expected))
+
+	childError := pipe.ReadFromChild()
+	if childError == nil {
+		t.Fatal("expected an error to be returned but did not receive anything")
+	}
+
+	if childError.Error() != expected {
+		t.Fatalf("expected %q but received error message %q", expected, childError.Error())
+	}
+}
+
+func TestSendPayloadToChild(t *testing.T) {
+	pipe, err := NewSyncPipe()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	defer func() {
+		if err := pipe.Close(); err != nil {
+			t.Fatal(err)
+		}
+	}()
+
+	expected := "libcontainer"
+
+	if err := pipe.SendToChild(&network.NetworkState{VethHost: expected}); err != nil {
+		t.Fatal(err)
+	}
+
+	payload, err := pipe.ReadFromParent()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if payload.VethHost != expected {
+		t.Fatalf("expected veth host %q but received %q", expected, payload.VethHost)
+	}
+}