Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: return error on interface not found and always clean up snat ep #3226

Merged
merged 3 commits into from
Dec 4, 2024
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
2 changes: 1 addition & 1 deletion network/endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ type apipaClient interface {
}

func (epInfo *EndpointInfo) PrettyString() string {
return fmt.Sprintf("Id:%s ContainerID:%s NetNsPath:%s IfName:%s IfIndex:%d MacAddr:%s IPAddrs:%v Gateways:%v Data:%+v NICType: %s NetworkContainerID: %s HostIfName: %s NetNs: %s Options: %v",
return fmt.Sprintf("EndpointID:%s ContainerID:%s NetNsPath:%s IfName:%s IfIndex:%d MacAddr:%s IPAddrs:%v Gateways:%v Data:%+v NICType: %s NetworkContainerID: %s HostIfName: %s NetNs: %s Options: %v",
epInfo.EndpointID, epInfo.ContainerID, epInfo.NetNsPath, epInfo.IfName, epInfo.IfIndex, epInfo.MacAddress.String(), epInfo.IPAddresses,
epInfo.Gateways, epInfo.Data, epInfo.NICType, epInfo.NetworkContainerID, epInfo.HostIfName, epInfo.NetNs, epInfo.Options)
}
Expand Down
1 change: 0 additions & 1 deletion network/endpoint_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,6 @@ func (nw *network) deleteEndpointImpl(nl netlink.NetlinkInterface, plc platform.
epInfo := ep.getInfo()
if nw.Mode == opModeTransparentVlan {
epClient = NewTransparentVlanEndpointClient(nw, epInfo, ep.HostIfName, "", ep.VlanID, ep.LocalIP, nl, plc, nsc, iptc)

} else {
epClient = NewOVSEndpointClient(nw, epInfo, ep.HostIfName, "", ep.VlanID, ep.LocalIP, nl, ovsctl.NewOvsctl(), plc, iptc)
}
Expand Down
1 change: 1 addition & 0 deletions network/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -439,6 +439,7 @@ func (nm *networkManager) UpdateEndpointState(eps []*endpoint) error {
logger.Info("Update endpoint API returend ", zap.String("podname: ", response.ReturnCode.String()))
return nil
}

func validateUpdateEndpointState(endpointID string, ifNameToIPInfoMap map[string]*restserver.IPInfo) error {
if endpointID == "" {
return errors.New("endpoint id empty while validating update endpoint state")
Expand Down
1 change: 1 addition & 0 deletions network/ovs_endpoint_snatroute_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ func (client *OVSEndpointClient) NewSnatClient(snatBridgeIP, localIP string, epI
client.netlink,
client.plClient,
client.iptablesClient,
client.netioshim,
)
}
}
Expand Down
18 changes: 15 additions & 3 deletions network/snat/snat_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"github.com/Azure/azure-container-networking/cni/log"
"github.com/Azure/azure-container-networking/ebtables"
"github.com/Azure/azure-container-networking/iptables"
"github.com/Azure/azure-container-networking/netio"
"github.com/Azure/azure-container-networking/netlink"
"github.com/Azure/azure-container-networking/network/networkutils"
"github.com/Azure/azure-container-networking/platform"
Expand Down Expand Up @@ -55,6 +56,7 @@ type Client struct {
netlink netlink.NetlinkInterface
plClient platform.ExecClient
ipTablesClient ipTablesClient
netioClient netio.NetIOInterface
}

func NewSnatClient(hostIfName string,
Expand All @@ -67,6 +69,7 @@ func NewSnatClient(hostIfName string,
nl netlink.NetlinkInterface,
plClient platform.ExecClient,
iptc ipTablesClient,
nio netio.NetIOInterface,
) Client {
snatClient := Client{
hostSnatVethName: hostIfName,
Expand All @@ -78,6 +81,7 @@ func NewSnatClient(hostIfName string,
netlink: nl,
plClient: plClient,
ipTablesClient: iptc,
netioClient: nio,
}

snatClient.SkipAddressesFromBlock = append(snatClient.SkipAddressesFromBlock, skipAddressesFromBlock...)
Expand Down Expand Up @@ -223,7 +227,11 @@ func (client *Client) AllowInboundFromHostToNC() error {
return newErrorSnatClient(err.Error())
}

snatContainerVeth, _ := net.InterfaceByName(client.containerSnatVethName)
snatContainerVeth, err := client.netioClient.GetNetworkInterfaceByName(client.containerSnatVethName)
if err != nil {
logger.Info("Could not find interface", zap.String("containerSnatVethName", client.containerSnatVethName))
return errors.Wrap(newErrorSnatClient(err.Error()), "could not find container snat veth name for allow host to nc")
}

// Add static arp entry for localIP to prevent arp going out of VM
logger.Info("Adding static arp entry for ip", zap.Any("containerIP", containerIP),
Expand Down Expand Up @@ -319,7 +327,11 @@ func (client *Client) AllowInboundFromNCToHost() error {
return err
}

snatContainerVeth, _ := net.InterfaceByName(client.containerSnatVethName)
snatContainerVeth, err := client.netioClient.GetNetworkInterfaceByName(client.containerSnatVethName)
if err != nil {
logger.Info("Could not find interface", zap.String("containerSnatVethName", client.containerSnatVethName))
return errors.Wrap(newErrorSnatClient(err.Error()), "could not find container snat veth name for allow nc to host")
}

// Add static arp entry for localIP to prevent arp going out of VM
logger.Info("Adding static arp entry for ip", zap.Any("containerIP", containerIP), zap.String("HardwareAddr", snatContainerVeth.HardwareAddr.String()))
Expand Down Expand Up @@ -416,7 +428,7 @@ func (client *Client) DropArpForSnatBridgeApipaRange(snatBridgeIP, azSnatVethIfN

// This function creates linux bridge which will be used for outbound connectivity by NCs
func (client *Client) createSnatBridge(snatBridgeIP, hostPrimaryMac string) error {
_, err := net.InterfaceByName(SnatBridgeName)
_, err := client.netioClient.GetNetworkInterfaceByName(SnatBridgeName)
if err == nil {
logger.Info("Snat Bridge already exists")
} else {
Expand Down
57 changes: 43 additions & 14 deletions network/snat/snat_linux_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,30 @@ import (
"os"
"testing"

"github.com/Azure/azure-container-networking/iptables"
"github.com/Azure/azure-container-networking/netio"
"github.com/Azure/azure-container-networking/netlink"
)

var anyInterface = "dummy"

type mockIPTablesClient struct{}

func (c mockIPTablesClient) InsertIptableRule(_, _, _, _, _ string) error {
return nil
}

func (c mockIPTablesClient) AppendIptableRule(_, _, _, _, _ string) error {
return nil
}

func (c mockIPTablesClient) DeleteIptableRule(_, _, _, _, _ string) error {
return nil
}

func (c mockIPTablesClient) CreateChain(_, _, _ string) error {
return nil
}

func TestMain(m *testing.M) {
exitCode := m.Run()

Expand All @@ -18,16 +36,22 @@ func TestMain(m *testing.M) {
os.Exit(exitCode)
}

func TestAllowInboundFromHostToNC(t *testing.T) {
nl := netlink.NewNetlink()
iptc := iptables.NewClient()
client := &Client{
func GetTestClient(nl netlink.NetlinkInterface, iptc ipTablesClient, nio netio.NetIOInterface) *Client {
return &Client{
SnatBridgeIP: "169.254.0.1/16",
localIP: "169.254.0.4/16",
containerSnatVethName: anyInterface,
netlink: nl,
ipTablesClient: iptc,
netioClient: nio,
}
}

func TestAllowInboundFromHostToNC(t *testing.T) {
nl := netlink.NewMockNetlink(false, "")
iptc := &mockIPTablesClient{}
nio := netio.NewMockNetIO(false, 0)
client := GetTestClient(nl, iptc, nio)

if err := nl.AddLink(&netlink.DummyLink{
LinkInfo: netlink.LinkInfo{
Expand Down Expand Up @@ -65,18 +89,18 @@ func TestAllowInboundFromHostToNC(t *testing.T) {
if err := nl.DeleteLink(SnatBridgeName); err != nil {
t.Errorf("Error removing snat bridge: %v", err)
}

client.netioClient = netio.NewMockNetIO(true, 1)
if err := client.AllowInboundFromHostToNC(); err == nil {
t.Errorf("Expected error when interface not found in allow host to nc but got nil")
}
}

func TestAllowInboundFromNCToHost(t *testing.T) {
nl := netlink.NewNetlink()
iptc := iptables.NewClient()
client := &Client{
SnatBridgeIP: "169.254.0.1/16",
localIP: "169.254.0.4/16",
containerSnatVethName: anyInterface,
netlink: nl,
ipTablesClient: iptc,
}
nl := netlink.NewMockNetlink(false, "")
iptc := &mockIPTablesClient{}
nio := netio.NewMockNetIO(false, 0)
client := GetTestClient(nl, iptc, nio)

if err := nl.AddLink(&netlink.DummyLink{
LinkInfo: netlink.LinkInfo{
Expand Down Expand Up @@ -114,4 +138,9 @@ func TestAllowInboundFromNCToHost(t *testing.T) {
if err := nl.DeleteLink(SnatBridgeName); err != nil {
t.Errorf("Error removing snat bridge: %v", err)
}

client.netioClient = netio.NewMockNetIO(true, 1)
if err := client.AllowInboundFromNCToHost(); err == nil {
t.Errorf("Expected error when interface not found in allow nc to host but got nil")
}
}
1 change: 1 addition & 0 deletions network/transparent_vlan_endpoint_snatroute_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ func (client *TransparentVlanEndpointClient) NewSnatClient(snatBridgeIP, localIP
client.netlink,
client.plClient,
client.iptablesClient,
client.netioshim,
)
}
}
Expand Down
2 changes: 1 addition & 1 deletion network/transparent_vlan_endpointclient_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -635,7 +635,7 @@ func (client *TransparentVlanEndpointClient) DeleteEndpoints(ep *endpoint) error
return client.DeleteEndpointsImpl(ep, getNumRoutesLeft)
})
if err != nil {
return err
logger.Warn("could not delete transparent vlan endpoints", zap.String("errorMsg", err.Error()))
}

// VM NS
Expand Down
8 changes: 5 additions & 3 deletions network/transparent_vlan_endpointclient_linux_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,11 @@ import (
"github.com/stretchr/testify/require"
)

var errNetnsMock = errors.New("mock netns error")
var errMockNetIOFail = errors.New("netio fail")
var errMockNetIONoIfFail = &net.OpError{Op: "route", Net: "ip+net", Source: nil, Addr: nil, Err: errors.New("no such network interface")}
var (
errNetnsMock = errors.New("mock netns error")
errMockNetIOFail = errors.New("netio fail")
errMockNetIONoIfFail = &net.OpError{Op: "route", Net: "ip+net", Source: nil, Addr: nil, Err: errors.New("no such network interface")}
)

func newNetnsErrorMock(errStr string) error {
return errors.Wrap(errNetnsMock, errStr)
Expand Down
Loading