Skip to content
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

Allow specifying multiple clients for run_examples #1148

Merged
merged 1 commit into from
Jun 15, 2020
Merged

Allow specifying multiple clients for run_examples #1148

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
22 changes: 2 additions & 20 deletions cloudbuild.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ steps:
waitFor: ['run_tests_tsan']
timeout: 60m
entrypoint: 'bash'
args: ['./scripts/run_examples', '-s', 'base']
args: ['./scripts/run_examples', '-s', 'base', '-c', 'rust cpp nodejs']

- name: 'gcr.io/oak-ci/oak:latest'
id: run_examples_cpp
Expand All @@ -90,24 +90,6 @@ steps:
entrypoint: 'bash'
args: ['./scripts/run_examples', '-s', 'base', '-a', 'cpp']

# TODO(#1145): Integrate examples with multiple clients into our
# run_examples script, instead of listing them individually.
- name: 'gcr.io/oak-ci/oak:latest'
id: run_example_hello_world_nodejs
waitFor: ['run_examples_cpp']
timeout: 15m
entrypoint: 'bash'
args:
[
'./scripts/run_example',
'-s',
'base',
'-e',
'hello_world',
'-c',
'nodejs',
]

- name: 'gcr.io/oak-ci/oak:latest'
id: build_experimental
waitFor: ['run_tests']
Expand All @@ -118,7 +100,7 @@ steps:
# Run clang-tidy after the examples finish running, since they share the same `output_base`.
- name: 'gcr.io/oak-ci/oak:latest'
id: run_clang_tidy
waitFor: ['run_example_hello_world_nodejs']
waitFor: ['run_examples_cpp']
Copy link
Contributor

Choose a reason for hiding this comment

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

It may be called run_examples_with_cpp_app, since it may be confusing what cpp stands for.

timeout: 30m
entrypoint: 'bash'
args: ['./scripts/run_clang_tidy']
Expand Down
67 changes: 40 additions & 27 deletions scripts/run_example
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,13 @@ source "${SCRIPTS_DIR}/common"

server="base"
application_language="rust"
client_language="cpp"
client_languages="cpp"
buildargs=""
serverargs=""
while getopts "s:a:c:de:vh" opt; do
case "${opt}" in
h)
echo -e "Usage: ${0} [-h] [-s base|logless|none] [-a rust|cpp] [-c rust|cpp|nodejs] [-d] [-v] -e EXAMPLE [-- CLIENT_ARGS]
echo -e "Usage: ${0} [-h] [-s base|logless|none] [-a rust|cpp] [-c [rust|cpp|nodejs]...] [-d] [-v] -e EXAMPLE [-- CLIENT_ARGS]

Build and run the given example Oak Application and client.

Expand All @@ -26,7 +26,8 @@ Options:
-a Example application variant:
- rust (used by default)
- cpp
-c Example client variant:
-c Example client variants. Multiple clients can be specified by space
seperating them. Eg -c \"rust cpp nodejs\".
- rust
- cpp (used by default)
- nodejs
Expand All @@ -37,7 +38,7 @@ Options after -- will be passed to the example client program."
a)
application_language="${OPTARG}";;
c)
client_language="${OPTARG}";;
client_languages="${OPTARG}";;
s)
case "${OPTARG}" in
base|logless|none)
Expand Down Expand Up @@ -97,28 +98,40 @@ fi

readonly CLIENT_ARGS=("${@-}") # Choose client args provided after '--'.

