From 528a179547a4b212c9f1a1659aabe3f79fcd17d9 Mon Sep 17 00:00:00 2001
From: Dario Faccin <35623244+dariofaccin@users.noreply.github.com>
Date: Tue, 20 Feb 2024 14:17:28 +0100
Subject: [PATCH] fix: recreate pod when hardware checksum config is changed
 (#96)

---
 src/charm.py             | 37 ++++++++++++++++++++++++++++++++++---
 tests/unit/test_charm.py | 17 +++++++++++++++++
 2 files changed, 51 insertions(+), 3 deletions(-)

diff --git a/src/charm.py b/src/charm.py
index e595ed3b..bcc46b1f 100755
--- a/src/charm.py
+++ b/src/charm.py
@@ -29,13 +29,13 @@
 from lightkube.core.client import Client
 from lightkube.models.core_v1 import ServicePort, ServiceSpec
 from lightkube.models.meta_v1 import ObjectMeta
-from lightkube.resources.core_v1 import Node, Service
+from lightkube.resources.core_v1 import Node, Pod, Service
 from ops import RemoveEvent
 from ops.charm import CharmBase, CharmEvents
 from ops.framework import EventBase, EventSource
 from ops.main import main
 from ops.model import ActiveStatus, BlockedStatus, Container, ModelError, WaitingStatus
-from ops.pebble import ExecError, Layer
+from ops.pebble import ExecError, Layer, PathError
 
 from charm_config import CharmConfig, CharmConfigInvalidError, CNIType, UpfMode
 from dpdk import DPDK
@@ -200,11 +200,25 @@ def _delete_external_upf_service(self) -> None:
         except HTTPStatusError as status:
             logger.info(f"Could not delete {self.app.name}-external due to: {status}")
 
+    def delete_pod(self):
+        """Delete the pod."""
+        client = Client()
+        client.delete(Pod, name=self._pod_name, namespace=self._namespace)
+
     @property
     def _namespace(self) -> str:
         """Returns the k8s namespace."""
         return self.model.name
 
+    @property
+    def _pod_name(self) -> str:
+        """Name of the unit's pod.
+
+        Returns:
+            str: A string containing the name of the current unit's pod.
+        """
+        return "-".join(self.model.unit.name.rsplit("/", 1))
+
     def _on_install(self, event: EventBase) -> None:
         """Handler for Juju install event.
 
@@ -462,6 +476,17 @@ def _bessd_config_file_content_matches(self, content: str) -> bool:
             return False
         return True
 
+    def _hwcksum_config_matches_pod_config(self) -> bool:
+        try:
+            existing_content = json.loads(
+                self._bessd_container.pull(
+                    path=f"{BESSD_CONTAINER_CONFIG_PATH}/{CONFIG_FILE_NAME}"
+                ).read()
+            )
+        except PathError:
+            existing_content = {}
+        return existing_content.get("hwcksum") != self._charm_config.enable_hw_checksum
+
     def _on_config_changed(self, event: EventBase):
         """Handler for config changed events."""
         # workaround for https://github.com/canonical/operator/issues/736
@@ -529,6 +554,7 @@ def _configure_bessd_workload(self) -> None:
         Writes configuration file, creates routes, creates iptable rule and pebble layer.
         """
         restart = False
+        recreate_pod = False
         core_ip_address = self._get_network_ip_config(CORE_INTERFACE_NAME)
         content = render_bessd_config_file(
             upf_hostname=self._upf_hostname,
@@ -543,6 +569,8 @@ def _configure_bessd_workload(self) -> None:
         if not self._bessd_config_file_is_written() or not self._bessd_config_file_content_matches(
             content=content
         ):
+            if self._hwcksum_config_matches_pod_config():
+                recreate_pod = True
             self._write_bessd_config_file(content=content)
             restart = True
         self._create_default_route()
@@ -554,7 +582,10 @@ def _configure_bessd_workload(self) -> None:
         if plan.services != layer.services:
             self._bessd_container.add_layer("bessd", self._bessd_pebble_layer, combine=True)
             restart = True
-        if restart:
+        if recreate_pod:
+            logger.warning("Recreating POD after changing hardware checksum offloading config")
+            self.delete_pod()
+        elif restart:
             self._bessd_container.restart(self._routectl_service_name)
             logger.info("Service `routectl` restarted")
             self._bessd_container.restart(self._bessd_service_name)
diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py
index 7cfb2acb..1fd9341f 100644
--- a/tests/unit/test_charm.py
+++ b/tests/unit/test_charm.py
@@ -1983,3 +1983,20 @@ def test_given_hardware_checksum_is_enabled_when_bessd_pebble_ready_then_config_
         config = json.loads((self.root / "etc/bess/conf/upf.json").read_text())
         self.assertIn("hwcksum", config)
         self.assertFalse(config["hwcksum"])
+
+    @patch("charm.UPFOperatorCharm.delete_pod")
+    @patch(f"{MULTUS_LIBRARY_PATH}.KubernetesMultusCharmLib.is_ready")
+    @patch(f"{HUGEPAGES_LIBRARY_PATH}.KubernetesHugePagesPatchCharmLib.is_patched")
+    def test_given_hardware_checksum_is_enabled_when_value_changes_then_delete_pod_is_called_once(  # noqa: E501
+        self,
+        patch_hugepages_is_patched,
+        patch_multus_is_ready,
+        patch_delete_pod,
+    ):
+        self.harness.handle_exec("bessd", [], result=0)
+        patch_hugepages_is_patched.return_value = True
+        patch_multus_is_ready.return_value = True
+        self.harness.set_can_connect(container="bessd", val=True)
+        self.harness.set_can_connect(container="pfcp-agent", val=True)
+        self.harness.update_config(key_values={"enable-hw-checksum": False})
+        patch_delete_pod.assert_called_once()