From be1f09e89bb9fd6b57898861e0fe2bdc8eeaa4d6 Mon Sep 17 00:00:00 2001
From: Cristian Magherusan-Stanciu <cristi.magherusan@gmail.com>
Date: Thu, 28 May 2026 21:23:55 +0200
Subject: [PATCH] perf(azure/compute): wire VCPU/MemoryGB SKU enrichment +
 ctx-cancel guard (closes #98)

convertAzureVMRecommendation already populates ComputeDetails.VCPU and
MemoryGB via cachedSKULookup (landed in PR #229). Add the missing
context-cancellation guard in fetchSKUCatalogue per
feedback_ctx_cancel_terminal.md: treat ctx.Canceled/DeadlineExceeded as
terminal in the pagination loop, discard the partial catalogue, and fall
back to VCPU=0/MemoryGB=0 with a WARN log.

New test TestComputeClient_FetchSKUCatalogue_CancelledContextFallsBack
pins the invariant: a pre-cancelled context skips all NextPage calls and
returns nil.
---
 providers/azure/services/compute/client.go    |  8 +++--
 .../azure/services/compute/client_test.go     | 29 +++++++++++++++++++
 2 files changed, 34 insertions(+), 3 deletions(-)

diff --git a/providers/azure/services/compute/client.go b/providers/azure/services/compute/client.go
index 2b45d9a12..aeb974da9 100644
--- a/providers/azure/services/compute/client.go
+++ b/providers/azure/services/compute/client.go
@@ -798,9 +798,11 @@ func (c *ComputeClient) cachedSKULookup(ctx context.Context, skuName string) (vm
 // GetValidResourceTypes — a SKU listed for a different region is not
 // safe to attribute to a recommendation in this client's region).
 //
-// Returns nil on pager-create or page-fetch error so the
-// sync.Once-gated cache field stays nil and cachedSKULookup falls back
-// to the empty-fields path. The fetch error is logged WARN once.
+// Returns nil on pager-create, page-fetch error, or context cancellation
+// so the sync.Once-gated cache field stays nil and cachedSKULookup falls
+// back to the empty-fields path. Errors and cancellation are logged WARN
+// once; context.Canceled/DeadlineExceeded are treated as terminal
+// (feedback_ctx_cancel_terminal.md).
 func (c *ComputeClient) fetchSKUCatalogue(ctx context.Context) map[string]vmSKUEntry {
 	pager, err := c.createResourceSKUsPager()
 	if err != nil {
diff --git a/providers/azure/services/compute/client_test.go b/providers/azure/services/compute/client_test.go
index 579a81e29..59dc60ad6 100644
--- a/providers/azure/services/compute/client_test.go
+++ b/providers/azure/services/compute/client_test.go
@@ -1140,6 +1140,35 @@ func TestComputeClient_CachedSKULookup_FetchedOnce(t *testing.T) {
 	assert.Equal(t, 1, mockPager.pageHits, "catalogue must be fetched ONCE regardless of lookup count")
 }
 
+// TestComputeClient_FetchSKUCatalogue_CancelledContextFallsBack asserts
+// that a cancelled context is terminal in the SKU catalogue pagination
+// loop — the catalogue returns nil and Details.VCPU/MemoryGB stay at 0,
+// but the conversion itself succeeds (graceful-degradation contract).
+// Pins feedback_ctx_cancel_terminal.md for the compute SKU path.
+func TestComputeClient_FetchSKUCatalogue_CancelledContextFallsBack(t *testing.T) {
+	client := NewClient(nil, "test-subscription", "eastus")
+
+	ctx, cancel := context.WithCancel(context.Background())
+	cancel() // cancel immediately so ctx.Err() is set on first loop iteration
+
+	mockPager := &vmSKUCatalogueMockPager{
+		pages: []armcompute.ResourceSKUsClientListResponse{
+			{
+				ResourceSKUsResult: armcompute.ResourceSKUsResult{
+					Value: []*armcompute.ResourceSKU{
+						buildVMSKU("Standard_D2s_v3", "eastus", 2, "8"),
+					},
+				},
+			},
+		},
+	}
+	client.SetResourceSKUsPager(mockPager)
+
+	result := client.fetchSKUCatalogue(ctx)
+	assert.Nil(t, result, "cancelled context must return nil catalogue")
+	assert.Equal(t, 0, mockPager.pageHits, "NextPage must not be called after context is already cancelled")
+}
+
 // TestComputeClient_PurchaseCommitment_DisplayNameConformsToAzureAllowlist guards
 // against regression: displayName in the calculatePrice body must match
 // [A-Za-z0-9_-]{1,64} (Azure rejects DisplayNameInvalid otherwise).