Skip to content

Commit

Permalink
docker: set platform when creating containers (#16886)
Browse files Browse the repository at this point in the history
Update `bollard` and make use of new ability to set platform upon container creation.

For tests, the `busybox` image is only available for Linux and not macOS. Thus on macOS the tests fail since no macOS version of busybox image is available. Solve by pinning the platform used for tests to Linux. On Linux this is fine since same OS. On macOS, this works due to macOS Docker support for running Linux containers.
  • Loading branch information
Tom Dyas authored Sep 16, 2022
1 parent ac73435 commit ba75665
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 23 deletions.
4 changes: 2 additions & 2 deletions src/rust/engine/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion src/rust/engine/process_execution/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ publish = false
async-stream = "0.3"
async-trait = "0.1"
async-lock = "2.5"
bollard = { git = "https://github.com/fussybeaver/bollard.git", rev = "2e10fe31076a6e298697fc6e31fa3676b7498b6c" }
bollard = { git = "https://github.com/fussybeaver/bollard.git", rev = "2d66d11b44aeff0373ece3d64a44b243e5152973" }
bollard-stubs = "1.42.0-rc.3"
walkdir = "2"
protos = { path = "../protos" }
Expand Down
31 changes: 23 additions & 8 deletions src/rust/engine/process_execution/src/docker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use std::sync::Arc;

use async_oncecell::OnceCell;
use async_trait::async_trait;
use bollard::container::{LogOutput, RemoveContainerOptions};
use bollard::container::{CreateContainerOptions, LogOutput, RemoveContainerOptions};
use bollard::exec::StartExecResults;
use bollard::image::CreateImageOptions;
use bollard::service::CreateImageInfo;
Expand Down Expand Up @@ -63,10 +63,23 @@ impl DockerOnceCell {
let docker = Docker::connect_with_local_defaults()
.map_err(|err| format!("Failed to connect to local Docker: {err}"))?;

docker
.ping()
.await
.map_err(|err| format!("Failed to receive response from local Docker: {err}"))?;
let version = docker.version().await
.map_err(|err| format!("Failed to obtain version from local Docker: {err}"))?;

let api_version = version.api_version.as_ref().ok_or("Docker failed to report its API version.")?;
let api_version_parts = api_version
.split('.')
.collect::<Vec<_>>();
match api_version_parts[..] {
[major, minor, ..] => {
let major = (*major).parse::<usize>().map_err(|err| format!("Failed to decode Docker API major version `{major}`: {err}"))?;
let minor = (*minor).parse::<usize>().map_err(|err| format!("Failed to decode Docker API minor version `{minor}`: {err}"))?;
if major < 1 || (major == 1 && minor < 41) {
return Err(format!("Pants requires Docker to support API version 1.41 or higher. Local Docker only supports: {:?}", &version.api_version));
}
}
_ => return Err(format!("Unparseable API version `{}` returned by Docker.", &api_version)),
}

Ok(docker)
})
Expand Down Expand Up @@ -641,10 +654,12 @@ impl ContainerCache {
&config
);

// TODO: Pass `platform` parameter via options argument once https://github.com/fussybeaver/bollard/pull/259
// is approved upstream.
let create_options = CreateContainerOptions::<&str> {
name: "",
platform: Some(docker_platform_identifier(&platform)),
};
let container = docker
.create_container::<&str, String>(None, config)
.create_container::<&str, String>(Some(create_options), config)
.await
.map_err(|err| format!("Failed to create Docker container: {:?}", err))?;

Expand Down
37 changes: 25 additions & 12 deletions src/rust/engine/process_execution/src/docker_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,14 @@ macro_rules! skip_if_no_docker_available_in_macos_ci {
}};
}

fn platform_for_tests() -> Result<Platform, String> {
Platform::current().map(|platform| match platform {
Platform::Macos_arm64 => Platform::Linux_arm64,
Platform::Macos_x86_64 => Platform::Linux_x86_64,
p => p,
})
}

#[tokio::test(flavor = "multi_thread", worker_threads = 1)]
#[cfg(unix)]
async fn runner_errors_if_docker_image_not_set() {
Expand Down Expand Up @@ -155,7 +163,7 @@ async fn capture_exit_code_signal() {
// assert_eq!(result.original.exit_code, -15);
assert_eq!(result.original.exit_code, 143);
assert_eq!(result.original.output_directory, *EMPTY_DIRECTORY_DIGEST);
assert_eq!(result.original.platform, Platform::current().unwrap());
assert_eq!(result.original.platform, platform_for_tests().unwrap());
}

fn extract_env(
Expand Down Expand Up @@ -277,7 +285,7 @@ async fn output_files_one() {
result.original.output_directory,
TestDirectory::containing_roland().directory_digest()
);
assert_eq!(result.original.platform, Platform::current().unwrap());
assert_eq!(result.original.platform, platform_for_tests().unwrap());
}

#[tokio::test(flavor = "multi_thread", worker_threads = 1)]
Expand Down Expand Up @@ -308,7 +316,7 @@ async fn output_dirs() {
result.original.output_directory,
TestDirectory::recursive().directory_digest()
);
assert_eq!(result.original.platform, Platform::current().unwrap());
assert_eq!(result.original.platform, platform_for_tests().unwrap());
}

