From bfb151c8123c8a28bd3964b08234040f63e4dafe Mon Sep 17 00:00:00 2001 From: Tamilmani Manoharan Date: Wed, 18 Oct 2017 14:33:48 -0700 Subject: [PATCH 1/3] Fix for leaking CA while deleting network. Even if deleting endpoint failed, CNI should invoke ipam release call. --- cni/network/network.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/cni/network/network.go b/cni/network/network.go index 2e51726c06..dc1314b50b 100644 --- a/cni/network/network.go +++ b/cni/network/network.go @@ -320,8 +320,8 @@ func (plugin *netPlugin) Delete(args *cniSkel.CmdArgs) error { // Delete the endpoint. err = plugin.nm.DeleteEndpoint(networkId, endpointId) if err != nil { - err = plugin.Errorf("Failed to delete endpoint: %v", err) - return err + // Log the error but don't return before releasing address + log.Printf("Container Namespace might have been removed. Failed to delete endpoint: %v", err) } // Call into IPAM plugin to release the endpoint's addresses. @@ -330,8 +330,7 @@ func (plugin *netPlugin) Delete(args *cniSkel.CmdArgs) error { nwCfg.Ipam.Address = address.IP.String() err = plugin.DelegateDel(nwCfg.Ipam.Type, nwCfg) if err != nil { - err = plugin.Errorf("Failed to release address: %v", err) - return err + log.Printf("Failed to release address: %v", err) } } From 3c5ae5da8c3362b152db46d81dd66e8a049164d3 Mon Sep 17 00:00:00 2001 From: Tamilmani Manoharan Date: Wed, 18 Oct 2017 14:59:21 -0700 Subject: [PATCH 2/3] Handled address not found during release address as false positive --- cni/network/network.go | 3 ++- ipam/pool.go | 8 ++++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/cni/network/network.go b/cni/network/network.go index dc1314b50b..458dbd17ab 100644 --- a/cni/network/network.go +++ b/cni/network/network.go @@ -330,7 +330,8 @@ func (plugin *netPlugin) Delete(args *cniSkel.CmdArgs) error { nwCfg.Ipam.Address = address.IP.String() err = plugin.DelegateDel(nwCfg.Ipam.Type, nwCfg) if err != nil { - log.Printf("Failed to release address: %v", err) + plugin.Errorf("Failed to release address: %v", err) + return err } } diff --git a/ipam/pool.go b/ipam/pool.go index 657142ce23..f61d3ee440 100644 --- a/ipam/pool.go +++ b/ipam/pool.go @@ -542,13 +542,13 @@ func (ap *addressPool) releaseAddress(address string, options map[string]string) // Fail if an address record with a matching ID is not found. if ar == nil || (id != "" && id != ar.ID) { - err = errAddressNotFound - return err + log.Printf("Address not found. Not Returning error") + return nil } if !ar.InUse { - err = errAddressNotInUse - return err + log.Printf("Address not in use. Not Returning error") + return nil } if ar.ID != "" { From 5ba70f9092facbfe12e8e745666b6e4903e39b75 Mon Sep 17 00:00:00 2001 From: Tamilmani Manoharan Date: Wed, 18 Oct 2017 17:15:53 -0700 Subject: [PATCH 3/3] Made changes in netlink code to ignore interface not found error --- cni/network/network.go | 6 +++--- netlink/link.go | 11 +++++++---- network/endpoint.go | 3 ++- 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/cni/network/network.go b/cni/network/network.go index 458dbd17ab..2e51726c06 100644 --- a/cni/network/network.go +++ b/cni/network/network.go @@ -320,8 +320,8 @@ func (plugin *netPlugin) Delete(args *cniSkel.CmdArgs) error { // Delete the endpoint. err = plugin.nm.DeleteEndpoint(networkId, endpointId) if err != nil { - // Log the error but don't return before releasing address - log.Printf("Container Namespace might have been removed. Failed to delete endpoint: %v", err) + err = plugin.Errorf("Failed to delete endpoint: %v", err) + return err } // Call into IPAM plugin to release the endpoint's addresses. @@ -330,7 +330,7 @@ func (plugin *netPlugin) Delete(args *cniSkel.CmdArgs) error { nwCfg.Ipam.Address = address.IP.String() err = plugin.DelegateDel(nwCfg.Ipam.Type, nwCfg) if err != nil { - plugin.Errorf("Failed to release address: %v", err) + err = plugin.Errorf("Failed to release address: %v", err) return err } } diff --git a/netlink/link.go b/netlink/link.go index 3021727bc8..7bf3ec893e 100644 --- a/netlink/link.go +++ b/netlink/link.go @@ -7,6 +7,7 @@ import ( "fmt" "net" + "github.com/Azure/azure-container-networking/log" "golang.org/x/sys/unix" ) @@ -146,15 +147,17 @@ func AddLink(link Link) error { // DeleteLink deletes a network interface. func DeleteLink(name string) error { if name == "" { - return fmt.Errorf("Invalid link name") + log.Printf("[net] Invalid link name. Not returning error") + return nil } - s, err := getSocket() + iface, err := net.InterfaceByName(name) if err != nil { - return err + log.Printf("[net] Interface not found. Not returning error") + return nil } - iface, err := net.InterfaceByName(name) + s, err := getSocket() if err != nil { return err } diff --git a/network/endpoint.go b/network/endpoint.go index 07ac9eb7ee..7b2e743860 100644 --- a/network/endpoint.go +++ b/network/endpoint.go @@ -83,7 +83,8 @@ func (nw *network) deleteEndpoint(endpointId string) error { // Look up the endpoint. ep, err := nw.getEndpoint(endpointId) if err != nil { - return err + log.Printf("[net] Endpoint %v not found. Not Returning error", endpointId) + return nil } // Call the platform implementation.