diff --git a/bundle-examples/internal-haproxy.bundle.yaml b/bundle-examples/internal-haproxy.bundle.yaml index c80084d0..5392e7a6 100644 --- a/bundle-examples/internal-haproxy.bundle.yaml +++ b/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 diff --git a/src/charm.py b/src/charm.py index c280200d..75fa2770 100755 --- a/src/charm.py +++ b/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: diff --git a/src/database.py b/src/database.py index 8e1d616e..48456c95 100644 --- a/src/database.py +++ b/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 diff --git a/tests/integration/test_bundle.py b/tests/integration/test_bundle.py index 83701e1a..80e56fe5 100644 --- a/tests/integration/test_bundle.py +++ b/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. diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index 7eee0c98..f656d044 100644 --- a/tests/unit/test_charm.py +++ b/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): diff --git a/tests/unit/test_database_relation.py b/tests/unit/test_database_relation.py index d99935af..8a57828e 100644 --- a/tests/unit/test_database_relation.py +++ b/tests/unit/test_database_relation.py @@ -3,12 +3,14 @@ from unittest.mock import call, Mock, patch from charms.data_platform_libs.v0.data_interfaces import DatabaseCreatedEvent -from ops.model import ActiveStatus, MaintenanceStatus +from ops.model import ActiveStatus, BlockedStatus, MaintenanceStatus from ops.testing import Context, Relation, State, StoredState import pytest +from scenario.errors import UncaughtCharmError from charm import ( LandscapeServerCharm, + SCHEMA_SCRIPT, ) from database import ( DatabaseConnectionContext, @@ -16,6 +18,7 @@ fetch_postgres_relation_data, get_postgres_owner_role_from_version, grant_role, + PgHbaNotReadyError, PostgresRoles, ) @@ -709,3 +712,197 @@ def test_grant_role_raises_on_error(self, execute_psql_mock): execute_psql_mock.assert_called_once() logger.error.assert_called_once() + + +class TestBootstrapAccount: + _ADMIN_CONFIG = { + "admin_email": "admin@example.com", + "admin_name": "Admin", + "admin_password": "password123", + "root_url": "https://landscape.local", + } + + @staticmethod + def _state( + *, account_bootstrapped: bool = False, config: dict | None = None + ) -> State: + stored = StoredState( + owner_path="LandscapeServerCharm", + content={ + "ready": { + "db": False, + "inbound-amqp": False, + "outbound-amqp": False, + "load-balancer-certificates": False, + }, + "account_bootstrapped": account_bootstrapped, + }, + ) + return State( + leader=True, + config=config or TestBootstrapAccount._ADMIN_CONFIG, + stored_states=[stored], + ) + + def test_pg_hba_error_raises(self): + """pg_hba.conf race raises PgHbaNotReadyError so Juju retries the hook.""" + ctx = Context(LandscapeServerCharm) + state_in = self._state() + pg_hba = Mock(returncode=1, stdout="", stderr="no pg_hba.conf entry for host") + + with patch("charm.subprocess.run", return_value=pg_hba): + with ctx(ctx.on.start(), state_in) as manager: + with pytest.raises(PgHbaNotReadyError): + manager.charm._bootstrap_account() + + def test_pg_hba_error_does_not_mark_bootstrapped(self): + ctx = Context(LandscapeServerCharm) + state_in = self._state() + pg_hba = Mock(returncode=1, stdout="", stderr="no pg_hba.conf entry for host") + + with patch("charm.subprocess.run", return_value=pg_hba): + with ctx(ctx.on.start(), state_in) as manager: + with pytest.raises(PgHbaNotReadyError): + manager.charm._bootstrap_account() + assert manager.charm._stored.account_bootstrapped is False + + def test_success_marks_bootstrapped(self): + ctx = Context(LandscapeServerCharm) + state_in = self._state() + + with patch( + "charm.subprocess.run", + return_value=Mock(returncode=0, stdout="", stderr=""), + ): + with ctx(ctx.on.start(), state_in) as manager: + manager.charm._bootstrap_account() + assert manager.charm._stored.account_bootstrapped is True + + def test_duplicate_account_marks_bootstrapped(self): + ctx = Context(LandscapeServerCharm) + state_in = self._state() + dup = Mock(returncode=1, stdout="", stderr="DuplicateAccountError") + + with patch("charm.subprocess.run", return_value=dup): + with ctx(ctx.on.start(), state_in) as manager: + manager.charm._bootstrap_account() + assert manager.charm._stored.account_bootstrapped is True + + def test_generic_error_does_not_raise(self): + """Non-pg_hba failures are logged and swallowed — no raise.""" + ctx = Context(LandscapeServerCharm) + state_in = self._state() + error = Mock(returncode=1, stdout="", stderr="some other error") + + with patch("charm.subprocess.run", return_value=error): + with ctx(ctx.on.start(), state_in) as manager: + manager.charm._bootstrap_account() + + def test_already_bootstrapped_skips_script(self): + ctx = Context(LandscapeServerCharm) + state_in = self._state(account_bootstrapped=True) + + with patch("charm.subprocess.run") as run_mock: + with ctx(ctx.on.start(), state_in) as manager: + manager.charm._bootstrap_account() + run_mock.assert_not_called() + + def test_migrate_schema_bootstrap_propagates_pg_hba_raise(self): + """_migrate_schema_bootstrap does not swallow PgHbaNotReadyError, + so it propagates to the event handler and fails the hook.""" + ctx = Context(LandscapeServerCharm) + state_in = self._state() + + with ( + patch("charm.check_call"), + patch.object( + LandscapeServerCharm, + "_bootstrap_account", + side_effect=PgHbaNotReadyError("no pg_hba.conf entry"), + ), + ): + with ctx(ctx.on.start(), state_in) as manager: + with pytest.raises(PgHbaNotReadyError): + manager.charm._migrate_schema_bootstrap() + + def test_migrate_schema_bootstrap_blocks_on_schema_failure(self): + ctx = Context(LandscapeServerCharm) + state_in = self._state() + + with patch( + "charm.check_call", side_effect=CalledProcessError(1, SCHEMA_SCRIPT) + ): + with ctx(ctx.on.start(), state_in) as manager: + result = manager.charm._migrate_schema_bootstrap() + assert result is False + assert isinstance(manager.charm.unit.status, BlockedStatus) + + def test_pg_hba_error_fails_database_hook(self): + """pg_hba.conf race causes UncaughtCharmError so Juju retries the hook.""" + ctx = Context(LandscapeServerCharm) + relation = Relation( + "database", + remote_app_name="postgresql", + remote_app_data={ + "username": "landscape", + "password": "secret", + "endpoints": "1.2.3.4:5432", + "version": "14.8", + "database": "landscape", + }, + ) + state_in = State( + leader=True, + relations=[relation], + config=self._ADMIN_CONFIG, + stored_states=[ + StoredState( + owner_path="LandscapeServerCharm", + content={ + "ready": { + "db": False, + "inbound-amqp": False, + "outbound-amqp": False, + "load-balancer-certificates": False, + }, + "account_bootstrapped": False, + }, + ) + ], + ) + pg_hba = Mock(returncode=1, stdout="", stderr="no pg_hba.conf entry for host") + + with ( + patch( + "charm.fetch_postgres_relation_data", + return_value=DatabaseConnectionContext( + host="1.2.3.4", + port="5432", + username="landscape", + password="secret", + version="14.8", + ), + ), + patch("charm.update_db_conf"), + patch("charm.check_call"), + patch( + "charm.get_postgres_roles", + return_value=PostgresRoles( + relation="landscape", + application="landscape-app", + owner="postgres", + superuser=None, + ), + ), + patch("charm.grant_role"), + patch("charm.subprocess.run", return_value=pg_hba), + patch( + "charm.LandscapeServerCharm._update_wsl_distributions", + return_value=True, + ), + patch("charm.LandscapeServerCharm._update_ready_status"), + ): + with pytest.raises(UncaughtCharmError) as exc_info: + ctx.run(ctx.on.relation_changed(relation), state_in) + + assert isinstance(exc_info.value.__cause__, PgHbaNotReadyError)