diff --git a/providers/aws/recommendations/client.go b/providers/aws/recommendations/client.go index 880abf529..5bd968af0 100644 --- a/providers/aws/recommendations/client.go +++ b/providers/aws/recommendations/client.go @@ -8,6 +8,7 @@ import ( "github.com/aws/aws-sdk-go-v2/aws" "github.com/aws/aws-sdk-go-v2/service/costexplorer" "github.com/aws/aws-sdk-go-v2/service/costexplorer/types" + awsec2 "github.com/aws/aws-sdk-go-v2/service/ec2" "golang.org/x/sync/errgroup" "github.com/LeanerCloud/CUDly/pkg/common" @@ -34,6 +35,20 @@ type Client struct { costExplorerClient CostExplorerAPI region string rateLimiter *RateLimiter + + // ec2API is the EC2 client used to build the DescribeInstanceTypes paginator. + // Populated by NewClient from aws.Config; nil when created via NewClientWithAPI. + ec2API DescribeInstanceTypesAPI + + // instanceTypePagerFactory creates a new InstanceTypePager on demand. + // Set by NewClient to wrap ec2API; overridable via SetInstanceTypePagerFactory + // for hermetic tests. When nil, instanceTypeLookup returns (0,0). + instanceTypePagerFactory func() InstanceTypePager + + // skuCatalog caches the per-instance-type vCPU/memory catalogue, fetched + // lazily once per Client lifetime via sync.Once (one DescribeInstanceTypes + // fan-out per scheduler tick). + skuCatalog skuCatalog } // NewClient creates a new recommendations client @@ -43,10 +58,17 @@ func NewClient(cfg aws.Config) *Client { ceConfig.Region = "us-east-1" ceConfig.BaseEndpoint = aws.String("https://ce.us-east-1.amazonaws.com") + ec2Client := awsec2.NewFromConfig(cfg) return &Client{ costExplorerClient: costexplorer.NewFromConfig(ceConfig), region: cfg.Region, rateLimiter: NewRateLimiter(), + ec2API: ec2Client, + // Factory wraps the EC2 client so the paginator is created lazily + // on the first EC2 recommendation parse (not at construction time). + instanceTypePagerFactory: func() InstanceTypePager { + return awsec2.NewDescribeInstanceTypesPaginator(ec2Client, &awsec2.DescribeInstanceTypesInput{}) + }, } } @@ -56,7 +78,29 @@ func NewClientWithAPI(api CostExplorerAPI, region string) *Client { costExplorerClient: api, region: region, rateLimiter: NewRateLimiter(), + // ec2API left nil: instanceTypeLookup falls back to VCPU=0/MemoryGB=0 + // unless the caller sets instanceTypePagerFactory. + } +} + +// SetInstanceTypePagerFactory injects a pager factory for the instance-type +// SKU catalogue. Must be called before the first GetRecommendations call. +// Intended for tests that need to verify the one-fetch-per-lifetime invariant +// without hitting AWS. +func (c *Client) SetInstanceTypePagerFactory(f func() InstanceTypePager) { + c.instanceTypePagerFactory = f +} + +// instanceTypeLookup returns the cached SKU entry for instanceType. +// On the first call the catalogue is built by calling the pager factory. +// ok=false when no factory is configured, the catalogue fetch failed, or +// the instance type was not in the catalogue — the caller falls back to +// VCPU=0/MemoryGB=0 (graceful-degradation contract from Azure PR #810). +func (c *Client) instanceTypeLookup(ctx context.Context, instanceType string) (instanceTypeSKUEntry, bool) { + if c.instanceTypePagerFactory == nil { + return instanceTypeSKUEntry{}, false } + return c.skuCatalog.lookup(ctx, instanceType, c.instanceTypePagerFactory) } // GetRecommendations fetches Reserved Instance recommendations for any service @@ -83,7 +127,7 @@ func (c *Client) GetRecommendations(ctx context.Context, params common.Recommend return nil, err } - return c.parseRecommendations(allRecs, params) + return c.parseRecommendations(ctx, allRecs, params) } // fetchRIAllPages paginates over all pages of RI recommendations for a single diff --git a/providers/aws/recommendations/parser_ri.go b/providers/aws/recommendations/parser_ri.go index b00825481..35aa90df9 100644 --- a/providers/aws/recommendations/parser_ri.go +++ b/providers/aws/recommendations/parser_ri.go @@ -1,6 +1,7 @@ package recommendations import ( + "context" "fmt" "log" "math" @@ -14,12 +15,12 @@ import ( ) // parseRecommendations converts AWS recommendations to common.Recommendation format -func (c *Client) parseRecommendations(awsRecs []types.ReservationPurchaseRecommendation, params common.RecommendationParams) ([]common.Recommendation, error) { +func (c *Client) parseRecommendations(ctx context.Context, awsRecs []types.ReservationPurchaseRecommendation, params common.RecommendationParams) ([]common.Recommendation, error) { var recommendations []common.Recommendation for _, awsRec := range awsRecs { for i, details := range awsRec.RecommendationDetails { - rec, err := c.parseRecommendationDetail(&details, params) + rec, err := c.parseRecommendationDetail(ctx, &details, params) if err != nil { fmt.Printf("Warning: Failed to parse recommendation detail %d: %v\n", i, err) continue @@ -35,7 +36,7 @@ func (c *Client) parseRecommendations(awsRecs []types.ReservationPurchaseRecomme } // parseRecommendationDetail converts a single AWS recommendation detail -func (c *Client) parseRecommendationDetail(details *types.ReservationPurchaseRecommendationDetail, params common.RecommendationParams) (*common.Recommendation, error) { +func (c *Client) parseRecommendationDetail(ctx context.Context, details *types.ReservationPurchaseRecommendationDetail, params common.RecommendationParams) (*common.Recommendation, error) { rec := &common.Recommendation{ Provider: common.ProviderAWS, Service: params.Service, @@ -74,7 +75,7 @@ func (c *Client) parseRecommendationDetail(details *types.ReservationPurchaseRec c.parseRIUtilizationSignals(rec, details) // Parse service-specific details - if err := c.parseServiceSpecificDetails(rec, details, params.Service); err != nil { + if err := c.parseServiceSpecificDetails(ctx, rec, details, params.Service); err != nil { return nil, err } @@ -178,10 +179,10 @@ func (c *Client) parseAWSCostDetails(rec *common.Recommendation, details *types. } // serviceParserFunc defines the signature for service-specific parsers -type serviceParserFunc func(*common.Recommendation, *types.ReservationPurchaseRecommendationDetail) error +type serviceParserFunc func(context.Context, *common.Recommendation, *types.ReservationPurchaseRecommendationDetail) error // parseServiceSpecificDetails routes to the appropriate service parser -func (c *Client) parseServiceSpecificDetails(rec *common.Recommendation, details *types.ReservationPurchaseRecommendationDetail, service common.ServiceType) error { +func (c *Client) parseServiceSpecificDetails(ctx context.Context, rec *common.Recommendation, details *types.ReservationPurchaseRecommendationDetail, service common.ServiceType) error { // Map of service types to their parser functions serviceParsers := map[common.ServiceType]serviceParserFunc{ common.ServiceRDS: c.parseRDSDetails, @@ -202,5 +203,5 @@ func (c *Client) parseServiceSpecificDetails(rec *common.Recommendation, details return fmt.Errorf("unsupported service: %s", service) } - return parser(rec, details) + return parser(ctx, rec, details) } diff --git a/providers/aws/recommendations/parser_ri_test.go b/providers/aws/recommendations/parser_ri_test.go index 05fce3121..4657fcb0c 100644 --- a/providers/aws/recommendations/parser_ri_test.go +++ b/providers/aws/recommendations/parser_ri_test.go @@ -1,6 +1,7 @@ package recommendations import ( + "context" "testing" "github.com/aws/aws-sdk-go-v2/aws" @@ -184,7 +185,7 @@ func TestParseRecommendationDetail_UnsupportedService(t *testing.T) { LookbackPeriod: "7d", } - rec, err := client.parseRecommendationDetail(details, params) + rec, err := client.parseRecommendationDetail(context.Background(), details, params) assert.Error(t, err) assert.Nil(t, rec) @@ -207,7 +208,7 @@ func TestParseRecommendationDetail_MissingQuantity(t *testing.T) { LookbackPeriod: "7d", } - rec, err := client.parseRecommendationDetail(details, params) + rec, err := client.parseRecommendationDetail(context.Background(), details, params) assert.Error(t, err) assert.Nil(t, rec) @@ -241,7 +242,7 @@ func TestParseRecommendationDetail_WithAccountAndCosts(t *testing.T) { LookbackPeriod: "7d", } - rec, err := client.parseRecommendationDetail(details, params) + rec, err := client.parseRecommendationDetail(context.Background(), details, params) require.NoError(t, err) require.NotNil(t, rec) @@ -300,7 +301,7 @@ func TestParseRecommendations(t *testing.T) { LookbackPeriod: "7d", } - recs, err := client.parseRecommendations(awsRecs, params) + recs, err := client.parseRecommendations(context.Background(), awsRecs, params) require.NoError(t, err) assert.Len(t, recs, 2) @@ -371,7 +372,7 @@ func TestParseRecommendations_SkipsInvalidDetails(t *testing.T) { LookbackPeriod: "7d", } - recs, err := client.parseRecommendations(awsRecs, params) + recs, err := client.parseRecommendations(context.Background(), awsRecs, params) require.NoError(t, err) // Should have 2 valid recommendations, skipping the invalid one @@ -390,7 +391,7 @@ func TestParseRecommendations_EmptyInput(t *testing.T) { LookbackPeriod: "7d", } - recs, err := client.parseRecommendations([]types.ReservationPurchaseRecommendation{}, params) + recs, err := client.parseRecommendations(context.Background(), []types.ReservationPurchaseRecommendation{}, params) require.NoError(t, err) assert.Empty(t, recs) diff --git a/providers/aws/recommendations/parser_services.go b/providers/aws/recommendations/parser_services.go index 34448feb2..cb456c58e 100644 --- a/providers/aws/recommendations/parser_services.go +++ b/providers/aws/recommendations/parser_services.go @@ -1,6 +1,7 @@ package recommendations import ( + "context" "fmt" "strings" @@ -11,7 +12,7 @@ import ( ) // parseRDSDetails extracts RDS-specific details -func (c *Client) parseRDSDetails(rec *common.Recommendation, details *types.ReservationPurchaseRecommendationDetail) error { +func (c *Client) parseRDSDetails(_ context.Context, rec *common.Recommendation, details *types.ReservationPurchaseRecommendationDetail) error { if details.InstanceDetails == nil || details.InstanceDetails.RDSInstanceDetails == nil { return fmt.Errorf("RDS instance details not found") } @@ -43,7 +44,7 @@ func (c *Client) parseRDSDetails(rec *common.Recommendation, details *types.Rese } // parseElastiCacheDetails extracts ElastiCache-specific details -func (c *Client) parseElastiCacheDetails(rec *common.Recommendation, details *types.ReservationPurchaseRecommendationDetail) error { +func (c *Client) parseElastiCacheDetails(_ context.Context, rec *common.Recommendation, details *types.ReservationPurchaseRecommendationDetail) error { if details.InstanceDetails == nil || details.InstanceDetails.ElastiCacheInstanceDetails == nil { return fmt.Errorf("ElastiCache instance details not found") } @@ -66,8 +67,48 @@ func (c *Client) parseElastiCacheDetails(rec *common.Recommendation, details *ty return nil } -// parseEC2Details extracts EC2-specific details -func (c *Client) parseEC2Details(rec *common.Recommendation, details *types.ReservationPurchaseRecommendationDetail) error { +// resolveEC2Tenancy maps a Cost Explorer tenancy value to the EC2 RI API +// tenancy string. CE uses "shared" for the default tenancy; "dedicated" maps +// directly. Any nil or unrecognised value is treated as default. +func resolveEC2Tenancy(tenancy *string) string { + if tenancy != nil && *tenancy == "dedicated" { + return string(ec2types.TenancyDedicated) + } + return string(ec2types.TenancyDefault) +} + +// resolveEC2Scope maps a Cost Explorer availability zone value to the EC2 RI +// API scope string. A non-empty AZ means AZ scope; otherwise region scope. +func resolveEC2Scope(az *string) string { + if az != nil && *az != "" { + return string(ec2types.ScopeAvailabilityZone) + } + return string(ec2types.ScopeRegional) +} + +// enrichFromCatalogue populates VCPU and MemoryGB on ec2Info from the +// lazily-cached DescribeInstanceTypes catalogue. Non-fatal on cache miss. +func (c *Client) enrichFromCatalogue(ctx context.Context, ec2Info *common.ComputeDetails) { + if ec2Info.InstanceType == "" { + return + } + entry, ok := c.instanceTypeLookup(ctx, ec2Info.InstanceType) + if !ok { + return + } + if entry.vCPUs > 0 { + ec2Info.VCPU = entry.vCPUs + } + if entry.memoryGB > 0 { + ec2Info.MemoryGB = entry.memoryGB + } +} + +// parseEC2Details extracts EC2-specific details and enriches the rec with +// vCPU and memory from the lazily-cached DescribeInstanceTypes catalogue. +// If the catalogue fetch failed or the instance type is not found, VCPU +// and MemoryGB remain 0 (the omitempty JSON tags hide them from payloads). +func (c *Client) parseEC2Details(ctx context.Context, rec *common.Recommendation, details *types.ReservationPurchaseRecommendationDetail) error { if details.InstanceDetails == nil || details.InstanceDetails.EC2InstanceDetails == nil { return fmt.Errorf("EC2 instance details not found") } @@ -85,29 +126,16 @@ func (c *Client) parseEC2Details(rec *common.Recommendation, details *types.Rese if ec2Details.Region != nil { rec.Region = normalizeRegionName(*ec2Details.Region) } - // Tenancy: CE returns "shared" for default tenancy; the EC2 RI filter API - // expects "default" (types.TenancyDefault). CE "dedicated" maps directly. - // Any nil or unrecognised value is treated as default. - if ec2Details.Tenancy != nil && *ec2Details.Tenancy == "dedicated" { - ec2Info.Tenancy = string(ec2types.TenancyDedicated) - } else { - ec2Info.Tenancy = string(ec2types.TenancyDefault) - } - - // Scope: the EC2 RI filter API expects "Region" (types.ScopeRegional) or - // "Availability Zone" (types.ScopeAvailabilityZone) - not lowercase/hyphenated. - if ec2Details.AvailabilityZone != nil && *ec2Details.AvailabilityZone != "" { - ec2Info.Scope = string(ec2types.ScopeAvailabilityZone) - } else { - ec2Info.Scope = string(ec2types.ScopeRegional) - } + ec2Info.Tenancy = resolveEC2Tenancy(ec2Details.Tenancy) + ec2Info.Scope = resolveEC2Scope(ec2Details.AvailabilityZone) + c.enrichFromCatalogue(ctx, ec2Info) rec.Details = ec2Info return nil } // parseOpenSearchDetails extracts OpenSearch-specific details -func (c *Client) parseOpenSearchDetails(rec *common.Recommendation, details *types.ReservationPurchaseRecommendationDetail) error { +func (c *Client) parseOpenSearchDetails(_ context.Context, rec *common.Recommendation, details *types.ReservationPurchaseRecommendationDetail) error { if details.InstanceDetails == nil || details.InstanceDetails.ESInstanceDetails == nil { return fmt.Errorf("OpenSearch/Elasticsearch instance details not found") } @@ -134,7 +162,7 @@ func (c *Client) parseOpenSearchDetails(rec *common.Recommendation, details *typ } // parseRedshiftDetails extracts Redshift-specific details -func (c *Client) parseRedshiftDetails(rec *common.Recommendation, details *types.ReservationPurchaseRecommendationDetail) error { +func (c *Client) parseRedshiftDetails(_ context.Context, rec *common.Recommendation, details *types.ReservationPurchaseRecommendationDetail) error { if details.InstanceDetails == nil || details.InstanceDetails.RedshiftInstanceDetails == nil { return fmt.Errorf("Redshift instance details not found") } @@ -172,9 +200,9 @@ func (c *Client) parseRedshiftDetails(rec *common.Recommendation, details *types // If rec.ResourceType is empty, the function returns an error so the // recommendation is skipped loudly (logged by parseRecommendations) rather // than silently substituting a wrong default instance type. -func (c *Client) parseMemoryDBDetails(rec *common.Recommendation, _ *types.ReservationPurchaseRecommendationDetail) error { +func (c *Client) parseMemoryDBDetails(_ context.Context, rec *common.Recommendation, _ *types.ReservationPurchaseRecommendationDetail) error { if rec.ResourceType == "" { - return fmt.Errorf("MemoryDB recommendation has no ResourceType; cannot determine offering — Cost Explorer did not populate instance details") + return fmt.Errorf("MemoryDB recommendation has no ResourceType; cannot determine offering - Cost Explorer did not populate instance details") } rec.Details = &common.CacheDetails{ Engine: "redis", diff --git a/providers/aws/recommendations/parser_services_test.go b/providers/aws/recommendations/parser_services_test.go index 0be67a8a5..e01c2bd94 100644 --- a/providers/aws/recommendations/parser_services_test.go +++ b/providers/aws/recommendations/parser_services_test.go @@ -1,6 +1,7 @@ package recommendations import ( + "context" "testing" "github.com/aws/aws-sdk-go-v2/aws" @@ -104,7 +105,7 @@ func TestParseRDSDetails(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { rec := &common.Recommendation{} - err := client.parseRDSDetails(rec, tt.details) + err := client.parseRDSDetails(context.Background(), rec, tt.details) if tt.expectError { assert.Error(t, err) @@ -181,7 +182,7 @@ func TestParseElastiCacheDetails(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { rec := &common.Recommendation{} - err := client.parseElastiCacheDetails(rec, tt.details) + err := client.parseElastiCacheDetails(context.Background(), rec, tt.details) if tt.expectError { assert.Error(t, err) @@ -334,7 +335,7 @@ func TestParseEC2Details(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { rec := &common.Recommendation{} - err := client.parseEC2Details(rec, tt.details) + err := client.parseEC2Details(context.Background(), rec, tt.details) if tt.expectError { assert.Error(t, err) @@ -408,7 +409,7 @@ func TestParseOpenSearchDetails(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { rec := &common.Recommendation{} - err := client.parseOpenSearchDetails(rec, tt.details) + err := client.parseOpenSearchDetails(context.Background(), rec, tt.details) if tt.expectError { assert.Error(t, err) @@ -491,7 +492,7 @@ func TestParseRedshiftDetails(t *testing.T) { rec := &common.Recommendation{ Count: tt.count, } - err := client.parseRedshiftDetails(rec, tt.details) + err := client.parseRedshiftDetails(context.Background(), rec, tt.details) if tt.expectError { assert.Error(t, err) @@ -511,7 +512,7 @@ func TestParseMemoryDBDetails(t *testing.T) { t.Run("empty ResourceType returns error", func(t *testing.T) { rec := &common.Recommendation{} - err := client.parseMemoryDBDetails(rec, details) + err := client.parseMemoryDBDetails(context.Background(), rec, details) require.Error(t, err, "should fail loudly when ResourceType is empty") assert.Contains(t, err.Error(), "ResourceType") assert.Nil(t, rec.Details, "Details should remain nil on error") @@ -519,7 +520,7 @@ func TestParseMemoryDBDetails(t *testing.T) { t.Run("non-default instance type populates Details", func(t *testing.T) { rec := &common.Recommendation{ResourceType: "db.r6g.large"} - err := client.parseMemoryDBDetails(rec, details) + err := client.parseMemoryDBDetails(context.Background(), rec, details) require.NoError(t, err) cacheDetails, ok := rec.Details.(*common.CacheDetails) require.True(t, ok, "Details should be *common.CacheDetails") @@ -529,7 +530,7 @@ func TestParseMemoryDBDetails(t *testing.T) { t.Run("xlarge instance type populates Details", func(t *testing.T) { rec := &common.Recommendation{ResourceType: "db.r6gd.xlarge"} - err := client.parseMemoryDBDetails(rec, details) + err := client.parseMemoryDBDetails(context.Background(), rec, details) require.NoError(t, err) cacheDetails, ok := rec.Details.(*common.CacheDetails) require.True(t, ok, "Details should be *common.CacheDetails") diff --git a/providers/aws/recommendations/sku.go b/providers/aws/recommendations/sku.go new file mode 100644 index 000000000..d5f4f7da5 --- /dev/null +++ b/providers/aws/recommendations/sku.go @@ -0,0 +1,116 @@ +package recommendations + +import ( + "context" + "sync" + + awsec2 "github.com/aws/aws-sdk-go-v2/service/ec2" + ec2types "github.com/aws/aws-sdk-go-v2/service/ec2/types" + + "github.com/LeanerCloud/CUDly/pkg/logging" +) + +// DescribeInstanceTypesAPI is the subset of the EC2 client interface +// needed to build a DescribeInstanceTypes paginator. The production +// implementation is *ec2.Client; tests inject a stub. +type DescribeInstanceTypesAPI interface { + DescribeInstanceTypes(ctx context.Context, params *awsec2.DescribeInstanceTypesInput, optFns ...func(*awsec2.Options)) (*awsec2.DescribeInstanceTypesOutput, error) +} + +// InstanceTypePager defines the iteration contract for DescribeInstanceTypes pages. +// Production code uses ec2.NewDescribeInstanceTypesPaginator; tests inject a stub. +type InstanceTypePager interface { + HasMorePages() bool + NextPage(ctx context.Context, optFns ...func(*awsec2.Options)) (*awsec2.DescribeInstanceTypesOutput, error) +} + +// instanceTypeSKUEntry caches the vCPU/memory shape for one instance type. +// Either field is 0 when the API returned no data for it; +// common.ComputeDetails treats 0 as "unknown" (omitempty JSON tags). +type instanceTypeSKUEntry struct { + vCPUs int + memoryGB float64 +} + +// skuCatalog holds a lazily-built per-Client instance-type catalogue. +// The catalogue is fetched ONCE per client lifetime via sync.Once +// so a single recommendations refresh issues at most one +// DescribeInstanceTypes fan-out regardless of how many EC2 recs are returned. +type skuCatalog struct { + once sync.Once + m map[string]instanceTypeSKUEntry // nil means fetch failed +} + +// lookup returns the catalogue entry for instanceType, building the +// catalogue on the first call. ok=false on cache miss or fetch failure; +// the caller falls back to VCPU=0 / MemoryGB=0 and does NOT fail the +// conversion (graceful-degradation contract from Azure PR #810). +func (s *skuCatalog) lookup(ctx context.Context, instanceType string, newPager func() InstanceTypePager) (instanceTypeSKUEntry, bool) { + s.once.Do(func() { + s.m = fetchInstanceTypeCatalogue(ctx, newPager()) + }) + if s.m == nil { + return instanceTypeSKUEntry{}, false + } + entry, ok := s.m[instanceType] + return entry, ok +} + +// fetchInstanceTypeCatalogue walks the DescribeInstanceTypes paginator and +// reduces each page into an instanceType->instanceTypeSKUEntry map. +// +// Context cancellation / deadline exceeded is treated as a hard stop +// (per feedback_ctx_cancel_terminal.md): the first ctx.Err() returns nil so +// instanceTypeLookup falls back to the empty-field path; the error is logged at +// WARN so operators can detect it. +// +// Any page-fetch error also returns nil (partial results are discarded so +// callers never see a half-populated catalogue). +func fetchInstanceTypeCatalogue(ctx context.Context, pager InstanceTypePager) map[string]instanceTypeSKUEntry { + out := make(map[string]instanceTypeSKUEntry) + for pager.HasMorePages() { + if err := ctx.Err(); err != nil { + logging.Warnf("aws ec2: instance type catalogue fetch interrupted: %v — Details.VCPU/MemoryGB left at 0", err) + return nil + } + page, err := pager.NextPage(ctx) + if err != nil { + logging.Warnf("aws ec2: instance type catalogue page fetch failed: %v — Details.VCPU/MemoryGB left at 0", err) + return nil + } + populateInstanceTypeSKUMap(out, page.InstanceTypes) + } + return out +} + +// populateInstanceTypeSKUMap writes one instanceTypeSKUEntry per item in +// instanceTypes into out. First-write-wins on duplicate names. +func populateInstanceTypeSKUMap(out map[string]instanceTypeSKUEntry, instanceTypes []ec2types.InstanceTypeInfo) { + for _, info := range instanceTypes { + name := string(info.InstanceType) + if name == "" { + continue + } + if _, exists := out[name]; exists { + continue + } + out[name] = extractInstanceTypeSKUEntry(info) + } +} + +// extractInstanceTypeSKUEntry reads the vCPU count and memory size from +// InstanceTypeInfo. Returns (0, 0) when either field is absent or nil; +// callers treat 0 as "unknown". +func extractInstanceTypeSKUEntry(info ec2types.InstanceTypeInfo) instanceTypeSKUEntry { + var vCPUs int + var memoryGB float64 + + if info.VCpuInfo != nil && info.VCpuInfo.DefaultVCpus != nil { + vCPUs = int(*info.VCpuInfo.DefaultVCpus) + } + if info.MemoryInfo != nil && info.MemoryInfo.SizeInMiB != nil { + memoryGB = float64(*info.MemoryInfo.SizeInMiB) / 1024.0 + } + + return instanceTypeSKUEntry{vCPUs: vCPUs, memoryGB: memoryGB} +} diff --git a/providers/aws/recommendations/sku_test.go b/providers/aws/recommendations/sku_test.go new file mode 100644 index 000000000..64842842b --- /dev/null +++ b/providers/aws/recommendations/sku_test.go @@ -0,0 +1,276 @@ +package recommendations + +import ( + "context" + "fmt" + "sync/atomic" + "testing" + + "github.com/aws/aws-sdk-go-v2/aws" + cetypes "github.com/aws/aws-sdk-go-v2/service/costexplorer/types" + awsec2 "github.com/aws/aws-sdk-go-v2/service/ec2" + ec2types "github.com/aws/aws-sdk-go-v2/service/ec2/types" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/LeanerCloud/CUDly/pkg/common" +) + +// stubInstanceTypePager is an in-memory pager that returns a fixed list of +// InstanceTypeInfo pages. It counts how many times NextPage is called so +// tests can assert the one-call-per-cache-lifetime invariant. +type stubInstanceTypePager struct { + pages [][]ec2types.InstanceTypeInfo + pageIndex int + callCount int32 // atomic +} + +func (p *stubInstanceTypePager) HasMorePages() bool { + return p.pageIndex < len(p.pages) +} + +func (p *stubInstanceTypePager) NextPage(_ context.Context, _ ...func(*awsec2.Options)) (*awsec2.DescribeInstanceTypesOutput, error) { + atomic.AddInt32(&p.callCount, 1) + if p.pageIndex >= len(p.pages) { + return nil, fmt.Errorf("no more pages") + } + page := p.pages[p.pageIndex] + p.pageIndex++ + return &awsec2.DescribeInstanceTypesOutput{InstanceTypes: page}, nil +} + +// newStubPager builds a single-page stub from the given entries. +func newStubPager(entries ...ec2types.InstanceTypeInfo) *stubInstanceTypePager { + return &stubInstanceTypePager{pages: [][]ec2types.InstanceTypeInfo{entries}} +} + +// knownInstanceTypes returns a slice with two well-known instance types for +// use in tests that need a populated catalogue. +func knownInstanceTypes() []ec2types.InstanceTypeInfo { + return []ec2types.InstanceTypeInfo{ + { + InstanceType: ec2types.InstanceTypeM5Large, + VCpuInfo: &ec2types.VCpuInfo{DefaultVCpus: aws.Int32(2)}, + MemoryInfo: &ec2types.MemoryInfo{SizeInMiB: aws.Int64(8192)}, + }, + { + InstanceType: ec2types.InstanceTypeR5Xlarge, + VCpuInfo: &ec2types.VCpuInfo{DefaultVCpus: aws.Int32(4)}, + MemoryInfo: &ec2types.MemoryInfo{SizeInMiB: aws.Int64(32768)}, + }, + } +} + +// TestExtractInstanceTypeSKUEntry verifies field extraction from InstanceTypeInfo. +func TestExtractInstanceTypeSKUEntry(t *testing.T) { + tests := []struct { + name string + info ec2types.InstanceTypeInfo + wantVCPU int + wantMemGB float64 + }{ + { + name: "m5.large -- 2 vCPU / 8 GB", + info: ec2types.InstanceTypeInfo{ + VCpuInfo: &ec2types.VCpuInfo{DefaultVCpus: aws.Int32(2)}, + MemoryInfo: &ec2types.MemoryInfo{SizeInMiB: aws.Int64(8192)}, + }, + wantVCPU: 2, + wantMemGB: 8.0, + }, + { + name: "r5.xlarge -- 4 vCPU / 32 GB", + info: ec2types.InstanceTypeInfo{ + VCpuInfo: &ec2types.VCpuInfo{DefaultVCpus: aws.Int32(4)}, + MemoryInfo: &ec2types.MemoryInfo{SizeInMiB: aws.Int64(32768)}, + }, + wantVCPU: 4, + wantMemGB: 32.0, + }, + { + name: "nil VCpuInfo and MemoryInfo -- both zero", + info: ec2types.InstanceTypeInfo{}, + wantVCPU: 0, + wantMemGB: 0.0, + }, + { + name: "VCpuInfo present but DefaultVCpus nil -- VCPU zero", + info: ec2types.InstanceTypeInfo{ + VCpuInfo: &ec2types.VCpuInfo{}, + MemoryInfo: &ec2types.MemoryInfo{SizeInMiB: aws.Int64(4096)}, + }, + wantVCPU: 0, + wantMemGB: 4.0, + }, + { + name: "odd MiB value -- fractional GB", + info: ec2types.InstanceTypeInfo{ + VCpuInfo: &ec2types.VCpuInfo{DefaultVCpus: aws.Int32(1)}, + MemoryInfo: &ec2types.MemoryInfo{SizeInMiB: aws.Int64(512)}, + }, + wantVCPU: 1, + wantMemGB: 0.5, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + entry := extractInstanceTypeSKUEntry(tt.info) + assert.Equal(t, tt.wantVCPU, entry.vCPUs) + assert.InDelta(t, tt.wantMemGB, entry.memoryGB, 0.001) + }) + } +} + +// TestFetchInstanceTypeCatalogue_PopulatesMap verifies the paginator walk +// produces a correctly-keyed map. +func TestFetchInstanceTypeCatalogue_PopulatesMap(t *testing.T) { + pager := newStubPager(knownInstanceTypes()...) + m := fetchInstanceTypeCatalogue(context.Background(), pager) + + require.NotNil(t, m) + assert.Len(t, m, 2) + + m5, ok := m["m5.large"] + require.True(t, ok, "m5.large must be in catalogue") + assert.Equal(t, 2, m5.vCPUs) + assert.InDelta(t, 8.0, m5.memoryGB, 0.001) + + r5, ok := m["r5.xlarge"] + require.True(t, ok, "r5.xlarge must be in catalogue") + assert.Equal(t, 4, r5.vCPUs) + assert.InDelta(t, 32.0, r5.memoryGB, 0.001) +} + +// TestFetchInstanceTypeCatalogue_PageError returns nil on page fetch failure. +func TestFetchInstanceTypeCatalogue_PageError(t *testing.T) { + errPager := &errorOnFirstPagePager{} + m := fetchInstanceTypeCatalogue(context.Background(), errPager) + assert.Nil(t, m, "catalogue must be nil on page fetch error") +} + +// TestFetchInstanceTypeCatalogue_ContextCanceled returns nil when ctx is canceled. +func TestFetchInstanceTypeCatalogue_ContextCanceled(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + cancel() // already canceled + + pager := newStubPager(knownInstanceTypes()...) + m := fetchInstanceTypeCatalogue(ctx, pager) + assert.Nil(t, m, "catalogue must be nil when ctx is already canceled") +} + +// TestInstanceTypeLookup_CachedOnce asserts that a single GetRecommendations +// run issues at most one DescribeInstanceTypes fan-out regardless of how many +// EC2 recs are returned (the N+1 invariant, issue #218 acceptance criterion). +func TestInstanceTypeLookup_CachedOnce(t *testing.T) { + pager := newStubPager(knownInstanceTypes()...) + var pagerCreations int32 + + client := NewClientWithAPI(&mockCostExplorerAPI{}, "us-east-1") + client.SetInstanceTypePagerFactory(func() InstanceTypePager { + atomic.AddInt32(&pagerCreations, 1) + return pager + }) + + // Call instanceTypeLookup twice for the same and different instance types. + ctx := context.Background() + _, _ = client.instanceTypeLookup(ctx, "m5.large") + _, _ = client.instanceTypeLookup(ctx, "r5.xlarge") + _, _ = client.instanceTypeLookup(ctx, "m5.large") + + assert.Equal(t, int32(1), atomic.LoadInt32(&pagerCreations), + "pager factory must be called exactly once per client lifetime") +} + +// TestParseEC2Details_VCPUAndMemoryPopulated asserts that parseEC2Details +// enriches ComputeDetails.VCPU and MemoryGB from the catalogue. +func TestParseEC2Details_VCPUAndMemoryPopulated(t *testing.T) { + pager := newStubPager(knownInstanceTypes()...) + client := NewClientWithAPI(&mockCostExplorerAPI{}, "us-east-1") + client.SetInstanceTypePagerFactory(func() InstanceTypePager { return pager }) + + details := &cetypes.ReservationPurchaseRecommendationDetail{ + InstanceDetails: &cetypes.InstanceDetails{ + EC2InstanceDetails: &cetypes.EC2InstanceDetails{ + InstanceType: aws.String("m5.large"), + Platform: aws.String("Linux/UNIX"), + Region: aws.String("us-east-1"), + Tenancy: aws.String("shared"), + }, + }, + } + + rec := &common.Recommendation{} + err := client.parseEC2Details(context.Background(), rec, details) + require.NoError(t, err) + + cd, ok := rec.Details.(*common.ComputeDetails) + require.True(t, ok) + assert.Equal(t, 2, cd.VCPU) + assert.InDelta(t, 8.0, cd.MemoryGB, 0.001) +} + +// TestParseEC2Details_CatalogueMiss leaves VCPU/MemoryGB at zero gracefully. +func TestParseEC2Details_CatalogueMiss(t *testing.T) { + pager := newStubPager(knownInstanceTypes()...) // does not contain c5.large + client := NewClientWithAPI(&mockCostExplorerAPI{}, "us-east-1") + client.SetInstanceTypePagerFactory(func() InstanceTypePager { return pager }) + + details := &cetypes.ReservationPurchaseRecommendationDetail{ + InstanceDetails: &cetypes.InstanceDetails{ + EC2InstanceDetails: &cetypes.EC2InstanceDetails{ + InstanceType: aws.String("c5.large"), + Platform: aws.String("Linux/UNIX"), + Region: aws.String("us-east-1"), + }, + }, + } + + rec := &common.Recommendation{} + err := client.parseEC2Details(context.Background(), rec, details) + require.NoError(t, err, "catalogue miss must not fail the conversion") + + cd, ok := rec.Details.(*common.ComputeDetails) + require.True(t, ok) + assert.Equal(t, 0, cd.VCPU, "VCPU must be 0 on catalogue miss") + assert.InDelta(t, 0.0, cd.MemoryGB, 0.001, "MemoryGB must be 0 on catalogue miss") +} + +// TestParseEC2Details_NoCatalogueConfigured leaves VCPU/MemoryGB at zero +// gracefully when no pager factory is set (legacy test path). +func TestParseEC2Details_NoCatalogueConfigured(t *testing.T) { + client := &Client{} // no factory + + details := &cetypes.ReservationPurchaseRecommendationDetail{ + InstanceDetails: &cetypes.InstanceDetails{ + EC2InstanceDetails: &cetypes.EC2InstanceDetails{ + InstanceType: aws.String("m5.large"), + Platform: aws.String("Linux/UNIX"), + Region: aws.String("us-east-1"), + }, + }, + } + + rec := &common.Recommendation{} + err := client.parseEC2Details(context.Background(), rec, details) + require.NoError(t, err) + + cd, ok := rec.Details.(*common.ComputeDetails) + require.True(t, ok) + assert.Equal(t, 0, cd.VCPU) + assert.InDelta(t, 0.0, cd.MemoryGB, 0.001) +} + +// errorOnFirstPagePager is a pager stub that returns an error on the first NextPage call. +type errorOnFirstPagePager struct { + called bool +} + +func (p *errorOnFirstPagePager) HasMorePages() bool { + return !p.called +} + +func (p *errorOnFirstPagePager) NextPage(_ context.Context, _ ...func(*awsec2.Options)) (*awsec2.DescribeInstanceTypesOutput, error) { + p.called = true + return nil, fmt.Errorf("simulated AWS API error") +}