From ccf0dd9c7e48bab816be4c1b3818320caf501b6f Mon Sep 17 00:00:00 2001 From: Rail Aliiev Date: Tue, 5 Oct 2021 21:47:16 -0400 Subject: [PATCH] bazel: run acceptance tests under Bazel This patch makes the acceptance test work under Bazel. * Add `AbsCertsDir()` in order to keep track of certificate path for cases when tests change the working directory. * docker-compose tests to use interpolation and environment variables in order to override `CERTS_DIR` and `COCKROACH_BINARY`. * Add `copyRunfiles()` in order to copy Bazel-generated symlinked runfiles as regular files to make them available in docker mounted volumes. Related: #71932, #71930 Fixes #59446 Release note: None --- BUILD.bazel | 2 + build/bazelutil/check.sh | 2 +- .../teamcity/cockroach/ci/tests/acceptance.sh | 20 +++++++ pkg/BUILD.bazel | 1 - pkg/acceptance/BUILD.bazel | 25 +++++++- pkg/acceptance/cli_test.go | 9 ++- pkg/acceptance/cluster/certs.go | 14 +++++ .../compose/flyway/docker-compose.yml | 2 +- .../compose/gss/docker-compose-python.yml | 6 +- pkg/acceptance/compose/gss/docker-compose.yml | 8 +-- pkg/acceptance/compose/gss/psql/BUILD.bazel | 8 --- pkg/acceptance/compose_test.go | 42 ++++++++++++- pkg/acceptance/util_docker.go | 60 ++++++++++++++++++- pkg/cli/BUILD.bazel | 16 +++++ scripts/gceworker.sh | 3 + 15 files changed, 190 insertions(+), 28 deletions(-) create mode 100755 build/teamcity/cockroach/ci/tests/acceptance.sh delete mode 100644 pkg/acceptance/compose/gss/psql/BUILD.bazel diff --git a/BUILD.bazel b/BUILD.bazel index fd152a261aba..c5734d2c3134 100644 --- a/BUILD.bazel +++ b/BUILD.bazel @@ -129,6 +129,8 @@ load("@bazel_gazelle//:def.bzl", "gazelle") # gazelle:exclude pkg/util/buildutil/crdb_test_dyn.go # gazelle:exclude pkg/util/buildutil/crdb_test_off.go # gazelle:exclude pkg/util/buildutil/crdb_test_on.go +# gazelle:exclude pkg/acceptance/compose/gss/psql/* +# gazelle:exclude pkg/acceptance/test_main.go # # Generally useful references: # diff --git a/build/bazelutil/check.sh b/build/bazelutil/check.sh index dc98d6731b38..f7d0dd116cee 100755 --- a/build/bazelutil/check.sh +++ b/build/bazelutil/check.sh @@ -83,7 +83,7 @@ git grep '//go:generate' -- './*.go' | grep -v stringer | grep -v 'add-leaktest\ exit 1 done -git grep 'broken_in_bazel' pkg | grep BUILD.bazel: | grep -v pkg/BUILD.bazel | grep -v generate-test-suites | cut -d: -f1 | while read LINE; do +git grep 'broken_in_bazel' pkg | grep BUILD.bazel: | grep -v pkg/BUILD.bazel | grep -v pkg/cli/BUILD.bazel | grep -v generate-test-suites | cut -d: -f1 | while read LINE; do if [[ "$EXISTING_BROKEN_TESTS_IN_BAZEL" == *"$LINE"* ]]; then # Grandfathered. continue diff --git a/build/teamcity/cockroach/ci/tests/acceptance.sh b/build/teamcity/cockroach/ci/tests/acceptance.sh new file mode 100755 index 000000000000..41aada6fa34f --- /dev/null +++ b/build/teamcity/cockroach/ci/tests/acceptance.sh @@ -0,0 +1,20 @@ +#!/usr/bin/env bash + +set -xeuo pipefail + +dir="$(dirname $(dirname $(dirname $(dirname $(dirname "${0}")))))" +source "$dir/teamcity-support.sh" + +tc_prepare + +export ARTIFACTSDIR=$PWD/artifacts/acceptance +mkdir -p "$ARTIFACTSDIR" + +tc_start_block "Run acceptance tests" +bazel run \ + //pkg/acceptance:acceptance_test \ + --config=crosslinux --config=test \ + --test_arg=-l="$ARTIFACTSDIR" \ + --test_env=TZ=America/New_York \ + --test_timeout=1800 +tc_end_block "Run acceptance tests" diff --git a/pkg/BUILD.bazel b/pkg/BUILD.bazel index ce9ee51097a4..cd4f94eb1bc1 100644 --- a/pkg/BUILD.bazel +++ b/pkg/BUILD.bazel @@ -2,7 +2,6 @@ # gazelle:proto_strip_import_prefix /pkg ALL_TESTS = [ - "//pkg/acceptance:acceptance_test", "//pkg/base:base_test", "//pkg/bench/rttanalysis:rttanalysis_test", "//pkg/bench:bench_test", diff --git a/pkg/acceptance/BUILD.bazel b/pkg/acceptance/BUILD.bazel index 0e7709e51a7f..f922fdafd8b1 100644 --- a/pkg/acceptance/BUILD.bazel +++ b/pkg/acceptance/BUILD.bazel @@ -4,7 +4,7 @@ go_library( name = "acceptance", srcs = [ "flags.go", - "test_main.go", + "test_acceptance.go", # keep "util_cluster.go", "util_docker.go", ], @@ -13,9 +13,15 @@ go_library( deps = [ "//pkg/acceptance/cluster", "//pkg/base", + "//pkg/build/bazel", "//pkg/security", + "//pkg/security/securitytest", #keep + "//pkg/server", # keep "//pkg/testutils", + "//pkg/testutils/serverutils", # keep + "//pkg/testutils/testcluster", # keep "//pkg/util/log", + "//pkg/util/randutil", # keep "//pkg/util/stop", "@com_github_cockroachdb_errors//:errors", "@com_github_containerd_containerd//platforms", @@ -34,11 +40,24 @@ go_test( "debug_remote_test.go", "main_test.go", ], - data = glob(["testdata/**"]), + args = [ + "-e", + "/cockroach/cockroach", + "-b", + "../../../../../../cmd/cockroach/cockroach_/cockroach", + ], + data = glob([ + "testdata/**", + "compose/**", + ]) + [ + "//pkg/cli:interactive_tests", + "//pkg/cmd/cockroach:cockroach", + ], embed = [":acceptance"], - tags = ["broken_in_bazel"], + gotags = ["acceptance"], deps = [ "//pkg/acceptance/cluster", + "//pkg/build/bazel", "//pkg/security", "//pkg/testutils/skip", "//pkg/util/log", diff --git a/pkg/acceptance/cli_test.go b/pkg/acceptance/cli_test.go index 79baf58a11a7..788b5377d874 100644 --- a/pkg/acceptance/cli_test.go +++ b/pkg/acceptance/cli_test.go @@ -18,14 +18,12 @@ import ( "testing" "github.com/cockroachdb/cockroach/pkg/acceptance/cluster" + "github.com/cockroachdb/cockroach/pkg/build/bazel" "github.com/cockroachdb/cockroach/pkg/security" "github.com/cockroachdb/cockroach/pkg/testutils/skip" "github.com/cockroachdb/cockroach/pkg/util/log" ) -const testGlob = "../cli/interactive_tests/test*.tcl" -const containerPath = "/go/src/github.com/cockroachdb/cockroach/cli/interactive_tests" - var cmdBase = []string{ "/usr/bin/env", "COCKROACH_SKIP_UPDATE_CHECK=1", @@ -46,6 +44,7 @@ func TestDockerCLI(t *testing.T) { skip.IgnoreLintf(t, `TODO(dt): No binary in one-shot container, see #6086: %s`, err) } + testGlob := "../cli/interactive_tests/test*.tcl" paths, err := filepath.Glob(testGlob) if err != nil { t.Fatal(err) @@ -54,6 +53,10 @@ func TestDockerCLI(t *testing.T) { t.Fatalf("no testfiles found (%v)", testGlob) } + containerPath := "/go/src/github.com/cockroachdb/cockroach/cli/interactive_tests" + if bazel.BuiltWithBazel() { + containerPath = "/mnt/interactive_tests" + } for _, p := range paths { testFile := filepath.Base(p) testPath := filepath.Join(containerPath, testFile) diff --git a/pkg/acceptance/cluster/certs.go b/pkg/acceptance/cluster/certs.go index 0e149eaae852..3cfe9279ab11 100644 --- a/pkg/acceptance/cluster/certs.go +++ b/pkg/acceptance/cluster/certs.go @@ -23,13 +23,27 @@ import ( const certsDir = ".localcluster.certs" +var absCertsDir string + // keyLen is the length (in bits) of the generated CA and node certs. const keyLen = 2048 +// AbsCertsDir returns the absolute path to the certificate directory. +func AbsCertsDir() string { + return absCertsDir +} + // GenerateCerts generates CA and client certificates and private keys to be // used with a cluster. It returns a function that will clean up the generated // files. func GenerateCerts(ctx context.Context) func() { + var err error + // docker-compose tests change their working directory, + // so they need to know the absolute path to the certificate directory. + absCertsDir, err = filepath.Abs(certsDir) + if err != nil { + panic(err) + } maybePanic(os.RemoveAll(certsDir)) maybePanic(security.CreateCAPair( diff --git a/pkg/acceptance/compose/flyway/docker-compose.yml b/pkg/acceptance/compose/flyway/docker-compose.yml index 93aa788d8c5a..0b1470ea9a61 100644 --- a/pkg/acceptance/compose/flyway/docker-compose.yml +++ b/pkg/acceptance/compose/flyway/docker-compose.yml @@ -4,7 +4,7 @@ services: image: ubuntu:xenial-20170214 command: /cockroach/cockroach start-single-node --insecure --listen-addr cockroach volumes: - - ../../../../cockroach-linux-2.6.32-gnu-amd64:/cockroach/cockroach + - ${COCKROACH_BINARY:-../../../../cockroach-linux-2.6.32-gnu-amd64}:/cockroach/cockroach flyway: depends_on: - cockroach diff --git a/pkg/acceptance/compose/gss/docker-compose-python.yml b/pkg/acceptance/compose/gss/docker-compose-python.yml index 8ab1b00278ef..0b68b0c94b73 100644 --- a/pkg/acceptance/compose/gss/docker-compose-python.yml +++ b/pkg/acceptance/compose/gss/docker-compose-python.yml @@ -13,9 +13,9 @@ services: environment: - KRB5_KTNAME=/keytab/crdb.keytab volumes: - - ../../.localcluster.certs:/certs + - ${CERTS_DIR:-../../.localcluster.certs}:/certs - keytab:/keytab - - ../../../../cockroach-linux-2.6.32-gnu-amd64:/cockroach/cockroach + - ${COCKROACH_BINARY:-../../../../cockroach-linux-2.6.32-gnu-amd64}:/cockroach/cockroach python: build: ./python depends_on: @@ -27,6 +27,6 @@ services: volumes: - ./kdc/krb5.conf:/etc/krb5.conf - ./python/start.sh:/start.sh - - ../../.localcluster.certs:/certs + - ${CERTS_DIR:-../../.localcluster.certs}:/certs volumes: keytab: diff --git a/pkg/acceptance/compose/gss/docker-compose.yml b/pkg/acceptance/compose/gss/docker-compose.yml index 1e3d8dd653ef..3734ac6e9def 100644 --- a/pkg/acceptance/compose/gss/docker-compose.yml +++ b/pkg/acceptance/compose/gss/docker-compose.yml @@ -13,9 +13,9 @@ services: environment: - KRB5_KTNAME=/keytab/crdb.keytab volumes: - - ../../.localcluster.certs:/certs + - ${CERTS_DIR:-../../.localcluster.certs}:/certs - keytab:/keytab - - ../../../../cockroach-linux-2.6.32-gnu-amd64:/cockroach/cockroach + - ${COCKROACH_BINARY:-../../../../cockroach-linux-2.6.32-gnu-amd64}:/cockroach/cockroach psql: build: ./psql depends_on: @@ -27,7 +27,7 @@ services: - ./kdc/krb5.conf:/etc/krb5.conf - ./psql/gss_test.go:/test/gss_test.go - ./psql/start.sh:/start.sh - - ../../.localcluster.certs:/certs - - ../../../../cockroach-linux-2.6.32-gnu-amd64:/cockroach/cockroach + - ${CERTS_DIR:-../../.localcluster.certs}:/certs + - ${COCKROACH_BINARY:-../../../../cockroach-linux-2.6.32-gnu-amd64}:/cockroach/cockroach volumes: keytab: diff --git a/pkg/acceptance/compose/gss/psql/BUILD.bazel b/pkg/acceptance/compose/gss/psql/BUILD.bazel deleted file mode 100644 index 3e830b16167b..000000000000 --- a/pkg/acceptance/compose/gss/psql/BUILD.bazel +++ /dev/null @@ -1,8 +0,0 @@ -load("@io_bazel_rules_go//go:def.bzl", "go_library") - -go_library( - name = "psql", - srcs = ["empty.go"], - importpath = "github.com/cockroachdb/cockroach/pkg/acceptance/compose/gss/psql", - visibility = ["//visibility:public"], -) diff --git a/pkg/acceptance/compose_test.go b/pkg/acceptance/compose_test.go index 0316ae37a894..8dc612ee5a3f 100644 --- a/pkg/acceptance/compose_test.go +++ b/pkg/acceptance/compose_test.go @@ -13,25 +13,61 @@ package acceptance import ( "bytes" "io" + "io/ioutil" "os" "os/exec" "path/filepath" "testing" + + "github.com/cockroachdb/cockroach/pkg/acceptance/cluster" + "github.com/cockroachdb/cockroach/pkg/build/bazel" ) +const composeDir = "compose" + func TestComposeGSS(t *testing.T) { - testCompose(t, filepath.Join("compose", "gss", "docker-compose.yml"), "psql") + testCompose(t, filepath.Join("gss", "docker-compose.yml"), "psql") } func TestComposeGSSPython(t *testing.T) { - testCompose(t, filepath.Join("compose", "gss", "docker-compose-python.yml"), "python") + testCompose(t, filepath.Join("gss", "docker-compose-python.yml"), "python") } func TestComposeFlyway(t *testing.T) { - testCompose(t, filepath.Join("compose", "flyway", "docker-compose.yml"), "flyway") + testCompose(t, filepath.Join("flyway", "docker-compose.yml"), "flyway") } func testCompose(t *testing.T, path string, exitCodeFrom string) { + if bazel.BuiltWithBazel() { + // Copy runfiles symlink content to a temporary directory to avoid broken symlinks in docker. + tmpComposeDir, err := ioutil.TempDir("", "") + if err != nil { + t.Fatalf(err.Error()) + } + err = copyRunfiles(composeDir, tmpComposeDir) + if err != nil { + t.Fatalf(err.Error()) + } + defer func() { + _ = os.RemoveAll(tmpComposeDir) + }() + path = filepath.Join(tmpComposeDir, path) + // If running under Bazel, export 2 environment variables that will be interpolated in docker-compose.yml files. + cockroachBinary, err := filepath.Abs(*cluster.CockroachBinary) + if err != nil { + t.Fatalf(err.Error()) + } + err = os.Setenv("COCKROACH_BINARY", cockroachBinary) + if err != nil { + t.Fatalf(err.Error()) + } + err = os.Setenv("CERTS_DIR", cluster.AbsCertsDir()) + if err != nil { + t.Fatalf(err.Error()) + } + } else { + path = filepath.Join(composeDir, path) + } cmd := exec.Command( "docker-compose", "--no-ansi", diff --git a/pkg/acceptance/util_docker.go b/pkg/acceptance/util_docker.go index 270a99dabd8f..51e59c0c9a52 100644 --- a/pkg/acceptance/util_docker.go +++ b/pkg/acceptance/util_docker.go @@ -13,12 +13,15 @@ package acceptance import ( "context" "fmt" + "io/ioutil" "os" "path/filepath" + "strings" "testing" "github.com/cockroachdb/cockroach/pkg/acceptance/cluster" "github.com/cockroachdb/cockroach/pkg/base" + "github.com/cockroachdb/cockroach/pkg/build/bazel" "github.com/cockroachdb/cockroach/pkg/security" "github.com/containerd/containerd/platforms" "github.com/docker/docker/api/types" @@ -85,9 +88,39 @@ func testDocker( if err != nil { return } + testdataDir := filepath.Join(pwd, "testdata") + if bazel.BuiltWithBazel() { + testdataDir, err = ioutil.TempDir("", "") + if err != nil { + t.Fatal(err) + } + // Copy runfiles symlink content to a temporary directory to avoid broken symlinks in docker. + err = copyRunfiles("testdata", testdataDir) + if err != nil { + t.Fatal(err) + } + defer func() { + _ = os.RemoveAll(testdataDir) + }() + } hostConfig := container.HostConfig{ NetworkMode: "host", - Binds: []string{filepath.Join(pwd, "testdata") + ":/mnt/data"}, + Binds: []string{testdataDir + ":/mnt/data"}, + } + if bazel.BuiltWithBazel() { + interactivetestsDir, err := ioutil.TempDir("", "") + if err != nil { + t.Fatal(err) + } + // Copy runfiles symlink content to a temporary directory to avoid broken symlinks in docker. + err = copyRunfiles("../cli/interactive_tests", interactivetestsDir) + if err != nil { + t.Fatal(err) + } + defer func() { + _ = os.RemoveAll(interactivetestsDir) + }() + hostConfig.Binds = append(hostConfig.Binds, interactivetestsDir+":/mnt/interactive_tests") } err = l.OneShot( ctx, acceptanceImage, types.ImagePullOptions{}, containerConfig, hostConfig, @@ -99,6 +132,31 @@ func testDocker( return err } +// Bazel uses symlinks in the runfiles directory. If a directory with symlinks is mounted inside a docker container, +// the symlinks point to not existing destination. +// This function copies the content of the symlinks to another directory, +// so the files can be used inside a docker container. The caller function is responsible for cleaning up. +// This function doesn't copy the original file permissions and uses 755 for directories and files. +func copyRunfiles(source, destination string) error { + return filepath.WalkDir(source, func(path string, dirEntry os.DirEntry, walkErr error) error { + if walkErr != nil { + return walkErr + } + relPath := strings.Replace(path, source, "", 1) + if relPath == "" { + return nil + } + if dirEntry.IsDir() { + return os.Mkdir(filepath.Join(destination, relPath), 0755) + } + data, err := ioutil.ReadFile(filepath.Join(source, relPath)) + if err != nil { + return err + } + return ioutil.WriteFile(filepath.Join(destination, relPath), data, 0755) + }) +} + func testDockerSingleNode( ctx context.Context, t *testing.T, name string, containerConfig container.Config, ) error { diff --git a/pkg/cli/BUILD.bazel b/pkg/cli/BUILD.bazel index 4a31959087e6..b314adc6bc62 100644 --- a/pkg/cli/BUILD.bazel +++ b/pkg/cli/BUILD.bazel @@ -374,3 +374,19 @@ stringer( src = "flags_util.go", typ = "keyType", ) + +filegroup( + name = "interactive_tests", + srcs = glob( + ["interactive_tests/**"], + # TODO(rail): The following tests fail under bazel. + # https://github.com/cockroachdb/cockroach/issues/71932, aka + # "broken_in_bazel" + exclude = [ + "*/test_multiline_statements.tcl", + "*/test_pretty.tcl", + "*/test_server_sig.tcl", + ], + ), + visibility = ["//visibility:public"], +) diff --git a/scripts/gceworker.sh b/scripts/gceworker.sh index d0b36477be8a..94accb22301e 100755 --- a/scripts/gceworker.sh +++ b/scripts/gceworker.sh @@ -132,7 +132,10 @@ case "${cmd}" in gcloud compute config-ssh --ssh-config-file "$tmpfile" > /dev/null unison "$host" "ssh://${NAME}.${CLOUDSDK_COMPUTE_ZONE}.${CLOUDSDK_CORE_PROJECT}/$worker" \ -sshargs "-F ${tmpfile}" -auto -prefer "$host" -repeat watch \ + -ignore 'Path .localcluster.certs*' \ -ignore 'Path .git' \ + -ignore 'Path _bazel*' \ + -ignore 'Path bazel-out*' \ -ignore 'Path bin*' \ -ignore 'Path build/builder_home' \ -ignore 'Path pkg/sql/parser/gen' \