Add baas-network-tests nightly task for testing sync client operation with non-ideal network conditions #6852
Conversation
f1d6087 to
530d1d4
Compare
|
Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: realm-ci.
|
|
Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: realm-ci.
|
d44fa80 to
4627f9c
Compare
evergreen/config.yml
Outdated
| commands: | ||
| - func: "launch remote baas" | ||
| vars: | ||
| baas_branch: 1a0d1c021460d2215b21bc4f7c7f668714ce81c7 |
There was a problem hiding this comment.
Is this the hash of a commit? If so, do we have to keep updating it?
There was a problem hiding this comment.
can we just make this a global variable and share it with this?
realm-core/evergreen/config.yml
Line 867 in 30b7a29
There was a problem hiding this comment.
This value was created to be the baas branch/commit "global value" in evergreen and would need to continue to be updated since we have been pinning our tests against a specific version of baas.
Are we stable/confident enough to just make this master and this value would still allow us to pin a version if we need it locally for testing?
There was a problem hiding this comment.
I would like to use latest master, but are we confident enough it will be green? 🙂 We could also start by running an experiment for a week.
There was a problem hiding this comment.
Updated baas to use master - I'll let it run for a bit on my branch before pushing up to master, but so far, the tests are green.
| { | ||
| std::string admin_url = config.admin_url; | ||
| nlohmann::json login_req_body{ | ||
| {"provider", "userpass"}, |
There was a problem hiding this comment.
nit: should this be part of AppCreateConfig too?
There was a problem hiding this comment.
It already is or are you questioning whether it should be?
I am making a copy here from the AppCreateConfig structure so we don't lose the value when it is "moved" into the AdminAPISession object when that object is created.
There was a problem hiding this comment.
I was suggesting that it should be. Right now it is hardcoded.
There was a problem hiding this comment.
I don't think I follow... it is already part of the AppCreateConfig structure:
AppCreateConfig default_app_config() // the same for minimal_app_config()
{
. . .
std::string app_url = get_base_url();
std::string admin_url = get_admin_url();
. . .
return AppCreateConfig{
"test",
std::move(app_url),
std::move(admin_url), // BAAS Admin API URL may be different
. . .
};
}There was a problem hiding this comment.
Maybe we are talking about different things. admin_username and admin_password are part of AppCreateConfig (they are used to create login_req_body), but the value for provider is hardcoded to userpass. My suggestion was to move the field to AppCreateConfig and give it a default value.
There was a problem hiding this comment.
Ah, sorry - I totally missed the line that you were referring to.
It is currently only configured for userpass in install_baas.sh and it would take additional work to support anything else.
Since I doubt it's going to change in the near term, I prefer to delay that change until other login methods are added.
| db_name, | ||
| get_default_schema(), | ||
| std::move(partition_key), | ||
| true, // dev_mode_enabled |
There was a problem hiding this comment.
do we run (pretty much) all our tests in dev mode? 🤔 I remember seeing tests where we explicitly enable it..
There was a problem hiding this comment.
Good point - I think many of our tests use the default schema or provide the schema when the app is created, and I'm not sure how many deviate from this. I can try running the tests with Dev mode disabled to see what the fallout is - if it is significant (> 5 tests), I'll disable it in a separate task.
There was a problem hiding this comment.
I was able to disable it by default without too many significant changes. There were less than 10 tests that needed to be updated.
|
Can this be run locally? Do we want/need an update to the README? |
…ore into mwb/add-baas-proxy
I will create a separate document for running the baas proxy locally (using and Evergreen spawn host) as well as running the baas-network-tests locally. |
…ore into mwb/add-baas-proxy
| DATA_TEMP_DIR="${DATA_DIR}/tmp" | ||
| BAAS_REMOTE_DIR="${DATA_DIR}/baas-remote" | ||
| BAAS_WORK_DIR="${BAAS_REMOTE_DIR}/baas-work-dir" | ||
| SERVER_PID_FILE="${BAAS_REMOTE_DIR}/baas-server.pid" | ||
| BAAS_STOPPED_FILE="${BAAS_WORK_DIR}/baas_stopped" | ||
|
|
||
| BAAS_PROXY_DIR="${BAAS_REMOTE_DIR}/baas-proxy-dir" | ||
| PROXY_PID_FILE="${BAAS_REMOTE_DIR}/baas-proxy.pid" | ||
| PROXY_STOPPED_FILE="${BAAS_PROXY_DIR}/baas_proxy_stopped" |
There was a problem hiding this comment.
IIUC these need to match the directories used in the evg config. should we provide them as args to avoid risks of them going out of sync?
There was a problem hiding this comment.
It is tricky to keep all the directories in sync across the different scripts - I can look to make these args in a separate PR.
| GO_URL="https://s3.amazonaws.com/static.realm.io/evergreen-assets/go1.19.1.darwin-amd64.tar.gz" | ||
| fi | ||
| ;; | ||
| Linux) | ||
| GO_URL="https://s3.amazonaws.com/static.realm.io/evergreen-assets/go1.19.1.linux-amd64.tar.gz" |
There was a problem hiding this comment.
are these intentionally using different patch versions than above?
There was a problem hiding this comment.
I just reused the go versions from the install_baas.sh script since we have these binaries readily available. Right now, it is only being run on linux, but it has been tested on both arm and x86_64 macOS as well.
…ore into mwb/add-baas-proxy
…ore into mwb/add-baas-proxy
…ore into mwb/add-baas-proxy
…ore into mwb/add-baas-proxy
* Add baas-network-tests nightly task for testing sync client operation with non-ideal network conditions (#6852) * Added support for starting baas proxy * Fixed some issues when running the scripts * minor updates to install_baas.sh * Updates to scripts to run on evergreen spawn host * Added total time output to object store tests * Increased initial ssh connect attempts; renamed proxy to 'baas_proxy' * Minor updates to help output * Added baas network test to run bass with the proxy * Added support for separate baas admin api url value * Minor port check adjustments * Removed 'compile' task from network_tests * Disable development mode by default * Added baas remote host and network tests instructions doc * Minor updates to the instructions * Minor updates to documentation * Add non-ideal transfer and network fault tests with non-ideal network conditions (#7063) * Added non-ideal transfer and network fault tests * Added baas-network-tests to be allowed for PR runs * Updated changelog * Updated curl command to be silent * Updated changelog * Reverted some changes and added descriptions to config.yml * Fixed test extra delay * Added some evergreen script updates from another branch * Disabled baas network faults tests * Update flx migration test to enable dev mode * Fix baas branch for network tests * add more time to 'too large sync message error handling' test * Added network faults test and pulled nonideal out of nightly tests until fixes are added
What, How & Why?
Add scripts to download and run the baas proxy (Shopify/toxiproxy) that will be used to simulate non-ideal networking conditions during the object store tests. Create a
baas-network-teststask that runs the network simulation tests during the nightly build.Fixes #6817
☑️ ToDos
[ ] 🚦 Tests (or not relevant)[ ] C-API, if public C++ API changed.