*/
@Configuration
@@ -11,6 +11,6 @@ public class GsonConfig {
@Bean
Gson gson() {
- return new JSON().getGson();
+ return KubernetesJson.GSON;
}
}
diff --git a/src/main/java/uk/ac/ebi/tsc/tesk/k8s/config/KubernetesClientConfig.java b/src/main/java/uk/ac/ebi/tsc/tesk/k8s/config/KubernetesClientConfig.java
index d6f85999..bdd5652f 100644
--- a/src/main/java/uk/ac/ebi/tsc/tesk/k8s/config/KubernetesClientConfig.java
+++ b/src/main/java/uk/ac/ebi/tsc/tesk/k8s/config/KubernetesClientConfig.java
@@ -15,6 +15,14 @@
@Configuration
public class KubernetesClientConfig {
+ /**
+ * Lets the Kubernetes client read objects of Kubernetes versions newer than itself, before any
+ * of the API clients below is used
+ */
+ public KubernetesClientConfig() {
+ KubernetesJson.install();
+ }
+
@Bean
@Primary
public ApiClient kubernetesApiClient() throws IOException {
@@ -38,13 +46,13 @@ public CoreV1Api coreApi(ApiClient apiClient) {
/**
* Workaround to enable merge patching (Kube client does JSON patch only - headers hardcoded)
- * Separate instance of ApiClient only for running PATCH calls
+ * Separate instance of ApiClient only for running PATCH calls, with the connection and
+ * credentials of the default one
*/
@Bean(name = "patchApiClient")
- public ApiClient patchApiClient() throws IOException {
+ public ApiClient patchApiClient(ApiClient kubernetesApiClient) {
- ApiClient defaultClient = Config.defaultClient();
- return new ApiClientWithMergePatching(defaultClient);
+ return new ApiClientWithMergePatching(kubernetesApiClient);
}
/** Separate object to perform merge-patching of Jobs */
@@ -71,7 +79,11 @@ private static class ApiClientWithMergePatching extends ApiClient {
ApiClientWithMergePatching(ApiClient apiClient) {
this.setBasePath(apiClient.getBasePath());
this.setHttpClient(apiClient.getHttpClient());
- this.setApiKey(((ApiKeyAuth) apiClient.getAuthentication("BearerToken")).getApiKey());
+ // the token (from a kubeconfig file; in a cluster, the HTTP client adds it) with its
+ // prefix ("Bearer"), without which Kubernetes refuses it
+ ApiKeyAuth auth = (ApiKeyAuth) apiClient.getAuthentication("BearerToken");
+ this.setApiKeyPrefix(auth.getApiKeyPrefix());
+ this.setApiKey(auth.getApiKey());
}
@Override
diff --git a/src/main/java/uk/ac/ebi/tsc/tesk/k8s/config/KubernetesJson.java b/src/main/java/uk/ac/ebi/tsc/tesk/k8s/config/KubernetesJson.java
new file mode 100644
index 00000000..a129c512
--- /dev/null
+++ b/src/main/java/uk/ac/ebi/tsc/tesk/k8s/config/KubernetesJson.java
@@ -0,0 +1,89 @@
+package uk.ac.ebi.tsc.tesk.k8s.config;
+
+import com.google.gson.Gson;
+import com.google.gson.TypeAdapter;
+import com.google.gson.TypeAdapterFactory;
+import com.google.gson.reflect.TypeToken;
+import com.google.gson.stream.JsonReader;
+import com.google.gson.stream.JsonWriter;
+import io.kubernetes.client.openapi.JSON;
+import java.io.IOException;
+import java.time.LocalDate;
+import java.time.OffsetDateTime;
+import java.util.Collection;
+import java.util.Map;
+
+/**
+ * Gson for the objects of the Kubernetes API that ignores fields it doesn't know, as the Kubernetes
+ * client did up to version 19 (and as client-go does).
+ *
+ * The models of the Kubernetes client since version 20 refuse JSON with fields that they don't
+ * know, so every object that a newer Kubernetes version extends (e.g. with a field of a job's or
+ * pod's status) would make reading it fail (https://github.com/kubernetes-client/java/issues/4941).
+ * TESK API reads jobs, pods, ConfigMaps and LimitRanges, and only creates objects from its own
+ * templates and patches them with its own patches, so ignoring unknown fields loses nothing. It
+ * doesn't check required fields either. Everything else is as in the client's Gson: times (RFC
+ * 3339), byte arrays (Base64), and the client's handling of Status objects and managed fields.
+ */
+public final class KubernetesJson {
+
+ /** The client's Gson, for its adapters of times and byte arrays */
+ private static final Gson CLIENT_GSON = JSON.getGson();
+
+ public static final Gson GSON =
+ JSON.createGson()
+ .registerTypeAdapter(
+ OffsetDateTime.class, CLIENT_GSON.getAdapter(OffsetDateTime.class))
+ .registerTypeAdapter(LocalDate.class, CLIENT_GSON.getAdapter(LocalDate.class))
+ .registerTypeAdapter(byte[].class, CLIENT_GSON.getAdapter(byte[].class))
+ .create();
+
+ /**
+ * {@link #GSON}, but leaving out lists and maps that are empty, as the Kubernetes client did up
+ * to version 19: its models now start with empty lists and maps instead of none. For the
+ * executors' jobs in the taskmaster's input, as the taskmaster tells "no limits" (which it sets
+ * to the requests) from limits by whether the key is there. Objects without fields (e.g. {@code
+ * emptyDir: {}}) are kept.
+ */
+ public static final Gson GSON_WITHOUT_EMPTY_COLLECTIONS =
+ GSON.newBuilder().registerTypeAdapterFactory(new WithoutEmptyCollections()).create();
+
+ private KubernetesJson() {}
+
+ /** Writes empty lists and maps as null, which Gson leaves out of objects */
+ private static final class WithoutEmptyCollections implements TypeAdapterFactory {
+
+ @Override
+ public TypeAdapter create(Gson gson, TypeToken type) {
+ Class raw = type.getRawType();
+ if (!Collection.class.isAssignableFrom(raw) && !Map.class.isAssignableFrom(raw)) {
+ return null;
+ }
+ TypeAdapter delegate = gson.getDelegateAdapter(this, type);
+ return new TypeAdapter() {
+ @Override
+ public void write(JsonWriter out, T value) throws IOException {
+ if (value instanceof Collection && ((Collection) value).isEmpty()
+ || value instanceof Map && ((Map) value).isEmpty()) {
+ out.nullValue();
+ } else {
+ delegate.write(out, value);
+ }
+ }
+
+ @Override
+ public T read(JsonReader in) throws IOException {
+ return delegate.read(in);
+ }
+ };
+ }
+ }
+
+ /**
+ * Lets the Kubernetes client read responses with {@link #GSON}: its Gson is global (static),
+ * shared by all API clients
+ */
+ public static void install() {
+ JSON.setGson(GSON);
+ }
+}
diff --git a/src/main/java/uk/ac/ebi/tsc/tesk/k8s/convert/KubernetesObjectsSupplier.java b/src/main/java/uk/ac/ebi/tsc/tesk/k8s/convert/KubernetesObjectsSupplier.java
index c7089f02..2b53248c 100644
--- a/src/main/java/uk/ac/ebi/tsc/tesk/k8s/convert/KubernetesObjectsSupplier.java
+++ b/src/main/java/uk/ac/ebi/tsc/tesk/k8s/convert/KubernetesObjectsSupplier.java
@@ -65,8 +65,8 @@ private static V1PodSecurityContext createSecurityContext(
confmap_namespace = System.getenv(envConfigMapNamespace);
}
V1ConfigMap config =
- coreApi.readNamespacedConfigMap(
- System.getenv(envConfigMap), confmap_namespace, null, false, false);
+ coreApi.readNamespacedConfigMap(System.getenv(envConfigMap), confmap_namespace)
+ .execute();
String config_key = "securityContext";
if (System.getenv(envConfigMapKey) != null) {
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 1bc36af0..28a2376d 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
@@ -13,6 +13,7 @@
import io.kubernetes.client.openapi.models.V1Pod;
import io.kubernetes.client.openapi.models.V1PodCondition;
import io.kubernetes.client.openapi.models.V1PodStatus;
+import java.time.OffsetDateTime;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashSet;
@@ -23,7 +24,6 @@
import java.util.function.Supplier;
import java.util.stream.Collectors;
import java.util.stream.Stream;
-import org.joda.time.DateTime;
import uk.ac.ebi.tsc.tesk.k8s.constant.K8sConstants;
import uk.ac.ebi.tsc.tesk.k8s.convert.data.Job;
import uk.ac.ebi.tsc.tesk.k8s.convert.data.Task;
@@ -99,11 +99,12 @@ private static boolean hasEnded(V1Pod pod) {
if (pod.getStatus() != null && ENDED_POD_PHASES.contains(pod.getStatus().getPhase())) {
return true;
}
- DateTime deletion =
+ OffsetDateTime deletion =
Optional.ofNullable(pod.getMetadata())
.map(V1ObjectMeta::getDeletionTimestamp)
.orElse(null);
- return deletion != null && deletion.plusSeconds(DELETION_SLACK_SECONDS).isBeforeNow();
+ return deletion != null
+ && deletion.plusSeconds(DELETION_SLACK_SECONDS).isBefore(OffsetDateTime.now());
}
/**
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 0c155fa0..c54fead7 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
@@ -12,18 +12,19 @@
import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.nio.charset.StandardCharsets;
+import java.time.ZoneOffset;
+import java.time.format.DateTimeFormatter;
import java.util.*;
import java.util.function.Supplier;
import java.util.stream.Collectors;
import java.util.stream.IntStream;
import java.util.zip.GZIPOutputStream;
-import org.joda.time.format.DateTimeFormatter;
-import org.joda.time.format.ISODateTimeFormat;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.beans.factory.annotation.Qualifier;
import org.springframework.stereotype.Component;
import uk.ac.ebi.tsc.tesk.config.security.User;
+import uk.ac.ebi.tsc.tesk.k8s.config.KubernetesJson;
import uk.ac.ebi.tsc.tesk.k8s.convert.data.Job;
import uk.ac.ebi.tsc.tesk.k8s.convert.data.Task;
import uk.ac.ebi.tsc.tesk.tes.model.*;
@@ -49,8 +50,9 @@ public class TesKubernetesConverter {
private final TrsToolClient trsClient;
+ /** Times in UTC, with milliseconds (e.g. 2018-05-31T09:21:57.000Z) */
private static final DateTimeFormatter DATE_FORMATTER =
- ISODateTimeFormat.dateTime().withZoneUTC();
+ DateTimeFormatter.ofPattern("yyyy-MM-dd'T'HH:mm:ss.SSSXXX").withZone(ZoneOffset.UTC);
public TesKubernetesConverter(
@Qualifier("executor") Supplier executorTemplateSupplier,
@@ -179,7 +181,13 @@ public V1ConfigMap fromTesTaskToK8sConfigMap(TesTask task, User user, V1Job job)
taskMasterConfigMap.getMetadata().getName()),
e);
}
- taskMasterInput.put(TASKMASTER_INPUT_EXEC_KEY, executorsAsJobs);
+ // without the empty lists and maps of the client's models, as before (see
+ // KubernetesJson)
+ taskMasterInput.put(
+ TASKMASTER_INPUT_EXEC_KEY,
+ executorsAsJobs.stream()
+ .map(KubernetesJson.GSON_WITHOUT_EMPTY_COLLECTIONS::toJsonTree)
+ .collect(Collectors.toList()));
String taskMasterInputAsJSON = this.gson.toJson(taskMasterInput);
try {
@@ -512,12 +520,12 @@ public TesExecutorLog extractExecutorLogFromK8sJobAndPod(Job executor) {
log.setStartTime(
Optional.ofNullable(executorJob.getStatus())
.map(V1JobStatus::getStartTime)
- .map(DATE_FORMATTER::print)
+ .map(DATE_FORMATTER::format)
.orElse(null));
log.setEndTime(
Optional.ofNullable(executorJob.getStatus())
.map(V1JobStatus::getCompletionTime)
- .map(DATE_FORMATTER::print)
+ .map(DATE_FORMATTER::format)
.orElse(null));
// workaround for py-tes compatibility. py-tes breaks for ExecutorLogs that can unmarshal to
// TaskLogs. Non-null stdout prevents that.
@@ -605,7 +613,11 @@ public TesTask fromK8sJobsToTesTaskBasic(
}
task.setId(taskMasterJobMetadata.getName());
task.setState(this.extractStateFromK8sJobs(taskmasterWithExecutors));
- task.setCreationTime(DATE_FORMATTER.print(taskMasterJobMetadata.getCreationTimestamp()));
+ // Kubernetes sets it for every job it stores
+ task.setCreationTime(
+ Optional.ofNullable(taskMasterJobMetadata.getCreationTimestamp())
+ .map(DATE_FORMATTER::format)
+ .orElse(null));
TesTaskLog log = new TesTaskLog();
task.addLogsItem(log);
// the files that the outputs filer uploaded, as the taskmaster stored them (issue #160)
@@ -618,12 +630,12 @@ public TesTask fromK8sJobsToTesTaskBasic(
log.setStartTime(
Optional.ofNullable(taskMasterJob.getStatus())
.map(V1JobStatus::getStartTime)
- .map(DATE_FORMATTER::print)
+ .map(DATE_FORMATTER::format)
.orElse(null));
log.setEndTime(
Optional.ofNullable(taskMasterJob.getStatus())
.map(V1JobStatus::getCompletionTime)
- .map(DATE_FORMATTER::print)
+ .map(DATE_FORMATTER::format)
.orElse(null));
for (Job executorJob : taskmasterWithExecutors.getExecutors()) {
TesExecutorLog executorLog = this.extractExecutorLogFromK8sJobAndPod(executorJob);
diff --git a/src/main/java/uk/ac/ebi/tsc/tesk/k8s/service/KubernetesClientWrapper.java b/src/main/java/uk/ac/ebi/tsc/tesk/k8s/service/KubernetesClientWrapper.java
index b2db4222..7bfd8051 100644
--- a/src/main/java/uk/ac/ebi/tsc/tesk/k8s/service/KubernetesClientWrapper.java
+++ b/src/main/java/uk/ac/ebi/tsc/tesk/k8s/service/KubernetesClientWrapper.java
@@ -64,7 +64,7 @@ public KubernetesClientWrapper(
public V1Job createJob(V1Job job) {
try {
- return this.batchApi.createNamespacedJob(namespace, job, null, null, null);
+ return this.batchApi.createNamespacedJob(namespace, job).execute();
} catch (ApiException e) {
throw KubernetesException.fromApiException(e);
}
@@ -77,8 +77,10 @@ public V1Job createJob(V1Job job) {
*/
public void deleteJob(String name) {
try {
- this.batchApi.deleteNamespacedJob(
- name, namespace, null, null, null, null, PROPAGATION_POLICY_BACKGROUND, null);
+ this.batchApi
+ .deleteNamespacedJob(name, namespace)
+ .propagationPolicy(PROPAGATION_POLICY_BACKGROUND)
+ .execute();
} catch (ApiException e) {
throw KubernetesException.fromApiException(e);
}
@@ -95,7 +97,7 @@ public void deleteJob(String name) {
*/
public void checkConfigMap(V1ConfigMap map) {
try {
- this.coreApi.createNamespacedConfigMap(namespace, map, null, DRY_RUN_ALL, null);
+ this.coreApi.createNamespacedConfigMap(namespace, map).dryRun(DRY_RUN_ALL).execute();
} catch (ApiException e) {
if (isDryRunUnsupported(e)) {
logger.warn(
@@ -120,7 +122,7 @@ private static boolean isDryRunUnsupported(ApiException e) {
public V1ConfigMap createConfigMap(V1ConfigMap map) {
try {
- return this.coreApi.createNamespacedConfigMap(namespace, map, null, null, null);
+ return this.coreApi.createNamespacedConfigMap(namespace, map).execute();
} catch (ApiException e) {
throw KubernetesException.fromApiException(e);
}
@@ -129,7 +131,7 @@ public V1ConfigMap createConfigMap(V1ConfigMap map) {
/** Checks the Secret's admission and permissions before creating the taskmaster job. */
public void checkSecret(V1Secret secret) {
try {
- this.coreApi.createNamespacedSecret(namespace, secret, null, DRY_RUN_ALL, null);
+ this.coreApi.createNamespacedSecret(namespace, secret).dryRun(DRY_RUN_ALL).execute();
} catch (ApiException e) {
if (isDryRunUnsupported(e)) {
logger.warn(
@@ -143,7 +145,7 @@ public void checkSecret(V1Secret secret) {
public V1Secret createSecret(V1Secret secret) {
try {
- return this.coreApi.createNamespacedSecret(namespace, secret, null, null, null);
+ return this.coreApi.createNamespacedSecret(namespace, secret).execute();
} catch (ApiException e) {
throw KubernetesException.fromSecretApiException(e);
}
@@ -159,7 +161,7 @@ public V1Secret createSecret(V1Secret secret) {
*/
public V1Job readTaskmasterJob(String taskId) {
try {
- V1Job job = this.batchApi.readNamespacedJob(taskId, namespace, null, null, null);
+ V1Job job = this.batchApi.readNamespacedJob(taskId, namespace).execute();
// a job without metadata or labels isn't a taskmaster's job either
V1ObjectMeta metadata = job.getMetadata();
Map labels = metadata == null ? null : metadata.getLabels();
@@ -178,18 +180,12 @@ public V1Job readTaskmasterJob(String taskId) {
private V1JobList listJobs(String _continue, String labelSelector, Integer limit) {
try {
- return this.batchApi.listNamespacedJob(
- namespace,
- null,
- null,
- _continue,
- null,
- labelSelector,
- limit,
- null,
- null,
- null,
- null);
+ return this.batchApi
+ .listNamespacedJob(namespace)
+ ._continue(_continue)
+ .labelSelector(labelSelector)
+ .limit(limit)
+ .execute();
} catch (ApiException e) {
throw KubernetesException.fromApiException(e);
}
@@ -202,18 +198,12 @@ private V1JobList listJobs(String _continue, String labelSelector, Integer limit
*/
private V1LimitRangeList listLimits(String _continue, String labelSelector, Integer limit) {
try {
- return this.coreApi.listNamespacedLimitRange(
- namespace,
- (String) null,
- (Boolean) null,
- _continue,
- (String) null,
- labelSelector,
- limit,
- (String) null,
- (String) null,
- (Integer) null,
- (Boolean) null);
+ return this.coreApi
+ .listNamespacedLimitRange(namespace)
+ ._continue(_continue)
+ .labelSelector(labelSelector)
+ .limit(limit)
+ .execute();
} catch (ApiException e) {
throw KubernetesException.fromApiException(e);
}
@@ -349,8 +339,9 @@ public V1JobList listSingleTaskExecutorJobs(String taskId) {
public Optional getSingleTaskOutputFilerJob(String taskId) {
try {
V1Job job =
- this.batchApi.readNamespacedJob(
- taskId + JOB_NAME_FILER_SUF, namespace, null, null, null);
+ this.batchApi
+ .readNamespacedJob(taskId + JOB_NAME_FILER_SUF, namespace)
+ .execute();
return Optional.of(job);
} catch (ApiException e) {
if (e.getCode() != HttpStatus.NOT_FOUND.value()) {
@@ -380,8 +371,7 @@ public V1PodList listSingleJobPods(V1Job job) {
.map(entry -> entry.getKey() + "=" + entry.getValue())
.collect(Collectors.joining(","));
try {
- return this.coreApi.listNamespacedPod(
- namespace, null, null, null, null, labelSelector, null, null, null, null, null);
+ return this.coreApi.listNamespacedPod(namespace).labelSelector(labelSelector).execute();
} catch (ApiException e) {
throw KubernetesException.fromApiException(e);
}
@@ -390,8 +380,7 @@ public V1PodList listSingleJobPods(V1Job job) {
public V1PodList listAllJobPods() {
String labelSelector = "job-name";
try {
- return this.coreApi.listNamespacedPod(
- namespace, null, null, null, null, labelSelector, null, null, null, null, null);
+ return this.coreApi.listNamespacedPod(namespace).labelSelector(labelSelector).execute();
} catch (ApiException e) {
throw KubernetesException.fromApiException(e);
}
@@ -399,8 +388,7 @@ public V1PodList listAllJobPods() {
public String readPodLog(String podName) {
try {
- return this.coreApi.readNamespacedPodLog(
- podName, namespace, null, null, null, null, null, null, null, null, null);
+ return this.coreApi.readNamespacedPodLog(podName, namespace).execute();
} catch (ApiException e) {
logger.info("Getting logs for pod " + podName + " failed.", e);
}
@@ -409,8 +397,7 @@ public String readPodLog(String podName) {
public void labelJobAsCancelled(String taskId) {
try {
- this.patchBatchApi.patchNamespacedJob(
- taskId, namespace, CANCEL_PATCH, null, null, null, null);
+ this.patchBatchApi.patchNamespacedJob(taskId, namespace, CANCEL_PATCH).execute();
} catch (ApiException e) {
throw KubernetesException.fromApiException(e);
}
@@ -423,8 +410,9 @@ public void labelJobAsCancelled(String taskId) {
*/
public void cancelJobBeforeStart(String taskId) {
try {
- this.patchBatchApi.patchNamespacedJob(
- taskId, namespace, CANCEL_BEFORE_START_PATCH, null, null, null, null);
+ this.patchBatchApi
+ .patchNamespacedJob(taskId, namespace, CANCEL_BEFORE_START_PATCH)
+ .execute();
} catch (ApiException e) {
throw KubernetesException.fromApiException(e);
}
@@ -432,8 +420,7 @@ public void cancelJobBeforeStart(String taskId) {
public void labelPodAsCancelled(String podName) {
try {
- this.patchCoreApi.patchNamespacedPod(
- podName, namespace, CANCEL_PATCH, null, null, null, null);
+ this.patchCoreApi.patchNamespacedPod(podName, namespace, CANCEL_PATCH).execute();
} catch (ApiException e) {
throw KubernetesException.fromApiException(e);
}
diff --git a/src/main/java/uk/ac/ebi/tsc/tesk/tes/service/ServiceInfoService.java b/src/main/java/uk/ac/ebi/tsc/tesk/tes/service/ServiceInfoService.java
index 9baa2146..5a11b1df 100644
--- a/src/main/java/uk/ac/ebi/tsc/tesk/tes/service/ServiceInfoService.java
+++ b/src/main/java/uk/ac/ebi/tsc/tesk/tes/service/ServiceInfoService.java
@@ -11,6 +11,7 @@
import org.springframework.core.io.Resource;
import org.springframework.scheduling.annotation.Scheduled;
import org.springframework.stereotype.Service;
+import org.yaml.snakeyaml.LoaderOptions;
import org.yaml.snakeyaml.TypeDescription;
import org.yaml.snakeyaml.Yaml;
import org.yaml.snakeyaml.constructor.Construct;
@@ -74,6 +75,8 @@ public TesServiceInfo getServiceInfo() {
// https://bitbucket.org/asomov/snakeyaml/wiki/Howto#markdown-header-how-to-parse-jodatime
class OffsetDateTimePropertyConstructor extends Constructor {
public OffsetDateTimePropertyConstructor() {
+ // SnakeYAML 2 has no constructor without options; these are its defaults
+ super(new LoaderOptions());
yamlClassConstructors.put(NodeId.scalar, new TimeStampConstruct());
// keys of the service info schema that aren't Java property names (TES 1.1)
TypeDescription serviceInfo = new TypeDescription(TesServiceInfo.class);
diff --git a/src/test/java/uk/ac/ebi/tsc/tesk/k8s/config/KubernetesClientConfigTest.java b/src/test/java/uk/ac/ebi/tsc/tesk/k8s/config/KubernetesClientConfigTest.java
new file mode 100644
index 00000000..bfc0f224
--- /dev/null
+++ b/src/test/java/uk/ac/ebi/tsc/tesk/k8s/config/KubernetesClientConfigTest.java
@@ -0,0 +1,67 @@
+package uk.ac.ebi.tsc.tesk.k8s.config;
+
+import static com.github.tomakehurst.wiremock.client.WireMock.containing;
+import static com.github.tomakehurst.wiremock.client.WireMock.equalTo;
+import static com.github.tomakehurst.wiremock.client.WireMock.okJson;
+import static com.github.tomakehurst.wiremock.client.WireMock.patch;
+import static com.github.tomakehurst.wiremock.client.WireMock.patchRequestedFor;
+import static com.github.tomakehurst.wiremock.client.WireMock.urlPathEqualTo;
+import static com.github.tomakehurst.wiremock.core.WireMockConfiguration.wireMockConfig;
+
+import com.github.tomakehurst.wiremock.junit.WireMockRule;
+import io.kubernetes.client.custom.V1Patch;
+import io.kubernetes.client.openapi.ApiClient;
+import io.kubernetes.client.openapi.ApiException;
+import io.kubernetes.client.openapi.apis.BatchV1Api;
+import io.kubernetes.client.openapi.apis.CoreV1Api;
+import io.kubernetes.client.util.ClientBuilder;
+import io.kubernetes.client.util.credentials.AccessTokenAuthentication;
+import org.junit.Rule;
+import org.junit.Test;
+
+/** The clients that merge-patch jobs and pods (labelling and cancelling tasks) */
+public class KubernetesClientConfigTest {
+
+ @Rule public WireMockRule kubernetes = new WireMockRule(wireMockConfig().dynamicPort());
+
+ private final KubernetesClientConfig config = new KubernetesClientConfig();
+
+ /** A client with a bearer token, as from a kubeconfig file */
+ private ApiClient client() {
+ return new ClientBuilder()
+ .setBasePath(kubernetes.baseUrl())
+ .setAuthentication(new AccessTokenAuthentication("token-123"))
+ .build();
+ }
+
+ /**
+ * Jobs are patched with a JSON merge patch, with the credentials of the client, including the
+ * "Bearer" prefix of its token
+ */
+ @Test
+ public void patchBatchApi() throws ApiException {
+ kubernetes.givenThat(
+ patch(urlPathEqualTo("/apis/batch/v1/namespaces/default/jobs/task-123"))
+ .willReturn(okJson("{}")));
+ BatchV1Api api = config.patchBatchApi(config.patchApiClient(client()));
+ api.patchNamespacedJob("task-123", "default", new V1Patch("{}")).execute();
+ kubernetes.verify(
+ patchRequestedFor(urlPathEqualTo("/apis/batch/v1/namespaces/default/jobs/task-123"))
+ .withHeader("Content-Type", containing("application/merge-patch+json"))
+ .withHeader("Authorization", equalTo("Bearer token-123")));
+ }
+
+ /** Pods are patched in the same way */
+ @Test
+ public void patchCoreApi() throws ApiException {
+ kubernetes.givenThat(
+ patch(urlPathEqualTo("/api/v1/namespaces/default/pods/pod-123"))
+ .willReturn(okJson("{}")));
+ CoreV1Api api = config.patchCoreApi(config.patchApiClient(client()));
+ api.patchNamespacedPod("pod-123", "default", new V1Patch("{}")).execute();
+ kubernetes.verify(
+ patchRequestedFor(urlPathEqualTo("/api/v1/namespaces/default/pods/pod-123"))
+ .withHeader("Content-Type", containing("application/merge-patch+json"))
+ .withHeader("Authorization", equalTo("Bearer token-123")));
+ }
+}
diff --git a/src/test/java/uk/ac/ebi/tsc/tesk/k8s/config/KubernetesJsonTest.java b/src/test/java/uk/ac/ebi/tsc/tesk/k8s/config/KubernetesJsonTest.java
new file mode 100644
index 00000000..728fbe5e
--- /dev/null
+++ b/src/test/java/uk/ac/ebi/tsc/tesk/k8s/config/KubernetesJsonTest.java
@@ -0,0 +1,120 @@
+package uk.ac.ebi.tsc.tesk.k8s.config;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import com.google.gson.Gson;
+import io.kubernetes.client.custom.Quantity;
+import io.kubernetes.client.openapi.JSON;
+import io.kubernetes.client.openapi.models.V1Container;
+import io.kubernetes.client.openapi.models.V1EmptyDirVolumeSource;
+import io.kubernetes.client.openapi.models.V1Job;
+import io.kubernetes.client.openapi.models.V1PodSpec;
+import io.kubernetes.client.openapi.models.V1ResourceRequirements;
+import io.kubernetes.client.openapi.models.V1Secret;
+import io.kubernetes.client.openapi.models.V1Volume;
+import java.nio.charset.StandardCharsets;
+import java.time.OffsetDateTime;
+import java.time.ZoneOffset;
+import org.junit.Test;
+
+/** Reading the Kubernetes objects of clusters newer than the client */
+public class KubernetesJsonTest {
+
+ /**
+ * A job with fields that the client doesn't know (as Kubernetes adds them in new versions, e.g.
+ * to the status) is read, and the fields it knows are kept
+ */
+ @Test
+ public void readsUnknownFields() {
+ V1Job job =
+ KubernetesJson.GSON.fromJson(
+ "{\"metadata\":{\"name\":\"task-123\",\"newField\":1},"
+ + "\"spec\":{\"template\":{},\"newSpecField\":\"x\"},"
+ + "\"status\":{\"succeeded\":1,\"newStatusField\":{\"a\":[]}}}",
+ V1Job.class);
+ assertThat(job.getMetadata().getName()).isEqualTo("task-123");
+ assertThat(job.getStatus().getSucceeded()).isEqualTo(1);
+ }
+
+ /** Times are read and written as Kubernetes writes them (RFC 3339, UTC) */
+ @Test
+ public void times() {
+ V1Job job =
+ KubernetesJson.GSON.fromJson(
+ "{\"metadata\":{\"creationTimestamp\":\"2018-05-31T09:21:57Z\"}}",
+ V1Job.class);
+ OffsetDateTime created = job.getMetadata().getCreationTimestamp();
+ assertThat(created).isEqualTo(OffsetDateTime.of(2018, 5, 31, 9, 21, 57, 0, ZoneOffset.UTC));
+ assertThat(KubernetesJson.GSON.toJson(job))
+ .contains("\"creationTimestamp\":\"2018-05-31T09:21:57.000000Z\"");
+ }
+
+ /**
+ * The configuration of the Kubernetes client lets it deserialize responses with the same Gson
+ * (it is global)
+ */
+ @Test
+ public void installed() {
+ Gson before = JSON.getGson();
+ try {
+ JSON.setGson(new Gson());
+ new KubernetesClientConfig();
+ assertThat(JSON.getGson()).isSameAs(KubernetesJson.GSON);
+ } finally {
+ JSON.setGson(before);
+ }
+ }
+
+ /** Required fields may be missing (e.g. in test fixtures) */
+ @Test
+ public void missingRequiredFields() {
+ V1Job job = KubernetesJson.GSON.fromJson("{\"spec\":{\"parallelism\":1}}", V1Job.class);
+ assertThat(job.getSpec().getParallelism()).isEqualTo(1);
+ }
+
+ /**
+ * Without empty collections: empty lists and maps are left out, objects without fields (e.g. an
+ * emptyDir volume) and non-empty collections are kept
+ */
+ @Test
+ public void withoutEmptyCollections() {
+ V1PodSpec spec =
+ new V1PodSpec()
+ .addContainersItem(
+ new V1Container()
+ .name("c")
+ .resources(
+ new V1ResourceRequirements()
+ .putRequestsItem(
+ "memory", new Quantity("1Gi"))))
+ .addVolumesItem(
+ new V1Volume().name("v").emptyDir(new V1EmptyDirVolumeSource()));
+ assertThat(KubernetesJson.GSON.toJson(spec)).contains("\"limits\":{}");
+ assertThat(KubernetesJson.GSON_WITHOUT_EMPTY_COLLECTIONS.toJson(spec))
+ .isEqualTo(
+ "{\"containers\":[{\"name\":\"c\",\"resources\":{\"requests\":"
+ + "{\"memory\":\"1Gi\"}}}],"
+ + "\"volumes\":[{\"emptyDir\":{},\"name\":\"v\"}]}");
+ }
+
+ /** Without empty collections, reading is as with {@link KubernetesJson#GSON} */
+ @Test
+ public void withoutEmptyCollectionsReads() {
+ V1PodSpec spec =
+ KubernetesJson.GSON_WITHOUT_EMPTY_COLLECTIONS.fromJson(
+ "{\"containers\":[{\"name\":\"c\",\"args\":[]}],\"unknown\":1}",
+ V1PodSpec.class);
+ assertThat(spec.getContainers()).hasSize(1);
+ assertThat(spec.getContainers().get(0).getName()).isEqualTo("c");
+ assertThat(spec.getContainers().get(0).getArgs()).isEmpty();
+ }
+
+ /** Binary data of Secrets and ConfigMaps (Base64), both ways */
+ @Test
+ public void binaryData() {
+ V1Secret secret =
+ KubernetesJson.GSON.fromJson("{\"data\":{\"key\":\"aGVsbG8=\"}}", V1Secret.class);
+ assertThat(secret.getData().get("key")).isEqualTo("hello".getBytes(StandardCharsets.UTF_8));
+ assertThat(KubernetesJson.GSON.toJson(secret)).contains("\"key\":\"aGVsbG8=\"");
+ }
+}
diff --git a/src/test/java/uk/ac/ebi/tsc/tesk/k8s/convert/IgnoredBackendParametersTest.java b/src/test/java/uk/ac/ebi/tsc/tesk/k8s/convert/IgnoredBackendParametersTest.java
index 3a283f37..fd7e0a46 100644
--- a/src/test/java/uk/ac/ebi/tsc/tesk/k8s/convert/IgnoredBackendParametersTest.java
+++ b/src/test/java/uk/ac/ebi/tsc/tesk/k8s/convert/IgnoredBackendParametersTest.java
@@ -91,7 +91,7 @@ void noKeysNoAnnotation() {
IgnoredBackendParameters.annotate(metadata, Collections.emptyList());
- assertThat(metadata.getAnnotations()).isNull();
+ assertThat(metadata.getAnnotations()).isNullOrEmpty();
assertThat(IgnoredBackendParameters.systemLog(metadata)).isEmpty();
}
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 dbbf494f..bf0e574e 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
@@ -14,6 +14,7 @@
import io.kubernetes.client.openapi.models.V1PodCondition;
import io.kubernetes.client.openapi.models.V1PodSpec;
import io.kubernetes.client.openapi.models.V1PodStatus;
+import java.time.OffsetDateTime;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.EnumSet;
@@ -21,7 +22,6 @@
import java.util.List;
import java.util.Set;
import java.util.stream.Stream;
-import org.joda.time.DateTime;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
@@ -597,12 +597,12 @@ void podPastItsDeletionTimeHasEnded() {
Task stuck = task(true, JobState.FAILED, JobState.FAILED, JobState.ABSENT, JobState.ABSENT);
V1Pod pod = pod(TASK + "-ex-00-pod-2", new V1PodStatus().phase("Running"));
int slack = TaskStateResolver.DELETION_SLACK_SECONDS;
- pod.getMetadata().deletionTimestamp(DateTime.now().minusSeconds(slack + 10));
+ pod.getMetadata().deletionTimestamp(OffsetDateTime.now().minusSeconds(slack + 10));
stuck.getExecutors().get(0).addPod(pod);
assertThat(TaskStateResolver.resolve(stuck, () -> 1)).isEqualTo(TesState.CANCELED);
- pod.getMetadata().deletionTimestamp(DateTime.now().minusSeconds(slack - 10));
+ pod.getMetadata().deletionTimestamp(OffsetDateTime.now().minusSeconds(slack - 10));
assertThat(TaskStateResolver.resolve(stuck, () -> 1)).isEqualTo(TesState.CANCELING);
- pod.getMetadata().deletionTimestamp(DateTime.now().plusMinutes(1));
+ pod.getMetadata().deletionTimestamp(OffsetDateTime.now().plusMinutes(1));
assertThat(TaskStateResolver.resolve(stuck, () -> 1)).isEqualTo(TesState.CANCELING);
}
diff --git a/src/test/java/uk/ac/ebi/tsc/tesk/k8s/convert/TesKubernetesConverterMinimalTest.java b/src/test/java/uk/ac/ebi/tsc/tesk/k8s/convert/TesKubernetesConverterMinimalTest.java
index 27044ba7..618e03bd 100644
--- a/src/test/java/uk/ac/ebi/tsc/tesk/k8s/convert/TesKubernetesConverterMinimalTest.java
+++ b/src/test/java/uk/ac/ebi/tsc/tesk/k8s/convert/TesKubernetesConverterMinimalTest.java
@@ -56,6 +56,7 @@
import uk.ac.ebi.tsc.tesk.k8s.constant.K8sConstants;
import uk.ac.ebi.tsc.tesk.k8s.convert.data.TaskBuilder;
import uk.ac.ebi.tsc.tesk.tes.model.TesExecutor;
+import uk.ac.ebi.tsc.tesk.tes.model.TesResources;
import uk.ac.ebi.tsc.tesk.tes.model.TesState;
import uk.ac.ebi.tsc.tesk.tes.model.TesTask;
import uk.ac.ebi.tsc.tesk.trs.TrsToolClient;
@@ -244,6 +245,45 @@ public void fromTesTaskToK8sConfigMap() throws IOException {
JSONCompareMode.NON_EXTENSIBLE);
}
+ /**
+ * An executor with memory but no CPU gets no limits in the taskmaster's input: not even empty
+ * ones, as the taskmaster sets the limits to the requests only if there are none (a pod without
+ * limits could use all memory of its node)
+ */
+ @Test
+ public void ramOnlyExecutorHasNoLimits() throws IOException {
+ given(this.jobNameGenerator.getTaskMasterName()).willReturn("task-98605447");
+ TesTask task =
+ new TesTask()
+ .addExecutorsItem(new TesExecutor().image("ubuntu").addCommandItem("true"))
+ .resources(new TesResources().ramGb(2.0));
+ User user = User.builder("test-user-id").build();
+ V1Job taskmaster = this.converter.fromTesTaskToK8sJob(task, user);
+ V1ConfigMap configMap = this.converter.fromTesTaskToK8sConfigMap(task, user, taskmaster);
+ try (InputStreamReader reader =
+ new InputStreamReader(
+ new GZIPInputStream(
+ new ByteArrayInputStream(
+ configMap.getBinaryData().get("JSON_INPUT.gz"))),
+ StandardCharsets.UTF_8)) {
+ JsonObject resources =
+ this.gson
+ .fromJson(reader, JsonObject.class)
+ .getAsJsonArray("executors")
+ .get(0)
+ .getAsJsonObject()
+ .getAsJsonObject("spec")
+ .getAsJsonObject("template")
+ .getAsJsonObject("spec")
+ .getAsJsonArray("containers")
+ .get(0)
+ .getAsJsonObject()
+ .getAsJsonObject("resources");
+ assertThat(resources.has("limits")).isFalse();
+ assertThat(resources.getAsJsonObject("requests").has("memory")).isTrue();
+ }
+ }
+
@Test
public void privateExecutorImageUsesOwnedSecretWithoutExposingCredentials() throws IOException {
given(this.jobNameGenerator.getTaskMasterName()).willReturn("task-98605447");
@@ -557,6 +597,15 @@ public void fromK8sToTask_queued_new_job() throws IOException {
assertThat(outputTask, CoreMatchers.is(expectedTask));
}
+ /** Without a creation time of the taskmaster's job (Kubernetes sets one), the task has none */
+ @Test
+ public void fromK8sToTask_noCreationTime() throws IOException {
+ TaskBuilder taskBuilder = this.prepareBaseTaskBuider(false, false);
+ taskBuilder.getTask().getTaskmaster().getJob().getMetadata().setCreationTimestamp(null);
+ TesTask outputTask = this.converter.fromK8sJobsToTesTaskBasic(taskBuilder.getTask(), false);
+ org.assertj.core.api.Assertions.assertThat(outputTask.getCreationTime()).isNull();
+ }
+
/** A taskmaster's job with failed pods counted, none active and no condition: an error */
@Test
public void fromK8sToTask_system_error_failed_pods() throws IOException {
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 faa36c9c..ce6a4319 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
@@ -120,8 +120,11 @@ public void setUpKubernetes() throws ApiException, IOException {
configMapKey, "fsGroup: 1000\nrunAsUser: 1001\nnonRoot: true\nrunAsGroup: 1002");
V1ConfigMap testConfigMap = new V1ConfigMap();
testConfigMap.setData(testConfigMapData);
- when(coreApi.readNamespacedConfigMap(configMapName, configMapNamespace, null, false, false))
- .thenReturn(testConfigMap);
+ CoreV1Api.APIreadNamespacedConfigMapRequest readConfigMap =
+ mock(CoreV1Api.APIreadNamespacedConfigMapRequest.class);
+ when(coreApi.readNamespacedConfigMap(configMapName, configMapNamespace))
+ .thenReturn(readConfigMap);
+ when(readConfigMap.execute()).thenReturn(testConfigMap);
KubernetesObjectsSupplier objectsSupplier =
new KubernetesObjectsSupplier(
gson, jobNameGenerator, taskmasterEnvProperties, "test-namespace", coreApi);
diff --git a/src/test/java/uk/ac/ebi/tsc/tesk/k8s/service/KubernetesClientWrapperHttpTest.java b/src/test/java/uk/ac/ebi/tsc/tesk/k8s/service/KubernetesClientWrapperHttpTest.java
new file mode 100644
index 00000000..dd0e2db4
--- /dev/null
+++ b/src/test/java/uk/ac/ebi/tsc/tesk/k8s/service/KubernetesClientWrapperHttpTest.java
@@ -0,0 +1,186 @@
+package uk.ac.ebi.tsc.tesk.k8s.service;
+
+import static com.github.tomakehurst.wiremock.client.WireMock.*;
+import static com.github.tomakehurst.wiremock.core.WireMockConfiguration.wireMockConfig;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
+import com.github.tomakehurst.wiremock.junit.WireMockRule;
+import io.kubernetes.client.openapi.ApiClient;
+import io.kubernetes.client.openapi.apis.BatchV1Api;
+import io.kubernetes.client.openapi.apis.CoreV1Api;
+import io.kubernetes.client.openapi.models.V1ConfigMap;
+import io.kubernetes.client.openapi.models.V1ObjectMeta;
+import io.kubernetes.client.util.ClientBuilder;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import uk.ac.ebi.tsc.tesk.config.security.User;
+import uk.ac.ebi.tsc.tesk.k8s.config.KubernetesClientConfig;
+import uk.ac.ebi.tsc.tesk.k8s.exception.KubernetesException;
+import uk.ac.ebi.tsc.tesk.tes.exception.TaskNotFoundException;
+
+/**
+ * The requests that the wrapper sends to Kubernetes through a real client: paths, query parameters,
+ * bodies and the handling of errors (the other tests mock the client)
+ */
+public class KubernetesClientWrapperHttpTest {
+
+ private static final String JOBS = "/apis/batch/v1/namespaces/tesk/jobs";
+
+ private static final String PODS = "/api/v1/namespaces/tesk/pods";
+
+ @Rule public WireMockRule kubernetes = new WireMockRule(wireMockConfig().dynamicPort());
+
+ private KubernetesClientWrapper wrapper;
+
+ @Before
+ public void setUp() {
+ KubernetesClientConfig config = new KubernetesClientConfig();
+ ApiClient client = new ClientBuilder().setBasePath(kubernetes.baseUrl()).build();
+ ApiClient patchClient = config.patchApiClient(client);
+ wrapper =
+ new KubernetesClientWrapper(
+ new BatchV1Api(client),
+ config.patchBatchApi(patchClient),
+ new CoreV1Api(client),
+ config.patchCoreApi(patchClient),
+ "tesk");
+ }
+
+ /** A page of taskmasters' jobs: the selector, the page token and the page size */
+ @Test
+ public void listTaskmasterJobs() {
+ kubernetes.givenThat(get(urlPathEqualTo(JOBS)).willReturn(okJson("{\"items\":[]}")));
+ wrapper.listAllTaskmasterJobsForUser("token-1", 10, User.builder("123").build());
+ kubernetes.verify(
+ getRequestedFor(urlPathEqualTo(JOBS))
+ .withQueryParam("labelSelector", equalTo("job-type=taskmaster"))
+ .withQueryParam("continue", equalTo("token-1"))
+ .withQueryParam("limit", equalTo("10")));
+ }
+
+ /** Without a page token or size, the request has neither */
+ @Test
+ public void listExecutorJobs() {
+ kubernetes.givenThat(get(urlPathEqualTo(JOBS)).willReturn(okJson("{\"items\":[]}")));
+ wrapper.listAllTaskExecutorJobs();
+ kubernetes.verify(
+ getRequestedFor(urlPathEqualTo(JOBS))
+ .withQueryParam("labelSelector", equalTo("job-type=executor"))
+ .withQueryParam("continue", absent())
+ .withQueryParam("limit", absent()));
+ }
+
+ /** A ConfigMap is checked in a dry run, and created without one */
+ @Test
+ public void configMaps() {
+ kubernetes.givenThat(
+ post(urlPathEqualTo("/api/v1/namespaces/tesk/configmaps"))
+ .willReturn(okJson("{\"metadata\":{\"name\":\"task-1\"}}")));
+ V1ConfigMap map = new V1ConfigMap().metadata(new V1ObjectMeta().name("task-1"));
+ wrapper.checkConfigMap(map);
+ wrapper.createConfigMap(map);
+ kubernetes.verify(
+ 1,
+ postRequestedFor(urlPathEqualTo("/api/v1/namespaces/tesk/configmaps"))
+ .withQueryParam("dryRun", equalTo("All")));
+ kubernetes.verify(
+ 1,
+ postRequestedFor(urlPathEqualTo("/api/v1/namespaces/tesk/configmaps"))
+ .withQueryParam("dryRun", absent()));
+ }
+
+ /** A job is deleted with its pods, in the background */
+ @Test
+ public void deleteJob() {
+ kubernetes.givenThat(delete(urlPathEqualTo(JOBS + "/task-1")).willReturn(okJson("{}")));
+ wrapper.deleteJob("task-1");
+ kubernetes.verify(
+ deleteRequestedFor(urlPathEqualTo(JOBS + "/task-1"))
+ .withQueryParam("propagationPolicy", equalTo("Background")));
+ }
+
+ /** Errors of Kubernetes become KubernetesExceptions with their status code */
+ @Test
+ public void error() {
+ kubernetes.givenThat(
+ delete(urlPathEqualTo(JOBS + "/task-1"))
+ .willReturn(aResponse().withStatus(403).withBody("{}")));
+ assertThatThrownBy(() -> wrapper.deleteJob("task-1"))
+ .isInstanceOfSatisfying(
+ KubernetesException.class,
+ e -> assertThat(e.getApiException().getCode()).isEqualTo(403));
+ }
+
+ /** A job that doesn't exist (404) is a task that doesn't exist */
+ @Test(expected = TaskNotFoundException.class)
+ public void readMissingJob() {
+ kubernetes.givenThat(
+ get(urlPathEqualTo(JOBS + "/task-1"))
+ .willReturn(aResponse().withStatus(404).withBody("{}")));
+ wrapper.readTaskmasterJob("task-1");
+ }
+
+ /** The minimum memory of the namespace's LimitRanges, in GB */
+ @Test
+ public void minimumRamGb() {
+ kubernetes.givenThat(
+ get(urlPathEqualTo("/api/v1/namespaces/tesk/limitranges"))
+ .willReturn(
+ okJson(
+ "{\"items\":[{\"spec\":{\"limits\":[{\"type\":\"Container\","
+ + "\"min\":{\"memory\":\"2Gi\"}}]}}]}")));
+ assertThat(wrapper.minimumRamGb()).isEqualTo(2.0);
+ kubernetes.verify(
+ getRequestedFor(urlPathEqualTo("/api/v1/namespaces/tesk/limitranges"))
+ .withQueryParam("labelSelector", absent())
+ .withQueryParam("continue", absent())
+ .withQueryParam("limit", absent()));
+ }
+
+ /** A pod's log as Kubernetes sends it (plain text), or null if Kubernetes refuses it */
+ @Test
+ public void readPodLog() {
+ kubernetes.givenThat(
+ get(urlPathEqualTo(PODS + "/pod-1/log"))
+ .willReturn(ok("hello\n").withHeader("Content-Type", "text/plain")));
+ kubernetes.givenThat(
+ get(urlPathEqualTo(PODS + "/pod-2/log"))
+ .willReturn(aResponse().withStatus(404).withBody("{}")));
+ assertThat(wrapper.readPodLog("pod-1")).isEqualTo("hello\n");
+ assertThat(wrapper.readPodLog("pod-2")).isNull();
+ }
+
+ /** Jobs and pods are labelled as cancelled with merge patches */
+ @Test
+ public void labelAsCancelled() {
+ kubernetes.givenThat(patch(urlPathEqualTo(JOBS + "/task-1")).willReturn(okJson("{}")));
+ kubernetes.givenThat(patch(urlPathEqualTo(PODS + "/pod-1")).willReturn(okJson("{}")));
+ wrapper.labelJobAsCancelled("task-1");
+ wrapper.labelPodAsCancelled("pod-1");
+ String body = "{\"metadata\":{\"labels\":{\"task-status\":\"Cancelled\"}}}";
+ kubernetes.verify(
+ patchRequestedFor(urlPathEqualTo(JOBS + "/task-1"))
+ .withHeader("Content-Type", containing("application/merge-patch+json"))
+ .withRequestBody(equalToJson(body)));
+ kubernetes.verify(
+ patchRequestedFor(urlPathEqualTo(PODS + "/pod-1"))
+ .withHeader("Content-Type", containing("application/merge-patch+json"))
+ .withRequestBody(equalToJson(body)));
+ }
+
+ /** A job whose taskmaster doesn't run yet is labelled and ended in one patch */
+ @Test
+ public void cancelJobBeforeStart() {
+ kubernetes.givenThat(patch(urlPathEqualTo(JOBS + "/task-1")).willReturn(okJson("{}")));
+ wrapper.cancelJobBeforeStart("task-1");
+ kubernetes.verify(
+ patchRequestedFor(urlPathEqualTo(JOBS + "/task-1"))
+ .withRequestBody(
+ equalToJson(
+ "{\"metadata\":{\"labels\":{\"task-status\":\"Cancelled\"}},"
+ + "\"spec\":{\"parallelism\":0,"
+ + "\"activeDeadlineSeconds\":1}}")));
+ }
+}
diff --git a/src/test/java/uk/ac/ebi/tsc/tesk/k8s/service/KubernetesClientWrapperTest.java b/src/test/java/uk/ac/ebi/tsc/tesk/k8s/service/KubernetesClientWrapperTest.java
index e4d019d3..53ecf738 100644
--- a/src/test/java/uk/ac/ebi/tsc/tesk/k8s/service/KubernetesClientWrapperTest.java
+++ b/src/test/java/uk/ac/ebi/tsc/tesk/k8s/service/KubernetesClientWrapperTest.java
@@ -8,6 +8,7 @@
import static org.junit.Assert.fail;
import static org.mockito.ArgumentMatchers.*;
import static org.mockito.BDDMockito.given;
+import static org.mockito.Mockito.mock;
import static uk.ac.ebi.tsc.tesk.k8s.constant.Constants.ANN_USERID_KEY;
import static uk.ac.ebi.tsc.tesk.k8s.constant.Constants.LABEL_GROUPNAME_KEY;
import static uk.ac.ebi.tsc.tesk.k8s.constant.Constants.LABEL_LEGACY_USERID_KEY;
@@ -16,12 +17,13 @@
import io.kubernetes.client.openapi.apis.BatchV1Api;
import io.kubernetes.client.openapi.apis.CoreV1Api;
import io.kubernetes.client.openapi.models.*;
+import java.time.OffsetDateTime;
import java.util.Arrays;
import java.util.Optional;
-import org.joda.time.DateTime;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
+import org.mockito.Answers;
import org.mockito.ArgumentCaptor;
import org.mockito.Mockito;
import org.springframework.beans.factory.annotation.Value;
@@ -49,12 +51,50 @@ public class KubernetesClientWrapperTest {
private KubernetesClientWrapper wrapper;
+ // The requests of the Kubernetes client: their setters return the request itself, only
+ // execute() needs to be stubbed
+ private BatchV1Api.APIcreateNamespacedJobRequest createJob;
+
+ private BatchV1Api.APIdeleteNamespacedJobRequest deleteJob;
+
+ private BatchV1Api.APIreadNamespacedJobRequest readJob;
+
+ private BatchV1Api.APIlistNamespacedJobRequest listJobs;
+
+ private BatchV1Api.APIpatchNamespacedJobRequest patchJob;
+
+ private CoreV1Api.APIcreateNamespacedConfigMapRequest createConfigMap;
+
+ private CoreV1Api.APIcreateNamespacedSecretRequest createSecret;
+
+ private CoreV1Api.APIlistNamespacedPodRequest listPods;
+
@Before
public void setUp() {
+ createJob = request(BatchV1Api.APIcreateNamespacedJobRequest.class);
+ given(batchApi.createNamespacedJob(any(), any())).willReturn(createJob);
+ deleteJob = request(BatchV1Api.APIdeleteNamespacedJobRequest.class);
+ given(batchApi.deleteNamespacedJob(any(), any())).willReturn(deleteJob);
+ readJob = request(BatchV1Api.APIreadNamespacedJobRequest.class);
+ given(batchApi.readNamespacedJob(any(), any())).willReturn(readJob);
+ listJobs = request(BatchV1Api.APIlistNamespacedJobRequest.class);
+ given(batchApi.listNamespacedJob(any())).willReturn(listJobs);
+ patchJob = request(BatchV1Api.APIpatchNamespacedJobRequest.class);
+ given(batchApi.patchNamespacedJob(any(), any(), any())).willReturn(patchJob);
+ createConfigMap = request(CoreV1Api.APIcreateNamespacedConfigMapRequest.class);
+ given(coreApi.createNamespacedConfigMap(any(), any())).willReturn(createConfigMap);
+ createSecret = request(CoreV1Api.APIcreateNamespacedSecretRequest.class);
+ given(coreApi.createNamespacedSecret(any(), any())).willReturn(createSecret);
+ listPods = request(CoreV1Api.APIlistNamespacedPodRequest.class);
+ given(coreApi.listNamespacedPod(any())).willReturn(listPods);
this.wrapper = new KubernetesClientWrapper(batchApi, batchApi, coreApi, coreApi, namespace);
}
+ private static T request(Class type) {
+ return mock(type, Answers.RETURNS_SELF);
+ }
+
/**
* A job cancelled before its pod runs is labelled as cancelled and gets parallelism 0 and an
* active deadline of 1 second, in one patch, so that Kubernetes removes its pods and fails it
@@ -66,14 +106,8 @@ public void cancelJobBeforeStart() throws ApiException {
org.mockito.ArgumentCaptor.forClass(io.kubernetes.client.custom.V1Patch.class);
wrapper.cancelJobBeforeStart("task-123");
Mockito.verify(batchApi)
- .patchNamespacedJob(
- eq("task-123"),
- eq("test-namespace"),
- patch.capture(),
- isNull(),
- isNull(),
- isNull(),
- isNull());
+ .patchNamespacedJob(eq("task-123"), eq("test-namespace"), patch.capture());
+ Mockito.verify(patchJob).execute();
assertEquals(
"{\"metadata\":{\"labels\":{\"task-status\":\"Cancelled\"}},"
+ "\"spec\":{\"parallelism\":0,\"activeDeadlineSeconds\":1}}",
@@ -82,8 +116,7 @@ public void cancelJobBeforeStart() throws ApiException {
@Test(expected = KubernetesException.class)
public void cancelJobBeforeStart_refused() throws ApiException {
- given(batchApi.patchNamespacedJob(any(), any(), any(), any(), any(), any(), any()))
- .willThrow(new ApiException(403, "forbidden"));
+ given(patchJob.execute()).willThrow(new ApiException(403, "forbidden"));
wrapper.cancelJobBeforeStart("task-123");
}
@@ -92,15 +125,14 @@ public void cancelJobBeforeStart_refused() throws ApiException {
public void checkConfigMap() throws ApiException {
V1ConfigMap map = new V1ConfigMap().metadata(new V1ObjectMeta().name("123"));
wrapper.checkConfigMap(map);
- Mockito.verify(coreApi)
- .createNamespacedConfigMap(
- eq("test-namespace"), same(map), isNull(), eq("All"), isNull());
+ Mockito.verify(coreApi).createNamespacedConfigMap(eq("test-namespace"), same(map));
+ Mockito.verify(createConfigMap).dryRun("All");
+ Mockito.verify(createConfigMap).execute();
}
@Test(expected = KubernetesException.class)
public void checkConfigMap_refused() throws ApiException {
- given(coreApi.createNamespacedConfigMap(any(), any(), any(), any(), any()))
- .willThrow(new ApiException(422, "too large"));
+ given(createConfigMap.execute()).willThrow(new ApiException(422, "too large"));
wrapper.checkConfigMap(new V1ConfigMap());
}
@@ -110,7 +142,7 @@ public void checkConfigMap_refused() throws ApiException {
*/
@Test
public void checkConfigMap_webhookWithoutDryRun() throws ApiException {
- given(coreApi.createNamespacedConfigMap(any(), any(), any(), any(), any()))
+ given(createConfigMap.execute())
.willThrow(
new ApiException(
"",
@@ -125,15 +157,14 @@ public void checkConfigMap_webhookWithoutDryRun() throws ApiException {
/** A 400 of the dry run without a Status to tell the cause isn't skipped */
@Test(expected = KubernetesException.class)
public void checkConfigMap_badRequestWithoutBody() throws ApiException {
- given(coreApi.createNamespacedConfigMap(any(), any(), any(), any(), any()))
- .willThrow(new ApiException("", 400, null, null));
+ given(createConfigMap.execute()).willThrow(new ApiException("", 400, null, null));
wrapper.checkConfigMap(new V1ConfigMap());
}
/** Other 400s of the dry run aren't skipped */
@Test(expected = KubernetesException.class)
public void checkConfigMap_otherBadRequest() throws ApiException {
- given(coreApi.createNamespacedConfigMap(any(), any(), any(), any(), any()))
+ given(createConfigMap.execute())
.willThrow(new ApiException("", 400, null, "{\"message\":\"invalid\"}"));
wrapper.checkConfigMap(new V1ConfigMap());
}
@@ -142,14 +173,14 @@ public void checkConfigMap_otherBadRequest() throws ApiException {
public void checkSecret() throws ApiException {
V1Secret secret = new V1Secret().metadata(new V1ObjectMeta().name("task-123-ex-00"));
wrapper.checkSecret(secret);
- Mockito.verify(coreApi)
- .createNamespacedSecret(
- eq("test-namespace"), same(secret), isNull(), eq("All"), isNull());
+ Mockito.verify(coreApi).createNamespacedSecret(eq("test-namespace"), same(secret));
+ Mockito.verify(createSecret).dryRun("All");
+ Mockito.verify(createSecret).execute();
}
@Test
public void checkSecret_webhookWithoutDryRun() throws ApiException {
- given(coreApi.createNamespacedSecret(any(), any(), any(), any(), any()))
+ given(createSecret.execute())
.willThrow(
new ApiException(
"",
@@ -161,8 +192,7 @@ public void checkSecret_webhookWithoutDryRun() throws ApiException {
@Test
public void checkSecret_refused() throws ApiException {
- given(coreApi.createNamespacedSecret(any(), any(), any(), any(), any()))
- .willThrow(new ApiException(403, "private-token"));
+ given(createSecret.execute()).willThrow(new ApiException(403, "private-token"));
try {
wrapper.checkSecret(new V1Secret());
fail("Expected KubernetesException");
@@ -175,15 +205,15 @@ public void checkSecret_refused() throws ApiException {
@Test
public void createSecret() throws ApiException {
V1Secret secret = new V1Secret().metadata(new V1ObjectMeta().name("task-123-ex-00"));
- given(coreApi.createNamespacedSecret("test-namespace", secret, null, null, null))
- .willReturn(secret);
+ given(createSecret.execute()).willReturn(secret);
assertThat(wrapper.createSecret(secret), is(secret));
+ Mockito.verify(coreApi).createNamespacedSecret("test-namespace", secret);
+ Mockito.verify(createSecret, Mockito.never()).dryRun(any());
}
@Test
public void createSecret_refusedDoesNotExposeCredentials() throws ApiException {
- given(coreApi.createNamespacedSecret(any(), any(), any(), any(), any()))
- .willThrow(new ApiException(422, "private-token"));
+ given(createSecret.execute()).willThrow(new ApiException(422, "private-token"));
try {
wrapper.createSecret(new V1Secret());
fail("Expected KubernetesException");
@@ -201,11 +231,11 @@ public void createJob() throws ApiException {
.metadata(
new V1ObjectMeta()
.name("test job")
- .creationTimestamp(new DateTime()));
- given(batchApi.createNamespacedJob("test-namespace", inJob, null, null, null))
- .willReturn(outJob);
+ .creationTimestamp(OffsetDateTime.now()));
+ given(createJob.execute()).willReturn(outJob);
V1Job result = wrapper.createJob(inJob);
assertThat(result, is(outJob));
+ Mockito.verify(batchApi).createNamespacedJob("test-namespace", inJob);
}
@Test
@@ -216,10 +246,10 @@ public void readTaskmasterJob() throws ApiException {
new V1ObjectMeta()
.name("test job")
.putLabelsItem("job-type", "taskmaster"));
- given(batchApi.readNamespacedJob("123", "test-namespace", null, null, null))
- .willReturn(job);
+ given(readJob.execute()).willReturn(job);
V1Job result = wrapper.readTaskmasterJob("123");
assertThat(result, is(job));
+ Mockito.verify(batchApi).readNamespacedJob("123", "test-namespace");
}
@Test(expected = TaskNotFoundException.class)
@@ -230,8 +260,7 @@ public void readTaskmasterJob_wrongType() throws Exception {
new V1ObjectMeta()
.name("test job")
.putLabelsItem("job-type", "executor"));
- given(batchApi.readNamespacedJob("123", "test-namespace", null, null, null))
- .willReturn(job);
+ given(readJob.execute()).willReturn(job);
wrapper.readTaskmasterJob("123");
}
@@ -243,8 +272,7 @@ public void readTaskmasterJob_noType() throws Exception {
new V1ObjectMeta()
.name("test job")
.putLabelsItem("other", "taskmaster"));
- given(batchApi.readNamespacedJob("123", "test-namespace", null, null, null))
- .willReturn(job);
+ given(readJob.execute()).willReturn(job);
wrapper.readTaskmasterJob("123");
}
@@ -252,22 +280,20 @@ public void readTaskmasterJob_noType() throws Exception {
@Test(expected = TaskNotFoundException.class)
public void readTaskmasterJob_noLabels() throws Exception {
V1Job job = new V1Job().metadata(new V1ObjectMeta().name("123"));
- given(batchApi.readNamespacedJob("123", "test-namespace", null, null, null))
- .willReturn(job);
+ given(readJob.execute()).willReturn(job);
wrapper.readTaskmasterJob("123");
}
/** A job without metadata isn't a taskmaster's job either (issue #125) */
@Test(expected = TaskNotFoundException.class)
public void readTaskmasterJob_noMetadata() throws Exception {
- given(batchApi.readNamespacedJob("123", "test-namespace", null, null, null))
- .willReturn(new V1Job());
+ given(readJob.execute()).willReturn(new V1Job());
wrapper.readTaskmasterJob("123");
}
@Test(expected = TaskNotFoundException.class)
public void readTaskmasterJob_noJob() throws Exception {
- given(batchApi.readNamespacedJob("123", "test-namespace", null, null, null))
+ given(readJob.execute())
.willThrow(new ApiException(HttpStatus.NOT_FOUND.value(), "No job"));
wrapper.readTaskmasterJob("123");
}
@@ -280,10 +306,9 @@ public void readTaskmasterJob_beingDeleted() throws Exception {
.metadata(
new V1ObjectMeta()
.name("123")
- .deletionTimestamp(new DateTime())
+ .deletionTimestamp(OffsetDateTime.now())
.putLabelsItem("job-type", "taskmaster"));
- given(batchApi.readNamespacedJob("123", "test-namespace", null, null, null))
- .willReturn(job);
+ given(readJob.execute()).willReturn(job);
wrapper.readTaskmasterJob("123");
}
@@ -296,11 +321,8 @@ public void listAllTaskmasterJobsForUser_skipsJobsBeingDeleted() throws ApiExcep
.metadata(
new V1ObjectMeta()
.name("deleted")
- .deletionTimestamp(new DateTime()));
- given(
- batchApi.listNamespacedJob(
- any(), isNull(), isNull(), isNull(), isNull(), any(), isNull(),
- isNull(), isNull(), isNull(), isNull()))
+ .deletionTimestamp(OffsetDateTime.now()));
+ given(listJobs.execute())
.willReturn(
new V1JobList()
.metadata(new V1ListMeta()._continue("next"))
@@ -315,21 +337,14 @@ public void listAllTaskmasterJobsForUser_skipsJobsBeingDeleted() throws ApiExcep
@Test
public void deleteJob() throws ApiException {
wrapper.deleteJob("123");
- Mockito.verify(batchApi)
- .deleteNamespacedJob(
- eq("123"),
- eq("test-namespace"),
- isNull(),
- isNull(),
- isNull(),
- isNull(),
- eq("Background"),
- isNull());
+ Mockito.verify(batchApi).deleteNamespacedJob("123", "test-namespace");
+ Mockito.verify(deleteJob).propagationPolicy("Background");
+ Mockito.verify(deleteJob).execute();
}
@Test(expected = KubernetesException.class)
public void deleteJob_fails() throws ApiException {
- given(batchApi.deleteNamespacedJob(any(), any(), any(), any(), any(), any(), any(), any()))
+ given(deleteJob.execute())
.willThrow(new ApiException(HttpStatus.FORBIDDEN.value(), "forbidden"));
wrapper.deleteJob("123");
}
@@ -339,25 +354,9 @@ public void listAllTaskmasterJobsForUser_any() throws ApiException {
User user = User.builder("123").build();
ArgumentCaptor argument = ArgumentCaptor.forClass(String.class);
- given(
- batchApi.listNamespacedJob(
- any(), isNull(), isNull(), isNull(), isNull(), any(), isNull(),
- isNull(), isNull(), isNull(), isNull()))
- .willReturn(this.resultList());
+ given(listJobs.execute()).willReturn(this.resultList());
V1JobList result = wrapper.listAllTaskmasterJobsForUser(null, null, user);
- Mockito.verify(batchApi)
- .listNamespacedJob(
- eq("test-namespace"),
- isNull(),
- isNull(),
- isNull(),
- isNull(),
- argument.capture(),
- isNull(),
- isNull(),
- isNull(),
- isNull(),
- isNull());
+ verifyListJobs("test-namespace", argument);
assertThat(argument.getValue(), is("job-type=taskmaster"));
assertThat(result, is(this.resultList()));
}
@@ -367,25 +366,9 @@ public void listAllTaskmasterJobsForUser_admin() throws ApiException {
User user = User.builder("123").teskAdmin(true).build();
ArgumentCaptor argument = ArgumentCaptor.forClass(String.class);
- given(
- batchApi.listNamespacedJob(
- any(), isNull(), isNull(), isNull(), isNull(), any(), isNull(),
- isNull(), isNull(), isNull(), isNull()))
- .willReturn(this.resultList());
+ given(listJobs.execute()).willReturn(this.resultList());
V1JobList result = wrapper.listAllTaskmasterJobsForUser(null, null, user);
- Mockito.verify(batchApi)
- .listNamespacedJob(
- eq(namespace),
- isNull(),
- isNull(),
- isNull(),
- isNull(),
- argument.capture(),
- isNull(),
- isNull(),
- isNull(),
- isNull(),
- isNull());
+ verifyListJobs(namespace, argument);
assertThat(argument.getValue(), is("job-type=taskmaster"));
assertThat(result, is(this.resultList()));
}
@@ -398,25 +381,9 @@ public void listAllTaskmasterJobsForUser_member() throws ApiException {
.teskMemberedGroups(StringUtils.commaDelimitedListToSet("TEST"))
.build();
ArgumentCaptor argument = ArgumentCaptor.forClass(String.class);
- given(
- batchApi.listNamespacedJob(
- any(), isNull(), isNull(), isNull(), isNull(), any(), isNull(),
- isNull(), isNull(), isNull(), isNull()))
- .willReturn(this.resultList());
+ given(listJobs.execute()).willReturn(this.resultList());
V1JobList result = wrapper.listAllTaskmasterJobsForUser(null, null, user);
- Mockito.verify(batchApi)
- .listNamespacedJob(
- eq(namespace),
- isNull(),
- isNull(),
- isNull(),
- isNull(),
- argument.capture(),
- isNull(),
- isNull(),
- isNull(),
- isNull(),
- isNull());
+ verifyListJobs(namespace, argument);
assertThat(
argument.getValue(),
is(
@@ -436,25 +403,9 @@ public void listAllTaskmasterJobsForUser_groupAdmin() throws ApiException {
.teskManagedGroups(StringUtils.commaDelimitedListToSet("TEST,XYZ"))
.build();
ArgumentCaptor argument = ArgumentCaptor.forClass(String.class);
- given(
- batchApi.listNamespacedJob(
- any(), isNull(), isNull(), isNull(), isNull(), any(), isNull(),
- isNull(), isNull(), isNull(), isNull()))
- .willReturn(this.resultList());
+ given(listJobs.execute()).willReturn(this.resultList());
V1JobList result = wrapper.listAllTaskmasterJobsForUser(null, null, user);
- Mockito.verify(batchApi)
- .listNamespacedJob(
- eq(namespace),
- isNull(),
- isNull(),
- isNull(),
- isNull(),
- argument.capture(),
- isNull(),
- isNull(),
- isNull(),
- isNull(),
- isNull());
+ verifyListJobs(namespace, argument);
assertThat(argument.getValue(), is("job-type=taskmaster,creator-group-name in (TEST,XYZ)"));
assertThat(result, is(this.resultList()));
}
@@ -468,25 +419,9 @@ public void listAllTaskmasterJobsForUser_mixed() throws ApiException {
.teskMemberedGroups(StringUtils.commaDelimitedListToSet("XYZ"))
.build();
ArgumentCaptor argument = ArgumentCaptor.forClass(String.class);
- given(
- batchApi.listNamespacedJob(
- any(), isNull(), isNull(), isNull(), isNull(), any(), isNull(),
- isNull(), isNull(), isNull(), isNull()))
- .willReturn(this.resultList());
+ given(listJobs.execute()).willReturn(this.resultList());
V1JobList result = wrapper.listAllTaskmasterJobsForUser(null, null, user);
- Mockito.verify(batchApi)
- .listNamespacedJob(
- eq(namespace),
- isNull(),
- isNull(),
- isNull(),
- isNull(),
- argument.capture(),
- isNull(),
- isNull(),
- isNull(),
- isNull(),
- isNull());
+ verifyListJobs(namespace, argument);
assertThat(argument.getValue(), is("job-type=taskmaster,creator-group-name in (XYZ,TEST)"));
assertThat(result, is(filteredResultList()));
}
@@ -495,34 +430,22 @@ public void listAllTaskmasterJobsForUser_mixed() throws ApiException {
public void listSingleTaskExecutorJobs() throws ApiException {
ArgumentCaptor argument = ArgumentCaptor.forClass(String.class);
wrapper.listSingleTaskExecutorJobs("123");
- Mockito.verify(batchApi)
- .listNamespacedJob(
- eq("test-namespace"),
- isNull(),
- isNull(),
- isNull(),
- isNull(),
- argument.capture(),
- isNull(),
- isNull(),
- isNull(),
- isNull(),
- isNull());
+ verifyListJobs("test-namespace", argument);
assertThat(argument.getValue(), is("taskmaster-name=123"));
}
@Test
public void getSingleTaskOutputFilerJob() throws ApiException {
V1Job job = new V1Job().metadata(new V1ObjectMeta().name("test job"));
- given(batchApi.readNamespacedJob("123-outputs-filer", "test-namespace", null, null, null))
- .willReturn(job);
+ given(readJob.execute()).willReturn(job);
Optional result = wrapper.getSingleTaskOutputFilerJob("123");
assertThat(result, is(Optional.of(job)));
+ Mockito.verify(batchApi).readNamespacedJob("123-outputs-filer", "test-namespace");
}
@Test
public void getSingleTaskOutputFilerJob_noJob() throws ApiException {
- given(batchApi.readNamespacedJob("123-outputs-filer", "test-namespace", null, null, null))
+ given(readJob.execute())
.willThrow(new ApiException(HttpStatus.NOT_FOUND.value(), "No Job"));
Optional result = wrapper.getSingleTaskOutputFilerJob("123");
assertThat(result, is(Optional.empty()));
@@ -532,19 +455,7 @@ public void getSingleTaskOutputFilerJob_noJob() throws ApiException {
public void listAllTaskExecutorJobs() throws ApiException {
ArgumentCaptor argument = ArgumentCaptor.forClass(String.class);
wrapper.listAllTaskExecutorJobs();
- Mockito.verify(batchApi)
- .listNamespacedJob(
- eq("test-namespace"),
- isNull(),
- isNull(),
- isNull(),
- isNull(),
- argument.capture(),
- isNull(),
- isNull(),
- isNull(),
- isNull(),
- isNull());
+ verifyListJobs("test-namespace", argument);
assertThat(argument.getValue(), is("job-type=executor"));
}
@@ -552,19 +463,7 @@ public void listAllTaskExecutorJobs() throws ApiException {
public void listAllFilerJobs() throws ApiException {
ArgumentCaptor argument = ArgumentCaptor.forClass(String.class);
wrapper.listAllFilerJobs();
- Mockito.verify(batchApi)
- .listNamespacedJob(
- eq("test-namespace"),
- isNull(),
- isNull(),
- isNull(),
- isNull(),
- argument.capture(),
- isNull(),
- isNull(),
- isNull(),
- isNull(),
- isNull());
+ verifyListJobs("test-namespace", argument);
assertThat(argument.getValue(), is("!job-type"));
}
@@ -581,38 +480,31 @@ public void listSingleJobPods() throws ApiException {
.putMatchLabelsItem("labelA", "valueB")));
ArgumentCaptor argument = ArgumentCaptor.forClass(String.class);
wrapper.listSingleJobPods(job);
- Mockito.verify(coreApi)
- .listNamespacedPod(
- eq("test-namespace"),
- isNull(),
- isNull(),
- isNull(),
- isNull(),
- argument.capture(),
- isNull(),
- isNull(),
- isNull(),
- isNull(),
- isNull());
+ Mockito.verify(coreApi).listNamespacedPod("test-namespace");
+ Mockito.verify(listPods).labelSelector(argument.capture());
+ Mockito.verify(listPods).execute();
assertThat(argument.getValue(), is("label1=value1,labelA=valueB"));
}
@Test
public void listAllJobPods() throws ApiException {
wrapper.listAllJobPods();
- Mockito.verify(coreApi)
- .listNamespacedPod(
- eq("test-namespace"),
- isNull(),
- isNull(),
- isNull(),
- isNull(),
- eq("job-name"),
- isNull(),
- isNull(),
- isNull(),
- isNull(),
- isNull());
+ Mockito.verify(coreApi).listNamespacedPod("test-namespace");
+ Mockito.verify(listPods).labelSelector("job-name");
+ Mockito.verify(listPods).execute();
+ }
+
+ /**
+ * Verifies that the jobs were listed once, in the namespace, without a page token or limit, and
+ * captures the label selector
+ */
+ private void verifyListJobs(String namespace, ArgumentCaptor labelSelector)
+ throws ApiException {
+ Mockito.verify(batchApi).listNamespacedJob(namespace);
+ Mockito.verify(listJobs)._continue(isNull());
+ Mockito.verify(listJobs).labelSelector(labelSelector.capture());
+ Mockito.verify(listJobs).limit(isNull());
+ Mockito.verify(listJobs).execute();
}
private static V1Job job(String group, String userId) {
@@ -655,11 +547,7 @@ private V1JobList filteredResultList() {
private V1JobList listJobsReturning(V1Job... jobs) throws ApiException {
V1JobList jobList = new V1JobList().items(Arrays.asList(jobs));
- given(
- batchApi.listNamespacedJob(
- any(), isNull(), isNull(), isNull(), isNull(), any(), isNull(),
- isNull(), isNull(), isNull(), isNull()))
- .willReturn(jobList);
+ given(listJobs.execute()).willReturn(jobList);
return jobList;
}
@@ -697,19 +585,8 @@ public void listAllTaskmasterJobsForUser_legacyLabels() throws ApiException {
assertThat(
wrapper.listAllTaskmasterJobsForUser(null, null, legacyUser).getItems(),
is(Arrays.asList(legacyJob("TEST", "a_b"), job("TEST", "a@b"))));
- Mockito.verify(batchApi, Mockito.times(2))
- .listNamespacedJob(
- eq(namespace),
- isNull(),
- isNull(),
- isNull(),
- isNull(),
- argument.capture(),
- isNull(),
- isNull(),
- isNull(),
- isNull(),
- isNull());
+ Mockito.verify(batchApi, Mockito.times(2)).listNamespacedJob(namespace);
+ Mockito.verify(listJobs, Mockito.times(2)).labelSelector(argument.capture());
assertThat(argument.getValue(), is("job-type=taskmaster,creator-group-name in (TEST)"));
}
@@ -761,7 +638,6 @@ public void listAllTaskmasterJobsForUser_malformedPageToken() throws ApiExceptio
private void assertInvalidPageToken(int code) throws ApiException {
this.listJobsFails(
- "secret-token",
code,
"{\"kind\":\"Status\",\"message\":\"continue key is not valid: secret-token\"}");
try {
@@ -771,12 +647,22 @@ private void assertInvalidPageToken(int code) throws ApiException {
assertTrue(e.getMessage().contains("page_token"));
assertFalse(e.getMessage().contains("secret-token"));
}
+ Mockito.verify(listJobs)._continue("secret-token");
+ }
+
+ /** The page token and the page size are passed on to Kubernetes */
+ @Test
+ public void listAllTaskmasterJobsForUser_page() throws ApiException {
+ given(listJobs.execute()).willReturn(this.resultList());
+ wrapper.listAllTaskmasterJobsForUser("token", 10, User.builder("123").build());
+ Mockito.verify(listJobs)._continue("token");
+ Mockito.verify(listJobs).limit(10);
}
/** Without a page token, a 400 of Kubernetes stays a server error */
@Test(expected = KubernetesException.class)
public void listAllTaskmasterJobsForUser_badRequestWithoutPageToken() throws ApiException {
- this.listJobsFails(null, HttpStatus.BAD_REQUEST.value(), "bad selector");
+ this.listJobsFails(HttpStatus.BAD_REQUEST.value(), "bad selector");
wrapper.listAllTaskmasterJobsForUser(null, null, User.builder("123").build());
}
@@ -784,7 +670,6 @@ public void listAllTaskmasterJobsForUser_badRequestWithoutPageToken() throws Api
@Test(expected = KubernetesException.class)
public void listAllTaskmasterJobsForUser_otherBadRequestWithPageToken() throws ApiException {
this.listJobsFails(
- "token",
HttpStatus.BAD_REQUEST.value(),
"{\"kind\":\"Status\",\"message\":\"unable to parse requirement\"}");
wrapper.listAllTaskmasterJobsForUser("token", null, User.builder("123").build());
@@ -794,36 +679,21 @@ public void listAllTaskmasterJobsForUser_otherBadRequestWithPageToken() throws A
@Test(expected = KubernetesException.class)
public void listAllTaskmasterJobsForUser_badRequestWithoutBodyWithPageToken()
throws ApiException {
- this.listJobsFails("token", HttpStatus.BAD_REQUEST.value(), null);
+ this.listJobsFails(HttpStatus.BAD_REQUEST.value(), null);
wrapper.listAllTaskmasterJobsForUser("token", null, User.builder("123").build());
}
/** Other errors of Kubernetes stay server errors, also with a page token */
@Test(expected = KubernetesException.class)
public void listAllTaskmasterJobsForUser_serverErrorWithPageToken() throws ApiException {
- this.listJobsFails("token", HttpStatus.INTERNAL_SERVER_ERROR.value(), "error");
+ this.listJobsFails(HttpStatus.INTERNAL_SERVER_ERROR.value(), "error");
wrapper.listAllTaskmasterJobsForUser("token", null, User.builder("123").build());
}
/**
- * Lets Kubernetes answer the list request with the page token (or none) with an error and the
- * response body (a Status object)
+ * Lets Kubernetes answer the list request with an error and the response body (a Status object)
*/
- private void listJobsFails(String pageToken, int code, String responseBody)
- throws ApiException {
- given(
- batchApi.listNamespacedJob(
- any(),
- isNull(),
- isNull(),
- pageToken == null ? isNull() : eq(pageToken),
- isNull(),
- any(),
- isNull(),
- isNull(),
- isNull(),
- isNull(),
- isNull()))
- .willThrow(new ApiException("", code, null, responseBody));
+ private void listJobsFails(int code, String responseBody) throws ApiException {
+ given(listJobs.execute()).willThrow(new ApiException("", code, null, responseBody));
}
}
diff --git a/src/test/java/uk/ac/ebi/tsc/tesk/tes/service/TesServiceImplCreateTest.java b/src/test/java/uk/ac/ebi/tsc/tesk/tes/service/TesServiceImplCreateTest.java
index db9c513a..8dc11ce1 100644
--- a/src/test/java/uk/ac/ebi/tsc/tesk/tes/service/TesServiceImplCreateTest.java
+++ b/src/test/java/uk/ac/ebi/tsc/tesk/tes/service/TesServiceImplCreateTest.java
@@ -387,7 +387,7 @@ void noBackendParametersNoNote(CapturedOutput output) {
ArgumentCaptor job = ArgumentCaptor.forClass(V1Job.class);
verify(this.client).createJob(job.capture());
- assertThat(job.getValue().getMetadata().getAnnotations()).isNull();
+ assertThat(job.getValue().getMetadata().getAnnotations()).isNullOrEmpty();
assertThat(output.getOut()).doesNotContain("backend_parameters");
}
}