diff --git a/cni/network/invoker_cns.go b/cni/network/invoker_cns.go index d8867efe85..62d84af287 100644 --- a/cni/network/invoker_cns.go +++ b/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, } diff --git a/cni/network/network.go b/cni/network/network.go index 4c0dd34e84..068c6cef87 100644 --- a/cni/network/network.go +++ b/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, }) } diff --git a/netio/mocknetio.go b/netio/mocknetio.go index b80a44bbb1..71a87ce085 100644 --- a/netio/mocknetio.go +++ b/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) } diff --git a/netlink/mocknetlink.go b/netlink/mocknetlink.go index 405038a263..9dfc5da1a1 100644 --- a/netlink/mocknetlink.go +++ b/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() } diff --git a/network/endpoint.go b/network/endpoint.go index ab873e92d0..8322de8edc 100644 --- a/network/endpoint.go +++ b/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 } diff --git a/network/endpoint_linux.go b/network/endpoint_linux.go index 3ceda20460..60cdf61744 100644 --- a/network/endpoint_linux.go +++ b/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) } } diff --git a/network/endpoint_test.go b/network/endpoint_test.go index 14927125df..669b16804a 100644 --- a/network/endpoint_test.go +++ b/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() { diff --git a/network/endpoint_windows.go b/network/endpoint_windows.go index 718dc613c1..4c17db94fb 100644 --- a/network/endpoint_windows.go +++ b/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 diff --git a/network/manager.go b/network/manager.go index 5204cc0c31..5e26de34e3 100644 --- a/network/manager.go +++ b/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 } diff --git a/network/secondary_endpoint_client_linux.go b/network/secondary_endpoint_client_linux.go index a5f256ea83..8cf7a0e5b7 100644 --- a/network/secondary_endpoint_client_linux.go +++ b/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, diff --git a/network/transparent_endpointclient_linux.go b/network/transparent_endpointclient_linux.go index 1159c1f19a..f39986c0e4 100644 --- a/network/transparent_endpointclient_linux.go +++ b/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), }