)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"427f24bbca6c5a072904daaf9ea927ac576c182b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"bef51f5d_a40ad592","updated":"2025-05-19 13:17:38.000000000","message":"I\u0027m happy to do this here, but it feels like a bug in the neutron API. Have you discussed this with the neutron team first? I\u0027d like them to take a look at this before we approve it.","commit_id":"6b641c39342df44ba6818aba7adb610dc7d0cbde"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"a007a563b184d018b72a80c14e05c191246b1098","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"8be65bd7_1e089752","updated":"2025-05-19 15:19:32.000000000","message":"There are several disconnected documents that explain this behaviour.\n\n* Neutron API [1]:\n```\nAgents that won’t be used anymore can be removed. Before deleting agents via API, the agent should be stopped/disabled.\n```\nThe OVN agent must be first deleted (ovn-controller stopped, OVN metadata agent deleted) before deleting in from the API.\n\n* A partial description of an OVN agent deletion: [2]\n\n* A related description of when the OVN agent deletion can be used: [3]\n\nIn any case, in order to avoid this kind of misunderstandings, I\u0027ll propose a doc patch for Neutron, in the OVN section.\n\n[1]https://docs.openstack.org/api-ref/network/v2/#delete-agent\n[2]https://github.com/openstack/neutron/blob/master/doc/source/admin/config-services-agent.rst#l2-agents\n[3]https://github.com/openstack/neutron/blob/master/doc/source/admin/ovn/troubleshooting.rst#duplicated-or-deleted-ovn-agents","commit_id":"6b641c39342df44ba6818aba7adb610dc7d0cbde"},{"author":{"_account_id":13252,"name":"Dr. Jens Harbott","display_name":"Jens Harbott (frickler)","email":"frickler@offenerstapel.de","username":"jrosenboom"},"change_message_id":"abe5ea0f4b361a2ace38ecd72e63fec01e85017a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"862fa228_149c374c","updated":"2025-05-19 15:04:23.000000000","message":"https://review.opendev.org/c/openstack/neutron/+/752795 looks to me like this should work and it is a bug in neutron if it doesn\u0027t.","commit_id":"6b641c39342df44ba6818aba7adb610dc7d0cbde"},{"author":{"_account_id":37971,"name":"Chaemin Lim","display_name":"antraxmin","email":"antraxmin@naver.com","username":"antraxmin"},"change_message_id":"d1eaee17806a5eb1b3c347a78640b2648b2db83b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"ba4d8015_3fca3a68","in_reply_to":"8be65bd7_1e089752","updated":"2025-05-20 09:59:49.000000000","message":"Thank you for the references and clarification. I\u0027ve updated the warning message \nto better reflect the documented behavior that OVN agent services must be stopped \nbefore deleting via API. The updated message includes a reference to the Neutron \nAPI documentation to guide users on the proper procedure.\n\nThe focus of this patch is to provide clearer feedback to users when they attempt \nto delete an OVN agent, rather than silently appearing to succeed while actually failing.","commit_id":"6b641c39342df44ba6818aba7adb610dc7d0cbde"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"69fe93af98e218b62250782f51dc00f58577f4a5","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"d4518766_638a6c55","in_reply_to":"ba4d8015_3fca3a68","updated":"2025-05-20 13:02:26.000000000","message":"That\u0027s not accurate. The agent is indeed deleted from the local Neutron API cache (the OVN agents are not stored in the DB). If you delete an OVN agent, it will be removed intermediately from the local cache. But it will restored once the Neutron API receives an update event from the agent.\n\nFor example, the OVN Metadata agent sends an update that is received in ``ChassisMetadataAgentWriteEvent``. If the agent is not present, the event will restore it in the Agent Cache.\n\nThis is why this implementation is not valid:\n* First because we indeed delete the agents. If you immediately check the presence of the deleted agent, it won\u0027t be there.\n* Second, as mentioned before, the delete command is for clean-up procedures.","commit_id":"6b641c39342df44ba6818aba7adb610dc7d0cbde"},{"author":{"_account_id":8313,"name":"Lajos Katona","display_name":"lajoskatona","email":"katonalala@gmail.com","username":"elajkat","status":"Ericsson Software Technology"},"change_message_id":"a46d3256ebc75dd0c735d4097e66b1860fafe279","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"92016237_68644ca8","updated":"2025-05-20 09:08:33.000000000","message":"I agree with Rodolfo, if this can be handled it must be handled in Neutron where the API code is, not in the CLI.","commit_id":"7b1b27a1ef239b2df16df10052b4638197df541a"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"3868628f9bda64b0814c83baed8416921e6e4375","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"93d919dd_6bad02cd","updated":"2025-05-20 06:40:19.000000000","message":"Keeping my -1, as commented in previous PS. This patch should not land in the repo.","commit_id":"7b1b27a1ef239b2df16df10052b4638197df541a"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"58c1f34c0e02b1e427b332331dcd84c75ca84c12","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"4917d509_06f5cdc1","updated":"2025-05-20 09:17:17.000000000","message":"Per feedback from neutron cores (thanks, folks)","commit_id":"7b1b27a1ef239b2df16df10052b4638197df541a"},{"author":{"_account_id":37971,"name":"Chaemin Lim","display_name":"antraxmin","email":"antraxmin@naver.com","username":"antraxmin"},"change_message_id":"d1eaee17806a5eb1b3c347a78640b2648b2db83b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"8d40ebfb_d37c56be","updated":"2025-05-20 09:59:49.000000000","message":"Thank you for your clear feedback. Until now, I didn\u0027t realize this should be handled by the Neutron repository, not the CLI. I\u0027ll abandon this patch and look at how to contribute to Neutron if appropriate.","commit_id":"7b1b27a1ef239b2df16df10052b4638197df541a"}],"openstackclient/network/v2/network_agent.py":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"427f24bbca6c5a072904daaf9ea927ac576c182b","unresolved":true,"context_lines":[{"line_number":142,"context_line":"        for agent_id in parsed_args.network_agent:"},{"line_number":143,"context_line":"            try:"},{"line_number":144,"context_line":"                agent \u003d client.get_agent(agent_id)"},{"line_number":145,"context_line":"                agent_type \u003d getattr(agent, \u0027agent_type\u0027, \u0027\u0027)"},{"line_number":146,"context_line":"                if \u0027ovn\u0027 in agent_type.lower():"},{"line_number":147,"context_line":"                    LOG.warning("},{"line_number":148,"context_line":"                        _("}],"source_content_type":"text/x-python","patch_set":1,"id":"b83f9a39_cd71dce4","line":145,"updated":"2025-05-19 13:17:38.000000000","message":"Do we need to use `getattr`? If so, can you add a comment on why.","commit_id":"6b641c39342df44ba6818aba7adb610dc7d0cbde"},{"author":{"_account_id":37971,"name":"Chaemin Lim","display_name":"antraxmin","email":"antraxmin@naver.com","username":"antraxmin"},"change_message_id":"d1eaee17806a5eb1b3c347a78640b2648b2db83b","unresolved":true,"context_lines":[{"line_number":142,"context_line":"        for agent_id in parsed_args.network_agent:"},{"line_number":143,"context_line":"            try:"},{"line_number":144,"context_line":"                agent \u003d client.get_agent(agent_id)"},{"line_number":145,"context_line":"                agent_type \u003d getattr(agent, \u0027agent_type\u0027, \u0027\u0027)"},{"line_number":146,"context_line":"                if \u0027ovn\u0027 in agent_type.lower():"},{"line_number":147,"context_line":"                    LOG.warning("},{"line_number":148,"context_line":"                        _("}],"source_content_type":"text/x-python","patch_set":1,"id":"99ae7e82_b02df9d7","line":145,"in_reply_to":"b83f9a39_cd71dce4","updated":"2025-05-20 09:59:49.000000000","message":"I used `getattr` here for safety when accessing the \u0027agent_type\u0027 attribute. This defensive programming approach handles cases where the agent object might not have this attribute (such as during partial data scenarios or API changes). By providing a default empty string, ensure the code continues to work even if the attribute is missing, rather than raising an AttributeError.","commit_id":"6b641c39342df44ba6818aba7adb610dc7d0cbde"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"427f24bbca6c5a072904daaf9ea927ac576c182b","unresolved":true,"context_lines":[{"line_number":159,"context_line":"                client.delete_agent(agent_id, ignore_missing\u003dFalse)"},{"line_number":160,"context_line":"                if agent and \u0027ovn\u0027 in agent_type.lower():"},{"line_number":161,"context_line":"                    try:"},{"line_number":162,"context_line":"                        client.get_agent(agent_id)"},{"line_number":163,"context_line":"                        LOG.error("},{"line_number":164,"context_line":"                            _("},{"line_number":165,"context_line":"                                \"Agent \u0027%s\u0027 still exists after deletion API call. \""}],"source_content_type":"text/x-python","patch_set":1,"id":"31d30efb_49ba50c9","line":162,"updated":"2025-05-19 13:17:38.000000000","message":"Is this not open to a TOCTOU race (or rather TOD(eletion)TOC) race here?","commit_id":"6b641c39342df44ba6818aba7adb610dc7d0cbde"},{"author":{"_account_id":37971,"name":"Chaemin Lim","display_name":"antraxmin","email":"antraxmin@naver.com","username":"antraxmin"},"change_message_id":"d1eaee17806a5eb1b3c347a78640b2648b2db83b","unresolved":true,"context_lines":[{"line_number":159,"context_line":"                client.delete_agent(agent_id, ignore_missing\u003dFalse)"},{"line_number":160,"context_line":"                if agent and \u0027ovn\u0027 in agent_type.lower():"},{"line_number":161,"context_line":"                    try:"},{"line_number":162,"context_line":"                        client.get_agent(agent_id)"},{"line_number":163,"context_line":"                        LOG.error("},{"line_number":164,"context_line":"                            _("},{"line_number":165,"context_line":"                                \"Agent \u0027%s\u0027 still exists after deletion API call. \""}],"source_content_type":"text/x-python","patch_set":1,"id":"7ac9630a_603d7f60","line":162,"in_reply_to":"31d30efb_49ba50c9","updated":"2025-05-20 09:59:49.000000000","message":"That\u0027s correct, thank you for pointing that out, in theory, TOCTOU competition status can occur, but in this case, from a practical point of view, the main goal was to solve the problem that OVN agent is not deleted normally through API. \n\nThe code is focused on immediately performing verification after deleting the agent to give the right feedback to the user. Of course, there is a possibility that another process will change the state between verification and use, but I thought the probability was low and the impact was limited. \n\nIf this is a problem, I will try to correct it using a different approach. If you give me any additional feedback, I will humbly accept it. Your help is appreciated!","commit_id":"6b641c39342df44ba6818aba7adb610dc7d0cbde"}]}