Improving IP address assignment for master nodes with Azure CNI. (#1966)

tamilmani1989 · jackfrancis · commit 767932c98562 · 2018-01-17T08:28:58.000-08:00
* Azure cni static ip change (#1) * VSTS#1828538 Modified master IPs from dynamic to static and made agent nic dependent on master nic * Modified firstconsecutive static IP for default vnet * updated documentation specific to Azure CNI * Fixed styling test errors * Handled no master scenario * updated one of the examples cluster config to use firstConsecutiveStaticIp from the start of subnet * removed azureCNI check so that agent nic will always depend on master nic * Specified dependency of agent nic on master nic in windows agents template * moving firstConsecutiveStaticIP away from the edge of usable address space * Update documentation to help customers how to specify firstConsecutiveStaticIP and ipAddressCount for master nodes.
diff --git a/docs/kubernetes/features.md b/docs/kubernetes/features.md
@@ -167,29 +167,15 @@ Per default Calico still allows all communication within the cluster. Using Kube
 
 ## Custom 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.
+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.
 
-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:
+See below profiles as an example:
 
 ```json
 "masterProfile": {
   ...
   "vnetSubnetId": "/subscriptions/SUB_ID/resourceGroups/RG_NAME/providers/Microsoft.Network/virtualNetworks/VNET_NAME/subnets/MASTER_SUBNET_NAME",
-  "firstConsecutiveStaticIP": "10.239.255.239",
+  "firstConsecutiveStaticIP": "10.239.0.5",
   "vnetCidr": "10.239.0.0/16",
   ...
 },
diff --git a/examples/e2e-tests/kubernetes/release/default/definition.json b/examples/e2e-tests/kubernetes/release/default/definition.json
@@ -13,7 +13,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.255.239",
+      "firstConsecutiveStaticIP": "10.239.0.4",
       "vnetCidr": "10.239.0.0/16"
     },
     "agentPoolProfiles": [
diff --git a/examples/vnet/kubernetesvnet-azure-cni.json b/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.239",
+      "firstConsecutiveStaticIP": "10.239.255.10",
       "vnetCidr": "10.239.0.0/16"
     },
     "agentPoolProfiles": [
diff --git a/parts/k8s/kubernetesagentresourcesvmas.t b/parts/k8s/kubernetesagentresourcesvmas.t
@@ -9,6 +9,9 @@
       "[variables('nsgID')]"
 {{else}}
       "[variables('vnetID')]"
+{{end}}
+{{range $masterOffset := loop 1 GetMasterCount}}
+      ,"[concat(variables('masterNICNamePrefix'), sub({{$masterOffset}}, 1))]"
 {{end}}
       ],
       "location": "[variables('location')]",
diff --git a/parts/k8s/kubernetesagentvars.t b/parts/k8s/kubernetesagentvars.t
@@ -24,4 +24,5 @@
     "{{.Name}}osImageSKU": "[parameters('{{.Name}}osImageSKU')]",
     "{{.Name}}osImagePublisher": "[parameters('{{.Name}}osImagePublisher')]",
     "{{.Name}}osImageVersion": "[parameters('{{.Name}}osImageVersion')]",
+    "masterNICNamePrefix": "[concat(variables('orchestratorName'), '-master-', variables('nameSuffix'), '-', 'nic-')]",
 
diff --git a/parts/k8s/kubernetesmasterresources.t b/parts/k8s/kubernetesmasterresources.t
@@ -338,13 +338,14 @@
             }
           }
 {{if IsAzureCNI}}
-          {{range $seq := loop 2 .MasterProfile.IPAddressCount}}
+          {{range $seq := loop 1 .MasterProfile.IPAddressCount}}
           ,
           {
-            "name": "ipconfig{{$seq}}",
+            "name": "[concat('ipconfig', add({{$seq}}, 1))]",
             "properties": {
+              "privateIPAddress": "[variables('masterSecondaryAddrs')[add(mul(copyIndex(variables('masterOffset')), variables('ipAddressCount')), sub({{$seq}}, 1))]]",
               "primary": false,
-              "privateIPAllocationMethod": "Dynamic",
+              "privateIPAllocationMethod": "Static",
               "subnet": {
                 "id": "[variables('vnetSubnetID')]"
               }
diff --git a/parts/k8s/kubernetesmastervars.t b/parts/k8s/kubernetesmastervars.t
@@ -181,6 +181,7 @@
     "orchestratorNameVersionTag": "{{.OrchestratorProfile.OrchestratorType}}:{{.OrchestratorProfile.OrchestratorVersion}}",
 {{if IsAzureCNI}}
     "allocateNodeCidrs": false,
+    "ipAddressCount": {{.MasterProfile.IPAddressCount}},
 {{else}}
     "allocateNodeCidrs": true,
 {{end}}
@@ -267,6 +268,9 @@
       "[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":[
diff --git a/parts/k8s/kuberneteswinagentresourcesvmas.t b/parts/k8s/kuberneteswinagentresourcesvmas.t
@@ -9,6 +9,9 @@
       "[variables('nsgID')]"
 {{else}}
       "[variables('vnetID')]"
+{{end}}
+{{range $masterOffset := loop 1 GetMasterCount}}
+      ,"[concat(variables('masterNICNamePrefix'), sub({{$masterOffset}}, 1))]"
 {{end}}
       ],
       "location": "[variables('location')]",
diff --git a/pkg/acsengine/const.go b/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.255.5"
+	DefaultFirstConsecutiveKubernetesStaticIP = "10.240.0.4"
 	// DefaultAgentSubnetTemplate specifies a default agent subnet
 	DefaultAgentSubnetTemplate = "10.%d.0.0/16"
 	// DefaultKubernetesSubnet specifies the default subnet used for all masters, agents and pods
diff --git a/pkg/acsengine/defaults.go b/pkg/acsengine/defaults.go
@@ -1,6 +1,7 @@
 package acsengine
 
 import (
+	"encoding/binary"
 	"fmt"
 	"net"
 
@@ -476,9 +477,6 @@ 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 {
@@ -676,27 +674,23 @@ 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 {
-	_, subnet, err := net.ParseCIDR(subnetStr)
+	ip, _, err := net.ParseCIDR(subnetStr)
 	if err != 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
-
-	// 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
+	ipv4 := ip.To4()
+	if ipv4 == nil {
+		return DefaultFirstConsecutiveKubernetesStaticIP
 	}
-	subnet.IP[lastOctet] = DefaultKubernetesFirstConsecutiveStaticIPOffset
 
-	return subnet.IP.String()
+	ipint := binary.BigEndian.Uint32(ipv4)
+
+	// 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()
 }
 
 func getAddonsIndexByName(addons []api.KubernetesAddon, name string) int {
diff --git a/pkg/acsengine/engine.go b/pkg/acsengine/engine.go
@@ -4,13 +4,15 @@ 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"
@@ -875,6 +877,62 @@ 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
 		},

Original file line number	Diff line number	Diff line change
`@@ -338,13 +338,14 @@`
`338`	`338`	`}`
`339`	`339`	`}`
`340`	`340`	`{{if IsAzureCNI}}`
`341`		`- {{range $seq := loop 2 .MasterProfile.IPAddressCount}}`
	`341`	`+ {{range $seq := loop 1 .MasterProfile.IPAddressCount}}`
`342`	`342`	`,`
`343`	`343`	`{`
`344`		`- "name": "ipconfig{{$seq}}",`
	`344`	`+ "name": "[concat('ipconfig', add({{$seq}}, 1))]",`
`345`	`345`	`"properties": {`
	`346`	`+ "privateIPAddress": "[variables('masterSecondaryAddrs')[add(mul(copyIndex(variables('masterOffset')), variables('ipAddressCount')), sub({{$seq}}, 1))]]",`
`346`	`347`	`"primary": false,`
`347`		`- "privateIPAllocationMethod": "Dynamic",`
	`348`	`+ "privateIPAllocationMethod": "Static",`
`348`	`349`	`"subnet": {`
`349`	`350`	`"id": "[variables('vnetSubnetID')]"`
`350`	`351`	`}`