DPE 71 Patroni resource install (canonical#4)

* Add Patroni resource

* Correctly handle calls from on install hook

charmcraft.yaml

@@ -9,3 +9,7 @@ bases:
     run-on:
       - name: "ubuntu"
         channel: "20.04"
+parts:
+  charm:
+    # pip==21.3.1 needed to install Patroni with raft support on bionic.
+    charm-python-packages: [pip==21.3.1]

metadata.yaml

@@ -16,6 +16,12 @@ peers:
   postgresql-replicas:
     interface: postgresql-replicas
 
+resources:
+  patroni:
+    type: file
+    filename: patroni.tar.gz
+    description: Patroni python package.
+
 storage:
   pgdata:
     type: filesystem

src/charm.py

@@ -17,6 +17,7 @@
     ActiveStatus,
     BlockedStatus,
     MaintenanceStatus,
+    ModelError,
     Relation,
     WaitingStatus,
 )
@@ -53,8 +54,27 @@ def _on_install(self, event) -> None:
         # Prevent the default cluster creation.
         self._cluster.inhibit_default_cluster_creation()
 
-        # Install the PostgreSQL packages.
-        self._install_apt_packages(event, ["postgresql"])
+        # Install the PostgreSQL and Patroni requirements packages.
+        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")
+        except ModelError as e:
+            logger.error(f"missing patroni resource {str(e)}")
+            self.unit.status = BlockedStatus("Missing 'patroni' resource")
+            return
+
+        # Build Patroni package path with raft dependency and install it.
+        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")
 
@@ -108,21 +128,48 @@ 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
 
+        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.
+
+        Raises:
+            SubprocessError if the packages could not be installed.
+        """
         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")
+            command = [
+                "pip3",
+                "install",
+                " ".join(packages),
+            ]
+            logger.debug(f"installing python packages: {', '.join(packages)}")
+            subprocess.check_call(command)
+        except subprocess.SubprocessError:
+            logger.error("could not install pip packages")
+            raise
 
     def _new_password(self) -> str:
         """Generate a random password string.

tests/integration/test_charm.py

@@ -38,10 +38,13 @@ async def test_deploy(ops_test: OpsTest, charm: str, series: str):
     # Set a composite application name in order to test in more than one series at the same time.
     application_name = f"{APP_NAME}-{series}"
 
-    # Deploy two units in order to test later the sharing of password through peer relation data.
+    # Deploy the charm with Patroni resource.
+    resources = {"patroni": "patroni.tar.gz"}
     await ops_test.model.deploy(
-        charm, application_name=application_name, num_units=2, series=series
+        charm, resources=resources, application_name=application_name, num_units=2, series=series
     )
+    # Attach the resource to the controller.
+    await ops_test.juju("attach-resource", application_name, "patroni=patroni.tar.gz")
 
     # Issuing dummy update_status just to trigger an event.
     await ops_test.model.set_config({"update-status-hook-interval": "10s"})

tests/unit/test_charm.py

@@ -2,6 +2,7 @@
 # See LICENSE file for licensing details.
 
 import re
+import subprocess
 import unittest
 from unittest.mock import Mock, patch
 
@@ -29,23 +30,70 @@ def setUp(self):
         self.harness.begin()
         self.charm = self.harness.charm
 
+    @patch("charm.PostgresqlOperatorCharm._install_pip_packages")
     @patch("charm.PostgresqlOperatorCharm._install_apt_packages")
     @patch("charm.PostgresqlCluster.inhibit_default_cluster_creation")
     def test_on_install(
-        self,
-        _inhibit_default_cluster_creation,
-        _install_apt_packages,
+        self, _inhibit_default_cluster_creation, _install_apt_packages, _install_pip_packages
     ):
+        # Test without adding Patroni resource.
         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()
+        # Assert that the needed calls were made.
+        _install_pip_packages.assert_not_called()
+        # Assert the status set by the event handler.
+        self.assertEqual(
+            self.harness.model.unit.status,
+            BlockedStatus("Missing 'patroni' resource"),
+        )
+
+        # 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()
+        _install_pip_packages.assert_called_once()
         # Assert the status set by the event handler.
         self.assertEqual(
             self.harness.model.unit.status,
             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)
@@ -141,23 +189,59 @@ def test_get_postgres_password(self):
     def test_install_apt_packages(self, _update, _add_package):
         mock_event = Mock()
 
+        # Mock the returns of apt-get update calls.
+        _update.side_effect = [
+            subprocess.CalledProcessError(returncode=1, cmd="apt-get update"),
+            None,
+            None,
+        ]
+
+        # Test for problem with apt update.
+        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_once()
-        _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):
+        # Fake pip packages.
+        packages = ["package1", "package2"]
+
+        _call.side_effect = [None, subprocess.SubprocessError]
+
+        # Then test for a succesful install.
+        self.charm._install_pip_packages(packages)
+        # Check that check_call was invoked with the correct arguments.
+        _call.assert_called_once_with(
+            [
+                "pip3",
+                "install",
+                "package1 package2",
+            ]
+        )
+        # Assert the status set by the event handler.
+        self.assertNotEqual(
+            self.harness.model.unit.status,
+            BlockedStatus("failed to install pip packages"),
+        )
+
+        # Then, test for an error.
+        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

tox.ini

@@ -73,4 +73,10 @@ deps =
     psycopg2-binary
     -r{toxinidir}/requirements.txt
 commands =
+    # Download patroni resource to use in the charm deployment.
+    sh -c 'stat patroni.tar.gz > /dev/null 2>&1 || curl "https://github.com/zalando/patroni/archive/refs/tags/v2.1.3.tar.gz" -L -s > patroni.tar.gz'
     pytest -v --tb native --ignore={[vars]tst_path}unit --log-cli-level=INFO -s {posargs}
+    # Remove the downloaded resource.
+    sh -c 'rm -f patroni.tar.gz'
+whitelist_externals =
+    sh