diff --git a/cmd/multi_service.go b/cmd/multi_service.go index d43a39bd1..90de40e00 100644 --- a/cmd/multi_service.go +++ b/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 diff --git a/cmd/multi_service_stats.go b/cmd/multi_service_stats.go index 52c6a3811..33bad3330 100644 --- a/cmd/multi_service_stats.go +++ b/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() diff --git a/cmd/multi_service_stats_test.go b/cmd/multi_service_stats_test.go index 5ce253782..38f735243 100644 --- a/cmd/multi_service_stats_test.go +++ b/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 { diff --git a/cmd/purchase_failure_exit_2157_test.go b/cmd/purchase_failure_exit_2157_test.go new file mode 100644 index 000000000..ab39cf37b --- /dev/null +++ b/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") +}