Skip to content

Commit

Permalink
Azure: Add functionality to change Azure Machine Disk Types
Browse files Browse the repository at this point in the history
Azure disk types for the master and worker nodes is by default
set to Premium SSD(Premium_LRS) setting. Made the changes to add
the disktype of the master and worker nodes in the install config
by adding an extra field in MachinePool DiskType to the OSDisk struct
which is part of the DefaultMachinePlatform attribute in the Azure
Platform type. To change the disk type, the install config must be
changed by adding the diskType under platform->Azure property by adding
the defaultMachinePlatform->osDisk->diskType and setting the value to
one of the following, Premium_LRS and StandardSSD_LRS.

For custom Azure modifications to the disk types, the worker nodes can have
an additional disk type Standard_LRS. HDD type disks for master machines are
not recommended so it cannot be added to the default values or to the
custom modifications for the master.

Currently, the UltraSSD_LRS is under Azure preview and it is only
available in three regions and hence is currently disabled(commented).
  • Loading branch information
rna-afk committed May 1, 2020
1 parent c1b6d06 commit 5b86cb3
Show file tree
Hide file tree
Showing 10 changed files with 173 additions and 18 deletions.
2 changes: 2 additions & 0 deletions docs/user/azure/customization.md
Expand Up @@ -18,6 +18,7 @@ The following options are available when using Azure:

* `osDisk` (optional object):
* `diskSizeGB` (optional integer): The size of the disk in gigabytes (GB).
* `diskType` (optional string): The type of disk (allowed values are: `Premium_LRS`, `Standard_LRS`, and `StandardSSD_LRS`).
* `type` (optional string): The Azure instance type.
* `zones` (optional string slice): List of Azure availability zones that can be used (for example, `["1", "2", "3"]`).

