diff options
83 files changed, 1658 insertions, 1093 deletions
@@ -12,7 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. -# RBE requires a strong hash function, such as SHA256. +# Ensure a strong hash function. startup --host_jvm_args=-Dbazel.DigestFunction=SHA256 # Build with C++17. @@ -20,27 +20,3 @@ build --cxxopt=-std=c++17 # Display the current git revision in the info block. build --stamp --workspace_status_command tools/workspace_status.sh - -# Enable remote execution so actions are performed on the remote systems. -build:remote --remote_executor=grpcs://remotebuildexecution.googleapis.com -build:remote --bes_backend=buildeventservice.googleapis.com -build:remote --bes_results_url="https://source.cloud.google.com/results/invocations" -build:remote --bes_timeout=600s -build:remote --project_id=gvisor-rbe -build:remote --remote_instance_name=projects/gvisor-rbe/instances/default_instance - -# Enable authentication. This will pick up application default credentials by -# default. You can use --google_credentials=some_file.json to use a service -# account credential instead. -build:remote --google_default_credentials=true -build:remote --auth_scope="https://www.googleapis.com/auth/cloud-source-tools" - -# Add a custom platform and toolchain that builds in a privileged docker -# container, which is required by our syscall tests. -build:remote --host_platform=//tools/bazeldefs:rbe_ubuntu1604 -build:remote --extra_toolchains=//tools/bazeldefs:cc-toolchain-clang-x86_64-default -build:remote --extra_execution_platforms=//tools/bazeldefs:rbe_ubuntu1604 -build:remote --platforms=//tools/bazeldefs:rbe_ubuntu1604 -build:remote --crosstool_top=@rbe_default//cc:toolchain -build:remote --jobs=100 -build:remote --remote_timeout=3600 diff --git a/.buildkite/hooks/post-command b/.buildkite/hooks/post-command index ce3111f3c..5cd974002 100644 --- a/.buildkite/hooks/post-command +++ b/.buildkite/hooks/post-command @@ -1,17 +1,49 @@ -# Upload test logs on failure, if there are any. -if [[ "${BUILDKITE_COMMAND_EXIT_STATUS}" -ne "0" ]]; then +# Upload all relevant test failures. +make -s testlogs 2>/dev/null | grep // | sort | uniq | ( declare log_count=0 - for log in $(make testlogs 2>/dev/null | sort | uniq); do - buildkite-agent artifact upload "${log}" - log_count=$((${log_count}+1)) + while read target log; do + if test -z "${target}"; then + continue + fi + # N.B. If *all* tests fail due to some common cause, then we will # end up spending way too much time uploading logs. Instead, we just - # upload the first 100 and stop. That is hopefully enough to debug. - if [[ "${log_count}" -ge 100 ]]; then - echo "Only uploaded first 100 failures; skipping the rest." - break + # upload the first 10 and stop. That is hopefully enough to debug. + # + # We include this test in the metadata, but note that we cannot + # upload the actual test logs. The user should rerun locally. + log_count=$((${log_count}+1)) + if test "${log_count}" -ge 10; then + echo " * ${target} (no upload)" | \ + buildkite-agent annotate --style error --context failures --append + else + buildkite-agent artifact upload "${log}" + echo " * [${target}](artifact://${log#/}) (${BUILDKITE_LABEL})" | \ + buildkite-agent annotate --style error --context failures --append fi done +) + +# Upload all profiles, and include in an annotation. +declare profile_output=$(mktemp --tmpdir) +for file in $(find /tmp/profile -name \*.pprof -print 2>/dev/null | sort); do + # Generate a link to the profile file at the top. + profile_name="${file#/tmp/profile/}" + buildkite-agent artifact upload "${file}" + echo "<li><a href='artifact://${file#/}'>${profile_name}</a></li>" >> "${profile_output}" +done + +# Upload if we had outputs. +if test -s "${profile_output}"; then + # Make the list a collapsible section in markdown. + sed -i "1s|^|<details><summary>${BUILDKITE_LABEL}</summary><ul>\n|" "${profile_output}" + echo "</ul></details>" >> "${profile_output}" + cat "${profile_output}" | buildkite-agent annotate --style info --context profiles --append +fi +rm -rf "${profile_output}" + +# Clean the bazel cache, if there's failure. +if test "${BUILDKITE_COMMAND_EXIT_STATUS}" -ne "0"; then # Attempt to clear the cache and shut down. make clean || echo "make clean failed with code $?" make bazel-shutdown || echo "make bazel-shutdown failed with code $?" @@ -19,6 +51,6 @@ fi # Kill any running containers (clear state). CONTAINERS="$(docker ps -q)" -if ! [[ -z "${CONTAINERS}" ]]; then +if ! test -z "${CONTAINERS}"; then docker container kill ${CONTAINERS} 2>/dev/null || true -fi
\ No newline at end of file +fi diff --git a/.buildkite/hooks/pre-command b/.buildkite/hooks/pre-command index 7d277202b..ba688f9ac 100644 --- a/.buildkite/hooks/pre-command +++ b/.buildkite/hooks/pre-command @@ -1,3 +1,15 @@ +# Install packages we need. Docker must be installed and configured, +# as should Go itself. We just install some extra bits and pieces. +function install_pkgs() { + while true; do + if sudo apt-get update && sudo apt-get install -y "$@"; then + break + fi + done +} +install_pkgs graphviz jq curl binutils gnupg gnupg-agent linux-libc-dev \ + apt-transport-https ca-certificates software-properties-common + # Setup for parallelization with PARTITION and TOTAL_PARTITIONS. export PARTITION=${BUILDKITE_PARALLEL_JOB:-0} PARTITION=$((${PARTITION}+1)) # 1-indexed, but PARALLEL_JOB is 0-indexed. @@ -9,3 +21,13 @@ if test "${EXPERIMENTAL}" != "true"; then make sudo TARGETS=//runsc:runsc ARGS="install --experimental=true" sudo systemctl restart docker fi + +# Helper for benchmarks, based on the branch. +if test "${BUILDKITE_BRANCH}" = "master"; then + export BENCHMARKS_OFFICIAL=true +else + export BENCHMARKS_OFFICIAL=false +fi + +# Clear existing profiles. +sudo rm -rf /tmp/profile diff --git a/.buildkite/pipeline.yaml b/.buildkite/pipeline.yaml index 79a80d9c8..d03847800 100644 --- a/.buildkite/pipeline.yaml +++ b/.buildkite/pipeline.yaml @@ -7,6 +7,20 @@ _templates: limit: 10 - exit_status: "*" limit: 2 + benchmarks: &benchmarks + timeout_in_minutes: 120 + retry: + automatic: false + soft_fail: true + if: build.message =~ /benchmarks/ || build.branch == "master" + env: + # BENCHMARKS_OFFICIAL is set from hooks/pre-command, based + # on whether this is executing on the master branch. + BENCHMARKS_DATASET: buildkite + BENCHMARKS_PLATFORMS: "ptrace kvm" + BENCHMARKS_PROJECT: gvisor-benchmarks + BENCHMARKS_TABLE: benchmarks + BENCHMARKS_UPLOAD: true steps: # Run basic smoke tests before preceding to other tests. @@ -105,3 +119,78 @@ steps: label: ":python: Python runtime tests" command: make python3.7.3-runtime-tests_vfs2 parallelism: 10 + + # Runtime tests (VFS1). + - <<: *common + label: ":php: PHP runtime tests (VFS1)" + command: make php7.3.6-runtime-tests + parallelism: 10 + if: build.message =~ /VFS1/ || build.branch == "master" + - <<: *common + label: ":java: Java runtime tests (VFS1)" + command: make java11-runtime-tests + parallelism: 40 + if: build.message =~ /VFS1/ || build.branch == "master" + - <<: *common + label: ":golang: Go runtime tests (VFS1)" + command: make go1.12-runtime-tests + parallelism: 10 + if: build.message =~ /VFS1/ || build.branch == "master" + - <<: *common + label: ":node: NodeJS runtime tests (VFS1)" + command: make nodejs12.4.0-runtime-tests + parallelism: 10 + if: build.message =~ /VFS1/ || build.branch == "master" + - <<: *common + label: ":python: Python runtime tests (VFS1)" + command: make python3.7.3-runtime-tests + parallelism: 10 + if: build.message =~ /VFS1/ || build.branch == "master" + + # Run basic benchmarks smoke tests (no upload). + - <<: *common + label: ":fire: Benchmarks smoke test" + command: make benchmark-platforms + # Use the opposite of the benchmarks filter. + if: build.message !~ /benchmarks/ && build.branch != "master" + + # Run all benchmarks. + - <<: *benchmarks + label: ":bazel: ABSL build benchmarks" + command: make benchmark-platforms BENCHMARKS_FILTER="ABSL/page_cache.clean" BENCHMARKS_SUITE=absl BENCHMARKS_TARGETS=test/benchmarks/fs:bazel_test + - <<: *benchmarks + label: ":metal: FFMPEG benchmarks" + command: make benchmark-platforms BENCHMARKS_SUITE=ffmpeg BENCHMARKS_TARGETS=test/benchmarks/media:ffmpeg_test + - <<: *benchmarks + label: ":floppy_disk: FIO benchmarks" + command: make benchmark-platforms BENCHMARKS_SUITE=fio BENCHMARKS_TARGETS=test/benchmarks/fs:fio_test + - <<: *benchmarks + label: ":globe_with_meridians: HTTPD benchmarks" + command: make benchmark-platforms BENCHMARKS_FILTER="Continuous" BENCHMARKS_SUITE=httpd BENCHMARKS_TARGETS=test/benchmarks/network:httpd_test + - <<: *benchmarks + label: ":piedpiper: iperf benchmarks" + command: make benchmark-platforms BENCHMARKS_SUITE=iperf BENCHMARKS_TARGETS=test/benchmarks/network:iperf_test + - <<: *benchmarks + label: ":nginx: nginx benchmarks" + command: make benchmark-platforms BENCHMARKS_FILTER="Continuous" BENCHMARKS_SUITE=nginx BENCHMARKS_TARGETS=test/benchmarks/network:nginx_test + - <<: *benchmarks + label: ":node: node benchmarks" + command: make benchmark-platforms BENCHMARKS_SUITE=node BENCHMARKS_TARGETS=test/benchmarks/network:node_test + - <<: *benchmarks + label: ":redis: Redis benchmarks" + command: make benchmark-platforms BENCHMARKS_SUITE=redis BENCHMARKS_TARGETS=test/benchmarks/database:redis_test + - <<: *benchmarks + label: ":ruby: Ruby benchmarks" + command: make benchmark-platforms BENCHMARKS_SUITE=ruby BENCHMARKS_TARGETS=test/benchmarks/network:ruby_test + - <<: *benchmarks + label: ":weight_lifter: Size benchmarks" + command: make benchmark-platforms BENCHMARKS_SUITE=size BENCHMARKS_TARGETS=test/benchmarks/base:size_test + - <<: *benchmarks + label: ":speedboat: Startup benchmarks" + command: make benchmark-platforms BENCHMARKS_SUITE=startup BENCHMARKS_TARGETS=test/benchmarks/base:startup_test + - <<: *benchmarks + label: ":computer: sysbench benchmarks" + command: make benchmark-platforms BENCHMARKS_SUITE=sysbench BENCHMARKS_TARGETS=test/benchmarks/base:sysbench_test + - <<: *benchmarks + label: ":tensorflow: TensorFlow benchmarks" + command: make benchmark-platforms BENCHMARKS_SUITE=tensorflow BENCHMARKS_TARGETS=test/benchmarks/ml:tensorflow_test diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 3be10b9bb..270aaf034 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -2,11 +2,8 @@ # posts them to GitHub, if applicable. This leverages the fact that the # workflow token has appropriate permissions to do so, and attempts to # leverage the GitHub workflow caches. -# -# This workflow also generates the build badge that is referred to by -# the main README. name: "Build" -on: +"on": push: branches: - master diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml index ff3059e2a..e62991691 100644 --- a/.github/workflows/go.yml +++ b/.github/workflows/go.yml @@ -3,7 +3,7 @@ # workflow simply generates and pushes the branch, as long as appropriate # permissions are available. name: "Go" -on: +"on": push: branches: - master diff --git a/.github/workflows/issue_reviver.yml b/.github/workflows/issue_reviver.yml index f03b814c9..3bd883035 100644 --- a/.github/workflows/issue_reviver.yml +++ b/.github/workflows/issue_reviver.yml @@ -1,7 +1,7 @@ # This workflow revives issues that are still referenced in the code, and may # have been accidentally closed or marked stale. name: "Issue reviver" -on: +"on": schedule: - cron: '0 0 * * *' diff --git a/.github/workflows/labeler.yml b/.github/workflows/labeler.yml index a53fdb3e9..3a19065e1 100644 --- a/.github/workflows/labeler.yml +++ b/.github/workflows/labeler.yml @@ -1,6 +1,6 @@ # Labeler labels incoming pull requests. name: "Labeler" -on: +"on": - pull_request jobs: diff --git a/.github/workflows/stale.yml b/.github/workflows/stale.yml index be10c5bc4..3a4aa22e2 100644 --- a/.github/workflows/stale.yml +++ b/.github/workflows/stale.yml @@ -1,7 +1,7 @@ # The stale workflow closes stale issues and pull requests, unless specific # tags have been applied in order to keep them open. name: "Close stale issues" -on: +"on": schedule: - cron: "0 0 * * *" @@ -1,5 +1,6 @@ load("//tools:defs.bzl", "build_test", "gazelle", "go_path") load("//tools/nogo:defs.bzl", "nogo_config") +load("//tools/yamltest:defs.bzl", "yaml_test") load("//website:defs.bzl", "doc") package(licenses = ["notice"]) @@ -50,6 +51,24 @@ doc( weight = "99", ) +yaml_test( + name = "nogo_config_test", + srcs = glob(["nogo*.yaml"]), + schema = "//tools/nogo:config-schema.json", +) + +yaml_test( + name = "github_workflows_test", + srcs = glob([".github/workflows/*.yml"]), + schema = "@github_workflow_schema//file", +) + +yaml_test( + name = "buildkite_pipelines_test", + srcs = glob([".buildkite/*.yaml"]), + schema = "@buildkite_pipeline_schema//file", +) + # The sandbox filegroup is used for sandbox-internal dependencies. package_group( name = "sandbox", @@ -23,6 +23,7 @@ header = echo --- $(1) >&2 # Make hacks. EMPTY := SPACE := $(EMPTY) $(EMPTY) +SHELL = /bin/bash ## usage: make <target> ## or @@ -59,7 +60,7 @@ build: ## Builds the given $(TARGETS) with the given $(OPTIONS). E.g. make build .PHONY: build test: ## Tests the given $(TARGETS) with the given $(OPTIONS). E.g. make test TARGETS=pkg/buffer:buffer_test - @$(call build,$(OPTIONS) $(TARGETS)) + @$(call test,$(OPTIONS) $(TARGETS)) .PHONY: test copy: ## Copies the given $(TARGETS) to the given $(DESTINATION). E.g. make copy TARGETS=runsc DESTINATION=/tmp @@ -129,10 +130,10 @@ reload_docker = \ configure = $(call configure_noreload,$(1),$(2)) && $(reload_docker) # Helpers for above. Requires $(RUNTIME_BIN) dependency. -install_runtime = $(call configure,$(RUNTIME),$(1) --TESTONLY-test-name-env=RUNSC_TEST_NAME) +install_runtime = $(call configure,$(1),$(2) --TESTONLY-test-name-env=RUNSC_TEST_NAME) # Don't use cached results, otherwise multiple runs using different runtimes -# are skipped. -test_runtime = $(call test,--test_arg=--runtime=$(RUNTIME) --nocache_test_results $(PARTITIONS) $(1)) +# may be skipped, if all other inputs are the same. +test_runtime = $(call test,--test_arg=--runtime=$(1) --nocache_test_results $(PARTITIONS) $(2)) refresh: $(RUNTIME_BIN) ## Updates the runtime binary. .PHONY: refresh @@ -218,12 +219,12 @@ syscall-tests: ## Run all system call tests. @$(call test,$(PARTITIONS) test/syscalls/...) %-runtime-tests: load-runtimes_% $(RUNTIME_BIN) - @$(call install_runtime,) # Ensure flags are cleared. - @$(call test_runtime,--test_timeout=10800 //test/runtimes:$*) + @$(call install_runtime,$(RUNTIME),) # Ensure flags are cleared. + @$(call test_runtime,$(RUNTIME),--test_timeout=10800 //test/runtimes:$*) %-runtime-tests_vfs2: load-runtimes_% $(RUNTIME_BIN) - @$(call install_runtime,--vfs2) - @$(call test_runtime,--test_timeout=10800 //test/runtimes:$*) + @$(call install_runtime,$(RUNTIME),--vfs2) + @$(call test_runtime,$(RUNTIME),--test_timeout=10800 //test/runtimes:$*) do-tests: @$(call run,//runsc,--rootless do true) @@ -238,58 +239,58 @@ simple-tests: unit-tests # Compatibility target. INTEGRATION_TARGETS := //test/image:image_test //test/e2e:integration_test docker-tests: load-basic $(RUNTIME_BIN) - @$(call install_runtime,) # Clear flags. - @$(call test_runtime,$(INTEGRATION_TARGETS)) - @$(call install_runtime,--vfs2) - @$(call test_runtime,$(INTEGRATION_TARGETS)) + @$(call install_runtime,$(RUNTIME),) # Clear flags. + @$(call test_runtime,$(RUNTIME),$(INTEGRATION_TARGETS)) + @$(call install_runtime,$(RUNTIME),--vfs2) + @$(call test_runtime,$(RUNTIME),$(INTEGRATION_TARGETS)) .PHONY: docker-tests overlay-tests: load-basic $(RUNTIME_BIN) - @$(call install_runtime,--overlay) - @$(call test_runtime,$(INTEGRATION_TARGETS)) + @$(call install_runtime,$(RUNTIME),--overlay) + @$(call test_runtime,$(RUNTIME),$(INTEGRATION_TARGETS)) .PHONY: overlay-tests swgso-tests: load-basic $(RUNTIME_BIN) - @$(call install_runtime,--software-gso=true --gso=false) - @$(call test_runtime,$(INTEGRATION_TARGETS)) + @$(call install_runtime,$(RUNTIME),--software-gso=true --gso=false) + @$(call test_runtime,$(RUNTIME),$(INTEGRATION_TARGETS)) .PHONY: swgso-tests hostnet-tests: load-basic $(RUNTIME_BIN) - @$(call install_runtime,--network=host) - @$(call test_runtime,--test_arg=-checkpoint=false --test_arg=-hostnet=true $(INTEGRATION_TARGETS)) + @$(call install_runtime,$(RUNTIME),--network=host) + @$(call test_runtime,$(RUNTIME),--test_arg=-checkpoint=false --test_arg=-hostnet=true $(INTEGRATION_TARGETS)) .PHONY: hostnet-tests kvm-tests: load-basic $(RUNTIME_BIN) @(lsmod | grep -E '^(kvm_intel|kvm_amd)') || sudo modprobe kvm @if ! test -w /dev/kvm; then sudo chmod a+rw /dev/kvm; fi @$(call test,//pkg/sentry/platform/kvm:kvm_test) - @$(call install_runtime,--platform=kvm) - @$(call test_runtime,$(INTEGRATION_TARGETS)) + @$(call install_runtime,$(RUNTIME),--platform=kvm) + @$(call test_runtime,$(RUNTIME),$(INTEGRATION_TARGETS)) .PHONY: kvm-tests iptables-tests: load-iptables $(RUNTIME_BIN) @sudo modprobe iptable_filter @sudo modprobe ip6table_filter @$(call test,--test_arg=-runtime=runc $(PARTITIONS) //test/iptables:iptables_test) - @$(call install_runtime,--net-raw) - @$(call test_runtime,//test/iptables:iptables_test) + @$(call install_runtime,$(RUNTIME),--net-raw) + @$(call test_runtime,$(RUNTIME),//test/iptables:iptables_test) .PHONY: iptables-tests packetdrill-tests: load-packetdrill $(RUNTIME_BIN) - @$(call install_runtime,) # Clear flags. - @$(call test_runtime,//test/packetdrill:all_tests) + @$(call install_runtime,$(RUNTIME),) # Clear flags. + @$(call test_runtime,$(RUNTIME),//test/packetdrill:all_tests) .PHONY: packetdrill-tests packetimpact-tests: load-packetimpact $(RUNTIME_BIN) @sudo modprobe iptable_filter @sudo modprobe ip6table_filter - @$(call install_runtime,) # Clear flags. - @$(call test_runtime,--jobs=HOST_CPUS*3 --local_test_jobs=HOST_CPUS*3 //test/packetimpact/tests:all_tests) + @$(call install_runtime,$(RUNTIME),) # Clear flags. + @$(call test_runtime,$(RUNTIME),--jobs=HOST_CPUS*3 --local_test_jobs=HOST_CPUS*3 //test/packetimpact/tests:all_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 $(RUNTIME_BIN) - @$(call install_runtime,) # Clear flags. + @$(call install_runtime,$(RUNTIME),) # Clear flags. @$(call sudo,tools/installers:containerd,$*) @$(call sudo,tools/installers:shim) @$(call sudo,test/root:root_test,--runtime=$(RUNTIME) -test.v) @@ -310,25 +311,27 @@ containerd-tests: containerd-test-1.4.3 ## Targets to run benchmarks. See //test/benchmarks for details. ## ## common arguments: -## RUNTIME_ARGS - arguments to runsc placed in /etc/docker/daemon.json -## e.g. "--platform=ptrace" -## BENCHMARKS_PROJECT - BigQuery project to which to send data. -## BENCHMARKS_DATASET - BigQuery dataset to which to send data. -## BENCHMARKS_TABLE - BigQuery table to which to send data. -## BENCHMARKS_SUITE - name of the benchmark suite. See //tools/bigquery/bigquery.go. -## BENCHMARKS_UPLOAD - if true, upload benchmark data from the run. -## BENCHMARKS_OFFICIAL - marks the data as official. +## BENCHMARKS_PROJECT - BigQuery project to which to send data. +## BENCHMARKS_DATASET - BigQuery dataset to which to send data. +## BENCHMARKS_TABLE - BigQuery table to which to send data. +## BENCHMARKS_SUITE - name of the benchmark suite. See //tools/bigquery/bigquery.go. +## BENCHMARKS_UPLOAD - if true, upload benchmark data from the run. +## BENCHMARKS_OFFICIAL - marks the data as official. ## BENCHMARKS_PLATFORMS - platforms to run benchmarks (e.g. ptrace kvm). +## BENCHMARKS_FILTER - filter to be applied to the test suite. +## BENCHMARKS_OPTIONS - options to be passed to the test. ## -BENCHMARKS_PROJECT := gvisor-benchmarks -BENCHMARKS_DATASET := kokoro -BENCHMARKS_TABLE := benchmarks -BENCHMARKS_SUITE := start -BENCHMARKS_UPLOAD := false -BENCHMARKS_OFFICIAL := false -BENCHMARKS_PLATFORMS := ptrace -BENCHMARKS_TARGETS := //test/benchmarks/base:startup_test -BENCHMARKS_ARGS := -test.bench=. -pprof-cpu -pprof-heap -pprof-heap -pprof-block +BENCHMARKS_PROJECT ?= gvisor-benchmarks +BENCHMARKS_DATASET ?= kokoro +BENCHMARKS_TABLE ?= benchmarks +BENCHMARKS_SUITE ?= ffmpeg +BENCHMARKS_UPLOAD ?= false +BENCHMARKS_OFFICIAL ?= false +BENCHMARKS_PLATFORMS ?= ptrace +BENCHMARKS_TARGETS := //test/benchmarks/media:ffmpeg_test +BENCHMARKS_FILTER := . +BENCHMARKS_OPTIONS := -test.benchtime=30s +BENCHMARKS_ARGS := -test.v -test.bench=$(BENCHMARKS_FILTER) -pprof-dir=/tmp/profile -pprof-cpu -pprof-heap -pprof-block -pprof-mutex $(BENCHMARKS_OPTIONS) init-benchmark-table: ## Initializes a BigQuery table with the benchmark schema. @$(call run,//tools/parsers:parser,init --project=$(BENCHMARKS_PROJECT) --dataset=$(BENCHMARKS_DATASET) --table=$(BENCHMARKS_TABLE)) @@ -336,27 +339,25 @@ init-benchmark-table: ## Initializes a BigQuery table with the benchmark schema. # $(1) is the runtime name, $(2) are the arguments. run_benchmark = \ - $(call header,BENCHMARK $(1) $(2)); \ - if test "$(1)" != "runc"; then $(call install_runtime,--profile $(2)); fi \ - @T=$$(mktemp --tmpdir logs.$(RUNTIME).XXXXXX); \ - $(call sudo,$(BENCHMARKS_TARGETS) --runtime=$(RUNTIME) $(BENCHMARKS_ARGS) | tee $$T); \ - rc=$$?; \ - if test $$rc -eq 0 && test "$(BENCHMARKS_UPLOAD)" == "true"; then \ - $(call run,tools/parsers:parser parse --debug --file=$$T --runtime=$(RUNTIME) --suite_name=$(BENCHMARKS_SUITE) --project=$(BENCHMARKS_PROJECT) --dataset=$(BENCHMARKS_DATASET) --table=$(BENCHMARKS_TABLE) --official=$(BENCHMARKS_OFFICIAL)); \ + ($(call header,BENCHMARK $(1) $(2)); \ + set -euo pipefail; \ + if test "$(1)" != "runc"; then $(call install_runtime,$(1),--profile $(2)); fi; \ + export T=$$(mktemp --tmpdir logs.$(1).XXXXXX); \ + $(call sudo,$(BENCHMARKS_TARGETS),-runtime=$(1) $(BENCHMARKS_ARGS)) | tee $$T; \ + if test "$(BENCHMARKS_UPLOAD)" = "true"; then \ + $(call run,tools/parsers:parser,parse --debug --file=$$T --runtime=$(1) --suite_name=$(BENCHMARKS_SUITE) --project=$(BENCHMARKS_PROJECT) --dataset=$(BENCHMARKS_DATASET) --table=$(BENCHMARKS_TABLE) --official=$(BENCHMARKS_OFFICIAL)); \ fi; \ - rm -rf $$T; \ - exit $$rc + rm -rf $$T) -benchmark-platforms: load-benchmarks ## Runs benchmarks for runc and all given platforms in BENCHMARK_PLATFORMS. +benchmark-platforms: load-benchmarks $(RUNTIME_BIN) ## Runs benchmarks for runc and all given platforms in BENCHMARK_PLATFORMS. @$(foreach PLATFORM,$(BENCHMARKS_PLATFORMS), \ - $(call run_benchmark,$(RUNTIME)+vfs2,$(BENCHMARK_ARGS) --platform=$(PLATFORM) --vfs2) && \ - $(call run_benchmark,$(RUNTIME),$(BENCHMARK_ARGS) --platform=$(PLATFORM)) && \ - ) \ - $(call run-benchmark,runc) + $(call run_benchmark,$(PLATFORM),--platform=$(PLATFORM) --vfs2) && \ + ) true + @$(call run-benchmark,runc) .PHONY: benchmark-platforms -run-benchmark: load-benchmarks ## Runs single benchmark and optionally sends data to BigQuery. - @$(call run_benchmark,$(RUNTIME),$(BENCHMARK_ARGS)) +run-benchmark: load-benchmarks $(RUNTIME_BIN) ## Runs single benchmark and optionally sends data to BigQuery. + @$(call run_benchmark,$(RUNTIME),) .PHONY: run-benchmark ## @@ -1,6 +1,6 @@  - +[](https://buildkite.com/gvisor/pipeline) [](https://gitter.im/gvisor/community) [](https://cs.opensource.google/gvisor/gvisor) @@ -1,4 +1,4 @@ -load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive") +load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive", "http_file") load("@bazel_tools//tools/build_defs/repo:git.bzl", "git_repository") # Bazel/starlark utilities. @@ -176,6 +176,19 @@ http_archive( ], ) +# Schemas for testing. +http_file( + name = "buildkite_pipeline_schema", + sha256 = "3369c58038b4d55c08928affafb653716eb1e7b3cabb4a391aef979dd921f4e1", + urls = ["https://raw.githubusercontent.com/buildkite/pipeline-schema/f7a0894074d194bcf19eec5411fec0528f7f4180/schema.json"], +) + +http_file( + name = "github_workflow_schema", + sha256 = "2c375bb43dbc8b32b1bed46c290d0b70a8fa2aca7a5484dfca1b6e9c38cf9e7a", + urls = ["https://raw.githubusercontent.com/SchemaStore/schemastore/27612065234778feaac216ce14dd47846fe0a2dd/src/schemas/json/github-workflow.json"], +) + # External Go repositories. # # Unfortunately, gazelle will automatically parse go modules in the @@ -1391,3 +1404,24 @@ go_repository( sum = "h1:+ySTxfHnfzZb9ys375PXNlLhkJPLKgHajBU0N62BDvE=", version = "v0.0.0-20190801114015-581e00157fb1", ) + +go_repository( + name = "com_github_xeipuuv_gojsonpointer", + importpath = "github.com/xeipuuv/gojsonpointer", + sum = "h1:zGWFAtiMcyryUHoUjUJX0/lt1H2+i2Ka2n+D3DImSNo=", + version = "v0.0.0-20190905194746-02993c407bfb", +) + +go_repository( + name = "com_github_xeipuuv_gojsonreference", + importpath = "github.com/xeipuuv/gojsonreference", + sum = "h1:EzJWgHovont7NscjpAxXsDA8S8BMYve8Y5+7cuRE7R0=", + version = "v0.0.0-20180127040603-bd5ef7bd5415", +) + +go_repository( + name = "com_github_xeipuuv_gojsonschema", + importpath = "github.com/xeipuuv/gojsonschema", + sum = "h1:LhYJRs+L4fBtjZUfuSZIKGeVu0QRy8e5Xi7D17UxZ74=", + version = "v1.2.0", +) diff --git a/pkg/control/server/server.go b/pkg/control/server/server.go index 41abe1f2d..629dae8f4 100644 --- a/pkg/control/server/server.go +++ b/pkg/control/server/server.go @@ -67,9 +67,10 @@ func (s *Server) Wait() { // and the server should not be used afterwards. func (s *Server) Stop() { s.socket.Close() - s.wg.Wait() + s.Wait() - // This will cause existing clients to be terminated safely. + // This will cause existing clients to be terminated safely. If the + // registered handlers have a Stop callback, it will be called. s.server.Stop() } diff --git a/pkg/sentry/control/pprof.go b/pkg/sentry/control/pprof.go index 91b8fb44f..2f3664c57 100644 --- a/pkg/sentry/control/pprof.go +++ b/pkg/sentry/control/pprof.go @@ -15,10 +15,10 @@ package control import ( - "errors" "runtime" "runtime/pprof" "runtime/trace" + "time" "gvisor.dev/gvisor/pkg/fd" "gvisor.dev/gvisor/pkg/sentry/kernel" @@ -26,184 +26,263 @@ import ( "gvisor.dev/gvisor/pkg/urpc" ) -var errNoOutput = errors.New("no output writer provided") +// Profile includes profile-related RPC stubs. It provides a way to +// control the built-in runtime profiling facilities. +// +// The profile object must be instantied via NewProfile. +type Profile struct { + // kernel is the kernel under profile. It's immutable. + kernel *kernel.Kernel -// ProfileOpts contains options for the StartCPUProfile/Goroutine RPC call. -type ProfileOpts struct { - // File is the filesystem path for the profile. - File string `json:"path"` + // cpuMu protects CPU profiling. + cpuMu sync.Mutex - // FilePayload is the destination for the profiling output. - urpc.FilePayload + // blockMu protects block profiling. + blockMu sync.Mutex + + // mutexMu protects mutex profiling. + mutexMu sync.Mutex + + // traceMu protects trace profiling. + traceMu sync.Mutex + + // done is closed when profiling is done. + done chan struct{} } -// Profile includes profile-related RPC stubs. It provides a way to -// control the built-in pprof facility in sentry via sentryctl. -// -// The following options to sentryctl are added: -// -// - collect CPU profile on-demand. -// sentryctl -pid <pid> pprof-cpu-start -// sentryctl -pid <pid> pprof-cpu-stop -// -// - dump out the stack trace of current go routines. -// sentryctl -pid <pid> pprof-goroutine -type Profile struct { - // Kernel is the kernel under profile. It's immutable. - Kernel *kernel.Kernel +// NewProfile returns a new Profile object. +func NewProfile(k *kernel.Kernel) *Profile { + return &Profile{ + kernel: k, + done: make(chan struct{}), + } +} - // mu protects the fields below. - mu sync.Mutex +// Stop implements urpc.Stopper.Stop. +func (p *Profile) Stop() { + close(p.done) +} - // cpuFile is the current CPU profile output file. - cpuFile *fd.FD +// CPUProfileOpts contains options specifically for CPU profiles. +type CPUProfileOpts struct { + // FilePayload is the destination for the profiling output. + urpc.FilePayload - // traceFile is the current execution trace output file. - traceFile *fd.FD + // Duration is the duration of the profile. + Duration time.Duration `json:"duration"` } -// StartCPUProfile is an RPC stub which starts recording the CPU profile in a -// file. -func (p *Profile) StartCPUProfile(o *ProfileOpts, _ *struct{}) error { +// CPU is an RPC stub which collects a CPU profile. +func (p *Profile) CPU(o *CPUProfileOpts, _ *struct{}) error { if len(o.FilePayload.Files) < 1 { - return errNoOutput + return nil // Allowed. } - output, err := fd.NewFromFile(o.FilePayload.Files[0]) - if err != nil { - return err - } + output := o.FilePayload.Files[0] + defer output.Close() - p.mu.Lock() - defer p.mu.Unlock() + p.cpuMu.Lock() + defer p.cpuMu.Unlock() // Returns an error if profiling is already started. if err := pprof.StartCPUProfile(output); err != nil { - output.Close() return err } + defer pprof.StopCPUProfile() + + // Collect the profile. + select { + case <-time.After(o.Duration): + case <-p.done: + } - p.cpuFile = output return nil } -// StopCPUProfile is an RPC stub which stops the CPU profiling and flush out the -// profile data. It takes no argument. -func (p *Profile) StopCPUProfile(_, _ *struct{}) error { - p.mu.Lock() - defer p.mu.Unlock() - - if p.cpuFile == nil { - return errors.New("CPU profiling not started") - } +// HeapProfileOpts contains options specifically for heap profiles. +type HeapProfileOpts struct { + // FilePayload is the destination for the profiling output. + urpc.FilePayload - pprof.StopCPUProfile() - p.cpuFile.Close() - p.cpuFile = nil - return nil + // Delay is the sleep time, similar to Duration. This may + // not affect the data collected however, as the heap will + // continue only the memory associated with the last alloc. + Delay time.Duration `json:"delay"` } -// HeapProfile generates a heap profile for the sentry. -func (p *Profile) HeapProfile(o *ProfileOpts, _ *struct{}) error { +// Heap generates a heap profile. +func (p *Profile) Heap(o *HeapProfileOpts, _ *struct{}) error { if len(o.FilePayload.Files) < 1 { - return errNoOutput + return nil // Allowed. } + output := o.FilePayload.Files[0] defer output.Close() - runtime.GC() // Get up-to-date statistics. - if err := pprof.WriteHeapProfile(output); err != nil { - return err + + // Wait for the given delay. + select { + case <-time.After(o.Delay): + case <-p.done: } - return nil + + // Get up-to-date statistics. + runtime.GC() + + // Write the given profile. + return pprof.WriteHeapProfile(output) +} + +// GoroutineProfileOpts contains options specifically for goroutine profiles. +type GoroutineProfileOpts struct { + // FilePayload is the destination for the profiling output. + urpc.FilePayload } -// GoroutineProfile is an RPC stub which dumps out the stack trace for all -// running goroutines. -func (p *Profile) GoroutineProfile(o *ProfileOpts, _ *struct{}) error { +// Goroutine dumps out the stack trace for all running goroutines. +func (p *Profile) Goroutine(o *GoroutineProfileOpts, _ *struct{}) error { if len(o.FilePayload.Files) < 1 { - return errNoOutput + return nil // Allowed. } + output := o.FilePayload.Files[0] defer output.Close() - if err := pprof.Lookup("goroutine").WriteTo(output, 2); err != nil { - return err - } - return nil + + return pprof.Lookup("goroutine").WriteTo(output, 2) +} + +// BlockProfileOpts contains options specifically for block profiles. +type BlockProfileOpts struct { + // FilePayload is the destination for the profiling output. + urpc.FilePayload + + // Duration is the duration of the profile. + Duration time.Duration `json:"duration"` + + // Rate is the block profile rate. + Rate int `json:"rate"` } -// BlockProfile is an RPC stub which dumps out the stack trace that led to -// blocking on synchronization primitives. -func (p *Profile) BlockProfile(o *ProfileOpts, _ *struct{}) error { +// Block dumps a blocking profile. +func (p *Profile) Block(o *BlockProfileOpts, _ *struct{}) error { if len(o.FilePayload.Files) < 1 { - return errNoOutput + return nil // Allowed. } + output := o.FilePayload.Files[0] defer output.Close() - if err := pprof.Lookup("block").WriteTo(output, 0); err != nil { - return err + + p.blockMu.Lock() + defer p.blockMu.Unlock() + + // Always set the rate. We then wait to collect a profile at this rate, + // and disable when we're done. Note that the default here is 10%, which + // will record a stacktrace 10% of the time when blocking occurs. Since + // these events should not be super frequent, we expect this to achieve + // a reasonable balance between collecting the data we need and imposing + // a high performance cost (e.g. skewing even the CPU profile). + rate := 10 + if o.Rate != 0 { + rate = o.Rate } - return nil + runtime.SetBlockProfileRate(rate) + defer runtime.SetBlockProfileRate(0) + + // Collect the profile. + select { + case <-time.After(o.Duration): + case <-p.done: + } + + return pprof.Lookup("block").WriteTo(output, 0) +} + +// MutexProfileOpts contains options specifically for mutex profiles. +type MutexProfileOpts struct { + // FilePayload is the destination for the profiling output. + urpc.FilePayload + + // Duration is the duration of the profile. + Duration time.Duration `json:"duration"` + + // Fraction is the mutex profile fraction. + Fraction int `json:"fraction"` } -// MutexProfile is an RPC stub which dumps out the stack trace of holders of -// contended mutexes. -func (p *Profile) MutexProfile(o *ProfileOpts, _ *struct{}) error { +// Mutex dumps a mutex profile. +func (p *Profile) Mutex(o *MutexProfileOpts, _ *struct{}) error { if len(o.FilePayload.Files) < 1 { - return errNoOutput + return nil // Allowed. } + output := o.FilePayload.Files[0] defer output.Close() - if err := pprof.Lookup("mutex").WriteTo(output, 0); err != nil { - return err + + p.mutexMu.Lock() + defer p.mutexMu.Unlock() + + // Always set the fraction. Like the block rate above, we use + // a default rate of 10% for the same reasons. + fraction := 10 + if o.Fraction != 0 { + fraction = o.Fraction } - return nil + runtime.SetMutexProfileFraction(fraction) + defer runtime.SetMutexProfileFraction(0) + + // Collect the profile. + select { + case <-time.After(o.Duration): + case <-p.done: + } + + return pprof.Lookup("mutex").WriteTo(output, 0) } -// StartTrace is an RPC stub which starts collection of an execution trace. -func (p *Profile) StartTrace(o *ProfileOpts, _ *struct{}) error { +// TraceProfileOpts contains options specifically for traces. +type TraceProfileOpts struct { + // FilePayload is the destination for the profiling output. + urpc.FilePayload + + // Duration is the duration of the profile. + Duration time.Duration `json:"duration"` +} + +// Trace is an RPC stub which starts collection of an execution trace. +func (p *Profile) Trace(o *TraceProfileOpts, _ *struct{}) error { if len(o.FilePayload.Files) < 1 { - return errNoOutput + return nil // Allowed. } output, err := fd.NewFromFile(o.FilePayload.Files[0]) if err != nil { return err } + defer output.Close() - p.mu.Lock() - defer p.mu.Unlock() + p.traceMu.Lock() + defer p.traceMu.Unlock() // Returns an error if profiling is already started. if err := trace.Start(output); err != nil { output.Close() return err } + defer trace.Stop() // Ensure all trace contexts are registered. - p.Kernel.RebuildTraceContexts() - - p.traceFile = output - return nil -} - -// StopTrace is an RPC stub which stops collection of an ongoing execution -// trace and flushes the trace data. It takes no argument. -func (p *Profile) StopTrace(_, _ *struct{}) error { - p.mu.Lock() - defer p.mu.Unlock() + p.kernel.RebuildTraceContexts() - if p.traceFile == nil { - return errors.New("execution tracing not started") + // Wait for the trace. + select { + case <-time.After(o.Duration): + case <-p.done: } // Similarly to the case above, if tasks have not ended traces, we will // lose information. Thus we need to rebuild the tasks in order to have // complete information. This will not lose information if multiple // traces are overlapping. - p.Kernel.RebuildTraceContexts() + p.kernel.RebuildTraceContexts() - trace.Stop() - p.traceFile.Close() - p.traceFile = nil return nil } diff --git a/pkg/sentry/fsimpl/ext/inode.go b/pkg/sentry/fsimpl/ext/inode.go index 9009ba3c7..4a555bf72 100644 --- a/pkg/sentry/fsimpl/ext/inode.go +++ b/pkg/sentry/fsimpl/ext/inode.go @@ -200,7 +200,9 @@ func (in *inode) open(rp *vfs.ResolvingPath, vfsd *vfs.Dentry, opts *vfs.OpenOpt } var fd symlinkFD fd.LockFD.Init(&in.locks) - fd.vfsfd.Init(&fd, opts.Flags, mnt, vfsd, &vfs.FileDescriptionOptions{}) + if err := fd.vfsfd.Init(&fd, opts.Flags, mnt, vfsd, &vfs.FileDescriptionOptions{}); err != nil { + return nil, err + } return &fd.vfsfd, nil default: panic(fmt.Sprintf("unknown inode type: %T", in.impl)) diff --git a/pkg/sentry/fsimpl/fuse/fusefs.go b/pkg/sentry/fsimpl/fuse/fusefs.go index 3af807a21..204d8d143 100644 --- a/pkg/sentry/fsimpl/fuse/fusefs.go +++ b/pkg/sentry/fsimpl/fuse/fusefs.go @@ -129,6 +129,9 @@ func (fsType FilesystemType) GetFilesystem(ctx context.Context, vfsObj *vfs.Virt return nil, nil, syserror.EINVAL } fuseFDGeneric := kernelTask.GetFileVFS2(int32(deviceDescriptor)) + if fuseFDGeneric == nil { + return nil, nil, syserror.EINVAL + } defer fuseFDGeneric.DecRef(ctx) fuseFD, ok := fuseFDGeneric.Impl().(*DeviceFD) if !ok { diff --git a/pkg/sentry/fsimpl/pipefs/pipefs.go b/pkg/sentry/fsimpl/pipefs/pipefs.go index 0ecb592cf..429733c10 100644 --- a/pkg/sentry/fsimpl/pipefs/pipefs.go +++ b/pkg/sentry/fsimpl/pipefs/pipefs.go @@ -164,11 +164,11 @@ func (i *inode) StatFS(ctx context.Context, fs *vfs.Filesystem) (linux.Statfs, e // and write ends of a newly-created pipe, as for pipe(2) and pipe2(2). // // Preconditions: mnt.Filesystem() must have been returned by NewFilesystem(). -func NewConnectedPipeFDs(ctx context.Context, mnt *vfs.Mount, flags uint32) (*vfs.FileDescription, *vfs.FileDescription) { +func NewConnectedPipeFDs(ctx context.Context, mnt *vfs.Mount, flags uint32) (*vfs.FileDescription, *vfs.FileDescription, error) { fs := mnt.Filesystem().Impl().(*filesystem) inode := newInode(ctx, fs) var d kernfs.Dentry d.Init(&fs.Filesystem, inode) defer d.DecRef(ctx) - return inode.pipe.ReaderWriterPair(mnt, d.VFSDentry(), flags) + return inode.pipe.ReaderWriterPair(ctx, mnt, d.VFSDentry(), flags) } diff --git a/pkg/sentry/kernel/pipe/vfs.go b/pkg/sentry/kernel/pipe/vfs.go index 7b23cbe86..2d47d2e82 100644 --- a/pkg/sentry/kernel/pipe/vfs.go +++ b/pkg/sentry/kernel/pipe/vfs.go @@ -63,10 +63,19 @@ func NewVFSPipe(isNamed bool, sizeBytes int64) *VFSPipe { // ReaderWriterPair returns read-only and write-only FDs for vp. // // Preconditions: statusFlags should not contain an open access mode. -func (vp *VFSPipe) ReaderWriterPair(mnt *vfs.Mount, vfsd *vfs.Dentry, statusFlags uint32) (*vfs.FileDescription, *vfs.FileDescription) { +func (vp *VFSPipe) ReaderWriterPair(ctx context.Context, mnt *vfs.Mount, vfsd *vfs.Dentry, statusFlags uint32) (*vfs.FileDescription, *vfs.FileDescription, error) { // Connected pipes share the same locks. locks := &vfs.FileLocks{} - return vp.newFD(mnt, vfsd, linux.O_RDONLY|statusFlags, locks), vp.newFD(mnt, vfsd, linux.O_WRONLY|statusFlags, locks) + r, err := vp.newFD(mnt, vfsd, linux.O_RDONLY|statusFlags, locks) + if err != nil { + return nil, nil, err + } + w, err := vp.newFD(mnt, vfsd, linux.O_WRONLY|statusFlags, locks) + if err != nil { + r.DecRef(ctx) + return nil, nil, err + } + return r, w, nil } // Allocate implements vfs.FileDescriptionImpl.Allocate. @@ -85,7 +94,10 @@ func (vp *VFSPipe) Open(ctx context.Context, mnt *vfs.Mount, vfsd *vfs.Dentry, s return nil, syserror.EINVAL } - fd := vp.newFD(mnt, vfsd, statusFlags, locks) + fd, err := vp.newFD(mnt, vfsd, statusFlags, locks) + if err != nil { + return nil, err + } // Named pipes have special blocking semantics during open: // @@ -137,16 +149,18 @@ func (vp *VFSPipe) Open(ctx context.Context, mnt *vfs.Mount, vfsd *vfs.Dentry, s } // Preconditions: vp.mu must be held. -func (vp *VFSPipe) newFD(mnt *vfs.Mount, vfsd *vfs.Dentry, statusFlags uint32, locks *vfs.FileLocks) *vfs.FileDescription { +func (vp *VFSPipe) newFD(mnt *vfs.Mount, vfsd *vfs.Dentry, statusFlags uint32, locks *vfs.FileLocks) (*vfs.FileDescription, error) { fd := &VFSPipeFD{ pipe: &vp.pipe, } fd.LockFD.Init(locks) - fd.vfsfd.Init(fd, statusFlags, mnt, vfsd, &vfs.FileDescriptionOptions{ + if err := fd.vfsfd.Init(fd, statusFlags, mnt, vfsd, &vfs.FileDescriptionOptions{ DenyPRead: true, DenyPWrite: true, UseDentryMetadata: true, - }) + }); err != nil { + return nil, err + } switch { case fd.vfsfd.IsReadable() && fd.vfsfd.IsWritable(): @@ -160,7 +174,7 @@ func (vp *VFSPipe) newFD(mnt *vfs.Mount, vfsd *vfs.Dentry, statusFlags uint32, l panic("invalid pipe flags: must be readable, writable, or both") } - return &fd.vfsfd + return &fd.vfsfd, nil } // VFSPipeFD implements vfs.FileDescriptionImpl for pipes. It also implements diff --git a/pkg/sentry/socket/control/BUILD b/pkg/sentry/socket/control/BUILD index fb7c5dc61..ebcc891b3 100644 --- a/pkg/sentry/socket/control/BUILD +++ b/pkg/sentry/socket/control/BUILD @@ -1,4 +1,4 @@ -load("//tools:defs.bzl", "go_library") +load("//tools:defs.bzl", "go_library", "go_test") package(licenses = ["notice"]) @@ -26,3 +26,17 @@ go_library( "//pkg/usermem", ], ) + +go_test( + name = "control_test", + size = "small", + srcs = ["control_test.go"], + library = ":control", + deps = [ + "//pkg/abi/linux", + "//pkg/binary", + "//pkg/sentry/socket", + "//pkg/usermem", + "@com_github_google_go_cmp//cmp:go_default_library", + ], +) diff --git a/pkg/sentry/socket/control/control.go b/pkg/sentry/socket/control/control.go index ff6b71802..65b556489 100644 --- a/pkg/sentry/socket/control/control.go +++ b/pkg/sentry/socket/control/control.go @@ -463,7 +463,7 @@ func CmsgsSpace(t *kernel.Task, cmsgs socket.ControlMessages) int { } // Parse parses a raw socket control message into portable objects. -func Parse(t *kernel.Task, socketOrEndpoint interface{}, buf []byte) (socket.ControlMessages, error) { +func Parse(t *kernel.Task, socketOrEndpoint interface{}, buf []byte, width uint) (socket.ControlMessages, error) { var ( cmsgs socket.ControlMessages fds linux.ControlMessageRights @@ -487,10 +487,6 @@ func Parse(t *kernel.Task, socketOrEndpoint interface{}, buf []byte) (socket.Con i += linux.SizeOfControlMessageHeader length := int(h.Length) - linux.SizeOfControlMessageHeader - // The use of t.Arch().Width() is analogous to Linux's use of - // sizeof(long) in CMSG_ALIGN. - width := t.Arch().Width() - switch h.Level { case linux.SOL_SOCKET: switch h.Type { @@ -526,8 +522,10 @@ func Parse(t *kernel.Task, socketOrEndpoint interface{}, buf []byte) (socket.Con if length < linux.SizeOfTimeval { return socket.ControlMessages{}, syserror.EINVAL } + var ts linux.Timeval + binary.Unmarshal(buf[i:i+linux.SizeOfTimeval], usermem.ByteOrder, &ts) + cmsgs.IP.Timestamp = ts.ToNsecCapped() cmsgs.IP.HasTimestamp = true - binary.Unmarshal(buf[i:i+linux.SizeOfTimeval], usermem.ByteOrder, &cmsgs.IP.Timestamp) i += binary.AlignUp(length, width) default: diff --git a/pkg/sentry/socket/control/control_test.go b/pkg/sentry/socket/control/control_test.go new file mode 100644 index 000000000..d40a4cc85 --- /dev/null +++ b/pkg/sentry/socket/control/control_test.go @@ -0,0 +1,59 @@ +// Copyright 2020 The gVisor Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Package control provides internal representations of socket control +// messages. +package control + +import ( + "testing" + + "github.com/google/go-cmp/cmp" + "gvisor.dev/gvisor/pkg/abi/linux" + "gvisor.dev/gvisor/pkg/binary" + "gvisor.dev/gvisor/pkg/sentry/socket" + "gvisor.dev/gvisor/pkg/usermem" +) + +func TestParse(t *testing.T) { + // Craft the control message to parse. + length := linux.SizeOfControlMessageHeader + linux.SizeOfTimeval + hdr := linux.ControlMessageHeader{ + Length: uint64(length), + Level: linux.SOL_SOCKET, + Type: linux.SO_TIMESTAMP, + } + buf := make([]byte, 0, length) + buf = binary.Marshal(buf, usermem.ByteOrder, &hdr) + ts := linux.Timeval{ + Sec: 2401, + Usec: 343, + } + buf = binary.Marshal(buf, usermem.ByteOrder, &ts) + + cmsg, err := Parse(nil, nil, buf, 8 /* width */) + if err != nil { + t.Fatalf("Parse(_, _, %+v, _): %v", cmsg, err) + } + + want := socket.ControlMessages{ + IP: socket.IPControlMessages{ + HasTimestamp: true, + Timestamp: ts.ToNsecCapped(), + }, + } + if diff := cmp.Diff(want, cmsg); diff != "" { + t.Errorf("unexpected message parsed, (-want, +got):\n%s", diff) + } +} diff --git a/pkg/sentry/syscalls/linux/sys_socket.go b/pkg/sentry/syscalls/linux/sys_socket.go index 4adfa6637..fe45225c1 100644 --- a/pkg/sentry/syscalls/linux/sys_socket.go +++ b/pkg/sentry/syscalls/linux/sys_socket.go @@ -1030,7 +1030,7 @@ func sendSingleMsg(t *kernel.Task, s socket.Socket, file *fs.File, msgPtr userme return 0, err } - controlMessages, err := control.Parse(t, s, controlData) + controlMessages, err := control.Parse(t, s, controlData, t.Arch().Width()) if err != nil { return 0, err } diff --git a/pkg/sentry/syscalls/linux/vfs2/pipe.go b/pkg/sentry/syscalls/linux/vfs2/pipe.go index ee38fdca0..6986e39fe 100644 --- a/pkg/sentry/syscalls/linux/vfs2/pipe.go +++ b/pkg/sentry/syscalls/linux/vfs2/pipe.go @@ -42,7 +42,10 @@ func pipe2(t *kernel.Task, addr usermem.Addr, flags int32) error { if flags&^(linux.O_NONBLOCK|linux.O_CLOEXEC) != 0 { return syserror.EINVAL } - r, w := pipefs.NewConnectedPipeFDs(t, t.Kernel().PipeMount(), uint32(flags&linux.O_NONBLOCK)) + r, w, err := pipefs.NewConnectedPipeFDs(t, t.Kernel().PipeMount(), uint32(flags&linux.O_NONBLOCK)) + if err != nil { + return err + } defer r.DecRef(t) defer w.DecRef(t) diff --git a/pkg/sentry/syscalls/linux/vfs2/socket.go b/pkg/sentry/syscalls/linux/vfs2/socket.go index 987012acc..f5795b4a8 100644 --- a/pkg/sentry/syscalls/linux/vfs2/socket.go +++ b/pkg/sentry/syscalls/linux/vfs2/socket.go @@ -1033,7 +1033,7 @@ func sendSingleMsg(t *kernel.Task, s socket.SocketVFS2, file *vfs.FileDescriptio return 0, err } - controlMessages, err := control.Parse(t, s, controlData) + controlMessages, err := control.Parse(t, s, controlData, t.Arch().Width()) if err != nil { return 0, err } diff --git a/pkg/state/tests/register_test.go b/pkg/state/tests/register_test.go index c829753cc..75bdbfc6e 100644 --- a/pkg/state/tests/register_test.go +++ b/pkg/state/tests/register_test.go @@ -12,6 +12,8 @@ // See the License for the specific language governing permissions and // limitations under the License. +// +build race + package tests import ( @@ -165,3 +167,12 @@ func TestRegisterBad(t *testing.T) { } } + +func TestRegisterTypeOnlyStruct(t *testing.T) { + defer func() { + if r := recover(); r == nil { + t.Errorf("Register did not panic") + } + }() + state.Register((*typeOnlyEmptyStruct)(nil)) +} diff --git a/pkg/state/tests/struct_test.go b/pkg/state/tests/struct_test.go index c91c2c032..9826f1ee9 100644 --- a/pkg/state/tests/struct_test.go +++ b/pkg/state/tests/struct_test.go @@ -17,8 +17,6 @@ package tests import ( "math/rand" "testing" - - "gvisor.dev/gvisor/pkg/state" ) func TestEmptyStruct(t *testing.T) { @@ -58,15 +56,6 @@ func TestEmptyStruct(t *testing.T) { }) } -func TestRegisterTypeOnlyStruct(t *testing.T) { - defer func() { - if r := recover(); r == nil { - t.Errorf("Register did not panic") - } - }() - state.Register((*typeOnlyEmptyStruct)(nil)) -} - func TestEmbeddedPointers(t *testing.T) { // Give each int64 a random value to prevent Go from using // runtime.staticuint64s, which confounds tests for struct duplication. diff --git a/pkg/state/types.go b/pkg/state/types.go index 84aed8732..420675880 100644 --- a/pkg/state/types.go +++ b/pkg/state/types.go @@ -329,47 +329,48 @@ var reverseTypeDatabase = map[reflect.Type]string{} // This must be called on init and only done once. func Register(t Type) { name := t.StateTypeName() - fields := t.StateFields() - assertValidType(name, fields) - // Register must always be called on pointers. typ := reflect.TypeOf(t) - if typ.Kind() != reflect.Ptr { - Failf("Register must be called on pointers") + if raceEnabled { + assertValidType(name, t.StateFields()) + // Register must always be called on pointers. + if typ.Kind() != reflect.Ptr { + Failf("Register must be called on pointers") + } } typ = typ.Elem() - if typ.Kind() == reflect.Struct { - // All registered structs must implement SaverLoader. We allow - // the registration is non-struct types with just the Type - // interface, but we need to call StateSave/StateLoad methods - // on aggregate types. - if _, ok := t.(SaverLoader); !ok { - Failf("struct %T does not implement SaverLoader", t) + if raceEnabled { + if typ.Kind() == reflect.Struct { + // All registered structs must implement SaverLoader. We allow + // the registration is non-struct types with just the Type + // interface, but we need to call StateSave/StateLoad methods + // on aggregate types. + if _, ok := t.(SaverLoader); !ok { + Failf("struct %T does not implement SaverLoader", t) + } + } else { + // Non-structs must not have any fields. We don't support + // calling StateSave/StateLoad methods on any non-struct types. + // If custom behavior is required, these types should be + // wrapped in a structure of some kind. + if fields := t.StateFields(); len(fields) != 0 { + Failf("non-struct %T has non-zero fields %v", t, fields) + } + // We don't allow non-structs to implement StateSave/StateLoad + // methods, because they won't be called and it's confusing. + if _, ok := t.(SaverLoader); ok { + Failf("non-struct %T implements SaverLoader", t) + } } - } else { - // Non-structs must not have any fields. We don't support - // calling StateSave/StateLoad methods on any non-struct types. - // If custom behavior is required, these types should be - // wrapped in a structure of some kind. - if len(fields) != 0 { - Failf("non-struct %T has non-zero fields %v", t, fields) + if _, ok := primitiveTypeDatabase[name]; ok { + Failf("conflicting primitiveTypeDatabase entry for %T: used by primitive", t) } - // We don't allow non-structs to implement StateSave/StateLoad - // methods, because they won't be called and it's confusing. - if _, ok := t.(SaverLoader); ok { - Failf("non-struct %T implements SaverLoader", t) + if _, ok := globalTypeDatabase[name]; ok { + Failf("conflicting globalTypeDatabase entries for %T: name conflict", t) + } + if name == interfaceType { + Failf("conflicting name for %T: matches interfaceType", t) } - } - if _, ok := primitiveTypeDatabase[name]; ok { - Failf("conflicting primitiveTypeDatabase entry for %T: used by primitive", t) - } - if _, ok := globalTypeDatabase[name]; ok { - Failf("conflicting globalTypeDatabase entries for %T: name conflict", t) - } - if name == interfaceType { - Failf("conflicting name for %T: matches interfaceType", t) - } - globalTypeDatabase[name] = typ - if raceEnabled { reverseTypeDatabase[typ] = name } + globalTypeDatabase[name] = typ } diff --git a/pkg/test/dockerutil/container.go b/pkg/test/dockerutil/container.go index 2bf0a22ff..7bacb70d3 100644 --- a/pkg/test/dockerutil/container.go +++ b/pkg/test/dockerutil/container.go @@ -55,11 +55,8 @@ type Container struct { copyErr error cleanups []func() - // Profiles are profiles added to this container. They contain methods - // that are run after Creation, Start, and Cleanup of this Container, along - // a handle to restart the profile. Generally, tests/benchmarks using - // profiles need to run as root. - profiles []Profile + // profile is the profiling hook associated with this container. + profile *profile } // RunOpts are options for running a container. @@ -105,22 +102,7 @@ type RunOpts struct { Links []string } -// MakeContainer sets up the struct for a Docker container. -// -// Names of containers will be unique. -// Containers will check flags for profiling requests. -func MakeContainer(ctx context.Context, logger testutil.Logger) *Container { - c := MakeNativeContainer(ctx, logger) - c.runtime = *runtime - if p := MakePprofFromFlags(c); p != nil { - c.AddProfile(p) - } - return c -} - -// MakeNativeContainer sets up the struct for a DockerContainer using runc. Native -// containers aren't profiled. -func MakeNativeContainer(ctx context.Context, logger testutil.Logger) *Container { +func makeContainer(ctx context.Context, logger testutil.Logger, runtime string) *Container { // Slashes are not allowed in container names. name := testutil.RandomID(logger.Name()) name = strings.ReplaceAll(name, "/", "-") @@ -132,29 +114,32 @@ func MakeNativeContainer(ctx context.Context, logger testutil.Logger) *Container return &Container{ logger: logger, Name: name, - runtime: "", + runtime: runtime, client: client, } } -// AddProfile adds a profile to this container. -func (c *Container) AddProfile(p Profile) { - c.profiles = append(c.profiles, p) +// MakeContainer constructs a suitable Container object. +// +// The runtime used is determined by the runtime flag. +// +// Containers will check flags for profiling requests. +func MakeContainer(ctx context.Context, logger testutil.Logger) *Container { + return makeContainer(ctx, logger, *runtime) } -// RestartProfiles calls Restart on all profiles for this container. -func (c *Container) RestartProfiles() error { - for _, profile := range c.profiles { - if err := profile.Restart(c); err != nil { - return err - } - } - return nil +// MakeNativeContainer constructs a suitable Container object. +// +// The runtime used will be the system default. +// +// Native containers aren't profiled. +func MakeNativeContainer(ctx context.Context, logger testutil.Logger) *Container { + return makeContainer(ctx, logger, "" /*runtime*/) } // Spawn is analogous to 'docker run -d'. func (c *Container) Spawn(ctx context.Context, r RunOpts, args ...string) error { - if err := c.create(ctx, c.config(r, args), c.hostConfig(r), nil); err != nil { + if err := c.create(ctx, r.Image, c.config(r, args), c.hostConfig(r), nil); err != nil { return err } return c.Start(ctx) @@ -167,7 +152,7 @@ func (c *Container) SpawnProcess(ctx context.Context, r RunOpts, args ...string) config.Tty = true config.OpenStdin = true - if err := c.CreateFrom(ctx, config, hostconf, netconf); err != nil { + if err := c.CreateFrom(ctx, r.Image, config, hostconf, netconf); err != nil { return Process{}, err } @@ -194,7 +179,7 @@ func (c *Container) SpawnProcess(ctx context.Context, r RunOpts, args ...string) // Run is analogous to 'docker run'. func (c *Container) Run(ctx context.Context, r RunOpts, args ...string) (string, error) { - if err := c.create(ctx, c.config(r, args), c.hostConfig(r), nil); err != nil { + if err := c.create(ctx, r.Image, c.config(r, args), c.hostConfig(r), nil); err != nil { return "", err } @@ -221,26 +206,26 @@ func (c *Container) MakeLink(target string) string { } // CreateFrom creates a container from the given configs. -func (c *Container) CreateFrom(ctx context.Context, conf *container.Config, hostconf *container.HostConfig, netconf *network.NetworkingConfig) error { - return c.create(ctx, conf, hostconf, netconf) +func (c *Container) CreateFrom(ctx context.Context, profileImage string, conf *container.Config, hostconf *container.HostConfig, netconf *network.NetworkingConfig) error { + return c.create(ctx, profileImage, conf, hostconf, netconf) } // Create is analogous to 'docker create'. func (c *Container) Create(ctx context.Context, r RunOpts, args ...string) error { - return c.create(ctx, c.config(r, args), c.hostConfig(r), nil) + return c.create(ctx, r.Image, c.config(r, args), c.hostConfig(r), nil) } -func (c *Container) create(ctx context.Context, conf *container.Config, hostconf *container.HostConfig, netconf *network.NetworkingConfig) error { +func (c *Container) create(ctx context.Context, profileImage string, conf *container.Config, hostconf *container.HostConfig, netconf *network.NetworkingConfig) error { + if c.runtime != "" { + // Use the image name as provided here; which normally represents the + // unmodified "basic/alpine" image name. This should be easy to grok. + c.profileInit(profileImage) + } cont, err := c.client.ContainerCreate(ctx, conf, hostconf, nil, c.Name) if err != nil { return err } c.id = cont.ID - for _, profile := range c.profiles { - if err := profile.OnCreate(c); err != nil { - return fmt.Errorf("OnCreate method failed with: %v", err) - } - } return nil } @@ -286,11 +271,13 @@ func (c *Container) Start(ctx context.Context) error { if err := c.client.ContainerStart(ctx, c.id, types.ContainerStartOptions{}); err != nil { return fmt.Errorf("ContainerStart failed: %v", err) } - for _, profile := range c.profiles { - if err := profile.OnStart(c); err != nil { - return fmt.Errorf("OnStart method failed: %v", err) + + if c.profile != nil { + if err := c.profile.Start(c); err != nil { + c.logger.Logf("profile.Start failed: %v", err) } } + return nil } @@ -442,6 +429,7 @@ func (c *Container) Status(ctx context.Context) (types.ContainerState, error) { // Wait waits for the container to exit. func (c *Container) Wait(ctx context.Context) error { + defer c.stopProfiling() statusChan, errChan := c.client.ContainerWait(ctx, c.id, container.WaitConditionNotRunning) select { case err := <-errChan: @@ -499,8 +487,20 @@ func (c *Container) WaitForOutputSubmatch(ctx context.Context, pattern string, t } } +// stopProfiling stops profiling. +func (c *Container) stopProfiling() { + if c.profile != nil { + if err := c.profile.Stop(c); err != nil { + // This most likely means that the runtime for the container + // was too short to connect and actually get a profile. + c.logger.Logf("warning: profile.Stop failed: %v", err) + } + } +} + // Kill kills the container. func (c *Container) Kill(ctx context.Context) error { + defer c.stopProfiling() return c.client.ContainerKill(ctx, c.id, "") } @@ -517,14 +517,6 @@ func (c *Container) Remove(ctx context.Context) error { // CleanUp kills and deletes the container (best effort). func (c *Container) CleanUp(ctx context.Context) { - // Execute profile cleanups before the container goes down. - for _, profile := range c.profiles { - profile.OnCleanUp(c) - } - - // Forget profiles. - c.profiles = nil - // Execute all cleanups. We execute cleanups here to close any // open connections to the container before closing. Open connections // can cause Kill and Remove to hang. @@ -538,10 +530,12 @@ func (c *Container) CleanUp(ctx context.Context) { // Just log; can't do anything here. c.logger.Logf("error killing container %q: %v", c.Name, err) } + // Remove the image. if err := c.Remove(ctx); err != nil { c.logger.Logf("error removing container %q: %v", c.Name, err) } + // Forget all mounts. c.mounts = nil } diff --git a/pkg/test/dockerutil/dockerutil.go b/pkg/test/dockerutil/dockerutil.go index 7027df1a5..a40005799 100644 --- a/pkg/test/dockerutil/dockerutil.go +++ b/pkg/test/dockerutil/dockerutil.go @@ -49,15 +49,11 @@ var ( // pprofBaseDir allows the user to change the directory to which profiles are // written. By default, profiles will appear under: // /tmp/profile/RUNTIME/CONTAINER_NAME/*.pprof. - pprofBaseDir = flag.String("pprof-dir", "/tmp/profile", "base directory in: BASEDIR/RUNTIME/CONTINER_NAME/FILENAME (e.g. /tmp/profile/runtime/mycontainer/cpu.pprof)") - - // duration is the max duration `runsc debug` will run and capture profiles. - // If the container's clean up method is called prior to duration, the - // profiling process will be killed. - duration = flag.Duration("pprof-duration", 10*time.Second, "duration to run the profile in seconds") + pprofBaseDir = flag.String("pprof-dir", "/tmp/profile", "base directory in: BASEDIR/RUNTIME/CONTINER_NAME/FILENAME (e.g. /tmp/profile/runtime/mycontainer/cpu.pprof)") + pprofDuration = flag.Duration("pprof-duration", time.Hour, "profiling duration (automatically stopped at container exit)") // The below flags enable each type of profile. Multiple profiles can be - // enabled for each run. + // enabled for each run. The profile will be collected from the start. pprofBlock = flag.Bool("pprof-block", false, "enables block profiling with runsc debug") pprofCPU = flag.Bool("pprof-cpu", false, "enables CPU profiling with runsc debug") pprofHeap = flag.Bool("pprof-heap", false, "enables heap profiling with runsc debug") diff --git a/pkg/test/dockerutil/profile.go b/pkg/test/dockerutil/profile.go index 55f9496cd..5cad3e959 100644 --- a/pkg/test/dockerutil/profile.go +++ b/pkg/test/dockerutil/profile.go @@ -17,72 +17,64 @@ package dockerutil import ( "context" "fmt" - "io" "os" "os/exec" "path/filepath" + "syscall" "time" ) -// Profile represents profile-like operations on a container, -// such as running perf or pprof. It is meant to be added to containers -// such that the container type calls the Profile during its lifecycle. -type Profile interface { - // OnCreate is called just after the container is created when the container - // has a valid ID (e.g. c.ID()). - OnCreate(c *Container) error - - // OnStart is called just after the container is started when the container - // has a valid Pid (e.g. c.SandboxPid()). - OnStart(c *Container) error - - // Restart restarts the Profile on request. - Restart(c *Container) error - - // OnCleanUp is called during the container's cleanup method. - // Cleanups should just log errors if they have them. - OnCleanUp(c *Container) error -} - -// Pprof is for running profiles with 'runsc debug'. Pprof workloads -// should be run as root and ONLY against runsc sandboxes. The runtime -// should have --profile set as an option in /etc/docker/daemon.json in -// order for profiling to work with Pprof. -type Pprof struct { - BasePath string // path to put profiles - BlockProfile bool - CPUProfile bool - HeapProfile bool - MutexProfile bool - Duration time.Duration // duration to run profiler e.g. '10s' or '1m'. - shouldRun bool - cmd *exec.Cmd - stdout io.ReadCloser - stderr io.ReadCloser +// profile represents profile-like operations on a container. +// +// It is meant to be added to containers such that the container type calls +// the profile during its lifecycle. Standard implementations are below. + +// profile is for running profiles with 'runsc debug'. +type profile struct { + BasePath string + Types []string + Duration time.Duration + cmd *exec.Cmd } -// MakePprofFromFlags makes a Pprof profile from flags. -func MakePprofFromFlags(c *Container) *Pprof { - if !(*pprofBlock || *pprofCPU || *pprofHeap || *pprofMutex) { - return nil +// profileInit initializes a profile object, if required. +// +// N.B. The profiling filename initialized here will use the *image* +// name, and not the unique container name. This is intentional. Most +// of the time, profiling will be used for benchmarks. Benchmarks will +// be run iteratively until a sufficiently large N is reached. It is +// useful in this context to overwrite previous runs, and generate a +// single profile result for the final test. +func (c *Container) profileInit(image string) { + if !*pprofBlock && !*pprofCPU && !*pprofMutex && !*pprofHeap { + return // Nothing to do. + } + c.profile = &profile{ + BasePath: filepath.Join(*pprofBaseDir, c.runtime, c.logger.Name(), image), + Duration: *pprofDuration, + } + if *pprofCPU { + c.profile.Types = append(c.profile.Types, "cpu") } - return &Pprof{ - BasePath: filepath.Join(*pprofBaseDir, c.runtime, c.Name), - BlockProfile: *pprofBlock, - CPUProfile: *pprofCPU, - HeapProfile: *pprofHeap, - MutexProfile: *pprofMutex, - Duration: *duration, + if *pprofHeap { + c.profile.Types = append(c.profile.Types, "heap") + } + if *pprofMutex { + c.profile.Types = append(c.profile.Types, "mutex") + } + if *pprofBlock { + c.profile.Types = append(c.profile.Types, "block") } } -// OnCreate implements Profile.OnCreate. -func (p *Pprof) OnCreate(c *Container) error { - return os.MkdirAll(p.BasePath, 0755) -} +// createProcess creates the collection process. +func (p *profile) createProcess(c *Container) error { + // Ensure our directory exists. + if err := os.MkdirAll(p.BasePath, 0755); err != nil { + return err + } -// OnStart implements Profile.OnStart. -func (p *Pprof) OnStart(c *Container) error { + // Find the runtime to invoke. path, err := RuntimePath() if err != nil { return fmt.Errorf("failed to get runtime path: %v", err) @@ -90,58 +82,63 @@ func (p *Pprof) OnStart(c *Container) error { // The root directory of this container's runtime. root := fmt.Sprintf("--root=/var/run/docker/runtime-%s/moby", c.runtime) - // Format is `runsc --root=rootdir debug --profile-*=file --duration=* containerID`. + + // Format is `runsc --root=rootdir debug --profile-*=file --duration=24h containerID`. args := []string{root, "debug"} - args = append(args, p.makeProfileArgs(c)...) + for _, profileArg := range p.Types { + outputPath := filepath.Join(p.BasePath, fmt.Sprintf("%s.pprof", profileArg)) + args = append(args, fmt.Sprintf("--profile-%s=%s", profileArg, outputPath)) + } + args = append(args, fmt.Sprintf("--duration=%s", p.Duration)) // Or until container exits. + args = append(args, fmt.Sprintf("--delay=%s", p.Duration)) // Ditto. args = append(args, c.ID()) // Best effort wait until container is running. for now := time.Now(); time.Since(now) < 5*time.Second; { if status, err := c.Status(context.Background()); err != nil { return fmt.Errorf("failed to get status with: %v", err) - } else if status.Running { break } - time.Sleep(500 * time.Millisecond) + time.Sleep(100 * time.Millisecond) } p.cmd = exec.Command(path, args...) + p.cmd.Stderr = os.Stderr // Pass through errors. if err := p.cmd.Start(); err != nil { - return fmt.Errorf("process failed: %v", err) + return fmt.Errorf("start process failed: %v", err) } + return nil } -// Restart implements Profile.Restart. -func (p *Pprof) Restart(c *Container) error { - p.OnCleanUp(c) - return p.OnStart(c) +// killProcess kills the process, if running. +func (p *profile) killProcess() error { + if p.cmd != nil && p.cmd.Process != nil { + return p.cmd.Process.Signal(syscall.SIGTERM) + } + return nil } -// OnCleanUp implements Profile.OnCleanup -func (p *Pprof) OnCleanUp(c *Container) error { +// waitProcess waits for the process, if running. +func (p *profile) waitProcess() error { defer func() { p.cmd = nil }() - if p.cmd != nil && p.cmd.Process != nil && p.cmd.ProcessState != nil && !p.cmd.ProcessState.Exited() { - return p.cmd.Process.Kill() + if p.cmd != nil { + return p.cmd.Wait() } return nil } -// makeProfileArgs turns Pprof fields into runsc debug flags. -func (p *Pprof) makeProfileArgs(c *Container) []string { - var ret []string - if p.BlockProfile { - ret = append(ret, fmt.Sprintf("--profile-block=%s", filepath.Join(p.BasePath, "block.pprof"))) - } - if p.CPUProfile { - ret = append(ret, fmt.Sprintf("--profile-cpu=%s", filepath.Join(p.BasePath, "cpu.pprof"))) - } - if p.HeapProfile { - ret = append(ret, fmt.Sprintf("--profile-heap=%s", filepath.Join(p.BasePath, "heap.pprof"))) - } - if p.MutexProfile { - ret = append(ret, fmt.Sprintf("--profile-mutex=%s", filepath.Join(p.BasePath, "mutex.pprof"))) +// Start is called when profiling is started. +func (p *profile) Start(c *Container) error { + return p.createProcess(c) +} + +// Stop is called when profiling is started. +func (p *profile) Stop(c *Container) error { + killErr := p.killProcess() + waitErr := p.waitProcess() + if waitErr != nil && killErr != nil { + return killErr } - ret = append(ret, fmt.Sprintf("--duration=%s", p.Duration)) - return ret + return waitErr // Ignore okay wait, err kill. } diff --git a/pkg/test/dockerutil/profile_test.go b/pkg/test/dockerutil/profile_test.go index 8c4ffe483..4fe9ce15c 100644 --- a/pkg/test/dockerutil/profile_test.go +++ b/pkg/test/dockerutil/profile_test.go @@ -17,6 +17,7 @@ package dockerutil import ( "context" "fmt" + "io/ioutil" "os" "path/filepath" "testing" @@ -25,52 +26,60 @@ import ( type testCase struct { name string - pprof Pprof + profile profile expectedFiles []string } -func TestPprof(t *testing.T) { +func TestProfile(t *testing.T) { // Basepath and expected file names for each type of profile. - basePath := "/tmp/test/profile" + tmpDir, err := ioutil.TempDir("", "") + if err != nil { + t.Fatalf("unable to create temporary directory: %v", err) + } + defer os.RemoveAll(tmpDir) + + // All expected names. + basePath := tmpDir block := "block.pprof" cpu := "cpu.pprof" - goprofle := "go.pprof" heap := "heap.pprof" mutex := "mutex.pprof" testCases := []testCase{ { - name: "Cpu", - pprof: Pprof{ - BasePath: basePath, - CPUProfile: true, - Duration: 2 * time.Second, + name: "One", + profile: profile{ + BasePath: basePath, + Types: []string{"cpu"}, + Duration: 2 * time.Second, }, expectedFiles: []string{cpu}, }, { name: "All", - pprof: Pprof{ - BasePath: basePath, - BlockProfile: true, - CPUProfile: true, - HeapProfile: true, - MutexProfile: true, - Duration: 2 * time.Second, + profile: profile{ + BasePath: basePath, + Types: []string{"block", "cpu", "heap", "mutex"}, + Duration: 2 * time.Second, }, - expectedFiles: []string{block, cpu, goprofle, heap, mutex}, + expectedFiles: []string{block, cpu, heap, mutex}, }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { ctx := context.Background() c := MakeContainer(ctx, t) + // Set basepath to include the container name so there are no conflicts. - tc.pprof.BasePath = filepath.Join(tc.pprof.BasePath, c.Name) - c.AddProfile(&tc.pprof) + localProfile := tc.profile // Copy it. + localProfile.BasePath = filepath.Join(localProfile.BasePath, tc.name) + + // Set directly on the container, to avoid flags. + c.profile = &localProfile func() { defer c.CleanUp(ctx) + // Start a container. if err := c.Spawn(ctx, RunOpts{ Image: "basic/alpine", @@ -83,24 +92,24 @@ func TestPprof(t *testing.T) { } // End early if the expected files exist and have data. - for start := time.Now(); time.Since(start) < tc.pprof.Duration; time.Sleep(500 * time.Millisecond) { - if err := checkFiles(tc); err == nil { + for start := time.Now(); time.Since(start) < localProfile.Duration; time.Sleep(100 * time.Millisecond) { + if err := checkFiles(localProfile.BasePath, tc.expectedFiles); err == nil { break } } }() // Check all expected files exist and have data. - if err := checkFiles(tc); err != nil { + if err := checkFiles(localProfile.BasePath, tc.expectedFiles); err != nil { t.Fatalf(err.Error()) } }) } } -func checkFiles(tc testCase) error { - for _, file := range tc.expectedFiles { - stat, err := os.Stat(filepath.Join(tc.pprof.BasePath, file)) +func checkFiles(basePath string, expectedFiles []string) error { + for _, file := range expectedFiles { + stat, err := os.Stat(filepath.Join(basePath, file)) if err != nil { return fmt.Errorf("stat failed with: %v", err) } else if stat.Size() < 1 { diff --git a/pkg/urpc/urpc.go b/pkg/urpc/urpc.go index 13b2ea314..0e9a829f6 100644 --- a/pkg/urpc/urpc.go +++ b/pkg/urpc/urpc.go @@ -170,6 +170,9 @@ type Server struct { // methods is the set of server methods. methods map[string]registeredMethod + // stoppers are all registered stoppers. + stoppers []Stopper + // clients is a map of clients. clients map[*unet.Socket]clientState @@ -195,6 +198,12 @@ func NewServerWithCallback(afterRPCCallback func()) *Server { } } +// Stopper is an optional interface, that when implemented, allows an object +// to have a callback executed when the server is shutting down. +type Stopper interface { + Stop() +} + // Register registers the given object as an RPC receiver. // // This functions is the same way as the built-in RPC package, but it does not @@ -206,6 +215,7 @@ func (s *Server) Register(obj interface{}) { defer s.mu.Unlock() typ := reflect.TypeOf(obj) + stopper, hasStop := obj.(Stopper) // If we got a pointer, deref it to the underlying object. We need this to // obtain the name of the underlying type. @@ -221,6 +231,10 @@ func (s *Server) Register(obj interface{}) { // Can't be anonymous. panic("type not named.") } + if hasStop && method.Name == "Stop" { + s.stoppers = append(s.stoppers, stopper) + continue // Legal stop method. + } prettyName := typDeref.Name() + "." + method.Name if _, ok := s.methods[prettyName]; ok { @@ -283,12 +297,10 @@ func (s *Server) handleOne(client *unet.Socket) error { // Client is dead. return err } + if s.afterRPCCallback != nil { + defer s.afterRPCCallback() + } - defer func() { - if s.afterRPCCallback != nil { - s.afterRPCCallback() - } - }() // Explicitly close all these files after the call. // // This is also explicitly a reference to the files after the call, @@ -450,6 +462,11 @@ func (s *Server) Stop() { // Wait for all outstanding requests. defer s.wg.Wait() + // Call any Stop callbacks. + for _, stopper := range s.stoppers { + stopper.Stop() + } + // Close all known clients. s.mu.Lock() defer s.mu.Unlock() diff --git a/runsc/boot/BUILD b/runsc/boot/BUILD index 8c73dc5dc..67307ab3c 100644 --- a/runsc/boot/BUILD +++ b/runsc/boot/BUILD @@ -33,6 +33,7 @@ go_library( "//pkg/cpuid", "//pkg/eventchannel", "//pkg/fd", + "//pkg/flipcall", "//pkg/fspath", "//pkg/log", "//pkg/memutil", diff --git a/runsc/boot/controller.go b/runsc/boot/controller.go index 865126ac5..cb5d8ea31 100644 --- a/runsc/boot/controller.go +++ b/runsc/boot/controller.go @@ -104,13 +104,11 @@ const ( // Profiling related commands (see pprof.go for more details). const ( - StartCPUProfile = "Profile.StartCPUProfile" - StopCPUProfile = "Profile.StopCPUProfile" - HeapProfile = "Profile.HeapProfile" - BlockProfile = "Profile.BlockProfile" - MutexProfile = "Profile.MutexProfile" - StartTrace = "Profile.StartTrace" - StopTrace = "Profile.StopTrace" + CPUProfile = "Profile.CPU" + HeapProfile = "Profile.Heap" + BlockProfile = "Profile.Block" + MutexProfile = "Profile.Mutex" + Trace = "Profile.Trace" ) // Logging related commands (see logging.go for more details). @@ -131,9 +129,6 @@ type controller struct { // manager holds the containerManager methods. manager *containerManager - - // pprop holds the profile instance if enabled. It may be nil. - pprof *control.Profile } // newController creates a new controller. The caller must call @@ -164,19 +159,14 @@ func newController(fd int, l *Loader) (*controller, error) { ctrl.srv.Register(&control.Logging{}) if l.root.conf.ProfileEnable { - ctrl.pprof = &control.Profile{Kernel: l.k} - ctrl.srv.Register(ctrl.pprof) + ctrl.srv.Register(control.NewProfile(l.k)) } return ctrl, nil } func (c *controller) stop() { - if c.pprof != nil { - // These are noop if there is nothing being profiled. - _ = c.pprof.StopCPUProfile(nil, nil) - _ = c.pprof.StopTrace(nil, nil) - } + c.srv.Stop() } // containerManager manages sandbox containers. diff --git a/runsc/boot/loader.go b/runsc/boot/loader.go index 98ea8db64..f41d6c665 100644 --- a/runsc/boot/loader.go +++ b/runsc/boot/loader.go @@ -598,7 +598,6 @@ func (l *Loader) run() error { if err != nil { return err } - } ep.tg = l.k.GlobalInit() @@ -1045,9 +1044,10 @@ func (l *Loader) WaitExit() kernel.ExitStatus { // Wait for container. l.k.WaitExited() - // Cleanup + // Stop the control server. l.ctrl.stop() + // Check all references. refs.OnExit() return l.k.GlobalInit().ExitStatus() diff --git a/runsc/cmd/debug.go b/runsc/cmd/debug.go index 1e5a7471a..b84142b0d 100644 --- a/runsc/cmd/debug.go +++ b/runsc/cmd/debug.go @@ -17,8 +17,10 @@ package cmd import ( "context" "os" + "os/signal" "strconv" "strings" + "sync" "syscall" "time" @@ -43,6 +45,7 @@ type Debug struct { strace string logLevel string logPackets string + delay time.Duration duration time.Duration ps bool } @@ -70,10 +73,11 @@ func (d *Debug) SetFlags(f *flag.FlagSet) { f.StringVar(&d.profileCPU, "profile-cpu", "", "writes CPU profile to the given file.") f.StringVar(&d.profileBlock, "profile-block", "", "writes block profile to the given file.") f.StringVar(&d.profileMutex, "profile-mutex", "", "writes mutex profile to the given file.") - f.DurationVar(&d.duration, "duration", time.Second, "amount of time to wait for CPU and trace profiles") + f.DurationVar(&d.delay, "delay", time.Hour, "amount of time to delay for collecting heap and goroutine profiles.") + f.DurationVar(&d.duration, "duration", time.Hour, "amount of time to wait for CPU and trace profiles.") f.StringVar(&d.trace, "trace", "", "writes an execution trace to the given file.") f.IntVar(&d.signal, "signal", -1, "sends signal to the sandbox") - f.StringVar(&d.strace, "strace", "", `A comma separated list of syscalls to trace. "all" enables all traces, "off" disables all`) + f.StringVar(&d.strace, "strace", "", `A comma separated list of syscalls to trace. "all" enables all traces, "off" disables all.`) f.StringVar(&d.logLevel, "log-level", "", "The log level to set: warning (0), info (1), or debug (2).") f.StringVar(&d.logPackets, "log-packets", "", "A boolean value to enable or disable packet logging: true or false.") f.BoolVar(&d.ps, "ps", false, "lists processes") @@ -123,11 +127,12 @@ func (d *Debug) Execute(_ context.Context, f *flag.FlagSet, args ...interface{}) } } - if c.IsSandboxRunning() { + if !c.IsSandboxRunning() { return Errorf("container sandbox is not running") } log.Infof("Found sandbox %q, PID: %d", c.Sandbox.ID, c.Sandbox.Pid) + // Perform synchronous actions. if d.signal > 0 { log.Infof("Sending signal %d to process: %d", d.signal, c.Sandbox.Pid) if err := syscall.Kill(c.Sandbox.Pid, syscall.Signal(d.signal)); err != nil { @@ -142,81 +147,6 @@ func (d *Debug) Execute(_ context.Context, f *flag.FlagSet, args ...interface{}) } log.Infof(" *** Stack dump ***\n%s", stacks) } - if d.profileHeap != "" { - f, err := os.Create(d.profileHeap) - if err != nil { - return Errorf(err.Error()) - } - defer f.Close() - - if err := c.Sandbox.HeapProfile(f); err != nil { - return Errorf(err.Error()) - } - log.Infof("Heap profile written to %q", d.profileHeap) - } - if d.profileBlock != "" { - f, err := os.Create(d.profileBlock) - if err != nil { - return Errorf(err.Error()) - } - defer f.Close() - - if err := c.Sandbox.BlockProfile(f); err != nil { - return Errorf(err.Error()) - } - log.Infof("Block profile written to %q", d.profileBlock) - } - if d.profileMutex != "" { - f, err := os.Create(d.profileMutex) - if err != nil { - return Errorf(err.Error()) - } - defer f.Close() - - if err := c.Sandbox.MutexProfile(f); err != nil { - return Errorf(err.Error()) - } - log.Infof("Mutex profile written to %q", d.profileMutex) - } - - delay := false - if d.profileCPU != "" { - delay = true - f, err := os.Create(d.profileCPU) - if err != nil { - return Errorf(err.Error()) - } - defer func() { - f.Close() - if err := c.Sandbox.StopCPUProfile(); err != nil { - Fatalf(err.Error()) - } - log.Infof("CPU profile written to %q", d.profileCPU) - }() - if err := c.Sandbox.StartCPUProfile(f); err != nil { - return Errorf(err.Error()) - } - log.Infof("CPU profile started for %v, writing to %q", d.duration, d.profileCPU) - } - if d.trace != "" { - delay = true - f, err := os.Create(d.trace) - if err != nil { - return Errorf(err.Error()) - } - defer func() { - f.Close() - if err := c.Sandbox.StopTrace(); err != nil { - Fatalf(err.Error()) - } - log.Infof("Trace written to %q", d.trace) - }() - if err := c.Sandbox.StartTrace(f); err != nil { - return Errorf(err.Error()) - } - log.Infof("Tracing started for %v, writing to %q", d.duration, d.trace) - } - if d.strace != "" || len(d.logLevel) != 0 || len(d.logPackets) != 0 { args := control.LoggingArgs{} switch strings.ToLower(d.strace) { @@ -285,8 +215,156 @@ func (d *Debug) Execute(_ context.Context, f *flag.FlagSet, args ...interface{}) log.Infof(o) } - if delay { - time.Sleep(d.duration) + // Open profiling files. + var ( + heapFile *os.File + cpuFile *os.File + traceFile *os.File + blockFile *os.File + mutexFile *os.File + ) + if d.profileHeap != "" { + f, err := os.OpenFile(d.profileHeap, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, 0644) + if err != nil { + return Errorf("error opening heap profile output: %v", err) + } + defer f.Close() + heapFile = f + } + if d.profileCPU != "" { + f, err := os.OpenFile(d.profileCPU, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, 0644) + if err != nil { + return Errorf("error opening cpu profile output: %v", err) + } + defer f.Close() + cpuFile = f + } + if d.trace != "" { + f, err := os.OpenFile(d.trace, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, 0644) + if err != nil { + return Errorf("error opening trace profile output: %v", err) + } + traceFile = f + } + if d.profileBlock != "" { + f, err := os.OpenFile(d.profileBlock, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, 0644) + if err != nil { + return Errorf("error opening blocking profile output: %v", err) + } + defer f.Close() + blockFile = f + } + if d.profileMutex != "" { + f, err := os.OpenFile(d.profileMutex, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, 0644) + if err != nil { + return Errorf("error opening mutex profile output: %v", err) + } + defer f.Close() + mutexFile = f + } + + // Collect profiles. + var ( + wg sync.WaitGroup + heapErr error + cpuErr error + traceErr error + blockErr error + mutexErr error + ) + if heapFile != nil { + wg.Add(1) + go func() { + defer wg.Done() + heapErr = c.Sandbox.HeapProfile(heapFile, d.delay) + }() + } + if cpuFile != nil { + wg.Add(1) + go func() { + defer wg.Done() + cpuErr = c.Sandbox.CPUProfile(cpuFile, d.duration) + }() + } + if traceFile != nil { + wg.Add(1) + go func() { + defer wg.Done() + traceErr = c.Sandbox.Trace(traceFile, d.duration) + }() + } + if blockFile != nil { + wg.Add(1) + go func() { + defer wg.Done() + blockErr = c.Sandbox.BlockProfile(blockFile, d.duration) + }() + } + if mutexFile != nil { + wg.Add(1) + go func() { + defer wg.Done() + mutexErr = c.Sandbox.MutexProfile(mutexFile, d.duration) + }() + } + + // Before sleeping, allow us to catch signals and try to exit + // gracefully before just exiting. If we can't wait for wg, then + // we will not be able to read the errors below safely. + readyChan := make(chan struct{}) + go func() { + defer close(readyChan) + wg.Wait() + }() + signals := make(chan os.Signal, 1) + signal.Notify(signals, syscall.SIGTERM, syscall.SIGINT) + select { + case <-readyChan: + break // Safe to proceed. + case <-signals: + log.Infof("caught signal, waiting at most one more second.") + select { + case <-signals: + log.Infof("caught second signal, exiting immediately.") + os.Exit(1) // Not finished. + case <-time.After(time.Second): + log.Infof("timeout, exiting.") + os.Exit(1) // Not finished. + case <-readyChan: + break // Safe to proceed. + } + } + + // Collect all errors. + errorCount := 0 + if heapErr != nil { + errorCount++ + log.Infof("error collecting heap profile: %v", heapErr) + os.Remove(heapFile.Name()) + } + if cpuErr != nil { + errorCount++ + log.Infof("error collecting cpu profile: %v", cpuErr) + os.Remove(cpuFile.Name()) + } + if traceErr != nil { + errorCount++ + log.Infof("error collecting trace profile: %v", traceErr) + os.Remove(traceFile.Name()) + } + if blockErr != nil { + errorCount++ + log.Infof("error collecting block profile: %v", blockErr) + os.Remove(blockFile.Name()) + } + if mutexErr != nil { + errorCount++ + log.Infof("error collecting mutex profile: %v", mutexErr) + os.Remove(mutexFile.Name()) + } + + if errorCount > 0 { + return subcommands.ExitFailure } return subcommands.ExitSuccess diff --git a/runsc/sandbox/sandbox.go b/runsc/sandbox/sandbox.go index c84ebcd8a..cfee9e63d 100644 --- a/runsc/sandbox/sandbox.go +++ b/runsc/sandbox/sandbox.go @@ -991,7 +991,7 @@ func (s *Sandbox) Stacks() (string, error) { } // HeapProfile writes a heap profile to the given file. -func (s *Sandbox) HeapProfile(f *os.File) error { +func (s *Sandbox) HeapProfile(f *os.File, delay time.Duration) error { log.Debugf("Heap profile %q", s.ID) conn, err := s.sandboxConnect() if err != nil { @@ -999,54 +999,31 @@ func (s *Sandbox) HeapProfile(f *os.File) error { } defer conn.Close() - opts := control.ProfileOpts{ - FilePayload: urpc.FilePayload{ - Files: []*os.File{f}, - }, + opts := control.HeapProfileOpts{ + FilePayload: urpc.FilePayload{Files: []*os.File{f}}, + Delay: delay, } - if err := conn.Call(boot.HeapProfile, &opts, nil); err != nil { - return fmt.Errorf("getting sandbox %q heap profile: %v", s.ID, err) - } - return nil + return conn.Call(boot.HeapProfile, &opts, nil) } -// StartCPUProfile start CPU profile writing to the given file. -func (s *Sandbox) StartCPUProfile(f *os.File) error { - log.Debugf("CPU profile start %q", s.ID) +// CPUProfile collects a CPU profile. +func (s *Sandbox) CPUProfile(f *os.File, duration time.Duration) error { + log.Debugf("CPU profile %q", s.ID) conn, err := s.sandboxConnect() if err != nil { return err } defer conn.Close() - opts := control.ProfileOpts{ - FilePayload: urpc.FilePayload{ - Files: []*os.File{f}, - }, - } - if err := conn.Call(boot.StartCPUProfile, &opts, nil); err != nil { - return fmt.Errorf("starting sandbox %q CPU profile: %v", s.ID, err) + opts := control.CPUProfileOpts{ + FilePayload: urpc.FilePayload{Files: []*os.File{f}}, + Duration: duration, } - return nil -} - -// StopCPUProfile stops a previously started CPU profile. -func (s *Sandbox) StopCPUProfile() error { - log.Debugf("CPU profile stop %q", s.ID) - conn, err := s.sandboxConnect() - if err != nil { - return err - } - defer conn.Close() - - if err := conn.Call(boot.StopCPUProfile, nil, nil); err != nil { - return fmt.Errorf("stopping sandbox %q CPU profile: %v", s.ID, err) - } - return nil + return conn.Call(boot.CPUProfile, &opts, nil) } // BlockProfile writes a block profile to the given file. -func (s *Sandbox) BlockProfile(f *os.File) error { +func (s *Sandbox) BlockProfile(f *os.File, duration time.Duration) error { log.Debugf("Block profile %q", s.ID) conn, err := s.sandboxConnect() if err != nil { @@ -1054,19 +1031,15 @@ func (s *Sandbox) BlockProfile(f *os.File) error { } defer conn.Close() - opts := control.ProfileOpts{ - FilePayload: urpc.FilePayload{ - Files: []*os.File{f}, - }, + opts := control.BlockProfileOpts{ + FilePayload: urpc.FilePayload{Files: []*os.File{f}}, + Duration: duration, } - if err := conn.Call(boot.BlockProfile, &opts, nil); err != nil { - return fmt.Errorf("getting sandbox %q block profile: %v", s.ID, err) - } - return nil + return conn.Call(boot.BlockProfile, &opts, nil) } // MutexProfile writes a mutex profile to the given file. -func (s *Sandbox) MutexProfile(f *os.File) error { +func (s *Sandbox) MutexProfile(f *os.File, duration time.Duration) error { log.Debugf("Mutex profile %q", s.ID) conn, err := s.sandboxConnect() if err != nil { @@ -1074,50 +1047,27 @@ func (s *Sandbox) MutexProfile(f *os.File) error { } defer conn.Close() - opts := control.ProfileOpts{ - FilePayload: urpc.FilePayload{ - Files: []*os.File{f}, - }, - } - if err := conn.Call(boot.MutexProfile, &opts, nil); err != nil { - return fmt.Errorf("getting sandbox %q mutex profile: %v", s.ID, err) - } - return nil -} - -// StartTrace start trace writing to the given file. -func (s *Sandbox) StartTrace(f *os.File) error { - log.Debugf("Trace start %q", s.ID) - conn, err := s.sandboxConnect() - if err != nil { - return err - } - defer conn.Close() - - opts := control.ProfileOpts{ - FilePayload: urpc.FilePayload{ - Files: []*os.File{f}, - }, - } - if err := conn.Call(boot.StartTrace, &opts, nil); err != nil { - return fmt.Errorf("starting sandbox %q trace: %v", s.ID, err) + opts := control.MutexProfileOpts{ + FilePayload: urpc.FilePayload{Files: []*os.File{f}}, + Duration: duration, } - return nil + return conn.Call(boot.MutexProfile, &opts, nil) } -// StopTrace stops a previously started trace. -func (s *Sandbox) StopTrace() error { - log.Debugf("Trace stop %q", s.ID) +// Trace collects an execution trace. +func (s *Sandbox) Trace(f *os.File, duration time.Duration) error { + log.Debugf("Trace %q", s.ID) conn, err := s.sandboxConnect() if err != nil { return err } defer conn.Close() - if err := conn.Call(boot.StopTrace, nil, nil); err != nil { - return fmt.Errorf("stopping sandbox %q trace: %v", s.ID, err) + opts := control.TraceProfileOpts{ + FilePayload: urpc.FilePayload{Files: []*os.File{f}}, + Duration: duration, } - return nil + return conn.Call(boot.Trace, &opts, nil) } // ChangeLogging changes logging options. diff --git a/test/benchmarks/README.md b/test/benchmarks/README.md index d1bbabf6f..1bfb4a129 100644 --- a/test/benchmarks/README.md +++ b/test/benchmarks/README.md @@ -81,11 +81,8 @@ benchmarks. In general, benchmarks should look like this: ```golang - -var h harness.Harness - func BenchmarkMyCoolOne(b *testing.B) { - machine, err := h.GetMachine() + machine, err := harness.GetMachine() // check err defer machine.CleanUp() @@ -95,14 +92,14 @@ func BenchmarkMyCoolOne(b *testing.B) { b.ResetTimer() - //Respect b.N. + // Respect b.N. for i := 0; i < b.N; i++ { out, err := container.Run(ctx, dockerutil.RunOpts{ Image: "benchmarks/my-cool-image", Env: []string{"MY_VAR=awesome"}, other options...see dockerutil }, "sh", "-c", "echo MY_VAR") - //check err + // check err... b.StopTimer() // Do parsing and reporting outside of the timer. @@ -114,16 +111,13 @@ func BenchmarkMyCoolOne(b *testing.B) { } func TestMain(m *testing.M) { - h.Init() + harness.Init() os.Exit(m.Run()) } ``` Some notes on the above: -* The harness is initiated in the TestMain method and made global to test - module. The harness will handle any presetup that needs to happen with - flags, remote virtual machines (eventually), and other services. * Respect `b.N` in that users of the benchmark may want to "run for an hour" or something of the sort. * Use the `b.ReportMetric()` method to report custom metrics. diff --git a/test/benchmarks/base/size_test.go b/test/benchmarks/base/size_test.go index acc49cc7c..452926e5f 100644 --- a/test/benchmarks/base/size_test.go +++ b/test/benchmarks/base/size_test.go @@ -26,12 +26,10 @@ import ( "gvisor.dev/gvisor/test/benchmarks/tools" ) -var testHarness harness.Harness - // BenchmarkSizeEmpty creates N empty containers and reads memory usage from // /proc/meminfo. func BenchmarkSizeEmpty(b *testing.B) { - machine, err := testHarness.GetMachine() + machine, err := harness.GetMachine() if err != nil { b.Fatalf("failed to get machine: %v", err) } @@ -81,7 +79,7 @@ func BenchmarkSizeEmpty(b *testing.B) { // BenchmarkSizeNginx starts N containers running Nginx, checks that they're // serving, and checks memory used based on /proc/meminfo. func BenchmarkSizeNginx(b *testing.B) { - machine, err := testHarness.GetMachine() + machine, err := harness.GetMachine() if err != nil { b.Fatalf("failed to get machine with: %v", err) } @@ -126,7 +124,7 @@ func BenchmarkSizeNginx(b *testing.B) { // BenchmarkSizeNode starts N containers running a Node app, checks that // they're serving, and checks memory used based on /proc/meminfo. func BenchmarkSizeNode(b *testing.B) { - machine, err := testHarness.GetMachine() + machine, err := harness.GetMachine() if err != nil { b.Fatalf("failed to get machine with: %v", err) } @@ -178,6 +176,6 @@ func BenchmarkSizeNode(b *testing.B) { // TestMain is the main method for package network. func TestMain(m *testing.M) { - testHarness.Init() + harness.Init() os.Exit(m.Run()) } diff --git a/test/benchmarks/base/startup_test.go b/test/benchmarks/base/startup_test.go index 8ef9f99c4..05a43ad17 100644 --- a/test/benchmarks/base/startup_test.go +++ b/test/benchmarks/base/startup_test.go @@ -25,11 +25,9 @@ import ( "gvisor.dev/gvisor/test/benchmarks/harness" ) -var testHarness harness.Harness - // BenchmarkStartEmpty times startup time for an empty container. func BenchmarkStartupEmpty(b *testing.B) { - machine, err := testHarness.GetMachine() + machine, err := harness.GetMachine() if err != nil { b.Fatalf("failed to get machine: %v", err) } @@ -53,7 +51,7 @@ func BenchmarkStartupEmpty(b *testing.B) { // Time is measured from start until the first request is served. func BenchmarkStartupNginx(b *testing.B) { // The machine to hold Nginx and the Node Server. - machine, err := testHarness.GetMachine() + machine, err := harness.GetMachine() if err != nil { b.Fatalf("failed to get machine with: %v", err) } @@ -76,7 +74,7 @@ func BenchmarkStartupNginx(b *testing.B) { // Time is measured from start until the first request is served. // Note that the Node app connects to a Redis instance before serving. func BenchmarkStartupNode(b *testing.B) { - machine, err := testHarness.GetMachine() + machine, err := harness.GetMachine() if err != nil { b.Fatalf("failed to get machine with: %v", err) } @@ -126,8 +124,8 @@ func runServerWorkload(ctx context.Context, b *testing.B, args base.ServerArgs) return fmt.Errorf("failed to get ip from server: %v", err) } - harness.DebugLog(b, "Waiting for container to start.") // Wait until the Client sees the server as up. + harness.DebugLog(b, "Waiting for container to start.") if err := harness.WaitUntilServing(ctx, args.Machine, servingIP, args.Port); err != nil { return fmt.Errorf("failed to wait for serving: %v", err) } @@ -141,6 +139,6 @@ func runServerWorkload(ctx context.Context, b *testing.B, args base.ServerArgs) // TestMain is the main method for package network. func TestMain(m *testing.M) { - testHarness.Init() + harness.Init() os.Exit(m.Run()) } diff --git a/test/benchmarks/base/sysbench_test.go b/test/benchmarks/base/sysbench_test.go index bbb797e14..80569687c 100644 --- a/test/benchmarks/base/sysbench_test.go +++ b/test/benchmarks/base/sysbench_test.go @@ -23,8 +23,6 @@ import ( "gvisor.dev/gvisor/test/benchmarks/tools" ) -var testHarness harness.Harness - type testCase struct { name string test tools.Sysbench @@ -32,42 +30,34 @@ type testCase struct { // BenchmarSysbench runs sysbench on the runtime. func BenchmarkSysbench(b *testing.B) { - testCases := []testCase{ testCase{ name: "CPU", test: &tools.SysbenchCPU{ - Base: tools.SysbenchBase{ + SysbenchBase: tools.SysbenchBase{ Threads: 1, - Time: 5, }, - MaxPrime: 50000, }, }, testCase{ name: "Memory", test: &tools.SysbenchMemory{ - Base: tools.SysbenchBase{ + SysbenchBase: tools.SysbenchBase{ Threads: 1, }, - BlockSize: "1M", - TotalSize: "500G", }, }, testCase{ name: "Mutex", test: &tools.SysbenchMutex{ - Base: tools.SysbenchBase{ + SysbenchBase: tools.SysbenchBase{ Threads: 8, }, - Loops: 1, - Locks: 10000000, - Num: 4, }, }, } - machine, err := testHarness.GetMachine() + machine, err := harness.GetMachine() if err != nil { b.Fatalf("failed to get machine: %v", err) } @@ -87,12 +77,15 @@ func BenchmarkSysbench(b *testing.B) { sysbench := machine.GetContainer(ctx, b) defer sysbench.CleanUp(ctx) + cmd := tc.test.MakeCmd(b) + b.ResetTimer() out, err := sysbench.Run(ctx, dockerutil.RunOpts{ Image: "benchmarks/sysbench", - }, tc.test.MakeCmd()...) + }, cmd...) if err != nil { b.Fatalf("failed to run sysbench: %v: logs:%s", err, out) } + b.StopTimer() tc.test.Report(b, out) }) } diff --git a/test/benchmarks/database/redis_test.go b/test/benchmarks/database/redis_test.go index f8075a04b..f3c4522ac 100644 --- a/test/benchmarks/database/redis_test.go +++ b/test/benchmarks/database/redis_test.go @@ -25,8 +25,6 @@ import ( "gvisor.dev/gvisor/test/benchmarks/tools" ) -var h harness.Harness - // All possible operations from redis. Note: "ping" will // run both PING_INLINE and PING_BUILD. var operations []string = []string{ @@ -52,13 +50,13 @@ var operations []string = []string{ // BenchmarkRedis runs redis-benchmark against a redis instance and reports // data in queries per second. Each is reported by named operation (e.g. LPUSH). func BenchmarkRedis(b *testing.B) { - clientMachine, err := h.GetMachine() + clientMachine, err := harness.GetMachine() if err != nil { b.Fatalf("failed to get machine: %v", err) } defer clientMachine.CleanUp() - serverMachine, err := h.GetMachine() + serverMachine, err := harness.GetMachine() if err != nil { b.Fatalf("failed to get machine: %v", err) } @@ -67,7 +65,6 @@ func BenchmarkRedis(b *testing.B) { // Redis runs on port 6379 by default. port := 6379 ctx := context.Background() - for _, operation := range operations { param := tools.Parameter{ Name: "operation", @@ -107,23 +104,19 @@ func BenchmarkRedis(b *testing.B) { b.Fatalf("failed to start redis with: %v", err) } + client := clientMachine.GetNativeContainer(ctx, b) + defer client.CleanUp(ctx) + redis := tools.Redis{ Operation: operation, } - - // Reset profiles and timer to begin the measurement. - server.RestartProfiles() b.ResetTimer() - client := clientMachine.GetNativeContainer(ctx, b) - defer client.CleanUp(ctx) out, err := client.Run(ctx, dockerutil.RunOpts{ Image: "benchmarks/redis", }, redis.MakeCmd(ip, serverPort, b.N /*requests*/)...) if err != nil { b.Fatalf("redis-benchmark failed with: %v", err) } - - // Stop time while we parse results. b.StopTimer() redis.Report(b, out) }) @@ -131,6 +124,6 @@ func BenchmarkRedis(b *testing.B) { } func TestMain(m *testing.M) { - h.Init() + harness.Init() os.Exit(m.Run()) } diff --git a/test/benchmarks/fs/bazel_test.go b/test/benchmarks/fs/bazel_test.go index 3fb4da9d1..8baeff0db 100644 --- a/test/benchmarks/fs/bazel_test.go +++ b/test/benchmarks/fs/bazel_test.go @@ -25,8 +25,6 @@ import ( "gvisor.dev/gvisor/test/benchmarks/tools" ) -var h harness.Harness - // Note: CleanCache versions of this test require running with root permissions. func BenchmarkBuildABSL(b *testing.B) { runBuildBenchmark(b, "benchmarks/absl", "/abseil-cpp", "absl/base/...") @@ -41,7 +39,7 @@ func BenchmarkBuildRunsc(b *testing.B) { func runBuildBenchmark(b *testing.B, image, workdir, target string) { b.Helper() // Get a machine from the Harness on which to run. - machine, err := h.GetMachine() + machine, err := harness.GetMachine() if err != nil { b.Fatalf("failed to get machine: %v", err) } @@ -102,21 +100,20 @@ func runBuildBenchmark(b *testing.B, image, workdir, target string) { prefix = "/tmp" } - // Restart profiles after the copy. - container.RestartProfiles() b.ResetTimer() + b.StopTimer() + // Drop Caches and bazel clean should happen inside the loop as we may use // time options with b.N. (e.g. Run for an hour.) for i := 0; i < b.N; i++ { - b.StopTimer() // Drop Caches for clear cache runs. if bm.clearCache { if err := harness.DropCaches(machine); err != nil { b.Skipf("failed to drop caches: %v. You probably need root.", err) } } - b.StartTimer() + b.StartTimer() got, err := container.Exec(ctx, dockerutil.ExecOpts{ WorkDir: prefix + workdir, }, "bazel", "build", "-c", "opt", target) @@ -138,7 +135,6 @@ func runBuildBenchmark(b *testing.B, image, workdir, target string) { b.Fatalf("build failed with: %v", err) } } - b.StartTimer() } }) } @@ -146,6 +142,7 @@ func runBuildBenchmark(b *testing.B, image, workdir, target string) { // TestMain is the main method for package fs. func TestMain(m *testing.M) { - h.Init() + harness.Init() + harness.SetFixedBenchmarks() os.Exit(m.Run()) } diff --git a/test/benchmarks/fs/fio_test.go b/test/benchmarks/fs/fio_test.go index 96340373c..83b8376a5 100644 --- a/test/benchmarks/fs/fio_test.go +++ b/test/benchmarks/fs/fio_test.go @@ -27,8 +27,6 @@ import ( "gvisor.dev/gvisor/test/benchmarks/tools" ) -var h harness.Harness - // BenchmarkFio runs fio on the runtime under test. There are 4 basic test // cases each run on a tmpfs mount and a bind mount. Fio requires root so that // caches can be dropped. @@ -36,33 +34,43 @@ func BenchmarkFio(b *testing.B) { testCases := []tools.Fio{ tools.Fio{ Test: "write", - Size: "5G", - Blocksize: "1M", - Iodepth: 4, + Size: b.N, + BlockSize: 4, + IODepth: 4, + }, + tools.Fio{ + Test: "write", + Size: b.N, + BlockSize: 1024, + IODepth: 4, + }, + tools.Fio{ + Test: "read", + Size: b.N, + BlockSize: 4, + IODepth: 4, }, tools.Fio{ Test: "read", - Size: "5G", - Blocksize: "1M", - Iodepth: 4, + Size: b.N, + BlockSize: 1024, + IODepth: 4, }, tools.Fio{ Test: "randwrite", - Size: "5G", - Blocksize: "4K", - Iodepth: 4, - Time: 30, + Size: b.N, + BlockSize: 4, + IODepth: 4, }, tools.Fio{ Test: "randread", - Size: "5G", - Blocksize: "4K", - Iodepth: 4, - Time: 30, + Size: b.N, + BlockSize: 4, + IODepth: 4, }, } - machine, err := h.GetMachine() + machine, err := harness.GetMachine() if err != nil { b.Fatalf("failed to get machine with: %v", err) } @@ -74,11 +82,15 @@ func BenchmarkFio(b *testing.B) { Name: "operation", Value: tc.Test, } + blockSize := tools.Parameter{ + Name: "blockSize", + Value: fmt.Sprintf("%dK", tc.BlockSize), + } filesystem := tools.Parameter{ Name: "filesystem", Value: string(fsType), } - name, err := tools.ParametersToName(operation, filesystem) + name, err := tools.ParametersToName(operation, blockSize, filesystem) if err != nil { b.Fatalf("Failed to parser paramters: %v", err) } @@ -116,7 +128,7 @@ func BenchmarkFio(b *testing.B) { // For reads, we need a file to read so make one inside the container. if strings.Contains(tc.Test, "read") { - fallocateCmd := fmt.Sprintf("fallocate -l %s %s", tc.Size, outfile) + fallocateCmd := fmt.Sprintf("fallocate -l %dK %s", tc.Size, outfile) if out, err := container.Exec(ctx, dockerutil.ExecOpts{}, strings.Split(fallocateCmd, " ")...); err != nil { b.Fatalf("failed to create readable file on mount: %v, %s", err, out) @@ -128,22 +140,24 @@ func BenchmarkFio(b *testing.B) { b.Skipf("failed to drop caches with %v. You probably need root.", err) } cmd := tc.MakeCmd(outfile) - container.RestartProfiles() + b.ResetTimer() + b.StopTimer() + for i := 0; i < b.N; i++ { + if err := harness.DropCaches(machine); err != nil { + b.Fatalf("failed to drop caches: %v", err) + } + // Run fio. + b.StartTimer() data, err := container.Exec(ctx, dockerutil.ExecOpts{}, cmd...) if err != nil { b.Fatalf("failed to run cmd %v: %v", cmd, err) } b.StopTimer() + b.SetBytes(1024 * 1024) // Bytes for go reporting (Size is in megabytes). tc.Report(b, data) - // If b.N is used (i.e. we run for an hour), we should drop caches - // after each run. - if err := harness.DropCaches(machine); err != nil { - b.Fatalf("failed to drop caches: %v", err) - } - b.StartTimer() } }) } @@ -185,6 +199,6 @@ func makeMount(machine harness.Machine, mountType mount.Type, target string) (mo // TestMain is the main method for package fs. func TestMain(m *testing.M) { - h.Init() + harness.Init() os.Exit(m.Run()) } diff --git a/test/benchmarks/harness/harness.go b/test/benchmarks/harness/harness.go index 4c6e724aa..a853b7ba8 100644 --- a/test/benchmarks/harness/harness.go +++ b/test/benchmarks/harness/harness.go @@ -28,12 +28,8 @@ var ( debug = flag.Bool("debug", false, "turns on debug messages for individual benchmarks") ) -// Harness is a handle for managing state in benchmark runs. -type Harness struct { -} - // Init performs any harness initilialization before runs. -func (h *Harness) Init() error { +func Init() error { flag.Usage = func() { fmt.Fprintf(os.Stderr, "Usage: %s -- --test.bench=<regex>\n", os.Args[0]) flag.PrintDefaults() @@ -47,7 +43,15 @@ func (h *Harness) Init() error { return nil } +// SetFixedBenchmarks causes all benchmarks to run once. +// +// This must be set if they cannot scale with N. Note that this uses 1ns +// instead of 1x due to https://github.com/golang/go/issues/32051. +func SetFixedBenchmarks() { + flag.Set("test.benchtime", "1ns") +} + // GetMachine returns this run's implementation of machine. -func (h *Harness) GetMachine() (Machine, error) { +func GetMachine() (Machine, error) { return &localMachine{}, nil } diff --git a/test/benchmarks/media/ffmpeg_test.go b/test/benchmarks/media/ffmpeg_test.go index a462ec2a6..1b99a319a 100644 --- a/test/benchmarks/media/ffmpeg_test.go +++ b/test/benchmarks/media/ffmpeg_test.go @@ -23,12 +23,10 @@ import ( "gvisor.dev/gvisor/test/benchmarks/harness" ) -var h harness.Harness - // BenchmarkFfmpeg runs ffmpeg in a container and records runtime. // BenchmarkFfmpeg should run as root to drop caches. func BenchmarkFfmpeg(b *testing.B) { - machine, err := h.GetMachine() + machine, err := harness.GetMachine() if err != nil { b.Fatalf("failed to get machine: %v", err) } @@ -38,24 +36,26 @@ func BenchmarkFfmpeg(b *testing.B) { cmd := strings.Split("ffmpeg -i video.mp4 -c:v libx264 -preset veryslow output.mp4", " ") b.ResetTimer() + b.StopTimer() + for i := 0; i < b.N; i++ { - b.StopTimer() container := machine.GetContainer(ctx, b) defer container.CleanUp(ctx) if err := harness.DropCaches(machine); err != nil { b.Skipf("failed to drop caches: %v. You probably need root.", err) } - b.StartTimer() + b.StartTimer() if _, err := container.Run(ctx, dockerutil.RunOpts{ Image: "benchmarks/ffmpeg", }, cmd...); err != nil { b.Fatalf("failed to run container: %v", err) } + b.StopTimer() } } func TestMain(m *testing.M) { - h.Init() + harness.Init() os.Exit(m.Run()) } diff --git a/test/benchmarks/ml/tensorflow_test.go b/test/benchmarks/ml/tensorflow_test.go index a55329d82..b0e0c4720 100644 --- a/test/benchmarks/ml/tensorflow_test.go +++ b/test/benchmarks/ml/tensorflow_test.go @@ -22,8 +22,6 @@ import ( "gvisor.dev/gvisor/test/benchmarks/harness" ) -var h harness.Harness - // BenchmarkTensorflow runs workloads from a TensorFlow tutorial. // See: https://github.com/aymericdamien/TensorFlow-Examples func BenchmarkTensorflow(b *testing.B) { @@ -38,7 +36,7 @@ func BenchmarkTensorflow(b *testing.B) { "NeuralNetwork": "3_NeuralNetworks/neural_network.py", } - machine, err := h.GetMachine() + machine, err := harness.GetMachine() if err != nil { b.Fatalf("failed to get machine: %v", err) } @@ -49,15 +47,17 @@ func BenchmarkTensorflow(b *testing.B) { ctx := context.Background() b.ResetTimer() + b.StopTimer() + for i := 0; i < b.N; i++ { - b.StopTimer() container := machine.GetContainer(ctx, b) defer container.CleanUp(ctx) if err := harness.DropCaches(machine); err != nil { b.Skipf("failed to drop caches: %v. You probably need root.", err) } - b.StartTimer() + // Run tensorflow. + b.StartTimer() if out, err := container.Run(ctx, dockerutil.RunOpts{ Image: "benchmarks/tensorflow", Env: []string{"PYTHONPATH=$PYTHONPATH:/TensorFlow-Examples/examples"}, @@ -65,13 +65,14 @@ func BenchmarkTensorflow(b *testing.B) { }, "python", workload); err != nil { b.Fatalf("failed to run container: %v logs: %s", err, out) } + b.StopTimer() } }) } - } func TestMain(m *testing.M) { - h.Init() + harness.Init() + harness.SetFixedBenchmarks() os.Exit(m.Run()) } diff --git a/test/benchmarks/network/httpd_test.go b/test/benchmarks/network/httpd_test.go index b07274662..629127250 100644 --- a/test/benchmarks/network/httpd_test.go +++ b/test/benchmarks/network/httpd_test.go @@ -23,8 +23,6 @@ import ( "gvisor.dev/gvisor/test/benchmarks/tools" ) -var h harness.Harness - // see Dockerfile '//images/benchmarks/httpd'. var httpdDocs = map[string]string{ "notfound": "notfound", @@ -38,13 +36,7 @@ var httpdDocs = map[string]string{ // BenchmarkHttpd iterates over different sized payloads and concurrency, testing // how well the runtime handles sending different payload sizes. func BenchmarkHttpd(b *testing.B) { - benchmarkHttpdDocSize(b, false /* reverse */) -} - -// BenchmarkReverseHttpd iterates over different sized payloads, testing -// how well the runtime handles receiving different payload sizes. -func BenchmarkReverseHttpd(b *testing.B) { - benchmarkHttpdDocSize(b, true /* reverse */) + benchmarkHttpdDocSize(b) } // BenchmarkContinuousHttpd runs specific benchmarks for continous jobs. @@ -52,20 +44,12 @@ func BenchmarkReverseHttpd(b *testing.B) { func BenchmarkContinuousHttpd(b *testing.B) { sizes := []string{"10Kb", "100Kb", "1Mb"} threads := []int{1, 25, 100, 1000} - benchmarkHttpdContinuous(b, threads, sizes, false /*reverse*/) -} - -// BenchmarkContinuousHttpdReverse runs specific benchmarks for continous jobs. -// The runtime under test is the client downloading from a runc server. -func BenchmarkContinuousHttpdReverse(b *testing.B) { - sizes := []string{"10Kb", "100Kb", "1Mb"} - threads := []int{1, 25, 100, 1000} - benchmarkHttpdContinuous(b, threads, sizes, true /*reverse*/) + benchmarkHttpdContinuous(b, threads, sizes) } // benchmarkHttpdDocSize iterates through all doc sizes, running subbenchmarks // for each size. -func benchmarkHttpdDocSize(b *testing.B, reverse bool) { +func benchmarkHttpdDocSize(b *testing.B) { b.Helper() for size, filename := range httpdDocs { concurrency := []int{1, 25, 50, 100, 1000} @@ -82,25 +66,20 @@ func benchmarkHttpdDocSize(b *testing.B, reverse bool) { if err != nil { b.Fatalf("Failed to parse parameters: %v", err) } - requests := b.N - if requests < c { - b.Logf("b.N is %d must be greater than threads %d. Consider running with --test.benchtime=Nx where N >= %d", b.N, c, c) - requests = c - } b.Run(name, func(b *testing.B) { hey := &tools.Hey{ - Requests: requests, + Requests: b.N, Concurrency: c, Doc: filename, } - runHttpd(b, hey, reverse) + runHttpd(b, hey) }) } } } // benchmarkHttpdContinuous iterates through given sizes and concurrencies. -func benchmarkHttpdContinuous(b *testing.B, concurrency []int, sizes []string, reverse bool) { +func benchmarkHttpdContinuous(b *testing.B, concurrency []int, sizes []string) { for _, size := range sizes { filename := httpdDocs[size] for _, c := range concurrency { @@ -118,26 +97,20 @@ func benchmarkHttpdContinuous(b *testing.B, concurrency []int, sizes []string, r if err != nil { b.Fatalf("Failed to parse parameters: %v", err) } - - requests := b.N - if requests < c { - b.Logf("b.N is %d must be greater than threads %d. Consider running with --test.benchtime=Nx where N >= %d", b.N, c, c) - requests = c - } b.Run(name, func(b *testing.B) { hey := &tools.Hey{ - Requests: requests, + Requests: b.N, Concurrency: c, Doc: filename, } - runHttpd(b, hey, reverse) + runHttpd(b, hey) }) } } } // runHttpd configures the static serving methods to run httpd. -func runHttpd(b *testing.B, hey *tools.Hey, reverse bool) { +func runHttpd(b *testing.B, hey *tools.Hey) { // httpd runs on port 80. port := 80 httpdRunOpts := dockerutil.RunOpts{ @@ -153,10 +126,10 @@ func runHttpd(b *testing.B, hey *tools.Hey, reverse bool) { }, } httpdCmd := []string{"sh", "-c", "mkdir -p /tmp/html; cp -r /local/* /tmp/html/.; apache2 -X"} - runStaticServer(b, h, httpdRunOpts, httpdCmd, port, hey, reverse) + runStaticServer(b, httpdRunOpts, httpdCmd, port, hey) } func TestMain(m *testing.M) { - h.Init() + harness.Init() os.Exit(m.Run()) } diff --git a/test/benchmarks/network/iperf_test.go b/test/benchmarks/network/iperf_test.go index 9d64db943..5e81149fe 100644 --- a/test/benchmarks/network/iperf_test.go +++ b/test/benchmarks/network/iperf_test.go @@ -24,20 +24,18 @@ import ( "gvisor.dev/gvisor/test/benchmarks/tools" ) -var h harness.Harness - func BenchmarkIperf(b *testing.B) { iperf := tools.Iperf{ - Time: b.N, // time in seconds to run client. + Num: b.N, } - clientMachine, err := h.GetMachine() + clientMachine, err := harness.GetMachine() if err != nil { b.Fatalf("failed to get machine: %v", err) } defer clientMachine.CleanUp() - serverMachine, err := h.GetMachine() + serverMachine, err := harness.GetMachine() if err != nil { b.Fatalf("failed to get machine: %v", err) } @@ -94,12 +92,9 @@ func BenchmarkIperf(b *testing.B) { if err := harness.WaitUntilServing(ctx, clientMachine, ip, servingPort); err != nil { b.Fatalf("failed to wait for server: %v", err) } + // Run the client. b.ResetTimer() - - // Restart the server profiles. If the server isn't being profiled - // this does nothing. - server.RestartProfiles() out, err := client.Run(ctx, dockerutil.RunOpts{ Image: "benchmarks/iperf", }, iperf.MakeCmd(ip, servingPort)...) @@ -113,6 +108,6 @@ func BenchmarkIperf(b *testing.B) { } func TestMain(m *testing.M) { - h.Init() + harness.Init() os.Exit(m.Run()) } diff --git a/test/benchmarks/network/network.go b/test/benchmarks/network/network.go index b18bc2b3c..d61002cea 100644 --- a/test/benchmarks/network/network.go +++ b/test/benchmarks/network/network.go @@ -25,33 +25,26 @@ import ( ) // runStaticServer runs static serving workloads (httpd, nginx). -func runStaticServer(b *testing.B, h harness.Harness, serverOpts dockerutil.RunOpts, serverCmd []string, port int, hey *tools.Hey, reverse bool) { +func runStaticServer(b *testing.B, serverOpts dockerutil.RunOpts, serverCmd []string, port int, hey *tools.Hey) { ctx := context.Background() // Get two machines: a client and server. - clientMachine, err := h.GetMachine() + clientMachine, err := harness.GetMachine() if err != nil { b.Fatalf("failed to get machine: %v", err) } defer clientMachine.CleanUp() - serverMachine, err := h.GetMachine() + serverMachine, err := harness.GetMachine() if err != nil { b.Fatalf("failed to get machine: %v", err) } defer serverMachine.CleanUp() - // Make the containers. 'reverse=true' specifies that the client should use the - // runtime under test. - var client, server *dockerutil.Container - if reverse { - client = clientMachine.GetContainer(ctx, b) - server = serverMachine.GetNativeContainer(ctx, b) - } else { - client = clientMachine.GetNativeContainer(ctx, b) - server = serverMachine.GetContainer(ctx, b) - } + // Make the containers. + client := clientMachine.GetNativeContainer(ctx, b) defer client.CleanUp(ctx) + server := serverMachine.GetContainer(ctx, b) defer server.CleanUp(ctx) // Start the server. @@ -73,16 +66,15 @@ func runStaticServer(b *testing.B, h harness.Harness, serverOpts dockerutil.RunO // Make sure the server is serving. harness.WaitUntilServing(ctx, clientMachine, ip, servingPort) + + // Run the client. b.ResetTimer() - server.RestartProfiles() out, err := client.Run(ctx, dockerutil.RunOpts{ Image: "benchmarks/hey", }, hey.MakeCmd(ip, servingPort)...) if err != nil { b.Fatalf("run failed with: %v", err) } - b.StopTimer() hey.Report(b, out) - b.StartTimer() } diff --git a/test/benchmarks/network/nginx_test.go b/test/benchmarks/network/nginx_test.go index 87449612a..74f3578fc 100644 --- a/test/benchmarks/network/nginx_test.go +++ b/test/benchmarks/network/nginx_test.go @@ -23,8 +23,6 @@ import ( "gvisor.dev/gvisor/test/benchmarks/tools" ) -var h harness.Harness - // see Dockerfile '//images/benchmarks/nginx'. var nginxDocs = map[string]string{ "notfound": "notfound", @@ -38,14 +36,8 @@ var nginxDocs = map[string]string{ // BenchmarkNginxDocSize iterates over different sized payloads, testing how // well the runtime handles sending different payload sizes. func BenchmarkNginxDocSize(b *testing.B) { - benchmarkNginxDocSize(b, false /* reverse */, true /* tmpfs */) - benchmarkNginxDocSize(b, false /* reverse */, false /* tmpfs */) -} - -// BenchmarkReverseNginxDocSize iterates over different sized payloads, testing -// how well the runtime handles receiving different payload sizes. -func BenchmarkReverseNginxDocSize(b *testing.B) { - benchmarkNginxDocSize(b, true /* reverse */, true /* tmpfs */) + benchmarkNginxDocSize(b, true /* tmpfs */) + benchmarkNginxDocSize(b, false /* tmpfs */) } // BenchmarkContinuousNginx runs specific benchmarks for continous jobs. @@ -53,20 +45,12 @@ func BenchmarkReverseNginxDocSize(b *testing.B) { func BenchmarkContinuousNginx(b *testing.B) { sizes := []string{"10Kb", "100Kb", "1Mb"} threads := []int{1, 25, 100, 1000} - benchmarkNginxContinuous(b, threads, sizes, false /*reverse*/) -} - -// BenchmarkContinuousNginxReverse runs specific benchmarks for continous jobs. -// The runtime under test is the client downloading from a runc server. -func BenchmarkContinuousNginxReverse(b *testing.B) { - sizes := []string{"10Kb", "100Kb", "1Mb"} - threads := []int{1, 25, 100, 1000} - benchmarkNginxContinuous(b, threads, sizes, true /*reverse*/) + benchmarkNginxContinuous(b, threads, sizes) } // benchmarkNginxDocSize iterates through all doc sizes, running subbenchmarks // for each size. -func benchmarkNginxDocSize(b *testing.B, reverse, tmpfs bool) { +func benchmarkNginxDocSize(b *testing.B, tmpfs bool) { for size, filename := range nginxDocs { concurrency := []int{1, 25, 50, 100, 1000} for _, c := range concurrency { @@ -91,26 +75,20 @@ func benchmarkNginxDocSize(b *testing.B, reverse, tmpfs bool) { if err != nil { b.Fatalf("Failed to parse parameters: %v", err) } - - requests := b.N - if requests < c { - b.Logf("b.N is %d must be greater than threads %d. Consider running with --test.benchtime=Nx where N >= %d", b.N, c, c) - requests = c - } b.Run(name, func(b *testing.B) { hey := &tools.Hey{ - Requests: requests, + Requests: b.N, Concurrency: c, Doc: filename, } - runNginx(b, hey, reverse, tmpfs) + runNginx(b, hey, tmpfs) }) } } } // benchmarkNginxContinuous iterates through given sizes and concurrencies on a tmpfs mount. -func benchmarkNginxContinuous(b *testing.B, concurrency []int, sizes []string, reverse bool) { +func benchmarkNginxContinuous(b *testing.B, concurrency []int, sizes []string) { for _, size := range sizes { filename := nginxDocs[size] for _, c := range concurrency { @@ -133,25 +111,20 @@ func benchmarkNginxContinuous(b *testing.B, concurrency []int, sizes []string, r if err != nil { b.Fatalf("Failed to parse parameters: %v", err) } - requests := b.N - if requests < c { - b.Logf("b.N is %d must be greater than threads %d. Consider running with --test.benchtime=Nx where N >= %d", b.N, c, c) - requests = c - } b.Run(name, func(b *testing.B) { hey := &tools.Hey{ - Requests: requests, + Requests: b.N, Concurrency: c, Doc: filename, } - runNginx(b, hey, reverse, true /*tmpfs*/) + runNginx(b, hey, true /*tmpfs*/) }) } } } // runNginx configures the static serving methods to run httpd. -func runNginx(b *testing.B, hey *tools.Hey, reverse, tmpfs bool) { +func runNginx(b *testing.B, hey *tools.Hey, tmpfs bool) { // nginx runs on port 80. port := 80 nginxRunOpts := dockerutil.RunOpts{ @@ -165,10 +138,10 @@ func runNginx(b *testing.B, hey *tools.Hey, reverse, tmpfs bool) { } // Command copies nginxDocs to tmpfs serving directory and runs nginx. - runStaticServer(b, h, nginxRunOpts, nginxCmd, port, hey, reverse) + runStaticServer(b, nginxRunOpts, nginxCmd, port, hey) } func TestMain(m *testing.M) { - h.Init() + harness.Init() os.Exit(m.Run()) } diff --git a/test/benchmarks/network/node_test.go b/test/benchmarks/network/node_test.go index 3e837a9e4..a1fc82f95 100644 --- a/test/benchmarks/network/node_test.go +++ b/test/benchmarks/network/node_test.go @@ -25,8 +25,6 @@ import ( "gvisor.dev/gvisor/test/benchmarks/tools" ) -var h harness.Harness - // BenchmarkNode runs requests using 'hey' against a Node server run on // 'runtime'. The server responds to requests by grabbing some data in a // redis instance and returns the data in its reponse. The test loops through @@ -42,14 +40,9 @@ func BenchmarkNode(b *testing.B) { if err != nil { b.Fatalf("Failed to parse parameters: %v", err) } - requests := b.N - if requests < c { - b.Logf("b.N is %d must be greater than threads %d. Consider running with --test.benchtime=Nx where N >= %d", b.N, c, c) - requests = c - } b.Run(name, func(b *testing.B) { hey := &tools.Hey{ - Requests: requests, + Requests: b.N, Concurrency: c, } runNode(b, hey) @@ -62,14 +55,14 @@ func runNode(b *testing.B, hey *tools.Hey) { b.Helper() // The machine to hold Redis and the Node Server. - serverMachine, err := h.GetMachine() + serverMachine, err := harness.GetMachine() if err != nil { b.Fatalf("failed to get machine with: %v", err) } defer serverMachine.CleanUp() // The machine to run 'hey'. - clientMachine, err := h.GetMachine() + clientMachine, err := harness.GetMachine() if err != nil { b.Fatalf("failed to get machine with: %v", err) } @@ -124,10 +117,8 @@ func runNode(b *testing.B, hey *tools.Hey) { heyCmd := hey.MakeCmd(servingIP, servingPort) - nodeApp.RestartProfiles() - b.ResetTimer() - // the client should run on Native. + b.ResetTimer() client := clientMachine.GetNativeContainer(ctx, b) out, err := client.Run(ctx, dockerutil.RunOpts{ Image: "benchmarks/hey", @@ -137,11 +128,10 @@ func runNode(b *testing.B, hey *tools.Hey) { } // Stop the timer to parse the data and report stats. - b.StopTimer() hey.Report(b, out) } func TestMain(m *testing.M) { - h.Init() + harness.Init() os.Exit(m.Run()) } diff --git a/test/benchmarks/network/ruby_test.go b/test/benchmarks/network/ruby_test.go index c89672873..b7ec16e0a 100644 --- a/test/benchmarks/network/ruby_test.go +++ b/test/benchmarks/network/ruby_test.go @@ -26,8 +26,6 @@ import ( "gvisor.dev/gvisor/test/benchmarks/tools" ) -var h harness.Harness - // BenchmarkRuby runs requests using 'hey' against a ruby application server. // On start, ruby app generates some random data and pushes it to a redis // instance. On a request, the app grabs for random entries from the redis @@ -43,14 +41,9 @@ func BenchmarkRuby(b *testing.B) { if err != nil { b.Fatalf("Failed to parse parameters: %v", err) } - requests := b.N - if requests < c { - b.Logf("b.N is %d must be greater than threads %d. Consider running with --test.benchtime=Nx where N >= %d", b.N, c, c) - requests = c - } b.Run(name, func(b *testing.B) { hey := &tools.Hey{ - Requests: requests, + Requests: b.N, Concurrency: c, } runRuby(b, hey) @@ -61,14 +54,14 @@ func BenchmarkRuby(b *testing.B) { // runRuby runs the test for a given # of requests and concurrency. func runRuby(b *testing.B, hey *tools.Hey) { // The machine to hold Redis and the Ruby Server. - serverMachine, err := h.GetMachine() + serverMachine, err := harness.GetMachine() if err != nil { b.Fatalf("failed to get machine with: %v", err) } defer serverMachine.CleanUp() // The machine to run 'hey'. - clientMachine, err := h.GetMachine() + clientMachine, err := harness.GetMachine() if err != nil { b.Fatalf("failed to get machine with: %v", err) } @@ -130,10 +123,9 @@ func runRuby(b *testing.B, hey *tools.Hey) { b.Fatalf("failed to wait until serving: %v", err) } heyCmd := hey.MakeCmd(servingIP, servingPort) - rubyApp.RestartProfiles() - b.ResetTimer() // the client should run on Native. + b.ResetTimer() client := clientMachine.GetNativeContainer(ctx, b) defer client.CleanUp(ctx) out, err := client.Run(ctx, dockerutil.RunOpts{ @@ -142,14 +134,11 @@ func runRuby(b *testing.B, hey *tools.Hey) { if err != nil { b.Fatalf("hey container failed: %v logs: %s", err, out) } - - // Stop the timer to parse the data and report stats. b.StopTimer() hey.Report(b, out) - b.StartTimer() } func TestMain(m *testing.M) { - h.Init() + harness.Init() os.Exit(m.Run()) } diff --git a/test/benchmarks/tools/fio.go b/test/benchmarks/tools/fio.go index f5f60fa84..f6324c3ab 100644 --- a/test/benchmarks/tools/fio.go +++ b/test/benchmarks/tools/fio.go @@ -25,25 +25,20 @@ import ( // Fio makes 'fio' commands and parses their output. type Fio struct { Test string // test to run: read, write, randread, randwrite. - Size string // total size to be read/written of format N[GMK] (e.g. 5G). - Blocksize string // blocksize to be read/write of format N[GMK] (e.g. 4K). - Iodepth int // iodepth for reads/writes. - Time int // time to run the test in seconds, usually for rand(read/write). + Size int // total size to be read/written in megabytes. + BlockSize int // block size to be read/written in kilobytes. + IODepth int // I/O depth for reads/writes. } // MakeCmd makes a 'fio' command. func (f *Fio) MakeCmd(filename string) []string { cmd := []string{"fio", "--output-format=json", "--ioengine=sync"} cmd = append(cmd, fmt.Sprintf("--name=%s", f.Test)) - cmd = append(cmd, fmt.Sprintf("--size=%s", f.Size)) - cmd = append(cmd, fmt.Sprintf("--blocksize=%s", f.Blocksize)) + cmd = append(cmd, fmt.Sprintf("--size=%dM", f.Size)) + cmd = append(cmd, fmt.Sprintf("--blocksize=%dK", f.BlockSize)) cmd = append(cmd, fmt.Sprintf("--filename=%s", filename)) - cmd = append(cmd, fmt.Sprintf("--iodepth=%d", f.Iodepth)) + cmd = append(cmd, fmt.Sprintf("--iodepth=%d", f.IODepth)) cmd = append(cmd, fmt.Sprintf("--rw=%s", f.Test)) - if f.Time != 0 { - cmd = append(cmd, "--time_based") - cmd = append(cmd, fmt.Sprintf("--runtime=%d", f.Time)) - } return cmd } diff --git a/test/benchmarks/tools/hey.go b/test/benchmarks/tools/hey.go index b8cb938fe..de908feeb 100644 --- a/test/benchmarks/tools/hey.go +++ b/test/benchmarks/tools/hey.go @@ -19,7 +19,6 @@ import ( "net" "regexp" "strconv" - "strings" "testing" ) @@ -32,8 +31,16 @@ type Hey struct { // MakeCmd returns a 'hey' command. func (h *Hey) MakeCmd(ip net.IP, port int) []string { - return strings.Split(fmt.Sprintf("hey -n %d -c %d http://%s:%d/%s", - h.Requests, h.Concurrency, ip, port, h.Doc), " ") + c := h.Concurrency + if c > h.Requests { + c = h.Requests + } + return []string{ + "hey", + "-n", fmt.Sprintf("%d", h.Requests), + "-c", fmt.Sprintf("%d", c), + fmt.Sprintf("http://%s:%d/%s", ip.String(), port, h.Doc), + } } // Report parses output from 'hey' and reports metrics. diff --git a/test/benchmarks/tools/iperf.go b/test/benchmarks/tools/iperf.go index 891d32704..abf296731 100644 --- a/test/benchmarks/tools/iperf.go +++ b/test/benchmarks/tools/iperf.go @@ -19,19 +19,27 @@ import ( "net" "regexp" "strconv" - "strings" "testing" ) +const length = 64 * 1024 + // Iperf is for the client side of `iperf`. type Iperf struct { - Time int + Num int } // MakeCmd returns a iperf client command. func (i *Iperf) MakeCmd(ip net.IP, port int) []string { - // iperf report in Kb realtime - return strings.Split(fmt.Sprintf("iperf -f K --realtime --time %d --client %s --port %d", i.Time, ip, port), " ") + return []string{ + "iperf", + "--format", "K", // Output in KBytes. + "--realtime", // Measured in realtime. + "--num", fmt.Sprintf("%d", i.Num), + "--length", fmt.Sprintf("%d", length), + "--client", ip.String(), + "--port", fmt.Sprintf("%d", port), + } } // Report parses output from iperf client and reports metrics. @@ -42,6 +50,7 @@ func (i *Iperf) Report(b *testing.B, output string) { if err != nil { b.Fatalf("failed to parse bandwitdth from %s: %v", output, err) } + b.SetBytes(length) // Measure Bytes/sec for b.N, although below is iperf output. ReportCustomMetric(b, bW*1024, "bandwidth" /*metric name*/, "bytes_per_second" /*unit*/) } diff --git a/test/benchmarks/tools/redis.go b/test/benchmarks/tools/redis.go index a42e3456e..12fdbc7cc 100644 --- a/test/benchmarks/tools/redis.go +++ b/test/benchmarks/tools/redis.go @@ -19,7 +19,6 @@ import ( "net" "regexp" "strconv" - "strings" "testing" ) @@ -33,13 +32,25 @@ func (r *Redis) MakeCmd(ip net.IP, port, requests int) []string { // There is no -t PING_BULK for redis-benchmark, so adjust the command in that case. // Note that "ping" will run both PING_INLINE and PING_BULK. if r.Operation == "PING_BULK" { - return strings.Split( - fmt.Sprintf("redis-benchmark --csv -t ping -h %s -p %d -n %d", ip, port, requests), " ") + return []string{ + "redis-benchmark", + "--csv", + "-t", "ping", + "-h", ip.String(), + "-p", fmt.Sprintf("%d", port), + "-n", fmt.Sprintf("%d", requests), + } } // runs redis-benchmark -t operation for 100K requests against server. - return strings.Split( - fmt.Sprintf("redis-benchmark --csv -t %s -h %s -p %d -n %d", r.Operation, ip, port, requests), " ") + return []string{ + "redis-benchmark", + "--csv", + "-t", r.Operation, + "-h", ip.String(), + "-p", fmt.Sprintf("%d", port), + "-n", fmt.Sprintf("%d", requests), + } } // Report parses output from redis-benchmark client and reports metrics. diff --git a/test/benchmarks/tools/sysbench.go b/test/benchmarks/tools/sysbench.go index 7ccacd8ff..350f8ec98 100644 --- a/test/benchmarks/tools/sysbench.go +++ b/test/benchmarks/tools/sysbench.go @@ -18,58 +18,48 @@ import ( "fmt" "regexp" "strconv" - "strings" "testing" ) -var warmup = "sysbench --threads=8 --memory-total-size=5G memory run > /dev/null &&" - // Sysbench represents a 'sysbench' command. type Sysbench interface { - MakeCmd() []string // Makes a sysbench command. - flags() []string - Report(*testing.B, string) // Reports results contained in string. + // MakeCmd constructs the relevant command line. + MakeCmd(*testing.B) []string + + // Report reports relevant custom metrics. + Report(*testing.B, string) } // SysbenchBase is the top level struct for sysbench and holds top-level arguments // for sysbench. See: 'sysbench --help' type SysbenchBase struct { - Threads int // number of Threads for the test. - Time int // time limit for test in seconds. + // Threads is the number of threads for the test. + Threads int } // baseFlags returns top level flags. -func (s *SysbenchBase) baseFlags() []string { +func (s *SysbenchBase) baseFlags(b *testing.B, useEvents bool) []string { var ret []string if s.Threads > 0 { ret = append(ret, fmt.Sprintf("--threads=%d", s.Threads)) } - if s.Time > 0 { - ret = append(ret, fmt.Sprintf("--time=%d", s.Time)) + ret = append(ret, "--time=0") // Ensure other mechanism is used. + if useEvents { + ret = append(ret, fmt.Sprintf("--events=%d", b.N)) } return ret } // SysbenchCPU is for 'sysbench [flags] cpu run' and holds CPU specific arguments. type SysbenchCPU struct { - Base SysbenchBase - MaxPrime int // upper limit for primes generator [10000]. + SysbenchBase } // MakeCmd makes commands for SysbenchCPU. -func (s *SysbenchCPU) MakeCmd() []string { - cmd := []string{warmup, "sysbench"} - cmd = append(cmd, s.flags()...) - cmd = append(cmd, "cpu run") - return []string{"sh", "-c", strings.Join(cmd, " ")} -} - -// flags makes flags for SysbenchCPU cmds. -func (s *SysbenchCPU) flags() []string { - cmd := s.Base.baseFlags() - if s.MaxPrime > 0 { - return append(cmd, fmt.Sprintf("--cpu-max-prime=%d", s.MaxPrime)) - } +func (s *SysbenchCPU) MakeCmd(b *testing.B) []string { + cmd := []string{"sysbench"} + cmd = append(cmd, s.baseFlags(b, true /* useEvents */)...) + cmd = append(cmd, "cpu", "run") return cmd } @@ -96,9 +86,8 @@ func (s *SysbenchCPU) parseEvents(data string) (float64, error) { // SysbenchMemory is for 'sysbench [FLAGS] memory run' and holds Memory specific arguments. type SysbenchMemory struct { - Base SysbenchBase - BlockSize string // size of test memory block [1K]. - TotalSize string // size of data to transfer [100G]. + SysbenchBase + BlockSize int // size of test memory block in megabytes [1]. Scope string // memory access scope {global, local} [global]. HugeTLB bool // allocate memory from HugeTLB [off]. OperationType string // type of memory ops {read, write, none} [write]. @@ -106,21 +95,18 @@ type SysbenchMemory struct { } // MakeCmd makes commands for SysbenchMemory. -func (s *SysbenchMemory) MakeCmd() []string { - cmd := []string{warmup, "sysbench"} - cmd = append(cmd, s.flags()...) - cmd = append(cmd, "memory run") - return []string{"sh", "-c", strings.Join(cmd, " ")} +func (s *SysbenchMemory) MakeCmd(b *testing.B) []string { + cmd := []string{"sysbench"} + cmd = append(cmd, s.flags(b)...) + cmd = append(cmd, "memory", "run") + return cmd } // flags makes flags for SysbenchMemory cmds. -func (s *SysbenchMemory) flags() []string { - cmd := s.Base.baseFlags() - if s.BlockSize != "" { - cmd = append(cmd, fmt.Sprintf("--memory-block-size=%s", s.BlockSize)) - } - if s.TotalSize != "" { - cmd = append(cmd, fmt.Sprintf("--memory-total-size=%s", s.TotalSize)) +func (s *SysbenchMemory) flags(b *testing.B) []string { + cmd := s.baseFlags(b, false /* useEvents */) + if s.BlockSize != 0 { + cmd = append(cmd, fmt.Sprintf("--memory-block-size=%dM", s.BlockSize)) } if s.Scope != "" { cmd = append(cmd, fmt.Sprintf("--memory-scope=%s", s.Scope)) @@ -134,6 +120,10 @@ func (s *SysbenchMemory) flags() []string { if s.AccessMode != "" { cmd = append(cmd, fmt.Sprintf("--memory-access-mode=%s", s.AccessMode)) } + // Sysbench ignores events for memory tests, and uses the total + // size parameter to determine when the test is done. We scale + // with this instead. + cmd = append(cmd, fmt.Sprintf("--memory-total-size=%dG", b.N)) return cmd } @@ -147,7 +137,7 @@ func (s *SysbenchMemory) Report(b *testing.B, output string) { ReportCustomMetric(b, result, "memory_operations" /*metric name*/, "ops_per_second" /*unit*/) } -var memoryOperationsRE = regexp.MustCompile(`Total\soperations:\s+\d*\s*\((\d*\.\d*)\sper\ssecond\)`) +var memoryOperationsRE = regexp.MustCompile(`Total\s+operations:\s+\d+\s+\((\s*\d+\.\d+\s*)\s+per\s+second\)`) // parseOperations parses memory operations per second form sysbench memory ouput. func (s *SysbenchMemory) parseOperations(data string) (float64, error) { @@ -160,33 +150,34 @@ func (s *SysbenchMemory) parseOperations(data string) (float64, error) { // SysbenchMutex is for 'sysbench [FLAGS] mutex run' and holds Mutex specific arguments. type SysbenchMutex struct { - Base SysbenchBase + SysbenchBase Num int // total size of mutex array [4096]. - Locks int // number of mutex locks per thread [50K]. - Loops int // number of loops to do outside mutex lock [10K]. + Loops int // number of loops to do outside mutex lock [10000]. } // MakeCmd makes commands for SysbenchMutex. -func (s *SysbenchMutex) MakeCmd() []string { - cmd := []string{warmup, "sysbench"} - cmd = append(cmd, s.flags()...) - cmd = append(cmd, "mutex run") - return []string{"sh", "-c", strings.Join(cmd, " ")} +func (s *SysbenchMutex) MakeCmd(b *testing.B) []string { + cmd := []string{"sysbench"} + cmd = append(cmd, s.flags(b)...) + cmd = append(cmd, "mutex", "run") + return cmd } // flags makes flags for SysbenchMutex commands. -func (s *SysbenchMutex) flags() []string { +func (s *SysbenchMutex) flags(b *testing.B) []string { var cmd []string - cmd = append(cmd, s.Base.baseFlags()...) + cmd = append(cmd, s.baseFlags(b, false /* useEvents */)...) if s.Num > 0 { cmd = append(cmd, fmt.Sprintf("--mutex-num=%d", s.Num)) } - if s.Locks > 0 { - cmd = append(cmd, fmt.Sprintf("--mutex-locks=%d", s.Locks)) - } if s.Loops > 0 { cmd = append(cmd, fmt.Sprintf("--mutex-loops=%d", s.Loops)) } + // Sysbench does not respect --events for mutex tests. From [1]: + // "Here --time or --events are completely ignored. Sysbench always + // runs one event per thread." + // [1] https://tomfern.com/posts/sysbench-guide-1 + cmd = append(cmd, fmt.Sprintf("--mutex-locks=%d", b.N)) return cmd } diff --git a/test/e2e/regression_test.go b/test/e2e/regression_test.go index 70bbe5121..84564cdaa 100644 --- a/test/e2e/regression_test.go +++ b/test/e2e/regression_test.go @@ -35,7 +35,7 @@ func TestBindOverlay(t *testing.T) { // Run the container. got, err := d.Run(ctx, dockerutil.RunOpts{ Image: "basic/ubuntu", - }, "bash", "-c", "nc -l -U /var/run/sock & p=$! && sleep 1 && echo foobar-asdf | nc -U /var/run/sock && wait $p") + }, "bash", "-c", "nc -q -1 -l -U /var/run/sock & p=$! && sleep 1 && echo foobar-asdf | nc -q 0 -U /var/run/sock && wait $p") if err != nil { t.Fatalf("docker run failed: %v", err) } diff --git a/test/fuse/linux/BUILD b/test/fuse/linux/BUILD index d1fb178e8..2f745bd47 100644 --- a/test/fuse/linux/BUILD +++ b/test/fuse/linux/BUILD @@ -235,6 +235,7 @@ cc_binary( srcs = ["mount_test.cc"], deps = [ gtest, + "//test/util:mount_util", "//test/util:temp_path", "//test/util:test_main", "//test/util:test_util", diff --git a/test/fuse/linux/mount_test.cc b/test/fuse/linux/mount_test.cc index a5c2fbb01..8a5478116 100644 --- a/test/fuse/linux/mount_test.cc +++ b/test/fuse/linux/mount_test.cc @@ -17,6 +17,7 @@ #include <sys/mount.h> #include "gtest/gtest.h" +#include "test/util/mount_util.h" #include "test/util/temp_path.h" #include "test/util/test_util.h" @@ -25,6 +26,17 @@ namespace testing { namespace { +TEST(FuseMount, Success) { + const FileDescriptor fd = + ASSERT_NO_ERRNO_AND_VALUE(Open("/dev/fuse", O_WRONLY)); + std::string mopts = absl::StrCat("fd=", std::to_string(fd.get())); + + const auto dir = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDir()); + + const auto mount = + ASSERT_NO_ERRNO_AND_VALUE(Mount("", dir.path(), "fuse", 0, mopts, 0)); +} + TEST(FuseMount, FDNotParsable) { int devfd; EXPECT_THAT(devfd = open("/dev/fuse", O_RDWR), SyscallSucceeds()); @@ -35,6 +47,36 @@ TEST(FuseMount, FDNotParsable) { SyscallFailsWithErrno(EINVAL)); } +TEST(FuseMount, NoDevice) { + const auto dir = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDir()); + + EXPECT_THAT(mount("", dir.path().c_str(), "fuse", 0, ""), + SyscallFailsWithErrno(EINVAL)); +} + +TEST(FuseMount, ClosedFD) { + FileDescriptor f = ASSERT_NO_ERRNO_AND_VALUE(Open("/dev/fuse", O_WRONLY)); + int fd = f.release(); + close(fd); + std::string mopts = absl::StrCat("fd=", std::to_string(fd)); + + const auto dir = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDir()); + + EXPECT_THAT(mount("", dir.path().c_str(), "fuse", 0, mopts.c_str()), + SyscallFailsWithErrno(EINVAL)); +} + +TEST(FuseMount, BadFD) { + const auto dir = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDir()); + auto file = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateFile()); + const FileDescriptor fd = + ASSERT_NO_ERRNO_AND_VALUE(Open(file.path(), O_RDWR)); + std::string mopts = absl::StrCat("fd=", std::to_string(fd.get())); + + EXPECT_THAT(mount("", dir.path().c_str(), "fuse", 0, mopts.c_str()), + SyscallFailsWithErrno(EINVAL)); +} + } // namespace } // namespace testing diff --git a/test/packetimpact/runner/dut.go b/test/packetimpact/runner/dut.go index 3e26c73cb..3da265b78 100644 --- a/test/packetimpact/runner/dut.go +++ b/test/packetimpact/runner/dut.go @@ -551,7 +551,7 @@ func StartContainer(ctx context.Context, runOpts dockerutil.RunOpts, c *dockerut hostconf.AutoRemove = true hostconf.Sysctls = map[string]string{"net.ipv6.conf.all.disable_ipv6": "0"} - if err := c.CreateFrom(ctx, conf, hostconf, nil); err != nil { + if err := c.CreateFrom(ctx, runOpts.Image, conf, hostconf, nil); err != nil { return fmt.Errorf("unable to create container %s: %w", c.Name, err) } diff --git a/test/root/cgroup_test.go b/test/root/cgroup_test.go index a26b83081..a74d6b1c1 100644 --- a/test/root/cgroup_test.go +++ b/test/root/cgroup_test.go @@ -249,12 +249,11 @@ func TestCgroup(t *testing.T) { case "pids-limit": val := attr.value hostconf.Resources.PidsLimit = &val - } } // Create container. - if err := d.CreateFrom(ctx, conf, hostconf, nil); err != nil { + if err := d.CreateFrom(ctx, "basic/alpine", conf, hostconf, nil); err != nil { t.Fatalf("create failed with: %v", err) } @@ -323,7 +322,7 @@ func TestCgroupParent(t *testing.T) { }, "sleep", "10000") hostconf.Resources.CgroupParent = parent - if err := d.CreateFrom(ctx, conf, hostconf, nil); err != nil { + if err := d.CreateFrom(ctx, "basic/alpine", conf, hostconf, nil); err != nil { t.Fatalf("create failed with: %v", err) } diff --git a/test/syscalls/linux/BUILD b/test/syscalls/linux/BUILD index 89d532c70..4e0c8a574 100644 --- a/test/syscalls/linux/BUILD +++ b/test/syscalls/linux/BUILD @@ -21,6 +21,7 @@ exports_files( "socket_ip_unbound.cc", "socket_ipv4_udp_unbound_external_networking_test.cc", "socket_ipv4_udp_unbound_loopback.cc", + "socket_ipv6_udp_unbound_loopback.cc", "socket_ipv4_udp_unbound_loopback_nogotsan.cc", "tcp_socket.cc", "udp_bind.cc", diff --git a/test/syscalls/linux/mount.cc b/test/syscalls/linux/mount.cc index d65b7d031..15b645fb7 100644 --- a/test/syscalls/linux/mount.cc +++ b/test/syscalls/linux/mount.cc @@ -345,42 +345,6 @@ TEST(MountTest, RenameRemoveMountPoint) { ASSERT_THAT(rmdir(dir.path().c_str()), SyscallFailsWithErrno(EBUSY)); } -TEST(MountTest, MountFuseFilesystemNoDevice) { - SKIP_IF(!ASSERT_NO_ERRNO_AND_VALUE(HaveCapability(CAP_SYS_ADMIN))); - SKIP_IF(IsRunningOnGvisor() && !IsFUSEEnabled()); - - auto const dir = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDir()); - - // Before kernel version 4.16-rc6, FUSE mount is protected by - // capable(CAP_SYS_ADMIN). After this version, it uses - // ns_capable(CAP_SYS_ADMIN) to protect. Before the 4.16 kernel, it was not - // allowed to mount fuse file systems without the global CAP_SYS_ADMIN. - int res = mount("", dir.path().c_str(), "fuse", 0, ""); - SKIP_IF(!IsRunningOnGvisor() && res == -1 && errno == EPERM); - - EXPECT_THAT(mount("", dir.path().c_str(), "fuse", 0, ""), - SyscallFailsWithErrno(EINVAL)); -} - -TEST(MountTest, MountFuseFilesystem) { - SKIP_IF(!ASSERT_NO_ERRNO_AND_VALUE(HaveCapability(CAP_SYS_ADMIN))); - SKIP_IF(IsRunningOnGvisor() && !IsFUSEEnabled()); - - const FileDescriptor fd = - ASSERT_NO_ERRNO_AND_VALUE(Open("/dev/fuse", O_WRONLY)); - std::string mopts = "fd=" + std::to_string(fd.get()); - - auto const dir = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDir()); - - // See comments in MountFuseFilesystemNoDevice for the reason why we skip - // EPERM when running on Linux. - int res = mount("", dir.path().c_str(), "fuse", 0, ""); - SKIP_IF(!IsRunningOnGvisor() && res == -1 && errno == EPERM); - - auto const mount = - ASSERT_NO_ERRNO_AND_VALUE(Mount("", dir.path(), "fuse", 0, mopts, 0)); -} - } // namespace } // namespace testing diff --git a/test/syscalls/linux/open.cc b/test/syscalls/linux/open.cc index 77f390f3c..fcd162ca2 100644 --- a/test/syscalls/linux/open.cc +++ b/test/syscalls/linux/open.cc @@ -505,6 +505,18 @@ TEST_F(OpenTest, OpenNonDirectoryWithTrailingSlash) { EXPECT_THAT(open(bad_path.c_str(), O_RDONLY), SyscallFailsWithErrno(ENOTDIR)); } +TEST_F(OpenTest, OpenWithStrangeFlags) { + // VFS1 incorrectly allows read/write operations on such file descriptors. + SKIP_IF(IsRunningWithVFS1()); + + const TempPath file = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateFile()); + const FileDescriptor fd = + ASSERT_NO_ERRNO_AND_VALUE(Open(file.path(), O_WRONLY | O_RDWR)); + EXPECT_THAT(write(fd.get(), "x", 1), SyscallFailsWithErrno(EBADF)); + char c; + EXPECT_THAT(read(fd.get(), &c, 1), SyscallFailsWithErrno(EBADF)); +} + } // namespace } // namespace testing diff --git a/test/syscalls/linux/semaphore.cc b/test/syscalls/linux/semaphore.cc index d485ad15a..0530fce44 100644 --- a/test/syscalls/linux/semaphore.cc +++ b/test/syscalls/linux/semaphore.cc @@ -32,8 +32,6 @@ #include "test/util/test_util.h" #include "test/util/thread_util.h" -using ::testing::Contains; - namespace gvisor { namespace testing { namespace { @@ -793,7 +791,6 @@ TEST(SemaphoreTest, IpcInfo) { struct seminfo info; // Drop CAP_IPC_OWNER which allows us to bypass semaphore permissions. ASSERT_NO_ERRNO(SetCapability(CAP_IPC_OWNER, false)); - ASSERT_THAT(semctl(0, 0, IPC_INFO, &info), SyscallSucceedsWithValue(0)); for (int i = 0; i < kLoops; i++) { AutoSem sem(semget(IPC_PRIVATE, 1, 0600 | IPC_CREAT)); ASSERT_THAT(sem.get(), SyscallSucceeds()); @@ -805,13 +802,12 @@ TEST(SemaphoreTest, IpcInfo) { EXPECT_THAT(max_used_index = semctl(0, 0, IPC_INFO, &info), SyscallSucceeds()); - int index_count = 0; + std::set<int> sem_ids_before_max_index; for (int i = 0; i <= max_used_index; i++) { struct semid_ds ds = {}; int sem_id = semctl(i, 0, SEM_STAT, &ds); // Only if index i is used within the registry. - if (sem_id != -1) { - ASSERT_THAT(sem_ids, Contains(sem_id)); + if (sem_ids.find(sem_id) != sem_ids.end()) { struct semid_ds ipc_stat_ds; ASSERT_THAT(semctl(sem_id, 0, IPC_STAT, &ipc_stat_ds), SyscallSucceeds()); EXPECT_EQ(ds.sem_perm.__key, ipc_stat_ds.sem_perm.__key); @@ -833,17 +829,15 @@ TEST(SemaphoreTest, IpcInfo) { ASSERT_THAT(semctl(sem_id, 0, IPC_SET, &ipc_set_ds), SyscallSucceeds()); ASSERT_THAT(semctl(i, 0, SEM_STAT, &ds), SyscallFailsWithErrno(EACCES)); - index_count += 1; + sem_ids_before_max_index.insert(sem_id); } } - EXPECT_EQ(index_count, kLoops); - ASSERT_THAT(semctl(0, 0, IPC_INFO, &info), - SyscallSucceedsWithValue(max_used_index)); + EXPECT_EQ(sem_ids_before_max_index.size(), kLoops); for (const int sem_id : sem_ids) { ASSERT_THAT(semctl(sem_id, 0, IPC_RMID), SyscallSucceeds()); } - ASSERT_THAT(semctl(0, 0, IPC_INFO, &info), SyscallSucceedsWithValue(0)); + ASSERT_THAT(semctl(0, 0, IPC_INFO, &info), SyscallSucceeds()); EXPECT_EQ(info.semmap, kSemMap); EXPECT_EQ(info.semmni, kSemMni); EXPECT_EQ(info.semmns, kSemMns); @@ -863,7 +857,6 @@ TEST(SemaphoreTest, SemInfo) { struct seminfo info; // Drop CAP_IPC_OWNER which allows us to bypass semaphore permissions. ASSERT_NO_ERRNO(SetCapability(CAP_IPC_OWNER, false)); - ASSERT_THAT(semctl(0, 0, IPC_INFO, &info), SyscallSucceedsWithValue(0)); for (int i = 0; i < kLoops; i++) { AutoSem sem(semget(IPC_PRIVATE, kSemSetSize, 0600 | IPC_CREAT)); ASSERT_THAT(sem.get(), SyscallSucceeds()); @@ -880,17 +873,19 @@ TEST(SemaphoreTest, SemInfo) { EXPECT_EQ(info.semmsl, kSemMsl); EXPECT_EQ(info.semopm, kSemOpm); EXPECT_EQ(info.semume, kSemUme); - EXPECT_EQ(info.semusz, sem_ids.size()); + // There could be semaphores existing in the system during the test, which + // prevents the test from getting a exact number, but the test could expect at + // least the number of sempahroes it creates in the begining of the test. + EXPECT_GE(info.semusz, sem_ids.size()); EXPECT_EQ(info.semvmx, kSemVmx); - EXPECT_EQ(info.semaem, sem_ids.size() * kSemSetSize); + EXPECT_GE(info.semaem, sem_ids.size() * kSemSetSize); - int index_count = 0; + std::set<int> sem_ids_before_max_index; for (int i = 0; i <= max_used_index; i++) { struct semid_ds ds = {}; int sem_id = semctl(i, 0, SEM_STAT, &ds); // Only if index i is used within the registry. - if (sem_id != -1) { - ASSERT_THAT(sem_ids, Contains(sem_id)); + if (sem_ids.find(sem_id) != sem_ids.end()) { struct semid_ds ipc_stat_ds; ASSERT_THAT(semctl(sem_id, 0, IPC_STAT, &ipc_stat_ds), SyscallSucceeds()); EXPECT_EQ(ds.sem_perm.__key, ipc_stat_ds.sem_perm.__key); @@ -912,17 +907,15 @@ TEST(SemaphoreTest, SemInfo) { ASSERT_THAT(semctl(sem_id, 0, IPC_SET, &ipc_set_ds), SyscallSucceeds()); ASSERT_THAT(semctl(i, 0, SEM_STAT, &ds), SyscallFailsWithErrno(EACCES)); - index_count += 1; + sem_ids_before_max_index.insert(sem_id); } } - EXPECT_EQ(index_count, kLoops); - ASSERT_THAT(semctl(0, 0, SEM_INFO, &info), - SyscallSucceedsWithValue(max_used_index)); + EXPECT_EQ(sem_ids_before_max_index.size(), kLoops); for (const int sem_id : sem_ids) { ASSERT_THAT(semctl(sem_id, 0, IPC_RMID), SyscallSucceeds()); } - ASSERT_THAT(semctl(0, 0, SEM_INFO, &info), SyscallSucceedsWithValue(0)); + ASSERT_THAT(semctl(0, 0, SEM_INFO, &info), SyscallSucceeds()); EXPECT_EQ(info.semmap, kSemMap); EXPECT_EQ(info.semmni, kSemMni); EXPECT_EQ(info.semmns, kSemMns); @@ -930,9 +923,11 @@ TEST(SemaphoreTest, SemInfo) { EXPECT_EQ(info.semmsl, kSemMsl); EXPECT_EQ(info.semopm, kSemOpm); EXPECT_EQ(info.semume, kSemUme); - EXPECT_EQ(info.semusz, 0); + // Apart from semapahores that are not created by the test, we can't determine + // the exact number of semaphore sets and semaphores, as a result, semusz and + // semaem range from 0 to a random number. Since the numbers are always + // non-negative, the test will not check the reslts of semusz and semaem. EXPECT_EQ(info.semvmx, kSemVmx); - EXPECT_EQ(info.semaem, 0); } } // namespace diff --git a/test/syscalls/linux/udp_socket.cc b/test/syscalls/linux/udp_socket.cc index 21727a2e7..650f12350 100644 --- a/test/syscalls/linux/udp_socket.cc +++ b/test/syscalls/linux/udp_socket.cc @@ -835,7 +835,12 @@ TEST_P(UdpSocketTest, RecvErrorConnRefused) { // Check the contents of msg. EXPECT_EQ(memcmp(got, buf, sizeof(buf)), 0); // iovec check - EXPECT_NE(msg.msg_flags & MSG_ERRQUEUE, 0); + // TODO(b/176251997): The next check fails on the gvisor platform due to the + // kernel bug. + if (!IsRunningWithHostinet() || GvisorPlatform() == Platform::kPtrace || + GvisorPlatform() == Platform::kKVM || + GvisorPlatform() == Platform::kNative) + EXPECT_NE(msg.msg_flags & MSG_ERRQUEUE, 0); EXPECT_EQ(memcmp(&remote, bind_addr_, addrlen_), 0); // Check the contents of the control message. diff --git a/tools/bazel.mk b/tools/bazel.mk index 396785e16..a0246a560 100644 --- a/tools/bazel.mk +++ b/tools/bazel.mk @@ -58,16 +58,11 @@ DOCKER_CONFIG := /etc/docker ## Bazel will be run with standard flags. You can specify the following flags ## to control which flags are passed: ## -## STARTUP_OPTIONS - Startup options passed to Bazel. -## BAZEL_CONFIG - A bazel config file. +## STARTUP_OPTIONS - Startup options passed to Bazel. ## STARTUP_OPTIONS := -BAZEL_CONFIG := BAZEL := bazel $(STARTUP_OPTIONS) BASE_OPTIONS := --color=no --curses=no -ifneq (,$(BAZEL_CONFIG)) -BASE_OPTIONS += --config=$(BAZEL_CONFIG) -endif TEST_OPTIONS := $(BASE_OPTIONS) \ --test_output=errors \ --keep_going \ @@ -160,8 +155,8 @@ bazel-image: load-default ## Ensures that the local builder exists. @$(call header,DOCKER BUILD) @docker rm -f $(BUILDER_NAME) 2>/dev/null || true @docker run --user 0:0 --entrypoint "" --name $(BUILDER_NAME) gvisor.dev/images/default \ - sh -c "$(GROUPADD_DOCKER) $(USERADD_DOCKER) if test -e /dev/kvm; then chmod a+rw /dev/kvm; fi" - @docker commit $(BUILDER_NAME) gvisor.dev/images/builder + sh -c "$(GROUPADD_DOCKER) $(USERADD_DOCKER) if test -e /dev/kvm; then chmod a+rw /dev/kvm; fi" >&2 + @docker commit $(BUILDER_NAME) gvisor.dev/images/builder >&2 .PHONY: bazel-image ifneq (true,$(shell $(wrapper echo true))) @@ -175,7 +170,7 @@ bazel-server: bazel-image ## Ensures that the server exists. --workdir "$(CURDIR)" \ $(DOCKER_RUN_OPTIONS) \ gvisor.dev/images/builder \ - sh -c "set -x; tail -f --pid=\$$($(BAZEL) info server_pid) /dev/null" + sh -c "set -x; tail -f --pid=\$$($(BAZEL) info server_pid) /dev/null" >&2 else bazel-server: @ @@ -191,15 +186,16 @@ endif # # The last line is used to prevent terminal shenanigans. build_paths = \ + (set -euo pipefail; \ $(call wrapper,$(BAZEL) build $(BASE_OPTIONS) $(1)) 2>&1 \ | tee /proc/self/fd/2 \ - | grep -A1 -E '^Target' \ - | grep -E '^ ($(subst $(SPACE),|,$(BUILD_ROOTS)))' \ - | sed "s/ /\n/g" \ - | strings -n 10 \ + | sed -n -e '/^Target/,$$p' \ + | sed -n -e '/^ \($(subst /,\/,$(subst $(SPACE),\|,$(BUILD_ROOTS)))\)/p' \ + | sed -e 's/ /\n/g' \ | awk '{$$1=$$1};1' \ - | xargs -n 1 -I {} readlink -f "{}" \ - | xargs -n 1 -I {} bash -c 'set -xeuo pipefail; $(2)' + | strings \ + | xargs -r -n 1 -I {} readlink -f "{}" \ + | xargs -r -n 1 -I {} bash -c 'set -xeuo pipefail; $(2)') clean = $(call header,CLEAN) && $(call wrapper,$(BAZEL) clean) build = $(call header,BUILD $(1)) && $(call build_paths,$(1),echo {}) @@ -215,7 +211,7 @@ clean: ## Cleans the bazel cache. testlogs: ## Returns the most recent set of test logs. @if test -f .build_events.json; then \ cat .build_events.json | jq -r \ - 'select(.testSummary?.overallStatus? | tostring | test("(FAILED|FLAKY|TIMEOUT)")) | .testSummary.failed | .[] | .uri' | \ - awk -Ffile:// '{print $$2;}'; \ + 'select(.testSummary?.overallStatus? | tostring | test("(FAILED|FLAKY|TIMEOUT)")) | "\(.id.testSummary.label) \(.testSummary.failed[].uri)"' | \ + sed -e 's|file://||'; \ fi .PHONY: testlogs diff --git a/tools/bazeldefs/BUILD b/tools/bazeldefs/BUILD index 97c7cb45f..a4a605346 100644 --- a/tools/bazeldefs/BUILD +++ b/tools/bazeldefs/BUILD @@ -1,46 +1,7 @@ -load("//tools:defs.bzl", "bzl_library", "rbe_platform", "rbe_toolchain") +load("//tools:defs.bzl", "bzl_library") package(licenses = ["notice"]) -# We need to define a bazel platform and toolchain to specify dockerPrivileged -# and dockerRunAsRoot options, they are required to run tests on the RBE -# cluster in Kokoro. -rbe_platform( - name = "rbe_ubuntu1604", - constraint_values = [ - "@bazel_tools//platforms:x86_64", - "@bazel_tools//platforms:linux", - "@bazel_tools//tools/cpp:clang", - "@bazel_toolchains//constraints:xenial", - "@bazel_toolchains//constraints/sanitizers:support_msan", - ], - remote_execution_properties = """ - properties: { - name: "container-image" - value:"docker://gcr.io/cloud-marketplace/google/rbe-ubuntu16-04@sha256:b516a2d69537cb40a7c6a7d92d0008abb29fba8725243772bdaf2c83f1be2272" - } - properties: { - name: "dockerAddCapabilities" - value: "SYS_ADMIN" - } - properties: { - name: "dockerPrivileged" - value: "true" - } - """, -) - -rbe_toolchain( - name = "cc-toolchain-clang-x86_64-default", - exec_compatible_with = [], - tags = [ - "manual", - ], - target_compatible_with = [], - toolchain = "@bazel_toolchains//configs/ubuntu16_04_clang/11.0.0/bazel_3.1.0/cc:cc-compiler-k8", - toolchain_type = "@bazel_tools//tools/cpp:toolchain_type", -) - bzl_library( name = "platforms_bzl", srcs = ["platforms.bzl"], diff --git a/tools/bazeldefs/defs.bzl b/tools/bazeldefs/defs.bzl index 279a38fed..58ced5167 100644 --- a/tools/bazeldefs/defs.bzl +++ b/tools/bazeldefs/defs.bzl @@ -5,8 +5,6 @@ load("@bazel_skylib//:bzl_library.bzl", _bzl_library = "bzl_library") build_test = _build_test bzl_library = _bzl_library -rbe_platform = native.platform -rbe_toolchain = native.toolchain more_shards = 4 most_shards = 8 diff --git a/tools/checkescape/checkescape.go b/tools/checkescape/checkescape.go index e5a7e23c7..011b8fee8 100644 --- a/tools/checkescape/checkescape.go +++ b/tools/checkescape/checkescape.go @@ -27,7 +27,7 @@ // heap: A direct allocation is made on the heap (hard). // builtin: A call is made to a built-in allocation function (hard). // stack: A stack split as part of a function preamble (soft). -// interface: A call is made via an interface whicy *may* escape (soft). +// interface: A call is made via an interface which *may* escape (soft). // dynamic: A dynamic function is dispatched which *may* escape (soft). // // To the use the package, annotate a function-level comment with either the @@ -618,12 +618,12 @@ func findReasons(pass *analysis.Pass, fdecl *ast.FuncDecl) ([]EscapeReason, bool // run performs the analysis. func run(pass *analysis.Pass, localEscapes bool) (interface{}, error) { - calls, err := loadObjdump() - if err != nil { + calls, callsErr := loadObjdump() + if callsErr != nil { // Note that if this analysis fails, then we don't actually // fail the analyzer itself. We simply report every possible // escape. In most cases this will work just fine. - log.Printf("WARNING: unable to load objdump: %v", err) + log.Printf("WARNING: unable to load objdump: %v", callsErr) } allEscapes := make(map[string][]Escapes) mergedEscapes := make(map[string]Escapes) @@ -645,10 +645,10 @@ func run(pass *analysis.Pass, localEscapes bool) (interface{}, error) { } hasCall := func(inst poser) (string, bool) { p := linePosition(inst, nil) - if calls == nil { + if callsErr != nil { // See above: we don't have access to the binary // itself, so need to include every possible call. - return "(possible)", true + return fmt.Sprintf("(possible, unable to load objdump: %v)", callsErr), true } s, ok := calls[p.Simplified()] if !ok { diff --git a/tools/defs.bzl b/tools/defs.bzl index 54d756e55..56c481f44 100644 --- a/tools/defs.bzl +++ b/tools/defs.bzl @@ -8,7 +8,7 @@ change for Google-internal and bazel-compatible rules. load("//tools/go_stateify:defs.bzl", "go_stateify") load("//tools/go_marshal:defs.bzl", "go_marshal", "marshal_deps", "marshal_test_deps") load("//tools/nogo:defs.bzl", "nogo_test") -load("//tools/bazeldefs:defs.bzl", _arch_genrule = "arch_genrule", _build_test = "build_test", _bzl_library = "bzl_library", _coreutil = "coreutil", _default_installer = "default_installer", _default_net_util = "default_net_util", _more_shards = "more_shards", _most_shards = "most_shards", _proto_library = "proto_library", _rbe_platform = "rbe_platform", _rbe_toolchain = "rbe_toolchain", _select_arch = "select_arch", _select_system = "select_system", _short_path = "short_path") +load("//tools/bazeldefs:defs.bzl", _arch_genrule = "arch_genrule", _build_test = "build_test", _bzl_library = "bzl_library", _coreutil = "coreutil", _default_installer = "default_installer", _default_net_util = "default_net_util", _more_shards = "more_shards", _most_shards = "most_shards", _proto_library = "proto_library", _select_arch = "select_arch", _select_system = "select_system", _short_path = "short_path") load("//tools/bazeldefs:cc.bzl", _cc_binary = "cc_binary", _cc_flags_supplier = "cc_flags_supplier", _cc_grpc_library = "cc_grpc_library", _cc_library = "cc_library", _cc_proto_library = "cc_proto_library", _cc_test = "cc_test", _cc_toolchain = "cc_toolchain", _gbenchmark = "gbenchmark", _grpcpp = "grpcpp", _gtest = "gtest", _vdso_linker_option = "vdso_linker_option") load("//tools/bazeldefs:go.bzl", _gazelle = "gazelle", _go_binary = "go_binary", _go_embed_data = "go_embed_data", _go_grpc_and_proto_libraries = "go_grpc_and_proto_libraries", _go_library = "go_library", _go_path = "go_path", _go_proto_library = "go_proto_library", _go_test = "go_test", _select_goarch = "select_goarch", _select_goos = "select_goos") load("//tools/bazeldefs:pkg.bzl", _pkg_deb = "pkg_deb", _pkg_tar = "pkg_tar") @@ -24,8 +24,6 @@ default_net_util = _default_net_util select_arch = _select_arch select_system = _select_system short_path = _short_path -rbe_platform = _rbe_platform -rbe_toolchain = _rbe_toolchain coreutil = _coreutil more_shards = _more_shards most_shards = _most_shards diff --git a/tools/go_branch.sh b/tools/go_branch.sh index ca07246a6..7ef4ddf83 100755 --- a/tools/go_branch.sh +++ b/tools/go_branch.sh @@ -89,8 +89,14 @@ git merge --no-commit --strategy ours "${head}" || \ find . -type f -exec chmod 0644 {} \; find . -type d -exec chmod 0755 {} \; -# Sync the entire gopath_dir. -rsync --recursive --delete --exclude .git -L "${gopath_dir}/" . +# Sync the entire gopath_dir. Note that we exclude auto-generated source +# files that will change here. Otherwise, it adds a tremendous amount of noise +# to commits. If this file disappears in the future, then presumably we will +# still delete the underlying directory. +rsync --recursive --delete \ + --exclude .git \ + --exclude webhook/pkg/injector/certs.go \ + -L "${gopath_dir}/" . # Add additional files. for file in "${othersrc[@]}"; do diff --git a/tools/go_marshal/gomarshal/generator.go b/tools/go_marshal/gomarshal/generator.go index 6f41b1b79..28ae6c4ef 100644 --- a/tools/go_marshal/gomarshal/generator.go +++ b/tools/go_marshal/gomarshal/generator.go @@ -447,7 +447,15 @@ func (g *Generator) Run() error { for i, a := range asts { // Collect type declarations marked for code generation and generate // Marshallable interfaces. + var sortedTypes []*marshallableType for _, t := range g.collectMarshallableTypes(a, fsets[i]) { + sortedTypes = append(sortedTypes, t) + } + sort.Slice(sortedTypes, func(x, y int) bool { + // Sort by type name, which should be unique within a package. + return sortedTypes[x].spec.Name.String() < sortedTypes[y].spec.Name.String() + }) + for _, t := range sortedTypes { impl := g.generateOne(t, fsets[i]) // Collect Marshallable types referenced by the generated code. for ref := range impl.ms { diff --git a/tools/images.mk b/tools/images.mk index 46f56bb2c..2003da5bd 100644 --- a/tools/images.mk +++ b/tools/images.mk @@ -108,9 +108,9 @@ $(foreach image, $(ALL_IMAGES), $(eval $(call tag_expand_rule,$(image)))) # ensure that caching works as expected, as well as the "latest" tag that is # used by the tests. local_tag = \ - docker tag $(call remote_image,$(1)):$(call tag,$(1)) $(call local_image,$(1)):$(call tag,$(1)) + docker tag $(call remote_image,$(1)):$(call tag,$(1)) $(call local_image,$(1)):$(call tag,$(1)) >&2 latest_tag = \ - docker tag $(call local_image,$(1)):$(call tag,$(1)) $(call local_image,$(1)) + docker tag $(call local_image,$(1)):$(call tag,$(1)) $(call local_image,$(1)) >&2 tag-%: ## Tag a local image. @$(call header,TAG $*) @$(call local_tag,$*) && $(call latest_tag,$*) @@ -118,7 +118,7 @@ tag-%: ## Tag a local image. # pull forces the image to be pulled. pull = \ $(call header,PULL $(1)) && \ - docker pull $(DOCKER_PLATFORM_ARGS) $(call remote_image,$(1)):$(call tag,$(1)) && \ + docker pull $(DOCKER_PLATFORM_ARGS) $(call remote_image,$(1)):$(call tag,$(1)) >&2 && \ $(call local_tag,$(1)) && \ $(call latest_tag,$(1)) pull-%: register-cross ## Force a repull of the image. @@ -131,11 +131,11 @@ pull-%: register-cross ## Force a repull of the image. rebuild = \ $(call header,REBUILD $(1)) && \ (T=$$(mktemp -d) && cp -a $(call path,$(1))/* $$T && \ - $(foreach image,$(shell grep FROM "$(call path,$(1))/$(call dockerfile,$(1))" 2>/dev/null | cut -d' ' -f2),docker pull $(DOCKER_PLATFORM_ARGS) $(image) &&) \ + $(foreach image,$(shell grep FROM "$(call path,$(1))/$(call dockerfile,$(1))" 2>/dev/null | cut -d' ' -f2),docker pull $(DOCKER_PLATFORM_ARGS) $(image) >&2 &&) \ docker build $(DOCKER_PLATFORM_ARGS) \ -f "$$T/$(call dockerfile,$(1))" \ -t "$(call remote_image,$(1)):$(call tag,$(1))" \ - $$T && \ + $$T >&2 && \ rm -rf $$T) && \ $(call local_tag,$(1)) && \ $(call latest_tag,$(1)) @@ -152,7 +152,7 @@ load-%: register-cross ## Pull or build an image locally. # already exists) or building manually. Note that this generic rule will match # the fully-expanded remote image tag. push-%: load-% ## Push a given image. - @docker push $(call remote_image,$*):$(call tag,$*) + @docker push $(call remote_image,$*):$(call tag,$*) >&2 # register-cross registers the necessary qemu binaries for cross-compilation. # This may be used by any target that may execute containers that are not the @@ -160,7 +160,7 @@ push-%: load-% ## Push a given image. register-cross: ifneq ($(ARCH),$(shell uname -m)) ifeq (,$(wildcard /proc/sys/fs/binfmt_misc/qemu-*)) - @docker run --rm --privileged multiarch/qemu-user-static --reset --persistent yes + @docker run --rm --privileged multiarch/qemu-user-static --reset --persistent yes >&2 else @ endif diff --git a/tools/nogo/BUILD b/tools/nogo/BUILD index 12b8b597c..566e0889e 100644 --- a/tools/nogo/BUILD +++ b/tools/nogo/BUILD @@ -3,6 +3,8 @@ load("//tools/nogo:defs.bzl", "nogo_objdump_tool", "nogo_stdlib", "nogo_target") package(licenses = ["notice"]) +exports_files(["config-schema.json"]) + nogo_target( name = "target", goarch = select_goarch(), diff --git a/tools/nogo/config-schema.json b/tools/nogo/config-schema.json new file mode 100644 index 000000000..3c25fe221 --- /dev/null +++ b/tools/nogo/config-schema.json @@ -0,0 +1,97 @@ +{ + "$schema": "http://json-schema.org/draft-07/schema", + "definitions": { + "group": { + "type": "object", + "properties": { + "name": { + "description": "The name of the group.", + "type": "string" + }, + "regex": { + "description": "A regular expression for matching paths.", + "type": "string" + }, + "default": { + "description": "Whether the group is enabled by default.", + "type": "boolean" + } + }, + "required": [ + "name", + "regex", + "default" + ], + "additionalProperties": false + }, + "regexlist": { + "description": "A list of regular expressions.", + "oneOf": [ + { + "type": "array", + "items": { + "type": "string" + } + }, + { + "type": "null" + } + ] + }, + "rule": { + "type": "object", + "properties": { + "exclude": { + "description": "A regular expression for paths to exclude.", + "$ref": "#/definitions/regexlist" + }, + "suppress": { + "description": "A regular expression for messages to suppress.", + "$ref": "#/definitions/regexlist" + } + }, + "additionalProperties": false + }, + "ruleList": { + "type": "object", + "additionalProperties": { + "oneOf": [ + { + "$ref": "#/definitions/rule" + }, + { + "type": "null" + } + ] + } + } + }, + "properties": { + "groups": { + "description": "A definition of all groups.", + "type": "array", + "items": { + "$ref": "#/definitions/group" + }, + "minItems": 1 + }, + "global": { + "description": "A global set of rules.", + "type": "object", + "additionalProperties": { + "$ref": "#/definitions/rule" + } + }, + "analyzers": { + "description": "A definition of all groups.", + "type": "object", + "additionalProperties": { + "$ref": "#/definitions/ruleList" + } + } + }, + "required": [ + "groups" + ], + "additionalProperties": false +} diff --git a/tools/workspace_status.sh b/tools/workspace_status.sh index a22c8c9f2..62d78ed3d 100755 --- a/tools/workspace_status.sh +++ b/tools/workspace_status.sh @@ -15,4 +15,4 @@ # limitations under the License. # The STABLE_ prefix will trigger a re-link if it changes. -echo STABLE_VERSION $(git describe --always --tags --abbrev=12 --dirty || echo 0.0.0) +echo STABLE_VERSION "$(git describe --always --tags --abbrev=12 --dirty 2>/dev/null || echo 0.0.0)" diff --git a/tools/yamltest/BUILD b/tools/yamltest/BUILD new file mode 100644 index 000000000..475b3badd --- /dev/null +++ b/tools/yamltest/BUILD @@ -0,0 +1,13 @@ +load("//tools:defs.bzl", "go_binary") + +package(licenses = ["notice"]) + +go_binary( + name = "yamltest", + srcs = ["main.go"], + visibility = ["//visibility:public"], + deps = [ + "@com_github_xeipuuv_gojsonschema//:go_default_library", + "@in_gopkg_yaml_v2//:go_default_library", + ], +) diff --git a/tools/yamltest/defs.bzl b/tools/yamltest/defs.bzl new file mode 100644 index 000000000..fd04f947d --- /dev/null +++ b/tools/yamltest/defs.bzl @@ -0,0 +1,41 @@ +"""Tools for testing yaml files against schemas.""" + +def _yaml_test_impl(ctx): + """Implementation for yaml_test.""" + runner = ctx.actions.declare_file(ctx.label.name) + ctx.actions.write(runner, "\n".join([ + "#!/bin/bash", + "set -euo pipefail", + "%s -schema=%s -- %s" % ( + ctx.files._tool[0].short_path, + ctx.files.schema[0].short_path, + " ".join([f.short_path for f in ctx.files.srcs]), + ), + ]), is_executable = True) + return [DefaultInfo( + runfiles = ctx.runfiles(files = ctx.files._tool + ctx.files.schema + ctx.files.srcs), + executable = runner, + )] + +yaml_test = rule( + implementation = _yaml_test_impl, + doc = "Tests a yaml file against a schema.", + attrs = { + "srcs": attr.label_list( + doc = "The input yaml files.", + mandatory = True, + allow_files = True, + ), + "schema": attr.label( + doc = "The schema file in JSON schema format.", + allow_single_file = True, + mandatory = True, + ), + "_tool": attr.label( + executable = True, + cfg = "host", + default = Label("//tools/yamltest:yamltest"), + ), + }, + test = True, +) diff --git a/tools/yamltest/main.go b/tools/yamltest/main.go new file mode 100644 index 000000000..88271fb66 --- /dev/null +++ b/tools/yamltest/main.go @@ -0,0 +1,133 @@ +// Copyright 2020 The gVisor Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Binary yamltest does strict yaml parsing and validation. +package main + +import ( + "encoding/json" + "errors" + "flag" + "fmt" + "os" + + "github.com/xeipuuv/gojsonschema" + yaml "gopkg.in/yaml.v2" +) + +func fixup(v interface{}) (interface{}, error) { + switch x := v.(type) { + case map[interface{}]interface{}: + // Coerse into a string-based map, required for yaml. + strMap := make(map[string]interface{}) + for k, v := range x { + strK, ok := k.(string) + if !ok { + // This cannot be converted to JSON at all. + return nil, fmt.Errorf("invalid key %T in (%#v)", k, x) + } + fv, err := fixup(v) + if err != nil { + return nil, fmt.Errorf(".%s%w", strK, err) + } + strMap[strK] = fv + } + return strMap, nil + case []interface{}: + for i := range x { + fv, err := fixup(x[i]) + if err != nil { + return nil, fmt.Errorf("[%d]%w", i, err) + } + x[i] = fv + } + return x, nil + default: + return v, nil + } +} + +func loadFile(filename string) (gojsonschema.JSONLoader, error) { + f, err := os.Open(filename) + if err != nil { + return nil, err + } + defer f.Close() + dec := yaml.NewDecoder(f) + dec.SetStrict(true) + var object interface{} + if err := dec.Decode(&object); err != nil { + return nil, err + } + fixedObject, err := fixup(object) // For serialization. + if err != nil { + return nil, err + } + bytes, err := json.Marshal(fixedObject) + if err != nil { + return nil, err + } + return gojsonschema.NewStringLoader(string(bytes)), nil +} + +var schema = flag.String("schema", "", "path to JSON schema file.") + +func main() { + flag.Parse() + if *schema == "" || len(flag.Args()) == 0 { + flag.Usage() + os.Exit(2) + } + + // Construct our schema loader. + schemaLoader := gojsonschema.NewReferenceLoader(fmt.Sprintf("file://%s", *schema)) + + // Parse all documents. + allErrors := make(map[string][]error) + for _, filename := range flag.Args() { + // Record the filename with an empty slice for below, where + // we will emit all files (even those without any errors). + allErrors[filename] = nil + documentLoader, err := loadFile(filename) + if err != nil { + allErrors[filename] = append(allErrors[filename], err) + continue + } + result, err := gojsonschema.Validate(schemaLoader, documentLoader) + if err != nil { + allErrors[filename] = append(allErrors[filename], err) + continue + } + for _, desc := range result.Errors() { + allErrors[filename] = append(allErrors[filename], errors.New(desc.String())) + } + } + + // Print errors in yaml format. + totalErrors := 0 + for filename, errs := range allErrors { + totalErrors += len(errs) + if len(errs) == 0 { + fmt.Fprintf(os.Stderr, "%s: ✓\n", filename) + continue + } + fmt.Fprintf(os.Stderr, "%s:\n", filename) + for _, err := range errs { + fmt.Fprintf(os.Stderr, "- %s\n", err) + } + } + if totalErrors != 0 { + os.Exit(1) + } +} |