Skip to content

Commit

Permalink
Correctly handle calls from on install hook
Browse files Browse the repository at this point in the history
  • Loading branch information
marceloneppel committed Apr 6, 2022
1 parent 6f0ef9e commit fac4ee2
Show file tree
Hide file tree
Showing 2 changed files with 81 additions and 35 deletions.
49 changes: 35 additions & 14 deletions src/charm.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,11 @@ def _on_install(self, event) -> None:
self._cluster.inhibit_default_cluster_creation()

# Install the PostgreSQL and Patroni requirements packages.
self._install_apt_packages(event, ["postgresql", "python3-pip", "python3-psycopg2"])
try:
self._install_apt_packages(event, ["postgresql", "python3-pip", "python3-psycopg2"])
except (subprocess.CalledProcessError, apt.PackageNotFoundError):
self.unit.status = BlockedStatus("failed to install apt packages")
return

try:
resource_path = self.model.resources.fetch("patroni")
Expand All @@ -65,8 +69,12 @@ def _on_install(self, event) -> None:
return

# Build Patroni package path with raft dependency and install it.
patroni_package_path = f"{str(resource_path)}[raft]"
self._install_pip_packages([patroni_package_path])
try:
patroni_package_path = f"{str(resource_path)}[raft]"
self._install_pip_packages([patroni_package_path])
except subprocess.SubprocessError:
self.unit.status = BlockedStatus("failed to install Patroni python package")
return

self.unit.status = WaitingStatus("waiting to start PostgreSQL")

Expand Down Expand Up @@ -120,24 +128,37 @@ def _get_postgres_password(self) -> str:
return data.get("postgres-password", None)

def _install_apt_packages(self, _, packages: List[str]) -> None:
"""Simple wrapper around 'apt-get install -y."""
"""Simple wrapper around 'apt-get install -y.
Raises:
CalledProcessError if it fails to update the apt cache.
PackageNotFoundError if the package is not in the cache.
PackageError if the packages could not be installed.
"""
try:
logger.debug("updating apt cache")
apt.update()
except subprocess.CalledProcessError as e:
logger.exception("failed to update apt cache, CalledProcessError", exc_info=e)
self.unit.status = BlockedStatus("failed to update apt cache")
return
raise

try:
logger.debug(f"installing apt packages: {', '.join(packages)}")
apt.add_package(packages)
except apt.PackageNotFoundError:
logger.error("a specified package not found in package cache or on system")
self.unit.status = BlockedStatus("failed to install packages")
for package in packages:
try:
apt.add_package(package)
logger.debug(f"installed package: {package}")
except apt.PackageNotFoundError:
logger.error(f"package not found: {package}")
raise
except apt.PackageError:
logger.error(f"package error: {package}")
raise

def _install_pip_packages(self, packages: List[str]) -> None:
"""Simple wrapper around pip install."""
"""Simple wrapper around pip install.
Raises:
SubprocessError if the packages could not be installed.
"""
try:
command = [
"pip3",
Expand All @@ -148,7 +169,7 @@ def _install_pip_packages(self, packages: List[str]) -> None:
subprocess.check_call(command)
except subprocess.SubprocessError:
logger.error("could not install pip packages")
self.unit.status = BlockedStatus("failed to install pip packages")
raise

def _new_password(self) -> str:
"""Generate a random password string.
Expand Down
67 changes: 46 additions & 21 deletions tests/unit/test_charm.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,41 @@ def test_on_install(
WaitingStatus("waiting to start PostgreSQL"),
)

@patch("charm.PostgresqlOperatorCharm._install_pip_packages")
@patch("charm.PostgresqlOperatorCharm._install_apt_packages")
@patch("charm.PostgresqlCluster.inhibit_default_cluster_creation")
def test_on_install_apt_failure(
self, _inhibit_default_cluster_creation, _install_apt_packages, _install_pip_packages
):
# Mock the result of the call.
_install_apt_packages.side_effect = apt.PackageNotFoundError
# Trigger the hook.
self.charm.on.install.emit()
# Assert that the needed calls were made.
_inhibit_default_cluster_creation.assert_called_once()
_install_apt_packages.assert_called_once()
_install_pip_packages.assert_not_called()
self.assertTrue(isinstance(self.harness.model.unit.status, BlockedStatus))

@patch("charm.PostgresqlOperatorCharm._install_pip_packages")
@patch("charm.PostgresqlOperatorCharm._install_apt_packages")
@patch("charm.PostgresqlCluster.inhibit_default_cluster_creation")
def test_on_install_pip_failure(
self, _inhibit_default_cluster_creation, _install_apt_packages, _install_pip_packages
):
# Mock the result of the call.
_install_pip_packages.side_effect = subprocess.CalledProcessError(
cmd="pip3 install patroni", returncode=1
)
# Add an empty file as Patroni resource just to check that the correct calls were made.
self.harness.add_resource("patroni", "")
self.charm.on.install.emit()
# Assert that the needed calls were made.
_inhibit_default_cluster_creation.assert_called_once()
_install_apt_packages.assert_called_once()
_install_pip_packages.assert_called_once()
self.assertTrue(isinstance(self.harness.model.unit.status, BlockedStatus))

def test_on_leader_elected(self):
# Assert that there is no password in the peer relation.
self.harness.add_relation(self._peer_relation, self.charm.app.name)
Expand Down Expand Up @@ -162,30 +197,24 @@ def test_install_apt_packages(self, _update, _add_package):
]

# Test for problem with apt update.
self.charm._install_apt_packages(mock_event, "postgresql")
_update.assert_called_once()
self.assertEqual(
self.harness.model.unit.status,
BlockedStatus("failed to update apt cache"),
)
with self.assertRaises(subprocess.CalledProcessError):
self.charm._install_apt_packages(mock_event, ["postgresql"])
_update.assert_called_once()

# Test with a not found package.
_add_package.side_effect = apt.PackageNotFoundError
self.charm._install_apt_packages(mock_event, "postgresql")
_update.assert_called()
_add_package.assert_called_once_with("postgresql")
self.assertEqual(
self.harness.model.unit.status,
BlockedStatus("failed to install packages"),
)
with self.assertRaises(apt.PackageNotFoundError):
self.charm._install_apt_packages(mock_event, ["postgresql"])
_update.assert_called()
_add_package.assert_called_with("postgresql")

# Then test a valid one.
_update.reset_mock()
_add_package.reset_mock()
_add_package.side_effect = None
self.charm._install_apt_packages(mock_event, "postgresql-12")
self.charm._install_apt_packages(mock_event, ["postgresql"])
_update.assert_called_once()
_add_package.assert_called_once_with("postgresql-12")
_add_package.assert_called_with("postgresql")

@patch("subprocess.call")
def test_install_pip_packages(self, _call):
Expand All @@ -211,12 +240,8 @@ def test_install_pip_packages(self, _call):
)

# Then, test for an error.
self.charm._install_pip_packages(packages)
# Assert the status set by the event handler.
self.assertEqual(
self.harness.model.unit.status,
BlockedStatus("failed to install pip packages"),
)
with self.assertRaises(subprocess.SubprocessError):
self.charm._install_pip_packages(packages)

def test_new_password(self):
# Test the password generation twice in order to check if we get different passwords and
Expand Down

0 comments on commit fac4ee2

Please sign in to comment.