#[tokio::test(flavor = "multi_thread", worker_threads = 1)]
Expand Down Expand Up @@ -338,7 +346,7 @@ async fn output_files_many() {
result.original.output_directory,
TestDirectory::recursive().directory_digest()
);
assert_eq!(result.original.platform, Platform::current().unwrap());
assert_eq!(result.original.platform, platform_for_tests().unwrap());
}

#[tokio::test(flavor = "multi_thread", worker_threads = 1)]
Expand Down Expand Up @@ -367,7 +375,7 @@ async fn output_files_execution_failure() {
result.original.output_directory,
TestDirectory::containing_roland().directory_digest()
);
assert_eq!(result.original.platform, Platform::current().unwrap());
assert_eq!(result.original.platform, platform_for_tests().unwrap());
}

#[tokio::test(flavor = "multi_thread", worker_threads = 1)]
Expand Down Expand Up @@ -397,7 +405,7 @@ async fn output_files_partial_output() {
result.original.output_directory,
TestDirectory::containing_roland().directory_digest()
);
assert_eq!(result.original.platform, Platform::current().unwrap());
// assert_eq!(result.original.platform, platform_for_tests().unwrap());
}

#[tokio::test(flavor = "multi_thread", worker_threads = 1)]
Expand All @@ -424,7 +432,7 @@ async fn output_overlapping_file_and_dir() {
result.original.output_directory,
TestDirectory::nested().directory_digest()
);
assert_eq!(result.original.platform, Platform::current().unwrap());
assert_eq!(result.original.platform, platform_for_tests().unwrap());
}

#[tokio::test(flavor = "multi_thread", worker_threads = 1)]
Expand All @@ -447,7 +455,7 @@ async fn append_only_cache_created() {
assert_eq!(result.stderr_bytes, "".as_bytes());
assert_eq!(result.original.exit_code, 0);
assert_eq!(result.original.output_directory, *EMPTY_DIRECTORY_DIGEST);
assert_eq!(result.original.platform, Platform::current().unwrap());
assert_eq!(result.original.platform, platform_for_tests().unwrap());
}

#[tokio::test(flavor = "multi_thread", worker_threads = 1)]
Expand Down Expand Up @@ -527,7 +535,7 @@ async fn all_containing_directories_for_outputs_are_created() {
result.original.output_directory,
TestDirectory::nested_dir_and_file().directory_digest()
);
assert_eq!(result.original.platform, Platform::current().unwrap());
assert_eq!(result.original.platform, platform_for_tests().unwrap());
}

#[tokio::test(flavor = "multi_thread", worker_threads = 1)]
Expand All @@ -553,7 +561,7 @@ async fn output_empty_dir() {
result.original.output_directory,
TestDirectory::containing_falcons_dir().directory_digest()
);
assert_eq!(result.original.platform, Platform::current().unwrap());
assert_eq!(result.original.platform, platform_for_tests().unwrap());
}

#[tokio::test(flavor = "multi_thread", worker_threads = 1)]
Expand Down Expand Up @@ -638,7 +646,7 @@ async fn working_directory() {
result.original.output_directory,
TestDirectory::containing_roland().directory_digest()
);
assert_eq!(result.original.platform, Platform::current().unwrap());
assert_eq!(result.original.platform, platform_for_tests().unwrap());
}

#[tokio::test(flavor = "multi_thread", worker_threads = 1)]
Expand Down Expand Up @@ -710,13 +718,18 @@ async fn immutable_inputs() {
}

async fn run_command_via_docker_in_dir(
req: Process,
mut req: Process,
dir: PathBuf,
cleanup: KeepSandboxes,
workunit: &mut RunningWorkunit,
store: Option<Store>,
executor: Option<task_executor::Executor>,
) -> Result<LocalTestResult, ProcessError> {
if req.platform_constraint.is_none() {
req.platform_constraint =
Some(platform_for_tests().map_err(|err| ProcessError::Unclassified(err))?);
}

let store_dir = TempDir::new().unwrap();
let executor = executor.unwrap_or_else(|| task_executor::Executor::new());
let store =
Expand Down

0 comments on commit ba75665

Please sign in to comment.