Skip to content

Commit a8f4615

Browse files
johnflavin-fwjohnflavin-fw
committed
CS-968 Switch client library from docker-client to docker-java (pull request #119)
* Migrate getContainerEvents * Migrate startDockerContainer and setSwarmServiceReplicasToOne * Migrate DockerControlApiIntegrationTest to use new client * Adjust docker http client timeouts * Add another getDockerClientNew method with no args, use default server * Massive change to migrate all integration test client operations use new client * Migrate create container and create service * Migrate delete image * Slight tweaks to BUSYBOX constants and versions * Migrate inspect image / get image by id * Standardize docker-java exception handling * Migrate list images * Migrate ping * Reuse RETURN_SELF answer in test mocks * Migrate container/service logs * Migrate getTaskForService * Migrate pull image (with auth) * Add support for generic resources (i.e. gpus) using a fork of docker-java * Migrate TASK_STATE_FAILED to TaskState.FAILED * Move client config into its own method * Migrate AuthConfig and getting registry URL for image * Complete removal of docker-client * Fix hub ping * Clean up some more fully qualified classes left over from transition * Changelog Approved-by: James Ransford Approved-by: Bin Zhang
1 parent 973ec78 commit a8f4615

23 files changed

+1098
-1349
lines changed

‎CHANGELOG.md

+5
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,13 @@
44
Not yet released.
55

66
* **Improvement** [CS-946][] Prevent setting mutually distinct k8s PVC mounting options
7+
* **Bugfix** [CS-968][] Switch the docker API library we use from [docker-client][] to [docker-java][].
8+
This should restore CS functionality on docker engine v25 and higher.
79

810
[CS-946]: https://radiologics.atlassian.net/browse/CS-946
11+
[CS-968]: https://radiologics.atlassian.net/browse/CS-968
12+
[docker-client]: https://github.com/dmandalidis/docker-client
13+
[docker-java]: https://github.com/docker-java/docker-java
914

1015
## 3.4.3
1116
[Released](https://bitbucket.org/xnatdev/container-service/src/3.4.3/).

‎build.gradle

+5-19
Original file line numberDiff line numberDiff line change
@@ -42,15 +42,15 @@ dependencyManagement {
4242
}
4343
}
4444
}
45-
def vDockerClient = "6.0.4"
45+
46+
def vDockerJava = "3.4.0.1"
4647
def vAutoValue = "1.3"
4748
def vKubernetesClient = "15.0.+"
4849
def vPowerMock = "1.7.0"
4950
def vGson = dependencyManagement.importedProperties["gson.version"]
5051
def vJackson = dependencyManagement.importedProperties["jackson.version"]
5152
def vSpringSecurity = dependencyManagement.importedProperties["spring-security.version"]
5253
def vActiveMQ = dependencyManagement.importedProperties["activemq.version"]
53-
def vJersey = "3.0.6"
5454
def vBouncyCastle = "1.64" // Included in xnat-web via spring-security-jwt 1.1.1, see XNAT-7907
5555

