Merge pull request #334 from kzys/fix-build

Fix BuildKite's race condition
firecracker-microvm · Mar 31, 2021 · b453d4a · b453d4a
2 parents 20161d1 + ff4933f
commit b453d4a
Show file tree

Hide file tree

Showing 5 changed files with 131 additions and 89 deletions.
diff --git a/.buildkite/pipeline.yml b/.buildkite/pipeline.yml
@@ -12,12 +12,14 @@
 # permissions and limitations under the License.
 env:
   PATH: "/usr/lib/go-1.13/bin:/usr/bin"
+  FC_TEST_DATA_PATH: "/tmp/buildkite_build_${BUILDKITE_BUILD_NUMBER}_testdata"
 
 steps:
-  - label: ':ec2: environment'
+  - label: ':ec2: prepare environment'
     command:
       - uname -a
       - go version
+      - "mkdir -p ${FC_TEST_DATA_PATH}"
     agents:
       queue: "${BUILDKITE_AGENT_META_DATA_QUEUE:-default}"
       distro: "${BUILDKITE_AGENT_META_DATA_DISTRO}"
@@ -40,18 +42,17 @@ steps:
       distro: "${BUILDKITE_AGENT_META_DATA_DISTRO}"
       hostname: "${BUILDKITE_AGENT_META_DATA_HOSTNAME}"
 
+  # Since make test-images mutates the source directory, it cannot be run with other steps.
+  - wait
+
   - label: ':linux: build docker images'
     commands:
-      - 'make test-images'
-      - 'buildkite-agent artifact upload testdata/firecracker-master'
-      - 'buildkite-agent artifact upload testdata/jailer-master'
+      - make test-images
     agents:
       queue: "${BUILDKITE_AGENT_META_DATA_QUEUE:-default}"
       distro: "${BUILDKITE_AGENT_META_DATA_DISTRO}"
       hostname: "${BUILDKITE_AGENT_META_DATA_HOSTNAME}"
 
-  # We use a "wait" step here, because Go's module logic freaks out when
-  # multiple go builds are downloading to the same cache.
   - wait
 
   - label: gofmt -s
@@ -68,19 +69,29 @@ steps:
     agents:
       queue: "${BUILDKITE_AGENT_META_DATA_QUEUE:-default}"
 
+  - wait
+
   - label: ':package: install'
+    env:
+      GOBIN: "$FC_TEST_DATA_PATH/bin"
     commands:
