From 25dd8c350397063be07530030d760175f6438f53 Mon Sep 17 00:00:00 2001 From: Bryce Soghigian Date: Thu, 17 Jul 2025 19:19:48 -0700 Subject: [PATCH 1/2] feat: added - HasSCSISupport() - checks SCSI support with backward compatibility - HasNVMeSupport() - checks NVMe support - SupportsNVMeEphemeralOSDisk() - checks NVMe ephemeral disk support - NVMeDiskSizeInMiB() - gets NVMe disk size fix: removing max os disk claude wasn't supposed to lift that afs --- const.go | 15 +++ disk.go | 26 +++++ disk_test.go | 282 +++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 323 insertions(+) create mode 100644 disk.go create mode 100644 disk_test.go diff --git a/const.go b/const.go index 1b6f8fb..d39808d 100644 --- a/const.go +++ b/const.go @@ -51,6 +51,12 @@ const ( CapabilityConfidentialComputingType = "ConfidentialComputingType" // ConfidentialComputingTypeSNP denoted the "SNP" ConfidentialComputing. ConfidentialComputingTypeSNP = "SNP" + // DiskControllerTypes identifies the disk controller types supported by the VM SKU. + DiskControllerTypes = "DiskControllerTypes" + // SupportedEphemeralOSDiskPlacements identifies supported ephemeral OS disk placements. + SupportedEphemeralOSDiskPlacements = "SupportedEphemeralOSDiskPlacements" + // NvmeDiskSizeInMiB identifies the NVMe disk size in MiB. + NvmeDiskSizeInMiB = "NvmeDiskSizeInMiB" ) const ( @@ -62,6 +68,15 @@ const ( HyperVGeneration2 = "V2" ) +const ( + // DiskControllerSCSI identifies the SCSI disk controller type. + DiskControllerSCSI = "SCSI" + // DiskControllerNVMe identifies the NVMe disk controller type. + DiskControllerNVMe = "NVMe" + // EphemeralDiskPlacementNvme identifies NVMe disk placement for ephemeral OS disk. + EphemeralDiskPlacementNvme = "NvmeDisk" +) + const ( ten = 10 sixtyFour = 64 diff --git a/disk.go b/disk.go new file mode 100644 index 0000000..bd8c1ff --- /dev/null +++ b/disk.go @@ -0,0 +1,26 @@ +package skewer + +// HasSCSISupport determines if a SKU supports SCSI disk controller type. +// If no disk controller types are declared, it assumes SCSI is supported for backward compatibility. +func (s *SKU) HasSCSISupport() bool { + declaresSCSI := s.HasCapabilityWithSeparator(DiskControllerTypes, DiskControllerSCSI) + declaresNVMe := s.HasCapabilityWithSeparator(DiskControllerTypes, DiskControllerNVMe) + declaresNothing := !(declaresSCSI || declaresNVMe) + return declaresSCSI || declaresNothing +} + +// HasNVMeSupport determines if a SKU supports NVMe disk controller type. +func (s *SKU) HasNVMeSupport() bool { + return s.HasCapabilityWithSeparator(DiskControllerTypes, DiskControllerNVMe) +} + +// SupportsNVMeEphemeralOSDisk determines if a SKU supports NVMe placement for ephemeral OS disk. +func (s *SKU) SupportsNVMeEphemeralOSDisk() bool { + return s.HasCapabilityWithSeparator(SupportedEphemeralOSDiskPlacements, EphemeralDiskPlacementNvme) +} + +// NVMeDiskSizeInMiB returns the NVMe disk size in MiB for the SKU. +// Returns an error if the capability is not found, nil, or cannot be parsed. +func (s *SKU) NVMeDiskSizeInMiB() (int64, error) { + return s.GetCapabilityIntegerQuantity(NvmeDiskSizeInMiB) +} diff --git a/disk_test.go b/disk_test.go new file mode 100644 index 0000000..b9315a7 --- /dev/null +++ b/disk_test.go @@ -0,0 +1,282 @@ +package skewer + +import ( + "testing" + + "github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute" //nolint:staticcheck + "github.com/Azure/go-autorest/autorest/to" +) + +func Test_SKU_HasSCSISupport(t *testing.T) { + cases := map[string]struct { + sku compute.ResourceSku + expect bool + }{ + "empty capability list should return true (backward compatibility)": { + sku: compute.ResourceSku{}, + expect: true, + }, + "no disk controller capability should return true": { + sku: compute.ResourceSku{ + Capabilities: &[]compute.ResourceSkuCapabilities{ + { + Name: to.StringPtr("vCPUs"), + Value: to.StringPtr("8"), + }, + }, + }, + expect: true, + }, + "SCSI only should return true": { + sku: compute.ResourceSku{ + Capabilities: &[]compute.ResourceSkuCapabilities{ + { + Name: to.StringPtr(DiskControllerTypes), + Value: to.StringPtr("SCSI"), + }, + }, + }, + expect: true, + }, + "SCSI and NVMe should return true": { + sku: compute.ResourceSku{ + Capabilities: &[]compute.ResourceSkuCapabilities{ + { + Name: to.StringPtr(DiskControllerTypes), + Value: to.StringPtr("SCSI,NVMe"), + }, + }, + }, + expect: true, + }, + "NVMe only should return false": { + sku: compute.ResourceSku{ + Capabilities: &[]compute.ResourceSkuCapabilities{ + { + Name: to.StringPtr(DiskControllerTypes), + Value: to.StringPtr("NVMe"), + }, + }, + }, + expect: false, + }, + } + + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + sku := SKU(tc.sku) + actual := sku.HasSCSISupport() + if actual != tc.expect { + t.Fatalf("expected %v but got %v", tc.expect, actual) + } + }) + } +} + +func Test_SKU_HasNVMeSupport(t *testing.T) { + cases := map[string]struct { + sku compute.ResourceSku + expect bool + }{ + "empty capability list should return false": { + sku: compute.ResourceSku{}, + expect: false, + }, + "no disk controller capability should return false": { + sku: compute.ResourceSku{ + Capabilities: &[]compute.ResourceSkuCapabilities{ + { + Name: to.StringPtr("vCPUs"), + Value: to.StringPtr("8"), + }, + }, + }, + expect: false, + }, + "SCSI only should return false": { + sku: compute.ResourceSku{ + Capabilities: &[]compute.ResourceSkuCapabilities{ + { + Name: to.StringPtr(DiskControllerTypes), + Value: to.StringPtr("SCSI"), + }, + }, + }, + expect: false, + }, + "SCSI and NVMe should return true": { + sku: compute.ResourceSku{ + Capabilities: &[]compute.ResourceSkuCapabilities{ + { + Name: to.StringPtr(DiskControllerTypes), + Value: to.StringPtr("SCSI,NVMe"), + }, + }, + }, + expect: true, + }, + "NVMe only should return true": { + sku: compute.ResourceSku{ + Capabilities: &[]compute.ResourceSkuCapabilities{ + { + Name: to.StringPtr(DiskControllerTypes), + Value: to.StringPtr("NVMe"), + }, + }, + }, + expect: true, + }, + "NVMe in mixed case should return true": { + sku: compute.ResourceSku{ + Capabilities: &[]compute.ResourceSkuCapabilities{ + { + Name: to.StringPtr(DiskControllerTypes), + Value: to.StringPtr("SCSI,NVMe,Other"), + }, + }, + }, + expect: true, + }, + } + + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + sku := SKU(tc.sku) + actual := sku.HasNVMeSupport() + if actual != tc.expect { + t.Fatalf("expected %v but got %v", tc.expect, actual) + } + }) + } +} + +func Test_SKU_SupportsNVMeEphemeralOSDisk(t *testing.T) { + cases := map[string]struct { + sku compute.ResourceSku + expect bool + }{ + "empty capability list should return false": { + sku: compute.ResourceSku{}, + expect: false, + }, + "no ephemeral placement capability should return false": { + sku: compute.ResourceSku{ + Capabilities: &[]compute.ResourceSkuCapabilities{ + { + Name: to.StringPtr("vCPUs"), + Value: to.StringPtr("8"), + }, + }, + }, + expect: false, + }, + "ResourceDisk only should return false": { + sku: compute.ResourceSku{ + Capabilities: &[]compute.ResourceSkuCapabilities{ + { + Name: to.StringPtr(SupportedEphemeralOSDiskPlacements), + Value: to.StringPtr("ResourceDisk"), + }, + }, + }, + expect: false, + }, + "NvmeDisk should return true": { + sku: compute.ResourceSku{ + Capabilities: &[]compute.ResourceSkuCapabilities{ + { + Name: to.StringPtr(SupportedEphemeralOSDiskPlacements), + Value: to.StringPtr("NvmeDisk"), + }, + }, + }, + expect: true, + }, + "ResourceDisk and NvmeDisk should return true": { + sku: compute.ResourceSku{ + Capabilities: &[]compute.ResourceSkuCapabilities{ + { + Name: to.StringPtr(SupportedEphemeralOSDiskPlacements), + Value: to.StringPtr("ResourceDisk,NvmeDisk"), + }, + }, + }, + expect: true, + }, + } + + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + sku := SKU(tc.sku) + actual := sku.SupportsNVMeEphemeralOSDisk() + if actual != tc.expect { + t.Fatalf("expected %v but got %v", tc.expect, actual) + } + }) + } +} + +func Test_SKU_NVMeDiskSizeInMiB(t *testing.T) { + cases := map[string]struct { + sku compute.ResourceSku + expect int64 + err string + }{ + "empty capability list should return error": { + sku: compute.ResourceSku{}, + err: (&ErrCapabilityNotFound{NvmeDiskSizeInMiB}).Error(), + }, + "no NVMe disk size capability should return error": { + sku: compute.ResourceSku{ + Capabilities: &[]compute.ResourceSkuCapabilities{ + { + Name: to.StringPtr("vCPUs"), + Value: to.StringPtr("8"), + }, + }, + }, + err: (&ErrCapabilityNotFound{NvmeDiskSizeInMiB}).Error(), + }, + "valid NVMe disk size should return value": { + sku: compute.ResourceSku{ + Capabilities: &[]compute.ResourceSkuCapabilities{ + { + Name: to.StringPtr(NvmeDiskSizeInMiB), + Value: to.StringPtr("1024000"), + }, + }, + }, + expect: 1024000, + }, + "invalid NVMe disk size should return parse error": { + sku: compute.ResourceSku{ + Capabilities: &[]compute.ResourceSkuCapabilities{ + { + Name: to.StringPtr(NvmeDiskSizeInMiB), + Value: to.StringPtr("not-a-number"), + }, + }, + }, + err: "NvmeDiskSizeInMiBCapabilityValueParse: failed to parse string 'not-a-number' as int64, error: 'strconv.ParseInt: parsing \"not-a-number\": invalid syntax'", + }, + } + + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + sku := SKU(tc.sku) + actual, err := sku.NVMeDiskSizeInMiB() + if tc.err != "" { + if err == nil || err.Error() != tc.err { + t.Fatalf("expected error '%s' but got '%v'", tc.err, err) + } + } else { + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if actual != tc.expect { + t.Fatalf("expected %d but got %d", tc.expect, actual) + } + } + }) + } +} From 119a1b6325ee38b610c537b239c5187ba5e3b053 Mon Sep 17 00:00:00 2001 From: Bryce Soghigian Date: Thu, 17 Jul 2025 21:12:24 -0700 Subject: [PATCH 2/2] refactor: replace declaresNVMe with HasNVMeSupport() and simplify tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Replace local variable declaresNVMe with call to HasNVMeSupport() method - Update test cases to use empty capabilities list when testing defaulting behavior 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude --- disk.go | 3 +-- disk_test.go | 14 ++------------ 2 files changed, 3 insertions(+), 14 deletions(-) diff --git a/disk.go b/disk.go index bd8c1ff..5fffbcd 100644 --- a/disk.go +++ b/disk.go @@ -4,8 +4,7 @@ package skewer // If no disk controller types are declared, it assumes SCSI is supported for backward compatibility. func (s *SKU) HasSCSISupport() bool { declaresSCSI := s.HasCapabilityWithSeparator(DiskControllerTypes, DiskControllerSCSI) - declaresNVMe := s.HasCapabilityWithSeparator(DiskControllerTypes, DiskControllerNVMe) - declaresNothing := !(declaresSCSI || declaresNVMe) + declaresNothing := !(declaresSCSI || s.HasNVMeSupport()) return declaresSCSI || declaresNothing } diff --git a/disk_test.go b/disk_test.go index b9315a7..ef45aa9 100644 --- a/disk_test.go +++ b/disk_test.go @@ -18,12 +18,7 @@ func Test_SKU_HasSCSISupport(t *testing.T) { }, "no disk controller capability should return true": { sku: compute.ResourceSku{ - Capabilities: &[]compute.ResourceSkuCapabilities{ - { - Name: to.StringPtr("vCPUs"), - Value: to.StringPtr("8"), - }, - }, + Capabilities: &[]compute.ResourceSkuCapabilities{}, }, expect: true, }, @@ -84,12 +79,7 @@ func Test_SKU_HasNVMeSupport(t *testing.T) { }, "no disk controller capability should return false": { sku: compute.ResourceSku{ - Capabilities: &[]compute.ResourceSkuCapabilities{ - { - Name: to.StringPtr("vCPUs"), - Value: to.StringPtr("8"), - }, - }, + Capabilities: &[]compute.ResourceSkuCapabilities{}, }, expect: false, },