Skip to content

Commit 8902e32

Browse files
fbuecklersKostyaSha
fbuecklers
authored and
KostyaSha
committed
fix errors in tests (#938)
* Improve PullImageCmd for pulling an image through docker swarm. * fix errors in tests * fix errors in pull tests will also fix #939 * Fix more concurrent test issues * Some more swarm fixes * Fix pull handling for swarm * More fixes * Suggest test code fixes * Restore original pull/push expected exception logic
1 parent fce9df9 commit 8902e32

File tree

13 files changed

+202
-118
lines changed

13 files changed

+202
-118
lines changed

‎.travis.yml‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,9 @@ env:
2626
# - repo="main" DOCKER_HOST="unix:///var/run/docker.sock" DOCKER_VERSION="17.09.0~ce-0~ubuntu-trusty"
2727
# - repo="main" DOCKER_HOST="tcp://127.0.0.1:2375" DOCKER_VERSION="17.06.2~ce-0~ubuntu-trusty" DEPLOY=true COVERITY=true
2828
- repo="main" DOCKER_HOST="tcp://127.0.0.1:2375" DOCKER_VERSION="17.05.0~ce-0~ubuntu-trusty" DEPLOY=true COVERITY=true
29-
- repo="main" DOCKER_HOST="tcp://127.0.0.1:2377" DOCKER_VERSION="17.05.0~ce-0~ubuntu-trusty" SWARM_VERSION="1.2.6"
29+
- repo="main" DOCKER_HOST="tcp://127.0.0.1:2377" DOCKER_VERSION="17.05.0~ce-0~ubuntu-trusty" SWARM_VERSION="1.2.8"
3030
- repo="main" DOCKER_HOST="unix:///var/run/docker.sock" DOCKER_VERSION="17.05.0~ce-0~ubuntu-trusty"
31-
- repo="main" DOCKER_HOST="tcp://127.0.0.1:2377" DOCKER_VERSION="1.13.1-0~ubuntu-trusty" SWARM_VERSION="1.2.6"
31+
- repo="main" DOCKER_HOST="tcp://127.0.0.1:2377" DOCKER_VERSION="1.13.1-0~ubuntu-trusty" SWARM_VERSION="1.2.8"
3232
- repo="main" DOCKER_HOST="tcp://127.0.0.1:2375" DOCKER_VERSION="1.13.1-0~ubuntu-trusty"
3333
- repo="main" DOCKER_HOST="unix:///var/run/docker.sock" DOCKER_VERSION="1.13.1-0~ubuntu-trusty"
3434
- repo="main" DOCKER_HOST="tcp://127.0.0.1:2375" DOCKER_VERSION="1.12.6-0~ubuntu-trusty"

‎src/main/java/com/github/dockerjava/api/command/InspectImageResponse.java‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,7 @@ public InspectImageResponse withComment(String comment) {
122122
}
123123

124124
/**
125+
* Get the image commit configuration
125126
* @see #config
126127
*/
127128
@CheckForNull
@@ -154,6 +155,8 @@ public InspectImageResponse withContainer(String container) {
154155
}
155156

156157
/**
158+
* If the image was created from a container, this config contains the configuration of the container
159+
* which was committed in this image
157160
* @see #containerConfig
158161
*/
159162
@CheckForNull

‎src/main/java/com/github/dockerjava/core/async/ResultCallbackTemplate.java‎

Lines changed: 21 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -3,19 +3,16 @@
33
*/
44
package com.github.dockerjava.core.async;
55

6+
import com.github.dockerjava.api.async.ResultCallback;
7+
import com.google.common.base.Throwables;
8+
import org.slf4j.Logger;
9+
import org.slf4j.LoggerFactory;
10+
611
import java.io.Closeable;
712
import java.io.IOException;
813
import java.util.concurrent.CountDownLatch;
914
import java.util.concurrent.TimeUnit;
1015

11-
import javax.annotation.CheckForNull;
12-
13-
import org.slf4j.Logger;
14-
import org.slf4j.LoggerFactory;
15-
16-
import com.github.dockerjava.api.async.ResultCallback;
17-
import com.google.common.base.Throwables;
18-
1916
/**
2017
* Abstract template implementation of {@link ResultCallback}
2118
*
@@ -91,7 +88,7 @@ public void close() throws IOException {
9188
public RC_T awaitCompletion() throws InterruptedException {
9289
completed.await();
9390
// eventually (re)throws RuntimeException
94-
getFirstError();
91+
throwFirstError();
9592
return (RC_T) this;
9693
}
9794

@@ -102,13 +99,14 @@ public RC_T awaitCompletion() throws InterruptedException {
10299
*/
103100
public boolean awaitCompletion(long timeout, TimeUnit timeUnit) throws InterruptedException {
104101
boolean result = completed.await(timeout, timeUnit);
105-
getFirstError();
102+
throwFirstError();
106103
return result;
107104
}
108105

109106
/**
110-
* Blocks until {@link ResultCallback#onStart()} was called. {@link ResultCallback#onStart()} is called when the request was processed
111-
* on the server side and the response is incoming.
107+
* Blocks until {@link ResultCallback#onStart(Closeable)} was called.
108+
* {@link ResultCallback#onStart(Closeable)} is called when the request was processed on the server
109+
* side and the response is incoming.
112110
*/
113111
@SuppressWarnings("unchecked")
114112
public RC_T awaitStarted() throws InterruptedException {
@@ -117,22 +115,25 @@ public RC_T awaitStarted() throws InterruptedException {
117115
}
118116

119117
/**
120-
* Blocks until {@link ResultCallback#onStart()} was called or the given timeout occurs. {@link ResultCallback#onStart()} is called when
121-
* the request was processed on the server side and the response is incoming.
118+
* Blocks until {@link ResultCallback#onStart(Closeable)} was called or the given timeout occurs.
119+
* {@link ResultCallback#onStart(Closeable)} is called when the request was processed on the server side
120+
* and the response is incoming.
122121
* @return {@code true} if started and {@code false} if the waiting time elapsed
123-
* before {@link ResultCallback#onStart()} was called.
122+
* before {@link ResultCallback#onStart(Closeable)} was called.
124123
*/
125124
public boolean awaitStarted(long timeout, TimeUnit timeUnit) throws InterruptedException {
126125
return started.await(timeout, timeUnit);
127126
}
128127

129-
@CheckForNull
130-
protected RuntimeException getFirstError() {
128+
/**
129+
* Throws the first occurred error as a runtime exception
130+
* @throws com.github.dockerjava.api.exception.DockerException The first docker based Error
131+
* @throws RuntimeException on any other occurred error
132+
*/
133+
protected void throwFirstError() {
131134
if (firstError != null) {
132135
// this call throws a RuntimeException
133-
return Throwables.propagate(firstError);
134-
} else {
135-
return null;
136+
Throwables.propagate(firstError);
136137
}
137138
}
138139
}

‎src/main/java/com/github/dockerjava/core/command/PullImageResultCallback.java‎

Lines changed: 89 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -3,14 +3,16 @@
33
*/
44
package com.github.dockerjava.core.command;
55

6-
import javax.annotation.CheckForNull;
7-
8-
import org.slf4j.Logger;
9-
import org.slf4j.LoggerFactory;
10-
116
import com.github.dockerjava.api.exception.DockerClientException;
127
import com.github.dockerjava.api.model.PullResponseItem;
138
import com.github.dockerjava.core.async.ResultCallbackTemplate;
9+
import org.slf4j.Logger;
10+
import org.slf4j.LoggerFactory;
11+
12+
import javax.annotation.CheckForNull;
13+
import java.util.HashMap;
14+
import java.util.Map;
15+
import java.util.concurrent.TimeUnit;
1416

1517
/**
1618
*
@@ -21,18 +23,99 @@ public class PullImageResultCallback extends ResultCallbackTemplate<PullImageRes
2123

2224
private static final Logger LOGGER = LoggerFactory.getLogger(PullImageResultCallback.class);
2325

26+
private boolean isSwarm = false;
27+
private Map<String, PullResponseItem> results = null;
28+
2429
@CheckForNull
2530
private PullResponseItem latestItem = null;
2631

2732
@Override
2833
public void onNext(PullResponseItem item) {
29-
this.latestItem = item;
34+
// only do it once
35+
if (results == null && latestItem == null) {
36+
checkForDockerSwarmResponse(item);
37+
}
38+
39+
if (isSwarm) {
40+
handleDockerSwarmResponse(item);
41+
} else {
42+
handleDockerClientResponse(item);
43+
}
44+
3045
LOGGER.debug(item.toString());
3146
}
3247

48+
private void checkForDockerSwarmResponse(PullResponseItem item) {
49+
if (item.getStatus().matches("Pulling\\s.+\\.{3}$")) {
50+
isSwarm = true;
51+
LOGGER.debug("Communicating with Docker Swarm.");
52+
}
53+
}
54+
55+
private void handleDockerSwarmResponse(final PullResponseItem item) {
56+
if (results == null) {
57+
results = new HashMap<>();
58+
}
59+
60+
// Swarm terminates a pull sometimes with an empty line.
61+
// Therefore keep first success message
62+
PullResponseItem currentItem = results.get(item.getId());
63+
if (currentItem == null || !currentItem.isPullSuccessIndicated()) {
64+
results.put(item.getId(), item);
65+
}
66+
}
67+
68+
private void handleDockerClientResponse(PullResponseItem item) {
69+
latestItem = item;
70+
}
71+
72+
private void checkDockerSwarmPullSuccessful() {
73+
if (results.isEmpty()) {
74+
throw new DockerClientException("Could not pull image through Docker Swarm");
75+
} else {
76+
boolean pullFailed = false;
77+
StringBuilder sb = new StringBuilder();
78+
79+
for (PullResponseItem pullResponseItem : results.values()) {
80+
if (!pullResponseItem.isPullSuccessIndicated()) {
81+
pullFailed = true;
82+
sb.append("[" + pullResponseItem.getId() + ":" + messageFromPullResult(pullResponseItem) + "]");
83+
}
84+
}
85+
86+
if (pullFailed) {
87+
throw new DockerClientException("Could not pull image: " + sb.toString());
88+
}
89+
}
90+
}
91+
92+
private void checkDockerClientPullSuccessful() {
93+
if (latestItem == null) {
94+
throw new DockerClientException("Could not pull image");
95+
} else if (!latestItem.isPullSuccessIndicated()) {
96+
throw new DockerClientException("Could not pull image: " + messageFromPullResult(latestItem));
97+
}
98+
}
99+
100+
private String messageFromPullResult(PullResponseItem pullResponseItem) {
101+
return (pullResponseItem.getError() != null) ? pullResponseItem.getError() : pullResponseItem.getStatus();
102+
}
103+
104+
@Override
105+
protected void throwFirstError() {
106+
super.throwFirstError();
107+
108+
if (isSwarm) {
109+
checkDockerSwarmPullSuccessful();
110+
} else {
111+
checkDockerClientPullSuccessful();
112+
}
113+
}
114+
33115
/**
34116
* Awaits the image to be pulled successful.
35117
*
118+
* @deprecated use {@link #awaitCompletion()} or {@link #awaitCompletion(long, TimeUnit)} instead
36119
* @throws DockerClientException
37120
* if the pull fails.
38121
*/
@@ -42,12 +125,5 @@ public void awaitSuccess() {
42125
} catch (InterruptedException e) {
43126
throw new DockerClientException("", e);
44127
}
45-
46-
if (latestItem == null) {
47-
throw new DockerClientException("Could not pull image");
48-
} else if (!latestItem.isPullSuccessIndicated()) {
49-
String message = (latestItem.getError() != null) ? latestItem.getError() : latestItem.getStatus();
50-
throw new DockerClientException("Could not pull image: " + message);
51-
}
52128
}
53129
}

