Skip to content

refactor(aws): use common.ErrOutcomeUnknown and drop the internal sentinel #335

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
cristim merged 3 commits into from
Oct 10, 2026
Merged

refactor(aws): use common.ErrOutcomeUnknown and drop the internal sentinel #335

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b20c420
refactor(aws): use common.ErrOutcomeUnknown and drop the internal sen…
cristim Oct 10, 2026
35978de
Merge branch 'main' into feat/301-switch-aws-sentinel
cristim Oct 10, 2026
9788bd7
Merge branch 'main' into feat/301-switch-aws-sentinel
cristim Oct 10, 2026
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 ci_cd_sanity_tests/go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ require (

require (
github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armresources v1.2.0
github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261009141026-a450912d1d4e
github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261009181525-30bf383795db
github.com/aws/aws-sdk-go-v2/service/sts v1.33.17
)

Expand Down
4 changes: 2 additions & 2 deletions ci_cd_sanity_tests/go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ github.com/AzureAD/microsoft-authentication-extensions-for-go/cache v0.1.1 h1:WJ
github.com/AzureAD/microsoft-authentication-extensions-for-go/cache v0.1.1/go.mod h1:tCcJZ0uHAmvjsVYzEFivsRTN00oz5BEsRgQHu5JZ9WE=
github.com/AzureAD/microsoft-authentication-library-for-go v1.6.0 h1:XRzhVemXdgvJqCH0sFfrBUTnUJSBrBf7++ypk+twtRs=
github.com/AzureAD/microsoft-authentication-library-for-go v1.6.0/go.mod h1:HKpQxkWaGLJ+D/5H8QRpyQXA1eKjxkFlOMwck5+33Jk=
github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261009141026-a450912d1d4e h1:guLTNuzHesGLDTCvEjz91ClEpzN9PT8CHPWDnynRKww=
github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261009141026-a450912d1d4e/go.mod h1:8RTmzPazA8IeVZfn7wNaU7ILx0VK5eHZQozO21Mvtq8=
github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261009181525-30bf383795db h1:d/eTZP31KewqE049NlQM3nZIUygRudHJ25Im2ibJk58=
github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261009181525-30bf383795db/go.mod h1:8RTmzPazA8IeVZfn7wNaU7ILx0VK5eHZQozO21Mvtq8=
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=
Expand Down
2 changes: 1 addition & 1 deletion providers/aws/go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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-20261009141026-a450912d1d4e
github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261009181525-30bf383795db
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
Expand Down
4 changes: 2 additions & 2 deletions providers/aws/go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261009141026-a450912d1d4e h1:guLTNuzHesGLDTCvEjz91ClEpzN9PT8CHPWDnynRKww=
github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261009141026-a450912d1d4e/go.mod h1:8RTmzPazA8IeVZfn7wNaU7ILx0VK5eHZQozO21Mvtq8=
github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261009181525-30bf383795db h1:d/eTZP31KewqE049NlQM3nZIUygRudHJ25Im2ibJk58=
github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261009181525-30bf383795db/go.mod h1:8RTmzPazA8IeVZfn7wNaU7ILx0VK5eHZQozO21Mvtq8=
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=
Expand Down
14 changes: 4 additions & 10 deletions providers/aws/internal/purchasecfg/outcome.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,19 +5,13 @@ import (
"fmt"
"net/http"

"github.com/LeanerCloud/cloud-commitments-go/pkg/common"
"github.com/aws/aws-sdk-go-v2/aws/retry"
awshttp "github.com/aws/aws-sdk-go-v2/aws/transport/http"
"github.com/aws/smithy-go"
)

// ErrOutcomeUnknown marks a purchase whose request may have reached AWS but
// whose result is not known: the commitment may already exist. Services whose
// purchase API has no idempotency token or caller-chosen ID (EC2, Redshift)
// must not be re-sent automatically after such an error; reconcile against
// DescribeReservedInstances / DescribeReservedNodes first.
var ErrOutcomeUnknown = errors.New("purchase outcome unknown")

// ClassifyPurchaseError wraps err with ErrOutcomeUnknown unless AWS definitely
// ClassifyPurchaseError wraps err with common.ErrOutcomeUnknown unless AWS definitely
// rejected the request before processing it. It errs toward "unknown": a false
// unknown makes a human check, a false definite allows a double-buy.
//
Expand All @@ -29,7 +23,7 @@ var ErrOutcomeUnknown = errors.New("purchase outcome unknown")
// cancellation, 5xx, a 200 whose body could not be deserialized, and pre-send
// failures (credentials, DNS, endpoint), which are treated conservatively.
func ClassifyPurchaseError(err error) error {
if err == nil || errors.Is(err, ErrOutcomeUnknown) {
if err == nil || errors.Is(err, common.ErrOutcomeUnknown) {
return err
}
var apiErr smithy.APIError
Expand All @@ -41,7 +35,7 @@ func ClassifyPurchaseError(err error) error {
return err
}
}
return fmt.Errorf("%w: the request may have been sent and the commitment may exist; reconcile before retrying: %w", ErrOutcomeUnknown, err)
return fmt.Errorf("%w: the request may have been sent and the commitment may exist; reconcile before retrying: %w", common.ErrOutcomeUnknown, err)
}

// httpStatus returns the HTTP status carried by err, or 0 when there is none.
Expand Down
3 changes: 2 additions & 1 deletion providers/aws/internal/purchasecfg/outcome_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"net/http"
"testing"

"github.com/LeanerCloud/cloud-commitments-go/pkg/common"
"github.com/aws/aws-sdk-go-v2/aws/retry"
awshttp "github.com/aws/aws-sdk-go-v2/aws/transport/http"
"github.com/aws/smithy-go"
Expand Down Expand Up @@ -63,7 +64,7 @@ func TestClassifyPurchaseError(t *testing.T) {
assert.NoError(t, got)
return
}
assert.Equal(t, tc.unknown, errors.Is(got, ErrOutcomeUnknown))
assert.Equal(t, tc.unknown, errors.Is(got, common.ErrOutcomeUnknown))
assert.ErrorIs(t, got, tc.err, "the original error must stay in the chain")
})
}
Expand Down
2 changes: 1 addition & 1 deletion providers/aws/services/ec2/client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ func (c *Client) PurchaseCommitment(ctx context.Context, rec common.Recommendati
}
} else {
// A 200 without an ID: the buy most likely happened.
result.Error = fmt.Errorf("purchase response was empty: %w", purchasecfg.ErrOutcomeUnknown)
result.Error = fmt.Errorf("purchase response was empty: %w", common.ErrOutcomeUnknown)
return result, result.Error
}

