Skip to content

Commit 1d89575

Browse files
committed
feat(supervisor): harden org placement override parsing and visibility
Blank env value now means no overrides instead of failing startup. Tolerations also accept an array of entries to match the Helm list shape, scalar node selector values are coerced to strings, keys and values are trimmed, and empty selector values are rejected since they would pin the org to nothing. Startup fails when an override pins the large-machine pool while non-large presets are required to stay off it. The supervisor logs configured override orgs at startup and warns when an override replaces a default node selector key.
1 parent 3d67620 commit 1d89575

6 files changed

Lines changed: 162 additions & 37 deletions

File tree

apps/supervisor/src/env.ts

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -266,9 +266,10 @@ export const Env = z
266266
KUBERNETES_RUNNER_TOLERATIONS: Tolerations.optional(), // every run pod
267267
KUBERNETES_SCHEDULED_RUN_TOLERATIONS: Tolerations.optional(), // schedule-tree runs only
268268

269-
// Per-org placement overrides, JSON keyed by org id:
270-
// {"<orgId>": {"nodeSelector": {"<key>": "<value>"}, "tolerations": "<csv>"}}
271-
KUBERNETES_ORG_PLACEMENT_OVERRIDES: OrgPlacementOverrides.optional(),
269+
// Per-org placement overrides, JSON keyed by the internal org id
270+
// (the `org` label on run pods):
271+
// {"<orgId>": {"nodeSelector": {"<key>": "<value>"}, "tolerations": "<csv or array>"}}
272+
KUBERNETES_ORG_PLACEMENT_OVERRIDES: OrgPlacementOverrides,
272273

