Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 12 additions & 12 deletions cni/network/network.go
Original file line number Diff line number Diff line change
Expand Up @@ -466,10 +466,11 @@ func (plugin *NetPlugin) Add(args *cniSkel.CmdArgs) error {

// iterate ipamAddResults and program the endpoint
for i := 0; i < len(ipamAddResults); i++ {
var networkID string
ipamAddResult = ipamAddResults[i]

options := make(map[string]any)
networkID, err := plugin.getNetworkName(args.Netns, &ipamAddResult, nwCfg)
networkID, err = plugin.getNetworkName(args.Netns, &ipamAddResult, nwCfg)

endpointID := GetEndpointID(args)
policies := cni.GetPoliciesFromNwCfg(nwCfg.AdditionalArgs)
Expand Down Expand Up @@ -557,7 +558,8 @@ func (plugin *NetPlugin) Add(args *cniSkel.CmdArgs) error {
natInfo: natInfo,
}

epInfo, err := plugin.createEndpointInternal(&createEndpointInternalOpt)
var epInfo network.EndpointInfo
epInfo, err = plugin.createEndpointInternal(&createEndpointInternalOpt)
if err != nil {
log.Errorf("Endpoint creation failed:%w", err)
return err
Expand Down Expand Up @@ -953,7 +955,7 @@ func (plugin *NetPlugin) Delete(args *cniSkel.CmdArgs) error {
numEndpointsToDelete := 1
// only get number of endpoints if it's multitenancy mode
if nwCfg.MultiTenancy {
numEndpointsToDelete = plugin.nm.GetNumEndpointsInNetNs(args.Netns)
numEndpointsToDelete = plugin.nm.GetNumEndpointsByContainerID(args.ContainerID)
}

log.Printf("[cni-net] number of endpoints to be deleted %d", numEndpointsToDelete)
Expand All @@ -973,23 +975,21 @@ func (plugin *NetPlugin) Delete(args *cniSkel.CmdArgs) error {
}
// Query the network.
if nwInfo, err = plugin.nm.GetNetworkInfo(networkID); err != nil {
if !nwCfg.MultiTenancy {
log.Printf("[cni-net] Failed to query network:%s: %v", networkID, err)
// Log the error but return success if the network is not found.
// if cni hits this, mostly state file would be missing and it can be reboot scenario where
// container runtime tries to delete and create pods which existed before reboot.
err = nil
return err
}
// Log the error but return success if the network is not found.
// if cni hits this, mostly state file would be missing and it can be reboot scenario where
// container runtime tries to delete and create pods which existed before reboot.
log.Printf("[cni-net] Failed to query network:%s: %v", networkID, err)
err = nil
return err
}

endpointID := GetEndpointID(args)
// Query the endpoint.
if epInfo, err = plugin.nm.GetEndpointInfo(networkID, endpointID); err != nil {
log.Printf("[cni-net] GetEndpoint for endpointID: %s returns: %v", endpointID, err)
if !nwCfg.MultiTenancy {
// attempt to release address associated with this Endpoint id
// This is to ensure clean up is done even in failure cases
log.Printf("[cni-net] Failed to query endpoint %s: %v", endpointID, err)
logAndSendEvent(plugin, fmt.Sprintf("Release ip by ContainerID (endpoint not found):%v", args.ContainerID))
if err = plugin.ipamInvoker.Delete(nil, nwCfg, args, nwInfo.Options); err != nil {
return plugin.RetriableError(fmt.Errorf("failed to release address(no endpoint): %w", err))
Expand Down
11 changes: 11 additions & 0 deletions netio/mocknetio.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,13 @@ import (
"net"
)

type getInterfaceValidationFn func(name string) (*net.Interface, error)

type MockNetIO struct {
fail bool
failAttempt int
numTimesCalled int
getInterfaceFn getInterfaceValidationFn
}

// ErrMockNetIOFail - mock netio error
Expand All @@ -22,13 +25,21 @@ func NewMockNetIO(fail bool, failAttempt int) *MockNetIO {
}
}

func (netshim *MockNetIO) SetGetInterfaceValidatonFn(fn getInterfaceValidationFn) {
netshim.getInterfaceFn = fn
}

func (netshim *MockNetIO) GetNetworkInterfaceByName(name string) (*net.Interface, error) {
netshim.numTimesCalled++

if netshim.fail && netshim.failAttempt == netshim.numTimesCalled {
return nil, fmt.Errorf("%w:%s", ErrMockNetIOFail, name)
}

if netshim.getInterfaceFn != nil {
return netshim.getInterfaceFn(name)
}

hwAddr, _ := net.ParseMAC("ab:cd:ef:12:34:56")

return &net.Interface{
Expand Down
26 changes: 22 additions & 4 deletions netlink/mocknetlink.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,13 @@ func newErrorMockNetlink(errStr string) error {
return fmt.Errorf("%w : %s", ErrorMockNetlink, errStr)
}

type routeValidateFn func(route *Route) error

type MockNetlink struct {
returnError bool
errorString string
returnError bool
errorString string
deleteRouteFn routeValidateFn
addRouteFn routeValidateFn
}

func NewMockNetlink(returnError bool, errorString string) *MockNetlink {
Expand All @@ -25,6 +29,14 @@ func NewMockNetlink(returnError bool, errorString string) *MockNetlink {
}
}

func (f *MockNetlink) SetDeleteRouteValidationFn(fn routeValidateFn) {
f.deleteRouteFn = fn
}

func (f *MockNetlink) SetAddRouteValidationFn(fn routeValidateFn) {
f.addRouteFn = fn
}

func (f *MockNetlink) error() error {
if f.returnError {
return newErrorMockNetlink(f.errorString)
Expand Down Expand Up @@ -88,10 +100,16 @@ func (f *MockNetlink) GetIPRoute(*Route) ([]*Route, error) {
return nil, f.error()
}

func (f *MockNetlink) AddIPRoute(*Route) error {
func (f *MockNetlink) AddIPRoute(r *Route) error {
if f.addRouteFn != nil {
return f.addRouteFn(r)
}
return f.error()
}

func (f *MockNetlink) DeleteIPRoute(*Route) error {
func (f *MockNetlink) DeleteIPRoute(r *Route) error {
if f.deleteRouteFn != nil {
return f.deleteRouteFn(r)
}
return f.error()
}
8 changes: 8 additions & 0 deletions netns/netns.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,3 +36,11 @@ func (f *Netns) NewNamed(name string) (int, error) {
func (f *Netns) DeleteNamed(name string) error {
return errors.Wrap(netns.DeleteNamed(name), "netns impl")
}

func (f *Netns) IsNamespaceEqual(fd1, fd2 int) bool {
return netns.NsHandle(fd1).Equal(netns.NsHandle(fd2))
}

func (f *Netns) NamespaceUniqueID(fd int) string {
return netns.NsHandle(fd).UniqueId()
}
2 changes: 1 addition & 1 deletion network/bridge_endpointclient_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ func (client *LinuxBridgeEndpointClient) ConfigureContainerInterfacesAndRoutes(e
return nil
}

func (client *LinuxBridgeEndpointClient) DeleteEndpoints(ep *endpoint, _ bool) error {
func (client *LinuxBridgeEndpointClient) DeleteEndpoints(ep *endpoint) error {
log.Printf("[net] Deleting veth pair %v %v.", ep.HostIfName, ep.IfName)
err := client.netlink.DeleteLink(ep.HostIfName)
if err != nil {
Expand Down
16 changes: 7 additions & 9 deletions network/endpoint_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ func (nw *network) newEndpointImpl(
}
// set deleteHostVeth to true to cleanup host veth interface if created
//nolint:errcheck // ignore error
epClient.DeleteEndpoints(endpt, true)
epClient.DeleteEndpoints(endpt)
}
}()

Expand Down Expand Up @@ -284,7 +284,7 @@ func (nw *network) deleteEndpointImpl(nl netlink.NetlinkInterface, plc platform.
// deleteHostVeth set to false not to delete veth as CRI will remove network namespace and
// veth will get removed as part of that.
//nolint:errcheck // ignore error
epClient.DeleteEndpoints(ep, false)
epClient.DeleteEndpoints(ep)

return nil
}
Expand All @@ -297,8 +297,6 @@ func addRoutes(nl netlink.NetlinkInterface, netioshim netio.NetIOInterface, inte
ifIndex := 0

for _, route := range routes {
log.Printf("[net] Adding IP route %+v to link %v.", route, interfaceName)

if route.DevName != "" {
devIf, _ := netioshim.GetNetworkInterfaceByName(route.DevName)
ifIndex = devIf.Index
Expand Down Expand Up @@ -327,6 +325,7 @@ func addRoutes(nl netlink.NetlinkInterface, netioshim netio.NetIOInterface, inte
Table: route.Table,
}

log.Printf("[net] Adding IP route %+v to link %v.", route, interfaceName)
if err := nl.AddIPRoute(nlRoute); err != nil {
if !strings.Contains(strings.ToLower(err.Error()), "file exists") {
return err
Expand All @@ -343,8 +342,6 @@ func deleteRoutes(nl netlink.NetlinkInterface, netioshim netio.NetIOInterface, i
ifIndex := 0

for _, route := range routes {
log.Printf("[net] Deleting IP route %+v from link %v.", route, interfaceName)

if route.DevName != "" {
devIf, _ := netioshim.GetNetworkInterfaceByName(route.DevName)
if devIf == nil {
Expand All @@ -353,15 +350,15 @@ func deleteRoutes(nl netlink.NetlinkInterface, netioshim netio.NetIOInterface, i
}

ifIndex = devIf.Index
} else {
} else if interfaceName != "" {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was this failing in a case where interfaceName is empty ?
Then should we add another else and log the error or something.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it would not fail but skip deleting the route if interface not provided but the requirement is to delete route even if interface name not provided

interfaceIf, _ := netioshim.GetNetworkInterfaceByName(interfaceName)
if interfaceIf == nil {
log.Printf("[net] Not deleting route. Interface %v doesn't exist", interfaceName)
continue
}

ifIndex = interfaceIf.Index
}

family := netlink.GetIPAddressFamily(route.Gw)
if route.Gw == nil {
family = netlink.GetIPAddressFamily(route.Dst.IP)
Expand All @@ -370,12 +367,13 @@ func deleteRoutes(nl netlink.NetlinkInterface, netioshim netio.NetIOInterface, i
nlRoute := &netlink.Route{
Family: family,
Dst: &route.Dst,
Gw: route.Gw,
LinkIndex: ifIndex,
Gw: route.Gw,
Protocol: route.Protocol,
Scope: route.Scope,
}

log.Printf("[net] Deleting IP route %+v from link %v.", route, interfaceName)
if err := nl.DeleteIPRoute(nlRoute); err != nil {
return err
}
Expand Down
137 changes: 137 additions & 0 deletions network/endpoint_linux_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,137 @@
package network

import (
"net"
"testing"

"github.com/Azure/azure-container-networking/netio"
"github.com/Azure/azure-container-networking/netlink"
. "github.com/onsi/ginkgo"
. "github.com/onsi/gomega"
"github.com/pkg/errors"
)

func TestEndpointLinux(t *testing.T) {
RegisterFailHandler(Fail)
RunSpecs(t, "Endpoint Suite")
}

var _ = Describe("Test TestEndpointLinux", func() {
Describe("Test deleteRoutes", func() {
_, dst, _ := net.ParseCIDR("192.168.0.0/16")

It("DeleteRoute with interfacename explicit", func() {
nlc := netlink.NewMockNetlink(false, "")
nlc.SetDeleteRouteValidationFn(func(r *netlink.Route) error {
Expect(r.LinkIndex).To(Equal(5))
return nil
})

netiocl := netio.NewMockNetIO(false, 0)
netiocl.SetGetInterfaceValidatonFn(func(ifName string) (*net.Interface, error) {
Expect(ifName).To(Equal("eth0"))
return &net.Interface{
Index: 5,
}, nil
})

err := deleteRoutes(nlc, netiocl, "eth0", []RouteInfo{{Dst: *dst, DevName: ""}})
Expect(err).To(BeNil())
})
It("DeleteRoute with interfacename set in Route", func() {
nlc := netlink.NewMockNetlink(false, "")
nlc.SetDeleteRouteValidationFn(func(r *netlink.Route) error {
Expect(r.LinkIndex).To(Equal(6))
return nil
})

netiocl := netio.NewMockNetIO(false, 0)
netiocl.SetGetInterfaceValidatonFn(func(ifName string) (*net.Interface, error) {
Expect(ifName).To(Equal("eth1"))
return &net.Interface{
Index: 6,
}, nil
})

err := deleteRoutes(nlc, netiocl, "", []RouteInfo{{Dst: *dst, DevName: "eth1"}})
Expect(err).To(BeNil())
})
It("DeleteRoute with no ifindex", func() {
nlc := netlink.NewMockNetlink(false, "")
nlc.SetDeleteRouteValidationFn(func(r *netlink.Route) error {
Expect(r.LinkIndex).To(Equal(0))
return nil
})

netiocl := netio.NewMockNetIO(false, 0)
netiocl.SetGetInterfaceValidatonFn(func(ifName string) (*net.Interface, error) {
Expect(ifName).To(Equal("eth1"))
return &net.Interface{
Index: 6,
}, nil
})

err := deleteRoutes(nlc, netiocl, "", []RouteInfo{{Dst: *dst, DevName: ""}})
Expect(err).To(BeNil())
})
})
Describe("Test addRoutes", func() {
_, dst, _ := net.ParseCIDR("192.168.0.0/16")
It("AddRoute with interfacename explicit", func() {
nlc := netlink.NewMockNetlink(false, "")
nlc.SetAddRouteValidationFn(func(r *netlink.Route) error {
Expect(r).NotTo(BeNil())
Expect(r.LinkIndex).To(Equal(5))
return nil
})

netiocl := netio.NewMockNetIO(false, 0)
netiocl.SetGetInterfaceValidatonFn(func(ifName string) (*net.Interface, error) {
Expect(ifName).To(Equal("eth0"))
return &net.Interface{
Index: 5,
}, nil
})

err := addRoutes(nlc, netiocl, "eth0", []RouteInfo{{Dst: *dst, DevName: ""}})
Expect(err).To(BeNil())
})
It("AddRoute with interfacename set in route", func() {
nlc := netlink.NewMockNetlink(false, "")
nlc.SetAddRouteValidationFn(func(r *netlink.Route) error {
Expect(r.LinkIndex).To(Equal(6))
return nil
})

netiocl := netio.NewMockNetIO(false, 0)
netiocl.SetGetInterfaceValidatonFn(func(ifName string) (*net.Interface, error) {
Expect(ifName).To(Equal("eth1"))
return &net.Interface{
Index: 6,
}, nil
})

err := addRoutes(nlc, netiocl, "", []RouteInfo{{Dst: *dst, DevName: "eth1"}})
Expect(err).To(BeNil())
})
It("AddRoute with interfacename not set should return error", func() {
nlc := netlink.NewMockNetlink(false, "")
nlc.SetAddRouteValidationFn(func(r *netlink.Route) error {
Expect(r.LinkIndex).To(Equal(0))
//nolint:goerr113 // for testing
return errors.New("Cannot add route")
})

netiocl := netio.NewMockNetIO(false, 0)
netiocl.SetGetInterfaceValidatonFn(func(ifName string) (*net.Interface, error) {
Expect(ifName).To(Equal(""))
return &net.Interface{
Index: 0,
}, errors.Wrapf(netio.ErrInterfaceNil, "Cannot get interface")
})

err := addRoutes(nlc, netiocl, "", []RouteInfo{{Dst: *dst, DevName: ""}})
Expect(err).ToNot(BeNil())
})
})
})
1 change: 1 addition & 0 deletions network/endpoint_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,7 @@ func (nw *network) newEndpointImplHnsV1(epInfo *EndpointInfo) (*endpoint, error)
VlanID: vlanid,
EnableSnatOnHost: epInfo.EnableSnatOnHost,
NetNs: epInfo.NetNsPath,
ContainerID: epInfo.ContainerID,
}

for _, route := range epInfo.Routes {
Expand Down
Loading