Fix traefik lb being constantly re-created on controller restart (#104)

Author: IbraAoad

lib/charms/observability_libs/v1/kubernetes_service_patch.py

@@ -145,14 +145,15 @@ def setUp(self, *unused):
 
 import logging
 from types import MethodType
-from typing import List, Literal, Optional, Union
+from typing import Any, List, Literal, Optional, Union
 
 from lightkube import ApiError, Client  # pyright: ignore
 from lightkube.core import exceptions
 from lightkube.models.core_v1 import ServicePort, ServiceSpec
 from lightkube.models.meta_v1 import ObjectMeta
 from lightkube.resources.core_v1 import Service
 from lightkube.types import PatchType
+from ops import UpgradeCharmEvent
 from ops.charm import CharmBase
 from ops.framework import BoundEvent, Object
 
@@ -166,7 +167,7 @@ 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 = 11
+LIBPATCH = 12
 
 ServiceType = Literal["ClusterIP", "LoadBalancer"]
 
@@ -225,9 +226,11 @@ def __init__(
         assert isinstance(self._patch, MethodType)
         # Ensure this patch is applied during the 'install' and 'upgrade-charm' events
         self.framework.observe(charm.on.install, self._patch)
-        self.framework.observe(charm.on.upgrade_charm, self._patch)
+        self.framework.observe(charm.on.upgrade_charm, self._on_upgrade_charm)
         self.framework.observe(charm.on.update_status, self._patch)
-        self.framework.observe(charm.on.stop, self._remove_service)
+        # Sometimes Juju doesn't clean-up a manually created LB service,
+        # so we clean it up ourselves just in case.
+        self.framework.observe(charm.on.remove, self._remove_service)
 
         # apply user defined events
         if refresh_event:
@@ -356,6 +359,36 @@ def _is_patched(self, client: Client) -> bool:
         ]  # noqa: E501
         return expected_ports == fetched_ports
 
+    def _on_upgrade_charm(self, event: UpgradeCharmEvent):
+        """Handle the upgrade charm event."""
+        # If a charm author changed the service type from LB to ClusterIP across an upgrade, we need to delete the previous LB.
+        if self.service_type == "ClusterIP":
+
+            client = Client()  # pyright: ignore
+
+            # Define a label selector to find services related to the app
+            selector: dict[str, Any] = {"app.kubernetes.io/name": self._app}
+
+            # Check if any service of type LoadBalancer exists
+            services = client.list(Service, namespace=self._namespace, labels=selector)
+            for service in services:
+                if (
+                    not service.metadata
+                    or not service.metadata.name
+                    or not service.spec
+                    or not service.spec.type
+                ):
+                    logger.warning(
+                        "Service patch: skipping resource with incomplete metadata: %s.", service
+                    )
+                    continue
+                if service.spec.type == "LoadBalancer":
+                    client.delete(Service, service.metadata.name, namespace=self._namespace)
+                    logger.info(f"LoadBalancer service {service.metadata.name} deleted.")
+
+        # Continue the upgrade flow normally
+        self._patch(event)
+
     def _remove_service(self, _):
         """Remove a Kubernetes service associated with this charm.
 
@@ -372,13 +405,13 @@ def _remove_service(self, _):
 
         try:
             client.delete(Service, self.service_name, namespace=self._namespace)
+            logger.info("The patched k8s service '%s' was deleted.", self.service_name)
         except ApiError as e:
             if e.status.code == 404:
                 # Service not found, so no action needed
-                pass
-            else:
-                # Re-raise for other statuses
-                raise
+                return
+            # Re-raise for other statuses
+            raise
 
     @property
     def _app(self) -> str:

tests/unit/test_kubernetes_service.py

@@ -374,8 +374,14 @@ def test_given_initialized_charm_when_install_event_then_event_listener_is_attac
             charm.on.install.emit()
             self.assertEqual(patch.call_count, 1)
 
-    def test_given_initialized_charm_when_upgrade_event_then_event_listener_is_attached(self):
+    @patch(f"{CL_PATH}._namespace", "test")
+    @patch(f"{MOD_PATH}.Client")
+    def test_given_initialized_charm_when_upgrade_event_then_event_listener_is_attached(
+        self, client
+    ):
         charm = self.harness.charm
+        client.return_value = client
+        client.list.return_value = []
         with mock.patch(f"{CL_PATH}._patch") as patch:
             charm.on.upgrade_charm.emit()
             self.assertEqual(patch.call_count, 1)