Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ 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-20261018003623-541e05e5fd29
github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261020134525-e237996e45b4
github.com/LeanerCloud/cloud-commitments-go/providers/aws v0.0.0-20261019192600-aeb8a6f7ff51
github.com/LeanerCloud/cloud-commitments-go/providers/azure v0.0.0-20261017134002-3def3f898617
github.com/LeanerCloud/cloud-commitments-go/providers/gcp v0.0.0-20261017134002-3def3f898617
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,8 @@ 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-20261018003623-541e05e5fd29 h1:QJkoZBHkwjvMGplDb1TcuBy3HdYPxlZ+AyR/7QEU8Hc=
github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261018003623-541e05e5fd29/go.mod h1:SCDEgHG6evjPh0RsoXof+J3aMmGXNFMrwbJIle7rMpQ=
github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261020134525-e237996e45b4 h1:tE+KD86HKsxS0js0KfaEo8XURVpi2ly7PMVwcfzzJWk=
github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261020134525-e237996e45b4/go.mod h1:/yCOnBtEJRRydFrDm6g1UcHuJwuB35AqYTBdaHDG8wo=
github.com/LeanerCloud/cloud-commitments-go/providers/aws v0.0.0-20261019192600-aeb8a6f7ff51 h1:fr8mS6aF7/y69cJ5UlF9MEvP15mSlH/4yKg25Cvk/YM=
github.com/LeanerCloud/cloud-commitments-go/providers/aws v0.0.0-20261019192600-aeb8a6f7ff51/go.mod h1:p6TrYRa4+lvcdsPCAJ+sa0tI0sRfGQ/0YT1Af3fZk3w=
github.com/LeanerCloud/cloud-commitments-go/providers/azure v0.0.0-20261017134002-3def3f898617 h1:K9wBX9ac1QwhkjW7BvDt0gl2Kt9fGdDytJyndoPRcBU=
Expand Down
9 changes: 7 additions & 2 deletions internal/api/handler_ri_exchange.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,12 @@ import (
// Cross-family alternatives no longer flow through here — they're
// sourced from the cached AWS Cost Explorer purchase recommendations
// in Postgres via purchaseRecLookupFromStore (see exchange_lookup.go),
// so the EC2 client only needs to enumerate convertible RIs.
// so the EC2 client only needs to enumerate convertible RIs and the
// region's offered instance types (DescribeInstanceTypeOfferings), which
// limit reshape targets to sizes the region actually sells.
type reshapeEC2Client interface {
ListConvertibleReservedInstances(ctx context.Context) ([]ec2svc.ConvertibleRI, error)
GetValidResourceTypes(ctx context.Context) ([]string, error)
}

// reshapeRecsClient is the narrow slice of the recommendations
Expand Down Expand Up @@ -1754,7 +1757,9 @@ func (h *Handler) getReshapeRecommendations(ctx context.Context, req *events.Lam
}
currencyCode := firstNonEmptyCurrency(instances)
lookup := purchaseRecLookupFromStore(h.config, cloudAccountID)
recs := exchange.AnalyzeReshapingWithRecs(ctx, riInfos, utilInfos, p.threshold, p.region, currencyCode, lookup)
offered := OfferedInstanceTypes(ctx, p.region, ec2Client.GetValidResourceTypes)
recs := exchange.AnalyzeReshapingWithRecs(ctx, riInfos, utilInfos, p.threshold, p.region, currencyCode, lookup,
exchange.WithOfferedInstanceTypes(offered))

resp := &ReshapeRecommendationsResponse{Recommendations: recs, ExcludedRIs: excluded}
h.attachReshapeStaleness(ctx, resp)
Expand Down
5 changes: 5 additions & 0 deletions internal/api/handler_ri_exchange_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,11 @@ func (f *fakeReshapeEC2) ListConvertibleReservedInstances(_ context.Context) ([]
return f.instances, nil
}

// GetValidResourceTypes returns no catalog, so reshape keeps the legacy ladder.
func (f *fakeReshapeEC2) GetValidResourceTypes(_ context.Context) ([]string, error) {
return nil, nil
}

// fakeReshapeRecs is a stub for reshapeRecsClient. Counts calls so the
// test can assert cache-hit behavior on the second request.
type fakeReshapeRecs struct {
Expand Down
13 changes: 12 additions & 1 deletion internal/api/handler_ri_exchange_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -905,14 +905,25 @@ func TestRejectRIExchange_EmptyStoredToken(t *testing.T) {
// called and the region-normalization assertion would never fire
// (silent test trap). Mirrors the integration-test fake but lives
// here so this unit test runs without the //go:build integration tag.
//
// offered / offeredErr drive GetValidResourceTypes (the region's
// instance type catalog); offeredCalls counts the catalog lookups.
type fakeReshapeEC2Stub struct {
instances []ec2svc.ConvertibleRI
instances []ec2svc.ConvertibleRI
offered []string
offeredErr error
offeredCalls int
}

func (f *fakeReshapeEC2Stub) ListConvertibleReservedInstances(_ context.Context) ([]ec2svc.ConvertibleRI, error) {
return f.instances, nil
}

func (f *fakeReshapeEC2Stub) GetValidResourceTypes(_ context.Context) ([]string, error) {
f.offeredCalls++
return f.offered, f.offeredErr
}

// fakeReshapeRecsStub returns a configurable RI utilization slice so
// the reshape pipeline runs without a real Cost Explorer dependency.
// AnalyzeReshapingWithRecs only emits a recommendation (and only then
Expand Down
4 changes: 4 additions & 0 deletions internal/api/handler_ri_utilization_scope_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,10 @@ func (f *countingUtilEC2) ListConvertibleReservedInstances(_ context.Context) ([
return f.instances, f.err
}

func (f *countingUtilEC2) GetValidResourceTypes(_ context.Context) ([]string, error) {
return nil, nil
}

type countingUtilRecs struct {
rows []recommendations.RIUtilization
calls int
Expand Down
29 changes: 29 additions & 0 deletions internal/api/reshape_offered_types.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
package api

import (
"context"

"github.com/LeanerCloud/cloud-commitments-go/pkg/logging"
)

// OfferedInstanceTypes returns the EC2 instance types offered in region, for
// exchange.WithOfferedInstanceTypes and RunAutoExchangeParams.OfferedInstanceTypes.
// fetch is normally ec2svc.Client.GetValidResourceTypes
// (DescribeInstanceTypeOfferings).
//
// On a fetch error or an empty catalog it logs a warning naming the region and
// returns nil, so reshape analysis proceeds on the legacy size ladder instead of
// failing the request. Nothing is cached, so the next call queries the catalog
// again rather than reusing the fallback.
func OfferedInstanceTypes(ctx context.Context, region string, fetch func(context.Context) ([]string, error)) []string {
types, err := fetch(ctx)
if err != nil {
logging.Warnf("reshape: EC2 instance type offerings for region %q failed, proceeding without the catalog: %v", region, err)
return nil
}
if len(types) == 0 {
logging.Warnf("reshape: EC2 instance type offerings for region %q came back empty, proceeding without the catalog", region)
return nil
}
return types
}
179 changes: 179 additions & 0 deletions internal/api/reshape_offered_types_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,179 @@
package api

import (
"bytes"
"context"
"errors"
"sync"
"testing"

"github.com/aws/aws-lambda-go/events"
"github.com/aws/aws-sdk-go-v2/aws"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require"

"github.com/LeanerCloud/cloud-commitments-go/pkg/logging"
"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"
)

// lockedBuffer is an io.Writer safe for the logger's concurrent writes.
type lockedBuffer struct {
mu sync.Mutex
buf bytes.Buffer
}

func (b *lockedBuffer) Write(p []byte) (int, error) {
b.mu.Lock()
defer b.mu.Unlock()
return b.buf.Write(p)
}

func (b *lockedBuffer) String() string {
b.mu.Lock()
defer b.mu.Unlock()
return b.buf.String()
}

// captureWarnings routes the default pkg/logging logger into a buffer at warn
// level for the rest of the test. Tests using it must not call t.Parallel.
func captureWarnings(t *testing.T) *lockedBuffer {
t.Helper()
out := &lockedBuffer{}
prevOut := logging.SetOutput(out)
prevLevel := logging.GetLevel()
logging.SetLevelValue(logging.LevelWarn)
t.Cleanup(func() {
logging.SetOutput(prevOut)
logging.SetLevelValue(prevLevel)
})
return out
}

const reshapeCatalogRegion = "eu-west-3"

// newCatalogReshapeHandler builds a reshape handler over one convertible
// 1x m5.4xlarge RI (32 normalized units) at 50% utilization (16 used). On
// the legacy ladder that reshapes to 1x m5.2xlarge; with a catalog lacking
// m5.2xlarge it must become 2x m5.xlarge.
func newCatalogReshapeHandler(ec2 *fakeReshapeEC2Stub) *Handler {
ec2.instances = []ec2svc.ConvertibleRI{
{ReservedInstanceID: "ri-1", InstanceType: "m5.4xlarge", InstanceCount: 1, CurrencyCode: "USD", Duration: 31536000},
}
mockStore := &MockConfigStore{}
mockStore.On("ListStoredRecommendations", mock.Anything, mock.Anything).
Return([]config.RecommendationRecord(nil), nil)
h := &Handler{
config: mockStore,
auth: &mockAuthForExchange{},
reshapeEC2Factory: func(_ aws.Config) reshapeEC2Client { return ec2 },
reshapeRecsFactory: func(_ aws.Config) reshapeRecsClient {
return &fakeReshapeRecsStub{utilization: []recommendations.RIUtilization{
{ReservedInstanceID: "ri-1", UtilizationPercent: 50.0},
}}
},
reshapeAccountResolver: func(_ context.Context) (string, error) { return "", nil },
}
h.awsCfgOnce.Do(func() { h.awsCfg = aws.Config{Region: reshapeCatalogRegion} })
return h
}

func reshapeOnce(t *testing.T, h *Handler) *ReshapeRecommendationsResponse {
t.Helper()
req := &events.LambdaFunctionURLRequest{
Headers: map[string]string{"authorization": "Bearer test-token"},
QueryStringParameters: map[string]string{"lookback_days": "30", "threshold": "95.0"},
}
out, err := h.getReshapeRecommendations(context.Background(), req)
require.NoError(t, err)
resp, ok := out.(*ReshapeRecommendationsResponse)
require.True(t, ok)
require.Len(t, resp.Recommendations, 1)
return resp
}

func TestGetReshapeRecommendations_CatalogLimitsTargetToOfferedSizes(t *testing.T) {
ec2 := &fakeReshapeEC2Stub{offered: []string{"m5.4xlarge", "m5.xlarge", "c5.large"}}
rec := reshapeOnce(t, newCatalogReshapeHandler(ec2)).Recommendations[0]

assert.Equal(t, 1, ec2.offeredCalls, "the reshape handler must query the region's instance type catalog")
assert.Equal(t, "m5.xlarge", rec.TargetInstanceType,
"m5.2xlarge is not offered, so the target must step down to an offered size")
assert.Equal(t, int32(2), rec.TargetCount)
}

func TestGetReshapeRecommendations_CatalogMissingFamilySurfacesReason(t *testing.T) {
ec2 := &fakeReshapeEC2Stub{offered: []string{"c5.large", "c5.xlarge"}}
rec := reshapeOnce(t, newCatalogReshapeHandler(ec2)).Recommendations[0]

assert.Empty(t, rec.TargetInstanceType, "an unoffered family must not get a target")
assert.Zero(t, rec.TargetCount)
assert.Contains(t, rec.Reason, "instance family not offered in region (m5)")
}

// A catalog error or an empty catalog falls back to the legacy ladder with a
// warning naming the region, and the fallback is not cached: the next request
// queries the catalog again.
func TestGetReshapeRecommendations_CatalogUnavailableFallsBackWithWarning(t *testing.T) {
cases := []struct {
name string
ec2 *fakeReshapeEC2Stub
warning string
}{
{
name: "catalog error",
ec2: &fakeReshapeEC2Stub{offeredErr: errors.New("UnauthorizedOperation: ec2:DescribeInstanceTypeOfferings")},
warning: "failed, proceeding without the catalog: UnauthorizedOperation",
},
{
name: "empty catalog",
ec2: &fakeReshapeEC2Stub{offered: []string{}},
warning: "came back empty, proceeding without the catalog",
},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
logs := captureWarnings(t)
h := newCatalogReshapeHandler(tc.ec2)

for i := 1; i <= 2; i++ {
rec := reshapeOnce(t, h).Recommendations[0]
assert.Equal(t, "m5.2xlarge", rec.TargetInstanceType, "request %d must use the legacy ladder", i)
assert.Equal(t, int32(1), rec.TargetCount, "request %d", i)
assert.Equal(t, i, tc.ec2.offeredCalls, "request %d must query the catalog again, not reuse a cached fallback", i)
}

out := logs.String()
assert.Contains(t, out, `region "`+reshapeCatalogRegion+`"`)
assert.Contains(t, out, tc.warning)
})
}
}

func TestOfferedInstanceTypes(t *testing.T) {
ctx := context.Background()
t.Run("returns the catalog unchanged", func(t *testing.T) {
logs := captureWarnings(t)
got := OfferedInstanceTypes(ctx, "us-east-1", func(context.Context) ([]string, error) {
return []string{"m5.large", "c5.xlarge"}, nil
})
assert.Equal(t, []string{"m5.large", "c5.xlarge"}, got)
assert.Empty(t, logs.String(), "a usable catalog must not log a warning")
})
t.Run("error returns nil and warns", func(t *testing.T) {
logs := captureWarnings(t)
got := OfferedInstanceTypes(ctx, "us-east-1", func(context.Context) ([]string, error) {
return []string{"m5.large"}, errors.New("throttled")
})
assert.Nil(t, got, "a partial result next to an error must not be used as the catalog")
assert.Contains(t, logs.String(), `region "us-east-1" failed, proceeding without the catalog: throttled`)
})
t.Run("empty returns nil and warns", func(t *testing.T) {
logs := captureWarnings(t)
got := OfferedInstanceTypes(ctx, "ap-south-2", func(context.Context) ([]string, error) { return nil, nil })
assert.Nil(t, got)
assert.Contains(t, logs.String(), `region "ap-south-2" came back empty`)
})
}
Loading
Loading