)]}'
{"neutron/agent/l3_agent.py":[{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"3b0a30d249f6598325eea6b9873506ae67dec5ed","unresolved":false,"context_lines":[{"line_number":278,"context_line":"            self._create_router_namespace(ri)"},{"line_number":279,"context_line":"        for c, r in self.metadata_filter_rules():"},{"line_number":280,"context_line":"            ri.iptables_manager.ipv4[\u0027filter\u0027].add_rule(c, r)"},{"line_number":281,"context_line":"        for c, r in self.metadata_nat_rules():"},{"line_number":282,"context_line":"            ri.iptables_manager.ipv4[\u0027nat\u0027].add_rule(c, r)"},{"line_number":283,"context_line":"        ri.iptables_manager.apply()"},{"line_number":284,"context_line":"        super(L3NATAgent, self).process_router_add(ri)"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAARH%2F%2F0Kc%3D","line":281,"updated":"2013-08-16 20:48:28.000000000","message":"Above are the metadata rules that get removed on router sync.\n\nNote that all of the other iptables rules are handled in process_router or in a helper function called from process router.\n\nIs there a reason that these rules need to be created here before the \"process_router\" method call?","commit_id":"09a503b2b38459cff4ed8a6674bea892cee50cd4"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"3b0a30d249f6598325eea6b9873506ae67dec5ed","unresolved":false,"context_lines":[{"line_number":293,"context_line":"        self.process_router(ri)"},{"line_number":294,"context_line":"        for c, r in self.metadata_filter_rules():"},{"line_number":295,"context_line":"            ri.iptables_manager.ipv4[\u0027filter\u0027].remove_rule(c, r)"},{"line_number":296,"context_line":"        for c, r in self.metadata_nat_rules():"},{"line_number":297,"context_line":"            ri.iptables_manager.ipv4[\u0027nat\u0027].remove_rule(c, r)"},{"line_number":298,"context_line":"        ri.iptables_manager.apply()"},{"line_number":299,"context_line":"        if self.conf.enable_metadata_proxy:"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAARH%2F%2F0KY%3D","line":296,"updated":"2013-08-16 20:48:28.000000000","message":"This is the only place where iptables rules are cleaned up when a router gets deleted.  iptables only gets partially cleaned up.  A bunch of other rules remain.\n\nCan we remove this cleanup and not bother.  If cleaning up is important here, then we should probably take care to clean it up properly.","commit_id":"09a503b2b38459cff4ed8a6674bea892cee50cd4"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"3b0a30d249f6598325eea6b9873506ae67dec5ed","unresolved":false,"context_lines":[{"line_number":375,"context_line":"                                        interface_name, internal_cidrs)"},{"line_number":376,"context_line":"        elif not ex_gw_port and ri.ex_gw_port:"},{"line_number":377,"context_line":"            self.external_gateway_removed(ri, ri.ex_gw_port,"},{"line_number":378,"context_line":"                                          interface_name, internal_cidrs)"},{"line_number":379,"context_line":""},{"line_number":380,"context_line":"        # Process SNAT rules for external gateway"},{"line_number":381,"context_line":"        ri.perform_snat_action(self._handle_router_snat_rules,"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAARH%2F%2F0KQ%3D","line":378,"updated":"2013-08-16 20:48:28.000000000","message":"All iptables rules (other than lines 279-283) are handled between here and line 391 (or by calls made from here).","commit_id":"09a503b2b38459cff4ed8a6674bea892cee50cd4"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"3b0a30d249f6598325eea6b9873506ae67dec5ed","unresolved":false,"context_lines":[{"line_number":419,"context_line":"        device \u003d ip_lib.IPDevice(interface_name, self.root_helper,"},{"line_number":420,"context_line":"                                 namespace\u003dri.ns_name())"},{"line_number":421,"context_line":""},{"line_number":422,"context_line":"        # Clear out all iptables rules for these chains."},{"line_number":423,"context_line":"        for chain, rule in self.floating_forward_rules(None, None):"},{"line_number":424,"context_line":"            ri.iptables_manager.ipv4[\u0027nat\u0027].empty_chain(chain)"},{"line_number":425,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAARH%2F%2F0JM%3D","line":422,"updated":"2013-08-16 20:48:28.000000000","message":"I still think that clearing out at least all of the floating ip rules here is the best approach.  Especially when iptables state has been corrupted by another error as I found in the bug that this patch addresses.\n\nHowever, this conflicts with the metadata rule in the nat PREROUTING chain.  For this approach to work, there has to be clear ownership of each chain.  Clear ownership has not been established.\n\nI think consolidating the metadata rule handling from above to here is a good start.  Any thoughts?","commit_id":"09a503b2b38459cff4ed8a6674bea892cee50cd4"},{"author":{"_account_id":2031,"name":"Nachi Ueno","email":"nati.ueno@gmail.com","username":"nati-ueno"},"change_message_id":"54520520229e7f4891dcb5ce72123d9755829855","unresolved":false,"context_lines":[{"line_number":409,"context_line":"                ri.iptables_manager.ipv4[\u0027nat\u0027].add_rule(*rule)"},{"line_number":410,"context_line":"        ri.iptables_manager.apply()"},{"line_number":411,"context_line":""},{"line_number":412,"context_line":"    def process_router_floating_ips(self, ri, ex_gw_port):"},{"line_number":413,"context_line":"        \"\"\"Configure the router\u0027s floating IPs"},{"line_number":414,"context_line":"        Configures floating ips in iptables and on the router\u0027s gateway device."},{"line_number":415,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAS3%2F%2FfHw%3D","line":412,"updated":"2013-09-25 21:35:27.000000000","message":"This method gets long. \nCould you separate this for multiple helper functions?","commit_id":"42cf641d5718e57c806878f66121175383fcc21d"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"07d12e3e74d7475b5fee4093544542fb57d58429","unresolved":false,"context_lines":[{"line_number":409,"context_line":"                ri.iptables_manager.ipv4[\u0027nat\u0027].add_rule(*rule)"},{"line_number":410,"context_line":"        ri.iptables_manager.apply()"},{"line_number":411,"context_line":""},{"line_number":412,"context_line":"    def process_router_floating_ips(self, ri, ex_gw_port):"},{"line_number":413,"context_line":"        \"\"\"Configure the router\u0027s floating IPs"},{"line_number":414,"context_line":"        Configures floating ips in iptables and on the router\u0027s gateway device."},{"line_number":415,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAS3%2F%2FcUc%3D","line":412,"in_reply_to":"AAAAS3%2F%2FfHw%3D","updated":"2013-09-26 15:47:30.000000000","message":"I\u0027ll keep this in mind as I attempt to incorporate the other feedback below.","commit_id":"42cf641d5718e57c806878f66121175383fcc21d"},{"author":{"_account_id":261,"name":"Salvatore Orlando","email":"salv.orlando@gmail.com","username":"salvatore-orlando"},"change_message_id":"a5d6684a0340dd0002b5a0b56bab752326f625d2","unresolved":false,"context_lines":[{"line_number":420,"context_line":"                                 namespace\u003dri.ns_name())"},{"line_number":421,"context_line":""},{"line_number":422,"context_line":"        # Clear out all iptables rules for these chains."},{"line_number":423,"context_line":"        for chain, rule in self.floating_forward_rules(None, None):"},{"line_number":424,"context_line":"            ri.iptables_manager.ipv4[\u0027nat\u0027].empty_chain(chain)"},{"line_number":425,"context_line":""},{"line_number":426,"context_line":"        for c, r in self.metadata_filter_rules():"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAS3%2F%2Ff%2Fg%3D","line":423,"updated":"2013-09-25 19:02:53.000000000","message":"I think it is a bit weird to call this function only to return the chain names. I reckon it might be ok to directly drop all the rules in float-snat (the names seem to be hardcoded anyway)\n\nnit: as rule is unused in the loop the convention should to prepend it with an underscore.\n\nOptional comment - I\u0027m rather pedant today,","commit_id":"42cf641d5718e57c806878f66121175383fcc21d"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"01a9b1b7eb0a32f32e7165ae4b9c3f0cf9cb85aa","unresolved":false,"context_lines":[{"line_number":420,"context_line":"                                 namespace\u003dri.ns_name())"},{"line_number":421,"context_line":""},{"line_number":422,"context_line":"        # Clear out all iptables rules for these chains."},{"line_number":423,"context_line":"        for chain, rule in self.floating_forward_rules(None, None):"},{"line_number":424,"context_line":"            ri.iptables_manager.ipv4[\u0027nat\u0027].empty_chain(chain)"},{"line_number":425,"context_line":""},{"line_number":426,"context_line":"        for c, r in self.metadata_filter_rules():"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAS3%2F%2FayY%3D","line":423,"in_reply_to":"AAAAS3%2F%2FcVc%3D","updated":"2013-09-27 04:02:19.000000000","message":"It seems to be between this and hard-coding the chain names here.  We do need to adjust PREROUTING, float-snat, and OUTPUT here.\n\nI will change rule to _rule per convention.","commit_id":"42cf641d5718e57c806878f66121175383fcc21d"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"07d12e3e74d7475b5fee4093544542fb57d58429","unresolved":false,"context_lines":[{"line_number":420,"context_line":"                                 namespace\u003dri.ns_name())"},{"line_number":421,"context_line":""},{"line_number":422,"context_line":"        # Clear out all iptables rules for these chains."},{"line_number":423,"context_line":"        for chain, rule in self.floating_forward_rules(None, None):"},{"line_number":424,"context_line":"            ri.iptables_manager.ipv4[\u0027nat\u0027].empty_chain(chain)"},{"line_number":425,"context_line":""},{"line_number":426,"context_line":"        for c, r in self.metadata_filter_rules():"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAS3%2F%2FcVc%3D","line":423,"in_reply_to":"AAAAS3%2F%2Ff%2Fg%3D","updated":"2013-09-26 15:47:30.000000000","message":"Fair comment.  I hope to get some time today.  I\u0027ll play with clearing only the chains that need to be cleared here and will put up a follow-on patch.","commit_id":"42cf641d5718e57c806878f66121175383fcc21d"},{"author":{"_account_id":2031,"name":"Nachi Ueno","email":"nati.ueno@gmail.com","username":"nati-ueno"},"change_message_id":"54520520229e7f4891dcb5ce72123d9755829855","unresolved":false,"context_lines":[{"line_number":423,"context_line":"        for chain, rule in self.floating_forward_rules(None, None):"},{"line_number":424,"context_line":"            ri.iptables_manager.ipv4[\u0027nat\u0027].empty_chain(chain)"},{"line_number":425,"context_line":""},{"line_number":426,"context_line":"        for c, r in self.metadata_filter_rules():"},{"line_number":427,"context_line":"            ri.iptables_manager.ipv4[\u0027filter\u0027].add_rule(c, r)"},{"line_number":428,"context_line":"        for c, r in self.metadata_nat_rules():"},{"line_number":429,"context_line":"            ri.iptables_manager.ipv4[\u0027nat\u0027].add_rule(c, r)"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAS3%2F%2FfHg%3D","line":426,"updated":"2013-09-25 21:35:27.000000000","message":"Adding metadata_filter in this method sound wired, and may be, not easy to understand.\nHow about just take sync all configuration strategies?","commit_id":"42cf641d5718e57c806878f66121175383fcc21d"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"01a9b1b7eb0a32f32e7165ae4b9c3f0cf9cb85aa","unresolved":false,"context_lines":[{"line_number":423,"context_line":"        for chain, rule in self.floating_forward_rules(None, None):"},{"line_number":424,"context_line":"            ri.iptables_manager.ipv4[\u0027nat\u0027].empty_chain(chain)"},{"line_number":425,"context_line":""},{"line_number":426,"context_line":"        for c, r in self.metadata_filter_rules():"},{"line_number":427,"context_line":"            ri.iptables_manager.ipv4[\u0027filter\u0027].add_rule(c, r)"},{"line_number":428,"context_line":"        for c, r in self.metadata_nat_rules():"},{"line_number":429,"context_line":"            ri.iptables_manager.ipv4[\u0027nat\u0027].add_rule(c, r)"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAS3%2F%2Fays%3D","line":426,"in_reply_to":"AAAAS3%2F%2Fd2E%3D","updated":"2013-09-27 04:02:19.000000000","message":"An oversight here.  There is no reason to do anything with the filter rules.  I\u0027m going to take these two lines out.","commit_id":"42cf641d5718e57c806878f66121175383fcc21d"},{"author":{"_account_id":261,"name":"Salvatore Orlando","email":"salv.orlando@gmail.com","username":"salvatore-orlando"},"change_message_id":"ce846e27c0f38b19b9494c1ab61030335e74f908","unresolved":false,"context_lines":[{"line_number":423,"context_line":"        for chain, rule in self.floating_forward_rules(None, None):"},{"line_number":424,"context_line":"            ri.iptables_manager.ipv4[\u0027nat\u0027].empty_chain(chain)"},{"line_number":425,"context_line":""},{"line_number":426,"context_line":"        for c, r in self.metadata_filter_rules():"},{"line_number":427,"context_line":"            ri.iptables_manager.ipv4[\u0027filter\u0027].add_rule(c, r)"},{"line_number":428,"context_line":"        for c, r in self.metadata_nat_rules():"},{"line_number":429,"context_line":"            ri.iptables_manager.ipv4[\u0027nat\u0027].add_rule(c, r)"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAS3%2F%2Fd2E%3D","line":426,"in_reply_to":"AAAAS3%2F%2FfHg%3D","updated":"2013-09-26 08:45:53.000000000","message":"what do you mean by \"wired\"?","commit_id":"42cf641d5718e57c806878f66121175383fcc21d"},{"author":{"_account_id":261,"name":"Salvatore Orlando","email":"salv.orlando@gmail.com","username":"salvatore-orlando"},"change_message_id":"a5d6684a0340dd0002b5a0b56bab752326f625d2","unresolved":false,"context_lines":[{"line_number":424,"context_line":"            ri.iptables_manager.ipv4[\u0027nat\u0027].empty_chain(chain)"},{"line_number":425,"context_line":""},{"line_number":426,"context_line":"        for c, r in self.metadata_filter_rules():"},{"line_number":427,"context_line":"            ri.iptables_manager.ipv4[\u0027filter\u0027].add_rule(c, r)"},{"line_number":428,"context_line":"        for c, r in self.metadata_nat_rules():"},{"line_number":429,"context_line":"            ri.iptables_manager.ipv4[\u0027nat\u0027].add_rule(c, r)"},{"line_number":430,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAS3%2F%2FfnU%3D","line":427,"updated":"2013-09-25 19:02:53.000000000","message":"the previous operation won\u0027t cause the INPUT chain to be cleared, so I\u0027m not sure whether we actually need to readd it. I don\u0027t see the rule duplicated anyway when testing","commit_id":"42cf641d5718e57c806878f66121175383fcc21d"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"01a9b1b7eb0a32f32e7165ae4b9c3f0cf9cb85aa","unresolved":false,"context_lines":[{"line_number":424,"context_line":"            ri.iptables_manager.ipv4[\u0027nat\u0027].empty_chain(chain)"},{"line_number":425,"context_line":""},{"line_number":426,"context_line":"        for c, r in self.metadata_filter_rules():"},{"line_number":427,"context_line":"            ri.iptables_manager.ipv4[\u0027filter\u0027].add_rule(c, r)"},{"line_number":428,"context_line":"        for c, r in self.metadata_nat_rules():"},{"line_number":429,"context_line":"            ri.iptables_manager.ipv4[\u0027nat\u0027].add_rule(c, r)"},{"line_number":430,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAS3%2F%2FayU%3D","line":427,"in_reply_to":"AAAAS3%2F%2FcVU%3D","updated":"2013-09-27 04:02:19.000000000","message":"Done","commit_id":"42cf641d5718e57c806878f66121175383fcc21d"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"07d12e3e74d7475b5fee4093544542fb57d58429","unresolved":false,"context_lines":[{"line_number":424,"context_line":"            ri.iptables_manager.ipv4[\u0027nat\u0027].empty_chain(chain)"},{"line_number":425,"context_line":""},{"line_number":426,"context_line":"        for c, r in self.metadata_filter_rules():"},{"line_number":427,"context_line":"            ri.iptables_manager.ipv4[\u0027filter\u0027].add_rule(c, r)"},{"line_number":428,"context_line":"        for c, r in self.metadata_nat_rules():"},{"line_number":429,"context_line":"            ri.iptables_manager.ipv4[\u0027nat\u0027].add_rule(c, r)"},{"line_number":430,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAS3%2F%2FcVU%3D","line":427,"in_reply_to":"AAAAS3%2F%2FfnU%3D","updated":"2013-09-26 15:47:30.000000000","message":"Will look at this.","commit_id":"42cf641d5718e57c806878f66121175383fcc21d"},{"author":{"_account_id":261,"name":"Salvatore Orlando","email":"salv.orlando@gmail.com","username":"salvatore-orlando"},"change_message_id":"a5d6684a0340dd0002b5a0b56bab752326f625d2","unresolved":false,"context_lines":[{"line_number":426,"context_line":"        for c, r in self.metadata_filter_rules():"},{"line_number":427,"context_line":"            ri.iptables_manager.ipv4[\u0027filter\u0027].add_rule(c, r)"},{"line_number":428,"context_line":"        for c, r in self.metadata_nat_rules():"},{"line_number":429,"context_line":"            ri.iptables_manager.ipv4[\u0027nat\u0027].add_rule(c, r)"},{"line_number":430,"context_line":""},{"line_number":431,"context_line":"        existing_cidrs \u003d set([addr[\u0027cidr\u0027] for addr in device.addr.list()])"},{"line_number":432,"context_line":"        new_cidrs \u003d set()"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAS3%2F%2FfnE%3D","line":429,"updated":"2013-09-25 19:02:53.000000000","message":"If we don\u0027t clear PREROUTING, because I think we can avoid doing that, we can also avoid readding these rules.","commit_id":"42cf641d5718e57c806878f66121175383fcc21d"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"01a9b1b7eb0a32f32e7165ae4b9c3f0cf9cb85aa","unresolved":false,"context_lines":[{"line_number":426,"context_line":"        for c, r in self.metadata_filter_rules():"},{"line_number":427,"context_line":"            ri.iptables_manager.ipv4[\u0027filter\u0027].add_rule(c, r)"},{"line_number":428,"context_line":"        for c, r in self.metadata_nat_rules():"},{"line_number":429,"context_line":"            ri.iptables_manager.ipv4[\u0027nat\u0027].add_rule(c, r)"},{"line_number":430,"context_line":""},{"line_number":431,"context_line":"        existing_cidrs \u003d set([addr[\u0027cidr\u0027] for addr in device.addr.list()])"},{"line_number":432,"context_line":"        new_cidrs \u003d set()"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAS3%2F%2FayQ%3D","line":429,"in_reply_to":"AAAAS3%2F%2FcVE%3D","updated":"2013-09-27 04:02:19.000000000","message":"I still think that clearing the rules is the easiest way here.  Since floating ips get DNAT rules added to the PREROUTING chain then I want to keep this here at this time.","commit_id":"42cf641d5718e57c806878f66121175383fcc21d"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"07d12e3e74d7475b5fee4093544542fb57d58429","unresolved":false,"context_lines":[{"line_number":426,"context_line":"        for c, r in self.metadata_filter_rules():"},{"line_number":427,"context_line":"            ri.iptables_manager.ipv4[\u0027filter\u0027].add_rule(c, r)"},{"line_number":428,"context_line":"        for c, r in self.metadata_nat_rules():"},{"line_number":429,"context_line":"            ri.iptables_manager.ipv4[\u0027nat\u0027].add_rule(c, r)"},{"line_number":430,"context_line":""},{"line_number":431,"context_line":"        existing_cidrs \u003d set([addr[\u0027cidr\u0027] for addr in device.addr.list()])"},{"line_number":432,"context_line":"        new_cidrs \u003d set()"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAS3%2F%2FcVE%3D","line":429,"in_reply_to":"AAAAS3%2F%2FfnE%3D","updated":"2013-09-26 15:47:30.000000000","message":"Will look at this too.","commit_id":"42cf641d5718e57c806878f66121175383fcc21d"},{"author":{"_account_id":261,"name":"Salvatore Orlando","email":"salv.orlando@gmail.com","username":"salvatore-orlando"},"change_message_id":"a5d6684a0340dd0002b5a0b56bab752326f625d2","unresolved":false,"context_lines":[{"line_number":441,"context_line":"            if ip_cidr not in existing_cidrs:"},{"line_number":442,"context_line":"                net \u003d netaddr.IPNetwork(ip_cidr)"},{"line_number":443,"context_line":"                device.addr.add(net.version, ip_cidr, str(net.broadcast))"},{"line_number":444,"context_line":"                self._send_gratuitous_arp_packet(ri, interface_name, fip_ip)"},{"line_number":445,"context_line":""},{"line_number":446,"context_line":"            # Rebuild iptables rules for the floating ip."},{"line_number":447,"context_line":"            fixed \u003d fip[\u0027fixed_ip_address\u0027]"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAS3%2F%2Ffls%3D","line":444,"updated":"2013-09-25 19:02:53.000000000","message":"Perhaps it\u0027s the case of considering sending the arps after the iptables manager has been applied.\nThis will avoid the chance of having a few nanoseconds where the ip is advertised but not mapped.\n\nsorry for this very pedant comment.","commit_id":"42cf641d5718e57c806878f66121175383fcc21d"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"07d12e3e74d7475b5fee4093544542fb57d58429","unresolved":false,"context_lines":[{"line_number":441,"context_line":"            if ip_cidr not in existing_cidrs:"},{"line_number":442,"context_line":"                net \u003d netaddr.IPNetwork(ip_cidr)"},{"line_number":443,"context_line":"                device.addr.add(net.version, ip_cidr, str(net.broadcast))"},{"line_number":444,"context_line":"                self._send_gratuitous_arp_packet(ri, interface_name, fip_ip)"},{"line_number":445,"context_line":""},{"line_number":446,"context_line":"            # Rebuild iptables rules for the floating ip."},{"line_number":447,"context_line":"            fixed \u003d fip[\u0027fixed_ip_address\u0027]"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAS3%2F%2FcU0%3D","line":444,"in_reply_to":"AAAAS3%2F%2Ffls%3D","updated":"2013-09-26 15:47:30.000000000","message":"I see what you mean.  I will keep this in mind when I get a chance to play with the other changes.","commit_id":"42cf641d5718e57c806878f66121175383fcc21d"},{"author":{"_account_id":2031,"name":"Nachi Ueno","email":"nati.ueno@gmail.com","username":"nati-ueno"},"change_message_id":"76b73c8b481d1fce7ddbcc2c0b8af82626d6ef58","unresolved":false,"context_lines":[{"line_number":440,"context_line":"        ri.iptables_manager.apply()"},{"line_number":441,"context_line":""},{"line_number":442,"context_line":"    def process_router_floating_ips(self, ri, ex_gw_port):"},{"line_number":443,"context_line":"        \"\"\"Configure the router\u0027s floating IPs"},{"line_number":444,"context_line":"        Configures floating ips in iptables and on the router\u0027s gateway device."},{"line_number":445,"context_line":""},{"line_number":446,"context_line":"        Cleans up floating ips that should not longer be configured."}],"source_content_type":"text/x-python","patch_set":16,"id":"AAAATn%2F%2FDtA%3D","line":443,"updated":"2013-11-15 02:19:28.000000000","message":"This method doing two task. (1 setup iptables,2, set up device ip) IMO, this code could be more readable if we separate two tasks.\nsomething like this process_router_floting_ips_iptables process_router_floting_ips_device_ip","commit_id":"a65188fab01f29d095031abbc8d1d194548cd8be"},{"author":{"_account_id":2031,"name":"Nachi Ueno","email":"nati.ueno@gmail.com","username":"nati-ueno"},"change_message_id":"bc0fd60d1612ed0ccaed4ec7798bb00b5f8841fe","unresolved":false,"context_lines":[{"line_number":440,"context_line":"        ri.iptables_manager.apply()"},{"line_number":441,"context_line":""},{"line_number":442,"context_line":"    def process_router_floating_ips(self, ri, ex_gw_port):"},{"line_number":443,"context_line":"        \"\"\"Configure the router\u0027s floating IPs"},{"line_number":444,"context_line":"        Configures floating ips in iptables and on the router\u0027s gateway device."},{"line_number":445,"context_line":""},{"line_number":446,"context_line":"        Cleans up floating ips that should not longer be configured."}],"source_content_type":"text/x-python","patch_set":16,"id":"AAAATn%2F%2FDTo%3D","line":443,"in_reply_to":"AAAATn%2F%2FDUM%3D","updated":"2013-11-15 05:58:51.000000000","message":"OK I got it.","commit_id":"a65188fab01f29d095031abbc8d1d194548cd8be"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"730ea7bd59b06ac99781f3e35141c68edd64a3ad","unresolved":false,"context_lines":[{"line_number":440,"context_line":"        ri.iptables_manager.apply()"},{"line_number":441,"context_line":""},{"line_number":442,"context_line":"    def process_router_floating_ips(self, ri, ex_gw_port):"},{"line_number":443,"context_line":"        \"\"\"Configure the router\u0027s floating IPs"},{"line_number":444,"context_line":"        Configures floating ips in iptables and on the router\u0027s gateway device."},{"line_number":445,"context_line":""},{"line_number":446,"context_line":"        Cleans up floating ips that should not longer be configured."}],"source_content_type":"text/x-python","patch_set":16,"id":"AAAATn%2F%2FDUM%3D","line":443,"in_reply_to":"AAAATn%2F%2FDtA%3D","updated":"2013-11-15 05:55:35.000000000","message":"I have a refactor that separates configuration from implementation, as per an earlier review comment.  Would you be more willing to accept this patch if I posted the follow-on first?","commit_id":"a65188fab01f29d095031abbc8d1d194548cd8be"}],"neutron/agent/linux/iptables_manager.py":[{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"d0456388b032e42128d749ac867ed532476c7da5","unresolved":false,"context_lines":[{"line_number":75,"context_line":"        return ((self.chain \u003d\u003d other.chain) and"},{"line_number":76,"context_line":"                (self.rule \u003d\u003d other.rule) and"},{"line_number":77,"context_line":"                (self.top \u003d\u003d other.top) and"},{"line_number":78,"context_line":"                (self.wrap \u003d\u003d other.wrap))"},{"line_number":79,"context_line":""},{"line_number":80,"context_line":"    def __ne__(self, other):"},{"line_number":81,"context_line":"        return not self \u003d\u003d other"}],"source_content_type":"text/x-python","patch_set":12,"id":"AAAATH%2F%2FVoI%3D","line":78,"updated":"2013-10-17 15:18:17.000000000","message":"self.tag \u003d other.tag ?\n\nI don\u0027t see any removes using tag\u003d in this change, but to be complete we should put it here.","commit_id":"d240911aec8f15c9c57f4df990cff870b8851a1e"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"c38b01793b13b0a88b9ccee8a44e899bb4276146","unresolved":false,"context_lines":[{"line_number":75,"context_line":"        return ((self.chain \u003d\u003d other.chain) and"},{"line_number":76,"context_line":"                (self.rule \u003d\u003d other.rule) and"},{"line_number":77,"context_line":"                (self.top \u003d\u003d other.top) and"},{"line_number":78,"context_line":"                (self.wrap \u003d\u003d other.wrap))"},{"line_number":79,"context_line":""},{"line_number":80,"context_line":"    def __ne__(self, other):"},{"line_number":81,"context_line":"        return not self \u003d\u003d other"}],"source_content_type":"text/x-python","patch_set":12,"id":"AAAATH%2F%2FVVA%3D","line":78,"in_reply_to":"AAAATH%2F%2FVoI%3D","updated":"2013-10-17 16:07:10.000000000","message":"I tend to disagree.  I chose to leave it out because the tag doesn\u0027t really make a rule unique.  Two rules that differ only in the tag are still really the same rule.  The tag is more about who owns the rule (e.g. floating ip).\n\nWhat do others think?  I\u0027m open to feedback.\n\nOne concrete reason that I left it out is so that callers of remove_rule do not need to worry about the tag if they know what rule they want to delete.","commit_id":"d240911aec8f15c9c57f4df990cff870b8851a1e"},{"author":{"_account_id":2031,"name":"Nachi Ueno","email":"nati.ueno@gmail.com","username":"nati-ueno"},"change_message_id":"76b73c8b481d1fce7ddbcc2c0b8af82626d6ef58","unresolved":false,"context_lines":[{"line_number":63,"context_line":"    \"\"\""},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"    def __init__(self, chain, rule, wrap\u003dTrue, top\u003dFalse,"},{"line_number":66,"context_line":"                 binary_name\u003dbinary_name, tag\u003dNone):"},{"line_number":67,"context_line":"        self.chain \u003d get_chain_name(chain, wrap)"},{"line_number":68,"context_line":"        self.rule \u003d rule"},{"line_number":69,"context_line":"        self.wrap \u003d wrap"}],"source_content_type":"text/x-python","patch_set":16,"id":"AAAATn%2F%2FDs4%3D","line":66,"updated":"2013-11-15 02:19:28.000000000","message":"nice!","commit_id":"a65188fab01f29d095031abbc8d1d194548cd8be"}],"neutron/tests/unit/test_l3_agent.py":[{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"3b0a30d249f6598325eea6b9873506ae67dec5ed","unresolved":false,"context_lines":[{"line_number":368,"context_line":"        agent.process_router(ri)"},{"line_number":369,"context_line":""},{"line_number":370,"context_line":"        agent.process_router_floating_ips.assert_called_with(ri, None)"},{"line_number":371,"context_line":""},{"line_number":372,"context_line":"    @mock.patch(\u0027neutron.agent.linux.ip_lib.IPDevice\u0027)"},{"line_number":373,"context_line":"    def test_process_router_floating_ip_add(self, IPDevice):"},{"line_number":374,"context_line":"        fip \u003d {"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAARH%2F%2F0FQ%3D","line":371,"updated":"2013-08-16 20:48:28.000000000","message":"Maru,\n\nPlease annotate this file with suggestions for testing.","commit_id":"09a503b2b38459cff4ed8a6674bea892cee50cd4"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"91ea5f3046a0d963ff6dd1e41be98fafaa26b2ba","unresolved":false,"context_lines":[{"line_number":425,"context_line":"        ri.router.get.return_value \u003d [fip]"},{"line_number":426,"context_line":""},{"line_number":427,"context_line":"        agent \u003d l3_agent.L3NATAgent(HOSTNAME, self.conf)"},{"line_number":428,"context_line":"        agent._send_gratuitous_arp_packet \u003d mock.Mock()"},{"line_number":429,"context_line":""},{"line_number":430,"context_line":"        agent.process_router_floating_ips(ri, {\u0027id\u0027: _uuid()})"},{"line_number":431,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"AAAATn%2F%2FcB0%3D","line":428,"updated":"2013-11-06 07:25:06.000000000","message":"This can be removed as it is now mocked-out in setUp()","commit_id":"396b3c50135914ec7842d49a67425f402f10e949"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"ec01eb0bf4b495e69cf9eb31c85eaec8c813595c","unresolved":false,"context_lines":[{"line_number":425,"context_line":"        ri.router.get.return_value \u003d [fip]"},{"line_number":426,"context_line":""},{"line_number":427,"context_line":"        agent \u003d l3_agent.L3NATAgent(HOSTNAME, self.conf)"},{"line_number":428,"context_line":"        agent._send_gratuitous_arp_packet \u003d mock.Mock()"},{"line_number":429,"context_line":""},{"line_number":430,"context_line":"        agent.process_router_floating_ips(ri, {\u0027id\u0027: _uuid()})"},{"line_number":431,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"AAAATn%2F%2Fb3w%3D","line":428,"in_reply_to":"AAAATn%2F%2FcB0%3D","updated":"2013-11-06 08:29:18.000000000","message":"Done","commit_id":"396b3c50135914ec7842d49a67425f402f10e949"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"91ea5f3046a0d963ff6dd1e41be98fafaa26b2ba","unresolved":false,"context_lines":[{"line_number":430,"context_line":"        agent.process_router_floating_ips(ri, {\u0027id\u0027: _uuid()})"},{"line_number":431,"context_line":""},{"line_number":432,"context_line":"        device.addr.add.assert_called_once_with(4, \u002715.1.2.3/32\u0027, \u002715.1.2.3\u0027)"},{"line_number":433,"context_line":"        self.assertTrue(agent._send_gratuitous_arp_packet.called)"},{"line_number":434,"context_line":""},{"line_number":435,"context_line":"        nat \u003d ri.iptables_manager.ipv4[\u0027nat\u0027]"},{"line_number":436,"context_line":"        nat.clear_rules_by_tag.assert_called_once_with(\u0027floating_ip\u0027)"}],"source_content_type":"text/x-python","patch_set":15,"id":"AAAATn%2F%2FcAk%3D","line":433,"updated":"2013-11-06 07:25:06.000000000","message":"This should be something like:\n\ninterface_name \u003d agent.get_internal_device_name(_uuid())\n\nself.send_arp.assert_called_once_with(ri, interface_name, \u002715.1.2.3\u0027)\n\nSimilar changes in the other places.","commit_id":"396b3c50135914ec7842d49a67425f402f10e949"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"ec01eb0bf4b495e69cf9eb31c85eaec8c813595c","unresolved":false,"context_lines":[{"line_number":430,"context_line":"        agent.process_router_floating_ips(ri, {\u0027id\u0027: _uuid()})"},{"line_number":431,"context_line":""},{"line_number":432,"context_line":"        device.addr.add.assert_called_once_with(4, \u002715.1.2.3/32\u0027, \u002715.1.2.3\u0027)"},{"line_number":433,"context_line":"        self.assertTrue(agent._send_gratuitous_arp_packet.called)"},{"line_number":434,"context_line":""},{"line_number":435,"context_line":"        nat \u003d ri.iptables_manager.ipv4[\u0027nat\u0027]"},{"line_number":436,"context_line":"        nat.clear_rules_by_tag.assert_called_once_with(\u0027floating_ip\u0027)"}],"source_content_type":"text/x-python","patch_set":15,"id":"AAAATn%2F%2Fb3s%3D","line":433,"in_reply_to":"AAAATn%2F%2FcAk%3D","updated":"2013-11-06 08:29:18.000000000","message":"I\u0027m going to take this out since there are other tests that test the send gratuitous arp functionality.  I\u0027d rather this test be focused on the specific functionality that it is testing.","commit_id":"396b3c50135914ec7842d49a67425f402f10e949"}]}
