Merge pull request #25891 from Microsoft/jjh/processlist

Windows: Docker top implementation

daemon/top_windows.go

@@ -2,31 +2,52 @@ package daemon
 
 import (
 	"errors"
-	"strconv"
+	"fmt"
+	"time"
 
 	"github.com/docker/docker/api/types"
+	"github.com/docker/go-units"
 )
 
-// ContainerTop is a minimal implementation on Windows currently.
-// TODO Windows: This needs more work, but needs platform API support.
-// All we can currently return (particularly in the case of Hyper-V containers)
-// is a PID and the command.
-func (daemon *Daemon) ContainerTop(containerID string, psArgs string) (*types.ContainerProcessList, error) {
-
-	// It's really not an equivalent to linux 'ps' on Windows
+// ContainerTop handles `docker top` client requests.
+// Future considerations:
+// -- Windows users are far more familiar with CPU% total.
+//    Further, users on Windows rarely see user/kernel CPU stats split.
+//    The kernel returns everything in terms of 100ns. To obtain
+//    CPU%, we could do something like docker stats does which takes two
+//    samples, subtract the difference and do the maths. Unfortunately this
+//    would slow the stat call down and require two kernel calls. So instead,
+//    we do something similar to linux and display the CPU as combined HH:MM:SS.mmm.
+// -- Perhaps we could add an argument to display "raw" stats
+// -- "Memory" is an extremely overloaded term in Windows. Hence we do what
+//    task manager does and use the private working set as the memory counter.
+//    We could return more info for those who really understand how memory
+//    management works in Windows if we introduced a "raw" stats (above).
+func (daemon *Daemon) ContainerTop(name string, psArgs string) (*types.ContainerProcessList, error) {
+	// It's not at all an equivalent to linux 'ps' on Windows
 	if psArgs != "" {
 		return nil, errors.New("Windows does not support arguments to top")
 	}
 
-	s, err := daemon.containerd.Summary(containerID)
+	container, err := daemon.GetContainer(name)
 	if err != nil {
 		return nil, err
 	}
 
+	s, err := daemon.containerd.Summary(container.ID)
+	if err != nil {
+		return nil, err
+	}
 	procList := &types.ContainerProcessList{}
+	procList.Titles = []string{"Name", "PID", "CPU", "Private Working Set"}
 
-	for _, v := range s {
-		procList.Titles = append(procList.Titles, strconv.Itoa(int(v.Pid))+" "+v.Command)
+	for _, j := range s {
+		d := time.Duration((j.KernelTime100ns + j.UserTime100ns) * 100) // Combined time in nanoseconds
+		procList.Processes = append(procList.Processes, []string{
+			j.ImageName,
+			fmt.Sprint(j.ProcessId),
+			fmt.Sprintf("%02d:%02d:%02d.%03d", int(d.Hours()), int(d.Minutes())%60, int(d.Seconds())%60, int(d.Nanoseconds()/1000000)%1000),
+			units.HumanSize(float64(j.MemoryWorkingSetPrivateBytes))})
 	}
 	return procList, nil
 }

integration-cli/benchmark_test.go

@@ -29,7 +29,7 @@ func (s *DockerSuite) BenchmarkConcurrentContainerActions(c *check.C) {
 				defer innerGroup.Done()
 				for i := 0; i < numIterations; i++ {
 					args := []string{"run", "-d", defaultSleepImage}
-					args = append(args, defaultSleepCommand...)
+					args = append(args, sleepCommandForDaemonPlatform()...)
 					out, _, err := dockerCmdWithError(args...)
 					if err != nil {
 						chErr <- fmt.Errorf(out)

integration-cli/docker_api_containers_test.go

@@ -881,7 +881,7 @@ func (s *DockerSuite) TestContainerApiStart(c *check.C) {
 	name := "testing-start"
 	config := map[string]interface{}{
 		"Image":     "busybox",
-		"Cmd":       append([]string{"/bin/sh", "-c"}, defaultSleepCommand...),
+		"Cmd":       append([]string{"/bin/sh", "-c"}, sleepCommandForDaemonPlatform()...),
 		"OpenStdin": true,
 	}
 
@@ -1117,7 +1117,7 @@ func (s *DockerSuite) TestContainerApiChunkedEncoding(c *check.C) {
 
 	config := map[string]interface{}{
 		"Image":     "busybox",
-		"Cmd":       append([]string{"/bin/sh", "-c"}, defaultSleepCommand...),
+		"Cmd":       append([]string{"/bin/sh", "-c"}, sleepCommandForDaemonPlatform()...),
 		"OpenStdin": true,
 	}
 	b, err := json.Marshal(config)

integration-cli/docker_cli_ps_test.go

@@ -640,7 +640,7 @@ func (s *DockerSuite) TestPsImageIDAfterUpdate(c *check.C) {
 	originalImageID, err := getIDByName(originalImageName)
 	c.Assert(err, checker.IsNil)
 
-	runCmd = exec.Command(dockerBinary, append([]string{"run", "-d", originalImageName}, defaultSleepCommand...)...)
+	runCmd = exec.Command(dockerBinary, append([]string{"run", "-d", originalImageName}, sleepCommandForDaemonPlatform()...)...)
 	out, _, err = runCommandWithOutput(runCmd)
 	c.Assert(err, checker.IsNil)
 	containerID := strings.TrimSpace(out)

integration-cli/docker_cli_service_update_test.go

@@ -14,7 +14,7 @@ func (s *DockerSwarmSuite) TestServiceUpdatePort(c *check.C) {
 	d := s.AddDaemon(c, true, true)
 
 	serviceName := "TestServiceUpdatePort"
-	serviceArgs := append([]string{"service", "create", "--name", serviceName, "-p", "8080:8081", defaultSleepImage}, defaultSleepCommand...)
+	serviceArgs := append([]string{"service", "create", "--name", serviceName, "-p", "8080:8081", defaultSleepImage}, sleepCommandForDaemonPlatform()...)
 
 	// Create a service with a port mapping of 8080:8081.
 	out, err := d.Cmd(serviceArgs...)

integration-cli/docker_cli_top_test.go

@@ -4,32 +4,62 @@ import (
 	"strings"
 
 	"github.com/docker/docker/pkg/integration/checker"
+	icmd "github.com/docker/docker/pkg/integration/cmd"
 	"github.com/go-check/check"
 )
 
 func (s *DockerSuite) TestTopMultipleArgs(c *check.C) {
-	testRequires(c, DaemonIsLinux)
-	out, _ := dockerCmd(c, "run", "-i", "-d", "busybox", "top")
+	out, _ := runSleepingContainer(c, "-d")
 	cleanedContainerID := strings.TrimSpace(out)
 
-	out, _ = dockerCmd(c, "top", cleanedContainerID, "-o", "pid")
-	c.Assert(out, checker.Contains, "PID", check.Commentf("did not see PID after top -o pid: %s", out))
+	var expected icmd.Expected
+	switch daemonPlatform {
+	case "windows":
+		expected = icmd.Expected{ExitCode: 1, Err: "Windows does not support arguments to top"}
+	default:
+		expected = icmd.Expected{Out: "PID"}
+	}
+	result := dockerCmdWithResult("top", cleanedContainerID, "-o", "pid")
+	c.Assert(result, icmd.Matches, expected)
 }
 
 func (s *DockerSuite) TestTopNonPrivileged(c *check.C) {
-	testRequires(c, DaemonIsLinux)
-	out, _ := dockerCmd(c, "run", "-i", "-d", "busybox", "top")
+	out, _ := runSleepingContainer(c, "-d")
 	cleanedContainerID := strings.TrimSpace(out)
 
 	out1, _ := dockerCmd(c, "top", cleanedContainerID)
 	out2, _ := dockerCmd(c, "top", cleanedContainerID)
 	dockerCmd(c, "kill", cleanedContainerID)
 
-	c.Assert(out1, checker.Contains, "top", check.Commentf("top should've listed `top` in the process list, but failed the first time"))
-	c.Assert(out2, checker.Contains, "top", check.Commentf("top should've listed `top` in the process list, but failed the second time"))
+	// Windows will list the name of the launched executable which in this case is busybox.exe, without the parameters.
+	// Linux will display the command executed in the container
+	var lookingFor string
+	if daemonPlatform == "windows" {
+		lookingFor = "busybox.exe"
+	} else {
+		lookingFor = "top"
+	}
+
+	c.Assert(out1, checker.Contains, lookingFor, check.Commentf("top should've listed `%s` in the process list, but failed the first time", lookingFor))
+	c.Assert(out2, checker.Contains, lookingFor, check.Commentf("top should've listed `%s` in the process list, but failed the second time", lookingFor))
+}
+
+// TestTopWindowsCoreProcesses validates that there are lines for the critical
+// processes which are found in a Windows container. Note Windows is architecturally
+// very different to Linux in this regard.
+func (s *DockerSuite) TestTopWindowsCoreProcesses(c *check.C) {
+	testRequires(c, DaemonIsWindows)
+	out, _ := runSleepingContainer(c, "-d")
+	cleanedContainerID := strings.TrimSpace(out)
+	out1, _ := dockerCmd(c, "top", cleanedContainerID)
+	lookingFor := []string{"smss.exe", "csrss.exe", "wininit.exe", "services.exe", "lsass.exe", "CExecSvc.exe"}
+	for i, s := range lookingFor {
+		c.Assert(out1, checker.Contains, s, check.Commentf("top should've listed `%s` in the process list, but failed. Test case %d", s, i))
+	}
 }
 
 func (s *DockerSuite) TestTopPrivileged(c *check.C) {
+	// Windows does not support --privileged
 	testRequires(c, DaemonIsLinux, NotUserNamespace)
 	out, _ := dockerCmd(c, "run", "--privileged", "-i", "-d", "busybox", "top")
 	cleanedContainerID := strings.TrimSpace(out)

integration-cli/docker_deprecated_api_v124_test.go

@@ -162,7 +162,7 @@ func (s *DockerSuite) TestDeprecatedPostContainersStartWithoutLinksInHostConfig(
 	// An alternate test could be written to validate the negative testing aspect of this
 	testRequires(c, DaemonIsLinux)
 	name := "test-host-config-links"
-	dockerCmd(c, append([]string{"create", "--name", name, "busybox"}, defaultSleepCommand...)...)
+	dockerCmd(c, append([]string{"create", "--name", name, "busybox"}, sleepCommandForDaemonPlatform()...)...)
 
 	hc := inspectFieldJSON(c, name, "HostConfig")
 	config := `{"HostConfig":` + hc + `}`

integration-cli/docker_utils.go

@@ -1435,7 +1435,7 @@ func runSleepingContainerInImage(c *check.C, image string, extraArgs ...string)
 	args := []string{"run", "-d"}
 	args = append(args, extraArgs...)
 	args = append(args, image)
-	args = append(args, defaultSleepCommand...)
+	args = append(args, sleepCommandForDaemonPlatform()...)
 	return dockerCmd(c, args...)
 }
 

integration-cli/test_vars.go

@@ -0,0 +1,11 @@
+package main
+
+// sleepCommandForDaemonPlatform is a helper function that determines what
+// the command is for a sleeping container based on the daemon platform.
+// The Windows busybox image does not have a `top` command.
+func sleepCommandForDaemonPlatform() []string {
+	if daemonPlatform == "windows" {
+		return []string{"sleep", "240"}
+	}
+	return []string{"top"}
+}

integration-cli/test_vars_unix.go

@@ -12,5 +12,3 @@ const (
 	// runs indefinitely while still being interruptible by a signal.
 	defaultSleepImage = "busybox"
 )
-
-var defaultSleepCommand = []string{"top"}

integration-cli/test_vars_windows.go

@@ -13,6 +13,3 @@ const (
 	// on `sleep` with a high duration.
 	defaultSleepImage = "busybox"
 )
-
-// TODO Windows: In TP5, decrease this sleep time, as performance will be better
-var defaultSleepCommand = []string{"sleep", "240"}

libcontainerd/client_windows.go

@@ -410,26 +410,23 @@ func (clnt *client) GetPidsForContainer(containerID string) ([]int, error) {
 // visible on the container host. However, libcontainerd does have
 // that information.
 func (clnt *client) Summary(containerID string) ([]Summary, error) {
-	var s []Summary
+
+	// Get the libcontainerd container object
 	clnt.lock(containerID)
 	defer clnt.unlock(containerID)
-	cont, err := clnt.getContainer(containerID)
+	container, err := clnt.getContainer(containerID)
 	if err != nil {
 		return nil, err
 	}
-
-	// Add the first process
-	s = append(s, Summary{
-		Pid:     cont.containerCommon.systemPid,
-		Command: cont.ociSpec.Process.Args[0]})
-	// And add all the exec'd processes
-	for _, p := range cont.processes {
-		s = append(s, Summary{
-			Pid:     p.processCommon.systemPid,
-			Command: p.commandLine})
+	p, err := container.hcsContainer.ProcessList()
+	if err != nil {
+		return nil, err
 	}
-	return s, nil
-
+	pl := make([]Summary, len(p))
+	for i := range p {
+		pl[i] = Summary(p[i])
+	}
+	return pl, nil
 }
 
 // UpdateResources updates resources for a running container.

libcontainerd/types_windows.go

@@ -1,6 +1,9 @@
 package libcontainerd
 
-import "github.com/docker/docker/libcontainerd/windowsoci"
+import (
+	"github.com/Microsoft/hcsshim"
+	"github.com/docker/docker/libcontainerd/windowsoci"
+)
 
 // Spec is the base configuration for the container.
 type Spec windowsoci.WindowsSpec
@@ -11,11 +14,8 @@ type Process windowsoci.Process
 // User specifies user information for the containers main process.
 type User windowsoci.User
 
-// Summary contains a container summary from containerd
-type Summary struct {
-	Pid     uint32
-	Command string
-}
+// Summary contains a ProcessList item from HCS to support `top`
+type Summary hcsshim.ProcessListItem
 
 // StateInfo contains description about the new state container has entered.
 type StateInfo struct {