)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"74322c88b5c1bf2524fff62ea5e2e287cf505f0b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"c0c82096_8494c248","updated":"2024-10-04 08:33:24.000000000","message":"Just a concern about if this also applies to virtual ports. Perhaps the issue does not reproduce in that case and there is no need to cover it, but if that is the case, worth leaving a comment on the nb_bgp_watcher","commit_id":"566c4dc5b61bade8606b87721e6aa2896c4d8286"},{"author":{"_account_id":24824,"name":"Dmitrii Shcherbakov","username":"dmitriis"},"change_message_id":"c53f52d6a70d55c0602234e269f8fbd8c273293b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"f668e83e_7cadc7e8","updated":"2024-10-03 00:57:28.000000000","message":"Noticed this behavior as well, thanks for the patch!","commit_id":"566c4dc5b61bade8606b87721e6aa2896c4d8286"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"59edb24a053000c54dfeddfb4e410fee8c035fdf","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"6ddc1a0e_15680684","updated":"2024-10-07 15:27:25.000000000","message":"LGTM, just one question. Do we need to handle the withdraw too? Perhaps if the port was down the event is not matched and the IP does not get withdrawn","commit_id":"5676a2c3179476d5bb8345ef38af5d9c4c8c585f"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"91ed1f85383d6573936fe4fbae39c90408892360","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"6c9391ff_57e31c14","updated":"2024-10-07 15:30:02.000000000","message":"Sorry, hit enter too soon. This is the code from the FIP unset event that I was refering too:\n            if (hasattr(old, \u0027up\u0027) and bool(old.up[0]) and  # port was up\n                    not bool(row.up[0])):                   # is now down\n                return True\n                \nAnd sorry also for not thinking about this before, I\u0027m doing one comment only per patch set :(","commit_id":"5676a2c3179476d5bb8345ef38af5d9c4c8c585f"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"edc3a51333732aad0ffbfaa4589d45b94bbb250f","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":4,"id":"2b986e9a_55df801f","in_reply_to":"26aace96_0732f711","updated":"2024-10-07 16:37:50.000000000","message":"If the port was down then it never got the external_mac set so the NAT event wasn\u0027t triggered. The NAT event is triggered by Neutron reacting on port coming up. Does that answer the question?","commit_id":"5676a2c3179476d5bb8345ef38af5d9c4c8c585f"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"c11f688f8e8fbedcf5b6d2d6ecabed53c5a3611d","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":4,"id":"26aace96_0732f711","in_reply_to":"6c9391ff_57e31c14","updated":"2024-10-07 16:35:54.000000000","message":"All good :) I thought about that too but I thought the `LogicalSwitchPortFIPDeleteEvent` would withdraw the FIP - from the comment:\n```\n2. [UPDATE] Port went down, withdraw if we announced it\n```","commit_id":"5676a2c3179476d5bb8345ef38af5d9c4c8c585f"}],"ovn_bgp_agent/drivers/openstack/nb_ovn_bgp_driver.py":[{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"f47460d3647213da0267b552df4029532b717bdc","unresolved":true,"context_lines":[{"line_number":154,"context_line":"                  watcher.OVNPFDeleteEvent(self),"},{"line_number":155,"context_line":"                  watcher.ChassisRedirectCreateEvent(self),"},{"line_number":156,"context_line":"                  watcher.ChassisRedirectDeleteEvent(self),"},{"line_number":157,"context_line":"                  watcher.NATMACAddedEvent(self)}"},{"line_number":158,"context_line":""},{"line_number":159,"context_line":"        if CONF.exposing_method \u003d\u003d constants.EXPOSE_METHOD_VRF:"},{"line_number":160,"context_line":"            # For vrf we require more information on the logical_switch"}],"source_content_type":"text/x-python","patch_set":6,"id":"bd680454_7845d0a7","line":157,"range":{"start_line":157,"start_character":48,"end_line":157,"end_character":49},"updated":"2024-11-04 10:02:40.000000000","message":"super nit: it is usually better to close the braces in the next line, that will preserve the git history in the next addition.","commit_id":"9e6095688d91bf4b9050f4bc692834992cb9b1a1"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"f5c8f16c526b9df73df994a2e7dac0b2c8fd6d3a","unresolved":true,"context_lines":[{"line_number":154,"context_line":"                  watcher.OVNPFDeleteEvent(self),"},{"line_number":155,"context_line":"                  watcher.ChassisRedirectCreateEvent(self),"},{"line_number":156,"context_line":"                  watcher.ChassisRedirectDeleteEvent(self),"},{"line_number":157,"context_line":"                  watcher.NATMACAddedEvent(self)}"},{"line_number":158,"context_line":""},{"line_number":159,"context_line":"        if CONF.exposing_method \u003d\u003d constants.EXPOSE_METHOD_VRF:"},{"line_number":160,"context_line":"            # For vrf we require more information on the logical_switch"}],"source_content_type":"text/x-python","patch_set":6,"id":"49a594eb_c1de9b6e","line":157,"range":{"start_line":157,"start_character":48,"end_line":157,"end_character":49},"in_reply_to":"bd680454_7845d0a7","updated":"2024-11-04 13:04:09.000000000","message":"I totally agree :) Will do if rebase is needed","commit_id":"9e6095688d91bf4b9050f4bc692834992cb9b1a1"}],"ovn_bgp_agent/drivers/openstack/watchers/nb_bgp_watcher.py":[{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"74322c88b5c1bf2524fff62ea5e2e287cf505f0b","unresolved":true,"context_lines":[{"line_number":881,"context_line":"                          \u0027nat\u0027: row._uuid})"},{"line_number":882,"context_line":"            return False"},{"line_number":883,"context_line":""},{"line_number":884,"context_line":"        if lsp.type !\u003d constants.OVN_VM_VIF_PORT_TYPE:"},{"line_number":885,"context_line":"            return False"},{"line_number":886,"context_line":""},{"line_number":887,"context_line":"        try:"},{"line_number":888,"context_line":"            if lsp.options[\u0027requested-chassis\u0027] !\u003d self.agent.chassis:"}],"source_content_type":"text/x-python","patch_set":2,"id":"597559a8_3c724a8d","line":885,"range":{"start_line":884,"start_character":0,"end_line":885,"end_character":24},"updated":"2024-10-04 08:33:24.000000000","message":"what about virtual ports?","commit_id":"566c4dc5b61bade8606b87721e6aa2896c4d8286"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"5d5d592aa5b7793264071068efceb522072b9e22","unresolved":true,"context_lines":[{"line_number":881,"context_line":"                          \u0027nat\u0027: row._uuid})"},{"line_number":882,"context_line":"            return False"},{"line_number":883,"context_line":""},{"line_number":884,"context_line":"        if lsp.type !\u003d constants.OVN_VM_VIF_PORT_TYPE:"},{"line_number":885,"context_line":"            return False"},{"line_number":886,"context_line":""},{"line_number":887,"context_line":"        try:"},{"line_number":888,"context_line":"            if lsp.options[\u0027requested-chassis\u0027] !\u003d self.agent.chassis:"}],"source_content_type":"text/x-python","patch_set":2,"id":"a7f9e005_392f7c15","line":885,"range":{"start_line":884,"start_character":0,"end_line":885,"end_character":24},"in_reply_to":"597559a8_3c724a8d","updated":"2024-10-04 13:06:30.000000000","message":"I thought virtual ports are always centralized - https://opendev.org/openstack/neutron/src/commit/5c3f883a42a64a89ef202c73386743f99d8dc6b4/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py#L901-L917\n\nIf LB member !\u003d virtual port then I got it wrong.","commit_id":"566c4dc5b61bade8606b87721e6aa2896c4d8286"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"30133e989fc579280c6689fb4438c958e02ea7ef","unresolved":false,"context_lines":[{"line_number":881,"context_line":"                          \u0027nat\u0027: row._uuid})"},{"line_number":882,"context_line":"            return False"},{"line_number":883,"context_line":""},{"line_number":884,"context_line":"        if lsp.type !\u003d constants.OVN_VM_VIF_PORT_TYPE:"},{"line_number":885,"context_line":"            return False"},{"line_number":886,"context_line":""},{"line_number":887,"context_line":"        try:"},{"line_number":888,"context_line":"            if lsp.options[\u0027requested-chassis\u0027] !\u003d self.agent.chassis:"}],"source_content_type":"text/x-python","patch_set":2,"id":"a095add7_68159c7d","line":885,"range":{"start_line":884,"start_character":0,"end_line":885,"end_character":24},"in_reply_to":"5d05615e_e96bcc5e","updated":"2024-10-04 18:36:01.000000000","message":"Done","commit_id":"566c4dc5b61bade8606b87721e6aa2896c4d8286"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"2566da7440269d5d2eaf99575c0adf3486ee7064","unresolved":true,"context_lines":[{"line_number":881,"context_line":"                          \u0027nat\u0027: row._uuid})"},{"line_number":882,"context_line":"            return False"},{"line_number":883,"context_line":""},{"line_number":884,"context_line":"        if lsp.type !\u003d constants.OVN_VM_VIF_PORT_TYPE:"},{"line_number":885,"context_line":"            return False"},{"line_number":886,"context_line":""},{"line_number":887,"context_line":"        try:"},{"line_number":888,"context_line":"            if lsp.options[\u0027requested-chassis\u0027] !\u003d self.agent.chassis:"}],"source_content_type":"text/x-python","patch_set":2,"id":"5d05615e_e96bcc5e","line":885,"range":{"start_line":884,"start_character":0,"end_line":885,"end_character":24},"in_reply_to":"a14b44f2_d540cf42","updated":"2024-10-04 13:46:31.000000000","message":"right, I didn\u0027t think about that case :) Thanks!","commit_id":"566c4dc5b61bade8606b87721e6aa2896c4d8286"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"4f0df52b912640d9208f6236c86bff3ec357686e","unresolved":true,"context_lines":[{"line_number":881,"context_line":"                          \u0027nat\u0027: row._uuid})"},{"line_number":882,"context_line":"            return False"},{"line_number":883,"context_line":""},{"line_number":884,"context_line":"        if lsp.type !\u003d constants.OVN_VM_VIF_PORT_TYPE:"},{"line_number":885,"context_line":"            return False"},{"line_number":886,"context_line":""},{"line_number":887,"context_line":"        try:"},{"line_number":888,"context_line":"            if lsp.options[\u0027requested-chassis\u0027] !\u003d self.agent.chassis:"}],"source_content_type":"text/x-python","patch_set":2,"id":"a14b44f2_d540cf42","line":885,"range":{"start_line":884,"start_character":0,"end_line":885,"end_character":24},"in_reply_to":"a7f9e005_392f7c15","updated":"2024-10-04 13:10:49.000000000","message":"that is for ovn lb, but what about for amphora loadbalancers, that is not centralized but where the VM is, right? (allow-addresspairs instead of ovn-lb)","commit_id":"566c4dc5b61bade8606b87721e6aa2896c4d8286"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"0be3446bdbc0a8cf4ecb467f83b933234649c5f2","unresolved":true,"context_lines":[{"line_number":886,"context_line":"            return False"},{"line_number":887,"context_line":""},{"line_number":888,"context_line":"        try:"},{"line_number":889,"context_line":"            if lsp.options[\u0027requested-chassis\u0027] !\u003d self.agent.chassis:"},{"line_number":890,"context_line":"                return False"},{"line_number":891,"context_line":"        except KeyError:"},{"line_number":892,"context_line":"            return False"}],"source_content_type":"text/x-python","patch_set":3,"id":"c9f35383_9298bf90","line":889,"range":{"start_line":889,"start_character":12,"end_line":889,"end_character":70},"updated":"2024-10-07 06:32:01.000000000","message":"we may need to use here \"_get_chassis\" instead, since virtual ports need to use external_ids instead. See the comment we have in get_port_chassis:\n    # row.options[\u0027requested-chassis\u0027] superseeds the id in external_ids.\n    # Since it is not used for virtual ports by ovn, this option will be\n    # ignored for virtual ports.","commit_id":"402a3f22cee0db2cb18fe654180749ade9244a2f"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"9b17590c29c7f622f072b941deb22e7dcbccf541","unresolved":true,"context_lines":[{"line_number":886,"context_line":"            return False"},{"line_number":887,"context_line":""},{"line_number":888,"context_line":"        try:"},{"line_number":889,"context_line":"            if lsp.options[\u0027requested-chassis\u0027] !\u003d self.agent.chassis:"},{"line_number":890,"context_line":"                return False"},{"line_number":891,"context_line":"        except KeyError:"},{"line_number":892,"context_line":"            return False"}],"source_content_type":"text/x-python","patch_set":3,"id":"06804e1c_5389b158","line":889,"range":{"start_line":889,"start_character":12,"end_line":889,"end_character":70},"in_reply_to":"c9f35383_9298bf90","updated":"2024-10-07 12:26:33.000000000","message":"Thanks! Will update","commit_id":"402a3f22cee0db2cb18fe654180749ade9244a2f"}]}