)]}'
{"neutron/agent/l3/dvr_local_router.py":[{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"5d133a2837ef865065f7eb0c2c6fc250022629bf","unresolved":false,"context_lines":[{"line_number":153,"context_line":"                           priority\u003dint(str(rule_pr)))"},{"line_number":154,"context_line":""},{"line_number":155,"context_line":"    def _remove_floating_ip_rule(self, floating_ip):"},{"line_number":156,"context_line":"        if floating_ip in self.floating_ips_dict:"},{"line_number":157,"context_line":"            fixed_ip, rule_pr \u003d self.floating_ips_dict[floating_ip]"},{"line_number":158,"context_line":"            ip_lib.delete_ip_rule(self.ns_name, ip\u003dfixed_ip,"},{"line_number":159,"context_line":"                                  table\u003ddvr_fip_ns.FIP_RT_TBL,"},{"line_number":160,"context_line":"                                  priority\u003dint(str(rule_pr)))"},{"line_number":161,"context_line":"            self.fip_ns.deallocate_rule_priority(floating_ip)"},{"line_number":162,"context_line":"            # TODO(rajeev): Handle else case - exception/log?"},{"line_number":163,"context_line":""},{"line_number":164,"context_line":"    def floating_ip_removed_dist(self, fip_cidr):"}],"source_content_type":"text/x-python","patch_set":2,"id":"9f560f44_7f624718","side":"PARENT","line":161,"range":{"start_line":156,"start_character":8,"end_line":161,"end_character":61},"updated":"2020-08-17 06:41:15.000000000","message":"What about handling \u0027else:\u0027 here to not delete current (fast) logic?","commit_id":"a7a24a559302cb3bfbb7408dda8f9119a2893e1e"},{"author":{"_account_id":6737,"name":"Edward Hope-Morley","email":"edward.hope-morley@canonical.com","username":"hopem"},"change_message_id":"573d73173c2e2a1b1e355a59a52cbeddc87e31d0","unresolved":false,"context_lines":[{"line_number":153,"context_line":"                           priority\u003dint(str(rule_pr)))"},{"line_number":154,"context_line":""},{"line_number":155,"context_line":"    def _remove_floating_ip_rule(self, floating_ip):"},{"line_number":156,"context_line":"        if floating_ip in self.floating_ips_dict:"},{"line_number":157,"context_line":"            fixed_ip, rule_pr \u003d self.floating_ips_dict[floating_ip]"},{"line_number":158,"context_line":"            ip_lib.delete_ip_rule(self.ns_name, ip\u003dfixed_ip,"},{"line_number":159,"context_line":"                                  table\u003ddvr_fip_ns.FIP_RT_TBL,"},{"line_number":160,"context_line":"                                  priority\u003dint(str(rule_pr)))"},{"line_number":161,"context_line":"            self.fip_ns.deallocate_rule_priority(floating_ip)"},{"line_number":162,"context_line":"            # TODO(rajeev): Handle else case - exception/log?"},{"line_number":163,"context_line":""},{"line_number":164,"context_line":"    def floating_ip_removed_dist(self, fip_cidr):"}],"source_content_type":"text/x-python","patch_set":2,"id":"9f560f44_824b8485","side":"PARENT","line":161,"range":{"start_line":156,"start_character":8,"end_line":161,"end_character":61},"in_reply_to":"9f560f44_7f624718","updated":"2020-08-17 08:15:28.000000000","message":"Hi Oleg, do you think there is a benefit of maintaining both code paths? Note that this patch is currently WIP and im trying out a couple of different solutions. Hoping to have one up by eod today.","commit_id":"a7a24a559302cb3bfbb7408dda8f9119a2893e1e"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"b0e3f09a2e91452c77be3b5a56f5f48d0e860872","unresolved":false,"context_lines":[{"line_number":153,"context_line":"                           priority\u003dint(str(rule_pr)))"},{"line_number":154,"context_line":""},{"line_number":155,"context_line":"    def _remove_floating_ip_rule(self, floating_ip):"},{"line_number":156,"context_line":"        if floating_ip in self.floating_ips_dict:"},{"line_number":157,"context_line":"            fixed_ip, rule_pr \u003d self.floating_ips_dict[floating_ip]"},{"line_number":158,"context_line":"            ip_lib.delete_ip_rule(self.ns_name, ip\u003dfixed_ip,"},{"line_number":159,"context_line":"                                  table\u003ddvr_fip_ns.FIP_RT_TBL,"},{"line_number":160,"context_line":"                                  priority\u003dint(str(rule_pr)))"},{"line_number":161,"context_line":"            self.fip_ns.deallocate_rule_priority(floating_ip)"},{"line_number":162,"context_line":"            # TODO(rajeev): Handle else case - exception/log?"},{"line_number":163,"context_line":""},{"line_number":164,"context_line":"    def floating_ip_removed_dist(self, fip_cidr):"}],"source_content_type":"text/x-python","patch_set":2,"id":"9f560f44_a502126e","side":"PARENT","line":161,"range":{"start_line":156,"start_character":8,"end_line":161,"end_character":61},"in_reply_to":"9f560f44_824b8485","updated":"2020-08-17 09:21:28.000000000","message":"yeah, I think the benefit is performance. As I see this bug fix should handle the case of agent restart (fip not in self.floating_ips_dict), otherwise in-memory approach should be fine.","commit_id":"a7a24a559302cb3bfbb7408dda8f9119a2893e1e"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"d090dbf626afb2da031c77d4df37f01da4d95cfa","unresolved":false,"context_lines":[{"line_number":820,"context_line":"        return {common_utils.ip_to_cidr(route[\u0027cidr\u0027])"},{"line_number":821,"context_line":"                for route in exist_routes}"},{"line_number":822,"context_line":""},{"line_number":823,"context_line":"    def process(self):"},{"line_number":824,"context_line":"        ex_gw_port \u003d self.get_ex_gw_port()"},{"line_number":825,"context_line":"        if ex_gw_port:"},{"line_number":826,"context_line":"            self.fip_ns \u003d self.agent.get_fip_ns(ex_gw_port[\u0027network_id\u0027])"}],"source_content_type":"text/x-python","patch_set":4,"id":"9f560f44_536dbc96","line":823,"range":{"start_line":823,"start_character":4,"end_line":823,"end_character":22},"updated":"2020-08-17 11:24:17.000000000","message":"This effectively means on each router update, not agent restart as commit message says..","commit_id":"f3d404b5c67ad38367bf022497a8a09080f6abb7"},{"author":{"_account_id":6737,"name":"Edward Hope-Morley","email":"edward.hope-morley@canonical.com","username":"hopem"},"change_message_id":"0f961b0b915741909d1f6706f25eae82ca76acdc","unresolved":false,"context_lines":[{"line_number":820,"context_line":"        return {common_utils.ip_to_cidr(route[\u0027cidr\u0027])"},{"line_number":821,"context_line":"                for route in exist_routes}"},{"line_number":822,"context_line":""},{"line_number":823,"context_line":"    def process(self):"},{"line_number":824,"context_line":"        ex_gw_port \u003d self.get_ex_gw_port()"},{"line_number":825,"context_line":"        if ex_gw_port:"},{"line_number":826,"context_line":"            self.fip_ns \u003d self.agent.get_fip_ns(ex_gw_port[\u0027network_id\u0027])"}],"source_content_type":"text/x-python","patch_set":4,"id":"9f560f44_b3e9b8b8","line":823,"range":{"start_line":823,"start_character":4,"end_line":823,"end_character":22},"in_reply_to":"9f560f44_536dbc96","updated":"2020-08-17 11:41:07.000000000","message":"Oh really, hmm ok I wil go and check that. To answer your other question, I originally put this in __init__ but the resources needed to do get_floating_ips() were not yet there - they are setup here in process(). So if doing this here is not efficient I can look at what it would take to do it in __init__.","commit_id":"f3d404b5c67ad38367bf022497a8a09080f6abb7"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"dde93e878196c0d9a72a025be228065b17c0abbf","unresolved":false,"context_lines":[{"line_number":820,"context_line":"        return {common_utils.ip_to_cidr(route[\u0027cidr\u0027])"},{"line_number":821,"context_line":"                for route in exist_routes}"},{"line_number":822,"context_line":""},{"line_number":823,"context_line":"    def process(self):"},{"line_number":824,"context_line":"        ex_gw_port \u003d self.get_ex_gw_port()"},{"line_number":825,"context_line":"        if ex_gw_port:"},{"line_number":826,"context_line":"            self.fip_ns \u003d self.agent.get_fip_ns(ex_gw_port[\u0027network_id\u0027])"}],"source_content_type":"text/x-python","patch_set":4,"id":"9f560f44_33fda8b1","line":823,"range":{"start_line":823,"start_character":4,"end_line":823,"end_character":22},"in_reply_to":"9f560f44_536dbc96","updated":"2020-08-17 11:25:27.000000000","message":"Why not in __init__ as in previos patch set?","commit_id":"f3d404b5c67ad38367bf022497a8a09080f6abb7"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"2b1d775c7a6f3bdbe38c52df96baaa05e1e59ac3","unresolved":false,"context_lines":[{"line_number":46,"context_line":"        self.fip_ns \u003d None"},{"line_number":47,"context_line":"        self._pending_arp_set \u003d set()"},{"line_number":48,"context_line":""},{"line_number":49,"context_line":"        ex_gw_port \u003d self.get_ex_gw_port()"},{"line_number":50,"context_line":"        if ex_gw_port:"},{"line_number":51,"context_line":"            self.fip_ns \u003d self.agent.get_fip_ns(ex_gw_port[\u0027network_id\u0027])"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"            # Ensure this in-memory dict is re-populated post agent-restart as"},{"line_number":54,"context_line":"            # it is used as the official record of fip ip rule priorities"},{"line_number":55,"context_line":"            # needed when removing a floating ip from a qrouter ns."},{"line_number":56,"context_line":"            for fip in self.get_floating_ips():"},{"line_number":57,"context_line":"                floating_ip \u003d fip[\u0027floating_ip_address\u0027]"},{"line_number":58,"context_line":"                fixed_ip \u003d fip[\u0027fixed_ip_address\u0027]"},{"line_number":59,"context_line":"                rule_pr \u003d self.fip_ns.lookup_rule_priority(floating_ip)"},{"line_number":60,"context_line":"                if rule_pr:"},{"line_number":61,"context_line":"                    self.floating_ips_dict[floating_ip] \u003d (fixed_ip, rule_pr)"},{"line_number":62,"context_line":"                else:"},{"line_number":63,"context_line":"                    LOG.error(\"Rule priority not found for floating ip %s\","},{"line_number":64,"context_line":"                              floating_ip)"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"    def migrate_centralized_floating_ip(self, fip, interface_name, device):"},{"line_number":67,"context_line":"        # Remove the centralized fip first and then add fip to the host"}],"source_content_type":"text/x-python","patch_set":5,"id":"9f560f44_2ec09f2a","line":64,"range":{"start_line":49,"start_character":8,"end_line":64,"end_character":42},"updated":"2020-08-17 12:51:27.000000000","message":"consider a separate method for this","commit_id":"2ad475a776b0add7d573adeacf1b456e77942526"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"6ca7b6d3eff8f11d5195a78516467915a97e5a9f","unresolved":false,"context_lines":[{"line_number":46,"context_line":"        self.fip_ns \u003d None"},{"line_number":47,"context_line":"        self._pending_arp_set \u003d set()"},{"line_number":48,"context_line":""},{"line_number":49,"context_line":"        ex_gw_port \u003d self.get_ex_gw_port()"},{"line_number":50,"context_line":"        if ex_gw_port:"},{"line_number":51,"context_line":"            self.fip_ns \u003d self.agent.get_fip_ns(ex_gw_port[\u0027network_id\u0027])"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"            # Ensure this in-memory dict is re-populated post agent-restart as"},{"line_number":54,"context_line":"            # it is used as the official record of fip ip rule priorities"},{"line_number":55,"context_line":"            # needed when removing a floating ip from a qrouter ns."},{"line_number":56,"context_line":"            for fip in self.get_floating_ips():"},{"line_number":57,"context_line":"                floating_ip \u003d fip[\u0027floating_ip_address\u0027]"},{"line_number":58,"context_line":"                fixed_ip \u003d fip[\u0027fixed_ip_address\u0027]"},{"line_number":59,"context_line":"                rule_pr \u003d self.fip_ns.lookup_rule_priority(floating_ip)"},{"line_number":60,"context_line":"                if rule_pr:"},{"line_number":61,"context_line":"                    self.floating_ips_dict[floating_ip] \u003d (fixed_ip, rule_pr)"},{"line_number":62,"context_line":"                else:"},{"line_number":63,"context_line":"                    LOG.error(\"Rule priority not found for floating ip %s\","},{"line_number":64,"context_line":"                              floating_ip)"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"    def migrate_centralized_floating_ip(self, fip, interface_name, device):"},{"line_number":67,"context_line":"        # Remove the centralized fip first and then add fip to the host"}],"source_content_type":"text/x-python","patch_set":5,"id":"9f560f44_8e2a8be4","line":64,"range":{"start_line":49,"start_character":8,"end_line":64,"end_character":42},"in_reply_to":"9f560f44_2ec09f2a","updated":"2020-08-17 12:54:07.000000000","message":"+1","commit_id":"2ad475a776b0add7d573adeacf1b456e77942526"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"6ca7b6d3eff8f11d5195a78516467915a97e5a9f","unresolved":false,"context_lines":[{"line_number":143,"context_line":""},{"line_number":144,"context_line":"        floating_ip \u003d fip[\u0027floating_ip_address\u0027]"},{"line_number":145,"context_line":"        fixed_ip \u003d fip[\u0027fixed_ip_address\u0027]"},{"line_number":146,"context_line":"        self._add_floating_ip_rule(floating_ip, fixed_ip)"},{"line_number":147,"context_line":"        fip_2_rtr_name \u003d self.fip_ns.get_int_device_name(self.router_id)"},{"line_number":148,"context_line":"        # Add routing rule in fip namespace"},{"line_number":149,"context_line":"        fip_ns_name \u003d self.fip_ns.get_name()"}],"source_content_type":"text/x-python","patch_set":5,"id":"9f560f44_2ed53fe6","line":146,"range":{"start_line":146,"start_character":1,"end_line":146,"end_character":57},"updated":"2020-08-17 12:54:07.000000000","message":"Everytime the agent is restarting, the router (with all its floating IPs) will be reprocessed. So here this line will add the priority rule cache back. So why need to add it in __init__ one more time?","commit_id":"2ad475a776b0add7d573adeacf1b456e77942526"},{"author":{"_account_id":6737,"name":"Edward Hope-Morley","email":"edward.hope-morley@canonical.com","username":"hopem"},"change_message_id":"124e2b2a537f2650e01b3eb3154eb8866130b33a","unresolved":false,"context_lines":[{"line_number":143,"context_line":""},{"line_number":144,"context_line":"        floating_ip \u003d fip[\u0027floating_ip_address\u0027]"},{"line_number":145,"context_line":"        fixed_ip \u003d fip[\u0027fixed_ip_address\u0027]"},{"line_number":146,"context_line":"        self._add_floating_ip_rule(floating_ip, fixed_ip)"},{"line_number":147,"context_line":"        fip_2_rtr_name \u003d self.fip_ns.get_int_device_name(self.router_id)"},{"line_number":148,"context_line":"        # Add routing rule in fip namespace"},{"line_number":149,"context_line":"        fip_ns_name \u003d self.fip_ns.get_name()"}],"source_content_type":"text/x-python","patch_set":5,"id":"9f560f44_d90d6b91","line":146,"range":{"start_line":146,"start_character":1,"end_line":146,"end_character":57},"in_reply_to":"9f560f44_2ed53fe6","updated":"2020-08-17 13:50:29.000000000","message":"Yes that\u0027s true but it isn\u0027t populating the dict that is needed when removing them. Having said that it seems this is a more suitable place to make that fix so i\u0027ll take a look. Thanks!","commit_id":"2ad475a776b0add7d573adeacf1b456e77942526"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"77be6406e6aeef318badef092cf2940ef4d9e19c","unresolved":false,"context_lines":[{"line_number":182,"context_line":"            self.fip_ns.deallocate_rule_priority(floating_ip)"},{"line_number":183,"context_line":"        else:"},{"line_number":184,"context_line":"            LOG.error(\"Unable to find necessary information to complete \""},{"line_number":185,"context_line":"                      \"removal of floating ip rules for %s  - will require \""},{"line_number":186,"context_line":"                      \"manual cleanup.\", floating_ip)"},{"line_number":187,"context_line":""},{"line_number":188,"context_line":"    def floating_ip_removed_dist(self, fip_cidr):"}],"source_content_type":"text/x-python","patch_set":8,"id":"9f560f44_890b8e22","line":185,"range":{"start_line":185,"start_character":58,"end_line":185,"end_character":60},"updated":"2020-08-18 15:04:52.000000000","message":"nit: extra space here","commit_id":"b5b9e2b0fc0dddbeb48c73d08f08b1c663e563e3"},{"author":{"_account_id":6737,"name":"Edward Hope-Morley","email":"edward.hope-morley@canonical.com","username":"hopem"},"change_message_id":"0e21310c7c6b0e530c58b91ba326de4e3dd1167b","unresolved":false,"context_lines":[{"line_number":182,"context_line":"            self.fip_ns.deallocate_rule_priority(floating_ip)"},{"line_number":183,"context_line":"        else:"},{"line_number":184,"context_line":"            LOG.error(\"Unable to find necessary information to complete \""},{"line_number":185,"context_line":"                      \"removal of floating ip rules for %s  - will require \""},{"line_number":186,"context_line":"                      \"manual cleanup.\", floating_ip)"},{"line_number":187,"context_line":""},{"line_number":188,"context_line":"    def floating_ip_removed_dist(self, fip_cidr):"}],"source_content_type":"text/x-python","patch_set":8,"id":"9f560f44_a16946bf","line":185,"range":{"start_line":185,"start_character":58,"end_line":185,"end_character":60},"in_reply_to":"9f560f44_890b8e22","updated":"2020-08-18 19:41:10.000000000","message":"Done","commit_id":"b5b9e2b0fc0dddbeb48c73d08f08b1c663e563e3"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"77be6406e6aeef318badef092cf2940ef4d9e19c","unresolved":false,"context_lines":[{"line_number":183,"context_line":"        else:"},{"line_number":184,"context_line":"            LOG.error(\"Unable to find necessary information to complete \""},{"line_number":185,"context_line":"                      \"removal of floating ip rules for %s  - will require \""},{"line_number":186,"context_line":"                      \"manual cleanup.\", floating_ip)"},{"line_number":187,"context_line":""},{"line_number":188,"context_line":"    def floating_ip_removed_dist(self, fip_cidr):"},{"line_number":189,"context_line":"        \"\"\"Remove floating IP from FIP namespace.\"\"\""}],"source_content_type":"text/x-python","patch_set":8,"id":"9f560f44_09301ee8","line":186,"updated":"2020-08-18 15:04:52.000000000","message":"What exactly does manual cleanup look like?  I\u0027m not sure what an admin can do with this information.","commit_id":"b5b9e2b0fc0dddbeb48c73d08f08b1c663e563e3"},{"author":{"_account_id":6737,"name":"Edward Hope-Morley","email":"edward.hope-morley@canonical.com","username":"hopem"},"change_message_id":"6b29b63d5ae6d4e48c1d3acc4e1a7b1a52cabe30","unresolved":false,"context_lines":[{"line_number":183,"context_line":"        else:"},{"line_number":184,"context_line":"            LOG.error(\"Unable to find necessary information to complete \""},{"line_number":185,"context_line":"                      \"removal of floating ip rules for %s  - will require \""},{"line_number":186,"context_line":"                      \"manual cleanup.\", floating_ip)"},{"line_number":187,"context_line":""},{"line_number":188,"context_line":"    def floating_ip_removed_dist(self, fip_cidr):"},{"line_number":189,"context_line":"        \"\"\"Remove floating IP from FIP namespace.\"\"\""}],"source_content_type":"text/x-python","patch_set":8,"id":"9f560f44_9fa45c0a","line":186,"in_reply_to":"9f560f44_09301ee8","updated":"2020-08-18 16:39:01.000000000","message":"Fair point. I wasn\u0027t sure what the best way to communicate that was tbh. I have a procedure for cleanup (remove unused allocations from fip-allocations file and delete associated rules from ns) and am planning to store it in a gist but I can also describe it in the LP bug and mention that in this error msg if that helps. I you have any other ideas please let me know.","commit_id":"b5b9e2b0fc0dddbeb48c73d08f08b1c663e563e3"},{"author":{"_account_id":6737,"name":"Edward Hope-Morley","email":"edward.hope-morley@canonical.com","username":"hopem"},"change_message_id":"1efb31fa9b4eb3319659249b93c03ca17c51d41f","unresolved":false,"context_lines":[{"line_number":183,"context_line":"        else:"},{"line_number":184,"context_line":"            LOG.error(\"Unable to find necessary information to complete \""},{"line_number":185,"context_line":"                      \"removal of floating ip rules for %s  - will require \""},{"line_number":186,"context_line":"                      \"manual cleanup.\", floating_ip)"},{"line_number":187,"context_line":""},{"line_number":188,"context_line":"    def floating_ip_removed_dist(self, fip_cidr):"},{"line_number":189,"context_line":"        \"\"\"Remove floating IP from FIP namespace.\"\"\""}],"source_content_type":"text/x-python","patch_set":8,"id":"9f560f44_7e08692c","line":186,"in_reply_to":"9f560f44_9fa45c0a","updated":"2020-08-18 19:35:33.000000000","message":"Brian, I\u0027ve pasted a workaround in the LP bug here - https://bugs.launchpad.net/neutron/+bug/1891673/comments/3","commit_id":"b5b9e2b0fc0dddbeb48c73d08f08b1c663e563e3"}]}