Skip to content

Commit

Permalink
Merge pull request #1971 from djdongjin/unify-multi-error
Browse files Browse the repository at this point in the history
Optimize id walkers and muti error handling
  • Loading branch information
AkihiroSuda authored Feb 9, 2023
2 parents d41705c + ff5f333 commit a1ed02a
Show file tree
Hide file tree
Showing 20 changed files with 145 additions and 217 deletions.
9 changes: 2 additions & 7 deletions cmd/nerdctl/container_stats.go
Original file line number Diff line number Diff line change
Expand Up @@ -277,13 +277,8 @@ func statsAction(cmd *cobra.Command, args []string) error {
},
}

for _, req := range args {
n, err := walker.Walk(ctx, req)
if err != nil {
return err
} else if n == 0 {
return fmt.Errorf("no such container %s", req)
}
if err := walker.WalkAll(ctx, args, false); err != nil {
return err
}

// make sure each container get at least one valid stat data
Expand Down
11 changes: 2 additions & 9 deletions cmd/nerdctl/container_update.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,15 +110,8 @@ func updateAction(cmd *cobra.Command, args []string) error {
return err
},
}
for _, req := range args {
n, err := walker.Walk(ctx, req)
if err != nil {
return err
} else if n == 0 {
return fmt.Errorf("no such container %s", req)
}
}
return nil

return walker.WalkAll(ctx, args, true)
}

func getUpdateOption(cmd *cobra.Command, globalOptions types.GlobalCommandOptions) (updateResourceOptions, error) {
Expand Down
14 changes: 1 addition & 13 deletions cmd/nerdctl/image_history.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,19 +143,7 @@ func historyAction(cmd *cobra.Command, args []string) error {
},
}

var errs []error
for _, req := range args {
n, err := walker.Walk(ctx, req)
if err != nil {
errs = append(errs, err)
} else if n == 0 {
errs = append(errs, fmt.Errorf("no such object: %s", req))
}
}
if len(errs) > 0 {
return fmt.Errorf("%d errors: %v", len(errs), errs)
}
return nil
return walker.WalkAll(ctx, args, true)
}

type historyPrinter struct {
Expand Down
18 changes: 6 additions & 12 deletions pkg/cmd/container/inspect.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
"github.com/containerd/nerdctl/pkg/formatter"
"github.com/containerd/nerdctl/pkg/idutil/containerwalker"
"github.com/containerd/nerdctl/pkg/inspecttypes/dockercompat"
"github.com/sirupsen/logrus"
)

// Inspect prints detailed information for each container in `containers`.
Expand All @@ -40,20 +41,13 @@ func Inspect(ctx context.Context, client *containerd.Client, containers []string
OnFound: f.Handler,
}

var errs []error
for _, req := range containers {
n, err := walker.Walk(ctx, req)
if err != nil {
errs = append(errs, err)
} else if n == 0 {
errs = append(errs, fmt.Errorf("no such container: %s", req))
err := walker.WalkAll(ctx, containers, true)
if len(f.entries) > 0 {
if formatErr := formatter.FormatSlice(options.Format, options.Stdout, f.entries); formatErr != nil {
logrus.Error(formatErr)
}
}
if len(errs) > 0 {
return fmt.Errorf("%d errors: %v", len(errs), errs)
}

return formatter.FormatSlice(options.Format, options.Stdout, f.entries)
return err
}

type containerInspector struct {
Expand Down
11 changes: 2 additions & 9 deletions pkg/cmd/container/kill.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,15 +60,8 @@ func Kill(ctx context.Context, client *containerd.Client, reqs []string, options
return err
},
}
for _, req := range reqs {
n, err := walker.Walk(ctx, req)
if err != nil {
return err
} else if n == 0 {
return fmt.Errorf("no such container %s", req)
}
}
return nil

return walker.WalkAll(ctx, reqs, true)
}

