Skip to content

fix: a failed CLEAN run was reported as completed with no results - #161

Open
matthewberry wants to merge 2 commits into
mjberry/fix/postjob-exit-codefrom
mjberry/fix/clean-pipefail
Open

matthewberry wants to merge 2 commits into
mjberry/fix/postjob-exit-codefrom
mjberry/fix/clean-pipefail

Conversation

@matthewberry

@matthewberry matthewberry commented Nov 5, 2026 •

Copy link
Copy Markdown
Collaborator

Stacked on #160. This changes app/postjob.py after #160's upload handling, so merge #160 first. This PR's base then switches to main.

Problem

A CLEAN run that crashes is reported as completed. The user gets a "ready" email, and the results page shows an error because there is no _maxsep.csv.

The CLEAN command (app/services/clean_service.py) runs:

python CLEAN_infer_fasta.py … 2>&1 | tee $JOB_OUTPUT_DIR/log

Without pipefail, a pipeline's exit status is that of its last command, tee, which succeeds. So when the inference fails:

  1. the || touch $JOB_OUTPUT_DIR/error fallback never runs;
  2. the job template's && touch "${JOB_OUTPUT_DIR}/finished" (job.tpl.yaml) does run;
  3. the job container exits 0, post-job uploads finished and log, and the watcher marks the job completed.

