Clean up build output.

This change also simplifies and documents the build_cmd pipeline, and reduces general noise for debugging Makefile issues. It also drops the mapping for /etc/docker/daemon.json, which if it does not exist initially will create this as a directory (causing lots of confusion and breaks). PiperOrigin-RevId: 343932456
2020-11-23 14:43:42 -08:00 · 2020-11-23 14:43:42 -08:00 · 756bc3e52b
parent 3deb5d0c04
commit 756bc3e52b
6 changed files with 112 additions and 54 deletions
--- a/.gitignore
+++ b/.gitignore
@ -1,2 +1,4 @@
 # Generated bazel symlinks.
-/bazel-*
+/bazel-*
+# Generated build event file.
+/.build_events.json
--- a/77
+++ b/77
@ -15,8 +15,16 @@
 # limitations under the License.

 # Helpful pretty-printer.
-MAKEBANNER := \033[1;34mmake\033[0m
-submake = echo -e '$(MAKEBANNER) $1' >&2; $(MAKE) $1
+ifeq (0,$(MAKELEVEL))
+OPENLAST := || (rc=$$?; echo '^^^ +++' >&2; exit $$rc)
+else
+OPENLAST :=
+endif
+CMDLINE := $(shell cut -d '' -f2- /proc/$$PPID/cmdline | sed 's|\x00| |g')
+submake = echo '--- make $1' >&2 && \
+  $(MAKE) -s $1 && \
+  echo '--- make $(CMDLINE) (resume)' >&2 \
+  $(OPENLAST)

 # Described below.
 OPTIONS :=
@ -109,6 +117,13 @@ list-images: ## List all available images.
 ##   convenient entrypoints for testing changes. If you're adding a
 ##   new subsystem or workflow, consider adding a new target here.
 ##
+##   Some targets support a PARTITION (1-indexed) and TOTAL_PARTITIONS
+##   environment variables for high-level test sharding. Unlike most
+##   other variables, these are sourced from the environment.
+##
+PARTITION        ?= 1
+TOTAL_PARTITIONS ?= 1
+
 runsc: ## Builds the runsc binary.
 	@$(call submake,build OPTIONS="-c opt" TARGETS="//runsc")
 .PHONY: runsc
@ -156,12 +171,6 @@ syscall-tests: ## Run all system call tests.
 	@$(call submake,test TARGETS="test/syscalls/...")

 %-runtime-tests: load-runtimes_%
-ifeq ($(PARTITION),)
-	@$(eval PARTITION := 1)
-endif
-ifeq ($(TOTAL_PARTITIONS),)
-	@$(eval TOTAL_PARTITIONS := 1)
-endif
 	@$(call submake,install-runtime)
 	@$(call submake,test-runtime OPTIONS="--test_timeout=10800 --test_arg=--partition=$(PARTITION) --test_arg=--total_partitions=$(TOTAL_PARTITIONS)" TARGETS="//test/runtimes:$*")

@ -233,22 +242,22 @@ iptables-runsc-tests: load-iptables

 packetdrill-tests: load-packetdrill
 	@$(call submake,install-runtime RUNTIME="packetdrill")
-	@$(call submake,test-runtime RUNTIME="packetdrill" TARGETS="$(shell $(MAKE) query TARGETS='attr(tags, packetdrill, tests(//...))')")
+	@$(call submake,test-runtime RUNTIME="packetdrill" TARGETS="$(shell $(MAKE) -s query TARGETS='attr(tags, packetdrill, tests(//...))')")
 .PHONY: packetdrill-tests

 packetimpact-tests: load-packetimpact
 	@sudo modprobe iptable_filter
 	@sudo modprobe ip6table_filter
 	@$(call submake,install-runtime RUNTIME="packetimpact")
-	@$(call submake,test-runtime OPTIONS="--jobs=HOST_CPUS*3 --local_test_jobs=HOST_CPUS*3" RUNTIME="packetimpact" TARGETS="$(shell $(MAKE) query TARGETS='attr(tags, packetimpact, tests(//...))')")
+	@$(call submake,test-runtime OPTIONS="--jobs=HOST_CPUS*3 --local_test_jobs=HOST_CPUS*3" RUNTIME="packetimpact" TARGETS="$(shell $(MAKE) -s query TARGETS='attr(tags, packetimpact, tests(//...))')")
 .PHONY: packetimpact-tests

 # Specific containerd version tests.
 containerd-test-%: load-basic_alpine load-basic_python load-basic_busybox load-basic_resolv load-basic_httpd load-basic_ubuntu
 	@$(call submake,install-runtime RUNTIME="root")
-	@CONTAINERD_VERSION=$* $(MAKE) sudo TARGETS="tools/installers:containerd"
-	@$(MAKE) sudo TARGETS="tools/installers:shim"
-	@$(MAKE) sudo TARGETS="test/root:root_test" ARGS="--runtime=root -test.v"
+	@CONTAINERD_VERSION=$* $(MAKE) -s sudo TARGETS="tools/installers:containerd"
+	@$(MAKE) -s sudo TARGETS="tools/installers:shim"
+	@$(MAKE) -s sudo TARGETS="test/root:root_test" ARGS="--runtime=root -test.v"

 # Note that we can't run containerd-test-1.1.8 tests here.
 #
@ -284,36 +293,37 @@ BENCHMARKS_UPLOAD    := false
 BENCHMARKS_OFFICIAL  := false
 BENCHMARKS_PLATFORMS := ptrace
 BENCHMARKS_TARGETS   := //test/benchmarks/base:startup_test
-BENCHMARKS_ARGS      := -test.bench=.
+BENCHMARKS_ARGS      := -test.bench=. -pprof-cpu -pprof-heap -pprof-heap -pprof-block

 init-benchmark-table: ## Initializes a BigQuery table with the benchmark schema
 ## (see //tools/bigquery/bigquery.go). If the table alread exists, this is a noop.
 	$(call submake, run TARGETS=//tools/parsers:parser ARGS="init --project=$(BENCHMARKS_PROJECT) \
-	--dataset=$(BENCHMARKS_DATASET) --table=$(BENCHMARKS_TABLE)")
+	  --dataset=$(BENCHMARKS_DATASET) --table=$(BENCHMARKS_TABLE)")
 .PHONY: init-benchmark-table

 benchmark-platforms: load-benchmarks-images ## Runs benchmarks for runc and all given platforms in BENCHMARK_PLATFORMS.
-	$(call submake, run-benchmark RUNTIME="runc")
 	$(foreach PLATFORM,$(BENCHMARKS_PLATFORMS), \
-		$(call submake,install-runtime RUNTIME="$(PLATFORM)" ARGS="--platform=$(PLATFORM) --vfs2") && \
-		$(call submake,run-benchmark RUNTIME="$(PLATFORM)") && \
-		$(call submake,install-runtime RUNTIME="$(PLATFORM)_vfs1" ARGS="--platform=$(PLATFORM)") && \
-		$(call submake,run-benchmark RUNTIME="$(PLATFORM)_vfs1") && \
+	  $(call submake,run-benchmark RUNTIME="$(PLATFORM)" ARGS="--platform=$(PLATFORM) --vfs2") && \
+	  $(call submake,run-benchmark RUNTIME="$(PLATFORM)_vfs1" ARGS="--platform=$(PLATFORM)") && \
 	) \
-	true
+	$(call submake, run-benchmark RUNTIME="runc")
 .PHONY: benchmark-platforms

-run-benchmark: ## Runs single benchmark and optionally sends data to BigQuery.
-	@set -xeuo pipefail; 	T=$$(mktemp --tmpdir logs.$(RUNTIME).XXXXXX); \
-	$(call submake,sudo TARGETS="$(BENCHMARKS_TARGETS)" ARGS="--runtime=$(RUNTIME) $(BENCHMARKS_ARGS)" | tee $$T); \
-	if [[ "$(BENCHMARKS_UPLOAD)" == "true" ]]; then \
-		$(call submake,run TARGETS=tools/parsers:parser ARGS="parse --debug --file=$$T \
-			--runtime=$(RUNTIME) --suite_name=$(BENCHMARKS_SUITE) \
-			--project=$(BENCHMARKS_PROJECT) --dataset=$(BENCHMARKS_DATASET) \
-			--table=$(BENCHMARKS_TABLE) --official=$(BENCHMARKS_OFFICIAL)"); \
+run-benchmark: load-benchmarks-images ## Runs single benchmark and optionally sends data to BigQuery.
+	@if [[ "$(RUNTIME)" != "runc" ]]; then $(call submake,install-runtime ARGS="$(ARGS) --profile"); fi
+	@T=$$(mktemp --tmpdir logs.$(RUNTIME).XXXXXX); \
+	$(call submake,sudo TARGETS="$(BENCHMARKS_TARGETS)" ARGS="--runtime=$(RUNTIME) $(BENCHMARKS_ARGS) | tee $$T"); \
+	rc=$$?; \
+	if [[ $$rc -eq 0 ]] && [[ "$(BENCHMARKS_UPLOAD)" == "true" ]]; then \
+	  $(call submake,run TARGETS="tools/parsers:parser" ARGS="parse --debug --file=$$T \
+	    --runtime=$(RUNTIME) --suite_name=$(BENCHMARKS_SUITE) \
+	    --project=$(BENCHMARKS_PROJECT) --dataset=$(BENCHMARKS_DATASET) \
+	    --table=$(BENCHMARKS_TABLE) --official=$(BENCHMARKS_OFFICIAL)"); \
 	fi; \
-	rm -rf $$T
+	rm -rf $$T; \
+	exit $$rc
 .PHONY: run-benchmark
+.PHONY: load-benchmarks-images

 ##
 ## Website & documentation helpers.
@ -419,7 +429,7 @@ RUNTIME_LOG_DIR := $(RUNTIME_DIR)/logs
 RUNTIME_LOGS    := $(RUNTIME_LOG_DIR)/runsc.log.%TEST%.%TIMESTAMP%.%COMMAND%

 dev: ## Installs a set of local runtimes. Requires sudo.
-	@$(call submake,refresh ARGS="--net-raw")
+	@$(call submake,refresh)
 	@$(call submake,configure RUNTIME_NAME="$(RUNTIME)" ARGS="--net-raw")
 	@$(call submake,configure RUNTIME_NAME="$(RUNTIME)-d" ARGS="--net-raw --debug --strace --log-packets")
 	@$(call submake,configure RUNTIME_NAME="$(RUNTIME)-p" ARGS="--net-raw --profile")
@ -433,9 +443,8 @@ refresh: ## Refreshes the runtime binary (for development only). Must have calle
 .PHONY: refresh

 install-runtime: ## Installs the runtime for testing. Requires sudo.
-	@$(call submake,refresh ARGS="--net-raw --TESTONLY-test-name-env=RUNSC_TEST_NAME $(ARGS)")
-	@$(call submake,configure RUNTIME_NAME=runsc)
-	@$(call submake,configure RUNTIME_NAME="$(RUNTIME)")
+	@$(call submake,refresh)
+	@$(call submake,configure RUNTIME_NAME="$(RUNTIME)" ARGS="$(ARGS) --TESTONLY-test-name-env=RUNSC_TEST_NAME")
 	@sudo systemctl restart docker
 	@if [[ -f /etc/docker/daemon.json ]]; then \
 		sudo chmod 0755 /etc/docker && \
--- a/6
+++ b/6
@ -38,6 +38,12 @@ http_archive(

 http_archive(
    name = "bazel_gazelle",
+    patch_args = ["-p1"],
+    patches = [
+        # False positive output complaining about Go logrus versions spam the
+        # logs. Strip this message in this case. Does not affect control flow.
+        "//tools:bazel_gazelle.patch",
+    ],
    sha256 = "b85f48fa105c4403326e9525ad2b2cc437babaa6e15a3fc0b1dbab0ab064bc7c",
    urls = [
        "https://mirror.bazel.build/github.com/bazelbuild/bazel-gazelle/releases/download/v0.22.2/bazel-gazelle-v0.22.2.tar.gz",
--- a/images/Makefile
+++ b/images/Makefile
@ -36,13 +36,13 @@ list-all-images:

 # Handy wrapper to allow load-all-images, push-all-images, etc.
 %-all-images:
-	@$(MAKE) $(patsubst %,$*-%,$(ALL_IMAGES))
+	@$(MAKE) -s $(patsubst %,$*-%,$(ALL_IMAGES))
 load-all-images:
-	@$(MAKE) $(patsubst %,load-%,$(ALL_IMAGES))
+	@$(MAKE) -s $(patsubst %,load-%,$(ALL_IMAGES))

 # Handy wrapper to load specified "groups", e.g. load-basic-images, etc.
 load-%-images:
-	@$(MAKE) $(patsubst %,load-%,$(subst /,_,$(subst ./,,$(shell find ./$* -name Dockerfile -exec dirname {} \;))))
+	@$(MAKE) -s $(patsubst %,load-%,$(subst /,_,$(subst ./,,$(shell find ./$* -name Dockerfile -exec dirname {} \;))))

 # tag is a function that returns the tag name, given an image.
 #
@ -83,7 +83,7 @@ pull-%:
 # entrypoint, as it should never fail. The local tag should always be set after
 # this returns (either by the pull or the build).
 load-%:
-	$(MAKE) pull-$* || $(MAKE) rebuild-$*
+	$(MAKE) -s pull-$* || $(MAKE) -s rebuild-$*
 	docker tag $(call remote_image,$*) $(call local_image,$*)

 # push pushes the remote image, after either pulling (to validate that the tag
--- a/tools/bazel.mk
+++ b/tools/bazel.mk
@ -36,11 +36,11 @@ DOCKER_PRIVILEGED := --privileged
 BAZEL_CACHE := $(shell readlink -m ~/.cache/bazel/)
 GCLOUD_CONFIG := $(shell readlink -m ~/.config/gcloud/)
 DOCKER_SOCKET := /var/run/docker.sock
-DOCKER_CONFIG := /etc/docker/daemon.json
+DOCKER_CONFIG := /etc/docker

 # Bazel flags.
 BAZEL := bazel $(STARTUP_OPTIONS)
-OPTIONS += --color=no --curses=no
+BASE_OPTIONS := --color=no --curses=no

 # Basic options.
 UID := $(shell id -u ${USER})
@ -72,7 +72,7 @@ endif
 # out of disk space.
 ifneq ($(UID),0)
 USERADD_DOCKER += useradd -l --uid $(UID) --non-unique --no-create-home \
-                    --gid $(GID) $(USERADD_OPTIONS) -d $(HOME) $(USER) &&
+  --gid $(GID) $(USERADD_OPTIONS) -d $(HOME) $(USER) &&
 endif
 ifneq ($(GID),0)
 GROUPADD_DOCKER += groupadd --gid $(GID) --non-unique $(USER) &&
@ -81,8 +81,6 @@ endif
 # Add docker passthrough options.
 ifneq ($(DOCKER_PRIVILEGED),)
 FULL_DOCKER_RUN_OPTIONS += -v "$(DOCKER_SOCKET):$(DOCKER_SOCKET)"
-# TODO(gvisor.dev/issue/1624): Remove docker config volume. This is required
-# temporarily for checking VFS1 vs VFS2 by some tests.
 FULL_DOCKER_RUN_OPTIONS += -v "$(DOCKER_CONFIG):$(DOCKER_CONFIG)"
 FULL_DOCKER_RUN_OPTIONS += $(DOCKER_PRIVILEGED)
 FULL_DOCKER_EXEC_OPTIONS += $(DOCKER_PRIVILEGED)
@ -161,20 +159,30 @@ bazel-alias: ## Emits an alias that can be used within the shell.
 .PHONY: bazel-alias

 bazel-server: ## Ensures that the server exists. Used as an internal target.
-	@docker exec $(FULL_DOCKER_EXEC_OPTIONS) $(DOCKER_NAME) true || $(MAKE) bazel-server-start
+	@docker exec $(FULL_DOCKER_EXEC_OPTIONS) $(DOCKER_NAME) true >&2 || $(MAKE) bazel-server-start >&2
 .PHONY: bazel-server

-build_cmd = docker exec $(FULL_DOCKER_EXEC_OPTIONS) $(DOCKER_NAME) sh -o pipefail -c '$(BAZEL) build $(OPTIONS) "$(TARGETS)"'
+# build_cmd builds the given targets in the bazel-server container.
+build_cmd = docker exec $(FULL_DOCKER_EXEC_OPTIONS) $(DOCKER_NAME) sh -o pipefail -c \
+  '$(BAZEL) build $(BASE_OPTIONS) $(OPTIONS) "$(TARGETS)"'

-build_paths = $(build_cmd) 2>&1 \
-		| tee /proc/self/fd/2 \
+# build_paths extracts the built binary from the bazel stderr output.
+#
+# This could be alternately done by parsing the bazel build event stream, but
+# this is a complex schema, and begs the question: what will build the thing
+# that parses the output? Bazel? Do we need a separate bootstrapping build
+# command here? Yikes, let's just stick with the ugly shell pipeline.
+#
+# The last line is used to prevent terminal shenanigans.
+build_paths = command_line=$$( $(build_cmd) 2>&1 \
 		| grep -A1 -E '^Target' \
 		| grep -E '^  ($(subst $(SPACE),|,$(BUILD_ROOTS)))' \
 		| sed "s/ /\n/g" \
 		| strings -n 10 \
 		| awk '{$$1=$$1};1' \
 		| xargs -n 1 -I {} readlink -f "{}" \
-		| xargs -n 1 -I {} sh -c "$(1)"
+		| xargs -n 1 -I {} echo "$(1)" ) && \
+		(set -xeuo pipefail; eval $${command_line})

 build: bazel-server
 	@$(call build_cmd)
@ -194,12 +202,21 @@ sudo: bazel-server
 	@$(call build_paths,sudo -E {} $(ARGS))
 .PHONY: sudo

-test: OPTIONS += --test_output=errors --keep_going --verbose_failures=true
 test: bazel-server
-	@docker exec $(FULL_DOCKER_EXEC_OPTIONS) $(DOCKER_NAME) $(BAZEL) test $(OPTIONS) $(TARGETS)
+	@docker exec $(FULL_DOCKER_EXEC_OPTIONS) $(DOCKER_NAME) \
+	  $(BAZEL) test $(BASE_OPTIONS) \
+	                --test_output=errors --keep_going --verbose_failures=true \
+	                --build_event_json_file=.build_events.json \
+	                $(OPTIONS) $(TARGETS)
 .PHONY: test

-query:
-	@$(MAKE) bazel-server >&2 # If we need to start, ensure stdout is not polluted.
-	@docker exec $(FULL_DOCKER_EXEC_OPTIONS) $(DOCKER_NAME) sh -o pipefail -c '$(BAZEL) query $(OPTIONS) "$(TARGETS)" 2>/dev/null'
+testlogs:
+	@cat .build_events.json | jq -r \
+	  'select(.testSummary?.overallStatus? | tostring | test("(FAILED|FLAKY|TIMEOUT)")) | .testSummary.failed | .[] | .uri' | \
+	  awk -Ffile:// '{print $2;}'
+.PHONY: testlogs
+
+query: bazel-server
+	@docker exec $(FULL_DOCKER_EXEC_OPTIONS) $(DOCKER_NAME) sh -o pipefail -c \
+	  '$(BAZEL) query $(BASE_OPTIONS) $(OPTIONS) "$(TARGETS)" 2>/dev/null'
 .PHONY: query
--- a/tools/bazel_gazelle.patch
+++ b/tools/bazel_gazelle.patch
@ -0,0 +1,24 @@
+diff -r -u2 a/language/go/resolve.go b/language/go/resolve.go
+--- a/language/go/resolve.go	2020-10-02 14:22:18.000000000 -0700
+++ b/language/go/resolve.go	2020-11-17 19:40:59.770648029 -0800
+@@ -20,5 +20,4 @@
+ 	"fmt"
+ 	"go/build"
+-	"log"
+ 	"path"
+ 	"regexp"
+@@ -80,5 +79,5 @@
+ 		resolve = ResolveGo
+ 	}
+-	deps, errs := imports.Map(func(imp string) (string, error) {
+	deps, _ := imports.Map(func(imp string) (string, error) {
+ 		l, err := resolve(c, ix, rc, imp, from)
+ 		if err == skipImportError {
+@@ -95,7 +94,4 @@
+ 		return l.String(), nil
+ 	})
+-	for _, err := range errs {
+-		log.Print(err)
+-	}
+ 	if !deps.IsEmpty() {
+ 		if r.Kind() == "go_proto_library" {