)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"8ed71e8b87342f6f170678bc2b826be6bc1afd9e","unresolved":true,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"If a logical switch port does not contain the network name key"},{"line_number":10,"context_line":"\"neutron:network_name\" or this value is empty, the port deletion"},{"line_number":11,"context_line":"method should continue the register deletion."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Closes-Bug: #2000252"},{"line_number":14,"context_line":"Change-Id: I360579a1b570098a9882f0e5e5e9eb01eba208e9"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"fd110a71_2a4b5cf9","line":11,"updated":"2023-01-03 14:14:02.000000000","message":"do we know how it can happen like that? Shouldn\u0027t this key be set always?","commit_id":"282ddc41dc568c9594329b24c5ddae6b2c66adcc"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"80cf4468fb8b5ebbac4dbd5ad414dfb4f79615db","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"If a logical switch port does not contain the network name key"},{"line_number":10,"context_line":"\"neutron:network_name\" or this value is empty, the port deletion"},{"line_number":11,"context_line":"method should continue the register deletion."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Closes-Bug: #2000252"},{"line_number":14,"context_line":"Change-Id: I360579a1b570098a9882f0e5e5e9eb01eba208e9"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"c3b515bf_8aa3db43","line":11,"in_reply_to":"fd110a71_2a4b5cf9","updated":"2023-01-03 16:08:44.000000000","message":"Yes and despite this, I have this situation in my env right now. I don\u0027t know exactly how I ended having this LSP without external_ids information. But this is the only place in ovn_client where we don\u0027t check what is retrieved from this dict","commit_id":"282ddc41dc568c9594329b24c5ddae6b2c66adcc"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"a304a0cc21637c4fd840abdfa1969f889c63d277","unresolved":true,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"[OVN] Consider a malformed LSP during the port deletion"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"If a logical switch port does not contain the network name key"},{"line_number":10,"context_line":"\"neutron:network_name\" or this value is empty, the port deletion"},{"line_number":11,"context_line":"method should continue the register deletion."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Closes-Bug: #2000252"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"6c9400ff_5fc16934","line":10,"range":{"start_line":9,"start_character":0,"end_line":10,"end_character":45},"updated":"2023-01-19 17:27:52.000000000","message":"Do you happen to know how this could be ? We always add this information to the LSP external_ids [0] for every port we create. Are these ports created by some other system that is not ML2/OVN ?\n\n[0] https://github.com/openstack/neutron/blob/b71b25820be6d61ed9f249eddf32bfa49ac76524/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py#L546-L547","commit_id":"5f44cebf391b7fe70e811673667d99abf104af31"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"b6714c41b640676123604fb08470c16d54c9b430","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"[OVN] Consider a malformed LSP during the port deletion"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"If a logical switch port does not contain the network name key"},{"line_number":10,"context_line":"\"neutron:network_name\" or this value is empty, the port deletion"},{"line_number":11,"context_line":"method should continue the register deletion."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Closes-Bug: #2000252"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"93d64c0c_73f1df0e","line":10,"range":{"start_line":9,"start_character":0,"end_line":10,"end_character":45},"in_reply_to":"324cfb46_5c61ceb0","updated":"2023-01-20 13:31:07.000000000","message":"Let\u0027s see what other reviewers think.\n\nI would rather let things fail if the database is manually modified because it would be impossible to address every case if that happens to all resources. I don\u0027t see why we would make this one a exception to the rule.","commit_id":"5f44cebf391b7fe70e811673667d99abf104af31"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"a3351bdb8764e61936aa84109c50974da9d4f19c","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"[OVN] Consider a malformed LSP during the port deletion"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"If a logical switch port does not contain the network name key"},{"line_number":10,"context_line":"\"neutron:network_name\" or this value is empty, the port deletion"},{"line_number":11,"context_line":"method should continue the register deletion."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Closes-Bug: #2000252"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"324cfb46_5c61ceb0","line":10,"range":{"start_line":9,"start_character":0,"end_line":10,"end_character":45},"in_reply_to":"5cba52a7_c6ceea64","updated":"2023-01-20 13:22:12.000000000","message":"I see but, we do have an assumption in ML2/OVN that the driver \"owns\" these databases, in that case this error would never happen.\n\nMy fear is setting a precedent here because others resources would also start failing if we remove external_ids flags from them, why would we make Logical Switch Port an exception ?","commit_id":"5f44cebf391b7fe70e811673667d99abf104af31"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"2f2cb978930e730494c70d9c1ade1d8f81a7c548","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"[OVN] Consider a malformed LSP during the port deletion"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"If a logical switch port does not contain the network name key"},{"line_number":10,"context_line":"\"neutron:network_name\" or this value is empty, the port deletion"},{"line_number":11,"context_line":"method should continue the register deletion."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Closes-Bug: #2000252"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"5cba52a7_c6ceea64","line":10,"range":{"start_line":9,"start_character":0,"end_line":10,"end_character":45},"in_reply_to":"6c9400ff_5fc16934","updated":"2023-01-19 17:51:50.000000000","message":"Yes, that happened to me when manually modifying the NB database. I\u0027m not saying this is something expected or recommended, for sure, but the \"check_for_inconsistencies\" method should not fail due to this error. Actually it should be resilient to this possibility.","commit_id":"5f44cebf391b7fe70e811673667d99abf104af31"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"afb6aebe8e6ed7b1713d0ca917e83c39862de179","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"2f5340b4_df8a0813","updated":"2023-02-01 09:04:31.000000000","message":"Hello folks, can you review this patch? Thanks in advance.","commit_id":"5f44cebf391b7fe70e811673667d99abf104af31"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"a304a0cc21637c4fd840abdfa1969f889c63d277","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"6e99a1c3_be1a365e","updated":"2023-01-19 17:27:52.000000000","message":"Questions inline","commit_id":"5f44cebf391b7fe70e811673667d99abf104af31"}],"neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py":[{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"8ed71e8b87342f6f170678bc2b826be6bc1afd9e","unresolved":true,"context_lines":[{"line_number":802,"context_line":"                txn.add(self._nb_idl.lsp_del(port_id, if_exists\u003dTrue))"},{"line_number":803,"context_line":""},{"line_number":804,"context_line":"            if ovn_network_name:"},{"line_number":805,"context_line":"                network_id \u003d ovn_network_name.replace(\u0027neutron-\u0027, \u0027\u0027)"},{"line_number":806,"context_line":"                p_object \u003d ({\u0027id\u0027: port_id, \u0027network_id\u0027: network_id}"},{"line_number":807,"context_line":"                            if not port_object else port_object)"},{"line_number":808,"context_line":"                self._qos_driver.delete_port(txn, p_object)"},{"line_number":809,"context_line":""},{"line_number":810,"context_line":"            if port_object and self.is_dns_required_for_port(port_object):"},{"line_number":811,"context_line":"                self.add_txns_to_remove_port_dns_records(txn, port_object)"}],"source_content_type":"text/x-python","patch_set":1,"id":"dfb9e5b0_5e549d61","line":808,"range":{"start_line":805,"start_character":16,"end_line":808,"end_character":59},"updated":"2023-01-03 14:14:02.000000000","message":"nit: this can be moved to L800 (before else clause) and You will have just one \"if ovn_network_name\" condition.","commit_id":"282ddc41dc568c9594329b24c5ddae6b2c66adcc"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"80cf4468fb8b5ebbac4dbd5ad414dfb4f79615db","unresolved":false,"context_lines":[{"line_number":802,"context_line":"                txn.add(self._nb_idl.lsp_del(port_id, if_exists\u003dTrue))"},{"line_number":803,"context_line":""},{"line_number":804,"context_line":"            if ovn_network_name:"},{"line_number":805,"context_line":"                network_id \u003d ovn_network_name.replace(\u0027neutron-\u0027, \u0027\u0027)"},{"line_number":806,"context_line":"                p_object \u003d ({\u0027id\u0027: port_id, \u0027network_id\u0027: network_id}"},{"line_number":807,"context_line":"                            if not port_object else port_object)"},{"line_number":808,"context_line":"                self._qos_driver.delete_port(txn, p_object)"},{"line_number":809,"context_line":""},{"line_number":810,"context_line":"            if port_object and self.is_dns_required_for_port(port_object):"},{"line_number":811,"context_line":"                self.add_txns_to_remove_port_dns_records(txn, port_object)"}],"source_content_type":"text/x-python","patch_set":1,"id":"c7624161_4065ae76","line":808,"range":{"start_line":805,"start_character":16,"end_line":808,"end_character":59},"in_reply_to":"dfb9e5b0_5e549d61","updated":"2023-01-03 16:08:44.000000000","message":"right","commit_id":"282ddc41dc568c9594329b24c5ddae6b2c66adcc"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"8ed71e8b87342f6f170678bc2b826be6bc1afd9e","unresolved":true,"context_lines":[{"line_number":812,"context_line":""},{"line_number":813,"context_line":"            # Check if the port being deleted is a virtual parent"},{"line_number":814,"context_line":"            if (ovn_port.type !\u003d ovn_const.LSP_TYPE_VIRTUAL and"},{"line_number":815,"context_line":"                    ovn_network_name):"},{"line_number":816,"context_line":"                ls \u003d self._nb_idl.ls_get(ovn_network_name).execute("},{"line_number":817,"context_line":"                    check_error\u003dTrue)"},{"line_number":818,"context_line":"                if ls:"}],"source_content_type":"text/x-python","patch_set":1,"id":"52a4c1d5_fa2318c4","line":815,"updated":"2023-01-03 14:14:02.000000000","message":"why this new condition is added here too? Should You then update comment in L813?","commit_id":"282ddc41dc568c9594329b24c5ddae6b2c66adcc"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"80cf4468fb8b5ebbac4dbd5ad414dfb4f79615db","unresolved":false,"context_lines":[{"line_number":812,"context_line":""},{"line_number":813,"context_line":"            # Check if the port being deleted is a virtual parent"},{"line_number":814,"context_line":"            if (ovn_port.type !\u003d ovn_const.LSP_TYPE_VIRTUAL and"},{"line_number":815,"context_line":"                    ovn_network_name):"},{"line_number":816,"context_line":"                ls \u003d self._nb_idl.ls_get(ovn_network_name).execute("},{"line_number":817,"context_line":"                    check_error\u003dTrue)"},{"line_number":818,"context_line":"                if ls:"}],"source_content_type":"text/x-python","patch_set":1,"id":"cef009df_f6766258","line":815,"in_reply_to":"52a4c1d5_fa2318c4","updated":"2023-01-03 16:08:44.000000000","message":"well, this is mandatory to avoid \"ls_get\" returning an exception if the network name is empty or None. In any case, I\u0027ll update it.","commit_id":"282ddc41dc568c9594329b24c5ddae6b2c66adcc"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"a304a0cc21637c4fd840abdfa1969f889c63d277","unresolved":true,"context_lines":[{"line_number":803,"context_line":"                            if not port_object else port_object)"},{"line_number":804,"context_line":"                self._qos_driver.delete_port(txn, p_object)"},{"line_number":805,"context_line":"            else:"},{"line_number":806,"context_line":"                txn.add(self._nb_idl.lsp_del(port_id, if_exists\u003dTrue))"},{"line_number":807,"context_line":""},{"line_number":808,"context_line":"            if port_object and self.is_dns_required_for_port(port_object):"},{"line_number":809,"context_line":"                self.add_txns_to_remove_port_dns_records(txn, port_object)"}],"source_content_type":"text/x-python","patch_set":2,"id":"41281de3_74c806ec","line":806,"updated":"2023-01-19 17:27:52.000000000","message":"So in the case the network ID is not in in the LSP external_ids we will not be calling the \"self._qos_driver.delete_port()\" method. What\u0027s the implication here ? \n\nIs it safe to assume that a port without that external_id field is not present in QoS ? Are we at risk of leaving some left over in the DB ?","commit_id":"5f44cebf391b7fe70e811673667d99abf104af31"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"2f2cb978930e730494c70d9c1ade1d8f81a7c548","unresolved":false,"context_lines":[{"line_number":803,"context_line":"                            if not port_object else port_object)"},{"line_number":804,"context_line":"                self._qos_driver.delete_port(txn, p_object)"},{"line_number":805,"context_line":"            else:"},{"line_number":806,"context_line":"                txn.add(self._nb_idl.lsp_del(port_id, if_exists\u003dTrue))"},{"line_number":807,"context_line":""},{"line_number":808,"context_line":"            if port_object and self.is_dns_required_for_port(port_object):"},{"line_number":809,"context_line":"                self.add_txns_to_remove_port_dns_records(txn, port_object)"}],"source_content_type":"text/x-python","patch_set":2,"id":"a1150be9_af169329","line":806,"in_reply_to":"41281de3_74c806ec","updated":"2023-01-19 17:51:50.000000000","message":"As you commented, we always set the network name in the LSP. If we have manually modified the LSP, then this is our responsibility to delete the leftovers.\n\nIn the case I reported [1], the LSP was manually added in the database. The \"check_for_inconsistencies\" called the OVNClient.delete_port method to remove this orphan register but failed here. With this patch we are ensuring that the LSP is deleted and the \"check_for_inconsistencies\" doesn\u0027t fail and stop the DB cleanup. If there is a related QoS register, we can\u0027t remove it without the network info.\n\n[1]https://bugs.launchpad.net/neutron/+bug/2000252","commit_id":"5f44cebf391b7fe70e811673667d99abf104af31"}]}