Expand Down
5 changes: 2 additions & 3 deletions providers/aws/services/ec2/purchase_retry_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import (
"testing"

"github.com/LeanerCloud/cloud-commitments-go/pkg/common"
"github.com/LeanerCloud/cloud-commitments-go/providers/aws/internal/purchasecfg"
"github.com/aws/aws-sdk-go-v2/aws"
"github.com/aws/aws-sdk-go-v2/credentials"
"github.com/aws/aws-sdk-go-v2/service/ec2"
Expand Down Expand Up @@ -70,7 +69,7 @@ func TestPurchaseCommitment_LostResponseIsNotRetriedBySDK(t *testing.T) {

require.Error(t, err, "a purchase whose response was lost must surface as an error, not a success")
assert.False(t, result.Success)
assert.ErrorIs(t, err, purchasecfg.ErrOutcomeUnknown, "a lost response must be reported as outcome-unknown, not as a definite failure")
assert.ErrorIs(t, err, common.ErrOutcomeUnknown, "a lost response must be reported as outcome-unknown, not as a definite failure")
assert.EqualValues(t, 1, purchases.Load(), "the SDK must not re-send a non-idempotent EC2 purchase (no ClientToken)")
}

Expand All @@ -91,5 +90,5 @@ func TestPurchaseCommitment_EmptyResponseIsOutcomeUnknown(t *testing.T) {
rec := common.Recommendation{ResourceType: "t3.micro", Count: 1, PaymentOption: "partial-upfront", Term: "3yr",
Details: &common.ComputeDetails{Platform: "Linux/UNIX", Tenancy: "default", Scope: "Region"}}
_, err := c.PurchaseCommitment(context.Background(), rec, common.PurchaseOptions{})
assert.ErrorIs(t, err, purchasecfg.ErrOutcomeUnknown)
assert.ErrorIs(t, err, common.ErrOutcomeUnknown)
}
2 changes: 1 addition & 1 deletion providers/aws/services/redshift/client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ func (c *Client) PurchaseCommitment(ctx context.Context, rec common.Recommendati
}
} else {
// A 200 without a node: the buy most likely happened.
result.Error = fmt.Errorf("purchase response was empty: %w", purchasecfg.ErrOutcomeUnknown)
result.Error = fmt.Errorf("purchase response was empty: %w", common.ErrOutcomeUnknown)
return result, result.Error
}

