Removing addition of default routes before HNS network creation (#2059)

removing addition of default routes before hns network creation

Author: ramiro-gamarra

network/network_windows.go

@@ -7,7 +7,6 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
-	"net"
 	"strconv"
 	"strings"
 	"time"
@@ -189,76 +188,6 @@ func (nm *networkManager) newNetworkImplHnsV1(nwInfo *NetworkInfo, extIf *extern
 	return nw, nil
 }
 
-// add ipv4 and ipv6 routes in dualstack overlay mode to windows Node
-// in dualstack overlay mode, pods are created from different subnets on different nodes, gateway has to be node ip if pods want to communicate with each other
-// add routes to make node understand pod IPs come from different subnets and VFP will take decisions based on these routes to forward traffic and avoid Natting
-func (nm *networkManager) addNewNetRules(nwInfo *NetworkInfo) error {
-	var (
-		err error
-		out string
-	)
-
-	// get interface name of the VM adapter
-	ifName := nwInfo.MasterIfName
-	if !strings.Contains(nwInfo.MasterIfName, ifNamePrefix) {
-		ifName = fmt.Sprintf("%s (%s)", ifNamePrefix, nwInfo.MasterIfName)
-	}
-
-	// check if external interface name is empty
-	if ifName == "" {
-		return fmt.Errorf("[net] external interface name is empty") // nolint
-	}
-
-	// check whether nwInfo subnets exist
-	if nwInfo.Subnets == nil {
-		return fmt.Errorf("[net] nwInfo subnets are not found") // nolint
-	}
-
-	// iterate subnet and add ipv4 and ipv6 default route and gateway only if it is not existing
-	for _, subnet := range nwInfo.Subnets {
-		prefix := subnet.Prefix.String()
-
-		ip, _, errParseCIDR := net.ParseCIDR(prefix)
-		if errParseCIDR != nil {
-			return fmt.Errorf("[net] failed to parse prefix %s due to %+v", prefix, errParseCIDR) // nolint
-		}
-
-		if subnet.Gateway == nil {
-			return fmt.Errorf("[net] failed to get subnet gateway") // nolint
-		}
-
-		log.Printf("[net] Adding ipv4 and ipv6 net rules to windows node")
-
-		// delete existing net rules before adding new rules to windows node in case:
-		// if hnsNetwork is not existing and new pod is creating, existing rules will be applied twice that will cause the pod creation failure
-		if ip.To4() != nil {
-			deleteNetshV4DefaultRoute := fmt.Sprintf(netRouteCmd, "ipv4", "delete", prefix, ifName, ipv4DefaultHop)
-			if _, delErr := nm.plClient.ExecuteCommand(deleteNetshV4DefaultRoute); delErr != nil {
-				log.Printf("[net] Deleting ipv4 default route failed: %v", err)
-			}
-
-			// netsh interface ipv4 add route $subnetV4 $hostInterfaceAlias "0.0.0.0"
-			addNetshV4DefaultRoute := fmt.Sprintf(netRouteCmd, "ipv4", "add", prefix, ifName, ipv4DefaultHop)
-			if out, err = nm.plClient.ExecuteCommand(addNetshV4DefaultRoute); err != nil {
-				log.Printf("[net] Adding ipv4 default route failed: %v:%v", out, err)
-			}
-		} else {
-			deleteNetshV6DefaultRoute := fmt.Sprintf(netRouteCmd, "ipv6", "delete", prefix, ifName, ipv6DefaultHop)
-			if _, delErr := nm.plClient.ExecuteCommand(deleteNetshV6DefaultRoute); delErr != nil {
-				log.Printf("[net] Deleting ipv6 default route failed: %v", delErr)
-			}
-
-			// netsh interface ipv6 add route $subnetV6 $hostInterfaceAlias "::"
-			addNetshV6DefaultRoute := fmt.Sprintf(netRouteCmd, "ipv6", "add", prefix, ifName, ipv6DefaultHop)
-			if out, err = nm.plClient.ExecuteCommand(addNetshV6DefaultRoute); err != nil {
-				log.Printf("[net] Adding ipv6 default route failed: %v:%v", out, err)
-			}
-		}
-	}
-
-	return err // nolint
-}
-
 func (nm *networkManager) appIPV6RouteEntry(nwInfo *NetworkInfo) error {
 	var (
 		err error
@@ -403,16 +332,8 @@ func (nm *networkManager) newNetworkImplHnsV2(nwInfo *NetworkInfo, extIf *extern
 	if err != nil {
 		// if network not found, create the HNS network.
 		if errors.As(err, &hcn.NetworkNotFoundError{}) {
-			// add net routes to windows node if we have IPv6 enabled
-			if nwInfo.IsIPv6Enabled {
-				if err := nm.addNewNetRules(nwInfo); err != nil { // nolint
-					log.Printf("[net] Failed to add net rules due to %+v", err)
-					return nil, err
-				}
-			}
 			log.Printf("[net] Creating hcn network: %+v", hcnNetwork)
 			hnsResponse, err = Hnsv2.CreateNetwork(hcnNetwork)
-
 			if err != nil {
 				return nil, fmt.Errorf("Failed to create hcn network: %s due to error: %v", hcnNetwork.Name, err)
 			}

network/network_windows_test.go

@@ -8,14 +8,11 @@ package network
 
 import (
 	"fmt"
-	"net"
 	"testing"
 	"time"
 
 	"github.com/Azure/azure-container-networking/network/hnswrapper"
-	"github.com/Azure/azure-container-networking/platform"
 	"github.com/Microsoft/hcsshim/hcn"
-	"github.com/stretchr/testify/assert"
 )
 
 func TestNewAndDeleteNetworkImplHnsV2(t *testing.T) {
@@ -230,76 +227,3 @@ func TestDeleteNetworkImplHnsV1WithTimeout(t *testing.T) {
 		t.Fatal("Failed to timeout HNS calls for deleting network")
 	}
 }
-
-// test addNewNetRules to add net rules from NetworkInfo
-func TestAddNewNetRules(t *testing.T) {
-	cnt := 0
-	plc := platform.NewMockExecClient(false)
-	nm := &networkManager{
-		ExternalInterfaces: map[string]*externalInterface{},
-		plClient:           plc,
-	}
-
-	nwInfo := &NetworkInfo{
-		Id:           "d3e97a83-ba4c-45d5-ba88-dc56757ece28",
-		MasterIfName: "eth0",
-		Mode:         "bridge",
-		Subnets: []SubnetInfo{
-			{
-				Prefix: net.IPNet{
-					IP:   net.IPv4(10, 0, 0, 1),
-					Mask: net.IPv4Mask(255, 255, 0, 0),
-				},
-				Gateway: net.ParseIP("0.0.0.0"),
-			},
-			{
-				Prefix: net.IPNet{
-					IP:   net.ParseIP("ff02::fb"),
-					Mask: net.CIDRMask(128, 128),
-				},
-				Gateway: net.ParseIP("::"),
-			},
-		},
-	}
-
-	// get each delete and add new rule entry
-	ifName := "vEthernet (eth0)"
-	var ipType, defaultHop string
-	expectedCmds := make([]string, 0)
-	expectedNumRules := 8
-	for _, subnet := range nwInfo.Subnets {
-		prefix := subnet.Prefix.String()
-		ip, _, _ := net.ParseCIDR(prefix)
-		if ip.To4() != nil {
-			ipType = "ipv4"
-			defaultHop = ipv4DefaultHop
-		} else {
-			ipType = "ipv6"
-			defaultHop = ipv6DefaultHop
-		}
-		gateway := subnet.Gateway.String()
-		netRouteCmd1 := fmt.Sprintf(netRouteCmd, ipType, "delete", prefix, ifName, defaultHop)
-		expectedCmds = append(expectedCmds, netRouteCmd1)
-		netRouteCmd2 := fmt.Sprintf(netRouteCmd, ipType, "add", prefix, ifName, defaultHop)
-		expectedCmds = append(expectedCmds, netRouteCmd2)
-		netRouteCmd3 := fmt.Sprintf(netRouteCmd, ipType, "delete", prefix, ifName, gateway)
-		expectedCmds = append(expectedCmds, netRouteCmd3)
-		netRouteCmd4 := fmt.Sprintf(netRouteCmd, ipType, "add", prefix, ifName, gateway)
-		expectedCmds = append(expectedCmds, netRouteCmd4)
-	}
-
-	plc.SetExecCommand(func(cmd string) (string, error) {
-		assert.Equal(t, expectedCmds[cnt], cmd)
-		cnt++
-		return "", nil
-	})
-
-	err := nm.addNewNetRules(nwInfo)
-	if err != nil {
-		t.Fatal("Failed to add/delete a new network rule")
-	}
-
-	if cnt != expectedNumRules {
-		t.Fatalf("Failed to add/delete expected number %d of new network rules", expectedNumRules)
-	}
-}