)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"f974e93f2330c90b6717ec8d0fbd1d654cd97005","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"00e28d17_ed878574","updated":"2023-01-18 20:17:35.000000000","message":"And I still think we need a unit test for the dadfailed:True case.","commit_id":"fe77cc07cc6c1cedb96b1c692cfa43b32033b10f"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"61cecc104f5effa3aa42e2da77db253d12ea8a26","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"6c2be83b_88054075","updated":"2023-01-18 22:41:40.000000000","message":"These are diffs based on PS4 that seem to work, feel free to use them if you like.\n\ndiff --git a/neutron/agent/linux/ip_lib.py b/neutron/agent/linux/ip_lib.py\nindex 965cec6f1c..68e15b6db6 100644\n--- a/neutron/agent/linux/ip_lib.py\n+++ b/neutron/agent/linux/ip_lib.py\n@@ -584,9 +584,10 @@ class IpAddrCommand(IpDeviceCommandBase):\n         return filtered_devices\n \n     def wait_until_address_ready(self, address, wait_time\u003d30):\n-        \"\"\"Wait until address is no longer marked \u0027tentative\u0027 nor \u0027dadfailed\u0027\n+        \"\"\"Wait until address is no longer marked \u0027tentative\u0027\n \n-        raises AddressNotReady if times out or address not present on interface\n+        raises AddressNotReady if times out, address not present on interface\n+               or DAD fails\n         \"\"\"\n         def is_address_ready():\n             try:\n@@ -595,11 +596,11 @@ class IpAddrCommand(IpDeviceCommandBase):\n                 raise AddressNotReady(\n                     address\u003daddress,\n                     reason\u003d_(\u0027Address not present on interface\u0027))\n-            if not addr_info[\u0027dadfailed\u0027] and addr_info[\u0027tentative\u0027]:\n-                return False\n             if addr_info[\u0027dadfailed\u0027]:\n                 raise AddressNotReady(\n                     address\u003daddress, reason\u003d_(\u0027Duplicate address detected\u0027))\n+            if addr_info[\u0027tentative\u0027]:\n+                return False\n             return True\n         errmsg \u003d _(\"Exceeded %s second limit waiting for \"\n                    \"address to leave the tentative state.\") % wait_time\ndiff --git a/neutron/tests/unit/agent/linux/test_ip_lib.py b/neutron/tests/unit/agent/linux/test_ip_lib.py\nindex b98d885b4a..9186eca5b4 100644\n--- a/neutron/tests/unit/agent/linux/test_ip_lib.py\n+++ b/neutron/tests/unit/agent/linux/test_ip_lib.py\n@@ -777,7 +777,8 @@ class TestIpAddrCommand(TestIPCmdBase):\n             6, self.parent.name, self.addr_cmd._parent.namespace)\n \n     def test_wait_until_address_ready(self):\n-        self.addr_cmd.list \u003d mock.Mock(return_value\u003d[{\u0027tentative\u0027: False}])\n+        self.addr_cmd.list \u003d mock.Mock(\n+            return_value\u003d[{\u0027tentative\u0027: False, \u0027dadfailed\u0027: False}])\n         # this address is not tentative or failed so it should return\n         self.assertIsNone(self.addr_cmd.wait_until_address_ready(\n             \u00272001:470:9:1224:fd91:272:581e:3a32\u0027))\n@@ -787,6 +788,24 @@ class TestIpAddrCommand(TestIPCmdBase):\n         with testtools.ExpectedException(ip_lib.AddressNotReady):\n             self.addr_cmd.wait_until_address_ready(\u0027abcd::1234\u0027)\n \n+    def test_wait_until_address_dadfailed(self):\n+        self.addr_cmd.list \u003d mock.Mock(\n+            return_value\u003d[{\u0027tentative\u0027: True, \u0027dadfailed\u0027: True}])\n+        with testtools.ExpectedException(ip_lib.AddressNotReady):\n+            self.addr_cmd.wait_until_address_ready(\u0027abcd::1234\u0027)\n+\n+    @mock.patch.object(common_utils, \u0027wait_until_true\u0027)\n+    def test_wait_until_address_ready_success_one_timeout(self, mock_wuntil):\n+        tentative_address \u003d \u0027fe80::3023:39ff:febc:22ae\u0027\n+        self.addr_cmd.list \u003d mock.Mock(return_value\u003d[\n+            dict(scope\u003d\u0027link\u0027, dadfailed\u003dFalse, tentative\u003dTrue, dynamic\u003dFalse,\n+                 cidr\u003dtentative_address + \u0027/64\u0027),\n+            dict(scope\u003d\u0027link\u0027, dadfailed\u003dFalse, tentative\u003dFalse, dynamic\u003dFalse,\n+                 cidr\u003dtentative_address + \u0027/64\u0027)])\n+        self.assertIsNone(self.addr_cmd.wait_until_address_ready(\n+            tentative_address, wait_time\u003d3))\n+        self.assertEqual(1, mock_wuntil.call_count)\n+\n     def test_wait_until_address_ready_timeout(self):\n         tentative_address \u003d \u0027fe80::3023:39ff:febc:22ae\u0027\n         self.addr_cmd.list \u003d mock.Mock(return_value\u003d[","commit_id":"fe77cc07cc6c1cedb96b1c692cfa43b32033b10f"}],"neutron/agent/linux/ip_lib.py":[{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"e8ef86ff137dbe15df4ce62dcdbd3d7c1f7c0887","unresolved":true,"context_lines":[{"line_number":597,"context_line":"                    reason\u003d_(\u0027Address not present on interface\u0027))"},{"line_number":598,"context_line":"            if addr_info[\u0027tentative\u0027]:"},{"line_number":599,"context_line":"                raise AddressNotReady("},{"line_number":600,"context_line":"                    address\u003daddress, reason\u003d_(\u0027Address still being verified\u0027))"},{"line_number":601,"context_line":"            if addr_info[\u0027dadfailed\u0027]:"},{"line_number":602,"context_line":"                raise AddressNotReady("},{"line_number":603,"context_line":"                    address\u003daddress, reason\u003d_(\u0027Duplicate address detected\u0027))"}],"source_content_type":"text/x-python","patch_set":1,"id":"627a5dbe_6b5d6d18","line":600,"updated":"2023-01-18 16:33:01.000000000","message":"Should this just be \u0027return False\u0027 as it can leave the tentative state eventually, even if initially it is set. Won\u0027t the raise cause it to fail if it\u0027s set the first check?","commit_id":"c01fb71e9f9cafb7da4b24d93fc1adee1987303b"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"4b31b364cf2f8ab02d873f1f0f4b97d6bb531324","unresolved":false,"context_lines":[{"line_number":597,"context_line":"                    reason\u003d_(\u0027Address not present on interface\u0027))"},{"line_number":598,"context_line":"            if addr_info[\u0027tentative\u0027]:"},{"line_number":599,"context_line":"                raise AddressNotReady("},{"line_number":600,"context_line":"                    address\u003daddress, reason\u003d_(\u0027Address still being verified\u0027))"},{"line_number":601,"context_line":"            if addr_info[\u0027dadfailed\u0027]:"},{"line_number":602,"context_line":"                raise AddressNotReady("},{"line_number":603,"context_line":"                    address\u003daddress, reason\u003d_(\u0027Duplicate address detected\u0027))"}],"source_content_type":"text/x-python","patch_set":1,"id":"9080a0ad_034e178f","line":600,"in_reply_to":"627a5dbe_6b5d6d18","updated":"2023-01-18 16:54:58.000000000","message":"Right, I just need to return False here.","commit_id":"c01fb71e9f9cafb7da4b24d93fc1adee1987303b"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"e8ef86ff137dbe15df4ce62dcdbd3d7c1f7c0887","unresolved":true,"context_lines":[{"line_number":598,"context_line":"            if addr_info[\u0027tentative\u0027]:"},{"line_number":599,"context_line":"                raise AddressNotReady("},{"line_number":600,"context_line":"                    address\u003daddress, reason\u003d_(\u0027Address still being verified\u0027))"},{"line_number":601,"context_line":"            if addr_info[\u0027dadfailed\u0027]:"},{"line_number":602,"context_line":"                raise AddressNotReady("},{"line_number":603,"context_line":"                    address\u003daddress, reason\u003d_(\u0027Duplicate address detected\u0027))"},{"line_number":604,"context_line":"            return True"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a8e56ef_ee147622","line":601,"updated":"2023-01-18 16:33:01.000000000","message":"Maybe just move this check above the other since it\u0027s always a failure?","commit_id":"c01fb71e9f9cafb7da4b24d93fc1adee1987303b"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"4b31b364cf2f8ab02d873f1f0f4b97d6bb531324","unresolved":false,"context_lines":[{"line_number":598,"context_line":"            if addr_info[\u0027tentative\u0027]:"},{"line_number":599,"context_line":"                raise AddressNotReady("},{"line_number":600,"context_line":"                    address\u003daddress, reason\u003d_(\u0027Address still being verified\u0027))"},{"line_number":601,"context_line":"            if addr_info[\u0027dadfailed\u0027]:"},{"line_number":602,"context_line":"                raise AddressNotReady("},{"line_number":603,"context_line":"                    address\u003daddress, reason\u003d_(\u0027Duplicate address detected\u0027))"},{"line_number":604,"context_line":"            return True"}],"source_content_type":"text/x-python","patch_set":1,"id":"602ebb4a_1465f5a3","line":601,"in_reply_to":"3a8e56ef_ee147622","updated":"2023-01-18 16:54:58.000000000","message":"No, we should check \"tentative\" first","commit_id":"c01fb71e9f9cafb7da4b24d93fc1adee1987303b"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"d634219d48cb11b0a8ed37d04b02b8b5c99aebe7","unresolved":false,"context_lines":[{"line_number":598,"context_line":"            if addr_info[\u0027tentative\u0027]:"},{"line_number":599,"context_line":"                raise AddressNotReady("},{"line_number":600,"context_line":"                    address\u003daddress, reason\u003d_(\u0027Address still being verified\u0027))"},{"line_number":601,"context_line":"            if addr_info[\u0027dadfailed\u0027]:"},{"line_number":602,"context_line":"                raise AddressNotReady("},{"line_number":603,"context_line":"                    address\u003daddress, reason\u003d_(\u0027Duplicate address detected\u0027))"},{"line_number":604,"context_line":"            return True"}],"source_content_type":"text/x-python","patch_set":1,"id":"db395854_ec4e4dad","line":601,"in_reply_to":"602ebb4a_1465f5a3","updated":"2023-01-18 17:20:42.000000000","message":"The only reason I mentioned it is that if DAD has failed we are done, but if tentative we should loop again. And since I didn\u0027t remember the IPv6 state machine I ran a test by adding my upstream gateway link-local address on my system, which failed immediately:\n\ninet6 fe80::1e3b:f3ff:fed6:8730/64 scope link dadfailed tentative\n\nSo in that case we could loop again unnecessarily because both flags are set.","commit_id":"c01fb71e9f9cafb7da4b24d93fc1adee1987303b"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"60564c719f2a7396d1220bf3eb85ed39749f780e","unresolved":false,"context_lines":[{"line_number":598,"context_line":"            if addr_info[\u0027tentative\u0027]:"},{"line_number":599,"context_line":"                raise AddressNotReady("},{"line_number":600,"context_line":"                    address\u003daddress, reason\u003d_(\u0027Address still being verified\u0027))"},{"line_number":601,"context_line":"            if addr_info[\u0027dadfailed\u0027]:"},{"line_number":602,"context_line":"                raise AddressNotReady("},{"line_number":603,"context_line":"                    address\u003daddress, reason\u003d_(\u0027Duplicate address detected\u0027))"},{"line_number":604,"context_line":"            return True"}],"source_content_type":"text/x-python","patch_set":1,"id":"51366bea_1a113907","line":601,"in_reply_to":"db395854_ec4e4dad","updated":"2023-01-18 17:56:24.000000000","message":"Right, I\u0027ll add a new condition there.","commit_id":"c01fb71e9f9cafb7da4b24d93fc1adee1987303b"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"9246ef320c32d6a531466115a5c3cdc2b4ed6f12","unresolved":true,"context_lines":[{"line_number":596,"context_line":"                    address\u003daddress,"},{"line_number":597,"context_line":"                    reason\u003d_(\u0027Address not present on interface\u0027))"},{"line_number":598,"context_line":"            if not addr_info[\u0027dadfailed\u0027] and addr_info[\u0027tentative\u0027]:"},{"line_number":599,"context_line":"                return False"},{"line_number":600,"context_line":"            if addr_info[\u0027dadfailed\u0027]:"},{"line_number":601,"context_line":"                raise AddressNotReady("},{"line_number":602,"context_line":"                    address\u003daddress, reason\u003d_(\u0027Duplicate address detected\u0027))"}],"source_content_type":"text/x-python","patch_set":4,"id":"10b9ce92_c45ff329","line":599,"updated":"2023-01-18 17:46:16.000000000","message":"I don\u0027t understand this now, as it will have the same effect of just moving the dadfailed check above the tentative one. DAD failed is fatal, while tentative is transitory typically, and they only seem to both be set when DAD fails.\n\nNote: there is no unit test for the dadfailed\u003dtrue case.","commit_id":"fe77cc07cc6c1cedb96b1c692cfa43b32033b10f"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"60564c719f2a7396d1220bf3eb85ed39749f780e","unresolved":true,"context_lines":[{"line_number":596,"context_line":"                    address\u003daddress,"},{"line_number":597,"context_line":"                    reason\u003d_(\u0027Address not present on interface\u0027))"},{"line_number":598,"context_line":"            if not addr_info[\u0027dadfailed\u0027] and addr_info[\u0027tentative\u0027]:"},{"line_number":599,"context_line":"                return False"},{"line_number":600,"context_line":"            if addr_info[\u0027dadfailed\u0027]:"},{"line_number":601,"context_line":"                raise AddressNotReady("},{"line_number":602,"context_line":"                    address\u003daddress, reason\u003d_(\u0027Duplicate address detected\u0027))"}],"source_content_type":"text/x-python","patch_set":4,"id":"59f94ae1_05a9a3eb","line":599,"in_reply_to":"10b9ce92_c45ff329","updated":"2023-01-18 17:56:24.000000000","message":"No, we are now first checking the tentative flag is not set. In this case, we check the \"dadfailed\" flag and raise the exception if needed.\n\nIn base PS, the \"tentative\" could be False and \"dadfailed\" True, but the method was returning True. This condition is now not possible.","commit_id":"fe77cc07cc6c1cedb96b1c692cfa43b32033b10f"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"f974e93f2330c90b6717ec8d0fbd1d654cd97005","unresolved":true,"context_lines":[{"line_number":596,"context_line":"                    address\u003daddress,"},{"line_number":597,"context_line":"                    reason\u003d_(\u0027Address not present on interface\u0027))"},{"line_number":598,"context_line":"            if not addr_info[\u0027dadfailed\u0027] and addr_info[\u0027tentative\u0027]:"},{"line_number":599,"context_line":"                return False"},{"line_number":600,"context_line":"            if addr_info[\u0027dadfailed\u0027]:"},{"line_number":601,"context_line":"                raise AddressNotReady("},{"line_number":602,"context_line":"                    address\u003daddress, reason\u003d_(\u0027Duplicate address detected\u0027))"}],"source_content_type":"text/x-python","patch_set":4,"id":"81d9f38d_e68b04c7","line":599,"in_reply_to":"59f94ae1_05a9a3eb","updated":"2023-01-18 20:17:35.000000000","message":"Rodolfo - if the dadfailed flag is true, then this function should raise, irregardless of the tentative flag, since the address will never transition out of that state. That\u0027s why I suggested it be first. Then, only if DAD is still running, would we see the tentative flag, and we should return False and loop.\n\nFor example, if self.list() returns this dict:\n\naddr_info \u003d {\u0027dadfailed\u0027: True, \u0027tentative\u0027: True}\n\nThat\u0027s what will happen, since we\u0027ll never check addr_info[\u0027tentative\u0027] at all.\n\nI verified this logic returns the same results locally with all possible dictionaries:\n\nhttps://paste.opendev.org/show/buJs58jSRVcscHJ72S74/","commit_id":"fe77cc07cc6c1cedb96b1c692cfa43b32033b10f"}]}