Skip to content

refactor: make devctr build process multi-staged.#5779

Open
JamesC1305 wants to merge 8 commits intofirecracker-microvm:mainfrom
JamesC1305:update-devctr-v89
Open

refactor: make devctr build process multi-staged.#5779
JamesC1305 wants to merge 8 commits intofirecracker-microvm:mainfrom
JamesC1305:update-devctr-v89

Conversation

@JamesC1305
Copy link
Copy Markdown
Contributor

Make the devctr build process multi-staged. This decouples the different build processes from each other, and in my measurement cut the build time from ~1300s to ~600s (21min to 8min).

In this PR we also update the poetry dependencies and rust toolchain to 1.94.0.

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

  • I have read and understand CONTRIBUTING.md.
  • I have run tools/devtool checkbuild --all to verify that the PR passes
    build checks on all supported architectures.
  • I have run tools/devtool checkstyle to verify that the PR passes the
    automated style checks.
  • I have described what is done in these changes, why they are needed, and
    how they are solving the problem in a clear and encompassing way.
  • I have updated any relevant documentation (both in code and in the docs)
    in the PR.
  • I have mentioned all user-facing changes in CHANGELOG.md.
  • If a specific issue led to this PR, this PR closes the issue.
  • When making API changes, I have followed the
    Runbook for Firecracker API changes.
  • I have tested all new and changed functionalities in unit tests and/or
    integration tests.
  • I have linked an issue to every new TODO.

  • This functionality cannot be added in rust-vmm.

@JamesC1305 JamesC1305 added the Status: Awaiting review Indicates that a pull request is ready to be reviewed label Mar 20, 2026
@JamesC1305 JamesC1305 force-pushed the update-devctr-v89 branch 3 times, most recently from 8633fcd to 5bb3b57 Compare March 20, 2026 16:54
Update the rust toolchain to version 1.94.0. This will close dependabot
PR firecracker-microvm#5803.

Signed-off-by: James Curtis <jxcurtis@amazon.co.uk>
Update python dependencies to close dependabot PRs:
- filelock firecracker-microvm#5626
- black firecracker-microvm#5756
- requests firecracker-microvm#5798
- pygments  firecracker-microvm#5804

Signed-off-by: James Curtis <jxcurtis@amazon.co.uk>
Switch from a single stage build to a multi-staged build for the devctr.
Although we don't often rebuild this, this should reduce the time taken
when we do, due to unchanged layers now being cached.

In my testing, this cut the full build time from ~1300s to ~600s without
inflating the image size further. We also don't have to manually clean
up after earlier build stages, as they are not included in the final
image.

Stage dependency graph:

                                   +--- qemu-builder -------+
                                   |                        |
                                   +--- libseccomp-builder -+
                                   |                        |
   base-image +--------------------+--- iperf3-builder -----+--- devctr
              |                    |                        |
              |                    +--- git-secrets-builder +
              |                                             |
              +--- apt-base                                 |
                      |                                     |
                      |                                     |
                  python-deps      +--- crosvm-builder -----+
                      |            |                        |
                 rust-toolchain ---+------------------------+

 Parallel builders (from base-image):
   qemu-builder, libseccomp-builder, iperf3-builder, git-secrets-builder

 Sequential base chain:
   base-image -> apt-base -> python-deps -> rust-toolchain

 Rust fork (from rust-toolchain):
   crosvm-builder  (build deps thrown away, only binary copied)
   devctr           (cargo tools, kani, nightly, then COPY
                      from all builders)

Signed-off-by: James Curtis <jxcurtis@amazon.co.uk>
With the latest devctr dependency updates, running `tools/devtool fmt`
results in some files being reformatted. These are resulting from new
versions of `black` and `mdformat`.

Signed-off-by: James Curtis <jxcurtis@amazon.co.uk>
As of rust toolchain version 1.94.0, CPUID-related functions from the
standard library are no longer unsafe [1]. However, we cannot simply
remove them as the nightly toolchain used by Kani has not been updated
to a version with this change.

