From b803195a9979f7b3b0fd9cea41699b33fc8cf2bb Mon Sep 17 00:00:00 2001
From: Yuki Nishiwaki <uckey.1067@gmail.com>
Date: Wed, 25 Jul 2018 16:05:30 +0900
Subject: [PATCH] Dont use dict.get() to know certain key is in dict

In CommonAgentLoop class, there is logic to detect tap device is changed
locally or not by comparing timestamp with previous.
Sometimes timestamp value could be None depending on the timing (see bug/1781129)

But current _get_devices_locally_modified logic can not detect local
change from None to something because _get_devices_locally_modified
function don't always compare if previous timestamp value was None.

In order not to miss updated device always, better not to use dict.get() to
know previous iteration have timestamp or not.

Change-Id: Ib0361ad5c281f88558e8e048cfec588b9f9b1de4
Closes-Bug: #1781129
---
 .../plugins/ml2/drivers/agent/_common_agent.py  |  2 +-
 .../ml2/drivers/agent/test__common_agent.py     | 17 +++++++++++++++++
 2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/neutron/plugins/ml2/drivers/agent/_common_agent.py b/neutron/plugins/ml2/drivers/agent/_common_agent.py
index 4e5b15ac950..c6efd43140a 100644
--- a/neutron/plugins/ml2/drivers/agent/_common_agent.py
+++ b/neutron/plugins/ml2/drivers/agent/_common_agent.py
@@ -366,7 +366,7 @@ class CommonAgentLoop(service.Service):
         returned because this means it is new.
         """
         return {device for device, timestamp in timestamps.items()
-                if previous_timestamps.get(device) and
+                if device in previous_timestamps and
                 timestamp != previous_timestamps.get(device)}
 
     def scan_devices(self, previous, sync):
diff --git a/neutron/tests/unit/plugins/ml2/drivers/agent/test__common_agent.py b/neutron/tests/unit/plugins/ml2/drivers/agent/test__common_agent.py
index c7091bbf5cf..d56796269cb 100644
--- a/neutron/tests/unit/plugins/ml2/drivers/agent/test__common_agent.py
+++ b/neutron/tests/unit/plugins/ml2/drivers/agent/test__common_agent.py
@@ -233,6 +233,23 @@ class TestCommonAgentLoop(base.BaseTestCase):
         self._test_scan_devices(previous, updated, fake_current, expected,
                                 sync=False)
 
+    def test_scan_devices_timestamp_triggers_updated_None_to_something(self):
+        previous = {'current': set([1, 2]),
+                    'updated': set(),
+                    'added': set(),
+                    'removed': set(),
+                    'timestamps': {2: None}}
+        fake_current = set([1, 2])
+        updated = set()
+        expected = {'current': set([1, 2]),
+                    'updated': set([2]),
+                    'added': set(),
+                    'removed': set(),
+                    'timestamps': {2: 1000}}
+
+        self._test_scan_devices(previous, updated, fake_current, expected,
+                                sync=False, fake_ts_current={2: 1000})
+
     def test_scan_devices_timestamp_triggers_updated(self):
         previous = {'current': set([1, 2]),
                     'updated': set(),