Skip to content

An unread exit code is unknown, not success #148

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
abhi18av merged 2 commits into from
Sep 4, 2026
Merged

An unread exit code is unknown, not success #148

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5f2f01d
test: reproduce the missing-exitcode-read-as-success defect
abhi18av Sep 4, 2026
dd15ccf
fix: an unread exit code is unknown, not success
abhi18av Sep 4, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 39 additions & 1 deletion src/main/groovy/nextflow/nomad/executor/NomadTaskHandler.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,9 @@ import java.nio.file.Path
@CompileStatic
class NomadTaskHandler extends TaskHandler implements FusionAwareTask {

/** Nomad's event type for the task process terminating; the only one carrying a real exit code. */
private static final String TASK_TERMINATED_EVENT = 'Terminated'

private final NomadConfig config

private final NomadService nomadService
Expand All @@ -69,6 +72,9 @@ class NomadTaskHandler extends TaskHandler implements FusionAwareTask {

private TaskState state

/** Where the value returned by {@link #defineExitCode()} came from, or null if it could not be determined. */
private String exitCodeSource = null

private long timestamp

private long submissionTime = 0L
Expand Down Expand Up @@ -244,14 +250,19 @@ class NomadTaskHandler extends TaskHandler implements FusionAwareTask {
// that as the error. defineExitCode returns Integer.MAX_VALUE when
// it couldn't read any signal, which trivially fails the `== 0`
// check, so a missing exit-file does NOT spuriously suppress.
//
// That guarantee depends on defineExitCode only accepting a code
// from an event that represents the task exiting. Nomad zeroes
// ExitCode on every other event type, so accepting any of them
// would turn an allocation that never ran into a reported success.
final boolean trustWorkerSuccess =
(remoteExit != null && remoteExit == 0) ||
(remoteExit == null && task.exitStatus == 0)
if ( !state || state.failed ) {
if( trustWorkerSuccess ) {
final String src = remoteExit != null
? "${workdirProvider.name()} remote .exitcode"
: 'local .exitcode'
: (exitCodeSource ?: 'local .exitcode')
log.warn "[NOMAD] task `${task.name}` reported Nomad alloc-state failure but ${src} = 0; trusting the worker exit code"
} else {
task.error = new ProcessException(failureMessage(state, task.exitStatus as Integer))
Expand Down Expand Up @@ -456,20 +467,37 @@ class NomadTaskHandler extends TaskHandler implements FusionAwareTask {
try {
def text = exitFile?.text?.trim()
if (text) {
exitCodeSource = 'local .exitcode'
return text as Integer
}
}
catch (Exception e) {
log.debug "[NOMAD] Cannot read exit status from file for task: `$task.name` | ${e.message}"
}

// Only an event that represents the task actually exiting carries a
// meaningful code. Nomad leaves ExitCode at its zero value on every
// other event type, so an allocation that failed before the task body
// ran looks like this:
//
// Received ExitCode 0 Task received by client
// Task Setup ExitCode 0 Building Task Directory
// Driver ExitCode 0 Downloading image
// Driver Failure ExitCode 0 Failed to pull image
// Not Restarting ExitCode 0 Policy allows no restarts
//
// Taking the first integer found there returns 0 from `Received` and
// reports a task that never ran as a success.
try {
if (state) {
List events = readListProperty(state, 'events')
if (events) {
for (Object event : events) {
if( !isTaskExitEvent(event) )
continue
def exitCode = readStringProperty(event, 'exitCode')
if (exitCode != null && exitCode.isInteger()) {
exitCodeSource = 'Nomad task events'
return exitCode as Integer
}
}
Expand All @@ -481,9 +509,19 @@ class NomadTaskHandler extends TaskHandler implements FusionAwareTask {
}

log.warn "[NOMAD] Cannot determine exit status for task: `$task.name`"
exitCodeSource = null
return Integer.MAX_VALUE
}

/**
* True when the event represents the task process terminating, which is
* the only case where Nomad populates ExitCode with a real value.
*/
protected static boolean isTaskExitEvent(Object event) {
final type = readStringProperty(event, 'type')
return type != null && type.equalsIgnoreCase(TASK_TERMINATED_EVENT)
}

protected Boolean shouldDelete(TaskState state) {
final cleanup = config.jobOpts().cleanup
if( cleanup == nextflow.nomad.config.NomadJobOpts.CLEANUP_ALWAYS ) {
Expand Down
100 changes: 100 additions & 0 deletions src/test/groovy/nextflow/nomad/executor/NomadTaskHandlerSpec.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -517,6 +517,60 @@ class NomadTaskHandlerSpec extends Specification{
assignedError == null // alloc-state failure suppressed by local .exitcode=0
}

void "surfaces the Nomad failure when the allocation never ran and wrote no .exitcode"() {
// End-to-end counterpart to the defineExitCode reproducer: an image-pull
// failure. The task body never runs, so the work dir holds no .exitcode,
// and Nomad's events all carry ExitCode 0 because none of them is a task
// exit. Previously this was reported as a success, Nextflow then looked
// for outputs that were never produced, and the user saw
// MissingFileException against a process script that was correct.
given:
Throwable assignedError = null
Integer assignedExit = null
def workDir = Files.createTempDirectory("nf-never-ran") // no .exitcode written
def task = Mock(TaskRun) {
getName() >> 'CHECK_DB'
getWorkDir() >> workDir
getConfig() >> [tag: null]
getProcessor() >> Mock(TaskProcessor) {
getExecutor() >> Mock(Executor) {
isFusionEnabled() >> false
}
}
setError(_ as Throwable) >> { Throwable value -> assignedError = value }
getError() >> { assignedError }
setExitStatus(_ as Integer) >> { Integer value -> assignedExit = value }
getExitStatus() >> { assignedExit }
}
def config = configWithCleanup(NomadJobOpts.CLEANUP_NEVER, false)
def service = Mock(NomadService) {
isPlacementFailure('job-never-ran', _ as Long) >> false
getTaskState('job-never-ran') >> new TaskState(
state: 'dead',
failed: true,
events: [
[type: 'Received', exitCode: 0, displayMessage: 'Task received by client'],
[type: 'Task Setup', exitCode: 0, displayMessage: 'Building Task Directory'],
[type: 'Driver', exitCode: 0, displayMessage: 'Downloading image'],
[type: 'Driver Failure', exitCode: 0, displayMessage: 'Failed to pull image'],
[type: 'Not Restarting', exitCode: 0, displayMessage: 'Policy allows no restarts'],
]
)
}
def handler = new NomadTaskHandler(task, config, service)
setPrivateField(handler, 'jobName', 'job-never-ran')
setPrivateField(handler, 'status', TaskStatus.SUBMITTED)

when:
def completed = handler.checkIfCompleted()

then:
completed
assignedExit == Integer.MAX_VALUE // unknown, not success
assignedError != null // Nomad's verdict is not overridden
assignedError.message.contains('Failed to pull image')
}

void "still surfaces error on vanilla path when local .exitcode is non-zero"() {
// Vanilla path mirror of the SPI non-zero test: a real failure must
// still raise even when the local exit-file is readable.
Expand Down Expand Up @@ -686,6 +740,52 @@ class NomadTaskHandlerSpec extends Specification{
exitStatus == 143
}

void "defineExitCode must not report success when the allocation never ran"() {
// Reproduces the event sequence a real Nomad allocation emits when it
// fails before the task body runs (captured from an image-pull failure
// on a live cluster, Nomad 1.11.2). Every event carries ExitCode 0,
// because Nomad leaves the field at its zero value on any event that
// is not a task exit.
//
// The task never ran, so no .exitcode file exists. Scanning these
// events for "the first integer exitCode" yields 0 from `Received`,
// which the caller then treats as the worker reporting success and
// uses to override Nomad's own alloc-state failure.
given:
def workDir = Files.createTempDirectory('nf-nomad-test')
def task = Mock(TaskRun) {
getWorkDir() >> workDir
getConfig() >> [tag: null]
getProcessor() >> Mock(TaskProcessor)
getName() >> "test_task"
}
def config = configWithCleanup(NomadJobOpts.CLEANUP_NEVER, false)
def handler = new NomadTaskHandler(task, config, Mock(NomadService))

def state = new TaskState(
state: 'dead',
failed: true,
events: [
[type: 'Received', exitCode: 0, displayMessage: 'Task received by client'],
[type: 'Task Setup', exitCode: 0, displayMessage: 'Building Task Directory'],
[type: 'Driver', exitCode: 0, displayMessage: 'Downloading image'],
[type: 'Driver Failure', exitCode: 0, displayMessage: 'Failed to pull image'],
[type: 'Not Restarting', exitCode: 0, displayMessage: 'Policy allows no restarts'],
]
)
setPrivateField(handler, 'state', state)

when:
int exitStatus = handler.defineExitCode()

then:
// An unread exit code is UNKNOWN, not success. Reporting 0 here makes
// Nextflow look for outputs that were never produced and blame the
// process script for a failure two layers away.
exitStatus != 0
exitStatus == Integer.MAX_VALUE
}

private static class TestNomadTaskHandler extends NomadTaskHandler {

TestNomadTaskHandler(TaskRun task, NomadConfig config, NomadService service) {
Expand Down
Loading