diff --git a/cli/command/config/remove.go b/cli/command/config/remove.go index cd5403db05..01cbe331c1 100644 --- a/cli/command/config/remove.go +++ b/cli/command/config/remove.go @@ -2,12 +2,11 @@ package config import ( "context" + "errors" "fmt" - "strings" "github.com/docker/cli/cli" "github.com/docker/cli/cli/command" - "github.com/pkg/errors" "github.com/spf13/cobra" ) @@ -35,23 +34,17 @@ func newConfigRemoveCommand(dockerCli command.Cli) *cobra.Command { } // RunConfigRemove removes the given Swarm configs. -func RunConfigRemove(ctx context.Context, dockerCli command.Cli, opts RemoveOptions) error { - client := dockerCli.Client() - - var errs []string +func RunConfigRemove(ctx context.Context, dockerCLI command.Cli, opts RemoveOptions) error { + apiClient := dockerCLI.Client() + var errs []error for _, name := range opts.Names { - if err := client.ConfigRemove(ctx, name); err != nil { - errs = append(errs, err.Error()) + if err := apiClient.ConfigRemove(ctx, name); err != nil { + errs = append(errs, err) continue } - - fmt.Fprintln(dockerCli.Out(), name) + _, _ = fmt.Fprintln(dockerCLI.Out(), name) } - if len(errs) > 0 { - return errors.Errorf("%s", strings.Join(errs, "\n")) - } - - return nil + return errors.Join(errs...) } diff --git a/cli/command/container/kill.go b/cli/command/container/kill.go index 0095198b5a..3d5c599418 100644 --- a/cli/command/container/kill.go +++ b/cli/command/container/kill.go @@ -2,13 +2,12 @@ package container import ( "context" + "errors" "fmt" - "strings" "github.com/docker/cli/cli" "github.com/docker/cli/cli/command" "github.com/docker/cli/cli/command/completion" - "github.com/pkg/errors" "github.com/spf13/cobra" ) @@ -44,20 +43,19 @@ func NewKillCommand(dockerCli command.Cli) *cobra.Command { return cmd } -func runKill(ctx context.Context, dockerCli command.Cli, opts *killOptions) error { - var errs []string +func runKill(ctx context.Context, dockerCLI command.Cli, opts *killOptions) error { + apiClient := dockerCLI.Client() errChan := parallelOperation(ctx, opts.containers, func(ctx context.Context, container string) error { - return dockerCli.Client().ContainerKill(ctx, container, opts.signal) + return apiClient.ContainerKill(ctx, container, opts.signal) }) + + var errs []error for _, name := range opts.containers { if err := <-errChan; err != nil { - errs = append(errs, err.Error()) - } else { - _, _ = fmt.Fprintln(dockerCli.Out(), name) + errs = append(errs, err) + continue } + _, _ = fmt.Fprintln(dockerCLI.Out(), name) } - if len(errs) > 0 { - return errors.New(strings.Join(errs, "\n")) - } - return nil + return errors.Join(errs...) } diff --git a/cli/command/container/pause.go b/cli/command/container/pause.go index 87fb0e10c5..b2a40aa705 100644 --- a/cli/command/container/pause.go +++ b/cli/command/container/pause.go @@ -2,14 +2,13 @@ package container import ( "context" + "errors" "fmt" - "strings" "github.com/docker/cli/cli" "github.com/docker/cli/cli/command" "github.com/docker/cli/cli/command/completion" "github.com/docker/docker/api/types/container" - "github.com/pkg/errors" "github.com/spf13/cobra" ) @@ -38,18 +37,17 @@ func NewPauseCommand(dockerCli command.Cli) *cobra.Command { } } -func runPause(ctx context.Context, dockerCli command.Cli, opts *pauseOptions) error { - var errs []string - errChan := parallelOperation(ctx, opts.containers, dockerCli.Client().ContainerPause) +func runPause(ctx context.Context, dockerCLI command.Cli, opts *pauseOptions) error { + apiClient := dockerCLI.Client() + errChan := parallelOperation(ctx, opts.containers, apiClient.ContainerPause) + + var errs []error for _, ctr := range opts.containers { if err := <-errChan; err != nil { - errs = append(errs, err.Error()) + errs = append(errs, err) continue } - _, _ = fmt.Fprintln(dockerCli.Out(), ctr) + _, _ = fmt.Fprintln(dockerCLI.Out(), ctr) } - if len(errs) > 0 { - return errors.New(strings.Join(errs, "\n")) - } - return nil + return errors.Join(errs...) } diff --git a/cli/command/container/restart.go b/cli/command/container/restart.go index 5460ff5228..379b6a12eb 100644 --- a/cli/command/container/restart.go +++ b/cli/command/container/restart.go @@ -2,14 +2,13 @@ package container import ( "context" + "errors" "fmt" - "strings" "github.com/docker/cli/cli" "github.com/docker/cli/cli/command" "github.com/docker/cli/cli/command/completion" "github.com/docker/docker/api/types/container" - "github.com/pkg/errors" "github.com/spf13/cobra" ) @@ -56,27 +55,25 @@ func NewRestartCommand(dockerCli command.Cli) *cobra.Command { return cmd } -func runRestart(ctx context.Context, dockerCli command.Cli, opts *restartOptions) error { - var errs []string +func runRestart(ctx context.Context, dockerCLI command.Cli, opts *restartOptions) error { var timeout *int if opts.timeoutChanged { timeout = &opts.timeout } + apiClient := dockerCLI.Client() + var errs []error // TODO(thaJeztah): consider using parallelOperation for restart, similar to "stop" and "remove" for _, name := range opts.containers { - err := dockerCli.Client().ContainerRestart(ctx, name, container.StopOptions{ + err := apiClient.ContainerRestart(ctx, name, container.StopOptions{ Signal: opts.signal, Timeout: timeout, }) if err != nil { - errs = append(errs, err.Error()) + errs = append(errs, err) continue } - _, _ = fmt.Fprintln(dockerCli.Out(), name) + _, _ = fmt.Fprintln(dockerCLI.Out(), name) } - if len(errs) > 0 { - return errors.New(strings.Join(errs, "\n")) - } - return nil + return errors.Join(errs...) } diff --git a/cli/command/container/rm.go b/cli/command/container/rm.go index 12beef64f6..627b042f0b 100644 --- a/cli/command/container/rm.go +++ b/cli/command/container/rm.go @@ -2,6 +2,7 @@ package container import ( "context" + "errors" "fmt" "strings" @@ -10,7 +11,6 @@ import ( "github.com/docker/cli/cli/command/completion" "github.com/docker/docker/api/types/container" "github.com/docker/docker/errdefs" - "github.com/pkg/errors" "github.com/spf13/cobra" ) @@ -50,33 +50,31 @@ func NewRmCommand(dockerCli command.Cli) *cobra.Command { return cmd } -func runRm(ctx context.Context, dockerCli command.Cli, opts *rmOptions) error { - var errs []string +func runRm(ctx context.Context, dockerCLI command.Cli, opts *rmOptions) error { + apiClient := dockerCLI.Client() errChan := parallelOperation(ctx, opts.containers, func(ctx context.Context, ctrID string) error { ctrID = strings.Trim(ctrID, "/") if ctrID == "" { - return errors.New("Container name cannot be empty") + return errors.New("container name cannot be empty") } - return dockerCli.Client().ContainerRemove(ctx, ctrID, container.RemoveOptions{ + return apiClient.ContainerRemove(ctx, ctrID, container.RemoveOptions{ RemoveVolumes: opts.rmVolumes, RemoveLinks: opts.rmLink, Force: opts.force, }) }) + var errs []error for _, name := range opts.containers { if err := <-errChan; err != nil { if opts.force && errdefs.IsNotFound(err) { - fmt.Fprintln(dockerCli.Err(), err) + _, _ = fmt.Fprintln(dockerCLI.Err(), err) continue } - errs = append(errs, err.Error()) + errs = append(errs, err) continue } - fmt.Fprintln(dockerCli.Out(), name) + _, _ = fmt.Fprintln(dockerCLI.Out(), name) } - if len(errs) > 0 { - return errors.New(strings.Join(errs, "\n")) - } - return nil + return errors.Join(errs...) } diff --git a/cli/command/container/stats.go b/cli/command/container/stats.go index 76f7765cd6..5538e61dce 100644 --- a/cli/command/container/stats.go +++ b/cli/command/container/stats.go @@ -3,6 +3,7 @@ package container import ( "bytes" "context" + "errors" "fmt" "io" "strings" @@ -17,7 +18,6 @@ import ( "github.com/docker/docker/api/types/container" "github.com/docker/docker/api/types/events" "github.com/docker/docker/api/types/filters" - "github.com/pkg/errors" "github.com/sirupsen/logrus" "github.com/spf13/cobra" ) @@ -238,16 +238,16 @@ func RunStats(ctx context.Context, dockerCLI command.Cli, options *StatsOptions) // make sure each container get at least one valid stat data waitFirst.Wait() - var errs []string + var errs []error cStats.mu.RLock() for _, c := range cStats.cs { if err := c.GetError(); err != nil { - errs = append(errs, err.Error()) + errs = append(errs, err) } } cStats.mu.RUnlock() - if len(errs) > 0 { - return errors.New(strings.Join(errs, "\n")) + if err := errors.Join(errs...); err != nil { + return err } } diff --git a/cli/command/container/stop.go b/cli/command/container/stop.go index af24f43caf..c6b331e964 100644 --- a/cli/command/container/stop.go +++ b/cli/command/container/stop.go @@ -2,14 +2,13 @@ package container import ( "context" + "errors" "fmt" - "strings" "github.com/docker/cli/cli" "github.com/docker/cli/cli/command" "github.com/docker/cli/cli/command/completion" "github.com/docker/docker/api/types/container" - "github.com/pkg/errors" "github.com/spf13/cobra" ) @@ -56,28 +55,26 @@ func NewStopCommand(dockerCli command.Cli) *cobra.Command { return cmd } -func runStop(ctx context.Context, dockerCli command.Cli, opts *stopOptions) error { +func runStop(ctx context.Context, dockerCLI command.Cli, opts *stopOptions) error { var timeout *int if opts.timeoutChanged { timeout = &opts.timeout } + apiClient := dockerCLI.Client() errChan := parallelOperation(ctx, opts.containers, func(ctx context.Context, id string) error { - return dockerCli.Client().ContainerStop(ctx, id, container.StopOptions{ + return apiClient.ContainerStop(ctx, id, container.StopOptions{ Signal: opts.signal, Timeout: timeout, }) }) - var errs []string + var errs []error for _, ctr := range opts.containers { if err := <-errChan; err != nil { - errs = append(errs, err.Error()) + errs = append(errs, err) continue } - _, _ = fmt.Fprintln(dockerCli.Out(), ctr) + _, _ = fmt.Fprintln(dockerCLI.Out(), ctr) } - if len(errs) > 0 { - return errors.New(strings.Join(errs, "\n")) - } - return nil + return errors.Join(errs...) } diff --git a/cli/command/container/unpause.go b/cli/command/container/unpause.go index cffd94d4c2..7824f4a76f 100644 --- a/cli/command/container/unpause.go +++ b/cli/command/container/unpause.go @@ -2,14 +2,13 @@ package container import ( "context" + "errors" "fmt" - "strings" "github.com/docker/cli/cli" "github.com/docker/cli/cli/command" "github.com/docker/cli/cli/command/completion" "github.com/docker/docker/api/types/container" - "github.com/pkg/errors" "github.com/spf13/cobra" ) @@ -39,18 +38,16 @@ func NewUnpauseCommand(dockerCli command.Cli) *cobra.Command { return cmd } -func runUnpause(ctx context.Context, dockerCli command.Cli, opts *unpauseOptions) error { - var errs []string - errChan := parallelOperation(ctx, opts.containers, dockerCli.Client().ContainerUnpause) +func runUnpause(ctx context.Context, dockerCLI command.Cli, opts *unpauseOptions) error { + apiClient := dockerCLI.Client() + errChan := parallelOperation(ctx, opts.containers, apiClient.ContainerUnpause) + var errs []error for _, ctr := range opts.containers { if err := <-errChan; err != nil { - errs = append(errs, err.Error()) + errs = append(errs, err) continue } - _, _ = fmt.Fprintln(dockerCli.Out(), ctr) + _, _ = fmt.Fprintln(dockerCLI.Out(), ctr) } - if len(errs) > 0 { - return errors.New(strings.Join(errs, "\n")) - } - return nil + return errors.Join(errs...) } diff --git a/cli/command/container/wait.go b/cli/command/container/wait.go index 8eb7d82f80..89f2ba17ac 100644 --- a/cli/command/container/wait.go +++ b/cli/command/container/wait.go @@ -2,13 +2,12 @@ package container import ( "context" + "errors" "fmt" - "strings" "github.com/docker/cli/cli" "github.com/docker/cli/cli/command" "github.com/docker/cli/cli/command/completion" - "github.com/pkg/errors" "github.com/spf13/cobra" ) @@ -37,20 +36,19 @@ func NewWaitCommand(dockerCli command.Cli) *cobra.Command { return cmd } -func runWait(ctx context.Context, dockerCli command.Cli, opts *waitOptions) error { - var errs []string +func runWait(ctx context.Context, dockerCLI command.Cli, opts *waitOptions) error { + apiClient := dockerCLI.Client() + + var errs []error for _, ctr := range opts.containers { - resultC, errC := dockerCli.Client().ContainerWait(ctx, ctr, "") + resultC, errC := apiClient.ContainerWait(ctx, ctr, "") select { case result := <-resultC: - _, _ = fmt.Fprintf(dockerCli.Out(), "%d\n", result.StatusCode) + _, _ = fmt.Fprintf(dockerCLI.Out(), "%d\n", result.StatusCode) case err := <-errC: - errs = append(errs, err.Error()) + errs = append(errs, err) } } - if len(errs) > 0 { - return errors.New(strings.Join(errs, "\n")) - } - return nil + return errors.Join(errs...) } diff --git a/cli/command/context/options.go b/cli/command/context/options.go index 7b39f7d768..b2c8a4b832 100644 --- a/cli/command/context/options.go +++ b/cli/command/context/options.go @@ -1,14 +1,14 @@ package context import ( + "errors" + "fmt" "strconv" - "strings" "github.com/docker/cli/cli/context" "github.com/docker/cli/cli/context/docker" "github.com/docker/cli/cli/context/store" "github.com/docker/docker/client" - "github.com/pkg/errors" ) const ( @@ -68,20 +68,17 @@ func parseBool(config map[string]string, name string) (bool, error) { return false, nil } res, err := strconv.ParseBool(strVal) - return res, errors.Wrap(err, name) + return res, fmt.Errorf("name: %w", err) } func validateConfig(config map[string]string, allowedKeys map[string]struct{}) error { - var errs []string + var errs []error for k := range config { if _, ok := allowedKeys[k]; !ok { - errs = append(errs, "unrecognized config key: "+k) + errs = append(errs, errors.New("unrecognized config key: "+k)) } } - if len(errs) == 0 { - return nil - } - return errors.New(strings.Join(errs, "\n")) + return errors.Join(errs...) } func getDockerEndpoint(contextStore store.Reader, config map[string]string) (docker.Endpoint, error) { @@ -96,7 +93,7 @@ func getDockerEndpoint(contextStore store.Reader, config map[string]string) (doc if ep, ok := metadata.Endpoints[docker.DockerEndpoint].(docker.EndpointMeta); ok { return docker.Endpoint{EndpointMeta: ep}, nil } - return docker.Endpoint{}, errors.Errorf("unable to get endpoint from context %q", contextName) + return docker.Endpoint{}, fmt.Errorf("unable to get endpoint from context %q", contextName) } tlsData, err := context.TLSDataFromFiles(config[keyCA], config[keyCert], config[keyKey]) if err != nil { @@ -116,10 +113,11 @@ func getDockerEndpoint(contextStore store.Reader, config map[string]string) (doc // try to resolve a docker client, validating the configuration opts, err := ep.ClientOpts() if err != nil { - return docker.Endpoint{}, errors.Wrap(err, "invalid docker endpoint options") + return docker.Endpoint{}, fmt.Errorf("invalid docker endpoint options: %w", err) } + // FIXME(thaJeztah): this creates a new client (but discards it) only to validate the options; are the validation steps above not enough? if _, err := client.NewClientWithOpts(opts...); err != nil { - return docker.Endpoint{}, errors.Wrap(err, "unable to apply docker endpoint options") + return docker.Endpoint{}, fmt.Errorf("unable to apply docker endpoint options: %w", err) } return ep, nil } diff --git a/cli/command/context/remove.go b/cli/command/context/remove.go index 9ba405d588..bcf17a0537 100644 --- a/cli/command/context/remove.go +++ b/cli/command/context/remove.go @@ -1,14 +1,13 @@ package context import ( + "errors" "fmt" "os" - "strings" "github.com/docker/cli/cli" "github.com/docker/cli/cli/command" "github.com/docker/docker/errdefs" - "github.com/pkg/errors" "github.com/spf13/cobra" ) @@ -33,28 +32,25 @@ func newRemoveCommand(dockerCli command.Cli) *cobra.Command { } // RunRemove removes one or more contexts -func RunRemove(dockerCli command.Cli, opts RemoveOptions, names []string) error { - var errs []string - currentCtx := dockerCli.CurrentContext() +func RunRemove(dockerCLI command.Cli, opts RemoveOptions, names []string) error { + var errs []error + currentCtx := dockerCLI.CurrentContext() for _, name := range names { if name == "default" { - errs = append(errs, `default: context "default" cannot be removed`) - } else if err := doRemove(dockerCli, name, name == currentCtx, opts.Force); err != nil { - errs = append(errs, err.Error()) + errs = append(errs, errors.New(`context "default" cannot be removed`)) + } else if err := doRemove(dockerCLI, name, name == currentCtx, opts.Force); err != nil { + errs = append(errs, err) } else { - fmt.Fprintln(dockerCli.Out(), name) + _, _ = fmt.Fprintln(dockerCLI.Out(), name) } } - if len(errs) > 0 { - return errors.New(strings.Join(errs, "\n")) - } - return nil + return errors.Join(errs...) } func doRemove(dockerCli command.Cli, name string, isCurrent, force bool) error { if isCurrent { if !force { - return errors.Errorf("context %q is in use, set -f flag to force remove", name) + return fmt.Errorf("context %q is in use, set -f flag to force remove", name) } // fallback to DOCKER_HOST cfg := dockerCli.ConfigFile() @@ -65,6 +61,7 @@ func doRemove(dockerCli command.Cli, name string, isCurrent, force bool) error { } if !force { + // TODO(thaJeztah): instead of checking before removing, can we make ContextStore().Remove() return a proper errdef and ignore "not found" errors? if err := checkContextExists(dockerCli, name); err != nil { return err } @@ -77,7 +74,7 @@ func checkContextExists(dockerCli command.Cli, name string) error { contextDir := dockerCli.ContextStore().GetStorageInfo(name).MetadataPath _, err := os.Stat(contextDir) if os.IsNotExist(err) { - return errdefs.NotFound(errors.Errorf("context %q does not exist", name)) + return errdefs.NotFound(fmt.Errorf("context %q does not exist", name)) } // Ignore other errors; if relevant, they will produce an error when // performing the actual delete. diff --git a/cli/command/context/remove_test.go b/cli/command/context/remove_test.go index ab2f8ba216..5f48e740c5 100644 --- a/cli/command/context/remove_test.go +++ b/cli/command/context/remove_test.go @@ -60,5 +60,5 @@ func TestRemoveDefault(t *testing.T) { createTestContext(t, cli, "other", nil) cli.SetCurrentContext("current") err := RunRemove(cli, RemoveOptions{}, []string{"default"}) - assert.ErrorContains(t, err, `default: context "default" cannot be removed`) + assert.ErrorContains(t, err, `context "default" cannot be removed`) } diff --git a/cli/command/image/remove.go b/cli/command/image/remove.go index 067e0541b6..4a17bfcdb0 100644 --- a/cli/command/image/remove.go +++ b/cli/command/image/remove.go @@ -2,15 +2,14 @@ package image import ( "context" + "errors" "fmt" - "strings" "github.com/docker/cli/cli" "github.com/docker/cli/cli/command" "github.com/docker/cli/cli/command/completion" "github.com/docker/docker/api/types/image" "github.com/docker/docker/errdefs" - "github.com/pkg/errors" "github.com/spf13/cobra" ) @@ -59,15 +58,16 @@ func runRemove(ctx context.Context, dockerCLI command.Cli, opts removeOptions, i PruneChildren: !opts.noPrune, } - var errs []string + // TODO(thaJeztah): this logic can likely be simplified: do we want to print "not found" errors at all when using "force"? fatalErr := false + var errs []error for _, img := range images { dels, err := apiClient.ImageRemove(ctx, img, options) if err != nil { if !errdefs.IsNotFound(err) { fatalErr = true } - errs = append(errs, err.Error()) + errs = append(errs, err) } else { for _, del := range dels { if del.Deleted != "" { @@ -79,12 +79,11 @@ func runRemove(ctx context.Context, dockerCLI command.Cli, opts removeOptions, i } } - if len(errs) > 0 { - msg := strings.Join(errs, "\n") + if err := errors.Join(errs...); err != nil { if !opts.force || fatalErr { - return errors.New(msg) + return err } - _, _ = fmt.Fprintln(dockerCLI.Err(), msg) + _, _ = fmt.Fprintln(dockerCLI.Err(), err) } return nil } diff --git a/cli/command/inspect/inspector.go b/cli/command/inspect/inspector.go index b46f896df8..0e39616251 100644 --- a/cli/command/inspect/inspector.go +++ b/cli/command/inspect/inspector.go @@ -6,13 +6,13 @@ package inspect import ( "bytes" "encoding/json" + "errors" + "fmt" "io" - "strings" "text/template" "github.com/docker/cli/cli" "github.com/docker/cli/templates" - "github.com/pkg/errors" "github.com/sirupsen/logrus" ) @@ -53,7 +53,7 @@ func NewTemplateInspectorFromString(out io.Writer, tmplStr string) (Inspector, e tmpl, err := templates.Parse(tmplStr) if err != nil { - return nil, errors.Errorf("template parsing error: %s", err) + return nil, fmt.Errorf("template parsing error: %w", err) } return NewTemplateInspector(out, tmpl), nil } @@ -70,16 +70,16 @@ func Inspect(out io.Writer, references []string, tmplStr string, getRef GetRefFu return cli.StatusError{StatusCode: 64, Status: err.Error()} } - var inspectErrs []string + var errs []error for _, ref := range references { element, raw, err := getRef(ref) if err != nil { - inspectErrs = append(inspectErrs, err.Error()) + errs = append(errs, err) continue } if err := inspector.Inspect(element, raw); err != nil { - inspectErrs = append(inspectErrs, err.Error()) + errs = append(errs, err) } } @@ -87,10 +87,10 @@ func Inspect(out io.Writer, references []string, tmplStr string, getRef GetRefFu logrus.Error(err) } - if len(inspectErrs) != 0 { + if err := errors.Join(errs...); err != nil { return cli.StatusError{ StatusCode: 1, - Status: strings.Join(inspectErrs, "\n"), + Status: err.Error(), } } return nil @@ -103,7 +103,7 @@ func (i *TemplateInspector) Inspect(typedElement any, rawElement []byte) error { buffer := new(bytes.Buffer) if err := i.tmpl.Execute(buffer, typedElement); err != nil { if rawElement == nil { - return errors.Errorf("template parsing error: %v", err) + return fmt.Errorf("template parsing error: %w", err) } return i.tryRawInspectFallback(rawElement) } @@ -121,13 +121,13 @@ func (i *TemplateInspector) tryRawInspectFallback(rawElement []byte) error { dec := json.NewDecoder(rdr) dec.UseNumber() - if rawErr := dec.Decode(&raw); rawErr != nil { - return errors.Errorf("unable to read inspect data: %v", rawErr) + if err := dec.Decode(&raw); err != nil { + return fmt.Errorf("unable to read inspect data: %w", err) } tmplMissingKey := i.tmpl.Option("missingkey=error") - if rawErr := tmplMissingKey.Execute(buffer, raw); rawErr != nil { - return errors.Errorf("template parsing error: %v", rawErr) + if err := tmplMissingKey.Execute(buffer, raw); err != nil { + return fmt.Errorf("template parsing error: %w", err) } i.buffer.Write(buffer.Bytes()) diff --git a/cli/command/manifest/rm.go b/cli/command/manifest/rm.go index 2e2dc04624..e59b716cd3 100644 --- a/cli/command/manifest/rm.go +++ b/cli/command/manifest/rm.go @@ -2,47 +2,47 @@ package manifest import ( "context" - "strings" + "errors" "github.com/docker/cli/cli" "github.com/docker/cli/cli/command" - "github.com/pkg/errors" + manifeststore "github.com/docker/cli/cli/manifest/store" "github.com/spf13/cobra" ) -func newRmManifestListCommand(dockerCli command.Cli) *cobra.Command { +func newRmManifestListCommand(dockerCLI command.Cli) *cobra.Command { cmd := &cobra.Command{ Use: "rm MANIFEST_LIST [MANIFEST_LIST...]", Short: "Delete one or more manifest lists from local storage", Args: cli.RequiresMinArgs(1), RunE: func(cmd *cobra.Command, args []string) error { - return runRm(cmd.Context(), dockerCli, args) + return runRemove(cmd.Context(), dockerCLI.ManifestStore(), args) }, } return cmd } -func runRm(_ context.Context, dockerCli command.Cli, targets []string) error { - var errs []string +func runRemove(ctx context.Context, store manifeststore.Store, targets []string) error { + var errs []error for _, target := range targets { - targetRef, refErr := normalizeReference(target) - if refErr != nil { - errs = append(errs, refErr.Error()) + if ctx.Err() != nil { + return ctx.Err() + } + targetRef, err := normalizeReference(target) + if err != nil { + errs = append(errs, err) continue } - _, searchErr := dockerCli.ManifestStore().GetList(targetRef) - if searchErr != nil { - errs = append(errs, searchErr.Error()) + _, err = store.GetList(targetRef) + if err != nil { + errs = append(errs, err) continue } - rmErr := dockerCli.ManifestStore().Remove(targetRef) - if rmErr != nil { - errs = append(errs, rmErr.Error()) + err = store.Remove(targetRef) + if err != nil { + errs = append(errs, err) } } - if len(errs) > 0 { - return errors.New(strings.Join(errs, "\n")) - } - return nil + return errors.Join(errs...) } diff --git a/cli/command/node/remove.go b/cli/command/node/remove.go index 809c6b16fa..8e9460c95a 100644 --- a/cli/command/node/remove.go +++ b/cli/command/node/remove.go @@ -2,13 +2,12 @@ package node import ( "context" + "errors" "fmt" - "strings" "github.com/docker/cli/cli" "github.com/docker/cli/cli/command" "github.com/docker/docker/api/types" - "github.com/pkg/errors" "github.com/spf13/cobra" ) @@ -36,20 +35,13 @@ func newRemoveCommand(dockerCli command.Cli) *cobra.Command { func runRemove(ctx context.Context, dockerCLI command.Cli, nodeIDs []string, opts removeOptions) error { apiClient := dockerCLI.Client() - var errs []string - + var errs []error for _, id := range nodeIDs { - err := apiClient.NodeRemove(ctx, id, types.NodeRemoveOptions{Force: opts.force}) - if err != nil { - errs = append(errs, err.Error()) + if err := apiClient.NodeRemove(ctx, id, types.NodeRemoveOptions{Force: opts.force}); err != nil { + errs = append(errs, err) continue } _, _ = fmt.Fprintln(dockerCLI.Out(), id) } - - if len(errs) > 0 { - return errors.Errorf("%s", strings.Join(errs, "\n")) - } - - return nil + return errors.Join(errs...) } diff --git a/cli/command/secret/remove.go b/cli/command/secret/remove.go index e51d03d345..2a2764a8e3 100644 --- a/cli/command/secret/remove.go +++ b/cli/command/secret/remove.go @@ -2,12 +2,11 @@ package secret import ( "context" + "errors" "fmt" - "strings" "github.com/docker/cli/cli" "github.com/docker/cli/cli/command" - "github.com/pkg/errors" "github.com/spf13/cobra" ) @@ -15,7 +14,7 @@ type removeOptions struct { names []string } -func newSecretRemoveCommand(dockerCli command.Cli) *cobra.Command { +func newSecretRemoveCommand(dockerCLI command.Cli) *cobra.Command { return &cobra.Command{ Use: "rm SECRET [SECRET...]", Aliases: []string{"remove"}, @@ -25,31 +24,24 @@ func newSecretRemoveCommand(dockerCli command.Cli) *cobra.Command { opts := removeOptions{ names: args, } - return runSecretRemove(cmd.Context(), dockerCli, opts) + return runRemove(cmd.Context(), dockerCLI, opts) }, ValidArgsFunction: func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) { - return completeNames(dockerCli)(cmd, args, toComplete) + return completeNames(dockerCLI)(cmd, args, toComplete) }, } } -func runSecretRemove(ctx context.Context, dockerCli command.Cli, opts removeOptions) error { - client := dockerCli.Client() - - var errs []string +func runRemove(ctx context.Context, dockerCLI command.Cli, opts removeOptions) error { + apiClient := dockerCLI.Client() + var errs []error for _, name := range opts.names { - if err := client.SecretRemove(ctx, name); err != nil { - errs = append(errs, err.Error()) + if err := apiClient.SecretRemove(ctx, name); err != nil { + errs = append(errs, err) continue } - - fmt.Fprintln(dockerCli.Out(), name) + _, _ = fmt.Fprintln(dockerCLI.Out(), name) } - - if len(errs) > 0 { - return errors.Errorf("%s", strings.Join(errs, "\n")) - } - - return nil + return errors.Join(errs...) } diff --git a/cli/command/service/remove.go b/cli/command/service/remove.go index a191aa97bf..a07ee36bff 100644 --- a/cli/command/service/remove.go +++ b/cli/command/service/remove.go @@ -2,12 +2,11 @@ package service import ( "context" + "errors" "fmt" - "strings" "github.com/docker/cli/cli" "github.com/docker/cli/cli/command" - "github.com/pkg/errors" "github.com/spf13/cobra" ) @@ -32,17 +31,13 @@ func newRemoveCommand(dockerCli command.Cli) *cobra.Command { func runRemove(ctx context.Context, dockerCLI command.Cli, serviceIDs []string) error { apiClient := dockerCLI.Client() - var errs []string + var errs []error for _, id := range serviceIDs { - err := apiClient.ServiceRemove(ctx, id) - if err != nil { - errs = append(errs, err.Error()) + if err := apiClient.ServiceRemove(ctx, id); err != nil { + errs = append(errs, err) continue } _, _ = fmt.Fprintln(dockerCLI.Out(), id) } - if len(errs) > 0 { - return errors.New(strings.Join(errs, "\n")) - } - return nil + return errors.Join(errs...) } diff --git a/cli/command/service/rollback.go b/cli/command/service/rollback.go index f7efa2d498..156def02bb 100644 --- a/cli/command/service/rollback.go +++ b/cli/command/service/rollback.go @@ -42,12 +42,9 @@ func runRollback(ctx context.Context, dockerCLI command.Cli, options *serviceOpt return err } - spec := &service.Spec - updateOpts := types.ServiceUpdateOptions{ - Rollback: "previous", - } - - response, err := apiClient.ServiceUpdate(ctx, service.ID, service.Version, *spec, updateOpts) + response, err := apiClient.ServiceUpdate(ctx, service.ID, service.Version, service.Spec, types.ServiceUpdateOptions{ + Rollback: "previous", // TODO(thaJeztah): this should have a const defined + }) if err != nil { return err } diff --git a/cli/command/service/scale.go b/cli/command/service/scale.go index 7d34359b6e..17912e4cd0 100644 --- a/cli/command/service/scale.go +++ b/cli/command/service/scale.go @@ -2,6 +2,7 @@ package service import ( "context" + "errors" "fmt" "strconv" "strings" @@ -10,7 +11,7 @@ import ( "github.com/docker/cli/cli/command" "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/versions" - "github.com/pkg/errors" + "github.com/docker/docker/client" "github.com/spf13/cobra" ) @@ -44,8 +45,8 @@ func scaleArgs(cmd *cobra.Command, args []string) error { } for _, arg := range args { if k, v, ok := strings.Cut(arg, "="); !ok || k == "" || v == "" { - return errors.Errorf( - "Invalid scale specifier '%s'.\nSee '%s --help'.\n\nUsage: %s\n\n%s", + return fmt.Errorf( + "invalid scale specifier '%s'.\nSee '%s --help'.\n\nUsage: %s\n\n%s", arg, cmd.CommandPath(), cmd.UseLine(), @@ -56,49 +57,48 @@ func scaleArgs(cmd *cobra.Command, args []string) error { return nil } -func runScale(ctx context.Context, dockerCli command.Cli, options *scaleOptions, args []string) error { - var errs []string - var serviceIDs []string - +func runScale(ctx context.Context, dockerCLI command.Cli, options *scaleOptions, args []string) error { + apiClient := dockerCLI.Client() + var ( + errs []error + serviceIDs = make([]string, 0, len(args)) + ) for _, arg := range args { serviceID, scaleStr, _ := strings.Cut(arg, "=") // validate input arg scale number scale, err := strconv.ParseUint(scaleStr, 10, 64) if err != nil { - errs = append(errs, fmt.Sprintf("%s: invalid replicas value %s: %v", serviceID, scaleStr, err)) + errs = append(errs, fmt.Errorf("%s: invalid replicas value %s: %v", serviceID, scaleStr, err)) continue } - if err := runServiceScale(ctx, dockerCli, serviceID, scale); err != nil { - errs = append(errs, fmt.Sprintf("%s: %v", serviceID, err)) - } else { - serviceIDs = append(serviceIDs, serviceID) + warnings, err := runServiceScale(ctx, apiClient, serviceID, scale) + if err != nil { + errs = append(errs, fmt.Errorf("%s: %v", serviceID, err)) + continue } + for _, warning := range warnings { + _, _ = fmt.Fprintln(dockerCLI.Err(), warning) + } + _, _ = fmt.Fprintf(dockerCLI.Out(), "%s scaled to %d\n", serviceID, scale) + serviceIDs = append(serviceIDs, serviceID) } - if len(serviceIDs) > 0 { - if !options.detach && versions.GreaterThanOrEqualTo(dockerCli.Client().ClientVersion(), "1.29") { - for _, serviceID := range serviceIDs { - if err := WaitOnService(ctx, dockerCli, serviceID, false); err != nil { - errs = append(errs, fmt.Sprintf("%s: %v", serviceID, err)) - } + if len(serviceIDs) > 0 && !options.detach && versions.GreaterThanOrEqualTo(dockerCLI.Client().ClientVersion(), "1.29") { + for _, serviceID := range serviceIDs { + if err := WaitOnService(ctx, dockerCLI, serviceID, false); err != nil { + errs = append(errs, fmt.Errorf("%s: %v", serviceID, err)) } } } - - if len(errs) == 0 { - return nil - } - return errors.New(strings.Join(errs, "\n")) + return errors.Join(errs...) } -func runServiceScale(ctx context.Context, dockerCli command.Cli, serviceID string, scale uint64) error { - client := dockerCli.Client() - - service, _, err := client.ServiceInspectWithRaw(ctx, serviceID, types.ServiceInspectOptions{}) +func runServiceScale(ctx context.Context, apiClient client.ServiceAPIClient, serviceID string, scale uint64) (warnings []string, _ error) { + service, _, err := apiClient.ServiceInspectWithRaw(ctx, serviceID, types.ServiceInspectOptions{}) if err != nil { - return err + return nil, err } serviceMode := &service.Spec.Mode @@ -108,18 +108,12 @@ func runServiceScale(ctx context.Context, dockerCli command.Cli, serviceID strin case serviceMode.ReplicatedJob != nil: serviceMode.ReplicatedJob.TotalCompletions = &scale default: - return errors.Errorf("scale can only be used with replicated or replicated-job mode") + return nil, errors.New("scale can only be used with replicated or replicated-job mode") } - response, err := client.ServiceUpdate(ctx, service.ID, service.Version, service.Spec, types.ServiceUpdateOptions{}) + response, err := apiClient.ServiceUpdate(ctx, service.ID, service.Version, service.Spec, types.ServiceUpdateOptions{}) if err != nil { - return err + return nil, err } - - for _, warning := range response.Warnings { - _, _ = fmt.Fprintln(dockerCli.Err(), warning) - } - - _, _ = fmt.Fprintf(dockerCli.Out(), "%s scaled to %d\n", serviceID, scale) - return nil + return response.Warnings, nil } diff --git a/cli/command/stack/remove_test.go b/cli/command/stack/remove_test.go index ed5c9e4f8b..1a6923b747 100644 --- a/cli/command/stack/remove_test.go +++ b/cli/command/stack/remove_test.go @@ -161,7 +161,7 @@ func TestRemoveContinueAfterError(t *testing.T) { cmd.SetErr(io.Discard) cmd.SetArgs([]string{"foo", "bar"}) - assert.Error(t, cmd.Execute(), "Failed to remove some resources from stack: foo") + assert.Error(t, cmd.Execute(), "failed to remove some resources from stack: foo") assert.Check(t, is.DeepEqual(allServiceIDs, removedServices)) assert.Check(t, is.DeepEqual(allNetworkIDs, cli.removedNetworks)) assert.Check(t, is.DeepEqual(allSecretIDs, cli.removedSecrets)) diff --git a/cli/command/stack/swarm/remove.go b/cli/command/stack/swarm/remove.go index 8c16a7fc04..cd426d5111 100644 --- a/cli/command/stack/swarm/remove.go +++ b/cli/command/stack/swarm/remove.go @@ -2,9 +2,9 @@ package swarm import ( "context" + "errors" "fmt" "sort" - "strings" "github.com/docker/cli/cli/command" "github.com/docker/cli/cli/command/stack/options" @@ -12,14 +12,13 @@ import ( "github.com/docker/docker/api/types/swarm" "github.com/docker/docker/api/types/versions" "github.com/docker/docker/client" - "github.com/pkg/errors" ) // RunRemove is the swarm implementation of docker stack remove func RunRemove(ctx context.Context, dockerCli command.Cli, opts options.Remove) error { apiClient := dockerCli.Client() - var errs []string + var errs []error for _, namespace := range opts.Namespaces { services, err := getStackServices(ctx, apiClient, namespace) if err != nil { @@ -52,28 +51,25 @@ func RunRemove(ctx context.Context, dockerCli command.Cli, opts options.Remove) continue } + // TODO(thaJeztah): change this "hasError" boolean to return a (multi-)error for each of these functions instead. hasError := removeServices(ctx, dockerCli, services) hasError = removeSecrets(ctx, dockerCli, secrets) || hasError hasError = removeConfigs(ctx, dockerCli, configs) || hasError hasError = removeNetworks(ctx, dockerCli, networks) || hasError if hasError { - errs = append(errs, "Failed to remove some resources from stack: "+namespace) + errs = append(errs, errors.New("failed to remove some resources from stack: "+namespace)) continue } if !opts.Detach { err = waitOnTasks(ctx, apiClient, namespace) if err != nil { - errs = append(errs, fmt.Sprintf("Failed to wait on tasks of stack: %s: %s", namespace, err)) + errs = append(errs, fmt.Errorf("failed to wait on tasks of stack: %s: %w", namespace, err)) } } } - - if len(errs) > 0 { - return errors.New(strings.Join(errs, "\n")) - } - return nil + return errors.Join(errs...) } func sortServiceByName(services []swarm.Service) func(i, j int) bool { diff --git a/cli/command/volume/remove.go b/cli/command/volume/remove.go index 1eb3887347..eab894b02b 100644 --- a/cli/command/volume/remove.go +++ b/cli/command/volume/remove.go @@ -2,13 +2,12 @@ package volume import ( "context" + "errors" "fmt" - "strings" "github.com/docker/cli/cli" "github.com/docker/cli/cli/command" "github.com/docker/cli/cli/command/completion" - "github.com/pkg/errors" "github.com/spf13/cobra" ) @@ -43,18 +42,13 @@ func newRemoveCommand(dockerCli command.Cli) *cobra.Command { func runRemove(ctx context.Context, dockerCLI command.Cli, opts *removeOptions) error { apiClient := dockerCLI.Client() - var errs []string - + var errs []error for _, name := range opts.volumes { if err := apiClient.VolumeRemove(ctx, name, opts.force); err != nil { - errs = append(errs, err.Error()) + errs = append(errs, err) continue } _, _ = fmt.Fprintln(dockerCLI.Out(), name) } - - if len(errs) > 0 { - return errors.Errorf("%s", strings.Join(errs, "\n")) - } - return nil + return errors.Join(errs...) }