diff --git a/azure-ipam/ipam.go b/azure-ipam/ipam.go index e71782ebd8..fdb09c4669 100644 --- a/azure-ipam/ipam.go +++ b/azure-ipam/ipam.go @@ -115,7 +115,7 @@ func (p *IPAMPlugin) CmdAdd(args *cniSkel.CmdArgs) error { p.logger.Debug("Received CNS IP config response", zap.Any("response", resp)) // Get Pod IP and gateway IP from ip config response - podIPNet, err := ipconfig.ProcessIPConfigsResp(resp) + podIPNet, gatewayIP, err := ipconfig.ProcessIPConfigsResp(resp) if err != nil { p.logger.Error("Failed to interpret CNS IPConfigResponse", zap.Error(err), zap.Any("response", resp)) return cniTypes.NewError(ErrProcessIPConfigResponse, err.Error(), "failed to interpret CNS IPConfigResponse") @@ -136,9 +136,33 @@ func (p *IPAMPlugin) CmdAdd(args *cniSkel.CmdArgs) error { Mask: net.CIDRMask(ipNet.Bits(), 128), // nolint } } + ipConfig.Gateway = (*gatewayIP)[i] cniResult.IPs[i] = ipConfig } + cniResult.Interfaces = []*types100.Interface{} + seenInterfaces := map[string]bool{} + + for _, podIPInfo := range resp.PodIPInfo { + // Skip if interface already seen + // This is to avoid duplicate interfaces in the result + // Deduplication is necessary because there is one podIPInfo entry for each IP family(IPv4 and IPv6), and both may point to the same interface + if podIPInfo.MacAddress == "" || seenInterfaces[podIPInfo.MacAddress] { + continue + } + + infMac, err := net.ParseMAC(podIPInfo.MacAddress) + if err != nil { + p.logger.Error("Failed to parse interface MAC address", zap.Error(err), zap.String("macAddress", podIPInfo.MacAddress)) + return cniTypes.NewError(cniTypes.ErrUnsupportedField, err.Error(), "failed to parse interface MAC address") + } + + cniResult.Interfaces = append(cniResult.Interfaces, &types100.Interface{ + Mac: infMac.String(), + }) + seenInterfaces[podIPInfo.MacAddress] = true + } + // Get versioned result versionedCniResult, err := cniResult.GetAsVersion(nwCfg.CNIVersion) if err != nil { diff --git a/azure-ipam/ipam_test.go b/azure-ipam/ipam_test.go index ecae57606c..ba6e426ef7 100644 --- a/azure-ipam/ipam_test.go +++ b/azure-ipam/ipam_test.go @@ -58,6 +58,33 @@ func (c *MockCNSClient) RequestIPAddress(ctx context.Context, ipconfig cns.IPCon }, } return result, nil + case "nilGateway": + result := &cns.IPConfigResponse{ + PodIpInfo: cns.PodIpInfo{ + PodIPConfig: cns.IPSubnet{ + IPAddress: "10.0.1.10", + PrefixLength: 24, + }, + NetworkContainerPrimaryIPConfig: cns.IPConfiguration{ + IPSubnet: cns.IPSubnet{ + IPAddress: "10.0.1.0", + PrefixLength: 24, + }, + DNSServers: nil, + GatewayIPAddress: "", // nil/empty gateway + }, + HostPrimaryIPInfo: cns.HostIPInfo{ + Gateway: "", + PrimaryIP: "10.0.0.1", + Subnet: "10.0.0.0/24", + }, + }, + Response: cns.Response{ + ReturnCode: 0, + Message: "", + }, + } + return result, nil default: result := &cns.IPConfigResponse{ PodIpInfo: cns.PodIpInfo{ @@ -65,6 +92,7 @@ func (c *MockCNSClient) RequestIPAddress(ctx context.Context, ipconfig cns.IPCon IPAddress: "10.0.1.10", PrefixLength: 24, }, + MacAddress: "00:11:22:33:44:55", NetworkContainerPrimaryIPConfig: cns.IPConfiguration{ IPSubnet: cns.IPSubnet{ IPAddress: "10.0.1.0", @@ -92,11 +120,61 @@ func (c *MockCNSClient) RequestIPs(ctx context.Context, ipconfig cns.IPConfigsRe switch ipconfig.InfraContainerID { case "failRequestCNSArgs": return nil, errFoo - case "happyArgsSingle", "failProcessCNSRespSingleIP", "failRequestCNSArgsSingleIP": + case "happyArgsSingle", "failProcessCNSRespSingleIP", "failRequestCNSArgsSingleIP", "nilGateway": e := &client.CNSClientError{} e.Code = types.UnsupportedAPI e.Err = errUnsupportedAPI return nil, e + case "happyArgsDual": + result := &cns.IPConfigsResponse{ + PodIPInfo: []cns.PodIpInfo{ + { + PodIPConfig: cns.IPSubnet{ + IPAddress: "10.0.1.10", + PrefixLength: 24, + }, + MacAddress: "00:11:22:33:44:55", + NetworkContainerPrimaryIPConfig: cns.IPConfiguration{ + IPSubnet: cns.IPSubnet{ + IPAddress: "10.0.1.0", + PrefixLength: 24, + }, + DNSServers: nil, + GatewayIPAddress: "10.0.0.1", + }, + HostPrimaryIPInfo: cns.HostIPInfo{ + Gateway: "10.0.0.1", + PrimaryIP: "10.0.0.1", + Subnet: "10.0.0.0/24", + }, + }, + { + PodIPConfig: cns.IPSubnet{ + IPAddress: "fd11:1234::1", + PrefixLength: 120, + }, + MacAddress: "00:11:22:33:44:55", // Same MAC for dual-stack scenario + NetworkContainerPrimaryIPConfig: cns.IPConfiguration{ + IPSubnet: cns.IPSubnet{ + IPAddress: "fd11:1234::", + PrefixLength: 120, + }, + DNSServers: nil, + GatewayIPv6Address: "fe80::1234:5678:9abc", + }, + HostPrimaryIPInfo: cns.HostIPInfo{ + Gateway: "fe80::1234:5678:9abc", + PrimaryIP: "fe80::1234:5678:9abc", + Subnet: "fd11:1234::/120", + }, + }, + }, + Response: cns.Response{ + ReturnCode: 0, + Message: "", + }, + } + return result, nil case "failProcessCNSResp": result := &cns.IPConfigsResponse{ PodIPInfo: []cns.PodIpInfo{ @@ -129,8 +207,8 @@ func (c *MockCNSClient) RequestIPs(ctx context.Context, ipconfig cns.IPConfigsRe IPAddress: "fd11:1234::", PrefixLength: 112, }, - DNSServers: nil, - GatewayIPAddress: "fe80::1234:5678:9abc", + DNSServers: nil, + GatewayIPv6Address: "fe80::1234:5678:9abc", }, HostPrimaryIPInfo: cns.HostIPInfo{ Gateway: "fe80::1234:5678:9abc", @@ -153,6 +231,7 @@ func (c *MockCNSClient) RequestIPs(ctx context.Context, ipconfig cns.IPConfigsRe IPAddress: "10.0.1.10", PrefixLength: 24, }, + MacAddress: "00:11:22:33:44:55", NetworkContainerPrimaryIPConfig: cns.IPConfiguration{ IPSubnet: cns.IPSubnet{ IPAddress: "10.0.1.0", @@ -172,13 +251,14 @@ func (c *MockCNSClient) RequestIPs(ctx context.Context, ipconfig cns.IPConfigsRe IPAddress: "fd11:1234::1", PrefixLength: 120, }, + MacAddress: "00:11:22:33:44:55", // Same MAC for dual-stack scenario NetworkContainerPrimaryIPConfig: cns.IPConfiguration{ IPSubnet: cns.IPSubnet{ IPAddress: "fd11:1234::", PrefixLength: 120, }, - DNSServers: nil, - GatewayIPAddress: "fe80::1234:5678:9abc", + DNSServers: nil, + GatewayIPv6Address: "fe80::1234:5678:9abc", }, HostPrimaryIPInfo: cns.HostIPInfo{ Gateway: "fe80::1234:5678:9abc", @@ -281,12 +361,18 @@ func TestCmdAdd(t *testing.T) { args: buildArgs("happyArgsSingle", happyPodArgs, happyNetConfByteArr), want: &types100.Result{ CNIVersion: "1.0.0", + Interfaces: []*types100.Interface{ + { + Mac: "00:11:22:33:44:55", + }, + }, IPs: []*types100.IPConfig{ { Address: net.IPNet{ IP: net.IPv4(10, 0, 1, 10), Mask: net.CIDRMask(24, 32), }, + Gateway: net.IPv4(10, 0, 0, 1), }, }, DNS: cniTypes.DNS{}, @@ -298,24 +384,36 @@ func TestCmdAdd(t *testing.T) { args: buildArgs("happyArgsDual", happyPodArgs, happyNetConfByteArr), want: &types100.Result{ CNIVersion: "1.0.0", + Interfaces: []*types100.Interface{ + { + Mac: "00:11:22:33:44:55", // Single interface for dual-stack + }, + }, IPs: []*types100.IPConfig{ { Address: net.IPNet{ IP: net.IPv4(10, 0, 1, 10), Mask: net.CIDRMask(24, 32), }, + Gateway: net.IPv4(10, 0, 0, 1), }, { Address: net.IPNet{ IP: net.ParseIP("fd11:1234::1"), Mask: net.CIDRMask(120, 128), }, + Gateway: net.ParseIP("fe80::1234:5678:9abc"), }, }, DNS: cniTypes.DNS{}, }, wantErr: false, }, + { + name: "CNI add with nil gateway IP", + args: buildArgs("nilGateway", happyPodArgs, happyNetConfByteArr), + wantErr: true, + }, { name: "Fail request CNS ipconfig during CmdAdd", args: buildArgs("failRequestCNSArgs", happyPodArgs, happyNetConfByteArr), diff --git a/azure-ipam/ipconfig/ipconfig.go b/azure-ipam/ipconfig/ipconfig.go index 9b2ecc97f4..f090409577 100644 --- a/azure-ipam/ipconfig/ipconfig.go +++ b/azure-ipam/ipconfig/ipconfig.go @@ -3,6 +3,7 @@ package ipconfig import ( "encoding/json" "fmt" + "net" "net/netip" "github.com/Azure/azure-container-networking/cns" @@ -63,10 +64,13 @@ func CreateIPConfigsReq(args *cniSkel.CmdArgs) (cns.IPConfigsRequest, error) { return req, nil } -func ProcessIPConfigsResp(resp *cns.IPConfigsResponse) (*[]netip.Prefix, error) { +func ProcessIPConfigsResp(resp *cns.IPConfigsResponse) (*[]netip.Prefix, *[]net.IP, error) { podIPNets := make([]netip.Prefix, len(resp.PodIPInfo)) + gatewaysIPs := make([]net.IP, len(resp.PodIPInfo)) for i := range resp.PodIPInfo { + var gatewayIP net.IP + podCIDR := fmt.Sprintf( "%s/%d", resp.PodIPInfo[i].PodIPConfig.IPAddress, @@ -74,12 +78,25 @@ func ProcessIPConfigsResp(resp *cns.IPConfigsResponse) (*[]netip.Prefix, error) ) podIPNet, err := netip.ParsePrefix(podCIDR) if err != nil { - return nil, errors.Wrapf(err, "cns returned invalid pod CIDR %q", podCIDR) + return nil, nil, errors.Wrapf(err, "cns returned invalid pod CIDR %q", podCIDR) } podIPNets[i] = podIPNet + + var gatewayStr string + if podIPNet.Addr().Is4() { + gatewayStr = resp.PodIPInfo[i].NetworkContainerPrimaryIPConfig.GatewayIPAddress + } else if podIPNet.Addr().Is6() { + gatewayStr = resp.PodIPInfo[i].NetworkContainerPrimaryIPConfig.GatewayIPv6Address + } + + gatewayIP = net.ParseIP(gatewayStr) + if gatewayIP == nil { + return nil, nil, errors.Errorf("failed to parse gateway IP %q for pod ip %s", gatewayStr, resp.PodIPInfo[i].PodIPConfig.IPAddress) + } + gatewaysIPs[i] = gatewayIP } - return &podIPNets, nil + return &podIPNets, &gatewaysIPs, nil } type k8sPodEnvArgs struct { diff --git a/cns/NetworkContainerContract.go b/cns/NetworkContainerContract.go index 406c45b554..8f5939c28e 100644 --- a/cns/NetworkContainerContract.go +++ b/cns/NetworkContainerContract.go @@ -398,9 +398,10 @@ type NetworkInterfaceInfo struct { // IPConfiguration contains details about ip config to provision in the VM. type IPConfiguration struct { - IPSubnet IPSubnet - DNSServers []string - GatewayIPAddress string + IPSubnet IPSubnet + DNSServers []string + GatewayIPAddress string + GatewayIPv6Address string } // SecondaryIPConfig contains IP info of SecondaryIP @@ -755,3 +756,11 @@ type NodeRegisterRequest struct { NumCores int NmAgentSupportedApis []string } + +// IPFamily - Enum for determining IPFamily when retrieving IPs from network containers +type IPFamily string + +const ( + IPv4 IPFamily = "ipv4" + IPv6 IPFamily = "ipv6" +) diff --git a/cns/kubecontroller/nodenetworkconfig/conversion.go b/cns/kubecontroller/nodenetworkconfig/conversion.go index 01bda6780e..59b6d3ad7c 100644 --- a/cns/kubecontroller/nodenetworkconfig/conversion.go +++ b/cns/kubecontroller/nodenetworkconfig/conversion.go @@ -7,6 +7,7 @@ import ( "strings" "github.com/Azure/azure-container-networking/cns" + "github.com/Azure/azure-container-networking/cns/configuration" "github.com/Azure/azure-container-networking/crd/nodenetworkconfig/api/v1alpha" "github.com/pkg/errors" ) @@ -73,7 +74,7 @@ func CreateNCRequestFromDynamicNC(nc v1alpha.NetworkContainer) (*cns.CreateNetwo // CreateNCRequestFromStaticNC generates a CreateNetworkContainerRequest from a static NetworkContainer. // //nolint:gocritic //ignore hugeparam -func CreateNCRequestFromStaticNC(nc v1alpha.NetworkContainer) (*cns.CreateNetworkContainerRequest, error) { +func CreateNCRequestFromStaticNC(nc v1alpha.NetworkContainer, config *configuration.CNSConfig) (*cns.CreateNetworkContainerRequest, error) { if nc.Type == v1alpha.Overlay { nc.Version = 0 // fix for NMA always giving us version 0 for Overlay NCs } @@ -97,7 +98,7 @@ func CreateNCRequestFromStaticNC(nc v1alpha.NetworkContainer) (*cns.CreateNetwor subnet.IPAddress = primaryPrefix.Addr().String() } - req, err := createNCRequestFromStaticNCHelper(nc, primaryPrefix, subnet) + req, err := createNCRequestFromStaticNCHelper(nc, primaryPrefix, subnet, config) if err != nil { return nil, errors.Wrapf(err, "error while creating NC request from static NC") } diff --git a/cns/kubecontroller/nodenetworkconfig/conversion_linux.go b/cns/kubecontroller/nodenetworkconfig/conversion_linux.go index c89d41646b..b43c1f96e2 100644 --- a/cns/kubecontroller/nodenetworkconfig/conversion_linux.go +++ b/cns/kubecontroller/nodenetworkconfig/conversion_linux.go @@ -5,6 +5,7 @@ import ( "strconv" "github.com/Azure/azure-container-networking/cns" + "github.com/Azure/azure-container-networking/cns/configuration" "github.com/Azure/azure-container-networking/crd/nodenetworkconfig/api/v1alpha" "github.com/pkg/errors" ) @@ -13,15 +14,18 @@ import ( // by adding all IPs in the the block to the secondary IP configs list. It does not skip any IPs. // //nolint:gocritic //ignore hugeparam -func createNCRequestFromStaticNCHelper(nc v1alpha.NetworkContainer, primaryIPPrefix netip.Prefix, subnet cns.IPSubnet) (*cns.CreateNetworkContainerRequest, error) { +func createNCRequestFromStaticNCHelper(nc v1alpha.NetworkContainer, primaryIPPrefix netip.Prefix, subnet cns.IPSubnet, config *configuration.CNSConfig) (*cns.CreateNetworkContainerRequest, error) { secondaryIPConfigs := map[string]cns.SecondaryIPConfig{} // iterate through all IP addresses in the subnet described by primaryPrefix and // add them to the request as secondary IPConfigs. - for addr := primaryIPPrefix.Masked().Addr(); primaryIPPrefix.Contains(addr); addr = addr.Next() { - secondaryIPConfigs[addr.String()] = cns.SecondaryIPConfig{ - IPAddress: addr.String(), - NCVersion: int(nc.Version), + // Process primary prefix IPs in all scenarios except when nc.Type is v1alpha.VNETBlock AND SwiftV2 is enabled + if !(config.EnableSwiftV2 && nc.Type == v1alpha.VNETBlock) { + for addr := primaryIPPrefix.Masked().Addr(); primaryIPPrefix.Contains(addr); addr = addr.Next() { + secondaryIPConfigs[addr.String()] = cns.SecondaryIPConfig{ + IPAddress: addr.String(), + NCVersion: int(nc.Version), + } } } @@ -52,9 +56,13 @@ func createNCRequestFromStaticNCHelper(nc v1alpha.NetworkContainer, primaryIPPre NetworkContainerType: cns.Docker, Version: strconv.FormatInt(nc.Version, 10), //nolint:gomnd // it's decimal IPConfiguration: cns.IPConfiguration{ - IPSubnet: subnet, - GatewayIPAddress: nc.DefaultGateway, + IPSubnet: subnet, + GatewayIPAddress: nc.DefaultGateway, + GatewayIPv6Address: nc.DefaultGatewayV6, }, NCStatus: nc.Status, + NetworkInterfaceInfo: cns.NetworkInterfaceInfo{ + MACAddress: nc.MacAddress, + }, }, nil } diff --git a/cns/kubecontroller/nodenetworkconfig/conversion_test.go b/cns/kubecontroller/nodenetworkconfig/conversion_test.go index eff8059a63..c7ee626a19 100644 --- a/cns/kubecontroller/nodenetworkconfig/conversion_test.go +++ b/cns/kubecontroller/nodenetworkconfig/conversion_test.go @@ -5,8 +5,10 @@ import ( "testing" "github.com/Azure/azure-container-networking/cns" + "github.com/Azure/azure-container-networking/cns/configuration" "github.com/Azure/azure-container-networking/crd/nodenetworkconfig/api/v1alpha" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) const ( @@ -339,7 +341,8 @@ func TestCreateNCRequestFromStaticNC(t *testing.T) { for _, tt := range tests { tt := tt t.Run(tt.name, func(t *testing.T) { - got, err := CreateNCRequestFromStaticNC(tt.input) + config := &configuration.CNSConfig{} + got, err := CreateNCRequestFromStaticNC(tt.input, config) if tt.wantErr { assert.Error(t, err) return @@ -349,3 +352,138 @@ func TestCreateNCRequestFromStaticNC(t *testing.T) { }) } } + +func TestCreateNCRequestFromStaticNCWithConfig(t *testing.T) { + tests := []struct { + name string + input v1alpha.NetworkContainer + config *configuration.CNSConfig + want *cns.CreateNetworkContainerRequest + wantErr bool + }{ + { + name: "SwiftV2 enabled with VNETBlock - should NOT process all IPs in prefix", + input: v1alpha.NetworkContainer{ + ID: ncID, + PrimaryIP: "10.0.0.0/32", + NodeIP: "10.0.0.1", + Type: v1alpha.VNETBlock, + SubnetAddressSpace: "10.0.0.0/24", + DefaultGateway: "10.0.0.1", + Version: 1, + Status: "Available", + }, + config: &configuration.CNSConfig{ + EnableSwiftV2: true, + }, + want: &cns.CreateNetworkContainerRequest{ + NetworkContainerid: ncID, + NetworkContainerType: cns.Docker, + Version: "1", + HostPrimaryIP: "10.0.0.1", + IPConfiguration: cns.IPConfiguration{ + IPSubnet: cns.IPSubnet{ + IPAddress: "10.0.0.1", + PrefixLength: 24, + }, + GatewayIPAddress: "10.0.0.1", + }, + SecondaryIPConfigs: map[string]cns.SecondaryIPConfig{ + // No IPs from primary prefix + }, + NCStatus: "Available", + }, + wantErr: false, + }, + { + name: "SwiftV2 disabled with VNETBlock - should process all IP in prefix", + input: v1alpha.NetworkContainer{ + ID: ncID, + PrimaryIP: "10.0.0.0/32", + NodeIP: "10.0.0.1", + Type: v1alpha.VNETBlock, + SubnetAddressSpace: "10.0.0.0/24", + DefaultGateway: "10.0.0.1", + Version: 1, + Status: "Available", + IPAssignments: []v1alpha.IPAssignment{ + { + Name: "test-ip", + IP: "10.0.0.10/32", + }, + }, + }, + config: &configuration.CNSConfig{}, + want: &cns.CreateNetworkContainerRequest{ + NetworkContainerid: ncID, + NetworkContainerType: cns.Docker, + Version: "1", + HostPrimaryIP: "10.0.0.1", + IPConfiguration: cns.IPConfiguration{ + IPSubnet: cns.IPSubnet{ + IPAddress: "10.0.0.1", + PrefixLength: 24, + }, + GatewayIPAddress: "10.0.0.1", + }, + SecondaryIPConfigs: map[string]cns.SecondaryIPConfig{ + "10.0.0.0": {IPAddress: "10.0.0.0", NCVersion: 1}, + // IP assignments + "10.0.0.10": {IPAddress: "10.0.0.10", NCVersion: 1}, + }, + NCStatus: "Available", + }, + wantErr: false, + }, + { + name: "SwiftV2 disabled with non-VNETBlock type - should process IP in prefix", + input: v1alpha.NetworkContainer{ + ID: ncID, + PrimaryIP: "10.0.0.0/32", + NodeIP: "10.0.0.1", + Type: v1alpha.Overlay, + SubnetAddressSpace: "10.0.0.0/24", + DefaultGateway: "10.0.0.1", + Version: 1, + Status: "Available", + IPAssignments: []v1alpha.IPAssignment{ + { + Name: "test-ip", + IP: "10.0.0.10/32", + }, + }, + }, + config: &configuration.CNSConfig{}, + want: &cns.CreateNetworkContainerRequest{ + NetworkContainerid: ncID, + NetworkContainerType: cns.Docker, + Version: "0", + HostPrimaryIP: "10.0.0.1", + IPConfiguration: cns.IPConfiguration{ + IPSubnet: cns.IPSubnet{ + IPAddress: "10.0.0.0", + PrefixLength: 24, + }, + GatewayIPAddress: "10.0.0.1", + }, + SecondaryIPConfigs: map[string]cns.SecondaryIPConfig{ + "10.0.0.0": {IPAddress: "10.0.0.0", NCVersion: 0}, + }, + NCStatus: "Available", + }, + wantErr: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := CreateNCRequestFromStaticNC(tt.input, tt.config) + if tt.wantErr { + assert.Error(t, err) + return + } + require.NoError(t, err) + assert.EqualValues(t, tt.want, got) + }) + } +} diff --git a/cns/kubecontroller/nodenetworkconfig/conversion_windows.go b/cns/kubecontroller/nodenetworkconfig/conversion_windows.go index 0a41ccd41c..236b997c4c 100644 --- a/cns/kubecontroller/nodenetworkconfig/conversion_windows.go +++ b/cns/kubecontroller/nodenetworkconfig/conversion_windows.go @@ -5,6 +5,7 @@ import ( "strconv" "github.com/Azure/azure-container-networking/cns" + "github.com/Azure/azure-container-networking/cns/configuration" "github.com/Azure/azure-container-networking/crd/nodenetworkconfig/api/v1alpha" "github.com/pkg/errors" ) @@ -14,7 +15,7 @@ import ( // secondary IPs. If the gateway is not empty, it will not reserve the 2nd IP and add it as a secondary IP. // //nolint:gocritic //ignore hugeparam -func createNCRequestFromStaticNCHelper(nc v1alpha.NetworkContainer, primaryIPPrefix netip.Prefix, subnet cns.IPSubnet) (*cns.CreateNetworkContainerRequest, error) { +func createNCRequestFromStaticNCHelper(nc v1alpha.NetworkContainer, primaryIPPrefix netip.Prefix, subnet cns.IPSubnet, config *configuration.CNSConfig) (*cns.CreateNetworkContainerRequest, error) { secondaryIPConfigs := map[string]cns.SecondaryIPConfig{} // the masked address is the 0th IP in the subnet and startingAddr is the 2nd IP (*.1) startingAddr := primaryIPPrefix.Masked().Addr().Next() @@ -27,12 +28,15 @@ func createNCRequestFromStaticNCHelper(nc v1alpha.NetworkContainer, primaryIPPre // iterate through all IP addresses in the subnet described by primaryPrefix and // add them to the request as secondary IPConfigs. - for addr := startingAddr; primaryIPPrefix.Contains(addr); addr = addr.Next() { - secondaryIPConfigs[addr.String()] = cns.SecondaryIPConfig{ - IPAddress: addr.String(), - NCVersion: int(nc.Version), + // Process primary prefix IPs in all scenarios except when nc.Type is v1alpha.VNETBlock AND SwiftV2 is enabled + if !(config.EnableSwiftV2 && nc.Type == v1alpha.VNETBlock) { + for addr := startingAddr; primaryIPPrefix.Contains(addr); addr = addr.Next() { + secondaryIPConfigs[addr.String()] = cns.SecondaryIPConfig{ + IPAddress: addr.String(), + NCVersion: int(nc.Version), + } + lastAddr = addr } - lastAddr = addr } if nc.Type == v1alpha.VNETBlock { diff --git a/cns/kubecontroller/nodenetworkconfig/reconciler.go b/cns/kubecontroller/nodenetworkconfig/reconciler.go index 4d5eb1bcd1..2c04532524 100644 --- a/cns/kubecontroller/nodenetworkconfig/reconciler.go +++ b/cns/kubecontroller/nodenetworkconfig/reconciler.go @@ -5,6 +5,7 @@ import ( "sync" "github.com/Azure/azure-container-networking/cns" + "github.com/Azure/azure-container-networking/cns/configuration" "github.com/Azure/azure-container-networking/cns/logger" "github.com/Azure/azure-container-networking/cns/restserver" cnstypes "github.com/Azure/azure-container-networking/cns/types" @@ -43,18 +44,20 @@ type Reconciler struct { once sync.Once started chan interface{} nodeIP string + config *configuration.CNSConfig } // NewReconciler creates a NodeNetworkConfig Reconciler which will get updates from the Kubernetes // apiserver for NNC events. // Provided nncListeners are passed the NNC after the Reconcile preprocesses it. Note: order matters! The // passed Listeners are notified in the order provided. -func NewReconciler(cnscli cnsClient, ipampoolmonitorcli nodeNetworkConfigListener, nodeIP string) *Reconciler { +func NewReconciler(cnscli cnsClient, ipampoolmonitorcli nodeNetworkConfigListener, nodeIP string, config *configuration.CNSConfig) *Reconciler { return &Reconciler{ cnscli: cnscli, ipampoolmonitorcli: ipampoolmonitorcli, started: make(chan interface{}), nodeIP: nodeIP, + config: config, } } @@ -104,7 +107,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (reco switch nnc.Status.NetworkContainers[i].AssignmentMode { //nolint:exhaustive // skipping dynamic case // For Overlay and Vnet Scale Scenarios case v1alpha.Static: - req, err = CreateNCRequestFromStaticNC(nnc.Status.NetworkContainers[i]) + req, err = CreateNCRequestFromStaticNC(nnc.Status.NetworkContainers[i], r.config) // For Pod Subnet scenario default: // For backward compatibility, default will be treated as Dynamic too. req, err = CreateNCRequestFromDynamicNC(nnc.Status.NetworkContainers[i]) diff --git a/cns/kubecontroller/nodenetworkconfig/reconciler_test.go b/cns/kubecontroller/nodenetworkconfig/reconciler_test.go index aed3de81b7..a2e3e5cf0b 100644 --- a/cns/kubecontroller/nodenetworkconfig/reconciler_test.go +++ b/cns/kubecontroller/nodenetworkconfig/reconciler_test.go @@ -5,6 +5,7 @@ import ( "testing" "github.com/Azure/azure-container-networking/cns" + "github.com/Azure/azure-container-networking/cns/configuration" "github.com/Azure/azure-container-networking/cns/logger" cnstypes "github.com/Azure/azure-container-networking/cns/types" "github.com/Azure/azure-container-networking/crd/nodenetworkconfig/api/v1alpha" @@ -192,7 +193,8 @@ func TestReconcile(t *testing.T) { } t.Run(tt.name, func(t *testing.T) { - r := NewReconciler(&tt.cnsClient, &tt.cnsClient, tt.nodeIP) + config := &configuration.CNSConfig{} // Use default config for tests + r := NewReconciler(&tt.cnsClient, &tt.cnsClient, tt.nodeIP, config) r.nnccli = &tt.ncGetter got, err := r.Reconcile(context.Background(), tt.in) if tt.wantErr { @@ -249,7 +251,8 @@ func TestReconcileStaleNCs(t *testing.T) { return &nncLog[len(nncLog)-1], nil } - r := NewReconciler(&cnsClient, &cnsClient, nodeIP) + config := &configuration.CNSConfig{} // Use default config for tests + r := NewReconciler(&cnsClient, &cnsClient, nodeIP, config) r.nnccli = &mockNCGetter{get: nncIterator} _, err := r.Reconcile(context.Background(), reconcile.Request{}) diff --git a/cns/restserver/internalapi.go b/cns/restserver/internalapi.go index 8d477cbab8..6aa44aa628 100644 --- a/cns/restserver/internalapi.go +++ b/cns/restserver/internalapi.go @@ -216,6 +216,8 @@ func (service *HTTPRestService) syncHostNCVersion(ctx context.Context, channelMo if len(outdatedNCs) == 0 { return len(programmedNCs), nil } + // Todo: Query NMA for the supported API to validate the build version. If supported, query IMDS for the Swiftv2 NC version list and perform synchostversion update. + // WorkItem: https://msazure.visualstudio.com/One/_workitems/edit/33460135 ncVersionListResp, err := service.nma.GetNCVersionList(ctx) if err != nil { return len(programmedNCs), errors.Wrap(err, "failed to get nc version list from nmagent") diff --git a/cns/restserver/internalapi_linux.go b/cns/restserver/internalapi_linux.go index ef30dabf03..cd2c77f463 100644 --- a/cns/restserver/internalapi_linux.go +++ b/cns/restserver/internalapi_linux.go @@ -68,6 +68,12 @@ func (service *HTTPRestService) programSNATRules(req *cns.CreateNetworkContainer // use any secondary ip + the nnc prefix length to get an iptables rule to allow dns and imds traffic from the pods for _, v := range req.SecondaryIPConfigs { + // check if the ip address is IPv4. A check is required because DNS and IMDS do not have IPv6 addresses. Since support currently exists only for IPv4, other ip families are skipped. + if net.ParseIP(v.IPAddress).To4() == nil { + // skip if the ip address is not IPv4 + continue + } + // put the ip address in standard cidr form (where we zero out the parts that are not relevant) _, podSubnet, _ := net.ParseCIDR(v.IPAddress + "/" + fmt.Sprintf("%d", req.IPConfiguration.IPSubnet.PrefixLength)) diff --git a/cns/restserver/ipam.go b/cns/restserver/ipam.go index 883d9aaef3..326979c979 100644 --- a/cns/restserver/ipam.go +++ b/cns/restserver/ipam.go @@ -8,6 +8,7 @@ import ( "fmt" "net" "net/http" + "net/netip" "strconv" "strings" @@ -995,38 +996,64 @@ func (service *HTTPRestService) AssignAvailableIPConfigs(podInfo cns.PodInfo) ([ if numOfNCs == 0 { return nil, ErrNoNCs } + + // Get the number of distinct IP families (IPv4/IPv6) across all NC's + numOfIPFamilies := service.GetIPFamilyCount() + + // Get the actual IP families map for validation + ncIPFamilies := service.getIPFamiliesMap() + + // Determine the number of IPs to assign based on IP families found + numberOfIPs := numOfNCs + if numOfIPFamilies != 0 { + numberOfIPs = numOfIPFamilies + } + service.Lock() defer service.Unlock() // Creates a slice of PodIpInfo with the size as number of NCs to hold the result for assigned IP configs - podIPInfo := make([]cns.PodIpInfo, numOfNCs) + podIPInfo := make([]cns.PodIpInfo, numberOfIPs) // This map is used to store whether or not we have found an available IP from an NC when looping through the pool ipsToAssign := make(map[string]cns.IPConfigurationStatus) // Searches for available IPs in the pool for _, ipState := range service.PodIPConfigState { - // check if an IP from this NC is already set side for assignment. - if _, ncAlreadyMarkedForAssignment := ipsToAssign[ipState.NCID]; ncAlreadyMarkedForAssignment { + + // get the IPFamily of the current ipState + var ipStateFamily cns.IPFamily + if net.ParseIP(ipState.IPAddress).To4() != nil { + ipStateFamily = cns.IPv4 + } else { + ipStateFamily = cns.IPv6 + } + + key := generateAssignedIPKey(ipState.NCID, ipStateFamily) + + // check if the IP with the same family type exists already + if _, ncIPFamilyAlreadyMarkedForAssignment := ipsToAssign[key]; ncIPFamilyAlreadyMarkedForAssignment { continue } // Checks if the current IP is available if ipState.GetState() != types.Available { continue } - ipsToAssign[ipState.NCID] = ipState - // Once one IP per container is found break out of the loop and stop searching - if len(ipsToAssign) == numOfNCs { + ipsToAssign[key] = ipState + // Once numberOfIPs per container is found break out of the loop and stop searching + if len(ipsToAssign) == numberOfIPs { break } } - // Checks to make sure we found one IP for each NC - if len(ipsToAssign) != numOfNCs { + // Checks to make sure we found one IP for each NCxIPFamily + if len(ipsToAssign) != numberOfIPs { for ncID := range service.state.ContainerStatus { - if _, found := ipsToAssign[ncID]; found { - continue + for ipFamily := range ncIPFamilies { + if _, found := ipsToAssign[generateAssignedIPKey(ncID, ipFamily)]; found { + continue + } + return podIPInfo, errors.Errorf("not enough IPs available of type %s for %s, waiting on Azure CNS to allocate more with NC Status: %s", + ipFamily, ncID, string(service.state.ContainerStatus[ncID].CreateNetworkContainerRequest.NCStatus)) } - return podIPInfo, errors.Errorf("not enough IPs available for %s, waiting on Azure CNS to allocate more with NC Status: %s", - ncID, string(service.state.ContainerStatus[ncID].CreateNetworkContainerRequest.NCStatus)) } } @@ -1061,10 +1088,18 @@ func (service *HTTPRestService) AssignAvailableIPConfigs(podInfo cns.PodInfo) ([ return podIPInfo, fmt.Errorf("not enough IPs available, waiting on Azure CNS to allocate more") } - logger.Printf("[AssignDesiredIPConfigs] Successfully assigned IPs for pod %+v", podInfo) + //nolint:staticcheck // SA1019: suppress deprecated logger.Printf usage. Todo: legacy logger usage is consistent in cns repo. Migrates when all logger usage is migrated + logger.Printf( + "[AssignAvailableIPConfigs] Successfully assigned IPs for pod %+v", + podInfo, + ) return podIPInfo, nil } +func generateAssignedIPKey(ncID string, ipFamily cns.IPFamily) string { + return fmt.Sprintf("%s_%s", ncID, string(ipFamily)) +} + // If IPConfigs are already assigned to the pod, it returns that else it returns the available ipconfigs. func requestIPConfigsHelper(service *HTTPRestService, req cns.IPConfigsRequest) ([]cns.PodIpInfo, error) { // check if ipconfigs already assigned to this pod and return if exists or error @@ -1327,3 +1362,41 @@ func verifyUpdateEndpointStateRequest(req map[string]*IPInfo) error { } return nil } + +// getIPFamiliesMap returns a map of IP families present across all NC's +func (service *HTTPRestService) getIPFamiliesMap() map[cns.IPFamily]struct{} { + ncIPFamilies := map[cns.IPFamily]struct{}{} + + for ncID := range service.state.ContainerStatus { + // Exit if we already found both IPv4 and IPv6 + if len(ncIPFamilies) == 2 { + break + } + + for _, secIPConfig := range service.state.ContainerStatus[ncID].CreateNetworkContainerRequest.SecondaryIPConfigs { + // Exit if we already found both IPv4 and IPv6 + if len(ncIPFamilies) == 2 { + break + } + addr, err := netip.ParseAddr(secIPConfig.IPAddress) + if err != nil { + continue + } + if addr.Is4() { + ncIPFamilies[cns.IPv4] = struct{}{} + } else if addr.Is6() { + ncIPFamilies[cns.IPv6] = struct{}{} + } + } + } + + return ncIPFamilies +} + +// GetIPFamilyCount returns the number of distinct IP families (IPv4/IPv6) across all NC's. +// This is used to determine how many IPs to assign per pod: +// - In single-stack: 1 IP per pod +// - In dual-stack: 2 IPs per pod (one IPv4, one IPv6) +func (service *HTTPRestService) GetIPFamilyCount() int { + return len(service.getIPFamiliesMap()) +} diff --git a/cns/restserver/ipam_test.go b/cns/restserver/ipam_test.go index 52c5b6d0d6..dc582a0e8d 100644 --- a/cns/restserver/ipam_test.go +++ b/cns/restserver/ipam_test.go @@ -178,36 +178,46 @@ func updatePnpIDMacAddressState(svc *HTTPRestService) { } } -// create an endpoint with only one IP -func TestEndpointStateReadAndWriteSingleNC(t *testing.T) { - ncStates := []ncState{ +func TestEndpointStateReadAndWrite(t *testing.T) { + testNcs := [][]ncState{ + // single stack NC { - ncID: testNCID, - ips: []string{ - testIP1, + { + ncID: testNCID, + ips: []string{ + testIP1, + }, }, }, - } - endpointStateReadAndWrite(t, ncStates) -} - -// create an endpoint with one IP from each NC -func TestEndpointStateReadAndWriteMultipleNCs(t *testing.T) { - ncStates := []ncState{ + // two NCs (one V4 and one V6) { - ncID: testNCID, - ips: []string{ - testIP1, + { + ncID: testNCID, + ips: []string{ + testIP1, + }, + }, + { + ncID: testNCIDv6, + ips: []string{ + testIP1v6, + }, }, }, + // dualstack NC { - ncID: testNCIDv6, - ips: []string{ - testIP1v6, + { + ncID: testNCID, + ips: []string{ + testIP1, + testIP1v6, + }, }, }, } - endpointStateReadAndWrite(t, ncStates) + for _, ncState := range testNcs { + endpointStateReadAndWrite(t, ncState) + } } // Tests the creation of an endpoint using the NCs and IPs as input and then tests the deletion of that endpoint @@ -282,35 +292,46 @@ func endpointStateReadAndWrite(t *testing.T, ncStates []ncState) { } // assign the available IP to the new pod -func TestIPAMGetAvailableIPConfigSingleNC(t *testing.T) { - ncStates := []ncState{ +func TestIPAMGetAvailableIPConfig(t *testing.T) { + testNcs := [][]ncState{ + // single stack NC { - ncID: testNCID, - ips: []string{ - testIP1, + { + ncID: testNCID, + ips: []string{ + testIP1, + }, }, }, - } - ipamGetAvailableIPConfig(t, ncStates) -} - -// assign one IP per NC to the pod -func TestIPAMGetAvailableIPConfigMultipleNCs(t *testing.T) { - nsStates := []ncState{ + // two NCs (one V4 and one V6) { - ncID: testNCID, - ips: []string{ - testIP1, + { + ncID: testNCID, + ips: []string{ + testIP1, + }, + }, + { + ncID: testNCIDv6, + ips: []string{ + testIP1v6, + }, }, }, + // dualstack NC { - ncID: testNCIDv6, - ips: []string{ - testIP1v6, + { + ncID: testNCID, + ips: []string{ + testIP1, + testIP1v6, + }, }, }, } - ipamGetAvailableIPConfig(t, nsStates) + for _, ncState := range testNcs { + ipamGetAvailableIPConfig(t, ncState) + } } // Add one IP per NC to the pool and request those IPs @@ -359,37 +380,51 @@ func ipamGetAvailableIPConfig(t *testing.T, ncStates []ncState) { } } -func TestIPAMGetNextAvailableIPConfigSingleNC(t *testing.T) { - ncStates := []ncState{ +func TestIPAMGetNextAvailableIPConfig(t *testing.T) { + testNcs := [][]ncState{ + // single stack NC { - ncID: testNCID, - ips: []string{ - testIP1, - testIP2, + { + ncID: testNCID, + ips: []string{ + testIP1, + testIP2, + }, }, }, - } - getNextAvailableIPConfig(t, ncStates) -} - -func TestIPAMGetNextAvailableIPConfigMultipleNCs(t *testing.T) { - ncStates := []ncState{ + // two NCs (one V4 and one V6) { - ncID: testNCID, - ips: []string{ - testIP1, - testIP2, + { + ncID: testNCID, + ips: []string{ + testIP1, + testIP2, + }, + }, + { + ncID: testNCIDv6, + ips: []string{ + testIP1v6, + testIP2v6, + }, }, }, + // dualstack NC { - ncID: testNCIDv6, - ips: []string{ - testIP1v6, - testIP2v6, + { + ncID: testNCID, + ips: []string{ + testIP1, + testIP2, + testIP1v6, + testIP2v6, + }, }, }, } - getNextAvailableIPConfig(t, ncStates) + for _, ncState := range testNcs { + getNextAvailableIPConfig(t, ncState) + } } // First IP is already assigned to a pod, want second IP @@ -442,34 +477,46 @@ func getNextAvailableIPConfig(t *testing.T, ncStates []ncState) { } } -func TestIPAMGetAlreadyAssignedIPConfigForSamePodSingleNC(t *testing.T) { - ncStates := []ncState{ +func TestIPAMGetAlreadyAssignedIPConfigForSamePod(t *testing.T) { + testNcs := [][]ncState{ + // single stack NC { - ncID: testNCID, - ips: []string{ - testIP1, + { + ncID: testNCID, + ips: []string{ + testIP1, + }, }, }, - } - ipamGetAlreadyAssignedIPConfigForSamePod(t, ncStates) -} - -func TestIPAMGetAlreadyAssignedIPConfigForSamePodMultipleNCs(t *testing.T) { - ncStates := []ncState{ + // two NCs (one V4 and one V6) { - ncID: testNCID, - ips: []string{ - testIP1, + { + ncID: testNCID, + ips: []string{ + testIP1, + }, + }, + { + ncID: testNCIDv6, + ips: []string{ + testIP1v6, + }, }, }, + // dualstack NC { - ncID: testNCIDv6, - ips: []string{ - testIP1v6, + { + ncID: testNCID, + ips: []string{ + testIP1, + testIP1v6, + }, }, }, } - ipamGetAlreadyAssignedIPConfigForSamePod(t, ncStates) + for _, ncState := range testNcs { + ipamGetAlreadyAssignedIPConfigForSamePod(t, ncState) + } } func ipamGetAlreadyAssignedIPConfigForSamePod(t *testing.T, ncStates []ncState) { @@ -517,37 +564,51 @@ func ipamGetAlreadyAssignedIPConfigForSamePod(t *testing.T, ncStates []ncState) } } -func TestIPAMAttemptToRequestIPNotFoundInPoolSingleNC(t *testing.T) { - ncStates := []ncState{ +func TestIPAMAttemptToRequestIPNotFoundInPool(t *testing.T) { + testNcs := [][]ncState{ + // single stack NC { - ncID: testNCID, - ips: []string{ - testIP1, - testIP2, + { + ncID: testNCID, + ips: []string{ + testIP1, + testIP2, + }, }, }, - } - ipamAttemptToRequestIPNotFoundInPool(t, ncStates) -} - -func TestIPAMAttemptToRequestIPNotFoundInPoolMultipleNCs(t *testing.T) { - ncStates := []ncState{ + // two NCs (one V4 and one V6) { - ncID: testNCID, - ips: []string{ - testIP1, - testIP2, + { + ncID: testNCID, + ips: []string{ + testIP1, + testIP2, + }, + }, + { + ncID: testNCIDv6, + ips: []string{ + testIP1v6, + testIP2v6, + }, }, }, + // dualstack NC { - ncID: testNCIDv6, - ips: []string{ - testIP1v6, - testIP2v6, + { + ncID: testNCID, + ips: []string{ + testIP1, + testIP2, + testIP1v6, + testIP2v6, + }, }, }, } - ipamAttemptToRequestIPNotFoundInPool(t, ncStates) + for _, ncState := range testNcs { + ipamAttemptToRequestIPNotFoundInPool(t, ncState) + } } func ipamAttemptToRequestIPNotFoundInPool(t *testing.T, ncStates []ncState) { @@ -580,34 +641,46 @@ func ipamAttemptToRequestIPNotFoundInPool(t *testing.T, ncStates []ncState) { } } -func TestIPAMGetDesiredIPConfigWithSpecfiedIPSingleNC(t *testing.T) { - ncStates := []ncState{ +func TestIPAMGetDesiredIPConfigWithSpecfiedIP(t *testing.T) { + testNcs := [][]ncState{ + // single stack NC { - ncID: testNCID, - ips: []string{ - testIP1, + { + ncID: testNCID, + ips: []string{ + testIP1, + }, }, }, - } - ipamGetDesiredIPConfigWithSpecfiedIP(t, ncStates) -} - -func TestIPAMGetDesiredIPConfigWithSpecfiedIPMultipleNCs(t *testing.T) { - ncStates := []ncState{ + // two NCs (one V4 and one V6) { - ncID: testNCID, - ips: []string{ - testIP1, + { + ncID: testNCID, + ips: []string{ + testIP1, + }, + }, + { + ncID: testNCIDv6, + ips: []string{ + testIP1v6, + }, }, }, + // dualstack NC { - ncID: testNCIDv6, - ips: []string{ - testIP1v6, + { + ncID: testNCID, + ips: []string{ + testIP1, + testIP1v6, + }, }, }, } - ipamGetDesiredIPConfigWithSpecfiedIP(t, ncStates) + for _, ncState := range testNcs { + ipamGetDesiredIPConfigWithSpecfiedIP(t, ncState) + } } func ipamGetDesiredIPConfigWithSpecfiedIP(t *testing.T, ncStates []ncState) { @@ -659,34 +732,46 @@ func ipamGetDesiredIPConfigWithSpecfiedIP(t *testing.T, ncStates []ncState) { } } -func TestIPAMFailToGetDesiredIPConfigWithAlreadyAssignedSpecfiedIPSingleNC(t *testing.T) { - ncStates := []ncState{ +func TestIPAMFailToGetDesiredIPConfigWithAlreadyAssignedSpecfiedIP(t *testing.T) { + testNcs := [][]ncState{ + // single stack NC { - ncID: testNCID, - ips: []string{ - testIP1, + { + ncID: testNCID, + ips: []string{ + testIP1, + }, }, }, - } - ipamFailToGetDesiredIPConfigWithAlreadyAssignedSpecfiedIP(t, ncStates) -} - -func TestIPAMFailToGetDesiredIPConfigWithAlreadyAssignedSpecfiedIPMultipleNCs(t *testing.T) { - ncStates := []ncState{ + // two NCs (one V4 and one V6) { - ncID: testNCID, - ips: []string{ - testIP1, + { + ncID: testNCID, + ips: []string{ + testIP1, + }, + }, + { + ncID: testNCIDv6, + ips: []string{ + testIP1v6, + }, }, }, + // dualstack NC { - ncID: testNCIDv6, - ips: []string{ - testIP1v6, + { + ncID: testNCID, + ips: []string{ + testIP1, + testIP1v6, + }, }, }, } - ipamFailToGetDesiredIPConfigWithAlreadyAssignedSpecfiedIP(t, ncStates) + for _, ncState := range testNcs { + ipamFailToGetDesiredIPConfigWithAlreadyAssignedSpecfiedIP(t, ncState) + } } func ipamFailToGetDesiredIPConfigWithAlreadyAssignedSpecfiedIP(t *testing.T, ncStates []ncState) { @@ -720,37 +805,51 @@ func ipamFailToGetDesiredIPConfigWithAlreadyAssignedSpecfiedIP(t *testing.T, ncS } } -func TestIPAMFailToGetIPWhenAllIPsAreAssignedSingleNC(t *testing.T) { - ncStates := []ncState{ +func TestIPAMFailToGetIPWhenAllIPsAreAssigned(t *testing.T) { + testNcs := [][]ncState{ + // single stack NC { - ncID: testNCID, - ips: []string{ - testIP1, - testIP2, + { + ncID: testNCID, + ips: []string{ + testIP1, + testIP2, + }, }, }, - } - ipamFailToGetIPWhenAllIPsAreAssigned(t, ncStates) -} - -func TestIPAMFailToGetIPWhenAllIPsAreAssignedMultipleNCs(t *testing.T) { - ncStates := []ncState{ + // two NCs (one V4 and one V6) { - ncID: testNCID, - ips: []string{ - testIP1, - testIP2, + { + ncID: testNCID, + ips: []string{ + testIP1, + testIP2, + }, + }, + { + ncID: testNCIDv6, + ips: []string{ + testIP1v6, + testIP2v6, + }, }, }, + // dualstack NC { - ncID: testNCIDv6, - ips: []string{ - testIP1v6, - testIP2v6, + { + ncID: testNCID, + ips: []string{ + testIP1, + testIP2, + testIP1v6, + testIP2v6, + }, }, }, } - ipamFailToGetIPWhenAllIPsAreAssigned(t, ncStates) + for _, ncState := range testNcs { + ipamFailToGetIPWhenAllIPsAreAssigned(t, ncState) + } } func ipamFailToGetIPWhenAllIPsAreAssigned(t *testing.T, ncStates []ncState) { @@ -780,34 +879,46 @@ func ipamFailToGetIPWhenAllIPsAreAssigned(t *testing.T, ncStates []ncState) { } } -func TestIPAMRequestThenReleaseThenRequestAgainSingleNC(t *testing.T) { - ncStates := []ncState{ +func TestIPAMRequestThenReleaseThenRequestAgain(t *testing.T) { + testNcs := [][]ncState{ + // single stack NC { - ncID: testNCID, - ips: []string{ - testIP1, + { + ncID: testNCID, + ips: []string{ + testIP1, + }, }, }, - } - ipamRequestThenReleaseThenRequestAgain(t, ncStates) -} - -func TestIPAMRequestThenReleaseThenRequestAgainMultipleNCs(t *testing.T) { - ncStates := []ncState{ + // two NCs (one V4 and one V6) { - ncID: testNCID, - ips: []string{ - testIP1, + { + ncID: testNCID, + ips: []string{ + testIP1, + }, + }, + { + ncID: testNCIDv6, + ips: []string{ + testIP1v6, + }, }, }, + // dualstack NC { - ncID: testNCIDv6, - ips: []string{ - testIP1v6, + { + ncID: testNCID, + ips: []string{ + testIP1, + testIP1v6, + }, }, }, } - ipamRequestThenReleaseThenRequestAgain(t, ncStates) + for _, ncState := range testNcs { + ipamRequestThenReleaseThenRequestAgain(t, ncState) + } } // 10.0.0.1 = PodInfo1 @@ -889,34 +1000,46 @@ func ipamRequestThenReleaseThenRequestAgain(t *testing.T, ncStates []ncState) { } } -func TestIPAMReleaseIPIdempotencySingleNC(t *testing.T) { - ncStates := []ncState{ +func TestIPAMReleaseIPIdempotency(t *testing.T) { + testNcs := [][]ncState{ + // single stack NC { - ncID: testNCID, - ips: []string{ - testIP1, + { + ncID: testNCID, + ips: []string{ + testIP1, + }, }, }, - } - ipamReleaseIPIdempotency(t, ncStates) -} - -func TestIPAMReleaseIPIdempotencyMultipleNCs(t *testing.T) { - ncStates := []ncState{ + // two NCs (one V4 and one V6) { - ncID: testNCID, - ips: []string{ - testIP1, + { + ncID: testNCID, + ips: []string{ + testIP1, + }, + }, + { + ncID: testNCIDv6, + ips: []string{ + testIP1v6, + }, }, }, + // dualstack NC { - ncID: testNCIDv6, - ips: []string{ - testIP1v6, + { + ncID: testNCID, + ips: []string{ + testIP1, + testIP1v6, + }, }, }, } - ipamReleaseIPIdempotency(t, ncStates) + for _, ncState := range testNcs { + ipamReleaseIPIdempotency(t, ncState) + } } func ipamReleaseIPIdempotency(t *testing.T, ncStates []ncState) { @@ -945,34 +1068,46 @@ func ipamReleaseIPIdempotency(t *testing.T, ncStates []ncState) { } } -func TestIPAMAllocateIPIdempotencySingleNC(t *testing.T) { - ncStates := []ncState{ +func TestIPAMAllocateIPIdempotency(t *testing.T) { + testNcs := [][]ncState{ + // single stack NC { - ncID: testNCID, - ips: []string{ - testIP1, + { + ncID: testNCID, + ips: []string{ + testIP1, + }, }, }, - } - ipamAllocateIPIdempotency(t, ncStates) -} - -func TestIPAMAllocateIPIdempotencyMultipleNCs(t *testing.T) { - ncStates := []ncState{ + // two NCs (one V4 and one V6) { - ncID: testNCID, - ips: []string{ - testIP1, + { + ncID: testNCID, + ips: []string{ + testIP1, + }, + }, + { + ncID: testNCIDv6, + ips: []string{ + testIP1v6, + }, }, }, + // dualstack NC { - ncID: testNCIDv6, - ips: []string{ - testIP1v6, + { + ncID: testNCID, + ips: []string{ + testIP1, + testIP1v6, + }, }, }, } - ipamAllocateIPIdempotency(t, ncStates) + for _, ncState := range testNcs { + ipamAllocateIPIdempotency(t, ncState) + } } func ipamAllocateIPIdempotency(t *testing.T, ncStates []ncState) { @@ -994,40 +1129,56 @@ func ipamAllocateIPIdempotency(t *testing.T, ncStates []ncState) { } } -func TestAvailableIPConfigsSingleNC(t *testing.T) { - ncStates := []ncState{ +func TestAvailableIPConfigs(t *testing.T) { + testNcs := [][]ncState{ + // single stack NC { - ncID: testNCID, - ips: []string{ - testIP1, - testIP2, - testIP3, + { + ncID: testNCID, + ips: []string{ + testIP1, + testIP2, + testIP3, + }, }, }, - } - availableIPConfigs(t, ncStates) -} - -func TestAvailableIPConfigsMultipleNCs(t *testing.T) { - ncStates := []ncState{ + // two NCs (one V4 and one V6) { - ncID: testNCID, - ips: []string{ - testIP1, - testIP2, - testIP3, + { + ncID: testNCID, + ips: []string{ + testIP1, + testIP2, + testIP3, + }, + }, + { + ncID: testNCIDv6, + ips: []string{ + testIP1v6, + testIP2v6, + testIP3v6, + }, }, }, + // dualstack NC { - ncID: testNCIDv6, - ips: []string{ - testIP1v6, - testIP2v6, - testIP3v6, + { + ncID: testNCID, + ips: []string{ + testIP1, + testIP2, + testIP3, + testIP1v6, + testIP2v6, + testIP3v6, + }, }, }, } - availableIPConfigs(t, ncStates) + for _, ncState := range testNcs { + availableIPConfigs(t, ncState) + } } func availableIPConfigs(t *testing.T, ncStates []ncState) { @@ -1113,34 +1264,46 @@ func validateIpState(t *testing.T, actualIps []cns.IPConfigurationStatus, expect } } -func TestIPAMMarkIPCountAsPendingSingleNC(t *testing.T) { - ncStates := []ncState{ +func TestIPAMMarkIPCountAsPending(t *testing.T) { + testNcs := [][]ncState{ + // single stack NC { - ncID: testNCID, - ips: []string{ - testIP1, + { + ncID: testNCID, + ips: []string{ + testIP1, + }, }, }, - } - ipamMarkIPCountAsPending(t, ncStates) -} - -func TestIPAMMarkIPCountAsPendingMultipleNCs(t *testing.T) { - ncStates := []ncState{ + // two NCs (one V4 and one V6) { - ncID: testNCID, - ips: []string{ - testIP1, + { + ncID: testNCID, + ips: []string{ + testIP1, + }, + }, + { + ncID: testNCIDv6, + ips: []string{ + testIP1v6, + }, }, }, + // dualstack NC { - ncID: testNCIDv6, - ips: []string{ - testIP1v6, + { + ncID: testNCID, + ips: []string{ + testIP1, + testIP1v6, + }, }, }, } - ipamMarkIPCountAsPending(t, ncStates) + for _, ncState := range testNcs { + ipamMarkIPCountAsPending(t, ncState) + } } func ipamMarkIPCountAsPending(t *testing.T, ncStates []ncState) { @@ -1272,37 +1435,51 @@ func constructSecondaryIPConfigs(ipAddress, uuid string, ncVersion int, secondar secondaryIPConfigs[uuid] = secIPConfig } -func TestIPAMMarkExistingIPConfigAsPendingSingleNC(t *testing.T) { - ncStates := []ncState{ +func TestIPAMMarkExistingIPConfigAsPending(t *testing.T) { + testNcs := [][]ncState{ + // single stack NC { - ncID: testNCID, - ips: []string{ - testIP1, - testIP2, + { + ncID: testNCID, + ips: []string{ + testIP1, + testIP2, + }, }, }, - } - ipamMarkExistingIPConfigAsPending(t, ncStates) -} - -func TestIPAMMarkExistingIPConfigAsPendingMultipleNCs(t *testing.T) { - ncStates := []ncState{ + // two NCs (one V4 and one V6) { - ncID: testNCID, - ips: []string{ - testIP1, - testIP2, + { + ncID: testNCID, + ips: []string{ + testIP1, + testIP2, + }, + }, + { + ncID: testNCIDv6, + ips: []string{ + testIP1v6, + testIP2v6, + }, }, }, + // dualstack NC { - ncID: testNCIDv6, - ips: []string{ - testIP1v6, - testIP2v6, + { + ncID: testNCID, + ips: []string{ + testIP1, + testIP2, + testIP1v6, + testIP2v6, + }, }, }, } - ipamMarkExistingIPConfigAsPending(t, ncStates) + for _, ncState := range testNcs { + ipamMarkExistingIPConfigAsPending(t, ncState) + } } func ipamMarkExistingIPConfigAsPending(t *testing.T, ncStates []ncState) { @@ -1433,27 +1610,32 @@ func TestIPAMReleaseOneIPWhenExpectedToHaveTwo(t *testing.T) { func TestIPAMFailToRequestOneIPWhenExpectedToHaveTwo(t *testing.T) { svc := getTestService(cns.KubernetesCRD) - // set state as already assigned - testState := newPodState(testIP1, ipIDs[0][0], testNCID, types.Available, 0) + testState1, _ := newPodStateWithOrchestratorContext(testIP1, testIPID1, testNCID, types.Assigned, 24, 0, testPod1Info) + testState2 := newPodState(testIP2, testIPID2, testNCID, types.Available, 0) ipconfigs := map[string]cns.IPConfigurationStatus{ - testState.ID: testState, + testState1.ID: testState1, + testState2.ID: testState2, + } + testState1v6, _ := newPodStateWithOrchestratorContext(testIP1v6, testIPID1v6, testNCIDv6, types.Assigned, 120, 0, testPod1Info) + // setting v6 IPs to Assigned so there are none available + ipconfigsV6 := map[string]cns.IPConfigurationStatus{ + testState1v6.ID: testState1v6, } - emptyIpconfigs := map[string]cns.IPConfigurationStatus{} err := updatePodIPConfigState(t, svc, ipconfigs, testNCID) if err != nil { t.Fatalf("Expected to not fail adding IPs to state: %+v", err) } - err = updatePodIPConfigState(t, svc, emptyIpconfigs, testNCIDv6) + err = updatePodIPConfigState(t, svc, ipconfigsV6, testNCIDv6) if err != nil { t.Fatalf("Expected to not fail adding empty NC to state: %+v", err) } // request should expect 2 IPs but there is only 1 in the pool req := cns.IPConfigsRequest{} - b, _ := testPod1Info.OrchestratorContext() + b, _ := testPod2Info.OrchestratorContext() req.OrchestratorContext = b _, err = requestIPAddressAndGetState(t, req) @@ -2128,6 +2310,91 @@ func setupMockNMAgent(t *testing.T, svc *HTTPRestService, mockNMAgent *fakes.NMA }) } +func TestIPAMRequestIPConfigsTwoFamiliesSameNC(t *testing.T) { + svc := getTestService(cns.KubernetesCRD) + + // Add single NC with both IPv4 and IPv6 IPs + mixedConfigs := map[string]cns.IPConfigurationStatus{ + "ipv4-1": newPodState("10.0.0.1", "ipv4-1", testNCID, types.Available, 0), + "ipv4-2": newPodState("10.0.0.2", "ipv4-2", testNCID, types.Available, 0), + "ipv6-1": newPodState("2001:db8::1", "ipv6-1", testNCID, types.Available, 0), + "ipv6-2": newPodState("2001:db8::2", "ipv6-2", testNCID, types.Available, 0), + } + + err := updatePodIPConfigState(t, svc, mixedConfigs, testNCID) + require.NoError(t, err) + + req := cns.IPConfigsRequest{ + PodInterfaceID: testPod1Info.InterfaceID(), + InfraContainerID: testPod1Info.InfraContainerID(), + } + b, _ := testPod1Info.OrchestratorContext() + req.OrchestratorContext = b + + podIPs, err := requestIPConfigsHelper(svc, req) + require.NoError(t, err) + + // Should assign 2 IPs (one of each family) of same NC + assert.Len(t, podIPs, 2) + + // Verify we got one of each family + hasIPv4 := false + hasIPv6 := false + for _, podIP := range podIPs { + ip := net.ParseIP(podIP.PodIPConfig.IPAddress) + if ip.To4() != nil { + hasIPv4 = true + } else { + hasIPv6 = true + } + } + assert.True(t, hasIPv4, "Should have IPv4 address") + assert.True(t, hasIPv6, "Should have IPv6 address") +} + +func TestIPAMRequestIPConfigsProgrammingPendingFailure(t *testing.T) { + svc := getTestService(cns.KubernetesCRD) + + // Add IPv4 NC with available IP + ipv4configs := map[string]cns.IPConfigurationStatus{ + "ipv4-1": newPodState("10.0.0.1", "ipv4-1", testNCID, types.Available, 0), + } + + // Add IPv6 NC with IP initially available + ipv6configs := map[string]cns.IPConfigurationStatus{ + "ipv6-1": newPodState("2001:db8::1", "ipv6-1", testNCIDv6, types.Available, 0), + } + + err := updatePodIPConfigState(t, svc, ipv4configs, testNCID) + require.NoError(t, err) + err = updatePodIPConfigState(t, svc, ipv6configs, testNCIDv6) + require.NoError(t, err) + + // Manually set IPv6 IP to PendingProgramming state after NC creation + ipv6State := svc.PodIPConfigState["ipv6-1"] + ipv6State.SetState(types.PendingProgramming) + svc.PodIPConfigState["ipv6-1"] = ipv6State + + req := cns.IPConfigsRequest{ + PodInterfaceID: testPod1Info.InterfaceID(), + InfraContainerID: testPod1Info.InfraContainerID(), + } + b, _ := testPod1Info.OrchestratorContext() + req.OrchestratorContext = b + + _, err = requestIPConfigsHelper(svc, req) + require.Error(t, err) + assert.Contains(t, err.Error(), "not enough IPs available") + + // Verify no IPs were assigned + assignedIPs := svc.GetAssignedIPConfigs() + assert.Empty(t, assignedIPs) + + // Verify IPv4 IP is still available and ipv6 is not available as the programming failed + availableIPs := svc.GetAvailableIPConfigs() + assert.Len(t, availableIPs, 1) +} + func createAndSaveMockNCRequest(t *testing.T, svc *HTTPRestService, ncID string, orchestratorContext json.RawMessage, desiredIP, mockGatewayIP, mockMACAddress string) { t.Helper() diff --git a/cns/restserver/util.go b/cns/restserver/util.go index 43d1e1aef9..a84eb8cef0 100644 --- a/cns/restserver/util.go +++ b/cns/restserver/util.go @@ -845,6 +845,7 @@ func (service *HTTPRestService) populateIPConfigInfoUntransacted(ipConfigStatus podIPInfo.HostPrimaryIPInfo.PrimaryIP = primaryHostInterface.PrimaryIP podIPInfo.HostPrimaryIPInfo.Subnet = primaryHostInterface.Subnet podIPInfo.HostPrimaryIPInfo.Gateway = primaryHostInterface.Gateway + podIPInfo.MacAddress = ncStatus.CreateNetworkContainerRequest.NetworkInterfaceInfo.MACAddress podIPInfo.NICType = cns.InfraNIC return nil diff --git a/cns/service/main.go b/cns/service/main.go index df1fc67453..f3f86f76dd 100644 --- a/cns/service/main.go +++ b/cns/service/main.go @@ -1311,7 +1311,13 @@ type ipamStateReconciler interface { // TODO(rbtr) where should this live?? // reconcileInitialCNSState initializes cns by passing pods and a CreateNetworkContainerRequest -func reconcileInitialCNSState(ctx context.Context, cli nodeNetworkConfigGetter, ipamReconciler ipamStateReconciler, podInfoByIPProvider cns.PodInfoByIPProvider) error { +func reconcileInitialCNSState( + ctx context.Context, + cli nodeNetworkConfigGetter, + ipamReconciler ipamStateReconciler, + podInfoByIPProvider cns.PodInfoByIPProvider, + config *configuration.CNSConfig, +) error { // Get nnc using direct client nnc, err := cli.Get(ctx) if err != nil { @@ -1350,7 +1356,7 @@ func reconcileInitialCNSState(ctx context.Context, cli nodeNetworkConfigGetter, ) switch nnc.Status.NetworkContainers[i].AssignmentMode { //nolint:exhaustive // skipping dynamic case case v1alpha.Static: - ncRequest, err = nncctrl.CreateNCRequestFromStaticNC(nnc.Status.NetworkContainers[i]) + ncRequest, err = nncctrl.CreateNCRequestFromStaticNC(nnc.Status.NetworkContainers[i], config) default: // For backward compatibility, default will be treated as Dynamic too. ncRequest, err = nncctrl.CreateNCRequestFromDynamicNC(nnc.Status.NetworkContainers[i]) } @@ -1458,7 +1464,7 @@ func InitializeCRDState(ctx context.Context, z *zap.Logger, httpRestService cns. _ = retry.Do(func() error { attempt++ logger.Printf("reconciling initial CNS state attempt: %d", attempt) - err = reconcileInitialCNSState(ctx, directscopedcli, httpRestServiceImplementation, podInfoByIPProvider) + err = reconcileInitialCNSState(ctx, directscopedcli, httpRestServiceImplementation, podInfoByIPProvider, cnsconfig) if err != nil { logger.Errorf("failed to reconcile initial CNS state, attempt: %d err: %v", attempt, err) nncInitFailure.Inc() @@ -1555,7 +1561,7 @@ func InitializeCRDState(ctx context.Context, z *zap.Logger, httpRestService cns. // get CNS Node IP to compare NC Node IP with this Node IP to ensure NCs were created for this node nodeIP := configuration.NodeIP() - nncReconciler := nncctrl.NewReconciler(httpRestServiceImplementation, poolMonitor, nodeIP) + nncReconciler := nncctrl.NewReconciler(httpRestServiceImplementation, poolMonitor, nodeIP, cnsconfig) // pass Node to the Reconciler for Controller xref // IPAMv1 - reconcile only status changes (where generation doesn't change). // IPAMv2 - reconcile all updates.