From e8993eba0c970fc0c4c401845dc04d2ea7bef502 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Thu, 28 May 2026 20:05:15 +0200 Subject: [PATCH] feat(azure/savings-plans): wire SP rec collection into scheduler (#593) Add savingsplans goroutine to the GetRecommendations fan-out in providers/azure/recommendations.go, mirroring the AWS SP wiring in providers/aws/recommendations/client.go GetAllRecommendations. The SP client's GetRecommendations intentionally returns an empty slice (Azure Benefits Recommendations API is still in preview), so no scheduler change is needed -- the goroutine slot is reserved and will start surfacing data once the upstream API stabilises. Also add TestGetRecommendations_SavingsPlansServiceIncluded to pin that shouldIncludeService passes ServiceSavingsPlans through on both empty-params (all-services) and explicit-service sweeps. --- providers/azure/recommendations.go | 28 +++++++++++++++----- providers/azure/recommendations_test.go | 35 +++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 7 deletions(-) diff --git a/providers/azure/recommendations.go b/providers/azure/recommendations.go index 4019c46ec..6968c2113 100644 --- a/providers/azure/recommendations.go +++ b/providers/azure/recommendations.go @@ -19,6 +19,7 @@ import ( "github.com/LeanerCloud/CUDly/providers/azure/services/compute" "github.com/LeanerCloud/CUDly/providers/azure/services/cosmosdb" "github.com/LeanerCloud/CUDly/providers/azure/services/database" + "github.com/LeanerCloud/CUDly/providers/azure/services/savingsplans" ) // RecommendationsClientAdapter aggregates Azure reservation recommendations across all services. @@ -60,14 +61,14 @@ func NewRecommendationsClientAdapter(cred azcore.TokenCredential, subscriptionID // (see known_issues/10_azure_provider.md CRITICAL "Recommendation converters // ignore the API response entirely" for the matching converter work). // -// All five service calls run concurrently under errgroup. Each goroutine captures +// All six 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 → advisor) after all goroutines finish. +// (compute → database → cache → cosmosdb → savingsplans → advisor) after all goroutines finish. func (r *RecommendationsClientAdapter) GetRecommendations(ctx context.Context, params common.RecommendationParams) ([]common.Recommendation, error) { var ( - computeRecs, dbRecs, cacheRecs, cosmosRecs, advisorRecs []common.Recommendation - computeErr, dbErr, cacheErr, cosmosErr, advisorErr error + computeRecs, dbRecs, cacheRecs, cosmosRecs, advisorRecs, spRecs []common.Recommendation + computeErr, dbErr, cacheErr, cosmosErr, advisorErr, spErr error ) g, gctx := errgroup.WithContext(ctx) @@ -128,6 +129,18 @@ func (r *RecommendationsClientAdapter) GetRecommendations(ctx context.Context, p }) } + // Savings Plans — Azure has no stable public API for SP purchase + // recommendations (Benefits Recommendations API is still in preview). + // The call returns an empty slice so the service appears in the fan-out + // and will start returning data once the API stabilises without requiring + // a scheduler change. + if shouldIncludeService(params, common.ServiceSavingsPlans) { + goService(&spErr, func() { + spClient := savingsplans.NewClient(r.cred, r.subscriptionID, "") + spRecs, spErr = spClient.GetRecommendations(gctx, params) + }) + } + // Azure Advisor adds cross-cutting cost recommendations independent of the // per-service Reservation API. Failures here are non-fatal — the per-service // results above are still useful on their own. @@ -151,6 +164,7 @@ func (r *RecommendationsClientAdapter) GetRecommendations(ctx context.Context, p serviceResult{"database", dbRecs, dbErr}, serviceResult{"cache", cacheRecs, cacheErr}, serviceResult{"cosmosdb", cosmosRecs, cosmosErr}, + serviceResult{"savingsplans", spRecs, spErr}, serviceResult{"advisor", advisorRecs, advisorErr}), nil } @@ -167,9 +181,9 @@ type serviceResult struct { // mergeServiceResults logs per-service errors (matches the previous sequential // behaviour 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 → 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. +// the canonical compute → database → cache → cosmosdb → 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. func mergeServiceResults(results ...serviceResult) []common.Recommendation { total := 0 for _, r := range results { diff --git a/providers/azure/recommendations_test.go b/providers/azure/recommendations_test.go index b2eb251db..ecbd1fd41 100644 --- a/providers/azure/recommendations_test.go +++ b/providers/azure/recommendations_test.go @@ -216,6 +216,41 @@ func TestRecommendationsClientAdapter_GetAllRecommendations(t *testing.T) { _, _ = adapter.GetAllRecommendations(context.Background()) } +// TestGetRecommendations_SavingsPlansServiceIncluded pins that shouldIncludeService +// allows ServiceSavingsPlans through both when params.Service is empty (all-services +// sweep) and when explicitly set to ServiceSavingsPlans, and does not include it +// when a different service is requested. This ensures the SP goroutine added to the +// fan-out in GetRecommendations is exercised on every scheduler collection run. +func TestGetRecommendations_SavingsPlansServiceIncluded(t *testing.T) { + tests := []struct { + name string + params common.RecommendationParams + expected bool + }{ + { + name: "empty params includes savingsplans", + params: common.RecommendationParams{}, + expected: true, + }, + { + name: "explicit savingsplans service is included", + params: common.RecommendationParams{Service: common.ServiceSavingsPlans}, + expected: true, + }, + { + name: "different service excludes savingsplans", + params: common.RecommendationParams{Service: common.ServiceCompute}, + expected: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := shouldIncludeService(tt.params, common.ServiceSavingsPlans) + assert.Equal(t, tt.expected, got) + }) + } +} + // TestRecommendationsClientAdapter_GetRecommendations_PropagatesContextCancellation // pins the contract that GetRecommendations propagates ctx.Err() to its caller // after the errgroup Wait() — the parent context being cancelled or its