Merge pull request #5232 from kolyshkin/int-wait-stderr

libct/int: better error reporting

libcontainer/integration/bench_test.go

@@ -1,7 +1,6 @@
 package integration
 
 import (
-	"bytes"
 	"math/rand"
 	"os"
 	"strings"
@@ -95,7 +94,7 @@ func BenchmarkExecInBigEnv(b *testing.B) {
 	const numEnv = 5000
 	env := append(standardEnvironment, genBigEnv(numEnv)...)
 	// Construct the expected output.
-	var wantOut bytes.Buffer
+	var wantOut strings.Builder
 	for _, e := range env {
 		wantOut.WriteString(e + "\n")
 	}
@@ -106,14 +105,13 @@ func BenchmarkExecInBigEnv(b *testing.B) {
 			Cwd:    "/",
 			Args:   []string{"env"},
 			Env:    env,
-			Stdin:  buffers.Stdin,
 			Stdout: buffers.Stdout,
 			Stderr: buffers.Stderr,
 		}
 		err = container.Run(exec)
 		ok(b, err)
 		waitProcess(exec, b)
-		if !bytes.Equal(buffers.Stdout.Bytes(), wantOut.Bytes()) {
+		if buffers.Stdout.String() != wantOut.String() {
 			b.Fatalf("unexpected output: %s (stderr: %s)", buffers.Stdout, buffers.Stderr)
 		}
 	}

libcontainer/integration/checkpoint_test.go

@@ -1,7 +1,6 @@
 package integration
 
 import (
-	"bytes"
 	"os"
 	"os/exec"
 	"path/filepath"
@@ -54,7 +53,7 @@ func testCheckpoint(t *testing.T, userns bool) {
 	stdinR, stdinW, err := os.Pipe()
 	ok(t, err)
 
-	var stdout bytes.Buffer
+	var stdout strings.Builder
 
 	pconfig := libcontainer.Process{
 		Cwd:    "/",
@@ -131,7 +130,7 @@ func testCheckpoint(t *testing.T, userns bool) {
 	restoreStdinR, restoreStdinW, err := os.Pipe()
 	ok(t, err)
 
-	var restoreStdout bytes.Buffer
+	var restoreStdout strings.Builder
 	restoreProcessConfig := &libcontainer.Process{
 		Cwd:    "/",
 		Stdin:  restoreStdinR,

libcontainer/integration/exec_test.go

@@ -1,7 +1,6 @@
 package integration
 
 import (
-	"bytes"
 	"encoding/json"
 	"errors"
 	"fmt"
@@ -63,7 +62,7 @@ func TestIPCPrivate(t *testing.T) {
 	ok(t, err)
 
 	config := newTemplateConfig(t, nil)
-	buffers := runContainerOk(t, config, "readlink", "/proc/self/ns/ipc")
+	buffers := runContainerOk(t, config, "readlink", "-v", "/proc/self/ns/ipc")
 
 	if actual := strings.Trim(buffers.Stdout.String(), "\n"); actual == l {
 		t.Fatalf("ipc link should be private to the container but equals host %q %q", actual, l)
@@ -80,7 +79,7 @@ func TestIPCHost(t *testing.T) {
 
 	config := newTemplateConfig(t, nil)
 	config.Namespaces.Remove(configs.NEWIPC)
-	buffers := runContainerOk(t, config, "readlink", "/proc/self/ns/ipc")
+	buffers := runContainerOk(t, config, "readlink", "-v", "/proc/self/ns/ipc")
 
 	if actual := strings.Trim(buffers.Stdout.String(), "\n"); actual != l {
 		t.Fatalf("ipc link not equal to host link %q %q", actual, l)
@@ -97,7 +96,7 @@ func TestIPCJoinPath(t *testing.T) {
 
 	config := newTemplateConfig(t, nil)
 	config.Namespaces.Add(configs.NEWIPC, "/proc/1/ns/ipc")
-	buffers := runContainerOk(t, config, "readlink", "/proc/self/ns/ipc")
+	buffers := runContainerOk(t, config, "readlink", "-v", "/proc/self/ns/ipc")
 
 	if actual := strings.Trim(buffers.Stdout.String(), "\n"); actual != l {
 		t.Fatalf("ipc link not equal to host link %q %q", actual, l)
@@ -163,14 +162,15 @@ func TestEnter(t *testing.T) {
 	stdinR, stdinW, err := os.Pipe()
 	ok(t, err)
 
-	var stdout, stdout2 bytes.Buffer
+	var stdout, stdout2 strings.Builder
 
 	pconfig := libcontainer.Process{
 		Cwd:    "/",
-		Args:   []string{"sh", "-c", "cat && readlink /proc/self/ns/pid"},
+		Args:   []string{"sh", "-c", "cat && readlink -v /proc/self/ns/pid"},
 		Env:    standardEnvironment,
 		Stdin:  stdinR,
 		Stdout: &stdout,
+		Stderr: new(strings.Builder),
 		Init:   true,
 	}
 	err = container.Run(&pconfig)
@@ -184,13 +184,13 @@ func TestEnter(t *testing.T) {
 	stdinR2, stdinW2, err := os.Pipe()
 	ok(t, err)
 	pconfig2 := libcontainer.Process{
-		Cwd: "/",
-		Env: standardEnvironment,
+		Cwd:    "/",
+		Args:   []string{"sh", "-c", "cat && readlink -v /proc/self/ns/pid"},
+		Env:    standardEnvironment,
+		Stdin:  stdinR2,
+		Stdout: &stdout2,
+		Stderr: new(strings.Builder),
 	}
-	pconfig2.Args = []string{"sh", "-c", "cat && readlink /proc/self/ns/pid"}
-	pconfig2.Stdin = stdinR2
-	pconfig2.Stdout = &stdout2
-
 	err = container.Run(&pconfig2)
 	_ = stdinR2.Close()
 	defer stdinW2.Close()
@@ -241,7 +241,7 @@ func TestProcessEnv(t *testing.T) {
 	ok(t, err)
 	defer destroyContainer(container)
 
-	var stdout bytes.Buffer
+	var stdout strings.Builder
 	pconfig := libcontainer.Process{
 		Cwd:  "/",
 		Args: []string{"sh", "-c", "env"},
@@ -253,6 +253,7 @@ func TestProcessEnv(t *testing.T) {
 		},
 		Stdin:  nil,
 		Stdout: &stdout,
+		Stderr: new(strings.Builder),
 		Init:   true,
 	}
 	err = container.Run(&pconfig)
@@ -286,13 +287,14 @@ func TestProcessEmptyCaps(t *testing.T) {
 	ok(t, err)
 	defer destroyContainer(container)
 
-	var stdout bytes.Buffer
+	var stdout strings.Builder
 	pconfig := libcontainer.Process{
 		Cwd:    "/",
 		Args:   []string{"sh", "-c", "cat /proc/self/status"},
 		Env:    standardEnvironment,
 		Stdin:  nil,
 		Stdout: &stdout,
+		Stderr: new(strings.Builder),
 		Init:   true,
 	}
 	err = container.Run(&pconfig)
@@ -329,13 +331,14 @@ func TestProcessCaps(t *testing.T) {
 	ok(t, err)
 	defer destroyContainer(container)
 
-	var stdout bytes.Buffer
+	var stdout strings.Builder
 	pconfig := libcontainer.Process{
 		Cwd:          "/",
 		Args:         []string{"sh", "-c", "cat /proc/self/status"},
 		Env:          standardEnvironment,
 		Stdin:        nil,
 		Stdout:       &stdout,
+		Stderr:       new(strings.Builder),
 		Capabilities: &configs.Capabilities{},
 		Init:         true,
 	}
@@ -389,13 +392,14 @@ func TestAdditionalGroups(t *testing.T) {
 	ok(t, err)
 	defer destroyContainer(container)
 
-	var stdout bytes.Buffer
+	var stdout strings.Builder
 	pconfig := libcontainer.Process{
 		Cwd:              "/",
 		Args:             []string{"sh", "-c", "id", "-Gn"},
 		Env:              standardEnvironment,
 		Stdin:            nil,
 		Stdout:           &stdout,
+		Stderr:           new(strings.Builder),
 		AdditionalGroups: []int{3333, 99999},
 		Init:             true,
 	}
@@ -767,7 +771,7 @@ func TestPassExtraFiles(t *testing.T) {
 	ok(t, err)
 	defer destroyContainer(container)
 
-	var stdout bytes.Buffer
+	var stdout strings.Builder
 	pipeout1, pipein1, err := os.Pipe()
 	ok(t, err)
 	pipeout2, pipein2, err := os.Pipe()
@@ -826,13 +830,14 @@ func TestSysctl(t *testing.T) {
 	ok(t, err)
 	defer destroyContainer(container)
 
-	var stdout bytes.Buffer
+	var stdout strings.Builder
 	pconfig := libcontainer.Process{
 		Cwd:    "/proc/sys/kernel",
 		Args:   []string{"sh", "-c", cmd},
 		Env:    standardEnvironment,
 		Stdin:  nil,
 		Stdout: &stdout,
+		Stderr: new(strings.Builder),
 		Init:   true,
 	}
 	err = container.Run(&pconfig)
@@ -935,13 +940,14 @@ func TestOomScoreAdj(t *testing.T) {
 	ok(t, err)
 	defer destroyContainer(container)
 
-	var stdout bytes.Buffer
+	var stdout strings.Builder
 	pconfig := libcontainer.Process{
 		Cwd:    "/",
 		Args:   []string{"sh", "-c", "cat /proc/self/oom_score_adj"},
 		Env:    standardEnvironment,
 		Stdin:  nil,
 		Stdout: &stdout,
+		Stderr: new(strings.Builder),
 		Init:   true,
 	}
 	err = container.Run(&pconfig)
@@ -1074,13 +1080,14 @@ func TestHook(t *testing.T) {
 		cmd.WriteString("/" + hook + " ")
 	}
 
-	var stdout bytes.Buffer
+	var stdout strings.Builder
 	pconfig := libcontainer.Process{
 		Cwd:    "/",
 		Args:   []string{"sh", "-c", cmd.String()},
 		Env:    standardEnvironment,
 		Stdin:  nil,
 		Stdout: &stdout,
+		Stderr: new(strings.Builder),
 		Init:   true,
 	}
 	err = container.Run(&pconfig)
@@ -1185,7 +1192,7 @@ func TestRootfsPropagationSlaveMount(t *testing.T) {
 	ok(t, err)
 
 	// Run "cat /proc/self/mountinfo" in container and look at mount points.
-	var stdout2 bytes.Buffer
+	var stdout2 strings.Builder
 
 	stdinR2, stdinW2, err := os.Pipe()
 	ok(t, err)
@@ -1196,6 +1203,7 @@ func TestRootfsPropagationSlaveMount(t *testing.T) {
 		Env:    standardEnvironment,
 		Stdin:  stdinR2,
 		Stdout: &stdout2,
+		Stderr: new(strings.Builder),
 	}
 
 	err = container.Run(pconfig2)
@@ -1296,7 +1304,7 @@ func TestRootfsPropagationSharedMount(t *testing.T) {
 	dir2cont = filepath.Join(dir1cont, filepath.Base(dir2host))
 
 	// Mount something in container and see if it is visible on host.
-	var stdout2 bytes.Buffer
+	var stdout2 strings.Builder
 
 	stdinR2, stdinW2, err := os.Pipe()
 	ok(t, err)
@@ -1307,6 +1315,7 @@ func TestRootfsPropagationSharedMount(t *testing.T) {
 		Env:          standardEnvironment,
 		Stdin:        stdinR2,
 		Stdout:       &stdout2,
+		Stderr:       new(strings.Builder),
 		Capabilities: &configs.Capabilities{},
 	}
 
@@ -1330,7 +1339,7 @@ func TestRootfsPropagationSharedMount(t *testing.T) {
 
 	// Check if mount is visible on host or not.
 	out, err := exec.Command("findmnt", "-n", "-f", "-oTARGET", dir2host).CombinedOutput()
-	outtrim := string(bytes.TrimSpace(out))
+	outtrim := strings.TrimSpace(string(out))
 	if err != nil {
 		t.Logf("findmnt error %q: %q", err, outtrim)
 	}
@@ -1350,7 +1359,7 @@ func TestPIDHost(t *testing.T) {
 
 	config := newTemplateConfig(t, nil)
 	config.Namespaces.Remove(configs.NEWPID)
-	buffers := runContainerOk(t, config, "readlink", "/proc/self/ns/pid")
+	buffers := runContainerOk(t, config, "readlink", "-v", "/proc/self/ns/pid")
 
 	if actual := strings.Trim(buffers.Stdout.String(), "\n"); actual != l {
 		t.Fatalf("ipc link not equal to host link %q %q", actual, l)
@@ -1490,6 +1499,7 @@ func TestInitJoinPID(t *testing.T) {
 		Args:   []string{"ps"},
 		Env:    standardEnvironment,
 		Stdout: buffers.Stdout,
+		Stderr: new(strings.Builder),
 	}
 	err = container1.Run(ps)
 	ok(t, err)
@@ -1616,12 +1626,13 @@ func TestTmpfsCopyUp(t *testing.T) {
 	ok(t, err)
 	defer destroyContainer(container)
 
-	var stdout bytes.Buffer
+	var stdout strings.Builder
 	pconfig := libcontainer.Process{
 		Args:   []string{"ls", "/etc/passwd"},
 		Env:    standardEnvironment,
 		Stdin:  nil,
 		Stdout: &stdout,
+		Stderr: new(strings.Builder),
 		Init:   true,
 	}
 	err = container.Run(&pconfig)
@@ -1651,7 +1662,7 @@ func TestCGROUPPrivate(t *testing.T) {
 
 	config := newTemplateConfig(t, nil)
 	config.Namespaces.Add(configs.NEWCGROUP, "")
-	buffers := runContainerOk(t, config, "readlink", "/proc/self/ns/cgroup")
+	buffers := runContainerOk(t, config, "readlink", "-v", "/proc/self/ns/cgroup")
 
 	if actual := strings.Trim(buffers.Stdout.String(), "\n"); actual == l {
 		t.Fatalf("cgroup link should be private to the container but equals host %q %q", actual, l)
@@ -1670,7 +1681,7 @@ func TestCGROUPHost(t *testing.T) {
 	ok(t, err)
 
 	config := newTemplateConfig(t, nil)
-	buffers := runContainerOk(t, config, "readlink", "/proc/self/ns/cgroup")
+	buffers := runContainerOk(t, config, "readlink", "-v", "/proc/self/ns/cgroup")
 
 	if actual := strings.Trim(buffers.Stdout.String(), "\n"); actual != l {
 		t.Fatalf("cgroup link not equal to host link %q %q", actual, l)
@@ -1806,7 +1817,7 @@ func TestBindMountAndUser(t *testing.T) {
 	ok(t, err)
 	defer destroyContainer(container)
 
-	var stdout bytes.Buffer
+	var stdout strings.Builder
 
 	pconfig := libcontainer.Process{
 		Cwd:    "/",

libcontainer/integration/execin_test.go

@@ -1,7 +1,6 @@
 package integration
 
 import (
-	"bytes"
 	"fmt"
 	"io"
 	"os"
@@ -49,7 +48,6 @@ func TestExecIn(t *testing.T) {
 		Cwd:    "/",
 		Args:   []string{"ps"},
 		Env:    standardEnvironment,
-		Stdin:  buffers.Stdin,
 		Stdout: buffers.Stdout,
 		Stderr: buffers.Stderr,
 	}
@@ -107,7 +105,6 @@ func testExecInRlimit(t *testing.T, userns bool) {
 		Cwd:    "/",
 		Args:   []string{"/bin/sh", "-c", "ulimit -n"},
 		Env:    standardEnvironment,
-		Stdin:  buffers.Stdin,
 		Stdout: buffers.Stdout,
 		Stderr: buffers.Stderr,
 		Rlimits: []configs.Rlimit{
@@ -153,13 +150,14 @@ func TestExecInAdditionalGroups(t *testing.T) {
 	defer stdinW.Close()
 	ok(t, err)
 
-	var stdout bytes.Buffer
+	var stdout strings.Builder
 	pconfig := libcontainer.Process{
 		Cwd:              "/",
 		Args:             []string{"sh", "-c", "id", "-Gn"},
 		Env:              standardEnvironment,
 		Stdin:            nil,
 		Stdout:           &stdout,
+		Stderr:           new(strings.Builder),
 		AdditionalGroups: []int{4444, 87654},
 	}
 	err = container.Run(&pconfig)
@@ -265,7 +263,7 @@ func TestExecInTTY(t *testing.T) {
 	// Repeat to increase chances to catch a race; see
 	// https://github.com/opencontainers/runc/issues/2425.
 	for range 300 {
-		var stdout bytes.Buffer
+		var stdout strings.Builder
 
 		parent, child, err := utils.NewSockPair("console")
 		ok(t, err)
@@ -356,7 +354,6 @@ func TestExecInEnvironment(t *testing.T) {
 		Cwd:    "/",
 		Args:   []string{"/bin/env"},
 		Env:    execEnv,
-		Stdin:  buffers.Stdin,
 		Stdout: buffers.Stdout,
 		Stderr: buffers.Stderr,
 	}
@@ -412,7 +409,7 @@ func TestExecinPassExtraFiles(t *testing.T) {
 	defer stdinW.Close()
 	ok(t, err)
 
-	var stdout bytes.Buffer
+	var stdout strings.Builder
 	pipeout1, pipein1, err := os.Pipe()
 	ok(t, err)
 	pipeout2, pipein2, err := os.Pipe()
@@ -482,7 +479,6 @@ func TestExecInOomScoreAdj(t *testing.T) {
 		Cwd:    "/",
 		Args:   []string{"/bin/sh", "-c", "cat /proc/self/oom_score_adj"},
 		Env:    standardEnvironment,
-		Stdin:  buffers.Stdin,
 		Stdout: buffers.Stdout,
 		Stderr: buffers.Stderr,
 	}
@@ -533,12 +529,12 @@ func TestExecInUserns(t *testing.T) {
 	buffers := newStdBuffers()
 	process2 := &libcontainer.Process{
 		Cwd:  "/",
-		Args: []string{"readlink", "/proc/self/ns/user"},
+		Args: []string{"readlink", "-v", "/proc/self/ns/user"},
 		Env: []string{
 			"PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin",
 		},
 		Stdout: buffers.Stdout,
-		Stderr: os.Stderr,
+		Stderr: new(strings.Builder),
 	}
 	err = container.Run(process2)
 	ok(t, err)

libcontainer/integration/seccomp_test.go

@@ -40,7 +40,6 @@ func TestSeccompDenySyslogWithErrno(t *testing.T) {
 		Cwd:    "/",
 		Args:   []string{"dmesg"},
 		Env:    standardEnvironment,
-		Stdin:  buffers.Stdin,
 		Stdout: buffers.Stdout,
 		Stderr: buffers.Stderr,
 		Init:   true,
@@ -88,7 +87,6 @@ func TestSeccompDenySyslog(t *testing.T) {
 		Cwd:    "/",
 		Args:   []string{"dmesg"},
 		Env:    standardEnvironment,
-		Stdin:  buffers.Stdin,
 		Stdout: buffers.Stdout,
 		Stderr: buffers.Stderr,
 		Init:   true,
@@ -143,7 +141,6 @@ func TestSeccompPermitWriteConditional(t *testing.T) {
 		Cwd:    "/",
 		Args:   []string{"busybox", "ls", "/"},
 		Env:    standardEnvironment,
-		Stdin:  buffers.Stdin,
 		Stdout: buffers.Stdout,
 		Stderr: buffers.Stderr,
 		Init:   true,
@@ -195,7 +192,6 @@ func TestSeccompDenyWriteConditional(t *testing.T) {
 		Cwd:    "/",
 		Args:   []string{"busybox", "ls", "does_not_exist"},
 		Env:    standardEnvironment,
-		Stdin:  buffers.Stdin,
 		Stdout: buffers.Stdout,
 		Stderr: buffers.Stderr,
 		Init:   true,

libcontainer/integration/update_test.go

@@ -1,7 +1,6 @@
 package integration
 
 import (
-	"bytes"
 	"os"
 	"strings"
 	"testing"
@@ -40,7 +39,7 @@ func testUpdateDevices(t *testing.T, systemd bool) {
 	}()
 	ok(t, err)
 
-	var buf bytes.Buffer
+	var buf strings.Builder
 	devCheck := &libcontainer.Process{
 		Cwd:    "/",
 		Args:   []string{"/bin/sh", "-c", "echo > /dev/full; cat /dev/null; true"},

libcontainer/integration/utils_test.go

@@ -1,7 +1,6 @@
 package integration
 
 import (
-	"bytes"
 	"errors"
 	"fmt"
 	"os"
@@ -55,16 +54,14 @@ func ptrInt(v int) *int {
 
 func newStdBuffers() *stdBuffers {
 	return &stdBuffers{
-		Stdin:  bytes.NewBuffer(nil),
-		Stdout: bytes.NewBuffer(nil),
-		Stderr: bytes.NewBuffer(nil),
+		Stdout: new(strings.Builder),
+		Stderr: new(strings.Builder),
 	}
 }
 
 type stdBuffers struct {
-	Stdin  *bytes.Buffer
-	Stdout *bytes.Buffer
-	Stderr *bytes.Buffer
+	Stdout *strings.Builder
+	Stderr *strings.Builder
 }
 
 func (b *stdBuffers) String() string {
@@ -88,13 +85,12 @@ func ok(t testing.TB, err error) {
 
 func waitProcess(p *libcontainer.Process, t testing.TB) {
 	t.Helper()
-	status, err := p.Wait()
+	_, err := p.Wait()
 	if err != nil {
-		t.Fatalf("unexpected error: %v", err)
-	}
-
-	if !status.Success() {
-		t.Fatalf("unexpected status: %v", status)
+		if stderr, ok := p.Stderr.(fmt.Stringer); ok {
+			err = fmt.Errorf("%w; stderr:\n%s", err, stderr)
+		}
+		t.Fatalf("command failed: %v", err)
 	}
 }
 
@@ -188,7 +184,6 @@ func runContainer(t testing.TB, config *configs.Config, args ...string) (buffers
 		Cwd:    "/",
 		Args:   args,
 		Env:    standardEnvironment,
-		Stdin:  buffers.Stdin,
 		Stdout: buffers.Stdout,
 		Stderr: buffers.Stderr,
 		Init:   true,