diff --git a/.bazelrc b/.bazelrc index 6023b902e..d1e18dd7a 100644 --- a/.bazelrc +++ b/.bazelrc @@ -185,7 +185,7 @@ build:rbe-toolchain-asan --linkopt -fuse-ld=lld build:rbe-toolchain-asan --action_env=ENVOY_UBSAN_VPTR=1 build:rbe-toolchain-asan --copt=-fsanitize=vptr,function build:rbe-toolchain-asan --linkopt=-fsanitize=vptr,function -build:rbe-toolchain-asan --linkopt=-L/opt/llvm/lib/clang/10.0.0/lib/linux +build:rbe-toolchain-asan --linkopt=-L/opt/llvm/lib/clang/11.0.1/lib/linux build:rbe-toolchain-asan --linkopt=-l:libclang_rt.ubsan_standalone-x86_64.a build:rbe-toolchain-asan --linkopt=-l:libclang_rt.ubsan_standalone_cxx-x86_64.a @@ -257,7 +257,7 @@ build:remote-clang-cl --config=rbe-toolchain-clang-cl # Docker sandbox # NOTE: Update this from https://github.com/envoyproxy/envoy-build-tools/blob/main/toolchains/rbe_toolchains_config.bzl#L8 -build:docker-sandbox --experimental_docker_image=envoyproxy/envoy-build-ubuntu:c8fa4235714003ba0896287ee2f91cae06e0e407 +build:docker-sandbox --experimental_docker_image=envoyproxy/envoy-build-ubuntu:e33c93e6d79804bf95ff80426d10bdcc9096c785 build:docker-sandbox --spawn_strategy=docker build:docker-sandbox --strategy=Javac=docker build:docker-sandbox --strategy=Closure=docker diff --git a/.circleci/config.yml b/.circleci/config.yml index 2ade7ad40..7a9d804a0 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -1,6 +1,6 @@ references: - envoy-build-image: &envoy-build-image # November 10th, 2020 - envoyproxy/envoy-build-ubuntu:19a268cfe3d12625380e7c61d2467c8779b58b56 + envoy-build-image: &envoy-build-image # March 8th, 2021 + envoyproxy/envoy-build-ubuntu:e33c93e6d79804bf95ff80426d10bdcc9096c785 version: 2 jobs: build: diff --git a/.vscode/tasks.json b/.vscode/tasks.json index 21b6ed8a7..488e2626c 100644 --- a/.vscode/tasks.json +++ b/.vscode/tasks.json @@ -221,31 +221,56 @@ "id": "testTarget", "description": "type", "options": [ - "//test:benchmark_http_client_test", - "//test:client_test", - "//test:client_worker_test", - "//test:factories_test", - "//test:frequency_test", - "//test:options_test", - "//test:output_formatter_test", - "//test:output_transform_main_test", - "//test:platform_util_test", - "//test:process_test", - "//test:python_test", - "//test:rate_limiter_test", - "//test:request_generator_test", - "//test:sequencer_test", - "//test:service_main_test", - "//test:service_test", - "//test:statistic_test", - "//test:stream_decoder_test", - "//test:termination_predicate_test", - "//test:utility_test", - "//test:worker_test", - "//test:sni_utility_test", - "//test/server:http_test_server_filter_integration_test", - "//test/server:http_dynamic_delay_filter_integration_test", - "//test/..." + "//test/sink:sink_test", + "//test/sink:nighthawk_sink_client_test", + "//test/server:http_time_tracking_filter_integration_test", + "//test/server:http_test_server_filter_integration_test", + "//test/server:http_filter_base_test", + "//test/server:http_dynamic_delay_filter_integration_test", + "//test/server:configuration_test", + "//test/request_source:request_source_plugin_test", + "//test/integration/unit_tests:test_nighthawk_test_server", + "//test/common:signal_handler_test", + "//test/common:nighthawk_service_client_test", + "//test/common:fake_time_source_test", + "//test/adaptive_load/fake_plugins/fake_step_controller:fake_step_controller_test", + "//test/adaptive_load/fake_plugins/fake_metrics_plugin:fake_metrics_plugin_test", + "//test/adaptive_load/fake_plugins/fake_input_variable_setter:fake_input_variable_setter_test", + "//test/adaptive_load:step_controller_test", + "//test/adaptive_load:session_spec_proto_helper_test", + "//test/adaptive_load:scoring_function_test", + "//test/adaptive_load:plugin_loader_test", + "//test/adaptive_load:metrics_plugin_test", + "//test/adaptive_load:metrics_evaluator_test", + "//test/adaptive_load:adaptive_load_controller_test", + "//test/adaptive_load:adaptive_load_client_main_test", + "//test:worker_test", + "//test:utility_test", + "//test:termination_predicate_test", + "//test:stream_decoder_test", + "//test:stopwatch_test", + "//test:statistic_test", + "//test:sni_utility_test", + "//test:service_test", + "//test:service_main_test", + "//test:sequencer_test", + "//test:request_stream_grpc_client_test", + "//test:request_generator_test", + "//test:rate_limiter_test", + "//test:python_test", + "//test/integration:integration_test", + "//test:process_test", + "//test:platform_util_test", + "//test:output_transform_main_test", + "//test:output_formatter_test", + "//test:options_test", + "//test:frequency_test", + "//test:flush_worker_test", + "//test:factories_test", + "//test:client_worker_test", + "//test:client_test", + "//test:benchmark_http_client_test", + "//test/..." ], "default": "//test/..." } diff --git a/MAINTAINERS.md b/MAINTAINERS.md index 1ba6beb17..e846e4c0e 100644 --- a/MAINTAINERS.md +++ b/MAINTAINERS.md @@ -56,15 +56,19 @@ important maintenance task. When performing the update, follow this procedure: ``` INFO: SHA256 (https://github.com/envoyproxy/envoy/archive/9753819331d1547c4b8294546a6461a3777958f5.tar.gz) = f4d26c7e78c0a478d959ea8bc877f260d4658a8b44e294e3a400f20ad44d41a3 ``` - 1. Update `ENVOY_SHA` in [bazel/repositories.bzl](bazel/repositories.bzl) to +1. Update `ENVOY_SHA` in [bazel/repositories.bzl](bazel/repositories.bzl) to this value. 1. Sync (copy) [.bazelrc](.bazelrc) from [Envoy's version](https://github.com/envoyproxy/envoy/blob/main/.bazelrc) to update our build configurations. Be sure to retain our local modifications, all lines that are unique to Nighthawk are marked with comment `# unique`. +1. In the updated [.bazelrc](.bazelrc) search for `experimental_docker_image`. + Copy the SHA and update `envoyproxy/envoy-build-ubuntu` over at the top of [.circleci/config.yml](.circleci/config.yml). 1. Sync (copy) [.bazelversion](.bazelversion) from [Envoy's version](https://github.com/envoyproxy/envoy/blob/main/.bazelversion) to ensure we are using the same build system version. +1. Sync (copy) [ci/run_envoy_docker.sh](ci/run_envoy_docker.sh) from + [Envoy's version](https://github.com/envoyproxy/envoy/blob/main/ci/run_envoy_docker.sh). 1. Run `ci/do_ci.sh test`. Sometimes the dependency update comes with changes that break our build. Include any changes required to Nighthawk to fix that in the same PR. diff --git a/README.md b/README.md index dbbd6d238..47927a487 100644 --- a/README.md +++ b/README.md @@ -43,8 +43,7 @@ bazel build -c opt //:nighthawk ``` USAGE: -bazel-bin/nighthawk_client [--allow-envoy-deprecated-v2-api] -[--latency-response-header-name ] +bazel-bin/nighthawk_client [--latency-response-header-name ] [--stats-flush-interval ] [--stats-sinks ] ... [--no-duration] [--simple-warmup] @@ -84,10 +83,6 @@ format> Where: ---allow-envoy-deprecated-v2-api -Set to allow usage of the v2 api. (Not recommended, support will stop -in Q1 2021). Default: false - --latency-response-header-name Set an optional header name that will be returned in responses, whose values will be tracked in a latency histogram if set. Can be used in diff --git a/WORKSPACE b/WORKSPACE index 17ed128a7..d101bd884 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -30,19 +30,11 @@ load("@envoy//bazel:dependency_imports.bzl", "envoy_dependency_imports") envoy_dependency_imports() # For PIP support: -load("@rules_python//python:pip.bzl", "pip3_import", "pip_repositories") - -pip_repositories() +load("@rules_python//python:pip.bzl", "pip_install") # This rule translates the specified requirements.txt into # @my_deps//:requirements.bzl, which itself exposes a pip_install method. -pip3_import( +pip_install( name = "python_pip_deps", requirements = "//:requirements.txt", ) - -# Load the pip_install symbol for my_deps, and create the dependencies' -# repositories. -load("@python_pip_deps//:requirements.bzl", "pip_install") - -pip_install() diff --git a/api/client/options.proto b/api/client/options.proto index 3274dd015..041083217 100644 --- a/api/client/options.proto +++ b/api/client/options.proto @@ -220,9 +220,9 @@ message CommandLineOptions { // "emit_previous_request_delta_in_response_header" to record elapsed time between request // arrivals. google.protobuf.StringValue latency_response_header_name = 36; - // Set to allow usage of the v2 api. (Not recommended, support will stop in Q1 2021). - google.protobuf.BoolValue allow_envoy_deprecated_v2_api = 38 [deprecated = true]; // Provide an execution starting date and time. Optional, any value specified must be in the // future. google.protobuf.Timestamp scheduled_start = 105; + + reserved 38; // deprecated } diff --git a/api/client/service.proto b/api/client/service.proto index 38d21304b..ca4a88f22 100644 --- a/api/client/service.proto +++ b/api/client/service.proto @@ -2,6 +2,7 @@ syntax = "proto3"; package nighthawk.client; +import "google/protobuf/duration.proto"; import "google/rpc/status.proto"; import "validate/validate.proto"; @@ -32,6 +33,10 @@ message ExecutionRequest { message ExecutionResponse { Output output = 1; google.rpc.Status error_detail = 7; + // Opaque identifier for lookup purposes. This will be taken from CommandLineOptions if set, + // if it is not set there it will be auto-generated. The format used for auto-generated + // identifiers may change at any time. + string execution_id = 8; } service NighthawkService { diff --git a/api/distributor/BUILD b/api/distributor/BUILD new file mode 100644 index 000000000..b47bafffe --- /dev/null +++ b/api/distributor/BUILD @@ -0,0 +1,32 @@ +load("@com_github_grpc_grpc//bazel:cc_grpc_library.bzl", "cc_grpc_library") +load("@envoy_api//bazel:api_build_system.bzl", "api_cc_py_proto_library") + +licenses(["notice"]) # Apache 2 + +api_cc_py_proto_library( + name = "distributor", + srcs = [ + "distributor.proto", + ], + visibility = ["//visibility:public"], + deps = [ + "//api/client:base", + "@envoy_api//envoy/config/core/v3:pkg", + ], +) + +cc_grpc_library( + name = "distributor_grpc_lib", + srcs = [ + ":distributor", + ], + generate_mocks = True, + grpc_only = True, + proto_only = False, + use_external = False, + visibility = ["//visibility:public"], + well_known_protos = True, + deps = [ + ":distributor_cc_proto", + ], +) diff --git a/api/distributor/distributor.proto b/api/distributor/distributor.proto new file mode 100644 index 000000000..81235748c --- /dev/null +++ b/api/distributor/distributor.proto @@ -0,0 +1,29 @@ +syntax = "proto3"; + +package nighthawk; + +import "envoy/config/core/v3/address.proto"; + +import "api/client/service.proto"; + +import "validate/validate.proto"; + +// Perform an execution request through an intermediate service that will in turn delegate to one or +// more other services Nighthawk load generator services. +message DistributedRequest { + client.ExecutionRequest execution_request = 1; + // Specify one or more services that will handle the inner message associated to this. + repeated envoy.config.core.v3.Address services = 3 [(validate.rules).repeated .min_items = 1]; +} + +// Carries responses associated with a DistributedRequest. +message DistributedResponse { +} + +// Service which distributes messages to one or more other services for handling, and streams back +// response messages. +service NighthawkDistributor { + // Propagate the message wrapped in DistributedRequest to one or more other services for handling. + rpc DistributedRequestStream(stream DistributedRequest) returns (stream DistributedResponse) { + } +} diff --git a/api/sink/BUILD b/api/sink/BUILD new file mode 100644 index 000000000..f3a01a41d --- /dev/null +++ b/api/sink/BUILD @@ -0,0 +1,32 @@ +load("@com_github_grpc_grpc//bazel:cc_grpc_library.bzl", "cc_grpc_library") +load("@envoy_api//bazel:api_build_system.bzl", "api_cc_py_proto_library") + +licenses(["notice"]) # Apache 2 + +api_cc_py_proto_library( + name = "sink", + srcs = [ + "sink.proto", + ], + visibility = ["//visibility:public"], + deps = [ + "//api/client:base", + "@envoy_api//envoy/config/core/v3:pkg", + ], +) + +cc_grpc_library( + name = "sink_grpc_lib", + srcs = [ + ":sink", + ], + generate_mocks = True, + grpc_only = True, + proto_only = False, + use_external = False, + visibility = ["//visibility:public"], + well_known_protos = True, + deps = [ + ":sink_cc_proto", + ], +) diff --git a/api/sink/sink.proto b/api/sink/sink.proto new file mode 100644 index 000000000..df98bd505 --- /dev/null +++ b/api/sink/sink.proto @@ -0,0 +1,37 @@ +syntax = "proto3"; + +package nighthawk; + +import "api/client/service.proto"; +import "validate/validate.proto"; + +// Encapsulates an ExecutionResponse. +message StoreExecutionRequest { + // Response contains the effective execution id, which will serve as the lookup key. + nighthawk.client.ExecutionResponse execution_response = 1; +} + +// Empty return value message, that serves as a future extension point. +message StoreExecutionResponse { +} + +message SinkRequest { + // Unique id for lookup purposes. Required. + string execution_id = 1 [(validate.rules).string.min_len = 1]; +} + +message SinkResponse { + // Response associated to the requested execution id. + nighthawk.client.ExecutionResponse execution_response = 1; +} + +service NighthawkSink { + // Accepts a stream of execution responses, which is the return value of + // NighthawkService.ExecutionStream. Workers can forward their results using this method. + rpc StoreExecutionResponseStream(stream StoreExecutionRequest) returns (StoreExecutionResponse) { + } + + // Gets the stored responses associated to an execution, keyed by execution id. + rpc SinkRequestStream(stream SinkRequest) returns (stream SinkResponse) { + } +} \ No newline at end of file diff --git a/bazel/repositories.bzl b/bazel/repositories.bzl index bf6e61ddb..e35f2ea6a 100644 --- a/bazel/repositories.bzl +++ b/bazel/repositories.bzl @@ -1,7 +1,7 @@ load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive") -ENVOY_COMMIT = "cfe0391ce24f7d11b3b125078ab77f6f4142e4ac" # Feb 5th, 2021 -ENVOY_SHA = "5c9fbd5b95837ef409d85b6408bbb616452c164be5016c62e1c5055d3eeee268" +ENVOY_COMMIT = "031f75dd113e2f7be41b94a0365145e7bf1e6c12" # March 14th, 2021 +ENVOY_SHA = "20f5f98cc9a5c1ddd2c3de434b019273e3dbe02afe52396b90d74fab38dba4ba" HDR_HISTOGRAM_C_VERSION = "0.11.2" # October 12th, 2020 HDR_HISTOGRAM_C_SHA = "637f28b5f64de2e268131e4e34e6eef0b91cf5ff99167db447d9b2825eae6bad" diff --git a/benchmarks/README.md b/benchmarks/README.md index 33244c8bf..b352082a6 100644 --- a/benchmarks/README.md +++ b/benchmarks/README.md @@ -101,7 +101,7 @@ docker run -it --rm \ --env NH_DOCKER_IMAGE="envoyproxy/nighthawk-dev:latest" \ --env ENVOY_DOCKER_IMAGE_TO_TEST="envoyproxy/envoy-dev:f61b096f6a2dd3a9c74b9a9369a6ea398dbe1f0f" \ --env TMPDIR="${OUTDIR}" \ - oschaaf/benchmark-dev:latest ./benchmarks --log-cli-level=info -vvvv + envoyproxy/nighthawk-benchmark-dev:latest ./benchmarks --log-cli-level=info -vvvv ``` # TODOs diff --git a/ci/do_ci.sh b/ci/do_ci.sh index 792f617a3..11128b3c4 100755 --- a/ci/do_ci.sh +++ b/ci/do_ci.sh @@ -34,13 +34,14 @@ function do_test() { } function do_clang_tidy() { - # TODO(#546): deflake clang tidy runs, and remove '|| true' here. - ci/run_clang_tidy.sh || true + # clang-tidy will warn on standard library issues with libc++ + BAZEL_BUILD_OPTIONS=("--config=clang" "${BAZEL_BUILD_OPTIONS[@]}") + BAZEL_BUILD_OPTIONS="${BAZEL_BUILD_OPTIONS[*]}" NUM_CPUS=4 ci/run_clang_tidy.sh } function do_unit_test_coverage() { export TEST_TARGETS="//test/... -//test:python_test" - export COVERAGE_THRESHOLD=94.3 + export COVERAGE_THRESHOLD=94.2 echo "bazel coverage build with tests ${TEST_TARGETS}" test/run_nighthawk_bazel_coverage.sh ${TEST_TARGETS} exit 0 @@ -148,6 +149,7 @@ function do_docker() { ./ci/docker/docker_build.sh ./ci/docker/docker_push.sh ./ci/docker/benchmark_build.sh + ./ci/docker/benchmark_push.sh } function do_fix_format() { diff --git a/ci/docker/Dockerfile-nighthawk-benchmark b/ci/docker/Dockerfile-nighthawk-benchmark index 5430784f4..4929e15e9 100644 --- a/ci/docker/Dockerfile-nighthawk-benchmark +++ b/ci/docker/Dockerfile-nighthawk-benchmark @@ -8,7 +8,7 @@ WORKDIR /usr/local/bin/benchmarks COPY benchmarks /usr/local/bin/benchmarks/ -RUN apk add --no-cache docker=20.10.3-r0 openrc=0.42.1-r19 python3=3.8.7-r0 +RUN apk add --no-cache docker=20.10.3-r0 openrc=0.42.1-r19 python3>=3.8.7-r0 RUN rc-update add docker boot RUN if [ ! -e /usr/bin/python ]; then ln -sf python3 /usr/bin/python; fi && \ diff --git a/ci/docker/benchmark_build.sh b/ci/docker/benchmark_build.sh index 280934664..693878a82 100755 --- a/ci/docker/benchmark_build.sh +++ b/ci/docker/benchmark_build.sh @@ -1,6 +1,6 @@ #!/bin/bash -# Builds a docker image benchmark-dev:latest containing the benchmark scripts +# Builds a docker image nighthawk-benchmark-dev:latest containing the benchmark scripts # Stop on errors. set -e diff --git a/ci/docker/benchmark_push.sh b/ci/docker/benchmark_push.sh new file mode 100755 index 000000000..88e678da4 --- /dev/null +++ b/ci/docker/benchmark_push.sh @@ -0,0 +1,5 @@ +#!/bin/bash + +DOCKER_IMAGE_PREFIX="${DOCKER_IMAGE_PREFIX:-envoyproxy/nighthawk-benchmark}" + +source ./ci/docker/docker_push.sh \ No newline at end of file diff --git a/ci/run_clang_tidy.sh b/ci/run_clang_tidy.sh index 040b5a46b..6412bedc1 100755 --- a/ci/run_clang_tidy.sh +++ b/ci/run_clang_tidy.sh @@ -31,7 +31,7 @@ echo "Generating compilation database..." # Do not run clang-tidy against win32 impl # TODO(scw00): We should run clang-tidy against win32 impl once we have clang-cl support for Windows function exclude_win32_impl() { - grep -v source/common/filesystem/win32/ | grep -v source/common/common/win32 | grep -v source/exe/win32 | grep -v source/common/api/win32 + grep -v source/common/filesystem/win32/ | grep -v source/common/common/win32 | grep -v source/exe/win32 | grep -v source/common/api/win32 | grep -v source/common/event/win32 } # Do not run clang-tidy against macOS impl @@ -81,8 +81,13 @@ function exclude_wasm_test_data() { grep -v wasm/test_data } +# Exclude files which are part of the Wasm examples +function exclude_wasm_examples() { + grep -v examples/wasm +} + function filter_excludes() { - exclude_check_format_testdata | exclude_headersplit_testdata | exclude_chromium_url | exclude_win32_impl | exclude_macos_impl | exclude_third_party | exclude_wasm_emscripten | exclude_wasm_sdk | exclude_wasm_host | exclude_wasm_test_data + exclude_check_format_testdata | exclude_headersplit_testdata | exclude_chromium_url | exclude_win32_impl | exclude_macos_impl | exclude_third_party | exclude_wasm_emscripten | exclude_wasm_sdk | exclude_wasm_host | exclude_wasm_test_data | exclude_wasm_examples } function run_clang_tidy() { @@ -108,9 +113,7 @@ elif [[ "${RUN_FULL_CLANG_TIDY}" == 1 ]]; then run_clang_tidy else if [[ -z "${DIFF_REF}" ]]; then - if [[ "${BUILD_REASON}" == "PullRequest" ]]; then - DIFF_REF="remotes/origin/${SYSTEM_PULLREQUEST_TARGETBRANCH}" - elif [[ "${BUILD_REASON}" == *CI ]]; then + if [[ "${BUILD_REASON}" == *CI ]]; then DIFF_REF="HEAD^" else DIFF_REF=$("${ENVOY_SRCDIR}"/tools/git/last_github_commit.sh) diff --git a/ci/run_envoy_docker.sh b/ci/run_envoy_docker.sh index c6b91fae5..5e88dd705 100755 --- a/ci/run_envoy_docker.sh +++ b/ci/run_envoy_docker.sh @@ -2,26 +2,98 @@ set -e -. ci/envoy_build_sha.sh +# shellcheck source=ci/envoy_build_sha.sh +. "$(dirname "$0")"/envoy_build_sha.sh -# We run as root and later drop permissions. This is required to setup the USER -# in useradd below, which is need for correct Python execution in the Docker -# environment. -USER=root -USER_GROUP=root +function is_windows() { + [[ "$(uname -s)" == *NT* ]] +} + +read -ra ENVOY_DOCKER_OPTIONS <<< "${ENVOY_DOCKER_OPTIONS:-}" + +# TODO(phlax): uppercase these env vars +export HTTP_PROXY="${http_proxy:-}" +export HTTPS_PROXY="${https_proxy:-}" +export NO_PROXY="${no_proxy:-}" + +if is_windows; then + [[ -z "${IMAGE_NAME}" ]] && IMAGE_NAME="envoyproxy/envoy-build-windows2019" + # TODO(sunjayBhatia): Currently ENVOY_DOCKER_OPTIONS is ignored on Windows because + # CI sets it to a Linux-specific value. Undo this once https://github.com/envoyproxy/envoy/issues/13272 + # is resolved. + ENVOY_DOCKER_OPTIONS=() + DEFAULT_ENVOY_DOCKER_BUILD_DIR=C:/Windows/Temp/envoy-docker-build + BUILD_DIR_MOUNT_DEST=C:/build + # Replace MSYS style drive letter (/c/) with driver letter designation (C:/) + SOURCE_DIR=$(echo "${PWD}" | sed -E "s#/([a-zA-Z])/#\1:/#") + SOURCE_DIR_MOUNT_DEST=C:/source + START_COMMAND=("bash" "-c" "cd source && $*") +else + [[ -z "${IMAGE_NAME}" ]] && IMAGE_NAME="envoyproxy/envoy-build-ubuntu" + # We run as root and later drop permissions. This is required to setup the USER + # in useradd below, which is need for correct Python execution in the Docker + # environment. + ENVOY_DOCKER_OPTIONS+=(-u root:root) + ENVOY_DOCKER_OPTIONS+=(-v /var/run/docker.sock:/var/run/docker.sock) + ENVOY_DOCKER_OPTIONS+=(--cap-add SYS_PTRACE --cap-add NET_RAW --cap-add NET_ADMIN) + DEFAULT_ENVOY_DOCKER_BUILD_DIR=/tmp/envoy-docker-build + BUILD_DIR_MOUNT_DEST=/build + SOURCE_DIR="${PWD}" + SOURCE_DIR_MOUNT_DEST=/source + START_COMMAND=("/bin/bash" "-lc" "groupadd --gid $(id -g) -f envoygroup \ + && useradd -o --uid $(id -u) --gid $(id -g) --no-create-home --home-dir /build envoybuild \ + && usermod -a -G pcap envoybuild \ + && chown envoybuild:envoygroup /build \ + && sudo -EHs -u envoybuild bash -c 'cd /source && $*'") +fi -[[ -z "${IMAGE_NAME}" ]] && IMAGE_NAME="envoyproxy/envoy-build-ubuntu" # The IMAGE_ID defaults to the CI hash but can be set to an arbitrary image ID (found with 'docker # images'). [[ -z "${IMAGE_ID}" ]] && IMAGE_ID="${ENVOY_BUILD_SHA}" -[[ -z "${ENVOY_DOCKER_BUILD_DIR}" ]] && ENVOY_DOCKER_BUILD_DIR=/tmp/envoy-docker-build +[[ -z "${ENVOY_DOCKER_BUILD_DIR}" ]] && ENVOY_DOCKER_BUILD_DIR="${DEFAULT_ENVOY_DOCKER_BUILD_DIR}" +# Replace backslash with forward slash for Windows style paths +ENVOY_DOCKER_BUILD_DIR="${ENVOY_DOCKER_BUILD_DIR//\\//}" +mkdir -p "${ENVOY_DOCKER_BUILD_DIR}" -[[ -f .git ]] && [[ ! -d .git ]] && GIT_VOLUME_OPTION="-v $(git rev-parse --git-common-dir):$(git rev-parse --git-common-dir)" +[[ -t 1 ]] && ENVOY_DOCKER_OPTIONS+=("-it") +[[ -f .git ]] && [[ ! -d .git ]] && ENVOY_DOCKER_OPTIONS+=(-v "$(git rev-parse --git-common-dir):$(git rev-parse --git-common-dir)") +[[ -n "${SSH_AUTH_SOCK}" ]] && ENVOY_DOCKER_OPTIONS+=(-v "${SSH_AUTH_SOCK}:${SSH_AUTH_SOCK}" -e SSH_AUTH_SOCK) + +export ENVOY_BUILD_IMAGE="${IMAGE_NAME}:${IMAGE_ID}" -mkdir -p "${ENVOY_DOCKER_BUILD_DIR}" # Since we specify an explicit hash, docker-run will pull from the remote repo if missing. -docker run --rm -t -i -e HTTP_PROXY=${http_proxy} -e HTTPS_PROXY=${https_proxy} \ - -u "${USER}":"${USER_GROUP}" -v "${ENVOY_DOCKER_BUILD_DIR}":/build ${GIT_VOLUME_OPTION} \ - -v "$PWD":/source -e NUM_CPUS --cap-add SYS_PTRACE --cap-add NET_RAW --cap-add NET_ADMIN "${IMAGE_NAME}":"${IMAGE_ID}" \ - /bin/bash -lc "groupadd --gid $(id -g) -f envoygroup && useradd -o --uid $(id -u) --gid $(id -g) --no-create-home \ - --home-dir /source envoybuild && usermod -a -G pcap envoybuild && su envoybuild -c \"cd source && $*\"" +docker run --rm \ + "${ENVOY_DOCKER_OPTIONS[@]}" \ + -v "${ENVOY_DOCKER_BUILD_DIR}":"${BUILD_DIR_MOUNT_DEST}" \ + -v "${SOURCE_DIR}":"${SOURCE_DIR_MOUNT_DEST}" \ + -e AZP_BRANCH \ + -e HTTP_PROXY \ + -e HTTPS_PROXY \ + -e NO_PROXY \ + -e BAZEL_STARTUP_OPTIONS \ + -e BAZEL_BUILD_EXTRA_OPTIONS \ + -e BAZEL_EXTRA_TEST_OPTIONS \ + -e BAZEL_REMOTE_CACHE \ + -e ENVOY_STDLIB \ + -e BUILD_REASON \ + -e BAZEL_REMOTE_INSTANCE \ + -e GCP_SERVICE_ACCOUNT_KEY \ + -e NUM_CPUS \ + -e ENVOY_RBE \ + -e ENVOY_BUILD_IMAGE \ + -e ENVOY_SRCDIR \ + -e ENVOY_BUILD_TARGET \ + -e SYSTEM_PULLREQUEST_PULLREQUESTNUMBER \ + -e GCS_ARTIFACT_BUCKET \ + -e GITHUB_TOKEN \ + -e BUILD_SOURCEBRANCHNAME \ + -e BAZELISK_BASE_URL \ + -e ENVOY_BUILD_ARCH \ + -e SLACK_TOKEN \ + -e BUILD_URI\ + -e REPO_URI \ + -e SYSTEM_STAGEDISPLAYNAME \ + -e SYSTEM_JOBDISPLAYNAME \ + -e SYSTEM_PULLREQUEST_PULLREQUESTNUMBER \ + "${ENVOY_BUILD_IMAGE}" \ + "${START_COMMAND[@]}" diff --git a/docs/root/examples/MULTIPLE_ENDPOINTS.md b/docs/root/examples/MULTIPLE_ENDPOINTS.md new file mode 100644 index 000000000..0d50aedc0 --- /dev/null +++ b/docs/root/examples/MULTIPLE_ENDPOINTS.md @@ -0,0 +1,43 @@ +# Hitting multiple endpoints with a traffic profile + +## Description + +Below is an example which will send requests to two endpoints (`127.0.0.1:80` and `127.0.0.2:80`), while alternating between two request headers which contain different paths and hosts. + +## Practical use + +This example has been useful to test a mesh that exposed multiple endpoints, which in turn would offer access to multiple applications via different hosts/paths. + +## Features used + +This example illustrates the following features: + +- [Request Source](https://github.com/envoyproxy/nighthawk/blob/261abb62c40afbdebb317f320fe67f1a1da1838f/api/request_source/request_source_plugin.proto#L15) (specifically the file-based implementation). +- [Multi-targeting](https://github.com/envoyproxy/nighthawk/blob/261abb62c40afbdebb317f320fe67f1a1da1838f/api/client/options.proto#L84) + +## Steps + +### Configure the file based request source + +Place a file called `traffic-profile.yaml` in your current working directory. This will act as your configuration for the file-based request source. + + +```yaml +options: + - request_method: 1 + request_headers: + - { header: { key: ":path", value: "/foo" } } + - { header: { key: ":authority", value: "foo.com" } } + - request_method: 1 + request_headers: + - { header: { key: ":path", value: "/bar" } } + - { header: { key: ":authority", value: "bar.com" } } +``` + +### Configure the CLI + +Below is a minimal CLI example which will consume the file based request source configuration created above, and hit multiple endpoints. + +```bash +bazel-bin/nighthawk_client --request-source-plugin-config "{name:\"nighthawk.file-based-request-source-plugin\",typed_config:{\"@type\":\"type.googleapis.com/nighthawk.request_source.FileBasedOptionsListRequestSourceConfig\",file_path:\"traffic-profile.yaml\",}}" --multi-target-endpoint 127.0.0.1:80 --multi-target-endpoint 127.0.0.2:80 --multi-target-path / +``` diff --git a/include/nighthawk/client/options.h b/include/nighthawk/client/options.h index a04292a86..0671ef039 100644 --- a/include/nighthawk/client/options.h +++ b/include/nighthawk/client/options.h @@ -75,7 +75,6 @@ class Options { virtual std::vector statsSinks() const PURE; virtual uint32_t statsFlushInterval() const PURE; virtual std::string responseHeaderWithLatencyInput() const PURE; - virtual bool allowEnvoyDeprecatedV2Api() const PURE; virtual absl::optional scheduled_start() const PURE; /** diff --git a/include/nighthawk/distributor/BUILD b/include/nighthawk/distributor/BUILD new file mode 100644 index 000000000..f3f84e34f --- /dev/null +++ b/include/nighthawk/distributor/BUILD @@ -0,0 +1,25 @@ +load( + "@envoy//bazel:envoy_build_system.bzl", + "envoy_basic_cc_library", + "envoy_package", +) + +licenses(["notice"]) # Apache 2 + +envoy_package() + +envoy_basic_cc_library( + name = "nighthawk_distributor_client", + hdrs = [ + "nighthawk_distributor_client.h", + ], + include_prefix = "nighthawk/common", + deps = [ + "//api/distributor:distributor_cc_proto", + "//api/distributor:distributor_grpc_lib", + "@envoy//include/envoy/common:base_includes", + "@envoy//source/common/common:assert_lib_with_external_headers", + "@envoy//source/common/common:statusor_lib_with_external_headers", + "@envoy//source/common/protobuf:protobuf_with_external_headers", + ], +) diff --git a/include/nighthawk/distributor/nighthawk_distributor_client.h b/include/nighthawk/distributor/nighthawk_distributor_client.h new file mode 100644 index 000000000..3efb3d9a0 --- /dev/null +++ b/include/nighthawk/distributor/nighthawk_distributor_client.h @@ -0,0 +1,31 @@ +#pragma once +#include "envoy/common/pure.h" + +#include "external/envoy/source/common/common/statusor.h" +#include "external/envoy/source/common/protobuf/protobuf.h" + +#include "api/distributor/distributor.grpc.pb.h" + +namespace Nighthawk { + +/** + * Interface of a gRPC distributor service client. + */ +class NighthawkDistributorClient { +public: + virtual ~NighthawkDistributorClient() = default; + + /** + * Propagate messages to one or more other services for handling. + * + * @param nighthawk_distributor_stub Used to open a channel to the distributor service. + * @param distributed_request Provide the message that the distributor service should propagate. + * @return absl::StatusOr<::nighthawk::DistributedResponse> Either a status indicating failure, or + * a DistributedResponse upon success. + */ + virtual absl::StatusOr<::nighthawk::DistributedResponse> + DistributedRequest(nighthawk::NighthawkDistributor::StubInterface& nighthawk_distributor_stub, + const nighthawk::DistributedRequest& distributed_request) const PURE; +}; + +} // namespace Nighthawk diff --git a/include/nighthawk/sink/BUILD b/include/nighthawk/sink/BUILD new file mode 100644 index 000000000..f1f1cda7a --- /dev/null +++ b/include/nighthawk/sink/BUILD @@ -0,0 +1,37 @@ +load( + "@envoy//bazel:envoy_build_system.bzl", + "envoy_basic_cc_library", + "envoy_package", +) + +licenses(["notice"]) # Apache 2 + +envoy_package() + +envoy_basic_cc_library( + name = "nighthawk_sink_client", + hdrs = [ + "nighthawk_sink_client.h", + ], + include_prefix = "nighthawk/sink", + deps = [ + "//api/sink:sink_cc_proto", + "//api/sink:sink_grpc_lib", + "@envoy//include/envoy/common:base_includes", + "@envoy//source/common/common:assert_lib_with_external_headers", + "@envoy//source/common/common:statusor_lib_with_external_headers", + "@envoy//source/common/protobuf:protobuf_with_external_headers", + ], +) + +envoy_basic_cc_library( + name = "sink_lib", + hdrs = [ + "sink.h", + ], + include_prefix = "nighthawk/sink", + deps = [ + "//api/client:grpc_service_lib", + "@envoy//source/common/common:statusor_lib_with_external_headers", + ], +) diff --git a/include/nighthawk/sink/nighthawk_sink_client.h b/include/nighthawk/sink/nighthawk_sink_client.h new file mode 100644 index 000000000..fae300db7 --- /dev/null +++ b/include/nighthawk/sink/nighthawk_sink_client.h @@ -0,0 +1,42 @@ +#pragma once +#include "envoy/common/pure.h" + +#include "external/envoy/source/common/common/statusor.h" +#include "external/envoy/source/common/protobuf/protobuf.h" + +#include "api/sink/sink.grpc.pb.h" + +namespace Nighthawk { + +/** + * Interface of a gRPC sink service client. + */ +class NighthawkSinkClient { +public: + virtual ~NighthawkSinkClient() = default; + + /** + * @brief Store an execution response. + * + * @param nighthawk_sink_stub Used to open a channel to the sink service. + * @param store_execution_request Provide the message that the sink should store. + * @return absl::StatusOr + */ + virtual absl::StatusOr StoreExecutionResponseStream( + nighthawk::NighthawkSink::StubInterface& nighthawk_sink_stub, + const nighthawk::StoreExecutionRequest& store_execution_request) const PURE; + + /** + * Look up ExecutionResponse messages in the sink. + * + * @param nighthawk_sink_stub Used to open a channel to the sink service. + * @param sink_request Provide the message that the sink should handle. + * @return absl::StatusOr Either a status indicating failure, or + * a SinkResponse upon success. + */ + virtual absl::StatusOr + SinkRequestStream(nighthawk::NighthawkSink::StubInterface& nighthawk_sink_stub, + const nighthawk::SinkRequest& sink_request) const PURE; +}; + +} // namespace Nighthawk diff --git a/include/nighthawk/sink/sink.h b/include/nighthawk/sink/sink.h new file mode 100644 index 000000000..c35e48dcd --- /dev/null +++ b/include/nighthawk/sink/sink.h @@ -0,0 +1,47 @@ + +#pragma once + +#include + +#include "envoy/common/pure.h" + +#include "external/envoy/source/common/common/statusor.h" + +#include "api/client/service.grpc.pb.h" + +#include "absl/strings/string_view.h" + +namespace Nighthawk { + +/** + * Abstract Sink interface. + */ +class Sink { +public: + virtual ~Sink() = default; + + /** + * Store an ExecutionResponse instance. Can be called multiple times for the same execution_id to + * persist multiple fragments that together will represent results belonging to a single + * execution. + * + * @param response Specify an ExecutionResponse instance that should be persisted. The + * ExecutionResponse must have its execution_id set. + * @return absl::Status Indicates if the operation succeeded or not. + */ + virtual absl::Status + StoreExecutionResultPiece(const nighthawk::client::ExecutionResponse& response) const PURE; + + /** + * Attempt to load a vector of ExecutionResponse instances associated to an execution id. + * + * @param execution_id Specify an execution_id that the desired set of ExecutionResponse + * instances are tagged with. + * @return absl::StatusOr>. + * When no fragments are found for the provided execution id, status kNotFound is returned. + */ + virtual absl::StatusOr> + LoadExecutionResult(absl::string_view execution_id) const PURE; +}; + +} // namespace Nighthawk \ No newline at end of file diff --git a/source/adaptive_load/adaptive_load_client_main.cc b/source/adaptive_load/adaptive_load_client_main.cc index be23d1051..2dc578685 100644 --- a/source/adaptive_load/adaptive_load_client_main.cc +++ b/source/adaptive_load/adaptive_load_client_main.cc @@ -111,7 +111,7 @@ uint32_t AdaptiveLoadClientMain::Run() { throw Nighthawk::NighthawkException("Unable to parse file \"" + spec_filename_ + "\" as a text protobuf (type " + spec.GetTypeName() + ")"); } - std::shared_ptr<::grpc::Channel> channel = grpc::CreateChannel( + std::shared_ptr channel = grpc::CreateChannel( nighthawk_service_address_, use_tls_ ? grpc::SslCredentials(grpc::SslCredentialsOptions()) : grpc::InsecureChannelCredentials()); std::unique_ptr stub( diff --git a/source/client/BUILD b/source/client/BUILD index affc901d1..1a6280a24 100644 --- a/source/client/BUILD +++ b/source/client/BUILD @@ -84,7 +84,7 @@ envoy_cc_library( "@envoy//source/exe:platform_header_lib_with_external_headers", "@envoy//source/exe:platform_impl_lib", "@envoy//source/exe:process_wide_lib_with_external_headers", - "@envoy//source/common/http:request_id_extension_lib_with_external_headers", + "@envoy//source/extensions/request_id/uuid:config_with_external_headers", "@envoy//source/extensions/transport_sockets:well_known_names_with_external_headers", "@envoy//source/extensions/transport_sockets/tls:context_lib_with_external_headers", "@envoy//source/server:options_lib_with_external_headers", diff --git a/source/client/client.cc b/source/client/client.cc index 8c08eda00..cd3d0b14f 100644 --- a/source/client/client.cc +++ b/source/client/client.cc @@ -53,7 +53,7 @@ bool Main::run() { Envoy::Event::RealTimeSystem time_system; // NO_CHECK_FORMAT(real_time) ProcessPtr process; std::unique_ptr stub; - std::shared_ptr<::grpc::Channel> channel; + std::shared_ptr channel; if (options_->nighthawkService() != "") { UriPtr uri; diff --git a/source/client/options_impl.cc b/source/client/options_impl.cc index 4cf761a41..6392e98a1 100644 --- a/source/client/options_impl.cc +++ b/source/client/options_impl.cc @@ -315,12 +315,6 @@ OptionsImpl::OptionsImpl(int argc, const char* const* argv) { "Default: \"\"", false, "", "string", cmd); - TCLAP::SwitchArg allow_envoy_deprecated_v2_api( - "", "allow-envoy-deprecated-v2-api", - "Set to allow usage of the v2 api. (Not recommended, support will stop in Q1 2021). Default: " - "false", - cmd); - Utility::parseCommand(cmd, argc, argv); // --duration and --no-duration are mutually exclusive @@ -453,7 +447,6 @@ OptionsImpl::OptionsImpl(int argc, const char* const* argv) { } TCLAP_SET_IF_SPECIFIED(stats_flush_interval, stats_flush_interval_); TCLAP_SET_IF_SPECIFIED(latency_response_header_name, latency_response_header_name_); - TCLAP_SET_IF_SPECIFIED(allow_envoy_deprecated_v2_api, allow_envoy_deprecated_v2_api_); // CLI-specific tests. // TODO(oschaaf): as per mergconflicts's remark, it would be nice to aggregate @@ -595,7 +588,7 @@ OptionsImpl::OptionsImpl(const nighthawk::client::CommandLineOptions& options) { request_headers_.push_back(header_string); } if (request_options.request_method() != - ::envoy::config::core::v3::RequestMethod::METHOD_UNSPECIFIED) { + envoy::config::core::v3::RequestMethod::METHOD_UNSPECIFIED) { request_method_ = request_options.request_method(); } request_body_size_ = @@ -659,8 +652,6 @@ OptionsImpl::OptionsImpl(const nighthawk::client::CommandLineOptions& options) { std::copy(options.labels().begin(), options.labels().end(), std::back_inserter(labels_)); latency_response_header_name_ = PROTOBUF_GET_WRAPPED_OR_DEFAULT( options, latency_response_header_name, latency_response_header_name_); - allow_envoy_deprecated_v2_api_ = PROTOBUF_GET_WRAPPED_OR_DEFAULT( - options, allow_envoy_deprecated_v2_api, allow_envoy_deprecated_v2_api_); if (options.has_scheduled_start()) { const auto elapsed_since_epoch = std::chrono::duration_cast( std::chrono::nanoseconds(options.scheduled_start().nanos()) + @@ -844,8 +835,6 @@ CommandLineOptionsPtr OptionsImpl::toCommandLineOptionsInternal() const { command_line_options->mutable_stats_flush_interval()->set_value(stats_flush_interval_); command_line_options->mutable_latency_response_header_name()->set_value( latency_response_header_name_); - command_line_options->mutable_allow_envoy_deprecated_v2_api()->set_value( - allow_envoy_deprecated_v2_api_); if (scheduled_start_.has_value()) { *(command_line_options->mutable_scheduled_start()) = Envoy::ProtobufUtil::TimeUtil::NanosecondsToTimestamp( diff --git a/source/client/options_impl.h b/source/client/options_impl.h index b84d80d3e..7132aba01 100644 --- a/source/client/options_impl.h +++ b/source/client/options_impl.h @@ -93,7 +93,6 @@ class OptionsImpl : public Options, public Envoy::Logger::Loggable scheduled_start() const override { return scheduled_start_; } private: @@ -151,7 +150,6 @@ class OptionsImpl : public Options, public Envoy::Logger::Loggable stats_sinks_; uint32_t stats_flush_interval_{5}; std::string latency_response_header_name_; - bool allow_envoy_deprecated_v2_api_{false}; absl::optional scheduled_start_; }; diff --git a/source/client/process_impl.cc b/source/client/process_impl.cc index f19f00cfc..69318a662 100644 --- a/source/client/process_impl.cc +++ b/source/client/process_impl.cc @@ -37,6 +37,7 @@ #include "external/envoy/source/extensions/tracers/zipkin/zipkin_tracer_impl.h" #endif #include "external/envoy/source/extensions/transport_sockets/well_known_names.h" +#include "external/envoy/source/server/options_impl.h" #include "external/envoy/source/server/options_impl_platform.h" #include "api/client/options.pb.h" @@ -64,13 +65,12 @@ class ClusterManagerFactory : public Envoy::Upstream::ProdClusterManagerFactory public: using Envoy::Upstream::ProdClusterManagerFactory::ProdClusterManagerFactory; - Envoy::Http::ConnectionPool::InstancePtr - allocateConnPool(Envoy::Event::Dispatcher& dispatcher, Envoy::Upstream::HostConstSharedPtr host, - Envoy::Upstream::ResourcePriority priority, - std::vector& protocols, - const Envoy::Network::ConnectionSocket::OptionsSharedPtr& options, - const Envoy::Network::TransportSocketOptionsSharedPtr& transport_socket_options, - Envoy::Upstream::ClusterConnectivityState& state) override { + Envoy::Http::ConnectionPool::InstancePtr allocateConnPool( + Envoy::Event::Dispatcher& dispatcher, Envoy::Upstream::HostConstSharedPtr host, + Envoy::Upstream::ResourcePriority priority, std::vector& protocols, + const Envoy::Network::ConnectionSocket::OptionsSharedPtr& options, + const Envoy::Network::TransportSocketOptionsSharedPtr& transport_socket_options, + Envoy::TimeSource& time_source, Envoy::Upstream::ClusterConnectivityState& state) override { // This changed in // https://github.com/envoyproxy/envoy/commit/93ee668a690d297ab5e8bd2cbf03771d852ebbda ALPN may // be set up to negotiate a protocol, in which case we'd need a HttpConnPoolImplMixed. However, @@ -98,7 +98,8 @@ class ClusterManagerFactory : public Envoy::Upstream::ProdClusterManagerFactory return Envoy::Http::ConnectionPool::InstancePtr{h1_pool}; } return Envoy::Upstream::ProdClusterManagerFactory::allocateConnPool( - dispatcher, host, priority, protocols, options, transport_socket_options, state); + dispatcher, host, priority, protocols, options, transport_socket_options, time_source, + state); } void setConnectionReuseStrategy( @@ -173,6 +174,7 @@ void ProcessImpl::shutdown() { cluster_manager_->shutdown(); } tls_.shutdownThread(); + dispatcher_->shutdown(); shutdown_ = true; } @@ -297,30 +299,10 @@ ProcessImpl::mergeWorkerStatistics(const std::vector& workers) return merged_statistics; } -void ProcessImpl::allowEnvoyDeprecatedV2Api(envoy::config::bootstrap::v3::Bootstrap& bootstrap) { - auto* admin_layer = bootstrap.mutable_layered_runtime()->add_layers(); - admin_layer->set_name("admin layer"); - admin_layer->mutable_admin_layer(); - envoy::config::bootstrap::v3::RuntimeLayer* runtime_layer = - bootstrap.mutable_layered_runtime()->add_layers(); - runtime_layer->set_name("static_layer"); - Envoy::ProtobufWkt::Value proto_true; - proto_true.set_string_value("true"); - (*runtime_layer->mutable_static_layer() - ->mutable_fields())["envoy.reloadable_features.enable_deprecated_v2_api"] = proto_true; - (*runtime_layer->mutable_static_layer() - ->mutable_fields())["envoy.reloadable_features.allow_prefetch"] = proto_true; -} - void ProcessImpl::createBootstrapConfiguration(envoy::config::bootstrap::v3::Bootstrap& bootstrap, const std::vector& uris, const UriPtr& request_source_uri, - int number_of_clusters, - bool allow_envoy_deprecated_v2_api) const { - if (allow_envoy_deprecated_v2_api) { - allowEnvoyDeprecatedV2Api(bootstrap); - } - + int number_of_clusters) const { for (int i = 0; i < number_of_clusters; i++) { auto* cluster = bootstrap.mutable_static_resources()->add_clusters(); RELEASE_ASSERT(!uris.empty(), "illegal configuration with zero endpoints"); @@ -507,8 +489,7 @@ bool ProcessImpl::runInternal(OutputCollector& collector, const std::vector( Envoy::Runtime::LoaderPtr{new Envoy::Runtime::LoaderImpl( - *dispatcher_, tls_, bootstrap.layered_runtime(), *local_info_, store_root_, generator_, + *dispatcher_, tls_, {}, *local_info_, store_root_, generator_, Envoy::ProtobufMessage::getStrictValidationVisitor(), *api_)}); ssl_context_manager_ = std::make_unique( time_system_); + + const Envoy::OptionsImpl::HotRestartVersionCb hot_restart_version_cb = [](bool) { + return "hot restart is disabled"; + }; + const Envoy::OptionsImpl envoy_options( + /* args = */ {"process_impl"}, hot_restart_version_cb, spdlog::level::info); cluster_manager_factory_ = std::make_unique( admin_, Envoy::Runtime::LoaderSingleton::get(), store_root_, tls_, dispatcher_->createDnsResolver({}, false), *ssl_context_manager_, *dispatcher_, *local_info_, secret_manager_, validation_context_, *api_, http_context_, grpc_context_, - router_context_, access_log_manager_, *singleton_manager_); + router_context_, access_log_manager_, *singleton_manager_, envoy_options); cluster_manager_factory_->setConnectionReuseStrategy( options_.h1ConnectionReuseStrategy() == nighthawk::client::H1ConnectionReuseStrategy::LRU ? Http1PoolImpl::ConnectionReuseStrategy::LRU diff --git a/source/client/process_impl.h b/source/client/process_impl.h index 5936007fc..81684815b 100644 --- a/source/client/process_impl.h +++ b/source/client/process_impl.h @@ -86,12 +86,6 @@ class ProcessImpl : public Process, public Envoy::Logger::Loggable& uris, - const UriPtr& request_source_uri, int number_of_workers, - bool allow_envoy_deprecated_v2_api) const; + const UriPtr& request_source_uri, int number_of_workers) const; void maybeCreateTracingDriver(const envoy::config::trace::v3::Tracing& configuration); void configureComponentLogLevels(spdlog::level::level_enum level); /** diff --git a/source/client/service_impl.cc b/source/client/service_impl.cc index fda3d69d7..36554e096 100644 --- a/source/client/service_impl.cc +++ b/source/client/service_impl.cc @@ -60,7 +60,7 @@ void ServiceImpl::writeResponse(const nighthawk::client::ExecutionResponse& resp } } -::grpc::Status ServiceImpl::finishGrpcStream(const bool success, absl::string_view description) { +grpc::Status ServiceImpl::finishGrpcStream(const bool success, absl::string_view description) { // We may get here while there's still an active future in-flight in the error-paths. // Allow it to wrap up and put it's response on the stream before finishing the stream. if (future_.valid()) { @@ -75,10 +75,10 @@ ::grpc::Status ServiceImpl::finishGrpcStream(const bool success, absl::string_vi // TODO(oschaaf): unit-test Process, create MockProcess & use in service_test.cc / client_test.cc // TODO(oschaaf): should we merge incoming request options with defaults? // TODO(oschaaf): aggregate the client's logs and forward them in the grpc response. -::grpc::Status ServiceImpl::ExecutionStream( - ::grpc::ServerContext* /*context*/, - ::grpc::ServerReaderWriter<::nighthawk::client::ExecutionResponse, - ::nighthawk::client::ExecutionRequest>* stream) { +grpc::Status ServiceImpl::ExecutionStream( + grpc::ServerContext* /*context*/, + grpc::ServerReaderWriter* stream) { nighthawk::client::ExecutionRequest request; stream_ = stream; @@ -122,10 +122,10 @@ RequestSourcePtr RequestSourceServiceImpl::createStaticEmptyRequestSource(const return std::make_unique(std::move(header), amount); } -::grpc::Status RequestSourceServiceImpl::RequestStream( - ::grpc::ServerContext* /*context*/, - ::grpc::ServerReaderWriter<::nighthawk::request_source::RequestStreamResponse, - ::nighthawk::request_source::RequestStreamRequest>* stream) { +grpc::Status RequestSourceServiceImpl::RequestStream( + grpc::ServerContext* /*context*/, + grpc::ServerReaderWriter* stream) { nighthawk::request_source::RequestStreamRequest request; bool ok = true; while (stream->Read(&request)) { diff --git a/source/client/service_impl.h b/source/client/service_impl.h index 760b6fb46..17cb7c3d8 100644 --- a/source/client/service_impl.h +++ b/source/client/service_impl.h @@ -45,22 +45,22 @@ class ServiceImpl final : public nighthawk::client::NighthawkService::Service, logging_context_ = std::move(logging_context); } - ::grpc::Status ExecutionStream( - ::grpc::ServerContext* context, - ::grpc::ServerReaderWriter<::nighthawk::client::ExecutionResponse, - ::nighthawk::client::ExecutionRequest>* stream) override; + grpc::Status + ExecutionStream(grpc::ServerContext* context, + grpc::ServerReaderWriter* stream) override; private: void handleExecutionRequest(const nighthawk::client::ExecutionRequest& request); void writeResponse(const nighthawk::client::ExecutionResponse& response); - ::grpc::Status finishGrpcStream(const bool success, absl::string_view description = ""); + grpc::Status finishGrpcStream(const bool success, absl::string_view description = ""); std::unique_ptr logging_context_; std::shared_ptr process_wide_; Envoy::Event::RealTimeSystem time_system_; // NO_CHECK_FORMAT(real_time) Envoy::Thread::MutexBasicLockable log_lock_; - ::grpc::ServerReaderWriter<::nighthawk::client::ExecutionResponse, - ::nighthawk::client::ExecutionRequest>* stream_; + grpc::ServerReaderWriter* stream_; std::future future_; // accepted_lock_ and accepted_event_ are used to synchronize the threads // when starting up a future to service a test, and ensure the code servicing it @@ -81,11 +81,10 @@ class RequestSourceServiceImpl final public Envoy::Logger::Loggable { public: - ::grpc::Status - RequestStream(::grpc::ServerContext* context, - ::grpc::ServerReaderWriter<::nighthawk::request_source::RequestStreamResponse, - ::nighthawk::request_source::RequestStreamRequest>* - stream) override; + grpc::Status RequestStream( + grpc::ServerContext* context, + grpc::ServerReaderWriter* stream) override; private: RequestSourcePtr createStaticEmptyRequestSource(const uint32_t amount); diff --git a/source/client/stream_decoder.cc b/source/client/stream_decoder.cc index c57555950..5c7c1afb1 100644 --- a/source/client/stream_decoder.cc +++ b/source/client/stream_decoder.cc @@ -3,10 +3,10 @@ #include #include "external/envoy/source/common/http/http1/codec_impl.h" -#include "external/envoy/source/common/http/request_id_extension_uuid_impl.h" #include "external/envoy/source/common/http/utility.h" #include "external/envoy/source/common/network/address_impl.h" #include "external/envoy/source/common/stream_info/stream_info_impl.h" +#include "external/envoy/source/extensions/request_id/uuid/config.h" namespace Nighthawk { namespace Client { @@ -172,9 +172,11 @@ void StreamDecoder::setupForTracing() { Envoy::Http::RequestHeaderMapPtr headers_copy = Envoy::Http::RequestHeaderMapImpl::create(); Envoy::Http::HeaderMapImpl::copyFrom(*headers_copy, *request_headers_); Envoy::Tracing::Decision tracing_decision = {Envoy::Tracing::Reason::ClientForced, true}; - Envoy::Http::UUIDRequestIDExtension uuid_generator(random_generator_); + envoy::extensions::request_id::uuid::v3::UuidRequestIdConfig uuid_request_id_config; + Envoy::Extensions::RequestId::UUIDRequestIDExtension uuid_generator(uuid_request_id_config, + random_generator_); uuid_generator.set(*headers_copy, true); - uuid_generator.setTraceStatus(*headers_copy, Envoy::Http::TraceStatus::Client); + uuid_generator.setTraceReason(*headers_copy, Envoy::Tracing::Reason::ClientForced); active_span_ = http_tracer_->startSpan(config_, *headers_copy, stream_info_, tracing_decision); active_span_->injectContext(*headers_copy); request_headers_.reset(headers_copy.release()); diff --git a/source/client/stream_decoder.h b/source/client/stream_decoder.h index cdb0653a0..a5c33978b 100644 --- a/source/client/stream_decoder.h +++ b/source/client/stream_decoder.h @@ -74,6 +74,7 @@ class StreamDecoder : public Envoy::Http::ResponseDecoder, void decodeData(Envoy::Buffer::Instance&, bool end_stream) override; void decodeTrailers(Envoy::Http::ResponseTrailerMapPtr&& trailers) override; void decodeMetadata(Envoy::Http::MetadataMapPtr&&) override { NOT_IMPLEMENTED_GCOVR_EXCL_LINE; } + void dumpState(std::ostream&, int) const override { NOT_IMPLEMENTED_GCOVR_EXCL_LINE; } // Http::StreamCallbacks void onResetStream(Envoy::Http::StreamResetReason reason, diff --git a/source/common/nighthawk_service_client_impl.cc b/source/common/nighthawk_service_client_impl.cc index 10dc82588..1c73c904c 100644 --- a/source/common/nighthawk_service_client_impl.cc +++ b/source/common/nighthawk_service_client_impl.cc @@ -12,9 +12,9 @@ NighthawkServiceClientImpl::PerformNighthawkBenchmark( nighthawk::client::ExecutionResponse response; *request.mutable_start_request()->mutable_options() = command_line_options; - ::grpc::ClientContext context; - std::shared_ptr<::grpc::ClientReaderWriterInterface> + grpc::ClientContext context; + std::shared_ptr> stream(nighthawk_service_stub->ExecutionStream(&context)); if (!stream->Write(request)) { @@ -32,7 +32,7 @@ NighthawkServiceClientImpl::PerformNighthawkBenchmark( if (!got_response) { return absl::InternalError("Nighthawk Service did not send a gRPC response."); } - ::grpc::Status status = stream->Finish(); + grpc::Status status = stream->Finish(); if (!status.ok()) { return absl::Status(static_cast(status.error_code()), status.error_message()); } diff --git a/source/common/request_source_impl.cc b/source/common/request_source_impl.cc index c2ce7d4b5..c6cea672e 100644 --- a/source/common/request_source_impl.cc +++ b/source/common/request_source_impl.cc @@ -43,7 +43,8 @@ void RemoteRequestSourceImpl::connectToRequestStreamGrpcService() { envoy::config::core::v3::GrpcService grpc_service; grpc_service.mutable_envoy_grpc()->set_cluster_name(service_cluster_name_); Envoy::Grpc::AsyncClientFactoryPtr cluster_manager = - cluster_manager_->grpcAsyncClientManager().factoryForGrpcService(grpc_service, scope_, true); + cluster_manager_->grpcAsyncClientManager().factoryForGrpcService(grpc_service, scope_, + /*skip_cluster_check=*/true); grpc_client_ = std::make_unique( cluster_manager->create(), dispatcher_, *base_header_, header_buffer_length_); grpc_client_->start(); @@ -63,4 +64,4 @@ RequestGenerator RemoteRequestSourceImpl::get() { return [this]() -> RequestPtr { return grpc_client_->maybeDequeue(); }; } -} // namespace Nighthawk \ No newline at end of file +} // namespace Nighthawk diff --git a/source/distributor/BUILD b/source/distributor/BUILD new file mode 100644 index 000000000..742ffe075 --- /dev/null +++ b/source/distributor/BUILD @@ -0,0 +1,24 @@ +load( + "@envoy//bazel:envoy_build_system.bzl", + "envoy_cc_library", + "envoy_package", +) + +licenses(["notice"]) # Apache 2 + +envoy_package() + +envoy_cc_library( + name = "nighthawk_distributor_client_impl", + srcs = [ + "nighthawk_distributor_client_impl.cc", + ], + hdrs = [ + "nighthawk_distributor_client_impl.h", + ], + repository = "@envoy", + visibility = ["//visibility:public"], + deps = [ + "//include/nighthawk/distributor:nighthawk_distributor_client", + ], +) diff --git a/source/distributor/nighthawk_distributor_client_impl.cc b/source/distributor/nighthawk_distributor_client_impl.cc new file mode 100644 index 000000000..2274d06c8 --- /dev/null +++ b/source/distributor/nighthawk_distributor_client_impl.cc @@ -0,0 +1,41 @@ +#include "distributor/nighthawk_distributor_client_impl.h" + +#include "external/envoy/source/common/common/assert.h" + +namespace Nighthawk { + +absl::StatusOr NighthawkDistributorClientImpl::DistributedRequest( + nighthawk::NighthawkDistributor::StubInterface& nighthawk_distributor_stub, + const nighthawk::DistributedRequest& distributed_request) const { + ::grpc::ClientContext context; + std::shared_ptr<::grpc::ClientReaderWriterInterface> + stream(nighthawk_distributor_stub.DistributedRequestStream(&context)); + ENVOY_LOG_MISC(trace, "Write {}", distributed_request.DebugString()); + if (!stream->Write(distributed_request)) { + return absl::UnavailableError( + "Failed to write request to the Nighthawk Distributor gRPC channel."); + } else if (!stream->WritesDone()) { + return absl::InternalError("WritesDone() failed on the Nighthawk Distributor gRPC channel."); + } + + bool got_response = false; + nighthawk::DistributedResponse response; + while (stream->Read(&response)) { + RELEASE_ASSERT(!got_response, + "Distributor Service has started responding with more than one message."); + got_response = true; + ENVOY_LOG_MISC(trace, "Read {}", response.DebugString()); + } + if (!got_response) { + return absl::InternalError("Distributor Service did not send a gRPC response."); + } + ::grpc::Status status = stream->Finish(); + ENVOY_LOG_MISC(trace, "Finish {}", status.ok()); + if (!status.ok()) { + return absl::Status(static_cast(status.error_code()), status.error_message()); + } + return response; +} + +} // namespace Nighthawk diff --git a/source/distributor/nighthawk_distributor_client_impl.h b/source/distributor/nighthawk_distributor_client_impl.h new file mode 100644 index 000000000..a43d32a1c --- /dev/null +++ b/source/distributor/nighthawk_distributor_client_impl.h @@ -0,0 +1,14 @@ +#pragma once + +#include "nighthawk/common/nighthawk_distributor_client.h" + +namespace Nighthawk { + +class NighthawkDistributorClientImpl : public NighthawkDistributorClient { +public: + absl::StatusOr<::nighthawk::DistributedResponse> + DistributedRequest(nighthawk::NighthawkDistributor::StubInterface& nighthawk_distributor_stub, + const nighthawk::DistributedRequest& distributed_request) const override; +}; + +} // namespace Nighthawk diff --git a/source/server/README.md b/source/server/README.md index 81ada3815..28cc51216 100644 --- a/source/server/README.md +++ b/source/server/README.md @@ -163,9 +163,10 @@ same time. ``` USAGE: -bazel-bin/nighthawk_test_server [--socket-mode ] [--socket-path -] [--disable-extensions -] [--cpuset-threads] +bazel-bin/nighthawk_test_server [--enable-core-dump] [--socket-mode +] [--socket-path ] +[--disable-extensions ] +[--cpuset-threads] [--enable-mutex-tracing] [--disable-hot-restart] [--mode ] [--parent-shutdown-time-s @@ -198,6 +199,9 @@ bazel-bin/nighthawk_test_server [--socket-mode ] [--socket-path Where: +--enable-core-dump +Enable core dumps + --socket-mode Socket file permission diff --git a/source/sink/BUILD b/source/sink/BUILD new file mode 100644 index 000000000..02d44f9bb --- /dev/null +++ b/source/sink/BUILD @@ -0,0 +1,41 @@ +load( + "@envoy//bazel:envoy_build_system.bzl", + "envoy_cc_library", + "envoy_package", +) + +licenses(["notice"]) # Apache 2 + +envoy_package() + +envoy_cc_library( + name = "nighthawk_sink_client_impl", + srcs = [ + "nighthawk_sink_client_impl.cc", + ], + hdrs = [ + "nighthawk_sink_client_impl.h", + ], + repository = "@envoy", + visibility = ["//visibility:public"], + deps = [ + "//include/nighthawk/sink:nighthawk_sink_client", + ], +) + +envoy_cc_library( + name = "sink_impl_lib", + srcs = [ + "sink_impl.cc", + ], + hdrs = [ + "sink_impl.h", + ], + repository = "@envoy", + visibility = ["//visibility:public"], + deps = [ + "//include/nighthawk/sink:sink_lib", + "@envoy//source/common/common:minimal_logger_lib_with_external_headers", + "@envoy//source/common/common:random_generator_lib_with_external_headers", + ], +) diff --git a/source/sink/nighthawk_sink_client_impl.cc b/source/sink/nighthawk_sink_client_impl.cc new file mode 100644 index 000000000..8a643b4a6 --- /dev/null +++ b/source/sink/nighthawk_sink_client_impl.cc @@ -0,0 +1,70 @@ +#include "sink/nighthawk_sink_client_impl.h" + +#include "external/envoy/source/common/common/assert.h" + +namespace Nighthawk { + +absl::StatusOr +NighthawkSinkClientImpl::StoreExecutionResponseStream( + nighthawk::NighthawkSink::StubInterface& nighthawk_sink_stub, + const nighthawk::StoreExecutionRequest& store_execution_request) const { + grpc::ClientContext context; + nighthawk::StoreExecutionResponse store_execution_response; + std::shared_ptr> stream( + nighthawk_sink_stub.StoreExecutionResponseStream(&context, &store_execution_response)); + if (!stream->Write(store_execution_request)) { + return absl::UnavailableError("Failed to write request to the Nighthawk Sink gRPC channel."); + } else if (!stream->WritesDone()) { + return absl::InternalError("WritesDone() failed on the Nighthawk Sink gRPC channel."); + } + grpc::Status status = stream->Finish(); + if (!status.ok()) { + return absl::Status(static_cast(status.error_code()), status.error_message()); + } + return store_execution_response; +} + +absl::StatusOr NighthawkSinkClientImpl::SinkRequestStream( + nighthawk::NighthawkSink::StubInterface& nighthawk_sink_stub, + const nighthawk::SinkRequest& sink_request) const { + nighthawk::SinkResponse response; + + grpc::ClientContext context; + std::shared_ptr< + grpc::ClientReaderWriterInterface> + stream(nighthawk_sink_stub.SinkRequestStream(&context)); + + if (!stream->Write(sink_request)) { + return absl::UnavailableError("Failed to write request to the Nighthawk Sink gRPC channel."); + } else if (!stream->WritesDone()) { + return absl::InternalError("WritesDone() failed on the Nighthawk Sink gRPC channel."); + } + + bool got_response = false; + while (stream->Read(&response)) { + /* + At the proto api level we support returning a stream of results. The sink service proto api + reflects this, and accepts what NighthawkService. ExecutionStream returns as a parameter + (though we wrap it in StoreExecutionRequest messages for extensibility purposes). So this + implies a stream, and not a single message. + + Having said that, today we constrain what we return to a single message in the implementations + where this is relevant. That's why we assert here, to make sure that stays put until an + explicit choice is made otherwise. + + Why do this? The intent of NighthawkService. ExecutionStream was to be able to stream + intermediate updates some day. So having streams in the api's keeps the door open on streaming + intermediary updates, without forcing a change the proto api. + */ + RELEASE_ASSERT(!got_response, + "Sink Service has started responding with more than one message."); + got_response = true; + } + grpc::Status status = stream->Finish(); + if (!status.ok()) { + return absl::Status(static_cast(status.error_code()), status.error_message()); + } + return response; +} + +} // namespace Nighthawk diff --git a/source/sink/nighthawk_sink_client_impl.h b/source/sink/nighthawk_sink_client_impl.h new file mode 100644 index 000000000..b2c920c8e --- /dev/null +++ b/source/sink/nighthawk_sink_client_impl.h @@ -0,0 +1,27 @@ +#pragma once + +#include "nighthawk/sink/nighthawk_sink_client.h" + +#include "external/envoy/source/common/common/statusor.h" +#include "external/envoy/source/common/protobuf/protobuf.h" + +namespace Nighthawk { + +/** + * Implements a the gRPC sink client interface. + * + * This class is stateless and may be called from multiple threads. Furthermore, the same gRPC stub + * is safe to use from multiple threads simultaneously. + */ +class NighthawkSinkClientImpl : public NighthawkSinkClient { +public: + absl::StatusOr StoreExecutionResponseStream( + nighthawk::NighthawkSink::StubInterface& nighthawk_sink_stub, + const nighthawk::StoreExecutionRequest& store_execution_request) const override; + + absl::StatusOr + SinkRequestStream(nighthawk::NighthawkSink::StubInterface& nighthawk_sink_stub, + const nighthawk::SinkRequest& sink_request) const override; +}; + +} // namespace Nighthawk diff --git a/source/sink/sink_impl.cc b/source/sink/sink_impl.cc new file mode 100644 index 000000000..ad15df326 --- /dev/null +++ b/source/sink/sink_impl.cc @@ -0,0 +1,107 @@ +#include "sink/sink_impl.h" + +#include +#include +#include + +#include "external/envoy/source/common/common/logger.h" +#include "external/envoy/source/common/common/random_generator.h" + +namespace Nighthawk { +namespace { + +absl::Status verifyCanBeUsedAsDirectoryName(absl::string_view s) { + Envoy::Random::RandomGeneratorImpl random; + const std::string reference_value = random.uuid(); + const std::string err_template = "'{}' is not a guid: {}"; + + if (s.size() != reference_value.size()) { + return absl::Status(absl::StatusCode::kInvalidArgument, + fmt::format(err_template, s, "bad string length.")); + } + for (size_t i = 0; i < s.size(); i++) { + if (reference_value[i] == '-') { + if (s[i] != '-') { + return absl::Status( + absl::StatusCode::kInvalidArgument, + fmt::format(err_template, s, "expectations around '-' positions not met.")); + } + continue; + } + if (!std::isxdigit(s[i])) { + return absl::Status(absl::StatusCode::kInvalidArgument, + fmt::format(err_template, s, "unexpected character encountered.")); + } + } + return absl::OkStatus(); +} + +} // namespace + +absl::Status FileSinkImpl::StoreExecutionResultPiece( + const nighthawk::client::ExecutionResponse& response) const { + const std::string& execution_id = response.execution_id(); + absl::Status status = verifyCanBeUsedAsDirectoryName(execution_id); + if (!status.ok()) { + return status; + } + std::error_code error_code; + std::filesystem::create_directories("/tmp/nh/" + std::string(execution_id) + "/", error_code); + // Note error_code will not be set if an existing directory existed. + if (error_code.value()) { + return absl::Status(absl::StatusCode::kInternal, error_code.message()); + } + // Write to a tmp file, and if that succeeds, we swap it atomically to the target path, + // to make the completely written file visible to consumers of LoadExecutionResult. + Envoy::Random::RandomGeneratorImpl random; + const std::string uid = "/tmp/nighthawk_" + random.uuid(); + { + std::ofstream ofs(uid.data(), std::ios_base::out | std::ios_base::binary); + if (!response.SerializeToOstream(&ofs)) { + return absl::Status(absl::StatusCode::kInternal, "Failure writing to temp file"); + } + } + std::filesystem::path filesystem_path(uid.data()); + const std::string new_name = + "/tmp/nh/" + std::string(execution_id) + "/" + std::string(filesystem_path.filename()); + std::filesystem::rename(uid.data(), new_name, error_code); + if (error_code.value()) { + return absl::Status(absl::StatusCode::kInternal, error_code.message()); + } + ENVOY_LOG_MISC(trace, "Stored '{}'.", new_name); + return absl::Status(); +} + +absl::StatusOr> +FileSinkImpl::LoadExecutionResult(absl::string_view execution_id) const { + absl::Status status = verifyCanBeUsedAsDirectoryName(execution_id); + if (!status.ok()) { + return status; + } + + std::filesystem::path filesystem_directory_path("/tmp/nh/" + std::string(execution_id) + "/"); + std::vector responses; + std::error_code error_code; + + for (const auto& it : + std::filesystem::directory_iterator(filesystem_directory_path, error_code)) { + if (error_code.value()) { + break; + } + nighthawk::client::ExecutionResponse response; + std::ifstream ifs(it.path(), std::ios_base::binary); + if (!response.ParseFromIstream(&ifs)) { + return absl::Status(absl::StatusCode::kInternal, + fmt::format("Failed to parse ExecutionResponse '{}'.", it.path())); + } else { + ENVOY_LOG_MISC(trace, "Loaded '{}'.", it.path()); + } + responses.push_back(response); + } + if (error_code.value()) { + return absl::Status(absl::StatusCode::kNotFound, error_code.message()); + } + return responses; +} + +} // namespace Nighthawk diff --git a/source/sink/sink_impl.h b/source/sink/sink_impl.h new file mode 100644 index 000000000..7c3013b11 --- /dev/null +++ b/source/sink/sink_impl.h @@ -0,0 +1,19 @@ +#pragma once + +#include "nighthawk/sink/sink.h" + +namespace Nighthawk { + +/** + * Filesystem based implementation of Sink. Uses /tmp/nh/{execution_id}/ to store and load + * data. + */ +class FileSinkImpl : public Sink { +public: + absl::Status + StoreExecutionResultPiece(const nighthawk::client::ExecutionResponse& response) const override; + absl::StatusOr> + LoadExecutionResult(absl::string_view id) const override; +}; + +} // namespace Nighthawk diff --git a/test/adaptive_load/fake_plugins/fake_metrics_plugin/fake_metrics_plugin_test.cc b/test/adaptive_load/fake_plugins/fake_metrics_plugin/fake_metrics_plugin_test.cc index bfbc39ad8..7b7ec0255 100644 --- a/test/adaptive_load/fake_plugins/fake_metrics_plugin/fake_metrics_plugin_test.cc +++ b/test/adaptive_load/fake_plugins/fake_metrics_plugin/fake_metrics_plugin_test.cc @@ -112,7 +112,7 @@ TEST(FakeMetricsPlugin, GetAllSupportedMetricNamesReturnsCorrectValues) { FakeMetricsPlugin metrics_plugin(config); EXPECT_THAT(metrics_plugin.GetAllSupportedMetricNames(), - ::testing::UnorderedElementsAre("metric1", "metric2")); + testing::UnorderedElementsAre("metric1", "metric2")); } TEST(MakeFakeMetricsPluginTypedExtensionConfig, SetsCorrectPluginName) { diff --git a/test/adaptive_load/fake_plugins/fake_step_controller/fake_step_controller_test.cc b/test/adaptive_load/fake_plugins/fake_step_controller/fake_step_controller_test.cc index 905c9fa02..da9648a7f 100644 --- a/test/adaptive_load/fake_plugins/fake_step_controller/fake_step_controller_test.cc +++ b/test/adaptive_load/fake_plugins/fake_step_controller/fake_step_controller_test.cc @@ -64,7 +64,7 @@ TEST(FakeStepControllerConfigFactory, ValidateConfigWithBadConfigProtoReturnsErr } TEST(FakeStepControllerConfigFactory, ValidateConfigWithAritificialValidationErrorReturnsError) { - const int kExpectedStatusCode = ::grpc::DATA_LOSS; + const int kExpectedStatusCode = grpc::DATA_LOSS; const std::string kExpectedStatusMessage = "artificial validation error"; FakeStepControllerConfig config; config.mutable_artificial_validation_failure()->set_code(kExpectedStatusCode); @@ -113,7 +113,7 @@ TEST(FakeStepController, GetCurrentCommandLineOptionsReturnsRpsFromConfig) { TEST(FakeStepController, GetCurrentCommandLineOptionsReturnsArtificialErrorImmediately) { FakeStepControllerConfig config; - const int kExpectedCode = ::grpc::DEADLINE_EXCEEDED; + const int kExpectedCode = grpc::DEADLINE_EXCEEDED; const std::string kExpectedMessage = "artificial input setting error"; config.mutable_artificial_input_setting_failure()->set_code(kExpectedCode); config.mutable_artificial_input_setting_failure()->set_message(kExpectedMessage); @@ -129,7 +129,7 @@ TEST(FakeStepController, GetCurrentCommandLineOptionsReturnsArtificialErrorImmed TEST(FakeStepController, GetCurrentCommandLineOptionsReturnsArtificialErrorAfterCountdown) { FakeStepControllerConfig config; - const int kExpectedCode = ::grpc::DEADLINE_EXCEEDED; + const int kExpectedCode = grpc::DEADLINE_EXCEEDED; const std::string kExpectedMessage = "artificial input setting error"; config.mutable_artificial_input_setting_failure()->set_code(kExpectedCode); config.mutable_artificial_input_setting_failure()->set_message(kExpectedMessage); diff --git a/test/adaptive_load/metrics_plugin_test.cc b/test/adaptive_load/metrics_plugin_test.cc index 415099bd2..8f8bce002 100644 --- a/test/adaptive_load/metrics_plugin_test.cc +++ b/test/adaptive_load/metrics_plugin_test.cc @@ -13,7 +13,7 @@ namespace Nighthawk { namespace { class NighthawkStatsEmulatedMetricsPluginFixture - : public ::testing::TestWithParam> {}; + : public testing::TestWithParam> {}; TEST_P(NighthawkStatsEmulatedMetricsPluginFixture, ComputesCorrectMetric) { NighthawkStatsEmulatedMetricsPlugin plugin = @@ -35,17 +35,17 @@ TEST_P(NighthawkStatsEmulatedMetricsPluginFixture, ComputesCorrectMetric) { INSTANTIATE_TEST_SUITE_P( NighthawkStatsEmulatedMetricsPluginValuesTests, NighthawkStatsEmulatedMetricsPluginFixture, - ::testing::Values(std::make_tuple("achieved-rps", 256), - std::make_tuple("attempted-rps", 1024), - std::make_tuple("latency-ns-max", 600.0), - std::make_tuple("latency-ns-mean", 500.0), - std::make_tuple("latency-ns-mean-plus-1stdev", 511.0), - std::make_tuple("latency-ns-mean-plus-2stdev", 522.0), - std::make_tuple("latency-ns-mean-plus-3stdev", 533.0), - std::make_tuple("latency-ns-min", 400.0), - std::make_tuple("latency-ns-pstdev", 11.0), - std::make_tuple("send-rate", 0.25), - std::make_tuple("success-rate", 0.125))); + testing::Values(std::make_tuple("achieved-rps", 256), + std::make_tuple("attempted-rps", 1024), + std::make_tuple("latency-ns-max", 600.0), + std::make_tuple("latency-ns-mean", 500.0), + std::make_tuple("latency-ns-mean-plus-1stdev", 511.0), + std::make_tuple("latency-ns-mean-plus-2stdev", 522.0), + std::make_tuple("latency-ns-mean-plus-3stdev", 533.0), + std::make_tuple("latency-ns-min", 400.0), + std::make_tuple("latency-ns-pstdev", 11.0), + std::make_tuple("send-rate", 0.25), + std::make_tuple("success-rate", 0.125))); TEST(NighthawkStatsEmulatedMetricsPlugin, ReturnsErrorIfGlobalResultMissing) { nighthawk::client::Output empty_output; @@ -221,11 +221,11 @@ TEST(NighthawkStatsEmulatedMetricsPlugin, ReturnsCorrectSupportedMetricNames) { NighthawkStatsEmulatedMetricsPlugin plugin = NighthawkStatsEmulatedMetricsPlugin({}); std::vector supported_metrics = plugin.GetAllSupportedMetricNames(); EXPECT_THAT(supported_metrics, - ::testing::ElementsAre("achieved-rps", "attempted-rps", "latency-ns-max", - "latency-ns-mean", "latency-ns-mean-plus-1stdev", - "latency-ns-mean-plus-2stdev", "latency-ns-mean-plus-3stdev", - "latency-ns-min", "latency-ns-pstdev", "send-rate", - "success-rate")); + testing::ElementsAre("achieved-rps", "attempted-rps", "latency-ns-max", + "latency-ns-mean", "latency-ns-mean-plus-1stdev", + "latency-ns-mean-plus-2stdev", "latency-ns-mean-plus-3stdev", + "latency-ns-min", "latency-ns-pstdev", "send-rate", + "success-rate")); } } // namespace diff --git a/test/adaptive_load/plugin_loader_test.cc b/test/adaptive_load/plugin_loader_test.cc index 8f92e815d..c5622e008 100644 --- a/test/adaptive_load/plugin_loader_test.cc +++ b/test/adaptive_load/plugin_loader_test.cc @@ -22,6 +22,7 @@ namespace Nighthawk { namespace { using ::envoy::config::core::v3::TypedExtensionConfig; +using ::testing::HasSubstr; // A special value that causes ValidateConfig to return an error when included in the config // protos of the fake plugins in this file. @@ -243,7 +244,7 @@ TEST(PluginUtilTest, ReturnsErrorFromInputVariableSetterConfigValidator) { config.set_name("nighthawk.test-input-variable-setter"); *config.mutable_typed_config() = CreateTypedConfigAny(kBadConfigThreshold); EXPECT_THAT(LoadInputVariableSetterPlugin(config).status().message(), - ::testing::HasSubstr("input validation failed")); + HasSubstr("input validation failed")); } TEST(PluginUtilTest, PropagatesConfigProtoToInputVariableSetter) { @@ -261,7 +262,7 @@ TEST(PluginUtilTest, ReturnsErrorWhenInputVariableSetterPluginNotFound) { config.set_name("nonexistent-input-variable-setter"); *config.mutable_typed_config() = CreateTypedConfigAny(0.0); EXPECT_THAT(LoadInputVariableSetterPlugin(config).status().message(), - ::testing::HasSubstr("Didn't find a registered implementation")); + HasSubstr("Didn't find a registered implementation")); } TEST(PluginUtilTest, CreatesCorrectScoringFunctionType) { @@ -278,7 +279,7 @@ TEST(PluginUtilTest, ReturnsErrorFromScoringFunctionConfigValidator) { config.set_name("nighthawk.test-scoring-function"); *config.mutable_typed_config() = CreateTypedConfigAny(kBadConfigThreshold); EXPECT_THAT(LoadScoringFunctionPlugin(config).status().message(), - ::testing::HasSubstr("input validation failed")); + HasSubstr("input validation failed")); } TEST(PluginUtilTest, PropagatesConfigProtoToScoringFunction) { @@ -296,7 +297,7 @@ TEST(PluginUtilTest, ReturnsErrorWhenScoringFunctionPluginNotFound) { config.set_name("nonexistent-scoring-function"); *config.mutable_typed_config() = CreateTypedConfigAny(0.0); EXPECT_THAT(LoadScoringFunctionPlugin(config).status().message(), - ::testing::HasSubstr("Didn't find a registered implementation")); + HasSubstr("Didn't find a registered implementation")); } TEST(PluginUtilTest, CreatesCorrectMetricsPluginType) { @@ -312,8 +313,7 @@ TEST(PluginUtilTest, ReturnsErrorFromMetricsPluginConfigValidator) { TypedExtensionConfig config; config.set_name("nighthawk.test-metrics-plugin"); *config.mutable_typed_config() = CreateTypedConfigAny(kBadConfigThreshold); - EXPECT_THAT(LoadMetricsPlugin(config).status().message(), - ::testing::HasSubstr("input validation failed")); + EXPECT_THAT(LoadMetricsPlugin(config).status().message(), HasSubstr("input validation failed")); } TEST(PluginUtilTest, PropagatesConfigProtoToMetricsPlugin) { @@ -331,7 +331,7 @@ TEST(PluginUtilTest, ReturnsErrorWhenMetricsPluginNotFound) { config.set_name("nonexistent-metrics-plugin"); *config.mutable_typed_config() = CreateTypedConfigAny(0.0); EXPECT_THAT(LoadMetricsPlugin(config).status().message(), - ::testing::HasSubstr("Didn't find a registered implementation")); + HasSubstr("Didn't find a registered implementation")); } TEST(PluginUtilTest, CreatesCorrectStepControllerType) { @@ -350,7 +350,7 @@ TEST(PluginUtilTest, ReturnsErrorFromStepControllerConfigValidator) { *config.mutable_typed_config() = CreateTypedConfigAny(kBadConfigThreshold); nighthawk::client::CommandLineOptions options_template; EXPECT_THAT(LoadStepControllerPlugin(config, options_template).status().message(), - ::testing::HasSubstr("input validation failed")); + HasSubstr("input validation failed")); } TEST(PluginUtilTest, PropagatesConfigProtoToStepController) { @@ -382,7 +382,7 @@ TEST(PluginUtilTest, ReturnsErrorWhenStepControllerPluginNotFound) { *config.mutable_typed_config() = CreateTypedConfigAny(0.0); nighthawk::client::CommandLineOptions options_template; EXPECT_THAT(LoadStepControllerPlugin(config, options_template).status().message(), - ::testing::HasSubstr("Didn't find a registered implementation")); + HasSubstr("Didn't find a registered implementation")); } } // namespace diff --git a/test/adaptive_load/scoring_function_test.cc b/test/adaptive_load/scoring_function_test.cc index 495a48291..c5eeb3a01 100644 --- a/test/adaptive_load/scoring_function_test.cc +++ b/test/adaptive_load/scoring_function_test.cc @@ -119,7 +119,7 @@ MakeBinaryConfigWithBothThresholds(double lower_threshold, double upper_threshol } class BinaryScoringFunctionFixture - : public ::testing::TestWithParam< + : public testing::TestWithParam< std::tuple> {}; @@ -133,23 +133,22 @@ TEST_P(BinaryScoringFunctionFixture, ComputesCorrectScore) { INSTANTIATE_TEST_SUITE_P( BinaryScoringFunctionTest, BinaryScoringFunctionFixture, - ::testing::ValuesIn( - std::vector>{ - {MakeBinaryConfigWithUpperThreshold(5.0), 4.0, 1.0}, - {MakeBinaryConfigWithUpperThreshold(5.0), 5.0, 1.0}, - {MakeBinaryConfigWithUpperThreshold(5.0), 6.0, -1.0}, - - {MakeBinaryConfigWithLowerThreshold(5.0), 4.0, -1.0}, - {MakeBinaryConfigWithLowerThreshold(5.0), 5.0, 1.0}, - {MakeBinaryConfigWithLowerThreshold(5.0), 6.0, 1.0}, - - {MakeBinaryConfigWithBothThresholds(5.0, 7.0), 6.0, 1.0}, - {MakeBinaryConfigWithBothThresholds(5.0, 7.0), 5.0, 1.0}, - {MakeBinaryConfigWithBothThresholds(5.0, 7.0), 7.0, 1.0}, - {MakeBinaryConfigWithBothThresholds(5.0, 7.0), 4.0, -1.0}, - {MakeBinaryConfigWithBothThresholds(5.0, 7.0), 8.0, -1.0}})); + testing::ValuesIn(std::vector>{ + {MakeBinaryConfigWithUpperThreshold(5.0), 4.0, 1.0}, + {MakeBinaryConfigWithUpperThreshold(5.0), 5.0, 1.0}, + {MakeBinaryConfigWithUpperThreshold(5.0), 6.0, -1.0}, + + {MakeBinaryConfigWithLowerThreshold(5.0), 4.0, -1.0}, + {MakeBinaryConfigWithLowerThreshold(5.0), 5.0, 1.0}, + {MakeBinaryConfigWithLowerThreshold(5.0), 6.0, 1.0}, + + {MakeBinaryConfigWithBothThresholds(5.0, 7.0), 6.0, 1.0}, + {MakeBinaryConfigWithBothThresholds(5.0, 7.0), 5.0, 1.0}, + {MakeBinaryConfigWithBothThresholds(5.0, 7.0), 7.0, 1.0}, + {MakeBinaryConfigWithBothThresholds(5.0, 7.0), 4.0, -1.0}, + {MakeBinaryConfigWithBothThresholds(5.0, 7.0), 8.0, -1.0}})); TEST(LinearScoringFunction, EvaluateMetricReturnsZeroForValueEqualToThreshold) { nighthawk::adaptive_load::LinearScoringFunctionConfig config; diff --git a/test/common/nighthawk_service_client_test.cc b/test/common/nighthawk_service_client_test.cc index f74e7d44f..ed3c0a34c 100644 --- a/test/common/nighthawk_service_client_test.cc +++ b/test/common/nighthawk_service_client_test.cc @@ -40,9 +40,9 @@ TEST(PerformNighthawkBenchmark, UsesSpecifiedCommandLineOptions) { EXPECT_CALL(*mock_reader_writer, Read(_)).WillOnce(Return(true)).WillOnce(Return(false)); // Capture the Nighthawk request PerformNighthawkBenchmark sends on the channel. EXPECT_CALL(*mock_reader_writer, Write(_, _)) - .WillOnce(::testing::DoAll(::testing::SaveArg<0>(&request), Return(true))); + .WillOnce(DoAll(SaveArg<0>(&request), Return(true))); EXPECT_CALL(*mock_reader_writer, WritesDone()).WillOnce(Return(true)); - EXPECT_CALL(*mock_reader_writer, Finish()).WillOnce(Return(::grpc::Status::OK)); + EXPECT_CALL(*mock_reader_writer, Finish()).WillOnce(Return(grpc::Status::OK)); return mock_reader_writer; }); @@ -71,7 +71,7 @@ TEST(PerformNighthawkBenchmark, ReturnsNighthawkResponseSuccessfully) { .WillOnce(Return(false)); EXPECT_CALL(*mock_reader_writer, Write(_, _)).WillOnce(Return(true)); EXPECT_CALL(*mock_reader_writer, WritesDone()).WillOnce(Return(true)); - EXPECT_CALL(*mock_reader_writer, Finish()).WillOnce(Return(::grpc::Status::OK)); + EXPECT_CALL(*mock_reader_writer, Finish()).WillOnce(Return(grpc::Status::OK)); return mock_reader_writer; }); @@ -157,8 +157,7 @@ TEST(PerformNighthawkBenchmark, PropagatesErrorIfNighthawkServiceGrpcStreamClose EXPECT_CALL(*mock_reader_writer, Write(_, _)).WillOnce(Return(true)); EXPECT_CALL(*mock_reader_writer, WritesDone()).WillOnce(Return(true)); EXPECT_CALL(*mock_reader_writer, Finish()) - .WillOnce( - Return(::grpc::Status(::grpc::PERMISSION_DENIED, "Finish failure status message"))); + .WillOnce(Return(grpc::Status(grpc::PERMISSION_DENIED, "Finish failure status message"))); return mock_reader_writer; }); diff --git a/test/distributor/BUILD b/test/distributor/BUILD new file mode 100644 index 000000000..ab4b6f45d --- /dev/null +++ b/test/distributor/BUILD @@ -0,0 +1,19 @@ +load( + "@envoy//bazel:envoy_build_system.bzl", + "envoy_cc_test", + "envoy_package", +) + +licenses(["notice"]) # Apache 2 + +envoy_package() + +envoy_cc_test( + name = "nighthawk_distributor_client_test", + srcs = ["nighthawk_distributor_client_test.cc"], + repository = "@envoy", + deps = [ + "//source/distributor:nighthawk_distributor_client_impl", + "@com_github_grpc_grpc//:grpc++_test", + ], +) diff --git a/test/distributor/nighthawk_distributor_client_test.cc b/test/distributor/nighthawk_distributor_client_test.cc new file mode 100644 index 000000000..8912faf9c --- /dev/null +++ b/test/distributor/nighthawk_distributor_client_test.cc @@ -0,0 +1,185 @@ +#include "external/envoy/source/common/protobuf/protobuf.h" + +#include "api/client/options.pb.h" +#include "api/distributor/distributor.grpc.pb.h" +#include "api/distributor/distributor_mock.grpc.pb.h" + +#include "distributor/nighthawk_distributor_client_impl.h" + +#include "grpcpp/test/mock_stream.h" + +#include "gmock/gmock.h" +#include "gtest/gtest.h" + +namespace Nighthawk { + +namespace { + +using ::Envoy::Protobuf::util::MessageDifferencer; +using ::nighthawk::DistributedRequest; +using ::nighthawk::DistributedResponse; +using ::nighthawk::client::CommandLineOptions; +using ::testing::_; +using ::testing::DoAll; +using ::testing::HasSubstr; +using ::testing::Return; +using ::testing::SaveArg; +using ::testing::SetArgPointee; + +TEST(DistributedRequest, UsesSpecifiedCommandLineOptions) { + const int kExpectedRps = 456; + DistributedRequest request; + nighthawk::MockNighthawkDistributorStub mock_nighthawk_service_stub; + // Configure the mock Nighthawk Service stub to return an inner mock channel when the code under + // test requests a channel. Set call expectations on the inner mock channel. + EXPECT_CALL(mock_nighthawk_service_stub, DistributedRequestStreamRaw) + .WillOnce([&request](grpc::ClientContext*) { + auto* mock_reader_writer = + new grpc::testing::MockClientReaderWriter(); + // DistributedRequest currently expects Read to return true exactly once. + EXPECT_CALL(*mock_reader_writer, Read(_)).WillOnce(Return(true)).WillOnce(Return(false)); + // Capture the Nighthawk request DistributedRequest sends on the channel. + EXPECT_CALL(*mock_reader_writer, Write(_, _)) + .WillOnce(DoAll(SaveArg<0>(&request), Return(true))); + EXPECT_CALL(*mock_reader_writer, WritesDone()).WillOnce(Return(true)); + EXPECT_CALL(*mock_reader_writer, Finish()).WillOnce(Return(::grpc::Status::OK)); + return mock_reader_writer; + }); + + nighthawk::DistributedRequest distributed_request; + nighthawk::client::ExecutionRequest execution_request; + nighthawk::client::StartRequest start_request; + CommandLineOptions command_line_options; + command_line_options.mutable_requests_per_second()->set_value(kExpectedRps); + *(start_request.mutable_options()) = command_line_options; + *(execution_request.mutable_start_request()) = start_request; + *(distributed_request.mutable_execution_request()) = execution_request; + NighthawkDistributorClientImpl client; + absl::StatusOr distributed_response_or = + client.DistributedRequest(mock_nighthawk_service_stub, distributed_request); + EXPECT_TRUE(distributed_response_or.ok()); + EXPECT_EQ(request.execution_request().start_request().options().requests_per_second().value(), + kExpectedRps); +} + +TEST(DistributedRequest, ReturnsNighthawkResponseSuccessfully) { + DistributedResponse expected_response; + nighthawk::MockNighthawkDistributorStub mock_nighthawk_service_stub; + // Configure the mock Nighthawk Service stub to return an inner mock channel when the code under + // test requests a channel. Set call expectations on the inner mock channel. + EXPECT_CALL(mock_nighthawk_service_stub, DistributedRequestStreamRaw) + .WillOnce([&expected_response](grpc::ClientContext*) { + auto* mock_reader_writer = + new grpc::testing::MockClientReaderWriter(); + // DistributedRequest currently expects Read to return true exactly once. + // Capture the gRPC response proto as it is written to the output parameter. + EXPECT_CALL(*mock_reader_writer, Read(_)) + .WillOnce(DoAll(SetArgPointee<0>(expected_response), Return(true))) + .WillOnce(Return(false)); + EXPECT_CALL(*mock_reader_writer, Write(_, _)).WillOnce(Return(true)); + EXPECT_CALL(*mock_reader_writer, WritesDone()).WillOnce(Return(true)); + EXPECT_CALL(*mock_reader_writer, Finish()).WillOnce(Return(::grpc::Status::OK)); + return mock_reader_writer; + }); + + NighthawkDistributorClientImpl client; + absl::StatusOr response_or = + client.DistributedRequest(mock_nighthawk_service_stub, nighthawk::DistributedRequest()); + EXPECT_TRUE(response_or.ok()); + DistributedResponse actual_response = response_or.value(); + EXPECT_TRUE(MessageDifferencer::Equivalent(actual_response, expected_response)); + EXPECT_EQ(actual_response.DebugString(), expected_response.DebugString()); +} + +TEST(DistributedRequest, ReturnsErrorIfNighthawkServiceDoesNotSendResponse) { + nighthawk::MockNighthawkDistributorStub mock_nighthawk_service_stub; + // Configure the mock Nighthawk Service stub to return an inner mock channel when the code under + // test requests a channel. Set call expectations on the inner mock channel. + EXPECT_CALL(mock_nighthawk_service_stub, DistributedRequestStreamRaw) + .WillOnce([](grpc::ClientContext*) { + auto* mock_reader_writer = + new grpc::testing::MockClientReaderWriter(); + EXPECT_CALL(*mock_reader_writer, Read(_)).WillOnce(Return(false)); + EXPECT_CALL(*mock_reader_writer, Write(_, _)).WillOnce(Return(true)); + EXPECT_CALL(*mock_reader_writer, WritesDone()).WillOnce(Return(true)); + return mock_reader_writer; + }); + + NighthawkDistributorClientImpl client; + absl::StatusOr response_or = + client.DistributedRequest(mock_nighthawk_service_stub, nighthawk::DistributedRequest()); + ASSERT_FALSE(response_or.ok()); + EXPECT_EQ(response_or.status().code(), absl::StatusCode::kInternal); + EXPECT_THAT(response_or.status().message(), + HasSubstr("Distributor Service did not send a gRPC response.")); +} + +TEST(DistributedRequest, ReturnsErrorIfNighthawkServiceWriteFails) { + nighthawk::MockNighthawkDistributorStub mock_nighthawk_service_stub; + // Configure the mock Nighthawk Service stub to return an inner mock channel when the code under + // test requests a channel. Set call expectations on the inner mock channel. + EXPECT_CALL(mock_nighthawk_service_stub, DistributedRequestStreamRaw) + .WillOnce([](grpc::ClientContext*) { + auto* mock_reader_writer = + new grpc::testing::MockClientReaderWriter(); + EXPECT_CALL(*mock_reader_writer, Write(_, _)).WillOnce(Return(false)); + return mock_reader_writer; + }); + + NighthawkDistributorClientImpl client; + absl::StatusOr response_or = + client.DistributedRequest(mock_nighthawk_service_stub, nighthawk::DistributedRequest()); + ASSERT_FALSE(response_or.ok()); + EXPECT_EQ(response_or.status().code(), absl::StatusCode::kUnavailable); + EXPECT_THAT(response_or.status().message(), HasSubstr("Failed to write")); +} + +TEST(DistributedRequest, ReturnsErrorIfNighthawkServiceWritesDoneFails) { + nighthawk::MockNighthawkDistributorStub mock_nighthawk_service_stub; + // Configure the mock Nighthawk Service stub to return an inner mock channel when the code under + // test requests a channel. Set call expectations on the inner mock channel. + EXPECT_CALL(mock_nighthawk_service_stub, DistributedRequestStreamRaw) + .WillOnce([](grpc::ClientContext*) { + auto* mock_reader_writer = + new grpc::testing::MockClientReaderWriter(); + EXPECT_CALL(*mock_reader_writer, Write(_, _)).WillOnce(Return(true)); + EXPECT_CALL(*mock_reader_writer, WritesDone()).WillOnce(Return(false)); + return mock_reader_writer; + }); + + NighthawkDistributorClientImpl client; + absl::StatusOr response_or = + client.DistributedRequest(mock_nighthawk_service_stub, nighthawk::DistributedRequest()); + ASSERT_FALSE(response_or.ok()); + EXPECT_EQ(response_or.status().code(), absl::StatusCode::kInternal); + EXPECT_THAT(response_or.status().message(), HasSubstr("WritesDone() failed")); +} + +TEST(DistributedRequest, PropagatesErrorIfNighthawkServiceGrpcStreamClosesAbnormally) { + nighthawk::MockNighthawkDistributorStub mock_nighthawk_service_stub; + // Configure the mock Nighthawk Service stub to return an inner mock channel when the code under + // test requests a channel. Set call expectations on the inner mock channel. + EXPECT_CALL(mock_nighthawk_service_stub, DistributedRequestStreamRaw) + .WillOnce([](grpc::ClientContext*) { + auto* mock_reader_writer = + new grpc::testing::MockClientReaderWriter(); + // DistributedRequest currently expects Read to return true exactly once. + EXPECT_CALL(*mock_reader_writer, Read(_)).WillOnce(Return(true)).WillOnce(Return(false)); + EXPECT_CALL(*mock_reader_writer, Write(_, _)).WillOnce(Return(true)); + EXPECT_CALL(*mock_reader_writer, WritesDone()).WillOnce(Return(true)); + EXPECT_CALL(*mock_reader_writer, Finish()) + .WillOnce( + Return(::grpc::Status(::grpc::PERMISSION_DENIED, "Finish failure status message"))); + return mock_reader_writer; + }); + + NighthawkDistributorClientImpl client; + absl::StatusOr response_or = + client.DistributedRequest(mock_nighthawk_service_stub, nighthawk::DistributedRequest()); + ASSERT_FALSE(response_or.ok()); + EXPECT_EQ(response_or.status().code(), absl::StatusCode::kPermissionDenied); + EXPECT_THAT(response_or.status().message(), HasSubstr("Finish failure status message")); +} + +} // namespace +} // namespace Nighthawk diff --git a/test/integration/BUILD b/test/integration/BUILD index c9cd5ad68..1290e6090 100644 --- a/test/integration/BUILD +++ b/test/integration/BUILD @@ -13,7 +13,6 @@ py_library( name = "integration_test_base", data = [ "configurations/nighthawk_http_origin.yaml", - "configurations/nighthawk_http_origin_envoy_deprecated_v2_api.yaml", "configurations/nighthawk_https_origin.yaml", "configurations/nighthawk_track_timings.yaml", "configurations/sni_origin.yaml", diff --git a/test/integration/configurations/nighthawk_http_origin_envoy_deprecated_v2_api.yaml b/test/integration/configurations/nighthawk_http_origin_envoy_deprecated_v2_api.yaml deleted file mode 100644 index 5e07954c7..000000000 --- a/test/integration/configurations/nighthawk_http_origin_envoy_deprecated_v2_api.yaml +++ /dev/null @@ -1,39 +0,0 @@ -# This file is intentionally using the v2 api: it is used to test support for that. -admin: - access_log_path: $tmpdir/nighthawk-test-server-admin-access.log - profile_path: $tmpdir/nighthawk-test-server.prof - address: - socket_address: { address: $server_ip, port_value: 0 } -static_resources: - listeners: - - address: - socket_address: - address: $server_ip - port_value: 0 - filter_chains: - - filters: - - name: envoy.http_connection_manager - config: - generate_request_id: false - codec_type: auto - stat_prefix: ingress_http - route_config: - name: local_route - virtual_hosts: - - name: service - domains: - - "*" - http_filters: - - name: test-server - config: - response_body_size: 10 - v3_response_headers: - - { header: { key: "x-nh", value: "1"}} - - name: envoy.router - config: - dynamic_stats: false -layered_runtime: - layers: - - name: static_layer - static_layer: - envoy.reloadable_features.enable_deprecated_v2_api: true diff --git a/test/integration/integration_test_fixtures.py b/test/integration/integration_test_fixtures.py index 8e29a1940..a9c931240 100644 --- a/test/integration/integration_test_fixtures.py +++ b/test/integration/integration_test_fixtures.py @@ -67,7 +67,7 @@ class IntegrationTestBase(): about the currently executing test case. """ - def __init__(self, request, server_config, backend_count=1, bootstrap_version_arg=None): + def __init__(self, request, server_config, backend_count=1): """Initialize the IntegrationTestBase instance. Args: @@ -76,7 +76,6 @@ def __init__(self, request, server_config, backend_count=1, bootstrap_version_ar about the currently executing test case. server_config: path to the server configuration backend_count: number of Nighthawk Test Server backends to run, to allow testing MultiTarget mode - bootstrap_version_arg: An optional int, specify a bootstrap cli argument value for the test server binary. If None is specified, no bootstrap cli argment will be passed. """ super(IntegrationTestBase, self).__init__() self.request = request @@ -97,7 +96,6 @@ def __init__(self, request, server_config, backend_count=1, bootstrap_version_ar self._test_servers = [] self._backend_count = backend_count self._test_id = "" - self._bootstrap_version_arg = bootstrap_version_arg # TODO(oschaaf): For the NH test server, add a way to let it determine a port by itself and pull that # out. @@ -165,8 +163,7 @@ def _tryStartTestServers(self): self.ip_version, self.request, parameters=self.parameters, - tag=self.tag, - bootstrap_version_arg=self._bootstrap_version_arg) + tag=self.tag) if not test_server.start(): return False self._test_servers.append(test_server) @@ -322,25 +319,6 @@ def getTestServerRootUri(self): return super(HttpIntegrationTestBase, self).getTestServerRootUri(False) -class HttpIntegrationTestBaseWithEnvoyDeprecatedV2Bootstrap(IntegrationTestBase): - """Base for running plain http tests against the Nighthawk test server. - - NOTE: any script that consumes derivations of this, needs to also explicitly - import server_config, to avoid errors caused by the server_config not being found - by pytest. - """ - - def __init__(self, request, server_config): - """See base class.""" - super(HttpIntegrationTestBaseWithEnvoyDeprecatedV2Bootstrap, - self).__init__(request, server_config, bootstrap_version_arg=2) - - def getTestServerRootUri(self): - """See base class.""" - return super(HttpIntegrationTestBaseWithEnvoyDeprecatedV2Bootstrap, - self).getTestServerRootUri(False) - - class MultiServerHttpIntegrationTestBase(IntegrationTestBase): """Base for running plain http tests against multiple Nighthawk test servers.""" @@ -420,19 +398,6 @@ def http_test_server_fixture(request, server_config, caplog): f.tearDown(caplog) -@pytest.fixture(params=determineIpVersionsFromEnvironment()) -def http_test_server_fixture_envoy_deprecated_v2_api(request, server_config, caplog): - """Fixture for setting up a test environment with http server configuration that uses v2 configuration. - - Yields: - HttpIntegrationTestBaseWithEnvoyDeprecatedV2Bootstrap: A fully set up instance. Tear down will happen automatically. - """ - f = HttpIntegrationTestBaseWithEnvoyDeprecatedV2Bootstrap(request, server_config) - f.setUp() - yield f - f.tearDown(caplog) - - @pytest.fixture(params=determineIpVersionsFromEnvironment()) def https_test_server_fixture(request, server_config, caplog): """Fixture for setting up a test environment with the stock https server configuration. diff --git a/test/integration/nighthawk_test_server.py b/test/integration/nighthawk_test_server.py index 3318afdc2..f073980de 100644 --- a/test/integration/nighthawk_test_server.py +++ b/test/integration/nighthawk_test_server.py @@ -86,6 +86,7 @@ class _TestCaseWarnErrorIgnoreList( ( # TODO(#582): Identify these and file issues or add explanation as necessary. "Unable to use runtime singleton for feature envoy.http.headermap.lazy_map_min_size", + "Unable to use runtime singleton for feature envoy.reloadable_features.header_map_correctly_coalesce_cookies", "Using deprecated extension name 'envoy.listener.tls_inspector' for 'envoy.filters.listener.tls_inspector'.", "there is no configured limit to the number of allowed active connections. Set a limit via the runtime key overload.global_downstream_max_connections", @@ -112,16 +113,8 @@ class TestServerBase(object): tmpdir: String, indicates the location used to store outputs like logs. """ - def __init__(self, - server_binary_path, - config_template_path, - server_ip, - ip_version, - request, - server_binary_config_path_arg, - parameters, - tag, - bootstrap_version_arg=None): + def __init__(self, server_binary_path, config_template_path, server_ip, ip_version, request, + server_binary_config_path_arg, parameters, tag): """Initialize a TestServerBase instance. Args: @@ -133,7 +126,6 @@ def __init__(self, server_binary_config_path_arg (str): Specify the name of the CLI argument the test server binary uses to accept a configuration path. parameters (dict): Supply to provide configuration template parameter replacement values. tag (str): Supply to get recognizeable output locations. - bootstrap_version_arg (int, optional): specify a bootstrap cli argument value for the test server binary. """ assert ip_version != IpVersion.UNKNOWN self.ip_version = ip_version @@ -153,7 +145,6 @@ def __init__(self, self._parameterized_config_path = "" self._instance_id = str(random.randint(1, 1024 * 1024 * 1024)) self._server_binary_config_path_arg = server_binary_config_path_arg - self._bootstrap_version_arg = bootstrap_version_arg self._prepareForExecution() self._request = request @@ -194,8 +185,6 @@ def _serverThreadRunner(self): self._parameterized_config_path, "-l", "debug", "--base-id", self._instance_id, "--admin-address-path", self._admin_address_path, "--concurrency", "1" ] - if self._bootstrap_version_arg is not None: - args = args + ["--bootstrap-version", str(self._bootstrap_version_arg)] logging.info("Test server popen() args: %s" % str.join(" ", args)) self._server_process = subprocess.Popen(args, stdout=subprocess.PIPE, stderr=subprocess.PIPE) @@ -312,8 +301,7 @@ def __init__(self, ip_version, request, parameters=dict(), - tag="", - bootstrap_version_arg=None): + tag=""): """Initialize a NighthawkTestServer instance. Args: @@ -324,17 +312,9 @@ def __init__(self, request: The pytest `request` fixture used to determin information about the currently executed test. parameters (dictionary, optional): Directionary with replacement values for substition purposes in the server configuration template. Defaults to dict(). tag (str, optional): Tags. Supply this to get recognizeable output locations. Defaults to "". - bootstrap_version_arg (String, optional): Specify a cli argument value for --bootstrap-version when running the server. """ - super(NighthawkTestServer, self).__init__(server_binary_path, - config_template_path, - server_ip, - ip_version, - request, - "--config-path", - parameters, - tag, - bootstrap_version_arg=bootstrap_version_arg) + super(NighthawkTestServer, self).__init__(server_binary_path, config_template_path, server_ip, + ip_version, request, "--config-path", parameters, tag) def getCliVersionString(self): """Get the version string as written to the output by the CLI.""" diff --git a/test/integration/test_integration_basics.py b/test/integration/test_integration_basics.py index ea3bf4e43..78b07fdc2 100644 --- a/test/integration/test_integration_basics.py +++ b/test/integration/test_integration_basics.py @@ -11,9 +11,8 @@ from test.integration.common import IpVersion from test.integration.integration_test_fixtures import ( - http_test_server_fixture, http_test_server_fixture_envoy_deprecated_v2_api, - https_test_server_fixture, https_test_server_fixture, multi_http_test_server_fixture, - multi_https_test_server_fixture, server_config) + http_test_server_fixture, https_test_server_fixture, https_test_server_fixture, + multi_http_test_server_fixture, multi_https_test_server_fixture, server_config) from test.integration import asserts from test.integration import utility @@ -36,9 +35,7 @@ def test_http_h1(http_test_server_fixture): asserts.assertCounterEqual(counters, "upstream_cx_http1_total", 1) asserts.assertCounterEqual(counters, "upstream_cx_rx_bytes_total", 3400) asserts.assertCounterEqual(counters, "upstream_cx_total", 1) - asserts.assertCounterEqual( - counters, "upstream_cx_tx_bytes_total", - 1375 if http_test_server_fixture.ip_version == IpVersion.IPV6 else 1450) + asserts.assertCounterGreaterEqual(counters, "upstream_cx_tx_bytes_total", 500) asserts.assertCounterEqual(counters, "upstream_rq_pending_total", 1) asserts.assertCounterEqual(counters, "upstream_rq_total", 25) asserts.assertCounterEqual(counters, "default.total_match_count", 1) @@ -68,48 +65,6 @@ def test_http_h1(http_test_server_fixture): asserts.assertEqual(len(counters), 12) -@pytest.mark.parametrize('server_config', [ - "nighthawk/test/integration/configurations/nighthawk_http_origin_envoy_deprecated_v2_api.yaml" -]) -def test_nighthawk_test_server_envoy_deprecated_v2_api( - http_test_server_fixture_envoy_deprecated_v2_api): - """Test that the v2 configuration works for the test server.""" - parsed_json, _ = http_test_server_fixture_envoy_deprecated_v2_api.runNighthawkClient([ - http_test_server_fixture_envoy_deprecated_v2_api.getTestServerRootUri(), "--duration", "100", - "--termination-predicate", "benchmark.http_2xx:24" - ]) - - counters = http_test_server_fixture_envoy_deprecated_v2_api.getNighthawkCounterMapFromJson( - parsed_json) - asserts.assertCounterEqual(counters, "benchmark.http_2xx", 25) - - -def test_nighthawk_client_v2_api_explicitly_set(http_test_server_fixture): - """Test that the v2 api works when requested to.""" - parsed_json, _ = http_test_server_fixture.runNighthawkClient([ - http_test_server_fixture.getTestServerRootUri(), "--duration", "100", - "--termination-predicate", "benchmark.pool_connection_failure:0", "--failure-predicate", - "foo:1", "--allow-envoy-deprecated-v2-api", "--transport-socket", - "{name:\"envoy.transport_sockets.tls\",typed_config:{\"@type\":\"type.googleapis.com/envoy.api.v2.auth.UpstreamTlsContext\",\"common_tls_context\":{}}}" - ]) - - counters = http_test_server_fixture.getNighthawkCounterMapFromJson(parsed_json) - asserts.assertCounterEqual(counters, "benchmark.pool_connection_failure", 1) - - -# TODO(oschaaf): This ought to work after the Envoy update. -def DISABLED_test_nighthawk_client_v2_api_breaks_by_default(http_test_server_fixture): - """Test that the v2 api breaks us when it's not explicitly requested.""" - _, _ = http_test_server_fixture.runNighthawkClient([ - http_test_server_fixture.getTestServerRootUri(), "--duration", "100", - "--termination-predicate", "benchmark.pool_connection_failure:0", "--failure-predicate", - "foo:1", "--transport-socket", - "{name:\"envoy.transport_sockets.tls\",typed_config:{\"@type\":\"type.googleapis.com/envoy.api.v2.auth.UpstreamTlsContext\",\"common_tls_context\":{}}}" - ], - expect_failure=True, - as_json=False) - - def _mini_stress_test(fixture, args): # run a test with more rps then we can handle, and a very small client-side queue. # we should observe both lots of successfull requests as well as time spend in blocking mode., @@ -262,9 +217,7 @@ def test_https_h1(https_test_server_fixture): asserts.assertCounterEqual(counters, "upstream_cx_http1_total", 1) asserts.assertCounterEqual(counters, "upstream_cx_rx_bytes_total", 3400) asserts.assertCounterEqual(counters, "upstream_cx_total", 1) - asserts.assertCounterEqual( - counters, "upstream_cx_tx_bytes_total", - 1375 if https_test_server_fixture.ip_version == IpVersion.IPV6 else 1450) + asserts.assertCounterGreaterEqual(counters, "upstream_cx_tx_bytes_total", 500) asserts.assertCounterEqual(counters, "upstream_rq_pending_total", 1) asserts.assertCounterEqual(counters, "upstream_rq_total", 25) asserts.assertCounterEqual(counters, "ssl.ciphers.ECDHE-RSA-AES128-GCM-SHA256", 1) diff --git a/test/mocks/client/mock_options.h b/test/mocks/client/mock_options.h index a6e85d42c..04fc35ec0 100644 --- a/test/mocks/client/mock_options.h +++ b/test/mocks/client/mock_options.h @@ -57,7 +57,6 @@ class MockOptions : public Options { MOCK_CONST_METHOD0(statsSinks, std::vector()); MOCK_CONST_METHOD0(statsFlushInterval, uint32_t()); MOCK_CONST_METHOD0(responseHeaderWithLatencyInput, std::string()); - MOCK_CONST_METHOD0(allowEnvoyDeprecatedV2Api, bool()); MOCK_CONST_METHOD0(scheduled_start, absl::optional()); }; diff --git a/test/mocks/sink/BUILD b/test/mocks/sink/BUILD new file mode 100644 index 000000000..5cb644514 --- /dev/null +++ b/test/mocks/sink/BUILD @@ -0,0 +1,19 @@ +load( + "@envoy//bazel:envoy_build_system.bzl", + "envoy_cc_mock", + "envoy_package", +) + +licenses(["notice"]) # Apache 2 + +envoy_package() + +envoy_cc_mock( + name = "mock_sink", + srcs = ["mock_sink.cc"], + hdrs = ["mock_sink.h"], + repository = "@envoy", + deps = [ + "//include/nighthawk/sink:sink_lib", + ], +) diff --git a/test/mocks/sink/mock_sink.cc b/test/mocks/sink/mock_sink.cc new file mode 100644 index 000000000..b6761395a --- /dev/null +++ b/test/mocks/sink/mock_sink.cc @@ -0,0 +1,7 @@ +#include "test/mocks/sink/mock_sink.h" + +namespace Nighthawk { + +MockSink::MockSink() = default; + +} // namespace Nighthawk diff --git a/test/mocks/sink/mock_sink.h b/test/mocks/sink/mock_sink.h new file mode 100644 index 000000000..3d57d6f14 --- /dev/null +++ b/test/mocks/sink/mock_sink.h @@ -0,0 +1,19 @@ +#pragma once + +#include "nighthawk/sink/sink.h" + +#include "gmock/gmock.h" + +namespace Nighthawk { + +class MockSink : public Sink { +public: + MockSink(); + MOCK_CONST_METHOD1(StoreExecutionResultPiece, + absl::Status(const nighthawk::client::ExecutionResponse&)); + MOCK_CONST_METHOD1( + LoadExecutionResult, + absl::StatusOr>(absl::string_view)); +}; + +} // namespace Nighthawk diff --git a/test/options_test.cc b/test/options_test.cc index a1f6c143b..c0b847fd1 100644 --- a/test/options_test.cc +++ b/test/options_test.cc @@ -118,7 +118,7 @@ TEST_F(OptionsImplTest, AlmostAll) { "--experimental-h2-use-multiple-connections " "--experimental-h1-connection-reuse-strategy lru --label label1 --label label2 {} " "--simple-warmup --stats-sinks {} --stats-sinks {} --stats-flush-interval 10 " - "--latency-response-header-name zz --allow-envoy-deprecated-v2-api", + "--latency-response-header-name zz", client_name_, "{name:\"envoy.transport_sockets.tls\"," "typed_config:{\"@type\":\"type.googleapis.com/" @@ -193,7 +193,6 @@ TEST_F(OptionsImplTest, AlmostAll) { "183412668: \"envoy.config.metrics.v2.StatsSink\"\n", options->statsSinks()[1].DebugString()); EXPECT_EQ("zz", options->responseHeaderWithLatencyInput()); - EXPECT_TRUE(options->allowEnvoyDeprecatedV2Api()); // Check that our conversion to CommandLineOptionsPtr makes sense. CommandLineOptionsPtr cmd = options->toCommandLineOptions(); @@ -252,8 +251,6 @@ TEST_F(OptionsImplTest, AlmostAll) { EXPECT_TRUE(util(cmd->stats_sinks(0), options->statsSinks()[0])); EXPECT_TRUE(util(cmd->stats_sinks(1), options->statsSinks()[1])); EXPECT_EQ(cmd->latency_response_header_name().value(), options->responseHeaderWithLatencyInput()); - ASSERT_TRUE(cmd->has_allow_envoy_deprecated_v2_api()); - EXPECT_EQ(cmd->allow_envoy_deprecated_v2_api().value(), options->allowEnvoyDeprecatedV2Api()); // TODO(#433) Here and below, replace comparisons once we choose a proto diff. OptionsImpl options_from_proto(*cmd); std::string s1 = Envoy::MessageUtil::getYamlStringFromMessage( @@ -354,7 +351,7 @@ std::vector RequestSourcePluginJsons() { } INSTANTIATE_TEST_SUITE_P(HappyPathRequestSourceConfigJsonSuccessfullyTranslatesIntoOptions, RequestSourcePluginTestFixture, - ::testing::ValuesIn(RequestSourcePluginJsons())); + testing::ValuesIn(RequestSourcePluginJsons())); // This test covers --RequestSourcePlugin, which can't be tested at the same time as --RequestSource // and some other options. This is the test for the inlineoptionslistplugin. @@ -597,22 +594,6 @@ TEST_F(OptionsImplTest, PrefetchConnectionsFlag) { MalformedArgvException, "Couldn't find match for argument"); } -TEST_F(OptionsImplTest, AllowEnvoyDeprecatedV2ApiFlag) { - EXPECT_FALSE(TestUtility::createOptionsImpl(fmt::format("{} {}", client_name_, good_test_uri_)) - ->allowEnvoyDeprecatedV2Api()); - EXPECT_TRUE(TestUtility::createOptionsImpl(fmt::format("{} --allow-envoy-deprecated-v2-api {}", - client_name_, good_test_uri_)) - ->allowEnvoyDeprecatedV2Api()); - EXPECT_THROW_WITH_REGEX( - TestUtility::createOptionsImpl( - fmt::format("{} --allow-envoy-deprecated-v2-api 0 {}", client_name_, good_test_uri_)), - MalformedArgvException, "Couldn't find match for argument"); - EXPECT_THROW_WITH_REGEX( - TestUtility::createOptionsImpl( - fmt::format("{} --allow-envoy-deprecated-v2-api true {}", client_name_, good_test_uri_)), - MalformedArgvException, "Couldn't find match for argument"); -} - // Test --concurrency, which is a bit special. It's an int option, which also accepts 'auto' as // a value. We need to implement some stuff ourselves to get this to work, hence we don't run it // through the OptionsImplIntTest. diff --git a/test/output_transform_main_test.cc b/test/output_transform_main_test.cc index 2e5c618c3..6ae792bba 100644 --- a/test/output_transform_main_test.cc +++ b/test/output_transform_main_test.cc @@ -9,6 +9,7 @@ #include "client/output_formatter_impl.h" #include "client/output_transform_main.h" +#include "absl/strings/match.h" #include "gtest/gtest.h" using namespace testing; @@ -59,7 +60,7 @@ TEST_F(OutputTransformMainTest, HappyFlowForAllOutputFormats) { for (const std::string& output_format : OutputFormatterImpl::getLowerCaseOutputFormats()) { std::vector argv = {"foo", "--output-format", output_format.c_str()}; nighthawk::client::Output output; - if (output_format.find("fortio") != std::string::npos) { + if (absl::StrContains(output_format, "fortio")) { // The fortio output formatter mandates at least a single global result or it throws. output.add_results()->set_name("global"); } diff --git a/test/process_test.cc b/test/process_test.cc index 4296ef770..7d1191790 100644 --- a/test/process_test.cc +++ b/test/process_test.cc @@ -9,7 +9,6 @@ #include "external/envoy/test/test_common/registry.h" #include "external/envoy/test/test_common/simulated_time_system.h" #include "external/envoy/test/test_common/utility.h" -#include "external/envoy_api/envoy/config/bootstrap/v3/bootstrap.pb.h" #include "common/uri_impl.h" @@ -181,38 +180,6 @@ TEST_P(ProcessTest, NoFlushWhenCancelExecutionBeforeLoadTestBegin) { EXPECT_EQ(numFlushes, 0); } -TEST(RuntimeConfiguration, allowEnvoyDeprecatedV2Api) { - envoy::config::bootstrap::v3::Bootstrap bootstrap; - EXPECT_EQ(bootstrap.DebugString(), ""); - ProcessImpl::allowEnvoyDeprecatedV2Api(bootstrap); - std::cerr << bootstrap.DebugString() << std::endl; - EXPECT_EQ(bootstrap.DebugString(), R"EOF(layered_runtime { - layers { - name: "admin layer" - admin_layer { - } - } - layers { - name: "static_layer" - static_layer { - fields { - key: "envoy.reloadable_features.allow_prefetch" - value { - string_value: "true" - } - } - fields { - key: "envoy.reloadable_features.enable_deprecated_v2_api" - value { - string_value: "true" - } - } - } - } -} -)EOF"); -} - /** * Fixture for executing the Nighthawk process with simulated time. */ diff --git a/test/server/http_filter_base_test.cc b/test/server/http_filter_base_test.cc index adf90896f..0a5ef3ceb 100644 --- a/test/server/http_filter_base_test.cc +++ b/test/server/http_filter_base_test.cc @@ -43,21 +43,21 @@ class HttpFilterBaseIntegrationTest INSTANTIATE_TEST_SUITE_P( IpVersions, HttpFilterBaseIntegrationTest, - ::testing::Combine(testing::ValuesIn(Envoy::TestEnvironment::getIpVersionsForTest()), - testing::ValuesIn({absl::string_view(R"EOF( + testing::Combine(testing::ValuesIn(Envoy::TestEnvironment::getIpVersionsForTest()), + testing::ValuesIn({absl::string_view(R"EOF( name: time-tracking typed_config: "@type": type.googleapis.com/nighthawk.server.ResponseOptions emit_previous_request_delta_in_response_header: "foo" )EOF"), - absl::string_view(R"EOF( + absl::string_view(R"EOF( name: dynamic-delay typed_config: "@type": type.googleapis.com/nighthawk.server.ResponseOptions static_delay: 0.1s )EOF"), - absl::string_view("name: test-server")}), - testing::ValuesIn({TestRequestMethod::GET, TestRequestMethod::POST}))); + absl::string_view("name: test-server")}), + testing::ValuesIn({TestRequestMethod::GET, TestRequestMethod::POST}))); TEST_P(HttpFilterBaseIntegrationTest, NoRequestLevelConfigurationShouldSucceed) { Envoy::IntegrationStreamDecoderPtr response = getResponse(getHappyFlowResponseOrigin()); diff --git a/test/server/http_test_server_filter_integration_test.cc b/test/server/http_test_server_filter_integration_test.cc index db2233ff5..3e246e64a 100644 --- a/test/server/http_test_server_filter_integration_test.cc +++ b/test/server/http_test_server_filter_integration_test.cc @@ -296,9 +296,9 @@ TEST(HttpTestServerDecoderFilterTest, HeaderMerge) { {":status", "200"}, {"foo", "bar2"}, {"foo2", "bar3"}})); EXPECT_FALSE(Server::Configuration::mergeJsonConfig("bad_json", options, error_message)); - EXPECT_EQ("Error merging json config: Unable to parse JSON as proto (INVALID_ARGUMENT:Unexpected " - "token.\nbad_json\n^): bad_json", - error_message); + EXPECT_THAT(error_message, + testing::HasSubstr("Error merging json config: Unable to parse JSON as proto " + "(INVALID_ARGUMENT:Unexpected token.\nbad_json\n^): bad_json")); EXPECT_EQ(3, options.response_headers_size()); } diff --git a/test/service_test.cc b/test/service_test.cc index 57a78f64d..fd4ff39d8 100644 --- a/test/service_test.cc +++ b/test/service_test.cc @@ -100,7 +100,7 @@ class ServiceTest : public TestWithParam { ASSERT_FALSE(match_error.empty()); EXPECT_TRUE(response_.has_error_detail()); EXPECT_EQ(response_.has_output(), expect_output); - EXPECT_EQ(::grpc::StatusCode::INTERNAL, response_.error_detail().code()); + EXPECT_EQ(grpc::StatusCode::INTERNAL, response_.error_detail().code()); EXPECT_THAT(response_.error_detail().message(), HasSubstr(std::string(match_error))); EXPECT_TRUE(status.ok()); } @@ -143,7 +143,7 @@ INSTANTIATE_TEST_SUITE_P(IpVersions, ServiceTestWithParameterizedConstructor, TEST_P(ServiceTestWithParameterizedConstructor, ConstructorWithLoggingContextParameterCanRespondToRequests) { - std::unique_ptr<::grpc::ClientReaderWriter> stream = + std::unique_ptr> stream = stub_->ExecutionStream(&context_); stream->Write(request_, {}); stream->WritesDone(); @@ -152,7 +152,7 @@ TEST_P(ServiceTestWithParameterizedConstructor, EXPECT_THAT(response_.error_detail().message(), HasSubstr(std::string("Unknown failure"))); EXPECT_TRUE(response_.has_output()); EXPECT_GE(response_.output().results(0).counters().size(), 8); - ::grpc::Status status = stream->Finish(); + grpc::Status status = stream->Finish(); EXPECT_TRUE(status.ok()); } diff --git a/test/sink/BUILD b/test/sink/BUILD new file mode 100644 index 000000000..a516495ac --- /dev/null +++ b/test/sink/BUILD @@ -0,0 +1,30 @@ +load( + "@envoy//bazel:envoy_build_system.bzl", + "envoy_cc_test", + "envoy_package", +) + +licenses(["notice"]) # Apache 2 + +envoy_package() + +envoy_cc_test( + name = "sink_test", + srcs = ["sink_test.cc"], + repository = "@envoy", + deps = [ + "//source/sink:sink_impl_lib", + "@com_github_grpc_grpc//:grpc++_test", # Avoids undefined symbol _ZN4grpc24g_core_codegen_interfaceE in coverage test build. + "@envoy//source/common/common:random_generator_lib_with_external_headers", + ], +) + +envoy_cc_test( + name = "nighthawk_sink_client_test", + srcs = ["nighthawk_sink_client_test.cc"], + repository = "@envoy", + deps = [ + "//source/sink:nighthawk_sink_client_impl", + "@com_github_grpc_grpc//:grpc++_test", + ], +) diff --git a/test/sink/nighthawk_sink_client_test.cc b/test/sink/nighthawk_sink_client_test.cc new file mode 100644 index 000000000..a7160b943 --- /dev/null +++ b/test/sink/nighthawk_sink_client_test.cc @@ -0,0 +1,299 @@ +#include "external/envoy/source/common/protobuf/protobuf.h" + +#include "api/client/output.pb.h" +#include "api/sink/sink.grpc.pb.h" +#include "api/sink/sink_mock.grpc.pb.h" + +#include "sink/nighthawk_sink_client_impl.h" + +#include "grpcpp/test/mock_stream.h" + +#include "gmock/gmock.h" +#include "gtest/gtest.h" + +namespace Nighthawk { + +namespace { + +using ::Envoy::Protobuf::util::MessageDifferencer; +using ::nighthawk::SinkRequest; +using ::nighthawk::SinkResponse; +using ::nighthawk::StoreExecutionRequest; +using ::nighthawk::StoreExecutionResponse; +using ::testing::_; +using ::testing::DoAll; +using ::testing::HasSubstr; +using ::testing::Return; +using ::testing::SaveArg; +using ::testing::SetArgPointee; + +TEST(StoreExecutionResponseStream, UsesSpecifiedExecutionResponseArguments) { + StoreExecutionRequest observed_request_1; + StoreExecutionRequest observed_request_2; + nighthawk::MockNighthawkSinkStub mock_nighthawk_sink_stub; + // Configure the mock Nighthawk Service stub to return an inner mock channel when the code under + // test requests a channel. Set call expectations on the inner mock channel. + EXPECT_CALL(mock_nighthawk_sink_stub, StoreExecutionResponseStreamRaw) + .WillOnce([&observed_request_1](grpc::ClientContext*, nighthawk::StoreExecutionResponse*) { + auto* mock_writer = new grpc::testing::MockClientWriter(); + EXPECT_CALL(*mock_writer, Write(_, _)) + .WillOnce(DoAll(SaveArg<0>(&observed_request_1), Return(true))); + EXPECT_CALL(*mock_writer, WritesDone()).WillOnce(Return(true)); + EXPECT_CALL(*mock_writer, Finish()).WillOnce(Return(grpc::Status::OK)); + return mock_writer; + }) + .WillOnce([&observed_request_2](grpc::ClientContext*, nighthawk::StoreExecutionResponse*) { + auto* mock_writer = new grpc::testing::MockClientWriter(); + EXPECT_CALL(*mock_writer, Write(_, _)) + .WillOnce(DoAll(SaveArg<0>(&observed_request_2), Return(true))); + EXPECT_CALL(*mock_writer, WritesDone()).WillOnce(Return(true)); + EXPECT_CALL(*mock_writer, Finish()).WillOnce(Return(grpc::Status::OK)); + return mock_writer; + }); + + NighthawkSinkClientImpl client; + StoreExecutionRequest request_1, request_2; + nighthawk::client::ExecutionResponse execution_response_1, execution_response_2; + nighthawk::client::Counter* counter = + execution_response_1.mutable_output()->add_results()->add_counters(); + counter->set_name("test_1"); + counter->set_value(1); + counter = execution_response_2.mutable_output()->add_results()->add_counters(); + counter->set_name("test_2"); + counter->set_value(2); + *request_1.mutable_execution_response() = execution_response_1; + *request_2.mutable_execution_response() = execution_response_2; + + absl::StatusOr response_1 = + client.StoreExecutionResponseStream(mock_nighthawk_sink_stub, request_1); + absl::StatusOr response_2 = + client.StoreExecutionResponseStream(mock_nighthawk_sink_stub, request_2); + EXPECT_EQ(observed_request_1.DebugString(), request_1.DebugString()); + EXPECT_EQ(observed_request_2.DebugString(), request_2.DebugString()); + EXPECT_TRUE(MessageDifferencer::Equivalent(observed_request_1, request_1)); + EXPECT_TRUE(MessageDifferencer::Equivalent(observed_request_2, request_2)); +} + +TEST(StoreExecutionResponseStream, ReturnsResponseSuccessfully) { + nighthawk::MockNighthawkSinkStub mock_nighthawk_sink_stub; + // Configure the mock Nighthawk Service stub to return an inner mock channel when the code under + // test requests a channel. Set call expectations on the inner mock channel. + EXPECT_CALL(mock_nighthawk_sink_stub, StoreExecutionResponseStreamRaw) + .WillOnce([](grpc::ClientContext*, nighthawk::StoreExecutionResponse*) { + auto* mock_writer = new grpc::testing::MockClientWriter(); + EXPECT_CALL(*mock_writer, Write(_, _)).WillOnce(Return(true)); + EXPECT_CALL(*mock_writer, WritesDone()).WillOnce(Return(true)); + EXPECT_CALL(*mock_writer, Finish()).WillOnce(Return(grpc::Status::OK)); + return mock_writer; + }); + + NighthawkSinkClientImpl client; + absl::StatusOr response = + client.StoreExecutionResponseStream(mock_nighthawk_sink_stub, {}); + EXPECT_TRUE(response.ok()); +} + +TEST(StoreExecutionResponseStream, ReturnsErrorIfNighthawkServiceWriteFails) { + nighthawk::MockNighthawkSinkStub mock_nighthawk_sink_stub; + // Configure the mock Nighthawk Service stub to return an inner mock channel when the code under + // test requests a channel. Set call expectations on the inner mock channel. + EXPECT_CALL(mock_nighthawk_sink_stub, StoreExecutionResponseStreamRaw) + .WillOnce([](grpc::ClientContext*, nighthawk::StoreExecutionResponse*) { + auto* mock_writer = new grpc::testing::MockClientWriter(); + EXPECT_CALL(*mock_writer, Write(_, _)).WillOnce(Return(false)); + return mock_writer; + }); + + NighthawkSinkClientImpl client; + absl::StatusOr response = + client.StoreExecutionResponseStream(mock_nighthawk_sink_stub, {}); + ASSERT_FALSE(response.ok()); + EXPECT_EQ(response.status().code(), absl::StatusCode::kUnavailable); + EXPECT_THAT(response.status().message(), HasSubstr("Failed to write")); +} + +TEST(StoreExecutionResponseStream, ReturnsErrorIfNighthawkServiceWritesDoneFails) { + nighthawk::MockNighthawkSinkStub mock_nighthawk_sink_stub; + // Configure the mock Nighthawk Service stub to return an inner mock channel when the code under + // test requests a channel. Set call expectations on the inner mock channel. + EXPECT_CALL(mock_nighthawk_sink_stub, StoreExecutionResponseStreamRaw) + .WillOnce([](grpc::ClientContext*, nighthawk::StoreExecutionResponse*) { + auto* mock_writer = new grpc::testing::MockClientWriter(); + EXPECT_CALL(*mock_writer, Write(_, _)).WillOnce(Return(true)); + EXPECT_CALL(*mock_writer, WritesDone()).WillOnce(Return(false)); + return mock_writer; + }); + + NighthawkSinkClientImpl client; + absl::StatusOr response = + client.StoreExecutionResponseStream(mock_nighthawk_sink_stub, {}); + ASSERT_FALSE(response.ok()); + EXPECT_EQ(response.status().code(), absl::StatusCode::kInternal); + EXPECT_THAT(response.status().message(), HasSubstr("WritesDone() failed")); +} + +TEST(StoreExecutionResponseStream, PropagatesErrorIfNighthawkServiceGrpcStreamClosesAbnormally) { + nighthawk::MockNighthawkSinkStub mock_nighthawk_sink_stub; + // Configure the mock Nighthawk Service stub to return an inner mock channel when the code under + // test requests a channel. Set call expectations on the inner mock channel. + EXPECT_CALL(mock_nighthawk_sink_stub, StoreExecutionResponseStreamRaw) + .WillOnce([](grpc::ClientContext*, nighthawk::StoreExecutionResponse*) { + auto* mock_writer = new grpc::testing::MockClientWriter(); + EXPECT_CALL(*mock_writer, Write(_, _)).WillOnce(Return(true)); + EXPECT_CALL(*mock_writer, WritesDone()).WillOnce(Return(true)); + EXPECT_CALL(*mock_writer, Finish()) + .WillOnce( + Return(grpc::Status(grpc::PERMISSION_DENIED, "Finish failure status message"))); + return mock_writer; + }); + + NighthawkSinkClientImpl client; + absl::StatusOr response = + client.StoreExecutionResponseStream(mock_nighthawk_sink_stub, {}); + ASSERT_FALSE(response.ok()); + EXPECT_EQ(response.status().code(), absl::StatusCode::kPermissionDenied); + EXPECT_THAT(response.status().message(), HasSubstr("Finish failure status message")); +} + +TEST(SinkRequest, UsesSpecifiedCommandLineOptions) { + SinkRequest request; + nighthawk::MockNighthawkSinkStub mock_nighthawk_sink_stub; + // Configure the mock Nighthawk Service stub to return an inner mock channel when the code under + // test requests a channel. Set call expectations on the inner mock channel. + EXPECT_CALL(mock_nighthawk_sink_stub, SinkRequestStreamRaw) + .WillOnce([&request](grpc::ClientContext*) { + auto* mock_reader_writer = + new grpc::testing::MockClientReaderWriter(); + // SinkRequest currently expects Read to return true exactly once. + EXPECT_CALL(*mock_reader_writer, Read(_)).WillOnce(Return(true)).WillOnce(Return(false)); + // Capture the Nighthawk request SinkRequest sends on the channel. + EXPECT_CALL(*mock_reader_writer, Write(_, _)) + .WillOnce(DoAll(SaveArg<0>(&request), Return(true))); + EXPECT_CALL(*mock_reader_writer, WritesDone()).WillOnce(Return(true)); + EXPECT_CALL(*mock_reader_writer, Finish()).WillOnce(Return(grpc::Status::OK)); + return mock_reader_writer; + }); + + nighthawk::SinkRequest sink_request; + *(sink_request.mutable_execution_id()) = "abc"; + NighthawkSinkClientImpl client; + absl::StatusOr distributed_response_or = + client.SinkRequestStream(mock_nighthawk_sink_stub, sink_request); + EXPECT_TRUE(distributed_response_or.ok()); + EXPECT_EQ(request.execution_id(), "abc"); +} + +TEST(SinkRequest, ReturnsNighthawkResponseSuccessfully) { + SinkResponse expected_response; + nighthawk::MockNighthawkSinkStub mock_nighthawk_sink_stub; + // Configure the mock Nighthawk Service stub to return an inner mock channel when the code under + // test requests a channel. Set call expectations on the inner mock channel. + EXPECT_CALL(mock_nighthawk_sink_stub, SinkRequestStreamRaw) + .WillOnce([&expected_response](grpc::ClientContext*) { + auto* mock_reader_writer = + new grpc::testing::MockClientReaderWriter(); + // SinkRequest currently expects Read to return true exactly once. + // Capture the gRPC response proto as it is written to the output parameter. + EXPECT_CALL(*mock_reader_writer, Read(_)) + .WillOnce(DoAll(SetArgPointee<0>(expected_response), Return(true))) + .WillOnce(Return(false)); + EXPECT_CALL(*mock_reader_writer, Write(_, _)).WillOnce(Return(true)); + EXPECT_CALL(*mock_reader_writer, WritesDone()).WillOnce(Return(true)); + EXPECT_CALL(*mock_reader_writer, Finish()).WillOnce(Return(grpc::Status::OK)); + return mock_reader_writer; + }); + + NighthawkSinkClientImpl client; + absl::StatusOr response_or = + client.SinkRequestStream(mock_nighthawk_sink_stub, nighthawk::SinkRequest()); + EXPECT_TRUE(response_or.ok()); + SinkResponse actual_response = response_or.value(); + EXPECT_TRUE(MessageDifferencer::Equivalent(actual_response, expected_response)); + EXPECT_EQ(actual_response.DebugString(), expected_response.DebugString()); +} + +TEST(SinkRequest, WillFinishIfNighthawkServiceDoesNotSendResponse) { + nighthawk::MockNighthawkSinkStub mock_nighthawk_sink_stub; + // Configure the mock Nighthawk Service stub to return an inner mock channel when the code under + // test requests a channel. Set call expectations on the inner mock channel. + EXPECT_CALL(mock_nighthawk_sink_stub, SinkRequestStreamRaw).WillOnce([](grpc::ClientContext*) { + auto* mock_reader_writer = + new grpc::testing::MockClientReaderWriter(); + EXPECT_CALL(*mock_reader_writer, Read(_)).WillOnce(Return(false)); + EXPECT_CALL(*mock_reader_writer, Write(_, _)).WillOnce(Return(true)); + EXPECT_CALL(*mock_reader_writer, WritesDone()).WillOnce(Return(true)); + EXPECT_CALL(*mock_reader_writer, Finish()).WillOnce(Return(grpc::Status::OK)); + return mock_reader_writer; + }); + + NighthawkSinkClientImpl client; + absl::StatusOr response_or = + client.SinkRequestStream(mock_nighthawk_sink_stub, nighthawk::SinkRequest()); + EXPECT_TRUE(response_or.ok()); +} + +TEST(SinkRequest, ReturnsErrorIfNighthawkServiceWriteFails) { + nighthawk::MockNighthawkSinkStub mock_nighthawk_sink_stub; + // Configure the mock Nighthawk Service stub to return an inner mock channel when the code under + // test requests a channel. Set call expectations on the inner mock channel. + EXPECT_CALL(mock_nighthawk_sink_stub, SinkRequestStreamRaw).WillOnce([](grpc::ClientContext*) { + auto* mock_reader_writer = + new grpc::testing::MockClientReaderWriter(); + EXPECT_CALL(*mock_reader_writer, Write(_, _)).WillOnce(Return(false)); + return mock_reader_writer; + }); + + NighthawkSinkClientImpl client; + absl::StatusOr response_or = + client.SinkRequestStream(mock_nighthawk_sink_stub, nighthawk::SinkRequest()); + ASSERT_FALSE(response_or.ok()); + EXPECT_EQ(response_or.status().code(), absl::StatusCode::kUnavailable); + EXPECT_THAT(response_or.status().message(), HasSubstr("Failed to write")); +} + +TEST(SinkRequest, ReturnsErrorIfNighthawkServiceWritesDoneFails) { + nighthawk::MockNighthawkSinkStub mock_nighthawk_sink_stub; + // Configure the mock Nighthawk Service stub to return an inner mock channel when the code under + // test requests a channel. Set call expectations on the inner mock channel. + EXPECT_CALL(mock_nighthawk_sink_stub, SinkRequestStreamRaw).WillOnce([](grpc::ClientContext*) { + auto* mock_reader_writer = + new grpc::testing::MockClientReaderWriter(); + EXPECT_CALL(*mock_reader_writer, Write(_, _)).WillOnce(Return(true)); + EXPECT_CALL(*mock_reader_writer, WritesDone()).WillOnce(Return(false)); + return mock_reader_writer; + }); + + NighthawkSinkClientImpl client; + absl::StatusOr response_or = + client.SinkRequestStream(mock_nighthawk_sink_stub, nighthawk::SinkRequest()); + ASSERT_FALSE(response_or.ok()); + EXPECT_EQ(response_or.status().code(), absl::StatusCode::kInternal); + EXPECT_THAT(response_or.status().message(), HasSubstr("WritesDone() failed")); +} + +TEST(SinkRequest, PropagatesErrorIfNighthawkServiceGrpcStreamClosesAbnormally) { + nighthawk::MockNighthawkSinkStub mock_nighthawk_sink_stub; + // Configure the mock Nighthawk Service stub to return an inner mock channel when the code under + // test requests a channel. Set call expectations on the inner mock channel. + EXPECT_CALL(mock_nighthawk_sink_stub, SinkRequestStreamRaw).WillOnce([](grpc::ClientContext*) { + auto* mock_reader_writer = + new grpc::testing::MockClientReaderWriter(); + // SinkRequest currently expects Read to return true exactly once. + EXPECT_CALL(*mock_reader_writer, Read(_)).WillOnce(Return(true)).WillOnce(Return(false)); + EXPECT_CALL(*mock_reader_writer, Write(_, _)).WillOnce(Return(true)); + EXPECT_CALL(*mock_reader_writer, WritesDone()).WillOnce(Return(true)); + EXPECT_CALL(*mock_reader_writer, Finish()) + .WillOnce(Return(grpc::Status(grpc::PERMISSION_DENIED, "Finish failure status message"))); + return mock_reader_writer; + }); + + NighthawkSinkClientImpl client; + absl::StatusOr response_or = + client.SinkRequestStream(mock_nighthawk_sink_stub, nighthawk::SinkRequest()); + ASSERT_FALSE(response_or.ok()); + EXPECT_EQ(response_or.status().code(), absl::StatusCode::kPermissionDenied); + EXPECT_THAT(response_or.status().message(), HasSubstr("Finish failure status message")); +} + +} // namespace +} // namespace Nighthawk diff --git a/test/sink/sink_test.cc b/test/sink/sink_test.cc new file mode 100644 index 000000000..df2169743 --- /dev/null +++ b/test/sink/sink_test.cc @@ -0,0 +1,143 @@ +#include +#include + +#include "external/envoy/source/common/common/random_generator.h" + +#include "sink/sink_impl.h" + +#include "gtest/gtest.h" + +namespace Nighthawk { +namespace { + +// Future sink implementations register here for testing top level generic sink behavior. +using SinkTypes = testing::Types; + +template class TypedSinkTest : public testing::Test { +public: + void SetUp() override { uuid_ = random_.uuid(); } + void TearDown() override { + std::error_code error_code; + std::filesystem::remove_all(std::filesystem::path("/tmp/nh/" + uuid_), error_code); + } + std::string executionIdForTest() const { return uuid_; } + +private: + Envoy::Random::RandomGeneratorImpl random_; + std::string uuid_; +}; + +TYPED_TEST_SUITE(TypedSinkTest, SinkTypes); + +TYPED_TEST(TypedSinkTest, BasicSaveAndLoad) { + TypeParam sink; + nighthawk::client::ExecutionResponse result_to_store; + *(result_to_store.mutable_execution_id()) = this->executionIdForTest(); + absl::Status status = sink.StoreExecutionResultPiece(result_to_store); + ASSERT_TRUE(status.ok()); + const auto status_or_execution_responses = sink.LoadExecutionResult(this->executionIdForTest()); + ASSERT_EQ(status_or_execution_responses.ok(), true); + ASSERT_EQ(status_or_execution_responses.value().size(), 1); + EXPECT_EQ(this->executionIdForTest(), status_or_execution_responses.value()[0].execution_id()); +} + +TYPED_TEST(TypedSinkTest, LoadNonExisting) { + TypeParam sink; + const auto status_or_execution_responses = sink.LoadExecutionResult(this->executionIdForTest()); + ASSERT_EQ(status_or_execution_responses.ok(), false); + EXPECT_EQ(status_or_execution_responses.status().code(), absl::StatusCode::kNotFound); +} + +TYPED_TEST(TypedSinkTest, EmptyKeyStoreFails) { + TypeParam sink; + nighthawk::client::ExecutionResponse result_to_store; + *(result_to_store.mutable_execution_id()) = ""; + const absl::Status status = sink.StoreExecutionResultPiece(result_to_store); + ASSERT_FALSE(status.ok()); + EXPECT_EQ(status.code(), absl::StatusCode::kInvalidArgument); + EXPECT_EQ(status.message(), "'' is not a guid: bad string length."); +} + +TYPED_TEST(TypedSinkTest, EmptyKeyLoadFails) { + TypeParam sink; + const auto status_or_execution_responses = sink.LoadExecutionResult(""); + ASSERT_EQ(status_or_execution_responses.ok(), false); + EXPECT_EQ(status_or_execution_responses.status().code(), absl::StatusCode::kInvalidArgument); + EXPECT_EQ(status_or_execution_responses.status().message(), + "'' is not a guid: bad string length."); +} + +TYPED_TEST(TypedSinkTest, Append) { + TypeParam sink; + nighthawk::client::ExecutionResponse result_to_store; + *(result_to_store.mutable_execution_id()) = this->executionIdForTest(); + absl::Status status = sink.StoreExecutionResultPiece(result_to_store); + ASSERT_TRUE(status.ok()); + status = sink.StoreExecutionResultPiece(result_to_store); + ASSERT_TRUE(status.ok()); + const auto status_or_execution_responses = sink.LoadExecutionResult(this->executionIdForTest()); + EXPECT_EQ(status_or_execution_responses.value().size(), 2); +} + +// As of today, we constrain execution id to a guid. This way the file sink implementation +// ensures that it can safely use it to create directories. In the future, other sinks may not +// have to worry about such things. In that case it makes sense to add a validation call +// to the sink interface to make this implementation specific, and make the tests below +// implementation specific too. +TYPED_TEST(TypedSinkTest, BadGuidShortString) { + TypeParam sink; + const auto status_or_execution_responses = + sink.LoadExecutionResult("14e75b2a-3e31-4a62-9279-add1e54091f"); + ASSERT_EQ(status_or_execution_responses.ok(), false); + EXPECT_EQ(status_or_execution_responses.status().code(), absl::StatusCode::kInvalidArgument); + EXPECT_EQ(status_or_execution_responses.status().message(), + "'14e75b2a-3e31-4a62-9279-add1e54091f' is not a guid: bad string length."); +} + +TYPED_TEST(TypedSinkTest, BadGuidBadDashPlacement) { + TypeParam sink; + const auto status_or_execution_responses = + sink.LoadExecutionResult("14e75b2a3-e31-4a62-9279-add1e54091f9"); + ASSERT_EQ(status_or_execution_responses.ok(), false); + EXPECT_EQ(status_or_execution_responses.status().code(), absl::StatusCode::kInvalidArgument); + EXPECT_EQ(status_or_execution_responses.status().message(), + "'14e75b2a3-e31-4a62-9279-add1e54091f9' is not a guid: expectations around '-' " + "positions not met."); +} + +TYPED_TEST(TypedSinkTest, BadGuidInvalidCharacter) { + TypeParam sink; + const auto status_or_execution_responses = + sink.LoadExecutionResult("14e75b2a-3e31-4x62-9279-add1e54091f9"); + ASSERT_EQ(status_or_execution_responses.ok(), false); + EXPECT_EQ(status_or_execution_responses.status().code(), absl::StatusCode::kInvalidArgument); + EXPECT_EQ( + status_or_execution_responses.status().message(), + "'14e75b2a-3e31-4x62-9279-add1e54091f9' is not a guid: unexpected character encountered."); +} + +TEST(FileSinkTest, CorruptedFile) { + FileSinkImpl sink; + const std::string execution_id = "14e75b2a-3e31-4162-9279-add1e54091f9"; + std::error_code error_code; + std::filesystem::remove_all("/tmp/nh/" + execution_id + "/", error_code); + nighthawk::client::ExecutionResponse result_to_store; + *(result_to_store.mutable_execution_id()) = execution_id; + ASSERT_TRUE(sink.StoreExecutionResultPiece(result_to_store).ok()); + auto status = sink.LoadExecutionResult(execution_id); + ASSERT_TRUE(status.ok()); + EXPECT_EQ(status.value().size(), 1); + { + std::ofstream outfile; + outfile.open("/tmp/nh/" + execution_id + "/badfile", std::ios_base::out); + outfile << "this makes no sense"; + } + status = sink.LoadExecutionResult(execution_id); + ASSERT_FALSE(status.ok()); + EXPECT_EQ(status.status().message(), + "Failed to parse ExecutionResponse " + "'\"/tmp/nh/14e75b2a-3e31-4162-9279-add1e54091f9/badfile\"'."); +} + +} // namespace +} // namespace Nighthawk diff --git a/tools/check_format.sh b/tools/check_format.sh index 847b28ddd..87045b961 100755 --- a/tools/check_format.sh +++ b/tools/check_format.sh @@ -8,7 +8,7 @@ TO_CHECK="${2:-$PWD}" bazel run @envoy//tools:code_format/check_format.py -- \ --skip_envoy_build_rule_check --namespace_check Nighthawk \ --build_fixer_check_excluded_paths=$(realpath ".") \ - --include_dir_order envoy,nighthawk,external/source/envoy,external,api,common,source,exe,server,client,grpcpp,request_source,test_common,test \ + --include_dir_order envoy,nighthawk,external/source/envoy,external,api,common,source,exe,server,client,distributor,sink,grpcpp,request_source,test_common,test \ $1 $TO_CHECK # The include checker doesn't support per-file checking, so we only