test: stop compiling firecracker inside A/B-tests

In the pre-PR A/B-tests we were compiling Firecracker. Instead,
explicitly rely on the precompiled binaries that we set up in the shared
buildkite build step. This has the slight downside that it makes it
harder to run these tests locally, as now you need to explicitly
pre-compile the binaries, but arguably running these vulnerability
A/B-tests locally doesnt make sense anyway, because they specifically
test the security configuration on our supported .metals.

Signed-off-by: Patrick Roy <[email protected]>

Author: roypat

tests/framework/ab_test.py

@@ -97,6 +97,44 @@ def git_ab_test(
     return result_a, result_b, comparison
 
 
+def precompiled_binary_ab_test(
+    test_runner: Callable[[Path, bool], T],
+    comparator: Callable[[T, T], U] = default_comparator,
+    *,
+    a_revision: str = DEFAULT_A_REVISION,
+    b_revision: Optional[str] = None,
+) -> (T, T, U):
+    """
+    Similar to `git_ab_test`, except that it does not check out the specified revisions,
+    but instead expect that they match names of subdirectories of ../build which contain
+    firecracker and jailer binaries
+    """
+
+    dir_a = Path("../build") / a_revision
+    dir_b = (
+        Path("../build") / b_revision
+        if b_revision
+        else get_binary("firecracker").parent
+    )
+
+    assert (
+        dir_a.exists()
+        and (dir_a / "firecracker").exists()
+        and (dir_a / "jailer").exists()
+    )
+    assert (
+        dir_b.exists()
+        and (dir_b / "firecracker").exists()
+        and (dir_b / "jailer").exists()
+    )
+
+    result_a = test_runner(dir_a, True)
+    result_b = test_runner(dir_b, False)
+
+    comparison = comparator(result_a, result_b)
+    return result_a, result_b, comparison
+
+
 def is_pr() -> bool:
     """Returns `True` iff we are executing in the context of a build kite run on a pull request"""
     return os.environ.get("BUILDKITE_PULL_REQUEST", "false") != "false"
@@ -153,7 +191,7 @@ def set_did_not_grow_comparator(
     )
 
 
-def git_ab_test_guest_command(
+def precompiled_ab_test_guest_command(
     microvm_factory: Callable[[Path, Path], Microvm],
     command: str,
     *,
@@ -164,20 +202,12 @@ def git_ab_test_guest_command(
     """The same as git_ab_test_command, but via SSH. The closure argument should setup a microvm using the passed
     paths to firecracker and jailer binaries."""
 
-    @with_filelock
-    def build_firecracker(workspace_dir):
-        utils.check_output("./tools/release.sh --profile release", cwd=workspace_dir)
-
-    def test_runner(workspace_dir, _is_a: bool):
-        firecracker = get_binary("firecracker", workspace_dir=workspace_dir)
-        if not firecracker.exists():
-            build_firecracker(workspace_dir)
-        bin_dir = firecracker.parent.resolve()
+    def test_runner(bin_dir, _is_a: bool):
         firecracker, jailer = bin_dir / "firecracker", bin_dir / "jailer"
         microvm = microvm_factory(firecracker, jailer)
         return microvm.ssh.run(command)
 
-    (_, old_out, old_err), (_, new_out, new_err), the_same = git_ab_test(
+    (_, old_out, old_err), (_, new_out, new_err), the_same = precompiled_binary_ab_test(
         test_runner, comparator, a_revision=a_revision, b_revision=b_revision
     )
 
@@ -186,7 +216,7 @@ def test_runner(workspace_dir, _is_a: bool):
     ), f"The output of running command `{command}` changed:\nOld:\nstdout:\n{old_out}\nstderr\n{old_err}\n\nNew:\nstdout:\n{new_out}\nstderr:\n{new_err}"
 
 
-def git_ab_test_guest_command_if_pr(
+def precompiled_ab_test_guest_command_if_pr(
     microvm_factory: Callable[[Path, Path], Microvm],
     command: str,
     *,
@@ -195,7 +225,9 @@ def git_ab_test_guest_command_if_pr(
 ):
     """The same as git_ab_test_command_if_pr, but via SSH"""
     if is_pr():
-        git_ab_test_guest_command(microvm_factory, command, comparator=comparator)
+        precompiled_ab_test_guest_command(
+            microvm_factory, command, comparator=comparator
+        )
         return None
 
     microvm = microvm_factory(*get_firecracker_binaries())

tests/integration_tests/security/test_vulnerabilities.py

@@ -13,9 +13,9 @@
 
 from framework import utils
 from framework.ab_test import (
-    git_ab_test_guest_command,
-    git_ab_test_guest_command_if_pr,
     is_pr,
+    precompiled_ab_test_guest_command,
+    precompiled_ab_test_guest_command_if_pr,
     set_did_not_grow_comparator,
 )
 from framework.properties import global_props
@@ -231,7 +231,7 @@ def test_spectre_meltdown_checker_on_guest(spectre_meltdown_checker, build_micro
     Test with the spectre / meltdown checker on guest.
     """
 
-    status = git_ab_test_guest_command_if_pr(
+    status = precompiled_ab_test_guest_command_if_pr(
         with_checker(build_microvm, spectre_meltdown_checker),
         REMOTE_CHECKER_COMMAND,
         comparator=set_did_not_grow_comparator(
@@ -249,7 +249,7 @@ def test_spectre_meltdown_checker_on_restored_guest(
     """
     Test with the spectre / meltdown checker on a restored guest.
     """
-    status = git_ab_test_guest_command_if_pr(
+    status = precompiled_ab_test_guest_command_if_pr(
         with_checker(
             with_restore(build_microvm, microvm_factory), spectre_meltdown_checker
         ),
@@ -270,7 +270,7 @@ def test_spectre_meltdown_checker_on_guest_with_template(
     Test with the spectre / meltdown checker on guest with CPU template.
     """
 
-    git_ab_test_guest_command_if_pr(
+    precompiled_ab_test_guest_command_if_pr(
         with_checker(build_microvm_with_template, spectre_meltdown_checker),
         REMOTE_CHECKER_COMMAND,
         comparator=set_did_not_grow_comparator(
@@ -285,7 +285,7 @@ def test_spectre_meltdown_checker_on_guest_with_custom_template(
     """
     Test with the spectre / meltdown checker on guest with a custom CPU template.
     """
-    git_ab_test_guest_command_if_pr(
+    precompiled_ab_test_guest_command_if_pr(
         with_checker(build_microvm_with_custom_template, spectre_meltdown_checker),
         REMOTE_CHECKER_COMMAND,
         comparator=set_did_not_grow_comparator(
@@ -300,7 +300,7 @@ def test_spectre_meltdown_checker_on_restored_guest_with_template(
     """
     Test with the spectre / meltdown checker on a restored guest with a CPU template.
     """
-    git_ab_test_guest_command_if_pr(
+    precompiled_ab_test_guest_command_if_pr(
         with_checker(
             with_restore(build_microvm_with_template, microvm_factory),
             spectre_meltdown_checker,
@@ -320,7 +320,7 @@ def test_spectre_meltdown_checker_on_restored_guest_with_custom_template(
     """
     Test with the spectre / meltdown checker on a restored guest with a custom CPU template.
     """
-    git_ab_test_guest_command_if_pr(
+    precompiled_ab_test_guest_command_if_pr(
         with_checker(
             with_restore(build_microvm_with_custom_template, microvm_factory),
             spectre_meltdown_checker,
@@ -420,7 +420,7 @@ def check_vulnerabilities_files_ab(builder):
     running in a PR pipeline, and otherwise calls `check_vulnerabilities_files_on_guest`
     """
     if is_pr():
-        git_ab_test_guest_command(
+        precompiled_ab_test_guest_command(
             builder,
             f"! grep -r Vulnerable {VULN_DIR}",
             comparator=set_did_not_grow_comparator(