Skip to content
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.

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

Fix problems with copying files and Docker-Compose on Windows #514

Merged
merged 7 commits into from
Dec 11, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions .gitattributes
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
*.sh text eol=lf
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ All notable changes to this project will be documented in this file.

## UNRELEASED
### Fixed
- Problems with using container based docker-compose on Windows ([\#514](https://github.com/testcontainers/testcontainers-java/pull/514))
- Problems with copying files on Windows ([\#514](https://github.com/testcontainers/testcontainers-java/pull/514))
- Fixed regression in 1.4.3 when using Docker Compose on Windows ([\#439](https://github.com/testcontainers/testcontainers-java/issues/439))
- Fixed local Docker Compose executable name resolution on Windows (#416)
- Fixed TAR composition on Windows (#444)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -395,6 +395,10 @@ default void validateFileList(List<File> composeFiles) {
* Use Docker Compose container.
*/
class ContainerisedDockerCompose extends GenericContainer<ContainerisedDockerCompose> implements DockerCompose {

private static final String DOCKER_SOCKET_PATH = "/var/run/docker.sock";
public static final char UNIX_PATH_SEPERATOR = ':';

public ContainerisedDockerCompose(List<File> composeFiles, String identifier) {

super(TestcontainersConfiguration.getInstance().getDockerComposeContainerImage());
Expand All @@ -405,14 +409,14 @@ public ContainerisedDockerCompose(List<File> composeFiles, String identifier) {
// Map the docker compose file into the container
final File dockerComposeBaseFile = composeFiles.get(0);
final String pwd = dockerComposeBaseFile.getAbsoluteFile().getParentFile().getAbsolutePath();
final String containerPwd = MountableFile.forHostPath(pwd).getResolvedPath();
final String containerPwd = MountableFile.forHostPath(pwd).getFilesystemPath();

final List<String> absoluteDockerComposeFiles = composeFiles.stream()
.map(File::getAbsolutePath)
.map(MountableFile::forHostPath)
.map(MountableFile::getResolvedPath)
.map(MountableFile::getFilesystemPath)
.collect(toList());
final String composeFileEnvVariableValue = Joiner.on(File.pathSeparator).join(absoluteDockerComposeFiles);
final String composeFileEnvVariableValue = Joiner.on(UNIX_PATH_SEPERATOR).join(absoluteDockerComposeFiles); // we always need the UNIX path separator
logger().debug("Set env COMPOSE_FILE={}", composeFileEnvVariableValue);
addEnv(ENV_COMPOSE_FILE, composeFileEnvVariableValue);
addFileSystemBind(pwd, containerPwd, READ_ONLY);
Expand All @@ -421,12 +425,18 @@ public ContainerisedDockerCompose(List<File> composeFiles, String identifier) {
// as the docker daemon, just mapping the docker control socket is OK.
// As there seems to be a problem with mapping to the /var/run directory in certain environments (e.g. CircleCI)
// we map the socket file outside of /var/run, as just /docker.sock
addFileSystemBind("/var/run/docker.sock", "/docker.sock", READ_WRITE);
addFileSystemBind(getDockerSocketHostPath(), "/docker.sock", READ_WRITE);
addEnv("DOCKER_HOST", "unix:///docker.sock");
setStartupCheckStrategy(new IndefiniteWaitOneShotStartupCheckStrategy());
setWorkingDirectory(containerPwd);
}

private String getDockerSocketHostPath() {
return SystemUtils.IS_OS_WINDOWS
Copy link

@trajano trajano Jan 4, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is making an assumption that we are only using docker via their sockets interface. However in Docker Machine setups this may not be available It should be able to use the DOCKER_HOSTand DOCKER_CERT_PATH and DOCKER_TLS values.

I just checked master it appears to make an assumption that the socket file does exist.

? "/" + DOCKER_SOCKET_PATH
: DOCKER_SOCKET_PATH;
}

@Override
public void invoke() {
super.start();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,6 @@ public interface ClasspathTrait<SELF extends ClasspathTrait<SELF> & BuildContext
default SELF withFileFromClasspath(String path, String resourcePath) {
final MountableFile mountableFile = MountableFile.forClasspathResource(resourcePath);

return ((SELF) this).withFileFromPath(path, Paths.get(mountableFile.getFilesystemPath()));
return ((SELF) this).withFileFromPath(path, Paths.get(mountableFile.getResolvedPath()));
}
}
37 changes: 20 additions & 17 deletions core/src/main/java/org/testcontainers/utility/MountableFile.java
Original file line number Diff line number Diff line change
@@ -1,9 +1,15 @@
package org.testcontainers.utility;

import static lombok.AccessLevel.PACKAGE;
import static org.testcontainers.utility.PathUtils.recursiveDeleteDir;

import com.google.common.base.Charsets;
import lombok.Getter;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.apache.commons.compress.archivers.tar.TarArchiveEntry;
import org.apache.commons.compress.archivers.tar.TarArchiveOutputStream;
import org.apache.commons.lang.SystemUtils;
import org.jetbrains.annotations.NotNull;
import org.testcontainers.images.builder.Transferable;

import java.io.File;
import java.io.IOException;
import java.io.InputStream;
Expand All @@ -19,14 +25,9 @@
import java.util.Set;
import java.util.jar.JarEntry;
import java.util.jar.JarFile;
import lombok.Getter;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.apache.commons.compress.archivers.tar.TarArchiveEntry;
import org.apache.commons.compress.archivers.tar.TarArchiveOutputStream;
import org.apache.commons.lang.SystemUtils;
import org.jetbrains.annotations.NotNull;
import org.testcontainers.images.builder.Transferable;

import static lombok.AccessLevel.PACKAGE;
import static org.testcontainers.utility.PathUtils.recursiveDeleteDir;

/**
* An abstraction over files and classpath resources aimed at encapsulating all the complexity of generating
Expand Down Expand Up @@ -165,8 +166,8 @@ private static String unencodeResourceURIToFilePath(@NotNull final String resour
private String resolvePath() {
String result = getResourcePath();

if (SystemUtils.IS_OS_WINDOWS) {
result = PathUtils.createMinGWPath(result);
if (SystemUtils.IS_OS_WINDOWS && result.startsWith("/")) {
result = result.substring(1);
}

return result;
Expand All @@ -177,13 +178,15 @@ private String resolvePath() {
* into a container. If this is a classpath resource residing in a JAR, it will be extracted to
* a temporary location so that the Docker daemon is able to access it.
*
* TODO: rename method accordingly and check if really needed like this
*
* @return
*/
private String resolveFilesystemPath() {
String result = getResourcePath();

if (SystemUtils.IS_OS_WINDOWS && result.startsWith("/")) {
result = result.substring(1);
result = PathUtils.createMinGWPath(result).substring(1);
}

return result;
Expand Down Expand Up @@ -285,7 +288,7 @@ private void deleteOnExit(final Path path) {
*/
@Override
public void transferTo(final TarArchiveOutputStream outputStream, String destinationPathInTar) {
recursiveTar(destinationPathInTar, this.getFilesystemPath(), this.getFilesystemPath(), outputStream);
recursiveTar(destinationPathInTar, this.getResolvedPath(), this.getResolvedPath(), outputStream);
}

/*
Expand Down Expand Up @@ -325,7 +328,7 @@ private void recursiveTar(String entryFilename, String rootPath, String itemPath
@Override
public long getSize() {

final File file = new File(this.getFilesystemPath());
final File file = new File(this.getResolvedPath());
if (file.isFile()) {
return file.length();
} else {
Expand All @@ -340,7 +343,7 @@ public String getDescription() {

@Override
public int getFileMode() {
return getUnixFileMode(this.getFilesystemPath());
return getUnixFileMode(this.getResolvedPath());
}

private int getUnixFileMode(final String pathAsString) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ private Path createTempDir() throws IOException {
}

private void performChecks(final MountableFile mountableFile) {
final String mountablePath = mountableFile.getFilesystemPath();
final String mountablePath = mountableFile.getResolvedPath();
assertTrue("The filesystem path '" + mountablePath + "' can be found", new File(mountablePath).exists());
assertFalse("The filesystem path '" + mountablePath + "' does not contain any URL escaping", mountablePath.contains("%20"));
}
Expand Down