Em seg., 27 de mai. de 2024 às 12:41, Ilya Maximets <i.maxim...@ovn.org> escreveu:
> On 5/27/24 20:31, Roberto Bartzen Acosta wrote: > > Thanks Ilya! > > > > Em seg., 27 de mai. de 2024 às 10:28, Ilya Maximets <i.maxim...@ovn.org > <mailto:i.maxim...@ovn.org>> escreveu: > > > > On 5/27/24 18:22, Roberto Bartzen Acosta via discuss wrote: > > > Hello everyone! > > > > > > I found an issue during testing with the OVN-IC daemon. I noticed > that applications that interact with python-idl show an error when deleting > datapath_binding when the table index changes. > > > > > > We can see the error in the logs below: > > > > > > 2024-05-27 09:49:38.779 23596 ERROR > ovsdbapp.backend.ovs_idl.connection [-] > Datapath_Binding(uuid=UUID('498e66a2-70bc-4587-a66f-0433baf82f60'), > tunnel_key=16711683, load_balancers=[], external_ids={}) not in list: > ValueError: > Datapath_Binding(uuid=UUID('498e66a2-70bc-4587-a66f-0433baf82f60'), > tunnel_key=16711683, load_balancers=[], external_ids={}) not in list > > > 2024-05-27 09:49:38.779 23596 ERROR > ovsdbapp.backend.ovs_idl.connection Traceback (most recent call last): > > > 2024-05-27 09:49:38.779 23596 ERROR > ovsdbapp.backend.ovs_idl.connection File > "/usr/lib/python3/dist-packages/ovsdbapp/backend/ovs_idl/connection.py", > line 110, in run > > > 2024-05-27 09:49:38.779 23596 ERROR > ovsdbapp.backend.ovs_idl.connection self.idl.run() > > > 2024-05-27 09:49:38.779 23596 ERROR > ovsdbapp.backend.ovs_idl.connection File > "/usr/lib/python3/dist-packages/ovs/db/idl.py", line 465, in run > > > 2024-05-27 09:49:38.779 23596 ERROR > ovsdbapp.backend.ovs_idl.connection self.__parse_update(msg.params[2], > OVSDB_UPDATE3) > > > 2024-05-27 09:49:38.779 23596 ERROR > ovsdbapp.backend.ovs_idl.connection File > "/usr/lib/python3/dist-packages/ovs/db/idl.py", line 924, in __parse_update > > > 2024-05-27 09:49:38.779 23596 ERROR > ovsdbapp.backend.ovs_idl.connection self.__do_parse_update(update, > version, self.tables) > > > 2024-05-27 09:49:38.779 23596 ERROR > ovsdbapp.backend.ovs_idl.connection File > "/usr/lib/python3/dist-packages/ovs/db/idl.py", line 964, in > __do_parse_update > > > 2024-05-27 09:49:38.779 23596 ERROR > ovsdbapp.backend.ovs_idl.connection changes = > self.__process_update2(table, uuid, row_update) > > > 2024-05-27 09:49:38.779 23596 ERROR > ovsdbapp.backend.ovs_idl.connection File > "/usr/lib/python3/dist-packages/ovs/db/idl.py", line 991, in > __process_update2 > > > 2024-05-27 09:49:38.779 23596 ERROR > ovsdbapp.backend.ovs_idl.connection del table.rows[uuid] > > > 2024-05-27 09:49:38.779 23596 ERROR > ovsdbapp.backend.ovs_idl.connection File > "/usr/lib/python3/dist-packages/ovs/db/custom_index.py", line 102, in > __delitem__ > > > 2024-05-27 09:49:38.779 23596 ERROR > ovsdbapp.backend.ovs_idl.connection index.remove(val) > > > 2024-05-27 09:49:38.779 23596 ERROR > ovsdbapp.backend.ovs_idl.connection File > "/usr/lib/python3/dist-packages/ovs/db/custom_index.py", line 66, in remove > > > 2024-05-27 09:49:38.779 23596 ERROR > ovsdbapp.backend.ovs_idl.connection > self.values.remove(self.index_entry_from_row(row)) > > > 2024-05-27 09:49:38.779 23596 ERROR > ovsdbapp.backend.ovs_idl.connection File > "/usr/lib/python3/dist-packages/sortedcontainers/sortedlist.py", line 2015, > in remove > > > 2024-05-27 09:49:38.779 23596 ERROR > ovsdbapp.backend.ovs_idl.connection raise ValueError('{0!r} not in > list'.format(value)) > > > 2024-05-27 09:49:38.779 23596 ERROR > ovsdbapp.backend.ovs_idl.connection ValueError: > Datapath_Binding(uuid=UUID('498e66a2-70bc-4587-a66f-0433baf82f60'), > tunnel_key=16711683, load_balancers=[], external_ids={}) not in list > > > 2024-05-27 09:49:38.779 23596 ERROR > ovsdbapp.backend.ovs_idl.connection > > > > > > > > > The complete logs showing the creation, update of the tunnel_key > and deletion of the datapath_binding can be accessed at [1]. > > > > > > Basically the problem occurs because of the tunnel_key update when > we create a transit-switch. If the table did not have this index the > problem would not be observed. > > > > > > This issue affects applications such as: neutron-server, > neutron-ovn-metadata-agent, and everyone that monitors the Datapath_Binding > table via python-idl. > > > > > > I've already talked to Ilya Maximets in the openvswitch IRC > channel and the issue seems to be in the row updating process: > python/ovs/db/idl.py. > > > > > > Thanks, Roberto! I beleive the fix I shared on IRC is mostly > correct though > > I didn't test it. I need to refine it a little and add a unit test > for the > > issue. With that I hope to post it for review somewhere soon. > > > > > > > > Just to update on the patch you proposed, the deletion of the entry > fails during the python-idl update execution. The content of the table row > has already been updated just above in the __apply_diff and/or __row_update > methods. If you request to delete the entry... you will delete an entry in > use from what I understand. > > > > The code below triggers an error when updating tunnel_key (error that > didn't happen before when updating rows): > > Yeah, I figured that. :) > > > > > + del table.rows[uuid] > > + table.rows[uuid] = row > > > > > > So I suggest just updating the table reference with the already updated > row. What do you think? > > The setattr method doesn't handle updates, so deletion is necessary. > The fenied approach is temporary here: > > https://github.com/igsilya/ovs/commit/4792aee0dcb97d9aefd182cc3c97865aa501be71 This way works for python-idl clients connected via ovsdbapp. Thanks :) > > > I still need to create a test, so it might change if I find something else. > > > > > $ git diff > > diff --git a/python/ovs/db/idl.py b/python/ovs/db/idl.py > > index a80da84e7..08c5acffb 100644 > > --- a/python/ovs/db/idl.py > > +++ b/python/ovs/db/idl.py > > @@ -1014,6 +1014,7 @@ class Idl(object): > > raise error.Error('Modify non-existing row') > > > > old_row = self.__apply_diff(table, row, > row_update['modify']) > > + table.rows[uuid] = row > > return Notice(ROW_UPDATE, row, Row(self, table, uuid, > old_row)) > > else: > > raise error.Error('<row-update> unknown operation', > > @@ -1062,6 +1063,7 @@ class Idl(object): > > if op == ROW_CREATE: > > table.rows[uuid] = row > > if changed: > > + table.rows[uuid] = row > > return Notice(op, row, Row.from_json(self, table, uuid, > old)) > > return False > > > > > > > > CC: Terry. > > > > Best regards, Ilya Maximets. > > > > > > > > Kind regards, > > > Roberto > > > > > > > > > [1] https://paste.openstack.org/show/bqLA7nxfk2C6loD5YlY1/ < > https://paste.openstack.org/show/bqLA7nxfk2C6loD5YlY1/> > > > > > > > > /‘Esta mensagem é direcionada apenas para os endereços constantes no > cabeçalho inicial. Se você não está listado nos endereços constantes no > cabeçalho, pedimos-lhe que desconsidere completamente o conteúdo dessa > mensagem e cuja cópia, encaminhamento e/ou execução das ações citadas estão > imediatamente anuladas e proibidas’./ > > > > / //‘Apesar do Magazine Luiza tomar todas as precauções razoáveis para > assegurar que nenhum vírus esteja presente nesse e-mail, a empresa não > poderá aceitar a responsabilidade por quaisquer perdas ou danos causados > por esse e-mail ou por seus anexos’./ > > > > -- _‘Esta mensagem é direcionada apenas para os endereços constantes no cabeçalho inicial. Se você não está listado nos endereços constantes no cabeçalho, pedimos-lhe que desconsidere completamente o conteúdo dessa mensagem e cuja cópia, encaminhamento e/ou execução das ações citadas estão imediatamente anuladas e proibidas’._ * **‘Apesar do Magazine Luiza tomar todas as precauções razoáveis para assegurar que nenhum vírus esteja presente nesse e-mail, a empresa não poderá aceitar a responsabilidade por quaisquer perdas ou danos causados por esse e-mail ou por seus anexos’.*
_______________________________________________ discuss mailing list disc...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-discuss