diff --git a/providers/aws/internal/offeringprice/offeringprice.go b/providers/aws/internal/offeringprice/offeringprice.go index 8cb70d92..a278a94c 100644 --- a/providers/aws/internal/offeringprice/offeringprice.go +++ b/providers/aws/internal/offeringprice/offeringprice.go @@ -16,22 +16,27 @@ const hourlyFrequency = "Hourly" // Charge is one recurring charge of an offering. type Charge struct { - Amount float64 Frequency string + Amount float64 } // Input is the price data of one offering. type Input struct { // Service names the AWS service in error messages and term parsing. Service string + // Term is the requested term (rec.Term); it must match the offering. Term string + + Charges []Charge + // FixedPrice is the upfront price per reservation. FixedPrice float64 + // UsagePrice is the legacy per-hour price per reservation. It is used // only when the recurring charges sum to zero. UsagePrice float64 - Charges []Charge + // DurationSeconds is the offering's own duration; 0 means missing. DurationSeconds int64 } diff --git a/providers/aws/internal/offeringprice/offeringprice_test.go b/providers/aws/internal/offeringprice/offeringprice_test.go index 41056a07..15dfb68c 100644 --- a/providers/aws/internal/offeringprice/offeringprice_test.go +++ b/providers/aws/internal/offeringprice/offeringprice_test.go @@ -20,11 +20,11 @@ func TestPrice(t *testing.T) { err string }{ {"all upfront", Input{Term: "1yr", FixedPrice: 1000, DurationSeconds: one}, 1000, 8760, 0, ""}, - {"hourly charge, usage 0", Input{Term: "1yr", Charges: []Charge{{0.05, "Hourly"}}, DurationSeconds: one}, 438, 8760, 0.05, ""}, + {"hourly charge, usage 0", Input{Term: "1yr", Charges: []Charge{{Amount: 0.05, Frequency: "Hourly"}}, DurationSeconds: one}, 438, 8760, 0.05, ""}, {"usage only, no charges", Input{Term: "1yr", UsagePrice: 0.05, DurationSeconds: one}, 438, 8760, 0.05, ""}, - {"partial 3yr", Input{Term: "3yr", FixedPrice: 500, Charges: []Charge{{0.03, "Hourly"}}, DurationSeconds: three}, 500 + 0.03*26280, 26280, 0.03, ""}, - {"both nonzero", Input{Term: "1yr", UsagePrice: 0.05, Charges: []Charge{{0.05, "Hourly"}}, DurationSeconds: one}, 0, 0, 0, "refusing to guess"}, - {"non hourly", Input{Term: "1yr", Charges: []Charge{{1, "Monthly"}}, DurationSeconds: one}, 0, 0, 0, "only \"Hourly\""}, + {"partial 3yr", Input{Term: "3yr", FixedPrice: 500, Charges: []Charge{{Amount: 0.03, Frequency: "Hourly"}}, DurationSeconds: three}, 500 + 0.03*26280, 26280, 0.03, ""}, + {"both nonzero", Input{Term: "1yr", UsagePrice: 0.05, Charges: []Charge{{Amount: 0.05, Frequency: "Hourly"}}, DurationSeconds: one}, 0, 0, 0, "refusing to guess"}, + {"non hourly", Input{Term: "1yr", Charges: []Charge{{Amount: 1, Frequency: "Monthly"}}, DurationSeconds: one}, 0, 0, 0, "only \"Hourly\""}, {"missing duration", Input{Term: "1yr"}, 0, 0, 0, "no duration"}, {"term mismatch", Input{Term: "3yr", DurationSeconds: one}, 0, 0, 0, "does not match"}, {"negative fixed", Input{Term: "1yr", FixedPrice: -1, DurationSeconds: one}, 0, 0, 0, "invalid FixedPrice"}, diff --git a/providers/aws/internal/purchasecfg/purchasetest/purchasetest.go b/providers/aws/internal/purchasecfg/purchasetest/purchasetest.go index 6bb92c67..aa0005da 100644 --- a/providers/aws/internal/purchasecfg/purchasetest/purchasetest.go +++ b/providers/aws/internal/purchasecfg/purchasetest/purchasetest.go @@ -56,18 +56,18 @@ func RunEmptyToken(t *testing.T, call Call, alreadyExists error) { lost := &net.OpError{Op: "read", Err: errors.New("connection reset")} for _, tc := range []struct { - name, token string apiErr error + name, token string emptyResp bool unknown bool }{ - {"transport error", "", lost, false, true}, - {"5xx", "", RespErr(http.StatusInternalServerError, &smithy.GenericAPIError{Code: "InternalFailure"}), false, true}, - {"empty response", "", nil, true, true}, - {"400 AlreadyExists", "", RespErr(http.StatusBadRequest, alreadyExists), false, false}, - {"throttle", "", RespErr(http.StatusServiceUnavailable, &smithy.GenericAPIError{Code: "Throttling"}), false, false}, - {"token: transport error", "exec-token", lost, false, false}, - {"token: empty response", "exec-token", nil, true, false}, + {apiErr: lost, name: "transport error", token: "", emptyResp: false, unknown: true}, + {apiErr: RespErr(http.StatusInternalServerError, &smithy.GenericAPIError{Code: "InternalFailure"}), name: "5xx", token: "", emptyResp: false, unknown: true}, + {apiErr: nil, name: "empty response", token: "", emptyResp: true, unknown: true}, + {apiErr: RespErr(http.StatusBadRequest, alreadyExists), name: "400 AlreadyExists", token: "", emptyResp: false, unknown: false}, + {apiErr: RespErr(http.StatusServiceUnavailable, &smithy.GenericAPIError{Code: "Throttling"}), name: "throttle", token: "", emptyResp: false, unknown: false}, + {apiErr: lost, name: "token: transport error", token: "exec-token", emptyResp: false, unknown: false}, + {apiErr: nil, name: "token: empty response", token: "exec-token", emptyResp: true, unknown: false}, } { t.Run(tc.name, func(t *testing.T) { _, result, err := call(t, tc.token, tc.apiErr, tc.emptyResp) diff --git a/providers/aws/provider.go b/providers/aws/provider.go index 0654d1f0..f8df8a52 100644 --- a/providers/aws/provider.go +++ b/providers/aws/provider.go @@ -81,16 +81,16 @@ func (r *realOrganizationsPaginator) NextPage(ctx context.Context, optFns ...fun // Provider implements the provider.Provider interface for AWS. type Provider struct { - cfg aws.Config - cfgOnce sync.Once cfgErr error // non-nil if config loading failed - profile string - region string configLoader ConfigLoader stsClient STSClient ec2Client EC2Client orgPaginator OrganizationsPaginator credentialsProvider aws.CredentialsProvider // optional override for per-account execution + profile string + region string + cfg aws.Config + cfgOnce sync.Once } // NewAWSProvider creates a new AWS provider instance. diff --git a/providers/aws/recommendations/client.go b/providers/aws/recommendations/client.go index 3007a1a8..c6c5f254 100644 --- a/providers/aws/recommendations/client.go +++ b/providers/aws/recommendations/client.go @@ -62,14 +62,14 @@ type Client struct { region string + // recLookbackPeriod is forwarded to GetReservationPurchaseRecommendation + // as LookbackPeriodInDays. Defaults to "7d" when empty. + recLookbackPeriod string + // skuCatalog caches the per-instance-type vCPU/memory catalog, fetched // lazily once per Client lifetime via sync.Once (one DescribeInstanceTypes // fan-out per scheduler tick). skuCatalog skuCatalog - - // recLookbackPeriod is forwarded to GetReservationPurchaseRecommendation - // as LookbackPeriodInDays. Defaults to "7d" when empty. - recLookbackPeriod string } // NewClient creates a new recommendations client. diff --git a/providers/aws/recommendations/collection_error.go b/providers/aws/recommendations/collection_error.go index 55ef3297..f7bc6530 100644 --- a/providers/aws/recommendations/collection_error.go +++ b/providers/aws/recommendations/collection_error.go @@ -25,9 +25,9 @@ func requireUSD(code string) error { // IncompleteRecommendationsError accompanies survivors of an incomplete collection. // Each cause represents one rejected detail or one failed collection scope. type IncompleteRecommendationsError struct { + Causes []error FailedDetails int FailedScopes int - Causes []error } func (e *IncompleteRecommendationsError) Error() string { diff --git a/providers/aws/recommendations/parser_sp.go b/providers/aws/recommendations/parser_sp.go index ff254e18..457a177d 100644 --- a/providers/aws/recommendations/parser_sp.go +++ b/providers/aws/recommendations/parser_sp.go @@ -176,16 +176,16 @@ type spNumbers struct { func parseSPNumbers(detail *types.SavingsPlansPurchaseRecommendationDetail) (spNumbers, error) { var n spNumbers for _, f := range []struct { - name string src *string dst *float64 + name string }{ - {"HourlyCommitmentToPurchase", detail.HourlyCommitmentToPurchase, &n.hourlyCommitment}, - {"EstimatedMonthlySavingsAmount", detail.EstimatedMonthlySavingsAmount, &n.monthlySavings}, - {"UpfrontCost", detail.UpfrontCost, &n.upfrontCost}, - {"EstimatedSavingsPercentage", detail.EstimatedSavingsPercentage, &n.savingsPercent}, - {"EstimatedAverageUtilization", detail.EstimatedAverageUtilization, &n.recommendedUtilization}, - {"CurrentAverageHourlyOnDemandSpend", detail.CurrentAverageHourlyOnDemandSpend, &n.hourlyOnDemand}, + {src: detail.HourlyCommitmentToPurchase, dst: &n.hourlyCommitment, name: "HourlyCommitmentToPurchase"}, + {src: detail.EstimatedMonthlySavingsAmount, dst: &n.monthlySavings, name: "EstimatedMonthlySavingsAmount"}, + {src: detail.UpfrontCost, dst: &n.upfrontCost, name: "UpfrontCost"}, + {src: detail.EstimatedSavingsPercentage, dst: &n.savingsPercent, name: "EstimatedSavingsPercentage"}, + {src: detail.EstimatedAverageUtilization, dst: &n.recommendedUtilization, name: "EstimatedAverageUtilization"}, + {src: detail.CurrentAverageHourlyOnDemandSpend, dst: &n.hourlyOnDemand, name: "CurrentAverageHourlyOnDemandSpend"}, } { v, err := parseOptionalFloat(f.name, f.src) if err != nil { diff --git a/providers/aws/recommendations/usage_history.go b/providers/aws/recommendations/usage_history.go index ae270b5f..8691c2d8 100644 --- a/providers/aws/recommendations/usage_history.go +++ b/providers/aws/recommendations/usage_history.go @@ -143,9 +143,9 @@ type regionKey struct{ service, region string } // regionBucket holds the distinct resource types requested for one regionKey // (in first-seen order) and the recommendation indices for each type. type regionBucket struct { + recsByType map[string][]int key regionKey types []string - recsByType map[string][]int } // groupRecsByRegion builds an ordered list of (service, region) buckets, each diff --git a/providers/aws/recommendations/utilization.go b/providers/aws/recommendations/utilization.go index df34fbfe..aa4ac1a6 100644 --- a/providers/aws/recommendations/utilization.go +++ b/providers/aws/recommendations/utilization.go @@ -180,13 +180,13 @@ func aggregateUtilizationDetail(agg map[string]*riAccumulator, detail types.Rese // parseUtilizationHours strictly parses the three required hours fields. func parseUtilizationHours(u *types.ReservationAggregates) (purchased, actual, unused float64, err error) { for _, f := range []struct { - name string src *string dst *float64 + name string }{ - {"PurchasedHours", u.PurchasedHours, &purchased}, - {"TotalActualHours", u.TotalActualHours, &actual}, - {"UnusedHours", u.UnusedHours, &unused}, + {src: u.PurchasedHours, dst: &purchased, name: "PurchasedHours"}, + {src: u.TotalActualHours, dst: &actual, name: "TotalActualHours"}, + {src: u.UnusedHours, dst: &unused, name: "UnusedHours"}, } { if f.src == nil { return 0, 0, 0, fmt.Errorf("%s is missing while other utilization hours are present", f.name) diff --git a/providers/aws/services/ec2/client.go b/providers/aws/services/ec2/client.go index e812f83f..6b8219cf 100644 --- a/providers/aws/services/ec2/client.go +++ b/providers/aws/services/ec2/client.go @@ -385,9 +385,9 @@ type ec2OfferingQuery struct { tenancy types.Tenancy scope types.Scope availabilityZone string // set only for zonal scope - duration int64 wantOfferingType types.OfferingTypeValues offeringClass types.OfferingClassType + duration int64 } // resolveOfferingClassType maps the GlobalConfig string value to the AWS SDK @@ -761,32 +761,36 @@ func (c *Client) getDurationValue(term string) (int64, error) { // ConvertibleRI represents an active convertible Reserved Instance. type ConvertibleRI struct { - ReservedInstanceID string `json:"reserved_instance_id"` - InstanceType string `json:"instance_type"` - AvailabilityZone string `json:"availability_zone"` - InstanceCount int32 `json:"instance_count"` - Start time.Time `json:"start"` - End time.Time `json:"end"` - OfferingType string `json:"offering_type"` - FixedPrice float64 `json:"fixed_price"` - UsagePrice float64 `json:"usage_price"` - State string `json:"state"` - NormalizationFactor float64 `json:"normalization_factor"` - ProductDescription string `json:"product_description"` - InstanceTenancy string `json:"instance_tenancy"` - Scope string `json:"scope"` - Duration int64 `json:"duration"` + Start time.Time `json:"start"` + End time.Time `json:"end"` + ReservedInstanceID string `json:"reserved_instance_id"` + InstanceType string `json:"instance_type"` + AvailabilityZone string `json:"availability_zone"` + OfferingType string `json:"offering_type"` + State string `json:"state"` + ProductDescription string `json:"product_description"` + InstanceTenancy string `json:"instance_tenancy"` + Scope string `json:"scope"` + // CurrencyCode is the ISO-4217 currency that FixedPrice / UsagePrice // are denominated in (typically "USD"). Plumbed through to // exchange.RIInfo so the dollar-units pre-filter on cross-family // alternatives can refuse comparisons across currencies. CurrencyCode string `json:"currency_code,omitempty"` + + FixedPrice float64 `json:"fixed_price"` + UsagePrice float64 `json:"usage_price"` + NormalizationFactor float64 `json:"normalization_factor"` + Duration int64 `json:"duration"` + // RecurringHourlyAmount is the per-hour recurring charge (sum across // the SDK's RecurringCharges slice) — this is non-zero for // no-upfront and partial-upfront RIs where some of the cost is // billed hourly rather than as the upfront FixedPrice. Used by the // exchange-package dollar-units calculation. RecurringHourlyAmount float64 `json:"recurring_hourly_amount,omitempty"` + + InstanceCount int32 `json:"instance_count"` } // ListConvertibleReservedInstances returns all active convertible RIs in the region. @@ -971,11 +975,11 @@ type TargetOffering struct { InstanceType string `json:"instance_type"` OfferingType string `json:"offering_type"` ProductDescription string `json:"product_description"` + CurrencyCode string `json:"currency_code"` + Scope string `json:"scope"` Duration int64 `json:"duration"` FixedPrice float64 `json:"fixed_price"` UsagePrice float64 `json:"usage_price"` - CurrencyCode string `json:"currency_code"` - Scope string `json:"scope"` NormalizationFactor float64 `json:"normalization_factor"` } @@ -986,8 +990,8 @@ type ListTargetOfferingsParams struct { Tenancy string Scope string AvailabilityZone string - Duration int64 OfferingType string + Duration int64 } // maxTargetOfferingPages caps the pagination walk for ListTargetOfferings. diff --git a/providers/aws/services/ec2/offering_match.go b/providers/aws/services/ec2/offering_match.go index a6190b53..bf20a51d 100644 --- a/providers/aws/services/ec2/offering_match.go +++ b/providers/aws/services/ec2/offering_match.go @@ -17,10 +17,10 @@ type offeringWant struct { productDesc string tenancy types.Tenancy scope types.Scope - duration int64 class types.OfferingClassType offeringType types.OfferingTypeValues az string + duration int64 } // want maps the lookup query onto the fields offeringMismatch re-checks. @@ -44,31 +44,31 @@ const vpcProductSuffix = " (Amazon VPC)" // field is a mismatch); instance type, scope and product are compared only // when AWS populated the offering field. var offeringChecks = []struct { - reason string mismatch func(o types.ReservedInstancesOffering, w offeringWant) bool + reason string }{ - {"zone", func(o types.ReservedInstancesOffering, w offeringWant) bool { + {reason: "zone", mismatch: func(o types.ReservedInstancesOffering, w offeringWant) bool { return !offeringInZone(o, w.az) }}, - {"class", func(o types.ReservedInstancesOffering, w offeringWant) bool { + {reason: "class", mismatch: func(o types.ReservedInstancesOffering, w offeringWant) bool { return w.class != "" && o.OfferingClass != w.class }}, - {"tenancy", func(o types.ReservedInstancesOffering, w offeringWant) bool { + {reason: "tenancy", mismatch: func(o types.ReservedInstancesOffering, w offeringWant) bool { return w.tenancy != "" && o.InstanceTenancy != w.tenancy }}, - {"duration", func(o types.ReservedInstancesOffering, w offeringWant) bool { + {reason: "duration", mismatch: func(o types.ReservedInstancesOffering, w offeringWant) bool { return w.duration != 0 && aws.ToInt64(o.Duration) != w.duration }}, - {"offering-type", func(o types.ReservedInstancesOffering, w offeringWant) bool { + {reason: "offering-type", mismatch: func(o types.ReservedInstancesOffering, w offeringWant) bool { return w.offeringType != "" && o.OfferingType != w.offeringType }}, - {"instance-type", func(o types.ReservedInstancesOffering, w offeringWant) bool { + {reason: "instance-type", mismatch: func(o types.ReservedInstancesOffering, w offeringWant) bool { return w.instanceType != "" && o.InstanceType != "" && string(o.InstanceType) != w.instanceType }}, - {"scope", func(o types.ReservedInstancesOffering, w offeringWant) bool { + {reason: "scope", mismatch: func(o types.ReservedInstancesOffering, w offeringWant) bool { return w.scope != "" && o.Scope != "" && o.Scope != w.scope }}, - {"product", func(o types.ReservedInstancesOffering, w offeringWant) bool { + {reason: "product", mismatch: func(o types.ReservedInstancesOffering, w offeringWant) bool { got, want := string(o.ProductDescription), w.productDesc if got == "" || want == "" { return false diff --git a/providers/aws/services/rds/client.go b/providers/aws/services/rds/client.go index db9a95b4..3bec057c 100644 --- a/providers/aws/services/rds/client.go +++ b/providers/aws/services/rds/client.go @@ -330,8 +330,8 @@ func (c *Client) findOfferingID(ctx context.Context, rec common.Recommendation, // rdsOfferingPageResult holds the outcome of a single DescribeReservedDBInstancesOfferings page. type rdsOfferingPageResult struct { - id string // non-empty when an exact match was found on this page marker *string // pagination cursor for the next page; nil when exhausted + id string // non-empty when an exact match was found on this page } // fetchRDSOfferingPage calls DescribeReservedDBInstancesOfferings for one page and @@ -519,9 +519,9 @@ func (c *Client) offeringDuration(term string) (duration string, seconds int64, type rdsOfferingKey struct { class string engine string - multiAZ bool - duration int64 offeringType string + duration int64 + multiAZ bool } func (k rdsOfferingKey) matches(o *types.ReservedDBInstancesOffering) bool {