Skip to content

Upgrade Firecracker to post-v0.20.0 #383

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.

Sign up for GitHub

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 5 commits into from
Feb 18, 2020
Merged

Upgrade Firecracker to post-v0.20.0 #383

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
c721a57
Upgrade Firecracker to post-0.20.0
kzys Jan 24, 2020
f9cf431
Upgrade Rust to 1.39
kzys Jan 24, 2020
38e3d39
Support Firecracker 0.20.0's vsock handshake protocol
kzys Jan 24, 2020
871e9fc
Fix TestStubBlockDevices_Isolated
kzys Jan 24, 2020
3b8d202
Install Firecracker from _submodules/firecracker
kzys Feb 18, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion .buildkite/al2env.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,3 @@ bin_path=$dir/bin
devmapper_path=$dir/devmapper
state_path=$dir/state
runtime_config_path=$dir/firecracker-runtime.json
firecracker_bin=firecracker-v0.19.0
8 changes: 4 additions & 4 deletions .buildkite/setup_al2.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,10 @@ mkdir -p $state_path
export INSTALLROOT=$dir
export FIRECRACKER_CONTAINERD_RUNTIME_DIR=$dir
make
sudo -E INSTALLROOT=$INSTALLROOT PATH=$PATH make install
cp /var/lib/fc-ci/vmlinux.bin $dir/default-vmlinux.bin
make image
sudo -E PATH=$PATH make install-default-rootfs
make image firecracker
sudo -E INSTALLROOT=$INSTALLROOT PATH=$PATH \
make install install-firecracker install-default-rootfs

cat << EOF > $dir/config.toml
disabled_plugins = ["cri"]
Expand All @@ -39,7 +39,7 @@ cat << EOF > $runtime_config_path
{
"cpu_template": "T2",
"debug": true,
"firecracker_binary_path": "/usr/local/bin/$firecracker_bin",
"firecracker_binary_path": "$bin_path/firecracker",
"shim_base_dir": "$dir",
"kernel_image_path": "$dir/default-vmlinux.bin",
"kernel_args": "ro console=ttyS0 noapic reboot=k panic=1 pci=off nomodules systemd.journald.forward_to_console systemd.log_color=false systemd.unit=firecracker.target init=/sbin/overlay-init",
Expand Down
4 changes: 2 additions & 2 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ export GO_CACHE_VOLUME_NAME?=gocache

FIRECRACKER_DIR=$(SUBMODULES)/firecracker
FIRECRACKER_TARGET?=x86_64-unknown-linux-musl
FIRECRACKER_BIN=$(FIRECRACKER_DIR)/target/$(FIRECRACKER_TARGET)/release/firecracker
JAILER_BIN=$(FIRECRACKER_DIR)/target/$(FIRECRACKER_TARGET)/release/jailer
FIRECRACKER_BIN=$(FIRECRACKER_DIR)/build/cargo_target/$(FIRECRACKER_TARGET)/release/firecracker
JAILER_BIN=$(FIRECRACKER_DIR)/build/cargo_target/$(FIRECRACKER_TARGET)/release/jailer
FIRECRACKER_BUILDER_NAME?=firecracker-builder
CARGO_CACHE_VOLUME_NAME?=cargocache

Expand Down
2 changes: 1 addition & 1 deletion _submodules/firecracker
Open in desktop
Submodule firecracker updated 270 files
44 changes: 15 additions & 29 deletions internal/vm/vsock.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,11 @@
package vm

import (
"bufio"
"context"
"fmt"
"net"
"strings"
"time"

"github.com/mdlayher/vsock"
Expand Down Expand Up @@ -184,14 +186,6 @@ func vsockConnectMsg(port uint32) string {
return fmt.Sprintf("CONNECT %d\n", port)
}

func vsockAckMsg(port uint32) string {
// The message a guest-side connection will write after accepting a connection from
// a host dial. This is not part of the official Firecracker vsock spec, but is
// recommended in order to allow the host to verify connections were established
// successfully: https://github.com/firecracker-microvm/firecracker/issues/1272#issuecomment-533004066
return fmt.Sprintf("IMALIVE %d\n", port)
}

// tryConnect attempts to dial a guest vsock listener at the provided host-side
// unix socket and provided guest-listener port.
func tryConnect(logger *logrus.Entry, udsPath string, port uint32) (net.Conn, error) {
Expand All @@ -215,10 +209,18 @@ func tryConnect(logger *logrus.Entry, udsPath string, port uint32) (net.Conn, er
return nil, vsockConnectMsgError{cause: err}
}

err = tryConnRead(conn, vsockAckMsg(port), vsockAckMsgTimeout)
line, err := tryConnReadUntil(conn, '\n', vsockAckMsgTimeout)
if err != nil {
return nil, vsockAckError{cause: err}
}

// The line would be "OK <assigned_hostside_port>\n", but we don't use the hostside port here.
// https://github.com/firecracker-microvm/firecracker/blob/master/docs/vsock.md#host-initiated-connections
if !strings.HasPrefix(line, "OK ") {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not do the full string compare here?

Copy link
Contributor Author

@kzys kzys Feb 17, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure. Updated b67f582

return nil, vsockAckError{
cause: errors.Errorf(`expected to read "OK <port>", but instead read %q`, line),
}
}
return conn, nil
}

Expand All @@ -240,35 +242,19 @@ func tryAccept(logger *logrus.Entry, listener net.Listener, port uint32) (net.Co
}
}()

err = tryConnWrite(conn, vsockAckMsg(port), vsockAckMsgTimeout)
if err != nil {
return nil, vsockAckError{cause: err}
}

return conn, nil
}

