m-lab / murakami

Run automated internet measurement tests in a Docker container.
Apache License 2.0
41 stars 11 forks source link

Add project setup utility scripts #92

Closed critzo closed 3 years ago

critzo commented 3 years ago

Setting up a new set of GCP resources to support a new measurement program with a fleet of devices was a bit manual. This PR adds some automation scripts to make the setup process easier. Two scripts and associated schema files are included. @robertodauria PTAL?


This change is Reviewable

critzo commented 3 years ago

utilities/bq_load_murakami.sh, line 24 at r1 (raw file):

Previously, robertodauria (Roberto D'Auria) wrote…
Could this path be made more specific? e.g. `/ndt7/${loaddate}*`

This path has two wildcards because each test result starts with the test name, and the GCS exporter pushes all results to the same bucket/path. So the load script was designed to push results based on the date, for each test, into BigQuery.

critzo commented 3 years ago

utilities/bq_load_murakami.sh, line 32 at r1 (raw file):

Previously, robertodauria (Roberto D'Auria) wrote…
Same as above.

same as above :)

critzo commented 3 years ago

utilities/bq_load_murakami.sh, line 12 at r1 (raw file):

Previously, robertodauria (Roberto D'Auria) wrote…
I would suggest taking these from the command line so the user does not need to edit the script to use it and provide a usage example. e.g.: ``` usage="$0 " gcp_project=${1:?Please provide the GCP project: ${usage}} bucket=${2:?Please provide the GCS bucket: ${usage}} bq_dataset=${3:?Please provide the dataset name: ${usage}} ```

This script is intended to be run by cron. Given that does this suggestion still make sense?

critzo commented 3 years ago

utilities/setup_bq.sh, line 7 at r1 (raw file):

Previously, robertodauria (Roberto D'Auria) wrote…
If these are meant to be replaced by the user, please get them from the command line. If they are not meant to be changed, just disregard this comment.

Agree in this case. Will change.

critzo commented 3 years ago

utilities/setup_bq.sh, line 19 at r1 (raw file):

Previously, robertodauria (Roberto D'Auria) wrote…
Please use the $(...) syntax to run commands (https://github.com/koalaman/shellcheck/wiki/SC2006) - same for the rest of these scripts.

ack

critzo commented 3 years ago

utilities/setup_bq.sh, line 83 at r1 (raw file):

Previously, robertodauria (Roberto D'Auria) wrote…
> ``` > ·· > ``` Nit: extra spaces after `WITH`.

ack

critzo commented 3 years ago

utilities/setup_bq.sh, line 142 at r1 (raw file):

Previously, robertodauria (Roberto D'Auria) wrote…
Could you use `"` here and simplify the variable expansion below? Same for the other queries.

Yes.

critzo commented 3 years ago

utilities/bq_load_murakami.sh, line 12 at r1 (raw file):

Previously, robertodauria (Roberto D'Auria) wrote…
I think it does: any user needs to provide their own project, bucket and dataset before running the script, and that approach ensures that they have explicitly provided them or the script won't even start. It also allows us to automate the deployment with either Travis or GCB in the future more easily (we can pass those arguments in build env variables and add them to the command line instead of doing string replacements into the script).

Thanks. Makes sense.

critzo commented 3 years ago

@robertodauria a few commits added to resolve your suggestions. PTAL again?