)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"3929606086f5298a4097b290704311ad87b69104","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Not remove the running router when MQ is unreachable"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"When the L3 agent get a router update notification, it will try to"},{"line_number":10,"context_line":"retrieve the router info from neutron server. But at this time, if"},{"line_number":11,"context_line":"the message queue is down/unreachable. It will get exceptions related"},{"line_number":12,"context_line":"message queue. The resync actions will be run then. Sometimes, rabbitMQ"},{"line_number":13,"context_line":"cluster is not so much easy to recover. Then Long time MQ recover time"},{"line_number":14,"context_line":"will cause the router info sync RPC never get successful until it meets"},{"line_number":15,"context_line":"the max retry time. Then the bad thing happens, L3 agent is trying to"},{"line_number":16,"context_line":"remove the router now. It basically shutdown all the existing L3 traffic"},{"line_number":17,"context_line":"of this router."},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"This patch directly removes the final router removal action, let the"},{"line_number":20,"context_line":"router run as it is."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"3f4c43b2_54915052","line":17,"range":{"start_line":9,"start_character":0,"end_line":17,"end_character":15},"updated":"2020-04-14 10:47:31.000000000","message":"Let\u0027s illustrate a case here:\n1. one router is processed once with 10 floating IPs.\n2. a new floating IPs is added to the router.\n3. Router update notification is just arrived.\n4. MQ is gone.\n5. Agent is trying to resync the info until the max times.\n6. [BUG] Agent is trying to remove the router.\n7. [BUG] Finally existing 10 floating IPs is unreachable.","commit_id":"641ae947847e0f55bd73679c3d7c3c8c49c47b24"}],"neutron/agent/l3/agent.py":[{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"331ff4dabda0e7ceaefd3524eb8d95fc4d7b820c","unresolved":false,"context_lines":[{"line_number":668,"context_line":"                        router_update.id, router_update.action)"},{"line_number":669,"context_line":"            if router_update.action !\u003d DELETE_ROUTER:"},{"line_number":670,"context_line":"                LOG.debug(\"Deleting router %s\", router_update.id)"},{"line_number":671,"context_line":"                self._safe_router_removed(router_update.id)"},{"line_number":672,"context_line":"            return"},{"line_number":673,"context_line":"        router_update.timestamp \u003d timeutils.utcnow()"},{"line_number":674,"context_line":"        router_update.priority \u003d priority"}],"source_content_type":"text/x-python","patch_set":1,"id":"3f4c43b2_66e66619","side":"PARENT","line":671,"updated":"2020-04-14 09:23:58.000000000","message":"I\u0027m partially OK with this change. But this code was added in case the router was being initialized. If the router is in the initializing process and there is not RPC reply, the router is deleted. IMO, we should still handle this case.\n\n\n[1]https://review.opendev.org/#/c/516322/","commit_id":"e74c8f8c887af0b101fbb879cb0eb87090d4696d"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"3929606086f5298a4097b290704311ad87b69104","unresolved":false,"context_lines":[{"line_number":668,"context_line":"                        router_update.id, router_update.action)"},{"line_number":669,"context_line":"            if router_update.action !\u003d DELETE_ROUTER:"},{"line_number":670,"context_line":"                LOG.debug(\"Deleting router %s\", router_update.id)"},{"line_number":671,"context_line":"                self._safe_router_removed(router_update.id)"},{"line_number":672,"context_line":"            return"},{"line_number":673,"context_line":"        router_update.timestamp \u003d timeutils.utcnow()"},{"line_number":674,"context_line":"        router_update.priority \u003d priority"}],"source_content_type":"text/x-python","patch_set":1,"id":"3f4c43b2_9418f81d","side":"PARENT","line":671,"in_reply_to":"3f4c43b2_66e66619","updated":"2020-04-14 10:47:31.000000000","message":"Sorry, I\u0027m a bit confused about the cases you mentioned. IMO, you mentioned two cases:\n1. If router is not in cache, aka it is trying to be added to agent for the first time. This patch should be fine, because router info was not retrieved successfully.\n2. If router is in cache and processed once before, it should be run as it is. Agent must not direct remove it here. Yes, this is the bug, and this patch is mainly to fix this issue.\n\nSo make sense?","commit_id":"e74c8f8c887af0b101fbb879cb0eb87090d4696d"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"df8172d66c04a882edb81349f2788cc0d9832e96","unresolved":false,"context_lines":[{"line_number":668,"context_line":"                        router_update.id, router_update.action)"},{"line_number":669,"context_line":"            if router_update.action !\u003d DELETE_ROUTER:"},{"line_number":670,"context_line":"                LOG.debug(\"Deleting router %s\", router_update.id)"},{"line_number":671,"context_line":"                self._safe_router_removed(router_update.id)"},{"line_number":672,"context_line":"            return"},{"line_number":673,"context_line":"        router_update.timestamp \u003d timeutils.utcnow()"},{"line_number":674,"context_line":"        router_update.priority \u003d priority"}],"source_content_type":"text/x-python","patch_set":1,"id":"3f4c43b2_eca9acdf","side":"PARENT","line":671,"in_reply_to":"3f4c43b2_9418f81d","updated":"2020-04-14 16:21:31.000000000","message":"I see that you want to address the case when the MQ is down and [1] is called.\n\nOnly when the router is created the cache is updated. As you said, we are safe on this.\n\nI don\u0027t want to block this patch, by no means. I\u0027m just trying to find any path were a router should be deleted by this line here.\n\n[1]https://github.com/openstack/neutron/blob/master/neutron/agent/l3/agent.py#L710","commit_id":"e74c8f8c887af0b101fbb879cb0eb87090d4696d"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"c623fbd51b79ec01c28d67fb74dcb17b5e689321","unresolved":false,"context_lines":[{"line_number":668,"context_line":"                        router_update.id, router_update.action)"},{"line_number":669,"context_line":"            if router_update.action !\u003d DELETE_ROUTER:"},{"line_number":670,"context_line":"                LOG.debug(\"Deleting router %s\", router_update.id)"},{"line_number":671,"context_line":"                self._safe_router_removed(router_update.id)"},{"line_number":672,"context_line":"            return"},{"line_number":673,"context_line":"        router_update.timestamp \u003d timeutils.utcnow()"},{"line_number":674,"context_line":"        router_update.priority \u003d priority"}],"source_content_type":"text/x-python","patch_set":1,"id":"3f4c43b2_ccbbdb0b","side":"PARENT","line":671,"in_reply_to":"3f4c43b2_ce7e4f6a","updated":"2020-04-17 02:56:10.000000000","message":"\u003e So it took me a minute to figure out why I added this, but it was\n \u003e basically to cover the case where the message had been in the queue\n \u003e so long that we had decided to give-up on the action, so felt the\n \u003e safest thing was just to remove the router since it probably isn\u0027t\n \u003e initialized correctly.  For example, we don\u0027t know what operation\n \u003e is failing, so assume it\u0027s broken and remove it.\n \u003e \n \u003e Maybe that was an aggressive decision, but seemed correct to not\n \u003e have a router namespace where we didn\u0027t know it it\u0027s state so\n \u003e couldn\u0027t assume it was functional.\n \u003e \n\nAnd you have no way to verify if it is running well or it is initialized correctly. So directly assuming it is not initialized, IMHO, looks not a good way to deal with such situation. At least, left it running as it is, should be safer than directly remove it.\n\n \u003e Is there a case where we still want to remove this router?  For\n \u003e example, on an Add but not on an Update?  I know those queue\n \u003e actions are the same, so would need more thought than just removing\n \u003e this code.\n\nFrom the code search [1], I may say we should not remove the router on every branch.\n\nDeleting a router should be considered very carefully, this code removal has no harm to running status of agent routers. On the contrary, remove a router has bad effect to existing resources.\n\n[1] http://codesearch.openstack.org/?q\u003d_resync_router\u0026i\u003dnope\u0026files\u003d\u0026repos\u003dopenstack/neutron","commit_id":"e74c8f8c887af0b101fbb879cb0eb87090d4696d"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"39323f94129d9ca407da8b863677c74ea2f1ab91","unresolved":false,"context_lines":[{"line_number":668,"context_line":"                        router_update.id, router_update.action)"},{"line_number":669,"context_line":"            if router_update.action !\u003d DELETE_ROUTER:"},{"line_number":670,"context_line":"                LOG.debug(\"Deleting router %s\", router_update.id)"},{"line_number":671,"context_line":"                self._safe_router_removed(router_update.id)"},{"line_number":672,"context_line":"            return"},{"line_number":673,"context_line":"        router_update.timestamp \u003d timeutils.utcnow()"},{"line_number":674,"context_line":"        router_update.priority \u003d priority"}],"source_content_type":"text/x-python","patch_set":1,"id":"3f4c43b2_ce7e4f6a","side":"PARENT","line":671,"in_reply_to":"3f4c43b2_eca9acdf","updated":"2020-04-15 16:37:20.000000000","message":"So it took me a minute to figure out why I added this, but it was basically to cover the case where the message had been in the queue so long that we had decided to give-up on the action, so felt the safest thing was just to remove the router since it probably isn\u0027t initialized correctly.  For example, we don\u0027t know what operation is failing, so assume it\u0027s broken and remove it.\n\nMaybe that was an aggressive decision, but seemed correct to not have a router namespace where we didn\u0027t know it it\u0027s state so couldn\u0027t assume it was functional.\n\nIs there a case where we still want to remove this router?  For example, on an Add but not on an Update?  I know those queue actions are the same, so would need more thought than just removing this code.","commit_id":"e74c8f8c887af0b101fbb879cb0eb87090d4696d"}],"neutron/tests/unit/agent/l3/test_agent.py":[{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"df83969b417b3a548a64e4f67b547e31505d9d98","unresolved":false,"context_lines":[{"line_number":2755,"context_line":"        agent._queue \u003d mock.Mock()"},{"line_number":2756,"context_line":"        update \u003d mock.Mock()"},{"line_number":2757,"context_line":"        update.resource \u003d None"},{"line_number":2758,"context_line":"        update.action \u003d 3  # ADD_UPDATE_ROUTER"},{"line_number":2759,"context_line":"        router_info \u003d mock.MagicMock()"},{"line_number":2760,"context_line":"        agent.router_info[update.id] \u003d router_info"},{"line_number":2761,"context_line":"        router_processor \u003d mock.Mock()"}],"source_content_type":"text/x-python","patch_set":2,"id":"1f493fa4_8ce0a598","line":2758,"updated":"2020-04-24 21:45:03.000000000","message":"We should just use the constants here and below as we already include the agent code that defines it.","commit_id":"71edac5ad5db170fe4ca7488f7882b129c33868b"}]}