From: radhika <radhika@curoverse.com>
Date: Fri, 7 Apr 2017 00:12:59 +0000 (-0400)
Subject: 8465: added stderr redirection and tests
X-Git-Tag: 1.1.0~312^2~3
X-Git-Url: https://git.arvados.org/arvados.git/commitdiff_plain/2ebd44960d1e7a0431d6ad612298012627c5019c

8465: added stderr redirection and tests
---

diff --git a/services/crunch-run/crunchrun.go b/services/crunch-run/crunchrun.go
index c8f77f4917..7ed5be998b 100644
--- a/services/crunch-run/crunchrun.go
+++ b/services/crunch-run/crunchrun.go
@@ -135,7 +135,7 @@ type ContainerRunner struct {
 	loggingDone   chan bool
 	CrunchLog     *ThrottledLogger
 	Stdout        io.WriteCloser
-	Stderr        *ThrottledLogger
+	Stderr        io.WriteCloser
 	LogCollection *CollectionWriter
 	LogsPDH       *string
 	RunArvMount
@@ -346,10 +346,10 @@ func (runner *ContainerRunner) SetupMounts() (err error) {
 
 	for _, bind := range binds {
 		mnt := runner.Container.Mounts[bind]
-		if bind == "stdout" {
+		if bind == "stdout" || bind == "stderr" {
 			// Is it a "file" mount kind?
 			if mnt.Kind != "file" {
-				return fmt.Errorf("Unsupported mount kind '%s' for stdout. Only 'file' is supported.", mnt.Kind)
+				return fmt.Errorf("Unsupported mount kind '%s' for %s. Only 'file' is supported.", mnt.Kind, bind)
 			}
 
 			// Does path start with OutputPath?
@@ -358,7 +358,7 @@ func (runner *ContainerRunner) SetupMounts() (err error) {
 				prefix += "/"
 			}
 			if !strings.HasPrefix(mnt.Path, prefix) {
-				return fmt.Errorf("Stdout path does not start with OutputPath: %s, %s", mnt.Path, prefix)
+				return fmt.Errorf("%s path does not start with OutputPath: %s, %s", strings.Title(bind), mnt.Path, prefix)
 			}
 		}
 
@@ -665,8 +665,8 @@ func (runner *ContainerRunner) LogContainerRecord() (err error) {
 	return nil
 }
 
-// AttachLogs connects the docker container stdout and stderr logs to the
-// Arvados logger which logs to Keep and the API server logs table.
+// AttachStreams connects the docker container stdin, stdout and stderr logs
+// to the Arvados logger which logs to Keep and the API server logs table.
 func (runner *ContainerRunner) AttachStreams() (err error) {
 
 	runner.CrunchLog.Print("Attaching container streams")
@@ -712,31 +712,24 @@ func (runner *ContainerRunner) AttachStreams() (err error) {
 	runner.loggingDone = make(chan bool)
 
 	if stdoutMnt, ok := runner.Container.Mounts["stdout"]; ok {
-		stdoutPath := stdoutMnt.Path[len(runner.Container.OutputPath):]
-		index := strings.LastIndex(stdoutPath, "/")
-		if index > 0 {
-			subdirs := stdoutPath[:index]
-			if subdirs != "" {
-				st, err := os.Stat(runner.HostOutputDir)
-				if err != nil {
-					return fmt.Errorf("While Stat on temp dir: %v", err)
-				}
-				stdoutPath := path.Join(runner.HostOutputDir, subdirs)
-				err = os.MkdirAll(stdoutPath, st.Mode()|os.ModeSetgid|0777)
-				if err != nil {
-					return fmt.Errorf("While MkdirAll %q: %v", stdoutPath, err)
-				}
-			}
-		}
-		stdoutFile, err := os.Create(path.Join(runner.HostOutputDir, stdoutPath))
+		stdoutFile, err := runner.getStdoutFile(stdoutMnt.Path)
 		if err != nil {
-			return fmt.Errorf("While creating stdout file: %v", err)
+			return err
 		}
 		runner.Stdout = stdoutFile
 	} else {
 		runner.Stdout = NewThrottledLogger(runner.NewLogWriter("stdout"))
 	}
-	runner.Stderr = NewThrottledLogger(runner.NewLogWriter("stderr"))
+
+	if stderrMnt, ok := runner.Container.Mounts["stderr"]; ok {
+		stderrFile, err := runner.getStdoutFile(stderrMnt.Path)
+		if err != nil {
+			return err
+		}
+		runner.Stderr = stderrFile
+	} else {
+		runner.Stderr = NewThrottledLogger(runner.NewLogWriter("stderr"))
+	}
 
 	if stdinRdr != nil {
 		go func() {
@@ -763,6 +756,31 @@ func (runner *ContainerRunner) AttachStreams() (err error) {
 	return nil
 }
 
+func (runner *ContainerRunner) getStdoutFile(mntPath string) (*os.File, error) {
+	stdoutPath := mntPath[len(runner.Container.OutputPath):]
+	index := strings.LastIndex(stdoutPath, "/")
+	if index > 0 {
+		subdirs := stdoutPath[:index]
+		if subdirs != "" {
+			st, err := os.Stat(runner.HostOutputDir)
+			if err != nil {
+				return nil, fmt.Errorf("While Stat on temp dir: %v", err)
+			}
+			stdoutPath := path.Join(runner.HostOutputDir, subdirs)
+			err = os.MkdirAll(stdoutPath, st.Mode()|os.ModeSetgid|0777)
+			if err != nil {
+				return nil, fmt.Errorf("While MkdirAll %q: %v", stdoutPath, err)
+			}
+		}
+	}
+	stdoutFile, err := os.Create(path.Join(runner.HostOutputDir, stdoutPath))
+	if err != nil {
+		return nil, fmt.Errorf("While creating file %q: %v", stdoutPath, err)
+	}
+
+	return stdoutFile, nil
+}
+
 // CreateContainer creates the docker container.
 func (runner *ContainerRunner) CreateContainer() error {
 	runner.CrunchLog.Print("Creating Docker container")
diff --git a/services/crunch-run/crunchrun_test.go b/services/crunch-run/crunchrun_test.go
index 3062fc0194..a78eb63def 100644
--- a/services/crunch-run/crunchrun_test.go
+++ b/services/crunch-run/crunchrun_test.go
@@ -1430,7 +1430,6 @@ func (s *TestSuite) TestStdinCollectionMountPoint(c *C) {
 			collection := v["collection"].(arvadosclient.Dict)
 			if strings.Index(collection["name"].(string), "output") == 0 {
 				manifest := collection["manifest_text"].(string)
-
 				c.Check(manifest, Equals, `./a/b 307372fa8fd5c146b22ae7a45b49bc31+6 0:6:c.out
 `)
 			}
@@ -1466,10 +1465,35 @@ func (s *TestSuite) TestStdinJsonMountPoint(c *C) {
 			collection := v["collection"].(arvadosclient.Dict)
 			if strings.Index(collection["name"].(string), "output") == 0 {
 				manifest := collection["manifest_text"].(string)
-
 				c.Check(manifest, Equals, `./a/b 307372fa8fd5c146b22ae7a45b49bc31+6 0:6:c.out
 `)
 			}
 		}
 	}
 }
+
+func (s *TestSuite) TestStderrMount(c *C) {
+	api, _, _ := FullRunHelper(c, `{
+    "command": ["/bin/sh", "-c", "echo hello;exit 1"],
+    "container_image": "d4ab34d3d4f8a72f5c4973051ae69fab+122",
+    "cwd": ".",
+    "environment": {},
+    "mounts": {"/tmp": {"kind": "tmp"},
+               "stdout": {"kind": "file", "path": "/tmp/a/out.txt"},
+               "stderr": {"kind": "file", "path": "/tmp/b/err.txt"}},
+    "output_path": "/tmp",
+    "priority": 1,
+    "runtime_constraints": {}
+}`, nil, 1, func(t *TestDockerClient) {
+		t.logWriter.Write(dockerLog(1, "hello\n"))
+		t.logWriter.Write(dockerLog(2, "oops\n"))
+		t.logWriter.Close()
+	})
+
+	final := api.CalledWith("container.state", "Complete")
+	c.Assert(final, NotNil)
+	c.Check(final["container"].(arvadosclient.Dict)["exit_code"], Equals, 1)
+	c.Check(final["container"].(arvadosclient.Dict)["log"], NotNil)
+
+	c.Check(api.CalledWith("collection.manifest_text", "./a b1946ac92492d2347c6235b4d2611184+6 0:6:out.txt\n./b 38af5c54926b620264ab1501150cf189+5 0:5:err.txt\n"), NotNil)
+}