‎src/main/java/com/github/dockerjava/core/command/PushImageResultCallback.java‎

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3,14 +3,14 @@
33
*/
44
package com.github.dockerjava.core.command;
55

6-
import javax.annotation.CheckForNull;
7-
8-
import org.slf4j.Logger;
9-
import org.slf4j.LoggerFactory;
10-
116
import com.github.dockerjava.api.exception.DockerClientException;
127
import com.github.dockerjava.api.model.PushResponseItem;
138
import com.github.dockerjava.core.async.ResultCallbackTemplate;
9+
import org.slf4j.Logger;
10+
import org.slf4j.LoggerFactory;
11+
12+
import javax.annotation.CheckForNull;
13+
import java.util.concurrent.TimeUnit;
1414

1515
/**
1616
*
@@ -30,9 +30,21 @@ public void onNext(PushResponseItem item) {
3030
LOGGER.debug(item.toString());
3131
}
3232

33+
@Override
34+
protected void throwFirstError() {
35+
super.throwFirstError();
36+
37+
if (latestItem == null) {
38+
throw new DockerClientException("Could not push image");
39+
} else if (latestItem.isErrorIndicated()) {
40+
throw new DockerClientException("Could not push image: " + latestItem.getError());
41+
}
42+
}
43+
3344
/**
3445
* Awaits the image to be pulled successful.
3546
*
47+
* @deprecated use {@link #awaitCompletion()} or {@link #awaitCompletion(long, TimeUnit)} instead
3648
* @throws DockerClientException
3749
* if the push fails.
3850
*/
@@ -42,11 +54,5 @@ public void awaitSuccess() {
4254
} catch (InterruptedException e) {
4355
throw new DockerClientException("", e);
4456
}
45-
46-
if (latestItem == null) {
47-
throw new DockerClientException("Could not push image");
48-
} else if (latestItem.isErrorIndicated()) {
49-
throw new DockerClientException("Could not push image: " + latestItem.getError());
50-
}
5157
}
5258
}

