test: improve test and fixtures

- split the tests so we can see which ones pass
- only test firecracker supported versions (drop 0.24, 0.25)
- make a new firecracker fixture so it is simple to use in tests
- give tests more descriptive names

Signed-off-by: Pablo Barbáchano <[email protected]>

Author: pb8o

tests/conftest.py

@@ -94,10 +94,11 @@ def test_with_any_microvm(test_microvm_any):
 from host_tools import proc
 from framework import utils
 from framework import defs
-from framework.artifacts import ArtifactCollection
+from framework.artifacts import ArtifactCollection, FirecrackerArtifact
 from framework.microvm import Microvm
 from framework.s3fetcher import MicrovmImageS3Fetcher
 from framework.scheduler import PytestScheduler
+from framework.utils import get_firecracker_version_from_toml
 
 # Tests root directory.
 SCRIPT_FOLDER = os.path.dirname(os.path.realpath(__file__))
@@ -503,6 +504,35 @@ def aarch64(body):
     assert mitigated, "SPECTREv2 not mitigated {}".format(body)
 
 
+def firecracker_id(fc):
+    """Render a nice ID for pytest parametrize."""
+    if isinstance(fc, FirecrackerArtifact):
+        return f"firecracker-{fc.version}"
+    return None
+
+
+def firecracker_artifacts(*args, **kwargs):
+    """Return all supported firecracker binaries."""
+    params = {
+        "min_version": "1.1.0",
+        "max_version": get_firecracker_version_from_toml(),
+    }
+    params.update(kwargs)
+    return ARTIFACTS_COLLECTION.firecrackers(
+        *args,
+        **params,
+    )
+
+
+@pytest.fixture(params=firecracker_artifacts(), ids=firecracker_id)
+def firecracker_release(request):
+    """Return all supported firecracker binaries."""
+    firecracker = request.param
+    firecracker.download()
+    firecracker.jailer().download()
+    return firecracker
+
+
 def pytest_generate_tests(metafunc):
     """Implement customized parametrization scheme.
 

tests/framework/artifacts.py

@@ -2,6 +2,7 @@
 # SPDX-License-Identifier: Apache-2.0
 """Define classes for interacting with CI artifacts in s3."""
 
+import functools
 import os
 import platform
 import tempfile
@@ -12,6 +13,7 @@
 
 import boto3
 import botocore.client
+
 from framework.defs import DEFAULT_TEST_SESSION_ROOT_PATH, SUPPORTED_KERNELS
 from framework.utils import compare_versions
 from host_tools.snapshot_helper import merge_memory_bitmaps
@@ -93,7 +95,7 @@ def download(self, target_folder=ARTIFACTS_LOCAL_ROOT, force=False):
         Path(self.local_dir()).mkdir(parents=True, exist_ok=True)
         if force or not os.path.exists(self.local_path()):
             self._bucket.download_file(self._key, self.local_path())
-            # Artifacts are read only by design.
+            # Artifacts are read-only by design.
             os.chmod(self.local_path(), S_IREAD)
 
     def local_path(self):
@@ -276,11 +278,12 @@ def ssh_key(self):
 class FirecrackerArtifact(Artifact):
     """Provides access to associated jailer artifact."""
 
+    @functools.lru_cache
     def jailer(self):
         """Return a jailer binary artifact."""
         # Jailer and FC binaries have different extensions and share
         # file name when stored in S3:
-        # Firecracker binary: v0.22.firecrcker
+        # Firecracker binary: v0.22.firecracker
         # Jailer binary: v0.23.0.jailer
         jailer_path = str(Path(self.key).with_suffix(".jailer"))
         return Artifact(self.bucket, jailer_path, artifact_type=ArtifactType.JAILER)
@@ -291,6 +294,25 @@ def version(self):
         # Get the filename, remove the extension and trim the leading 'v'.
         return os.path.splitext(os.path.basename(self.key))[0][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`."""
+        return self.version_tuple[:2] + (0,)
+
+    @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)
+
 
 class ArtifactCollection:
     """Provides easy access to different artifact types."""

tests/integration_tests/functional/test_cmd_line_parameters.py

@@ -7,81 +7,64 @@
 from pathlib import Path
 
 import host_tools.logging as log_tools
-from host_tools.cargo_build import get_firecracker_binaries
-from conftest import _test_images_s3_bucket
-from framework.artifacts import ArtifactCollection
 from framework.builder import MicrovmBuilder, SnapshotBuilder, SnapshotType
