0e5605
From 3d99c51e1b38a440804a55c9f314f62cc50b8902 Mon Sep 17 00:00:00 2001
81896e
From: Giuseppe Scrivano <gscrivan@redhat.com>
81896e
Date: Fri, 25 May 2018 18:04:06 +0200
81896e
Subject: [PATCH] sd-notify: do not hang when NOTIFY_SOCKET is used with create
81896e
81896e
if NOTIFY_SOCKET is used, do not block the main runc process waiting
81896e
for events on the notify socket.  Bind mount the parent directory of
81896e
the notify socket, so that "start" can create the socket and it is
81896e
still accessible from the container.
81896e
81896e
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
81896e
---
0e5605
 notify_socket.go | 112 ++++++++++++++++++++++++++++++++++-------------
11acd1
 signals.go       |   4 +-
81896e
 start.go         |  13 +++++-
81896e
 utils_linux.go   |  12 ++++-
0e5605
 4 files changed, 105 insertions(+), 36 deletions(-)
81896e
81896e
diff --git a/notify_socket.go b/notify_socket.go
0e5605
index e7453c62..d961453a 100644
81896e
--- a/notify_socket.go
81896e
+++ b/notify_socket.go
0e5605
@@ -7,11 +7,13 @@ import (
81896e
 	"fmt"
81896e
 	"net"
0e5605
 	"os"
81896e
+	"path"
81896e
 	"path/filepath"
81896e
+	"strconv"
81896e
+	"time"
81896e
 
81896e
+	"github.com/opencontainers/runc/libcontainer"
81896e
 	"github.com/opencontainers/runtime-spec/specs-go"
81896e
-
81896e
-	"github.com/sirupsen/logrus"
81896e
 	"github.com/urfave/cli"
81896e
 )
81896e
 
0e5605
@@ -27,12 +29,12 @@ func newNotifySocket(context *cli.Context, notifySocketHost string, id string) *
81896e
 	}
81896e
 
81896e
 	root := filepath.Join(context.GlobalString("root"), id)
81896e
-	path := filepath.Join(root, "notify.sock")
81896e
+	socketPath := filepath.Join(root, "notify", "notify.sock")
81896e
 
81896e
 	notifySocket := &notifySocket{
81896e
 		socket:     nil,
81896e
 		host:       notifySocketHost,
81896e
-		socketPath: path,
81896e
+		socketPath: socketPath,
81896e
 	}
81896e
 
81896e
 	return notifySocket
