From 76107f32d8e356266e86126db0eda59a12e1317d Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Sat, 17 Oct 2026 04:10:55 +0200 Subject: [PATCH 1/3] build(aws): bump the pkg pin past ExchangeOutcome.OutcomeUnknown Co-Authored-By: Claude Opus 5.5 --- providers/aws/go.mod | 2 +- providers/aws/go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/providers/aws/go.mod b/providers/aws/go.mod index 29ad8236..147e29a7 100644 --- a/providers/aws/go.mod +++ b/providers/aws/go.mod @@ -3,7 +3,7 @@ module github.com/LeanerCloud/cloud-commitments-go/providers/aws go 1.26.9 require ( - github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261016194019-89772cf80f67 + github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261017020613-36d8b1100c05 github.com/aws/aws-sdk-go-v2 v1.41.5 github.com/aws/aws-sdk-go-v2/config v1.29.12 github.com/aws/aws-sdk-go-v2/credentials v1.17.65 diff --git a/providers/aws/go.sum b/providers/aws/go.sum index 33275e77..78318a17 100644 --- a/providers/aws/go.sum +++ b/providers/aws/go.sum @@ -1,5 +1,5 @@ -github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261016194019-89772cf80f67 h1:zpr+lPSdlXeA9ySU7FqHtgn4ADinmRhfJ5Y6jm9ankw= -github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261016194019-89772cf80f67/go.mod h1:SCDEgHG6evjPh0RsoXof+J3aMmGXNFMrwbJIle7rMpQ= +github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261017020613-36d8b1100c05 h1:YtSRyJHVUTDnxykCGYpGX7Pt/z+mq4/sCphPmO6Pj5M= +github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261017020613-36d8b1100c05/go.mod h1:SCDEgHG6evjPh0RsoXof+J3aMmGXNFMrwbJIle7rMpQ= github.com/aws/aws-sdk-go-v2 v1.41.5 h1:dj5kopbwUsVUVFgO4Fi5BIT3t4WyqIDjGKCangnV/yY= github.com/aws/aws-sdk-go-v2 v1.41.5/go.mod h1:mwsPRE8ceUUpiTgF7QmQIJ7lgsKUPQOUl3o72QBrE1o= github.com/aws/aws-sdk-go-v2/config v1.29.12 h1:Y/2a+jLPrPbHpFkpAAYkVEtJmxORlXoo5k2g1fa2sUo= From 36c2bbde30c4ef7f9354a71cf335e2fbc0a03304 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Sat, 17 Oct 2026 04:11:49 +0200 Subject: [PATCH 2/3] fix(aws/ladder): report an exchange with an unknown accept outcome as unknown, not failed Closes #443 Co-Authored-By: Claude Opus 5.5 --- providers/aws/ladder/reshape.go | 18 ++++++++++++++---- providers/aws/ladder/reshape_test.go | 23 +++++++++++++++++++++++ 2 files changed, 37 insertions(+), 4 deletions(-) diff --git a/providers/aws/ladder/reshape.go b/providers/aws/ladder/reshape.go index 0cd2019e..4708fb5f 100644 --- a/providers/aws/ladder/reshape.go +++ b/providers/aws/ladder/reshape.go @@ -165,6 +165,7 @@ func summarizeExchangeResult(result *exchange.AutoExchangeResult) (ladder.Reshap len(result.Completed)+len(result.Pending)+len(result.Failed)+len(result.Skipped)), } + unknown := 0 for i := range result.Completed { o := &result.Completed[i] summary.Details = append(summary.Details, fmt.Sprintf( @@ -179,9 +180,14 @@ func summarizeExchangeResult(result *exchange.AutoExchangeResult) (ladder.Reshap } for i := range result.Failed { o := &result.Failed[i] + label := "failed" + if o.OutcomeUnknown { + unknown++ + label = "outcome unknown, reconcile before retrying" + } summary.Details = append(summary.Details, fmt.Sprintf( - "failed: %s (%s) -> %s: %s", - o.SourceRIID, o.SourceInstanceType, o.TargetInstanceType, o.Error)) + "%s: %s (%s) -> %s: %s", + label, o.SourceRIID, o.SourceInstanceType, o.TargetInstanceType, o.Error)) } for i := range result.Skipped { s := &result.Skipped[i] @@ -194,9 +200,13 @@ func summarizeExchangeResult(result *exchange.AutoExchangeResult) (ladder.Reshap // summary.Analyzed: pending and skipped items were never attempted, // and an inflated denominator would misread during an incident. attempts := len(result.Completed) + len(result.Failed) + suffix := "" + if unknown > 0 { + suffix = fmt.Sprintf("; %d with unknown outcome", unknown) + } return summary, fmt.Errorf( - "ReshapeBuffer: %d of %d exchange attempt(s) failed (first: %s: %s); see summary details for the full list", - len(result.Failed), attempts, result.Failed[0].SourceRIID, result.Failed[0].Error) + "ReshapeBuffer: %d of %d exchange attempt(s) failed (first: %s: %s)%s; see summary details for the full list", + len(result.Failed), attempts, result.Failed[0].SourceRIID, result.Failed[0].Error, suffix) } return summary, nil } diff --git a/providers/aws/ladder/reshape_test.go b/providers/aws/ladder/reshape_test.go index 1e3c5638..caba85ed 100644 --- a/providers/aws/ladder/reshape_test.go +++ b/providers/aws/ladder/reshape_test.go @@ -260,6 +260,7 @@ func TestReshapeBuffer_PartialFailure_SummaryPlusError(t *testing.T) { assert.Contains(t, err.Error(), "1 of 2 exchange attempt(s) failed", "denominator must be completed+failed attempts, not Analyzed") assert.Contains(t, err.Error(), "ri-bad") + assert.NotContains(t, err.Error(), "unknown outcome", "suffix only when K > 0") // The summary is still populated for audit alongside the error. assert.Equal(t, 3, summary.Analyzed) @@ -310,3 +311,25 @@ func TestReshapeBuffer_WrongScope_ReturnsError(t *testing.T) { assert.Contains(t, err.Error(), "does not match configured account") assert.Equal(t, 0, ex.calls) } + +func TestReshapeBuffer_UnknownOutcome_RenderedAsUnknownNotFailed(t *testing.T) { + ex := &fakeExchangeRunner{result: &exchange.AutoExchangeResult{ + Mode: string(exchange.ExchangeModeAuto), + Failed: []exchange.ExchangeOutcome{ + {SourceRIID: "ri-unk", SourceInstanceType: "c5.large", TargetInstanceType: "c5.xlarge", Error: "accept timed out", OutcomeUnknown: true}, + {SourceRIID: "ri-bad", SourceInstanceType: "m5.large", TargetInstanceType: "m5.xlarge", Error: "rejected"}, + }, + }} + a := newWiredLadder(t, &fakePurchaser{}, &fakePurchaser{}, ex) + + summary, err := a.ReshapeBuffer(context.Background(), testScope(), validReshapeCfg()) + require.Error(t, err) + assert.Contains(t, err.Error(), "2 of 2 exchange attempt(s) failed (first: ri-unk: accept timed out)") + assert.Contains(t, err.Error(), "; 1 with unknown outcome") + + require.Len(t, summary.Details, 2) + assert.Contains(t, summary.Details[0], "outcome unknown, reconcile before retrying: ri-unk") + assert.NotContains(t, summary.Details[0], "failed:") + assert.Contains(t, summary.Details[1], "failed: ri-bad") + assert.NotContains(t, summary.Details[1], "outcome unknown") +} From d995f1039695fd6dba4248580b697fd233446716 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Sat, 17 Oct 2026 04:19:41 +0200 Subject: [PATCH 3/3] fix(aws/ladder): wrap ErrOutcomeUnknown in the ReshapeBuffer error Callers can now errors.Is(err, common.ErrOutcomeUnknown) to tell a reshape run with at least one unknown-outcome exchange (needs reconciliation before retry) from definite failures. The sentinel is wrapped only when unknown > 0; the message text is unchanged. The "first:" field now names the first unknown-outcome entry when one exists, since that is the one needing reconciliation. Co-Authored-By: Claude Opus 5.5 --- providers/aws/ladder/reshape.go | 20 +++++++++++++------ providers/aws/ladder/reshape_test.go | 29 ++++++++++++++++++++++------ 2 files changed, 37 insertions(+), 12 deletions(-) diff --git a/providers/aws/ladder/reshape.go b/providers/aws/ladder/reshape.go index 4708fb5f..d427a677 100644 --- a/providers/aws/ladder/reshape.go +++ b/providers/aws/ladder/reshape.go @@ -5,6 +5,7 @@ import ( "fmt" "math" + "github.com/LeanerCloud/cloud-commitments-go/pkg/common" "github.com/LeanerCloud/cloud-commitments-go/pkg/exchange" "github.com/LeanerCloud/cloud-commitments-go/pkg/ladder" ) @@ -166,6 +167,7 @@ func summarizeExchangeResult(result *exchange.AutoExchangeResult) (ladder.Reshap } unknown := 0 + firstUnknown := -1 for i := range result.Completed { o := &result.Completed[i] summary.Details = append(summary.Details, fmt.Sprintf( @@ -183,6 +185,9 @@ func summarizeExchangeResult(result *exchange.AutoExchangeResult) (ladder.Reshap label := "failed" if o.OutcomeUnknown { unknown++ + if firstUnknown < 0 { + firstUnknown = i + } label = "outcome unknown, reconcile before retrying" } summary.Details = append(summary.Details, fmt.Sprintf( @@ -200,13 +205,16 @@ func summarizeExchangeResult(result *exchange.AutoExchangeResult) (ladder.Reshap // summary.Analyzed: pending and skipped items were never attempted, // and an inflated denominator would misread during an incident. attempts := len(result.Completed) + len(result.Failed) - suffix := "" - if unknown > 0 { - suffix = fmt.Sprintf("; %d with unknown outcome", unknown) + const format = "ReshapeBuffer: %d of %d exchange attempt(s) failed (first: %s: %s)%s; see summary details for the full list" + if unknown == 0 { + first := &result.Failed[0] + return summary, fmt.Errorf(format, len(result.Failed), attempts, first.SourceRIID, first.Error, "") } - return summary, fmt.Errorf( - "ReshapeBuffer: %d of %d exchange attempt(s) failed (first: %s: %s)%s; see summary details for the full list", - len(result.Failed), attempts, result.Failed[0].SourceRIID, result.Failed[0].Error, suffix) + // The unknown entry is the one needing reconciliation, so name it + // and wrap the sentinel for errors.Is. + first := &result.Failed[firstUnknown] + return summary, fmt.Errorf(format+": %w", len(result.Failed), attempts, first.SourceRIID, first.Error, + fmt.Sprintf("; %d with unknown outcome", unknown), common.ErrOutcomeUnknown) } return summary, nil } diff --git a/providers/aws/ladder/reshape_test.go b/providers/aws/ladder/reshape_test.go index caba85ed..68e8f44e 100644 --- a/providers/aws/ladder/reshape_test.go +++ b/providers/aws/ladder/reshape_test.go @@ -261,6 +261,7 @@ func TestReshapeBuffer_PartialFailure_SummaryPlusError(t *testing.T) { "denominator must be completed+failed attempts, not Analyzed") assert.Contains(t, err.Error(), "ri-bad") assert.NotContains(t, err.Error(), "unknown outcome", "suffix only when K > 0") + require.NotErrorIs(t, err, common.ErrOutcomeUnknown, "definite failures must not claim an unknown outcome") // The summary is still populated for audit alongside the error. assert.Equal(t, 3, summary.Analyzed) @@ -316,20 +317,36 @@ func TestReshapeBuffer_UnknownOutcome_RenderedAsUnknownNotFailed(t *testing.T) { ex := &fakeExchangeRunner{result: &exchange.AutoExchangeResult{ Mode: string(exchange.ExchangeModeAuto), Failed: []exchange.ExchangeOutcome{ - {SourceRIID: "ri-unk", SourceInstanceType: "c5.large", TargetInstanceType: "c5.xlarge", Error: "accept timed out", OutcomeUnknown: true}, {SourceRIID: "ri-bad", SourceInstanceType: "m5.large", TargetInstanceType: "m5.xlarge", Error: "rejected"}, + {SourceRIID: "ri-unk", SourceInstanceType: "c5.large", TargetInstanceType: "c5.xlarge", Error: "accept timed out", OutcomeUnknown: true}, }, }} a := newWiredLadder(t, &fakePurchaser{}, &fakePurchaser{}, ex) summary, err := a.ReshapeBuffer(context.Background(), testScope(), validReshapeCfg()) require.Error(t, err) - assert.Contains(t, err.Error(), "2 of 2 exchange attempt(s) failed (first: ri-unk: accept timed out)") + require.ErrorIs(t, err, common.ErrOutcomeUnknown) + assert.Contains(t, err.Error(), "2 of 2 exchange attempt(s) failed (first: ri-unk: accept timed out)", + "first: names the unknown entry even when a definite failure precedes it") assert.Contains(t, err.Error(), "; 1 with unknown outcome") require.Len(t, summary.Details, 2) - assert.Contains(t, summary.Details[0], "outcome unknown, reconcile before retrying: ri-unk") - assert.NotContains(t, summary.Details[0], "failed:") - assert.Contains(t, summary.Details[1], "failed: ri-bad") - assert.NotContains(t, summary.Details[1], "outcome unknown") + assert.Contains(t, summary.Details[0], "failed: ri-bad") + assert.NotContains(t, summary.Details[0], "outcome unknown") + assert.Contains(t, summary.Details[1], "outcome unknown, reconcile before retrying: ri-unk") + assert.NotContains(t, summary.Details[1], "failed:") +} + +func TestReshapeBuffer_UnknownOnly_WrapsErrOutcomeUnknown(t *testing.T) { + ex := &fakeExchangeRunner{result: &exchange.AutoExchangeResult{ + Mode: string(exchange.ExchangeModeAuto), + Failed: []exchange.ExchangeOutcome{ + {SourceRIID: "ri-unk", SourceInstanceType: "c5.large", TargetInstanceType: "c5.xlarge", Error: "accept timed out", OutcomeUnknown: true}, + }, + }} + a := newWiredLadder(t, &fakePurchaser{}, &fakePurchaser{}, ex) + + _, err := a.ReshapeBuffer(context.Background(), testScope(), validReshapeCfg()) + require.ErrorIs(t, err, common.ErrOutcomeUnknown) + assert.Contains(t, err.Error(), "first: ri-unk: accept timed out") }