)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"cff2b079faf35fdf8f67821f8de2eb1625939f0a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"edb75eb9_a6be137b","updated":"2022-11-18 15:19:52.000000000","message":"Manually tested the fix on single node devstack as follows:\n\n1) install latest with ovn21.09 that includes the offending ovn commit 3ae8470edc64 (\"I-P: Handle runtime data changes for pflow_output engine.\")\n2) revert neutron patch that renamed the metadata namespace for ovn agent: 00c7c0eeab73392eba6fd86c456b8c6f19592d7e [ovn] Add neutron network to metadata namespace names\n3) restart metadata agent\n4) start a vm\n5) curl http://169.254.169.254/latest/meta-data/ - it works\n6) unrevert neutron patch that renamed the metadata namespace\n7) restart metadata agent\n8) curl http://169.254.169.254/latest/meta-data/ - it fails\n\nwith the patch under review applied before step (7), step (8) succeeds.","commit_id":"430729dd1198b522170a72ca4c595ede2a44c203"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"02cb6a447c19c51f4ba70cba39ab224adf11837e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"46c2e564_d13f925c","updated":"2022-11-17 18:08:33.000000000","message":"The code looks good to me.","commit_id":"430729dd1198b522170a72ca4c595ede2a44c203"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"edefc6c6453e4d95713b8c401e706841849db935","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"8ab65ce9_5509283c","updated":"2022-11-18 15:20:37.000000000","message":"need to report lp bug and update commit message to refer to it","commit_id":"430729dd1198b522170a72ca4c595ede2a44c203"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"94d5f8cdb8566395fa0fe46746065b0f3a9103dc","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"297771b8_115dc585","in_reply_to":"edb75eb9_a6be137b","updated":"2022-11-18 15:27:02.000000000","message":"Step (2) should be fixed to refer to: e4fb06b24299a7ecf10b05ef6ddc2d883c40e5a1","commit_id":"430729dd1198b522170a72ca4c595ede2a44c203"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"3b41c3548ae02e5009c21591db74b1413321e1fc","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"ea488427_b5b4d1b9","updated":"2022-11-18 21:11:45.000000000","message":"Guess this should be fine since VMs should retry metadata if it\u0027s unresponsive on initial request.","commit_id":"3093aaab13dd6ba04ef0e686eb4c6cc386c58941"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"e1bd796cd56adb56daac636f2702d09d73a8db1c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"10d1b70b_4fa98090","in_reply_to":"dab78887_a1759b4d","updated":"2022-11-18 21:37:37.000000000","message":"Ihar - yes, I\u0027d assume that\u0027s the reason it was done the way it was, to supposedly have the metadata IP reachable at all times. I don\u0027t think this makes it worse, since it was unreachable during that time as the bug states. I don\u0027t think it needs more refactoring to reduce the time based on what I know of cloud-init requests.","commit_id":"3093aaab13dd6ba04ef0e686eb4c6cc386c58941"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"288feb303e5f2644e2850d93af1abd2f64105535","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"dab78887_a1759b4d","in_reply_to":"ea488427_b5b4d1b9","updated":"2022-11-18 21:17:41.000000000","message":"Could you please clarify? Are you concerned about the time when metadata service won\u0027t be available because we have to clean up all the namespaces first, then create new ones? If that\u0027s a concern, we may have to cleanup-then-create-new one by one, to shorten the time between cleanup and create-new phases for all ports. Let me know if that\u0027s what you meant.","commit_id":"3093aaab13dd6ba04ef0e686eb4c6cc386c58941"}],"neutron/agent/ovn/metadata/agent.py":[{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"600c20995447051cfdd865a7f6abefd2cda3d6a5","unresolved":true,"context_lines":[{"line_number":330,"context_line":"        which were serving metadata but are no longer needed."},{"line_number":331,"context_line":"        \"\"\""},{"line_number":332,"context_line":""},{"line_number":333,"context_line":"        # first, clean up namespaces that should no longer deploy"},{"line_number":334,"context_line":"        system_namespaces \u003d tuple("},{"line_number":335,"context_line":"            ns.decode(\u0027utf-8\u0027) if isinstance(ns, bytes) else ns"},{"line_number":336,"context_line":"            for ns in ip_lib.list_network_namespaces())"},{"line_number":337,"context_line":"        nets \u003d self.get_networks()"},{"line_number":338,"context_line":"        metadata_namespaces \u003d ["},{"line_number":339,"context_line":"            self._get_namespace_name(net[1])"},{"line_number":340,"context_line":"            for net in nets"},{"line_number":341,"context_line":"        ]"},{"line_number":342,"context_line":"        unused_namespaces \u003d [ns for ns in system_namespaces if"},{"line_number":343,"context_line":"                             ns.startswith(NS_PREFIX) and"},{"line_number":344,"context_line":"                             ns not in metadata_namespaces]"},{"line_number":345,"context_line":"        for ns in unused_namespaces:"},{"line_number":346,"context_line":"            self.teardown_datapath(self._get_datapath_name(ns))"},{"line_number":347,"context_line":""},{"line_number":348,"context_line":"        # now that all obsolete namespaces are cleaned up, deploy required"},{"line_number":349,"context_line":"        # networks"}],"source_content_type":"text/x-python","patch_set":1,"id":"be7ef90e_c6572ac7","line":346,"range":{"start_line":333,"start_character":0,"end_line":346,"end_character":63},"updated":"2022-11-16 19:12:44.000000000","message":"Just thinking out loud - instead of doing cleanup per each sync() call, wouldn\u0027t it be better to do it once after agent starts? Somewhere under L208","commit_id":"430729dd1198b522170a72ca4c595ede2a44c203"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"68ddfd4af8928fff735eca88102530c829af120c","unresolved":true,"context_lines":[{"line_number":330,"context_line":"        which were serving metadata but are no longer needed."},{"line_number":331,"context_line":"        \"\"\""},{"line_number":332,"context_line":""},{"line_number":333,"context_line":"        # first, clean up namespaces that should no longer deploy"},{"line_number":334,"context_line":"        system_namespaces \u003d tuple("},{"line_number":335,"context_line":"            ns.decode(\u0027utf-8\u0027) if isinstance(ns, bytes) else ns"},{"line_number":336,"context_line":"            for ns in ip_lib.list_network_namespaces())"},{"line_number":337,"context_line":"        nets \u003d self.get_networks()"},{"line_number":338,"context_line":"        metadata_namespaces \u003d ["},{"line_number":339,"context_line":"            self._get_namespace_name(net[1])"},{"line_number":340,"context_line":"            for net in nets"},{"line_number":341,"context_line":"        ]"},{"line_number":342,"context_line":"        unused_namespaces \u003d [ns for ns in system_namespaces if"},{"line_number":343,"context_line":"                             ns.startswith(NS_PREFIX) and"},{"line_number":344,"context_line":"                             ns not in metadata_namespaces]"},{"line_number":345,"context_line":"        for ns in unused_namespaces:"},{"line_number":346,"context_line":"            self.teardown_datapath(self._get_datapath_name(ns))"},{"line_number":347,"context_line":""},{"line_number":348,"context_line":"        # now that all obsolete namespaces are cleaned up, deploy required"},{"line_number":349,"context_line":"        # networks"}],"source_content_type":"text/x-python","patch_set":1,"id":"393c8670_715c1924","line":346,"range":{"start_line":333,"start_character":0,"end_line":346,"end_character":63},"in_reply_to":"12287f72_8f651191","updated":"2022-11-16 20:23:26.000000000","message":"But you\u0027re right, looks like it\u0027s intentional based on the comment on L330.","commit_id":"430729dd1198b522170a72ca4c595ede2a44c203"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"739b272d9a5fd9dd6e91a8d843a2ba4581dc0f08","unresolved":true,"context_lines":[{"line_number":330,"context_line":"        which were serving metadata but are no longer needed."},{"line_number":331,"context_line":"        \"\"\""},{"line_number":332,"context_line":""},{"line_number":333,"context_line":"        # first, clean up namespaces that should no longer deploy"},{"line_number":334,"context_line":"        system_namespaces \u003d tuple("},{"line_number":335,"context_line":"            ns.decode(\u0027utf-8\u0027) if isinstance(ns, bytes) else ns"},{"line_number":336,"context_line":"            for ns in ip_lib.list_network_namespaces())"},{"line_number":337,"context_line":"        nets \u003d self.get_networks()"},{"line_number":338,"context_line":"        metadata_namespaces \u003d ["},{"line_number":339,"context_line":"            self._get_namespace_name(net[1])"},{"line_number":340,"context_line":"            for net in nets"},{"line_number":341,"context_line":"        ]"},{"line_number":342,"context_line":"        unused_namespaces \u003d [ns for ns in system_namespaces if"},{"line_number":343,"context_line":"                             ns.startswith(NS_PREFIX) and"},{"line_number":344,"context_line":"                             ns not in metadata_namespaces]"},{"line_number":345,"context_line":"        for ns in unused_namespaces:"},{"line_number":346,"context_line":"            self.teardown_datapath(self._get_datapath_name(ns))"},{"line_number":347,"context_line":""},{"line_number":348,"context_line":"        # now that all obsolete namespaces are cleaned up, deploy required"},{"line_number":349,"context_line":"        # networks"}],"source_content_type":"text/x-python","patch_set":1,"id":"12287f72_8f651191","line":346,"range":{"start_line":333,"start_character":0,"end_line":346,"end_character":63},"in_reply_to":"17d818e9_ebcef53a","updated":"2022-11-16 20:21:09.000000000","message":"The vif removal is done by handling an event for it - L115, that calls update_datapath() - L393, isn\u0027t it? The sync is called only on chassis events from resync() is call.","commit_id":"430729dd1198b522170a72ca4c595ede2a44c203"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"621feca1fe68d30a71004d405f75f63c350f065f","unresolved":true,"context_lines":[{"line_number":330,"context_line":"        which were serving metadata but are no longer needed."},{"line_number":331,"context_line":"        \"\"\""},{"line_number":332,"context_line":""},{"line_number":333,"context_line":"        # first, clean up namespaces that should no longer deploy"},{"line_number":334,"context_line":"        system_namespaces \u003d tuple("},{"line_number":335,"context_line":"            ns.decode(\u0027utf-8\u0027) if isinstance(ns, bytes) else ns"},{"line_number":336,"context_line":"            for ns in ip_lib.list_network_namespaces())"},{"line_number":337,"context_line":"        nets \u003d self.get_networks()"},{"line_number":338,"context_line":"        metadata_namespaces \u003d ["},{"line_number":339,"context_line":"            self._get_namespace_name(net[1])"},{"line_number":340,"context_line":"            for net in nets"},{"line_number":341,"context_line":"        ]"},{"line_number":342,"context_line":"        unused_namespaces \u003d [ns for ns in system_namespaces if"},{"line_number":343,"context_line":"                             ns.startswith(NS_PREFIX) and"},{"line_number":344,"context_line":"                             ns not in metadata_namespaces]"},{"line_number":345,"context_line":"        for ns in unused_namespaces:"},{"line_number":346,"context_line":"            self.teardown_datapath(self._get_datapath_name(ns))"},{"line_number":347,"context_line":""},{"line_number":348,"context_line":"        # now that all obsolete namespaces are cleaned up, deploy required"},{"line_number":349,"context_line":"        # networks"}],"source_content_type":"text/x-python","patch_set":1,"id":"17d818e9_ebcef53a","line":346,"range":{"start_line":333,"start_character":0,"end_line":346,"end_character":63},"in_reply_to":"241b00c4_9f18da02","updated":"2022-11-16 19:51:28.000000000","message":"Actually, I think doing it periodically is intentional. When a vif is is removed, and there are no other ports for the datapath, get_networks() will not return the network and we will need to clean the \"new style\" namespace. This cleanup code handles both the \"old style named\" namespaces as well as namespaces that belong to datapaths that are no longer of interest. The latter should be taken care of periodically, not just on agent start.","commit_id":"430729dd1198b522170a72ca4c595ede2a44c203"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"bd8060dcea28416bedca27a619e0592ab23c3a94","unresolved":true,"context_lines":[{"line_number":330,"context_line":"        which were serving metadata but are no longer needed."},{"line_number":331,"context_line":"        \"\"\""},{"line_number":332,"context_line":""},{"line_number":333,"context_line":"        # first, clean up namespaces that should no longer deploy"},{"line_number":334,"context_line":"        system_namespaces \u003d tuple("},{"line_number":335,"context_line":"            ns.decode(\u0027utf-8\u0027) if isinstance(ns, bytes) else ns"},{"line_number":336,"context_line":"            for ns in ip_lib.list_network_namespaces())"},{"line_number":337,"context_line":"        nets \u003d self.get_networks()"},{"line_number":338,"context_line":"        metadata_namespaces \u003d ["},{"line_number":339,"context_line":"            self._get_namespace_name(net[1])"},{"line_number":340,"context_line":"            for net in nets"},{"line_number":341,"context_line":"        ]"},{"line_number":342,"context_line":"        unused_namespaces \u003d [ns for ns in system_namespaces if"},{"line_number":343,"context_line":"                             ns.startswith(NS_PREFIX) and"},{"line_number":344,"context_line":"                             ns not in metadata_namespaces]"},{"line_number":345,"context_line":"        for ns in unused_namespaces:"},{"line_number":346,"context_line":"            self.teardown_datapath(self._get_datapath_name(ns))"},{"line_number":347,"context_line":""},{"line_number":348,"context_line":"        # now that all obsolete namespaces are cleaned up, deploy required"},{"line_number":349,"context_line":"        # networks"}],"source_content_type":"text/x-python","patch_set":1,"id":"3c3f4d03_477987aa","line":346,"range":{"start_line":333,"start_character":0,"end_line":346,"end_character":63},"in_reply_to":"393c8670_715c1924","updated":"2022-11-16 21:05:53.000000000","message":"Oh interesting.\n\nIt may be that there is space for improvement here if we are sure that there\u0027s no other way for a no longer needed netns to happen except through PortBindingChassisDeletedEvent. I\u0027m just not sure.","commit_id":"430729dd1198b522170a72ca4c595ede2a44c203"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"c7db89f70fb0f6964d583b31a932878c0fd21d70","unresolved":true,"context_lines":[{"line_number":330,"context_line":"        which were serving metadata but are no longer needed."},{"line_number":331,"context_line":"        \"\"\""},{"line_number":332,"context_line":""},{"line_number":333,"context_line":"        # first, clean up namespaces that should no longer deploy"},{"line_number":334,"context_line":"        system_namespaces \u003d tuple("},{"line_number":335,"context_line":"            ns.decode(\u0027utf-8\u0027) if isinstance(ns, bytes) else ns"},{"line_number":336,"context_line":"            for ns in ip_lib.list_network_namespaces())"},{"line_number":337,"context_line":"        nets \u003d self.get_networks()"},{"line_number":338,"context_line":"        metadata_namespaces \u003d ["},{"line_number":339,"context_line":"            self._get_namespace_name(net[1])"},{"line_number":340,"context_line":"            for net in nets"},{"line_number":341,"context_line":"        ]"},{"line_number":342,"context_line":"        unused_namespaces \u003d [ns for ns in system_namespaces if"},{"line_number":343,"context_line":"                             ns.startswith(NS_PREFIX) and"},{"line_number":344,"context_line":"                             ns not in metadata_namespaces]"},{"line_number":345,"context_line":"        for ns in unused_namespaces:"},{"line_number":346,"context_line":"            self.teardown_datapath(self._get_datapath_name(ns))"},{"line_number":347,"context_line":""},{"line_number":348,"context_line":"        # now that all obsolete namespaces are cleaned up, deploy required"},{"line_number":349,"context_line":"        # networks"}],"source_content_type":"text/x-python","patch_set":1,"id":"241b00c4_9f18da02","line":346,"range":{"start_line":333,"start_character":0,"end_line":346,"end_character":63},"in_reply_to":"a9907821_ae7009bb","updated":"2022-11-16 19:26:31.000000000","message":"Depends if we do want to do it in two patches. This patch moves the time we cleanup old-style name resources. If we do it before ensure_all_networks_provisioned() or on startup is imho the same. I\u0027m fine with both approaches, one patch sounds simpler to me.","commit_id":"430729dd1198b522170a72ca4c595ede2a44c203"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"b2626656e4bf6bef3036243998454d8a022a0d6a","unresolved":true,"context_lines":[{"line_number":330,"context_line":"        which were serving metadata but are no longer needed."},{"line_number":331,"context_line":"        \"\"\""},{"line_number":332,"context_line":""},{"line_number":333,"context_line":"        # first, clean up namespaces that should no longer deploy"},{"line_number":334,"context_line":"        system_namespaces \u003d tuple("},{"line_number":335,"context_line":"            ns.decode(\u0027utf-8\u0027) if isinstance(ns, bytes) else ns"},{"line_number":336,"context_line":"            for ns in ip_lib.list_network_namespaces())"},{"line_number":337,"context_line":"        nets \u003d self.get_networks()"},{"line_number":338,"context_line":"        metadata_namespaces \u003d ["},{"line_number":339,"context_line":"            self._get_namespace_name(net[1])"},{"line_number":340,"context_line":"            for net in nets"},{"line_number":341,"context_line":"        ]"},{"line_number":342,"context_line":"        unused_namespaces \u003d [ns for ns in system_namespaces if"},{"line_number":343,"context_line":"                             ns.startswith(NS_PREFIX) and"},{"line_number":344,"context_line":"                             ns not in metadata_namespaces]"},{"line_number":345,"context_line":"        for ns in unused_namespaces:"},{"line_number":346,"context_line":"            self.teardown_datapath(self._get_datapath_name(ns))"},{"line_number":347,"context_line":""},{"line_number":348,"context_line":"        # now that all obsolete namespaces are cleaned up, deploy required"},{"line_number":349,"context_line":"        # networks"}],"source_content_type":"text/x-python","patch_set":1,"id":"a9907821_ae7009bb","line":346,"range":{"start_line":333,"start_character":0,"end_line":346,"end_character":63},"in_reply_to":"be7ef90e_c6572ac7","updated":"2022-11-16 19:19:15.000000000","message":"Yes but also, wouldn\u0027t this go in a separate patch since we already do it now?","commit_id":"430729dd1198b522170a72ca4c595ede2a44c203"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"3b41c3548ae02e5009c21591db74b1413321e1fc","unresolved":true,"context_lines":[{"line_number":315,"context_line":"                        \"br-int instead.\")"},{"line_number":316,"context_line":"            return \u0027br-int\u0027"},{"line_number":317,"context_line":""},{"line_number":318,"context_line":"    def get_networks(self):"},{"line_number":319,"context_line":"        ports \u003d self.sb_idl.get_ports_on_chassis(self.chassis)"},{"line_number":320,"context_line":"        return {(str(p.datapath.uuid),"},{"line_number":321,"context_line":"            ovn_utils.get_network_name_from_datapath(p.datapath))"}],"source_content_type":"text/x-python","patch_set":2,"id":"d32c85d0_002d78f2","line":318,"updated":"2022-11-18 21:11:45.000000000","message":"nit: no docstring comment, but it\u0027s obvious what it returns :)","commit_id":"3093aaab13dd6ba04ef0e686eb4c6cc386c58941"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"0a2f0a507f95b013ada5c36b0c48e6a5ea9386a3","unresolved":false,"context_lines":[{"line_number":315,"context_line":"                        \"br-int instead.\")"},{"line_number":316,"context_line":"            return \u0027br-int\u0027"},{"line_number":317,"context_line":""},{"line_number":318,"context_line":"    def get_networks(self):"},{"line_number":319,"context_line":"        ports \u003d self.sb_idl.get_ports_on_chassis(self.chassis)"},{"line_number":320,"context_line":"        return {(str(p.datapath.uuid),"},{"line_number":321,"context_line":"            ovn_utils.get_network_name_from_datapath(p.datapath))"}],"source_content_type":"text/x-python","patch_set":2,"id":"a1769cd9_e69d11db","line":318,"in_reply_to":"d32c85d0_002d78f2","updated":"2022-11-18 21:19:42.000000000","message":"Ack","commit_id":"3093aaab13dd6ba04ef0e686eb4c6cc386c58941"}]}