)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":24824,"name":"Dmitrii Shcherbakov","username":"dmitriis"},"change_message_id":"0890f08b843eea02ac72bae092fd81594759bb30","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":24,"id":"f734e488_d10243fb","updated":"2023-04-27 22:13:25.000000000","message":"recheck","commit_id":"0673e1e71609a9fd212d94213953d29463fe260c"},{"author":{"_account_id":24824,"name":"Dmitrii Shcherbakov","username":"dmitriis"},"change_message_id":"f6179d3a624a4aae5cfd50458b192fb9ac2bb9e8","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":24,"id":"2049ceeb_9e29bbd3","updated":"2023-04-28 13:46:07.000000000","message":"recheck vexxhost nested virt hosts disabled","commit_id":"0673e1e71609a9fd212d94213953d29463fe260c"},{"author":{"_account_id":24824,"name":"Dmitrii Shcherbakov","username":"dmitriis"},"change_message_id":"6ac36a67ebec6ae1329e5ea0a115d7a507837ed0","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":24,"id":"38303d23_8ad4811c","updated":"2023-05-01 20:48:37.000000000","message":"recheck vexxhost nested virt hosts disabled","commit_id":"0673e1e71609a9fd212d94213953d29463fe260c"},{"author":{"_account_id":24824,"name":"Dmitrii Shcherbakov","username":"dmitriis"},"change_message_id":"1ae5596c1797e6ad20f66aeaebc9682c09e1fbe9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":24,"id":"3a12d780_e85cc03f","updated":"2023-04-30 14:11:57.000000000","message":"recheck vexxhost nested virt hosts disabled","commit_id":"0673e1e71609a9fd212d94213953d29463fe260c"},{"author":{"_account_id":24824,"name":"Dmitrii Shcherbakov","username":"dmitriis"},"change_message_id":"3f4174cdf50d687b4c0390b7cebd85a5dbfe9644","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":28,"id":"0f8d940f_8775cc72","updated":"2023-05-11 18:33:37.000000000","message":"Reproduced a failure in `neutron_tempest_plugin.scenario.test_connectivity.NetworkConnectivityTest.test_connectivity_through_2_routers` consistently in a local Devstack environment.\n\nThe issue is that some ovn-nb state related to the gateway port gets removed after a static route is added to a router which is why the test code cannot ssh into an instance afterwards.\n\nI am investigating this further to come up with a fix but it seems like a legitimate issue somewhere in the currently proposed code of this specific change (it does not happen in the earlier ones).","commit_id":"11c604c45992b71bf2ff37f88a11017399e68f04"},{"author":{"_account_id":24824,"name":"Dmitrii Shcherbakov","username":"dmitriis"},"change_message_id":"ee0bf4c74e8a00e2b63f865ac1f86b7017f8dcb2","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":28,"id":"f35f6bc8_288eed83","updated":"2023-05-09 10:31:48.000000000","message":"https://zuul.opendev.org/t/openstack/build/0219dd5bf3ec4b0fbf433f6622243a5f\n```\n    Response - Headers: {\u0027date\u0027: \u0027Mon, 08 May 2023 23:29:59 GMT\u0027, \u0027server\u0027: \u0027Apache/2.4.41 (Ubuntu)\u0027, \u0027openstack-api-version\u0027: \u0027compute 2.1\u0027, \u0027x-openstack-nova-api-version\u0027: \u00272.1\u0027, \u0027vary\u0027: \u0027OpenStack-API-Version,X-OpenStack-Nova-API-Version\u0027, \u0027content-type\u0027: \u0027application/json; charset\u003dUTF-8\u0027, \u0027content-length\u0027: \u0027111\u0027, \u0027x-openstack-request-id\u0027: \u0027req-4b7fca07-f03d-4514-ba37-59932156fe68\u0027, \u0027x-compute-request-id\u0027: \u0027req-4b7fca07-f03d-4514-ba37-59932156fe68\u0027, \u0027connection\u0027: \u0027close\u0027, \u0027status\u0027: \u0027404\u0027, \u0027content-location\u0027: \u0027https://158.69.77.37/compute/v2.1/servers/6b6781e0-e5ed-403e-bdbf-4b42890f3a5a\u0027}\n        Body: b\u0027{\"itemNotFound\": {\"code\": 404, \"message\": \"Instance 6b6781e0-e5ed-403e-bdbf-4b42890f3a5a could not be found.\"}}\u0027\n```","commit_id":"11c604c45992b71bf2ff37f88a11017399e68f04"},{"author":{"_account_id":24824,"name":"Dmitrii Shcherbakov","username":"dmitriis"},"change_message_id":"6157e37bf0fb5fc60016ca2239306a35cb865cd4","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":28,"id":"6eb659dc_523e87b1","updated":"2023-05-08 22:02:56.000000000","message":"recheck","commit_id":"11c604c45992b71bf2ff37f88a11017399e68f04"},{"author":{"_account_id":24824,"name":"Dmitrii Shcherbakov","username":"dmitriis"},"change_message_id":"fa70c828e38dc011428646880746f15cdf18f24e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":28,"id":"3de63b39_9019d81e","updated":"2023-05-09 10:32:23.000000000","message":"recheck vexxhost nested virt hosts disabled","commit_id":"11c604c45992b71bf2ff37f88a11017399e68f04"},{"author":{"_account_id":13686,"name":"Frode Nordahl","email":"fnordahl@ubuntu.com","username":"fnordahl"},"change_message_id":"9f8818fcd92c8ab74d945d193a28bcbe7d56fce1","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":30,"id":"e856ce4a_301edef6","updated":"2023-05-16 14:13:15.000000000","message":"Another note on this discussion is that we can\u0027t use the request data as long as it does not provide the gw_ports in a structured form.\n\nIt used to work before because the single `gw_port_id` is injected in the request data, but we do not presently have the same for multiple gw ports.","commit_id":"2f30d7cefa452765187b8b4e36c60732a9faca54"},{"author":{"_account_id":13686,"name":"Frode Nordahl","email":"fnordahl@ubuntu.com","username":"fnordahl"},"change_message_id":"27d715af1b7a4072ff196be0e5c6ea4f23061175","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":31,"id":"b8893cf4_52e3e4d5","updated":"2023-05-16 15:53:42.000000000","message":"Last failure for both of the failed jobs due to environment/infra issues:\n\nConnecting to cloud-images.ubuntu.com (cloud-images.ubuntu.com)|2620:2d:4000:1::1a|:443... connected.\nUnable to establish SSL connection.\n+ /opt/stack/neutron-tempest-plugin/devstack/customize_image.sh:upload_image:1 :   exit_trap\n...","commit_id":"e21fe2ef8c6347ec56154b9e281b87ff0142f3b0"},{"author":{"_account_id":13686,"name":"Frode Nordahl","email":"fnordahl@ubuntu.com","username":"fnordahl"},"change_message_id":"8a7b877cd69ed24b577e9b84a6d597c599b5a66c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":31,"id":"59503f08_0e9d7706","updated":"2023-05-16 15:53:56.000000000","message":"recheck","commit_id":"e21fe2ef8c6347ec56154b9e281b87ff0142f3b0"},{"author":{"_account_id":24824,"name":"Dmitrii Shcherbakov","username":"dmitriis"},"change_message_id":"4a10b27c9bb6324283c5ae881f12932a5fb1d538","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":34,"id":"4bb51145_9f9351e8","updated":"2023-05-19 14:14:21.000000000","message":"recheck","commit_id":"ac434ebaa4f836389a99026fb7628e8a0ea52670"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"dd2abe8853f9438e4d973fae2d8191d0599d1609","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":37,"id":"0f2777ef_3e4670bf","updated":"2023-06-14 20:36:57.000000000","message":"Had a few question, looks good otherwise.","commit_id":"ee1ecad6b709b52f10f945063f8faac32e27f757"},{"author":{"_account_id":13686,"name":"Frode Nordahl","email":"fnordahl@ubuntu.com","username":"fnordahl"},"change_message_id":"4c7169b04f523e7b1d57c752575006e8925c7b2b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":37,"id":"19166cea_8825befa","updated":"2023-06-21 08:19:40.000000000","message":"New patch set imminent.","commit_id":"ee1ecad6b709b52f10f945063f8faac32e27f757"},{"author":{"_account_id":13686,"name":"Frode Nordahl","email":"fnordahl@ubuntu.com","username":"fnordahl"},"change_message_id":"1b84284dfabf908c3e561f2f600d295df156902b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":37,"id":"218be5cb_e3ea918e","updated":"2023-06-19 14:22:19.000000000","message":"Thank you for the review, Brian, much appreciated. New patch set on the way shortly.","commit_id":"ee1ecad6b709b52f10f945063f8faac32e27f757"},{"author":{"_account_id":13686,"name":"Frode Nordahl","email":"fnordahl@ubuntu.com","username":"fnordahl"},"change_message_id":"f27c4dd68c9d2c73cd122fc8e346976f0c04afe0","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":43,"id":"89be6a2f_9b7d8d9f","updated":"2023-07-06 19:19:45.000000000","message":"recheck timed out","commit_id":"8d7eb454c0ab754f6045a1a4ec3da36bcbad03ef"},{"author":{"_account_id":13686,"name":"Frode Nordahl","email":"fnordahl@ubuntu.com","username":"fnordahl"},"change_message_id":"c490637e12eb216bc6560fc6a34c4908bfcadcc3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":43,"id":"9f52cc58_e75aa65a","updated":"2023-07-07 05:10:41.000000000","message":"recheck timed out","commit_id":"8d7eb454c0ab754f6045a1a4ec3da36bcbad03ef"},{"author":{"_account_id":8313,"name":"Lajos Katona","display_name":"lajoskatona","email":"katonalala@gmail.com","username":"elajkat","status":"Ericsson Software Technology"},"change_message_id":"9ec1479f23f474031bc437c5690af88dfd93f49e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":46,"id":"7c4e5032_5c610577","updated":"2023-08-24 07:10:02.000000000","message":"As far as I tested it in my limited env it is ok","commit_id":"a87148dd68c8d256d7894cb63518963bd8dd9491"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"899be28693ad48ff7f9e1187431a34555df23f0c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":53,"id":"93e1a56d_08df5200","updated":"2024-01-21 23:47:54.000000000","message":"Needs a manual rebase to master","commit_id":"8c4e6a1476e63e6d9f032e6d18adb89f04c48253"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"4e6c96ea79bef8e6ef7534bbf07814f22c73cbe1","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":54,"id":"e42623be_4057a707","updated":"2024-01-26 01:55:52.000000000","message":"Just had to manually rebase","commit_id":"0bc9a713872d9639b234eaffce9f43c2fd4be98f"}],"neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py":[{"author":{"_account_id":24824,"name":"Dmitrii Shcherbakov","username":"dmitriis"},"change_message_id":"106cc66e6c65934a53434945822f3eee88b248a2","unresolved":true,"context_lines":[{"line_number":1431,"context_line":"        router_id \u003d new_router[\u0027id\u0027]"},{"line_number":1432,"context_line":"        router_name \u003d utils.ovn_name(router_id)"},{"line_number":1433,"context_line":"        ovn_router \u003d self._nb_idl.get_lrouter(router_name)"},{"line_number":1434,"context_line":"        gateway_new \u003d self._get_router_gw_ports(context, router_id)"},{"line_number":1435,"context_line":"        gateway_old \u003d utils.get_lrouter_ext_gw_static_route(ovn_router)"},{"line_number":1436,"context_line":"        added_gw_ports \u003d []"},{"line_number":1437,"context_line":"        deleted_gw_port_ids \u003d []"}],"source_content_type":"text/x-python","patch_set":28,"id":"c8d49c7f_50561bdf","line":1434,"updated":"2023-05-11 22:01:55.000000000","message":"This call here causes the CI failure (https://review.opendev.org/c/openstack/neutron/+/874199/comments/0f8d940f_8775cc72) because it returns an empty list upon a router update and overwrites the existing external gateway configuration.\n\nhttps://paste.opendev.org/show/bqDz18P2CR3V5zIaeBuA/\n\nA router update is done at this point in the test and it overrides the existing state:\n\nhttps://github.com/openstack/neutron-tempest-plugin/blob/bd7d0e8402af758a5de13ebc98dcb96ef558e61a/neutron_tempest_plugin/scenario/test_connectivity.py#L108-L111\n\n```\n\u003e /opt/stack/neutron/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py(1451)update_router()-\u003eNone\n-\u003e txn.add(check_rev_cmd)\nn\n\u003e /opt/stack/neutron/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py(1452)update_router()-\u003eNone\n-\u003e if gateway_new and not gateway_old:\ngateway_new\n[]\ngateway_old\n[\u003covs.db.idl.Row object at 0x7fa945ed5c00\u003e, \u003covs.db.idl.Row object at 0x7fa9467ebaf0\u003e]\nn\n\u003e /opt/stack/neutron/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py(1456)update_router()-\u003eNone\n-\u003e elif gateway_old and not gateway_new:\nn\n\u003e /opt/stack/neutron/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py(1458)update_router()-\u003eNone\n-\u003e txn.add(self._nb_idl.delete_lrouter_ext_gw(router_name))\n```\n\nIn the old code the information is fetched from the request data `new_router.get(l3.EXTERNAL_GW_INFO)`, not the DB and the new code should do something along those lines too.","commit_id":"11c604c45992b71bf2ff37f88a11017399e68f04"},{"author":{"_account_id":13686,"name":"Frode Nordahl","email":"fnordahl@ubuntu.com","username":"fnordahl"},"change_message_id":"b0822d157cd56d97c83d3d01454e5b6e4b7e0686","unresolved":false,"context_lines":[{"line_number":1431,"context_line":"        router_id \u003d new_router[\u0027id\u0027]"},{"line_number":1432,"context_line":"        router_name \u003d utils.ovn_name(router_id)"},{"line_number":1433,"context_line":"        ovn_router \u003d self._nb_idl.get_lrouter(router_name)"},{"line_number":1434,"context_line":"        gateway_new \u003d self._get_router_gw_ports(context, router_id)"},{"line_number":1435,"context_line":"        gateway_old \u003d utils.get_lrouter_ext_gw_static_route(ovn_router)"},{"line_number":1436,"context_line":"        added_gw_ports \u003d []"},{"line_number":1437,"context_line":"        deleted_gw_port_ids \u003d []"}],"source_content_type":"text/x-python","patch_set":28,"id":"622f093e_4b50a131","line":1434,"in_reply_to":"c8d49c7f_50561bdf","updated":"2023-05-12 08:35:25.000000000","message":"Great catch, the test passes in the updated patch set.","commit_id":"11c604c45992b71bf2ff37f88a11017399e68f04"},{"author":{"_account_id":13686,"name":"Frode Nordahl","email":"fnordahl@ubuntu.com","username":"fnordahl"},"change_message_id":"53e8bcad597584346ca0f611d2a7b613c94402b8","unresolved":true,"context_lines":[{"line_number":1194,"context_line":"                LOG.error(\u0027Unable to disassociate floating ip in gateway \u0027"},{"line_number":1195,"context_line":"                          \u0027router. Error: %s\u0027, e)"},{"line_number":1196,"context_line":""},{"line_number":1197,"context_line":"    def _get_gw_info(self, context, port_dict):"},{"line_number":1198,"context_line":"        gateways_info \u003d []"},{"line_number":1199,"context_line":"        network_id \u003d port_dict.get(\u0027network_id\u0027)"},{"line_number":1200,"context_line":"        for fixed_ip in port_dict.get(\u0027fixed_ips\u0027):"}],"source_content_type":"text/x-python","patch_set":29,"id":"80c9e499_b21694e8","line":1197,"updated":"2023-05-16 05:27:37.000000000","message":"This may be problematic though, because it was apparently used both on existing router objects from DB and request objects in the old code, so this change may be a source of issues.","commit_id":"7cd912de3a4e12980fd3625d92f5475a0f26f1cf"},{"author":{"_account_id":13686,"name":"Frode Nordahl","email":"fnordahl@ubuntu.com","username":"fnordahl"},"change_message_id":"0ff0d99a9aaf14c05c77a910195b971ddb0f0591","unresolved":false,"context_lines":[{"line_number":1194,"context_line":"                LOG.error(\u0027Unable to disassociate floating ip in gateway \u0027"},{"line_number":1195,"context_line":"                          \u0027router. Error: %s\u0027, e)"},{"line_number":1196,"context_line":""},{"line_number":1197,"context_line":"    def _get_gw_info(self, context, port_dict):"},{"line_number":1198,"context_line":"        gateways_info \u003d []"},{"line_number":1199,"context_line":"        network_id \u003d port_dict.get(\u0027network_id\u0027)"},{"line_number":1200,"context_line":"        for fixed_ip in port_dict.get(\u0027fixed_ips\u0027):"}],"source_content_type":"text/x-python","patch_set":29,"id":"2440a501_7ee79896","line":1197,"in_reply_to":"80c9e499_b21694e8","updated":"2023-05-16 07:18:35.000000000","message":"Done","commit_id":"7cd912de3a4e12980fd3625d92f5475a0f26f1cf"},{"author":{"_account_id":24824,"name":"Dmitrii Shcherbakov","username":"dmitriis"},"change_message_id":"78de39b8f0a139ab45f9fead1d485aec679db64d","unresolved":true,"context_lines":[{"line_number":1412,"context_line":"            if add_external_gateway:"},{"line_number":1413,"context_line":"                networks \u003d self._get_v4_network_of_all_router_ports("},{"line_number":1414,"context_line":"                    context, router[\u0027id\u0027])"},{"line_number":1415,"context_line":"                if self._get_router_gw_ports("},{"line_number":1416,"context_line":"                        context, router[\u0027id\u0027]) and networks is not None:"},{"line_number":1417,"context_line":"                    added_gw_ports \u003d self._add_router_ext_gw("},{"line_number":1418,"context_line":"                        router, networks, txn)"}],"source_content_type":"text/x-python","patch_set":29,"id":"7e18bee6_8aa4b987","line":1415,"updated":"2023-05-16 00:44:43.000000000","message":"Same issue here: need to retrieve the gw ports from the request instead.","commit_id":"7cd912de3a4e12980fd3625d92f5475a0f26f1cf"},{"author":{"_account_id":13686,"name":"Frode Nordahl","email":"fnordahl@ubuntu.com","username":"fnordahl"},"change_message_id":"6a3a90a468ab5fdc8ef638e1949a4d410006684c","unresolved":true,"context_lines":[{"line_number":1412,"context_line":"            if add_external_gateway:"},{"line_number":1413,"context_line":"                networks \u003d self._get_v4_network_of_all_router_ports("},{"line_number":1414,"context_line":"                    context, router[\u0027id\u0027])"},{"line_number":1415,"context_line":"                if self._get_router_gw_ports("},{"line_number":1416,"context_line":"                        context, router[\u0027id\u0027]) and networks is not None:"},{"line_number":1417,"context_line":"                    added_gw_ports \u003d self._add_router_ext_gw("},{"line_number":1418,"context_line":"                        router, networks, txn)"}],"source_content_type":"text/x-python","patch_set":29,"id":"9d60488d_d3c7205d","line":1415,"in_reply_to":"7e18bee6_8aa4b987","updated":"2023-05-16 05:09:19.000000000","message":"Thx, will check what changing that would lead to.","commit_id":"7cd912de3a4e12980fd3625d92f5475a0f26f1cf"},{"author":{"_account_id":13686,"name":"Frode Nordahl","email":"fnordahl@ubuntu.com","username":"fnordahl"},"change_message_id":"0ff0d99a9aaf14c05c77a910195b971ddb0f0591","unresolved":false,"context_lines":[{"line_number":1412,"context_line":"            if add_external_gateway:"},{"line_number":1413,"context_line":"                networks \u003d self._get_v4_network_of_all_router_ports("},{"line_number":1414,"context_line":"                    context, router[\u0027id\u0027])"},{"line_number":1415,"context_line":"                if self._get_router_gw_ports("},{"line_number":1416,"context_line":"                        context, router[\u0027id\u0027]) and networks is not None:"},{"line_number":1417,"context_line":"                    added_gw_ports \u003d self._add_router_ext_gw("},{"line_number":1418,"context_line":"                        router, networks, txn)"}],"source_content_type":"text/x-python","patch_set":29,"id":"621d0c8a_798d64bd","line":1415,"in_reply_to":"9d60488d_d3c7205d","updated":"2023-05-16 07:18:35.000000000","message":"Done","commit_id":"7cd912de3a4e12980fd3625d92f5475a0f26f1cf"},{"author":{"_account_id":24824,"name":"Dmitrii Shcherbakov","username":"dmitriis"},"change_message_id":"78de39b8f0a139ab45f9fead1d485aec679db64d","unresolved":true,"context_lines":[{"line_number":1725,"context_line":"            else:"},{"line_number":1726,"context_line":"                self._create_lrouter_port(context, router, port, txn\u003dtxn)"},{"line_number":1727,"context_line":""},{"line_number":1728,"context_line":"            gw_ports \u003d self._get_router_gw_ports(context, router_id)"},{"line_number":1729,"context_line":"            if gw_ports:"},{"line_number":1730,"context_line":"                cidr \u003d None"},{"line_number":1731,"context_line":"                for fixed_ip in port[\u0027fixed_ips\u0027]:"}],"source_content_type":"text/x-python","patch_set":29,"id":"f1fa553f_50e0c9a5","line":1728,"updated":"2023-05-16 00:44:43.000000000","message":"Same issue here: need to retrieve the gw ports from the request instead.\n\nThe test_reuse_ip_address_with_other_fip_on_other_router failure is a result of that: the way it fails is that a gateway port stays in the DOWN state until a port update (4 minutes later) which changes it into the ACTIVE state. But the test obviously does not wait for that to happen and proceeds to create FIPs and trying to reach them which isn\u0027t possible without the GW port LRP state.","commit_id":"7cd912de3a4e12980fd3625d92f5475a0f26f1cf"},{"author":{"_account_id":13686,"name":"Frode Nordahl","email":"fnordahl@ubuntu.com","username":"fnordahl"},"change_message_id":"0ff0d99a9aaf14c05c77a910195b971ddb0f0591","unresolved":false,"context_lines":[{"line_number":1725,"context_line":"            else:"},{"line_number":1726,"context_line":"                self._create_lrouter_port(context, router, port, txn\u003dtxn)"},{"line_number":1727,"context_line":""},{"line_number":1728,"context_line":"            gw_ports \u003d self._get_router_gw_ports(context, router_id)"},{"line_number":1729,"context_line":"            if gw_ports:"},{"line_number":1730,"context_line":"                cidr \u003d None"},{"line_number":1731,"context_line":"                for fixed_ip in port[\u0027fixed_ips\u0027]:"}],"source_content_type":"text/x-python","patch_set":29,"id":"a84fcd87_5ca9ca3b","line":1728,"in_reply_to":"07d369cd_1331be61","updated":"2023-05-16 07:18:35.000000000","message":"(Mark as resolved)","commit_id":"7cd912de3a4e12980fd3625d92f5475a0f26f1cf"},{"author":{"_account_id":13686,"name":"Frode Nordahl","email":"fnordahl@ubuntu.com","username":"fnordahl"},"change_message_id":"6a3a90a468ab5fdc8ef638e1949a4d410006684c","unresolved":true,"context_lines":[{"line_number":1725,"context_line":"            else:"},{"line_number":1726,"context_line":"                self._create_lrouter_port(context, router, port, txn\u003dtxn)"},{"line_number":1727,"context_line":""},{"line_number":1728,"context_line":"            gw_ports \u003d self._get_router_gw_ports(context, router_id)"},{"line_number":1729,"context_line":"            if gw_ports:"},{"line_number":1730,"context_line":"                cidr \u003d None"},{"line_number":1731,"context_line":"                for fixed_ip in port[\u0027fixed_ips\u0027]:"}],"source_content_type":"text/x-python","patch_set":29,"id":"07d369cd_1331be61","line":1728,"in_reply_to":"f1fa553f_50e0c9a5","updated":"2023-05-16 05:09:19.000000000","message":"Hum, at this point in the code the request is not available, the router variable in this functions come from the DB, see line 1714, and it also worked that way before the change.","commit_id":"7cd912de3a4e12980fd3625d92f5475a0f26f1cf"},{"author":{"_account_id":24824,"name":"Dmitrii Shcherbakov","username":"dmitriis"},"change_message_id":"78de39b8f0a139ab45f9fead1d485aec679db64d","unresolved":true,"context_lines":[{"line_number":1740,"context_line":""},{"line_number":1741,"context_line":"                if ovn_conf.is_ovn_emit_need_to_frag_enabled():"},{"line_number":1742,"context_line":"                    for gw_port in gw_ports:"},{"line_number":1743,"context_line":"                        provider_net \u003d self._plugin.get_network("},{"line_number":1744,"context_line":"                            context, gw_port[\u0027network_id\u0027])"},{"line_number":1745,"context_line":"                        self.set_gateway_mtu(context, provider_net)"},{"line_number":1746,"context_line":""}],"source_content_type":"text/x-python","patch_set":29,"id":"70743035_c6c88d88","line":1743,"updated":"2023-05-16 00:44:43.000000000","message":"Same issue here: need to retrieve the gw ports from the request instead.","commit_id":"7cd912de3a4e12980fd3625d92f5475a0f26f1cf"},{"author":{"_account_id":13686,"name":"Frode Nordahl","email":"fnordahl@ubuntu.com","username":"fnordahl"},"change_message_id":"0ff0d99a9aaf14c05c77a910195b971ddb0f0591","unresolved":false,"context_lines":[{"line_number":1740,"context_line":""},{"line_number":1741,"context_line":"                if ovn_conf.is_ovn_emit_need_to_frag_enabled():"},{"line_number":1742,"context_line":"                    for gw_port in gw_ports:"},{"line_number":1743,"context_line":"                        provider_net \u003d self._plugin.get_network("},{"line_number":1744,"context_line":"                            context, gw_port[\u0027network_id\u0027])"},{"line_number":1745,"context_line":"                        self.set_gateway_mtu(context, provider_net)"},{"line_number":1746,"context_line":""}],"source_content_type":"text/x-python","patch_set":29,"id":"49e22099_e0e4a516","line":1743,"in_reply_to":"61609802_92f53bbe","updated":"2023-05-16 07:18:35.000000000","message":"(Mark as resolved)","commit_id":"7cd912de3a4e12980fd3625d92f5475a0f26f1cf"},{"author":{"_account_id":13686,"name":"Frode Nordahl","email":"fnordahl@ubuntu.com","username":"fnordahl"},"change_message_id":"6a3a90a468ab5fdc8ef638e1949a4d410006684c","unresolved":true,"context_lines":[{"line_number":1740,"context_line":""},{"line_number":1741,"context_line":"                if ovn_conf.is_ovn_emit_need_to_frag_enabled():"},{"line_number":1742,"context_line":"                    for gw_port in gw_ports:"},{"line_number":1743,"context_line":"                        provider_net \u003d self._plugin.get_network("},{"line_number":1744,"context_line":"                            context, gw_port[\u0027network_id\u0027])"},{"line_number":1745,"context_line":"                        self.set_gateway_mtu(context, provider_net)"},{"line_number":1746,"context_line":""}],"source_content_type":"text/x-python","patch_set":29,"id":"61609802_92f53bbe","line":1743,"in_reply_to":"70743035_c6c88d88","updated":"2023-05-16 05:09:19.000000000","message":"The request is not available in this function, and it was not available before the change either.","commit_id":"7cd912de3a4e12980fd3625d92f5475a0f26f1cf"},{"author":{"_account_id":24824,"name":"Dmitrii Shcherbakov","username":"dmitriis"},"change_message_id":"78de39b8f0a139ab45f9fead1d485aec679db64d","unresolved":true,"context_lines":[{"line_number":1855,"context_line":"            if router_id:"},{"line_number":1856,"context_line":"                try:"},{"line_number":1857,"context_line":"                    router \u003d self._l3_plugin.get_router(context, router_id)"},{"line_number":1858,"context_line":"                    gw_ports \u003d self._get_router_gw_ports(context, router_id)"},{"line_number":1859,"context_line":"                except l3_exc.RouterNotFound:"},{"line_number":1860,"context_line":"                    # If the router is gone, the router port is also gone"},{"line_number":1861,"context_line":"                    port_removed \u003d True"}],"source_content_type":"text/x-python","patch_set":29,"id":"20a121c4_21831fa9","line":1858,"updated":"2023-05-16 00:44:43.000000000","message":"Same issue here: need to retrieve the gw ports from the request instead.","commit_id":"7cd912de3a4e12980fd3625d92f5475a0f26f1cf"},{"author":{"_account_id":13686,"name":"Frode Nordahl","email":"fnordahl@ubuntu.com","username":"fnordahl"},"change_message_id":"6a3a90a468ab5fdc8ef638e1949a4d410006684c","unresolved":true,"context_lines":[{"line_number":1855,"context_line":"            if router_id:"},{"line_number":1856,"context_line":"                try:"},{"line_number":1857,"context_line":"                    router \u003d self._l3_plugin.get_router(context, router_id)"},{"line_number":1858,"context_line":"                    gw_ports \u003d self._get_router_gw_ports(context, router_id)"},{"line_number":1859,"context_line":"                except l3_exc.RouterNotFound:"},{"line_number":1860,"context_line":"                    # If the router is gone, the router port is also gone"},{"line_number":1861,"context_line":"                    port_removed \u003d True"}],"source_content_type":"text/x-python","patch_set":29,"id":"41257181_bda684b8","line":1858,"in_reply_to":"20a121c4_21831fa9","updated":"2023-05-16 05:09:19.000000000","message":"The request is not available in this function, and it was not available before the change either.","commit_id":"7cd912de3a4e12980fd3625d92f5475a0f26f1cf"},{"author":{"_account_id":13686,"name":"Frode Nordahl","email":"fnordahl@ubuntu.com","username":"fnordahl"},"change_message_id":"0ff0d99a9aaf14c05c77a910195b971ddb0f0591","unresolved":false,"context_lines":[{"line_number":1855,"context_line":"            if router_id:"},{"line_number":1856,"context_line":"                try:"},{"line_number":1857,"context_line":"                    router \u003d self._l3_plugin.get_router(context, router_id)"},{"line_number":1858,"context_line":"                    gw_ports \u003d self._get_router_gw_ports(context, router_id)"},{"line_number":1859,"context_line":"                except l3_exc.RouterNotFound:"},{"line_number":1860,"context_line":"                    # If the router is gone, the router port is also gone"},{"line_number":1861,"context_line":"                    port_removed \u003d True"}],"source_content_type":"text/x-python","patch_set":29,"id":"cd7d7bb1_275f51d5","line":1858,"in_reply_to":"41257181_bda684b8","updated":"2023-05-16 07:18:35.000000000","message":"(Mark as resolved)","commit_id":"7cd912de3a4e12980fd3625d92f5475a0f26f1cf"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"dd2abe8853f9438e4d973fae2d8191d0599d1609","unresolved":true,"context_lines":[{"line_number":1139,"context_line":""},{"line_number":1140,"context_line":"        # 2. Add default route with nexthop as gateway ip"},{"line_number":1141,"context_line":"        lrouter_name \u003d utils.ovn_name(router[\u0027id\u0027])"},{"line_number":1142,"context_line":"        for gw_port in self._get_router_gw_ports(admin_context, router[\u0027id\u0027]):"},{"line_number":1143,"context_line":"            for gw_info in self._get_gw_info(admin_context, gw_port):"},{"line_number":1144,"context_line":"                if gw_info.gateway_ip is None:"},{"line_number":1145,"context_line":"                    continue"}],"source_content_type":"text/x-python","patch_set":37,"id":"5648d651_1fea562d","line":1142,"updated":"2023-06-14 20:36:57.000000000","message":"Just curious if you have to make the same call as above, which will call plugin.get_ports() again?","commit_id":"ee1ecad6b709b52f10f945063f8faac32e27f757"},{"author":{"_account_id":13686,"name":"Frode Nordahl","email":"fnordahl@ubuntu.com","username":"fnordahl"},"change_message_id":"1b84284dfabf908c3e561f2f600d295df156902b","unresolved":false,"context_lines":[{"line_number":1139,"context_line":""},{"line_number":1140,"context_line":"        # 2. Add default route with nexthop as gateway ip"},{"line_number":1141,"context_line":"        lrouter_name \u003d utils.ovn_name(router[\u0027id\u0027])"},{"line_number":1142,"context_line":"        for gw_port in self._get_router_gw_ports(admin_context, router[\u0027id\u0027]):"},{"line_number":1143,"context_line":"            for gw_info in self._get_gw_info(admin_context, gw_port):"},{"line_number":1144,"context_line":"                if gw_info.gateway_ip is None:"},{"line_number":1145,"context_line":"                    continue"}],"source_content_type":"text/x-python","patch_set":37,"id":"a9787821_6d5939b8","line":1142,"in_reply_to":"5648d651_1fea562d","updated":"2023-06-19 14:22:19.000000000","message":"We could indeed omit this and move the below calls into the for loop started above. Thank you for pointing it out!","commit_id":"ee1ecad6b709b52f10f945063f8faac32e27f757"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"dd2abe8853f9438e4d973fae2d8191d0599d1609","unresolved":true,"context_lines":[{"line_number":1715,"context_line":"                router_id \u003d port.get(\u0027device_id\u0027)"},{"line_number":1716,"context_line":""},{"line_number":1717,"context_line":"            router \u003d None"},{"line_number":1718,"context_line":"            gw_ports \u003d None"},{"line_number":1719,"context_line":"            if router_id:"},{"line_number":1720,"context_line":"                try:"},{"line_number":1721,"context_line":"                    router \u003d self._l3_plugin.get_router(context, router_id)"}],"source_content_type":"text/x-python","patch_set":37,"id":"506ea7b1_152bba0c","line":1718,"updated":"2023-06-14 20:36:57.000000000","message":"nit: change to [] and don\u0027t need check below","commit_id":"ee1ecad6b709b52f10f945063f8faac32e27f757"},{"author":{"_account_id":13686,"name":"Frode Nordahl","email":"fnordahl@ubuntu.com","username":"fnordahl"},"change_message_id":"1b84284dfabf908c3e561f2f600d295df156902b","unresolved":false,"context_lines":[{"line_number":1715,"context_line":"                router_id \u003d port.get(\u0027device_id\u0027)"},{"line_number":1716,"context_line":""},{"line_number":1717,"context_line":"            router \u003d None"},{"line_number":1718,"context_line":"            gw_ports \u003d None"},{"line_number":1719,"context_line":"            if router_id:"},{"line_number":1720,"context_line":"                try:"},{"line_number":1721,"context_line":"                    router \u003d self._l3_plugin.get_router(context, router_id)"}],"source_content_type":"text/x-python","patch_set":37,"id":"63b65431_59604877","line":1718,"in_reply_to":"506ea7b1_152bba0c","updated":"2023-06-19 14:22:19.000000000","message":"Thx, will check","commit_id":"ee1ecad6b709b52f10f945063f8faac32e27f757"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"dd2abe8853f9438e4d973fae2d8191d0599d1609","unresolved":true,"context_lines":[{"line_number":1739,"context_line":"            elif port:"},{"line_number":1740,"context_line":"                subnet_ids \u003d utils.get_port_subnet_ids(port)"},{"line_number":1741,"context_line":""},{"line_number":1742,"context_line":"            if ovn_conf.is_ovn_emit_need_to_frag_enabled() and gw_ports:"},{"line_number":1743,"context_line":"                for gw_port in gw_ports:"},{"line_number":1744,"context_line":"                    provider_net \u003d self._plugin.get_network("},{"line_number":1745,"context_line":"                        context, gw_port[\u0027network_id\u0027])"}],"source_content_type":"text/x-python","patch_set":37,"id":"afe5f6f8_e67f4b50","line":1742,"range":{"start_line":1742,"start_character":59,"end_line":1742,"end_character":71},"updated":"2023-06-14 20:36:57.000000000","message":"from above, if [] the for() loop does nothing so wouldn\u0027t need check here","commit_id":"ee1ecad6b709b52f10f945063f8faac32e27f757"},{"author":{"_account_id":13686,"name":"Frode Nordahl","email":"fnordahl@ubuntu.com","username":"fnordahl"},"change_message_id":"1b84284dfabf908c3e561f2f600d295df156902b","unresolved":false,"context_lines":[{"line_number":1739,"context_line":"            elif port:"},{"line_number":1740,"context_line":"                subnet_ids \u003d utils.get_port_subnet_ids(port)"},{"line_number":1741,"context_line":""},{"line_number":1742,"context_line":"            if ovn_conf.is_ovn_emit_need_to_frag_enabled() and gw_ports:"},{"line_number":1743,"context_line":"                for gw_port in gw_ports:"},{"line_number":1744,"context_line":"                    provider_net \u003d self._plugin.get_network("},{"line_number":1745,"context_line":"                        context, gw_port[\u0027network_id\u0027])"}],"source_content_type":"text/x-python","patch_set":37,"id":"f88f5040_4ea7e8da","line":1742,"range":{"start_line":1742,"start_character":59,"end_line":1742,"end_character":71},"in_reply_to":"afe5f6f8_e67f4b50","updated":"2023-06-19 14:22:19.000000000","message":"Assuming `self._get_router_gw_ports(context, router_id)` always returns a List, that would indeed be the case.\n\nWill check and update accordingly, thx for pointing it out!","commit_id":"ee1ecad6b709b52f10f945063f8faac32e27f757"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"12284db52ce66e3a07975356b7dfcbb1c74874bd","unresolved":true,"context_lines":[{"line_number":1098,"context_line":"        network_id \u003d port_dict.get(\u0027network_id\u0027)"},{"line_number":1099,"context_line":"        for fixed_ip in port_dict.get(\u0027fixed_ips\u0027):"},{"line_number":1100,"context_line":"            subnet_id \u003d fixed_ip.get(\u0027subnet_id\u0027)"},{"line_number":1101,"context_line":"            subnet \u003d self._plugin.get_subnet(context, subnet_id)"},{"line_number":1102,"context_line":"            ip_version \u003d subnet.get(\u0027ip_version\u0027)"},{"line_number":1103,"context_line":"            gateways_info.append(GW_INFO("},{"line_number":1104,"context_line":"                network_id, subnet_id, fixed_ip.get(\u0027ip_address\u0027),"}],"source_content_type":"text/x-python","patch_set":41,"id":"f120efc2_7fc0bbe5","line":1101,"range":{"start_line":1101,"start_character":12,"end_line":1101,"end_character":64},"updated":"2023-07-04 13:08:08.000000000","message":"I know this is how it was implemented in base PS. But this is inefficient. Collect the subnet_ids and request all off them in one call.","commit_id":"6a0fd5b40ffa0e605632ea5b6f5d4a24c125af84"},{"author":{"_account_id":13686,"name":"Frode Nordahl","email":"fnordahl@ubuntu.com","username":"fnordahl"},"change_message_id":"99a137e7deffbcf062851188a7b8c70fc3877a13","unresolved":false,"context_lines":[{"line_number":1098,"context_line":"        network_id \u003d port_dict.get(\u0027network_id\u0027)"},{"line_number":1099,"context_line":"        for fixed_ip in port_dict.get(\u0027fixed_ips\u0027):"},{"line_number":1100,"context_line":"            subnet_id \u003d fixed_ip.get(\u0027subnet_id\u0027)"},{"line_number":1101,"context_line":"            subnet \u003d self._plugin.get_subnet(context, subnet_id)"},{"line_number":1102,"context_line":"            ip_version \u003d subnet.get(\u0027ip_version\u0027)"},{"line_number":1103,"context_line":"            gateways_info.append(GW_INFO("},{"line_number":1104,"context_line":"                network_id, subnet_id, fixed_ip.get(\u0027ip_address\u0027),"}],"source_content_type":"text/x-python","patch_set":41,"id":"40237a8c_6b94337d","line":1101,"range":{"start_line":1101,"start_character":12,"end_line":1101,"end_character":64},"in_reply_to":"f120efc2_7fc0bbe5","updated":"2023-07-06 12:13:42.000000000","message":"Ack, done.","commit_id":"6a0fd5b40ffa0e605632ea5b6f5d4a24c125af84"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"12284db52ce66e3a07975356b7dfcbb1c74874bd","unresolved":true,"context_lines":[{"line_number":1208,"context_line":"                lrp \u003d self._nb_idl.get_lrouter_port("},{"line_number":1209,"context_line":"                    utils.ovn_lrouter_port_name(gw_port[\u0027id\u0027]))"},{"line_number":1210,"context_line":"                if not lrp:"},{"line_number":1211,"context_line":"                    return True"},{"line_number":1212,"context_line":"                lrp_ext_ids \u003d getattr(lrp, \u0027external_ids\u0027, {})"},{"line_number":1213,"context_line":"                if (ovn_const.OVN_NETWORK_NAME_EXT_ID_KEY in lrp_ext_ids and"},{"line_number":1214,"context_line":"                        lrp_ext_ids[ovn_const.OVN_NETWORK_NAME_EXT_ID_KEY] !\u003d ("}],"source_content_type":"text/x-python","patch_set":41,"id":"c6acbafc_eeac380c","line":1211,"range":{"start_line":1211,"start_character":20,"end_line":1211,"end_character":31},"updated":"2023-07-04 13:08:08.000000000","message":"Sorry, I don\u0027t understand this check: if there is no GW info and there is no LRP, why this returns True?","commit_id":"6a0fd5b40ffa0e605632ea5b6f5d4a24c125af84"},{"author":{"_account_id":13686,"name":"Frode Nordahl","email":"fnordahl@ubuntu.com","username":"fnordahl"},"change_message_id":"99a137e7deffbcf062851188a7b8c70fc3877a13","unresolved":false,"context_lines":[{"line_number":1208,"context_line":"                lrp \u003d self._nb_idl.get_lrouter_port("},{"line_number":1209,"context_line":"                    utils.ovn_lrouter_port_name(gw_port[\u0027id\u0027]))"},{"line_number":1210,"context_line":"                if not lrp:"},{"line_number":1211,"context_line":"                    return True"},{"line_number":1212,"context_line":"                lrp_ext_ids \u003d getattr(lrp, \u0027external_ids\u0027, {})"},{"line_number":1213,"context_line":"                if (ovn_const.OVN_NETWORK_NAME_EXT_ID_KEY in lrp_ext_ids and"},{"line_number":1214,"context_line":"                        lrp_ext_ids[ovn_const.OVN_NETWORK_NAME_EXT_ID_KEY] !\u003d ("}],"source_content_type":"text/x-python","patch_set":41,"id":"a1ac3be3_9fbb941f","line":1211,"range":{"start_line":1211,"start_character":20,"end_line":1211,"end_character":31},"in_reply_to":"c6acbafc_eeac380c","updated":"2023-07-06 12:13:42.000000000","message":"Thank you for pointing that out, I most likely meant to use `continue` here. Done.","commit_id":"6a0fd5b40ffa0e605632ea5b6f5d4a24c125af84"}],"neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_db_sync.py":[{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"dd2abe8853f9438e4d973fae2d8191d0599d1609","unresolved":true,"context_lines":[{"line_number":472,"context_line":"                continue"},{"line_number":473,"context_line":"            gateways \u003d self._ovn_client._get_gw_info(ctx, router)"},{"line_number":474,"context_line":"            for gw_info in gateways:"},{"line_number":475,"context_line":"                prefix \u003d (constants.IPv4_ANY if"},{"line_number":476,"context_line":"                          gw_info.ip_version \u003d\u003d constants.IP_VERSION_4 else"},{"line_number":477,"context_line":"                          constants.IPv6_ANY)"},{"line_number":478,"context_line":"                if gw_info.gateway_ip:"},{"line_number":479,"context_line":"                    db_extends[router[\u0027id\u0027]][\u0027routes\u0027].append("},{"line_number":480,"context_line":"                        {\u0027destination\u0027: prefix,"}],"source_content_type":"text/x-python","patch_set":37,"id":"232cc8ab_25fe0180","side":"PARENT","line":477,"range":{"start_line":475,"start_character":16,"end_line":477,"end_character":45},"updated":"2023-06-14 20:36:57.000000000","message":"So was gw_info.ip_prefix already correct in that it would be the \"any\" address?","commit_id":"55e7e7ab6978b0736add549e4cb4b482d9d58451"},{"author":{"_account_id":13686,"name":"Frode Nordahl","email":"fnordahl@ubuntu.com","username":"fnordahl"},"change_message_id":"4c7169b04f523e7b1d57c752575006e8925c7b2b","unresolved":false,"context_lines":[{"line_number":472,"context_line":"                continue"},{"line_number":473,"context_line":"            gateways \u003d self._ovn_client._get_gw_info(ctx, router)"},{"line_number":474,"context_line":"            for gw_info in gateways:"},{"line_number":475,"context_line":"                prefix \u003d (constants.IPv4_ANY if"},{"line_number":476,"context_line":"                          gw_info.ip_version \u003d\u003d constants.IP_VERSION_4 else"},{"line_number":477,"context_line":"                          constants.IPv6_ANY)"},{"line_number":478,"context_line":"                if gw_info.gateway_ip:"},{"line_number":479,"context_line":"                    db_extends[router[\u0027id\u0027]][\u0027routes\u0027].append("},{"line_number":480,"context_line":"                        {\u0027destination\u0027: prefix,"}],"source_content_type":"text/x-python","patch_set":37,"id":"000e8c47_abb87886","side":"PARENT","line":477,"range":{"start_line":475,"start_character":16,"end_line":477,"end_character":45},"in_reply_to":"04ec258b_ab5675a1","updated":"2023-06-21 08:19:40.000000000","message":"Ack, thx for checking!","commit_id":"55e7e7ab6978b0736add549e4cb4b482d9d58451"},{"author":{"_account_id":13686,"name":"Frode Nordahl","email":"fnordahl@ubuntu.com","username":"fnordahl"},"change_message_id":"1b84284dfabf908c3e561f2f600d295df156902b","unresolved":true,"context_lines":[{"line_number":472,"context_line":"                continue"},{"line_number":473,"context_line":"            gateways \u003d self._ovn_client._get_gw_info(ctx, router)"},{"line_number":474,"context_line":"            for gw_info in gateways:"},{"line_number":475,"context_line":"                prefix \u003d (constants.IPv4_ANY if"},{"line_number":476,"context_line":"                          gw_info.ip_version \u003d\u003d constants.IP_VERSION_4 else"},{"line_number":477,"context_line":"                          constants.IPv6_ANY)"},{"line_number":478,"context_line":"                if gw_info.gateway_ip:"},{"line_number":479,"context_line":"                    db_extends[router[\u0027id\u0027]][\u0027routes\u0027].append("},{"line_number":480,"context_line":"                        {\u0027destination\u0027: prefix,"}],"source_content_type":"text/x-python","patch_set":37,"id":"87a0e4ef_19b578df","side":"PARENT","line":477,"range":{"start_line":475,"start_character":16,"end_line":477,"end_character":45},"in_reply_to":"232cc8ab_25fe0180","updated":"2023-06-19 14:22:19.000000000","message":"For this part of the sync code, I believe it is dealing with default routes, so the existing code was strictly speaking correct.\n\nHowever, since it also did get, and continues to get this and other information from the `self._ovn_client._get_gw_info` method, I see no need to have the same logic multiple places.\n\nIt can only lead to bugs somewhere down the line. Or am I missing something?","commit_id":"55e7e7ab6978b0736add549e4cb4b482d9d58451"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"5b1e4f0182080c42bdb93e14551f869abc62ec01","unresolved":true,"context_lines":[{"line_number":472,"context_line":"                continue"},{"line_number":473,"context_line":"            gateways \u003d self._ovn_client._get_gw_info(ctx, router)"},{"line_number":474,"context_line":"            for gw_info in gateways:"},{"line_number":475,"context_line":"                prefix \u003d (constants.IPv4_ANY if"},{"line_number":476,"context_line":"                          gw_info.ip_version \u003d\u003d constants.IP_VERSION_4 else"},{"line_number":477,"context_line":"                          constants.IPv6_ANY)"},{"line_number":478,"context_line":"                if gw_info.gateway_ip:"},{"line_number":479,"context_line":"                    db_extends[router[\u0027id\u0027]][\u0027routes\u0027].append("},{"line_number":480,"context_line":"                        {\u0027destination\u0027: prefix,"}],"source_content_type":"text/x-python","patch_set":37,"id":"04ec258b_ab5675a1","side":"PARENT","line":477,"range":{"start_line":475,"start_character":16,"end_line":477,"end_character":45},"in_reply_to":"87a0e4ef_19b578df","updated":"2023-06-20 14:07:42.000000000","message":"Hi Frode, I don\u0027t think you\u0027re missing anything. It seems to all work the same I was just curious why the code never used ip_prefix previously and just set the default route. But now there there could be more routes here, with different gateways, etc. Sometimes I just notice these changes and want to make sense of it so we don\u0027t introduce a regression.","commit_id":"55e7e7ab6978b0736add549e4cb4b482d9d58451"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"12284db52ce66e3a07975356b7dfcbb1c74874bd","unresolved":true,"context_lines":[{"line_number":478,"context_line":"                                 ovn_const.OVN_ROUTER_IS_EXT_GW: \u0027true\u0027,"},{"line_number":479,"context_line":"                                 ovn_const.OVN_SUBNET_EXT_ID_KEY:"},{"line_number":480,"context_line":"                                 gw_info.subnet_id}})"},{"line_number":481,"context_line":"                    if gw_info.ip_version \u003d\u003d constants.IP_VERSION_6:"},{"line_number":482,"context_line":"                        continue"},{"line_number":483,"context_line":"                    if gw_info.router_ip and utils.is_snat_enabled(router):"},{"line_number":484,"context_line":"                        networks \u003d self._ovn_client.\\"},{"line_number":485,"context_line":"                            _get_v4_network_of_all_router_ports("}],"source_content_type":"text/x-python","patch_set":41,"id":"24a67023_14ecd31b","line":482,"range":{"start_line":481,"start_character":20,"end_line":482,"end_character":32},"updated":"2023-07-04 13:08:08.000000000","message":"nit: this check should be done first (L473)","commit_id":"6a0fd5b40ffa0e605632ea5b6f5d4a24c125af84"},{"author":{"_account_id":13686,"name":"Frode Nordahl","email":"fnordahl@ubuntu.com","username":"fnordahl"},"change_message_id":"99a137e7deffbcf062851188a7b8c70fc3877a13","unresolved":true,"context_lines":[{"line_number":478,"context_line":"                                 ovn_const.OVN_ROUTER_IS_EXT_GW: \u0027true\u0027,"},{"line_number":479,"context_line":"                                 ovn_const.OVN_SUBNET_EXT_ID_KEY:"},{"line_number":480,"context_line":"                                 gw_info.subnet_id}})"},{"line_number":481,"context_line":"                    if gw_info.ip_version \u003d\u003d constants.IP_VERSION_6:"},{"line_number":482,"context_line":"                        continue"},{"line_number":483,"context_line":"                    if gw_info.router_ip and utils.is_snat_enabled(router):"},{"line_number":484,"context_line":"                        networks \u003d self._ovn_client.\\"},{"line_number":485,"context_line":"                            _get_v4_network_of_all_router_ports("}],"source_content_type":"text/x-python","patch_set":41,"id":"40ab42e3_ddf2aca9","line":482,"range":{"start_line":481,"start_character":20,"end_line":482,"end_character":32},"in_reply_to":"24a67023_14ecd31b","updated":"2023-07-06 12:13:42.000000000","message":"Would that not change the behavior?\n\nAs far as I understand this, the check is there because the processing of NAT records below is not relevant if the gw info we recorded above is for IPv6 gateway","commit_id":"6a0fd5b40ffa0e605632ea5b6f5d4a24c125af84"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"d33bf97c10d5148118c0de45f99401ca568101ec","unresolved":true,"context_lines":[{"line_number":478,"context_line":"                                 ovn_const.OVN_ROUTER_IS_EXT_GW: \u0027true\u0027,"},{"line_number":479,"context_line":"                                 ovn_const.OVN_SUBNET_EXT_ID_KEY:"},{"line_number":480,"context_line":"                                 gw_info.subnet_id}})"},{"line_number":481,"context_line":"                    if gw_info.ip_version \u003d\u003d constants.IP_VERSION_6:"},{"line_number":482,"context_line":"                        continue"},{"line_number":483,"context_line":"                    if gw_info.router_ip and utils.is_snat_enabled(router):"},{"line_number":484,"context_line":"                        networks \u003d self._ovn_client.\\"},{"line_number":485,"context_line":"                            _get_v4_network_of_all_router_ports("}],"source_content_type":"text/x-python","patch_set":41,"id":"3e9a95d3_bd391bf5","line":482,"range":{"start_line":481,"start_character":20,"end_line":482,"end_character":32},"in_reply_to":"40ab42e3_ddf2aca9","updated":"2023-08-01 20:52:47.000000000","message":"Yes, it seems like it would change the current behavior.","commit_id":"6a0fd5b40ffa0e605632ea5b6f5d4a24c125af84"}],"neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovn_client.py":[{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"12284db52ce66e3a07975356b7dfcbb1c74874bd","unresolved":true,"context_lines":[{"line_number":114,"context_line":"            self.ovn_client._add_router_ext_gw(mock.Mock(), router, networks,"},{"line_number":115,"context_line":"                                               txn))"},{"line_number":116,"context_line":"        self.nb_idl.add_static_route.assert_not_called()"},{"line_number":117,"context_line":""},{"line_number":118,"context_line":"    def test_update_lsp_host_info_up(self):"},{"line_number":119,"context_line":"        context \u003d mock.MagicMock()"},{"line_number":120,"context_line":"        host_id \u003d \u0027fake-binding-host-id\u0027"}],"source_content_type":"text/x-python","patch_set":41,"id":"b4d520f8_dc512d00","line":117,"updated":"2023-07-04 13:08:08.000000000","message":"\"_check_external_ips_changed\" method is not tested","commit_id":"6a0fd5b40ffa0e605632ea5b6f5d4a24c125af84"},{"author":{"_account_id":13686,"name":"Frode Nordahl","email":"fnordahl@ubuntu.com","username":"fnordahl"},"change_message_id":"66e3d68b5bbb710f920dbc17f4bcb3b35a29e4aa","unresolved":true,"context_lines":[{"line_number":114,"context_line":"            self.ovn_client._add_router_ext_gw(mock.Mock(), router, networks,"},{"line_number":115,"context_line":"                                               txn))"},{"line_number":116,"context_line":"        self.nb_idl.add_static_route.assert_not_called()"},{"line_number":117,"context_line":""},{"line_number":118,"context_line":"    def test_update_lsp_host_info_up(self):"},{"line_number":119,"context_line":"        context \u003d mock.MagicMock()"},{"line_number":120,"context_line":"        host_id \u003d \u0027fake-binding-host-id\u0027"}],"source_content_type":"text/x-python","patch_set":41,"id":"87087a13_a44cab36","line":117,"in_reply_to":"2c032738_66540a29","updated":"2023-10-10 09:43:06.000000000","message":"fwiw; I have started the work on tempest tests for this feature. I uncovered a few bugs in the already committed code in the process. Yay for tests!\n\nI will put up one review with tests for the API part shortly, and I\u0027ll make an attempt at a scenario test after that.\n\nAwaiting rechecks for the two bottom approved reviews to avoid messing with them when pushing a rebased set.","commit_id":"6a0fd5b40ffa0e605632ea5b6f5d4a24c125af84"},{"author":{"_account_id":13686,"name":"Frode Nordahl","email":"fnordahl@ubuntu.com","username":"fnordahl"},"change_message_id":"ecb78143d694d93380a998aac5f3e3b131f30e35","unresolved":true,"context_lines":[{"line_number":114,"context_line":"            self.ovn_client._add_router_ext_gw(mock.Mock(), router, networks,"},{"line_number":115,"context_line":"                                               txn))"},{"line_number":116,"context_line":"        self.nb_idl.add_static_route.assert_not_called()"},{"line_number":117,"context_line":""},{"line_number":118,"context_line":"    def test_update_lsp_host_info_up(self):"},{"line_number":119,"context_line":"        context \u003d mock.MagicMock()"},{"line_number":120,"context_line":"        host_id \u003d \u0027fake-binding-host-id\u0027"}],"source_content_type":"text/x-python","patch_set":41,"id":"2c032738_66540a29","line":117,"in_reply_to":"3b8ab446_aa95a62b","updated":"2023-08-31 12:49:40.000000000","message":"Scenario tests would indeed be good to have, does a harness for this exist, or are you asking us to create one?\n\nI see that the Neutron fullstack tests appear to be specific to ML2/OVS and the harness appear quite entangled with it. I see no ML2/OVN fullstack tests, is this observation correct?\n\nThere appears to exist higher level tests in tempest that make use of external fixtures such as the neutron dynamic routing test, so I guess we could follow that model as an approach to this?","commit_id":"6a0fd5b40ffa0e605632ea5b6f5d4a24c125af84"},{"author":{"_account_id":8313,"name":"Lajos Katona","display_name":"lajoskatona","email":"katonalala@gmail.com","username":"elajkat","status":"Ericsson Software Technology"},"change_message_id":"bdf8b66f3cef35ab9df5ad6254e6301483d1bee7","unresolved":true,"context_lines":[{"line_number":114,"context_line":"            self.ovn_client._add_router_ext_gw(mock.Mock(), router, networks,"},{"line_number":115,"context_line":"                                               txn))"},{"line_number":116,"context_line":"        self.nb_idl.add_static_route.assert_not_called()"},{"line_number":117,"context_line":""},{"line_number":118,"context_line":"    def test_update_lsp_host_info_up(self):"},{"line_number":119,"context_line":"        context \u003d mock.MagicMock()"},{"line_number":120,"context_line":"        host_id \u003d \u0027fake-binding-host-id\u0027"}],"source_content_type":"text/x-python","patch_set":41,"id":"3b8ab446_aa95a62b","line":117,"in_reply_to":"765c17c5_8e2e7883","updated":"2023-08-24 07:09:28.000000000","message":"Slightly related but do you have higher level tests also? at least some tempest API tests would be good to have but perhaps some scenario should also work in upstream CI.","commit_id":"6a0fd5b40ffa0e605632ea5b6f5d4a24c125af84"},{"author":{"_account_id":13686,"name":"Frode Nordahl","email":"fnordahl@ubuntu.com","username":"fnordahl"},"change_message_id":"9a276e8568604328513407fb4547d50cdaf2245e","unresolved":true,"context_lines":[{"line_number":114,"context_line":"            self.ovn_client._add_router_ext_gw(mock.Mock(), router, networks,"},{"line_number":115,"context_line":"                                               txn))"},{"line_number":116,"context_line":"        self.nb_idl.add_static_route.assert_not_called()"},{"line_number":117,"context_line":""},{"line_number":118,"context_line":"    def test_update_lsp_host_info_up(self):"},{"line_number":119,"context_line":"        context \u003d mock.MagicMock()"},{"line_number":120,"context_line":"        host_id \u003d \u0027fake-binding-host-id\u0027"}],"source_content_type":"text/x-python","patch_set":41,"id":"eb08fd50_50a90b91","line":117,"in_reply_to":"87087a13_a44cab36","updated":"2023-10-19 11:34:33.000000000","message":"Both API and Scenario tempest tests are now up:\nhttps://review.opendev.org/c/openstack/neutron-tempest-plugin/+/897823/\nhttps://review.opendev.org/c/openstack/neutron-tempest-plugin/+/898810/","commit_id":"6a0fd5b40ffa0e605632ea5b6f5d4a24c125af84"},{"author":{"_account_id":13686,"name":"Frode Nordahl","email":"fnordahl@ubuntu.com","username":"fnordahl"},"change_message_id":"99a137e7deffbcf062851188a7b8c70fc3877a13","unresolved":true,"context_lines":[{"line_number":114,"context_line":"            self.ovn_client._add_router_ext_gw(mock.Mock(), router, networks,"},{"line_number":115,"context_line":"                                               txn))"},{"line_number":116,"context_line":"        self.nb_idl.add_static_route.assert_not_called()"},{"line_number":117,"context_line":""},{"line_number":118,"context_line":"    def test_update_lsp_host_info_up(self):"},{"line_number":119,"context_line":"        context \u003d mock.MagicMock()"},{"line_number":120,"context_line":"        host_id \u003d \u0027fake-binding-host-id\u0027"}],"source_content_type":"text/x-python","patch_set":41,"id":"765c17c5_8e2e7883","line":117,"in_reply_to":"b4d520f8_dc512d00","updated":"2023-07-06 12:13:42.000000000","message":"The method does indeed not have a dedicated unit test in the existing code base, it is fairly well tested indirectly by other unit tests that does not mock this method out and in the functional tests though. Do you still think a dedicated unit test would be necessary?","commit_id":"6a0fd5b40ffa0e605632ea5b6f5d4a24c125af84"}],"neutron/tests/unit/services/ovn_l3/test_plugin.py":[{"author":{"_account_id":13686,"name":"Frode Nordahl","email":"fnordahl@ubuntu.com","username":"fnordahl"},"change_message_id":"d1e2111ba82e244a178c818f96cba086f40fd9ae","unresolved":true,"context_lines":[{"line_number":50,"context_line":"# TODO(mjozefcz): Find out a way to not inherit from"},{"line_number":51,"context_line":"# Ml2PluginV2TestCase."},{"line_number":52,"context_line":"class TestOVNL3RouterPlugin(test_mech_driver.Ml2PluginV2TestCase,"},{"line_number":53,"context_line":"                            obj_test_base.BaseDbObjectTestCase):"},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"    _mechanism_drivers \u003d [\u0027ovn\u0027]"},{"line_number":56,"context_line":"    l3_plugin \u003d \u0027neutron.services.ovn_l3.plugin.OVNL3RouterPlugin\u0027"}],"source_content_type":"text/x-python","patch_set":10,"id":"5471f5d6_2fe52cfb","line":53,"range":{"start_line":53,"start_character":28,"end_line":53,"end_character":62},"updated":"2023-03-22 20:31:51.000000000","message":"Drop","commit_id":"0c40f728fac2d54fad1d85c7a7d991889272c7bd"},{"author":{"_account_id":13686,"name":"Frode Nordahl","email":"fnordahl@ubuntu.com","username":"fnordahl"},"change_message_id":"d06a3722acaad211dd889ffe5938fce83700df9a","unresolved":false,"context_lines":[{"line_number":50,"context_line":"# TODO(mjozefcz): Find out a way to not inherit from"},{"line_number":51,"context_line":"# Ml2PluginV2TestCase."},{"line_number":52,"context_line":"class TestOVNL3RouterPlugin(test_mech_driver.Ml2PluginV2TestCase,"},{"line_number":53,"context_line":"                            obj_test_base.BaseDbObjectTestCase):"},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"    _mechanism_drivers \u003d [\u0027ovn\u0027]"},{"line_number":56,"context_line":"    l3_plugin \u003d \u0027neutron.services.ovn_l3.plugin.OVNL3RouterPlugin\u0027"}],"source_content_type":"text/x-python","patch_set":10,"id":"80575837_c29e249c","line":53,"range":{"start_line":53,"start_character":28,"end_line":53,"end_character":62},"in_reply_to":"5471f5d6_2fe52cfb","updated":"2023-03-24 11:55:58.000000000","message":"Done","commit_id":"0c40f728fac2d54fad1d85c7a7d991889272c7bd"}]}
