Skip to content

Commit

Permalink
feat: freight summary on stage status (akuity#2306)
Browse files Browse the repository at this point in the history
Signed-off-by: Kent Rancourt <kent.rancourt@gmail.com>
Co-authored-by: Hidde Beydals <hiddeco@users.noreply.github.com>
  • Loading branch information
krancour and hiddeco authored Jul 16, 2024
1 parent d92955e commit 2bbfb54
Show file tree
Hide file tree
Showing 9 changed files with 441 additions and 283 deletions.
571 changes: 306 additions & 265 deletions api/v1alpha1/generated.pb.go

Large diffs are not rendered by default.

12 changes: 12 additions & 0 deletions api/v1alpha1/generated.proto

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

13 changes: 12 additions & 1 deletion api/v1alpha1/stage_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ const FreightOriginKindWarehouse FreightOriginKind = "Warehouse"
// +kubebuilder:object:root=true
// +kubebuilder:subresource:status
// +kubebuilder:printcolumn:name=Shard,type=string,JSONPath=`.spec.shard`
// +kubebuilder:printcolumn:name=Current Freight,type=string,JSONPath=`.status.currentFreight.name`
// +kubebuilder:printcolumn:name=Current Freight,type=string,JSONPath=`.status.freightSummary`
// +kubebuilder:printcolumn:name=Health,type=string,JSONPath=`.status.health.status`
// +kubebuilder:printcolumn:name=Phase,type=string,JSONPath=`.status.phase`
// +kubebuilder:printcolumn:name=Age,type=date,JSONPath=`.metadata.creationTimestamp`
Expand Down Expand Up @@ -745,6 +745,17 @@ type StageStatus struct {
// The first item in the list is the most recent Freight selection and
// currently deployed to the Stage, subsequent items are older selections.
FreightHistory FreightHistory `json:"freightHistory,omitempty" protobuf:"bytes,4,rep,name=freightHistory" protobuf_key:"bytes,1,opt,name=key" protobuf_val:"bytes,2,opt,name=value"`
// FreightSummary is human-readable text maintained by the controller that
// summarizes what Freight is currently deployed to the Stage. For Stages that
// request a single piece of Freight AND the request has been fulfilled, this
// field will simply contain the name of the Freight. For Stages that request
// a single piece of Freight AND the request has NOT been fulfilled, or for
// Stages that request multiple pieces of Freight, this field will contain a
// summary of fulfilled/requested Freight. The existence of this field is a
// workaround for kubectl limitations so that this complex but valuable
// information can be displayed in a column in response to `kubectl get
// stages`.
FreightSummary string `json:"freightSummary,omitempty" protobuf:"bytes,12,opt,name=freightSummary"`
// CurrentFreight is a simplified representation of the Stage's current
// Freight describing what is currently deployed to the Stage.
//
Expand Down
15 changes: 14 additions & 1 deletion charts/kargo/resources/crds/kargo.akuity.io_stages.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ spec:
- jsonPath: .spec.shard
name: Shard
type: string
- jsonPath: .status.currentFreight.name
- jsonPath: .status.freightSummary
name: Current Freight
type: string
- jsonPath: .status.health.status
Expand Down Expand Up @@ -2596,6 +2596,19 @@ spec:
- id
type: object
type: array
freightSummary:
description: |-
FreightSummary is human-readable text maintained by the controller that
summarizes what Freight is currently deployed to the Stage. For Stages that
request a single piece of Freight AND the request has been fulfilled, this
field will simply contain the name of the Freight. For Stages that request
a single piece of Freight AND the request has NOT been fulfilled, or for
Stages that request multiple pieces of Freight, this field will contain a
summary of fulfilled/requested Freight. The existence of this field is a
workaround for kubectl limitations so that this complex but valuable
information can be displayed in a column in response to `kubectl get
stages`.
type: string
health:
description: Health is the Stage's last observed health.
properties:
Expand Down
15 changes: 1 addition & 14 deletions internal/cli/cmd/get/stages.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import (
"errors"
"fmt"
"slices"
"strings"
"time"

"connectrpc.com/connect"
Expand Down Expand Up @@ -168,18 +167,6 @@ func newStageTable(list *metav1.List) *metav1.Table {
rows := make([]metav1.TableRow, len(list.Items))
for i, item := range list.Items {
stage := item.Object.(*kargoapi.Stage) // nolint: forcetypeassert

var currentFreightIDs string
// TODO: Figure out the best way to display the (collection) of Freight
// IDs for "multi-pipeline" Stages.
if current := stage.Status.FreightHistory.Current(); current != nil {
var freightIDs []string
for _, f := range current.Freight {
freightIDs = append(freightIDs, f.Name)
}
currentFreightIDs = strings.Join(freightIDs, ", ")
}

var health string
if stage.Status.Health != nil {
health = string(stage.Status.Health.Status)
Expand All @@ -188,7 +175,7 @@ func newStageTable(list *metav1.List) *metav1.Table {
Cells: []any{
stage.Name,
stage.Spec.Shard,
currentFreightIDs,
stage.Status.FreightSummary,
health,
stage.Status.Phase,
duration.HumanDuration(time.Since(stage.CreationTimestamp.Time)),
Expand Down
17 changes: 17 additions & 0 deletions internal/controller/stages/stages.go
Original file line number Diff line number Diff line change
Expand Up @@ -534,6 +534,10 @@ func (r *reconciler) Reconcile(
newStatus.Message = err.Error()
logger.Error(err, "error syncing Stage")
} else {
newStatus.FreightSummary = buildFreightSummary(
len(stage.Spec.RequestedFreight),
newStatus.FreightHistory.Current(),
)
// Be sure to blank this out in case there's an error in this field from
// the previous reconciliation
newStatus.Message = ""
Expand Down Expand Up @@ -589,6 +593,7 @@ func (r *reconciler) syncControlFlowStage(
status.Health = nil
status.CurrentPromotion = nil
status.LastPromotion = nil
status.FreightSummary = "N/A"

// TODO: Remove this once we remove the fields from the API.
status.History = nil // nolint: staticcheck
Expand Down Expand Up @@ -1624,3 +1629,15 @@ func (r *reconciler) recordFreightVerificationEvent(

r.recorder.AnnotatedEventf(fr, annotations, corev1.EventTypeNormal, reason, message)
}

func buildFreightSummary(requested int, current *kargoapi.FreightCollection) string {
if current == nil {
return fmt.Sprintf("0/%d Fulfilled", requested)
}
if requested == 1 && len(current.Freight) == 1 {
for _, f := range current.Freight {
return f.Name
}
}
return fmt.Sprintf("%d/%d Fulfilled", len(current.Freight), requested)
}
60 changes: 58 additions & 2 deletions internal/controller/stages/stages_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,8 @@ func TestSyncControlFlowStage(t *testing.T) {
) {
require.ErrorContains(t, err, "error getting available Freight for control flow Stage")
require.ErrorContains(t, err, "something went wrong")
// Status should be returned unchanged
newStatus.FreightSummary = ""
// Status should be otherwise unchanged
require.Equal(t, initialStatus, newStatus)
// No events should be recorded
require.Empty(t, recorder.Events)
Expand Down Expand Up @@ -152,7 +153,8 @@ func TestSyncControlFlowStage(t *testing.T) {
) {
require.ErrorContains(t, err, "error marking Freight")
require.ErrorContains(t, err, "something went wrong")
// Status should be returned unchanged
newStatus.FreightSummary = ""
// Status should be otherwise unchanged
require.Equal(t, initialStatus, newStatus)

// No events should be recorded
Expand Down Expand Up @@ -204,6 +206,7 @@ func TestSyncControlFlowStage(t *testing.T) {
require.Nil(t, newStatus.Health) // Cleared
require.Nil(t, newStatus.CurrentFreight) // nolint: staticcheck
require.Nil(t, newStatus.History) // nolint: staticcheck
require.Equal(t, "N/A", newStatus.FreightSummary)

require.Len(t, recorder.Events, 1)
event := <-recorder.Events
Expand Down Expand Up @@ -3687,6 +3690,59 @@ func TestGetAvailableFreightByOrigin(t *testing.T) {
}
}

func TestBuildFreightSummary(t *testing.T) {
testCases := []struct {
name string
requested int
currentFreight *kargoapi.FreightCollection
expectedSummary string
}{
{
name: "requested 1, got none",
requested: 1,
expectedSummary: "0/1 Fulfilled",
},
{
name: "requested 1, got 1",
requested: 1,
currentFreight: &kargoapi.FreightCollection{
Freight: map[string]kargoapi.FreightReference{
"Warehouse/fake-warehouse": {
Name: "fake-freight",
},
},
},
expectedSummary: "fake-freight",
},
{
name: "requested multiple, got none",
requested: 2,
expectedSummary: "0/2 Fulfilled",
},
{
name: "requested multiple, got some",
requested: 2,
currentFreight: &kargoapi.FreightCollection{
Freight: map[string]kargoapi.FreightReference{
"Warehouse/fake-warehouse": {
Name: "fake-freight",
},
},
},
expectedSummary: "1/2 Fulfilled",
},
}
for _, testCase := range testCases {
t.Run(testCase.name, func(t *testing.T) {
require.Equal(
t,
testCase.expectedSummary,
buildFreightSummary(testCase.requested, testCase.currentFreight),
)
})
}
}

func fakeNow() time.Time {
return fakeTime
}
4 changes: 4 additions & 0 deletions ui/src/gen/schema/stages.kargo.akuity.io_v1alpha1.json
Original file line number Diff line number Diff line change
Expand Up @@ -2220,6 +2220,10 @@
},
"type": "array"
},
"freightSummary": {
"description": "FreightSummary is human-readable text maintained by the controller that\nsummarizes what Freight is currently deployed to the Stage. For Stages that\nrequest a single piece of Freight AND the request has been fulfilled, this\nfield will simply contain the name of the Freight. For Stages that request\na single piece of Freight AND the request has NOT been fulfilled, or for\nStages that request multiple pieces of Freight, this field will contain a\nsummary of fulfilled/requested Freight. The existence of this field is a\nworkaround for kubectl limitations so that this complex but valuable\ninformation can be displayed in a column in response to `kubectl get\nstages`.",
"type": "string"
},
"health": {
"description": "Health is the Stage's last observed health.",
"properties": {
Expand Down
17 changes: 17 additions & 0 deletions ui/src/gen/v1alpha1/generated_pb.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4318,6 +4318,22 @@ export class StageStatus extends Message<StageStatus> {
*/
freightHistory: FreightCollection[] = [];

/**
* FreightSummary is human-readable text maintained by the controller that
* summarizes what Freight is currently deployed to the Stage. For Stages that
* request a single piece of Freight AND the request has been fulfilled, this
* field will simply contain the name of the Freight. For Stages that request
* a single piece of Freight AND the request has NOT been fulfilled, or for
* Stages that request multiple pieces of Freight, this field will contain a
* summary of fulfilled/requested Freight. The existence of this field is a
* workaround for kubectl limitations so that this complex but valuable
* information can be displayed in a column in response to `kubectl get
* stages`.
*
* @generated from field: optional string freightSummary = 12;
*/
freightSummary?: string;

/**
* CurrentFreight is a simplified representation of the Stage's current
* Freight describing what is currently deployed to the Stage.
Expand Down Expand Up @@ -4386,6 +4402,7 @@ export class StageStatus extends Message<StageStatus> {
{ no: 11, name: "lastHandledRefresh", kind: "scalar", T: 9 /* ScalarType.STRING */, opt: true },
{ no: 1, name: "phase", kind: "scalar", T: 9 /* ScalarType.STRING */, opt: true },
{ no: 4, name: "freightHistory", kind: "message", T: FreightCollection, repeated: true },
{ no: 12, name: "freightSummary", kind: "scalar", T: 9 /* ScalarType.STRING */, opt: true },
{ no: 2, name: "currentFreight", kind: "message", T: FreightReference, opt: true },
{ no: 3, name: "history", kind: "message", T: FreightReference, repeated: true },
{ no: 8, name: "health", kind: "message", T: Health, opt: true },
Expand Down

0 comments on commit 2bbfb54

Please sign in to comment.