From cdbbd79155a7752843f2b420c3036ce6c390a3b6 Mon Sep 17 00:00:00 2001 From: Thomas Weber Date: Wed, 21 Jul 2021 07:42:39 +0200 Subject: stats: add a interval parameter to cli and api stream mode podman stats polled by default in a 1 sec period. This can put quite some load on a machine if you run many containers. The default value is now 5 seconds. You can change this interval with a new, optional, --interval, -i cli flag. The api request got also a interval query parameter for the same purpose. Additionally a unused const was removed. Api and cli will fail the request if a 0 or negative value is passed in. Signed-off-by: Thomas Weber --- pkg/api/handlers/libpod/containers_stats.go | 10 +++++----- pkg/api/server/register_containers.go | 5 +++++ pkg/bindings/containers/types.go | 3 ++- pkg/bindings/containers/types_stats_options.go | 16 ++++++++++++++++ pkg/domain/entities/containers.go | 2 ++ pkg/domain/infra/abi/containers.go | 5 ++++- pkg/domain/infra/tunnel/containers.go | 2 +- 7 files changed, 35 insertions(+), 8 deletions(-) (limited to 'pkg') diff --git a/pkg/api/handlers/libpod/containers_stats.go b/pkg/api/handlers/libpod/containers_stats.go index 75c404d4f..1807823fa 100644 --- a/pkg/api/handlers/libpod/containers_stats.go +++ b/pkg/api/handlers/libpod/containers_stats.go @@ -3,7 +3,6 @@ package libpod import ( "encoding/json" "net/http" - "time" "github.com/containers/podman/v3/libpod" "github.com/containers/podman/v3/pkg/api/handlers/utils" @@ -14,8 +13,6 @@ import ( "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) @@ -23,8 +20,10 @@ func StatsContainer(w http.ResponseWriter, r *http.Request) { query := struct { Containers []string `schema:"containers"` Stream bool `schema:"stream"` + Interval int `schema:"interval"` }{ - Stream: true, + Stream: true, + Interval: 5, } 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())) @@ -36,7 +35,8 @@ func StatsContainer(w http.ResponseWriter, r *http.Request) { containerEngine := abi.ContainerEngine{Libpod: runtime} statsOptions := entities.ContainerStatsOptions{ - Stream: query.Stream, + Stream: query.Stream, + Interval: query.Interval, } // Stats will stop if the connection is closed. diff --git a/pkg/api/server/register_containers.go b/pkg/api/server/register_containers.go index 50e059ecc..5a4137533 100644 --- a/pkg/api/server/register_containers.go +++ b/pkg/api/server/register_containers.go @@ -1106,6 +1106,11 @@ func (s *APIServer) registerContainersHandlers(r *mux.Router) error { // type: boolean // default: true // description: Stream the output + // - in: query + // name: interval + // type: integer + // default: 5 + // description: Time in seconds between stats reports // produces: // - application/json // responses: diff --git a/pkg/bindings/containers/types.go b/pkg/bindings/containers/types.go index 1058c7a48..28c644841 100644 --- a/pkg/bindings/containers/types.go +++ b/pkg/bindings/containers/types.go @@ -165,7 +165,8 @@ type StartOptions struct { //go:generate go run ../generator/generator.go StatsOptions // StatsOptions are optional options for getting stats on containers type StatsOptions struct { - Stream *bool + Stream *bool + Interval *int } //go:generate go run ../generator/generator.go TopOptions diff --git a/pkg/bindings/containers/types_stats_options.go b/pkg/bindings/containers/types_stats_options.go index 8f6a03301..604004eb6 100644 --- a/pkg/bindings/containers/types_stats_options.go +++ b/pkg/bindings/containers/types_stats_options.go @@ -35,3 +35,19 @@ func (o *StatsOptions) GetStream() bool { } return *o.Stream } + +// WithInterval +func (o *StatsOptions) WithInterval(value int) *StatsOptions { + v := &value + o.Interval = v + return o +} + +// GetInterval +func (o *StatsOptions) GetInterval() int { + var interval int + if o.Interval == nil { + return interval + } + return *o.Interval +} diff --git a/pkg/domain/entities/containers.go b/pkg/domain/entities/containers.go index 302b35a47..dcb351e82 100644 --- a/pkg/domain/entities/containers.go +++ b/pkg/domain/entities/containers.go @@ -435,6 +435,8 @@ type ContainerStatsOptions struct { Latest bool // Stream stats. Stream bool + // Interval in seconds + Interval int } // ContainerStatsReport is used for streaming container stats. diff --git a/pkg/domain/infra/abi/containers.go b/pkg/domain/infra/abi/containers.go index 50751aa12..6eeef870f 100644 --- a/pkg/domain/infra/abi/containers.go +++ b/pkg/domain/infra/abi/containers.go @@ -1281,6 +1281,9 @@ func (ic *ContainerEngine) Shutdown(_ context.Context) { } func (ic *ContainerEngine) ContainerStats(ctx context.Context, namesOrIds []string, options entities.ContainerStatsOptions) (statsChan chan entities.ContainerStatsReport, err error) { + if options.Interval < 1 { + return nil, errors.New("Invalid interval, must be a positive number greater zero") + } statsChan = make(chan entities.ContainerStatsReport, 1) containerFunc := ic.Libpod.GetRunningContainers @@ -1361,7 +1364,7 @@ func (ic *ContainerEngine) ContainerStats(ctx context.Context, namesOrIds []stri return } - time.Sleep(time.Second) + time.Sleep(time.Second * time.Duration(options.Interval)) goto stream }() diff --git a/pkg/domain/infra/tunnel/containers.go b/pkg/domain/infra/tunnel/containers.go index c17d7b54f..1df79c373 100644 --- a/pkg/domain/infra/tunnel/containers.go +++ b/pkg/domain/infra/tunnel/containers.go @@ -871,7 +871,7 @@ func (ic *ContainerEngine) ContainerStats(ctx context.Context, namesOrIds []stri if options.Latest { return nil, errors.New("latest is not supported for the remote client") } - return containers.Stats(ic.ClientCtx, namesOrIds, new(containers.StatsOptions).WithStream(options.Stream)) + return containers.Stats(ic.ClientCtx, namesOrIds, new(containers.StatsOptions).WithStream(options.Stream).WithInterval(options.Interval)) } // ShouldRestart reports back whether the container will restart -- cgit v1.2.3-54-g00ecf From 34b28d95986a08bdd74dd89ce6647458cda75731 Mon Sep 17 00:00:00 2001 From: Thomas Weber Date: Mon, 26 Jul 2021 21:25:01 +0200 Subject: e2e tests: re-enable and fix podman stats tests Renamed podman pod stats test specs to distinguish them from podman stats tests. podman stats tests where disabled by a +build flag. Fix podman stats format test, add negative test. Fix podman stats cli command, exit non-zero on invalid format string. Add tests for podman stats interval flag. Signed-off-by: Thomas Weber --- cmd/podman/containers/stats.go | 3 +-- pkg/bindings/containers/containers.go | 3 +++ test/e2e/pod_stats_test.go | 22 +++++++++--------- test/e2e/stats_test.go | 44 ++++++++++++++++++++++++++++++----- 4 files changed, 53 insertions(+), 19 deletions(-) (limited to 'pkg') diff --git a/cmd/podman/containers/stats.go b/cmd/podman/containers/stats.go index 69fef7b79..11e8f6870 100644 --- a/cmd/podman/containers/stats.go +++ b/cmd/podman/containers/stats.go @@ -17,7 +17,6 @@ import ( "github.com/containers/podman/v3/utils" "github.com/docker/go-units" "github.com/pkg/errors" - "github.com/sirupsen/logrus" "github.com/spf13/cobra" ) @@ -140,7 +139,7 @@ func stats(cmd *cobra.Command, args []string) error { return report.Error } if err := outputStats(report.Stats); err != nil { - logrus.Error(err) + return err } } return nil diff --git a/pkg/bindings/containers/containers.go b/pkg/bindings/containers/containers.go index 86304f392..bc7b0c8c9 100644 --- a/pkg/bindings/containers/containers.go +++ b/pkg/bindings/containers/containers.go @@ -223,6 +223,9 @@ func Stats(ctx context.Context, containers []string, options *StatsOptions) (cha if err != nil { return nil, err } + if !response.IsSuccess() { + return nil, response.Process(nil) + } statsChan := make(chan entities.ContainerStatsReport) diff --git a/test/e2e/pod_stats_test.go b/test/e2e/pod_stats_test.go index 46043b16d..5ec209034 100644 --- a/test/e2e/pod_stats_test.go +++ b/test/e2e/pod_stats_test.go @@ -37,19 +37,19 @@ var _ = Describe("Podman pod stats", func() { processTestResult(f) }) - It("podman stats should run with no pods", func() { + It("podman pod stats should run with no pods", func() { session := podmanTest.Podman([]string{"pod", "stats", "--no-stream"}) session.WaitWithDefaultTimeout() Expect(session).Should(Exit(0)) }) - It("podman stats with a bogus pod", func() { + It("podman pod stats with a bogus pod", func() { session := podmanTest.Podman([]string{"pod", "stats", "foobar"}) session.WaitWithDefaultTimeout() Expect(session).Should(Exit(125)) }) - It("podman stats on a specific running pod", func() { + It("podman pod stats on a specific running pod", func() { _, ec, podid := podmanTest.CreatePod(nil) Expect(ec).To(Equal(0)) @@ -66,7 +66,7 @@ var _ = Describe("Podman pod stats", func() { Expect(stats).Should(Exit(0)) }) - It("podman stats on a specific running pod with shortID", func() { + It("podman pod stats on a specific running pod with shortID", func() { _, ec, podid := podmanTest.CreatePod(nil) Expect(ec).To(Equal(0)) @@ -83,7 +83,7 @@ var _ = Describe("Podman pod stats", func() { Expect(stats).Should(Exit(0)) }) - It("podman stats on a specific running pod with name", func() { + It("podman pod stats on a specific running pod with name", func() { _, ec, podid := podmanTest.CreatePod(map[string][]string{"--name": {"test"}}) Expect(ec).To(Equal(0)) @@ -100,7 +100,7 @@ var _ = Describe("Podman pod stats", func() { Expect(stats).Should(Exit(0)) }) - It("podman stats on running pods", func() { + It("podman pod stats on running pods", func() { _, ec, podid := podmanTest.CreatePod(nil) Expect(ec).To(Equal(0)) @@ -117,7 +117,7 @@ var _ = Describe("Podman pod stats", func() { Expect(stats).Should(Exit(0)) }) - It("podman stats on all pods", func() { + It("podman pod stats on all pods", func() { _, ec, podid := podmanTest.CreatePod(nil) Expect(ec).To(Equal(0)) @@ -134,7 +134,7 @@ var _ = Describe("Podman pod stats", func() { Expect(stats).Should(Exit(0)) }) - It("podman stats with json output", func() { + It("podman pod stats with json output", func() { _, ec, podid := podmanTest.CreatePod(nil) Expect(ec).To(Equal(0)) @@ -151,7 +151,7 @@ var _ = Describe("Podman pod stats", func() { Expect(stats).Should(Exit(0)) Expect(stats.IsJSONOutputValid()).To(BeTrue()) }) - It("podman stats with GO template", func() { + It("podman pod stats with GO template", func() { _, ec, podid := podmanTest.CreatePod(nil) Expect(ec).To(Equal(0)) @@ -163,7 +163,7 @@ var _ = Describe("Podman pod stats", func() { Expect(stats).To(Exit(0)) }) - It("podman stats with invalid GO template", func() { + It("podman pod stats with invalid GO template", func() { _, ec, podid := podmanTest.CreatePod(nil) Expect(ec).To(Equal(0)) @@ -175,7 +175,7 @@ var _ = Describe("Podman pod stats", func() { Expect(stats).To(ExitWithError()) }) - It("podman stats on net=host post", func() { + It("podman pod stats on net=host post", func() { SkipIfRootless("--net=host not supported for rootless pods at present") podName := "testPod" podCreate := podmanTest.Podman([]string{"pod", "create", "--net=host", "--name", podName}) diff --git a/test/e2e/stats_test.go b/test/e2e/stats_test.go index e32d515a0..a0be5d462 100644 --- a/test/e2e/stats_test.go +++ b/test/e2e/stats_test.go @@ -1,5 +1,3 @@ -// +build - package integration import ( @@ -84,15 +82,49 @@ var _ = Describe("Podman stats", func() { Expect(session).Should(Exit(0)) }) - It("podman stats only output CPU data", func() { + It("podman stats with GO template", func() { session := podmanTest.RunTopContainer("") session.WaitWithDefaultTimeout() Expect(session).Should(Exit(0)) - session = podmanTest.Podman([]string{"stats", "--all", "--no-stream", "--format", "\"{{.ID}} {{.UpTime}} {{.AVGCPU}}\""}) + stats := podmanTest.Podman([]string{"stats", "-a", "--no-reset", "--no-stream", "--format", "table {{.ID}} {{.AVGCPU}} {{.MemUsage}} {{.CPU}} {{.NetIO}} {{.BlockIO}} {{.PIDS}}"}) + stats.WaitWithDefaultTimeout() + Expect(stats).To(Exit(0)) + }) + + It("podman stats with invalid GO template", func() { + session := podmanTest.RunTopContainer("") session.WaitWithDefaultTimeout() - Expect(session.LineInOutputContains("UpTime")).To(BeTrue()) - Expect(session.LineInOutputContains("AVGCPU")).To(BeTrue()) Expect(session).Should(Exit(0)) + stats := podmanTest.Podman([]string{"stats", "-a", "--no-reset", "--no-stream", "--format", "\"table {{.ID}} {{.NoSuchField}} \""}) + stats.WaitWithDefaultTimeout() + Expect(stats).To(ExitWithError()) + }) + + It("podman stats with negative interval", func() { + session := podmanTest.RunTopContainer("") + session.WaitWithDefaultTimeout() + Expect(session).Should(Exit(0)) + stats := podmanTest.Podman([]string{"stats", "-a", "--no-reset", "--no-stream", "--interval=-1"}) + stats.WaitWithDefaultTimeout() + Expect(stats).To(ExitWithError()) + }) + + It("podman stats with zero interval", func() { + session := podmanTest.RunTopContainer("") + session.WaitWithDefaultTimeout() + Expect(session).Should(Exit(0)) + stats := podmanTest.Podman([]string{"stats", "-a", "--no-reset", "--no-stream", "--interval=0"}) + stats.WaitWithDefaultTimeout() + Expect(stats).To(ExitWithError()) + }) + + It("podman stats with interval", func() { + session := podmanTest.RunTopContainer("") + session.WaitWithDefaultTimeout() + Expect(session).Should(Exit(0)) + stats := podmanTest.Podman([]string{"stats", "-a", "--no-reset", "--no-stream", "--interval=5"}) + stats.WaitWithDefaultTimeout() + Expect(stats).Should(Exit(0)) }) It("podman stats with json output", func() { -- cgit v1.2.3-54-g00ecf