feat(controller): record a Stage's current and last PromotionRequest

api/v1alpha1/stage_types.go

@@ -479,14 +479,15 @@ type StageStatus struct {
 	// fanning Freight out to this Stage's Targets. It is absent for a Stage that
 	// governs no Targets.
 	//
-	// Kargo Enterprise only: This field is ignored in Kargo OSS.
+	// Fanning Freight out to Targets is a Kargo Enterprise-only feature. Kargo
+	// OSS maintains this field all the same, but the PromotionRequest it refers
+	// to never gets further than being marked Errored for that reason.
 	//
 	// +optional
 	CurrentPromotionRequest *PromotionRequestReference `json:"currentPromotionRequest,omitempty"`
 	// LastPromotionRequest is a reference to the last PromotionRequest to reach a
-	// terminal phase. It is absent for a Stage that governs no Targets.
-	//
-	// Kargo Enterprise only: This field is ignored in Kargo OSS.
+	// terminal phase. It is absent for a Stage that governs no Targets, and only
+	// ever moves forward, so it outlives the PromotionRequest it refers to.
 	//
 	// +optional
 	LastPromotionRequest *PromotionRequestReference `json:"lastPromotionRequest,omitempty"`

charts/kargo/resources/crds/kargo.akuity.io_stages.yaml

@@ -1431,7 +1431,9 @@ spec:
                   fanning Freight out to this Stage's Targets. It is absent for a Stage that
                   governs no Targets.
 
-                  Kargo Enterprise only: This field is ignored in Kargo OSS.
+                  Fanning Freight out to Targets is a Kargo Enterprise-only feature. Kargo
+                  OSS maintains this field all the same, but the PromotionRequest it refers
+                  to never gets further than being marked Errored for that reason.
                 properties:
                   finishedAt:
                     description: FinishedAt is the time at which the PromotionRequest
@@ -2571,9 +2573,8 @@ spec:
               lastPromotionRequest:
                 description: |-
                   LastPromotionRequest is a reference to the last PromotionRequest to reach a
-                  terminal phase. It is absent for a Stage that governs no Targets.
-
-                  Kargo Enterprise only: This field is ignored in Kargo OSS.
+                  terminal phase. It is absent for a Stage that governs no Targets, and only
+                  ever moves forward, so it outlives the PromotionRequest it refers to.
                 properties:
                   finishedAt:
                     description: FinishedAt is the time at which the PromotionRequest

pkg/api/promotion_request.go

@@ -3,6 +3,7 @@ package api
 import (
 	"context"
 	"fmt"
+	"strings"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -39,6 +40,80 @@ func GenerateChildPromotionName(stageName, targetName, freight string) string {
 	)
 }
 
+// ComparePromotionRequestByPhaseAndCreationTime compares two PromotionRequests
+// by their phase and creation time. It returns a negative value if
+// PromotionRequest `a` should come before PromotionRequest `b`, a positive
+// value if `a` should come after `b`, or zero if they are considered equal for
+// sorting purposes. It can be used in conjunction with slices.SortFunc to sort
+// a list of PromotionRequests.
+//
+// The order is the one ComparePromotionByPhaseAndCreationTime imposes on
+// Promotions, so that a Stage chooses the PromotionRequest it is promoting
+// through exactly as it chooses its current Promotion:
+//
+//  1. Running PromotionRequests
+//  2. Non-terminal PromotionRequests (ordered by ULID in ascending order)
+//  3. Terminal PromotionRequests (ordered by ULID in descending order)
+//
+// As there, name order stands in for creation order: a generated
+// PromotionRequest name is <stage>.<ulid>.<short-hash>, so among the requests
+// of a single Stage everything left of the ULID is identical and comparing
+// names whole is comparing the ULIDs.
+func ComparePromotionRequestByPhaseAndCreationTime(a, b kargoapi.PromotionRequest) int {
+	// Compare the phases of the PromotionRequests first.
+	if phaseCompare := ComparePromotionRequestPhase(
+		a.Status.Phase,
+		b.Status.Phase,
+	); phaseCompare != 0 {
+		return phaseCompare
+	}
+
+	switch {
+	case !a.Status.Phase.IsTerminal():
+		// Non-terminal PromotionRequests are ordered in ascending order, so that
+		// the request which was (or will be) worked first is at the top.
+		return strings.Compare(a.Name, b.Name)
+	default:
+		// Terminal PromotionRequests are ordered in descending order, so that the
+		// most recent request is at the top, limiting the number of requests which
+		// have to be further inspected.
+		return strings.Compare(b.Name, a.Name)
+	}
+}
+
+// ComparePromotionRequestPhase compares two PromotionRequest phases. It returns
+// a negative value if phase `a` should come before phase `b`, a positive value
+// if phase `a` should come after phase `b`, or zero if they are considered
+// equal for sorting purposes. It can be used in combination with
+// slices.SortFunc to sort a list of PromotionRequest phases.
+//
+// The order of PromotionRequest phases matches the one ComparePromotionPhase
+// imposes on Promotion phases:
+//
+//  1. Running
+//  2. Non-terminal phases
+//  3. Terminal phases
+func ComparePromotionRequestPhase(a, b kargoapi.PromotionRequestPhase) int {
+	aRunning := a == kargoapi.PromotionRequestPhaseRunning
+	bRunning := b == kargoapi.PromotionRequestPhaseRunning
+	aTerminal, bTerminal := a.IsTerminal(), b.IsTerminal()
+
+	// NB: As in ComparePromotionPhase, the order of the cases here is important:
+	// "Running" is a special case that should always come before any other phase.
+	switch {
+	case aRunning && !bRunning:
+		return -1
+	case !aRunning && bRunning:
+		return 1
+	case !aTerminal && bTerminal:
+		return -1
+	case aTerminal && !bTerminal:
+		return 1
+	default:
+		return 0
+	}
+}
+
 // NewPromotionRequest constructs a PromotionRequest expressing the intent to
 // promote the given Freight to the Targets the Stage governs.
 //

pkg/api/promotion_request_test.go

@@ -143,6 +143,135 @@ func TestGenerateChildPromotionName(t *testing.T) {
 	}
 }
 