func killContainer(ctx context.Context, container containerd.Container, signal syscall.Signal) error {
Expand Down
17 changes: 1 addition & 16 deletions pkg/cmd/container/pause.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,7 @@ package container

import (
"context"
"errors"
"fmt"
"strings"

"github.com/containerd/containerd"
"github.com/containerd/nerdctl/pkg/api/types"
Expand All @@ -45,18 +43,5 @@ func Pause(ctx context.Context, client *containerd.Client, reqs []string, option
},
}

var errs []string
for _, req := range reqs {
n, err := walker.Walk(ctx, req)
if err != nil {
errs = append(errs, err.Error())
} else if n == 0 {
errs = append(errs, fmt.Sprintf("no such container %s", req))
}
}

if len(errs) > 0 {
return errors.New(strings.Join(errs, "\n"))
}
return nil
return walker.WalkAll(ctx, reqs, true)
}
19 changes: 6 additions & 13 deletions pkg/cmd/container/remove.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,20 +77,13 @@ func Remove(ctx context.Context, client *containerd.Client, containers []string,
return err
},
}
for _, req := range containers {
n, err := walker.Walk(ctx, req)
if err == nil && n == 0 {
err = fmt.Errorf("no such container %s", req)
}
if err != nil {
if options.Force {
logrus.Error(err)
} else {
return err
}
}

err := walker.WalkAll(ctx, containers, true)
if err != nil && options.Force {
logrus.Error(err)
return nil
}
return nil
return err
}

// RemoveContainer removes a container from containerd store.
Expand Down
10 changes: 1 addition & 9 deletions pkg/cmd/container/restart.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,14 +44,6 @@ func Restart(ctx context.Context, client *containerd.Client, containers []string
return err
},
}
for _, req := range containers {
n, err := walker.Walk(ctx, req)
if err != nil {
return err
} else if n == 0 {
return fmt.Errorf("no such container %s", req)
}
}

return nil
return walker.WalkAll(ctx, containers, true)
}
10 changes: 1 addition & 9 deletions pkg/cmd/container/start.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,13 +52,5 @@ func Start(ctx context.Context, client *containerd.Client, reqs []string, option
},
}

for _, req := range reqs {
n, err := walker.Walk(ctx, req)
if err != nil {
return err
} else if n == 0 {
return fmt.Errorf("no such container %s", req)
}
}
return nil
return walker.WalkAll(ctx, reqs, true)
}
12 changes: 3 additions & 9 deletions pkg/cmd/container/stop.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
"github.com/containerd/nerdctl/pkg/idutil/containerwalker"
)

// Stop stops a list of containers specified by `reqs`.
func Stop(ctx context.Context, client *containerd.Client, reqs []string, opt types.ContainerStopOptions) error {
walker := &containerwalker.ContainerWalker{
Client: client,
Expand All @@ -45,13 +46,6 @@ func Stop(ctx context.Context, client *containerd.Client, reqs []string, opt typ
return err
},
}
for _, req := range reqs {
n, err := walker.Walk(ctx, req)
if err != nil {
return err
} else if n == 0 {
return fmt.Errorf("no such container %s", req)
}
}
return nil

return walker.WalkAll(ctx, reqs, true)
}
17 changes: 1 addition & 16 deletions pkg/cmd/container/unpause.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,7 @@ package container

import (
"context"
"errors"
"fmt"
"strings"

"github.com/containerd/containerd"
"github.com/containerd/nerdctl/pkg/api/types"
Expand All @@ -45,18 +43,5 @@ func Unpause(ctx context.Context, client *containerd.Client, reqs []string, opti
},
}

var errs []string
for _, req := range reqs {
n, err := walker.Walk(ctx, req)
if err != nil {
errs = append(errs, err.Error())
} else if n == 0 {
errs = append(errs, fmt.Sprintf("no such container %s", req))
}
}

if len(errs) > 0 {
return errors.New(strings.Join(errs, "\n"))
}
return nil
return walker.WalkAll(ctx, reqs, true)
}
12 changes: 4 additions & 8 deletions pkg/cmd/container/wait.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,15 +41,11 @@ func Wait(ctx context.Context, client *containerd.Client, reqs []string, options
},
}

