diff --git a/.buildkite/pipeline_perf.py b/.buildkite/pipeline_perf.py index 7604d48982e..66a9314f2d4 100755 --- a/.buildkite/pipeline_perf.py +++ b/.buildkite/pipeline_perf.py @@ -60,6 +60,11 @@ "tests": "integration_tests/performance/test_memory_overhead.py integration_tests/performance/test_boottime.py::test_boottime", "devtool_opts": "-c 1-10 -m 0", }, + "jailer": { + "label": "⛓️ jailer", + "tests": "integration_tests/performance/test_jailer.py", + "devtool_opts": "-c 1-10 -m 0", + }, } REVISION_A = os.environ.get("REVISION_A") diff --git a/CHANGELOG.md b/CHANGELOG.md index d243b8ef923..579f1193877 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,8 @@ and this project adheres to - [#5165](https://github.com/firecracker-microvm/firecracker/pull/5165): Changed Firecracker snapshot feature from developer preview to generally available. Incremental snapshots remain in developer preview. +- [#5282](https://github.com/firecracker-microvm/firecracker/pull/5282): Updated + jailer to no longer require the executable file name to contain `firecracker`. ### Deprecated diff --git a/src/jailer/src/env.rs b/src/jailer/src/env.rs index e2953584a32..57d28724a57 100644 --- a/src/jailer/src/env.rs +++ b/src/jailer/src/env.rs @@ -301,10 +301,6 @@ impl Env { .unwrap() .to_string(); - if !exec_file_name.contains("firecracker") { - return Err(JailerError::ExecFileName(exec_file_name)); - } - Ok((exec_file_path, exec_file_name)) } @@ -1048,17 +1044,6 @@ mod tests { "/tmp/firecracker_test_dir is not a file" ); - // Error case 3: Filename without "firecracker" - File::create("/tmp/firecracker_test_dir/foobarbaz").unwrap(); - assert_eq!( - format!( - "{}", - Env::validate_exec_file("/tmp/firecracker_test_dir/foobarbaz").unwrap_err() - ), - "Invalid filename. The filename of `--exec-file` option must contain \"firecracker\": \ - foobarbaz" - ); - std::fs::remove_file("/tmp/firecracker_test_dir/foobarbaz").unwrap(); std::fs::remove_dir_all("/tmp/firecracker_test_dir").unwrap(); } diff --git a/src/jailer/src/main.rs b/src/jailer/src/main.rs index 721531e49ba..9532fa1eba4 100644 --- a/src/jailer/src/main.rs +++ b/src/jailer/src/main.rs @@ -73,10 +73,6 @@ pub enum JailerError { Dup2(io::Error), #[error("Failed to exec into Firecracker: {0}")] Exec(io::Error), - #[error( - "Invalid filename. The filename of `--exec-file` option must contain \"firecracker\": {0}" - )] - ExecFileName(String), #[error("{}", format!("Failed to extract filename from path {:?}", .0).replace('\"', ""))] ExtractFileName(PathBuf), #[error("{}", format!("Failed to open file {:?}: {}", .0, .1).replace('\"', ""))] @@ -351,8 +347,7 @@ fn main_exec() -> Result<(), JailerError> { fs::create_dir_all(env.chroot_dir()) .map_err(|err| JailerError::CreateDir(env.chroot_dir().to_owned(), err))?; env.run() - }) - .unwrap_or_else(|err| panic!("Jailer error: {}", err)); + })?; Ok(()) } diff --git a/tests/conftest.py b/tests/conftest.py index 1b5dcafe713..0c71a212b56 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -274,6 +274,17 @@ def msr_reader_bin(test_fc_session_root_path): yield msr_reader_bin_path +@pytest.fixture(scope="session") +def jailer_time_bin(test_fc_session_root_path): + """Build a binary that fakes fc""" + jailer_time_bin_path = os.path.join(test_fc_session_root_path, "jailer_time") + build_tools.gcc_compile( + "host_tools/jailer_time.c", + jailer_time_bin_path, + ) + yield jailer_time_bin_path + + @pytest.fixture def bin_seccomp_paths(): """Build jailers and jailed binaries to test seccomp. diff --git a/tests/host_tools/jailer_time.c b/tests/host_tools/jailer_time.c new file mode 100644 index 00000000000..f8e86b4a86c --- /dev/null +++ b/tests/host_tools/jailer_time.c @@ -0,0 +1,19 @@ +// Copyright 2025 Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +// This is used by `performance/test_jailer.py` + +#include +#include + +int main(int argc, char** argv) { + // print current time in us + struct timespec now = {0}; + clock_gettime(CLOCK_MONOTONIC, &now); + unsigned long long current_ns = (unsigned long long)now.tv_sec * 1000000000 + (unsigned long long)now.tv_nsec; + unsigned long long current_us = current_ns / 1000; + printf("%llu\n", current_us); + + // print the --start-time-us value + printf("%s", argv[4]); +} diff --git a/tests/integration_tests/performance/test_jailer.py b/tests/integration_tests/performance/test_jailer.py new file mode 100644 index 00000000000..21ef0c13666 --- /dev/null +++ b/tests/integration_tests/performance/test_jailer.py @@ -0,0 +1,88 @@ +# Copyright 2025 Amazon.com, Inc. or its affiliates. All Rights Reserved. +# SPDX-License-Identifier: Apache-2.0 +"""Performance benchmark for the jailer.""" + +import os +import shutil +from concurrent.futures import ProcessPoolExecutor + +import pytest + +from framework import utils +from framework.jailer import DEFAULT_CHROOT_PATH, JailerContext +from framework.properties import global_props + + +def setup_bind_mounts(tmp_path, n): + """ + Create bind mount points. The exact location of them + does not matter, they just need to exist. + """ + mounts_paths = tmp_path / "mounts" + os.makedirs(mounts_paths) + for m in range(n): + mount_path = f"{mounts_paths}/mount{m}" + os.makedirs(mount_path) + utils.check_output(f"mount --bind {mount_path} {mount_path}") + + +def clean_up_mounts(tmp_path): + """Cleanup mounts and jailer dirs""" + mounts_paths = tmp_path / "mounts" + for d in os.listdir(mounts_paths): + utils.check_output(f"umount {mounts_paths}/{d}") + + +@pytest.mark.nonci +@pytest.mark.parametrize("parallel", [1, 5, 10]) +@pytest.mark.parametrize("mounts", [0, 100, 300, 500]) +def test_jailer_startup( + jailer_time_bin, tmp_path, microvm_factory, parallel, mounts, metrics +): + """ + Test the overhead of jailer startup without and with bind mounts + with different parallelism options. + """ + + jailer_binary = microvm_factory.jailer_binary_path + + setup_bind_mounts(tmp_path, mounts) + + metrics.set_dimensions( + { + "instance": global_props.instance, + "cpu_model": global_props.cpu_model, + "performance_test": "test_jailer_startup", + "parallel": str(parallel), + "mounts": str(mounts), + } + ) + + cmds = [] + for i in range(500): + jailer = JailerContext( + jailer_id=f"fakefc{i}", + exec_file=jailer_time_bin, + # Don't deamonize to get the stdout + daemonize=False, + ) + jailer.setup() + + cmd = [str(jailer_binary), *jailer.construct_param_list()] + cmds.append(cmd) + + with ProcessPoolExecutor(max_workers=parallel) as executor: + # Submit all commands and get results + results = executor.map(utils.check_output, cmds) + + # Get results as they complete + for result in results: + end_time, start_time = result.stdout.split() + metrics.put_metric( + "startup", + int(end_time) - int(start_time), + unit="Microseconds", + ) + + clean_up_mounts(tmp_path) + shutil.rmtree(DEFAULT_CHROOT_PATH) diff --git a/tests/integration_tests/security/test_jail.py b/tests/integration_tests/security/test_jail.py index ad6a16e48f5..277d33b17b3 100644 --- a/tests/integration_tests/security/test_jail.py +++ b/tests/integration_tests/security/test_jail.py @@ -69,7 +69,7 @@ def test_empty_jailer_id(uvm_plain): # we can set an empty ID. with pytest.raises( ChildProcessError, - match=r"Jailer error: Invalid instance ID: Invalid len \(0\); the length must be between 1 and 64", + match=r"Invalid instance ID: Invalid len \(0\); the length must be between 1 and 64", ): test_microvm.spawn() @@ -88,7 +88,7 @@ def test_exec_file_not_exist(uvm_plain, tmp_path): with pytest.raises( Exception, - match=rf"Jailer error: Failed to canonicalize path {pseudo_exec_file_path}:" + match=rf"Failed to canonicalize path {pseudo_exec_file_path}:" rf" No such file or directory \(os error 2\)", ): test_microvm.spawn() @@ -102,21 +102,7 @@ def test_exec_file_not_exist(uvm_plain, tmp_path): with pytest.raises( Exception, - match=rf"Jailer error: {pseudo_exec_dir_path} is not a file", - ): - test_microvm.spawn() - - # Error case 3: Filename without "firecracker" - pseudo_exec_file_path = tmp_path / "foobarbaz" - pseudo_exec_file_path.touch() - fc_dir = Path("/srv/jailer") / pseudo_exec_file_path.name / test_microvm.id - fc_dir.mkdir(parents=True, exist_ok=True) - test_microvm.jailer.exec_file = pseudo_exec_file_path - - with pytest.raises( - Exception, - match=r"Jailer error: Invalid filename. The filename of `--exec-file` option" - r' must contain "firecracker": foobarbaz', + match=rf"{pseudo_exec_dir_path} is not a file", ): test_microvm.spawn()