fix: DPE-6485 Manage mysqld directly (#575)

* Manage mysqld without mysqld_safe

* set behavior in child

* refactor to ditch rolling ops event defer

* method moved to charm.py

* remove `charm`

* speed up test

* keep try catch to keep RetryError import

* enabling sql `RESTART`

* fix bump version

* 10s fast_forward prevent all units idle at the same time, timing out test

* pythonic failure injection method

tests consistently fails on CI but work on my machine(tm), so this eliminates any differences from the env

Author: paulomach

lib/charms/mysql/v0/mysql.py

@@ -133,7 +133,7 @@ def wait_until_mysql_connection(self) -> None:
 # Increment this major API version when introducing breaking changes
 LIBAPI = 0
 
-LIBPATCH = 82
+LIBPATCH = 83
 
 UNIT_TEARDOWN_LOCKNAME = "unit-teardown"
 UNIT_ADD_LOCKNAME = "unit-add"
@@ -1463,7 +1463,11 @@ def get_variable_value(self, variable: str) -> str:
         return rows[0][1]
 
     def configure_instance(self, create_cluster_admin: bool = True) -> None:
-        """Configure the instance to be used in an InnoDB cluster."""
+        """Configure the instance to be used in an InnoDB cluster.
+
+        Args:
+            create_cluster_admin: Whether to create the cluster admin user.
+        """
         options = {
             "restart": "true",
         }
@@ -1955,6 +1959,7 @@ def is_instance_in_cluster(self, unit_label: str) -> bool:
                 user=self.server_config_user,
                 password=self.server_config_password,
                 host=self.instance_def(self.server_config_user),
+                exception_as_warning=True,
             )
             return (
                 MySQLMemberState.ONLINE in output.lower()

src/charm.py

@@ -61,6 +61,7 @@
     WaitingStatus,
 )
 from ops.pebble import Layer