‎src/main/java/com/github/dockerjava/netty/InvocationBuilder.java‎

Lines changed: 14 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,18 @@
11
package com.github.dockerjava.netty;
22

3+
import com.fasterxml.jackson.core.JsonProcessingException;
4+
import com.fasterxml.jackson.core.type.TypeReference;
5+
import com.fasterxml.jackson.databind.ObjectMapper;
6+
import com.github.dockerjava.api.async.ResultCallback;
7+
import com.github.dockerjava.api.exception.DockerClientException;
8+
import com.github.dockerjava.api.model.Frame;
9+
import com.github.dockerjava.core.async.ResultCallbackTemplate;
10+
import com.github.dockerjava.netty.handler.FramedResponseStreamHandler;
11+
import com.github.dockerjava.netty.handler.HttpConnectionHijackHandler;
12+
import com.github.dockerjava.netty.handler.HttpRequestProvider;
13+
import com.github.dockerjava.netty.handler.HttpResponseHandler;
14+
import com.github.dockerjava.netty.handler.HttpResponseStreamHandler;
15+
import com.github.dockerjava.netty.handler.JsonResponseCallbackHandler;
316
import io.netty.buffer.Unpooled;
417
import io.netty.channel.Channel;
518
import io.netty.channel.ChannelFuture;
@@ -29,20 +42,6 @@
2942
import java.util.Map;
3043
import java.util.concurrent.CountDownLatch;
3144

