Skip to content

Commit 0ed137c

Browse files
bryan-coxclaude
andcommitted
refactor(scalesets): extract helper methods from validateSpec()
Split the validateSpec() method (107 lines, complexity 30) into focused helper methods to improve readability and maintainability. Extracted methods: - validateVMCapabilities: validate vCPU, memory, ephemeral OS, encryption - validateUltraDiskSupport: validate ultra disk support for location/zones - requiresUltraDiskValidation: check if ultra disk validation is needed - validateUltraDiskInZone: validate ultra disk support per zone - validateDiagnosticsProfile: validate diagnostics configuration - validateAvailabilityZones: validate availability zone support No functional changes - all existing tests pass. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> Co-Authored-By: Bryan Cox <[email protected]>
1 parent a91606d commit 0ed137c

File tree

1 file changed

+119
-50
lines changed

1 file changed

+119
-50
lines changed

azure/services/scalesets/scalesets.go

Lines changed: 119 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -197,93 +197,162 @@ func (s *Service) validateSpec(ctx context.Context) error {
197197
return errors.Wrapf(err, "failed to get SKU %s in compute api", scaleSetSpec.Size)
198198
}
199199

200-
// Checking if the requested VM size has at least 2 vCPUS
200+
if err := s.validateVMCapabilities(sku, scaleSetSpec); err != nil {
201+
return err
202+
}
203+
204+
if err := s.validateUltraDiskSupport(ctx, sku, scaleSetSpec); err != nil {
205+
return err
206+
}
207+
208+
if err := validateDiagnosticsProfile(scaleSetSpec); err != nil {
209+
return err
210+
}
211+
212+
return s.validateAvailabilityZones(ctx, scaleSetSpec)
213+
}
214+
215+
// validateVMCapabilities validates the VM size capabilities (vCPU, memory, ephemeral OS, encryption).
216+
func (s *Service) validateVMCapabilities(sku resourceskus.SKU, spec *ScaleSetSpec) error {
217+
// Validate minimum vCPU requirement.
201218
vCPUCapability, err := sku.HasCapabilityWithCapacity(resourceskus.VCPUs, resourceskus.MinimumVCPUS)
202219
if err != nil {
203220
return azure.WithTerminalError(errors.Wrap(err, "failed to validate the vCPU capability"))
204221
}
205-
206222
if !vCPUCapability {
207223
return azure.WithTerminalError(errors.New("vm size should be bigger or equal to at least 2 vCPUs"))
208224
}
209225

210-
// Checking if the requested VM size has at least 2 Gi of memory
211-
MemoryCapability, err := sku.HasCapabilityWithCapacity(resourceskus.MemoryGB, resourceskus.MinimumMemory)
226+
// Validate minimum memory requirement.
227+
memoryCapability, err := sku.HasCapabilityWithCapacity(resourceskus.MemoryGB, resourceskus.MinimumMemory)
212228
if err != nil {
213229
return azure.WithTerminalError(errors.Wrap(err, "failed to validate the memory capability"))
214230
}
215-
216-
if !MemoryCapability {
231+
if !memoryCapability {
217232
return azure.WithTerminalError(errors.New("vm memory should be bigger or equal to at least 2Gi"))
218233
}
219234

220-
// enable ephemeral OS
221-
if scaleSetSpec.OSDisk.DiffDiskSettings != nil && !sku.HasCapability(resourceskus.EphemeralOSDisk) {
222-
return azure.WithTerminalError(fmt.Errorf("vm size %s does not support ephemeral os. select a different vm size or disable ephemeral os", scaleSetSpec.Size))
235+
// Validate ephemeral OS disk support.
236+
if spec.OSDisk.DiffDiskSettings != nil && !sku.HasCapability(resourceskus.EphemeralOSDisk) {
237+
return azure.WithTerminalError(fmt.Errorf("vm size %s does not support ephemeral os. select a different vm size or disable ephemeral os", spec.Size))
223238
}
224239

225-
if scaleSetSpec.SecurityProfile != nil && !sku.HasCapability(resourceskus.EncryptionAtHost) {
226-
return azure.WithTerminalError(errors.Errorf("encryption at host is not supported for VM type %s", scaleSetSpec.Size))
240+
// Validate encryption at host support.
241+
if spec.SecurityProfile != nil && !sku.HasCapability(resourceskus.EncryptionAtHost) {
242+
return azure.WithTerminalError(errors.Errorf("encryption at host is not supported for VM type %s", spec.Size))
227243
}
228244

229-
// Fetch location and zone to check for their support of ultra disks.
230-
zones, err := s.resourceSKUCache.GetZones(ctx, scaleSetSpec.Location)
245+
return nil
246+
}
247+
248+
// validateUltraDiskSupport validates ultra disk support for the specified location and zones.
249+
func (s *Service) validateUltraDiskSupport(ctx context.Context, sku resourceskus.SKU, spec *ScaleSetSpec) error {
250+
if !s.requiresUltraDiskValidation(spec) {
251+
return nil
252+
}
253+
254+
zones, err := s.resourceSKUCache.GetZones(ctx, spec.Location)
231255
if err != nil {
232-
return azure.WithTerminalError(errors.Wrapf(err, "failed to get the zones for location %s", scaleSetSpec.Location))
256+
return azure.WithTerminalError(errors.Wrapf(err, "failed to get the zones for location %s", spec.Location))
233257
}
234258

235259
for _, zone := range zones {
236-
hasLocationCapability := sku.HasLocationCapability(resourceskus.UltraSSDAvailable, scaleSetSpec.Location, zone)
237-
err := fmt.Errorf("vm size %s does not support ultra disks in location %s. select a different vm size or disable ultra disks", scaleSetSpec.Size, scaleSetSpec.Location)
238-
239-
// Check support for ultra disks as data disks.
240-
for _, disks := range scaleSetSpec.DataDisks {
241-
if disks.ManagedDisk != nil &&
242-
disks.ManagedDisk.StorageAccountType == string(armcompute.StorageAccountTypesUltraSSDLRS) &&
243-
!hasLocationCapability {
244-
return azure.WithTerminalError(err)
245-
}
260+
hasLocationCapability := sku.HasLocationCapability(resourceskus.UltraSSDAvailable, spec.Location, zone)
261+
if err := s.validateUltraDiskInZone(spec, hasLocationCapability); err != nil {
262+
return err
246263
}
247-
// Check support for ultra disks as persistent volumes.
248-
if scaleSetSpec.AdditionalCapabilities != nil && scaleSetSpec.AdditionalCapabilities.UltraSSDEnabled != nil {
249-
if *scaleSetSpec.AdditionalCapabilities.UltraSSDEnabled &&
250-
!hasLocationCapability {
251-
return azure.WithTerminalError(err)
252-
}
264+
}
265+
266+
return nil
267+
}
268+
269+
// requiresUltraDiskValidation checks if ultra disk validation is needed.
270+
func (s *Service) requiresUltraDiskValidation(spec *ScaleSetSpec) bool {
271+
// Check if any data disk uses ultra SSD.
272+
for _, disk := range spec.DataDisks {
273+
if disk.ManagedDisk != nil && disk.ManagedDisk.StorageAccountType == string(armcompute.StorageAccountTypesUltraSSDLRS) {
274+
return true
253275
}
254276
}
255277

256-
// Validate DiagnosticProfile spec
257-
if scaleSetSpec.DiagnosticsProfile != nil && scaleSetSpec.DiagnosticsProfile.Boot != nil {
258-
if scaleSetSpec.DiagnosticsProfile.Boot.StorageAccountType == infrav1.UserManagedDiagnosticsStorage {
259-
if scaleSetSpec.DiagnosticsProfile.Boot.UserManaged == nil {
260-
return azure.WithTerminalError(fmt.Errorf("userManaged must be specified when storageAccountType is '%s'", infrav1.UserManagedDiagnosticsStorage))
261-
} else if scaleSetSpec.DiagnosticsProfile.Boot.UserManaged.StorageAccountURI == "" {
262-
return azure.WithTerminalError(fmt.Errorf("storageAccountURI cannot be empty when storageAccountType is '%s'", infrav1.UserManagedDiagnosticsStorage))
263-
}
278+
// Check if ultra SSD is enabled via additional capabilities.
279+
if spec.AdditionalCapabilities != nil && spec.AdditionalCapabilities.UltraSSDEnabled != nil {
280+
return *spec.AdditionalCapabilities.UltraSSDEnabled
281+
}
282+
283+
return false
284+
}
285+
286+
// validateUltraDiskInZone validates ultra disk support for a specific zone.
287+
func (s *Service) validateUltraDiskInZone(spec *ScaleSetSpec, hasLocationCapability bool) error {
288+
if hasLocationCapability {
289+
return nil
290+
}
291+
292+
ultraDiskErr := fmt.Errorf("vm size %s does not support ultra disks in location %s. select a different vm size or disable ultra disks", spec.Size, spec.Location)
293+
294+
// Check data disks.
295+
for _, disk := range spec.DataDisks {
296+
if disk.ManagedDisk != nil && disk.ManagedDisk.StorageAccountType == string(armcompute.StorageAccountTypesUltraSSDLRS) {
297+
return azure.WithTerminalError(ultraDiskErr)
264298
}
299+
}
265300

266-
possibleStorageAccountTypeValues := []string{
267-
string(infrav1.DisabledDiagnosticsStorage),
268-
string(infrav1.ManagedDiagnosticsStorage),
269-
string(infrav1.UserManagedDiagnosticsStorage),
301+
// Check additional capabilities.
302+
if spec.AdditionalCapabilities != nil && spec.AdditionalCapabilities.UltraSSDEnabled != nil {
303+
if *spec.AdditionalCapabilities.UltraSSDEnabled {
304+
return azure.WithTerminalError(ultraDiskErr)
270305
}
306+
}
307+
308+
return nil
309+
}
271310

272-
if !slice.Contains(possibleStorageAccountTypeValues, string(scaleSetSpec.DiagnosticsProfile.Boot.StorageAccountType)) {
273-
return azure.WithTerminalError(fmt.Errorf("invalid storageAccountType: %s. Allowed values are %v",
274-
scaleSetSpec.DiagnosticsProfile.Boot.StorageAccountType, possibleStorageAccountTypeValues))
311+
// validateDiagnosticsProfile validates the diagnostics profile configuration.
312+
func validateDiagnosticsProfile(spec *ScaleSetSpec) error {
313+
if spec.DiagnosticsProfile == nil || spec.DiagnosticsProfile.Boot == nil {
314+
return nil
315+
}
316+
317+
boot := spec.DiagnosticsProfile.Boot
318+
319+
// Validate user-managed storage account configuration.
320+
if boot.StorageAccountType == infrav1.UserManagedDiagnosticsStorage {
321+
if boot.UserManaged == nil {
322+
return azure.WithTerminalError(fmt.Errorf("userManaged must be specified when storageAccountType is '%s'", infrav1.UserManagedDiagnosticsStorage))
275323
}
324+
if boot.UserManaged.StorageAccountURI == "" {
325+
return azure.WithTerminalError(fmt.Errorf("storageAccountURI cannot be empty when storageAccountType is '%s'", infrav1.UserManagedDiagnosticsStorage))
326+
}
327+
}
328+
329+
// Validate storage account type is valid.
330+
validStorageTypes := []string{
331+
string(infrav1.DisabledDiagnosticsStorage),
332+
string(infrav1.ManagedDiagnosticsStorage),
333+
string(infrav1.UserManagedDiagnosticsStorage),
334+
}
335+
if !slice.Contains(validStorageTypes, string(boot.StorageAccountType)) {
336+
return azure.WithTerminalError(fmt.Errorf("invalid storageAccountType: %s. Allowed values are %v", boot.StorageAccountType, validStorageTypes))
337+
}
338+
339+
return nil
340+
}
341+
342+
// validateAvailabilityZones validates that the requested availability zones are available for the VM size.
343+
func (s *Service) validateAvailabilityZones(ctx context.Context, spec *ScaleSetSpec) error {
344+
if len(spec.FailureDomains) == 0 {
345+
return nil
276346
}
277347

278-
// Checking if selected availability zones are available selected VM type in location
279-
azsInLocation, err := s.resourceSKUCache.GetZonesWithVMSize(ctx, scaleSetSpec.Size, scaleSetSpec.Location)
348+
azsInLocation, err := s.resourceSKUCache.GetZonesWithVMSize(ctx, spec.Size, spec.Location)
280349
if err != nil {
281-
return errors.Wrapf(err, "failed to get zones for VM type %s in location %s", scaleSetSpec.Size, scaleSetSpec.Location)
350+
return errors.Wrapf(err, "failed to get zones for VM type %s in location %s", spec.Size, spec.Location)
282351
}
283352

284-
for _, az := range scaleSetSpec.FailureDomains {
353+
for _, az := range spec.FailureDomains {
285354
if !slice.Contains(azsInLocation, az) {
286-
return azure.WithTerminalError(errors.Errorf("availability zone %s is not available for VM type %s in location %s", az, scaleSetSpec.Size, scaleSetSpec.Location))
355+
return azure.WithTerminalError(errors.Errorf("availability zone %s is not available for VM type %s in location %s", az, spec.Size, spec.Location))
287356
}
288357
}
289358

0 commit comments

Comments
 (0)