Improve cleanup on network failure paths

network/endpoint.go

@@ -45,44 +45,51 @@ func (nw *network) newEndpoint(epInfo *EndpointInfo) (*endpoint, error) {
 	var err error
 
 	log.Printf("[net] Creating endpoint %+v in network %v.", epInfo, nw.Id)
+	defer func() {
+		if err != nil {
+			log.Printf("[net] Failed to create endpoint %v, err:%v.", epInfo.Id, err)
+		}
+	}()
 
 	if nw.Endpoints[epInfo.Id] != nil {
 		err = errEndpointExists
-		goto fail
+		return nil, err
 	}
 
 	// Call the platform implementation.
 	ep, err = nw.newEndpointImpl(epInfo)
 	if err != nil {
-		goto fail
+		return nil, err
 	}
 
 	nw.Endpoints[epInfo.Id] = ep
 
 	log.Printf("[net] Created endpoint %+v.", ep)
 
 	return ep, nil
-
-fail:
-	log.Printf("[net] Creating endpoint %v failed, err:%v.", epInfo.Id, err)
-
-	return nil, err
 }
 
 // DeleteEndpoint deletes an existing endpoint from the network.
 func (nw *network) deleteEndpoint(endpointId string) error {
+	var err error
+
 	log.Printf("[net] Deleting endpoint %v from network %v.", endpointId, nw.Id)
+	defer func() {
+		if err != nil {
+			log.Printf("[net] Failed to delete endpoint %v, err:%v.", endpointId, err)
+		}
+	}()
 
 	// Look up the endpoint.
 	ep, err := nw.getEndpoint(endpointId)
 	if err != nil {
-		goto fail
+		return err
 	}
 
 	// Call the platform implementation.
 	err = nw.deleteEndpointImpl(ep)
 	if err != nil {
-		goto fail
+		return err
 	}
 
 	// Remove the endpoint object.
@@ -91,11 +98,6 @@ func (nw *network) deleteEndpoint(endpointId string) error {
 	log.Printf("[net] Deleted endpoint %+v.", ep)
 
 	return nil
-
-fail:
-	log.Printf("[net] Deleting endpoint %v failed, err:%v.", endpointId, err)
-
-	return err
 }
 
 // GetEndpoint returns the endpoint with the given ID.

network/endpoint_linux.go

@@ -52,6 +52,13 @@ func (nw *network) newEndpointImpl(epInfo *EndpointInfo) (*endpoint, error) {
 		return nil, err
 	}
 
+	// On failure, delete the veth pair.
+	defer func() {
+		if err != nil {
+			netlink.DeleteLink(contIfName)
+		}
+	}()
+
 	//
 	// Host network interface setup.
 	//
@@ -60,14 +67,14 @@ func (nw *network) newEndpointImpl(epInfo *EndpointInfo) (*endpoint, error) {
 	log.Printf("[net] Setting link %v state up.", hostIfName)
 	err = netlink.SetLinkState(hostIfName, true)
 	if err != nil {
-		goto cleanup
+		return nil, err
 	}
 
 	// Connect host interface to the bridge.
 	log.Printf("[net] Setting link %v master %v.", hostIfName, nw.extIf.BridgeName)
 	err = netlink.SetLinkMaster(hostIfName, nw.extIf.BridgeName)
 	if err != nil {
-		goto cleanup
+		return nil, err
 	}
 
 	//
@@ -77,7 +84,7 @@ func (nw *network) newEndpointImpl(epInfo *EndpointInfo) (*endpoint, error) {
 	// Query container network interface info.
 	containerIf, err = net.InterfaceByName(contIfName)
 	if err != nil {
-		goto cleanup
+		return nil, err
 	}
 
 	// Setup rules for IP addresses on the container interface.
@@ -86,14 +93,14 @@ func (nw *network) newEndpointImpl(epInfo *EndpointInfo) (*endpoint, error) {
 		log.Printf("[net] Adding ARP reply rule for IP address %v on %v.", ipAddr.String(), contIfName)
 		err = ebtables.SetArpReply(ipAddr.IP, nw.getArpReplyAddress(containerIf.HardwareAddr), ebtables.Append)
 		if err != nil {
-			goto cleanup
+			return nil, err
 		}
 
 		// Add MAC address translation rule.
 		log.Printf("[net] Adding MAC DNAT rule for IP address %v on %v.", ipAddr.String(), contIfName)
 		err = ebtables.SetDnatForIPAddress(nw.extIf.Name, ipAddr.IP, containerIf.HardwareAddr, ebtables.Append)
 		if err != nil {
-			goto cleanup
+			return nil, err
 		}
 	}
 
@@ -103,23 +110,32 @@ func (nw *network) newEndpointImpl(epInfo *EndpointInfo) (*endpoint, error) {
 		log.Printf("[net] Opening netns %v.", epInfo.NetNsPath)
 		ns, err = OpenNamespace(epInfo.NetNsPath)
 		if err != nil {
-			goto cleanup
+			return nil, err
 		}
 		defer ns.Close()
 
 		// Move the container interface to container's network namespace.
 		log.Printf("[net] Setting link %v netns %v.", contIfName, epInfo.NetNsPath)
 		err = netlink.SetLinkNetNs(contIfName, ns.GetFd())
 		if err != nil {
-			goto cleanup
+			return nil, err
 		}
 
 		// Enter the container network namespace.
 		log.Printf("[net] Entering netns %v.", epInfo.NetNsPath)
 		err = ns.Enter()
 		if err != nil {
-			goto cleanup
+			return nil, err
 		}
+
+		// Return to host network namespace.
+		defer func() {
+			log.Printf("[net] Exiting netns %v.", epInfo.NetNsPath)
+			err = ns.Exit()
+			if err != nil {
+				log.Printf("[net] Failed to exit netns, err:%v.", err)
+			}
+		}()
 	}
 
 	// If a name for the container interface is specified...
@@ -128,22 +144,22 @@ func (nw *network) newEndpointImpl(epInfo *EndpointInfo) (*endpoint, error) {
 		log.Printf("[net] Setting link %v state down.", contIfName)
 		err = netlink.SetLinkState(contIfName, false)
 		if err != nil {
-			goto cleanup
+			return nil, err
 		}
 
 		// Rename the container interface.
 		log.Printf("[net] Setting link %v name %v.", contIfName, epInfo.IfName)
 		err = netlink.SetLinkName(contIfName, epInfo.IfName)
 		if err != nil {
-			goto cleanup
+			return nil, err
 		}
 		contIfName = epInfo.IfName
 
 		// Bring the interface back up.
 		log.Printf("[net] Setting link %v state up.", contIfName)
 		err = netlink.SetLinkState(contIfName, true)
 		if err != nil {
-			goto cleanup
+			return nil, err
 		}
 	}
 
@@ -152,7 +168,7 @@ func (nw *network) newEndpointImpl(epInfo *EndpointInfo) (*endpoint, error) {
 		log.Printf("[net] Adding IP address %v to link %v.", ipAddr.String(), contIfName)
 		err = netlink.AddIpAddress(contIfName, ipAddr.IP, &ipAddr)
 		if err != nil {
-			goto cleanup
+			return nil, err
 		}
 	}
 
@@ -169,17 +185,7 @@ func (nw *network) newEndpointImpl(epInfo *EndpointInfo) (*endpoint, error) {
 
 		err = netlink.AddIpRoute(nlRoute)
 		if err != nil {
-			goto cleanup
-		}
-	}
-
-	// If inside the container network namespace...
-	if ns != nil {
-		// Return to host network namespace.
-		log.Printf("[net] Exiting netns %v.", epInfo.NetNsPath)
-		err = ns.Exit()
-		if err != nil {
-			goto cleanup
+			return nil, err
 		}
 	}
 
@@ -194,12 +200,6 @@ func (nw *network) newEndpointImpl(epInfo *EndpointInfo) (*endpoint, error) {
 	}
 
 	return ep, nil
-
-cleanup:
-	// Roll back the changes for the endpoint.
-	netlink.DeleteLink(contIfName)
-
-	return nil, err
 }
 
 // deleteEndpointImpl deletes an existing endpoint from the network.

network/network.go

@@ -121,6 +121,11 @@ func (nm *networkManager) newNetwork(nwInfo *NetworkInfo) (*network, error) {
 	var err error
 
 	log.Printf("[net] Creating network %+v.", nwInfo)
+	defer func() {
+		if err != nil {
+			log.Printf("[net] Failed to create network %v, err:%v.", nwInfo.Id, err)
+		}
+	}()
 
 	// Set defaults.
 	if nwInfo.Mode == "" {
@@ -131,19 +136,19 @@ func (nm *networkManager) newNetwork(nwInfo *NetworkInfo) (*network, error) {
 	extIf := nm.findExternalInterfaceBySubnet(nwInfo.Subnets[0].Prefix.String())
 	if extIf == nil {
 		err = errSubnetNotFound
-		goto fail
+		return nil, err
 	}
 
 	// Make sure this network does not already exist.
 	if extIf.Networks[nwInfo.Id] != nil {
 		err = errNetworkExists
-		goto fail
+		return nil, err
 	}
 
 	// Call the OS-specific implementation.
 	nw, err = nm.newNetworkImpl(nwInfo, extIf)
 	if err != nil {
-		goto fail
+		return nil, err
 	}
 
 	// Add the network object.
@@ -152,37 +157,36 @@ func (nm *networkManager) newNetwork(nwInfo *NetworkInfo) (*network, error) {
 
 	log.Printf("[net] Created network %v on interface %v.", nwInfo.Id, extIf.Name)
 	return nw, nil
-
-fail:
-	log.Printf("[net] Failed to create network %v, err:%v.", nwInfo.Id, err)
-	return nil, err
 }
 
 // DeleteNetwork deletes an existing container network.
 func (nm *networkManager) deleteNetwork(networkId string) error {
+	var err error
+
 	log.Printf("[net] Deleting network %v.", networkId)
+	defer func() {
+		if err != nil {
+			log.Printf("[net] Failed to delete network %v, err:%v.", networkId, err)
+		}
+	}()
 
 	// Find the network.
 	nw, err := nm.getNetwork(networkId)
 	if err != nil {
-		goto fail
+		return err
 	}
 
 	// Call the OS-specific implementation.
 	err = nm.deleteNetworkImpl(nw)
 	if err != nil {
-		goto fail
+		return err
 	}
 
 	// Remove the network object.
 	delete(nw.extIf.Networks, networkId)
 
 	log.Printf("[net] Deleted network %+v.", nw)
 	return nil
-
-fail:
-	log.Printf("[net] Failed to delete network %v, err:%v.", networkId, err)
-	return err
 }
 
 // GetNetwork returns the network with the given ID.