fix(azure): make managedredis the only Azure Redis recommendation producer

CHANGELOG.md

@@ -177,6 +177,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/).
 
 ### Changed
 
+- **BREAKING `azure`:** Azure Redis recommendations now come only from managedredis and are labelled `memorydb` (`ServiceMemoryDB`), both from Consumption and from Advisor rows. The cache client's `GetRecommendations` returns none, so `GetRecommendationsForService(ServiceCache)` is empty for Azure, and consumers that stored Azure Redis plans or settings under `cache` must also match `memorydb`. Azure Redis recommendations keep `CacheDetails` `Engine` and `NodeType` but no longer report `Shards` (0, unknown) (#692).
 - `ListTargetOfferings` now walks up to 50 pages (about 5000 offerings)
   instead of 10, and returns an error wrapping the new
   `ec2.ErrTargetOfferingsTruncated` (with no list) when that bound is hit,
@@ -498,6 +499,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/).
 
 ### Removed
 
+- **BREAKING:** `providers/azure/services/cache` no longer exports `RecommendationsPager`, `(*Client).SetRecommendationsPager` or `(*Client).ReservationUnitPrice`. The cache client no longer collects or prices recommendations; managedredis does. No cli, mcp or platform code references them. (#692)
 - **BREAKING:** `common.Matches` (`pkg/common`) is removed. It compared only
   Provider, Region, Service, ResourceType and engine and had no callers in
   this repo, the CLI, the MCP server or the platform; use `pkg/recfilter`

providers/azure/recommendations.go

@@ -19,23 +19,23 @@ import (
 	"github.com/LeanerCloud/cloud-commitments-go/pkg/logging"
 	azrecs "github.com/LeanerCloud/cloud-commitments-go/providers/azure/internal/recommendations"
 	"github.com/LeanerCloud/cloud-commitments-go/providers/azure/internal/slot"
-	"github.com/LeanerCloud/cloud-commitments-go/providers/azure/services/cache"
 	"github.com/LeanerCloud/cloud-commitments-go/providers/azure/services/compute"
 	"github.com/LeanerCloud/cloud-commitments-go/providers/azure/services/cosmosdb"
 	"github.com/LeanerCloud/cloud-commitments-go/providers/azure/services/database"
+	"github.com/LeanerCloud/cloud-commitments-go/providers/azure/services/managedredis"
 	"github.com/LeanerCloud/cloud-commitments-go/providers/azure/services/savingsplans"
 	"github.com/LeanerCloud/cloud-commitments-go/providers/azure/services/synapse"
 )
 
 // serviceRecsGetter is the narrow interface satisfied by each per-service
-// client (compute, database, cache, cosmosdb). The interface exists solely to
+// client (compute, database, managedredis, cosmosdb). The interface exists solely to
 // allow tests to substitute fake implementations; production code uses the
 // concrete types via the newXxxClientFn variables below.
 type serviceRecsGetter interface {
 	GetRecommendations(ctx context.Context, params *common.RecommendationParams) ([]common.Recommendation, error)
 }
 
-// newComputeClientFn, newDatabaseClientFn, newCacheClientFn,
+// newComputeClientFn, newDatabaseClientFn, newManagedRedisClientFn,
 // newCosmosDBClientFn, and newSavingsPlansClientFn default to the real
 // constructors and are overridden in tests to inject fakes. The variables are
 // package-level (not fields on the adapter) so that the constructor signature
@@ -53,8 +53,8 @@ var (
 	newDatabaseClientFn func(azcore.TokenCredential, string, string) serviceRecsGetter = func(cred azcore.TokenCredential, sub, region string) serviceRecsGetter {
 		return database.NewClient(cred, sub, region)
 	}
-	newCacheClientFn func(azcore.TokenCredential, string, string) serviceRecsGetter = func(cred azcore.TokenCredential, sub, region string) serviceRecsGetter {
-		return cache.NewClient(cred, sub, region)
+	newManagedRedisClientFn func(azcore.TokenCredential, string, string) serviceRecsGetter = func(cred azcore.TokenCredential, sub, region string) serviceRecsGetter {
+		return managedredis.NewClient(cred, sub, region)
 	}
 	newCosmosDBClientFn func(azcore.TokenCredential, string, string) serviceRecsGetter = func(cred azcore.TokenCredential, sub, region string) serviceRecsGetter {
 		return cosmosdb.NewClient(cred, sub, region)
@@ -113,14 +113,14 @@ func NewRecommendationsClientAdapter(cred azcore.TokenCredential, subscriptionID
 // All seven service calls run concurrently under errgroup. Each goroutine captures
 // its own error and returns nil to the group so that a single service failure
 // does not cancel sibling calls. Results are appended in a deterministic order
-// (compute → database → cache → cosmosdb → synapse → savingsplans → advisor) after all goroutines finish.
+// (compute → database → redis → cosmosdb → synapse → savingsplans → advisor) after all goroutines finish.
 func (r *RecommendationsClientAdapter) GetRecommendations(ctx context.Context, params *common.RecommendationParams) ([]common.Recommendation, error) {
 	if params == nil {
 		return nil, fmt.Errorf("params cannot be nil")
 	}
 	var (
-		computeRecs, dbRecs, cacheRecs, cosmosRecs, advisorRecs, spRecs, synapseRecs []common.Recommendation
-		computeErr, dbErr, cacheErr, cosmosErr, advisorErr, spErr, synapseErr        error
+		computeRecs, dbRecs, redisRecs, cosmosRecs, advisorRecs, spRecs, synapseRecs []common.Recommendation
+		computeErr, dbErr, redisErr, cosmosErr, advisorErr, spErr, synapseErr        error
 	)
 
 	ctx, skips := azrecs.WithSkipLog(ctx)
@@ -144,7 +144,7 @@ func (r *RecommendationsClientAdapter) GetRecommendations(ctx context.Context, p
 	// recommendations" when applying the all-attempted-failed guard.
 	includeCompute := shouldIncludeService(*params, common.ServiceCompute)
 	includeDB := shouldIncludeService(*params, common.ServiceRelationalDB)
-	includeCache := shouldIncludeService(*params, common.ServiceCache)
+	includeRedis := shouldIncludeService(*params, common.ServiceMemoryDB)
 	includeCosmos := shouldIncludeService(*params, common.ServiceNoSQL)
 	includeSP := shouldIncludeService(*params, common.ServiceSavingsPlansAll)
 	includeSynapse := shouldIncludeService(*params, common.ServiceDataWarehouse)
@@ -165,11 +165,11 @@ func (r *RecommendationsClientAdapter) GetRecommendations(ctx context.Context, p
 		})
 	}
 
-	// Cache (Redis) recommendations — subscription-wide.
-	if includeCache {
+	// Redis recommendations (labeled memorydb) — subscription-wide.
+	if includeRedis {
 		goService(func() {
-			cacheClient := newCacheClientFn(r.cred, r.subscriptionID, "")
-			cacheRecs, cacheErr = cacheClient.GetRecommendations(gctx, params)
+			redisClient := newManagedRedisClientFn(r.cred, r.subscriptionID, "")
+			redisRecs, redisErr = redisClient.GetRecommendations(gctx, params)
 		})
 	}
 
@@ -181,8 +181,8 @@ func (r *RecommendationsClientAdapter) GetRecommendations(ctx context.Context, p
 		})
 	}
 
-	// Synapse (SqlDataWarehouse) recommendations — subscription-wide. Search and
-	// Managed Redis are deliberately not collected here: see #47.
+	// Synapse (SqlDataWarehouse) recommendations — subscription-wide. Search is
+	// deliberately not collected here: see #47. Redis is collected above.
 	if includeSynapse {
 		goService(func() {
 			synapseRecs, synapseErr = newSynapseClientFn(r.cred, r.subscriptionID, "").GetRecommendations(gctx, params)
@@ -224,15 +224,15 @@ func (r *RecommendationsClientAdapter) GetRecommendations(ctx context.Context, p
 		return nil, err
 	}
 
-	consumptionRIs := slices.Concat(okRecs(computeRecs, computeErr), okRecs(dbRecs, dbErr), okRecs(cacheRecs, cacheErr))
+	consumptionRIs := slices.Concat(okRecs(computeRecs, computeErr), okRecs(dbRecs, dbErr), okRecs(redisRecs, redisErr))
 	// Priced with the parent ctx: gctx is canceled once Wait returns.
 	advisorRecs = r.priceAdvisor(ctx, dropCoveredAdvisorReservations(dropCoveredAdvisorSavingsPlans(advisorRecs, spRecs), consumptionRIs), advisorErr)
 	if err := ctx.Err(); err != nil {
 		return nil, err
 	}
 	merged, err := mergeServiceResults(serviceResult{err: computeErr, name: "compute", recs: computeRecs, attempted: includeCompute},
 		serviceResult{err: dbErr, name: "database", recs: dbRecs, attempted: includeDB},
-		serviceResult{err: cacheErr, name: "cache", recs: cacheRecs, attempted: includeCache},
+		serviceResult{err: redisErr, name: "managedredis", recs: redisRecs, attempted: includeRedis},
 		serviceResult{err: cosmosErr, name: "cosmosdb", recs: cosmosRecs, attempted: includeCosmos},
 		serviceResult{err: synapseErr, name: "synapse", recs: synapseRecs, attempted: includeSynapse},
 		// The savingsplans client is a stub that unconditionally returns
@@ -400,7 +400,7 @@ type serviceResult struct {
 // mergeServiceResults logs per-service errors (matches the previous sequential
 // behavior where each error was logged inline via logging.Warnf) and appends
 // successful results in the order the slice is passed — callers must preserve
-// the canonical compute → database → cache → cosmosdb → synapse → savingsplans → advisor
+// the canonical compute → database → redis → cosmosdb → synapse → savingsplans → advisor
 // order so that order-sensitive consumers remain stable. The advisor entry's
 // error is logged via logging.Errorf to match the pre-parallelisation severity.
 //
@@ -651,6 +651,9 @@ func (r *RecommendationsClientAdapter) convertAdvisorRecommendation(ctx context.
 		azrecs.RecordSkip(ctx, "advisor", fmt.Errorf("%s recommendation in %q: %w", service, rec.Region, err))
 		return nil
 	}
+	if rec.Service == common.ServiceMemoryDB && rec.Details == nil {
+		rec.Details = managedredis.Details(rec.ResourceType)
+	}
 	return rec
 }
 
@@ -753,7 +756,7 @@ func serviceFromExtendedProperties(ext map[string]*string) string {
 		case "sqldatabases":
 			return string(common.ServiceRelationalDB)
 		case "rediscache":
-			return string(common.ServiceCache)
+			return string(common.ServiceMemoryDB)
 		}
 	}
 	if subcat, ok := ext["recommendationSubCategory"]; ok && subcat != nil {

providers/azure/recommendations_advisor_pricing.go

@@ -8,12 +8,12 @@ import (
 	"github.com/LeanerCloud/cloud-commitments-go/pkg/common"
 	"github.com/LeanerCloud/cloud-commitments-go/pkg/logging"
 	azrecs "github.com/LeanerCloud/cloud-commitments-go/providers/azure/internal/recommendations"
-	"github.com/LeanerCloud/cloud-commitments-go/providers/azure/services/cache"
 	"github.com/LeanerCloud/cloud-commitments-go/providers/azure/services/compute"
 	"github.com/LeanerCloud/cloud-commitments-go/providers/azure/services/database"
+	"github.com/LeanerCloud/cloud-commitments-go/providers/azure/services/managedredis"
 )
 
-// reservationUnitPricer is the price lookup the compute, database and cache
+// reservationUnitPricer is the price lookup the compute, database and Redis
 // clients export for pricing Advisor rows.
 type reservationUnitPricer interface {
 	ReservationUnitPrice(ctx context.Context, sku, region string, termYears int) (azrecs.ReservationPrice, error)
@@ -22,7 +22,7 @@ type reservationUnitPricer interface {
 var (
 	_ reservationUnitPricer = (*compute.Client)(nil)
 	_ reservationUnitPricer = (*database.Client)(nil)
-	_ reservationUnitPricer = (*cache.Client)(nil)
+	_ reservationUnitPricer = (*managedredis.Client)(nil)
 )
 
 // priceAdvisor prices the Advisor reservation rows with the service clients'
@@ -36,7 +36,7 @@ func (r *RecommendationsClientAdapter) priceAdvisor(ctx context.Context, rows []
 	clients := map[common.ServiceType]serviceRecsGetter{
 		common.ServiceCompute:      newComputeClientFn(r.cred, r.subscriptionID, ""),
 		common.ServiceRelationalDB: newDatabaseClientFn(r.cred, r.subscriptionID, ""),
-		common.ServiceCache:        newCacheClientFn(r.cred, r.subscriptionID, ""),
+		common.ServiceMemoryDB:     newManagedRedisClientFn(r.cred, r.subscriptionID, ""),
 	}
 	lookups := make(map[common.ServiceType]azrecs.PriceFunc, len(clients))
 	for service, client := range clients {

providers/azure/recommendations_advisor_pricing_test.go

@@ -97,7 +97,7 @@ func TestPriceAdvisorReservations_PassesNonReservationsThrough(t *testing.T) {
 func TestPriceAdvisorReservations_ServiceWithoutPricerIsSkipped(t *testing.T) {
 	t.Parallel()
 	ctx, skips := azrecs.WithSkipLog(context.Background())
-	got := priceAdvisorReservations(ctx, []common.Recommendation{riRow(common.ServiceCache, "eastus", "1yr", "Premium_P1", 1, "")}, nil)
+	got := priceAdvisorReservations(ctx, []common.Recommendation{riRow(common.ServiceMemoryDB, "eastus", "1yr", "Premium_P1", 1, "")}, nil)
 	assert.Empty(t, got)
 	require.Len(t, skips.Entries(), 1)
 	assert.Contains(t, skips.Entries()[0], "no reservation pricer for service")

providers/azure/recommendations_redis_held_test.go

@@ -0,0 +1,21 @@
+package azure
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"github.com/LeanerCloud/cloud-commitments-go/pkg/common"
+	"github.com/LeanerCloud/cloud-commitments-go/providers/azure/services/managedredis"
+)
+
+// The dedupe key includes the engine, and held Redis reservations are keyed
+// as "redis", so an Advisor-only Redis row must carry the same engine or a
+// held reservation stops reducing it.
+func TestAdvisorRedisRowCarriesRedisEngine(t *testing.T) {
+	adv := redisAdvisorRow(redisAdapter(t, realRedisClient(), nil), "westus")
+	require.Equal(t, common.ServiceMemoryDB, adv.Service)
+	assert.Equal(t, "redis", common.EngineFromDetails(adv.Details))
+	assert.Equal(t, managedredis.Details(adv.ResourceType), adv.Details)
+}

providers/azure/recommendations_redis_routing_test.go

@@ -0,0 +1,135 @@
+package azure
+
+import (
+	"context"
+	"slices"
+	"sync/atomic"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"github.com/LeanerCloud/cloud-commitments-go/pkg/common"
+	"github.com/LeanerCloud/cloud-commitments-go/providers/azure/mocks"
+	"github.com/LeanerCloud/cloud-commitments-go/providers/azure/services/managedredis"
+)
+
+func TestServiceFromExtendedProperties_RedisCacheIsMemoryDB(t *testing.T) {
+	t.Parallel()
+	got := serviceFromExtendedProperties(map[string]*string{"reservedResourceType": strPtr("RedisCache")})
+	assert.Equal(t, string(common.ServiceMemoryDB), got)
+}
+
+func redisRetailRows() []map[string]any {
+	row := func(region, term string, price float64) map[string]any {
+		return mocks.ReservationRow("Redis Cache", "Azure Redis Cache Premium", region, "Azure_Redis_Cache_Premium_P1_Cache", "P1", "P1 Cache Instance", term, price)
+	}
+	return []map[string]any{row("eastus", "1 Year", 1100), row("eastus", "3 Years", 2600), row("westus", "1 Year", 900)}
+}
+
+// realRedisClient is the production managedredis client over a Consumption
+// RedisCache payload and a stub Retail Prices catalog.
+func realRedisClient() *managedredis.Client {
+	c := managedredis.NewClientWithHTTP(nil, "sub", "", &mocks.PricingHTTP{Items: redisRetailRows()})
+	c.SetRecommendationsPager(&mocks.MockRecommendationsPager{Results: mocks.ConsumptionUnitFixtures("Premium_P1", "eastus"), HasMore: true})
+	return c
+}
+
+func redisAdapter(t *testing.T, redis serviceRecsGetter, advisor func(*RecommendationsClientAdapter) []common.Recommendation) *RecommendationsClientAdapter {
+	t.Helper()
+	overrideServiceClientFns(t, &fakeServiceClient{}, &fakeServiceClient{}, redis, &fakeServiceClient{}, &fakeServiceClient{})
+	a := &RecommendationsClientAdapter{cred: &mockAzureTokenCredential{}, subscriptionID: "sub"}
+	a.getAdvisorRecsFn = func(context.Context, common.RecommendationParams) ([]common.Recommendation, error) {
+		if advisor == nil {
+			return nil, nil
+		}
+		return advisor(a), nil
+	}
+	return a
+}
+
+func TestGetRecommendations_RedisCollectedViaManagedRedisOnly(t *testing.T) {
+	recs, err := redisAdapter(t, realRedisClient(), nil).GetRecommendations(context.Background(), &common.RecommendationParams{})
+
+	require.NoError(t, err)
+	require.Len(t, recs, 4, "one Consumption payload of two recommendations expands to four variants, listed once")
+	for _, rec := range recs {
+		assert.Equal(t, common.ServiceMemoryDB, rec.Service)
+	}
+}
+
+func TestGetRecommendations_RedisFilterFollowsMemoryDB(t *testing.T) {
+	for _, tc := range []struct {
+		service common.ServiceType
+		calls   int32
+	}{{common.ServiceMemoryDB, 1}, {common.ServiceCache, 0}} {
+		redis := &countingFake{}
+		_, err := redisAdapter(t, redis, nil).GetRecommendations(context.Background(), &common.RecommendationParams{Service: tc.service})
+
+		require.NoError(t, err)
+		assert.Equal(t, tc.calls, redis.calls.Load(), "params.Service=%s", tc.service)
+	}
+}
+
+type countingFake struct {
+	fakeServiceClient
+	calls atomic.Int32
+}
+
+func (f *countingFake) GetRecommendations(ctx context.Context, p *common.RecommendationParams) ([]common.Recommendation, error) {
+	f.calls.Add(1)
+	return f.fakeServiceClient.GetRecommendations(ctx, p)
+}
+
+func redisAdvisorRow(a *RecommendationsClientAdapter, region string) common.Recommendation {
+	ext := reservationExt(map[string]*string{"reservedResourceType": strPtr("RedisCache"), "sku": strPtr("Premium_P1"), "term": strPtr("P1Y"), "region": strPtr(region)})
+	return *a.convertAdvisorRecommendation(context.Background(), advisorRec(subscriptionScope, ext))
+}
+
+func TestGetRecommendations_AdvisorRedisIsMemoryDBAndPricedThroughIt(t *testing.T) {
+	advisor := func(a *RecommendationsClientAdapter) []common.Recommendation {
+		return []common.Recommendation{redisAdvisorRow(a, "westus")}
+	}
+	// Only the Advisor row: the Consumption pager is empty, so nothing covers it.
+	redis := managedredis.NewClientWithHTTP(nil, "sub", "", &mocks.PricingHTTP{Items: redisRetailRows()})
+	redis.SetRecommendationsPager(&mocks.MockRecommendationsPager{HasMore: true})
+
+	for _, params := range []common.RecommendationParams{{}, {Service: common.ServiceMemoryDB}} {
+		recs, err := redisAdapter(t, redis, advisor).GetRecommendations(context.Background(), &params)
+
+		require.NoError(t, err)
+		require.Len(t, recs, 2, "upfront and monthly legs of the priced row")
+		for _, rec := range recs {
+			assert.Equal(t, common.ServiceMemoryDB, rec.Service)
+		}
+		assert.True(t, slices.ContainsFunc(recs, func(r common.Recommendation) bool { return r.CommitmentCost == 1800 }), "2 units x 900 from the managedredis price lookup")
+	}
+}
+
+func TestGetRecommendations_AdvisorRedisCoveredByConsumptionRedis(t *testing.T) {
+	// Same account, region and term as the Consumption payload (eastus, 1yr, 3yr).
+	advisor := func(a *RecommendationsClientAdapter) []common.Recommendation {
+		return []common.Recommendation{redisAdvisorRow(a, "eastus")}
+	}
+
+	recs, err := redisAdapter(t, realRedisClient(), advisor).GetRecommendations(context.Background(), &common.RecommendationParams{})
+
+	require.NoError(t, err)
+	assert.Len(t, recs, 4, "the Advisor row is dropped: Consumption already covers that need")
+}
+
+// Tripwire: no Redis source may relabel recommendations as ServiceCache, whose
+// client is going to refuse Redis purchases.
+func TestGetRecommendations_NoRecommendationIsLabelledServiceCache(t *testing.T) {
+	advisor := func(a *RecommendationsClientAdapter) []common.Recommendation {
+		return []common.Recommendation{redisAdvisorRow(a, "westus")}
+	}
+
+	recs, err := redisAdapter(t, realRedisClient(), advisor).GetRecommendations(context.Background(), &common.RecommendationParams{})
+
+	require.NoError(t, err)
+	require.Len(t, recs, 6, "four Consumption variants plus the two legs of the uncovered westus Advisor row")
+	for _, rec := range recs {
+		assert.NotEqual(t, common.ServiceCache, rec.Service)
+	}
+}