)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":2711,"name":"Zhang Hua","display_name":"Zhang Hua","email":"joshua.zhang@canonical.com","username":"zhhuabj"},"change_message_id":"d1aace740ddd7200c1a4e5066cf7227312e03c95","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Add HA support to the l3 agent"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This patch is a part of the blueprint l3-high-availability"},{"line_number":10,"context_line":""},{"line_number":11,"context_line":"Still WIP, have to improve unit tests and"},{"line_number":12,"context_line":"functionnal tests."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"AAAAVn%2F%2FSgI%3D","line":9,"updated":"2014-03-07 01:49:57.000000000","message":"nit: Partially Implements: blueprint l3-high-availability","commit_id":"2e440c30adc54997be4360c3da294bd7be8548fd"},{"author":{"_account_id":2733,"name":"Darragh O\u0027Reilly","email":"doreilly@suse.com","username":"darragh-oreilly"},"change_message_id":"c84a8e76295825da5f7873eb31448df67df14791","unresolved":false,"context_lines":[{"line_number":9,"context_line":"This patch is a part of the blueprint l3-high-availability."},{"line_number":10,"context_line":""},{"line_number":11,"context_line":"Future Patches:"},{"line_number":12,"context_line":"- Agent side implementation of conversion of router from/to HA"},{"line_number":13,"context_line":"- conntrackd"},{"line_number":14,"context_line":"- Report HA state (active / standby) for each HA router via API"},{"line_number":15,"context_line":"- Add ability to migrate HA routers from network node by"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":38,"id":"fa98f980_94006435","line":12,"updated":"2014-09-04 14:56:36.000000000","message":"should this be disallowed at the api level until then?","commit_id":"2a9108692a88233f615db45143723037e0833e68"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"cfbe36f9ec96a20662fb8019bddda8449997427a","unresolved":false,"context_lines":[{"line_number":9,"context_line":"This patch is a part of the blueprint l3-high-availability."},{"line_number":10,"context_line":""},{"line_number":11,"context_line":"Future Patches:"},{"line_number":12,"context_line":"- Agent side implementation of conversion of router from/to HA"},{"line_number":13,"context_line":"- conntrackd"},{"line_number":14,"context_line":"- Report HA state (active / standby) for each HA router via API"},{"line_number":15,"context_line":"- Add ability to migrate HA routers from network node by"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":38,"id":"fa98f980_a22fd83d","line":12,"in_reply_to":"fa98f980_94006435","updated":"2014-09-07 14:08:44.000000000","message":"It will be in the next patchset to the server patch.","commit_id":"2a9108692a88233f615db45143723037e0833e68"},{"author":{"_account_id":2733,"name":"Darragh O\u0027Reilly","email":"doreilly@suse.com","username":"darragh-oreilly"},"change_message_id":"c84a8e76295825da5f7873eb31448df67df14791","unresolved":false,"context_lines":[{"line_number":10,"context_line":""},{"line_number":11,"context_line":"Future Patches:"},{"line_number":12,"context_line":"- Agent side implementation of conversion of router from/to HA"},{"line_number":13,"context_line":"- conntrackd"},{"line_number":14,"context_line":"- Report HA state (active / standby) for each HA router via API"},{"line_number":15,"context_line":"- Add ability to migrate HA routers from network node by"},{"line_number":16,"context_line":"  setting admin_state down on the l3 agent"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":38,"id":"fa98f980_4e382b3d","line":13,"updated":"2014-09-04 14:56:36.000000000","message":"I\u0027m surprised to find that tcp connections using floating ips survive a failover without this conntrack replication patch.","commit_id":"2a9108692a88233f615db45143723037e0833e68"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"46c6f8f8ab75b827ee9084a475fa1a8b77990e81","unresolved":false,"context_lines":[{"line_number":10,"context_line":""},{"line_number":11,"context_line":"Future Patches:"},{"line_number":12,"context_line":"- Agent side implementation of conversion of router from/to HA"},{"line_number":13,"context_line":"- conntrackd"},{"line_number":14,"context_line":"- Report HA state (active / standby) for each HA router via API"},{"line_number":15,"context_line":"- Add ability to migrate HA routers from network node by"},{"line_number":16,"context_line":"  setting admin_state down on the l3 agent"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":38,"id":"fa98f980_85ba69df","line":13,"in_reply_to":"fa98f980_376df704","updated":"2014-09-09 15:01:26.000000000","message":"(sorry to be commenting on PS38, but I was pointed to this comment by Carl)\n\nDarragh,\n\nIs the reason this is working due to this setting?\n\nnet.netfilter.nf_conntrack_tcp_loose \u003d 1\n\nnf_conntrack_tcp_loose - BOOLEAN\n        0 - disabled\n        not 0 - enabled (default)\n\n        If it is set to zero, we disable picking up already established\n        connections.\n\nThat\u0027s going to allow a packet without a SYN to create a conntrack entry.  Without it things like moving a router from one l3-agent to another will drop all the TCP connections, with it things will most likely just keep working.","commit_id":"2a9108692a88233f615db45143723037e0833e68"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"b5f3345a5bc5d34cbed91605a2a4aa2c20bc5102","unresolved":false,"context_lines":[{"line_number":10,"context_line":""},{"line_number":11,"context_line":"Future Patches:"},{"line_number":12,"context_line":"- Agent side implementation of conversion of router from/to HA"},{"line_number":13,"context_line":"- conntrackd"},{"line_number":14,"context_line":"- Report HA state (active / standby) for each HA router via API"},{"line_number":15,"context_line":"- Add ability to migrate HA routers from network node by"},{"line_number":16,"context_line":"  setting admin_state down on the l3 agent"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":38,"id":"fa98f980_6b1f2f0a","line":13,"in_reply_to":"fa98f980_376df704","updated":"2014-09-09 00:07:23.000000000","message":"Darragh, this is a good point that seems kind of obvious to me now that you\u0027ve pointed it out.  :)\n\nThe floating ip nat rules in the router don\u0027t really have any dependence on connection tracking at all.  In fact, connection tracking is kind of wasted on floating ips.  The DNAT rules will apply to incoming traffic regardless of the connection state or which side initiated the connection.\n\nConnection tracking is only import for traffic originating from a VM without a floating ip where the default SNAT rule will be used.  This is consistent with the behavior you discovered.\n\nOverall, exactly the behaviors that you described here are what we should expect.","commit_id":"2a9108692a88233f615db45143723037e0833e68"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"d796c5361f860729d3e4c8786338ea19b65ccf4c","unresolved":false,"context_lines":[{"line_number":10,"context_line":""},{"line_number":11,"context_line":"Future Patches:"},{"line_number":12,"context_line":"- Agent side implementation of conversion of router from/to HA"},{"line_number":13,"context_line":"- conntrackd"},{"line_number":14,"context_line":"- Report HA state (active / standby) for each HA router via API"},{"line_number":15,"context_line":"- Add ability to migrate HA routers from network node by"},{"line_number":16,"context_line":"  setting admin_state down on the l3 agent"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":38,"id":"fa98f980_4f5bc5f5","line":13,"in_reply_to":"fa98f980_4e382b3d","updated":"2014-09-04 15:05:33.000000000","message":"This is very interesting - How exactly did you test this?","commit_id":"2a9108692a88233f615db45143723037e0833e68"},{"author":{"_account_id":2733,"name":"Darragh O\u0027Reilly","email":"doreilly@suse.com","username":"darragh-oreilly"},"change_message_id":"3b5705c5aa61dcff7a8040deb85db4b94351a345","unresolved":false,"context_lines":[{"line_number":10,"context_line":""},{"line_number":11,"context_line":"Future Patches:"},{"line_number":12,"context_line":"- Agent side implementation of conversion of router from/to HA"},{"line_number":13,"context_line":"- conntrackd"},{"line_number":14,"context_line":"- Report HA state (active / standby) for each HA router via API"},{"line_number":15,"context_line":"- Add ability to migrate HA routers from network node by"},{"line_number":16,"context_line":"  setting admin_state down on the l3 agent"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":38,"id":"fa98f980_376df704","line":13,"in_reply_to":"fa98f980_4f5bc5f5","updated":"2014-09-04 17:02:03.000000000","message":"I\u0027m running a controller+compute+dhcp node and two l3-agent nodes (all on virtualbox vms, ovs and vxlan). Steps:\n\nCreate a ha router, add internal net, set gateway, boot vm, add floating ip.\nSsh from outside into vm.\nObserve the ip addresses and conntrack entries in namespace on node running the active side.\nObserve absence of these on the other node.\nPower down the node hosting the active side.\nObserve the standby node becomes active and the ips appear in its namespace.\nSsh connection stays intact. \n\n\nI\u0027m still trying to understand this, but I think it\u0027s because the iptables rules in the router namespace do not try to match specific states like RELATED,ESTABLISHED (unlike those for security groups on the compute nodes). \n\nDid other things like create netcat connections like this:\n\noutside$ nc -l 172.24.4.1 3001\n\ninside$ nc 172.24.4.1 3001\n\nThen type stuff on one side and it appears on the other. When using a floating ip, after failover this still works (the nc processes stay running on both sides), even if you type stuff on the outside first - I guess the first packet hits the dnat rule and a new connection is created.\n\nThen tried with an snat connection out (no floating ip). Failover and start typing on the inside, and it works, and then typing on the outside and it also works. But failover and start typing on the outside and the connection breaks. It is an snat that must be made from the inside out, but a tcp SYN packet does not seem to be required to start the connection.","commit_id":"2a9108692a88233f615db45143723037e0833e68"},{"author":{"_account_id":2733,"name":"Darragh O\u0027Reilly","email":"doreilly@suse.com","username":"darragh-oreilly"},"change_message_id":"f5834e68100f72db4a1e610ca455de818e77b455","unresolved":false,"context_lines":[{"line_number":10,"context_line":""},{"line_number":11,"context_line":"Future Patches:"},{"line_number":12,"context_line":"- Agent side implementation of conversion of router from/to HA"},{"line_number":13,"context_line":"- conntrackd"},{"line_number":14,"context_line":"- Report HA state (active / standby) for each HA router via API"},{"line_number":15,"context_line":"- Add ability to migrate HA routers from network node by"},{"line_number":16,"context_line":"  setting admin_state down on the l3 agent"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":38,"id":"fa98f980_58ca27a2","line":13,"in_reply_to":"fa98f980_85ba69df","updated":"2014-09-09 17:09:41.000000000","message":"Brian, yes that was set to 1 in the qrouter namespace. As a test, I set it to 0 on standby router namespace, started an ssh to the floating ip, turned off the node with the active router, it failed over and the ssh connection broke. Was able to start a new ssh.","commit_id":"2a9108692a88233f615db45143723037e0833e68"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"6ae9705ab2c6b34ab669cab590f0f7c3fe620af8","unresolved":false,"context_lines":[{"line_number":25,"context_line":""},{"line_number":26,"context_line":"Change-Id: I83f2a5d2af42164c42773b385ba7b00872eed54e"},{"line_number":27,"context_line":"Implements: blueprint l3-high-availability"},{"line_number":28,"context_line":"Co-Authored-By: Assaf Muller \u003camuller@redhat.com\u003e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":42,"id":"fa98f980_36990e6f","line":28,"updated":"2014-09-09 00:50:35.000000000","message":"nit:  You\u0027re the author and the co-author.  Busy man!","commit_id":"000f08d0f1e8d1801da8f654f400c4dc3ba3e544"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"ef99423b2582ce5749a0375e41f0c135ff21d857","unresolved":false,"context_lines":[{"line_number":25,"context_line":""},{"line_number":26,"context_line":"Change-Id: I83f2a5d2af42164c42773b385ba7b00872eed54e"},{"line_number":27,"context_line":"Implements: blueprint l3-high-availability"},{"line_number":28,"context_line":"Co-Authored-By: Assaf Muller \u003camuller@redhat.com\u003e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":42,"id":"fa98f980_f793fd4c","line":28,"in_reply_to":"fa98f980_36990e6f","updated":"2014-09-09 05:50:36.000000000","message":"Lol, I was about to say the same, may be must change the author to sylvain, or the other way around :)","commit_id":"000f08d0f1e8d1801da8f654f400c4dc3ba3e544"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"8a679dc72394d47a70fa28401f92dc0087f790ff","unresolved":false,"context_lines":[{"line_number":25,"context_line":""},{"line_number":26,"context_line":"Change-Id: I83f2a5d2af42164c42773b385ba7b00872eed54e"},{"line_number":27,"context_line":"Implements: blueprint l3-high-availability"},{"line_number":28,"context_line":"Co-Authored-By: Assaf Muller \u003camuller@redhat.com\u003e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":42,"id":"fa98f980_9909ff2e","line":28,"in_reply_to":"fa98f980_f793fd4c","updated":"2014-09-09 11:46:39.000000000","message":"I don\u0027t know when this happened but I guess the author changed some time when I cherry picked or changed branches or whatever \u003d/","commit_id":"000f08d0f1e8d1801da8f654f400c4dc3ba3e544"}],"neutron/agent/l3_agent.py":[{"author":{"_account_id":2711,"name":"Zhang Hua","display_name":"Zhang Hua","email":"joshua.zhang@canonical.com","username":"zhhuabj"},"change_message_id":"81166899156b60a963a389beccb1c8f97cb66da9","unresolved":false,"context_lines":[{"line_number":562,"context_line":"        instance \u003d keepalived.KeepalivedInstance("},{"line_number":563,"context_line":"            HA_VIRTUAL_ROUTER_NAME_PREFIX + str(ri.ha_vr_id),"},{"line_number":564,"context_line":"            \u0027BACKUP\u0027, interface_name, ri.ha_vr_id,"},{"line_number":565,"context_line":"            ri.ha_port[\u0027priority\u0027], nopreempt\u003dTrue,"},{"line_number":566,"context_line":"            advert_int\u003dself.conf.ha_vrrp_advert_int)"},{"line_number":567,"context_line":"        instance.track_interfaces.append(interface_name)"},{"line_number":568,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAVn%2F%2Fa%2Fs%3D","line":565,"updated":"2014-03-06 07:04:15.000000000","message":"I think backup will preempt as master after master is dead, but master shouldn\u0027t preemp as master again after it comes again.\nso for master, status\u003dBACKUP + nopreempt\u003dTrue + high priority; for backup, status\u003dBACKUP + nopreempt\u003dFalse + low priority\nany idea ?","commit_id":"caedee128f7232efc339153f8611405a47aa3fb1"},{"author":{"_account_id":2711,"name":"Zhang Hua","display_name":"Zhang Hua","email":"joshua.zhang@canonical.com","username":"zhhuabj"},"change_message_id":"f3517d17ea8b3921c73859422aa04c21cbb65c69","unresolved":false,"context_lines":[{"line_number":562,"context_line":"        instance \u003d keepalived.KeepalivedInstance("},{"line_number":563,"context_line":"            HA_VIRTUAL_ROUTER_NAME_PREFIX + str(ri.ha_vr_id),"},{"line_number":564,"context_line":"            \u0027BACKUP\u0027, interface_name, ri.ha_vr_id,"},{"line_number":565,"context_line":"            ri.ha_port[\u0027priority\u0027], nopreempt\u003dTrue,"},{"line_number":566,"context_line":"            advert_int\u003dself.conf.ha_vrrp_advert_int)"},{"line_number":567,"context_line":"        instance.track_interfaces.append(interface_name)"},{"line_number":568,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAVn%2F%2FSuU%3D","line":565,"in_reply_to":"AAAAVn%2F%2FZr4%3D","updated":"2014-03-07 01:28:31.000000000","message":"hi, Sylvain, thanks for clarifing, you are right, one article I saw is wrong which confused me, just now I did an experiment which can prove it will work well in the case of changing both server state as BACKUP, primary having higher priority and with nopreempt, both having the same router ID. below is my configurations:\n\nvrrp_instance master_ins {\n   state BACKUP\n   nopreempt\n   interface wlp3s0\n   virtual_router_id 1\n   priority 250\n   advert_int 2\n   virtual_ipaddress {\n     192.168.99.122/24\n   } \n}\n\n\nvrrp_instance backup_ins {\n   state BACKUP\n   nopreempt\n   interface wlp2s6\n   virtual_router_id 1\n   priority 249\n   advert_int 2\n   virtual_ipaddress {\n     192.168.99.122/24\n   }\n}","commit_id":"caedee128f7232efc339153f8611405a47aa3fb1"},{"author":{"_account_id":7141,"name":"Sylvain Afchain","email":"safchain@redhat.com","username":"sylvainA"},"change_message_id":"db718fc7a7f4f009f4e6b6c81736a8630b6efcd4","unresolved":false,"context_lines":[{"line_number":562,"context_line":"        instance \u003d keepalived.KeepalivedInstance("},{"line_number":563,"context_line":"            HA_VIRTUAL_ROUTER_NAME_PREFIX + str(ri.ha_vr_id),"},{"line_number":564,"context_line":"            \u0027BACKUP\u0027, interface_name, ri.ha_vr_id,"},{"line_number":565,"context_line":"            ri.ha_port[\u0027priority\u0027], nopreempt\u003dTrue,"},{"line_number":566,"context_line":"            advert_int\u003dself.conf.ha_vrrp_advert_int)"},{"line_number":567,"context_line":"        instance.track_interfaces.append(interface_name)"},{"line_number":568,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAVn%2F%2FZr4%3D","line":565,"in_reply_to":"AAAAVn%2F%2Fa%2Fs%3D","updated":"2014-03-06 11:09:10.000000000","message":"After reading the man page :\n\n\"\"\"\n\nVRRP will normally preempt a lower priority\nmachine when a higher priority machine comes\nonline.  \"nopreempt\" allows the lower priority\nmachine to maintain the master role, even when\na higher priority machine comes back online.\nNOTE: For this to work, the initial state of this\nentry must be BACKUP.\n\n\"\"\"\n\nSo I think all nodes should be in BACKUP state with nopreempt as True, No ?","commit_id":"caedee128f7232efc339153f8611405a47aa3fb1"},{"author":{"_account_id":2711,"name":"Zhang Hua","display_name":"Zhang Hua","email":"joshua.zhang@canonical.com","username":"zhhuabj"},"change_message_id":"81166899156b60a963a389beccb1c8f97cb66da9","unresolved":false,"context_lines":[{"line_number":894,"context_line":"            if ri.is_ha():"},{"line_number":895,"context_line":"                instance \u003d ri.keepalived_manager.config.get_instance("},{"line_number":896,"context_line":"                    HA_VIRTUAL_ROUTER_NAME_PREFIX + str(ri.ha_vr_id))"},{"line_number":897,"context_line":"                vroute \u003d keepalived.KeepalivedVirtualRoute("},{"line_number":898,"context_line":"                    \u00270.0.0.0/0\u0027, gw_ip, interface_name)"},{"line_number":899,"context_line":"                instance.virtual_routes.append(vroute)"},{"line_number":900,"context_line":"            else:"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAVn%2F%2Fa4s%3D","line":897,"updated":"2014-03-06 07:04:15.000000000","message":"not sure if the configuration item virtual_routes of keeplived support namespace, does here will work well when enabling use_namespace\u003dTrue ?","commit_id":"caedee128f7232efc339153f8611405a47aa3fb1"},{"author":{"_account_id":7141,"name":"Sylvain Afchain","email":"safchain@redhat.com","username":"sylvainA"},"change_message_id":"6a9ca0b8c2f1bf6437c6122b376ccb7bb07511eb","unresolved":false,"context_lines":[{"line_number":894,"context_line":"            if ri.is_ha():"},{"line_number":895,"context_line":"                instance \u003d ri.keepalived_manager.config.get_instance("},{"line_number":896,"context_line":"                    HA_VIRTUAL_ROUTER_NAME_PREFIX + str(ri.ha_vr_id))"},{"line_number":897,"context_line":"                vroute \u003d keepalived.KeepalivedVirtualRoute("},{"line_number":898,"context_line":"                    \u00270.0.0.0/0\u0027, gw_ip, interface_name)"},{"line_number":899,"context_line":"                instance.virtual_routes.append(vroute)"},{"line_number":900,"context_line":"            else:"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAVn%2F%2FOpM%3D","line":897,"in_reply_to":"AAAAVn%2F%2FSaM%3D","updated":"2014-03-07 13:21:37.000000000","message":"Not sure to understand, Do you mean that you are wondering if the vrrp traffic going through the ha interface will not be tracked by conntrackd ?","commit_id":"caedee128f7232efc339153f8611405a47aa3fb1"},{"author":{"_account_id":2711,"name":"Zhang Hua","display_name":"Zhang Hua","email":"joshua.zhang@canonical.com","username":"zhhuabj"},"change_message_id":"e08b48184e39585d3e30485d80b2c5bef9917476","unresolved":false,"context_lines":[{"line_number":894,"context_line":"            if ri.is_ha():"},{"line_number":895,"context_line":"                instance \u003d ri.keepalived_manager.config.get_instance("},{"line_number":896,"context_line":"                    HA_VIRTUAL_ROUTER_NAME_PREFIX + str(ri.ha_vr_id))"},{"line_number":897,"context_line":"                vroute \u003d keepalived.KeepalivedVirtualRoute("},{"line_number":898,"context_line":"                    \u00270.0.0.0/0\u0027, gw_ip, interface_name)"},{"line_number":899,"context_line":"                instance.virtual_routes.append(vroute)"},{"line_number":900,"context_line":"            else:"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAVn%2F%2FSaM%3D","line":897,"in_reply_to":"AAAAVn%2F%2FShQ%3D","updated":"2014-03-07 01:49:42.000000000","message":"following the above, just a new question in my mind, maybe I\u0027m wrong, I guess the keepalived will execute a RAW PREROUTING rule after adding route_ha_interface ports into \u0027interface\u0027 configuration item to separate different tcp flows to improve conntrack\u0027s performance (just guess, not sure, I don\u0027t know how keepalived implement this yet ):\niptables -t raw -A PREROUTING ! –I $router_ha_interface_port -j NOTRACK\nso if it\u0027s necesseary to add below rules as well ?\niptables –t raw –A OUTPUT –j NOTRACK \niptables -A FORWARD -m state --state UNTRACKED -j ACCEPT","commit_id":"caedee128f7232efc339153f8611405a47aa3fb1"},{"author":{"_account_id":2711,"name":"Zhang Hua","display_name":"Zhang Hua","email":"joshua.zhang@canonical.com","username":"zhhuabj"},"change_message_id":"5fd957c9390abe8441ac174b7e8516c7503c0416","unresolved":false,"context_lines":[{"line_number":894,"context_line":"            if ri.is_ha():"},{"line_number":895,"context_line":"                instance \u003d ri.keepalived_manager.config.get_instance("},{"line_number":896,"context_line":"                    HA_VIRTUAL_ROUTER_NAME_PREFIX + str(ri.ha_vr_id))"},{"line_number":897,"context_line":"                vroute \u003d keepalived.KeepalivedVirtualRoute("},{"line_number":898,"context_line":"                    \u00270.0.0.0/0\u0027, gw_ip, interface_name)"},{"line_number":899,"context_line":"                instance.virtual_routes.append(vroute)"},{"line_number":900,"context_line":"            else:"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAVn%2F%2FRFE%3D","line":897,"in_reply_to":"AAAAVn%2F%2FShQ%3D","updated":"2014-03-07 06:24:16.000000000","message":"keepalived has -f parameter (/opt/stack/data/neutron/ha_confs/\u003crouter id\u003e/keepalived.conf ) to support every router namespace to boot one keepalived instance, got it, thanks.","commit_id":"caedee128f7232efc339153f8611405a47aa3fb1"},{"author":{"_account_id":2711,"name":"Zhang Hua","display_name":"Zhang Hua","email":"joshua.zhang@canonical.com","username":"zhhuabj"},"change_message_id":"f3517d17ea8b3921c73859422aa04c21cbb65c69","unresolved":false,"context_lines":[{"line_number":894,"context_line":"            if ri.is_ha():"},{"line_number":895,"context_line":"                instance \u003d ri.keepalived_manager.config.get_instance("},{"line_number":896,"context_line":"                    HA_VIRTUAL_ROUTER_NAME_PREFIX + str(ri.ha_vr_id))"},{"line_number":897,"context_line":"                vroute \u003d keepalived.KeepalivedVirtualRoute("},{"line_number":898,"context_line":"                    \u00270.0.0.0/0\u0027, gw_ip, interface_name)"},{"line_number":899,"context_line":"                instance.virtual_routes.append(vroute)"},{"line_number":900,"context_line":"            else:"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAVn%2F%2FShQ%3D","line":897,"in_reply_to":"AAAAVn%2F%2FZaM%3D","updated":"2014-03-07 01:28:31.000000000","message":"Hi Sylvain, are you saying the router_ha_interface ports across different namesapce has been added into \u0027interface\u0027 configuration item of keepalived, if yes, got it.\nInitially, I mistake keeplived itself can pass and specify the ns_name when execting the \u0027route\u0027 command or you plan to use \u0027mount --bind\u0027 to implement similiar usage by yourself.\nthanks for your clarify","commit_id":"caedee128f7232efc339153f8611405a47aa3fb1"},{"author":{"_account_id":7141,"name":"Sylvain Afchain","email":"safchain@redhat.com","username":"sylvainA"},"change_message_id":"db718fc7a7f4f009f4e6b6c81736a8630b6efcd4","unresolved":false,"context_lines":[{"line_number":894,"context_line":"            if ri.is_ha():"},{"line_number":895,"context_line":"                instance \u003d ri.keepalived_manager.config.get_instance("},{"line_number":896,"context_line":"                    HA_VIRTUAL_ROUTER_NAME_PREFIX + str(ri.ha_vr_id))"},{"line_number":897,"context_line":"                vroute \u003d keepalived.KeepalivedVirtualRoute("},{"line_number":898,"context_line":"                    \u00270.0.0.0/0\u0027, gw_ip, interface_name)"},{"line_number":899,"context_line":"                instance.virtual_routes.append(vroute)"},{"line_number":900,"context_line":"            else:"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAVn%2F%2FZaM%3D","line":897,"in_reply_to":"AAAAVn%2F%2Fa4s%3D","updated":"2014-03-06 11:09:10.000000000","message":"Yes since there is a keepalived instance per router and each instance is running into router namespace.","commit_id":"caedee128f7232efc339153f8611405a47aa3fb1"},{"author":{"_account_id":2711,"name":"Zhang Hua","display_name":"Zhang Hua","email":"joshua.zhang@canonical.com","username":"zhhuabj"},"change_message_id":"81166899156b60a963a389beccb1c8f97cb66da9","unresolved":false,"context_lines":[{"line_number":957,"context_line":"        ip_address \u003d internal_cidr.split(\u0027/\u0027)[0]"},{"line_number":958,"context_line":""},{"line_number":959,"context_line":"        if ri.is_ha():"},{"line_number":960,"context_line":"            self._add_keepalived_vip(ri, internal_cidr, interface_name)"},{"line_number":961,"context_line":"        else:"},{"line_number":962,"context_line":"            self.driver.init_l3(interface_name, [internal_cidr],"},{"line_number":963,"context_line":"                                namespace\u003dri.ns_name())"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAVn%2F%2Fa5w%3D","line":960,"updated":"2014-03-06 07:04:15.000000000","message":"I know for HA port it will use \u0027l3_ha_net_cidr\u003d172.31.255.0/24\u0027 use default HA subnet, but for the internal gw port in different l3-agent maybe they will have the same IP. just want to know whether it will cause the problem for this case having the same IP in multiple ports ?","commit_id":"caedee128f7232efc339153f8611405a47aa3fb1"},{"author":{"_account_id":2711,"name":"Zhang Hua","display_name":"Zhang Hua","email":"joshua.zhang@canonical.com","username":"zhhuabj"},"change_message_id":"bdeb1a675bdaeb45819fa3508aa58a2b6bcbae5c","unresolved":false,"context_lines":[{"line_number":957,"context_line":"        ip_address \u003d internal_cidr.split(\u0027/\u0027)[0]"},{"line_number":958,"context_line":""},{"line_number":959,"context_line":"        if ri.is_ha():"},{"line_number":960,"context_line":"            self._add_keepalived_vip(ri, internal_cidr, interface_name)"},{"line_number":961,"context_line":"        else:"},{"line_number":962,"context_line":"            self.driver.init_l3(interface_name, [internal_cidr],"},{"line_number":963,"context_line":"                                namespace\u003dri.ns_name())"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAVn%2F%2FaLM%3D","line":960,"in_reply_to":"AAAAVn%2F%2Fa5w%3D","updated":"2014-03-06 08:59:34.000000000","message":"sorry, just now mistake on this, now I have understood it, thanks.","commit_id":"caedee128f7232efc339153f8611405a47aa3fb1"},{"author":{"_account_id":2711,"name":"Zhang Hua","display_name":"Zhang Hua","email":"joshua.zhang@canonical.com","username":"zhhuabj"},"change_message_id":"81166899156b60a963a389beccb1c8f97cb66da9","unresolved":false,"context_lines":[{"line_number":1084,"context_line":""},{"line_number":1085,"context_line":"    def _router_ids(self):"},{"line_number":1086,"context_line":"        if not self.conf.use_namespaces:"},{"line_number":1087,"context_line":"            return [self.conf.router_id]"},{"line_number":1088,"context_line":""},{"line_number":1089,"context_line":"    @periodic_task.periodic_task"},{"line_number":1090,"context_line":"    @lockutils.synchronized(\u0027l3-agent\u0027, \u0027neutron-\u0027)"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAVn%2F%2Fa%2Bo%3D","line":1087,"updated":"2014-03-06 07:04:15.000000000","message":"I can\u0027t search the class ConntrackdManager in the https://review.openstack.org/#/c/71586/5/neutron/agent/linux/conntrackd_manager.py, just want to know where we should use ConntrackdManager ?","commit_id":"caedee128f7232efc339153f8611405a47aa3fb1"},{"author":{"_account_id":2711,"name":"Zhang Hua","display_name":"Zhang Hua","email":"joshua.zhang@canonical.com","username":"zhhuabj"},"change_message_id":"f3517d17ea8b3921c73859422aa04c21cbb65c69","unresolved":false,"context_lines":[{"line_number":1084,"context_line":""},{"line_number":1085,"context_line":"    def _router_ids(self):"},{"line_number":1086,"context_line":"        if not self.conf.use_namespaces:"},{"line_number":1087,"context_line":"            return [self.conf.router_id]"},{"line_number":1088,"context_line":""},{"line_number":1089,"context_line":"    @periodic_task.periodic_task"},{"line_number":1090,"context_line":"    @lockutils.synchronized(\u0027l3-agent\u0027, \u0027neutron-\u0027)"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAVn%2F%2FSuA%3D","line":1087,"in_reply_to":"AAAAVn%2F%2FZNg%3D","updated":"2014-03-07 01:28:31.000000000","message":"got it, thanks.","commit_id":"caedee128f7232efc339153f8611405a47aa3fb1"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"26f812fb880c64cb094db8c6d535620fbca5148e","unresolved":false,"context_lines":[{"line_number":1084,"context_line":""},{"line_number":1085,"context_line":"    def _router_ids(self):"},{"line_number":1086,"context_line":"        if not self.conf.use_namespaces:"},{"line_number":1087,"context_line":"            return [self.conf.router_id]"},{"line_number":1088,"context_line":""},{"line_number":1089,"context_line":"    @periodic_task.periodic_task"},{"line_number":1090,"context_line":"    @lockutils.synchronized(\u0027l3-agent\u0027, \u0027neutron-\u0027)"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAVn%2F%2FZNg%3D","line":1087,"in_reply_to":"AAAAVn%2F%2FZZs%3D","updated":"2014-03-06 11:38:58.000000000","message":"Here\u0027s the conntrackd patch:\n\nhttps://review.openstack.org/#/c/71586/","commit_id":"caedee128f7232efc339153f8611405a47aa3fb1"},{"author":{"_account_id":7141,"name":"Sylvain Afchain","email":"safchain@redhat.com","username":"sylvainA"},"change_message_id":"db718fc7a7f4f009f4e6b6c81736a8630b6efcd4","unresolved":false,"context_lines":[{"line_number":1084,"context_line":""},{"line_number":1085,"context_line":"    def _router_ids(self):"},{"line_number":1086,"context_line":"        if not self.conf.use_namespaces:"},{"line_number":1087,"context_line":"            return [self.conf.router_id]"},{"line_number":1088,"context_line":""},{"line_number":1089,"context_line":"    @periodic_task.periodic_task"},{"line_number":1090,"context_line":"    @lockutils.synchronized(\u0027l3-agent\u0027, \u0027neutron-\u0027)"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAVn%2F%2FZZs%3D","line":1087,"in_reply_to":"AAAAVn%2F%2Fa%2Bo%3D","updated":"2014-03-06 11:09:10.000000000","message":"I plan to introduce  conntrackd in another patch depending of this one in order to keep this one as simple as possible.","commit_id":"caedee128f7232efc339153f8611405a47aa3fb1"},{"author":{"_account_id":6788,"name":"Rossella Sblendido","email":"rsblendido@suse.com","username":"rossella-o"},"change_message_id":"c2c1b9d7d5da2f84db50041d4e42443f9dc6a5b8","unresolved":false,"context_lines":[{"line_number":411,"context_line":"        if self.conf.enable_metadata_proxy:"},{"line_number":412,"context_line":"            self._spawn_metadata_proxy(ri)"},{"line_number":413,"context_line":""},{"line_number":414,"context_line":"        if ri.is_ha():"},{"line_number":415,"context_line":"            m, b, f \u003d ri.keepalived_notifiers.get_notifiers_path()"},{"line_number":416,"context_line":"            group \u003d ri.keepalived_manager.config.get_group("},{"line_number":417,"context_line":"                HA_VIRTUAL_ROUTER_GROUP_PREFIX + str(ri.ha_vr_id))"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAVn%2F%2FBf4%3D","line":414,"updated":"2014-03-10 17:02:09.000000000","message":"Any reason why you added a second \u0027if\u0027 here and didn\u0027t put line 415 to 418 under the \u0027if\u0027 of line 408? Do you need the metadata proxy to be spawn before, doesn\u0027t seem related from a quick view.","commit_id":"5fa07dbe323b31e678babe007e043c88425d2e93"},{"author":{"_account_id":7141,"name":"Sylvain Afchain","email":"safchain@redhat.com","username":"sylvainA"},"change_message_id":"1b570f6eb18153b89d7bdfb63c9b903554c2fa76","unresolved":false,"context_lines":[{"line_number":411,"context_line":"        if self.conf.enable_metadata_proxy:"},{"line_number":412,"context_line":"            self._spawn_metadata_proxy(ri)"},{"line_number":413,"context_line":""},{"line_number":414,"context_line":"        if ri.is_ha():"},{"line_number":415,"context_line":"            m, b, f \u003d ri.keepalived_notifiers.get_notifiers_path()"},{"line_number":416,"context_line":"            group \u003d ri.keepalived_manager.config.get_group("},{"line_number":417,"context_line":"                HA_VIRTUAL_ROUTER_GROUP_PREFIX + str(ri.ha_vr_id))"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAVn%2F%2FA60%3D","line":414,"in_reply_to":"AAAAVn%2F%2FBf4%3D","updated":"2014-03-10 17:52:53.000000000","message":"Yes because keepalived notifiers are built in the _spawn_metadata_proxy, then add to the group here.","commit_id":"5fa07dbe323b31e678babe007e043c88425d2e93"},{"author":{"_account_id":6788,"name":"Rossella Sblendido","email":"rsblendido@suse.com","username":"rossella-o"},"change_message_id":"c2c1b9d7d5da2f84db50041d4e42443f9dc6a5b8","unresolved":false,"context_lines":[{"line_number":599,"context_line":""},{"line_number":600,"context_line":"    def process_router(self, ri):"},{"line_number":601,"context_line":"        ri.iptables_manager.defer_apply_on()"},{"line_number":602,"context_line":""},{"line_number":603,"context_line":"        ex_gw_port \u003d self._get_ex_gw_port(ri)"},{"line_number":604,"context_line":"        internal_ports \u003d ri.router.get(l3_constants.INTERFACE_KEY, [])"},{"line_number":605,"context_line":"        existing_port_ids \u003d set([p[\u0027id\u0027] for p in ri.internal_ports])"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAVn%2F%2FBUU%3D","line":602,"updated":"2014-03-10 17:02:09.000000000","message":"nit: remove blank line","commit_id":"5fa07dbe323b31e678babe007e043c88425d2e93"},{"author":{"_account_id":6788,"name":"Rossella Sblendido","email":"rsblendido@suse.com","username":"rossella-o"},"change_message_id":"c2c1b9d7d5da2f84db50041d4e42443f9dc6a5b8","unresolved":false,"context_lines":[{"line_number":753,"context_line":"        vip_addresses.sort(key\u003dlambda vip: vip.ip_address)"},{"line_number":754,"context_line":""},{"line_number":755,"context_line":"        vip_address \u003d vip_addresses.pop(0)"},{"line_number":756,"context_line":"        instance.vip_addresses \u003d [vip_address]"},{"line_number":757,"context_line":""},{"line_number":758,"context_line":"        if vip_addresses:"},{"line_number":759,"context_line":"            instance.vip_addresses_excluded \u003d vip_addresses"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAVn%2F%2FBSY%3D","line":756,"updated":"2014-03-10 17:02:09.000000000","message":"I am confused, why are you re-assigning the vip to the instance?","commit_id":"5fa07dbe323b31e678babe007e043c88425d2e93"},{"author":{"_account_id":7141,"name":"Sylvain Afchain","email":"safchain@redhat.com","username":"sylvainA"},"change_message_id":"1b570f6eb18153b89d7bdfb63c9b903554c2fa76","unresolved":false,"context_lines":[{"line_number":753,"context_line":"        vip_addresses.sort(key\u003dlambda vip: vip.ip_address)"},{"line_number":754,"context_line":""},{"line_number":755,"context_line":"        vip_address \u003d vip_addresses.pop(0)"},{"line_number":756,"context_line":"        instance.vip_addresses \u003d [vip_address]"},{"line_number":757,"context_line":""},{"line_number":758,"context_line":"        if vip_addresses:"},{"line_number":759,"context_line":"            instance.vip_addresses_excluded \u003d vip_addresses"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAVn%2F%2FA3k%3D","line":756,"in_reply_to":"AAAAVn%2F%2FBSY%3D","updated":"2014-03-10 17:52:53.000000000","message":"We always have to keep only 1 IP in this list which is the first one (sorted) of the merge of the instance.vip_addresses and instance.vip_addresses_excluded. All remaining IPs will be pushed in the instance.vip_addresses_excluded.\n\nThe vip_address \"re-assigned\" couldn\u0027t be the same, could has been removed line #746","commit_id":"5fa07dbe323b31e678babe007e043c88425d2e93"},{"author":{"_account_id":2874,"name":"yong sheng gong","email":"gong.yongsheng@99cloud.net","username":"gongysh"},"change_message_id":"bb26bb0618031509f2d1c0c29a7934849c13a43b","unresolved":false,"context_lines":[{"line_number":4,"context_line":"#    not use this file except in compliance with the License. You may obtain"},{"line_number":5,"context_line":"#    a copy of the License at"},{"line_number":6,"context_line":"#"},{"line_number":7,"context_line":"#         http://www.apache.org/licenses/LICENSE-2.0"},{"line_number":8,"context_line":"#"},{"line_number":9,"context_line":"#    Unless required by applicable law or agreed to in writing, software"},{"line_number":10,"context_line":"#    distributed under the License is distributed on an \"AS IS\" BASIS, WITHOUT"}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAAW3%2F%2F9ek%3D","line":7,"updated":"2014-04-09 08:29:05.000000000","message":"I think we can separate the ha related stuff into one mixin class in another file.","commit_id":"b90e816c17fe466369a784dda156537d42fc862d"},{"author":{"_account_id":7141,"name":"Sylvain Afchain","email":"safchain@redhat.com","username":"sylvainA"},"change_message_id":"1a4ba7bd07633c3d901e297cff6c991916f0ac04","unresolved":false,"context_lines":[{"line_number":4,"context_line":"#    not use this file except in compliance with the License. You may obtain"},{"line_number":5,"context_line":"#    a copy of the License at"},{"line_number":6,"context_line":"#"},{"line_number":7,"context_line":"#         http://www.apache.org/licenses/LICENSE-2.0"},{"line_number":8,"context_line":"#"},{"line_number":9,"context_line":"#    Unless required by applicable law or agreed to in writing, software"},{"line_number":10,"context_line":"#    distributed under the License is distributed on an \"AS IS\" BASIS, WITHOUT"}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAAW3%2F%2FwU0%3D","line":7,"in_reply_to":"AAAAW3%2F%2F9ek%3D","updated":"2014-04-10 16:11:49.000000000","message":"I agree with you","commit_id":"b90e816c17fe466369a784dda156537d42fc862d"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"d961807f618d9ef1d691c31820c96a8137f74653","unresolved":false,"context_lines":[{"line_number":4,"context_line":"#    not use this file except in compliance with the License. You may obtain"},{"line_number":5,"context_line":"#    a copy of the License at"},{"line_number":6,"context_line":"#"},{"line_number":7,"context_line":"#         http://www.apache.org/licenses/LICENSE-2.0"},{"line_number":8,"context_line":"#"},{"line_number":9,"context_line":"#    Unless required by applicable law or agreed to in writing, software"},{"line_number":10,"context_line":"#    distributed under the License is distributed on an \"AS IS\" BASIS, WITHOUT"}],"source_content_type":"text/x-python","patch_set":9,"id":"baada198_fe27a0c4","line":7,"in_reply_to":"AAAAW3%2F%2FwU0%3D","updated":"2014-07-15 14:59:53.000000000","message":"+100\n\nWe *have* to do this - This file is getting insanely complicated and DVR will only make that worse. We need to find a way to abstract away some of the complexity.\n\nA related note is that the L3 HA implementation should be using a keepalive driver - Right now it\u0027s tightly coupled. This was kind of hinted at by the spec.","commit_id":"b90e816c17fe466369a784dda156537d42fc862d"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"f2249f67776bf3c75bda4398124064a93b739d3f","unresolved":false,"context_lines":[{"line_number":4,"context_line":"#    not use this file except in compliance with the License. You may obtain"},{"line_number":5,"context_line":"#    a copy of the License at"},{"line_number":6,"context_line":"#"},{"line_number":7,"context_line":"#         http://www.apache.org/licenses/LICENSE-2.0"},{"line_number":8,"context_line":"#"},{"line_number":9,"context_line":"#    Unless required by applicable law or agreed to in writing, software"},{"line_number":10,"context_line":"#    distributed under the License is distributed on an \"AS IS\" BASIS, WITHOUT"}],"source_content_type":"text/x-python","patch_set":9,"id":"baada198_2f8e61d1","line":7,"in_reply_to":"baada198_2a6376de","updated":"2014-08-06 21:00:22.000000000","message":"I moved all of the HA stuff to a new module. No driver abstraction layer for now.","commit_id":"b90e816c17fe466369a784dda156537d42fc862d"},{"author":{"_account_id":7141,"name":"Sylvain Afchain","email":"safchain@redhat.com","username":"sylvainA"},"change_message_id":"ffa18c193bc650438e0560d0dfc68c723356ef18","unresolved":false,"context_lines":[{"line_number":4,"context_line":"#    not use this file except in compliance with the License. You may obtain"},{"line_number":5,"context_line":"#    a copy of the License at"},{"line_number":6,"context_line":"#"},{"line_number":7,"context_line":"#         http://www.apache.org/licenses/LICENSE-2.0"},{"line_number":8,"context_line":"#"},{"line_number":9,"context_line":"#    Unless required by applicable law or agreed to in writing, software"},{"line_number":10,"context_line":"#    distributed under the License is distributed on an \"AS IS\" BASIS, WITHOUT"}],"source_content_type":"text/x-python","patch_set":9,"id":"baada198_2a6376de","line":7,"in_reply_to":"baada198_fe27a0c4","updated":"2014-07-16 08:16:24.000000000","message":"I\u0027m still on this line, that\u0027s why the specs is about implementing an abstraction with drivers. This patch-set is an old one before the specs and the DVR, has to be reworked for sure.","commit_id":"b90e816c17fe466369a784dda156537d42fc862d"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"511a81f62deb80cbdfd69c04e0de3a4030ed46ef","unresolved":false,"context_lines":[{"line_number":105,"context_line":"class RouterInfo(object):"},{"line_number":106,"context_line":""},{"line_number":107,"context_line":"    def __init__(self, router_id, root_helper, use_namespaces, router,"},{"line_number":108,"context_line":"                 ha_confs_path\u003dNone):"},{"line_number":109,"context_line":"        self.router_id \u003d router_id"},{"line_number":110,"context_line":"        self.ex_gw_port \u003d None"},{"line_number":111,"context_line":"        self._snat_enabled \u003d None"}],"source_content_type":"text/x-python","patch_set":9,"id":"baada198_1b1ef80d","line":108,"updated":"2014-07-24 15:13:37.000000000","message":"Seems like RouterInfo doesn\u0027t use the new argument yet.","commit_id":"b90e816c17fe466369a784dda156537d42fc862d"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"f2249f67776bf3c75bda4398124064a93b739d3f","unresolved":false,"context_lines":[{"line_number":105,"context_line":"class RouterInfo(object):"},{"line_number":106,"context_line":""},{"line_number":107,"context_line":"    def __init__(self, router_id, root_helper, use_namespaces, router,"},{"line_number":108,"context_line":"                 ha_confs_path\u003dNone):"},{"line_number":109,"context_line":"        self.router_id \u003d router_id"},{"line_number":110,"context_line":"        self.ex_gw_port \u003d None"},{"line_number":111,"context_line":"        self._snat_enabled \u003d None"}],"source_content_type":"text/x-python","patch_set":9,"id":"baada198_b61028f5","line":108,"in_reply_to":"baada198_1b1ef80d","updated":"2014-08-06 21:00:22.000000000","message":"Done","commit_id":"b90e816c17fe466369a784dda156537d42fc862d"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"d961807f618d9ef1d691c31820c96a8137f74653","unresolved":false,"context_lines":[{"line_number":157,"context_line":"        self._snat_action \u003d None"},{"line_number":158,"context_line":""},{"line_number":159,"context_line":"    def is_ha(self):"},{"line_number":160,"context_line":"        return ((self.router is not None) and"},{"line_number":161,"context_line":"                self.router.get(\u0027ha_vr_id\u0027))"},{"line_number":162,"context_line":""},{"line_number":163,"context_line":"    @property"}],"source_content_type":"text/x-python","patch_set":9,"id":"baada198_fbfb32bb","line":160,"updated":"2014-07-15 14:59:53.000000000","message":"You can probably check router.ha now with the change to the plugin side patch?","commit_id":"b90e816c17fe466369a784dda156537d42fc862d"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"f2249f67776bf3c75bda4398124064a93b739d3f","unresolved":false,"context_lines":[{"line_number":157,"context_line":"        self._snat_action \u003d None"},{"line_number":158,"context_line":""},{"line_number":159,"context_line":"    def is_ha(self):"},{"line_number":160,"context_line":"        return ((self.router is not None) and"},{"line_number":161,"context_line":"                self.router.get(\u0027ha_vr_id\u0027))"},{"line_number":162,"context_line":""},{"line_number":163,"context_line":"    @property"}],"source_content_type":"text/x-python","patch_set":9,"id":"baada198_56942c43","line":160,"in_reply_to":"baada198_8a80024b","updated":"2014-08-06 21:00:22.000000000","message":"Done","commit_id":"b90e816c17fe466369a784dda156537d42fc862d"},{"author":{"_account_id":7141,"name":"Sylvain Afchain","email":"safchain@redhat.com","username":"sylvainA"},"change_message_id":"ffa18c193bc650438e0560d0dfc68c723356ef18","unresolved":false,"context_lines":[{"line_number":157,"context_line":"        self._snat_action \u003d None"},{"line_number":158,"context_line":""},{"line_number":159,"context_line":"    def is_ha(self):"},{"line_number":160,"context_line":"        return ((self.router is not None) and"},{"line_number":161,"context_line":"                self.router.get(\u0027ha_vr_id\u0027))"},{"line_number":162,"context_line":""},{"line_number":163,"context_line":"    @property"}],"source_content_type":"text/x-python","patch_set":9,"id":"baada198_8a80024b","line":160,"in_reply_to":"baada198_fbfb32bb","updated":"2014-07-16 08:16:24.000000000","message":"Right","commit_id":"b90e816c17fe466369a784dda156537d42fc862d"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"d961807f618d9ef1d691c31820c96a8137f74653","unresolved":false,"context_lines":[{"line_number":162,"context_line":""},{"line_number":163,"context_line":"    @property"},{"line_number":164,"context_line":"    def ha_vr_id(self):"},{"line_number":165,"context_line":"        return self.router.get(\u0027ha_vr_id\u0027)"},{"line_number":166,"context_line":""},{"line_number":167,"context_line":""},{"line_number":168,"context_line":"class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback, manager.Manager):"}],"source_content_type":"text/x-python","patch_set":9,"id":"baada198_19338e14","line":165,"updated":"2014-07-15 14:59:53.000000000","message":"What do you think about adding a method called \"is_ha_active\" that would be:\nreturn self.is_ha() and self.ha_port is not None\n?\n\nThen you could use that in line 1102 (For example) - I think it would improve readability.","commit_id":"b90e816c17fe466369a784dda156537d42fc862d"},{"author":{"_account_id":7141,"name":"Sylvain Afchain","email":"safchain@redhat.com","username":"sylvainA"},"change_message_id":"ffa18c193bc650438e0560d0dfc68c723356ef18","unresolved":false,"context_lines":[{"line_number":162,"context_line":""},{"line_number":163,"context_line":"    @property"},{"line_number":164,"context_line":"    def ha_vr_id(self):"},{"line_number":165,"context_line":"        return self.router.get(\u0027ha_vr_id\u0027)"},{"line_number":166,"context_line":""},{"line_number":167,"context_line":""},{"line_number":168,"context_line":"class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback, manager.Manager):"}],"source_content_type":"text/x-python","patch_set":9,"id":"baada198_aa85c65a","line":165,"in_reply_to":"baada198_19338e14","updated":"2014-07-16 08:16:24.000000000","message":"ok","commit_id":"b90e816c17fe466369a784dda156537d42fc862d"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"f2249f67776bf3c75bda4398124064a93b739d3f","unresolved":false,"context_lines":[{"line_number":162,"context_line":""},{"line_number":163,"context_line":"    @property"},{"line_number":164,"context_line":"    def ha_vr_id(self):"},{"line_number":165,"context_line":"        return self.router.get(\u0027ha_vr_id\u0027)"},{"line_number":166,"context_line":""},{"line_number":167,"context_line":""},{"line_number":168,"context_line":"class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback, manager.Manager):"}],"source_content_type":"text/x-python","patch_set":9,"id":"baada198_768f702e","line":165,"in_reply_to":"baada198_aa85c65a","updated":"2014-08-06 21:00:22.000000000","message":"This seems unneeded on 2nd thought.","commit_id":"b90e816c17fe466369a784dda156537d42fc862d"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"d961807f618d9ef1d691c31820c96a8137f74653","unresolved":false,"context_lines":[{"line_number":216,"context_line":"                   default\u003d\u0027PASS\u0027,"},{"line_number":217,"context_line":"                   help\u003d_(\u0027VRRP authentication type AH/PASS\u0027)),"},{"line_number":218,"context_line":"        cfg.StrOpt(\u0027ha_vrrp_auth_password\u0027,"},{"line_number":219,"context_line":"                   help\u003d_(\u0027VRRP authentication password\u0027)),"},{"line_number":220,"context_line":"        cfg.ListOpt(\u0027ha_notification_emails\u0027,"},{"line_number":221,"context_line":"                    help\u003d_(\u0027Email accounts that will receive the \u0027"},{"line_number":222,"context_line":"                           \u0027notification mail\u0027)),"}],"source_content_type":"text/x-python","patch_set":9,"id":"baada198_bbe37a03","line":219,"updated":"2014-07-15 14:59:53.000000000","message":"Do we really want to support all of these mail options?","commit_id":"b90e816c17fe466369a784dda156537d42fc862d"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"f2249f67776bf3c75bda4398124064a93b739d3f","unresolved":false,"context_lines":[{"line_number":216,"context_line":"                   default\u003d\u0027PASS\u0027,"},{"line_number":217,"context_line":"                   help\u003d_(\u0027VRRP authentication type AH/PASS\u0027)),"},{"line_number":218,"context_line":"        cfg.StrOpt(\u0027ha_vrrp_auth_password\u0027,"},{"line_number":219,"context_line":"                   help\u003d_(\u0027VRRP authentication password\u0027)),"},{"line_number":220,"context_line":"        cfg.ListOpt(\u0027ha_notification_emails\u0027,"},{"line_number":221,"context_line":"                    help\u003d_(\u0027Email accounts that will receive the \u0027"},{"line_number":222,"context_line":"                           \u0027notification mail\u0027)),"}],"source_content_type":"text/x-python","patch_set":9,"id":"baada198_969a446d","line":219,"in_reply_to":"baada198_bbe37a03","updated":"2014-08-06 21:00:22.000000000","message":"I\u0027m going to remove the email related options.","commit_id":"b90e816c17fe466369a784dda156537d42fc862d"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"511a81f62deb80cbdfd69c04e0de3a4030ed46ef","unresolved":false,"context_lines":[{"line_number":231,"context_line":"                   help\u003d_(\u0027Timeout for SMTP stream processing\u0027)),"},{"line_number":232,"context_line":"        cfg.IntOpt(\u0027ha_vrrp_advert_int\u0027,"},{"line_number":233,"context_line":"                   default\u003d2,"},{"line_number":234,"context_line":"                   help\u003d_(\u0027The advertisement interval in seconds\u0027)),"},{"line_number":235,"context_line":"        cfg.IntOpt(\u0027ha_garp_master_delay\u0027,"},{"line_number":236,"context_line":"                   help\u003d_(\u0027Delay for gratuitous ARP after \u0027"},{"line_number":237,"context_line":"                          \u0027transition to MASTER\u0027)),"}],"source_content_type":"text/x-python","patch_set":9,"id":"baada198_51e5af36","line":234,"updated":"2014-07-24 15:13:37.000000000","message":"Is there a reason to expose this configuration option?","commit_id":"b90e816c17fe466369a784dda156537d42fc862d"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"f2249f67776bf3c75bda4398124064a93b739d3f","unresolved":false,"context_lines":[{"line_number":231,"context_line":"                   help\u003d_(\u0027Timeout for SMTP stream processing\u0027)),"},{"line_number":232,"context_line":"        cfg.IntOpt(\u0027ha_vrrp_advert_int\u0027,"},{"line_number":233,"context_line":"                   default\u003d2,"},{"line_number":234,"context_line":"                   help\u003d_(\u0027The advertisement interval in seconds\u0027)),"},{"line_number":235,"context_line":"        cfg.IntOpt(\u0027ha_garp_master_delay\u0027,"},{"line_number":236,"context_line":"                   help\u003d_(\u0027Delay for gratuitous ARP after \u0027"},{"line_number":237,"context_line":"                          \u0027transition to MASTER\u0027)),"}],"source_content_type":"text/x-python","patch_set":9,"id":"baada198_16ac7487","line":234,"in_reply_to":"baada198_51e5af36","updated":"2014-08-06 21:00:22.000000000","message":"Removing this - keepalived will use the default of 5 seconds.","commit_id":"b90e816c17fe466369a784dda156537d42fc862d"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"511a81f62deb80cbdfd69c04e0de3a4030ed46ef","unresolved":false,"context_lines":[{"line_number":393,"context_line":""},{"line_number":394,"context_line":"    def _router_added(self, router_id, router):"},{"line_number":395,"context_line":"        ri \u003d RouterInfo(router_id, self.root_helper,"},{"line_number":396,"context_line":"                        self.conf.use_namespaces, router,"},{"line_number":397,"context_line":"                        self.conf.ha_confs_path)"},{"line_number":398,"context_line":"        self.router_info[router_id] \u003d ri"},{"line_number":399,"context_line":"        if self.conf.use_namespaces:"}],"source_content_type":"text/x-python","patch_set":9,"id":"baada198_b161abaa","line":396,"updated":"2014-07-24 15:13:37.000000000","message":"The new argument is unused in this method?","commit_id":"b90e816c17fe466369a784dda156537d42fc862d"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"f2249f67776bf3c75bda4398124064a93b739d3f","unresolved":false,"context_lines":[{"line_number":393,"context_line":""},{"line_number":394,"context_line":"    def _router_added(self, router_id, router):"},{"line_number":395,"context_line":"        ri \u003d RouterInfo(router_id, self.root_helper,"},{"line_number":396,"context_line":"                        self.conf.use_namespaces, router,"},{"line_number":397,"context_line":"                        self.conf.ha_confs_path)"},{"line_number":398,"context_line":"        self.router_info[router_id] \u003d ri"},{"line_number":399,"context_line":"        if self.conf.use_namespaces:"}],"source_content_type":"text/x-python","patch_set":9,"id":"baada198_16fab476","line":396,"in_reply_to":"baada198_b161abaa","updated":"2014-08-06 21:00:22.000000000","message":"Done","commit_id":"b90e816c17fe466369a784dda156537d42fc862d"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"511a81f62deb80cbdfd69c04e0de3a4030ed46ef","unresolved":false,"context_lines":[{"line_number":434,"context_line":"            self._destroy_metadata_proxy(ri.router_id, ri.ns_name())"},{"line_number":435,"context_line":"        del self.router_info[router_id]"},{"line_number":436,"context_line":"        self._destroy_router_namespace(ri.ns_name())"},{"line_number":437,"context_line":""},{"line_number":438,"context_line":"    def _spawn_metadata_proxy(self, ri):"},{"line_number":439,"context_line":"        router_id, ns_name \u003d ri.router_id, ri.ns_name()"},{"line_number":440,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"baada198_51012fae","line":437,"updated":"2014-07-24 15:13:37.000000000","message":"This change seems unrelated to the patch.","commit_id":"b90e816c17fe466369a784dda156537d42fc862d"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"f2249f67776bf3c75bda4398124064a93b739d3f","unresolved":false,"context_lines":[{"line_number":434,"context_line":"            self._destroy_metadata_proxy(ri.router_id, ri.ns_name())"},{"line_number":435,"context_line":"        del self.router_info[router_id]"},{"line_number":436,"context_line":"        self._destroy_router_namespace(ri.ns_name())"},{"line_number":437,"context_line":""},{"line_number":438,"context_line":"    def _spawn_metadata_proxy(self, ri):"},{"line_number":439,"context_line":"        router_id, ns_name \u003d ri.router_id, ri.ns_name()"},{"line_number":440,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"baada198_563eec29","line":437,"in_reply_to":"baada198_51012fae","updated":"2014-08-06 21:00:22.000000000","message":"Nevermind, the change is needed because we know need access to ri.is_ha, ri.keepalived_notifiers, ri.keepalived_manager, as well as the id and ns name.","commit_id":"b90e816c17fe466369a784dda156537d42fc862d"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"d961807f618d9ef1d691c31820c96a8137f74653","unresolved":false,"context_lines":[{"line_number":457,"context_line":"            self.root_helper,"},{"line_number":458,"context_line":"            ns_name)"},{"line_number":459,"context_line":""},{"line_number":460,"context_line":"        if ri.is_ha():"},{"line_number":461,"context_line":"            pid_file \u003d pm.get_pid_file_name(ensure_pids_dir\u003dTrue)"},{"line_number":462,"context_line":"            cmd \u003d callback(pid_file)"},{"line_number":463,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"baada198_de557c1e","line":460,"updated":"2014-07-15 14:59:53.000000000","message":"This block of code can be made into a function.","commit_id":"b90e816c17fe466369a784dda156537d42fc862d"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"f2249f67776bf3c75bda4398124064a93b739d3f","unresolved":false,"context_lines":[{"line_number":457,"context_line":"            self.root_helper,"},{"line_number":458,"context_line":"            ns_name)"},{"line_number":459,"context_line":""},{"line_number":460,"context_line":"        if ri.is_ha():"},{"line_number":461,"context_line":"            pid_file \u003d pm.get_pid_file_name(ensure_pids_dir\u003dTrue)"},{"line_number":462,"context_line":"            cmd \u003d callback(pid_file)"},{"line_number":463,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"baada198_3659788d","line":460,"in_reply_to":"baada198_de557c1e","updated":"2014-08-06 21:00:22.000000000","message":"Moving this to the keepalived manager (Previous patch in series).","commit_id":"b90e816c17fe466369a784dda156537d42fc862d"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"d961807f618d9ef1d691c31820c96a8137f74653","unresolved":false,"context_lines":[{"line_number":462,"context_line":"            cmd \u003d callback(pid_file)"},{"line_number":463,"context_line":""},{"line_number":464,"context_line":"            ri.keepalived_notifiers.add_notify(\u0027master\u0027, \u0027 \u0027.join(cmd) + \u0027 \u0026\u0027)"},{"line_number":465,"context_line":""},{"line_number":466,"context_line":"            cmd \u003d [\u0027kill\u0027, \u0027-9\u0027, \u0027$(cat \u0027 + pid_file + \u0027)\u0027]"},{"line_number":467,"context_line":"            ri.keepalived_notifiers.add_notify(\u0027backup\u0027, \u0027 \u0027.join(cmd))"},{"line_number":468,"context_line":"            ri.keepalived_notifiers.add_notify(\u0027fault\u0027, \u0027 \u0027.join(cmd))"}],"source_content_type":"text/x-python","patch_set":9,"id":"baada198_4c0666cf","line":465,"updated":"2014-07-15 14:59:53.000000000","message":"I\u0027d use a different variable here other than \u0027cmd\u0027, it\u0027s slightly confusing at first to a simpleton like me.","commit_id":"b90e816c17fe466369a784dda156537d42fc862d"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"f2249f67776bf3c75bda4398124064a93b739d3f","unresolved":false,"context_lines":[{"line_number":462,"context_line":"            cmd \u003d callback(pid_file)"},{"line_number":463,"context_line":""},{"line_number":464,"context_line":"            ri.keepalived_notifiers.add_notify(\u0027master\u0027, \u0027 \u0027.join(cmd) + \u0027 \u0026\u0027)"},{"line_number":465,"context_line":""},{"line_number":466,"context_line":"            cmd \u003d [\u0027kill\u0027, \u0027-9\u0027, \u0027$(cat \u0027 + pid_file + \u0027)\u0027]"},{"line_number":467,"context_line":"            ri.keepalived_notifiers.add_notify(\u0027backup\u0027, \u0027 \u0027.join(cmd))"},{"line_number":468,"context_line":"            ri.keepalived_notifiers.add_notify(\u0027fault\u0027, \u0027 \u0027.join(cmd))"}],"source_content_type":"text/x-python","patch_set":9,"id":"baada198_f61460a0","line":465,"in_reply_to":"baada198_4c0666cf","updated":"2014-08-06 21:00:22.000000000","message":"Done","commit_id":"b90e816c17fe466369a784dda156537d42fc862d"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"d961807f618d9ef1d691c31820c96a8137f74653","unresolved":false,"context_lines":[{"line_number":495,"context_line":"    def get_ha_device_name(self, port_id):"},{"line_number":496,"context_line":"        return (HA_DEV_PREFIX + port_id)[:self.driver.DEV_NAME_LEN]"},{"line_number":497,"context_line":""},{"line_number":498,"context_line":"    def ha_network_added(self, ri, network_id, port_id,"},{"line_number":499,"context_line":"                         internal_cidr, mac_address,"},{"line_number":500,"context_line":"                         set_address\u003dFalse):"},{"line_number":501,"context_line":"        interface_name \u003d self.get_ha_device_name(port_id)"}],"source_content_type":"text/x-python","patch_set":9,"id":"baada198_393cd292","line":498,"updated":"2014-07-15 14:59:53.000000000","message":"Lines 498 to 509 are pretty much in common with 952 to 961 - We could probably make a helper function for that. Same for ha_network_removed.","commit_id":"b90e816c17fe466369a784dda156537d42fc862d"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"f2249f67776bf3c75bda4398124064a93b739d3f","unresolved":false,"context_lines":[{"line_number":495,"context_line":"    def get_ha_device_name(self, port_id):"},{"line_number":496,"context_line":"        return (HA_DEV_PREFIX + port_id)[:self.driver.DEV_NAME_LEN]"},{"line_number":497,"context_line":""},{"line_number":498,"context_line":"    def ha_network_added(self, ri, network_id, port_id,"},{"line_number":499,"context_line":"                         internal_cidr, mac_address,"},{"line_number":500,"context_line":"                         set_address\u003dFalse):"},{"line_number":501,"context_line":"        interface_name \u003d self.get_ha_device_name(port_id)"}],"source_content_type":"text/x-python","patch_set":9,"id":"baada198_7b3a8536","line":498,"in_reply_to":"baada198_393cd292","updated":"2014-08-06 21:00:22.000000000","message":"Done","commit_id":"b90e816c17fe466369a784dda156537d42fc862d"},{"author":{"_account_id":11280,"name":"Jun Wu","email":"quark@lihdd.net","username":"quark"},"change_message_id":"5130722e006b1ff63477919541c9f9d431d3c818","unresolved":false,"context_lines":[{"line_number":525,"context_line":"            LOG.warn(_(\"Unable to process HA router without ha port\"))"},{"line_number":526,"context_line":"            return"},{"line_number":527,"context_line":""},{"line_number":528,"context_line":"        if len(ha_ports) \u003e 1:"},{"line_number":529,"context_line":"            LOG.warn(_(\"A l3 agent can\u0027t handle more than once HA port\"))"},{"line_number":530,"context_line":"        ha_port \u003d ha_ports[0]"},{"line_number":531,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"baada198_fb709209","line":528,"updated":"2014-07-08 08:16:43.000000000","message":"Because ha_ports can only has one element, how about checking it server side (fails eairlier)? The logic here could be much cleaner:\n\n  # here\n    ha_port \u003d ri.ha_port\n  # class RouterInfo(object):\n    @property\n    def ha_port(self):\n        return self.router.get(\u0027ha_port\u0027)\n\nThe reason I think this is better is it helps to reduce unnecessary state (routerInfo.ha_port).","commit_id":"b90e816c17fe466369a784dda156537d42fc862d"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"f2249f67776bf3c75bda4398124064a93b739d3f","unresolved":false,"context_lines":[{"line_number":525,"context_line":"            LOG.warn(_(\"Unable to process HA router without ha port\"))"},{"line_number":526,"context_line":"            return"},{"line_number":527,"context_line":""},{"line_number":528,"context_line":"        if len(ha_ports) \u003e 1:"},{"line_number":529,"context_line":"            LOG.warn(_(\"A l3 agent can\u0027t handle more than once HA port\"))"},{"line_number":530,"context_line":"        ha_port \u003d ha_ports[0]"},{"line_number":531,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"baada198_8da72009","line":528,"in_reply_to":"baada198_fb709209","updated":"2014-08-06 21:00:22.000000000","message":"I think this makes sense if it isn\u0027t too complicated server-side, which I think it isn\u0027t. I\u0027ll add it as a review comment:\nhttps://review.openstack.org/#/c/64553/55/neutron/db/l3_hamode_db.py","commit_id":"b90e816c17fe466369a784dda156537d42fc862d"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"511a81f62deb80cbdfd69c04e0de3a4030ed46ef","unresolved":false,"context_lines":[{"line_number":541,"context_line":"        self._destroy_keepalived(ri)"},{"line_number":542,"context_line":"        self.ha_network_removed(ri)"},{"line_number":543,"context_line":"        ri.ha_port \u003d None"},{"line_number":544,"context_line":""},{"line_number":545,"context_line":"    def _init_keepalived_notifiers(self, ri):"},{"line_number":546,"context_line":"        ri.keepalived_notifiers \u003d ("},{"line_number":547,"context_line":"            keepalived.KeepalivedNotifyScriptManager("}],"source_content_type":"text/x-python","patch_set":9,"id":"baada198_116b3720","line":544,"updated":"2014-07-24 15:13:37.000000000","message":"_init_keepalived_notifiers and manager should be methods of the RouterInfo class and not of the agent in my opinion.","commit_id":"b90e816c17fe466369a784dda156537d42fc862d"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"f2249f67776bf3c75bda4398124064a93b739d3f","unresolved":false,"context_lines":[{"line_number":541,"context_line":"        self._destroy_keepalived(ri)"},{"line_number":542,"context_line":"        self.ha_network_removed(ri)"},{"line_number":543,"context_line":"        ri.ha_port \u003d None"},{"line_number":544,"context_line":""},{"line_number":545,"context_line":"    def _init_keepalived_notifiers(self, ri):"},{"line_number":546,"context_line":"        ri.keepalived_notifiers \u003d ("},{"line_number":547,"context_line":"            keepalived.KeepalivedNotifyScriptManager("}],"source_content_type":"text/x-python","patch_set":9,"id":"baada198_72e83c16","line":544,"in_reply_to":"baada198_116b3720","updated":"2014-08-06 21:00:22.000000000","message":"Done","commit_id":"b90e816c17fe466369a784dda156537d42fc862d"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"511a81f62deb80cbdfd69c04e0de3a4030ed46ef","unresolved":false,"context_lines":[{"line_number":556,"context_line":"            root_helper\u003dself.root_helper)"},{"line_number":557,"context_line":""},{"line_number":558,"context_line":"        config \u003d ri.keepalived_manager.config"},{"line_number":559,"context_line":""},{"line_number":560,"context_line":"        global_defs \u003d keepalived.KeepalivedGlobaldefs()"},{"line_number":561,"context_line":"        global_defs.set_smtp_server(self.conf.ha_smtp_server,"},{"line_number":562,"context_line":"                                    self.conf.ha_smtp_timeout)"}],"source_content_type":"text/x-python","patch_set":9,"id":"baada198_71f7135a","line":559,"updated":"2014-07-24 15:13:37.000000000","message":"Why not add all of the information that the Globaldefs needs to its initialization? I mean send ha_smtp_server, timeout and the rest of the parameters to Globaldefs.__init__.","commit_id":"b90e816c17fe466369a784dda156537d42fc862d"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"f2249f67776bf3c75bda4398124064a93b739d3f","unresolved":false,"context_lines":[{"line_number":556,"context_line":"            root_helper\u003dself.root_helper)"},{"line_number":557,"context_line":""},{"line_number":558,"context_line":"        config \u003d ri.keepalived_manager.config"},{"line_number":559,"context_line":""},{"line_number":560,"context_line":"        global_defs \u003d keepalived.KeepalivedGlobaldefs()"},{"line_number":561,"context_line":"        global_defs.set_smtp_server(self.conf.ha_smtp_server,"},{"line_number":562,"context_line":"                                    self.conf.ha_smtp_timeout)"}],"source_content_type":"text/x-python","patch_set":9,"id":"baada198_b2c57491","line":559,"in_reply_to":"baada198_71f7135a","updated":"2014-08-06 21:00:22.000000000","message":"Done","commit_id":"b90e816c17fe466369a784dda156537d42fc862d"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"511a81f62deb80cbdfd69c04e0de3a4030ed46ef","unresolved":false,"context_lines":[{"line_number":677,"context_line":"        ri.ex_gw_port \u003d ex_gw_port"},{"line_number":678,"context_line":"        ri.enable_snat \u003d ri.router.get(\u0027enable_snat\u0027)"},{"line_number":679,"context_line":""},{"line_number":680,"context_line":"        if ri.is_ha():"},{"line_number":681,"context_line":"            self._spawn_keepalived(ri)"},{"line_number":682,"context_line":""},{"line_number":683,"context_line":"    def _handle_router_snat_rules(self, ri, ex_gw_port, internal_cidrs,"}],"source_content_type":"text/x-python","patch_set":9,"id":"baada198_d4560dce","line":680,"updated":"2014-07-24 15:13:37.000000000","message":"Hmm keep in mind that process_router is called from _router_removed as well, so it seems problematic to spawn keepalived here.","commit_id":"b90e816c17fe466369a784dda156537d42fc862d"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"f2249f67776bf3c75bda4398124064a93b739d3f","unresolved":false,"context_lines":[{"line_number":677,"context_line":"        ri.ex_gw_port \u003d ex_gw_port"},{"line_number":678,"context_line":"        ri.enable_snat \u003d ri.router.get(\u0027enable_snat\u0027)"},{"line_number":679,"context_line":""},{"line_number":680,"context_line":"        if ri.is_ha():"},{"line_number":681,"context_line":"            self._spawn_keepalived(ri)"},{"line_number":682,"context_line":""},{"line_number":683,"context_line":"    def _handle_router_snat_rules(self, ri, ex_gw_port, internal_cidrs,"}],"source_content_type":"text/x-python","patch_set":9,"id":"baada198_f2c28c84","line":680,"in_reply_to":"baada198_d4560dce","updated":"2014-08-06 21:00:22.000000000","message":"Done","commit_id":"b90e816c17fe466369a784dda156537d42fc862d"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"d961807f618d9ef1d691c31820c96a8137f74653","unresolved":false,"context_lines":[{"line_number":721,"context_line":""},{"line_number":722,"context_line":"        ri.iptables_manager.apply()"},{"line_number":723,"context_line":""},{"line_number":724,"context_line":"    def _add_keepalived_vip(self, ri, ip_cidr, interface):"},{"line_number":725,"context_line":"        instance \u003d ri.keepalived_manager.config.get_instance("},{"line_number":726,"context_line":"            HA_VIRTUAL_ROUTER_NAME_PREFIX + str(ri.ha_vr_id))"},{"line_number":727,"context_line":"        vip_address \u003d keepalived.KeepalivedVipAddress(ip_cidr, interface)"}],"source_content_type":"text/x-python","patch_set":9,"id":"baada198_f9bc1ab7","line":724,"updated":"2014-07-15 14:59:53.000000000","message":"Perhaps add_keepalived_vip should belong to the keepalived manager? All of this complexity could be moved there. Same for the remove of course.","commit_id":"b90e816c17fe466369a784dda156537d42fc862d"},{"author":{"_account_id":7141,"name":"Sylvain Afchain","email":"safchain@redhat.com","username":"sylvainA"},"change_message_id":"ffa18c193bc650438e0560d0dfc68c723356ef18","unresolved":false,"context_lines":[{"line_number":721,"context_line":""},{"line_number":722,"context_line":"        ri.iptables_manager.apply()"},{"line_number":723,"context_line":""},{"line_number":724,"context_line":"    def _add_keepalived_vip(self, ri, ip_cidr, interface):"},{"line_number":725,"context_line":"        instance \u003d ri.keepalived_manager.config.get_instance("},{"line_number":726,"context_line":"            HA_VIRTUAL_ROUTER_NAME_PREFIX + str(ri.ha_vr_id))"},{"line_number":727,"context_line":"        vip_address \u003d keepalived.KeepalivedVipAddress(ip_cidr, interface)"}],"source_content_type":"text/x-python","patch_set":9,"id":"baada198_aabdc6d9","line":724,"in_reply_to":"baada198_f9bc1ab7","updated":"2014-07-16 08:16:24.000000000","message":"Almost agree with you, this kind of accessors were previously existing in the manager but removed following a comment of Mark McClain. Maybe we could re add them as it is more than just accessors now.","commit_id":"b90e816c17fe466369a784dda156537d42fc862d"},{"author":{"_account_id":11280,"name":"Jun Wu","email":"quark@lihdd.net","username":"quark"},"change_message_id":"5130722e006b1ff63477919541c9f9d431d3c818","unresolved":false,"context_lines":[{"line_number":733,"context_line":""},{"line_number":734,"context_line":"        # number of vip is limited to 20, so the first one will be used"},{"line_number":735,"context_line":"        # and the others will be not included in the protocol."},{"line_number":736,"context_line":"        vip_address \u003d vip_addresses.pop(0)"},{"line_number":737,"context_line":"        if vip_address:"},{"line_number":738,"context_line":"            instance.vip_addresses \u003d [vip_address]"},{"line_number":739,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"baada198_7bdf223a","line":736,"updated":"2014-07-08 08:16:43.000000000","message":"No need to pop(0):\n\n        instance.vip_addresses \u003d vip_addresses[0:1]\n        instance.vip_addresses_excluded \u003d vip_addresses[1:]\n\nAnd the sort stuff is duplicated with _remove_keepalived_vip.\n\nI think it\u0027s better to move add_vip_address, remove_vip_address to KeepalivedInstance and then make the sort stuff a private method there.","commit_id":"b90e816c17fe466369a784dda156537d42fc862d"},{"author":{"_account_id":7141,"name":"Sylvain Afchain","email":"safchain@redhat.com","username":"sylvainA"},"change_message_id":"ffa18c193bc650438e0560d0dfc68c723356ef18","unresolved":false,"context_lines":[{"line_number":733,"context_line":""},{"line_number":734,"context_line":"        # number of vip is limited to 20, so the first one will be used"},{"line_number":735,"context_line":"        # and the others will be not included in the protocol."},{"line_number":736,"context_line":"        vip_address \u003d vip_addresses.pop(0)"},{"line_number":737,"context_line":"        if vip_address:"},{"line_number":738,"context_line":"            instance.vip_addresses \u003d [vip_address]"},{"line_number":739,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"baada198_6a672eb8","line":736,"in_reply_to":"baada198_7bdf223a","updated":"2014-07-16 08:16:24.000000000","message":"See my comment just above","commit_id":"b90e816c17fe466369a784dda156537d42fc862d"},{"author":{"_account_id":11280,"name":"Jun Wu","email":"quark@lihdd.net","username":"quark"},"change_message_id":"5130722e006b1ff63477919541c9f9d431d3c818","unresolved":false,"context_lines":[{"line_number":771,"context_line":"        fip_statuses \u003d {}"},{"line_number":772,"context_line":"        interface_name \u003d self.get_external_device_name(ex_gw_port[\u0027id\u0027])"},{"line_number":773,"context_line":""},{"line_number":774,"context_line":"        instance \u003d ri.keepalived_manager.config.get_instance("},{"line_number":775,"context_line":"            HA_VIRTUAL_ROUTER_NAME_PREFIX + str(ri.ha_vr_id))"},{"line_number":776,"context_line":"        existing_cidrs \u003d set([vip.ip_address"},{"line_number":777,"context_line":"                              for vip in instance.vip_addresses])"}],"source_content_type":"text/x-python","patch_set":9,"id":"baada198_1bbf0662","line":774,"updated":"2014-07-08 08:16:43.000000000","message":"Duplicated code. Maybe add a property to RouterInfo is better:\n\n  class RouterInfo(object):\n    @property\n    def keepalived_instance(self):\n        return self.keepalived_manager.config.get_instance(\n            HA_VIRTUAL_ROUTER_NAME_PREFIX + str(self.ha_vr_id))","commit_id":"b90e816c17fe466369a784dda156537d42fc862d"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"f2249f67776bf3c75bda4398124064a93b739d3f","unresolved":false,"context_lines":[{"line_number":771,"context_line":"        fip_statuses \u003d {}"},{"line_number":772,"context_line":"        interface_name \u003d self.get_external_device_name(ex_gw_port[\u0027id\u0027])"},{"line_number":773,"context_line":""},{"line_number":774,"context_line":"        instance \u003d ri.keepalived_manager.config.get_instance("},{"line_number":775,"context_line":"            HA_VIRTUAL_ROUTER_NAME_PREFIX + str(ri.ha_vr_id))"},{"line_number":776,"context_line":"        existing_cidrs \u003d set([vip.ip_address"},{"line_number":777,"context_line":"                              for vip in instance.vip_addresses])"}],"source_content_type":"text/x-python","patch_set":9,"id":"baada198_12be2000","line":774,"in_reply_to":"baada198_1bbf0662","updated":"2014-08-06 21:00:22.000000000","message":"I removed the code duplication by doing something a tad differently: get_instance now accepts only a ha_vr_id and lookup is done by that id. The names are auto-generated from the id.","commit_id":"b90e816c17fe466369a784dda156537d42fc862d"},{"author":{"_account_id":2874,"name":"yong sheng gong","email":"gong.yongsheng@99cloud.net","username":"gongysh"},"change_message_id":"bb26bb0618031509f2d1c0c29a7934849c13a43b","unresolved":false,"context_lines":[{"line_number":820,"context_line":"            if ip_cidr not in existing_cidrs:"},{"line_number":821,"context_line":"                net \u003d netaddr.IPNetwork(ip_cidr)"},{"line_number":822,"context_line":"                try:"},{"line_number":823,"context_line":"                    device.addr.add(net.version, ip_cidr,"},{"line_number":824,"context_line":"                                    str(net.broadcast))"},{"line_number":825,"context_line":"                except (processutils.UnknownArgumentError,"},{"line_number":826,"context_line":"                        processutils.ProcessExecutionError):"}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAAW3%2F%2F9e0%3D","line":823,"updated":"2014-04-09 08:29:05.000000000","message":"no point of making format change","commit_id":"b90e816c17fe466369a784dda156537d42fc862d"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"f2249f67776bf3c75bda4398124064a93b739d3f","unresolved":false,"context_lines":[{"line_number":820,"context_line":"            if ip_cidr not in existing_cidrs:"},{"line_number":821,"context_line":"                net \u003d netaddr.IPNetwork(ip_cidr)"},{"line_number":822,"context_line":"                try:"},{"line_number":823,"context_line":"                    device.addr.add(net.version, ip_cidr,"},{"line_number":824,"context_line":"                                    str(net.broadcast))"},{"line_number":825,"context_line":"                except (processutils.UnknownArgumentError,"},{"line_number":826,"context_line":"                        processutils.ProcessExecutionError):"}],"source_content_type":"text/x-python","patch_set":9,"id":"baada198_920790c8","line":823,"in_reply_to":"AAAAW3%2F%2F9e0%3D","updated":"2014-08-06 21:00:22.000000000","message":"Done","commit_id":"b90e816c17fe466369a784dda156537d42fc862d"},{"author":{"_account_id":2874,"name":"yong sheng gong","email":"gong.yongsheng@99cloud.net","username":"gongysh"},"change_message_id":"bb26bb0618031509f2d1c0c29a7934849c13a43b","unresolved":false,"context_lines":[{"line_number":833,"context_line":"                    continue"},{"line_number":834,"context_line":"                # As GARP is processed in a distinct thread the call below"},{"line_number":835,"context_line":"                # won\u0027t raise an exception to be handled."},{"line_number":836,"context_line":"                if not ri.is_ha():"},{"line_number":837,"context_line":"                    self._send_gratuitous_arp_packet(ri, interface_name,"},{"line_number":838,"context_line":"                                                     fip_ip)"},{"line_number":839,"context_line":"            fip_statuses[fip[\u0027id\u0027]] \u003d ("}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAAW3%2F%2F9e4%3D","line":836,"updated":"2014-04-09 08:29:05.000000000","message":"no need to do this branch since we have a guard at line 654","commit_id":"b90e816c17fe466369a784dda156537d42fc862d"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"f2249f67776bf3c75bda4398124064a93b739d3f","unresolved":false,"context_lines":[{"line_number":833,"context_line":"                    continue"},{"line_number":834,"context_line":"                # As GARP is processed in a distinct thread the call below"},{"line_number":835,"context_line":"                # won\u0027t raise an exception to be handled."},{"line_number":836,"context_line":"                if not ri.is_ha():"},{"line_number":837,"context_line":"                    self._send_gratuitous_arp_packet(ri, interface_name,"},{"line_number":838,"context_line":"                                                     fip_ip)"},{"line_number":839,"context_line":"            fip_statuses[fip[\u0027id\u0027]] \u003d ("}],"source_content_type":"text/x-python","patch_set":9,"id":"baada198_72b91ceb","line":836,"in_reply_to":"AAAAW3%2F%2F9e4%3D","updated":"2014-08-06 21:00:22.000000000","message":"Done","commit_id":"b90e816c17fe466369a784dda156537d42fc862d"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"d961807f618d9ef1d691c31820c96a8137f74653","unresolved":false,"context_lines":[{"line_number":900,"context_line":""},{"line_number":901,"context_line":"        gw_ip \u003d ex_gw_port[\u0027subnet\u0027][\u0027gateway_ip\u0027]"},{"line_number":902,"context_line":"        if ex_gw_port[\u0027subnet\u0027][\u0027gateway_ip\u0027]:"},{"line_number":903,"context_line":"            if ri.is_ha():"},{"line_number":904,"context_line":"                instance \u003d ri.keepalived_manager.config.get_instance("},{"line_number":905,"context_line":"                    HA_VIRTUAL_ROUTER_NAME_PREFIX + str(ri.ha_vr_id))"},{"line_number":906,"context_line":"                vroute \u003d keepalived.KeepalivedVirtualRoute("}],"source_content_type":"text/x-python","patch_set":9,"id":"baada198_b922a2fe","line":903,"updated":"2014-07-15 14:59:53.000000000","message":"Could this bit be made into a function of the keepalived manager?","commit_id":"b90e816c17fe466369a784dda156537d42fc862d"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"f2249f67776bf3c75bda4398124064a93b739d3f","unresolved":false,"context_lines":[{"line_number":900,"context_line":""},{"line_number":901,"context_line":"        gw_ip \u003d ex_gw_port[\u0027subnet\u0027][\u0027gateway_ip\u0027]"},{"line_number":902,"context_line":"        if ex_gw_port[\u0027subnet\u0027][\u0027gateway_ip\u0027]:"},{"line_number":903,"context_line":"            if ri.is_ha():"},{"line_number":904,"context_line":"                instance \u003d ri.keepalived_manager.config.get_instance("},{"line_number":905,"context_line":"                    HA_VIRTUAL_ROUTER_NAME_PREFIX + str(ri.ha_vr_id))"},{"line_number":906,"context_line":"                vroute \u003d keepalived.KeepalivedVirtualRoute("}],"source_content_type":"text/x-python","patch_set":9,"id":"baada198_92a310b0","line":903,"in_reply_to":"baada198_b922a2fe","updated":"2014-08-06 21:00:22.000000000","message":"Done","commit_id":"b90e816c17fe466369a784dda156537d42fc862d"},{"author":{"_account_id":2874,"name":"yong sheng gong","email":"gong.yongsheng@99cloud.net","username":"gongysh"},"change_message_id":"bb26bb0618031509f2d1c0c29a7934849c13a43b","unresolved":false,"context_lines":[{"line_number":974,"context_line":"                                root_helper\u003dself.root_helper,"},{"line_number":975,"context_line":"                                namespace\u003dri.ns_name()):"},{"line_number":976,"context_line":"            if ri.is_ha():"},{"line_number":977,"context_line":"                self._remove_keepalived_vip(ri, ip_cidr\u003dinternal_cidr)"},{"line_number":978,"context_line":""},{"line_number":979,"context_line":"            self.driver.unplug(interface_name, namespace\u003dri.ns_name(),"},{"line_number":980,"context_line":"                               prefix\u003dINTERNAL_DEV_PREFIX)"}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAAW3%2F%2F9iQ%3D","line":977,"updated":"2014-04-09 08:29:05.000000000","message":"why don\u0027t remove by interface just like the gateway interface?","commit_id":"b90e816c17fe466369a784dda156537d42fc862d"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"f2249f67776bf3c75bda4398124064a93b739d3f","unresolved":false,"context_lines":[{"line_number":974,"context_line":"                                root_helper\u003dself.root_helper,"},{"line_number":975,"context_line":"                                namespace\u003dri.ns_name()):"},{"line_number":976,"context_line":"            if ri.is_ha():"},{"line_number":977,"context_line":"                self._remove_keepalived_vip(ri, ip_cidr\u003dinternal_cidr)"},{"line_number":978,"context_line":""},{"line_number":979,"context_line":"            self.driver.unplug(interface_name, namespace\u003dri.ns_name(),"},{"line_number":980,"context_line":"                               prefix\u003dINTERNAL_DEV_PREFIX)"}],"source_content_type":"text/x-python","patch_set":9,"id":"baada198_a3015309","line":977,"in_reply_to":"AAAAW3%2F%2F9iQ%3D","updated":"2014-08-06 21:00:22.000000000","message":"You\u0027re correct.","commit_id":"b90e816c17fe466369a784dda156537d42fc862d"},{"author":{"_account_id":6788,"name":"Rossella Sblendido","email":"rsblendido@suse.com","username":"rossella-o"},"change_message_id":"d6ba80b44905573fe54772b42ad87d2388d6b7bb","unresolved":false,"context_lines":[{"line_number":784,"context_line":"                         internal_cidr, mac_address,"},{"line_number":785,"context_line":"                         set_address\u003dFalse):"},{"line_number":786,"context_line":"        interface_name \u003d self.get_ha_device_name(port_id)"},{"line_number":787,"context_line":"        if not ip_lib.device_exists(interface_name,"},{"line_number":788,"context_line":"                                    root_helper\u003dself.root_helper,"},{"line_number":789,"context_line":"                                    namespace\u003dri.ns_name):"},{"line_number":790,"context_line":"            self.driver.plug(network_id, port_id, interface_name, mac_address,"}],"source_content_type":"text/x-python","patch_set":12,"id":"baada198_36150ae6","line":787,"updated":"2014-07-25 16:01:12.000000000","message":"It\u0027s not only in your code...anyway driver.plug method for all drivers present in neutron/agent/linux/interface.py is already checking for if a device exist. This check is expensive and it\u0027s done twice, once here, once in the driver.","commit_id":"3cbefa7ef0bba2a122fdcbb0620fde9495f94c18"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"7025605c193749bbf64f1a338456cef1f30b37c4","unresolved":false,"context_lines":[{"line_number":784,"context_line":"                         internal_cidr, mac_address,"},{"line_number":785,"context_line":"                         set_address\u003dFalse):"},{"line_number":786,"context_line":"        interface_name \u003d self.get_ha_device_name(port_id)"},{"line_number":787,"context_line":"        if not ip_lib.device_exists(interface_name,"},{"line_number":788,"context_line":"                                    root_helper\u003dself.root_helper,"},{"line_number":789,"context_line":"                                    namespace\u003dri.ns_name):"},{"line_number":790,"context_line":"            self.driver.plug(network_id, port_id, interface_name, mac_address,"}],"source_content_type":"text/x-python","patch_set":12,"id":"baada198_ac8274c4","line":787,"in_reply_to":"baada198_36150ae6","updated":"2014-07-28 16:06:09.000000000","message":"Thanks, I\u0027ll look into this shortly.","commit_id":"3cbefa7ef0bba2a122fdcbb0620fde9495f94c18"},{"author":{"_account_id":6788,"name":"Rossella Sblendido","email":"rsblendido@suse.com","username":"rossella-o"},"change_message_id":"d6ba80b44905573fe54772b42ad87d2388d6b7bb","unresolved":false,"context_lines":[{"line_number":801,"context_line":"        if ip_lib.device_exists(interface_name,"},{"line_number":802,"context_line":"                                root_helper\u003dself.root_helper,"},{"line_number":803,"context_line":"                                namespace\u003dri.ns_name):"},{"line_number":804,"context_line":"            self.driver.unplug(interface_name, namespace\u003dri.ns_name,"},{"line_number":805,"context_line":"                               prefix\u003dHA_DEV_PREFIX)"},{"line_number":806,"context_line":""},{"line_number":807,"context_line":"    def process_ha_router_added(self, ri):"}],"source_content_type":"text/x-python","patch_set":12,"id":"baada198_76b942eb","line":804,"updated":"2014-07-25 16:01:12.000000000","message":"unplug will log an error if the device doesn\u0027t exists, so I think check at line 800 can also be removed.","commit_id":"3cbefa7ef0bba2a122fdcbb0620fde9495f94c18"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"7025605c193749bbf64f1a338456cef1f30b37c4","unresolved":false,"context_lines":[{"line_number":801,"context_line":"        if ip_lib.device_exists(interface_name,"},{"line_number":802,"context_line":"                                root_helper\u003dself.root_helper,"},{"line_number":803,"context_line":"                                namespace\u003dri.ns_name):"},{"line_number":804,"context_line":"            self.driver.unplug(interface_name, namespace\u003dri.ns_name,"},{"line_number":805,"context_line":"                               prefix\u003dHA_DEV_PREFIX)"},{"line_number":806,"context_line":""},{"line_number":807,"context_line":"    def process_ha_router_added(self, ri):"}],"source_content_type":"text/x-python","patch_set":12,"id":"baada198_0c4a80ac","line":804,"in_reply_to":"baada198_76b942eb","updated":"2014-07-28 16:06:09.000000000","message":"Sounds good. Right now I\u0027m still handling our own dirty laundry. Afterwards I\u0027ll go back and respond to all reviewer notes.","commit_id":"3cbefa7ef0bba2a122fdcbb0620fde9495f94c18"},{"author":{"_account_id":4149,"name":"Itsuro Oda","email":"oda@valinux.co.jp","username":"oda-g"},"change_message_id":"39b0f56d656213b3abe6b8c0645951bcd8b51f2c","unresolved":false,"context_lines":[{"line_number":146,"context_line":"class RouterInfo(object):"},{"line_number":147,"context_line":""},{"line_number":148,"context_line":"    def __init__(self, router_id, root_helper, use_namespaces, router,"},{"line_number":149,"context_line":"                 ha_confs_path\u003dNone):"},{"line_number":150,"context_line":"        self.router_id \u003d router_id"},{"line_number":151,"context_line":"        self.ex_gw_port \u003d None"},{"line_number":152,"context_line":"        self._snat_enabled \u003d None"}],"source_content_type":"text/x-python","patch_set":15,"id":"baada198_2589a025","line":149,"updated":"2014-07-31 06:46:24.000000000","message":"ha_confs_path is not used.","commit_id":"6e41ea30bb9174c397546a3a71d3846c9dcf85e1"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"33402c3255b258bc61f47fe153353efc9957d25f","unresolved":false,"context_lines":[{"line_number":146,"context_line":"class RouterInfo(object):"},{"line_number":147,"context_line":""},{"line_number":148,"context_line":"    def __init__(self, router_id, root_helper, use_namespaces, router,"},{"line_number":149,"context_line":"                 ha_confs_path\u003dNone):"},{"line_number":150,"context_line":"        self.router_id \u003d router_id"},{"line_number":151,"context_line":"        self.ex_gw_port \u003d None"},{"line_number":152,"context_line":"        self._snat_enabled \u003d None"}],"source_content_type":"text/x-python","patch_set":15,"id":"baada198_c764c84f","line":149,"in_reply_to":"baada198_2589a025","updated":"2014-08-06 21:00:54.000000000","message":"Done","commit_id":"6e41ea30bb9174c397546a3a71d3846c9dcf85e1"},{"author":{"_account_id":4149,"name":"Itsuro Oda","email":"oda@valinux.co.jp","username":"oda-g"},"change_message_id":"39b0f56d656213b3abe6b8c0645951bcd8b51f2c","unresolved":false,"context_lines":[{"line_number":1385,"context_line":"                                     internal_cidrs, ri.ns_name,"},{"line_number":1386,"context_line":"                                     preserve_ips)"},{"line_number":1387,"context_line":""},{"line_number":1388,"context_line":"        if ri.is_ha():"},{"line_number":1389,"context_line":"            self._add_keepalived_vip(ri, ex_gw_port[\u0027ip_cidr\u0027],"},{"line_number":1390,"context_line":"                                     interface_name)"},{"line_number":1391,"context_line":"            gw_ip \u003d ex_gw_port[\u0027subnet\u0027][\u0027gateway_ip\u0027]"}],"source_content_type":"text/x-python","patch_set":15,"id":"baada198_a5c69067","line":1388,"updated":"2014-07-31 06:46:24.000000000","message":"this block is better in _external_gateway_added (else part of 1412).","commit_id":"6e41ea30bb9174c397546a3a71d3846c9dcf85e1"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"33402c3255b258bc61f47fe153353efc9957d25f","unresolved":false,"context_lines":[{"line_number":1385,"context_line":"                                     internal_cidrs, ri.ns_name,"},{"line_number":1386,"context_line":"                                     preserve_ips)"},{"line_number":1387,"context_line":""},{"line_number":1388,"context_line":"        if ri.is_ha():"},{"line_number":1389,"context_line":"            self._add_keepalived_vip(ri, ex_gw_port[\u0027ip_cidr\u0027],"},{"line_number":1390,"context_line":"                                     interface_name)"},{"line_number":1391,"context_line":"            gw_ip \u003d ex_gw_port[\u0027subnet\u0027][\u0027gateway_ip\u0027]"}],"source_content_type":"text/x-python","patch_set":15,"id":"baada198_275bc48a","line":1388,"in_reply_to":"baada198_a5c69067","updated":"2014-08-06 21:00:54.000000000","message":"Done","commit_id":"6e41ea30bb9174c397546a3a71d3846c9dcf85e1"},{"author":{"_account_id":4149,"name":"Itsuro Oda","email":"oda@valinux.co.jp","username":"oda-g"},"change_message_id":"39b0f56d656213b3abe6b8c0645951bcd8b51f2c","unresolved":false,"context_lines":[{"line_number":1390,"context_line":"                                     interface_name)"},{"line_number":1391,"context_line":"            gw_ip \u003d ex_gw_port[\u0027subnet\u0027][\u0027gateway_ip\u0027]"},{"line_number":1392,"context_line":"            if gw_ip:"},{"line_number":1393,"context_line":"                if ri.is_ha():"},{"line_number":1394,"context_line":"                    instance \u003d ri.keepalived_manager.config.get_instance("},{"line_number":1395,"context_line":"                        HA_VIRTUAL_ROUTER_NAME_PREFIX + str(ri.ha_vr_id))"},{"line_number":1396,"context_line":"                    vroute \u003d keepalived.KeepalivedVirtualRoute("}],"source_content_type":"text/x-python","patch_set":15,"id":"baada198_25bb60e5","line":1393,"updated":"2014-07-31 06:46:24.000000000","message":"unnecessary. dup 1388.","commit_id":"6e41ea30bb9174c397546a3a71d3846c9dcf85e1"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"33402c3255b258bc61f47fe153353efc9957d25f","unresolved":false,"context_lines":[{"line_number":1390,"context_line":"                                     interface_name)"},{"line_number":1391,"context_line":"            gw_ip \u003d ex_gw_port[\u0027subnet\u0027][\u0027gateway_ip\u0027]"},{"line_number":1392,"context_line":"            if gw_ip:"},{"line_number":1393,"context_line":"                if ri.is_ha():"},{"line_number":1394,"context_line":"                    instance \u003d ri.keepalived_manager.config.get_instance("},{"line_number":1395,"context_line":"                        HA_VIRTUAL_ROUTER_NAME_PREFIX + str(ri.ha_vr_id))"},{"line_number":1396,"context_line":"                    vroute \u003d keepalived.KeepalivedVirtualRoute("}],"source_content_type":"text/x-python","patch_set":15,"id":"baada198_e771cc0e","line":1393,"in_reply_to":"baada198_25bb60e5","updated":"2014-08-06 21:00:54.000000000","message":"Done","commit_id":"6e41ea30bb9174c397546a3a71d3846c9dcf85e1"},{"author":{"_account_id":4149,"name":"Itsuro Oda","email":"oda@valinux.co.jp","username":"oda-g"},"change_message_id":"fc0e62997a17941cd5fd110c6e9834faf0975e8c","unresolved":false,"context_lines":[{"line_number":1979,"context_line":"        ip_wrapper \u003d ip_lib.IPWrapper(self.root_helper,"},{"line_number":1980,"context_line":"                                      namespace\u003dri.ns_name)"},{"line_number":1981,"context_line":"        ip_wrapper.netns.execute(cmd, check_exit_code\u003dFalse)"},{"line_number":1982,"context_line":""},{"line_number":1983,"context_line":"    def routes_updated(self, ri):"},{"line_number":1984,"context_line":"        new_routes \u003d ri.router[\u0027routes\u0027]"},{"line_number":1985,"context_line":"        old_routes \u003d ri.routes"}],"source_content_type":"text/x-python","patch_set":17,"id":"baada198_0fa57602","line":1982,"updated":"2014-08-01 01:18:44.000000000","message":"\u0027ip route\u0027 fails if HA case since an interface has no ip address.\nI think routes should be handled via \u0027virtual_routes\u0027 if HA case.","commit_id":"40f62cb92e97ef0c082441d3f47e0f0d343d0b95"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"4c23146c541f6267388564f169d17dfcf39c9b36","unresolved":false,"context_lines":[{"line_number":1979,"context_line":"        ip_wrapper \u003d ip_lib.IPWrapper(self.root_helper,"},{"line_number":1980,"context_line":"                                      namespace\u003dri.ns_name)"},{"line_number":1981,"context_line":"        ip_wrapper.netns.execute(cmd, check_exit_code\u003dFalse)"},{"line_number":1982,"context_line":""},{"line_number":1983,"context_line":"    def routes_updated(self, ri):"},{"line_number":1984,"context_line":"        new_routes \u003d ri.router[\u0027routes\u0027]"},{"line_number":1985,"context_line":"        old_routes \u003d ri.routes"}],"source_content_type":"text/x-python","patch_set":17,"id":"baada198_e37d0b72","line":1982,"in_reply_to":"baada198_0fa57602","updated":"2014-08-06 21:01:03.000000000","message":"I think you\u0027re right. Adding this as a known limitation for now.","commit_id":"40f62cb92e97ef0c082441d3f47e0f0d343d0b95"},{"author":{"_account_id":8213,"name":"Claudiu Belu","email":"cbelu@cloudbasesolutions.com","username":"claudiub"},"change_message_id":"bba22d9ce63d6e99d4dfc1529b89407b6e2aad05","unresolved":false,"context_lines":[{"line_number":1315,"context_line":"                                     interface_name, INTERNAL_DEV_PREFIX,"},{"line_number":1316,"context_line":"                                     ri.is_ha())"},{"line_number":1317,"context_line":""},{"line_number":1318,"context_line":"        if ri.is_ha():"},{"line_number":1319,"context_line":"            self._add_keepalived_vip(ri, internal_cidr, interface_name)"},{"line_number":1320,"context_line":""},{"line_number":1321,"context_line":"        ex_gw_port \u003d self._get_ex_gw_port(ri)"}],"source_content_type":"text/x-python","patch_set":19,"id":"baada198_e8e11898","line":1318,"updated":"2014-08-08 14:43:02.000000000","message":"tiny nit: since you already call ri.is_ha() on this line and also on 1316, maybe you could assign it to a variable instead of calling it twice. :)","commit_id":"82006c4398f5ef9426fd0ebaa91b63a8110d5647"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"0aba862cfeddd6621d86107ff41c801e2b8884e5","unresolved":false,"context_lines":[{"line_number":1315,"context_line":"                                     interface_name, INTERNAL_DEV_PREFIX,"},{"line_number":1316,"context_line":"                                     ri.is_ha())"},{"line_number":1317,"context_line":""},{"line_number":1318,"context_line":"        if ri.is_ha():"},{"line_number":1319,"context_line":"            self._add_keepalived_vip(ri, internal_cidr, interface_name)"},{"line_number":1320,"context_line":""},{"line_number":1321,"context_line":"        ex_gw_port \u003d self._get_ex_gw_port(ri)"}],"source_content_type":"text/x-python","patch_set":19,"id":"baada198_1f15804b","line":1318,"in_reply_to":"baada198_e8e11898","updated":"2014-08-10 13:23:04.000000000","message":"Done","commit_id":"82006c4398f5ef9426fd0ebaa91b63a8110d5647"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"ff61f1c33f8ff64e2ae55e0acb3496de2dd261ef","unresolved":false,"context_lines":[{"line_number":30,"context_line":"from neutron.agent.linux import interface"},{"line_number":31,"context_line":"from neutron.agent.linux import ip_lib"},{"line_number":32,"context_line":"from neutron.agent.linux import iptables_manager"},{"line_number":33,"context_line":"from neutron.agent.linux import ovs_lib  # noqa"},{"line_number":34,"context_line":"from neutron.agent.linux import ra"},{"line_number":35,"context_line":"from neutron.agent import rpc as agent_rpc"},{"line_number":36,"context_line":"from neutron.common import config as common_config"}],"source_content_type":"text/x-python","patch_set":33,"id":"1abeadc6_a21ef75c","line":33,"updated":"2014-08-29 23:09:55.000000000","message":"See the following example of how to explicitly import config:\n\nhttps://github.com/openstack/neutron/blob/master/neutron/tests/unit/ml2/test_ml2_plugin.py#L46","commit_id":"3ea68c8e5ac6a7908448e495b8398324f76ce8a5"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"63b011caaab7d190064a3a465d32603989a64f4d","unresolved":false,"context_lines":[{"line_number":30,"context_line":"from neutron.agent.linux import interface"},{"line_number":31,"context_line":"from neutron.agent.linux import ip_lib"},{"line_number":32,"context_line":"from neutron.agent.linux import iptables_manager"},{"line_number":33,"context_line":"from neutron.agent.linux import ovs_lib  # noqa"},{"line_number":34,"context_line":"from neutron.agent.linux import ra"},{"line_number":35,"context_line":"from neutron.agent import rpc as agent_rpc"},{"line_number":36,"context_line":"from neutron.common import config as common_config"}],"source_content_type":"text/x-python","patch_set":33,"id":"fa98f980_6ff3e60a","line":33,"in_reply_to":"1abeadc6_a21ef75c","updated":"2014-08-31 15:39:43.000000000","message":"Seems like this was unneeded anyway.","commit_id":"3ea68c8e5ac6a7908448e495b8398324f76ce8a5"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"ff61f1c33f8ff64e2ae55e0acb3496de2dd261ef","unresolved":false,"context_lines":[{"line_number":739,"context_line":"        ri.iptables_manager.apply()"},{"line_number":740,"context_line":"        super(L3NATAgent, self).process_router_add(ri)"},{"line_number":741,"context_line":""},{"line_number":742,"context_line":"        if ri.is_ha():"},{"line_number":743,"context_line":"            self.process_ha_router_added(ri)"},{"line_number":744,"context_line":""},{"line_number":745,"context_line":"        if self.conf.enable_metadata_proxy:"}],"source_content_type":"text/x-python","patch_set":33,"id":"1abeadc6_8263bbaf","line":742,"updated":"2014-08-29 23:09:55.000000000","message":"(No action required) I\u0027m having a hard time not getting frustrated at the constant conditionals everywhere.  It\u0027s reminding me of the invasive nature of dvr, and making me wish we had time to separate concerns as part of this effort.","commit_id":"3ea68c8e5ac6a7908448e495b8398324f76ce8a5"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"63b011caaab7d190064a3a465d32603989a64f4d","unresolved":false,"context_lines":[{"line_number":739,"context_line":"        ri.iptables_manager.apply()"},{"line_number":740,"context_line":"        super(L3NATAgent, self).process_router_add(ri)"},{"line_number":741,"context_line":""},{"line_number":742,"context_line":"        if ri.is_ha():"},{"line_number":743,"context_line":"            self.process_ha_router_added(ri)"},{"line_number":744,"context_line":""},{"line_number":745,"context_line":"        if self.conf.enable_metadata_proxy:"}],"source_content_type":"text/x-python","patch_set":33,"id":"fa98f980_6f20469c","line":742,"in_reply_to":"1abeadc6_8263bbaf","updated":"2014-08-31 15:39:43.000000000","message":"I couldn\u0027t agree more. The original procedural design (A blob of functions, no classes or other forms of abstractions) of the L3 agent works up to a certain complexity but now with DVR and HA routers it\u0027s gotten out of control. This current approach of conditionals everywhere will be difficult to maintain, not to mention hard to comprehend for current and new developers alike. I think realistically the earliest we could get a decent refactor going is in mid Kilo sadly.","commit_id":"3ea68c8e5ac6a7908448e495b8398324f76ce8a5"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"ff61f1c33f8ff64e2ae55e0acb3496de2dd261ef","unresolved":false,"context_lines":[{"line_number":1112,"context_line":"                    # won\u0027t raise an exception to be handled."},{"line_number":1113,"context_line":"                    self._send_gratuitous_arp_packet("},{"line_number":1114,"context_line":"                        ri.ns_name, interface_name, fip_ip)"},{"line_number":1115,"context_line":""},{"line_number":1116,"context_line":"            fip_statuses[fip[\u0027id\u0027]] \u003d ("},{"line_number":1117,"context_line":"                l3_constants.FLOATINGIP_STATUS_ACTIVE)"},{"line_number":1118,"context_line":""}],"source_content_type":"text/x-python","patch_set":33,"id":"1abeadc6_62ebff12","line":1115,"updated":"2014-08-29 23:09:55.000000000","message":"Remove","commit_id":"3ea68c8e5ac6a7908448e495b8398324f76ce8a5"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"63b011caaab7d190064a3a465d32603989a64f4d","unresolved":false,"context_lines":[{"line_number":1112,"context_line":"                    # won\u0027t raise an exception to be handled."},{"line_number":1113,"context_line":"                    self._send_gratuitous_arp_packet("},{"line_number":1114,"context_line":"                        ri.ns_name, interface_name, fip_ip)"},{"line_number":1115,"context_line":""},{"line_number":1116,"context_line":"            fip_statuses[fip[\u0027id\u0027]] \u003d ("},{"line_number":1117,"context_line":"                l3_constants.FLOATINGIP_STATUS_ACTIVE)"},{"line_number":1118,"context_line":""}],"source_content_type":"text/x-python","patch_set":33,"id":"fa98f980_4f3c6294","line":1115,"in_reply_to":"1abeadc6_62ebff12","updated":"2014-08-31 15:39:43.000000000","message":"Done","commit_id":"3ea68c8e5ac6a7908448e495b8398324f76ce8a5"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"061d57603e77fc0aa15ebbf56fa4c2fd0f3d9c89","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":38,"id":"fa98f980_096c9a07","updated":"2014-09-03 04:53:06.000000000","message":"Just thinking out loud...\n\nThe L3NATAgent has so much \"if is_ha ... [else ...] and \"if not is_ha ...\" I wonder if a mixin is the right thing here.  Should the l3_ha_agent be a subclass?  I haven\u0027t thought this out completely but something smells.  I see that it would be difficult because of existing subclasses like L3NATAgentWithStateReport (which itself may be better off as a mixin).\n\nSo, I\u0027m certainly not requesting you make this refactor now unless you can think of a sensible way to do it.  This is probably a longer-term effort.","commit_id":"2a9108692a88233f615db45143723037e0833e68"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"cfbe36f9ec96a20662fb8019bddda8449997427a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":38,"id":"fa98f980_47c0cce0","in_reply_to":"fa98f980_096c9a07","updated":"2014-09-07 14:08:44.000000000","message":"As discussed in the L3 sub team meeting - I agree, and I think this is a great candidate for early Kilo. We need to move away from the non-design of the L3 agent to a general mechanism that would execute the correct code per router configuration. A router should be able to configure itself, and not just be a weird \"struct\" type \"class\". Even the naming scheme is awful - ri is the existing router and ri.router is the router received in the update... Could it get more confusing than that? How about the agent diffing between two separate Router objects - Existing and new. There\u0027s a ton of work that needs to be done here...","commit_id":"2a9108692a88233f615db45143723037e0833e68"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"b5f3345a5bc5d34cbed91605a2a4aa2c20bc5102","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":38,"id":"fa98f980_16d88a52","in_reply_to":"fa98f980_47c0cce0","updated":"2014-09-09 00:07:23.000000000","message":"+1","commit_id":"2a9108692a88233f615db45143723037e0833e68"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"061d57603e77fc0aa15ebbf56fa4c2fd0f3d9c89","unresolved":false,"context_lines":[{"line_number":433,"context_line":""},{"line_number":434,"context_line":""},{"line_number":435,"context_line":"class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback,"},{"line_number":436,"context_line":"                 l3_ha_agent.AgentMixin, manager.Manager):"},{"line_number":437,"context_line":"    \"\"\"Manager for L3NatAgent"},{"line_number":438,"context_line":""},{"line_number":439,"context_line":"        API version history:"}],"source_content_type":"text/x-python","patch_set":38,"id":"fa98f980_e9511637","line":436,"updated":"2014-09-03 04:53:06.000000000","message":"nit:  I think if we\u0027re going to start adding newlines I\u0027d rather each of the super classes be listed on its own line.","commit_id":"2a9108692a88233f615db45143723037e0833e68"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"cfbe36f9ec96a20662fb8019bddda8449997427a","unresolved":false,"context_lines":[{"line_number":433,"context_line":""},{"line_number":434,"context_line":""},{"line_number":435,"context_line":"class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback,"},{"line_number":436,"context_line":"                 l3_ha_agent.AgentMixin, manager.Manager):"},{"line_number":437,"context_line":"    \"\"\"Manager for L3NatAgent"},{"line_number":438,"context_line":""},{"line_number":439,"context_line":"        API version history:"}],"source_content_type":"text/x-python","patch_set":38,"id":"fa98f980_e7a8e022","line":436,"in_reply_to":"fa98f980_e9511637","updated":"2014-09-07 14:08:44.000000000","message":"Done","commit_id":"2a9108692a88233f615db45143723037e0833e68"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"061d57603e77fc0aa15ebbf56fa4c2fd0f3d9c89","unresolved":false,"context_lines":[{"line_number":768,"context_line":"        del self.router_info[router_id]"},{"line_number":769,"context_line":"        self._destroy_router_namespace(ri.ns_name)"},{"line_number":770,"context_line":""},{"line_number":771,"context_line":"    def _spawn_metadata_proxy(self, ri):"},{"line_number":772,"context_line":"        router_id, ns_name \u003d ri.router_id, ri.ns_name"},{"line_number":773,"context_line":""},{"line_number":774,"context_line":"        def callback(pid_file):"}],"source_content_type":"text/x-python","patch_set":38,"id":"fa98f980_0943ba6c","line":771,"updated":"2014-09-03 04:53:06.000000000","message":"Don\u0027t make this change.  This goes against the law of demeter [1] for no good reason.  Maru asked me to refactor this in [2].  \n\n\nIncidentally, you missed one in neutron/services/firewall/agents/varmour/varmour_router.py.  Better to just not do this.\n\n [1] http://en.wikipedia.org/wiki/Law_of_Demeter\n [2] https://review.openstack.org/#/c/57293","commit_id":"2a9108692a88233f615db45143723037e0833e68"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"cfbe36f9ec96a20662fb8019bddda8449997427a","unresolved":false,"context_lines":[{"line_number":768,"context_line":"        del self.router_info[router_id]"},{"line_number":769,"context_line":"        self._destroy_router_namespace(ri.ns_name)"},{"line_number":770,"context_line":""},{"line_number":771,"context_line":"    def _spawn_metadata_proxy(self, ri):"},{"line_number":772,"context_line":"        router_id, ns_name \u003d ri.router_id, ri.ns_name"},{"line_number":773,"context_line":""},{"line_number":774,"context_line":"        def callback(pid_file):"}],"source_content_type":"text/x-python","patch_set":38,"id":"fa98f980_07a6c453","line":771,"in_reply_to":"fa98f980_0943ba6c","updated":"2014-09-07 14:08:44.000000000","message":"Point taken.","commit_id":"2a9108692a88233f615db45143723037e0833e68"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"061d57603e77fc0aa15ebbf56fa4c2fd0f3d9c89","unresolved":false,"context_lines":[{"line_number":791,"context_line":"            ns_name)"},{"line_number":792,"context_line":""},{"line_number":793,"context_line":"        if ri.is_ha:"},{"line_number":794,"context_line":"            self._add_keepalived_notifiers(pm, callback, ri)"},{"line_number":795,"context_line":"        else:"},{"line_number":796,"context_line":"            pm.enable(callback)"},{"line_number":797,"context_line":""}],"source_content_type":"text/x-python","patch_set":38,"id":"fa98f980_29487e86","line":794,"updated":"2014-09-03 04:53:06.000000000","message":"What does this have to do with spawning the metadata proxy?  I was surprised to see this called from _spawn_metadata_proxy.\n\nnit:  Is it good to call a _single_underscore method from a mixin?  I wouldn\u0027t typically do this but maybe others think it is okay.","commit_id":"2a9108692a88233f615db45143723037e0833e68"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"cfbe36f9ec96a20662fb8019bddda8449997427a","unresolved":false,"context_lines":[{"line_number":791,"context_line":"            ns_name)"},{"line_number":792,"context_line":""},{"line_number":793,"context_line":"        if ri.is_ha:"},{"line_number":794,"context_line":"            self._add_keepalived_notifiers(pm, callback, ri)"},{"line_number":795,"context_line":"        else:"},{"line_number":796,"context_line":"            pm.enable(callback)"},{"line_number":797,"context_line":""}],"source_content_type":"text/x-python","patch_set":38,"id":"fa98f980_394cd076","line":794,"in_reply_to":"fa98f980_29487e86","updated":"2014-09-07 14:08:44.000000000","message":"For HA routers we only want to spawn the proxy on the active instance. That\u0027s one reason we use the keepalived notifier scripts - The master script is executed when the router transitions to the master state and spawns the proxy. The backup and fault scripts kill the proxy.","commit_id":"2a9108692a88233f615db45143723037e0833e68"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"061d57603e77fc0aa15ebbf56fa4c2fd0f3d9c89","unresolved":false,"context_lines":[{"line_number":793,"context_line":"        if ri.is_ha:"},{"line_number":794,"context_line":"            self._add_keepalived_notifiers(pm, callback, ri)"},{"line_number":795,"context_line":"        else:"},{"line_number":796,"context_line":"            pm.enable(callback)"},{"line_number":797,"context_line":""},{"line_number":798,"context_line":"    def _destroy_metadata_proxy(self, router_id, ns_name):"},{"line_number":799,"context_line":"        pm \u003d external_process.ProcessManager("}],"source_content_type":"text/x-python","patch_set":38,"id":"fa98f980_49606201","line":796,"updated":"2014-09-03 04:53:06.000000000","message":"Why not spawn the metadata proxy for ha?","commit_id":"2a9108692a88233f615db45143723037e0833e68"},{"author":{"_account_id":2733,"name":"Darragh O\u0027Reilly","email":"doreilly@suse.com","username":"darragh-oreilly"},"change_message_id":"c84a8e76295825da5f7873eb31448df67df14791","unresolved":false,"context_lines":[{"line_number":793,"context_line":"        if ri.is_ha:"},{"line_number":794,"context_line":"            self._add_keepalived_notifiers(pm, callback, ri)"},{"line_number":795,"context_line":"        else:"},{"line_number":796,"context_line":"            pm.enable(callback)"},{"line_number":797,"context_line":""},{"line_number":798,"context_line":"    def _destroy_metadata_proxy(self, router_id, ns_name):"},{"line_number":799,"context_line":"        pm \u003d external_process.ProcessManager("}],"source_content_type":"text/x-python","patch_set":38,"id":"fa98f980_d41aecc4","line":796,"in_reply_to":"fa98f980_49606201","updated":"2014-09-04 14:56:36.000000000","message":"The metadata ns proxy listens on 0.0.0.0:9697, so I don\u0027t think it matters wheather the internal ip addresses exist or not when it is spawned.\n\nI reverted the changes in this method, and metadata continues to work after failover.","commit_id":"2a9108692a88233f615db45143723037e0833e68"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"061d57603e77fc0aa15ebbf56fa4c2fd0f3d9c89","unresolved":false,"context_lines":[{"line_number":941,"context_line":"                ri.iptables_manager.defer_apply_off()"},{"line_number":942,"context_line":"                # Once NAT rules for floating IPs are safely in place"},{"line_number":943,"context_line":"                # configure their addresses on the external gateway port"},{"line_number":944,"context_line":"                if ri.is_ha:"},{"line_number":945,"context_line":"                    fip_statuses \u003d ("},{"line_number":946,"context_line":"                        self.process_ha_router_floating_ip_addresses("},{"line_number":947,"context_line":"                            ri, ex_gw_port))"}],"source_content_type":"text/x-python","patch_set":38,"id":"fa98f980_695d2647","line":944,"updated":"2014-09-03 04:53:06.000000000","message":"See file level note above.","commit_id":"2a9108692a88233f615db45143723037e0833e68"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"061d57603e77fc0aa15ebbf56fa4c2fd0f3d9c89","unresolved":false,"context_lines":[{"line_number":943,"context_line":"                # configure their addresses on the external gateway port"},{"line_number":944,"context_line":"                if ri.is_ha:"},{"line_number":945,"context_line":"                    fip_statuses \u003d ("},{"line_number":946,"context_line":"                        self.process_ha_router_floating_ip_addresses("},{"line_number":947,"context_line":"                            ri, ex_gw_port))"},{"line_number":948,"context_line":"                else:"},{"line_number":949,"context_line":"                    fip_statuses \u003d self.process_router_floating_ip_addresses("}],"source_content_type":"text/x-python","patch_set":38,"id":"fa98f980_2c5c0ced","line":946,"updated":"2014-09-03 04:53:06.000000000","message":"Is this split going to be a maintenance problem?  Do these two \"process...floating_ip...\" methods share some implementation that could be factored out?  I\u0027d be okay if you did this as a follow on if there is some action to be taken.","commit_id":"2a9108692a88233f615db45143723037e0833e68"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"cfbe36f9ec96a20662fb8019bddda8449997427a","unresolved":false,"context_lines":[{"line_number":943,"context_line":"                # configure their addresses on the external gateway port"},{"line_number":944,"context_line":"                if ri.is_ha:"},{"line_number":945,"context_line":"                    fip_statuses \u003d ("},{"line_number":946,"context_line":"                        self.process_ha_router_floating_ip_addresses("},{"line_number":947,"context_line":"                            ri, ex_gw_port))"},{"line_number":948,"context_line":"                else:"},{"line_number":949,"context_line":"                    fip_statuses \u003d self.process_router_floating_ip_addresses("}],"source_content_type":"text/x-python","patch_set":38,"id":"fa98f980_dc21f2fd","line":946,"in_reply_to":"fa98f980_2c5c0ced","updated":"2014-09-07 14:08:44.000000000","message":"I refactored to reuse code as much as possible, please share your thoughts on the next patchset. Regardless of the refactor I just noticed that the floating IP statuses will always be ACTIVE when working with a HA router... I\u0027ll report a bug so we can consider how to deal with this in a future patch, it\u0027s not easy: We\u0027d have to report ACTIVE here, then when keepalived transforms to master, in the notify script, check that the configuration of the floating IP succeeded, and if not change the status to ERROR. I wonder how keepalived could fail to configure the IP though? The IP should be valid on the interface, if not we should catch that much earlier than configuration time...","commit_id":"2a9108692a88233f615db45143723037e0833e68"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"061d57603e77fc0aa15ebbf56fa4c2fd0f3d9c89","unresolved":false,"context_lines":[{"line_number":1448,"context_line":""},{"line_number":1449,"context_line":"        interface_name \u003d self.get_internal_device_name(port_id)"},{"line_number":1450,"context_line":""},{"line_number":1451,"context_line":"        is_ha \u003d ri.is_ha"},{"line_number":1452,"context_line":"        self._internal_network_added(ri.ns_name, network_id, port_id,"},{"line_number":1453,"context_line":"                                     internal_cidr, mac_address,"},{"line_number":1454,"context_line":"                                     interface_name, INTERNAL_DEV_PREFIX,"}],"source_content_type":"text/x-python","patch_set":38,"id":"fa98f980_8956ca28","line":1451,"updated":"2014-09-03 04:53:06.000000000","message":"nit:  Temporary doesn\u0027t add any value.","commit_id":"2a9108692a88233f615db45143723037e0833e68"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"cfbe36f9ec96a20662fb8019bddda8449997427a","unresolved":false,"context_lines":[{"line_number":1448,"context_line":""},{"line_number":1449,"context_line":"        interface_name \u003d self.get_internal_device_name(port_id)"},{"line_number":1450,"context_line":""},{"line_number":1451,"context_line":"        is_ha \u003d ri.is_ha"},{"line_number":1452,"context_line":"        self._internal_network_added(ri.ns_name, network_id, port_id,"},{"line_number":1453,"context_line":"                                     internal_cidr, mac_address,"},{"line_number":1454,"context_line":"                                     interface_name, INTERNAL_DEV_PREFIX,"}],"source_content_type":"text/x-python","patch_set":38,"id":"fa98f980_19ee2cb5","line":1451,"in_reply_to":"fa98f980_8956ca28","updated":"2014-09-07 14:08:44.000000000","message":"Done","commit_id":"2a9108692a88233f615db45143723037e0833e68"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"061d57603e77fc0aa15ebbf56fa4c2fd0f3d9c89","unresolved":false,"context_lines":[{"line_number":1864,"context_line":"        new_routes \u003d ri.router[\u0027routes\u0027]"},{"line_number":1865,"context_line":"        if ri.is_ha:"},{"line_number":1866,"context_line":"            self._process_virtual_routes(ri, new_routes)"},{"line_number":1867,"context_line":"            return"},{"line_number":1868,"context_line":""},{"line_number":1869,"context_line":"        old_routes \u003d ri.routes"},{"line_number":1870,"context_line":"        adds, removes \u003d common_utils.diff_list_of_dict(old_routes,"}],"source_content_type":"text/x-python","patch_set":38,"id":"fa98f980_a9538e37","line":1867,"updated":"2014-09-03 04:53:06.000000000","message":"See file level comment above.","commit_id":"2a9108692a88233f615db45143723037e0833e68"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"6ae9705ab2c6b34ab669cab590f0f7c3fe620af8","unresolved":false,"context_lines":[{"line_number":746,"context_line":"            if ri.is_ha:"},{"line_number":747,"context_line":"                self._add_keepalived_notifiers(ri)"},{"line_number":748,"context_line":"            else:"},{"line_number":749,"context_line":"                self._spawn_metadata_proxy(ri.router_id, ri.ns_name)"},{"line_number":750,"context_line":""},{"line_number":751,"context_line":"    def _router_removed(self, router_id):"},{"line_number":752,"context_line":"        ri \u003d self.router_info.get(router_id)"}],"source_content_type":"text/x-python","patch_set":42,"id":"fa98f980_f62a26e6","line":749,"updated":"2014-09-09 00:50:35.000000000","message":"This is better that PS38.  I\u0027m still wondering about Darragh\u0027s comments there.  Have you weighed pros/cons of running MD proxy in all routers and not worrying about keepalive notifications?\n\nGranted, there is some resource consumption with the proxy that is non-trivial when a network node hosts many routers.  I\u0027d like to see what Brian Haley thinks.","commit_id":"000f08d0f1e8d1801da8f654f400c4dc3ba3e544"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"8a679dc72394d47a70fa28401f92dc0087f790ff","unresolved":false,"context_lines":[{"line_number":746,"context_line":"            if ri.is_ha:"},{"line_number":747,"context_line":"                self._add_keepalived_notifiers(ri)"},{"line_number":748,"context_line":"            else:"},{"line_number":749,"context_line":"                self._spawn_metadata_proxy(ri.router_id, ri.ns_name)"},{"line_number":750,"context_line":""},{"line_number":751,"context_line":"    def _router_removed(self, router_id):"},{"line_number":752,"context_line":"        ri \u003d self.router_info.get(router_id)"}],"source_content_type":"text/x-python","patch_set":42,"id":"fa98f980_b9cde3b9","line":749,"in_reply_to":"fa98f980_77cd4d5d","updated":"2014-09-09 11:46:39.000000000","message":"@Miguel - We do have metadata with HA routers, it\u0027s just that only the master instance opens up the proxy and the standbys don\u0027t. We do this via the notifier scripts.\n\n@Carl - \u0027Legacy\u0027 routers were only scheduled on a single node, so you only had one metadata proxy total per router. Here we\u0027re scheduling the router on multiple agents and I don\u0027t want to degrade performance by opening up the proxy on all nodes. The con is the added complexity of the notify scripts, but that\u0027s a non-point because we\u0027re actually going to be *expanding* their usage and not eliminating them in the future, since it\u0027s the only way to know when a router has transitioned to backup/fault/master. We want to know this so we can update l2pop, update the controller so the admin can know the status of each router on each agent, etc.","commit_id":"000f08d0f1e8d1801da8f654f400c4dc3ba3e544"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"89732d2b5321d988654171488f82c1656c6e45f6","unresolved":false,"context_lines":[{"line_number":746,"context_line":"            if ri.is_ha:"},{"line_number":747,"context_line":"                self._add_keepalived_notifiers(ri)"},{"line_number":748,"context_line":"            else:"},{"line_number":749,"context_line":"                self._spawn_metadata_proxy(ri.router_id, ri.ns_name)"},{"line_number":750,"context_line":""},{"line_number":751,"context_line":"    def _router_removed(self, router_id):"},{"line_number":752,"context_line":"        ri \u003d self.router_info.get(router_id)"}],"source_content_type":"text/x-python","patch_set":42,"id":"fa98f980_762b7deb","line":749,"in_reply_to":"fa98f980_b9cde3b9","updated":"2014-09-09 17:22:59.000000000","message":"I would agree that running only a single MD proxy right now is fine.  I think it was Miguel (or someone else at Red Hat) that profiled the memory consumption of the MD proxy and it was larger than I would have expected.","commit_id":"000f08d0f1e8d1801da8f654f400c4dc3ba3e544"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"cdd1e93d12f3927c85a2bf5912c3d09aec82b0f8","unresolved":false,"context_lines":[{"line_number":746,"context_line":"            if ri.is_ha:"},{"line_number":747,"context_line":"                self._add_keepalived_notifiers(ri)"},{"line_number":748,"context_line":"            else:"},{"line_number":749,"context_line":"                self._spawn_metadata_proxy(ri.router_id, ri.ns_name)"},{"line_number":750,"context_line":""},{"line_number":751,"context_line":"    def _router_removed(self, router_id):"},{"line_number":752,"context_line":"        ri \u003d self.router_info.get(router_id)"}],"source_content_type":"text/x-python","patch_set":42,"id":"fa98f980_a73bf5e9","line":749,"in_reply_to":"fa98f980_b9cde3b9","updated":"2014-09-10 16:25:33.000000000","message":"ok","commit_id":"000f08d0f1e8d1801da8f654f400c4dc3ba3e544"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"ef99423b2582ce5749a0375e41f0c135ff21d857","unresolved":false,"context_lines":[{"line_number":746,"context_line":"            if ri.is_ha:"},{"line_number":747,"context_line":"                self._add_keepalived_notifiers(ri)"},{"line_number":748,"context_line":"            else:"},{"line_number":749,"context_line":"                self._spawn_metadata_proxy(ri.router_id, ri.ns_name)"},{"line_number":750,"context_line":""},{"line_number":751,"context_line":"    def _router_removed(self, router_id):"},{"line_number":752,"context_line":"        ri \u003d self.router_info.get(router_id)"}],"source_content_type":"text/x-python","patch_set":42,"id":"fa98f980_77cd4d5d","line":749,"in_reply_to":"fa98f980_f62a26e6","updated":"2014-09-09 05:50:36.000000000","message":"I believe we could enable this later as a follow up, if it\u0027s possible. I agree metadata would be good to have if possible.","commit_id":"000f08d0f1e8d1801da8f654f400c4dc3ba3e544"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"6ae9705ab2c6b34ab669cab590f0f7c3fe620af8","unresolved":false,"context_lines":[{"line_number":1058,"context_line":"        ri.iptables_manager.apply()"},{"line_number":1059,"context_line":""},{"line_number":1060,"context_line":"    def _get_external_device_interface_name(self, ri, ex_gw_port,"},{"line_number":1061,"context_line":"                                            floating_ips):"},{"line_number":1062,"context_line":"        if ri.router[\u0027distributed\u0027]:"},{"line_number":1063,"context_line":"            # filter out only FIPs for this host/agent"},{"line_number":1064,"context_line":"            floating_ips \u003d [i for i in floating_ips if i[\u0027host\u0027] \u003d\u003d self.host]"}],"source_content_type":"text/x-python","patch_set":42,"id":"fa98f980_3182f800","line":1061,"updated":"2014-09-09 00:50:35.000000000","message":"I think this should be refactored independently of the L3 HA changes.  It is easier on reviewers to separate pure refactoring from changes.","commit_id":"000f08d0f1e8d1801da8f654f400c4dc3ba3e544"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"a43ccdfc942d30241b870de4773ea4ded1f0b2de","unresolved":false,"context_lines":[{"line_number":1058,"context_line":"        ri.iptables_manager.apply()"},{"line_number":1059,"context_line":""},{"line_number":1060,"context_line":"    def _get_external_device_interface_name(self, ri, ex_gw_port,"},{"line_number":1061,"context_line":"                                            floating_ips):"},{"line_number":1062,"context_line":"        if ri.router[\u0027distributed\u0027]:"},{"line_number":1063,"context_line":"            # filter out only FIPs for this host/agent"},{"line_number":1064,"context_line":"            floating_ips \u003d [i for i in floating_ips if i[\u0027host\u0027] \u003d\u003d self.host]"}],"source_content_type":"text/x-python","patch_set":42,"id":"fa98f980_de751f33","line":1061,"in_reply_to":"fa98f980_3182f800","updated":"2014-09-09 18:32:16.000000000","message":"Done","commit_id":"000f08d0f1e8d1801da8f654f400c4dc3ba3e544"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"6ae9705ab2c6b34ab669cab590f0f7c3fe620af8","unresolved":false,"context_lines":[{"line_number":1070,"context_line":"                if floating_ips and ri.dist_fip_count \u003d\u003d 0:"},{"line_number":1071,"context_line":"                    self.create_rtr_2_fip_link(ri, floating_ips[0]"},{"line_number":1072,"context_line":"                                               [\u0027floating_network_id\u0027])"},{"line_number":1073,"context_line":"                interface_name \u003d self.get_rtr_int_device_name(ri.router_id)"},{"line_number":1074,"context_line":"            else:"},{"line_number":1075,"context_line":"                # there are no fips or agent port, no work to do"},{"line_number":1076,"context_line":"                return None"}],"source_content_type":"text/x-python","patch_set":42,"id":"fa98f980_b18fc874","line":1073,"updated":"2014-09-09 00:50:35.000000000","message":"return interface_name\n\nGet rid of a local variable as part of the refactor.","commit_id":"000f08d0f1e8d1801da8f654f400c4dc3ba3e544"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"a43ccdfc942d30241b870de4773ea4ded1f0b2de","unresolved":false,"context_lines":[{"line_number":1070,"context_line":"                if floating_ips and ri.dist_fip_count \u003d\u003d 0:"},{"line_number":1071,"context_line":"                    self.create_rtr_2_fip_link(ri, floating_ips[0]"},{"line_number":1072,"context_line":"                                               [\u0027floating_network_id\u0027])"},{"line_number":1073,"context_line":"                interface_name \u003d self.get_rtr_int_device_name(ri.router_id)"},{"line_number":1074,"context_line":"            else:"},{"line_number":1075,"context_line":"                # there are no fips or agent port, no work to do"},{"line_number":1076,"context_line":"                return None"}],"source_content_type":"text/x-python","patch_set":42,"id":"fa98f980_3e154b9b","line":1073,"in_reply_to":"fa98f980_b18fc874","updated":"2014-09-09 18:32:16.000000000","message":"Done","commit_id":"000f08d0f1e8d1801da8f654f400c4dc3ba3e544"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"6ae9705ab2c6b34ab669cab590f0f7c3fe620af8","unresolved":false,"context_lines":[{"line_number":1075,"context_line":"                # there are no fips or agent port, no work to do"},{"line_number":1076,"context_line":"                return None"},{"line_number":1077,"context_line":"        else:"},{"line_number":1078,"context_line":"            interface_name \u003d self.get_external_device_name(ex_gw_port[\u0027id\u0027])"},{"line_number":1079,"context_line":""},{"line_number":1080,"context_line":"        return interface_name"},{"line_number":1081,"context_line":""}],"source_content_type":"text/x-python","patch_set":42,"id":"fa98f980_d18c4c82","line":1078,"updated":"2014-09-09 00:50:35.000000000","message":"return self.get_...","commit_id":"000f08d0f1e8d1801da8f654f400c4dc3ba3e544"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"a43ccdfc942d30241b870de4773ea4ded1f0b2de","unresolved":false,"context_lines":[{"line_number":1075,"context_line":"                # there are no fips or agent port, no work to do"},{"line_number":1076,"context_line":"                return None"},{"line_number":1077,"context_line":"        else:"},{"line_number":1078,"context_line":"            interface_name \u003d self.get_external_device_name(ex_gw_port[\u0027id\u0027])"},{"line_number":1079,"context_line":""},{"line_number":1080,"context_line":"        return interface_name"},{"line_number":1081,"context_line":""}],"source_content_type":"text/x-python","patch_set":42,"id":"fa98f980_1e1247b4","line":1078,"in_reply_to":"fa98f980_d18c4c82","updated":"2014-09-09 18:32:16.000000000","message":"Done","commit_id":"000f08d0f1e8d1801da8f654f400c4dc3ba3e544"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"6ae9705ab2c6b34ab669cab590f0f7c3fe620af8","unresolved":false,"context_lines":[{"line_number":1113,"context_line":"            if ri.router[\u0027distributed\u0027]:"},{"line_number":1114,"context_line":"                self.floating_ip_removed_dist(ri, ip_cidr)"},{"line_number":1115,"context_line":""},{"line_number":1116,"context_line":"    def process_router_floating_ip_addresses(self, ri, ex_gw_port):"},{"line_number":1117,"context_line":"        fip_statuses \u003d {}"},{"line_number":1118,"context_line":"        floating_ips \u003d ri.router.get(l3_constants.FLOATINGIP_KEY, [])"},{"line_number":1119,"context_line":"        interface_name \u003d self._get_external_device_interface_name("}],"source_content_type":"text/x-python","patch_set":42,"id":"fa98f980_51aa1cd5","line":1116,"updated":"2014-09-09 00:50:35.000000000","message":"docstring?","commit_id":"000f08d0f1e8d1801da8f654f400c4dc3ba3e544"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"a43ccdfc942d30241b870de4773ea4ded1f0b2de","unresolved":false,"context_lines":[{"line_number":1113,"context_line":"            if ri.router[\u0027distributed\u0027]:"},{"line_number":1114,"context_line":"                self.floating_ip_removed_dist(ri, ip_cidr)"},{"line_number":1115,"context_line":""},{"line_number":1116,"context_line":"    def process_router_floating_ip_addresses(self, ri, ex_gw_port):"},{"line_number":1117,"context_line":"        fip_statuses \u003d {}"},{"line_number":1118,"context_line":"        floating_ips \u003d ri.router.get(l3_constants.FLOATINGIP_KEY, [])"},{"line_number":1119,"context_line":"        interface_name \u003d self._get_external_device_interface_name("}],"source_content_type":"text/x-python","patch_set":42,"id":"fa98f980_feeca3bd","line":1116,"in_reply_to":"fa98f980_51aa1cd5","updated":"2014-09-09 18:32:16.000000000","message":"Oops, it must have escaped during the refactor.","commit_id":"000f08d0f1e8d1801da8f654f400c4dc3ba3e544"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"f1a581b3264bf61213a6b9394033fc28d3542765","unresolved":false,"context_lines":[{"line_number":1094,"context_line":"        existing_cidrs \u003d set([addr[\u0027cidr\u0027] for addr in device.addr.list()])"},{"line_number":1095,"context_line":"        new_cidrs \u003d set()"},{"line_number":1096,"context_line":""},{"line_number":1097,"context_line":"        # Loop once to ensure that floating ips are configured."},{"line_number":1098,"context_line":"        for fip in floating_ips:"},{"line_number":1099,"context_line":"            fip_ip \u003d fip[\u0027floating_ip_address\u0027]"},{"line_number":1100,"context_line":"            ip_cidr \u003d str(fip_ip) + FLOATING_IP_CIDR_SUFFIX"}],"source_content_type":"text/x-python","patch_set":43,"id":"fa98f980_7a033fb8","side":"PARENT","line":1097,"updated":"2014-09-10 21:38:58.000000000","message":"nit:  random comment removal.","commit_id":"1b44f79133b6fd79a79d07b21ec88250b46b61bc"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"a0be84f6c95d10bbab879ad1d68eb557e719ee57","unresolved":false,"context_lines":[{"line_number":1094,"context_line":"        existing_cidrs \u003d set([addr[\u0027cidr\u0027] for addr in device.addr.list()])"},{"line_number":1095,"context_line":"        new_cidrs \u003d set()"},{"line_number":1096,"context_line":""},{"line_number":1097,"context_line":"        # Loop once to ensure that floating ips are configured."},{"line_number":1098,"context_line":"        for fip in floating_ips:"},{"line_number":1099,"context_line":"            fip_ip \u003d fip[\u0027floating_ip_address\u0027]"},{"line_number":1100,"context_line":"            ip_cidr \u003d str(fip_ip) + FLOATING_IP_CIDR_SUFFIX"}],"source_content_type":"text/x-python","patch_set":43,"id":"fa98f980_6d01a069","side":"PARENT","line":1097,"in_reply_to":"fa98f980_7a033fb8","updated":"2014-09-11 18:28:46.000000000","message":"Done","commit_id":"1b44f79133b6fd79a79d07b21ec88250b46b61bc"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"f1a581b3264bf61213a6b9394033fc28d3542765","unresolved":false,"context_lines":[{"line_number":1398,"context_line":"                           namespace\u003dns_name,"},{"line_number":1399,"context_line":"                           prefix\u003dEXTERNAL_DEV_PREFIX)"},{"line_number":1400,"context_line":"        if ri.router[\u0027distributed\u0027]:"},{"line_number":1401,"context_line":"            self._destroy_snat_namespace(ns_name)"},{"line_number":1402,"context_line":""},{"line_number":1403,"context_line":"    def metadata_filter_rules(self):"},{"line_number":1404,"context_line":"        rules \u003d []"}],"source_content_type":"text/x-python","patch_set":43,"id":"fa98f980_dd15059c","line":1401,"updated":"2014-09-10 21:38:58.000000000","message":"Is there a corresponding _ha_external_gateway_removed that needs to happen here?","commit_id":"d883a0a52125fa62730ff5a9ad9f5c2381cb2340"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"a0be84f6c95d10bbab879ad1d68eb557e719ee57","unresolved":false,"context_lines":[{"line_number":1398,"context_line":"                           namespace\u003dns_name,"},{"line_number":1399,"context_line":"                           prefix\u003dEXTERNAL_DEV_PREFIX)"},{"line_number":1400,"context_line":"        if ri.router[\u0027distributed\u0027]:"},{"line_number":1401,"context_line":"            self._destroy_snat_namespace(ns_name)"},{"line_number":1402,"context_line":""},{"line_number":1403,"context_line":"    def metadata_filter_rules(self):"},{"line_number":1404,"context_line":"        rules \u003d []"}],"source_content_type":"text/x-python","patch_set":43,"id":"fa98f980_0d9734ed","line":1401,"in_reply_to":"fa98f980_dd15059c","updated":"2014-09-11 18:28:46.000000000","message":"I\u0027ll put _clear_vips into a new _ha_external_gateway_removed for consistency. I think it\u0027s a good idea.","commit_id":"d883a0a52125fa62730ff5a9ad9f5c2381cb2340"}],"neutron/agent/l3_ha_agent.py":[{"author":{"_account_id":8213,"name":"Claudiu Belu","email":"cbelu@cloudbasesolutions.com","username":"claudiub"},"change_message_id":"bba22d9ce63d6e99d4dfc1529b89407b6e2aad05","unresolved":false,"context_lines":[{"line_number":120,"context_line":"    def process_ha_router_added(self, ri):"},{"line_number":121,"context_line":"        ha_port \u003d ri.router.get(l3_constants.HA_INTERFACE_KEY)"},{"line_number":122,"context_line":"        if not ha_port:"},{"line_number":123,"context_line":"            LOG.warn(_(\"Unable to process HA router without ha port\"))"},{"line_number":124,"context_line":"            return"},{"line_number":125,"context_line":""},{"line_number":126,"context_line":"        self._set_subnet_info(ha_port)"}],"source_content_type":"text/x-python","patch_set":19,"id":"baada198_c39ef568","line":123,"updated":"2014-08-08 14:43:02.000000000","message":"Wouldn\u0027t it be helpful to also log the HA router (name or id)?","commit_id":"82006c4398f5ef9426fd0ebaa91b63a8110d5647"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"0aba862cfeddd6621d86107ff41c801e2b8884e5","unresolved":false,"context_lines":[{"line_number":120,"context_line":"    def process_ha_router_added(self, ri):"},{"line_number":121,"context_line":"        ha_port \u003d ri.router.get(l3_constants.HA_INTERFACE_KEY)"},{"line_number":122,"context_line":"        if not ha_port:"},{"line_number":123,"context_line":"            LOG.warn(_(\"Unable to process HA router without ha port\"))"},{"line_number":124,"context_line":"            return"},{"line_number":125,"context_line":""},{"line_number":126,"context_line":"        self._set_subnet_info(ha_port)"}],"source_content_type":"text/x-python","patch_set":19,"id":"baada198_3f1a447c","line":123,"in_reply_to":"baada198_c39ef568","updated":"2014-08-10 13:23:04.000000000","message":"Absolutely, good point.","commit_id":"82006c4398f5ef9426fd0ebaa91b63a8110d5647"},{"author":{"_account_id":8213,"name":"Claudiu Belu","email":"cbelu@cloudbasesolutions.com","username":"claudiub"},"change_message_id":"7317dae79718b424094c8ef3ce6dcea991abfdb9","unresolved":false,"context_lines":[{"line_number":221,"context_line":"                                                       gw_ip, interface_name)"},{"line_number":222,"context_line":"            instance.virtual_routes.append(vroute)"},{"line_number":223,"context_line":""},{"line_number":224,"context_line":"    def _process_virtual_routes(self, ri, new_routes):"},{"line_number":225,"context_line":"        instance \u003d ri.keepalived_manager.config.get_instance(ri.ha_vr_id)"},{"line_number":226,"context_line":""},{"line_number":227,"context_line":"        # Filter out all of the old routes while keeping only the default route"}],"source_content_type":"text/x-python","patch_set":27,"id":"7ac7a964_e9797d94","line":224,"updated":"2014-08-14 14:50:19.000000000","message":"this doesn\u0027t seem to have a unit test. :)","commit_id":"0df33a7c5d986d6b1cb6b61a53eeaee9b314a690"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"8303f5e35862673a0c9b9d60ca0847fd9fdce249","unresolved":false,"context_lines":[{"line_number":221,"context_line":"                                                       gw_ip, interface_name)"},{"line_number":222,"context_line":"            instance.virtual_routes.append(vroute)"},{"line_number":223,"context_line":""},{"line_number":224,"context_line":"    def _process_virtual_routes(self, ri, new_routes):"},{"line_number":225,"context_line":"        instance \u003d ri.keepalived_manager.config.get_instance(ri.ha_vr_id)"},{"line_number":226,"context_line":""},{"line_number":227,"context_line":"        # Filter out all of the old routes while keeping only the default route"}],"source_content_type":"text/x-python","patch_set":27,"id":"7ac7a964_49f991c7","line":224,"in_reply_to":"7ac7a964_e9797d94","updated":"2014-08-14 14:55:31.000000000","message":"It\u0027s tested in test_l3_agent.test_ha_router_keepalived_config.","commit_id":"0df33a7c5d986d6b1cb6b61a53eeaee9b314a690"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"ff61f1c33f8ff64e2ae55e0acb3496de2dd261ef","unresolved":false,"context_lines":[{"line_number":45,"context_line":"]"},{"line_number":46,"context_line":""},{"line_number":47,"context_line":""},{"line_number":48,"context_line":"class RouterMixin(object):"},{"line_number":49,"context_line":"    def __init__(self):"},{"line_number":50,"context_line":"        self.ha_port \u003d None"},{"line_number":51,"context_line":"        self.keepalived_manager \u003d None"}],"source_content_type":"text/x-python","patch_set":33,"id":"1abeadc6_e28aafbd","line":48,"updated":"2014-08-29 23:09:55.000000000","message":"(No action required) Not another mixin!!","commit_id":"3ea68c8e5ac6a7908448e495b8398324f76ce8a5"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"63b011caaab7d190064a3a465d32603989a64f4d","unresolved":false,"context_lines":[{"line_number":45,"context_line":"]"},{"line_number":46,"context_line":""},{"line_number":47,"context_line":""},{"line_number":48,"context_line":"class RouterMixin(object):"},{"line_number":49,"context_line":"    def __init__(self):"},{"line_number":50,"context_line":"        self.ha_port \u003d None"},{"line_number":51,"context_line":"        self.keepalived_manager \u003d None"}],"source_content_type":"text/x-python","patch_set":33,"id":"fa98f980_6f5a0685","line":48,"in_reply_to":"1abeadc6_e28aafbd","updated":"2014-08-31 15:39:43.000000000","message":":)","commit_id":"3ea68c8e5ac6a7908448e495b8398324f76ce8a5"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"ff61f1c33f8ff64e2ae55e0acb3496de2dd261ef","unresolved":false,"context_lines":[{"line_number":50,"context_line":"        self.ha_port \u003d None"},{"line_number":51,"context_line":"        self.keepalived_manager \u003d None"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"    def is_ha(self):"},{"line_number":54,"context_line":"        return self.router is not None and self.router.get(\u0027ha\u0027)"},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"    @property"}],"source_content_type":"text/x-python","patch_set":33,"id":"1abeadc6_221e67bb","line":53,"updated":"2014-08-29 23:09:55.000000000","message":"Why isn\u0027t this a property, too?","commit_id":"3ea68c8e5ac6a7908448e495b8398324f76ce8a5"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"63b011caaab7d190064a3a465d32603989a64f4d","unresolved":false,"context_lines":[{"line_number":50,"context_line":"        self.ha_port \u003d None"},{"line_number":51,"context_line":"        self.keepalived_manager \u003d None"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"    def is_ha(self):"},{"line_number":54,"context_line":"        return self.router is not None and self.router.get(\u0027ha\u0027)"},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"    @property"}],"source_content_type":"text/x-python","patch_set":33,"id":"fa98f980_2f31de0b","line":53,"in_reply_to":"1abeadc6_221e67bb","updated":"2014-08-31 15:39:43.000000000","message":"Done. Also removed all mocking of this in the unit tests.","commit_id":"3ea68c8e5ac6a7908448e495b8398324f76ce8a5"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"ff61f1c33f8ff64e2ae55e0acb3496de2dd261ef","unresolved":false,"context_lines":[{"line_number":55,"context_line":""},{"line_number":56,"context_line":"    @property"},{"line_number":57,"context_line":"    def ha_vr_id(self):"},{"line_number":58,"context_line":"        return self.router.get(\u0027ha_vr_id\u0027)"},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"    @property"},{"line_number":61,"context_line":"    def ha_state(self):"}],"source_content_type":"text/x-python","patch_set":33,"id":"1abeadc6_62be9f66","line":58,"updated":"2014-08-29 23:09:55.000000000","message":"I\u0027m kind of confused why is_ha() checks for router being None and that\u0027s not done here.","commit_id":"3ea68c8e5ac6a7908448e495b8398324f76ce8a5"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"63b011caaab7d190064a3a465d32603989a64f4d","unresolved":false,"context_lines":[{"line_number":55,"context_line":""},{"line_number":56,"context_line":"    @property"},{"line_number":57,"context_line":"    def ha_vr_id(self):"},{"line_number":58,"context_line":"        return self.router.get(\u0027ha_vr_id\u0027)"},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"    @property"},{"line_number":61,"context_line":"    def ha_state(self):"}],"source_content_type":"text/x-python","patch_set":33,"id":"fa98f980_121a816e","line":58,"in_reply_to":"1abeadc6_62be9f66","updated":"2014-08-31 15:39:43.000000000","message":"Done. I added more aggressive error checking so we can fail early.","commit_id":"3ea68c8e5ac6a7908448e495b8398324f76ce8a5"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"ff61f1c33f8ff64e2ae55e0acb3496de2dd261ef","unresolved":false,"context_lines":[{"line_number":67,"context_line":"        try:"},{"line_number":68,"context_line":"            with open(state_path, \u0027r\u0027) as f:"},{"line_number":69,"context_line":"                output \u003d f.read()"},{"line_number":70,"context_line":"        except (OSError, IOError):"},{"line_number":71,"context_line":"            return None"},{"line_number":72,"context_line":"        else:"},{"line_number":73,"context_line":"            return output"}],"source_content_type":"text/x-python","patch_set":33,"id":"1abeadc6_fdd96e83","line":70,"updated":"2014-08-29 23:09:55.000000000","message":"Consider logging this exception if only at debug level.","commit_id":"3ea68c8e5ac6a7908448e495b8398324f76ce8a5"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"63b011caaab7d190064a3a465d32603989a64f4d","unresolved":false,"context_lines":[{"line_number":67,"context_line":"        try:"},{"line_number":68,"context_line":"            with open(state_path, \u0027r\u0027) as f:"},{"line_number":69,"context_line":"                output \u003d f.read()"},{"line_number":70,"context_line":"        except (OSError, IOError):"},{"line_number":71,"context_line":"            return None"},{"line_number":72,"context_line":"        else:"},{"line_number":73,"context_line":"            return output"}],"source_content_type":"text/x-python","patch_set":33,"id":"fa98f980_3226a5bb","line":70,"in_reply_to":"1abeadc6_fdd96e83","updated":"2014-08-31 15:39:43.000000000","message":"Done. I\u0027d be really surprised if we ever see this log message.","commit_id":"3ea68c8e5ac6a7908448e495b8398324f76ce8a5"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"ff61f1c33f8ff64e2ae55e0acb3496de2dd261ef","unresolved":false,"context_lines":[{"line_number":148,"context_line":""},{"line_number":149,"context_line":"        self.driver.init_l3(interface_name, [internal_cidr],"},{"line_number":150,"context_line":"                            namespace\u003dri.ns_name)"},{"line_number":151,"context_line":"        ip_address \u003d internal_cidr.split(\u0027/\u0027)[0]"},{"line_number":152,"context_line":"        self._send_gratuitous_arp_packet(ri, interface_name, ip_address)"},{"line_number":153,"context_line":""},{"line_number":154,"context_line":"    def ha_network_removed(self, ri):"}],"source_content_type":"text/x-python","patch_set":33,"id":"1abeadc6_bd59b6fe","line":151,"updated":"2014-08-29 23:09:55.000000000","message":"(No action required) Maybe we could write helpers for manipulating ip addresses?  Oh wait, netaddr already exists.","commit_id":"3ea68c8e5ac6a7908448e495b8398324f76ce8a5"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"63b011caaab7d190064a3a465d32603989a64f4d","unresolved":false,"context_lines":[{"line_number":148,"context_line":""},{"line_number":149,"context_line":"        self.driver.init_l3(interface_name, [internal_cidr],"},{"line_number":150,"context_line":"                            namespace\u003dri.ns_name)"},{"line_number":151,"context_line":"        ip_address \u003d internal_cidr.split(\u0027/\u0027)[0]"},{"line_number":152,"context_line":"        self._send_gratuitous_arp_packet(ri, interface_name, ip_address)"},{"line_number":153,"context_line":""},{"line_number":154,"context_line":"    def ha_network_removed(self, ri):"}],"source_content_type":"text/x-python","patch_set":33,"id":"fa98f980_32d06553","line":151,"in_reply_to":"1abeadc6_bd59b6fe","updated":"2014-08-31 15:39:43.000000000","message":"Done","commit_id":"3ea68c8e5ac6a7908448e495b8398324f76ce8a5"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"ff61f1c33f8ff64e2ae55e0acb3496de2dd261ef","unresolved":false,"context_lines":[{"line_number":211,"context_line":"            callback(pid), \u0027master\u0027, ri.ha_vr_id)"},{"line_number":212,"context_line":"        for state in (\u0027backup\u0027, \u0027fault\u0027):"},{"line_number":213,"context_line":"            ri.keepalived_manager.add_notifier("},{"line_number":214,"context_line":"                [\u0027kill\u0027, \u0027-9\u0027, \u0027$(cat \u0027 + pid + \u0027)\u0027], state, ri.ha_vr_id)"},{"line_number":215,"context_line":""},{"line_number":216,"context_line":"    def _ha_external_gateway_added(self, ri, ex_gw_port, interface_name):"},{"line_number":217,"context_line":"        self._add_keepalived_vip(ri, ex_gw_port[\u0027ip_cidr\u0027],"}],"source_content_type":"text/x-python","patch_set":33,"id":"1abeadc6_1d746a34","line":214,"updated":"2014-08-29 23:09:55.000000000","message":"Consider using the signal constant instead of -9.","commit_id":"3ea68c8e5ac6a7908448e495b8398324f76ce8a5"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"63b011caaab7d190064a3a465d32603989a64f4d","unresolved":false,"context_lines":[{"line_number":211,"context_line":"            callback(pid), \u0027master\u0027, ri.ha_vr_id)"},{"line_number":212,"context_line":"        for state in (\u0027backup\u0027, \u0027fault\u0027):"},{"line_number":213,"context_line":"            ri.keepalived_manager.add_notifier("},{"line_number":214,"context_line":"                [\u0027kill\u0027, \u0027-9\u0027, \u0027$(cat \u0027 + pid + \u0027)\u0027], state, ri.ha_vr_id)"},{"line_number":215,"context_line":""},{"line_number":216,"context_line":"    def _ha_external_gateway_added(self, ri, ex_gw_port, interface_name):"},{"line_number":217,"context_line":"        self._add_keepalived_vip(ri, ex_gw_port[\u0027ip_cidr\u0027],"}],"source_content_type":"text/x-python","patch_set":33,"id":"fa98f980_b267f5e9","line":214,"in_reply_to":"1abeadc6_1d746a34","updated":"2014-08-31 15:39:43.000000000","message":"Done","commit_id":"3ea68c8e5ac6a7908448e495b8398324f76ce8a5"},{"author":{"_account_id":12737,"name":"Jacek Swiderski","email":"jacek.swiderski@codilime.com","username":"Altran"},"change_message_id":"6d1b019e95371096ffe9260552b308e7d97fc425","unresolved":false,"context_lines":[{"line_number":39,"context_line":"    cfg.StrOpt(\u0027ha_vrrp_auth_type\u0027,"},{"line_number":40,"context_line":"               default\u003d\u0027PASS\u0027,"},{"line_number":41,"context_line":"               help\u003d_(\u0027VRRP authentication type AH/PASS\u0027)),"},{"line_number":42,"context_line":"    cfg.StrOpt(\u0027ha_vrrp_auth_password\u0027,"},{"line_number":43,"context_line":"               help\u003d_(\u0027VRRP authentication password\u0027)),"},{"line_number":44,"context_line":"    cfg.IntOpt(\u0027ha_vrrp_advert_int\u0027,"},{"line_number":45,"context_line":"               default\u003d2,"}],"source_content_type":"text/x-python","patch_set":35,"id":"fa98f980_716c969d","line":42,"updated":"2014-09-01 11:00:35.000000000","message":"secret\u003dTrue ?","commit_id":"0c10d4869b3163800d82ec9a3d2ff68033175ba1"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"291400d4be54b965a4f057263fd9190de51b7bd2","unresolved":false,"context_lines":[{"line_number":39,"context_line":"    cfg.StrOpt(\u0027ha_vrrp_auth_type\u0027,"},{"line_number":40,"context_line":"               default\u003d\u0027PASS\u0027,"},{"line_number":41,"context_line":"               help\u003d_(\u0027VRRP authentication type AH/PASS\u0027)),"},{"line_number":42,"context_line":"    cfg.StrOpt(\u0027ha_vrrp_auth_password\u0027,"},{"line_number":43,"context_line":"               help\u003d_(\u0027VRRP authentication password\u0027)),"},{"line_number":44,"context_line":"    cfg.IntOpt(\u0027ha_vrrp_advert_int\u0027,"},{"line_number":45,"context_line":"               default\u003d2,"}],"source_content_type":"text/x-python","patch_set":35,"id":"fa98f980_e23192a3","line":42,"in_reply_to":"fa98f980_716c969d","updated":"2014-09-01 12:23:08.000000000","message":"Done","commit_id":"0c10d4869b3163800d82ec9a3d2ff68033175ba1"},{"author":{"_account_id":12737,"name":"Jacek Swiderski","email":"jacek.swiderski@codilime.com","username":"Altran"},"change_message_id":"6d1b019e95371096ffe9260552b308e7d97fc425","unresolved":false,"context_lines":[{"line_number":54,"context_line":""},{"line_number":55,"context_line":"    def _verify_ha(self):"},{"line_number":56,"context_line":"        if not self.is_ha:"},{"line_number":57,"context_line":"            raise ValueError(\u0027Router %s is not a HA router\u0027 % self.router_id)"},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"    @property"},{"line_number":60,"context_line":"    def is_ha(self):"}],"source_content_type":"text/x-python","patch_set":35,"id":"fa98f980_112c2a47","line":57,"updated":"2014-09-01 11:00:35.000000000","message":"No i18n for this error message ?","commit_id":"0c10d4869b3163800d82ec9a3d2ff68033175ba1"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"291400d4be54b965a4f057263fd9190de51b7bd2","unresolved":false,"context_lines":[{"line_number":54,"context_line":""},{"line_number":55,"context_line":"    def _verify_ha(self):"},{"line_number":56,"context_line":"        if not self.is_ha:"},{"line_number":57,"context_line":"            raise ValueError(\u0027Router %s is not a HA router\u0027 % self.router_id)"},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"    @property"},{"line_number":60,"context_line":"    def is_ha(self):"}],"source_content_type":"text/x-python","patch_set":35,"id":"fa98f980_42f97ec7","line":57,"in_reply_to":"fa98f980_112c2a47","updated":"2014-09-01 12:23:08.000000000","message":"Done","commit_id":"0c10d4869b3163800d82ec9a3d2ff68033175ba1"},{"author":{"_account_id":2733,"name":"Darragh O\u0027Reilly","email":"doreilly@suse.com","username":"darragh-oreilly"},"change_message_id":"a88d069f1a161735f29fd584d54165bb7a5f4ea0","unresolved":false,"context_lines":[{"line_number":163,"context_line":""},{"line_number":164,"context_line":"        self.driver.init_l3(interface_name, [internal_cidr],"},{"line_number":165,"context_line":"                            namespace\u003dri.ns_name)"},{"line_number":166,"context_line":"        self._send_gratuitous_arp_packet("},{"line_number":167,"context_line":"            ri.ns_name, interface_name,"},{"line_number":168,"context_line":"            str(netaddr.IPNetwork(internal_cidr).ip))"},{"line_number":169,"context_line":""}],"source_content_type":"text/x-python","patch_set":37,"id":"fa98f980_a8271db5","line":166,"updated":"2014-09-02 17:34:34.000000000","message":"why is this needed?","commit_id":"0b8a4de81bcb5f3bb0e11e7e04117a995cca4126"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"54313bad171ba5b874e90d951cd594d0693bad65","unresolved":false,"context_lines":[{"line_number":163,"context_line":""},{"line_number":164,"context_line":"        self.driver.init_l3(interface_name, [internal_cidr],"},{"line_number":165,"context_line":"                            namespace\u003dri.ns_name)"},{"line_number":166,"context_line":"        self._send_gratuitous_arp_packet("},{"line_number":167,"context_line":"            ri.ns_name, interface_name,"},{"line_number":168,"context_line":"            str(netaddr.IPNetwork(internal_cidr).ip))"},{"line_number":169,"context_line":""}],"source_content_type":"text/x-python","patch_set":37,"id":"fa98f980_a43ff3b2","line":166,"in_reply_to":"fa98f980_007d551c","updated":"2014-09-04 15:43:25.000000000","message":"Darragh, Sylvain and I gave it some more thought and this could be removed - Sending a GARP here has nothing to do with the VRRP election process.","commit_id":"0b8a4de81bcb5f3bb0e11e7e04117a995cca4126"},{"author":{"_account_id":2733,"name":"Darragh O\u0027Reilly","email":"doreilly@suse.com","username":"darragh-oreilly"},"change_message_id":"4ed397a3be04a7e68f30c34cdcfd491d52b1c0c7","unresolved":false,"context_lines":[{"line_number":163,"context_line":""},{"line_number":164,"context_line":"        self.driver.init_l3(interface_name, [internal_cidr],"},{"line_number":165,"context_line":"                            namespace\u003dri.ns_name)"},{"line_number":166,"context_line":"        self._send_gratuitous_arp_packet("},{"line_number":167,"context_line":"            ri.ns_name, interface_name,"},{"line_number":168,"context_line":"            str(netaddr.IPNetwork(internal_cidr).ip))"},{"line_number":169,"context_line":""}],"source_content_type":"text/x-python","patch_set":37,"id":"fa98f980_b7056715","line":166,"in_reply_to":"fa98f980_a43ff3b2","updated":"2014-09-04 17:02:26.000000000","message":"I can\u0027t remember, or maybe never knew why GARP was been used at all. In this case the ip addresses on the ha- interfaces never move.","commit_id":"0b8a4de81bcb5f3bb0e11e7e04117a995cca4126"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"2e03eb7e044229ff40709bb56fe0a4748a8a088a","unresolved":false,"context_lines":[{"line_number":163,"context_line":""},{"line_number":164,"context_line":"        self.driver.init_l3(interface_name, [internal_cidr],"},{"line_number":165,"context_line":"                            namespace\u003dri.ns_name)"},{"line_number":166,"context_line":"        self._send_gratuitous_arp_packet("},{"line_number":167,"context_line":"            ri.ns_name, interface_name,"},{"line_number":168,"context_line":"            str(netaddr.IPNetwork(internal_cidr).ip))"},{"line_number":169,"context_line":""}],"source_content_type":"text/x-python","patch_set":37,"id":"fa98f980_007d551c","line":166,"in_reply_to":"fa98f980_a8271db5","updated":"2014-09-02 22:24:05.000000000","message":"HA ports aren\u0027t covered by keepalived. We send GARPs for internal and external devices that aren\u0027t taken care of by keepalived. Sending a GARP for the HA device can slightly speed up the VRRP election process I believe.","commit_id":"0b8a4de81bcb5f3bb0e11e7e04117a995cca4126"},{"author":{"_account_id":2733,"name":"Darragh O\u0027Reilly","email":"doreilly@suse.com","username":"darragh-oreilly"},"change_message_id":"c2fe81d4b15e7f5efd6d2448c5e2ed03f1f9ecea","unresolved":false,"context_lines":[{"line_number":198,"context_line":""},{"line_number":199,"context_line":"        instance \u003d ri.keepalived_manager.config.get_instance(ri.ha_vr_id)"},{"line_number":200,"context_line":"        existing_cidrs \u003d set([vip.ip_address"},{"line_number":201,"context_line":"                              for vip in instance.vip_addresses])"},{"line_number":202,"context_line":"        new_cidrs \u003d set()"},{"line_number":203,"context_line":""},{"line_number":204,"context_line":"        for fip in ri.router.get(l3_constants.FLOATINGIP_KEY, []):"}],"source_content_type":"text/x-python","patch_set":37,"id":"fa98f980_5a367be8","line":201,"updated":"2014-09-02 17:04:36.000000000","message":"floating ips are not working for me. Attribute vip_addresses does not exist - the exception is caught by an \"except Exception\" in l3_agent.py. My guess is that this line should be:\n\nexisting_cidrs \u003d set([vip.ip_address for vip in instance.vips])","commit_id":"0b8a4de81bcb5f3bb0e11e7e04117a995cca4126"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"2e03eb7e044229ff40709bb56fe0a4748a8a088a","unresolved":false,"context_lines":[{"line_number":198,"context_line":""},{"line_number":199,"context_line":"        instance \u003d ri.keepalived_manager.config.get_instance(ri.ha_vr_id)"},{"line_number":200,"context_line":"        existing_cidrs \u003d set([vip.ip_address"},{"line_number":201,"context_line":"                              for vip in instance.vip_addresses])"},{"line_number":202,"context_line":"        new_cidrs \u003d set()"},{"line_number":203,"context_line":""},{"line_number":204,"context_line":"        for fip in ri.router.get(l3_constants.FLOATINGIP_KEY, []):"}],"source_content_type":"text/x-python","patch_set":37,"id":"fa98f980_6068e187","line":201,"in_reply_to":"fa98f980_5a367be8","updated":"2014-09-02 22:24:05.000000000","message":"This wasn\u0027t covered by the functional test. It is now - Thanks!","commit_id":"0b8a4de81bcb5f3bb0e11e7e04117a995cca4126"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"249c6f22eadfe68d1a3cd65a98d0e69214239088","unresolved":false,"context_lines":[{"line_number":198,"context_line":""},{"line_number":199,"context_line":"        instance \u003d ri.keepalived_manager.config.get_instance(ri.ha_vr_id)"},{"line_number":200,"context_line":"        existing_cidrs \u003d set([vip.ip_address"},{"line_number":201,"context_line":"                              for vip in instance.vip_addresses])"},{"line_number":202,"context_line":"        new_cidrs \u003d set()"},{"line_number":203,"context_line":""},{"line_number":204,"context_line":"        for fip in ri.router.get(l3_constants.FLOATINGIP_KEY, []):"}],"source_content_type":"text/x-python","patch_set":37,"id":"fa98f980_052484ad","line":201,"in_reply_to":"fa98f980_6068e187","updated":"2014-09-03 04:37:46.000000000","message":"Darragh, it is comforting to know that a reviewer is actually running the code.  :)","commit_id":"0b8a4de81bcb5f3bb0e11e7e04117a995cca4126"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"061d57603e77fc0aa15ebbf56fa4c2fd0f3d9c89","unresolved":false,"context_lines":[{"line_number":80,"context_line":"            \u0027state\u0027)"},{"line_number":81,"context_line":"        try:"},{"line_number":82,"context_line":"            with open(state_path, \u0027r\u0027) as f:"},{"line_number":83,"context_line":"                output \u003d f.read()"},{"line_number":84,"context_line":"        except (OSError, IOError):"},{"line_number":85,"context_line":"            LOG.debug(\u0027Error while reading HA state for %s\u0027, self.router_id)"},{"line_number":86,"context_line":"            return None"}],"source_content_type":"text/x-python","patch_set":38,"id":"fa98f980_c9337212","line":83,"updated":"2014-09-03 04:53:06.000000000","message":"nit:  Just \"return f.read()\" to avoid an extra temporary and an extra \"else\" statement with this try.","commit_id":"2a9108692a88233f615db45143723037e0833e68"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"cfbe36f9ec96a20662fb8019bddda8449997427a","unresolved":false,"context_lines":[{"line_number":80,"context_line":"            \u0027state\u0027)"},{"line_number":81,"context_line":"        try:"},{"line_number":82,"context_line":"            with open(state_path, \u0027r\u0027) as f:"},{"line_number":83,"context_line":"                output \u003d f.read()"},{"line_number":84,"context_line":"        except (OSError, IOError):"},{"line_number":85,"context_line":"            LOG.debug(\u0027Error while reading HA state for %s\u0027, self.router_id)"},{"line_number":86,"context_line":"            return None"}],"source_content_type":"text/x-python","patch_set":38,"id":"fa98f980_9cd5aabb","line":83,"in_reply_to":"fa98f980_c9337212","updated":"2014-09-07 14:08:44.000000000","message":"Done","commit_id":"2a9108692a88233f615db45143723037e0833e68"},{"author":{"_account_id":7183,"name":"Xu Han Peng","email":"xuhanp@linux.vnet.ibm.com","username":"xuhanp"},"change_message_id":"7422ae29ff9c25458b4d87b541c3cbaf12ec72c6","unresolved":false,"context_lines":[{"line_number":165,"context_line":"                            namespace\u003dri.ns_name)"},{"line_number":166,"context_line":"        self._send_gratuitous_arp_packet("},{"line_number":167,"context_line":"            ri.ns_name, interface_name,"},{"line_number":168,"context_line":"            str(netaddr.IPNetwork(internal_cidr).ip))"},{"line_number":169,"context_line":""},{"line_number":170,"context_line":"    def ha_network_removed(self, ri):"},{"line_number":171,"context_line":"        interface_name \u003d self.get_ha_device_name(ri.ha_port[\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":38,"id":"fa98f980_dcf599ed","line":168,"updated":"2014-09-05 09:01:24.000000000","message":"I planned to leave this comment on patch 37 but a new patch updated, so put it here:\n\nSending GARP is documented in VRRP RFC:\n\nhttp://tools.ietf.org/html/rfc5798#section-6.4.2\n\n     (365) - If the Master_Down_Timer fires, then:\n\n         (370) + Send an ADVERTISEMENT\n\n         (375) + If the protected IPvX address is an IPv4 address, then:\n\n            (380) * Broadcast a gratuitous ARP request on that interface\n            containing the virtual router MAC address for each IPv4\n            address associated with the virtual router.\n\nIt\u0027s also documented at http://tools.ietf.org/html/rfc5798#section-2.4, to trigger a message immediately after transitioning to the Master to update the station learning. \n\nI think GARP is needed during failover, but not sure it\u0027s relation with VRRP election process.","commit_id":"2a9108692a88233f615db45143723037e0833e68"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"cfbe36f9ec96a20662fb8019bddda8449997427a","unresolved":false,"context_lines":[{"line_number":165,"context_line":"                            namespace\u003dri.ns_name)"},{"line_number":166,"context_line":"        self._send_gratuitous_arp_packet("},{"line_number":167,"context_line":"            ri.ns_name, interface_name,"},{"line_number":168,"context_line":"            str(netaddr.IPNetwork(internal_cidr).ip))"},{"line_number":169,"context_line":""},{"line_number":170,"context_line":"    def ha_network_removed(self, ri):"},{"line_number":171,"context_line":"        interface_name \u003d self.get_ha_device_name(ri.ha_port[\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":38,"id":"fa98f980_1ce1ba9a","line":168,"in_reply_to":"fa98f980_85bf3060","updated":"2014-09-07 14:08:44.000000000","message":"We don\u0027t send GARPs for the internal interfaces for HA routers and I removed the GARP here as well, it\u0027s not needed.","commit_id":"2a9108692a88233f615db45143723037e0833e68"},{"author":{"_account_id":2733,"name":"Darragh O\u0027Reilly","email":"doreilly@suse.com","username":"darragh-oreilly"},"change_message_id":"586314c62889b32d183ede8f56c0fc492d470420","unresolved":false,"context_lines":[{"line_number":165,"context_line":"                            namespace\u003dri.ns_name)"},{"line_number":166,"context_line":"        self._send_gratuitous_arp_packet("},{"line_number":167,"context_line":"            ri.ns_name, interface_name,"},{"line_number":168,"context_line":"            str(netaddr.IPNetwork(internal_cidr).ip))"},{"line_number":169,"context_line":""},{"line_number":170,"context_line":"    def ha_network_removed(self, ri):"},{"line_number":171,"context_line":"        interface_name \u003d self.get_ha_device_name(ri.ha_port[\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":38,"id":"fa98f980_85bf3060","line":168,"in_reply_to":"fa98f980_dcf599ed","updated":"2014-09-05 11:05:19.000000000","message":"The keepalived.conf manpage has this:\n\n    # delay for gratuitous ARP after transition to MASTER\n    garp_master_delay 10 # secs, default 5\n\nSo we probably don\u0027t need to send more arps with arping for ips on the qr- and qg- interfaces. And I don\u0027t see the point of doing it for ips on the ha- interfaces - like this method is doing.","commit_id":"2a9108692a88233f615db45143723037e0833e68"},{"author":{"_account_id":2733,"name":"Darragh O\u0027Reilly","email":"doreilly@suse.com","username":"darragh-oreilly"},"change_message_id":"24f59ba1f0a429808773d251cabec8250a74fa20","unresolved":false,"context_lines":[{"line_number":230,"context_line":"                [\u0027kill\u0027, \u0027-%s\u0027 % signal.SIGKILL,"},{"line_number":231,"context_line":"                 \u0027$(cat \u0027 + pid + \u0027)\u0027], state, ri.ha_vr_id)"},{"line_number":232,"context_line":""},{"line_number":233,"context_line":"    def _ha_external_gateway_added(self, ri, ex_gw_port, interface_name):"},{"line_number":234,"context_line":"        self._add_keepalived_vip(ri, ex_gw_port[\u0027ip_cidr\u0027],"},{"line_number":235,"context_line":"                                 interface_name)"},{"line_number":236,"context_line":"        gw_ip \u003d ex_gw_port[\u0027subnet\u0027][\u0027gateway_ip\u0027]"}],"source_content_type":"text/x-python","patch_set":38,"id":"fa98f980_ec7812c6","line":233,"updated":"2014-09-04 10:19:52.000000000","message":"Every time a new floating ip is created, the snat ip and default route gets added again. e.g. here it is after 2 floating ips have been added:\n\n...\n    virtual_ipaddress_excluded {\n        172.24.4.2/24 dev qg-ad41c629-bc\n        172.24.4.2/24 dev qg-ad41c629-bc\n        172.24.4.2/24 dev qg-ad41c629-bc\n        172.24.4.3/32 dev qg-ad41c629-bc\n        172.24.4.4/32 dev qg-ad41c629-bc\n    }\n    virtual_routes {\n        0.0.0.0/0 via 172.24.4.1 dev qg-ad41c629-bc\n        0.0.0.0/0 via 172.24.4.1 dev qg-ad41c629-bc\n        0.0.0.0/0 via 172.24.4.1 dev qg-ad41c629-bc\n    }","commit_id":"2a9108692a88233f615db45143723037e0833e68"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"cfbe36f9ec96a20662fb8019bddda8449997427a","unresolved":false,"context_lines":[{"line_number":230,"context_line":"                [\u0027kill\u0027, \u0027-%s\u0027 % signal.SIGKILL,"},{"line_number":231,"context_line":"                 \u0027$(cat \u0027 + pid + \u0027)\u0027], state, ri.ha_vr_id)"},{"line_number":232,"context_line":""},{"line_number":233,"context_line":"    def _ha_external_gateway_added(self, ri, ex_gw_port, interface_name):"},{"line_number":234,"context_line":"        self._add_keepalived_vip(ri, ex_gw_port[\u0027ip_cidr\u0027],"},{"line_number":235,"context_line":"                                 interface_name)"},{"line_number":236,"context_line":"        gw_ip \u003d ex_gw_port[\u0027subnet\u0027][\u0027gateway_ip\u0027]"}],"source_content_type":"text/x-python","patch_set":38,"id":"fa98f980_3cbbde76","line":233,"in_reply_to":"fa98f980_ec7812c6","updated":"2014-09-07 14:08:44.000000000","message":"I commented in the main comments section as well, I\u0027ll just add a final update here: First off thanks Darragh for taking the time to actually run the code, it\u0027s much appreciated. Secondly I nailed this down to a patch that changed the way we add/remove/update the external gateway device. The code now compares the current and new ex_gw devices, and if they differ, it runs an update function, which internally added the new VIP and virtual route. It was comparing fields that made no sense in the HA context so the update method was called every time.","commit_id":"2a9108692a88233f615db45143723037e0833e68"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"061d57603e77fc0aa15ebbf56fa4c2fd0f3d9c89","unresolved":false,"context_lines":[{"line_number":252,"context_line":"                route[\u0027nexthop\u0027]))"},{"line_number":253,"context_line":""},{"line_number":254,"context_line":"    @periodic_task.periodic_task"},{"line_number":255,"context_line":"    def _check_keepalived_alive(self, context):"},{"line_number":256,"context_line":"        router_infos \u003d self.router_info.values()"},{"line_number":257,"context_line":"        for ri in router_infos:"},{"line_number":258,"context_line":"            if ri.is_ha:"}],"source_content_type":"text/x-python","patch_set":38,"id":"fa98f980_e93836f1","line":255,"updated":"2014-09-03 04:53:06.000000000","message":"s/check/ensure/ maybe?","commit_id":"2a9108692a88233f615db45143723037e0833e68"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"cfbe36f9ec96a20662fb8019bddda8449997427a","unresolved":false,"context_lines":[{"line_number":252,"context_line":"                route[\u0027nexthop\u0027]))"},{"line_number":253,"context_line":""},{"line_number":254,"context_line":"    @periodic_task.periodic_task"},{"line_number":255,"context_line":"    def _check_keepalived_alive(self, context):"},{"line_number":256,"context_line":"        router_infos \u003d self.router_info.values()"},{"line_number":257,"context_line":"        for ri in router_infos:"},{"line_number":258,"context_line":"            if ri.is_ha:"}],"source_content_type":"text/x-python","patch_set":38,"id":"fa98f980_fca87622","line":255,"in_reply_to":"fa98f980_e93836f1","updated":"2014-09-07 14:08:44.000000000","message":"Done","commit_id":"2a9108692a88233f615db45143723037e0833e68"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"f1a581b3264bf61213a6b9394033fc28d3542765","unresolved":false,"context_lines":[{"line_number":28,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"HA_DEV_PREFIX \u003d \u0027ha-\u0027"},{"line_number":31,"context_line":"FLOATING_IP_CIDR_SUFFIX \u003d \u0027/32\u0027"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"OPTS \u003d ["},{"line_number":34,"context_line":"    cfg.StrOpt(\u0027ha_confs_path\u0027,"}],"source_content_type":"text/x-python","patch_set":43,"id":"fa98f980_9b638219","line":31,"updated":"2014-09-10 21:38:58.000000000","message":"This is unused.  Only the equivalent in l3_agent.py is used.  Remove this one.","commit_id":"d883a0a52125fa62730ff5a9ad9f5c2381cb2340"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"a0be84f6c95d10bbab879ad1d68eb557e719ee57","unresolved":false,"context_lines":[{"line_number":28,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"HA_DEV_PREFIX \u003d \u0027ha-\u0027"},{"line_number":31,"context_line":"FLOATING_IP_CIDR_SUFFIX \u003d \u0027/32\u0027"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"OPTS \u003d ["},{"line_number":34,"context_line":"    cfg.StrOpt(\u0027ha_confs_path\u0027,"}],"source_content_type":"text/x-python","patch_set":43,"id":"fa98f980_70d417bd","line":31,"in_reply_to":"fa98f980_9b638219","updated":"2014-09-11 18:28:46.000000000","message":"Done. Good catch. I must have added this in, used this somewhere and then removed the usage.","commit_id":"d883a0a52125fa62730ff5a9ad9f5c2381cb2340"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"f1a581b3264bf61213a6b9394033fc28d3542765","unresolved":false,"context_lines":[{"line_number":145,"context_line":""},{"line_number":146,"context_line":"        self._init_keepalived_manager(ri)"},{"line_number":147,"context_line":""},{"line_number":148,"context_line":"    def process_ha_router_removed(self, ri):"},{"line_number":149,"context_line":"        self.ha_network_removed(ri)"},{"line_number":150,"context_line":""},{"line_number":151,"context_line":"    def get_ha_device_name(self, port_id):"}],"source_content_type":"text/x-python","patch_set":43,"id":"fa98f980_dd77c59d","line":148,"updated":"2014-09-10 21:38:58.000000000","message":"nit:  This is a trivial pass-through.  Why not just inline?  ha_network_removed doesn\u0027t seem to be called from anywhere else.","commit_id":"d883a0a52125fa62730ff5a9ad9f5c2381cb2340"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"a0be84f6c95d10bbab879ad1d68eb557e719ee57","unresolved":false,"context_lines":[{"line_number":145,"context_line":""},{"line_number":146,"context_line":"        self._init_keepalived_manager(ri)"},{"line_number":147,"context_line":""},{"line_number":148,"context_line":"    def process_ha_router_removed(self, ri):"},{"line_number":149,"context_line":"        self.ha_network_removed(ri)"},{"line_number":150,"context_line":""},{"line_number":151,"context_line":"    def get_ha_device_name(self, port_id):"}],"source_content_type":"text/x-python","patch_set":43,"id":"fa98f980_cd036c0e","line":148,"in_reply_to":"fa98f980_dd77c59d","updated":"2014-09-11 18:28:46.000000000","message":"I think there\u0027s something to be said for consistency with process_ha_router_added. Aids to readability a bit in my opinion.","commit_id":"d883a0a52125fa62730ff5a9ad9f5c2381cb2340"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"f1a581b3264bf61213a6b9394033fc28d3542765","unresolved":false,"context_lines":[{"line_number":168,"context_line":""},{"line_number":169,"context_line":"    def _add_vip(self, ri, ip_cidr, interface):"},{"line_number":170,"context_line":"        instance \u003d ri.keepalived_manager.config.get_instance(ri.ha_vr_id)"},{"line_number":171,"context_line":"        instance.vips.append("},{"line_number":172,"context_line":"            keepalived.KeepalivedVipAddress(ip_cidr, interface))"},{"line_number":173,"context_line":""},{"line_number":174,"context_line":"    def _remove_vip(self, ri, ip_cidr):"}],"source_content_type":"text/x-python","patch_set":43,"id":"fa98f980_7a933ffa","line":171,"updated":"2014-09-10 21:38:58.000000000","message":"nit: direct access to vips.append here where others below access through a method on instance.  Access through method would be preferred.","commit_id":"d883a0a52125fa62730ff5a9ad9f5c2381cb2340"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"a0be84f6c95d10bbab879ad1d68eb557e719ee57","unresolved":false,"context_lines":[{"line_number":168,"context_line":""},{"line_number":169,"context_line":"    def _add_vip(self, ri, ip_cidr, interface):"},{"line_number":170,"context_line":"        instance \u003d ri.keepalived_manager.config.get_instance(ri.ha_vr_id)"},{"line_number":171,"context_line":"        instance.vips.append("},{"line_number":172,"context_line":"            keepalived.KeepalivedVipAddress(ip_cidr, interface))"},{"line_number":173,"context_line":""},{"line_number":174,"context_line":"    def _remove_vip(self, ri, ip_cidr):"}],"source_content_type":"text/x-python","patch_set":43,"id":"fa98f980_300c4fed","line":171,"in_reply_to":"fa98f980_7a933ffa","updated":"2014-09-11 18:28:46.000000000","message":"Done","commit_id":"d883a0a52125fa62730ff5a9ad9f5c2381cb2340"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"f1a581b3264bf61213a6b9394033fc28d3542765","unresolved":false,"context_lines":[{"line_number":192,"context_line":""},{"line_number":193,"context_line":"    def _ha_external_gateway_updated(self, ri, ex_gw_port, interface_name):"},{"line_number":194,"context_line":"        old_gateway_cidr \u003d ri.ex_gw_port[\u0027ip_cidr\u0027]"},{"line_number":195,"context_line":"        self._remove_vip(ri, old_gateway_cidr)"},{"line_number":196,"context_line":"        self._ha_external_gateway_added(ri, ex_gw_port, interface_name)"},{"line_number":197,"context_line":""},{"line_number":198,"context_line":"    def _add_default_gw_virtual_route(self, ri, ex_gw_port, interface_name):"}],"source_content_type":"text/x-python","patch_set":43,"id":"fa98f980_ddd3455d","line":195,"updated":"2014-09-10 21:38:58.000000000","message":"nit: _ha_external_gateway_removed for symmetry?","commit_id":"d883a0a52125fa62730ff5a9ad9f5c2381cb2340"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"a0be84f6c95d10bbab879ad1d68eb557e719ee57","unresolved":false,"context_lines":[{"line_number":192,"context_line":""},{"line_number":193,"context_line":"    def _ha_external_gateway_updated(self, ri, ex_gw_port, interface_name):"},{"line_number":194,"context_line":"        old_gateway_cidr \u003d ri.ex_gw_port[\u0027ip_cidr\u0027]"},{"line_number":195,"context_line":"        self._remove_vip(ri, old_gateway_cidr)"},{"line_number":196,"context_line":"        self._ha_external_gateway_added(ri, ex_gw_port, interface_name)"},{"line_number":197,"context_line":""},{"line_number":198,"context_line":"    def _add_default_gw_virtual_route(self, ri, ex_gw_port, interface_name):"}],"source_content_type":"text/x-python","patch_set":43,"id":"fa98f980_30e44fe0","line":195,"in_reply_to":"fa98f980_ddd3455d","updated":"2014-09-11 18:28:46.000000000","message":"I added the function and I call it in another place, but I can\u0027t call it here.","commit_id":"d883a0a52125fa62730ff5a9ad9f5c2381cb2340"},{"author":{"_account_id":6854,"name":"YAMAMOTO Takashi","email":"yamamoto@midokura.com","username":"yamamoto"},"change_message_id":"86dc75fbff3702670e32a0fd501f9f500c92b262","unresolved":false,"context_lines":[{"line_number":83,"context_line":"            LOG.debug(\u0027Error while reading HA state for %s\u0027, self.router_id)"},{"line_number":84,"context_line":"            return None"},{"line_number":85,"context_line":""},{"line_number":86,"context_line":"    def spawn_keepalived(self):"},{"line_number":87,"context_line":"        self.keepalived_manager.spawn_or_restart()"},{"line_number":88,"context_line":""},{"line_number":89,"context_line":"    def disable_keepalived(self):"}],"source_content_type":"text/x-python","patch_set":46,"id":"fa98f980_39bb4ce6","line":86,"updated":"2014-09-12 07:03:54.000000000","message":"can you try to avoid exposing implementation (keepalived) in method names?","commit_id":"be9a3efc99988b423a9e6cfc1b387c63ed06fdaa"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"05d421bdf45ff49f08641acd032968c4faa63d2f","unresolved":false,"context_lines":[{"line_number":83,"context_line":"            LOG.debug(\u0027Error while reading HA state for %s\u0027, self.router_id)"},{"line_number":84,"context_line":"            return None"},{"line_number":85,"context_line":""},{"line_number":86,"context_line":"    def spawn_keepalived(self):"},{"line_number":87,"context_line":"        self.keepalived_manager.spawn_or_restart()"},{"line_number":88,"context_line":""},{"line_number":89,"context_line":"    def disable_keepalived(self):"}],"source_content_type":"text/x-python","patch_set":46,"id":"fa98f980_34d4311f","line":86,"in_reply_to":"fa98f980_39bb4ce6","updated":"2014-09-12 07:46:55.000000000","message":"The entire implementation leaks details like crazy, I wouldn\u0027t worry about the naming of this specific method ;-)\n\nWe have plans to introduce a driver API for HA, where keepalived will be the only implementation, just in the sake of better separation of concerns. We also aim to refactor the L3 agent itself to get rid of the conditionals: If distributed do this, if HA do that...","commit_id":"be9a3efc99988b423a9e6cfc1b387c63ed06fdaa"}],"neutron/agent/linux/keepalived.py":[{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"ff594973e6d86df5cbe6041c29e347f10b19a93c","unresolved":false,"context_lines":[{"line_number":82,"context_line":"    def __init__(self, ha_vr_id):"},{"line_number":83,"context_line":"        self.ha_vr_id \u003d ha_vr_id"},{"line_number":84,"context_line":"        self.name \u003d \u0027VG_%s\u0027 % ha_vr_id"},{"line_number":85,"context_line":"        self.instance_names \u003d set()"},{"line_number":86,"context_line":"        self.notifiers \u003d []"},{"line_number":87,"context_line":""},{"line_number":88,"context_line":"    def add_instance(self, instance):"}],"source_content_type":"text/x-python","patch_set":41,"id":"fa98f980_52088991","line":85,"updated":"2014-09-07 17:14:01.000000000","message":"This patch adds a functional test for keepalived configuration, but notifiers were persisted in a dict so the output could have been in any order, and the functional test failed every once in a while.","commit_id":"19773ac03e46a95d45d7c44853bc486d900105b4"}],"neutron/common/constants.py":[{"author":{"_account_id":7141,"name":"Sylvain Afchain","email":"safchain@redhat.com","username":"sylvainA"},"change_message_id":"058d5f6bbfc4f97e6ea077a17cc311ab1a44e8af","unresolved":false,"context_lines":[{"line_number":41,"context_line":""},{"line_number":42,"context_line":"FLOATINGIP_KEY \u003d \u0027_floatingips\u0027"},{"line_number":43,"context_line":"INTERFACE_KEY \u003d \u0027_interfaces\u0027"},{"line_number":44,"context_line":"HA_INTERFACE_KEY \u003d \u0027_ha_interfaces\u0027"},{"line_number":45,"context_line":"METERING_LABEL_KEY \u003d \u0027_metering_labels\u0027"},{"line_number":46,"context_line":"FLOATINGIP_AGENT_INTF_KEY \u003d \u0027_floatingip_agent_interfaces\u0027"},{"line_number":47,"context_line":"SNAT_ROUTER_INTF_KEY \u003d \u0027_snat_router_interfaces\u0027"}],"source_content_type":"text/x-python","patch_set":19,"id":"baada198_65a55802","line":44,"updated":"2014-08-07 14:49:08.000000000","message":"Assaf, since the list of dict has been replaced by a dict, I changed _ha_interfaces to _ha_interface","commit_id":"82006c4398f5ef9426fd0ebaa91b63a8110d5647"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"4ad78cb5c50a8b0167f8e8897e43f3457091d1b9","unresolved":false,"context_lines":[{"line_number":41,"context_line":""},{"line_number":42,"context_line":"FLOATINGIP_KEY \u003d \u0027_floatingips\u0027"},{"line_number":43,"context_line":"INTERFACE_KEY \u003d \u0027_interfaces\u0027"},{"line_number":44,"context_line":"HA_INTERFACE_KEY \u003d \u0027_ha_interfaces\u0027"},{"line_number":45,"context_line":"METERING_LABEL_KEY \u003d \u0027_metering_labels\u0027"},{"line_number":46,"context_line":"FLOATINGIP_AGENT_INTF_KEY \u003d \u0027_floatingip_agent_interfaces\u0027"},{"line_number":47,"context_line":"SNAT_ROUTER_INTF_KEY \u003d \u0027_snat_router_interfaces\u0027"}],"source_content_type":"text/x-python","patch_set":19,"id":"baada198_45ce942e","line":44,"in_reply_to":"baada198_65a55802","updated":"2014-08-07 15:04:18.000000000","message":"Done - Thanks, sloppy of me to forget this detail.","commit_id":"82006c4398f5ef9426fd0ebaa91b63a8110d5647"}],"neutron/services/firewall/agents/varmour/varmour_router.py":[{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"061d57603e77fc0aa15ebbf56fa4c2fd0f3d9c89","unresolved":false,"context_lines":[{"line_number":91,"context_line":""},{"line_number":92,"context_line":"            del self.router_info[router_id]"},{"line_number":93,"context_line":""},{"line_number":94,"context_line":"    def _spawn_metadata_proxy(self, router_id, ns_name):"},{"line_number":95,"context_line":"        return"},{"line_number":96,"context_line":""},{"line_number":97,"context_line":"    def _destroy_metadata_proxy(self, router_id, ns_name):"}],"source_content_type":"text/x-python","patch_set":38,"id":"fa98f980_293f9ed7","line":94,"updated":"2014-09-03 04:53:06.000000000","message":"Oops, doesn\u0027t match the super class any longer.","commit_id":"2a9108692a88233f615db45143723037e0833e68"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"cfbe36f9ec96a20662fb8019bddda8449997427a","unresolved":false,"context_lines":[{"line_number":91,"context_line":""},{"line_number":92,"context_line":"            del self.router_info[router_id]"},{"line_number":93,"context_line":""},{"line_number":94,"context_line":"    def _spawn_metadata_proxy(self, router_id, ns_name):"},{"line_number":95,"context_line":"        return"},{"line_number":96,"context_line":""},{"line_number":97,"context_line":"    def _destroy_metadata_proxy(self, router_id, ns_name):"}],"source_content_type":"text/x-python","patch_set":38,"id":"fa98f980_5ccb421e","line":94,"in_reply_to":"fa98f980_293f9ed7","updated":"2014-09-07 14:08:44.000000000","message":"Done","commit_id":"2a9108692a88233f615db45143723037e0833e68"}],"neutron/tests/functional/agent/test_l3_agent.py":[{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"231c4ded04037ce2ddff54337f8ffddd6f6d46d4","unresolved":false,"context_lines":[{"line_number":111,"context_line":"            expected_device[\u0027ip_cidr\u0027],"},{"line_number":112,"context_line":"            expected_device[\u0027mac_address\u0027],"},{"line_number":113,"context_line":"            self.root_helper, namespace)"},{"line_number":114,"context_line":""},{"line_number":115,"context_line":"    def _wait_until(self, predicate, timeout\u003dTIMEOUT,"},{"line_number":116,"context_line":"                    sleep_interval\u003dSLEEP_INTERVAL, *args, **kwargs):"},{"line_number":117,"context_line":"        retry_until \u003d time.time() + timeout"}],"source_content_type":"text/x-python","patch_set":23,"id":"baada198_06593800","line":114,"updated":"2014-08-13 14:21:32.000000000","message":"Moving this to a reusable place + Changing it so hitting the timeout will raise an exception.","commit_id":"319855385855259839e097f6259e623315e7ee18"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"231c4ded04037ce2ddff54337f8ffddd6f6d46d4","unresolved":false,"context_lines":[{"line_number":129,"context_line":"    def _router_lifecycle(self, enable_ha):"},{"line_number":130,"context_line":"        router \u003d self.manage_router(enable_ha)"},{"line_number":131,"context_line":""},{"line_number":132,"context_line":"        if enable_ha:"},{"line_number":133,"context_line":"            self._wait_until(lambda: router.ha_state \u003d\u003d \u0027master\u0027)"},{"line_number":134,"context_line":""},{"line_number":135,"context_line":"            # Ugly hack time: The state transition notification occurs when"}],"source_content_type":"text/x-python","patch_set":23,"id":"baada198_a6decc3a","line":132,"updated":"2014-08-13 14:21:32.000000000","message":"I\u0027m going to change it so that until waiting until the router becomes the master + a flat amount of seconds for the keepalived operation to complete, I\u0027ll wait until the last VIP is configured instead.","commit_id":"319855385855259839e097f6259e623315e7ee18"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"658f440848950be4a5cfef6865cf1817d4d2b6de","unresolved":false,"context_lines":[{"line_number":118,"context_line":"        if enable_ha:"},{"line_number":119,"context_line":"            # Wait until the router becomes the VRRP master"},{"line_number":120,"context_line":"            self.wait_until(lambda: router.ha_state \u003d\u003d \u0027master\u0027)"},{"line_number":121,"context_line":""},{"line_number":122,"context_line":"            # Then wait that keepalived finished configuring everything."},{"line_number":123,"context_line":"            # We verify this by waiting until the last device has an IP."},{"line_number":124,"context_line":"            device \u003d router.router[l3_constants.INTERFACE_KEY][-1]"}],"source_content_type":"text/x-python","patch_set":25,"id":"7ac7a964_a25fb371","line":121,"updated":"2014-08-14 06:27:57.000000000","message":"I\u0027ll reword this comment a bit.","commit_id":"dba15f21c9eaed56ef7c706b5e506be778bfe26f"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"ff61f1c33f8ff64e2ae55e0acb3496de2dd261ef","unresolved":false,"context_lines":[{"line_number":99,"context_line":"            namespace, self.root_helper)"},{"line_number":100,"context_line":""},{"line_number":101,"context_line":""},{"line_number":102,"context_line":"class L3AgentTestCase(L3AgentTestFramework):"},{"line_number":103,"context_line":"    def test_legacy_router_lifecycle(self):"},{"line_number":104,"context_line":"        self._router_lifecycle(enable_ha\u003dFalse)"},{"line_number":105,"context_line":""}],"source_content_type":"text/x-python","patch_set":33,"id":"1abeadc6_e2f9cf80","line":102,"updated":"2014-08-29 23:09:55.000000000","message":"This is a good baseline, but do you think there is still time for a test that validates failover?","commit_id":"3ea68c8e5ac6a7908448e495b8398324f76ce8a5"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"63b011caaab7d190064a3a465d32603989a64f4d","unresolved":false,"context_lines":[{"line_number":99,"context_line":"            namespace, self.root_helper)"},{"line_number":100,"context_line":""},{"line_number":101,"context_line":""},{"line_number":102,"context_line":"class L3AgentTestCase(L3AgentTestFramework):"},{"line_number":103,"context_line":"    def test_legacy_router_lifecycle(self):"},{"line_number":104,"context_line":"        self._router_lifecycle(enable_ha\u003dFalse)"},{"line_number":105,"context_line":""}],"source_content_type":"text/x-python","patch_set":33,"id":"fa98f980_efda7616","line":102,"in_reply_to":"1abeadc6_e2f9cf80","updated":"2014-08-31 15:39:43.000000000","message":"Done in the next patch :) It was interesting technically so I obsessed over it and did it over the weekend.","commit_id":"3ea68c8e5ac6a7908448e495b8398324f76ce8a5"},{"author":{"_account_id":12737,"name":"Jacek Swiderski","email":"jacek.swiderski@codilime.com","username":"Altran"},"change_message_id":"6d1b019e95371096ffe9260552b308e7d97fc425","unresolved":false,"context_lines":[{"line_number":14,"context_line":"#    under the License."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"import mock"},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"from oslo.config import cfg"},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"from neutron.agent.common import config"}],"source_content_type":"text/x-python","patch_set":35,"id":"fa98f980_d1a622c1","line":17,"updated":"2014-09-01 11:00:35.000000000","message":"nit: remove - mock isn\u0027t part of stdlib in python2","commit_id":"0c10d4869b3163800d82ec9a3d2ff68033175ba1"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"291400d4be54b965a4f057263fd9190de51b7bd2","unresolved":false,"context_lines":[{"line_number":14,"context_line":"#    under the License."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"import mock"},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"from oslo.config import cfg"},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"from neutron.agent.common import config"}],"source_content_type":"text/x-python","patch_set":35,"id":"fa98f980_02de963f","line":17,"in_reply_to":"fa98f980_d1a622c1","updated":"2014-09-01 12:23:08.000000000","message":"Correct, done.","commit_id":"0c10d4869b3163800d82ec9a3d2ff68033175ba1"}],"neutron/tests/functional/base.py":[{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"658f440848950be4a5cfef6865cf1817d4d2b6de","unresolved":false,"context_lines":[{"line_number":58,"context_line":"    def check_sudo_enabled(self):"},{"line_number":59,"context_line":"        if not self.sudo_enabled:"},{"line_number":60,"context_line":"            self.skipTest(\u0027testing with sudo is not enabled\u0027)"},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"    def wait_until(self, predicate, *args, **kwargs):"},{"line_number":63,"context_line":"        retry_until \u003d time.time() + TIMEOUT"},{"line_number":64,"context_line":"        while not predicate(*args, **kwargs) and time.time() \u003c retry_until:"}],"source_content_type":"text/x-python","patch_set":25,"id":"7ac7a964_42c7bf12","line":61,"updated":"2014-08-14 06:27:57.000000000","message":"I forgot to fail the test if this function times out. I\u0027ll add in the next patchset.","commit_id":"dba15f21c9eaed56ef7c706b5e506be778bfe26f"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"ff61f1c33f8ff64e2ae55e0acb3496de2dd261ef","unresolved":false,"context_lines":[{"line_number":60,"context_line":"            self.skipTest(\u0027testing with sudo is not enabled\u0027)"},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"    def wait_until(self, predicate, *args, **kwargs):"},{"line_number":63,"context_line":"        with self.assert_max_execution_time(TIMEOUT):"},{"line_number":64,"context_line":"            while not predicate(*args, **kwargs):"},{"line_number":65,"context_line":"                time.sleep(SLEEP_INTERVAL)"}],"source_content_type":"text/x-python","patch_set":33,"id":"1abeadc6_42fc43b5","line":63,"updated":"2014-08-29 23:09:55.000000000","message":"I don\u0027t think a timeout of 60s is particular reasonable - do you actually expect it to take that long?","commit_id":"3ea68c8e5ac6a7908448e495b8398324f76ce8a5"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"63b011caaab7d190064a3a465d32603989a64f4d","unresolved":false,"context_lines":[{"line_number":60,"context_line":"            self.skipTest(\u0027testing with sudo is not enabled\u0027)"},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"    def wait_until(self, predicate, *args, **kwargs):"},{"line_number":63,"context_line":"        with self.assert_max_execution_time(TIMEOUT):"},{"line_number":64,"context_line":"            while not predicate(*args, **kwargs):"},{"line_number":65,"context_line":"                time.sleep(SLEEP_INTERVAL)"}],"source_content_type":"text/x-python","patch_set":33,"id":"fa98f980_8f03aa72","line":63,"in_reply_to":"1abeadc6_42fc43b5","updated":"2014-08-31 15:39:43.000000000","message":"It could if the VM running the job is busy. I wouldn\u0027t want weird time outs just because the VM is busy but the test would have eventually succeeded. The failover test for example, I\u0027ve seen take over 20 seconds, so 60 seems OK to me.","commit_id":"3ea68c8e5ac6a7908448e495b8398324f76ce8a5"}],"neutron/tests/unit/services/firewall/agents/l3reference/test_firewall_l3_agent.py":[{"author":{"_account_id":8213,"name":"Claudiu Belu","email":"cbelu@cloudbasesolutions.com","username":"claudiub"},"change_message_id":"fb3e7e93b2675631ac466f52428e5f70d9776eac","unresolved":false,"context_lines":[{"line_number":44,"context_line":"    neutron_service_plugins \u003d []"},{"line_number":45,"context_line":""},{"line_number":46,"context_line":""},{"line_number":47,"context_line":"def _setup_test_agent_class(service_plugins):"},{"line_number":48,"context_line":"    class FWaasTestAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback,"},{"line_number":49,"context_line":"                         FWaasHelper):"},{"line_number":50,"context_line":"        neutron_service_plugins \u003d service_plugins"}],"source_content_type":"text/x-python","patch_set":35,"id":"fa98f980_37ce7e84","line":47,"updated":"2014-09-01 11:53:42.000000000","message":"What was wrong with how it was before? (Having it as a class). I can see that you set the neutron_service_plugins, but that could have been passed to __init__ as an arg too. \n\nWhat is the reason that this form is preferred?","commit_id":"0c10d4869b3163800d82ec9a3d2ff68033175ba1"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"291400d4be54b965a4f057263fd9190de51b7bd2","unresolved":false,"context_lines":[{"line_number":44,"context_line":"    neutron_service_plugins \u003d []"},{"line_number":45,"context_line":""},{"line_number":46,"context_line":""},{"line_number":47,"context_line":"def _setup_test_agent_class(service_plugins):"},{"line_number":48,"context_line":"    class FWaasTestAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback,"},{"line_number":49,"context_line":"                         FWaasHelper):"},{"line_number":50,"context_line":"        neutron_service_plugins \u003d service_plugins"}],"source_content_type":"text/x-python","patch_set":35,"id":"fa98f980_a2c5ca5c","line":47,"in_reply_to":"fa98f980_37ce7e84","updated":"2014-09-01 12:23:08.000000000","message":"If you compare to \u0027base\u0027 you can see this change is a rebase, unrelated to this patch.","commit_id":"0c10d4869b3163800d82ec9a3d2ff68033175ba1"}],"neutron/tests/unit/test_l3_agent.py":[{"author":{"_account_id":8213,"name":"Claudiu Belu","email":"cbelu@cloudbasesolutions.com","username":"claudiub"},"change_message_id":"9e07ba4a3eabecfc9d8a3bc0a7ffcd69ef9cefb4","unresolved":false,"context_lines":[{"line_number":445,"context_line":"        router[l3_constants.FLOATINGIP_KEY] \u003d fake_fip[\u0027floatingips\u0027]"},{"line_number":446,"context_line":"        agent.external_gateway_updated(ri, ex_gw_port,"},{"line_number":447,"context_line":"                                     interface_name)"},{"line_number":448,"context_line":"        self.assertEqual(self.mock_driver.plug.call_count, 0)"},{"line_number":449,"context_line":"        self.assertEqual(self.mock_driver.init_l3.call_count, 1)"},{"line_number":450,"context_line":"        self.send_arp.assert_called_once_with(ri.ns_name, interface_name,"},{"line_number":451,"context_line":"                                              \u002720.0.0.30\u0027)"}],"source_content_type":"text/x-python","patch_set":21,"id":"baada198_27beabce","line":448,"updated":"2014-08-11 10:17:22.000000000","message":"I would prefer:\n\nself.assertFalse(self.mock_driver.plug.called)","commit_id":"7b6c61ef9bf0d15cad068556dcdbd47e6e9b8045"},{"author":{"_account_id":8213,"name":"Claudiu Belu","email":"cbelu@cloudbasesolutions.com","username":"claudiub"},"change_message_id":"9e07ba4a3eabecfc9d8a3bc0a7ffcd69ef9cefb4","unresolved":false,"context_lines":[{"line_number":446,"context_line":"        agent.external_gateway_updated(ri, ex_gw_port,"},{"line_number":447,"context_line":"                                     interface_name)"},{"line_number":448,"context_line":"        self.assertEqual(self.mock_driver.plug.call_count, 0)"},{"line_number":449,"context_line":"        self.assertEqual(self.mock_driver.init_l3.call_count, 1)"},{"line_number":450,"context_line":"        self.send_arp.assert_called_once_with(ri.ns_name, interface_name,"},{"line_number":451,"context_line":"                                              \u002720.0.0.30\u0027)"},{"line_number":452,"context_line":"        kwargs \u003d {\u0027preserve_ips\u0027: [\u0027192.168.1.34/32\u0027],"}],"source_content_type":"text/x-python","patch_set":21,"id":"baada198_a7a9db90","line":449,"updated":"2014-08-11 10:17:22.000000000","message":"I would prefer:\n\nself.mock_driver.init_l3.assert_called_once_with(...)","commit_id":"7b6c61ef9bf0d15cad068556dcdbd47e6e9b8045"},{"author":{"_account_id":8213,"name":"Claudiu Belu","email":"cbelu@cloudbasesolutions.com","username":"claudiub"},"change_message_id":"9e07ba4a3eabecfc9d8a3bc0a7ffcd69ef9cefb4","unresolved":false,"context_lines":[{"line_number":790,"context_line":"        agent.process_router_floating_ip_nat_rules.assert_called_with(ri)"},{"line_number":791,"context_line":"        agent.process_router_floating_ip_nat_rules.reset_mock()"},{"line_number":792,"context_line":"        self.assertEqual(agent.external_gateway_added.call_count, 0)"},{"line_number":793,"context_line":"        self.assertEqual(agent.external_gateway_updated.call_count, 0)"},{"line_number":794,"context_line":"        agent.external_gateway_added.reset_mock()"},{"line_number":795,"context_line":"        agent.external_gateway_updated.reset_mock()"},{"line_number":796,"context_line":""}],"source_content_type":"text/x-python","patch_set":21,"id":"baada198_27622ba7","line":793,"updated":"2014-08-11 10:17:22.000000000","message":"I would prefer:\n\nself.assertFalse(agent.external_gateway_added.called)\n\nself.assertFalse(agent.external_gateway_updated.called)","commit_id":"7b6c61ef9bf0d15cad068556dcdbd47e6e9b8045"},{"author":{"_account_id":8213,"name":"Claudiu Belu","email":"cbelu@cloudbasesolutions.com","username":"claudiub"},"change_message_id":"9e07ba4a3eabecfc9d8a3bc0a7ffcd69ef9cefb4","unresolved":false,"context_lines":[{"line_number":805,"context_line":"        ex_gw_port \u003d agent._get_ex_gw_port(ri)"},{"line_number":806,"context_line":"        agent.process_router_floating_ip_addresses.reset_mock()"},{"line_number":807,"context_line":"        agent.process_router_floating_ip_nat_rules.reset_mock()"},{"line_number":808,"context_line":"        self.assertEqual(agent.external_gateway_added.call_count, 0)"},{"line_number":809,"context_line":"        self.assertEqual(agent.external_gateway_updated.call_count, 1)"},{"line_number":810,"context_line":""},{"line_number":811,"context_line":"        # remove just the floating ips"}],"source_content_type":"text/x-python","patch_set":21,"id":"baada198_e7191324","line":808,"updated":"2014-08-11 10:17:22.000000000","message":"self.assertFalse(agent.external_gateway_added.called)","commit_id":"7b6c61ef9bf0d15cad068556dcdbd47e6e9b8045"},{"author":{"_account_id":8213,"name":"Claudiu Belu","email":"cbelu@cloudbasesolutions.com","username":"claudiub"},"change_message_id":"9e07ba4a3eabecfc9d8a3bc0a7ffcd69ef9cefb4","unresolved":false,"context_lines":[{"line_number":806,"context_line":"        agent.process_router_floating_ip_addresses.reset_mock()"},{"line_number":807,"context_line":"        agent.process_router_floating_ip_nat_rules.reset_mock()"},{"line_number":808,"context_line":"        self.assertEqual(agent.external_gateway_added.call_count, 0)"},{"line_number":809,"context_line":"        self.assertEqual(agent.external_gateway_updated.call_count, 1)"},{"line_number":810,"context_line":""},{"line_number":811,"context_line":"        # remove just the floating ips"},{"line_number":812,"context_line":"        del router[l3_constants.FLOATINGIP_KEY]"}],"source_content_type":"text/x-python","patch_set":21,"id":"baada198_07ea87d8","line":809,"updated":"2014-08-11 10:17:22.000000000","message":"I would prefer:\n\nagent.external_gateway_updated.assert_called_once_with(...)","commit_id":"7b6c61ef9bf0d15cad068556dcdbd47e6e9b8045"},{"author":{"_account_id":8213,"name":"Claudiu Belu","email":"cbelu@cloudbasesolutions.com","username":"claudiub"},"change_message_id":"784bd68a06b101de37be22b1feb23bfceb170d2a","unresolved":false,"context_lines":[{"line_number":870,"context_line":"            self.send_arp.assert_has_calls(expected_send_arp_call)"},{"line_number":871,"context_line":""},{"line_number":872,"context_line":"            self.external_process_p.start()"},{"line_number":873,"context_line":"            self.is_ha_p.start()"},{"line_number":874,"context_line":""},{"line_number":875,"context_line":"    def test_process_ha_router_removed(self):"},{"line_number":876,"context_line":"        self.is_ha_p.stop()"}],"source_content_type":"text/x-python","patch_set":21,"id":"baada198_cddc2674","line":873,"updated":"2014-08-11 15:26:58.000000000","message":"You don\u0027t have to start them again. The test is over and setUp will run before the next test, starting them anyways. :)","commit_id":"7b6c61ef9bf0d15cad068556dcdbd47e6e9b8045"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"d39e7c587912c1c9c1043361f0365e002e259268","unresolved":false,"context_lines":[{"line_number":870,"context_line":"            self.send_arp.assert_has_calls(expected_send_arp_call)"},{"line_number":871,"context_line":""},{"line_number":872,"context_line":"            self.external_process_p.start()"},{"line_number":873,"context_line":"            self.is_ha_p.start()"},{"line_number":874,"context_line":""},{"line_number":875,"context_line":"    def test_process_ha_router_removed(self):"},{"line_number":876,"context_line":"        self.is_ha_p.stop()"}],"source_content_type":"text/x-python","patch_set":21,"id":"baada198_29dd3942","line":873,"in_reply_to":"baada198_cddc2674","updated":"2014-08-13 11:58:59.000000000","message":"Done","commit_id":"7b6c61ef9bf0d15cad068556dcdbd47e6e9b8045"},{"author":{"_account_id":8213,"name":"Claudiu Belu","email":"cbelu@cloudbasesolutions.com","username":"claudiub"},"change_message_id":"784bd68a06b101de37be22b1feb23bfceb170d2a","unresolved":false,"context_lines":[{"line_number":908,"context_line":""},{"line_number":909,"context_line":"            expected \u003d [mock.call(ha_iface, prefix\u003d\u0027ha-\u0027,"},{"line_number":910,"context_line":"                                  namespace\u003dri.ns_name)]"},{"line_number":911,"context_line":"            self.mock_driver.unplug.assert_has_calls(expected)"},{"line_number":912,"context_line":""},{"line_number":913,"context_line":"        self.is_ha_p.start()"},{"line_number":914,"context_line":""}],"source_content_type":"text/x-python","patch_set":21,"id":"baada198_2d7da21e","line":911,"updated":"2014-08-11 15:26:58.000000000","message":"since unplug is called only once, you could use assert_called_once_with(...)","commit_id":"7b6c61ef9bf0d15cad068556dcdbd47e6e9b8045"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"d39e7c587912c1c9c1043361f0365e002e259268","unresolved":false,"context_lines":[{"line_number":908,"context_line":""},{"line_number":909,"context_line":"            expected \u003d [mock.call(ha_iface, prefix\u003d\u0027ha-\u0027,"},{"line_number":910,"context_line":"                                  namespace\u003dri.ns_name)]"},{"line_number":911,"context_line":"            self.mock_driver.unplug.assert_has_calls(expected)"},{"line_number":912,"context_line":""},{"line_number":913,"context_line":"        self.is_ha_p.start()"},{"line_number":914,"context_line":""}],"source_content_type":"text/x-python","patch_set":21,"id":"baada198_89be45ae","line":911,"in_reply_to":"baada198_2d7da21e","updated":"2014-08-13 11:58:59.000000000","message":"Done","commit_id":"7b6c61ef9bf0d15cad068556dcdbd47e6e9b8045"},{"author":{"_account_id":8213,"name":"Claudiu Belu","email":"cbelu@cloudbasesolutions.com","username":"claudiub"},"change_message_id":"784bd68a06b101de37be22b1feb23bfceb170d2a","unresolved":false,"context_lines":[{"line_number":910,"context_line":"                                  namespace\u003dri.ns_name)]"},{"line_number":911,"context_line":"            self.mock_driver.unplug.assert_has_calls(expected)"},{"line_number":912,"context_line":""},{"line_number":913,"context_line":"        self.is_ha_p.start()"},{"line_number":914,"context_line":""},{"line_number":915,"context_line":"    @mock.patch(\u0027neutron.agent.linux.ip_lib.IPDevice\u0027)"},{"line_number":916,"context_line":"    def _test_process_router_floating_ip_addresses_add(self, ri,"}],"source_content_type":"text/x-python","patch_set":21,"id":"baada198_2dc5a28f","line":913,"updated":"2014-08-11 15:26:58.000000000","message":"ditto.","commit_id":"7b6c61ef9bf0d15cad068556dcdbd47e6e9b8045"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"d39e7c587912c1c9c1043361f0365e002e259268","unresolved":false,"context_lines":[{"line_number":910,"context_line":"                                  namespace\u003dri.ns_name)]"},{"line_number":911,"context_line":"            self.mock_driver.unplug.assert_has_calls(expected)"},{"line_number":912,"context_line":""},{"line_number":913,"context_line":"        self.is_ha_p.start()"},{"line_number":914,"context_line":""},{"line_number":915,"context_line":"    @mock.patch(\u0027neutron.agent.linux.ip_lib.IPDevice\u0027)"},{"line_number":916,"context_line":"    def _test_process_router_floating_ip_addresses_add(self, ri,"}],"source_content_type":"text/x-python","patch_set":21,"id":"baada198_e9dc211a","line":913,"in_reply_to":"baada198_2dc5a28f","updated":"2014-08-13 11:58:59.000000000","message":"Done","commit_id":"7b6c61ef9bf0d15cad068556dcdbd47e6e9b8045"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"ff61f1c33f8ff64e2ae55e0acb3496de2dd261ef","unresolved":false,"context_lines":[{"line_number":979,"context_line":"        self.assertFalse(agent.process_router_floating_ip_addresses.called)"},{"line_number":980,"context_line":"        self.assertFalse(agent.process_router_floating_ip_nat_rules.called)"},{"line_number":981,"context_line":""},{"line_number":982,"context_line":"    def test_process_ha_router(self):"},{"line_number":983,"context_line":"        self.is_ha_p.stop()"},{"line_number":984,"context_line":"        self.external_process_p.stop()"},{"line_number":985,"context_line":""}],"source_content_type":"text/x-python","patch_set":33,"id":"1abeadc6_2218271d","line":982,"updated":"2014-08-29 23:09:55.000000000","message":"Holy mockmockmock.  Is it too late to ask for a refactor so this isn\u0027t necessary?  Or a functional test addition that renders it unnecessary?  In its current form I think little of the value of this test.","commit_id":"3ea68c8e5ac6a7908448e495b8398324f76ce8a5"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"63b011caaab7d190064a3a465d32603989a64f4d","unresolved":false,"context_lines":[{"line_number":979,"context_line":"        self.assertFalse(agent.process_router_floating_ip_addresses.called)"},{"line_number":980,"context_line":"        self.assertFalse(agent.process_router_floating_ip_nat_rules.called)"},{"line_number":981,"context_line":""},{"line_number":982,"context_line":"    def test_process_ha_router(self):"},{"line_number":983,"context_line":"        self.is_ha_p.stop()"},{"line_number":984,"context_line":"        self.external_process_p.stop()"},{"line_number":985,"context_line":""}],"source_content_type":"text/x-python","patch_set":33,"id":"fa98f980_2f79defc","line":982,"in_reply_to":"1abeadc6_2218271d","updated":"2014-08-31 15:39:43.000000000","message":"We already have a functional test that covers the same functionality (And more) as this test and the next. I\u0027ll remove them.","commit_id":"3ea68c8e5ac6a7908448e495b8398324f76ce8a5"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"ff61f1c33f8ff64e2ae55e0acb3496de2dd261ef","unresolved":false,"context_lines":[{"line_number":1058,"context_line":"            self.mock_driver.unplug.assert_called_once_with("},{"line_number":1059,"context_line":"                ha_iface, prefix\u003d\u0027ha-\u0027, namespace\u003dri.ns_name)"},{"line_number":1060,"context_line":""},{"line_number":1061,"context_line":"    def test_ha_router_keepalived_config(self):"},{"line_number":1062,"context_line":"        self.is_ha_p.stop()"},{"line_number":1063,"context_line":""},{"line_number":1064,"context_line":"        agent \u003d l3_agent.L3NATAgent(HOSTNAME, self.conf)"}],"source_content_type":"text/x-python","patch_set":33,"id":"1abeadc6_a2db574d","line":1061,"updated":"2014-08-29 23:09:55.000000000","message":"Again, this either shouldn\u0027t be done or should be able to be done without mocking.","commit_id":"3ea68c8e5ac6a7908448e495b8398324f76ce8a5"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"63b011caaab7d190064a3a465d32603989a64f4d","unresolved":false,"context_lines":[{"line_number":1058,"context_line":"            self.mock_driver.unplug.assert_called_once_with("},{"line_number":1059,"context_line":"                ha_iface, prefix\u003d\u0027ha-\u0027, namespace\u003dri.ns_name)"},{"line_number":1060,"context_line":""},{"line_number":1061,"context_line":"    def test_ha_router_keepalived_config(self):"},{"line_number":1062,"context_line":"        self.is_ha_p.stop()"},{"line_number":1063,"context_line":""},{"line_number":1064,"context_line":"        agent \u003d l3_agent.L3NATAgent(HOSTNAME, self.conf)"}],"source_content_type":"text/x-python","patch_set":33,"id":"fa98f980_0f46dabf","line":1061,"in_reply_to":"1abeadc6_a2db574d","updated":"2014-08-31 15:39:43.000000000","message":"I disagree for two reasons:\n\n1) I found it useful during development. Removing it entirely is not an option in my mind.\n\n2) Keepalived has a functional test, but it doesn\u0027t test the translation of of a router configuration to a keepalived configuration, because it tests keepalived in isolation (As it should). We could translate this to a functional test but I actually think a unit test works here: It\u0027s quick and easy and it tests what I need at a good-enough level while a functional test would be more expensive at the gate.","commit_id":"3ea68c8e5ac6a7908448e495b8398324f76ce8a5"}]}