From 7367713d1637cd0742e5a9dcd02a5a83c2ff6af0 Mon Sep 17 00:00:00 2001 From: Orfeas Kourkakis Date: Wed, 19 Jun 2024 17:11:01 +0300 Subject: [PATCH] fix: Remove k8s resources on charm removal (#37) Add `remove_event` handler that removes all k8s resources created by the charm. Closes #34 --- src/charm.py | 43 +++++++++++++++++++++++++++++---- tests/integration/test_charm.py | 14 +++++++++++ tests/unit/test_operator.py | 20 +++++++++++++++ 3 files changed, 72 insertions(+), 5 deletions(-) diff --git a/src/charm.py b/src/charm.py index 80ee21d..cca1f8c 100755 --- a/src/charm.py +++ b/src/charm.py @@ -10,13 +10,20 @@ import yaml from charmed_kubeflow_chisme.exceptions import ErrorWithStatus -from charmed_kubeflow_chisme.kubernetes import KubernetesResourceHandler +from charmed_kubeflow_chisme.kubernetes import ( + KubernetesResourceHandler, + create_charm_default_labels, +) from lightkube import ApiError from lightkube.generic_resource import load_in_cluster_generic_resources +from lightkube.resources.admissionregistration_v1 import MutatingWebhookConfiguration +from lightkube.resources.apps_v1 import Deployment +from lightkube.resources.core_v1 import ConfigMap, Secret, Service, ServiceAccount +from lightkube.resources.rbac_authorization_v1 import Role, RoleBinding from ops.charm import CharmBase from ops.framework import StoredState from ops.main import main -from ops.model import ActiveStatus, BlockedStatus, MaintenanceStatus, WaitingStatus +from ops.model import ActiveStatus, BlockedStatus, ErrorStatus, MaintenanceStatus, WaitingStatus from certs import gen_certs @@ -49,6 +56,7 @@ def __init__(self, *args): self.framework.observe(self.on.leader_elected, self.main) self.framework.observe(self.on.remove, self.main) self.framework.observe(self.on.upgrade_charm, self.main) + self.framework.observe(self.on.remove, self._on_remove) def main(self, _): """Entrypoint for most charm events.""" @@ -110,6 +118,21 @@ def k8s_resource_handler(self): template_files=K8S_RESOURCE_FILES, context=self._context, logger=self.logger, + labels=create_charm_default_labels( + self.app.name, + self.model.name, + scope="auths-deploy-configmaps-sa-secrets-svc-webhooks", + ), + resource_types={ + MutatingWebhookConfiguration, + Deployment, + Role, + RoleBinding, + Service, + ServiceAccount, + Secret, + ConfigMap, + }, ) load_in_cluster_generic_resources(self._k8s_resource_handler.lightkube_client) return self._k8s_resource_handler @@ -151,9 +174,19 @@ def _cert_key(self): def _cert_ca(self): return self._stored.ca - # todo: add a remove handler? - # def _on_remove(self, event): - # raise NotImplementedError + def _on_remove(self, event): + """Remove K8S resources.""" + self.logger.info("Removing k8s resources") + try: + self.unit.status = MaintenanceStatus("Removing k8s resources") + self.k8s_resource_handler.delete() + except ApiError: + self.logger.error("K8s resource removal failed with ApiError:") + self.logger.error(str(ApiError)) + self.logger.error(ApiError.status) + self.model.unit.status = ErrorStatus("K8S resources removal failed") + + self.model.unit.status = MaintenanceStatus("K8s resources removed") if __name__ == "__main__": diff --git a/tests/integration/test_charm.py b/tests/integration/test_charm.py index 19ab84a..dba20cf 100644 --- a/tests/integration/test_charm.py +++ b/tests/integration/test_charm.py @@ -152,3 +152,17 @@ async def test_webhook_workload(ops_test: OpsTest, temp_pod_deleter): test_pod, name=test_pod.metadata.name, namespace=ops_test.model.name ) assert test_pod_created.spec.affinity is not None + + +async def test_charm_removal(ops_test: OpsTest): + """Test that the charm can be removed without errors and leaves no leftovers.""" + await ops_test.model.remove_application(APP_NAME, block_until_done=True) + lightkube_client = Client() + pods = lightkube_client.list( + Pod, namespace=ops_test.model.name, labels={"app": "namespace-node-affinity-pod-webhook"} + ) + pods_list = list(pods) + assert len(pods_list) == 0, ( + "Workload pod with label 'app': 'namespace-node-affinity-pod-webhook'" + "still present on the cluster." + ) diff --git a/tests/unit/test_operator.py b/tests/unit/test_operator.py index b1ab9cc..3fc275b 100644 --- a/tests/unit/test_operator.py +++ b/tests/unit/test_operator.py @@ -7,6 +7,11 @@ import pytest import yaml +from charmed_kubeflow_chisme.kubernetes import create_charm_default_labels +from lightkube.resources.admissionregistration_v1 import MutatingWebhookConfiguration +from lightkube.resources.apps_v1 import Deployment +from lightkube.resources.core_v1 import ConfigMap, Secret, Service, ServiceAccount +from lightkube.resources.rbac_authorization_v1 import Role, RoleBinding from ops.model import WaitingStatus from ops.testing import Harness @@ -101,6 +106,21 @@ def test_k8s_resource_handler(self, harness: Harness, mocker): template_files=k8s_resource_files, context=context, logger=logger, + labels=create_charm_default_labels( + harness.model.app.name, + harness.model.name, + scope="auths-deploy-configmaps-sa-secrets-svc-webhooks", + ), + resource_types={ + MutatingWebhookConfiguration, + Deployment, + Role, + RoleBinding, + Service, + ServiceAccount, + Secret, + ConfigMap, + }, ) mocked_load_in_cluster_generic_resources.assert_called_once_with("lightkube_client")