Skip to content

Commit

Permalink
fix docker process factory test on ci (airbytehq#9695)
Browse files Browse the repository at this point in the history
* fix docker process factory test on ci

* fmt

* fix compilation
  • Loading branch information
jrhizor authored Jan 21, 2022
1 parent 2ddf0bc commit e9dba45
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 64 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -196,8 +196,7 @@ private static ProcessFactory getProcessBuilderFactory(final Configs configs, fi
configs.getWorkspaceRoot(),
configs.getWorkspaceDockerMount(),
configs.getLocalDockerMount(),
configs.getDockerNetwork(),
false);
configs.getDockerNetwork());
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -269,8 +269,7 @@ private static ProcessFactory getJobProcessFactory(final Configs configs) throws
configs.getWorkspaceRoot(),
configs.getWorkspaceDockerMount(),
configs.getLocalDockerMount(),
configs.getDockerNetwork(),
false);
configs.getDockerNetwork());
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ public class DockerProcessFactory implements ProcessFactory {
private final Path workspaceRoot;
private final String localMountSource;
private final String networkName;
private final boolean isOrchestrator;
private final Path imageExistsScriptPath;

/**
Expand All @@ -48,20 +47,17 @@ public class DockerProcessFactory implements ProcessFactory {
* @param workspaceMountSource workspace volume
* @param localMountSource local volume
* @param networkName docker network
* @param isOrchestrator if the process needs to be able to launch containers
*/
public DockerProcessFactory(final WorkerConfigs workerConfigs,
final Path workspaceRoot,
final String workspaceMountSource,
final String localMountSource,
final String networkName,
final boolean isOrchestrator) {
final String networkName) {
this.workerConfigs = workerConfigs;
this.workspaceRoot = workspaceRoot;
this.workspaceMountSource = workspaceMountSource;
this.localMountSource = localMountSource;
this.networkName = networkName;
this.isOrchestrator = isOrchestrator;
this.imageExistsScriptPath = prepareImageExistsScript();
}

Expand Down Expand Up @@ -105,46 +101,30 @@ public Process create(final String jobId,
IOs.writeFile(jobRoot, file.getKey(), file.getValue());
}

List<String> cmd;

// todo: add --expose 80 to each

if (isOrchestrator) {
cmd = Lists.newArrayList(
"docker",
"run",
"--rm",
"--init",
"-i",
"-v",
String.format("%s:%s", workspaceMountSource, workspaceRoot), // real workspace root, not a rebased version
"-v",
String.format("%s:%s", localMountSource, LOCAL_MOUNT_DESTINATION),
"-v",
"/var/run/docker.sock:/var/run/docker.sock", // needs to be able to run docker in docker
"-w",
jobRoot.toString(), // real jobroot, not rebased version
"--network",
networkName,
"--log-driver",
"none");
} else {
cmd = Lists.newArrayList(
"docker",
"run",
"--rm",
"--init",
"-i",
"-v",
String.format("%s:%s", workspaceMountSource, DATA_MOUNT_DESTINATION), // uses job data mount
"-v",
String.format("%s:%s", localMountSource, LOCAL_MOUNT_DESTINATION),
"-w",
rebasePath(jobRoot).toString(), // rebases the job root on the job data mount
"--network",
networkName,
"--log-driver",
"none");
final List<String> cmd = Lists.newArrayList(
"docker",
"run",
"--rm",
"--init",
"-i",
"-w",
rebasePath(jobRoot).toString(), // rebases the job root on the job data mount
"--log-driver",
"none");

if (networkName != null) {
cmd.add("--network");
cmd.add(networkName);
}

if (workspaceMountSource != null) {
cmd.add("-v");
cmd.add(String.format("%s:%s", workspaceMountSource, DATA_MOUNT_DESTINATION));
}

if (localMountSource != null) {
cmd.add("-v");
cmd.add(String.format("%s:%s", localMountSource, LOCAL_MOUNT_DESTINATION));
}

for (final var envEntry : workerConfigs.getEnvMap().entrySet()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,6 @@
import java.util.Map;
import java.util.concurrent.TimeUnit;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ValueSource;

// todo (cgardens) - these are not truly "unit" tests as they are check resources on the internet.
// we should move them to "integration" tests, when we have facility to do so.
Expand Down Expand Up @@ -61,26 +59,25 @@ public void testJqExists() throws IOException {
public void testImageExists() throws IOException, WorkerException {
final Path workspaceRoot = Files.createTempDirectory(Files.createDirectories(TEST_ROOT), "process_factory");

final DockerProcessFactory processFactory = new DockerProcessFactory(new WorkerConfigs(new EnvConfigs()), workspaceRoot, "", "", "", false);
final DockerProcessFactory processFactory = new DockerProcessFactory(new WorkerConfigs(new EnvConfigs()), workspaceRoot, null, null, null);
assertTrue(processFactory.checkImageExists("busybox"));
}

@Test
public void testImageDoesNotExist() throws IOException, WorkerException {
final Path workspaceRoot = Files.createTempDirectory(Files.createDirectories(TEST_ROOT), "process_factory");

final DockerProcessFactory processFactory = new DockerProcessFactory(new WorkerConfigs(new EnvConfigs()), workspaceRoot, "", "", "", false);
final DockerProcessFactory processFactory = new DockerProcessFactory(new WorkerConfigs(new EnvConfigs()), workspaceRoot, null, null, null);
assertFalse(processFactory.checkImageExists("airbyte/fake:0.1.2"));
}

@ParameterizedTest
@ValueSource(booleans = {true, false})
public void testFileWriting(boolean isOrchestrator) throws IOException, WorkerException {
@Test
public void testFileWriting() throws IOException, WorkerException {
final Path workspaceRoot = Files.createTempDirectory(Files.createDirectories(TEST_ROOT), "process_factory");
final Path jobRoot = workspaceRoot.resolve("job");

final DockerProcessFactory processFactory =
new DockerProcessFactory(new WorkerConfigs(new EnvConfigs()), workspaceRoot, "", "", "", isOrchestrator);
new DockerProcessFactory(new WorkerConfigs(new EnvConfigs()), workspaceRoot, null, null, null);
processFactory.create("job_id", 0, jobRoot, "busybox", false, ImmutableMap.of("config.json", "{\"data\": 2}"), "echo hi",
new WorkerConfigs(new EnvConfigs()).getResourceRequirements(), Map.of(), Map.of());

Expand All @@ -92,9 +89,8 @@ public void testFileWriting(boolean isOrchestrator) throws IOException, WorkerEx
/**
* Tests that the env var map passed in is accessible within the process.
*/
@ParameterizedTest
@ValueSource(booleans = {true, false})
public void testEnvMapSet(boolean isOrchestrator) throws IOException, WorkerException {
@Test
public void testEnvMapSet() throws IOException, WorkerException {
final Path workspaceRoot = Files.createTempDirectory(Files.createDirectories(TEST_ROOT), "process_factory");
final Path jobRoot = workspaceRoot.resolve("job");

Expand All @@ -105,10 +101,9 @@ public void testEnvMapSet(boolean isOrchestrator) throws IOException, WorkerExce
new DockerProcessFactory(
workerConfigs,
workspaceRoot,
"",
"",
"host",
isOrchestrator);
null,
null,
"host");

final Process process = processFactory.create(
"job_id",
Expand Down

0 comments on commit e9dba45

Please sign in to comment.