From d3da16be6173c43124259534dc1e710ab3d697df Mon Sep 17 00:00:00 2001 From: kaushikpuneet07 Date: Fri, 25 Sep 2026 12:28:50 +0530 Subject: [PATCH 1/2] GR tests: match router image to server version, surface docker probe errors - Default ROUTER_IMAGE is now derived from SERVER_IMAGE (/percona-server:X.Y.Z -> /percona-mysql-router:X.Y.Z), falling back to percona/percona-mysql-router:8.4. A fixed 8.4 router cannot bootstrap against a 9.7 cluster and crash-looped, failing the [router] tests. - mysqlsh_available() only returns False for a missing binary (exit 127); other docker failures (e.g. socket permission denied) now raise with the real error instead of skipping as "mysqlsh not available". - README: document the derived router default. Co-Authored-By: Claude Opus 5.5 (1M context) --- test_scripts/ps/group-replication/README.md | 9 ++++- .../group_replication_helper.py | 40 ++++++++++++++++++- 2 files changed, 46 insertions(+), 3 deletions(-) diff --git a/test_scripts/ps/group-replication/README.md b/test_scripts/ps/group-replication/README.md index bc3d8d5..ac260b7 100644 --- a/test_scripts/ps/group-replication/README.md +++ b/test_scripts/ps/group-replication/README.md @@ -355,10 +355,17 @@ internal registry mirror without editing code. |--------------------|----------------|-------------------------------------| | `SERVER_IMAGE` | Percona Server | `percona/percona-server:8.4` | | `HAPROXY_IMAGE` | HAProxy | `percona/haproxy:2` | -| `ROUTER_IMAGE` | MySQL Router | `percona/percona-mysql-router:8.4` | +| `ROUTER_IMAGE` | MySQL Router | derived from `SERVER_IMAGE`¹ | | `XTRABACKUP_IMAGE` | XtraBackup | `percona/percona-xtrabackup:8.4` | | `SYSBENCH_IMAGE` | sysbench | `pingwinator/sysbench:latest` | +¹ Router can't bootstrap against a newer server, so when `ROUTER_IMAGE` is unset it +follows the server version: `/percona-server:` → `/percona-mysql-router:` +(e.g. `perconalab/percona-server:9.7.2` → `perconalab/percona-mysql-router:9.7.2`). +Build suffixes are dropped; images that don't match that pattern fall back to +`percona/percona-mysql-router:8.4`. Set `ROUTER_IMAGE` explicitly if the matching tag +isn't published. + ```bash SERVER_IMAGE=percona/percona-server:8.4.5 pytest -v test_basic.py ``` diff --git a/test_scripts/ps/group-replication/group_replication_helper.py b/test_scripts/ps/group-replication/group_replication_helper.py index 98aa363..566d50f 100644 --- a/test_scripts/ps/group-replication/group_replication_helper.py +++ b/test_scripts/ps/group-replication/group_replication_helper.py @@ -1,5 +1,6 @@ import logging import os +import re import shlex import time from urllib.parse import quote @@ -9,6 +10,27 @@ _logger = logging.getLogger("GR") +_DEFAULT_ROUTER_IMAGE = "percona/percona-mysql-router:8.4" + + +def default_router_image(server_image: str) -> str: + """Pick a MySQL Router image matching the server's version. + + Router refuses to bootstrap against a cluster whose server/metadata is newer than + itself, so a fixed 8.4 router crash-loops in front of e.g. a 9.7 server. Map + /percona-server: to /percona-mysql-router:, keeping + the registry/namespace (percona vs perconalab publish different tags) and dropping any + build suffix, which differs between the server and router tags. Anything that doesn't + fit that shape (other repos, digests, "latest") falls back to the 8.4 default. + """ + repo, sep, tag = server_image.rpartition(":") + if not sep or "/" in tag or "@" in server_image: + return _DEFAULT_ROUTER_IMAGE + m = re.match(r"\d+\.\d+(?:\.\d+)?", tag) + if not m or not repo.endswith("percona-server"): + return _DEFAULT_ROUTER_IMAGE + return f"{repo[: -len('percona-server')]}percona-mysql-router:{m.group()}" + class GroupReplication: def __init__( @@ -66,7 +88,9 @@ def __init__( self.single_primary = single_primary self.start_on_boot = start_on_boot self.mysql_router = mysql_router - self.router_image = router_image or os.environ.get("ROUTER_IMAGE") or "percona/percona-mysql-router:8.4" + self.router_image = ( + router_image or os.environ.get("ROUTER_IMAGE") or default_router_image(self.server_image) + ) self.router_name = f"{node_prefix}router" self.router_rw_port = router_rw_port self.router_ro_port = router_ro_port @@ -1205,6 +1229,11 @@ def mysqlsh_available(self) -> bool: Cluster bootstrap and instance-add go through mysqlsh's AdminAPI (see create()), so a server build without it cannot run this suite. Probed with a throwaway --rm container running `mysqlsh --version` (no node startup, no connection). + + Only a missing binary (exit 127, how docker/podman report "executable file not + found") returns False. Any other failure (daemon down, permission denied on the + socket, image pull error) raises with the real error instead of being misreported + as a missing mysqlsh. """ result = self.docker.run( image=self.server_image, @@ -1212,7 +1241,14 @@ def mysqlsh_available(self) -> bool: command=["--version"], check=False, ) - return result.ok + if result.ok: + return True + if result.returncode == 127: + return False + raise RuntimeError( + f"could not probe mysqlsh in server image {self.server_image!r} " + f"(exit {result.returncode}): {(result.stderr or result.stdout).strip()}" + ) def create(self) -> None: """Create the network and nodes, bootstrap the cluster, add instances, and persist GR settings.""" From 9b45941e7c4591da3dac2638aa7a1bad90ef44fa Mon Sep 17 00:00:00 2001 From: kaushikpuneet07 Date: Fri, 25 Sep 2026 15:42:09 +0530 Subject: [PATCH 2/2] GR tests: match XtraBackup image to server version, keep node logs on failure - Default XTRABACKUP_IMAGE is now derived from SERVER_IMAGE (/percona-server:X.Y.Z -> /percona-xtrabackup:X.Y). XtraBackup refuses servers of another X.Y ("Unsupported server version: '9.7.2-2'"), so the fixed 8.4 default failed test_backup_restore on 9.7. - Move the router/xtrabackup image derivation into generic_helper.companion_image(). - On a failed test, attach each node's container state and docker logs tail to the pytest report; teardown otherwise destroys all server-side evidence. - README: document the derived defaults. Co-Authored-By: Claude Opus 5.5 (1M context) --- test_scripts/ps/group-replication/README.md | 16 ++++++---- test_scripts/ps/group-replication/conftest.py | 20 ++++++++++++ .../ps/group-replication/docker_helper.py | 5 +++ .../ps/group-replication/generic_helper.py | 28 ++++++++++++++-- .../group_replication_helper.py | 32 ++++--------------- .../ps/group-replication/xtrabackup_helper.py | 12 ++++++- 6 files changed, 77 insertions(+), 36 deletions(-) diff --git a/test_scripts/ps/group-replication/README.md b/test_scripts/ps/group-replication/README.md index ac260b7..9b79510 100644 --- a/test_scripts/ps/group-replication/README.md +++ b/test_scripts/ps/group-replication/README.md @@ -356,15 +356,17 @@ internal registry mirror without editing code. | `SERVER_IMAGE` | Percona Server | `percona/percona-server:8.4` | | `HAPROXY_IMAGE` | HAProxy | `percona/haproxy:2` | | `ROUTER_IMAGE` | MySQL Router | derived from `SERVER_IMAGE`¹ | -| `XTRABACKUP_IMAGE` | XtraBackup | `percona/percona-xtrabackup:8.4` | +| `XTRABACKUP_IMAGE` | XtraBackup | derived from `SERVER_IMAGE`¹ | | `SYSBENCH_IMAGE` | sysbench | `pingwinator/sysbench:latest` | -¹ Router can't bootstrap against a newer server, so when `ROUTER_IMAGE` is unset it -follows the server version: `/percona-server:` → `/percona-mysql-router:` -(e.g. `perconalab/percona-server:9.7.2` → `perconalab/percona-mysql-router:9.7.2`). -Build suffixes are dropped; images that don't match that pattern fall back to -`percona/percona-mysql-router:8.4`. Set `ROUTER_IMAGE` explicitly if the matching tag -isn't published. +¹ Router and XtraBackup refuse to work against a newer server, so when unset they follow +the server version, keeping its registry/namespace: +`/percona-server:` → `/percona-mysql-router:` and +`/percona-xtrabackup:` (e.g. `perconalab/percona-server:9.7.2` → +`perconalab/percona-mysql-router:9.7.2` and `perconalab/percona-xtrabackup:9.7`). +Build suffixes are dropped; images that don't match that pattern fall back to the `8.4` +tags. Set `ROUTER_IMAGE` / `XTRABACKUP_IMAGE` explicitly if the matching tag isn't +published. ```bash SERVER_IMAGE=percona/percona-server:8.4.5 pytest -v test_basic.py diff --git a/test_scripts/ps/group-replication/conftest.py b/test_scripts/ps/group-replication/conftest.py index 045c9a9..946810f 100644 --- a/test_scripts/ps/group-replication/conftest.py +++ b/test_scripts/ps/group-replication/conftest.py @@ -30,6 +30,25 @@ } +@pytest.hookimpl(hookwrapper=True) +def pytest_runtest_makereport(item, call): + """On a failed setup/call, attach each node's container state and log tail to the report. + + Teardown destroys the containers, so without this a failure leaves no server-side + evidence (e.g. whether mysqld crashed and restart=always brought it back). + """ + outcome = yield + report = outcome.get_result() + cluster = item.funcargs.get("gr_cluster") if hasattr(item, "funcargs") else None + if report.when == "teardown" or not report.failed or not isinstance(cluster, GroupReplication): + return + for name in [*cluster.containers, cluster.proxy_name]: + if not name or not cluster.docker.container_exists(name): + continue + state = cluster.docker.container_state(name) + report.sections.append((f"docker logs {name} ({state})", cluster.docker.logs(name))) + + def _worker_id(request) -> str: """Return the pytest-xdist worker id (e.g. 'gw0'), or '0' when running serially. @@ -157,6 +176,7 @@ def xtrabackup(request, gr_cluster): gr_cluster.docker, network=gr_cluster.network, backup_volume=backup_volume, + server_image=gr_cluster.server_image, root_password=gr_cluster.root_password, name_prefix=f"xtrabackup_{prefix}", log=gr_cluster.log, diff --git a/test_scripts/ps/group-replication/docker_helper.py b/test_scripts/ps/group-replication/docker_helper.py index e265db1..49453c1 100644 --- a/test_scripts/ps/group-replication/docker_helper.py +++ b/test_scripts/ps/group-replication/docker_helper.py @@ -359,6 +359,11 @@ def container_state(self, name: str) -> str: ) return result.stdout.strip() if result.ok else "" + def logs(self, name: str, tail: int = 80) -> str: + """Return the last `tail` lines of a container's output (stdout and stderr), or "".""" + result = self._run(["logs", "--tail", str(tail), name], check=False) + return (result.stdout + result.stderr).strip() if result.ok else "" + def container_exists(self, name: str) -> bool: """Return True if a container with the exact given name exists (running or stopped).""" result = self._run( diff --git a/test_scripts/ps/group-replication/generic_helper.py b/test_scripts/ps/group-replication/generic_helper.py index 8e88e35..191e654 100644 --- a/test_scripts/ps/group-replication/generic_helper.py +++ b/test_scripts/ps/group-replication/generic_helper.py @@ -1,10 +1,12 @@ -"""Small, dependency-free string-escaping helpers shared across the suite. +"""Small, dependency-free helpers shared across the suite. -These keep dynamic values (database names, credentials, identifiers) from breaking — or -being injectable into — the SQL, mysqlsh JS, and connection strings the helpers build. +The escaping helpers keep dynamic values (database names, credentials, identifiers) from +breaking — or being injectable into — the SQL, mysqlsh JS, and connection strings the +helpers build. companion_image() picks tool images that match the server version. """ import json +import re def js_str(value: str) -> str: @@ -31,3 +33,23 @@ def sql_str(value: str) -> str: def sql_ident(name: str) -> str: """Quote a MySQL identifier (e.g. schema/table), escaping embedded backticks.""" return "`" + name.replace("`", "``") + "`" + + +def companion_image(server_image: str, repo_name: str, parts: int, default: str) -> str: + """Derive a tool image (router, xtrabackup) matching the server image's version. + + Router and XtraBackup both refuse to work against a server newer than themselves, so a + fixed 8.4 default breaks as soon as SERVER_IMAGE points at e.g. 9.7. Map + /percona-server: to /:, keeping the registry/namespace (percona vs perconalab publish different + tags) and dropping any build suffix, which differs between server and tool tags. + Anything that doesn't fit that shape (other repos, digests, "latest") gets `default`. + """ + repo, sep, tag = server_image.rpartition(":") + if not sep or "/" in tag or "@" in server_image or not repo.endswith("percona-server"): + return default + m = re.match(r"\d+(?:\.\d+){1,2}", tag) + if not m: + return default + version = ".".join(m.group().split(".")[:parts]) + return f"{repo[: -len('percona-server')]}{repo_name}:{version}" diff --git a/test_scripts/ps/group-replication/group_replication_helper.py b/test_scripts/ps/group-replication/group_replication_helper.py index 566d50f..330b295 100644 --- a/test_scripts/ps/group-replication/group_replication_helper.py +++ b/test_scripts/ps/group-replication/group_replication_helper.py @@ -1,37 +1,14 @@ import logging import os -import re import shlex import time from urllib.parse import quote from docker_helper import DockerHelper -from generic_helper import js_str, sql_ident, sql_str +from generic_helper import companion_image, js_str, sql_ident, sql_str _logger = logging.getLogger("GR") -_DEFAULT_ROUTER_IMAGE = "percona/percona-mysql-router:8.4" - - -def default_router_image(server_image: str) -> str: - """Pick a MySQL Router image matching the server's version. - - Router refuses to bootstrap against a cluster whose server/metadata is newer than - itself, so a fixed 8.4 router crash-loops in front of e.g. a 9.7 server. Map - /percona-server: to /percona-mysql-router:, keeping - the registry/namespace (percona vs perconalab publish different tags) and dropping any - build suffix, which differs between the server and router tags. Anything that doesn't - fit that shape (other repos, digests, "latest") falls back to the 8.4 default. - """ - repo, sep, tag = server_image.rpartition(":") - if not sep or "/" in tag or "@" in server_image: - return _DEFAULT_ROUTER_IMAGE - m = re.match(r"\d+\.\d+(?:\.\d+)?", tag) - if not m or not repo.endswith("percona-server"): - return _DEFAULT_ROUTER_IMAGE - return f"{repo[: -len('percona-server')]}percona-mysql-router:{m.group()}" - - class GroupReplication: def __init__( self, @@ -89,7 +66,12 @@ def __init__( self.start_on_boot = start_on_boot self.mysql_router = mysql_router self.router_image = ( - router_image or os.environ.get("ROUTER_IMAGE") or default_router_image(self.server_image) + router_image + or os.environ.get("ROUTER_IMAGE") + # Router can't bootstrap against a newer server: follow the server's X.Y.Z. + or companion_image( + self.server_image, "percona-mysql-router", 3, "percona/percona-mysql-router:8.4" + ) ) self.router_name = f"{node_prefix}router" self.router_rw_port = router_rw_port diff --git a/test_scripts/ps/group-replication/xtrabackup_helper.py b/test_scripts/ps/group-replication/xtrabackup_helper.py index 880a438..2f57b6f 100644 --- a/test_scripts/ps/group-replication/xtrabackup_helper.py +++ b/test_scripts/ps/group-replication/xtrabackup_helper.py @@ -3,6 +3,7 @@ from collections.abc import Callable from docker_helper import DockerHelper +from generic_helper import companion_image class XtraBackup: @@ -20,6 +21,7 @@ def __init__( network: str, backup_volume: str, image: str | None = None, + server_image: str | None = None, platform: str | None = None, root_password: str = "rootpass", name_prefix: str = "xtrabackup", @@ -28,7 +30,15 @@ def __init__( self.docker = docker self.network = network self.backup_volume = backup_volume - self.image = image or os.environ.get("XTRABACKUP_IMAGE") or "percona/percona-xtrabackup:8.4" + # XtraBackup only backs up a server of its own X.Y ("Please use Percona XtraBackup + # 9.7 for this database"), so follow the server image's major.minor. + self.image = ( + image + or os.environ.get("XTRABACKUP_IMAGE") + or companion_image( + server_image or "", "percona-xtrabackup", 2, "percona/percona-xtrabackup:8.4" + ) + ) self.platform = platform self.root_password = root_password self.name_prefix = name_prefix