+      # To use ${FC_TEST_DATA_PATH} as testdata/, all files in the original directory must be
+      # copied to the new directory.
+      - cp -r testdata/* ${FC_TEST_DATA_PATH}
       # Install tc-redirect-tap.
-      - 'mkdir -p testdata/bin'
-      - 'GOBIN=$(pwd)/testdata/bin go get github.com/awslabs/tc-redirect-tap/cmd/tc-redirect-tap'
+      - 'go get github.com/awslabs/tc-redirect-tap/cmd/tc-redirect-tap'
       # Copy vmlinux and root-drive.img.
-      - 'ln -s /var/lib/fc-ci/vmlinux.bin testdata/vmlinux'
-      - 'ln -s /var/lib/fc-ci/rootfs.ext4 testdata/root-drive.img'
+      - ln -s /var/lib/fc-ci/vmlinux.bin ${FC_TEST_DATA_PATH}/vmlinux
+      - ln -s /var/lib/fc-ci/rootfs.ext4 ${FC_TEST_DATA_PATH}/root-drive.img
+      # Download Firecracker and its jailer.
+      - make deps
     agents:
       queue: "${BUILDKITE_AGENT_META_DATA_QUEUE:-default}"
       distro: "${BUILDKITE_AGENT_META_DATA_DISTRO}"
       hostname: "${BUILDKITE_AGENT_META_DATA_HOSTNAME}"
 
+  - wait
+
   - label: 'build'
     commands:
       - 'make'
@@ -89,11 +100,8 @@ steps:
       distro: "${BUILDKITE_AGENT_META_DATA_DISTRO}"
       hostname: "${BUILDKITE_AGENT_META_DATA_HOSTNAME}"
 
-
   - label: ':hammer: tests'
     commands:
-      - 'ln -s /var/lib/fc-ci/vmlinux.bin testdata/vmlinux'
-      - 'ln -s /var/lib/fc-ci/rootfs.ext4 testdata/root-drive.img'
       - "FC_TEST_TAP=fc-test-tap${BUILDKITE_BUILD_NUMBER} make test EXTRAGOARGS='-v -count=1 -race' DISABLE_ROOT_TESTS=true"
     agents:
       queue: "${BUILDKITE_AGENT_META_DATA_QUEUE:-default}"
@@ -102,31 +110,18 @@ steps:
 
   - label: ':hammer: root tests'
     commands:
-      - 'ln -s /var/lib/fc-ci/vmlinux.bin testdata/vmlinux'
-      - 'ln -s /var/lib/fc-ci/rootfs.ext4 testdata/root-drive.img'
-      - 'mkdir -p $(pwd)/testdata/bin'
-      - 'GOBIN=$(pwd)/testdata/bin go get github.com/awslabs/tc-redirect-tap/cmd/tc-redirect-tap'
-      - "sudo PATH=$PATH FC_TEST_TAP=fc-root-tap${BUILDKITE_BUILD_NUMBER} make test EXTRAGOARGS='-v -count=1 -race' DISABLE_ROOT_TESTS="
+      - "sudo PATH=$PATH FC_TEST_TAP=fc-root-tap${BUILDKITE_BUILD_NUMBER} FC_TEST_DATA_PATH=${FC_TEST_DATA_PATH} make test EXTRAGOARGS='-v -count=1 -race' DISABLE_ROOT_TESTS="
     agents:
       queue: "${BUILDKITE_AGENT_META_DATA_QUEUE:-default}"
       distro: "${BUILDKITE_AGENT_META_DATA_DISTRO}"
       hostname: "${BUILDKITE_AGENT_META_DATA_HOSTNAME}"
 
   - label: ':hammer: test against firecracker master'
     env:
-      FC_TEST_BIN: "testdata/firecracker-master"
+      FC_TEST_BIN: "${FC_TEST_DATA_PATH}/firecracker-master"
+      FC_TEST_JAILER_BIN: "${FC_TEST_DATA_PATH}/jailer-master"
       DOCKER_IMAGE_TAG: "$BUILDKITE_BUILD_NUMBER"
-      FC_TEST_JAILER_BIN: "testdata/jailer-master"
     commands:
-      - 'ln -s /var/lib/fc-ci/vmlinux.bin testdata/vmlinux'
-      - 'ln -s /var/lib/fc-ci/rootfs.ext4 testdata/root-drive.img'
-      - 'buildkite-agent artifact download testdata/firecracker-master .'
-      - 'chmod +x testdata/firecracker-master'
-      - 'buildkite-agent artifact download testdata/jailer-master .'
-      - 'chmod +x testdata/jailer-master'
-      # Install tc-redirect-tap.
-      - 'mkdir -p testdata/bin'
-      - 'GOBIN=$(pwd)/testdata/bin go get github.com/awslabs/tc-redirect-tap/cmd/tc-redirect-tap'
       - "sudo -E PATH=$PATH FC_TEST_TAP=fc-mst-tap${BUILDKITE_BUILD_NUMBER} make test EXTRAGOARGS='-v -count=1 -race' DISABLE_ROOT_TESTS="
     agents:
       queue: "${BUILDKITE_AGENT_META_DATA_QUEUE:-default}"
@@ -148,11 +143,12 @@ steps:
   - wait: ~
     continue_on_failure: true
 
-  - label: ':linux: cleanup taps'
+  - label: ':linux: cleanup'
     commands:
       - 'sudo ip tuntap del fc-test-tap${BUILDKITE_BUILD_NUMBER} mode tap'
       - 'sudo ip tuntap del fc-root-tap${BUILDKITE_BUILD_NUMBER} mode tap'
       - 'sudo ip tuntap del fc-mst-tap${BUILDKITE_BUILD_NUMBER} mode tap'
+      - 'sudo rm -fr ${FC_TEST_DATA_PATH}'
     agents:
       queue: "${BUILDKITE_AGENT_META_DATA_QUEUE:-default}"
       distro: "${BUILDKITE_AGENT_META_DATA_DISTRO}"

diff --git a/Makefile b/Makefile
@@ -17,10 +17,12 @@ DISABLE_ROOT_TESTS?=1
 DOCKER_IMAGE_TAG?=latest
 EXTRAGOARGS:=
 FIRECRACKER_BUILDER_NAME=firecracker-builder
-FIRECRACKER_BIN=testdata/firecracker-master
-JAILER_BIN=testdata/jailer-master
 FIRECRACKER_TARGET?=x86_64-unknown-linux-musl
 
+FC_TEST_DATA_PATH?=testdata
+FIRECRACKER_BIN=$(FC_TEST_DATA_PATH)/firecracker-master
+JAILER_BIN=$(FC_TEST_DATA_PATH)/jailer-master
+
 UID = $(shell id -u)
 GID = $(shell id -g)
 
@@ -29,7 +31,7 @@ arch=$(shell uname -m)
 
 # The below files are needed and can be downloaded from the internet
 release_url=https://github.com/firecracker-microvm/firecracker/releases/download/$(firecracker_version)/firecracker-$(firecracker_version)-$(arch).tgz
-testdata_objects = testdata/vmlinux testdata/root-drive.img testdata/binaries testdata/jailer testdata/firecracker
+testdata_objects = $(FC_TEST_DATA_PATH)/vmlinux $(FC_TEST_DATA_PATH)/root-drive.img $(FC_TEST_DATA_PATH)/jailer $(FC_TEST_DATA_PATH)/firecracker
 testdata_dir = testdata/firecracker.tgz testdata/firecracker_spec-$(firecracker_version).yaml testdata/LICENSE testdata/NOTICE testdata/THIRD-PARTY
 
 # --location is needed to follow redirects on github.com
@@ -39,10 +41,10 @@ all: build
 
 test: all-tests
 
-unit-tests: $(testdata_objects) check-kvm
+unit-tests: check-kvm $(testdata_objects)
 	DISABLE_ROOT_TESTS=$(DISABLE_ROOT_TESTS) go test -short ./... $(EXTRAGOARGS)
 
-all-tests: $(testdata_objects) check-kvm
+all-tests: check-kvm $(testdata_objects)
 	DISABLE_ROOT_TESTS=$(DISABLE_ROOT_TESTS) go test ./... $(EXTRAGOARGS)
 
 check-kvm:
@@ -60,19 +62,20 @@ distclean: clean
 	rm -rfv $(testdata_dir)
 	docker volume rm -f $(CARGO_CACHE_VOLUME_NAME)
 
-testdata/vmlinux:
+deps: $(testdata_objects)
+
+$(FC_TEST_DATA_PATH)/vmlinux:
 	$(curl) -o $@ https://s3.amazonaws.com/spec.ccfc.min/img/quickstart_guide/$(arch)/kernels/vmlinux.bin
 
-testdata/binaries:
-	$(curl) -o testdata/firecracker.tgz ${release_url}
-	tar -xvzf testdata/firecracker.tgz -C ./testdata
-	mv testdata/firecracker-$(firecracker_version)-$(arch) testdata/firecracker
-	mv testdata/jailer-$(firecracker_version)-$(arch) testdata/jailer
-	chmod +x testdata/firecracker
-	chmod +x testdata/jailer
-	touch testdata/binaries
+$(FC_TEST_DATA_PATH)/firecracker $(FC_TEST_DATA_PATH)/jailer: $(FC_TEST_DATA_PATH)/fc.stamp
+
+$(FC_TEST_DATA_PATH)/fc.stamp:
+	$(curl) ${release_url} | tar -xvzf - -C $(FC_TEST_DATA_PATH)
+	mv $(FC_TEST_DATA_PATH)/firecracker-$(firecracker_version)-$(arch) $(FC_TEST_DATA_PATH)/firecracker
+	mv $(FC_TEST_DATA_PATH)/jailer-$(firecracker_version)-$(arch) $(FC_TEST_DATA_PATH)/jailer
+	touch $@
 
-testdata/root-drive.img:
+$(FC_TEST_DATA_PATH)/root-drive.img:
 	$(curl) -o $@ https://s3.amazonaws.com/spec.ccfc.min/img/hello/fsfiles/hello-rootfs.ext4
 
 tools/firecracker-builder-stamp: tools/docker/Dockerfile
@@ -109,4 +112,4 @@ firecracker-clean:
 		cargo clean
 	- rm $(FIRECRACKER_BIN) $(JAILER_BIN)
 
-.PHONY: all generate clean distclean build test unit-tests all-tests check-kvm
+.PHONY: all generate clean distclean build test unit-tests all-tests check-kvm
diff --git a/firecracker_test.go b/firecracker_test.go
@@ -2,7 +2,6 @@ package firecracker
 
 import (
 	"context"
-	"os"
 	"path/filepath"
 	"testing"
 	"time"
@@ -17,7 +16,8 @@ func TestClient(t *testing.T) {
 	}
 
 	ctx := context.Background()
-	socketpath := filepath.Join(testDataPath, "test.socket")
+	socketpath, cleanup := makeSocketPath(t)
+	defer cleanup()
 
 	cmd := VMCommandBuilder{}.
 		WithBin(getFirecrackerBinaryPath()).
@@ -32,7 +32,6 @@ func TestClient(t *testing.T) {
 		if err := cmd.Process.Kill(); err != nil {
 			t.Errorf("failed to kill process: %v", err)
 		}
-		os.Remove(socketpath)
 	}()
 
 	drive := &models.Drive{