Skip to content

Commit

Permalink
PR feedback + unit tests WIP
Browse files Browse the repository at this point in the history
  • Loading branch information
MiaAltieri committed Sep 2, 2024
1 parent 301bfbe commit 710135e
Show file tree
Hide file tree
Showing 4 changed files with 99 additions and 7 deletions.
12 changes: 9 additions & 3 deletions src/node_port.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
from lightkube.core.exceptions import ApiError
from lightkube.resources.core_v1 import Pod, Service
from lightkube.models.core_v1 import ServicePort, ServiceSpec

from ops.model import BlockedStatus

logger = logging.getLogger(__name__)

Expand Down Expand Up @@ -71,6 +71,12 @@ def get_unit_service(self) -> Service | None:
# END: getters

# BEGIN: helpers
def on_deployed_without_trust(self) -> None:
"""Blocks the application and returns a specific error message for deployments made without --trust."""
logger.error("Could not apply service, application needs `juju trust`")
self.charm.unit.status = BlockedStatus(
f"Insufficient permissions, try: `juju trust {self.app.name} --scope=cluster`"
)

def build_node_port_services(self, port: str) -> Service:
"""Builds a ClusterIP service for initial client connection."""
Expand Down Expand Up @@ -117,7 +123,7 @@ def apply_service(self, service: Service) -> None:
self.client.apply(service)
except ApiError as e:
if e.status.code == 403:
logger.error("Could not apply service, application needs `juju trust`")
self.on_deployed_without_trust()
return
if e.status.code == 422 and "port is already allocated" in e.status.message:
logger.error(e.status.message)
Expand All @@ -143,7 +149,7 @@ def delete_unit_service(self) -> None:
self.client.delete(Service, service.metadata.name)
except ApiError as e:
if e.status.code == 403:
logger.error("Could not delete service, application needs `juju trust`")
self.on_deployed_without_trust()
return
else:
raise
Expand Down
4 changes: 2 additions & 2 deletions tests/integration/client_relations/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -114,12 +114,12 @@ async def assert_all_unit_node_ports_available(ops_test: OpsTest):
ops_test, node_port_name=f"{MONGOS_APP_NAME}-{unit_id}-external"
)

assert await is_external_mongos_client_reachble(
assert await is_external_mongos_client_reachable(
ops_test, exposed_node_port
), "client is not reachable"


async def is_external_mongos_client_reachble(
async def is_external_mongos_client_reachable(
ops_test: OpsTest, exposed_node_port: str
) -> bool:
"""Returns True if the mongos client is reachable on the provided node port via the k8s ip."""
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
assert_all_unit_node_ports_available,
assert_all_unit_node_ports_are_unavailable,
get_port_from_node_port,
is_external_mongos_client_reachble,
is_external_mongos_client_reachable,
)


Expand Down Expand Up @@ -104,7 +104,7 @@ async def test_mongos_disable_external_connections(ops_test: OpsTest) -> None:
# verify each unit has a node port available
await assert_all_unit_node_ports_are_unavailable(ops_test)

assert not await is_external_mongos_client_reachble(ops_test, exposed_node_port)
assert not await is_external_mongos_client_reachable(ops_test, exposed_node_port)


@pytest.mark.group(1)
Expand Down
86 changes: 86 additions & 0 deletions tests/unit/test_nodeport.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
# Copyright 2024 Canonical Ltd.
# See LICENSE file for licensing details.
import logging
import unittest
from unittest import mock
from unittest.mock import patch, PropertyMock
from ops.model import BlockedStatus
from ops.testing import Harness
from node_port import NodePortManager, ApiError
from charms.data_platform_libs.v0.data_interfaces import DatabaseRequiresEvents
from charm import MongosCharm


logger = logging.getLogger(__name__)


STATUS_JUJU_TRUST = (
"Insufficient permissions, try: `juju trust mongos-k8s --scope=cluster`"
)
CLUSTER_ALIAS = "cluster"


class TestNodePort(unittest.TestCase):
def setUp(self, *unused):
"""Set up the charm for each unit test."""
try:
# runs before each test to delete the custom events created for the aliases. This is
# needed because the events are created again in the next test, which causes an error
# related to duplicated events.
delattr(DatabaseRequiresEvents, f"{CLUSTER_ALIAS}_database_created")
delattr(DatabaseRequiresEvents, f"{CLUSTER_ALIAS}_endpoints_changed")
delattr(
DatabaseRequiresEvents, f"{CLUSTER_ALIAS}_read_only_endpoints_changed"
)
except AttributeError:
# Ignore the events not existing before the first test.
pass

self.harness = Harness(MongosCharm)
self.addCleanup(self.harness.cleanup)
self.harness.begin()

@patch("charm.NodePortManager.get_service")
def test_delete_unit_service_has_no_metadata(self, get_service):
"""Verify that when no metadata is present, the charm raises an error."""
service = mock.Mock()
service.metadata = None
get_service.return_value = service

with self.assertRaises(Exception):
self.harness.charm.node_port_manager.delete_unit_service()

@patch("charm.NodePortManager.get_service")
@patch("charm.NodePortManager.client", new_callable=PropertyMock)
def test_delete_unit_service_raises_ApiError(self, client, get_service):
"""Verify that when charm needs juju trust a status is logged."""
metadata_mock = mock.Mock()
metadata_mock.name = "serice-name"
service = mock.Mock()
service.metadata = metadata_mock
get_service.return_value = service

client.return_value.delete.side_effect = ApiError

with self.assertRaises(ApiError):
self.harness.charm.node_port_manager.delete_unit_service()

@patch("src.node_port.NodePortManager.get_service")
@patch("charm.NodePortManager.client", new_callable=PropertyMock)
def test_delete_unit_service_needs_juju_trust(self, get_service, client):
"""Verify that when charm needs juju trust a status is logged."""

metadata_mock = mock.Mock()
metadata_mock.name = "serice-name"
service = mock.Mock()
service.metadata = metadata_mock
get_service.return_value = service

api_error = mock.Mock()
api_error.status.code = 403

NodePortManager.delete_unit_service()

self.assertTrue(
isinstance(self.harness.charm.unit.status, BlockedStatus(STATUS_JUJU_TRUST))
)

0 comments on commit 710135e

Please sign in to comment.