From 9dbfac0e4375497f99791fce877d26416fce939d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Micha=C5=82=20Dulko?= <mdulko@redhat.com>
Date: Mon, 19 Feb 2018 18:14:53 +0100
Subject: [PATCH] Delete pods from registry in CNI daemon

Initial implementation of kuryr-daemon was only adding new pods to the
in-memory registry and not deleting them. While the amount of data is
tiny, it will accumulate over time and in practice be a memory leak. We
should aim to prevent that, especially taking into account most recent
efforts to base health checks on memory usage.

This commit makes kuryr-daemon react to pod deletions with removals of
pods from the in-memory registry.

Change-Id: I45babc325b7a727d6a973717f6653a40503ad991
Closes-Bug: 1750394
(cherry picked from commit 89a8bd08b5f4ba3410d015ae0f71d10cf4f3b885)
---
 kuryr_kubernetes/cni/daemon/service.py | 15 ++++++++++++++-
 kuryr_kubernetes/cni/handlers.py       |  8 +++++++-
 2 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/kuryr_kubernetes/cni/daemon/service.py b/kuryr_kubernetes/cni/daemon/service.py
index cdadcadc0..e50867905 100644
--- a/kuryr_kubernetes/cni/daemon/service.py
+++ b/kuryr_kubernetes/cni/daemon/service.py
@@ -256,7 +256,8 @@ class CNIDaemonWatcherService(cotyledon.Service):
 
     def run(self):
         self.pipeline = h_cni.CNIPipeline()
-        self.pipeline.register(h_cni.CallbackHandler(self.on_done))
+        self.pipeline.register(h_cni.CallbackHandler(self.on_done,
+                                                     self.on_deleted))
         self.watcher = k_watcher.Watcher(self.pipeline)
         self.watcher.add(
             "%(base)s/pods?fieldSelector=spec.nodeName=%(node_name)s" % {
@@ -283,6 +284,18 @@ class CNIDaemonWatcherService(cotyledon.Service):
                     pod_dict['vif'] = vif_dict
                     self.registry[pod_name] = pod_dict
 
+    def on_deleted(self, pod):
+        pod_name = pod['metadata']['name']
+        try:
+            if pod_name in self.registry:
+                # NOTE(dulek): del on dict is atomic as long as we use standard
+                #              types as keys. This is the case, so we don't
+                #              need to lock here.
+                del self.registry[pod_name]
+        except KeyError:
+            # This means someone else removed it. It's odd but safe to ignore.
+            pass
+
     def terminate(self):
         if self.watcher:
             self.watcher.stop()
diff --git a/kuryr_kubernetes/cni/handlers.py b/kuryr_kubernetes/cni/handlers.py
index fdd2ed351..960088685 100644
--- a/kuryr_kubernetes/cni/handlers.py
+++ b/kuryr_kubernetes/cni/handlers.py
@@ -92,12 +92,18 @@ class DelHandler(CNIHandlerBase):
 
 class CallbackHandler(CNIHandlerBase):
 
-    def __init__(self, on_vif):
+    def __init__(self, on_vif, on_del=None):
         super(CallbackHandler, self).__init__(None, on_vif)
+        self._del_callback = on_del
 
     def on_vif(self, pod, vif):
         self._callback(pod, vif)
 
+    def on_deleted(self, pod):
+        LOG.debug("Got pod %s deletion event.", pod['metadata']['name'])
+        if self._del_callback:
+            self._del_callback(pod)
+
 
 class CNIPipeline(k_dis.EventPipeline):