fix(exchange): show unknown-outcome auto-exchanges as needing reconciliation in the notification email

internal/email/ri_exchange_reconcile_test.go

@@ -0,0 +1,156 @@
+package email
+
+import (
+	"context"
+	"strings"
+	"testing"
+
+	"github.com/aws/aws-sdk-go-v2/aws"
+	"github.com/aws/aws-sdk-go-v2/service/sns"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+const (
+	reconcileHeading = "NEEDS RECONCILIATION"
+	completedHeading = "The following RI exchanges have been automatically completed:"
+)
+
+// outcomeUnknownExchangeData is an auto run with one completed exchange, one
+// whose accept outcome is unknown, and one definite failure.
+func outcomeUnknownExchangeData() RIExchangeNotificationData {
+	return RIExchangeNotificationData{
+		DashboardURL: "https://dashboard.example.com",
+		Mode:         "auto",
+		Exchanges: []RIExchangeItem{
+			{RecordID: "rec-done", SourceRIID: "ri-done", SourceInstanceType: "m5.large",
+				TargetInstanceType: "m6i.large", TargetCount: 1, PaymentDue: "12.00", ExchangeID: "exch-done"},
+			{RecordID: "rec-unknown", SourceRIID: "ri-unknown", SourceInstanceType: "c5.large",
+				TargetInstanceType: "c6i.large", TargetCount: 2, PaymentDue: "34.00",
+				Error: "purchase outcome unknown: reconcile before retrying", OutcomeUnknown: true},
+			{RecordID: "rec-failed", SourceRIID: "ri-failed", SourceInstanceType: "r5.large",
+				TargetInstanceType: "r6i.large", TargetCount: 1, PaymentDue: "5.00",
+				Error: "InvalidParameterValue: offering not available"},
+		},
+	}
+}
+
+// splitCompletedEmail returns the reconciliation group and the completed group
+// of a rendered completed email. The reconciliation group comes first.
+func splitCompletedEmail(t *testing.T, body string) (reconcile, completed string) {
+	t.Helper()
+	reconcileAt := strings.Index(body, reconcileHeading)
+	completedAt := strings.Index(body, completedHeading)
+	require.GreaterOrEqual(t, reconcileAt, 0, "no reconciliation group:\n%s", body)
+	require.GreaterOrEqual(t, completedAt, 0, "no completed group:\n%s", body)
+	require.Less(t, reconcileAt, completedAt, "reconciliation group must come first:\n%s", body)
+	return body[reconcileAt:completedAt], body[completedAt:]
+}
+
+func TestRenderRIExchangeCompletedEmail_OutcomeUnknownNeedsReconciliation(t *testing.T) {
+	body, err := RenderRIExchangeCompletedEmail(outcomeUnknownExchangeData())
+	require.NoError(t, err)
+
+	reconcile, completed := splitCompletedEmail(t, body)
+	for _, want := range []string{
+		"ri-unknown (c5.large)",
+		"c6i.large x2",
+		"$34.00",
+		"Record: rec-unknown",
+		"POST /api/ri-exchange/resolve/rec-unknown",
+		"purchase outcome unknown",
+		"do not retry",
+	} {
+		assert.Contains(t, reconcile, want)
+	}
+	assert.NotContains(t, reconcile, "ri-done", "a completed exchange is not unknown")
+	assert.NotContains(t, reconcile, "ri-failed", "a definite failure is not unknown")
+	assert.NotContains(t, completed, "ri-unknown", "an unknown outcome is not a completion")
+	assert.Contains(t, completed, "ri-done")
+	assert.NotContains(t, body, "ri-failed", "definite failures stay out of this email, as before")
+}
+
+// TestRenderRIExchangeCompletedEmail_OutcomeUnknownWithoutError pins that the
+// flag alone decides the group: an unknown outcome with an empty Error must
+// not slip into the completed list.
+func TestRenderRIExchangeCompletedEmail_OutcomeUnknownWithoutError(t *testing.T) {
+	data := outcomeUnknownExchangeData()
+	data.Exchanges[1].Error = ""
+
+	body, err := RenderRIExchangeCompletedEmail(data)
+	require.NoError(t, err)
+
+	reconcile, completed := splitCompletedEmail(t, body)
+	assert.Contains(t, reconcile, "ri-unknown")
+	assert.NotContains(t, completed, "ri-unknown")
+}
+
+func TestRenderRIExchangeCompletedEmail_NoUnknownOutcomeHasNoReconciliationGroup(t *testing.T) {
+	data := outcomeUnknownExchangeData()
+	data.Exchanges[1].OutcomeUnknown = false
+
+	body, err := RenderRIExchangeCompletedEmail(data)
+	require.NoError(t, err)
+
+	assert.NotContains(t, body, reconcileHeading)
+	assert.NotContains(t, body, "/api/ri-exchange/resolve/")
+	assert.Contains(t, body, completedHeading)
+}
+
+func TestRIExchangeCompletedSubject(t *testing.T) {
+	data := outcomeUnknownExchangeData()
+	assert.Equal(t, "CUDly - RI Exchanges Need Reconciliation (1 of 3 exchanges)", riExchangeCompletedSubject(data))
+
+	data.Exchanges[1].OutcomeUnknown = false
+	assert.Equal(t, "CUDly - RI Exchanges Completed (3 exchanges)", riExchangeCompletedSubject(data))
+}
+
+// recordingSNSPublisher captures what would be published, never touching AWS.
+type recordingSNSPublisher struct {
+	subject string
+	message string
+	calls   int
+}
+
+func (r *recordingSNSPublisher) Publish(_ context.Context, params *sns.PublishInput, _ ...func(*sns.Options)) (*sns.PublishOutput, error) {
+	r.calls++
+	r.subject = aws.ToString(params.Subject)
+	r.message = aws.ToString(params.Message)
+	return &sns.PublishOutput{}, nil
+}
+
+func TestSender_SendRIExchangeCompleted_OutcomeUnknown(t *testing.T) {
+	pub := &recordingSNSPublisher{}
+	sender := NewSenderWithClients(pub, &mockSESEmailSender{}, SenderConfig{
+		FromEmail: "from@example.com",
+		TopicARN:  "arn:aws:sns:us-east-1:123:topic",
+	})
+
+	require.NoError(t, sender.SendRIExchangeCompleted(context.Background(), outcomeUnknownExchangeData()))
+
+	require.Equal(t, 1, pub.calls)
+	assert.Equal(t, "CUDly - RI Exchanges Need Reconciliation (1 of 3 exchanges)", pub.subject)
+	reconcile, _ := splitCompletedEmail(t, pub.message)
+	assert.Contains(t, reconcile, "ri-unknown")
+}
+
+func TestSMTPSender_SendRIExchangeCompleted_OutcomeUnknown(t *testing.T) {
+	server := newMockSMTPServer(t, false)
+	server.start(t)
+
+	sender := &SMTPSender{
+		host:        "127.0.0.1",
+		port:        server.port,
+		fromEmail:   "notifications@cudly.io",
+		fromName:    "CUDly",
+		notifyEmail: "ops@example.com",
+	}
+
+	require.NoError(t, sender.SendRIExchangeCompleted(context.Background(), outcomeUnknownExchangeData()))
+	server.stop() // flush the connection goroutine before reading receivedMsg
+
+	got := server.receivedMsg
+	assert.Contains(t, got, "Subject: CUDly - RI Exchanges Need Reconciliation (1 of 3 exchanges)")
+	assert.Contains(t, got, reconcileHeading)
+	assert.Contains(t, got, "POST /api/ri-exchange/resolve/rec-unknown")
+}

internal/email/sender.go

@@ -669,6 +669,12 @@ type RIExchangeItem struct {
 	Error              string
 	TargetCount        int
 	UtilizationPct     float64
+	// OutcomeUnknown marks an exchange whose accept was sent but whose
+	// response was lost, so AWS may or may not have executed it. Such an
+	// exchange is neither completed nor failed: the completed email lists it
+	// in its own reconciliation group so the operator verifies it in AWS
+	// before retrying. Mirrors exchange.ExchangeOutcome.OutcomeUnknown.
+	OutcomeUnknown bool
 }
 
 // SkippedExchange represents an exchange that was skipped.

internal/email/smtp_sender.go

@@ -512,7 +512,7 @@ func (s *SMTPSender) SendRIExchangePendingApproval(ctx context.Context, data RIE
 
 // SendRIExchangeCompleted sends an RI exchange completion email via SMTP.
 func (s *SMTPSender) SendRIExchangeCompleted(ctx context.Context, data RIExchangeNotificationData) error {
-	subject := fmt.Sprintf("CUDly - RI Exchanges Completed (%d exchanges)", len(data.Exchanges))
+	subject := riExchangeCompletedSubject(data)
 	body, err := RenderRIExchangeCompletedEmail(data)
 	if err != nil {
 		return fmt.Errorf("failed to render ri exchange completed email: %w", err)

internal/email/template_renderers.go

@@ -152,6 +152,29 @@ func RenderRIExchangeCompletedEmail(data RIExchangeNotificationData) (string, er
 	return renderTextTemplate("ri-exchange-completed", riExchangeCompletedTemplate, data)
 }
 
+// ReconciliationExchanges returns the exchanges whose outcome at AWS is
+// unknown (OutcomeUnknown set). The completed email lists them in their own
+// group, apart from the completed exchanges and the definite failures.
+func (d RIExchangeNotificationData) ReconciliationExchanges() []RIExchangeItem {
+	var out []RIExchangeItem
+	for i := range d.Exchanges {
+		if d.Exchanges[i].OutcomeUnknown {
+			out = append(out, d.Exchanges[i])
+		}
+	}
+	return out
+}
+
+// riExchangeCompletedSubject is the subject of the completed email, shared by
+// the SNS/SES and SMTP senders. A run with an unknown-outcome exchange must
+// not arrive under a plain "Completed" subject.
+func riExchangeCompletedSubject(data RIExchangeNotificationData) string {
+	if n := len(data.ReconciliationExchanges()); n > 0 {
+		return fmt.Sprintf("CUDly - RI Exchanges Need Reconciliation (%d of %d exchanges)", n, len(data.Exchanges))
+	}
+	return fmt.Sprintf("CUDly - RI Exchanges Completed (%d exchanges)", len(data.Exchanges))
+}
+
 // RenderPurchaseApprovalRequestEmail renders the plain-text purchase
 // approval request email template. Issue #287: this is the multipart
 // text/plain half -- pair with RenderPurchaseApprovalRequestEmailHTML

internal/email/templates.go

@@ -404,10 +404,28 @@ const riExchangePendingApprovalHTMLTemplate = `<!DOCTYPE html>
 
 const riExchangeCompletedTemplate = `CUDly - RI Exchanges Completed
 ==============================
-
+{{with .ReconciliationExchanges}}
+NEEDS RECONCILIATION - verify in AWS before acting
+--------------------------------------------------
+
+CUDly sent the exchange(s) below to AWS but got no definite answer, so AWS
+may or may not have executed them. They are not failures: do not retry them
+until they are reconciled. The run stopped at the first one, and each one
+keeps counting against the daily exchange cap until it is resolved. Check the
+source RI in the AWS console, then record what happened with the resolve
+request shown for each exchange (the API accepts it once the record has been
+unchanged for a few hours).
+{{range .}}
+- Source: {{.SourceRIID}} ({{.SourceInstanceType}})
+  Target: {{.TargetInstanceType}} x{{.TargetCount}}
+  Quoted payment: ${{.PaymentDue}}
+  Record: {{.RecordID}}
+  Resolve: POST /api/ri-exchange/resolve/{{.RecordID}}
+  Detail: {{.Error}}
+{{end}}{{end}}
 The following RI exchanges have been {{if eq .Mode "auto"}}automatically {{end}}completed:
 
-{{range .Exchanges}}{{if eq .Error ""}}
+{{range .Exchanges}}{{if and (eq .Error "") (not .OutcomeUnknown)}}
 - Source: {{.SourceRIID}} ({{.SourceInstanceType}})
   Target: {{.TargetInstanceType}} x{{.TargetCount}}
   Payment: ${{.PaymentDue}}
@@ -591,8 +609,7 @@ func (s *Sender) SendRIExchangeCompleted(ctx context.Context, data RIExchangeNot
 		return fmt.Errorf("failed to render ri exchange completed email: %w", err)
 	}
 
-	subject := fmt.Sprintf("CUDly - RI Exchanges Completed (%d exchanges)", len(data.Exchanges))
-	return s.SendNotification(ctx, subject, body)
+	return s.SendNotification(ctx, riExchangeCompletedSubject(data), body)
 }
 
 const purchaseApprovalRequestTemplate = `CUDly - Purchase Approval Required

internal/server/handler_ri_exchange.go

@@ -257,6 +257,7 @@ func buildExchangeNotificationData(result *exchange.AutoExchangeResult, dashboar
 			ExchangeID:         o.ExchangeID,
 			UtilizationPct:     o.UtilizationPct,
 			Error:              o.Error,
+			OutcomeUnknown:     o.OutcomeUnknown,
 		})
 	}
 

internal/server/handler_ri_exchange_reconcile_test.go

@@ -0,0 +1,114 @@
+package server
+
+import (
+	"context"
+	"strings"
+	"testing"
+
+	"github.com/LeanerCloud/cloud-commitments-go/pkg/exchange"
+	"github.com/LeanerCloud/cloud-commitments-platform/internal/email"
+)
+
+// outcomeUnknownAutoResult is the shape cloud-commitments-go returns when the
+// accept response for an auto exchange is lost: the outcome stays in Failed
+// with OutcomeUnknown set (go#497/#499), and the run halts after it.
+func outcomeUnknownAutoResult() *exchange.AutoExchangeResult {
+	return &exchange.AutoExchangeResult{
+		Mode: "auto",
+		Completed: []exchange.ExchangeOutcome{{
+			RecordID:           "rec-done",
+			SourceRIID:         "ri-done",
+			SourceInstanceType: "m5.large",
+			TargetInstanceType: "m6i.large",
+			TargetCount:        1,
+			PaymentDue:         "12.00",
+			ExchangeID:         "exch-done",
+		}},
+		Failed: []exchange.ExchangeOutcome{
+			{
+				RecordID:           "rec-unknown",
+				SourceRIID:         "ri-unknown",
+				SourceInstanceType: "c5.large",
+				TargetInstanceType: "c6i.large",
+				TargetCount:        2,
+				PaymentDue:         "34.00",
+				Error:              "purchase outcome unknown: the accept may have been processed and the exchange may exist; reconcile before retrying: connection reset",
+				OutcomeUnknown:     true,
+			},
+			{
+				RecordID:           "rec-failed",
+				SourceRIID:         "ri-failed",
+				SourceInstanceType: "r5.large",
+				TargetInstanceType: "r6i.large",
+				TargetCount:        1,
+				PaymentDue:         "5.00",
+				Error:              "InvalidParameterValue: offering not available",
+			},
+		},
+	}
+}
+
+// TestSendExchangeNotification_OutcomeUnknownRendersReconciliation drives an
+// auto run with an unknown-outcome exchange through sendExchangeNotification,
+// captures the data handed to the (recording, never-sending) email sender, and
+// renders the real completed-email template from it. The unknown-outcome
+// exchange must show up in its own reconciliation group, apart from both the
+// completed exchanges and the definite failures.
+func TestSendExchangeNotification_OutcomeUnknownRendersReconciliation(t *testing.T) {
+	var got *email.RIExchangeNotificationData
+	app := &Application{
+		Email: &mockEmailSender{
+			sendCompletedFunc: func(_ context.Context, data email.RIExchangeNotificationData) error {
+				got = &data
+				return nil
+			},
+			sendApprovalFunc: func(_ context.Context, _ email.RIExchangeNotificationData) error {
+				t.Error("an auto run must not send the approval email")
+				return nil
+			},
+		},
+	}
+
+	app.sendExchangeNotification(context.Background(), outcomeUnknownAutoResult(), "")
+	if got == nil {
+		t.Fatal("expected the completed email to be sent")
+	}
+
+	body, err := email.RenderRIExchangeCompletedEmail(*got)
+	if err != nil {
+		t.Fatalf("render completed email: %v", err)
+	}
+
+	const reconcileHeading = "NEEDS RECONCILIATION"
+	const completedHeading = "The following RI exchanges have been automatically completed:"
+	reconcileAt := strings.Index(body, reconcileHeading)
+	completedAt := strings.Index(body, completedHeading)
+	if reconcileAt < 0 {
+		t.Fatalf("email has no %q group for the unknown-outcome exchange:\n%s", reconcileHeading, body)
+	}
+	if completedAt < 0 {
+		t.Fatalf("email lost its completed group:\n%s", body)
+	}
+	if reconcileAt > completedAt {
+		t.Fatalf("reconciliation group must come before the completed group:\n%s", body)
+	}
+
+	reconcile := body[reconcileAt:completedAt]
+	completed := body[completedAt:]
+	for _, want := range []string{"ri-unknown", "rec-unknown", "c6i.large x2", "purchase outcome unknown", "/api/ri-exchange/resolve/"} {
+		if !strings.Contains(reconcile, want) {
+			t.Errorf("reconciliation group is missing %q:\n%s", want, reconcile)
+		}
+	}
+	if strings.Contains(completed, "ri-unknown") {
+		t.Errorf("unknown-outcome exchange is listed as completed:\n%s", completed)
+	}
+	if !strings.Contains(completed, "ri-done") {
+		t.Errorf("completed exchange is missing from the completed group:\n%s", completed)
+	}
+	// A definite failure is not an unknown outcome: it must not be put in the
+	// reconciliation group.
+	if strings.Contains(reconcile, "ri-failed") {
+		t.Errorf("definite failure is listed as needing reconciliation:\n%s", reconcile)
+	}
+}