From 910baf433fdbc6ad6d85cfeacb5ccc59b37aa634 Mon Sep 17 00:00:00 2001 From: baude Date: Mon, 5 Feb 2018 09:29:46 -0600 Subject: Deprecate --format for podman top Issue #169 describes a common failure when running podman top where if the commands inside the container container a space in them, podman will panic. This was occuring because we take the output from ps and attempt to format it nicely for output and things like JSON. Given that this cannot be predicted or dealt with programatically, the decision was made to deprecate the format switch and simply output what ps provides us. Migrated top integration tests to ginkgo. Resolves Issue: https://github.com/projectatomic/libpod/issues/169 Signed-off-by: baude Closes: #291 Approved by: rhatdan --- cmd/podman/top.go | 190 +-------------------------------------------------- docs/podman-top.1.md | 17 ----- test/e2e/top_test.go | 72 +++++++++++++++++++ test/podman_top.bats | 51 -------------- 4 files changed, 75 insertions(+), 255 deletions(-) create mode 100644 test/e2e/top_test.go delete mode 100644 test/podman_top.bats diff --git a/cmd/podman/top.go b/cmd/podman/top.go index 5b1dbd05c..6aff25a71 100644 --- a/cmd/podman/top.go +++ b/cmd/podman/top.go @@ -1,22 +1,16 @@ package main import ( - "encoding/json" "fmt" - "strings" "github.com/pkg/errors" - "github.com/projectatomic/libpod/cmd/podman/formats" "github.com/projectatomic/libpod/libpod" "github.com/urfave/cli" ) var ( topFlags = []cli.Flag{ - cli.StringFlag{ - Name: "format", - Usage: "Change the output to JSON", - }, LatestFlag, + LatestFlag, } topDescription = ` podman top @@ -38,14 +32,6 @@ var ( func topCmd(c *cli.Context) error { var container *libpod.Container var err error - doJSON := false - if c.IsSet("format") { - if strings.ToUpper(c.String("format")) == "JSON" { - doJSON = true - } else { - return errors.Errorf("only 'json' is supported for a format option") - } - } args := c.Args() var psArgs []string psOpts := []string{"-o", "uid,pid,ppid,c,stime,tname,time,cmd"} @@ -88,178 +74,8 @@ func topCmd(c *cli.Context) error { if err != nil { return err } - headers := getHeaders(results[0]) - format := genTopFormat(headers) - var out formats.Writer - psParams, err := psDataToPSParams(results[1:], headers) - if err != nil { - return errors.Wrap(err, "unable to convert ps data to proper structure") - } - if doJSON { - out = formats.JSONStructArray{Output: topToGeneric(psParams)} - } else { - out = formats.StdoutTemplateArray{Output: topToGeneric(psParams), Template: format, Fields: createTopHeaderMap(headers)} + for _, line := range results { + fmt.Println(line) } - formats.Writer(out).Out() return nil } - -func getHeaders(s string) []string { - var headers []string - tmpHeaders := strings.Fields(s) - for _, header := range tmpHeaders { - headers = append(headers, strings.Replace(header, "%", "", -1)) - } - return headers -} - -func genTopFormat(headers []string) string { - format := "table " - for _, header := range headers { - format = fmt.Sprintf("%s{{.%s}}\t", format, header) - } - return format -} - -// imagesToGeneric creates an empty array of interfaces for output -func topToGeneric(templParams []PSParams) (genericParams []interface{}) { - for _, v := range templParams { - genericParams = append(genericParams, interface{}(v)) - } - return -} - -// generate the header based on the template provided -func createTopHeaderMap(v []string) map[string]string { - values := make(map[string]string) - for _, key := range v { - value := key - if value == "CPU" { - value = "%CPU" - } else if value == "MEM" { - value = "%MEM" - } - values[key] = strings.ToUpper(splitCamelCase(value)) - } - return values -} - -// PSDataToParams converts a string array of data and its headers to an -// arra if PSParams -func psDataToPSParams(data []string, headers []string) ([]PSParams, error) { - var params []PSParams - for _, line := range data { - tmpMap := make(map[string]string) - tmpArray := strings.Fields(line) - if len(tmpArray) == 0 { - continue - } - for index, v := range tmpArray { - header := headers[index] - tmpMap[header] = v - } - jsonData, _ := json.Marshal(tmpMap) - var r PSParams - err := json.Unmarshal(jsonData, &r) - if err != nil { - return []PSParams{}, err - } - params = append(params, r) - } - return params, nil -} - -//PSParams is a list of options that the command line ps recognizes -type PSParams struct { - CPU string - MEM string - COMMAND string - BLOCKED string - START string - TIME string - C string - CAUGHT string - CGROUP string - CLSCLS string - CLS string - CMD string - CP string - DRS string - EGID string - EGROUP string - EIP string - ESP string - ELAPSED string - EUIDE string - USER string - F string - FGID string - FGROUP string - FUID string - FUSER string - GID string - GROUP string - IGNORED string - IPCNS string - LABEL string - STARTED string - SESSION string - LWP string - MACHINE string - MAJFLT string - MINFLT string - MNTNS string - NETNS string - NI string - NLWP string - OWNER string - PENDING string - PGID string - PGRP string - PID string - PIDNS string - POL string - PPID string - PRI string - PSR string - RGID string - RGROUP string - RSS string - RSZ string - RTPRIO string - RUID string - RUSER string - S string - SCH string - SEAT string - SESS string - P string - SGID string - SGROUP string - SID string - SIZE string - SLICE string - SPID string - STACKP string - STIME string - SUID string - SUPGID string - SUPGRP string - SUSER string - SVGID string - SZ string - TGID string - THCNT string - TID string - TTY string - TPGID string - TRS string - TT string - UID string - UNIT string - USERNS string - UTSNS string - UUNIT string - VSZ string - WCHAN string -} diff --git a/docs/podman-top.1.md b/docs/podman-top.1.md index eed15c584..1fca50bda 100644 --- a/docs/podman-top.1.md +++ b/docs/podman-top.1.md @@ -18,9 +18,6 @@ Display the running process of the container. ps-OPTION can be any of the option **--help, -h** Print usage statement -**--format** - Display the output in an alternate format. The only supported format is **JSON**. - **--latest, -l** Instead of providing the container name or ID, use the last created container. If you use methods other than Podman to run containers such as CRI-O, the last started container could be from either of those methods. @@ -42,20 +39,6 @@ root 4 bash system_u:system_r:container_t:s0:c429,c1016 root 0 vi system_u:system_r:container_t:s0:c429,c1016 # ``` -``` -# podman top --format=json f5a62a71b07b -o %cpu,%mem,command,blocked -[ - { - "CPU": "0.0", - "MEM": "0.0", - "COMMAND": "vi", - "BLOCKED": "0000000000000000", - "START": "", - "TIME": "", - "C": "", - "CAUGHT": "", - ... -``` ## SEE ALSO podman(1), ps(1) diff --git a/test/e2e/top_test.go b/test/e2e/top_test.go new file mode 100644 index 000000000..d4438293b --- /dev/null +++ b/test/e2e/top_test.go @@ -0,0 +1,72 @@ +package integration + +import ( + "os" + + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" +) + +var _ = Describe("Podman top", func() { + var ( + tempdir string + err error + podmanTest PodmanTest + ) + + BeforeEach(func() { + tempdir, err = CreateTempDirInTempDir() + if err != nil { + os.Exit(1) + } + podmanTest = PodmanCreate(tempdir) + podmanTest.RestoreAllArtifacts() + }) + + AfterEach(func() { + podmanTest.Cleanup() + + }) + + It("podman top without container name or id", func() { + result := podmanTest.Podman([]string{"top"}) + result.WaitWithDefaultTimeout() + Expect(result.ExitCode()).To(Equal(125)) + }) + + It("podman top on bogus container", func() { + result := podmanTest.Podman([]string{"top", "1234"}) + result.WaitWithDefaultTimeout() + Expect(result.ExitCode()).To(Equal(125)) + }) + + It("podman top on non-running container", func() { + _, ec, cid := podmanTest.RunLsContainer("") + Expect(ec).To(Equal(0)) + result := podmanTest.Podman([]string{"top", cid}) + result.WaitWithDefaultTimeout() + Expect(result.ExitCode()).To(Equal(125)) + }) + + It("podman top on container", func() { + session := podmanTest.Podman([]string{"run", "-d", ALPINE, "top", "-d", "2"}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + + result := podmanTest.Podman([]string{"top", "-l"}) + result.WaitWithDefaultTimeout() + Expect(result.ExitCode()).To(Equal(0)) + Expect(len(result.OutputToStringArray())).To(BeNumerically(">", 1)) + }) + + It("podman top on non-running container", 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(), "-o", "fuser,f,comm,label"}) + result.WaitWithDefaultTimeout() + Expect(result.ExitCode()).To(Equal(0)) + Expect(len(result.OutputToStringArray())).To(BeNumerically(">", 1)) + }) +}) diff --git a/test/podman_top.bats b/test/podman_top.bats deleted file mode 100644 index cfa037aa6..000000000 --- a/test/podman_top.bats +++ /dev/null @@ -1,51 +0,0 @@ -#!/usr/bin/env bats - -load helpers - -function teardown() { - cleanup_test -} - -function setup() { - copy_images -} - -@test "top without container name or id" { - run ${PODMAN_BINARY} ${PODMAN_OPTIONS} top - echo "$output" - [ "$status" -eq 125 ] -} - -@test "top a bogus container" { - run ${PODMAN_BINARY} ${PODMAN_OPTIONS} top foobar - echo "$output" - [ "$status" -eq 125 ] -} - -@test "top non-running container by id with defaults" { - run ${PODMAN_BINARY} ${PODMAN_OPTIONS} create -d ${ALPINE} sleep 60 - [ "$status" -eq 0 ] - ctr_id="$output" - run ${PODMAN_BINARY} ${PODMAN_OPTIONS} top $ctr_id - echo "$output" - [ "$status" -eq 125 ] -} - -@test "top running container by id with defaults" { - run ${PODMAN_BINARY} ${PODMAN_OPTIONS} run -dt ${ALPINE} /bin/sh - [ "$status" -eq 0 ] - ctr_id="$output" - echo $ctr_id - run ${PODMAN_BINARY} ${PODMAN_OPTIONS} top $ctr_id - echo "$output" - [ "$status" -eq 0 ] -} - -@test "top running container by id with ps opts" { - run ${PODMAN_BINARY} ${PODMAN_OPTIONS} run -d ${ALPINE} sleep 60 - [ "$status" -eq 0 ] - ctr_id="$output" - run ${PODMAN_BINARY} ${PODMAN_OPTIONS} top $ctr_id -o fuser,f,comm,label - echo "$output" - [ "$status" -eq 0 ] -} -- cgit v1.2.3-54-g00ecf