diff options
author | Fabricio Voznika <fvoznika@google.com> | 2019-09-16 08:15:40 -0700 |
---|---|---|
committer | gVisor bot <gvisor-bot@google.com> | 2019-09-16 08:17:00 -0700 |
commit | 010b0932583711ab3f6a88b1136cf8d87c2a53d2 (patch) | |
tree | 5d97437e3f7c03918071a677fe02893f0cc7d76e | |
parent | 239a07aabfad8991556b43c85c30270d09353f86 (diff) |
Bring back to life features lost in recent refactor
- Sandbox logs are generated when running tests
- Kokoro uploads the sandbox logs
- Supports multiple parallel runs
- Revive script to install locally built runsc with docker
PiperOrigin-RevId: 269337274
-rw-r--r-- | CONTRIBUTING.md | 32 | ||||
-rw-r--r-- | runsc/boot/config.go | 26 | ||||
-rw-r--r-- | runsc/container/container.go | 9 | ||||
-rw-r--r-- | runsc/dockerutil/dockerutil.go | 15 | ||||
-rw-r--r-- | runsc/main.go | 4 | ||||
-rw-r--r-- | runsc/sandbox/sandbox.go | 10 | ||||
-rw-r--r-- | runsc/specutils/specutils.go | 16 | ||||
-rwxr-xr-x | scripts/common.sh | 59 | ||||
-rwxr-xr-x | scripts/common_bazel.sh | 34 | ||||
-rwxr-xr-x | scripts/dev.sh | 72 | ||||
-rwxr-xr-x | scripts/docker_tests.sh | 6 | ||||
-rwxr-xr-x | scripts/go.sh | 2 | ||||
-rwxr-xr-x | scripts/hostnet_tests.sh | 5 | ||||
-rwxr-xr-x | scripts/kvm_tests.sh | 5 | ||||
-rwxr-xr-x | scripts/overlay_tests.sh | 5 | ||||
-rwxr-xr-x | scripts/root_tests.sh | 6 | ||||
-rw-r--r-- | test/root/main_test.go | 5 |
17 files changed, 265 insertions, 46 deletions
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 638942a42..5d46168bc 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -83,6 +83,8 @@ Rules: ### Code reviews +Before sending code reviews, run `bazel test ...` to ensure tests are passing. + Code changes are accepted via [pull request][github]. When approved, the change will be submitted by a team member and automatically @@ -100,6 +102,36 @@ form `b/1234`. These correspond to bugs in our internal bug tracker. Eventually these bugs will be moved to the GitHub Issues, but until then they can simply be ignored. +### Build and test with Docker + +`scripts/dev.sh` is a convenient script that builds and installs `runsc` as a +new Docker runtime for you. The scripts tries to extract the runtime name from +your local environment and will print it at the end. You can also customize it. +The script creates one regular runtime and another with debug flags enabled. +Here are a few examples: + +```bash +# Default case (inside branch my-branch) +$ scripts/dev.sh +... +Runtimes my-branch and my-branch-d (debug enabled) setup. +Use --runtime=my-branch with your Docker command. + docker run --rm --runtime=my-branch --rm hello-world + +If you rebuild, use scripts/dev.sh --refresh. +Logs are in: /tmp/my-branch/logs + +# --refresh just updates the runtime binary and doesn't restart docker. +$ git/my_branch> scripts/dev.sh --refresh + +# Using a custom runtime name +$ git/my_branch> scripts/dev.sh my-runtime +... +Runtimes my-runtime and my-runtime-d (debug enabled) setup. +Use --runtime=my-runtime with your Docker command. + docker run --rm --runtime=my-runtime --rm hello-world +``` + ### The small print Contributions made by corporations are covered by a different agreement than the diff --git a/runsc/boot/config.go b/runsc/boot/config.go index 05b8f8761..31103367d 100644 --- a/runsc/boot/config.go +++ b/runsc/boot/config.go @@ -211,12 +211,6 @@ type Config struct { // RestoreFile is the path to the saved container image RestoreFile string - // TestOnlyAllowRunAsCurrentUserWithoutChroot should only be used in - // tests. It allows runsc to start the sandbox process as the current - // user, and without chrooting the sandbox process. This can be - // necessary in test environments that have limited capabilities. - TestOnlyAllowRunAsCurrentUserWithoutChroot bool - // NumNetworkChannels controls the number of AF_PACKET sockets that map // to the same underlying network device. This allows netstack to better // scale for high throughput use cases. @@ -233,6 +227,19 @@ type Config struct { // ReferenceLeakMode sets reference leak check mode ReferenceLeakMode refs.LeakMode + + // TestOnlyAllowRunAsCurrentUserWithoutChroot should only be used in + // tests. It allows runsc to start the sandbox process as the current + // user, and without chrooting the sandbox process. This can be + // necessary in test environments that have limited capabilities. + TestOnlyAllowRunAsCurrentUserWithoutChroot bool + + // TestOnlyTestNameEnv should only be used in tests. It looks up for the + // test name in the container environment variables and adds it to the debug + // log file name. This is done to help identify the log with the test when + // multiple tests are run in parallel, since there is no way to pass + // parameters to the runtime from docker. + TestOnlyTestNameEnv string } // ToFlags returns a slice of flags that correspond to the given Config. @@ -261,9 +268,12 @@ func (c *Config) ToFlags() []string { "--alsologtostderr=" + strconv.FormatBool(c.AlsoLogToStderr), "--ref-leak-mode=" + refsLeakModeToString(c.ReferenceLeakMode), } + // Only include these if set since it is never to be used by users. if c.TestOnlyAllowRunAsCurrentUserWithoutChroot { - // Only include if set since it is never to be used by users. - f = append(f, "-TESTONLY-unsafe-nonroot=true") + f = append(f, "--TESTONLY-unsafe-nonroot=true") + } + if len(c.TestOnlyTestNameEnv) != 0 { + f = append(f, "--TESTONLY-test-name-env="+c.TestOnlyTestNameEnv) } return f } diff --git a/runsc/container/container.go b/runsc/container/container.go index 00f1b1de9..a721c1c31 100644 --- a/runsc/container/container.go +++ b/runsc/container/container.go @@ -946,7 +946,14 @@ func (c *Container) createGoferProcess(spec *specs.Spec, conf *boot.Config, bund } if conf.DebugLog != "" { - debugLogFile, err := specutils.DebugLogFile(conf.DebugLog, "gofer") + test := "" + if len(conf.TestOnlyTestNameEnv) != 0 { + // Fetch test name if one is provided and the test only flag was set. + if t, ok := specutils.EnvVar(spec.Process.Env, conf.TestOnlyTestNameEnv); ok { + test = t + } + } + debugLogFile, err := specutils.DebugLogFile(conf.DebugLog, "gofer", test) if err != nil { return nil, nil, fmt.Errorf("opening debug log file in %q: %v", conf.DebugLog, err) } diff --git a/runsc/dockerutil/dockerutil.go b/runsc/dockerutil/dockerutil.go index 41f5fe1e8..c073d8f75 100644 --- a/runsc/dockerutil/dockerutil.go +++ b/runsc/dockerutil/dockerutil.go @@ -240,7 +240,7 @@ func (d *Docker) Stop() error { // Run calls 'docker run' with the arguments provided. The container starts // running in the background and the call returns immediately. func (d *Docker) Run(args ...string) error { - a := []string{"run", "--runtime", d.Runtime, "--name", d.Name, "-d"} + a := d.runArgs("-d") a = append(a, args...) _, err := do(a...) if err == nil { @@ -251,7 +251,7 @@ func (d *Docker) Run(args ...string) error { // RunWithPty is like Run but with an attached pty. func (d *Docker) RunWithPty(args ...string) (*exec.Cmd, *os.File, error) { - a := []string{"run", "--runtime", d.Runtime, "--name", d.Name, "-it"} + a := d.runArgs("-it") a = append(a, args...) return doWithPty(a...) } @@ -259,8 +259,7 @@ func (d *Docker) RunWithPty(args ...string) (*exec.Cmd, *os.File, error) { // RunFg calls 'docker run' with the arguments provided in the foreground. It // blocks until the container exits and returns the output. func (d *Docker) RunFg(args ...string) (string, error) { - a := []string{"run", "--runtime", d.Runtime, "--name", d.Name} - a = append(a, args...) + a := d.runArgs(args...) out, err := do(a...) if err == nil { d.logDockerID() @@ -268,6 +267,14 @@ func (d *Docker) RunFg(args ...string) (string, error) { return string(out), err } +func (d *Docker) runArgs(args ...string) []string { + // Environment variable RUNSC_TEST_NAME is picked up by the runtime and added + // to the log name, so one can easily identify the corresponding logs for + // this test. + rv := []string{"run", "--runtime", d.Runtime, "--name", d.Name, "-e", "RUNSC_TEST_NAME=" + d.Name} + return append(rv, args...) +} + // Logs calls 'docker logs'. func (d *Docker) Logs() (string, error) { return do("logs", d.Name) diff --git a/runsc/main.go b/runsc/main.go index 0ff68160d..ff74c0a3d 100644 --- a/runsc/main.go +++ b/runsc/main.go @@ -79,6 +79,7 @@ var ( // Test flags, not to be used outside tests, ever. testOnlyAllowRunAsCurrentUserWithoutChroot = flag.Bool("TESTONLY-unsafe-nonroot", false, "TEST ONLY; do not ever use! This skips many security measures that isolate the host from the sandbox.") + testOnlyTestNameEnv = flag.String("TESTONLY-test-name-env", "", "TEST ONLY; do not ever use! Used for automated tests to improve logging.") ) func main() { @@ -211,6 +212,7 @@ func main() { ReferenceLeakMode: refsLeakMode, TestOnlyAllowRunAsCurrentUserWithoutChroot: *testOnlyAllowRunAsCurrentUserWithoutChroot, + TestOnlyTestNameEnv: *testOnlyTestNameEnv, } if len(*straceSyscalls) != 0 { conf.StraceSyscalls = strings.Split(*straceSyscalls, ",") @@ -244,7 +246,7 @@ func main() { e = newEmitter(*debugLogFormat, f) } else if *debugLog != "" { - f, err := specutils.DebugLogFile(*debugLog, subcommand) + f, err := specutils.DebugLogFile(*debugLog, subcommand, "" /* name */) if err != nil { cmd.Fatalf("error opening debug log file in %q: %v", *debugLog, err) } diff --git a/runsc/sandbox/sandbox.go b/runsc/sandbox/sandbox.go index df3c0c5ef..4c6c83fbd 100644 --- a/runsc/sandbox/sandbox.go +++ b/runsc/sandbox/sandbox.go @@ -351,7 +351,15 @@ func (s *Sandbox) createSandboxProcess(conf *boot.Config, args *Args, startSyncF nextFD++ } if conf.DebugLog != "" { - debugLogFile, err := specutils.DebugLogFile(conf.DebugLog, "boot") + test := "" + if len(conf.TestOnlyTestNameEnv) == 0 { + // Fetch test name if one is provided and the test only flag was set. + if t, ok := specutils.EnvVar(args.Spec.Process.Env, conf.TestOnlyTestNameEnv); ok { + test = t + } + } + + debugLogFile, err := specutils.DebugLogFile(conf.DebugLog, "boot", test) if err != nil { return fmt.Errorf("opening debug log file in %q: %v", conf.DebugLog, err) } diff --git a/runsc/specutils/specutils.go b/runsc/specutils/specutils.go index df435f88d..cb9e58dfb 100644 --- a/runsc/specutils/specutils.go +++ b/runsc/specutils/specutils.go @@ -399,13 +399,15 @@ func WaitForReady(pid int, timeout time.Duration, ready func() (bool, error)) er // - %TIMESTAMP%: is replaced with a timestamp using the following format: // <yyyymmdd-hhmmss.uuuuuu> // - %COMMAND%: is replaced with 'command' -func DebugLogFile(logPattern, command string) (*os.File, error) { +// - %TEST%: is replaced with 'test' (omitted by default) +func DebugLogFile(logPattern, command, test string) (*os.File, error) { if strings.HasSuffix(logPattern, "/") { // Default format: <debug-log>/runsc.log.<yyyymmdd-hhmmss.uuuuuu>.<command> logPattern += "runsc.log.%TIMESTAMP%.%COMMAND%" } logPattern = strings.Replace(logPattern, "%TIMESTAMP%", time.Now().Format("20060102-150405.000000"), -1) logPattern = strings.Replace(logPattern, "%COMMAND%", command, -1) + logPattern = strings.Replace(logPattern, "%TEST%", test, -1) dir := filepath.Dir(logPattern) if err := os.MkdirAll(dir, 0775); err != nil { @@ -542,3 +544,15 @@ func GetParentPid(pid int) (int, error) { return ppid, nil } + +// EnvVar looks for a varible value in the env slice assuming the following +// format: "NAME=VALUE". +func EnvVar(env []string, name string) (string, bool) { + prefix := name + "=" + for _, e := range env { + if strings.HasPrefix(e, prefix) { + return strings.TrimPrefix(e, prefix), true + } + } + return "", false +} diff --git a/scripts/common.sh b/scripts/common.sh index f2b9e24d8..6dabad141 100755 --- a/scripts/common.sh +++ b/scripts/common.sh @@ -14,10 +14,67 @@ # See the License for the specific language governing permissions and # limitations under the License. -set -xeo pipefail +set -xeou pipefail if [[ -f $(dirname $0)/common_google.sh ]]; then source $(dirname $0)/common_google.sh else source $(dirname $0)/common_bazel.sh fi + +# Ensure it attempts to collect logs in all cases. +trap collect_logs EXIT + +function set_runtime() { + RUNTIME=${1:-runsc} + RUNSC_BIN=/tmp/"${RUNTIME}"/runsc + RUNSC_LOGS_DIR="$(dirname ${RUNSC_BIN})"/logs + RUNSC_LOGS="${RUNSC_LOGS_DIR}"/runsc.log.%TEST%.%TIMESTAMP%.%COMMAND% +} + +function test_runsc() { + test --test_arg=--runtime=${RUNTIME} "$@" +} + +function install_runsc_for_test() { + local -r test_name=$1 + shift + if [[ -z "${test_name}" ]]; then + echo "Missing mandatory test name" + exit 1 + fi + + # Add test to the name, so it doesn't conflict with other runtimes. + set_runtime $(find_branch_name)_"${test_name}" + + # ${RUNSC_TEST_NAME} is set by tests (see dockerutil) to pass the test name + # down to the runtime. + install_runsc "${RUNTIME}" \ + --TESTONLY-test-name-env=RUNSC_TEST_NAME \ + --debug \ + --strace \ + --log-packets \ + "$@" +} + +# Installs the runsc with given runtime name. set_runtime must have been called +# to set runtime and logs location. +function install_runsc() { + local -r runtime=$1 + shift + + # Prepare the runtime binary. + local -r output=$(build //runsc) + mkdir -p "$(dirname ${RUNSC_BIN})" + cp -f "${output}" "${RUNSC_BIN}" + chmod 0755 "${RUNSC_BIN}" + + # Install the runtime. + sudo "${RUNSC_BIN}" install --experimental=true --runtime="${runtime}" -- --debug-log "${RUNSC_LOGS}" "$@" + + # Clear old logs files that may exist. + sudo rm -f "${RUNSC_LOGS_DIR}"/* + + # Restart docker to pick up the new runtime configuration. + sudo systemctl restart docker +} diff --git a/scripts/common_bazel.sh b/scripts/common_bazel.sh index dc0e2041d..dde0b51ed 100755 --- a/scripts/common_bazel.sh +++ b/scripts/common_bazel.sh @@ -53,16 +53,7 @@ function build() { } function test() { - (bazel test "${BAZEL_RBE_FLAGS[@]}" "${BAZEL_RBE_AUTH_FLAGS[@]}" "${BAZEL_FLAGS[@]}" "$@" && rc=0) || rc=$? - - # Zip out everything into a convenient form. - if [[ -v KOKORO_ARTIFACTS_DIR ]] && [[ -e bazel-testlogs ]]; then - find -L "bazel-testlogs" -name "test.xml" -o -name "test.log" -o -name "outputs.zip" | - tar --create --files-from - --transform 's/test\./sponge_log./' | - tar --extract --directory ${KOKORO_ARTIFACTS_DIR} - fi - - return $rc + bazel test "${BAZEL_RBE_FLAGS[@]}" "${BAZEL_RBE_AUTH_FLAGS[@]}" "${BAZEL_FLAGS[@]}" "$@" } function run() { @@ -76,3 +67,26 @@ function run_as_root() { shift bazel run --run_under="sudo" "${binary}" -- "$@" } + +function collect_logs() { + # Zip out everything into a convenient form. + if [[ -v KOKORO_ARTIFACTS_DIR ]] && [[ -e bazel-testlogs ]]; then + # Move test logs to Kokoro directory. tar is used to conveniently perform + # renames while moving files. + find -L "bazel-testlogs" -name "test.xml" -o -name "test.log" -o -name "outputs.zip" | + tar --create --files-from - --transform 's/test\./sponge_log./' | + tar --extract --directory ${KOKORO_ARTIFACTS_DIR} + + # Collect sentry logs, if any. + if [[ -v RUNSC_LOGS_DIR ]] && [[ -d "${RUNSC_LOGS_DIR}" ]]; then + local -r logs=$(ls "${RUNSC_LOGS_DIR}") + if [[ -z "${logs}" ]]; then + tar --create --gzip --file="${KOKORO_ARTIFACTS_DIR}/${RUNTIME}.tar.gz" -C "${RUNSC_LOGS_DIR}" . + fi + fi + fi +} + +function find_branch_name() { + git branch --show-current || git rev-parse HEAD || bazel info workspace | xargs basename +} diff --git a/scripts/dev.sh b/scripts/dev.sh new file mode 100755 index 000000000..64151c558 --- /dev/null +++ b/scripts/dev.sh @@ -0,0 +1,72 @@ +#!/bin/bash + +# Copyright 2019 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. + +source $(dirname $0)/common.sh + +# common.sh sets '-x', but it's annoying to see so much output. +set +x + +# Defaults +declare -i REFRESH=0 +declare NAME=$(find_branch_name) + +while [[ $# -gt 0 ]]; do + case "$1" in + --refresh) + REFRESH=1 + ;; + --help) + echo "Use this script to build and install runsc with Docker." + echo + echo "usage: $0 [--refresh] [runtime_name]" + exit 1 + ;; + *) + NAME=$1 + ;; + esac + shift +done + +set_runtime "${NAME}" +echo +echo "Using runtime=${RUNTIME}" +echo + +echo Building runsc... +# Build first and fail on error. $() prevents "set -e" from reporting errors. +build //runsc +declare OUTPUT="$(build //runsc)" + +if [[ ${REFRESH} -eq 0 ]]; then + install_runsc "${RUNTIME}" --net-raw + install_runsc "${RUNTIME}-d" --net-raw --debug --strace --log-packets + + echo + echo "Runtimes ${RUNTIME} and ${RUNTIME}-d (debug enabled) setup." + echo "Use --runtime="${RUNTIME}" with your Docker command." + echo " docker run --rm --runtime="${RUNTIME}" --rm hello-world" + echo + echo "If you rebuild, use $0 --refresh." + +else + cp -f ${OUTPUT} "${RUNSC_BIN}" + + echo + echo "Runtime ${RUNTIME} refreshed." +fi + +echo "Logs are in: ${RUNSC_LOGS_DIR}" diff --git a/scripts/docker_tests.sh b/scripts/docker_tests.sh index d6b18a35b..72ba05260 100755 --- a/scripts/docker_tests.sh +++ b/scripts/docker_tests.sh @@ -16,7 +16,5 @@ source $(dirname $0)/common.sh -# Install the runtime and perform basic tests. -run_as_root //runsc install --experimental=true -- --debug --strace --log-packets -sudo systemctl restart docker -test //test/image:image_test //test/e2e:integration_test +install_runsc_for_test docker +test_runsc //test/image:image_test //test/e2e:integration_test diff --git a/scripts/go.sh b/scripts/go.sh index f24fad04c..0dbfb7747 100755 --- a/scripts/go.sh +++ b/scripts/go.sh @@ -29,7 +29,7 @@ git checkout go && git clean -f go build ./... # Push, if required. -if [[ "${KOKORO_GO_PUSH}" == "true" ]]; then +if [[ -v KOKORO_GO_PUSH ]] && [[ "${KOKORO_GO_PUSH}" == "true" ]]; then if [[ -v KOKORO_GITHUB_ACCESS_TOKEN ]]; then git config --global credential.helper cache git credential approve <<EOF diff --git a/scripts/hostnet_tests.sh b/scripts/hostnet_tests.sh index 0631c5510..41298293d 100755 --- a/scripts/hostnet_tests.sh +++ b/scripts/hostnet_tests.sh @@ -17,6 +17,5 @@ source $(dirname $0)/common.sh # Install the runtime and perform basic tests. -run_as_root //runsc install --experimental=true -- --debug --strace --log-packets --network=host -sudo systemctl restart docker -test --test_arg=-checkpoint=false //test/image:image_test //test/e2e:integration_test +install_runsc_for_test hostnet --network=host +test_runsc --test_arg=-checkpoint=false //test/image:image_test //test/e2e:integration_test diff --git a/scripts/kvm_tests.sh b/scripts/kvm_tests.sh index b6d787f0f..5662401df 100755 --- a/scripts/kvm_tests.sh +++ b/scripts/kvm_tests.sh @@ -24,6 +24,5 @@ sudo chmod a+rw /dev/kvm run_as_root //pkg/sentry/platform/kvm:kvm_test # Install the KVM runtime and run all integration tests. -run_as_root //runsc install --experimental=true -- --debug --strace --log-packets --platform=kvm -sudo systemctl restart docker -test //test/image:image_test //test/e2e:integration_test +install_runsc_for_test kvm --platform=kvm +test_runsc //test/image:image_test //test/e2e:integration_test diff --git a/scripts/overlay_tests.sh b/scripts/overlay_tests.sh index 651a51f70..2a1f12c0b 100755 --- a/scripts/overlay_tests.sh +++ b/scripts/overlay_tests.sh @@ -17,6 +17,5 @@ source $(dirname $0)/common.sh # Install the runtime and perform basic tests. -run_as_root //runsc install --experimental=true -- --debug --strace --log-packets --overlay -sudo systemctl restart docker -test //test/image:image_test //test/e2e:integration_test +install_runsc_for_test overlay --overlay +test_runsc //test/image:image_test //test/e2e:integration_test diff --git a/scripts/root_tests.sh b/scripts/root_tests.sh index e42c0e3ec..4e4fcc76b 100755 --- a/scripts/root_tests.sh +++ b/scripts/root_tests.sh @@ -26,6 +26,6 @@ chmod +x ${shim_path} sudo mv ${shim_path} /usr/local/bin/gvisor-containerd-shim # Run the tests that require root. -run_as_root //runsc install --experimental=true -- --debug --strace --log-packets -sudo systemctl restart docker -run_as_root //test/root:root_test +install_runsc_for_test root +run_as_root //test/root:root_test --runtime=${RUNTIME} + diff --git a/test/root/main_test.go b/test/root/main_test.go index a3a2a91d9..d74dec85f 100644 --- a/test/root/main_test.go +++ b/test/root/main_test.go @@ -29,13 +29,15 @@ import ( // supported docker version, required capabilities, and configures the executable // path for runsc. func TestMain(m *testing.M) { - dockerutil.EnsureSupportedDockerVersion() + flag.Parse() if !specutils.HasCapabilities(capability.CAP_SYS_ADMIN, capability.CAP_DAC_OVERRIDE) { fmt.Println("Test requires sysadmin privileges to run. Try again with sudo.") os.Exit(1) } + dockerutil.EnsureSupportedDockerVersion() + // Configure exe for tests. path, err := dockerutil.RuntimePath() if err != nil { @@ -43,6 +45,5 @@ func TestMain(m *testing.M) { } specutils.ExePath = path - flag.Parse() os.Exit(m.Run()) } |