# Run the application client.
case "${client_language}" in
rust)
cargo run --release --target=x86_64-unknown-linux-musl --manifest-path="${SCRIPTS_DIR}/../examples/${EXAMPLE}/client/rust/Cargo.toml" -- \
--root-tls-certificate="${SCRIPTS_DIR}/../examples/certs/local/ca.pem" \
"${CLIENT_ARGS[@]-}"
;;
cpp)
"./bazel-client-bin/examples/${EXAMPLE}/client/client" "${TLS_ARGS[@]}" "${ADDITIONAL_ARGS[@]-}" "${CLIENT_ARGS[@]-}"
;;
nodejs)
nodejs_client="./examples/${EXAMPLE}/client/nodejs"
if [[ -d "${nodejs_client}" ]]; then
npm start --prefix "./examples/${EXAMPLE}/client/nodejs"
else
echo "The ${EXAMPLE} example does not contain a ${client_language} client."
exit 1
fi
;;
*)
echo "Invalid example client variant: ${client_language}"
exit 1;;
esac
# Run the application clients.
for client_language in $client_languages
do
case "${client_language}" in
rust)
rust_client_manifest="${SCRIPTS_DIR}/../examples/${EXAMPLE}/client/rust/Cargo.toml"
if [[ -d "${rust_client_manifest}" ]]; then
cargo run --release --target=x86_64-unknown-linux-musl --manifest-path="${rust_client_manifest}" -- \
--root-tls-certificate="${SCRIPTS_DIR}/../examples/certs/local/ca.pem" \
"${CLIENT_ARGS[@]-}"
else
echo "The ${EXAMPLE} example does not contain a ${client_language} client. Skipping this client."
Copy link
Contributor

Choose a reason for hiding this comment

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

IIUC If we will run -c rust on the example that doesn't support it, there will be no error, and the example script will return 0 as a status.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that is indeed correct. It will print a warning, but it won't fail.

If we think it's important to fail in this case, we'd need to implement a slightly different approach, such as -c all to run all available clients.

What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think the checks for which client languages exist are working properly, and the fail-open nature of the checks means that our Cloudbuild is currently running almost none of the examples:

% grep "example does not contain" log-bdcc0238-2511-4076-90da-4810b7719888.txt  | grep -v ': +'
Step #9 - "run_examples": The abitest example does not contain a rust client. Skipping this client.
Step #9 - "run_examples": The abitest example does not contain a cpp client. Skipping this client.
Step #9 - "run_examples": The abitest example does not contain a nodejs client. Skipping this client.
Step #9 - "run_examples": The translator example does not contain a rust client. Skipping this client.
Step #9 - "run_examples": The translator example does not contain a cpp client. Skipping this client.
Step #9 - "run_examples": The translator example does not contain a nodejs client. Skipping this client.
Step #9 - "run_examples": The trusted_information_retrieval example does not contain a rust client. Skipping this client.
Step #9 - "run_examples": The trusted_information_retrieval example does not contain a cpp client. Skipping this client.
Step #9 - "run_examples": The trusted_information_retrieval example does not contain a nodejs client. Skipping this client.
Step #9 - "run_examples": The hello_world example does not contain a rust client. Skipping this client.
Step #9 - "run_examples": The hello_world example does not contain a cpp client. Skipping this client.
Step #9 - "run_examples": The chat example does not contain a rust client. Skipping this client.
Step #9 - "run_examples": The chat example does not contain a cpp client. Skipping this client.
Step #9 - "run_examples": The chat example does not contain a nodejs client. Skipping this client.
Step #9 - "run_examples": The running_average example does not contain a rust client. Skipping this client.
Step #9 - "run_examples": The running_average example does not contain a cpp client. Skipping this client.
Step #9 - "run_examples": The running_average example does not contain a nodejs client. Skipping this client.
Step #9 - "run_examples": The private_set_intersection example does not contain a rust client. Skipping this client.
Step #9 - "run_examples": The private_set_intersection example does not contain a cpp client. Skipping this client.
Step #9 - "run_examples": The private_set_intersection example does not contain a nodejs client. Skipping this client.
Step #9 - "run_examples": The machine_learning example does not contain a rust client. Skipping this client.
Step #9 - "run_examples": The machine_learning example does not contain a cpp client. Skipping this client.
Step #9 - "run_examples": The machine_learning example does not contain a nodejs client. Skipping this client.
Step #9 - "run_examples": The aggregator example does not contain a rust client. Skipping this client.
Step #9 - "run_examples": The aggregator example does not contain a cpp client. Skipping this client.
Step #9 - "run_examples": The aggregator example does not contain a nodejs client. Skipping this client.
Step #10 - "run_examples_cpp": The hello_world example does not contain a cpp client. Skipping this client.
Step #10 - "run_examples_cpp": The tensorflow example does not contain a cpp client. Skipping this client.

