Skip to content
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

Upgrade firecracker from v1.1.0 to v1.4.1 #757

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

fangn2
Copy link
Contributor

@fangn2 fangn2 commented Jul 13, 2023

Issue #, if available:

Description of changes:
Upgrade firecracker from v1.1.0 to v.1.4.1

Firecracker diff firecracker-microvm/firecracker@v1.1.0...v1.4.1

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@fangn2 fangn2 requested a review from a team as a code owner July 13, 2023 20:01
@henry118
Copy link
Member

Not sure how the upgrade to v1.1 was performed previously, but it seems like fc-go-sdk remained at v1.0. The correct process should be

  1. upgrade fc in fc-go-sdk; and
  2. upgrade fccd

@fangn2
Copy link
Contributor Author

fangn2 commented Jul 14, 2023

The correct process should be
upgrade fc in fc-go-sdk;
and upgrade fccd

firecracker submodule in the repo is used to compile firecracker binary for CI testing.
We then uses firecracker-go-sdk to talk to firecracker.
As long as API from different versions of firecracker are compatible, we won't have issue.

Yeah, it's better to align though.

@fangn2
Copy link
Contributor Author

fangn2 commented Jul 14, 2023

CI is failing due to issue compiling firecracker using upstream devtool.

_submodules/firecracker/tools/devtool -y build --release && \
_submodules/firecracker/tools/devtool -y strip
  | [Firecracker devtool] About to pull docker image public.ecr.aws/firecracker/fcuvm:v55
  | v55: Pulling from firecracker/fcuvm
  | 74ac377868f8: Pull complete
  | 83ee436935aa: Pull complete
  | 46994605c311: Pull complete
  | 85a877bd2809: Pull complete
  | e39a8eb621aa: Pull complete
  | 0ddcea765f91: Pull complete
  | be45bf2bd951: Pull complete
  | ffdb30d3fd42: Pull complete
  | edbdb3176e37: Pull complete
  | Digest: sha256:aa52d6030f44ceb19b4f0e75c885639729a4f43196397016df41ebc1e7497686
  | Status: Downloaded newer image for public.ecr.aws/firecracker/fcuvm:v55
  | public.ecr.aws/firecracker/fcuvm:v55
  | fatal: not a git repository: /firecracker/../../.git/modules/firecracker
  | make: *** [Makefile:348: bin/firecracker] Error 128
  | 🚨 Error: The command exited with status 2
  | user command error: exit status 2

The root cause is the build script release.sh of firecracker v1.3.3 uses git describe to get firecracker version. This won't be an issue if you directly build from firecracker repo. However firecracker-containerd includes firecracker as a submodule, which links firecrcker's .git to firecrecker-containerd's directory.

→ cat .git
gitdir: ../.git/modules/firecracker    

Since firecracker is complied in a docker container, not mounting the git directory from fc-cd but using git command will give us the above error.

Talked to firecracker, they have a fix in the upcoming release. We probably should wait until the new release, and directly upgrade to it.

@fangn2 fangn2 changed the title Upgrade firecracker from v1.1.0 to v1.3.3 Upgrade firecracker from v1.1.0 to v1.4.0 Jul 20, 2023
@fangn2 fangn2 force-pushed the upgrade-fc branch 3 times, most recently from f926eea to 21fe6fa Compare July 20, 2023 14:55
Update firecracker-go-sdk dependency

Remove devtool strip which has been removed in latest firecracker release

Signed-off-by: Tony Fang <[email protected]>
@fangn2 fangn2 changed the title Upgrade firecracker from v1.1.0 to v1.4.0 Upgrade firecracker from v1.1.0 to v1.4.1 Aug 10, 2023
@henry118
Copy link
Member

@fangn2 do you know the reason of the fc kernel build failure?

make: *** [Makefile:363: _submodules/firecracker/build/kernel/linux-4.14/vmlinux-4.14-x86_64.bin] Error 141

This change LGTM if the failure is fixed.

@Kern--
Copy link
Contributor

Kern-- commented Sep 21, 2023

The kernel build failures are coming from here: https://github.com/firecracker-microvm/firecracker/blob/02dd0328589c59ae11b7ad13eaf412410f8e72e1/resources/tests/build_kernel.sh#L45-L49

head exits after reading 1 line which sends SIGPIPE to sort which returns 141. The set -euo pipefail causes the whole script to exit at this point.

It's worth noting that upstream firecracker moved to prebuilt binaries and devtool build_kernel is going to go away on our next update: firecracker-microvm/firecracker#3896

@Kern--
Copy link
Contributor

Kern-- commented Sep 22, 2023

I misread what FC did. They're still building from source, but you can't specify a config any more and it's not built in a container. The actual scripts to build the kernel are pretty small, so I think we can just copy the bits we need and have it all in this project.

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