Skip to content

Commit

Permalink
Merge pull request #3520 from rna-afk/azure-disktype-change
Browse files Browse the repository at this point in the history
Bug 1836337: Azure: Add functionality to change Azure Machine Disk Types
  • Loading branch information
openshift-merge-robot authored May 19, 2020
2 parents f6716ea + 22db280 commit 8303d48
Show file tree
Hide file tree
Showing 14 changed files with 275 additions and 2 deletions.
30 changes: 30 additions & 0 deletions data/data/install.openshift.io_installconfigs.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,16 @@ spec:
format: int32
minimum: 0
type: integer
diskType:
description: DiskType defines the type of disk. The
valid values are Standard_LRS, Premium_LRS, StandardSSD_LRS
For control plane nodes, the valid values are Premium_LRS
and StandardSSD_LRS. Default is Premium_LRS
enum:
- Standard_LRS
- Premium_LRS
- StandardSSD_LRS
type: string
required:
- diskSizeGB
type: object
Expand Down Expand Up @@ -403,6 +413,16 @@ spec:
format: int32
minimum: 0
type: integer
diskType:
description: DiskType defines the type of disk. The valid
values are Standard_LRS, Premium_LRS, StandardSSD_LRS
For control plane nodes, the valid values are Premium_LRS
and StandardSSD_LRS. Default is Premium_LRS
enum:
- Standard_LRS
- Premium_LRS
- StandardSSD_LRS
type: string
required:
- diskSizeGB
type: object
Expand Down Expand Up @@ -848,6 +868,16 @@ spec:
format: int32
minimum: 0
type: integer
diskType:
description: DiskType defines the type of disk. The valid
values are Standard_LRS, Premium_LRS, StandardSSD_LRS
For control plane nodes, the valid values are Premium_LRS
and StandardSSD_LRS. Default is Premium_LRS
enum:
- Standard_LRS
- Premium_LRS
- StandardSSD_LRS
type: string
required:
- diskSizeGB
type: object
Expand Down
9 changes: 9 additions & 0 deletions docs/user/azure/customization.md
Original file line number Diff line number Diff line change
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 All @@ -73,6 +75,7 @@ compute:
type: Standard_DS4_v2
osDisk:
diskSizeGB: 512
diskType: Standard_LRS
zones:
- "1"
- "2"
Expand All @@ -84,6 +87,9 @@ platform:
azure:
region: centralus
baseDomainResourceGroupName: os4-common
osDisk:
diskSizeGB: 512
diskType: Premium_LRS
pullSecret: '{"auths": ...}'
sshKey: ssh-ed25519 AAAA...
```
Expand All @@ -104,6 +110,9 @@ platform:
virtualNetwork: example_vnet
controlPlaneSubnet: example_master_subnet
computeSubnet: example_worker_subnet
osDisk:
diskSizeGB: 512
diskType: Premium_LRS
pullSecret: '{"auths": ...}'
sshKey: ssh-ed25519 AAAA...
```
33 changes: 33 additions & 0 deletions pkg/asset/installconfig/azure/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,16 @@ package azure

import (
"context"
"strings"
"time"

"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"
"github.com/Azure/go-autorest/autorest/to"
)

//go:generate mockgen -source=./client.go -destination=mock/azureclient_generated.go -package=mock
Expand All @@ -20,6 +23,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)
GetDiskSkus(ctx context.Context, region string) ([]azsku.ResourceSku, error)
}

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

// GetDiskSkus returns all the disk SKU pages for a given region.
func (c *Client) GetDiskSkus(ctx context.Context, region 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()

var sku []azsku.ResourceSku

for skuPage, err := client.List(ctx); skuPage.NotDone(); err = skuPage.NextWithContext(ctx) {
if err != nil {
return nil, errors.Wrap(err, "error fetching SKU pages")
}
for _, page := range skuPage.Values() {
for _, diskRegion := range to.StringSlice(page.Locations) {
if strings.EqualFold(diskRegion, region) {
sku = append(sku, page)
}
}
}
}

if len(sku) != 0 {
return sku, nil
}

return nil, errors.Errorf("no disks for specified subscription in region %s", region)
}
16 changes: 16 additions & 0 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.

28 changes: 28 additions & 0 deletions pkg/asset/installconfig/azure/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"fmt"
"net"
"strings"
"time"

azdns "github.com/Azure/azure-sdk-for-go/profiles/latest/dns/mgmt/dns"
aznetwork "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2018-12-01/network"
Expand Down Expand Up @@ -119,6 +120,33 @@ func validateRegion(client API, fieldPath *field.Path, p *aztypes.Platform) fiel
return field.ErrorList{field.Invalid(fieldPath, p.Region, fmt.Sprintf("region %q does not support resource creation", p.Region))}
}

// validateRegionForUltraDisks checks that the Ultra SSD disks are available for the user.
func validateRegionForUltraDisks(fldPath *field.Path, region string) *field.Error {
diskType := "UltraSSD_LRS"

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

regionDisks, err := client.GetDiskSkus(ctx, region)
if err != nil {
return field.InternalError(fldPath.Child("diskType"), err)
}

for _, page := range regionDisks {
for _, location := range to.StringSlice(page.Locations) {
if location == diskType {
return nil
}
}
}

return field.NotFound(fldPath.Child("diskType"), fmt.Sprintf("%s is not available in specified subscription for region %s", diskType, region))
}