Expand Down
5 changes: 2 additions & 3 deletions providers/aws/services/redshift/purchase_retry_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import (
"testing"

"github.com/LeanerCloud/cloud-commitments-go/pkg/common"
"github.com/LeanerCloud/cloud-commitments-go/providers/aws/internal/purchasecfg"
"github.com/aws/aws-sdk-go-v2/aws"
"github.com/aws/aws-sdk-go-v2/credentials"
"github.com/aws/aws-sdk-go-v2/service/redshift"
Expand Down Expand Up @@ -69,7 +68,7 @@ func TestPurchaseCommitment_LostResponseIsNotRetriedBySDK(t *testing.T) {

require.Error(t, err, "a purchase whose response was lost must surface as an error, not a success")
assert.False(t, result.Success)
assert.ErrorIs(t, err, purchasecfg.ErrOutcomeUnknown, "a lost response must be reported as outcome-unknown, not as a definite failure")
assert.ErrorIs(t, err, common.ErrOutcomeUnknown, "a lost response must be reported as outcome-unknown, not as a definite failure")
assert.EqualValues(t, 1, purchases.Load(), "the SDK must not re-send a non-idempotent Redshift purchase (no ClientToken)")
}

Expand All @@ -90,5 +89,5 @@ func TestPurchaseCommitment_EmptyResponseIsOutcomeUnknown(t *testing.T) {
rec := common.Recommendation{ResourceType: "dc2.large", Count: 1, PaymentOption: "all-upfront", Term: "1yr",
Details: common.DataWarehouseDetails{NodeType: "dc2.large", NumberOfNodes: 1}}
_, err := c.PurchaseCommitment(context.Background(), rec, common.PurchaseOptions{})
assert.ErrorIs(t, err, purchasecfg.ErrOutcomeUnknown)
assert.ErrorIs(t, err, common.ErrOutcomeUnknown)
}
2 changes: 1 addition & 1 deletion providers/azure/go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ require (
github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armsubscriptions v1.3.0
github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/search/armsearch v1.4.0
github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/sql/armsql v1.2.0
github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261009141026-a450912d1d4e
github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261009181525-30bf383795db
github.com/stretchr/testify v1.11.1
golang.org/x/sync v0.23.0
)
Expand Down
4 changes: 2 additions & 2 deletions providers/azure/go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@ github.com/AzureAD/microsoft-authentication-extensions-for-go/cache v0.1.1 h1:WJ
github.com/AzureAD/microsoft-authentication-extensions-for-go/cache v0.1.1/go.mod h1:tCcJZ0uHAmvjsVYzEFivsRTN00oz5BEsRgQHu5JZ9WE=
github.com/AzureAD/microsoft-authentication-library-for-go v1.6.0 h1:XRzhVemXdgvJqCH0sFfrBUTnUJSBrBf7++ypk+twtRs=
github.com/AzureAD/microsoft-authentication-library-for-go v1.6.0/go.mod h1:HKpQxkWaGLJ+D/5H8QRpyQXA1eKjxkFlOMwck5+33Jk=
github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261009141026-a450912d1d4e h1:guLTNuzHesGLDTCvEjz91ClEpzN9PT8CHPWDnynRKww=
github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261009141026-a450912d1d4e/go.mod h1:8RTmzPazA8IeVZfn7wNaU7ILx0VK5eHZQozO21Mvtq8=
github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261009181525-30bf383795db h1:d/eTZP31KewqE049NlQM3nZIUygRudHJ25Im2ibJk58=
github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261009181525-30bf383795db/go.mod h1:8RTmzPazA8IeVZfn7wNaU7ILx0VK5eHZQozO21Mvtq8=
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/smithy-go v1.24.2 h1:FzA3bu/nt/vDvmnkg+R8Xl46gmzEDam6mZ1hzmwXFng=
Expand Down
2 changes: 1 addition & 1 deletion providers/gcp/go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ require (
cloud.google.com/go/redis v1.18.3
cloud.google.com/go/resourcemanager v1.10.7
cloud.google.com/go/storage v1.59.2
github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261009141026-a450912d1d4e
github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261009181525-30bf383795db
github.com/googleapis/gax-go/v2 v2.21.0
github.com/stretchr/testify v1.11.1
golang.org/x/oauth2 v0.36.0
Expand Down
4 changes: 2 additions & 2 deletions providers/gcp/go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,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-20261009141026-a450912d1d4e h1:guLTNuzHesGLDTCvEjz91ClEpzN9PT8CHPWDnynRKww=
github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261009141026-a450912d1d4e/go.mod h1:8RTmzPazA8IeVZfn7wNaU7ILx0VK5eHZQozO21Mvtq8=
github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261009181525-30bf383795db h1:d/eTZP31KewqE049NlQM3nZIUygRudHJ25Im2ibJk58=
github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261009181525-30bf383795db/go.mod h1:8RTmzPazA8IeVZfn7wNaU7ILx0VK5eHZQozO21Mvtq8=
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/smithy-go v1.24.2 h1:FzA3bu/nt/vDvmnkg+R8Xl46gmzEDam6mZ1hzmwXFng=
Expand Down
Loading