fix: Add ILB for vmss masters (Azure#319)

* fix: Add ILB for vmss masters

* Add ut and e2e test

* Update parts/k8s/kubernetesmastercustomdata.yml

Co-Authored-By: ritazh <rita.z.zhang@gmail.com>

* Remove masterFirstAddr variable

parts/k8s/kubernetesmastercustomdata.yml

@@ -394,7 +394,7 @@ MASTER_ARTIFACTS_CONFIG_PLACEHOLDER
   {{if IsMasterVirtualMachineScaleSets}}
     MASTER_VM_NAME=$(hostname)
     MASTER_VM_NAME_BASE=$(hostname | sed "s/.$//")
-    MASTER_FIRSTADDR={{WrapAsVariable "kubernetesAPIServerIP"}}
+    MASTER_FIRSTADDR={{WrapAsParameter "firstConsecutiveStaticIP"}}
     MASTER_INDEX=$(hostname | tail -c 2)
     PRIVATE_IP=$(hostname -I | cut -d" " -f1)
     MASTER_COUNT={{WrapAsVariable "masterCount"}}

parts/k8s/kubernetesmasterresourcesvmss.t

@@ -326,6 +326,75 @@
         "name": "[variables('loadBalancerSku')]"
     }
 },
+{{if gt .MasterProfile.Count 1}}
+    {
+      "apiVersion": "[variables('apiVersionNetwork')]",
+      "dependsOn": [
+{{if .MasterProfile.IsCustomVNET}}
+        "[variables('nsgID')]"
+{{else}}
+        "[variables('vnetID')]"
+{{end}}
+      ],
+      "location": "[variables('location')]",
+      "name": "[variables('masterInternalLbName')]",
+      "properties": {
+        "backendAddressPools": [
+          {
+            "name": "[variables('masterLbBackendPoolName')]"
+          }
+        ],
+        "frontendIPConfigurations": [
+          {
+            "name": "[variables('masterInternalLbIPConfigName')]",
+            "properties": {
+              "privateIPAddress": "[variables('kubernetesAPIServerIP')]",
+              "privateIPAllocationMethod": "Static",
+              "subnet": {
+                "id": "[variables('vnetSubnetIDMaster')]"
+              }
+            }
+          }
+        ],
+        "loadBalancingRules": [
+          {
+            "name": "InternalLBRuleHTTPS",
+            "properties": {
+              "backendAddressPool": {
+                "id": "[concat(variables('masterInternalLbID'), '/backendAddressPools/', variables('masterLbBackendPoolName'))]"
+              },
+              "backendPort": 4443,
+              "enableFloatingIP": false,
+              "frontendIPConfiguration": {
+                "id": "[variables('masterInternalLbIPConfigID')]"
+              },
+              "frontendPort": 443,
+              "idleTimeoutInMinutes": 5,
+              "protocol": "Tcp",
+              "probe": {
+                "id": "[concat(variables('masterInternalLbID'),'/probes/tcpHTTPSProbe')]"
+              }
+            }
+          }
+        ],
+        "probes": [
+          {
+            "name": "tcpHTTPSProbe",
+            "properties": {
+              "intervalInSeconds": 5,
+              "numberOfProbes": 2,
+              "port": 4443,
+              "protocol": "Tcp"
+            }
+          }
+        ]
+      },
+      "sku": {
+        "name": "[variables('loadBalancerSku')]"
+      },
+      "type": "Microsoft.Network/loadBalancers"
+    },
+{{end}}
 {
     "apiVersion": "[variables('apiVersionCompute')]",
     "dependsOn": [
@@ -334,6 +403,9 @@
     {{else}}
       "[variables('vnetID')]"
     {{end}}
+    {{if gt .MasterProfile.Count 1}}
+        ,"[variables('masterInternalLbName')]"
+    {{end}}
     {{ if HasCosmosEtcd }}
       ,"[resourceId('Microsoft.DocumentDB/databaseAccounts/', variables('cosmosAccountName'))]"
     {{ end }}
@@ -395,6 +467,11 @@
                         {
                           "id": "[concat(variables('masterLbID'), '/backendAddressPools/', variables('masterLbBackendPoolName'))]"
                         }
+                        {{if gt $.MasterProfile.Count 1}}
+                          ,{
+                            "id": "[concat(variables('masterInternalLbID'), '/backendAddressPools/', variables('masterLbBackendPoolName'))]"
+                          }
+                        {{end}}
                       ],
                       "loadBalancerInboundNatPools": [
                         {

parts/k8s/kubernetesmastervars.t

@@ -244,7 +244,7 @@
         "masterInternalLbIPConfigID": "[concat(variables('masterInternalLbID'),'/frontendIPConfigurations/', variables('masterInternalLbIPConfigName'))]",
         "masterInternalLbIPOffset": {{GetDefaultInternalLbStaticIPOffset}},
         {{if IsMasterVirtualMachineScaleSets}}
-        "kubernetesAPIServerIP": "[parameters('firstConsecutiveStaticIP')]",
+        "kubernetesAPIServerIP": "[concat(variables('masterFirstAddrOctets')[0],'.',variables('masterFirstAddrOctets')[1],'.255.', variables('masterInternalLbIPOffset'))]",
         {{else}}
         "kubernetesAPIServerIP": "[concat(variables('masterFirstAddrPrefix'), add(variables('masterInternalLbIPOffset'), int(variables('masterFirstAddrOctet4'))))]",
         {{end}}

pkg/api/common/const.go

@@ -37,6 +37,8 @@ const (
 	MinIPAddressCount = 1
 	// MaxIPAddressCount specifies the maximum number of IP addresses per network interface
 	MaxIPAddressCount = 256
+	// address relative to the first consecutive Kubernetes static IP
+	DefaultInternalLbStaticIPOffset = 10
 )
 
 // Availability profiles

pkg/api/defaults.go

@@ -531,9 +531,14 @@ func (p *Properties) setDefaultCerts() (bool, []net.IP, error) {
 	}
 
 	ips := []net.IP{firstMasterIP, localhostIP}
-	// Add the Internal Loadbalancer IP which is always at at p known offset from the firstMasterIP
-	ips = append(ips, net.IP{firstMasterIP[0], firstMasterIP[1], firstMasterIP[2], firstMasterIP[3] + byte(DefaultInternalLbStaticIPOffset)})
+
 	// Include the Internal load balancer as well
+	if p.MasterProfile.IsVirtualMachineScaleSets() {
+		ips = append(ips, net.IP{firstMasterIP[0], firstMasterIP[1], byte(255), byte(DefaultInternalLbStaticIPOffset)})
+	} else {
+		// Add the Internal Loadbalancer IP which is always at p known offset from the firstMasterIP
+		ips = append(ips, net.IP{firstMasterIP[0], firstMasterIP[1], firstMasterIP[2], firstMasterIP[3] + byte(DefaultInternalLbStaticIPOffset)})
+	}
 
 	var offsetMultiplier int
 	if p.MasterProfile.IsVirtualMachineScaleSets() {

pkg/api/defaults_test.go

@@ -1239,6 +1239,71 @@ func TestDefaultCloudProvider(t *testing.T) {
 	}
 }
 func TestSetCertDefaults(t *testing.T) {
+	cs := &ContainerService{
+		Properties: &Properties{
+			ServicePrincipalProfile: &ServicePrincipalProfile{
+				ClientID: "barClientID",
+				Secret:   "bazSecret",
+			},
+			MasterProfile: &MasterProfile{
+				Count:     3,
+				DNSPrefix: "myprefix1",
+				VMSize:    "Standard_DS2_v2",
+			},
+			OrchestratorProfile: &OrchestratorProfile{
+				OrchestratorType:    Kubernetes,
+				OrchestratorVersion: "1.10.2",
+				KubernetesConfig: &KubernetesConfig{
+					NetworkPlugin: "azure",
+				},
+			},
+		},
+	}
+
+	cs.setOrchestratorDefaults(false)
+	cs.Properties.setMasterProfileDefaults(false)
+	result, ips, err := cs.Properties.setDefaultCerts()
+
+	if !result {
+		t.Error("expected setDefaultCerts to return true")
+	}
+
+	if err != nil {
+		t.Errorf("unexpected error thrown while executing setDefaultCerts %s", err.Error())
+	}
+
+	if ips == nil {
+		t.Error("expected setDefaultCerts to create a list of IPs")
+	} else {
+
+		if len(ips) != cs.Properties.MasterProfile.Count+3 {
+			t.Errorf("expected length of IPs from setDefaultCerts %d, actual length %d", cs.Properties.MasterProfile.Count+3, len(ips))
+		}
+
+		firstMasterIP := net.ParseIP(cs.Properties.MasterProfile.FirstConsecutiveStaticIP).To4()
+		offsetMultiplier := 1
+		addr := binary.BigEndian.Uint32(firstMasterIP)
+		expectedNewAddr := getNewAddr(addr, cs.Properties.MasterProfile.Count-1, offsetMultiplier)
+		actualLastIPAddr := binary.BigEndian.Uint32(ips[len(ips)-2])
+		if actualLastIPAddr != expectedNewAddr {
+			expectedLastIP := make(net.IP, 4)
+			binary.BigEndian.PutUint32(expectedLastIP, expectedNewAddr)
+			t.Errorf("expected last IP of master vm from setDefaultCerts %d, actual %d", expectedLastIP, ips[len(ips)-2])
+		}
+
+		if cs.Properties.MasterProfile.Count > 1 {
+			expectedILBIP := net.IP{firstMasterIP[0], firstMasterIP[1], firstMasterIP[2], firstMasterIP[3] + byte(DefaultInternalLbStaticIPOffset)}
+			actualILBIPAddr := binary.BigEndian.Uint32(ips[2])
+			expectedILBIPAddr := binary.BigEndian.Uint32(expectedILBIP)
+
+			if actualILBIPAddr != expectedILBIPAddr {
+				t.Errorf("expected IP of master ILB from setDefaultCerts %d, actual %d", expectedILBIP, ips[2])
+			}
+		}
+	}
+}
+
+func TestSetCertDefaultsVMSS(t *testing.T) {
 	cs := &ContainerService{
 		Properties: &Properties{
 			ServicePrincipalProfile: &ServicePrincipalProfile{
@@ -1282,12 +1347,7 @@ func TestSetCertDefaults(t *testing.T) {
 		}
 
 		firstMasterIP := net.ParseIP(cs.Properties.MasterProfile.FirstConsecutiveStaticIP).To4()
-		var offsetMultiplier int
-		if cs.Properties.MasterProfile.IsVirtualMachineScaleSets() {
-			offsetMultiplier = cs.Properties.MasterProfile.IPAddressCount
-		} else {
-			offsetMultiplier = 1
-		}
+		offsetMultiplier := cs.Properties.MasterProfile.IPAddressCount
 		addr := binary.BigEndian.Uint32(firstMasterIP)
 		expectedNewAddr := getNewAddr(addr, cs.Properties.MasterProfile.Count-1, offsetMultiplier)
 		actualLastIPAddr := binary.BigEndian.Uint32(ips[len(ips)-2])
@@ -1296,8 +1356,17 @@ func TestSetCertDefaults(t *testing.T) {
 			binary.BigEndian.PutUint32(expectedLastIP, expectedNewAddr)
 			t.Errorf("expected last IP of master vm from setDefaultCerts %d, actual %d", expectedLastIP, ips[len(ips)-2])
 		}
-	}
 
+		if cs.Properties.MasterProfile.Count > 1 {
+			expectedILBIP := net.IP{firstMasterIP[0], firstMasterIP[1], byte(255), byte(DefaultInternalLbStaticIPOffset)}
+			actualILBIPAddr := binary.BigEndian.Uint32(ips[2])
+			expectedILBIPAddr := binary.BigEndian.Uint32(expectedILBIP)
+
+			if actualILBIPAddr != expectedILBIPAddr {
+				t.Errorf("expected IP of master ILB from setDefaultCerts %d, actual %d", expectedILBIP, ips[2])
+			}
+		}
+	}
 }
 
 func getMockBaseContainerService(orchestratorVersion string) ContainerService {

test/e2e/kubernetes/kubernetes_test.go

@@ -7,6 +7,7 @@ import (
 	"fmt"
 	"log"
 	"math/rand"
+	"net"
 	"os"
 	"os/exec"
 	"path/filepath"
@@ -193,6 +194,25 @@ var _ = Describe("Azure Container Cluster using the Kubernetes Orchestrator", fu
 			}
 		})
 
+		It("should have the correct IP address for the apiserver", func() {
+			pods, err := pod.GetAllByPrefix("kube-apiserver", "kube-system")
+			Expect(err).NotTo(HaveOccurred())
+			By("Ensuring that the correct IP address has been applied to the apiserver")
+			expectedIPAddress := eng.ExpandedDefinition.Properties.MasterProfile.FirstConsecutiveStaticIP
+			if eng.ExpandedDefinition.Properties.MasterProfile.Count > 1 {
+				firstMasterIP := net.ParseIP(eng.ExpandedDefinition.Properties.MasterProfile.FirstConsecutiveStaticIP).To4()
+				expectedIP := net.IP{firstMasterIP[0], firstMasterIP[1], firstMasterIP[2], firstMasterIP[3] + byte(common.DefaultInternalLbStaticIPOffset)}
+				if eng.ExpandedDefinition.Properties.MasterProfile.IsVirtualMachineScaleSets() {
+					expectedIP = net.IP{firstMasterIP[0], firstMasterIP[1], byte(255), byte(common.DefaultInternalLbStaticIPOffset)}
+				}
+				expectedIPAddress = expectedIP.String()
+			}
+
+			actualIPAddress, err := pods[0].Spec.Containers[0].GetArg("--advertise-address")
+			Expect(err).NotTo(HaveOccurred())
+			Expect(actualIPAddress).To(Equal(expectedIPAddress))
+		})
+
 		It("should have addons running", func() {
 			for _, addonName := range []string{"tiller", "aci-connector", "cluster-autoscaler", "blobfuse-flexvolume", "smb-flexvolume", "keyvault-flexvolume", "kubernetes-dashboard", "rescheduler", "metrics-server", "nvidia-device-plugin", "container-monitoring", "azure-cni-networkmonitor", "azure-npm-daemonset", "ip-masq-agent"} {
 				var addonPods = []string{addonName}
@@ -622,7 +642,7 @@ var _ = Describe("Azure Container Cluster using the Kubernetes Orchestrator", fu
 					if i > 28 {
 						log.Printf("Error while running kubectl top nodes:%s\n", err)
 						pods, _ := pod.GetAllByPrefix("metrics-server", "kube-system")
-						if pods != nil {
+						if len(pods) != 0 {
 							for _, p := range pods {
 								p.Logs()
 							}

test/e2e/kubernetes/pod/pod.go

@@ -60,6 +60,7 @@ type Container struct {
 	Env       []EnvVar  `json:"env"`
 	Resources Resources `json:"resources"`
 	Name      string    `json:"name"`
+	Args      []string  `json:"args"`
 }
 
 // TerminatedContainerState shows terminated state of a container
@@ -494,7 +495,7 @@ func WaitOnReady(podPrefix, namespace string, successesNeeded int, sleep, durati
 		select {
 		case err := <-errCh:
 			pods, _ := GetAllByPrefix(podPrefix, namespace)
-			if pods != nil {
+			if len(pods) != 0 {
 				for _, p := range pods {
 					e := p.Logs()
 					if e != nil {
@@ -906,6 +907,17 @@ func (c *Container) GetEnvironmentVariable(varName string) (string, error) {
 	return "", errors.New("environment variable not found")
 }
 
+// GetArg returns an arg's value from a container within a pod
+func (c *Container) GetArg(argKey string) (string, error) {
+	for _, argvar := range c.Args {
+		if strings.Contains(argvar, argKey) {
+			value := strings.SplitAfter(argvar, "=")[1]
+			return value, nil
+		}
+	}
+	return "", errors.New("container argument not found")
+}
+
 // getCPURequests returns an the CPU Requests value from a container within a pod
 func (c *Container) getCPURequests() string {
 	return c.Resources.Requests.CPU