This happened to job f289fd27… on 2026-11-04. Its log ends with RuntimeError: torch.cat(): expected a non-empty list of Tensors. It left finished and log in MinIO, with no error and no _maxsep.csv. (Why that run crashed is a separate question. It isn't addressed here.)

CRISPR-Copies and Mutagenesis use the same | tee log pattern and already set pipefail. A word search for tee across the repo finds only these three commands, and CLEAN was the only one without it.

Why the Job is failed in post-job, not in the job container

Adding pipefail alone (this PR's first commit) makes the job container exit non-zero, but that loses the log:

  • in job.tpl.yaml, job is an init container, and post-job (postjob.py, which uploads the output directory) is the pod's main container;
  • Kubernetes starts main containers only after every init container succeeds, so post-job never runs;
  • error and log stay on the shared volume and never reach MinIO. The f289fd27 diagnosis above was only possible because its log was uploaded.

So a failed inference writes error and exits 0 instead, and post-job fails the Job after uploading.

Change

  • clean_service.py: prefix the command with set -o pipefail && , and change the fallback from || (touch error && false) to || touch error. A failed inference now writes error and exits 0; the template then touches finished.
  • postjob.py: after the upload, exit 1 if $JOB_OUTPUT_DIR/error exists. With restartPolicy: Never and backoffLimit: 0, the Job fails; the watcher marks the job error and sends the "failed" email.
  • If the upload itself fails, fix: exit non-zero when postjob fails to upload a job's outputs #160 already exits 1 before this check, so the job is still marked error.
  • Other tools: CRISPR-Copies and Mutagenesis still exit non-zero from the job container (&& false), so their behavior is unchanged. Tools that never write error are unaffected.
  • A failed CLEAN run now uploads error, finished and log. Nothing in app/ reads finished or error from MinIO (grepped), but I haven't checked the frontend.

Deploy note

post-job always runs moleculemaker/mmli-backend:main (hardcoded in job.tpl.yaml, imagePullPolicy: Always), whatever backend version created the Job. A backend running this branch's CLEAN command against a :main image without the new postjob.py check would mark failed runs completed. Don't deploy this branch to an environment before :main contains the postjob.py change.

Testing

  • tests/test_clean_job_exit_code.py:
    • What it runs: it renders the real job template through create_job with the real build_clean_job_command output, as test_job_shm.py does. It then runs the job init container's bash -c script, with a stand-in python on PATH.
    • Inference exits 1, or is killed with SIGKILL as an OOM kill would do: the container exits 0, and the output directory holds error, finished and log.
    • Inference succeeds: the container exits 0, with _maxsep.csv, finished and log (no error).
    • Without pipefail: both failure cases fail (no error).
  • tests/test_postjob_exit_code.py (from fix: exit non-zero when postjob fails to upload a job's outputs #160) gains one case: with error present, the upload runs and then the script exits 1. It fails with the check removed.
  • Full suite in the Dockerfile.test image: 630 passed.
  • Not run against a real CLEAN image or on the cluster.

The CLEAN command pipes the inference through `tee` into the job log, so without
`set -o pipefail` the pipeline's status was tee's. A crash skipped the `error` sentinel,
the job template touched `finished`, and the job was marked completed.
…tead

With pipefail, a failed inference exited the `job` container non-zero. That container
is an init container, so post-job never ran and the log (and `error`) never reached
MinIO; the only record of the crash was on the shared volume.

A failed inference now writes `error` and exits 0, so post-job uploads the outputs.
postjob.py then exits 1 when `error` is present, which fails the Job and the watcher
marks it 'error' as before. CRISPR-Copies and Mutagenesis still exit non-zero and are
unchanged.
@matthewberry
matthewberry force-pushed the mjberry/fix/clean-pipefail branch from 599d052 to 829ea8d Compare November 5, 2026 15:36
@matthewberry
matthewberry changed the base branch from main to mjberry/fix/postjob-exit-code November 5, 2026 15:36
matthewberry added a commit that referenced this pull request Nov 5, 2026
fetch_organism.py and convert_annotations.py run as the crispr-copies prejob_command in
the backend's init container. A failure there failed that container, so the job
container and post-job never ran and the traceback reached no one.

- prejob_command now tees the setup scripts' output to `log` and touches `error` on
  failure, exiting 0 (in chart/values.yaml, app/cfg/config.yaml and
  local.config.yaml).
- The CRISPR-Copies command skips the tool when `error` already exists, and appends
  to `log` (tee -a) so the setup output is kept on every path.
- post-job then uploads `log` and `error`, and #161's check fails the Job.

Tests run the rendered init container and then the job container, sharing one output
directory: a failed fetch or convert keeps its traceback and skips the tool; a
successful or failed tool run keeps the setup output too. Reverting either the
prejob_command or the command fails four of them. A further test checks the chart's
prejob_command matches app/cfg/config.yaml.
matthewberry added a commit that referenced this pull request Nov 5, 2026
…st-job `fail`

Review on #171: checking `fail` in postjob.py applied it to every job type,
though only the novostoic commands write it. The four commands now write
`error`, which postjob.py already fails the Job on, so postjob.py is back to
#161's version. The new test renders each novostoic command from the chart
through the real job template, and builds the job type list from the chart.
matthewberry added a commit to moleculemaker/clean-frontend that referenced this pull request Nov 5, 2026
…load

getResponseResult() only logged an error from the results request, so
contentLoaded stayed false and the loader spun forever. That happens for a
completed CLEAN job with no _maxsep.csv: the backend's results endpoint
answers 404. Jobs like that already exist (crashed runs that were marked
completed before moleculemaker/mmli-backend#161).

Set failedJob and contentLoaded in that error handler, as the status-polling
error handler already does.
@matthewberry

Copy link
Copy Markdown
Collaborator Author

Review of 829ea8d. No blocking findings.

Ran: Dockerfile.test image built from this branch. The 7 tests in test_postjob_exit_code.py and test_clean_job_exit_code.py pass, and the full suite passes (630). Not run against a real CLEAN image or on the cluster.

Checked and cleared

  • pipefail reaches the pipeline. The template's folded bash -c script is ls -al ${JOB_INPUT_DIR} && set -o pipefail && … | tee …. Because it's all one && chain in a single shell, the option is set before the | tee pipeline runs.
  • A failed setup step still fails the container. If echo | base64 -d > … fails, the outer && skips the whole group, || touch error included. The job container exits non-zero, post-job never runs, and the job is marked error, as before this PR.
  • Either merge order is safe. If :main has the new postjob.py check but a backend still sends the old CLEAN command, the old command's (touch error && false) fails the init container, so nothing is wrongly marked completed. The only risky order is the one your deploy note already covers.
  • Only these three commands write error. git grep over app/services/*_service.py and chart/ finds error written only by CLEAN, CRISPR-Copies and Mutagenesis. The latter two still end with && false, so post-job never runs for them.
  • No frontend reads finished or error. I searched src/ in clean-frontend (4730799), crispr-copies-frontend (eb80673) and mutagenesis-frontend (9ba1bbf) for those names, /out/ and artifacts. The only reads of job output are results fetches gated on completed. This settles the "I haven't checked the frontend" item in the description.

Notes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant