PS-11393 - Add more group replication test cases

[tplavcic]

PS-11393: This is adding following test cases in PS GR framework requested in this ticket:
test_secondary_isolation_ist.py - secondary network-partitioned, rejoins via IST
test_secondary_isolation_sst.py - same, binlogs purged so it rejoins via clone/SST
test_primary_isolation_failover.py - primary partitioned: automatic failover
test_majority_loss.py - both secondaries cut off: quorum loss, no writes
test_equal_partition.py - 4-node 2-2 split: split-brain prevention

PS-11378: It is also adding part of the cases requested in this ticket:

#	Scenario	Covered by
2	Kill the node, confirm a secondary is promoted	test_primary_shutdown_failover.py::test_primary_shutdown_failover_and_recovery[kill-router] / [kill-haproxy]
3	Kill node, join a 4th, then join the killed node back	test_rejoin_after_scale_up.py::test_rejoin_after_scale_up
4	Kill 2 secondaries of 3 → primary majority loss → unblock → load	test_majority_loss_kill.py::test_majority_loss_by_kill[secondaries-router] / [secondaries-haproxy]
5	Kill primary + 1 secondary → secondary majority loss → unblock → promoted → load	test_majority_loss_kill.py::test_majority_loss_by_kill[primary_and_secondary-router] / [primary_and_secondary-haproxy]
11	Primary, after both → expels, auto-rejoins, new primary	test_primary_isolation_failover.py::test_primary_isolation_failover

Scenario 11 was already exercised by the existing primary-isolation test — it sets
group_replication_unreachable_majority_timeout=30 and isolates past both timeouts —
but the auto-rejoin was only logged. This PR turns it into an assertion, so the
group_replication_autorejoin_tries path is now actually enforced.

[Copilot]

Pull request overview

Adds a set of Group Replication (GR) network-partition and quorum-loss test cases to the Percona Server GR test framework, plus supporting framework and documentation updates to reliably simulate partitions, observe membership from surviving nodes, and recover groups after quorum loss.

Changes:

• Introduces new GR partition tests covering secondary IST/SST recovery, primary isolation failover, majority loss, and 4-node equal split behavior.
• Extends the GR helper framework with network-partition primitives (disconnect/connect, link sever/restore), quorum recovery helpers, and improved observer selection for membership polling.
• Updates docs/README and refines proxy startup/refresh behavior to account for partitions and IP changes.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
test_scripts/ps/group-replication/test_secondary_isolation_sst.py	New test: isolate a secondary, purge binlogs on donors, validate clone/SST recovery and data consistency.
test_scripts/ps/group-replication/test_secondary_isolation_ist.py	New test: isolate a secondary, heal before binlog purge, validate IST recovery and data consistency.
test_scripts/ps/group-replication/test_primary_shutdown_failover.py	Renames/clarifies the shutdown-based failover test and points to the new isolation variant.
test_scripts/ps/group-replication/test_primary_isolation_failover.py	New test: isolate the primary via network partition, assert automatic failover, write refusal on isolated primary, and rejoin behavior.
test_scripts/ps/group-replication/test_majority_loss.py	New test: quorum loss (3-node), assert writes refused, then force/recover membership and validate no leaked writes.
test_scripts/ps/group-replication/test_equal_partition.py	New test: 4-node 2–2 split using in-container route blackholing, assert no split brain and operator recovery via force_members().
test_scripts/ps/group-replication/README.md	Documents new tests and adds detailed guidance for partition/quorum scenarios and helper APIs.
test_scripts/ps/group-replication/group_replication_helper.py	Adds partition/heal/force-members helpers, observer selection for polling, proxy refresh changes, and other test-enabling utilities.
test_scripts/ps/group-replication/docker_helper.py	Adds capabilities/network helpers (cap_add, network connect/disconnect, container state/IP) used by new partition tests.
test_scripts/ps/group-replication/conftest.py	Extends fixture parameterization to support variable cluster sizes (e.g., 4-node equal partition test).

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.

Suppressed comments (2)

Previously missed (2) — in code that hasn't changed since the last review.

test_scripts/ps/group-replication/group_replication_helper.py:445

• wait_members_unreachable() returns the last observed membership map on timeout (it does not guarantee members are UNREACHABLE), but the docstring reads like it always waits until the condition is met. This can mislead callers into assuming the condition held without checking the return value.

    def wait_members_unreachable(
        self, names: list[str], node: str, timeout: int = 60
    ) -> dict[str, tuple[str, str]]:
        """Wait until every named member is seen as UNREACHABLE from `node`; return that view."""

