Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DPE 71 Patroni resource install #4

Merged
merged 2 commits into from
Apr 6, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions charmcraft.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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]
6 changes: 6 additions & 0 deletions metadata.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
67 changes: 57 additions & 10 deletions src/charm.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
ActiveStatus,
BlockedStatus,
MaintenanceStatus,
ModelError,
Relation,
WaitingStatus,
)
Expand Down Expand Up @@ -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")

Expand Down Expand Up @@ -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.
Expand Down
7 changes: 5 additions & 2 deletions tests/integration/test_charm.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"})
Expand Down
108 changes: 96 additions & 12 deletions tests/unit/test_charm.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
# See LICENSE file for licensing details.

import re
import subprocess
import unittest
from unittest.mock import Mock, patch

Expand Down Expand Up @@ -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(
marceloneppel marked this conversation as resolved.
Show resolved Hide resolved
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))

marceloneppel marked this conversation as resolved.
Show resolved Hide resolved
@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))

marceloneppel marked this conversation as resolved.
Show resolved Hide resolved
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 @@ -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
Expand Down
6 changes: 6 additions & 0 deletions tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this be some sort of fixture, or downloaded in the pytest code using urllib then cleaned up after the test?

The benefit here, is that once you have an uploaded version on chamrhub, you can fetch the resource straight from Charmhub using the API and get a bit more certainty in the test? (Perhaps a future PR?)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it can be, Jon. Nice suggestion! I'll create a new PR to change this that and upload the resource to Charmhub to have this improved flow of testing with the resource. I created an issue to remind about it.

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