5656
// Use this configuration to put dependencies into the fat jar
@@ -67,23 +67,8 @@ dependencies {
6767

6868
compileOnly "com.google.auto.value:auto-value:${vAutoValue}"
6969

70-
implementAndInclude ("org.mandas:docker-client:${vDockerClient}") {
71-
exclude group: "ch.qos.logback"
72-
exclude group: "org.slf4j"
73-
exclude group: "org.bouncycastle" // included in xnat-web via spring-security-jwt
74-
exclude group: "com.fasterxml.jackson.core"
75-
exclude group: "org.apache.commons", module: "commons-compress" // included in xnat-web via parent
76-
}
77-
78-
implementAndInclude "org.glassfish.jersey.core:jersey-client:${vJersey}"
79-
implementAndInclude "org.glassfish.jersey.inject:jersey-hk2:${vJersey}"
80-
implementAndInclude ("org.glassfish.jersey.connectors:jersey-apache-connector:${vJersey}") {
81-
exclude group: "org.apache.httpcomponents", module: "httpclient" // included in xnat-web directly
82-
}
83-
implementAndInclude ("org.glassfish.jersey.media:jersey-media-json-jackson:${vJersey}") {
84-
exclude group: "com.fasterxml.jackson.core" // Included in xnat-web directly
85-
exclude group: "com.fasterxml.jackson.module", module: "jackson-module-jaxb-annotations" // Included in xnat-web via spawner -> jackson-dataformat-xml
86-
}
70+
implementAndInclude "com.github.docker-java:docker-java-core:${vDockerJava}"
71+
implementAndInclude "com.github.docker-java:docker-java-transport-okhttp:${vDockerJava}"
8772

8873
implementAndInclude ("io.kubernetes:client-java:${vKubernetesClient}") {
8974
exclude group: "com.google.code.gson"
@@ -95,6 +80,7 @@ dependencies {
9580
exclude group: "org.slf4j"
9681
exclude group: "org.bouncycastle"
9782
exclude group: "org.yaml", module: "snakeyaml"
83+
exclude group: "com.squareup.okhttp3", module: "okhttp" // Included via docker-java-transport-okhttp
9884
}
9985
implementAndInclude ("io.kubernetes:client-java-api:${vKubernetesClient}") { // Explicitly including to exclude transitive deps
10086
exclude group: "com.google.code.gson"

‎src/main/java/org/nrg/containers/api/ContainerControlApi.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
11
package org.nrg.containers.api;
22

3-
import org.mandas.docker.client.exceptions.ServiceNotFoundException;
4-
import org.mandas.docker.client.exceptions.TaskNotFoundException;
53
import org.nrg.containers.events.model.DockerContainerEvent;
64
import org.nrg.containers.exceptions.ContainerBackendException;
75
import org.nrg.containers.exceptions.ContainerException;
86
import org.nrg.containers.exceptions.DockerServerException;
97
import org.nrg.containers.exceptions.NoContainerServerException;
108
import org.nrg.containers.exceptions.NoDockerServerException;
9+
import org.nrg.containers.exceptions.ServiceNotFoundException;
10+
import org.nrg.containers.exceptions.TaskNotFoundException;
1111
import org.nrg.containers.model.command.auto.ResolvedCommand;
1212
import org.nrg.containers.model.container.auto.Container;
1313
import org.nrg.containers.model.container.auto.ServiceTask;

‎src/main/java/org/nrg/containers/api/DockerControlApi.java

+535-557
Large diffs are not rendered by default.

‎src/main/java/org/nrg/containers/events/ContainerStatusUpdater.java

+9-6
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,19 @@
11
package org.nrg.containers.events;
22

3+
import com.github.dockerjava.api.model.TaskState;
34
import com.google.common.collect.Lists;
45
import lombok.extern.slf4j.Slf4j;
5-
import org.mandas.docker.client.exceptions.ServiceNotFoundException;
6-
import org.mandas.docker.client.exceptions.TaskNotFoundException;
76
import org.nrg.containers.api.ContainerControlApi;
87
import org.nrg.containers.api.KubernetesClientFactory;
98
import org.nrg.containers.events.model.DockerContainerEvent;
109
import org.nrg.containers.events.model.ServiceTaskEvent;
11-
import org.nrg.containers.exceptions.*;
10+
import org.nrg.containers.exceptions.ContainerException;
11+
import org.nrg.containers.exceptions.DockerServerException;
12+
import org.nrg.containers.exceptions.InvalidDefinitionException;
13+
import org.nrg.containers.exceptions.NoContainerServerException;
14+
import org.nrg.containers.exceptions.NoDockerServerException;
15+
import org.nrg.containers.exceptions.ServiceNotFoundException;
16+
import org.nrg.containers.exceptions.TaskNotFoundException;
1217
import org.nrg.containers.model.container.auto.Container;
1318
import org.nrg.containers.model.container.auto.ServiceTask;
1419
import org.nrg.containers.model.server.docker.DockerServerBase.DockerServer;
@@ -28,8 +33,6 @@
2833
import java.util.Date;
2934
import java.util.List;
3035

31-
import static org.mandas.docker.client.messages.swarm.TaskStatus.TASK_STATE_FAILED;
32-
3336
@Slf4j
3437
@Component
3538
public class ContainerStatusUpdater implements Runnable {
@@ -339,7 +342,7 @@ private void throwLostTaskEventForService(@Nonnull final Container service) {
339342
.serviceId(service.serviceId())
340343
.taskId(null)
341344
.nodeId(null)
342-
.status(TASK_STATE_FAILED)
345+
.status(TaskState.FAILED.getValue())
343346
.swarmNodeError(true)
344347
.statusTime(null)
345348
.message(ServiceTask.swarmNodeErrMsg)

‎src/main/java/org/nrg/containers/exceptions/ServiceNotFoundException.java

+7
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
package org.nrg.containers.exceptions;
2+
3+
public class ServiceNotFoundException extends Exception {
4+
public ServiceNotFoundException(Throwable e) {
5+
super(e);
6+
}
7+
}

‎src/main/java/org/nrg/containers/exceptions/TaskNotFoundException.java

+7
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
package org.nrg.containers.exceptions;
2+
3+
public class TaskNotFoundException extends Exception {
4+
public TaskNotFoundException(Throwable e) {
5+
super(e);
6+
}
7+
}

‎src/main/java/org/nrg/containers/model/container/auto/ServiceTask.java

+28-35
Original file line numberDiff line numberDiff line change
@@ -1,29 +1,26 @@
11
package org.nrg.containers.model.container.auto;
22

3+
import com.github.dockerjava.api.model.Task;
4+
import com.github.dockerjava.api.model.TaskState;
5+
import com.github.dockerjava.api.model.TaskStatus;
6+
import com.github.dockerjava.api.model.TaskStatusContainerStatus;
37
import com.google.auto.value.AutoValue;
4-
import org.mandas.docker.client.messages.swarm.ContainerStatus;
5-
import org.mandas.docker.client.messages.swarm.Task;
6-
import org.mandas.docker.client.messages.swarm.TaskStatus;
7-
import org.apache.commons.lang3.StringUtils;
88

99
import javax.annotation.Nonnull;
1010
import javax.annotation.Nullable;
11-
import java.util.Arrays;
12-
import java.util.Date;
11+
import javax.validation.constraints.NotNull;
1312
import java.util.regex.Pattern;
13+
import java.util.stream.Collectors;
14+
import java.util.stream.Stream;
1415

1516
@AutoValue
1617
public abstract class ServiceTask {
17-
private static final Pattern successStatusPattern = Pattern.compile(TaskStatus.TASK_STATE_COMPLETE);
18+
private static final Pattern successStatusPattern = Pattern.compile(TaskState.COMPLETE.getValue());
1819
private static final Pattern exitStatusPattern = Pattern.compile(
19-
StringUtils.join(
20-
Arrays.asList(TaskStatus.TASK_STATE_FAILED, TaskStatus.TASK_STATE_COMPLETE,
21-
TaskStatus.TASK_STATE_REJECTED, TaskStatus.TASK_STATE_SHUTDOWN), '|'));
22-
private static final Pattern hasNotStartedPattern = Pattern.compile(
23-
StringUtils.join(
24-
Arrays.asList(TaskStatus.TASK_STATE_NEW, TaskStatus.TASK_STATE_ALLOCATED, TaskStatus.TASK_STATE_PENDING,
25-
TaskStatus.TASK_STATE_ASSIGNED, TaskStatus.TASK_STATE_ACCEPTED, TaskStatus.TASK_STATE_PREPARING,
26-
TaskStatus.TASK_STATE_READY, TaskStatus.TASK_STATE_STARTING), '|'));
20+
Stream.of(TaskState.FAILED, TaskState.COMPLETE,
21+
TaskState.REJECTED, TaskState.SHUTDOWN)
22+
.map(TaskState::getValue)
23+
.collect(Collectors.joining("|")));
2724

2825
public static String swarmNodeErrMsg = "Swarm node error";
2926

@@ -38,37 +35,38 @@ public abstract class ServiceTask {
3835
@Nullable public abstract String err();
3936
@Nullable public abstract Long exitCode();
4037

41-
public static ServiceTask create(final @Nonnull Task task, final String serviceId) {
42-
final ContainerStatus containerStatus = task.status().containerStatus();
43-
Long exitCode = containerStatus == null ? null : containerStatus.exitCode();
38+
public static ServiceTask create(final @NotNull Task task, final String serviceId) {
39+
final TaskStatus taskStatus = task.getStatus();
40+
final TaskStatusContainerStatus containerStatus = taskStatus.getContainerStatus();
41+
Long exitCode = containerStatus == null ? null : containerStatus.getExitCodeLong();
4442
// swarmNodeError occurs when node is terminated / spot instance lost while service still trying to run on it
4543
// Criteria: current state = [not an exit status] AND either desired state = shutdown OR exit code = -1
4644
// OR current state = shutdown
47-
String curState = task.status().state();
48-
String msg = task.status().message();
49-
String err = task.status().err();
50-
if (curState.equals(TaskStatus.TASK_STATE_PENDING)) {
45+
TaskState curState = taskStatus.getState();
46+
String msg = taskStatus.getMessage();
47+
String err = taskStatus.getErr();
48+
if (curState.equals(TaskState.PENDING)) {
5149
msg = "";
5250
err = "";
5351
}
54-
boolean swarmNodeError = (!isExitStatus(curState) &&
55-
(task.desiredState().equals(TaskStatus.TASK_STATE_SHUTDOWN) || (exitCode != null && exitCode < 0))) ||
56-
curState.equals(TaskStatus.TASK_STATE_SHUTDOWN);
52+
boolean swarmNodeError = (!isExitStatus(curState.getValue()) &&
53+
(task.getDesiredState().equals(TaskState.SHUTDOWN) || (exitCode != null && exitCode < 0))) ||
54+
curState.equals(TaskState.SHUTDOWN);
5755

5856
if (swarmNodeError) {
5957
msg = swarmNodeErrMsg;
6058
}
6159
return ServiceTask.builder()
6260
.serviceId(serviceId)
63-
.taskId(task.id())
64-
.nodeId(task.nodeId())
65-
.status(task.status().state())
61+
.taskId(task.getId())
62+
.nodeId(task.getNodeId())
63+
.status(curState.getValue())
6664
.swarmNodeError(swarmNodeError)
67-
.statusTime(task.status().timestamp().toString())
65+
.statusTime(taskStatus.getTimestamp())
6866
.message(msg)
6967
.err(err)
7068
.exitCode(exitCode)
71-
.containerId(containerStatus == null ? null : containerStatus.containerId())
69+
.containerId(containerStatus == null ? null : containerStatus.getContainerID())
7270
.build();
7371
}
7472

@@ -104,11 +102,6 @@ public boolean isSuccessfulStatus(){
104102
return isSuccessfulStatus(status);
105103
}
106104

107-
public boolean hasNotStarted() {
108-
final String status = status();
109-
return status == null || hasNotStartedPattern.matcher(status).matches();
110-
}
111-
112105
public static Builder builder() {
113106
return new AutoValue_ServiceTask.Builder();
114107
}

‎src/main/java/org/nrg/containers/services/DockerHubService.java

+1-3
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,11 @@
1111
public interface DockerHubService extends BaseHibernateService<DockerHubEntity> {
1212
DockerHubEntity retrieve(String name) throws NotUniqueException;
1313
DockerHubEntity get(String name) throws NotUniqueException, NotFoundException;
14-
DockerHubEntity getByUrl(final String url) throws NotUniqueException, NotFoundException;
1514
DockerHub retrieveHub(long id);
1615
DockerHub retrieveHub(String name) throws NotUniqueException;
1716
DockerHub getHub(long hubId) throws NotFoundException;
1817
DockerHub getHub(String hubName) throws NotFoundException, NotUniqueException;
18+
DockerHub getByUrl(final String url);
1919
List<DockerHub> getHubs();
2020

2121
void setDefault(DockerHub dockerHub, String username, String reason);
@@ -34,8 +34,6 @@ public interface DockerHubService extends BaseHibernateService<DockerHubEntity>
3434
long getDefaultHubId();
3535
DockerHub getDefault();
3636

37-
DockerHub getHubForImage(String imageName);
38-
3937
class DockerHubDeleteDefaultException extends RuntimeException {
4038
public DockerHubDeleteDefaultException(final String message) {
4139
super(message);

‎src/main/java/org/nrg/containers/services/impl/ContainerFinalizeServiceImpl.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package org.nrg.containers.services.impl;
22

3+
import com.github.dockerjava.api.model.TaskState;
34
import com.google.common.base.Function;
45
import com.google.common.base.Joiner;
56
import com.google.common.collect.Lists;
@@ -8,7 +9,6 @@
89
import org.apache.commons.lang3.StringUtils;
910
import org.apache.commons.lang3.tuple.ImmutablePair;
1011
import org.apache.commons.lang3.tuple.Pair;
11-
import org.mandas.docker.client.messages.swarm.TaskStatus;
1212
import org.nrg.action.ClientException;
1313
import org.nrg.containers.api.ContainerControlApi;
1414
import org.nrg.containers.api.LogType;
@@ -288,7 +288,7 @@ public String apply(final Exception input) {
288288
: "";
289289
final boolean startsWithFailed = containerStatus.startsWith(PersistentWorkflowUtils.FAILED);
290290
if (startsWithFailed ||
291-
containerStatus.equals(TaskStatus.TASK_STATE_FAILED) ||
291+
containerStatus.equals(TaskState.FAILED.getValue()) ||
292292
containerStatus.equals("die")) {
293293
// This history entry should give us the details that we need
294294
if (startsWithFailed) {

‎src/main/java/org/nrg/containers/services/impl/ContainerServiceImpl.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
import com.fasterxml.jackson.core.type.TypeReference;
55
import com.fasterxml.jackson.databind.ObjectMapper;
6+
import com.github.dockerjava.api.model.TaskState;
67
import com.google.common.annotations.VisibleForTesting;
78
import com.google.common.base.Strings;
89
import com.google.common.cache.CacheBuilder;
@@ -13,7 +14,6 @@
1314
import org.apache.commons.io.IOUtils;
1415
import org.apache.commons.lang3.StringUtils;
1516
import org.apache.commons.lang3.tuple.Pair;
16-
import org.mandas.docker.client.messages.swarm.TaskStatus;
1717
import org.nrg.action.ClientException;
1818
import org.nrg.containers.api.ContainerControlApi;
1919
import org.nrg.containers.api.LogType;
@@ -1072,7 +1072,7 @@ public boolean restartService(Container service, UserI userI) {
10721072
final ContainerHistory newHistoryItem = ContainerHistory.builder()
10731073
.entityType("service")
10741074
.entityId(null)
1075-
.status(TaskStatus.TASK_STATE_FAILED)
1075+
.status(TaskState.FAILED.getValue())
10761076
.exitCode("126")
10771077
.timeRecorded(now)
10781078
.externalTimestamp(String.valueOf(now.getTime()))

‎src/main/java/org/nrg/containers/services/impl/HibernateDockerHubService.java

+11-22
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
import com.google.common.base.Function;
44
import com.google.common.collect.Lists;
55
import lombok.extern.slf4j.Slf4j;
6-
import org.mandas.docker.client.ImageRef;
76
import org.nrg.containers.daos.DockerHubDao;
87
import org.nrg.containers.exceptions.NotUniqueException;
98
import org.nrg.containers.model.dockerhub.DockerHubBase.DockerHub;
@@ -62,16 +61,18 @@ public DockerHubEntity get(final String name) throws NotUniqueException, NotFoun
6261
}
6362

6463
@Override
65-
public DockerHubEntity getByUrl(final String url) throws NotUniqueException, NotFoundException {
66-
DockerHubEntity dockerHubEntity = getDao().findByUrl(url);
67-
if (dockerHubEntity == null && url.startsWith("https")) {
68-
// try with http (org.mandas.docker.client.ImageRef always returns https)
69-
dockerHubEntity = getDao().findByUrl(url.replace("https", "http"));
70-
}
71-
if (dockerHubEntity == null) {
72-
throw new NotFoundException("Could not find hub with name " + url);
64+
public DockerHub getByUrl(final String url) {
65+
try {
66+
DockerHubEntity entity = getDao().findByUrl(url);
67+
if (entity == null && url.startsWith("https")) {
68+
// try with http (org.mandas.docker.client.ImageRef always returns https)
69+
entity = getDao().findByUrl(url.replace("https", "http"));
70+
}
71+
return toPojo(entity, getDefaultHubId());
72+
} catch (NotUniqueException e) {
73+
log.error("Found multiple DockerHubs with url {}", url);
74+
return null;
7375
}
74-
return dockerHubEntity;
7576
}
7677

7778
@Override
@@ -148,18 +149,6 @@ public DockerHub getDefault() {
148149
return retrieveHub(getDefaultHubId());
149150
}
150151

151-
@Override
152-
public DockerHub getHubForImage(String imageName) {
153-
final String url = new ImageRef(imageName).getRegistryUrl();
154-
DockerHubEntity entity = null;
155-
try {
156-
entity = getByUrl(url);
157-
} catch (NotUniqueException | NotFoundException e) {
158-
log.error("Unable to locate docker hub for url {}", url, e);
159-
}
160-
return toPojo(entity, getDefaultHubId());
161-
}
162-
163152
@Override
164153
public void setDefault(final DockerHub dockerHub, final String username, final String reason) {
165154
setDefault(dockerHub.id(), username, reason);

0 commit comments

Comments
 (0)