[amh1k]

Summary

Add benchmark runner placement controls with administrator-defined Helm settings and optional per-run node affinity.

Closes #40.

Changes

• Add runner.nodeSelector and runner.tolerations Helm values, applied to every runner Job.
• Add a Job placement editor with label key/value rows; all rows must match.
• Preserve the full nodeAffinity API contract for advanced scheduling rules.
• Validate placement settings and display saved affinity alongside run results.
• Default the scheduling grace period to 2m, configurable through Helm.
• Fail persistently unschedulable runs, clean up Jobs and credential Secrets, and release run capacity.
• Return generic scheduling errors to users and log detailed diagnostics on the backend.
• Document that pending time counts against the overall execution timeout.

Validation

• Backend tests pass.
• Frontend form, component, and status tests pass.
• Frontend production build and Helm lint pass.

[spron-in]

Main concern: the 60s scheduling failure applies to every run (see jobs.go). Also some test-tooling and typing nits; suggestions inline.

[spron-in]

To render components under node:test, this now compiles the whole BenchmarkForm.tsx graph with tsc (--jsx, --esModuleInterop, --skipLibCheck). That's why value imports in src/ now need .js extensions while type imports don't. The mixed style will spread to every component a test touches.

Vite is already the build tool, so Vitest + @testing-library/react (jsdom environment) would run the .tsx directly, with no compile step and no import-extension changes. Proposed split: keep the pure-logic tests on the existing runner and move the component test to Vitest:

"test:form": "tsc src/benchmarkFormValidation.ts --target ES2020 --module ESNext --moduleResolution Bundler --outDir dist/form-test && node tests/benchmarkForm.test.mjs && node tests/benchmarkNodeAffinity.test.mjs",
"test:components": "vitest run",

Then revert the .js suffixes in src/, port benchmarkNodeAffinityForm.test.mjs to a *.test.tsx, and add npm run test:components to ci.yml. (Not a one-click suggestion on purpose: applying the script change alone would silently drop the component test.)

[spron-in]

Nit: jsdom and react-test-renderer are pinned exactly while every other devDependency uses ^, and together they add ~800 lines to package-lock.json. react-test-renderer is also deprecated upstream; React recommends @testing-library/react. Ideally both get replaced by vitest + @testing-library/react (see the test:form comment). If the current runner stays, at least align the ranges:

Suggested change

	"jsdom": "24.1.3",
	"react": "^18.3.1",
	"react-dom": "^18.3.1",
	"react-test-renderer": "18.3.1",
	"jsdom": "^24.1.3",
	"react": "^18.3.1",
	"react-dom": "^18.3.1",
	"react-test-renderer": "^18.3.1",

[spron-in]

Follow-up suggestion: tolerations. Dedicated benchmark node pools are usually tainted (e.g. dedicated=benchmark:NoSchedule) to keep other workloads off. Node affinity alone can't place the runner there. With a required rule, the run would sit Unschedulable and fail.

Not a blocker for this PR, but could we track it in a follow-up issue? Rough scope:

• per-run tolerations (key/operator/value/effect/tolerationSeconds), validated like affinity
• possibly an operator-set allowlist of toleration keys, since a toleration lets a user schedule onto nodes the cluster admin reserved
• a section in the Job placement editor plus the run summary
• README note that affinity + tolerations together are needed for tainted pools

[amh1k]

Follow-up suggestion: tolerations. Dedicated benchmark node pools are usually tainted (e.g. dedicated=benchmark:NoSchedule) to keep other workloads off. Node affinity alone can't place the runner there. With a required rule, the run would sit Unschedulable and fail.

Not a blocker for this PR, but could we track it in a follow-up issue? Rough scope:

• per-run tolerations (key/operator/value/effect/tolerationSeconds), validated like affinity
• possibly an operator-set allowlist of toleration keys, since a toleration lets a user schedule onto nodes the cluster admin reserved
• a section in the Job placement editor plus the run summary
• README note that affinity + tolerations together are needed for tainted pools

Yes we definitely implement this. I have opened up issue #54 for this. Will open up a pr shortly for this

[spron-in]

Thanks! The backend validation, the scheduling-failure tracking, and cleanup look solid. Requested changes below. Most of this is removing code; the backend and API contract stay as they are.

Needs to change

1. Admin placement in Helm. Add runner.nodeSelector and runner.tolerations, applied to every runner Job. Affinity alone can't reach tainted, dedicated benchmark nodes, which is the usual setup for #40. Use nodeSelector rather than a Helm-level affinity: Kubernetes applies it together with the per-run nodeAffinity, so the two never need merging.
2. Simplify the per-run editor. Replace the AND/OR, weights, and Gt/Lt editor with a list of key = value rows, sent as a single required rule using In. Keep the nodeAffinity API field so a richer editor can come later without an API change. Drop most of the TypeScript copy of the Kubernetes label rules in benchmarkNodeAffinity.ts; basic checks are enough, and the backend's 400 messages can be shown for the rest.
3. Default schedulingGracePeriod to something like 2m, not 0. With 0, a run with an impossible rule stays pending for the full 5-minute timeout and holds one of the 2 run slots.
4. Hide scheduler messages from users. Messages like 0/5 nodes are available: ...had untolerated taint reveal cluster topology to users who may have no Kubernetes access. Return a generic "no node matches the placement rules" and log the details on the backend.

Minor

• The PR description still says unschedulable runs fail after 60 seconds; the code currently disables early failure by default.
• A blank label value submits In [""], which only matches nodes whose label value is empty. Treat it as a form error.
• Use stable IDs as React keys for groups and conditions instead of the array index.
• Show validation errors only after a field has been touched.
• The affinity is stored twice (Run.Request.NodeAffinity and Run.NodeAffinity); keep only the prepared copy.
• Document that pending time counts against the execution timeout.