Skip to content

Fix CI build by using pre-built rootfs image #488

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 23, 2023

Conversation

fangn2
Copy link
Contributor

@fangn2 fangn2 commented Mar 16, 2023

Issue #, if available:
#418
The main branch is failing due to hardcoded rootfs name as the name changed with upstream firecracker moving from bionic to jammy. Also devtool used to build rootfs is unstable.

cp: cannot stat 'build/firecracker/build/rootfs/bionic.rootfs.ext4': No such file or directory

Description of changes:

  1. Not specify the ubuntu version name but using regex to match it. The approach is safe since we will only have one rootfs built at a time.
  2. Use prebuilt rootfs and ssh key from S3 instead of building from devtool on fly which seems pretty unstable.
  3. Update readme of snapshotting example to reflect current status.
  4. Minor update for the snapshotting example code to clean up generated files.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
Signed-off-by: Tony Fang [email protected]

@fangn2 fangn2 requested a review from a team as a code owner March 16, 2023 19:10
Copy link
Contributor

@swagatbora90 swagatbora90 left a comment

Choose a reason for hiding this comment

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

LGTM. The buildkite go-sdk and go-sdk-amd CI steps have failed. PTAL before merging.

@fangn2 fangn2 force-pushed the fix-build branch 4 times, most recently from 1645a32 to 5523649 Compare March 21, 2023 15:48
@fangn2 fangn2 changed the title Fix CI build by not hardcoding distro name Fix CI build Mar 21, 2023
cp temp/build/rootfs/ssh/id_rsa root-drive-ssh-key
rm -rf temp

curl -L -o root-drive-with-ssh.img https://s3.amazonaws.com/spec.ccfc.min/ci-artifacts/disks/${ARCH}/ubuntu-18.04.ext4
Copy link
Contributor

Choose a reason for hiding this comment

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

devtool is using ubuntu 22.04, correct? We should keep the same upgraded version here imo.

Copy link
Contributor Author

@fangn2 fangn2 Mar 21, 2023

Choose a reason for hiding this comment

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

Current HEAD of firecracker uses 22.04. Older versions of firecracker use 18.04.
Current issue is firecracker didn't publish ubuntu-22.04.ext4 in the same S3 bucket.
I can follow up with them and address this in a follow-up PR?

@fangn2 fangn2 force-pushed the fix-build branch 2 times, most recently from f2ed91e to 13d7450 Compare March 21, 2023 19:50
@fangn2 fangn2 changed the title Fix CI build Fix CI build by using pre-built rootfs image Mar 21, 2023
# Download Firecracker and its jailer.
- make deps
# Build a rootfs with SSH enabled.
- sudo -E FC_TEST_DATA_PATH=${FC_TEST_DATA_PATH} make ${FC_TEST_DATA_PATH}/root-drive-ssh-key
Copy link
Contributor Author

@fangn2 fangn2 Mar 21, 2023

Choose a reason for hiding this comment

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

With pre-built rootfs, we no longer need to build them with root, now they are part of deps build(check Makrfile changes below)

@fangn2
Copy link
Contributor Author

fangn2 commented Mar 22, 2023

Updated few other parts(please check Description of changes for details) to pass the build.
Cleaned up the examples code base a little bit.

CI seems a little bit unstable when it ran step test against Firecracker above which uses firecracker built from HEAD of main, but I think we can monitor from there.

@swagatbora90 @austinvazquez @ginglis13 PTAL

@fangn2 fangn2 force-pushed the fix-build branch 3 times, most recently from b3033e8 to 6c0ab8f Compare March 22, 2023 15:55
Copy link
Contributor

@swagatbora90 swagatbora90 left a comment

Choose a reason for hiding this comment

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

Minor nits, otherwise looks good.

We are using hello-rootfs.ext4 for non-ssh testing, which I believe is also uploaded by firecracker team. Lets confirm what OS version this is based of and whether FC team can do something similar for ssh version, i.e. with OS agnostic naming.

By not hardcoding distro name and switching to pre-built rootfs image instead of using devtool
which is unstable

Signed-off-by: Tony Fang <[email protected]>
@fangn2 fangn2 merged commit aa97886 into firecracker-microvm:main Mar 23, 2023
@fangn2 fangn2 deleted the fix-build branch March 23, 2023 23:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants