From 8855eaf3d627ba8316b627d6f1be2d55181aa579 Mon Sep 17 00:00:00 2001 From: Alex Kanitz Date: Fri, 13 Nov 2026 22:56:08 +0100 Subject: [PATCH 1/5] feat: support executors[].ignore_error Accept `ignore_error: true` instead of refusing it. Such executors' jobs get the annotation `ignore-error: "true"`, with which the taskmaster (elixir-cloud-aai/tesk-core#142) goes on after they fail. The task state ignores their failed jobs, unless the taskmaster ended the job itself (`activeDeadlineSeconds`, reason `DeadlineExceeded`), as its executor may never have run. Co-Authored-By: Claude Opus 5.5 --- docs/api.md | 31 +++++-- docs/index.md | 8 +- docs/upgrading.md | 8 ++ .../ebi/tsc/tesk/k8s/constant/Constants.java | 7 ++ .../tesk/k8s/convert/TaskStateResolver.java | 41 ++++++++- .../k8s/convert/TesKubernetesConverter.java | 4 + .../ebi/tsc/tesk/tes/model/TesExecutor.java | 12 +-- .../k8s/convert/TaskStateResolverTest.java | 91 +++++++++++++++++++ .../convert/TesKubernetesConverterTest.java | 27 ++++++ .../tesk/tes/api/TasksApiControllerTest.java | 16 +++- .../tsc/tesk/tes/model/Tes11FieldsTest.java | 3 - .../ignore_error_true.json | 0 12 files changed, 223 insertions(+), 25 deletions(-) rename src/test/resources/{invalid_tasks/executors => valid_tasks}/ignore_error_true.json (100%) diff --git a/docs/api.md b/docs/api.md index c2c94052..8c616239 100644 --- a/docs/api.md +++ b/docs/api.md @@ -107,9 +107,26 @@ TESK accepts the fields that TES 1.1 added to tasks: `400 Bad Request`. - `inputs[].streamable`: accepted and ignored, as TES allows: inputs are always downloaded before the executors start. -- `executors[].ignore_error`: only `false` is accepted. TESK stops at the - first executor that fails, so `true` is refused with `400 Bad Request` - rather than run differently than asked. +- `executors[].ignore_error`: with `true`, a failure of the executor + doesn't end the task. The executor's job gets the annotation + `ignore-error: "true"`, and the taskmaster goes on with the next executor + and the outputs filer, as if the executor had completed. The task ends + `COMPLETE` if the other jobs complete. Its executor log keeps the + executor's `exit_code` (and, with `FULL`, `stdout`), so clients see the + failure there. Failures that the executor's pods report count as the + executor's own and are ignored: a non-zero exit code, but also an + eviction, preemption or OOM kill. Some failures still end the task with + `EXECUTOR_ERROR`, because the executor may never have run: these are jobs + the taskmaster ended itself, e.g. as their pod couldn't start (an image + that can't be pulled) or wasn't scheduled in time. Kubernetes marks such + a job failed with the reason `DeadlineExceeded`. A job the taskmaster + deleted is missing, which gives `SYSTEM_ERROR`. This needs a taskmaster + that supports the annotation + ([tesk-core#142](https://github.com/elixir-cloud-aai/tesk-core/issues/142)); + older ones stop at the failure, and the task then ends `SYSTEM_ERROR` (a + later executor is missing) or, if it was the last executor, `COMPLETE` + without its outputs uploaded. Upgrade the taskmaster first (see + [Upgrading](upgrading.md)). - `resources.backend_parameters`: TESK supports none (service info lists none, see [Service info](#service-info)). - With `backend_parameters_strict` set to `true`, for which TES asks to @@ -131,10 +148,10 @@ Service info reports the TES version 1.1.0 (`type.version`). TESK API implements TES 1.1 with these differences, which clients should know about: -- `executors[].ignore_error: true` is refused with `400 Bad Request` (see - [TES 1.1 fields](#tes-11-fields); - [#159](https://github.com/elixir-cloud-aai/tesk-api/issues/159) is about - supporting it). +- `executors[].ignore_error: true` ignores only the failures that the + executor's pods report, not those of executors that the taskmaster ended + because their pod couldn't start or be scheduled (see [TES 1.1 + fields](#tes-11-fields)). - `resources.preemptible` and `resources.zones` are accepted and returned, but have no effect on where the task runs, as TES allows. - `inputs[].streamable` is accepted and ignored (see [TES 1.1 diff --git a/docs/index.md b/docs/index.md index a69aed8a..0a55bd14 100644 --- a/docs/index.md +++ b/docs/index.md @@ -96,10 +96,14 @@ applies gives the state: as active (see below); `CANCELED` once none has. 2. An executor's job failed, whichever executor it is: `PREEMPTED` if every failed executor's job was disrupted (see below), otherwise - `EXECUTOR_ERROR`. + `EXECUTOR_ERROR`. Executors that ignore errors (`ignore_error: true`, + see [TES 1.1 fields](api.md#tes-11-fields)) don't count here, unless the + taskmaster ended their job itself (it has `activeDeadlineSeconds`, or + the reason `DeadlineExceeded`). 3. The taskmaster's job or the outputs filer's job failed: `SYSTEM_ERROR`. 4. The taskmaster's job completed: `COMPLETE` if the jobs of all the task's - executors exist and completed, otherwise `SYSTEM_ERROR` (e.g., the inputs + executors exist and completed (or failed, for executors whose errors are + ignored as in rule 2), otherwise `SYSTEM_ERROR` (e.g., the inputs filer failed, so that no executor ran, or an executor's job is missing). The number of executors comes from the task stored with the taskmaster's job; without it, the executor jobs that exist count as all executors. diff --git a/docs/upgrading.md b/docs/upgrading.md index a4045152..1fcb50bf 100644 --- a/docs/upgrading.md +++ b/docs/upgrading.md @@ -16,6 +16,14 @@ From 1.2.0, clients may notice changed answers: are more (see [Listing tasks](api.md#listing-tasks)). Before, it returned all tasks at once. Clients that don't pass `page_size` must follow `next_page_token`, or pass a `page_size` of up to 2048. +- **`ignore_error`.** `executors[].ignore_error: true` is accepted rather + than refused with `400 Bad Request` (see [TES 1.1 + fields](api.md#tes-11-fields)). It needs a taskmaster that goes on after + such an executor fails + ([tesk-core#142](https://github.com/elixir-cloud-aai/tesk-core/issues/142)): + upgrade the taskmaster image first. With an older taskmaster, the task + stops at the failure, and ends `SYSTEM_ERROR`, or `COMPLETE` without its + outputs if the executor was the last one. ## To 1.2.0 diff --git a/src/main/java/uk/ac/ebi/tsc/tesk/k8s/constant/Constants.java b/src/main/java/uk/ac/ebi/tsc/tesk/k8s/constant/Constants.java index 8e3e4971..e975c6f8 100644 --- a/src/main/java/uk/ac/ebi/tsc/tesk/k8s/constant/Constants.java +++ b/src/main/java/uk/ac/ebi/tsc/tesk/k8s/constant/Constants.java @@ -47,6 +47,13 @@ public class Constants { /** Key of the annotation, that stores whole input TES task serialized to JSON */ public static final String ANN_JSON_INPUT_KEY = "json-input"; + /** + * Key of the annotation of an executor's job that ignores errors (TES {@code ignore_error}), + * with the value {@code "true"}: the taskmaster goes on after the executor fails, and the + * failure doesn't fail the task (needs a taskmaster with elixir-cloud-aai/tesk-core#142) + */ + public static final String ANN_IGNORE_ERROR_KEY = "ignore-error"; + /** * Key of the annotation of the taskmaster's job with the keys of the task's backend parameters, * which TESK doesn't support and therefore doesn't store (a JSON array of strings) diff --git a/src/main/java/uk/ac/ebi/tsc/tesk/k8s/convert/TaskStateResolver.java b/src/main/java/uk/ac/ebi/tsc/tesk/k8s/convert/TaskStateResolver.java index c41ffb09..db6395bb 100644 --- a/src/main/java/uk/ac/ebi/tsc/tesk/k8s/convert/TaskStateResolver.java +++ b/src/main/java/uk/ac/ebi/tsc/tesk/k8s/convert/TaskStateResolver.java @@ -1,10 +1,12 @@ package uk.ac.ebi.tsc.tesk.k8s.convert; +import static uk.ac.ebi.tsc.tesk.k8s.constant.Constants.ANN_IGNORE_ERROR_KEY; import static uk.ac.ebi.tsc.tesk.k8s.constant.Constants.LABEL_TASKSTATE_KEY; import static uk.ac.ebi.tsc.tesk.k8s.constant.Constants.LABEL_TASKSTATE_VALUE_CANC; import io.kubernetes.client.openapi.models.V1Job; import io.kubernetes.client.openapi.models.V1JobCondition; +import io.kubernetes.client.openapi.models.V1JobSpec; import io.kubernetes.client.openapi.models.V1JobStatus; import io.kubernetes.client.openapi.models.V1ObjectMeta; import io.kubernetes.client.openapi.models.V1Pod; @@ -161,6 +163,7 @@ public static TesState resolve(Task task, Supplier expectedExecutors) { List failedExecutors = executors.stream() .filter(executor -> hasFailed(executor.getJob(), taskmasterEnded)) + .filter(executor -> !ignoresError(executor.getJob())) .collect(Collectors.toList()); if (!failedExecutors.isEmpty()) { return failedExecutors.stream().allMatch(TaskStateResolver::wasDisrupted) @@ -181,7 +184,13 @@ public static TesState resolve(Task task, Supplier expectedExecutors) { expected > 0 && executors.size() == expected && executors.stream() - .allMatch(executor -> hasSucceeded(executor.getJob())); + .allMatch( + executor -> + hasSucceeded(executor.getJob()) + || ignoresError(executor.getJob()) + && hasFailed( + executor.getJob(), + true)); return allCompleted ? TesState.COMPLETE : TesState.SYSTEM_ERROR; } if (hasPendingPod(taskmaster)) { @@ -209,6 +218,36 @@ public static TesState resolve(Task task, Supplier expectedExecutors) { return TesState.SYSTEM_ERROR; } + /** + * Whether a failure of an executor's job is ignored (TES {@code ignore_error}, issue #159): the + * job has the annotation {@code ignore-error} with the value {@code "true"}, and the taskmaster + * didn't end it itself. The taskmaster ends a job whose pod can't start or be scheduled by + * setting its {@code activeDeadlineSeconds}, so that Kubernetes fails it with the reason {@code + * DeadlineExceeded}; the executor may then never have run, and the taskmaster stops the task. + */ + private static boolean ignoresError(V1Job job) { + boolean annotated = + Optional.ofNullable(job.getMetadata()) + .map(V1ObjectMeta::getAnnotations) + .map(annotations -> "true".equals(annotations.get(ANN_IGNORE_ERROR_KEY))) + .orElse(false); + return annotated && !endedByDeadline(job); + } + + private static boolean endedByDeadline(V1Job job) { + boolean hasDeadline = + Optional.ofNullable(job.getSpec()) + .map(V1JobSpec::getActiveDeadlineSeconds) + .isPresent(); + List conditions = + Optional.ofNullable(job.getStatus()) + .map(V1JobStatus::getConditions) + .orElse(Collections.emptyList()); + return hasDeadline + || conditions.stream() + .anyMatch(condition -> "DeadlineExceeded".equals(condition.getReason())); + } + private static boolean isCancelled(V1Job job) { return isLabelledCancelled(job.getMetadata()); } diff --git a/src/main/java/uk/ac/ebi/tsc/tesk/k8s/convert/TesKubernetesConverter.java b/src/main/java/uk/ac/ebi/tsc/tesk/k8s/convert/TesKubernetesConverter.java index 1e78830d..d94d6b7b 100644 --- a/src/main/java/uk/ac/ebi/tsc/tesk/k8s/convert/TesKubernetesConverter.java +++ b/src/main/java/uk/ac/ebi/tsc/tesk/k8s/convert/TesKubernetesConverter.java @@ -263,6 +263,10 @@ public V1Job fromTesExecutorToK8sJob( job.getMetadata() .putLabelsItem(LABEL_EXECNO_KEY, Integer.valueOf(executorIndex).toString()); job.getMetadata().putAnnotationsItem(ANN_TESTASK_NAME_KEY, tesTaskName); + // The taskmaster goes on after such an executor fails (issue #159) + if (Boolean.TRUE.equals(executor.getIgnoreError())) { + job.getMetadata().putAnnotationsItem(ANN_IGNORE_ERROR_KEY, "true"); + } CreatorLabels.put(job.getMetadata(), user.getUsername()); V1Container container = job.getSpec().getTemplate().getSpec().getContainers().get(0); // Tries to convert potential TRS URI into docker image. If conversion unsuccessful -> diff --git a/src/main/java/uk/ac/ebi/tsc/tesk/tes/model/TesExecutor.java b/src/main/java/uk/ac/ebi/tsc/tesk/tes/model/TesExecutor.java index 3e3d51f1..54c87f10 100644 --- a/src/main/java/uk/ac/ebi/tsc/tesk/tes/model/TesExecutor.java +++ b/src/main/java/uk/ac/ebi/tsc/tesk/tes/model/TesExecutor.java @@ -257,7 +257,7 @@ public TesExecutor ignoreError(Boolean ignoreError) { * TESK stops at the first executor that fails, so it accepts only `false`. * @return ignoreError */ - @ApiModelProperty(value = "Whether errors of this executor are ignored, and the next one runs. TESK doesn't support it: only false is accepted.") + @ApiModelProperty(value = "Whether errors of this executor are ignored: if it fails, the next executor runs, and the task can still complete.") public Boolean getIgnoreError() { return ignoreError; } @@ -266,16 +266,6 @@ public void setIgnoreError(Boolean ignoreError) { this.ignoreError = ignoreError; } - /** - * Refuses `ignore_error: true`, rather than running the task differently than asked. - * @return whether the executor doesn't ask to ignore its errors - */ - @JsonIgnore - @AssertTrue(message = "ignore_error: true is not supported; TESK stops at the first executor that fails") - public boolean isIgnoreErrorUnset() { - return ignoreError == null || !ignoreError; - } - public TesExecutor env(Map env) { this.env = env; return this; diff --git a/src/test/java/uk/ac/ebi/tsc/tesk/k8s/convert/TaskStateResolverTest.java b/src/test/java/uk/ac/ebi/tsc/tesk/k8s/convert/TaskStateResolverTest.java index 4372eeb6..975f7f9d 100644 --- a/src/test/java/uk/ac/ebi/tsc/tesk/k8s/convert/TaskStateResolverTest.java +++ b/src/test/java/uk/ac/ebi/tsc/tesk/k8s/convert/TaskStateResolverTest.java @@ -1,12 +1,14 @@ package uk.ac.ebi.tsc.tesk.k8s.convert; import static org.assertj.core.api.Assertions.assertThat; +import static uk.ac.ebi.tsc.tesk.k8s.constant.Constants.ANN_IGNORE_ERROR_KEY; import static uk.ac.ebi.tsc.tesk.k8s.constant.Constants.JOB_NAME_FILER_SUF; import static uk.ac.ebi.tsc.tesk.k8s.constant.Constants.LABEL_TASKSTATE_KEY; import static uk.ac.ebi.tsc.tesk.k8s.constant.Constants.LABEL_TASKSTATE_VALUE_CANC; import io.kubernetes.client.openapi.models.V1Job; import io.kubernetes.client.openapi.models.V1JobCondition; +import io.kubernetes.client.openapi.models.V1JobSpec; import io.kubernetes.client.openapi.models.V1JobStatus; import io.kubernetes.client.openapi.models.V1ObjectMeta; import io.kubernetes.client.openapi.models.V1Pod; @@ -619,6 +621,95 @@ void activeCountWithoutPodsIsCanceling() { assertThat(TaskStateResolver.resolve(task, () -> 1)).isEqualTo(TesState.CANCELING); } + /** + * An executor that ignores errors (TES ignore_error, issue #159) and failed doesn't make the + * task fail: the task's state comes from the other jobs, whichever executor it is, also a + * preempted one. + */ + @ParameterizedTest + @ValueSource(ints = {0, 1}) + void ignoredExecutorFailureCountsAsDone(int ignored) { + for (JobState failure : EnumSet.of(JobState.FAILED, JobState.PREEMPTED)) { + JobState ex0 = ignored == 0 ? failure : JobState.SUCCEEDED; + JobState ex1 = ignored == 1 ? failure : JobState.SUCCEEDED; + Task done = task(false, JobState.SUCCEEDED, ex0, ex1, JobState.SUCCEEDED); + ignoreError(done, ignored, "true"); + assertThat(TaskStateResolver.resolve(done, () -> 2)).isEqualTo(TesState.COMPLETE); + Task running = task(false, JobState.RUNNING, ex0, ex1, JobState.RUNNING); + ignoreError(running, ignored, "true"); + assertThat(TaskStateResolver.resolve(running, () -> 2)).isEqualTo(TesState.RUNNING); + } + } + + /** Errors of executors without the annotation value "true" aren't ignored (issue #159). */ + @ParameterizedTest + @ValueSource(strings = {"false", "True", ""}) + void otherAnnotationValuesDontIgnoreErrors(String value) { + Task task = + task( + false, + JobState.SUCCEEDED, + JobState.FAILED, + JobState.SUCCEEDED, + JobState.SUCCEEDED); + ignoreError(task, 0, value); + assertThat(TaskStateResolver.resolve(task, () -> 2)).isEqualTo(TesState.EXECUTOR_ERROR); + } + + /** An executor that fails without ignoring errors still fails the task (issue #159). */ + @Test + void laterExecutorErrorIsNotIgnored() { + Task task = + task(false, JobState.SUCCEEDED, JobState.FAILED, JobState.FAILED, JobState.ABSENT); + ignoreError(task, 0, "true"); + assertThat(TaskStateResolver.resolve(task, () -> 2)).isEqualTo(TesState.EXECUTOR_ERROR); + } + + /** + * An executor's job that the taskmaster ended itself (it set activeDeadlineSeconds, e.g. as the + * pod couldn't start; Kubernetes then fails the job with the reason DeadlineExceeded) isn't + * ignored: the executor may never have run, and the taskmaster stopped (issue #159). + */ + @Test + void executorEndedByTaskmasterIsNotIgnored() { + Task deadline = + task( + false, + JobState.SUCCEEDED, + JobState.FAILED, + JobState.SUCCEEDED, + JobState.SUCCEEDED); + ignoreError(deadline, 0, "true"); + deadline.getExecutors().get(0).getJob().spec(new V1JobSpec().activeDeadlineSeconds(1L)); + assertThat(TaskStateResolver.resolve(deadline, () -> 2)).isEqualTo(TesState.EXECUTOR_ERROR); + + Task reason = + task( + false, + JobState.SUCCEEDED, + JobState.FAILED, + JobState.SUCCEEDED, + JobState.SUCCEEDED); + ignoreError(reason, 0, "true"); + reason.getExecutors() + .get(0) + .getJob() + .getStatus() + .getConditions() + .get(0) + .reason("DeadlineExceeded"); + assertThat(TaskStateResolver.resolve(reason, () -> 2)).isEqualTo(TesState.EXECUTOR_ERROR); + } + + /** Gives an executor's job the annotation with which TESK passes ignore_error. */ + private static void ignoreError(Task task, int executor, String value) { + task.getExecutors() + .get(executor) + .getJob() + .getMetadata() + .putAnnotationsItem(ANN_IGNORE_ERROR_KEY, value); + } + private static Task task( boolean cancelled, JobState taskmaster, diff --git a/src/test/java/uk/ac/ebi/tsc/tesk/k8s/convert/TesKubernetesConverterTest.java b/src/test/java/uk/ac/ebi/tsc/tesk/k8s/convert/TesKubernetesConverterTest.java index cdd76444..58439d9f 100644 --- a/src/test/java/uk/ac/ebi/tsc/tesk/k8s/convert/TesKubernetesConverterTest.java +++ b/src/test/java/uk/ac/ebi/tsc/tesk/k8s/convert/TesKubernetesConverterTest.java @@ -8,6 +8,7 @@ import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.BDDMockito.given; import static org.mockito.Mockito.*; +import static uk.ac.ebi.tsc.tesk.k8s.constant.Constants.ANN_IGNORE_ERROR_KEY; import static uk.ac.ebi.tsc.tesk.k8s.constant.Constants.LABEL_TASKSTATE_KEY; import static uk.ac.ebi.tsc.tesk.k8s.constant.Constants.LABEL_TASKSTATE_VALUE_CANC; @@ -866,6 +867,32 @@ public void fromTesExecutorToK8sJob_decimalCpuCores() throws IOException { assertEquals("500m", requirements.getRequests().get("cpu").toSuffixedString()); } + /** + * An executor that ignores errors gets the annotation with which the taskmaster goes on after + * it fails; other executors get none (issue #159) + */ + @Test + public void fromTesExecutorToK8sJob_ignoreError() { + Boolean[] values = {true, false, null}; + for (Boolean ignoreError : values) { + TesExecutor executor = + new TesExecutor() + .image("alpine") + .addCommandItem("true") + .ignoreError(ignoreError); + V1Job job = + this.converter.fromTesExecutorToK8sJob( + "task-123", + "name", + executor, + 0, + null, + User.builder("test-user-id").build()); + String annotation = job.getMetadata().getAnnotations().get(ANN_IGNORE_ERROR_KEY); + assertEquals(Boolean.TRUE.equals(ignoreError) ? "true" : null, annotation); + } + } + /** * RAM is requested with a decimal point whatever the JVM's default locale is, e.g. also under a * locale with a decimal comma diff --git a/src/test/java/uk/ac/ebi/tsc/tesk/tes/api/TasksApiControllerTest.java b/src/test/java/uk/ac/ebi/tsc/tesk/tes/api/TasksApiControllerTest.java index 99d9b0b3..c870f3b0 100644 --- a/src/test/java/uk/ac/ebi/tsc/tesk/tes/api/TasksApiControllerTest.java +++ b/src/test/java/uk/ac/ebi/tsc/tesk/tes/api/TasksApiControllerTest.java @@ -221,7 +221,6 @@ public void createTask_invalid_executors() throws Exception { new String[] {"no_image.json", "image"}, new String[] {"no_command.json", "command"}, new String[] {"invalid_stdin.json", "stdin"}, - new String[] {"ignore_error_true.json", "ignore_error: true is not supported"}, }; for (String[] file : files) { this.mvc @@ -483,6 +482,21 @@ public void createTask_valid_tes11Fields() throws Exception { .andExpect(status().isOk()); } + /** Executors that ignore errors are accepted (issue #159) */ + @Test + public void createTask_valid_ignoreErrorTrue() throws Exception { + this.mvc + .perform( + post(TASK_URL) + .header("Authorization", "Bearer BAR") + .content( + getFileContentFromResources( + "valid_tasks/ignore_error_true.json")) + .contentType(MediaType.APPLICATION_JSON) + .accept(MediaType.APPLICATION_JSON)) + .andExpect(status().isOk()); + } + /** * Outputs with wildcards to file: URLs, or URLs without a scheme, still need no path_prefix, as * in TES 1.0 (issue #148) diff --git a/src/test/java/uk/ac/ebi/tsc/tesk/tes/model/Tes11FieldsTest.java b/src/test/java/uk/ac/ebi/tsc/tesk/tes/model/Tes11FieldsTest.java index e0a6eb07..ab0c8dba 100644 --- a/src/test/java/uk/ac/ebi/tsc/tesk/tes/model/Tes11FieldsTest.java +++ b/src/test/java/uk/ac/ebi/tsc/tesk/tes/model/Tes11FieldsTest.java @@ -60,13 +60,10 @@ public void input_equalsAndHashCode_includeStreamable() { public void executor_ignoreError() { TesExecutor executor = new TesExecutor().image("ubuntu"); assertNull(executor.getIgnoreError()); - assertTrue(executor.isIgnoreErrorUnset()); assertSame(executor, executor.ignoreError(false)); assertFalse(executor.getIgnoreError()); - assertTrue(executor.isIgnoreErrorUnset()); executor.setIgnoreError(true); assertTrue(executor.getIgnoreError()); - assertFalse(executor.isIgnoreErrorUnset()); assertTrue(executor.toString().contains("ignoreError: true")); } diff --git a/src/test/resources/invalid_tasks/executors/ignore_error_true.json b/src/test/resources/valid_tasks/ignore_error_true.json similarity index 100% rename from src/test/resources/invalid_tasks/executors/ignore_error_true.json rename to src/test/resources/valid_tasks/ignore_error_true.json From d3d1e835fe6242512f0b32cbc15a1c3fffe54701 Mon Sep 17 00:00:00 2001 From: Alex Kanitz Date: Fri, 13 Nov 2026 23:25:34 +0100 Subject: [PATCH 2/5] fix: need outputs after ignored executor errors A taskmaster can stop at an ignored failure without uploading the outputs: one without support for the annotation (as the default image still is), or one whose failed pods didn't end in time. After an ignored failure, a task with outputs (or unknown ones, read from the stored task) is now COMPLETE only if its outputs filer completed; otherwise SYSTEM_ERROR. Also from the review: a constant for the annotation value, an `isDone` helper, clearer tests and docs. Co-Authored-By: Claude Opus 5.5 --- docs/api.md | 39 ++++----- docs/upgrading.md | 7 +- .../ebi/tsc/tesk/k8s/constant/Constants.java | 3 + .../tesk/k8s/convert/TaskStateResolver.java | 60 +++++++++++--- .../k8s/convert/TesKubernetesConverter.java | 21 ++++- .../ebi/tsc/tesk/tes/model/TesExecutor.java | 3 +- .../k8s/convert/TaskStateResolverTest.java | 81 ++++++++++++++++--- .../convert/TesKubernetesConverterTest.java | 26 +++++- 8 files changed, 192 insertions(+), 48 deletions(-) diff --git a/docs/api.md b/docs/api.md index 8c616239..0077f1f7 100644 --- a/docs/api.md +++ b/docs/api.md @@ -108,25 +108,26 @@ TESK accepts the fields that TES 1.1 added to tasks: - `inputs[].streamable`: accepted and ignored, as TES allows: inputs are always downloaded before the executors start. - `executors[].ignore_error`: with `true`, a failure of the executor - doesn't end the task. The executor's job gets the annotation - `ignore-error: "true"`, and the taskmaster goes on with the next executor - and the outputs filer, as if the executor had completed. The task ends - `COMPLETE` if the other jobs complete. Its executor log keeps the - executor's `exit_code` (and, with `FULL`, `stdout`), so clients see the - failure there. Failures that the executor's pods report count as the - executor's own and are ignored: a non-zero exit code, but also an - eviction, preemption or OOM kill. Some failures still end the task with - `EXECUTOR_ERROR`, because the executor may never have run: these are jobs - the taskmaster ended itself, e.g. as their pod couldn't start (an image - that can't be pulled) or wasn't scheduled in time. Kubernetes marks such - a job failed with the reason `DeadlineExceeded`. A job the taskmaster - deleted is missing, which gives `SYSTEM_ERROR`. This needs a taskmaster - that supports the annotation - ([tesk-core#142](https://github.com/elixir-cloud-aai/tesk-core/issues/142)); - older ones stop at the failure, and the task then ends `SYSTEM_ERROR` (a - later executor is missing) or, if it was the last executor, `COMPLETE` - without its outputs uploaded. Upgrade the taskmaster first (see - [Upgrading](upgrading.md)). + doesn't end the task. + - The executor's job gets the annotation `ignore-error: "true"`, and + the taskmaster goes on with the next executor and the outputs filer, + as if the executor had completed. The task ends `COMPLETE` if the + other jobs complete, including the outputs filer if the task has + outputs: a taskmaster that stopped at the failure without uploading + them gives `SYSTEM_ERROR`. The executor log keeps the executor's + `exit_code` (and, with `FULL`, `stdout`), so clients see the failure + there. + - Failures that the executor's pods report are ignored: a non-zero exit + code, but also an eviction, preemption or OOM kill. + - Failures of jobs that the taskmaster ended itself aren't, as the + executor may never have run. A pod that couldn't start (e.g. an image + that can't be pulled) fails its job with the reason + `DeadlineExceeded`, which gives `EXECUTOR_ERROR`. An executor whose + pod wasn't scheduled in time usually has its job deleted, and the + missing executor gives `SYSTEM_ERROR`. + - It needs a taskmaster that supports the annotation + ([tesk-core#142](https://github.com/elixir-cloud-aai/tesk-core/issues/142)); + see [Upgrading](upgrading.md) for what older ones do. - `resources.backend_parameters`: TESK supports none (service info lists none, see [Service info](#service-info)). - With `backend_parameters_strict` set to `true`, for which TES asks to diff --git a/docs/upgrading.md b/docs/upgrading.md index 1fcb50bf..cddc2c36 100644 --- a/docs/upgrading.md +++ b/docs/upgrading.md @@ -21,9 +21,10 @@ From 1.2.0, clients may notice changed answers: fields](api.md#tes-11-fields)). It needs a taskmaster that goes on after such an executor fails ([tesk-core#142](https://github.com/elixir-cloud-aai/tesk-core/issues/142)): - upgrade the taskmaster image first. With an older taskmaster, the task - stops at the failure, and ends `SYSTEM_ERROR`, or `COMPLETE` without its - outputs if the executor was the last one. + upgrade the taskmaster image first (`TESK_API_TASKMASTER_IMAGE_VERSION`, + see [Configuration](configuration.md)). An older taskmaster stops at the + failure: the task then ends `SYSTEM_ERROR`, unless the failed executor was + the last one and the task has no outputs, which completes as asked. ## To 1.2.0 diff --git a/src/main/java/uk/ac/ebi/tsc/tesk/k8s/constant/Constants.java b/src/main/java/uk/ac/ebi/tsc/tesk/k8s/constant/Constants.java index e975c6f8..acd0dfb9 100644 --- a/src/main/java/uk/ac/ebi/tsc/tesk/k8s/constant/Constants.java +++ b/src/main/java/uk/ac/ebi/tsc/tesk/k8s/constant/Constants.java @@ -54,6 +54,9 @@ public class Constants { */ public static final String ANN_IGNORE_ERROR_KEY = "ignore-error"; + /** Value of the annotation {@link #ANN_IGNORE_ERROR_KEY} of an executor that ignores errors */ + public static final String ANN_IGNORE_ERROR_VALUE = "true"; + /** * Key of the annotation of the taskmaster's job with the keys of the task's backend parameters, * which TESK doesn't support and therefore doesn't store (a JSON array of strings) diff --git a/src/main/java/uk/ac/ebi/tsc/tesk/k8s/convert/TaskStateResolver.java b/src/main/java/uk/ac/ebi/tsc/tesk/k8s/convert/TaskStateResolver.java index db6395bb..1bc36af0 100644 --- a/src/main/java/uk/ac/ebi/tsc/tesk/k8s/convert/TaskStateResolver.java +++ b/src/main/java/uk/ac/ebi/tsc/tesk/k8s/convert/TaskStateResolver.java @@ -1,6 +1,7 @@ package uk.ac.ebi.tsc.tesk.k8s.convert; import static uk.ac.ebi.tsc.tesk.k8s.constant.Constants.ANN_IGNORE_ERROR_KEY; +import static uk.ac.ebi.tsc.tesk.k8s.constant.Constants.ANN_IGNORE_ERROR_VALUE; import static uk.ac.ebi.tsc.tesk.k8s.constant.Constants.LABEL_TASKSTATE_KEY; import static uk.ac.ebi.tsc.tesk.k8s.constant.Constants.LABEL_TASKSTATE_VALUE_CANC; @@ -146,6 +147,24 @@ private static boolean isDisrupted(V1Pod pod) { * @return the TES state of the task */ public static TesState resolve(Task task, Supplier expectedExecutors) { + return resolve(task, expectedExecutors, () -> null); + } + + /** + * Derives the state of a task. + * + * @param task the task's jobs and their pods + * @param expectedExecutors supplies the number of executors of the task, or {@code null} if + * unknown (then the executor jobs that exist count as all executors); only called once the + * taskmaster's job completed + * @param hasOutputs supplies whether the task has outputs, or {@code null} if unknown; only + * called once the taskmaster's job completed after an executor whose failure is ignored + * (then, with outputs or if unknown, the task completed only if the outputs filer did, as a + * taskmaster can stop at such a failure without uploading the outputs) + * @return the TES state of the task + */ + public static TesState resolve( + Task task, Supplier expectedExecutors, Supplier hasOutputs) { Job taskmaster = task.getTaskmaster(); V1Job taskmasterJob = taskmaster.getJob(); if (isCancelled(taskmasterJob)) { @@ -183,14 +202,18 @@ public static TesState resolve(Task task, Supplier expectedExecutors) { boolean allCompleted = expected > 0 && executors.size() == expected - && executors.stream() - .allMatch( - executor -> - hasSucceeded(executor.getJob()) - || ignoresError(executor.getJob()) - && hasFailed( - executor.getJob(), - true)); + && executors.stream().allMatch(executor -> isDone(executor.getJob())); + // After an ignored failure, a taskmaster can stop without uploading the outputs + // (one without support for ignore_error, or one whose failed executor's pods didn't + // end in time): the outputs filer must have completed, if there are outputs + boolean ignoredFailure = + executors.stream().anyMatch(executor -> !hasSucceeded(executor.getJob())); + if (allCompleted + && ignoredFailure + && !Boolean.FALSE.equals(hasOutputs.get()) + && !outputFiler.map(filer -> hasSucceeded(filer.getJob())).orElse(false)) { + return TesState.SYSTEM_ERROR; + } return allCompleted ? TesState.COMPLETE : TesState.SYSTEM_ERROR; } if (hasPendingPod(taskmaster)) { @@ -229,11 +252,30 @@ private static boolean ignoresError(V1Job job) { boolean annotated = Optional.ofNullable(job.getMetadata()) .map(V1ObjectMeta::getAnnotations) - .map(annotations -> "true".equals(annotations.get(ANN_IGNORE_ERROR_KEY))) + .map( + annotations -> + ANN_IGNORE_ERROR_VALUE.equals( + annotations.get(ANN_IGNORE_ERROR_KEY))) .orElse(false); return annotated && !endedByDeadline(job); } + /** + * Whether an executor's job is done, once the taskmaster's job has completed: it succeeded, or + * it failed and its failure is ignored ({@link #ignoresError}). + */ + private static boolean isDone(V1Job job) { + // The taskmaster's job has ended, so failed pods alone count as a failure + boolean taskmasterEnded = true; + return hasSucceeded(job) || (ignoresError(job) && hasFailed(job, taskmasterEnded)); + } + + /** + * Whether the taskmaster ended the job itself: it fails a job whose pod can't start or be + * scheduled by setting {@code activeDeadlineSeconds} (to 1, {@code FAIL_DEADLINE} in + * tesk-core's {@code job.py}), so that Kubernetes fails it with the reason {@code + * DeadlineExceeded}. TESK API sets no deadline on executor jobs. + */ private static boolean endedByDeadline(V1Job job) { boolean hasDeadline = Optional.ofNullable(job.getSpec()) diff --git a/src/main/java/uk/ac/ebi/tsc/tesk/k8s/convert/TesKubernetesConverter.java b/src/main/java/uk/ac/ebi/tsc/tesk/k8s/convert/TesKubernetesConverter.java index d94d6b7b..22756383 100644 --- a/src/main/java/uk/ac/ebi/tsc/tesk/k8s/convert/TesKubernetesConverter.java +++ b/src/main/java/uk/ac/ebi/tsc/tesk/k8s/convert/TesKubernetesConverter.java @@ -265,7 +265,7 @@ public V1Job fromTesExecutorToK8sJob( job.getMetadata().putAnnotationsItem(ANN_TESTASK_NAME_KEY, tesTaskName); // The taskmaster goes on after such an executor fails (issue #159) if (Boolean.TRUE.equals(executor.getIgnoreError())) { - job.getMetadata().putAnnotationsItem(ANN_IGNORE_ERROR_KEY, "true"); + job.getMetadata().putAnnotationsItem(ANN_IGNORE_ERROR_KEY, ANN_IGNORE_ERROR_VALUE); } CreatorLabels.put(job.getMetadata(), user.getUsername()); V1Container container = job.getSpec().getTemplate().getSpec().getContainers().get(0); @@ -346,9 +346,26 @@ public TesCreateTaskResponse fromK8sJobToTesCreateTaskResponse(V1Job job) { */ public TesState extractStateFromK8sJobs(Task taskmasterWithExecutors) { // the stored task is read only if needed: once the taskmaster's job completed + V1Job taskmasterJob = taskmasterWithExecutors.getTaskmaster().getJob(); return TaskStateResolver.resolve( taskmasterWithExecutors, - () -> this.extractExecutorCount(taskmasterWithExecutors.getTaskmaster().getJob())); + () -> this.extractExecutorCount(taskmasterJob), + () -> this.extractHasOutputs(taskmasterJob)); + } + + /** + * Reads whether the task stored with the taskmaster's job (annotation {@code json-input}) has + * outputs. + * + * @param taskMasterJob - taskmaster's job + * @return whether the task has outputs, or null if the task isn't stored or can't be read + */ + Boolean extractHasOutputs(V1Job taskMasterJob) { + if (this.readStoredTaskField(taskMasterJob, "executors") == null) { + return null; + } + JsonNode outputs = this.readStoredTaskField(taskMasterJob, "outputs"); + return outputs != null && outputs.isArray() && outputs.size() > 0; } /** diff --git a/src/main/java/uk/ac/ebi/tsc/tesk/tes/model/TesExecutor.java b/src/main/java/uk/ac/ebi/tsc/tesk/tes/model/TesExecutor.java index 54c87f10..f576d270 100644 --- a/src/main/java/uk/ac/ebi/tsc/tesk/tes/model/TesExecutor.java +++ b/src/main/java/uk/ac/ebi/tsc/tesk/tes/model/TesExecutor.java @@ -254,7 +254,8 @@ public TesExecutor ignoreError(Boolean ignoreError) { /** * Whether to run the next executor, and finish the task, after this executor fails (TES 1.1). - * TESK stops at the first executor that fails, so it accepts only `false`. + * With `true`, the executor's job gets the annotation `ignore-error`, so that the taskmaster goes + * on after it fails (see `TaskStateResolver`). * @return ignoreError */ @ApiModelProperty(value = "Whether errors of this executor are ignored: if it fails, the next executor runs, and the task can still complete.") diff --git a/src/test/java/uk/ac/ebi/tsc/tesk/k8s/convert/TaskStateResolverTest.java b/src/test/java/uk/ac/ebi/tsc/tesk/k8s/convert/TaskStateResolverTest.java index 975f7f9d..04ee6a69 100644 --- a/src/test/java/uk/ac/ebi/tsc/tesk/k8s/convert/TaskStateResolverTest.java +++ b/src/test/java/uk/ac/ebi/tsc/tesk/k8s/convert/TaskStateResolverTest.java @@ -26,6 +26,7 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.CsvSource; import org.junit.jupiter.params.provider.MethodSource; import org.junit.jupiter.params.provider.ValueSource; import uk.ac.ebi.tsc.tesk.k8s.convert.data.Job; @@ -626,19 +627,73 @@ void activeCountWithoutPodsIsCanceling() { * task fail: the task's state comes from the other jobs, whichever executor it is, also a * preempted one. */ - @ParameterizedTest - @ValueSource(ints = {0, 1}) - void ignoredExecutorFailureCountsAsDone(int ignored) { - for (JobState failure : EnumSet.of(JobState.FAILED, JobState.PREEMPTED)) { - JobState ex0 = ignored == 0 ? failure : JobState.SUCCEEDED; - JobState ex1 = ignored == 1 ? failure : JobState.SUCCEEDED; - Task done = task(false, JobState.SUCCEEDED, ex0, ex1, JobState.SUCCEEDED); - ignoreError(done, ignored, "true"); - assertThat(TaskStateResolver.resolve(done, () -> 2)).isEqualTo(TesState.COMPLETE); - Task running = task(false, JobState.RUNNING, ex0, ex1, JobState.RUNNING); - ignoreError(running, ignored, "true"); - assertThat(TaskStateResolver.resolve(running, () -> 2)).isEqualTo(TesState.RUNNING); - } + @ParameterizedTest(name = "executor {0} {1}, taskmaster {2}: {3}") + @CsvSource({ + "0, FAILED, SUCCEEDED, COMPLETE", + "1, FAILED, SUCCEEDED, COMPLETE", + "0, PREEMPTED, SUCCEEDED, COMPLETE", + "1, PREEMPTED, SUCCEEDED, COMPLETE", + "0, FAILED, RUNNING, RUNNING", + "1, FAILED, RUNNING, RUNNING", + "0, FAILED_PODS, SUCCEEDED, COMPLETE", + }) + void ignoredExecutorFailureCountsAsDone( + int ignored, JobState failure, JobState taskmaster, TesState expected) { + JobState ex0 = ignored == 0 ? failure : JobState.SUCCEEDED; + JobState ex1 = ignored == 1 ? failure : JobState.SUCCEEDED; + JobState filer = taskmaster == JobState.RUNNING ? JobState.RUNNING : JobState.SUCCEEDED; + Task task = task(false, taskmaster, ex0, ex1, filer); + ignoreError(task, ignored, "true"); + assertThat(TaskStateResolver.resolve(task, () -> 2)).isEqualTo(expected); + } + + /** + * An ignored failure doesn't make up for a missing executor: if a later executor's job is + * missing (e.g., a taskmaster without support for ignore_error stopped at the failure), the + * task has a system error (issue #159). + */ + @Test + void ignoredFailureWithMissingLaterExecutor() { + Task missing = + task(false, JobState.SUCCEEDED, JobState.FAILED, JobState.ABSENT, JobState.ABSENT); + ignoreError(missing, 0, "true"); + assertThat(TaskStateResolver.resolve(missing, () -> 2, () -> false)) + .isEqualTo(TesState.SYSTEM_ERROR); + } + + /** + * After an ignored failure of the last executor, a task with outputs (or unknown ones) + * completed only if its outputs filer did, as the taskmaster may have stopped at the failure; a + * task without outputs needs none (issue #159). + */ + @ParameterizedTest(name = "outputs {0}, filer {1}: {2}") + @CsvSource({ + "true, ABSENT, SYSTEM_ERROR", + ", ABSENT, SYSTEM_ERROR", + "false, ABSENT, COMPLETE", + "true, SUCCEEDED, COMPLETE", + ", SUCCEEDED, COMPLETE", + "true, NEW, SYSTEM_ERROR", + }) + void ignoredFailureOfLastExecutorNeedsOutputsFiler( + Boolean hasOutputs, JobState filer, TesState expected) { + Task task = task(false, JobState.SUCCEEDED, JobState.SUCCEEDED, JobState.FAILED, filer); + ignoreError(task, 1, "true"); + assertThat(TaskStateResolver.resolve(task, () -> 2, () -> hasOutputs)).isEqualTo(expected); + } + + /** Without ignored failures, the outputs filer's job isn't needed for COMPLETE, as before. */ + @Test + void completeWithoutIgnoredFailureNeedsNoOutputsFiler() { + Task task = + task( + false, + JobState.SUCCEEDED, + JobState.SUCCEEDED, + JobState.SUCCEEDED, + JobState.ABSENT); + assertThat(TaskStateResolver.resolve(task, () -> 2, () -> true)) + .isEqualTo(TesState.COMPLETE); } /** Errors of executors without the annotation value "true" aren't ignored (issue #159). */ diff --git a/src/test/java/uk/ac/ebi/tsc/tesk/k8s/convert/TesKubernetesConverterTest.java b/src/test/java/uk/ac/ebi/tsc/tesk/k8s/convert/TesKubernetesConverterTest.java index 58439d9f..5fce9a91 100644 --- a/src/test/java/uk/ac/ebi/tsc/tesk/k8s/convert/TesKubernetesConverterTest.java +++ b/src/test/java/uk/ac/ebi/tsc/tesk/k8s/convert/TesKubernetesConverterTest.java @@ -830,6 +830,27 @@ private Map tagsOf(String storedTask) throws IOException { return this.converter.extractTags(taskmaster); } + /** + * Whether the stored task has outputs: unknown (null) without a readable stored task (issue + * #159). + */ + @Test + public void extractHasOutputs() throws IOException { + String executors = "\"executors\": [{}]"; + assertEquals(Boolean.TRUE, this.hasOutputsOf("{" + executors + ", \"outputs\": [{}]}")); + assertEquals(Boolean.FALSE, this.hasOutputsOf("{" + executors + ", \"outputs\": []}")); + assertEquals(Boolean.FALSE, this.hasOutputsOf("{" + executors + "}")); + assertEquals(Boolean.FALSE, this.hasOutputsOf("{" + executors + ", \"outputs\": {}}")); + assertNull(this.hasOutputsOf("not JSON")); + assertNull(this.hasOutputsOf(null)); + } + + private Boolean hasOutputsOf(String storedTask) throws IOException { + V1Job taskmaster = this.prepareBaseTaskBuider().getTask().getTaskmaster().getJob(); + taskmaster.getMetadata().getAnnotations().put(Constants.ANN_JSON_INPUT_KEY, storedTask); + return this.converter.extractHasOutputs(taskmaster); + } + private Integer executorCountOf(String storedTask) throws IOException { V1Job taskmaster = this.prepareBaseTaskBuider().getTask().getTaskmaster().getJob(); taskmaster.getMetadata().getAnnotations().put(Constants.ANN_JSON_INPUT_KEY, storedTask); @@ -889,7 +910,10 @@ public void fromTesExecutorToK8sJob_ignoreError() { null, User.builder("test-user-id").build()); String annotation = job.getMetadata().getAnnotations().get(ANN_IGNORE_ERROR_KEY); - assertEquals(Boolean.TRUE.equals(ignoreError) ? "true" : null, annotation); + assertEquals( + "ignore_error: " + ignoreError, + Boolean.TRUE.equals(ignoreError) ? "true" : null, + annotation); } } From 0671b6fd4eb37c55ebbb1227789e055efb2ebfca Mon Sep 17 00:00:00 2001 From: Alex Kanitz Date: Fri, 13 Nov 2026 23:50:30 +0100 Subject: [PATCH 3/5] docs: state the outputs filer rule in rule 4 Co-Authored-By: Claude Opus 5.5 --- docs/index.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/index.md b/docs/index.md index 0a55bd14..4d11a148 100644 --- a/docs/index.md +++ b/docs/index.md @@ -105,7 +105,9 @@ applies gives the state: executors exist and completed (or failed, for executors whose errors are ignored as in rule 2), otherwise `SYSTEM_ERROR` (e.g., the inputs filer failed, so that no executor ran, or an executor's job is missing). - The number of executors comes from the task stored with the taskmaster's + After an ignored failure, a task with outputs (or whose stored task + can't be read) also needs its outputs filer's job to have completed; + otherwise it's `SYSTEM_ERROR`. The number of executors comes from the task stored with the taskmaster's job; without it, the executor jobs that exist count as all executors. 5. A pod of the taskmaster's job is pending: `QUEUED`. 6. The taskmaster's job is active: `INITIALIZING` until the first executor's From 14dfa5eeb8a9521f3866b94c0cd11465b3913e2d Mon Sep 17 00:00:00 2001 From: Alex Kanitz Date: Sat, 14 Nov 2026 00:06:45 +0100 Subject: [PATCH 4/5] test: cover ignored failures end to end Covers the stored task's outputs in extractStateFromK8sJobs and an ignoring executor that hasn't ended, for Codecov's patch check. Co-Authored-By: Claude Opus 5.5 --- .../k8s/convert/TaskStateResolverTest.java | 18 +++++++++++ .../convert/TesKubernetesConverterTest.java | 30 +++++++++++++++++++ 2 files changed, 48 insertions(+) diff --git a/src/test/java/uk/ac/ebi/tsc/tesk/k8s/convert/TaskStateResolverTest.java b/src/test/java/uk/ac/ebi/tsc/tesk/k8s/convert/TaskStateResolverTest.java index 04ee6a69..7fb2db81 100644 --- a/src/test/java/uk/ac/ebi/tsc/tesk/k8s/convert/TaskStateResolverTest.java +++ b/src/test/java/uk/ac/ebi/tsc/tesk/k8s/convert/TaskStateResolverTest.java @@ -661,6 +661,24 @@ void ignoredFailureWithMissingLaterExecutor() { .isEqualTo(TesState.SYSTEM_ERROR); } + /** + * An executor that ignores errors but whose job neither succeeded nor failed isn't done: once + * the taskmaster's job completed, the task has a system error (issue #159). + */ + @Test + void ignoringExecutorNotEndedIsNotDone() { + Task task = + task( + false, + JobState.SUCCEEDED, + JobState.RUNNING, + JobState.SUCCEEDED, + JobState.ABSENT); + ignoreError(task, 0, "true"); + assertThat(TaskStateResolver.resolve(task, () -> 2, () -> false)) + .isEqualTo(TesState.SYSTEM_ERROR); + } + /** * After an ignored failure of the last executor, a task with outputs (or unknown ones) * completed only if its outputs filer did, as the taskmaster may have stopped at the failure; a diff --git a/src/test/java/uk/ac/ebi/tsc/tesk/k8s/convert/TesKubernetesConverterTest.java b/src/test/java/uk/ac/ebi/tsc/tesk/k8s/convert/TesKubernetesConverterTest.java index 5fce9a91..a915c8e7 100644 --- a/src/test/java/uk/ac/ebi/tsc/tesk/k8s/convert/TesKubernetesConverterTest.java +++ b/src/test/java/uk/ac/ebi/tsc/tesk/k8s/convert/TesKubernetesConverterTest.java @@ -657,6 +657,36 @@ public void test_status_failed_executor() throws IOException { assertThat(state, is(TesState.EXECUTOR_ERROR)); } + /** + * An ignored failure needs the outputs filer if the stored task has outputs, and none without + * outputs (issue #159). + */ + @Test + public void test_status_ignored_failure_reads_outputs() throws IOException { + TaskBuilder taskBuilder = this.prepareBaseTaskBuider(); + V1Job executor = taskBuilder.getTask().getExecutors().get(0).getJob(); + executor.getStatus().succeeded(0).failed(1); + executor.getMetadata() + .putAnnotationsItem( + Constants.ANN_IGNORE_ERROR_KEY, Constants.ANN_IGNORE_ERROR_VALUE); + assertThat( + "stored task with outputs, no outputs filer", + this.converter.extractStateFromK8sJobs(taskBuilder.getTask()), + is(TesState.SYSTEM_ERROR)); + + taskBuilder + .getTask() + .getTaskmaster() + .getJob() + .getMetadata() + .getAnnotations() + .put(Constants.ANN_JSON_INPUT_KEY, "{\"executors\": [{}]}"); + assertThat( + "stored task without outputs", + this.converter.extractStateFromK8sJobs(taskBuilder.getTask()), + is(TesState.COMPLETE)); + } + @Test public void test_status_complete_with_failed_executor() throws IOException { TaskBuilder taskBuilder = this.prepareBaseTaskBuider(); From 9c97218ceb847a7234872c70e0a1d61d375f40af Mon Sep 17 00:00:00 2001 From: Alex Kanitz Date: Sat, 14 Nov 2026 00:26:58 +0100 Subject: [PATCH 5/5] test: use the literal ignore-error key The tests now write and check the annotation key of the contract with the taskmaster literally, so that a changed constant fails them (Sourcery). Co-Authored-By: Claude Opus 5.5 --- .../uk/ac/ebi/tsc/tesk/k8s/convert/TaskStateResolverTest.java | 3 +-- .../ebi/tsc/tesk/k8s/convert/TesKubernetesConverterTest.java | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/test/java/uk/ac/ebi/tsc/tesk/k8s/convert/TaskStateResolverTest.java b/src/test/java/uk/ac/ebi/tsc/tesk/k8s/convert/TaskStateResolverTest.java index 7fb2db81..dbbf494f 100644 --- a/src/test/java/uk/ac/ebi/tsc/tesk/k8s/convert/TaskStateResolverTest.java +++ b/src/test/java/uk/ac/ebi/tsc/tesk/k8s/convert/TaskStateResolverTest.java @@ -1,7 +1,6 @@ package uk.ac.ebi.tsc.tesk.k8s.convert; import static org.assertj.core.api.Assertions.assertThat; -import static uk.ac.ebi.tsc.tesk.k8s.constant.Constants.ANN_IGNORE_ERROR_KEY; import static uk.ac.ebi.tsc.tesk.k8s.constant.Constants.JOB_NAME_FILER_SUF; import static uk.ac.ebi.tsc.tesk.k8s.constant.Constants.LABEL_TASKSTATE_KEY; import static uk.ac.ebi.tsc.tesk.k8s.constant.Constants.LABEL_TASKSTATE_VALUE_CANC; @@ -780,7 +779,7 @@ private static void ignoreError(Task task, int executor, String value) { .get(executor) .getJob() .getMetadata() - .putAnnotationsItem(ANN_IGNORE_ERROR_KEY, value); + .putAnnotationsItem("ignore-error", value); // the contract with the taskmaster } private static Task task( diff --git a/src/test/java/uk/ac/ebi/tsc/tesk/k8s/convert/TesKubernetesConverterTest.java b/src/test/java/uk/ac/ebi/tsc/tesk/k8s/convert/TesKubernetesConverterTest.java index a915c8e7..e947fa83 100644 --- a/src/test/java/uk/ac/ebi/tsc/tesk/k8s/convert/TesKubernetesConverterTest.java +++ b/src/test/java/uk/ac/ebi/tsc/tesk/k8s/convert/TesKubernetesConverterTest.java @@ -8,7 +8,6 @@ import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.BDDMockito.given; import static org.mockito.Mockito.*; -import static uk.ac.ebi.tsc.tesk.k8s.constant.Constants.ANN_IGNORE_ERROR_KEY; import static uk.ac.ebi.tsc.tesk.k8s.constant.Constants.LABEL_TASKSTATE_KEY; import static uk.ac.ebi.tsc.tesk.k8s.constant.Constants.LABEL_TASKSTATE_VALUE_CANC; @@ -939,7 +938,7 @@ public void fromTesExecutorToK8sJob_ignoreError() { 0, null, User.builder("test-user-id").build()); - String annotation = job.getMetadata().getAnnotations().get(ANN_IGNORE_ERROR_KEY); + String annotation = job.getMetadata().getAnnotations().get("ignore-error"); assertEquals( "ignore_error: " + ignoreError, Boolean.TRUE.equals(ignoreError) ? "true" : null,