Expand Down Expand Up @@ -65,6 +66,7 @@ controlPlane:
type: Standard_DS4_v2
osDisk:
diskSizeGB: 512
diskType: Premium_LRS
replicas: 3
compute:
- name: worker
Expand Down
28 changes: 28 additions & 0 deletions pkg/asset/installconfig/azure/client.go
Expand Up @@ -6,6 +6,7 @@ import (

"github.com/pkg/errors"

azsku "github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2018-10-01/compute"
aznetwork "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2018-12-01/network"
azres "github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2019-05-01/resources"
azsubs "github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2019-06-01/subscriptions"
Expand All @@ -20,6 +21,7 @@ type API interface {
GetControlPlaneSubnet(ctx context.Context, resourceGroupName, virtualNetwork, subnet string) (*aznetwork.Subnet, error)
ListLocations(ctx context.Context) (*[]azsubs.Location, error)
GetResourcesProvider(ctx context.Context, resourceProviderNamespace string) (*azres.Provider, error)
GetResourceSku(ctx context.Context, region, diskType string) (*azsku.ResourceSku, error)
}

// Client makes calls to the Azure API.
Expand Down Expand Up @@ -152,3 +154,29 @@ func (c *Client) getProvidersClient(ctx context.Context) (azres.ProvidersClient,
client.Authorizer = c.ssn.Authorizer
return client, nil
}

// GetResourceSku checks if the diskType exists for the given region.
func (c *Client) GetResourceSku(ctx context.Context, region string, diskType string) (*azsku.ResourceSku, error) {
client := azsku.NewResourceSkusClient(c.ssn.Credentials.SubscriptionID)
client.Authorizer = c.ssn.Authorizer
ctx, cancel := context.WithTimeout(ctx, 30*time.Second)
defer cancel()

resourceSkus, err := client.List(ctx)
if err != nil {
return nil, err
}

for _, page := range resourceSkus.Values() {
if *page.Name == diskType {
for _, diskRegion := range *page.Locations {
if diskRegion == region {
return &page, nil
}
}
return nil, errors.New("UltraSSD_LRS is not available in specified region")
}
}

return nil, errors.New("UltraSSD_LRS not available for specified subscription")
}
44 changes: 30 additions & 14 deletions pkg/asset/installconfig/azure/mock/azureclient_generated.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 5 additions & 1 deletion pkg/asset/machines/azure/machines.go
Expand Up @@ -90,6 +90,10 @@ func provider(platform *azure.Platform, mpool *azure.MachinePool, osImage string
return nil, err
}

if mpool.OSDisk.DiskType == "" {
mpool.OSDisk.DiskType = "Premium_LRS"
}

return &azureprovider.AzureMachineProviderSpec{
TypeMeta: metav1.TypeMeta{
APIVersion: "azureproviderconfig.openshift.io/v1beta1",
Expand All @@ -106,7 +110,7 @@ func provider(platform *azure.Platform, mpool *azure.MachinePool, osImage string
OSType: "Linux",
DiskSizeGB: mpool.OSDisk.DiskSizeGB,
ManagedDisk: azureprovider.ManagedDisk{
StorageAccountType: "Premium_LRS",
StorageAccountType: mpool.OSDisk.DiskType,
},
},
Zone: az,
Expand Down
1 change: 1 addition & 0 deletions pkg/asset/machines/master.go
Expand Up @@ -261,6 +261,7 @@ func (m *Master) Generate(dependencies asset.Parents) error {
mpool.Zones = []string{""}
}
}

pool.Platform.Azure = &mpool

machines, err = azure.Machines(clusterID.InfraID, ic, pool, string(*rhcosImage), "master", "master-user-data")
Expand Down
6 changes: 6 additions & 0 deletions pkg/types/azure/machinepool.go
Expand Up @@ -19,6 +19,8 @@ type MachinePool struct {
type OSDisk struct {
// DiskSizeGB defines the size of disk in GB.
DiskSizeGB int32 `json:"diskSizeGB"`
// DiskType defines the type of disk.
DiskType string `json:"diskType"`
}

// Set sets the values from `required` to `a`.
Expand All @@ -38,4 +40,8 @@ func (a *MachinePool) Set(required *MachinePool) {
if required.OSDisk.DiskSizeGB != 0 {
a.OSDisk.DiskSizeGB = required.OSDisk.DiskSizeGB
}

if required.OSDisk.DiskType != "" {
a.OSDisk.DiskType = required.OSDisk.DiskType
}
}
68 changes: 68 additions & 0 deletions pkg/types/azure/validation/machinepool.go
@@ -1,7 +1,14 @@
package validation

import (
"context"
"fmt"
"time"

azconfig "github.com/openshift/installer/pkg/asset/installconfig/azure"
"github.com/openshift/installer/pkg/types"
"github.com/openshift/installer/pkg/types/azure"
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/apimachinery/pkg/util/validation/field"
)

Expand All @@ -12,5 +19,66 @@ func ValidateMachinePool(p *azure.MachinePool, fldPath *field.Path) field.ErrorL
if p.OSDisk.DiskSizeGB < 0 {
allErrs = append(allErrs, field.Invalid(fldPath.Child("diskSizeGB"), p.OSDisk.DiskSizeGB, "Storage DiskSizeGB must be positive"))
}

return allErrs
}

// ValidateDiskType checks that the specified disk type is valid for control plane.
func ValidateDiskType(p *types.MachinePool, fldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{}

if p.Name == "master" && p.Platform.Azure.OSDisk.DiskType == "Standard_LRS" {
allErrs = append(allErrs, field.Invalid(fldPath.Child("diskType"), p.Platform.Azure.OSDisk.DiskType, fmt.Sprintf("%s not compatible with control planes.", p.Platform.Azure.OSDisk.DiskType)))
}

if p.Platform.Azure.OSDisk.DiskType != "" {
diskTypes := sets.NewString("Standard_LRS", "StandardSSD_LRS", "Premium_LRS") // "UltraSSD_LRS"

if !diskTypes.Has(p.Platform.Azure.OSDisk.DiskType) {
allErrs = append(allErrs, field.NotSupported(fldPath.Child("diskType"), p.Platform.Azure.OSDisk.DiskType, diskTypes.List()))
}
}

return allErrs
}

// ValidateDefaultDiskType checks that the specified disk type is valid for default Azure Machine Platform.
func ValidateDefaultDiskType(p *azure.MachinePool, fldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{}

if p.OSDisk.DiskType != "" {
diskTypes := sets.NewString("StandardSSD_LRS", "Premium_LRS") // "UltraSSD_LRS"

if !diskTypes.Has(p.OSDisk.DiskType) {
allErrs = append(allErrs, field.NotSupported(fldPath.Child("diskType"), p.OSDisk.DiskType, diskTypes.List()))
}
}

// if p.OSDisk.DiskType == "UltraSSD_LRS" {
// allErrs = append(allErrs, ValidateRegionForUltraDisks(fldPath, p)...)
// }

return allErrs
}

// ValidateRegionForUltraDisks checks that the Ultra SSD disks are available for the user.
func ValidateRegionForUltraDisks(fldPath *field.Path, region string) field.ErrorList {
allErrs := field.ErrorList{}

ctx, cancel := context.WithTimeout(context.TODO(), 30*time.Second)
defer cancel()
client, err := azconfig.NewClient(ctx)
if err != nil {
allErrs = append(allErrs, field.InternalError(fldPath.Child("disktype"), err))
}

contxt, canc := context.WithTimeout(context.TODO(), 30*time.Second)
defer canc()

_, err = client.GetResourceSku(contxt, region, "UltraSSD_LRS")
if err != nil {
allErrs = append(allErrs, field.InternalError(fldPath.Child("disktype"), err))
}

return allErrs
}
17 changes: 17 additions & 0 deletions pkg/types/azure/validation/machinepool_test.go
Expand Up @@ -35,6 +35,23 @@ func TestValidateMachinePool(t *testing.T) {
},
expected: `^test-path\.diskSizeGB: Invalid value: -120: Storage DiskSizeGB must be positive$`,
},
{
name: "valid disk",
pool: &azure.MachinePool{
OSDisk: azure.OSDisk{
DiskType: "Premium_LRS",
},
},
},
{
name: "invalid disk",
pool: &azure.MachinePool{
OSDisk: azure.OSDisk{
DiskType: "LRS",
},
},
expected: `^test-path\.diskType: Invalid value: \"LRS\": Storage DiskType value invalid$`,
},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
Expand Down
4 changes: 4 additions & 0 deletions pkg/types/azure/validation/platform.go
Expand Up @@ -20,6 +20,10 @@ func ValidatePlatform(p *azure.Platform, publish types.PublishingStrategy, fldPa
}
if p.DefaultMachinePlatform != nil {
allErrs = append(allErrs, ValidateMachinePool(p.DefaultMachinePlatform, fldPath.Child("defaultMachinePlatform"))...)
allErrs = append(allErrs, ValidateDefaultDiskType(p.DefaultMachinePlatform, fldPath.Child("defaultMachinePlatform"))...)
// if p.DefaultMachinePlatform.OSDisk.DiskType == "UltraSSD_LRS" {
// allErrs = append(allErrs, ValidateRegionForUltraDisks(fldPath.Child("defaultMachinePlatform"), p.Region)...)
// }
}
if p.VirtualNetwork != "" {
if p.ComputeSubnet == "" {
Expand Down
15 changes: 12 additions & 3 deletions pkg/types/validation/machinepools.go
Expand Up @@ -61,11 +61,11 @@ func ValidateMachinePool(platform *types.Platform, p *types.MachinePool, fldPath
if !validArchitectures[p.Architecture] {
allErrs = append(allErrs, field.NotSupported(fldPath.Child("architecture"), p.Architecture, validArchitectureValues))
}
allErrs = append(allErrs, validateMachinePoolPlatform(platform, &p.Platform, fldPath.Child("platform"))...)
allErrs = append(allErrs, validateMachinePoolPlatform(platform, &p.Platform, p, fldPath.Child("platform"))...)
return allErrs
}

func validateMachinePoolPlatform(platform *types.Platform, p *types.MachinePoolPlatform, fldPath *field.Path) field.ErrorList {
func validateMachinePoolPlatform(platform *types.Platform, p *types.MachinePoolPlatform, pool *types.MachinePool, fldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{}
platformName := platform.Name()
validate := func(n string, value interface{}, validation func(*field.Path) field.ErrorList) {
Expand All @@ -80,7 +80,7 @@ func validateMachinePoolPlatform(platform *types.Platform, p *types.MachinePoolP
validate(aws.Name, p.AWS, func(f *field.Path) field.ErrorList { return awsvalidation.ValidateMachinePool(platform.AWS, p.AWS, f) })
}
if p.Azure != nil {
validate(azure.Name, p.Azure, func(f *field.Path) field.ErrorList { return azurevalidation.ValidateMachinePool(p.Azure, f) })
validate(azure.Name, p.Azure, func(f *field.Path) field.ErrorList { return validateAzureMachinePool(p, pool, f) })
}
if p.Libvirt != nil {
validate(libvirt.Name, p.Libvirt, func(f *field.Path) field.ErrorList { return libvirtvalidation.ValidateMachinePool(p.Libvirt, f) })
Expand All @@ -93,3 +93,12 @@ func validateMachinePoolPlatform(platform *types.Platform, p *types.MachinePoolP
}
return allErrs
}

func validateAzureMachinePool(p *types.MachinePoolPlatform, pool *types.MachinePool, f *field.Path) field.ErrorList {
allErrs := field.ErrorList{}

allErrs = append(allErrs, azurevalidation.ValidateMachinePool(p.Azure, f)...)
allErrs = append(allErrs, azurevalidation.ValidateDiskType(pool, f)...)

return allErrs
}

0 comments on commit 5b86cb3

Please sign in to comment.