diff --git a/providers/azure/internal/recommendations/consumption.go b/providers/azure/internal/recommendations/consumption.go index 12f53a5c..354510a3 100644 --- a/providers/azure/internal/recommendations/consumption.go +++ b/providers/azure/internal/recommendations/consumption.go @@ -185,8 +185,8 @@ func TermYears(term string) (int, error) { // ReservationPrice is the retail price of one reservation unit for a whole // term (the Retail Prices API's priceType 'Reservation' row), in Currency. type ReservationPrice struct { - Total float64 Currency string + Total float64 } // PriceFunc resolves the term-total retail price of one reservation unit. @@ -198,8 +198,8 @@ type priceKey struct { } type priceResult struct { - price float64 err error + price float64 } // Pricer resolves reservation unit prices through lookup, once per distinct @@ -208,9 +208,9 @@ type priceResult struct { // catalog (see IsCollectionFailure) is never cached. It is meant // to live for one GetRecommendations call and is not safe for concurrent use. type Pricer struct { - service string lookup PriceFunc cache map[priceKey]priceResult + service string } // NewPricer returns a Pricer for service backed by lookup. @@ -300,8 +300,8 @@ func ExpandConsumptionVariants(ctx context.Context, base common.Recommendation, // SkipLog collects the recommendations a collection dropped. It is attached // to the collection context by WithSkipLog and is safe for concurrent use. type SkipLog struct { - mu sync.Mutex entries []string + mu sync.Mutex } type skipLogKey struct{} diff --git a/providers/azure/internal/recommendations/converter.go b/providers/azure/internal/recommendations/converter.go index 8cef5f7c..838e1f40 100644 --- a/providers/azure/internal/recommendations/converter.go +++ b/providers/azure/internal/recommendations/converter.go @@ -33,16 +33,18 @@ import ( // reservation recommendation services, normalised across both Legacy // and Modern API response shapes. type ExtractedFields struct { - Region string - ResourceType string - Count int - OnDemandCost float64 - CommitmentCost float64 - EstimatedSavings float64 - Term string - // LookBackDays is the lookback window the cost fields were computed over; - // set only by ExtractConsumption (0 from plain Extract). - LookBackDays int + // RecurringMonthlyCost is the covered/effective cost over Azure's lookback + // window, set by plain Extract only (from TotalCostWithReservedInstances, or + // OnDemandCost - NetSavings when that is absent; nil, never a fabricated 0, + // when neither is available). ExtractConsumption always leaves it nil: + // ExpandConsumptionVariants sets it per payment variant (0 for upfront, + // Count x reservation price / term months for monthly). + RecurringMonthlyCost *float64 + + Region string + ResourceType string + Term string + // Scope is populated from the API response ("Shared" or a subscription ID) // but is not yet threaded into the purchase body. All service clients // currently send armreservations.AppliedScopeTypeShared, which is correct because @@ -51,13 +53,15 @@ type ExtractedFields struct { // ever requested. This field is retained for future wiring if single- // subscription scoped recommendations are ever requested. Scope string - // RecurringMonthlyCost is the covered/effective cost over Azure's lookback - // window, set by plain Extract only (from TotalCostWithReservedInstances, or - // OnDemandCost - NetSavings when that is absent; nil, never a fabricated 0, - // when neither is available). ExtractConsumption always leaves it nil: - // ExpandConsumptionVariants sets it per payment variant (0 for upfront, - // Count x reservation price / term months for monthly). - RecurringMonthlyCost *float64 + + Count int + OnDemandCost float64 + CommitmentCost float64 + EstimatedSavings float64 + + // LookBackDays is the lookback window the cost fields were computed over; + // set only by ExtractConsumption (0 from plain Extract). + LookBackDays int } // float64Ptr returns a pointer to the given float64 value. Used to diff --git a/providers/azure/mocks/consumption_fixtures.go b/providers/azure/mocks/consumption_fixtures.go index 7b45f3b4..4275ebdc 100644 --- a/providers/azure/mocks/consumption_fixtures.go +++ b/providers/azure/mocks/consumption_fixtures.go @@ -46,13 +46,13 @@ func AssertConsumptionUnitVariants(t *testing.T, recs []common.Recommendation, p t.Helper() require.Len(t, recs, 4, "two recommendations expand to four variants") cases := []struct { + term string upfront, monthly common.Recommendation onDemand, total, days float64 unit, months float64 - term string }{ - {recs[0], recs[1], fixture1yOnDemand, fixture1yTotal, fixtureLegacyWinDy, price1y, 12, "1yr"}, - {recs[2], recs[3], fixture3yOnDemand, fixture3yTotal, fixtureModernWinDy, price3y, 36, "3yr"}, + {"1yr", recs[0], recs[1], fixture1yOnDemand, fixture1yTotal, fixtureLegacyWinDy, price1y, 12}, + {"3yr", recs[2], recs[3], fixture3yOnDemand, fixture3yTotal, fixtureModernWinDy, price3y, 36}, } for i := range cases { c := &cases[i] diff --git a/providers/azure/mocks/pricing_http.go b/providers/azure/mocks/pricing_http.go index 7972c5e5..4acf7bab 100644 --- a/providers/azure/mocks/pricing_http.go +++ b/providers/azure/mocks/pricing_http.go @@ -14,13 +14,13 @@ import ( // the lookup for one SKU fail while the others succeed. The rows are // hand-built (synthetic prices, not a live Azure quote). type PricingHTTP struct { - Items []map[string]any - FailMatch string // Err, when set, is returned from every request (a transport failure). Err error - mu sync.Mutex - calls int + FailMatch string + Items []map[string]any + calls int + mu sync.Mutex } // Do implements the HTTPClient interface the service clients accept. diff --git a/providers/azure/provider.go b/providers/azure/provider.go index c413f46e..eccd2f4c 100644 --- a/providers/azure/provider.go +++ b/providers/azure/provider.go @@ -96,12 +96,11 @@ func (r *realCredentialProvider) NewDefaultAzureCredential() (azcore.TokenCreden // Provider implements the provider.Provider interface for Azure. type Provider struct { cred azcore.TokenCredential - credOnce sync.Once credErr error - subscriptionID string - region string // Default region for operations subscriptionsClient SubscriptionsClient credProvider CredentialProvider + subscriptionID string + region string // Default region for operations // accountsMu guards cachedAccounts. GetAccounts, GetServiceClient, and // GetRecommendationsClient all resolve the subscription list on the hot @@ -121,10 +120,12 @@ type Provider struct { // singleflight call and gates the cache write, so an ARM fetch that // started before an invalidation can neither publish its stale snapshot // nor be joined by a caller that arrived after it. - accountsMu sync.RWMutex + accountsSF singleflight.Group cachedAccounts []common.Account accountsGen uint64 - accountsSF singleflight.Group + accountsMu sync.RWMutex + + credOnce sync.Once // cache subsystem implementation (getOrFetchAccounts, fetchAccounts, // cloneAccounts, InvalidateAccountsCache) lives in accounts_cache.go. diff --git a/providers/azure/recommendations.go b/providers/azure/recommendations.go index 2ffcc7f3..36a1daf8 100644 --- a/providers/azure/recommendations.go +++ b/providers/azure/recommendations.go @@ -79,8 +79,8 @@ var ( // overridden per-instance in tests to avoid real ARM network calls. type RecommendationsClientAdapter struct { cred azcore.TokenCredential - subscriptionID string getAdvisorRecsFn func(ctx context.Context, params common.RecommendationParams) ([]common.Recommendation, error) + subscriptionID string } // NewRecommendationsClientAdapter builds a RecommendationsClientAdapter with @@ -222,11 +222,11 @@ func (r *RecommendationsClientAdapter) GetRecommendations(ctx context.Context, p return nil, err } - merged, err := mergeServiceResults(serviceResult{"compute", computeRecs, computeErr, includeCompute}, - serviceResult{"database", dbRecs, dbErr, includeDB}, - serviceResult{"cache", cacheRecs, cacheErr, includeCache}, - serviceResult{"cosmosdb", cosmosRecs, cosmosErr, includeCosmos}, - serviceResult{"synapse", synapseRecs, synapseErr, includeSynapse}, + 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: 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 // ([], nil) until the Benefits Recommendations API stabilizes (see // services/savingsplans Client.GetRecommendations). Counting its @@ -234,7 +234,7 @@ func (r *RecommendationsClientAdapter) GetRecommendations(ctx context.Context, p // all-attempted-failed guard from ever firing on a total provider // failure (expired credential, subscription-wide throttle), so it is // excluded from the guard until it makes real API calls. - serviceResult{"savingsplans", spRecs, spErr, false}, + serviceResult{err: spErr, name: "savingsplans", recs: spRecs, attempted: false}, // The Advisor client is excluded from the all-attempted-failed guard // for the same reason: getAdvisorRecommendations swallows pagination // errors (the auth failure from an expired credential surfaces as a @@ -244,7 +244,7 @@ func (r *RecommendationsClientAdapter) GetRecommendations(ctx context.Context, p // credential failure -- the same hazard the savingsplans exclusion // prevents. When getAdvisorRecommendations is changed to propagate // hard errors, flip this flag back to true. - serviceResult{"advisor", dropCoveredAdvisorSavingsPlans(advisorRecs, spRecs), advisorErr, false}) + serviceResult{err: advisorErr, name: "advisor", recs: dropCoveredAdvisorSavingsPlans(advisorRecs, spRecs), attempted: false}) return withSkips(merged, err, skips) } @@ -302,9 +302,10 @@ func (r *RecommendationsClientAdapter) advisorFn() func(context.Context, common. // stays under the cyclomatic-complexity gate after the post-Wait ctx.Err() // propagation was added. type serviceResult struct { + err error name string recs []common.Recommendation - err error + // attempted records whether the service call was actually launched // (i.e. not skipped by the params service filter). Skipped services // carry nil recs and nil err, which is indistinguishable from a diff --git a/providers/azure/recommendations_multi_subscription.go b/providers/azure/recommendations_multi_subscription.go index 31d4f215..c8f7a2d2 100644 --- a/providers/azure/recommendations_multi_subscription.go +++ b/providers/azure/recommendations_multi_subscription.go @@ -32,15 +32,15 @@ var newSubscriptionRecommendationsClientFn = func(cred azcore.TokenCredential, s // client so fan-out logs and error messages can identify which subscription // a failure came from. type subscriptionClient struct { - subscriptionID string client provider.RecommendationsClient + subscriptionID string } // SubscriptionFailure records one subscription that could not be queried // during an org-wide fan-out. type SubscriptionFailure struct { - SubscriptionID string Err error + SubscriptionID string } // ErrSubscriptionNotAccessible is the cause recorded for a subscription named @@ -93,17 +93,19 @@ func AsPartialResult(err error) *PartialResultError { // Failing the whole sweep on one transient subscription error would be worse // than a partial result, which is why the successful data is still returned. type PartialSubscriptionFailureError struct { + // Failed carries every subscription that errored, with its cause. + Failed []SubscriptionFailure + // Attempted is how many subscriptions the sweep was supposed to cover: // the ones actually queried plus any named by AccountFilter that the // principal cannot see (those are never queried, but they were asked // for, so leaving them out of the denominator would under-report the // gap this error exists to surface). Attempted int + // Succeeded is how many returned a result. Always < Attempted and > 0: // an all-failed sweep is a plain error, not a partial one. Succeeded int - // Failed carries every subscription that errored, with its cause. - Failed []SubscriptionFailure } func (e *PartialSubscriptionFailureError) Error() string { diff --git a/providers/azure/recommendations_test.go b/providers/azure/recommendations_test.go index 44f82153..fbf9157e 100644 --- a/providers/azure/recommendations_test.go +++ b/providers/azure/recommendations_test.go @@ -426,12 +426,12 @@ func TestMergeServiceResults_OrderIsStable(t *testing.T) { // Replicate the exact call order from GetRecommendations. result, err := mergeServiceResults( - serviceResult{"compute", []common.Recommendation{computeRec}, nil, true}, - serviceResult{"database", []common.Recommendation{dbRec}, nil, true}, - serviceResult{"cache", []common.Recommendation{cacheRec}, nil, true}, - serviceResult{"cosmosdb", []common.Recommendation{cosmosRec}, nil, true}, - serviceResult{"savingsplans", []common.Recommendation{spRec}, nil, true}, - serviceResult{"advisor", []common.Recommendation{advisorRec}, nil, true}, + serviceResult{err: nil, name: "compute", recs: []common.Recommendation{computeRec}, attempted: true}, + serviceResult{err: nil, name: "database", recs: []common.Recommendation{dbRec}, attempted: true}, + serviceResult{err: nil, name: "cache", recs: []common.Recommendation{cacheRec}, attempted: true}, + serviceResult{err: nil, name: "cosmosdb", recs: []common.Recommendation{cosmosRec}, attempted: true}, + serviceResult{err: nil, name: "savingsplans", recs: []common.Recommendation{spRec}, attempted: true}, + serviceResult{err: nil, name: "advisor", recs: []common.Recommendation{advisorRec}, attempted: true}, ) require.NoError(t, err, "all-success merge must not error") @@ -464,12 +464,12 @@ func TestMergeServiceResults_AllAttemptedFailed(t *testing.T) { authErr := errors.New("DefaultAzureCredential: federated token expired") recs, err := mergeServiceResults( - serviceResult{"compute", nil, authErr, true}, - serviceResult{"database", nil, authErr, true}, - serviceResult{"cache", nil, authErr, true}, - serviceResult{"cosmosdb", nil, authErr, true}, - serviceResult{"savingsplans", nil, nil, false}, - serviceResult{"advisor", nil, nil, false}, + serviceResult{err: authErr, name: "compute", recs: nil, attempted: true}, + serviceResult{err: authErr, name: "database", recs: nil, attempted: true}, + serviceResult{err: authErr, name: "cache", recs: nil, attempted: true}, + serviceResult{err: authErr, name: "cosmosdb", recs: nil, attempted: true}, + serviceResult{err: nil, name: "savingsplans", recs: nil, attempted: false}, + serviceResult{err: nil, name: "advisor", recs: nil, attempted: false}, ) require.Error(t, err, "all-attempted-failed merge must fail loud, not return (empty, nil)") @@ -490,12 +490,12 @@ func TestMergeServiceResults_SkippedServicesDoNotMaskTotalFailure(t *testing.T) throttleErr := errors.New("429 too many requests") _, err := mergeServiceResults( - serviceResult{"compute", nil, throttleErr, true}, - serviceResult{"database", nil, nil, false}, - serviceResult{"cache", nil, nil, false}, - serviceResult{"cosmosdb", nil, nil, false}, - serviceResult{"savingsplans", nil, nil, false}, - serviceResult{"advisor", nil, nil, false}, + serviceResult{err: throttleErr, name: "compute", recs: nil, attempted: true}, + serviceResult{err: nil, name: "database", recs: nil, attempted: false}, + serviceResult{err: nil, name: "cache", recs: nil, attempted: false}, + serviceResult{err: nil, name: "cosmosdb", recs: nil, attempted: false}, + serviceResult{err: nil, name: "savingsplans", recs: nil, attempted: false}, + serviceResult{err: nil, name: "advisor", recs: nil, attempted: false}, ) require.Error(t, err, "skipped services must not count as successes in the all-failed guard") @@ -511,12 +511,12 @@ func TestMergeServiceResults_PartialFailureStillSucceeds(t *testing.T) { computeRec := common.Recommendation{Service: common.ServiceCompute, Provider: common.ProviderAzure} recs, err := mergeServiceResults( - serviceResult{"compute", []common.Recommendation{computeRec}, nil, true}, - serviceResult{"database", nil, svcErr, true}, - serviceResult{"cache", nil, svcErr, true}, - serviceResult{"cosmosdb", nil, svcErr, true}, - serviceResult{"savingsplans", nil, nil, false}, - serviceResult{"advisor", nil, nil, false}, + serviceResult{err: nil, name: "compute", recs: []common.Recommendation{computeRec}, attempted: true}, + serviceResult{err: svcErr, name: "database", recs: nil, attempted: true}, + serviceResult{err: svcErr, name: "cache", recs: nil, attempted: true}, + serviceResult{err: svcErr, name: "cosmosdb", recs: nil, attempted: true}, + serviceResult{err: nil, name: "savingsplans", recs: nil, attempted: false}, + serviceResult{err: nil, name: "advisor", recs: nil, attempted: false}, ) require.NoError(t, err, "partial failure must still return the successful services' recs") @@ -537,12 +537,12 @@ func TestMergeServiceResults_StubsDoNotMaskTotalFailure(t *testing.T) { authErr := errors.New("DefaultAzureCredential: federated token expired") recs, err := mergeServiceResults( - serviceResult{"compute", nil, authErr, true}, - serviceResult{"database", nil, authErr, true}, - serviceResult{"cache", nil, authErr, true}, - serviceResult{"cosmosdb", nil, authErr, true}, - serviceResult{"savingsplans", []common.Recommendation{}, nil, false}, - serviceResult{"advisor", []common.Recommendation{}, nil, false}, + serviceResult{err: authErr, name: "compute", recs: nil, attempted: true}, + serviceResult{err: authErr, name: "database", recs: nil, attempted: true}, + serviceResult{err: authErr, name: "cache", recs: nil, attempted: true}, + serviceResult{err: authErr, name: "cosmosdb", recs: nil, attempted: true}, + serviceResult{err: nil, name: "savingsplans", recs: []common.Recommendation{}, attempted: false}, + serviceResult{err: nil, name: "advisor", recs: []common.Recommendation{}, attempted: false}, ) require.Error(t, err, "stubs that swallow errors must not mask a total provider failure") diff --git a/providers/azure/services/cache/client.go b/providers/azure/services/cache/client.go index 7cbc96f5..198a26c7 100644 --- a/providers/azure/services/cache/client.go +++ b/providers/azure/services/cache/client.go @@ -70,13 +70,23 @@ type RedisCachesPager interface { // Client handles Azure Cache for Redis Reserved Capacity. type Client struct { cred azcore.TokenCredential - subscriptionID string - region string httpClient HTTPClient recommendationsPager RecommendationsPager reservationsPager ReservationsDetailsPager redisCachesPager RedisCachesPager + // Lazy SKU catalog cache. armredis exposes no per-region "list all + // possible SKUs" surface, so we derive shard counts from the existing + // caches in the subscription (NewListBySubscriptionPager). Fetched + // ONCE per client lifetime; subsequent converter calls in the same + // GetRecommendations run hit the in-memory map. A failed fetch leaves + // skuCacheMap nil and converters fall back to Shards=0 with a WARN + // log — the conversion itself does NOT fail. + skuCacheMap map[string]redisSKUEntry + + subscriptionID string + region string + // Lazy SKU catalog cache. armredis exposes no per-region "list all // possible SKUs" surface, so we derive shard counts from the existing // caches in the subscription (NewListBySubscriptionPager). Fetched @@ -85,7 +95,6 @@ type Client struct { // skuCacheMap nil and converters fall back to Shards=0 with a WARN // log — the conversion itself does NOT fail. skuCacheOnce sync.Once - skuCacheMap map[string]redisSKUEntry } // NewClient creates a new Azure Cache client. @@ -541,9 +550,9 @@ func (c *Client) getCommonSKUs() []string { // RedisPricing contains pricing information for Redis Cache. type RedisPricing struct { + Currency string HourlyRate float64 ReservationPrice float64 - Currency string } func (c *Client) getRedisPricing(ctx context.Context, sku, region string, termYears int) (*RedisPricing, error) { diff --git a/providers/azure/services/compute/client.go b/providers/azure/services/compute/client.go index 2f25efa8..1b30578f 100644 --- a/providers/azure/services/compute/client.go +++ b/providers/azure/services/compute/client.go @@ -87,15 +87,22 @@ type vmSKUEntry struct { // Client handles Azure VM Reserved Instances. type Client struct { - cred azcore.TokenCredential - subscriptionID string - region string - httpClient HTTPClient + cred azcore.TokenCredential + httpClient HTTPClient // For testing - these can be set to mock implementations recommendationsPager RecommendationsPager resourceSKUsPager ResourceSKUsPager + // Microsoft.Capacity provider registration check (cached per client lifetime) + capacityProviderErr error + + // Optional injected pager for ListExchangeableReservations. When nil + // (the production default) the method creates a real + // armreservations.ReservationClient. Tests inject a stub to run + // hermetically without Azure credentials. + exchangeablePager ExchangeableReservationPager + // Optional injected factories for the reservation-inventory path used by // GetExistingCommitments. When nil (the production default) the method // builds real armreservations SDK clients via @@ -103,10 +110,6 @@ type Client struct { // SetInventoryFactories to run hermetically without Azure credentials. inventoryFactories *InventoryFactories - // Microsoft.Capacity provider registration check (cached per client lifetime) - capacityProviderOnce sync.Once - capacityProviderErr error - // Lazy SKU catalog cache. armcompute.ResourceSKUsClient.NewListPager // returns every SKU available to the subscription with its // Capabilities (vCPUs, MemoryGB). Fetched ONCE per client lifetime; @@ -115,14 +118,7 @@ type Client struct { // converters fall back to VCPU=0/MemoryGB=0 with a WARN log — the // conversion itself does NOT fail (graceful-degradation contract, // matches cache/cosmosdb/database from PR #81). - skuCacheOnce sync.Once - skuCacheMap map[string]vmSKUEntry - - // Optional injected pager for ListExchangeableReservations. When nil - // (the production default) the method creates a real - // armreservations.ReservationClient. Tests inject a stub to run - // hermetically without Azure credentials. - exchangeablePager ExchangeableReservationPager + skuCacheMap map[string]vmSKUEntry // Optional injected LRO callers for CalculateExchange and // ExecuteExchange. When nil (the production default) the methods @@ -137,6 +133,22 @@ type Client struct { // fake HTTP transport so the production LRO wrappers and the SDK's own // request serialization run without reaching Azure. exchangeClientOptions *arm.ClientOptions + + subscriptionID string + region string + + // Microsoft.Capacity provider registration check (cached per client lifetime) + capacityProviderOnce sync.Once + + // Lazy SKU catalog cache. armcompute.ResourceSKUsClient.NewListPager + // returns every SKU available to the subscription with its + // Capabilities (vCPUs, MemoryGB). Fetched ONCE per client lifetime; + // subsequent converter calls in the same GetRecommendations run hit + // the in-memory map. A failed fetch leaves skuCacheMap nil and + // converters fall back to VCPU=0/MemoryGB=0 with a WARN log — the + // conversion itself does NOT fail (graceful-degradation contract, + // matches cache/cosmosdb/database from PR #81). + skuCacheOnce sync.Once } // NewClient creates a new Azure Compute client. @@ -674,9 +686,9 @@ func (c *Client) isAvailableInRegion(sku *armcompute.ResourceSKU, region string) // VMPricing contains VM pricing information. type VMPricing struct { + Currency string HourlyRate float64 ReservationPrice float64 - Currency string } // getVMPricing gets real VM pricing from Azure Retail Prices API. diff --git a/providers/azure/services/compute/exchange.go b/providers/azure/services/compute/exchange.go index 72206101..0c0a32c5 100644 --- a/providers/azure/services/compute/exchange.go +++ b/providers/azure/services/compute/exchange.go @@ -28,6 +28,9 @@ import ( // ExchangeableReservation represents an Azure VM reservation that is // eligible for exchange. type ExchangeableReservation struct { + // ExpiryDate is when the reservation expires. Zero if not set by Azure. + ExpiryDate time.Time `json:"expiry_date,omitempty"` + // ReservationOrderID is the GUID of the parent reservation order, // parsed from the ARM resource ID. Required by CalculateExchange. ReservationOrderID string `json:"reservation_order_id"` @@ -59,16 +62,9 @@ type ExchangeableReservation struct { // Azure did not report one, which CalculateExchange refuses. AppliedScopeType armreservations.AppliedScopeType `json:"applied_scope_type,omitempty"` - // AppliedScopes are the ARM scopes (e.g. "/subscriptions/{id}") a Single - // scoped reservation applies to. Empty for Shared. - AppliedScopes []string `json:"applied_scopes,omitempty"` - // SKU is the VM size (e.g. "Standard_D2s_v3"). SKU string `json:"sku"` - // Quantity is the number of VM instances covered. - Quantity int32 `json:"quantity"` - // Region is the ARM region name (e.g. "eastus"). May be empty for // reservations with AppliedScopeType == Shared. Region string `json:"region,omitempty"` @@ -79,15 +75,19 @@ type ExchangeableReservation struct { // pair as unsupported. Term string `json:"term,omitempty"` - // ExpiryDate is when the reservation expires. Zero if not set by Azure. - ExpiryDate time.Time `json:"expiry_date,omitempty"` - // InstanceFlexibility reports the instance size flexibility setting. // Always "On" for reservations returned by this function. InstanceFlexibility string `json:"instance_flexibility"` // DisplayName is the human-readable reservation name set at purchase time. DisplayName string `json:"display_name,omitempty"` + + // AppliedScopes are the ARM scopes (e.g. "/subscriptions/{id}") a Single + // scoped reservation applies to. Empty for Shared. + AppliedScopes []string `json:"applied_scopes,omitempty"` + + // Quantity is the number of VM instances covered. + Quantity int32 `json:"quantity"` } // ExchangeableReservationPager defines the paging contract for listing @@ -194,13 +194,13 @@ func isExchangeEligible(item *armreservations.ReservationResponse) bool { // when the source pointer is nil. Grouped into a struct (rather than 7 // positional returns) per gocritic's tooManyResultsChecker. type reservationFields struct { + expiryDate time.Time id string sku string region string term string displayName string quantity int32 - expiryDate time.Time } // extractReservationFields reads the optional pointer fields from item and its diff --git a/providers/azure/services/compute/exchange_operations.go b/providers/azure/services/compute/exchange_operations.go index 298b8a36..b519da5c 100644 --- a/providers/azure/services/compute/exchange_operations.go +++ b/providers/azure/services/compute/exchange_operations.go @@ -33,6 +33,11 @@ import ( // CompatibleOffering describes one candidate target SKU that Azure priced as // an exchange destination for the source reservations. type CompatibleOffering struct { + // BillingCurrencyTotal is the net amount the customer would pay in their + // billing currency for this offering. Nil when Azure did not report an + // amount (never coerced to 0 -- absent is not the same as free). + BillingCurrencyTotal *float64 `json:"billing_currency_total"` + // SKU is the VM size (e.g. "Standard_D4s_v3"). SKU string `json:"sku"` @@ -46,16 +51,11 @@ type CompatibleOffering struct { // pair as unsupported. Term string `json:"term"` - // Quantity is the number of instances that would be purchased. - Quantity int32 `json:"quantity"` - - // BillingCurrencyTotal is the net amount the customer would pay in their - // billing currency for this offering. Nil when Azure did not report an - // amount (never coerced to 0 -- absent is not the same as free). - BillingCurrencyTotal *float64 `json:"billing_currency_total"` - // CurrencyCode is the ISO 4217 code for BillingCurrencyTotal (e.g. "USD"). CurrencyCode string `json:"currency_code,omitempty"` + + // Quantity is the number of instances that would be purchased. + Quantity int32 `json:"quantity"` } // ExchangePreview holds the priced result of a CalculateExchange call: what @@ -123,9 +123,6 @@ type ExchangeTarget struct { // CalculateExchange rather than second-guessed here. Term armreservations.ReservationTerm - // Quantity is the number of instances to reserve. Required: must be >= 1. - Quantity int32 - // BillingScopeID is the subscription or billing account that will be // charged. Required by the Azure exchange API. BillingScopeID string @@ -145,6 +142,9 @@ type ExchangeTarget struct { // CalculateExchange session; ExecuteExchange sends only the session ID, // so the executed exchange is billed with the plan that was priced. BillingPlan armreservations.ReservationBillingPlan + + // Quantity is the number of instances to reserve. Required: must be >= 1. + Quantity int32 } // CalculateExchangeCallerFunc is the narrow LRO-invoker interface that diff --git a/providers/azure/services/cosmosdb/client.go b/providers/azure/services/cosmosdb/client.go index 0d149f16..29a0e32a 100644 --- a/providers/azure/services/cosmosdb/client.go +++ b/providers/azure/services/cosmosdb/client.go @@ -68,12 +68,12 @@ type CosmosAccountsPager interface { // Client handles Azure Cosmos DB Reserved Capacity. type Client struct { cred azcore.TokenCredential - subscriptionID string - region string httpClient HTTPClient recommendationsPager RecommendationsPager reservationsPager ReservationsDetailsPager cosmosAccountsPager CosmosAccountsPager + subscriptionID string + region string // Lazy account-API-type cache. Cosmos DB has no per-SKU APIType (the // API type lives on the account, not the reservation SKU which is @@ -82,8 +82,8 @@ type Client struct { // dominant APIType the converter can apply to recommendations. // "Dominant" = the only API type observed if there's exactly one, // else empty (meaning: subscription is multi-API, can't infer). - apiTypeOnce sync.Once apiType string + apiTypeOnce sync.Once } // NewClient creates a new Azure Cosmos DB client. @@ -539,9 +539,9 @@ func (c *Client) getCommonSKUs() []string { // CosmosPricing contains pricing information for Cosmos DB. type CosmosPricing struct { + Currency string HourlyRate float64 ReservationPrice float64 - Currency string } func (c *Client) getCosmosPricing(ctx context.Context, sku, region string, termYears int) (*CosmosPricing, error) { diff --git a/providers/azure/services/database/client.go b/providers/azure/services/database/client.go index 1955d3d2..27b44ff0 100644 --- a/providers/azure/services/database/client.go +++ b/providers/azure/services/database/client.go @@ -85,9 +85,15 @@ type SQLManagedInstancesPager interface { // Client handles Azure SQL Database Reserved Capacity. type Client struct { + // Lazy server-info cache. Populated by fetchServerInfo, which walks the + // managed-instances and servers pagers. Both azConfig and deployment are + // derived in a single pass so the injected test pager is consumed once. + // Only a complete walk is cached (serverInfoLoaded); a failed or + // canceled walk is retried, but not before serverInfoRetryAfter so a + // persistently failing subscription is not re-walked per recommendation. + serverInfoRetryAfter time.Time + cred azcore.TokenCredential - subscriptionID string - region string httpClient HTTPClient recommendationsPager RecommendationsPager reservationsPager ReservationsDetailsPager @@ -100,8 +106,10 @@ type Client struct { // call — single ListByLocation per region instead of N+1 per rec. // A failed fetch leaves skuCacheMap nil; converters then fall back // to empty EngineVersion with a one-time WARN log. - skuCacheOnce sync.Once - skuCacheMap map[string]sqlSKUEntry + skuCacheMap map[string]sqlSKUEntry + + subscriptionID string + region string // Lazy server-info cache. Populated by fetchServerInfo, which walks the // managed-instances and servers pagers. Both azConfig and deployment are @@ -109,12 +117,25 @@ type Client struct { // Only a complete walk is cached (serverInfoLoaded); a failed or // canceled walk is retried, but not before serverInfoRetryAfter so a // persistently failing subscription is not re-walked per recommendation. - serverInfoMu sync.Mutex - serverInfoLoaded bool - serverInfoRetryAfter time.Time - serverInfoRetryDelay time.Duration azConfig string deployment string + serverInfoRetryDelay time.Duration + + // Lazy SKU catalog cache. Populated once on the first + // recommendation conversion in this client's GetRecommendations + // call — single ListByLocation per region instead of N+1 per rec. + // A failed fetch leaves skuCacheMap nil; converters then fall back + // to empty EngineVersion with a one-time WARN log. + skuCacheOnce sync.Once + + // Lazy server-info cache. Populated by fetchServerInfo, which walks the + // managed-instances and servers pagers. Both azConfig and deployment are + // derived in a single pass so the injected test pager is consumed once. + // Only a complete walk is cached (serverInfoLoaded); a failed or + // canceled walk is retried, but not before serverInfoRetryAfter so a + // persistently failing subscription is not re-walked per recommendation. + serverInfoMu sync.Mutex + serverInfoLoaded bool } // serverInfoRetryDelayDefault is the minimum wait before re-walking the @@ -570,9 +591,9 @@ func (c *Client) extractManagedInstanceSKUs(capabilities armsql.LocationCapabili // SQLPricing contains pricing information for SQL Database. type SQLPricing struct { + Currency string HourlyRate float64 ReservationPrice float64 - Currency string } // getSQLPricing gets real pricing from Azure Retail Prices API. diff --git a/providers/azure/services/internal/reservations/displayname.go b/providers/azure/services/internal/reservations/displayname.go index 97b55d88..0767c173 100644 --- a/providers/azure/services/internal/reservations/displayname.go +++ b/providers/azure/services/internal/reservations/displayname.go @@ -52,6 +52,12 @@ func SanitizeDisplayName(s string) string { // deterministic assertions. Production callers leave randSource nil (the // builder then uses crypto/rand) and pass time.Now(). type DisplayNameFields struct { + // Now is the timestamp baseline. Tests inject a fixed value for + // determinism; production callers should pass time.Now(). A zero + // time.Time is replaced with time.Now() by the builder so the + // timestamp segment never emits the placeholder "00010101T000000". + Now time.Time + // Service is a short identifier for the Azure service, e.g. "vm", // "redis", "cosmos", "sql", "search". Set per-call by the service // client; the builder treats it as opaque and sanitizes it. @@ -63,9 +69,6 @@ type DisplayNameFields struct { // ResourceType is the Azure SKU name (e.g. "Standard_D2a_v4"). ResourceType string - // Count is the reservation quantity. Always rendered as "{N}x". - Count int - // Term is the commitment term, normalized to "1yr" / "3yr" by upstream // recommendation parsers. Pass through as-is; the builder collapses // it to "1yr"/"3yr" when possible, and sanitizes otherwise. @@ -77,16 +80,13 @@ type DisplayNameFields struct { // "monthly") so the segment stays under 8 chars. Payment string - // Now is the timestamp baseline. Tests inject a fixed value for - // determinism; production callers should pass time.Now(). A zero - // time.Time is replaced with time.Now() by the builder so the - // timestamp segment never emits the placeholder "00010101T000000". - Now time.Time - // randSource is an optional 4-byte source for the random suffix. // When nil (production), the builder reads from crypto/rand. Tests set // it via WithRandSource to make output deterministic. randSource []byte + + // Count is the reservation quantity. Always rendered as "{N}x". + Count int } // WithRandSource returns a copy of f with the given bytes used as the diff --git a/providers/azure/services/internal/reservations/purchase.go b/providers/azure/services/internal/reservations/purchase.go index 9ff4912f..3bc3e115 100644 --- a/providers/azure/services/internal/reservations/purchase.go +++ b/providers/azure/services/internal/reservations/purchase.go @@ -445,7 +445,8 @@ func doCalculatePrice(ctx context.Context, httpClient HTTPClient, calcURL string // the list-reservation-orders endpoint. Only the fields needed for the // idempotency-token lookup are decoded. type reservationOrdersListResponse struct { - Value []struct { + NextLink string `json:"nextLink"` + Value []struct { // Name is the reservation order GUID; this is what Azure mints as // reservationOrderId during calculatePrice and what DoPurchaseTwoStep // returns on success. @@ -468,7 +469,6 @@ type reservationOrdersListResponse struct { DisplayName string `json:"displayName"` } `json:"properties"` } `json:"value"` - NextLink string `json:"nextLink"` } // orderDisposition says what an existing reservation order that carries the diff --git a/providers/azure/services/internal/reservations/purchasecommitment.go b/providers/azure/services/internal/reservations/purchasecommitment.go index b1def490..54d8a708 100644 --- a/providers/azure/services/internal/reservations/purchasecommitment.go +++ b/providers/azure/services/internal/reservations/purchasecommitment.go @@ -15,10 +15,11 @@ import ( // PurchaseParams holds what differs between the Azure service clients when // they build a reservation purchase body. type PurchaseParams struct { - // ReservedResourceType is the properties.reservedResourceType value. - ReservedResourceType string // DisplayName returns properties.displayName for the recommendation. DisplayName func(common.Recommendation) string + + // ReservedResourceType is the properties.reservedResourceType value. + ReservedResourceType string } // BuildPurchaseBody parses rec.Term, builds the calculatePrice/purchase body diff --git a/providers/azure/services/internal/reservations/reservationstest/capture.go b/providers/azure/services/internal/reservations/reservationstest/capture.go index 685cc756..cbeba983 100644 --- a/providers/azure/services/internal/reservations/reservationstest/capture.go +++ b/providers/azure/services/internal/reservations/reservationstest/capture.go @@ -18,14 +18,17 @@ import ( // Capture is an HTTP client that records the calculatePrice request body and // answers every call with a minimal success response. It performs no network IO. type Capture struct { - mu sync.Mutex - Calls int - BodyBytes []byte - // Requests lists every request received, in order. - Requests []Request // FailWhen, when set, is consulted after the request is recorded. A // non-nil return is returned from Do as the transport error. FailWhen func(*Request) error + + BodyBytes []byte + + // Requests lists every request received, in order. + Requests []Request + + Calls int + mu sync.Mutex } // Request is one recorded HTTP request. @@ -98,10 +101,11 @@ func (c *Capture) BodyWithoutDisplayName() ([]byte, error) { // CountingCred is an azcore.TokenCredential that counts GetToken calls and can // fail on demand. type CountingCred struct { - mu sync.Mutex - Calls int // Err, when non-nil, is returned from every GetToken call. Err error + + Calls int + mu sync.Mutex } var _ azcore.TokenCredential = (*CountingCred)(nil) diff --git a/providers/azure/services/managedredis/client.go b/providers/azure/services/managedredis/client.go index e258fcea..95551369 100644 --- a/providers/azure/services/managedredis/client.go +++ b/providers/azure/services/managedredis/client.go @@ -63,12 +63,12 @@ type RedisCachesPager interface { // provider-dispatch level. type Client struct { cred azcore.TokenCredential - subscriptionID string - region string httpClient HTTPClient recommendationsPager RecommendationsPager reservationsPager ReservationsDetailsPager redisCachesPager RedisCachesPager + subscriptionID string + region string } // recommendationsListArgs builds the (scope, options) for the Consumption @@ -495,9 +495,9 @@ func (c *Client) commonSKUs() []string { // RedisPricing holds pricing details for a given Redis Cache SKU. type RedisPricing struct { + Currency string HourlyRate float64 ReservationPrice float64 - Currency string } func (c *Client) getRedisPricing(ctx context.Context, sku, region string, termYears int) (*RedisPricing, error) { diff --git a/providers/azure/services/savingsplans/benefit_recommendations.go b/providers/azure/services/savingsplans/benefit_recommendations.go index c0bf2f31..1741ac8a 100644 --- a/providers/azure/services/savingsplans/benefit_recommendations.go +++ b/providers/azure/services/savingsplans/benefit_recommendations.go @@ -50,25 +50,25 @@ const ( ) type benefitRecsPage struct { - Value []benefitRecommendation `json:"value"` NextLink string `json:"nextLink"` + Value []benefitRecommendation `json:"value"` } type benefitRecommendation struct { + Properties *benefitRecommendationProps `json:"properties"` ID string `json:"id"` Kind string `json:"kind"` - Properties *benefitRecommendationProps `json:"properties"` } type benefitRecommendationProps struct { + TotalHours *int32 `json:"totalHours"` + CostWithoutBenefit *float64 `json:"costWithoutBenefit"` + RecommendationDetails *benefitRecommendDetls `json:"recommendationDetails"` ArmSkuName string `json:"armSkuName"` Term string `json:"term"` Scope string `json:"scope"` CommitmentGranularity string `json:"commitmentGranularity"` CurrencyCode string `json:"currencyCode"` - TotalHours *int32 `json:"totalHours"` - CostWithoutBenefit *float64 `json:"costWithoutBenefit"` - RecommendationDetails *benefitRecommendDetls `json:"recommendationDetails"` } type benefitRecommendDetls struct { diff --git a/providers/azure/services/savingsplans/client.go b/providers/azure/services/savingsplans/client.go index beb53986..7c85790f 100644 --- a/providers/azure/services/savingsplans/client.go +++ b/providers/azure/services/savingsplans/client.go @@ -62,7 +62,7 @@ type HTTPClient interface { } // Client handles Azure Savings Plans. -type Client struct { +type Client struct { //nolint:govet // fieldalignment: file owned by open PR #491 cred azcore.TokenCredential subscriptionID string region string diff --git a/providers/azure/services/search/client.go b/providers/azure/services/search/client.go index 78b38cdb..f00ecde0 100644 --- a/providers/azure/services/search/client.go +++ b/providers/azure/services/search/client.go @@ -54,12 +54,12 @@ type ServicesPager interface { // Client handles Azure Cognitive Search Reserved Capacity. type Client struct { cred azcore.TokenCredential - subscriptionID string - region string httpClient HTTPClient recommendationsPager RecommendationsPager reservationsPager ReservationsDetailsPager searchServicesPager ServicesPager + subscriptionID string + region string } // NewClient creates a new Azure Search client. @@ -116,8 +116,8 @@ func (c *Client) GetRegion() string { // AzureRetailPrice represents pricing information from Azure Retail Prices API. type AzureRetailPrice struct { - Items []pricing.RetailPriceItem `json:"Items"` NextPageLink string `json:"NextPageLink"` + Items []pricing.RetailPriceItem `json:"Items"` Count int `json:"Count"` } @@ -428,9 +428,9 @@ func (c *Client) getCommonSKUs() []string { // Pricing contains pricing information for Azure Search. type Pricing struct { + Currency string HourlyRate float64 ReservationPrice float64 - Currency string } var searchDisplaySKUs = map[string]string{ diff --git a/providers/azure/services/synapse/client.go b/providers/azure/services/synapse/client.go index b4a03b3c..a56f317a 100644 --- a/providers/azure/services/synapse/client.go +++ b/providers/azure/services/synapse/client.go @@ -58,11 +58,11 @@ type ReservationsDetailsPager interface { // Client handles Azure Synapse Analytics Reserved Capacity. type Client struct { cred azcore.TokenCredential - subscriptionID string - region string httpClient HTTPClient recommendationsPager RecommendationsPager reservationsPager ReservationsDetailsPager + subscriptionID string + region string } // NewClient creates a new Azure Synapse Analytics client. @@ -116,8 +116,6 @@ func (c *Client) GetRegion() string { // Synapse Analytics. Private pricing selection uses the shared item shape. type RetailPriceItem struct { CurrencyCode string `json:"currencyCode"` - RetailPrice float64 `json:"retailPrice"` - UnitPrice float64 `json:"unitPrice"` ArmRegionName string `json:"armRegionName"` ProductName string `json:"productName"` ServiceName string `json:"serviceName"` @@ -126,6 +124,8 @@ type RetailPriceItem struct { SKUName string `json:"skuName"` ReservationTerm string `json:"reservationTerm"` Type string `json:"type"` + RetailPrice float64 `json:"retailPrice"` + UnitPrice float64 `json:"unitPrice"` } // GetRecommendations retrieves Synapse reservation recommendations from the @@ -440,9 +440,9 @@ func (c *Client) GetValidResourceTypes(_ context.Context) ([]string, error) { // Pricing holds pricing information for Synapse Analytics. type Pricing struct { + Currency string HourlyRate float64 ReservationPrice float64 - Currency string } // getSynapsePricing fetches pricing from the Azure Retail Prices API.