)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"9de775bd86c1eb09328c145766f081eea57739f0","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"0339413a_308e3998","updated":"2024-01-22 07:28:02.000000000","message":"I have a concern about this. Based on the example at https://bugs.launchpad.net/ovn-bgp-agent/+bug/2049902, during the migration, with the old code, the traffic to the VM will still be directed to the initial location (which, unless postcopy method is used, looks about right). With the new method, given that the options has both requested chassis, the traffic will be directed to any of both nodes as you would expose it in both, no?\n\nAdding -1 just for discussion","commit_id":"30022ea9214314bebdd2f8d0a3c2f251f51e046e"},{"author":{"_account_id":25468,"name":"Michel Nederlof","email":"michel@nederlof.info","username":"pellucid"},"change_message_id":"e3633a340d2fca52624635a3c35bd11540a2c12d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"3e84bac2_2ecfcf3a","updated":"2024-01-19 16:36:25.000000000","message":"I\u0027m reviewing the nb_bgp_watcher events to see if there is logic to be updated to verify if it now works as expected.","commit_id":"30022ea9214314bebdd2f8d0a3c2f251f51e046e"},{"author":{"_account_id":25468,"name":"Michel Nederlof","email":"michel@nederlof.info","username":"pellucid"},"change_message_id":"d1e4bab2b8f602379d391e6651baf8a857c587eb","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"85236058_1a268831","in_reply_to":"0339413a_308e3998","updated":"2024-01-22 09:06:38.000000000","message":"We are using post-copy with live migrations, so maybe this has to do with it, but the time where the ip is on 2 locations, is really small (maybe few seconds). \n\nBut yes, during that time, the ip would be exposed to both nodes.\n\nBut from reading what OVN says about the options field, is that this field is the one that should/is used internally. So while it is great that neutron maintains it too, i think it would be better to use the options field, since that is directly maintained by OVN without external intervention. So when neutron is lagging behind (or for some unknown reason it crashed), then this information should still be correct and more up-to-date.\n\nI\u0027ll work on some examples today with the nb_watcher where we might be able to streamline the events more to check if the port has been bound/unbound to the host.\n\nFor example, maybe we should check if there is even a chassis location in the `old` record (since these are only the updates applied in OVN), if not, it has not moved, so then we would not need to announce or withdraw the ip.","commit_id":"30022ea9214314bebdd2f8d0a3c2f251f51e046e"},{"author":{"_account_id":25468,"name":"Michel Nederlof","email":"michel@nederlof.info","username":"pellucid"},"change_message_id":"7b461c644e5536da9816545e14f43b732cd748a0","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"3ec8e2ac_edfc2fba","updated":"2024-01-22 17:11:57.000000000","message":"Other events need to be checked as well, but for me these two at least seem to work very well during live migrations.\n\nMaybe they can be even smaller than this, but as an example and talking point this could make sense to me.\n\nI have also timed the events and created some logs:\n```\n# Start of migration\n2024-01-22 16:57:18.357 [-] UPDATE: up: True, neutron:host_id: host1, options: {\u0027requested-chassis\u0027: \u0027host1,host2\u0027, \u0027activation-strategy\u0027: \u0027rarp\u0027} | old up: -, host_id: host1, old options: {\u0027requested-chassis\u0027: \u0027host1\u0027}\n2024-01-22 16:57:18.912 [-] UPDATE: up: True, neutron:host_id: host1, options: {\u0027requested-chassis\u0027: \u0027host1,host2\u0027, \u0027activation-strategy\u0027: \u0027rarp\u0027} | old up: -, host_id: host1, old options: {}\n\n# Here the port goes down at source host\n2024-01-22 16:57:21.590 [-] UPDATE: up: False, neutron:host_id: host1, options: {\u0027requested-chassis\u0027: \u0027host1,host2\u0027, \u0027activation-strategy\u0027: \u0027rarp\u0027} | old up: True, host_id: -, old options: {}\n2024-01-22 16:57:21.999 [-] UPDATE: up: False, neutron:host_id: host1, options: {\u0027requested-chassis\u0027: \u0027host1,host2\u0027, \u0027activation-strategy\u0027: \u0027rarp\u0027} | old up: -, host_id: host1, old options: {}\n2024-01-22 16:57:22.313 [-] UPDATE: up: False, neutron:host_id: NOT SET, options: {\u0027requested-chassis\u0027: \u0027host1,host2\u0027, \u0027activation-strategy\u0027: \u0027rarp\u0027} | old up: -, host_id: host1, old options: {}\n2024-01-22 16:57:22.837 [-] UPDATE: up: False, neutron:host_id: NOT SET, options: {\u0027requested-chassis\u0027: \u0027host2\u0027} | old up: -, host_id: -, old options: {\u0027requested-chassis\u0027: \u0027host1,host2\u0027, \u0027activation-strategy\u0027: \u0027rarp\u0027}\n\n# Here the port goes up again, now at the new host\n2024-01-22 16:57:22.904 [-] UPDATE: up: True, neutron:host_id: NOT SET, options: {\u0027requested-chassis\u0027: \u0027host2\u0027} | old up: False, host_id: -, old options: {}\n2024-01-22 16:57:23.187 [-] UPDATE: up: True, neutron:host_id: NOT SET, options: {\u0027requested-chassis\u0027: \u0027host2\u0027} | old up: -, host_id: -, old options: {}\n2024-01-22 16:57:23.375 [-] UPDATE: up: True, neutron:host_id: host1, options: {\u0027requested-chassis\u0027: \u0027host2\u0027} | old up: -, host_id: -, old options: {}\n```\n\nBased on these log lines we might be able to deduce a better way to handle the events 😊\n\novn-controller logs (source):\n```\n2024-01-22T16:57:21.498Z|21425|binding|INFO|Releasing lport b0d84030-d4b9-48d3-8915-29f518b52bb9 from this chassis (sb_readonly\u003d0)\n2024-01-22T16:57:21.498Z|21426|binding|INFO|Setting lport b0d84030-d4b9-48d3-8915-29f518b52bb9 down in Southbound\n```\novn-controller logs (dest):\n```\n2024-01-22T16:57:19.812Z|00460|binding|INFO|Claiming lport b0d84030-d4b9-48d3-8915-29f518b52bb9 for this additional chassis.\n2024-01-22T16:57:19.812Z|00461|binding|INFO|b0d84030-d4b9-48d3-8915-29f518b52bb9: Claiming fa:16:3e:a9:a6:75 2001:db8:1200::148 198.51.100.220\n2024-01-22T16:57:19.840Z|00462|binding|INFO|Setting lport b0d84030-d4b9-48d3-8915-29f518b52bb9 ovn-installed in OVS\n\n2024-01-22T16:57:22.820Z|00464|binding|INFO|Claiming lport b0d84030-d4b9-48d3-8915-29f518b52bb9 for this chassis.\n2024-01-22T16:57:22.820Z|00465|binding|INFO|b0d84030-d4b9-48d3-8915-29f518b52bb9: Claiming fa:16:3e:a9:a6:75 2001:db8:1200::148 198.51.100.220\n2024-01-22T16:57:22.839Z|00466|binding|INFO|Setting lport b0d84030-d4b9-48d3-8915-29f518b52bb9 up in Southbound\n```","commit_id":"78852d8b3c0aeedeffbde0f38841c70587f1719e"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"ede3c8ef4d8ccd84755d128565274fc860dab2c9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"398ca701_1fe33cdd","updated":"2024-01-23 06:59:36.000000000","message":"lots of great points in here! thanks for the contribution. I just have one concern about the virtual ports, as I remember there was an issue with its value in options and that was the reason to move to external_ids","commit_id":"78852d8b3c0aeedeffbde0f38841c70587f1719e"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"ede3c8ef4d8ccd84755d128565274fc860dab2c9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"f17b94af_f8fc7a1c","in_reply_to":"3ec8e2ac_edfc2fba","updated":"2024-01-23 06:59:36.000000000","message":"ohh, nice, the up false/true has the key here... we only expose the IP if is up... so it should be doable","commit_id":"78852d8b3c0aeedeffbde0f38841c70587f1719e"},{"author":{"_account_id":25468,"name":"Michel Nederlof","email":"michel@nederlof.info","username":"pellucid"},"change_message_id":"c9474c7de624b7f390e8b5ed46ee8f93388a8897","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":3,"id":"fb272f2a_461603b4","in_reply_to":"f17b94af_f8fc7a1c","updated":"2024-01-23 08:36:32.000000000","message":"Hmm yes, though i do not see a clear path to act on it. It works now too though, but on the dest host is is added, deleted and then added again (so it would be nice to remove the route flap here)\n\nSo what\u0027d you think about a event sequence like this:\n\nIf more then one host in options field, we start announcing on both hosts (this allows bgp to have some time to reach all hops (if needed).\n\nIf down event when more than 1 host in options field, we ignore this event, since a live migration is happening. \n\nThen, once the options field has 1 host again, we can drop the announcement on source host, and only the destination host has the announcement.","commit_id":"78852d8b3c0aeedeffbde0f38841c70587f1719e"},{"author":{"_account_id":25468,"name":"Michel Nederlof","email":"michel@nederlof.info","username":"pellucid"},"change_message_id":"a447680027c2bc02ccfa9bc546cedc3887035643","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"f8fda77e_5efa50de","in_reply_to":"fb272f2a_461603b4","updated":"2024-01-29 11:29:41.000000000","message":"Done","commit_id":"78852d8b3c0aeedeffbde0f38841c70587f1719e"},{"author":{"_account_id":25468,"name":"Michel Nederlof","email":"michel@nederlof.info","username":"pellucid"},"change_message_id":"fa94fcb2998129b0f66d27136a6bdb3001dc53da","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"b7a4011e_f7716113","updated":"2024-01-23 11:47:29.000000000","message":"First patchset with lsp watcher updates, checking and working on fip and others","commit_id":"6f2b761159accf25ee37dced0f27e157690a8e65"},{"author":{"_account_id":25468,"name":"Michel Nederlof","email":"michel@nederlof.info","username":"pellucid"},"change_message_id":"aafda7796a7a7579b61ebbd484f91971dcf271a7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"706d5b33_736590a1","updated":"2024-01-23 15:47:00.000000000","message":"I\u0027ll try to add some tests too in a next patchset","commit_id":"92e144c813ed33372fa670f301f7d4926e4b7bc5"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"76a136cf54a810e8d0acfcf830ef30be1698e0f9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"a4e0582d_62974035","updated":"2024-01-24 07:27:51.000000000","message":"Looks great! Just a couple of nits here and there","commit_id":"19fc96554d958ba9a50fb5916e7b9e1303b3c9f4"},{"author":{"_account_id":25468,"name":"Michel Nederlof","email":"michel@nederlof.info","username":"pellucid"},"change_message_id":"25be53d646cd9ead6aee043ffd2cb1d45beb3c83","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"fcca5e0e_58373174","updated":"2024-01-24 07:47:26.000000000","message":"thanks for the quick reviews!","commit_id":"19fc96554d958ba9a50fb5916e7b9e1303b3c9f4"},{"author":{"_account_id":25468,"name":"Michel Nederlof","email":"michel@nederlof.info","username":"pellucid"},"change_message_id":"a447680027c2bc02ccfa9bc546cedc3887035643","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":13,"id":"51f760d9_cc47c2df","updated":"2024-01-29 11:29:41.000000000","message":"This change is ready from my side.","commit_id":"4823d053f3ac1e6604e6d4b60d6ad4f9e1e89fb1"},{"author":{"_account_id":25468,"name":"Michel Nederlof","email":"michel@nederlof.info","username":"pellucid"},"change_message_id":"a16049005f09cfa3e71cd9b6264e7d5b954d5457","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":14,"id":"562d459a_fa9a2903","updated":"2024-01-29 16:34:28.000000000","message":"Ok, now ready, found some tests that were applicable to this change.","commit_id":"a8956ee2c44087fc63e82a1cc53c650ea591430c"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"53d802ef9a5b8f2242f2a0d2eb13c8f30e146058","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":14,"id":"5160a1a1_fef9c644","updated":"2024-02-02 07:40:03.000000000","message":"looks great! just one nit and one thing to simplify.","commit_id":"a8956ee2c44087fc63e82a1cc53c650ea591430c"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"ff2bd7999fbdf7fa4399a403d097852c1415764d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":16,"id":"848d9eb2_2786eb76","updated":"2024-02-26 12:11:20.000000000","message":"Thank u for the patch!","commit_id":"6e0d5766501ad28dabc84941f84a2edbc976d0c8"}],"ovn_bgp_agent/drivers/openstack/nb_ovn_bgp_driver.py":[{"author":{"_account_id":25468,"name":"Michel Nederlof","email":"michel@nederlof.info","username":"pellucid"},"change_message_id":"aafda7796a7a7579b61ebbd484f91971dcf271a7","unresolved":false,"context_lines":[{"line_number":345,"context_line":"            if lb.external_ids.get(constants.OVN_LB_VIP_FIP_EXT_ID_KEY):"},{"line_number":346,"context_line":"                self._withdraw_ovn_lb_fip(lb)"},{"line_number":347,"context_line":""},{"line_number":348,"context_line":"    def is_ls_exposed(self, logical_switch):"},{"line_number":349,"context_line":"        # Check to see if given logical switch is accessible through the"},{"line_number":350,"context_line":"        # exposed provider networks."},{"line_number":351,"context_line":"        if logical_switch is None:"}],"source_content_type":"text/x-python","patch_set":7,"id":"3b21479b_597cbc99","line":348,"updated":"2024-01-23 15:47:00.000000000","message":"This method can also greatly decrease some similar code in this driver 😊 (which i will pick up later on)","commit_id":"92e144c813ed33372fa670f301f7d4926e4b7bc5"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"76a136cf54a810e8d0acfcf830ef30be1698e0f9","unresolved":true,"context_lines":[{"line_number":536,"context_line":"        if logical_switch is None:"},{"line_number":537,"context_line":"            return None, None, None"},{"line_number":538,"context_line":""},{"line_number":539,"context_line":"        if logical_switch not in self.ovn_provider_ls:"},{"line_number":540,"context_line":"            localnet, bridge_dev, bridge_vlan \u003d self._get_ls_localnet_info("},{"line_number":541,"context_line":"                logical_switch)"},{"line_number":542,"context_line":""},{"line_number":543,"context_line":"            self.ovn_provider_ls[logical_switch] \u003d {"},{"line_number":544,"context_line":"                \u0027bridge_device\u0027: bridge_dev,"},{"line_number":545,"context_line":"                \u0027bridge_vlan\u0027: bridge_vlan,"},{"line_number":546,"context_line":"                \u0027localnet\u0027: localnet"},{"line_number":547,"context_line":"            }"},{"line_number":548,"context_line":""},{"line_number":549,"context_line":"        ls \u003d self.ovn_provider_ls[logical_switch]"},{"line_number":550,"context_line":"        return ls[\u0027localnet\u0027], ls[\u0027bridge_device\u0027], ls[\u0027bridge_vlan\u0027]"}],"source_content_type":"text/x-python","patch_set":9,"id":"822a336d_4c138c1a","line":547,"range":{"start_line":539,"start_character":0,"end_line":547,"end_character":13},"updated":"2024-01-24 07:27:51.000000000","message":"looks a bit weird that \"get_ls_info\" also assumes the logical_switch is a provider switch and adds it to the ovn_provider_ls. This may lead to issues is this is called for tenant logical_switches. Perhaps it should be renamed to get_provider_ls_info? or get_and_update_provider_ls_info?","commit_id":"19fc96554d958ba9a50fb5916e7b9e1303b3c9f4"},{"author":{"_account_id":25468,"name":"Michel Nederlof","email":"michel@nederlof.info","username":"pellucid"},"change_message_id":"25be53d646cd9ead6aee043ffd2cb1d45beb3c83","unresolved":true,"context_lines":[{"line_number":536,"context_line":"        if logical_switch is None:"},{"line_number":537,"context_line":"            return None, None, None"},{"line_number":538,"context_line":""},{"line_number":539,"context_line":"        if logical_switch not in self.ovn_provider_ls:"},{"line_number":540,"context_line":"            localnet, bridge_dev, bridge_vlan \u003d self._get_ls_localnet_info("},{"line_number":541,"context_line":"                logical_switch)"},{"line_number":542,"context_line":""},{"line_number":543,"context_line":"            self.ovn_provider_ls[logical_switch] \u003d {"},{"line_number":544,"context_line":"                \u0027bridge_device\u0027: bridge_dev,"},{"line_number":545,"context_line":"                \u0027bridge_vlan\u0027: bridge_vlan,"},{"line_number":546,"context_line":"                \u0027localnet\u0027: localnet"},{"line_number":547,"context_line":"            }"},{"line_number":548,"context_line":""},{"line_number":549,"context_line":"        ls \u003d self.ovn_provider_ls[logical_switch]"},{"line_number":550,"context_line":"        return ls[\u0027localnet\u0027], ls[\u0027bridge_device\u0027], ls[\u0027bridge_vlan\u0027]"}],"source_content_type":"text/x-python","patch_set":9,"id":"d4972a0c_34a918b5","line":547,"range":{"start_line":539,"start_character":0,"end_line":547,"end_character":13},"in_reply_to":"822a336d_4c138c1a","updated":"2024-01-24 07:47:26.000000000","message":"There is a fine balance in keeping method names short (because of the 79 char limit per line), and making them more understandable 😊, which is why i chose the short name `get_ls_info`. \n\nBut i\u0027m totally fine with renaming it to `get_provider_ls_info`","commit_id":"19fc96554d958ba9a50fb5916e7b9e1303b3c9f4"},{"author":{"_account_id":25468,"name":"Michel Nederlof","email":"michel@nederlof.info","username":"pellucid"},"change_message_id":"a447680027c2bc02ccfa9bc546cedc3887035643","unresolved":false,"context_lines":[{"line_number":536,"context_line":"        if logical_switch is None:"},{"line_number":537,"context_line":"            return None, None, None"},{"line_number":538,"context_line":""},{"line_number":539,"context_line":"        if logical_switch not in self.ovn_provider_ls:"},{"line_number":540,"context_line":"            localnet, bridge_dev, bridge_vlan \u003d self._get_ls_localnet_info("},{"line_number":541,"context_line":"                logical_switch)"},{"line_number":542,"context_line":""},{"line_number":543,"context_line":"            self.ovn_provider_ls[logical_switch] \u003d {"},{"line_number":544,"context_line":"                \u0027bridge_device\u0027: bridge_dev,"},{"line_number":545,"context_line":"                \u0027bridge_vlan\u0027: bridge_vlan,"},{"line_number":546,"context_line":"                \u0027localnet\u0027: localnet"},{"line_number":547,"context_line":"            }"},{"line_number":548,"context_line":""},{"line_number":549,"context_line":"        ls \u003d self.ovn_provider_ls[logical_switch]"},{"line_number":550,"context_line":"        return ls[\u0027localnet\u0027], ls[\u0027bridge_device\u0027], ls[\u0027bridge_vlan\u0027]"}],"source_content_type":"text/x-python","patch_set":9,"id":"17c1af69_a7199c71","line":547,"range":{"start_line":539,"start_character":0,"end_line":547,"end_character":13},"in_reply_to":"d4972a0c_34a918b5","updated":"2024-01-29 11:29:41.000000000","message":"Done","commit_id":"19fc96554d958ba9a50fb5916e7b9e1303b3c9f4"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"53d802ef9a5b8f2242f2a0d2eb13c8f30e146058","unresolved":true,"context_lines":[{"line_number":345,"context_line":"            if lb.external_ids.get(constants.OVN_LB_VIP_FIP_EXT_ID_KEY):"},{"line_number":346,"context_line":"                self._withdraw_ovn_lb_fip(lb)"},{"line_number":347,"context_line":""},{"line_number":348,"context_line":"    def is_ls_exposed(self, logical_switch):"},{"line_number":349,"context_line":"        # Check to see if given logical switch is accessible through the"},{"line_number":350,"context_line":"        # exposed provider networks."},{"line_number":351,"context_line":"        if logical_switch is None:"}],"source_content_type":"text/x-python","patch_set":14,"id":"a0ec45a9_89817b27","line":348,"range":{"start_line":348,"start_character":8,"end_line":348,"end_character":21},"updated":"2024-02-02 07:40:03.000000000","message":"nit: this is actually not checking if it is exposed, but if it is a provider LS and if it is configured, right? perhaps a better name could be something like: is_provider_ls_configured","commit_id":"a8956ee2c44087fc63e82a1cc53c650ea591430c"},{"author":{"_account_id":25468,"name":"Michel Nederlof","email":"michel@nederlof.info","username":"pellucid"},"change_message_id":"8911b053a5e61aef98a3a0b50a807b1e4c37f77b","unresolved":true,"context_lines":[{"line_number":345,"context_line":"            if lb.external_ids.get(constants.OVN_LB_VIP_FIP_EXT_ID_KEY):"},{"line_number":346,"context_line":"                self._withdraw_ovn_lb_fip(lb)"},{"line_number":347,"context_line":""},{"line_number":348,"context_line":"    def is_ls_exposed(self, logical_switch):"},{"line_number":349,"context_line":"        # Check to see if given logical switch is accessible through the"},{"line_number":350,"context_line":"        # exposed provider networks."},{"line_number":351,"context_line":"        if logical_switch is None:"}],"source_content_type":"text/x-python","patch_set":14,"id":"fcf75e1e_d7ad9070","line":348,"range":{"start_line":348,"start_character":8,"end_line":348,"end_character":21},"in_reply_to":"a0ec45a9_89817b27","updated":"2024-02-02 10:46:10.000000000","message":"i generally try to keep method names short, since we have a line character length, \nAlso, if i am not mistaken, we can also check if tenant networks have been exposed too, and if they are (through an lrp), it is not necessarily a provider network.","commit_id":"a8956ee2c44087fc63e82a1cc53c650ea591430c"},{"author":{"_account_id":25468,"name":"Michel Nederlof","email":"michel@nederlof.info","username":"pellucid"},"change_message_id":"a82260fe98ed3829309762b042849d4d3b9307b5","unresolved":false,"context_lines":[{"line_number":345,"context_line":"            if lb.external_ids.get(constants.OVN_LB_VIP_FIP_EXT_ID_KEY):"},{"line_number":346,"context_line":"                self._withdraw_ovn_lb_fip(lb)"},{"line_number":347,"context_line":""},{"line_number":348,"context_line":"    def is_ls_exposed(self, logical_switch):"},{"line_number":349,"context_line":"        # Check to see if given logical switch is accessible through the"},{"line_number":350,"context_line":"        # exposed provider networks."},{"line_number":351,"context_line":"        if logical_switch is None:"}],"source_content_type":"text/x-python","patch_set":14,"id":"95542126_603bf740","line":348,"range":{"start_line":348,"start_character":8,"end_line":348,"end_character":21},"in_reply_to":"fcf75e1e_d7ad9070","updated":"2024-02-22 10:47:56.000000000","message":"As discussed, will rename to `is_ls_provider`","commit_id":"a8956ee2c44087fc63e82a1cc53c650ea591430c"}],"ovn_bgp_agent/drivers/openstack/watchers/base_watcher.py":[{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"ede3c8ef4d8ccd84755d128565274fc860dab2c9","unresolved":true,"context_lines":[{"line_number":77,"context_line":""},{"line_number":78,"context_line":"    def _get_chassis(self, row):"},{"line_number":79,"context_line":"        # row.options[\u0027requested-chassis\u0027] superseeds the id in external_ids"},{"line_number":80,"context_line":"        if (hasattr(row, \u0027options\u0027) and"},{"line_number":81,"context_line":"                row.options.get(constants.OVN_REQUESTED_CHASSIS)):"},{"line_number":82,"context_line":""},{"line_number":83,"context_line":"            # requested-chassis can be a comma separated list,"}],"source_content_type":"text/x-python","patch_set":3,"id":"8182a83c_e6d367a5","line":80,"range":{"start_line":80,"start_character":1,"end_line":80,"end_character":39},"updated":"2024-01-23 06:59:36.000000000","message":"I remember there were some issues with virtual ports and options update, and that was the reason to prefer external_ids if available. could you double check this is not breaking virtual ports?","commit_id":"78852d8b3c0aeedeffbde0f38841c70587f1719e"},{"author":{"_account_id":25468,"name":"Michel Nederlof","email":"michel@nederlof.info","username":"pellucid"},"change_message_id":"c9474c7de624b7f390e8b5ed46ee8f93388a8897","unresolved":false,"context_lines":[{"line_number":77,"context_line":""},{"line_number":78,"context_line":"    def _get_chassis(self, row):"},{"line_number":79,"context_line":"        # row.options[\u0027requested-chassis\u0027] superseeds the id in external_ids"},{"line_number":80,"context_line":"        if (hasattr(row, \u0027options\u0027) and"},{"line_number":81,"context_line":"                row.options.get(constants.OVN_REQUESTED_CHASSIS)):"},{"line_number":82,"context_line":""},{"line_number":83,"context_line":"            # requested-chassis can be a comma separated list,"}],"source_content_type":"text/x-python","patch_set":3,"id":"54bb0d4e_01cf3dd3","line":80,"range":{"start_line":80,"start_character":1,"end_line":80,"end_character":39},"in_reply_to":"8182a83c_e6d367a5","updated":"2024-01-23 08:36:32.000000000","message":"When i check the virtual ports in our env, i see the following options:\n```\noptions             : {virtual-ip\u003d\"198.51.100.30\", virtual-parents\u003d\"1c5c0044-0e09-4ed8-aaca-d4fb134dcd8d,08836178-25ba-4e4e-8a90-b91e2d436459\"}\n```\n\nAnd in the external_ids field there is the neutron:host_id field set, so that should be fine.","commit_id":"78852d8b3c0aeedeffbde0f38841c70587f1719e"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"76a136cf54a810e8d0acfcf830ef30be1698e0f9","unresolved":true,"context_lines":[{"line_number":75,"context_line":"    def _check_ip_associated(self, mac):"},{"line_number":76,"context_line":"        return len(mac.strip().split(\u0027 \u0027)) \u003e 1"},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"    def _get_chassis(self, row):"},{"line_number":79,"context_line":"        # row.options[\u0027requested-chassis\u0027] superseeds the id in external_ids"},{"line_number":80,"context_line":"        if (hasattr(row, \u0027options\u0027) and"},{"line_number":81,"context_line":"                row.options.get(constants.OVN_REQUESTED_CHASSIS)):"}],"source_content_type":"text/x-python","patch_set":9,"id":"dae0b6d3_baeabe71","line":78,"range":{"start_line":78,"start_character":0,"end_line":78,"end_character":2},"updated":"2024-01-24 07:27:51.000000000","message":"this was added in https://review.opendev.org/c/openstack/ovn-bgp-agent/+/883187 to fix an issue with virtual ports. As the related patch in neutron was backported till wallaby (https://review.opendev.org/c/openstack/neutron/+/882705) I suppose we can assume there will be no requested chassis and this will go to the external_ids checking anyway, so it should be fine","commit_id":"19fc96554d958ba9a50fb5916e7b9e1303b3c9f4"},{"author":{"_account_id":25468,"name":"Michel Nederlof","email":"michel@nederlof.info","username":"pellucid"},"change_message_id":"a447680027c2bc02ccfa9bc546cedc3887035643","unresolved":false,"context_lines":[{"line_number":75,"context_line":"    def _check_ip_associated(self, mac):"},{"line_number":76,"context_line":"        return len(mac.strip().split(\u0027 \u0027)) \u003e 1"},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"    def _get_chassis(self, row):"},{"line_number":79,"context_line":"        # row.options[\u0027requested-chassis\u0027] superseeds the id in external_ids"},{"line_number":80,"context_line":"        if (hasattr(row, \u0027options\u0027) and"},{"line_number":81,"context_line":"                row.options.get(constants.OVN_REQUESTED_CHASSIS)):"}],"source_content_type":"text/x-python","patch_set":9,"id":"0162e460_7c34ab19","line":78,"range":{"start_line":78,"start_character":0,"end_line":78,"end_character":2},"in_reply_to":"974a6706_9827fa42","updated":"2024-01-29 11:29:41.000000000","message":"Done","commit_id":"19fc96554d958ba9a50fb5916e7b9e1303b3c9f4"},{"author":{"_account_id":25468,"name":"Michel Nederlof","email":"michel@nederlof.info","username":"pellucid"},"change_message_id":"25be53d646cd9ead6aee043ffd2cb1d45beb3c83","unresolved":true,"context_lines":[{"line_number":75,"context_line":"    def _check_ip_associated(self, mac):"},{"line_number":76,"context_line":"        return len(mac.strip().split(\u0027 \u0027)) \u003e 1"},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"    def _get_chassis(self, row):"},{"line_number":79,"context_line":"        # row.options[\u0027requested-chassis\u0027] superseeds the id in external_ids"},{"line_number":80,"context_line":"        if (hasattr(row, \u0027options\u0027) and"},{"line_number":81,"context_line":"                row.options.get(constants.OVN_REQUESTED_CHASSIS)):"}],"source_content_type":"text/x-python","patch_set":9,"id":"974a6706_9827fa42","line":78,"range":{"start_line":78,"start_character":0,"end_line":78,"end_character":2},"in_reply_to":"dae0b6d3_baeabe71","updated":"2024-01-24 07:47:26.000000000","message":"I also fixed an issue with placement for virtual ports [1], as the host_id was sometimes one failover step behind 😞.\n\nBut what we could do is completely ignore the row.options for virtual ports, in case a older environment will use the newer code? (so then virtual ports completely rely on the row.external_ids information from neutron, which is the case with NB anyway)\n\n[1] https://review.opendev.org/c/openstack/neutron/+/896883","commit_id":"19fc96554d958ba9a50fb5916e7b9e1303b3c9f4"}],"ovn_bgp_agent/drivers/openstack/watchers/nb_bgp_watcher.py":[{"author":{"_account_id":25468,"name":"Michel Nederlof","email":"michel@nederlof.info","username":"pellucid"},"change_message_id":"7b461c644e5536da9816545e14f43b732cd748a0","unresolved":true,"context_lines":[{"line_number":133,"context_line":"        return False"},{"line_number":134,"context_line":""},{"line_number":135,"context_line":"    def _run(self, event, row, old):"},{"line_number":136,"context_line":"        if row.type not in [constants.OVN_VM_VIF_PORT_TYPE,"},{"line_number":137,"context_line":"                            constants.OVN_VIRTUAL_VIF_PORT_TYPE]:"},{"line_number":138,"context_line":"            return"},{"line_number":139,"context_line":"        with _SYNC_STATE_LOCK.read_lock():"}],"source_content_type":"text/x-python","patch_set":3,"id":"d9003716_fb6d3793","side":"PARENT","line":136,"updated":"2024-01-22 17:11:57.000000000","message":"Why did we do this in the _run method, and not in the match_fn? (makes more sense to me to do it in the match_fn)?","commit_id":"2fcf3a4c48dd5ab315b2be34daf620670232de2b"},{"author":{"_account_id":25468,"name":"Michel Nederlof","email":"michel@nederlof.info","username":"pellucid"},"change_message_id":"c9474c7de624b7f390e8b5ed46ee8f93388a8897","unresolved":true,"context_lines":[{"line_number":133,"context_line":"        return False"},{"line_number":134,"context_line":""},{"line_number":135,"context_line":"    def _run(self, event, row, old):"},{"line_number":136,"context_line":"        if row.type not in [constants.OVN_VM_VIF_PORT_TYPE,"},{"line_number":137,"context_line":"                            constants.OVN_VIRTUAL_VIF_PORT_TYPE]:"},{"line_number":138,"context_line":"            return"},{"line_number":139,"context_line":"        with _SYNC_STATE_LOCK.read_lock():"}],"source_content_type":"text/x-python","patch_set":3,"id":"520717f8_b798e2f5","side":"PARENT","line":136,"in_reply_to":"1eefd469_eea3513a","updated":"2024-01-23 08:36:32.000000000","message":"nice, will check the other watcher events as well.","commit_id":"2fcf3a4c48dd5ab315b2be34daf620670232de2b"},{"author":{"_account_id":25468,"name":"Michel Nederlof","email":"michel@nederlof.info","username":"pellucid"},"change_message_id":"aafda7796a7a7579b61ebbd484f91971dcf271a7","unresolved":false,"context_lines":[{"line_number":133,"context_line":"        return False"},{"line_number":134,"context_line":""},{"line_number":135,"context_line":"    def _run(self, event, row, old):"},{"line_number":136,"context_line":"        if row.type not in [constants.OVN_VM_VIF_PORT_TYPE,"},{"line_number":137,"context_line":"                            constants.OVN_VIRTUAL_VIF_PORT_TYPE]:"},{"line_number":138,"context_line":"            return"},{"line_number":139,"context_line":"        with _SYNC_STATE_LOCK.read_lock():"}],"source_content_type":"text/x-python","patch_set":3,"id":"c6c36ab6_6d483f20","side":"PARENT","line":136,"in_reply_to":"520717f8_b798e2f5","updated":"2024-01-23 15:47:00.000000000","message":"moved.","commit_id":"2fcf3a4c48dd5ab315b2be34daf620670232de2b"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"ede3c8ef4d8ccd84755d128565274fc860dab2c9","unresolved":true,"context_lines":[{"line_number":133,"context_line":"        return False"},{"line_number":134,"context_line":""},{"line_number":135,"context_line":"    def _run(self, event, row, old):"},{"line_number":136,"context_line":"        if row.type not in [constants.OVN_VM_VIF_PORT_TYPE,"},{"line_number":137,"context_line":"                            constants.OVN_VIRTUAL_VIF_PORT_TYPE]:"},{"line_number":138,"context_line":"            return"},{"line_number":139,"context_line":"        with _SYNC_STATE_LOCK.read_lock():"}],"source_content_type":"text/x-python","patch_set":3,"id":"1eefd469_eea3513a","side":"PARENT","line":136,"in_reply_to":"d9003716_fb6d3793","updated":"2024-01-23 06:59:36.000000000","message":"you are right!","commit_id":"2fcf3a4c48dd5ab315b2be34daf620670232de2b"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"ede3c8ef4d8ccd84755d128565274fc860dab2c9","unresolved":true,"context_lines":[{"line_number":50,"context_line":"            # ROW_UPDATE EVENT"},{"line_number":51,"context_line":"            # Check if we are hosting this row, if not we can return False."},{"line_number":52,"context_line":"            ips \u003d row.addresses[0].split(\u0027 \u0027)[1:]"},{"line_number":53,"context_line":"            logical_switch \u003d row.external_ids.get("},{"line_number":54,"context_line":"                constants.OVN_LS_NAME_EXT_ID_KEY)"},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"            is_exported \u003d self.agent.is_ip_exposed(logical_switch, ips)"},{"line_number":57,"context_line":"            return not is_exported"}],"source_content_type":"text/x-python","patch_set":3,"id":"fcd60111_516e187c","line":54,"range":{"start_line":53,"start_character":0,"end_line":54,"end_character":49},"updated":"2024-01-23 06:59:36.000000000","message":"there is a chance the logical_switch is not yet set, we need to double check that","commit_id":"78852d8b3c0aeedeffbde0f38841c70587f1719e"},{"author":{"_account_id":25468,"name":"Michel Nederlof","email":"michel@nederlof.info","username":"pellucid"},"change_message_id":"aafda7796a7a7579b61ebbd484f91971dcf271a7","unresolved":false,"context_lines":[{"line_number":50,"context_line":"            # ROW_UPDATE EVENT"},{"line_number":51,"context_line":"            # Check if we are hosting this row, if not we can return False."},{"line_number":52,"context_line":"            ips \u003d row.addresses[0].split(\u0027 \u0027)[1:]"},{"line_number":53,"context_line":"            logical_switch \u003d row.external_ids.get("},{"line_number":54,"context_line":"                constants.OVN_LS_NAME_EXT_ID_KEY)"},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"            is_exported \u003d self.agent.is_ip_exposed(logical_switch, ips)"},{"line_number":57,"context_line":"            return not is_exported"}],"source_content_type":"text/x-python","patch_set":3,"id":"ea8e756f_fe3317e0","line":54,"range":{"start_line":53,"start_character":0,"end_line":54,"end_character":49},"in_reply_to":"6b620f82_5059f475","updated":"2024-01-23 15:47:00.000000000","message":"Done","commit_id":"78852d8b3c0aeedeffbde0f38841c70587f1719e"},{"author":{"_account_id":25468,"name":"Michel Nederlof","email":"michel@nederlof.info","username":"pellucid"},"change_message_id":"c9474c7de624b7f390e8b5ed46ee8f93388a8897","unresolved":true,"context_lines":[{"line_number":50,"context_line":"            # ROW_UPDATE EVENT"},{"line_number":51,"context_line":"            # Check if we are hosting this row, if not we can return False."},{"line_number":52,"context_line":"            ips \u003d row.addresses[0].split(\u0027 \u0027)[1:]"},{"line_number":53,"context_line":"            logical_switch \u003d row.external_ids.get("},{"line_number":54,"context_line":"                constants.OVN_LS_NAME_EXT_ID_KEY)"},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"            is_exported \u003d self.agent.is_ip_exposed(logical_switch, ips)"},{"line_number":57,"context_line":"            return not is_exported"}],"source_content_type":"text/x-python","patch_set":3,"id":"6b620f82_5059f475","line":54,"range":{"start_line":53,"start_character":0,"end_line":54,"end_character":49},"in_reply_to":"fcd60111_516e187c","updated":"2024-01-23 08:36:32.000000000","message":"Isn\u0027t this step one in neutron when creating a port?\novn create port [1] -\u003e get_external_ids_from_port [2]\n\n\nSo i\u0027d say we\u0027re covered there (eventhough the constants variable names are a slightly different).\n\n\n[1] https://opendev.org/openstack/neutron/src/commit/5ce17647c60448c9cae00ffc886d4e17a9fe0890/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py#L528\n[2] https://opendev.org/openstack/neutron/src/commit/5ce17647c60448c9cae00ffc886d4e17a9fe0890/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py#L513","commit_id":"78852d8b3c0aeedeffbde0f38841c70587f1719e"},{"author":{"_account_id":25468,"name":"Michel Nederlof","email":"michel@nederlof.info","username":"pellucid"},"change_message_id":"7b461c644e5536da9816545e14f43b732cd748a0","unresolved":true,"context_lines":[{"line_number":53,"context_line":"            logical_switch \u003d row.external_ids.get("},{"line_number":54,"context_line":"                constants.OVN_LS_NAME_EXT_ID_KEY)"},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"            is_exported \u003d self.agent.is_ip_exposed(logical_switch, ips)"},{"line_number":57,"context_line":"            return not is_exported"},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"        except (IndexError, AttributeError):"}],"source_content_type":"text/x-python","patch_set":3,"id":"209c77de_ba6f3557","line":56,"updated":"2024-01-22 17:11:57.000000000","message":"Makes more sense to me, to only expose the ip if we not already have done so. Otherwise in bigger envs we just end up eating resources we could use for other events, unless i\u0027m missing something here?","commit_id":"78852d8b3c0aeedeffbde0f38841c70587f1719e"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"ede3c8ef4d8ccd84755d128565274fc860dab2c9","unresolved":true,"context_lines":[{"line_number":53,"context_line":"            logical_switch \u003d row.external_ids.get("},{"line_number":54,"context_line":"                constants.OVN_LS_NAME_EXT_ID_KEY)"},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"            is_exported \u003d self.agent.is_ip_exposed(logical_switch, ips)"},{"line_number":57,"context_line":"            return not is_exported"},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"        except (IndexError, AttributeError):"}],"source_content_type":"text/x-python","patch_set":3,"id":"857deba0_a0523631","line":56,"in_reply_to":"209c77de_ba6f3557","updated":"2024-01-23 06:59:36.000000000","message":"I agree, this checking makes sense","commit_id":"78852d8b3c0aeedeffbde0f38841c70587f1719e"},{"author":{"_account_id":25468,"name":"Michel Nederlof","email":"michel@nederlof.info","username":"pellucid"},"change_message_id":"c9474c7de624b7f390e8b5ed46ee8f93388a8897","unresolved":true,"context_lines":[{"line_number":53,"context_line":"            logical_switch \u003d row.external_ids.get("},{"line_number":54,"context_line":"                constants.OVN_LS_NAME_EXT_ID_KEY)"},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"            is_exported \u003d self.agent.is_ip_exposed(logical_switch, ips)"},{"line_number":57,"context_line":"            return not is_exported"},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"        except (IndexError, AttributeError):"}],"source_content_type":"text/x-python","patch_set":3,"id":"ce0a3490_9801a414","line":56,"in_reply_to":"857deba0_a0523631","updated":"2024-01-23 08:36:32.000000000","message":"Ok, will check the other events for NB watcher as well.","commit_id":"78852d8b3c0aeedeffbde0f38841c70587f1719e"},{"author":{"_account_id":25468,"name":"Michel Nederlof","email":"michel@nederlof.info","username":"pellucid"},"change_message_id":"aafda7796a7a7579b61ebbd484f91971dcf271a7","unresolved":false,"context_lines":[{"line_number":53,"context_line":"            logical_switch \u003d row.external_ids.get("},{"line_number":54,"context_line":"                constants.OVN_LS_NAME_EXT_ID_KEY)"},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"            is_exported \u003d self.agent.is_ip_exposed(logical_switch, ips)"},{"line_number":57,"context_line":"            return not is_exported"},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"        except (IndexError, AttributeError):"}],"source_content_type":"text/x-python","patch_set":3,"id":"55872aca_2b5dfccc","line":56,"in_reply_to":"ce0a3490_9801a414","updated":"2024-01-23 15:47:00.000000000","message":"I\u0027ve checked the other LSP events, not sure if we should check all other events too, or just take that into another change?","commit_id":"78852d8b3c0aeedeffbde0f38841c70587f1719e"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"76a136cf54a810e8d0acfcf830ef30be1698e0f9","unresolved":true,"context_lines":[{"line_number":42,"context_line":"            current_chassis, _ \u003d self._get_chassis(row)"},{"line_number":43,"context_line":"            logical_switch \u003d self._get_network(row)"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"            # Do not run, if:"},{"line_number":46,"context_line":"            # 1. we do not own this lsp"},{"line_number":47,"context_line":"            # 2. port is down"},{"line_number":48,"context_line":"            # 3. logical switch is not exposed with agent"},{"line_number":49,"context_line":"            if (current_chassis !\u003d self.agent.chassis or"},{"line_number":50,"context_line":"                    not bool(row.up[0]) or"},{"line_number":51,"context_line":"                    not self.agent.is_ls_exposed(logical_switch)):"}],"source_content_type":"text/x-python","patch_set":9,"id":"e9d2889a_793d6df1","line":48,"range":{"start_line":45,"start_character":2,"end_line":48,"end_character":57},"updated":"2024-01-24 07:27:51.000000000","message":"I like the docstring format in L77-82, perhaps you can do the same here","commit_id":"19fc96554d958ba9a50fb5916e7b9e1303b3c9f4"},{"author":{"_account_id":25468,"name":"Michel Nederlof","email":"michel@nederlof.info","username":"pellucid"},"change_message_id":"a447680027c2bc02ccfa9bc546cedc3887035643","unresolved":false,"context_lines":[{"line_number":42,"context_line":"            current_chassis, _ \u003d self._get_chassis(row)"},{"line_number":43,"context_line":"            logical_switch \u003d self._get_network(row)"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"            # Do not run, if:"},{"line_number":46,"context_line":"            # 1. we do not own this lsp"},{"line_number":47,"context_line":"            # 2. port is down"},{"line_number":48,"context_line":"            # 3. logical switch is not exposed with agent"},{"line_number":49,"context_line":"            if (current_chassis !\u003d self.agent.chassis or"},{"line_number":50,"context_line":"                    not bool(row.up[0]) or"},{"line_number":51,"context_line":"                    not self.agent.is_ls_exposed(logical_switch)):"}],"source_content_type":"text/x-python","patch_set":9,"id":"46837fa7_3ba48d3b","line":48,"range":{"start_line":45,"start_character":2,"end_line":48,"end_character":57},"in_reply_to":"a39236ec_d4a5de2a","updated":"2024-01-29 11:29:41.000000000","message":"Done","commit_id":"19fc96554d958ba9a50fb5916e7b9e1303b3c9f4"},{"author":{"_account_id":25468,"name":"Michel Nederlof","email":"michel@nederlof.info","username":"pellucid"},"change_message_id":"25be53d646cd9ead6aee043ffd2cb1d45beb3c83","unresolved":true,"context_lines":[{"line_number":42,"context_line":"            current_chassis, _ \u003d self._get_chassis(row)"},{"line_number":43,"context_line":"            logical_switch \u003d self._get_network(row)"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"            # Do not run, if:"},{"line_number":46,"context_line":"            # 1. we do not own this lsp"},{"line_number":47,"context_line":"            # 2. port is down"},{"line_number":48,"context_line":"            # 3. logical switch is not exposed with agent"},{"line_number":49,"context_line":"            if (current_chassis !\u003d self.agent.chassis or"},{"line_number":50,"context_line":"                    not bool(row.up[0]) or"},{"line_number":51,"context_line":"                    not self.agent.is_ls_exposed(logical_switch)):"}],"source_content_type":"text/x-python","patch_set":9,"id":"a39236ec_d4a5de2a","line":48,"range":{"start_line":45,"start_character":2,"end_line":48,"end_character":57},"in_reply_to":"e9d2889a_793d6df1","updated":"2024-01-24 07:47:26.000000000","message":"will do!","commit_id":"19fc96554d958ba9a50fb5916e7b9e1303b3c9f4"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"53d802ef9a5b8f2242f2a0d2eb13c8f30e146058","unresolved":true,"context_lines":[{"line_number":170,"context_line":"                # Port changed up."},{"line_number":171,"context_line":"                return True"},{"line_number":172,"context_line":""},{"line_number":173,"context_line":"            old_port_fip \u003d getattr(old, \u0027external_ids\u0027, {}).get("},{"line_number":174,"context_line":"                constants.OVN_FIP_EXT_ID_KEY)"},{"line_number":175,"context_line":"            if old_port_fip \u003d\u003d current_port_fip:"},{"line_number":176,"context_line":"                # not a real new update for us"},{"line_number":177,"context_line":"                return False"},{"line_number":178,"context_line":""},{"line_number":179,"context_line":"            # Check if the current port_fip has been exposed"},{"line_number":180,"context_line":"            return not self.agent.is_ip_exposed(self._get_network(row),"},{"line_number":181,"context_line":"                                                current_port_fip)"}],"source_content_type":"text/x-python","patch_set":14,"id":"d7deb35f_dbed100e","line":178,"range":{"start_line":173,"start_character":0,"end_line":178,"end_character":1},"updated":"2024-02-02 07:40:03.000000000","message":"what if a port with a fip changed chassis (migration)? Could it be we reach this point, the external_ids is maintained, and we don\u0027t expose it due to FIP not being changed. Perhaps it is enough to check if the IP is exposed, right? (lines 179-182), as at this point we don\u0027t care about if the IP has changed or not, but about if we need to expose the current_port_fip","commit_id":"a8956ee2c44087fc63e82a1cc53c650ea591430c"},{"author":{"_account_id":25468,"name":"Michel Nederlof","email":"michel@nederlof.info","username":"pellucid"},"change_message_id":"a82260fe98ed3829309762b042849d4d3b9307b5","unresolved":false,"context_lines":[{"line_number":170,"context_line":"                # Port changed up."},{"line_number":171,"context_line":"                return True"},{"line_number":172,"context_line":""},{"line_number":173,"context_line":"            old_port_fip \u003d getattr(old, \u0027external_ids\u0027, {}).get("},{"line_number":174,"context_line":"                constants.OVN_FIP_EXT_ID_KEY)"},{"line_number":175,"context_line":"            if old_port_fip \u003d\u003d current_port_fip:"},{"line_number":176,"context_line":"                # not a real new update for us"},{"line_number":177,"context_line":"                return False"},{"line_number":178,"context_line":""},{"line_number":179,"context_line":"            # Check if the current port_fip has been exposed"},{"line_number":180,"context_line":"            return not self.agent.is_ip_exposed(self._get_network(row),"},{"line_number":181,"context_line":"                                                current_port_fip)"}],"source_content_type":"text/x-python","patch_set":14,"id":"59983ad3_1ce59618","line":178,"range":{"start_line":173,"start_character":0,"end_line":178,"end_character":1},"in_reply_to":"8dd4c3f7_af072ba9","updated":"2024-02-22 10:47:56.000000000","message":"I\u0027ve updated the docstring and comments to explain why 😊","commit_id":"a8956ee2c44087fc63e82a1cc53c650ea591430c"},{"author":{"_account_id":25468,"name":"Michel Nederlof","email":"michel@nederlof.info","username":"pellucid"},"change_message_id":"8911b053a5e61aef98a3a0b50a807b1e4c37f77b","unresolved":true,"context_lines":[{"line_number":170,"context_line":"                # Port changed up."},{"line_number":171,"context_line":"                return True"},{"line_number":172,"context_line":""},{"line_number":173,"context_line":"            old_port_fip \u003d getattr(old, \u0027external_ids\u0027, {}).get("},{"line_number":174,"context_line":"                constants.OVN_FIP_EXT_ID_KEY)"},{"line_number":175,"context_line":"            if old_port_fip \u003d\u003d current_port_fip:"},{"line_number":176,"context_line":"                # not a real new update for us"},{"line_number":177,"context_line":"                return False"},{"line_number":178,"context_line":""},{"line_number":179,"context_line":"            # Check if the current port_fip has been exposed"},{"line_number":180,"context_line":"            return not self.agent.is_ip_exposed(self._get_network(row),"},{"line_number":181,"context_line":"                                                current_port_fip)"}],"source_content_type":"text/x-python","patch_set":14,"id":"8dd4c3f7_af072ba9","line":178,"range":{"start_line":173,"start_character":0,"end_line":178,"end_character":1},"in_reply_to":"d7deb35f_dbed100e","updated":"2024-02-02 10:46:10.000000000","message":"Yes good point, i will test whether this happens or not (or why i included it, because it could have something to do with multiple events happening at once)","commit_id":"a8956ee2c44087fc63e82a1cc53c650ea591430c"}]}