Skip to content

Streamline rootfs building#5727

Merged
JamesC1305 merged 3 commits intofirecracker-microvm:mainfrom
JamesC1305:fix-docker-popular-containers-rootfs
Mar 10, 2026
Merged

Streamline rootfs building#5727
JamesC1305 merged 3 commits intofirecracker-microvm:mainfrom
JamesC1305:fix-docker-popular-containers-rootfs

Conversation

@JamesC1305
Copy link
Copy Markdown
Contributor

@JamesC1305 JamesC1305 commented Feb 27, 2026

Changes

Move rebuild.sh rootfs building code into common functions file and refactor slightly, allowing the function to take an extra script as a parameter, with that script being run inside the container. This allows different behaviour to be specified for CI rootfs and docker popular rootfs.

Then change test-docker-popular to reuse this common rootfs builder code.

NB: docker-popular now uses squashfs rather than ext4 as before.

Reason

We currently have two very similar processes for building rootfs. One for building a CI rootfs, and another for running Firecracker in popular docker images (test-docker-popular). We should instead have a common implementation that works for both, for easier extensibility and maintainability.

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.

@codecov
Copy link
Copy Markdown

codecov Bot commented Feb 27, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.13%. Comparing base (7f08760) to head (8c6ae7b).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #5727   +/-   ##
=======================================
  Coverage   83.13%   83.13%           
=======================================
  Files         277      277           
  Lines       29466    29466           
=======================================
  Hits        24498    24498           
  Misses       4968     4968           
