Merge pull request #1245 from junnplus/bridge-interface

cmd/nerdctl/network_create.go

@@ -19,8 +19,8 @@ package main
 import (
 	"fmt"
 
+	"github.com/containerd/containerd/errdefs"
 	"github.com/containerd/containerd/identifiers"
-	"github.com/containerd/nerdctl/pkg/lockutil"
 	"github.com/containerd/nerdctl/pkg/netutil"
 	"github.com/containerd/nerdctl/pkg/strutil"
 
@@ -95,52 +95,36 @@ func networkCreateAction(cmd *cobra.Command, args []string) error {
 	if err != nil {
 		return err
 	}
+	labels = strutil.DedupeStrSlice(labels)
 
-	fn := func() error {
-		e, err := netutil.NewCNIEnv(cniPath, cniNetconfpath)
-		if err != nil {
-			return err
-		}
-		for _, n := range e.Networks {
-			if n.Name == name {
-				return fmt.Errorf("network with name %s already exists", name)
-			}
-			// TODO: check CIDR collision
-		}
-		id, err := e.AcquireNextID()
-		if err != nil {
-			return err
-		}
-
-		if subnetStr == "" {
-			if gatewayStr != "" || ipRangeStr != "" {
-				return fmt.Errorf("cannot set gateway or ip-range without subnet, specify --subnet manually")
-			}
-			if id > 255 {
-				return fmt.Errorf("cannot determine subnet for ID %d, specify --subnet manually", id)
-			}
-			subnetStr = fmt.Sprintf("10.4.%d.0/24", id)
+	if subnetStr == "" {
+		if gatewayStr != "" || ipRangeStr != "" {
+			return fmt.Errorf("cannot set gateway or ip-range without subnet, specify --subnet manually")
 		}
+	}
 
-		labels := strutil.DedupeStrSlice(labels)
-		ipam, err := netutil.GenerateIPAM(ipamDriver, subnetStr, gatewayStr, ipRangeStr, strutil.ConvertKVStringsToMap(ipamOpts))
-		if err != nil {
-			return err
-		}
-		cniPlugins, err := e.GenerateCNIPlugins(driver, id, name, ipam, strutil.ConvertKVStringsToMap(opts))
-		if err != nil {
-			return err
-		}
-		net, err := e.GenerateNetworkConfig(labels, id, name, cniPlugins)
-		if err != nil {
-			return err
-		}
-		if err := e.WriteNetworkConfig(net); err != nil {
-			return err
+	e, err := netutil.NewCNIEnv(cniPath, cniNetconfpath)
+	if err != nil {
+		return err
+	}
+	createOpts := netutil.CreateOptions{
+		Name:        name,
+		Driver:      driver,
+		Options:     strutil.ConvertKVStringsToMap(opts),
+		IPAMDriver:  ipamDriver,
+		IPAMOptions: strutil.ConvertKVStringsToMap(ipamOpts),
+		Subnet:      subnetStr,
+		Gateway:     gatewayStr,
+		IPRange:     ipRangeStr,
+		Labels:      labels,
+	}
+	net, err := e.CreateNetwork(createOpts)
+	if err != nil {
+		if errdefs.IsAlreadyExists(err) {
+			return fmt.Errorf("network with name %s already exists", name)
 		}
-		fmt.Fprintf(cmd.OutOrStdout(), "%d\n", id)
-		return nil
+		return err
 	}
-
-	return lockutil.WithDirLock(cniNetconfpath, fn)
+	_, err = fmt.Fprintf(cmd.OutOrStdout(), "%s\n", *net.NerdctlID)
+	return err
 }

cmd/nerdctl/network_create_linux_test.go

@@ -20,9 +20,11 @@ import (
 	"testing"
 
 	"github.com/containerd/nerdctl/pkg/testutil"
+	"gotest.tools/v3/assert"
 )
 
 func TestNetworkCreateWithMTU(t *testing.T) {
+	t.Parallel()
 	testNetwork := testutil.Identifier(t)
 	base := testutil.NewBase(t)
 
@@ -35,3 +37,22 @@ func TestNetworkCreateWithMTU(t *testing.T) {
 
 	base.Cmd("run", "--rm", "--net", testNetwork, testutil.AlpineImage, "ifconfig", "eth0").AssertOutContains("MTU:9216")
 }
+
+func TestNetworkCreate(t *testing.T) {
+	t.Parallel()
+	base := testutil.NewBase(t)
+	testNetwork := testutil.Identifier(t)
+
+	base.Cmd("network", "create", testNetwork).AssertOK()
+	defer base.Cmd("network", "rm", testNetwork).Run()
+
+	net := base.InspectNetwork(testNetwork)
+	assert.Equal(t, len(net.IPAM.Config), 1)
+
+	base.Cmd("run", "--rm", "--net", testNetwork, testutil.CommonImage, "ip", "route").AssertOutContains(net.IPAM.Config[0].Subnet)
+
+	base.Cmd("network", "create", testNetwork+"-1").AssertOK()
+	defer base.Cmd("network", "rm", testNetwork+"-1").Run()
+
+	base.Cmd("run", "--rm", "--net", testNetwork+"-1", testutil.CommonImage, "ip", "route").AssertNoOut(net.IPAM.Config[0].Subnet)
+}

cmd/nerdctl/network_ls.go

@@ -20,7 +20,6 @@ import (
 	"bytes"
 	"errors"
 	"fmt"
-	"strconv"
 	"text/tabwriter"
 	"text/template"
 
@@ -105,7 +104,10 @@ func networkLsAction(cmd *cobra.Command, args []string) error {
 			file: n.File,
 		}
 		if n.NerdctlID != nil {
-			p.ID = strconv.Itoa(*n.NerdctlID)
+			p.ID = *n.NerdctlID
+			if len(p.ID) > 12 {
+				p.ID = p.ID[:12]
+			}
 		}
 		if n.NerdctlLabels != nil {
 			p.Labels = formatLabels(*n.NerdctlLabels)

cmd/nerdctl/network_rm.go

@@ -18,9 +18,7 @@ package main
 
 import (
 	"fmt"
-	"os"
 
-	"github.com/containerd/nerdctl/pkg/lockutil"
 	"github.com/containerd/nerdctl/pkg/netutil"
 
 	"github.com/spf13/cobra"
@@ -54,36 +52,28 @@ func networkRmAction(cmd *cobra.Command, args []string) error {
 	if err != nil {
 		return err
 	}
-	fn := func() error {
-		netMap := e.NetworkMap()
+	netMap := e.NetworkMap()
 
-		for _, name := range args {
-			if name == "host" || name == "none" {
-				return fmt.Errorf("pseudo network %q cannot be removed", name)
-			}
-			l, ok := netMap[name]
-			if !ok {
-				return fmt.Errorf("no such network: %s", name)
-			}
-			if l.NerdctlID == nil {
-				return fmt.Errorf("%s is managed outside nerdctl and cannot be removed", name)
-			}
-			if l.File == "" {
-				return fmt.Errorf("%s is a pre-defined network and cannot be removed", name)
-			}
-			if err := os.RemoveAll(l.File); err != nil {
-				return err
-			}
-			// Remove the bridge network interface on the host.
-			if l.Plugins[0].Network.Type == "bridge" {
-				netIf := netutil.GetBridgeName(*l.NerdctlID)
-				removeBridgeNetworkInterface(netIf)
-			}
-			fmt.Fprintln(cmd.OutOrStdout(), name)
+	for _, name := range args {
+		if name == "host" || name == "none" {
+			return fmt.Errorf("pseudo network %q cannot be removed", name)
 		}
-		return nil
+		net, ok := netMap[name]
+		if !ok {
+			return fmt.Errorf("no such network: %s", name)
+		}
+		if net.NerdctlID == nil {
+			return fmt.Errorf("%s is managed outside nerdctl and cannot be removed", name)
+		}
+		if net.File == "" {
+			return fmt.Errorf("%s is a pre-defined network and cannot be removed", name)
+		}
+		if err := e.RemoveNetwork(net); err != nil {
+			return err
+		}
+		fmt.Fprintln(cmd.OutOrStdout(), name)
 	}
-	return lockutil.WithDirLock(cniNetconfpath, fn)
+	return nil
 }
 
 func networkRmShellComplete(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) {

cmd/nerdctl/network_rm_linux_test.go

@@ -0,0 +1,51 @@
+/*
+   Copyright The containerd Authors.
+
+   Licensed under the Apache License, Version 2.0 (the "License");
+   you may not use this file except in compliance with the License.
+   You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+*/
+
+package main
+
+import (
+	"testing"
+
+	"github.com/containerd/nerdctl/pkg/rootlessutil"
+	"github.com/containerd/nerdctl/pkg/testutil"
+	"github.com/vishvananda/netlink"
+	"gotest.tools/v3/assert"
+)
+
+func TestNetworkRemove(t *testing.T) {
+	t.Parallel()
+	if rootlessutil.IsRootless() {
+		t.Skip("test skipped for remove rootless network")
+	}
+	base := testutil.NewBase(t)
+	networkName := testutil.Identifier(t)
+
+	base.Cmd("network", "create", networkName).AssertOK()
+	defer base.Cmd("network", "rm", networkName).Run()
+
+	networkID := base.InspectNetwork(networkName).ID
+
+	tID := testutil.Identifier(t)
+	base.Cmd("run", "--rm", "--net", networkName, "--name", tID, testutil.CommonImage).AssertOK()
+
+	_, err := netlink.LinkByName("br-" + networkID[:12])
+	assert.NilError(t, err)
+
+	base.Cmd("network", "rm", networkName).AssertOK()
+
+	_, err = netlink.LinkByName("br-" + networkID[:12])
+	assert.Error(t, err, "Link not found")
+}

go.mod

@@ -124,7 +124,7 @@ require (
 	github.com/minio/blake2b-simd v0.0.0-20160723061019-3f5f724cb5b1 // indirect
 	github.com/minio/sha256-simd v1.0.0 // indirect
 	github.com/mitchellh/go-homedir v1.1.0 // indirect
-	github.com/mitchellh/mapstructure v1.5.0 // indirect
+	github.com/mitchellh/mapstructure v1.5.0
 	github.com/moby/locker v1.0.1 // indirect
 	github.com/moby/sys/mountinfo v0.6.2 // indirect
 	github.com/moby/sys/signal v0.7.0

pkg/inspecttypes/dockercompat/dockercompat.go

@@ -463,7 +463,7 @@ func NetworkFromNative(n *native.Network) (*Network, error) {
 	}
 
 	if n.NerdctlID != nil {
-		res.ID = strconv.Itoa(*n.NerdctlID)
+		res.ID = *n.NerdctlID
 	}
 
 	if n.NerdctlLabels != nil {

pkg/inspecttypes/native/network.go

@@ -21,7 +21,7 @@ import "encoding/json"
 // Network corresponds to pkg/netutil.NetworkConfigList
 type Network struct {
 	CNI           json.RawMessage    `json:"CNI,omitempty"`
-	NerdctlID     *int               `json:"NerdctlID"`
+	NerdctlID     *string            `json:"NerdctlID"`
 	NerdctlLabels *map[string]string `json:"NerdctlLabels,omitempty"`
 	File          string             `json:"File,omitempty"`
 }