From 756bc3e52b2344dc48e0cefb929b0ebf1b9becf6 Mon Sep 17 00:00:00 2001 From: Adin Scannell Date: Mon, 23 Nov 2020 14:43:42 -0800 Subject: [PATCH] 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 --- .gitignore | 4 +- Makefile | 77 ++++++++++++++++++++++----------------- WORKSPACE | 6 +++ images/Makefile | 8 ++-- tools/bazel.mk | 47 ++++++++++++++++-------- tools/bazel_gazelle.patch | 24 ++++++++++++ 6 files changed, 112 insertions(+), 54 deletions(-) create mode 100644 tools/bazel_gazelle.patch diff --git a/.gitignore b/.gitignore index a56f6ebcd..95fe857dd 100644 --- a/.gitignore +++ b/.gitignore @@ -1,2 +1,4 @@ # Generated bazel symlinks. -/bazel-* \ No newline at end of file +/bazel-* +# Generated build event file. +/.build_events.json \ No newline at end of file diff --git a/Makefile b/Makefile index 79d8fd791..47d89c438 100644 --- a/Makefile +++ b/Makefile @@ -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 && \ diff --git a/WORKSPACE b/WORKSPACE index 2f3408709..2405bfd80 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -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", diff --git a/images/Makefile b/images/Makefile index 12927c509..66aac7802 100644 --- 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 diff --git a/tools/bazel.mk b/tools/bazel.mk index 3a7de427f..77b8746c6 100644 --- 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 diff --git a/tools/bazel_gazelle.patch b/tools/bazel_gazelle.patch new file mode 100644 index 000000000..e35f38933 --- /dev/null +++ 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" {