)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":25468,"name":"Michel Nederlof","email":"michel@nederlof.info","username":"pellucid"},"change_message_id":"7783cbe147c44070a7a0b725f0723dddb380e2eb","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"85004ee4_d51799ad","updated":"2023-10-04 14:03:57.000000000","message":"lgtm","commit_id":"bcc6553329e6ab376cde4a8707871da7df726099"},{"author":{"_account_id":25468,"name":"Michel Nederlof","email":"michel@nederlof.info","username":"pellucid"},"change_message_id":"e62f0c3300ce508451d761aa7654630076267a93","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"60e0c948_49120744","updated":"2023-10-04 18:00:17.000000000","message":"recheck","commit_id":"bcc6553329e6ab376cde4a8707871da7df726099"},{"author":{"_account_id":25468,"name":"Michel Nederlof","email":"michel@nederlof.info","username":"pellucid"},"change_message_id":"7e18480b24d2312ac55f0d823b54ce5f0648309f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"2346c3e3_bc5ff660","updated":"2023-10-05 14:51:01.000000000","message":"recheck neutron-functional-with-uwsgi not related.","commit_id":"61a2fae7a1164c5fac2fde883b4c4ea59f965e44"},{"author":{"_account_id":25468,"name":"Michel Nederlof","email":"michel@nederlof.info","username":"pellucid"},"change_message_id":"a5b7bf477f6fd8b8abbc1d85ab7a0044431cf02c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"7882f6e5_5272dabf","updated":"2023-10-24 13:55:15.000000000","message":"With this latest patchset we update the OVN DB at the same spot where we update the neutron DB, since the move is only triggered by this PortBindingUpdateVirtualPortsEvent","commit_id":"aae7db0f2a8d931a85f5520ac0af14e284e3ed53"},{"author":{"_account_id":25468,"name":"Michel Nederlof","email":"michel@nederlof.info","username":"pellucid"},"change_message_id":"33fa38d40c5eacbcc2efffbddac8b04ad8c0b381","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"a6fd1e00_9dde4b0c","updated":"2023-10-25 15:12:45.000000000","message":"this patchset looks good to me.","commit_id":"078876aae0c47a0c90a35b58cbe96aa3d9898217"},{"author":{"_account_id":25468,"name":"Michel Nederlof","email":"michel@nederlof.info","username":"pellucid"},"change_message_id":"e998bff9727af20e13383afd946ee33e01c0c17c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":13,"id":"c83c9ead_12ac7af4","updated":"2024-01-09 14:43:43.000000000","message":"@Rodolfo or @Brian, is there anything required from my side to get this patch moving forward?","commit_id":"e2f8894ce86ecb06d80c1905b9bafea5d00966fe"},{"author":{"_account_id":25468,"name":"Michel Nederlof","email":"michel@nederlof.info","username":"pellucid"},"change_message_id":"7eed915f7820b44594d81fe9eb4771dd9a782b78","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":13,"id":"36fbd29b_6f168589","updated":"2023-12-07 14:12:29.000000000","message":"Updated the code with less deps.","commit_id":"e2f8894ce86ecb06d80c1905b9bafea5d00966fe"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"987060026b9f5f794933f57fddb9294755dfa4a2","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":15,"id":"2b6c0602_86929bfa","updated":"2024-01-15 07:46:55.000000000","message":"We need to backport this patch up to Wallaby (if possible); at least up to Yoga, that is still **not** in EM state.","commit_id":"e68a920c114010a36a667d90866bfb6243148b6d"}],"neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py":[{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"c10a4ea87cf23d3165402f184da3ee02f112ac42","unresolved":true,"context_lines":[{"line_number":1078,"context_line":"            # could be that we are processing a delete event"},{"line_number":1079,"context_line":"            return"},{"line_number":1080,"context_line":""},{"line_number":1081,"context_line":"        self._ovn_client.update_lsp_host_info(admin_context, db_port)"},{"line_number":1082,"context_line":""},{"line_number":1083,"context_line":"    def get_workers(self):"},{"line_number":1084,"context_line":"        \"\"\"Get any worker instances that should have their own process"}],"source_content_type":"text/x-python","patch_set":7,"id":"ef709fff_cec420dc","line":1081,"updated":"2023-11-03 16:52:28.000000000","message":"I think we should update/add a test to cover this.","commit_id":"078876aae0c47a0c90a35b58cbe96aa3d9898217"},{"author":{"_account_id":25468,"name":"Michel Nederlof","email":"michel@nederlof.info","username":"pellucid"},"change_message_id":"71601edfeb69296b1fc9ecd62347160628694037","unresolved":false,"context_lines":[{"line_number":1078,"context_line":"            # could be that we are processing a delete event"},{"line_number":1079,"context_line":"            return"},{"line_number":1080,"context_line":""},{"line_number":1081,"context_line":"        self._ovn_client.update_lsp_host_info(admin_context, db_port)"},{"line_number":1082,"context_line":""},{"line_number":1083,"context_line":"    def get_workers(self):"},{"line_number":1084,"context_line":"        \"\"\"Get any worker instances that should have their own process"}],"source_content_type":"text/x-python","patch_set":7,"id":"0152d264_97aaa691","line":1081,"in_reply_to":"3b33810f_6ecc3f11","updated":"2023-11-13 15:56:37.000000000","message":"I\u0027ve implemented a functional test, which actually tests if this method does what it is supposed to do, which is updating the hostname on the neutron port and on the ovn port-binding","commit_id":"078876aae0c47a0c90a35b58cbe96aa3d9898217"},{"author":{"_account_id":25468,"name":"Michel Nederlof","email":"michel@nederlof.info","username":"pellucid"},"change_message_id":"0749a457f27c297eaa664043c9bfa37bec9d5adf","unresolved":true,"context_lines":[{"line_number":1078,"context_line":"            # could be that we are processing a delete event"},{"line_number":1079,"context_line":"            return"},{"line_number":1080,"context_line":""},{"line_number":1081,"context_line":"        self._ovn_client.update_lsp_host_info(admin_context, db_port)"},{"line_number":1082,"context_line":""},{"line_number":1083,"context_line":"    def get_workers(self):"},{"line_number":1084,"context_line":"        \"\"\"Get any worker instances that should have their own process"}],"source_content_type":"text/x-python","patch_set":7,"id":"3b33810f_6ecc3f11","line":1081,"in_reply_to":"ef709fff_cec420dc","updated":"2023-11-06 09:31:41.000000000","message":"I\u0027ll see what i can do; i\u0027m assuming you are referring to functional tests, not unit tests, right?","commit_id":"078876aae0c47a0c90a35b58cbe96aa3d9898217"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"2ee98b0b41ce48c27dddf9f2c636ab1998a97505","unresolved":true,"context_lines":[{"line_number":1073,"context_line":"                                              hostname)"},{"line_number":1074,"context_line":""},{"line_number":1075,"context_line":"        # Get record from database, and update NB DB"},{"line_number":1076,"context_line":"        db_port \u003d ml2_db.get_port(admin_context, port_id)"},{"line_number":1077,"context_line":"        if not db_port:"},{"line_number":1078,"context_line":"            # could be that we are processing a delete event"},{"line_number":1079,"context_line":"            return"},{"line_number":1080,"context_line":""},{"line_number":1081,"context_line":"        self._ovn_client.update_lsp_host_info(admin_context, db_port)"},{"line_number":1082,"context_line":""},{"line_number":1083,"context_line":"    def get_workers(self):"},{"line_number":1084,"context_line":"        \"\"\"Get any worker instances that should have their own process"}],"source_content_type":"text/x-python","patch_set":12,"id":"d7271ecb_5c28edd9","line":1081,"range":{"start_line":1076,"start_character":8,"end_line":1081,"end_character":69},"updated":"2023-12-07 08:22:48.000000000","message":"I think I said that before: there is no need to call \"update_lsp_host_info\". If the host info is needed in the VIP LSP external_ids, add this in \"update_virtual_port_host\" method; do not call \"update_lsp_host_info\" with the DB port (that is an additional DB call).","commit_id":"b373fbd3fb7f2dbe898946ff97860cb747b22fd2"},{"author":{"_account_id":25468,"name":"Michel Nederlof","email":"michel@nederlof.info","username":"pellucid"},"change_message_id":"7eed915f7820b44594d81fe9eb4771dd9a782b78","unresolved":false,"context_lines":[{"line_number":1073,"context_line":"                                              hostname)"},{"line_number":1074,"context_line":""},{"line_number":1075,"context_line":"        # Get record from database, and update NB DB"},{"line_number":1076,"context_line":"        db_port \u003d ml2_db.get_port(admin_context, port_id)"},{"line_number":1077,"context_line":"        if not db_port:"},{"line_number":1078,"context_line":"            # could be that we are processing a delete event"},{"line_number":1079,"context_line":"            return"},{"line_number":1080,"context_line":""},{"line_number":1081,"context_line":"        self._ovn_client.update_lsp_host_info(admin_context, db_port)"},{"line_number":1082,"context_line":""},{"line_number":1083,"context_line":"    def get_workers(self):"},{"line_number":1084,"context_line":"        \"\"\"Get any worker instances that should have their own process"}],"source_content_type":"text/x-python","patch_set":12,"id":"f5c3c147_84fd10a4","line":1081,"range":{"start_line":1076,"start_character":8,"end_line":1081,"end_character":69},"in_reply_to":"d7271ecb_5c28edd9","updated":"2023-12-07 14:12:29.000000000","message":"I believe i get what you mean now. I\u0027ve added the functionality required to update the LSP inside that method.","commit_id":"b373fbd3fb7f2dbe898946ff97860cb747b22fd2"}],"neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovsdb_monitor.py":[{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"7ba6c95886432d53314390c3e41dd438b81d561c","unresolved":true,"context_lines":[{"line_number":584,"context_line":"            chassis_uuid \u003d (row.chassis[0].uuid if"},{"line_number":585,"context_line":"                            row.chassis and virtual_parents else None)"},{"line_number":586,"context_line":"        self.driver.update_virtual_port_host(row.logical_port, chassis_uuid)"},{"line_number":587,"context_line":"        self.driver.set_port_status_up(port_id)"},{"line_number":588,"context_line":""},{"line_number":589,"context_line":""},{"line_number":590,"context_line":"class FIPAddDeleteEvent(row_event.RowEvent):"}],"source_content_type":"text/x-python","patch_set":2,"id":"52d58b28_b103de59","line":587,"range":{"start_line":587,"start_character":8,"end_line":587,"end_character":47},"updated":"2023-10-04 08:35:22.000000000","message":"A VIP is not bound to a host not set to UP. We manually define the host ID catching this event. What port do you want to set to UP.\n\nDo you have in your env the following patch: https://review.opendev.org/q/Ide3204748274cbab9731310c2e2bd8bd8c9a53ff","commit_id":"b09752c46a61acba31ba874e5f33b50bc312f818"},{"author":{"_account_id":25468,"name":"Michel Nederlof","email":"michel@nederlof.info","username":"pellucid"},"change_message_id":"b8ae4b6fec211b1e3b6bb0b37f3f305623307f87","unresolved":false,"context_lines":[{"line_number":584,"context_line":"            chassis_uuid \u003d (row.chassis[0].uuid if"},{"line_number":585,"context_line":"                            row.chassis and virtual_parents else None)"},{"line_number":586,"context_line":"        self.driver.update_virtual_port_host(row.logical_port, chassis_uuid)"},{"line_number":587,"context_line":"        self.driver.set_port_status_up(port_id)"},{"line_number":588,"context_line":""},{"line_number":589,"context_line":""},{"line_number":590,"context_line":"class FIPAddDeleteEvent(row_event.RowEvent):"}],"source_content_type":"text/x-python","patch_set":2,"id":"aabcd8cc_fb2711db","line":587,"range":{"start_line":587,"start_character":8,"end_line":587,"end_character":47},"in_reply_to":"52d58b28_b103de59","updated":"2023-10-04 09:00:54.000000000","message":"Actually, this method will not set the port UP (unless the device owner is set to some specific/reserved values), but it will update the host info on the lsp\n\nThe real method i require to call is\n```\nself._ovn_client.update_lsp_host_info(admin_context, db_port)\n```\n\nbecause only that method is updating the `neutron:host_id` property in the external_ids field in the OVN DB.\n\nFrom what i\u0027ve found, this is only called from `set_port_status_up` and `set_port_status_down`, so that\u0027s why i chose to re-use this call here too.\n\nAlso, because the method is called from the PortBindingChassisUpdateEvent as well, but this could be triggered a few milliseconds too early, updating the OVN DB with the old data that was stored in the database. So i\u0027m just re-using the same call the other event did just a few moments earlier, but now with the data updated in the database.\n\nLast, no we did not have that patch, but that only applies if the port_binding aka lsp (logical switch port) aka neutron port is deleted from the database, which is not the case when a IP does a failover (the port binding is just updated).","commit_id":"b09752c46a61acba31ba874e5f33b50bc312f818"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"3a1989e1605196c1e1a242da6a1f65e9f4366549","unresolved":true,"context_lines":[{"line_number":584,"context_line":"            chassis_uuid \u003d (row.chassis[0].uuid if"},{"line_number":585,"context_line":"                            row.chassis and virtual_parents else None)"},{"line_number":586,"context_line":""},{"line_number":587,"context_line":"            # Only update the value in OVN if the event is a UPDATE"},{"line_number":588,"context_line":"            self.driver.set_port_status_up(row.logical_port)"},{"line_number":589,"context_line":""},{"line_number":590,"context_line":"        self.driver.update_virtual_port_host(row.logical_port, chassis_uuid)"},{"line_number":591,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"9a60d34d_0e326d58","line":588,"range":{"start_line":587,"start_character":12,"end_line":588,"end_character":60},"updated":"2023-10-09 13:45:31.000000000","message":"I\u0027ll comment that again: Neutron does not set the port status for virtual ports. The method \"update_virtual_port_host\" is used to define the port host.\n\nI\u0027ve been testing with some environments and what is happening is that when the VIP is moved to another host is that \"port_binding.chassis\" is changing without modifying anything else in the register. This change is not captured by the \"PortBindingUpdateVirtualPortsEvent\" event and this is the change you need to do here.","commit_id":"61a2fae7a1164c5fac2fde883b4c4ea59f965e44"},{"author":{"_account_id":25468,"name":"Michel Nederlof","email":"michel@nederlof.info","username":"pellucid"},"change_message_id":"a5b7bf477f6fd8b8abbc1d85ab7a0044431cf02c","unresolved":false,"context_lines":[{"line_number":584,"context_line":"            chassis_uuid \u003d (row.chassis[0].uuid if"},{"line_number":585,"context_line":"                            row.chassis and virtual_parents else None)"},{"line_number":586,"context_line":""},{"line_number":587,"context_line":"            # Only update the value in OVN if the event is a UPDATE"},{"line_number":588,"context_line":"            self.driver.set_port_status_up(row.logical_port)"},{"line_number":589,"context_line":""},{"line_number":590,"context_line":"        self.driver.update_virtual_port_host(row.logical_port, chassis_uuid)"},{"line_number":591,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"87ef929d_30e66e4d","line":588,"range":{"start_line":587,"start_character":12,"end_line":588,"end_character":60},"in_reply_to":"872bed7e_05efbdff","updated":"2023-10-24 13:55:15.000000000","message":"When using master branch i do see that the other events no longer match on the vip move. Therefore i implemented the host_id change for OVN NBDB within the update_virtual_port_host method inside the mech_driver.py\n\nThen the host update is entirely updated by this event for both the neutron DB as well as the OVN DB.","commit_id":"61a2fae7a1164c5fac2fde883b4c4ea59f965e44"},{"author":{"_account_id":25468,"name":"Michel Nederlof","email":"michel@nederlof.info","username":"pellucid"},"change_message_id":"6fb9c9a9179546eadad2fddb2a5cf3948b011a9e","unresolved":true,"context_lines":[{"line_number":584,"context_line":"            chassis_uuid \u003d (row.chassis[0].uuid if"},{"line_number":585,"context_line":"                            row.chassis and virtual_parents else None)"},{"line_number":586,"context_line":""},{"line_number":587,"context_line":"            # Only update the value in OVN if the event is a UPDATE"},{"line_number":588,"context_line":"            self.driver.set_port_status_up(row.logical_port)"},{"line_number":589,"context_line":""},{"line_number":590,"context_line":"        self.driver.update_virtual_port_host(row.logical_port, chassis_uuid)"},{"line_number":591,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"872bed7e_05efbdff","line":588,"range":{"start_line":587,"start_character":12,"end_line":588,"end_character":60},"in_reply_to":"9a60d34d_0e326d58","updated":"2023-10-09 14:54:08.000000000","message":"I\u0027ll start testing some more options here, maybe modifying the match_fn would be sufficient as you imply. I\u0027ll get back on this.","commit_id":"61a2fae7a1164c5fac2fde883b4c4ea59f965e44"},{"author":{"_account_id":25468,"name":"Michel Nederlof","email":"michel@nederlof.info","username":"pellucid"},"change_message_id":"a5b7bf477f6fd8b8abbc1d85ab7a0044431cf02c","unresolved":false,"context_lines":[{"line_number":567,"context_line":"        if len(old._data) \u003d\u003d 1 and \u0027external_ids\u0027 in old._data:"},{"line_number":568,"context_line":"            # We should not trigger on a update which only contains external_ids"},{"line_number":569,"context_line":"            return False"},{"line_number":570,"context_line":""},{"line_number":571,"context_line":"        virtual_parents \u003d (row.options or {}).get("},{"line_number":572,"context_line":"            ovn_const.LSP_OPTIONS_VIRTUAL_PARENTS_KEY)"},{"line_number":573,"context_line":"        old_virtual_parents \u003d getattr(old, \u0027options\u0027, {}).get("}],"source_content_type":"text/x-python","patch_set":6,"id":"a1b58a0b_e2839e04","line":570,"updated":"2023-10-24 13:55:15.000000000","message":"When we update the OVN DB with the host_id, it also triggers a update event, which we should not trigger upon.","commit_id":"aae7db0f2a8d931a85f5520ac0af14e284e3ed53"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"2ee98b0b41ce48c27dddf9f2c636ab1998a97505","unresolved":true,"context_lines":[{"line_number":560,"context_line":"            return row.type \u003d\u003d ovn_const.LSP_TYPE_VIRTUAL"},{"line_number":561,"context_line":""},{"line_number":562,"context_line":"        if len(old._data) \u003d\u003d 1 and \u0027external_ids\u0027 in old._data:"},{"line_number":563,"context_line":"            # When we update the OVN DB with the host_id, it also triggers"},{"line_number":564,"context_line":"            # a update event, which we should not trigger upon, when the"},{"line_number":565,"context_line":"            # update only contains external_ids"},{"line_number":566,"context_line":"            return False"},{"line_number":567,"context_line":""},{"line_number":568,"context_line":"        virtual_parents \u003d (row.options or {}).get("}],"source_content_type":"text/x-python","patch_set":12,"id":"f4e6cb1d_670c26fb","line":565,"range":{"start_line":563,"start_character":12,"end_line":565,"end_character":47},"updated":"2023-12-07 08:22:48.000000000","message":"I know what you mean here because I know the code, but this description is not clear. I would suggest:\n\"\"\"\nWhen a virtual port host ID is updated in the OVN DB LSP register (``LSP.external_ids[\"neutron:host_id\"]``), that triggers a new ``PortBindingUpdateVirtualPortsEvent`` updating only the ``external_ids`` field; that should be skipped.\n\"\"\"","commit_id":"b373fbd3fb7f2dbe898946ff97860cb747b22fd2"},{"author":{"_account_id":25468,"name":"Michel Nederlof","email":"michel@nederlof.info","username":"pellucid"},"change_message_id":"7eed915f7820b44594d81fe9eb4771dd9a782b78","unresolved":false,"context_lines":[{"line_number":560,"context_line":"            return row.type \u003d\u003d ovn_const.LSP_TYPE_VIRTUAL"},{"line_number":561,"context_line":""},{"line_number":562,"context_line":"        if len(old._data) \u003d\u003d 1 and \u0027external_ids\u0027 in old._data:"},{"line_number":563,"context_line":"            # When we update the OVN DB with the host_id, it also triggers"},{"line_number":564,"context_line":"            # a update event, which we should not trigger upon, when the"},{"line_number":565,"context_line":"            # update only contains external_ids"},{"line_number":566,"context_line":"            return False"},{"line_number":567,"context_line":""},{"line_number":568,"context_line":"        virtual_parents \u003d (row.options or {}).get("}],"source_content_type":"text/x-python","patch_set":12,"id":"5cfe059d_5f7a6b40","line":565,"range":{"start_line":563,"start_character":12,"end_line":565,"end_character":47},"in_reply_to":"f4e6cb1d_670c26fb","updated":"2023-12-07 14:12:29.000000000","message":"Check, i\u0027ve updated the comment.","commit_id":"b373fbd3fb7f2dbe898946ff97860cb747b22fd2"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"5c76cd87004d824861215ed115aa5e20bf38f42f","unresolved":true,"context_lines":[{"line_number":559,"context_line":"            # port was not deleted before)."},{"line_number":560,"context_line":"            return row.type \u003d\u003d ovn_const.LSP_TYPE_VIRTUAL"},{"line_number":561,"context_line":""},{"line_number":562,"context_line":"        if len(old._data) \u003d\u003d 1 and \u0027external_ids\u0027 in old._data:"},{"line_number":563,"context_line":"            # When the virtual port is updated with the host_id in the"},{"line_number":564,"context_line":"            # OVN DB LSP register (``LSP.external_ids[\"neutron:host_id\"]``),"},{"line_number":565,"context_line":"            # it will trigger a new ``PortBindingUpdateVirtualPortsEvent``"}],"source_content_type":"text/x-python","patch_set":13,"id":"96b3818e_50cd8960","line":562,"range":{"start_line":562,"start_character":8,"end_line":562,"end_character":63},"updated":"2024-01-09 15:50:37.000000000","message":"By default a change in the external_ids:neutron:host_id won\u0027t trigger the call of \"run\" method. Why is that needed?","commit_id":"e2f8894ce86ecb06d80c1905b9bafea5d00966fe"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"e3d6450c09fd28ad3bfc95dfb9fc6e71fb273c70","unresolved":true,"context_lines":[{"line_number":559,"context_line":"            # port was not deleted before)."},{"line_number":560,"context_line":"            return row.type \u003d\u003d ovn_const.LSP_TYPE_VIRTUAL"},{"line_number":561,"context_line":""},{"line_number":562,"context_line":"        if len(old._data) \u003d\u003d 1 and \u0027external_ids\u0027 in old._data:"},{"line_number":563,"context_line":"            # When the virtual port is updated with the host_id in the"},{"line_number":564,"context_line":"            # OVN DB LSP register (``LSP.external_ids[\"neutron:host_id\"]``),"},{"line_number":565,"context_line":"            # it will trigger a new ``PortBindingUpdateVirtualPortsEvent``"}],"source_content_type":"text/x-python","patch_set":13,"id":"69e9b8b9_cbba2088","line":562,"range":{"start_line":562,"start_character":8,"end_line":562,"end_character":63},"in_reply_to":"3e39fe09_a92f90d7","updated":"2024-01-09 16:14:57.000000000","message":"Then what I think is that the virtual parents check must be more robust, to prevent returning True in the related case. Any condition outside the virtual parents check, should default to L584: \"return False\".\n\nThus what we need to do is to check that \"options\" is in old and then retrieve the virtual parents.\n```\n    try:\n        old_options \u003d getattr(old, \u0027options\u0027)\n    except AttributeError:\n        # The \"old.options\" dictionary is not being modified,\n        # thus the virtual parents didn\u0027t change.\n        return False\n\n    virtual_parents \u003d (row.options or {}).get(\n        ovn_const.LSP_OPTIONS_VIRTUAL_PARENTS_KEY)\n    old_virtual_parents \u003d old_options.get(\n        ovn_const.LSP_OPTIONS_VIRTUAL_PARENTS_KEY)\n```","commit_id":"e2f8894ce86ecb06d80c1905b9bafea5d00966fe"},{"author":{"_account_id":25468,"name":"Michel Nederlof","email":"michel@nederlof.info","username":"pellucid"},"change_message_id":"2dfc6d9e9211be08549aea1c96838a1b0ab580f4","unresolved":false,"context_lines":[{"line_number":559,"context_line":"            # port was not deleted before)."},{"line_number":560,"context_line":"            return row.type \u003d\u003d ovn_const.LSP_TYPE_VIRTUAL"},{"line_number":561,"context_line":""},{"line_number":562,"context_line":"        if len(old._data) \u003d\u003d 1 and \u0027external_ids\u0027 in old._data:"},{"line_number":563,"context_line":"            # When the virtual port is updated with the host_id in the"},{"line_number":564,"context_line":"            # OVN DB LSP register (``LSP.external_ids[\"neutron:host_id\"]``),"},{"line_number":565,"context_line":"            # it will trigger a new ``PortBindingUpdateVirtualPortsEvent``"}],"source_content_type":"text/x-python","patch_set":13,"id":"d465985b_768711e6","line":562,"range":{"start_line":562,"start_character":8,"end_line":562,"end_character":63},"in_reply_to":"69e9b8b9_cbba2088","updated":"2024-01-11 11:23:01.000000000","message":"I\u0027ve updated the check to see if the chassis changed (and if it does to return True), which actually might only be the check we really need for this event. (since updating the host_id is about the only thing we do here in this event class)\n\nWhen changing it in my dev, the checks run fine, though that might not be all-inclusive.\n\nWhat do you think @Rodolfo Alonso ?","commit_id":"e2f8894ce86ecb06d80c1905b9bafea5d00966fe"},{"author":{"_account_id":25468,"name":"Michel Nederlof","email":"michel@nederlof.info","username":"pellucid"},"change_message_id":"ea4e75c85322e06847ee7887da23bc544228f71e","unresolved":true,"context_lines":[{"line_number":559,"context_line":"            # port was not deleted before)."},{"line_number":560,"context_line":"            return row.type \u003d\u003d ovn_const.LSP_TYPE_VIRTUAL"},{"line_number":561,"context_line":""},{"line_number":562,"context_line":"        if len(old._data) \u003d\u003d 1 and \u0027external_ids\u0027 in old._data:"},{"line_number":563,"context_line":"            # When the virtual port is updated with the host_id in the"},{"line_number":564,"context_line":"            # OVN DB LSP register (``LSP.external_ids[\"neutron:host_id\"]``),"},{"line_number":565,"context_line":"            # it will trigger a new ``PortBindingUpdateVirtualPortsEvent``"}],"source_content_type":"text/x-python","patch_set":13,"id":"b14e79a6_5f078233","line":562,"range":{"start_line":562,"start_character":8,"end_line":562,"end_character":63},"in_reply_to":"96b3818e_50cd8960","updated":"2024-01-09 15:56:25.000000000","message":"Well, if the old data only holds `external_ids` as property, then (with the comparison below) it would return True.\n\nAs virtual_parents is received from the actual record (and is present at this point), and old_virtual_parents is None (as the virtual_parents_key is not present in the old parameter).\n\nSo virtual_parents is not equal to old_virtual_parents, and the match_fn would return True.\n\nOr am i missing something here?","commit_id":"e2f8894ce86ecb06d80c1905b9bafea5d00966fe"},{"author":{"_account_id":25468,"name":"Michel Nederlof","email":"michel@nederlof.info","username":"pellucid"},"change_message_id":"c0e54a47332a06cf9b7e89cb1da8a8760af55706","unresolved":true,"context_lines":[{"line_number":559,"context_line":"            # port was not deleted before)."},{"line_number":560,"context_line":"            return row.type \u003d\u003d ovn_const.LSP_TYPE_VIRTUAL"},{"line_number":561,"context_line":""},{"line_number":562,"context_line":"        if len(old._data) \u003d\u003d 1 and \u0027external_ids\u0027 in old._data:"},{"line_number":563,"context_line":"            # When the virtual port is updated with the host_id in the"},{"line_number":564,"context_line":"            # OVN DB LSP register (``LSP.external_ids[\"neutron:host_id\"]``),"},{"line_number":565,"context_line":"            # it will trigger a new ``PortBindingUpdateVirtualPortsEvent``"}],"source_content_type":"text/x-python","patch_set":13,"id":"3e39fe09_a92f90d7","line":562,"range":{"start_line":562,"start_character":8,"end_line":562,"end_character":63},"in_reply_to":"b14e79a6_5f078233","updated":"2024-01-09 16:04:54.000000000","message":"Because what i\u0027m trying to accomplish here is to prevent the execution of the run method, when it is updated by OVN.\n\nSince the update in the `NB/Logical_Switch_Port` table (by update_virtual_port_host) also triggers a update in the SB/Port_Binding table, I just want to prevent a unnecessary run, since we already processed the actual update.","commit_id":"e2f8894ce86ecb06d80c1905b9bafea5d00966fe"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"e34e35eaa42dd6964facfbb43bb88e2ec555c0c4","unresolved":true,"context_lines":[{"line_number":567,"context_line":"            # which means we need to update the host_id information"},{"line_number":568,"context_line":"            return True"},{"line_number":569,"context_line":""},{"line_number":570,"context_line":"        if getattr(old, \u0027options\u0027, None) is not None:"},{"line_number":571,"context_line":"            # The \"old.options\" dictionary is not being modified,"},{"line_number":572,"context_line":"            # thus the virtual parents didn\u0027t change."},{"line_number":573,"context_line":"            return False"}],"source_content_type":"text/x-python","patch_set":15,"id":"148eda42_a478d550","line":570,"updated":"2024-06-07 15:32:36.000000000","message":"Shouldn\u0027t it just be \u0027is\u0027?\n\nAccording to the commend inside the block, getattr(old, \u0027options\u0027, None) will return None when attr \u0027options\u0027 is not present in old. Alternative one is to return True in L573. Or did I misunderstand?\"","commit_id":"e68a920c114010a36a667d90866bfb6243148b6d"},{"author":{"_account_id":25468,"name":"Michel Nederlof","email":"michel@nederlof.info","username":"pellucid"},"change_message_id":"c4f4f099fbe76fbf3a5c79bfe9abd6e535da3b45","unresolved":true,"context_lines":[{"line_number":567,"context_line":"            # which means we need to update the host_id information"},{"line_number":568,"context_line":"            return True"},{"line_number":569,"context_line":""},{"line_number":570,"context_line":"        if getattr(old, \u0027options\u0027, None) is not None:"},{"line_number":571,"context_line":"            # The \"old.options\" dictionary is not being modified,"},{"line_number":572,"context_line":"            # thus the virtual parents didn\u0027t change."},{"line_number":573,"context_line":"            return False"}],"source_content_type":"text/x-python","patch_set":15,"id":"d93ffe01_d4df0f8b","line":570,"in_reply_to":"148eda42_a478d550","updated":"2024-06-10 09:42:11.000000000","message":"Ah yes i believe you are correct, good find! I think we need to fix this in a new bugfix? (e.g. new review, as this one is merged)","commit_id":"e68a920c114010a36a667d90866bfb6243148b6d"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"bb8bee9a62e3a0963a42c5adaf88710e64451213","unresolved":true,"context_lines":[{"line_number":567,"context_line":"            # which means we need to update the host_id information"},{"line_number":568,"context_line":"            return True"},{"line_number":569,"context_line":""},{"line_number":570,"context_line":"        if getattr(old, \u0027options\u0027, None) is not None:"},{"line_number":571,"context_line":"            # The \"old.options\" dictionary is not being modified,"},{"line_number":572,"context_line":"            # thus the virtual parents didn\u0027t change."},{"line_number":573,"context_line":"            return False"}],"source_content_type":"text/x-python","patch_set":15,"id":"e38fb0e5_6cb41bbf","line":570,"in_reply_to":"d93ffe01_d4df0f8b","updated":"2024-06-10 10:02:05.000000000","message":"yeah, make totally sense to cover in a new easy patch.","commit_id":"e68a920c114010a36a667d90866bfb6243148b6d"},{"author":{"_account_id":25468,"name":"Michel Nederlof","email":"michel@nederlof.info","username":"pellucid"},"change_message_id":"c0464bbffec3ca95c89e247e610a1c3a4101f077","unresolved":false,"context_lines":[{"line_number":567,"context_line":"            # which means we need to update the host_id information"},{"line_number":568,"context_line":"            return True"},{"line_number":569,"context_line":""},{"line_number":570,"context_line":"        if getattr(old, \u0027options\u0027, None) is not None:"},{"line_number":571,"context_line":"            # The \"old.options\" dictionary is not being modified,"},{"line_number":572,"context_line":"            # thus the virtual parents didn\u0027t change."},{"line_number":573,"context_line":"            return False"}],"source_content_type":"text/x-python","patch_set":15,"id":"aacf2014_347089f4","line":570,"in_reply_to":"e38fb0e5_6cb41bbf","updated":"2024-06-10 12:13:52.000000000","message":"new patch: https://review.opendev.org/c/openstack/neutron/+/921659","commit_id":"e68a920c114010a36a667d90866bfb6243148b6d"}],"neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovsdb_monitor.py":[{"author":{"_account_id":25468,"name":"Michel Nederlof","email":"michel@nederlof.info","username":"pellucid"},"change_message_id":"2dfc6d9e9211be08549aea1c96838a1b0ab580f4","unresolved":false,"context_lines":[{"line_number":323,"context_line":"        self.sb_api.db_set("},{"line_number":324,"context_line":"            \u0027Port_Binding\u0027, pb_port_vip.uuid,"},{"line_number":325,"context_line":"            (\u0027chassis\u0027, pb_port_parent.chassis),"},{"line_number":326,"context_line":"            (\u0027virtual_parent\u0027, pb_virtual_parent)).execute(check_error\u003dTrue)"},{"line_number":327,"context_line":""},{"line_number":328,"context_line":"    def _check_port_binding_type(self, port_id, port_type):"},{"line_number":329,"context_line":"        def is_port_binding_type(port_id, port_type):"}],"source_content_type":"text/x-python","patch_set":14,"id":"981f8ef2_79b5b10e","line":326,"updated":"2024-01-11 11:23:01.000000000","message":"I noticed that the virtual parent was not changed, because it was not a string object (it was a uuid object), so this fixes that potential issue.","commit_id":"cddd4e08611184512b8190988697455bf4f3a300"}]}