fix(cmd): exit non-zero and count Savings Plan failures after a purchase run

cmd/multi_service.go

@@ -266,7 +266,7 @@ func executeAndReport(ctx context.Context, awsCfg aws.Config, passed []common.Re
 	if auditErr != nil {
 		AppLogger.Print(auditStopMessage(len(allResults), len(passed), allResults, auditErr))
 	}
-	return auditErr
+	return errors.Join(auditErr, purchaseFailureError(allResults, isDryRun))
 }
 
 // confirmPurchaseRun asks for confirmation once against the full
@@ -762,7 +762,7 @@ func runToolFromCSV(ctx context.Context, cfg Config) error {
 
 // runCSVPurchases groups the confirmed CSV rows by service and region, buys
 // them, and writes the report and summary for whatever ran. A non-nil error
-// is errAuditTrailLost.
+// is errAuditTrailLost and/or errPurchasesFailed.
 func runCSVPurchases(ctx context.Context, awsCfg aws.Config, recs []common.Recommendation, isDryRun bool, cfg Config, runID string) error {
 	// Create account alias cache for lookup
 	accountCache := NewAccountAliasCache(awsCfg)
@@ -827,7 +827,7 @@ func runCSVPurchases(ctx context.Context, awsCfg aws.Config, recs []common.Recom
 	if auditErr != nil {
 		AppLogger.Print(auditStopMessage(len(allResults), len(recs), allResults, auditErr))
 	}
-	return auditErr
+	return errors.Join(auditErr, purchaseFailureError(allResults, isDryRun))
 }
 
 // processCSVRegionPurchases handles a single (service, region) pair within

cmd/multi_service_stats.go

@@ -1,9 +1,35 @@
 package main
 
 import (
+	"errors"
+	"fmt"
+
 	"github.com/LeanerCloud/cloud-commitments-go/pkg/common"
 )
 
+// errPurchasesFailed marks a real purchase run in which at least one
+// purchase failed. The run exits non-zero so a scripted caller can detect a
+// partial or total failure (#2157).
+var errPurchasesFailed = errors.New("purchases failed")
+
+// purchaseFailureError returns errPurchasesFailed, with counts, when a real
+// run has any failed result. Dry runs buy nothing, so they never fail here.
+func purchaseFailureError(results []common.PurchaseResult, isDryRun bool) error {
+	if isDryRun {
+		return nil
+	}
+	failed := 0
+	for i := range results {
+		if !results[i].Success {
+			failed++
+		}
+	}
+	if failed == 0 {
+		return nil
+	}
+	return fmt.Errorf("%w: %d of %d purchase(s) failed", errPurchasesFailed, failed, len(results))
+}
+
 // ServiceProcessingStats holds statistics for each service.
 type ServiceProcessingStats struct {
 	Service                 common.ServiceType
@@ -73,8 +99,11 @@ func printMultiServiceSummary(allRecommendations []common.Recommendation, allRes
 		printComparisonSection(allRecommendations, riStats, riAggregates.savings)
 	}
 
-	printSuccessRate(riAggregates.success, riAggregates.failed)
-	printFinalMessage(isDryRun, riAggregates.success)
+	// Savings Plan results count alongside RI results (#2157).
+	success := riAggregates.success + spStats.SuccessfulPurchases
+	failed := riAggregates.failed + spStats.FailedPurchases
+	printSuccessRate(success, failed)
+	printFinalMessage(isDryRun, success, failed)
 }
 
 // riAggregateStats holds aggregated RI statistics.
@@ -165,7 +194,8 @@ func printSuccessRate(success, failed int) {
 	totalResults := success + failed
 	if totalResults > 0 {
 		successRate := (float64(success) / float64(totalResults)) * 100
-		AppLogger.Printf("\nOverall success rate: %.1f%%\n", successRate)
+		AppLogger.Printf("\nPurchases: %d succeeded, %d failed\n", success, failed)
+		AppLogger.Printf("Overall success rate: %.1f%%\n", successRate)
 	}
 }
 
@@ -178,10 +208,16 @@ func printSuccessRate(success, failed int) {
 const archeraSignupURL = common.ArcheraSignupURL
 
 // printFinalMessage prints the final message based on mode and results.
-func printFinalMessage(isDryRun bool, riSuccess int) {
+// success and failed cover both Reserved Instances and Savings Plans.
+func printFinalMessage(isDryRun bool, success, failed int) {
 	if isDryRun {
 		AppLogger.Println("\n💡 To actually purchase these commitments, run with --purchase flag")
-	} else if riSuccess > 0 {
+		return
+	}
+	if failed > 0 {
+		AppLogger.Printf("\n⚠️  %d of %d purchase(s) failed; see the per-purchase errors above\n", failed, success+failed)
+	}
+	if success > 0 {
 		AppLogger.Println("\n🎉 Purchase operations completed!")
 		AppLogger.Println("⏰ Allow up to 15 minutes for RIs to appear in your account")
 		printArcheraPitch()

cmd/multi_service_stats_test.go

@@ -559,7 +559,7 @@ func TestPrintFinalMessage(t *testing.T) {
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			output := captureAppOutput(t, func() {
-				printFinalMessage(tt.isDryRun, tt.riSuccess)
+				printFinalMessage(tt.isDryRun, tt.riSuccess, 0)
 			})
 
 			for _, want := range tt.wantOutput {

cmd/purchase_failure_exit_2157_test.go

@@ -0,0 +1,176 @@
+package main
+
+import (
+	"context"
+	"io"
+	"net/http"
+	"strings"
+	"testing"
+
+	"github.com/LeanerCloud/cloud-commitments-go/pkg/common"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// spHookFailing answers the Savings Plan calls like spHook, but fails the
+// CreateSavingsPlan call for the given offering IDs. Every other call (the
+// EC2 RI API included) gets a 400, so an RI purchase through it fails.
+func spHookFailing(failOfferings ...string) *hookTransport {
+	h := &hookTransport{}
+	h.handle = func(r *http.Request) (int, string) {
+		switch {
+		case strings.HasSuffix(r.URL.Path, "/CreateSavingsPlan"):
+			body, _ := io.ReadAll(r.Body)
+			for _, id := range failOfferings {
+				if strings.Contains(string(body), `"`+id+`"`) {
+					return http.StatusBadRequest, `{"__type":"ValidationException","message":"offering rejected"}`
+				}
+			}
+			return http.StatusOK, `{"savingsPlanId":"sp-111"}`
+		case strings.HasSuffix(r.URL.Path, "/DescribeSavingsPlans"):
+			return http.StatusOK, `{"savingsPlans":[]}`
+		}
+		return http.StatusBadRequest, `{"__type":"ValidationException","message":"unexpected call"}`
+	}
+	return h
+}
+
+func TestExecuteAndReport_SavingsPlanFailureFailsRun(t *testing.T) {
+	t.Setenv("DISABLE_PURCHASE_DELAY", "true")
+	out := captureAppLog(t)
+	cfg := auditCfg(t)
+	h := spHookFailing("o-2")
+	recs := []common.Recommendation{offeringSP("o-1", 1), offeringSP("o-2", 2)}
+
+	err := executeAndReport(context.Background(), hookedAWSConfig(h), recs, false, "run-1", cfg, nil)
+
+	require.ErrorIs(t, err, errPurchasesFailed)
+	assert.NotErrorIs(t, err, errAuditTrailLost)
+	assert.Contains(t, err.Error(), "1 of 2")
+	assert.Equal(t, 2, h.count("/CreateSavingsPlan"))
+	assert.Contains(t, out.String(), "Purchases: 1 succeeded, 1 failed")
+	assert.Contains(t, out.String(), "1 of 2 purchase(s) failed")
+}
+
+func TestExecuteAndReport_AllSavingsPlansFailFailsRun(t *testing.T) {
+	t.Setenv("DISABLE_PURCHASE_DELAY", "true")
+	out := captureAppLog(t)
+	cfg := auditCfg(t)
+	h := spHookFailing("o-1", "o-2")
+	recs := []common.Recommendation{offeringSP("o-1", 1), offeringSP("o-2", 2)}
+
+	err := executeAndReport(context.Background(), hookedAWSConfig(h), recs, false, "run-1", cfg, nil)
+
+	require.ErrorIs(t, err, errPurchasesFailed)
+	assert.Contains(t, out.String(), "Purchases: 0 succeeded, 2 failed")
+	assert.NotContains(t, out.String(), "Purchase operations completed")
+}
+
+func TestExecuteAndReport_SavingsPlanAndRIFailuresCountedTogether(t *testing.T) {
+	t.Setenv("DISABLE_PURCHASE_DELAY", "true")
+	out := captureAppLog(t)
+	cfg := auditCfg(t)
+	h := spHookFailing("o-2")
+	recs := []common.Recommendation{offeringSP("o-1", 1), offeringSP("o-2", 2), validEC2Rec()}
+
+	err := executeAndReport(context.Background(), hookedAWSConfig(h), recs, false, "run-1", cfg, nil)
+
+	require.ErrorIs(t, err, errPurchasesFailed)
+	assert.Contains(t, err.Error(), "2 of 3")
+	assert.Contains(t, out.String(), "Purchases: 1 succeeded, 2 failed")
+	assert.Contains(t, out.String(), "Overall success rate: 33.3%")
+}
+
+func TestExecuteAndReport_AllSavingsPlansSucceedReturnsNil(t *testing.T) {
+	t.Setenv("DISABLE_PURCHASE_DELAY", "true")
+	out := captureAppLog(t)
+	cfg := auditCfg(t)
+	h := spHookFailing()
+	recs := []common.Recommendation{offeringSP("o-1", 1), offeringSP("o-2", 2)}
+
+	err := executeAndReport(context.Background(), hookedAWSConfig(h), recs, false, "run-1", cfg, nil)
+
+	require.NoError(t, err)
+	assert.Contains(t, out.String(), "Purchases: 2 succeeded, 0 failed")
+	assert.Contains(t, out.String(), "Purchase operations completed")
+	assert.NotContains(t, out.String(), "purchase(s) failed")
+}
+
+func TestExecuteAndReport_DryRunPreconditionFailureReturnsNil(t *testing.T) {
+	out := captureAppLog(t)
+	cfg := auditCfg(t)
+	h := spHookFailing()
+	bad := offeringSP("o-1", 1)
+	bad.Details = nil // fails the purchase preconditions
+
+	err := executeAndReport(context.Background(), hookedAWSConfig(h), []common.Recommendation{bad}, true, "run-1", cfg, nil)
+
+	require.NoError(t, err, "a dry run buys nothing, so it does not fail the run")
+	assert.Zero(t, h.count("/CreateSavingsPlan"))
+	assert.Contains(t, out.String(), "cannot purchase", "the precondition failure is still reported")
+}
+
+func TestRunCSVPurchases_SavingsPlanFailureFailsRun(t *testing.T) {
+	t.Setenv("DISABLE_PURCHASE_DELAY", "true")
+	out := captureAppLog(t)
+	cfg := auditCfg(t)
+	h := spHookFailing("o-2")
+	recs := []common.Recommendation{offeringSP("o-1", 1), offeringSP("o-2", 2)}
+
+	err := runCSVPurchases(context.Background(), hookedAWSConfig(h), recs, false, cfg, "run-1")
+
+	require.ErrorIs(t, err, errPurchasesFailed)
+	assert.Contains(t, err.Error(), "1 of 2")
+	assert.Contains(t, out.String(), "Purchases: 1 succeeded, 1 failed")
+}
+
+func TestRunCSVPurchases_AllSucceedReturnsNil(t *testing.T) {
+	t.Setenv("DISABLE_PURCHASE_DELAY", "true")
+	out := captureAppLog(t)
+	cfg := auditCfg(t)
+	h := spHookFailing()
+	recs := []common.Recommendation{offeringSP("o-1", 1), offeringSP("o-2", 2)}
+
+	err := runCSVPurchases(context.Background(), hookedAWSConfig(h), recs, false, cfg, "run-1")
+
+	require.NoError(t, err)
+	assert.Contains(t, out.String(), "Purchases: 2 succeeded, 0 failed")
+}
+
+func TestPrintMultiServiceSummary_CountsSavingsPlanFailures(t *testing.T) {
+	stats := map[common.ServiceType]ServiceProcessingStats{
+		common.ServiceEC2:                     {Service: common.ServiceEC2, RecommendationsSelected: 2, SuccessfulPurchases: 1, FailedPurchases: 1},
+		common.ServiceSavingsPlansCompute:     {Service: common.ServiceSavingsPlansCompute, RecommendationsSelected: 2, FailedPurchases: 2},
+		common.ServiceSavingsPlansEC2Instance: {Service: common.ServiceSavingsPlansEC2Instance, RecommendationsSelected: 1, SuccessfulPurchases: 1},
+	}
+
+	output := captureAppOutput(t, func() {
+		printMultiServiceSummary(nil, nil, stats, false)
+	})
+
+	assert.Contains(t, output, "Purchases: 2 succeeded, 3 failed")
+	assert.Contains(t, output, "Overall success rate: 40.0%")
+	assert.Contains(t, output, "3 of 5 purchase(s) failed")
+}
+
+func TestPrintFinalMessage_SavingsPlanOnlySuccessShowsCompletion(t *testing.T) {
+	output := captureAppOutput(t, func() {
+		printMultiServiceSummary(nil, nil, map[common.ServiceType]ServiceProcessingStats{
+			common.ServiceSavingsPlansCompute: {Service: common.ServiceSavingsPlansCompute, RecommendationsSelected: 1, SuccessfulPurchases: 1},
+		}, false)
+	})
+
+	assert.Contains(t, output, "Purchase operations completed")
+	assert.NotContains(t, output, "purchase(s) failed")
+}
+
+func TestPurchaseFailureError(t *testing.T) {
+	ok := common.PurchaseResult{Success: true}
+	bad := common.PurchaseResult{Success: false}
+	assert.NoError(t, purchaseFailureError(nil, false))
+	assert.NoError(t, purchaseFailureError([]common.PurchaseResult{ok, ok}, false))
+	assert.NoError(t, purchaseFailureError([]common.PurchaseResult{bad}, true), "dry run")
+	err := purchaseFailureError([]common.PurchaseResult{ok, bad, bad}, false)
+	require.ErrorIs(t, err, errPurchasesFailed)
+	assert.Contains(t, err.Error(), "2 of 3")
+}