From 1d22cfa99a529933d436a449f1971a96c95f2b66 Mon Sep 17 00:00:00 2001 From: Brian Haley Date: Fri, 20 Jan 2023 16:59:26 -0500 Subject: [PATCH] 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 --- neutron/agent/linux/ip_lib.py | 13 +++++++----- neutron/tests/unit/agent/linux/test_ip_lib.py | 21 ++++++++++++++++++- 2 files changed, 28 insertions(+), 6 deletions(-) diff --git a/neutron/agent/linux/ip_lib.py b/neutron/agent/linux/ip_lib.py index 99996928a93..57f027e0c0d 100644 --- a/neutron/agent/linux/ip_lib.py +++ b/neutron/agent/linux/ip_lib.py @@ -586,9 +586,10 @@ class IpAddrCommand(IpDeviceCommandBase): 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: @@ -597,12 +598,14 @@ class IpAddrCommand(IpDeviceCommandBase): 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( diff --git a/neutron/tests/unit/agent/linux/test_ip_lib.py b/neutron/tests/unit/agent/linux/test_ip_lib.py index 352d7ce14d7..7188cd45f2b 100644 --- a/neutron/tests/unit/agent/linux/test_ip_lib.py +++ b/neutron/tests/unit/agent/linux/test_ip_lib.py @@ -777,7 +777,8 @@ class TestIpAddrCommand(TestIPCmdBase): 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')) @@ -787,6 +788,24 @@ class TestIpAddrCommand(TestIPCmdBase): 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=[