Merge pull request #1250 from YaoZengzeng/mux

bugfix: disable mux stdout and stderr if backend is not http

ctrd/container.go

@@ -50,7 +50,7 @@ func (c *Client) ExecContainer(ctx context.Context, process *Process) error {
 		pStderr io.Writer = process.IO.Stderr
 	)
 
-	if !process.P.Terminal {
+	if !process.P.Terminal && !process.IO.MuxDisabled {
 		pStdout = stdcopy.NewStdWriter(pStdout, stdcopy.Stdout)
 		pStderr = stdcopy.NewStdWriter(pStderr, stdcopy.Stderr)
 	}

daemon/containerio/container_io.go

@@ -37,15 +37,22 @@ type IO struct {
 	Stdout *ContainerIO
 	Stderr *ContainerIO
 	Stdin  *ContainerIO
+
+	// For IO backend like http, we need to mux stdout & stderr
+	// if terminal is disabled.
+	// But for other IO backend, it is not necessary.
+	// So we should make it configurable.
+	MuxDisabled bool
 }
 
 // NewIO creates the container's ios of stdout, stderr, stdin.
 func NewIO(opt *Option) *IO {
 	backends := createBackend(opt)
 
 	i := &IO{
-		Stdout: create(opt, stdout, backends),
-		Stderr: create(opt, stderr, backends),
+		Stdout:      create(opt, stdout, backends),
+		Stderr:      create(opt, stderr, backends),
+		MuxDisabled: opt.muxDisabled,
 	}
 
 	if opt.stdin {

daemon/containerio/options.go

@@ -13,6 +13,7 @@ type Option struct {
 	id            string
 	rootDir       string
 	stdin         bool
+	muxDisabled   bool
 	backends      map[string]struct{}
 	hijack        http.Hijacker
 	hijackUpgrade bool
@@ -54,6 +55,13 @@ func WithStdin(stdin bool) func(*Option) {
 	}
 }
 
+// WithMuxDisabled specified whether mux stdout & stderr of container IO.
+func WithMuxDisabled(muxDisabled bool) func(*Option) {
+	return func(opt *Option) {
+		opt.muxDisabled = muxDisabled
+	}
+}
+
 // WithDiscard specified the discard backend.
 func WithDiscard() func(*Option) {
 	return func(opt *Option) {

daemon/mgr/container.go

@@ -1613,6 +1613,7 @@ func (mgr *ContainerManager) openExecIO(id string, attach *AttachConfig) (*conta
 	options := []func(*containerio.Option){
 		containerio.WithID(id),
 		containerio.WithStdin(attach.Stdin),
+		containerio.WithMuxDisabled(attach.MuxDisabled),
 	}
 
 	if attach != nil {

daemon/mgr/container_types.go

@@ -61,6 +61,12 @@ type AttachConfig struct {
 	Stdout bool
 	Stderr bool
 
+	// For IO backend like http, we need to mux stdout & stderr
+	// if terminal is disabled.
+	// But for other IO backend, it is not necessary.
+	// So we should make it configurable.
+	MuxDisabled bool
+
 	// Attach using http.
 	Hijack  http.Hijacker
 	Upgrade bool

daemon/mgr/cri.go

@@ -709,9 +709,10 @@ func (c *CriManager) ExecSync(ctx context.Context, r *runtime.ExecSyncRequest) (
 	var output bytes.Buffer
 	startConfig := &apitypes.ExecStartConfig{}
 	attachConfig := &AttachConfig{
-		Stdout:    true,
-		Stderr:    true,
-		MemBuffer: &output,
+		Stdout:      true,
+		Stderr:      true,
+		MemBuffer:   &output,
+		MuxDisabled: true,
 	}
 
 	err = c.ContainerMgr.StartExec(ctx, execid, startConfig, attachConfig)

daemon/mgr/cri_stream.go

@@ -51,7 +51,8 @@ func (s *streamRuntime) Exec(containerID string, cmd []string, streamOpts *remot
 
 	startConfig := &apitypes.ExecStartConfig{}
 	attachConfig := &AttachConfig{
-		Streams: streams,
+		Streams:     streams,
+		MuxDisabled: true,
 	}
 
 	err = s.containerMgr.StartExec(ctx, execid, startConfig, attachConfig)

hack/cri-test/test-cri.sh

@@ -26,7 +26,7 @@ POUCH_SOCK="/var/run/pouchcri.sock"
 CRI_FOCUS=${CRI_FOCUS:-}
 
 # CRI_SKIP skips the test to skip.
-CRI_SKIP=${CRI_SKIP:-"RunAsUserName|seccomp localhost|should error on create with wrong options|runtime should support RunAsUser|should support safe sysctls|runtime should support exec|runtime should support HostPID|runtime should support execSync|should support unsafe sysctls"}
+CRI_SKIP=${CRI_SKIP:-"RunAsUserName|seccomp localhost|should error on create with wrong options"}
 # REPORT_DIR is the the directory to store test logs.
 REPORT_DIR=${REPORT_DIR:-"/tmp/test-cri"}