diff --git a/virtcontainers/cni.go b/virtcontainers/cni.go index 96074b23b6..41e0e71e34 100644 --- a/virtcontainers/cni.go +++ b/virtcontainers/cni.go @@ -153,10 +153,10 @@ func (n *cni) add(sandbox *Sandbox, config NetworkConfig, netNsPath string, netN return networkNS, nil } -// remove unbridges and deletes TAP interfaces. It also removes virtual network -// interfaces and deletes the network namespace for the CNI network. -func (n *cni) remove(sandbox *Sandbox, networkNS NetworkNamespace) error { - if err := removeNetworkCommon(networkNS); err != 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 *cni) remove(sandbox *Sandbox, networkNS NetworkNamespace, netNsCreated bool) error { + if err := removeNetworkCommon(networkNS, netNsCreated); err != nil { return err } @@ -164,5 +164,9 @@ func (n *cni) remove(sandbox *Sandbox, networkNS NetworkNamespace) error { return err } - return deleteNetNS(networkNS.NetNsPath, true) + if netNsCreated { + return deleteNetNS(networkNS.NetNsPath) + } + + return nil } diff --git a/virtcontainers/cnm.go b/virtcontainers/cnm.go index 98c7c467fc..e565ce0e83 100644 --- a/virtcontainers/cnm.go +++ b/virtcontainers/cnm.go @@ -47,12 +47,16 @@ func (n *cnm) add(sandbox *Sandbox, config NetworkConfig, netNsPath string, netN return networkNS, nil } -// remove unbridges and deletes TAP interfaces. It also removes virtual network -// interfaces and deletes the network namespace for the CNM network. -func (n *cnm) remove(sandbox *Sandbox, networkNS NetworkNamespace) error { - if err := removeNetworkCommon(networkNS); err != 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 *cnm) remove(sandbox *Sandbox, networkNS NetworkNamespace, netNsCreated bool) error { + if err := removeNetworkCommon(networkNS, netNsCreated); err != nil { return err } - return deleteNetNS(networkNS.NetNsPath, true) + if netNsCreated { + return deleteNetNS(networkNS.NetNsPath) + } + + return nil } diff --git a/virtcontainers/network.go b/virtcontainers/network.go index fb6f109aab..53e188374b 100644 --- a/virtcontainers/network.go +++ b/virtcontainers/network.go @@ -153,7 +153,7 @@ type Endpoint interface { SetProperties(NetworkInfo) Attach(hypervisor) error - Detach() error + Detach(netNsCreated bool, netNsPath string) error } // VirtualEndpoint gathers a network pair and its properties. @@ -230,9 +230,18 @@ func (endpoint *VirtualEndpoint) Attach(h hypervisor) error { // Detach for the virtual endpoint tears down the tap and bridge // created for the veth interface. -func (endpoint *VirtualEndpoint) Detach() error { +func (endpoint *VirtualEndpoint) Detach(netNsCreated bool, netNsPath string) error { + // The network namespace would have been deleted at this point + // if it has not been created by virtcontainers. + if !netNsCreated { + return nil + } + networkLogger().Info("Detaching virtual endpoint") - return xconnectVMNetwork(&(endpoint.NetPair), false) + + return doNetNS(netNsPath, func(_ ns.NetNS) error { + return xconnectVMNetwork(&(endpoint.NetPair), false) + }) } // Properties returns the properties of the interface. @@ -281,7 +290,7 @@ func (endpoint *VhostUserEndpoint) Attach(h hypervisor) error { } // Detach for vhostuser endpoint -func (endpoint *VhostUserEndpoint) Detach() error { +func (endpoint *VhostUserEndpoint) Detach(netNsCreated bool, netNsPath string) error { networkLogger().Info("Detaching vhostuser based endpoint") return nil } @@ -343,9 +352,14 @@ func (endpoint *PhysicalEndpoint) Attach(h hypervisor) error { // Detach for physical endpoint unbinds the physical network interface from vfio-pci // and binds it back to the saved host driver. -func (endpoint *PhysicalEndpoint) Detach() error { +func (endpoint *PhysicalEndpoint) Detach(netNsCreated bool, netNsPath string) error { // Bind back the physical network interface to host. + // We need to do this even if a new network namespace has not + // been created by virtcontainers. networkLogger().Info("Detaching physical endpoint") + + // We do not need to enter the network namespace to bind back the + // physical interface to host driver. return bindNICToHost(endpoint) } @@ -606,16 +620,16 @@ func addNetworkCommon(sandbox *Sandbox, networkNS *NetworkNamespace) error { return err } -func removeNetworkCommon(networkNS NetworkNamespace) error { - return doNetNS(networkNS.NetNsPath, func(_ ns.NetNS) error { - for _, endpoint := range networkNS.Endpoints { - if err := endpoint.Detach(); err != nil { - return err - } +func removeNetworkCommon(networkNS NetworkNamespace, netNsCreated bool) error { + for _, endpoint := range networkNS.Endpoints { + // Detach for an endpoint should enter the network namespace + // if required. + if err := endpoint.Detach(netNsCreated, networkNS.NetNsPath); err != nil { + return err } + } - return nil - }) + return nil } func createLink(netHandle *netlink.Handle, name string, expectedLink netlink.Link) (netlink.Link, []*os.File, error) { @@ -1098,7 +1112,7 @@ func doNetNS(netNSPath string, cb func(ns.NetNS) error) error { return cb(targetNS) } -func deleteNetNS(netNSPath string, mounted bool) error { +func deleteNetNS(netNSPath string) error { n, err := ns.GetNS(netNSPath) if err != nil { return err @@ -1109,15 +1123,11 @@ func deleteNetNS(netNSPath string, mounted bool) error { return err } - // This unmount part is supposed to be done in the cni/ns package, but the "mounted" - // flag is not updated when retrieving NetNs handler from GetNS(). - if mounted { - if err = unix.Unmount(netNSPath, unix.MNT_DETACH); err != nil { - return fmt.Errorf("Failed to unmount namespace %s: %v", netNSPath, err) - } - if err := os.RemoveAll(netNSPath); err != nil { - return fmt.Errorf("Failed to clean up namespace %s: %v", netNSPath, err) - } + if err = unix.Unmount(netNSPath, unix.MNT_DETACH); err != nil { + return fmt.Errorf("Failed to unmount namespace %s: %v", netNSPath, err) + } + if err := os.RemoveAll(netNSPath); err != nil { + return fmt.Errorf("Failed to clean up namespace %s: %v", netNSPath, err) } return nil @@ -1410,5 +1420,5 @@ type network interface { // remove unbridges and deletes TAP interfaces. It also removes virtual network // interfaces and deletes the network namespace. - remove(sandbox *Sandbox, networkNS NetworkNamespace) error + remove(sandbox *Sandbox, networkNS NetworkNamespace, netNsCreated bool) error } diff --git a/virtcontainers/network_test.go b/virtcontainers/network_test.go index addd5e759e..21e999587f 100644 --- a/virtcontainers/network_test.go +++ b/virtcontainers/network_test.go @@ -123,7 +123,7 @@ func TestCreateDeleteNetNS(t *testing.T) { t.Fatal(err) } - err = deleteNetNS(netNSPath, true) + err = deleteNetNS(netNSPath) if err != nil { t.Fatal(err) } diff --git a/virtcontainers/noop_network.go b/virtcontainers/noop_network.go index 7d37476866..e4ef408753 100644 --- a/virtcontainers/noop_network.go +++ b/virtcontainers/noop_network.go @@ -32,6 +32,6 @@ func (n *noopNetwork) add(sandbox *Sandbox, config NetworkConfig, netNsPath stri // 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) error { +func (n *noopNetwork) remove(sandbox *Sandbox, networkNS NetworkNamespace, netNsCreated bool) error { return nil } diff --git a/virtcontainers/sandbox.go b/virtcontainers/sandbox.go index bf17fc5d9c..ba70fadb94 100644 --- a/virtcontainers/sandbox.go +++ b/virtcontainers/sandbox.go @@ -923,11 +923,7 @@ func (s *Sandbox) createNetwork() error { } func (s *Sandbox) removeNetwork() error { - if s.networkNS.NetNsCreated { - return s.network.remove(s, s.networkNS) - } - - return nil + return s.network.remove(s, s.networkNS, s.networkNS.NetNsCreated) } // startVM starts the VM. diff --git a/virtcontainers/utils/virtcontainers-setup.sh b/virtcontainers/utils/virtcontainers-setup.sh index d241ad8cd3..ef56c220a7 100755 --- a/virtcontainers/utils/virtcontainers-setup.sh +++ b/virtcontainers/utils/virtcontainers-setup.sh @@ -60,12 +60,13 @@ echo -e "Move to ${tmpdir}/${virtcontainers_build_dir}" pushd ${tmpdir}/${virtcontainers_build_dir} echo "Clone cni" git clone https://github.com/containernetworking/plugins.git +pushd plugins +git checkout 7f98c94613021d8b57acfa1a2f0c8d0f6fd7ae5a echo "Copy CNI config files" cp $GOPATH/src/github.com/kata-containers/runtime/virtcontainers/test/cni/10-mynet.conf ${ETCDIR}/cni/net.d/ cp $GOPATH/src/github.com/kata-containers/runtime/virtcontainers/test/cni/99-loopback.conf ${ETCDIR}/cni/net.d/ -pushd plugins ./build.sh cp ./bin/bridge ${TMPDIR}/cni/bin/cni-bridge cp ./bin/loopback ${TMPDIR}/cni/bin/loopback