test_scripts/ps/group-replication/README.md:576

• README says wait_members_unreachable() will "block until" members are UNREACHABLE, but the helper returns the last view when the timeout expires. Update the wording so future test authors know they still need to assert on the returned view.

- `gr_cluster.wait_members_unreachable(nodes, node=...)` — block until the given members are
  seen as `UNREACHABLE` from an observer node, and return that view.

[Copilot]

🟡 Changes recommended

force_members() can return before the forced configuration actually settles (ONLINE count can already match), risking hangs/misleading results in the new quorum-recovery tests.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

test_scripts/ps/group-replication/test_primary_shutdown_failover.py:43

• The assertion message uses f"{fault}ed", which renders as "stoped" when fault=="stop". Use an explicit past-tense mapping so the failure output is readable and correctly spelled.

• Files reviewed: 13/13 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

One assertion message constructs an incorrect past tense (“stoped”) and should be fixed to keep test failures clear and correctly spelled.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

test_scripts/ps/group-replication/test_primary_shutdown_failover.py:46

• The assertion messages build past-tense verbs via f"{fault}ed", which yields the misspelling "stoped" when fault=="stop" and reads awkwardly for the follow-up assertion. Use an explicit mapping (or conditional) for human-readable wording so failures are clearer and correctly spelled.

• Files reviewed: 13/13 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

Sysbench teardown/cleanup can still hang due to missing timeout plumbing, which risks making failing or flaky partition tests stall the suite and obscure root causes.

Review details

Suppressed comments (2)

Previously missed (2) — in code that hasn't changed since the last review.

test_scripts/ps/group-replication/sysbench_helper.py:51

