Skip to content

Commit ea98e59

Browse files
cristimclaude
andcommitted
fix(csv): keep the MemoryDB engine on CSV replays so dedupe matches
csvDetails had no MemoryDB case, so MemoryDB CSV rows loaded with nil Details and an empty duplicate-check engine, which never matched an existing node's "redis". An --input-csv replay re-bought them. Build CacheDetails from the CSV engine column, as for ElastiCache, and cover run and replay through the real MemoryDB client against the stub. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent d6c7373 commit ea98e59

3 files changed

Lines changed: 74 additions & 9 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/).
1414
### Fixed
1515

1616
- Replaying or retrying an `--input-csv` purchase no longer re-buys an
17-
ElastiCache reservation that the first run already bought: CSV rows now carry
18-
the AWS provider, so the duplicate check matches them. CSV Savings Plan rows
17+
ElastiCache or MemoryDB reservation that the first run already bought: CSV
18+
rows now carry the AWS provider, and MemoryDB rows keep their engine, so the
19+
duplicate check matches them. CSV Savings Plan rows
1920
also now match `--include-regions` and `--exclude-regions` on their plan
2021
region (#2228).
2122
- `configure-gcp`: skipping (or giving an unknown answer at) the "Create Service

‎cmd/csv_real_purchase_test.go‎

Lines changed: 70 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"context"
66
"encoding/json"
77
"fmt"
8+
"maps"
89
"net/http"
910
"net/url"
1011
"os"
@@ -105,6 +106,12 @@ type csvPurchaseEnv struct {
105106
// purchase fixtures. Account stays empty in the CSV; CallerAccountID is set
106107
// because entering at runCSVPurchases skips withCallerAccount.
107108
func newCSVPurchaseEnv(t *testing.T) *csvPurchaseEnv {
109+
t.Helper()
110+
return newCSVPurchaseEnvFor(t, csvPurchases, rdsFixtures(), cacheFixtures())
111+
}
112+
113+
// newCSVPurchaseEnvFor is newCSVPurchaseEnv with its own CSV and fixtures.
114+
func newCSVPurchaseEnvFor(t *testing.T, csv string, fixtureSets ...map[string]awsFixture) *csvPurchaseEnv {
108115
t.Helper()
109116
origCfg := toolCfg
110117
t.Cleanup(func() { toolCfg = origCfg })
@@ -115,14 +122,11 @@ func newCSVPurchaseEnv(t *testing.T) *csvPurchaseEnv {
115122
// purchasecfg.NewConfig swaps any other HTTPClient type for the real network.
116123
_, isStd := awsCfg.HTTPClient.(*http.Client)
117124
require.True(t, isStd, "the fixture transport must ride on *http.Client")
118-
for k, v := range rdsFixtures() {
119-
stub.fixtures[k] = v
120-
}
121-
for k, v := range cacheFixtures() {
122-
stub.fixtures[k] = v
125+
for _, set := range fixtureSets {
126+
maps.Copy(stub.fixtures, set)
123127
}
124128
dir := t.TempDir()
125-
csvPath := writeTestRecommendationsCSV(t, csvPurchases)
129+
csvPath := writeTestRecommendationsCSV(t, csv)
126130
env := &csvPurchaseEnv{stub: stub, log: stub.log, denyHits: denyHits, dir: dir}
127131
env.run = func(report string) {
128132
recs, err := loadRecommendationsFromCSV(csvPath)
@@ -307,3 +311,63 @@ func TestLoadRecommendationsFromCSV_StampsAWSProvider(t *testing.T) {
307311
assert.Equal(t, common.ProviderAWS, rec.Provider, rec.Service)
308312
}
309313
}
314+
315+
const (
316+
memdbHost = "memory-db.eu-west-1.amazonaws.com"
317+
memdbPurchase = "PurchaseReservedNodesOffering"
318+
memdbRespID = "aws-ri-memdb-1"
319+
memdbCSV = `Service,Region,ResourceType,Engine,Deployment,Count,Term,PaymentOption
320+
memorydb,eu-west-1,db.t4g.small,redis,,3,1yr,all-upfront
321+
`
322+
)
323+
324+
// memdbFixtures answers MemoryDB (awsJson1.1) with no existing node, one
325+
// offering and a purchase; existing, when set, is the node a replay finds.
326+
func memdbFixtures(existing bool) map[string]awsFixture {
327+
nodes := `[]`
328+
if existing {
329+
nodes = fmt.Sprintf(`[{"ReservationId":%q,"NodeType":"db.t4g.small","NodeCount":3,"Duration":31536000,"StartTime":%d,"State":"active"}]`,
330+
memdbRespID, time.Now().Unix())
331+
}
332+
return map[string]awsFixture{
333+
"memory-db DescribeReservedNodes": {body: `{"ReservedNodes":` + nodes + `}`},
334+
"memory-db DescribeReservedNodesOfferings": {body: `{"ReservedNodesOfferings":[{"ReservedNodesOfferingId":"memdb-offering-1",` +
335+
`"NodeType":"db.t4g.small","Duration":31536000,"OfferingType":"All Upfront"}]}`},
336+
"memory-db " + memdbPurchase: {body: `{"ReservedNode":{"ReservationId":"` + memdbRespID + `","NodeCount":3}}`},
337+
}
338+
}
339+
340+
// The loader must keep the MemoryDB engine column: the dedupe key of a CSV row
341+
// is only equal to an existing node's ("redis") when Details carry it.
342+
func TestCSVReplay_MemoryDB(t *testing.T) {
343+
env := newCSVPurchaseEnvFor(t, memdbCSV, memdbFixtures(false))
344+
env.run("report1.csv")
345+
assert.Equal(t, []string{"true"}, env.report(t, "report1.csv")["Success"])
346+
assert.Equal(t, []string{memdbRespID}, env.report(t, "report1.csv")["CommitmentID"])
347+
env.stub.requireRequests(t, map[string]int{
348+
memdbHost + " DescribeReservedNodes": 1,
349+
memdbHost + " DescribeReservedNodesOfferings": 1,
350+
memdbHost + " " + memdbPurchase: 1,
351+
})
352+
353+
maps.Copy(env.stub.fixtures, memdbFixtures(true))
354+
env.log.Reset()
355+
env.run("report2.csv")
356+
env.stub.requireRequests(t, map[string]int{
357+
memdbHost + " DescribeReservedNodes": 2,
358+
memdbHost + " DescribeReservedNodesOfferings": 1,
359+
memdbHost + " " + memdbPurchase: 1,
360+
})
361+
assert.Contains(t, env.log.String(), "instances → 0 instances", "the dedup must be what removed the row")
362+
assert.NotContains(t, env.log.String(), "Could not check for existing RIs")
363+
_, err := os.Stat(filepath.Join(env.dir, "report2.csv"))
364+
assert.ErrorIs(t, err, os.ErrNotExist)
365+
assert.Zero(t, env.denyHits.Load())
366+
}
367+
368+
func TestLoadRecommendationsFromCSV_KeepsMemoryDBEngine(t *testing.T) {
369+
recs, err := loadRecommendationsFromCSV(writeTestRecommendationsCSV(t, memdbCSV))
370+
require.NoError(t, err)
371+
require.Len(t, recs, 1)
372+
assert.Equal(t, &common.CacheDetails{Engine: "redis", NodeType: "db.t4g.small"}, recs[0].Details)
373+
}

‎cmd/multi_service_csv_details.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ func csvDetails(rec common.Recommendation, record []string, colIdx map[string]in
6060
AZConfig: getCSVField(record, colIdx, "Deployment"),
6161
InstanceClass: rec.ResourceType,
6262
}, nil
63-
case common.ServiceElastiCache, common.ServiceCache:
63+
case common.ServiceElastiCache, common.ServiceCache, common.ServiceMemoryDB:
6464
return &common.CacheDetails{Engine: engine, NodeType: rec.ResourceType}, nil
6565
case common.ServiceEC2, common.ServiceCompute:
6666
return &common.ComputeDetails{

0 commit comments

Comments
 (0)