Skip to content
This repository has been archived by the owner on Jan 11, 2023. It is now read-only.

Support for Low-priority VMs on Virtual Machine Scale Sets (VMSS) for Kubernetes #2980

Merged
merged 3 commits into from
May 22, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions docs/clusterdefinition.md
Original file line number Diff line number Diff line change
Expand Up @@ -442,6 +442,8 @@ A cluster can have 0 to 12 agent pool profiles. Agent Pool Profiles are used for
|---|---|---|
|availabilityProfile|no|Supported values are `VirtualMachineScaleSets` (default) and `AvailabilitySet`. For Kubernetes clusters before version 1.10, use `AvailabilitySet`. Otherwise, you should use `VirtualMachineScaleSets`|
|count|yes|Describes the node count|
|scaleSetPriority|no|Supported values are `Regular` (default) and `Low`. Only applies to clusters with availabilityProfile `VirtualMachineScaleSets`. Enables the usage of [Low-priority VMs on Scale Sets](https://docs.microsoft.com/en-us/azure/virtual-machine-scale-sets/virtual-machine-scale-sets-use-low-priority).|
|scaleSetEvictionPolicy|no|Supported values are `Delete` (default) and `Deallocate`. Only applies to clusters with availabilityProfile of `VirtualMachineScaleSets` and scaleSetPriority of `Low`.|
|diskSizesGB|no|Describes an array of up to 4 attached disk sizes. Valid disk size values are between 1 and 1024|
|dnsPrefix|Required if agents are to be exposed publically with a load balancer|The dns prefix that forms the FQDN to access the loadbalancer for this agent pool. This must be a unique name among all agent pools. Not supported for Kubernetes clusters|
|name|yes|This is the unique name for the agent pool profile. The resources of the agent pool profile are derived from this name|
Expand Down
38 changes: 38 additions & 0 deletions examples/kubernetes-vmss-low-priority/kubernetes.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
{
"apiVersion": "vlabs",
"properties": {
"orchestratorProfile": {
"orchestratorType": "Kubernetes",
"orchestratorRelease": "1.10",
"kubernetesConfig": {
"useManagedIdentity": true
}
},
"masterProfile": {
"count": 1,
"dnsPrefix": "",
"vmSize": "Standard_D2_v2"
},
"agentPoolProfiles": [
{
"name": "agentpool1",
"count": 1,
"vmSize": "Standard_D2_v2",
"availabilityProfile": "VirtualMachineScaleSets",
"scaleSetPriority": "Low",
"scaleSetEvictionPolicy": "Delete",
"storageProfile": "ManagedDisks"
}
],
"linuxProfile": {
"adminUsername": "azureuser",
"ssh": {
"publicKeys": [
{
"keyData": ""
}
]
}
}
}
}
26 changes: 26 additions & 0 deletions parts/agentparams.t
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,32 @@
"type": "int"
},
{{end}}
{{if .IsLowPriorityScaleSet}}
"{{.Name}}ScaleSetPriority": {
"allowedValues":[
"Low",
"Regular",
""
],
"defaultValue": "{{.ScaleSetPriority}}",
"metadata": {
"description": "The priority for the VM Scale Set. This value can be Low or Regular."
},
"type": "string"
},
"{{.Name}}ScaleSetEvictionPolicy": {
"allowedValues":[
"Delete",
"Deallocate",
""
],
"defaultValue": "{{.ScaleSetEvictionPolicy}}",
"metadata": {
"description": "The Eviction Policy for a Low-priority VM Scale Set."
},
"type": "string"
},
{{end}}
"{{.Name}}VMSize": {
{{GetAgentAllowedSizes}}
"defaultValue": "{{.VMSize}}",
Expand Down
4 changes: 4 additions & 0 deletions parts/k8s/kubernetesagentresourcesvmss.t
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,10 @@
"mode": "Manual"
},
"virtualMachineProfile": {
{{if .IsLowPriorityScaleSet}}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Becuse we're simply assigning the "priority" value that is present in the {{.Name}}ScaleSetPriority variable, would it make more sense to remove this {{if .IsLowPriorityScaleSet}} clause? In other words, let's just make sure that a sane default value is always present for template variable expansion (either "Regular" or "Low").

The advantage of doing this is to remove an unnecessary if clause from the template (which already has a lot of conditional blocks and is rather hard to read!)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Jack, I was in two minds on this and leant towards making the output template less verbose (whilst also reducing the number of parameters, and variables), despite setting the default values, as people who don't use or care about Low-priority VMSS aren't likely to encounter this in their templates (particularly because "Regular" is implicit). And also because it is currently preview. But happy to simplify if you prefer. Any thoughts?

"priority": "[variables('{{.Name}}ScaleSetPriority')]",
"evictionPolicy": "[variables('{{.Name}}ScaleSetEvictionPolicy')]",
{{end}}
"networkProfile": {
"networkInterfaceConfigurations": [
{
Expand Down
4 changes: 4 additions & 0 deletions parts/k8s/kubernetesagentvars.t
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@
"{{.Name}}VMNamePrefix": "[concat(variables('orchestratorName'), '-{{.Name}}-', variables('nameSuffix'), '-')]",
{{else}}
"{{.Name}}VMNamePrefix": "[concat(variables('orchestratorName'), '-{{.Name}}-', variables('nameSuffix'), '-vmss')]",
{{if .IsLowPriorityScaleSet}}
"{{.Name}}ScaleSetPriority": "[parameters('{{.Name}}ScaleSetPriority')]",
"{{.Name}}ScaleSetEvictionPolicy": "[parameters('{{.Name}}ScaleSetEvictionPolicy')]",
{{end}}
{{end}}
{{end}}
"{{.Name}}VMSize": "[parameters('{{.Name}}VMSize')]",
Expand Down
3 changes: 3 additions & 0 deletions pkg/acsengine/defaults.go
Original file line number Diff line number Diff line change
Expand Up @@ -764,6 +764,9 @@ func setStorageDefaults(a *api.Properties) {
if len(profile.AvailabilityProfile) == 0 {
profile.AvailabilityProfile = api.VirtualMachineScaleSets
}
if len(profile.ScaleSetEvictionPolicy) == 0 && profile.ScaleSetPriority == api.ScaleSetPriorityLow {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extra credit: add a unit test in default_test.go to make sure EvictionPolicy / Priority are set coherently 🙂

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great point. I've just added TestAgentPoolProfile which check default values remain empty, and that the ScaleSetEvictionPolicy is set correctly when ScaleSetPriority is Low.

profile.ScaleSetEvictionPolicy = api.ScaleSetEvictionPolicyDelete
}
}
}

Expand Down
23 changes: 23 additions & 0 deletions pkg/acsengine/defaults_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -491,6 +491,28 @@ func TestStorageProfile(t *testing.T) {
}
}

func TestAgentPoolProfile(t *testing.T) {
mockCS := getMockBaseContainerService("1.10")
properties := mockCS.Properties
properties.OrchestratorProfile.OrchestratorType = "Kubernetes"
properties.MasterProfile.Count = 1
SetPropertiesDefaults(&mockCS, false)
if properties.AgentPoolProfiles[0].ScaleSetPriority != "" {
t.Fatalf("AgentPoolProfiles[0].ScaleSetPriority did not have the expected configuration, got %s, expected %s",
properties.AgentPoolProfiles[0].ScaleSetPriority, "")
}
if properties.AgentPoolProfiles[0].ScaleSetEvictionPolicy != "" {
t.Fatalf("AgentPoolProfiles[0].ScaleSetEvictionPolicy did not have the expected configuration, got %s, expected %s",
properties.AgentPoolProfiles[0].ScaleSetEvictionPolicy, "")
}
properties.AgentPoolProfiles[0].ScaleSetPriority = api.ScaleSetPriorityLow
SetPropertiesDefaults(&mockCS, false)
if properties.AgentPoolProfiles[0].ScaleSetEvictionPolicy != api.ScaleSetEvictionPolicyDelete {
t.Fatalf("AgentPoolProfile[0].ScaleSetEvictionPolicy did not have the expected configuration, got %s, expected %s",
properties.AgentPoolProfiles[0].ScaleSetEvictionPolicy, api.ScaleSetEvictionPolicyDelete)
}
}

func getMockAddon(name string) api.KubernetesAddon {
return api.KubernetesAddon{
Name: name,
Expand Down Expand Up @@ -521,6 +543,7 @@ func getMockBaseContainerService(orchestratorVersion string) api.ContainerServic
},
}
}

func getKubernetesConfigWithFeatureGates(featureGates string) *api.KubernetesConfig {
return &api.KubernetesConfig{
KubeletConfig: map[string]string{"--feature-gates": featureGates},
Expand Down
8 changes: 8 additions & 0 deletions pkg/api/const.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,14 @@ const (
AvailabilitySet = "AvailabilitySet"
// VirtualMachineScaleSets means that the vms are in a virtual machine scaleset
VirtualMachineScaleSets = "VirtualMachineScaleSets"
// ScaleSetPriorityRegular is the default ScaleSet Priority
ScaleSetPriorityRegular = "Regular"
// ScaleSetPriorityLow means the ScaleSet will use Low-priority VMs
ScaleSetPriorityLow = "Low"
// ScaleSetEvictionPolicyDelete is the default Eviction Policy for Low-priority VM ScaleSets
ScaleSetEvictionPolicyDelete = "Delete"
// ScaleSetEvictionPolicyDeallocate means a Low-priority VM ScaleSet will deallocate, rather than delete, VMs.
ScaleSetEvictionPolicyDeallocate = "Deallocate"
)

// storage profiles
Expand Down
2 changes: 2 additions & 0 deletions pkg/api/converterfromapi.go
Original file line number Diff line number Diff line change
Expand Up @@ -960,6 +960,8 @@ func convertAgentPoolProfileToVLabs(api *AgentPoolProfile, p *vlabs.AgentPoolPro
p.Ports = []int{}
p.Ports = append(p.Ports, api.Ports...)
p.AvailabilityProfile = api.AvailabilityProfile
p.ScaleSetPriority = api.ScaleSetPriority
p.ScaleSetEvictionPolicy = api.ScaleSetEvictionPolicy
p.StorageProfile = api.StorageProfile
p.DiskSizesGB = []int{}
p.DiskSizesGB = append(p.DiskSizesGB, api.DiskSizesGB...)
Expand Down
2 changes: 2 additions & 0 deletions pkg/api/convertertoapi.go
Original file line number Diff line number Diff line change
Expand Up @@ -968,6 +968,8 @@ func convertVLabsAgentPoolProfile(vlabs *vlabs.AgentPoolProfile, api *AgentPoolP
api.Ports = []int{}
api.Ports = append(api.Ports, vlabs.Ports...)
api.AvailabilityProfile = vlabs.AvailabilityProfile
api.ScaleSetPriority = vlabs.ScaleSetPriority
api.ScaleSetEvictionPolicy = vlabs.ScaleSetEvictionPolicy
api.StorageProfile = vlabs.StorageProfile
api.DiskSizesGB = []int{}
api.DiskSizesGB = append(api.DiskSizesGB, vlabs.DiskSizesGB...)
Expand Down
37 changes: 22 additions & 15 deletions pkg/api/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -402,21 +402,23 @@ type Extension struct {

// AgentPoolProfile represents an agent pool definition
type AgentPoolProfile struct {
Name string `json:"name"`
Count int `json:"count"`
VMSize string `json:"vmSize"`
OSDiskSizeGB int `json:"osDiskSizeGB,omitempty"`
DNSPrefix string `json:"dnsPrefix,omitempty"`
OSType OSType `json:"osType,omitempty"`
Ports []int `json:"ports,omitempty"`
AvailabilityProfile string `json:"availabilityProfile"`
StorageProfile string `json:"storageProfile,omitempty"`
DiskSizesGB []int `json:"diskSizesGB,omitempty"`
VnetSubnetID string `json:"vnetSubnetID,omitempty"`
Subnet string `json:"subnet"`
IPAddressCount int `json:"ipAddressCount,omitempty"`
Distro Distro `json:"distro,omitempty"`
Role AgentPoolProfileRole `json:"role,omitempty"`
Name string `json:"name"`
Count int `json:"count"`
VMSize string `json:"vmSize"`
OSDiskSizeGB int `json:"osDiskSizeGB,omitempty"`
DNSPrefix string `json:"dnsPrefix,omitempty"`
OSType OSType `json:"osType,omitempty"`
Ports []int `json:"ports,omitempty"`
AvailabilityProfile string `json:"availabilityProfile"`
ScaleSetPriority string `json:"scaleSetPriority,omitempty"`
ScaleSetEvictionPolicy string `json:"scaleSetEvictionPolicy,omitempty"`
StorageProfile string `json:"storageProfile,omitempty"`
DiskSizesGB []int `json:"diskSizesGB,omitempty"`
VnetSubnetID string `json:"vnetSubnetID,omitempty"`
Subnet string `json:"subnet"`
IPAddressCount int `json:"ipAddressCount,omitempty"`
Distro Distro `json:"distro,omitempty"`
Role AgentPoolProfileRole `json:"role,omitempty"`

FQDN string `json:"fqdn,omitempty"`
CustomNodeLabels map[string]string `json:"customNodeLabels,omitempty"`
Expand Down Expand Up @@ -708,6 +710,11 @@ func (a *AgentPoolProfile) IsVirtualMachineScaleSets() bool {
return a.AvailabilityProfile == VirtualMachineScaleSets
}

// IsLowPriorityScaleSet returns true if the VMSS is Low Priority
func (a *AgentPoolProfile) IsLowPriorityScaleSet() bool {
return a.AvailabilityProfile == VirtualMachineScaleSets && a.ScaleSetPriority == ScaleSetPriorityLow
}

// IsManagedDisks returns true if the customer specified disks
func (a *AgentPoolProfile) IsManagedDisks() bool {
return a.StorageProfile == ManagedDisks
Expand Down
34 changes: 18 additions & 16 deletions pkg/api/vlabs/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -401,22 +401,24 @@ type Extension struct {

// AgentPoolProfile represents an agent pool definition
type AgentPoolProfile struct {
Name string `json:"name" validate:"required"`
Count int `json:"count" validate:"required,min=1,max=100"`
VMSize string `json:"vmSize" validate:"required"`
OSDiskSizeGB int `json:"osDiskSizeGB,omitempty" validate:"min=0,max=1023"`
DNSPrefix string `json:"dnsPrefix,omitempty"`
OSType OSType `json:"osType,omitempty"`
Ports []int `json:"ports,omitempty" validate:"dive,min=1,max=65535"`
AvailabilityProfile string `json:"availabilityProfile"`
StorageProfile string `json:"storageProfile" validate:"eq=StorageAccount|eq=ManagedDisks|len=0"`
DiskSizesGB []int `json:"diskSizesGB,omitempty" validate:"max=4,dive,min=1,max=1023"`
VnetSubnetID string `json:"vnetSubnetID,omitempty"`
IPAddressCount int `json:"ipAddressCount,omitempty" validate:"min=0,max=256"`
Distro Distro `json:"distro,omitempty"`
KubernetesConfig *KubernetesConfig `json:"kubernetesConfig,omitempty"`
ImageRef *ImageReference `json:"imageReference,omitempty"`
Role AgentPoolProfileRole `json:"role,omitempty"`
Name string `json:"name" validate:"required"`
Count int `json:"count" validate:"required,min=1,max=100"`
VMSize string `json:"vmSize" validate:"required"`
OSDiskSizeGB int `json:"osDiskSizeGB,omitempty" validate:"min=0,max=1023"`
DNSPrefix string `json:"dnsPrefix,omitempty"`
OSType OSType `json:"osType,omitempty"`
Ports []int `json:"ports,omitempty" validate:"dive,min=1,max=65535"`
AvailabilityProfile string `json:"availabilityProfile"`
ScaleSetPriority string `json:"scaleSetPriority,omitempty" validate:"eq=Regular|eq=Low|len=0"`
ScaleSetEvictionPolicy string `json:"scaleSetEvictionPolicy,omitempty" validate:"eq=Delete|eq=Deallocate|len=0"`
StorageProfile string `json:"storageProfile" validate:"eq=StorageAccount|eq=ManagedDisks|len=0"`
DiskSizesGB []int `json:"diskSizesGB,omitempty" validate:"max=4,dive,min=1,max=1023"`
VnetSubnetID string `json:"vnetSubnetID,omitempty"`
IPAddressCount int `json:"ipAddressCount,omitempty" validate:"min=0,max=256"`
Distro Distro `json:"distro,omitempty"`
KubernetesConfig *KubernetesConfig `json:"kubernetesConfig,omitempty"`
ImageRef *ImageReference `json:"imageReference,omitempty"`
Role AgentPoolProfileRole `json:"role,omitempty"`

// subnet is internal
subnet string
Expand Down
3 changes: 3 additions & 0 deletions pkg/api/vlabs/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,9 @@ func (a *AgentPoolProfile) Validate(orchestratorType string) error {
if e := validate.Var(a.Ports, "len=0"); e != nil {
return fmt.Errorf("AgentPoolProfile.Ports must be empty for Kubernetes")
}
if validate.Var(a.ScaleSetPriority, "eq=Regular") == nil && validate.Var(a.ScaleSetEvictionPolicy, "len=0") != nil {
return fmt.Errorf("property 'AgentPoolProfile.ScaleSetEvictionPolicy' must be empty for AgentPoolProfile.Priority of Regular")
}
}

if a.DNSPrefix != "" {
Expand Down