fix(azure): filter L-series NVMe VMs from RHEL AI candidates

rishupk · claude · rishupk · commit f88e03f7f310 · 2026-06-04T13:20:36.000+01:00
- Add NvmeDiskSizeInMiB and DiskControllerTypes fields to virtualMachine
  struct and parse them in resourceSKUToVirtualMachine(); previously both
  capabilities fell through to default and were discarded
- Rename emptyDiskSupported() to noLocalStorageAttached() and tighten
  condition to MaxResourceVolumeMB == 0 &amp;&amp; NvmeDiskSizeInMiB == 0; fixes
  L-series VMs (e.g. Standard_L8aos_v4) incorrectly passing the filter
- Add FilterNoLocalStorageSizes() to validate explicit --compute-sizes in
  the RHEL AI path; sizes with local storage or absent from the SKU catalog
  are warned and dropped before allocation proceeds
- Add 12 unit tests covering noLocalStorageAttached(), SKU capability
  parsing, filterLocalStorage() (including unknown-size path), and a
  baseFeaturesSupported() regression for L-series

Co-Authored-By: Claude Sonnet 4.6 &lt;noreply@anthropic.com&gt;
Signed-off-by: Rishabh Kothari &lt;rkothari@redhat.com&gt;
diff --git a/.claude/settings.local.json b/.claude/settings.local.json
@@ -0,0 +1,10 @@
+{
+  "permissions": {
+    "allow": [
+      "WebFetch(domain:martinfowler.com)",
+      "Bash(gh pr *)",
+      "Bash(python3 -c \"import json,sys; d=json.load\\(sys.stdin\\); print\\(json.dumps\\(d.get\\('skills',d.get\\('skill_definitions',[]\\)\\), indent=2\\)\\)\")",
+      "Read(//Users/rish/Developer/claudio-skills/claudio-plugin/**)"
+    ]
+  }
+}
diff --git a/pkg/provider/azure/action/rhel-ai/rhelai.go b/pkg/provider/azure/action/rhel-ai/rhelai.go
@@ -1,6 +1,7 @@
 package rhelai
 
 import (
+	"context"
 	"fmt"
 	"strings"
 
@@ -36,6 +37,20 @@ func imageId(accelerator, version string) string {
 
 func Create(mCtxArgs *maptContext.ContextArgs, args *apiRHELAI.RHELAIArgs) (err error) {
 	logging.Debug("Creating RHEL Server")
+	if len(args.ComputeRequest.ComputeSizes) > 0 {
+		ctx := mCtxArgs.Context
+		if ctx == nil {
+			ctx = context.Background()
+		}
+		args.ComputeRequest.ComputeSizes, err = data.FilterNoLocalStorageSizes(
+			ctx, args.ComputeRequest.ComputeSizes)
+		if err != nil {
+			return err
+		}
+		if len(args.ComputeRequest.ComputeSizes) == 0 {
+			return fmt.Errorf("no valid compute sizes: all provided sizes have local NVMe/temp-disk storage, incompatible with RHEL AI")
+		}
+	}
 	sharedImageID := imageId(args.Accelerator, args.Version)
 	if args.CustomImage != "" {
 		sharedImageID = imageIdFromName(args.CustomImage)
diff --git a/pkg/provider/azure/data/compute-request.go b/pkg/provider/azure/data/compute-request.go
@@ -12,6 +12,7 @@ import (
 	"github.com/Azure/azure-sdk-for-go/sdk/azidentity"
 	"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v7"
 	cr "github.com/redhat-developer/mapt/pkg/provider/api/compute-request"
+	"github.com/redhat-developer/mapt/pkg/util/logging"
 )
 
 const (
@@ -66,6 +67,69 @@ func FilterComputeSizesByLocation(ctx context.Context, location *string, compute
 	return supportedSizes, nil
 }
 
+// FilterNoLocalStorageSizes returns only the sizes from computeSizes that have no local
+// storage (no temp disk, no NVMe). Sizes with local storage are logged as warnings and
+// dropped. Sizes not found in the Azure SKU catalog (typo or restricted SKU) are logged
+// as warnings and excluded.
+func FilterNoLocalStorageSizes(ctx context.Context, computeSizes []string) ([]string, error) {
+	creds, subscriptionID, err := getCredentials()
+	if err != nil {
+		return nil, err
+	}
+	client, err := armcompute.NewResourceSKUsClient(*subscriptionID, creds, nil)
+	if err != nil {
+		return nil, err
+	}
+	pager := client.NewListPager(nil)
+	capabilities := make(map[string]*virtualMachine, len(computeSizes))
+	for pager.More() {
+		page, err := pager.NextPage(ctx)
+		if err != nil {
+			return nil, err
+		}
+		for _, sku := range page.Value {
+			if sku.ResourceType == nil || *sku.ResourceType != string(RTVirtualMachines) {
+				continue
+			}
+			if sku.Name == nil || !slices.Contains(computeSizes, *sku.Name) {
+				continue
+			}
+			if _, seen := capabilities[*sku.Name]; seen {
+				continue
+			}
+			if vm := resourceSKUToVirtualMachine(sku); vm != nil {
+				capabilities[*sku.Name] = vm
+			}
+		}
+	}
+	valid, dropped, unknown := filterLocalStorage(computeSizes, capabilities)
+	for _, size := range dropped {
+		logging.Warnf("dropping compute size %q: has local NVMe/temp-disk storage, incompatible with RHEL AI", size)
+	}
+	for _, size := range unknown {
+		logging.Warnf("dropping compute size %q: not found in Azure SKU catalog (typo or restricted SKU)", size)
+	}
+	return valid, nil
+}
+
+// filterLocalStorage classifies each size into valid (no local storage), dropped (has
+// local storage), or unknown (absent from capabilities — typo or restricted SKU).
+func filterLocalStorage(computeSizes []string, capabilities map[string]*virtualMachine) (valid, dropped, unknown []string) {
+	for _, size := range computeSizes {
+		vm, ok := capabilities[size]
+		if !ok {
+			unknown = append(unknown, size)
+			continue
+		}
+		if vm.noLocalStorageAttached() {
+			valid = append(valid, size)
+		} else {
+			dropped = append(dropped, size)
+		}
+	}
+	return valid, dropped, unknown
+}
+
 func getAzureVMSKUs(ctx context.Context, args *cr.ComputeRequestArgs) ([]string, error) {
 	cred, err := azidentity.NewDefaultAzureCredential(nil)
 	if err != nil {
@@ -109,6 +173,10 @@ type virtualMachine struct {
 	// Spot capable
 	LowPriorityCapable  bool
 	MaxResourceVolumeMB int32
+	// L-series VMs expose NVMe storage separately from the temp disk
+	NvmeDiskSizeInMiB int32
+	// Parsed for Bug 2 fix (NVMe controller filtering); not used in current filtering logic
+	DiskControllerTypes []string
 	// IaaS or PaaS
 	VMDeploymentTypes []string
 	// Fast SSD
@@ -144,17 +212,17 @@ func (vm *virtualMachine) hypervGen2Supported() bool {
 	return slices.Contains(vm.HyperVGenerations, "V2")
 }
 
-func (vm *virtualMachine) emptyDiskSupported() bool {
-	return vm.MaxResourceVolumeMB == 0
+func (vm *virtualMachine) noLocalStorageAttached() bool {
+	return vm.MaxResourceVolumeMB == 0 && vm.NvmeDiskSizeInMiB == 0
 }
 
 func (vm *virtualMachine) baseFeaturesSupported() bool {
 	return vm.AcceleratedNetworkingEnabled && vm.PremiumIO && vm.EncryptionAtHostSupported &&
-		vm.emptyDiskSupported() && vm.hypervGen2Supported()
+		vm.noLocalStorageAttached() && vm.hypervGen2Supported()
 }
 
 func resourceSKUToVirtualMachine(res *armcompute.ResourceSKU) *virtualMachine {
-	if res.ResourceType != nil && *res.ResourceType != "virtualMachines" {
+	if res.ResourceType != nil && *res.ResourceType != string(RTVirtualMachines) {
 		return nil
 	}
 	// If Machine type has any type of restriccions discard
@@ -219,6 +287,14 @@ func resourceSKUToVirtualMachine(res *armcompute.ResourceSKU) *virtualMachine {
 				return nil
 			}
 			vm.MaxResourceVolumeMB = int32(disk)
+		case "NvmeDiskSizeInMiB":
+			nvme, err := strconv.ParseUint(*capability.Value, 10, 64)
+			if err != nil {
+				return nil
+			}
+			vm.NvmeDiskSizeInMiB = int32(nvme)
+		case "DiskControllerTypes":
+			vm.DiskControllerTypes = strings.Split(*capability.Value, ",")
 		case "VMDeploymentTypes":
 			vm.VMDeploymentTypes = strings.Split(*capability.Value, ",")
 		default:
diff --git a/pkg/provider/azure/data/compute-request_test.go b/pkg/provider/azure/data/compute-request_test.go
@@ -0,0 +1,185 @@
+package data
+
+import (
+	"testing"
+
+	"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v7"
+)
+
+func ptr[T any](v T) *T { return &v }
+
+// noLocalStorageAttached tests
+
+func TestNoLocalStorageAttached_NoTempDiskNoNvme(t *testing.T) {
+	vm := &virtualMachine{MaxResourceVolumeMB: 0, NvmeDiskSizeInMiB: 0}
+	if !vm.noLocalStorageAttached() {
+		t.Error("expected true: VM with no temp disk and no NVMe should have no local storage")
+	}
+}
+
+func TestNoLocalStorageAttached_HasTempDisk(t *testing.T) {
+	vm := &virtualMachine{MaxResourceVolumeMB: 512, NvmeDiskSizeInMiB: 0}
+	if vm.noLocalStorageAttached() {
+		t.Error("expected false: VM with temp disk should have local storage")
+	}
+}
+
+func TestNoLocalStorageAttached_HasNvmeDisk(t *testing.T) {
+	// L-series bug case: MaxResourceVolumeMB=0 but NvmeDiskSizeInMiB>0
+	vm := &virtualMachine{MaxResourceVolumeMB: 0, NvmeDiskSizeInMiB: 5492736}
+	if vm.noLocalStorageAttached() {
+		t.Error("expected false: L-series VM with NVMe storage should have local storage")
+	}
+}
+
+func TestNoLocalStorageAttached_HasBoth(t *testing.T) {
+	vm := &virtualMachine{MaxResourceVolumeMB: 512, NvmeDiskSizeInMiB: 5492736}
+	if vm.noLocalStorageAttached() {
+		t.Error("expected false: VM with both temp disk and NVMe should have local storage")
+	}
+}
+
+// resourceSKUToVirtualMachine parsing tests
+
+func TestResourceSKUToVirtualMachine_ParsesNvmeDiskSizeInMiB(t *testing.T) {
+	sku := &armcompute.ResourceSKU{
+		ResourceType: ptr("virtualMachines"),
+		Name:         ptr("Standard_L8aos_v4"),
+		Family:       ptr("standardLasv4Family"),
+		Capabilities: []*armcompute.ResourceSKUCapabilities{
+			{Name: ptr("NvmeDiskSizeInMiB"), Value: ptr("5492736")},
+		},
+	}
+	vm := resourceSKUToVirtualMachine(sku)
+	if vm == nil {
+		t.Fatal("expected non-nil virtualMachine")
+	}
+	if vm.NvmeDiskSizeInMiB != 5492736 {
+		t.Errorf("NvmeDiskSizeInMiB: got %d, want 5492736", vm.NvmeDiskSizeInMiB)
+	}
+}
+
+func TestResourceSKUToVirtualMachine_ParsesDiskControllerTypes(t *testing.T) {
+	sku := &armcompute.ResourceSKU{
+		ResourceType: ptr("virtualMachines"),
+		Name:         ptr("Standard_L8aos_v4"),
+		Family:       ptr("standardLasv4Family"),
+		Capabilities: []*armcompute.ResourceSKUCapabilities{
+			{Name: ptr("DiskControllerTypes"), Value: ptr("NVMe,SCSI")},
+		},
+	}
+	vm := resourceSKUToVirtualMachine(sku)
+	if vm == nil {
+		t.Fatal("expected non-nil virtualMachine")
+	}
+	if len(vm.DiskControllerTypes) != 2 {
+		t.Fatalf("DiskControllerTypes: got %v, want [NVMe SCSI]", vm.DiskControllerTypes)
+	}
+	if vm.DiskControllerTypes[0] != "NVMe" || vm.DiskControllerTypes[1] != "SCSI" {
+		t.Errorf("DiskControllerTypes: got %v, want [NVMe SCSI]", vm.DiskControllerTypes)
+	}
+}
+
+func TestResourceSKUToVirtualMachine_NvmeDiskSizeDefaultsToZero(t *testing.T) {
+	sku := &armcompute.ResourceSKU{
+		ResourceType: ptr("virtualMachines"),
+		Name:         ptr("Standard_D8as_v5"),
+		Family:       ptr("standardDasv5Family"),
+		Capabilities: []*armcompute.ResourceSKUCapabilities{
+			{Name: ptr("MaxResourceVolumeMB"), Value: ptr("307200")},
+		},
+	}
+	vm := resourceSKUToVirtualMachine(sku)
+	if vm == nil {
+		t.Fatal("expected non-nil virtualMachine")
+	}
+	if vm.NvmeDiskSizeInMiB != 0 {
+		t.Errorf("NvmeDiskSizeInMiB: got %d, want 0 for non-NVMe SKU", vm.NvmeDiskSizeInMiB)
+	}
+}
+
+// filterLocalStorage tests
+
+func TestFilterLocalStorage_DropsNvmeSizes(t *testing.T) {
+	capabilities := map[string]*virtualMachine{
+		"Standard_D8as_v5":  {MaxResourceVolumeMB: 0, NvmeDiskSizeInMiB: 0},
+		"Standard_L8aos_v4": {MaxResourceVolumeMB: 0, NvmeDiskSizeInMiB: 5492736},
+	}
+	got, dropped, unknown := filterLocalStorage([]string{"Standard_D8as_v5", "Standard_L8aos_v4"}, capabilities)
+	if len(got) != 1 || got[0] != "Standard_D8as_v5" {
+		t.Errorf("filtered: got %v, want [Standard_D8as_v5]", got)
+	}
+	if len(dropped) != 1 || dropped[0] != "Standard_L8aos_v4" {
+		t.Errorf("dropped: got %v, want [Standard_L8aos_v4]", dropped)
+	}
+	if len(unknown) != 0 {
+		t.Errorf("unknown: got %v, want []", unknown)
+	}
+}
+
+func TestFilterLocalStorage_DropsTempDiskSizes(t *testing.T) {
+	capabilities := map[string]*virtualMachine{
+		"Standard_D4s_v3": {MaxResourceVolumeMB: 32768, NvmeDiskSizeInMiB: 0},
+	}
+	got, dropped, unknown := filterLocalStorage([]string{"Standard_D4s_v3"}, capabilities)
+	if len(got) != 0 {
+		t.Errorf("filtered: got %v, want []", got)
+	}
+	if len(dropped) != 1 {
+		t.Errorf("dropped: got %v, want [Standard_D4s_v3]", dropped)
+	}
+	if len(unknown) != 0 {
+		t.Errorf("unknown: got %v, want []", unknown)
+	}
+}
+
+func TestFilterLocalStorage_PassesCleanSizes(t *testing.T) {
+	capabilities := map[string]*virtualMachine{
+		"Standard_D8as_v5": {MaxResourceVolumeMB: 0, NvmeDiskSizeInMiB: 0},
+	}
+	got, dropped, unknown := filterLocalStorage([]string{"Standard_D8as_v5"}, capabilities)
+	if len(got) != 1 {
+		t.Errorf("filtered: got %v, want [Standard_D8as_v5]", got)
+	}
+	if len(dropped) != 0 {
+		t.Errorf("dropped: got %v, want []", dropped)
+	}
+	if len(unknown) != 0 {
+		t.Errorf("unknown: got %v, want []", unknown)
+	}
+}
+
+func TestFilterLocalStorage_ReportsUnknownSizes(t *testing.T) {
+	// Sizes absent from the catalog (typo, restricted SKU) must appear in unknown,
+	// not silently disappear.
+	capabilities := map[string]*virtualMachine{
+		"Standard_D8as_v5": {MaxResourceVolumeMB: 0, NvmeDiskSizeInMiB: 0},
+	}
+	got, dropped, unknown := filterLocalStorage(
+		[]string{"Standard_D8as_v5", "Standard_Typo_v99"}, capabilities)
+	if len(got) != 1 || got[0] != "Standard_D8as_v5" {
+		t.Errorf("filtered: got %v, want [Standard_D8as_v5]", got)
+	}
+	if len(dropped) != 0 {
+		t.Errorf("dropped: got %v, want []", dropped)
+	}
+	if len(unknown) != 1 || unknown[0] != "Standard_Typo_v99" {
+		t.Errorf("unknown: got %v, want [Standard_Typo_v99]", unknown)
+	}
+}
+
+// baseFeaturesSupported regression: L-series must be rejected
+
+func TestBaseFeaturesSupported_LSeriesWithNvmeIsRejected(t *testing.T) {
+	vm := &virtualMachine{
+		MaxResourceVolumeMB:          0,
+		NvmeDiskSizeInMiB:            5492736,
+		AcceleratedNetworkingEnabled: true,
+		PremiumIO:                    true,
+		EncryptionAtHostSupported:    true,
+		HyperVGenerations:            []string{"V1", "V2"},
+	}
+	if vm.baseFeaturesSupported() {
+		t.Error("expected false: L-series VM with NVMe storage must not pass baseFeaturesSupported")
+	}
+}
