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.

Sign up for GitHub

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: Revert "refactor: code changes for stateless cni and swift v2 (#2688)" on release/v1.5 #2847

Merged
merged 1 commit into from
Jul 12, 2024
Merged

fix: Revert "refactor: code changes for stateless cni and swift v2 (#2688)" on release/v1.5 #2847

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
20 changes: 8 additions & 12 deletions cni/network/invoker.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"net"

"github.com/Azure/azure-container-networking/cni"
"github.com/Azure/azure-container-networking/cns"
"github.com/Azure/azure-container-networking/network"
cniSkel "github.com/containernetworking/cni/pkg/skel"
)
Expand All @@ -26,16 +27,11 @@ type IPAMAddConfig struct {
}

type IPAMAddResult struct {
interfaceInfo map[string]network.InterfaceInfo
// ncResponse and host subnet prefix were moved into interface info
ipv6Enabled bool
}

func (ipamAddResult IPAMAddResult) PrettyString() string {
pStr := "InterfaceInfo: "
for key := range ipamAddResult.interfaceInfo {
val := ipamAddResult.interfaceInfo[key]
pStr += val.PrettyString()
}
return pStr
// Splitting defaultInterfaceInfo from secondaryInterfacesInfo so we don't need to loop for default CNI result every time
defaultInterfaceInfo network.InterfaceInfo
secondaryInterfacesInfo []network.InterfaceInfo
// ncResponse is used for Swift 1.0 multitenancy
ncResponse *cns.GetNetworkContainerResponse
hostSubnetPrefix net.IPNet
ipv6Enabled bool
}
33 changes: 9 additions & 24 deletions cni/network/invoker_azure.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ const (

type AzureIPAMInvoker struct {
plugin delegatePlugin
nwInfo *network.EndpointInfo
nwInfo *network.NetworkInfo
}

type delegatePlugin interface {
Expand All @@ -39,15 +39,15 @@ type delegatePlugin interface {
}

// Create an IPAM instance every time a CNI action is called.
func NewAzureIpamInvoker(plugin *NetPlugin, nwInfo *network.EndpointInfo) *AzureIPAMInvoker {
func NewAzureIpamInvoker(plugin *NetPlugin, nwInfo *network.NetworkInfo) *AzureIPAMInvoker {
return &AzureIPAMInvoker{
plugin: plugin,
nwInfo: nwInfo,
}
}

func (invoker *AzureIPAMInvoker) Add(addConfig IPAMAddConfig) (IPAMAddResult, error) {
addResult := IPAMAddResult{interfaceInfo: make(map[string]network.InterfaceInfo)}
addResult := IPAMAddResult{}

if addConfig.nwCfg == nil {
return addResult, invoker.plugin.Errorf("nil nwCfg passed to CNI ADD, stack: %+v", string(debug.Stack()))
Expand All @@ -69,11 +69,14 @@ func (invoker *AzureIPAMInvoker) Add(addConfig IPAMAddConfig) (IPAMAddResult, er
err = invoker.plugin.Errorf("Failed to allocate pool: %v", err)
return addResult, err
}
if len(result.IPs) > 0 {
addResult.hostSubnetPrefix = result.IPs[0].Address
}

defer func() {
if err != nil {
if len(addResult.interfaceInfo) > 0 && len(addResult.interfaceInfo[invoker.getInterfaceInfoKey(cns.InfraNIC)].IPConfigs) > 0 {
if er := invoker.Delete(&addResult.interfaceInfo[invoker.getInterfaceInfoKey(cns.InfraNIC)].IPConfigs[0].Address, addConfig.nwCfg, nil, addConfig.options); er != nil {
if len(addResult.defaultInterfaceInfo.IPConfigs) > 0 {
if er := invoker.Delete(&addResult.defaultInterfaceInfo.IPConfigs[0].Address, addConfig.nwCfg, nil, addConfig.options); er != nil {
err = invoker.plugin.Errorf("Failed to clean up IP's during Delete with error %v, after Add failed with error %w", er, err)
}
} else {
Expand Down Expand Up @@ -113,21 +116,7 @@ func (invoker *AzureIPAMInvoker) Add(addConfig IPAMAddConfig) (IPAMAddResult, er
routes[i] = network.RouteInfo{Dst: route.Dst, Gw: route.GW}
}

// TODO: changed how host subnet prefix populated (check)
hostSubnetPrefix := net.IPNet{}
if len(result.IPs) > 0 {
hostSubnetPrefix = result.IPs[0].Address
}
addResult.interfaceInfo[invoker.getInterfaceInfoKey(cns.InfraNIC)] = network.InterfaceInfo{
IPConfigs: ipconfigs,
Routes: routes,
DNS: network.DNSInfo{
Suffix: result.DNS.Domain,
Servers: result.DNS.Nameservers,
},
NICType: cns.InfraNIC,
HostSubnetPrefix: hostSubnetPrefix,
}
addResult.defaultInterfaceInfo = network.InterfaceInfo{IPConfigs: ipconfigs, Routes: routes, DNS: network.DNSInfo{Suffix: result.DNS.Domain, Servers: result.DNS.Nameservers}, NICType: cns.InfraNIC}

return addResult, err
}
Expand Down Expand Up @@ -208,7 +197,3 @@ func (invoker *AzureIPAMInvoker) Delete(address *net.IPNet, nwCfg *cni.NetworkCo

return nil
}

func (invoker *AzureIPAMInvoker) getInterfaceInfoKey(nicType cns.NICType) string {
return string(nicType)
}
40 changes: 10 additions & 30 deletions cni/network/invoker_azure_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (

"github.com/Azure/azure-container-networking/cni"
"github.com/Azure/azure-container-networking/cni/log"
"github.com/Azure/azure-container-networking/cns"
"github.com/Azure/azure-container-networking/ipam"
"github.com/Azure/azure-container-networking/network"
cniSkel "github.com/containernetworking/cni/pkg/skel"
Expand Down Expand Up @@ -82,9 +81,6 @@ func (m *mockDelegatePlugin) Errorf(format string, args ...interface{}) *cniType
}
}

// net.ParseCIDR will first get the ip, which contains byte data for the ip and mask,
// and the ipnet, which has a field for the *masked* ip and a field for the mask
// this function then replaces the masked ip with the "ip" field retrieved earlier and returns the ipnet
func getCIDRNotationForAddress(ipaddresswithcidr string) *net.IPNet {
ip, ipnet, err := net.ParseCIDR(ipaddresswithcidr)
if err != nil {
Expand All @@ -94,15 +90,6 @@ func getCIDRNotationForAddress(ipaddresswithcidr string) *net.IPNet {
return ipnet
}

// returns an ipnet, which contains the *masked* ip (zeroed out based on CIDR) and the mask itself
func parseCIDR(ipaddresswithcidr string) *net.IPNet {
_, ipnet, err := net.ParseCIDR(ipaddresswithcidr)
if err != nil {
panic(fmt.Sprintf("failed to parse cidr with err: %v", err))
}
return ipnet
}

func getSingleResult(ip string) []*cniTypesCurr.Result {
return []*cniTypesCurr.Result{
{
Expand All @@ -124,26 +111,26 @@ func getResult(ips ...string) []*network.IPConfig {
return res
}

func getNwInfo(subnetv4, subnetv6 string) *network.EndpointInfo {
nwInfo := &network.EndpointInfo{}
func getNwInfo(subnetv4, subnetv6 string) *network.NetworkInfo {
nwinfo := &network.NetworkInfo{}
if subnetv4 != "" {
nwInfo.Subnets = append(nwInfo.Subnets, network.SubnetInfo{
nwinfo.Subnets = append(nwinfo.Subnets, network.SubnetInfo{
Prefix: *getCIDRNotationForAddress(subnetv4),
})
}
if subnetv6 != "" {
nwInfo.Subnets = append(nwInfo.Subnets, network.SubnetInfo{
nwinfo.Subnets = append(nwinfo.Subnets, network.SubnetInfo{
Prefix: *getCIDRNotationForAddress(subnetv6),
})
}
return nwInfo
return nwinfo
}

func TestAzureIPAMInvoker_Add(t *testing.T) {
require := require.New(t)
type fields struct {
plugin delegatePlugin
nwInfo *network.EndpointInfo
nwInfo *network.NetworkInfo
}
type args struct {
nwCfg *cni.NetworkConfig
Expand Down Expand Up @@ -251,15 +238,8 @@ func TestAzureIPAMInvoker_Add(t *testing.T) {
require.Nil(err)
}

for key, ifInfo := range ipamAddResult.interfaceInfo {
if ifInfo.NICType == cns.InfraNIC {
fmt.Printf("want:%+v\nrest:%+v\n", tt.want, ifInfo.IPConfigs)
require.Exactly(tt.want, ifInfo.IPConfigs)
}
// azure ipam invoker always sets key as infra nic
require.Equal(string(cns.InfraNIC), key)
require.Equal(cns.InfraNIC, ifInfo.NICType)
}
fmt.Printf("want:%+v\nrest:%+v\n", tt.want, ipamAddResult.defaultInterfaceInfo.IPConfigs)
require.Exactly(tt.want, ipamAddResult.defaultInterfaceInfo.IPConfigs)
})
}
}
Expand All @@ -268,7 +248,7 @@ func TestAzureIPAMInvoker_Delete(t *testing.T) {
require := require.New(t)
type fields struct {
plugin delegatePlugin
nwInfo *network.EndpointInfo
nwInfo *network.NetworkInfo
}
type args struct {
address *net.IPNet
Expand Down Expand Up @@ -403,7 +383,7 @@ func TestRemoveIpamState_Add(t *testing.T) {
requires := require.New(t)
type fields struct {
plugin delegatePlugin
nwInfo *network.EndpointInfo
nwInfo *network.NetworkInfo
}
type args struct {
nwCfg *cni.NetworkConfig
Expand Down
79 changes: 28 additions & 51 deletions cni/network/invoker_cns.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,8 +141,9 @@ func (invoker *CNSIPAMInvoker) Add(addConfig IPAMAddConfig) (IPAMAddResult, erro
}
}

addResult := IPAMAddResult{interfaceInfo: make(map[string]network.InterfaceInfo)}
addResult := IPAMAddResult{}
numInterfacesWithDefaultRoutes := 0

for i := 0; i < len(response.PodIPInfo); i++ {
info := IPResultInfo{
podIPAddress: response.PodIPInfo[i].PodIPConfig.IPAddress,
Expand All @@ -163,42 +164,29 @@ func (invoker *CNSIPAMInvoker) Add(addConfig IPAMAddConfig) (IPAMAddResult, erro
zap.Any("podInfo", podInfo))

//nolint:exhaustive // ignore exhaustive types check
// Do we want to leverage this lint skip in other places of our code?
key := invoker.getInterfaceInfoKey(info.nicType, info.macAddress)
switch info.nicType {
case cns.DelegatedVMNIC:
// only handling single v4 PodIPInfo for Frontend NICs at the moment, will have to update once v6 gets added
if !info.skipDefaultRoutes {
numInterfacesWithDefaultRoutes++
}

// Add secondary interface info from podIPInfo to ipamAddResult
info.hostSubnet = response.PodIPInfo[i].HostPrimaryIPInfo.Subnet
info.hostPrimaryIP = response.PodIPInfo[i].HostPrimaryIPInfo.PrimaryIP
info.hostGateway = response.PodIPInfo[i].HostPrimaryIPInfo.Gateway

if err := configureSecondaryAddResult(&info, &addResult, &response.PodIPInfo[i].PodIPConfig, key); err != nil {
if err := configureSecondaryAddResult(&info, &addResult, &response.PodIPInfo[i].PodIPConfig); err != nil {
return IPAMAddResult{}, err
}
case cns.InfraNIC, "":
// if we change from legacy cns, the nicType will be empty, so we assume it is infra nic
info.nicType = cns.InfraNIC

default:
// only count dualstack interface once
_, exist := addResult.interfaceInfo[key]
if !exist {
addResult.interfaceInfo[key] = network.InterfaceInfo{}
if addResult.defaultInterfaceInfo.IPConfigs == nil {
addResult.defaultInterfaceInfo.IPConfigs = make([]*network.IPConfig, 0)
if !info.skipDefaultRoutes {
numInterfacesWithDefaultRoutes++
}
}

overlayMode := (invoker.ipamMode == util.V4Overlay) || (invoker.ipamMode == util.DualStackOverlay) || (invoker.ipamMode == util.Overlay)
if err := configureDefaultAddResult(&info, &addConfig, &addResult, overlayMode, key); err != nil {
if err := configureDefaultAddResult(&info, &addConfig, &addResult, overlayMode); err != nil {
return IPAMAddResult{}, err
}
default:
logger.Warn("Unknown NIC type received from cns pod ip info", zap.String("nicType", string(info.nicType)))
}
}

Expand Down Expand Up @@ -365,15 +353,15 @@ func getRoutes(cnsRoutes []cns.Route, skipDefaultRoutes bool) ([]network.RouteIn
return routes, nil
}

func configureDefaultAddResult(info *IPResultInfo, addConfig *IPAMAddConfig, addResult *IPAMAddResult, overlayMode bool, key string) error {
func configureDefaultAddResult(info *IPResultInfo, addConfig *IPAMAddConfig, addResult *IPAMAddResult, overlayMode bool) error {
// set the NC Primary IP in options
// SNATIPKey is not set for ipv6
if net.ParseIP(info.ncPrimaryIP).To4() != nil {
addConfig.options[network.SNATIPKey] = info.ncPrimaryIP
}

ip, ncIPNet, err := net.ParseCIDR(info.podIPAddress + "/" + fmt.Sprint(info.ncSubnetPrefix))
if ip == nil || err != nil {
if ip == nil {
return errors.Wrap(err, "Unable to parse IP from response: "+info.podIPAddress+" with err %w")
}

Expand All @@ -396,21 +384,15 @@ func configureDefaultAddResult(info *IPResultInfo, addConfig *IPAMAddConfig, add
}
}

// get the name of the primary IP address
_, hostIPNet, err := net.ParseCIDR(info.hostSubnet)
if err != nil {
return errors.Wrap(err, "unable to parse hostSubnet")
}

if ip := net.ParseIP(info.podIPAddress); ip != nil {
defaultInterfaceInfo := &addResult.defaultInterfaceInfo
defaultRouteDstPrefix := network.Ipv4DefaultRouteDstPrefix
if ip.To4() == nil {
defaultRouteDstPrefix = network.Ipv6DefaultRouteDstPrefix
addResult.ipv6Enabled = true
}

ipConfigs := addResult.interfaceInfo[key].IPConfigs
ipConfigs = append(ipConfigs,
defaultInterfaceInfo.IPConfigs = append(defaultInterfaceInfo.IPConfigs,
&network.IPConfig{
Address: net.IPNet{
IP: ip,
Expand All @@ -424,26 +406,27 @@ func configureDefaultAddResult(info *IPResultInfo, addConfig *IPAMAddConfig, add
return getRoutesErr
}

resRoute := addResult.interfaceInfo[key].Routes
if len(routes) > 0 {
resRoute = append(resRoute, routes...)
defaultInterfaceInfo.Routes = append(defaultInterfaceInfo.Routes, routes...)
} else { // add default routes if none are provided
resRoute = append(resRoute, network.RouteInfo{
defaultInterfaceInfo.Routes = append(defaultInterfaceInfo.Routes, network.RouteInfo{
Dst: defaultRouteDstPrefix,
Gw: ncgw,
})
}
// if we have multiple infra ip result infos, we effectively append routes and ip configs to that same interface info each time
// the host subnet prefix (in ipv4 or ipv6) will always refer to the same interface regardless of which ip result info we look at
addResult.interfaceInfo[key] = network.InterfaceInfo{
NICType: cns.InfraNIC,
SkipDefaultRoutes: info.skipDefaultRoutes,
IPConfigs: ipConfigs,
Routes: resRoute,
HostSubnetPrefix: *hostIPNet,
}

addResult.defaultInterfaceInfo.SkipDefaultRoutes = info.skipDefaultRoutes
}

// get the name of the primary IP address
_, hostIPNet, err := net.ParseCIDR(info.hostSubnet)
if err != nil {
return fmt.Errorf("unable to parse hostSubnet: %w", err)
}

addResult.hostSubnetPrefix = *hostIPNet
addResult.defaultInterfaceInfo.NICType = cns.InfraNIC

// set subnet prefix for host vm
// setHostOptions will execute if IPAM mode is not v4 overlay and not dualStackOverlay mode
// TODO: Remove v4overlay and dualstackoverlay options, after 'overlay' rolls out in AKS-RP
Expand All @@ -456,7 +439,7 @@ func configureDefaultAddResult(info *IPResultInfo, addConfig *IPAMAddConfig, add
return nil
}

func configureSecondaryAddResult(info *IPResultInfo, addResult *IPAMAddResult, podIPConfig *cns.IPSubnet, key string) error {
func configureSecondaryAddResult(info *IPResultInfo, addResult *IPAMAddResult, podIPConfig *cns.IPSubnet) error {
ip, ipnet, err := podIPConfig.GetIPNet()
if ip == nil {
return errors.Wrap(err, "Unable to parse IP from response: "+info.podIPAddress+" with err %w")
Expand All @@ -472,14 +455,13 @@ func configureSecondaryAddResult(info *IPResultInfo, addResult *IPAMAddResult, p
return err
}

addResult.interfaceInfo[key] = network.InterfaceInfo{
result := network.InterfaceInfo{
IPConfigs: []*network.IPConfig{
{
Address: net.IPNet{
IP: ip,
Mask: ipnet.Mask,
},
Gateway: net.ParseIP(info.ncGatewayIPAddress),
},
},
Routes: routes,
Expand All @@ -488,12 +470,7 @@ func configureSecondaryAddResult(info *IPResultInfo, addResult *IPAMAddResult, p
SkipDefaultRoutes: info.skipDefaultRoutes,
}

return nil
}
addResult.secondaryInterfacesInfo = append(addResult.secondaryInterfacesInfo, result)

func (invoker *CNSIPAMInvoker) getInterfaceInfoKey(nicType cns.NICType, macAddress string) string {
if nicType == cns.DelegatedVMNIC {
return macAddress
}
return string(nicType)
return nil
}
Loading
Loading