+from tenacity import RetryError, Retrying, stop_after_attempt, wait_fixed
 
 from config import CharmConfig, MySQLConfig
 from constants import (
@@ -79,7 +80,8 @@
     MYSQLD_CONFIG_FILE,
     MYSQLD_EXPORTER_PORT,
     MYSQLD_EXPORTER_SERVICE,
-    MYSQLD_SAFE_SERVICE,
+    MYSQLD_LOCATION,
+    MYSQLD_SERVICE,
     PASSWORD_LENGTH,
     PEER,
     ROOT_PASSWORD_KEY,
@@ -225,18 +227,30 @@ def _mysql(self) -> MySQL:
     @property
     def _pebble_layer(self) -> Layer:
         """Return a layer for the mysqld pebble service."""
+        mysqld_cmd = [
+            MYSQLD_LOCATION,
+            "--basedir=/usr",
+            "--datadir=/var/lib/mysql",
+            "--plugin-dir=/usr/lib/mysql/plugin",
+            "--log-error=/var/log/mysql/error.log",
+            f"--pid-file={self.unit_label}.pid",
+        ]
+
         layer = {
             "summary": "mysqld services layer",
             "description": "pebble config layer for mysqld safe and exporter",
             "services": {
-                MYSQLD_SAFE_SERVICE: {
+                MYSQLD_SERVICE: {
                     "override": "replace",
                     "summary": "mysqld safe",
-                    "command": MYSQLD_SAFE_SERVICE,
+                    "command": " ".join(mysqld_cmd),
                     "startup": "enabled",
                     "user": MYSQL_SYSTEM_USER,
                     "group": MYSQL_SYSTEM_GROUP,
                     "kill-delay": "24h",
+                    "environment": {
+                        "MYSQLD_PARENT_PID": 1,
+                    },
                 },
                 MYSQLD_EXPORTER_SERVICE: {
                     "override": "replace",
@@ -451,7 +465,7 @@ def _reconcile_pebble_layer(self, container: Container) -> None:
         if new_layer.services != current_layer.services:
             logger.info("Reconciling the pebble layer")
 
-            container.add_layer(MYSQLD_SAFE_SERVICE, new_layer, combine=True)
+            container.add_layer(MYSQLD_SERVICE, new_layer, combine=True)
             container.replan()
             self._mysql.wait_until_mysql_connection()
 
@@ -463,34 +477,55 @@ def _reconcile_pebble_layer(self, container: Container) -> None:
             ):
                 container.stop(MYSQLD_EXPORTER_SERVICE)
 
-    def _restart(self, event: EventBase) -> None:
+    def recover_unit_after_restart(self) -> None:
+        """Wait for unit recovery/rejoin after restart."""
+        recovery_attempts = 30
+        logger.info("Recovering unit")
+        if self.app.planned_units() == 1:
+            self._mysql.reboot_from_complete_outage()
+        else:
+            try:
+                for attempt in Retrying(
+                    stop=stop_after_attempt(recovery_attempts), wait=wait_fixed(15)
+                ):
+                    with attempt:
+                        self._mysql.hold_if_recovering()
+                        if not self._mysql.is_instance_in_cluster(self.unit_label):
+                            logger.debug(
+                                "Instance not yet back in the cluster."
+                                f" Retry {attempt.retry_state.attempt_number}/{recovery_attempts}"
+                            )
+                            raise Exception
+            except RetryError:
+                raise
+
+    def _restart(self, _: EventBase) -> None:
         """Restart the service."""
-        if self.peers.units != self.restart_peers.units:
-            # defer restart until all units are in the relation
-            logger.debug("Deferring restart until all units are in the relation")
-            event.defer()
+        container = self.unit.get_container(CONTAINER_NAME)
+        if not container.can_connect():
             return
-        if self.peers.units and self._mysql.is_unit_primary(self.unit_label):
-            # delay primary on multi units
-            restart_states = {
-                self.restart_peers.data[unit].get("state", "unset") for unit in self.peers.units
-            }
-            if restart_states == {"unset"}:
-                logger.info("Restarting primary")
-            elif restart_states != {"release"}:
-                # Wait other units restart first to minimize primary switchover
-                message = "Primary restart deferred after other units"
-                logger.info(message)
-                self.unit.status = WaitingStatus(message)
-                event.defer()
-                return
+
+        if not self.unit_initialized:
+            logger.debug("Restarting standalone mysqld")
+            container.restart(MYSQLD_SERVICE)
+            return
+
+        if self.app.planned_units() > 1 and self._mysql.is_unit_primary(self.unit_label):
+            try:
+                new_primary = self.get_unit_address(self.peers.units.pop())
+                logger.debug(f"Switching primary to {new_primary}")
+                self._mysql.set_cluster_primary(new_primary)
+            except MySQLSetClusterPrimaryError:
+                logger.warning("Changing primary failed")
+
+        logger.debug("Restarting mysqld")
         self.unit.status = MaintenanceStatus("restarting MySQL")
-        container = self.unit.get_container(CONTAINER_NAME)
-        if container.can_connect():
-            logger.debug("Restarting mysqld")
-            container.pebble.restart_services([MYSQLD_SAFE_SERVICE], timeout=3600)
-            sleep(10)
-            self._on_update_status(None)
+        container.pebble.restart_services([MYSQLD_SERVICE], timeout=3600)
+        self.unit.status = MaintenanceStatus("recovering unit after restart")
+        sleep(10)
+        self.recover_unit_after_restart()
+
+        self._on_update_status(None)
 
     # =========================================================================
     # Charm event handlers
@@ -647,8 +682,8 @@ def _configure_instance(self, container) -> None:
 
             # Add the pebble layer
             logger.info("Adding pebble layer")
-            container.add_layer(MYSQLD_SAFE_SERVICE, self._pebble_layer, combine=True)
-            container.restart(MYSQLD_SAFE_SERVICE)
+            container.add_layer(MYSQLD_SERVICE, self._pebble_layer, combine=True)
+            container.restart(MYSQLD_SERVICE)
 
             logger.info("Waiting for instance to be ready")
             self._mysql.wait_until_mysql_connection(check_port=False)

src/constants.py

@@ -6,8 +6,8 @@
 PASSWORD_LENGTH = 24
 PEER = "database-peers"
 CONTAINER_NAME = "mysql"
-MYSQLD_LOCATION = "mysqld"
-MYSQLD_SAFE_SERVICE = "mysqld_safe"
+MYSQLD_SERVICE = "mysqld"
+MYSQLD_LOCATION = f"/usr/sbin/{MYSQLD_SERVICE}"
 ROOT_USERNAME = "root"
 CLUSTER_ADMIN_USERNAME = "clusteradmin"
 SERVER_CONFIG_USERNAME = "serverconfig"

src/mysql_k8s_helpers.py

@@ -42,7 +42,7 @@
     MYSQL_SYSTEM_USER,
     MYSQLD_DEFAULTS_CONFIG_FILE,
     MYSQLD_LOCATION,
-    MYSQLD_SAFE_SERVICE,
+    MYSQLD_SERVICE,
     MYSQLD_SOCK_FILE,
     MYSQLSH_LOCATION,
     ROOT_SYSTEM_USER,
@@ -561,22 +561,22 @@ def stop_mysqld(self) -> None:
         """Stops the mysqld process."""
         try:
             # call low-level pebble API to access timeout parameter
-            self.container.pebble.stop_services([MYSQLD_SAFE_SERVICE], timeout=5 * 60)
+            self.container.pebble.stop_services([MYSQLD_SERVICE], timeout=5 * 60)
         except ChangeError:
-            error_message = f"Failed to stop service {MYSQLD_SAFE_SERVICE}"
+            error_message = f"Failed to stop service {MYSQLD_SERVICE}"
             logger.exception(error_message)
             raise MySQLStopMySQLDError(error_message)
 
     def start_mysqld(self) -> None:
         """Starts the mysqld process."""
         try:
-            self.container.start(MYSQLD_SAFE_SERVICE)
+            self.container.start(MYSQLD_SERVICE)
             self.wait_until_mysql_connection()
         except (
             ChangeError,
             MySQLServiceNotRunningError,
         ):
-            error_message = f"Failed to start service {MYSQLD_SAFE_SERVICE}"
+            error_message = f"Failed to start service {MYSQLD_SERVICE}"
             logger.exception(error_message)
             raise MySQLStartMySQLDError(error_message)
 
@@ -792,7 +792,7 @@ def check_if_mysqld_process_stopped(self) -> bool:
             for line in stdout.strip().split("\n"):
                 [comm, stat] = line.split()
 
-                if comm == MYSQLD_SAFE_SERVICE:
+                if comm == MYSQLD_SERVICE:
                     return "T" in stat
 
             return True

src/upgrade.py

@@ -28,23 +28,18 @@
 from ops.model import BlockedStatus, MaintenanceStatus, RelationDataContent
 from ops.pebble import ChangeError
 from pydantic import BaseModel
-from tenacity import RetryError, Retrying
-from tenacity.stop import stop_after_attempt
-from tenacity.wait import wait_fixed
+from tenacity import RetryError
 from typing_extensions import override
 
 import k8s_helpers
-from constants import CONTAINER_NAME, MYSQLD_SAFE_SERVICE
+from constants import CONTAINER_NAME, MYSQLD_SERVICE
 
 if TYPE_CHECKING:
     from charm import MySQLOperatorCharm
 
 logger = logging.getLogger(__name__)
 
 
-RECOVER_ATTEMPTS = 10
-
-
 class MySQLK8sDependenciesModel(BaseModel):
     """MySQL dependencies model."""
 
@@ -231,10 +226,7 @@ def _on_pebble_ready(self, event) -> None:
             self.charm._reconcile_pebble_layer(container)
             self._check_server_upgradeability()
             self.charm.unit.status = MaintenanceStatus("recovering unit after upgrade")
-            if self.charm.app.planned_units() > 1:
-                self._recover_multi_unit_cluster()
-            else:
-                self._recover_single_unit_cluster()
+            self.charm.recover_unit_after_restart()
             if self.charm.config.plugin_audit_enabled:
                 self.charm._mysql.install_plugins(["audit_log"])
             self._complete_upgrade()
@@ -269,28 +261,6 @@ def _on_pebble_ready(self, event) -> None:
             self._reset_on_unsupported_downgrade(container)
             self._complete_upgrade()
 
-    def _recover_multi_unit_cluster(self) -> None:
-        logger.info("Recovering unit")
-        try:
-            for attempt in Retrying(
-                stop=stop_after_attempt(RECOVER_ATTEMPTS), wait=wait_fixed(10)
-            ):
-                with attempt:
-                    self.charm._mysql.hold_if_recovering()
-                    if not self.charm._mysql.is_instance_in_cluster(self.charm.unit_label):
-                        logger.debug(
-                            "Instance not yet back in the cluster."
-                            f" Retry {attempt.retry_state.attempt_number}/{RECOVER_ATTEMPTS}"
-                        )
-                        raise Exception
-        except RetryError:
-            raise
-
-    def _recover_single_unit_cluster(self) -> None:
-        """Recover single unit cluster."""
-        logger.debug("Recovering single unit cluster")
-        self.charm._mysql.reboot_from_complete_outage()
-
     def _complete_upgrade(self):
         # complete upgrade for the unit
         logger.debug("Upgraded unit is healthy. Set upgrade state to `completed`")
@@ -346,7 +316,7 @@ def _check_server_unsupported_downgrade(self) -> bool:
 
     def _reset_on_unsupported_downgrade(self, container: Container) -> None:
         """Reset the cluster on unsupported downgrade."""
-        container.stop(MYSQLD_SAFE_SERVICE)
+        container.stop(MYSQLD_SERVICE)
         self.charm._mysql.reset_data_dir()
         self.charm._write_mysqld_configuration()
         self.charm._configure_instance(container)

tests/integration/helpers.py

@@ -24,7 +24,7 @@
 from pytest_operator.plugin import OpsTest
 from tenacity import RetryError, Retrying, retry, stop_after_attempt, wait_fixed
 
-from constants import CONTAINER_NAME, MYSQLD_SAFE_SERVICE, SERVER_CONFIG_USERNAME
+from constants import CONTAINER_NAME, MYSQLD_SERVICE, SERVER_CONFIG_USERNAME
 
 from . import juju_
 from .connector import MySQLConnector
@@ -480,7 +480,7 @@ async def stop_mysqld_service(ops_test: OpsTest, unit_name: str) -> None:
         unit_name: The name of the unit
     """
     await ops_test.juju(
-        "ssh", "--container", CONTAINER_NAME, unit_name, "pebble", "stop", MYSQLD_SAFE_SERVICE
+        "ssh", "--container", CONTAINER_NAME, unit_name, "pebble", "stop", MYSQLD_SERVICE
     )
 
 
@@ -492,7 +492,7 @@ async def start_mysqld_service(ops_test: OpsTest, unit_name: str) -> None:
         unit_name: The name of the unit
     """
     await ops_test.juju(
-        "ssh", "--container", CONTAINER_NAME, unit_name, "pebble", "start", MYSQLD_SAFE_SERVICE
+        "ssh", "--container", CONTAINER_NAME, unit_name, "pebble", "start", MYSQLD_SERVICE
     )
 
 

tests/integration/high_availability/high_availability_helpers.py

@@ -207,6 +207,7 @@ async def deploy_and_scale_application(
             num_units=1,
             channel="latest/edge",
             base="[email protected]",
+            config={"sleep_interval": 300},
         )
 
         await ops_test.model.wait_for_idle(

tests/integration/high_availability/manifests/extend_pebble_restart_delay.yml

@@ -1,5 +1,5 @@
 services:
-  mysqld_safe:
+  mysqld:
     override: merge
     backoff-delay: 300s
     backoff-limit: 300s

tests/integration/high_availability/manifests/reduce_pebble_restart_delay.yml

@@ -1,5 +1,5 @@
 services:
-  mysqld_safe:
+  mysqld:
     override: merge
     backoff-delay: 500ms
     backoff-limit: 30s