for _, req := range reqs {
n, err := walker.Walk(ctx, req)
if err != nil {
return err
}
if n == 0 {
return fmt.Errorf("no such container: %s", req)
}
// check if all containers from `reqs` exist
if err := walker.WalkAll(ctx, reqs, false); err != nil {
return err
}

var allErr error
w := options.Stdout
for _, container := range containers {
Expand Down
17 changes: 6 additions & 11 deletions pkg/cmd/image/inspect.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
"github.com/containerd/nerdctl/pkg/idutil/imagewalker"
"github.com/containerd/nerdctl/pkg/imageinspector"
"github.com/containerd/nerdctl/pkg/inspecttypes/dockercompat"
"github.com/sirupsen/logrus"
)

// Inspect prints detailed information of each image in `images`.
Expand Down Expand Up @@ -60,19 +61,13 @@ func Inspect(ctx context.Context, client *containerd.Client, images []string, op
},
}

var errs []error
for _, req := range images {
n, err := walker.Walk(ctx, req)
if err != nil {
errs = append(errs, err)
} else if n == 0 {
errs = append(errs, fmt.Errorf("no such object: %s", req))
err := walker.WalkAll(ctx, images, true)
if len(f.entries) > 0 {
if formatErr := formatter.FormatSlice(options.Format, options.Stdout, f.entries); formatErr != nil {
logrus.Error(formatErr)
}
}
if len(errs) > 0 {
return fmt.Errorf("%d errors: %v", len(errs), errs)
}
return formatter.FormatSlice(options.Format, options.Stdout, f.entries)
return err
}

type imageInspector struct {
Expand Down
19 changes: 6 additions & 13 deletions pkg/cmd/image/remove.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,18 +88,11 @@ func Remove(ctx context.Context, client *containerd.Client, args []string, optio
return nil
},
}
for _, req := range args {
n, err := walker.Walk(ctx, req)
if err == nil && n == 0 {
err = fmt.Errorf("no such image %s", req)
}
if err != nil {
if options.Force {
logrus.Error(err)
} else {
return err
}
}

err = walker.WalkAll(ctx, args, true)
if err != nil && options.Force {
logrus.Error(err)
return nil
}
return nil
return err
}
12 changes: 4 additions & 8 deletions pkg/cmd/image/save.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,14 +77,10 @@ func SaveImages(ctx context.Context, client *containerd.Client, images []string,
},
}

for _, img := range images {
count, err := walker.Walk(ctx, img)
if err != nil {
return err
}
if count == 0 {
return fmt.Errorf("no such image: %s", img)
}
// check if all images exist
if err := walker.WalkAll(ctx, images, false); err != nil {
return err
}

return client.Export(ctx, options.Stdout, exportOpts...)
}
31 changes: 5 additions & 26 deletions pkg/cmd/network/inspect.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import (
"fmt"

"github.com/containerd/nerdctl/pkg/api/types"
"github.com/containerd/nerdctl/pkg/errutil"
"github.com/containerd/nerdctl/pkg/formatter"
"github.com/containerd/nerdctl/pkg/idutil/netwalker"
"github.com/containerd/nerdctl/pkg/inspecttypes/dockercompat"
Expand All @@ -43,7 +42,6 @@ func Inspect(ctx context.Context, options types.NetworkInspectOptions) error {
}

var result []interface{}
var errs []error
walker := netwalker.NetworkWalker{
Client: e,
OnFound: func(ctx context.Context, found netwalker.Found) error {
Expand All @@ -70,31 +68,12 @@ func Inspect(ctx context.Context, options types.NetworkInspectOptions) error {
},
}

for _, name := range options.Networks {
if name == "host" || name == "none" {
errs = append(errs, fmt.Errorf("pseudo network %q cannot be inspected", name))
continue
}
n, err := walker.Walk(ctx, name)
if err != nil {
errs = append(errs, err)
} else if n == 0 {
errs = append(errs, fmt.Errorf("no such network: %s", name))
}
}

// `network inspect` doesn't support pseudo network.
err = walker.WalkAll(ctx, options.Networks, true, false)
if len(result) > 0 {
err = formatter.FormatSlice(options.Format, options.Stdout, result)
if err != nil {
errs = append(errs, err)
}
}

if len(errs) > 0 {
for _, err := range errs {
logrus.Error(err)
if formatErr := formatter.FormatSlice(options.Format, options.Stdout, result); formatErr != nil {
logrus.Error(formatErr)
}
return errutil.NewExitCoderErr(1)
}
return nil
return err
}
Loading

0 comments on commit a1ed02a

Please sign in to comment.