From e195321c48e742113e0172c95613e5ea60b2bd35 Mon Sep 17 00:00:00 2001 From: Jochen Schalanda Date: Thu, 29 Feb 2024 22:06:25 +0100 Subject: [PATCH 1/2] Use local disk for GenericContainer#withCopyFileToContainer() This is an attempt to fix `OutOfMemoryError` (out of heap memory) when copying large files into a container via `GenericContainer#withCopyFileToContainer()`. The previous implementation relied on an in-memory array backing a `ByteArrayInputStream` which represents the tar archive sent to the Docker daemon which is limited by the available heap memory of the JVM at the moment of execution. By using the local disk (temporary directory), we can copy files into the container which are larger than the available heap memory of the JVM. The caveat being that we are now limited by the available disk space in the temporary directory of the machine running Testcontainers. Fixes #4203 --- .../containers/ContainerState.java | 34 ++++++++++----- .../junit/FileOperationsTest.java | 41 +++++++++++++++++++ 2 files changed, 65 insertions(+), 10 deletions(-) diff --git a/core/src/main/java/org/testcontainers/containers/ContainerState.java b/core/src/main/java/org/testcontainers/containers/ContainerState.java index b3a38febf73..b289f35f7a3 100644 --- a/core/src/main/java/org/testcontainers/containers/ContainerState.java +++ b/core/src/main/java/org/testcontainers/containers/ContainerState.java @@ -22,14 +22,17 @@ import org.testcontainers.utility.MountableFile; import org.testcontainers.utility.ThrowingFunction; -import java.io.ByteArrayInputStream; -import java.io.ByteArrayOutputStream; +import java.io.BufferedInputStream; +import java.io.BufferedOutputStream; import java.io.File; import java.io.FileOutputStream; import java.io.IOException; import java.io.InputStream; +import java.io.OutputStream; import java.nio.charset.Charset; import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; import java.util.ArrayList; import java.util.List; import java.util.Map; @@ -345,21 +348,32 @@ default void copyFileToContainer(Transferable transferable, String containerPath throw new IllegalStateException("copyFileToContainer can only be used with created / running container"); } + Path tempFile = Files.createTempFile("tc-", "-copy"); + try ( - ByteArrayOutputStream byteArrayOutputStream = new ByteArrayOutputStream(); - TarArchiveOutputStream tarArchive = new TarArchiveOutputStream(byteArrayOutputStream) + OutputStream os = Files.newOutputStream(tempFile); + BufferedOutputStream bos = new BufferedOutputStream(os); + TarArchiveOutputStream tarArchive = new TarArchiveOutputStream(bos) ) { tarArchive.setLongFileMode(TarArchiveOutputStream.LONGFILE_POSIX); tarArchive.setBigNumberMode(TarArchiveOutputStream.BIGNUMBER_POSIX); transferable.transferTo(tarArchive, containerPath); tarArchive.finish(); - - getDockerClient() - .copyArchiveToContainerCmd(getContainerId()) - .withTarInputStream(new ByteArrayInputStream(byteArrayOutputStream.toByteArray())) - .withRemotePath("/") - .exec(); + bos.flush(); + + try ( + InputStream is = Files.newInputStream(tempFile); + BufferedInputStream bis = new BufferedInputStream(is) + ) { + getDockerClient() + .copyArchiveToContainerCmd(getContainerId()) + .withTarInputStream(bis) + .withRemotePath("/") + .exec(); + } + } finally { + Files.deleteIfExists(tempFile); } } diff --git a/core/src/test/java/org/testcontainers/junit/FileOperationsTest.java b/core/src/test/java/org/testcontainers/junit/FileOperationsTest.java index d9655443480..4b3aa804b50 100644 --- a/core/src/test/java/org/testcontainers/junit/FileOperationsTest.java +++ b/core/src/test/java/org/testcontainers/junit/FileOperationsTest.java @@ -3,15 +3,19 @@ import com.github.dockerjava.api.exception.NotFoundException; import org.apache.commons.io.FileUtils; import org.apache.commons.io.IOUtils; +import org.apache.commons.io.output.CountingOutputStream; import org.junit.Rule; import org.junit.Test; import org.junit.rules.TemporaryFolder; import org.testcontainers.TestImages; +import org.testcontainers.containers.Container; import org.testcontainers.containers.GenericContainer; import org.testcontainers.containers.startupcheck.OneShotStartupCheckStrategy; import org.testcontainers.utility.MountableFile; +import java.io.BufferedOutputStream; import java.io.File; +import java.io.FileOutputStream; import java.io.IOException; import static org.assertj.core.api.Assertions.assertThat; @@ -40,6 +44,43 @@ public void copyFileToContainerFileTest() throws Exception { } } + @Test + public void copyLargeFilesToContainer() throws Exception { + File tempFile = temporaryFolder.newFile(); + try ( + GenericContainer alpineCopyToContainer = new GenericContainer(TestImages.ALPINE_IMAGE) // + .withCommand("sleep", "infinity") + ) { + alpineCopyToContainer.start(); + final long byteCount; + try ( + FileOutputStream fos = new FileOutputStream(tempFile); + CountingOutputStream cos = new CountingOutputStream(fos); + BufferedOutputStream bos = new BufferedOutputStream(cos) + ) { + for (int i = 0; i < 0x4000; i++) { + byte[] bytes = new byte[0xFFFF]; + bos.write(bytes); + } + bos.flush(); + byteCount = cos.getByteCount(); + } + final MountableFile mountableFile = MountableFile.forHostPath(tempFile.getPath()); + final String containerPath = "/test.bin"; + alpineCopyToContainer.copyFileToContainer(mountableFile, containerPath); + + final Container.ExecResult execResult = alpineCopyToContainer.execInContainer( // + "stat", + "-c", + "%s", + containerPath + ); + assertThat(execResult.getStdout()).isEqualToIgnoringNewLines(Long.toString(byteCount)); + } finally { + tempFile.delete(); + } + } + @Test public void copyFileToContainerFolderTest() throws Exception { try ( From 07bb6078e77fa42bfcf98a45f904d6ead0c2034b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edd=C3=BA=20Mel=C3=A9ndez?= Date: Thu, 4 Jul 2024 12:28:18 -0600 Subject: [PATCH 2/2] Switch to pipes instead of temp files --- .../containers/ContainerState.java | 56 +++++++++---------- .../junit/FileOperationsTest.java | 2 +- 2 files changed, 27 insertions(+), 31 deletions(-) diff --git a/core/src/main/java/org/testcontainers/containers/ContainerState.java b/core/src/main/java/org/testcontainers/containers/ContainerState.java index b289f35f7a3..e19f7a85310 100644 --- a/core/src/main/java/org/testcontainers/containers/ContainerState.java +++ b/core/src/main/java/org/testcontainers/containers/ContainerState.java @@ -22,17 +22,14 @@ import org.testcontainers.utility.MountableFile; import org.testcontainers.utility.ThrowingFunction; -import java.io.BufferedInputStream; -import java.io.BufferedOutputStream; import java.io.File; import java.io.FileOutputStream; import java.io.IOException; import java.io.InputStream; -import java.io.OutputStream; +import java.io.PipedInputStream; +import java.io.PipedOutputStream; import java.nio.charset.Charset; import java.nio.charset.StandardCharsets; -import java.nio.file.Files; -import java.nio.file.Path; import java.util.ArrayList; import java.util.List; import java.util.Map; @@ -342,38 +339,37 @@ default void copyFileToContainer(MountableFile mountableFile, String containerPa * @param transferable file which is copied into the container * @param containerPath destination path inside the container */ - @SneakyThrows(IOException.class) + @SneakyThrows({ IOException.class, InterruptedException.class }) default void copyFileToContainer(Transferable transferable, String containerPath) { if (getContainerId() == null) { throw new IllegalStateException("copyFileToContainer can only be used with created / running container"); } - Path tempFile = Files.createTempFile("tc-", "-copy"); - try ( - OutputStream os = Files.newOutputStream(tempFile); - BufferedOutputStream bos = new BufferedOutputStream(os); - TarArchiveOutputStream tarArchive = new TarArchiveOutputStream(bos) + PipedOutputStream pipedOutputStream = new PipedOutputStream(); + PipedInputStream pipedInputStream = new PipedInputStream(pipedOutputStream); + TarArchiveOutputStream tarArchive = new TarArchiveOutputStream(pipedOutputStream) ) { - tarArchive.setLongFileMode(TarArchiveOutputStream.LONGFILE_POSIX); - tarArchive.setBigNumberMode(TarArchiveOutputStream.BIGNUMBER_POSIX); - - transferable.transferTo(tarArchive, containerPath); - tarArchive.finish(); - bos.flush(); - - try ( - InputStream is = Files.newInputStream(tempFile); - BufferedInputStream bis = new BufferedInputStream(is) - ) { - getDockerClient() - .copyArchiveToContainerCmd(getContainerId()) - .withTarInputStream(bis) - .withRemotePath("/") - .exec(); - } - } finally { - Files.deleteIfExists(tempFile); + Thread thread = new Thread(() -> { + try { + tarArchive.setLongFileMode(TarArchiveOutputStream.LONGFILE_POSIX); + tarArchive.setBigNumberMode(TarArchiveOutputStream.BIGNUMBER_POSIX); + + transferable.transferTo(tarArchive, containerPath); + } finally { + IOUtils.closeQuietly(tarArchive); + } + }); + + thread.start(); + + getDockerClient() + .copyArchiveToContainerCmd(getContainerId()) + .withTarInputStream(pipedInputStream) + .withRemotePath("/") + .exec(); + + thread.join(); } } diff --git a/core/src/test/java/org/testcontainers/junit/FileOperationsTest.java b/core/src/test/java/org/testcontainers/junit/FileOperationsTest.java index 4b3aa804b50..767deacfb00 100644 --- a/core/src/test/java/org/testcontainers/junit/FileOperationsTest.java +++ b/core/src/test/java/org/testcontainers/junit/FileOperationsTest.java @@ -48,7 +48,7 @@ public void copyFileToContainerFileTest() throws Exception { public void copyLargeFilesToContainer() throws Exception { File tempFile = temporaryFolder.newFile(); try ( - GenericContainer alpineCopyToContainer = new GenericContainer(TestImages.ALPINE_IMAGE) // + GenericContainer alpineCopyToContainer = new GenericContainer<>(TestImages.ALPINE_IMAGE) // .withCommand("sleep", "infinity") ) { alpineCopyToContainer.start();