From e3830a7045f49317ec9ff54632fd8befc27a3603 Mon Sep 17 00:00:00 2001 From: Michael Weibel Date: Tue, 18 Jun 2024 14:37:04 +0200 Subject: [PATCH] feat: support OSDisk.DiffDiskPlacement --- api/v1beta1/azuremachine_validation.go | 22 ++++++ api/v1beta1/azuremachine_validation_test.go | 47 ++++++++++++ api/v1beta1/types.go | 33 ++++++++ api/v1beta1/zz_generated.deepcopy.go | 7 +- azure/converters/spotinstances_test.go | 15 ++++ azure/services/scalesets/spec.go | 4 + azure/services/scalesets/spec_test.go | 34 +++++++++ azure/services/virtualmachines/spec.go | 5 ++ azure/services/virtualmachines/spec_test.go | 29 +++++++ ...re.cluster.x-k8s.io_azuremachinepools.yaml | 9 +++ ...ucture.cluster.x-k8s.io_azuremachines.yaml | 9 +++ ...luster.x-k8s.io_azuremachinetemplates.yaml | 9 +++ exp/api/v1beta1/azuremachinepool_default.go | 11 +++ .../v1beta1/azuremachinepool_default_test.go | 4 + exp/api/v1beta1/azuremachinepool_test.go | 39 +++++++++- exp/api/v1beta1/azuremachinepool_webhook.go | 12 ++- .../v1beta1/azuremachinepool_webhook_test.go | 75 +++++++++++++++++++ 17 files changed, 360 insertions(+), 4 deletions(-) diff --git a/api/v1beta1/azuremachine_validation.go b/api/v1beta1/azuremachine_validation.go index 972e9816b3c..890cbab70fb 100644 --- a/api/v1beta1/azuremachine_validation.go +++ b/api/v1beta1/azuremachine_validation.go @@ -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" @@ -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 } diff --git a/api/v1beta1/azuremachine_validation_test.go b/api/v1beta1/azuremachine_validation_test.go index 2621022f04f..7e8fc71b2ba 100644 --- a/api/v1beta1/azuremachine_validation_test.go +++ b/api/v1beta1/azuremachine_validation_test.go @@ -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, diff --git a/api/v1beta1/types.go b/api/v1beta1/types.go index 4cc74fb56f2..444bc50ca66 100644 --- a/api/v1beta1/types.go +++ b/api/v1beta1/types.go @@ -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,omitempty"` } // SubnetRole defines the unique role of a subnet. diff --git a/api/v1beta1/zz_generated.deepcopy.go b/api/v1beta1/zz_generated.deepcopy.go index 2b1fc5cff89..8667e7d5012 100644 --- a/api/v1beta1/zz_generated.deepcopy.go +++ b/api/v1beta1/zz_generated.deepcopy.go @@ -2305,6 +2305,11 @@ func (in *Diagnostics) DeepCopy() *Diagnostics { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *DiffDiskSettings) DeepCopyInto(out *DiffDiskSettings) { *out = *in + if in.Placement != nil { + in, out := &in.Placement, &out.Placement + *out = new(DiffDiskPlacement) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new DiffDiskSettings. @@ -3212,7 +3217,7 @@ func (in *OSDisk) DeepCopyInto(out *OSDisk) { if in.DiffDiskSettings != nil { in, out := &in.DiffDiskSettings, &out.DiffDiskSettings *out = new(DiffDiskSettings) - **out = **in + (*in).DeepCopyInto(*out) } } diff --git a/azure/converters/spotinstances_test.go b/azure/converters/spotinstances_test.go index 867530804ad..b94a29755f7 100644 --- a/azure/converters/spotinstances_test.go +++ b/azure/converters/spotinstances_test.go @@ -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{ diff --git a/azure/services/scalesets/spec.go b/azure/services/scalesets/spec.go index 516fafc4feb..c014d7db5c2 100644 --- a/azure/services/scalesets/spec.go +++ b/azure/services/scalesets/spec.go @@ -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 { diff --git a/azure/services/scalesets/spec_test.go b/azure/services/scalesets/spec_test.go index d87f74dcdc3..361741dab7b 100644 --- a/azure/services/scalesets/spec_test.go +++ b/azure/services/scalesets/spec_test.go @@ -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() @@ -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 @@ -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, diff --git a/azure/services/virtualmachines/spec.go b/azure/services/virtualmachines/spec.go index 391d4537b45..b60e88e81d0 100644 --- a/azure/services/virtualmachines/spec.go +++ b/azure/services/virtualmachines/spec.go @@ -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) { @@ -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 { diff --git a/azure/services/virtualmachines/spec_test.go b/azure/services/virtualmachines/spec_test.go index 3a20db0617c..1e5613b74c2 100644 --- a/azure/services/virtualmachines/spec_test.go +++ b/azure/services/virtualmachines/spec_test.go @@ -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{ diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremachinepools.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremachinepools.yaml index 74f43e986cd..f01fe1e4040 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremachinepools.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremachinepools.yaml @@ -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 diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremachines.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremachines.yaml index cc80014c7b0..67a7ce6af1a 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremachines.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremachines.yaml @@ -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 diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremachinetemplates.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremachinetemplates.yaml index 60f3f10bb26..3040fdd9fd4 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremachinetemplates.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremachinetemplates.yaml @@ -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 diff --git a/exp/api/v1beta1/azuremachinepool_default.go b/exp/api/v1beta1/azuremachinepool_default.go index e4009be4bcb..aeeddd1f65c 100644 --- a/exp/api/v1beta1/azuremachinepool_default.go +++ b/exp/api/v1beta1/azuremachinepool_default.go @@ -42,6 +42,7 @@ func (amp *AzureMachinePool) SetDefaults(client client.Client) error { } amp.SetDiagnosticsDefaults() amp.SetNetworkInterfacesDefaults() + amp.SetOSDiskDefaults() return kerrors.NewAggregate(errs) } @@ -159,3 +160,13 @@ func (amp *AzureMachinePool) SetNetworkInterfacesDefaults() { } } } + +// SetOSDiskDefaults sets the defaults for the OSDisk. +func (amp *AzureMachinePool) SetOSDiskDefaults() { + if amp.Spec.Template.OSDisk.OSType == "" { + amp.Spec.Template.OSDisk.OSType = "Linux" + } + if amp.Spec.Template.OSDisk.CachingType == "" { + amp.Spec.Template.OSDisk.CachingType = "None" + } +} diff --git a/exp/api/v1beta1/azuremachinepool_default_test.go b/exp/api/v1beta1/azuremachinepool_default_test.go index 22e462bdbe2..65adc0be3d8 100644 --- a/exp/api/v1beta1/azuremachinepool_default_test.go +++ b/exp/api/v1beta1/azuremachinepool_default_test.go @@ -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{ diff --git a/exp/api/v1beta1/azuremachinepool_test.go b/exp/api/v1beta1/azuremachinepool_test.go index c430d59f685..72d4441744f 100644 --- a/exp/api/v1beta1/azuremachinepool_test.go +++ b/exp/api/v1beta1/azuremachinepool_test.go @@ -34,7 +34,16 @@ func TestAzureMachinePool_Validate(t *testing.T) { { Name: "HasNoImage", Factory: func(_ *gomega.GomegaWithT) *infrav1exp.AzureMachinePool { - return new(infrav1exp.AzureMachinePool) + return &infrav1exp.AzureMachinePool{ + Spec: infrav1exp.AzureMachinePoolSpec{ + Template: infrav1exp.AzureMachinePoolMachineTemplate{ + OSDisk: infrav1.OSDisk{ + OSType: "Linux", + CachingType: "None", + }, + }, + }, + } }, Expect: func(g *gomega.GomegaWithT, actual error) { g.Expect(actual).NotTo(gomega.HaveOccurred()) @@ -55,6 +64,10 @@ func TestAzureMachinePool_Validate(t *testing.T) { Version: "1.2.3", }, }, + OSDisk: infrav1.OSDisk{ + OSType: "Linux", + CachingType: "None", + }, }, }, } @@ -70,6 +83,10 @@ func TestAzureMachinePool_Validate(t *testing.T) { Spec: infrav1exp.AzureMachinePoolSpec{ Template: infrav1exp.AzureMachinePoolMachineTemplate{ Image: new(infrav1.Image), + OSDisk: infrav1.OSDisk{ + OSType: "Linux", + CachingType: "None", + }, }, }, } @@ -86,6 +103,10 @@ func TestAzureMachinePool_Validate(t *testing.T) { Spec: infrav1exp.AzureMachinePoolSpec{ Template: infrav1exp.AzureMachinePoolMachineTemplate{ TerminateNotificationTimeout: ptr.To(7), + OSDisk: infrav1.OSDisk{ + OSType: "Linux", + CachingType: "None", + }, }, }, } @@ -101,6 +122,10 @@ func TestAzureMachinePool_Validate(t *testing.T) { Spec: infrav1exp.AzureMachinePoolSpec{ Template: infrav1exp.AzureMachinePoolMachineTemplate{ TerminateNotificationTimeout: ptr.To(20), + OSDisk: infrav1.OSDisk{ + OSType: "Linux", + CachingType: "None", + }, }, }, } @@ -117,6 +142,10 @@ func TestAzureMachinePool_Validate(t *testing.T) { Spec: infrav1exp.AzureMachinePoolSpec{ Template: infrav1exp.AzureMachinePoolMachineTemplate{ TerminateNotificationTimeout: ptr.To(3), + OSDisk: infrav1.OSDisk{ + OSType: "Linux", + CachingType: "None", + }, }, }, } @@ -133,6 +162,10 @@ func TestAzureMachinePool_Validate(t *testing.T) { Spec: infrav1exp.AzureMachinePoolSpec{ Template: infrav1exp.AzureMachinePoolMachineTemplate{ Diagnostics: nil, + OSDisk: infrav1.OSDisk{ + OSType: "Linux", + CachingType: "None", + }, }, }, } @@ -152,6 +185,10 @@ func TestAzureMachinePool_Validate(t *testing.T) { StorageAccountType: infrav1.ManagedDiagnosticsStorage, }, }, + OSDisk: infrav1.OSDisk{ + OSType: "Linux", + CachingType: "None", + }, }, }, } diff --git a/exp/api/v1beta1/azuremachinepool_webhook.go b/exp/api/v1beta1/azuremachinepool_webhook.go index 15c7af5ee3b..047b3f777bd 100644 --- a/exp/api/v1beta1/azuremachinepool_webhook.go +++ b/exp/api/v1beta1/azuremachinepool_webhook.go @@ -110,6 +110,7 @@ func (amp *AzureMachinePool) Validate(old runtime.Object, client client.Client) amp.ValidateSystemAssignedIdentity(old), amp.ValidateSystemAssignedIdentityRole, amp.ValidateNetwork, + amp.ValidateOSDisk, } var errs []error @@ -130,13 +131,20 @@ func (amp *AzureMachinePool) ValidateNetwork() error { return nil } +// ValidateOSDisk of an AzureMachinePool. +func (amp *AzureMachinePool) ValidateOSDisk() error { + if errs := infrav1.ValidateOSDisk(amp.Spec.Template.OSDisk, field.NewPath("osDisk")); len(errs) > 0 { + return errs.ToAggregate() + } + return nil +} + // ValidateImage of an AzureMachinePool. func (amp *AzureMachinePool) ValidateImage() error { if amp.Spec.Template.Image != nil { image := amp.Spec.Template.Image if errs := infrav1.ValidateImage(image, field.NewPath("image")); len(errs) > 0 { - agg := kerrors.NewAggregate(errs.ToAggregate().Errors()) - return agg + return errs.ToAggregate() } } diff --git a/exp/api/v1beta1/azuremachinepool_webhook_test.go b/exp/api/v1beta1/azuremachinepool_webhook_test.go index 49079898577..4714a7768e9 100644 --- a/exp/api/v1beta1/azuremachinepool_webhook_test.go +++ b/exp/api/v1beta1/azuremachinepool_webhook_test.go @@ -233,6 +233,21 @@ func TestAzureMachinePool_ValidateCreate(t *testing.T) { ownerNotFound: true, wantErr: true, }, + { + name: "azuremachinepool with invalid DiffDiskSettings", + amp: createMachinePoolWithDiffDiskSettings(infrav1.DiffDiskSettings{ + Placement: ptr.To(infrav1.DiffDiskPlacementResourceDisk), + }), + wantErr: true, + }, + { + name: "azuremachinepool with valid DiffDiskSettings", + amp: createMachinePoolWithDiffDiskSettings(infrav1.DiffDiskSettings{ + Option: string(armcompute.DiffDiskOptionsLocal), + Placement: ptr.To(infrav1.DiffDiskPlacementResourceDisk), + }), + wantErr: true, + }, } for _, tc := range tests { @@ -492,6 +507,10 @@ func createMachinePoolWithMarketPlaceImage(publisher, offer, sku, version string Image: &image, SSHPublicKey: validSSHPublicKey, TerminateNotificationTimeout: terminateNotificationTimeout, + OSDisk: infrav1.OSDisk{ + CachingType: "None", + OSType: "Linux", + }, }, }, } @@ -514,6 +533,10 @@ func createMachinePoolWithSharedImage(subscriptionID, resourceGroup, name, galle Image: &image, SSHPublicKey: validSSHPublicKey, TerminateNotificationTimeout: terminateNotificationTimeout, + OSDisk: infrav1.OSDisk{ + CachingType: "None", + OSType: "Linux", + }, }, }, } @@ -525,6 +548,10 @@ func createMachinePoolWithNetworkConfig(subnetName string, interfaces []infrav1. Template: AzureMachinePoolMachineTemplate{ SubnetName: subnetName, NetworkInterfaces: interfaces, + OSDisk: infrav1.OSDisk{ + CachingType: "None", + OSType: "Linux", + }, }, }, } @@ -541,6 +568,10 @@ func createMachinePoolWithImageByID(imageID string, terminateNotificationTimeout Image: &image, SSHPublicKey: validSSHPublicKey, TerminateNotificationTimeout: terminateNotificationTimeout, + OSDisk: infrav1.OSDisk{ + CachingType: "None", + OSType: "Linux", + }, }, }, } @@ -555,6 +586,12 @@ func createMachinePoolWithSystemAssignedIdentity(role string) *AzureMachinePool Scope: "scope", DefinitionID: "definitionID", }, + Template: AzureMachinePoolMachineTemplate{ + OSDisk: infrav1.OSDisk{ + CachingType: "None", + OSType: "Linux", + }, + }, }, } } @@ -578,6 +615,10 @@ func createMachinePoolWithDiagnostics(diagnosticsType infrav1.BootDiagnosticsSto Spec: AzureMachinePoolSpec{ Template: AzureMachinePoolMachineTemplate{ Diagnostics: diagnostics, + OSDisk: infrav1.OSDisk{ + CachingType: "None", + OSType: "Linux", + }, }, }, } @@ -596,6 +637,12 @@ func createMachinePoolWithUserAssignedIdentity(providerIds []string) *AzureMachi Spec: AzureMachinePoolSpec{ Identity: infrav1.VMIdentityUserAssigned, UserAssignedIdentities: userAssignedIdentities, + Template: AzureMachinePoolMachineTemplate{ + OSDisk: infrav1.OSDisk{ + CachingType: "None", + OSType: "Linux", + }, + }, }, } } @@ -613,6 +660,12 @@ func createMachinePoolWithStrategy(strategy AzureMachinePoolDeploymentStrategy) return &AzureMachinePool{ Spec: AzureMachinePoolSpec{ Strategy: strategy, + Template: AzureMachinePoolMachineTemplate{ + OSDisk: infrav1.OSDisk{ + CachingType: "None", + OSType: "Linux", + }, + }, }, } } @@ -621,6 +674,24 @@ func createMachinePoolWithOrchestrationMode(mode armcompute.OrchestrationMode) * return &AzureMachinePool{ Spec: AzureMachinePoolSpec{ OrchestrationMode: infrav1.OrchestrationModeType(mode), + Template: AzureMachinePoolMachineTemplate{ + OSDisk: infrav1.OSDisk{ + CachingType: "None", + OSType: "Linux", + }, + }, + }, + } +} + +func createMachinePoolWithDiffDiskSettings(settings infrav1.DiffDiskSettings) *AzureMachinePool { + return &AzureMachinePool{ + Spec: AzureMachinePoolSpec{ + Template: AzureMachinePoolMachineTemplate{ + OSDisk: infrav1.OSDisk{ + DiffDiskSettings: &settings, + }, + }, }, } } @@ -678,6 +749,10 @@ func getKnownValidAzureMachinePool() *AzureMachinePool { Image: &image, SSHPublicKey: validSSHPublicKey, TerminateNotificationTimeout: ptr.To(10), + OSDisk: infrav1.OSDisk{ + CachingType: "None", + OSType: "Linux", + }, }, Identity: infrav1.VMIdentitySystemAssigned, SystemAssignedIdentityRole: &infrav1.SystemAssignedIdentityRole{