From: Sergey Vilgelm <sergey@vilgelm.info>
Date: Mon, 31 Aug 2015 14:06:48 +0000 (+0300)
Subject: Fix a wrong condition for the _purge_metering_info function
X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=997aa86fa12e3209b65741ef95906d491895a493;p=openstack-build%2Fneutron-build.git

Fix a wrong condition for the _purge_metering_info function

Fix a situation for the _purge_metering_info function
when the items will never be deleted from the metering_info.
Delete the metering_info dict and use the metering_infos instead.
Fix the problem with changing a dictionary during iteration.
Add the unit tests for the _purge_metering_info and
_add_metering_info functions.

Co-Authored-By: Yaroslav Isakov <yisakov@mirantis.com>

Change-Id: I9031a5f27ae6438ffd5c5a48b0cf5cdc6867eff3
Closes-Bug: #1490581
---

diff --git a/neutron/services/metering/agents/metering_agent.py b/neutron/services/metering/agents/metering_agent.py
index 7531f41e9..10ead2b37 100644
--- a/neutron/services/metering/agents/metering_agent.py
+++ b/neutron/services/metering/agents/metering_agent.py
@@ -77,7 +77,6 @@ class MeteringAgent(MeteringPluginRpc, manager.Manager):
         self.conf = conf or cfg.CONF
         self._load_drivers()
         self.context = context.get_admin_context_without_session()
-        self.metering_info = {}
         self.metering_loop = loopingcall.FixedIntervalLoopingCall(
             self._metering_loop
         )
@@ -118,11 +117,13 @@ class MeteringAgent(MeteringPluginRpc, manager.Manager):
             info['time'] = 0
 
     def _purge_metering_info(self):
-        ts = int(time.time())
-        report_interval = self.conf.report_interval
-        for label_id, info in self.metering_info.items():
-            if info['last_update'] > ts + report_interval:
-                del self.metering_info[label_id]
+        deadline_timestamp = int(time.time()) - self.conf.report_interval
+        label_ids = [
+            label_id
+            for label_id, info in self.metering_infos.items()
+            if info['last_update'] < deadline_timestamp]
+        for label_id in label_ids:
+            del self.metering_infos[label_id]
 
     def _add_metering_info(self, label_id, pkts, bytes):
         ts = int(time.time())
diff --git a/neutron/tests/unit/services/metering/agents/test_metering_agent.py b/neutron/tests/unit/services/metering/agents/test_metering_agent.py
index e95ef9bd3..da2a82a4b 100644
--- a/neutron/tests/unit/services/metering/agents/test_metering_agent.py
+++ b/neutron/tests/unit/services/metering/agents/test_metering_agent.py
@@ -138,6 +138,49 @@ class TestMeteringOperations(base.BaseTestCase):
 
         self.agent._add_metering_info.assert_called_with(label_id, 44, 222)
 
+    @mock.patch('time.time')
+    def _test_purge_metering_info(self, current_timestamp, is_empty,
+                                  mock_time):
+        mock_time.return_value = current_timestamp
+        self.agent.metering_infos = {'fake': {'last_update': 1}}
+        self.config(report_interval=1)
+
+        self.agent._purge_metering_info()
+        self.assertEqual(0 if is_empty else 1, len(self.agent.metering_infos))
+        self.assertEqual(1, mock_time.call_count)
+
+    def test_purge_metering_info(self):
+        # 1 < 2 - 1 -> False
+        self._test_purge_metering_info(2, False)
+
+    def test_purge_metering_info_delete(self):
+        # 1 < 3 - 1 -> False
+        self._test_purge_metering_info(3, True)
+
+    @mock.patch('time.time')
+    def _test_add_metering_info(self, expected_info, current_timestamp,
+                                mock_time):
+        mock_time.return_value = current_timestamp
+        actual_info = self.agent._add_metering_info('fake_label_id', 1, 1)
+        self.assertEqual(1, len(self.agent.metering_infos))
+        self.assertEqual(expected_info, actual_info)
+        self.assertEqual(expected_info,
+                         self.agent.metering_infos['fake_label_id'])
+        self.assertEqual(1, mock_time.call_count)
+
+    def test_add_metering_info_create(self):
+        expected_info = {'bytes': 1, 'pkts': 1, 'time': 0, 'first_update': 1,
+                         'last_update': 1}
+        self._test_add_metering_info(expected_info, 1)
+
+    def test_add_metering_info_update(self):
+        expected_info = {'bytes': 1, 'pkts': 1, 'time': 0, 'first_update': 1,
+                         'last_update': 1}
+        self.agent.metering_infos = {'fake_label_id': expected_info}
+        expected_info.update({'bytes': 2, 'pkts': 2, 'time': 1,
+                              'last_update': 2})
+        self._test_add_metering_info(expected_info, 2)
+
 
 class TestMeteringDriver(base.BaseTestCase):
     def setUp(self):