Merge pull request #508 from mrunalp/create_error

Capture errors from runtime create failures

Dockerfile

@@ -19,6 +19,7 @@ RUN apt-get update && apt-get install -y \
     protobuf-compiler \
     python-minimal \
     libglib2.0-dev \
+    libjson-glib-dev \
     libapparmor-dev \
     btrfs-tools \
     libdevmapper1.02.1 \

conmon/Makefile

@@ -1,8 +1,8 @@
 src = $(wildcard *.c)
 obj = $(src:.c=.o)
 
-override LIBS += $(shell pkg-config --libs glib-2.0)
-override CFLAGS += -std=c99 -Wall -Wextra $(shell pkg-config --cflags glib-2.0)
+override LIBS += $(shell pkg-config --libs glib-2.0 json-glib-1.0)
+override CFLAGS += -std=c99 -Wall -Wextra $(shell pkg-config --cflags glib-2.0 json-glib-1.0)
 
 conmon: $(obj)
 	$(CC) -o $@ $^ $(CFLAGS) $(LIBS)

conmon/conmon.c

@@ -17,6 +17,7 @@
 #include <unistd.h>
 
 #include <glib.h>
+#include <json-glib/json-glib.h>
 
 #include "cmsg.h"
 
@@ -475,8 +476,55 @@ int main(int argc, char *argv[])
 		errno = old_errno;
 		pexit("Failed to wait for `runtime %s`", exec ? "exec" : "create");
 	}
-	if (!WIFEXITED(runtime_status) || WEXITSTATUS(runtime_status) != 0)
+	if (!WIFEXITED(runtime_status) || WEXITSTATUS(runtime_status) != 0) {
+		if (sync_pipe_fd > 0 && !exec) {
+			if (terminal) {
+				/* 
+				 * For this case, the stderr is captured in the parent when terminal is passed down.
+			         * We send -1 as pid to signal to parent that create container has failed.
+				 */
+				len = snprintf(buf, BUF_SIZE, "{\"pid\": %d}\n", -1);
+				if (len < 0 || write(sync_pipe_fd, buf, len) != len) {
+					pexit("unable to send container pid to parent");
+				}
+			} else {
+				/*
+				 * Read from container stderr for any error and send it to parent
+			         * We send -1 as pid to signal to parent that create container has failed.
+				 */
+				num_read = read(masterfd_stderr, buf, BUF_SIZE);
+				if (num_read > 0) {
+					buf[num_read] = '\0';
+					JsonGenerator *generator = json_generator_new();
+					JsonNode *root;
+					JsonObject *object;
+					gchar *data;
+					gsize len;
+
+					root = json_node_new(JSON_NODE_OBJECT);
+					object = json_object_new();
+
+					json_object_set_int_member(object, "pid", -1);
+					json_object_set_string_member(object, "message", buf);
+
+					json_node_take_object(root, object);
+					json_generator_set_root(generator, root);
+
+					g_object_set(generator, "pretty", FALSE, NULL);
+					data = json_generator_to_data (generator, &len);
+					fprintf(stderr, "%s\n", data);
+					if (write(sync_pipe_fd, data, len) != (int)len) {
+						ninfo("Unable to send container stderr message to parent");
+					}
+
+					g_free(data);
+					json_node_free(root);
+					g_object_unref(generator);
+				}
+			}
+		}
 		nexit("Failed to create container: exit status %d", WEXITSTATUS(runtime_status));
+	}
 
 	/* Read the pid so we can wait for the process to exit */
 	g_file_get_contents(pid_file, &contents, NULL, &err);

oci/oci.go

@@ -54,7 +54,8 @@ type Runtime struct {
 
 // syncInfo is used to return data from monitor process to daemon
 type syncInfo struct {
-	Pid int `json:"pid"`
+	Pid     int    `json:"pid"`
+	Message string `json:"message,omitempty"`
 }
 
 // exitCodeInfo is used to return the monitored process exit code to the daemon
@@ -100,6 +101,7 @@ func getOCIVersion(name string, args ...string) (string, error) {
 
 // CreateContainer creates a container.
 func (r *Runtime) CreateContainer(c *Container, cgroupParent string) error {
+	var stderrBuf bytes.Buffer
 	parentPipe, childPipe, err := newPipe()
 	if err != nil {
 		return fmt.Errorf("error creating socket pair: %v", err)
@@ -130,6 +132,9 @@ func (r *Runtime) CreateContainer(c *Container, cgroupParent string) error {
 	cmd.Stdin = os.Stdin
 	cmd.Stdout = os.Stdout
 	cmd.Stderr = os.Stderr
+	if c.terminal {
+		cmd.Stderr = &stderrBuf
+	}
 	cmd.ExtraFiles = append(cmd.ExtraFiles, childPipe)
 	// 0, 1 and 2 are stdin, stdout and stderr
 	cmd.Env = append(r.conmonEnv, fmt.Sprintf("_OCI_SYNCPIPE=%d", 3))
@@ -171,15 +176,44 @@ func (r *Runtime) CreateContainer(c *Container, cgroupParent string) error {
 	select {
 	case ss := <-ch:
 		if ss.err != nil {
-			return err
+			return fmt.Errorf("error reading container (probably exited) json message: %v", ss.err)
 		}
 		logrus.Infof("Received container pid: %d", ss.si.Pid)
+		errorMessage := ""
+		if c.terminal {
+			errorMessage = stderrBuf.String()
+			fmt.Fprintf(os.Stderr, errorMessage)
+			errorMessage = sanitizeConmonErrorMessage(errorMessage)
+		} else {
+			if ss.si.Message != "" {
+				errorMessage = ss.si.Message
+			}
+		}
+
+		if ss.si.Pid == -1 {
+			if errorMessage != "" {
+				return fmt.Errorf("container create failed: %s", errorMessage)
+			}
+			return fmt.Errorf("container create failed")
+		}
 	case <-time.After(ContainerCreateTimeout):
 		return fmt.Errorf("create container timeout")
 	}
 	return nil
 }
 
+// sanitizeConmonErrorMessage removes conmon debug messages from error string
+func sanitizeConmonErrorMessage(errString string) string {
+	var sanitizedLines []string
+	lines := strings.Split(errString, "\n")
+	for _, line := range lines {
+		if !strings.HasPrefix(line, "[conmon") {
+			sanitizedLines = append(sanitizedLines, line)
+		}
+	}
+	return strings.Join(sanitizedLines, "\n")
+}
+
 func createUnitName(prefix string, name string) string {
 	return fmt.Sprintf("%s-%s.scope", prefix, name)
 }