From 58fc973c31e5aa2b07cf038a4c37532aef1f8a0f Mon Sep 17 00:00:00 2001 From: Dragomir Penev <6687393+dragomirp@users.noreply.github.com> Date: Mon, 25 Nov 2024 18:50:56 +0200 Subject: [PATCH] [DPE-3684] Factor out minor changes from the raft reinit PR (#681) * Factor out minor changes from the raft reinit PR * Bump ruff * Missed namespace change --- poetry.lock | 80 ++++++++++++---- pyproject.toml | 6 +- src/charm.py | 26 ++++-- src/cluster.py | 37 ++++---- tests/integration/ha_tests/helpers.py | 53 +++++++++-- tests/integration/ha_tests/test_smoke.py | 6 +- tests/unit/test_cluster.py | 114 ++++++++++++++++++----- 7 files changed, 234 insertions(+), 88 deletions(-) diff --git a/poetry.lock b/poetry.lock index 7b4d2ac24f..8c9b49e840 100644 --- a/poetry.lock +++ b/poetry.lock @@ -1449,6 +1449,36 @@ files = [ {file = "protobuf-4.25.5.tar.gz", hash = "sha256:7f8249476b4a9473645db7f8ab42b02fe1488cbe5fb72fddd445e0665afd8584"}, ] +[[package]] +name = "psutil" +version = "6.1.0" +description = "Cross-platform lib for process and system monitoring in Python." +optional = false +python-versions = "!=3.0.*,!=3.1.*,!=3.2.*,!=3.3.*,!=3.4.*,!=3.5.*,>=2.7" +files = [ + {file = "psutil-6.1.0-cp27-cp27m-macosx_10_9_x86_64.whl", hash = "sha256:ff34df86226c0227c52f38b919213157588a678d049688eded74c76c8ba4a5d0"}, + {file = "psutil-6.1.0-cp27-cp27m-manylinux2010_i686.whl", hash = "sha256:c0e0c00aa18ca2d3b2b991643b799a15fc8f0563d2ebb6040f64ce8dc027b942"}, + {file = "psutil-6.1.0-cp27-cp27m-manylinux2010_x86_64.whl", hash = "sha256:000d1d1ebd634b4efb383f4034437384e44a6d455260aaee2eca1e9c1b55f047"}, + {file = "psutil-6.1.0-cp27-cp27mu-manylinux2010_i686.whl", hash = "sha256:5cd2bcdc75b452ba2e10f0e8ecc0b57b827dd5d7aaffbc6821b2a9a242823a76"}, + {file = "psutil-6.1.0-cp27-cp27mu-manylinux2010_x86_64.whl", hash = "sha256:045f00a43c737f960d273a83973b2511430d61f283a44c96bf13a6e829ba8fdc"}, + {file = "psutil-6.1.0-cp27-none-win32.whl", hash = "sha256:9118f27452b70bb1d9ab3198c1f626c2499384935aaf55388211ad982611407e"}, + {file = "psutil-6.1.0-cp27-none-win_amd64.whl", hash = "sha256:a8506f6119cff7015678e2bce904a4da21025cc70ad283a53b099e7620061d85"}, + {file = "psutil-6.1.0-cp36-abi3-macosx_10_9_x86_64.whl", hash = "sha256:6e2dcd475ce8b80522e51d923d10c7871e45f20918e027ab682f94f1c6351688"}, + {file = "psutil-6.1.0-cp36-abi3-macosx_11_0_arm64.whl", hash = "sha256:0895b8414afafc526712c498bd9de2b063deaac4021a3b3c34566283464aff8e"}, + {file = "psutil-6.1.0-cp36-abi3-manylinux_2_12_i686.manylinux2010_i686.manylinux_2_17_i686.manylinux2014_i686.whl", hash = "sha256:9dcbfce5d89f1d1f2546a2090f4fcf87c7f669d1d90aacb7d7582addece9fb38"}, + {file = "psutil-6.1.0-cp36-abi3-manylinux_2_12_x86_64.manylinux2010_x86_64.manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:498c6979f9c6637ebc3a73b3f87f9eb1ec24e1ce53a7c5173b8508981614a90b"}, + {file = "psutil-6.1.0-cp36-abi3-manylinux_2_17_aarch64.manylinux2014_aarch64.whl", hash = "sha256:d905186d647b16755a800e7263d43df08b790d709d575105d419f8b6ef65423a"}, + {file = "psutil-6.1.0-cp36-cp36m-win32.whl", hash = "sha256:6d3fbbc8d23fcdcb500d2c9f94e07b1342df8ed71b948a2649b5cb060a7c94ca"}, + {file = "psutil-6.1.0-cp36-cp36m-win_amd64.whl", hash = "sha256:1209036fbd0421afde505a4879dee3b2fd7b1e14fee81c0069807adcbbcca747"}, + {file = "psutil-6.1.0-cp37-abi3-win32.whl", hash = "sha256:1ad45a1f5d0b608253b11508f80940985d1d0c8f6111b5cb637533a0e6ddc13e"}, + {file = "psutil-6.1.0-cp37-abi3-win_amd64.whl", hash = "sha256:a8fb3752b491d246034fa4d279ff076501588ce8cbcdbb62c32fd7a377d996be"}, + {file = "psutil-6.1.0.tar.gz", hash = "sha256:353815f59a7f64cdaca1c0307ee13558a0512f6db064e92fe833784f08539c7a"}, +] + +[package.extras] +dev = ["black", "check-manifest", "coverage", "packaging", "pylint", "pyperf", "pypinfo", "pytest-cov", "requests", "rstcheck", "ruff", "sphinx", "sphinx_rtd_theme", "toml-sort", "twine", "virtualenv", "wheel"] +test = ["pytest", "pytest-xdist", "setuptools"] + [[package]] name = "psycopg2" version = "2.9.10" @@ -1750,6 +1780,16 @@ files = [ [package.dependencies] pytz = "*" +[[package]] +name = "pysyncobj" +version = "0.3.13" +description = "A library for replicating your python class between multiple servers, based on raft protocol" +optional = false +python-versions = "*" +files = [ + {file = "pysyncobj-0.3.13.tar.gz", hash = "sha256:1785930b738fa21af298ebb04c213af25c31af148faa32f53af337ed1492d5a2"}, +] + [[package]] name = "pytest" version = "8.3.3" @@ -2132,29 +2172,29 @@ pyasn1 = ">=0.1.3" [[package]] name = "ruff" -version = "0.7.1" +version = "0.8.0" description = "An extremely fast Python linter and code formatter, written in Rust." optional = false python-versions = ">=3.7" files = [ - {file = "ruff-0.7.1-py3-none-linux_armv6l.whl", hash = "sha256:cb1bc5ed9403daa7da05475d615739cc0212e861b7306f314379d958592aaa89"}, - {file = "ruff-0.7.1-py3-none-macosx_10_12_x86_64.whl", hash = "sha256:27c1c52a8d199a257ff1e5582d078eab7145129aa02721815ca8fa4f9612dc35"}, - {file = "ruff-0.7.1-py3-none-macosx_11_0_arm64.whl", hash = "sha256:588a34e1ef2ea55b4ddfec26bbe76bc866e92523d8c6cdec5e8aceefeff02d99"}, - {file = "ruff-0.7.1-py3-none-manylinux_2_17_aarch64.manylinux2014_aarch64.whl", hash = "sha256:94fc32f9cdf72dc75c451e5f072758b118ab8100727168a3df58502b43a599ca"}, - {file = "ruff-0.7.1-py3-none-manylinux_2_17_armv7l.manylinux2014_armv7l.whl", hash = "sha256:985818742b833bffa543a84d1cc11b5e6871de1b4e0ac3060a59a2bae3969250"}, - {file = "ruff-0.7.1-py3-none-manylinux_2_17_i686.manylinux2014_i686.whl", hash = "sha256:32f1e8a192e261366c702c5fb2ece9f68d26625f198a25c408861c16dc2dea9c"}, - {file = "ruff-0.7.1-py3-none-manylinux_2_17_ppc64.manylinux2014_ppc64.whl", hash = "sha256:699085bf05819588551b11751eff33e9ca58b1b86a6843e1b082a7de40da1565"}, - {file = "ruff-0.7.1-py3-none-manylinux_2_17_ppc64le.manylinux2014_ppc64le.whl", hash = "sha256:344cc2b0814047dc8c3a8ff2cd1f3d808bb23c6658db830d25147339d9bf9ea7"}, - {file = "ruff-0.7.1-py3-none-manylinux_2_17_s390x.manylinux2014_s390x.whl", hash = "sha256:4316bbf69d5a859cc937890c7ac7a6551252b6a01b1d2c97e8fc96e45a7c8b4a"}, - {file = "ruff-0.7.1-py3-none-manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:79d3af9dca4c56043e738a4d6dd1e9444b6d6c10598ac52d146e331eb155a8ad"}, - {file = "ruff-0.7.1-py3-none-musllinux_1_2_aarch64.whl", hash = "sha256:c5c121b46abde94a505175524e51891f829414e093cd8326d6e741ecfc0a9112"}, - {file = "ruff-0.7.1-py3-none-musllinux_1_2_armv7l.whl", hash = "sha256:8422104078324ea250886954e48f1373a8fe7de59283d747c3a7eca050b4e378"}, - {file = "ruff-0.7.1-py3-none-musllinux_1_2_i686.whl", hash = "sha256:56aad830af8a9db644e80098fe4984a948e2b6fc2e73891538f43bbe478461b8"}, - {file = "ruff-0.7.1-py3-none-musllinux_1_2_x86_64.whl", hash = "sha256:658304f02f68d3a83c998ad8bf91f9b4f53e93e5412b8f2388359d55869727fd"}, - {file = "ruff-0.7.1-py3-none-win32.whl", hash = "sha256:b517a2011333eb7ce2d402652ecaa0ac1a30c114fbbd55c6b8ee466a7f600ee9"}, - {file = "ruff-0.7.1-py3-none-win_amd64.whl", hash = "sha256:f38c41fcde1728736b4eb2b18850f6d1e3eedd9678c914dede554a70d5241307"}, - {file = "ruff-0.7.1-py3-none-win_arm64.whl", hash = "sha256:19aa200ec824c0f36d0c9114c8ec0087082021732979a359d6f3c390a6ff2a37"}, - {file = "ruff-0.7.1.tar.gz", hash = "sha256:9d8a41d4aa2dad1575adb98a82870cf5db5f76b2938cf2206c22c940034a36f4"}, + {file = "ruff-0.8.0-py3-none-linux_armv6l.whl", hash = "sha256:fcb1bf2cc6706adae9d79c8d86478677e3bbd4ced796ccad106fd4776d395fea"}, + {file = "ruff-0.8.0-py3-none-macosx_10_12_x86_64.whl", hash = "sha256:295bb4c02d58ff2ef4378a1870c20af30723013f441c9d1637a008baaf928c8b"}, + {file = "ruff-0.8.0-py3-none-macosx_11_0_arm64.whl", hash = "sha256:7b1f1c76b47c18fa92ee78b60d2d20d7e866c55ee603e7d19c1e991fad933a9a"}, + {file = "ruff-0.8.0-py3-none-manylinux_2_17_aarch64.manylinux2014_aarch64.whl", hash = "sha256:eb0d4f250a7711b67ad513fde67e8870109e5ce590a801c3722580fe98c33a99"}, + {file = "ruff-0.8.0-py3-none-manylinux_2_17_armv7l.manylinux2014_armv7l.whl", hash = "sha256:0e55cce9aa93c5d0d4e3937e47b169035c7e91c8655b0974e61bb79cf398d49c"}, + {file = "ruff-0.8.0-py3-none-manylinux_2_17_i686.manylinux2014_i686.whl", hash = "sha256:3f4cd64916d8e732ce6b87f3f5296a8942d285bbbc161acee7fe561134af64f9"}, + {file = "ruff-0.8.0-py3-none-manylinux_2_17_ppc64.manylinux2014_ppc64.whl", hash = "sha256:c5c1466be2a2ebdf7c5450dd5d980cc87c8ba6976fb82582fea18823da6fa362"}, + {file = "ruff-0.8.0-py3-none-manylinux_2_17_ppc64le.manylinux2014_ppc64le.whl", hash = "sha256:2dabfd05b96b7b8f2da00d53c514eea842bff83e41e1cceb08ae1966254a51df"}, + {file = "ruff-0.8.0-py3-none-manylinux_2_17_s390x.manylinux2014_s390x.whl", hash = "sha256:facebdfe5a5af6b1588a1d26d170635ead6892d0e314477e80256ef4a8470cf3"}, + {file = "ruff-0.8.0-py3-none-manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:87a8e86bae0dbd749c815211ca11e3a7bd559b9710746c559ed63106d382bd9c"}, + {file = "ruff-0.8.0-py3-none-musllinux_1_2_aarch64.whl", hash = "sha256:85e654f0ded7befe2d61eeaf3d3b1e4ef3894469cd664ffa85006c7720f1e4a2"}, + {file = "ruff-0.8.0-py3-none-musllinux_1_2_armv7l.whl", hash = "sha256:83a55679c4cb449fa527b8497cadf54f076603cc36779b2170b24f704171ce70"}, + {file = "ruff-0.8.0-py3-none-musllinux_1_2_i686.whl", hash = "sha256:812e2052121634cf13cd6fddf0c1871d0ead1aad40a1a258753c04c18bb71bbd"}, + {file = "ruff-0.8.0-py3-none-musllinux_1_2_x86_64.whl", hash = "sha256:780d5d8523c04202184405e60c98d7595bdb498c3c6abba3b6d4cdf2ca2af426"}, + {file = "ruff-0.8.0-py3-none-win32.whl", hash = "sha256:5fdb6efecc3eb60bba5819679466471fd7d13c53487df7248d6e27146e985468"}, + {file = "ruff-0.8.0-py3-none-win_amd64.whl", hash = "sha256:582891c57b96228d146725975fbb942e1f30a0c4ba19722e692ca3eb25cc9b4f"}, + {file = "ruff-0.8.0-py3-none-win_arm64.whl", hash = "sha256:ba93e6294e9a737cd726b74b09a6972e36bb511f9a102f1d9a7e1ce94dd206a6"}, + {file = "ruff-0.8.0.tar.gz", hash = "sha256:a7ccfe6331bf8c8dad715753e157457faf7351c2b69f62f32c165c2dbcbacd44"}, ] [[package]] @@ -2533,4 +2573,4 @@ type = ["pytest-mypy"] [metadata] lock-version = "2.0" python-versions = "^3.10" -content-hash = "a24006bb8af98b161cd722b73b93b3ce7fbc5f44e46ee2d4faa24e438c09e0de" +content-hash = "bd400db2f5f7879b01201588f6328b68629db3dedcf803e1b83074c87f7f78bb" diff --git a/pyproject.toml b/pyproject.toml index ea24e76a47..c577c3f35a 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -18,6 +18,8 @@ pydantic = "^1.10.18" poetry-core = "^1.9.1" pyOpenSSL = "^24.2.1" jinja2 = "^3.1.4" +pysyncobj = "^0.3.13" +psutil = "^6.0.0" [tool.poetry.group.charm-libs.dependencies] # data_platform_libs/v0/data_interfaces.py @@ -38,7 +40,7 @@ opentelemetry-exporter-otlp-proto-http = "1.21.0" optional = true [tool.poetry.group.format.dependencies] -ruff = "^0.7.1" +ruff = "^0.8.0" [tool.poetry.group.lint] optional = true @@ -106,7 +108,7 @@ line-length = 99 [tool.ruff.lint] explicit-preview-rules = true -select = ["A", "E", "W", "F", "C", "N", "D", "I001", "B", "CPY", "RUF", "S", "SIM", "UP", "TCH"] +select = ["A", "E", "W", "F", "C", "N", "D", "I001", "B", "CPY", "RUF", "S", "SIM", "UP", "TC"] extend-ignore = [ "D203", "D204", diff --git a/src/charm.py b/src/charm.py index 37fbfb1552..afc46729d9 100755 --- a/src/charm.py +++ b/src/charm.py @@ -511,25 +511,31 @@ def _on_pgdata_storage_detaching(self, _) -> None: if self.primary_endpoint: self._update_relation_endpoints() - def _on_peer_relation_changed(self, event: HookEvent): # noqa: C901 - """Reconfigure cluster members when something changes.""" + def _peer_relation_changed_checks(self, event: HookEvent) -> bool: + """Split of to reduce complexity.""" # Prevents the cluster to be reconfigured before it's bootstrapped in the leader. if "cluster_initialised" not in self._peers.data[self.app]: logger.debug("Deferring on_peer_relation_changed: cluster not initialized") event.defer() - return + return False # If the unit is the leader, it can reconfigure the cluster. if self.unit.is_leader() and not self._reconfigure_cluster(event): event.defer() - return + return False if self._update_member_ip(): - return + return False # Don't update this member before it's part of the members list. if self._unit_ip not in self.members_ips: logger.debug("Early exit on_peer_relation_changed: Unit not in the members list") + return False + return True + + def _on_peer_relation_changed(self, event: HookEvent): + """Reconfigure cluster members when something changes.""" + if not self._peer_relation_changed_checks(event): return # Update the list of the cluster members in the replicas to make them know each other. @@ -736,14 +742,16 @@ def add_cluster_member(self, member: str) -> None: def _get_unit_ip(self, unit: Unit) -> str | None: """Get the IP address of a specific unit.""" # Check if host is current host. + ip = None if unit == self.unit: - return str(self.model.get_binding(PEER).network.bind_address) + ip = self.model.get_binding(PEER).network.bind_address # Check if host is a peer. elif unit in self._peers.data: - return str(self._peers.data[unit].get("private-address")) + ip = self._peers.data[unit].get("private-address") # Return None if the unit is not a peer neither the current unit. - else: - return None + if ip: + return str(ip) + return None @property def _hosts(self) -> set: diff --git a/src/cluster.py b/src/cluster.py index cebfc468ee..374964a5e5 100644 --- a/src/cluster.py +++ b/src/cluster.py @@ -15,6 +15,7 @@ import requests from charms.operator_libs_linux.v2 import snap from jinja2 import Template +from pysyncobj.utility import TcpUtility, UtilityException from tenacity import ( AttemptManager, RetryError, @@ -766,32 +767,28 @@ def remove_raft_member(self, member_ip: str) -> None: """ # Suppressing since the call will be removed soon # Get the status of the raft cluster. - raft_status = subprocess.check_output([ # noqa - "charmed-postgresql.syncobj-admin", - "-conn", - "127.0.0.1:2222", - "-pass", - self.raft_password, - "-status", - ]).decode("UTF-8") + syncobj_util = TcpUtility(password=self.raft_password, timeout=3) + + raft_host = "127.0.0.1:2222" + try: + raft_status = syncobj_util.executeCommand(raft_host, ["status"]) + except UtilityException: + logger.warning("Remove raft member: Cannot connect to raft cluster") + raise RemoveRaftMemberFailedError() from None # Check whether the member is still part of the raft cluster. - if not member_ip or member_ip not in raft_status: + if not member_ip or f"partner_node_status_server_{member_ip}:2222" not in raft_status: return # Suppressing since the call will be removed soon # Remove the member from the raft cluster. - result = subprocess.check_output([ # noqa - "charmed-postgresql.syncobj-admin", - "-conn", - "127.0.0.1:2222", - "-pass", - self.raft_password, - "-remove", - f"{member_ip}:2222", - ]).decode("UTF-8") - - if "SUCCESS" not in result: + try: + result = syncobj_util.executeCommand(raft_host, ["remove", f"{member_ip}:2222"]) + except UtilityException: + logger.debug("Remove raft member: Remove call failed") + raise RemoveRaftMemberFailedError() from None + + if not result.startswith("SUCCESS"): raise RemoveRaftMemberFailedError() @retry(stop=stop_after_attempt(10), wait=wait_exponential(multiplier=1, min=2, max=10)) diff --git a/tests/integration/ha_tests/helpers.py b/tests/integration/ha_tests/helpers.py index 1c5760598b..57ddac6dd9 100644 --- a/tests/integration/ha_tests/helpers.py +++ b/tests/integration/ha_tests/helpers.py @@ -93,21 +93,27 @@ async def are_writes_increasing( extra_model: Model = None, ) -> None: """Verify new writes are continuing by counting the number of writes.""" + down_units = [down_unit] if isinstance(down_unit, str) or not down_unit else down_unit writes, _ = await count_writes( ops_test, - down_unit=down_unit, + down_unit=down_units[0], use_ip_from_inside=use_ip_from_inside, extra_model=extra_model, ) - for member, count in writes.items(): - for attempt in Retrying(stop=stop_after_delay(60 * 3), wait=wait_fixed(3)): - with attempt: - more_writes, _ = await count_writes( - ops_test, - down_unit=down_unit, - use_ip_from_inside=use_ip_from_inside, - extra_model=extra_model, - ) + logger.info(f"Initial writes {writes}") + + for attempt in Retrying(stop=stop_after_delay(60 * 3), wait=wait_fixed(3), reraise=True): + with attempt: + more_writes, _ = await count_writes( + ops_test, + down_unit=down_unit, + use_ip_from_inside=use_ip_from_inside, + extra_model=extra_model, + ) + logger.info(f"Retry writes {more_writes}") + for member, count in writes.items(): + if "/".join(member.split(".", 1)[-1].rsplit("-", 1)) in down_units: + continue assert more_writes[member] > count, ( f"{member}: writes not continuing to DB (current writes: {more_writes[member]} - previous writes: {count})" ) @@ -616,6 +622,33 @@ async def is_replica(ops_test: OpsTest, unit_name: str, use_ip_from_inside: bool return False +async def get_cluster_roles( + ops_test: OpsTest, unit_name: str, use_ip_from_inside: bool = False +) -> dict[str, str | list[str] | None]: + """Returns whether the unit a replica in the cluster.""" + unit_ip = await ( + get_ip_from_inside_the_unit(ops_test, unit_name) + if use_ip_from_inside + else get_unit_ip(ops_test, unit_name) + ) + + members = {"replicas": [], "primaries": [], "sync_standbys": []} + cluster_info = requests.get(f"http://{unit_ip}:8008/cluster") + member_list = cluster_info.json()["members"] + logger.info(f"Cluster members are: {member_list}") + for member in member_list: + role = member["role"] + name = "/".join(member["name"].rsplit("-", 1)) + if role == "leader": + members["primaries"].append(name) + elif role == "sync_standby": + members["sync_standbys"].append(name) + else: + members["replicas"].append(name) + + return members + + async def instance_ip(ops_test: OpsTest, instance: str) -> str: """Translate juju instance name to IP. diff --git a/tests/integration/ha_tests/test_smoke.py b/tests/integration/ha_tests/test_smoke.py index 2c954ee466..ea872d45d0 100644 --- a/tests/integration/ha_tests/test_smoke.py +++ b/tests/integration/ha_tests/test_smoke.py @@ -2,8 +2,8 @@ # Copyright 2021 Canonical Ltd. # See LICENSE file for licensing details. +import asyncio import logging -from asyncio import TimeoutError import pytest from juju import tag @@ -163,7 +163,7 @@ async def test_app_resources_conflicts_v3(ops_test: OpsTest, charm: str): await ops_test.model.wait_for_idle( apps=[DUP_APPLICATION_NAME], timeout=1000, status="blocked" ) - except TimeoutError: + except asyncio.TimeoutError: logger.info("Application is not in blocked state. Checking logs...") # Since application have postgresql db in storage from external application it should not be able to connect due to new password @@ -211,7 +211,7 @@ async def test_app_resources_conflicts_v2(ops_test: OpsTest, charm: str): await ops_test.model.wait_for_idle( apps=[DUP_APPLICATION_NAME], timeout=1000, status="blocked" ) - except TimeoutError: + except asyncio.TimeoutError: logger.info("Application is not in blocked state. Checking logs...") # Since application have postgresql db in storage from external application it should not be able to connect due to new password diff --git a/tests/unit/test_cluster.py b/tests/unit/test_cluster.py index 048aab046e..9cb737f0bf 100644 --- a/tests/unit/test_cluster.py +++ b/tests/unit/test_cluster.py @@ -5,14 +5,21 @@ import pytest import requests as requests -import tenacity as tenacity from charms.operator_libs_linux.v2 import snap from jinja2 import Template from ops.testing import Harness -from tenacity import RetryError, stop_after_delay, wait_fixed +from pysyncobj.utility import UtilityException +from tenacity import ( + AttemptManager, + RetryCallState, + RetryError, + Retrying, + stop_after_delay, + wait_fixed, +) from charm import PostgresqlOperatorCharm -from cluster import PATRONI_TIMEOUT, Patroni +from cluster import PATRONI_TIMEOUT, Patroni, RemoveRaftMemberFailedError from constants import ( PATRONI_CONF_PATH, PATRONI_LOGS_PATH, @@ -82,9 +89,9 @@ def patroni(harness, peers_ips): def test_get_alternative_patroni_url(peers_ips, patroni): # Mock tenacity attempt. - retry = tenacity.Retrying() - retry_state = tenacity.RetryCallState(retry, None, None, None) - attempt = tenacity.AttemptManager(retry_state) + retry = Retrying() + retry_state = RetryCallState(retry, None, None, None) + attempt = AttemptManager(retry_state) # Test the first URL that is returned (it should have the current unit IP). url = patroni._get_alternative_patroni_url(attempt) @@ -104,7 +111,7 @@ def test_get_member_ip(peers_ips, patroni): ): # Test error on trying to get the member IP. _get_alternative_patroni_url.side_effect = "http://server2" - with pytest.raises(tenacity.RetryError): + with pytest.raises(RetryError): patroni.get_member_ip(patroni.member_name) assert False @@ -147,7 +154,7 @@ def test_get_patroni_health(peers_ips, patroni): # Test when the Patroni API is not reachable. _patroni_url.return_value = "http://server2" - with pytest.raises(tenacity.RetryError): + with pytest.raises(RetryError): patroni.get_patroni_health() assert False @@ -174,7 +181,7 @@ def test_get_primary(peers_ips, patroni): ): # Test error on trying to get the member IP. _get_alternative_patroni_url.side_effect = "http://server2" - with pytest.raises(tenacity.RetryError): + with pytest.raises(RetryError): patroni.get_primary(patroni.member_name) assert False @@ -238,8 +245,8 @@ def test_is_replication_healthy(peers_ips, patroni): def test_is_member_isolated(peers_ips, patroni): with ( - patch("cluster.stop_after_delay", return_value=tenacity.stop_after_delay(0)), - patch("cluster.wait_fixed", return_value=tenacity.wait_fixed(0)), + patch("cluster.stop_after_delay", return_value=stop_after_delay(0)), + patch("cluster.wait_fixed", return_value=wait_fixed(0)), patch("requests.get", side_effect=mocked_requests_get) as _get, patch("charm.Patroni._patroni_url", new_callable=PropertyMock) as _patroni_url, ): @@ -419,7 +426,7 @@ def test_member_replication_lag(peers_ips, patroni): # Test when the API call fails. _patroni_url.return_value = "http://server2" - with patch.object(tenacity.Retrying, "iter", Mock(side_effect=tenacity.RetryError(None))): + with patch.object(Retrying, "iter", Mock(side_effect=RetryError(None))): lag = patroni.member_replication_lag assert lag == "unknown" @@ -509,8 +516,8 @@ def test_configure_patroni_on_unit(peers_ips, patroni): def test_member_started_true(peers_ips, patroni): with ( patch("cluster.requests.get") as _get, - patch("cluster.stop_after_delay", return_value=tenacity.stop_after_delay(0)), - patch("cluster.wait_fixed", return_value=tenacity.wait_fixed(0)), + patch("cluster.stop_after_delay", return_value=stop_after_delay(0)), + patch("cluster.wait_fixed", return_value=wait_fixed(0)), ): _get.return_value.json.return_value = {"state": "running"} @@ -524,8 +531,8 @@ def test_member_started_true(peers_ips, patroni): def test_member_started_false(peers_ips, patroni): with ( patch("cluster.requests.get") as _get, - patch("cluster.stop_after_delay", return_value=tenacity.stop_after_delay(0)), - patch("cluster.wait_fixed", return_value=tenacity.wait_fixed(0)), + patch("cluster.stop_after_delay", return_value=stop_after_delay(0)), + patch("cluster.wait_fixed", return_value=wait_fixed(0)), ): _get.return_value.json.return_value = {"state": "stopped"} @@ -539,8 +546,8 @@ def test_member_started_false(peers_ips, patroni): def test_member_started_error(peers_ips, patroni): with ( patch("cluster.requests.get") as _get, - patch("cluster.stop_after_delay", return_value=tenacity.stop_after_delay(0)), - patch("cluster.wait_fixed", return_value=tenacity.wait_fixed(0)), + patch("cluster.stop_after_delay", return_value=stop_after_delay(0)), + patch("cluster.wait_fixed", return_value=wait_fixed(0)), ): _get.side_effect = Exception @@ -554,8 +561,8 @@ def test_member_started_error(peers_ips, patroni): def test_member_inactive_true(peers_ips, patroni): with ( patch("cluster.requests.get") as _get, - patch("cluster.stop_after_delay", return_value=tenacity.stop_after_delay(0)), - patch("cluster.wait_fixed", return_value=tenacity.wait_fixed(0)), + patch("cluster.stop_after_delay", return_value=stop_after_delay(0)), + patch("cluster.wait_fixed", return_value=wait_fixed(0)), ): _get.return_value.json.return_value = {"state": "stopped"} @@ -569,8 +576,8 @@ def test_member_inactive_true(peers_ips, patroni): def test_member_inactive_false(peers_ips, patroni): with ( patch("cluster.requests.get") as _get, - patch("cluster.stop_after_delay", return_value=tenacity.stop_after_delay(0)), - patch("cluster.wait_fixed", return_value=tenacity.wait_fixed(0)), + patch("cluster.stop_after_delay", return_value=stop_after_delay(0)), + patch("cluster.wait_fixed", return_value=wait_fixed(0)), ): _get.return_value.json.return_value = {"state": "starting"} @@ -584,8 +591,8 @@ def test_member_inactive_false(peers_ips, patroni): def test_member_inactive_error(peers_ips, patroni): with ( patch("cluster.requests.get") as _get, - patch("cluster.stop_after_delay", return_value=tenacity.stop_after_delay(0)), - patch("cluster.wait_fixed", return_value=tenacity.wait_fixed(0)), + patch("cluster.stop_after_delay", return_value=stop_after_delay(0)), + patch("cluster.wait_fixed", return_value=wait_fixed(0)), ): _get.side_effect = Exception @@ -665,6 +672,65 @@ def test_update_patroni_restart_condition(patroni, new_restart_condition): _run.assert_called_once_with(["/bin/systemctl", "daemon-reload"]) +def test_remove_raft_member(patroni): + with patch("cluster.TcpUtility") as _tcp_utility: + # Member already removed + _tcp_utility.return_value.executeCommand.return_value = "" + + patroni.remove_raft_member("1.2.3.4") + + _tcp_utility.assert_called_once_with(password="fake-raft-password", timeout=3) + _tcp_utility.return_value.executeCommand.assert_called_once_with( + "127.0.0.1:2222", ["status"] + ) + _tcp_utility.reset_mock() + + # Removing member + _tcp_utility.return_value.executeCommand.side_effect = [ + {"partner_node_status_server_1.2.3.4:2222": 0, "has_quorum": True}, + "SUCCESS", + ] + + patroni.remove_raft_member("1.2.3.4") + + _tcp_utility.assert_called_once_with(password="fake-raft-password", timeout=3) + assert _tcp_utility.return_value.executeCommand.call_count == 2 + _tcp_utility.return_value.executeCommand.assert_any_call("127.0.0.1:2222", ["status"]) + _tcp_utility.return_value.executeCommand.assert_any_call( + "127.0.0.1:2222", ["remove", "1.2.3.4:2222"] + ) + _tcp_utility.reset_mock() + + # Raises on failed status + _tcp_utility.return_value.executeCommand.side_effect = [ + {"partner_node_status_server_1.2.3.4:2222": 0, "has_quorum": True}, + "FAIL", + ] + + with pytest.raises(RemoveRaftMemberFailedError): + patroni.remove_raft_member("1.2.3.4") + assert False + + # Raises on remove error + _tcp_utility.return_value.executeCommand.side_effect = [ + {"partner_node_status_server_1.2.3.4:2222": 0, "has_quorum": True}, + UtilityException, + ] + + with pytest.raises(RemoveRaftMemberFailedError): + patroni.remove_raft_member("1.2.3.4") + assert False + + # Raises on status error + _tcp_utility.return_value.executeCommand.side_effect = [ + UtilityException, + ] + + with pytest.raises(RemoveRaftMemberFailedError): + patroni.remove_raft_member("1.2.3.4") + assert False + + def test_are_replicas_up(patroni): with ( patch("requests.get") as _get,