Skip to content

Fix incomplete log entries read by FramedInputStreamConsumer #1754

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
yuting-liu wants to merge 11 commits into from
Closed

Fix incomplete log entries read by FramedInputStreamConsumer #1754

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
642c0ec
change pom.xml
yuting-liu Dec 14, 2017
72fe550
Merge pull request #1 from yuting-liu/yuting-fork
yuting-liu Dec 14, 2017
f15ad11
resync
yuting-liu Dec 15, 2021
aedf866
Merge branch 'docker-java-master'
yuting-liu Dec 15, 2021
655e72c
Fix incomplete frame
yuting-liu Dec 15, 2021
e230878
Add the unit test
yuting-liu Jan 13, 2022
0510ef4
Add pulling image
yuting-liu Jan 13, 2022
66d7efd
Fix the code with unit test
yuting-liu Jan 15, 2022
905c8e9
Merge branch 'master' into yuting-fix-frame
yuting-liu Jan 16, 2022
43d12e6
Fix unit test
yuting-liu Jan 19, 2022
b399388
Fix test
yuting-liu Feb 2, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 6 additions & 8 deletions docker-java-core/src/main/java/com/github/dockerjava/core/FramedInputStreamConsumer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,20 +65,18 @@ public void accept(DockerHttpClient.Response response) {
bytesToRead |= (readByte & 0xff) << (8 * (3 - i));
}

