Merge pull request #37131 from kolyshkin/top-opt

Optimize ContainerTop() aka docker top

daemon/daemon_linux_test.go

@@ -229,6 +229,10 @@ func checkMounted(t *testing.T, p string, expect bool) {
 }
 
 func TestRootMountCleanup(t *testing.T) {
+	if os.Getuid() != 0 {
+		t.Skip("root required")
+	}
+
 	t.Parallel()
 
 	testRoot, err := ioutil.TempDir("", t.Name())

daemon/daemon_test.go

@@ -153,6 +153,10 @@ func TestValidContainerNames(t *testing.T) {
 }
 
 func TestContainerInitDNS(t *testing.T) {
+	if os.Getuid() != 0 {
+		t.Skip("root required") // for chown
+	}
+
 	tmp, err := ioutil.TempDir("", "docker-container-test-")
 	if err != nil {
 		t.Fatal(err)

daemon/reload_test.go

@@ -1,6 +1,7 @@
 package daemon // import "github.com/docker/docker/daemon"
 
 import (
+	"os"
 	"reflect"
 	"sort"
 	"testing"
@@ -499,6 +500,9 @@ func TestDaemonDiscoveryReloadOnlyClusterAdvertise(t *testing.T) {
 }
 
 func TestDaemonReloadNetworkDiagnosticPort(t *testing.T) {
+	if os.Getuid() != 0 {
+		t.Skip("root required")
+	}
 	daemon := &Daemon{
 		imageService: images.NewImageService(images.ImageServiceConfig{}),
 	}

daemon/top_unix.go

@@ -3,6 +3,7 @@
 package daemon // import "github.com/docker/docker/daemon"
 
 import (
+	"bytes"
 	"context"
 	"fmt"
 	"os/exec"
@@ -11,6 +12,8 @@ import (
 	"strings"
 
 	"github.com/docker/docker/api/types/container"
+	"github.com/docker/docker/errdefs"
+	"github.com/pkg/errors"
 )
 
 func validatePSArgs(psArgs string) error {
@@ -70,6 +73,7 @@ func parsePSOutput(output []byte, procs []uint32) (*container.ContainerTopOKBody
 	for i, name := range procList.Titles {
 		if name == "PID" {
 			pidIndex = i
+			break
 		}
 	}
 	if pidIndex == -1 {
@@ -112,6 +116,20 @@ func parsePSOutput(output []byte, procs []uint32) (*container.ContainerTopOKBody
 	return procList, nil
 }
 
+// psPidsArg converts a slice of PIDs to a string consisting
+// of comma-separated list of PIDs prepended by "-q".
+// For example, psPidsArg([]uint32{1,2,3}) returns "-q1,2,3".
+func psPidsArg(pids []uint32) string {
+	b := []byte{'-', 'q'}
+	for i, p := range pids {
+		b = strconv.AppendUint(b, uint64(p), 10)
+		if i < len(pids)-1 {
+			b = append(b, ',')
+		}
+	}
+	return string(b)
+}
+
 // ContainerTop lists the processes running inside of the given
 // container by calling ps with the given args, or with the flags
 // "-ef" if no args are given.  An error is returned if the container
@@ -144,9 +162,23 @@ func (daemon *Daemon) ContainerTop(name string, psArgs string) (*container.Conta
 		return nil, err
 	}
 
-	output, err := exec.Command("ps", strings.Split(psArgs, " ")...).Output()
+	args := strings.Split(psArgs, " ")
+	pids := psPidsArg(procs)
+	output, err := exec.Command("ps", append(args, pids)...).Output()
 	if err != nil {
-		return nil, fmt.Errorf("Error running ps: %v", err)
+		// some ps options (such as f) can't be used together with q,
+		// so retry without it
+		output, err = exec.Command("ps", args...).Output()
+		if err != nil {
+			if ee, ok := err.(*exec.ExitError); ok {
+				// first line of stderr shows why ps failed
+				line := bytes.SplitN(ee.Stderr, []byte{'\n'}, 2)
+				if len(line) > 0 && len(line[0]) > 0 {
+					err = errors.New(string(line[0]))
+				}
+			}
+			return nil, errdefs.System(errors.Wrap(err, "ps"))
+		}
 	}
 	procList, err := parsePSOutput(output, procs)
 	if err != nil {