fix: Fail network creation if handleCommonOptions return error (#1063)

* handle errors in setting up routes and iptables for AKS-Swift

* added netio interface in networkmanager

Author: tamilmani1989

cni/network/network.go

@@ -19,6 +19,7 @@ import (
 	"github.com/Azure/azure-container-networking/common"
 	"github.com/Azure/azure-container-networking/iptables"
 	"github.com/Azure/azure-container-networking/log"
+	"github.com/Azure/azure-container-networking/netio"
 	"github.com/Azure/azure-container-networking/netlink"
 	"github.com/Azure/azure-container-networking/network"
 	"github.com/Azure/azure-container-networking/network/policy"
@@ -114,7 +115,7 @@ func NewPlugin(name string,
 
 	nl := netlink.NewNetlink()
 	// Setup network manager.
-	nm, err := network.NewNetworkManager(nl, platform.NewExecClient())
+	nm, err := network.NewNetworkManager(nl, platform.NewExecClient(), &netio.NetIO{})
 	if err != nil {
 		return nil, err
 	}

cnm/network/network.go

@@ -12,6 +12,7 @@ import (
 	cnsclient "github.com/Azure/azure-container-networking/cns/client"
 	"github.com/Azure/azure-container-networking/common"
 	"github.com/Azure/azure-container-networking/log"
+	"github.com/Azure/azure-container-networking/netio"
 	"github.com/Azure/azure-container-networking/netlink"
 	"github.com/Azure/azure-container-networking/network"
 	"github.com/Azure/azure-container-networking/platform"
@@ -52,7 +53,7 @@ func NewPlugin(config *common.PluginConfig) (NetPlugin, error) {
 
 	nl := netlink.NewNetlink()
 	// Setup network manager.
-	nm, err := network.NewNetworkManager(nl, platform.NewExecClient())
+	nm, err := network.NewNetworkManager(nl, platform.NewExecClient(), &netio.NetIO{})
 	if err != nil {
 		return nil, err
 	}

cnms/service/networkmonitor.go

@@ -11,6 +11,7 @@ import (
 	cnms "github.com/Azure/azure-container-networking/cnms/cnmspackage"
 	acn "github.com/Azure/azure-container-networking/common"
 	"github.com/Azure/azure-container-networking/log"
+	"github.com/Azure/azure-container-networking/netio"
 	"github.com/Azure/azure-container-networking/netlink"
 	"github.com/Azure/azure-container-networking/network"
 	"github.com/Azure/azure-container-networking/platform"
@@ -148,7 +149,7 @@ func main() {
 		}
 
 		nl := netlink.NewNetlink()
-		nm, err := network.NewNetworkManager(nl, platform.NewExecClient())
+		nm, err := network.NewNetworkManager(nl, platform.NewExecClient(), &netio.NetIO{})
 		if err != nil {
 			log.Printf("[monitor] Failed while creating network manager")
 			return

network/manager.go

@@ -11,6 +11,7 @@ import (
 	cnms "github.com/Azure/azure-container-networking/cnms/cnmspackage"
 	"github.com/Azure/azure-container-networking/common"
 	"github.com/Azure/azure-container-networking/log"
+	"github.com/Azure/azure-container-networking/netio"
 	"github.com/Azure/azure-container-networking/netlink"
 	"github.com/Azure/azure-container-networking/platform"
 	"github.com/Azure/azure-container-networking/store"
@@ -58,6 +59,7 @@ type networkManager struct {
 	ExternalInterfaces map[string]*externalInterface
 	store              store.KeyValueStore
 	netlink            netlink.NetlinkInterface
+	netio              netio.NetIOInterface
 	plClient           platform.ExecClient
 	sync.Mutex
 }
@@ -86,11 +88,12 @@ type NetworkManager interface {
 }
 
 // Creates a new network manager.
-func NewNetworkManager(nl netlink.NetlinkInterface, plc platform.ExecClient) (NetworkManager, error) {
+func NewNetworkManager(nl netlink.NetlinkInterface, plc platform.ExecClient, netioCli netio.NetIOInterface) (NetworkManager, error) {
 	nm := &networkManager{
 		ExternalInterfaces: make(map[string]*externalInterface),
 		netlink:            nl,
 		plClient:           plc,
+		netio:              netioCli,
 	}
 
 	return nm, nil

network/network_linux.go

@@ -59,32 +59,29 @@ type route netlink.Route
 // NewNetworkImpl creates a new container network.
 func (nm *networkManager) newNetworkImpl(nwInfo *NetworkInfo, extIf *externalInterface) (*network, error) {
 	// Connect the external interface.
-	var vlanid int
+	var (
+		vlanid int
+		ifName string
+	)
 	opt, _ := nwInfo.Options[genericData].(map[string]interface{})
 	log.Printf("opt %+v options %+v", opt, nwInfo.Options)
 
 	switch nwInfo.Mode {
 	case opModeTunnel:
-		err := nm.handleCommonOptions(extIf.Name, nwInfo)
-		if err != nil {
-			log.Printf("tunnel handleCommonOptions failed with error %s", err.Error())
-		}
 		fallthrough
 	case opModeBridge:
 		log.Printf("create bridge")
+		ifName = extIf.BridgeName
 		if err := nm.connectExternalInterface(extIf, nwInfo); err != nil {
 			return nil, err
 		}
 
 		if opt != nil && opt[VlanIDKey] != nil {
 			vlanid, _ = strconv.Atoi(opt[VlanIDKey].(string))
 		}
-		err := nm.handleCommonOptions(extIf.BridgeName, nwInfo)
-		if err != nil {
-			log.Printf("bridge handleCommonOptions failed with error %s", err.Error())
-		}
 	case opModeTransparent:
 		log.Printf("Transparent mode")
+		ifName = extIf.Name
 		if nwInfo.IPV6Mode != "" {
 			nu := networkutils.NewNetworkUtils(nm.netlink, nm.plClient)
 			if err := nu.EnableIPV6Forwarding(); err != nil {
@@ -95,6 +92,12 @@ func (nm *networkManager) newNetworkImpl(nwInfo *NetworkInfo, extIf *externalInt
 		return nil, errNetworkModeInvalid
 	}
 
+	err := nm.handleCommonOptions(ifName, nwInfo)
+	if err != nil {
+		log.Printf("handleCommonOptions failed with error %s", err.Error())
+		return nil, err
+	}
+
 	// Create the network object.
 	nw := &network{
 		Id:               nwInfo.Id,
@@ -109,10 +112,10 @@ func (nm *networkManager) newNetworkImpl(nwInfo *NetworkInfo, extIf *externalInt
 	return nw, nil
 }
 
-func (nm *networkManager) handleCommonOptions(ifname string, nwInfo *NetworkInfo) error {
+func (nm *networkManager) handleCommonOptions(ifName string, nwInfo *NetworkInfo) error {
 	var err error
 	if routes, exists := nwInfo.Options[RoutesKey]; exists {
-		err = nm.addBridgeRoutes(ifname, routes.([]RouteInfo))
+		err = addRoutes(nm.netlink, nm.netio, ifName, routes.([]RouteInfo))
 		if err != nil {
 			return err
 		}
@@ -571,34 +574,6 @@ func (*networkManager) addToIptables(cmds []iptables.IPTableEntry) error {
 	return nil
 }
 
-func (nm *networkManager) addBridgeRoutes(bridgeName string, routes []RouteInfo) error {
-	log.Printf("Adding routes...")
-	for _, route := range routes {
-		route.DevName = bridgeName
-		devIf, _ := net.InterfaceByName(route.DevName)
-		ifIndex := devIf.Index
-		gwfamily := netlink.GetIPAddressFamily(route.Gw)
-
-		nlRoute := &netlink.Route{
-			Family:    gwfamily,
-			Dst:       &route.Dst,
-			Gw:        route.Gw,
-			LinkIndex: ifIndex,
-		}
-
-		if err := nm.netlink.AddIPRoute(nlRoute); err != nil {
-			if !strings.Contains(strings.ToLower(err.Error()), "file exists") {
-				return fmt.Errorf("Failed to add %+v to host interface with error: %v", nlRoute, err)
-			}
-			log.Printf("[cni-net] route already exists: dst %+v, gw %+v, interfaceName %v", nlRoute.Dst, nlRoute.Gw, route.DevName)
-		}
-
-		log.Printf("[cni-net] Added route %+v", route)
-	}
-
-	return nil
-}
-
 // Add ipv6 nat gateway IP on bridge
 func (nm *networkManager) addIpv6NatGateway(nwInfo *NetworkInfo) error {
 	log.Printf("[net] Adding ipv6 nat gateway on azure bridge")