refactor: redesign SGW upgrades + greenboard uploads#414
Conversation
There was a problem hiding this comment.
Pull request overview
Refactors the SGW upgrade and rolling-upgrade QE jobs to run as a single source→target upgrade (instead of iterating through a list of versions), and redesigns Greenboard uploads to write into a per-upgrade-type matrix document.
Changes:
- Update upgrade/rolling-upgrade Jenkins pipelines and shell wrappers to accept
SGW_VERSION_FROM+SGW_VERSION_TO, run a pre-upgrade seed session (no upload), then a post-upgrade session (uploads). - Simplify upgrade test cases to derive the running SGW version via
get_version()and remove phase-based doc decoration. - Replace the previous “write per-iteration results + batch uploader script” approach with
GreenboardUploader.upload_upgrade_result()that upserts into deterministic docs (sgw-upgrade::waterfall/sgw-upgrade::rolling).
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/QE/test_upg_sgw.py | Uses live SGW version instead of env var; removes setup/print noise. |
| tests/QE/test_rolling_upgrade_sgw.py | Removes phase-based behavior and stamps docs only with the running version. |
| jenkins/pipelines/QE/upg-sgw/upload_greenboard_batch.py | Deletes the batch greenboard uploader script (no longer used). |
| jenkins/pipelines/QE/upg-sgw/test.sh | Switches to <sgw_from> <sgw_to> flow with pre/post pytest runs and env vars for upgrade uploads. |
| jenkins/pipelines/QE/upg-sgw/test_rolling.sh | Same as above for rolling upgrades; performs node roll then runs post-upgrade pytest upload. |
| jenkins/pipelines/QE/upg-sgw/Jenkinsfile | Replaces SGW_VERSIONS with SGW_VERSION_FROM/TO and wires them into the runner script. |
| jenkins/pipelines/QE/upg-sgw/Jenkinsfile_rolling | Same parameter/wiring update for rolling upgrade job. |
| client/src/cbltest/plugins/greenboard_fixture.py | Routes SGW upgrade runs to the new matrix upload path when env vars are set; removes --upgrade-versions. |
| client/src/cbltest/greenboarduploader.py | Implements deterministic matrix doc upsert for upgrade results and factors out collection opening. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (2)
jenkins/pipelines/QE/upg-sgw/test.sh:24
- The script now accepts any extra positional arguments beyond
<cbl_version> <sgw_from> <sgw_to> [--setup-only]and silently ignores them (e.g., the old multi-version invocation would quietly drop versions after$3). Consider validating#is either 3 or 4 (and error on unexpected args) to avoid running the wrong upgrade path due to misconfigured Jenkins/UI parameters.
if [ "$#" -lt 3 ]; then usage; fi
CBL_VERSION="$1"
SGW_FROM="$2"
SGW_TO="$3"
SETUP_ONLY=false
if [ "${4:-}" = "--setup-only" ]; then
SETUP_ONLY=true
fi
jenkins/pipelines/QE/upg-sgw/test_rolling.sh:24
- The script now accepts any extra positional arguments beyond
<cbl_version> <sgw_from> <sgw_to> [--setup-only]and silently ignores them. This can mask misconfiguration (e.g., passing the old SGW_VERSIONS list would still run but not test the intended target). Consider validating argument count is exactly 3 or 4 and erroring otherwise.
if [ "$#" -lt 3 ]; then usage; fi
CBL_VERSION="$1"
SGW_FROM="$2"
SGW_TO="$3"
SETUP_ONLY=false
if [ "${4:-}" = "--setup-only" ]; then
SETUP_ONLY=true
fi
borrrden
left a comment
There was a problem hiding this comment.
Double check the test steps to make sure they are not simply logging messages in disguise. They are meant to closely, if not identically, follow the markdown spec testing steps. Furthermore I think stuffing everything into a single document is not a good way to go, but I won't push back against it too much now. NoSQL is good for creating a bunch of small entries and aggregating them and the fact that they all need to be stuffed into a single document means that there is a lot of complicated branching happening purely in the greenboard uploader. The more special cases there are, the more the code smells. There needs to be some difference because of the information presented, but this is creating dozens of lines dedicated to just uploading sync gateway upload results.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
| uv run pytest -v --no-header --maxfail=7 -W ignore::DeprecationWarning --config config.json --dataset-version $DATASET_VERSION \ | ||
| --junitxml="$GREENBOARD_RESULTS_DIR/junit_dev_e2e_android.xml" |
There was a problem hiding this comment.
As discussed in the shared channel with @borrrden on slack, let's figure out the simplest way to implement just modifying junit files, and put this in a separate PR. Let's implement this for all test runs in one PR, and then we can worry about how to implement this for upgrade tests in a different PR.
- add
junitxml=argument to pytest.ini to sayjunit_report.xmlwhich will always write this xml file to the cwd where pytest is launched - register that file via junit in jenkinsfiles
It might not be necessary to actually upload these xml files, because ultimately these artifacts are uploaded and stored on jenkins, and these don't need to be registered with archiveArtifacts
Can you create a PR which changes only this, and show an example of a mobile jenkins run that uses this output? I am also interested in how this looks for a pipeline job which wraps up multiple platforms http://jenkins.mobiledev.couchbase.com/view/QE/job/qe-java-pipeline/
After that PR looks OK, then we can think about how to extend the greenboard code in client/src/cbltest , but these are separate problems. For that case, we can override the names of the arguments to pytest with -o junitxml.
| # Per-Jenkins-build directory where every pytest invocation drops its | ||
| # --junitxml output and the greenboard fixture drops its meta_*.json | ||
| # sidecar. The aggregator (python -m cbltest.greenboard_upload) reads | ||
| # this dir at the end of the build to produce one greenboard doc. | ||
| # Default to $WORKSPACE on Jenkins agents; fall back to $PWD locally. | ||
| readonly GREENBOARD_RESULTS_DIR="${WORKSPACE:-$PWD}/.greenboard-results" |
There was a problem hiding this comment.
I think you can avoid having any extra data in shell scripts to just picking up all files that are junit_*.xml in. the test files.
I would say anytime I see a shell script modified, especially if it is duplicated, I ask myself if there's a way to avoid writing this.
| finally: | ||
| pytestconfig.pluginmanager.unregister(uploader) |
There was a problem hiding this comment.
I think this is intentionally unregistering the value - and in fact, I think there are several unintentional regressions.
The way I like to think about modifying code like this is to write tests against the existing behavior, so then the reviewer can see what code is changed. I wrote tests: #418 for this code.
I think this part of this PR should be put on hold in favor of seeing how the junit output will be registered for the non upgrade case, so then I can understand how it should look/be modified for that case.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
CBG-5387
Redesigning of the Upgrade tests for SGW. Now upgrade tests work like a one shot source to target upgrade rather than a linear array of versions to keep upgrading through till reaching target.
The end goal in a different PR would be to wrap this inside a list of versions where all permutations would run for each version as source and would upgrade to each major version.
That can be changed as discussed and agreed upon.
Please refer to the above ticket to see how the upgrades are technically designed to upgrade now.