Flag Coverage Δ
5.10-m5n.metal 83.42% <ø> (ø)
5.10-m6a.metal 82.76% <ø> (-0.01%) ⬇️
5.10-m6g.metal 80.14% <ø> (+<0.01%) ⬆️
5.10-m6i.metal 83.42% <ø> (-0.01%) ⬇️
5.10-m7a.metal-48xl 82.75% <ø> (-0.01%) ⬇️
5.10-m7g.metal 80.14% <ø> (ø)
5.10-m7i.metal-24xl 83.40% <ø> (+<0.01%) ⬆️
5.10-m7i.metal-48xl 83.40% <ø> (+<0.01%) ⬆️
5.10-m8g.metal-24xl 80.14% <ø> (ø)
5.10-m8g.metal-48xl 80.14% <ø> (ø)
5.10-m8i.metal-48xl 83.40% <ø> (-0.01%) ⬇️
5.10-m8i.metal-96xl 83.40% <ø> (ø)
6.1-m5n.metal 83.45% <ø> (-0.01%) ⬇️
6.1-m6a.metal 82.79% <ø> (+<0.01%) ⬆️
6.1-m6g.metal 80.14% <ø> (ø)
6.1-m6i.metal 83.45% <ø> (ø)
6.1-m7a.metal-48xl 82.78% <ø> (-0.01%) ⬇️
6.1-m7g.metal 80.14% <ø> (ø)
6.1-m7i.metal-24xl 83.46% <ø> (ø)
6.1-m7i.metal-48xl 83.47% <ø> (-0.01%) ⬇️
6.1-m8g.metal-24xl 80.13% <ø> (-0.01%) ⬇️
6.1-m8g.metal-48xl 80.14% <ø> (ø)
6.1-m8i.metal-48xl 83.48% <ø> (+<0.01%) ⬆️
6.1-m8i.metal-96xl 83.47% <ø> (+<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.

@JamesC1305 JamesC1305 force-pushed the fix-docker-popular-containers-rootfs branch 10 times, most recently from 7147cd6 to a140d81 Compare March 2, 2026 13:09
@JamesC1305 JamesC1305 marked this pull request as ready for review March 2, 2026 13:28
@JamesC1305 JamesC1305 changed the title [DRAFT] Streamline rootfs building Streamline rootfs building Mar 2, 2026
@JamesC1305 JamesC1305 added the Status: Awaiting review Indicates that a pull request is ready to be reviewed label Mar 2, 2026
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.

LGTM, just a nit.

Comment thread .buildkite/pipeline_docker_popular.py Outdated
@JamesC1305 JamesC1305 force-pushed the fix-docker-popular-containers-rootfs branch from a140d81 to c7e5494 Compare March 5, 2026 14:36
zulinx86
zulinx86 previously approved these changes Mar 5, 2026
Comment thread tools/functions
@JamesC1305
Copy link
Copy Markdown
Contributor Author

Looks like our CI tests are getting stuck with the new rootfs. I'll convert back to draft whilst I investigate this.

@JamesC1305 JamesC1305 marked this pull request as draft March 5, 2026 16:33
@JamesC1305 JamesC1305 force-pushed the fix-docker-popular-containers-rootfs branch from c7e5494 to 5a125da Compare March 6, 2026 13:30
@JamesC1305
Copy link
Copy Markdown
Contributor Author

JamesC1305 commented Mar 6, 2026

Change since last version:

  • Instead of copying /var from the container image in tools/functions, just make /var/lib/{dpkg,systemd} in chroot.sh. (this saves about 40MB on the squashfs size)

I'm running docker popular on my personal pipeline as we speak and building new CI artifacts. Once I've run docker popular and nightly PR, I'll move it back to ready for review :)

@JamesC1305 JamesC1305 force-pushed the fix-docker-popular-containers-rootfs branch from 5a125da to 67877d8 Compare March 6, 2026 14:39
@JamesC1305 JamesC1305 marked this pull request as ready for review March 6, 2026 16:09
@JamesC1305
Copy link
Copy Markdown
Contributor Author

Had to update setup-minimal.sh to copy /var from the container to the rootfs. We are not as picky with the size of these images, so for the sake of compatibility, just copy the whole directory back.

Comment thread tools/test-popular-containers/setup-minimal.sh
Comment thread tools/test-popular-containers/setup-minimal.sh
@JamesC1305 JamesC1305 force-pushed the fix-docker-popular-containers-rootfs branch from 67877d8 to aedd089 Compare March 6, 2026 18:10
Comment thread tools/test-popular-containers/build_rootfs.sh Outdated
Comment thread tools/test-popular-containers/setup-minimal.sh Outdated
Comment thread tools/test-popular-containers/setup-minimal.sh Outdated
Comment thread tools/test-popular-containers/setup-minimal.sh Outdated
@JamesC1305 JamesC1305 force-pushed the fix-docker-popular-containers-rootfs branch from aedd089 to b3fbd4e Compare March 9, 2026 10:21
Move code from CI rootfs rebuild script into common functions file. This
will allow it to be shared with the docker popular code as well.

Refactor it slightly too, so that it takes another bash script as a
parameter that we then run inside the docker image. This is what allows
the implementation to be shared with docker-popular – all of the
build-specific functionality is defined inside the supplied script.

Signed-off-by: James Curtis <jxcurtis@amazon.co.uk>
@JamesC1305 JamesC1305 force-pushed the fix-docker-popular-containers-rootfs branch from b3fbd4e to 2333251 Compare March 9, 2026 10:25
zulinx86
zulinx86 previously approved these changes Mar 9, 2026
Simplify the docker-popular rootfs building using common functions.
Define a new file `setup-minimal.sh` that is responsible for the
image-specific setups.

Also, use squashfs for test-popular-containers tests, as there is no
specific reason for them to be ext4.

Signed-off-by: James Curtis <jxcurtis@amazon.co.uk>
@JamesC1305 JamesC1305 force-pushed the fix-docker-popular-containers-rootfs branch from ca9bd5d to 2e0da39 Compare March 9, 2026 15:36
Comment thread tools/test-popular-containers/setup-minimal.sh
@JamesC1305
Copy link
Copy Markdown
Contributor Author

The docker-popular tests pass with the new build process, and PR tests passed with newly built CI artifacts. Merging.

@JamesC1305 JamesC1305 merged commit e6e5c20 into firecracker-microvm:main Mar 10, 2026
7 checks passed
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