From 6b4519193371a9a3e31ead09c3dcc3660baf051a Mon Sep 17 00:00:00 2001
From: Hunt Xu <mhuntxu@gmail.com>
Date: Thu, 12 Jul 2018 18:16:48 +0800
Subject: [PATCH] periodics: fix service of function version 0 not expiring
 issue

If a function version is created and then we create an execution using
version 0 of that function, then the service of version 0 of that
function will never expire. This commit fixes the problem.

Change-Id: I67ad2611b121b36368c15f9bf5d2268d430df3a6
Story: 2002956
Task: 22958
---
 qinling/services/periodics.py                 |  3 +-
 qinling/tests/unit/services/test_periodics.py | 55 ++++++++++++++-----
 2 files changed, 43 insertions(+), 15 deletions(-)

diff --git a/qinling/services/periodics.py b/qinling/services/periodics.py
index 4ebd35a9..a605ad5e 100644
--- a/qinling/services/periodics.py
+++ b/qinling/services/periodics.py
@@ -51,8 +51,7 @@ def handle_function_service_expiration(ctx, engine):
     results = db_api.get_functions(
         sort_keys=['updated_at'],
         insecure=True,
-        updated_at={'lte': expiry_time},
-        latest_version=0
+        updated_at={'lte': expiry_time}
     )
 
     for func_db in results:
diff --git a/qinling/tests/unit/services/test_periodics.py b/qinling/tests/unit/services/test_periodics.py
index 7b2bf0ba..c2a54ebe 100644
--- a/qinling/tests/unit/services/test_periodics.py
+++ b/qinling/tests/unit/services/test_periodics.py
@@ -48,11 +48,9 @@ class TestPeriodics(base.DbTestCase):
 
         periodics.handle_function_service_expiration(self.ctx, mock_engine)
 
-        self.assertEqual(1, mock_engine.delete_function.call_count)
-        args, _ = mock_engine.delete_function.call_args
-        self.assertIn(function_id, args)
-        self.assertIn(0, args)
-
+        mock_engine.delete_function.assert_called_once_with(
+            self.ctx, function_id, 0
+        )
         mock_etcd_delete.assert_called_once_with(function_id, 0)
 
     @mock.patch('qinling.utils.etcd_util.delete_function')
@@ -61,23 +59,54 @@ class TestPeriodics(base.DbTestCase):
                                                             mock_etcd_delete):
         db_func = self.create_function()
         function_id = db_func.id
-        db_api.increase_function_version(function_id, 0,
-                                         description="new version")
+        self.create_function_version(0, function_id, description="new_version")
         db_api.update_function_version(function_id, 1, count=1)
         time.sleep(1.5)
 
+        self.override_config('function_service_expiration', 1, 'engine')
+
+        # NOTE(huntxu): although we didn't create any execution using version 0
+        # of the function, it is updated as a new version is created. So the
+        # call to get_service_url with version 0 should return None as there is
+        # not any worker for function version 0.
+        def mock_srv_url_side_effect(function_id, function_version):
+            return 'http://localhost:37718' if function_version != 0 else None
+
+        mock_srv_url.side_effect = mock_srv_url_side_effect
+        mock_engine = mock.Mock()
+
+        periodics.handle_function_service_expiration(self.ctx, mock_engine)
+
+        mock_engine.delete_function.assert_called_once_with(
+            self.ctx, function_id, 1
+        )
+        mock_etcd_delete.assert_called_once_with(function_id, 1)
+
+    @mock.patch('qinling.utils.etcd_util.delete_function')
+    @mock.patch('qinling.utils.etcd_util.get_service_url')
+    def test_handle_function_service_with_versioned_function_version_0(
+            self, mock_srv_url, mock_etcd_delete
+    ):
+        # This case tests that if a function has multiple versions, service
+        # which serves executions of function version 0 is correctly handled
+        # when expired.
+        db_func = self.create_function()
+        function_id = db_func.id
+        self.create_function_version(0, function_id, description="new_version")
+        # Simulate an execution using version 0
+        db_api.update_function(function_id, {'count': 1})
+        time.sleep(1.5)
+
         self.override_config('function_service_expiration', 1, 'engine')
         mock_srv_url.return_value = 'http://localhost:37718'
         mock_engine = mock.Mock()
 
         periodics.handle_function_service_expiration(self.ctx, mock_engine)
 
-        self.assertEqual(1, mock_engine.delete_function.call_count)
-        args, _ = mock_engine.delete_function.call_args
-        self.assertIn(function_id, args)
-        self.assertIn(1, args)
-
-        mock_etcd_delete.assert_called_once_with(function_id, 1)
+        mock_engine.delete_function.assert_called_once_with(
+            self.ctx, function_id, 0
+        )
+        mock_etcd_delete.assert_called_once_with(function_id, 0)
 
     @mock.patch('qinling.utils.jobs.get_next_execution_time')
     def test_job_handler(self, mock_get_next):