@juliettepretot please could you take a look?

fi
;;
cpp)
cpp_client="./bazel-client-bin/examples/${EXAMPLE}/client/client"
if [[ -d "${cpp_client}" ]]; then
"${cpp_client}" "${TLS_ARGS[@]}" "${ADDITIONAL_ARGS[@]-}" "${CLIENT_ARGS[@]-}"
else
echo "The ${EXAMPLE} example does not contain a ${client_language} client. Skipping this client."
fi
;;
nodejs)
nodejs_client="./examples/${EXAMPLE}/client/nodejs"
if [[ -d "${nodejs_client}" ]]; then
npm start --prefix "${nodejs_client}"
else
echo "The ${EXAMPLE} example does not contain a ${client_language} client. Skipping this client."
fi
;;
*)
echo "Invalid example client variant: ${client_language}"
exit 1;;
esac
done

kill_bg_pids
18 changes: 13 additions & 5 deletions scripts/run_examples
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ source "${SCRIPTS_DIR}/common"

server="base"
application_language="rust"
while getopts "s:a:h" opt; do
client_languages="cpp"
while getopts "s:a:c:h" opt; do
case "${opt}" in
h)
echo -e "Usage: ${0} [-s base|logless] [-a rust|cpp]
Expand All @@ -16,10 +17,17 @@ while getopts "s:a:h" opt; do
-a Example application variant:
- rust (default)
- cpp
-c Example client variants. Multiple clients can be specified by space
seperating them. Eg -c \"rust cpp nodejs\".
- rust
- cpp (used by default)
- nodejs
-h Print Help (this message) and exit"
exit 0;;
a)
application_language="${OPTARG}";;
c)
client_languages="${OPTARG}";;
s)
case "${OPTARG}" in
base|logless)
Expand All @@ -38,12 +46,12 @@ done
examples="$(find examples -mindepth 2 -maxdepth 4 -type d -regex '.*/module.*/'"${application_language}"'$' | cut -d'/' -f2 | uniq)"
for example in ${examples}; do
if [[ "${example}" == 'aggregator' ]]; then
"${SCRIPTS_DIR}/run_example" -s "${server}" -a "${application_language}" -e aggregator -- --bucket=test --data=1:10,2:20,3:30
"${SCRIPTS_DIR}/run_example" -s "${server}" -a "${application_language}" -c "${client_languages}" -e aggregator -- --bucket=test --data=1:10,2:20,3:30
Copy link
Contributor

Choose a reason for hiding this comment

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

I think at this point we might have to implement all lang clients for all examples, since example folders are often exhaustive - so developers will have less time figuring out how to work with Oak.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe yes.

elif [[ "${example}" == 'chat' ]]; then
"${SCRIPTS_DIR}/run_example" -s "${server}" -a "${application_language}" -e chat -- --test
"${SCRIPTS_DIR}/run_example" -s "${server}" -a "${application_language}" -c "${client_languages}" -e chat -- --test
elif [[ "${example}" == 'trusted_information_retrieval' ]]; then
"${SCRIPTS_DIR}/run_example" -s "${server}" -a "${application_language}" -c rust -e trusted_information_retrieval -- --latitude 0.0 --longitude 0.0
"${SCRIPTS_DIR}/run_example" -s "${server}" -a "${application_language}" -c "${client_languages}" -e trusted_information_retrieval -- --latitude 0.0 --longitude 0.0
else
"${SCRIPTS_DIR}/run_example" -s "${server}" -a "${application_language}" -e "${example}"
"${SCRIPTS_DIR}/run_example" -s "${server}" -a "${application_language}" -c "${client_languages}" -e "${example}"
fi
done