Merge pull request moby#1743 from coryb/runc-started-chan

update go-runc mod, use runc Started chan

executor/runcexecutor/executor_linux.go

@@ -1,14 +1,9 @@
 package runcexecutor
 
 import (
-	"bufio"
 	"context"
-	"fmt"
 	"io"
-	"io/ioutil"
 	"os"
-	"strconv"
-	"strings"
 	"syscall"
 	"time"
 
@@ -28,40 +23,33 @@ func updateRuncFieldsForHostOS(runtime *runc.Runc) {
 }
 
 func (w *runcExecutor) run(ctx context.Context, id, bundle string, process executor.ProcessInfo) error {
-	return w.callWithIO(ctx, id, bundle, process, func(ctx context.Context, pidfile string, io runc.IO) error {
+	return w.callWithIO(ctx, id, bundle, process, func(ctx context.Context, started chan<- int, io runc.IO) error {
 		_, err := w.runc.Run(ctx, id, bundle, &runc.CreateOpts{
 			NoPivot: w.noPivot,
-			PidFile: pidfile,
+			Started: started,
 			IO:      io,
 		})
 		return err
 	})
 }
 
 func (w *runcExecutor) exec(ctx context.Context, id, bundle string, specsProcess *specs.Process, process executor.ProcessInfo) error {
-	return w.callWithIO(ctx, id, bundle, process, func(ctx context.Context, pidfile string, io runc.IO) error {
+	return w.callWithIO(ctx, id, bundle, process, func(ctx context.Context, started chan<- int, io runc.IO) error {
 		return w.runc.Exec(ctx, id, *specsProcess, &runc.ExecOpts{
-			PidFile: pidfile,
+			Started: started,
 			IO:      io,
 		})
 	})
 }
 
-type runcCall func(ctx context.Context, pidfile string, io runc.IO) error
+type runcCall func(ctx context.Context, started chan<- int, io runc.IO) error
 
 func (w *runcExecutor) callWithIO(ctx context.Context, id, bundle string, process executor.ProcessInfo, call runcCall) error {
 	ctx, cancel := context.WithCancel(ctx)
 	defer cancel()
 
-	pidfile, err := ioutil.TempFile(bundle, "*.pid")
-	if err != nil {
-		return errors.Wrap(err, "failed to create pidfile")
-	}
-	defer os.Remove(pidfile.Name())
-	pidfile.Close()
-
 	if !process.Meta.Tty {
-		return call(ctx, pidfile.Name(), &forwardIO{stdin: process.Stdin, stdout: process.Stdout, stderr: process.Stderr})
+		return call(ctx, nil, &forwardIO{stdin: process.Stdin, stdout: process.Stdout, stderr: process.Stderr})
 	}
 
 	ptm, ptsName, err := console.NewPty()
@@ -117,37 +105,26 @@ func (w *runcExecutor) callWithIO(ctx context.Context, id, bundle string, proces
 		})
 	}
 
+	started := make(chan int, 1)
+
 	eg.Go(func() error {
-		// need to poll until the pidfile has the pid written to it
-		pidfileCtx, timeout := context.WithTimeout(ctx, 10*time.Second)
+		startedCtx, timeout := context.WithTimeout(ctx, 10*time.Second)
 		defer timeout()
-
 		var runcProcess *os.Process
-		for {
-			st, err := os.Stat(pidfile.Name())
-			if err == nil && st.Size() > 0 {
-				pid, err := runc.ReadPidFile(pidfile.Name())
-				if err != nil {
-					return errors.Wrapf(err, "unable to read pid file: %s", pidfile.Name())
-				}
-				// pid will be for the process in process.Meta, not the parent runc process.
-				// We need to send SIGWINCH to the runc process, not the process.Meta process.
-				ppid, err := getppid(pid)
-				if err != nil {
-					return errors.Wrapf(err, "unable to find runc process (parent of %d)", pid)
-				}
-				runcProcess, err = os.FindProcess(ppid)
-				if err != nil {
-					return errors.Wrapf(err, "unable to find process for pid %d", ppid)
-				}
-				break
+		select {
+		case <-startedCtx.Done():
+			return errors.New("runc started message never received")
+		case pid, ok := <-started:
+			if !ok {
+				return errors.New("runc process failed to send pid")
 			}
-			select {
-			case <-pidfileCtx.Done():
-				return errors.New("pidfile never updated")
-			case <-time.After(100 * time.Microsecond):
+			runcProcess, err = os.FindProcess(pid)
+			if err != nil {
+				return errors.Wrapf(err, "unable to find runc process for pid %d", pid)
 			}
+			defer runcProcess.Release()
 		}
+
 		for {
 			select {
 			case <-ctx.Done():
@@ -179,24 +156,5 @@ func (w *runcExecutor) callWithIO(ctx context.Context, id, bundle string, proces
 		runcIO.stderr = pts
 	}
 
-	return call(ctx, pidfile.Name(), runcIO)
-}
-
-const PPidStatusPrefix = "PPid:\t"
-
-func getppid(pid int) (int, error) {
-	fh, err := os.Open(fmt.Sprintf("/proc/%d/status", pid))
-	if err != nil {
-		return -1, err
-	}
-
-	defer fh.Close()
-	scanner := bufio.NewScanner(fh)
-	for scanner.Scan() {
-		line := scanner.Text()
-		if strings.HasPrefix(line, PPidStatusPrefix) {
-			return strconv.Atoi(strings.TrimPrefix(line, PPidStatusPrefix))
-		}
-	}
-	return -1, errors.Errorf("PPid line not found in /proc/%d/status", pid)
+	return call(ctx, started, runcIO)
 }

go.mod

@@ -12,7 +12,7 @@ require (
 	github.com/containerd/containerd v1.4.1-0.20200903181227-d4e78200d6da
 	github.com/containerd/continuity v0.0.0-20200710164510-efbc4488d8fe
 	github.com/containerd/go-cni v1.0.1
-	github.com/containerd/go-runc v0.0.0-20201017060547-8c4be61c2e34
+	github.com/containerd/go-runc v0.0.0-20201020171139-16b287bc67d0
 	github.com/containerd/stargz-snapshotter v0.0.0-20200903042824-2ee75e91f8f9
 	github.com/containerd/typeurl v1.0.1
 	github.com/coreos/go-systemd/v22 v22.1.0

go.sum

@@ -127,8 +127,8 @@ github.com/containerd/go-cni v1.0.1/go.mod h1:+vUpYxKvAF72G9i1WoDOiPGRtQpqsNW/ZH
 github.com/containerd/go-runc v0.0.0-20180907222934-5a6d9f37cfa3/go.mod h1:IV7qH3hrUgRmyYrtgEeGWJfWbgcHL9CSRruz2Vqcph0=
 github.com/containerd/go-runc v0.0.0-20200220073739-7016d3ce2328 h1:PRTagVMbJcCezLcHXe8UJvR1oBzp2lG3CEumeFOLOds=
 github.com/containerd/go-runc v0.0.0-20200220073739-7016d3ce2328/go.mod h1:PpyHrqVs8FTi9vpyHwPwiNEGaACDxT/N/pLcvMSRA9g=
-github.com/containerd/go-runc v0.0.0-20201017060547-8c4be61c2e34 h1:jFRg/hwx0DPpcg23MNusAppCDJa5nndN3/RAxSblN58=
-github.com/containerd/go-runc v0.0.0-20201017060547-8c4be61c2e34/go.mod h1:1CDPys/h0SMZoki7dv3bPQ1wJWnmaeZIO026WPts2xM=
+github.com/containerd/go-runc v0.0.0-20201020171139-16b287bc67d0 h1:e+50zk22gvHLJKe8+d+xSMyA88PPQk/XfWuUw1BdnPA=
+github.com/containerd/go-runc v0.0.0-20201020171139-16b287bc67d0/go.mod h1:cNU0ZbCgCQVZK4lgG3P+9tn9/PaJNmoDXPpoJhDR+Ok=
 github.com/containerd/stargz-snapshotter v0.0.0-20200903042824-2ee75e91f8f9 h1:JEcj3rNCg0Ho5t9kiOa4LV/vaNXbRQ9l2PIRzz2LWCQ=
 github.com/containerd/stargz-snapshotter v0.0.0-20200903042824-2ee75e91f8f9/go.mod h1:f+gZLtYcuNQWxucWyfVEQXSBoGbXNpQ76+XWVMgp+34=
 github.com/containerd/ttrpc v0.0.0-20190828154514-0e0f228740de/go.mod h1:PvCDdDGpgqzQIzDW1TphrGLssLDZp2GuS+X5DkEJB8o=

vendor/modules.txt

@@ -117,7 +117,7 @@ github.com/containerd/continuity/sysx
 github.com/containerd/fifo
 # github.com/containerd/go-cni v1.0.1
 github.com/containerd/go-cni
-# github.com/containerd/go-runc v0.0.0-20201017060547-8c4be61c2e34
+# github.com/containerd/go-runc v0.0.0-20201020171139-16b287bc67d0
 github.com/containerd/go-runc
 # github.com/containerd/stargz-snapshotter v0.0.0-20200903042824-2ee75e91f8f9
 github.com/containerd/stargz-snapshotter/cache