Artifacts and how to get them

.buildkite/common.py

@@ -376,6 +376,12 @@ def to_json(self):
         """Serialize the pipeline to JSON"""
         return json.dumps(self.to_dict(), indent=4, sort_keys=True, ensure_ascii=False)
 
+    def devtool_download_artifacts(self, artifacts):
+        """Generate a `devtool download_ci_artifacts` command"""
+        parts = ["./tools/devtool -y download_ci_artifacts"]
+        parts += artifacts
+        return " ".join(parts)
+
     def devtool_test(self, devtool_opts=None, pytest_opts=None):
         """Generate a `devtool test` command"""
         cmds = []

.buildkite/pipeline_docker_popular.py

@@ -28,7 +28,7 @@
 pipeline.build_group(
     "docker-popular-containers",
     [
-        "./tools/devtool download_ci_artifacts",
+        "./tools/devtool ensure_current_artifacts",
         f'buildkite-agent artifact download "{ROOTFS_TAR}" .',
         f'tar xzf "{ROOTFS_TAR}" -C tools/test-popular-containers',
         './tools/devtool sh "cd ./tools/test-popular-containers; PYTHONPATH=../../tests ./test-docker-rootfs.py"',

.buildkite/pipeline_perf.py

@@ -94,11 +94,16 @@
 
 REVISION_A = os.environ.get("REVISION_A")
 REVISION_B = os.environ.get("REVISION_B")
+REVISION_A_ARTIFACTS = os.environ.get("REVISION_A_ARTIFACTS")
+REVISION_B_ARTIFACTS = os.environ.get("REVISION_B_ARTIFACTS")
 
 # Either both are specified or neither. Only doing either is a bug. If you want to
 # run performance tests _on_ a specific commit, specify neither and put your commit
 # into buildkite's "commit" field.
 assert (REVISION_A and REVISION_B) or (not REVISION_A and not REVISION_B)
+assert (REVISION_A_ARTIFACTS and REVISION_B_ARTIFACTS) or (
+    not REVISION_A_ARTIFACTS and not REVISION_B_ARTIFACTS
+)
 
 BKPipeline.parser.add_argument(
     "--test",
@@ -132,17 +137,26 @@
     ab_opts = test.pop("ab_opts", "")
     devtool_opts += " --performance"
     test_script_opts = ""
+    artifacts = []
     if REVISION_A:
         devtool_opts += " --ab"
-        test_script_opts = f'{ab_opts} run build/{REVISION_A}/ build/{REVISION_B} --pytest-opts "{test_selector}"'
+        test_script_opts = f'{ab_opts} run --binaries-a build/{REVISION_A}/ --binaries-b build/{REVISION_B} --pytest-opts "{test_selector}"'
+        if REVISION_A_ARTIFACTS:
+            artifacts.append(REVISION_A_ARTIFACTS)
+            artifacts.append(REVISION_B_ARTIFACTS)
+            test_script_opts += f" --artifacts-a {REVISION_A_ARTIFACTS} --artifacts-b {REVISION_B_ARTIFACTS}"
     else:
         # Passing `-m ''` below instructs pytest to collect tests regardless of
         # their markers (e.g. it will collect both tests marked as nonci, and
         # tests without any markers).
         test_script_opts += f" -m '' {test_selector}"
 
+    command = []
+    if artifacts:
+        command.append(pipeline.devtool_download_artifacts(artifacts))
+    command.extend(pipeline.devtool_test(devtool_opts, test_script_opts))
     pipeline.build_group(
-        command=pipeline.devtool_test(devtool_opts, test_script_opts),
+        command=command,
         # and the rest can be command arguments
         **test,
     )

tests/README.md

@@ -184,23 +184,30 @@ function to [`.buildkite/pipeline_perf.py`](../.buildkite/pipeline_perf.py). To
 manually run an A/B-Test, use
 
 ```sh
-tools/devtool -y test --ab [optional arguments to ab_test.py] run <dir A> <dir B> --pytest-opts <test specification>
+tools/devtool -y test --ab [optional arguments to ab_test.py] run --binaries-a <dir A> --binaries-b <dir B> [optional --artifacts-a <name A> --artifacts-b <name B>] --pytest-opts <test specification>
 ```
 
-Here, _dir A_ and _dir B_ are directories containing firecracker and jailer
-binaries whose performance characteristics you wish to compare. You can use
-`./tools/devtool build --rev <revision> --release` to compile binaries from an
-arbitrary git object (commit SHAs, branches, tags etc.). This will create
-sub-directories in `build` containing the binaries. For example, to compare
-boottime of microVMs between Firecracker binaries compiled from the `main`
-branch and the `HEAD` of your current branch, run
+- _dir A_ and _dir B_ are directories containing firecracker and jailer binaries
+  whose performance characteristics you wish to compare
+- _name A_ and _name B_ are names of artifacts A and B run will to use
+  respectively
+
+You can use `./tools/devtool build --rev <revision> --release` to compile
+binaries from an arbitrary git object (commit SHAs, branches, tags etc.). This
+will create sub-directories in `build` containing the binaries. For example, to
+compare boottime of microVMs between Firecracker binaries compiled from the
+`main` branch and the `HEAD` of your current branch, run
 
 ```sh
 tools/devtool -y build --rev main --release
 tools/devtool -y build --rev HEAD --release
 tools/devtool -y test --no-build --ab -- run build/main build/HEAD --pytest-opts integration_tests/performance/test_boottime.py::test_boottime
 ```
 
+To download custom artifacts use
+`./tools/devtool download_ci_artifacts <S3 URI>...`. This will place artifacts
+into `build/artifacts` directory.
+
 #### How to Write an A/B-Compatible Test and Common Pitfalls
 
 First, **A/B-Compatible tests need to emit more than one data point for each
@@ -443,9 +450,9 @@ there. `pytest` will bring them into scope for all your tests.
 
 `Q4:` *I want to use more/other microvm test images, but I don't want to add
 them to the common s3 bucket.*\
-`A4:` Add your custom images to the `build/img` subdirectory in the Firecracker
-source tree. This directory is bind-mounted in the container and used as a local
-image cache.
+`A4:` Add your custom images to the `build/artifacts` subdirectory in the
+Firecracker source tree. This directory is bind-mounted in the container and
+used as a local image cache.
 
 `Q5:` *How can I get live logger output from the tests?*\
 `A5:` Accessing **pytest.ini** will allow you to modify logger settings.

tests/conftest.py

@@ -22,7 +22,6 @@
 import inspect
 import json
 import os
-import platform
 import shutil
 import sys
 import tempfile
@@ -33,7 +32,7 @@
 import host_tools.cargo_build as build_tools
 from framework import defs, utils
 from framework.artifacts import disks, kernel_params
-from framework.defs import DEFAULT_BINARY_DIR
+from framework.defs import ARTIFACT_DIR, DEFAULT_BINARY_DIR
 from framework.microvm import HugePagesConfig, MicroVMFactory, SnapshotType
 from framework.properties import global_props
 from framework.utils_cpu_templates import (
@@ -399,7 +398,7 @@ def microvm_factory(request, record_property, results_dir, netns_factory):
             uvm_data.joinpath("host-dmesg.log").write_text(
                 utils.run_cmd(["dmesg", "-dPx"]).stdout
             )
-            shutil.copy(f"/firecracker/build/img/{platform.machine()}/id_rsa", uvm_data)
+            shutil.copy(ARTIFACT_DIR / "id_rsa", uvm_data)
             if Path(uvm.screen_log).exists():
                 shutil.copy(uvm.screen_log, uvm_data)
 

tests/framework/artifacts.py

@@ -4,16 +4,12 @@
 """Define classes for interacting with CI artifacts"""
 
 import re
-from dataclasses import dataclass
 from pathlib import Path
 from typing import Iterator
 
 import pytest
 
 from framework.defs import ARTIFACT_DIR
-from framework.utils import check_output, get_firecracker_version_from_toml
-from framework.with_filelock import with_filelock
-from host_tools.cargo_build import get_binary
 
 
 def select_supported_kernels():
@@ -55,101 +51,3 @@ def kernel_params(
     """Return supported kernels"""
     for kernel in select(glob, artifact_dir):
         yield pytest.param(kernel, id=kernel.name)
-
-
-@dataclass(frozen=True, repr=True)
-class FirecrackerArtifact:
-    """Utility class for Firecracker binary artifacts."""
-
-    path: Path
-
-    @property
-    def name(self):
-        """Get the Firecracker name."""
-        return self.path.name
-
-    @property
-    def jailer(self):
-        """Get the jailer with the same version."""
-        return self.path.with_name(f"jailer-v{self.version}")
-
-    @property
-    def version(self):
-        """Return Firecracker's version: `X.Y.Z-prerelase`."""
-        # Get the filename, split on the first '-' and trim the leading 'v'.
-        # sample: firecracker-v1.2.0-alpha
-        return self.path.name.split("-", 1)[1][1:]
-
-    @property
-    def version_tuple(self):
-        """Return the artifact's version as a tuple `(X, Y, Z)`."""
-        return tuple(int(x) for x in self.version.split("."))
-
-    @property
-    def snapshot_version_tuple(self):
-        """Return the artifact's snapshot version as a tuple: `X.Y.0`."""
-
-        # Starting from Firecracker v1.7.0, snapshots have their own version that is
-        # independent of Firecracker versions. For these Firecracker versions, use
-        # the --snapshot-version Firecracker flag, to figure out which snapshot version
-        # it supports.
-
-        return (
-            check_output([self.path, "--snapshot-version"])
-            .stdout.strip()
-            .split("\n")[0]
-            .split(".")
-        )
-
-    @property
-    def snapshot_version(self):
-        """Return the artifact's snapshot version: `X.Y.0`.
-
-        Due to how Firecracker maps release versions to snapshot versions, we
-        have to request the minor version instead of the actual version.
-        """
-        return ".".join(str(x) for x in self.snapshot_version_tuple)
-
-
-@with_filelock
-def current_release(version):
-    """Massage this working copy Firecracker binary to look like a normal
-    release, so it can run the same tests.
-    """
-    binaries = []
-    for binary in ["firecracker", "jailer"]:
-        bin_path1 = get_binary(binary)
-        bin_path2 = bin_path1.with_name(f"{binary}-v{version}")
-        if not bin_path2.exists():
-            bin_path2.unlink(missing_ok=True)
-            bin_path2.hardlink_to(bin_path1)
-        binaries.append(bin_path2)
-    return binaries
-
-
-def working_version_as_artifact():
-    """
-    Return working copy of Firecracker as a release artifact
-    """
-    cargo_version = get_firecracker_version_from_toml()
-    return FirecrackerArtifact(current_release(str(cargo_version))[0])
-
-
-def firecracker_artifacts():
-    """Return all supported firecracker binaries."""
-    cargo_version = get_firecracker_version_from_toml()
-    # until the next minor version (but *not* including)
-    max_version = (cargo_version.major, cargo_version.minor + 1, 0)
-    prefix = "firecracker/firecracker-*"
-    for firecracker in sorted(ARTIFACT_DIR.glob(prefix)):
-        match = re.match(r"firecracker-v(\d+)\.(\d+)\.(\d+)", firecracker.name)
-        if not match:
-            continue
-        fc = FirecrackerArtifact(firecracker)
-        version = fc.version_tuple
-        if version >= max_version:
-            continue
-        yield pytest.param(fc, id=fc.name)
-
-    fc = working_version_as_artifact()
-    yield pytest.param(fc, id=fc.name)

tests/framework/defs.py

@@ -32,10 +32,15 @@
 
 SUPPORTED_HOST_KERNELS = ["5.10", "6.1"]
 
-IMG_DIR = Path(DEFAULT_TEST_SESSION_ROOT_PATH) / "img"
-
-# fall-back to the local directory
-if not IMG_DIR.exists():
-    IMG_DIR = LOCAL_BUILD_PATH / "img"
-
-ARTIFACT_DIR = IMG_DIR / platform.machine()
+# When pytest is run in the devctr the test.sh scipt copies artifacts (rootfs, guest kernels, etc)
+# to the /srv/test_artifacts within the container
+ARTIFACT_DIR = Path(DEFAULT_TEST_SESSION_ROOT_PATH) / "test_artifacts"
+
+# Fall-back to the local directory if pytest was run without test.sh script
+if not ARTIFACT_DIR.exists():
+    current_artifacts_dir = (
+        (Path(LOCAL_BUILD_PATH) / "current_artifacts")
+        .read_text(encoding="utf-8")
+        .strip()
+    )
+    ARTIFACT_DIR = LOCAL_BUILD_PATH / current_artifacts_dir

tests/integration_tests/functional/test_cmd_line_parameters.py

@@ -11,39 +11,31 @@
 from host_tools.fcmetrics import validate_fc_metrics
 
 
-def test_describe_snapshot_all_versions(
-    microvm_factory, guest_kernel, rootfs, firecracker_release
-):
+def test_describe_snapshot(uvm_plain):
     """
     Test `--describe-snapshot` correctness for all snapshot versions.
 
     For each release create a snapshot and verify the data version of the
     snapshot state file.
     """
 
-    target_version = firecracker_release.snapshot_version
-    vm = microvm_factory.build(
-        guest_kernel,
-        rootfs,
-        fc_binary_path=firecracker_release.path,
-        jailer_binary_path=firecracker_release.jailer,
-    )
-    # FIXME: Once only FC versions >= 1.12 are supported, drop log_level="warn"
-    vm.spawn(log_level="warn", serial_out_path=None)
+    vm = uvm_plain
+    fc_binary = vm.fc_binary_path
+
+    cmd = [fc_binary, "--snapshot-version"]
+    snap_version_tuple = check_output(cmd).stdout.strip().split("\n")[0].split(".")
+    snap_version = ".".join(str(x) for x in snap_version_tuple)
+
+    vm.spawn()
     vm.basic_config(track_dirty_pages=True)
     vm.start()
     snapshot = vm.snapshot_diff()
-    print("========== Firecracker create snapshot log ==========")
-    print(vm.log_data)
     vm.kill()
 
-    # Fetch Firecracker binary
-    fc_binary = microvm_factory.fc_binary_path
-    # Verify the output of `--describe-snapshot` command line parameter
-    cmd = [fc_binary] + ["--describe-snapshot", snapshot.vmstate]
+    cmd = [fc_binary, "--describe-snapshot", snapshot.vmstate]
     _, stdout, stderr = check_output(cmd)
     assert stderr == ""
-    assert target_version in stdout
+    assert snap_version in stdout
 
 
 def test_cli_metrics_path(uvm_plain):