Upgrade Firecracker to post-v0.20.0 by kzys · Pull Request #383 · firecracker-microvm/firecracker-containerd

kzys · 2020-01-24T00:58:32Z

Issue #, if available:

Fixes #253 finally!

Description of changes:

Upgrade Firecracker to v0.20.0

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

kzys · 2020-01-24T00:59:08Z

I'm going to upgrade Firecracker Go SDK to 0.20.0 (which will be released hopefully this week) in this PR.

kzys · 2020-01-24T17:37:04Z

I'm going to upgrade Go SDK to untagged master for making sure it doesn't break firecracker-containerd.

kzys · 2020-01-24T19:57:44Z

The "failed to create VM: failed to start the VM: invalid character 'v' looking for beginning of value" error seems due to Firecracker. Opening a pull request...

firecracker-microvm/firecracker#1547

kzys · 2020-01-28T18:53:16Z

7f1aa72 is not really related to Firecracker upgrade, but better to fix anyways.

kzys · 2020-01-28T19:44:59Z

I'm not sure the root cause of "vCPUs resume failed". The code below is sending the error and .recv_timeout(Duration::from_millis(100)) looks a bit suspicious. Would it be always 100ms?

https://github.com/firecracker-microvm/firecracker/blob/53cf1bacadc23a21df361a894ac2887cafcb7139/src/vmm/src/lib.rs#L941

"exit status 148" is firecracker-microvm/firecracker#1456 and not a regression technically.

kzys · 2020-01-28T20:50:29Z

I'm going to move Firecracker fixes to upstream. In the meantime, we can say that the Go SDK is working correctly and we can release 0.20.0!

kzys · 2020-01-28T21:16:16Z

Firecracker patches are kept in https://github.com/kzys/firecracker-containerd/tree/upgrade-fc-with-fixes for reference and a successful BuildKite build with the patches is https://buildkite.com/firecracker-microvm/firecracker-containerd/builds/1477. So I'm going to remove them from this PR.

internal/vm/vsock.go

kzys · 2020-02-13T16:50:19Z

Right now 31e7627 is referencing an open pull request. I'd like to merge this pull request after the pull request is merged into master.

kzys · 2020-02-17T17:23:02Z

@mxpv @xibz @IRCody This branch is ready for review! It will fix #253 and #358 finally.

xibz · 2020-02-17T21:21:47Z

internal/vm/vsock.go

 	if err != nil {
 		return nil, vsockAckError{cause: err}
 	}