273274
// Placement tags settings
274275
PLACEMENT_TAGS_ENABLED: BoolEnv.default(false),
@@ -315,6 +316,22 @@ export const Env = z
315316
path: ["TRIGGER_DEQUEUE_BACKPRESSURE_POD_COUNT_RELEASE"],
316317
});
317318
}
319+
if (data.KUBERNETES_LARGE_MACHINE_AFFINITY_ENABLED && data.KUBERNETES_ORG_PLACEMENT_OVERRIDES) {
320+
// Non-large presets carry a hard NotIn on the large-machine pool, so an org
321+
// pinned to that pool could never schedule its non-large runs.
322+
for (const [orgId, override] of Object.entries(data.KUBERNETES_ORG_PLACEMENT_OVERRIDES)) {
323+
const pinnedPool =
324+
override.nodeSelector?.[data.KUBERNETES_LARGE_MACHINE_AFFINITY_POOL_LABEL_KEY];
325+
326+
if (pinnedPool === data.KUBERNETES_LARGE_MACHINE_AFFINITY_POOL_LABEL_VALUE) {
327+
ctx.addIssue({
328+
code: z.ZodIssueCode.custom,
329+
message: `Org "${orgId}" pins run pods to the large-machine pool, but non-large presets are required to stay off it, so those runs would never schedule. Use a different pool or disable KUBERNETES_LARGE_MACHINE_AFFINITY_ENABLED.`,
330+
path: ["KUBERNETES_ORG_PLACEMENT_OVERRIDES"],
331+
});
332+
}
333+
}
334+
}
318335
if (data.COMPUTE_SNAPSHOTS_ENABLED && !data.TRIGGER_METADATA_URL) {
319336
ctx.addIssue({
320337
code: z.ZodIssueCode.custom,

apps/supervisor/src/envUtil.test.ts

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -251,6 +251,53 @@ describe("OrgPlacementOverrides", () => {
251251
}
252252
});
253253

254+
it("should treat a blank or missing value as no overrides, like the sibling settings", () => {
255+
expect(OrgPlacementOverrides.parse(undefined)).toBeUndefined();
256+
expect(OrgPlacementOverrides.parse("")).toBeUndefined();
257+
expect(OrgPlacementOverrides.parse(" ")).toBeUndefined();
258+
});
259+
260+
it("should accept tolerations as an array of entries, matching the Helm list shape", () => {
261+
expect(
262+
OrgPlacementOverrides.parse(
263+
JSON.stringify({
264+
org_123: { tolerations: ["dedicated=pool:NoSchedule", "spot:NoExecute"] },
265+
})
266+
)
267+
).toEqual({
268+
org_123: {
269+
tolerations: [
270+
{ key: "dedicated", operator: "Equal", value: "pool", effect: "NoSchedule" },
271+
{ key: "spot", operator: "Exists", effect: "NoExecute" },
272+
],
273+
},
274+
});
275+
});
276+
277+
it("should coerce scalar node selector values to strings, as Kubernetes labels are", () => {
278+
expect(
279+
OrgPlacementOverrides.parse(
280+
JSON.stringify({ org_123: { nodeSelector: { paid: true, replicas: 3 } } })
281+
)
282+
).toEqual({ org_123: { nodeSelector: { paid: "true", replicas: "3" } } });
283+
});
284+
285+
it("should trim whitespace around node selector keys and values", () => {
286+
expect(
287+
OrgPlacementOverrides.parse(JSON.stringify({ org_123: { nodeSelector: { " pool ": " a " } } }))
288+
).toEqual({ org_123: { nodeSelector: { pool: "a" } } });
289+
});
290+
291+
it("should reject an empty node selector value instead of pinning the org to nothing", () => {
292+
for (const value of ["", " "]) {
293+
expect(
294+
OrgPlacementOverrides.safeParse(
295+
JSON.stringify({ org_123: { nodeSelector: { pool: value } } })
296+
).success
297+
).toBe(false);
298+
}
299+
});
300+
254301
it("should reject an unknown field, so a typo cannot silently drop an override", () => {
255302
expect(
256303
OrgPlacementOverrides.safeParse(

apps/supervisor/src/envUtil.ts

Lines changed: 68 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -146,34 +146,68 @@ export const Tolerations = z.string().transform((val, ctx) => {
146146
});
147147
});
148148

149-
const NodeSelector = z.record(z.string(), z.string()).superRefine((selector, ctx) => {
150-
for (const [key, value] of Object.entries(selector)) {
151-
if (!isQualifiedName(key)) {
152-
ctx.addIssue({
153-
code: z.ZodIssueCode.custom,
154-
message: `Invalid node selector key "${key}". Must be a Kubernetes label key, optionally prefixed with a DNS subdomain.`,
155-
});
156-
}
149+
/**
150+
* Scalar values are coerced: YAML/JSON easily produce `true` or `3` where a label
151+
* value is meant, and Kubernetes label values are always strings. An empty value
152+
* is rejected rather than passed through - as a selector it matches only nodes
153+
* carrying a literal empty-valued label, which pins the org to nothing.
154+
*/
155+
const NodeSelector = z
156+
.record(z.string(), z.union([z.string(), z.number(), z.boolean()]))
157+
.transform((selector, ctx) => {
158+
const result: Record<string, string> = {};
157159

158-
if (!isLabelValue(value)) {
159-
ctx.addIssue({
160-
code: z.ZodIssueCode.custom,
161-
message: `Invalid node selector value "${value}" for key "${key}". Must be a Kubernetes label value: alphanumeric, with dashes, underscores and dots inside, at most 63 characters.`,
162-
});
160+
for (const [rawKey, rawValue] of Object.entries(selector)) {
161+
const key = rawKey.trim();
162+
const value = String(rawValue).trim();
163+
164+
if (!isQualifiedName(key)) {
165+
ctx.addIssue({
166+
code: z.ZodIssueCode.custom,
167+
message: `Invalid node selector key "${rawKey}". Must be a Kubernetes label key, optionally prefixed with a DNS subdomain.`,
168+
});
169+
continue;
170+
}
171+
172+
if (!value) {
173+
ctx.addIssue({
174+
code: z.ZodIssueCode.custom,
175+
message: `Empty node selector value for key "${key}". Remove the key instead of blanking the value.`,
176+
});
177+
continue;
178+
}
179+
180+
if (!isLabelValue(value)) {
181+
ctx.addIssue({
182+
code: z.ZodIssueCode.custom,
183+
message: `Invalid node selector value "${value}" for key "${key}". Must be a Kubernetes label value: alphanumeric, with dashes, underscores and dots inside, at most 63 characters.`,
184+
});
185+
continue;
186+
}
187+
188+
result[key] = value;
163189
}
164-
}
165-
});
190+
191+
return result;
192+
});
166193

167194
/**
168-
* Per-organization placement overrides for run pods, as JSON keyed by org id:
195+
* Per-organization placement overrides for run pods, as JSON keyed by the
196+
* internal org id (the `org` label on run pods):
169197
* `{"<orgId>": {"nodeSelector": {"<key>": "<value>"}, "tolerations": "<csv>"}}`.
170-
* Tolerations use the same CSV format as `Tolerations`. Everything is validated
171-
* at startup for the same reason as tolerations above: a typo would otherwise
172-
* reject every pod create for that org, with the cause buried in API errors.
198+
* Tolerations use the same CSV format as `Tolerations`, or an array of such
199+
* entries. Everything is validated at startup for the same reason as
200+
* tolerations above: a typo would otherwise reject every pod create for that
201+
* org, with the cause buried in API errors. A blank value means no overrides.
173202
*/
174203
export const OrgPlacementOverrides = z
175204
.string()
205+
.optional()
176206
.transform((val, ctx) => {
207+
if (val === undefined || val.trim() === "") {
208+
return undefined;
209+
}
210+
177211
try {
178212
return JSON.parse(val) as unknown;
179213
} catch {
@@ -185,15 +219,21 @@ export const OrgPlacementOverrides = z
185219
}
186220
})
187221
.pipe(
188-
z.record(
189-
z.string().min(1),
190-
z
191-
.object({
192-
nodeSelector: NodeSelector.optional(),
193-
tolerations: Tolerations.optional(),
194-
})
195-
.strict()
196-
)
222+
z
223+
.record(
224+
z.string().min(1),
225+
z
226+
.object({
227+
nodeSelector: NodeSelector.optional(),
228+
tolerations: z
229+
.union([z.string(), z.array(z.string())])
230+
.transform((val) => (Array.isArray(val) ? val.join(",") : val))
231+
.pipe(Tolerations)
232+
.optional(),
233+
})
234+
.strict()
235+
)
236+
.optional()
197237
);
198238

199239
export const AdditionalEnvVars = z.preprocess((val) => {

apps/supervisor/src/workloadManager/kubernetes.ts

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,12 @@ export class KubernetesWorkloadManager implements WorkloadManager {
7070
domain: opts.workloadApiDomain,
7171
});
7272
}
73+
74+
if (env.KUBERNETES_ORG_PLACEMENT_OVERRIDES) {
75+
this.logger.info("[KubernetesWorkloadManager] Org placement overrides enabled", {
76+
orgIds: Object.keys(env.KUBERNETES_ORG_PLACEMENT_OVERRIDES),
77+
});
78+
}
7379
}
7480

7581
private addPlacementTags(
@@ -112,10 +118,23 @@ export class KubernetesWorkloadManager implements WorkloadManager {
112118

113119
try {
114120
const orgOverride = env.KUBERNETES_ORG_PLACEMENT_OVERRIDES?.[opts.orgId];
115-
const basePodSpec = withNodeSelector(
116-
this.addPlacementTags(this.#defaultPodSpec, opts.placementTags),
117-
orgOverride?.nodeSelector
118-
);
121+
const taggedPodSpec = this.addPlacementTags(this.#defaultPodSpec, opts.placementTags);
122+
const basePodSpec = withNodeSelector(taggedPodSpec, orgOverride?.nodeSelector);
123+
124+
if (orgOverride?.nodeSelector) {
125+
const replacedKeys = Object.keys(orgOverride.nodeSelector).filter(
126+
(key) =>
127+
taggedPodSpec.nodeSelector?.[key] !== undefined &&
128+
taggedPodSpec.nodeSelector[key] !== orgOverride.nodeSelector?.[key]
129+
);
130+
131+
if (replacedKeys.length > 0) {
132+
this.logger.warn(
133+
"[KubernetesWorkloadManager] Org placement override replaces node selector keys",
134+
{ orgId: opts.orgId, replacedKeys }
135+
);
136+
}
137+
}
119138
const podSpec = this.opts.checkpointsEnabled
120139
? withBlockIoUringSeccompProfile(basePodSpec, opts.runtime)
121140
: basePodSpec;

docs/self-hosting/env/supervisor.mdx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ mode: "wide"
4848
| `KUBERNETES_NAMESPACE` | No | default | The namespace that runs should be in. |
4949
| `KUBERNETES_WORKER_NODETYPE_LABEL` | No | v4-worker | Nodes for runs need `nodetype=<this>`. Empty: any node. |
5050
| `KUBERNETES_RUNNER_TOLERATIONS` | No || Run pod tolerations. CSV: `key=value:effect`/`key:effect`. |
51-
| `KUBERNETES_ORG_PLACEMENT_OVERRIDES` | No || Per-org node selector and tolerations. JSON keyed by org ID.|
51+
| `KUBERNETES_ORG_PLACEMENT_OVERRIDES` | No || Per-org node selector and tolerations for run pods, merged over the defaults. JSON keyed by the org's internal ID (the `org` label on run pods). |
5252
| `KUBERNETES_IMAGE_PULL_SECRETS` | No || Image pull secrets (CSV). |
5353
| `KUBERNETES_EPHEMERAL_STORAGE_SIZE_LIMIT` | No | 10Gi | Ephemeral storage size limit. Applies to all runs. |
5454
| `KUBERNETES_EPHEMERAL_STORAGE_SIZE_REQUEST` | No | 2Gi | Ephemeral storage size request. Applies to all runs. |

hosting/k8s/helm/values.yaml

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -297,7 +297,9 @@ supervisor:
297297
namespace: "" # Default: uses release namespace
298298
workerNodetypeLabel: "" # When set, runs will only be scheduled on nodes with "nodetype=<label>"
299299
runnerTolerations: [] # Run pod tolerations, e.g. ["dedicated=runs:NoSchedule"]
300-
orgPlacementOverrides: {} # Per-org run pod placement, e.g. {"<orgId>": {"nodeSelector": {"pool": "dedicated"}, "tolerations": "dedicated=runs:NoSchedule"}}
300+
# Per-org run pod placement, keyed by the org's internal ID (the `org` label on run pods).
301+
# Merged over the default node selector. e.g. {"<orgId>": {"nodeSelector": {"pool": "dedicated"}, "tolerations": ["dedicated=runs:NoSchedule"]}}
302+
orgPlacementOverrides: {}
301303
ephemeralStorageSizeLimit: "" # Default: 10Gi
302304
ephemeralStorageSizeRequest: "" # Default: 2Gi´
303305
podCleaner:

0 commit comments

Comments
 (0)