fix: Correctly order emissary combined output. Fixes argoproj#8159 (a…

…rgoproj#8175) * fix: Correctly order emissary combined output. Fixes argoproj#8168 Signed-off-by: Alex Collins <[email protected]> * fix: Correctly order emissary combined output. Fixes argoproj#8168 Signed-off-by: Alex Collins <[email protected]> * fix: Correctly order emissary combined output. Fixes argoproj#8168 Signed-off-by: Alex Collins <[email protected]> * fix: Correctly order emissary combined output. Fixes argoproj#8168 Signed-off-by: Alex Collins <[email protected]>
dpadhiar · Mar 23, 2022 · 925747c · 925747c
1 parent f72f4f7
commit 925747c
Show file tree

Hide file tree

Showing 5 changed files with 18 additions and 81 deletions.
diff --git a/cmd/argoexec/commands/emissary.go b/cmd/argoexec/commands/emissary.go
@@ -29,7 +29,7 @@ import (
 var (
 	varRunArgo          = "/var/run/argo"
 	containerName       = os.Getenv(common.EnvVarContainerName)
-	includeScriptOutput = os.Getenv(common.EnvVarIncludeScriptOutput) == "true" // capture stdout/stderr
+	includeScriptOutput = os.Getenv(common.EnvVarIncludeScriptOutput) == "true" // capture stdout/combined
 	template            = &wfv1.Template{}
 	logger              = log.WithField("argo", true)
 )
@@ -129,15 +129,15 @@ func NewEmissaryCommand() *cobra.Command {
 
 			var command *exec.Cmd
 			var stdout *os.File
-			var stderr *os.File
+			var combined *os.File
 			cmdErr := retry.OnError(backoff, func(error) bool { return true }, func() error {
 				if stdout != nil {
 					stdout.Close()
 				}
-				if stderr != nil {
-					stderr.Close()
+				if combined != nil {
+					combined.Close()
 				}
-				command, stdout, stderr, err = createCommand(name, args, template)
+				command, stdout, combined, err = createCommand(name, args, template)
 				if err != nil {
 					return fmt.Errorf("failed to create command: %w", err)
 				}
@@ -167,7 +167,7 @@ func NewEmissaryCommand() *cobra.Command {
 				return command.Wait()
 			})
 			defer stdout.Close()
-			defer stderr.Close()
+			defer combined.Close()
 
 			if _, ok := os.LookupEnv("ARGO_DEBUG_PAUSE_AFTER"); ok {
 				for {
@@ -225,25 +225,23 @@ func createCommand(name string, args []string, template *wfv1.Template) (*exec.C
 	command.Stderr = os.Stderr
 
 	var stdout *os.File
-	var stderr *os.File
+	var combined *os.File
 	var err error
 	// this may not be that important an optimisation, except for very long logs we don't want to capture
 	if includeScriptOutput || template.SaveLogsAsArtifact() {
 		logger.Info("capturing logs")
 		stdout, err = os.OpenFile(varRunArgo+"/ctr/"+containerName+"/stdout", os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0666)
 		if err != nil {
 			return nil, nil, nil, fmt.Errorf("failed to open stdout: %w", err)
-
 		}
-		command.Stdout = io.MultiWriter(os.Stdout, stdout)
-		stderr, err = os.OpenFile(varRunArgo+"/ctr/"+containerName+"/stderr", os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0666)
+		combined, err = os.OpenFile(varRunArgo+"/ctr/"+containerName+"/combined", os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0666)
 		if err != nil {
-			return nil, nil, nil, fmt.Errorf("failed to open stderr: %w", err)
-
+			return nil, nil, nil, fmt.Errorf("failed to open combined: %w", err)
 		}
-		command.Stderr = io.MultiWriter(os.Stderr, stderr)
+		command.Stdout = io.MultiWriter(os.Stdout, stdout, combined)
+		command.Stderr = io.MultiWriter(os.Stderr, combined)
 	}
-	return command, stdout, stderr, nil
+	return command, stdout, combined, nil
 }
 
 func saveArtifact(srcPath string) error {

diff --git a/cmd/argoexec/commands/emissary_test.go b/cmd/argoexec/commands/emissary_test.go
@@ -50,10 +50,10 @@ func TestEmissary(t *testing.T) {
 		assert.NoError(t, err)
 		assert.Contains(t, string(data), "hello")
 	})
-	t.Run("Stderr", func(t *testing.T) {
+	t.Run("Comined", func(t *testing.T) {
 		err := run(x, []string{"echo", "hello", "/dev/stderr"})
 		assert.NoError(t, err)
-		data, err := ioutil.ReadFile(varRunArgo + "/ctr/main/stderr")
+		data, err := ioutil.ReadFile(varRunArgo + "/ctr/main/combined")
 		assert.NoError(t, err)
 		assert.Contains(t, string(data), "hello")
 	})

diff --git a/workflow/executor/emissary/emissary.go b/workflow/executor/emissary/emissary.go
@@ -31,7 +31,7 @@ The init container creates these files:
 In the main container, the emissary creates these files:
 
 * `/var/run/argo/ctr/${containerName}/exitcode` The container exit code.
-* `/var/run/argo/ctr/${containerName}/stderr` A copy of stderr (if needed).
+* `/var/run/argo/ctr/${containerName}/combined` A copy of stdout+stderr (if needed).
 * `/var/run/argo/ctr/${containerName}/stdout`  A copy of stdout (if needed).
 
 If the container is named `main` it also copies base-layer artifacts to the shared volume:
@@ -103,21 +103,11 @@ func (e emissary) CopyFile(_ string, sourcePath string, destPath string, _ int)
 }
 
 func (e emissary) GetOutputStream(_ context.Context, containerName string, combinedOutput bool) (io.ReadCloser, error) {
-	names := []string{"stdout"}
+	name := "stdout"
 	if combinedOutput {
-		names = append(names, "stderr")
+		name = "combined"
 	}
-	var files []io.ReadCloser
-	for _, name := range names {
-		f, err := os.Open(filepath.Clean("/var/run/argo/ctr/" + containerName + "/" + name))
-		if os.IsNotExist(err) {
-			continue
-		} else if err != nil {
-			return nil, err
-		}
-		files = append(files, f)
-	}
-	return newMultiReaderCloser(files...), nil
+	return os.Open(filepath.Clean("/var/run/argo/ctr/" + containerName + "/" + name))
 }
 
 func (e emissary) Wait(ctx context.Context, containerNames []string) error {

diff --git a/workflow/executor/emissary/multi_reader_closer.go b/workflow/executor/emissary/multi_reader_closer.go
diff --git a/workflow/executor/emissary/multi_reader_closer_test.go b/workflow/executor/emissary/multi_reader_closer_test.go