byte[] payload = new byte[bytesToRead];
int actualPayloadSize = 0;
do {
int readBytes = body.read(buffer, 0, Math.min(buffer.length, bytesToRead));
int readBytes = body.read(payload, actualPayloadSize, bytesToRead - actualPayloadSize);
if (readBytes < 0) {
// TODO log?
return;
}

if (readBytes == buffer.length) {
resultCallback.onNext(new Frame(streamType, buffer));
} else {
resultCallback.onNext(new Frame(streamType, Arrays.copyOf(buffer, readBytes)));
}
bytesToRead -= readBytes;
} while (bytesToRead > 0);
actualPayloadSize += readBytes;
} while (actualPayloadSize < bytesToRead);
resultCallback.onNext(new Frame(streamType, payload));
}
} catch (Exception e) {
resultCallback.onError(e);
Expand Down
100 changes: 68 additions & 32 deletions docker-java/src/test/java/com/github/dockerjava/cmd/LogContainerCmdIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,9 @@ public class LogContainerCmdIT extends CmdIT {
public void asyncLogContainerWithTtyEnabled() throws Exception {

CreateContainerResponse container = dockerRule.getClient().createContainerCmd("busybox")
.withCmd("/bin/sh", "-c", "while true; do echo hello; sleep 1; done")
.withTty(true)
.exec();
.withCmd("/bin/sh", "-c", "while true; do echo hello; sleep 1; done")
.withTty(true)
.exec();

LOG.info("Created container: {}", container.toString());
assertThat(container.getId(), not(is(emptyString())));
Expand Down Expand Up @@ -73,9 +73,9 @@ public void asyncLogContainerWithTtyEnabled() throws Exception {
public void asyncLogContainerWithTtyDisabled() throws Exception {

CreateContainerResponse container = dockerRule.getClient().createContainerCmd("busybox")
.withCmd("/bin/sh", "-c", "while true; do echo hello; sleep 1; done")
.withTty(false)
.exec();
.withCmd("/bin/sh", "-c", "while true; do echo hello; sleep 1; done")
.withTty(false)
.exec();

LOG.info("Created container: {}", container.toString());
assertThat(container.getId(), not(is(emptyString())));
Expand Down Expand Up @@ -122,11 +122,13 @@ public void onError(Throwable throwable) {
public void onComplete() {
super.onComplete();
throw new AssertionError("expected NotFoundException");
};
}

;
};

dockerRule.getClient().logContainerCmd("non-existing").withStdErr(true).withStdOut(true).exec(loggingCallback)
.awaitCompletion();
.awaitCompletion();
}

@Test
Expand All @@ -135,44 +137,44 @@ public void asyncMultipleLogContainer() throws Exception {
String snippet = "hello world";

CreateContainerResponse container = dockerRule.getClient().createContainerCmd("busybox")
.withCmd("/bin/echo", snippet)
.exec();
.withCmd("/bin/echo", snippet)
.exec();

LOG.info("Created container: {}", container.toString());
assertThat(container.getId(), not(is(emptyString())));

dockerRule.getClient().startContainerCmd(container.getId()).exec();

int exitCode = dockerRule.getClient().waitContainerCmd(container.getId())
.start()
.awaitStatusCode();
.start()
.awaitStatusCode();

assertThat(exitCode, equalTo(0));

LogContainerTestCallback loggingCallback = new LogContainerTestCallback();

dockerRule.getClient().logContainerCmd(container.getId())
.withStdErr(true)
.withStdOut(true)
.exec(loggingCallback);
.withStdErr(true)
.withStdOut(true)
.exec(loggingCallback);

loggingCallback.close();

loggingCallback = new LogContainerTestCallback();

dockerRule.getClient().logContainerCmd(container.getId())
.withStdErr(true)
.withStdOut(true)
.exec(loggingCallback);
.withStdErr(true)
.withStdOut(true)
.exec(loggingCallback);

loggingCallback.close();

loggingCallback = new LogContainerTestCallback();

dockerRule.getClient().logContainerCmd(container.getId())
.withStdErr(true)
.withStdOut(true)
.exec(loggingCallback);
.withStdErr(true)
.withStdOut(true)
.exec(loggingCallback);

loggingCallback.awaitCompletion();

Expand All @@ -184,8 +186,8 @@ public void asyncLogContainerWithSince() throws Exception {
String snippet = "hello world";

CreateContainerResponse container = dockerRule.getClient().createContainerCmd("busybox")
.withCmd("/bin/echo", snippet)
.exec();
.withCmd("/bin/echo", snippet)
.exec();

LOG.info("Created container: {}", container.toString());
assertThat(container.getId(), not(is(emptyString())));
Expand All @@ -195,19 +197,19 @@ public void asyncLogContainerWithSince() throws Exception {
dockerRule.getClient().startContainerCmd(container.getId()).exec();

int exitCode = dockerRule.getClient().waitContainerCmd(container.getId())
.start()
.awaitStatusCode();
.start()
.awaitStatusCode();

assertThat(exitCode, equalTo(0));

LogContainerTestCallback loggingCallback = new LogContainerTestCallback();

dockerRule.getClient().logContainerCmd(container.getId())
.withStdErr(true)
.withStdOut(true)
.withSince(timestamp)
.withUntil(timestamp + 1000)
.exec(loggingCallback);
.withStdErr(true)
.withStdOut(true)
.withSince(timestamp)
.withUntil(timestamp + 1000)
.exec(loggingCallback);

loggingCallback.awaitCompletion();

Expand All @@ -219,8 +221,8 @@ public void simultaneousCommands() throws Exception {
// Create a new client to not affect other tests
DockerClient client = dockerRule.newClient();
CreateContainerResponse container = client.createContainerCmd("busybox")
.withCmd("/bin/sh", "-c", "echo hello world; sleep infinity")
.exec();
.withCmd("/bin/sh", "-c", "echo hello world; sleep infinity")
.exec();

client.startContainerCmd(container.getId()).exec();

Expand Down Expand Up @@ -261,4 +263,38 @@ public void onNext(Frame object) {
executor.shutdownNow();
}
}

@Test
public void asyncLogContainerWithTailAll() throws Exception {
// Create a new client to not affect other tests
String testImage = "icevivek/logreader";

// Pulling image icevivek/logreader
try {
dockerRule.getClient().inspectImageCmd(testImage).exec();
} catch (NotFoundException e) {
LOG.info("Pulling image ");
// need to block until image is pulled completely
dockerRule.getClient().pullImageCmd(testImage)
.withTag("latest")
.start()
.awaitCompletion(30, TimeUnit.SECONDS);
}

LogContainerTestCallback loggingCallback = new LogContainerTestCallback(true);

for (int i = 0; i < 5; ++i) {
CreateContainerResponse container = dockerRule.getClient().createContainerCmd("icevivek/logreader").exec();
dockerRule.getClient().startContainerCmd(container.getId()).exec();
// this essentially test the since=0 case
dockerRule.getClient().logContainerCmd(container.getId())
.withStdErr(true)
.withStdOut(true)
.withFollowStream(true)
.exec(loggingCallback)
.awaitCompletion();
}

assertEquals(748, loggingCallback.getCollectedFrames().size());
}
}