+func TestComparePromotionRequestPhase(t *testing.T) {
+	t.Parallel()
+
+	testCases := []struct {
+		name     string
+		a        kargoapi.PromotionRequestPhase
+		b        kargoapi.PromotionRequestPhase
+		expected int
+	}{
+		{
+			name:     "Running before Pending",
+			a:        kargoapi.PromotionRequestPhaseRunning,
+			b:        kargoapi.PromotionRequestPhasePending,
+			expected: -1,
+		},
+		{
+			name:     "Pending after Running",
+			a:        kargoapi.PromotionRequestPhasePending,
+			b:        kargoapi.PromotionRequestPhaseRunning,
+			expected: 1,
+		},
+		{
+			name:     "non-terminal before terminal",
+			a:        kargoapi.PromotionRequestPhasePending,
+			b:        kargoapi.PromotionRequestPhaseSucceeded,
+			expected: -1,
+		},
+		{
+			name:     "terminal after non-terminal",
+			a:        kargoapi.PromotionRequestPhaseSucceeded,
+			b:        kargoapi.PromotionRequestPhasePending,
+			expected: 1,
+		},
+		{
+			name: "a PromotionRequest without a phase yet is non-terminal",
+			a:    "",
+			b:    kargoapi.PromotionRequestPhaseErrored,
+			// The reconciler has yet to record a phase, so the request still has
+			// work ahead of it.
+			expected: -1,
+		},
+		{
+			name:     "terminal phases are equal to one another",
+			a:        kargoapi.PromotionRequestPhaseSucceeded,
+			b:        kargoapi.PromotionRequestPhaseFailed,
+			expected: 0,
+		},
+		{
+			name:     "identical phases",
+			a:        kargoapi.PromotionRequestPhaseRunning,
+			b:        kargoapi.PromotionRequestPhaseRunning,
+			expected: 0,
+		},
+	}
+	for _, testCase := range testCases {
+		t.Run(testCase.name, func(t *testing.T) {
+			t.Parallel()
+			require.Equal(
+				t,
+				testCase.expected,
+				ComparePromotionRequestPhase(testCase.a, testCase.b),
+			)
+		})
+	}
+}
+
+func TestComparePromotionRequestByPhaseAndCreationTime(t *testing.T) {
+	t.Parallel()
+
+	// Generated in this order, so the ULID in older precedes the ULID in newer.
+	older := GeneratePromotionRequestName("test-stage", "fake-freight")
+	newer := GeneratePromotionRequestName("test-stage", "fake-freight")
+
+	request := func(name string, phase kargoapi.PromotionRequestPhase) kargoapi.PromotionRequest {
+		return kargoapi.PromotionRequest{
+			ObjectMeta: metav1.ObjectMeta{Name: name},
+			Status:     kargoapi.PromotionRequestStatus{Phase: phase},
+		}
+	}
+
+	testCases := []struct {
+		name       string
+		a          kargoapi.PromotionRequest
+		b          kargoapi.PromotionRequest
+		assertions func(*testing.T, int)
+	}{
+		{
+			name: "phase is compared before name",
+			// The Running request is the newer of the two, so name order alone
+			// would put the other one first.
+			a: request(newer, kargoapi.PromotionRequestPhaseRunning),
+			b: request(older, kargoapi.PromotionRequestPhasePending),
+			assertions: func(t *testing.T, result int) {
+				require.Negative(t, result)
+			},
+		},
+		{
+			name: "older of two non-terminal requests comes first",
+			a:    request(older, kargoapi.PromotionRequestPhasePending),
+			b:    request(newer, kargoapi.PromotionRequestPhasePending),
+			assertions: func(t *testing.T, result int) {
+				require.Negative(t, result)
+			},
+		},
+		{
+			name: "newer of two terminal requests comes first",
+			a:    request(newer, kargoapi.PromotionRequestPhaseSucceeded),
+			b:    request(older, kargoapi.PromotionRequestPhaseFailed),
+			assertions: func(t *testing.T, result int) {
+				require.Negative(t, result)
+			},
+		},
+	}
+	for _, testCase := range testCases {
+		t.Run(testCase.name, func(t *testing.T) {
+			t.Parallel()
+			testCase.assertions(
+				t,
+				ComparePromotionRequestByPhaseAndCreationTime(testCase.a, testCase.b),
+			)
+			// The comparator must be antisymmetric, or slices.SortFunc gives no
+			// guarantees about the order it produces.
+			forward := ComparePromotionRequestByPhaseAndCreationTime(testCase.a, testCase.b)
+			reverse := ComparePromotionRequestByPhaseAndCreationTime(testCase.b, testCase.a)
+			require.Equal(t, forward, -reverse)
+		})
+	}
+}
+
 func TestNewPromotionRequest(t *testing.T) {
 	t.Parallel()