Enable multiple Windows vmss agent pools - refactor pool names (#3907)

cmd/scale.go

@@ -323,7 +323,7 @@ func (sc *scaleCmd) run(cmd *cobra.Command, args []string) error {
 
 				osPublisher := vmss.VirtualMachineProfile.StorageProfile.ImageReference.Publisher
 				if osPublisher != nil && strings.EqualFold(*osPublisher, "MicrosoftWindowsServer") {
-					_, _, winPoolIndex, err = utils.WindowsVMSSNameParts(vmName)
+					_, _, winPoolIndex, _, err = utils.WindowsVMNameParts(vmName)
 					log.Errorln(err)
 				}
 

parts/k8s/kuberneteswinagentresourcesvmss.t

@@ -83,7 +83,7 @@
           ]
         },
         "osProfile": {
-          "computerNamePrefix": "[concat(substring(parameters('nameSuffix'), 0, 5), 'acs')]",
+          "computerNamePrefix": "[variables('{{.Name}}VMNamePrefix')]",
           {{GetKubernetesWindowsAgentCustomData .}}
           "adminUsername": "[parameters('windowsAdminUsername')]",
           "adminPassword": "[parameters('windowsAdminPassword')]"

pkg/api/types.go

@@ -791,7 +791,7 @@ func (p *Properties) GetAgentVMPrefix(a *AgentPoolProfile) string {
 	vmPrefix := ""
 	if index != -1 {
 		if a.IsWindows() {
-			vmPrefix = nameSuffix[:5] + p.K8sOrchestratorName() + strconv.Itoa(900+index)
+			vmPrefix = nameSuffix[:4] + p.K8sOrchestratorName() + fmt.Sprintf("%02d", index)
 		} else {
 			vmPrefix = p.K8sOrchestratorName() + "-" + a.Name + "-" + nameSuffix + "-"
 			if a.IsVirtualMachineScaleSets() {

pkg/api/types_test.go

@@ -2381,7 +2381,7 @@ func TestGetAgentVMPrefix(t *testing.T) {
 					},
 				},
 			},
-			expectedVMPrefix: "24789k8s900",
+			expectedVMPrefix: "2478k8s00",
 		},
 		{
 			name: "agent profile doesn't exist",

pkg/armhelpers/utils/util.go

@@ -1,7 +1,6 @@
 package utils
 
 import (
-	"fmt"
 	"net/url"
 	"regexp"
 	"strconv"
@@ -20,33 +19,28 @@ const (
 	k8sLinuxVMAgentClusterIDIndex  = 2
 	k8sLinuxVMAgentIndexArrayIndex = 3
 
-	k8sWindowsVMNamingFormat               = "^([a-fA-F0-9]{5})([0-9a-zA-Z]{3})([a-zA-Z0-9]{4,6})$"
-	k8sWindowsVMAgentPoolPrefixIndex       = 1
-	k8sWindowsVMAgentOrchestratorNameIndex = 2
-	k8sWindowsVMAgentPoolInfoIndex         = 3
 	// here there are 2 capture groups
 	//  the first is the agent pool name, which can contain -s
 	//  the second group is the Cluster ID for the cluster
 	vmssNamingFormat       = "^[0-9a-zA-Z]+-(.+)-([0-9a-fA-F]{8})-vmss$"
 	vmssAgentPoolNameIndex = 1
 	vmssClusterIDIndex     = 2
 
-	windowsVmssNamingFormat                   = "^([a-fA-F0-9]{5})([0-9a-zA-Z]{3})([a-zA-Z0-9]{3})$"
-	windowsVmssAgentPoolNameIndex             = 1
-	windowsVmssAgentPoolOrchestratorNameIndex = 2
-	windowsVmssAgentPoolIndex                 = 3
+	k8sWindowsOldVMNamingFormat = "^([a-fA-F0-9]{5})([0-9a-zA-Z]{3})([9])([a-zA-Z0-9]{3,5})$"
+	k8sWindowsVMNamingFormat    = "^([a-fA-F0-9]{4})([0-9a-zA-Z]{3})([0-9]{3,8})$"
 )
 
 var vmnameLinuxRegexp *regexp.Regexp
 var vmssnameRegexp *regexp.Regexp
 var vmnameWindowsRegexp *regexp.Regexp
-var vmssnameWindowsRegexp *regexp.Regexp
+var oldvmnameWindowsRegexp *regexp.Regexp
 
 func init() {
 	vmnameLinuxRegexp = regexp.MustCompile(k8sLinuxVMNamingFormat)
 	vmnameWindowsRegexp = regexp.MustCompile(k8sWindowsVMNamingFormat)
+	oldvmnameWindowsRegexp = regexp.MustCompile(k8sWindowsOldVMNamingFormat)
+
 	vmssnameRegexp = regexp.MustCompile(vmssNamingFormat)
-	vmssnameWindowsRegexp = regexp.MustCompile(windowsVmssNamingFormat)
 }
 
 // ResourceName returns the last segment (the resource name) for the specified resource identifier.
@@ -102,46 +96,33 @@ func VmssNameParts(vmssName string) (poolIdentifier, nameSuffix string, err erro
 	return vmssNameParts[vmssAgentPoolNameIndex], vmssNameParts[vmssClusterIDIndex], nil
 }
 
-// WindowsVMNameParts returns parts of Windows VM name e.g: 50621k8s9000
-func WindowsVMNameParts(vmName string) (poolPrefix string, acsStr string, poolIndex int, agentIndex int, err error) {
-	vmNameParts := vmnameWindowsRegexp.FindStringSubmatch(vmName)
-	if len(vmNameParts) != 4 {
-		return "", "", -1, -1, errors.New("resource name was missing from identifier")
+// WindowsVMNameParts returns parts of Windows VM name
+func WindowsVMNameParts(vmName string) (poolPrefix string, orch string, poolIndex int, agentIndex int, err error) {
+	var poolInfo string
+	vmNameParts := oldvmnameWindowsRegexp.FindStringSubmatch(vmName)
+	if len(vmNameParts) != 5 {
+		vmNameParts = vmnameWindowsRegexp.FindStringSubmatch(vmName)
+		if len(vmNameParts) != 4 {
+			return "", "", -1, -1, errors.New("resource name was missing from identifier")
+		}
+		poolInfo = vmNameParts[3]
+	} else {
+		poolInfo = vmNameParts[4]
 	}
 
-	poolPrefix = vmNameParts[k8sWindowsVMAgentPoolPrefixIndex]
-	acsStr = vmNameParts[k8sWindowsVMAgentOrchestratorNameIndex]
-	poolInfo := vmNameParts[k8sWindowsVMAgentPoolInfoIndex]
+	poolPrefix = vmNameParts[1]
+	orch = vmNameParts[2]
 
-	poolIndex, err = strconv.Atoi(poolInfo[:3])
+	poolIndex, err = strconv.Atoi(poolInfo[:2])
 	if err != nil {
 		return "", "", -1, -1, errors.Wrap(err, "Error parsing VM Name")
 	}
-	poolIndex -= 900
-	agentIndex, _ = strconv.Atoi(poolInfo[3:])
-	fmt.Printf("%d\n", agentIndex)
-
-	return poolPrefix, acsStr, poolIndex, agentIndex, nil
-}
-
-// WindowsVMSSNameParts returns parts of Windows VM name e.g: 50621k8s900
-func WindowsVMSSNameParts(vmssName string) (poolPrefix string, acsStr string, poolIndex int, err error) {
-	vmssNameParts := vmssnameWindowsRegexp.FindStringSubmatch(vmssName)
-	if len(vmssNameParts) != 4 {
-		return "", "", -1, errors.Errorf("resource name was missing from identifier")
-	}
-
-	poolPrefix = vmssNameParts[windowsVmssAgentPoolNameIndex]
-	acsStr = vmssNameParts[windowsVmssAgentPoolOrchestratorNameIndex]
-	poolInfo := vmssNameParts[windowsVmssAgentPoolIndex]
-
-	poolIndex, err = strconv.Atoi(poolInfo)
+	agentIndex, err = strconv.Atoi(poolInfo[2:])
 	if err != nil {
-		return "", "", -1, errors.Wrap(err, "Error parsing VM Name")
+		return "", "", -1, -1, errors.Wrap(err, "Error parsing VM Name")
 	}
-	poolIndex -= 900
 
-	return poolPrefix, acsStr, poolIndex, nil
+	return poolPrefix, orch, poolIndex, agentIndex, nil
 }
 
 // GetVMNameIndex return VM index of a node in the Kubernetes cluster
@@ -165,17 +146,13 @@ func GetVMNameIndex(osType compute.OperatingSystemTypes, vmName string) (int, er
 	return agentIndex, nil
 }
 
-// GetK8sVMName reconstructs VM name
-func GetK8sVMName(osType api.OSType, isAKS bool, nameSuffix, agentPoolName string, agentPoolIndex, agentIndex int) (string, error) {
-	prefix := "k8s"
-	if isAKS {
-		prefix = "aks"
-	}
-	if osType == api.Linux {
-		return fmt.Sprintf("%s-%s-%s-%d", prefix, agentPoolName, nameSuffix, agentIndex), nil
-	}
-	if osType == api.Windows {
-		return fmt.Sprintf("%s%s%d%d", nameSuffix[:5], prefix, 900+agentPoolIndex, agentIndex), nil
+// GetK8sVMName reconstructs the VM name
+func GetK8sVMName(p *api.Properties, agentPoolIndex, agentIndex int) (string, error) {
+	if len(p.AgentPoolProfiles) > agentPoolIndex {
+		vmPrefix := p.GetAgentVMPrefix(p.AgentPoolProfiles[agentPoolIndex])
+		if vmPrefix != "" {
+			return vmPrefix + strconv.Itoa(agentIndex), nil
+		}
 	}
 	return "", errors.Errorf("Failed to reconstruct VM Name")
 }

pkg/armhelpers/utils/util_test.go

@@ -80,46 +80,33 @@ func Test_VmssNameParts(t *testing.T) {
 }
 
 func Test_WindowsVMNameParts(t *testing.T) {
-	expectedPoolPrefix := "38988"
-	expectedAcs := "k8s"
-	expectedPoolIndex := 3
-	expectedAgentIndex := 12
-
-	poolPrefix, acs, poolIndex, agentIndex, err := WindowsVMNameParts("38988k8s90312")
-	if poolPrefix != expectedPoolPrefix {
-		t.Fatalf("incorrect poolPrefix. expected=%s actual=%s", expectedPoolPrefix, poolPrefix)
-	}
-	if acs != expectedAcs {
-		t.Fatalf("incorrect acs string. expected=%s actual=%s", expectedAcs, acs)
-	}
-	if poolIndex != expectedPoolIndex {
-		t.Fatalf("incorrect poolIndex. expected=%d actual=%d", expectedPoolIndex, poolIndex)
-	}
-	if agentIndex != expectedAgentIndex {
-		t.Fatalf("incorrect agentIndex. expected=%d actual=%d", expectedAgentIndex, agentIndex)
-	}
-	if err != nil {
-		t.Fatalf("unexpected error: %s", err)
+	data := []struct {
+		VMName, expectedPoolPrefix, expectedOrch string
+		expectedPoolIndex, expectedAgentIndex    int
+	}{
+		{"38988k8s90312", "38988", "k8s", 3, 12},
+		{"4506k8s010", "4506", "k8s", 1, 0},
+		{"2314k8s03000001", "2314", "k8s", 3, 1},
+		{"2314k8s0310", "2314", "k8s", 3, 10},
 	}
-}
-
-func Test_WindowsVMSSNameParts(t *testing.T) {
-	expectedPoolPrefix := "38988"
-	expectedAcs := "k8s"
-	expectedPoolIndex := 3
 
-	poolPrefix, acs, poolIndex, err := WindowsVMSSNameParts("38988k8s903")
-	if poolPrefix != expectedPoolPrefix {
-		t.Fatalf("incorrect poolPrefix. expected=%s actual=%s", expectedPoolPrefix, poolPrefix)
-	}
-	if acs != expectedAcs {
-		t.Fatalf("incorrect acs string. expected=%s actual=%s", expectedAcs, acs)
-	}
-	if poolIndex != expectedPoolIndex {
-		t.Fatalf("incorrect poolIndex. expected=%d actual=%d", expectedPoolIndex, poolIndex)
-	}
-	if err != nil {
-		t.Fatalf("unexpected error: %s", err)
+	for _, d := range data {
+		poolPrefix, orch, poolIndex, agentIndex, err := WindowsVMNameParts(d.VMName)
+		if poolPrefix != d.expectedPoolPrefix {
+			t.Fatalf("incorrect poolPrefix. expected=%s actual=%s", d.expectedPoolPrefix, poolPrefix)
+		}
+		if orch != d.expectedOrch {
+			t.Fatalf("incorrect acs string. expected=%s actual=%s", d.expectedOrch, orch)
+		}
+		if poolIndex != d.expectedPoolIndex {
+			t.Fatalf("incorrect poolIndex. expected=%d actual=%d", d.expectedPoolIndex, poolIndex)
+		}
+		if agentIndex != d.expectedAgentIndex {
+			t.Fatalf("incorrect agentIndex. expected=%d actual=%d", d.expectedAgentIndex, agentIndex)
+		}
+		if err != nil {
+			t.Fatalf("unexpected error: %s", err)
+		}
 	}
 }
 
@@ -150,28 +137,54 @@ func Test_GetVMNameIndexWindows(t *testing.T) {
 }
 
 func Test_GetK8sVMName(t *testing.T) {
+	p := &api.Properties{
+		OrchestratorProfile: &api.OrchestratorProfile{
+			OrchestratorType: api.Kubernetes,
+		},
+		HostedMasterProfile: &api.HostedMasterProfile{
+			DNSPrefix: "foo",
+		},
+		AgentPoolProfiles: []*api.AgentPoolProfile{
+			{
+				Name:   "linux1",
+				VMSize: "Standard_D2_v2",
+				Count:  3,
+				OSType: "Linux",
+			},
+			{
+				Name:   "windows2",
+				VMSize: "Standard_D2_v2",
+				Count:  2,
+				OSType: "Windows",
+			},
+			{
+				Name:   "someotherpool",
+				VMSize: "Standard_D2_v2",
+				Count:  5,
+				OSType: "Linux",
+			},
+		},
+	}
 
 	for _, s := range []struct {
-		osType                     api.OSType
-		isAKS                      bool
-		nameSuffix, agentPoolName  string
+		properties                 *api.Properties
 		agentPoolIndex, agentIndex int
 		expected                   string
 		expectedErr                bool
 	}{
-		{api.Linux, true, "35953384", "agentpool1", 0, 2, "aks-agentpool1-35953384-2", false},
-		{api.Windows, false, "35953384", "agentpool1", 0, 2, "35953k8s9002", false},
-		{"macOS", false, "35953384", "agentpool1", 0, 2, "", true},
+		{properties: p, agentPoolIndex: 0, agentIndex: 2, expected: "aks-linux1-28513887-2", expectedErr: false},
+		{properties: p, agentPoolIndex: 1, agentIndex: 1, expected: "2851aks011", expectedErr: false},
+		{properties: p, agentPoolIndex: 3, agentIndex: 0, expected: "", expectedErr: true},
 	} {
-		vmName, err := GetK8sVMName(s.osType, s.isAKS, s.nameSuffix, s.agentPoolName, s.agentPoolIndex, s.agentIndex)
+		vmName, err := GetK8sVMName(s.properties, s.agentPoolIndex, s.agentIndex)
 
 		if !s.expectedErr {
 			if err != nil {
 				t.Fatalf("unexpected error: %s", err)
 			}
 		}
 		if vmName != s.expected {
-			t.Fatalf("vmName %s, expected %s", vmName, s.expected)
+			t.Fatalf("Got vmName %s, expected %s", vmName, s.expected)
 		}
 	}
 }

pkg/operations/kubernetesupgrade/upgradecluster.go

@@ -338,21 +338,21 @@ func (uc *UpgradeCluster) addVMToAgentPool(vm compute.VirtualMachine, isUpgradab
 	}
 
 	if vm.StorageProfile.OsDisk.OsType == compute.Windows {
-		poolPrefix, _, _, _, err := utils.WindowsVMNameParts(*vm.Name)
-		if err != nil {
-			uc.Logger.Errorf(err.Error())
-			return err
-		}
-
-		//The k8s Windows VM Naming Format is "^([a-fA-F0-9]{5})([0-9a-zA-Z]{3})([a-zA-Z0-9]{4,6})$" (i.e.: 50621k8s9000)
-		//The pool identifier is made of the first 11 characters
-		poolIdentifier = (*vm.Name)[:11]
-
+		poolPrefix, _, _, _, err = utils.WindowsVMNameParts(*vm.Name)
 		if !strings.Contains(uc.NameSuffix, poolPrefix) {
 			uc.Logger.Infof("Skipping VM: %s for upgrade as it does not belong to cluster with expected name suffix: %s\n",
 				*vm.Name, uc.NameSuffix)
 			return nil
 		}
+
+		// The k8s Windows VM Naming Format was previously "^([a-fA-F0-9]{5})([0-9a-zA-Z]{3})([a-zA-Z0-9]{4,6})$" (i.e.: 50621k8s9000)
+		// The k8s Windows VM Naming Format is now "^([a-fA-F0-9]{4})([0-9a-zA-Z]{3})([0-9]{3,8})$" (i.e.: 1708k8s020)
+		// The pool identifier is made of the first 11 or 9 characters
+		if string((*vm.Name)[8]) == "9" {
+			poolIdentifier = (*vm.Name)[:11]
+		} else {
+			poolIdentifier = (*vm.Name)[:9]
+		}
 	} else { // vm.StorageProfile.OsDisk.OsType == compute.Linux
 		poolIdentifier, poolPrefix, _, err = utils.K8sLinuxVMNameParts(*vm.Name)
 		if err != nil {

pkg/operations/kubernetesupgrade/upgrader.go

@@ -226,13 +226,9 @@ func (ku *Upgrader) upgradeAgentPools(ctx context.Context) error {
 		}
 
 		var agentCount, agentPoolIndex int
-		var agentOsType api.OSType
-		var agentPoolName string
 		for indx, app := range ku.ClusterTopology.DataModel.Properties.AgentPoolProfiles {
 			if app.Name == *agentPool.Name {
 				agentCount = app.Count
-				agentOsType = app.OSType
-				agentPoolName = app.Name
 				agentPoolIndex = indx
 				break
 			}
@@ -316,8 +312,7 @@ func (ku *Upgrader) upgradeAgentPools(ctx context.Context) error {
 		for upgradedCount+toBeUpgradedCount < agentCount {
 			agentIndex := getAvailableIndex(agentVMs)
 
-			vmName, err := utils.GetK8sVMName(agentOsType, ku.DataModel.Properties.HostedMasterProfile != nil,
-				ku.NameSuffix, agentPoolName, agentPoolIndex, agentIndex)
+			vmName, err := utils.GetK8sVMName(ku.DataModel.Properties, agentPoolIndex, agentIndex)
 			if err != nil {
 				ku.logger.Errorf("Error reconstructing agent VM name with index %d: %v", agentIndex, err)
 				return err
@@ -359,20 +354,26 @@ func (ku *Upgrader) upgradeAgentPools(ctx context.Context) error {
 				return err
 			}
 
+			vmName, err := utils.GetK8sVMName(ku.DataModel.Properties, agentPoolIndex, agentIndex)
+			if err != nil {
+				ku.logger.Errorf("Error fetching new VM name: %v", err)
+				return err
+			}
+
 			// do not create last node in favor of already created extra node.
 			if upgradedCount == toBeUpgradedCount-1 {
-				ku.logger.Infof("Skipping creation of VM %s (index %d)", vm.name, agentIndex)
+				ku.logger.Infof("Skipping creation of VM %s (index %d)", vmName, agentIndex)
 				delete(agentVMs, agentIndex)
 			} else {
 				err = upgradeAgentNode.CreateNode(ctx, *agentPool.Name, agentIndex)
 				if err != nil {
-					ku.logger.Errorf("Error creating upgraded agent VM %s: %v", vm.name, err)
+					ku.logger.Errorf("Error creating upgraded agent VM %s: %v", vmName, err)
 					return err
 				}
 
-				err = upgradeAgentNode.Validate(&vm.name)
+				err = upgradeAgentNode.Validate(&vmName)
 				if err != nil {
-					ku.logger.Errorf("Error validating upgraded agent VM %s: %v", vm.name, err)
+					ku.logger.Errorf("Error validating upgraded agent VM %s: %v", vmName, err)
 					return err
 				}
 				vm.status = vmStatusUpgraded