From 2389768fb7c6738cceab5ac50dcf6cc8ec7c2c33 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Wed, 20 Dec 2023 11:23:05 +0100 Subject: [PATCH] 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 --- cli/command/container/stats.go | 92 +++++++++++++++++----------------- 1 file changed, 45 insertions(+), 47 deletions(-) diff --git a/cli/command/container/stats.go b/cli/command/container/stats.go index 1e20445a5d..a972c41225 100644 --- a/cli/command/container/stats.go +++ b/cli/command/container/stats.go @@ -29,29 +29,29 @@ type statsOptions struct { containers []string } -// NewStatsCommand creates a new cobra.Command for `docker stats` -func NewStatsCommand(dockerCli command.Cli) *cobra.Command { - var opts statsOptions +// NewStatsCommand creates a new [cobra.Command] for "docker stats". +func NewStatsCommand(dockerCLI command.Cli) *cobra.Command { + 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 - return runStats(cmd.Context(), dockerCli, &opts) + options.containers = args + 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.BoolVar(&opts.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.StringVar(&opts.format, "format", "", flagsHelper.FormatHelp) + flags.BoolVarP(&options.all, "all", "a", false, "Show all containers (default shows just running)") + flags.BoolVar(&options.noStream, "no-stream", false, "Disable streaming stats and only pull the first result") + flags.BoolVar(&options.noTrunc, "no-trunc", false, "Do not truncate output") + 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 { - showAll := len(opts.containers) == 0 +func runStats(ctx context.Context, dockerCLI command.Cli, options *statsOptions) error { + 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") - options := types.EventsOptions{ + f.Add("type", string(events.ContainerEventType)) + eventChan, errChan := apiClient.Events(ctx, types.EventsOptions{ Filters: f, - } + }) - eventq, errq := dockerCli.Client().Events(ctx, options) - - // Whether we successfully subscribed to eventq or not, we can now + // Whether we successfully subscribed to eventChan 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{ - All: opts.all, - } - cs, err := dockerCli.Client().ContainerList(ctx, options) + cs, err := apiClient.ContainerList(ctx, container.ListOptions{ + All: options.all, + }) 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 { - format = dockerCli.ConfigFile().StatsFormat + if len(dockerCLI.ConfigFile().StatsFormat) > 0 { + 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 { - fmt.Fprint(dockerCli.Out(), "\033[2J") - fmt.Fprint(dockerCli.Out(), "\033[H") + if !options.noStream { + _, _ = fmt.Fprint(dockerCLI.Out(), "\033[2J") + _, _ = 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 - // daemon restarts so it shutdowns cleanly - if err == io.ErrUnexpectedEOF { + // Suppress "unexpected EOF" errors in the CLI so that + // it shuts down cleanly when the daemon restarts. + if errors.Is(err, io.ErrUnexpectedEOF) { return nil } return err