Skip to content

Commit

Permalink
feat: support OSDisk.DiffDiskPlacement
Browse files Browse the repository at this point in the history
  • Loading branch information
mweibel committed Jun 18, 2024
1 parent a52056d commit 4ff8586
Show file tree
Hide file tree
Showing 17 changed files with 357 additions and 4 deletions.
22 changes: 22 additions & 0 deletions api/v1beta1/azuremachine_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ package v1beta1
import (
"encoding/base64"
"fmt"
"slices"
"strings"

"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v5"
"github.com/google/uuid"
Expand Down Expand Up @@ -248,6 +250,26 @@ func ValidateOSDisk(osDisk OSDisk, fieldPath *field.Path) field.ErrorList {
"diskEncryptionSet is not supported when diffDiskSettings.option is 'Local'",
))
}
if osDisk.DiffDiskSettings != nil && osDisk.DiffDiskSettings.Placement != nil {
if osDisk.DiffDiskSettings.Option != string(armcompute.DiffDiskOptionsLocal) {
allErrs = append(allErrs, field.Invalid(
fieldPath.Child("diffDiskSettings"),
osDisk.DiffDiskSettings,
"placement is only supported when diffDiskSettings.option is 'Local'",
))
}
if !slices.Contains(PossibleDiffDiskPlacementValues(), *osDisk.DiffDiskSettings.Placement) {
options := make([]string, len(PossibleDiffDiskPlacementValues()))
for i, value := range PossibleDiffDiskPlacementValues() {
options[i] = string(value)
}
allErrs = append(allErrs, field.Invalid(
fieldPath.Child("diffDiskSettings").Child("placement"),
osDisk.DiffDiskSettings.Placement,
"placement must be one of "+strings.Join(options, ","),
))
}
}

