From 552e43c786f4bfc122c7d69178ef32379ae16a6d Mon Sep 17 00:00:00 2001 From: Sam Batschelet Date: Wed, 7 Oct 2026 12:26:34 -0400 Subject: [PATCH 01/10] feat(operator)!: the InferenceProvider CRD drops maxRunning and status.state and names its ready-endpoints signal Capacity is no longer declared, so a provider's maxRunning and the derived status.state leave the CRD. signalNames.readyEndpoints names the EPP's ready-endpoint series when it differs from the defaults. Generated manifests and the CRD reference follow. Signed-off-by: Sam Batschelet --- charts/grid-operator/README.md | 1 + .../templates/crds/inferenceprovider.yaml | 59 +++++++++++++++++++ deploy/crds/inferenceprovider.yaml | 59 +++++++++++++++++++ docs/architecture/crds.md | 40 +++++++++++++ operator/src/crd/inference_provider.rs | 46 ++++++++++++++- 5 files changed, 204 insertions(+), 1 deletion(-) diff --git a/charts/grid-operator/README.md b/charts/grid-operator/README.md index 73357804f..c58f4e8e9 100644 --- a/charts/grid-operator/README.md +++ b/charts/grid-operator/README.md @@ -156,6 +156,7 @@ RELEASE=grid-operator; NAMESPACE=grid-system; for crd in agenttoolproviders grid |-----|------|---------|-------------| | `crds.enabled` | bool | `true` | Install and upgrade the Grid CRDs. `false` when a platform owns them. | | `crds.keep` | bool | `true` | Keep the CRDs on `helm uninstall` and an Argo CD delete or prune. | +| `grid.providers` | object | `{}` | InferenceProviders this site serves, keyed by name. `model` defaults to the name, `providerKind` to `vllm`, `backendKind` to `local_model`. | | `rbac.enrollmentNamespace` | string | `""` | The grid-enrollment namespace. The render fails if the operator would get Secret access there. | | `rbac.metricsScraper` | bool | `true` | Create the metrics scraper ServiceAccount, allowed only GET on the nonResourceURL /metrics, and let the operator mint short-lived tokens for it. An llm-d EPP serving bearer-authenticated metrics (the default) admits a scrape with that token (metricsConfig.auth type serviceAccountToken). The operator never sends its own token. | | `replicaCount` | int | `1` | Operator replicas. Must be 1 (schema-enforced). | diff --git a/charts/grid-operator/templates/crds/inferenceprovider.yaml b/charts/grid-operator/templates/crds/inferenceprovider.yaml index 89aa24417..db876026c 100644 --- a/charts/grid-operator/templates/crds/inferenceprovider.yaml +++ b/charts/grid-operator/templates/crds/inferenceprovider.yaml @@ -24,8 +24,19 @@ spec: - jsonPath: .spec.providerKind name: Provider type: string + - jsonPath: .status.conditions[?(@.type=="Ready")].status + name: Ready + type: string + - jsonPath: .metadata.creationTimestamp + name: Age + type: date + - jsonPath: .status.conditions[?(@.type=="Ready")].reason + name: Reason + priority: 1 + type: string - jsonPath: .status.phase name: Phase + priority: 1 type: string name: v1alpha1 schema: @@ -407,6 +418,14 @@ spec: description: Metric name for normalised queue depth (0.0–1.0). nullable: true type: string + readyEndpoints: + description: |- + Metric name counting the pool's ready endpoints, read for the `Ready` condition. + + Defaults to `llm_d_epp_ready_endpoints`, then `inference_pool_ready_pods`. + Filtered by `poolName` when set. Zero marks the provider not ready. + nullable: true + type: string type: object staleMetricsSeconds: description: |- @@ -872,6 +891,46 @@ spec: description: Observed status of an [`InferenceProvider`]. nullable: true properties: + conditions: + description: |- + Observed conditions. `Ready` says whether the provider can currently serve a request. + + Written by the operator's signals loop, never by provider reconciliation. + items: + description: One observed condition, shaped like `metav1.Condition`. + properties: + lastTransitionTime: + description: When `status` last changed, RFC 3339. + format: date-time + type: string + message: + default: "" + description: Human-readable detail. + type: string + observedGeneration: + description: The `metadata.generation` this was computed against. + format: int64 + nullable: true + type: integer + reason: + description: CamelCase reason for the status. + type: string + status: + description: '`True`, `False`, or `Unknown`.' + type: string + type: + description: Condition type, such as `Ready`. + type: string + required: + - lastTransitionTime + - reason + - status + - type + type: object + type: array + x-kubernetes-list-map-keys: + - type + x-kubernetes-list-type: map matchingSites: default: [] description: Sites matched by the site selector. diff --git a/deploy/crds/inferenceprovider.yaml b/deploy/crds/inferenceprovider.yaml index 510b850e2..d83310137 100644 --- a/deploy/crds/inferenceprovider.yaml +++ b/deploy/crds/inferenceprovider.yaml @@ -16,8 +16,19 @@ spec: - jsonPath: .spec.providerKind name: Provider type: string + - jsonPath: .status.conditions[?(@.type=="Ready")].status + name: Ready + type: string + - jsonPath: .metadata.creationTimestamp + name: Age + type: date + - jsonPath: .status.conditions[?(@.type=="Ready")].reason + name: Reason + priority: 1 + type: string - jsonPath: .status.phase name: Phase + priority: 1 type: string name: v1alpha1 schema: @@ -399,6 +410,14 @@ spec: description: Metric name for normalised queue depth (0.0–1.0). nullable: true type: string + readyEndpoints: + description: |- + Metric name counting the pool's ready endpoints, read for the `Ready` condition. + + Defaults to `llm_d_epp_ready_endpoints`, then `inference_pool_ready_pods`. + Filtered by `poolName` when set. Zero marks the provider not ready. + nullable: true + type: string type: object staleMetricsSeconds: description: |- @@ -864,6 +883,46 @@ spec: description: Observed status of an [`InferenceProvider`]. nullable: true properties: + conditions: + description: |- + Observed conditions. `Ready` says whether the provider can currently serve a request. + + Written by the operator's signals loop, never by provider reconciliation. + items: + description: One observed condition, shaped like `metav1.Condition`. + properties: + lastTransitionTime: + description: When `status` last changed, RFC 3339. + format: date-time + type: string + message: + default: "" + description: Human-readable detail. + type: string + observedGeneration: + description: The `metadata.generation` this was computed against. + format: int64 + nullable: true + type: integer + reason: + description: CamelCase reason for the status. + type: string + status: + description: '`True`, `False`, or `Unknown`.' + type: string + type: + description: Condition type, such as `Ready`. + type: string + required: + - lastTransitionTime + - reason + - status + - type + type: object + type: array + x-kubernetes-list-map-keys: + - type + x-kubernetes-list-type: map matchingSites: default: [] description: Sites matched by the site selector. diff --git a/docs/architecture/crds.md b/docs/architecture/crds.md index 9dc2c5e6a..590a60fcb 100644 --- a/docs/architecture/crds.md +++ b/docs/architecture/crds.md @@ -563,6 +563,45 @@ credential projection can become available. **Phases**: Pending → Available → Degraded → Unavailable +**Readiness**: `status.conditions` carries a `Ready` condition, written by the +operator's signals loop from each scrape of the provider's metrics. Readiness is +a condition, not a phase: `phase` follows the provider's configuration only, and +says nothing about whether it serves now. + +| Status | Reason | When | +|---|---|---| +| `True` | `Ready` | The latest scrape succeeded with at least one ready endpoint. | +| `False` | `NoEndpointsReady` | Two consecutive scrapes counted zero ready endpoints, and the EPP recorded no engine answer in the last 30s. | +| `Unknown` | `NoLivenessCheck` | The scrape answered without the pool's ready-endpoint series (`llm_d_epp_ready_endpoints`, then `inference_pool_ready_pods`, for `poolName`). Readiness is unknown rather than false, so the provider is not excluded: a provider pointed at vLLM's own `/metrics` carries no such series. The message names what was missing. | +| `False` | `ScrapeTimedOut` | No scrape succeeded within `staleMetricsSeconds`, and the latest timed out. | +| `False` | `ScrapeUnauthorized` | As above, and the latest was refused with 401 or 403. | +| `False` | `TLSHandshakeFailed` | As above, and the latest failed TLS, including the TLS material. | +| `False` | `ScrapeFailed` | As above, and the latest failed otherwise. The message names the class: `dns`, `connect`, `http`, `body_cap`, `parse`, or `config`. | +| `False` | `MetricsStale` | No scrape succeeded within `staleMetricsSeconds`, and none failed. | +| `False` | `ProviderUnavailable` | The provider is `Unavailable`. | +| `Unknown` | `AwaitingFirstScrape` | No scrape has succeeded yet, within the grace window. | +| `Unknown` | `MetricsNotConfigured` | No `metricsConfig`, so readiness cannot be read. | + +The operator logs each change of reason once: at WARN when the provider turns not +ready, at INFO when it returns to `Ready` or waits. Each scrape counts in +`grid_provider_scrape_total{grid_provider,result}`, where `result` is `success`, +`no_series`, or a failure class, and +`grid_provider_last_scrape_success_timestamp_seconds{grid_provider}` holds the time +of the last scrape with the ready-endpoint series. + +A provider whose `Ready` is `False` is excluded from routing: its site publishes +`grid_provider_ready 0`, and its serving config entry carries `admission: none`. +See [Polling Cross-Site Load Signals](polling-metrics.md#provider-readiness). + +The READY column reads the condition's status directly, so no status field +repeats it. `-o wide` adds REASON, the condition's reason, and PHASE. + +```text +NAME PROVIDER READY AGE +qwen3-site-a self_hosted True 3d +qwen3-site-b self_hosted False 3d +``` + `spec.capacityWeight` is an optional positive relative provider capacity from `1` through `1000`, used only with `GridNetwork.spec.selectionPolicy.mode: weightedRandom` and `placementPolicy.strategy: static`. If omitted, the @@ -651,6 +690,7 @@ routing architecture for full semantics. | `prefixCacheHitRatio` | Prefix-cache hit ratio from `0.0` to `1.0`. | | `errorRate` | Error rate from `0.0` to `1.0`. | | `healthy` | Health gauge interpreted by the metrics parser. | +| `readyEndpoints` | Ready endpoints in the pool, read for the `Ready` condition. Defaults to `llm_d_epp_ready_endpoints`, then `inference_pool_ready_pods`, filtered by `poolName`. | #### TLS and mTLS diff --git a/operator/src/crd/inference_provider.rs b/operator/src/crd/inference_provider.rs index 682ac49ae..357205593 100644 --- a/operator/src/crd/inference_provider.rs +++ b/operator/src/crd/inference_provider.rs @@ -5,6 +5,11 @@ //! cloud-managed services (Bedrock, Vertex), and third-party //! APIs (OpenAI, Anthropic). +#![expect( + clippy::large_stack_frames, + reason = "the derived schema builder runs once at startup" +)] + use kube::CustomResource; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; @@ -29,7 +34,10 @@ use super::{ status = "InferenceProviderStatus", namespaced = false, printcolumn = r#"{"name":"Provider","type":"string","jsonPath":".spec.providerKind"}"#, - printcolumn = r#"{"name":"Phase","type":"string","jsonPath":".status.phase"}"# + printcolumn = r#"{"name":"Ready","type":"string","jsonPath":".status.conditions[?(@.type==\"Ready\")].status"}"#, + printcolumn = r#"{"name":"Age","type":"date","jsonPath":".metadata.creationTimestamp"}"#, + printcolumn = r#"{"name":"Reason","type":"string","jsonPath":".status.conditions[?(@.type==\"Ready\")].reason","priority":1}"#, + printcolumn = r#"{"name":"Phase","type":"string","jsonPath":".status.phase","priority":1}"# )] #[serde(rename_all = "camelCase")] pub struct InferenceProviderSpec { @@ -461,6 +469,13 @@ pub struct MetricSignalNames { /// Metric name for a health gauge (any positive value = healthy). #[serde(skip_serializing_if = "Option::is_none")] pub healthy: Option, + + /// Metric name counting the pool's ready endpoints, read for the `Ready` condition. + /// + /// Defaults to `llm_d_epp_ready_endpoints`, then `inference_pool_ready_pods`. + /// Filtered by `poolName` when set. Zero marks the provider not ready. + #[serde(skip_serializing_if = "Option::is_none")] + pub ready_endpoints: Option, } /// Returns the default metrics scrape path. @@ -610,6 +625,13 @@ fn default_models_path() -> String { #[derive(Clone, Debug, Default, Deserialize, Eq, JsonSchema, PartialEq, Serialize)] #[serde(rename_all = "camelCase")] pub struct InferenceProviderStatus { + /// Observed conditions. `Ready` says whether the provider can currently serve a request. + /// + /// Written by the operator's signals loop, never by provider reconciliation. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + #[schemars(extend("x-kubernetes-list-type" = "map", "x-kubernetes-list-map-keys" = ["type"]))] + pub conditions: Vec, + /// Sites matched by the site selector. #[serde(default)] pub matching_sites: Vec, @@ -659,6 +681,28 @@ impl InferenceProviderStatus { } } +/// One observed condition, shaped like `metav1.Condition`. +#[derive(Clone, Debug, Default, Deserialize, Eq, JsonSchema, PartialEq, Serialize)] +#[serde(rename_all = "camelCase")] +pub struct Condition { + /// Condition type, such as `Ready`. + #[serde(rename = "type")] + pub type_: String, + /// `True`, `False`, or `Unknown`. + pub status: String, + /// CamelCase reason for the status. + pub reason: String, + /// Human-readable detail. + #[serde(default)] + pub message: String, + /// When `status` last changed, RFC 3339. + #[schemars(extend("format" = "date-time"))] + pub last_transition_time: String, + /// The `metadata.generation` this was computed against. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub observed_generation: Option, +} + /// Lifecycle phase of a provider resource. #[derive(Clone, Debug, Default, Deserialize, Eq, JsonSchema, PartialEq, Serialize)] pub enum ProviderPhase { From 53aaad1a485dc675bbc2b8ef3d7f84b9233561f3 Mon Sep 17 00:00:00 2001 From: Sam Batschelet Date: Wed, 7 Oct 2026 12:26:34 -0400 Subject: [PATCH 02/10] feat(operator)!: resolve provider readiness, say why, and count every scrape A provider is not ready when its EPP reports zero ready endpoints for two scrapes with no engine answer in the last 30 s, when a scrape lacks the pool's ready-endpoint series, when no scrape has succeeded within staleMetricsSeconds, or when it is Unavailable. The verdict is the Ready condition with the cause as its reason. Only a response counts as the engine answering, a busy pool that still answers stays ready, and a provider that answers again returns to routing. Every scrape is counted by outcome. A lost write race is not a reconciliation failure. Signed-off-by: Sam Batschelet --- mock-providers/src/openai.rs | 11 +- operator/src/controller/inference_provider.rs | 19 + operator/src/error.rs | 34 + operator/src/lib.rs | 3 + operator/src/main.rs | 16 +- operator/src/metrics_scraper.rs | 124 +- operator/src/readiness.rs | 1119 +++++++++++++++++ 7 files changed, 1320 insertions(+), 6 deletions(-) create mode 100644 operator/src/readiness.rs diff --git a/mock-providers/src/openai.rs b/mock-providers/src/openai.rs index 04886a6f6..57bdb354e 100644 --- a/mock-providers/src/openai.rs +++ b/mock-providers/src/openai.rs @@ -159,7 +159,11 @@ async fn metrics(axum::extract::State(state): axum::extract::State) -> Response::builder() .status(StatusCode::OK) .header(http::header::CONTENT_TYPE, "text/plain; version=0.0.4") - .body(Body::from(format!("grid_demo_queue_depth {}\n", state.queue_depth))) + // A ready count, as an EPP exposes, so a grid operator reads the mock as ready. + .body(Body::from(format!( + "grid_demo_queue_depth {}\ninference_pool_ready_pods 1\n", + state.queue_depth + ))) .unwrap_or_default() } @@ -448,7 +452,10 @@ mod tests { let resp = send(req).await; assert_eq!(resp.status(), StatusCode::OK); let body = to_bytes(resp.into_body(), 4096).await.unwrap_or_default(); - assert_eq!(body.as_ref(), b"grid_demo_queue_depth 0.25\n"); + assert_eq!( + body.as_ref(), + b"grid_demo_queue_depth 0.25\ninference_pool_ready_pods 1\n" + ); } #[tokio::test] diff --git a/operator/src/controller/inference_provider.rs b/operator/src/controller/inference_provider.rs index 8ab180438..f03b77cb8 100644 --- a/operator/src/controller/inference_provider.rs +++ b/operator/src/controller/inference_provider.rs @@ -691,6 +691,7 @@ async fn update_status( ModelDiscoveryConfig::OpenAiModels(openai) => openai.effective_url(&provider.spec.endpoint), }); let status = InferenceProviderStatus { + conditions: Vec::new(), matching_sites, model_discovery_error: None, model_discovery_url, @@ -733,6 +734,7 @@ mod tests { #[test] fn reconciler_status_matches_only_reconciler_fields() { let baseline = InferenceProviderStatus { + conditions: Vec::new(), matching_sites: vec!["site-a".to_owned()], model_discovery_error: None, model_discovery_url: None, @@ -761,6 +763,23 @@ mod tests { assert!(!baseline.matches_reconciler_status(&changed_discovery_url)); } + #[test] + fn a_conditions_change_is_not_the_reconcilers_to_write() { + let baseline = InferenceProviderStatus::default(); + let changed_conditions = InferenceProviderStatus { + conditions: vec![crate::crd::inference_provider::Condition { + type_: "Ready".to_owned(), + status: "False".to_owned(), + ..Default::default() + }], + ..baseline.clone() + }; + assert!( + baseline.matches_reconciler_status(&changed_conditions), + "the signals loop owns conditions" + ); + } + // ----------------------------------------------------------------------- // Test utilities // ----------------------------------------------------------------------- diff --git a/operator/src/error.rs b/operator/src/error.rs index fe7b51577..4c3ebb899 100644 --- a/operator/src/error.rs +++ b/operator/src/error.rs @@ -39,3 +39,37 @@ pub enum OperatorError { #[error("invalid resource: {0}")] InvalidResource(String), } + +impl OperatorError { + /// Whether this is the apiserver refusing a write because the object moved underneath it. + /// + /// Two writers on one object is the normal case, not a fault: the next reconcile reads the + /// new version and reapplies. Logging it at error made a steady state look like a failure. + #[must_use] + pub fn is_conflict(&self) -> bool { + matches!(self, Self::Kube(kube::Error::Api(status)) if status.code == 409) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + /// An apiserver status with `code`, as `enroll::tests` scripts one. + fn api(code: u16) -> OperatorError { + OperatorError::Kube(kube::Error::Api(Box::new( + kube::core::Status::failure("scripted", "r").with_code(code), + ))) + } + + #[test] + fn only_a_409_is_a_lost_write_race() { + assert!(api(409).is_conflict(), "409 is the apiserver refusing a stale write"); + assert!(!api(404).is_conflict(), "a missing object is not a race"); + assert!(!api(500).is_conflict(), "a server error is not a race"); + assert!( + !OperatorError::NotFound("grid".to_owned()).is_conflict(), + "a non-kube error is not a race" + ); + } +} diff --git a/operator/src/lib.rs b/operator/src/lib.rs index b38bf14af..cfadae0e6 100644 --- a/operator/src/lib.rs +++ b/operator/src/lib.rs @@ -31,6 +31,8 @@ pub mod crd; pub mod enroll; /// Operator error types. pub mod error; +/// A provider's recent request latency and error ratio, from deltas of its EPP's histograms. +pub mod latency; /// Prometheus metrics for gateway probe and phase-transition observability. pub mod metrics; /// Pure Prometheus text-format parser for inference backend metrics. @@ -41,6 +43,7 @@ pub mod metrics_scraper; pub mod metrics_tls; /// Short-lived tokens for the metrics scraper ServiceAccount. pub(crate) mod metrics_token; +pub mod readiness; /// Kubernetes resource builders. pub mod resources; pub mod served_models; diff --git a/operator/src/main.rs b/operator/src/main.rs index 4ea1eb0bf..2a3098331 100644 --- a/operator/src/main.rs +++ b/operator/src/main.rs @@ -76,6 +76,7 @@ use operator::{ grid_site::GridSite, inference_provider::InferenceProvider, }, + error::OperatorError, gateway, resources::tls_backend::{self, ServerTlsConfig}, served_models, swim_advertise, @@ -179,8 +180,11 @@ async fn main() { .with_declared_trust(declared_trust) .with_rotation(rotation_running) .with_site_name(config.swim.site_name.clone()) + .with_scrape_interval(config.signals.scrape_interval()) .hold_membership(), ); + // This site's providers first, then what its peers publish, on /metrics. + operator::metrics::register_provider_signals(vec![ctx.signals(), ctx.peers()]); // Controllers run now. Only SWIM and what dials peers wait on the advertise address. let (swim_tx, swim_rx) = tokio::sync::watch::channel(SwimStage::Starting); @@ -1087,14 +1091,24 @@ async fn run_network_controller( } /// Log a controller error, an expired watch at debug since kube-runtime relists. -fn log_controller_error(kind: &str, error: &kube::runtime::controller::Error) { +fn log_controller_error(kind: &str, error: &kube::runtime::controller::Error) { if watch_expired(error) { tracing::debug!(kind, ?error, "watch expired; relisting"); + } else if reconcile_conflicted(error) { + tracing::debug!(kind, ?error, "object moved underneath the write; reapplying"); } else { tracing::error!(error = ?error, "{kind} watch error"); } } +/// Whether a controller error is the reconciler losing a write race. +/// +/// The error policy already requeues these. Both sites have to agree, or a conflict the +/// reconciler treats as routine still reaches the log as a failure. +fn reconcile_conflicted(error: &kube::runtime::controller::Error) -> bool { + matches!(error, kube::runtime::controller::Error::ReconcilerFailed(e, _) if e.is_conflict()) +} + /// Whether a controller error is the apiserver expiring a watch's resource version. fn watch_expired(error: &kube::runtime::controller::Error) -> bool { let kube::runtime::controller::Error::QueueError(queue) = error else { diff --git a/operator/src/metrics_scraper.rs b/operator/src/metrics_scraper.rs index 8fb2e1488..2772e5066 100644 --- a/operator/src/metrics_scraper.rs +++ b/operator/src/metrics_scraper.rs @@ -93,6 +93,58 @@ pub enum MetricsScrapeError { /// A credential would have gone over plain HTTP without `allowPlaintext`. #[error("refusing to send a metrics credential over plain http: {0}")] PlaintextCredential(String), + /// The body passed the 1 MiB response limit. + #[error("metrics response body exceeds {0} byte limit")] + BodyTooLarge(usize), +} + +impl MetricsScrapeError { + /// The bounded class of this failure, for the provider's condition and scrape counters. + #[must_use] + pub(crate) fn class(&self) -> crate::readiness::ScrapeClass { + use crate::readiness::ScrapeClass; + match self { + Self::Timeout(_) => ScrapeClass::Timeout, + Self::NonOkStatus { status: 401 | 403, .. } => ScrapeClass::Unauthorized, + Self::NonOkStatus { .. } => ScrapeClass::Http, + Self::Transport(source) => transport_class(source.as_ref()), + Self::Encoding(_) => ScrapeClass::Parse, + Self::TlsMaterial(_) => ScrapeClass::Tls, + Self::BodyTooLarge(_) => ScrapeClass::BodyCap, + Self::InvalidUrl(_) | Self::HttpWithTls(_) | Self::Credential(_) | Self::PlaintextCredential(_) => { + ScrapeClass::Config + }, + } + } +} + +/// TLS when the chain holds a TLS error, DNS when resolution failed, timeout when the socket +/// timed out, else a connect failure. +fn transport_class(source: &(dyn std::error::Error + 'static)) -> crate::readiness::ScrapeClass { + use crate::readiness::ScrapeClass; + let mut cause = Some(source); + while let Some(error) = cause { + // io::Error's source skips the error it wraps, which is where a TLS failure sits. + let wrapped = error + .downcast_ref::() + .and_then(std::io::Error::get_ref) + .is_some_and(|inner| crate::resources::tls_backend::is_tls_error(inner)); + if wrapped || crate::resources::tls_backend::is_tls_error(error) { + return ScrapeClass::Tls; + } + let text = error.to_string(); + if text.contains("dns error") || text.contains("failed to lookup address") { + return ScrapeClass::Dns; + } + if error + .downcast_ref::() + .is_some_and(|io| io.kind() == std::io::ErrorKind::TimedOut) + { + return ScrapeClass::Timeout; + } + cause = error.source(); + } + ScrapeClass::Connect } // --------------------------------------------------------------------------- @@ -299,9 +351,7 @@ async fn read_response( .await .map_err(|e| { if e.downcast_ref::().is_some() { - MetricsScrapeError::Transport( - format!("metrics response body exceeds {MAX_RESPONSE_BODY_BYTES} byte limit").into(), - ) + MetricsScrapeError::BodyTooLarge(MAX_RESPONSE_BODY_BYTES) } else { MetricsScrapeError::Transport(e) } @@ -327,6 +377,74 @@ mod tests { use tokio::io::{AsyncReadExt as _, AsyncWriteExt as _}; use super::*; + + #[test] + #[cfg(not(feature = "fips"))] + fn a_tls_failure_wrapped_in_an_io_error_classes_as_tls() { + let tls: Box = Box::new(std::io::Error::new( + std::io::ErrorKind::InvalidData, + rustls::Error::General("bad certificate".to_owned()), + )); + assert_eq!( + MetricsScrapeError::Transport(tls).class(), + crate::readiness::ScrapeClass::Tls + ); + } + + #[test] + #[expect(clippy::too_many_lines, reason = "one table of failures and their classes")] + fn a_scrape_failure_has_a_bounded_class() { + use crate::readiness::ScrapeClass; + let status = |status| MetricsScrapeError::NonOkStatus { + status, + url: "https://epp:9090/metrics".to_owned(), + }; + let transport = |error: Box| MetricsScrapeError::Transport(error); + let cases = [ + ( + "timeout", + MetricsScrapeError::Timeout(Duration::from_secs(2)), + ScrapeClass::Timeout, + ), + ("401", status(401), ScrapeClass::Unauthorized), + ("403", status(403), ScrapeClass::Unauthorized), + ("500", status(500), ScrapeClass::Http), + ("404", status(404), ScrapeClass::Http), + ( + "dns", + transport("dns error: failed to lookup address information".into()), + ScrapeClass::Dns, + ), + ( + "refused", + transport(Box::new(std::io::Error::from(std::io::ErrorKind::ConnectionRefused))), + ScrapeClass::Connect, + ), + ( + "socket timeout", + transport(Box::new(std::io::Error::from(std::io::ErrorKind::TimedOut))), + ScrapeClass::Timeout, + ), + ( + "body cap", + MetricsScrapeError::BodyTooLarge(MAX_RESPONSE_BODY_BYTES), + ScrapeClass::BodyCap, + ), + ( + "tls material", + MetricsScrapeError::TlsMaterial("no key".to_owned()), + ScrapeClass::Tls, + ), + ( + "plaintext", + MetricsScrapeError::PlaintextCredential("http://epp".to_owned()), + ScrapeClass::Config, + ), + ]; + for (label, error, class) in cases { + assert_eq!(error.class(), class, "{label}"); + } + } use crate::resources::tls_backend::{MAX_CA_PEM_BYTES, MAX_CLIENT_CERT_PEM_BYTES, MAX_CLIENT_KEY_PEM_BYTES}; /// Start a local HTTP server on a random port and return the URL. diff --git a/operator/src/readiness.rs b/operator/src/readiness.rs new file mode 100644 index 000000000..b80199802 --- /dev/null +++ b/operator/src/readiness.rs @@ -0,0 +1,1119 @@ +//! Whether each provider can serve a request now, resolved from its scraped metrics. +//! +//! The signals loop records each scrape here. The verdict feeds three readers: the +//! provider's `Ready` condition, the `grid_provider_ready` series peers poll, and +//! admission in the serving config. + +use std::{ + collections::HashMap, + sync::{Mutex, PoisonError}, + time::{Duration, Instant}, +}; + +use crate::{crd::inference_provider::Condition, signals::Observation}; + +/// Metric names that count a pool's ready endpoints, preferred first. +pub(crate) const DEFAULT_READY_ENDPOINTS: [&str; 2] = ["llm_d_epp_ready_endpoints", "inference_pool_ready_pods"]; + +/// Consecutive scrapes reading zero ready endpoints before a provider is not ready, +/// and reading some before it is ready again. +/// +/// One reading can catch a pod between its metrics going stale and a fresh one, and a +/// pool that just drained can read ready once before its first request lands. +pub(crate) const STREAK: u32 = 2; + +/// How recently the engine must have answered for a zero count to read as busy, not down. +/// +/// The EPP counts endpoints whose metrics are fresh, so a saturated engine whose /metrics +/// answers late reads as zero while it still serves. +pub(crate) const PROGRESS_WINDOW: Duration = Duration::from_secs(30); + + +/// The series this site publishes per provider: 1 when ready, 0 when not. +pub const READY_SIGNAL: &str = "grid_provider_ready"; + +/// The resolved signal for requests a provider has running and queued. +pub const IN_FLIGHT_SIGNAL: &str = "grid_provider_in_flight_requests"; + +/// The endpoints behind the provider that answered with fresh metrics. +/// +/// Published so the in-flight count has a denominator at the reader: without it a +/// 16-endpoint site holding 40 requests sorts as more loaded than a 2-endpoint site +/// holding 10. +pub const READY_ENDPOINTS_SIGNAL: &str = "grid_provider_ready_endpoints"; + +/// The EPP's per-endpoint in-flight count, from its inflight-load-producer plugin. It +/// increments after flow control admits a request, once per scheduling profile's target, so +/// on a P/D pool a request counts on its prefill and its decode endpoint. +const EPP_IN_FLIGHT: &str = "llm_d_epp_inflight_requests"; + +/// Requests the EPP's flow control holds before scheduling, labeled by `inference_pool`. +const EPP_FLOW_CONTROL_QUEUE: &str = "llm_d_epp_flow_control_queue_size"; + +/// Pool averages that stand in when the EPP runs no inflight-load-producer. +const EPP_AVERAGES: [[&str; 2]; 2] = [ + [ + "llm_d_epp_average_running_requests", + "inference_pool_average_running_requests", + ], + ["llm_d_epp_average_queue_size", "inference_pool_average_queue_size"], +]; + +/// The `Ready` condition's status and reason for one provider. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum Reason { + /// Scraped recently with at least one ready endpoint. + Ready, + /// The latest scrapes counted zero ready endpoints. + NoEndpointsReady, + /// The scrape answered, but without the pool's ready-endpoint series, so + /// readiness is unknown rather than false: a documented vLLM provider + /// scrapes its own /metrics, which carries no such series. + NoLivenessCheck, + /// No successful scrape within the staleness window, and no attempt failed. + MetricsStale, + /// No successful scrape within the window, and the latest attempt timed out. + ScrapeTimedOut, + /// No successful scrape within the window, and the latest attempt was refused (401 or 403). + ScrapeUnauthorized, + /// No successful scrape within the window, and the latest attempt failed TLS. + TlsHandshakeFailed, + /// No successful scrape within the window, and the latest attempt failed otherwise. + ScrapeFailed, + /// The provider itself is `Unavailable`. + ProviderUnavailable, + /// The provider declares no metrics, so readiness is unknown. + MetricsNotConfigured, + /// Attempted, with no scrape succeeding yet inside the grace window. + AwaitingFirstScrape, +} + +impl Reason { + /// The condition reason as written to status. + #[must_use] + pub const fn as_str(self) -> &'static str { + match self { + Self::Ready => "Ready", + Self::NoEndpointsReady => "NoEndpointsReady", + Self::NoLivenessCheck => "NoLivenessCheck", + Self::MetricsStale => "MetricsStale", + Self::ScrapeTimedOut => "ScrapeTimedOut", + Self::ScrapeUnauthorized => "ScrapeUnauthorized", + Self::TlsHandshakeFailed => "TLSHandshakeFailed", + Self::ScrapeFailed => "ScrapeFailed", + Self::ProviderUnavailable => "ProviderUnavailable", + Self::MetricsNotConfigured => "MetricsNotConfigured", + Self::AwaitingFirstScrape => "AwaitingFirstScrape", + } + } + + /// The condition status: `True`, `False`, or `Unknown`. + #[must_use] + pub const fn status(self) -> &'static str { + match self { + Self::Ready => "True", + Self::MetricsNotConfigured | Self::AwaitingFirstScrape | Self::NoLivenessCheck => "Unknown", + Self::NoEndpointsReady + | Self::MetricsStale + | Self::ScrapeTimedOut + | Self::ScrapeUnauthorized + | Self::TlsHandshakeFailed + | Self::ScrapeFailed + | Self::ProviderUnavailable => "False", + } + } + + /// The one-word status shown in the STATUS column: `Ready`, `NotReady`, or `Unknown`. + #[must_use] + pub const fn display(self) -> &'static str { + match self { + Self::Ready => "Ready", + Self::MetricsNotConfigured | Self::AwaitingFirstScrape | Self::NoLivenessCheck => "Unknown", + Self::NoEndpointsReady + | Self::MetricsStale + | Self::ScrapeTimedOut + | Self::ScrapeUnauthorized + | Self::TlsHandshakeFailed + | Self::ScrapeFailed + | Self::ProviderUnavailable => "NotReady", + } + } + + /// Whether the provider is known not to serve: unknown counts as ready. + #[must_use] + pub const fn excludes(self) -> bool { + !matches!( + self, + Self::Ready | Self::MetricsNotConfigured | Self::AwaitingFirstScrape | Self::NoLivenessCheck + ) + } +} + +/// Why one scrape failed, bounded so it can label a metric. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum ScrapeClass { + /// The request timed out. + Timeout, + /// The endpoint answered 401 or 403. + Unauthorized, + /// TLS failed: a handshake, a certificate, or the TLS material. + Tls, + /// The endpoint's name did not resolve. + Dns, + /// The connection failed. + Connect, + /// The endpoint answered with another non-2xx status. + Http, + /// The body passed the size limit. + BodyCap, + /// The body could not be read as text. + Parse, + /// The scrape could not be built: the URL, the credential, or plaintext refused. + Config, +} + +impl ScrapeClass { + /// The class as a metric label and message word. + #[must_use] + pub const fn as_str(self) -> &'static str { + match self { + Self::Timeout => "timeout", + Self::Unauthorized => "unauthorized", + Self::Tls => "tls", + Self::Dns => "dns", + Self::Connect => "connect", + Self::Http => "http", + Self::BodyCap => "body_cap", + Self::Parse => "parse", + Self::Config => "config", + } + } + + /// The `Ready` reason when this was the latest failure and nothing succeeded since. + const fn reason(self) -> Reason { + match self { + Self::Timeout => Reason::ScrapeTimedOut, + Self::Unauthorized => Reason::ScrapeUnauthorized, + Self::Tls => Reason::TlsHandshakeFailed, + Self::Dns | Self::Connect | Self::Http | Self::BodyCap | Self::Parse | Self::Config => Reason::ScrapeFailed, + } + } +} + +/// What the signals loop last saw from one provider. +#[derive(Clone, Debug, Default)] +struct Probe { + /// When the first scrape was attempted, which starts the grace before any success. + first_attempt: Option, + /// When the last scrape succeeded. + last_good: Option, + /// Ready endpoints in that scrape, `None` when it exposed no count. + ready_endpoints: Option, + /// Signals from the latest success, until the signals loop publishes them once. + unpublished: Option>, + /// The latest attempt's failure, `None` when it succeeded. + failure: Option, + /// What the latest success lacked, when it exposed no ready-endpoint count. + missing: Option, + /// Consecutive successful scrapes that read zero ready endpoints. + zero_streak: u32, + /// Consecutive successful scrapes that read some ready endpoints. + ready_streak: u32, + /// Whether the count marks the provider not ready, with [`STREAK`] hysteresis both ways. + no_endpoints: bool, + /// When the EPP last recorded an engine answer for this provider. + last_progress: Option, + /// Whether the latest zero count came while the engine was answering. + busy: bool, + /// Recent EPP latency snapshots. + latency: crate::latency::History, +} + +/// One provider's verdict and the detail behind it. +#[derive(Clone, Debug, PartialEq)] +pub struct Verdict { + /// Status and reason. + pub reason: Reason, + /// Human detail for the condition message. + pub message: String, +} + +/// The latest scrape per provider, keyed by network and routing identity. +#[derive(Debug, Default)] +pub struct ReadinessStore(Mutex>); + +/// The store key for `identity` in `network`: identities are unique per network only. +pub(crate) fn key(network: &str, identity: &str) -> String { + format!("{network}/{identity}") +} + +impl ReadinessStore { + /// The probes, recovered if a panicking holder poisoned the lock: each write is whole. + fn probes(&self) -> std::sync::MutexGuard<'_, HashMap> { + self.0.lock().unwrap_or_else(PoisonError::into_inner) + } + + /// Record a scrape's latency counters and return the latency series it publishes. + #[expect(clippy::significant_drop_tightening, reason = "the guard covers one whole update")] + pub(crate) fn record_latency(&self, key: &str, observations: &[Observation], now: Instant) -> Vec { + // Built before locking, so the store is held only to append it. + let snapshot = crate::latency::Snapshot::of(observations); + // The counters carry no pool label, so an EPP serving several pools proves nothing. + let attributable = !serves_several_pools(observations); + let mut probes = self.probes(); + let probe = probes.entry(key.to_owned()).or_default(); + if attributable + && probe + .latency + .latest() + .is_some_and(|earlier| snapshot.produced_since(earlier)) + { + probe.last_progress = Some(now); + } + probe.latency.record(snapshot, now) + } + + /// Record a successful scrape at `now`, with `missing` naming the series it lacked. + #[expect(clippy::significant_drop_tightening, reason = "the guard covers one whole update")] + #[expect(clippy::too_many_arguments, reason = "one scrape's count, gap, signals, and time")] + pub(crate) fn record_success( + &self, + key: &str, + ready_endpoints: Option, + missing: Option, + observations: Vec, + now: Instant, + ) { + let mut probes = self.probes(); + let probe = probes.entry(key.to_owned()).or_default(); + probe.first_attempt.get_or_insert(now); + match ready_endpoints { + Some(count) if count < 1.0 => { + probe.zero_streak = probe.zero_streak.saturating_add(1); + probe.ready_streak = 0; + probe.busy = probe + .last_progress + .is_some_and(|at| now.saturating_duration_since(at) <= PROGRESS_WINDOW); + // Answering releases the verdict as well as holding it off. Recovery that + // required the endpoint count to come back could not fire when the count + // was what broke: exclusion stops the traffic, and a verdict that needs + // traffic to clear would hold for as long as the metrics stayed down. + probe.no_endpoints = (probe.no_endpoints || probe.zero_streak >= STREAK) && !probe.busy; + }, + Some(_) => { + probe.ready_streak = probe.ready_streak.saturating_add(1); + probe.zero_streak = 0; + probe.busy = false; + probe.no_endpoints &= probe.ready_streak < STREAK; + }, + // No count to read: reachable is all that is known. + None => { + probe.zero_streak = 0; + probe.ready_streak = 0; + probe.busy = false; + probe.no_endpoints = false; + }, + } + probe.last_good = Some(now); + probe.ready_endpoints = ready_endpoints; + probe.unpublished = Some(observations); + probe.failure = None; + probe.missing = ready_endpoints.is_none().then_some(missing).flatten(); + } + + /// Record a failed scrape at `now`, keeping what the last good one saw. + #[expect(clippy::significant_drop_tightening, reason = "the guard covers one whole update")] + pub(crate) fn record_failure(&self, key: &str, class: ScrapeClass, now: Instant) { + let mut probes = self.probes(); + let probe = probes.entry(key.to_owned()).or_default(); + probe.first_attempt.get_or_insert(now); + probe.failure = Some(class); + } + + /// The verdict for `key` at `now`. + /// + /// `AwaitingFirstScrape`, which is `Unknown` and publishes nothing, until the + /// provider has been attempted and through the first `stale_after` while no + /// scrape has succeeded yet, so a starting operator states that it is waiting. + #[expect( + clippy::significant_drop_tightening, + reason = "the guard covers one small evaluation" + )] + pub(crate) fn verdict(&self, key: &str, unavailable: bool, stale_after: Duration, now: Instant) -> Option { + let awaiting = Verdict { + reason: Reason::AwaitingFirstScrape, + message: "awaiting the first metrics scrape".to_owned(), + }; + let probes = self.probes(); + let Some(probe) = probes.get(key) else { + return (!unavailable).then_some(awaiting); + }; + let within = |at: Instant| now.saturating_duration_since(at) <= stale_after; + if probe.last_good.is_none() && !unavailable && probe.first_attempt.is_some_and(within) { + return Some(awaiting); + } + Some(evaluate( + probe, + probe.last_good.is_some_and(within), + unavailable, + stale_after, + )) + } + + /// The signals of the latest successful scrape, once: stale load is never republished. + pub(crate) fn take_fresh(&self, key: &str) -> Option> { + self.probes().get_mut(key)?.unpublished.take() + } + + /// Ready endpoints in the provider's last successful scrape, `None` when it exposed no count. + pub(crate) fn ready_endpoints(&self, key: &str) -> Option { + self.probes().get(key)?.ready_endpoints + } + + /// Forget providers not in `keep`. + pub(crate) fn retain(&self, keep: &std::collections::BTreeSet) { + self.probes().retain(|key, _| { + let kept = keep.contains(key); + if !kept && let Some((_, identity)) = key.split_once('/') { + crate::metrics::forget_provider_scrapes(identity); + } + kept + }); + } +} + +/// The verdict for one probe. `fresh` says whether its last success is within `stale_after`. +fn evaluate(probe: &Probe, fresh: bool, unavailable: bool, stale_after: Duration) -> Verdict { + if unavailable { + return Verdict { + reason: Reason::ProviderUnavailable, + message: "provider is Unavailable".to_owned(), + }; + } + if !fresh { + let window = stale_after.as_secs(); + return match probe.failure { + Some(class) => Verdict { + reason: class.reason(), + message: format!( + "no successful metrics scrape in the last {window}s; the latest failed: {}", + class.as_str() + ), + }, + None => Verdict { + reason: Reason::MetricsStale, + message: format!("no successful metrics scrape in the last {window}s"), + }, + }; + } + from_ready_endpoints(probe) +} + +/// The verdict for a freshly scraped provider, from its ready-endpoint count. +fn from_ready_endpoints(probe: &Probe) -> Verdict { + let count = probe.ready_endpoints; + if probe.no_endpoints { + let message = match count { + Some(count) if count >= 1.0 => format!("{count} ready endpoints, awaiting a second scrape"), + _ => format!("0 ready endpoints for {STREAK} or more scrapes"), + }; + return Verdict { + reason: Reason::NoEndpointsReady, + message, + }; + } + let Some(count) = count else { + return Verdict { + reason: Reason::NoLivenessCheck, + message: probe + .missing + .clone() + .unwrap_or_else(|| "metrics reachable, but no ready-endpoint series".to_owned()), + }; + }; + Verdict { + reason: Reason::Ready, + message: ready_message(count, probe.busy), + } +} + +/// The detail for a ready provider counting `count` endpoints. +fn ready_message(count: f64, busy: bool) -> String { + if count >= 1.0 { + format!("{count} ready endpoints") + } else if busy { + format!( + "0 ready endpoints, but the engine answered in the last {}s", + PROGRESS_WINDOW.as_secs() + ) + } else { + "0 ready endpoints in the latest scrape only".to_owned() + } +} + +/// The condition type this module owns. +pub const READY_CONDITION: &str = "Ready"; + +/// The `Ready` condition to write, or `None` when `current` already says the same. +/// +/// A changed message alone is not written, so a moving endpoint count does not +/// churn status. A new generation is. `lastTransitionTime` moves only when the status does. +pub(crate) fn ready_condition( + current: &[Condition], + verdict: &Verdict, + now_rfc3339: &str, + generation: Option, +) -> Option { + let held = current.iter().find(|c| c.type_ == READY_CONDITION); + let status = verdict.reason.status(); + if held.is_some_and(|c| { + c.status == status && c.reason == verdict.reason.as_str() && c.observed_generation == generation + }) { + return None; + } + let last_transition_time = held + .filter(|c| c.status == status) + .map_or_else(|| now_rfc3339.to_owned(), |c| c.last_transition_time.clone()); + Some(Condition { + type_: READY_CONDITION.to_owned(), + status: status.to_owned(), + reason: verdict.reason.as_str().to_owned(), + message: verdict.message.clone(), + last_transition_time, + observed_generation: generation, + }) +} + +/// Ready endpoints for `pool` in `observations`, from the first of `names` present. +/// +/// With a pool, only series whose `name` label is that pool count, so an EPP that +/// serves several pools is read for the configured one. +pub(crate) fn ready_endpoints(observations: &[Observation], names: &[&str], pool: Option<&str>) -> Option { + names.iter().find_map(|name| { + observations + .iter() + .filter(|o| o.metric == *name) + .filter(|o| pool.is_none_or(|pool| o.labels.get("name").is_some_and(|n| n == pool))) + .map(|o| o.value) + .reduce(f64::max) + }) +} + +/// Where a provider's in-flight count came from. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(crate) enum InFlightSource { + /// The EPP's inflight-load-producer, at or above the pool averages. + Epp, + /// The pool averages of running and queued times ready endpoints, above the EPP count or + /// in place of it. + EngineAverages, +} + +/// Requests the provider holds: the larger of the EPP's per-endpoint count summed and its +/// pool averages of running and queued times `ready`, plus requests flow control holds. +/// +/// The larger of the two, so an EPP restart that zeroes its count does not read as idle. The +/// per-endpoint count carries no pool label, so it is skipped when the EPP serves several +/// pools. `None` when neither source is present, or the site has no fresh endpoint behind +/// frozen averages. +pub(crate) fn in_flight( + observations: &[Observation], + ready: Option, + pool: Option<&str>, +) -> Option<(f64, InFlightSource)> { + let counted = (!serves_several_pools(observations)) + .then(|| epp_count(observations)) + .flatten(); + // No fresh endpoint means the averages are frozen at their last value: unknown, not idle. + let averaged = ready.filter(|ready| *ready > 0.0).and_then(|ready| { + EPP_AVERAGES + .iter() + .map(|names| ready_endpoints(observations, names, pool)) + .sum::>() + .map(|per_endpoint| per_endpoint * ready) + }); + let (held, source) = match (counted, averaged) { + (Some(counted), Some(averaged)) if averaged > counted => (averaged, InFlightSource::EngineAverages), + (Some(counted), _) => (counted, InFlightSource::Epp), + (None, Some(averaged)) => (averaged, InFlightSource::EngineAverages), + (None, None) => return None, + }; + Some((held + flow_control_queued(observations, pool), source)) +} + +/// The EPP's in-flight count summed over endpoints, each endpoint's largest across producer +/// instances so two producers do not double it. `None` when the EPP exports none. +fn epp_count(observations: &[Observation]) -> Option { + let mut producers: std::collections::BTreeMap<&str, std::collections::BTreeMap<(&str, &str), f64>> = + std::collections::BTreeMap::new(); + for observation in observations.iter().filter(|o| o.metric == EPP_IN_FLIGHT) { + let label = |name: &str| observation.labels.get(name).map_or("", String::as_str); + // Series per fairness and priority add up; producer instances repeat the same requests. + *producers + .entry(label("producer_name")) + .or_default() + .entry((label("namespace"), label("endpoint_name"))) + .or_insert(0.0) += observation.value; + } + let mut peak: std::collections::BTreeMap<(&str, &str), f64> = std::collections::BTreeMap::new(); + for counts in producers.values() { + for (endpoint, count) in counts { + let held = peak.entry(*endpoint).or_insert(0.0); + *held = held.max(*count); + } + } + (!peak.is_empty()).then(|| peak.values().sum()) +} + +/// Whether the EPP reports more than one pool or namespace, so its unlabeled per-endpoint +/// count cannot be attributed to one pool. +fn serves_several_pools(observations: &[Observation]) -> bool { + let distinct = |metrics: &[&str], label: &str| { + observations + .iter() + .filter(|o| metrics.contains(&o.metric.as_str())) + .filter_map(|o| o.labels.get(label)) + .collect::>() + .len() + }; + distinct(&DEFAULT_READY_ENDPOINTS, "name") > 1 || distinct(&[EPP_IN_FLIGHT], "namespace") > 1 +} + +/// Requests the EPP's flow control holds for `pool`, 0 when it exports none. +fn flow_control_queued(observations: &[Observation], pool: Option<&str>) -> f64 { + observations + .iter() + .filter(|o| o.metric == EPP_FLOW_CONTROL_QUEUE) + .filter(|o| crate::signals::in_pool(o, pool)) + .map(|o| o.value) + .sum() +} + +#[cfg(test)] +#[expect(clippy::allow_attributes, reason = "blanket test suppressions")] +#[allow( + clippy::unwrap_used, + clippy::expect_used, + clippy::too_many_lines, + clippy::shadow_unrelated, + reason = "tests" +)] +mod tests { + use std::collections::BTreeMap; + + use super::*; + + const STALE: Duration = Duration::from_secs(15); + + fn sample(metric: &str, pool: &str, value: f64) -> Observation { + Observation { + metric: metric.to_owned(), + labels: BTreeMap::from([("name".to_owned(), pool.to_owned())]), + value, + timestamp_ms: None, + } + } + + #[test] + fn ready_endpoints_prefers_the_current_name_and_reads_the_configured_pool() { + let observations = [ + sample("inference_pool_ready_pods", "qwen3", 4.0), + sample("llm_d_epp_ready_endpoints", "other", 9.0), + sample("llm_d_epp_ready_endpoints", "qwen3", 2.0), + ]; + assert_eq!( + ready_endpoints(&observations, &DEFAULT_READY_ENDPOINTS, Some("qwen3")), + Some(2.0) + ); + assert_eq!( + ready_endpoints(&observations[..1], &DEFAULT_READY_ENDPOINTS, Some("qwen3")), + Some(4.0), + "falls back to the deprecated name" + ); + assert_eq!( + ready_endpoints(&observations, &DEFAULT_READY_ENDPOINTS, Some("absent")), + None + ); + } + + fn reason(store: &ReadinessStore, now: Instant) -> Option { + store.verdict("p", false, STALE, now).map(|verdict| verdict.reason) + } + + #[test] + fn verdict_follows_the_latest_scrape_and_staleness() { + let store = ReadinessStore::default(); + let start = Instant::now(); + assert_eq!( + reason(&store, start), + Some(Reason::AwaitingFirstScrape), + "never attempted" + ); + + store.record_success("p", Some(2.0), None, vec![sample("q", "p", 1.0)], start); + assert_eq!(reason(&store, start), Some(Reason::Ready)); + assert_eq!(store.take_fresh("p").map(|held| held.len()), Some(1)); + assert_eq!(store.take_fresh("p"), None, "fresh signals are published once"); + + let missing = "metrics reachable, but no llm_d_epp_ready_endpoints series for poolName qwen3"; + store.record_success("p", None, Some(missing.to_owned()), Vec::new(), start); + let verdict = store.verdict("p", false, STALE, start).expect("judged"); + assert_eq!( + (verdict.reason, verdict.message.as_str()), + (Reason::NoLivenessCheck, missing), + "a 200 without the pool's series leaves readiness unknown, and says which series" + ); + assert!( + !verdict.reason.excludes(), + "unknown readiness does not exclude: a vLLM provider carries no such series" + ); + assert_eq!(store.take_fresh("p").map(|held| held.len()), Some(0)); + + store.record_failure("p", ScrapeClass::Connect, start + STALE); + assert_eq!( + reason(&store, start + STALE), + Some(Reason::NoLivenessCheck), + "a failure within the window keeps the last verdict" + ); + assert_eq!(store.take_fresh("p"), None, "a failure publishes no held load"); + let stale = store + .verdict("p", false, STALE, start + STALE + Duration::from_secs(1)) + .expect("judged"); + assert_eq!(stale.reason, Reason::ScrapeFailed); + assert!( + stale.message.ends_with("the latest failed: connect"), + "{}", + stale.message + ); + } + + #[test] + fn zero_endpoints_take_two_scrapes_to_exclude_and_two_to_readmit() { + let store = ReadinessStore::default(); + let now = Instant::now(); + let scrape = |count| { + store.record_success("p", Some(count), None, Vec::new(), now); + reason(&store, now) + }; + assert_eq!(scrape(0.0), Some(Reason::Ready), "one zero reading is not enough"); + assert_eq!(scrape(0.0), Some(Reason::NoEndpointsReady)); + assert_eq!( + scrape(1.0), + Some(Reason::NoEndpointsReady), + "one ready reading is not enough" + ); + assert_eq!( + scrape(0.0), + Some(Reason::NoEndpointsReady), + "a relapse restarts the count" + ); + assert_eq!(scrape(1.0), Some(Reason::NoEndpointsReady)); + assert_eq!(scrape(1.0), Some(Reason::Ready)); + } + + /// One EPP scrape for pool qwen3: its ready count, a running average frozen at 128, and + /// `answered` usage reports so far. + fn saturated(ready: f64, answered: f64) -> Vec { + let unlabeled = |metric: &str, value: f64| Observation { + metric: metric.to_owned(), + labels: BTreeMap::new(), + value, + timestamp_ms: None, + }; + vec![ + sample("llm_d_epp_ready_endpoints", "qwen3", ready), + sample("llm_d_epp_average_running_requests", "qwen3", 128.0), + unlabeled("llm_d_epp_request_input_tokens_count", answered), + unlabeled("llm_d_epp_request_input_tokens_sum", answered * 100.0), + ] + } + + /// Record `observations` as the signals loop does, and return the verdict. + fn scrape_at(store: &ReadinessStore, observations: &[Observation], now: Instant) -> Option { + let ready = ready_endpoints(observations, &DEFAULT_READY_ENDPOINTS, Some("qwen3")); + store.record_latency("p", observations, now); + store.record_success("p", ready, None, observations.to_vec(), now); + reason(store, now) + } + + const STEP: Duration = Duration::from_secs(5); + + #[test] + fn a_saturated_pool_that_still_answers_stays_ready_on_zero_counts() { + let store = ReadinessStore::default(); + let start = Instant::now(); + assert_eq!(scrape_at(&store, &saturated(2.0, 100.0), start), Some(Reason::Ready)); + for (n, answered) in [(1_u32, 140.0), (2, 180.0), (3, 220.0)] { + assert_eq!( + scrape_at(&store, &saturated(0.0, answered), start + STEP * n), + Some(Reason::Ready), + "scrape {n}: running 128 and answering is busy, not down" + ); + } + let verdict = store.verdict("p", false, STALE, start + STEP * 3).unwrap(); + assert!(verdict.message.contains("answered"), "{}", verdict.message); + } + + #[test] + fn a_zero_count_with_no_answers_excludes_even_with_running_requests() { + let store = ReadinessStore::default(); + let start = Instant::now(); + assert_eq!(scrape_at(&store, &saturated(2.0, 100.0), start), Some(Reason::Ready)); + assert_eq!( + scrape_at(&store, &saturated(0.0, 100.0), start + STEP), + Some(Reason::Ready) + ); + assert_eq!( + scrape_at(&store, &saturated(0.0, 100.0), start + STEP * 2), + Some(Reason::NoEndpointsReady), + "the running average freezes at zero endpoints, so it is no evidence" + ); + } + + #[test] + fn answering_again_readmits_a_pool_whose_endpoint_count_never_returns() { + let store = ReadinessStore::default(); + let start = Instant::now(); + scrape_at(&store, &saturated(2.0, 100.0), start); + // Eight scrapes of zero with no answers: 35s past the last one, so excluded. + for n in 1..=8_u32 { + scrape_at(&store, &saturated(0.0, 100.0), start + STEP * n); + } + assert_eq!( + reason(&store, start + STEP * 8), + Some(Reason::NoEndpointsReady), + "zero endpoints and nothing answering is down" + ); + // The count stays zero, as it does when the metrics path is what broke, and the + // pool answers again. Recovery must not require the series that caused the verdict. + assert_eq!( + scrape_at(&store, &saturated(0.0, 140.0), start + STEP * 9), + Some(Reason::Ready), + "answering readmits it even though the endpoint count never came back" + ); + } + + #[test] + fn a_pool_that_stops_answering_is_excluded_once_the_window_passes() { + let store = ReadinessStore::default(); + let start = Instant::now(); + scrape_at(&store, &saturated(2.0, 100.0), start); + let answered = start + STEP; + scrape_at(&store, &saturated(0.0, 140.0), answered); + // Six more scrapes reach 30s after the last answer: still inside the window. + for n in 1..=6_u32 { + assert_eq!( + scrape_at(&store, &saturated(0.0, 140.0), answered + STEP * n), + Some(Reason::Ready), + "{}s after the last answer", + (STEP * n).as_secs() + ); + } + assert_eq!( + scrape_at(&store, &saturated(0.0, 140.0), answered + STEP * 7), + Some(Reason::NoEndpointsReady) + ); + } + + #[test] + fn answers_from_an_epp_serving_several_pools_prove_nothing() { + let store = ReadinessStore::default(); + let start = Instant::now(); + let with_other_pool = |ready, answered| { + let mut observations = saturated(ready, answered); + observations.push(sample("llm_d_epp_ready_endpoints", "llama", 3.0)); + observations + }; + scrape_at(&store, &with_other_pool(2.0, 100.0), start); + scrape_at(&store, &with_other_pool(0.0, 140.0), start + STEP); + assert_eq!( + scrape_at(&store, &with_other_pool(0.0, 180.0), start + STEP * 2), + Some(Reason::NoEndpointsReady) + ); + } + + #[test] + fn a_first_scrape_that_fails_gets_the_staleness_window_before_a_verdict() { + let store = ReadinessStore::default(); + let start = Instant::now(); + store.record_failure("p", ScrapeClass::Timeout, start); + assert_eq!( + reason(&store, start + STALE), + Some(Reason::AwaitingFirstScrape), + "grace, not a guess" + ); + assert_eq!( + reason(&store, start + STALE + Duration::from_secs(1)), + Some(Reason::ScrapeTimedOut) + ); + } + + #[test] + fn a_failed_scrape_names_its_class() { + let cases = [ + (ScrapeClass::Timeout, Reason::ScrapeTimedOut), + (ScrapeClass::Unauthorized, Reason::ScrapeUnauthorized), + (ScrapeClass::Tls, Reason::TlsHandshakeFailed), + (ScrapeClass::Dns, Reason::ScrapeFailed), + (ScrapeClass::Connect, Reason::ScrapeFailed), + (ScrapeClass::Http, Reason::ScrapeFailed), + (ScrapeClass::BodyCap, Reason::ScrapeFailed), + (ScrapeClass::Parse, Reason::ScrapeFailed), + (ScrapeClass::Config, Reason::ScrapeFailed), + ]; + let start = Instant::now(); + for (class, expected) in cases { + let store = ReadinessStore::default(); + store.record_failure("p", class, start); + let verdict = store + .verdict("p", false, STALE, start + STALE + Duration::from_secs(1)) + .expect("judged"); + assert_eq!(verdict.reason, expected, "{class:?}"); + assert!( + verdict.message.ends_with(class.as_str()), + "{class:?}: {}", + verdict.message + ); + assert!(verdict.reason.excludes(), "{class:?}"); + } + assert_eq!( + Reason::TlsHandshakeFailed.as_str(), + "TLSHandshakeFailed", + "the contract's spelling" + ); + } + + #[test] + fn providers_are_keyed_per_network() { + let store = ReadinessStore::default(); + let now = Instant::now(); + for _ in 0..STREAK { + store.record_success(&key("east", "p"), Some(0.0), None, Vec::new(), now); + } + store.record_success(&key("west", "p"), Some(3.0), None, Vec::new(), now); + let reason_in = |network| { + store + .verdict(&key(network, "p"), false, STALE, now) + .map(|verdict| verdict.reason) + }; + assert_eq!(reason_in("east"), Some(Reason::NoEndpointsReady)); + assert_eq!(reason_in("west"), Some(Reason::Ready)); + } + + #[test] + fn an_unavailable_provider_is_not_ready_whatever_its_metrics_say() { + let store = ReadinessStore::default(); + let now = Instant::now(); + store.record_success("p", Some(3.0), None, Vec::new(), now); + let verdict = store.verdict("p", true, STALE, now).unwrap(); + assert_eq!(verdict.reason, Reason::ProviderUnavailable); + assert!(verdict.reason.excludes()); + } + + #[test] + fn the_status_column_reads_like_a_node() { + assert_eq!(Reason::Ready.display(), "Ready"); + assert_eq!(Reason::MetricsNotConfigured.display(), "Unknown"); + assert_eq!(Reason::AwaitingFirstScrape.display(), "Unknown"); + assert_eq!(Reason::NoLivenessCheck.display(), "Unknown"); + for reason in [ + Reason::NoEndpointsReady, + Reason::MetricsStale, + Reason::ScrapeFailed, + Reason::ProviderUnavailable, + ] { + assert_eq!(reason.display(), "NotReady", "{reason:?}"); + } + } + + #[test] + fn the_condition_is_written_on_a_status_or_reason_change_only() { + let ready = Verdict { + reason: Reason::Ready, + message: "2 ready endpoints".to_owned(), + }; + let first = ready_condition(&[], &ready, "t0", Some(3)).expect("absent is written"); + assert_eq!( + (first.status.as_str(), first.last_transition_time.as_str()), + ("True", "t0") + ); + + let recount = Verdict { + message: "3 ready endpoints".to_owned(), + ..ready + }; + assert!( + ready_condition(std::slice::from_ref(&first), &recount, "t1", Some(3)).is_none(), + "a new count alone is not news" + ); + let regenerated = ready_condition(std::slice::from_ref(&first), &recount, "t1", Some(4)) + .expect("a new generation is written"); + assert_eq!( + regenerated.last_transition_time, "t0", + "the same status keeps its transition time" + ); + + let down = Verdict { + reason: Reason::NoEndpointsReady, + message: "0 ready endpoints".to_owned(), + }; + let second = ready_condition(std::slice::from_ref(&first), &down, "t2", Some(3)).unwrap(); + assert_eq!( + (second.status.as_str(), second.last_transition_time.as_str()), + ("False", "t2") + ); + + let unreachable = Verdict { + reason: Reason::ScrapeFailed, + message: "no scrape".to_owned(), + }; + let third = ready_condition(std::slice::from_ref(&second), &unreachable, "t3", Some(3)).unwrap(); + assert_eq!( + third.last_transition_time, "t2", + "a new reason under the same status keeps the transition time" + ); + } + + #[test] + fn statuses_match_the_condition_contract() { + assert_eq!(Reason::Ready.status(), "True"); + assert_eq!(Reason::MetricsNotConfigured.status(), "Unknown"); + assert!(!Reason::MetricsNotConfigured.excludes(), "unknown is not down"); + assert_eq!(Reason::AwaitingFirstScrape.status(), "Unknown"); + assert!(!Reason::AwaitingFirstScrape.excludes(), "waiting is not down"); + assert_eq!(Reason::NoLivenessCheck.status(), "Unknown"); + assert!( + !Reason::NoLivenessCheck.excludes(), + "a scrape carrying no ready-endpoint series leaves readiness unknown, not false" + ); + for reason in [ + Reason::NoEndpointsReady, + Reason::MetricsStale, + Reason::ScrapeTimedOut, + Reason::ScrapeUnauthorized, + Reason::TlsHandshakeFailed, + Reason::ScrapeFailed, + Reason::ProviderUnavailable, + ] { + assert_eq!(reason.status(), "False"); + assert!(reason.excludes()); + } + } + + fn counted(endpoint: &str, namespace: &str, producer: &str, value: f64) -> Observation { + Observation { + metric: "llm_d_epp_inflight_requests".to_owned(), + labels: BTreeMap::from([ + ("endpoint_name".to_owned(), endpoint.to_owned()), + ("namespace".to_owned(), namespace.to_owned()), + ("producer_name".to_owned(), producer.to_owned()), + ]), + value, + timestamp_ms: None, + } + } + + fn queued(pool: &str, value: f64) -> Observation { + Observation { + metric: "llm_d_epp_flow_control_queue_size".to_owned(), + labels: BTreeMap::from([("inference_pool".to_owned(), pool.to_owned())]), + value, + timestamp_ms: None, + } + } + + #[test] + fn in_flight_reads_the_larger_of_the_epp_count_and_the_pool_averages() { + let averages = [ + sample("llm_d_epp_average_running_requests", "qwen3", 6.0), + sample("llm_d_epp_average_queue_size", "qwen3", 1.5), + ]; + let mut observations = vec![counted("a", "ns", "p", 7.0), counted("b", "ns", "p", 5.0)]; + assert_eq!( + in_flight(&observations, Some(2.0), None), + Some((12.0, InFlightSource::Epp)), + "the EPP's count alone" + ); + observations.extend(averages.clone()); + assert_eq!( + in_flight(&observations, Some(1.0), Some("qwen3")), + Some((12.0, InFlightSource::Epp)), + "12 counted is above (6 + 1.5) x 1" + ); + let restarted = [vec![counted("a", "ns", "p", 0.0)], averages.to_vec()].concat(); + assert_eq!( + in_flight(&restarted, Some(2.0), Some("qwen3")), + Some((15.0, InFlightSource::EngineAverages)), + "an EPP restart reading 0 is floored by (6 + 1.5) x 2" + ); + assert_eq!( + in_flight(&averages, Some(2.0), Some("qwen3")), + Some((15.0, InFlightSource::EngineAverages)), + "the averages alone" + ); + } + + #[test] + fn in_flight_counts_each_endpoint_once_across_producers() { + let observations = [ + counted("a", "ns", "first", 4.0), + counted("a", "ns", "second", 3.0), + counted("b", "ns", "first", 2.0), + ]; + assert_eq!( + in_flight(&observations, None, None), + Some((6.0, InFlightSource::Epp)), + "4 + 2, not 9" + ); + } + + #[test] + fn in_flight_adds_requests_flow_control_holds_for_the_pool() { + let observations = [counted("a", "ns", "p", 3.0), queued("qwen3", 4.0), queued("other", 9.0)]; + assert_eq!( + in_flight(&observations, None, Some("qwen3")), + Some((7.0, InFlightSource::Epp)), + "3 dispatched plus 4 held for qwen3, not other's 9" + ); + } + + #[test] + fn in_flight_uses_pool_averages_when_the_epp_serves_several_pools() { + let observations = [ + counted("a", "ns", "p", 50.0), + sample("llm_d_epp_ready_endpoints", "qwen3", 2.0), + sample("llm_d_epp_ready_endpoints", "llama", 1.0), + sample("llm_d_epp_average_running_requests", "qwen3", 3.0), + sample("llm_d_epp_average_queue_size", "qwen3", 1.0), + ]; + assert_eq!( + in_flight(&observations, Some(2.0), Some("qwen3")), + Some((8.0, InFlightSource::EngineAverages)), + "the unlabeled count covers both pools, so (3 + 1) x 2 for qwen3" + ); + } + + #[test] + fn in_flight_is_unknown_rather_than_idle() { + let averages = [ + sample("llm_d_epp_average_running_requests", "qwen3", 6.0), + sample("llm_d_epp_average_queue_size", "qwen3", 1.5), + ]; + assert_eq!(in_flight(&averages, None, None), None, "no endpoint count, no estimate"); + assert_eq!( + in_flight(&averages, Some(0.0), Some("qwen3")), + None, + "no fresh endpoint behind frozen averages" + ); + assert_eq!( + in_flight(&averages[..1], Some(2.0), None), + None, + "a missing average is not zero" + ); + assert_eq!( + in_flight(&[queued("qwen3", 4.0)], None, Some("qwen3")), + None, + "a queue alone is no count" + ); + } +} From 5c6df57a5513983027165740c1299f71399172fe Mon Sep 17 00:00:00 2001 From: Sam Batschelet Date: Wed, 7 Oct 2026 12:26:34 -0400 Subject: [PATCH 03/10] feat(operator): conclude a provider's in-flight, ready endpoints, latency, and error ratio from its EPP In-flight is the larger of the EPP's per-endpoint count summed and the pool's running plus queued times ready endpoints, plus what flow control holds. Latency comes from the EPP's histograms and counters over the last 30 s, published once 20 requests completed in the window, with an error ratio because the histograms record successes only. One pool filter, and an unlabeled series joins no named pool. No prefill estimate, and no series nothing publishes. Signed-off-by: Sam Batschelet --- operator/src/latency.rs | 389 +++++++++++++++++++++ operator/src/resources/provider_metrics.rs | 291 ++++++++++++--- 2 files changed, 634 insertions(+), 46 deletions(-) create mode 100644 operator/src/latency.rs diff --git a/operator/src/latency.rs b/operator/src/latency.rs new file mode 100644 index 000000000..4d0ffdb0c --- /dev/null +++ b/operator/src/latency.rs @@ -0,0 +1,389 @@ +//! A provider's recent request latency, from deltas of its EPP's cumulative histograms. +//! +//! A value with too few completed requests in the window is left unpublished, so it reads as unknown. + +use std::{ + collections::{BTreeMap, VecDeque}, + time::{Duration, Instant}, +}; + +use crate::signals::Observation; + +/// How far back the published latency looks. +pub(crate) const WINDOW: Duration = Duration::from_secs(30); + +/// Completed requests a value needs in the window before it is published. +pub(crate) const MIN_SAMPLES: f64 = 20.0; + +/// Time to first token at the median, streaming requests only, seconds. +pub const TTFT_P50_SIGNAL: &str = "grid_provider_ttft_p50_seconds"; +/// Time to first token at the 90th percentile, streaming requests only, seconds. +pub const TTFT_P90_SIGNAL: &str = "grid_provider_ttft_p90_seconds"; +/// Mean time per output token, streaming requests only, seconds. +pub const TPOT_SIGNAL: &str = "grid_provider_tpot_seconds"; +/// Failed requests over all requests. +pub const ERROR_RATIO_SIGNAL: &str = "grid_provider_error_ratio"; + +/// EPP time to first token, labeled `streaming`. +const TTFT: &str = "llm_d_epp_request_ttft_seconds"; +/// EPP time per output token, streaming requests. +const TPOT: &str = "llm_d_epp_request_streaming_tpot_seconds"; +/// EPP input tokens per request. +const INPUT_TOKENS: &str = "llm_d_epp_request_input_tokens"; +/// EPP requests processed. +const REQUESTS: &str = "llm_d_epp_request_total"; +/// EPP requests that failed. +const ERRORS: &str = "llm_d_epp_request_error_total"; + +/// A histogram's `_sum` and `_count`. +#[derive(Clone, Copy, Debug, Default, PartialEq)] +struct SumCount { + /// Summed observations. + sum: f64, + /// Observations. + count: f64, +} + +impl SumCount { + /// `self - earlier`, `None` when a counter went backwards, as after an EPP restart. + fn since(self, earlier: Self) -> Option { + (self.sum >= earlier.sum && self.count >= earlier.count).then_some(Self { + sum: self.sum - earlier.sum, + count: self.count - earlier.count, + }) + } + + /// The mean observation, `None` with none. + fn mean(self) -> Option { + (self.count > 0.0).then(|| self.sum / self.count) + } +} + +/// The EPP's counters summed over models, fairness and priority: cumulative at one scrape, +/// or the difference across a window. +#[derive(Clone, Debug, Default, PartialEq)] +pub(crate) struct Snapshot { + /// Streaming TTFT bucket counts by upper bound, in bound order. + ttft_buckets: Vec<(f64, f64)>, + /// Streaming TTFT. + ttft: SumCount, + /// Streaming time per output token. + tpot: SumCount, + /// Input tokens per request. + input: SumCount, + /// Requests processed. + requests: f64, + /// Requests failed. + errors: f64, +} + +/// The sum of `metric` over the observations `keep` accepts, `None` when none match. +fn summed(observations: &[Observation], metric: &str, keep: impl Fn(&Observation) -> bool) -> Option { + observations + .iter() + .filter(|o| o.metric == metric && keep(o)) + .map(|o| o.value) + .reduce(|a, b| a + b) +} + +/// The `_sum` and `_count` of histogram `name` over the observations `keep` accepts. +fn sum_count(observations: &[Observation], name: &str, keep: impl Fn(&Observation) -> bool + Copy) -> Option { + Some(SumCount { + sum: summed(observations, &format!("{name}_sum"), keep)?, + count: summed(observations, &format!("{name}_count"), keep)?, + }) +} + +/// Whether a TTFT series counts streaming requests. +fn streaming(o: &Observation) -> bool { + o.labels.get("streaming").is_some_and(|value| value == "true") +} + +/// Streaming TTFT bucket counts by upper bound, summed across series. +fn ttft_buckets(observations: &[Observation]) -> Vec<(f64, f64)> { + let mut buckets: BTreeMap = BTreeMap::new(); + let bucket = format!("{TTFT}_bucket"); + for o in observations.iter().filter(|o| o.metric == bucket && streaming(o)) { + if let Some(bound) = o.labels.get("le").and_then(|le| le.parse::().ok()) { + // Bounds are positive, so their bit patterns sort as the values do. + buckets.entry(bound.to_bits()).or_insert((bound, 0.0)).1 += o.value; + } + } + buckets.into_values().collect() +} + +impl Snapshot { + /// The cumulative counters in one scrape. + pub(crate) fn of(observations: &[Observation]) -> Self { + let any = |_: &Observation| true; + Self { + ttft_buckets: ttft_buckets(observations), + ttft: sum_count(observations, TTFT, streaming).unwrap_or_default(), + tpot: sum_count(observations, TPOT, any).unwrap_or_default(), + input: sum_count(observations, INPUT_TOKENS, any).unwrap_or_default(), + requests: summed(observations, REQUESTS, any).unwrap_or(0.0), + errors: summed(observations, ERRORS, any).unwrap_or(0.0), + } + } + + /// Whether the engine answered since `earlier`: a first token, a completion, or a usage + /// report. + /// + /// Response-derived only. The request counter rises once scheduling succeeds, before any + /// response, so a stream that ends on EOF or cancellation would count as an answer and + /// hold a pool that accepts requests and never serves them out of its verdict. + pub(crate) fn produced_since(&self, earlier: &Self) -> bool { + self.ttft.count > earlier.ttft.count + || self.tpot.count > earlier.tpot.count + || self.input.count > earlier.input.count + } + + /// `self - earlier`, `None` when any counter went backwards. + fn since(&self, earlier: &Self) -> Option { + (self.requests >= earlier.requests && self.errors >= earlier.errors).then_some(())?; + Some(Self { + ttft_buckets: self.bucket_deltas(earlier)?, + ttft: self.ttft.since(earlier.ttft)?, + tpot: self.tpot.since(earlier.tpot)?, + input: self.input.since(earlier.input)?, + requests: self.requests - earlier.requests, + errors: self.errors - earlier.errors, + }) + } + + /// Each TTFT bucket's count less `earlier`'s, `None` when one went backwards. + fn bucket_deltas(&self, earlier: &Self) -> Option> { + self.ttft_buckets + .iter() + .map(|(bound, count)| { + let then = earlier + .ttft_buckets + .iter() + .find(|(b, _)| b.to_bits() == bound.to_bits()) + .map_or(0.0, |(_, earlier_count)| *earlier_count); + (*count >= then).then_some((*bound, count - then)) + }) + .collect() + } + + /// The series this window has enough samples to publish. + fn observations(&self) -> Vec { + let enough = |count: f64| count >= MIN_SAMPLES; + let mut published = Vec::new(); + if enough(self.ttft.count) { + published.extend(quantile(&self.ttft_buckets, 0.5).map(|value| sample(TTFT_P50_SIGNAL, value))); + published.extend(quantile(&self.ttft_buckets, 0.9).map(|value| sample(TTFT_P90_SIGNAL, value))); + } + if enough(self.tpot.count) { + published.extend(self.tpot.mean().map(|value| sample(TPOT_SIGNAL, value))); + } + if enough(self.requests) { + published.push(sample( + ERROR_RATIO_SIGNAL, + (self.errors / self.requests).clamp(0.0, 1.0), + )); + } + published + } +} + +/// One provider's recent cumulative snapshots. +#[derive(Clone, Debug, Default)] +pub(crate) struct History { + /// Snapshots by scrape time, oldest first, one at or before the window's start. + snapshots: VecDeque<(Instant, Snapshot)>, +} + +impl History { + /// The latest recorded scrape. + pub(crate) fn latest(&self) -> Option<&Snapshot> { + self.snapshots.back().map(|(_, snapshot)| snapshot) + } + + /// Record this scrape and return the latency series to publish for it. + pub(crate) fn record(&mut self, snapshot: Snapshot, now: Instant) -> Vec { + while self + .snapshots + .get(1) + .is_some_and(|(at, _)| now.saturating_duration_since(*at) >= WINDOW) + { + self.snapshots.pop_front(); + } + let published = self + .snapshots + .front() + .and_then(|(_, oldest)| snapshot.since(oldest)) + .map_or_else(Vec::new, |window| window.observations()); + if self + .snapshots + .back() + .is_some_and(|(_, last)| snapshot.since(last).is_none()) + { + // A counter reset: start over from this scrape. + self.snapshots.clear(); + } + self.snapshots.push_back((now, snapshot)); + published + } +} + +/// The `q` quantile of cumulative bucket counts by upper bound, interpolated within its bucket. +fn quantile(buckets: &[(f64, f64)], q: f64) -> Option { + let total = buckets.last()?.1; + if total <= 0.0 { + return None; + } + let rank = q * total; + let mut lower = (0.0, 0.0); + for (bound, count) in buckets { + if *count >= rank { + if bound.is_infinite() { + return Some(lower.0); + } + let within = count - lower.1; + let fraction = if within > 0.0 { (rank - lower.1) / within } else { 1.0 }; + return Some(lower.0 + (bound - lower.0) * fraction); + } + lower = (*bound, *count); + } + Some(lower.0) +} + +/// An unlabeled sample of `metric`. +fn sample(metric: &str, value: f64) -> Observation { + Observation { + metric: metric.to_owned(), + labels: BTreeMap::new(), + value, + timestamp_ms: None, + } +} + +#[cfg(test)] +#[expect(clippy::allow_attributes, reason = "blanket test suppressions")] +#[allow( + clippy::unwrap_used, + clippy::float_arithmetic, + clippy::arithmetic_side_effects, + clippy::indexing_slicing, + clippy::too_many_arguments, + reason = "tests" +)] +mod tests { + use super::*; + + impl History { + /// Record `observations` as one scrape, as the readiness store does. + fn record_observations(&mut self, observations: &[Observation], now: Instant) -> Vec { + self.record(Snapshot::of(observations), now) + } + } + + + fn obs(metric: &str, labels: &[(&str, &str)], value: f64) -> Observation { + Observation { + metric: metric.to_owned(), + labels: labels.iter().map(|(k, v)| ((*k).to_owned(), (*v).to_owned())).collect(), + value, + timestamp_ms: None, + } + } + + /// An EPP exposition after `requests` streaming requests, each with TTFT `ttft`, TPOT + /// `tpot`, `input` tokens, and `errors` failures. + fn scrape(requests: f64, ttft: f64, tpot: f64, input: f64, errors: f64) -> Vec { + let streaming = [("streaming", "true"), ("model_name", "m")]; + let mut out = vec![]; + for bound in [0.1, 0.2, 0.4, 0.8, 1.6, f64::INFINITY] { + let le = if bound.is_infinite() { + "+Inf".to_owned() + } else { + bound.to_string() + }; + let count = if ttft <= bound { requests } else { 0.0 }; + out.push(obs( + &format!("{TTFT}_bucket"), + &[streaming[0], streaming[1], ("le", &le)], + count, + )); + } + out.push(obs(&format!("{TTFT}_sum"), &streaming, ttft * requests)); + out.push(obs(&format!("{TTFT}_count"), &streaming, requests)); + // A non-streaming series must not count. + out.push(obs(&format!("{TTFT}_sum"), &[("streaming", "false")], 999.0)); + out.push(obs(&format!("{TTFT}_count"), &[("streaming", "false")], 1.0)); + out.push(obs(&format!("{TPOT}_sum"), &[], tpot * requests)); + out.push(obs(&format!("{TPOT}_count"), &[], requests)); + out.push(obs(&format!("{INPUT_TOKENS}_sum"), &[], input * requests)); + out.push(obs(&format!("{INPUT_TOKENS}_count"), &[], requests)); + out.push(obs(REQUESTS, &[], requests)); + out.push(obs(ERRORS, &[("error_code", "500")], errors)); + out + } + + fn value(published: &[Observation], metric: &str) -> Option { + published.iter().find(|o| o.metric == metric).map(|o| o.value) + } + + #[test] + fn a_window_publishes_latency_from_the_requests_inside_it() { + let mut history = History::default(); + let start = Instant::now(); + let first = history.record_observations(&scrape(100.0, 0.3, 0.02, 1000.0, 0.0), start); + assert!(first.is_empty(), "a first scrape has no window"); + let later = scrape(200.0, 0.3, 0.02, 1000.0, 5.0); + let published = history.record_observations(&later, start + Duration::from_secs(10)); + let p50 = value(&published, TTFT_P50_SIGNAL).unwrap(); + assert!( + (0.2..=0.4).contains(&p50), + "every request in the 0.2..0.4 bucket: {p50}" + ); + assert!(value(&published, TTFT_P90_SIGNAL).unwrap() >= p50); + assert!((value(&published, TPOT_SIGNAL).unwrap() - 0.02).abs() < 1e-9); + assert!((value(&published, ERROR_RATIO_SIGNAL).unwrap() - 0.05).abs() < 1e-9); + } + + #[test] + fn too_few_requests_publish_nothing() { + let mut history = History::default(); + let start = Instant::now(); + history.record_observations(&scrape(100.0, 0.3, 0.02, 1000.0, 0.0), start); + let published = + history.record_observations(&scrape(105.0, 0.3, 0.02, 1000.0, 0.0), start + Duration::from_secs(10)); + assert!(published.is_empty(), "5 requests is too few: {published:?}"); + } + + #[test] + fn a_counter_reset_publishes_nothing_and_starts_over() { + let mut history = History::default(); + let start = Instant::now(); + history.record_observations(&scrape(500.0, 0.3, 0.02, 1000.0, 0.0), start); + let reset = history.record_observations(&scrape(50.0, 0.3, 0.02, 1000.0, 0.0), start + Duration::from_secs(5)); + assert!(reset.is_empty(), "an EPP restart is not negative latency"); + let after = + history.record_observations(&scrape(100.0, 0.3, 0.02, 1000.0, 0.0), start + Duration::from_secs(10)); + assert!(value(&after, TPOT_SIGNAL).is_some(), "the window restarts at the reset"); + } + + #[test] + fn the_window_drops_snapshots_older_than_it() { + let mut history = History::default(); + let start = Instant::now(); + for (seconds, requests) in [(0, 0.0), (20, 100.0), (40, 100.0)] { + history.record_observations( + &scrape(requests, 0.3, 0.02, 1000.0, 0.0), + start + Duration::from_secs(seconds), + ); + } + let idle = history.record_observations(&scrape(100.0, 0.3, 0.02, 1000.0, 0.0), start + Duration::from_secs(60)); + assert!(idle.is_empty(), "nothing completed in the last 30s: {idle:?}"); + } + + + #[test] + fn quantiles_interpolate_within_a_bucket() { + let buckets = [(1.0, 0.0), (2.0, 10.0), (f64::INFINITY, 10.0)]; + assert!((quantile(&buckets, 0.5).unwrap() - 1.5).abs() < 1e-9); + assert_eq!(quantile(&[(1.0, 0.0), (f64::INFINITY, 0.0)], 0.5), None); + } +} diff --git a/operator/src/resources/provider_metrics.rs b/operator/src/resources/provider_metrics.rs index 7f9817b7c..ea877b277 100644 --- a/operator/src/resources/provider_metrics.rs +++ b/operator/src/resources/provider_metrics.rs @@ -107,89 +107,183 @@ pub(crate) struct CollectedMetrics { // Signals collection (poll mode) // --------------------------------------------------------------------------- -/// Scrape each provider's endpoint and keep only its configured coarse signals. +/// Scrape each provider's endpoint, recording its coarse signals and readiness. /// /// Sibling of [`collect_provider_metrics_with_refresh_interval`], which parses /// the same text into [`scoring::BackendMetrics`] for local scoring. This keeps /// the provider's own exposition narrowed to its declared `signalNames`, so the /// wire carries a coarse rollup rather than the full `/metrics` firehose. Fails /// closed on TLS: a provider whose TLS will not resolve is skipped, never -/// scraped in plaintext. A failed scrape leaves the last value to expire. +/// scraped in plaintext, and counts as a failed scrape. pub(crate) async fn collect_provider_signals( network_name: &str, providers: &[InferenceProvider], client: Option<&kube::Client>, -) -> HashMap> { - let mut out = HashMap::new(); + readiness: &crate::readiness::ReadinessStore, +) { for provider in providers { if provider.spec.grid_network_ref != network_name { continue; } - if let Some((identity, observations)) = scrape_provider_signals(provider, client).await { - out.insert(identity, observations); + let Some(plan) = signal_scrape_plan(provider) else { + continue; + }; + let key = crate::readiness::key(network_name, plan.identity); + match scrape_provider_signals(provider, &plan, client).await { + Ok(text) => record_scrape(readiness, &key, provider, &plan, &text), + Err(class) => { + crate::metrics::record_provider_scrape(plan.identity, class.as_str()); + readiness.record_failure(&key, class, Instant::now()); + }, } } - out } -/// Scrape one provider's coarse signals, or `None` to leave its last value be. -/// -/// Every skip and failure returns `None`: a provider absent from the collection -/// is left alone rather than erased, so a missed scrape expires on its own. +/// Record one successful scrape: its ready-endpoint count and its declared signals. +fn record_scrape( + readiness: &crate::readiness::ReadinessStore, + key: &str, + provider: &InferenceProvider, + plan: &SignalScrapePlan<'_>, + text: &str, +) { + // Every sample feeds this site's own windows; only gauges are republished. + let (parsed, republishable): (Vec<_>, Vec<_>) = crate::signals::parse_scrape(text).into_iter().unzip(); + let pool = provider + .spec + .metrics_config + .as_ref() + .and_then(|mc| mc.pool_name.as_deref()); + let ready = crate::readiness::ready_endpoints(&parsed, plan.ready_names(), pool); + let missing = count_scrape(plan, pool, ready); + let in_flight = in_flight_observation(&parsed, ready, pool, plan.identity); + let latency = readiness.record_latency(key, &parsed, Instant::now()); + let observations = parsed + .into_iter() + .zip(republishable) + .filter_map(|(o, republishable)| republishable.then_some(o)) + .filter(|o| plan.wanted.contains(o.metric.as_str())) + // A local sample's freshness is its collection time, so drop any trailing + // timestamp. Only relayed peer samples carry a per-sample stamp. + .map(|mut o| { + o.timestamp_ms = None; + o + }) + .chain(in_flight) + .chain(latency) + .collect(); + readiness.record_success(key, ready, missing, observations, Instant::now()); +} + +/// Count a scrape that answered: `success` with the pool's ready-endpoint series, else +/// `no_series`, returned as the message naming what was missing. +fn count_scrape(plan: &SignalScrapePlan<'_>, pool: Option<&str>, ready: Option) -> Option { + if ready.is_some() { + crate::metrics::record_provider_scrape(plan.identity, "success"); + crate::metrics::set_provider_last_scrape_success(plan.identity, std::time::SystemTime::now()); + return None; + } + crate::metrics::record_provider_scrape(plan.identity, "no_series"); + let series = plan.ready_names().join(" or "); + Some(match pool { + Some(pool) => format!("metrics reachable, but no {series} series for poolName {pool}"), + None => format!("metrics reachable, but no {series} series"), + }) +} + +/// The resolved `grid_provider_in_flight_requests` sample, logging which source it came from. +fn in_flight_observation( + parsed: &[crate::signals::Observation], + ready: Option, + pool: Option<&str>, + identity: &str, +) -> Option { + let (value, source) = crate::readiness::in_flight(parsed, ready, pool)?; + tracing::debug!( + provider = identity, + in_flight = value, + ?source, + "signals: provider in-flight resolved" + ); + Some(crate::signals::Observation { + metric: crate::readiness::IN_FLIGHT_SIGNAL.to_owned(), + labels: std::collections::BTreeMap::new(), + value, + timestamp_ms: None, + }) +} + +/// Scrape one provider's exposition, or the bounded class of the TLS or scrape failure. async fn scrape_provider_signals( provider: &InferenceProvider, + plan: &SignalScrapePlan<'_>, client: Option<&kube::Client>, -) -> Option<(String, Vec)> { - let (identity, url, wanted) = signal_scrape_plan(provider)?; - let mc = provider.spec.metrics_config.as_ref()?; +) -> Result { + let mc = provider + .spec + .metrics_config + .as_ref() + .ok_or(crate::readiness::ScrapeClass::Config)?; + let identity = plan.identity; let tls_settings = mc.tls.as_ref().or(provider.spec.tls.as_ref()); let tls_config = match resolve_tls_config(tls_settings, client, identity).await { Ok(cfg) => cfg, Err((_reason, e)) => { if tls_settings.is_some() { - tracing::warn!(provider = identity, error = %e, "signals: provider metrics TLS unavailable; not scraping in plaintext"); + tracing::debug!(provider = identity, error = %e, "signals: provider metrics TLS unavailable; not scraping in plaintext"); } - return None; + return Err(crate::readiness::ScrapeClass::Tls); }, }; let timeout = parse_metrics_timeout(&mc.timeout); - let text = scrape_metrics(&url, timeout, tls_config, mc.auth.as_ref().zip(client)) + scrape_metrics(&plan.url, timeout, tls_config, mc.auth.as_ref().zip(client)) .await - .inspect_err(|e| { - tracing::debug!(provider = identity, error = %e, "signals: provider scrape failed; last value left to expire"); - }) - .ok()?; - let observations = crate::signals::parse(&text) - .into_iter() - .filter(|o| wanted.contains(o.metric.as_str())) - // A local sample's freshness is its collection time, so drop any trailing - // timestamp. Only relayed peer samples carry a per-sample stamp. - .map(|mut o| { - o.timestamp_ms = None; - o + .map_err(|e| { + tracing::debug!(provider = identity, error = %e, "signals: provider scrape failed"); + e.class() }) - .collect(); - Some((identity.to_owned(), observations)) } -/// The scrape target for a provider's coarse signals, if it is eligible. +/// What to scrape for one provider and which of its series to republish. +pub(crate) struct SignalScrapePlan<'provider> { + /// The provider's routing identity. + pub(crate) identity: &'provider str, + /// The metrics URL. + pub(crate) url: String, + /// Declared signal names republished as coarse signals; may be empty. + pub(crate) wanted: std::collections::BTreeSet, + /// A declared ready-endpoints metric, replacing the defaults. + ready_override: Option<&'provider str>, +} + +impl SignalScrapePlan<'_> { + /// The ready-endpoints names to read, preferred first. + fn ready_names(&self) -> &[&str] { + self.ready_override + .as_ref() + .map_or(&crate::readiness::DEFAULT_READY_ENDPOINTS[..], std::slice::from_ref) + } +} + +/// The scrape plan for a provider, if it is eligible. /// /// Pure and synchronous: eligibility is decided here so the scrape path stays /// the I/O alone. `None` for a provider with no metrics config, no routing -/// identity, a blank endpoint, or no declared signal names. -fn signal_scrape_plan(provider: &InferenceProvider) -> Option<(&str, String, std::collections::BTreeSet)> { +/// identity, or a blank endpoint. A provider with no signal names is still +/// scraped, for readiness. +pub(crate) fn signal_scrape_plan(provider: &InferenceProvider) -> Option> { let mc = provider.spec.metrics_config.as_ref()?; let identity = routing_identity(provider)?; let endpoint = provider.spec.endpoint.trim(); if endpoint.is_empty() || mc.metrics_endpoint.as_deref().is_some_and(|ep| ep.trim().is_empty()) { return None; } - let wanted = signal_metric_names(&mc.signal_names); - if wanted.is_empty() { - return None; - } - let url = metrics_url(mc.metrics_endpoint.as_deref().unwrap_or(endpoint), &mc.path); - Some((identity, url, wanted)) + Some(SignalScrapePlan { + identity, + url: metrics_url(mc.metrics_endpoint.as_deref().unwrap_or(endpoint), &mc.path), + wanted: signal_metric_names(&mc.signal_names), + ready_override: mc.signal_names.ready_endpoints.as_deref(), + }) } /// The source metric names a provider declares for its coarse signals. @@ -656,6 +750,7 @@ pub(crate) fn classify_scrape_error(err: &metrics_scraper::MetricsScrapeError) - }, metrics_scraper::MetricsScrapeError::Credential(_) | metrics_scraper::MetricsScrapeError::PlaintextCredential(_) => "MetricsCredentialUnavailable", + metrics_scraper::MetricsScrapeError::BodyTooLarge(_) => "MetricsBodyTooLarge", metrics_scraper::MetricsScrapeError::InvalidUrl(_) | metrics_scraper::MetricsScrapeError::NonOkStatus { .. } | metrics_scraper::MetricsScrapeError::Encoding(_) => "MetricsScrapeError", @@ -755,6 +850,103 @@ mod tests { .unwrap_or_else(|_| std::process::abort()) } + /// A `KServe` EPP exposition for pool `qwen3-kserve`, in the label sets the lab EPP emits: + /// `served` streaming requests answered, about 104 usage reports each. + #[expect(clippy::too_many_lines, reason = "one exposition, line for line")] + fn kserve_epp_exposition(served: f64, ready: f64) -> String { + let l = r#"fairness_id="default-flow",model_name="qwen3",priority="0",target_model_name="qwen3""#; + let (b01, b05, b1) = (served * 0.2, served * 0.6, served * 0.9); + let (ttft_sum, tpot_sum) = (served * 0.4, served * 0.02); + let (reports, tokens) = (served * 104.0, served * 104.0 * 300.0); + let (requests, errors) = (served + 14.0, served / 100.0); + format!( + r#"# HELP llm_d_epp_ready_endpoints Ready endpoints. +# TYPE llm_d_epp_ready_endpoints gauge +llm_d_epp_ready_endpoints{{name="qwen3-kserve"}} {ready} +# TYPE llm_d_epp_average_running_requests gauge +llm_d_epp_average_running_requests{{name="qwen3-kserve"}} 128 +# TYPE llm_d_epp_request_ttft_seconds histogram +llm_d_epp_request_ttft_seconds_bucket{{{l},streaming="false",le="0.1"}} 14 +llm_d_epp_request_ttft_seconds_bucket{{{l},streaming="false",le="+Inf"}} 14 +llm_d_epp_request_ttft_seconds_sum{{{l},streaming="false"}} 1.9 +llm_d_epp_request_ttft_seconds_count{{{l},streaming="false"}} 14 +llm_d_epp_request_ttft_seconds_bucket{{{l},streaming="true",le="0.1"}} {b01} +llm_d_epp_request_ttft_seconds_bucket{{{l},streaming="true",le="0.5"}} {b05} +llm_d_epp_request_ttft_seconds_bucket{{{l},streaming="true",le="1"}} {b1} +llm_d_epp_request_ttft_seconds_bucket{{{l},streaming="true",le="2.5"}} {served} +llm_d_epp_request_ttft_seconds_bucket{{{l},streaming="true",le="+Inf"}} {served} +llm_d_epp_request_ttft_seconds_sum{{{l},streaming="true"}} {ttft_sum} +llm_d_epp_request_ttft_seconds_count{{{l},streaming="true"}} {served} +# TYPE llm_d_epp_request_streaming_tpot_seconds histogram +llm_d_epp_request_streaming_tpot_seconds_sum{{{l}}} {tpot_sum} +llm_d_epp_request_streaming_tpot_seconds_count{{{l}}} {served} +# TYPE llm_d_epp_request_input_tokens histogram +llm_d_epp_request_input_tokens_sum{{{l}}} {tokens} +llm_d_epp_request_input_tokens_count{{{l}}} {reports} +# TYPE llm_d_epp_request_total counter +llm_d_epp_request_total{{{l}}} {requests} +# TYPE llm_d_epp_request_error_total counter +llm_d_epp_request_error_total{{{l},error_code="503"}} {errors} +"# + ) + } + + #[test] + #[expect(clippy::too_many_lines, reason = "two scrapes and every published series")] + fn a_kserve_epp_scrape_publishes_latency_and_republishes_gauges_only() { + let readiness = crate::readiness::ReadinessStore::default(); + let mut mc = mc_with_queue("llm_d_epp_average_running_requests"); + mc.pool_name = Some("qwen3-kserve".to_owned()); + // A counter declared as a signal is still never republished. + mc.signal_names.error_rate = Some("llm_d_epp_request_total".to_owned()); + let provider = provider_fixture("qwen3-hq-east", "http://epp:9090", Some(mc)); + let plan = signal_scrape_plan(&provider).unwrap_or_else(|| std::process::abort()); + record_scrape( + &readiness, + "net/p", + &provider, + &plan, + &kserve_epp_exposition(11_000.0, 2.0), + ); + assert!(readiness.take_fresh("net/p").is_some(), "first scrape recorded"); + record_scrape( + &readiness, + "net/p", + &provider, + &plan, + &kserve_epp_exposition(11_581.0, 0.0), + ); + let published = readiness.take_fresh("net/p").unwrap_or_default(); + let value = |name: &str| published.iter().find(|o| o.metric == name).map(|o| o.value); + for name in [ + crate::latency::TTFT_P50_SIGNAL, + crate::latency::TTFT_P90_SIGNAL, + crate::latency::TPOT_SIGNAL, + crate::latency::ERROR_RATIO_SIGNAL, + ] { + assert!(value(name).is_some(), "{name} missing from {published:?}"); + } + assert!((value(crate::latency::TPOT_SIGNAL).unwrap_or_default() - 0.02).abs() < 1e-9); + assert!( + published.iter().all(|o| !o.metric.ends_with("_total") + && !o.metric.ends_with("_count") + && !o.metric.ends_with("_sum") + && !o.metric.ends_with("_bucket")), + "counters and histogram parts stay local: {published:?}" + ); + assert!( + value("llm_d_epp_average_running_requests").is_some(), + "declared gauge republished" + ); + let verdict = readiness + .verdict("net/p", false, Duration::from_secs(15), Instant::now()) + .unwrap_or_else(|| std::process::abort()); + assert!( + verdict.message.contains("answered"), + "progress seen through the parser: {verdict:?}" + ); + } + fn mc_with_queue(metric_name: &str) -> MetricsConfig { MetricsConfig { path: "/metrics".to_owned(), @@ -876,6 +1068,7 @@ mod tests { prefix_cache_hit_ratio: Some("my_prefix".to_owned()), error_rate: Some("my_errors".to_owned()), healthy: Some("my_health".to_owned()), + ready_endpoints: None, }; let names = metric_names_from_config(&cfg, None, None); assert_eq!(names.queue_depth.as_deref(), Some("my_queue")); @@ -1017,8 +1210,9 @@ mod tests { let mut provider = provider_fixture("prov-a", &endpoint, Some(mc_with_queue("queue_depth"))); provider.spec.tls = Some(crate::resources::test_doubles::endpoint_tls_for_ca("shared-ca")); - let signals = collect_provider_signals("net", &[provider], Some(&client)).await; - let observations = signals.get("prov-a").unwrap_or_else(|| std::process::abort()); + let readiness = crate::readiness::ReadinessStore::default(); + collect_provider_signals("net", &[provider], Some(&client), &readiness).await; + let observations = readiness.take_fresh("net/prov-a").unwrap_or_default(); assert!( observations @@ -1041,8 +1235,9 @@ mod tests { let mut provider = provider_fixture("prov-a", &endpoint, Some(metrics_config)); provider.spec.tls = Some(crate::resources::test_doubles::endpoint_tls_for_ca("missing-shared-ca")); - let signals = collect_provider_signals("net", &[provider], Some(&client)).await; - let observations = signals.get("prov-a").unwrap_or_else(|| std::process::abort()); + let readiness = crate::readiness::ReadinessStore::default(); + collect_provider_signals("net", &[provider], Some(&client), &readiness).await; + let observations = readiness.take_fresh("net/prov-a").unwrap_or_default(); assert!( observations @@ -1058,9 +1253,13 @@ mod tests { let mut provider = provider_fixture("prov-a", "https://localhost:1", Some(mc_with_queue("queue_depth"))); provider.spec.tls = Some(crate::resources::test_doubles::endpoint_tls_for_ca("missing-shared-ca")); - let signals = collect_provider_signals("net", &[provider], Some(&client)).await; + let readiness = crate::readiness::ReadinessStore::default(); + collect_provider_signals("net", &[provider], Some(&client), &readiness).await; - assert!(signals.is_empty(), "invalid shared TLS material must skip the scrape"); + assert!( + readiness.take_fresh("net/prov-a").is_none(), + "invalid shared TLS material must skip the scrape" + ); } #[tokio::test] From f7890ee2cd6154e1681d8df5857902778a247956 Mon Sep 17 00:00:00 2001 From: Sam Batschelet Date: Wed, 7 Oct 2026 12:26:34 -0400 Subject: [PATCH 04/10] feat(operator): export provider series on /metrics and bound what a hub keeps from peers The operator exports its provider series on its Prometheus listener for its own providers and the peers it polls. A hub keeps an allowlist of series from a peer, bounded per peer, so a peer cannot grow the hub's store. Signed-off-by: Sam Batschelet --- operator/src/metrics.rs | 311 ++++++++++++++++++++++++++++++++++++++++ operator/src/signals.rs | 237 +++++++++++++++++++++++++++++- 2 files changed, 542 insertions(+), 6 deletions(-) diff --git a/operator/src/metrics.rs b/operator/src/metrics.rs index 8369598e0..c27ad3417 100644 --- a/operator/src/metrics.rs +++ b/operator/src/metrics.rs @@ -39,6 +39,12 @@ static REGISTRY: LazyLock = LazyLock::new(|| { .unwrap_or_else(|_| std::process::abort()); r.register(Box::new(PEER_POLL_RETRIES.clone())) .unwrap_or_else(|_| std::process::abort()); + r.register(Box::new(PEER_SIGNALS_REFUSED.clone())) + .unwrap_or_else(|_| std::process::abort()); + r.register(Box::new(PROVIDER_SCRAPES.clone())) + .unwrap_or_else(|_| std::process::abort()); + r.register(Box::new(PROVIDER_LAST_SCRAPE_SUCCESS.clone())) + .unwrap_or_else(|_| std::process::abort()); r.register(Box::new(PEER_POLL_DURATION.clone())) .unwrap_or_else(|_| std::process::abort()); r.register(Box::new(PEER_POLL_SLOW.clone())) @@ -124,6 +130,37 @@ static PEER_POLL_TOTAL: LazyLock = LazyLock::new(|| { .unwrap_or_else(|_| std::process::abort()) }); +/// Provider metrics scrapes by provider and result: `success`, `no_series`, or a failure class +/// (`timeout`, `unauthorized`, `tls`, `dns`, `connect`, `http`, `body_cap`, `parse`, `config`). +static PROVIDER_SCRAPES: LazyLock = LazyLock::new(|| { + IntCounterVec::new( + Opts::new("grid_provider_scrape_total", "Provider metrics scrapes by result"), + &["grid_provider", "result"], + ) + .unwrap_or_else(|_| std::process::abort()) +}); + +/// When each provider's metrics last scraped with its ready-endpoint series, Unix seconds. +static PROVIDER_LAST_SCRAPE_SUCCESS: LazyLock = LazyLock::new(|| { + IntGaugeVec::new( + Opts::new( + "grid_provider_last_scrape_success_timestamp_seconds", + "Unix time of the provider's last scrape with its ready-endpoint series", + ), + &["grid_provider"], + ) + .unwrap_or_else(|_| std::process::abort()) +}); + +/// Peer observations refused at ingest, by peer and reason. +static PEER_SIGNALS_REFUSED: LazyLock = LazyLock::new(|| { + IntCounterVec::new( + Opts::new("grid_peer_signals_refused_total", "Peer observations refused at ingest"), + &["peer", "reason"], + ) + .unwrap_or_else(|_| std::process::abort()) +}); + /// Retried attempts by peer and the outcome that prompted the retry. /// /// Separate from the poll counter because a poll that succeeded on its third @@ -426,6 +463,44 @@ pub(crate) fn record_peer_poll(peer: &str, outcome: &str, duration: Duration, by } } +/// Count one scrape of `provider` with `result`. +pub(crate) fn record_provider_scrape(provider: &str, result: &str) { + PROVIDER_SCRAPES.with_label_values(&[provider, result]).inc(); +} + +/// Record that `provider` last scraped with its ready-endpoint series at `at`. +pub(crate) fn set_provider_last_scrape_success(provider: &str, at: std::time::SystemTime) { + let secs = at.duration_since(std::time::UNIX_EPOCH).unwrap_or_default().as_secs(); + PROVIDER_LAST_SCRAPE_SUCCESS + .with_label_values(&[provider]) + .set(secs.try_into().unwrap_or(i64::MAX)); +} + +/// Drop `provider`'s scrape series once it is gone, so they do not outlive it. +pub(crate) fn forget_provider_scrapes(provider: &str) { + let _absent = PROVIDER_LAST_SCRAPE_SUCCESS.remove_label_values(&[provider]); + for result in [ + "success", + "no_series", + "timeout", + "unauthorized", + "tls", + "dns", + "connect", + "http", + "body_cap", + "parse", + "config", + ] { + let _absent_result = PROVIDER_SCRAPES.remove_label_values(&[provider, result]); + } +} + +/// Count a peer observation this hub refused: `name`, `provider`, `value`, or `provider_cap`. +pub(crate) fn record_peer_signal_refused(peer: &str, reason: &str) { + PEER_SIGNALS_REFUSED.with_label_values(&[peer, reason]).inc(); +} + /// Record an attempt that failed and will be tried again. pub(crate) fn record_peer_retry(peer: &str, reason: &str) { PEER_POLL_RETRIES.with_label_values(&[peer, reason]).inc(); @@ -475,6 +550,111 @@ pub fn record_site_identity_renewal(result: &str) { SITE_IDENTITY_RENEWALS.with_label_values(&[result]).inc(); } +/// Provider series exported on `/metrics`, as each site's operator resolves them. +const PROVIDER_SIGNALS: [(&str, &str); 7] = [ + ("grid_provider_ready", "1 when the provider can serve, 0 when not."), + ( + "grid_provider_ready_endpoints", + "Endpoints behind the provider answering with fresh metrics.", + ), + ( + "grid_provider_in_flight_requests", + "Requests the provider holds, running, engine-queued, and held by flow control.", + ), + ( + "grid_provider_ttft_p50_seconds", + "Median streaming time to first token over the last 30s.", + ), + ( + "grid_provider_ttft_p90_seconds", + "90th percentile streaming time to first token over the last 30s.", + ), + ( + "grid_provider_tpot_seconds", + "Mean streaming time per output token over the last 30s.", + ), + ( + "grid_provider_error_ratio", + "Failed requests over all requests in the last 30s.", + ), +]; + +/// Labels on each exported provider series: both bounded by the grid's sites and providers. +const PROVIDER_LABELS: [&str; 2] = ["grid_site", "grid_provider"]; + +/// Exports the provider series held in `stores` at each scrape, this site's own and those +/// polled from peers. A series this site does not hold is absent, not 0. +struct ProviderSignals { + /// The local and peer signal stores. + stores: Vec, + /// One gauge per exported series, paired with its metric name so a failed constructor + /// drops only its own series. + gauges: Vec<(&'static str, prometheus::GaugeVec)>, + /// Serializes a scrape's reset and refill of the shared gauges. + collecting: std::sync::Mutex<()>, +} + +impl prometheus::core::Collector for ProviderSignals { + fn desc(&self) -> Vec<&prometheus::core::Desc> { + self.gauges.iter().flat_map(|(_, gauge)| gauge.desc()).collect() + } + + fn collect(&self) -> Vec { + let _collecting = self + .collecting + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + for (_, gauge) in &self.gauges { + gauge.reset(); + } + let mut seen = std::collections::BTreeSet::new(); + for sample in self.stores.iter().flat_map(crate::signals::SignalStore::current) { + let (Some(site), Some(provider)) = (sample.labels.get("grid_site"), sample.labels.get("grid_provider")) + else { + continue; + }; + let Some((_, gauge)) = self.gauges.iter().find(|(name, _)| *name == sample.metric) else { + continue; + }; + // The first store holding a series wins: this site's own before a peer's. + if seen.insert((sample.metric.clone(), site.clone(), provider.clone())) { + gauge + .with_label_values(&[site.as_str(), provider.as_str()]) + .set(sample.value); + } + } + self.gauges + .iter() + .flat_map(|(_, gauge)| prometheus::core::Collector::collect(gauge)) + .collect() + } +} + +/// Export the provider series held in `stores` on `/metrics`. Call once, with this site's +/// store first. +pub fn register_provider_signals(stores: Vec) { + if let Err(error) = REGISTRY.register(Box::new(provider_signals(stores))) { + tracing::warn!(%error, "metrics: provider signals already registered"); + } +} + +/// The collector over `stores`. +fn provider_signals(stores: Vec) -> ProviderSignals { + let gauges = PROVIDER_SIGNALS + .iter() + .filter_map(|(name, help)| { + prometheus::GaugeVec::new(Opts::new(*name, *help), &PROVIDER_LABELS) + .ok() + .map(|gauge| (*name, gauge)) + }) + .collect(); + ProviderSignals { + stores, + gauges, + collecting: std::sync::Mutex::new(()), + } +} + /// Gather all registered metrics for serialization. pub(crate) fn gather_metrics() -> Vec { expire_site_phases(); @@ -644,4 +824,135 @@ mod tests { "output should contain duration histogram" ); } + + #[test] + fn provider_scrapes_are_counted_by_result_and_forgotten_with_the_provider() { + let provider = "scrape-count-test"; + record_provider_scrape(provider, "success"); + record_provider_scrape(provider, "no_series"); + record_provider_scrape(provider, "no_series"); + set_provider_last_scrape_success(provider, std::time::UNIX_EPOCH + Duration::from_secs(42)); + assert_eq!(PROVIDER_SCRAPES.with_label_values(&[provider, "success"]).get(), 1); + assert_eq!(PROVIDER_SCRAPES.with_label_values(&[provider, "no_series"]).get(), 2); + assert_eq!(PROVIDER_LAST_SCRAPE_SUCCESS.with_label_values(&[provider]).get(), 42); + forget_provider_scrapes(provider); + assert!( + PROVIDER_SCRAPES.remove_label_values(&[provider, "success"]).is_err(), + "forgotten with the provider" + ); + assert!(PROVIDER_LAST_SCRAPE_SUCCESS.remove_label_values(&[provider]).is_err()); + } + + #[test] + #[expect(clippy::too_many_lines, reason = "one store across two scrapes")] + fn provider_signals_drop_a_series_the_store_no_longer_holds() { + use prometheus::core::Collector as _; + let ready = crate::signals::Observation { + metric: "grid_provider_ready".to_owned(), + labels: [("grid_site", "hq"), ("grid_provider", "pool")] + .iter() + .map(|(k, v)| ((*k).to_owned(), (*v).to_owned())) + .collect(), + value: 1.0, + timestamp_ms: None, + }; + let store = crate::signals::SignalStore::new(); + let collector = provider_signals(vec![store.clone()]); + let count = |families: &[MetricFamily]| -> usize { + families + .iter() + .filter(|family| family.name() == "grid_provider_ready") + .map(|family| family.get_metric().len()) + .sum() + }; + store.refresh( + std::collections::BTreeMap::from([("pool".to_owned(), vec![ready])]), + Duration::from_secs(60), + ); + assert_eq!(count(&collector.collect()), 1, "a held series is exported"); + store.refresh( + std::collections::BTreeMap::from([("pool".to_owned(), Vec::new())]), + Duration::from_secs(60), + ); + assert_eq!( + count(&collector.collect()), + 0, + "a dropped series is not exported from the last scrape" + ); + assert_eq!( + collector.desc().len(), + PROVIDER_SIGNALS.len(), + "one desc per exported series" + ); + } + + #[test] + #[expect(clippy::too_many_lines, reason = "one scenario across both stores")] + fn provider_signals_export_this_site_and_its_peers_and_omit_what_is_not_held() { + use prometheus::core::Collector as _; + let held = |site: &str, metric: &str, value: f64| crate::signals::Observation { + metric: metric.to_owned(), + labels: [("grid_site", site), ("grid_provider", "pool")] + .iter() + .map(|(k, v)| ((*k).to_owned(), (*v).to_owned())) + .collect(), + value, + timestamp_ms: None, + }; + let local = crate::signals::SignalStore::new(); + local.refresh( + std::collections::BTreeMap::from([( + "pool".to_owned(), + vec![ + held("hq", "grid_provider_in_flight_requests", 0.5), + held("hq", "grid_provider_ttft_p50_seconds", 0.2), + held("hq", "llm_d_epp_average_queue_size", 3.0), + ], + )]), + Duration::from_secs(60), + ); + let peers = crate::signals::SignalStore::new(); + peers.refresh( + std::collections::BTreeMap::from([( + "retail".to_owned(), + vec![ + held("retail", "grid_provider_in_flight_requests", 0.9), + held("hq", "grid_provider_in_flight_requests", 7.0), + ], + )]), + Duration::from_secs(60), + ); + let families = provider_signals(vec![local, peers]).collect(); + let series = |name: &str| -> Vec<(String, f64)> { + families + .iter() + .filter(|family| family.name() == name) + .flat_map(|family| family.get_metric().iter()) + .map(|metric| { + let site = metric + .get_label() + .iter() + .find(|label| label.name() == "grid_site") + .map(|label| label.value().to_owned()) + .unwrap_or_default(); + (site, metric.get_gauge().value()) + }) + .collect() + }; + let mut in_flight = series("grid_provider_in_flight_requests"); + in_flight.sort_by(|a, b| a.0.cmp(&b.0)); + assert_eq!( + in_flight, + [("hq".to_owned(), 0.5), ("retail".to_owned(), 0.9)], + "this site's value wins" + ); + assert_eq!(series("grid_provider_ttft_p50_seconds"), [("hq".to_owned(), 0.2)]); + assert!(series("grid_provider_tpot_seconds").is_empty(), "not held, not 0"); + assert!( + families + .iter() + .all(|family| family.name() != "llm_d_epp_average_queue_size"), + "only provider series" + ); + } } diff --git a/operator/src/signals.rs b/operator/src/signals.rs index 63ffc569f..a81a706c8 100644 --- a/operator/src/signals.rs +++ b/operator/src/signals.rs @@ -61,12 +61,38 @@ pub struct Observation { pub timestamp_ms: Option, } +/// The label the EPP puts the pool on. +pub(crate) const POOL_LABEL: &str = "inference_pool"; + +/// Whether `o` belongs to `pool`. An unlabeled series never does. +/// +/// Strict on purpose: on an EPP serving more than one pool, a series carrying no +/// `inference_pool` label is the total across pools. Counting it would overstate one +/// pool's readiness and, by inflating the wait subtracted from TTFT, understate its +/// prefill. Both errors flatter the provider, so the unlabeled series is left out and +/// the caller sees the series as absent. +pub(crate) fn in_pool(o: &Observation, pool: Option<&str>) -> bool { + pool.is_none_or(|pool| o.labels.get(POOL_LABEL).is_some_and(|p| p == pool)) +} + /// Parse an exposition response into observations. /// /// An unparseable response yields nothing rather than an error: it is a copy of /// somebody else's scrape and the caller cannot repair it. #[must_use] pub fn parse(text: &str) -> Vec { + parse_scrape(text) + .into_iter() + .filter_map(|(o, republishable)| republishable.then_some(o)) + .collect() +} + +/// Parse a scrape this site made itself: every finite sample, each paired with whether +/// [`parse`] would keep it for republishing. +/// +/// Counters and histogram parts stay, for this site's own latency and error windows. +#[must_use] +pub(crate) fn parse_scrape(text: &str) -> Vec<(Observation, bool)> { // Types first: a declaration may follow its samples, and reading in one // pass would admit a counter that had not been typed yet. let types: HashMap<&str, &str> = text @@ -81,13 +107,14 @@ pub fn parse(text: &str) -> Vec { text.lines() .filter(|l| !l.trim_start().starts_with('#')) .filter_map(parse_sample) - .filter(|o| { + .filter(|o| o.value.is_finite()) + .map(|o| { // Gauges and untyped only: a relayed counter reports our restarts, // and an aggregate we did not observe cannot be recombined. The name // check catches aggregates that arrive untyped. - matches!(types.get(o.metric.as_str()), None | Some(&("gauge" | "untyped"))) - && !is_aggregate_part(&o.metric) - && o.value.is_finite() + let republishable = matches!(types.get(o.metric.as_str()), None | Some(&("gauge" | "untyped"))) + && !is_aggregate_part(&o.metric); + (o, republishable) }) .collect() } @@ -479,6 +506,20 @@ impl SignalStore { (out, oldest) } + /// Every sample still served, across targets. + #[must_use] + pub fn current(&self) -> Vec { + let Ok(guard) = self.inner.read() else { + return Vec::new(); + }; + let now = Instant::now(); + guard + .values() + .filter(|held| held.expires_at > now) + .flat_map(|held| held.samples.iter().cloned()) + .collect() + } + /// Targets currently held and unexpired. #[must_use] pub fn targets(&self) -> Vec { @@ -815,6 +856,7 @@ fn classify(error: &MetricsScrapeError) -> PollOutcome { } }, MetricsScrapeError::Encoding(_) => PollOutcome::Encoding, + MetricsScrapeError::BodyTooLarge(_) => PollOutcome::Transport, MetricsScrapeError::InvalidUrl(_) | MetricsScrapeError::HttpWithTls(_) | MetricsScrapeError::TlsMaterial(_) @@ -1105,7 +1147,7 @@ impl PollPeers { .map(|(peer, url, pins)| async move { let (body, date) = self.poll_one(&peer, &url, &pins).await?; let now_ms = wall_millis(SystemTime::now(), Duration::ZERO); - let mut observations = retain_origin(parse(&body), &peer); + let mut observations = bound_peer(retain_origin(parse(&body), &peer), &peer); reexpress_peer_ages(&mut observations, date, now_ms); Some((peer, observations)) }); @@ -1207,6 +1249,84 @@ fn with_query(url: &str, query: &str) -> Option { http::Uri::from_parts(parts).ok().map(|uri| uri.to_string()) } +/// Names a hub keeps from a peer: the cross-site contract, plus the EPP pool averages and +/// ready counts the gateway routes on until it routes on saturation. A peer's custom +/// `signalNames` are dropped here. +const PEER_SIGNAL_NAMES: [&str; 14] = [ + crate::readiness::READY_SIGNAL, + crate::readiness::READY_ENDPOINTS_SIGNAL, + crate::readiness::IN_FLIGHT_SIGNAL, + crate::latency::TTFT_P50_SIGNAL, + crate::latency::TTFT_P90_SIGNAL, + crate::latency::TPOT_SIGNAL, + crate::latency::ERROR_RATIO_SIGNAL, + "inference_pool_average_queue_size", + "llm_d_epp_average_queue_size", + "inference_pool_average_running_requests", + "llm_d_epp_average_running_requests", + "inference_pool_average_kv_cache_utilization", + "llm_d_epp_average_kv_cache_utilization", + "llm_d_epp_ready_endpoints", +]; + +/// Most providers a hub keeps from one peer, so a peer cannot grow the hub's series without +/// bound. The first in name order are kept. +pub const MAX_PEER_PROVIDERS: usize = 64; + +/// Signals that are shares, which a peer may not report above one. +const UNIT_SIGNALS: [&str; 2] = [crate::readiness::READY_SIGNAL, crate::latency::ERROR_RATIO_SIGNAL]; + +/// A value a hub accepts: finite and non-negative, and at most one for a share. +fn plausible(observation: &Observation) -> bool { + observation.value.is_finite() + && observation.value >= 0.0 + && (observation.value <= 1.0 || !UNIT_SIGNALS.contains(&observation.metric.as_str())) +} + +/// Why a hub refuses `observation` from a peer, `None` when it accepts it. +fn refusal(observation: &Observation) -> Option<&'static str> { + if !PEER_SIGNAL_NAMES.contains(&observation.metric.as_str()) { + Some("name") + } else if observation + .labels + .get(PROVIDER_LABEL) + .is_none_or(|provider| certs::validate_site_name(provider).is_err()) + { + Some("provider") + } else if !plausible(observation) { + Some("value") + } else { + None + } +} + +/// Keep what a hub accepts from `peer`: an allowed name, a `grid_provider` that is a DNS-1123 +/// label, a plausible value, and at most [`MAX_PEER_PROVIDERS`] providers. Each refusal is +/// counted by reason. +fn bound_peer(observations: Vec, peer: &str) -> Vec { + let mut kept = Vec::with_capacity(observations.len()); + for observation in observations { + match refusal(&observation) { + Some(reason) => crate::metrics::record_peer_signal_refused(peer, reason), + None => kept.push(observation), + } + } + let providers: std::collections::BTreeSet<&str> = kept + .iter() + .filter_map(|o| o.labels.get(PROVIDER_LABEL).map(String::as_str)) + .collect(); + let Some(&last) = providers.iter().nth(MAX_PEER_PROVIDERS.saturating_sub(1)) else { + return kept; + }; + let last = last.to_owned(); + let before = kept.len(); + kept.retain(|o| o.labels.get(PROVIDER_LABEL).is_some_and(|p| *p <= last)); + for _ in kept.len()..before { + crate::metrics::record_peer_signal_refused(peer, "provider_cap"); + } + kept +} + /// Keep only the observations a peer made itself. /// /// A relayed copy is dropped rather than trusted, so every site's data reaches @@ -1258,6 +1378,111 @@ fn reexpress_peer_ages(observations: &mut [Observation], date: Option Observation { + Observation { + metric: metric.to_owned(), + labels: BTreeMap::from([ + (SITE_LABEL.to_owned(), "retail".to_owned()), + (PROVIDER_LABEL.to_owned(), provider.to_owned()), + ]), + value, + timestamp_ms: None, + } + } + + #[test] + #[expect(clippy::too_many_lines, reason = "one table of accepted and refused samples")] + fn a_hub_refuses_peer_names_providers_and_values_outside_the_contract() { + let cases = [ + ( + "contract name", + peer_sample(crate::readiness::IN_FLIGHT_SIGNAL, "pool", 0.5), + true, + ), + ( + "gateway load name", + peer_sample("inference_pool_average_queue_size", "pool", 3.0), + true, + ), + ("custom name", peer_sample("my_custom_queue", "pool", 3.0), false), + ( + "provider not a label", + peer_sample(crate::readiness::READY_SIGNAL, "Pool_1", 1.0), + false, + ), + ( + "not finite", + peer_sample(crate::readiness::IN_FLIGHT_SIGNAL, "pool", f64::NAN), + false, + ), + ( + "infinite", + peer_sample(crate::readiness::IN_FLIGHT_SIGNAL, "pool", f64::INFINITY), + false, + ), + ( + "negative", + peer_sample(crate::readiness::IN_FLIGHT_SIGNAL, "pool", -1.0), + false, + ), + ( + "share above one", + peer_sample(crate::latency::ERROR_RATIO_SIGNAL, "pool", 1.5), + false, + ), + ( + "saturation above one", + peer_sample(crate::readiness::IN_FLIGHT_SIGNAL, "pool", 1.5), + true, + ), + ]; + for (label, observation, kept) in cases { + assert_eq!( + bound_peer(vec![observation], "retail").len(), + usize::from(kept), + "{label}" + ); + } + } + + #[test] + fn a_hub_keeps_at_most_the_first_providers_of_a_peer_in_name_order() { + let observations: Vec = (0..MAX_PEER_PROVIDERS + 5) + .map(|i| peer_sample(crate::readiness::READY_SIGNAL, &format!("p{i:03}"), 1.0)) + .collect(); + let kept = bound_peer(observations, "retail"); + assert_eq!(kept.len(), MAX_PEER_PROVIDERS); + assert!( + kept.iter() + .all(|o| o.labels.get(PROVIDER_LABEL).is_some_and(|p| p.as_str() < "p064")), + "the first {MAX_PEER_PROVIDERS} in name order are kept" + ); + } + #[test] fn peers_are_dialed_at_a_dialable_signals_endpoint() { let member = |site: &str, endpoint: &str, signals: Option<&str>| MemberRecord { @@ -1302,7 +1527,7 @@ mod tests { let mut request = [0_u8; 1024]; drop(stream.read(&mut request).await); drop(release.await); - let body = format!("load{{grid_site=\"{site}\"}} 1\n"); + let body = format!("inference_pool_average_queue_size{{grid_site=\"{site}\",grid_provider=\"pool\"}} 1\n"); let response = format!("HTTP/1.1 200 OK\r\nContent-Length: {}\r\n\r\n{body}", body.len()); drop(stream.write_all(response.as_bytes()).await); }); From b63b4a61fa3213016ec114a3435b5b34aff0a03f Mon Sep 17 00:00:00 2001 From: Sam Batschelet Date: Wed, 7 Oct 2026 12:26:34 -0400 Subject: [PATCH 05/10] fix(operator): place providers at their own site, refuse bad serving sites, and keep the always-write serving config The serving config carries each candidate's admission from the readiness verdict. Providers are placed at their own site, a serving site that fails validation is refused, the candidate dedup is folded, and a model declared twice is refused. Signed-off-by: Sam Batschelet --- .../testdata/serving-config.json | 2 +- operator/src/controller/grid_network.rs | 336 +++++++++++++++++- operator/src/resources/consumer_config.rs | 2 + operator/src/resources/overlay_bridge.rs | 1 + operator/src/resources/overlay_envelope.rs | 2 + operator/src/resources/routing_overlay.rs | 175 +++++++-- operator/src/resources/serving_config.rs | 257 ++++++++++++-- 7 files changed, 704 insertions(+), 71 deletions(-) diff --git a/gateway/ai-grid-filters/testdata/serving-config.json b/gateway/ai-grid-filters/testdata/serving-config.json index 570b01ab2..40ae0a517 100644 --- a/gateway/ai-grid-filters/testdata/serving-config.json +++ b/gateway/ai-grid-filters/testdata/serving-config.json @@ -1,7 +1,7 @@ { "local_site": "site-a", "window_secs": 60, - "load_window_ms": 30000, + "load_window_ms": 17000, "candidates": [ { "kind": "inference_model", diff --git a/operator/src/controller/grid_network.rs b/operator/src/controller/grid_network.rs index afd9ed49f..9168e0410 100644 --- a/operator/src/controller/grid_network.rs +++ b/operator/src/controller/grid_network.rs @@ -38,6 +38,7 @@ use crate::{ inference_provider::InferenceProvider, }, error::OperatorError, + readiness, resources::{ consumer_config::{self, ConsumerConfigError}, overlay_envelope, provider_admission, provider_metrics, routing_overlay, secret, @@ -102,6 +103,9 @@ pub struct OperatorCtx { /// after async DNS resolution and the SWIM channel announcement. pub(crate) last_seeds: std::sync::Mutex>>, + /// Sites each `GridNetwork`'s serving config last refused, for warning on change. + pub(crate) refused_sites: serving_config::RefusedSites, + /// Who may read the signals endpoint, keyed by presented-cert fingerprint. /// Set by reconcile from each `GridSite`'s trust pins. pub(crate) peer_identities: signals::PeerIdentities, @@ -116,6 +120,12 @@ pub struct OperatorCtx { /// Served-model sets discovered from this site's providers. pub(crate) served_models: served_models::ServedModelStore, + /// Each provider's latest scrape, from which its readiness is resolved. + pub(crate) readiness: readiness::ReadinessStore, + + /// How often the signals loop scrapes providers; a readiness window is at least two. + scrape_interval: Duration, + /// Signal transport resolved once at startup: gossip or poll. /// /// Under poll the operator stops carrying metrics in gossip and scoring the @@ -275,10 +285,13 @@ impl OperatorCtx { metrics_cache: Mutex::new(provider_metrics::MetricsCache::new()), admission_memory: Mutex::new(provider_admission::AdmissionMemory::default()), last_seeds: std::sync::Mutex::new(HashMap::new()), + refused_sites: std::sync::Mutex::new(HashMap::new()), peer_identities: signals::PeerIdentities::new(), peers: signals::SignalStore::new(), signals: signals::SignalStore::new(), served_models: served_models::ServedModelStore::new(), + readiness: readiness::ReadinessStore::default(), + scrape_interval: Duration::ZERO, signal_mode, serving_writes: WriteGate::default(), peer_settings: PeerSettings::default(), @@ -361,6 +374,13 @@ impl OperatorCtx { self.site_name.as_deref() } + /// Record how often the signals loop scrapes providers. + #[must_use] + pub const fn with_scrape_interval(mut self, interval: Duration) -> Self { + self.scrape_interval = interval; + self + } + /// Name the certificate the operator issues itself after this site, not the network. #[must_use] pub fn with_site_name(mut self, site_name: Option) -> Self { @@ -423,11 +443,226 @@ pub async fn refresh_signals(ctx: &OperatorCtx, client: &Client, network_name: & let providers = list_all_inference_providers(client).await?; ctx.signals.set_access(signal_access(&providers)); Box::pin(register_peers(ctx, client)).await?; - let collected = provider_metrics::collect_provider_signals(network_name, &providers, Some(client)).await; + Box::pin(provider_metrics::collect_provider_signals( + network_name, + &providers, + Some(client), + &ctx.readiness, + )) + .await; + let collected = Box::pin(resolve_readiness(ctx, client, &providers, network_name)).await; publish_signals(ctx, collected); Ok(()) } +/// Resolve each provider's readiness, write its `Ready` condition, and return what to publish. +async fn resolve_readiness( + ctx: &OperatorCtx, + client: &Client, + providers: &[InferenceProvider], + network_name: &str, +) -> HashMap> { + ctx.readiness.retain( + &providers + .iter() + .filter_map(|p| { + provider_metrics::signal_scrape_plan(p) + .map(|plan| readiness::key(&p.spec.grid_network_ref, plan.identity)) + }) + .collect(), + ); + let now = Instant::now(); + let mut collected = HashMap::new(); + for provider in providers.iter().filter(|p| p.spec.grid_network_ref == network_name) { + let Some(entry) = readiness_entry(ctx, provider, now) else { + continue; + }; + if let Err(error) = Box::pin(apply_ready_condition(client, provider, &entry.verdict)).await { + tracing::warn!(provider = entry.identity, %error, "provider Ready condition not written"); + } + if let Some(published) = entry.published { + collected.insert(entry.identity.to_owned(), published); + } + } + collected +} + +/// One provider's readiness and the signals to publish for it. +struct ReadinessEntry<'provider> { + /// Routing identity. + identity: &'provider str, + /// The verdict. + verdict: readiness::Verdict, + /// Signals to publish, `None` for a provider without metrics: absent reads as ready. + published: Option>, +} + +/// A provider's readiness entry, `None` for no identity or not yet judged. +/// +/// A provider not scraped for readiness, with no metrics config or no usable +/// endpoint, is `Unknown` and publishes nothing: absent reads as ready. +fn readiness_entry<'provider>( + ctx: &OperatorCtx, + provider: &'provider InferenceProvider, + now: Instant, +) -> Option> { + let identity = routing_overlay::routing_identity(provider)?; + let key = readiness::key(&provider.spec.grid_network_ref, identity); + if provider_metrics::signal_scrape_plan(provider).is_none() { + return Some(ReadinessEntry { + identity, + verdict: readiness::Verdict { + reason: readiness::Reason::MetricsNotConfigured, + message: "no metricsConfig endpoint to read readiness from".to_owned(), + }, + published: None, + }); + } + let verdict = provider_readiness(ctx, provider, now)?; + let ready = ready_sample(&verdict); + let endpoints = ctx.readiness.ready_endpoints(&key).map(|count| signals::Observation { + metric: readiness::READY_ENDPOINTS_SIGNAL.to_owned(), + labels: std::collections::BTreeMap::new(), + value: count, + timestamp_ms: None, + }); + // A fresh scrape republishes with the verdict. Without one, the last published + // entry ages on its own, unless the verdict turned not ready. + let fresh = ctx.readiness.take_fresh(&key); + Some(ReadinessEntry { + identity, + published: published_signals(fresh, ready, endpoints, verdict.reason.excludes()), + verdict, + }) +} + +/// What to publish for a scraped provider: a fresh scrape with the verdict, or the +/// verdict alone when it is not ready. Without either, the last entry ages on its own. +fn published_signals( + fresh: Option>, + ready: signals::Observation, + endpoints: Option, + excluded: bool, +) -> Option> { + match fresh { + Some(mut held) => { + held.push(ready); + held.extend(endpoints); + Some(held) + }, + None if excluded => Some(std::iter::once(ready).chain(endpoints).collect()), + None => None, + } +} + + +/// The `grid_provider_ready` sample for `verdict`: 1 when it serves, 0 when not. +fn ready_sample(verdict: &readiness::Verdict) -> signals::Observation { + signals::Observation { + metric: readiness::READY_SIGNAL.to_owned(), + labels: std::collections::BTreeMap::new(), + value: if verdict.reason.excludes() { 0.0 } else { 1.0 }, + timestamp_ms: None, + } +} + +/// How long a provider's last good scrape stands. +/// +/// `staleMetricsSeconds`, else half the signal TTL, and never under two scrape +/// intervals, so a provider is not judged stale between successful scrapes. +fn readiness_stale_after(ctx: &OperatorCtx, provider: &InferenceProvider) -> Duration { + provider + .spec + .metrics_config + .as_ref() + .and_then(|mc| mc.stale_metrics_seconds) + .filter(|secs| *secs > 0) + .map_or_else(|| site_signals_ttl() / 2, |secs| Duration::from_secs(secs.into())) + .max(ctx.scrape_interval.saturating_mul(2)) +} + +/// The provider's readiness verdict, `None` when it is not scraped for readiness or not yet judged. +pub(crate) fn provider_readiness( + ctx: &OperatorCtx, + provider: &InferenceProvider, + now: Instant, +) -> Option { + let plan = provider_metrics::signal_scrape_plan(provider)?; + let unavailable = provider + .status + .as_ref() + .is_some_and(|status| status.phase == crate::crd::inference_provider::ProviderPhase::Unavailable); + ctx.readiness.verdict( + &readiness::key(&provider.spec.grid_network_ref, plan.identity), + unavailable, + readiness_stale_after(ctx, provider), + now, + ) +} + +/// Write the provider's `Ready` condition when its status or reason changed. +async fn apply_ready_condition( + client: &Client, + provider: &InferenceProvider, + verdict: &readiness::Verdict, +) -> Result<(), OperatorError> { + let Some((name, patch)) = ready_condition_patch(provider, verdict) else { + return Ok(()); + }; + let api: Api = Api::all(client.clone()); + Box::pin(api.patch_status( + name, + &PatchParams::apply(READINESS_FIELD_MANAGER).force(), + &Patch::Apply(patch), + )) + .await?; + // A turn away from Ready warns; a return to Ready or a wait is informational. + if verdict.reason.excludes() { + tracing::warn!( + provider = name, + status = verdict.reason.status(), + reason = verdict.reason.as_str(), + message = %verdict.message, + "provider readiness changed" + ); + } else { + tracing::info!( + provider = name, + status = verdict.reason.status(), + reason = verdict.reason.as_str(), + message = %verdict.message, + "provider readiness changed" + ); + } + Ok(()) +} + +/// The status patch carrying the provider's new `Ready` condition, `None` when unchanged. +fn ready_condition_patch<'provider>( + provider: &'provider InferenceProvider, + verdict: &readiness::Verdict, +) -> Option<(&'provider str, serde_json::Value)> { + let name = provider.metadata.name.as_deref()?; + let status = provider.status.as_ref(); + let current = status.map_or(&[][..], |status| status.conditions.as_slice()); + let now = time::OffsetDateTime::now_utc() + .format(&time::format_description::well_known::Rfc3339) + .ok()?; + let condition = readiness::ready_condition(current, verdict, &now, provider.metadata.generation)?; + Some(( + name, + serde_json::json!({ + "apiVersion": "grid.praxis.fast/v1alpha1", + "kind": "InferenceProvider", + "metadata": { "name": name }, + "status": { "conditions": [condition] } + }), + )) +} + +/// Server-side apply manager for the `Ready` condition, apart from provider reconciliation. +const READINESS_FIELD_MANAGER: &str = "grid-operator-readiness"; + /// Poll this site's providers once and hold the models they serve. /// /// Called from its own loop rather than from reconcile, like @@ -1092,6 +1327,13 @@ pub async fn reconcile(network: Arc, ctx: Arc) -> Resu memory.evaluate(&memory_key, observation, admission_policy, now), provider.spec.traffic_policy.as_ref().is_some_and(|p| p.drain), ); + // Readiness gates admission whatever the scoring strategy, NoMetrics included. + let not_ready = provider_readiness(&ctx, provider, now).is_some_and(|verdict| verdict.reason.excludes()); + let state = if not_ready { + crate::resources::geography::AdmissionState::Excluded + } else { + state + }; admission_keys.push(memory_key); admission_states.insert(identity, state); } @@ -1342,6 +1584,11 @@ fn cert_broadcast_revision() -> u64 { /// Error policy for the [`GridNetwork`] controller. pub fn error_policy(_network: Arc, error: &OperatorError, _ctx: Arc) -> Action { + if error.is_conflict() { + // Another writer won the race. Re-read and reapply rather than wait out the backoff. + tracing::debug!(%error, "GridNetwork moved underneath the write; reapplying"); + return Action::requeue(Duration::from_secs(1)); + } tracing::error!(%error, "GridNetwork reconciliation failed"); Action::requeue(Duration::from_secs(30)) } @@ -1959,6 +2206,10 @@ struct ServingSource<'src> { gate: &'src WriteGate, /// Peer addressing resolved at startup. settings: &'src PeerSettings, + /// How often this operator scrapes its providers. + scrape_interval: Duration, + /// Sites each network's serving config last refused. + refused: &'src serving_config::RefusedSites, } /// Build the serving source from membership, `None` outside poll mode. @@ -1990,6 +2241,8 @@ fn serving_source<'src>( pins, gate: &ctx.serving_writes, settings: &ctx.peer_settings, + scrape_interval: ctx.scrape_interval, + refused: &ctx.refused_sites, }) } @@ -2009,6 +2262,7 @@ fn render_serving_text( tls_mount, local_signals_addr: source.settings.local_signals_addr.as_deref(), pins: &source.pins, + scrape_interval: source.scrape_interval, }; let members = source.members.iter().map(|(site, endpoint)| (*site, endpoint.as_str())); serving_config::to_text(&serving_config::render(overlay, members, &inputs)).map_err(OperatorError::Json) @@ -2022,6 +2276,7 @@ async fn apply_serving_config( gw_ref: &GatewayRef, client: &Client, ) -> Result, OperatorError> { + serving_config::warn_refused_sites(overlay, network_name, source.refused); let text = render_serving_text(overlay, source, gw_ref)?; let name = serving_config::configmap_name(network_name, &gw_ref.name); let api: Api = Api::namespaced(client.clone(), &gw_ref.namespace); @@ -4238,6 +4493,84 @@ fn parse_metrics_refresh_interval(value: &str) -> Result InferenceProvider { + serde_json::from_value(serde_json::json!({ + "apiVersion": "grid.praxis.fast/v1alpha1", + "kind": "InferenceProvider", + "metadata": { "name": "qwen3-site-b", "generation": 1 }, + "spec": { + "gridNetworkRef": "grid", + "providerKind": "self_hosted", + "backendKind": "local", + "endpoint": "http://localhost:8000", + "models": [] + }, + "status": status + })) + .expect("provider") + } + + fn ready_condition_json(status: &str, reason: &str) -> serde_json::Value { + serde_json::json!({ + "type": "Ready", "status": status, "reason": reason, "message": "m", + "lastTransitionTime": "2026-10-03T00:00:00Z", "observedGeneration": 1 + }) + } + + #[test] + #[expect( + clippy::expect_used, + clippy::indexing_slicing, + reason = "test assertions on a JSON patch" + )] + fn the_ready_condition_is_patched_only_on_a_change() { + let down = readiness::Verdict { + reason: readiness::Reason::NoEndpointsReady, + message: "0 ready endpoints".to_owned(), + }; + let current = provider_with_status(&serde_json::json!({ + "conditions": [ready_condition_json("False", "NoEndpointsReady")] + })); + assert!( + ready_condition_patch(¤t, &down).is_none(), + "the condition already says so, so there is nothing to write" + ); + let up = readiness::Verdict { + reason: readiness::Reason::Ready, + message: "1 ready endpoints".to_owned(), + }; + let (_, patch) = ready_condition_patch(¤t, &up).expect("a transition is written"); + assert_eq!(patch["status"]["conditions"][0]["status"], "True"); + assert!( + patch["status"].get("state").is_none(), + "the column reads the condition, so status carries no copy of it" + ); + } + fn network_with_modes(spec: &serde_json::Value) -> GridNetwork { serde_json::from_value(serde_json::json!({ "apiVersion": "grid.praxis.fast/v1alpha1", @@ -7147,6 +7480,7 @@ mod tests { network: "net".to_owned(), local_site: "site".to_owned(), candidates: Vec::new(), + excluded: Vec::new(), selection_policy: None, generated_at: Some("2026-07-29T01:00:00Z".to_owned()), }, diff --git a/operator/src/resources/consumer_config.rs b/operator/src/resources/consumer_config.rs index 5cd8e81f6..78f324c81 100644 --- a/operator/src/resources/consumer_config.rs +++ b/operator/src/resources/consumer_config.rs @@ -739,6 +739,7 @@ mod tests { network: "test-net".to_owned(), local_site: "site-a".to_owned(), candidates, + excluded: Vec::new(), selection_policy: None, generated_at: None, } @@ -1390,6 +1391,7 @@ mod tests { network: "n".to_owned(), local_site: String::new(), candidates: vec![], + excluded: Vec::new(), selection_policy: None, generated_at: None, }; diff --git a/operator/src/resources/overlay_bridge.rs b/operator/src/resources/overlay_bridge.rs index c79849c22..d1c1ff1d0 100644 --- a/operator/src/resources/overlay_bridge.rs +++ b/operator/src/resources/overlay_bridge.rs @@ -118,6 +118,7 @@ mod tests { capacity_weight: 1, }) .collect(), + excluded: Vec::new(), selection_policy: None, generated_at: None, } diff --git a/operator/src/resources/overlay_envelope.rs b/operator/src/resources/overlay_envelope.rs index eb4e9acaf..85b78e764 100644 --- a/operator/src/resources/overlay_envelope.rs +++ b/operator/src/resources/overlay_envelope.rs @@ -300,6 +300,7 @@ mod tests { RoutingOverlay { network: "test-net".to_owned(), local_site: "site-a".to_owned(), + excluded: Vec::new(), candidates: vec![RoutingCandidate { kind: "inference_model".to_owned(), name: "model-a".to_owned(), @@ -330,6 +331,7 @@ mod tests { RoutingOverlay { network: "glb-demo".to_owned(), local_site: "east-edge".to_owned(), + excluded: Vec::new(), candidates: vec![ RoutingCandidate { kind: "inference_model".to_owned(), diff --git a/operator/src/resources/routing_overlay.rs b/operator/src/resources/routing_overlay.rs index 7b876b347..c9797e924 100644 --- a/operator/src/resources/routing_overlay.rs +++ b/operator/src/resources/routing_overlay.rs @@ -12,9 +12,9 @@ //! # Phase 1 / OP-01 semantics //! //! - [`GridSite`]s are used to resolve per-provider site membership via `spec.siteSelector.matchLabels`. An empty -//! selector matches all sites in the same [`GridNetwork`]. +//! selector means this site alone, when the network lists it. //! - Each `(model, site)` pair becomes one `RoutingCandidate`. -//! - `candidate.site` = the [`GridSite`] name (resolved via selector). +//! - `candidate.site` = the [`GridSite`]'s site id (resolved via selector), as peers are keyed. //! - `candidate.cluster` = `spec.routingClusterRef` when set, otherwise the [`InferenceProvider`] metadata name. The //! gateway uses this as the upstream cluster reference in its local routing configuration. //! - When no [`GridSite`]s are provided, the routing identity (`spec.routingClusterRef` or provider name) is used as @@ -48,6 +48,7 @@ use k8s_openapi::api::core::v1::ConfigMap; use serde::{Deserialize, Serialize}; use crate::{ + controller::grid_network::peer_site_key, crd::{ agent_tool_provider::AgentToolProvider, auth::{AccessPolicy, AuthStrategy}, @@ -992,6 +993,11 @@ pub struct RoutingOverlay { /// freshness, score, then alphabetical tiebreak. pub candidates: Vec, + /// Candidates excluded from routing, kept for the serving config so the gateway + /// can answer 503 for a known model. Never on the overlay's own wire. + #[serde(skip)] + pub excluded: Vec, + /// Optional explicit local selection policy for Praxis. An absent field is /// intentionally backward-compatible and means deterministic selection. #[serde(skip_serializing_if = "Option::is_none")] @@ -1179,7 +1185,7 @@ fn assign_selection_groups(candidates: &mut [RoutingCandidate], policy: crate::c /// Only [`InferenceProvider`]s whose `spec.gridNetworkRef` matches /// `network.metadata.name` are included. Each provider's /// `spec.siteSelector.matchLabels` is matched against the supplied -/// `sites`; an empty selector matches all sites in the network. +/// `sites`; an empty selector means `local_site` alone. /// /// The `local_site` parameter identifies this gateway's own site. /// Praxis uses it to score candidates running on the local site higher @@ -1388,7 +1394,7 @@ pub fn render_routing_overlay_with_admission( .find(|site| site.metadata.name.as_deref() == Some(local_site) && site.spec.grid_network_ref == network_name) .and_then(|site| site.metadata.labels.as_ref()); - let mut candidates = collect_candidates(network_name, sites, providers, consumer_site_labels)?; + let mut candidates = collect_candidates(network_name, sites, providers, consumer_site_labels, local_site)?; candidates.extend(collect_tool_candidates( network_name, sites, @@ -1427,7 +1433,9 @@ pub fn render_routing_overlay_with_admission( } enrich_candidates(&mut candidates, local_site, sites, network_name, &admission_map); - candidates.retain(|c| c.admission_state != Some(AdmissionState::Excluded)); + let (excluded, mut candidates): (Vec<_>, Vec<_>) = candidates + .into_iter() + .partition(|c| c.admission_state == Some(AdmissionState::Excluded)); retain_valid_candidate_identifiers(&mut candidates, network_name); let policy = network @@ -1506,6 +1514,7 @@ pub fn render_routing_overlay_with_admission( network: network_name.to_owned(), local_site: local_site.to_owned(), candidates, + excluded, selection_policy, generated_at: generated_at.map(str::to_owned), }) @@ -1579,6 +1588,7 @@ fn collect_candidates( sites: &[GridSite], providers: &[InferenceProvider], consumer_site_labels: Option<&BTreeMap>, + local_site: &str, ) -> Result, String> { // Pre-filter sites to those in this network. let network_sites: Vec<&GridSite> = sites @@ -1600,7 +1610,7 @@ fn collect_candidates( continue; } - let resolution = resolve_sites(provider, &network_sites); + let resolution = resolve_sites(provider, &network_sites, local_site); all.extend(candidates_from_provider(provider, &resolution)?); } Ok(all) @@ -1665,12 +1675,25 @@ fn collect_tool_candidates( /// which enables the Phase 1 provider-name fallback. Returns /// [`SiteResolution::Known`] otherwise — with an empty `Vec` if the /// selector matched nothing, which suppresses candidate generation. -fn resolve_sites(provider: &InferenceProvider, network_sites: &[&GridSite]) -> SiteResolution { +/// +/// An empty selector means this site alone, when the inventory lists it, as the +/// provider controller attributes it. Sites are keyed as peers are, so a +/// discovered peer reads as its site id, not its object name. +fn resolve_sites(provider: &InferenceProvider, network_sites: &[&GridSite], local_site: &str) -> SiteResolution { if network_sites.is_empty() { return SiteResolution::Unavailable; } let selector = &provider.spec.site_selector.match_labels; + let key = |site: &&GridSite| peer_site_key(site).map(|(key, _)| key); + if selector.is_empty() { + let listed = network_sites.iter().filter_map(key).any(|site| site == local_site); + return SiteResolution::Known(if listed { + vec![local_site.to_owned()] + } else { + Vec::new() + }); + } let names: Vec = network_sites .iter() @@ -1680,7 +1703,7 @@ fn resolve_sites(provider: &InferenceProvider, network_sites: &[&GridSite]) -> S .iter() .all(|(k, v)| site_labels.is_some_and(|labels| labels.get(k).is_some_and(|sv| sv == v))) }) - .map(|site| site.metadata.name.clone().unwrap_or_else(|| "unknown-site".to_owned())) + .filter_map(key) .collect::>() .into_iter() .collect(); @@ -1716,10 +1739,20 @@ fn candidates_from_provider( .as_deref() .ok_or_else(|| "InferenceProvider has no name".to_owned())?; + // Compared untrimmed, because that is the name the candidate carries: two equal + // names at one provider collapse to a single routing candidate, so the second is + // silently lost rather than served. + let mut seen = BTreeSet::new(); for model in &provider.spec.models { if model.name.trim().is_empty() { return Err(format!("provider {provider_name} has a blank model name")); } + if !seen.insert(model.name.as_str()) { + return Err(format!( + "provider {provider_name} declares model {} more than once", + model.name + )); + } } // Use routing_identity for cluster (and site in Phase 1 fallback). @@ -2216,9 +2249,19 @@ mod tests { .unwrap_or_else(|_| std::process::abort()) } + /// A label every labeled test site carries, so a provider can be placed at all of them. + const PLACEMENT: (&str, &str) = ("test.grid/placement", "all"); + + /// `provider` placed at every labeled test site, as an explicit selector does. + fn placed_everywhere(mut provider: InferenceProvider) -> InferenceProvider { + provider.spec.site_selector.match_labels = BTreeMap::from([(PLACEMENT.0.to_owned(), PLACEMENT.1.to_owned())]); + provider + } + fn test_site_with_labels(name: &str, network: &str, labels: &[(&str, &str)]) -> GridSite { let labels_map: serde_json::Map = labels .iter() + .chain(std::iter::once(&PLACEMENT)) .map(|(k, v)| (k.to_string(), serde_json::Value::String(v.to_string()))) .collect(); serde_json::from_value(serde_json::json!({ @@ -3546,36 +3589,83 @@ mod tests { assert!(result.is_err(), "blank model name must return an error"); } + #[test] + fn a_model_declared_twice_returns_error() { + let provider = test_provider("prov", "net", &["llama", "llama"]); + let result = render_routing_overlay( + &test_network("net"), + &[], + &[provider], + &[], + &[], + "test-site", + None, + None, + &scoring::ScoringWeights::default(), + ); + let message = result.expect_err("a repeated model name must return an error"); + assert!( + message.contains("more than once"), + "the error names the repeat, got: {message}" + ); + } + // ----------------------------------------------------------------------- // Site selector // ----------------------------------------------------------------------- #[test] - fn empty_selector_matches_all_sites_in_network() { - let network = test_network("net"); - let site_a = test_site("site-a", "net"); - let site_b = test_site("site-b", "net"); - let provider = test_provider("prov", "net", &["model"]); + fn an_empty_selector_places_a_provider_at_the_local_site_only() { + let render = |local: &str| { + render_routing_overlay( + &test_network("net"), + &[test_site("site-a", "net"), test_site("site-b", "net")], + &[test_provider("prov", "net", &["model"])], + &[], + &[], + local, + None, + None, + &scoring::ScoringWeights::default(), + ) + .map(|overlay| overlay.candidates.into_iter().map(|c| c.site).collect::>()) + }; + assert!( + matches!(render("site-a"), Ok(sites) if sites == ["site-a"]), + "not every site in the network" + ); + assert!( + matches!(render("absent"), Ok(sites) if sites.is_empty()), + "a local site the inventory does not list hosts nothing" + ); + } + + #[test] + fn a_selector_names_a_discovered_peer_by_its_site_id() { + let mut peer = test_site_with_labels("grid-retail", "net", &[("hw", "gpu")]); + peer.metadata.labels.get_or_insert_with(BTreeMap::new).insert( + crate::controller::grid_network::LABEL_AUTO_DISCOVERED.to_owned(), + "true".to_owned(), + ); + peer.metadata.annotations = Some(BTreeMap::from([( + crate::controller::grid_network::ANNOTATION_SITE_ID.to_owned(), + "retail".to_owned(), + )])); + let provider = test_provider_with_selector("prov", "net", &["model"], &[("hw", "gpu")]); let overlay = render_routing_overlay( - &network, - &[site_a, site_b], + &test_network("net"), + &[peer], &[provider], &[], &[], - "test-site", + "hq", None, None, &scoring::ScoringWeights::default(), ) .unwrap_or_else(|_| std::process::abort()); - assert_eq!( - overlay.candidates.len(), - 2, - "empty selector should produce one candidate per site" - ); let sites: Vec<&str> = overlay.candidates.iter().map(|c| c.site.as_str()).collect(); - assert!(sites.contains(&"site-a"), "site-a must be in candidates"); - assert!(sites.contains(&"site-b"), "site-b must be in candidates"); + assert_eq!(sites, ["retail"], "keyed as peers are, not by object name"); } #[test] @@ -3676,7 +3766,7 @@ mod tests { &[p1, p2], &[], &[], - "test-site", + "site-a", None, None, &scoring::ScoringWeights::default(), @@ -3703,7 +3793,7 @@ mod tests { &[provider], &[], &[], - "test-site", + "site-a", None, None, &scoring::ScoringWeights::default(), @@ -3982,13 +4072,17 @@ mod tests { &[provider], &[], &[], - "test-site", + "site-a", None, None, &scoring::ScoringWeights::default(), ) .unwrap_or_else(|_| std::process::abort()); - assert_eq!(overlay.candidates.len(), 2, "one candidate per matched site"); + assert_eq!( + overlay.candidates.len(), + 1, + "an empty selector places it at the local site" + ); assert!( overlay.candidates.iter().all(|c| !c.fresh), "every (model, site) candidate from a Degraded provider must have fresh=false" @@ -6641,7 +6735,7 @@ mod tests { let network = test_network("net"); let site_prod = test_site_with_labels("site-prod", "net", &[("env", "prod")]); let site_staging = test_site_with_labels("site-staging", "net", &[("env", "staging")]); - let provider = test_provider("unrestricted-prov", "net", &["model-a"]); + let provider = placed_everywhere(test_provider("unrestricted-prov", "net", &["model-a"])); // The provider has an empty siteSelector, so it should generate candidates // for ALL sites in the network when it appears in any overlay. @@ -6696,7 +6790,12 @@ mod tests { let network = test_network("net"); let site_prod = test_site_with_labels("site-prod", "net", &[("env", "prod"), ("region", "us-west")]); let site_staging = test_site_with_labels("site-staging", "net", &[("env", "staging")]); - let provider = test_provider_with_access_policy("prod-only-prov", "net", &["model-a"], &[("env", "prod")]); + let provider = placed_everywhere(test_provider_with_access_policy( + "prod-only-prov", + "net", + &["model-a"], + &[("env", "prod")], + )); // Consumer from prod site should get candidates // Since the provider has an empty siteSelector but restricted access policy, @@ -6822,7 +6921,7 @@ mod tests { let site = test_site_with_labels("known-site", "net", &[("env", "prod")]); let provider_restricted = test_provider_with_access_policy("restricted-prov", "net", &["model-a"], &[("env", "prod")]); - let provider_unrestricted = test_provider("unrestricted-prov", "net", &["model-b"]); + let provider_unrestricted = placed_everywhere(test_provider("unrestricted-prov", "net", &["model-b"])); // Consumer site not in the sites list (unknown identity) let overlay = render_routing_overlay( @@ -6858,15 +6957,19 @@ mod tests { let site_prod = test_site_with_labels("site-prod", "net", &[("env", "prod"), ("team", "platform")]); let site_staging = test_site_with_labels("site-staging", "net", &[("env", "staging"), ("team", "platform")]); - let provider_unrestricted = test_provider("unrestricted-prov", "net", &["model-general"]); - let provider_prod_only = - test_provider_with_access_policy("prod-only-prov", "net", &["model-prod"], &[("env", "prod")]); - let provider_platform_only = test_provider_with_access_policy( + let provider_unrestricted = placed_everywhere(test_provider("unrestricted-prov", "net", &["model-general"])); + let provider_prod_only = placed_everywhere(test_provider_with_access_policy( + "prod-only-prov", + "net", + &["model-prod"], + &[("env", "prod")], + )); + let provider_platform_only = placed_everywhere(test_provider_with_access_policy( "platform-only-prov", "net", &["model-platform"], &[("team", "platform")], - ); + )); // Prod consumer should get all three providers (each provider generates candidates for both sites) let overlay = render_routing_overlay( @@ -7053,7 +7156,7 @@ mod tests { let site = test_site_with_labels("known-site", "net", &[("env", "prod")]); let provider_restricted = test_provider_with_access_policy("local-restricted-prov", "net", &["model-a"], &[("env", "prod")]); - let provider_unrestricted = test_provider("local-unrestricted-prov", "net", &["model-b"]); + let provider_unrestricted = placed_everywhere(test_provider("local-unrestricted-prov", "net", &["model-b"])); // Consumer site not in the sites list (unknown identity) let overlay = render_routing_overlay( diff --git a/operator/src/resources/serving_config.rs b/operator/src/resources/serving_config.rs index 04e07e036..ec6580da6 100644 --- a/operator/src/resources/serving_config.rs +++ b/operator/src/resources/serving_config.rs @@ -35,15 +35,24 @@ pub(crate) const MIN_WRITE_INTERVAL: Duration = Duration::from_secs(30); /// Store retention per series, seconds. const WINDOW_SECS: u64 = 60; -/// Freshness window the gateway orders over, milliseconds. -const LOAD_WINDOW_MS: i64 = 30_000; - /// Peer poll interval, the operator's default local scrape interval. const PEER_INTERVAL_MS: u64 = 5_000; /// Peer connect and request timeout, milliseconds. const PEER_TIMEOUT_MS: u64 = 2_000; +/// The freshness window the gateway reads load over, milliseconds: the oldest a sample can +/// be when it arrives (a scrape, a poll, and a poll timeout) plus one poll of slack, so one +/// late poll does not turn a healthy site unknown. 17s at a 5s scrape. +fn load_window_ms(scrape: Duration) -> i64 { + let scrape_ms = u64::try_from(scrape.as_millis()).unwrap_or(u64::MAX); + let window = PEER_INTERVAL_MS + .saturating_add(scrape_ms) + .saturating_add(PEER_TIMEOUT_MS) + .saturating_add(PEER_INTERVAL_MS); + i64::try_from(window).unwrap_or(i64::MAX) +} + /// Gateway-side cap on candidates (`validate_candidates`). const MAX_CANDIDATES: usize = 1024; @@ -81,6 +90,16 @@ pub(crate) struct ServingCandidate { pub(crate) cluster: String, /// Freshness carried from the overlay. pub(crate) fresh: bool, + /// Whether it takes new requests. Omitted when it does, so such a config + /// still loads on a gateway that predates the field. + #[serde(skip_serializing_if = "admits_new")] + pub(crate) admission: AdmissionState, +} + +/// Whether `admission` is the default, which the gateway assumes when absent. +#[expect(clippy::trivially_copy_pass_by_ref, reason = "serde skip_serializing_if signature")] +fn admits_new(admission: &AdmissionState) -> bool { + *admission == AdmissionState::NewAndExisting } /// One peer signals endpoint and the local identity material to reach it. @@ -122,6 +141,8 @@ pub(crate) struct ServingInputs<'input> { pub(crate) local_signals_addr: Option<&'input str>, /// Declared leaf digests per remote site, empty outside pin trust. pub(crate) pins: &'input BTreeMap>, + /// How often operators scrape their providers, taken as every site's. + pub(crate) scrape_interval: Duration, } /// Render from `(site, signals endpoint)` members. @@ -136,7 +157,7 @@ pub(crate) fn render<'member, Members>( where Members: IntoIterator, { - let candidates = candidates(&overlay.candidates, &overlay.local_site); + let candidates = candidates(overlay); let sites: BTreeSet<&str> = candidates.iter().map(|candidate| candidate.site.as_str()).collect(); let mut addrs = remote_addrs(members, &sites, &overlay.local_site); if let Some(addr) = inputs.local_signals_addr @@ -155,7 +176,7 @@ where ServingConfig { local_site: overlay.local_site.clone(), window_secs: WINDOW_SECS, - load_window_ms: LOAD_WINDOW_MS, + load_window_ms: load_window_ms(inputs.scrape_interval), candidates, peers, } @@ -188,51 +209,101 @@ pub(crate) fn dialable_members<'snap>( .collect() } -/// Admitted inference candidates within gateway limits, deduplicated and ordered. -fn candidates(overlay: &[RoutingCandidate], local_site: &str) -> Vec { - // Local first so cold start, before any signal, prefers this site. - let mut unique: BTreeMap<(bool, &str, &str, &str), bool> = BTreeMap::new(); - for candidate in overlay.iter().filter(|candidate| routable(candidate)) { +/// A candidate's identity: remote after local, then site, name, cluster. +type CandidateKey<'overlay> = (bool, &'overlay str, &'overlay str, &'overlay str); + +/// Each routable candidate once, with its freshness and admission. +/// +/// Any stale or more restricted duplicate marks the tuple, whatever the order. +fn dedup<'overlay>( + overlay: impl Iterator, + local_site: &str, +) -> BTreeMap, (bool, AdmissionState)> { + let mut unique = BTreeMap::new(); + for candidate in overlay.filter(|candidate| routable(candidate)) { let key = ( candidate.site != local_site, candidate.site.as_str(), candidate.name.as_str(), candidate.cluster.as_str(), ); - // Any stale duplicate marks the tuple stale, whatever the order. - unique - .entry(key) - .and_modify(|fresh| *fresh &= candidate.fresh) - .or_insert(candidate.fresh); + let admission = candidate.admission_state.unwrap_or(AdmissionState::NewAndExisting); + // Seeded with the identities of both folds, so the first candidate and every + // duplicate after it take the same path: fresh only if all are, admission as + // restricted as the most restricted. + let (fresh, held) = unique.entry(key).or_insert((true, AdmissionState::NewAndExisting)); + *fresh &= candidate.fresh; + *held = (*held).max(admission); } - if unique.len() > MAX_CANDIDATES { + unique +} + +/// Inference candidates within gateway limits, deduplicated and ordered. +/// +/// Past the cap, candidates taking new requests are kept before the rest. +fn candidates(overlay: &RoutingOverlay) -> Vec { + // Local first so cold start, before any signal, prefers this site. + let unique = dedup(overlay.candidates.iter().chain(&overlay.excluded), &overlay.local_site); + let mut kept: Vec<_> = unique.into_iter().collect(); + if kept.len() > MAX_CANDIDATES { tracing::warn!( - candidates = unique.len(), + candidates = kept.len(), "serving config: dropping candidates past the gateway cap" ); + kept.sort_by_key(|(key, (_, admission))| (*admission != AdmissionState::NewAndExisting, *key)); + kept.truncate(MAX_CANDIDATES); + kept.sort_by_key(|(key, _)| *key); } - unique - .into_iter() - .take(MAX_CANDIDATES) - .map(|((_, site, name, cluster), fresh)| ServingCandidate { + kept.into_iter() + .map(|((_, site, name, cluster), (fresh, admission))| ServingCandidate { kind: INFERENCE_MODEL, name: name.to_owned(), site: site.to_owned(), cluster: cluster.to_owned(), fresh, + admission, }) .collect() } -/// An inference candidate admitting new requests that the gateway will accept. +/// An inference candidate the gateway will accept, whatever its admission. +/// +/// A candidate not taking new requests stays in, so the gateway can tell a known +/// down model, answered with 503, from an unknown one. Its site must be a DNS-1123 +/// label, as enrolled site names are: the gateway carries it in headers and ids. fn routable(candidate: &RoutingCandidate) -> bool { candidate.kind == INFERENCE_MODEL - && candidate - .admission_state - .is_none_or(|state| state == AdmissionState::NewAndExisting) && [&candidate.name, &candidate.site, &candidate.cluster] .into_iter() .all(|id| valid_id(id)) + && certs::validate_site_name(&candidate.site).is_ok() +} + +/// The candidate sites the serving config refuses for not being DNS-1123 labels. +fn refused_sites(overlay: &RoutingOverlay) -> BTreeSet { + overlay + .candidates + .iter() + .chain(&overlay.excluded) + .filter(|candidate| certs::validate_site_name(&candidate.site).is_err()) + .map(|candidate| candidate.site.clone()) + .collect() +} + +/// The refused sites last seen per `GridNetwork`, keyed by network name. +pub(crate) type RefusedSites = Mutex>>; + +/// Warn when `network`'s set of refused sites changes, so an operator sees why a site never +/// routes. Kept per network, so two networks do not flip each other's warning. +pub(crate) fn warn_refused_sites(overlay: &RoutingOverlay, network: &str, last: &RefusedSites) { + let refused = refused_sites(overlay); + let mut last = last.lock().unwrap_or_else(PoisonError::into_inner); + if last.get(network) != Some(&refused) { + if !refused.is_empty() { + tracing::warn!(network, sites = ?refused, "serving config: refusing candidates whose site is not a DNS-1123 label"); + } + last.insert(network.to_owned(), refused); + } } /// Non-blank and within the gateway's identifier bound. @@ -386,6 +457,7 @@ mod tests { tls_mount: "/etc/praxis/tls", local_signals_addr: None, pins: &NO_PINS, + scrape_interval: Duration::from_secs(5), }; fn cand(name: &str, site: &str, cluster: &str, admission: Option<&str>) -> RoutingCandidate { @@ -403,6 +475,7 @@ mod tests { network: "grid".to_owned(), local_site: "site-a".to_owned(), candidates, + excluded: Vec::new(), selection_policy: None, generated_at: None, } @@ -423,6 +496,21 @@ mod tests { .collect() } + #[test] + fn the_load_window_covers_the_oldest_a_sample_can_arrive_plus_a_poll() { + assert_eq!( + load_window_ms(Duration::from_secs(5)), + 17_000, + "5s poll + 5s scrape + 2s timeout + 5s" + ); + assert_eq!(load_window_ms(Duration::from_secs(1)), 13_000); + let oldest_arrival = PEER_INTERVAL_MS + 5_000 + PEER_TIMEOUT_MS; + assert!( + i64::try_from(oldest_arrival).unwrap_or(i64::MAX) < load_window_ms(Duration::from_secs(5)), + "a sample at the worst-case age is still fresh" + ); + } + #[test] fn renders_the_contract_the_gateway_parses() { let members = [("site-b", "203.0.113.7:9091"), ("site-a", "198.51.100.1:9091")]; @@ -461,18 +549,42 @@ mod tests { } #[test] - fn candidates_the_gateway_would_reject_or_never_admit_are_dropped() { + fn refused_sites_name_only_the_invalid_ones() { + let sites = refused_sites(&overlay(vec![ + cand("llama", "site-a", "pool-a", None), + cand("llama", "site.b", "pool-b", None), + ])); + assert_eq!(sites, BTreeSet::from(["site.b".to_owned()])); + } + + #[test] + fn refused_sites_are_kept_per_network() { + let last = RefusedSites::default(); + let bad = overlay(vec![cand("llama", "site.b", "pool-b", None)]); + let good = overlay(vec![cand("llama", "site-a", "pool-a", None)]); + warn_refused_sites(&bad, "east", &last); + warn_refused_sites(&good, "west", &last); + let held = last.into_inner().unwrap_or_else(PoisonError::into_inner); + assert_eq!(held.get("east"), Some(&BTreeSet::from(["site.b".to_owned()]))); + assert_eq!( + held.get("west"), + Some(&BTreeSet::new()), + "another network does not clear it" + ); + } + + #[test] + fn candidates_the_gateway_would_reject_are_dropped() { let cases = [ - ("excluded", cand("llama", "site-b", "pool-b", Some("none"))), - ( - "existing only", - cand("llama", "site-b", "pool-b", Some("existing_only")), - ), ("blank cluster", cand("llama", "site-b", " ", None)), ( "oversized name", cand(&"m".repeat(MAX_NAME_LEN + 1), "site-b", "pool-b", None), ), + ("site with a dot", cand("llama", "site.b", "pool-b", None)), + ("site with a slash", cand("llama", "site/b", "pool-b", None)), + ("site with a quote", cand("llama", "site\"b", "pool-b", None)), + ("uppercase site", cand("llama", "Site-B", "pool-b", None)), ]; for (label, bad) in cases { assert!( @@ -488,6 +600,67 @@ mod tests { ); } + #[test] + fn an_excluded_candidate_reaches_the_serving_config_but_not_the_overlay_wire() { + let mut source = overlay(vec![cand("llama", "site-a", "pool-a", None)]); + source.excluded = vec![cand("llama", "site-b", "pool-b", Some("none"))]; + let rendered = render(&source, [], &INPUTS); + let config: serde_json::Value = serde_json::from_str(&to_text(&rendered).expect("text")).expect("json"); + let pool_b = config["candidates"] + .as_array() + .expect("candidates") + .iter() + .find(|c| c["cluster"] == "pool-b") + .expect("the excluded candidate is listed"); + assert_eq!(pool_b["admission"], "none"); + let wire = serde_json::to_string(&source).expect("overlay json"); + assert!(!wire.contains("pool-b"), "the overlay wire is unchanged: {wire}"); + } + + #[test] + fn past_the_cap_candidates_taking_new_requests_are_kept_first() { + let excluded: Vec<_> = (0..MAX_CANDIDATES) + .map(|i| cand("llama", "site-a", &format!("down-{i:04}"), Some("none"))) + .collect(); + let mut source = overlay(vec![cand("llama", "site-z", "up", None)]); + source.excluded = excluded; + let rendered = render(&source, [], &INPUTS); + assert_eq!(rendered.candidates.len(), MAX_CANDIDATES); + assert!( + rendered.candidates.iter().any(|c| c.cluster == "up"), + "the one admitted candidate survives the cap" + ); + } + + #[test] + fn a_candidate_not_taking_new_requests_stays_in_with_its_admission() { + let rendered = render( + &overlay(vec![ + cand("llama", "site-a", "pool-a", None), + cand("llama", "site-b", "pool-b", Some("none")), + cand("llama", "site-d", "pool-d", Some("existing_only")), + ]), + [], + &INPUTS, + ); + let config: serde_json::Value = serde_json::from_str(&to_text(&rendered).expect("text")).expect("json"); + let admission: Vec<(&str, Option<&str>)> = config["candidates"] + .as_array() + .expect("candidates") + .iter() + .map(|c| (c["site"].as_str().unwrap_or(""), c["admission"].as_str())) + .collect(); + assert_eq!( + admission, + [ + ("site-a", None), + ("site-b", Some("none")), + ("site-d", Some("existing_only")) + ], + "the default is omitted, so an older gateway still loads an all-admitted config" + ); + } + #[test] fn unsafe_or_irrelevant_peers_are_refused() { let cases = [ @@ -516,8 +689,8 @@ mod tests { let bad_name = overlay(vec![cand("llama", "Site_B", "pool-b", None)]); let config = render(&bad_name, [("Site_B", "203.0.113.7:9091")], &INPUTS); assert!( - config.peers.is_empty(), - "a non-DNS site name would fail the gateway's SNI" + config.candidates.is_empty() && config.peers.is_empty(), + "a non-DNS site name is neither a candidate nor a peer" ); } @@ -562,6 +735,24 @@ mod tests { } } + #[test] + fn the_most_restricted_duplicate_wins_in_either_order() { + let excluded = cand("llama", "site-b", "pool-b", Some("none")); + let admitted = cand("llama", "site-b", "pool-b", Some("new_and_existing")); + for (label, pair) in [ + ("excluded first", [excluded.clone(), admitted.clone()]), + ("excluded last", [admitted, excluded]), + ] { + let config = render(&overlay(pair.to_vec()), [], &INPUTS); + assert_eq!(config.candidates.len(), 1, "{label}: one entry"); + assert_eq!( + config.candidates[0].admission, + AdmissionState::Excluded, + "{label}: one cluster, so a duplicate calling it unservable closes it" + ); + } + } + fn member(site: &str, status: MemberStatus) -> crate::swim::MemberRecord { crate::swim::MemberRecord { site_id: site.to_owned(), From d189f2ba68b0e464175c9405cf1487844a75cff7 Mon Sep 17 00:00:00 2001 From: Sam Batschelet Date: Wed, 7 Oct 2026 12:26:34 -0400 Subject: [PATCH 06/10] docs(operator): provider readiness, in-flight, latency, and the series on /metrics Signed-off-by: Sam Batschelet --- docs/architecture/polling-metrics.md | 160 +++++++++++++++++---------- 1 file changed, 102 insertions(+), 58 deletions(-) diff --git a/docs/architecture/polling-metrics.md b/docs/architecture/polling-metrics.md index c026acd91..69044032d 100644 --- a/docs/architecture/polling-metrics.md +++ b/docs/architecture/polling-metrics.md @@ -1,71 +1,115 @@ # Polling Cross-Site Load Signals -A gateway routes across sites by preferring the least-loaded peer that serves a -model. To do that it needs each peer's current load. Each gateway polls its -peers' signal endpoints over mutual TLS, records the readings in a local store, -and the router orders cross-site candidates from that store. - -This document covers the poll path and its failure behavior. The signal wire -format and the store are described in [Signals](signals.md). How the ordered -candidates are selected is in [Routing](routing.md) and [Scoring](scoring.md). +A gateway prefers the least-loaded peer serving a model, so it needs each peer's +load. Each gateway polls its peers over mutual TLS, records the readings in a +local store, and orders cross-site candidates from it. Wire format and store: +[Signals](signals.md). Selection: [Routing](routing.md), [Scoring](scoring.md). +Peer identity: [Authentication](auth.md). ## The Poll Path -Polling is direct per peer, not through a relay. One poller dials one peer, -verifies that peer's Grid site certificate, and attributes the readings to that +Polling is direct per peer, not through a relay, and readings are attributed to one verified identity. | Step | What happens | |---|---| -| Dial | The poller opens a mutual-TLS connection to the peer's `/v1/site/signals`. Both ends present Grid site certificates. | -| Verify | The peer's certificate is checked against the Grid CA and its SPIFFE identity, then the verified identity is compared to the site the poller intended to reach. A valid Grid peer answering for a site the poller did not dial is refused. | -| Read | The exposition body is read under a byte ceiling and a time bound, so a slow or oversized peer cannot hold the poll open or exhaust memory. | -| Store | Each reading is keyed on the verified peer identity, never a value the response body carries. A body label that disagrees with the verified owner is dropped. | - -The verified peer identity is the store key. The response body cannot choose -where its readings land, so one peer cannot inject readings attributed to -another. - -## When a Gateway Polls - -A gateway polls its peers only while its own endpoint is actively serving. - -Cross-site routing is a decision this gateway makes when it receives a request -it can place elsewhere. A gateway that is not serving receives no such requests. -It has no routing decision to inform, so it holds no mutual-TLS connections to -its peers and adds no scrape load to them. Polling follows serving. It starts -when the gateway begins serving and stops when it drains. - -The number of live peer connections is then proportional to the serving -gateways, not to the size of the grid. A drained or standby gateway is silent on -the peer signal endpoints. - -## From Poll to Route - -The poll path and the route path meet at the store, and only at the store. - -The poller writes readings into the store as they arrive. Ordering runs off the -request path as a control step: it reads each candidate's recent worst load from -the store and produces a candidate list ordered least-loaded-first. The request -path reads one ordered snapshot and takes the front admitted candidate. The -request path does not read raw signals or compute load. It reads resolved order. +| Dial | Mutual TLS to the peer's `/v1/site/signals`, both ends presenting Grid site certificates. | +| Verify | The certificate is checked against the Grid CA and its SPIFFE identity, then the verified identity is compared to the site the poller dialed. A valid Grid peer answering for another site is refused. | +| Read | Under a byte ceiling and a time bound, so a slow or oversized peer cannot hold the poll open or exhaust memory. | +| Store | Keyed on the verified identity, never on anything the body carries, so no peer can inject readings as another. A body label disagreeing with the verified owner is dropped. | +| Bound | Only the contract names and EPP pool averages the gateway routes on, a DNS-1123 `grid_provider`, a finite non-negative value (at most one for `grid_provider_ready` and `grid_provider_error_ratio`), and the peer's first 64 providers by name. Custom `signalNames` are dropped. Refusals count in `grid_peer_signals_refused_total{peer,reason}`. | + +A gateway polls only while serving, since one that is not serving has no routing +decision to inform. Live connections are proportional to the serving gateways, +not to the grid. + +Poll and route meet at the store, and only there. Ordering runs off the request +path, reading each candidate's recent worst load into a least-loaded-first list. +The request path reads one ordered snapshot and takes the front admitted +candidate; it never reads raw signals or computes load. + +## Provider Readiness + +Each operator publishes whether its providers can serve, as +`grid_provider_ready{grid_site,grid_provider}`. A provider is not ready when its +EPP reports zero ready endpoints for two scrapes running and recorded no engine +answer in 30s, when no scrape succeeded within `staleMetricsSeconds` (half the +signal TTL when unset), or when it is `Unavailable`. The EPP counts endpoints +with fresh metrics, so a saturated engine can read zero while serving, which is +why two scrapes and the engine answer both gate the verdict. + +A scrape answering without the pool's ready-endpoint series leaves readiness +unknown, not false, and does not exclude: a provider pointed at vLLM's own +`/metrics` carries no such series. + +The verdict is also the provider's `Ready` condition, whose reason names the +cause and, for a failed scrape, its class. See [crds.md](./crds.md). + +The gateway reads the latest sample when it orders. A 0 excludes, so a site +drops within one poll of its operator deciding and returns within one poll of +recovery. A missing series reads as ready, so a site whose operator predates +readiness still routes. The serving config carries the same verdict for local +providers as `admission: none`. When every candidate for a model is excluded the +gateway answers 503 with `Retry-After`, not 404: the model exists but cannot be +served now. At the default 5s scrape and 5s poll, exclusion takes about 15s and +rejoin about 10s. + +## Provider In-flight + +Each operator also publishes `grid_provider_in_flight_requests`, beside +`grid_provider_ready_endpoints` so the count has a denominator at the reader. +Every input comes from the EPP's `/metrics`; nothing scrapes vLLM. The value is the larger of +two estimates, plus what the EPP's flow control holds for the pool +(`llm_d_epp_flow_control_queue_size`): + +- the EPP's per-endpoint `llm_d_epp_inflight_requests`, summed, taking each + endpoint's largest count across producer instances. Needs the EPP's + inflight-load-producer. +- the pool's average running plus average queued, times ready endpoints. + +The larger, so an EPP restart that zeroes its count does not make the site look +idle. The per-endpoint count carries no pool label, so an EPP serving several +pools falls back to the averages alone. A site with no fresh endpoint publishes +nothing, since its averages are frozen, and the gateway reads it as unknown. +Point `metricsConfig.metricsEndpoint` at the EPP Service: a pod or headless +address can reach a standby replica, which reports no series. + +On a prefill/decode pool the EPP counts a request on both endpoints, so the value +measures endpoint occupancy, up to twice the requests. + +## Provider Latency + +Each operator also publishes recent latency from the EPP's request histograms +over 30s, only when at least 20 requests completed in that window, and never +borrowed from another site. + +- `grid_provider_ttft_p50_seconds`, `grid_provider_ttft_p90_seconds`: time to + first token, streaming requests, from `llm_d_epp_request_ttft_seconds`. Timed + from receiving the request, so it includes flow-control wait and network. +- `grid_provider_tpot_seconds`: mean time per output token, streaming requests. +- `grid_provider_error_ratio`: failed over all requests. The latency histograms + record only successes, so a failing site can read fast; this shows it. + +The EPP labels these by model, not pool, so an EPP serving several pools reports +their combined latency. A restart resets its counters and the window starts over. + +## Provider Series on /metrics + +The operator exports these series on its Prometheus `/metrics` listener, labeled +`grid_site` and `grid_provider`, for its own providers and those it polls, so a +Prometheus scraping one hub sees every site. A peer's value there is what the hub +last polled, up to one poll old. A series the operator does not hold is absent, +not 0. ## Failure Behavior -| Condition | Signal produced | Routing effect | -|---|---|---| -| No reading for a candidate | The candidate scores as maximally loaded. | It sorts after every candidate that has a reading, so a measured healthy peer is preferred over an unmeasured one. | -| Readings all stale (older than the window) | Same as no reading. | The candidate sorts last until a fresh reading arrives. | -| Peer unreachable or slow | The poll returns an error and no reading is written. | The candidate ages out of the window and then sorts last. The poll loop continues, and one unreachable peer does not wedge the others. | -| Peer presents an untrusted or mismatched certificate | The connection is refused, so no reading is written. | The peer contributes nothing to the order. | - -Loss of signal degrades to "least preferred," never to "silently treated as -idle." A drained burst stays penalized until it ages out of the window rather -than snapping back to idle on the first missing sample. - -## See Also +A reading that is missing, stale, or never written because the peer was +unreachable, slow, or presented an untrusted or mismatched certificate scores the +candidate as maximally loaded, so it sorts after every candidate that has one. +The poll loop continues, so one unreachable peer does not wedge the others. A +peer reporting `grid_provider_ready 0` is excluded outright until a later reading +says 1, and if every candidate is excluded the model answers 503. -- [Signals](signals.md), the signal wire format and the store. -- [Routing](routing.md), how an ordered candidate is selected. -- [Scoring](scoring.md), how load maps to order. -- [Authentication](auth.md), the Grid mTLS peer identity layer the poll path uses. +Loss of signal degrades to least preferred, not to idle. A drained burst stays +penalized until it ages out rather than snapping back on the first missing +sample. From 862f3701dbc53daffe67f94ce18a1234498a28e3 Mon Sep 17 00:00:00 2001 From: Sam Batschelet Date: Wed, 7 Oct 2026 12:24:50 -0400 Subject: [PATCH 07/10] feat(operator): relay every serving gauge a provider publishes Lab port of feat/relay-provider-gauges onto the PR 286 head. The relay carried only the series a provider declared under metricsConfig.signalNames, which exist for scoring, so the EPP's running and ready-endpoint series never reached a gateway. It now carries every gauge the provider publishes except the go_, process_, controller_runtime_ and workqueue_ families. Signed-off-by: Sam Batschelet --- operator/src/resources/provider_metrics.rs | 70 ++++++++++------------ 1 file changed, 31 insertions(+), 39 deletions(-) diff --git a/operator/src/resources/provider_metrics.rs b/operator/src/resources/provider_metrics.rs index ea877b277..dc86e6c29 100644 --- a/operator/src/resources/provider_metrics.rs +++ b/operator/src/resources/provider_metrics.rs @@ -107,12 +107,13 @@ pub(crate) struct CollectedMetrics { // Signals collection (poll mode) // --------------------------------------------------------------------------- -/// Scrape each provider's endpoint, recording its coarse signals and readiness. +/// Scrape each provider's endpoint, recording its serving gauges and readiness. /// /// Sibling of [`collect_provider_metrics_with_refresh_interval`], which parses /// the same text into [`scoring::BackendMetrics`] for local scoring. This keeps -/// the provider's own exposition narrowed to its declared `signalNames`, so the -/// wire carries a coarse rollup rather than the full `/metrics` firehose. Fails +/// the provider's own exposition as published, minus the families every process +/// exports about itself, so a reader concludes from the provider's raw series +/// and `signalNames` stays a scoring concern. Fails /// closed on TLS: a provider whose TLS will not resolve is skipped, never /// scraped in plaintext, and counts as a failed scrape. pub(crate) async fn collect_provider_signals( @@ -162,7 +163,7 @@ fn record_scrape( .into_iter() .zip(republishable) .filter_map(|(o, republishable)| republishable.then_some(o)) - .filter(|o| plan.wanted.contains(o.metric.as_str())) + .filter(|o| relayed(&o.metric)) // A local sample's freshness is its collection time, so drop any trailing // timestamp. Only relayed peer samples carry a per-sample stamp. .map(|mut o| { @@ -250,8 +251,6 @@ pub(crate) struct SignalScrapePlan<'provider> { pub(crate) identity: &'provider str, /// The metrics URL. pub(crate) url: String, - /// Declared signal names republished as coarse signals; may be empty. - pub(crate) wanted: std::collections::BTreeSet, /// A declared ready-endpoints metric, replacing the defaults. ready_override: Option<&'provider str>, } @@ -281,25 +280,17 @@ pub(crate) fn signal_scrape_plan(provider: &InferenceProvider) -> Option std::collections::BTreeSet { - [ - &cfg.queue_depth, - &cfg.kv_cache_utilization, - &cfg.latency_p99_ms, - &cfg.prefix_cache_hit_ratio, - &cfg.error_rate, - &cfg.healthy, - ] - .into_iter() - .flatten() - .cloned() - .collect() +/// Metric families every Go and controller-runtime process exports about itself. +const RUNTIME_FAMILIES: [&str; 4] = ["go_", "process_", "controller_runtime_", "workqueue_"]; + +/// Whether a gauge the provider publishes is relayed: everything it says about serving, +/// nothing about its own process, so the wire stays well under the store's series cap. +fn relayed(metric: &str) -> bool { + !RUNTIME_FAMILIES.iter().any(|family| metric.starts_with(family)) } // --------------------------------------------------------------------------- @@ -1080,24 +1071,25 @@ llm_d_epp_request_error_total{{{l},error_code="503"}} {errors} } #[test] - fn signal_metric_names_keeps_only_declared_coarse_signals() { - // The rollup carries the provider's declared coarse signals, not the - // full /metrics firehose: absent signals contribute no metric name, so - // the later filter drops everything the provider did not declare. - let cfg = MetricSignalNames { - queue_depth: Some("vllm:num_requests_waiting".to_owned()), - kv_cache_utilization: Some("vllm:gpu_cache_usage_perc".to_owned()), - ..Default::default() - }; - let wanted = signal_metric_names(&cfg); - assert_eq!(wanted.len(), 2, "only the two declared signals are kept"); - assert!(wanted.contains("vllm:num_requests_waiting")); - assert!(wanted.contains("vllm:gpu_cache_usage_perc")); - assert!( - !wanted.contains("vllm:gpu_memory_usage_bytes"), - "an undeclared series is not in the rollup" - ); - assert!(signal_metric_names(&MetricSignalNames::default()).is_empty()); + fn serving_gauges_are_relayed_and_runtime_families_are_not() { + // The relay carries what the provider says about serving, declared or not, and + // nothing a process says about itself. + for metric in [ + "llm_d_epp_average_running_requests", + "inference_pool_ready_pods", + "vllm:num_requests_waiting", + "queue_depth", + ] { + assert!(relayed(metric), "{metric} is relayed"); + } + for metric in [ + "go_goroutines", + "process_resident_memory_bytes", + "controller_runtime_active_workers", + "workqueue_depth", + ] { + assert!(!relayed(metric), "{metric} is runtime noise"); + } } #[test] From 0b240c828a171975e99b9dfe36e1f44c3a991a9f Mon Sep 17 00:00:00 2001 From: Sam Batschelet Date: Wed, 7 Oct 2026 15:35:21 -0400 Subject: [PATCH 08/10] fix(operator): relay the backlog and per-unit series from peers A hub kept only the pool averages and ready counts from a peer, so the flow-control hold, the per-unit queues, and inference_pool_ready_pods reached a gateway for the local site only. A peer's backlog could still teach its ceiling, and a peer pool that drained could not be told from one whose gauges froze. Signed-off-by: Sam Batschelet --- operator/src/signals.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/operator/src/signals.rs b/operator/src/signals.rs index a81a706c8..d6cdf17b6 100644 --- a/operator/src/signals.rs +++ b/operator/src/signals.rs @@ -1252,7 +1252,7 @@ fn with_query(url: &str, query: &str) -> Option { /// Names a hub keeps from a peer: the cross-site contract, plus the EPP pool averages and /// ready counts the gateway routes on until it routes on saturation. A peer's custom /// `signalNames` are dropped here. -const PEER_SIGNAL_NAMES: [&str; 14] = [ +const PEER_SIGNAL_NAMES: [&str; 18] = [ crate::readiness::READY_SIGNAL, crate::readiness::READY_ENDPOINTS_SIGNAL, crate::readiness::IN_FLIGHT_SIGNAL, @@ -1267,6 +1267,12 @@ const PEER_SIGNAL_NAMES: [&str; 14] = [ "inference_pool_average_kv_cache_utilization", "llm_d_epp_average_kv_cache_utilization", "llm_d_epp_ready_endpoints", + "inference_pool_ready_pods", + // What a gateway reads as a backlog beside the averages, and the per-unit series whose + // disappearance tells a drained pool from one whose gauges froze. + "llm_d_epp_flow_control_queue_size", + "inference_pool_per_pod_queue_size", + "llm_d_epp_per_endpoint_queue_size", ]; /// Most providers a hub keeps from one peer, so a peer cannot grow the hub's series without From 64f60b5d3f9a6846a7c508137faa7d9e825db057 Mon Sep 17 00:00:00 2001 From: Sam Batschelet Date: Wed, 7 Oct 2026 15:45:28 -0400 Subject: [PATCH 09/10] fix(operator): a positive ready count with the per-unit series gone reads as zero The EPP's metrics refresh returns early when a pool has no pods, so llm_d_epp_ready_endpoints and the pool averages freeze at their last values while the per-endpoint queue collector stops reporting. A reachable EPP could therefore keep a drained pool apparently ready. A scrape with no per-unit queue series for the pool, where an earlier scrape had them, now reads its ready count as zero, so the verdict follows the usual two-scrape path to NoEndpointsReady. A provider whose EPP never exposes per-unit series keeps its count. Signed-off-by: Sam Batschelet --- operator/src/readiness.rs | 41 ++++++++++++++++++++++ operator/src/resources/provider_metrics.rs | 16 ++++++++- 2 files changed, 56 insertions(+), 1 deletion(-) diff --git a/operator/src/readiness.rs b/operator/src/readiness.rs index b80199802..5467e11e4 100644 --- a/operator/src/readiness.rs +++ b/operator/src/readiness.rs @@ -15,6 +15,11 @@ use crate::{crd::inference_provider::Condition, signals::Observation}; /// Metric names that count a pool's ready endpoints, preferred first. pub(crate) const DEFAULT_READY_ENDPOINTS: [&str; 2] = ["llm_d_epp_ready_endpoints", "inference_pool_ready_pods"]; +/// The EPP's per-unit queue series, one per serving unit. They come from a collector that +/// stops reporting when the pool has no units, while the pool gauges freeze at their last +/// values, so their absence tells a drained pool from a live one. +pub(crate) const PER_UNIT_QUEUE: [&str; 2] = ["inference_pool_per_pod_queue_size", "llm_d_epp_per_endpoint_queue_size"]; + /// Consecutive scrapes reading zero ready endpoints before a provider is not ready, /// and reading some before it is ready again. /// @@ -225,6 +230,8 @@ struct Probe { last_progress: Option, /// Whether the latest zero count came while the engine was answering. busy: bool, + /// When a scrape last carried a per-unit series, so their absence means something. + units_last_seen: Option, /// Recent EPP latency snapshots. latency: crate::latency::History, } @@ -253,6 +260,23 @@ impl ReadinessStore { self.0.lock().unwrap_or_else(PoisonError::into_inner) } + /// The ready count a scrape really carries: a positive gauge in a scrape with no + /// per-unit series, where earlier scrapes had them, is frozen at its last value because + /// the pool drained, and reads as zero. + pub(crate) fn thawed(&self, key: &str, ready: Option, units_reporting: bool, now: Instant) -> Option { + let mut probes = self.probes(); + let probe = probes.entry(key.to_owned()).or_default(); + if units_reporting { + probe.units_last_seen = Some(now); + } + let units_seen = probe.units_last_seen.is_some(); + drop(probes); + if !units_reporting && units_seen && ready.is_some_and(|count| count >= 1.0) { + return Some(0.0); + } + ready + } + /// Record a scrape's latency counters and return the latency series it publishes. #[expect(clippy::significant_drop_tightening, reason = "the guard covers one whole update")] pub(crate) fn record_latency(&self, key: &str, observations: &[Observation], now: Instant) -> Vec { @@ -1116,4 +1140,21 @@ mod tests { "a queue alone is no count" ); } + + #[test] + fn a_positive_count_with_the_per_unit_series_gone_reads_as_zero() { + let store = ReadinessStore::default(); + let now = Instant::now(); + // Units reporting: the count stands. + assert_eq!(store.thawed("p", Some(2.0), true, now), Some(2.0)); + // The pool drains: the gauge freezes at 2 while the per-unit collector stops. + assert_eq!( + store.thawed("p", Some(2.0), false, now), + Some(0.0), + "a frozen gauge is not a ready pool" + ); + // A provider that never had per-unit series keeps its count. + assert_eq!(store.thawed("q", Some(3.0), false, now), Some(3.0)); + assert_eq!(store.thawed("p", None, false, now), None); + } } diff --git a/operator/src/resources/provider_metrics.rs b/operator/src/resources/provider_metrics.rs index dc86e6c29..f0146ea5e 100644 --- a/operator/src/resources/provider_metrics.rs +++ b/operator/src/resources/provider_metrics.rs @@ -155,7 +155,12 @@ fn record_scrape( .metrics_config .as_ref() .and_then(|mc| mc.pool_name.as_deref()); - let ready = crate::readiness::ready_endpoints(&parsed, plan.ready_names(), pool); + let ready = readiness.thawed( + key, + crate::readiness::ready_endpoints(&parsed, plan.ready_names(), pool), + units_reporting(&parsed, pool), + Instant::now(), + ); let missing = count_scrape(plan, pool, ready); let in_flight = in_flight_observation(&parsed, ready, pool, plan.identity); let latency = readiness.record_latency(key, &parsed, Instant::now()); @@ -176,6 +181,15 @@ fn record_scrape( readiness.record_success(key, ready, missing, observations, Instant::now()); } +/// Whether the scrape carried a per-unit queue series for the pool: the EPP's collector for +/// those stops reporting when the pool has no units, while its pool gauges freeze. +fn units_reporting(parsed: &[crate::signals::Observation], pool: Option<&str>) -> bool { + parsed.iter().any(|o| { + crate::readiness::PER_UNIT_QUEUE.contains(&o.metric.as_str()) + && pool.is_none_or(|pool| o.labels.get("name").is_some_and(|n| n == pool)) + }) +} + /// Count a scrape that answered: `success` with the pool's ready-endpoint series, else /// `no_series`, returned as the message naming what was missing. fn count_scrape(plan: &SignalScrapePlan<'_>, pool: Option<&str>, ready: Option) -> Option { From e2e3237043615f85eb572f655dee4c0eda0d99ac Mon Sep 17 00:00:00 2001 From: Sam Batschelet Date: Wed, 7 Oct 2026 16:30:47 -0400 Subject: [PATCH 10/10] fix(operator): review findings on readiness, latency, the overlay, and peer signals Latency: a counter that went backwards since the last scrape clears history before any window is computed, so no sample straddles a restart, and bucket deltas are one merge pass over the two bound-ordered lists instead of a quadratic search under the readiness lock. Readiness: probes are keyed by provider name so two providers sharing a routing cluster keep their own streaks, the published entry per identity is the stricter one, and a counter rise only counts as progress when its baseline is within the window. Ready condition writes run in a spawned task with a deadline each, off the publication path. The overlay takes the operator's own site separately from the gateway's, so an empty selector resolves to the site that owns the provider, and geography looks sites up by the same peer site key candidates carry. A peer's grid_provider passes the cluster-name rule rather than the site-name rule, so a name such as pool.v1 keeps its series. The scrape error classifier walks io::Error::get_ref recursively, so a TLS failure wrapped twice by hyper-rustls still classes as TLS. Signed-off-by: Sam Batschelet --- operator/src/controller/grid_network.rs | 83 ++++++++++++++++++---- operator/src/latency.rs | 79 +++++++++++++++----- operator/src/metrics_scraper.rs | 34 +++++++-- operator/src/readiness.rs | 40 ++++++++--- operator/src/resources/geography.rs | 34 ++++++++- operator/src/resources/provider_metrics.rs | 5 +- operator/src/resources/routing_overlay.rs | 49 ++++++++++--- operator/src/signals.rs | 34 +++++++-- 8 files changed, 296 insertions(+), 62 deletions(-) diff --git a/operator/src/controller/grid_network.rs b/operator/src/controller/grid_network.rs index 9168e0410..f72a4c5ee 100644 --- a/operator/src/controller/grid_network.rs +++ b/operator/src/controller/grid_network.rs @@ -450,13 +450,13 @@ pub async fn refresh_signals(ctx: &OperatorCtx, client: &Client, network_name: & &ctx.readiness, )) .await; - let collected = Box::pin(resolve_readiness(ctx, client, &providers, network_name)).await; + let collected = resolve_readiness(ctx, client, &providers, network_name); publish_signals(ctx, collected); Ok(()) } /// Resolve each provider's readiness, write its `Ready` condition, and return what to publish. -async fn resolve_readiness( +fn resolve_readiness( ctx: &OperatorCtx, client: &Client, providers: &[InferenceProvider], @@ -465,28 +465,84 @@ async fn resolve_readiness( ctx.readiness.retain( &providers .iter() + .filter(|p| provider_metrics::signal_scrape_plan(p).is_some()) .filter_map(|p| { - provider_metrics::signal_scrape_plan(p) - .map(|plan| readiness::key(&p.spec.grid_network_ref, plan.identity)) + p.metadata + .name + .as_deref() + .map(|name| readiness::key(&p.spec.grid_network_ref, name)) }) .collect(), ); let now = Instant::now(); - let mut collected = HashMap::new(); + let mut collected: HashMap> = HashMap::new(); + let mut writes = Vec::new(); for provider in providers.iter().filter(|p| p.spec.grid_network_ref == network_name) { let Some(entry) = readiness_entry(ctx, provider, now) else { continue; }; - if let Err(error) = Box::pin(apply_ready_condition(client, provider, &entry.verdict)).await { - tracing::warn!(provider = entry.identity, %error, "provider Ready condition not written"); - } + writes.push((provider.clone(), entry.verdict)); if let Some(published) = entry.published { - collected.insert(entry.identity.to_owned(), published); + publish_strictest(&mut collected, entry.identity, published); } } + write_ready_conditions(client.clone(), writes); collected } +/// Keep one entry per routing identity: the stricter verdict wins, so one provider +/// reporting no endpoints is not hidden by another's ready behind the same cluster. +fn publish_strictest( + collected: &mut HashMap>, + identity: &str, + published: Vec, +) { + match collected.entry(identity.to_owned()) { + std::collections::hash_map::Entry::Vacant(slot) => { + slot.insert(published); + }, + std::collections::hash_map::Entry::Occupied(mut slot) => { + if excludes(&published) && !excludes(slot.get()) { + slot.insert(published); + } + }, + } +} + +/// Whether a published entry carries a not-ready verdict. +fn excludes(published: &[signals::Observation]) -> bool { + published + .iter() + .any(|o| o.metric == readiness::READY_SIGNAL && o.value < 1.0) +} + +/// How long one `Ready` condition write may take before it is abandoned. +const READY_WRITE_TIMEOUT: Duration = Duration::from_secs(10); + +/// Write each provider's `Ready` condition off the publication path, each under a deadline, +/// so a stalled status PATCH (the default client has no read timeout) cannot hold every +/// provider's signals from publishing and let them expire. +fn write_ready_conditions(client: Client, writes: Vec<(InferenceProvider, readiness::Verdict)>) { + if writes.is_empty() { + return; + } + tokio::spawn(async move { + for (provider, verdict) in writes { + let name = provider.metadata.name.as_deref().unwrap_or("?"); + match timeout( + READY_WRITE_TIMEOUT, + Box::pin(apply_ready_condition(&client, &provider, &verdict)), + ) + .await + { + Ok(Ok(())) => {}, + Ok(Err(error)) => tracing::warn!(provider = name, %error, "provider Ready condition not written"), + Err(_elapsed) => tracing::warn!(provider = name, "provider Ready condition write timed out"), + } + } + }); +} + /// One provider's readiness and the signals to publish for it. struct ReadinessEntry<'provider> { /// Routing identity. @@ -507,7 +563,7 @@ fn readiness_entry<'provider>( now: Instant, ) -> Option> { let identity = routing_overlay::routing_identity(provider)?; - let key = readiness::key(&provider.spec.grid_network_ref, identity); + let key = readiness::key(&provider.spec.grid_network_ref, provider.metadata.name.as_deref()?); if provider_metrics::signal_scrape_plan(provider).is_none() { return Some(ReadinessEntry { identity, @@ -587,13 +643,13 @@ pub(crate) fn provider_readiness( provider: &InferenceProvider, now: Instant, ) -> Option { - let plan = provider_metrics::signal_scrape_plan(provider)?; + provider_metrics::signal_scrape_plan(provider)?; let unavailable = provider .status .as_ref() .is_some_and(|status| status.phase == crate::crd::inference_provider::ProviderPhase::Unavailable); ctx.readiness.verdict( - &readiness::key(&provider.spec.grid_network_ref, plan.identity), + &readiness::key(&provider.spec.grid_network_ref, provider.metadata.name.as_deref()?), unavailable, readiness_stale_after(ctx, provider), now, @@ -1383,6 +1439,7 @@ pub async fn reconcile(network: Arc, ctx: Arc) -> Resu &scoring_weights, &admission_states, serving.as_ref(), + ctx.site_name(), ) .await?; @@ -1995,6 +2052,7 @@ async fn reconcile_routing_overlay_inner( scoring_weights: &scoring::ScoringWeights, admission_states: &HashMap, serving: Option<&ServingSource<'_>>, + owning_site: Option<&str>, ) -> Result { let network_name = grid_network_name(network)?; @@ -2050,6 +2108,7 @@ async fn reconcile_routing_overlay_inner( tool_providers, &eligible_remote_owned, local_site, + owning_site, metrics_arg, timestamp.as_deref(), scoring_weights, diff --git a/operator/src/latency.rs b/operator/src/latency.rs index 4d0ffdb0c..c91f87dab 100644 --- a/operator/src/latency.rs +++ b/operator/src/latency.rs @@ -151,17 +151,22 @@ impl Snapshot { }) } - /// Each TTFT bucket's count less `earlier`'s, `None` when one went backwards. + /// Each TTFT bucket's count less `earlier`'s, `None` when one went backwards. Both bucket + /// lists are in bound order, so this is one merge pass; a bound `earlier` lacks counts + /// from zero. fn bucket_deltas(&self, earlier: &Self) -> Option> { + let mut then = earlier.ttft_buckets.iter().peekable(); self.ttft_buckets .iter() .map(|(bound, count)| { - let then = earlier - .ttft_buckets - .iter() - .find(|(b, _)| b.to_bits() == bound.to_bits()) + while then.peek().is_some_and(|(b, _)| b.to_bits() < bound.to_bits()) { + then.next(); + } + let before = then + .peek() + .filter(|(b, _)| b.to_bits() == bound.to_bits()) .map_or(0.0, |(_, earlier_count)| *earlier_count); - (*count >= then).then_some((*bound, count - then)) + (*count >= before).then_some((*bound, count - before)) }) .collect() } @@ -195,13 +200,25 @@ pub(crate) struct History { } impl History { - /// The latest recorded scrape. - pub(crate) fn latest(&self) -> Option<&Snapshot> { - self.snapshots.back().map(|(_, snapshot)| snapshot) + /// The latest recorded scrape and when it was taken. + pub(crate) fn latest(&self) -> Option<(Instant, &Snapshot)> { + self.snapshots.back().map(|(at, snapshot)| (*at, snapshot)) } /// Record this scrape and return the latency series to publish for it. + /// + /// A counter that went backwards since the last scrape is a restart: history starts over + /// from this scrape and it publishes nothing, so no window straddles the reset. pub(crate) fn record(&mut self, snapshot: Snapshot, now: Instant) -> Vec { + if self + .snapshots + .back() + .is_some_and(|(_, last)| snapshot.since(last).is_none()) + { + self.snapshots.clear(); + self.snapshots.push_back((now, snapshot)); + return Vec::new(); + } while self .snapshots .get(1) @@ -214,14 +231,6 @@ impl History { .front() .and_then(|(_, oldest)| snapshot.since(oldest)) .map_or_else(Vec::new, |window| window.observations()); - if self - .snapshots - .back() - .is_some_and(|(_, last)| snapshot.since(last).is_none()) - { - // A counter reset: start over from this scrape. - self.snapshots.clear(); - } self.snapshots.push_back((now, snapshot)); published } @@ -365,6 +374,42 @@ mod tests { assert!(value(&after, TPOT_SIGNAL).is_some(), "the window restarts at the reset"); } + #[test] + fn a_reset_below_the_oldest_snapshot_still_publishes_nothing() { + // 10, 100, then 50: the delta against the oldest snapshot is positive, but the counter + // went backwards since the last one, so the scrape straddles a restart. + let mut history = History::default(); + let start = Instant::now(); + history.record_observations(&scrape(10.0, 0.3, 0.02, 1000.0, 0.0), start); + history.record_observations(&scrape(100.0, 0.3, 0.02, 1000.0, 0.0), start + Duration::from_secs(5)); + let reset = history.record_observations(&scrape(50.0, 0.3, 0.02, 1000.0, 0.0), start + Duration::from_secs(10)); + assert!(reset.is_empty(), "nothing crosses the restart: {reset:?}"); + let after = + history.record_observations(&scrape(100.0, 0.3, 0.02, 1000.0, 0.0), start + Duration::from_secs(15)); + assert!(value(&after, TPOT_SIGNAL).is_some(), "the window restarts at the reset"); + } + + #[test] + fn bucket_deltas_merge_in_bound_order_and_a_new_bound_counts_from_zero() { + let earlier = Snapshot { + ttft_buckets: vec![(0.1, 1.0), (0.4, 3.0), (f64::INFINITY, 3.0)], + ..Snapshot::default() + }; + let later = Snapshot { + ttft_buckets: vec![(0.1, 2.0), (0.2, 2.0), (0.4, 5.0), (f64::INFINITY, 6.0)], + ..Snapshot::default() + }; + assert_eq!( + later.bucket_deltas(&earlier), + Some(vec![(0.1, 1.0), (0.2, 2.0), (0.4, 2.0), (f64::INFINITY, 3.0)]) + ); + let backwards = Snapshot { + ttft_buckets: vec![(0.1, 0.0), (0.4, 3.0), (f64::INFINITY, 3.0)], + ..Snapshot::default() + }; + assert_eq!(backwards.bucket_deltas(&earlier), None); + } + #[test] fn the_window_drops_snapshots_older_than_it() { let mut history = History::default(); diff --git a/operator/src/metrics_scraper.rs b/operator/src/metrics_scraper.rs index 2772e5066..4ef3fea9e 100644 --- a/operator/src/metrics_scraper.rs +++ b/operator/src/metrics_scraper.rs @@ -124,12 +124,7 @@ fn transport_class(source: &(dyn std::error::Error + 'static)) -> crate::readine use crate::readiness::ScrapeClass; let mut cause = Some(source); while let Some(error) = cause { - // io::Error's source skips the error it wraps, which is where a TLS failure sits. - let wrapped = error - .downcast_ref::() - .and_then(std::io::Error::get_ref) - .is_some_and(|inner| crate::resources::tls_backend::is_tls_error(inner)); - if wrapped || crate::resources::tls_backend::is_tls_error(error) { + if tls_within(error) { return ScrapeClass::Tls; } let text = error.to_string(); @@ -147,6 +142,19 @@ fn transport_class(source: &(dyn std::error::Error + 'static)) -> crate::readine ScrapeClass::Connect } +/// Whether `error` is a TLS failure or wraps one through any depth of `io::Error`. +/// +/// `io::Error::source` skips the error it wraps, which is where a TLS failure sits, and +/// hyper-rustls wraps the connector's `io::Error` in another, so the chain is walked by +/// `get_ref`, not `source`. +fn tls_within(error: &(dyn std::error::Error + 'static)) -> bool { + crate::resources::tls_backend::is_tls_error(error) + || error + .downcast_ref::() + .and_then(std::io::Error::get_ref) + .is_some_and(|inner| tls_within(inner)) +} + // --------------------------------------------------------------------------- // Scrape // --------------------------------------------------------------------------- @@ -391,6 +399,20 @@ mod tests { ); } + #[test] + #[cfg(not(feature = "fips"))] + fn a_tls_failure_wrapped_twice_classes_as_tls() { + // hyper-rustls wraps the connector's io::Error in another io::Error::other. + let tls: Box = Box::new(std::io::Error::other(std::io::Error::new( + std::io::ErrorKind::InvalidData, + rustls::Error::General("bad certificate".to_owned()), + ))); + assert_eq!( + MetricsScrapeError::Transport(tls).class(), + crate::readiness::ScrapeClass::Tls + ); + } + #[test] #[expect(clippy::too_many_lines, reason = "one table of failures and their classes")] fn a_scrape_failure_has_a_bounded_class() { diff --git a/operator/src/readiness.rs b/operator/src/readiness.rs index 5467e11e4..f21603ce9 100644 --- a/operator/src/readiness.rs +++ b/operator/src/readiness.rs @@ -245,13 +245,16 @@ pub struct Verdict { pub message: String, } -/// The latest scrape per provider, keyed by network and routing identity. +/// The latest scrape per provider, keyed by network and provider name. #[derive(Debug, Default)] pub struct ReadinessStore(Mutex>); -/// The store key for `identity` in `network`: identities are unique per network only. -pub(crate) fn key(network: &str, identity: &str) -> String { - format!("{network}/{identity}") +/// The store key for the provider named `provider` in `network`: names are unique per +/// network only. Keyed by provider, not routing identity, so two providers sharing a +/// cluster keep their own history and one reporting zero endpoints is not cleared by the +/// other. +pub(crate) fn key(network: &str, provider: &str) -> String { + format!("{network}/{provider}") } impl ReadinessStore { @@ -286,11 +289,12 @@ impl ReadinessStore { let attributable = !serves_several_pools(observations); let mut probes = self.probes(); let probe = probes.entry(key.to_owned()).or_default(); + // Progress is only as recent as the scrape it is measured from: a delta against a + // baseline older than the window says nothing about the last 30 s. if attributable - && probe - .latency - .latest() - .is_some_and(|earlier| snapshot.produced_since(earlier)) + && probe.latency.latest().is_some_and(|(at, earlier)| { + now.saturating_duration_since(at) <= PROGRESS_WINDOW && snapshot.produced_since(earlier) + }) { probe.last_progress = Some(now); } @@ -817,6 +821,26 @@ mod tests { ); } + #[test] + fn progress_measured_against_a_stale_baseline_does_not_readmit() { + let store = ReadinessStore::default(); + let start = Instant::now(); + scrape_at(&store, &saturated(2.0, 100.0), start); + // Eight zero scrapes with no answers: excluded. + for n in 1..=8_u32 { + scrape_at(&store, &saturated(0.0, 100.0), start + STEP * n); + } + assert_eq!(reason(&store, start + STEP * 8), Some(Reason::NoEndpointsReady)); + // A 70 s gap, then a scrape whose counters rose since the last one. The rise is + // measured from a baseline older than the window, so it is not recent progress. + let late = start + STEP * 8 + Duration::from_secs(70); + assert_eq!( + scrape_at(&store, &saturated(0.0, 140.0), late), + Some(Reason::NoEndpointsReady), + "a delta from before the window is not an answer in the window" + ); + } + #[test] fn a_pool_that_stops_answering_is_excluded_once_the_window_passes() { let store = ReadinessStore::default(); diff --git a/operator/src/resources/geography.rs b/operator/src/resources/geography.rs index cf1beec08..b7ae401b8 100644 --- a/operator/src/resources/geography.rs +++ b/operator/src/resources/geography.rs @@ -107,7 +107,8 @@ pub(crate) fn derive_locality_tier( } } -/// Look up region and zone for a named site in a network. +/// Look up region and zone for a site in a network, keyed as candidates and peers name it: +/// a discovered site by its site id, not its object name. fn resolve_site_geography<'site>( site_name: &str, sites: &'site [GridSite], @@ -115,7 +116,8 @@ fn resolve_site_geography<'site>( ) -> (Option<&'site str>, Option<&'site str>) { sites .iter() - .find(|s| s.metadata.name.as_deref() == Some(site_name) && s.spec.grid_network_ref == network_name) + .filter(|s| s.spec.grid_network_ref == network_name) + .find(|s| crate::controller::grid_network::peer_site_key(s).is_some_and(|(key, _)| key == site_name)) .map_or((None, None), |s| (s.spec.region.as_deref(), s.spec.zone.as_deref())) } @@ -181,6 +183,34 @@ mod tests { // Test utilities // ----------------------------------------------------------------------- + #[test] + fn a_discovered_site_is_found_by_its_site_id() { + let mut site = test_site_with_geography("grid-retail", "net", Some("us-east"), Some("a")); + site.metadata.labels = Some( + [( + crate::controller::grid_network::LABEL_AUTO_DISCOVERED.to_owned(), + "true".to_owned(), + )] + .into_iter() + .collect(), + ); + site.metadata.annotations = Some( + [( + crate::controller::grid_network::ANNOTATION_SITE_ID.to_owned(), + "retail".to_owned(), + )] + .into_iter() + .collect(), + ); + let sites = [site]; + assert_eq!( + resolve_site_geography("retail", &sites, "net"), + (Some("us-east"), Some("a")), + "candidates carry the site id" + ); + assert_eq!(resolve_site_geography("grid-retail", &sites, "net"), (None, None)); + } + fn test_site_with_geography(name: &str, network: &str, region: Option<&str>, zone: Option<&str>) -> GridSite { let mut spec = serde_json::json!({ "gridNetworkRef": network }); if let Some(r) = region { diff --git a/operator/src/resources/provider_metrics.rs b/operator/src/resources/provider_metrics.rs index f0146ea5e..5a30ad8b9 100644 --- a/operator/src/resources/provider_metrics.rs +++ b/operator/src/resources/provider_metrics.rs @@ -129,7 +129,10 @@ pub(crate) async fn collect_provider_signals( let Some(plan) = signal_scrape_plan(provider) else { continue; }; - let key = crate::readiness::key(network_name, plan.identity); + let Some(name) = provider.metadata.name.as_deref() else { + continue; + }; + let key = crate::readiness::key(network_name, name); match scrape_provider_signals(provider, &plan, client).await { Ok(text) => record_scrape(readiness, &key, provider, &plan, &text), Err(class) => { diff --git a/operator/src/resources/routing_overlay.rs b/operator/src/resources/routing_overlay.rs index c9797e924..abc9b2464 100644 --- a/operator/src/resources/routing_overlay.rs +++ b/operator/src/resources/routing_overlay.rs @@ -808,6 +808,7 @@ pub(crate) fn evaluate_access_policy( /// | `Known(names)` | Selector matched these sites | Emit one candidate per `(model, site)` | /// /// [`GridSite`]: crate::crd::grid_site::GridSite +#[derive(Debug, PartialEq, Eq)] enum SiteResolution { /// No [`GridSite`] CRDs were supplied to the renderer for this network. /// @@ -1185,7 +1186,8 @@ fn assign_selection_groups(candidates: &mut [RoutingCandidate], policy: crate::c /// Only [`InferenceProvider`]s whose `spec.gridNetworkRef` matches /// `network.metadata.name` are included. Each provider's /// `spec.siteSelector.matchLabels` is matched against the supplied -/// `sites`; an empty selector means `local_site` alone. +/// `sites`; an empty selector means `owning_site` alone, the site whose operator holds the +/// provider, or `local_site` when the caller names none. /// /// The `local_site` parameter identifies this gateway's own site. /// Praxis uses it to score candidates running on the local site higher @@ -1278,6 +1280,7 @@ pub fn render_routing_overlay( tool_providers, remote_crdt_providers, local_site, + None, metrics, generated_at, weights, @@ -1360,6 +1363,7 @@ pub fn render_routing_overlay_with_admission( tool_providers: &[AgentToolProvider], remote_crdt_providers: &[crdt::ProviderState], local_site: &str, + owning_site: Option<&str>, metrics: Option<&HashMap<&str, scoring::BackendMetrics>>, generated_at: Option<&str>, weights: &scoring::ScoringWeights, @@ -1394,7 +1398,13 @@ pub fn render_routing_overlay_with_admission( .find(|site| site.metadata.name.as_deref() == Some(local_site) && site.spec.grid_network_ref == network_name) .and_then(|site| site.metadata.labels.as_ref()); - let mut candidates = collect_candidates(network_name, sites, providers, consumer_site_labels, local_site)?; + let mut candidates = collect_candidates( + network_name, + sites, + providers, + consumer_site_labels, + owning_site.unwrap_or(local_site), + )?; candidates.extend(collect_tool_candidates( network_name, sites, @@ -1588,7 +1598,7 @@ fn collect_candidates( sites: &[GridSite], providers: &[InferenceProvider], consumer_site_labels: Option<&BTreeMap>, - local_site: &str, + owning_site: &str, ) -> Result, String> { // Pre-filter sites to those in this network. let network_sites: Vec<&GridSite> = sites @@ -1610,7 +1620,7 @@ fn collect_candidates( continue; } - let resolution = resolve_sites(provider, &network_sites, local_site); + let resolution = resolve_sites(provider, &network_sites, owning_site); all.extend(candidates_from_provider(provider, &resolution)?); } Ok(all) @@ -1676,10 +1686,11 @@ fn collect_tool_candidates( /// [`SiteResolution::Known`] otherwise — with an empty `Vec` if the /// selector matched nothing, which suppresses candidate generation. /// -/// An empty selector means this site alone, when the inventory lists it, as the -/// provider controller attributes it. Sites are keyed as peers are, so a -/// discovered peer reads as its site id, not its object name. -fn resolve_sites(provider: &InferenceProvider, network_sites: &[&GridSite], local_site: &str) -> SiteResolution { +/// An empty selector means the site that owns the provider alone, when the inventory +/// lists it, as the provider controller attributes it. That is the operator's own site, +/// not the site of the gateway the overlay is rendered for, which may differ. Sites are +/// keyed as peers are, so a discovered peer reads as its site id, not its object name. +fn resolve_sites(provider: &InferenceProvider, network_sites: &[&GridSite], owning_site: &str) -> SiteResolution { if network_sites.is_empty() { return SiteResolution::Unavailable; } @@ -1687,9 +1698,9 @@ fn resolve_sites(provider: &InferenceProvider, network_sites: &[&GridSite], loca let selector = &provider.spec.site_selector.match_labels; let key = |site: &&GridSite| peer_site_key(site).map(|(key, _)| key); if selector.is_empty() { - let listed = network_sites.iter().filter_map(key).any(|site| site == local_site); + let listed = network_sites.iter().filter_map(key).any(|site| site == owning_site); return SiteResolution::Known(if listed { - vec![local_site.to_owned()] + vec![owning_site.to_owned()] } else { Vec::new() }); @@ -2373,6 +2384,24 @@ mod tests { } } + #[test] + fn an_empty_selector_resolves_to_the_owning_site_not_the_gateway_site() { + let sites = [test_site("site-a", "net"), test_site("site-b", "net")]; + let network_sites: Vec<&GridSite> = sites.iter().collect(); + let provider = test_provider("p", "net", &["m"]); + // The operator at site-a owns the provider; the overlay is rendered for a gateway + // at site-b, and the candidate must still live at site-a. + assert_eq!( + resolve_sites(&provider, &network_sites, "site-a"), + SiteResolution::Known(vec!["site-a".to_owned()]) + ); + assert_eq!( + resolve_sites(&provider, &network_sites, "site-c"), + SiteResolution::Known(Vec::new()), + "an owning site the inventory does not list yields no candidate" + ); + } + fn test_provider_with_selector( name: &str, network: &str, diff --git a/operator/src/signals.rs b/operator/src/signals.rs index d6cdf17b6..3dea68b72 100644 --- a/operator/src/signals.rs +++ b/operator/src/signals.rs @@ -1289,6 +1289,18 @@ fn plausible(observation: &Observation) -> bool { && (observation.value <= 1.0 || !UNIT_SIGNALS.contains(&observation.metric.as_str())) } +/// Most bytes in a `grid_provider` label a hub keeps, as a routing cluster name is bounded. +const MAX_PROVIDER_LABEL_LEN: usize = 253; + +/// Whether `provider` names a routing cluster a hub can key on: non-blank, bounded, and free +/// of the store's `/` separator and control characters. Cluster names such as `pool.v1` are +/// valid, so this is wider than a site name. +fn valid_provider(provider: &str) -> bool { + !provider.trim().is_empty() + && provider.len() <= MAX_PROVIDER_LABEL_LEN + && !provider.chars().any(|ch| ch.is_control() || ch == '/') +} + /// Why a hub refuses `observation` from a peer, `None` when it accepts it. fn refusal(observation: &Observation) -> Option<&'static str> { if !PEER_SIGNAL_NAMES.contains(&observation.metric.as_str()) { @@ -1296,7 +1308,7 @@ fn refusal(observation: &Observation) -> Option<&'static str> { } else if observation .labels .get(PROVIDER_LABEL) - .is_none_or(|provider| certs::validate_site_name(provider).is_err()) + .is_none_or(|provider| !valid_provider(provider)) { Some("provider") } else if !plausible(observation) { @@ -1306,9 +1318,9 @@ fn refusal(observation: &Observation) -> Option<&'static str> { } } -/// Keep what a hub accepts from `peer`: an allowed name, a `grid_provider` that is a DNS-1123 -/// label, a plausible value, and at most [`MAX_PEER_PROVIDERS`] providers. Each refusal is -/// counted by reason. +/// Keep what a hub accepts from `peer`: an allowed name, a `grid_provider` that names a +/// routing cluster, a plausible value, and at most [`MAX_PEER_PROVIDERS`] providers. Each +/// refusal is counted by reason. fn bound_peer(observations: Vec, peer: &str) -> Vec { let mut kept = Vec::with_capacity(observations.len()); for observation in observations { @@ -1436,8 +1448,18 @@ mod tests { ), ("custom name", peer_sample("my_custom_queue", "pool", 3.0), false), ( - "provider not a label", - peer_sample(crate::readiness::READY_SIGNAL, "Pool_1", 1.0), + "provider with a dot, a valid cluster name", + peer_sample(crate::readiness::IN_FLIGHT_SIGNAL, "pool.v1", 3.0), + true, + ), + ( + "provider with the store separator", + peer_sample(crate::readiness::IN_FLIGHT_SIGNAL, "pool/v1", 3.0), + false, + ), + ( + "blank provider", + peer_sample(crate::readiness::IN_FLIGHT_SIGNAL, " ", 3.0), false, ), (