Skip to content

Commit b42e5c3

Browse files
authored
fix(supervisor): count pods from a limit=1 list instead of an aggregate metric (#4442)
The pod-count backpressure source read `apiserver_storage_objects{resource="pods"}` from an apiserver `/metrics` scrape. That gauge is a periodically-refreshed cached count, and it is served by whichever apiserver replica the scrape lands on — replicas disagree with each other at the same instant, by enough to swamp the engage/release hysteresis band. Engage and release timing was therefore partly a function of scrape routing. This replaces it with a single `limit=1` list of the workload namespace and computes `remainingItemCount + items.length`. One pod object transferred, no informer, no watch cache. Two request-shape constraints are load-bearing and called out in the code: passing a label or field selector makes the apiserver omit `remainingItemCount` entirely, and setting `resourceVersion` serves a cached count rather than a quorum read. Neither is passed. `remainingItemCount` is only set when the list is truncated, so `_continue` is the truncation signal — if it is absent the returned page is the whole collection and `items.length` is already exact. If the list *is* truncated and the count is missing or implausible, the fetcher throws rather than guessing. Failure semantics are unchanged: a throw lands in the monitor's existing catch, exactly as the previous parse did. The hysteresis, verdict shape, and gauge are untouched. RBAC is unchanged — the existing role already grants `pods: list`. The `/metrics` non-resource grant in the deployment role becomes unused, and the scrape-timeout env var is now a slight misnomer. Both left alone deliberately: the grant may be wanted again for other apiserver signals, and renaming the var would need a coordinated config change for no behavioural gain. Tests cover the not-truncated, truncated, missing-count, negative-count and timeout paths.
1 parent f10bc23 commit b42e5c3

5 files changed

Lines changed: 124 additions & 101 deletions

File tree

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
area: supervisor
3+
type: improvement
4+
---
5+
6+
Self-hosted Kubernetes deployments now measure running-task count more accurately when deciding whether to pause pulling new work, so the safeguard engages closer to its configured thresholds.

apps/supervisor/src/backpressure/k8sPodCountSignalSource.test.ts

Lines changed: 31 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,52 +1,49 @@
11
import { describe, it, expect } from "vitest";
2-
import { parsePodCount, K8sPodCountSignalSource } from "./k8sPodCountSignalSource.js";
2+
import { K8sPodCountSignalSource } from "./k8sPodCountSignalSource.js";
3+
import { podCountFromList, withTimeout } from "../clients/kubernetes.js";
34

4-
describe("parsePodCount", () => {
5-
it("reads the pods object count", () => {
6-
const text = [
7-
"# HELP apiserver_storage_objects Number of stored objects",
8-
"# TYPE apiserver_storage_objects gauge",
9-
'apiserver_storage_objects{resource="pods"} 8421',
10-
'apiserver_storage_objects{resource="configmaps"} 17',
11-
].join("\n");
12-
expect(parsePodCount(text)).toBe(8421);
5+
describe("podCountFromList", () => {
6+
it("returns items.length when the list is not truncated", () => {
7+
expect(podCountFromList({ items: [{}], metadata: {} })).toBe(1);
138
});
149

15-
it("is tolerant of extra labels in any order", () => {
16-
const text = 'apiserver_storage_objects{group="",resource="pods",extra="x"} 12';
17-
expect(parsePodCount(text)).toBe(12);
10+
it("returns zero for an empty namespace", () => {
11+
expect(podCountFromList({ items: [], metadata: {} })).toBe(0);
1812
});
1913

20-
it("parses scientific notation", () => {
21-
const text = 'apiserver_storage_objects{resource="pods"} 1.2e+04';
22-
expect(parsePodCount(text)).toBe(12000);
14+
it("adds remainingItemCount when the list is truncated", () => {
15+
const list = { items: [{}], metadata: { _continue: "tok", remainingItemCount: 24492 } };
16+
expect(podCountFromList(list)).toBe(24493);
2317
});
2418

25-
it("throws when the pods metric is absent", () => {
26-
const text = 'apiserver_storage_objects{resource="configmaps"} 17';
27-
expect(() => parsePodCount(text)).toThrow(/not found/);
19+
it("throws when truncated but remainingItemCount is absent", () => {
20+
const list = { items: [{}], metadata: { _continue: "tok" } };
21+
expect(() => podCountFromList(list)).toThrow(/remainingItemCount/);
2822
});
2923

30-
it("throws on a non-finite value (e.g. 1e999)", () => {
31-
const text = 'apiserver_storage_objects{resource="pods"} 1e999';
32-
expect(() => parsePodCount(text)).toThrow();
24+
it("throws when truncated but remainingItemCount is negative", () => {
25+
const list = { items: [{}], metadata: { _continue: "tok", remainingItemCount: -1 } };
26+
expect(() => podCountFromList(list)).toThrow(/remainingItemCount/);
3327
});
28+
});
3429

35-
it("throws on a negative value", () => {
36-
const text = 'apiserver_storage_objects{resource="pods"} -5';
37-
expect(() => parsePodCount(text)).toThrow();
30+
describe("withTimeout", () => {
31+
it("rejects once the deadline passes", async () => {
32+
await expect(withTimeout(new Promise(() => {}), 10, "pod count list")).rejects.toThrow(
33+
/timed out/
34+
);
3835
});
39-
});
4036

41-
function metrics(count: number): string {
42-
return `apiserver_storage_objects{resource="pods"} ${count}`;
43-
}
37+
it("passes a value through when it settles first", async () => {
38+
await expect(withTimeout(Promise.resolve(7), 1000, "pod count list")).resolves.toBe(7);
39+
});
40+
});
4441

4542
describe("K8sPodCountSignalSource", () => {
4643
it("engages at the engage threshold and reports the count", async () => {
4744
const counts: number[] = [];
4845
const source = new K8sPodCountSignalSource({
49-
fetchMetrics: async () => metrics(10000),
46+
fetchPodCount: async () => 10000,
5047
engageThreshold: 10000,
5148
releaseThreshold: 5000,
5249
reportPodCount: (c) => counts.push(c),
@@ -59,7 +56,7 @@ describe("K8sPodCountSignalSource", () => {
5956

6057
it("does not engage below the engage threshold", async () => {
6158
const source = new K8sPodCountSignalSource({
62-
fetchMetrics: async () => metrics(9999),
59+
fetchPodCount: async () => 9999,
6360
engageThreshold: 10000,
6461
releaseThreshold: 5000,
6562
});
@@ -69,7 +66,7 @@ describe("K8sPodCountSignalSource", () => {
6966
it("stays engaged in the hysteresis band, releases only below release threshold", async () => {
7067
let count = 10000;
7168
const source = new K8sPodCountSignalSource({
72-
fetchMetrics: async () => metrics(count),
69+
fetchPodCount: async () => count,
7370
engageThreshold: 10000,
7471
releaseThreshold: 5000,
7572
});
@@ -82,9 +79,9 @@ describe("K8sPodCountSignalSource", () => {
8279
expect((await source.read()).engaged).toBe(false); // band again -> stays off
8380
});
8481

85-
it("propagates scrape failures (monitor fails open on throw)", async () => {
82+
it("propagates fetch failures (monitor fails open on throw)", async () => {
8683
const source = new K8sPodCountSignalSource({
87-
fetchMetrics: async () => {
84+
fetchPodCount: async () => {
8885
throw new Error("connection refused");
8986
},
9087
engageThreshold: 10000,

apps/supervisor/src/backpressure/k8sPodCountSignalSource.ts

Lines changed: 2 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,7 @@
11
import type { BackpressureSignalSource, BackpressureVerdict } from "./backpressureMonitor.js";
22

3-
// Reads the apiserver's stored-pod-object count from a Prometheus /metrics scrape.
4-
const POD_COUNT_RE = /^apiserver_storage_objects\{[^}]*resource="pods"[^}]*\}\s+([0-9.eE+]+)/m;
5-
6-
export function parsePodCount(metricsText: string): number {
7-
const match = metricsText.match(POD_COUNT_RE);
8-
if (!match) {
9-
throw new Error('apiserver_storage_objects{resource="pods"} not found in metrics');
10-
}
11-
const value = Number(match[1]);
12-
if (!Number.isFinite(value)) {
13-
throw new Error(`unparseable pod count: ${match[1]}`);
14-
}
15-
return value;
16-
}
17-
183
export type K8sPodCountSignalSourceOptions = {
19-
fetchMetrics: () => Promise<string>;
4+
fetchPodCount: () => Promise<number>;
205
engageThreshold: number;
216
releaseThreshold: number;
227
reportPodCount?: (count: number) => void;
@@ -29,8 +14,7 @@ export class K8sPodCountSignalSource implements BackpressureSignalSource {
2914
constructor(private readonly opts: K8sPodCountSignalSourceOptions) {}
3015

3116
async read(): Promise<BackpressureVerdict> {
32-
const text = await this.opts.fetchMetrics();
33-
const count = parsePodCount(text);
17+
const count = await this.opts.fetchPodCount();
3418
this.opts.reportPodCount?.(count);
3519

3620
if (this.engaged) {

apps/supervisor/src/clients/kubernetes.ts

Lines changed: 80 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ import * as k8s from "@kubernetes/client-node";
22
import type { Informer, KubernetesObject, ListPromise } from "@kubernetes/client-node";
33
import { assertExhaustive } from "@trigger.dev/core/utils";
44
import { SimpleStructuredLogger } from "@trigger.dev/core/v3/utils/structuredLogger";
5-
import * as https from "node:https";
65

76
export const RUNTIME_ENV = process.env.KUBERNETES_PORT ? "kubernetes" : "local";
87

@@ -54,55 +53,90 @@ function getKubeConfig() {
5453
export { k8s };
5554

5655
/**
57-
* Builds a function that scrapes the apiserver's Prometheus /metrics endpoint.
58-
* One lightweight aggregate read - not a pod listing. Requires the service
59-
* account to be granted GET on the /metrics non-resource URL.
56+
* createPodCountFetcher sizes a namespace's pod collection with a single `limit=1`
57+
* list: one pod transferred, no informer, no watch cache.
58+
*
59+
* This is an ESTIMATE, not an exact count. Kubernetes documents `remainingItemCount`
60+
* as intended for estimating collection size and reserves the right not to set it or
61+
* make it exact. Counting exactly would mean paginating the whole collection, which is
62+
* what this deliberately avoids. Treat the value as a tight estimate from a quorum read
63+
* at request time, and set thresholds with that in mind.
64+
*
65+
* Two request-shape constraints, both load-bearing. A label or field selector makes
66+
* the apiserver omit `remainingItemCount` entirely, and setting `resourceVersion`
67+
* serves a cached count instead of a quorum read - so neither is passed.
6068
*/
61-
export function createApiserverMetricsFetcher(timeoutMs: number): () => Promise<string> {
62-
const kubeConfig = getKubeConfig();
69+
export function createPodCountFetcher(
70+
api: K8sApi,
71+
namespace: string,
72+
timeoutMs: number
73+
): () => Promise<number> {
74+
const serverTimeoutSeconds = Math.max(1, Math.floor(timeoutMs / 1000));
75+
let pending: Promise<unknown> | undefined;
6376

6477
return async () => {
65-
const cluster = kubeConfig.getCurrentCluster();
66-
if (!cluster) {
67-
throw new Error("no current cluster in kubeconfig");
78+
if (pending) {
79+
throw new Error("pod count list still in flight from a previous tick");
6880
}
69-
const url = new URL(`${cluster.server}/metrics`);
70-
const opts: https.RequestOptions = {
71-
method: "GET",
72-
protocol: url.protocol,
73-
hostname: url.hostname,
74-
port: url.port,
75-
path: url.pathname,
76-
};
77-
// applyToHTTPSOptions sets the cluster CA, client cert/key, and auth headers
78-
// (incl. exec plugins) on the request - so TLS verifies against the cluster
79-
// CA, not the system store. The fetch-options path attaches the CA as an
80-
// https.Agent, which global fetch (undici) ignores.
81-
await kubeConfig.applyToHTTPSOptions(opts);
82-
83-
return new Promise<string>((resolve, reject) => {
84-
const req = https.request(opts, (res) => {
85-
const status = res.statusCode ?? 0;
86-
let body = "";
87-
res.setEncoding("utf8");
88-
res.on("data", (chunk) => {
89-
body += chunk;
90-
});
91-
res.on("end", () => {
92-
if (status >= 200 && status < 300) {
93-
resolve(body);
94-
} else {
95-
reject(new Error(`apiserver /metrics scrape failed: ${status}`));
96-
}
97-
});
98-
});
99-
// Without this a hung connect/TLS/read never settles, and the monitor's
100-
// refreshInFlight guard would freeze the source (silent fail-open).
101-
req.setTimeout(timeoutMs, () => {
102-
req.destroy(new Error(`apiserver /metrics scrape timed out after ${timeoutMs}ms`));
103-
});
104-
req.on("error", reject);
105-
req.end();
81+
82+
const request = api.core.listNamespacedPod({
83+
namespace,
84+
limit: 1,
85+
timeoutSeconds: serverTimeoutSeconds,
10686
});
87+
88+
pending = request
89+
.catch(() => {})
90+
.finally(() => {
91+
pending = undefined;
92+
});
93+
94+
return podCountFromList(await withTimeout(request, timeoutMs, "pod count list"));
10795
};
10896
}
97+
98+
/**
99+
* podCountFromList turns a `limit=1` pod list into a population estimate.
100+
*
101+
* `remainingItemCount` is only set when the list is truncated, so `_continue` is the
102+
* truncation signal: absent means the returned page is the whole collection and its
103+
* length is exact. When truncated the total leans on `remainingItemCount`, which is
104+
* documented as an estimate - so the result is an estimate too. Truncated without a
105+
* usable count is unknowable, so it throws rather than returning a low number the
106+
* caller would act on.
107+
*/
108+
export function podCountFromList(list: {
109+
items: unknown[];
110+
metadata?: { _continue?: string; remainingItemCount?: number };
111+
}): number {
112+
if (!list.metadata?._continue) {
113+
return list.items.length;
114+
}
115+
116+
const remaining = list.metadata.remainingItemCount;
117+
if (typeof remaining !== "number" || !Number.isFinite(remaining) || remaining < 0) {
118+
throw new Error("pod list truncated but remainingItemCount absent or invalid");
119+
}
120+
121+
return list.items.length + remaining;
122+
}
123+
124+
/**
125+
* withTimeout rejects if `promise` outlives `timeoutMs`, so a hung request cannot
126+
* freeze the caller. It cannot cancel: the k8s client threads no AbortSignal through to
127+
* fetch, so an abandoned request keeps running. Callers must therefore also bound the
128+
* request server-side (`timeoutSeconds`) and refuse to start a second one while the
129+
* first is pending, or a blackholed connection accumulates one socket per attempt.
130+
*/
131+
export function withTimeout<T>(promise: Promise<T>, timeoutMs: number, what: string): Promise<T> {
132+
let timer: NodeJS.Timeout;
133+
const deadline = new Promise<never>((_resolve, reject) => {
134+
timer = setTimeout(
135+
() => reject(new Error(`${what} timed out after ${timeoutMs}ms`)),
136+
timeoutMs
137+
);
138+
timer.unref();
139+
});
140+
141+
return Promise.race([promise, deadline]).finally(() => clearTimeout(timer));
142+
}

apps/supervisor/src/index.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ import {
2121
CheckpointClient,
2222
isKubernetesEnvironment,
2323
} from "@trigger.dev/core/v3/serverOnly";
24-
import { createK8sApi, createApiserverMetricsFetcher } from "./clients/kubernetes.js";
24+
import { createK8sApi, createPodCountFetcher } from "./clients/kubernetes.js";
2525
import { collectDefaultMetrics, Counter, Gauge, Histogram } from "prom-client";
2626
import { register } from "./metrics.js";
2727
import { PodCleaner } from "./services/podCleaner.js";
@@ -276,14 +276,16 @@ class ManagedSupervisor {
276276
// RELEASE < ENGAGE is enforced in env.ts (superRefine), so it's valid here.
277277
const podCountGauge = new Gauge({
278278
name: "supervisor_cluster_pod_count",
279-
help: "Total pod objects stored in the cluster, scraped for backpressure",
279+
help: "Pod objects in the workload namespace, counted for backpressure",
280280
registers: [register],
281281
});
282282
this.backpressureMonitors.push(
283283
new BackpressureMonitor({
284284
enabled: true,
285285
source: new K8sPodCountSignalSource({
286-
fetchMetrics: createApiserverMetricsFetcher(
286+
fetchPodCount: createPodCountFetcher(
287+
createK8sApi(),
288+
env.KUBERNETES_NAMESPACE,
287289
env.TRIGGER_DEQUEUE_BACKPRESSURE_POD_COUNT_SCRAPE_TIMEOUT_MS
288290
),
289291
engageThreshold: env.TRIGGER_DEQUEUE_BACKPRESSURE_POD_COUNT_ENGAGE,

0 commit comments

Comments
 (0)