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

raise error on duplicate ports passed to patch #73

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
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
18 changes: 16 additions & 2 deletions lib/charms/observability_libs/v1/kubernetes_service_patch.py
Original file line number Diff line number Diff line change
Expand Up @@ -146,11 +146,15 @@ def setUp(self, *unused):

# Increment this PATCH version before using `charmcraft publish-lib` or reset
# to 0 if you are raising the major API version
LIBPATCH = 9
LIBPATCH = 10

ServiceType = Literal["ClusterIP", "LoadBalancer"]


class DuplicatePortsError(Exception):
"""Raised if the user attempts to pass to the patch duplicate ports."""


class KubernetesServicePatch(Object):
"""A utility for patching the Kubernetes service set up by Juju."""

Expand All @@ -165,6 +169,7 @@ def __init__(
additional_annotations: Optional[dict] = None,
*,
refresh_event: Optional[Union[BoundEvent, List[BoundEvent]]] = None,
key: str = "kubernetes-service-patch"
):
"""Constructor for KubernetesServicePatch.

Expand All @@ -184,7 +189,7 @@ def __init__(
will be observed to re-apply the patch (e.g. on port change).
The `install` and `upgrade-charm` events would be observed regardless.
"""
super().__init__(charm, "kubernetes-service-patch")
super().__init__(charm, key)
self.charm = charm
self.service_name = service_name if service_name else self._app
self.service = self._service_object(
Expand Down Expand Up @@ -237,6 +242,15 @@ def _service_object(
Returns:
Service: A valid representation of a Kubernetes Service with the correct ports.
"""
# check for duplicate ports
ports_with_dupes = list((p.port, p.protocol) for p in ports)
ports_dedup = set((p.port, p.protocol) for p in ports)
for port in ports_dedup:
ports_with_dupes.remove(port)

if ports_with_dupes:
raise DuplicatePortsError(f"duplicate ports: {ports_with_dupes}")

if not service_name:
service_name = self._app
labels = {"app.kubernetes.io/name": self._app}
Expand Down
15 changes: 14 additions & 1 deletion tests/unit/test_kubernetes_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
from unittest import mock
from unittest.mock import Mock, mock_open, patch

from charms.observability_libs.v1.kubernetes_service_patch import KubernetesServicePatch
from charms.observability_libs.v1.kubernetes_service_patch import KubernetesServicePatch, DuplicatePortsError
from lightkube import ApiError
from lightkube.models.core_v1 import ServicePort, ServiceSpec
from lightkube.models.meta_v1 import ObjectMeta
Expand Down Expand Up @@ -361,6 +361,18 @@ def test_protocols(self):

self.assertEqual(service_patch.service, expected_service)

def test_patch_raises_on_duplicate_ports(self):
"""Check that patch raises if duplicate ports are requested."""
with self.assertRaises(DuplicatePortsError):
KubernetesServicePatch(
self.harness.charm,
ports=[
ServicePort(name="svc1", port=1234, protocol="TCP"),
ServicePort(name="svc2", port=1234, protocol="TCP"),
],
key="foo"
)

def test_custom_event_is_fired(self):
"""Check that events provided via refresh_event are called."""
charm = self.harness.charm
Expand Down Expand Up @@ -500,3 +512,4 @@ def test_is_patched_k8s_service_api_error_not_404(self, client):
with self.assertRaises(_FakeApiError):
charm.custom_service_name_service_patch.is_patched()
self.assertIn("Kubernetes service get failed: broken", ";".join(logs.output))

Loading