Artifacts and how to get them #5585

ShadowCurse · 2025-12-16T16:28:20Z

Changes

make devtool pull latest artifacts from s3
add ability to download multiple custom artifacts
add ability to specify which artifacts to use in tests
add ability to specify artifacts for A/B tests

Reason

Artifacts are rebuilt periodically now, so update our tools to be able to pull latest ones.
Additionally, while we are at it, add ability to download and switch between multiple
artifacts. This also opened a path for A/B tests of artifacts as well.

License Acceptance

By submitting this pull request, I confirm that my contribution is made under
the terms of the Apache 2.0 license. For more information on following Developer
Certificate of Origin and signing off your commits, please check
CONTRIBUTING.md.

PR Checklist

This functionality cannot be added in rust-vmm.

codecov · 2025-12-16T16:35:09Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.23%. Comparing base (f0691f8) to head (075b628).

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #5585   +/-   ##
=======================================
  Coverage   83.23%   83.23%           
=======================================
  Files         277      277           
  Lines       29263    29263           
=======================================
  Hits        24358    24358           
  Misses       4905     4905

Flag	Coverage Δ
5.10-m5n.metal	`83.58% <ø> (+<0.01%)`	⬆️
5.10-m6a.metal	`82.91% <ø> (ø)`
5.10-m6g.metal	`80.19% <ø> (ø)`
5.10-m6i.metal	`83.57% <ø> (ø)`
5.10-m7a.metal-48xl	`82.90% <ø> (ø)`
5.10-m7g.metal	`80.19% <ø> (ø)`
5.10-m7i.metal-24xl	`83.54% <ø> (ø)`
5.10-m7i.metal-48xl	`83.54% <ø> (ø)`
5.10-m8g.metal-24xl	`80.18% <ø> (ø)`
5.10-m8g.metal-48xl	`80.18% <ø> (ø)`
6.1-m5n.metal	`83.60% <ø> (+<0.01%)`	⬆️
6.1-m6a.metal	`82.94% <ø> (ø)`
6.1-m6g.metal	`80.18% <ø> (-0.01%)`	⬇️
6.1-m6i.metal	`83.60% <ø> (ø)`
6.1-m7a.metal-48xl	`82.93% <ø> (ø)`
6.1-m7g.metal	`80.18% <ø> (-0.01%)`	⬇️
6.1-m7i.metal-24xl	`83.62% <ø> (+<0.01%)`	⬆️
6.1-m7i.metal-48xl	`83.61% <ø> (ø)`
6.1-m8g.metal-24xl	`80.18% <ø> (ø)`
6.1-m8g.metal-48xl	`80.18% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Manciukic · 2025-12-18T10:54:07Z

tests/framework/defs.py

+    current_artifacts_dir = (
+        open(Path(LOCAL_BUILD_PATH) / "current_artifacts", "r", encoding="utf-8")
+        .read()
+        .strip()
+    )


why don't we just use the directory (which I'm assuming it's a link) directly?

links do not work because of docker

Manciukic · 2025-12-18T10:54:54Z

tests/conftest.py

                utils.run_cmd(["dmesg", "-dPx"]).stdout
            )
-            shutil.copy(f"/firecracker/build/img/{platform.machine()}/id_rsa", uvm_data)
+            shutil.copy("/srv/current_artifacts/id_rsa", uvm_data)


shouldn't this be /firecracker/build/current_artifacts? Why can't we use the constants defined in defs?

you are correct, switch to using ARTIFACT_DIR

Manciukic · 2025-12-18T10:55:57Z

tools/devtool

+# File with a single line specifing the name of the
+# currently used artifacts
+LOCAL_ARTIFACTS_CURRENT_DIR_FILE="build/current_artifacts"


why not a symlink?

also, why not in build/artifacts/current ?

I had this idea initially, but as it turns out symlinks will not work because we will create symlinks outside docker, but tests are run inside, so links will be broken

no particular reason, I just happened to choose current_artifacts, can change if you prefer the other version

Manciukic · 2025-12-18T10:56:38Z

tools/devtool

 # Query default S3 bucket with artifacts and return the most recient path
 get_newest_s3_artifacts() {
  echo $(aws s3 ls $DEFAULT_ARTIFACTS_S3_BUCKET/ --no-sign-request | grep "PRE" | sort -k2 | tail -1 | awk -v bucket="$DEFAULT_ARTIFACTS_S3_BUCKET" '{print bucket "/" $2}')
+  # echo "s3://spec.ccfc.min/firecracker-ci/v1.15"


Manciukic · 2025-12-18T10:58:43Z

tools/devtool

 cmd_download_ci_artifacts() {
    if [ "$1" = "--force" ]; then
-        rm -rf $FC_BUILD_DIR/img
+        rm -rf $ARTIFACTS_DIR


I can't find where this is defined. Also, the action of --force should be to override that specific artifacts if present, not to delete all of them (rm -rf artifacts_local_path)

Added a commit where --force is removed. I think it is pretty simple to just delete build/artifacts/... if you need to.

Manciukic · 2025-12-18T11:03:51Z

tools/devtool

 ensure_ci_artifacts() {
-    if ! command -v aws >/dev/null; then
-      die "AWS CLI not installed, which is required for downloading artifacts for integration tests."
+    local artifacts=$1


nit: please use singular and plural consistently. Also, it's not clear whether this is an S3 key, a URI, or something else

Manciukic · 2025-12-18T11:04:46Z

tools/devtool

+            "--no-ci-artifacts-check")
+                do_ci_artifacts_check=0


maybe --no-download-artifacts?

Also, please add it to the docs for the command

added a separate commit with this

Manciukic · 2025-12-18T11:08:59Z

.buildkite/pipeline_perf.py

+        command.append(pipeline.devtool_download_artifacts(artifacts))
+    # Hack because devtool_test already returns an array, just this array always
+    # has just 1 element
+    command.append(pipeline.devtool_test(devtool_opts, test_script_opts)[0])


nit: you could use extend

Manciukic · 2025-12-18T11:11:40Z

tools/devtool

    done
 }

+cmd_set_current_artifacts() {


why do we need a separate command when the download one would just do the same when they're already downloaded?

Change the logic a bit. Now downloading is separate from setting current artifacts.

Manciukic · 2025-12-18T11:14:55Z

tools/devtool

    fi

    for artifact in "${artifacts[@]}"; do
      ensure_ci_artifacts $artifact


I think we need a way to specify which ones we want to be set as the current, or to be explicit that the last one will be set as current

don't think download command should mess with current artifacts. There is a separate command to set it.

I selected approve by mistakr

The snapshot version check was iterating over last several FC versions pulled from S3, but it was unnecessary since there were no cross FC checks. Remove checks of old FC versions and only test the current version. Signed-off-by: Egor Lazarchuk <yegorlz@amazon.co.uk>

Instead of using fixed artifact path for a current FC version pull newest artifacts from S3. This way we can update them independently while keeping old versions around. Unfortunately there is no simple way to query newest added directory in S3, so do the next best thing of looking through all files in all directories and determining the `newest` directory be the `LastModified` status of files inside. Signed-off-by: Egor Lazarchuk <yegorlz@amazon.co.uk>

The old setup used `build/img` as the base directory for artifacts pulled from S3. In S3 the artifacts directory contains 1 subdirectory per architecture (x86_64 and aarch64). This was causing final local artifacts path to be `build/img/x86_64` or `build/img/aarch64`. This commit changes the structure a to have a separate `build/artifacts` directory with subdirectories containing different versions of artifacts. The path to currently used artifacts will be placed in `build/current_artifacts` file. This make is easy to switch between multiple versions of artifacts without a need to delete/download them. This also opens a door for A/B testing of artifacts. Signed-off-by: Egor Lazarchuk <yegorlz@amazon.co.uk>

Downloading artifacts is a separate step that should not mess with currently selected artifacts. Signed-off-by: Egor Lazarchuk <yegorlz@amazon.co.uk>

It will interfere with following multi-artifacts downloads later. Additionally it is simple enough to just delete build/artifacts by hand. Signed-off-by: Egor Lazarchuk <yegorlz@amazon.co.uk>

Modify `download_ci_artifacts` to accept artifacts paths as input and download by passing these paths to `ensure_ci_artifacts`. `ensure_ci_artifacts` will default to latest s3 artifacts if no args were provided. Signed-off-by: Egor Lazarchuk <yegorlz@amazon.co.uk>

Add arguments names for binary paths for ab_test.py Signed-off-by: Egor Lazarchuk <yegorlz@amazon.co.uk>

Update `ab_test.py` with ability to accept custom artifacts for A and B runs. Additionally update `pipeline_perf.py` as well. Now REVISION_A_ARTIFACTS and REVISION_B_ARTIFACTS environment variables specify custom artifacts which will be used in A/B test Signed-off-by: Egor Lazarchuk <yegorlz@amazon.co.uk>

Some tests like style checks do not require artifacts to be present. Signed-off-by: Egor Lazarchuk <yegorlz@amazon.co.uk>

There is no reason for style check to download artifacts Signed-off-by: Egor Lazarchuk <yegorlz@amazon.co.uk>

Add missing descriptions for additional arguments Signed-off-by: Egor Lazarchuk <yegorlz@amazon.co.uk>

ShadowCurse requested review from Manciukic, kalyazin, pb8o and xmarcalx as code owners December 16, 2025 16:28

ShadowCurse force-pushed the pull_latest_s3_artifacts branch from 7682ec8 to 24326d6 Compare December 16, 2025 16:31

ShadowCurse force-pushed the pull_latest_s3_artifacts branch 7 times, most recently from 1de4267 to 59fb38f Compare December 17, 2025 17:10

ShadowCurse self-assigned this Dec 17, 2025

ShadowCurse added Status: Awaiting review Indicates that a pull request is ready to be reviewed Type: Enhancement Indicates new feature requests labels Dec 17, 2025

Manciukic previously approved these changes Dec 18, 2025

View reviewed changes

ShadowCurse force-pushed the pull_latest_s3_artifacts branch 5 times, most recently from 46a7c58 to 80e3f2d Compare December 18, 2025 17:50

ShadowCurse added 7 commits December 18, 2025 17:51

refactor: split downloading and setting current artifacts

928ac6e

Downloading artifacts is a separate step that should not mess with currently selected artifacts. Signed-off-by: Egor Lazarchuk <yegorlz@amazon.co.uk>

refactor: remove --force flag from download_ci_artifacts command

215e612

It will interfere with following multi-artifacts downloads later. Additionally it is simple enough to just delete build/artifacts by hand. Signed-off-by: Egor Lazarchuk <yegorlz@amazon.co.uk>

refactor: specify explicit arguments for ab_test.py

3886e89

Add arguments names for binary paths for ab_test.py Signed-off-by: Egor Lazarchuk <yegorlz@amazon.co.uk>

ShadowCurse added 4 commits December 18, 2025 17:51

chore(test.sh): skip aritfact setup if no current is specified

5e27336

Some tests like style checks do not require artifacts to be present. Signed-off-by: Egor Lazarchuk <yegorlz@amazon.co.uk>

chore(devtool): remove artifacts check from checkstyle command

f17db88

There is no reason for style check to download artifacts Signed-off-by: Egor Lazarchuk <yegorlz@amazon.co.uk>

chore(devtool): update cmd_test help

075b628

Add missing descriptions for additional arguments Signed-off-by: Egor Lazarchuk <yegorlz@amazon.co.uk>

ShadowCurse force-pushed the pull_latest_s3_artifacts branch from 80e3f2d to 075b628 Compare December 18, 2025 17:51

Artifacts and how to get them #5585

Are you sure you want to change the base?

Artifacts and how to get them #5585

Conversation

ShadowCurse commented Dec 16, 2025

Changes

Reason

License Acceptance

PR Checklist

Uh oh!

codecov bot commented Dec 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

codecov bot commented Dec 16, 2025 •

edited

Loading