From d358eadd775662adcf1e5a05fbfd399dab35989d Mon Sep 17 00:00:00 2001 From: huskcass <333684917+huskcass@users.noreply.github.com> Date: Sun, 25 Oct 2026 21:49:21 -0700 Subject: [PATCH 1/6] autoscaling: add a Ready summary, observedGeneration and lastScaleTime to LLMScaler Clients had no single condition for scaler health. A UI that treats any False condition as failing showed every healthy scaler as broken, since ScalingDeferred=False/NotDeferred is the normal state. Follow HPA v2 and KEDA: - status.observedGeneration: metadata.generation at the last status write. - status.lastScaleTime: when spec.replicas was last written on the target (normal or forced write); unchanged otherwise. - Ready condition: True/Synced when the sync wrote, was a no-op, or a guard is holding the write. False with InvalidSpec, TargetNotFound, TargetUnreadable, TargetWriteFailed, DecisionUnavailable or MetricsUnavailable. Target read failures, an unparseable apiVersion and a failed target write now record Ready instead of skipping the status write. - A Ready printer column before Deferred. DecisionAvailable and ScalingDeferred, their reasons and when they are set are unchanged. docs/llmscaler.md gains a Conditions table covering every condition and reason. Co-authored-by: aceforeverd <11715200+aceforeverd@users.noreply.github.com> --- api/autoscaling/v1alpha1/llmscaler_types.go | 43 +++ .../v1alpha1/zz_generated.deepcopy.go | 4 + ...utoscaling.modelsphere.dev_llmscalers.yaml | 19 ++ ...lmscalers.autoscaling.modelsphere.dev.yaml | 19 ++ dist/install.yaml | 19 ++ docs/llmscaler.md | 41 ++- .../autoscaling/customprovider_test.go | 14 +- .../autoscaling/llmscaler_casefile_test.go | 100 ++++--- .../autoscaling/llmscaler_controller.go | 166 +++++++++-- .../autoscaling/llmscaler_controller_test.go | 2 + .../autoscaling/llmscaler_helpers_test.go | 12 +- .../autoscaling/llmscaler_scenario_test.go | 221 +++++++++++---- .../autoscaling/llmscaler_status_test.go | 258 ++++++++++++++++++ ...en-the-decision-server-is-unreachable.yaml | 8 + ...the-count-the-decision-server-reports.yaml | 8 + ...-rollout-exceeds-the-deferral-timeout.yaml | 16 ++ ...-step-until-a-sync-period-has-elapsed.yaml | 16 ++ ...n-targetref-apiversion-does-not-parse.yaml | 48 ++++ ...ilable-when-prometheus-is-unreachable.yaml | 30 ++ ...scaletime-when-a-scale-write-succeeds.yaml | 28 ++ ...dy-on-a-no-op-without-a-lastscaletime.yaml | 30 ++ ...ound-and-leave-the-deferral-untouched.yaml | 57 ++++ ...while-a-rollout-defers-the-next-scale.yaml | 52 ++++ ...mprovider-holds-the-fleet-where-it-is.yaml | 2 + ...-decision-holds-the-fleet-where-it-is.yaml | 1 + 25 files changed, 1087 insertions(+), 127 deletions(-) create mode 100644 internal/controller/autoscaling/llmscaler_status_test.go create mode 100644 internal/controller/autoscaling/testdata/controller/ready-status/should-report-invalid-spec-when-targetref-apiversion-does-not-parse.yaml create mode 100644 internal/controller/autoscaling/testdata/controller/ready-status/should-report-metrics-unavailable-when-prometheus-is-unreachable.yaml create mode 100644 internal/controller/autoscaling/testdata/controller/ready-status/should-report-ready-and-lastscaletime-when-a-scale-write-succeeds.yaml create mode 100644 internal/controller/autoscaling/testdata/controller/ready-status/should-report-ready-on-a-no-op-without-a-lastscaletime.yaml create mode 100644 internal/controller/autoscaling/testdata/controller/ready-status/should-report-target-not-found-and-leave-the-deferral-untouched.yaml create mode 100644 internal/controller/autoscaling/testdata/controller/ready-status/should-stay-ready-while-a-rollout-defers-the-next-scale.yaml diff --git a/api/autoscaling/v1alpha1/llmscaler_types.go b/api/autoscaling/v1alpha1/llmscaler_types.go index 4bf93a1..88f5501 100644 --- a/api/autoscaling/v1alpha1/llmscaler_types.go +++ b/api/autoscaling/v1alpha1/llmscaler_types.go @@ -61,6 +61,14 @@ const ( // after the timeout) or when nothing is deferred. ConditionScalingDeferred = "ScalingDeferred" + // ConditionReady is the summary of the last sync, and the condition to + // watch. It is True when the sync completed cleanly: the controller wrote + // spec.replicas, the replica count was already at the step target, or a + // guard is holding the write. It is False when the sync could not do its + // job. ScalingDeferred False with NotDeferred is healthy, not a failure; + // the detail of a hold stays on ScalingDeferred. + ConditionReady = "Ready" + ReasonDecisionReceived = "DecisionReceived" ReasonDecisionUnavailable = "DecisionUnavailable" @@ -80,6 +88,27 @@ const ( // start a deferral, and a no-op while a guard still matches is not this // reason when deferredSince is already set. ReasonNotDeferred = "NotDeferred" + + // ReasonSynced means the last sync completed cleanly. It is the reason on + // ConditionReady when the controller wrote, had nothing to write, or a + // guard is holding the write. The message says which, and a deferral + // points at ScalingDeferred for the detail. + ReasonSynced = "Synced" + // ReasonMetricsUnavailable means the Prometheus provider could not evaluate + // any metric this sync, so the replica count was held. + ReasonMetricsUnavailable = "MetricsUnavailable" + // ReasonInvalidSpec means the scaler cannot be reconciled as written: + // targetRef.apiVersion does not parse, or metricProvider is Custom and + // customProvider is not set. + ReasonInvalidSpec = "InvalidSpec" + // ReasonTargetNotFound means getting the scale target returned NotFound. + ReasonTargetNotFound = "TargetNotFound" + // ReasonTargetUnreadable means getting the scale target failed for a + // reason other than NotFound. + ReasonTargetUnreadable = "TargetUnreadable" + // ReasonTargetWriteFailed means setting or updating spec.replicas on the + // scale target failed. + ReasonTargetWriteFailed = "TargetWriteFailed" ) // CustomProviderSpec configures the "Custom" metric provider: an external @@ -322,6 +351,19 @@ type LLMScalerStatus struct { // +optional DeferredSince *metav1.Time `json:"deferredSince,omitempty"` + // observedGeneration is the most recent generation observed by the + // controller. It is set to metadata.generation on every status write. + // +optional + ObservedGeneration int64 `json:"observedGeneration,omitempty"` + + // lastScaleTime is when the controller last successfully wrote + // spec.replicas on the target, taken from the reconciler clock. It is set + // on a normal write and on a write forced after the deferral timeout, and + // left unchanged on every other sync: a no-op, a deferral, pacing, or an + // error. + // +optional + LastScaleTime *metav1.Time `json:"lastScaleTime,omitempty"` + // conditions represent the current state of the LLMScaler resource. // +listType=map // +listMapKey=type @@ -335,6 +377,7 @@ type LLMScalerStatus struct { // +kubebuilder:printcolumn:name="MaxReplicas",type="integer",JSONPath=".spec.maxReplicas",description="The upper limit for the number of replicas" // +kubebuilder:printcolumn:name="CurrentReplicas",type="integer",JSONPath=".status.currentReplicas",description="The current number of replicas" // +kubebuilder:printcolumn:name="DesiredReplicas",type="integer",JSONPath=".status.desiredReplicas",description="The desired number of replicas" +// +kubebuilder:printcolumn:name="Ready",type="string",JSONPath=".status.conditions[?(@.type==\"Ready\")].status",description="Whether the last sync completed cleanly" // +kubebuilder:printcolumn:name="Deferred",type="string",JSONPath=".status.conditions[?(@.type==\"ScalingDeferred\")].status",description="Whether a scale action is currently deferred" // LLMScaler is the Schema for the llmscalers API diff --git a/api/autoscaling/v1alpha1/zz_generated.deepcopy.go b/api/autoscaling/v1alpha1/zz_generated.deepcopy.go index 3b6be94..044eccb 100644 --- a/api/autoscaling/v1alpha1/zz_generated.deepcopy.go +++ b/api/autoscaling/v1alpha1/zz_generated.deepcopy.go @@ -146,6 +146,10 @@ func (in *LLMScalerStatus) DeepCopyInto(out *LLMScalerStatus) { in, out := &in.DeferredSince, &out.DeferredSince *out = (*in).DeepCopy() } + if in.LastScaleTime != nil { + in, out := &in.LastScaleTime, &out.LastScaleTime + *out = (*in).DeepCopy() + } if in.Conditions != nil { in, out := &in.Conditions, &out.Conditions *out = make([]v1.Condition, len(*in)) diff --git a/config/crd/bases/autoscaling.modelsphere.dev_llmscalers.yaml b/config/crd/bases/autoscaling.modelsphere.dev_llmscalers.yaml index 7727afa..8b5fb29 100644 --- a/config/crd/bases/autoscaling.modelsphere.dev_llmscalers.yaml +++ b/config/crd/bases/autoscaling.modelsphere.dev_llmscalers.yaml @@ -31,6 +31,10 @@ spec: jsonPath: .status.desiredReplicas name: DesiredReplicas type: integer + - description: Whether the last sync completed cleanly + jsonPath: .status.conditions[?(@.type=="Ready")].status + name: Ready + type: string - description: Whether a scale action is currently deferred jsonPath: .status.conditions[?(@.type=="ScalingDeferred")].status name: Deferred @@ -391,6 +395,21 @@ spec: pods managed by this autoscaler format: int32 type: integer + lastScaleTime: + description: |- + lastScaleTime is when the controller last successfully wrote + spec.replicas on the target, taken from the reconciler clock. It is set + on a normal write and on a write forced after the deferral timeout, and + left unchanged on every other sync: a no-op, a deferral, pacing, or an + error. + format: date-time + type: string + observedGeneration: + description: |- + observedGeneration is the most recent generation observed by the + controller. It is set to metadata.generation on every status write. + format: int64 + type: integer type: object required: - spec diff --git a/dist/chart/templates/crd/llmscalers.autoscaling.modelsphere.dev.yaml b/dist/chart/templates/crd/llmscalers.autoscaling.modelsphere.dev.yaml index 8ca7645..90844a0 100644 --- a/dist/chart/templates/crd/llmscalers.autoscaling.modelsphere.dev.yaml +++ b/dist/chart/templates/crd/llmscalers.autoscaling.modelsphere.dev.yaml @@ -34,6 +34,10 @@ spec: jsonPath: .status.desiredReplicas name: DesiredReplicas type: integer + - description: Whether the last sync completed cleanly + jsonPath: .status.conditions[?(@.type=="Ready")].status + name: Ready + type: string - description: Whether a scale action is currently deferred jsonPath: .status.conditions[?(@.type=="ScalingDeferred")].status name: Deferred @@ -394,6 +398,21 @@ spec: pods managed by this autoscaler format: int32 type: integer + lastScaleTime: + description: |- + lastScaleTime is when the controller last successfully wrote + spec.replicas on the target, taken from the reconciler clock. It is set + on a normal write and on a write forced after the deferral timeout, and + left unchanged on every other sync: a no-op, a deferral, pacing, or an + error. + format: date-time + type: string + observedGeneration: + description: |- + observedGeneration is the most recent generation observed by the + controller. It is set to metadata.generation on every status write. + format: int64 + type: integer type: object required: - spec diff --git a/dist/install.yaml b/dist/install.yaml index a3dd946..d1014dd 100644 --- a/dist/install.yaml +++ b/dist/install.yaml @@ -39,6 +39,10 @@ spec: jsonPath: .status.desiredReplicas name: DesiredReplicas type: integer + - description: Whether the last sync completed cleanly + jsonPath: .status.conditions[?(@.type=="Ready")].status + name: Ready + type: string - description: Whether a scale action is currently deferred jsonPath: .status.conditions[?(@.type=="ScalingDeferred")].status name: Deferred @@ -399,6 +403,21 @@ spec: pods managed by this autoscaler format: int32 type: integer + lastScaleTime: + description: |- + lastScaleTime is when the controller last successfully wrote + spec.replicas on the target, taken from the reconciler clock. It is set + on a normal write and on a write forced after the deferral timeout, and + left unchanged on every other sync: a no-op, a deferral, pacing, or an + error. + format: date-time + type: string + observedGeneration: + description: |- + observedGeneration is the most recent generation observed by the + controller. It is set to metadata.generation on every status write. + format: int64 + type: integer type: object required: - spec diff --git a/docs/llmscaler.md b/docs/llmscaler.md index 043ab53..1c6ea01 100644 --- a/docs/llmscaler.md +++ b/docs/llmscaler.md @@ -80,22 +80,39 @@ Only read when `metricProvider: Custom`. | `desiredReplicas` | int32 | Last recommendation after clamping to min/max and scale-down stabilization, but **before** the `maxStepReplicas` cap. While a step cap is walking the fleet down it therefore reports the destination, not the replica count just written. | | `appliedReplicas` | int32 | The target's `spec.replicas` after this sync: the value just written, or the unchanged value when the sync deferred or had nothing to do. This is the replica count left on the target, separate from the `desiredReplicas` recommendation. | | `deferredSince` | metav1.Time | When the current deferral started. Set on the first sync that holds a scale action back. Kept across guard reason changes, no-op syncs while a guard still matches, and forced writes. Cleared only when a sync finds no guard matching (a normal write, or a no-op with no guard). A no-op does not start it. Stored in status so it survives an operator restart. | -| `conditions` | []metav1.Condition | `DecisionAvailable` under the Custom provider (see [Custom metric provider](#custom-metric-provider)) and `ScalingDeferred` (see below). | +| `observedGeneration` | int64 | `metadata.generation` at the last status write. | +| `lastScaleTime` | metav1.Time | When the controller last successfully wrote `spec.replicas` on the target. Set on a normal write and on a write forced after the deferral timeout. Left unchanged on a no-op, a deferral, pacing, or an error. | +| `conditions` | []metav1.Condition | `Ready`, `DecisionAvailable`, and `ScalingDeferred`. See [Conditions](#conditions). | -`kubectl get llmscalers` prints `MinReplicas`, `MaxReplicas`, `CurrentReplicas`, `DesiredReplicas`, `Deferred` (the `ScalingDeferred` condition status). +`kubectl get llmscalers` prints `MinReplicas`, `MaxReplicas`, `CurrentReplicas`, `DesiredReplicas`, `Ready` (the `Ready` condition status), `Deferred` (the `ScalingDeferred` condition status). -#### `ScalingDeferred` condition +### Conditions -Set on every sync that gets as far as a scale decision. It describes that sync. +Read `Ready` for overall health. `DecisionAvailable` and `ScalingDeferred` carry detail. Polarity differs: `ScalingDeferred=True` is the held case, and `False` (`NotDeferred`) is the normal state, not a failure. Every condition carries the `metadata.generation` it was computed from as its `observedGeneration`. A sync that stops early updates `Ready` only and leaves the other two as they were. -A no-op while a guard still matches leaves `deferredSince` unchanged. If it is set, the condition stays `True` with that guard's reason and a message that nothing needs writing this sync, the guard is still active, and the deferral clock keeps running. If it is nil, the condition is `False`/`NotDeferred`: a no-op does not start a deferral. +| Type | Status | Reason | Set when | +| --- | --- | --- | --- | +| `Ready` | `True` | `Synced` | The sync reached a scale decision and none of the failures below applies. That covers a write (`scaled 3 -> 5`), a no-op (`replica count is at the step target`), and a guard holding the write, paced forced writes included (`scale action deferred: ; see ScalingDeferred`). | +| `Ready` | `False` | `InvalidSpec` | `targetRef.apiVersion` does not parse: the sync stops, and only `Ready` and `observedGeneration` are written. Also set when `metricProvider` is `Custom` with no `customProvider` (the replica count is held). | +| `Ready` | `False` | `TargetNotFound` | Getting the target returned NotFound. The sync stops. Only `Ready` and `observedGeneration` are written, and the sync is retried after `retryPeriodSeconds`. | +| `Ready` | `False` | `TargetUnreadable` | Getting the target failed for another reason. Handled like `TargetNotFound`. | +| `Ready` | `False` | `TargetWriteFailed` | Writing `spec.replicas` on the target failed. Only `Ready` and `observedGeneration` are written (best effort). The error is returned, so the sync is retried with backoff. | +| `Ready` | `False` | `DecisionUnavailable` | Custom provider, no decision this sync. Same message as `DecisionAvailable`. The replica count is held. | +| `Ready` | `False` | `MetricsUnavailable` | Prometheus provider, and no metric could be evaluated (query error, no data, NaN/Inf, or an invalid target). The replica count is held. The message includes the last error. | +| `DecisionAvailable` | `True` | `DecisionReceived` | Custom provider: a decision was read this sync. Message: `decision server recommends N replicas`. | +| `DecisionAvailable` | `False` | `DecisionUnavailable` | Custom provider: the request failed or the reply could not be used (see [Custom metric provider](#custom-metric-provider)). The error is the message. | +| `ScalingDeferred` | `True` | `RolloutInProgress` | A `Deployment` target is mid-rollout: `observedGeneration < generation`, `updatedReplicas < spec.replicas`, or `replicas > updatedReplicas`. Holds scale-up, scale-down, and a no-op while a deferral is running. Checked first. | +| `ScalingDeferred` | `True` | `ScaleDownSettling` | A scale-down while `readyReplicas != spec.replicas`, or, when the terminating check is on (off by default), `terminatingReplicas > 0`. | +| `ScalingDeferred` | `False` | `DeferralTimedOut` | A guard still matched and `deferTimeoutSeconds` has elapsed since `deferredSince`, so the held write was applied anyway. `deferredSince` is kept. | +| `ScalingDeferred` | `False` | `NotDeferred` | No guard matched and the sync wrote (`scaled X -> Y`), or the sync was a no-op with no deferral running. `deferredSince` is cleared. | + +- `Ready` precedence when several apply: `InvalidSpec`, then `TargetNotFound`/`TargetUnreadable`, then `TargetWriteFailed`, then `DecisionUnavailable`/`MetricsUnavailable`, then `Synced`. A held replica count from a missing decision or metric outranks a clean write or deferral. +- `DecisionAvailable` is set only under the Custom provider with `customProvider` present. It is never set under Prometheus, and it is not removed if the provider is switched away from Custom. +- `ScalingDeferred` is set on every sync that reaches a scale decision. Syncs that stop at the target read or fail the target write leave it unchanged. + +#### `ScalingDeferred` details -| Status | Reason | Meaning | -| --- | --- | --- | -| `True` | `RolloutInProgress` | A Deployment rollout is in progress; the scale action is held in either direction. | -| `True` | `ScaleDownSettling` | A scale-down is waiting for `readyReplicas` to equal `spec.replicas`. | -| `False` | `DeferralTimedOut` | The deferral reached `deferTimeoutSeconds` and the held write was applied anyway. `deferredSince` stays set when the guard still matches. | -| `False` | `NotDeferred` | No guard matches: this sync wrote, or the target was already at the step target. | +A no-op while a guard still matches leaves `deferredSince` unchanged. If it is set, the condition stays `True` with that guard's reason and a message that nothing needs writing this sync, the guard is still active, and the deferral clock keeps running. If it is nil, the condition is `False`/`NotDeferred`: a no-op does not start a deferral. When the deadline has passed but a forced write already happened within the last sync period, the condition is `True` with the current guard reason and the message notes that the forced write is paced to one per sync period. @@ -168,7 +185,7 @@ Only `decisions[].replicas.active` is read; every other field either identifies - **`apiVersion` selects the schema.** The reply says how to read itself, and only `llmscaling.inference.x-k8s.io/v1alpha1` is understood; anything else is refused rather than parsed hopefully, since a later schema is free to reuse `replicas` for something that is not an absolute count. - **The reply is matched, not trusted.** `serviceId` is sent as a query parameter *and* checked against `decisions[].serviceId`, because what comes back is the server's choice — a decision for a service this scaler did not ask about must not be able to drive it. Exactly one decision must match; **more than one is an error**, the same rule (and the same reasoning) as a multi-series PromQL result. Set `customProvider.namespace` if the server reports the same `serviceId` in several namespaces. - **A failure holds the fleet, it never reads as zero.** A server that is down, a non-200, an unknown `apiVersion`, no matching decision, or a decision with no `replicas.active` all skip that sync and leave `spec.replicas` where it is — mirroring how an unevaluable PromQL query is handled. -- **The outcome is on the status, not in the error log.** Each sync sets the `DecisionAvailable` condition: `True` (`DecisionReceived`) when a decision arrived, `False` (`DecisionUnavailable`) with the error as its message when replicas were held. Check it with `kubectl get llmscaler -o jsonpath='{.status.conditions}'`. +- **The outcome is on the status, not in the error log.** Each sync sets the `DecisionAvailable` condition: `True` (`DecisionReceived`) when a decision arrived, `False` (`DecisionUnavailable`) with the error as its message when replicas were held. `Ready` is `False` with the same reason and message. Check it with `kubectl get llmscaler -o jsonpath='{.status.conditions}'`. - **`scaleDown.deletionCostQuery` is ignored** under this provider: it is PromQL and `serverAddress` is now a decision server, so victim selection falls back to the newest-pod-first heuristic. The controller logs when it skips it. ```yaml diff --git a/internal/controller/autoscaling/customprovider_test.go b/internal/controller/autoscaling/customprovider_test.go index fc16894..ddc4f75 100644 --- a/internal/controller/autoscaling/customprovider_test.go +++ b/internal/controller/autoscaling/customprovider_test.go @@ -238,7 +238,9 @@ type recommendCustomFile struct { AbsentCustomProvider bool `json:"absentCustomProvider"` } `json:"inputs"` Expected struct { - Replicas int32 `json:"replicas"` + Replicas int32 `json:"replicas"` + HoldReason string `json:"holdReason"` + HoldMessageContains string `json:"holdMessageContains"` } `json:"expected"` } @@ -277,10 +279,16 @@ func TestRecommendReplicasCustomProvider(t *testing.T) { key := types.NamespacedName{Namespace: "ns", Name: "s"} // specReplicas is what "hold where it is" means. The files set it to the // same figure as readyReplicas so a hold is not confused with a clamp. - got := r.recommendReplicas(context.Background(), scaler, key, in.ReadyReplicas, in.SpecReplicas) + got, hold := r.recommendReplicas(context.Background(), scaler, key, in.ReadyReplicas, in.SpecReplicas) if got != tc.Expected.Replicas { t.Fatalf("recommendReplicas = %d, want %d", got, tc.Expected.Replicas) } + if tc.Expected.HoldReason != "" && hold.reason != tc.Expected.HoldReason { + t.Errorf("hold reason = %q, want %q", hold.reason, tc.Expected.HoldReason) + } + if sub := tc.Expected.HoldMessageContains; sub != "" && !strings.Contains(hold.message, sub) { + t.Errorf("hold message = %q, want it to contain %q", hold.message, sub) + } }) } @@ -335,7 +343,7 @@ func TestRecommendReplicasDefaultProviderIsPrometheus(t *testing.T) { r := &LLMScalerReconciler{} key := types.NamespacedName{Namespace: "ns", Name: "s"} - got := r.recommendReplicas(context.Background(), scaler, key, tc.Inputs.ReadyReplicas, tc.Inputs.SpecReplicas) + got, _ := r.recommendReplicas(context.Background(), scaler, key, tc.Inputs.ReadyReplicas, tc.Inputs.SpecReplicas) if got != tc.Expected.Replicas { t.Errorf("recommendReplicas = %d, want %d", got, tc.Expected.Replicas) } diff --git a/internal/controller/autoscaling/llmscaler_casefile_test.go b/internal/controller/autoscaling/llmscaler_casefile_test.go index 65c7d22..6e36216 100644 --- a/internal/controller/autoscaling/llmscaler_casefile_test.go +++ b/internal/controller/autoscaling/llmscaler_casefile_test.go @@ -110,6 +110,8 @@ type stepInput struct { RestoreStatus bool `json:"restoreStatus"` SkipReconcile bool `json:"skipReconcile"` SnapshotStatus bool `json:"snapshotStatus"` + DeleteDeploy bool `json:"deleteDeploy"` + APIVersion string `json:"apiVersion"` } type condInput struct { @@ -138,35 +140,44 @@ func (e eventInput) expect() eventExpect { } type stepExpected struct { - Replicas *int32 `json:"replicas"` - Desired *int32 `json:"desired"` - Applied *int32 `json:"applied"` - Current *int32 `json:"current"` - DeferredSince sinceCheck `json:"deferredSince"` - Condition *condInput `json:"condition"` - RequeueAfter *cases.Duration `json:"requeueAfter"` - Events []eventInput `json:"events"` - NoMoreEvents bool `json:"noMoreEvents"` - ForcedWrite *bool `json:"forcedWrite"` - ForcedWriteAt *cases.Duration `json:"forcedWriteAt"` + Replicas *int32 `json:"replicas"` + Desired *int32 `json:"desired"` + Applied *int32 `json:"applied"` + Current *int32 `json:"current"` + DeferredSince sinceCheck `json:"deferredSince"` + Condition *condInput `json:"condition"` + RequeueAfter *cases.Duration `json:"requeueAfter"` + Events []eventInput `json:"events"` + NoMoreEvents bool `json:"noMoreEvents"` + ForcedWrite *bool `json:"forcedWrite"` + ForcedWriteAt *cases.Duration `json:"forcedWriteAt"` + Ready *condInput `json:"ready"` + ObservedGenerationCurrent bool `json:"observedGenerationCurrent"` + LastScaleTime string `json:"lastScaleTime"` } func (e stepExpected) expect() stepExpect { out := stepExpect{ - replicas: e.Replicas, - desired: e.Desired, - applied: e.Applied, - current: e.Current, - deferredSince: e.DeferredSince.expect(), - requeueAfter: durPtr(e.RequeueAfter), - noMoreEvents: e.NoMoreEvents, - forcedWrite: e.ForcedWrite, - forcedWriteAt: durPtr(e.ForcedWriteAt), + replicas: e.Replicas, + desired: e.Desired, + applied: e.Applied, + current: e.Current, + deferredSince: e.DeferredSince.expect(), + requeueAfter: durPtr(e.RequeueAfter), + noMoreEvents: e.NoMoreEvents, + forcedWrite: e.ForcedWrite, + forcedWriteAt: durPtr(e.ForcedWriteAt), + observedGenerationCurrent: e.ObservedGenerationCurrent, + lastScaleTime: parseLastScale(e.LastScaleTime), } if e.Condition != nil { cond := e.Condition.expect() out.condition = &cond } + if e.Ready != nil { + ready := e.Ready.expect() + out.ready = &ready + } if len(e.Events) > 0 { out.events = make([]eventExpect, len(e.Events)) for i, ev := range e.Events { @@ -186,6 +197,8 @@ func (s stepInput) step(exp stepExpected) scenarioStep { restoreStatus: s.RestoreStatus, skipReconcile: s.SkipReconcile, snapshotStatus: s.SnapshotStatus, + deleteDeploy: s.DeleteDeploy, + apiVersion: s.APIVersion, expect: exp.expect(), } if s.Deploy != nil { @@ -213,6 +226,7 @@ type scalerInputs struct { CheckTerminating *bool `json:"checkTerminating"` PlainReconciler bool `json:"plainReconciler"` ExpectNilRecommendations bool `json:"expectNilRecommendations"` + CloseServer bool `json:"closeServer"` Steps []stepInput `json:"steps"` } @@ -247,6 +261,7 @@ func (c scalerCase) scenario() scalerScenario { checkTerminating: in.CheckTerminating, plainReconciler: in.PlainReconciler, expectNilRecommendations: in.ExpectNilRecommendations, + closeServer: in.CloseServer, steps: steps, } if in.Deploy != nil { @@ -274,29 +289,40 @@ type customReconcileFile struct { StatusBy string `json:"statusBy"` } `json:"inputs"` Expected struct { - Replicas int32 `json:"replicas"` - Desired *int32 `json:"desired"` - DecisionStatus string `json:"decisionStatus"` - DecisionReason string `json:"decisionReason"` - DecisionMessageNonEmpty bool `json:"decisionMessageNonEmpty"` + Replicas int32 `json:"replicas"` + Desired *int32 `json:"desired"` + DecisionStatus string `json:"decisionStatus"` + DecisionReason string `json:"decisionReason"` + DecisionMessageNonEmpty bool `json:"decisionMessageNonEmpty"` + Ready *condInput `json:"ready"` + ObservedGenerationCurrent bool `json:"observedGenerationCurrent"` + LastScaleTime string `json:"lastScaleTime"` + MirrorDecisionMessage bool `json:"mirrorDecisionMessage"` } `json:"expected"` } func runCustomReconcileFile(c customReconcileFile) { exp := c.Expected tc := customReconcileCase{ - createBy: c.Inputs.CreateBy, - deployBy: c.Inputs.DeployBy, - closeServer: c.Inputs.CloseServer, - closeBy: c.Inputs.CloseBy, - reconcileBy: c.Inputs.ReconcileBy, - replicasBy: c.Inputs.ReplicasBy, - wantReplicas: exp.Replicas, - statusBy: c.Inputs.StatusBy, - wantDesired: exp.Desired, - decisionStatus: metav1.ConditionStatus(exp.DecisionStatus), - decisionReason: exp.DecisionReason, - decisionMessageNonEmpty: exp.DecisionMessageNonEmpty, + createBy: c.Inputs.CreateBy, + deployBy: c.Inputs.DeployBy, + closeServer: c.Inputs.CloseServer, + closeBy: c.Inputs.CloseBy, + reconcileBy: c.Inputs.ReconcileBy, + replicasBy: c.Inputs.ReplicasBy, + wantReplicas: exp.Replicas, + statusBy: c.Inputs.StatusBy, + wantDesired: exp.Desired, + decisionStatus: metav1.ConditionStatus(exp.DecisionStatus), + decisionReason: exp.DecisionReason, + decisionMessageNonEmpty: exp.DecisionMessageNonEmpty, + observedGenerationCurrent: exp.ObservedGenerationCurrent, + lastScaleTime: parseLastScale(exp.LastScaleTime), + mirrorDecisionMessage: exp.MirrorDecisionMessage, + } + if exp.Ready != nil { + ready := exp.Ready.expect() + tc.ready = &ready } if c.Inputs.Deploy != nil { st := c.Inputs.Deploy.state() diff --git a/internal/controller/autoscaling/llmscaler_controller.go b/internal/controller/autoscaling/llmscaler_controller.go index 568dfc4..9a07d22 100644 --- a/internal/controller/autoscaling/llmscaler_controller.go +++ b/internal/controller/autoscaling/llmscaler_controller.go @@ -138,6 +138,8 @@ func (r *LLMScalerReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( gv, err := schema.ParseGroupVersion(targetRef.APIVersion) if err != nil { logger.Error(err, "invalid apiVersion in targetRef") + r.recordReady(ctx, &scaler, metav1.ConditionFalse, autoscalingv1alpha1.ReasonInvalidSpec, + fmt.Sprintf("targetRef.apiVersion %q does not parse: %s", targetRef.APIVersion, err.Error())) return ctrl.Result{}, nil // Stop reconciling if spec is invalid } @@ -159,9 +161,12 @@ func (r *LLMScalerReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( if err != nil { logger.Error(err, "unable to fetch target resource", "kind", targetRef.Kind, "name", targetRef.Name) - // Requeue since the target might be created later. deferredSince and - // ScalingDeferred are left as they are, so the deadline carries over - // once the target is readable again. + // Requeue since the target might be created later. deferredSince, + // ScalingDeferred, and the replica counts are left as they are, so the + // deadline carries over once the target is readable again. Ready + // records why this sync could not read the target. + reason, msg := targetReadFailure(err, targetRef.Kind, targetRef.Name) + r.recordReady(ctx, &scaler, metav1.ConditionFalse, reason, msg) return ctrl.Result{RequeueAfter: retryPeriod}, nil } @@ -188,7 +193,7 @@ func (r *LLMScalerReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( // 3. Compute the recommendation: metrics, clamped to min/max, damped on the // way down by the stabilization window. This is what we want, independent of // how fast we are allowed to get there. - recommendedReplicas := r.recommendReplicas(ctx, &scaler, req.NamespacedName, specReplicas, metricReady) + recommendedReplicas, hold := r.recommendReplicas(ctx, &scaler, req.NamespacedName, specReplicas, metricReady) // 4. Execute Scale Action, rate-limited by maxStepReplicas. The cap belongs // here rather than in the recommendation: it governs how far a single write @@ -204,8 +209,8 @@ func (r *LLMScalerReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( // sync that holds a write and kept across guard changes, no-ops while a // guard still matches, and forced writes. It is cleared only when no guard // matches. It lives in status, so the deadline survives an operator restart. - // applied is the target's spec.replicas after this sync; a failed target - // update returns before the status write. + // applied is the target's spec.replicas after this sync. A failed target + // update records Ready and returns before replica counts are written. now := r.now() timeout := deferTimeout(&scaler) deferral := evaluateGuards(state, ScaleIntent{From: state.Spec, To: targetReplicas}) @@ -217,6 +222,7 @@ func (r *LLMScalerReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( paceRemaining := r.forcedWriteRemaining(req.NamespacedName, syncPeriod, now) requeueAfter := syncPeriod + var syncedMsg string switch { case noop && (!guarded || scaler.Status.DeferredSince == nil): // Nothing is being held back. A guard that matches on a no-op does not @@ -224,6 +230,7 @@ func (r *LLMScalerReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( r.endDeferral(req.NamespacedName, &scaler) setScalingDeferredCondition(&scaler, metav1.ConditionFalse, autoscalingv1alpha1.ReasonNotDeferred, "replica count is already at the step target") + syncedMsg = "replica count is at the step target" case noop: // A guard still matches and the clock is already running. There is // nothing to write, so deferredSince stays and this sync is not holding @@ -231,11 +238,13 @@ func (r *LLMScalerReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( logger.Info("No scale write this sync; deferral clock still running", "reason", deferral.message, "deferredSince", scaler.Status.DeferredSince.Time) setScalingDeferredCondition(&scaler, metav1.ConditionTrue, deferral.reason, scalingDeferredNoopMessage(deferral.message, scaler.Status.DeferredSince.Time, timeout)) + syncedMsg = readyDeferredMessage(deferral.reason) case guarded && !pastDeferTimeout(scaler.Status.DeferredSince, timeout, now): r.holdScaleDeferred(&scaler, deferral, int32(specReplicas), targetReplicas, timeout, now) logger.Info("Deferring scaling", "reason", deferral.message, "current", specReplicas, "ready", readyReplicas, "target", targetReplicas) until := scaler.Status.DeferredSince.Time.Add(timeout).Sub(now) requeueAfter = deferralRequeueAfter(syncPeriod, true, timeout, until, 0) + syncedMsg = readyDeferredMessage(deferral.reason) case guarded && paceRemaining > 0: // Deadline passed, but this process already forced a write within the // last sync period. See lastForcedWrite. @@ -246,21 +255,28 @@ func (r *LLMScalerReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( } setScalingDeferredCondition(&scaler, metav1.ConditionTrue, deferral.reason, msg) requeueAfter = deferralRequeueAfter(syncPeriod, true, timeout, 0, paceRemaining) + syncedMsg = readyDeferredMessage(deferral.reason) default: // A normal write (no guard) or a forced write the pace allows. A failed // target update returns before any deferral state is recorded. wrote, err := r.writeScaleTarget(ctx, &scaler, targetObj, req.NamespacedName, int32(specReplicas), targetReplicas, guarded, deferral, timeout, now) if err != nil { + r.recordTargetWriteFailure(ctx, &scaler, err, hold) return ctrl.Result{}, err } applied = wrote + syncedMsg = scaledMessage(int32(specReplicas), targetReplicas) } // 4. Update Status. currentReplicas is the raw ready count, not the ratio - // baseline: a fully-unready fleet reports 0. + // baseline: a fully-unready fleet reports 0. A missing recommendation + // outranks an otherwise clean sync: Ready names that, and ScalingDeferred + // still describes the scale decision. + applyReady(&scaler, hold, syncedMsg) scaler.Status.CurrentReplicas = int32(readyReplicas) scaler.Status.DesiredReplicas = recommendedReplicas scaler.Status.AppliedReplicas = applied + scaler.Status.ObservedGeneration = scaler.Generation if err := r.Status().Update(ctx, &scaler); err != nil { logger.Error(err, "failed to update scaler status") return ctrl.Result{}, err @@ -319,6 +335,9 @@ func (r *LLMScalerReconciler) writeScaleTarget(ctx context.Context, scaler *auto logger.Error(err, "failed to update target resource") return 0, err } + // lastScaleTime moves only when the target accepted the new replica count. + stamped := metav1.NewTime(now) + scaler.Status.LastScaleTime = &stamped // Pace the descent: pin the fleet at what we just wrote for one // stabilization window, and drop the recommendations that were measured @@ -339,7 +358,7 @@ func (r *LLMScalerReconciler) writeScaleTarget(ctx context.Context, scaler *auto } else { r.endDeferral(key, scaler) setScalingDeferredCondition(scaler, metav1.ConditionFalse, autoscalingv1alpha1.ReasonNotDeferred, - fmt.Sprintf("scaled %d -> %d", specReplicas, targetReplicas)) + scaledMessage(specReplicas, targetReplicas)) } return targetReplicas, nil } @@ -535,6 +554,79 @@ func setScalingDeferredCondition(scaler *autoscalingv1alpha1.LLMScaler, status m }) } +func setReadyCondition(scaler *autoscalingv1alpha1.LLMScaler, status metav1.ConditionStatus, reason, msg string) { + meta.SetStatusCondition(&scaler.Status.Conditions, metav1.Condition{ + Type: autoscalingv1alpha1.ConditionReady, + Status: status, + Reason: reason, + Message: msg, + ObservedGeneration: scaler.Generation, + }) +} + +// syncHold is why this sync has no usable recommendation. A zero reason means +// a metric or a decision produced one. Precedence when several failures apply +// is decided by the caller: InvalidSpec, then a target read or write failure, +// then a missing signal, then Synced. +type syncHold struct { + reason string + message string +} + +// readyDeferredMessage is the Ready summary while a guard holds the write. +// The guard reason and the clock stay on ScalingDeferred. +func readyDeferredMessage(reason string) string { + return fmt.Sprintf("scale action deferred: %s; see ScalingDeferred", reason) +} + +// scaledMessage is the Ready and ScalingDeferred text for a normal write. +func scaledMessage(from, to int32) string { + return fmt.Sprintf("scaled %d -> %d", from, to) +} + +// applyReady sets Ready for a sync that reached a scale decision. A hold +// (invalid spec, no decision, no metric) outranks an otherwise clean sync. +func applyReady(scaler *autoscalingv1alpha1.LLMScaler, hold syncHold, syncedMsg string) { + if hold.reason != "" { + setReadyCondition(scaler, metav1.ConditionFalse, hold.reason, hold.message) + return + } + setReadyCondition(scaler, metav1.ConditionTrue, autoscalingv1alpha1.ReasonSynced, syncedMsg) +} + +// targetReadFailure is the Ready reason for a target Get that failed. +func targetReadFailure(err error, kind, name string) (string, string) { + if apierrors.IsNotFound(err) { + return autoscalingv1alpha1.ReasonTargetNotFound, fmt.Sprintf("target %s %q not found", kind, name) + } + return autoscalingv1alpha1.ReasonTargetUnreadable, fmt.Sprintf("could not get target %s %q: %s", kind, name, err.Error()) +} + +// recordReady sets Ready and observedGeneration, then writes status. Replica +// counts, deferredSince, and ScalingDeferred are left as they stand. A +// status-write error is logged; the caller keeps its original result. +func (r *LLMScalerReconciler) recordReady(ctx context.Context, scaler *autoscalingv1alpha1.LLMScaler, status metav1.ConditionStatus, reason, msg string) { + logger := logf.FromContext(ctx) + setReadyCondition(scaler, status, reason, msg) + scaler.Status.ObservedGeneration = scaler.Generation + if err := r.Status().Update(ctx, scaler); err != nil { + logger.Error(err, "failed to update scaler status") + } +} + +// recordTargetWriteFailure records Ready after spec.replicas could not be +// written. InvalidSpec outranks the write failure; a missing metric or +// decision does not. The status write is best-effort. +func (r *LLMScalerReconciler) recordTargetWriteFailure(ctx context.Context, scaler *autoscalingv1alpha1.LLMScaler, writeErr error, hold syncHold) { + reason := autoscalingv1alpha1.ReasonTargetWriteFailed + msg := fmt.Sprintf("could not write spec.replicas: %s", writeErr.Error()) + if hold.reason == autoscalingv1alpha1.ReasonInvalidSpec { + reason = hold.reason + msg = hold.message + } + r.recordReady(ctx, scaler, metav1.ConditionFalse, reason, msg) +} + func (r *LLMScalerReconciler) event(obj client.Object, eventType, reason, msg string) { if r.Recorder == nil { return @@ -584,7 +676,10 @@ func (r *LLMScalerReconciler) stabilizeDesired(key types.NamespacedName, desired // from per-replica measurements, Custom is handed the count outright. Both feed // the same clamping and damping below, so a scaler behaves identically once the // number exists. -func (r *LLMScalerReconciler) recommendReplicas(ctx context.Context, scaler *autoscalingv1alpha1.LLMScaler, key types.NamespacedName, specReplicas, readyReplicas int64) int32 { +// +// hold says why there is no recommendation. It is zero when the count came +// from a metric or a decision. The numeric result does not depend on it. +func (r *LLMScalerReconciler) recommendReplicas(ctx context.Context, scaler *autoscalingv1alpha1.LLMScaler, key types.NamespacedName, specReplicas, readyReplicas int64) (int32, syncHold) { logger := logf.FromContext(ctx) // Use the metric recommendation when at least one metric was evaluated — it @@ -593,11 +688,15 @@ func (r *LLMScalerReconciler) recommendReplicas(ctx context.Context, scaler *aut var ( maxDesiredReplicas int32 haveMetric bool + hold syncHold ) if scaler.Spec.MetricProvider == autoscalingv1alpha1.MetricProviderCustom { - maxDesiredReplicas, haveMetric = r.desiredFromCustomProvider(ctx, scaler) + maxDesiredReplicas, haveMetric, hold = r.desiredFromCustomProvider(ctx, scaler) } else { - maxDesiredReplicas, haveMetric = r.computeDesiredFromMetrics(ctx, scaler, readyReplicas) + maxDesiredReplicas, haveMetric, hold = r.computeDesiredFromMetrics(ctx, scaler, readyReplicas) + } + if haveMetric { + hold = syncHold{} } desiredReplicas := int32(specReplicas) @@ -624,7 +723,7 @@ func (r *LLMScalerReconciler) recommendReplicas(ctx context.Context, scaler *aut desiredReplicas = stabilized } - return desiredReplicas + return desiredReplicas, hold } // scaleGuard limits a single scale-down write to maxStep replicas below @@ -685,16 +784,19 @@ func (r *LLMScalerReconciler) forgetRecommendations(key types.NamespacedName) { } // computeDesiredFromMetrics evaluates each metric's PromQL query and returns the -// maximum desired replica count (HPA-style: ceil(readyReplicas * value/target)) -// and whether any metric was successfully evaluated. -func (r *LLMScalerReconciler) computeDesiredFromMetrics(ctx context.Context, scaler *autoscalingv1alpha1.LLMScaler, readyReplicas int64) (int32, bool) { +// maximum desired replica count (HPA-style: ceil(readyReplicas * value/target)), +// whether any metric was successfully evaluated, and why none was when that is +// the case. +func (r *LLMScalerReconciler) computeDesiredFromMetrics(ctx context.Context, scaler *autoscalingv1alpha1.LLMScaler, readyReplicas int64) (int32, bool, syncHold) { logger := logf.FromContext(ctx) var maxDesired int32 haveMetric := false + var lastErr error for _, metric := range scaler.Spec.Metrics { currentValue, err := queryPrometheusScalar(scaler.Spec.ServerAddress, metric.Query, scaler.Spec.ServerHeaders) if err != nil { + lastErr = err logger.Error(err, "failed to fetch metric", "name", metric.Name, "query", metric.Query) continue } @@ -708,13 +810,15 @@ func (r *LLMScalerReconciler) computeDesiredFromMetrics(ctx context.Context, sca // haveMetric: a NaN guardrail alongside a failing primary metric leaves // maxDesired at 0, which clamps the whole fleet to minReplicas. if math.IsNaN(currentValue) || math.IsInf(currentValue, 0) { - logger.Error(fmt.Errorf("query returned %v", currentValue), "ignoring non-finite metric value", + lastErr = fmt.Errorf("query returned %v", currentValue) + logger.Error(lastErr, "ignoring non-finite metric value", "name", metric.Name, "query", metric.Query) continue } targetValue, err := parseTargetValue(metric.Target) if err != nil { + lastErr = err logger.Error(err, "invalid target value", "target", metric.Target) continue } @@ -737,7 +841,19 @@ func (r *LLMScalerReconciler) computeDesiredFromMetrics(ctx context.Context, sca maxDesired = metricDesired } } - return maxDesired, haveMetric + if !haveMetric { + return maxDesired, false, metricsHold(lastErr) + } + return maxDesired, true, syncHold{} +} + +// metricsHold is the Ready summary when no Prometheus metric could be evaluated. +func metricsHold(lastErr error) syncHold { + msg := "no metric could be evaluated; holding replicas" + if lastErr != nil { + msg = fmt.Sprintf("%s: %s", msg, lastErr.Error()) + } + return syncHold{reason: autoscalingv1alpha1.ReasonMetricsUnavailable, message: msg} } // desiredFromCustomProvider fetches the decision server's replica @@ -752,8 +868,8 @@ func (r *LLMScalerReconciler) computeDesiredFromMetrics(ctx context.Context, sca // A failure returns false rather than 0, so a decision server that is down or // answering in a schema we don't know holds the fleet where it is instead of // collapsing it to minReplicas — the same rule the Prometheus path applies to a -// query that cannot be evaluated. -func (r *LLMScalerReconciler) desiredFromCustomProvider(ctx context.Context, scaler *autoscalingv1alpha1.LLMScaler) (int32, bool) { +// query that cannot be evaluated. The hold says why, when the bool is false. +func (r *LLMScalerReconciler) desiredFromCustomProvider(ctx context.Context, scaler *autoscalingv1alpha1.LLMScaler) (int32, bool, syncHold) { logger := logf.FromContext(ctx) provider := scaler.Spec.CustomProvider @@ -762,7 +878,10 @@ func (r *LLMScalerReconciler) desiredFromCustomProvider(ctx context.Context, sca // the cluster predates the validation rule. logger.Error(fmt.Errorf("spec.customProvider is not set"), "Could not query the custom metric provider", "metricProvider", scaler.Spec.MetricProvider) - return 0, false + return 0, false, syncHold{ + reason: autoscalingv1alpha1.ReasonInvalidSpec, + message: "spec.customProvider is not set", + } } replicas, err := queryDecisionReplicas(scaler.Spec.ServerAddress, provider.Path, @@ -773,13 +892,16 @@ func (r *LLMScalerReconciler) desiredFromCustomProvider(ctx context.Context, sca logger.Info("No decision from the custom metric provider; holding replicas", "serverAddress", scaler.Spec.ServerAddress, "serviceId", provider.ServiceID, "error", err.Error()) setDecisionCondition(scaler, metav1.ConditionFalse, autoscalingv1alpha1.ReasonDecisionUnavailable, err.Error()) - return 0, false + return 0, false, syncHold{ + reason: autoscalingv1alpha1.ReasonDecisionUnavailable, + message: err.Error(), + } } logger.Info("Custom provider decision", "serviceId", provider.ServiceID, "calculatedDesired", replicas) setDecisionCondition(scaler, metav1.ConditionTrue, autoscalingv1alpha1.ReasonDecisionReceived, fmt.Sprintf("decision server recommends %d replicas", replicas)) - return replicas, true + return replicas, true, syncHold{} } func setDecisionCondition(scaler *autoscalingv1alpha1.LLMScaler, status metav1.ConditionStatus, reason, msg string) { diff --git a/internal/controller/autoscaling/llmscaler_controller_test.go b/internal/controller/autoscaling/llmscaler_controller_test.go index fc0d495..0745fd4 100644 --- a/internal/controller/autoscaling/llmscaler_controller_test.go +++ b/internal/controller/autoscaling/llmscaler_controller_test.go @@ -51,6 +51,8 @@ var _ = Describe("LLMScaler Controller", func() { ginkgocases.Entries("testdata/controller/deferral-status", scalerDesc)) DescribeTable("forced-write pacing", runScalerCase, ginkgocases.Entries("testdata/controller/forced-write-pacing", scalerDesc)) + DescribeTable("ready status", runScalerCase, + ginkgocases.Entries("testdata/controller/ready-status", scalerDesc)) for _, f := range cases.MustLoad[deferDefaultFile]("testdata/controller/crd-default") { tc := f.Case diff --git a/internal/controller/autoscaling/llmscaler_helpers_test.go b/internal/controller/autoscaling/llmscaler_helpers_test.go index 8f8916c..b320c92 100644 --- a/internal/controller/autoscaling/llmscaler_helpers_test.go +++ b/internal/controller/autoscaling/llmscaler_helpers_test.go @@ -381,7 +381,7 @@ func TestComputeDesiredFromMetrics(t *testing.T) { }}, }, } - desired, haveMetric := (&LLMScalerReconciler{}).computeDesiredFromMetrics( + desired, haveMetric, hold := (&LLMScalerReconciler{}).computeDesiredFromMetrics( context.Background(), scaler, tc.Inputs.Ready) if haveMetric != tc.Expected.Have { t.Errorf("haveMetric = %v, want %v", haveMetric, tc.Expected.Have) @@ -389,6 +389,16 @@ func TestComputeDesiredFromMetrics(t *testing.T) { if desired != tc.Expected.Desired { t.Errorf("desired = %d, want %d", desired, tc.Expected.Desired) } + if !tc.Expected.Have { + if hold.reason != autoscalingv1alpha1.ReasonMetricsUnavailable { + t.Errorf("hold reason = %q, want %q", hold.reason, autoscalingv1alpha1.ReasonMetricsUnavailable) + } + if !strings.Contains(hold.message, "no metric could be evaluated; holding replicas") { + t.Errorf("hold message = %q, want it to name the held replicas", hold.message) + } + } else if hold.reason != "" { + t.Errorf("hold reason = %q, want none", hold.reason) + } }) } diff --git a/internal/controller/autoscaling/llmscaler_scenario_test.go b/internal/controller/autoscaling/llmscaler_scenario_test.go index df4fc37..a44c586 100644 --- a/internal/controller/autoscaling/llmscaler_scenario_test.go +++ b/internal/controller/autoscaling/llmscaler_scenario_test.go @@ -146,28 +146,62 @@ type scenarioStep struct { restoreStatus bool skipReconcile bool snapshotStatus bool + deleteDeploy bool + apiVersion string expect stepExpect } +// lastScaleExpect says how a step checks status.lastScaleTime. The zero value +// leaves the field unchecked. set means this sync wrote it (and, when the +// step reconciled on the fake clock, that it equals that instant). unset +// means nil. unchanged means equal to the value from before the reconcile. +type lastScaleExpect int + +const ( + lastScaleSkip lastScaleExpect = iota + lastScaleSet + lastScaleUnset + lastScaleUnchanged +) + +func parseLastScale(s string) lastScaleExpect { + switch s { + case "": + return lastScaleSkip + case "set": + return lastScaleSet + case "unset": + return lastScaleUnset + case "unchanged": + return lastScaleUnchanged + default: + Fail(fmt.Sprintf("lastScaleTime %q", s)) + return lastScaleSkip + } +} + // stepExpect is the assertions for one step. Nil pointers and the zero // sinceExpect are skipped, so a step only states what that beat checked. type stepExpect struct { - replicas *int32 - desired *int32 - applied *int32 - current *int32 - deferredSince sinceExpect - condition *condExpect - requeueAfter *time.Duration - events []eventExpect - noMoreEvents bool - forcedWrite *bool - forcedWriteAt *time.Duration -} - -// condExpect is the ScalingDeferred condition. messageContains are substrings -// that must all appear in the message. observedGenerationCurrent requires -// the condition's ObservedGeneration to equal the scaler's generation. + replicas *int32 + desired *int32 + applied *int32 + current *int32 + deferredSince sinceExpect + condition *condExpect + requeueAfter *time.Duration + events []eventExpect + noMoreEvents bool + forcedWrite *bool + forcedWriteAt *time.Duration + ready *condExpect + observedGenerationCurrent bool + lastScaleTime lastScaleExpect +} + +// condExpect is one status condition. messageContains are substrings that +// must all appear in the message. observedGenerationCurrent requires the +// condition's ObservedGeneration to equal the scaler's generation. type condExpect struct { status metav1.ConditionStatus reason string @@ -196,6 +230,7 @@ type scalerScenario struct { checkTerminating *bool plainReconciler bool expectNilRecommendations bool + closeServer bool steps []scenarioStep } @@ -510,6 +545,9 @@ func runScalerScenario(sc scalerScenario) { if sc.expectNilRecommendations { Expect(run.r.recommendations).To(BeNil()) } + if sc.closeServer { + f.server.Close() + } for _, step := range sc.steps { runScenarioStep(run, step) } @@ -536,8 +574,18 @@ func runScenarioStep(run *scenarioRun, step scenarioStep) { if step.restoreStatus { restoreScalerStatus(f.ctx, f.scaler, run.held) } + if step.deleteDeploy { + deleteDeployment(f.ctx, f.deploy) + } + if step.apiVersion != "" { + setTargetAPIVersion(f.ctx, f.scaler, step.apiVersion) + } + var prevLastScale *metav1.Time + if step.expect.lastScaleTime == lastScaleUnchanged { + prevLastScale = scalerLastScaleTime(f.ctx, f.scaler) + } if step.skipReconcile { - assertStep(run, step) + assertStep(run, step, prevLastScale) return } if run.clk != nil { @@ -546,7 +594,7 @@ func runScenarioStep(run *scenarioRun, step scenarioStep) { var err error run.result, err = run.r.Reconcile(f.ctx, reconcile.Request{NamespacedName: f.scaler}) Expect(err).NotTo(HaveOccurred()) - assertStep(run, step) + assertStep(run, step, prevLastScale) if step.snapshotStatus { run.held = snapshotScalerStatus(f.ctx, f.scaler) } @@ -566,12 +614,12 @@ func restoreScalerStatus(ctx context.Context, nn types.NamespacedName, held *aut Expect(k8sClient.Status().Update(ctx, scaler)).To(Succeed()) } -func assertStep(run *scenarioRun, step scenarioStep) { +func assertStep(run *scenarioRun, step scenarioStep, prevLastScale *metav1.Time) { exp := step.expect if exp.replicas != nil { Expect(deploymentReplicas(run.fix.ctx, run.fix.deploy)).To(Equal(*exp.replicas)) } - assertScalerStatus(run, exp) + assertScalerStatus(run, step, prevLastScale) if exp.requeueAfter != nil { Expect(run.result.RequeueAfter).To(Equal(*exp.requeueAfter)) } @@ -579,9 +627,11 @@ func assertStep(run *scenarioRun, step scenarioStep) { assertForcedWrite(run, exp) } -func assertScalerStatus(run *scenarioRun, exp stepExpect) { +func assertScalerStatus(run *scenarioRun, step scenarioStep, prevLastScale *metav1.Time) { + exp := step.expect if exp.desired == nil && exp.applied == nil && exp.current == nil && - exp.deferredSince.kind == sinceUnchecked && exp.condition == nil { + exp.deferredSince.kind == sinceUnchecked && exp.condition == nil && + exp.ready == nil && !exp.observedGenerationCurrent && exp.lastScaleTime == lastScaleSkip { return } scaler := &autoscalingv1alpha1.LLMScaler{} @@ -597,8 +647,15 @@ func assertScalerStatus(run *scenarioRun, exp stepExpect) { } assertDeferredSince(scaler, exp.deferredSince, run.seed) if exp.condition != nil { - assertCondition(scaler, *exp.condition) + assertCondition(scaler, autoscalingv1alpha1.ConditionScalingDeferred, *exp.condition) + } + if exp.ready != nil { + assertCondition(scaler, autoscalingv1alpha1.ConditionReady, *exp.ready) } + if exp.observedGenerationCurrent { + Expect(scaler.Status.ObservedGeneration).To(Equal(scaler.Generation)) + } + assertLastScaleTime(scaler.Status.LastScaleTime, exp.lastScaleTime, prevLastScale, deferralStart.Add(step.at), run.clk != nil && !step.skipReconcile) } func assertDeferredSince(scaler *autoscalingv1alpha1.LLMScaler, exp sinceExpect, seed *time.Duration) { @@ -619,8 +676,9 @@ func assertDeferredSince(scaler *autoscalingv1alpha1.LLMScaler, exp sinceExpect, } } -func assertCondition(scaler *autoscalingv1alpha1.LLMScaler, exp condExpect) { - cond := scalingDeferredCondition(scaler) +func assertCondition(scaler *autoscalingv1alpha1.LLMScaler, condType string, exp condExpect) { + cond := meta.FindStatusCondition(scaler.Status.Conditions, condType) + Expect(cond).NotTo(BeNil()) if exp.status != "" { Expect(cond.Status).To(Equal(exp.status)) } @@ -639,6 +697,29 @@ func assertCondition(scaler *autoscalingv1alpha1.LLMScaler, exp condExpect) { } } +func assertLastScaleTime(got *metav1.Time, exp lastScaleExpect, prev *metav1.Time, when time.Time, checkWhen bool) { + switch exp { + case lastScaleSkip: + return + case lastScaleUnset: + Expect(got).To(BeNil()) + case lastScaleSet: + Expect(got).NotTo(BeNil()) + if checkWhen { + Expect(got.Time).To(BeTemporally("==", when)) + } + case lastScaleUnchanged: + if prev == nil { + Expect(got).To(BeNil()) + return + } + Expect(got).NotTo(BeNil()) + Expect(got.Time).To(BeTemporally("==", prev.Time)) + default: + Fail(fmt.Sprintf("unknown lastScaleTime check %d", exp)) + } +} + func assertEvents(run *scenarioRun, exp stepExpect) { if len(exp.events) == 0 && !exp.noMoreEvents { return @@ -688,28 +769,48 @@ func newScalerReconciler(clk *testingclock.FakeClock, rec events.EventRecorder) } } -func scalingDeferredCondition(scaler *autoscalingv1alpha1.LLMScaler) *metav1.Condition { - cond := meta.FindStatusCondition(scaler.Status.Conditions, autoscalingv1alpha1.ConditionScalingDeferred) - Expect(cond).NotTo(BeNil()) - return cond +func deleteDeployment(ctx context.Context, nn types.NamespacedName) { + deploy := &appsv1.Deployment{} + Expect(k8sClient.Get(ctx, nn, deploy)).To(Succeed()) + Expect(k8sClient.Delete(ctx, deploy)).To(Succeed()) +} + +func setTargetAPIVersion(ctx context.Context, nn types.NamespacedName, apiVersion string) { + scaler := &autoscalingv1alpha1.LLMScaler{} + Expect(k8sClient.Get(ctx, nn, scaler)).To(Succeed()) + scaler.Spec.TargetRef.APIVersion = apiVersion + Expect(k8sClient.Update(ctx, scaler)).To(Succeed()) +} + +func scalerLastScaleTime(ctx context.Context, nn types.NamespacedName) *metav1.Time { + scaler := &autoscalingv1alpha1.LLMScaler{} + Expect(k8sClient.Get(ctx, nn, scaler)).To(Succeed()) + if scaler.Status.LastScaleTime == nil { + return nil + } + return scaler.Status.LastScaleTime.DeepCopy() } // customReconcileCase drives one decision-server reconcile. The *By fields are // By texts for each phase; empty strings skip the label. type customReconcileCase struct { - createBy string - deploy *deployState - deployBy string - closeServer bool - closeBy string - reconcileBy string - replicasBy string - wantReplicas int32 - statusBy string - wantDesired *int32 - decisionStatus metav1.ConditionStatus - decisionReason string - decisionMessageNonEmpty bool + createBy string + deploy *deployState + deployBy string + closeServer bool + closeBy string + reconcileBy string + replicasBy string + wantReplicas int32 + statusBy string + wantDesired *int32 + decisionStatus metav1.ConditionStatus + decisionReason string + decisionMessageNonEmpty bool + ready *condExpect + observedGenerationCurrent bool + lastScaleTime lastScaleExpect + mirrorDecisionMessage bool } func runCustomProviderReconcile(tc customReconcileCase) { @@ -745,7 +846,9 @@ func runCustomProviderReconcile(tc customReconcileCase) { } Expect(deploymentReplicas(f.ctx, f.deploy)).To(Equal(tc.wantReplicas)) - if tc.wantDesired == nil && tc.decisionStatus == "" { + needStatus := tc.wantDesired != nil || tc.decisionStatus != "" || tc.ready != nil || + tc.observedGenerationCurrent || tc.lastScaleTime != lastScaleSkip || tc.mirrorDecisionMessage + if !needStatus { return } if tc.statusBy != "" { @@ -756,17 +859,31 @@ func runCustomProviderReconcile(tc customReconcileCase) { if tc.wantDesired != nil { Expect(scaler.Status.DesiredReplicas).To(Equal(*tc.wantDesired)) } - if tc.decisionStatus == "" { - return + if tc.observedGenerationCurrent { + Expect(scaler.Status.ObservedGeneration).To(Equal(scaler.Generation)) } - cond := meta.FindStatusCondition(scaler.Status.Conditions, autoscalingv1alpha1.ConditionDecisionAvailable) - Expect(cond).NotTo(BeNil()) - Expect(cond.Status).To(Equal(tc.decisionStatus)) - if tc.decisionReason != "" { - Expect(cond.Reason).To(Equal(tc.decisionReason)) + assertLastScaleTime(scaler.Status.LastScaleTime, tc.lastScaleTime, nil, time.Time{}, false) + var decision *metav1.Condition + if tc.decisionStatus != "" || tc.mirrorDecisionMessage { + decision = meta.FindStatusCondition(scaler.Status.Conditions, autoscalingv1alpha1.ConditionDecisionAvailable) + Expect(decision).NotTo(BeNil()) + } + if tc.decisionStatus != "" { + Expect(decision.Status).To(Equal(tc.decisionStatus)) + if tc.decisionReason != "" { + Expect(decision.Reason).To(Equal(tc.decisionReason)) + } + if tc.decisionMessageNonEmpty { + Expect(decision.Message).NotTo(BeEmpty()) + } + } + if tc.ready != nil { + assertCondition(scaler, autoscalingv1alpha1.ConditionReady, *tc.ready) } - if tc.decisionMessageNonEmpty { - Expect(cond.Message).NotTo(BeEmpty()) + if tc.mirrorDecisionMessage { + ready := meta.FindStatusCondition(scaler.Status.Conditions, autoscalingv1alpha1.ConditionReady) + Expect(ready).NotTo(BeNil()) + Expect(ready.Message).To(Equal(decision.Message)) } } diff --git a/internal/controller/autoscaling/llmscaler_status_test.go b/internal/controller/autoscaling/llmscaler_status_test.go new file mode 100644 index 0000000..91289ee --- /dev/null +++ b/internal/controller/autoscaling/llmscaler_status_test.go @@ -0,0 +1,258 @@ +/* +Copyright 2026. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package controller + +import ( + "context" + "fmt" + "net/http" + "net/http/httptest" + "strings" + "testing" + "time" + + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/client/interceptor" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + + autoscalingv1alpha1 "github.com/modelsphere/llm-operator/api/autoscaling/v1alpha1" +) + +// TestTargetWriteFailedRecordsReady covers the path envtest cannot force: the +// target update returns an error. Ready and observedGeneration are recorded; +// the deferral and the replica counts already in status are left alone, and +// the original error is returned. +func TestTargetWriteFailedRecordsReady(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + _, _ = fmt.Fprintf(w, `{"status":"success","data":{"result":[{"value":[0,"%s"]}]}}`, "0.85") + })) + defer srv.Close() + + deferred := metav1.NewTime(time.Date(2026, 3, 1, 12, 0, 0, 0, time.UTC)) + scaler := prometheusScaler(srv.URL) + scaler.Status = autoscalingv1alpha1.LLMScalerStatus{ + CurrentReplicas: 4, + DesiredReplicas: 4, + AppliedReplicas: 4, + DeferredSince: &deferred, + } + meta.SetStatusCondition(&scaler.Status.Conditions, metav1.Condition{ + Type: autoscalingv1alpha1.ConditionScalingDeferred, + Status: metav1.ConditionTrue, + Reason: autoscalingv1alpha1.ReasonRolloutInProgress, + Message: "seeded", + ObservedGeneration: 2, + }) + + scheme := statusScheme(t) + c := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(rolledDeployment(), scaler). + WithStatusSubresource(&autoscalingv1alpha1.LLMScaler{}). + WithInterceptorFuncs(interceptor.Funcs{ + Update: func(ctx context.Context, c client.WithWatch, obj client.Object, opts ...client.UpdateOption) error { + if _, ok := obj.(*unstructured.Unstructured); ok { + return fmt.Errorf("injected write failure") + } + return c.Update(ctx, obj, opts...) + }, + }). + Build() + + result, err := (&LLMScalerReconciler{Client: c, Scheme: scheme}).Reconcile(context.Background(), reconcile.Request{ + NamespacedName: types.NamespacedName{Namespace: scaler.Namespace, Name: scaler.Name}, + }) + if err == nil || !strings.Contains(err.Error(), "injected write failure") { + t.Fatalf("Reconcile error = %v, want the injected write failure", err) + } + if result.RequeueAfter != 0 { + t.Errorf("RequeueAfter = %s, want 0", result.RequeueAfter) + } + + got := &autoscalingv1alpha1.LLMScaler{} + if err := c.Get(context.Background(), client.ObjectKeyFromObject(scaler), got); err != nil { + t.Fatal(err) + } + if got.Generation == 0 { + t.Fatal("generation = 0, the fixture did not stick") + } + if got.Status.ObservedGeneration != got.Generation { + t.Errorf("observedGeneration = %d, want generation %d", got.Status.ObservedGeneration, got.Generation) + } + ready := meta.FindStatusCondition(got.Status.Conditions, autoscalingv1alpha1.ConditionReady) + if ready == nil { + t.Fatal("Ready condition missing") + } + if ready.Status != metav1.ConditionFalse || ready.Reason != autoscalingv1alpha1.ReasonTargetWriteFailed { + t.Errorf("Ready = %s/%s, want False/%s", ready.Status, ready.Reason, autoscalingv1alpha1.ReasonTargetWriteFailed) + } + if !strings.Contains(ready.Message, "injected write failure") { + t.Errorf("Ready message = %q, want the injected error", ready.Message) + } + if ready.ObservedGeneration != got.Generation { + t.Errorf("Ready.observedGeneration = %d, want %d", ready.ObservedGeneration, got.Generation) + } + if got.Status.LastScaleTime != nil { + t.Errorf("lastScaleTime = %s, want unset", got.Status.LastScaleTime.Time) + } + if got.Status.CurrentReplicas != 4 || got.Status.DesiredReplicas != 4 || got.Status.AppliedReplicas != 4 { + t.Errorf("replicas current/desired/applied = %d/%d/%d, want 4/4/4", + got.Status.CurrentReplicas, got.Status.DesiredReplicas, got.Status.AppliedReplicas) + } + if got.Status.DeferredSince == nil || !got.Status.DeferredSince.Time.Equal(deferred.Time) { + t.Errorf("deferredSince = %v, want %s", got.Status.DeferredSince, deferred.Time) + } + deferredCond := meta.FindStatusCondition(got.Status.Conditions, autoscalingv1alpha1.ConditionScalingDeferred) + if deferredCond == nil || deferredCond.Status != metav1.ConditionTrue || + deferredCond.Reason != autoscalingv1alpha1.ReasonRolloutInProgress || deferredCond.Message != "seeded" { + t.Errorf("ScalingDeferred = %+v, want the seeded condition", deferredCond) + } + + deploy := &appsv1.Deployment{} + if err := c.Get(context.Background(), client.ObjectKey{Namespace: "ns", Name: "d"}, deploy); err != nil { + t.Fatal(err) + } + if deploy.Spec.Replicas == nil || *deploy.Spec.Replicas != 1 { + t.Errorf("deployment replicas = %v, want 1", deploy.Spec.Replicas) + } +} + +// TestMissingCustomProviderRecordsInvalidSpec runs the admission-rejected +// pairing through the controller. The fleet stays where it is, and Ready +// records InvalidSpec without a DecisionAvailable condition. +func TestMissingCustomProviderRecordsInvalidSpec(t *testing.T) { + scaler := prometheusScaler("http://127.0.0.1:9") + scaler.Spec.MetricProvider = autoscalingv1alpha1.MetricProviderCustom + scaler.Spec.CustomProvider = nil + scaler.Spec.Metrics = nil + + scheme := statusScheme(t) + c := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(rolledDeployment(), scaler). + WithStatusSubresource(&autoscalingv1alpha1.LLMScaler{}). + Build() + + result, err := (&LLMScalerReconciler{Client: c, Scheme: scheme}).Reconcile(context.Background(), reconcile.Request{ + NamespacedName: types.NamespacedName{Namespace: scaler.Namespace, Name: scaler.Name}, + }) + if err != nil { + t.Fatalf("Reconcile error = %v", err) + } + if result.RequeueAfter != 15*time.Second { + t.Errorf("RequeueAfter = %s, want 15s", result.RequeueAfter) + } + + got := &autoscalingv1alpha1.LLMScaler{} + if err := c.Get(context.Background(), client.ObjectKeyFromObject(scaler), got); err != nil { + t.Fatal(err) + } + ready := meta.FindStatusCondition(got.Status.Conditions, autoscalingv1alpha1.ConditionReady) + if ready == nil || ready.Status != metav1.ConditionFalse || ready.Reason != autoscalingv1alpha1.ReasonInvalidSpec { + t.Fatalf("Ready = %+v, want False/InvalidSpec", ready) + } + if ready.Message != "spec.customProvider is not set" { + t.Errorf("Ready message = %q", ready.Message) + } + if meta.FindStatusCondition(got.Status.Conditions, autoscalingv1alpha1.ConditionDecisionAvailable) != nil { + t.Error("DecisionAvailable was set; a missing customProvider does not query") + } + deferredCond := meta.FindStatusCondition(got.Status.Conditions, autoscalingv1alpha1.ConditionScalingDeferred) + if deferredCond == nil || deferredCond.Status != metav1.ConditionFalse || deferredCond.Reason != autoscalingv1alpha1.ReasonNotDeferred { + t.Errorf("ScalingDeferred = %+v, want False/NotDeferred", deferredCond) + } + if got.Status.DesiredReplicas != 1 || got.Status.AppliedReplicas != 1 || got.Status.CurrentReplicas != 1 { + t.Errorf("replicas desired/applied/current = %d/%d/%d, want 1/1/1", + got.Status.DesiredReplicas, got.Status.AppliedReplicas, got.Status.CurrentReplicas) + } + if got.Status.LastScaleTime != nil { + t.Errorf("lastScaleTime = %s, want unset", got.Status.LastScaleTime.Time) + } + if got.Generation == 0 || got.Status.ObservedGeneration != got.Generation { + t.Errorf("observedGeneration = %d, generation = %d", got.Status.ObservedGeneration, got.Generation) + } + + deploy := &appsv1.Deployment{} + if err := c.Get(context.Background(), client.ObjectKey{Namespace: "ns", Name: "d"}, deploy); err != nil { + t.Fatal(err) + } + if deploy.Spec.Replicas == nil || *deploy.Spec.Replicas != 1 { + t.Errorf("deployment replicas = %v, want 1", deploy.Spec.Replicas) + } +} + +func statusScheme(t *testing.T) *runtime.Scheme { + t.Helper() + scheme := runtime.NewScheme() + if err := autoscalingv1alpha1.AddToScheme(scheme); err != nil { + t.Fatal(err) + } + if err := appsv1.AddToScheme(scheme); err != nil { + t.Fatal(err) + } + return scheme +} + +func prometheusScaler(serverURL string) *autoscalingv1alpha1.LLMScaler { + return &autoscalingv1alpha1.LLMScaler{ + ObjectMeta: metav1.ObjectMeta{Name: "s", Namespace: "ns", Generation: 7}, + Spec: autoscalingv1alpha1.LLMScalerSpec{ + TargetRef: autoscalingv1alpha1.TargetRef{ + APIVersion: "apps/v1", + Kind: deploymentKind, + Name: "d", + }, + ServerAddress: serverURL, + MinReplicas: 1, + MaxReplicas: 5, + Metrics: []autoscalingv1alpha1.MetricSpec{{ + Query: "avg(vllm:kv_cache_usage_perc)", + Target: "0.5", + }}, + }, + } +} + +func rolledDeployment() *appsv1.Deployment { + replicas := int32(1) + return &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{Name: "d", Namespace: "ns", Generation: 1}, + Spec: appsv1.DeploymentSpec{ + Replicas: &replicas, + Selector: &metav1.LabelSelector{MatchLabels: map[string]string{appLabelKey: "d"}}, + Template: corev1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{appLabelKey: "d"}}, + Spec: corev1.PodSpec{Containers: []corev1.Container{{Name: "nginx", Image: "nginx"}}}, + }, + }, + Status: appsv1.DeploymentStatus{ + ObservedGeneration: 1, + Replicas: 1, + UpdatedReplicas: 1, + ReadyReplicas: 1, + AvailableReplicas: 1, + }, + } +} diff --git a/internal/controller/autoscaling/testdata/controller/custom-reconcile/should-hold-replicas-when-the-decision-server-is-unreachable.yaml b/internal/controller/autoscaling/testdata/controller/custom-reconcile/should-hold-replicas-when-the-decision-server-is-unreachable.yaml index 6537faa..69ba0a1 100644 --- a/internal/controller/autoscaling/testdata/controller/custom-reconcile/should-hold-replicas-when-the-decision-server-is-unreachable.yaml +++ b/internal/controller/autoscaling/testdata/controller/custom-reconcile/should-hold-replicas-when-the-decision-server-is-unreachable.yaml @@ -15,6 +15,14 @@ "statusBy": "checking the failure is reported on the status" "expected": "replicas": 3 + "desired": 3 "decisionStatus": "False" "decisionReason": "DecisionUnavailable" "decisionMessageNonEmpty": true + "ready": + "status": "False" + "reason": "DecisionUnavailable" + "observedGenerationCurrent": true + "mirrorDecisionMessage": true + "lastScaleTime": "unset" + "observedGenerationCurrent": true diff --git a/internal/controller/autoscaling/testdata/controller/custom-reconcile/should-scale-the-deployment-to-the-count-the-decision-server-reports.yaml b/internal/controller/autoscaling/testdata/controller/custom-reconcile/should-scale-the-deployment-to-the-count-the-decision-server-reports.yaml index 734a302..7691e95 100644 --- a/internal/controller/autoscaling/testdata/controller/custom-reconcile/should-scale-the-deployment-to-the-count-the-decision-server-reports.yaml +++ b/internal/controller/autoscaling/testdata/controller/custom-reconcile/should-scale-the-deployment-to-the-count-the-decision-server-reports.yaml @@ -10,3 +10,11 @@ "replicas": 4 "desired": 4 "decisionStatus": "True" + "ready": + "status": "True" + "reason": "Synced" + "messageContains": + - "scaled 1 -> 4" + "observedGenerationCurrent": true + "lastScaleTime": "set" + "observedGenerationCurrent": true diff --git a/internal/controller/autoscaling/testdata/controller/deferral-timeout/should-force-a-scale-up-once-a-stuck-rollout-exceeds-the-deferral-timeout.yaml b/internal/controller/autoscaling/testdata/controller/deferral-timeout/should-force-a-scale-up-once-a-stuck-rollout-exceeds-the-deferral-timeout.yaml index a97d880..3bdd820 100644 --- a/internal/controller/autoscaling/testdata/controller/deferral-timeout/should-force-a-scale-up-once-a-stuck-rollout-exceeds-the-deferral-timeout.yaml +++ b/internal/controller/autoscaling/testdata/controller/deferral-timeout/should-force-a-scale-up-once-a-stuck-rollout-exceeds-the-deferral-timeout.yaml @@ -27,6 +27,14 @@ - "2026-03-01T12:00:00Z" - "will apply after 60s" "observedGenerationCurrent": true + "ready": + "status": "True" + "reason": "Synced" + "messageContains": + - "scale action deferred: RolloutInProgress; see ScalingDeferred" + "observedGenerationCurrent": true + "lastScaleTime": "unset" + "observedGenerationCurrent": true "events": - "prefix": "Normal ScalingDeferred" "contains": @@ -47,6 +55,14 @@ "messageContains": - "scaling 1 -> 2 anyway" "observedGenerationCurrent": true + "ready": + "status": "True" + "reason": "Synced" + "messageContains": + - "scaled 1 -> 2" + "observedGenerationCurrent": true + "lastScaleTime": "set" + "observedGenerationCurrent": true "events": - "prefix": "Warning DeferralTimedOut" "contains": diff --git a/internal/controller/autoscaling/testdata/controller/forced-write-pacing/should-not-take-a-second-forced-step-until-a-sync-period-has-elapsed.yaml b/internal/controller/autoscaling/testdata/controller/forced-write-pacing/should-not-take-a-second-forced-step-until-a-sync-period-has-elapsed.yaml index 6e348a5..43f369b 100644 --- a/internal/controller/autoscaling/testdata/controller/forced-write-pacing/should-not-take-a-second-forced-step-until-a-sync-period-has-elapsed.yaml +++ b/internal/controller/autoscaling/testdata/controller/forced-write-pacing/should-not-take-a-second-forced-step-until-a-sync-period-has-elapsed.yaml @@ -30,6 +30,14 @@ "steps": - {} - "replicas": 3 + "lastScaleTime": "set" + "ready": + "status": "True" + "reason": "Synced" + "messageContains": + - "scaled 5 -> 3" + "observedGenerationCurrent": true + "observedGenerationCurrent": true - {} - "replicas": 3 "requeueAfter": "15s" @@ -39,6 +47,14 @@ "status": "True" "messageContains": - "forced write paced to one per sync period" + "ready": + "status": "True" + "reason": "Synced" + "messageContains": + - "scale action deferred:" + - "see ScalingDeferred" + "observedGenerationCurrent": true + "lastScaleTime": "unchanged" - "replicas": 3 "requeueAfter": "1s" "deferredSince": "0s" diff --git a/internal/controller/autoscaling/testdata/controller/ready-status/should-report-invalid-spec-when-targetref-apiversion-does-not-parse.yaml b/internal/controller/autoscaling/testdata/controller/ready-status/should-report-invalid-spec-when-targetref-apiversion-does-not-parse.yaml new file mode 100644 index 0000000..befe8ff --- /dev/null +++ b/internal/controller/autoscaling/testdata/controller/ready-status/should-report-invalid-spec-when-targetref-apiversion-does-not-parse.yaml @@ -0,0 +1,48 @@ +# A normal write lands first. An apiVersion with more than one slash does not +# parse, so the next sync records InvalidSpec and leaves the previous status +# and the target replica count alone. +"name": "should-report-invalid-spec-when-targetref-apiversion-does-not-parse" +"description": "should report InvalidSpec when targetRef.apiVersion does not parse" +"inputs": + "steps": + - "by": "scaling once so status is populated" + "at": "0s" + - "by": "setting an apiVersion that does not parse" + "at": "5s" + "apiVersion": "apps/v1/extra" +"expected": + "steps": + - "replicas": 2 + "desired": 2 + "applied": 2 + "current": 1 + "condition": + "status": "False" + "reason": "NotDeferred" + "messageContains": + - "scaled 1 -> 2" + "ready": + "status": "True" + "reason": "Synced" + "messageContains": + - "scaled 1 -> 2" + "lastScaleTime": "set" + "observedGenerationCurrent": true + - "replicas": 2 + "desired": 2 + "applied": 2 + "current": 1 + "requeueAfter": "0s" + "condition": + "status": "False" + "reason": "NotDeferred" + "messageContains": + - "scaled 1 -> 2" + "ready": + "status": "False" + "reason": "InvalidSpec" + "messageContains": + - "apps/v1/extra" + "observedGenerationCurrent": true + "lastScaleTime": "unchanged" + "observedGenerationCurrent": true diff --git a/internal/controller/autoscaling/testdata/controller/ready-status/should-report-metrics-unavailable-when-prometheus-is-unreachable.yaml b/internal/controller/autoscaling/testdata/controller/ready-status/should-report-metrics-unavailable-when-prometheus-is-unreachable.yaml new file mode 100644 index 0000000..513c6db --- /dev/null +++ b/internal/controller/autoscaling/testdata/controller/ready-status/should-report-metrics-unavailable-when-prometheus-is-unreachable.yaml @@ -0,0 +1,30 @@ +# The query cannot be evaluated, so the replica count stays at spec. +"name": "should-report-metrics-unavailable-when-prometheus-is-unreachable" +"description": "should report MetricsUnavailable when Prometheus is unreachable" +"inputs": + "closeServer": true + "steps": + - "by": "reconciling with Prometheus down" + "at": "0s" +"expected": + "steps": + - "replicas": 1 + "desired": 1 + "applied": 1 + "current": 1 + "deferredSince": null + "requeueAfter": "15s" + "condition": + "status": "False" + "reason": "NotDeferred" + "messageContains": + - "already at the step target" + "ready": + "status": "False" + "reason": "MetricsUnavailable" + "messageContains": + - "no metric could be evaluated; holding replicas" + - "failed to query prometheus" + "observedGenerationCurrent": true + "lastScaleTime": "unset" + "observedGenerationCurrent": true diff --git a/internal/controller/autoscaling/testdata/controller/ready-status/should-report-ready-and-lastscaletime-when-a-scale-write-succeeds.yaml b/internal/controller/autoscaling/testdata/controller/ready-status/should-report-ready-and-lastscaletime-when-a-scale-write-succeeds.yaml new file mode 100644 index 0000000..63367f2 --- /dev/null +++ b/internal/controller/autoscaling/testdata/controller/ready-status/should-report-ready-and-lastscaletime-when-a-scale-write-succeeds.yaml @@ -0,0 +1,28 @@ +# ceil(1 * 0.85/0.5) = ceil(1.7) = 2. The mock sample is 0.85 and the target is 0.5. +"name": "should-report-ready-and-lastscaletime-when-a-scale-write-succeeds" +"description": "should report Ready and lastScaleTime when a scale write succeeds" +"inputs": + "steps": + - "by": "scaling the deployment" + "at": "0s" +"expected": + "steps": + - "replicas": 2 + "desired": 2 + "applied": 2 + "current": 1 + "requeueAfter": "15s" + "condition": + "status": "False" + "reason": "NotDeferred" + "messageContains": + - "scaled 1 -> 2" + "observedGenerationCurrent": true + "ready": + "status": "True" + "reason": "Synced" + "messageContains": + - "scaled 1 -> 2" + "observedGenerationCurrent": true + "lastScaleTime": "set" + "observedGenerationCurrent": true diff --git a/internal/controller/autoscaling/testdata/controller/ready-status/should-report-ready-on-a-no-op-without-a-lastscaletime.yaml b/internal/controller/autoscaling/testdata/controller/ready-status/should-report-ready-on-a-no-op-without-a-lastscaletime.yaml new file mode 100644 index 0000000..8730f97 --- /dev/null +++ b/internal/controller/autoscaling/testdata/controller/ready-status/should-report-ready-on-a-no-op-without-a-lastscaletime.yaml @@ -0,0 +1,30 @@ +# ceil(1 * 0.5/0.5) = 1, which is already spec, and the deployment is fully rolled out. +"name": "should-report-ready-on-a-no-op-without-a-lastscaletime" +"description": "should report Ready on a no-op without a lastScaleTime" +"inputs": + "metric": "0.5" + "steps": + - "by": "reconciling a fleet that is already at the step target" + "at": "0s" +"expected": + "steps": + - "replicas": 1 + "desired": 1 + "applied": 1 + "current": 1 + "deferredSince": null + "requeueAfter": "15s" + "condition": + "status": "False" + "reason": "NotDeferred" + "messageContains": + - "already at the step target" + "observedGenerationCurrent": true + "ready": + "status": "True" + "reason": "Synced" + "messageContains": + - "replica count is at the step target" + "observedGenerationCurrent": true + "lastScaleTime": "unset" + "observedGenerationCurrent": true diff --git a/internal/controller/autoscaling/testdata/controller/ready-status/should-report-target-not-found-and-leave-the-deferral-untouched.yaml b/internal/controller/autoscaling/testdata/controller/ready-status/should-report-target-not-found-and-leave-the-deferral-untouched.yaml new file mode 100644 index 0000000..bdb272f --- /dev/null +++ b/internal/controller/autoscaling/testdata/controller/ready-status/should-report-target-not-found-and-leave-the-deferral-untouched.yaml @@ -0,0 +1,57 @@ +# The first sync defers a scale-up (wanted 2) while the deployment rolls out. +# Deleting the target must record Ready without clearing that deferral or the +# replica counts already written. +"name": "should-report-target-not-found-and-leave-the-deferral-untouched" +"description": "should report TargetNotFound and leave the deferral untouched" +"inputs": + "rollout": true + "steps": + - "by": "deferring the scale-up and stamping deferredSince" + "at": "0s" + - "by": "deleting the target and reconciling again" + "at": "10s" + "deleteDeploy": true +"expected": + "steps": + - "replicas": 1 + "desired": 2 + "applied": 1 + "current": 1 + "deferredSince": "0s" + "requeueAfter": "15s" + "condition": + "status": "True" + "reason": "RolloutInProgress" + "messageContains": + - "holding 1 replicas (wanted 2)" + - "Deployment rollout in progress" + - "will apply after 600s" + "observedGenerationCurrent": true + "ready": + "status": "True" + "reason": "Synced" + "messageContains": + - "scale action deferred: RolloutInProgress; see ScalingDeferred" + "observedGenerationCurrent": true + "lastScaleTime": "unset" + "observedGenerationCurrent": true + - "desired": 2 + "applied": 1 + "current": 1 + "deferredSince": "0s" + "requeueAfter": "10s" + "condition": + "status": "True" + "reason": "RolloutInProgress" + "messageContains": + - "holding 1 replicas (wanted 2)" + - "Deployment rollout in progress" + - "will apply after 600s" + "ready": + "status": "False" + "reason": "TargetNotFound" + "messageContains": + - "not found" + "observedGenerationCurrent": true + "lastScaleTime": "unset" + "observedGenerationCurrent": true diff --git a/internal/controller/autoscaling/testdata/controller/ready-status/should-stay-ready-while-a-rollout-defers-the-next-scale.yaml b/internal/controller/autoscaling/testdata/controller/ready-status/should-stay-ready-while-a-rollout-defers-the-next-scale.yaml new file mode 100644 index 0000000..6717e0d --- /dev/null +++ b/internal/controller/autoscaling/testdata/controller/ready-status/should-stay-ready-while-a-rollout-defers-the-next-scale.yaml @@ -0,0 +1,52 @@ +# First sync: ceil(1 * 0.85/0.5) = 2, a normal write. Second sync raises the +# metric to 2.5 while the new spec is still unobserved, so ceil(1 * 2.5/0.5) = 5 +# is held by the rollout guard. lastScaleTime stays at the first write. +"name": "should-stay-ready-while-a-rollout-defers-the-next-scale" +"description": "should stay Ready while a rollout defers the next scale" +"inputs": + "steps": + - "by": "scaling 1 -> 2" + "at": "0s" + - "by": "raising the metric while the rollout is still in progress" + "at": "10s" + "metric": "2.5" + "rollout": true +"expected": + "steps": + - "replicas": 2 + "desired": 2 + "applied": 2 + "condition": + "status": "False" + "reason": "NotDeferred" + "messageContains": + - "scaled 1 -> 2" + "ready": + "status": "True" + "reason": "Synced" + "messageContains": + - "scaled 1 -> 2" + "observedGenerationCurrent": true + "lastScaleTime": "set" + "observedGenerationCurrent": true + - "replicas": 2 + "desired": 5 + "applied": 2 + "current": 1 + "deferredSince": "10s" + "requeueAfter": "15s" + "condition": + "status": "True" + "reason": "RolloutInProgress" + "messageContains": + - "holding 2 replicas (wanted 5)" + - "Deployment rollout in progress" + "observedGenerationCurrent": true + "ready": + "status": "True" + "reason": "Synced" + "messageContains": + - "scale action deferred: RolloutInProgress; see ScalingDeferred" + "observedGenerationCurrent": true + "lastScaleTime": "unchanged" + "observedGenerationCurrent": true diff --git a/internal/controller/autoscaling/testdata/helpers/recommend-custom/a-missing-customprovider-holds-the-fleet-where-it-is.yaml b/internal/controller/autoscaling/testdata/helpers/recommend-custom/a-missing-customprovider-holds-the-fleet-where-it-is.yaml index 33ecae8..7429797 100644 --- a/internal/controller/autoscaling/testdata/helpers/recommend-custom/a-missing-customprovider-holds-the-fleet-where-it-is.yaml +++ b/internal/controller/autoscaling/testdata/helpers/recommend-custom/a-missing-customprovider-holds-the-fleet-where-it-is.yaml @@ -13,3 +13,5 @@ "pinAPIVersion": true "expected": "replicas": 8 + "holdReason": "InvalidSpec" + "holdMessageContains": "spec.customProvider is not set" diff --git a/internal/controller/autoscaling/testdata/helpers/recommend-custom/an-unreadable-decision-holds-the-fleet-where-it-is.yaml b/internal/controller/autoscaling/testdata/helpers/recommend-custom/an-unreadable-decision-holds-the-fleet-where-it-is.yaml index 26c0199..4122aa6 100644 --- a/internal/controller/autoscaling/testdata/helpers/recommend-custom/an-unreadable-decision-holds-the-fleet-where-it-is.yaml +++ b/internal/controller/autoscaling/testdata/helpers/recommend-custom/an-unreadable-decision-holds-the-fleet-where-it-is.yaml @@ -11,3 +11,4 @@ "apiVersion": "some.other.api/v1" "expected": "replicas": 8 + "holdReason": "DecisionUnavailable" From 3b21b63ea8febaa00fe36fe17295d34d8ffe085b Mon Sep 17 00:00:00 2001 From: huskcass <333684917+huskcass@users.noreply.github.com> Date: Sun, 25 Oct 2026 22:04:49 -0700 Subject: [PATCH 2/6] autoscaling: replace ScalingDeferred with an AbleToScale condition AbleToScale follows the HPA convention: True is the normal state. It is True with Synced when no guard holds, with RolloutInProgress or ScaleDownSettling while a guard holds the write, and with DeferralTimedOut when the timeout forces it. It is False with InvalidSpec, TargetNotFound, TargetUnreadable or TargetWriteFailed when the sync could not scale the target, and Ready copies that reason. The ScalingDeferred condition and the NotDeferred reason are removed. deferredSince, deferTimeoutSeconds, the deferral clock and the event reasons are unchanged. The Deferred printer column becomes AbleToScale and Scaling. Co-authored-by: aceforeverd <11715200+aceforeverd@users.noreply.github.com> --- api/autoscaling/v1alpha1/llmscaler_types.go | 61 +++++---- ...utoscaling.modelsphere.dev_llmscalers.yaml | 11 +- ...lmscalers.autoscaling.modelsphere.dev.yaml | 11 +- dist/install.yaml | 11 +- docs/llmscaler.md | 48 ++++--- internal/controller/autoscaling/guards.go | 10 +- .../autoscaling/llmscaler_controller.go | 128 ++++++++++-------- .../autoscaling/llmscaler_scenario_test.go | 2 +- .../autoscaling/llmscaler_status_test.go | 125 +++++++++++++++-- ...rral-on-a-no-op-once-no-guard-matches.yaml | 4 +- ...s-while-a-rollout-guard-still-matches.yaml | 6 +- ...ded-the-timeout-on-a-fresh-reconciler.yaml | 2 +- ...ady-fleet-once-the-deferral-times-out.yaml | 2 +- ...-rollout-exceeds-the-deferral-timeout.yaml | 4 +- ...rced-write-pace-when-no-guard-matches.yaml | 4 +- ...scale-down-step-after-one-sync-period.yaml | 2 +- ...-step-until-a-sync-period-has-elapsed.yaml | 2 +- ...a-fallback-for-the-scaling-ratio-only.yaml | 4 +- ...n-targetref-apiversion-does-not-parse.yaml | 13 +- ...ilable-when-prometheus-is-unreachable.yaml | 4 +- ...scaletime-when-a-scale-write-succeeds.yaml | 4 +- ...dy-on-a-no-op-without-a-lastscaletime.yaml | 4 +- ...ound-and-leave-the-deferral-untouched.yaml | 15 +- ...while-a-rollout-defers-the-next-scale.yaml | 6 +- 24 files changed, 319 insertions(+), 164 deletions(-) diff --git a/api/autoscaling/v1alpha1/llmscaler_types.go b/api/autoscaling/v1alpha1/llmscaler_types.go index 88f5501..5a2dbe1 100644 --- a/api/autoscaling/v1alpha1/llmscaler_types.go +++ b/api/autoscaling/v1alpha1/llmscaler_types.go @@ -55,59 +55,71 @@ const ( // replicas instead. ConditionDecisionAvailable = "DecisionAvailable" - // ConditionScalingDeferred is true while a rollout or settle guard is holding - // a scale action back, including a no-op sync whose deferral clock is still - // running. It is false when this sync wrote (a normal write or one forced - // after the timeout) or when nothing is deferred. - ConditionScalingDeferred = "ScalingDeferred" + // ConditionAbleToScale reports whether this sync could scale the target. + // True is the normal state. It stays True while a rollout or settle guard + // holds the write, and when the deferral timeout forces that write. The + // reason then names the guard (RolloutInProgress, ScaleDownSettling) or + // DeferralTimedOut, or Synced when no guard holds. False means this sync + // could not scale: targetRef.apiVersion does not parse (InvalidSpec), the + // target could not be read (TargetNotFound, TargetUnreadable), or the + // replica write failed (TargetWriteFailed). + ConditionAbleToScale = "AbleToScale" // ConditionReady is the summary of the last sync, and the condition to // watch. It is True when the sync completed cleanly: the controller wrote // spec.replicas, the replica count was already at the step target, or a // guard is holding the write. It is False when the sync could not do its - // job. ScalingDeferred False with NotDeferred is healthy, not a failure; - // the detail of a hold stays on ScalingDeferred. + // job. When AbleToScale is False, Ready is False with AbleToScale's + // reason. A Custom scaler with no customProvider, a missing decision, and + // an unevaluable metric are False on Ready while AbleToScale still + // describes the scale decision. A guard hold is not a failure: read Ready + // for overall health and AbleToScale for why a write is or is not happening. ConditionReady = "Ready" ReasonDecisionReceived = "DecisionReceived" ReasonDecisionUnavailable = "DecisionUnavailable" // ReasonRolloutInProgress means a Deployment rollout is in progress, so the - // scale action is held in both directions. + // scale action is held in both directions. AbleToScale is True with this + // reason: the target can be scaled, and the guard is holding the write. ReasonRolloutInProgress = "RolloutInProgress" // ReasonScaleDownSettling means a scale-down is waiting for the target to - // finish absorbing the previous replica count. + // finish absorbing the previous replica count. AbleToScale is True with + // this reason. ReasonScaleDownSettling = "ScaleDownSettling" // ReasonDeferralTimedOut means the scale action was held longer than // spec.deferTimeoutSeconds and was applied anyway. The value matches the - // DeferralTimedOut event. The deferral clock keeps running when the guard - // still matches. + // DeferralTimedOut event. AbleToScale is True with this reason. The + // deferral clock keeps running when the guard still matches. ReasonDeferralTimedOut = "DeferralTimedOut" - // ReasonNotDeferred means this sync found no guard matching: it wrote, or - // it was a no-op that did not have a deferral in progress. A no-op does not - // start a deferral, and a no-op while a guard still matches is not this - // reason when deferredSince is already set. - ReasonNotDeferred = "NotDeferred" // ReasonSynced means the last sync completed cleanly. It is the reason on // ConditionReady when the controller wrote, had nothing to write, or a // guard is holding the write. The message says which, and a deferral - // points at ScalingDeferred for the detail. + // points at AbleToScale for the detail. It is also the AbleToScale reason + // when no guard holds: a normal write, or a no-op with no deferral + // running. deferredSince is cleared in that case. ReasonSynced = "Synced" // ReasonMetricsUnavailable means the Prometheus provider could not evaluate - // any metric this sync, so the replica count was held. + // any metric this sync, so the replica count was held. Ready is False with + // this reason. AbleToScale still describes the scale decision. ReasonMetricsUnavailable = "MetricsUnavailable" - // ReasonInvalidSpec means the scaler cannot be reconciled as written: - // targetRef.apiVersion does not parse, or metricProvider is Custom and - // customProvider is not set. + // ReasonInvalidSpec means the scaler cannot be reconciled as written. + // On AbleToScale it means targetRef.apiVersion does not parse. On Ready it + // is that, or metricProvider is Custom and customProvider is not set (in + // which case AbleToScale still describes the scale decision). ReasonInvalidSpec = "InvalidSpec" // ReasonTargetNotFound means getting the scale target returned NotFound. + // AbleToScale and Ready are both False. deferredSince and the replica + // counts are left as they are. ReasonTargetNotFound = "TargetNotFound" // ReasonTargetUnreadable means getting the scale target failed for a - // reason other than NotFound. + // reason other than NotFound. AbleToScale and Ready are both False. + // deferredSince and the replica counts are left as they are. ReasonTargetUnreadable = "TargetUnreadable" // ReasonTargetWriteFailed means setting or updating spec.replicas on the - // scale target failed. + // scale target failed. AbleToScale and Ready are both False. deferredSince + // and the replica counts are left as they are. ReasonTargetWriteFailed = "TargetWriteFailed" ) @@ -378,7 +390,8 @@ type LLMScalerStatus struct { // +kubebuilder:printcolumn:name="CurrentReplicas",type="integer",JSONPath=".status.currentReplicas",description="The current number of replicas" // +kubebuilder:printcolumn:name="DesiredReplicas",type="integer",JSONPath=".status.desiredReplicas",description="The desired number of replicas" // +kubebuilder:printcolumn:name="Ready",type="string",JSONPath=".status.conditions[?(@.type==\"Ready\")].status",description="Whether the last sync completed cleanly" -// +kubebuilder:printcolumn:name="Deferred",type="string",JSONPath=".status.conditions[?(@.type==\"ScalingDeferred\")].status",description="Whether a scale action is currently deferred" +// +kubebuilder:printcolumn:name="AbleToScale",type="string",JSONPath=".status.conditions[?(@.type==\"AbleToScale\")].status",description="Whether this sync could scale the target" +// +kubebuilder:printcolumn:name="Scaling",type="string",JSONPath=".status.conditions[?(@.type==\"AbleToScale\")].reason",description="Why the scaler is or is not scaling: Synced, a hold reason, DeferralTimedOut, or a failure" // LLMScaler is the Schema for the llmscalers API type LLMScaler struct { diff --git a/config/crd/bases/autoscaling.modelsphere.dev_llmscalers.yaml b/config/crd/bases/autoscaling.modelsphere.dev_llmscalers.yaml index 8b5fb29..53e520d 100644 --- a/config/crd/bases/autoscaling.modelsphere.dev_llmscalers.yaml +++ b/config/crd/bases/autoscaling.modelsphere.dev_llmscalers.yaml @@ -35,9 +35,14 @@ spec: jsonPath: .status.conditions[?(@.type=="Ready")].status name: Ready type: string - - description: Whether a scale action is currently deferred - jsonPath: .status.conditions[?(@.type=="ScalingDeferred")].status - name: Deferred + - description: Whether this sync could scale the target + jsonPath: .status.conditions[?(@.type=="AbleToScale")].status + name: AbleToScale + type: string + - description: 'Why the scaler is or is not scaling: Synced, a hold reason, DeferralTimedOut, + or a failure' + jsonPath: .status.conditions[?(@.type=="AbleToScale")].reason + name: Scaling type: string name: v1alpha1 schema: diff --git a/dist/chart/templates/crd/llmscalers.autoscaling.modelsphere.dev.yaml b/dist/chart/templates/crd/llmscalers.autoscaling.modelsphere.dev.yaml index 90844a0..106af49 100644 --- a/dist/chart/templates/crd/llmscalers.autoscaling.modelsphere.dev.yaml +++ b/dist/chart/templates/crd/llmscalers.autoscaling.modelsphere.dev.yaml @@ -38,9 +38,14 @@ spec: jsonPath: .status.conditions[?(@.type=="Ready")].status name: Ready type: string - - description: Whether a scale action is currently deferred - jsonPath: .status.conditions[?(@.type=="ScalingDeferred")].status - name: Deferred + - description: Whether this sync could scale the target + jsonPath: .status.conditions[?(@.type=="AbleToScale")].status + name: AbleToScale + type: string + - description: 'Why the scaler is or is not scaling: Synced, a hold reason, DeferralTimedOut, + or a failure' + jsonPath: .status.conditions[?(@.type=="AbleToScale")].reason + name: Scaling type: string name: v1alpha1 schema: diff --git a/dist/install.yaml b/dist/install.yaml index d1014dd..afd1b69 100644 --- a/dist/install.yaml +++ b/dist/install.yaml @@ -43,9 +43,14 @@ spec: jsonPath: .status.conditions[?(@.type=="Ready")].status name: Ready type: string - - description: Whether a scale action is currently deferred - jsonPath: .status.conditions[?(@.type=="ScalingDeferred")].status - name: Deferred + - description: Whether this sync could scale the target + jsonPath: .status.conditions[?(@.type=="AbleToScale")].status + name: AbleToScale + type: string + - description: 'Why the scaler is or is not scaling: Synced, a hold reason, DeferralTimedOut, + or a failure' + jsonPath: .status.conditions[?(@.type=="AbleToScale")].reason + name: Scaling type: string name: v1alpha1 schema: diff --git a/docs/llmscaler.md b/docs/llmscaler.md index 1c6ea01..d9a1b42 100644 --- a/docs/llmscaler.md +++ b/docs/llmscaler.md @@ -82,43 +82,51 @@ Only read when `metricProvider: Custom`. | `deferredSince` | metav1.Time | When the current deferral started. Set on the first sync that holds a scale action back. Kept across guard reason changes, no-op syncs while a guard still matches, and forced writes. Cleared only when a sync finds no guard matching (a normal write, or a no-op with no guard). A no-op does not start it. Stored in status so it survives an operator restart. | | `observedGeneration` | int64 | `metadata.generation` at the last status write. | | `lastScaleTime` | metav1.Time | When the controller last successfully wrote `spec.replicas` on the target. Set on a normal write and on a write forced after the deferral timeout. Left unchanged on a no-op, a deferral, pacing, or an error. | -| `conditions` | []metav1.Condition | `Ready`, `DecisionAvailable`, and `ScalingDeferred`. See [Conditions](#conditions). | +| `conditions` | []metav1.Condition | `Ready`, `DecisionAvailable`, and `AbleToScale`. See [Conditions](#conditions). | -`kubectl get llmscalers` prints `MinReplicas`, `MaxReplicas`, `CurrentReplicas`, `DesiredReplicas`, `Ready` (the `Ready` condition status), `Deferred` (the `ScalingDeferred` condition status). +`kubectl get llmscalers` prints `MinReplicas`, `MaxReplicas`, `CurrentReplicas`, `DesiredReplicas`, `Ready` (the `Ready` condition status), `AbleToScale` (the `AbleToScale` condition status), and `Scaling` (the `AbleToScale` condition reason). ### Conditions -Read `Ready` for overall health. `DecisionAvailable` and `ScalingDeferred` carry detail. Polarity differs: `ScalingDeferred=True` is the held case, and `False` (`NotDeferred`) is the normal state, not a failure. Every condition carries the `metadata.generation` it was computed from as its `observedGeneration`. A sync that stops early updates `Ready` only and leaves the other two as they were. +Read `Ready` for overall health. `AbleToScale=True` is the normal state, including while a guard holds the write. `DecisionAvailable` and `AbleToScale` carry detail. Every condition carries the `metadata.generation` it was computed from as its `observedGeneration`. `AbleToScale` replaces the earlier `ScalingDeferred` condition (polarity inverted; `NotDeferred` became `Synced`). | Type | Status | Reason | Set when | | --- | --- | --- | --- | -| `Ready` | `True` | `Synced` | The sync reached a scale decision and none of the failures below applies. That covers a write (`scaled 3 -> 5`), a no-op (`replica count is at the step target`), and a guard holding the write, paced forced writes included (`scale action deferred: ; see ScalingDeferred`). | -| `Ready` | `False` | `InvalidSpec` | `targetRef.apiVersion` does not parse: the sync stops, and only `Ready` and `observedGeneration` are written. Also set when `metricProvider` is `Custom` with no `customProvider` (the replica count is held). | -| `Ready` | `False` | `TargetNotFound` | Getting the target returned NotFound. The sync stops. Only `Ready` and `observedGeneration` are written, and the sync is retried after `retryPeriodSeconds`. | -| `Ready` | `False` | `TargetUnreadable` | Getting the target failed for another reason. Handled like `TargetNotFound`. | -| `Ready` | `False` | `TargetWriteFailed` | Writing `spec.replicas` on the target failed. Only `Ready` and `observedGeneration` are written (best effort). The error is returned, so the sync is retried with backoff. | -| `Ready` | `False` | `DecisionUnavailable` | Custom provider, no decision this sync. Same message as `DecisionAvailable`. The replica count is held. | -| `Ready` | `False` | `MetricsUnavailable` | Prometheus provider, and no metric could be evaluated (query error, no data, NaN/Inf, or an invalid target). The replica count is held. The message includes the last error. | +| `Ready` | `True` | `Synced` | `AbleToScale` is not `False` and none of the failures below applies. A write (`scaled 3 -> 5`), a no-op (`replica count is at the step target`), a guard holding the write, paced forced writes included (`scale action deferred: ; see AbleToScale`), and a write forced after the timeout (still `scaled X -> Y`). | +| `Ready` | `False` | `InvalidSpec` | `AbleToScale` is `False` because `targetRef.apiVersion` does not parse: same message, the sync stops. Also set, with `AbleToScale` still describing the scale decision, when `metricProvider` is `Custom` and `customProvider` is not set (`spec.customProvider is not set`); the replica count is held. | +| `Ready` | `False` | `TargetNotFound` | `AbleToScale` is `False` for the same reason. Getting the target returned NotFound. `deferredSince` and the replica counts are left as they are. Retried after `retryPeriodSeconds`. | +| `Ready` | `False` | `TargetUnreadable` | `AbleToScale` is `False` for the same reason. Getting the target failed for another reason. Handled like `TargetNotFound`. | +| `Ready` | `False` | `TargetWriteFailed` | `AbleToScale` is `False` for the same reason. Writing `spec.replicas` failed (`could not write spec.replicas: ...`). `deferredSince` and the replica counts are left as they are. The error is returned, so the sync is retried with backoff. A missing `customProvider` does not outrank this. | +| `Ready` | `False` | `DecisionUnavailable` | Custom provider, and `DecisionAvailable` is `False` this sync. Same message as `DecisionAvailable`. The replica count is held. `AbleToScale` still describes the scale decision. | +| `Ready` | `False` | `MetricsUnavailable` | Prometheus provider, and no metric could be evaluated (query error, no data, NaN/Inf, or an invalid target). The replica count is held. The message includes the last error. `AbleToScale` still describes the scale decision. | | `DecisionAvailable` | `True` | `DecisionReceived` | Custom provider: a decision was read this sync. Message: `decision server recommends N replicas`. | | `DecisionAvailable` | `False` | `DecisionUnavailable` | Custom provider: the request failed or the reply could not be used (see [Custom metric provider](#custom-metric-provider)). The error is the message. | -| `ScalingDeferred` | `True` | `RolloutInProgress` | A `Deployment` target is mid-rollout: `observedGeneration < generation`, `updatedReplicas < spec.replicas`, or `replicas > updatedReplicas`. Holds scale-up, scale-down, and a no-op while a deferral is running. Checked first. | -| `ScalingDeferred` | `True` | `ScaleDownSettling` | A scale-down while `readyReplicas != spec.replicas`, or, when the terminating check is on (off by default), `terminatingReplicas > 0`. | -| `ScalingDeferred` | `False` | `DeferralTimedOut` | A guard still matched and `deferTimeoutSeconds` has elapsed since `deferredSince`, so the held write was applied anyway. `deferredSince` is kept. | -| `ScalingDeferred` | `False` | `NotDeferred` | No guard matched and the sync wrote (`scaled X -> Y`), or the sync was a no-op with no deferral running. `deferredSince` is cleared. | - -- `Ready` precedence when several apply: `InvalidSpec`, then `TargetNotFound`/`TargetUnreadable`, then `TargetWriteFailed`, then `DecisionUnavailable`/`MetricsUnavailable`, then `Synced`. A held replica count from a missing decision or metric outranks a clean write or deferral. +| `AbleToScale` | `True` | `Synced` | No guard holds. A normal write (`scaled X -> Y`), or a no-op with no deferral running (`replica count is already at the step target`). `deferredSince` is cleared. | +| `AbleToScale` | `True` | `RolloutInProgress` | A guard holds the write and this one matched first. A `Deployment` is mid-rollout: `observedGeneration < generation`, `updatedReplicas < spec.replicas`, or `replicas > updatedReplicas`. Covers the deferring case, a no-op while a deferral is already running, and forced-write pacing. | +| `AbleToScale` | `True` | `ScaleDownSettling` | A guard holds the write and the rollout guard did not. A scale-down while `readyReplicas != spec.replicas`, or, when the terminating check is on (off by default), `terminatingReplicas > 0`. Same three hold cases as `RolloutInProgress`. | +| `AbleToScale` | `True` | `DeferralTimedOut` | A guard still matched and `deferTimeoutSeconds` has elapsed since `deferredSince`, so the held write was applied. Same message as the Warning event. `deferredSince` is kept. | +| `AbleToScale` | `False` | `InvalidSpec` | `targetRef.apiVersion` does not parse. The sync stops. Message: `targetRef.apiVersion "" does not parse: ...`. | +| `AbleToScale` | `False` | `TargetNotFound` | Getting the target returned NotFound. `deferredSince` and the replica counts stay untouched. | +| `AbleToScale` | `False` | `TargetUnreadable` | Getting the target failed for another reason. `deferredSince` and the replica counts stay untouched. | +| `AbleToScale` | `False` | `TargetWriteFailed` | Updating `spec.replicas` on the target failed. `deferredSince` and the replica counts stay untouched. | + +- `Ready` precedence when several apply: if `AbleToScale` is `False`, Ready is `False` with that reason (`InvalidSpec`, `TargetNotFound`, `TargetUnreadable`, or `TargetWriteFailed`). Otherwise `InvalidSpec` when `metricProvider` is `Custom` without `customProvider` (`AbleToScale` still describes the scale decision for that sync). Otherwise `DecisionUnavailable` when `DecisionAvailable` is `False` this sync, or `MetricsUnavailable` when the Prometheus provider has no evaluable metric. Otherwise `Synced`. A held replica count from a missing decision or metric outranks a clean write or deferral. A failed target write outranks a missing `customProvider`. - `DecisionAvailable` is set only under the Custom provider with `customProvider` present. It is never set under Prometheus, and it is not removed if the provider is switched away from Custom. -- `ScalingDeferred` is set on every sync that reaches a scale decision. Syncs that stop at the target read or fail the target write leave it unchanged. +- `AbleToScale` is set on every sync that reaches a scale decision, and on a sync that stops because `targetRef.apiVersion` does not parse, the target cannot be read, or the target update fails. An unparseable apiVersion or a failed read leaves `DecisionAvailable`, `deferredSince`, and the replica counts unchanged. A failed write leaves `deferredSince` and the replica counts unchanged. + +#### `AbleToScale` details -#### `ScalingDeferred` details +A no-op while a guard still matches leaves `deferredSince` unchanged. If it is set, the condition stays `True` with that guard's reason and a message that nothing needs writing this sync, the guard is still active, and the deferral clock keeps running. If it is nil, the condition is `True`/`Synced` (`replica count is already at the step target`): a no-op does not start a deferral. -A no-op while a guard still matches leaves `deferredSince` unchanged. If it is set, the condition stays `True` with that guard's reason and a message that nothing needs writing this sync, the guard is still active, and the deferral clock keeps running. If it is nil, the condition is `False`/`NotDeferred`: a no-op does not start a deferral. +While a guard is holding a write that has not yet reached `deferTimeoutSeconds`, the condition is `True` with that guard's reason. The message is `holding N replicas (wanted M): ; deferred since