fix: retry account bootstrap with modern database relation to accomodate pg_hba not being ready yet

bundle-examples/internal-haproxy.bundle.yaml

@@ -27,6 +27,9 @@ applications:
       enable_hostagent_messenger: True
       enable_ubuntu_installer_attach: True
       root_url: https://landscape.local/
+      admin_email: john@example.com
+      admin_name: john
+      admin_password: pwd
     base: ubuntu@24.04
   lb-certs:
     charm: self-signed-certificates

src/charm.py

@@ -73,6 +73,7 @@
     DatabaseConnectionContext,
     fetch_postgres_relation_data,
     grant_role,
+    PgHbaNotReadyError,
 )
 import haproxy
 from helpers import get_modified_env_vars, logger, migrate_service_conf
@@ -104,6 +105,7 @@
 UPDATE_WSL_DISTRIBUTIONS_SCRIPT = "/opt/canonical/landscape/update-wsl-distributions"
 LANDSCAPE_ERROR_FILES_DIR = "/opt/canonical/landscape/canonical/landscape/offline"
 
+
 LANDSCAPE_SERVER = "landscape-server"
 LANDSCAPE_PACKAGES = (
     LANDSCAPE_SERVER,
@@ -1027,15 +1029,17 @@ def _migrate_schema_bootstrap(self, owner_role: str | None = None):
 
         try:
             check_call(call, env=get_modified_env_vars())
-            self._bootstrap_account()
-            self._set_autoregistration()
-            return True
         except CalledProcessError as e:
             logger.error(
                 "Landscape Server schema update failed with return code %d",
                 e.returncode,
             )
             self.unit.status = BlockedStatus("Failed to update database schema")
+            return False
+
+        self._bootstrap_account()
+        self._set_autoregistration()
+        return True
 
     def _update_wsl_distributions(self) -> bool | None:
         logger.info("Updating WSL distributions...")
@@ -1558,6 +1562,11 @@ def _bootstrap_account(self):
             if "DuplicateAccountError" in result.stderr:
                 logger.error("Cannot bootstrap b/c account is already there!")
                 self._stored.account_bootstrapped = True
+            elif "no pg_hba.conf entry" in result.stderr:
+                # Patroni regenerates pg_hba.conf asynchronously after the
+                # landscape role is created by the schema script. Raise so
+                # Juju retries the hook once Patroni has reloaded.
+                raise PgHbaNotReadyError(result.stderr)
             else:
                 logger.error(result.stderr)
         else:

src/database.py

@@ -6,6 +6,11 @@
 from helpers import get_modified_env_vars, logger
 
 
+class PgHbaNotReadyError(Exception):
+    """Raised when bootstrap-account fails because pg_hba.conf hasn't been
+    updated by Patroni yet. Propagating this causes Juju to retry the hook."""
+
+
 @dataclass
 class DatabaseConnectionContext:
     host: str | None = None

tests/integration/test_bundle.py

@@ -225,6 +225,42 @@ def test_modern_database_relation(juju: jubilant.Juju, bundle: None):
     restore_db_relations(juju, initial_relations)
 
 
+def test_bootstrap_account_created_with_modern_database(
+    juju: jubilant.Juju, bundle: None
+):
+    """
+    When admin_email/name/password are configured, the bootstrap-account script
+    must succeed even though Patroni regenerates pg_hba.conf asynchronously after
+    the landscape role is created by landscape-schema --bootstrap.
+    """
+    if not has_modern_pg(juju):
+        pytest.skip("Modern database relation not active")
+
+    admin_email = juju.config("landscape-server").get("admin_email")
+    if not admin_email:
+        pytest.skip("admin_email not configured")
+
+    juju.wait(jubilant.all_active, timeout=300)
+
+    conf = juju.ssh(
+        "landscape-server/leader",
+        "sudo awk -F' = ' '/^\\[stores\\]/{f=1} /^\\[/{if(!/^\\[stores\\]/)f=0} "
+        "f && /^host/{h=$2} f && /^password/{pw=$2} "
+        "f && /^user/{u=$2} f && /^main/{m=$2} "
+        "END{print h,pw,u,m}' /etc/landscape/service.conf",
+    ).split()
+    host, port = conf[0].split(":")
+    password, user, dbname = conf[1], conf[2], conf[3]
+    result = juju.ssh(
+        "landscape-server/leader",
+        f"PGPASSWORD={password} psql -h {host} -p {port} -U {user} -d {dbname}"
+        " -tAc 'SELECT email FROM person;'",
+    )
+    assert (
+        admin_email in result
+    ), f"Admin {admin_email} not found in person table after bootstrap. "
+
+
 def test_legacy_db_relation(juju: jubilant.Juju, bundle: None):
     """
     Test the legacy `db` interface.

tests/unit/test_charm.py

@@ -30,6 +30,7 @@
 from scenario.errors import UncaughtCharmError
 
 from charm import (
+    BOOTSTRAP_ACCOUNT_SCRIPT,
     DEFAULT_SERVICES,
     get_modified_env_vars,
     HASH_ID_DATABASES,
@@ -1735,15 +1736,13 @@ def test_on_replicas_relation_changed_non_leader(self):
         )
 
 
-# TODO fix from broken commit.
-@unittest.skip("Broken in `de29548e2b09c71db3a55f606ab318b5ea25550d`")
 class TestBootstrapAccount(unittest.TestCase):
     def setUp(self):
         self.harness = Harness(LandscapeServerCharm)
         self.addCleanup(self.harness.cleanup)
 
         self.harness.model.get_binding = Mock(
-            return_value=Mock(bind_address="123.123.123.123")
+            return_value=Mock(network=Mock(bind_address="123.123.123.123"))
         )
         self.harness.add_relation("replicas", "landscape-server")
         self.harness.set_leader()
@@ -1753,24 +1752,32 @@ def setUp(self):
         grp_mock = patch("charm.group_exists").start()
         grp_mock.return_value = Mock(spec_set=struct_group, gr_gid=1000)
 
-        self.process_mock = patch("subprocess.run").start()
+        self.process_mock = patch("charm.subprocess.run").start()
         self.log_mock = patch("charm.logger.error").start()
         self.log_info_mock = patch("charm.logger.info").start()
 
-        env_mock = patch("os.environ").start()
-        env_mock.copy.return_value = {}
+        patch("os.environ.copy", return_value={}).start()
 
         self.addCleanup(patch.stopall)
 
         self.harness.begin()
 
+    def _bootstrap_calls(self):
+        return [
+            c
+            for c in self.process_mock.call_args_list
+            if c.args and c.args[0] and c.args[0][0] == BOOTSTRAP_ACCOUNT_SCRIPT
+        ]
+
     @patch("charm.update_service_conf")
     def test_bootstrap_account_doesnt_run_with_missing_configs(self, _):
         self.harness.update_config(
             {"admin_email": "hello@ubuntu.com", "admin_name": "Hello Ubuntu"}
         )
-        self.assertIn("password required", self.log_mock.call_args.args[0])
-        self.process_mock.assert_not_called()
+        self.log_mock.assert_any_call(
+            "Admin email, name, and password required for bootstrap account"
+        )
+        self.assertEqual(self._bootstrap_calls(), [])
 
     @patch("charm.update_service_conf")
     def test_bootstrap_account_password_redacted(self, _):
@@ -1795,8 +1802,8 @@ def test_bootstrap_account_doesnt_run_with_missing_rooturl(self, _):
                 "admin_password": "password",
             }
         )
-        self.assertIn("root url", self.log_mock.call_args.args[0])
-        self.process_mock.assert_not_called()
+        self.log_mock.assert_any_call("Bootstrap account waiting on default root url..")
+        self.assertEqual(self._bootstrap_calls(), [])
 
     @patch("charm.update_service_conf")
     def test_bootstrap_account_default_root_url_is_used(self, _):
@@ -1808,9 +1815,11 @@ def test_bootstrap_account_default_root_url_is_used(self, _):
                 "admin_password": "password",
             }
         )
+        calls = self._bootstrap_calls()
+        self.assertEqual(len(calls), 1)
         self.assertIn(
             self.harness.charm._stored.default_root_url,
-            self.process_mock.call_args.args[0],
+            calls[0].args[0],
         )
 
     @patch("charm.update_service_conf")
@@ -1826,15 +1835,14 @@ def test_bootstrap_account_config_url_over_default(self, _):
                 "root_url": config_root_url,
             }
         )
-        self.assertIn(config_root_url, self.process_mock.call_args.args[0])
+        calls = self._bootstrap_calls()
+        self.assertEqual(len(calls), 1)
+        self.assertIn(config_root_url, calls[0].args[0])
 
     @patch("charm.update_service_conf")
-    def test_bootstrap_account_runs_once_with_correct_args(self, _):
-        """
-        Test that bootstrap account runs with correct args and that it can't
-        run again after a successful run
-        """
-        self.process_mock.return_value.returncode = 0  # Success
+    def test_bootstrap_account_runs_once_and_not_again_on_success(self, _):
+        """bootstrap-account runs once on success and does not run again."""
+        self.process_mock.return_value.returncode = 0
         admin_email = "hello@ubuntu.com"
         admin_name = "Hello Ubuntu"
         admin_password = "password"
@@ -1846,6 +1854,8 @@ def test_bootstrap_account_runs_once_with_correct_args(self, _):
             "root_url": root_url,
         }
         self.harness.update_config(config)
+        calls = self._bootstrap_calls()
+        self.assertEqual(len(calls), 1)
         self.assertEqual(
             [
                 "/opt/canonical/landscape/bootstrap-account",
@@ -1858,33 +1868,27 @@ def test_bootstrap_account_runs_once_with_correct_args(self, _):
                 "--root_url",
                 root_url,
             ],
-            self.process_mock.call_args.args[0],
+            calls[0].args[0],
         )
         self.harness.update_config(config)
-        self.process_mock.assert_called_once()
+        self.assertEqual(len(self._bootstrap_calls()), 1)
 
     @patch("charm.update_service_conf")
-    def test_bootstrap_account_runs_twice_if_error(self, _):
-        """
-        If there's an error ensure that bootstrap account runs again and not
-        a third time if successful
-        """
-        self.process_mock.return_value.returncode = 1  # Error here
-        admin_email = "hello@ubuntu.com"
-        admin_name = "Hello Ubuntu"
-        admin_password = "password"
-        root_url = "https://www.landscape.com"
+    def test_bootstrap_account_retries_after_generic_error(self, _):
+        """After a generic error, bootstrap runs again on the next config-changed."""
+        self.process_mock.return_value.returncode = 1
+        self.process_mock.return_value.stderr = "some transient error"
         config = {
-            "admin_email": admin_email,
-            "admin_name": admin_name,
-            "admin_password": admin_password,
-            "root_url": root_url,
+            "admin_email": "hello@ubuntu.com",
+            "admin_name": "Hello Ubuntu",
+            "admin_password": "password",
+            "root_url": "https://www.landscape.com",
         }
         self.harness.update_config(config)
         self.process_mock.return_value.returncode = 0
         self.harness.update_config(config)
         self.harness.update_config(config)  # Third time
-        self.assertEqual(self.process_mock.call_count, 2)
+        self.assertEqual(len(self._bootstrap_calls()), 2)
 
     @patch("charm.update_service_conf")
     def test_bootstrap_account_cannot_run_if_already_bootstrapped(
@@ -1909,7 +1913,7 @@ def test_bootstrap_account_cannot_run_if_already_bootstrapped(
         self.harness.update_config(config)
         self.harness.update_config(config)
         self.harness.update_config(config)  # Third time
-        self.process_mock.assert_called_once()
+        self.assertEqual(len(self._bootstrap_calls()), 1)
 
     @patch("subprocess.run")
     def test_hash_id_databases(self, run_mock):