test: validate secondary endpoint client failure (#2345)

test: validate secondary endpoint client failure will prevent endpoint creation

Co-authored-by: Jaeryn <tsch@microsoft.com>

cni/network/invoker_cns.go

@@ -463,7 +463,7 @@ func configureSecondaryAddResult(info *IPResultInfo, addResult *IPAMAddResult, p
 				},
 			},
 		},
-		nicType:           cns.DelegatedVMNIC,
+		nicType:           info.nicType,
 		macAddress:        macAddress,
 		skipDefaultRoutes: info.skipDefaultRoutes,
 	}

cni/network/network.go

@@ -819,7 +819,7 @@ func (plugin *NetPlugin) createEndpointInternal(opt *createEndpointInternalOpt)
 				IPAddresses:       addresses,
 				Routes:            routes,
 				MacAddress:        secondaryCniResult.macAddress,
-				NICType:           cns.DelegatedVMNIC,
+				NICType:           secondaryCniResult.nicType,
 				SkipDefaultRoutes: secondaryCniResult.skipDefaultRoutes,
 			})
 	}

netio/mocknetio.go

@@ -5,6 +5,8 @@ import (
 	"errors"
 	"fmt"
 	"net"
+
+	"github.com/Azure/azure-container-networking/netlink"
 )
 
 type getInterfaceValidationFn func(name string) (*net.Interface, error)
