From 0dcb3d1870e6040b70225ea179d8cbffee7becae Mon Sep 17 00:00:00 2001 From: Sudan Landge Date: Thu, 21 Sep 2023 13:47:26 +0000 Subject: [PATCH 1/5] fix: remove default target for cargo The global `target` directive in .cargo/build forced the x86_64-unknown-linux-musl target whenever `--target` flag was not provided and because of this `cargo build` produced x86_64 binaries even if we tried to build on ARM. Though `devtool build` is the documented way to build Firecracker and does not have this issue since it always specifies the right arch `--target`, we would like to fix issues like #3221 and be able to just run `cargo run` without specifying the target. Signed-off-by: Sudan Landge --- .cargo/config | 1 - 1 file changed, 1 deletion(-) diff --git a/.cargo/config b/.cargo/config index 73375de08a2e..a50af8c383cd 100644 --- a/.cargo/config +++ b/.cargo/config @@ -1,6 +1,5 @@ [build] target-dir = "build/cargo_target" -target = "x86_64-unknown-linux-musl" rustflags = [ "-Wclippy::ptr_as_ptr", "-Wclippy::undocumented_unsafe_blocks", From 3c1db9cc3a7a1675e956391511634449fab710d6 Mon Sep 17 00:00:00 2001 From: Sudan Landge Date: Fri, 22 Sep 2023 00:03:56 +0000 Subject: [PATCH 2/5] fix: Add test to check static linking of binary host.get_binary() today returns path of a binary which is compiled with the --release flag and because we use default target as musl this also means that today we this function returns a statically linked release profile binary. Since we use this binary in our CI add a test to check if the Firecracker binary is actually compiled using musl (static link) or gnu (dynamic link). We use `file` command because it is already available in devctr. Note: This test doesn't guarantee that we use a statically linked binary in our Firecracker release, the commit after this handles that. Signed-off-by: Sudan Landge --- .../build/test_binary_static_linking.py | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) create mode 100644 tests/integration_tests/build/test_binary_static_linking.py diff --git a/tests/integration_tests/build/test_binary_static_linking.py b/tests/integration_tests/build/test_binary_static_linking.py new file mode 100644 index 000000000000..fabcb8a66ac0 --- /dev/null +++ b/tests/integration_tests/build/test_binary_static_linking.py @@ -0,0 +1,22 @@ +# Copyright 2019 Amazon.com, Inc. or its affiliates. All Rights Reserved. +# SPDX-License-Identifier: Apache-2.0 +"""Tests to check if the release binary is statically linked. + +""" + +import pytest +import host_tools.cargo_build as host +from framework import utils + +@pytest.mark.timeout(500) +def test_firecracker_binary_static_linking(): + """ + Test to make sure the firecracker binary is statically linked. + """ + fc_binary_path = host.get_binary("firecracker") + _, stdout,stderr = utils.run_cmd(f"file {fc_binary_path}") + assert "" in stderr + # expected "statically linked" for aarch64 and + # "static-pie linked" for x86_64 + assert "statically linked" in stdout or \ + "static-pie linked" in stdout From 93dc8575b233361094a5f1ebdc32cd47fd25ba7f Mon Sep 17 00:00:00 2001 From: Sudan Landge Date: Fri, 22 Sep 2023 00:05:29 +0000 Subject: [PATCH 3/5] fix: test static linking of release binary Make sure that Firecracker release binary is statically linked before making a release. Signed-off-by: Sudan Landge --- tools/release.sh | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/tools/release.sh b/tools/release.sh index 1c84e5106bba..e17ba93b9012 100755 --- a/tools/release.sh +++ b/tools/release.sh @@ -134,6 +134,16 @@ cargo build --target "$CARGO_TARGET" $CARGO_OPTS --workspace say "Binaries placed under $CARGO_TARGET_DIR" +# Check static linking: +# expected "statically linked" for aarch64 and +# "static-pie linked" for x86_64 +binary_format=$(file $CARGO_TARGET_DIR/firecracker) +if [[ "$PROFILE" = "release" + && "$binary_format" != *"statically linked"* + && "$binary_format" != *"static-pie linked"* ]]; then + die "Binary not statically linked: $binary_format" +fi + # # # # Make a release if [ -z "$MAKE_RELEASE" ]; then exit 0 From 00e234c958d1bb575cd910f2d8eab75ddac0281a Mon Sep 17 00:00:00 2001 From: Sudan Landge Date: Fri, 22 Sep 2023 00:06:31 +0000 Subject: [PATCH 4/5] fix:create dummy test-report for release.sh sanity test Release process involves running the command `devtool -y make_release` which runs a bunch of tests first and then runs the release.sh script. The tests create a report at tests/test-report.json which is copied by release.sh to a release folder. The release.sh does not check contents of the report and simply copies it. However, when release.sh is run as part of buildkite's release sanity build no other tests are run and so test-report.json is not available resulting in below error: ``` cp: cannot stat 'tests/test-report.json': No such file or directory ``` So, create a dummy tests/test-report that release.sh can copy and avoid above the error. Signed-off-by: Sudan Landge --- .buildkite/pipeline_pr.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.buildkite/pipeline_pr.py b/.buildkite/pipeline_pr.py index 33828fb097f4..8dd3dadd3d71 100755 --- a/.buildkite/pipeline_pr.py +++ b/.buildkite/pipeline_pr.py @@ -49,7 +49,7 @@ release_grp = group( "📦 Release Sanity Build", - "./tools/devtool -y sh ./tools/release.sh --libc musl --profile release --make-release", + "touch ./tests/test-report.json && ./tools/devtool -y sh ./tools/release.sh --libc musl --profile release --make-release", **defaults_once_per_architecture, ) From 42f986e5cb7ef80b831883b8035cc114daba0000 Mon Sep 17 00:00:00 2001 From: Sudan Landge Date: Fri, 22 Sep 2023 09:12:46 +0000 Subject: [PATCH 5/5] fix: use consistent path for pytest test reports With #41412c1 pytest reports are generated in "test_results/". Update devtool and release.sh to align with this change. Signed-off-by: Sudan Landge --- .buildkite/pipeline_pr.py | 2 +- tools/devtool | 2 +- tools/release.sh | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.buildkite/pipeline_pr.py b/.buildkite/pipeline_pr.py index 8dd3dadd3d71..7ae27854d427 100755 --- a/.buildkite/pipeline_pr.py +++ b/.buildkite/pipeline_pr.py @@ -49,7 +49,7 @@ release_grp = group( "📦 Release Sanity Build", - "touch ./tests/test-report.json && ./tools/devtool -y sh ./tools/release.sh --libc musl --profile release --make-release", + "mkdir -p ./test_results && touch ./test_results/test-report.json && ./tools/devtool -y sh ./tools/release.sh --libc musl --profile release --make-release", **defaults_once_per_architecture, ) diff --git a/tools/devtool b/tools/devtool index f6e990a7aad1..fb20139ff131 100755 --- a/tools/devtool +++ b/tools/devtool @@ -483,7 +483,7 @@ cmd_build() { } function cmd_make_release { - cmd_test -- --json-report --json-report-file=test-report.json || die "Tests failed!" + cmd_test || die "Tests failed!" run_devctr \ --user "$(id -u):$(id -g)" \ diff --git a/tools/release.sh b/tools/release.sh index e17ba93b9012..b5cfc6eeb308 100755 --- a/tools/release.sh +++ b/tools/release.sh @@ -167,7 +167,7 @@ cp -v -t "$RELEASE_DIR" LICENSE NOTICE THIRD-PARTY check_swagger_artifact src/api_server/swagger/firecracker.yaml "$VERSION" cp -v src/api_server/swagger/firecracker.yaml "$RELEASE_DIR/firecracker_spec-$VERSION.yaml" -cp -v tests/test-report.json "$RELEASE_DIR/" +cp -v test_results/test-report.json "$RELEASE_DIR/" ( cd "$RELEASE_DIR"