From 30ba1e1e2713218df3ab49bdaf4ae25bdaf12564 Mon Sep 17 00:00:00 2001 From: Cecile Robert-Michon Date: Thu, 15 Apr 2021 13:37:51 -0700 Subject: [PATCH 1/2] :sparkles: Make ManagedDisk options optional --- api/v1alpha3/azurecluster_conversion.go | 5 - api/v1alpha3/azuremachine_conversion.go | 52 +++++++++ .../azuremachinetemplate_conversion.go | 8 ++ api/v1alpha3/zz_generated.conversion.go | 107 +++--------------- api/v1alpha4/azuremachine_validation.go | 83 +++++++------- api/v1alpha4/azuremachine_validation_test.go | 46 ++++---- api/v1alpha4/azuremachine_webhook.go | 6 +- api/v1alpha4/azuremachine_webhook_test.go | 2 +- api/v1alpha4/types.go | 24 ++-- api/v1alpha4/zz_generated.deepcopy.go | 16 ++- azure/services/scalesets/scalesets.go | 13 ++- azure/services/scalesets/scalesets_test.go | 6 +- .../virtualmachines/virtualmachines.go | 15 ++- .../virtualmachines/virtualmachines_test.go | 22 ++-- ...re.cluster.x-k8s.io_azuremachinepools.yaml | 9 +- ...ucture.cluster.x-k8s.io_azuremachines.yaml | 9 +- ...luster.x-k8s.io_azuremachinetemplates.yaml | 9 +- docs/book/src/SUMMARY.md | 4 +- docs/book/src/topics/data-disks.md | 2 +- docs/book/src/topics/failure-domains.md | 2 - .../topics/{ephemeral-os.md => os-disk.md} | 28 ++++- .../v1alpha3/azuremachinepool_conversion.go | 9 ++ templates/cluster-template-aad.yaml | 2 - templates/cluster-template-ephemeral.yaml | 4 - ...ster-template-external-cloud-provider.yaml | 4 - templates/cluster-template-ipv6.yaml | 2 - ...-machinepool-system-assigned-identity.yaml | 2 - ...te-machinepool-user-assigned-identity.yaml | 2 - .../cluster-template-machinepool-windows.yaml | 2 - templates/cluster-template-machinepool.yaml | 2 - templates/cluster-template-multi-tenancy.yaml | 4 - templates/cluster-template-nvidia-gpu.yaml | 2 - templates/cluster-template-private.yaml | 4 - ...ter-template-system-assigned-identity.yaml | 4 - ...uster-template-user-assigned-identity.yaml | 4 - templates/cluster-template-windows.yaml | 2 - templates/cluster-template.yaml | 4 - templates/flavors/base/cluster-template.yaml | 2 - .../flavors/default/machine-deployment.yaml | 2 - .../flavors/ephemeral/patches/ephemeral.yaml | 7 +- .../ci/cluster-template-prow-ci-version.yaml | 4 - ...template-prow-external-cloud-provider.yaml | 4 - .../test/ci/cluster-template-prow-ipv6.yaml | 2 - ...template-prow-machine-pool-ci-version.yaml | 2 - ...er-template-prow-machine-pool-windows.yaml | 2 - .../cluster-template-prow-machine-pool.yaml | 2 - .../cluster-template-prow-multi-tenancy.yaml | 4 - .../ci/cluster-template-prow-nvidia-gpu.yaml | 2 - .../ci/cluster-template-prow-private.yaml | 4 - .../ci/cluster-template-prow-windows.yaml | 2 - templates/test/ci/cluster-template-prow.yaml | 4 - .../dev/cluster-template-custom-builds.yaml | 4 - 52 files changed, 247 insertions(+), 321 deletions(-) rename docs/book/src/topics/{ephemeral-os.md => os-disk.md} (66%) diff --git a/api/v1alpha3/azurecluster_conversion.go b/api/v1alpha3/azurecluster_conversion.go index e6b098b5011..bcedc4a174d 100644 --- a/api/v1alpha3/azurecluster_conversion.go +++ b/api/v1alpha3/azurecluster_conversion.go @@ -267,11 +267,6 @@ func Convert_v1alpha4_SecurityRule_To_v1alpha3_IngressRule(in *infrav1alpha4.Sec return nil } -// Convert_v1alpha4_ManagedDisk_To_v1alpha3_ManagedDisk converts between api versions -func Convert_v1alpha4_ManagedDisk_To_v1alpha3_ManagedDisk(in *infrav1alpha4.ManagedDisk, out *ManagedDisk, s apiconversion.Scope) error { - return autoConvert_v1alpha4_ManagedDisk_To_v1alpha3_ManagedDisk(in, out, s) -} - // Convert_v1alpha3_APIEndpoint_To_v1alpha4_APIEndpoint is an autogenerated conversion function. func Convert_v1alpha3_APIEndpoint_To_v1alpha4_APIEndpoint(in *apiv1alpha3.APIEndpoint, out *apiv1alpha4.APIEndpoint, s apiconversion.Scope) error { return apiv1alpha3.Convert_v1alpha3_APIEndpoint_To_v1alpha4_APIEndpoint(in, out, s) diff --git a/api/v1alpha3/azuremachine_conversion.go b/api/v1alpha3/azuremachine_conversion.go index 32444e5ba2f..4c9cf77287b 100644 --- a/api/v1alpha3/azuremachine_conversion.go +++ b/api/v1alpha3/azuremachine_conversion.go @@ -37,6 +37,14 @@ func (src *AzureMachine) ConvertTo(dstRaw conversion.Hub) error { // nolint return err } + // Handle special case for conversion of ManagedDisk to pointer. + if restored.Spec.OSDisk.ManagedDisk == nil && dst.Spec.OSDisk.ManagedDisk != nil { + if *dst.Spec.OSDisk.ManagedDisk == (v1alpha4.ManagedDiskParameters{}) { + // restore nil value if nothing has changed since conversion + dst.Spec.OSDisk.ManagedDisk = nil + } + } + return nil } @@ -101,3 +109,47 @@ func Convert_v1alpha4_AzureMachineStatus_To_v1alpha3_AzureMachineStatus(in *v1al return nil } + +// Convert_v1alpha3_OSDisk_To_v1alpha4_OSDisk converts this OSDisk to the Hub version (v1alpha4). +func Convert_v1alpha3_OSDisk_To_v1alpha4_OSDisk(in *OSDisk, out *v1alpha4.OSDisk, s apiconversion.Scope) error { // nolint + if err := autoConvert_v1alpha3_OSDisk_To_v1alpha4_OSDisk(in, out, s); err != nil { + return err + } + + out.ManagedDisk = &v1alpha4.ManagedDiskParameters{} + if err := Convert_v1alpha3_ManagedDisk_To_v1alpha4_ManagedDiskOptions(&in.ManagedDisk, out.ManagedDisk, s); err != nil { + return err + } + + return nil +} + +// Convert_v1alpha4_OSDisk_To_v1alpha3_OSDisk converts from the Hub version (v1alpha4) of the AzureMachineStatus to this version. +func Convert_v1alpha4_OSDisk_To_v1alpha3_OSDisk(in *v1alpha4.OSDisk, out *OSDisk, s apiconversion.Scope) error { // nolint + if err := autoConvert_v1alpha4_OSDisk_To_v1alpha3_OSDisk(in, out, s); err != nil { + return err + } + + if in.ManagedDisk != nil { + out.ManagedDisk = ManagedDisk{} + if err := Convert_v1alpha4_ManagedDiskOptions_To_v1alpha3_ManagedDisk(in.ManagedDisk, &out.ManagedDisk, s); err != nil { + return err + } + } + + return nil +} + +// Convert_v1alpha3_ManagedDisk_To_v1alpha4_ManagedDiskOptions converts this ManagedDisk to the Hub version (v1alpha4). +func Convert_v1alpha3_ManagedDisk_To_v1alpha4_ManagedDiskOptions(in *ManagedDisk, out *v1alpha4.ManagedDiskParameters, s apiconversion.Scope) error { // nolint + out.StorageAccountType = in.StorageAccountType + out.DiskEncryptionSet = (*v1alpha4.DiskEncryptionSetParameters)(in.DiskEncryptionSet) + return nil +} + +// Convert_v1alpha4_ManagedDiskOptions_To_v1alpha3_ManagedDisk converts from the Hub version (v1alpha4) of the ManagedDiskParameters to this version. +func Convert_v1alpha4_ManagedDiskOptions_To_v1alpha3_ManagedDisk(in *v1alpha4.ManagedDiskParameters, out *ManagedDisk, s apiconversion.Scope) error { // nolint + out.StorageAccountType = in.StorageAccountType + out.DiskEncryptionSet = (*DiskEncryptionSetParameters)(in.DiskEncryptionSet) + return nil +} diff --git a/api/v1alpha3/azuremachinetemplate_conversion.go b/api/v1alpha3/azuremachinetemplate_conversion.go index a26ae94ff2d..053a0489139 100644 --- a/api/v1alpha3/azuremachinetemplate_conversion.go +++ b/api/v1alpha3/azuremachinetemplate_conversion.go @@ -36,6 +36,14 @@ func (src *AzureMachineTemplate) ConvertTo(dstRaw conversion.Hub) error { // nol return err } + // Handle special case for conversion of ManagedDisk to pointer. + if restored.Spec.Template.Spec.OSDisk.ManagedDisk == nil && dst.Spec.Template.Spec.OSDisk.ManagedDisk != nil { + if *dst.Spec.Template.Spec.OSDisk.ManagedDisk == (infrav1alpha4.ManagedDiskParameters{}) { + // restore nil value if nothing has changed since conversion + dst.Spec.Template.Spec.OSDisk.ManagedDisk = nil + } + } + return nil } diff --git a/api/v1alpha3/zz_generated.conversion.go b/api/v1alpha3/zz_generated.conversion.go index 85a4632d44e..5f6ebffaf22 100644 --- a/api/v1alpha3/zz_generated.conversion.go +++ b/api/v1alpha3/zz_generated.conversion.go @@ -265,21 +265,6 @@ func RegisterConversions(s *runtime.Scheme) error { }); err != nil { return err } - if err := s.AddGeneratedConversionFunc((*ManagedDisk)(nil), (*v1alpha4.ManagedDisk)(nil), func(a, b interface{}, scope conversion.Scope) error { - return Convert_v1alpha3_ManagedDisk_To_v1alpha4_ManagedDisk(a.(*ManagedDisk), b.(*v1alpha4.ManagedDisk), scope) - }); err != nil { - return err - } - if err := s.AddGeneratedConversionFunc((*OSDisk)(nil), (*v1alpha4.OSDisk)(nil), func(a, b interface{}, scope conversion.Scope) error { - return Convert_v1alpha3_OSDisk_To_v1alpha4_OSDisk(a.(*OSDisk), b.(*v1alpha4.OSDisk), scope) - }); err != nil { - return err - } - if err := s.AddGeneratedConversionFunc((*v1alpha4.OSDisk)(nil), (*OSDisk)(nil), func(a, b interface{}, scope conversion.Scope) error { - return Convert_v1alpha4_OSDisk_To_v1alpha3_OSDisk(a.(*v1alpha4.OSDisk), b.(*OSDisk), scope) - }); err != nil { - return err - } if err := s.AddGeneratedConversionFunc((*PublicIPSpec)(nil), (*v1alpha4.PublicIPSpec)(nil), func(a, b interface{}, scope conversion.Scope) error { return Convert_v1alpha3_PublicIPSpec_To_v1alpha4_PublicIPSpec(a.(*PublicIPSpec), b.(*v1alpha4.PublicIPSpec), scope) }); err != nil { @@ -375,6 +360,11 @@ func RegisterConversions(s *runtime.Scheme) error { }); err != nil { return err } + if err := s.AddConversionFunc((*OSDisk)(nil), (*v1alpha4.OSDisk)(nil), func(a, b interface{}, scope conversion.Scope) error { + return Convert_v1alpha3_OSDisk_To_v1alpha4_OSDisk(a.(*OSDisk), b.(*v1alpha4.OSDisk), scope) + }); err != nil { + return err + } if err := s.AddConversionFunc((*SecurityGroup)(nil), (*v1alpha4.SecurityGroup)(nil), func(a, b interface{}, scope conversion.Scope) error { return Convert_v1alpha3_SecurityGroup_To_v1alpha4_SecurityGroup(a.(*SecurityGroup), b.(*v1alpha4.SecurityGroup), scope) }); err != nil { @@ -420,13 +410,13 @@ func RegisterConversions(s *runtime.Scheme) error { }); err != nil { return err } - if err := s.AddConversionFunc((*v1alpha4.ManagedDisk)(nil), (*ManagedDisk)(nil), func(a, b interface{}, scope conversion.Scope) error { - return Convert_v1alpha4_ManagedDisk_To_v1alpha3_ManagedDisk(a.(*v1alpha4.ManagedDisk), b.(*ManagedDisk), scope) + if err := s.AddConversionFunc((*v1alpha4.NetworkSpec)(nil), (*NetworkSpec)(nil), func(a, b interface{}, scope conversion.Scope) error { + return Convert_v1alpha4_NetworkSpec_To_v1alpha3_NetworkSpec(a.(*v1alpha4.NetworkSpec), b.(*NetworkSpec), scope) }); err != nil { return err } - if err := s.AddConversionFunc((*v1alpha4.NetworkSpec)(nil), (*NetworkSpec)(nil), func(a, b interface{}, scope conversion.Scope) error { - return Convert_v1alpha4_NetworkSpec_To_v1alpha3_NetworkSpec(a.(*v1alpha4.NetworkSpec), b.(*NetworkSpec), scope) + if err := s.AddConversionFunc((*v1alpha4.OSDisk)(nil), (*OSDisk)(nil), func(a, b interface{}, scope conversion.Scope) error { + return Convert_v1alpha4_OSDisk_To_v1alpha3_OSDisk(a.(*v1alpha4.OSDisk), b.(*OSDisk), scope) }); err != nil { return err } @@ -784,17 +774,7 @@ func autoConvert_v1alpha3_AzureMachineSpec_To_v1alpha4_AzureMachineSpec(in *Azur if err := Convert_v1alpha3_OSDisk_To_v1alpha4_OSDisk(&in.OSDisk, &out.OSDisk, s); err != nil { return err } - if in.DataDisks != nil { - in, out := &in.DataDisks, &out.DataDisks - *out = make([]v1alpha4.DataDisk, len(*in)) - for i := range *in { - if err := Convert_v1alpha3_DataDisk_To_v1alpha4_DataDisk(&(*in)[i], &(*out)[i], s); err != nil { - return err - } - } - } else { - out.DataDisks = nil - } + out.DataDisks = *(*[]v1alpha4.DataDisk)(unsafe.Pointer(&in.DataDisks)) // WARNING: in.Location requires manual conversion: does not exist in peer-type out.SSHPublicKey = in.SSHPublicKey out.AdditionalTags = *(*v1alpha4.Tags)(unsafe.Pointer(&in.AdditionalTags)) @@ -817,17 +797,7 @@ func autoConvert_v1alpha4_AzureMachineSpec_To_v1alpha3_AzureMachineSpec(in *v1al if err := Convert_v1alpha4_OSDisk_To_v1alpha3_OSDisk(&in.OSDisk, &out.OSDisk, s); err != nil { return err } - if in.DataDisks != nil { - in, out := &in.DataDisks, &out.DataDisks - *out = make([]DataDisk, len(*in)) - for i := range *in { - if err := Convert_v1alpha4_DataDisk_To_v1alpha3_DataDisk(&(*in)[i], &(*out)[i], s); err != nil { - return err - } - } - } else { - out.DataDisks = nil - } + out.DataDisks = *(*[]DataDisk)(unsafe.Pointer(&in.DataDisks)) out.SSHPublicKey = in.SSHPublicKey out.AdditionalTags = *(*Tags)(unsafe.Pointer(&in.AdditionalTags)) out.AllocatePublicIP = in.AllocatePublicIP @@ -1063,15 +1033,7 @@ func Convert_v1alpha4_BuildParams_To_v1alpha3_BuildParams(in *v1alpha4.BuildPara func autoConvert_v1alpha3_DataDisk_To_v1alpha4_DataDisk(in *DataDisk, out *v1alpha4.DataDisk, s conversion.Scope) error { out.NameSuffix = in.NameSuffix out.DiskSizeGB = in.DiskSizeGB - if in.ManagedDisk != nil { - in, out := &in.ManagedDisk, &out.ManagedDisk - *out = new(v1alpha4.ManagedDisk) - if err := Convert_v1alpha3_ManagedDisk_To_v1alpha4_ManagedDisk(*in, *out, s); err != nil { - return err - } - } else { - out.ManagedDisk = nil - } + out.ManagedDisk = (*v1alpha4.ManagedDiskParameters)(unsafe.Pointer(in.ManagedDisk)) out.Lun = (*int32)(unsafe.Pointer(in.Lun)) out.CachingType = in.CachingType return nil @@ -1085,15 +1047,7 @@ func Convert_v1alpha3_DataDisk_To_v1alpha4_DataDisk(in *DataDisk, out *v1alpha4. func autoConvert_v1alpha4_DataDisk_To_v1alpha3_DataDisk(in *v1alpha4.DataDisk, out *DataDisk, s conversion.Scope) error { out.NameSuffix = in.NameSuffix out.DiskSizeGB = in.DiskSizeGB - if in.ManagedDisk != nil { - in, out := &in.ManagedDisk, &out.ManagedDisk - *out = new(ManagedDisk) - if err := Convert_v1alpha4_ManagedDisk_To_v1alpha3_ManagedDisk(*in, *out, s); err != nil { - return err - } - } else { - out.ManagedDisk = nil - } + out.ManagedDisk = (*ManagedDisk)(unsafe.Pointer(in.ManagedDisk)) out.Lun = (*int32)(unsafe.Pointer(in.Lun)) out.CachingType = in.CachingType return nil @@ -1242,23 +1196,6 @@ func autoConvert_v1alpha4_LoadBalancerSpec_To_v1alpha3_LoadBalancerSpec(in *v1al return nil } -func autoConvert_v1alpha3_ManagedDisk_To_v1alpha4_ManagedDisk(in *ManagedDisk, out *v1alpha4.ManagedDisk, s conversion.Scope) error { - out.StorageAccountType = in.StorageAccountType - out.DiskEncryptionSet = (*v1alpha4.DiskEncryptionSetParameters)(unsafe.Pointer(in.DiskEncryptionSet)) - return nil -} - -// Convert_v1alpha3_ManagedDisk_To_v1alpha4_ManagedDisk is an autogenerated conversion function. -func Convert_v1alpha3_ManagedDisk_To_v1alpha4_ManagedDisk(in *ManagedDisk, out *v1alpha4.ManagedDisk, s conversion.Scope) error { - return autoConvert_v1alpha3_ManagedDisk_To_v1alpha4_ManagedDisk(in, out, s) -} - -func autoConvert_v1alpha4_ManagedDisk_To_v1alpha3_ManagedDisk(in *v1alpha4.ManagedDisk, out *ManagedDisk, s conversion.Scope) error { - out.StorageAccountType = in.StorageAccountType - out.DiskEncryptionSet = (*DiskEncryptionSetParameters)(unsafe.Pointer(in.DiskEncryptionSet)) - return nil -} - func autoConvert_v1alpha3_NetworkSpec_To_v1alpha4_NetworkSpec(in *NetworkSpec, out *v1alpha4.NetworkSpec, s conversion.Scope) error { if err := Convert_v1alpha3_VnetSpec_To_v1alpha4_VnetSpec(&in.Vnet, &out.Vnet, s); err != nil { return err @@ -1306,35 +1243,21 @@ func autoConvert_v1alpha4_NetworkSpec_To_v1alpha3_NetworkSpec(in *v1alpha4.Netwo func autoConvert_v1alpha3_OSDisk_To_v1alpha4_OSDisk(in *OSDisk, out *v1alpha4.OSDisk, s conversion.Scope) error { out.OSType = in.OSType out.DiskSizeGB = in.DiskSizeGB - if err := Convert_v1alpha3_ManagedDisk_To_v1alpha4_ManagedDisk(&in.ManagedDisk, &out.ManagedDisk, s); err != nil { - return err - } + // WARNING: in.ManagedDisk requires manual conversion: inconvertible types (sigs.k8s.io/cluster-api-provider-azure/api/v1alpha3.ManagedDisk vs *sigs.k8s.io/cluster-api-provider-azure/api/v1alpha4.ManagedDiskParameters) out.DiffDiskSettings = (*v1alpha4.DiffDiskSettings)(unsafe.Pointer(in.DiffDiskSettings)) out.CachingType = in.CachingType return nil } -// Convert_v1alpha3_OSDisk_To_v1alpha4_OSDisk is an autogenerated conversion function. -func Convert_v1alpha3_OSDisk_To_v1alpha4_OSDisk(in *OSDisk, out *v1alpha4.OSDisk, s conversion.Scope) error { - return autoConvert_v1alpha3_OSDisk_To_v1alpha4_OSDisk(in, out, s) -} - func autoConvert_v1alpha4_OSDisk_To_v1alpha3_OSDisk(in *v1alpha4.OSDisk, out *OSDisk, s conversion.Scope) error { out.OSType = in.OSType out.DiskSizeGB = in.DiskSizeGB - if err := Convert_v1alpha4_ManagedDisk_To_v1alpha3_ManagedDisk(&in.ManagedDisk, &out.ManagedDisk, s); err != nil { - return err - } + // WARNING: in.ManagedDisk requires manual conversion: inconvertible types (*sigs.k8s.io/cluster-api-provider-azure/api/v1alpha4.ManagedDiskParameters vs sigs.k8s.io/cluster-api-provider-azure/api/v1alpha3.ManagedDisk) out.DiffDiskSettings = (*DiffDiskSettings)(unsafe.Pointer(in.DiffDiskSettings)) out.CachingType = in.CachingType return nil } -// Convert_v1alpha4_OSDisk_To_v1alpha3_OSDisk is an autogenerated conversion function. -func Convert_v1alpha4_OSDisk_To_v1alpha3_OSDisk(in *v1alpha4.OSDisk, out *OSDisk, s conversion.Scope) error { - return autoConvert_v1alpha4_OSDisk_To_v1alpha3_OSDisk(in, out, s) -} - func autoConvert_v1alpha3_PublicIPSpec_To_v1alpha4_PublicIPSpec(in *PublicIPSpec, out *v1alpha4.PublicIPSpec, s conversion.Scope) error { out.Name = in.Name out.DNSName = in.DNSName diff --git a/api/v1alpha4/azuremachine_validation.go b/api/v1alpha4/azuremachine_validation.go index 36fd73b25bd..5b5ff7aa753 100644 --- a/api/v1alpha4/azuremachine_validation.go +++ b/api/v1alpha4/azuremachine_validation.go @@ -96,9 +96,7 @@ func ValidateDataDisks(dataDisks []DataDisk, fieldPath *field.Path) field.ErrorL // validate optional managed disk option if disk.ManagedDisk != nil { - allErrs = append(allErrs, validateStorageAccountType(disk.ManagedDisk.StorageAccountType, fieldPath)...) - - if errs := ValidateManagedDisk(*disk.ManagedDisk, *disk.ManagedDisk, fieldPath.Child("managedDisk")); len(errs) > 0 { + if errs := validateManagedDisk(disk.ManagedDisk, fieldPath.Child("managedDisk"), false); len(errs) > 0 { allErrs = append(allErrs, errs...) } } @@ -132,27 +130,15 @@ func ValidateOSDisk(osDisk OSDisk, fieldPath *field.Path) field.ErrorList { allErrs = append(allErrs, field.Required(fieldPath.Child("OSType"), "the OS type cannot be empty")) } - allErrs = append(allErrs, validateStorageAccountType(osDisk.ManagedDisk.StorageAccountType, fieldPath)...) - allErrs = append(allErrs, validateCachingType(osDisk.CachingType, fieldPath)...) - if errs := ValidateManagedDisk(osDisk.ManagedDisk, osDisk.ManagedDisk, fieldPath.Child("managedDisk")); len(errs) > 0 { - allErrs = append(allErrs, errs...) - } - - if err := validateDiffDiskSetings(osDisk.DiffDiskSettings, fieldPath.Child("diffDiskSettings")); err != nil { - allErrs = append(allErrs, err) - } - - if osDisk.DiffDiskSettings != nil && osDisk.DiffDiskSettings.Option == string(compute.Local) && osDisk.ManagedDisk.StorageAccountType != "Standard_LRS" { - allErrs = append(allErrs, field.Invalid( - fieldPath.Child("managedDisks").Child("storageAccountType"), - osDisk.ManagedDisk.StorageAccountType, - "storageAccountType must be Standard_LRS when diffDiskSettings.option is 'Local'", - )) + if osDisk.ManagedDisk != nil { + if errs := validateManagedDisk(osDisk.ManagedDisk, fieldPath.Child("managedDisk"), true); len(errs) > 0 { + allErrs = append(allErrs, errs...) + } } - if osDisk.DiffDiskSettings != nil && osDisk.DiffDiskSettings.Option == string(compute.Local) && osDisk.ManagedDisk.DiskEncryptionSet != nil { + if osDisk.DiffDiskSettings != nil && osDisk.ManagedDisk != nil && osDisk.ManagedDisk.DiskEncryptionSet != nil { allErrs = append(allErrs, field.Invalid( fieldPath.Child("managedDisks").Child("diskEncryptionSet"), osDisk.ManagedDisk.DiskEncryptionSet.ID, @@ -163,12 +149,12 @@ func ValidateOSDisk(osDisk OSDisk, fieldPath *field.Path) field.ErrorList { return allErrs } -// ValidateManagedDisk validates updates to the ManagedDisk field. -func ValidateManagedDisk(old, new ManagedDisk, fieldPath *field.Path) field.ErrorList { +// validateManagedDisk validates updates to the ManagedDiskParameters field. +func validateManagedDisk(m *ManagedDiskParameters, fieldPath *field.Path, isOSDisk bool) field.ErrorList { allErrs := field.ErrorList{} - if old.StorageAccountType != new.StorageAccountType { - allErrs = append(allErrs, field.Invalid(fieldPath.Child("storageAccountType"), new, "changing storage account type after machine creation is not allowed")) + if m != nil { + allErrs = append(allErrs, validateStorageAccountType(m.StorageAccountType, fieldPath.Child("StorageAccountType"), isOSDisk)...) } return allErrs @@ -198,11 +184,7 @@ func ValidateDataDisksUpdate(oldDataDisks, newDataDisks []DataDisk, fieldPath *f allErrs = append(allErrs, field.Invalid(fieldPath.Index(i).Child("diskSizeGB"), newDataDisks, fieldErrMsg)) } - if newDisk.ManagedDisk != nil && oldDisk.ManagedDisk != nil { - allErrs = append(allErrs, ValidateManagedDisk(*oldDisk.ManagedDisk, *newDisk.ManagedDisk, fieldPath.Index(i).Child("managedDisk"))...) - } else if (newDisk.ManagedDisk != nil && oldDisk.ManagedDisk == nil) || (newDisk.ManagedDisk == nil && oldDisk.ManagedDisk != nil) { - allErrs = append(allErrs, field.Invalid(fieldPath.Index(i).Child("managedDisk"), newDataDisks, fieldErrMsg)) - } + allErrs = append(allErrs, validateManagedDisksUpdate(oldDisk.ManagedDisk, newDisk.ManagedDisk, fieldPath.Index(i).Child("managedDisk"))...) if (newDisk.Lun != nil && oldDisk.Lun != nil) && (*newDisk.Lun != *oldDisk.Lun) { allErrs = append(allErrs, field.Invalid(fieldPath.Index(i).Child("lun"), newDataDisks, fieldErrMsg)) @@ -221,16 +203,6 @@ func ValidateDataDisksUpdate(oldDataDisks, newDataDisks []DataDisk, fieldPath *f return allErrs } -func validateDiffDiskSetings(d *DiffDiskSettings, fldPath *field.Path) *field.Error { - if d != nil { - if d.Option != string(compute.Local) { - msg := "changing ephemeral os settings after machine creation is not allowed" - return field.Invalid(fldPath.Child("option"), d, msg) - } - } - return nil -} - func validateDiffDiskSettingsUpdate(old, new *DiffDiskSettings, fieldPath *field.Path) field.ErrorList { allErrs := field.ErrorList{} fldPath := fieldPath.Child("diffDiskSettings") @@ -254,12 +226,37 @@ func validateDiffDiskSettingsUpdate(old, new *DiffDiskSettings, fieldPath *field return allErrs } -func validateStorageAccountType(storageAccountType string, fieldPath *field.Path) field.ErrorList { +func validateManagedDisksUpdate(old, new *ManagedDiskParameters, fieldPath *field.Path) field.ErrorList { allErrs := field.ErrorList{} - storageAccTypeChildPath := fieldPath.Child("ManagedDisk").Child("StorageAccountType") + fieldErrMsg := "changing managed disk options after machine creation is not allowed" + + if new != nil && old != nil { + if new.StorageAccountType != old.StorageAccountType { + allErrs = append(allErrs, field.Invalid(fieldPath.Child("storageAccountType"), new, fieldErrMsg)) + } + if new.DiskEncryptionSet != nil && old.DiskEncryptionSet != nil { + if new.DiskEncryptionSet.ID != old.DiskEncryptionSet.ID { + allErrs = append(allErrs, field.Invalid(fieldPath.Child("diskEncryptionSet").Child("ID"), new, fieldErrMsg)) + } + } else if (new.DiskEncryptionSet != nil && old.DiskEncryptionSet == nil) || (new.DiskEncryptionSet == nil && old.DiskEncryptionSet != nil) { + allErrs = append(allErrs, field.Invalid(fieldPath.Child("diskEncryptionSet"), new, fieldErrMsg)) + } + } else if (new != nil && old == nil) || (new == nil && old != nil) { + allErrs = append(allErrs, field.Invalid(fieldPath, new, fieldErrMsg)) + } + + return allErrs +} + +func validateStorageAccountType(storageAccountType string, fieldPath *field.Path, isOSDisk bool) field.ErrorList { + allErrs := field.ErrorList{} + + if isOSDisk && storageAccountType == string(compute.StorageAccountTypesUltraSSDLRS) { + allErrs = append(allErrs, field.Invalid(fieldPath.Child("managedDisks").Child("storageAccountType"), storageAccountType, "UltraSSD_LRS can only be used with data disks, it cannot be used with OS Disks")) + } if storageAccountType == "" { - allErrs = append(allErrs, field.Required(storageAccTypeChildPath, "the Storage Account Type for Managed Disk cannot be empty")) + allErrs = append(allErrs, field.Required(fieldPath, "the Storage Account Type for Managed Disk cannot be empty")) return allErrs } @@ -268,7 +265,7 @@ func validateStorageAccountType(storageAccountType string, fieldPath *field.Path return allErrs } } - allErrs = append(allErrs, field.Invalid(storageAccTypeChildPath, "", fmt.Sprintf("allowed values are %v", compute.PossibleDiskStorageAccountTypesValues()))) + allErrs = append(allErrs, field.Invalid(fieldPath, "", fmt.Sprintf("allowed values are %v", compute.PossibleDiskStorageAccountTypesValues()))) return allErrs } diff --git a/api/v1alpha4/azuremachine_validation_test.go b/api/v1alpha4/azuremachine_validation_test.go index f9085b19ade..a2b2be9cd44 100644 --- a/api/v1alpha4/azuremachine_validation_test.go +++ b/api/v1alpha4/azuremachine_validation_test.go @@ -109,7 +109,7 @@ func TestAzureMachine_ValidateOSDisk(t *testing.T) { DiffDiskSettings: &DiffDiskSettings{ Option: string(compute.Local), }, - ManagedDisk: ManagedDisk{ + ManagedDisk: &ManagedDiskParameters{ StorageAccountType: "Standard_LRS", }, }, @@ -124,7 +124,7 @@ func TestAzureMachine_ValidateOSDisk(t *testing.T) { DiffDiskSettings: &DiffDiskSettings{ Option: string(compute.Local), }, - ManagedDisk: ManagedDisk{ + ManagedDisk: &ManagedDiskParameters{ StorageAccountType: "Standard_LRS", DiskEncryptionSet: &DiskEncryptionSetParameters{ ID: "disk-encryption-set", @@ -172,26 +172,26 @@ func generateNegativeTestCases() []osDiskTestInput { { DiskSizeGB: 30, OSType: "blah", - ManagedDisk: ManagedDisk{}, + ManagedDisk: &ManagedDiskParameters{}, }, { DiskSizeGB: 30, OSType: "blah", - ManagedDisk: ManagedDisk{ + ManagedDisk: &ManagedDiskParameters{ StorageAccountType: "", }, }, { DiskSizeGB: 30, OSType: "blah", - ManagedDisk: ManagedDisk{ + ManagedDisk: &ManagedDiskParameters{ StorageAccountType: "invalid_type", }, }, { DiskSizeGB: 30, OSType: "blah", - ManagedDisk: ManagedDisk{ + ManagedDisk: &ManagedDiskParameters{ StorageAccountType: "Premium_LRS", }, DiffDiskSettings: &DiffDiskSettings{ @@ -215,7 +215,7 @@ func generateValidOSDisk() OSDisk { return OSDisk{ DiskSizeGB: 30, OSType: "Linux", - ManagedDisk: ManagedDisk{ + ManagedDisk: &ManagedDiskParameters{ StorageAccountType: "Premium_LRS", }, CachingType: string(compute.PossibleCachingTypesValues()[0]), @@ -354,7 +354,7 @@ func TestAzureMachine_ValidateDataDisks(t *testing.T) { { NameSuffix: "my_disk_1", DiskSizeGB: 64, - ManagedDisk: &ManagedDisk{ + ManagedDisk: &ManagedDiskParameters{ StorageAccountType: "Premium_LRS", }, Lun: to.Int32Ptr(0), @@ -363,7 +363,7 @@ func TestAzureMachine_ValidateDataDisks(t *testing.T) { { NameSuffix: "my_disk_2", DiskSizeGB: 64, - ManagedDisk: &ManagedDisk{ + ManagedDisk: &ManagedDiskParameters{ StorageAccountType: "Standard_LRS", }, Lun: to.Int32Ptr(1), @@ -378,7 +378,7 @@ func TestAzureMachine_ValidateDataDisks(t *testing.T) { { NameSuffix: "my_disk_1", DiskSizeGB: 64, - ManagedDisk: &ManagedDisk{ + ManagedDisk: &ManagedDiskParameters{ StorageAccountType: "invalid storage account", }, Lun: to.Int32Ptr(0), @@ -484,7 +484,7 @@ func TestAzureMachine_ValidateDataDisksUpdate(t *testing.T) { NameSuffix: "my_disk", DiskSizeGB: 64, Lun: to.Int32Ptr(0), - ManagedDisk: &ManagedDisk{ + ManagedDisk: &ManagedDiskParameters{ StorageAccountType: "Standard_LRS", }, CachingType: string(compute.PossibleCachingTypesValues()[0]), @@ -493,7 +493,7 @@ func TestAzureMachine_ValidateDataDisksUpdate(t *testing.T) { NameSuffix: "my_other_disk", DiskSizeGB: 64, Lun: to.Int32Ptr(1), - ManagedDisk: &ManagedDisk{ + ManagedDisk: &ManagedDiskParameters{ StorageAccountType: "Standard_LRS", }, CachingType: string(compute.PossibleCachingTypesValues()[0]), @@ -504,7 +504,7 @@ func TestAzureMachine_ValidateDataDisksUpdate(t *testing.T) { NameSuffix: "my_disk", DiskSizeGB: 64, Lun: to.Int32Ptr(0), - ManagedDisk: &ManagedDisk{ + ManagedDisk: &ManagedDiskParameters{ StorageAccountType: "Standard_LRS", }, CachingType: string(compute.PossibleCachingTypesValues()[0]), @@ -513,7 +513,7 @@ func TestAzureMachine_ValidateDataDisksUpdate(t *testing.T) { NameSuffix: "my_other_disk", DiskSizeGB: 64, Lun: to.Int32Ptr(1), - ManagedDisk: &ManagedDisk{ + ManagedDisk: &ManagedDiskParameters{ StorageAccountType: "Standard_LRS", }, CachingType: string(compute.PossibleCachingTypesValues()[0]), @@ -527,7 +527,7 @@ func TestAzureMachine_ValidateDataDisksUpdate(t *testing.T) { { NameSuffix: "my_disk_1", DiskSizeGB: 64, - ManagedDisk: &ManagedDisk{ + ManagedDisk: &ManagedDiskParameters{ StorageAccountType: "Standard_LRS", }, Lun: to.Int32Ptr(0), @@ -538,7 +538,7 @@ func TestAzureMachine_ValidateDataDisksUpdate(t *testing.T) { { NameSuffix: "my_disk_1", DiskSizeGB: 128, - ManagedDisk: &ManagedDisk{ + ManagedDisk: &ManagedDiskParameters{ StorageAccountType: "Premium_LRS", }, Lun: to.Int32Ptr(0), @@ -553,7 +553,7 @@ func TestAzureMachine_ValidateDataDisksUpdate(t *testing.T) { { NameSuffix: "my_disk_1", DiskSizeGB: 128, - ManagedDisk: &ManagedDisk{ + ManagedDisk: &ManagedDiskParameters{ StorageAccountType: "Standard_LRS", }, Lun: to.Int32Ptr(0), @@ -574,7 +574,7 @@ func TestAzureMachine_ValidateDataDisksUpdate(t *testing.T) { { NameSuffix: "my_disk_1", DiskSizeGB: 64, - ManagedDisk: &ManagedDisk{ + ManagedDisk: &ManagedDiskParameters{ StorageAccountType: "Standard_LRS", }, Lun: to.Int32Ptr(0), @@ -585,7 +585,7 @@ func TestAzureMachine_ValidateDataDisksUpdate(t *testing.T) { { NameSuffix: "my_disk_1", DiskSizeGB: 64, - ManagedDisk: &ManagedDisk{ + ManagedDisk: &ManagedDiskParameters{ StorageAccountType: "Premium_LRS", }, Lun: to.Int32Ptr(0), @@ -594,7 +594,7 @@ func TestAzureMachine_ValidateDataDisksUpdate(t *testing.T) { { NameSuffix: "my_disk_2", DiskSizeGB: 64, - ManagedDisk: &ManagedDisk{ + ManagedDisk: &ManagedDiskParameters{ StorageAccountType: "Premium_LRS", }, Lun: to.Int32Ptr(2), @@ -609,7 +609,7 @@ func TestAzureMachine_ValidateDataDisksUpdate(t *testing.T) { { NameSuffix: "my_disk_1", DiskSizeGB: 64, - ManagedDisk: &ManagedDisk{ + ManagedDisk: &ManagedDiskParameters{ StorageAccountType: "Standard_LRS", }, Lun: to.Int32Ptr(0), @@ -618,7 +618,7 @@ func TestAzureMachine_ValidateDataDisksUpdate(t *testing.T) { { NameSuffix: "my_disk_2", DiskSizeGB: 64, - ManagedDisk: &ManagedDisk{ + ManagedDisk: &ManagedDiskParameters{ StorageAccountType: "Premium_LRS", }, Lun: to.Int32Ptr(2), @@ -629,7 +629,7 @@ func TestAzureMachine_ValidateDataDisksUpdate(t *testing.T) { { NameSuffix: "my_disk_1", DiskSizeGB: 64, - ManagedDisk: &ManagedDisk{ + ManagedDisk: &ManagedDiskParameters{ StorageAccountType: "Standard_LRS", }, Lun: to.Int32Ptr(0), diff --git a/api/v1alpha4/azuremachine_webhook.go b/api/v1alpha4/azuremachine_webhook.go index c5ded4a6d06..cc8a84e6721 100644 --- a/api/v1alpha4/azuremachine_webhook.go +++ b/api/v1alpha4/azuremachine_webhook.go @@ -109,7 +109,11 @@ func (m *AzureMachine) ValidateUpdate(oldRaw runtime.Object) error { allErrs = append(allErrs, errs...) } - if errs := ValidateManagedDisk(old.Spec.OSDisk.ManagedDisk, m.Spec.OSDisk.ManagedDisk, field.NewPath("osDisk").Child("managedDisk")); len(errs) > 0 { + if errs := validateManagedDisksUpdate(old.Spec.OSDisk.ManagedDisk, old.Spec.OSDisk.ManagedDisk, field.NewPath("osDisk").Child("managedDisk")); len(errs) > 0 { + allErrs = append(allErrs, errs...) + } + + if errs := validateManagedDisk(m.Spec.OSDisk.ManagedDisk, field.NewPath("osDisk").Child("managedDisk"), true); len(errs) > 0 { allErrs = append(allErrs, errs...) } diff --git a/api/v1alpha4/azuremachine_webhook_test.go b/api/v1alpha4/azuremachine_webhook_test.go index f7ad70975dd..864c0779bd4 100644 --- a/api/v1alpha4/azuremachine_webhook_test.go +++ b/api/v1alpha4/azuremachine_webhook_test.go @@ -303,7 +303,7 @@ func createDataDisk(t *testing.T, diskSizeGB int32, lun int32, name string, stor NameSuffix: name, DiskSizeGB: diskSizeGB, Lun: to.Int32Ptr(lun), - ManagedDisk: &ManagedDisk{ + ManagedDisk: &ManagedDiskParameters{ StorageAccountType: storageAccountType, }, CachingType: cacheType, diff --git a/api/v1alpha4/types.go b/api/v1alpha4/types.go index b57b6cdc40e..c1496041ec1 100644 --- a/api/v1alpha4/types.go +++ b/api/v1alpha4/types.go @@ -362,10 +362,13 @@ const ( // OSDisk defines the operating system disk for a VM. type OSDisk struct { - OSType string `json:"osType"` - DiskSizeGB int32 `json:"diskSizeGB"` - ManagedDisk ManagedDisk `json:"managedDisk"` - DiffDiskSettings *DiffDiskSettings `json:"diffDiskSettings,omitempty"` + OSType string `json:"osType"` + DiskSizeGB int32 `json:"diskSizeGB"` + // ManagedDisk specifies the Managed Disk parameters for the OS disk. + // +optional + ManagedDisk *ManagedDiskParameters `json:"managedDisk,omitempty"` + DiffDiskSettings *DiffDiskSettings `json:"diffDiskSettings,omitempty"` + // CachingType specifies the caching requirements. // +optional CachingType string `json:"cachingType,omitempty"` } @@ -377,8 +380,9 @@ type DataDisk struct { NameSuffix string `json:"nameSuffix"` // DiskSizeGB is the size in GB to assign to the data disk. DiskSizeGB int32 `json:"diskSizeGB"` + // ManagedDisk specifies the Managed Disk parameters for the data disk. // +optional - ManagedDisk *ManagedDisk `json:"managedDisk,omitempty"` + ManagedDisk *ManagedDiskParameters `json:"managedDisk,omitempty"` // Lun Specifies the logical unit number of the data disk. This value is used to identify data disks within the VM and therefore must be unique for each data disk attached to a VM. // The value must be between 0 and 63. Lun *int32 `json:"lun,omitempty"` @@ -386,10 +390,12 @@ type DataDisk struct { CachingType string `json:"cachingType,omitempty"` } -// ManagedDisk defines the managed disk options for a VM. -type ManagedDisk struct { - StorageAccountType string `json:"storageAccountType"` - DiskEncryptionSet *DiskEncryptionSetParameters `json:"diskEncryptionSet,omitempty"` +// ManagedDiskParameters defines the parameters of a managed disk. +type ManagedDiskParameters struct { + // +optional + StorageAccountType string `json:"storageAccountType,omitempty"` + // +optional + DiskEncryptionSet *DiskEncryptionSetParameters `json:"diskEncryptionSet,omitempty"` } // DiskEncryptionSetParameters defines disk encryption options. diff --git a/api/v1alpha4/zz_generated.deepcopy.go b/api/v1alpha4/zz_generated.deepcopy.go index 3feac127571..c5a96c5b45d 100644 --- a/api/v1alpha4/zz_generated.deepcopy.go +++ b/api/v1alpha4/zz_generated.deepcopy.go @@ -584,7 +584,7 @@ func (in *DataDisk) DeepCopyInto(out *DataDisk) { *out = *in if in.ManagedDisk != nil { in, out := &in.ManagedDisk, &out.ManagedDisk - *out = new(ManagedDisk) + *out = new(ManagedDiskParameters) (*in).DeepCopyInto(*out) } if in.Lun != nil { @@ -727,7 +727,7 @@ func (in *LoadBalancerSpec) DeepCopy() *LoadBalancerSpec { } // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *ManagedDisk) DeepCopyInto(out *ManagedDisk) { +func (in *ManagedDiskParameters) DeepCopyInto(out *ManagedDiskParameters) { *out = *in if in.DiskEncryptionSet != nil { in, out := &in.DiskEncryptionSet, &out.DiskEncryptionSet @@ -736,12 +736,12 @@ func (in *ManagedDisk) DeepCopyInto(out *ManagedDisk) { } } -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ManagedDisk. -func (in *ManagedDisk) DeepCopy() *ManagedDisk { +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ManagedDiskParameters. +func (in *ManagedDiskParameters) DeepCopy() *ManagedDiskParameters { if in == nil { return nil } - out := new(ManagedDisk) + out := new(ManagedDiskParameters) in.DeepCopyInto(out) return out } @@ -778,7 +778,11 @@ func (in *NetworkSpec) DeepCopy() *NetworkSpec { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *OSDisk) DeepCopyInto(out *OSDisk) { *out = *in - in.ManagedDisk.DeepCopyInto(&out.ManagedDisk) + if in.ManagedDisk != nil { + in, out := &in.ManagedDisk, &out.ManagedDisk + *out = new(ManagedDiskParameters) + (*in).DeepCopyInto(*out) + } if in.DiffDiskSettings != nil { in, out := &in.DiffDiskSettings, &out.DiffDiskSettings *out = new(DiffDiskSettings) diff --git a/azure/services/scalesets/scalesets.go b/azure/services/scalesets/scalesets.go index b1692067c78..191b7c23d91 100644 --- a/azure/services/scalesets/scalesets.go +++ b/azure/services/scalesets/scalesets.go @@ -549,9 +549,6 @@ func (s *Service) generateStorageProfile(vmssSpec azure.ScaleSetSpec, sku resour OsType: compute.OperatingSystemTypes(vmssSpec.OSDisk.OSType), CreateOption: compute.DiskCreateOptionTypesFromImage, DiskSizeGB: to.Int32Ptr(vmssSpec.OSDisk.DiskSizeGB), - ManagedDisk: &compute.VirtualMachineScaleSetManagedDiskParameters{ - StorageAccountType: compute.StorageAccountTypes(vmssSpec.OSDisk.ManagedDisk.StorageAccountType), - }, }, } @@ -566,8 +563,14 @@ func (s *Service) generateStorageProfile(vmssSpec azure.ScaleSetSpec, sku resour } } - if vmssSpec.OSDisk.ManagedDisk.DiskEncryptionSet != nil { - storageProfile.OsDisk.ManagedDisk.DiskEncryptionSet = &compute.DiskEncryptionSetParameters{ID: to.StringPtr(vmssSpec.OSDisk.ManagedDisk.DiskEncryptionSet.ID)} + if vmssSpec.OSDisk.ManagedDisk != nil { + storageProfile.OsDisk.ManagedDisk = &compute.VirtualMachineScaleSetManagedDiskParameters{} + if vmssSpec.OSDisk.ManagedDisk.StorageAccountType != "" { + storageProfile.OsDisk.ManagedDisk.StorageAccountType = compute.StorageAccountTypes(vmssSpec.OSDisk.ManagedDisk.StorageAccountType) + } + if vmssSpec.OSDisk.ManagedDisk.DiskEncryptionSet != nil { + storageProfile.OsDisk.ManagedDisk.DiskEncryptionSet = &compute.DiskEncryptionSetParameters{ID: to.StringPtr(vmssSpec.OSDisk.ManagedDisk.DiskEncryptionSet.ID)} + } } dataDisks := make([]compute.VirtualMachineScaleSetDataDisk, len(vmssSpec.DataDisks)) diff --git a/azure/services/scalesets/scalesets_test.go b/azure/services/scalesets/scalesets_test.go index 7c5755d891d..f031e8f3749 100644 --- a/azure/services/scalesets/scalesets_test.go +++ b/azure/services/scalesets/scalesets_test.go @@ -808,7 +808,7 @@ func newDefaultVMSSSpec() azure.ScaleSetSpec { OSDisk: infrav1.OSDisk{ OSType: "Linux", DiskSizeGB: 120, - ManagedDisk: infrav1.ManagedDisk{ + ManagedDisk: &infrav1.ManagedDiskParameters{ StorageAccountType: "Premium_LRS", }, }, @@ -822,7 +822,7 @@ func newDefaultVMSSSpec() azure.ScaleSetSpec { NameSuffix: "my_disk_with_managed_disk", DiskSizeGB: 128, Lun: to.Int32Ptr(1), - ManagedDisk: &infrav1.ManagedDisk{ + ManagedDisk: &infrav1.ManagedDiskParameters{ StorageAccountType: "Standard_LRS", }, }, @@ -830,7 +830,7 @@ func newDefaultVMSSSpec() azure.ScaleSetSpec { NameSuffix: "managed_disk_with_encryption", DiskSizeGB: 128, Lun: to.Int32Ptr(2), - ManagedDisk: &infrav1.ManagedDisk{ + ManagedDisk: &infrav1.ManagedDiskParameters{ StorageAccountType: "Standard_LRS", DiskEncryptionSet: &infrav1.DiskEncryptionSetParameters{ ID: "encryption_id", diff --git a/azure/services/virtualmachines/virtualmachines.go b/azure/services/virtualmachines/virtualmachines.go index 33a4b278a4c..457bc963cd5 100644 --- a/azure/services/virtualmachines/virtualmachines.go +++ b/azure/services/virtualmachines/virtualmachines.go @@ -351,10 +351,7 @@ func (s *Service) generateStorageProfile(ctx context.Context, vmSpec azure.VMSpe OsType: compute.OperatingSystemTypes(vmSpec.OSDisk.OSType), CreateOption: compute.DiskCreateOptionTypesFromImage, DiskSizeGB: to.Int32Ptr(vmSpec.OSDisk.DiskSizeGB), - ManagedDisk: &compute.ManagedDiskParameters{ - StorageAccountType: compute.StorageAccountTypes(vmSpec.OSDisk.ManagedDisk.StorageAccountType), - }, - Caching: compute.CachingTypes(vmSpec.OSDisk.CachingType), + Caching: compute.CachingTypes(vmSpec.OSDisk.CachingType), }, } @@ -387,8 +384,14 @@ func (s *Service) generateStorageProfile(ctx context.Context, vmSpec azure.VMSpe } } - if vmSpec.OSDisk.ManagedDisk.DiskEncryptionSet != nil { - storageProfile.OsDisk.ManagedDisk.DiskEncryptionSet = &compute.DiskEncryptionSetParameters{ID: to.StringPtr(vmSpec.OSDisk.ManagedDisk.DiskEncryptionSet.ID)} + if vmSpec.OSDisk.ManagedDisk != nil { + storageProfile.OsDisk.ManagedDisk = &compute.ManagedDiskParameters{} + if vmSpec.OSDisk.ManagedDisk.StorageAccountType != "" { + storageProfile.OsDisk.ManagedDisk.StorageAccountType = compute.StorageAccountTypes(vmSpec.OSDisk.ManagedDisk.StorageAccountType) + } + if vmSpec.OSDisk.ManagedDisk.DiskEncryptionSet != nil { + storageProfile.OsDisk.ManagedDisk.DiskEncryptionSet = &compute.DiskEncryptionSetParameters{ID: to.StringPtr(vmSpec.OSDisk.ManagedDisk.DiskEncryptionSet.ID)} + } } dataDisks := make([]compute.DataDisk, len(vmSpec.DataDisks)) diff --git a/azure/services/virtualmachines/virtualmachines_test.go b/azure/services/virtualmachines/virtualmachines_test.go index 2801faaf5cd..ac37fb2de08 100644 --- a/azure/services/virtualmachines/virtualmachines_test.go +++ b/azure/services/virtualmachines/virtualmachines_test.go @@ -297,7 +297,7 @@ func TestReconcileVM(t *testing.T) { OSDisk: infrav1.OSDisk{ OSType: "Linux", DiskSizeGB: 128, - ManagedDisk: infrav1.ManagedDisk{ + ManagedDisk: &infrav1.ManagedDiskParameters{ StorageAccountType: "Premium_LRS", }, }, @@ -311,7 +311,7 @@ func TestReconcileVM(t *testing.T) { NameSuffix: "myDiskWithManagedDisk", DiskSizeGB: 128, Lun: to.Int32Ptr(1), - ManagedDisk: &infrav1.ManagedDisk{ + ManagedDisk: &infrav1.ManagedDiskParameters{ StorageAccountType: "Premium_LRS", }, }, @@ -319,7 +319,7 @@ func TestReconcileVM(t *testing.T) { NameSuffix: "managedDiskWithEncryption", DiskSizeGB: 128, Lun: to.Int32Ptr(2), - ManagedDisk: &infrav1.ManagedDisk{ + ManagedDisk: &infrav1.ManagedDiskParameters{ StorageAccountType: "Premium_LRS", DiskEncryptionSet: &infrav1.DiskEncryptionSetParameters{ ID: "my_id", @@ -710,7 +710,7 @@ func TestReconcileVM(t *testing.T) { OSDisk: infrav1.OSDisk{ OSType: "Windows", DiskSizeGB: 128, - ManagedDisk: infrav1.ManagedDisk{ + ManagedDisk: &infrav1.ManagedDiskParameters{ StorageAccountType: "Premium_LRS", }, }, @@ -795,7 +795,7 @@ func TestReconcileVM(t *testing.T) { Zone: "1", Identity: "", OSDisk: infrav1.OSDisk{ - ManagedDisk: infrav1.ManagedDisk{ + ManagedDisk: &infrav1.ManagedDiskParameters{ StorageAccountType: "Premium_LRS", DiskEncryptionSet: &infrav1.DiskEncryptionSetParameters{ ID: "my-diskencryptionset-id", @@ -948,7 +948,7 @@ func TestReconcileVM(t *testing.T) { OSDisk: infrav1.OSDisk{ OSType: "Linux", DiskSizeGB: 128, - ManagedDisk: infrav1.ManagedDisk{ + ManagedDisk: &infrav1.ManagedDiskParameters{ StorageAccountType: "Premium_LRS", }, }, @@ -1231,7 +1231,7 @@ func TestReconcileVM(t *testing.T) { OSDisk: infrav1.OSDisk{ OSType: "Linux", DiskSizeGB: 128, - ManagedDisk: infrav1.ManagedDisk{ + ManagedDisk: &infrav1.ManagedDiskParameters{ StorageAccountType: "Premium_LRS", }, }, @@ -1298,7 +1298,7 @@ func TestReconcileVM(t *testing.T) { OSDisk: infrav1.OSDisk{ OSType: "Linux", DiskSizeGB: 128, - ManagedDisk: infrav1.ManagedDisk{ + ManagedDisk: &infrav1.ManagedDiskParameters{ StorageAccountType: "Premium_LRS", }, }, @@ -1365,7 +1365,7 @@ func TestReconcileVM(t *testing.T) { OSDisk: infrav1.OSDisk{ OSType: "Linux", DiskSizeGB: 128, - ManagedDisk: infrav1.ManagedDisk{ + ManagedDisk: &infrav1.ManagedDiskParameters{ StorageAccountType: "Premium_LRS", }, DiffDiskSettings: &infrav1.DiffDiskSettings{ @@ -1439,7 +1439,7 @@ func TestReconcileVM(t *testing.T) { OSDisk: infrav1.OSDisk{ OSType: "Linux", DiskSizeGB: 128, - ManagedDisk: infrav1.ManagedDisk{ + ManagedDisk: &infrav1.ManagedDiskParameters{ StorageAccountType: "Premium_LRS", }, DiffDiskSettings: &infrav1.DiffDiskSettings{ @@ -1604,7 +1604,7 @@ func TestReconcileVM(t *testing.T) { OSDisk: infrav1.OSDisk{ OSType: "Linux", DiskSizeGB: 128, - ManagedDisk: infrav1.ManagedDisk{ + ManagedDisk: &infrav1.ManagedDiskParameters{ StorageAccountType: "Premium_LRS", }, }, diff --git a/config/crd/bases/exp.infrastructure.cluster.x-k8s.io_azuremachinepools.yaml b/config/crd/bases/exp.infrastructure.cluster.x-k8s.io_azuremachinepools.yaml index f2304d13db4..cf32ec249e8 100644 --- a/config/crd/bases/exp.infrastructure.cluster.x-k8s.io_azuremachinepools.yaml +++ b/config/crd/bases/exp.infrastructure.cluster.x-k8s.io_azuremachinepools.yaml @@ -495,7 +495,7 @@ spec: format: int32 type: integer managedDisk: - description: ManagedDisk defines the managed disk options for a VM. + description: ManagedDisk specifies the Managed Disk parameters for the data disk. properties: diskEncryptionSet: description: DiskEncryptionSetParameters defines disk encryption options. @@ -506,8 +506,6 @@ spec: type: object storageAccountType: type: string - required: - - storageAccountType type: object nameSuffix: description: NameSuffix is the suffix to be appended to the machine name to generate the disk name. Each disk name will be in format _. @@ -603,7 +601,7 @@ spec: format: int32 type: integer managedDisk: - description: ManagedDisk defines the managed disk options for a VM. + description: ManagedDisk specifies the Managed Disk parameters for the OS disk. properties: diskEncryptionSet: description: DiskEncryptionSetParameters defines disk encryption options. @@ -614,14 +612,11 @@ spec: type: object storageAccountType: type: string - required: - - storageAccountType type: object osType: type: string required: - diskSizeGB - - managedDisk - osType type: object securityProfile: 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 01f15f07265..a2d90c03c70 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremachines.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremachines.yaml @@ -428,7 +428,7 @@ spec: format: int32 type: integer managedDisk: - description: ManagedDisk defines the managed disk options for a VM. + description: ManagedDisk specifies the Managed Disk parameters for the data disk. properties: diskEncryptionSet: description: DiskEncryptionSetParameters defines disk encryption options. @@ -439,8 +439,6 @@ spec: type: object storageAccountType: type: string - required: - - storageAccountType type: object nameSuffix: description: NameSuffix is the suffix to be appended to the machine name to generate the disk name. Each disk name will be in format _. @@ -550,7 +548,7 @@ spec: format: int32 type: integer managedDisk: - description: ManagedDisk defines the managed disk options for a VM. + description: ManagedDisk specifies the Managed Disk parameters for the OS disk. properties: diskEncryptionSet: description: DiskEncryptionSetParameters defines disk encryption options. @@ -561,14 +559,11 @@ spec: type: object storageAccountType: type: string - required: - - storageAccountType type: object osType: type: string required: - diskSizeGB - - managedDisk - osType type: object providerID: 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 8d6039b387e..d2c353cdc5a 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremachinetemplates.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremachinetemplates.yaml @@ -325,7 +325,7 @@ spec: format: int32 type: integer managedDisk: - description: ManagedDisk defines the managed disk options for a VM. + description: ManagedDisk specifies the Managed Disk parameters for the data disk. properties: diskEncryptionSet: description: DiskEncryptionSetParameters defines disk encryption options. @@ -336,8 +336,6 @@ spec: type: object storageAccountType: type: string - required: - - storageAccountType type: object nameSuffix: description: NameSuffix is the suffix to be appended to the machine name to generate the disk name. Each disk name will be in format _. @@ -447,7 +445,7 @@ spec: format: int32 type: integer managedDisk: - description: ManagedDisk defines the managed disk options for a VM. + description: ManagedDisk specifies the Managed Disk parameters for the OS disk. properties: diskEncryptionSet: description: DiskEncryptionSetParameters defines disk encryption options. @@ -458,14 +456,11 @@ spec: type: object storageAccountType: type: string - required: - - storageAccountType type: object osType: type: string required: - diskSizeGB - - managedDisk - osType type: object providerID: diff --git a/docs/book/src/SUMMARY.md b/docs/book/src/SUMMARY.md index 4e9e3f37a22..be0c4bf75b0 100644 --- a/docs/book/src/SUMMARY.md +++ b/docs/book/src/SUMMARY.md @@ -10,7 +10,7 @@ - [Custom Private DNS Zone Name](./topics/custom-dns.md) - [Custom Images](./topics/custom-images.md) - [Data Disks](./topics/data-disks.md) - - [Ephemeral OS](./topics/ephemeral-os.md) + - [OS Disk](./topics/os-disk.md) - [Failure Domains](./topics/failure-domains.md) - [GPU-enabled Clusters](./topics/gpu.md) - [Identity](./topics/identity.md) @@ -19,7 +19,7 @@ - [Machine Pools (VMSS)](./topics/machinepools.md) - [Managed Clusters (AKS)](./topics/managedcluster.md) - [Multitenancy](./topics/multitenancy.md) - - [Node Ooutbound Load Balancer](./topics/node-outbound-lb.md) + - [Node Outbound Load Balancer](./topics/node-outbound-lb.md) - [Spot Virtual Machines](./topics/spot-vms.md) - [Virtual Networks](./topics/custom-vnet.md) - [Windows](./topics/windows.md) diff --git a/docs/book/src/topics/data-disks.md b/docs/book/src/topics/data-disks.md index 813e3ace0fa..8adcaaa0da1 100644 --- a/docs/book/src/topics/data-disks.md +++ b/docs/book/src/topics/data-disks.md @@ -10,7 +10,7 @@ Azure Machines support optionally specifying a list of data disks to be attached - `managedDisk` - (optional) the managed disk for a VM (see below) - `lun` - the logical unit number (see below) -### Managed Disk +### Managed Disk Options See [Introduction to Azure managed disks](https://docs.microsoft.com/en-us/azure/virtual-machines/managed-disks-overview) for more information. diff --git a/docs/book/src/topics/failure-domains.md b/docs/book/src/topics/failure-domains.md index 89f95667370..274c68160eb 100644 --- a/docs/book/src/topics/failure-domains.md +++ b/docs/book/src/topics/failure-domains.md @@ -175,8 +175,6 @@ spec: template: osDisk: diskSizeGB: 30 - managedDisk: - storageAccountType: Premium_LRS osType: Linux vmSize: Standard_D2s_v3 ``` diff --git a/docs/book/src/topics/ephemeral-os.md b/docs/book/src/topics/os-disk.md similarity index 66% rename from docs/book/src/topics/ephemeral-os.md rename to docs/book/src/topics/os-disk.md index c2535c763b4..1562d2b7b01 100644 --- a/docs/book/src/topics/ephemeral-os.md +++ b/docs/book/src/topics/os-disk.md @@ -1,7 +1,29 @@ -# Ephemeral OS +# OS Disk -This document describes how to use ephemeral OS for VMs provisioned in -Azure. Ephemeral OS uses local VM storage for changes to the OS disk. +This document describes how to configure the OS disk for VMs provisioned in Azure. + +### Managed Disk Options + +### Storage Account Type + +By default, Azure will pick the supported storage account type for your AzureMachine based on the specified VM size. If you'd like to specify a specific storage type, you can do so by specifying a `storageAccountType`: + +```yaml + managedDisk: + storageAccountType: Premium_LRS +``` + +Supported values are `Premium_LRS`, `Standard_LRS`, and `StandardSSDLRS`. Note that `UltraSSD_LRS` can only be used with data disks, it cannot be used with OS Disk. + +Also, note that not all Azure VM sizes support Premium storage. To learn more about which sizes are premium storage-compatible, see [Sizes for virtual machines in Azure](https://docs.microsoft.com/en-us/azure/virtual-machines/sizes). + +See [Azure documentation on disk types](https://docs.microsoft.com/en-us/azure/virtual-machines/disks-types) to learn more about the different storage types. + +See [Introduction to Azure managed disks](https://docs.microsoft.com/en-us/azure/virtual-machines/managed-disks-overview) for more information on managed disks. + +## Ephemeral OS + +Ephemeral OS uses local VM storage for changes to the OS disk. Storage devices local to the VM host will not be bound by normal managed disk SKU limits. Instead they will always be capable of saturating the VM level limits. This can significantly improve performance on the OS diff --git a/exp/api/v1alpha3/azuremachinepool_conversion.go b/exp/api/v1alpha3/azuremachinepool_conversion.go index 98e1d930291..7b5de553ff8 100644 --- a/exp/api/v1alpha3/azuremachinepool_conversion.go +++ b/exp/api/v1alpha3/azuremachinepool_conversion.go @@ -17,6 +17,7 @@ limitations under the License. package v1alpha3 import ( + infrav1alpha4 "sigs.k8s.io/cluster-api-provider-azure/api/v1alpha4" expv1alpha4 "sigs.k8s.io/cluster-api-provider-azure/exp/api/v1alpha4" utilconversion "sigs.k8s.io/cluster-api/util/conversion" "sigs.k8s.io/controller-runtime/pkg/conversion" @@ -36,6 +37,14 @@ func (src *AzureMachinePool) ConvertTo(dstRaw conversion.Hub) error { // nolint return err } + // Handle special case for conversion of ManagedDisk to pointer. + if restored.Spec.Template.OSDisk.ManagedDisk == nil && dst.Spec.Template.OSDisk.ManagedDisk != nil { + if *dst.Spec.Template.OSDisk.ManagedDisk == (infrav1alpha4.ManagedDiskParameters{}) { + // restore nil value if nothing has changed since conversion + dst.Spec.Template.OSDisk.ManagedDisk = nil + } + } + return nil } diff --git a/templates/cluster-template-aad.yaml b/templates/cluster-template-aad.yaml index 3fef1e8f277..86f028dae5a 100644 --- a/templates/cluster-template-aad.yaml +++ b/templates/cluster-template-aad.yaml @@ -131,8 +131,6 @@ spec: nameSuffix: etcddisk osDisk: diskSizeGB: 128 - managedDisk: - storageAccountType: Premium_LRS osType: Linux sshPublicKey: ${AZURE_SSH_PUBLIC_KEY_B64:=""} vmSize: ${AZURE_CONTROL_PLANE_MACHINE_TYPE} diff --git a/templates/cluster-template-ephemeral.yaml b/templates/cluster-template-ephemeral.yaml index bde7a535c5c..4cc514bfa35 100644 --- a/templates/cluster-template-ephemeral.yaml +++ b/templates/cluster-template-ephemeral.yaml @@ -127,8 +127,6 @@ spec: diffDiskSettings: option: Local diskSizeGB: 128 - managedDisk: - storageAccountType: Standard_LRS osType: Linux sshPublicKey: ${AZURE_SSH_PUBLIC_KEY_B64:=""} vmSize: ${AZURE_CONTROL_PLANE_MACHINE_TYPE} @@ -169,8 +167,6 @@ spec: diffDiskSettings: option: Local diskSizeGB: 128 - managedDisk: - storageAccountType: Standard_LRS osType: Linux sshPublicKey: ${AZURE_SSH_PUBLIC_KEY_B64:=""} vmSize: ${AZURE_NODE_MACHINE_TYPE} diff --git a/templates/cluster-template-external-cloud-provider.yaml b/templates/cluster-template-external-cloud-provider.yaml index 9dae5900221..8d7cb7f25c4 100644 --- a/templates/cluster-template-external-cloud-provider.yaml +++ b/templates/cluster-template-external-cloud-provider.yaml @@ -126,8 +126,6 @@ spec: nameSuffix: etcddisk osDisk: diskSizeGB: 128 - managedDisk: - storageAccountType: Premium_LRS osType: Linux sshPublicKey: ${AZURE_SSH_PUBLIC_KEY_B64:=""} vmSize: ${AZURE_CONTROL_PLANE_MACHINE_TYPE} @@ -166,8 +164,6 @@ spec: spec: osDisk: diskSizeGB: 128 - managedDisk: - storageAccountType: Premium_LRS osType: Linux sshPublicKey: ${AZURE_SSH_PUBLIC_KEY_B64:=""} vmSize: ${AZURE_NODE_MACHINE_TYPE} diff --git a/templates/cluster-template-ipv6.yaml b/templates/cluster-template-ipv6.yaml index 32c71efb013..e631dbeb958 100644 --- a/templates/cluster-template-ipv6.yaml +++ b/templates/cluster-template-ipv6.yaml @@ -166,8 +166,6 @@ spec: enableIPForwarding: true osDisk: diskSizeGB: 128 - managedDisk: - storageAccountType: Premium_LRS osType: Linux sshPublicKey: ${AZURE_SSH_PUBLIC_KEY_B64:=""} vmSize: ${AZURE_CONTROL_PLANE_MACHINE_TYPE} diff --git a/templates/cluster-template-machinepool-system-assigned-identity.yaml b/templates/cluster-template-machinepool-system-assigned-identity.yaml index 3a693f8b26d..867bb2b1d1c 100644 --- a/templates/cluster-template-machinepool-system-assigned-identity.yaml +++ b/templates/cluster-template-machinepool-system-assigned-identity.yaml @@ -125,8 +125,6 @@ spec: nameSuffix: etcddisk osDisk: diskSizeGB: 128 - managedDisk: - storageAccountType: Premium_LRS osType: Linux sshPublicKey: ${AZURE_SSH_PUBLIC_KEY_B64:=""} vmSize: ${AZURE_CONTROL_PLANE_MACHINE_TYPE} diff --git a/templates/cluster-template-machinepool-user-assigned-identity.yaml b/templates/cluster-template-machinepool-user-assigned-identity.yaml index 7e1bb742313..c10a96aa122 100644 --- a/templates/cluster-template-machinepool-user-assigned-identity.yaml +++ b/templates/cluster-template-machinepool-user-assigned-identity.yaml @@ -125,8 +125,6 @@ spec: nameSuffix: etcddisk osDisk: diskSizeGB: 128 - managedDisk: - storageAccountType: Premium_LRS osType: Linux sshPublicKey: ${AZURE_SSH_PUBLIC_KEY_B64:=""} vmSize: ${AZURE_CONTROL_PLANE_MACHINE_TYPE} diff --git a/templates/cluster-template-machinepool-windows.yaml b/templates/cluster-template-machinepool-windows.yaml index c7317c14b08..2b573b77f0a 100644 --- a/templates/cluster-template-machinepool-windows.yaml +++ b/templates/cluster-template-machinepool-windows.yaml @@ -126,8 +126,6 @@ spec: nameSuffix: etcddisk osDisk: diskSizeGB: 128 - managedDisk: - storageAccountType: Premium_LRS osType: Linux sshPublicKey: ${AZURE_SSH_PUBLIC_KEY_B64:=""} vmSize: ${AZURE_CONTROL_PLANE_MACHINE_TYPE} diff --git a/templates/cluster-template-machinepool.yaml b/templates/cluster-template-machinepool.yaml index 158f6dcdcfc..876a13f4c5a 100644 --- a/templates/cluster-template-machinepool.yaml +++ b/templates/cluster-template-machinepool.yaml @@ -125,8 +125,6 @@ spec: nameSuffix: etcddisk osDisk: diskSizeGB: 128 - managedDisk: - storageAccountType: Premium_LRS osType: Linux sshPublicKey: ${AZURE_SSH_PUBLIC_KEY_B64:=""} vmSize: ${AZURE_CONTROL_PLANE_MACHINE_TYPE} diff --git a/templates/cluster-template-multi-tenancy.yaml b/templates/cluster-template-multi-tenancy.yaml index 81add9814b4..4305a90306a 100644 --- a/templates/cluster-template-multi-tenancy.yaml +++ b/templates/cluster-template-multi-tenancy.yaml @@ -130,8 +130,6 @@ spec: nameSuffix: etcddisk osDisk: diskSizeGB: 128 - managedDisk: - storageAccountType: Premium_LRS osType: Linux sshPublicKey: ${AZURE_SSH_PUBLIC_KEY_B64:=""} vmSize: ${AZURE_CONTROL_PLANE_MACHINE_TYPE} @@ -170,8 +168,6 @@ spec: spec: osDisk: diskSizeGB: 128 - managedDisk: - storageAccountType: Premium_LRS osType: Linux sshPublicKey: ${AZURE_SSH_PUBLIC_KEY_B64:=""} vmSize: ${AZURE_NODE_MACHINE_TYPE} diff --git a/templates/cluster-template-nvidia-gpu.yaml b/templates/cluster-template-nvidia-gpu.yaml index aa6c64876a4..fbf2e72bec4 100644 --- a/templates/cluster-template-nvidia-gpu.yaml +++ b/templates/cluster-template-nvidia-gpu.yaml @@ -126,8 +126,6 @@ spec: nameSuffix: etcddisk osDisk: diskSizeGB: 128 - managedDisk: - storageAccountType: Premium_LRS osType: Linux sshPublicKey: ${AZURE_SSH_PUBLIC_KEY_B64:=""} vmSize: ${AZURE_CONTROL_PLANE_MACHINE_TYPE} diff --git a/templates/cluster-template-private.yaml b/templates/cluster-template-private.yaml index d250af1ef2a..02d0a34b03c 100644 --- a/templates/cluster-template-private.yaml +++ b/templates/cluster-template-private.yaml @@ -134,8 +134,6 @@ spec: nameSuffix: etcddisk osDisk: diskSizeGB: 128 - managedDisk: - storageAccountType: Premium_LRS osType: Linux sshPublicKey: ${AZURE_SSH_PUBLIC_KEY_B64:=""} vmSize: ${AZURE_CONTROL_PLANE_MACHINE_TYPE} @@ -174,8 +172,6 @@ spec: spec: osDisk: diskSizeGB: 128 - managedDisk: - storageAccountType: Premium_LRS osType: Linux sshPublicKey: ${AZURE_SSH_PUBLIC_KEY_B64:=""} vmSize: ${AZURE_NODE_MACHINE_TYPE} diff --git a/templates/cluster-template-system-assigned-identity.yaml b/templates/cluster-template-system-assigned-identity.yaml index 2eb56fba9ac..66f18258025 100644 --- a/templates/cluster-template-system-assigned-identity.yaml +++ b/templates/cluster-template-system-assigned-identity.yaml @@ -126,8 +126,6 @@ spec: identity: SystemAssigned osDisk: diskSizeGB: 128 - managedDisk: - storageAccountType: Premium_LRS osType: Linux sshPublicKey: ${AZURE_SSH_PUBLIC_KEY_B64:=""} vmSize: ${AZURE_CONTROL_PLANE_MACHINE_TYPE} @@ -167,8 +165,6 @@ spec: identity: SystemAssigned osDisk: diskSizeGB: 128 - managedDisk: - storageAccountType: Premium_LRS osType: Linux sshPublicKey: ${AZURE_SSH_PUBLIC_KEY_B64:=""} vmSize: ${AZURE_NODE_MACHINE_TYPE} diff --git a/templates/cluster-template-user-assigned-identity.yaml b/templates/cluster-template-user-assigned-identity.yaml index b85191d352e..a69b1ad9e9e 100644 --- a/templates/cluster-template-user-assigned-identity.yaml +++ b/templates/cluster-template-user-assigned-identity.yaml @@ -126,8 +126,6 @@ spec: identity: UserAssigned osDisk: diskSizeGB: 128 - managedDisk: - storageAccountType: Premium_LRS osType: Linux sshPublicKey: ${AZURE_SSH_PUBLIC_KEY_B64:=""} userAssignedIdentities: @@ -169,8 +167,6 @@ spec: identity: UserAssigned osDisk: diskSizeGB: 128 - managedDisk: - storageAccountType: Premium_LRS osType: Linux sshPublicKey: ${AZURE_SSH_PUBLIC_KEY_B64:=""} userAssignedIdentities: diff --git a/templates/cluster-template-windows.yaml b/templates/cluster-template-windows.yaml index f1cf4ac00c7..68281a88bdf 100644 --- a/templates/cluster-template-windows.yaml +++ b/templates/cluster-template-windows.yaml @@ -126,8 +126,6 @@ spec: nameSuffix: etcddisk osDisk: diskSizeGB: 128 - managedDisk: - storageAccountType: Premium_LRS osType: Linux sshPublicKey: ${AZURE_SSH_PUBLIC_KEY_B64:=""} vmSize: ${AZURE_CONTROL_PLANE_MACHINE_TYPE} diff --git a/templates/cluster-template.yaml b/templates/cluster-template.yaml index edb6fb6b588..43f79771fb3 100644 --- a/templates/cluster-template.yaml +++ b/templates/cluster-template.yaml @@ -125,8 +125,6 @@ spec: nameSuffix: etcddisk osDisk: diskSizeGB: 128 - managedDisk: - storageAccountType: Premium_LRS osType: Linux sshPublicKey: ${AZURE_SSH_PUBLIC_KEY_B64:=""} vmSize: ${AZURE_CONTROL_PLANE_MACHINE_TYPE} @@ -165,8 +163,6 @@ spec: spec: osDisk: diskSizeGB: 128 - managedDisk: - storageAccountType: Premium_LRS osType: Linux sshPublicKey: ${AZURE_SSH_PUBLIC_KEY_B64:=""} vmSize: ${AZURE_NODE_MACHINE_TYPE} diff --git a/templates/flavors/base/cluster-template.yaml b/templates/flavors/base/cluster-template.yaml index 62ab15cd16a..1f7b2d35992 100644 --- a/templates/flavors/base/cluster-template.yaml +++ b/templates/flavors/base/cluster-template.yaml @@ -119,8 +119,6 @@ spec: osDisk: osType: "Linux" diskSizeGB: 128 - managedDisk: - storageAccountType: "Premium_LRS" dataDisks: - nameSuffix: etcddisk diskSizeGB: 256 diff --git a/templates/flavors/default/machine-deployment.yaml b/templates/flavors/default/machine-deployment.yaml index 87ebd82f16f..51ee081bef8 100644 --- a/templates/flavors/default/machine-deployment.yaml +++ b/templates/flavors/default/machine-deployment.yaml @@ -33,8 +33,6 @@ spec: osDisk: osType: "Linux" diskSizeGB: 128 - managedDisk: - storageAccountType: "Premium_LRS" sshPublicKey: ${AZURE_SSH_PUBLIC_KEY_B64:=""} --- apiVersion: bootstrap.cluster.x-k8s.io/v1alpha4 diff --git a/templates/flavors/ephemeral/patches/ephemeral.yaml b/templates/flavors/ephemeral/patches/ephemeral.yaml index be3fe804110..c72b7a99632 100644 --- a/templates/flavors/ephemeral/patches/ephemeral.yaml +++ b/templates/flavors/ephemeral/patches/ephemeral.yaml @@ -1,6 +1,7 @@ -- op: replace - path: /spec/template/spec/osDisk/managedDisk/storageAccountType - value: "Standard_LRS" +#- op: replace +# path: /spec/template/spec/osDisk/managedDisk +# value: +# storageAccountType: "Standard_LRS" - op: replace path: /spec/template/spec/osDisk/diffDiskSettings value: diff --git a/templates/test/ci/cluster-template-prow-ci-version.yaml b/templates/test/ci/cluster-template-prow-ci-version.yaml index b4b14594dfd..5acfc634bf5 100644 --- a/templates/test/ci/cluster-template-prow-ci-version.yaml +++ b/templates/test/ci/cluster-template-prow-ci-version.yaml @@ -214,8 +214,6 @@ spec: version: 2020.08.17 osDisk: diskSizeGB: 128 - managedDisk: - storageAccountType: Premium_LRS osType: Linux sshPublicKey: ${AZURE_SSH_PUBLIC_KEY_B64:=""} vmSize: ${AZURE_CONTROL_PLANE_MACHINE_TYPE} @@ -262,8 +260,6 @@ spec: version: 2020.08.17 osDisk: diskSizeGB: 128 - managedDisk: - storageAccountType: Premium_LRS osType: Linux sshPublicKey: ${AZURE_SSH_PUBLIC_KEY_B64:=""} vmSize: ${AZURE_NODE_MACHINE_TYPE} diff --git a/templates/test/ci/cluster-template-prow-external-cloud-provider.yaml b/templates/test/ci/cluster-template-prow-external-cloud-provider.yaml index a0339623302..49ffa7c8e4f 100644 --- a/templates/test/ci/cluster-template-prow-external-cloud-provider.yaml +++ b/templates/test/ci/cluster-template-prow-external-cloud-provider.yaml @@ -131,8 +131,6 @@ spec: nameSuffix: etcddisk osDisk: diskSizeGB: 128 - managedDisk: - storageAccountType: Premium_LRS osType: Linux sshPublicKey: ${AZURE_SSH_PUBLIC_KEY_B64:=""} vmSize: ${AZURE_CONTROL_PLANE_MACHINE_TYPE} @@ -171,8 +169,6 @@ spec: spec: osDisk: diskSizeGB: 128 - managedDisk: - storageAccountType: Premium_LRS osType: Linux sshPublicKey: ${AZURE_SSH_PUBLIC_KEY_B64:=""} vmSize: ${AZURE_NODE_MACHINE_TYPE} diff --git a/templates/test/ci/cluster-template-prow-ipv6.yaml b/templates/test/ci/cluster-template-prow-ipv6.yaml index a8ee5d2f8aa..6c26f0da487 100644 --- a/templates/test/ci/cluster-template-prow-ipv6.yaml +++ b/templates/test/ci/cluster-template-prow-ipv6.yaml @@ -171,8 +171,6 @@ spec: enableIPForwarding: true osDisk: diskSizeGB: 128 - managedDisk: - storageAccountType: Premium_LRS osType: Linux sshPublicKey: ${AZURE_SSH_PUBLIC_KEY_B64:=""} vmSize: ${AZURE_CONTROL_PLANE_MACHINE_TYPE} diff --git a/templates/test/ci/cluster-template-prow-machine-pool-ci-version.yaml b/templates/test/ci/cluster-template-prow-machine-pool-ci-version.yaml index 95c80284816..522b179c218 100644 --- a/templates/test/ci/cluster-template-prow-machine-pool-ci-version.yaml +++ b/templates/test/ci/cluster-template-prow-machine-pool-ci-version.yaml @@ -214,8 +214,6 @@ spec: version: 2020.08.17 osDisk: diskSizeGB: 128 - managedDisk: - storageAccountType: Premium_LRS osType: Linux sshPublicKey: ${AZURE_SSH_PUBLIC_KEY_B64:=""} vmSize: ${AZURE_CONTROL_PLANE_MACHINE_TYPE} diff --git a/templates/test/ci/cluster-template-prow-machine-pool-windows.yaml b/templates/test/ci/cluster-template-prow-machine-pool-windows.yaml index bc6fe1e71c7..692173c378a 100644 --- a/templates/test/ci/cluster-template-prow-machine-pool-windows.yaml +++ b/templates/test/ci/cluster-template-prow-machine-pool-windows.yaml @@ -131,8 +131,6 @@ spec: nameSuffix: etcddisk osDisk: diskSizeGB: 128 - managedDisk: - storageAccountType: Premium_LRS osType: Linux sshPublicKey: ${AZURE_SSH_PUBLIC_KEY_B64:=""} vmSize: ${AZURE_CONTROL_PLANE_MACHINE_TYPE} diff --git a/templates/test/ci/cluster-template-prow-machine-pool.yaml b/templates/test/ci/cluster-template-prow-machine-pool.yaml index 726c6fcb61b..55b142692ff 100644 --- a/templates/test/ci/cluster-template-prow-machine-pool.yaml +++ b/templates/test/ci/cluster-template-prow-machine-pool.yaml @@ -130,8 +130,6 @@ spec: nameSuffix: etcddisk osDisk: diskSizeGB: 128 - managedDisk: - storageAccountType: Premium_LRS osType: Linux sshPublicKey: ${AZURE_SSH_PUBLIC_KEY_B64:=""} vmSize: ${AZURE_CONTROL_PLANE_MACHINE_TYPE} diff --git a/templates/test/ci/cluster-template-prow-multi-tenancy.yaml b/templates/test/ci/cluster-template-prow-multi-tenancy.yaml index b5a48fc60bd..cb39099f47e 100644 --- a/templates/test/ci/cluster-template-prow-multi-tenancy.yaml +++ b/templates/test/ci/cluster-template-prow-multi-tenancy.yaml @@ -135,8 +135,6 @@ spec: nameSuffix: etcddisk osDisk: diskSizeGB: 128 - managedDisk: - storageAccountType: Premium_LRS osType: Linux sshPublicKey: ${AZURE_SSH_PUBLIC_KEY_B64:=""} vmSize: ${AZURE_CONTROL_PLANE_MACHINE_TYPE} @@ -175,8 +173,6 @@ spec: spec: osDisk: diskSizeGB: 128 - managedDisk: - storageAccountType: Premium_LRS osType: Linux sshPublicKey: ${AZURE_SSH_PUBLIC_KEY_B64:=""} vmSize: ${AZURE_NODE_MACHINE_TYPE} diff --git a/templates/test/ci/cluster-template-prow-nvidia-gpu.yaml b/templates/test/ci/cluster-template-prow-nvidia-gpu.yaml index 2808c8c674d..077c49b3417 100644 --- a/templates/test/ci/cluster-template-prow-nvidia-gpu.yaml +++ b/templates/test/ci/cluster-template-prow-nvidia-gpu.yaml @@ -131,8 +131,6 @@ spec: nameSuffix: etcddisk osDisk: diskSizeGB: 128 - managedDisk: - storageAccountType: Premium_LRS osType: Linux sshPublicKey: ${AZURE_SSH_PUBLIC_KEY_B64:=""} vmSize: ${AZURE_CONTROL_PLANE_MACHINE_TYPE} diff --git a/templates/test/ci/cluster-template-prow-private.yaml b/templates/test/ci/cluster-template-prow-private.yaml index f4954d52b84..26d1400a680 100644 --- a/templates/test/ci/cluster-template-prow-private.yaml +++ b/templates/test/ci/cluster-template-prow-private.yaml @@ -151,8 +151,6 @@ spec: nameSuffix: etcddisk osDisk: diskSizeGB: 128 - managedDisk: - storageAccountType: Premium_LRS osType: Linux sshPublicKey: ${AZURE_SSH_PUBLIC_KEY_B64:=""} vmSize: ${AZURE_CONTROL_PLANE_MACHINE_TYPE} @@ -191,8 +189,6 @@ spec: spec: osDisk: diskSizeGB: 128 - managedDisk: - storageAccountType: Premium_LRS osType: Linux sshPublicKey: ${AZURE_SSH_PUBLIC_KEY_B64:=""} vmSize: ${AZURE_NODE_MACHINE_TYPE} diff --git a/templates/test/ci/cluster-template-prow-windows.yaml b/templates/test/ci/cluster-template-prow-windows.yaml index bb75cc00d08..fac03c17486 100644 --- a/templates/test/ci/cluster-template-prow-windows.yaml +++ b/templates/test/ci/cluster-template-prow-windows.yaml @@ -131,8 +131,6 @@ spec: nameSuffix: etcddisk osDisk: diskSizeGB: 128 - managedDisk: - storageAccountType: Premium_LRS osType: Linux sshPublicKey: ${AZURE_SSH_PUBLIC_KEY_B64:=""} vmSize: ${AZURE_CONTROL_PLANE_MACHINE_TYPE} diff --git a/templates/test/ci/cluster-template-prow.yaml b/templates/test/ci/cluster-template-prow.yaml index cbfeb47d529..9fd96746ba4 100644 --- a/templates/test/ci/cluster-template-prow.yaml +++ b/templates/test/ci/cluster-template-prow.yaml @@ -130,8 +130,6 @@ spec: nameSuffix: etcddisk osDisk: diskSizeGB: 128 - managedDisk: - storageAccountType: Premium_LRS osType: Linux sshPublicKey: ${AZURE_SSH_PUBLIC_KEY_B64:=""} vmSize: ${AZURE_CONTROL_PLANE_MACHINE_TYPE} @@ -172,8 +170,6 @@ spec: spec: osDisk: diskSizeGB: 128 - managedDisk: - storageAccountType: Premium_LRS osType: Linux sshPublicKey: ${AZURE_SSH_PUBLIC_KEY_B64:=""} vmSize: ${AZURE_NODE_MACHINE_TYPE} diff --git a/templates/test/dev/cluster-template-custom-builds.yaml b/templates/test/dev/cluster-template-custom-builds.yaml index cd1a4fec2bf..7d04c293bdb 100644 --- a/templates/test/dev/cluster-template-custom-builds.yaml +++ b/templates/test/dev/cluster-template-custom-builds.yaml @@ -194,8 +194,6 @@ spec: nameSuffix: etcddisk osDisk: diskSizeGB: 128 - managedDisk: - storageAccountType: Premium_LRS osType: Linux sshPublicKey: ${AZURE_SSH_PUBLIC_KEY_B64:=""} vmSize: ${AZURE_CONTROL_PLANE_MACHINE_TYPE} @@ -234,8 +232,6 @@ spec: spec: osDisk: diskSizeGB: 128 - managedDisk: - storageAccountType: Premium_LRS osType: Linux sshPublicKey: ${AZURE_SSH_PUBLIC_KEY_B64:=""} vmSize: ${AZURE_NODE_MACHINE_TYPE} From a58e6a220450b9c676322c2787c03e342ce1acca Mon Sep 17 00:00:00 2001 From: Cecile Robert-Michon Date: Fri, 16 Apr 2021 10:35:56 -0700 Subject: [PATCH 2/2] fix ephemeral OS disk caching --- api/v1alpha4/types.go | 3 +++ ...rastructure.cluster.x-k8s.io_azuremachinepools.yaml | 10 ++++++++++ .../infrastructure.cluster.x-k8s.io_azuremachines.yaml | 10 ++++++++++ ...ructure.cluster.x-k8s.io_azuremachinetemplates.yaml | 10 ++++++++++ templates/cluster-template-ephemeral.yaml | 6 ++++-- templates/flavors/ephemeral/patches/ephemeral.yaml | 10 ++++++---- 6 files changed, 43 insertions(+), 6 deletions(-) diff --git a/api/v1alpha4/types.go b/api/v1alpha4/types.go index c1496041ec1..79c9b1264a0 100644 --- a/api/v1alpha4/types.go +++ b/api/v1alpha4/types.go @@ -370,6 +370,7 @@ type OSDisk struct { DiffDiskSettings *DiffDiskSettings `json:"diffDiskSettings,omitempty"` // CachingType specifies the caching requirements. // +optional + // +kubebuilder:validation:Enum=None;ReadOnly;ReadWrite CachingType string `json:"cachingType,omitempty"` } @@ -386,7 +387,9 @@ type DataDisk struct { // Lun Specifies the logical unit number of the data disk. This value is used to identify data disks within the VM and therefore must be unique for each data disk attached to a VM. // The value must be between 0 and 63. Lun *int32 `json:"lun,omitempty"` + // CachingType specifies the caching requirements. // +optional + // +kubebuilder:validation:Enum=None;ReadOnly;ReadWrite CachingType string `json:"cachingType,omitempty"` } diff --git a/config/crd/bases/exp.infrastructure.cluster.x-k8s.io_azuremachinepools.yaml b/config/crd/bases/exp.infrastructure.cluster.x-k8s.io_azuremachinepools.yaml index cf32ec249e8..84141e57cfc 100644 --- a/config/crd/bases/exp.infrastructure.cluster.x-k8s.io_azuremachinepools.yaml +++ b/config/crd/bases/exp.infrastructure.cluster.x-k8s.io_azuremachinepools.yaml @@ -485,6 +485,11 @@ spec: description: DataDisk specifies the parameters that are used to add one or more data disks to the machine. properties: cachingType: + description: CachingType specifies the caching requirements. + enum: + - None + - ReadOnly + - ReadWrite type: string diskSizeGB: description: DiskSizeGB is the size in GB to assign to the data disk. @@ -585,6 +590,11 @@ spec: description: OSDisk contains the operating system disk information for a Virtual Machine properties: cachingType: + description: CachingType specifies the caching requirements. + enum: + - None + - ReadOnly + - ReadWrite type: string diffDiskSettings: description: DiffDiskSettings describe ephemeral disk settings for the os disk. 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 a2d90c03c70..152859a9663 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremachines.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremachines.yaml @@ -418,6 +418,11 @@ spec: description: DataDisk specifies the parameters that are used to add one or more data disks to the machine. properties: cachingType: + description: CachingType specifies the caching requirements. + enum: + - None + - ReadOnly + - ReadWrite type: string diskSizeGB: description: DiskSizeGB is the size in GB to assign to the data disk. @@ -532,6 +537,11 @@ spec: description: OSDisk specifies the parameters for the operating system disk of the machine properties: cachingType: + description: CachingType specifies the caching requirements. + enum: + - None + - ReadOnly + - ReadWrite type: string diffDiskSettings: description: DiffDiskSettings describe ephemeral disk settings for the os disk. 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 d2c353cdc5a..ba504acdcdf 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremachinetemplates.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremachinetemplates.yaml @@ -315,6 +315,11 @@ spec: description: DataDisk specifies the parameters that are used to add one or more data disks to the machine. properties: cachingType: + description: CachingType specifies the caching requirements. + enum: + - None + - ReadOnly + - ReadWrite type: string diskSizeGB: description: DiskSizeGB is the size in GB to assign to the data disk. @@ -429,6 +434,11 @@ spec: description: OSDisk specifies the parameters for the operating system disk of the machine properties: cachingType: + description: CachingType specifies the caching requirements. + enum: + - None + - ReadOnly + - ReadWrite type: string diffDiskSettings: description: DiffDiskSettings describe ephemeral disk settings for the os disk. diff --git a/templates/cluster-template-ephemeral.yaml b/templates/cluster-template-ephemeral.yaml index 4cc514bfa35..3433627f197 100644 --- a/templates/cluster-template-ephemeral.yaml +++ b/templates/cluster-template-ephemeral.yaml @@ -124,9 +124,10 @@ spec: lun: 0 nameSuffix: etcddisk osDisk: + cachingType: ReadOnly diffDiskSettings: option: Local - diskSizeGB: 128 + diskSizeGB: 50 osType: Linux sshPublicKey: ${AZURE_SSH_PUBLIC_KEY_B64:=""} vmSize: ${AZURE_CONTROL_PLANE_MACHINE_TYPE} @@ -164,9 +165,10 @@ spec: template: spec: osDisk: + cachingType: ReadOnly diffDiskSettings: option: Local - diskSizeGB: 128 + diskSizeGB: 50 osType: Linux sshPublicKey: ${AZURE_SSH_PUBLIC_KEY_B64:=""} vmSize: ${AZURE_NODE_MACHINE_TYPE} diff --git a/templates/flavors/ephemeral/patches/ephemeral.yaml b/templates/flavors/ephemeral/patches/ephemeral.yaml index c72b7a99632..8c8ebd2398a 100644 --- a/templates/flavors/ephemeral/patches/ephemeral.yaml +++ b/templates/flavors/ephemeral/patches/ephemeral.yaml @@ -1,8 +1,10 @@ -#- op: replace -# path: /spec/template/spec/osDisk/managedDisk -# value: -# storageAccountType: "Standard_LRS" - op: replace path: /spec/template/spec/osDisk/diffDiskSettings value: option: Local +- op: replace + path: /spec/template/spec/osDisk/diskSizeGB + value: 50 +- op: replace + path: /spec/template/spec/osDisk/cachingType + value: "ReadOnly"