-from framework.utils import run_cmd, get_firecracker_version_from_toml
+from framework.utils import run_cmd
+from host_tools.cargo_build import get_firecracker_binaries
 
 
-def test_describe_snapshot_all_versions(bin_cloner_path):
+def test_describe_snapshot_all_versions(bin_cloner_path, firecracker_release):
     """
     Test `--describe-snapshot` correctness for all snapshot versions.
 
+    For each release create a snapshot and verify the data version of the
+    snapshot state file.
+
     @type: functional
     """
     logger = logging.getLogger("describe_snapshot")
     builder = MicrovmBuilder(bin_cloner_path)
-    artifacts = ArtifactCollection(_test_images_s3_bucket())
-    # Fetch all firecracker binaries.
-    # For each binary create a snapshot and verify the data version
-    # of the snapshot state file.
+    jailer = firecracker_release.jailer()
+    target_version = firecracker_release.snapshot_version
+
+    logger.info(
+        "Creating snapshot with Firecracker: %s", firecracker_release.local_path()
+    )
+    logger.info("Using Jailer: %s", jailer.local_path())
+    logger.info("Using target version: %s", target_version)
 
-    firecracker_artifacts = artifacts.firecrackers(
-        max_version=get_firecracker_version_from_toml()
+    vm_instance = builder.build_vm_nano(
+        fc_binary=firecracker_release.local_path(),
+        jailer_binary=jailer.local_path(),
+        diff_snapshots=True,
     )
+    vm = vm_instance.vm
+    vm.start()
+
+    # Create a snapshot builder from a microvm.
+    snapshot_builder = SnapshotBuilder(vm)
+    disks = [vm_instance.disks[0].local_path()]
 
-    for firecracker in firecracker_artifacts:
-        firecracker.download()
-        jailer = firecracker.jailer()
-        jailer.download()
-
-        target_version = firecracker.base_name()[1:]
-        # Skip for aarch64, since the snapshotting feature
-        # was introduced in v0.24.0.
-        if platform.machine() == "aarch64" and "v0.23" in target_version:
-            continue
-
-        logger.info("Creating snapshot with Firecracker: %s", firecracker.local_path())
-        logger.info("Using Jailer: %s", jailer.local_path())
-        logger.info("Using target version: %s", target_version)
-
-        # v0.23 does not support creating diff snapshots.
-        diff_snapshots = "0.23" not in target_version
-        vm_instance = builder.build_vm_nano(
-            fc_binary=firecracker.local_path(),
-            jailer_binary=jailer.local_path(),
-            diff_snapshots=diff_snapshots,
-        )
-        vm = vm_instance.vm
-        vm.start()
-
-        # Create a snapshot builder from a microvm.
-        snapshot_builder = SnapshotBuilder(vm)
-        disks = [vm_instance.disks[0].local_path()]
-
-        # Version 0.24 and greater have Diff support.
-        snap_type = SnapshotType.DIFF if diff_snapshots else SnapshotType.FULL
-
-        snapshot = snapshot_builder.create(
-            disks,
-            vm_instance.ssh_key,
-            target_version=target_version,
-            snapshot_type=snap_type,
-        )
-        logger.debug("========== Firecracker create snapshot log ==========")
-        logger.debug(vm.log_data)
-        vm.kill()
-
-        # Fetch Firecracker binary for the latest version
-        fc_binary, _ = get_firecracker_binaries()
-        # Verify the output of `--describe-snapshot` command line parameter
-        cmd = [fc_binary] + ["--describe-snapshot", snapshot.vmstate]
-
-        code, stdout, stderr = run_cmd(cmd)
-        assert code == 0
-        assert stderr == ""
-        assert target_version in stdout
+    # Version 0.24 and greater have Diff support.
+    snap_type = SnapshotType.DIFF
+
+    snapshot = snapshot_builder.create(
+        disks,
+        vm_instance.ssh_key,
+        snapshot_type=snap_type,
+    )
+    logger.debug("========== Firecracker create snapshot log ==========")
+    logger.debug(vm.log_data)
+    vm.kill()
+
+    # Fetch Firecracker binary for the latest version
+    fc_binary, _ = get_firecracker_binaries()
+    # Verify the output of `--describe-snapshot` command line parameter
+    cmd = [fc_binary] + ["--describe-snapshot", snapshot.vmstate]
+
+    code, stdout, stderr = run_cmd(cmd)
+    assert code == 0, stderr
+    assert stderr == ""
+    assert target_version in stdout
 
 
 def test_cli_metrics_path(test_microvm_with_api):