// tryConnRead will try to do a read from the provided conn, returning an error if
// the bytes read does not match what was provided or if the read does not complete
// tryConnReadUntil will try to do a read from the provided conn until the specified
// end character is encounteed. Returning an error if the read does not complete
// within the provided timeout. It will reset socket deadlines to none after returning.
// It's only intended to be used for connect/ack messages, not general purpose reads
// after the vsock connection is established fully.
func tryConnRead(conn net.Conn, expectedRead string, timeout time.Duration) error {
func tryConnReadUntil(conn net.Conn, end byte, timeout time.Duration) (string, error) {
conn.SetDeadline(time.Now().Add(timeout))
defer conn.SetDeadline(time.Time{})

actualRead := make([]byte, len(expectedRead))
_, err := conn.Read(actualRead)
if err != nil {
return err
}

if expectedRead != string(actualRead) {
return errors.Errorf("expected to read %q, but instead read %q",
expectedRead, string(actualRead))
}

return nil
return bufio.NewReaderSize(conn, 32).ReadString(end)
}

// tryConnWrite will try to do a write to the provided conn, returning an error if
Expand Down
10 changes: 5 additions & 5 deletions runtime/service_integ_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -734,11 +734,11 @@ func TestStubBlockDevices_Isolated(t *testing.T) {
}

const expectedOutput = `
vdb 254:16 0 0B 0 |
vdc 254:32 0 512B 0 | 214 244 216 245 215 177 177 177
vdd 254:48 0 512B 0 | 214 244 216 245 215 177 177 177
vde 254:64 0 512B 0 | 214 244 216 245 215 177 177 177
vdf 254:80 0 512B 0 | 214 244 216 245 215 177 177 177`
vdb 254:16 0 1073741824B 0 | 0 0 0 0 0 0 0 0
vdc 254:32 0 512B 0 | 214 244 216 245 215 177 177 177
vdd 254:48 0 512B 0 | 214 244 216 245 215 177 177 177
vde 254:64 0 512B 0 | 214 244 216 245 215 177 177 177
vdf 254:80 0 512B 0 | 214 244 216 245 215 177 177 177`

parts := strings.Split(stdout.String(), "vdb")
require.Equal(t, strings.TrimSpace(expectedOutput), strings.TrimSpace("vdb"+parts[1]))
Expand Down
2 changes: 1 addition & 1 deletion tools/docker/Dockerfile.firecracker-builder
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
# express or implied. See the License for the specific language governing
# permissions and limitations under the License.

FROM rust:1.35-stretch
FROM rust:1.39-stretch

ENV DEBIAN_FRONTEND="noninteractive"
RUN apt-get update && apt-get install --yes --no-install-recommends \
Expand Down
2 changes: 1 addition & 1 deletion tools/docker/scripts/lsblk.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ do
# https://github.com/firecracker-microvm/firecracker-containerd/blob/2578f3df9d899aa48decb39c9f7f23fa41635ede/internal/common.go#L67
magic=$(head -c 8 /dev/$name | od -A n -t u1)

printf "%-4s %-7s %2d %8dB %2d | %s\n" \
printf "%-4s %-7s %2d %10dB %2d | %s\n" \
"$name" \
$(cat /sys/block/$name/dev) \
$(cat /sys/block/$name/removable) \
Expand Down