Revert static IP allocation logic in Azure CNI, PR 1966. (#2315)

(cherry picked from commit 47f2a4f)

docs/kubernetes/features.md

@@ -169,15 +169,29 @@ Per default Calico still allows all communication within the cluster. Using Kube
 
 *Note: Custom VNET for Kubernetes Windows cluster has a [known issue](https://github.com/Azure/acs-engine/issues/1767).*
 
-ACS Engine supports deploying into an existing VNET. Operators must specify the ARM path/id of Subnets for the `masterProfile` and  any `agentPoolProfiles`, as well as the first IP address to use for static IP allocation in `firstConsecutiveStaticIP`. Please note that in any azure subnet, the first four and the last ip address is reserved and can not be used. Additionally, each POD now gets the IP address from the Subnet. As a result, for the master nodes, enough IP addresses (equal to `ipAddressCount`) should be available beyond `firstConsecutiveStaticIP`. By default, the `ipAddressCount` has a value of 30, and can be changed if desired. Furthermore, to prevent source address NAT'ing within the VNET, we assign to the `vnetCidr` property in `masterProfile` the CIDR block that represents the usable address space in the existing VNET.
+ACS Engine supports deploying into an existing VNET. Operators must specify the ARM path/id of Subnets for the `masterProfile` and  any `agentPoolProfiles`, as well as the first IP address to use for IP static IP allocation in `firstConsecutiveStaticIP`. Additionally, to prevent source address NAT'ing within the VNET, we assign to the `vnetCidr` property in `masterProfile` the CIDR block that represents the usable address space in the existing VNET.
 
-See below profiles as an example:
+Depending upon the size of the VNET address space, during deployment, it is possible to experience IP address assignment collision between the required Kubernetes static IPs (one each per master and one for the API server load balancer, if more than one masters) and Azure CNI-assigned dynamic IPs (one for each NIC on the agent nodes). In practice, the larger the VNET the less likely this is to happen; some detail, and then a guideline.
+
+First, the detail:
+
+* Azure CNI assigns dynamic IP addresses from the "beginning" of the subnet IP address space (specifically, it looks for available addresses starting at ".4" ["10.0.0.4" in a "10.0.0.0/24" network])
+* acs-engine will require a range of up to 16 unused IP addresses in multi-master scenarios (1 per master for up to 5 masters, and then the next 10 IP addresses immediately following the "last" master for headroom reservation, and finally 1 more for the load balancer immediately adjacent to the afore-described _n_ masters+10 sequence) to successfully scaffold the network stack for your cluster
+
+A guideline that will remove the danger of IP address allocation collision during deployment:
+
+* If possible, assign to the `firstConsecutiveStaticIP` configuration property an IP address that is near the "end" of the available IP address space in the desired  subnet.
+  * For example, if the desired subnet is a `/24`, choose the "239" address in that network space
+
+In larger subnets (e.g., `/16`) it's not as practically useful to push static IP assignment to the very "end" of large subnet, but as long as it's not in the "first" `/24` (for example) your deployment will be resilient to this edge case behavior.
+
+Before provisioning, modify the `masterProfile` and `agentPoolProfiles` to match the above requirements, with the below being a representative example:
 
 ```json
 "masterProfile": {
   ...
   "vnetSubnetId": "/subscriptions/SUB_ID/resourceGroups/RG_NAME/providers/Microsoft.Network/virtualNetworks/VNET_NAME/subnets/MASTER_SUBNET_NAME",
-  "firstConsecutiveStaticIP": "10.239.0.5",
+  "firstConsecutiveStaticIP": "10.239.255.239",
   "vnetCidr": "10.239.0.0/16",
   ...
 },

examples/e2e-tests/kubernetes/release/default/definition.json

@@ -22,7 +22,7 @@
       "vmSize": "Standard_D2_v2",
       "OSDiskSizeGB": 200,
       "vnetSubnetId": "/subscriptions/SUB_ID/resourceGroups/RG_NAME/providers/Microsoft.Network/virtualNetworks/VNET_NAME/subnets/SUBNET_NAME",
-      "firstConsecutiveStaticIP": "10.239.0.4",
+      "firstConsecutiveStaticIP": "10.239.255.239",
       "vnetCidr": "10.239.0.0/16"
     },
     "agentPoolProfiles": [

examples/vnet/kubernetesvnet-azure-cni.json

@@ -9,7 +9,7 @@
       "dnsPrefix": "test",
       "vmSize": "Standard_D2_v2",
       "vnetSubnetId": "/subscriptions/SUB_ID/resourceGroups/RG_NAME/providers/Microsoft.Network/virtualNetworks/VNET_NAME/subnets/SUBNET_NAME",
-      "firstConsecutiveStaticIP": "10.239.255.10",
+      "firstConsecutiveStaticIP": "10.239.255.239",
       "vnetCidr": "10.239.0.0/16"
     },
     "agentPoolProfiles": [

parts/k8s/kubernetesagentresourcesvmas.t

@@ -9,9 +9,6 @@
       "[variables('nsgID')]"
 {{else}}
       "[variables('vnetID')]"
-{{end}}
-{{range $masterOffset := loop 1 GetMasterCount}}
-      ,"[concat(variables('masterNICNamePrefix'), sub({{$masterOffset}}, 1))]"
 {{end}}
       ],
       "location": "[variables('location')]",

parts/k8s/kubernetesagentvars.t

@@ -24,5 +24,4 @@
     "{{.Name}}osImageSKU": "[parameters('{{.Name}}osImageSKU')]",
     "{{.Name}}osImagePublisher": "[parameters('{{.Name}}osImagePublisher')]",
     "{{.Name}}osImageVersion": "[parameters('{{.Name}}osImageVersion')]",
-    "masterNICNamePrefix": "[concat(variables('orchestratorName'), '-master-', variables('nameSuffix'), '-', 'nic-')]",
 

parts/k8s/kubernetesmasterresources.t

@@ -338,14 +338,13 @@
             }
           }
 {{if IsAzureCNI}}
-          {{range $seq := loop 1 .MasterProfile.IPAddressCount}}
+          {{range $seq := loop 2 .MasterProfile.IPAddressCount}}
           ,
           {
-            "name": "[concat('ipconfig', add({{$seq}}, 1))]",
+            "name": "ipconfig{{$seq}}",
             "properties": {
-              "privateIPAddress": "[variables('masterSecondaryAddrs')[add(mul(copyIndex(variables('masterOffset')), variables('ipAddressCount')), sub({{$seq}}, 1))]]",
               "primary": false,
-              "privateIPAllocationMethod": "Static",
+              "privateIPAllocationMethod": "Dynamic",
               "subnet": {
                 "id": "[variables('vnetSubnetID')]"
               }

parts/k8s/kubernetesmastervars.t

@@ -217,7 +217,6 @@
     "orchestratorNameVersionTag": "{{.OrchestratorProfile.OrchestratorType}}:{{.OrchestratorProfile.OrchestratorVersion}}",
 {{if IsAzureCNI}}
     "allocateNodeCidrs": false,
-    "ipAddressCount": {{.MasterProfile.IPAddressCount}},
 {{else}}
     "allocateNodeCidrs": true,
 {{end}}
@@ -304,9 +303,6 @@
       "[concat(variables('masterFirstAddrPrefix'), add(3, int(variables('masterFirstAddrOctet4'))))]",
       "[concat(variables('masterFirstAddrPrefix'), add(4, int(variables('masterFirstAddrOctet4'))))]"
     ],
-{{if IsAzureCNI}}
-    "masterSecondaryAddrs": [{{GetMasterSecondaryIP}}],
-{{end}}
     "masterEtcdServerPort": {{GetMasterEtcdServerPort}},
     "masterEtcdClientPort": {{GetMasterEtcdClientPort}},
     "masterEtcdPeerURLs":[

parts/k8s/kuberneteswinagentresourcesvmas.t

@@ -9,9 +9,6 @@
       "[variables('nsgID')]"
 {{else}}
       "[variables('vnetID')]"
-{{end}}
-{{range $masterOffset := loop 1 GetMasterCount}}
-      ,"[concat(variables('masterNICNamePrefix'), sub({{$masterOffset}}, 1))]"
 {{end}}
       ],
       "location": "[variables('location')]",

pkg/acsengine/const.go

@@ -18,7 +18,7 @@ const (
 	// DefaultNonMasqueradeCidr specifies the subnet that should not be masqueraded on host
 	DefaultNonMasqueradeCidr = "10.0.0.0/8"
 	// DefaultFirstConsecutiveKubernetesStaticIP specifies the static IP address on Kubernetes master 0
-	DefaultFirstConsecutiveKubernetesStaticIP = "10.240.0.4"
+	DefaultFirstConsecutiveKubernetesStaticIP = "10.240.255.5"
 	// DefaultAgentSubnetTemplate specifies a default agent subnet
 	DefaultAgentSubnetTemplate = "10.%d.0.0/16"
 	// DefaultKubernetesSubnet specifies the default subnet used for all masters, agents and pods

pkg/acsengine/defaults.go

@@ -2,7 +2,6 @@ package acsengine
 
 import (
 	"bytes"
-	"encoding/binary"
 	"fmt"
 	"net"
 	"sort"
@@ -488,6 +487,9 @@ func setMasterNetworkDefaults(a *api.Properties) {
 
 	// Set the default number of IP addresses allocated for masters.
 	if a.MasterProfile.IPAddressCount == 0 {
+		// Allocate one IP address for the node.
+		a.MasterProfile.IPAddressCount = 1
+
 		// Allocate IP addresses for pods if VNET integration is enabled.
 		if a.OrchestratorProfile.IsAzureCNI() {
 			if a.OrchestratorProfile.OrchestratorType == api.Kubernetes {
@@ -685,23 +687,27 @@ func certsAlreadyPresent(c *api.CertificateProfile, m int) map[string]bool {
 
 // getFirstConsecutiveStaticIPAddress returns the first static IP address of the given subnet.
 func getFirstConsecutiveStaticIPAddress(subnetStr string) string {
-	ip, _, err := net.ParseCIDR(subnetStr)
+	_, subnet, err := net.ParseCIDR(subnetStr)
 	if err != nil {
 		return DefaultFirstConsecutiveKubernetesStaticIP
 	}
 
-	ipv4 := ip.To4()
-	if ipv4 == nil {
-		return DefaultFirstConsecutiveKubernetesStaticIP
-	}
+	// Find the first and last octet of the host bits.
+	ones, bits := subnet.Mask.Size()
+	firstOctet := ones / 8
+	lastOctet := bits/8 - 1
 
-	ipint := binary.BigEndian.Uint32(ipv4)
+	// Set the remaining host bits in the first octet.
+	subnet.IP[firstOctet] |= (1 << byte((8 - (ones % 8)))) - 1
+
+	// Fill the intermediate octets with 1s and last octet with offset. This is done so to match
+	// the existing behavior of allocating static IP addresses from the last /24 of the subnet.
+	for i := firstOctet + 1; i < lastOctet; i++ {
+		subnet.IP[i] = 255
+	}
+	subnet.IP[lastOctet] = DefaultKubernetesFirstConsecutiveStaticIPOffset
 
-	// First 4 IPs will be reserved in Azure and cannot use.
-	ipint += 4
-	startIP := make(net.IP, 4)
-	binary.BigEndian.PutUint32(startIP, ipint)
-	return startIP.String()
+	return subnet.IP.String()
 }
 
 func getAddonsIndexByName(addons []api.KubernetesAddon, name string) int {

pkg/acsengine/engine.go

@@ -4,15 +4,13 @@ import (
 	"bytes"
 	"compress/gzip"
 	"encoding/base64"
-	"encoding/binary"
 	"encoding/json"
 	"errors"
 	"fmt"
 	"hash/fnv"
 	"io/ioutil"
 	"log"
 	"math/rand"
-	"net"
 	"net/http"
 	"regexp"
 	"runtime/debug"
@@ -876,62 +874,6 @@ func (t *TemplateGenerator) getTemplateFuncMap(cs *api.ContainerService) templat
 			}
 			return strings.TrimSuffix(buf.String(), ", ")
 		},
-		"GetMasterCount": func() int {
-			masterProfile := cs.Properties.MasterProfile
-			if masterProfile == nil {
-				return 0
-			}
-			return masterProfile.Count
-		},
-		"GetMasterSecondaryIP": func() string {
-			var ips string
-			var ipint uint32
-
-			profile := cs.Properties.MasterProfile
-			if profile == nil {
-				return ""
-			}
-
-			ip := net.ParseIP(profile.FirstConsecutiveStaticIP)
-			ipv4 := ip.To4()
-			if ipv4 != nil {
-				ipint = binary.BigEndian.Uint32(ipv4)
-			} else {
-				log.Fatalf("Net IP To4() function returns nil")
-			}
-
-			// Make space for first few IPs.
-			ipint += uint32(profile.Count) + uint32(DefaultInternalLbStaticIPOffset) - 1
-			startIP := make(net.IP, 4)
-			binary.BigEndian.PutUint32(startIP, ipint)
-
-			totalIPCount := profile.Count * profile.IPAddressCount
-			ipint = 0
-
-			// Generate All secondary IPs that master will use.
-			for count := totalIPCount; count > 0; count-- {
-				ipv4 = startIP.To4()
-				if ipv4 != nil {
-					ipint = binary.BigEndian.Uint32(ipv4)
-				} else {
-					log.Fatalf("Net IP To4() function returns nil for startIP")
-				}
-
-				ipint++
-				newIP := make(net.IP, 4)
-				binary.BigEndian.PutUint32(newIP, ipint)
-
-				if ips != "" {
-					ips = ips + "," + "\"" + newIP.String() + "\""
-				} else {
-					ips = "\"" + newIP.String() + "\""
-				}
-
-				startIP = newIP
-			}
-
-			return ips
-		},
 		"RequiresFakeAgentOutput": func() bool {
 			return cs.Properties.OrchestratorProfile.OrchestratorType == api.Kubernetes
 		},