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 <rothberg@redhat.com>

5 files changed, 36 insertions, 14 deletions

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)
 }