Change flag check order in wait_until_address_ready()

When DAD fails on an IPv6 address, both the 'dadfailed'
and 'tentative' flags will be set. So change the code
to check for 'dadfailed' first, just to be explicit.

Added better unit testing to cover more cases as well.

Trivialfix

Change-Id: I2dddc296826e5ab5e057c32a554e353577cc36e8
(cherry picked from commit 1d22cfa)
(cherry picked from commit 0c18181)

Author: brianphaley

neutron/agent/linux/ip_lib.py

@@ -578,9 +578,10 @@ def filter_device(device, filters):
         return filtered_devices
 
     def wait_until_address_ready(self, address, wait_time=30):
-        """Wait until an address is no longer marked 'tentative'
+        """Wait until an address is no longer marked 'tentative' or 'dadfailed'
 
-        raises AddressNotReady if times out or address not present on interface
+        raises AddressNotReady if times out, address not present on interface
+               or DAD fails
         """
         def is_address_ready():
             try:
@@ -589,12 +590,14 @@ def is_address_ready():
                 raise AddressNotReady(
                     address=address,
                     reason=_('Address not present on interface'))
-            if not addr_info['tentative']:
-                return True
+            # Since both 'dadfailed' and 'tentative' will be set if DAD fails,
+            # check 'dadfailed' first just to be explicit
             if addr_info['dadfailed']:
                 raise AddressNotReady(
                     address=address, reason=_('Duplicate address detected'))
-            return False
+            if addr_info['tentative']:
+                return False
+            return True
         errmsg = _("Exceeded %s second limit waiting for "
                    "address to leave the tentative state.") % wait_time
         common_utils.wait_until_true(

neutron/tests/unit/agent/linux/test_ip_lib.py

@@ -776,7 +776,8 @@ def test_flush(self, flush):
             6, self.parent.name, self.addr_cmd._parent.namespace)
 
     def test_wait_until_address_ready(self):
-        self.addr_cmd.list = mock.Mock(return_value=[{'tentative': False}])
+        self.addr_cmd.list = mock.Mock(
+            return_value=[{'tentative': False, 'dadfailed': False}])
         # this address is not tentative or failed so it should return
         self.assertIsNone(self.addr_cmd.wait_until_address_ready(
             '2001:470:9:1224:fd91:272:581e:3a32'))
@@ -786,6 +787,24 @@ def test_wait_until_address_ready_non_existent_address(self):
         with testtools.ExpectedException(ip_lib.AddressNotReady):
             self.addr_cmd.wait_until_address_ready('abcd::1234')
 
+    def test_wait_until_address_dadfailed(self):
+        self.addr_cmd.list = mock.Mock(
+            return_value=[{'tentative': True, 'dadfailed': True}])
+        with testtools.ExpectedException(ip_lib.AddressNotReady):
+            self.addr_cmd.wait_until_address_ready('abcd::1234')
+
+    @mock.patch.object(common_utils, 'wait_until_true')
+    def test_wait_until_address_ready_success_one_timeout(self, mock_wuntil):
+        tentative_address = 'fe80::3023:39ff:febc:22ae'
+        self.addr_cmd.list = mock.Mock(return_value=[
+            dict(scope='link', dadfailed=False, tentative=True, dynamic=False,
+                 cidr=tentative_address + '/64'),
+            dict(scope='link', dadfailed=False, tentative=False, dynamic=False,
+                 cidr=tentative_address + '/64')])
+        self.assertIsNone(self.addr_cmd.wait_until_address_ready(
+            tentative_address, wait_time=3))
+        self.assertEqual(1, mock_wuntil.call_count)
+
     def test_wait_until_address_ready_timeout(self):
         tentative_address = 'fe80::3023:39ff:febc:22ae'
         self.addr_cmd.list = mock.Mock(return_value=[