)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"7a7c4ac0c1699d2476f3172de3c8887293d5c438","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"6a23d0d2_5f6bf07c","updated":"2023-08-03 21:25:58.000000000","message":"Thanks for taking a look at this issue.","commit_id":"9e34437d7a7871bda39cdbb5dc9c448660ef9d69"},{"author":{"_account_id":36269,"name":"Alban PRATS","display_name":"aprats","email":"alban.prats@ovhcloud.com","username":"aprats"},"change_message_id":"55cf7bc4456016205f4ba4a2e7a474a36bd4b030","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"dfc72efb_9a80f778","updated":"2023-08-04 13:34:35.000000000","message":"Thanks for your feedback ! I\u0027ll upload a patch set to take this into account !","commit_id":"9e34437d7a7871bda39cdbb5dc9c448660ef9d69"},{"author":{"_account_id":36269,"name":"Alban PRATS","display_name":"aprats","email":"alban.prats@ovhcloud.com","username":"aprats"},"change_message_id":"1f240d3a4d4f7fdecd94dab1890bfa0b435c9b52","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"b03b4c4b_eb9f0a30","updated":"2023-08-17 14:43:38.000000000","message":"recheck","commit_id":"166b0ee93a1f2ce796cd5e4ab2f0c9ff972ae3d1"},{"author":{"_account_id":11583,"name":"Arnaud Morin","email":"arnaud.morin@gmail.com","username":"arnaudmorin"},"change_message_id":"203c57de455f9c5838b5f6d3b662c225e7c19144","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"c021cba5_6875e1f6","updated":"2023-08-18 07:26:28.000000000","message":"recheck neutron-ovs-rally-task","commit_id":"166b0ee93a1f2ce796cd5e4ab2f0c9ff972ae3d1"},{"author":{"_account_id":36269,"name":"Alban PRATS","display_name":"aprats","email":"alban.prats@ovhcloud.com","username":"aprats"},"change_message_id":"aac244e595116c674cf8d61e106cca6f4cd16ec4","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"8af37c32_96b156b8","updated":"2023-09-13 15:34:25.000000000","message":"recheck","commit_id":"4a38b21616be4d96779810cba760406983116db3"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"9d552b54ad0cb5c4191208beaf3240bce5d35bc0","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"29bd1915_1098495d","updated":"2024-03-06 08:48:01.000000000","message":"As commented by Elvira, please add unit or functional tests to check this new code. \n\nAdd a better description in the commit message of what you are doing (similar to the LP bug description)\n\nAdd a release note with this fix that is not trivial.","commit_id":"b001c15f4c351f18437a56f3f67492d4c950dc7d"},{"author":{"_account_id":32586,"name":"Elvira García Ruiz","display_name":"Elvira","email":"egarciar@redhat.com","username":"elvira"},"change_message_id":"3322db2dde900ce71a77a3937038f8e22ea8d7c8","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"e03c1e0f_d4965208","updated":"2023-10-30 11:22:58.000000000","message":"Maybe adding some testing to this would be nice","commit_id":"b001c15f4c351f18437a56f3f67492d4c950dc7d"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"a2c3b861bda74acc4923bacbf18b69f06de7a212","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"1f9053f9_59af8d38","updated":"2024-03-21 16:44:24.000000000","message":"Sorry, been a while since I looked at this and had some questions.","commit_id":"b001c15f4c351f18437a56f3f67492d4c950dc7d"},{"author":{"_account_id":36269,"name":"Alban PRATS","display_name":"aprats","email":"alban.prats@ovhcloud.com","username":"aprats"},"change_message_id":"99e5fba5367a77d6ee5d4bb06cb57e981a668ed4","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"3c4b7d96_32539368","updated":"2023-10-10 17:12:06.000000000","message":"recheck","commit_id":"b001c15f4c351f18437a56f3f67492d4c950dc7d"}],"neutron/agent/l3/dvr_local_router.py":[{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"7a7c4ac0c1699d2476f3172de3c8887293d5c438","unresolved":true,"context_lines":[{"line_number":504,"context_line":"        \"\"\"Removes rules and routes for SNAT redirection.\"\"\""},{"line_number":505,"context_line":"        self._snat_redirect_modify(gateway, sn_port, sn_int, is_add\u003dFalse)"},{"line_number":506,"context_line":""},{"line_number":507,"context_line":"    def _update_routing_table(self, operation, route, namespace):"},{"line_number":508,"context_line":"        \"\"\"Adds redirection through NAT for static route \"\"\""},{"line_number":509,"context_line":"        cmd \u003d [\u0027ip\u0027, \u0027route\u0027, operation, \u0027to\u0027, route[\u0027destination\u0027],"},{"line_number":510,"context_line":"               \u0027via\u0027, route[\u0027nexthop\u0027]]"}],"source_content_type":"text/x-python","patch_set":1,"id":"d918854d_1ac03e69","line":507,"updated":"2023-08-03 21:25:58.000000000","message":"Should this instead go in the update_routing_table() we override below in this class?","commit_id":"9e34437d7a7871bda39cdbb5dc9c448660ef9d69"},{"author":{"_account_id":36269,"name":"Alban PRATS","display_name":"aprats","email":"alban.prats@ovhcloud.com","username":"aprats"},"change_message_id":"55cf7bc4456016205f4ba4a2e7a474a36bd4b030","unresolved":false,"context_lines":[{"line_number":504,"context_line":"        \"\"\"Removes rules and routes for SNAT redirection.\"\"\""},{"line_number":505,"context_line":"        self._snat_redirect_modify(gateway, sn_port, sn_int, is_add\u003dFalse)"},{"line_number":506,"context_line":""},{"line_number":507,"context_line":"    def _update_routing_table(self, operation, route, namespace):"},{"line_number":508,"context_line":"        \"\"\"Adds redirection through NAT for static route \"\"\""},{"line_number":509,"context_line":"        cmd \u003d [\u0027ip\u0027, \u0027route\u0027, operation, \u0027to\u0027, route[\u0027destination\u0027],"},{"line_number":510,"context_line":"               \u0027via\u0027, route[\u0027nexthop\u0027]]"}],"source_content_type":"text/x-python","patch_set":1,"id":"a9d5777f_ad0a3d9d","line":507,"in_reply_to":"d918854d_1ac03e69","updated":"2023-08-04 13:34:35.000000000","message":"Ack","commit_id":"9e34437d7a7871bda39cdbb5dc9c448660ef9d69"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"7a7c4ac0c1699d2476f3172de3c8887293d5c438","unresolved":true,"context_lines":[{"line_number":510,"context_line":"               \u0027via\u0027, route[\u0027nexthop\u0027]]"},{"line_number":511,"context_line":"        ip_wrapper \u003d ip_lib.IPWrapper(namespace\u003dnamespace)"},{"line_number":512,"context_line":"        ip_wrapper.netns.execute(cmd, check_exit_code\u003dFalse,"},{"line_number":513,"context_line":"                                 log_fail_as_error\u003dFalse)"},{"line_number":514,"context_line":"        if self._get_gw_ips_cidr():  # Check if router has a gateway"},{"line_number":515,"context_line":"            for port in self.internal_ports:"},{"line_number":516,"context_line":"                for ip in port[\u0027fixed_ips\u0027]:"}],"source_content_type":"text/x-python","patch_set":1,"id":"fe6b2e1d_ede3ab4e","line":513,"updated":"2023-08-03 21:25:58.000000000","message":"The code above seems like it would be done in the super() of this? If so should just be called directly. We also don\u0027t want to call \u0027ip route\u0027 directly, but instead use the ip_lib.*_ip_route() calls.","commit_id":"9e34437d7a7871bda39cdbb5dc9c448660ef9d69"},{"author":{"_account_id":36269,"name":"Alban PRATS","display_name":"aprats","email":"alban.prats@ovhcloud.com","username":"aprats"},"change_message_id":"55cf7bc4456016205f4ba4a2e7a474a36bd4b030","unresolved":false,"context_lines":[{"line_number":510,"context_line":"               \u0027via\u0027, route[\u0027nexthop\u0027]]"},{"line_number":511,"context_line":"        ip_wrapper \u003d ip_lib.IPWrapper(namespace\u003dnamespace)"},{"line_number":512,"context_line":"        ip_wrapper.netns.execute(cmd, check_exit_code\u003dFalse,"},{"line_number":513,"context_line":"                                 log_fail_as_error\u003dFalse)"},{"line_number":514,"context_line":"        if self._get_gw_ips_cidr():  # Check if router has a gateway"},{"line_number":515,"context_line":"            for port in self.internal_ports:"},{"line_number":516,"context_line":"                for ip in port[\u0027fixed_ips\u0027]:"}],"source_content_type":"text/x-python","patch_set":1,"id":"d2153b40_9726e5bd","line":513,"in_reply_to":"fe6b2e1d_ede3ab4e","updated":"2023-08-04 13:34:35.000000000","message":"Ack","commit_id":"9e34437d7a7871bda39cdbb5dc9c448660ef9d69"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"7a7c4ac0c1699d2476f3172de3c8887293d5c438","unresolved":true,"context_lines":[{"line_number":518,"context_line":"                        \"/\".join([ip[\u0027ip_address\u0027], str(ip[\u0027prefixlen\u0027])]))"},{"line_number":519,"context_line":"                    if netaddr.IPAddress(route[\u0027nexthop\u0027]) in sn_port_cidr:"},{"line_number":520,"context_line":"                        snat_idx \u003d self._get_snat_idx(str(sn_port_cidr))"},{"line_number":521,"context_line":"                        if operation in (\u0027del\u0027, \u0027replace\u0027):"},{"line_number":522,"context_line":"                            # TODO(aprats) how to cleanup"},{"line_number":523,"context_line":"                            try:"},{"line_number":524,"context_line":"                                ip_lib.delete_ip_rule(self.ns_name,"}],"source_content_type":"text/x-python","patch_set":1,"id":"af15a387_ab342f78","line":521,"updated":"2023-08-03 21:25:58.000000000","message":"I *think* we only ever pass \u0027delete\u0027 or \u0027replace\u0027 and not any other string, where replace would mean add.","commit_id":"9e34437d7a7871bda39cdbb5dc9c448660ef9d69"},{"author":{"_account_id":36269,"name":"Alban PRATS","display_name":"aprats","email":"alban.prats@ovhcloud.com","username":"aprats"},"change_message_id":"4f9c2d4dd6124fd72849db58e3cbb0dfd43391c5","unresolved":false,"context_lines":[{"line_number":518,"context_line":"                        \"/\".join([ip[\u0027ip_address\u0027], str(ip[\u0027prefixlen\u0027])]))"},{"line_number":519,"context_line":"                    if netaddr.IPAddress(route[\u0027nexthop\u0027]) in sn_port_cidr:"},{"line_number":520,"context_line":"                        snat_idx \u003d self._get_snat_idx(str(sn_port_cidr))"},{"line_number":521,"context_line":"                        if operation in (\u0027del\u0027, \u0027replace\u0027):"},{"line_number":522,"context_line":"                            # TODO(aprats) how to cleanup"},{"line_number":523,"context_line":"                            try:"},{"line_number":524,"context_line":"                                ip_lib.delete_ip_rule(self.ns_name,"}],"source_content_type":"text/x-python","patch_set":1,"id":"98621f45_1dc42c80","line":521,"in_reply_to":"7070ac5b_5153f319","updated":"2023-08-30 19:50:23.000000000","message":"Ack","commit_id":"9e34437d7a7871bda39cdbb5dc9c448660ef9d69"},{"author":{"_account_id":36269,"name":"Alban PRATS","display_name":"aprats","email":"alban.prats@ovhcloud.com","username":"aprats"},"change_message_id":"55cf7bc4456016205f4ba4a2e7a474a36bd4b030","unresolved":true,"context_lines":[{"line_number":518,"context_line":"                        \"/\".join([ip[\u0027ip_address\u0027], str(ip[\u0027prefixlen\u0027])]))"},{"line_number":519,"context_line":"                    if netaddr.IPAddress(route[\u0027nexthop\u0027]) in sn_port_cidr:"},{"line_number":520,"context_line":"                        snat_idx \u003d self._get_snat_idx(str(sn_port_cidr))"},{"line_number":521,"context_line":"                        if operation in (\u0027del\u0027, \u0027replace\u0027):"},{"line_number":522,"context_line":"                            # TODO(aprats) how to cleanup"},{"line_number":523,"context_line":"                            try:"},{"line_number":524,"context_line":"                                ip_lib.delete_ip_rule(self.ns_name,"}],"source_content_type":"text/x-python","patch_set":1,"id":"7070ac5b_5153f319","line":521,"in_reply_to":"af15a387_ab342f78","updated":"2023-08-04 13:34:35.000000000","message":"By looking quickly at the code I think you are right. I\u0027m letting this here until someone confirms but I agree with you.","commit_id":"9e34437d7a7871bda39cdbb5dc9c448660ef9d69"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"2fe4eafde635a43c9cfb020315977470e9c9949f","unresolved":true,"context_lines":[{"line_number":887,"context_line":"                            ip_lib.add_ip_rule(namespace\u003dself.ns_name,"},{"line_number":888,"context_line":"                                               ip\u003droute[\u0027destination\u0027],"},{"line_number":889,"context_line":"                                               table\u003dsnat_idx,"},{"line_number":890,"context_line":"                                               priority\u003dsnat_idx)"},{"line_number":891,"context_line":""},{"line_number":892,"context_line":"    def _update_fip_route_table_with_next_hop_routes(self, operation, route,"},{"line_number":893,"context_line":"                                                     fip_ns_name, tbl_index):"}],"source_content_type":"text/x-python","patch_set":2,"id":"219a9e19_edc3277f","line":890,"updated":"2023-08-07 20:59:04.000000000","message":"So I think a lot of the code to do this work is already in this file, for example via external_gateway_added() -\u003e enable_snat_redirect_rules() -\u003e _snat_redirect_add().\n\nSorry to ask you to do more work, but what order are you running openstack commands? Sometimes one way works and another shows a bug in the code.\n\nI just haven\u0027t had time to set this up and debug myself locally yet.","commit_id":"166b0ee93a1f2ce796cd5e4ab2f0c9ff972ae3d1"},{"author":{"_account_id":36269,"name":"Alban PRATS","display_name":"aprats","email":"alban.prats@ovhcloud.com","username":"aprats"},"change_message_id":"6e119925a6db10e50cdbd7864bb4d0cc488edb20","unresolved":false,"context_lines":[{"line_number":887,"context_line":"                            ip_lib.add_ip_rule(namespace\u003dself.ns_name,"},{"line_number":888,"context_line":"                                               ip\u003droute[\u0027destination\u0027],"},{"line_number":889,"context_line":"                                               table\u003dsnat_idx,"},{"line_number":890,"context_line":"                                               priority\u003dsnat_idx)"},{"line_number":891,"context_line":""},{"line_number":892,"context_line":"    def _update_fip_route_table_with_next_hop_routes(self, operation, route,"},{"line_number":893,"context_line":"                                                     fip_ns_name, tbl_index):"}],"source_content_type":"text/x-python","patch_set":2,"id":"efae3b23_d28f4222","line":890,"in_reply_to":"219a9e19_edc3277f","updated":"2023-08-08 14:37:21.000000000","message":"In my tests I tried to create the router with gateway then create the route. Doing it the other way lead to the same issue. \nIn my opinion \"external_gateway_added\" only parse directly attached subnets. Therefore not creating the rule for the routed subnets.","commit_id":"166b0ee93a1f2ce796cd5e4ab2f0c9ff972ae3d1"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"a2c3b861bda74acc4923bacbf18b69f06de7a212","unresolved":true,"context_lines":[{"line_number":867,"context_line":"                tbl_index \u003d self._get_snat_idx(fip_2_rtr)"},{"line_number":868,"context_line":"                self._update_fip_route_table_with_next_hop_routes("},{"line_number":869,"context_line":"                    operation, route, fip_ns_name, tbl_index)"},{"line_number":870,"context_line":"        super(DvrLocalRouter, self).update_routing_table(operation, route)"},{"line_number":871,"context_line":"        if self._get_gw_ips_cidr():  # Check if router has a gateway"},{"line_number":872,"context_line":"            for port in self.internal_ports:"},{"line_number":873,"context_line":"                for ip in port[\u0027fixed_ips\u0027]:"}],"source_content_type":"text/x-python","patch_set":6,"id":"342e33fd_17a94aa2","line":870,"updated":"2024-03-21 16:44:24.000000000","message":"I would put all the below in it\u0027s own method _update_routing_table() like is done in the parent class.","commit_id":"b001c15f4c351f18437a56f3f67492d4c950dc7d"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"a2c3b861bda74acc4923bacbf18b69f06de7a212","unresolved":true,"context_lines":[{"line_number":868,"context_line":"                self._update_fip_route_table_with_next_hop_routes("},{"line_number":869,"context_line":"                    operation, route, fip_ns_name, tbl_index)"},{"line_number":870,"context_line":"        super(DvrLocalRouter, self).update_routing_table(operation, route)"},{"line_number":871,"context_line":"        if self._get_gw_ips_cidr():  # Check if router has a gateway"},{"line_number":872,"context_line":"            for port in self.internal_ports:"},{"line_number":873,"context_line":"                for ip in port[\u0027fixed_ips\u0027]:"},{"line_number":874,"context_line":"                    sn_port_cidr \u003d netaddr.IPNetwork("}],"source_content_type":"text/x-python","patch_set":6,"id":"921e22bd_c1079388","line":871,"updated":"2024-03-21 16:44:24.000000000","message":"Do we only care if there is an external gateway port? We never use the cidrs this returns.\n\nif self.get_ex_gw_port(): ?","commit_id":"b001c15f4c351f18437a56f3f67492d4c950dc7d"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"a2c3b861bda74acc4923bacbf18b69f06de7a212","unresolved":true,"context_lines":[{"line_number":879,"context_line":"                            ip_lib.delete_ip_rule(self.ns_name,"},{"line_number":880,"context_line":"                                                  ip\u003droute[\u0027destination\u0027],"},{"line_number":881,"context_line":"                                                  table\u003dsnat_idx,"},{"line_number":882,"context_line":"                                                  priority\u003dsnat_idx)"},{"line_number":883,"context_line":"                        except pyroute2_exc.NetlinkError as e:"},{"line_number":884,"context_line":"                            # Pass only in case of a non-existing rule."},{"line_number":885,"context_line":"                            if e.code !\u003d errno.ENOENT:"}],"source_content_type":"text/x-python","patch_set":6,"id":"db938f2a_c8fffece","line":882,"updated":"2024-03-21 16:44:24.000000000","message":"Can you explain why we are always deleting the rule? Shouldn\u0027t it just run add or delete?","commit_id":"b001c15f4c351f18437a56f3f67492d4c950dc7d"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"a2c3b861bda74acc4923bacbf18b69f06de7a212","unresolved":true,"context_lines":[{"line_number":880,"context_line":"                                                  ip\u003droute[\u0027destination\u0027],"},{"line_number":881,"context_line":"                                                  table\u003dsnat_idx,"},{"line_number":882,"context_line":"                                                  priority\u003dsnat_idx)"},{"line_number":883,"context_line":"                        except pyroute2_exc.NetlinkError as e:"},{"line_number":884,"context_line":"                            # Pass only in case of a non-existing rule."},{"line_number":885,"context_line":"                            if e.code !\u003d errno.ENOENT:"},{"line_number":886,"context_line":"                                raise e"}],"source_content_type":"text/x-python","patch_set":6,"id":"921b4642_18ef18e6","line":883,"updated":"2024-03-21 16:44:24.000000000","message":"The parent class catches all these, should keep it the same:\n\nexcept (RuntimeError, OSError, pyroute2_exc.NetlinkError):","commit_id":"b001c15f4c351f18437a56f3f67492d4c950dc7d"}]}