Merge pull request containerd#3508 from apostasie/chore-clean-network…

…-walker

Fix raft of issues related to network listing

cmd/nerdctl/completion/completion.go

@@ -124,9 +124,13 @@ func NetworkNames(cmd *cobra.Command, exclude []string) ([]string, cobra.ShellCo
  if err != nil {
  return nil, cobra.ShellCompDirectiveError
  }
- for netName := range netConfigs {
+ for netName, network := range netConfigs {
  if _, ok := excludeMap[netName]; !ok {
  candidates = append(candidates, netName)
+ if network.NerdctlID != nil {
+ candidates = append(candidates, *network.NerdctlID)
+ candidates = append(candidates, (*network.NerdctlID)[0:12])
+ }
  }
  }
  for _, s := range []string{"host", "none"} {

cmd/nerdctl/network/network_inspect_test.go

@@ -19,6 +19,7 @@ package network
 import (
  "encoding/json"
  "errors"
+ "strings"
  "testing"
 
  "gotest.tools/v3/assert"
@@ -38,6 +39,128 @@ func TestNetworkInspect(t *testing.T) {
  )
 
  testGroup := &test.Group{
+ {
+ Description: "non existent network",
+ Command: test.RunCommand("network", "inspect", "nonexistent"),
+ // FIXME: where is this error even comin from?
+ Expected: test.Expects(1, []error{errors.New("no network found matching")}, nil),
+ },
+ {
+ Description: "invalid name network",
+ Command: test.RunCommand("network", "inspect", "∞"),
+ // FIXME: this is not even a valid identifier
+ Expected: test.Expects(1, []error{errors.New("no network found matching")}, nil),
+ },
+ {
+ Description: "none",
+ Require: nerdtest.NerdctlNeedsFixing,
+ Command: test.RunCommand("network", "inspect", "none"),
+ Expected: test.Expects(0, nil, func(stdout string, info string, t *testing.T) {
+ var dc []dockercompat.Network
+ err := json.Unmarshal([]byte(stdout), &dc)
+ assert.NilError(t, err, "Unable to unmarshal output\n"+info)
+ assert.Equal(t, 1, len(dc), "Unexpectedly got multiple results\n"+info)
+ assert.Equal(t, dc[0].Name, "none")
+ }),
+ },
+ {
+ Description: "host",
+ Require: nerdtest.NerdctlNeedsFixing,
+ Command: test.RunCommand("network", "inspect", "host"),
+ Expected: test.Expects(0, nil, func(stdout string, info string, t *testing.T) {
+ var dc []dockercompat.Network
+ err := json.Unmarshal([]byte(stdout), &dc)
+ assert.NilError(t, err, "Unable to unmarshal output\n"+info)
+ assert.Equal(t, 1, len(dc), "Unexpectedly got multiple results\n"+info)
+ assert.Equal(t, dc[0].Name, "host")
+ }),
+ },
+ {
+ Description: "bridge",
+ Require: test.Not(test.Windows),
+ Command: test.RunCommand("network", "inspect", "bridge"),
+ Expected: test.Expects(0, nil, func(stdout string, info string, t *testing.T) {
+ var dc []dockercompat.Network
+ err := json.Unmarshal([]byte(stdout), &dc)
+ assert.NilError(t, err, "Unable to unmarshal output\n"+info)
+ assert.Equal(t, 1, len(dc), "Unexpectedly got multiple results\n"+info)
+ assert.Equal(t, dc[0].Name, "bridge")
+ }),
+ },
+ {
+ Description: "custom",
+ Setup: func(data test.Data, helpers test.Helpers) {
+ helpers.Ensure("network", "create", "custom")
+ },
+ Cleanup: func(data test.Data, helpers test.Helpers) {
+ helpers.Anyhow("network", "remove", "custom")
+ },
+ Command: test.RunCommand("network", "inspect", "custom"),
+ Expected: test.Expects(0, nil, func(stdout string, info string, t *testing.T) {
+ var dc []dockercompat.Network
+ err := json.Unmarshal([]byte(stdout), &dc)
+ assert.NilError(t, err, "Unable to unmarshal output\n"+info)
+ assert.Equal(t, 1, len(dc), "Unexpectedly got multiple results\n"+info)
+ assert.Equal(t, dc[0].Name, "custom")
+ }),
+ },
+ {
+ Description: "match exact id",
+ Require: test.Not(test.Windows),
+ Command: func(data test.Data, helpers test.Helpers) test.Command {
+ id := strings.TrimSpace(helpers.Capture("network", "inspect", "bridge", "--format", "{{ .Id }}"))
+ return helpers.Command("network", "inspect", id)
+ },
+ Expected: test.Expects(0, nil, func(stdout string, info string, t *testing.T) {
+ var dc []dockercompat.Network
+ err := json.Unmarshal([]byte(stdout), &dc)
+ assert.NilError(t, err, "Unable to unmarshal output\n"+info)
+ assert.Equal(t, 1, len(dc), "Unexpectedly got multiple results\n"+info)
+ assert.Equal(t, dc[0].Name, "bridge")
+ }),
+ },
+ {
+ Description: "match part of id",
+ Require: test.Not(test.Windows),
+ Command: func(data test.Data, helpers test.Helpers) test.Command {
+ id := strings.TrimSpace(helpers.Capture("network", "inspect", "bridge", "--format", "{{ .Id }}"))
+ return helpers.Command("network", "inspect", id[0:25])
+ },
+ Expected: test.Expects(0, nil, func(stdout string, info string, t *testing.T) {
+ var dc []dockercompat.Network
+ err := json.Unmarshal([]byte(stdout), &dc)
+ assert.NilError(t, err, "Unable to unmarshal output\n"+info)
+ assert.Equal(t, 1, len(dc), "Unexpectedly got multiple results\n"+info)
+ assert.Equal(t, dc[0].Name, "bridge")
+ }),
+ },
+ {
+ Description: "using another net short id",
+ Require: test.Not(test.Windows),
+ Setup: func(data test.Data, helpers test.Helpers) {
+ id := strings.TrimSpace(helpers.Capture("network", "inspect", "bridge", "--format", "{{ .Id }}"))
+ helpers.Ensure("network", "create", id[0:12])
+ data.Set("netname", id[0:12])
+ },
+ Cleanup: func(data test.Data, helpers test.Helpers) {
+ id := strings.TrimSpace(helpers.Capture("network", "inspect", "bridge", "--format", "{{ .Id }}"))
+ helpers.Anyhow("network", "remove", id[0:12])
+ },
+ Command: func(data test.Data, helpers test.Helpers) test.Command {
+ return helpers.Command("network", "inspect", data.Get("netname"))
+ },
+ Expected: func(data test.Data, helpers test.Helpers) *test.Expected {
+ return &test.Expected{
+ Output: func(stdout string, info string, t *testing.T) {
+ var dc []dockercompat.Network
+ err := json.Unmarshal([]byte(stdout), &dc)
+ assert.NilError(t, err, "Unable to unmarshal output\n"+info)
+ assert.Equal(t, 1, len(dc), "Unexpectedly got multiple results\n"+info)
+ assert.Equal(t, dc[0].Name, data.Get("netname"))
+ },
+ }
+ },
+ },
  {
  Description: "Test network inspect",
  // IPAMConfig is not implemented on Windows yet
@@ -88,16 +211,16 @@ func TestNetworkInspect(t *testing.T) {
  return &test.Expected{
  ExitCode: 0,
  Output: func(stdout string, info string, t *testing.T) {
- cmd := helpers.Command().Clear().WithBinary("nerdctl").WithArgs("--namespace", data.Identifier())
+ cmd := helpers.CustomCommand("nerdctl", "--namespace", data.Identifier())
 
  cmd.Clone().WithArgs("network", "inspect", data.Identifier()).Run(&test.Expected{
  ExitCode: 1,
- Errors: []error{errors.New("no such network")},
+ Errors: []error{errors.New("no network found")},
  })
 
  cmd.Clone().WithArgs("network", "remove", data.Identifier()).Run(&test.Expected{
  ExitCode: 1,
- Errors: []error{errors.New("no such network")},
+ Errors: []error{errors.New("no network found")},
  })
 
  cmd.Clone().WithArgs("network", "ls").Run(&test.Expected{

pkg/cmd/container/kill.go

@@ -161,16 +161,12 @@ func cleanupNetwork(ctx context.Context, container containerd.Container, globalO
  cniOpts := []gocni.Opt{
  gocni.WithPluginDir([]string{globalOpts.CNIPath}),
  }
- netMap, err := e.NetworkMap()
- if err != nil {
- return err
- }
+ var netw *netutil.NetworkConfig
  for _, netstr := range networks {
- net, ok := netMap[netstr]
- if !ok {
- return fmt.Errorf("no such network: %q", netstr)
+ if netw, err = e.NetworkByNameOrID(netstr); err != nil {
+ return err
  }
- cniOpts = append(cniOpts, gocni.WithConfListBytes(net.Bytes))
+ cniOpts = append(cniOpts, gocni.WithConfListBytes(netw.Bytes))
  }
  cni, err := gocni.New(cniOpts...)
  if err != nil {

pkg/cmd/network/inspect.go

@@ -19,62 +19,71 @@ package network
 import (
  "context"
  "encoding/json"
+ "errors"
  "fmt"
 
  "github.com/containerd/log"
 
  "github.com/containerd/nerdctl/v2/pkg/api/types"
  "github.com/containerd/nerdctl/v2/pkg/formatter"
- "github.com/containerd/nerdctl/v2/pkg/idutil/netwalker"
  "github.com/containerd/nerdctl/v2/pkg/inspecttypes/dockercompat"
  "github.com/containerd/nerdctl/v2/pkg/inspecttypes/native"
  "github.com/containerd/nerdctl/v2/pkg/netutil"
 )
 
 func Inspect(ctx context.Context, options types.NetworkInspectOptions) error {
- globalOptions := options.GOptions
- e, err := netutil.NewCNIEnv(globalOptions.CNIPath, globalOptions.CNINetConfPath, netutil.WithNamespace(options.GOptions.Namespace))
+ if options.Mode != "native" && options.Mode != "dockercompat" {
+ return fmt.Errorf("unknown mode %q", options.Mode)
+ }
 
+ cniEnv, err := netutil.NewCNIEnv(options.GOptions.CNIPath, options.GOptions.CNINetConfPath, netutil.WithNamespace(options.GOptions.Namespace))
  if err != nil {
  return err
  }
- if options.Mode != "native" && options.Mode != "dockercompat" {
- return fmt.Errorf("unknown mode %q", options.Mode)
- }
 
  var result []interface{}
- walker := netwalker.NetworkWalker{
- Client: e,
- OnFound: func(ctx context.Context, found netwalker.Found) error {
- if found.MatchCount > 1 {
- return fmt.Errorf("multiple IDs found with provided prefix: %s", found.Req)
- }
- r := &native.Network{
- CNI: json.RawMessage(found.Network.Bytes),
- NerdctlID: found.Network.NerdctlID,
- NerdctlLabels: found.Network.NerdctlLabels,
- File: found.Network.File,
- }
- switch options.Mode {
- case "native":
- result = append(result, r)
- case "dockercompat":
- compat, err := dockercompat.NetworkFromNative(r)
- if err != nil {
- return err
- }
- result = append(result, compat)
+ netLists, errs := cniEnv.ListNetworksMatch(options.Networks, true)
+
+ for req, netList := range netLists {
+ if len(netList) > 1 {
+ errs = append(errs, fmt.Errorf("multiple IDs found with provided prefix: %s", req))
+ continue
+ }
+ if len(netList) == 0 {
+ errs = append(errs, fmt.Errorf("no network found matching: %s", req))
+ continue
+ }
+ network := netList[0]
+ r := &native.Network{
+ CNI: json.RawMessage(network.Bytes),
+ NerdctlID: network.NerdctlID,
+ NerdctlLabels: network.NerdctlLabels,
+ File: network.File,
+ }
+ switch options.Mode {
+ case "native":
+ result = append(result, r)
+ case "dockercompat":
+ compat, err := dockercompat.NetworkFromNative(r)
+ if err != nil {
+ return err
  }
- return nil
- },
+ result = append(result, compat)
+ }
  }
 
- // `network inspect` doesn't support pseudo network.
- err = walker.WalkAll(ctx, options.Networks, true, false)
  if len(result) > 0 {
  if formatErr := formatter.FormatSlice(options.Format, options.Stdout, result); formatErr != nil {
  log.G(ctx).Error(formatErr)
  }
+ err = nil
+ } else {
+ err = errors.New("unable to find any network matching the provided request")
+ }
+
+ for _, unErr := range errs {
+ log.G(ctx).Error(unErr)
  }
+
  return err
 }

pkg/cmd/network/remove.go

@@ -18,17 +18,18 @@ package network
 
 import (
  "context"
+ "errors"
  "fmt"
 
  containerd "github.com/containerd/containerd/v2/client"
+ "github.com/containerd/log"
 
  "github.com/containerd/nerdctl/v2/pkg/api/types"
- "github.com/containerd/nerdctl/v2/pkg/idutil/netwalker"
  "github.com/containerd/nerdctl/v2/pkg/netutil"
 )
 
 func Remove(ctx context.Context, client *containerd.Client, options types.NetworkRemoveOptions) error {
- e, err := netutil.NewCNIEnv(options.GOptions.CNIPath, options.GOptions.CNINetConfPath, netutil.WithNamespace(options.GOptions.Namespace))
+ cniEnv, err := netutil.NewCNIEnv(options.GOptions.CNIPath, options.GOptions.CNINetConfPath, netutil.WithNamespace(options.GOptions.Namespace))
  if err != nil {
  return err
  }
@@ -38,28 +39,52 @@ func Remove(ctx context.Context, client *containerd.Client, options types.Networ
  return err
  }
 
- walker := netwalker.NetworkWalker{
- Client: e,
- OnFound: func(ctx context.Context, found netwalker.Found) error {
- if found.MatchCount > 1 {
- return fmt.Errorf("multiple IDs found with provided prefix: %s", found.Req)
- }
- if value, ok := usedNetworkInfo[found.Network.Name]; ok {
- return fmt.Errorf("network %q is in use by container %q", found.Req, value)
- }
- if found.Network.NerdctlID == nil {
- return fmt.Errorf("%s is managed outside nerdctl and cannot be removed", found.Req)
- }
- if found.Network.File == "" {
- return fmt.Errorf("%s is a pre-defined network and cannot be removed", found.Req)
- }
- if err := e.RemoveNetwork(found.Network); err != nil {
- return err
- }
- fmt.Fprintln(options.Stdout, found.Req)
- return nil
- },
+ var result []string
+ netLists, errs := cniEnv.ListNetworksMatch(options.Networks, false)
+
+ for req, netList := range netLists {
+ if len(netList) > 1 {
+ errs = append(errs, fmt.Errorf("multiple IDs found with provided prefix: %s", req))
+ continue
+ }
+ if len(netList) == 0 {
+ errs = append(errs, fmt.Errorf("no network found matching: %s", req))
+ continue
+ }
+ network := netList[0]
+ if value, ok := usedNetworkInfo[network.Name]; ok {
+ errs = append(errs, fmt.Errorf("network %q is in use by container %q", req, value))
+ continue
+ }
+ if network.Name == "bridge" {
+ errs = append(errs, errors.New("cannot remove pre-defined network bridge"))
+ continue
+ }
+ if network.File == "" {
+ errs = append(errs, fmt.Errorf("%s is a pre-defined network and cannot be removed", req))
+ continue
+ }
+ if network.NerdctlID == nil {
+ errs = append(errs, fmt.Errorf("%s is managed outside nerdctl and cannot be removed", req))
+ continue
+ }
+ if err := cniEnv.RemoveNetwork(network); err != nil {
+ errs = append(errs, err)
+ } else {
+ result = append(result, req)
+ }
+ }
+ for _, unErr := range errs {
+ log.G(ctx).Error(unErr)
+ }
+ if len(result) > 0 {
+ for _, id := range result {
+ fmt.Fprintln(options.Stdout, id)
+ }
+ err = nil
+ } else {
+ err = errors.New("no network could be removed")
  }
 
- return walker.WalkAll(ctx, options.Networks, true, false)
+ return err
 }