Edge Server Greenboard Integration - #475
Open
barkha06 wants to merge 47 commits into
Open
Conversation
barkha06
commented
Aug 13, 2026
Contributor
- Added edge server parameter in greenboard uploader and fixture, made version parameter optional since majority of edge server tests do not use testserver
- Added fixes to changes feed tests and logging tests where creation of an existing bucket throws an error
- Added pytest skip decorator to edge server blobs tests since the functionality is not yet implemented
- Marked all edge server tests for easy idenitification
- Fixed version fetch of edge server to match the format required to upload on greenboard
Collaborator
|
I pushed up a version to resolve the merge conflicts from the formatting merge but I didn't make any other changes. |
Member
|
Fix the validation please |
borrrden
removed their request for review
August 14, 2026 18:00
Conflicts resolved: - client/pyproject.toml: keep both sets of pytest11 entry points - test_changes_feed.py / test_logging.py: take main's sync_gateway_cluster.create_database(); the autouse cluster_cleanup fixture from #478 supersedes the manual delete_database workaround - test_jwt_rotation.py / test_greenboarduploader.py: additive, kept both sides with main's type annotations Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
- edgeserver.py: type the three ClientSession attributes as optional (close() sets them to None) and raise CblTestError in _send_request when the chosen session has already been closed - es_collect_fixture.py: annotate the fixture, and take Sequence rather than list for edge_servers — cblpytest.edge_servers is a Sequence, which typing the fixture surfaced - write_file_on_es / test_all_fields_es_run: add return annotations - test_blobs.py: drop three sync_gateway.delete_database() calls; that method now lives on SyncGatewayCluster and the autouse cluster_cleanup fixture already wipes SGW databases before every test Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The tests used to write their rendered config back over these checked-in templates, so each file accumulated whatever host the last run happened to use. Now that write_derived_json_file() renders to a temp copy, the templates can hold placeholders again: - replication sources -> sgw.example.com / edge.example.com - test_e2e_audit.json: drop logging.audit.enable="*", left behind by a previous run's [enabled] parametrization. This was an active failure: test_audit_logging[default] asserts audit event 57346 is absent, but the clobbered template enabled auditing, so it failed with "Audit log for public HTTP request event found". Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
test_valid_tls_mtls failed at the plain-TLS step with
certificate verify failed: Missing Authority Key Identifier
Two independent bugs, both required for TLS against Edge Server to work.
1. environment/aws/common/x509_certificate.py added no key identifiers or
keyUsage. Python 3.13 enables ssl.VERIFY_X509_STRICT in
create_default_context(), which enforces RFC 5280: a CA needs a
subjectKeyIdentifier and a keyUsage with keyCertSign, and a CA-issued
certificate needs an authorityKeyIdentifier. #511 fixed this for the
client-side generator in client/src/cbltest/api/x509_certificate.py but
not for this one, which is what the Edge Server setup uses.
2. setup_edge_servers.setup_server() minted a fresh CA per host and rewrote
~/.cbl_certs each time, so the client trust store ended up holding only
the last-provisioned host's CA. With three Edge Servers the client could
never verify edge_servers[0] even once (1) was fixed. The CA and the
client identity are now created once in write_client_certs() and the
same CA signs every host's certificate.
Verified against the live 3-node AWS topology: all three hosts now present
certificates chaining to the one CA in ~/.cbl_certs, and
test_authentication.py passes 2/2 including the mTLS step.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two bugs, accounting for all four test_system failures.
The rev-change assertions raced replication. A document created on Sync
Gateway is read back from Edge Server after a 1-5s sleep, but the update
path asserted immediately:
await sync_gateway.update_document(sg_db_name, doc_id, ..., rev_id)
remote_doc = await edge_server.get_document(es_db_name, doc_id)
assert remote_doc.revid != rev_id
Edge Server still held the create revision, so this failed with
"assert '1-8741e6...' != '1-8741e6...'". Replaced the immediate read at all
four sites with _get_document_after_rev(), which polls until the revision
changes or a timeout expires, leaving each site's own assertion to report a
genuine mismatch.
test_system_one_client_chaos also never cleared its restart deadline. After
chaos killed Edge Server, `end` was set one minute out and the loop
restarted the process when it passed -- but `end` stayed in the past, so
unless fresh chaos pushed it forward, the next iteration restarted an
already-running server:
start-edgeserver returned 500 ... Failed to start server: Address already in use
Replaced the 2400-minute "not scheduled" sentinel with an explicit
`restart_at: datetime | None` that is cleared once consumed.
Verified against the live AWS topology with the soak window shortened: all
4 tests pass, and one_client_chaos passes with chaos forced on every
iteration (previously it hit EADDRINUSE on the iteration after a restart).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sync Gateway and Couchbase Server got a guaranteed clean slate before every test, but nothing reset Edge Server. That was left to each test calling configure_dataset(), which only resets the single database it names, only runs on the happy path inside the test body, and never touches iptables. A test that failed partway -- or a run that was interrupted -- stranded the host for every test after it: the chaos tests leave the process killed and DROP rules installed, and test_system's three other tests never reset Edge Server at all. - reset-all-dbs.sh: new shell2http endpoint that wipes ~/database/*.cblite2 and restores the provisioned *.cblite2.zip datasets. It takes no body and needs no auth, so unlike /_all_dbs + /reset-db it works when Edge Server is stopped or running a config the caller cannot authenticate against, which is the normal state after a failure. - EdgeServer.reset_to_initial_state(): drop firewall rules first (a leftover DROP rule would hide the host), kill the process, reset every database, restart on the config the object was constructed with. - cluster_cleanup: reset Edge Servers as the first phase, before Sync Gateway databases are deleted -- the provisioned Edge Server config declares no replications, so nothing is pulling from those databases once the reset completes. The clusters guard now also admits an Edge Server topology that has no cluster at all. Verified against the live 3-node AWS topology: with all three hosts deliberately stranded (process killed and a DROP rule blocking Sync Gateway, which configure_dataset cannot repair), test_replication_sanity passes with no manual intervention, and test_crud passes 6/6 with the reset running before each test at ~1.5-2.8s for all three hosts in parallel. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Replaces four copies of timedelta(minutes=360) with a SOAK_DURATION constant, which gives the six-hour window one place to be explained. The comment records what the surrounding code does not make obvious: the elapsed window is the only thing that ends these tests. No loop breaks out early -- the single `break`, in multi_client_chaos's chaos_controller, only re-checks end_time after its 5-20 minute sleep -- and nothing bounds them from outside either. The Edge Server job sets neither PYTEST_TIMEOUT nor CBL_PYTEST_SESSION_TIMEOUT (only QE/sgw uses the latter), and a session timeout would not help regardless: pytest-timeout checks it between tests and sets shouldfail, so it never interrupts a test already running. The upshot is that nothing in CI runs this file. jenkins/pipelines/QE/es selects one file via TEST_NAME, defaulting to test_crud.py, and hard-kills the stage at 60 minutes, so TEST_NAME=test_system.py would die a sixth of the way in with no result. No behaviour change here; the duration is what the spec asks for and picking a different one is a spec decision. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
test_audit_logging[enabled] failed with "Audit log for create document event not found" while the event was in fact being written. The CRUD phase read the audit log immediately after each operation, but Edge Server flushes the audit file asynchronously. Measured against the live host: after a PUT returns, event 57358 takes ~1.15s to appear, so a read at 0s misses it every time. The earlier phase in the same test gets away with it only because it sleeps 5s first; the CRUD phase had no wait at all. Every one of the ten reads in the failing run returned an identical six-line log ending at "GET /db/_all_docs", which is what made this look like the events were absent rather than late. _wait_for_audit_event() polls until the event appears. The absence assertions keep their existing fixed sleep, since polling cannot prove a negative. check_log()'s print(content) became a cbl_trace: polling calls it repeatedly, and dumping the whole audit log to stdout on each call buried the output. Also dropped the always-true `expected_non_empty and` conjunct from the CRUD assertion. test_logging.py now passes 3/3 ([default] and [disabled] were already fixed by restoring the clobbered audit template in 2554e0a). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three try/except blocks logged a message and asserted nothing, so the test
passed whether or not the expected failure happened. Two of them were hiding
real bugs.
test_database_edge_server: the point of the test is that a bad config fails
and that writes to a read-only collection are rejected, but neither was
asserted. Now pytest.raises(CblEdgeServerBadResponseError).
test_crud: same shape around get_document after a delete. get_document raises
rather than returning None for a missing document -- _send_request rejects the
404 before get_document's own None branch is reachable -- so these are
pytest.raises too. One of them also deleted id3 and then fetched id2, which
the absent assertion is exactly why nobody noticed.
test_chaos_scenarios.perform_operation ended in `except Exception: return
False`, turning every real error into a bare `assert False` with no
diagnostic. Removing it surfaced the cause of the long-standing
test_edge_server_with_concurrent_rest_requests failure immediately:
KeyError: 'data' in json_generator.update_document
generate_document returns {doc_id: body} so batch_process can merge results,
but update_document takes a body and indexes doc["data"]. The create path
stored the wrapper in docs_dict, so a later update of a doc created during the
workload could not index it -- and it also PUT the wrapper as the document
body. Both call sites now unwrap. The same helper no longer falls off the end
returning None for an update/delete of a document with no known revision.
test_crud 6/6, test_database_edge_server 1/1, test_chaos_scenarios 4/4.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…marker Spec updates for behaviour changed earlier in this branch, as AGENTS.md requires: - test_system.md: the update verification polls until the revision differs, and the chaos restart time is cleared once consumed - test_logging.md: CRUD audit entries are polled, because Edge Server flushes the audit file asynchronously - test_crud.md: fetching a deleted document raises rather than returning None - test_database_edge_server.md: the two expected failures are asserted, and the config no longer needs restoring - test_chaos_scenarios.md: workload errors propagate, and locally tracked bodies use the shape generate_all_documents produces Converted the last eight `failed = True ... assert failed` blocks to pytest.raises, which is already the idiom elsewhere in this suite. The two query-test cases fold their substring checks into match=. Removed @pytest.mark.es in favour of @pytest.mark.min_edge_servers, which every edge-server test already carries for topology validation and which no class had without. The marker's only reader was the greenboard platform switch, now keyed on min_edge_servers; nothing selected tests with -m es. Replaced write_derived_json_file with pytest's tmp_path fixture plus plain write_json_file. pytest owns the temp lifecycle, dirs are per-test, and the files survive for debugging instead of being removed at exit -- so asyncfile.py loses the atexit/tempfile/uuid machinery and is back to three functions. Tests rendering one template per Edge Server now name the copies (es1_config.json and friends) rather than carrying uuid suffixes. es_collect_fixture uses asyncio.TaskGroup rather than asyncio.gather, matching cluster_cleanup. Behaviour is unchanged because _collect_one already swallows and logs per node, so nothing ever propagated. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
kill-edgeserver.sh sent SIGHUP, then checked once, immediately, whether the
server was still answering, escalating to SIGTERM only if it was. It never
waited for the process to exit or for the port to be released, so it returned
while shutdown was still in progress. The next /start-edgeserver then raced it:
Server is up and running.
Stopping server...
Failed to start server: Address already in use
configure_dataset() and reset_to_initial_state() both kill and immediately
restart, so this stranded the host for every test that followed. It turned a
full-suite run into 27 setup errors, and it is why killing an interrupted run
left the boxes unrecoverable even with the new cluster_cleanup reset in place.
Now escalates SIGHUP -> SIGTERM -> SIGKILL, and returns only once no
edge-server process remains and the listening port is free (checked with ss),
or fails loudly after exhausting all three signals.
Verified on a live host: 10 consecutive kill-then-start cycles with no delay
between them, 0 failures; and 5 rounds of reset_to_initial_state() across all
3 hosts in parallel, all clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The comment claimed this disables hostname verification "as we do in _create_session", but the two are not equivalent: _create_session builds a trust store from the bundled CA and relaxes only check_hostname, whereas verify=False skips chain and hostname verification alike. Behaviour is unchanged -- this is a test harness talking to a per-environment test Sync Gateway. The comment now also records that this call is the source of the single InsecureRequestWarning each session, which gets attributed to whichever test runs first and reads like a failure in the warnings summary. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
reset_to_initial_state() restored the config file but not the admin credentials. set_auth() overwrites __auth_name/__auth_password permanently and _apply_config() rebuilds the admin session from them, so once configure_dataset() started returning self, the invalid user that test_basic_auth deliberately ends on survived into cluster_cleanup and every later test: reproduced as a 401 on the first admin call after a reset. The constructor's credentials are now recorded alongside the initial config file and restored with it. test_basic_auth sorts first in the suite, so this was about to strand every run. test_crud's "fetch update doc after 20 seconds" checked id1, which had already expired in the previous block, so the assertion passed unconditionally and the behaviour under test -- that updating a TTL=50 document to TTL=20 expires it -- was never verified. It checks id2 now. This is the same bug as the id2/id3 one fixed in 92dd6dd; I fixed that instance and missed this one. check_topology_markers traces topology usage through imported helpers and autouse fixtures, so cluster_cleanup's new cblpytest.edge_servers read reached the two upgrade tests that override the fixture, and the hook demanded min_edge_servers of them. Cleanup acts on whatever the environment provides and no-ops when it provides nothing, so requiring a marker inverts the meaning; perform_cleanup is now excluded by name. collect-logs.sh was committed non-executable with a shebang, and it and test.sh both lacked the formatting shfmt wants -- three pre-commit hooks were failing, which .github/workflows/pre-commit.yml runs over all files. `uvx pre-commit run --all-files` now passes. Verified live: test_authentication (first, so contamination would show), test_crud, test_replication_sanity and test_query_edge_server pass 14/14, with zero "Unclosed client session" messages. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
CblSyncGatewayBadResponseError exposes code and body; the Edge Server error
exposed only code, so the response body was reachable only by parsing it back
out of the message string. That is what forced the query tests to assert
things like `"forbidden" in str(e).lower() or "403" in str(e)`.
The signature now matches Sync Gateway's, with body keyword-only and required
so no raise site can quietly omit it. All 23 raise sites in edgeserver.py --
the only place that constructs it -- pass one:
- _send_request, the single site with a real HTTP body, passes the body text
- the 17 that raise a synthetic 500 after finding "error" in a parsed 200
response pass dumps() of that response, as Sync Gateway already does
- the 5 "Unexpected response type" sites pass str(response), since there the
type is the point
Messages are unchanged, so existing pytest.raises(match=...) patterns keep
working; this is purely additive.
Verified live against Edge Server: code 401 with body
'{"status": 401, "error": "Unauthorized"}' that parses. test_query_edge_server,
test_ttl_expires, test_crud and test_database_edge_server pass 18/18 -- between
them the heaviest users of these assertions.
Noted but not changed: 17 of those sites index cast_resp['reason'], and Edge
Server's 404 body has no 'reason' key, so they raise KeyError instead of the
intended error. Now that body exists they could read from it instead.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…oard counts
Edge Server sends `reason` only sometimes -- a 404 on a missing collection has
it, a 401 does not:
{"status":404,"error":"Not Found","reason":"No such collection"}
{"status":401,"error":"Unauthorized"}
Twelve raise sites indexed cast_resp['reason'] and so raised KeyError instead
of reporting the error they were called about; five more used .get('reason')
and rendered "had error 'None'". A single _error_detail() helper now prefers
reason, falls back to error, then to the whole body, and all 17 sites use it.
collect-logs.sh created config/ and system/ but only ever filled logs/ and
audit/, so the archive did not contain what --es-collect promises. It now
copies the live /opt/couchbase-edge-server/etc/config.json and writes system
info: version, uname, disk, memory, edge-server processes, listening ports and
the ES_RULES chain. That last one matters most -- a DROP rule left behind by a
chaos test is invisible from the logs alone. users.json is deliberately left
out; it is a credentials file and this archive is published as a Jenkins build
artifact.
greenboarduploader gated its pass/fail tallies and marker recording on
__overall_fail, which latches on the first setup or teardown failure and is
never cleared, so everything after that point was dropped. Narrower in effect
than it looks -- upload() refuses to upload at all once the flag is set -- but
record_upgrade_step does run in that state and writes
"passCount": self.__pass_count, understating passes in the upgrade batch doc.
greenboard_fixture claimed platform "edge-server" whenever any edge-server test
ran, unlike the sgw branch above it which only ever *keeps* a platform. A
session that also drove a CBL test server (tests/dev_e2e includes
dev_e2e/edge_server) filed all its CBL results under edge-server and dropped
the CBL version; it now only claims the platform when there is no test server.
Verified live: a collect on a real host produces config/config.json and a
populated system/info.txt with all seven sections and no users.json. Four new
unit tests cover _error_detail; 177 framework tests pass.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A fresh deployment failed provisioning at "Stopping Edge Server":
Command 'bash /home/ec2-user/shell2http/kill-edgeserver.sh' failed with
exit status 1
The RPM installs couchbase-edge-server.service and starts it, so on a
newly provisioned host Edge Server is service-managed. Signalling the process
just makes systemd spawn another one -- confirmed on the host, a new pid with
PPID 1 appearing after each of SIGHUP, SIGTERM and SIGKILL -- so the port never
frees and the escalation loop added in ac04326 exhausted all three signals and
failed.
This never showed on the existing hosts because there Edge Server had long
since been restarted by hand with setsid, outside systemd's control. The old
script did not notice either, because it never verified that anything stopped;
provisioning then raced systemd for the port and usually won.
kill-edgeserver.sh now stops the unit first when one exists, then falls through
to the signal escalation for the setsid-launched process the tests use.
Verified by tearing down the whole topology and deploying from scratch: all
three Edge Servers provision, and kill-then-start with no delay between them
returns "Edge server running" on all three.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The chaos controller slept its 5-20 minute quiet period in a single call, so
the elapsed window -- the only thing that ends these tests -- went unnoticed
until the sleep returned. A run shorter than the quiet period then sat idle for
the remainder of it: with a one-minute window, test_system.py took 27 minutes,
almost all of it waiting after the workers had finished. _sleep_until() now
sleeps a second at a time and stops at the deadline, which also subsumes the
redundant post-sleep end_time check. Same 5-20 minute behaviour for a real
six-hour run; 27 minutes down to 4.
The three timings are now named constants next to each other rather than magic
numbers inside the coroutines: RUN_DURATION, CHAOS_QUIET_PERIOD_MIN/MAX and
CHAOS_DOWN_WINDOW. The down window is shared by both chaos tests, which had it
written twice -- as a timedelta in one and as sleep(60) in the other -- for the
one value the spec gives them both.
Making the test fast immediately exposed a real bug the idle wait had been
masking:
[Client 1] cc1_doc_5 update not reflected on Sync Gateway
This is the mirror of the race fixed in 386982a. That covered
Sync Gateway -> Edge Server; these four sites update on Edge Server and then
read Sync Gateway back immediately, with no wait for replication. Rather than a
second poller, _get_document_after_rev now takes EdgeServer | SyncGateway --
both expose get_document(db_name, doc_id) -> RemoteDocument | None -- so one
helper serves all eight sites in both directions.
Renamed SOAK_DURATION to RUN_DURATION. "Soak" is standard endurance-testing
jargon but appears nowhere else in this repo; the spec says "the 6-hour run"
and "the full 6-hour window", so the constant now uses the same word.
test_system.py passes 4/4 in 4m55s, and the full edge server suite passes
35/45 with 10 skipped on a from-scratch deployment.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The block above RUN_DURATION had grown to fourteen lines of CI archaeology -- pytest-timeout's internals, which environment variables the Jenkins job sets, how TEST_NAME defaults -- none of which a reader of that constant needs. It had also been patched enough times to show the seams. Three lines carry the durable facts: the spec fixes six hours, nothing times these tests out, and the stage budget is shorter than the run. Also shortened the chaos timing block, the greenboard platform override comment and the kill-edgeserver.sh header, each of which restated what the code below it already showed. Left alone the comments that explain something genuinely non-obvious and would otherwise invite the bug back: systemd respawning Edge Server after a signal, Python 3.13's strict X.509 rules, the __overall_fail latch interacting with upload() and record_upgrade_step differently, and why perform_cleanup is excluded from the topology-marker check. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.