0e5605
@@ -44,13 +46,19 @@ func (s *notifySocket) Close() error {
81896e
 
81896e
 // If systemd is supporting sd_notify protocol, this function will add support
81896e
 // for sd_notify protocol from within the container.
81896e
-func (s *notifySocket) setupSpec(context *cli.Context, spec *specs.Spec) {
81896e
-	mount := specs.Mount{Destination: s.host, Source: s.socketPath, Options: []string{"bind"}}
81896e
+func (s *notifySocket) setupSpec(context *cli.Context, spec *specs.Spec) error {
81896e
+	pathInContainer := filepath.Join("/run/notify", path.Base(s.socketPath))
81896e
+	mount := specs.Mount{
81896e
+		Destination: path.Dir(pathInContainer),
81896e
+		Source:      path.Dir(s.socketPath),
81896e
+		Options:     []string{"bind", "nosuid", "noexec", "nodev", "ro"},
81896e
+	}
81896e
 	spec.Mounts = append(spec.Mounts, mount)
81896e
-	spec.Process.Env = append(spec.Process.Env, fmt.Sprintf("NOTIFY_SOCKET=%s", s.host))
81896e
+	spec.Process.Env = append(spec.Process.Env, fmt.Sprintf("NOTIFY_SOCKET=%s", pathInContainer))
81896e
+	return nil
81896e
 }
81896e
 
81896e
-func (s *notifySocket) setupSocket() error {
81896e
+func (s *notifySocket) bindSocket() error {
81896e
 	addr := net.UnixAddr{
81896e
 		Name: s.socketPath,
81896e
 		Net:  "unixgram",
0e5605
@@ -71,45 +79,89 @@ func (s *notifySocket) setupSocket() error {
81896e
 	return nil
81896e
 }
81896e
 
81896e
-// pid1 must be set only with -d, as it is used to set the new process as the main process
81896e
-// for the service in systemd
11acd1
-func (s *notifySocket) run(pid1 int) {
81896e
-	buf := make([]byte, 512)
11acd1
-	notifySocketHostAddr := net.UnixAddr{Name: s.host, Net: "unixgram"}
81896e
+func (s *notifySocket) setupSocketDirectory() error {
81896e
+	return os.Mkdir(path.Dir(s.socketPath), 0755)
81896e
+}
81896e
+
81896e
+func notifySocketStart(context *cli.Context, notifySocketHost, id string) (*notifySocket, error) {
81896e
+	notifySocket := newNotifySocket(context, notifySocketHost, id)
81896e
+	if notifySocket == nil {
81896e
+		return nil, nil
81896e
+	}
81896e
+
81896e
+	if err := notifySocket.bindSocket(); err != nil {
81896e
+		return nil, err
81896e
+	}
81896e
+	return notifySocket, nil
81896e
+}
81896e
+
81896e
+func (n *notifySocket) waitForContainer(container libcontainer.Container) error {
81896e
+	s, err := container.State()
81896e
+	if err != nil {
81896e
+		return err
81896e
+	}
81896e
+	return n.run(s.InitProcessPid)
81896e
+}
81896e
+
81896e
+func (n *notifySocket) run(pid1 int) error {
81896e
+	if n.socket == nil {
81896e
+		return nil
81896e
+	}
81896e
+	notifySocketHostAddr := net.UnixAddr{Name: n.host, Net: "unixgram"}
81896e
 	client, err := net.DialUnix("unixgram", nil, &notifySocketHostAddr)
81896e
 	if err != nil {
81896e
-		logrus.Error(err)
81896e
-		return
81896e
+		return err
81896e
 	}
81896e
-	for {
11acd1
-		r, err := s.socket.Read(buf)
81896e
-		if err != nil {
81896e
-			break
81896e
+
81896e
+	ticker := time.NewTicker(time.Millisecond * 100)
81896e
+	defer ticker.Stop()
81896e
+
81896e
+	fileChan := make(chan []byte)
81896e
+	go func() {
81896e
+		for {
81896e
+			buf := make([]byte, 512)
81896e
+			r, err := n.socket.Read(buf)
81896e
+			if err != nil {
81896e
+				return
81896e
+			}
81896e
+			got := buf[0:r]
81896e
+			if !bytes.HasPrefix(got, []byte("READY=")) {
81896e
+				continue
81896e
+			}
81896e
+			fileChan <- got
81896e
+			return
81896e
 		}
81896e
-		var out bytes.Buffer
81896e
-		for _, line := range bytes.Split(buf[0:r], []byte{'\n'}) {
81896e
-			if bytes.HasPrefix(line, []byte("READY=")) {
81896e
+	}()
81896e
+
81896e
+	for {
81896e
+		select {
81896e
+		case <-ticker.C:
81896e
+			_, err := os.Stat(filepath.Join("/proc", strconv.Itoa(pid1)))
81896e
+			if err != nil {
81896e
+				return nil
81896e
+			}
81896e
+		case b := <-fileChan:
81896e
+			for _, line := range bytes.Split(b, []byte{'\n'}) {
81896e
+				var out bytes.Buffer
81896e
 				_, err = out.Write(line)
81896e
 				if err != nil {
81896e
-					return
81896e
+					return err
81896e
 				}
81896e
 
81896e
 				_, err = out.Write([]byte{'\n'})
81896e
 				if err != nil {
81896e
-					return
81896e
+					return err
81896e
 				}
81896e
 
81896e
 				_, err = client.Write(out.Bytes())
81896e
 				if err != nil {
81896e
-					return
81896e
+					return err
81896e
 				}
81896e
 
81896e
 				// now we can inform systemd to use pid1 as the pid to monitor
81896e
-				if pid1 > 0 {
81896e
-					newPid := fmt.Sprintf("MAINPID=%d\n", pid1)
81896e
-					client.Write([]byte(newPid))
81896e
-				}
81896e
-				return
81896e
+				newPid := fmt.Sprintf("MAINPID=%d\n", pid1)
81896e
+				client.Write([]byte(newPid))
81896e
+				return nil
81896e
 			}
81896e
 		}
81896e
 	}
81896e
diff --git a/signals.go b/signals.go
0e5605
index b67f65a0..dd25e094 100644
81896e
--- a/signals.go
81896e
+++ b/signals.go
11acd1
@@ -70,6 +70,7 @@ func (h *signalHandler) forward(process *libcontainer.Process, tty *tty, detach
81896e
 			h.notifySocket.run(pid1)
81896e
 			return 0, nil
81896e
 		}
11acd1
+		h.notifySocket.run(os.Getpid())
11acd1
 		go h.notifySocket.run(0)
81896e
 	}
81896e
 
11acd1
@@ -97,9 +98,6 @@ func (h *signalHandler) forward(process *libcontainer.Process, tty *tty, detach
81896e
 					// status because we must ensure that any of the go specific process
81896e
 					// fun such as flushing pipes are complete before we return.
81896e
 					process.Wait()
81896e
-					if h.notifySocket != nil {
81896e
-						h.notifySocket.Close()
81896e
-					}
81896e
 					return e.status, nil
81896e
 				}
81896e
 			}
81896e
diff --git a/start.go b/start.go
0e5605
index 2bb698b2..3a1769a4 100644
81896e
--- a/start.go
81896e
+++ b/start.go
81896e
@@ -3,6 +3,7 @@ package main
81896e
 import (
81896e
 	"errors"
81896e
 	"fmt"
81896e
+	"os"
81896e
 
81896e
 	"github.com/opencontainers/runc/libcontainer"
81896e
 	"github.com/urfave/cli"
81896e
@@ -31,7 +32,17 @@ your host.`,
81896e
 		}
81896e
 		switch status {
81896e
 		case libcontainer.Created:
81896e
-			return container.Exec()
81896e
+			notifySocket, err := notifySocketStart(context, os.Getenv("NOTIFY_SOCKET"), container.ID())
81896e
+			if err != nil {
81896e
+				return err
81896e
+			}
81896e
+			if err := container.Exec(); err != nil {
81896e
+				return err
81896e
+			}
81896e
+			if notifySocket != nil {
81896e
+				return notifySocket.waitForContainer(container)
81896e
+			}
81896e
+			return nil
81896e
 		case libcontainer.Stopped:
81896e
 			return errors.New("cannot start a container that has stopped")
81896e
 		case libcontainer.Running:
81896e
diff --git a/utils_linux.go b/utils_linux.go
0e5605
index 984e6b0f..46c26246 100644
81896e
--- a/utils_linux.go
81896e
+++ b/utils_linux.go
0e5605
@@ -408,7 +408,9 @@ func startContainer(context *cli.Context, spec *specs.Spec, action CtAct, criuOp
81896e
 
81896e
 	notifySocket := newNotifySocket(context, os.Getenv("NOTIFY_SOCKET"), id)
81896e
 	if notifySocket != nil {
81896e
-		notifySocket.setupSpec(context, spec)
81896e
+		if err := notifySocket.setupSpec(context, spec); err != nil {
81896e
+			return -1, err
81896e
+		}
81896e
 	}
81896e
 
81896e
 	container, err := createContainer(context, id, spec)
0e5605
@@ -417,10 +419,16 @@ func startContainer(context *cli.Context, spec *specs.Spec, action CtAct, criuOp
81896e
 	}
81896e
 
81896e
 	if notifySocket != nil {
81896e
-		err := notifySocket.setupSocket()
81896e
+		err := notifySocket.setupSocketDirectory()
81896e
 		if err != nil {
81896e
 			return -1, err
81896e
 		}
81896e
+		if action == CT_ACT_RUN {
81896e
+			err := notifySocket.bindSocket()
81896e
+			if err != nil {
81896e
+				return -1, err
81896e
+			}
81896e
+		}
81896e
 	}
81896e
 
81896e
 	// Support on-demand socket activation by passing file descriptors into the container init process.
0e5605
-- 
0e5605
2.21.0
0e5605