DPE-3115 Fix MAAS deployment (#444)

* refactor hostname resolution with maas fix

* call on init

* proper version definition and more specific offending address filtering

Author: paulomach

poetry.lock

@@ -17,6 +17,7 @@ boto3 = "^1.28.23"
 pyopenssl = "^24.0.0"
 typing_extensions = "^4.7.1"
 jinja2 = "^3.1.2"
+python_hosts = "^1.0.6"
 
 [tool.poetry.group.charm-libs.dependencies]
 # data_platform_libs/v0/data_interfaces.py

pyproject.toml

@@ -620,6 +620,9 @@ def workload_initialise(self) -> None:
         self._mysql.reset_root_password_and_start_mysqld()
         self._mysql.configure_mysql_users()
 
+        # ensure hostname can be resolved
+        self.hostname_resolution.update_etc_hosts(None)
+
         current_mysqld_pid = self._mysql.get_pid_of_port_3306()
         self._mysql.configure_instance()
 

src/charm.py

@@ -3,15 +3,14 @@
 
 """Library containing logic pertaining to hostname resolutions in the VM charm."""
 
-import io
 import json
 import logging
 import socket
 import typing
 
-from ops.charm import RelationDepartedEvent
 from ops.framework import Object
-from ops.model import BlockedStatus, Unit
+from ops.model import Unit
+from python_hosts import Hosts, HostsEntry
 
 from constants import HOSTNAME_DETAILS, PEER
 from ip_address_observer import IPAddressChangeCharmEvents, IPAddressObserver
@@ -22,11 +21,15 @@
 if typing.TYPE_CHECKING:
     from charm import MySQLOperatorCharm
 
+COMMENT = "Managed by mysql charm"
+
 
 class MySQLMachineHostnameResolution(Object):
     """Encapsulation of the the machine hostname resolution."""
 
-    on = IPAddressChangeCharmEvents()
+    on = (  # pyright: ignore [reportIncompatibleMethodOverride, reportAssignmentType
+        IPAddressChangeCharmEvents()
+    )
 
     def __init__(self, charm: "MySQLOperatorCharm"):
         super().__init__(charm, "hostname-resolution")
@@ -38,12 +41,8 @@ def __init__(self, charm: "MySQLOperatorCharm"):
         self.framework.observe(self.charm.on.config_changed, self._update_host_details_in_databag)
         self.framework.observe(self.on.ip_address_change, self._update_host_details_in_databag)
 
-        self.framework.observe(
-            self.charm.on[PEER].relation_changed, self._potentially_update_etc_hosts
-        )
-        self.framework.observe(
-            self.charm.on[PEER].relation_departed, self._remove_host_from_etc_hosts
-        )
+        self.framework.observe(self.charm.on[PEER].relation_changed, self.update_etc_hosts)
+        self.framework.observe(self.charm.on[PEER].relation_departed, self.update_etc_hosts)
 
         self.ip_address_observer.start_observer()
 
@@ -60,111 +59,58 @@ def _update_host_details_in_databag(self, _) -> None:
             logger.exception("Unable to get local IP address")
             ip = "127.0.0.1"
 
-        host_details = {
-            "hostname": hostname,
-            "fqdn": fqdn,
-            "ip": ip,
-        }
+        host_details = {"names": [hostname, fqdn], "address": ip}
 
         self.charm.unit_peer_data[HOSTNAME_DETAILS] = json.dumps(host_details)
 
-    def _get_host_details(self) -> dict[str, str]:
-        host_details = {}
+    def _get_host_details(self) -> list[HostsEntry]:
+        host_details = list()
+
+        if not self.charm.peers:
+            return []
 
         for key, data in self.charm.peers.data.items():
             if isinstance(key, Unit) and data.get(HOSTNAME_DETAILS):
                 unit_details = json.loads(data[HOSTNAME_DETAILS])
-                unit_details["unit"] = key.name
-                host_details[unit_details["hostname"]] = unit_details
-
-        return host_details
-
-    def _does_etc_hosts_need_update(self, host_details: dict[str, str]) -> bool:
-        outdated_hosts = host_details.copy()
 
-        with open("/etc/hosts", "r") as hosts_file:
-            for line in hosts_file:
-                if "# unit=" not in line:
-                    continue
+                if unit_details.get("address"):
+                    entry = HostsEntry(comment=COMMENT, entry_type="ipv4", **unit_details)
+                else:
+                    # case when migrating from old format
+                    entry = HostsEntry(
+                        address=unit_details["ip"],
+                        names=[unit_details["hostname"], unit_details["fqdn"]],
+                        comment=COMMENT,
+                        entry_type="ipv4",
+                    )
 
-                ip, fqdn, hostname = line.split("#")[0].strip().split()
-                if outdated_hosts.get(hostname).get("ip") == ip:
-                    outdated_hosts.pop(hostname)
+                host_details.append(entry)
 
-        return bool(outdated_hosts)
+        return host_details
 
-    def _potentially_update_etc_hosts(self, _) -> None:
+    def update_etc_hosts(self, _) -> None:
         """Potentially update the /etc/hosts file with new hostname to IP for units."""
         if not self.charm._is_peer_data_set:
             return
 
-        host_details = self._get_host_details()
-        if not host_details:
+        host_entries = self._get_host_details()
+        if not host_entries:
             logger.debug("No hostnames in the peer databag. Skipping update of /etc/hosts")
             return
 
-        if not self._does_etc_hosts_need_update(host_details):
-            logger.debug("No hostnames in /etc/hosts changed. Skipping update to /etc/hosts")
-            return
-
-        hosts_in_file = []
-
-        with io.StringIO() as updated_hosts_file:
-            with open("/etc/hosts", "r") as hosts_file:
-                for line in hosts_file:
-                    if "# unit=" not in line:
-                        updated_hosts_file.write(line)
-                        continue
-
-                    for hostname, details in host_details.items():
-                        if hostname == line.split()[2]:
-                            hosts_in_file.append(hostname)
-
-                            fqdn, ip, unit = details["fqdn"], details["ip"], details["unit"]
-
-                            logger.debug(
-                                f"Overwriting {hostname} ({unit=}) with {ip=}, {fqdn=} in /etc/hosts"
-                            )
-                            updated_hosts_file.write(f"{ip} {fqdn} {hostname} # unit={unit}\n")
-                            break
-
-            for hostname, details in host_details.items():
-                if hostname not in hosts_in_file:
-                    fqdn, ip, unit = details["fqdn"], details["ip"], details["unit"]
-
-                    logger.debug(f"Adding {hostname} ({unit=} with {ip=}, {fqdn=} in /etc/hosts")
-                    updated_hosts_file.write(f"{ip} {fqdn} {hostname} # unit={unit}\n")
-
-            with open("/etc/hosts", "w") as hosts_file:
-                hosts_file.write(updated_hosts_file.getvalue())
-
-        try:
-            self.charm._mysql.flush_host_cache()
-        except MySQLFlushHostCacheError:
-            self.charm.unit.status = BlockedStatus("Unable to flush MySQL host cache")
-
-    def _remove_host_from_etc_hosts(self, event: RelationDepartedEvent) -> None:
-        departing_unit_name = event.unit.name
-
-        logger.debug(f"Checking if an entry for {departing_unit_name} is in /etc/hosts")
-        with open("/etc/hosts", "r") as hosts_file:
-            for line in hosts_file:
-                if f"# unit={departing_unit_name}" in line:
-                    break
-            else:
-                return
+        logger.debug("Updating /etc/hosts with new hostname to IP mappings")
+        hosts = Hosts()
 
-        logger.debug(f"Removing entry for {departing_unit_name} from /etc/hosts")
-        with io.StringIO() as updated_hosts_file:
-            with open("/etc/hosts", "r") as hosts_file:
-                for line in hosts_file:
-                    if f"# unit={departing_unit_name}" not in line:
-                        updated_hosts_file.write(line)
+        if hosts.exists(address="127.0.1.1", names=[socket.getfqdn()]):
+            # remove MAAS injected entry
+            logger.debug("Removing MAAS injected entry from /etc/hosts")
+            hosts.remove_all_matching(address="127.0.1.1")
 
-            with open("/etc/hosts", "w") as hosts_file:
-                hosts_file.write(updated_hosts_file.getvalue())
+        hosts.remove_all_matching(comment=COMMENT)
+        hosts.add(host_entries)
+        hosts.write()
 
         try:
             self.charm._mysql.flush_host_cache()
         except MySQLFlushHostCacheError:
-            self.charm.unit.status = BlockedStatus("Unable to flush MySQL host cache")
+            logger.warning("Unable to flush MySQL host cache")

src/hostname_resolution.py

@@ -327,7 +327,7 @@ def test_can_unit_perform_backup(
     @patch_network_get(private_address="1.1.1.1")
     @patch("mysql_vm_helpers.MySQL.offline_mode_and_hidden_instance_exists", return_value=False)
     @patch("mysql_vm_helpers.MySQL.get_member_state")
-    @patch("hostname_resolution.MySQLMachineHostnameResolution._remove_host_from_etc_hosts")
+    @patch("python_hosts.Hosts.write")
     def test_can_unit_perform_backup_failure(
         self,
         _,
@@ -379,7 +379,7 @@ def test_can_unit_perform_backup_failure(
     @patch_network_get(private_address="1.1.1.1")
     @patch("mysql_vm_helpers.MySQL.set_instance_option")
     @patch("mysql_vm_helpers.MySQL.set_instance_offline_mode")
-    @patch("hostname_resolution.MySQLMachineHostnameResolution._remove_host_from_etc_hosts")
+    @patch("python_hosts.Hosts.write")
     def test_pre_backup(
         self,
         _,
@@ -549,7 +549,7 @@ def test_pre_restore_checks(
     @patch_network_get(private_address="1.1.1.1")
     @patch("mysql_vm_helpers.MySQL.is_server_connectable", return_value=True)
     @patch("charm.MySQLOperatorCharm.is_unit_busy", return_value=False)
-    @patch("hostname_resolution.MySQLMachineHostnameResolution._remove_host_from_etc_hosts")
+    @patch("python_hosts.Hosts.write")
     def test_pre_restore_checks_failure(
         self,
         _,

tests/unit/test_backups.py

@@ -290,7 +290,7 @@ def test_on_start_exceptions(
     @patch("charm.is_volume_mounted", return_value=True)
     @patch("mysql_vm_helpers.MySQL.reboot_from_complete_outage")
     @patch("charm.snap_service_operation")
-    @patch("hostname_resolution.MySQLMachineHostnameResolution._remove_host_from_etc_hosts")
+    @patch("python_hosts.Hosts.write")
     def test_on_update(
         self,
         _,

tests/unit/test_charm.py

@@ -0,0 +1,62 @@
+# Copyright 2024 Canonical Ltd.
+# See LICENSE file for licensing details.
+
+import json
+import unittest
+from unittest.mock import patch
+
+from ops.testing import Harness
+
+from charm import MySQLOperatorCharm
+from constants import HOSTNAME_DETAILS, PEER
+
+APP_NAME = "mysql"
+
+
+class TestHostnameResolution(unittest.TestCase):
+    def setUp(self):
+        self.harness = Harness(MySQLOperatorCharm)
+        self.addCleanup(self.harness.cleanup)
+        self.harness.begin()
+        self.charm = self.harness.charm
+        self.hostname_resolution = self.charm.hostname_resolution
+
+    def test_get_host_details(self):
+        """Test get_peer_host_details method."""
+        host_entries = self.hostname_resolution._get_host_details()
+
+        # before relation
+        self.assertEqual(host_entries, [])
+
+        # Add relation
+        id = self.harness.add_relation(PEER, APP_NAME)
+
+        host_entries = self.hostname_resolution._get_host_details()
+        self.assertEqual(host_entries, [])
+
+        # Add unit
+        self.harness.add_relation_unit(id, f"{APP_NAME}/0")
+        self.harness.update_relation_data(
+            id,
+            f"{APP_NAME}/0",
+            {
+                HOSTNAME_DETAILS: json.dumps(
+                    {"address": "1.1.1.1", "names": ["name1", "name2", "name3"]}
+                )
+            },
+        )
+
+        host_entries = self.hostname_resolution._get_host_details()
+        self.assertEqual(len(host_entries), 1)
+        self.assertEqual(host_entries[0].address, "1.1.1.1")
+
+    @patch("socket.gethostname", return_value="mysql-0")
+    def test_update_host_details_in_databag(self, _gethostname_mock):
+        """Test update_host_details_in_databag method."""
+        # Add relation
+        self.harness.add_relation(PEER, APP_NAME)
+        self.assertEqual(self.charm.unit_peer_data.get(HOSTNAME_DETAILS), None)
+        self.hostname_resolution._update_host_details_in_databag(None)
+        _gethostname_mock.assert_called()
+
+        self.assertTrue("mysql-0" in self.charm.unit_peer_data[HOSTNAME_DETAILS])