diff --git a/go.mod b/go.mod index 8c5ef42e..5a9c3e3a 100644 --- a/go.mod +++ b/go.mod @@ -84,10 +84,10 @@ require ( github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/reservations/armreservations v1.1.0 github.com/Azure/azure-sdk-for-go/sdk/security/keyvault/azkeys v1.4.0 github.com/Azure/azure-sdk-for-go/sdk/security/keyvault/azsecrets v1.4.0 - github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261015143912-740dfbd72456 - github.com/LeanerCloud/cloud-commitments-go/providers/aws v0.0.0-20261015143912-740dfbd72456 - github.com/LeanerCloud/cloud-commitments-go/providers/azure v0.0.0-20261015143912-740dfbd72456 - github.com/LeanerCloud/cloud-commitments-go/providers/gcp v0.0.0-20261015143912-740dfbd72456 + github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261016145956-551be568dc21 + github.com/LeanerCloud/cloud-commitments-go/providers/aws v0.0.0-20261016145956-551be568dc21 + github.com/LeanerCloud/cloud-commitments-go/providers/azure v0.0.0-20261016145956-551be568dc21 + github.com/LeanerCloud/cloud-commitments-go/providers/gcp v0.0.0-20261016145956-551be568dc21 github.com/aws/aws-lambda-go v1.47.0 github.com/aws/aws-sdk-go-v2/service/kms v1.50.4 github.com/aws/aws-sdk-go-v2/service/lambda v1.89.0 diff --git a/go.sum b/go.sum index 9001e046..e7eab142 100644 --- a/go.sum +++ b/go.sum @@ -92,14 +92,14 @@ github.com/GoogleCloudPlatform/opentelemetry-operations-go/internal/cloudmock v0 github.com/GoogleCloudPlatform/opentelemetry-operations-go/internal/cloudmock v0.54.0/go.mod h1:vB2GH9GAYYJTO3mEn8oYwzEdhlayZIdQz6zdzgUIRvA= github.com/GoogleCloudPlatform/opentelemetry-operations-go/internal/resourcemapping v0.54.0 h1:s0WlVbf9qpvkh1c/uDAPElam0WrL7fHRIidgZJ7UqZI= github.com/GoogleCloudPlatform/opentelemetry-operations-go/internal/resourcemapping v0.54.0/go.mod h1:Mf6O40IAyB9zR/1J8nGDDPirZQQPbYJni8Yisy7NTMc= -github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261015143912-740dfbd72456 h1:ZPVVIwrZ28I+f+Fe6DUK6g/nVGIJj06nNhI32M7rt9I= -github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261015143912-740dfbd72456/go.mod h1:8RTmzPazA8IeVZfn7wNaU7ILx0VK5eHZQozO21Mvtq8= -github.com/LeanerCloud/cloud-commitments-go/providers/aws v0.0.0-20261015143912-740dfbd72456 h1:3lNYgah7tF+bnOXv619jgiSguUhpiI2I2sFmLrj7Ba8= -github.com/LeanerCloud/cloud-commitments-go/providers/aws v0.0.0-20261015143912-740dfbd72456/go.mod h1:uOUuTVRjTm0Fhu/Vra2M2NES0w4pvZUSfZLsGyCd18s= -github.com/LeanerCloud/cloud-commitments-go/providers/azure v0.0.0-20261015143912-740dfbd72456 h1:agVjEMOqGsehx98Lb4IyIPlCzf6VIBNHjGekUfCxkpU= -github.com/LeanerCloud/cloud-commitments-go/providers/azure v0.0.0-20261015143912-740dfbd72456/go.mod h1:8vqO2ZlDzjX/Z4bESZVp/IenhTifkySeF0VJwQKihLA= -github.com/LeanerCloud/cloud-commitments-go/providers/gcp v0.0.0-20261015143912-740dfbd72456 h1:+pjt8F5bHFHz9OFYEOXUESdIE1KVSfdQH3URqsjaIfw= -github.com/LeanerCloud/cloud-commitments-go/providers/gcp v0.0.0-20261015143912-740dfbd72456/go.mod h1:KV+Ov04+ZMBGv1wA0s9lEHLinlg1p74XrNafXYLwKmE= +github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261016145956-551be568dc21 h1:gZSO852B+hqQWi6lSvPaOWFtWigIw3vVr7HDtX6CMR8= +github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261016145956-551be568dc21/go.mod h1:SCDEgHG6evjPh0RsoXof+J3aMmGXNFMrwbJIle7rMpQ= +github.com/LeanerCloud/cloud-commitments-go/providers/aws v0.0.0-20261016145956-551be568dc21 h1:Es0vAPXDxERSD6KGQr/ZUg4aVlBAmMsB2XkosI0IqPI= +github.com/LeanerCloud/cloud-commitments-go/providers/aws v0.0.0-20261016145956-551be568dc21/go.mod h1:uOUuTVRjTm0Fhu/Vra2M2NES0w4pvZUSfZLsGyCd18s= +github.com/LeanerCloud/cloud-commitments-go/providers/azure v0.0.0-20261016145956-551be568dc21 h1:PSl5J95u1vS3cvVeci9SkXk/R5iek0NbrxwxxozqyaY= +github.com/LeanerCloud/cloud-commitments-go/providers/azure v0.0.0-20261016145956-551be568dc21/go.mod h1:8vqO2ZlDzjX/Z4bESZVp/IenhTifkySeF0VJwQKihLA= +github.com/LeanerCloud/cloud-commitments-go/providers/gcp v0.0.0-20261016145956-551be568dc21 h1:J+c1GcA/i1c3korxqxwkRMOFgJ5OtyeoPxlsU3a3q1U= +github.com/LeanerCloud/cloud-commitments-go/providers/gcp v0.0.0-20261016145956-551be568dc21/go.mod h1:KV+Ov04+ZMBGv1wA0s9lEHLinlg1p74XrNafXYLwKmE= github.com/Microsoft/go-winio v0.6.2 h1:F2VQgta7ecxGYO8k3ZZz3RS8fVIXVxONVUPlNERoyfY= github.com/Microsoft/go-winio v0.6.2/go.mod h1:yd8OoFMLzJbo9gZq8j5qaps8bJ9aShtEA8Ipt1oGCvU= github.com/aws/aws-lambda-go v1.47.0 h1:0H8s0vumYx/YKs4sE7YM0ktwL2eWse+kfopsRI1sXVI= diff --git a/internal/api/handler_ri_exchange.go b/internal/api/handler_ri_exchange.go index 181190fd..caa29230 100644 --- a/internal/api/handler_ri_exchange.go +++ b/internal/api/handler_ri_exchange.go @@ -156,18 +156,56 @@ func (h *Handler) listTargetOfferings(ctx context.Context, req *events.LambdaFun return nil, NewClientError(404, fmt.Sprintf("source RI %q not found in region %s", sourceRIID, cfg.Region)) } + offerings, err := targetOfferingsForSource(ctx, ec2Client, sourceRI) + if err != nil { + return nil, err + } + + return &TargetOfferingsResponse{Offerings: offerings}, nil +} + +// targetOfferingsForSource lists the exchange targets for sourceRI, keeping a +// zonal RI's targets in its own AZ. A zone refusal (the source RI's scope and +// AZ disagree) is a 422. +func targetOfferingsForSource(ctx context.Context, ec2Client targetOfferingsEC2Client, sourceRI *ec2svc.ConvertibleRI) ([]ec2svc.TargetOffering, error) { offerings, err := ec2Client.ListTargetOfferings(ctx, ec2svc.ListTargetOfferingsParams{ ProductDescription: sourceRI.ProductDescription, Tenancy: sourceRI.InstanceTenancy, Scope: sourceRI.Scope, + AvailabilityZone: sourceRI.AvailabilityZone, Duration: sourceRI.Duration, OfferingType: sourceRI.OfferingType, }) if err != nil { + if isOfferingZoneRefusal(err) { + return nil, NewClientError(422, fmt.Sprintf("source RI %q: %v", sourceRI.ReservedInstanceID, err)) + } return nil, fmt.Errorf("failed to list target offerings: %w", err) } + return offerings, nil +} - return &TargetOfferingsResponse{Offerings: offerings}, nil +// offeringZoneRefusals are the messages the EC2 provider returns when it +// refuses a zone-inconsistent offering lookup before calling AWS: a zonal RI +// without an AZ, or a regional RI that names one. The provider exports no +// sentinel, so these are matched by text; the target-offerings tests run the +// real provider client so a reworded message fails them. +var offeringZoneRefusals = []string{ + "zonal RI offering needs an availability zone", + "regional RI offering must not name an availability zone", +} + +// isOfferingZoneRefusal reports whether err is the provider's zone refusal. +// It is the source RI's data that is inconsistent, so the caller maps it to a +// 4xx rather than a 500. +func isOfferingZoneRefusal(err error) bool { + msg := err.Error() + for _, r := range offeringZoneRefusals { + if strings.Contains(msg, r) { + return true + } + } + return false } // azureExchangeClient is the narrow interface that listExchangeableAzureRIs, diff --git a/internal/api/target_offerings_zone_test.go b/internal/api/target_offerings_zone_test.go new file mode 100644 index 00000000..9aa5b82f --- /dev/null +++ b/internal/api/target_offerings_zone_test.go @@ -0,0 +1,121 @@ +package api + +import ( + "context" + "testing" + + "github.com/aws/aws-sdk-go-v2/aws" + awsec2sdk "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" + + ec2svc "github.com/LeanerCloud/cloud-commitments-go/providers/aws/services/ec2" +) + +// Issue #921: GET /api/ri-exchange/target-offerings sends the source RI's AZ +// to the provider, and the provider's zone refusal is a 422. The real +// ec2svc.Client runs over a fake EC2 API: no AWS call. + +// zoneRecordingTargetsAPI serves the source RI and records the AZ of every +// offering lookup. The embedded nil API panics on any other call. +type zoneRecordingTargetsAPI struct { + ec2svc.API + ri ec2types.ReservedInstances + lookupZones []*string +} + +func (a *zoneRecordingTargetsAPI) DescribeReservedInstances(context.Context, *awsec2sdk.DescribeReservedInstancesInput, ...func(*awsec2sdk.Options)) (*awsec2sdk.DescribeReservedInstancesOutput, error) { + return &awsec2sdk.DescribeReservedInstancesOutput{ReservedInstances: []ec2types.ReservedInstances{a.ri}}, nil +} + +func (a *zoneRecordingTargetsAPI) DescribeReservedInstancesOfferings(_ context.Context, in *awsec2sdk.DescribeReservedInstancesOfferingsInput, _ ...func(*awsec2sdk.Options)) (*awsec2sdk.DescribeReservedInstancesOfferingsOutput, error) { + a.lookupZones = append(a.lookupZones, in.AvailabilityZone) + return &awsec2sdk.DescribeReservedInstancesOfferingsOutput{ + ReservedInstancesOfferings: []ec2types.ReservedInstancesOffering{{ + ReservedInstancesOfferingId: aws.String("4b2293b4-5fbc-4017-9c75-d5a9d3aa8c91"), + InstanceType: ec2types.InstanceType("m5.xlarge"), + AvailabilityZone: in.AvailabilityZone, + }}, + }, nil +} + +func runTargetOfferingsForZone(t *testing.T, scope ec2types.Scope, az string) (any, *zoneRecordingTargetsAPI, error) { + t.Helper() + ri := ec2types.ReservedInstances{ + ReservedInstancesId: aws.String(targetOfferingsSourceRI), + InstanceType: ec2types.InstanceType("m5.large"), + InstanceCount: aws.Int32(1), + ProductDescription: ec2types.RIProductDescription("Linux/UNIX"), + InstanceTenancy: ec2types.TenancyDefault, + Scope: scope, + Duration: aws.Int64(31536000), + OfferingType: ec2types.OfferingTypeValuesNoUpfront, + OfferingClass: ec2types.OfferingClassTypeConvertible, + } + if az != "" { + ri.AvailabilityZone = aws.String(az) + } + api := &zoneRecordingTargetsAPI{ri: ri} + h := targetOfferingsHandler(t, scopedOutAccount, false) + h.targetOfferingsEC2Factory = func(cfg aws.Config) targetOfferingsEC2Client { + c := ec2svc.NewClient(cfg) + c.SetEC2API(api) + return c + } + res, err := NewRouter(h).Route(context.Background(), "GET", "/api/ri-exchange/target-offerings", targetOfferingsRequest()) + return res, api, err +} + +func TestListTargetOfferings_ZonalSourceForwardsAvailabilityZone(t *testing.T) { + res, api, err := runTargetOfferingsForZone(t, ec2types.ScopeAvailabilityZone, "us-east-1b") + + require.NoError(t, err) + require.NotEmpty(t, api.lookupZones) + for _, z := range api.lookupZones { + assert.Equal(t, "us-east-1b", aws.ToString(z)) + } + resp, ok := res.(*TargetOfferingsResponse) + require.True(t, ok) + assert.Len(t, resp.Offerings, 1) +} + +func TestListTargetOfferings_RegionalSourcePassesNoAvailabilityZone(t *testing.T) { + _, api, err := runTargetOfferingsForZone(t, ec2types.ScopeRegional, "") + + require.NoError(t, err) + require.NotEmpty(t, api.lookupZones) + for _, z := range api.lookupZones { + assert.Nil(t, z) + } +} + +func TestListTargetOfferings_ZonalSourceWithoutAZIs422(t *testing.T) { + _, api, err := runTargetOfferingsForZone(t, ec2types.ScopeAvailabilityZone, "") + + require.Error(t, err) + ce, ok := IsClientError(err) + require.True(t, ok, "zone refusal must be a client error, got %v", err) + assert.Equal(t, 422, ce.code) + assert.Contains(t, ce.message, "zonal RI offering needs an availability zone") + assert.Contains(t, ce.message, targetOfferingsSourceRI) + assert.Empty(t, api.lookupZones, "no AWS offering lookup without an AZ") +} + +func TestIsOfferingZoneRefusal(t *testing.T) { + assert.False(t, isOfferingZoneRefusal(assert.AnError)) + for _, msg := range offeringZoneRefusals { + assert.True(t, isOfferingZoneRefusal(&clientError{message: "wrapped: " + msg}), msg) + } +} + +func TestListTargetOfferings_RegionalSourceWithAZIs422(t *testing.T) { + _, api, err := runTargetOfferingsForZone(t, ec2types.ScopeRegional, "us-east-1b") + + require.Error(t, err) + ce, ok := IsClientError(err) + require.True(t, ok, "zone refusal must be a client error, got %v", err) + assert.Equal(t, 422, ce.code) + assert.Contains(t, ce.message, "regional RI offering must not name an availability zone") + assert.Empty(t, api.lookupZones) +} diff --git a/internal/server/handler_ri_exchange.go b/internal/server/handler_ri_exchange.go index 156488d6..73406b5b 100644 --- a/internal/server/handler_ri_exchange.go +++ b/internal/server/handler_ri_exchange.go @@ -26,7 +26,7 @@ type riExchangeClients struct { listConvertibleRIs func(ctx context.Context) ([]ec2svc.ConvertibleRI, error) getRIUtilization func(ctx context.Context, lookbackDays int) ([]recommendations.RIUtilization, error) exchangeClient exchange.ExchangeClientInterface - lookupOffering func(ctx context.Context, instanceType, productDesc, tenancy, scope string, duration int64) (string, error) + lookupOffering exchange.LookupOfferingFunc accountID string region string } @@ -59,22 +59,37 @@ func (app *Application) handleRIExchangeReshape(ctx context.Context) (*exchange. return recsClient.GetRIUtilization(ctx, lookbackDays, awsCfg.Region) }, exchangeClient: exchange.NewExchangeClient(awsCfg), - lookupOffering: func(ctx context.Context, instanceType, productDesc, tenancy, scope string, duration int64) (string, error) { - return ec2Client.FindConvertibleOffering(ctx, ec2svc.FindConvertibleOfferingParams{ - InstanceType: instanceType, - ProductDescription: productDesc, - Tenancy: tenancy, - Scope: scope, - Duration: duration, - }) - }, - accountID: resolveAccountID(ctx, awsCfg), - region: awsCfg.Region, + lookupOffering: newConvertibleOfferingLookup(ec2Client), + accountID: resolveAccountID(ctx, awsCfg), + region: awsCfg.Region, } return app.executeRIExchangeReshape(ctx, cfg, clients) } +// convertibleOfferingFinder is the slice of the EC2 client the offering +// lookup needs. Satisfied by *ec2svc.Client. +type convertibleOfferingFinder interface { + FindConvertibleOffering(ctx context.Context, params ec2svc.FindConvertibleOfferingParams) (string, error) +} + +// newConvertibleOfferingLookup adapts the EC2 client to exchange.LookupOfferingFunc. +// AvailabilityZone is the source RI's AZ (empty for regional RIs); the provider +// refuses a zonal lookup without one, so a zonal RI never exchanges into an +// offering in another zone. +func newConvertibleOfferingLookup(finder convertibleOfferingFinder) exchange.LookupOfferingFunc { + return func(ctx context.Context, p exchange.LookupOfferingParams) (string, error) { + return finder.FindConvertibleOffering(ctx, ec2svc.FindConvertibleOfferingParams{ + InstanceType: p.InstanceType, + ProductDescription: p.ProductDescription, + Tenancy: p.Tenancy, + Scope: p.Scope, + AvailabilityZone: p.AvailabilityZone, + Duration: p.Duration, + }) + } +} + // executeRIExchangeReshape contains the core exchange logic, separated from AWS // client construction for testability. func (app *Application) executeRIExchangeReshape(ctx context.Context, cfg *config.GlobalConfig, clients riExchangeClients) (*exchange.AutoExchangeResult, error) { @@ -263,6 +278,7 @@ func convertForAutoExchange(instances []ec2svc.ConvertibleRI, utilData []recomme ProductDescription: inst.ProductDescription, InstanceTenancy: inst.InstanceTenancy, Scope: inst.Scope, + AvailabilityZone: inst.AvailabilityZone, Duration: inst.Duration, } } diff --git a/internal/server/handler_ri_exchange_reservation_integration_test.go b/internal/server/handler_ri_exchange_reservation_integration_test.go index e7a055d1..058e1741 100644 --- a/internal/server/handler_ri_exchange_reservation_integration_test.go +++ b/internal/server/handler_ri_exchange_reservation_integration_test.go @@ -88,7 +88,7 @@ func TestRIExchangeAutoWorkers_ConcreteStoreAndFixtureClient(t *testing.T) { listConvertibleRIs: func(context.Context) ([]ec2svc.ConvertibleRI, error) { return ri(standaloneRIID), nil }, getRIUtilization: func(context.Context, int) ([]recommendations.RIUtilization, error) { return util(standaloneRIID), nil }, exchangeClient: quoteClient(standaloneRIID, &standaloneExecutions), - lookupOffering: func(context.Context, string, string, string, string, int64) (string, error) { + lookupOffering: func(context.Context, exchange.LookupOfferingParams) (string, error) { return "offering-test", nil }, accountID: "555555555555", region: "us-east-1", @@ -108,7 +108,7 @@ func TestRIExchangeAutoWorkers_ConcreteStoreAndFixtureClient(t *testing.T) { riInfos, utilInfos, metadata := convertForAutoExchange(ri(ladderRIID), util(ladderRIID)) ladder, err := exchange.RunAutoExchange(ctx, exchange.RunAutoExchangeParams{ Store: newConfigExchangeStoreAdapter(store), ExchangeClient: quoteClient(ladderRIID, &ladderExecutions), - LookupOffering: func(context.Context, string, string, string, string, int64) (string, error) { + LookupOffering: func(context.Context, exchange.LookupOfferingParams) (string, error) { return "offering-test", nil }, RIs: riInfos, Utilization: utilInfos, RIMetadata: metadata, diff --git a/internal/server/handler_ri_exchange_test.go b/internal/server/handler_ri_exchange_test.go index a5d58e1e..5ab105a1 100644 --- a/internal/server/handler_ri_exchange_test.go +++ b/internal/server/handler_ri_exchange_test.go @@ -460,7 +460,7 @@ func TestExecuteRIExchangeReshape_NoConvertibleRIs(t *testing.T) { return nil, nil }, exchangeClient: &mockExchangeClient{}, - lookupOffering: func(ctx context.Context, instanceType, productDesc, tenancy, scope string, duration int64) (string, error) { + lookupOffering: func(ctx context.Context, _ exchange.LookupOfferingParams) (string, error) { return "", nil }, accountID: "123456789", @@ -587,7 +587,7 @@ func TestExecuteRIExchangeReshape_ManualMode(t *testing.T) { }, nil }, }, - lookupOffering: func(ctx context.Context, instanceType, productDesc, tenancy, scope string, duration int64) (string, error) { + lookupOffering: func(ctx context.Context, _ exchange.LookupOfferingParams) (string, error) { return "offering-123", nil }, accountID: "123456789", @@ -706,7 +706,7 @@ func TestExecuteRIExchangeReshape_AutoMode(t *testing.T) { }, nil }, }, - lookupOffering: func(ctx context.Context, instanceType, productDesc, tenancy, scope string, duration int64) (string, error) { + lookupOffering: func(ctx context.Context, _ exchange.LookupOfferingParams) (string, error) { return "offering-456", nil }, accountID: "123456789", @@ -821,7 +821,7 @@ func TestExecuteRIExchangeReshape_DailyCapHitMidRun(t *testing.T) { }, nil }, }, - lookupOffering: func(ctx context.Context, instanceType, productDesc, tenancy, scope string, duration int64) (string, error) { + lookupOffering: func(ctx context.Context, _ exchange.LookupOfferingParams) (string, error) { return "offering-cap-test", nil }, accountID: "123456789", diff --git a/internal/server/ladder_write.go b/internal/server/ladder_write.go index 8f8b9666..afce36e1 100644 --- a/internal/server/ladder_write.go +++ b/internal/server/ladder_write.go @@ -11,7 +11,6 @@ import ( pkgladder "github.com/LeanerCloud/cloud-commitments-go/pkg/ladder" awsprovider "github.com/LeanerCloud/cloud-commitments-go/providers/aws" awsladder "github.com/LeanerCloud/cloud-commitments-go/providers/aws/ladder" - ec2svc "github.com/LeanerCloud/cloud-commitments-go/providers/aws/services/ec2" ) // exchangeRunnerAdapter bridges internal/server wiring (exchange store, EC2 exchange @@ -60,20 +59,10 @@ func (a *exchangeRunnerAdapter) RunAutoExchange(ctx context.Context, cfg exchang riInfos, utilInfos, riMetadata := convertForAutoExchange(instances, utilData) store := newConfigExchangeStoreAdapter(a.app.Config) - lookupOffering := func(ctx context.Context, instanceType, productDesc, tenancy, scope string, duration int64) (string, error) { - return ec2Client.FindConvertibleOffering(ctx, ec2svc.FindConvertibleOfferingParams{ - InstanceType: instanceType, - ProductDescription: productDesc, - Tenancy: tenancy, - Scope: scope, - Duration: duration, - }) - } - return exchange.RunAutoExchange(ctx, exchange.RunAutoExchangeParams{ Store: store, ExchangeClient: exchange.NewExchangeClient(awsCfg), - LookupOffering: lookupOffering, + LookupOffering: newConvertibleOfferingLookup(ec2Client), RIs: riInfos, Utilization: utilInfos, Config: cfg, diff --git a/internal/server/ri_exchange_zone_test.go b/internal/server/ri_exchange_zone_test.go new file mode 100644 index 00000000..dda90ad6 --- /dev/null +++ b/internal/server/ri_exchange_zone_test.go @@ -0,0 +1,184 @@ +package server + +import ( + "context" + "math/big" + "testing" + + "github.com/aws/aws-sdk-go-v2/aws" + awsec2sdk "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/cloud-commitments-go/pkg/exchange" + "github.com/LeanerCloud/cloud-commitments-go/providers/aws/recommendations" + ec2svc "github.com/LeanerCloud/cloud-commitments-go/providers/aws/services/ec2" + "github.com/LeanerCloud/cloud-commitments-platform/internal/config" +) + +// Issue #921: the scheduled RI exchange forwards the source RI's AZ through +// RunAutoExchange to the provider's offering lookup. These tests run the real +// ec2svc.Client over a fake EC2 API: no AWS call, no exchange accept. + +const zoneTestRIID = "ri-zone-test" + +// zoneRecordingEC2API serves one convertible RI and records the AZ of every +// offering lookup. The embedded nil API makes any other call (including the +// exchange accept) panic. +type zoneRecordingEC2API struct { + ec2svc.API + ri ec2types.ReservedInstances + lookupZones []*string +} + +func (a *zoneRecordingEC2API) DescribeReservedInstances(context.Context, *awsec2sdk.DescribeReservedInstancesInput, ...func(*awsec2sdk.Options)) (*awsec2sdk.DescribeReservedInstancesOutput, error) { + return &awsec2sdk.DescribeReservedInstancesOutput{ReservedInstances: []ec2types.ReservedInstances{a.ri}}, nil +} + +func (a *zoneRecordingEC2API) DescribeReservedInstancesOfferings(_ context.Context, in *awsec2sdk.DescribeReservedInstancesOfferingsInput, _ ...func(*awsec2sdk.Options)) (*awsec2sdk.DescribeReservedInstancesOfferingsOutput, error) { + a.lookupZones = append(a.lookupZones, in.AvailabilityZone) + return &awsec2sdk.DescribeReservedInstancesOfferingsOutput{ + ReservedInstancesOfferings: []ec2types.ReservedInstancesOffering{{ + ReservedInstancesOfferingId: aws.String("offering-in-zone"), + AvailabilityZone: in.AvailabilityZone, + }}, + }, nil +} + +func zoneTestRI(scope ec2types.Scope, az string) ec2types.ReservedInstances { + ri := ec2types.ReservedInstances{ + ReservedInstancesId: aws.String(zoneTestRIID), + InstanceType: ec2types.InstanceType("m5.large"), + InstanceCount: aws.Int32(2), + ProductDescription: ec2types.RIProductDescription("Linux/UNIX"), + InstanceTenancy: ec2types.TenancyDefault, + Scope: scope, + Duration: aws.Int64(31536000), + OfferingClass: ec2types.OfferingClassTypeConvertible, + CurrencyCode: ec2types.CurrencyCodeValuesUsd, + } + if az != "" { + ri.AvailabilityZone = aws.String(az) + } + return ri +} + +// runZoneReshape runs a manual-mode reshape for one underutilized RI and +// returns the result, the fake API and whether a quote was requested. +func runZoneReshape(t *testing.T, ri ec2types.ReservedInstances) (*exchange.AutoExchangeResult, *zoneRecordingEC2API, bool) { + t.Helper() + api := &zoneRecordingEC2API{ri: ri} + c := ec2svc.NewClient(aws.Config{Region: "us-east-1"}) + c.SetEC2API(api) + + quoted := false + store := &mockConfigStoreForExchange{ + saveRIExchangeRecordFunc: func(context.Context, *config.RIExchangeRecord) error { return nil }, + } + app := &Application{Config: store, Email: &mockEmailSender{}} + cfg := &config.GlobalConfig{ + RIExchangeEnabled: true, + RIExchangeMode: "manual", + RIExchangeUtilizationThreshold: 95.0, + RIExchangeMaxPerExchangeUSD: 100.0, + RIExchangeMaxDailyUSD: 500.0, + RIExchangeLookbackDays: 30, + } + clients := riExchangeClients{ + listConvertibleRIs: c.ListConvertibleReservedInstances, + getRIUtilization: func(context.Context, int) ([]recommendations.RIUtilization, error) { + return []recommendations.RIUtilization{{ReservedInstanceID: zoneTestRIID, UtilizationPercent: 40.0}}, nil + }, + exchangeClient: &mockExchangeClient{ + getQuoteFunc: func(context.Context, exchange.ExchangeQuoteRequest) (*exchange.ExchangeQuoteSummary, error) { + quoted = true + return &exchange.ExchangeQuoteSummary{ + IsValidExchange: true, + PaymentDueUSD: new(big.Rat).SetFloat64(5.00), + PaymentDueUSDStr: "5.00", + CurrencyCode: "USD", + }, nil + }, + }, + lookupOffering: newConvertibleOfferingLookup(c), + accountID: "123456789012", + region: "us-east-1", + } + + result, err := app.executeRIExchangeReshape(context.Background(), cfg, clients) + require.NoError(t, err) + return result, api, quoted +} + +func TestExecuteRIExchangeReshape_ZonalRIForwardsAvailabilityZone(t *testing.T) { + result, api, quoted := runZoneReshape(t, zoneTestRI(ec2types.ScopeAvailabilityZone, "us-east-1b")) + + require.NotEmpty(t, api.lookupZones, "offering lookup must reach the provider") + for _, z := range api.lookupZones { + assert.Equal(t, "us-east-1b", aws.ToString(z), "every lookup must be pinned to the source RI's AZ") + } + assert.True(t, quoted) + require.Len(t, result.Pending, 1) + assert.Empty(t, result.Skipped) +} + +func TestExecuteRIExchangeReshape_RegionalRIPassesNoAvailabilityZone(t *testing.T) { + result, api, quoted := runZoneReshape(t, zoneTestRI(ec2types.ScopeRegional, "")) + + require.NotEmpty(t, api.lookupZones, "offering lookup must reach the provider") + for _, z := range api.lookupZones { + assert.Nil(t, z, "a regional lookup must not name an AZ") + } + assert.True(t, quoted) + require.Len(t, result.Pending, 1) +} + +// A zonal RI whose AZ is missing is refused by the provider before any AWS +// call; the refusal reaches the result as the skip reason, not a silent drop +// and not a quote in an arbitrary zone. +func TestExecuteRIExchangeReshape_ZonalRIWithoutAZRefused(t *testing.T) { + result, api, quoted := runZoneReshape(t, zoneTestRI(ec2types.ScopeAvailabilityZone, "")) + + assert.Empty(t, api.lookupZones, "no offering lookup without an AZ") + assert.False(t, quoted, "no quote without an AZ") + assert.Empty(t, result.Pending) + require.NotEmpty(t, result.Skipped) + for _, s := range result.Skipped { + assert.Equal(t, zoneTestRIID, s.SourceRIID) + assert.Contains(t, s.Reason, "zonal RI offering needs an availability zone") + } +} + +// fakeOfferingFinder records the params the lookup adapter sends. +type fakeOfferingFinder struct { + got ec2svc.FindConvertibleOfferingParams +} + +func (f *fakeOfferingFinder) FindConvertibleOffering(_ context.Context, p ec2svc.FindConvertibleOfferingParams) (string, error) { + f.got = p + return "offering-1", nil +} + +func TestNewConvertibleOfferingLookup_MapsEveryField(t *testing.T) { + f := &fakeOfferingFinder{} + id, err := newConvertibleOfferingLookup(f)(context.Background(), exchange.LookupOfferingParams{ + InstanceType: "m5.xlarge", ProductDescription: "Linux/UNIX", Tenancy: "default", + Scope: "Availability Zone", AvailabilityZone: "us-east-1b", Duration: 31536000, + }) + require.NoError(t, err) + assert.Equal(t, "offering-1", id) + assert.Equal(t, ec2svc.FindConvertibleOfferingParams{ + InstanceType: "m5.xlarge", ProductDescription: "Linux/UNIX", Tenancy: "default", + Scope: "Availability Zone", AvailabilityZone: "us-east-1b", Duration: 31536000, + }, f.got) +} + +func TestConvertForAutoExchange_CarriesAvailabilityZone(t *testing.T) { + _, _, meta := convertForAutoExchange([]ec2svc.ConvertibleRI{ + {ReservedInstanceID: "ri-zonal", Scope: "Availability Zone", AvailabilityZone: "us-east-1b"}, + {ReservedInstanceID: "ri-regional", Scope: "Region"}, + }, nil) + assert.Equal(t, "us-east-1b", meta["ri-zonal"].AvailabilityZone) + assert.Empty(t, meta["ri-regional"].AvailabilityZone) +}