virtcontainers: Remove the network interface

There's only one real implementer of the network interface and no real need to implement anything else. We can just go ahead and remove this abstraction. Fixes: #1179 Signed-off-by: Samuel Ortiz <[email protected]>
kata-containers · Jan 25, 2019 · cc74a9a · cc74a9a
1 parent 6f2c036
commit cc74a9a
Show file tree

Hide file tree

Showing 10 changed files with 127 additions and 320 deletions.
diff --git a/virtcontainers/api_test.go b/virtcontainers/api_test.go
@@ -155,7 +155,6 @@ func newTestSandboxConfigHyperstartAgentDefaultNetwork() SandboxConfig {
 		AgentType:   HyperstartAgent,
 		AgentConfig: agentConfig,
 
-		NetworkModel:  DefaultNetworkModel,
 		NetworkConfig: netConfig,
 
 		Containers:  []ContainerConfig{container},
@@ -2020,7 +2019,7 @@ func TestProcessListContainer(t *testing.T) {
  * Benchmarks
  */
 
-func createNewSandboxConfig(hType HypervisorType, aType AgentType, aConfig interface{}, netModel NetworkModel) SandboxConfig {
+func createNewSandboxConfig(hType HypervisorType, aType AgentType, aConfig interface{}) SandboxConfig {
 	hypervisorConfig := HypervisorConfig{
 		KernelPath:     "/usr/share/kata-containers/vmlinux.container",
 		ImagePath:      "/usr/share/kata-containers/kata-containers.img",
@@ -2037,7 +2036,6 @@ func createNewSandboxConfig(hType HypervisorType, aType AgentType, aConfig inter
 		AgentType:   aType,
 		AgentConfig: aConfig,
 
-		NetworkModel:  netModel,
 		NetworkConfig: netConfig,
 	}
 }
@@ -2187,7 +2185,7 @@ func createStartStopDeleteContainers(b *testing.B, sandboxConfig SandboxConfig,
 
 func BenchmarkCreateStartStopDeleteSandboxQemuHypervisorHyperstartAgentNetworkNoop(b *testing.B) {
 	for i := 0; i < b.N; i++ {
-		sandboxConfig := createNewSandboxConfig(QemuHypervisor, HyperstartAgent, HyperConfig{}, NoopNetworkModel)
+		sandboxConfig := createNewSandboxConfig(QemuHypervisor, HyperstartAgent, HyperConfig{})
 
 		sockDir, err := testGenerateCCProxySockDir()
 		if err != nil {
@@ -2208,21 +2206,21 @@ func BenchmarkCreateStartStopDeleteSandboxQemuHypervisorHyperstartAgentNetworkNo
 
 func BenchmarkCreateStartStopDeleteSandboxQemuHypervisorNoopAgentNetworkNoop(b *testing.B) {
 	for i := 0; i < b.N; i++ {
-		sandboxConfig := createNewSandboxConfig(QemuHypervisor, NoopAgentType, nil, NoopNetworkModel)
+		sandboxConfig := createNewSandboxConfig(QemuHypervisor, NoopAgentType, nil)
 		createStartStopDeleteSandbox(b, sandboxConfig)
 	}
 }
 
 func BenchmarkCreateStartStopDeleteSandboxMockHypervisorNoopAgentNetworkNoop(b *testing.B) {
 	for i := 0; i < b.N; i++ {
-		sandboxConfig := createNewSandboxConfig(MockHypervisor, NoopAgentType, nil, NoopNetworkModel)
+		sandboxConfig := createNewSandboxConfig(MockHypervisor, NoopAgentType, nil)
 		createStartStopDeleteSandbox(b, sandboxConfig)
 	}
 }
 
 func BenchmarkStartStop1ContainerQemuHypervisorHyperstartAgentNetworkNoop(b *testing.B) {
 	for i := 0; i < b.N; i++ {
-		sandboxConfig := createNewSandboxConfig(QemuHypervisor, HyperstartAgent, HyperConfig{}, NoopNetworkModel)
+		sandboxConfig := createNewSandboxConfig(QemuHypervisor, HyperstartAgent, HyperConfig{})
 		contConfigs := createNewContainerConfigs(1)
 
 		sockDir, err := testGenerateCCProxySockDir()
@@ -2244,7 +2242,7 @@ func BenchmarkStartStop1ContainerQemuHypervisorHyperstartAgentNetworkNoop(b *tes
 
 func BenchmarkStartStop10ContainerQemuHypervisorHyperstartAgentNetworkNoop(b *testing.B) {
 	for i := 0; i < b.N; i++ {
-		sandboxConfig := createNewSandboxConfig(QemuHypervisor, HyperstartAgent, HyperConfig{}, NoopNetworkModel)
+		sandboxConfig := createNewSandboxConfig(QemuHypervisor, HyperstartAgent, HyperConfig{})
 		contConfigs := createNewContainerConfigs(10)
 
 		sockDir, err := testGenerateCCProxySockDir()
@@ -2448,7 +2446,6 @@ func TestNetworkOperation(t *testing.T) {
 	defer deleteNetNS(netNSPath)
 
 	config := newTestSandboxConfigNoop()
-	config.NetworkModel = DefaultNetworkModel
 	config.NetworkConfig = NetworkConfig{
 		NetNSPath: netNSPath,
 	}

diff --git a/virtcontainers/default_network.go b/virtcontainers/default_network.go
diff --git a/virtcontainers/iostream_test.go b/virtcontainers/iostream_test.go
@@ -13,7 +13,7 @@ import (
 
 func TestIOStream(t *testing.T) {
 	hConfig := newHypervisorConfig(nil, nil)
-	s, err := testCreateSandbox(t, testSandboxID, MockHypervisor, hConfig, NoopAgentType, NoopNetworkModel, NetworkConfig{}, []ContainerConfig{}, nil)
+	s, err := testCreateSandbox(t, testSandboxID, MockHypervisor, hConfig, NoopAgentType, NetworkConfig{}, []ContainerConfig{}, nil)
 	if err != nil {
 		t.Fatal(err)
 	}

diff --git a/virtcontainers/monitor_test.go b/virtcontainers/monitor_test.go
@@ -18,7 +18,7 @@ func TestMonitorSuccess(t *testing.T) {
 	hConfig := newHypervisorConfig(nil, nil)
 
 	// create a sandbox
-	s, err := testCreateSandbox(t, testSandboxID, MockHypervisor, hConfig, NoopAgentType, NoopNetworkModel, NetworkConfig{}, []ContainerConfig{contConfig}, nil)
+	s, err := testCreateSandbox(t, testSandboxID, MockHypervisor, hConfig, NoopAgentType, NetworkConfig{}, []ContainerConfig{contConfig}, nil)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -43,7 +43,7 @@ func TestMonitorClosedChannel(t *testing.T) {
 	hConfig := newHypervisorConfig(nil, nil)
 
 	// create a sandbox
-	s, err := testCreateSandbox(t, testSandboxID, MockHypervisor, hConfig, NoopAgentType, NoopNetworkModel, NetworkConfig{}, []ContainerConfig{contConfig}, nil)
+	s, err := testCreateSandbox(t, testSandboxID, MockHypervisor, hConfig, NoopAgentType, NetworkConfig{}, []ContainerConfig{contConfig}, nil)
 	if err != nil {
 		t.Fatal(err)
 	}

diff --git a/virtcontainers/network.go b/virtcontainers/network.go
@@ -6,6 +6,7 @@
 package virtcontainers
 
 import (
+	"context"
 	cryptoRand "crypto/rand"
 	"encoding/json"
 	"fmt"
@@ -17,6 +18,7 @@ import (
 	"time"
 
 	"github.com/containernetworking/plugins/pkg/ns"
+	opentracing "github.com/opentracing/opentracing-go"
 	"github.com/sirupsen/logrus"
 	"github.com/vishvananda/netlink"
 	"github.com/vishvananda/netns"
@@ -347,55 +349,6 @@ func (n *NetworkNamespace) UnmarshalJSON(b []byte) error {
 	return nil
 }
 
-// NetworkModel describes the type of network specification.
-type NetworkModel string
-
-const (
-	// NoopNetworkModel is the No-Op network.
-	NoopNetworkModel NetworkModel = "noop"
-
-	// DefaultNetworkModel is the default network.
-	DefaultNetworkModel NetworkModel = "default"
-)
-
-// Set sets a network type based on the input string.
-func (networkType *NetworkModel) Set(value string) error {
-	switch value {
-	case "noop":
-		*networkType = NoopNetworkModel
-		return nil
-	case "default":
-		*networkType = DefaultNetworkModel
-		return nil
-	default:
-		return fmt.Errorf("Unknown network type %s", value)
-	}
-}
-
-// String converts a network type to a string.
-func (networkType *NetworkModel) String() string {
-	switch *networkType {
-	case NoopNetworkModel:
-		return string(NoopNetworkModel)
-	case DefaultNetworkModel:
-		return string(DefaultNetworkModel)
-	default:
-		return ""
-	}
-}
-
-// newNetwork returns a network from a network type.
-func newNetwork(networkType NetworkModel) network {
-	switch networkType {
-	case NoopNetworkModel:
-		return &noopNetwork{}
-	case DefaultNetworkModel:
-		return &defNetwork{}
-	default:
-		return &noopNetwork{}
-	}
-}
-
 func createLink(netHandle *netlink.Handle, name string, expectedLink netlink.Link, queues int) (netlink.Link, []*os.File, error) {
 	var newLink netlink.Link
 	var fds []*os.File
@@ -1464,17 +1417,93 @@ func createEndpoint(netInfo NetworkInfo, idx int, model NetInterworkingModel) (E
 	return endpoint, err
 }
 
-// network is the virtcontainers network interface.
-// Container network plugins are used to setup virtual network
-// between VM netns and the host network physical interface.
-type network interface {
-	// run runs a callback function in a specified network namespace.
-	run(networkNSPath string, cb func() error) error
+// Network is the virtcontainer network structure
+type Network struct {
+}
+
+func (n *Network) trace(ctx context.Context, name string) (opentracing.Span, context.Context) {
+	span, ct := opentracing.StartSpanFromContext(ctx, name)
+
+	span.SetTag("subsystem", "network")
+	span.SetTag("type", "default")
+
+	return span, ct
+}
+
+// Run runs a callback in the specified network namespace.
+func (n *Network) Run(networkNSPath string, cb func() error) error {
+	span, _ := n.trace(context.Background(), "run")
+	defer span.Finish()
+
+	return doNetNS(networkNSPath, func(_ ns.NetNS) error {
+		return cb()
+	})
+}
+
+// Add adds all needed interfaces inside the network namespace.
+func (n *Network) Add(s *Sandbox, hotplug bool) error {
+	span, _ := n.trace(s.ctx, "add")
+	defer span.Finish()
 
-	// add adds all needed interfaces inside the network namespace.
-	add(sandbox *Sandbox, hotplug bool) error
+	endpoints, err := createEndpointsFromScan(s.config.NetworkConfig.NetNSPath, s.config.NetworkConfig)
+	if err != nil {
+		return err
+	}
+
+	s.networkNS.Endpoints = endpoints
+
+	err = doNetNS(s.config.NetworkConfig.NetNSPath, func(_ ns.NetNS) error {
+		for _, endpoint := range s.networkNS.Endpoints {
+			networkLogger().WithField("endpoint-type", endpoint.Type()).WithField("hotplug", hotplug).Info("Attaching endpoint")
+			if hotplug {
+				if err := endpoint.HotAttach(s.hypervisor); err != nil {
+					return err
+				}
+			} else {
+				if err := endpoint.Attach(s.hypervisor); err != nil {
+					return err
+				}
+			}
+		}
+
+		return nil
+	})
+	if err != nil {
+		return err
+	}
 
-	// remove unbridges and deletes TAP interfaces. It also removes virtual network
-	// interfaces and deletes the network namespace.
-	remove(sandbox *Sandbox, hotunplug bool) error
+	networkLogger().Debug("Network added")
+
+	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 *Network) Remove(s *Sandbox, hotunplug bool) error {
+	span, _ := n.trace(s.ctx, "remove")
+	defer span.Finish()
+
+	for _, endpoint := range s.networkNS.Endpoints {
+		// Detach for an endpoint should enter the network namespace
+		// if required.
+		networkLogger().WithField("endpoint-type", endpoint.Type()).WithField("hotunplug", hotunplug).Info("Detaching endpoint")
+		if hotunplug {
+			if err := endpoint.HotDetach(s.hypervisor, s.networkNS.NetNsCreated, s.networkNS.NetNsPath); err != nil {
+				return err
+			}
+		} else {
+			if err := endpoint.Detach(s.networkNS.NetNsCreated, s.networkNS.NetNsPath); err != nil {
+				return err
+			}
+		}
+	}
+
+	networkLogger().Debug("Network removed")
+
+	if s.networkNS.NetNsCreated {
+		networkLogger().Infof("Network namespace %q deleted", s.networkNS.NetNsPath)
+		return deleteNetNS(s.networkNS.NetNsPath)
+	}
+
+	return nil
 }