From f113cc80dd6b49e9d6356fd10b4779c687af3954 Mon Sep 17 00:00:00 2001 From: Philipp Matthes Date: Thu, 18 Jun 2026 08:31:29 +0200 Subject: [PATCH] Move committed resource status summary business logic to internal --- .../committed_resource_controller.go | 4 +- .../committed_resource_summary.go | 17 ++--- .../committed_resource_summary_test.go | 65 ++++++++++--------- .../commitments/usage_reconciler.go | 6 +- 4 files changed, 47 insertions(+), 45 deletions(-) rename {api/v1alpha1 => internal/scheduling/reservations/commitments}/committed_resource_summary.go (81%) rename {api/v1alpha1 => internal/scheduling/reservations/commitments}/committed_resource_summary_test.go (61%) diff --git a/internal/scheduling/reservations/commitments/committed_resource_controller.go b/internal/scheduling/reservations/commitments/committed_resource_controller.go index a24e0c9e3..a72058ee5 100644 --- a/internal/scheduling/reservations/commitments/committed_resource_controller.go +++ b/internal/scheduling/reservations/commitments/committed_resource_controller.go @@ -401,7 +401,7 @@ func (r *CommittedResourceController) setAccepted(ctx context.Context, cr *v1alp LastTransitionTime: now, ObservedGeneration: cr.Generation, }) - cr.Status.StatusSummary = v1alpha1.ComputeStatusSummary(cr.Spec, cr.Status, now.Time) + cr.Status.StatusSummary = computeStatusSummary(cr.Spec, cr.Status, now.Time) if err := r.Status().Patch(ctx, cr, client.MergeFrom(old)); err != nil { return client.IgnoreNotFound(err) } @@ -529,7 +529,7 @@ func (r *CommittedResourceController) setNotReadyRetry(ctx context.Context, cr * func (r *CommittedResourceController) patchNotReady(ctx context.Context, cr *v1alpha1.CommittedResource, reason, message string, resetTimer bool) error { old := cr.DeepCopy() setReadyConditionFalse(&cr.Status.Conditions, reason, message, cr.Generation, resetTimer) - cr.Status.StatusSummary = v1alpha1.ComputeStatusSummary(cr.Spec, cr.Status, time.Now()) + cr.Status.StatusSummary = computeStatusSummary(cr.Spec, cr.Status, time.Now()) if err := r.Status().Patch(ctx, cr, client.MergeFrom(old)); err != nil { return client.IgnoreNotFound(err) } diff --git a/api/v1alpha1/committed_resource_summary.go b/internal/scheduling/reservations/commitments/committed_resource_summary.go similarity index 81% rename from api/v1alpha1/committed_resource_summary.go rename to internal/scheduling/reservations/commitments/committed_resource_summary.go index 83048f0fa..63fb18443 100644 --- a/api/v1alpha1/committed_resource_summary.go +++ b/internal/scheduling/reservations/commitments/committed_resource_summary.go @@ -1,22 +1,23 @@ // Copyright SAP SE // SPDX-License-Identifier: Apache-2.0 -package v1alpha1 +package commitments import ( "fmt" "strings" "time" + "github.com/cobaltcore-dev/cortex/api/v1alpha1" "k8s.io/apimachinery/pkg/api/meta" ) -// ComputeStatusSummary produces a compact human-readable summary of the committed resource's +// computeStatusSummary produces a compact human-readable summary of the committed resource's // current state for the kubectl wide view. // // Format: {reason}[( diff)] [· {N} VM[s]] [· exp in {duration}|no expiry] -func ComputeStatusSummary(spec CommittedResourceSpec, status CommittedResourceStatus, now time.Time) string { - cond := meta.FindStatusCondition(status.Conditions, CommittedResourceConditionReady) +func computeStatusSummary(spec v1alpha1.CommittedResourceSpec, status v1alpha1.CommittedResourceStatus, now time.Time) string { + cond := meta.FindStatusCondition(status.Conditions, v1alpha1.CommittedResourceConditionReady) if cond == nil { return "" } @@ -37,7 +38,7 @@ func ComputeStatusSummary(spec CommittedResourceSpec, status CommittedResourceSt // generic "waiting for reservation placement" which adds nothing beyond the reason. msg := cond.Message showMsg := msg != "" && msg != "waiting for reservation placement" && - (reason == CommittedResourceReasonRejected || reason == CommittedResourceReasonReserving) + (reason == v1alpha1.CommittedResourceReasonRejected || reason == v1alpha1.CommittedResourceReasonReserving) if showMsg { if len(msg) > 80 { msg = msg[:77] + "..." @@ -46,7 +47,7 @@ func ComputeStatusSummary(spec CommittedResourceSpec, status CommittedResourceSt } // VM count — only meaningful once placement is accepted. - if reason == CommittedResourceReasonAccepted { + if reason == v1alpha1.CommittedResourceReasonAccepted { n := len(status.AssignedInstances) if n == 1 { parts = append(parts, "1 VM") @@ -56,7 +57,7 @@ func ComputeStatusSummary(spec CommittedResourceSpec, status CommittedResourceSt } // Expiry — omit for Rejected (CR is terminal, expiry irrelevant). - if reason != CommittedResourceReasonRejected { + if reason != v1alpha1.CommittedResourceReasonRejected { if spec.EndTime == nil { parts = append(parts, "no expiry") } else if remaining := spec.EndTime.Sub(now); remaining <= 0 { @@ -71,7 +72,7 @@ func ComputeStatusSummary(spec CommittedResourceSpec, status CommittedResourceSt // buildSpecDiff returns a semicolon-separated list of placement-relevant field changes // between spec and the last accepted spec. -func buildSpecDiff(spec CommittedResourceSpec, accepted *CommittedResourceSpec) string { +func buildSpecDiff(spec v1alpha1.CommittedResourceSpec, accepted *v1alpha1.CommittedResourceSpec) string { if accepted == nil { return "" } diff --git a/api/v1alpha1/committed_resource_summary_test.go b/internal/scheduling/reservations/commitments/committed_resource_summary_test.go similarity index 61% rename from api/v1alpha1/committed_resource_summary_test.go rename to internal/scheduling/reservations/commitments/committed_resource_summary_test.go index 13fc13c92..56dec3304 100644 --- a/api/v1alpha1/committed_resource_summary_test.go +++ b/internal/scheduling/reservations/commitments/committed_resource_summary_test.go @@ -1,28 +1,29 @@ // Copyright SAP SE // SPDX-License-Identifier: Apache-2.0 -package v1alpha1 +package commitments import ( "testing" "time" + "github.com/cobaltcore-dev/cortex/api/v1alpha1" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) func ptr[T any](v T) *T { return &v } -func makeSpec(amount, fg, az string, endTime *time.Time) CommittedResourceSpec { - s := CommittedResourceSpec{ +func makeSpec(amount, fg, az string, endTime *time.Time) v1alpha1.CommittedResourceSpec { + s := v1alpha1.CommittedResourceSpec{ Amount: resource.MustParse(amount), FlavorGroupName: fg, AvailabilityZone: az, - ResourceType: CommittedResourceTypeMemory, + ResourceType: v1alpha1.CommittedResourceTypeMemory, ProjectID: "proj-1", DomainID: "dom-1", CommitmentUUID: "uuid-1", - State: CommitmentStatusConfirmed, + State: v1alpha1.CommitmentStatusConfirmed, } if endTime != nil { s.EndTime = &metav1.Time{Time: *endTime} @@ -30,7 +31,7 @@ func makeSpec(amount, fg, az string, endTime *time.Time) CommittedResourceSpec { return s } -func makeStatusWithMessage(reason, message string, accepted *CommittedResourceSpec) CommittedResourceStatus { +func makeStatusWithMessage(reason, message string, accepted *v1alpha1.CommittedResourceSpec) v1alpha1.CommittedResourceStatus { status := makeStatus(reason, accepted, nil) if len(status.Conditions) == 0 { panic("makeStatus returned no conditions") @@ -39,19 +40,19 @@ func makeStatusWithMessage(reason, message string, accepted *CommittedResourceSp return status } -func makeStatus(reason string, accepted *CommittedResourceSpec, instances []string) CommittedResourceStatus { - status := CommittedResourceStatus{ +func makeStatus(reason string, accepted *v1alpha1.CommittedResourceSpec, instances []string) v1alpha1.CommittedResourceStatus { + status := v1alpha1.CommittedResourceStatus{ AssignedInstances: instances, } if accepted != nil { status.AcceptedSpec = accepted.DeepCopy() } condStatus := metav1.ConditionTrue - if reason != CommittedResourceReasonAccepted { + if reason != v1alpha1.CommittedResourceReasonAccepted { condStatus = metav1.ConditionFalse } status.Conditions = []metav1.Condition{{ - Type: CommittedResourceConditionReady, + Type: v1alpha1.CommittedResourceConditionReady, Status: condStatus, Reason: reason, }} @@ -64,49 +65,49 @@ func TestComputeStatusSummary(t *testing.T) { tests := []struct { name string - spec CommittedResourceSpec - status CommittedResourceStatus + spec v1alpha1.CommittedResourceSpec + status v1alpha1.CommittedResourceStatus want string }{ { name: "no Ready condition", spec: makeSpec("2Gi", "fg1", "az1", in(4*time.Hour)), - status: CommittedResourceStatus{}, + status: v1alpha1.CommittedResourceStatus{}, want: "", }, { name: "Accepted, 3 VMs, exp in hours", spec: makeSpec("2Gi", "fg1", "az1", in(4*time.Hour+30*time.Minute)), - status: makeStatus(CommittedResourceReasonAccepted, ptr(makeSpec("2Gi", "fg1", "az1", in(4*time.Hour+30*time.Minute))), []string{"a", "b", "c"}), + status: makeStatus(v1alpha1.CommittedResourceReasonAccepted, ptr(makeSpec("2Gi", "fg1", "az1", in(4*time.Hour+30*time.Minute))), []string{"a", "b", "c"}), want: "Accepted · 3 VMs · exp in 4h 30m", }, { name: "Accepted, 1 VM, singular", spec: makeSpec("2Gi", "fg1", "az1", in(25*time.Hour+2*time.Minute)), - status: makeStatus(CommittedResourceReasonAccepted, ptr(makeSpec("2Gi", "fg1", "az1", in(25*time.Hour+2*time.Minute))), []string{"a"}), + status: makeStatus(v1alpha1.CommittedResourceReasonAccepted, ptr(makeSpec("2Gi", "fg1", "az1", in(25*time.Hour+2*time.Minute))), []string{"a"}), want: "Accepted · 1 VM · exp in 1d 1h", }, { name: "Accepted, 0 VMs, no expiry", spec: makeSpec("2Gi", "fg1", "az1", nil), - status: makeStatus(CommittedResourceReasonAccepted, ptr(makeSpec("2Gi", "fg1", "az1", nil)), nil), + status: makeStatus(v1alpha1.CommittedResourceReasonAccepted, ptr(makeSpec("2Gi", "fg1", "az1", nil)), nil), want: "Accepted · 0 VMs · no expiry", }, { name: "Accepted with amount diff", spec: makeSpec("5Gi", "fg1", "az1", in(3*24*time.Hour+2*time.Hour)), - status: makeStatus(CommittedResourceReasonAccepted, + status: makeStatus(v1alpha1.CommittedResourceReasonAccepted, ptr(makeSpec("2Gi", "fg1", "az1", in(3*24*time.Hour+2*time.Hour))), []string{"a", "b", "c"}), want: "Accepted (amount 2Gi→5Gi) · 3 VMs · exp in 3d 2h", }, { name: "Accepted with az and fg diff", - spec: func() CommittedResourceSpec { + spec: func() v1alpha1.CommittedResourceSpec { s := makeSpec("2Gi", "fg2", "az2", nil) return s }(), - status: makeStatus(CommittedResourceReasonAccepted, + status: makeStatus(v1alpha1.CommittedResourceReasonAccepted, ptr(makeSpec("2Gi", "fg1", "az1", nil)), []string{"a"}), want: "Accepted (fg fg1→fg2; az az1→az2) · 1 VM · no expiry", @@ -114,80 +115,80 @@ func TestComputeStatusSummary(t *testing.T) { { name: "Reserving with expiry", spec: makeSpec("2Gi", "fg1", "az1", in(3*24*time.Hour+2*time.Hour)), - status: makeStatus(CommittedResourceReasonReserving, ptr(makeSpec("2Gi", "fg1", "az1", in(3*24*time.Hour+2*time.Hour))), nil), + status: makeStatus(v1alpha1.CommittedResourceReasonReserving, ptr(makeSpec("2Gi", "fg1", "az1", in(3*24*time.Hour+2*time.Hour))), nil), want: "Reserving · exp in 3d 2h", }, { name: "Reserving with amount diff", spec: makeSpec("5Gi", "fg1", "az1", in(time.Hour+3*time.Minute)), - status: makeStatus(CommittedResourceReasonReserving, ptr(makeSpec("2Gi", "fg1", "az1", in(time.Hour+3*time.Minute))), nil), + status: makeStatus(v1alpha1.CommittedResourceReasonReserving, ptr(makeSpec("2Gi", "fg1", "az1", in(time.Hour+3*time.Minute))), nil), want: "Reserving (amount 2Gi→5Gi) · exp in 1h 3m", }, { name: "Rejected with message", spec: makeSpec("2Gi", "fg1", "az1", in(4*time.Hour)), - status: makeStatusWithMessage(CommittedResourceReasonRejected, "no hosts found for reservation (4/4 slots failed)", ptr(makeSpec("2Gi", "fg1", "az1", in(4*time.Hour)))), + status: makeStatusWithMessage(v1alpha1.CommittedResourceReasonRejected, "no hosts found for reservation (4/4 slots failed)", ptr(makeSpec("2Gi", "fg1", "az1", in(4*time.Hour)))), want: "Rejected · no hosts found for reservation (4/4 slots failed)", }, { name: "Rejected without message (unchanged)", spec: makeSpec("2Gi", "fg1", "az1", in(4*time.Hour)), - status: makeStatus(CommittedResourceReasonRejected, ptr(makeSpec("2Gi", "fg1", "az1", in(4*time.Hour))), nil), + status: makeStatus(v1alpha1.CommittedResourceReasonRejected, ptr(makeSpec("2Gi", "fg1", "az1", in(4*time.Hour))), nil), want: "Rejected", }, { name: "Reserving with failure message shows it", spec: makeSpec("2Gi", "fg1", "az1", in(time.Hour)), - status: makeStatusWithMessage(CommittedResourceReasonReserving, "no hosts found for reservation (2/4 slots failed)", ptr(makeSpec("2Gi", "fg1", "az1", in(time.Hour)))), + status: makeStatusWithMessage(v1alpha1.CommittedResourceReasonReserving, "no hosts found for reservation (2/4 slots failed)", ptr(makeSpec("2Gi", "fg1", "az1", in(time.Hour)))), want: "Reserving · no hosts found for reservation (2/4 slots failed) · exp in 1h 0m", }, { name: "Reserving with waiting message skips it", spec: makeSpec("2Gi", "fg1", "az1", in(time.Hour)), - status: makeStatusWithMessage(CommittedResourceReasonReserving, "waiting for reservation placement", ptr(makeSpec("2Gi", "fg1", "az1", in(time.Hour)))), + status: makeStatusWithMessage(v1alpha1.CommittedResourceReasonReserving, "waiting for reservation placement", ptr(makeSpec("2Gi", "fg1", "az1", in(time.Hour)))), want: "Reserving · exp in 1h 0m", }, { name: "Rejected with long message truncated", spec: makeSpec("2Gi", "fg1", "az1", in(4*time.Hour)), - status: makeStatusWithMessage(CommittedResourceReasonRejected, "no hosts found for reservation because all hypervisors in this flavor group and availability zone are fully committed and no further capacity exists", ptr(makeSpec("2Gi", "fg1", "az1", in(4*time.Hour)))), + status: makeStatusWithMessage(v1alpha1.CommittedResourceReasonRejected, "no hosts found for reservation because all hypervisors in this flavor group and availability zone are fully committed and no further capacity exists", ptr(makeSpec("2Gi", "fg1", "az1", in(4*time.Hour)))), want: "Rejected · no hosts found for reservation because all hypervisors in this flavor group a...", }, { name: "Planned with expiry in days", spec: makeSpec("2Gi", "fg1", "az1", in(3*24*time.Hour+2*time.Hour)), - status: makeStatus(CommittedResourceReasonPlanned, nil, nil), + status: makeStatus(v1alpha1.CommittedResourceReasonPlanned, nil, nil), want: "Planned · exp in 3d 2h", }, { name: "Planned no expiry", spec: makeSpec("2Gi", "fg1", "az1", nil), - status: makeStatus(CommittedResourceReasonPlanned, nil, nil), + status: makeStatus(v1alpha1.CommittedResourceReasonPlanned, nil, nil), want: "Planned · no expiry", }, { name: "expired EndTime shows expired", spec: makeSpec("2Gi", "fg1", "az1", in(-time.Hour)), - status: makeStatus(CommittedResourceReasonAccepted, ptr(makeSpec("2Gi", "fg1", "az1", in(-time.Hour))), []string{"a"}), + status: makeStatus(v1alpha1.CommittedResourceReasonAccepted, ptr(makeSpec("2Gi", "fg1", "az1", in(-time.Hour))), []string{"a"}), want: "Accepted · 1 VM · expired", }, { name: "sub-minute expiry shows seconds", spec: makeSpec("2Gi", "fg1", "az1", in(18*time.Second)), - status: makeStatus(CommittedResourceReasonAccepted, ptr(makeSpec("2Gi", "fg1", "az1", in(18*time.Second))), nil), + status: makeStatus(v1alpha1.CommittedResourceReasonAccepted, ptr(makeSpec("2Gi", "fg1", "az1", in(18*time.Second))), nil), want: "Accepted · 0 VMs · exp in 18s", }, { name: "sub-hour expiry shows minutes and seconds", spec: makeSpec("2Gi", "fg1", "az1", in(23*time.Minute+5*time.Second)), - status: makeStatus(CommittedResourceReasonAccepted, ptr(makeSpec("2Gi", "fg1", "az1", in(23*time.Minute+5*time.Second))), nil), + status: makeStatus(v1alpha1.CommittedResourceReasonAccepted, ptr(makeSpec("2Gi", "fg1", "az1", in(23*time.Minute+5*time.Second))), nil), want: "Accepted · 0 VMs · exp in 23m 5s", }, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - got := ComputeStatusSummary(tc.spec, tc.status, now) + got := computeStatusSummary(tc.spec, tc.status, now) if got != tc.want { t.Errorf("got %q, want %q", got, tc.want) } diff --git a/internal/scheduling/reservations/commitments/usage_reconciler.go b/internal/scheduling/reservations/commitments/usage_reconciler.go index 18c53eee6..b15aa0777 100644 --- a/internal/scheduling/reservations/commitments/usage_reconciler.go +++ b/internal/scheduling/reservations/commitments/usage_reconciler.go @@ -57,7 +57,7 @@ func (r *UsageReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl cr.Status.UsedResources = nil cr.Status.LastUsageReconcileAt = nil cr.Status.UsageObservedGeneration = nil - cr.Status.StatusSummary = v1alpha1.ComputeStatusSummary(cr.Spec, cr.Status, start) + cr.Status.StatusSummary = computeStatusSummary(cr.Spec, cr.Status, start) if err := r.Status().Patch(ctx, &cr, client.MergeFrom(old)); err != nil { return ctrl.Result{}, client.IgnoreNotFound(err) } @@ -76,7 +76,7 @@ func (r *UsageReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl cr.Status.UsedResources = nil cr.Status.LastUsageReconcileAt = nil cr.Status.UsageObservedGeneration = nil - cr.Status.StatusSummary = v1alpha1.ComputeStatusSummary(cr.Spec, cr.Status, start) + cr.Status.StatusSummary = computeStatusSummary(cr.Spec, cr.Status, start) if err := r.Status().Patch(ctx, &cr, client.MergeFrom(old)); err != nil { return ctrl.Result{}, client.IgnoreNotFound(err) } @@ -224,7 +224,7 @@ func (r *UsageReconciler) writeUsageStatus(ctx context.Context, state *Commitmen } target.Status.LastUsageReconcileAt = &now target.Status.UsageObservedGeneration = &target.Generation - target.Status.StatusSummary = v1alpha1.ComputeStatusSummary(target.Spec, target.Status, now.Time) + target.Status.StatusSummary = computeStatusSummary(target.Spec, target.Status, now.Time) return r.Status().Patch(ctx, target, client.MergeFrom(old)) }