diff --git a/pkg/cloudprovider/provider/azure/provider.go b/pkg/cloudprovider/provider/azure/provider.go index 2f7ef1d9a..0ed00fe2a 100644 --- a/pkg/cloudprovider/provider/azure/provider.go +++ b/pkg/cloudprovider/provider/azure/provider.go @@ -22,6 +22,8 @@ import ( "encoding/json" "errors" "fmt" + "slices" + "strconv" "strings" "sync" "time" @@ -102,10 +104,11 @@ type config struct { ImagePlan *compute.Plan ImageReference *compute.ImageReference - OSDiskSize int32 - OSDiskSKU *compute.StorageAccountTypes - DataDiskSize int32 - DataDiskSKU *compute.StorageAccountTypes + OSDiskSize int32 + OSDiskSKU *compute.StorageAccountTypes + DataDiskSize int32 + DataDiskSKU *compute.StorageAccountTypes + DiskControllerType *compute.DiskControllerTypes AssignPublicIP bool PublicIPSKU *network.PublicIPAddressSkuName @@ -343,6 +346,11 @@ func (p *provider) getConfig(provSpec clusterv1alpha1.ProviderSpec) (*config, *p c.DataDiskSKU = storageTypePtr(*rawCfg.DataDiskSKU) } + if rawCfg.DiskControllerType != nil { + dct := compute.DiskControllerTypes(*rawCfg.DiskControllerType) + c.DiskControllerType = &dct + } + if rawCfg.ImagePlan != nil && rawCfg.ImagePlan.Name != "" { c.ImagePlan = &compute.Plan{ Name: ptr.To(rawCfg.ImagePlan.Name), @@ -627,6 +635,13 @@ func getStorageProfile(config *config, providerCfg *providerconfig.Config) (*com } } } + + if config.DiskControllerType != nil { + sp.DiskControllerType = *config.DiskControllerType + } else if vmSizeRequiresNVMe(config.VMSize) { + sp.DiskControllerType = compute.NVMe + } + return sp, nil } @@ -1025,6 +1040,18 @@ func validateDiskSKUs(_ context.Context, c *config, sku compute.ResourceSku) err return nil } +func validateDiskControllerType(_ context.Context, c *config, sku compute.ResourceSku) error { + if c.DiskControllerType != nil { + if !slices.Contains(compute.PossibleDiskControllerTypesValues(), *c.DiskControllerType) { + return fmt.Errorf("invalid diskControllerType %q, valid values are: %v", *c.DiskControllerType, compute.PossibleDiskControllerTypesValues()) + } + if *c.DiskControllerType == compute.SCSI && skuRequiresNVMe(sku) { + return fmt.Errorf("VM size %q only supports NVMe disk controller, cannot use %q", c.VMSize, compute.SCSI) + } + } + return nil +} + func validateSKUCapabilities(_ context.Context, c *config, sku compute.ResourceSku) error { if c.EnableAcceleratedNetworking != nil && *c.EnableAcceleratedNetworking { if !SKUHasCapability(sku, capabilityAcceleratedNetworking) { @@ -1185,6 +1212,9 @@ func (p *provider) Validate(ctx context.Context, log *zap.SugaredLogger, spec cl if err := validateSecurityProfile(ctx, c, sku); err != nil { return fmt.Errorf("failed to validate security profile: %w", err) } + if err := validateDiskControllerType(ctx, c, sku); err != nil { + return fmt.Errorf("failed to validate disk controller type: %w", err) + } _, err = getOSImageReference(c, providerConfig.OperatingSystem) return err @@ -1368,13 +1398,7 @@ func supportsDiskSKU(vmSKU compute.ResourceSku, diskSKU compute.StorageAccountTy for _, zone := range zones { found := false for _, details := range *(*vmSKU.LocationInfo)[0].ZoneDetails { - matchesZone := false - for _, zoneName := range *details.Name { - if zone == zoneName { - matchesZone = true - break - } - } + matchesZone := slices.Contains(*details.Name, zone) // we only check this zone details for capabilities if it actually includes the zone we're checking for if matchesZone { @@ -1428,6 +1452,34 @@ func skuSupportsGen2(sku compute.ResourceSku) bool { return strings.Contains(generations, "V2") } +// skuRequiresNVMe checks if a VM SKU only supports NVMe disk controller using the Azure SKU API. +// v6 and later generation VMs exclusively support NVMe; the DiskControllerTypes capability +// value is "NVMe" (no "SCSI") for those SKUs. +func skuRequiresNVMe(sku compute.ResourceSku) bool { + if sku.Capabilities == nil { + return false + } + for _, cap := range *sku.Capabilities { + if cap.Name != nil && *cap.Name == "DiskControllerTypes" && cap.Value != nil { + v := strings.ToLower(*cap.Value) + return v == "nvme" || (strings.Contains(v, "nvme") && !strings.Contains(v, "scsi")) + } + } + return false +} + +// vmSizeRequiresNVMe checks if a VM size requires NVMe disk controller using heuristics. +// Azure v6 and later generation VMs (e.g. Standard_D4s_v6) do not support SCSI disk controllers. +func vmSizeRequiresNVMe(vmSize string) bool { + size := strings.ToLower(vmSize) + idx := strings.LastIndex(size, "_v") + if idx == -1 { + return false + } + n, err := strconv.Atoi(size[idx+2:]) + return err == nil && n >= 6 +} + // vmSizeSupportsGen2 checks if a VM size is known to support Generation 2 VMs using heuristics. func vmSizeSupportsGen2(vmSize string) bool { size := strings.ToLower(vmSize) diff --git a/pkg/cloudprovider/provider/azure/provider_test.go b/pkg/cloudprovider/provider/azure/provider_test.go index 2c359d227..35d727256 100644 --- a/pkg/cloudprovider/provider/azure/provider_test.go +++ b/pkg/cloudprovider/provider/azure/provider_test.go @@ -121,6 +121,20 @@ func skuWithoutGenCap() compute.ResourceSku { } } +func skuWithDiskControllerTypes(value string) compute.ResourceSku { + return compute.ResourceSku{ + Capabilities: &[]compute.ResourceSkuCapabilities{ + {Name: to.StringPtr("DiskControllerTypes"), Value: to.StringPtr(value)}, + }, + } +} + +func nvmeOnlySKU() compute.ResourceSku { return skuWithDiskControllerTypes("NVMe") } +func scsiAndNvmeSKU() compute.ResourceSku { + return skuWithDiskControllerTypes("SCSI,NVMe") +} +func scsiOnlySKU() compute.ResourceSku { return skuWithDiskControllerTypes("SCSI") } + func TestValidateSecurityProfile(t *testing.T) { tests := []struct { name string @@ -475,7 +489,6 @@ func TestBuildSecurityProfile(t *testing.T) { }, }, } - for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { got := buildSecurityProfile(tt.raw) @@ -485,3 +498,196 @@ func TestBuildSecurityProfile(t *testing.T) { }) } } +func TestVMSizeRequiresNVMe(t *testing.T) { + tests := []struct { + name string + vmSize string + expected bool + }{ + { + name: "Standard_D4s_v6 requires NVMe", + vmSize: "Standard_D4s_v6", + expected: true, + }, + { + name: "Standard_E8s_v6 requires NVMe", + vmSize: "Standard_E8s_v6", + expected: true, + }, + { + name: "lowercase standard_d4s_v6 requires NVMe", + vmSize: "standard_d4s_v6", + expected: true, + }, + { + name: "Standard_D4s_v5 does not require NVMe", + vmSize: "Standard_D4s_v5", + expected: false, + }, + { + name: "Standard_D4s_v3 does not require NVMe", + vmSize: "Standard_D4s_v3", + expected: false, + }, + { + name: "Standard_NC40ads_H100_v5 does not require NVMe", + vmSize: "Standard_NC40ads_H100_v5", + expected: false, + }, + { + name: "Standard_B2ms (no version suffix) does not require NVMe", + vmSize: "Standard_B2ms", + expected: false, + }, + { + name: "Standard_A2 does not require NVMe", + vmSize: "Standard_A2", + expected: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := vmSizeRequiresNVMe(tt.vmSize) + if result != tt.expected { + t.Errorf("vmSizeRequiresNVMe(%s) = %v, expected %v", tt.vmSize, result, tt.expected) + } + }) + } +} + +func TestSkuRequiresNVMe(t *testing.T) { + tests := []struct { + name string + sku compute.ResourceSku + expected bool + }{ + { + name: "SKU with only NVMe capability requires NVMe", + sku: nvmeOnlySKU(), + expected: true, + }, + { + name: "SKU with lowercase nvme capability requires NVMe", + sku: skuWithDiskControllerTypes("nvme"), + expected: true, + }, + { + name: "SKU supporting both SCSI and NVMe does not require NVMe", + sku: scsiAndNvmeSKU(), + expected: false, + }, + { + name: "SKU with only SCSI capability does not require NVMe", + sku: scsiOnlySKU(), + expected: false, + }, + { + name: "SKU without DiskControllerTypes capability does not require NVMe", + sku: skuWithoutGenCap(), + expected: false, + }, + { + name: "SKU with nil Capabilities does not require NVMe", + sku: compute.ResourceSku{}, + expected: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := skuRequiresNVMe(tt.sku) + if result != tt.expected { + t.Errorf("skuRequiresNVMe() = %v, expected %v", result, tt.expected) + } + }) + } +} + +func TestValidateDiskControllerType(t *testing.T) { + tests := []struct { + name string + config *config + sku compute.ResourceSku + expectError bool + }{ + { + name: "nil DiskControllerType passes regardless of SKU", + config: &config{VMSize: testVMSizeGen2}, + sku: nvmeOnlySKU(), + expectError: false, + }, + { + name: "explicit NVMe on NVMe-only SKU passes", + config: &config{ + VMSize: "Standard_D4s_v6", + DiskControllerType: ptr.To(compute.NVMe), + }, + sku: nvmeOnlySKU(), + expectError: false, + }, + { + name: "explicit SCSI on NVMe-only SKU fails", + config: &config{ + VMSize: "Standard_D4s_v6", + DiskControllerType: ptr.To(compute.SCSI), + }, + sku: nvmeOnlySKU(), + expectError: true, + }, + { + name: "explicit SCSI on SKU supporting both SCSI and NVMe passes", + config: &config{ + VMSize: testVMSizeGen2, + DiskControllerType: ptr.To(compute.SCSI), + }, + sku: scsiAndNvmeSKU(), + expectError: false, + }, + { + name: "explicit SCSI on SCSI-only SKU passes", + config: &config{ + VMSize: testVMSizeGen1, + DiskControllerType: ptr.To(compute.SCSI), + }, + sku: scsiOnlySKU(), + expectError: false, + }, + { + name: "explicit SCSI on SKU without DiskControllerTypes capability passes", + config: &config{ + VMSize: testVMSizeGen1, + DiskControllerType: ptr.To(compute.SCSI), + }, + sku: skuWithoutGenCap(), + expectError: false, + }, + { + name: "invalid diskControllerType value fails", + config: &config{ + VMSize: testVMSizeGen2, + DiskControllerType: ptr.To(compute.DiskControllerTypes("Nonsense")), + }, + sku: gen2SKU(), + expectError: true, + }, + { + name: "lowercase nvme fails (case-sensitive)", + config: &config{ + VMSize: testVMSizeGen2, + DiskControllerType: ptr.To(compute.DiskControllerTypes("nvme")), + }, + sku: gen2SKU(), + expectError: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := validateDiskControllerType(context.Background(), tt.config, tt.sku) + if (err != nil) != tt.expectError { + t.Errorf("validateDiskControllerType() error = %v, expectError %v", err, tt.expectError) + } + }) + } +} diff --git a/sdk/cloudprovider/azure/types.go b/sdk/cloudprovider/azure/types.go index ebdb4e670..4cc7b8514 100644 --- a/sdk/cloudprovider/azure/types.go +++ b/sdk/cloudprovider/azure/types.go @@ -52,14 +52,15 @@ type RawConfig struct { EnableAcceleratedNetworking *bool `json:"enableAcceleratedNetworking"` EnableBootDiagnostics *bool `json:"enableBootDiagnostics,omitempty"` - ImageID providerconfig.ConfigVarString `json:"imageID"` - OSDiskSize int32 `json:"osDiskSize"` - OSDiskSKU *string `json:"osDiskSKU,omitempty"` - DataDiskSize int32 `json:"dataDiskSize"` - DataDiskSKU *string `json:"dataDiskSKU,omitempty"` - AssignPublicIP providerconfig.ConfigVarBool `json:"assignPublicIP"` - PublicIPSKU *string `json:"publicIPSKU,omitempty"` - Tags map[string]string `json:"tags,omitempty"` + ImageID providerconfig.ConfigVarString `json:"imageID"` + OSDiskSize int32 `json:"osDiskSize"` + OSDiskSKU *string `json:"osDiskSKU,omitempty"` + DiskControllerType *string `json:"diskControllerType,omitempty"` + DataDiskSize int32 `json:"dataDiskSize"` + DataDiskSKU *string `json:"dataDiskSKU,omitempty"` + AssignPublicIP providerconfig.ConfigVarBool `json:"assignPublicIP"` + PublicIPSKU *string `json:"publicIPSKU,omitempty"` + Tags map[string]string `json:"tags,omitempty"` SecurityProfile *SecurityProfile `json:"securityProfile,omitempty"` }