return allErrs
}
Expand Down
47 changes: 47 additions & 0 deletions api/v1beta1/azuremachine_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,53 @@ func TestAzureMachine_ValidateOSDisk(t *testing.T) {
},
},
},
{
name: "valid resourceDisk placement spec with option local",
wantErr: false,
osDisk: OSDisk{
DiskSizeGB: ptr.To[int32](30),
CachingType: "None",
OSType: "blah",
DiffDiskSettings: &DiffDiskSettings{
Option: string(armcompute.DiffDiskOptionsLocal),
Placement: ptr.To(DiffDiskPlacementResourceDisk),
},
ManagedDisk: &ManagedDiskParameters{
StorageAccountType: "Standard_LRS",
},
},
},
{
name: "valid resourceDisk placement spec requires option local",
wantErr: true,
osDisk: OSDisk{
DiskSizeGB: ptr.To[int32](30),
CachingType: "None",
OSType: "blah",
DiffDiskSettings: &DiffDiskSettings{
Placement: ptr.To(DiffDiskPlacementResourceDisk),
},
ManagedDisk: &ManagedDiskParameters{
StorageAccountType: "Standard_LRS",
},
},
},
{
name: "invalid resourceDisk placement spec",
wantErr: true,
osDisk: OSDisk{
DiskSizeGB: ptr.To[int32](30),
CachingType: "None",
OSType: "blah",
DiffDiskSettings: &DiffDiskSettings{
Option: string(armcompute.DiffDiskOptionsLocal),
Placement: ptr.To(DiffDiskPlacement("invalidDisk")),
},
ManagedDisk: &ManagedDiskParameters{
StorageAccountType: "Standard_LRS",
},
},
},
{
name: "byoc encryption with ephemeral os disk spec",
wantErr: true,
Expand Down
33 changes: 33 additions & 0 deletions api/v1beta1/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -687,12 +687,45 @@ type DiskEncryptionSetParameters struct {
ID string `json:"id,omitempty"`
}

// DiffDiskPlacement - Specifies the ephemeral disk placement for operating system disk. This property can be used by user
// in the request to choose the location i.e, cache disk, resource disk or nvme disk space for
// Ephemeral OS disk provisioning. For more information on Ephemeral OS disk size requirements, please refer Ephemeral OS
// disk size requirements for Windows VM at
// https://docs.microsoft.com/azure/virtual-machines/windows/ephemeral-os-disks#size-requirements and Linux VM at
// https://docs.microsoft.com/azure/virtual-machines/linux/ephemeral-os-disks#size-requirements.
type DiffDiskPlacement string

const (
// DiffDiskPlacementCacheDisk places the OsDisk on cache disk.
DiffDiskPlacementCacheDisk DiffDiskPlacement = "CacheDisk"

// DiffDiskPlacementNvmeDisk places the OsDisk on NVMe disk.
DiffDiskPlacementNvmeDisk DiffDiskPlacement = "NvmeDisk"

// DiffDiskPlacementResourceDisk places the OsDisk on temp disk.
DiffDiskPlacementResourceDisk DiffDiskPlacement = "ResourceDisk"
)

// PossibleDiffDiskPlacementValues returns the possible values for the DiffDiskPlacement const type.
func PossibleDiffDiskPlacementValues() []DiffDiskPlacement {
return []DiffDiskPlacement{
DiffDiskPlacementCacheDisk,
DiffDiskPlacementNvmeDisk,
DiffDiskPlacementResourceDisk,
}
}

// DiffDiskSettings describe ephemeral disk settings for the os disk.
type DiffDiskSettings struct {
// Option enables ephemeral OS when set to "Local"
// See https://learn.microsoft.com/azure/virtual-machines/ephemeral-os-disks for full details
// +kubebuilder:validation:Enum=Local
Option string `json:"option"`

// Placement specifies the ephemeral disk placement for operating system disk. If placement is specified, Option must be set to "Local".
// +kubebuilder:validation:Enum=CacheDisk;NvmeDisk;ResourceDisk
// +optional
Placement *DiffDiskPlacement `json:"placement"`
}

// SubnetRole defines the unique role of a subnet.
Expand Down
7 changes: 6 additions & 1 deletion api/v1beta1/zz_generated.deepcopy.go

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

15 changes: 15 additions & 0 deletions azure/converters/spotinstances_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,21 @@ func TestGetSpotVMOptions(t *testing.T) {
billingProfile: nil,
},
},
{
name: "spot with ResourceDisk",
spot: &infrav1.SpotVMOptions{
MaxPrice: nil,
},
diffDiskSettings: &infrav1.DiffDiskSettings{
Option: string(armcompute.DiffDiskOptionsLocal),
Placement: ptr.To(infrav1.DiffDiskPlacementResourceDisk),
},
want: resultParams{
vmPriorityTypes: ptr.To(armcompute.VirtualMachinePriorityTypesSpot),
vmEvictionPolicyTypes: nil,
billingProfile: nil,
},
},
{
name: "spot with eviction policy",
spot: &infrav1.SpotVMOptions{
Expand Down
4 changes: 4 additions & 0 deletions azure/services/scalesets/spec.go
Original file line number Diff line number Diff line change
Expand Up @@ -391,6 +391,10 @@ func (s *ScaleSetSpec) generateStorageProfile(ctx context.Context) (*armcompute.
storageProfile.OSDisk.DiffDiskSettings = &armcompute.DiffDiskSettings{
Option: ptr.To(armcompute.DiffDiskOptions(s.OSDisk.DiffDiskSettings.Option)),
}

if s.OSDisk.DiffDiskSettings.Placement != nil {
storageProfile.OSDisk.DiffDiskSettings.Placement = ptr.To(armcompute.DiffDiskPlacement(*s.OSDisk.DiffDiskSettings.Placement))
}
}

if s.OSDisk.ManagedDisk != nil {
Expand Down
34 changes: 34 additions & 0 deletions azure/services/scalesets/spec_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ var (
customNetworkingSpec, customNetworkingVMSS = getCustomNetworkingVMSS()
spotVMSpec, spotVMVMSS = getSpotVMVMSS()
ephemeralSpec, ephemeralVMSS = getEPHVMSSS()
resourceDiskSpec, resourceDiskVMSS = getResourceDiskVMSS()
evictionSpec, evictionVMSS = getEvictionPolicyVMSS()
maxPriceSpec, maxPriceVMSS = getMaxPriceVMSS()
encryptionSpec, encryptionVMSS = getEncryptionVMSS()
Expand Down Expand Up @@ -233,6 +234,32 @@ func getEPHVMSSS() (ScaleSetSpec, armcompute.VirtualMachineScaleSet) {
return spec, vmss
}

func getResourceDiskVMSS() (ScaleSetSpec, armcompute.VirtualMachineScaleSet) {
spec := newDefaultVMSSSpec()
spec.Size = vmSizeEPH
spec.SKU = resourceskus.SKU{
Capabilities: []*armcompute.ResourceSKUCapabilities{
{
Name: ptr.To(resourceskus.EphemeralOSDisk),
Value: ptr.To("True"),
},
},
}
spec.SpotVMOptions = &infrav1.SpotVMOptions{}
spec.OSDisk.DiffDiskSettings = &infrav1.DiffDiskSettings{
Option: string(armcompute.DiffDiskOptionsLocal),
Placement: ptr.To(infrav1.DiffDiskPlacementResourceDisk),
}
vmss := newDefaultVMSS(vmSizeEPH)
vmss.Properties.VirtualMachineProfile.StorageProfile.OSDisk.DiffDiskSettings = &armcompute.DiffDiskSettings{
Option: ptr.To(armcompute.DiffDiskOptionsLocal),
Placement: ptr.To(armcompute.DiffDiskPlacementResourceDisk),
}
vmss.Properties.VirtualMachineProfile.Priority = ptr.To(armcompute.VirtualMachinePriorityTypesSpot)

return spec, vmss
}

func getEvictionPolicyVMSS() (ScaleSetSpec, armcompute.VirtualMachineScaleSet) {
spec := newDefaultVMSSSpec()
spec.Size = vmSizeEPH
Expand Down Expand Up @@ -586,6 +613,13 @@ func TestScaleSetParameters(t *testing.T) {
expected: ephemeralVMSS,
expectedError: "",
},
{
name: "spot vm and ephemeral disk with resourceDisk placement vmss",
spec: resourceDiskSpec,
existing: nil,
expected: resourceDiskVMSS,
expectedError: "",
},
{
name: "spot vm and eviction policy vmss",
spec: evictionSpec,
Expand Down
5 changes: 5 additions & 0 deletions azure/services/virtualmachines/spec.go
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,7 @@ func (s *VMSpec) generateStorageProfile() (*armcompute.StorageProfile, error) {
if !MemoryCapability {
return nil, azure.WithTerminalError(errors.New("VM memory should be bigger or equal to at least 2Gi"))
}

// enable ephemeral OS
if s.OSDisk.DiffDiskSettings != nil {
if !s.SKU.HasCapability(resourceskus.EphemeralOSDisk) {
Expand All @@ -192,6 +193,10 @@ func (s *VMSpec) generateStorageProfile() (*armcompute.StorageProfile, error) {
storageProfile.OSDisk.DiffDiskSettings = &armcompute.DiffDiskSettings{
Option: ptr.To(armcompute.DiffDiskOptions(s.OSDisk.DiffDiskSettings.Option)),
}

if s.OSDisk.DiffDiskSettings.Placement != nil {
storageProfile.OSDisk.DiffDiskSettings.Placement = ptr.To(armcompute.DiffDiskPlacement(*s.OSDisk.DiffDiskSettings.Placement))
}
}

if s.OSDisk.ManagedDisk != nil {
Expand Down
29 changes: 29 additions & 0 deletions azure/services/virtualmachines/spec_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -461,6 +461,35 @@ func TestParameters(t *testing.T) {
},
expectedError: "",
},
{
name: "can create a vm with DiffDiskPlacement ResourceDisk",
spec: &VMSpec{
Name: "my-vm",
Role: infrav1.Node,
NICIDs: []string{"my-nic"},
SSHKeyData: "fakesshpublickey",
Size: "Standard_D2v3",
OSDisk: infrav1.OSDisk{
OSType: "Linux",
DiskSizeGB: ptr.To[int32](128),
ManagedDisk: &infrav1.ManagedDiskParameters{
StorageAccountType: string(armcompute.StorageAccountTypesPremiumLRS),
},
DiffDiskSettings: &infrav1.DiffDiskSettings{
Option: string(armcompute.DiffDiskOptionsLocal),
Placement: ptr.To(infrav1.DiffDiskPlacementResourceDisk),
},
},
Image: &infrav1.Image{ID: ptr.To("fake-image-id")},
SKU: validSKUWithEphemeralOS,
},
existing: nil,
expect: func(g *WithT, result interface{}) {
g.Expect(result).To(BeAssignableToTypeOf(armcompute.VirtualMachine{}))
g.Expect(result.(armcompute.VirtualMachine).Properties.StorageProfile.OSDisk.DiffDiskSettings.Placement).To(Equal(ptr.To(armcompute.DiffDiskPlacementResourceDisk)))
},
expectedError: "",
},
{
name: "can create a trusted launch vm",
spec: &VMSpec{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -578,6 +578,15 @@ spec:
enum:
- Local
type: string
placement:
description: Placement specifies the ephemeral disk placement
for operating system disk. If placement is specified,
Option must be set to "Local".
enum:
- CacheDisk
- NvmeDisk
- ResourceDisk
type: string
required:
- option
type: object
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -498,6 +498,15 @@ spec:
enum:
- Local
type: string
placement:
description: Placement specifies the ephemeral disk placement
for operating system disk. If placement is specified, Option
must be set to "Local".
enum:
- CacheDisk
- NvmeDisk
- ResourceDisk
type: string
required:
- option
type: object
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -513,6 +513,15 @@ spec:
enum:
- Local
type: string
placement:
description: Placement specifies the ephemeral disk
placement for operating system disk. If placement
is specified, Option must be set to "Local".
enum:
- CacheDisk
- NvmeDisk
- ResourceDisk
type: string
required:
- option
type: object
Expand Down
8 changes: 8 additions & 0 deletions exp/api/v1beta1/azuremachinepool_default.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ func (amp *AzureMachinePool) SetDefaults(client client.Client) error {
}
amp.SetDiagnosticsDefaults()
amp.SetNetworkInterfacesDefaults()
amp.SetOSDiskDefaults()

return kerrors.NewAggregate(errs)
}
Expand Down Expand Up @@ -159,3 +160,10 @@ func (amp *AzureMachinePool) SetNetworkInterfacesDefaults() {
}
}
}

// SetOSDiskDefaults sets the defaults for the OSDisk.
func (amp *AzureMachinePool) SetOSDiskDefaults() {
if amp.Spec.Template.OSDisk.CachingType == "" {
amp.Spec.Template.OSDisk.CachingType = "None"
}
}
4 changes: 4 additions & 0 deletions exp/api/v1beta1/azuremachinepool_default_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -413,6 +413,10 @@ func hardcodedAzureMachinePoolWithSSHKey(sshPublicKey string) *AzureMachinePool
Spec: AzureMachinePoolSpec{
Template: AzureMachinePoolMachineTemplate{
SSHPublicKey: sshPublicKey,
OSDisk: infrav1.OSDisk{
CachingType: "None",
OSType: "Linux",
},
},
},
ObjectMeta: metav1.ObjectMeta{
Expand Down
Loading

0 comments on commit 4ff8586

Please sign in to comment.