)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"db8173e97d25723ed48b5807944932c90851b454","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"16b26150_381e5a2e","updated":"2024-06-11 06:03:16.000000000","message":"looks good, just a minor comment about only adding the routes when needed. Also, perhaps worth adapting the testing side to cover this","commit_id":"b47b2d70a10b0116c81577087d6c5a1cef672cf6"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"30ce53f23f510c3f1ac12c46a152ef47bd0a87e5","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":17,"id":"d4c42c7f_81baab57","updated":"2024-06-13 06:26:35.000000000","message":"LGTM, have you try that the re-sync action works as expected too? like if you stop the agent, create the network and then restart the agent, the route for the network will be added/exposed as expected, right?","commit_id":"bc1dc8ec9ea2438fe89ad0d9535dfddd05c463d9"},{"author":{"_account_id":36670,"name":"Jay Jahns","email":"jayjahns@gmail.com","username":"jayjahns"},"change_message_id":"c129075d12f1bd5b8225022dbf896d657965b54f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":17,"id":"c1ddb80e_9858f3cd","in_reply_to":"d4c42c7f_81baab57","updated":"2024-06-13 09:44:09.000000000","message":"Verified that the route is announced and IPs are reachable in this scenario, where the agent was not running when the network/router was created and attached accordingly.","commit_id":"bc1dc8ec9ea2438fe89ad0d9535dfddd05c463d9"}],"ovn_bgp_agent/drivers/openstack/utils/wire.py":[{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"db8173e97d25723ed48b5807944932c90851b454","unresolved":true,"context_lines":[{"line_number":793,"context_line":""},{"line_number":794,"context_line":"            # NOTE(jayjahns): The route for the tenant subnet needs to be added to"},{"line_number":795,"context_line":"            # the bgp_nic in order to announce the network."},{"line_number":796,"context_line":"            linux_net.add_ip_route("},{"line_number":797,"context_line":"                routing_tables_routes,"},{"line_number":798,"context_line":"                ip.split(\"/\")[0],"},{"line_number":799,"context_line":"                CONF.bgp_vrf_table_id,"}],"source_content_type":"text/x-python","patch_set":7,"id":"a55cc201_5d070c32","line":796,"range":{"start_line":796,"start_character":12,"end_line":796,"end_character":35},"updated":"2024-06-11 06:03:16.000000000","message":"I would only do this if the exposing method is subnet, as otherwise it is not needed, right?","commit_id":"b47b2d70a10b0116c81577087d6c5a1cef672cf6"},{"author":{"_account_id":36670,"name":"Jay Jahns","email":"jayjahns@gmail.com","username":"jayjahns"},"change_message_id":"7f7a6f285ac156b996f1b0ca67430ff484cd5d3d","unresolved":false,"context_lines":[{"line_number":793,"context_line":""},{"line_number":794,"context_line":"            # NOTE(jayjahns): The route for the tenant subnet needs to be added to"},{"line_number":795,"context_line":"            # the bgp_nic in order to announce the network."},{"line_number":796,"context_line":"            linux_net.add_ip_route("},{"line_number":797,"context_line":"                routing_tables_routes,"},{"line_number":798,"context_line":"                ip.split(\"/\")[0],"},{"line_number":799,"context_line":"                CONF.bgp_vrf_table_id,"}],"source_content_type":"text/x-python","patch_set":7,"id":"b850afb2_84fe7a91","line":796,"range":{"start_line":796,"start_character":12,"end_line":796,"end_character":35},"in_reply_to":"a55cc201_5d070c32","updated":"2024-06-11 06:42:29.000000000","message":"Updated new patch set with the conditionals.","commit_id":"b47b2d70a10b0116c81577087d6c5a1cef672cf6"}],"ovn_bgp_agent/tests/unit/drivers/openstack/utils/test_wire.py":[{"author":{"_account_id":36670,"name":"Jay Jahns","email":"jayjahns@gmail.com","username":"jayjahns"},"change_message_id":"50d7bf9605f5df6ae15e724144f09fa1d81a3a25","unresolved":false,"context_lines":[{"line_number":753,"context_line":"    @mock.patch.object(linux_net, \u0027add_ip_route\u0027)"},{"line_number":754,"context_line":"    def test__wire_lrp_port_underlay_advertisement_subnet(self, m_ip_version,"},{"line_number":755,"context_line":"                                                          m_ip_route):"},{"line_number":756,"context_line":"        CONF.set_override(\u0027advertisement_method_tenant_networks\u0027, "},{"line_number":757,"context_line":"                          constants.ADVERTISEMENT_METHOD_SUBNET)"},{"line_number":758,"context_line":"        routing_tables_routes \u003d {}"},{"line_number":759,"context_line":"        ip \u003d \u002710.0.0.1/24\u0027"}],"source_content_type":"text/x-python","patch_set":10,"id":"94e09849_f9b8497c","line":756,"in_reply_to":"7456b618_85f25053","updated":"2024-06-11 18:44:08.000000000","message":"\u003e pep8: W291 trailing whitespace\n\nPlease fix.","commit_id":"cc0e6b7ce3adf38a306d908d106a2cbb6dedc19d"},{"author":{"_account_id":36670,"name":"Jay Jahns","email":"jayjahns@gmail.com","username":"jayjahns"},"change_message_id":"50d7bf9605f5df6ae15e724144f09fa1d81a3a25","unresolved":false,"context_lines":[{"line_number":774,"context_line":"            CONF.bgp_nic, mask\u003d\u002724\u0027, via\u003d\u0027fake-crlrp-ip\u0027)"},{"line_number":775,"context_line":""},{"line_number":776,"context_line":""},{"line_number":777,"context_line":"    @mock.patch.object(linux_net, \u0027del_ip_route\u0027)"},{"line_number":778,"context_line":"    @mock.patch.object(linux_net, \u0027get_ip_version\u0027)"},{"line_number":779,"context_line":"    @mock.patch.object(linux_net, \u0027del_ip_rule\u0027)"},{"line_number":780,"context_line":"    def test__unwire_lrp_port_underlay(self, m_ip_rule, m_ip_version,"}],"source_content_type":"text/x-python","patch_set":10,"id":"1c01cd54_df12222c","line":777,"in_reply_to":"ad474747_a9063da3","updated":"2024-06-11 18:44:08.000000000","message":"\u003e pep8: E303 too many blank lines (2)\n\nPlease fix.","commit_id":"cc0e6b7ce3adf38a306d908d106a2cbb6dedc19d"},{"author":{"_account_id":36670,"name":"Jay Jahns","email":"jayjahns@gmail.com","username":"jayjahns"},"change_message_id":"50d7bf9605f5df6ae15e724144f09fa1d81a3a25","unresolved":false,"context_lines":[{"line_number":835,"context_line":"    @mock.patch.object(linux_net, \u0027del_ip_route\u0027)"},{"line_number":836,"context_line":"    def test__unwire_lrp_port_underlay_advertisement_subnet(self, m_ip_version,"},{"line_number":837,"context_line":"                                                            m_ip_route):"},{"line_number":838,"context_line":"        CONF.set_override(\u0027advertisement_method_tenant_networks\u0027, "},{"line_number":839,"context_line":"                          constants.ADVERTISEMENT_METHOD_SUBNET)"},{"line_number":840,"context_line":"        routing_tables_routes \u003d {}"},{"line_number":841,"context_line":"        ip \u003d \u002710.0.0.1/24\u0027"}],"source_content_type":"text/x-python","patch_set":10,"id":"f2c364fa_4e720d1d","line":838,"in_reply_to":"a1193d55_9deec620","updated":"2024-06-11 18:44:08.000000000","message":"\u003e pep8: W291 trailing whitespace\n\nPlease fix.","commit_id":"cc0e6b7ce3adf38a306d908d106a2cbb6dedc19d"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"a4e8ae3bbd7025d750a0e8812e9ec4a6c73bce5f","unresolved":true,"context_lines":[{"line_number":769,"context_line":"                                           routing_tables, cr_lrp_ips)"},{"line_number":770,"context_line":"        self.assertTrue(ret)"},{"line_number":771,"context_line":"        m_ip_rule.assert_called_once_with(ip, 5)"},{"line_number":772,"context_line":"        m_ip_route.assert_called_with("},{"line_number":773,"context_line":"            routing_tables_routes, ip.split(\u0027/\u0027)[0], 5,"},{"line_number":774,"context_line":"            routing_tables[bridge_device], vlan\u003dbridge_vlan,"},{"line_number":775,"context_line":"            mask\u003dip.split(\u0027/\u0027)[1], via\u003dcr_lrp_ips[0])"},{"line_number":776,"context_line":"        m_ip_route.assert_called_with("},{"line_number":777,"context_line":"            routing_tables_routes, ip.split(\u0027/\u0027)[0],"},{"line_number":778,"context_line":"            CONF.bgp_vrf_table_id, CONF.bgp_nic,"},{"line_number":779,"context_line":"            mask\u003dip.split(\u0027/\u0027)[1], via\u003dcr_lrp_ips[0])"},{"line_number":780,"context_line":""},{"line_number":781,"context_line":"    @mock.patch.object(linux_net, \u0027del_ip_route\u0027)"},{"line_number":782,"context_line":"    @mock.patch.object(linux_net, \u0027get_ip_version\u0027)"}],"source_content_type":"text/x-python","patch_set":13,"id":"be59cb8c_0ee132c7","line":779,"range":{"start_line":772,"start_character":0,"end_line":779,"end_character":53},"updated":"2024-06-12 12:27:37.000000000","message":"As you are calling twice to add_ip_route, instead of assert_called_with, you should use \"has_calls\". You can find examples in the other test files, for example: https://opendev.org/openstack/ovn-bgp-agent/src/commit/d24d6fbf9fbd8dab260fd6c720d42edd052f76a9/ovn_bgp_agent/tests/unit/drivers/openstack/test_nb_ovn_bgp_driver.py#L1105","commit_id":"79068e35bb997785b1a19d6e84d20cd8d62ae932"},{"author":{"_account_id":36670,"name":"Jay Jahns","email":"jayjahns@gmail.com","username":"jayjahns"},"change_message_id":"04c239c9ec42b190d6af2dc74b098b719f4d9889","unresolved":false,"context_lines":[{"line_number":769,"context_line":"                                           routing_tables, cr_lrp_ips)"},{"line_number":770,"context_line":"        self.assertTrue(ret)"},{"line_number":771,"context_line":"        m_ip_rule.assert_called_once_with(ip, 5)"},{"line_number":772,"context_line":"        m_ip_route.assert_called_with("},{"line_number":773,"context_line":"            routing_tables_routes, ip.split(\u0027/\u0027)[0], 5,"},{"line_number":774,"context_line":"            routing_tables[bridge_device], vlan\u003dbridge_vlan,"},{"line_number":775,"context_line":"            mask\u003dip.split(\u0027/\u0027)[1], via\u003dcr_lrp_ips[0])"},{"line_number":776,"context_line":"        m_ip_route.assert_called_with("},{"line_number":777,"context_line":"            routing_tables_routes, ip.split(\u0027/\u0027)[0],"},{"line_number":778,"context_line":"            CONF.bgp_vrf_table_id, CONF.bgp_nic,"},{"line_number":779,"context_line":"            mask\u003dip.split(\u0027/\u0027)[1], via\u003dcr_lrp_ips[0])"},{"line_number":780,"context_line":""},{"line_number":781,"context_line":"    @mock.patch.object(linux_net, \u0027del_ip_route\u0027)"},{"line_number":782,"context_line":"    @mock.patch.object(linux_net, \u0027get_ip_version\u0027)"}],"source_content_type":"text/x-python","patch_set":13,"id":"66d20552_5cc687ee","line":779,"range":{"start_line":772,"start_character":0,"end_line":779,"end_character":53},"in_reply_to":"be59cb8c_0ee132c7","updated":"2024-06-12 19:46:13.000000000","message":"Done","commit_id":"79068e35bb997785b1a19d6e84d20cd8d62ae932"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"b933315c2b2d0b7be886d4e1fc11c3c9fa279993","unresolved":true,"context_lines":[{"line_number":770,"context_line":"        self.assertTrue(ret)"},{"line_number":771,"context_line":"        m_ip_rule.assert_called_once_with(ip, 5)"},{"line_number":772,"context_line":"        expected_calls \u003d ["},{"line_number":773,"context_line":"            mock.call(ip),"},{"line_number":774,"context_line":"            mock.call(cr_lrp_ips[0])]"},{"line_number":775,"context_line":"        m_ip_route.assert_has_calls(expected_calls)"},{"line_number":776,"context_line":""},{"line_number":777,"context_line":"    @mock.patch.object(linux_net, \u0027del_ip_route\u0027)"}],"source_content_type":"text/x-python","patch_set":15,"id":"4e578f7d_36b094f7","line":774,"range":{"start_line":773,"start_character":22,"end_line":774,"end_character":35},"updated":"2024-06-12 13:07:54.000000000","message":"you need to add all the parameters here","commit_id":"ad5821b310cdb695044fbb95f5cc8cab33a9ce7c"},{"author":{"_account_id":36670,"name":"Jay Jahns","email":"jayjahns@gmail.com","username":"jayjahns"},"change_message_id":"04c239c9ec42b190d6af2dc74b098b719f4d9889","unresolved":false,"context_lines":[{"line_number":770,"context_line":"        self.assertTrue(ret)"},{"line_number":771,"context_line":"        m_ip_rule.assert_called_once_with(ip, 5)"},{"line_number":772,"context_line":"        expected_calls \u003d ["},{"line_number":773,"context_line":"            mock.call(ip),"},{"line_number":774,"context_line":"            mock.call(cr_lrp_ips[0])]"},{"line_number":775,"context_line":"        m_ip_route.assert_has_calls(expected_calls)"},{"line_number":776,"context_line":""},{"line_number":777,"context_line":"    @mock.patch.object(linux_net, \u0027del_ip_route\u0027)"}],"source_content_type":"text/x-python","patch_set":15,"id":"7f87873a_13c917de","line":774,"range":{"start_line":773,"start_character":22,"end_line":774,"end_character":35},"in_reply_to":"48124e4d_7537ee63","updated":"2024-06-12 19:46:13.000000000","message":"Done","commit_id":"ad5821b310cdb695044fbb95f5cc8cab33a9ce7c"},{"author":{"_account_id":36670,"name":"Jay Jahns","email":"jayjahns@gmail.com","username":"jayjahns"},"change_message_id":"264af6cc7d4aac8bc491ced697a022946a84824e","unresolved":true,"context_lines":[{"line_number":770,"context_line":"        self.assertTrue(ret)"},{"line_number":771,"context_line":"        m_ip_rule.assert_called_once_with(ip, 5)"},{"line_number":772,"context_line":"        expected_calls \u003d ["},{"line_number":773,"context_line":"            mock.call(ip),"},{"line_number":774,"context_line":"            mock.call(cr_lrp_ips[0])]"},{"line_number":775,"context_line":"        m_ip_route.assert_has_calls(expected_calls)"},{"line_number":776,"context_line":""},{"line_number":777,"context_line":"    @mock.patch.object(linux_net, \u0027del_ip_route\u0027)"}],"source_content_type":"text/x-python","patch_set":15,"id":"48124e4d_7537ee63","line":774,"range":{"start_line":773,"start_character":22,"end_line":774,"end_character":35},"in_reply_to":"4e578f7d_36b094f7","updated":"2024-06-12 14:03:10.000000000","message":"The test fails with the other parameters. Apparently that is all that returns.","commit_id":"ad5821b310cdb695044fbb95f5cc8cab33a9ce7c"}]}