+	if !strings.HasPrefix(line, "OK ") {


Why not do the full string compare here?

The line contains assigned_hostside_port (see https://github.com/firecracker-microvm/firecracker/blob/master/docs/vsock.md#examples) which we don't know. Also we are not using the number by ourselves.

We can check whether the following token is a number or not, but I'm unsure how much value we could get from doing so.

Ahh, I thought this was the port we were passing in. Can we add the vsock.md link in the comments stating we are following a custom Firecracker protocol?

Sure. Updated b67f582

v0.20.0 has some issues that block firecracker-containerd to use. Signed-off-by: Kazuyoshi Kato <katokazu@amazon.com>

- Rust 1.35 cannot parse Firecracker's Cargo.lock somehow - While Rust's latest release is 1.40.0, we'd like to keep in sync with Firecracker upstream. firecracker-microvm/firecracker#1419 Signed-off-by: Kazuyoshi Kato <katokazu@amazon.com>

kzys · 2020-02-18T00:47:23Z

@xibz Can you take a look the AL build? Seems I cannot retry the job.

kzys · 2020-02-18T01:03:51Z

According to @xibz, we cannot retry AL builds, but we can rebuild
I manually run the following commands on the AL host.

sh-4.2# ./tools/thinpool.sh remove 181
Device /dev/mapper/fcci--vg-181-snap-* not found
Command failed
  Logical volume "181" successfully removed

We may need to check our post-build scripts.

IRCody

LGTM.

internal/vm/vsock.go

Since 0.20.0, Firecracker itself writes "OK <port>" as ACK. So we don't have to write our own "IMALIVE" message anymore. Signed-off-by: Kazuyoshi Kato <katokazu@amazon.com>

Since Firecracker 0.20.0 can report the size of a special block device correctly, the size of the block device from devmapper snapshotter won't be zero. Signed-off-by: Kazuyoshi Kato <katokazu@amazon.com>

The submodule is used to install a specific version of Firecracker on our Docker-based build. Amazon Linux 2 build should use that as well. Signed-off-by: Kazuyoshi Kato <katokazu@amazon.com>

mxpv

LGTM

…ependabot/go_modules/github.com/mdlayher/vsock-1.1.0 Bump github.com/mdlayher/vsock from 1.0.0 to 1.1.0

kzys force-pushed the upgrade-fc branch from 6303213 to dcbf3e2 Compare January 24, 2020 01:34

kzys marked this pull request as ready for review January 24, 2020 17:35

kzys force-pushed the upgrade-fc branch from 7d100e0 to abe8ae7 Compare January 28, 2020 18:13

kzys changed the title ~~Upgrade Firecracker to v0.20.0~~ Upgrade Firecracker to post-v0.20.0 Jan 28, 2020

kzys mentioned this pull request Jan 28, 2020

Occasional KVM_EXIT_SHUTDOWN and bad syscall (14) during VM shutdown firecracker-microvm/firecracker#1456

Closed

kzys force-pushed the upgrade-fc branch from 00c4bca to 7209f70 Compare January 28, 2020 20:49

kzys mentioned this pull request Jan 28, 2020

Occasional "vCPUs resume failed" firecracker-microvm/firecracker#1555

Closed

kzys force-pushed the upgrade-fc branch 2 times, most recently from 4a4954e to 7ce63ed Compare February 3, 2020 18:28

kzys force-pushed the upgrade-fc branch from 7ce63ed to 91221d8 Compare February 5, 2020 16:35

This was referenced Feb 5, 2020

vmm: keep vCPU threads running to avoid calling sigprocmask firecracker-microvm/firecracker#1586

Closed

Don't share err between multiple goroutines #391

Merged

kzys force-pushed the upgrade-fc branch from 913a6c5 to 762197b Compare February 10, 2020 17:44

mxpv reviewed Feb 11, 2020

View reviewed changes

internal/vm/vsock.go Outdated Show resolved Hide resolved

kzys force-pushed the upgrade-fc branch 2 times, most recently from f53f11e to f4e6dbd Compare February 13, 2020 16:25

kzys mentioned this pull request Feb 13, 2020

Change vCPU threads exit behavior in order to avoid sigprocmask firecracker-microvm/firecracker#1603

Merged

8 tasks

kzys force-pushed the upgrade-fc branch from f4e6dbd to fc4110c Compare February 17, 2020 16:53

xibz reviewed Feb 17, 2020

View reviewed changes

Kazuyoshi Kato added 2 commits February 17, 2020 16:39

Upgrade Firecracker to post-0.20.0

c721a57

v0.20.0 has some issues that block firecracker-containerd to use. Signed-off-by: Kazuyoshi Kato <katokazu@amazon.com>

Upgrade Rust to 1.39

f9cf431

- Rust 1.35 cannot parse Firecracker's Cargo.lock somehow - While Rust's latest release is 1.40.0, we'd like to keep in sync with Firecracker upstream. firecracker-microvm/firecracker#1419 Signed-off-by: Kazuyoshi Kato <katokazu@amazon.com>

kzys force-pushed the upgrade-fc branch from fc4110c to 755ac8f Compare February 18, 2020 00:39

IRCody approved these changes Feb 18, 2020

View reviewed changes

internal/vm/vsock.go Outdated Show resolved Hide resolved

kzys force-pushed the upgrade-fc branch from 0c92f3c to 9e9255f Compare February 18, 2020 16:24

Kazuyoshi Kato added 3 commits February 18, 2020 08:42

Support Firecracker 0.20.0's vsock handshake protocol

38e3d39

Since 0.20.0, Firecracker itself writes "OK <port>" as ACK. So we don't have to write our own "IMALIVE" message anymore. Signed-off-by: Kazuyoshi Kato <katokazu@amazon.com>

Fix TestStubBlockDevices_Isolated

871e9fc

Since Firecracker 0.20.0 can report the size of a special block device correctly, the size of the block device from devmapper snapshotter won't be zero. Signed-off-by: Kazuyoshi Kato <katokazu@amazon.com>

Install Firecracker from _submodules/firecracker

3b8d202

The submodule is used to install a specific version of Firecracker on our Docker-based build. Amazon Linux 2 build should use that as well. Signed-off-by: Kazuyoshi Kato <katokazu@amazon.com>

kzys force-pushed the upgrade-fc branch from 9e9255f to 3b8d202 Compare February 18, 2020 16:42

mxpv approved these changes Feb 18, 2020

View reviewed changes

kzys merged commit 0dd32fc into firecracker-microvm:master Feb 18, 2020

fangn2 pushed a commit to fangn2/firecracker-containerd that referenced this pull request Mar 23, 2023

Merge pull request firecracker-microvm#383 from firecracker-microvm/d…

76617bd

…ependabot/go_modules/github.com/mdlayher/vsock-1.1.0 Bump github.com/mdlayher/vsock from 1.0.0 to 1.1.0

Conversation

kzys commented Jan 24, 2020

Uh oh!

kzys commented Jan 24, 2020

Uh oh!

kzys commented Jan 24, 2020

Uh oh!

kzys commented Jan 24, 2020

Uh oh!

kzys commented Jan 28, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kzys commented Jan 28, 2020

Uh oh!

kzys commented Jan 28, 2020

Uh oh!

kzys commented Jan 28, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

kzys commented Feb 13, 2020

Uh oh!

kzys commented Feb 17, 2020

Uh oh!

xibz Feb 17, 2020

Choose a reason for hiding this comment

Uh oh!

kzys Feb 17, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xibz Feb 17, 2020

Choose a reason for hiding this comment

Uh oh!

kzys Feb 18, 2020

Choose a reason for hiding this comment

Uh oh!

kzys commented Feb 18, 2020

Uh oh!

kzys commented Feb 18, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

IRCody left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mxpv left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

kzys commented Jan 28, 2020 •

edited

Loading

kzys commented Jan 28, 2020 •

edited

Loading

kzys Feb 17, 2020 •

edited

Loading

kzys commented Feb 18, 2020 •

edited

Loading