Merge pull request #872 from bergwolf/network-new

enable default network endpoints hotplug for vm factory

virtcontainers/default_network.go

@@ -40,7 +40,7 @@ func (n *defNetwork) run(networkNSPath string, cb func() error) error {
 }
 
 // add adds all needed interfaces inside the network namespace.
-func (n *defNetwork) add(s *Sandbox) error {
+func (n *defNetwork) add(s *Sandbox, hotplug bool) error {
 	span, _ := n.trace(s.ctx, "add")
 	defer span.Finish()
 
@@ -53,9 +53,15 @@ func (n *defNetwork) add(s *Sandbox) error {
 
 	err = doNetNS(s.config.NetworkConfig.NetNSPath, func(_ ns.NetNS) error {
 		for _, endpoint := range s.networkNS.Endpoints {
-			n.logger().WithField("endpoint-type", endpoint.Type()).Info("Attaching endpoint")
+			n.logger().WithField("endpoint-type", endpoint.Type()).WithField("hotplug", hotplug).Info("Attaching endpoint")
-			if err := endpoint.Attach(s.hypervisor); err != nil {
+			if hotplug {
-				return err
+				if err := endpoint.HotAttach(s.hypervisor); err != nil {
+					return err
+				}
+			} else {
+				if err := endpoint.Attach(s.hypervisor); err != nil {
+					return err
+				}
 			}
 		}
 
@@ -72,16 +78,22 @@ func (n *defNetwork) add(s *Sandbox) error {
 
 // 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(s *Sandbox) error {
+func (n *defNetwork) 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.
-		n.logger().WithField("endpoint-type", endpoint.Type()).Info("Detaching endpoint")
+		n.logger().WithField("endpoint-type", endpoint.Type()).WithField("hotunplug", hotunplug).Info("Detaching endpoint")
-		if err := endpoint.Detach(s.networkNS.NetNsCreated, s.networkNS.NetNsPath); err != nil {
+		if hotunplug {
-			return err
+			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
+			}
 		}
 	}
 

virtcontainers/network.go

@@ -1194,6 +1194,7 @@ func generateInterfacesAndRoutes(networkNS NetworkNamespace) ([]*types.Interface
 			Name:        endpoint.Name(),
 			Mtu:         uint64(endpoint.Properties().Iface.MTU),
 			HwAddr:      endpoint.HardwareAddr(),
+			PciAddr:     endpoint.PciAddr(),
 		}
 
 		ifaces = append(ifaces, &ifc)
@@ -1429,9 +1430,9 @@ type network interface {
 	run(networkNSPath string, cb func() error) error
 
 	// add adds all needed interfaces inside the network namespace.
-	add(sandbox *Sandbox) error
+	add(sandbox *Sandbox, hotplug bool) error
 
 	// remove unbridges and deletes TAP interfaces. It also removes virtual network
 	// interfaces and deletes the network namespace.
-	remove(sandbox *Sandbox) error
+	remove(sandbox *Sandbox, hotunplug bool) error
 }

virtcontainers/noop_network.go

@@ -19,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) error {
+func (n *noopNetwork) add(sandbox *Sandbox, hotattach bool) error {
 	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) error {
+func (n *noopNetwork) remove(sandbox *Sandbox, hotdetach bool) error {
 	return nil
 }

virtcontainers/sandbox.go

@@ -1049,22 +1049,18 @@ func (s *Sandbox) createNetwork() error {
 		NetNsCreated: s.config.NetworkConfig.NetNsCreated,
 	}
 
-	// In case there is a factory, the network should be handled
+	// In case there is a factory, network interfaces are hotplugged
-	// through some calls at the API level, in order to add or
+	// after vm is started.
-	// remove interfaces and routes.
-	// This prevents from any assumptions that could be made from
-	// virtcontainers, in particular that the VM has not been started
-	// before it starts to scan the current network.
 	if s.factory == nil {
 		// Add the network
-		if err := s.network.add(s); err != nil {
+		if err := s.network.add(s, false); err != nil {
 			return err
 		}
-	}
 
-	if s.config.NetworkConfig.NetmonConfig.Enable {
+		if s.config.NetworkConfig.NetmonConfig.Enable {
-		if err := s.startNetworkMonitor(); err != nil {
+			if err := s.startNetworkMonitor(); err != nil {
-			return err
+				return err
+			}
 		}
 	}
 
@@ -1082,14 +1078,7 @@ func (s *Sandbox) removeNetwork() error {
 		}
 	}
 
-	// In case there is a factory, the network has been handled through
+	return s.network.remove(s, s.factory != nil)
-	// some API calls to hotplug some interfaces and routes. This means
-	// the removal of the network should follow the same logic.
-	if s.factory != nil {
-		return nil
-	}
-
-	return s.network.remove(s)
 }
 
 func (s *Sandbox) generateNetInfo(inf *types.Interface) (NetworkInfo, error) {
@@ -1200,8 +1189,6 @@ func (s *Sandbox) startVM() error {
 
 	s.Logger().Info("Starting VM")
 
-	// FIXME: This would break cached VMs. We need network hotplug and move
-	// oci hooks and netns handling to cli. See #273.
 	if err := s.network.run(s.networkNS.NetNsPath, func() error {
 		if s.factory != nil {
 			vm, err := s.factory.GetVM(ctx, VMConfig{
@@ -1231,6 +1218,24 @@ func (s *Sandbox) startVM() error {
 		return err
 	}
 
+	// In case of vm factory, network interfaces are hotplugged
+	// after vm is started.
+	if s.factory != nil {
+		if err := s.network.add(s, true); err != nil {
+			return err
+		}
+
+		if s.config.NetworkConfig.NetmonConfig.Enable {
+			if err := s.startNetworkMonitor(); err != nil {
+				return err
+			}
+		}
+		if err := s.storage.storeSandboxNetwork(s.id, s.networkNS); err != nil {
+			return err
+		}
+	}
+
+	// Store the network
 	s.Logger().Info("VM started")
 
 	return nil