32-
import com.fasterxml.jackson.core.JsonProcessingException;
33-
import com.fasterxml.jackson.core.type.TypeReference;
34-
import com.fasterxml.jackson.databind.ObjectMapper;
35-
import com.github.dockerjava.api.async.ResultCallback;
36-
import com.github.dockerjava.api.exception.DockerClientException;
37-
import com.github.dockerjava.api.model.Frame;
38-
import com.github.dockerjava.core.async.ResultCallbackTemplate;
39-
import com.github.dockerjava.netty.handler.FramedResponseStreamHandler;
40-
import com.github.dockerjava.netty.handler.HttpConnectionHijackHandler;
41-
import com.github.dockerjava.netty.handler.HttpRequestProvider;
42-
import com.github.dockerjava.netty.handler.HttpResponseHandler;
43-
import com.github.dockerjava.netty.handler.HttpResponseStreamHandler;
44-
import com.github.dockerjava.netty.handler.JsonResponseCallbackHandler;
45-
4645
/**
4746
* This class is basically a replacement of javax.ws.rs.client.Invocation.Builder to allow simpler migration of JAX-RS code to a netty based
4847
* implementation.
@@ -122,7 +121,7 @@ public A_RES_T awaitResult() {
122121
} catch (InterruptedException e) {
123122
throw new RuntimeException(e);
124123
}
125-
getFirstError();
124+
throwFirstError();
126125
return result;
127126
}
128127
}

‎src/test/java/com/github/dockerjava/cmd/AttachContainerCmdIT.java‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -214,11 +214,6 @@ public void onNext(Frame item) {
214214
super.onNext(item);
215215
}
216216

217-
@Override
218-
public RuntimeException getFirstError() {
219-
return super.getFirstError();
220-
}
221-
222217
@Override
223218
public String toString() {
224219
return log.toString();

0 commit comments

Comments
 (0)