Merge pull request #46315 from thaJeztah/libnetwork_remove_sandbox_interface_step2

libnetwork/osl: Namespace: assorted cleanups

libnetwork/osl/interface_linux.go

@@ -14,6 +14,31 @@ import (
 	"github.com/vishvananda/netns"
 )
 
+// newInterface creates a new interface in the given namespace using the
+// provided options.
+func newInterface(ns *Namespace, srcName, dstPrefix string, options ...IfaceOption) (*Interface, error) {
+	i := &Interface{
+		srcName: srcName,
+		dstName: dstPrefix,
+		ns:      ns,
+	}
+	for _, opt := range options {
+		if opt != nil {
+			// TODO(thaJeztah): use multi-error instead of returning early.
+			if err := opt(i); err != nil {
+				return nil, err
+			}
+		}
+	}
+	if i.master != "" {
+		i.dstMaster = ns.findDst(i.master, true)
+		if i.dstMaster == "" {
+			return nil, fmt.Errorf("could not find an appropriate master %q for %q", i.master, i.srcName)
+		}
+	}
+	return i, nil
+}
+
 // Interface represents the settings and identity of a network device.
 // It is used as a return type for Network.Link, and it is common practice
 // for the caller to use this information when moving interface SrcName from
@@ -88,53 +113,8 @@ func (i *Interface) Routes() []*net.IPNet {
 // Remove an interface from the sandbox by renaming to original name
 // and moving it out of the sandbox.
 func (i *Interface) Remove() error {
-	i.ns.Lock()
-	isDefault := i.ns.isDefault
-	nlh := i.ns.nlHandle
-	i.ns.Unlock()
-
-	// Find the network interface identified by the DstName attribute.
-	iface, err := nlh.LinkByName(i.DstName())
-	if err != nil {
-		return err
-	}
-
-	// Down the interface before configuring
-	if err := nlh.LinkSetDown(iface); err != nil {
-		return err
-	}
-
-	err = nlh.LinkSetName(iface, i.SrcName())
-	if err != nil {
-		log.G(context.TODO()).Debugf("LinkSetName failed for interface %s: %v", i.SrcName(), err)
-		return err
-	}
-
-	// if it is a bridge just delete it.
-	if i.Bridge() {
-		if err := nlh.LinkDel(iface); err != nil {
-			return fmt.Errorf("failed deleting bridge %q: %v", i.SrcName(), err)
-		}
-	} else if !isDefault {
-		// Move the network interface to caller namespace.
-		if err := nlh.LinkSetNsFd(iface, ns.ParseHandlerInt()); err != nil {
-			log.G(context.TODO()).Debugf("LinkSetNsPid failed for interface %s: %v", i.SrcName(), err)
-			return err
-		}
-	}
-
-	i.ns.Lock()
-	for index, intf := range i.ns.iFaces {
-		if intf == i {
-			i.ns.iFaces = append(i.ns.iFaces[:index], i.ns.iFaces[index+1:]...)
-			break
-		}
-	}
-	i.ns.Unlock()
-
-	i.ns.checkLoV6()
-
-	return nil
+	nameSpace := i.ns
+	return nameSpace.RemoveInterface(i)
 }
 
 // Statistics returns the sandbox's side veth interface statistics.
@@ -160,8 +140,8 @@ func (i *Interface) Statistics() (*types.InterfaceStatistics, error) {
 }
 
 func (n *Namespace) findDst(srcName string, isBridge bool) string {
-	n.Lock()
-	defer n.Unlock()
+	n.mu.Lock()
+	defer n.mu.Unlock()
 
 	for _, i := range n.iFaces {
 		// The master should match the srcname of the interface and the
@@ -180,24 +160,12 @@ func (n *Namespace) findDst(srcName string, isBridge bool) string {
 // to only provide a prefix for DstName. The AddInterface api will auto-generate
 // an appropriate suffix for the DstName to disambiguate.
 func (n *Namespace) AddInterface(srcName, dstPrefix string, options ...IfaceOption) error {
-	i := &Interface{
-		srcName: srcName,
-		dstName: dstPrefix,
-		ns:      n,
-	}
-	if err := i.processInterfaceOptions(options...); err != nil {
+	i, err := newInterface(n, srcName, dstPrefix, options...)
+	if err != nil {
 		return err
 	}
 
-	if i.master != "" {
-		i.dstMaster = n.findDst(i.master, true)
-		if i.dstMaster == "" {
-			return fmt.Errorf("could not find an appropriate master %q for %q",
-				i.master, i.srcName)
-		}
-	}
-
-	n.Lock()
+	n.mu.Lock()
 	if n.isDefault {
 		i.dstName = i.srcName
 	} else {
@@ -209,7 +177,7 @@ func (n *Namespace) AddInterface(srcName, dstPrefix string, options ...IfaceOpti
 	isDefault := n.isDefault
 	nlh := n.nlHandle
 	nlhHost := ns.NlHandle()
-	n.Unlock()
+	n.mu.Unlock()
 
 	// If it is a bridge interface we have to create the bridge inside
 	// the namespace so don't try to lookup the interface using srcName
@@ -285,15 +253,69 @@ func (n *Namespace) AddInterface(srcName, dstPrefix string, options ...IfaceOpti
 		return fmt.Errorf("error setting interface %q routes to %q: %v", iface.Attrs().Name, i.Routes(), err)
 	}
 
-	n.Lock()
+	n.mu.Lock()
 	n.iFaces = append(n.iFaces, i)
-	n.Unlock()
+	n.mu.Unlock()
 
 	n.checkLoV6()
 
 	return nil
 }
 
+// RemoveInterface removes an interface from the namespace by renaming to
+// original name and moving it out of the sandbox.
+func (n *Namespace) RemoveInterface(i *Interface) error {
+	n.mu.Lock()
+	isDefault := n.isDefault
+	nlh := n.nlHandle
+	n.mu.Unlock()
+
+	// Find the network interface identified by the DstName attribute.
+	iface, err := nlh.LinkByName(i.DstName())
+	if err != nil {
+		return err
+	}
+
+	// Down the interface before configuring
+	if err := nlh.LinkSetDown(iface); err != nil {
+		return err
+	}
+
+	// TODO(aker): Why are we doing this? This would fail if the initial interface set up failed before the "dest interface" was moved into its own namespace; see https://github.com/moby/moby/pull/46315/commits/108595c2fe852a5264b78e96f9e63cda284990a6#r1331253578
+	err = nlh.LinkSetName(iface, i.SrcName())
+	if err != nil {
+		log.G(context.TODO()).Debugf("LinkSetName failed for interface %s: %v", i.SrcName(), err)
+		return err
+	}
+
+	// if it is a bridge just delete it.
+	if i.Bridge() {
+		if err := nlh.LinkDel(iface); err != nil {
+			return fmt.Errorf("failed deleting bridge %q: %v", i.SrcName(), err)
+		}
+	} else if !isDefault {
+		// Move the network interface to caller namespace.
+		// TODO(aker): What's this really doing? There are no calls to LinkDel in this package: is this code really used? (Interface.Remove() has 3 callers); see https://github.com/moby/moby/pull/46315/commits/108595c2fe852a5264b78e96f9e63cda284990a6#r1331265335
+		if err := nlh.LinkSetNsFd(iface, ns.ParseHandlerInt()); err != nil {
+			log.G(context.TODO()).Debugf("LinkSetNsFd failed for interface %s: %v", i.SrcName(), err)
+			return err
+		}
+	}
+
+	n.mu.Lock()
+	for index, intf := range i.ns.iFaces {
+		if intf == i {
+			i.ns.iFaces = append(i.ns.iFaces[:index], i.ns.iFaces[index+1:]...)
+			break
+		}
+	}
+	n.mu.Unlock()
+
+	// TODO(aker): This function will disable IPv6 on lo interface if the removed interface was the last one offering IPv6 connectivity. That's a weird behavior, and shouldn't be hiding this deep down in this function.
+	n.checkLoV6()
+	return nil
+}
+
 func configureInterface(nlh *netlink.Handle, iface netlink.Link, i *Interface) error {
 	ifaceName := iface.Attrs().Name
 	ifaceConfigurators := []struct {

libnetwork/osl/namespace_linux.go

@@ -285,16 +285,16 @@ func createNetworkNamespace(path string, osCreate bool) error {
 }
 
 func unmountNamespaceFile(path string) {
-	if _, err := os.Stat(path); err == nil {
-		if err := syscall.Unmount(path, syscall.MNT_DETACH); err != nil && !errors.Is(err, unix.EINVAL) {
-			log.G(context.TODO()).WithError(err).Error("Error unmounting namespace file")
-		}
+	if _, err := os.Stat(path); err != nil {
+		// ignore when we cannot stat the path
+		return
+	}
+	if err := syscall.Unmount(path, syscall.MNT_DETACH); err != nil && !errors.Is(err, unix.EINVAL) {
+		log.G(context.TODO()).WithError(err).Error("Error unmounting namespace file")
 	}
 }
 
-func createNamespaceFile(path string) (err error) {
-	var f *os.File
-
+func createNamespaceFile(path string) error {
 	once.Do(createBasePath)
 	// Remove it from garbage collection list if present
 	removeFromGarbagePaths(path)
@@ -304,13 +304,16 @@ func createNamespaceFile(path string) (err error) {
 
 	// wait for garbage collection to complete if it is in progress
 	// before trying to create the file.
+	//
+	// TODO(aker): This garbage-collection was for a kernel bug in kernels 3.18-4.0.1: is this still needed on current kernels (and on kernel 3.10)? see https://github.com/moby/moby/pull/46315/commits/c0a6beba8e61d4019e1806d5241ba22007072ca2#r1331327103
 	gpmWg.Wait()
 
-	if f, err = os.Create(path); err == nil {
-		f.Close()
+	f, err := os.Create(path)
+	if err != nil {
+		return err
 	}
-
-	return err
+	_ = f.Close()
+	return nil
 }
 
 // Namespace represents a network sandbox. It represents a Linux network
@@ -328,7 +331,7 @@ type Namespace struct {
 	isDefault    bool
 	nlHandle     *netlink.Handle
 	loV6Enabled  bool
-	sync.Mutex
+	mu           sync.Mutex
 }
 
 // Interfaces returns the collection of Interface previously added with the AddInterface
@@ -450,8 +453,8 @@ func (n *Namespace) InvokeFunc(f func()) error {
 }
 
 func (n *Namespace) nsPath() string {
-	n.Lock()
-	defer n.Unlock()
+	n.mu.Lock()
+	defer n.mu.Unlock()
 
 	return n.path
 }
@@ -478,24 +481,13 @@ func (n *Namespace) Destroy() error {
 }
 
 // Restore restores the network namespace.
-func (n *Namespace) Restore(ifsopt map[Iface][]IfaceOption, routes []*types.StaticRoute, gw net.IP, gw6 net.IP) error {
+func (n *Namespace) Restore(interfaces map[Iface][]IfaceOption, routes []*types.StaticRoute, gw net.IP, gw6 net.IP) error {
 	// restore interfaces
-	for name, opts := range ifsopt {
-		i := &Interface{
-			srcName: name.SrcName,
-			dstName: name.DstPrefix,
-			ns:      n,
-		}
-		if err := i.processInterfaceOptions(opts...); err != nil {
+	for iface, opts := range interfaces {
+		i, err := newInterface(n, iface.SrcName, iface.DstPrefix, opts...)
+		if err != nil {
 			return err
 		}
-		if i.master != "" {
-			i.dstMaster = n.findDst(i.master, true)
-			if i.dstMaster == "" {
-				return fmt.Errorf("could not find an appropriate master %q for %q",
-					i.master, i.srcName)
-			}
-		}
 		if n.isDefault {
 			i.dstName = i.srcName
 		} else {
@@ -506,76 +498,60 @@ func (n *Namespace) Restore(ifsopt map[Iface][]IfaceOption, routes []*types.Stat
 			// due to the docker network connect/disconnect, so the dstName should
 			// restore from the namespace
 			for _, link := range links {
-				addrs, err := n.nlHandle.AddrList(link, netlink.FAMILY_V4)
-				if err != nil {
-					return err
-				}
 				ifaceName := link.Attrs().Name
-				if strings.HasPrefix(ifaceName, "vxlan") {
-					if i.dstName == "vxlan" {
-						i.dstName = ifaceName
-						break
-					}
+				if i.dstName == "vxlan" && strings.HasPrefix(ifaceName, "vxlan") {
+					i.dstName = ifaceName
+					break
 				}
 				// find the interface name by ip
 				if i.address != nil {
-					for _, addr := range addrs {
+					addresses, err := n.nlHandle.AddrList(link, netlink.FAMILY_V4)
+					if err != nil {
+						return err
+					}
+					for _, addr := range addresses {
 						if addr.IPNet.String() == i.address.String() {
 							i.dstName = ifaceName
 							break
 						}
-						continue
 					}
 					if i.dstName == ifaceName {
 						break
 					}
 				}
 				// This is to find the interface name of the pair in overlay sandbox
-				if strings.HasPrefix(ifaceName, "veth") {
-					if i.master != "" && i.dstName == "veth" {
-						i.dstName = ifaceName
-					}
+				if i.master != "" && i.dstName == "veth" && strings.HasPrefix(ifaceName, "veth") {
+					i.dstName = ifaceName
 				}
 			}
 
 			var index int
-			indexStr := strings.TrimPrefix(i.dstName, name.DstPrefix)
-			if indexStr != "" {
-				index, err = strconv.Atoi(indexStr)
+			if idx := strings.TrimPrefix(i.dstName, iface.DstPrefix); idx != "" {
+				index, err = strconv.Atoi(idx)
 				if err != nil {
-					return err
+					return fmt.Errorf("failed to restore interface in network namespace %q: invalid dstName for interface: %s: %v", n.path, i.dstName, err)
 				}
 			}
 			index++
-			n.Lock()
-			if index > n.nextIfIndex[name.DstPrefix] {
-				n.nextIfIndex[name.DstPrefix] = index
+			n.mu.Lock()
+			if index > n.nextIfIndex[iface.DstPrefix] {
+				n.nextIfIndex[iface.DstPrefix] = index
 			}
 			n.iFaces = append(n.iFaces, i)
-			n.Unlock()
+			n.mu.Unlock()
 		}
 	}
 
-	// restore routes
-	for _, r := range routes {
-		n.Lock()
-		n.staticRoutes = append(n.staticRoutes, r)
-		n.Unlock()
-	}
-
-	// restore gateway
+	// restore routes and gateways
+	n.mu.Lock()
+	n.staticRoutes = append(n.staticRoutes, routes...)
 	if len(gw) > 0 {
-		n.Lock()
 		n.gw = gw
-		n.Unlock()
 	}
-
 	if len(gw6) > 0 {
-		n.Lock()
 		n.gwv6 = gw6
-		n.Unlock()
 	}
-
+	n.mu.Unlock()
 	return nil
 }
 
@@ -586,7 +562,7 @@ func (n *Namespace) checkLoV6() {
 		action = "disable"
 	)
 
-	n.Lock()
+	n.mu.Lock()
 	for _, iface := range n.iFaces {
 		if iface.AddressIPv6() != nil {
 			enable = true
@@ -594,7 +570,7 @@ func (n *Namespace) checkLoV6() {
 			break
 		}
 	}
-	n.Unlock()
+	n.mu.Unlock()
 
 	if n.loV6Enabled == enable {
 		return

libnetwork/osl/neigh_linux.go

@@ -32,8 +32,8 @@ type neigh struct {
 }
 
 func (n *Namespace) findNeighbor(dstIP net.IP, dstMac net.HardwareAddr) *neigh {
-	n.Lock()
-	defer n.Unlock()
+	n.mu.Lock()
+	defer n.mu.Unlock()
 
 	for _, nh := range n.neighbors {
 		if nh.dstIP.Equal(dstIP) && bytes.Equal(nh.dstMac, dstMac) {
@@ -51,9 +51,9 @@ func (n *Namespace) DeleteNeighbor(dstIP net.IP, dstMac net.HardwareAddr) error
 		return NeighborSearchError{dstIP, dstMac, false}
 	}
 
-	n.Lock()
+	n.mu.Lock()
 	nlh := n.nlHandle
-	n.Unlock()
+	n.mu.Unlock()
 
 	var linkIndex int
 	if nh.linkDst != "" {
@@ -96,14 +96,14 @@ func (n *Namespace) DeleteNeighbor(dstIP net.IP, dstMac net.HardwareAddr) error
 		}
 	}
 
-	n.Lock()
+	n.mu.Lock()
 	for i, neighbor := range n.neighbors {
 		if neighbor.dstIP.Equal(dstIP) && bytes.Equal(neighbor.dstMac, dstMac) {
 			n.neighbors = append(n.neighbors[:i], n.neighbors[i+1:]...)
 			break
 		}
 	}
-	n.Unlock()
+	n.mu.Unlock()
 	log.G(context.TODO()).Debugf("Neighbor entry deleted for IP %v, mac %v", dstIP, dstMac)
 
 	return nil
@@ -142,9 +142,9 @@ func (n *Namespace) AddNeighbor(dstIP net.IP, dstMac net.HardwareAddr, force boo
 		}
 	}
 
-	n.Lock()
+	n.mu.Lock()
 	nlh := n.nlHandle
-	n.Unlock()
+	n.mu.Unlock()
 
 	if nh.linkDst != "" {
 		iface, err = nlh.LinkByName(nh.linkDst)
@@ -176,9 +176,9 @@ func (n *Namespace) AddNeighbor(dstIP net.IP, dstMac net.HardwareAddr, force boo
 		return nil
 	}
 
-	n.Lock()
+	n.mu.Lock()
 	n.neighbors = append(n.neighbors, nh)
-	n.Unlock()
+	n.mu.Unlock()
 	log.G(context.TODO()).Debugf("Neighbor entry added for IP:%v, mac:%v on ifc:%s", dstIP, dstMac, nh.linkName)
 
 	return nil

libnetwork/osl/options_linux.go

@@ -24,18 +24,6 @@ func WithFamily(family int) NeighOption {
 	}
 }
 
-func (i *Interface) processInterfaceOptions(options ...IfaceOption) error {
-	for _, opt := range options {
-		if opt != nil {
-			// TODO(thaJeztah): use multi-error instead of returning early.
-			if err := opt(i); err != nil {
-				return err
-			}
-		}
-	}
-	return nil
-}
-
 // WithIsBridge sets whether the interface is a bridge.
 func WithIsBridge(isBridge bool) IfaceOption {
 	return func(i *Interface) error {

libnetwork/osl/route_linux.go

@@ -10,16 +10,16 @@ import (
 
 // Gateway returns the IPv4 gateway for the sandbox.
 func (n *Namespace) Gateway() net.IP {
-	n.Lock()
-	defer n.Unlock()
+	n.mu.Lock()
+	defer n.mu.Unlock()
 
 	return n.gw
 }
 
 // GatewayIPv6 returns the IPv6 gateway for the sandbox.
 func (n *Namespace) GatewayIPv6() net.IP {
-	n.Lock()
-	defer n.Unlock()
+	n.mu.Lock()
+	defer n.mu.Unlock()
 
 	return n.gwv6
 }
@@ -28,8 +28,8 @@ func (n *Namespace) GatewayIPv6() net.IP {
 // directly connected routes are stored on the particular interface they
 // refer to.
 func (n *Namespace) StaticRoutes() []*types.StaticRoute {
-	n.Lock()
-	defer n.Unlock()
+	n.mu.Lock()
+	defer n.mu.Unlock()
 
 	routes := make([]*types.StaticRoute, len(n.staticRoutes))
 	for i, route := range n.staticRoutes {
@@ -40,48 +40,37 @@ func (n *Namespace) StaticRoutes() []*types.StaticRoute {
 	return routes
 }
 
-func (n *Namespace) setGateway(gw net.IP) {
-	n.Lock()
-	n.gw = gw
-	n.Unlock()
-}
-
-func (n *Namespace) setGatewayIPv6(gwv6 net.IP) {
-	n.Lock()
-	n.gwv6 = gwv6
-	n.Unlock()
-}
-
-// SetGateway sets the default IPv4 gateway for the sandbox.
+// SetGateway sets the default IPv4 gateway for the sandbox. It is a no-op
+// if the given gateway is empty.
 func (n *Namespace) SetGateway(gw net.IP) error {
-	// Silently return if the gateway is empty
 	if len(gw) == 0 {
 		return nil
 	}
 
-	err := n.programGateway(gw, true)
-	if err == nil {
-		n.setGateway(gw)
+	if err := n.programGateway(gw, true); err != nil {
+		return err
 	}
-
-	return err
+	n.mu.Lock()
+	n.gw = gw
+	n.mu.Unlock()
+	return nil
 }
 
 // UnsetGateway the previously set default IPv4 gateway in the sandbox.
+// It is a no-op if no gateway was set.
 func (n *Namespace) UnsetGateway() error {
 	gw := n.Gateway()
-
-	// Silently return if the gateway is empty
 	if len(gw) == 0 {
 		return nil
 	}
 
-	err := n.programGateway(gw, false)
-	if err == nil {
-		n.setGateway(net.IP{})
+	if err := n.programGateway(gw, false); err != nil {
+		return err
 	}
-
-	return err
+	n.mu.Lock()
+	n.gw = net.IP{}
+	n.mu.Unlock()
+	return nil
 }
 
 func (n *Namespace) programGateway(gw net.IP, isAdd bool) error {
@@ -99,7 +88,7 @@ func (n *Namespace) programGateway(gw net.IP, isAdd bool) error {
 	}
 
 	if linkIndex == 0 {
-		return fmt.Errorf("Direct route for the gateway %s could not be found", gw)
+		return fmt.Errorf("direct route for the gateway %s could not be found", gw)
 	}
 
 	if isAdd {
@@ -118,7 +107,7 @@ func (n *Namespace) programGateway(gw net.IP, isAdd bool) error {
 }
 
 // Program a route in to the namespace routing table.
-func (n *Namespace) programRoute(path string, dest *net.IPNet, nh net.IP) error {
+func (n *Namespace) programRoute(dest *net.IPNet, nh net.IP) error {
 	gwRoutes, err := n.nlHandle.RouteGet(nh)
 	if err != nil {
 		return fmt.Errorf("route for the next hop %s could not be found: %v", nh, err)
@@ -133,7 +122,7 @@ func (n *Namespace) programRoute(path string, dest *net.IPNet, nh net.IP) error
 }
 
 // Delete a route from the namespace routing table.
-func (n *Namespace) removeRoute(path string, dest *net.IPNet, nh net.IP) error {
+func (n *Namespace) removeRoute(dest *net.IPNet, nh net.IP) error {
 	gwRoutes, err := n.nlHandle.RouteGet(nh)
 	if err != nil {
 		return fmt.Errorf("route for the next hop could not be found: %v", err)
@@ -147,67 +136,70 @@ func (n *Namespace) removeRoute(path string, dest *net.IPNet, nh net.IP) error {
 	})
 }
 
-// SetGatewayIPv6 sets the default IPv6 gateway for the sandbox.
+// SetGatewayIPv6 sets the default IPv6 gateway for the sandbox. It is a no-op
+// if the given gateway is empty.
 func (n *Namespace) SetGatewayIPv6(gwv6 net.IP) error {
-	// Silently return if the gateway is empty
 	if len(gwv6) == 0 {
 		return nil
 	}
 
-	err := n.programGateway(gwv6, true)
-	if err == nil {
-		n.setGatewayIPv6(gwv6)
+	if err := n.programGateway(gwv6, true); err != nil {
+		return err
 	}
 
-	return err
+	n.mu.Lock()
+	n.gwv6 = gwv6
+	n.mu.Unlock()
+	return nil
 }
 
 // UnsetGatewayIPv6 unsets the previously set default IPv6 gateway in the sandbox.
+// It is a no-op if no gateway was set.
 func (n *Namespace) UnsetGatewayIPv6() error {
 	gwv6 := n.GatewayIPv6()
-
-	// Silently return if the gateway is empty
 	if len(gwv6) == 0 {
 		return nil
 	}
 
-	err := n.programGateway(gwv6, false)
-	if err == nil {
-		n.Lock()
-		n.gwv6 = net.IP{}
-		n.Unlock()
+	if err := n.programGateway(gwv6, false); err != nil {
+		return err
 	}
 
-	return err
+	n.mu.Lock()
+	n.gwv6 = net.IP{}
+	n.mu.Unlock()
+	return nil
 }
 
 // AddStaticRoute adds a static route to the sandbox.
 func (n *Namespace) AddStaticRoute(r *types.StaticRoute) error {
-	err := n.programRoute(n.nsPath(), r.Destination, r.NextHop)
-	if err == nil {
-		n.Lock()
-		n.staticRoutes = append(n.staticRoutes, r)
-		n.Unlock()
+	if err := n.programRoute(r.Destination, r.NextHop); err != nil {
+		return err
 	}
-	return err
+
+	n.mu.Lock()
+	n.staticRoutes = append(n.staticRoutes, r)
+	n.mu.Unlock()
+	return nil
 }
 
 // RemoveStaticRoute removes a static route from the sandbox.
 func (n *Namespace) RemoveStaticRoute(r *types.StaticRoute) error {
-	err := n.removeRoute(n.nsPath(), r.Destination, r.NextHop)
-	if err == nil {
-		n.Lock()
-		lastIndex := len(n.staticRoutes) - 1
-		for i, v := range n.staticRoutes {
-			if v == r {
-				// Overwrite the route we're removing with the last element
-				n.staticRoutes[i] = n.staticRoutes[lastIndex]
-				// Shorten the slice to trim the extra element
-				n.staticRoutes = n.staticRoutes[:lastIndex]
-				break
-			}
+	if err := n.removeRoute(r.Destination, r.NextHop); err != nil {
+		return err
+	}
+
+	n.mu.Lock()
+	lastIndex := len(n.staticRoutes) - 1
+	for i, v := range n.staticRoutes {
+		if v == r {
+			// Overwrite the route we're removing with the last element
+			n.staticRoutes[i] = n.staticRoutes[lastIndex]
+			// Shorten the slice to trim the extra element
+			n.staticRoutes = n.staticRoutes[:lastIndex]
+			break
 		}
-		n.Unlock()
 	}
-	return err
+	n.mu.Unlock()
+	return nil
 }