// ValidatePublicDNS checks DNS for CNAME, A, and AAA records for
// api.zoneName. If a record exists, it's likely a cluster already exists.
func ValidatePublicDNS(ic *types.InstallConfig) error {
Expand Down
16 changes: 16 additions & 0 deletions pkg/asset/installconfig/azure/validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,11 @@ import (
"net"
"testing"

"github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2019-07-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"
"github.com/Azure/go-autorest/autorest/to"
"github.com/golang/mock/gomock"
"github.com/openshift/installer/pkg/asset/installconfig/azure/mock"
"github.com/openshift/installer/pkg/ipnet"
Expand All @@ -31,13 +33,17 @@ var (
validControlPlaneSubnetCIDR = "10.0.32.0/24"
validResourceGroupNamespace = "Microsoft.Resources"
validResourceGroupResourceType = "resourceGroups"
validResourceSkuRegions = "southeastasia"
validDiskSkuType = "UltraSSD_LRS"

invalidateMachineCIDR = func(ic *types.InstallConfig) {
_, newCidr, _ := net.ParseCIDR("192.168.111.0/24")
ic.MachineNetwork = []types.MachineNetworkEntry{
{CIDR: ipnet.IPNet{IPNet: *newCidr}},
}
}
invalidResourceSkuRegion = "centralus"
invalidDiskType = "LRS"

invalidateNetworkResourceGroup = func(ic *types.InstallConfig) {
ic.Azure.NetworkResourceGroupName = "invalid-network-resource-group"
Expand All @@ -50,6 +56,9 @@ var (
invalidateRegionLetterCase = func(ic *types.InstallConfig) { ic.Azure.Region = "Central US" }
removeVirtualNetwork = func(ic *types.InstallConfig) { ic.Azure.VirtualNetwork = "" }
removeSubnets = func(ic *types.InstallConfig) { ic.Azure.ComputeSubnet, ic.Azure.ControlPlaneSubnet = "", "" }
invalidateDiskType = func(ic *types.InstallConfig) { ic.Azure.DefaultMachinePlatform.OSDisk.DiskType = invalidDiskType }
invalidateRegionForDiskType = func(ic *types.InstallConfig) { ic.Azure.Region = invalidResourceSkuRegion }
validResourceSkuDisk = func(ic *types.InstallConfig) { ic.Azure.DefaultMachinePlatform.OSDisk.DiskType = validDiskSkuType }

virtualNetworkAPIResult = &aznetwork.VirtualNetwork{
Name: &validVirtualNetwork,
Expand Down Expand Up @@ -85,6 +94,9 @@ var (
},
},
}
validateResourceSkuResult = &compute.ResourceSku{
Name: to.StringPtr("UltraSSD_LRS"),
}
)

func validInstallConfig() *types.InstallConfig {
Expand All @@ -101,6 +113,7 @@ func validInstallConfig() *types.InstallConfig {
VirtualNetwork: validVirtualNetwork,
ComputeSubnet: validComputeSubnet,
ControlPlaneSubnet: validControlPlaneSubnet,
DefaultMachinePlatform: &azure.MachinePool{},
},
},
}
Expand Down Expand Up @@ -192,6 +205,9 @@ func TestAzureInstallConfigValidation(t *testing.T) {
// ResourceProvider
azureClient.EXPECT().GetResourcesProvider(gomock.Any(), validResourceGroupNamespace).Return(resourcesProviderAPIResult, nil).AnyTimes()

//Resource SKUs
azureClient.EXPECT().GetDiskSkus(gomock.Any(), validResourceSkuRegions).Return(nil, fmt.Errorf("invalid disk type")).AnyTimes()
azureClient.EXPECT().GetDiskSkus(gomock.Any(), invalidResourceSkuRegion).Return(nil, fmt.Errorf("invalid region")).AnyTimes()
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
editedInstallConfig := validInstallConfig()
Expand Down
6 changes: 5 additions & 1 deletion pkg/asset/machines/azure/machines.go
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
1 change: 1 addition & 0 deletions pkg/asset/machines/worker.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ func defaultAzureMachinePoolPlatform() azuretypes.MachinePool {
return azuretypes.MachinePool{
OSDisk: azuretypes.OSDisk{
DiskSizeGB: 128,
DiskType: "Premium_LRS",
},
}
}
Expand Down
11 changes: 11 additions & 0 deletions pkg/types/azure/machinepool.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,13 @@ type OSDisk struct {
//
// +kubebuilder:validation:Minimum=0
DiskSizeGB int32 `json:"diskSizeGB"`
// DiskType defines the type of disk.
// The valid values are Standard_LRS, Premium_LRS, StandardSSD_LRS
// For control plane nodes, the valid values are Premium_LRS and StandardSSD_LRS.
// Default is Premium_LRS
// +optional
// +kubebuilder:validation:Enum=Standard_LRS;Premium_LRS;StandardSSD_LRS
DiskType string `json:"diskType"`
}

// Set sets the values from `required` to `a`.
Expand All @@ -46,4 +53,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
}
}
Loading

0 comments on commit 8303d48

Please sign in to comment.