[DPE-7129] Fix network cut without ip change test (#900)

* Move _update_member_ip call to correctly remove Raft cluster member when network is cut

Signed-off-by: Marcelo Henrique Neppel <[email protected]>

* Fix coverage

Signed-off-by: Marcelo Henrique Neppel <[email protected]>

* Update refresh tests to modify charm to ensure refresh off edge or stable

* Fix lint warnings

* Store temporary charms in /tmp for upgrade_from_stable tests

* Use force-refresh-start instead of forcing refresh by updating versions

* Handle same snap revision situation in upgrade tests

Signed-off-by: Marcelo Henrique Neppel <[email protected]>

* Improvement to avoid replica restart while syncing from primary

Signed-off-by: Marcelo Henrique Neppel <[email protected]>

* Run stop-continuous-writes action only once

Signed-off-by: Marcelo Henrique Neppel <[email protected]>

---------

Signed-off-by: Marcelo Henrique Neppel <[email protected]>
Co-authored-by: Shayan Patel <[email protected]>

Author: marceloneppel

src/charm.py

@@ -851,9 +851,6 @@ def _peer_relation_changed_checks(self, event: HookEvent) -> bool:
             event.defer()
             return False
 
-        if self._update_member_ip():
-            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")
@@ -1446,6 +1443,11 @@ def _on_config_changed(self, event) -> None:  # noqa: C901
         if self.is_blocked and "Configuration Error" in self.unit.status.message:
             self.set_unit_status(ActiveStatus())
 
+        if self._update_member_ip():
+            # Update the sync-standby endpoint in the async replication data.
+            self.async_replication.update_async_replication_data()
+            return
+
         # Update the sync-standby endpoint in the async replication data.
         self.async_replication.update_async_replication_data()
 

src/cluster.py

@@ -532,7 +532,12 @@ def member_inactive(self) -> bool:
         except RetryError:
             return True
 
-        return response["state"] not in [*RUNNING_STATES, "starting", "restarting"]
+        return response["state"] not in [
+            *RUNNING_STATES,
+            "creating replica",
+            "starting",
+            "restarting",
+        ]
 
     @property
     def member_replication_lag(self) -> str:

tests/integration/ha_tests/helpers.py

@@ -241,9 +241,7 @@ async def is_cluster_updated(
 
     # Verify that no writes to the database were missed after stopping the writes.
     logger.info("checking that no writes to the database were missed after stopping the writes")
-    for attempt in Retrying(stop=stop_after_attempt(3), wait=wait_fixed(5), reraise=True):
-        with attempt:
-            total_expected_writes = await check_writes(ops_test, use_ip_from_inside)
+    total_expected_writes = await check_writes(ops_test, use_ip_from_inside)
 
     # Verify that old primary is up-to-date.
     logger.info("checking that the former primary is up to date with the cluster after restarting")
@@ -259,17 +257,19 @@ async def check_writes(
 ) -> int:
     """Gets the total writes from the test charm and compares to the writes from db."""
     total_expected_writes = await stop_continuous_writes(ops_test)
-    actual_writes, max_number_written = await count_writes(
-        ops_test, use_ip_from_inside=use_ip_from_inside, extra_model=extra_model
-    )
-    for member, count in actual_writes.items():
-        print(
-            f"member: {member}, count: {count}, max_number_written: {max_number_written[member]}, total_expected_writes: {total_expected_writes}"
-        )
-        assert count == max_number_written[member], (
-            f"{member}: writes to the db were missed: count of actual writes different from the max number written."
-        )
-        assert total_expected_writes == count, f"{member}: writes to the db were missed."
+    for attempt in Retrying(stop=stop_after_attempt(3), wait=wait_fixed(5), reraise=True):
+        with attempt:
+            actual_writes, max_number_written = await count_writes(
+                ops_test, use_ip_from_inside=use_ip_from_inside, extra_model=extra_model
+            )
+            for member, count in actual_writes.items():
+                logger.info(
+                    f"member: {member}, count: {count}, max_number_written: {max_number_written[member]}, total_expected_writes: {total_expected_writes}"
+                )
+                assert count == max_number_written[member], (
+                    f"{member}: writes to the db were missed: count of actual writes different from the max number written."
+                )
+                assert total_expected_writes == count, f"{member}: writes to the db were missed."
     return total_expected_writes
 
 

tests/unit/test_charm.py

@@ -267,6 +267,9 @@ def test_is_cluster_initialised(harness):
 
 def test_on_config_changed(harness):
     with (
+        patch(
+            "charm.PostgresqlOperatorCharm._update_member_ip", return_value=False
+        ) as _update_member_ip,
         patch(
             "charm.PostgresqlOperatorCharm._validate_config_options"
         ) as _validate_config_options,
@@ -316,6 +319,23 @@ def test_on_config_changed(harness):
         harness.charm.on.config_changed.emit()
         _enable_disable_extensions.assert_called_once()
 
+        # Test when there is an error related to the config options.
+        _update_member_ip.reset_mock()
+        _enable_disable_extensions.reset_mock()
+        harness.charm.unit.status = BlockedStatus("Configuration Error")
+        harness.charm.on.config_changed.emit()
+        assert isinstance(harness.model.unit.status, ActiveStatus)
+        _update_member_ip.assert_called_once()
+        _enable_disable_extensions.assert_called_once()
+
+        # Test when the unit has updated its member IP.
+        _update_member_ip.reset_mock()
+        _enable_disable_extensions.reset_mock()
+        _update_member_ip.return_value = True
+        harness.charm.on.config_changed.emit()
+        _update_member_ip.assert_called_once()
+        _enable_disable_extensions.assert_not_called()
+
 
 def test_check_extension_dependencies(harness):
     with (
@@ -1426,32 +1446,20 @@ def test_on_peer_relation_changed(harness):
         mock_event.defer.reset_mock()
         _reconfigure_cluster.reset_mock()
         _reconfigure_cluster.return_value = True
-        _update_member_ip.return_value = False
         _member_started.return_value = True
         _primary_endpoint.return_value = "192.0.2.0"
         harness.model.unit.status = WaitingStatus("awaiting for cluster to start")
         harness.charm._on_peer_relation_changed(mock_event)
         mock_event.defer.assert_not_called()
         _reconfigure_cluster.assert_called_once_with(mock_event)
-        _update_member_ip.assert_called_once()
         _update_config.assert_called_once()
         _start_patroni.assert_called_once()
         _update_new_unit_status.assert_called_once()
 
-        # Test when the cluster member updates its IP.
-        _update_member_ip.reset_mock()
+        # Test when the unit fails to update the Patroni configuration.
         _update_config.reset_mock()
         _start_patroni.reset_mock()
-        _update_member_ip.return_value = True
         _update_new_unit_status.reset_mock()
-        harness.charm._on_peer_relation_changed(mock_event)
-        _update_member_ip.assert_called_once()
-        _update_config.assert_not_called()
-        _start_patroni.assert_not_called()
-        _update_new_unit_status.assert_not_called()
-
-        # Test when the unit fails to update the Patroni configuration.
-        _update_member_ip.return_value = False
         _update_config.side_effect = RetryError(last_attempt=1)
         harness.charm._on_peer_relation_changed(mock_event)
         _update_config.assert_called_once()