Skip to content

test: Fix hot restart test under --runs_per_test #2609

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
dnoe merged 3 commits into from
Feb 15, 2018
Merged

test: Fix hot restart test under --runs_per_test #2609

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f62f1b2
Fix hot restart test under --runs_per_test
dnoe Feb 14, 2018
320cd11
Fix a few more cases where --base-id was required.
dnoe Feb 14, 2018
25a06b3
Merge branch 'master' into hot-restart-test-concurrency
dnoe Feb 15, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 18 additions & 8 deletions test/integration/hotrestart_test.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ enableHeapCheck () {
HEAPCHECK=${SAVED_HEAPCHECK}
}


[[ -z "${ENVOY_BIN}" ]] && ENVOY_BIN="${TEST_RUNDIR}"/source/exe/envoy-static

# TODO(htuch): In this test script, we are duplicating work done in test_environment.cc via sed.
Expand Down Expand Up @@ -45,20 +46,29 @@ if [[ -z "${ENVOY_IP_TEST_VERSIONS}" ]] || [[ "${ENVOY_IP_TEST_VERSIONS}" == "al
JSON_TEST_ARRAY+=("${HOT_RESTART_JSON_V6}")
fi

# Enable this test to work with --runs_per_test
if [[ -z "${TEST_RANDOM_SEED}" ]]; then
BASE_ID=1
else
BASE_ID="${TEST_RANDOM_SEED}"
Copy link
Member

@htuch htuch Feb 14, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Arguably this still has the issue that if you run two distinct bazel test on the same machine, with different run IDs, you will have potential conflict. I think it's fine for now, this is a useful improvement over status quo.

Copy link
Contributor Author

@dnoe dnoe Feb 14, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't Bazel serialize separate build or test invocations made on the same machine in the same code base? When I try it in a separate window it tells me it's waiting for the first command to complete.

Copy link
Member

@ggreenway ggreenway Feb 14, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's per-bazel-daemon. On an interactive system, that's generally 1 per user (I think). On a build machine, it's probably 1 per container.

Copy link
Member

@htuch htuch Feb 14, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's per source directory actually (I know because I run multiple concurrently in different Envoy trees :).

fi

echo "Hot restart test using --base-id ${BASE_ID}"

TEST_INDEX=0
for HOT_RESTART_JSON in "${JSON_TEST_ARRAY[@]}"
do
# Test validation.
# TODO(jun03): instead of setting the base-id, the validate server should use the nop hot restart
"${ENVOY_BIN}" -c "${HOT_RESTART_JSON}" --mode validate --service-cluster cluster \
--service-node node --base-id 1
--service-node node --base-id "${BASE_ID}"

# Now start the real server, hot restart it twice, and shut it all down as a basic hot restart
# sanity test.
echo "Starting epoch 0"
ADMIN_ADDRESS_PATH_0="${TEST_TMPDIR}"/admin.0."${TEST_INDEX}".address
"${ENVOY_BIN}" -c "${HOT_RESTART_JSON}" \
--restart-epoch 0 --base-id 1 --service-cluster cluster --service-node node \
--restart-epoch 0 --base-id "${BASE_ID}" --service-cluster cluster --service-node node \
--admin-address-path "${ADMIN_ADDRESS_PATH_0}" &

FIRST_SERVER_PID=$!
Expand All @@ -81,23 +91,23 @@ do
echo "Checking for match of --hot-restart-version and admin /hot_restart_version"
ADMIN_ADDRESS_0=$(cat "${ADMIN_ADDRESS_PATH_0}")
ADMIN_HOT_RESTART_VERSION=$(curl -sg http://${ADMIN_ADDRESS_0}/hot_restart_version)
CLI_HOT_RESTART_VERSION=$("${ENVOY_BIN}" --hot-restart-version 2>&1)
CLI_HOT_RESTART_VERSION=$("${ENVOY_BIN}" --hot-restart-version --base-id "${BASE_ID}" 2>&1)
if [[ "${ADMIN_HOT_RESTART_VERSION}" != "${CLI_HOT_RESTART_VERSION}" ]]; then
echo "Hot restart version mismatch: ${ADMIN_HOT_RESTART_VERSION} != " \
"${CLI_HOT_RESTART_VERSION}"
exit 2
fi

echo "Checking max-obj-name-len"
CLI_HOT_RESTART_VERSION=$("${ENVOY_BIN}" --hot-restart-version --max-obj-name-len 1234 2>&1)
CLI_HOT_RESTART_VERSION=$("${ENVOY_BIN}" --hot-restart-version --max-obj-name-len 1234 --base-id "${BASE_ID}" 2>&1)
if [[ "${ADMIN_HOT_RESTART_VERSION}" = "${CLI_HOT_RESTART_VERSION}" ]]; then
echo "Hot restart version match when it should mismatch: ${ADMIN_HOT_RESTART_VERSION} == " \
"${CLI_HOT_RESTART_VERSION}"
exit 2
fi

echo "Checking max-stats"
CLI_HOT_RESTART_VERSION=$("${ENVOY_BIN}" --hot-restart-version --max-stats 12345 2>&1)
CLI_HOT_RESTART_VERSION=$("${ENVOY_BIN}" --hot-restart-version --max-stats 12345 --base-id "${BASE_ID}" 2>&1)
if [[ "${ADMIN_HOT_RESTART_VERSION}" = "${CLI_HOT_RESTART_VERSION}" ]]; then
echo "Hot restart version match when it should mismatch: ${ADMIN_HOT_RESTART_VERSION} == " \
"${CLI_HOT_RESTART_VERSION}"
Expand All @@ -109,7 +119,7 @@ do
echo "Starting epoch 1"
ADMIN_ADDRESS_PATH_1="${TEST_TMPDIR}"/admin.1."${TEST_INDEX}".address
"${ENVOY_BIN}" -c "${UPDATED_HOT_RESTART_JSON}" \
--restart-epoch 1 --base-id 1 --service-cluster cluster --service-node node \
--restart-epoch 1 --base-id "${BASE_ID}" --service-cluster cluster --service-node node \
--admin-address-path "${ADMIN_ADDRESS_PATH_1}" &

SECOND_SERVER_PID=$!
Expand All @@ -126,7 +136,7 @@ do
ADMIN_ADDRESS_PATH_2="${TEST_TMPDIR}"/admin.2."${TEST_INDEX}".address
echo "Starting epoch 2"
"${ENVOY_BIN}" -c "${UPDATED_HOT_RESTART_JSON}" \
--restart-epoch 2 --base-id 1 --service-cluster cluster --service-node node \
--restart-epoch 2 --base-id "${BASE_ID}" --service-cluster cluster --service-node node \
--admin-address-path "${ADMIN_ADDRESS_PATH_2}" &

THIRD_SERVER_PID=$!
Expand Down Expand Up @@ -167,7 +177,7 @@ set +e
disableHeapCheck

echo "Launching envoy with no parameters. Check the exit value is 1"
${ENVOY_BIN}
${ENVOY_BIN} --base_id "${BASE_ID}"
EXIT_CODE=$?
# The test should fail if the Envoy binary exits with anything other than 1.
if [[ $EXIT_CODE -ne 1 ]]; then
Expand Down