diff --git a/docs/api.md b/docs/api.md index 45a90950..c2c94052 100644 --- a/docs/api.md +++ b/docs/api.md @@ -27,10 +27,12 @@ Kubernetes lists their taskmaster jobs (by task ID), in the views `MINIMAL` (the default), `BASIC` and `FULL`. - `page_size` sets how many tasks TESK API reads from Kubernetes for one - page, from 1 to 2048; without it, it reads all of them at once. Other - values, also those that aren't whole numbers, are refused with - `400 Bad Request`. `next_page_token` is the continue token of Kubernetes; - pass it as `page_token` to get the next page. + page, from 1 to 2048 (TES asks for less than 2048). Without it, or if it + is empty, a page has at most 256 tasks, the default of TES, so a request + without `page_size` can also return a `next_page_token`: follow it to get + all tasks. Other values, also those that aren't whole numbers, are + refused with `400 Bad Request`. `next_page_token` is the continue token + of Kubernetes; pass it as `page_token` to get the next page. It is left out on the last page. Kubernetes accepts it for a limited time only (by default about 5 minutes, until etcd compacts its history). - `name_prefix` lists only the tasks whose names start with it @@ -38,12 +40,8 @@ Kubernetes lists their taskmaster jobs (by task ID), in the views `MINIMAL` filters nothing. - `state` lists only the tasks in that state. It accepts every `tesState` of TES 1.1, in capitals (e.g. `RUNNING`, also `CANCELING` and -<<<<<<< HEAD `PREEMPTED`; see [Getting tasks](index.md#getting-tasks) for when TESK API reports them); -======= - `PREEMPTED`, which TESK API doesn't report yet, so they list no tasks); ->>>>>>> origin/dev other values are refused with `400 Bad Request`, and the message names `state`. An empty `state` filters nothing. - `tag_key` and `tag_value` list only the tasks with these tags. Repeat @@ -128,6 +126,32 @@ TESK accepts the fields that TES 1.1 added to tasks: Service info reports the TES version 1.1.0 (`type.version`). +## Differences from TES 1.1 + +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). +- `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 + fields](#tes-11-fields)). +- `page_size` may also be 2048, while TES asks for less (see [Listing + tasks](#listing-tasks)). +- In executor logs, `exit_code` is left out until the executor's container + has terminated, and `stdout` holds both output streams of the executor, + while `stderr` is left out (see [Executor logs](#executor-logs)). +- `logs[].outputs` is always empty (see [Output file + logs](#output-file-logs); + [#160](https://github.com/elixir-cloud-aai/tesk-api/issues/160) is about + reporting them). +- TESK API sends no CORS headers. To use it from web pages of other origins, + as TES asks of such implementations, configure CORS where you expose it, + e.g. in your ingress controller. + ## Resources `resources.cpu_cores` of a task sets both the CPU request and the CPU limit of diff --git a/docs/upgrading.md b/docs/upgrading.md index ccf08f9f..a4045152 100644 --- a/docs/upgrading.md +++ b/docs/upgrading.md @@ -11,6 +11,11 @@ From 1.2.0, clients may notice changed answers: Before, such a task ran, and its output then failed or uploaded nothing useful. Clients that send such outputs to other URLs set `path_prefix`, e.g. to the part of `path` before its first wildcard. +- **Default page size.** `GET /tasks` without `page_size` returns at most + 256 tasks per page, as TES specifies, with a `next_page_token` if there + 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. ## To 1.2.0 diff --git a/src/integration-test/java/uk/ac/ebi/tsc/tesk/AuthIT.java b/src/integration-test/java/uk/ac/ebi/tsc/tesk/AuthIT.java index 209440b4..3a57a2f6 100644 --- a/src/integration-test/java/uk/ac/ebi/tsc/tesk/AuthIT.java +++ b/src/integration-test/java/uk/ac/ebi/tsc/tesk/AuthIT.java @@ -561,7 +561,8 @@ public void member_getList() throws Exception { mockKubernetes.givenThat( WireMock.get( "/apis/batch/v1/namespaces/default/jobs?labelSelector=job-type%3Dtaskmaster" - + "%2Ccreator-group-name%20in%20%28TEST%29%2Ccreator-user-hash%3Da665a45920422f9d417e4867efdc4fb8") + + "%2Ccreator-group-name%20in%20%28TEST%29%2Ccreator-user-hash%3Da665a45920422f9d417e4867efdc4fb8" + + "&limit=256") .willReturn(aResponse().withBodyFile("list/taskmasters.json"))); MockUtil.mockListTaskKubernetesResponses(this.mockKubernetes); @@ -580,7 +581,8 @@ public void superAdmin_getList() throws Exception { mockKubernetes.givenThat( WireMock.get( - "/apis/batch/v1/namespaces/default/jobs?labelSelector=job-type%3Dtaskmaster") + "/apis/batch/v1/namespaces/default/jobs?labelSelector=job-type%3Dtaskmaster" + + "&limit=256") .willReturn(aResponse().withBodyFile("list/taskmasters.json"))); MockUtil.mockListTaskKubernetesResponses(this.mockKubernetes); @@ -599,7 +601,8 @@ public void groupAdmin_getList() throws Exception { mockKubernetes.givenThat( WireMock.get( "/apis/batch/v1/namespaces/default/jobs?labelSelector=job-type%3Dtaskmaster" - + "%2Ccreator-group-name%20in%20%28TEST%29") + + "%2Ccreator-group-name%20in%20%28TEST%29" + + "&limit=256") .willReturn(aResponse().withBodyFile("list/taskmasters.json"))); MockUtil.mockListTaskKubernetesResponses(this.mockKubernetes); @@ -620,7 +623,8 @@ public void mixed_getList() throws Exception { mockKubernetes.givenThat( WireMock.get( "/apis/batch/v1/namespaces/default/jobs?labelSelector=job-type%3Dtaskmaster" - + "%2Ccreator-group-name%20in%20%28TEST%2CABC%29") + + "%2Ccreator-group-name%20in%20%28TEST%2CABC%29" + + "&limit=256") .willReturn(aResponse().withBodyFile("list/taskmasters.json"))); MockUtil.mockListTaskKubernetesResponses(this.mockKubernetes); diff --git a/src/integration-test/java/uk/ac/ebi/tsc/tesk/ClaimAuthIT.java b/src/integration-test/java/uk/ac/ebi/tsc/tesk/ClaimAuthIT.java index 84f23786..84f0f6a1 100644 --- a/src/integration-test/java/uk/ac/ebi/tsc/tesk/ClaimAuthIT.java +++ b/src/integration-test/java/uk/ac/ebi/tsc/tesk/ClaimAuthIT.java @@ -217,7 +217,8 @@ public void claimUser_getList() throws Exception { mockKubernetes.givenThat( WireMock.get( "/apis/batch/v1/namespaces/default/jobs?labelSelector=job-type%3Dtaskmaster" - + "%2Ccreator-user-hash%3Dd159ef624ed86697b4f1f3ff086aacdd%2C%21creator-group-name") + + "%2Ccreator-user-hash%3Dd159ef624ed86697b4f1f3ff086aacdd%2C%21creator-group-name" + + "&limit=256") .willReturn( okJson( MockUtil.jobList( @@ -236,7 +237,8 @@ public void superAdmin_getList() throws Exception { userInfo("123", assurance, "GA4GH:GA4GH-CAP:EBI:ADMIN"); mockKubernetes.givenThat( WireMock.get( - "/apis/batch/v1/namespaces/default/jobs?labelSelector=job-type%3Dtaskmaster") + "/apis/batch/v1/namespaces/default/jobs?labelSelector=job-type%3Dtaskmaster" + + "&limit=256") .willReturn(aResponse().withBodyFile("list/taskmasters.json"))); MockUtil.mockListTaskKubernetesResponses(this.mockKubernetes); @@ -252,7 +254,8 @@ public void member_getList() throws Exception { mockKubernetes.givenThat( WireMock.get( "/apis/batch/v1/namespaces/default/jobs?labelSelector=job-type%3Dtaskmaster" - + "%2Ccreator-group-name%20in%20%28TEST%29%2Ccreator-user-hash%3Da665a45920422f9d417e4867efdc4fb8") + + "%2Ccreator-group-name%20in%20%28TEST%29%2Ccreator-user-hash%3Da665a45920422f9d417e4867efdc4fb8" + + "&limit=256") .willReturn(aResponse().withBodyFile("list/taskmasters.json"))); MockUtil.mockListTaskKubernetesResponses(this.mockKubernetes); diff --git a/src/integration-test/java/uk/ac/ebi/tsc/tesk/LegacyUserLabelsIT.java b/src/integration-test/java/uk/ac/ebi/tsc/tesk/LegacyUserLabelsIT.java index b814edd3..1c96fe17 100644 --- a/src/integration-test/java/uk/ac/ebi/tsc/tesk/LegacyUserLabelsIT.java +++ b/src/integration-test/java/uk/ac/ebi/tsc/tesk/LegacyUserLabelsIT.java @@ -112,7 +112,8 @@ public void member_getList() throws Exception { mockKubernetes.givenThat( WireMock.get( "/apis/batch/v1/namespaces/default/jobs?labelSelector=job-type%3Dtaskmaster" - + "%2Ccreator-group-name%20in%20%28TEST%29") + + "%2Ccreator-group-name%20in%20%28TEST%29" + + "&limit=256") .willReturn(aResponse().withBodyFile("list/taskmasters.json"))); MockUtil.mockListTaskKubernetesResponses(this.mockKubernetes); diff --git a/src/integration-test/java/uk/ac/ebi/tsc/tesk/NoAuthIT.java b/src/integration-test/java/uk/ac/ebi/tsc/tesk/NoAuthIT.java index 4adb5bdf..39cb8572 100644 --- a/src/integration-test/java/uk/ac/ebi/tsc/tesk/NoAuthIT.java +++ b/src/integration-test/java/uk/ac/ebi/tsc/tesk/NoAuthIT.java @@ -252,7 +252,8 @@ public void getList() throws Exception { mockKubernetes.givenThat( WireMock.get( - "/apis/batch/v1/namespaces/default/jobs?labelSelector=job-type%3Dtaskmaster") + "/apis/batch/v1/namespaces/default/jobs?labelSelector=job-type%3Dtaskmaster" + + "&limit=256") .willReturn(aResponse().withBodyFile("list/taskmasters.json"))); MockUtil.mockListTaskKubernetesResponses(this.mockKubernetes); @@ -260,6 +261,12 @@ public void getList() throws Exception { .perform(get(TASK_URL).header("Authorization", "Bearer BAR")) .andExpect(status().isOk()) .andExpect(jsonPath("$.tasks.length()").value(4)); + // without page_size, Kubernetes is asked for a page of 256 taskmaster jobs, the default + // of TES (issue #137) + mockKubernetes.verify( + getRequestedFor(urlPathEqualTo("/apis/batch/v1/namespaces/default/jobs")) + .withQueryParam("labelSelector", equalTo("job-type=taskmaster")) + .withQueryParam("limit", equalTo("256"))); this.mvc .perform(get(TASK_URL + "?view=BASIC").header("Authorization", "Bearer BAR")) .andExpect(status().isOk()) @@ -313,7 +320,8 @@ public void getListFilteredByStateAndTags() throws Exception { cancelled.getMetadata().putLabelsItem("task-status", "Cancelled"); mockKubernetes.givenThat( WireMock.get( - "/apis/batch/v1/namespaces/default/jobs?labelSelector=job-type%3Dtaskmaster") + "/apis/batch/v1/namespaces/default/jobs?labelSelector=job-type%3Dtaskmaster" + + "&limit=256") .willReturn( okJson( MockUtil.jobList( diff --git a/src/integration-test/java/uk/ac/ebi/tsc/tesk/ServiceClientAuthIT.java b/src/integration-test/java/uk/ac/ebi/tsc/tesk/ServiceClientAuthIT.java index 2e102dd8..e84a4628 100644 --- a/src/integration-test/java/uk/ac/ebi/tsc/tesk/ServiceClientAuthIT.java +++ b/src/integration-test/java/uk/ac/ebi/tsc/tesk/ServiceClientAuthIT.java @@ -210,7 +210,8 @@ public void serviceClient_getList_ownTasksOnly() throws Exception { "/apis/batch/v1/namespaces/default/jobs?labelSelector=job-type%3Dtaskmaster" + "%2Ccreator-user-hash%3D" + CreatorLabels.hash(CLIENT) - + "%2C%21creator-group-name") + + "%2C%21creator-group-name" + + "&limit=256") .willReturn( okJson( MockUtil.jobList( @@ -309,7 +310,8 @@ public void userToken_getList_byGroups() throws Exception { + "GA4GH:GA4GH-CAP:EBI:ADMIN#perun.elixir-czech.cz\"]}"))); mockKubernetes.givenThat( WireMock.get( - "/apis/batch/v1/namespaces/default/jobs?labelSelector=job-type%3Dtaskmaster") + "/apis/batch/v1/namespaces/default/jobs?labelSelector=job-type%3Dtaskmaster" + + "&limit=256") .willReturn(aResponse().withBodyFile("list/taskmasters.json"))); MockUtil.mockListTaskKubernetesResponses(this.mockKubernetes); diff --git a/src/main/java/uk/ac/ebi/tsc/tesk/tes/api/TasksApiController.java b/src/main/java/uk/ac/ebi/tsc/tesk/tes/api/TasksApiController.java index 7c1c90aa..a222196d 100644 --- a/src/main/java/uk/ac/ebi/tsc/tesk/tes/api/TasksApiController.java +++ b/src/main/java/uk/ac/ebi/tsc/tesk/tes/api/TasksApiController.java @@ -40,6 +40,9 @@ public class TasksApiController implements TasksApi { */ static final long MAX_PAGE_SIZE = 2048; + /** page_size of listTasks if it is left out or empty: 256, the default of TES (issue #137) */ + static final long DEFAULT_PAGE_SIZE = 256; + private final TesService tesService; /** @@ -113,7 +116,9 @@ public ResponseEntity listTasks( value = "OPTIONAL. Number of tasks to return in one page, from 1 to " + MAX_PAGE_SIZE - + ". Without it, all tasks are returned in one page. Pages can be shorter, also with name_prefix.") + + ". Defaults to " + + DEFAULT_PAGE_SIZE + + ". Pages can be shorter, also with name_prefix.") @RequestParam(value = "page_size", required = false) Long pageSize, @ApiParam( @@ -129,7 +134,8 @@ public ResponseEntity listTasks( @RequestParam(value = "view", required = false, defaultValue = "MINIMAL") String view) { - if (pageSize != null && (pageSize < 1 || pageSize > MAX_PAGE_SIZE)) { + long size = pageSize == null ? DEFAULT_PAGE_SIZE : pageSize; + if (size < 1 || size > MAX_PAGE_SIZE) { throw new InvalidPageSizeException(MAX_PAGE_SIZE); } TaskFilter filter = @@ -137,7 +143,7 @@ public ResponseEntity listTasks( namePrefix, state, TaskFilter.zipTags(asList(tagKey), asList(tagValue))); TesListTasksResponse response = this.tesService.listTasks( - filter, pageSize, pageToken, TaskView.fromString(view), this.getUser()); + filter, size, pageToken, TaskView.fromString(view), this.getUser()); return new ResponseEntity(response, HttpStatus.OK); } diff --git a/src/main/java/uk/ac/ebi/tsc/tesk/tes/service/TesService.java b/src/main/java/uk/ac/ebi/tsc/tesk/tes/service/TesService.java index 5a2fab78..b96871e7 100644 --- a/src/main/java/uk/ac/ebi/tsc/tesk/tes/service/TesService.java +++ b/src/main/java/uk/ac/ebi/tsc/tesk/tes/service/TesService.java @@ -51,7 +51,8 @@ public interface TesService { * * @param filter - only tasks that match it (name prefix, state and tags; see {@link * TaskFilter}); {@link TaskFilter#NONE} for all tasks - * @param pageSize - maximum number of taskmaster jobs read for the page; all if null + * @param pageSize - maximum number of taskmaster jobs read for the page; all if null (the API + * passes its default of 256 instead, see TasksApiController) * @param pageToken - continue token of Kubernetes, from the previous page's next_page_token * @param view - one of {@link TaskView} values, decides on how much detail is put in each * resulting task 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 e4e33a95..99d9b0b3 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 @@ -11,6 +11,7 @@ import static org.mockito.BDDMockito.given; import static org.mockito.Mockito.inOrder; import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; @@ -631,6 +632,20 @@ public void listTasks_validPageSize() throws Exception { } } + /** + * Without page_size, or with an empty one, a page has at most 256 tasks, the default of TES + * (issue #137) + */ + @Test + public void listTasks_defaultPageSize() throws Exception { + given(this.tesService.listTasks(any(), any(), any(), any(), any())) + .willReturn(new TesListTasksResponse()); + listWith().andExpect(status().isOk()); + listWith("page_size", "").andExpect(status().isOk()); + verify(this.tesService, times(2)) + .listTasks(eq(TaskFilter.NONE), eq(256L), isNull(), eq(TaskView.MINIMAL), any()); + } + /** * An expired or malformed page_token is a client error: 400 with a message that names the * parameter, never its value (issue #114) @@ -672,7 +687,7 @@ public void listTasks_filtersByEveryState() throws Exception { verify(this.tesService) .listTasks( eq(new TaskFilter(null, state, null)), - isNull(), + eq(TasksApiController.DEFAULT_PAGE_SIZE), isNull(), eq(TaskView.MINIMAL), any()); @@ -741,7 +756,7 @@ public void listTasks_singleTagKeyKeepsCommas() throws Exception { null, null, Collections.singletonList(TaskFilter.tag("a,b", "c,d")))), - isNull(), + eq(TasksApiController.DEFAULT_PAGE_SIZE), isNull(), eq(TaskView.MINIMAL), any()); @@ -769,7 +784,7 @@ public void listTasks_emptyFiltersFilterNothing() throws Exception { verify(this.tesService) .listTasks( eq(new TaskFilter("", null, null)), - isNull(), + eq(TasksApiController.DEFAULT_PAGE_SIZE), isNull(), eq(TaskView.MINIMAL), any());