From d4326f34854a21a631cb651352208b19c599a034 Mon Sep 17 00:00:00 2001 From: Rita Zhang Date: Sat, 19 Jan 2019 09:06:27 -0800 Subject: [PATCH] fix: Add ILB for vmss masters (#319) * fix: Add ILB for vmss masters * Add ut and e2e test * Update parts/k8s/kubernetesmastercustomdata.yml Co-Authored-By: ritazh * Remove masterFirstAddr variable --- parts/k8s/kubernetesmastercustomdata.yml | 2 +- parts/k8s/kubernetesmasterresourcesvmss.t | 77 +++++++++++++++++++++ parts/k8s/kubernetesmastervars.t | 2 +- pkg/api/common/const.go | 2 + pkg/api/defaults.go | 9 ++- pkg/api/defaults_test.go | 83 +++++++++++++++++++++-- test/e2e/kubernetes/kubernetes_test.go | 22 +++++- test/e2e/kubernetes/pod/pod.go | 14 +++- 8 files changed, 198 insertions(+), 13 deletions(-) diff --git a/parts/k8s/kubernetesmastercustomdata.yml b/parts/k8s/kubernetesmastercustomdata.yml index 03ece697623..86200f057f1 100644 --- a/parts/k8s/kubernetesmastercustomdata.yml +++ b/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"}} diff --git a/parts/k8s/kubernetesmasterresourcesvmss.t b/parts/k8s/kubernetesmasterresourcesvmss.t index 9a60e33ff89..9107858c7d8 100644 --- a/parts/k8s/kubernetesmasterresourcesvmss.t +++ b/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": [ { diff --git a/parts/k8s/kubernetesmastervars.t b/parts/k8s/kubernetesmastervars.t index 882b42e4832..2ed5b4b914e 100644 --- a/parts/k8s/kubernetesmastervars.t +++ b/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}} diff --git a/pkg/api/common/const.go b/pkg/api/common/const.go index fc44c36330b..0cdf8714a55 100644 --- a/pkg/api/common/const.go +++ b/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 diff --git a/pkg/api/defaults.go b/pkg/api/defaults.go index a87f7b6d430..99be013fd54 100644 --- a/pkg/api/defaults.go +++ b/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() { diff --git a/pkg/api/defaults_test.go b/pkg/api/defaults_test.go index 759ca3a6809..4fe5d162d89 100644 --- a/pkg/api/defaults_test.go +++ b/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 { diff --git a/test/e2e/kubernetes/kubernetes_test.go b/test/e2e/kubernetes/kubernetes_test.go index 7b33c34619f..d76a2f1f75c 100644 --- a/test/e2e/kubernetes/kubernetes_test.go +++ b/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() } diff --git a/test/e2e/kubernetes/pod/pod.go b/test/e2e/kubernetes/pod/pod.go index 0cda8cfa145..39adcc6063d 100644 --- a/test/e2e/kubernetes/pod/pod.go +++ b/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