From 376ba349bfadc47a938084ccb3c2d112c92f09ca Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Wed, 23 Sep 2020 13:32:58 +0200 Subject: stats refactor Refactor the entities' stats API to simplify using it and reduce the risk of running into concurrency issues at the call sites. Further simplify the stats code by de-spaghetti-ing the logic and reducing duplicate code. `ContainerStats` now returns a data channel and an error. If the error is nil, callers can read from the channel. Signed-off-by: Valentin Rothberg --- pkg/domain/infra/tunnel/containers.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'pkg/domain/infra/tunnel/containers.go') diff --git a/pkg/domain/infra/tunnel/containers.go b/pkg/domain/infra/tunnel/containers.go index d0f90d900..60136faae 100644 --- a/pkg/domain/infra/tunnel/containers.go +++ b/pkg/domain/infra/tunnel/containers.go @@ -682,6 +682,6 @@ func (ic *ContainerEngine) ContainerCp(ctx context.Context, source, dest string, func (ic *ContainerEngine) Shutdown(_ context.Context) { } -func (ic *ContainerEngine) ContainerStats(ctx context.Context, namesOrIds []string, options entities.ContainerStatsOptions) error { - return errors.New("not implemented") +func (ic *ContainerEngine) ContainerStats(ctx context.Context, namesOrIds []string, options entities.ContainerStatsOptions) (statsChan chan entities.ContainerStatsReport, err error) { + return nil, errors.New("not implemented") } -- cgit v1.2.3-54-g00ecf From 762b787fbf741eec0e59d81aaebbfc467351ceaa Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Wed, 23 Sep 2020 13:33:17 +0200 Subject: new endpoint: /libpod/containers/stats Add a new endpoint for container stats allowing for batch operations on more than one container. The new endpoint deprecates the single-container endpoint which will eventually be removed with the next major release. Signed-off-by: Valentin Rothberg --- pkg/api/handlers/libpod/containers_stats.go | 73 +++++++++++++++++++++++++++++ pkg/api/server/register_containers.go | 31 +++++++++++- pkg/bindings/containers/containers.go | 51 +++++++++++++++++++- pkg/domain/infra/abi/containers.go | 2 +- pkg/domain/infra/tunnel/containers.go | 3 +- test/e2e/stats_test.go | 2 +- 6 files changed, 157 insertions(+), 5 deletions(-) create mode 100644 pkg/api/handlers/libpod/containers_stats.go (limited to 'pkg/domain/infra/tunnel/containers.go') diff --git a/pkg/api/handlers/libpod/containers_stats.go b/pkg/api/handlers/libpod/containers_stats.go new file mode 100644 index 000000000..3066d5ebc --- /dev/null +++ b/pkg/api/handlers/libpod/containers_stats.go @@ -0,0 +1,73 @@ +package libpod + +import ( + "encoding/json" + "net/http" + "time" + + "github.com/containers/podman/v2/libpod" + "github.com/containers/podman/v2/pkg/api/handlers/utils" + "github.com/containers/podman/v2/pkg/domain/entities" + "github.com/containers/podman/v2/pkg/domain/infra/abi" + "github.com/gorilla/schema" + "github.com/pkg/errors" + "github.com/sirupsen/logrus" +) + +const DefaultStatsPeriod = 5 * time.Second + +func StatsContainer(w http.ResponseWriter, r *http.Request) { + runtime := r.Context().Value("runtime").(*libpod.Runtime) + decoder := r.Context().Value("decoder").(*schema.Decoder) + + query := struct { + Containers []string `schema:"containers"` + Stream bool `schema:"stream"` + }{ + Stream: true, + } + if err := decoder.Decode(&query, r.URL.Query()); err != nil { + utils.Error(w, "Something went wrong.", http.StatusBadRequest, errors.Wrapf(err, "Failed to parse parameters for %s", r.URL.String())) + return + } + + // Reduce code duplication and use the local/abi implementation of + // container stats. + containerEngine := abi.ContainerEngine{Libpod: runtime} + + statsOptions := entities.ContainerStatsOptions{ + All: len(query.Containers) == 0, // no containers -> query all of them + NoStream: !query.Stream, + } + + // Stats will stop if the connection is closed. + statsChan, err := containerEngine.ContainerStats(r.Context(), query.Containers, statsOptions) + if err != nil { + utils.InternalServerError(w, err) + return + } + + // Write header and content type. + w.WriteHeader(http.StatusOK) + w.Header().Add("Content-Type", "application/json") + if flusher, ok := w.(http.Flusher); ok { + flusher.Flush() + } + + // Setup JSON encoder for streaming. + coder := json.NewEncoder(w) + coder.SetEscapeHTML(true) + + for stats := range statsChan { + if err := coder.Encode(stats); err != nil { + // Note: even when streaming, the stats goroutine will + // be notified (and stop) as the connection will be + // closed. + logrus.Errorf("Unable to encode stats: %v", err) + return + } + if flusher, ok := w.(http.Flusher); ok { + flusher.Flush() + } + } +} diff --git a/pkg/api/server/register_containers.go b/pkg/api/server/register_containers.go index 0ad5d29ea..870c6a90c 100644 --- a/pkg/api/server/register_containers.go +++ b/pkg/api/server/register_containers.go @@ -1013,7 +1013,7 @@ func (s *APIServer) registerContainersHandlers(r *mux.Router) error { // tags: // - containers // summary: Get stats for a container - // description: This returns a live stream of a container’s resource usage statistics. + // description: DEPRECATED. This endpoint will be removed with the next major release. Please use /libpod/containers/stats instead. // parameters: // - in: path // name: name @@ -1035,6 +1035,35 @@ func (s *APIServer) registerContainersHandlers(r *mux.Router) error { // 500: // $ref: "#/responses/InternalError" r.HandleFunc(VersionedPath("/libpod/containers/{name}/stats"), s.APIHandler(compat.StatsContainer)).Methods(http.MethodGet) + // swagger:operation GET /libpod/containers/stats libpod libpodStatsContainers + // --- + // tags: + // - containers + // summary: Get stats for one or more containers + // description: Return a live stream of resource usage statistics of one or more container. If no container is specified, the statistics of all containers are returned. + // parameters: + // - in: query + // name: containers + // description: names or IDs of containers + // type: array + // items: + // type: string + // - in: query + // name: stream + // type: boolean + // default: true + // description: Stream the output + // produces: + // - application/json + // responses: + // 200: + // description: no error + // 404: + // $ref: "#/responses/NoSuchContainer" + // 500: + // $ref: "#/responses/InternalError" + r.HandleFunc(VersionedPath("/libpod/containers/stats"), s.APIHandler(libpod.StatsContainer)).Methods(http.MethodGet) + // swagger:operation GET /libpod/containers/{name}/top libpod libpodTopContainer // --- // tags: diff --git a/pkg/bindings/containers/containers.go b/pkg/bindings/containers/containers.go index 981912665..46e4df1d2 100644 --- a/pkg/bindings/containers/containers.go +++ b/pkg/bindings/containers/containers.go @@ -197,7 +197,56 @@ func Start(ctx context.Context, nameOrID string, detachKeys *string) error { return response.Process(nil) } -func Stats() {} +func Stats(ctx context.Context, containers []string, stream *bool) (chan entities.ContainerStatsReport, error) { + conn, err := bindings.GetClient(ctx) + if err != nil { + return nil, err + } + params := url.Values{} + if stream != nil { + params.Set("stream", strconv.FormatBool(*stream)) + } + for _, c := range containers { + params.Add("containers", c) + } + + response, err := conn.DoRequest(nil, http.MethodGet, "/containers/stats", params, nil) + if err != nil { + return nil, err + } + + statsChan := make(chan entities.ContainerStatsReport) + + go func() { + defer close(statsChan) + + dec := json.NewDecoder(response.Body) + doStream := true + if stream != nil { + doStream = *stream + } + + streamLabel: // label to flatten the scope + select { + case <-response.Request.Context().Done(): + return // lost connection - maybe the server quit + default: + // fall through and do some work + } + var report entities.ContainerStatsReport + if err := dec.Decode(&report); err != nil { + report = entities.ContainerStatsReport{Error: err} + } + statsChan <- report + + if report.Error != nil || !doStream { + return + } + goto streamLabel + }() + + return statsChan, nil +} // Top gathers statistics about the running processes in a container. The nameOrID can be a container name // or a partial/full ID. The descriptors allow for specifying which data to collect from the process. diff --git a/pkg/domain/infra/abi/containers.go b/pkg/domain/infra/abi/containers.go index d55bc5c17..c77d27cb2 100644 --- a/pkg/domain/infra/abi/containers.go +++ b/pkg/domain/infra/abi/containers.go @@ -1216,7 +1216,7 @@ func (ic *ContainerEngine) ContainerStats(ctx context.Context, namesOrIds []stri report.Stats, report.Error = computeStats() statsChan <- report - if options.NoStream { + if report.Error != nil || options.NoStream { return } diff --git a/pkg/domain/infra/tunnel/containers.go b/pkg/domain/infra/tunnel/containers.go index 60136faae..41fb62ab6 100644 --- a/pkg/domain/infra/tunnel/containers.go +++ b/pkg/domain/infra/tunnel/containers.go @@ -683,5 +683,6 @@ func (ic *ContainerEngine) Shutdown(_ context.Context) { } func (ic *ContainerEngine) ContainerStats(ctx context.Context, namesOrIds []string, options entities.ContainerStatsOptions) (statsChan chan entities.ContainerStatsReport, err error) { - return nil, errors.New("not implemented") + stream := !options.NoStream + return containers.Stats(ic.ClientCxt, namesOrIds, &stream) } diff --git a/test/e2e/stats_test.go b/test/e2e/stats_test.go index ff6ddce7e..7ab435007 100644 --- a/test/e2e/stats_test.go +++ b/test/e2e/stats_test.go @@ -1,4 +1,4 @@ -// +build !remote +// +build package integration -- cgit v1.2.3-54-g00ecf From 19b955f0999f7fe9e187e94d60327e4d6ee891c0 Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Thu, 24 Sep 2020 14:28:10 +0200 Subject: stats: break out CLI options Have a clear separation of concerns for the CLI-only options (and their logic) from the backend. The backend logic is now easier to understand (e.g., `stream` instead of `noStream`). Signed-off-by: Valentin Rothberg --- cmd/podman/containers/stats.go | 20 ++++++++++++++++++-- pkg/api/handlers/libpod/containers_stats.go | 3 +-- pkg/domain/entities/containers.go | 12 +++++++----- pkg/domain/infra/abi/containers.go | 9 ++++++--- pkg/domain/infra/tunnel/containers.go | 6 ++++-- 5 files changed, 36 insertions(+), 14 deletions(-) (limited to 'pkg/domain/infra/tunnel/containers.go') diff --git a/cmd/podman/containers/stats.go b/cmd/podman/containers/stats.go index 1a4adb376..bbd389bbf 100644 --- a/cmd/podman/containers/stats.go +++ b/cmd/podman/containers/stats.go @@ -47,8 +47,18 @@ var ( } ) +// statsOptionsCLI is used for storing CLI arguments. Some fields are later +// used in the backend. +type statsOptionsCLI struct { + All bool + Format string + Latest bool + NoReset bool + NoStream bool +} + var ( - statsOptions entities.ContainerStatsOptions + statsOptions statsOptionsCLI defaultStatsRow = "{{.ID}}\t{{.Name}}\t{{.CPUPerc}}\t{{.MemUsage}}\t{{.MemPerc}}\t{{.NetIO}}\t{{.BlockIO}}\t{{.PIDS}}\n" defaultStatsHeader = "ID\tNAME\tCPU %\tMEM USAGE / LIMIT\tMEM %\tNET IO\tBLOCK IO\tPIDS\n" ) @@ -107,7 +117,13 @@ func stats(cmd *cobra.Command, args []string) error { } } - statsChan, err := registry.ContainerEngine().ContainerStats(registry.Context(), args, statsOptions) + // Convert to the entities options. We should not leak CLI-only + // options into the backend and separate concerns. + opts := entities.ContainerStatsOptions{ + Latest: statsOptions.Latest, + Stream: !statsOptions.NoStream, + } + statsChan, err := registry.ContainerEngine().ContainerStats(registry.Context(), args, opts) if err != nil { return err } diff --git a/pkg/api/handlers/libpod/containers_stats.go b/pkg/api/handlers/libpod/containers_stats.go index 3066d5ebc..4d5abe118 100644 --- a/pkg/api/handlers/libpod/containers_stats.go +++ b/pkg/api/handlers/libpod/containers_stats.go @@ -36,8 +36,7 @@ func StatsContainer(w http.ResponseWriter, r *http.Request) { containerEngine := abi.ContainerEngine{Libpod: runtime} statsOptions := entities.ContainerStatsOptions{ - All: len(query.Containers) == 0, // no containers -> query all of them - NoStream: !query.Stream, + Stream: query.Stream, } // Stats will stop if the connection is closed. diff --git a/pkg/domain/entities/containers.go b/pkg/domain/entities/containers.go index b6f86785f..7b272f01e 100644 --- a/pkg/domain/entities/containers.go +++ b/pkg/domain/entities/containers.go @@ -411,15 +411,17 @@ type ContainerCpReport struct { // ContainerStatsOptions describes input options for getting // stats on containers type ContainerStatsOptions struct { - All bool - Format string - Latest bool - NoReset bool - NoStream bool + // Operate on the latest known container. Only supported for local + // clients. + Latest bool + // Stream stats. + Stream bool } // ContainerStatsReport is used for streaming container stats. type ContainerStatsReport struct { + // Error from reading stats. Error error + // Results, set when there is no error. Stats []define.ContainerStats } diff --git a/pkg/domain/infra/abi/containers.go b/pkg/domain/infra/abi/containers.go index c77d27cb2..8b0d53940 100644 --- a/pkg/domain/infra/abi/containers.go +++ b/pkg/domain/infra/abi/containers.go @@ -1146,6 +1146,7 @@ func (ic *ContainerEngine) ContainerStats(ctx context.Context, namesOrIds []stri statsChan = make(chan entities.ContainerStatsReport, 1) containerFunc := ic.Libpod.GetRunningContainers + queryAll := false switch { case options.Latest: containerFunc = func() ([]*libpod.Container, error) { @@ -1157,7 +1158,9 @@ func (ic *ContainerEngine) ContainerStats(ctx context.Context, namesOrIds []stri } case len(namesOrIds) > 0: containerFunc = func() ([]*libpod.Container, error) { return ic.Libpod.GetContainersByList(namesOrIds) } - case options.All: + default: + // No containers, no latest -> query all! + queryAll = true containerFunc = ic.Libpod.GetAllContainers } @@ -1197,7 +1200,7 @@ func (ic *ContainerEngine) ContainerStats(ctx context.Context, namesOrIds []stri stats, err := ctr.GetContainerStats(prev) if err != nil { cause := errors.Cause(err) - if options.All && (cause == define.ErrCtrRemoved || cause == define.ErrNoSuchCtr || cause == define.ErrCtrStateInvalid) { + if queryAll && (cause == define.ErrCtrRemoved || cause == define.ErrNoSuchCtr || cause == define.ErrCtrStateInvalid) { continue } if cause == cgroups.ErrCgroupV1Rootless { @@ -1216,7 +1219,7 @@ func (ic *ContainerEngine) ContainerStats(ctx context.Context, namesOrIds []stri report.Stats, report.Error = computeStats() statsChan <- report - if report.Error != nil || options.NoStream { + if report.Error != nil || !options.Stream { return } diff --git a/pkg/domain/infra/tunnel/containers.go b/pkg/domain/infra/tunnel/containers.go index 41fb62ab6..31b94ea4b 100644 --- a/pkg/domain/infra/tunnel/containers.go +++ b/pkg/domain/infra/tunnel/containers.go @@ -683,6 +683,8 @@ func (ic *ContainerEngine) Shutdown(_ context.Context) { } func (ic *ContainerEngine) ContainerStats(ctx context.Context, namesOrIds []string, options entities.ContainerStatsOptions) (statsChan chan entities.ContainerStatsReport, err error) { - stream := !options.NoStream - return containers.Stats(ic.ClientCxt, namesOrIds, &stream) + if options.Latest { + return nil, errors.New("latest is not supported for the remote client") + } + return containers.Stats(ic.ClientCxt, namesOrIds, &options.Stream) } -- cgit v1.2.3-54-g00ecf