@@ -19,7 +21,8 @@ type MockNetIO struct {
 // ErrMockNetIOFail - mock netio error
 var (
 	ErrMockNetIOFail = errors.New("netio fail")
-	hwAddr, _        = net.ParseMAC("ab:cd:ef:12:34:56")
+	HwAddr, _        = net.ParseMAC("ab:cd:ef:12:34:56")
+	BadHwAddr, _     = net.ParseMAC("56:34:12:ef:cd:ab")
 )
 
 func NewMockNetIO(fail bool, failAttempt int) *MockNetIO {
@@ -48,7 +51,7 @@ func (netshim *MockNetIO) GetNetworkInterfaceByName(name string) (*net.Interface
 		//nolint:gomnd // Dummy MTU
 		MTU:          1000,
 		Name:         name,
-		HardwareAddr: hwAddr,
+		HardwareAddr: HwAddr,
 		//nolint:gomnd // Dummy interface index
 		Index: 2,
 	}, nil
@@ -59,16 +62,27 @@ func (netshim *MockNetIO) GetNetworkInterfaceAddrs(iface *net.Interface) ([]net.
 }
 
 func (netshim *MockNetIO) GetNetworkInterfaceByMac(mac net.HardwareAddr) (*net.Interface, error) {
-	if !bytes.Equal(mac, hwAddr) {
-		return nil, fmt.Errorf("%w: %s", ErrMockNetIOFail, mac)
+	if bytes.Equal(mac, HwAddr) {
+		return &net.Interface{
+			//nolint:gomnd // Dummy MTU
+			MTU:          1000,
+			Name:         "eth1",
+			HardwareAddr: mac,
+			//nolint:gomnd // Dummy interface index
+			Index: 2,
+		}, nil
 	}
 
-	return &net.Interface{
-		//nolint:gomnd // Dummy MTU
-		MTU:          1000,
-		Name:         "eth1",
-		HardwareAddr: mac,
-		//nolint:gomnd // Dummy interface index
-		Index: 2,
-	}, nil
+	if bytes.Equal(mac, BadHwAddr) {
+		return &net.Interface{
+			//nolint:gomnd // Dummy MTU
+			MTU:          1000,
+			Name:         netlink.BadEth,
+			HardwareAddr: mac,
+			//nolint:gomnd // Dummy interface index
+			Index: 1,
+		}, nil
+	}
+
+	return nil, fmt.Errorf("%w: %s", ErrMockNetIOFail, mac)
 }

netlink/mocknetlink.go

@@ -6,6 +6,8 @@ import (
 	"net"
 )
 
+const BadEth = "badeth"
+
 // ErrorMockNetlink - netlink mock error
 var ErrorMockNetlink = errors.New("Mock Netlink Error")
 
@@ -60,7 +62,11 @@ func (f *MockNetlink) SetLinkName(string, string) error {
 	return f.error()
 }
 
-func (f *MockNetlink) SetLinkState(string, bool) error {
+func (f *MockNetlink) SetLinkState(name string, _ bool) error {
+	if name == BadEth {
+		return ErrorMockNetlink
+	}
+
 	return f.error()
 }
 

network/endpoint.go

@@ -157,7 +157,7 @@ func (nw *network) newEndpoint(
 }
 
 // DeleteEndpoint deletes an existing endpoint from the network.
-func (nw *network) deleteEndpoint(nl netlink.NetlinkInterface, plc platform.ExecClient, nsc NamespaceClientInterface, endpointID string) error {
+func (nw *network) deleteEndpoint(nl netlink.NetlinkInterface, plc platform.ExecClient, nioc netio.NetIOInterface, nsc NamespaceClientInterface, endpointID string) error {
 	var err error
 
 	logger.Info("Deleting endpoint from network", zap.String("endpointID", endpointID), zap.String("id", nw.Id))
@@ -176,7 +176,7 @@ func (nw *network) deleteEndpoint(nl netlink.NetlinkInterface, plc platform.Exec
 
 	// Call the platform implementation.
 	// Pass nil for epClient and will be initialized in deleteEndpointImpl
-	err = nw.deleteEndpointImpl(nl, plc, nil, nsc, ep)
+	err = nw.deleteEndpointImpl(nl, plc, nil, nioc, nsc, ep)
 	if err != nil {
 		return err
 	}

network/endpoint_linux.go

@@ -157,10 +157,10 @@ func (nw *network) newEndpointImpl(
 				epClient = NewLinuxBridgeEndpointClient(nw.extIf, hostIfName, contIfName, nw.Mode, nl, plc)
 			} else if epInfo.NICType == cns.DelegatedVMNIC {
 				logger.Info("Secondary client")
-				epClient = NewSecondaryEndpointClient(nl, plc, nsc, ep)
+				epClient = NewSecondaryEndpointClient(nl, netioCli, plc, nsc, ep)
 			} else {
 				logger.Info("Transparent client")
-				epClient = NewTransparentEndpointClient(nw.extIf, hostIfName, contIfName, nw.Mode, nl, plc)
+				epClient = NewTransparentEndpointClient(nw.extIf, hostIfName, contIfName, nw.Mode, nl, netioCli, plc)
 			}
 		}
 
@@ -255,7 +255,7 @@ func (nw *network) newEndpointImpl(
 }
 
 // deleteEndpointImpl deletes an existing endpoint from the network.
-func (nw *network) deleteEndpointImpl(nl netlink.NetlinkInterface, plc platform.ExecClient, epClient EndpointClient, nsc NamespaceClientInterface, ep *endpoint) error {
+func (nw *network) deleteEndpointImpl(nl netlink.NetlinkInterface, plc platform.ExecClient, epClient EndpointClient, nioc netio.NetIOInterface, nsc NamespaceClientInterface, ep *endpoint) error {
 	// Delete the veth pair by deleting one of the peer interfaces.
 	// Deleting the host interface is more convenient since it does not require
 	// entering the container netns and hence works both for CNI and CNM.
@@ -276,13 +276,13 @@ func (nw *network) deleteEndpointImpl(nl netlink.NetlinkInterface, plc platform.
 			epClient = NewLinuxBridgeEndpointClient(nw.extIf, ep.HostIfName, "", nw.Mode, nl, plc)
 		} else {
 			if len(ep.SecondaryInterfaces) > 0 {
-				epClient = NewSecondaryEndpointClient(nl, plc, nsc, ep)
+				epClient = NewSecondaryEndpointClient(nl, nioc, plc, nsc, ep)
 				epClient.DeleteEndpointRules(ep)
 				//nolint:errcheck // ignore error
 				epClient.DeleteEndpoints(ep)
 			}
 
-			epClient = NewTransparentEndpointClient(nw.extIf, ep.HostIfName, "", nw.Mode, nl, plc)
+			epClient = NewTransparentEndpointClient(nw.extIf, ep.HostIfName, "", nw.Mode, nl, nioc, plc)
 		}
 	}
 

network/endpoint_test.go

@@ -227,11 +227,11 @@ var _ = Describe("Test Endpoint", func() {
 				Expect(len(mockCli.endpoints)).To(Equal(1))
 				// Deleting the endpoint
 				//nolint:errcheck // ignore error
-				nw.deleteEndpointImpl(netlink.NewMockNetlink(false, ""), platform.NewMockExecClient(false), mockCli, NewMockNamespaceClient(), ep2)
+				nw.deleteEndpointImpl(netlink.NewMockNetlink(false, ""), platform.NewMockExecClient(false), mockCli, netio.NewMockNetIO(false, 0), NewMockNamespaceClient(), ep2)
 				Expect(len(mockCli.endpoints)).To(Equal(0))
 				// Deleting same endpoint with same id should not fail
 				//nolint:errcheck // ignore error
-				nw.deleteEndpointImpl(netlink.NewMockNetlink(false, ""), platform.NewMockExecClient(false), mockCli, NewMockNamespaceClient(), ep2)
+				nw.deleteEndpointImpl(netlink.NewMockNetlink(false, ""), platform.NewMockExecClient(false), mockCli, netio.NewMockNetIO(false, 0), NewMockNamespaceClient(), ep2)
 				Expect(len(mockCli.endpoints)).To(Equal(0))
 			})
 		})
@@ -262,6 +262,46 @@ var _ = Describe("Test Endpoint", func() {
 				Expect(ep.Id).To(Equal(epInfo.Id))
 			})
 		})
+		Context("When secondary endpoint client is used", func() {
+			_, ipnet, _ := net.ParseCIDR("0.0.0.0/0")
+			nw := &network{
+				Endpoints: map[string]*endpoint{},
+				Mode:      opModeTransparent,
+				extIf:     &externalInterface{BridgeName: "testbridge"},
+			}
+			epInfo := &EndpointInfo{
+				Id:      "768e8deb-eth1",
+				IfName:  eth0IfName,
+				NICType: cns.InfraNIC,
+			}
+			secondaryEpInfo := &EndpointInfo{
+				NICType: cns.DelegatedVMNIC,
+				Routes:  []RouteInfo{{Dst: *ipnet}},
+			}
+
+			It("Should not endpoint to the network when there is an error", func() {
+				secondaryEpInfo.MacAddress = netio.BadHwAddr // mock netlink will fail to set link state on bad eth
+				ep, err := nw.newEndpointImpl(nil, netlink.NewMockNetlink(false, ""), platform.NewMockExecClient(false),
+					netio.NewMockNetIO(false, 0), nil, NewMockNamespaceClient(), []*EndpointInfo{epInfo, secondaryEpInfo})
+				Expect(err).To(HaveOccurred())
+				Expect(err.Error()).To(Equal("SecondaryEndpointClient Error: " + netlink.ErrorMockNetlink.Error()))
+				Expect(ep).To(BeNil())
+
+				secondaryEpInfo.MacAddress = netio.HwAddr
+				ep, err = nw.newEndpointImpl(nil, netlink.NewMockNetlink(false, ""), platform.NewMockExecClient(false),
+					netio.NewMockNetIO(false, 0), nil, NewMockNamespaceClient(), []*EndpointInfo{epInfo, secondaryEpInfo})
+				Expect(err).ToNot(HaveOccurred())
+				Expect(ep.Id).To(Equal(epInfo.Id))
+			})
+
+			It("Should add endpoint when there are no errors", func() {
+				secondaryEpInfo.MacAddress = netio.HwAddr
+				ep, err := nw.newEndpointImpl(nil, netlink.NewMockNetlink(false, ""), platform.NewMockExecClient(false),
+					netio.NewMockNetIO(false, 0), nil, NewMockNamespaceClient(), []*EndpointInfo{epInfo, secondaryEpInfo})
+				Expect(err).ToNot(HaveOccurred())
+				Expect(ep.Id).To(Equal(epInfo.Id))
+			})
+		})
 	})
 
 	Describe("Test updateEndpoint", func() {

network/endpoint_windows.go

@@ -409,7 +409,7 @@ func (nw *network) newEndpointImplHnsV2(cli apipaClient, epInfo *EndpointInfo) (
 }
 
 // deleteEndpointImpl deletes an existing endpoint from the network.
-func (nw *network) deleteEndpointImpl(_ netlink.NetlinkInterface, _ platform.ExecClient, _ EndpointClient, _ NamespaceClientInterface, ep *endpoint) error {
+func (nw *network) deleteEndpointImpl(_ netlink.NetlinkInterface, _ platform.ExecClient, _ EndpointClient, _ netio.NetIOInterface, _ NamespaceClientInterface, ep *endpoint) error {
 	if useHnsV2, err := UseHnsV2(ep.NetNs); useHnsV2 {
 		if err != nil {
 			return err

network/manager.go

@@ -369,7 +369,7 @@ func (nm *networkManager) DeleteEndpoint(networkID, endpointID string) error {
 		return err
 	}
 
-	err = nw.deleteEndpoint(nm.netlink, nm.plClient, nm.nsClient, endpointID)
+	err = nw.deleteEndpoint(nm.netlink, nm.plClient, nm.netio, nm.nsClient, endpointID)
 	if err != nil {
 		return err
 	}

network/secondary_endpoint_client_linux.go

@@ -30,13 +30,14 @@ type SecondaryEndpointClient struct {
 
 func NewSecondaryEndpointClient(
 	nl netlink.NetlinkInterface,
+	nioc netio.NetIOInterface,
 	plc platform.ExecClient,
 	nsc NamespaceClientInterface,
 	endpoint *endpoint,
 ) *SecondaryEndpointClient {
 	client := &SecondaryEndpointClient{
 		netlink:        nl,
-		netioshim:      &netio.NetIO{},
+		netioshim:      nioc,
 		plClient:       plc,
 		netUtilsClient: networkutils.NewNetworkUtils(nl, plc),
 		nsClient:       nsc,

network/transparent_endpointclient_linux.go

@@ -52,6 +52,7 @@ func NewTransparentEndpointClient(
 	containerVethName string,
 	mode string,
 	nl netlink.NetlinkInterface,
+	nioc netio.NetIOInterface,
 	plc platform.ExecClient,
 ) *TransparentEndpointClient {
 	client := &TransparentEndpointClient{
@@ -62,7 +63,7 @@ func NewTransparentEndpointClient(
 		hostPrimaryMac:    extIf.MacAddress,
 		mode:              mode,
 		netlink:           nl,
-		netioshim:         &netio.NetIO{},
+		netioshim:         nioc,
 		plClient:          plc,
 		netUtilsClient:    networkutils.NewNetworkUtils(nl, plc),
 	}