network: Create network namespace from the CLI

This commit moves the network namespace creation out of virtcontainers
in order to anticipate the move of the OCI hooks to the CLI through a
follow up commit.

Signed-off-by: Sebastien Boeuf <sebastien.boeuf@intel.com>

cli/create.go

@@ -266,6 +266,13 @@ func createSandbox(ctx context.Context, ociSpec oci.CompatOCISpec, runtimeConfig
 		return vc.Process{}, err
 	}
 
+	// Important to create the network namespace before the sandbox is
+	// created, because it is not responsible for the creation of the
+	// netns if it does not exist.
+	if err := setupNetworkNamespace(&sandboxConfig.NetworkConfig); err != nil {
+		return vc.Process{}, err
+	}
+
 	sandbox, err := vci.CreateSandbox(ctx, sandboxConfig)
 	if err != nil {
 		return vc.Process{}, err

cli/create_test.go

@@ -474,6 +474,10 @@ func TestCreateContainerInvalid(t *testing.T) {
 }
 
 func TestCreateProcessCgroupsPathSuccessful(t *testing.T) {
+	if os.Geteuid() != 0 {
+		t.Skip(testDisabledNeedNonRoot)
+	}
+
 	assert := assert.New(t)
 
 	sandbox := &vcmock.Sandbox{
@@ -725,6 +729,10 @@ func TestCreateCreateCreatePidFileFail(t *testing.T) {
 }
 
 func TestCreate(t *testing.T) {
+	if os.Geteuid() != 0 {
+		t.Skip(testDisabledNeedNonRoot)
+	}
+
 	assert := assert.New(t)
 
 	sandbox := &vcmock.Sandbox{
@@ -891,6 +899,10 @@ func TestCreateSandboxConfigFail(t *testing.T) {
 }
 
 func TestCreateCreateSandboxFail(t *testing.T) {
+	if os.Geteuid() != 0 {
+		t.Skip(testDisabledNeedNonRoot)
+	}
+
 	assert := assert.New(t)
 
 	path, err := ioutil.TempDir("", "containers-mapping")

cli/network.go

@@ -6,11 +6,17 @@
 package main
 
 import (
+	"bufio"
 	"context"
 	"encoding/json"
 	"fmt"
 	"os"
+	"path/filepath"
+	"strings"
 
+	"golang.org/x/sys/unix"
+
+	"github.com/containernetworking/plugins/pkg/ns"
 	"github.com/kata-containers/agent/protocols/grpc"
 	vc "github.com/kata-containers/runtime/virtcontainers"
 	"github.com/sirupsen/logrus"
@@ -227,3 +233,128 @@ func networkListCommand(ctx context.Context, containerID string, opType networkT
 	}
 	return err
 }
+
+const procMountInfoFile = "/proc/self/mountinfo"
+
+// getNetNsFromBindMount returns the network namespace for the bind-mounted path
+func getNetNsFromBindMount(nsPath string, procMountFile string) (string, error) {
+	netNsMountType := "nsfs"
+
+	// Resolve all symlinks in the path as the mountinfo file contains
+	// resolved paths.
+	nsPath, err := filepath.EvalSymlinks(nsPath)
+	if err != nil {
+		return "", err
+	}
+
+	f, err := os.Open(procMountFile)
+	if err != nil {
+		return "", err
+	}
+	defer f.Close()
+
+	scanner := bufio.NewScanner(f)
+	for scanner.Scan() {
+		text := scanner.Text()
+
+		// Scan the mountinfo file to search for the network namespace path
+		// This file contains mounts in the eg format:
+		// "711 26 0:3 net:[4026532009] /run/docker/netns/default rw shared:535 - nsfs nsfs rw"
+		//
+		// Reference: https://www.kernel.org/doc/Documentation/filesystems/proc.txt
+
+		// We are interested in the first 9 fields of this file,
+		// to check for the correct mount type.
+		fields := strings.Split(text, " ")
+		if len(fields) < 9 {
+			continue
+		}
+
+		// We check here if the mount type is a network namespace mount type, namely "nsfs"
+		mountTypeFieldIdx := 8
+		if fields[mountTypeFieldIdx] != netNsMountType {
+			continue
+		}
+
+		// This is the mount point/destination for the mount
+		mntDestIdx := 4
+		if fields[mntDestIdx] != nsPath {
+			continue
+		}
+
+		// This is the root/source of the mount
+		return fields[3], nil
+	}
+
+	return "", nil
+}
+
+// hostNetworkingRequested checks if the network namespace requested is the
+// same as the current process.
+func hostNetworkingRequested(configNetNs string) (bool, error) {
+	var evalNS, nsPath, currentNsPath string
+	var err error
+
+	// Net namespace provided as "/proc/pid/ns/net" or "/proc/<pid>/task/<tid>/ns/net"
+	if strings.HasPrefix(configNetNs, "/proc") && strings.HasSuffix(configNetNs, "/ns/net") {
+		if _, err := os.Stat(configNetNs); err != nil {
+			return false, err
+		}
+
+		// Here we are trying to resolve the path but it fails because
+		// namespaces links don't really exist. For this reason, the
+		// call to EvalSymlinks will fail when it will try to stat the
+		// resolved path found. As we only care about the path, we can
+		// retrieve it from the PathError structure.
+		if _, err = filepath.EvalSymlinks(configNetNs); err != nil {
+			nsPath = err.(*os.PathError).Path
+		} else {
+			return false, fmt.Errorf("Net namespace path %s is not a symlink", configNetNs)
+		}
+
+		_, evalNS = filepath.Split(nsPath)
+
+	} else {
+		// Bind-mounted path provided
+		evalNS, _ = getNetNsFromBindMount(configNetNs, procMountInfoFile)
+	}
+
+	currentNS := fmt.Sprintf("/proc/%d/task/%d/ns/net", os.Getpid(), unix.Gettid())
+	if _, err = filepath.EvalSymlinks(currentNS); err != nil {
+		currentNsPath = err.(*os.PathError).Path
+	} else {
+		return false, fmt.Errorf("Unexpected: Current network namespace path is not a symlink")
+	}
+
+	_, evalCurrentNS := filepath.Split(currentNsPath)
+
+	if evalNS == evalCurrentNS {
+		return true, nil
+	}
+
+	return false, nil
+}
+
+func setupNetworkNamespace(config *vc.NetworkConfig) error {
+	if config.NetNSPath == "" {
+		n, err := ns.NewNS()
+		if err != nil {
+			return err
+		}
+
+		config.NetNSPath = n.Path()
+		config.NetNsCreated = true
+
+		return nil
+	}
+
+	isHostNs, err := hostNetworkingRequested(config.NetNSPath)
+	if err != nil {
+		return err
+	}
+	if isHostNs {
+		return fmt.Errorf("Host networking requested, not supported by runtime")
+	}
+
+	return nil
+}

cli/network_test.go

@@ -8,10 +8,16 @@ package main
 import (
 	"context"
 	"flag"
+	"fmt"
 	"io/ioutil"
 	"os"
+	"path/filepath"
+	"syscall"
 	"testing"
 
+	"golang.org/x/sys/unix"
+
+	"github.com/containernetworking/plugins/pkg/ns"
 	"github.com/kata-containers/agent/protocols/grpc"
 	vc "github.com/kata-containers/runtime/virtcontainers"
 	"github.com/stretchr/testify/assert"
@@ -87,3 +93,128 @@ func TestNetworkCliFunction(t *testing.T) {
 	f.Close()
 	execCLICommandFunc(assert, updateRoutesCommand, set, false)
 }
+
+func TestGetNetNsFromBindMount(t *testing.T) {
+	assert := assert.New(t)
+
+	tmpdir, err := ioutil.TempDir("", "")
+	assert.NoError(err)
+	defer os.RemoveAll(tmpdir)
+
+	mountFile := filepath.Join(tmpdir, "mountInfo")
+	nsPath := filepath.Join(tmpdir, "ns123")
+
+	// Non-existent namespace path
+	_, err = getNetNsFromBindMount(nsPath, mountFile)
+	assert.NotNil(err)
+
+	tmpNSPath := filepath.Join(tmpdir, "testNetNs")
+	f, err := os.Create(tmpNSPath)
+	assert.NoError(err)
+	defer f.Close()
+
+	type testData struct {
+		contents       string
+		expectedResult string
+	}
+
+	data := []testData{
+		{fmt.Sprintf("711 26 0:3 net:[4026532008] %s rw shared:535 - nsfs nsfs rw", tmpNSPath), "net:[4026532008]"},
+		{"711 26 0:3 net:[4026532008] /run/netns/ns123 rw shared:535 - tmpfs tmpfs rw", ""},
+		{"a a a a a a a - b c d", ""},
+		{"", ""},
+	}
+
+	for i, d := range data {
+		err := ioutil.WriteFile(mountFile, []byte(d.contents), 0640)
+		assert.NoError(err)
+
+		path, err := getNetNsFromBindMount(tmpNSPath, mountFile)
+		assert.NoError(err, fmt.Sprintf("got %q, test data: %+v", path, d))
+
+		assert.Equal(d.expectedResult, path, "Test %d, expected %s, got %s", i, d.expectedResult, path)
+	}
+}
+
+func TestHostNetworkingRequested(t *testing.T) {
+	assert := assert.New(t)
+
+	if os.Geteuid() != 0 {
+		t.Skip(testDisabledNeedRoot)
+	}
+
+	// Network namespace same as the host
+	selfNsPath := "/proc/self/ns/net"
+	isHostNs, err := hostNetworkingRequested(selfNsPath)
+	assert.NoError(err)
+	assert.True(isHostNs)
+
+	// Non-existent netns path
+	nsPath := "/proc/123456789/ns/net"
+	_, err = hostNetworkingRequested(nsPath)
+	assert.Error(err)
+
+	// Bind-mounted Netns
+	tmpdir, err := ioutil.TempDir("", "")
+	assert.NoError(err)
+	defer os.RemoveAll(tmpdir)
+
+	// Create a bind mount to the current network namespace.
+	tmpFile := filepath.Join(tmpdir, "testNetNs")
+	f, err := os.Create(tmpFile)
+	assert.NoError(err)
+	defer f.Close()
+
+	err = syscall.Mount(selfNsPath, tmpFile, "bind", syscall.MS_BIND, "")
+	assert.Nil(err)
+
+	isHostNs, err = hostNetworkingRequested(tmpFile)
+	assert.NoError(err)
+	assert.True(isHostNs)
+
+	syscall.Unmount(tmpFile, 0)
+}
+
+func TestSetupNetworkNamespace(t *testing.T) {
+	if os.Geteuid() != 0 {
+		t.Skip(testDisabledNeedNonRoot)
+	}
+
+	assert := assert.New(t)
+
+	// Network namespace same as the host
+	config := &vc.NetworkConfig{
+		NetNSPath: "/proc/self/ns/net",
+	}
+	err := setupNetworkNamespace(config)
+	assert.Error(err)
+
+	// Non-existent netns path
+	config = &vc.NetworkConfig{
+		NetNSPath: "/proc/123456789/ns/net",
+	}
+	err = setupNetworkNamespace(config)
+	assert.Error(err)
+
+	// Existent netns path
+	n, err := ns.NewNS()
+	assert.NoError(err)
+	config = &vc.NetworkConfig{
+		NetNSPath: n.Path(),
+	}
+	err = setupNetworkNamespace(config)
+	assert.NoError(err)
+	n.Close()
+
+	// Empty netns path
+	config = &vc.NetworkConfig{}
+	err = setupNetworkNamespace(config)
+	assert.NoError(err)
+	n, err = ns.GetNS(config.NetNSPath)
+	assert.NoError(err)
+	assert.NotNil(n)
+	assert.True(config.NetNsCreated)
+	n.Close()
+	unix.Unmount(config.NetNSPath, unix.MNT_DETACH)
+	os.RemoveAll(config.NetNSPath)
+}

cli/run_test.go

@@ -221,6 +221,10 @@ func testRunContainerSetup(t *testing.T) runContainerData {
 }
 
 func TestRunContainerSuccessful(t *testing.T) {
+	if os.Geteuid() != 0 {
+		t.Skip(testDisabledNeedNonRoot)
+	}
+
 	assert := assert.New(t)
 
 	d := testRunContainerSetup(t)
@@ -295,6 +299,10 @@ func TestRunContainerSuccessful(t *testing.T) {
 }
 
 func TestRunContainerDetachSuccessful(t *testing.T) {
+	if os.Geteuid() != 0 {
+		t.Skip(testDisabledNeedNonRoot)
+	}
+
 	assert := assert.New(t)
 
 	d := testRunContainerSetup(t)

virtcontainers/api_test.go

@@ -17,6 +17,7 @@ import (
 	"syscall"
 	"testing"
 
+	"github.com/containernetworking/plugins/pkg/ns"
 	"github.com/kata-containers/agent/protocols/grpc"
 	"github.com/kata-containers/runtime/virtcontainers/pkg/mock"
 	specs "github.com/opencontainers/runtime-spec/specs-go"
@@ -1392,6 +1393,15 @@ func TestStartStopSandboxHyperstartAgentSuccessfulWithDefaultNetwork(t *testing.
 
 	config := newTestSandboxConfigHyperstartAgentDefaultNetwork()
 
+	n, err := ns.NewNS()
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer n.Close()
+
+	config.NetworkConfig.NetNSPath = n.Path()
+	config.NetworkConfig.NetNsCreated = true
+
 	sockDir, err := testGenerateCCProxySockDir()
 	if err != nil {
 		t.Fatal(err)

virtcontainers/default_network.go

@@ -15,63 +15,24 @@ import (
 )
 
 type defNetwork struct {
-	ctx context.Context
 }
 
 func (n *defNetwork) logger() *logrus.Entry {
 	return virtLog.WithField("subsystem", "default-network")
 }
 
-func (n *defNetwork) trace(name string) (opentracing.Span, context.Context) {
+func (n *defNetwork) trace(ctx context.Context, name string) (opentracing.Span, context.Context) {
-	if n.ctx == nil {
+	span, ct := opentracing.StartSpanFromContext(ctx, name)
-		n.logger().WithField("type", "bug").Error("trace called before context set")
-		n.ctx = context.Background()
-	}
-
-	span, ctx := opentracing.StartSpanFromContext(n.ctx, name)
 
 	span.SetTag("subsystem", "network")
 	span.SetTag("type", "default")
 
-	return span, ctx
+	return span, ct
-}
-
-// init initializes the network, setting a new network namespace.
-func (n *defNetwork) init(ctx context.Context, config NetworkConfig) (string, bool, error) {
-	// Set context
-	n.ctx = ctx
-
-	span, _ := n.trace("init")
-	defer span.Finish()
-
-	if !config.InterworkingModel.IsValid() || config.InterworkingModel == NetXConnectDefaultModel {
-		config.InterworkingModel = DefaultNetInterworkingModel
-	}
-
-	if config.NetNSPath == "" {
-		path, err := createNetNS()
-		if err != nil {
-			return "", false, err
-		}
-
-		return path, true, nil
-	}
-
-	isHostNs, err := hostNetworkingRequested(config.NetNSPath)
-	if err != nil {
-		return "", false, err
-	}
-
-	if isHostNs {
-		return "", false, fmt.Errorf("Host networking requested, not supported by runtime")
-	}
-
-	return config.NetNSPath, false, nil
 }
 
 // run runs a callback in the specified network namespace.
 func (n *defNetwork) run(networkNSPath string, cb func() error) error {
-	span, _ := n.trace("run")
+	span, _ := n.trace(context.Background(), "run")
 	defer span.Finish()
 
 	if networkNSPath == "" {
@@ -84,24 +45,24 @@ func (n *defNetwork) run(networkNSPath string, cb func() error) error {
 }
 
 // add adds all needed interfaces inside the network namespace.
-func (n *defNetwork) add(sandbox *Sandbox, config NetworkConfig, netNsPath string, netNsCreated bool) (NetworkNamespace, error) {
+func (n *defNetwork) add(s *Sandbox) error {
-	span, _ := n.trace("add")
+	span, _ := n.trace(s.ctx, "add")
 	defer span.Finish()
 
-	endpoints, err := createEndpointsFromScan(netNsPath, config)
+	endpoints, err := createEndpointsFromScan(s.config.NetworkConfig.NetNSPath, s.config.NetworkConfig)
 	if err != nil {
-		return NetworkNamespace{}, err
+		return err
 	}
 
-	networkNS := NetworkNamespace{
+	s.networkNS = NetworkNamespace{
-		NetNsPath:    netNsPath,
+		NetNsPath:    s.config.NetworkConfig.NetNSPath,
-		NetNsCreated: netNsCreated,
+		NetNsCreated: s.config.NetworkConfig.NetNsCreated,
 		Endpoints:    endpoints,
 	}
 
-	err = doNetNS(networkNS.NetNsPath, func(_ ns.NetNS) error {
+	err = doNetNS(s.config.NetworkConfig.NetNSPath, func(_ ns.NetNS) error {
-		for _, endpoint := range networkNS.Endpoints {
+		for _, endpoint := range s.networkNS.Endpoints {
-			if err := endpoint.Attach(sandbox.hypervisor); err != nil {
+			if err := endpoint.Attach(s.hypervisor); err != nil {
 				return err
 			}
 		}
@@ -109,42 +70,33 @@ func (n *defNetwork) add(sandbox *Sandbox, config NetworkConfig, netNsPath strin
 		return nil
 	})
 	if err != nil {
-		return NetworkNamespace{}, err
+		return err
 	}
 
 	n.logger().Debug("Network added")
 
-	return networkNS, nil
+	return nil
 }
 
 // remove network endpoints in the network namespace. It also deletes the network
 // namespace in case the namespace has been created by us.
-func (n *defNetwork) remove(sandbox *Sandbox, networkNS NetworkNamespace, netNsCreated bool) error {
+func (n *defNetwork) remove(s *Sandbox) error {
-	// Set the context again.
+	span, _ := n.trace(s.ctx, "remove")
-	//
-	// This is required since when deleting networks, the init() method is
-	// not called since the network config state is simply read from disk.
-	// However, the context part of that state is not stored fully since
-	// context.Context is an interface type meaning all the trace metadata
-	// stored in the on-disk network config file is missing.
-	n.ctx = sandbox.ctx
-
-	span, _ := n.trace("remove")
 	defer span.Finish()
 
-	for _, endpoint := range networkNS.Endpoints {
+	for _, endpoint := range s.networkNS.Endpoints {
 		// Detach for an endpoint should enter the network namespace
 		// if required.
-		if err := endpoint.Detach(netNsCreated, networkNS.NetNsPath); err != nil {
+		if err := endpoint.Detach(s.networkNS.NetNsCreated, s.networkNS.NetNsPath); err != nil {
 			return err
 		}
 	}
 
 	n.logger().Debug("Network removed")
 
-	if netNsCreated {
+	if s.networkNS.NetNsCreated {
-		n.logger().Infof("Network namespace %q deleted", networkNS.NetNsPath)
+		n.logger().Infof("Network namespace %q deleted", s.networkNS.NetNsPath)
-		return deleteNetNS(networkNS.NetNsPath)
+		return deleteNetNS(s.networkNS.NetNsPath)
 	}
 
 	return nil

virtcontainers/network.go

@@ -6,8 +6,6 @@
 package virtcontainers
 
 import (
-	"bufio"
-	"context"
 	"encoding/hex"
 	"encoding/json"
 	"fmt"
@@ -144,6 +142,7 @@ type NetworkInterfacePair struct {
 // NetworkConfig is the network configuration related to a network.
 type NetworkConfig struct {
 	NetNSPath         string
+	NetNsCreated      bool
 	InterworkingModel NetInterworkingModel
 }
 
@@ -642,107 +641,6 @@ func newNetwork(networkType NetworkModel) network {
 	}
 }
 
-const procMountInfoFile = "/proc/self/mountinfo"
-
-// getNetNsFromBindMount returns the network namespace for the bind-mounted path
-func getNetNsFromBindMount(nsPath string, procMountFile string) (string, error) {
-	netNsMountType := "nsfs"
-
-	// Resolve all symlinks in the path as the mountinfo file contains
-	// resolved paths.
-	nsPath, err := filepath.EvalSymlinks(nsPath)
-	if err != nil {
-		return "", err
-	}
-
-	f, err := os.Open(procMountFile)
-	if err != nil {
-		return "", err
-	}
-	defer f.Close()
-
-	scanner := bufio.NewScanner(f)
-	for scanner.Scan() {
-		text := scanner.Text()
-
-		// Scan the mountinfo file to search for the network namespace path
-		// This file contains mounts in the eg format:
-		// "711 26 0:3 net:[4026532009] /run/docker/netns/default rw shared:535 - nsfs nsfs rw"
-		//
-		// Reference: https://www.kernel.org/doc/Documentation/filesystems/proc.txt
-
-		// We are interested in the first 9 fields of this file,
-		// to check for the correct mount type.
-		fields := strings.Split(text, " ")
-		if len(fields) < 9 {
-			continue
-		}
-
-		// We check here if the mount type is a network namespace mount type, namely "nsfs"
-		mountTypeFieldIdx := 8
-		if fields[mountTypeFieldIdx] != netNsMountType {
-			continue
-		}
-
-		// This is the mount point/destination for the mount
-		mntDestIdx := 4
-		if fields[mntDestIdx] != nsPath {
-			continue
-		}
-
-		// This is the root/source of the mount
-		return fields[3], nil
-	}
-
-	return "", nil
-}
-
-// hostNetworkingRequested checks if the network namespace requested is the
-// same as the current process.
-func hostNetworkingRequested(configNetNs string) (bool, error) {
-	var evalNS, nsPath, currentNsPath string
-	var err error
-
-	// Net namespace provided as "/proc/pid/ns/net" or "/proc/<pid>/task/<tid>/ns/net"
-	if strings.HasPrefix(configNetNs, "/proc") && strings.HasSuffix(configNetNs, "/ns/net") {
-		if _, err := os.Stat(configNetNs); err != nil {
-			return false, err
-		}
-
-		// Here we are trying to resolve the path but it fails because
-		// namespaces links don't really exist. For this reason, the
-		// call to EvalSymlinks will fail when it will try to stat the
-		// resolved path found. As we only care about the path, we can
-		// retrieve it from the PathError structure.
-		if _, err = filepath.EvalSymlinks(configNetNs); err != nil {
-			nsPath = err.(*os.PathError).Path
-		} else {
-			return false, fmt.Errorf("Net namespace path %s is not a symlink", configNetNs)
-		}
-
-		_, evalNS = filepath.Split(nsPath)
-
-	} else {
-		// Bind-mounted path provided
-		evalNS, _ = getNetNsFromBindMount(configNetNs, procMountInfoFile)
-	}
-
-	currentNS := fmt.Sprintf("/proc/%d/task/%d/ns/net", os.Getpid(), unix.Gettid())
-	if _, err = filepath.EvalSymlinks(currentNS); err != nil {
-		currentNsPath = err.(*os.PathError).Path
-	} else {
-		return false, fmt.Errorf("Unexpected: Current network namespace path is not a symlink")
-	}
-
-	_, evalCurrentNS := filepath.Split(currentNsPath)
-
-	if evalNS == evalCurrentNS {
-		return true, nil
-	}
-
-	return false, nil
-}
-
 func createLink(netHandle *netlink.Handle, name string, expectedLink netlink.Link) (netlink.Link, []*os.File, error) {
 	var newLink netlink.Link
 	var fds []*os.File
@@ -1607,16 +1505,13 @@ func vhostUserSocketPath(info interface{}) (string, error) {
 // Container network plugins are used to setup virtual network
 // between VM netns and the host network physical interface.
 type network interface {
-	// init initializes the network, setting a new network namespace.
-	init(ctx context.Context, config NetworkConfig) (string, bool, error)
-
 	// run runs a callback function in a specified network namespace.
 	run(networkNSPath string, cb func() error) error
 
 	// add adds all needed interfaces inside the network namespace.
-	add(sandbox *Sandbox, config NetworkConfig, netNsPath string, netNsCreated bool) (NetworkNamespace, error)
+	add(sandbox *Sandbox) error
 
 	// remove unbridges and deletes TAP interfaces. It also removes virtual network
 	// interfaces and deletes the network namespace.
-	remove(sandbox *Sandbox, networkNS NetworkNamespace, netNsCreated bool) error
+	remove(sandbox *Sandbox) error
 }

virtcontainers/network_test.go

@@ -7,12 +7,9 @@ package virtcontainers
 
 import (
 	"fmt"
-	"io/ioutil"
 	"net"
 	"os"
-	"path/filepath"
 	"reflect"
-	"syscall"
 	"testing"
 
 	"github.com/containernetworking/plugins/pkg/ns"
@@ -535,87 +532,6 @@ func TestPhysicalEndpoint_HotDetach(t *testing.T) {
 	assert.Error(err)
 }
 
-func TestGetNetNsFromBindMount(t *testing.T) {
-	assert := assert.New(t)
-
-	tmpdir, err := ioutil.TempDir("", "")
-	assert.NoError(err)
-	defer os.RemoveAll(tmpdir)
-
-	mountFile := filepath.Join(tmpdir, "mountInfo")
-	nsPath := filepath.Join(tmpdir, "ns123")
-
-	// Non-existent namespace path
-	_, err = getNetNsFromBindMount(nsPath, mountFile)
-	assert.NotNil(err)
-
-	tmpNSPath := filepath.Join(tmpdir, "testNetNs")
-	f, err := os.Create(tmpNSPath)
-	assert.NoError(err)
-	defer f.Close()
-
-	type testData struct {
-		contents       string
-		expectedResult string
-	}
-
-	data := []testData{
-		{fmt.Sprintf("711 26 0:3 net:[4026532008] %s rw shared:535 - nsfs nsfs rw", tmpNSPath), "net:[4026532008]"},
-		{"711 26 0:3 net:[4026532008] /run/netns/ns123 rw shared:535 - tmpfs tmpfs rw", ""},
-		{"a a a a a a a - b c d", ""},
-		{"", ""},
-	}
-
-	for i, d := range data {
-		err := ioutil.WriteFile(mountFile, []byte(d.contents), 0640)
-		assert.NoError(err)
-
-		path, err := getNetNsFromBindMount(tmpNSPath, mountFile)
-		assert.NoError(err, fmt.Sprintf("got %q, test data: %+v", path, d))
-
-		assert.Equal(d.expectedResult, path, "Test %d, expected %s, got %s", i, d.expectedResult, path)
-	}
-}
-
-func TestHostNetworkingRequested(t *testing.T) {
-	if os.Geteuid() != 0 {
-		t.Skip(testDisabledAsNonRoot)
-	}
-
-	assert := assert.New(t)
-
-	// Network namespace same as the host
-	selfNsPath := "/proc/self/ns/net"
-	isHostNs, err := hostNetworkingRequested(selfNsPath)
-	assert.NoError(err)
-	assert.True(isHostNs)
-
-	// Non-existent netns path
-	nsPath := "/proc/123/ns/net"
-	_, err = hostNetworkingRequested(nsPath)
-	assert.Error(err)
-
-	// Bind-mounted Netns
-	tmpdir, err := ioutil.TempDir("", "")
-	assert.NoError(err)
-	defer os.RemoveAll(tmpdir)
-
-	// Create a bind mount to the current network namespace.
-	tmpFile := filepath.Join(tmpdir, "testNetNs")
-	f, err := os.Create(tmpFile)
-	assert.NoError(err)
-	defer f.Close()
-
-	err = syscall.Mount(selfNsPath, tmpFile, "bind", syscall.MS_BIND, "")
-	assert.Nil(err)
-
-	isHostNs, err = hostNetworkingRequested(tmpFile)
-	assert.NoError(err)
-	assert.True(isHostNs)
-
-	syscall.Unmount(tmpFile, 0)
-}
-
 func TestGenerateInterfacesAndRoutes(t *testing.T) {
 	//
 	//Create a couple of addresses

virtcontainers/noop_network.go

@@ -5,19 +5,11 @@
 
 package virtcontainers
 
-import "context"
-
 // noopNetwork a.k.a. NO-OP Network is an empty network implementation, for
 // testing and mocking purposes.
 type noopNetwork struct {
 }
 
-// init initializes the network, setting a new network namespace for the Noop network.
-// It does nothing.
-func (n *noopNetwork) init(ctx context.Context, config NetworkConfig) (string, bool, error) {
-	return "", true, nil
-}
-
 // run runs a callback in the specified network namespace for
 // the Noop network.
 // It does nothing.
@@ -27,13 +19,13 @@ func (n *noopNetwork) run(networkNSPath string, cb func() error) error {
 
 // add adds all needed interfaces inside the network namespace the Noop network.
 // It does nothing.
-func (n *noopNetwork) add(sandbox *Sandbox, config NetworkConfig, netNsPath string, netNsCreated bool) (NetworkNamespace, error) {
+func (n *noopNetwork) add(sandbox *Sandbox) error {
-	return NetworkNamespace{}, nil
+	return nil
 }
 
 // remove unbridges and deletes TAP interfaces. It also removes virtual network
 // interfaces and deletes the network namespace for the Noop network.
 // It does nothing.
-func (n *noopNetwork) remove(sandbox *Sandbox, networkNS NetworkNamespace, netNsCreated bool) error {
+func (n *noopNetwork) remove(sandbox *Sandbox) error {
 	return nil
 }

virtcontainers/sandbox.go

@@ -971,49 +971,27 @@ func (s *Sandbox) createNetwork() error {
 	span, _ := s.trace("createNetwork")
 	defer span.Finish()
 
-	var netNsPath string
-	var netNsCreated bool
-	var networkNS NetworkNamespace
-	var err error
-
-	//rollback the NetNs when createNetwork failed
-	defer func() {
-		if err != nil && netNsPath != "" && netNsCreated {
-			deleteNetNS(netNsPath)
-		}
-	}()
-
-	// Initialize the network.
-	netNsPath, netNsCreated, err = s.network.init(s.ctx, s.config.NetworkConfig)
-	if err != nil {
-		return err
-	}
-
 	// Execute prestart hooks inside netns
-	if err := s.network.run(netNsPath, func() error {
+	if err := s.network.run(s.config.NetworkConfig.NetNSPath, func() error {
 		return s.config.Hooks.preStartHooks(s)
 	}); err != nil {
 		return err
 	}
 
 	// Add the network
-	networkNS, err = s.network.add(s, s.config.NetworkConfig, netNsPath, netNsCreated)
+	if err := s.network.add(s); err != nil {
-	if err != nil {
 		return err
 	}
-	s.networkNS = networkNS
 
 	// Store the network
-	err = s.storage.storeSandboxNetwork(s.id, networkNS)
+	return s.storage.storeSandboxNetwork(s.id, s.networkNS)
-
-	return err
 }
 
 func (s *Sandbox) removeNetwork() error {
 	span, _ := s.trace("removeNetwork")
 	defer span.Finish()
 
-	return s.network.remove(s, s.networkNS, s.networkNS.NetNsCreated)
+	return s.network.remove(s)
 }
 
 func (s *Sandbox) generateNetInfo(inf *grpc.Interface) (NetworkInfo, error) {