Merge pull request #3034 from vrothberg/top-fallback

top: fallback to execing ps(1)

7 files changed, 126 insertions, 11 deletions

diff --git a/cmd/podman/top.go b/cmd/podman/top.go
index 2e0a22d92..8583eccb5 100644
--- a/cmd/podman/top.go
+++ b/cmd/podman/top.go
@@ -33,7 +33,7 @@ var (
 %s`, getDescriptorString())
 
 	_topCommand = &cobra.Command{
-		Use:   "top [flags] CONTAINER [FORMAT-DESCRIPTORS]",
+		Use:   "top [flags] CONTAINER [FORMAT-DESCRIPTORS|ARGS]",
 		Short: "Display the running processes of a container",
 		Long:  topDescription,
 		RunE: func(cmd *cobra.Command, args []string) error {
@@ -42,9 +42,11 @@ var (
 			topCommand.Remote = remoteclient
 			return topCmd(&topCommand)
 		},
+		Args: cobra.ArbitraryArgs,
 		Example: `podman top ctrID
-  podman top --latest
-  podman top ctrID pid seccomp args %C`,
+podman top --latest
+podman top ctrID pid seccomp args %C
+podman top ctrID -eo user,pid,comm`,
 	}
 )
 
@@ -53,6 +55,7 @@ func init() {
 	topCommand.SetHelpTemplate(HelpTemplate())
 	topCommand.SetUsageTemplate(UsageTemplate())
 	flags := topCommand.Flags()
+	flags.SetInterspersed(false)
 	flags.BoolVar(&topCommand.ListDescriptors, "list-descriptors", false, "")
 	flags.MarkHidden("list-descriptors")
 	flags.BoolVarP(&topCommand.Latest, "latest", "l", false, "Act on the latest container podman is aware of")
diff --git a/docs/podman-pod-top.1.md b/docs/podman-pod-top.1.md
index b235a70ad..fbab6bc09 100644
--- a/docs/podman-pod-top.1.md
+++ b/docs/podman-pod-top.1.md
@@ -7,7 +7,7 @@ podman\-pod\-top - Display the running processes of containers in a pod
 **podman pod top** [*options*] *pod* [*format-descriptors*]
 
 ## DESCRIPTION
-Display the running process of containers in a pod. The *format-descriptors* are ps (1) compatible AIX format descriptors but extended to print additional information, such as the seccomp mode or the effective capabilities of a given process.
+Display the running processes of containers in a pod. The *format-descriptors* are ps (1) compatible AIX format descriptors but extended to print additional information, such as the seccomp mode or the effective capabilities of a given process. The descriptors can either be passed as separated arguments or as a single comma-separated argument. Note that you can also specify options and or flags of ps(1); in this case, Podman will fallback to executing ps with the specified arguments and flags in the container.
 
 ## OPTIONS
 
diff --git a/docs/podman-top.1.md b/docs/podman-top.1.md
index 52d1238ef..74175b753 100644
--- a/docs/podman-top.1.md
+++ b/docs/podman-top.1.md
@@ -7,7 +7,7 @@ podman\-top - Display the running processes of a container
 **podman top** [*options*] *container* [*format-descriptors*]
 
 ## DESCRIPTION
-Display the running process of the container. The *format-descriptors* are ps (1) compatible AIX format descriptors but extended to print additional information, such as the seccomp mode or the effective capabilities of a given process.
+Display the running processes of the container. The *format-descriptors* are ps (1) compatible AIX format descriptors but extended to print additional information, such as the seccomp mode or the effective capabilities of a given process. The descriptors can either be passed as separated arguments or as a single comma-separated argument. Note that you can also specify options and or flags of ps(1); in this case, Podman will fallback to executing ps with the specified arguments and flags in the container.
 
 ## OPTIONS
 
@@ -83,12 +83,20 @@ root   8     1      0.000   11.386886562s   pts/0   0s     vi
 The output can be controlled by specifying format descriptors as arguments after the container:
 
 ```
-$ sudo ./bin/podman top -l pid seccomp args %C
+$ podman top -l pid seccomp args %C
 PID   SECCOMP   COMMAND     %CPU
 1     filter    sh          0.000
 8     filter    vi /etc/    0.000
 ```
 
+Podman will fallback to executing ps(1) in the container if an unknown descriptor is specified.
+
+```
+$ podman top -l -- aux
+USER   PID   PPID   %CPU    ELAPSED             TTY   TIME   COMMAND
+root   1     0      0.000   1h2m12.497061672s   ?     0s     sleep 100000
+```
+
 ## SEE ALSO
 podman(1), ps(1), seccomp(2), proc(5), capabilities(7)
 
diff --git a/libpod/container_top_linux.go b/libpod/container_top_linux.go
index b370495fe..392a7029e 100644
--- a/libpod/container_top_linux.go
+++ b/libpod/container_top_linux.go
@@ -20,14 +20,24 @@ func (c *Container) Top(descriptors []string) ([]string, error) {
 	if conStat != ContainerStateRunning {
 		return nil, errors.Errorf("top can only be used on running containers")
 	}
-	return c.GetContainerPidInformation(descriptors)
+
+	// Also support comma-separated input.
+	psgoDescriptors := []string{}
+	for _, d := range descriptors {
+		for _, s := range strings.Split(d, ",") {
+			if s != "" {
+				psgoDescriptors = append(psgoDescriptors, s)
+			}
+		}
+	}
+	return c.GetContainerPidInformation(psgoDescriptors)
 }
 
 // GetContainerPidInformation returns process-related data of all processes in
 // the container.  The output data can be controlled via the `descriptors`
 // argument which expects format descriptors and supports all AIXformat
 // descriptors of ps (1) plus some additional ones to for instance inspect the
-// set of effective capabilities.  Eeach element in the returned string slice
+// set of effective capabilities.  Each element in the returned string slice
 // is a tab-separated string.
 //
 // For more details, please refer to github.com/containers/psgo.
diff --git a/pkg/adapter/containers.go b/pkg/adapter/containers.go
index d575bc9b0..0721af773 100644
--- a/pkg/adapter/containers.go
+++ b/pkg/adapter/containers.go
@@ -3,6 +3,7 @@
 package adapter
 
 import (
+	"bufio"
 	"context"
 	"fmt"
 	"io/ioutil"
@@ -19,6 +20,7 @@ import (
 	"github.com/containers/libpod/libpod"
 	"github.com/containers/libpod/pkg/adapter/shortcuts"
 	"github.com/containers/libpod/pkg/systemdgen"
+	"github.com/containers/psgo"
 	"github.com/containers/storage"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
@@ -822,7 +824,69 @@ func (r *LocalRuntime) Top(cli *cliconfig.TopValues) ([]string, error) {
 	if err != nil {
 		return nil, errors.Wrapf(err, "unable to lookup requested container")
 	}
-	return container.Top(descriptors)
+
+	output, psgoErr := container.Top(descriptors)
+	if psgoErr == nil {
+		return output, nil
+	}
+
+	// If we encountered an ErrUnknownDescriptor error, fallback to executing
+	// ps(1). This ensures backwards compatibility to users depending on ps(1)
+	// and makes sure we're ~compatible with docker.
+	if errors.Cause(psgoErr) != psgo.ErrUnknownDescriptor {
+		return nil, psgoErr
+	}
+
+	output, err = r.execPS(container, descriptors)
+	if err != nil {
+		// Note: return psgoErr to guide users into using the AIX descriptors
+		// instead of using ps(1).
+		return nil, psgoErr
+	}
+
+	// Trick: filter the ps command from the output instead of
+	// checking/requiring PIDs in the output.
+	filtered := []string{}
+	cmd := strings.Join(descriptors, " ")
+	for _, line := range output {
+		if !strings.Contains(line, cmd) {
+			filtered = append(filtered, line)
+		}
+	}
+
+	return filtered, nil
+}
+
+func (r *LocalRuntime) execPS(c *libpod.Container, args []string) ([]string, error) {
+	rPipe, wPipe, err := os.Pipe()
+	if err != nil {
+		return nil, err
+	}
+	defer wPipe.Close()
+	defer rPipe.Close()
+
+	streams := new(libpod.AttachStreams)
+	streams.OutputStream = wPipe
+	streams.ErrorStream = wPipe
+	streams.InputStream = os.Stdin
+	streams.AttachOutput = true
+	streams.AttachError = true
+	streams.AttachInput = true
+
+	psOutput := []string{}
+	go func() {
+		scanner := bufio.NewScanner(rPipe)
+		for scanner.Scan() {
+			psOutput = append(psOutput, scanner.Text())
+		}
+	}()
+
+	cmd := append([]string{"ps"}, args...)
+	if err := c.Exec(false, false, []string{}, cmd, "", "", streams, 0); err != nil {
+		return nil, err
+	}
+
+	return psOutput, nil
 }
 
 // Prune removes stopped containers
diff --git a/test/e2e/pod_top_test.go b/test/e2e/pod_top_test.go
index 964ee075f..420e4aca9 100644
--- a/test/e2e/pod_top_test.go
+++ b/test/e2e/pod_top_test.go
@@ -93,7 +93,11 @@ var _ = Describe("Podman top", func() {
 		session.WaitWithDefaultTimeout()
 		Expect(session.ExitCode()).To(Equal(0))
 
-		result := podmanTest.Podman([]string{"pod", "top", podid, "invalid"})
+		// We need to pass -eo to force executing ps in the Alpine container.
+		// Alpines stripped down ps(1) is accepting any kind of weird input in
+		// contrast to others, such that a `ps invalid` will silently ignore
+		// the wrong input and still print the -ef output instead.
+		result := podmanTest.Podman([]string{"pod", "top", podid, "-eo", "invalid"})
 		result.WaitWithDefaultTimeout()
 		Expect(result.ExitCode()).To(Equal(125))
 	})
diff --git a/test/e2e/top_test.go b/test/e2e/top_test.go
index 2d3a5629c..4c2cdb7b5 100644
--- a/test/e2e/top_test.go
+++ b/test/e2e/top_test.go
@@ -87,13 +87,39 @@ var _ = Describe("Podman top", func() {
 		Expect(len(result.OutputToStringArray())).To(BeNumerically(">", 1))
 	})
 
+	It("podman top with ps(1) options", func() {
+		session := podmanTest.Podman([]string{"run", "-d", ALPINE, "top", "-d", "2"})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(0))
+
+		result := podmanTest.Podman([]string{"top", session.OutputToString(), "aux"})
+		result.WaitWithDefaultTimeout()
+		Expect(result.ExitCode()).To(Equal(0))
+		Expect(len(result.OutputToStringArray())).To(BeNumerically(">", 1))
+	})
+
+	It("podman top with comma-separated options", func() {
+		session := podmanTest.Podman([]string{"run", "-d", ALPINE, "top", "-d", "2"})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(0))
+
+		result := podmanTest.Podman([]string{"top", session.OutputToString(), "user,pid,comm"})
+		result.WaitWithDefaultTimeout()
+		Expect(result.ExitCode()).To(Equal(0))
+		Expect(len(result.OutputToStringArray())).To(BeNumerically(">", 1))
+	})
+
 	It("podman top on container invalid options", func() {
 		top := podmanTest.RunTopContainer("")
 		top.WaitWithDefaultTimeout()
 		Expect(top.ExitCode()).To(Equal(0))
 		cid := top.OutputToString()
 
-		result := podmanTest.Podman([]string{"top", cid, "invalid"})
+		// We need to pass -eo to force executing ps in the Alpine container.
+		// Alpines stripped down ps(1) is accepting any kind of weird input in
+		// contrast to others, such that a `ps invalid` will silently ignore
+		// the wrong input and still print the -ef output instead.
+		result := podmanTest.Podman([]string{"top", cid, "-eo", "invalid"})
 		result.WaitWithDefaultTimeout()
 		Expect(result.ExitCode()).To(Equal(125))
 	})