Merge ~davigar15/charm-k8s-ingress:master into charm-k8s-ingress:master

[David]

Fix bug #1939698: remove k8s resources on relation-departed

This change fixes the deletion of the k8s resources when the relation is departed. It will delete both the service and the ingress resources that the charm creates.

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

This merge proposal is being monitored by mergebot. Change the status to Approved to merge.

[Tom Haddon (mthaddon)]

Some comments inline

review: Needs Fixing

[Tom Haddon (mthaddon)]

One very minor lint fix needed.

[Tom Haddon (mthaddon)]

LGTM, thx

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

Change successfully merged at revision 3322e29004b466b33e8ed912cb62f321ea8f79e1

diff --git a/lib/charms/nginx_ingress_integrator/v0/ingress.py b/lib/charms/nginx_ingress_integrator/v0/ingress.py
index d10839c..1871e20 100644
--- a/lib/charms/nginx_ingress_integrator/v0/ingress.py
+++ b/lib/charms/nginx_ingress_integrator/v0/ingress.py
@@ -94,10 +94,15 @@ class IngressAvailableEvent(EventBase):
     pass
 
 
+class IngressBrokenEvent(EventBase):
+    pass
+
+
 class IngressCharmEvents(CharmEvents):
     """Custom charm events."""
 
     ingress_available = EventSource(IngressAvailableEvent)
+    ingress_broken = EventSource(IngressBrokenEvent)
 
 
 class IngressRequires(Object):
@@ -173,6 +178,7 @@ class IngressProvides(Object):
         # Observe the relation-changed hook event and bind
         # self.on_relation_changed() to handle the event.
         self.framework.observe(charm.on["ingress"].relation_changed, self._on_relation_changed)
+        self.framework.observe(charm.on["ingress"].relation_broken, self._on_relation_broken)
         self.charm = charm
 
     def _on_relation_changed(self, event):
@@ -209,3 +215,11 @@ class IngressProvides(Object):
         # Create an event that our charm can use to decide it's okay to
         # configure the ingress.
         self.charm.on.ingress_available.emit()
+
+    def _on_relation_broken(self, _):
+        """Handle a relation-broken event in the ingress relation."""
+        if not self.model.unit.is_leader():
+            return
+
+        # Create an event that our charm can use to remove the ingress resource.
+        self.charm.on.ingress_broken.emit()
diff --git a/src/charm.py b/src/charm.py
index 421df63..38a5255 100755
--- a/src/charm.py
+++ b/src/charm.py
@@ -45,6 +45,7 @@ class NginxIngressCharm(CharmBase):
         self.ingress = IngressProvides(self)
         # When the 'ingress' is ready to configure, do so.
         self.framework.observe(self.on.ingress_available, self._on_config_changed)
+        self.framework.observe(self.on.ingress_broken, self._on_ingress_broken)
 
     def _describe_ingresses_action(self, event):
         """Handle the 'describe-ingresses' action."""
@@ -315,6 +316,22 @@ class NginxIngressCharm(CharmBase):
                 self._service_name,
             )
 
+    def _remove_service(self):
+        """Remove the created service in kubernetes."""
+        self.k8s_auth()
+        api = _core_v1_api()
+        services = api.list_namespaced_service(namespace=self._namespace)
+        if self._k8s_service_name in [x.metadata.name for x in services.items]:
+            api.delete_namespaced_service(
+                name=self._k8s_service_name,
+                namespace=self._namespace,
+            )
+            logger.info(
+                "Service deleted in namespace %s with name %s",
+                self._namespace,
+                self._service_name,
+            )
+
     def _look_up_and_set_ingress_class(self, api, body):
         """Set the configured ingress class, otherwise the cluster's default ingress class."""
         ingress_class = self.config['ingress-class']
@@ -373,6 +390,19 @@ class NginxIngressCharm(CharmBase):
                 self._ingress_name,
             )
 
+    def _remove_ingress(self):
+        """Remove ingress resource."""
+        self.k8s_auth()
+        api = _networking_v1_api()
+        ingresses = api.list_namespaced_ingress(namespace=self._namespace)
+        if self._ingress_name in [x.metadata.name for x in ingresses.items]:
+            api.delete_namespaced_ingress(self._ingress_name, self._namespace)
+            logger.info(
+                "Ingress deleted in namespace %s with name %s",
+                self._namespace,
+                self._ingress_name,
+            )
+
     def _on_config_changed(self, _):
         """Handle the config changed event."""
         msg = ""
@@ -403,6 +433,28 @@ class NginxIngressCharm(CharmBase):
                     raise
         self.unit.status = ActiveStatus(msg)
 
+    def _on_ingress_broken(self, _):
+        """Handle the ingress broken event."""
+        if self.unit.is_leader() and self._ingress_name:
+            try:
+                self._remove_ingress()
+                self._remove_service()
+            except kubernetes.client.exceptions.ApiException as e:
+                if e.status == 403:
+                    logger.error(
+                        "Insufficient permissions to delete the k8s ingress resource, "
+                        "will request `juju trust` to be run"
+                    )
+                    self.unit.status = BlockedStatus(
+                        "Insufficient permissions, try: `juju trust {} --scope=cluster`".format(
+                            self.app.name
+                        )
+                    )
+                    return
+                else:
+                    raise
+        self.unit.status = ActiveStatus()
+
 
 if __name__ == "__main__":  # pragma: no cover
     main(NginxIngressCharm)
diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py
index c3e52c2..936eddc 100644
--- a/tests/unit/test_charm.py
+++ b/tests/unit/test_charm.py
@@ -385,6 +385,18 @@ class TestCharm(unittest.TestCase):
         self.assertEqual(self.harness.charm._service_name, "gunicorn")
         self.assertEqual(self.harness.charm._service_port, 80)
 
+    @patch('charm.NginxIngressCharm._remove_ingress')
+    @patch('charm.NginxIngressCharm._remove_service')
+    def test_on_ingress_relation_broken(self, _remove_service, _remove_ingress):
+        """Test relation-broken."""
+        # Call the test test_on_ingress_relation_changed first
+        # to make sure the relation is created and therefore can be removed.
+        self.test_on_ingress_relation_changed()
+        relation = self.harness.charm.model.get_relation("ingress")
+        self.harness.charm.on.ingress_relation_broken.emit(relation)
+        self.assertEqual(_remove_ingress.call_count, 1)
+        self.assertEqual(_remove_service.call_count, 1)
+
     def test_get_k8s_ingress(self):
         """Test getting our definition of a k8s ingress."""
         self.harness.disable_hooks()