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.

While we're at it, rename the ab_* utility functions to make it obvious
that they expect pre-compiled binaries.

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

Author: roypat

tests/framework/ab_test.py

@@ -33,11 +33,7 @@
 from framework.microvm import Microvm
 from framework.utils import CommandReturn
 from framework.with_filelock import with_filelock
-from host_tools.cargo_build import (
-    DEFAULT_TARGET_DIR,
-    get_binary,
-    get_firecracker_binaries,
-)
+from host_tools.cargo_build import DEFAULT_TARGET_DIR, get_firecracker_binaries
 
 # Locally, this will always compare against main, even if we try to merge into, say, a feature branch.
 # We might want to do a more sophisticated way to determine a "parent" branch here.
@@ -178,7 +174,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,
     *,
@@ -188,30 +184,29 @@ 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."""
+    b_directory = (
+        DEFAULT_B_DIRECTORY
+        if b_revision is None
+        else FC_WORKSPACE_DIR / "build" / b_revision
+    )
 
-    @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()
-        firecracker, jailer = bin_dir / "firecracker", bin_dir / "jailer"
-        microvm = microvm_factory(firecracker, jailer)
+    def test_runner(bin_dir, _is_a: bool):
+        microvm = microvm_factory(bin_dir / "firecracker", bin_dir / "jailer")
         return microvm.ssh.run(command)
 
-    (_, old_out, old_err), (_, new_out, new_err), the_same = git_ab_test(
-        test_runner, comparator, a_revision=a_revision, b_revision=b_revision
+    (_, old_out, old_err), (_, new_out, new_err), the_same = binary_ab_test(
+        test_runner,
+        comparator,
+        a_directory=FC_WORKSPACE_DIR / "build" / a_revision,
+        b_directory=b_directory,
     )
 
     assert (
         the_same
     ), 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,
     *,
@@ -220,7 +215,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
@@ -233,7 +233,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(
@@ -251,7 +251,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
         ),
@@ -272,7 +272,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(
@@ -287,7 +287,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(
@@ -302,7 +302,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,
@@ -322,7 +322,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,
@@ -424,7 +424,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(