• Sysbench._exec() (and therefore cleanup/run) has no way to bound how long sysbench can block, because DockerHelper.run() isn’t called with a timeout. This can make failures in partition/quorum tests much harder to debug if sysbench hangs instead of returning promptly. It would be safer to add an optional timeout parameter here and pass it down into DockerHelper.run().

    def _exec(self, command: list[str], check: bool = True):
        return self.docker.run(
            self.image,
            name=self.name,
            network=self.network,

test_scripts/ps/group-replication/conftest.py:142

• The sysbench fixture’s teardown calls sb.cleanup(), but Sysbench._exec ultimately uses DockerHelper.run() without any timeout. If a test fails mid-partition or leaves the proxy unusable, teardown can block for a long time (or indefinitely) and obscure the original failure. Consider plumbing a timeout through Sysbench._exec/cleanup and DockerHelper.run(), and use a short timeout here in teardown.

• Files reviewed: 14/14 changed files
• Comments generated: 0 new
• Review effort level: Lite

[saikumar-vs]

1. test_equal_partition.py — deterministic failure (NET_ADMIN doesn't propagate to non-root docker exec)

File: test_scripts/ps/group-replication/group_replication_helper.py (_peer_route(), used by sever_link()/restore_link())

sever_link()'s underlying route add -host reject command requires root privileges (or ambient capabilities), but docker_helper.py's exec_command() (which _peer_route() calls) never passes -u root — so it runs as whatever user docker exec defaults to, which for percona/percona-server:8.4 is the non-root mysql user (uid 1001).

--cap-add NET_ADMIN on container creation only populates the bounding capability set; it is not inherited by a non-root process without ambient capabilities or file capabilities on the route binary. So the capability is silently ineffective here.

Reproduced directly, outside the test framework:

$ docker run --rm --cap-add NET_ADMIN percona/percona-server:8.4 sh -c "id; route add -host 8.8.8.8 reject; echo exit=$?"
uid=1001(mysql) gid=1001(mysql) groups=1001(mysql)
SIOCADDRT: Operation not permitted
exit=7

$ docker run --rm --cap-add NET_ADMIN -u root percona/percona-server:8.4 sh -c "id; route add -host 8.8.8.8 reject; echo exit=$?"
uid=0(root) gid=0(root) groups=0(root)
exit=0

And confirmed via the actual test run:

FAILED test_equal_partition.py::test_equal_partition[router] - RuntimeError: docker exec ps0-1 sh -c route add -host 172.18.0.4 reject failed (exit 7)
stderr: SIOCADDRT: Operation not permitted

This means test_equal_partition.py fails on every run in any standard Docker setup — this isn't environment-specific flakiness, it's structural.

Fix: have exec_command() (or a dedicated variant used only by _peer_route()) run as root, e.g. self._run(["exec", "-u", "root", name, "sh", "-c", command], check=check). Confirmed that fixes it: the same route add command succeeds immediately with -u root.

2. test_majority_loss.py[router] — flaky recovery, failed 3/3 live runs

File: test_scripts/ps/group-replication/group_replication_helper.py (heal_group() / force_members()), exercised from test_majority_loss.py

The recovery sequence reconnects both secondaries via heal_group(secondaries), then immediately calls force_members([seed], node=seed) targeting only one of them. This races XCOM's "majority of members must be unreachable" precondition: once both secondaries are physically back on the network, they can re-establish mutual contact with each other before (or during) the force call — at which point, from seed's perspective, only the still-isolated primary (1 of 3) is unreachable, not a majority (2 of 3).

Ran the test 3 times back to back; it failed all 3, with two different symptoms:

Run 1 (87s) and run 3 (86s): force rejected outright —
ERROR 4118 (HY000): The 'SET GLOBAL group_replication_force_members=ps0-2:33061' command
encountered a failure. The group_replication_force_members can only be updated when Group
Replication is running and majority of the members are unreachable.
Run 2 (207s): force accepted (confirmed via direct query that the target still saw its peer as stale-UNREACHABLE at that moment), but the view change then hung and timed out —
ERROR 4118 (HY000): ... Timeout on wait for view after setting group_replication_force_members.

Fix direction: don't reconnect both secondaries before forcing membership on one of them. Either (a) force membership on the seed before calling heal_group() for the other secondary (forcing down to a single node the way test_majority_loss_kill.py does, then bringing the second one back via restart_group_replication() afterward), or (b) after heal_group(), poll until the seed's view of its peer has genuinely settled (reachable or still-suspected, consistently) before attempting the force, rather than firing it immediately.

3. test_majority_loss_kill.py — leak check skips NOTE_PROXY and gates NOTE_DIRECT backwards

File: test_scripts/ps/group-replication/test_majority_loss_kill.py:139-142

if not survivor_was_primary:
for node in gr_cluster.active_nodes:
leaked = _count_note(gr_cluster, node, NOTE_DIRECT)
assert leaked == "0", f"a refused write leaked onto {node} ({leaked} rows)"

This check only runs when survivor_was_primary is False (the primary_and_secondary parametrization, where the surviving secondary refuses the write outright — a case where a leak was never possible to begin with). In the secondaries parametrization (survivor_was_primary=True), the surviving primary parks the write awaiting consensus rather than refusing it — exactly the case where it can commit later once force_members() unblocks the primary, per the file's own comment at lines 135-138. That's precisely the parametrization where the check is skipped. NOTE_PROXY (the write attempted through the proxy in the same window) has no post-recovery leak check anywhere in the file, in either parametrization.

Empirically confirmed, twice — instrumented the test with extra (non-assertive) logging and ran [secondaries-router] twice:

[DIAG] post-recovery count of 'no-quorum-direct' on ps0-1: 1
[DIAG] post-recovery count of 'no-quorum-proxy' on ps0-1: 1
[DIAG] post-recovery count of 'no-quorum-direct' on ps0-2: 1
[DIAG] post-recovery count of 'no-quorum-proxy' on ps0-2: 1
[DIAG] post-recovery count of 'no-quorum-direct' on ps0-3: 1
[DIAG] post-recovery count of 'no-quorum-proxy' on ps0-3: 1

Both writes — issued with zero quorum — end up committed and replicated to all three nodes, and the test still reports PASSED both times, since nothing checks for it.

Fix: move the leak check outside the if not survivor_was_primary: guard (or invert it — the parked-write case is exactly where it's needed), and extend it to cover NOTE_PROXY as well as NOTE_DIRECT. If a parked write is expected to sometimes legitimately commit post-recovery (per the existing comment), the check needs to be reframed around what invariant actually matters here — e.g. consistency across nodes rather than "must never exist" — but as written today it silently doesn't check the one case where a real leak is possible.

4. test_secondary_isolation_ist.py / test_secondary_isolation_sst.py — missing refresh_proxy() after heal_node()

Files: test_scripts/ps/group-replication/test_secondary_isolation_ist.py:69, test_scripts/ps/group-replication/test_secondary_isolation_sst.py (equivalent line)

Both tests reconnect the isolated secondary via heal_node() but never call refresh_proxy() afterward — unlike test_majority_loss.py, test_primary_isolation_failover.py, test_primary_shutdown_failover.py, and test_rejoin_after_scale_up.py, which all call it after reconnecting a node. Per refresh_proxy()'s own docstring, reconnecting a container can give it a new IP, and HAProxy resolves backend addresses once at config-parse time — so a stale address silently drops the healed secondary out of the read backend. Both tests parametrize over ["router", "haproxy"], but neither calls gr_cluster.ro_endpoint() to catch this; every other post-heal check (clone_status, gtid_executed/gtid_subset, verify_checksums) connects directly per-node, bypassing the proxy entirely.

Note on severity: this is real per the documented mechanism, but in two separate live runs in my environment, Docker's bridge IPAM happened to reassign the isolated node the same IP both times (no other network churn during the single-node disconnect/reconnect), so the bug didn't actually manifest in those runs. Its practical trigger rate likely depends on environment/IPAM behavior (e.g. more concurrent churn under pytest-xdist, or a different container runtime) — worth fixing for robustness even though it may not fail reliably in every CI environment.

Fix: add gr_cluster.refresh_proxy() (and ideally a wait_proxy_ready() + a read-through-proxy assertion) after heal_node() in both files, matching the pattern already used elsewhere in the suite.

[tplavcic]

@saikumar-vs I made some fixes so please review (re-run) again.
Most of the issues were related to the fact that I was running these tests on Podman and it handles some networking stuff a bit differently than Docker.
So I have re-runned them with Docker on QA machine and fixed the issues.

1. NET_ADMIN doesn't propagate to non-root docker exec — valid, fixed in 7aeb9b8.

Confirmed, and the mechanism explains why it passed for me: podman grants --cap-add
ambiently, Docker doesn't.

So uid 1001 already holds the capability on podman and the route succeeds; on Docker the
ambient set is empty and it fails with EPERM exactly as you reproduced.

Fixed as suggested: exec_command() takes a user parameter, _peer_route() passes
user="root". Verified -u root also works on podman (no regression) and
test_equal_partition.py is green on both proxies. The other exec_command() caller
(HAProxy stats socket) needs no capability and stays non-root.

---

2. test_majority_loss[router] flaky recovery — valid.

Root cause: reconnecting a secondary can restore a 2-of-3 quorum on its own, because the
old primary stays on the network throughout (only the secondaries are cut off) and rejoins as
soon as a secondary returns — provided XCOM still has a usable address for it. Whether it does
comes down to whether the runtime handed the container its old IP back, since XCOM does not
follow a peer to a new address: Docker usually does, podman usually does not. The test then
forced a membership that either had nothing to do (your runs 1 and 3, refused with 4118) or
raced a view change (run 2, accepted then timed out).

Fixed: after reconnecting, wait for the seed's view to stop changing
(wait_view_stable(), three consecutive identical reads) and only then decide —
if quorum genuinely did not come back. Deciding immediately is what raced the for
force_members() also now reports both of error 4118's conditions on failure, so a
recurrence is self-diagnosing.

Both branches verified locally:

	outcome
podman, natural (addresses change)	`still has no quorum, forcing the membersh
Docker-simulated (addresses pinned on reconnect)	regained quorum without forcing → 1 passed

---

3. test_majority_loss_kill.py leak check — gap is real, fixed in 3107ceb, with one
correction.

The guard wasn't backwards in effect — it asserted absence only where absence is guaranteed.
In the secondaries case the survivor is the primary, so the write parks awaiting
consensus and force_members() legitimately lets it commit; your [DIAG] counts of 1 on all
three nodes are that, not a leak. Moving the check outside the guard as first suggested would
have failed both [secondaries-*] cases.

What was genuinely wrong, and is fixed: NOTE_PROXY was never checked (including in
primary_and_secondary, where it is refused and absence is assertable), and the gate keyed
off the survivor's role instead of how the write failed — which is why NOTE_PROXY got missed.
Probes are now classified by observed failure mode (returncode == 124 = parked): refused →
asserted absent everywhere; parked → asserted consistent across nodes, which is your
"reframe around consistency".

---

4. Missing refresh_proxy() after heal_node() — valid, fixed in 48e42dc, and it was
live here.

Your severity note was the one thing that turned out understated: on podman a single-node
reconnect does change the address (10.89.0.3 → 10.89.0.5), so this was failin
my environment, not just theoretically. A negative control confirmed it — assertion in place,
refresh_proxy() removed:

ps0-2 is not serving reads after rejoining — the haproxy read endpoint
only ever answered from ['ps0-1', 'ps0-3']

Fixed as suggested, including the read-through-proxy assertion: refresh_proxy() +
wait_proxy_ready() + a read-path check in both tests, placed after the clone/GT
so a recovery failure still reports as one. The probe loop became
GroupReplication.wait_node_serving_reads() rather than a third copy, and
test_primary_isolation_failover.py's inline version was refactored onto it.
test_equal_partition.py stays the documented exception — sever_link() uses reject routes
and leaves addresses untouched.