From 63d8ba64e4fd1ed1bef910b313029ca6535f43cf Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Tue, 14 Jan 2020 13:58:03 +0100 Subject: v2 api: top improvements * Use `pkg/adapter` to increase code reuse and reduce code redundancy. * Extend swagger docs to mention AIX descriptors. * Document the libpod endpoint which shares the same handler. Signed-off-by: Valentin Rothberg --- pkg/adapter/containers.go | 6 +----- pkg/api/handlers/containers_top.go | 27 ++++++++++++--------------- pkg/api/handlers/types.go | 1 - pkg/api/server/register_containers.go | 30 +++++++++++++++++++++++++++++- 4 files changed, 42 insertions(+), 22 deletions(-) diff --git a/pkg/adapter/containers.go b/pkg/adapter/containers.go index 8b21d6b94..df9f13944 100644 --- a/pkg/adapter/containers.go +++ b/pkg/adapter/containers.go @@ -927,9 +927,7 @@ func (r *LocalRuntime) Top(cli *cliconfig.TopValues) ([]string, error) { 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 + return nil, errors.Wrapf(err, "error executing ps(1) in the container") } // Trick: filter the ps command from the output instead of @@ -956,10 +954,8 @@ func (r *LocalRuntime) execPS(c *libpod.Container, args []string) ([]string, err streams := new(libpod.AttachStreams) streams.OutputStream = wPipe streams.ErrorStream = wPipe - streams.InputStream = bufio.NewReader(os.Stdin) streams.AttachOutput = true streams.AttachError = true - streams.AttachInput = true psOutput := []string{} go func() { diff --git a/pkg/api/handlers/containers_top.go b/pkg/api/handlers/containers_top.go index bab559da1..711f6858f 100644 --- a/pkg/api/handlers/containers_top.go +++ b/pkg/api/handlers/containers_top.go @@ -4,8 +4,10 @@ import ( "net/http" "strings" + "github.com/containers/libpod/cmd/podman/cliconfig" "github.com/containers/libpod/libpod" "github.com/containers/libpod/libpod/define" + "github.com/containers/libpod/pkg/adapter" "github.com/containers/libpod/pkg/api/handlers/utils" "github.com/gorilla/mux" "github.com/gorilla/schema" @@ -28,24 +30,19 @@ func TopContainer(w http.ResponseWriter, r *http.Request) { } name := mux.Vars(r)["name"] - ctnr, err := runtime.LookupContainer(name) - if err != nil { - utils.ContainerNotFound(w, name, err) - return - } - state, err := ctnr.State() - if err != nil { - utils.InternalServerError(w, err) - return - } - if state != define.ContainerStateRunning { - utils.ContainerNotRunning(w, name, errors.Errorf("Container %s must be running to perform top operation", name)) - return - } + adapterRuntime := adapter.LocalRuntime{} + adapterRuntime.Runtime = runtime - output, err := ctnr.Top([]string{}) + topValues := cliconfig.TopValues{} + topValues.InputArgs = []string{name, query.PsArgs} + + output, err := adapterRuntime.Top(&topValues) if err != nil { + if errors.Cause(err) == define.ErrNoSuchCtr { + utils.ContainerNotFound(w, name, err) + return + } utils.InternalServerError(w, err) return } diff --git a/pkg/api/handlers/types.go b/pkg/api/handlers/types.go index 9edbbdccc..2526a3317 100644 --- a/pkg/api/handlers/types.go +++ b/pkg/api/handlers/types.go @@ -135,7 +135,6 @@ type Stats struct { type ContainerTopOKBody struct { dockerContainer.ContainerTopOKBody - ID string `json:"Id"` } type PodCreateConfig struct { diff --git a/pkg/api/server/register_containers.go b/pkg/api/server/register_containers.go index 95b986170..ad7b2b0ca 100644 --- a/pkg/api/server/register_containers.go +++ b/pkg/api/server/register_containers.go @@ -345,7 +345,7 @@ func (s *APIServer) RegisterContainersHandlers(r *mux.Router) error { // - in: query // name: ps_args // type: string - // description: arguments to pass to ps such as aux + // description: arguments to pass to ps such as aux. Requires ps(1) to be installed in the container if no ps(1) compatible AIX descriptors are used. // produces: // - application/json // responses: @@ -653,6 +653,34 @@ func (s *APIServer) RegisterContainersHandlers(r *mux.Router) error { // '500': // "$ref": "#/responses/InternalError" r.HandleFunc(VersionedPath("/libpod/containers/{name:..*}/stats"), APIHandler(s.Context, generic.StatsContainer)).Methods(http.MethodGet) + // swagger:operation GET /libpod/containers/{nameOrID}/top containers topContainer + // + // List processes running inside a container. Note + // + // --- + // parameters: + // - in: path + // name: nameOrID + // required: true + // description: the name or ID of the container + // - in: query + // name: stream + // type: bool + // default: true + // description: Stream the output + // name: ps_args + // type: string + // description: arguments to pass to ps such as aux. Requires ps(1) to be installed in the container if no ps(1) compatible AIX descriptors are used. + // produces: + // - application/json + // responses: + // '200': + // description: no error + // "ref": "#/responses/DockerTopResponse" + // '404': + // "$ref": "#/responses/NoSuchContainer" + // '500': + // "$ref": "#/responses/InternalError" r.HandleFunc(VersionedPath("/libpod/containers/{name:..*}/top"), APIHandler(s.Context, handlers.TopContainer)).Methods(http.MethodGet) // swagger:operation POST /libpod/containers/{nameOrID}/unpause containers libpodUnpauseContainer // --- -- cgit v1.2.3-54-g00ecf From 88372c2c21b653131f993825484bf9a5ea509bdf Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Tue, 14 Jan 2020 15:41:38 +0100 Subject: top: use a separate pipe for the error stream Let's not mix apples and oranges and give stderr a dedicated pipe. This way, we don't return conmon log messages if run in debug mode. Signed-off-by: Valentin Rothberg --- pkg/adapter/containers.go | 32 +++++++++++++++++++++++++++----- 1 file changed, 27 insertions(+), 5 deletions(-) diff --git a/pkg/adapter/containers.go b/pkg/adapter/containers.go index df9f13944..a257ef4f4 100644 --- a/pkg/adapter/containers.go +++ b/pkg/adapter/containers.go @@ -951,17 +951,31 @@ func (r *LocalRuntime) execPS(c *libpod.Container, args []string) ([]string, err defer wPipe.Close() defer rPipe.Close() + rErrPipe, wErrPipe, err := os.Pipe() + if err != nil { + return nil, err + } + defer wErrPipe.Close() + defer rErrPipe.Close() + streams := new(libpod.AttachStreams) streams.OutputStream = wPipe - streams.ErrorStream = wPipe + streams.ErrorStream = wErrPipe streams.AttachOutput = true streams.AttachError = true - psOutput := []string{} + stdout := []string{} go func() { scanner := bufio.NewScanner(rPipe) for scanner.Scan() { - psOutput = append(psOutput, scanner.Text()) + stdout = append(stdout, scanner.Text()) + } + }() + stderr := []string{} + go func() { + scanner := bufio.NewScanner(rErrPipe) + for scanner.Scan() { + stderr = append(stderr, scanner.Text()) } }() @@ -970,10 +984,18 @@ func (r *LocalRuntime) execPS(c *libpod.Container, args []string) ([]string, err if err != nil { return nil, err } else if ec != 0 { - return nil, errors.Errorf("Runtime failed with exit status: %d and output: %s", ec, strings.Join(psOutput, " ")) + return nil, errors.Errorf("Runtime failed with exit status: %d and output: %s", ec, strings.Join(stderr, " ")) + } + + if logrus.GetLevel() >= logrus.DebugLevel { + // If we're running in debug mode or higher, we might want to have a + // look at stderr which includes debug logs from conmon. + for _, log := range stderr { + logrus.Debugf("%s", log) + } } - return psOutput, nil + return stdout, nil } // ExecContainer executes a command in the container -- cgit v1.2.3-54-g00ecf From 9d54815c2690353a295b602b67084fd1f3c095e8 Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Wed, 15 Jan 2020 16:36:27 +0100 Subject: refactor top code Move the top logic from pkg/adapter into the (*libpod.Container).Top(). This way, we drop the dependency from pkg/api on pkg/adapters and have a clearer separation of concerns. Signed-off-by: Valentin Rothberg --- libpod/container_top_linux.go | 88 +++++++++++++++++++++++++++++++++++++- pkg/adapter/containers.go | 85 +----------------------------------- pkg/api/handlers/containers_top.go | 20 +++------ 3 files changed, 94 insertions(+), 99 deletions(-) diff --git a/libpod/container_top_linux.go b/libpod/container_top_linux.go index 5f4f28130..98edc340a 100644 --- a/libpod/container_top_linux.go +++ b/libpod/container_top_linux.go @@ -3,6 +3,8 @@ package libpod import ( + "bufio" + "os" "strconv" "strings" @@ -10,6 +12,7 @@ import ( "github.com/containers/libpod/pkg/rootless" "github.com/containers/psgo" "github.com/pkg/errors" + "github.com/sirupsen/logrus" ) // Top gathers statistics about the running processes in a container. It returns a @@ -36,7 +39,34 @@ func (c *Container) Top(descriptors []string) ([]string, error) { } } } - return c.GetContainerPidInformation(psgoDescriptors) + + // 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. + output, psgoErr := c.GetContainerPidInformation(psgoDescriptors) + if psgoErr == nil { + return output, nil + } + if errors.Cause(psgoErr) != psgo.ErrUnknownDescriptor { + return nil, psgoErr + } + + output, err = c.execPS(descriptors) + if err != nil { + return nil, errors.Wrapf(err, "error executing ps(1) in the container") + } + + // 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 } // GetContainerPidInformation returns process-related data of all processes in @@ -65,3 +95,59 @@ func (c *Container) GetContainerPidInformation(descriptors []string) ([]string, } return res, nil } + +// execPS executes ps(1) with the specified args in the container. +func (c *Container) execPS(args []string) ([]string, error) { + rPipe, wPipe, err := os.Pipe() + if err != nil { + return nil, err + } + defer wPipe.Close() + defer rPipe.Close() + + rErrPipe, wErrPipe, err := os.Pipe() + if err != nil { + return nil, err + } + defer wErrPipe.Close() + defer rErrPipe.Close() + + streams := new(AttachStreams) + streams.OutputStream = wPipe + streams.ErrorStream = wErrPipe + streams.AttachOutput = true + streams.AttachError = true + + stdout := []string{} + go func() { + scanner := bufio.NewScanner(rPipe) + for scanner.Scan() { + stdout = append(stdout, scanner.Text()) + } + }() + stderr := []string{} + go func() { + scanner := bufio.NewScanner(rErrPipe) + for scanner.Scan() { + stderr = append(stderr, scanner.Text()) + } + }() + + cmd := append([]string{"ps"}, args...) + ec, err := c.Exec(false, false, map[string]string{}, cmd, "", "", streams, 0, nil, "") + if err != nil { + return nil, err + } else if ec != 0 { + return nil, errors.Errorf("Runtime failed with exit status: %d and output: %s", ec, strings.Join(stderr, " ")) + } + + if logrus.GetLevel() >= logrus.DebugLevel { + // If we're running in debug mode or higher, we might want to have a + // look at stderr which includes debug logs from conmon. + for _, log := range stderr { + logrus.Debugf("%s", log) + } + } + + return stdout, nil +} diff --git a/pkg/adapter/containers.go b/pkg/adapter/containers.go index a257ef4f4..f66999ffa 100644 --- a/pkg/adapter/containers.go +++ b/pkg/adapter/containers.go @@ -27,7 +27,6 @@ import ( "github.com/containers/libpod/libpod/logs" "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" @@ -913,89 +912,7 @@ func (r *LocalRuntime) Top(cli *cliconfig.TopValues) ([]string, error) { return nil, errors.Wrapf(err, "unable to lookup requested container") } - 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 { - return nil, errors.Wrapf(err, "error executing ps(1) in the container") - } - - // 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() - - rErrPipe, wErrPipe, err := os.Pipe() - if err != nil { - return nil, err - } - defer wErrPipe.Close() - defer rErrPipe.Close() - - streams := new(libpod.AttachStreams) - streams.OutputStream = wPipe - streams.ErrorStream = wErrPipe - streams.AttachOutput = true - streams.AttachError = true - - stdout := []string{} - go func() { - scanner := bufio.NewScanner(rPipe) - for scanner.Scan() { - stdout = append(stdout, scanner.Text()) - } - }() - stderr := []string{} - go func() { - scanner := bufio.NewScanner(rErrPipe) - for scanner.Scan() { - stderr = append(stderr, scanner.Text()) - } - }() - - cmd := append([]string{"ps"}, args...) - ec, err := c.Exec(false, false, map[string]string{}, cmd, "", "", streams, 0, nil, "") - if err != nil { - return nil, err - } else if ec != 0 { - return nil, errors.Errorf("Runtime failed with exit status: %d and output: %s", ec, strings.Join(stderr, " ")) - } - - if logrus.GetLevel() >= logrus.DebugLevel { - // If we're running in debug mode or higher, we might want to have a - // look at stderr which includes debug logs from conmon. - for _, log := range stderr { - logrus.Debugf("%s", log) - } - } - - return stdout, nil + return container.Top(descriptors) } // ExecContainer executes a command in the container diff --git a/pkg/api/handlers/containers_top.go b/pkg/api/handlers/containers_top.go index 711f6858f..1a310262c 100644 --- a/pkg/api/handlers/containers_top.go +++ b/pkg/api/handlers/containers_top.go @@ -4,10 +4,7 @@ import ( "net/http" "strings" - "github.com/containers/libpod/cmd/podman/cliconfig" "github.com/containers/libpod/libpod" - "github.com/containers/libpod/libpod/define" - "github.com/containers/libpod/pkg/adapter" "github.com/containers/libpod/pkg/api/handlers/utils" "github.com/gorilla/mux" "github.com/gorilla/schema" @@ -30,19 +27,14 @@ func TopContainer(w http.ResponseWriter, r *http.Request) { } name := mux.Vars(r)["name"] + c, err := runtime.LookupContainer(name) + if err != nil { + utils.ContainerNotFound(w, name, err) + return + } - adapterRuntime := adapter.LocalRuntime{} - adapterRuntime.Runtime = runtime - - topValues := cliconfig.TopValues{} - topValues.InputArgs = []string{name, query.PsArgs} - - output, err := adapterRuntime.Top(&topValues) + output, err := c.Top([]string{query.PsArgs}) if err != nil { - if errors.Cause(err) == define.ErrNoSuchCtr { - utils.ContainerNotFound(w, name, err) - return - } utils.InternalServerError(w, err) return } -- cgit v1.2.3-54-g00ecf From a65068aff8d0cd9fdb8993c20e609364eb1213af Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Wed, 15 Jan 2020 16:51:46 +0100 Subject: api: utils: add an `IsLibpodRequest` handler Add a hanlder to figure out if the specified http request came through a libpod endpoint. A first user is the top endpoint which has a different default value for `ps_args` depending if the request came through the docker or libpod endpoint. Signed-off-by: Valentin Rothberg --- pkg/api/handlers/containers_top.go | 6 +++++- pkg/api/handlers/utils/handler.go | 16 ++++++++++++---- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/pkg/api/handlers/containers_top.go b/pkg/api/handlers/containers_top.go index 1a310262c..6b7688eb0 100644 --- a/pkg/api/handlers/containers_top.go +++ b/pkg/api/handlers/containers_top.go @@ -15,10 +15,14 @@ func TopContainer(w http.ResponseWriter, r *http.Request) { runtime := r.Context().Value("runtime").(*libpod.Runtime) decoder := r.Context().Value("decoder").(*schema.Decoder) + defaultValue := "-ef" + if utils.IsLibpodRequest(r) { + defaultValue = "" + } query := struct { PsArgs string `schema:"ps_args"` }{ - PsArgs: "-ef", + PsArgs: defaultValue, } if err := decoder.Decode(&query, r.URL.Query()); err != nil { utils.Error(w, http.StatusText(http.StatusBadRequest), http.StatusBadRequest, diff --git a/pkg/api/handlers/utils/handler.go b/pkg/api/handlers/utils/handler.go index 65698bfd3..2fd9bffba 100644 --- a/pkg/api/handlers/utils/handler.go +++ b/pkg/api/handlers/utils/handler.go @@ -6,10 +6,18 @@ import ( "io" "net/http" "os" + "strings" - log "github.com/sirupsen/logrus" + "github.com/sirupsen/logrus" ) +// IsLibpodRequest returns true if the request related to a libpod endpoint +// (e.g., /v2/libpod/...). +func IsLibpodRequest(r *http.Request) bool { + split := strings.Split(r.URL.String(), "/") + return len(split) >= 3 && split[2] == "libpod" +} + // WriteResponse encodes the given value as JSON or string and renders it for http client func WriteResponse(w http.ResponseWriter, code int, value interface{}) { switch v := value.(type) { @@ -18,14 +26,14 @@ func WriteResponse(w http.ResponseWriter, code int, value interface{}) { w.WriteHeader(code) if _, err := fmt.Fprintln(w, v); err != nil { - log.Errorf("unable to send string response: %q", err) + logrus.Errorf("unable to send string response: %q", err) } case *os.File: w.Header().Set("Content-Type", "application/octet; charset=us-ascii") w.WriteHeader(code) if _, err := io.Copy(w, v); err != nil { - log.Errorf("unable to copy to response: %q", err) + logrus.Errorf("unable to copy to response: %q", err) } default: WriteJSON(w, code, value) @@ -40,6 +48,6 @@ func WriteJSON(w http.ResponseWriter, code int, value interface{}) { coder := json.NewEncoder(w) coder.SetEscapeHTML(true) if err := coder.Encode(value); err != nil { - log.Errorf("unable to write json: %q", err) + logrus.Errorf("unable to write json: %q", err) } } -- cgit v1.2.3-54-g00ecf From 69bd57f518cbc7c97808a663d76066d0d247ac0f Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Wed, 15 Jan 2020 17:04:12 +0100 Subject: api: stats: fix typo Signed-off-by: Valentin Rothberg --- pkg/api/handlers/generic/containers_stats.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/api/handlers/generic/containers_stats.go b/pkg/api/handlers/generic/containers_stats.go index 26c8efa15..e33d37606 100644 --- a/pkg/api/handlers/generic/containers_stats.go +++ b/pkg/api/handlers/generic/containers_stats.go @@ -43,7 +43,7 @@ func StatsContainer(w http.ResponseWriter, r *http.Request) { return } - // If the container isn't returning, then let's not bother and return + // If the container isn't running, then let's not bother and return // immediately. state, err := ctnr.State() if err != nil { -- cgit v1.2.3-54-g00ecf