)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"674a1f7204cacbffd5a388b357744d360a7b416f","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"*** DO NOT MERGE - NEEDS MORE WORK ***"},{"line_number":10,"context_line":""},{"line_number":11,"context_line":"Be optmistic and let the server set the FIP status to DOWN when disassociating"},{"line_number":12,"context_line":"a floating ip from a port. Waiting for the agent to make the state transition"},{"line_number":13,"context_line":"is prone to random delays due to its load, and there is really no good reason"},{"line_number":14,"context_line":"for waiting."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"9aa7fdbe_c85b7885","line":11,"updated":"2014-10-23 07:46:58.000000000","message":"keeping in mind https://bugs.launchpad.net/neutron/+bug/1334926 is it fair to say FIP is down (from user perspective) while it\u0027s actually active for some time before agent handles disassociation?","commit_id":"eb2118157f0cfa3f4955fabd2f13af4a8dcb8ad4"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"06153013f40adf2e49f78ee011f5678df207e03e","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"*** DO NOT MERGE - NEEDS MORE WORK ***"},{"line_number":10,"context_line":""},{"line_number":11,"context_line":"Be optmistic and let the server set the FIP status to DOWN when disassociating"},{"line_number":12,"context_line":"a floating ip from a port. Waiting for the agent to make the state transition"},{"line_number":13,"context_line":"is prone to random delays due to its load, and there is really no good reason"},{"line_number":14,"context_line":"for waiting."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"9aa7fdbe_5e957485","line":11,"in_reply_to":"9aa7fdbe_68744736","updated":"2014-10-23 14:44:11.000000000","message":"Oleg, yes the possibility here is that the FIP is marked down, but the agent has not processed it yet (or even worse ever). This is clearly a potential source of inconsistency.\n\nBut ultimately wouldn\u0027t a client check whether the FIP is first and foremost reachable? What does ACTIVE really mean? As a matter of fact today there\u0027s a window where the FIP is no longer reachable but its status it\u0027s still ACTIVE.\n\nAnother potential fix would be to change the L3 agent logic so that the agent turn the status of the FIP\u0027s to DOWN first before tearing down rules and namespaces. However this is not an easy fix.\n\nThe problem here stems from the fact that the tempest test does not wait for the state transition at all:\n\nhttps://review.openstack.org/#/c/102700/30/tempest/scenario/manager.py\n\nAs soon as the the FIP is no longer reachable, the test \u0027assumes\u0027 the status is DOWN. So another potential fix is to make method:\n\n  check_floating_ip_status\n\nwait that the state transition occurs.","commit_id":"eb2118157f0cfa3f4955fabd2f13af4a8dcb8ad4"},{"author":{"_account_id":8449,"name":"Marios Andreou","email":"marios.andreou@gmail.com","username":"marios"},"change_message_id":"58e2e52fb60fd3a774d5c6559b4f3ed118d7cd93","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"*** DO NOT MERGE - NEEDS MORE WORK ***"},{"line_number":10,"context_line":""},{"line_number":11,"context_line":"Be optmistic and let the server set the FIP status to DOWN when disassociating"},{"line_number":12,"context_line":"a floating ip from a port. Waiting for the agent to make the state transition"},{"line_number":13,"context_line":"is prone to random delays due to its load, and there is really no good reason"},{"line_number":14,"context_line":"for waiting."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"9aa7fdbe_68744736","line":11,"in_reply_to":"9aa7fdbe_c85b7885","updated":"2014-10-23 10:42:25.000000000","message":"yeah that\u0027s right Oleg - the update occurs but the subesquent (+ immediate) GET on that resource still shows the status as ACTIVE (cos the status update didn\u0027t get through yet)","commit_id":"eb2118157f0cfa3f4955fabd2f13af4a8dcb8ad4"},{"author":{"_account_id":8449,"name":"Marios Andreou","email":"marios.andreou@gmail.com","username":"marios"},"change_message_id":"b66102ce082963972e41b9122b60548ce6d66baf","unresolved":false,"context_lines":[{"line_number":17,"context_line":"can still occur, based on whether the L3 agent successfully disposed of the"},{"line_number":18,"context_line":"agent-side FIP resources."},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"Closes-bug: #1381617"},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"Change-Id: I2ad41e7deb33c8037d00364d402b109c3e64a246"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"9aa7fdbe_286f9f2c","line":20,"updated":"2014-10-23 10:53:01.000000000","message":"partial bug? (thought not 100% sure which one the \u0027centralised routers\u0027 bug was)","commit_id":"eb2118157f0cfa3f4955fabd2f13af4a8dcb8ad4"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"06153013f40adf2e49f78ee011f5678df207e03e","unresolved":false,"context_lines":[{"line_number":17,"context_line":"can still occur, based on whether the L3 agent successfully disposed of the"},{"line_number":18,"context_line":"agent-side FIP resources."},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"Closes-bug: #1381617"},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"Change-Id: I2ad41e7deb33c8037d00364d402b109c3e64a246"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"9aa7fdbe_7ee5d836","line":20,"in_reply_to":"9aa7fdbe_286f9f2c","updated":"2014-10-23 14:44:11.000000000","message":"Well, this plus yours does fix the bug ;)","commit_id":"eb2118157f0cfa3f4955fabd2f13af4a8dcb8ad4"}],"neutron/agent/l3_agent.py":[{"author":{"_account_id":8449,"name":"Marios Andreou","email":"marios.andreou@gmail.com","username":"marios"},"change_message_id":"b66102ce082963972e41b9122b60548ce6d66baf","unresolved":false,"context_lines":[{"line_number":988,"context_line":"        fip_statuses \u003d {}"},{"line_number":989,"context_line":"        try:"},{"line_number":990,"context_line":"            if ex_gw_port:"},{"line_number":991,"context_line":"                existing_floating_ips \u003d ri.floating_ips"},{"line_number":992,"context_line":"                self.process_router_floating_ip_nat_rules(ri)"},{"line_number":993,"context_line":"                ri.iptables_manager.defer_apply_off()"},{"line_number":994,"context_line":"                # Once NAT rules for floating IPs are safely in place"}],"source_content_type":"text/x-python","patch_set":2,"id":"9aa7fdbe_28ccff69","side":"PARENT","line":991,"updated":"2014-10-23 10:53:01.000000000","message":"is this stuff here just tidy up/drive-by nit fixes?","commit_id":"8a64427a5d169e16caa85b609034699823aae5c0"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"06153013f40adf2e49f78ee011f5678df207e03e","unresolved":false,"context_lines":[{"line_number":988,"context_line":"        fip_statuses \u003d {}"},{"line_number":989,"context_line":"        try:"},{"line_number":990,"context_line":"            if ex_gw_port:"},{"line_number":991,"context_line":"                existing_floating_ips \u003d ri.floating_ips"},{"line_number":992,"context_line":"                self.process_router_floating_ip_nat_rules(ri)"},{"line_number":993,"context_line":"                ri.iptables_manager.defer_apply_off()"},{"line_number":994,"context_line":"                # Once NAT rules for floating IPs are safely in place"}],"source_content_type":"text/x-python","patch_set":2,"id":"9aa7fdbe_3ec6f09a","side":"PARENT","line":991,"in_reply_to":"9aa7fdbe_28ccff69","updated":"2014-10-23 14:44:11.000000000","message":"since we set the status to DOWN in the server, this is no longer necessary","commit_id":"8a64427a5d169e16caa85b609034699823aae5c0"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"005d978768469f875a4b0fa746aa91ca0974a5bf","unresolved":false,"context_lines":[{"line_number":988,"context_line":"        fip_statuses \u003d {}"},{"line_number":989,"context_line":"        try:"},{"line_number":990,"context_line":"            if ex_gw_port:"},{"line_number":991,"context_line":"                existing_floating_ips \u003d ri.floating_ips"},{"line_number":992,"context_line":"                self.process_router_floating_ip_nat_rules(ri)"},{"line_number":993,"context_line":"                ri.iptables_manager.defer_apply_off()"},{"line_number":994,"context_line":"                # Once NAT rules for floating IPs are safely in place"}],"source_content_type":"text/x-python","patch_set":4,"id":"9aa7fdbe_31e61ee9","side":"PARENT","line":991,"updated":"2014-10-23 19:43:36.000000000","message":"Hey, if you take these 4 lines out of process_router, it is now down to a slim and trim 137 lines.  No need to refactor it now.  ;)","commit_id":"dfe800bbd5b8cfe13b6c3b662995064ffed055a6"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"b2ef0e0c3cacea21da6cdd89c286555cca50172e","unresolved":false,"context_lines":[{"line_number":988,"context_line":"        fip_statuses \u003d {}"},{"line_number":989,"context_line":"        try:"},{"line_number":990,"context_line":"            if ex_gw_port:"},{"line_number":991,"context_line":"                existing_floating_ips \u003d ri.floating_ips"},{"line_number":992,"context_line":"                self.process_router_floating_ip_nat_rules(ri)"},{"line_number":993,"context_line":"                ri.iptables_manager.defer_apply_off()"},{"line_number":994,"context_line":"                # Once NAT rules for floating IPs are safely in place"}],"source_content_type":"text/x-python","patch_set":4,"id":"9aa7fdbe_e5be9064","side":"PARENT","line":991,"in_reply_to":"9aa7fdbe_31e61ee9","updated":"2014-10-23 20:23:27.000000000","message":":)","commit_id":"dfe800bbd5b8cfe13b6c3b662995064ffed055a6"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"005d978768469f875a4b0fa746aa91ca0974a5bf","unresolved":false,"context_lines":[{"line_number":1187,"context_line":"                fip_statuses[fip[\u0027id\u0027]] \u003d self._add_floating_ip("},{"line_number":1188,"context_line":"                    ri, fip, interface_name, device)"},{"line_number":1189,"context_line":""},{"line_number":1190,"context_line":"        # Set status to DOWN before doing the removal."},{"line_number":1191,"context_line":"        down_fips \u003d dict((fip_id, l3_constants.FLOATINGIP_STATUS_DOWN)"},{"line_number":1192,"context_line":"            for fip_id in ri.floating_ips - set(fip_statuses.keys()))"},{"line_number":1193,"context_line":"        self.plugin_rpc.update_floatingip_statuses("}],"source_content_type":"text/x-python","patch_set":4,"id":"9aa7fdbe_e5013006","line":1190,"updated":"2014-10-23 19:43:36.000000000","message":"I know that we cannot update to the real status reported to the server atomically with the operation that actually affects the status change.  So, we\u0027ve got to do one or the other first.\n\nI guess I prefer sending the update after the change has taken effect.  Otherwise, it feels like we\u0027re assuming opportunistically that nothing will go wrong between the time we report status and the time we actually carry out what we said we did.  Also, that it will be reasonably quick so that no one notices the discrepancy.\n\nComparing this to real life, I typically don\u0027t tell someone that I\u0027ve done something before I\u0027ve actually done it.  There are those that do; maybe they will disagree with me here.","commit_id":"7e85defb55634f695d4323c074af9d7993bef33d"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"b2ef0e0c3cacea21da6cdd89c286555cca50172e","unresolved":false,"context_lines":[{"line_number":1187,"context_line":"                fip_statuses[fip[\u0027id\u0027]] \u003d self._add_floating_ip("},{"line_number":1188,"context_line":"                    ri, fip, interface_name, device)"},{"line_number":1189,"context_line":""},{"line_number":1190,"context_line":"        # Set status to DOWN before doing the removal."},{"line_number":1191,"context_line":"        down_fips \u003d dict((fip_id, l3_constants.FLOATINGIP_STATUS_DOWN)"},{"line_number":1192,"context_line":"            for fip_id in ri.floating_ips - set(fip_statuses.keys()))"},{"line_number":1193,"context_line":"        self.plugin_rpc.update_floatingip_statuses("}],"source_content_type":"text/x-python","patch_set":4,"id":"9aa7fdbe_e54cd0a0","line":1190,"in_reply_to":"9aa7fdbe_e5013006","updated":"2014-10-23 20:23:27.000000000","message":"You can look it both ways...to some degree the fact that a FIP is unreachable is a side effect of the fact that\u0027s being disassociated and that its status is no longer active. So to some degree I\u0027d argue that it\u0027s better to mark it down before we process it for removal.","commit_id":"7e85defb55634f695d4323c074af9d7993bef33d"}],"neutron/db/l3_db.py":[{"author":{"_account_id":8449,"name":"Marios Andreou","email":"marios.andreou@gmail.com","username":"marios"},"change_message_id":"63baf797095ac3b2ee8552c0b4385a856d06c216","unresolved":false,"context_lines":[{"line_number":758,"context_line":"        is_fip_disassociated \u003d not port_id and floatingip_db[\u0027fixed_port_id\u0027]"},{"line_number":759,"context_line":"        if is_fip_disassociated:"},{"line_number":760,"context_line":"            fip_update[\u0027status\u0027] \u003d l3_constants.FLOATINGIP_STATUS_DOWN"},{"line_number":761,"context_line":"        floatingip_db.update(fip_update)"},{"line_number":762,"context_line":""},{"line_number":763,"context_line":"    def create_floatingip(self, context, floatingip,"},{"line_number":764,"context_line":"            initial_status\u003dl3_constants.FLOATINGIP_STATUS_ACTIVE):"}],"source_content_type":"text/x-python","patch_set":2,"id":"9aa7fdbe_4d1eba64","line":761,"updated":"2014-10-23 06:28:27.000000000","message":"I SO wanted to do this in my review :) - just set the status yourself. \n\nBut now I\u0027m really confused, more on the frontpage for discussion visibility","commit_id":"eb2118157f0cfa3f4955fabd2f13af4a8dcb8ad4"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"06153013f40adf2e49f78ee011f5678df207e03e","unresolved":false,"context_lines":[{"line_number":758,"context_line":"        is_fip_disassociated \u003d not port_id and floatingip_db[\u0027fixed_port_id\u0027]"},{"line_number":759,"context_line":"        if is_fip_disassociated:"},{"line_number":760,"context_line":"            fip_update[\u0027status\u0027] \u003d l3_constants.FLOATINGIP_STATUS_DOWN"},{"line_number":761,"context_line":"        floatingip_db.update(fip_update)"},{"line_number":762,"context_line":""},{"line_number":763,"context_line":"    def create_floatingip(self, context, floatingip,"},{"line_number":764,"context_line":"            initial_status\u003dl3_constants.FLOATINGIP_STATUS_ACTIVE):"}],"source_content_type":"text/x-python","patch_set":2,"id":"9aa7fdbe_9eb99c17","line":761,"in_reply_to":"9aa7fdbe_4d1eba64","updated":"2014-10-23 14:44:11.000000000","message":"hopefully my feedback has cleared your confusion","commit_id":"eb2118157f0cfa3f4955fabd2f13af4a8dcb8ad4"}],"neutron/tests/unit/test_l3_agent.py":[{"author":{"_account_id":8449,"name":"Marios Andreou","email":"marios.andreou@gmail.com","username":"marios"},"change_message_id":"b66102ce082963972e41b9122b60548ce6d66baf","unresolved":false,"context_lines":[{"line_number":1584,"context_line":"            # Assess the call for putting the floating IP up was performed"},{"line_number":1585,"context_line":"            mock_update_fip_status.assert_called_once_with("},{"line_number":1586,"context_line":"                mock.ANY, ri.router_id,"},{"line_number":1587,"context_line":"                {fip_id: l3_constants.FLOATINGIP_STATUS_DOWN})"},{"line_number":1588,"context_line":""},{"line_number":1589,"context_line":"    def test_process_router_floatingip_exception(self):"},{"line_number":1590,"context_line":"        agent \u003d l3_agent.L3NATAgent(HOSTNAME, self.conf)"}],"source_content_type":"text/x-python","patch_set":2,"id":"9aa7fdbe_08101ba7","side":"PARENT","line":1587,"updated":"2014-10-23 10:53:01.000000000","message":"same, nit fix?","commit_id":"8a64427a5d169e16caa85b609034699823aae5c0"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"06153013f40adf2e49f78ee011f5678df207e03e","unresolved":false,"context_lines":[{"line_number":1584,"context_line":"            # Assess the call for putting the floating IP up was performed"},{"line_number":1585,"context_line":"            mock_update_fip_status.assert_called_once_with("},{"line_number":1586,"context_line":"                mock.ANY, ri.router_id,"},{"line_number":1587,"context_line":"                {fip_id: l3_constants.FLOATINGIP_STATUS_DOWN})"},{"line_number":1588,"context_line":""},{"line_number":1589,"context_line":"    def test_process_router_floatingip_exception(self):"},{"line_number":1590,"context_line":"        agent \u003d l3_agent.L3NATAgent(HOSTNAME, self.conf)"}],"source_content_type":"text/x-python","patch_set":2,"id":"9aa7fdbe_9e50bcd5","side":"PARENT","line":1587,"in_reply_to":"9aa7fdbe_08101ba7","updated":"2014-10-23 14:44:11.000000000","message":"again, this is a result of the fact that we no longer set the status down from the agent.","commit_id":"8a64427a5d169e16caa85b609034699823aae5c0"}]}