Add TODO comments to remove unsafe blocks when Kani toolchain is
updated.

[1]: rust-lang/stdarch#1935

Signed-off-by: James Curtis <jxcurtis@amazon.co.uk>
zulinx86
zulinx86 previously approved these changes Mar 31, 2026
Copy link
Copy Markdown
Contributor

@zulinx86 zulinx86 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Let me know when you bumped the tag.

Update the devctr to the latest tag v89

Signed-off-by: James Curtis <jxcurtis@amazon.co.uk>
@codecov
Copy link
Copy Markdown

codecov bot commented Mar 31, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.04%. Comparing base (86aac24) to head (8a598ee).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #5779   +/-   ##
=======================================
  Coverage   83.04%   83.04%           
=======================================
  Files         276      276           
  Lines       29466    29466           
=======================================
  Hits        24471    24471           
  Misses       4995     4995           
Flag Coverage Δ
5.10-m5n.metal 83.32% <100.00%> (ø)
5.10-m6a.metal 82.66% <100.00%> (+<0.01%) ⬆️
5.10-m6g.metal 79.96% <ø> (ø)
5.10-m6i.metal 83.32% <100.00%> (-0.01%) ⬇️
5.10-m7a.metal-48xl 82.65% <100.00%> (+<0.01%) ⬆️
5.10-m7g.metal 79.96% <ø> (ø)
5.10-m7i.metal-24xl 83.30% <100.00%> (ø)
5.10-m7i.metal-48xl 83.29% <100.00%> (-0.01%) ⬇️
5.10-m8g.metal-24xl 79.95% <ø> (ø)
5.10-m8g.metal-48xl 79.95% <ø> (-0.01%) ⬇️
5.10-m8i.metal-48xl 83.30% <100.00%> (-0.01%) ⬇️
5.10-m8i.metal-96xl 83.30% <100.00%> (-0.01%) ⬇️
6.1-m5n.metal 83.35% <100.00%> (+<0.01%) ⬆️
6.1-m6a.metal 82.69% <100.00%> (+<0.01%) ⬆️
6.1-m6g.metal 79.95% <ø> (-0.01%) ⬇️
6.1-m6i.metal 83.35% <100.00%> (ø)
6.1-m7a.metal-48xl 82.67% <100.00%> (ø)
6.1-m7g.metal 79.96% <ø> (ø)
6.1-m7i.metal-24xl 83.36% <100.00%> (ø)
6.1-m7i.metal-48xl 83.36% <100.00%> (ø)
6.1-m8g.metal-24xl 79.94% <ø> (-0.01%) ⬇️
6.1-m8g.metal-48xl 79.95% <ø> (ø)
6.1-m8i.metal-48xl 83.37% <100.00%> (+<0.01%) ⬆️
6.1-m8i.metal-96xl 83.37% <100.00%> (+<0.01%) ⬆️

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.

The LTS ubuntu 24.04 version seems to have been updated since the
last devctr build. Update the get_os_version() function accordingly.

Signed-off-by: James Curtis <jxcurtis@amazon.co.uk>
@JamesC1305 JamesC1305 enabled auto-merge (rebase) March 31, 2026 18:37
Copy link
Copy Markdown
Contributor

@Manciukic Manciukic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there's 2 fewer dependencies in the final image. Are they used at runtime during the vhost-user tests?

RUN apt-get update \
&& apt-get -y install --no-install-recommends \
# essential build tools
gcc make libc-dev binutils-dev libssl-dev \
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

libcap2 looks like a runtime dependency for running vhost-user tests. libglib2.0-dev looks like a build dependency but maybe it's pulling in the runtime dependency as well so we should double check

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All the tests in test_drive_vhost_user.py seem to be passing. I checked the installed packages in the devctr and it seems like libcap2 is already installed, so it must be dragged in by something else. Perhaps we should just make it explicit then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Status: Awaiting review Indicates that a pull request is ready to be reviewed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants