cli/command/container: runStats(): minor cleanup and fixes

- memoize the API-client in a local variable.
- use struct-literals in some places.
- rename some variables for clarity and to prevent colliding with imports.
- make use of the event-constants (events.ContainerEventType).
- fix some grammar
- fix some minor linting warnings

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

cli/command/container/stats.go

@@ -29,29 +29,29 @@ type statsOptions struct {
 	containers []string
 }
 
-// NewStatsCommand creates a new cobra.Command for `docker stats`
+// NewStatsCommand creates a new [cobra.Command] for "docker stats".
-func NewStatsCommand(dockerCli command.Cli) *cobra.Command {
+func NewStatsCommand(dockerCLI command.Cli) *cobra.Command {
-	var opts statsOptions
+	var options statsOptions
 
 	cmd := &cobra.Command{
 		Use:   "stats [OPTIONS] [CONTAINER...]",
 		Short: "Display a live stream of container(s) resource usage statistics",
 		Args:  cli.RequiresMinArgs(0),
 		RunE: func(cmd *cobra.Command, args []string) error {
-			opts.containers = args
+			options.containers = args
-			return runStats(cmd.Context(), dockerCli, &opts)
+			return runStats(cmd.Context(), dockerCLI, &options)
 		},
 		Annotations: map[string]string{
 			"aliases": "docker container stats, docker stats",
 		},
-		ValidArgsFunction: completion.ContainerNames(dockerCli, false),
+		ValidArgsFunction: completion.ContainerNames(dockerCLI, false),
 	}
 
 	flags := cmd.Flags()
-	flags.BoolVarP(&opts.all, "all", "a", false, "Show all containers (default shows just running)")
+	flags.BoolVarP(&options.all, "all", "a", false, "Show all containers (default shows just running)")
-	flags.BoolVar(&opts.noStream, "no-stream", false, "Disable streaming stats and only pull the first result")
+	flags.BoolVar(&options.noStream, "no-stream", false, "Disable streaming stats and only pull the first result")
-	flags.BoolVar(&opts.noTrunc, "no-trunc", false, "Do not truncate output")
+	flags.BoolVar(&options.noTrunc, "no-trunc", false, "Do not truncate output")
-	flags.StringVar(&opts.format, "format", "", flagsHelper.FormatHelp)
+	flags.StringVar(&options.format, "format", "", flagsHelper.FormatHelp)
 	return cmd
 }
 
@@ -59,22 +59,21 @@ func NewStatsCommand(dockerCli command.Cli) *cobra.Command {
 // This shows real-time information on CPU usage, memory usage, and network I/O.
 //
 //nolint:gocyclo
-func runStats(ctx context.Context, dockerCli command.Cli, opts *statsOptions) error {
+func runStats(ctx context.Context, dockerCLI command.Cli, options *statsOptions) error {
-	showAll := len(opts.containers) == 0
+	showAll := len(options.containers) == 0
 	closeChan := make(chan error)
+	apiClient := dockerCLI.Client()
 
 	// monitorContainerEvents watches for container creation and removal (only
 	// used when calling `docker stats` without arguments).
 	monitorContainerEvents := func(started chan<- struct{}, c chan events.Message, stopped <-chan struct{}) {
 		f := filters.NewArgs()
-		f.Add("type", "container")
+		f.Add("type", string(events.ContainerEventType))
-		options := types.EventsOptions{
+		eventChan, errChan := apiClient.Events(ctx, types.EventsOptions{
 			Filters: f,
-		}
+		})
 
-		eventq, errq := dockerCli.Client().Events(ctx, options)
+		// Whether we successfully subscribed to eventChan or not, we can now
-
-		// Whether we successfully subscribed to eventq or not, we can now
 		// unblock the main goroutine.
 		close(started)
 		defer close(c)
@@ -83,9 +82,9 @@ func runStats(ctx context.Context, dockerCli command.Cli, opts *statsOptions) er
 			select {
 			case <-stopped:
 				return
-			case event := <-eventq:
+			case event := <-eventChan:
 				c <- event
-			case err := <-errq:
+			case err := <-errChan:
 				closeChan <- err
 				return
 			}
@@ -94,7 +93,7 @@ func runStats(ctx context.Context, dockerCli command.Cli, opts *statsOptions) er
 
 	// Get the daemonOSType if not set already
 	if daemonOSType == "" {
-		sv, err := dockerCli.Client().ServerVersion(ctx)
+		sv, err := apiClient.ServerVersion(ctx)
 		if err != nil {
 			return err
 		}
@@ -108,10 +107,9 @@ func runStats(ctx context.Context, dockerCli command.Cli, opts *statsOptions) er
 	// getContainerList simulates creation event for all previously existing
 	// containers (only used when calling `docker stats` without arguments).
 	getContainerList := func() {
-		options := container.ListOptions{
+		cs, err := apiClient.ContainerList(ctx, container.ListOptions{
-			All: opts.all,
+			All: options.all,
-		}
+		})
-		cs, err := dockerCli.Client().ContainerList(ctx, options)
 		if err != nil {
 			closeChan <- err
 		}
@@ -119,24 +117,24 @@ func runStats(ctx context.Context, dockerCli command.Cli, opts *statsOptions) er
 			s := NewStats(ctr.ID[:12])
 			if cStats.add(s) {
 				waitFirst.Add(1)
-				go collect(ctx, s, dockerCli.Client(), !opts.noStream, waitFirst)
+				go collect(ctx, s, apiClient, !options.noStream, waitFirst)
 			}
 		}
 	}
 
 	if showAll {
-		// If no names were specified, start a long running goroutine which
+		// If no names were specified, start a long-running goroutine which
 		// monitors container events. We make sure we're subscribed before
 		// retrieving the list of running containers to avoid a race where we
 		// would "miss" a creation.
 		started := make(chan struct{})
 		eh := command.InitEventHandler()
 		eh.Handle(events.ActionCreate, func(e events.Message) {
-			if opts.all {
+			if options.all {
 				s := NewStats(e.Actor.ID[:12])
 				if cStats.add(s) {
 					waitFirst.Add(1)
-					go collect(ctx, s, dockerCli.Client(), !opts.noStream, waitFirst)
+					go collect(ctx, s, apiClient, !options.noStream, waitFirst)
 				}
 			}
 		})
@@ -145,12 +143,12 @@ func runStats(ctx context.Context, dockerCli command.Cli, opts *statsOptions) er
 			s := NewStats(e.Actor.ID[:12])
 			if cStats.add(s) {
 				waitFirst.Add(1)
-				go collect(ctx, s, dockerCli.Client(), !opts.noStream, waitFirst)
+				go collect(ctx, s, apiClient, !options.noStream, waitFirst)
 			}
 		})
 
 		eh.Handle(events.ActionDie, func(e events.Message) {
-			if !opts.all {
+			if !options.all {
 				cStats.remove(e.Actor.ID[:12])
 			}
 		})
@@ -171,11 +169,11 @@ func runStats(ctx context.Context, dockerCli command.Cli, opts *statsOptions) er
 	} else {
 		// Artificially send creation events for the containers we were asked to
 		// monitor (same code path than we use when monitoring all containers).
-		for _, name := range opts.containers {
+		for _, name := range options.containers {
 			s := NewStats(name)
 			if cStats.add(s) {
 				waitFirst.Add(1)
-				go collect(ctx, s, dockerCli.Client(), !opts.noStream, waitFirst)
+				go collect(ctx, s, apiClient, !options.noStream, waitFirst)
 			}
 		}
 
@@ -198,22 +196,22 @@ func runStats(ctx context.Context, dockerCli command.Cli, opts *statsOptions) er
 		}
 	}
 
-	format := opts.format
+	format := options.format
 	if len(format) == 0 {
-		if len(dockerCli.ConfigFile().StatsFormat) > 0 {
+		if len(dockerCLI.ConfigFile().StatsFormat) > 0 {
-			format = dockerCli.ConfigFile().StatsFormat
+			format = dockerCLI.ConfigFile().StatsFormat
 		} else {
 			format = formatter.TableFormatKey
 		}
 	}
 	statsCtx := formatter.Context{
-		Output: dockerCli.Out(),
+		Output: dockerCLI.Out(),
 		Format: NewStatsFormat(format, daemonOSType),
 	}
 	cleanScreen := func() {
-		if !opts.noStream {
+		if !options.noStream {
-			fmt.Fprint(dockerCli.Out(), "\033[2J")
+			_, _ = fmt.Fprint(dockerCLI.Out(), "\033[2J")
-			fmt.Fprint(dockerCli.Out(), "\033[H")
+			_, _ = fmt.Fprint(dockerCLI.Out(), "\033[H")
 		}
 	}
 
@@ -222,28 +220,28 @@ func runStats(ctx context.Context, dockerCli command.Cli, opts *statsOptions) er
 	defer ticker.Stop()
 	for range ticker.C {
 		cleanScreen()
-		ccstats := []StatsEntry{}
+		var ccStats []StatsEntry
 		cStats.mu.RLock()
 		for _, c := range cStats.cs {
-			ccstats = append(ccstats, c.GetStatistics())
+			ccStats = append(ccStats, c.GetStatistics())
 		}
 		cStats.mu.RUnlock()
-		if err = statsFormatWrite(statsCtx, ccstats, daemonOSType, !opts.noTrunc); err != nil {
+		if err = statsFormatWrite(statsCtx, ccStats, daemonOSType, !options.noTrunc); err != nil {
 			break
 		}
 		if len(cStats.cs) == 0 && !showAll {
 			break
 		}
-		if opts.noStream {
+		if options.noStream {
 			break
 		}
 		select {
 		case err, ok := <-closeChan:
 			if ok {
 				if err != nil {
-					// this is suppressing "unexpected EOF" in the cli when the
+					// Suppress "unexpected EOF" errors in the CLI so that
-					// daemon restarts so it shutdowns cleanly
+					// it shuts down cleanly when the daemon restarts.
-					if err == io.ErrUnexpectedEOF {
+					if errors.Is(err, io.ErrUnexpectedEOF) {
 						return nil
 					}
 					return err