Merge branch 'main' into misc_fixes

Author: roypat

.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")

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
 

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();
     }
 

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(())
 }
 

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.

tests/framework/microvm.py

@@ -1206,13 +1206,20 @@ def build_n_from_snapshot(
         uffd_handler_name=None,
         incremental=False,
         use_snapshot_editor=True,
+        no_netns_reuse=False,
     ):
         """A generator of `n` microvms restored, either all restored from the same given snapshot
         (incremental=False), or created by taking successive snapshots of restored VMs
         """
         last_snapshot = None
         for _ in range(nr_vms):
-            microvm = self.build()
+            microvm = self.build(
+                **(
+                    {"netns": net_tools.NetNs(str(uuid.uuid4()))}
+                    if no_netns_reuse
+                    else {}
+                )
+            )
             microvm.spawn()
 
             snapshot_copy = microvm.restore_from_snapshot(

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 <stdio.h>
+#include <time.h>
+
+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]);
+}

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)

tests/integration_tests/performance/test_snapshot.py

@@ -122,7 +122,9 @@ def test_restore_latency(
 
     snapshot = vm.snapshot_full()
     vm.kill()
-    for microvm in microvm_factory.build_n_from_snapshot(snapshot, ITERATIONS):
+    for microvm in microvm_factory.build_n_from_snapshot(
+        snapshot, ITERATIONS, no_netns_reuse=True
+    ):
         value = 0
         # Parse all metric data points in search of load_snapshot time.
         microvm.flush_metrics()

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()