)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"0a08de18c87621ce4b031cbf93f25b589e69e74c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"ee985350_e6c3d63c","updated":"2023-02-13 07:52:27.000000000","message":"I know this is WIP, but just some early feedback","commit_id":"4f67b13af3b220fdba2fff253bc7bbf7ce833b93"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"27450b7fc368c1f1a78f88bf9011dcc5d940510d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"e614ffc0_0a14c0e4","updated":"2023-02-14 13:39:46.000000000","message":"Overall looks good, just one concern, adding -1 just to ensure discussion on that point","commit_id":"9344881b0908a5bfae3f71bc220fbd943a1a90ae"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"26cde5b758acb1649ec022eb9f8747402fcf14f3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"f04092c7_fda73074","updated":"2023-02-15 07:19:20.000000000","message":"Should we also include some functional testing coverage? I checked and there is nothing for the HM already, so it is more than ok to leave it for a follow up patch","commit_id":"4bb1e426dbca6c1be57beb3056c46391dfb7d97e"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"4bf339c6925c1b91e3f5dbbee709d8e20b4471c3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"e1f6004d_860bc381","updated":"2023-02-16 07:21:59.000000000","message":"Looks good, just a couple of nits","commit_id":"824a1e19747989d59211db7f65d81de80af8c83c"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"ba4845911197dfba96df211b7669e2feb054884f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"4f47cee7_b4a73e43","updated":"2023-02-16 15:53:27.000000000","message":"Looks good! Lets see what gates","commit_id":"54d96ca07237dff8bccdaf4a9d5492f9103ea261"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"ac1a89edd90cb1c38d3ccd75529bce2e583c6fb9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"b374452d_6d2d3d30","updated":"2023-02-17 08:16:07.000000000","message":"Please, in a follow-up patch, add new FTs covering this new functionality.","commit_id":"54d96ca07237dff8bccdaf4a9d5492f9103ea261"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"28f7dda22a3360c6236e697f05646d0c868e9361","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"8f92e736_4f3e2992","updated":"2023-02-17 11:37:15.000000000","message":"recheck openstack-tox-docs unrelated","commit_id":"54d96ca07237dff8bccdaf4a9d5492f9103ea261"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"2bee6d110bfc6e2c9e3614d2e77f145b9c427ec0","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"21c6323b_4bb8b39e","updated":"2023-02-17 09:15:52.000000000","message":"recheck ovn-octavia-provider-tempest-release timeout","commit_id":"54d96ca07237dff8bccdaf4a9d5492f9103ea261"}],"ovn_octavia_provider/helper.py":[{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"0a08de18c87621ce4b031cbf93f25b589e69e74c","unresolved":true,"context_lines":[{"line_number":134,"context_line":"            raise idlutils.RowNotFound(table\u003drow._table.name,"},{"line_number":135,"context_line":"                                       col\u003dcol, match\u003dkey) from e"},{"line_number":136,"context_line":""},{"line_number":137,"context_line":"    def _create_hm_port(self, network_id, subnet_id, project_id):"},{"line_number":138,"context_line":"        port \u003d {\u0027port\u0027: {\u0027name\u0027: ovn_const.LB_HM_PORT_PREFIX + str(subnet_id),"},{"line_number":139,"context_line":"                         \u0027network_id\u0027: network_id,"},{"line_number":140,"context_line":"                         \u0027fixed_ips\u0027: [{\u0027subnet_id\u0027: subnet_id}],"}],"source_content_type":"text/x-python","patch_set":2,"id":"f9901004_f6c058a1","line":137,"range":{"start_line":137,"start_character":8,"end_line":137,"end_character":23},"updated":"2023-02-13 07:52:27.000000000","message":"the delete_port has a tenacity retry, should this also have it?","commit_id":"4f67b13af3b220fdba2fff253bc7bbf7ce833b93"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"9e098848efdfc68b6b8251bd548c26588c340e46","unresolved":false,"context_lines":[{"line_number":134,"context_line":"            raise idlutils.RowNotFound(table\u003drow._table.name,"},{"line_number":135,"context_line":"                                       col\u003dcol, match\u003dkey) from e"},{"line_number":136,"context_line":""},{"line_number":137,"context_line":"    def _create_hm_port(self, network_id, subnet_id, project_id):"},{"line_number":138,"context_line":"        port \u003d {\u0027port\u0027: {\u0027name\u0027: ovn_const.LB_HM_PORT_PREFIX + str(subnet_id),"},{"line_number":139,"context_line":"                         \u0027network_id\u0027: network_id,"},{"line_number":140,"context_line":"                         \u0027fixed_ips\u0027: [{\u0027subnet_id\u0027: subnet_id}],"}],"source_content_type":"text/x-python","patch_set":2,"id":"e0f1802d_d7512509","line":137,"range":{"start_line":137,"start_character":8,"end_line":137,"end_character":23},"in_reply_to":"0a652937_d38e09cd","updated":"2023-02-14 11:48:48.000000000","message":"Done","commit_id":"4f67b13af3b220fdba2fff253bc7bbf7ce833b93"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"25c95902ec84470964ba26e4d8db6264cf1e96ae","unresolved":true,"context_lines":[{"line_number":134,"context_line":"            raise idlutils.RowNotFound(table\u003drow._table.name,"},{"line_number":135,"context_line":"                                       col\u003dcol, match\u003dkey) from e"},{"line_number":136,"context_line":""},{"line_number":137,"context_line":"    def _create_hm_port(self, network_id, subnet_id, project_id):"},{"line_number":138,"context_line":"        port \u003d {\u0027port\u0027: {\u0027name\u0027: ovn_const.LB_HM_PORT_PREFIX + str(subnet_id),"},{"line_number":139,"context_line":"                         \u0027network_id\u0027: network_id,"},{"line_number":140,"context_line":"                         \u0027fixed_ips\u0027: [{\u0027subnet_id\u0027: subnet_id}],"}],"source_content_type":"text/x-python","patch_set":2,"id":"0a652937_d38e09cd","line":137,"range":{"start_line":137,"start_character":8,"end_line":137,"end_character":23},"in_reply_to":"e0985975_a5f38038","updated":"2023-02-13 10:36:43.000000000","message":"ohh, makes sense","commit_id":"4f67b13af3b220fdba2fff253bc7bbf7ce833b93"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"a681408440ad69cbc85d9224007f926e7165c3c4","unresolved":true,"context_lines":[{"line_number":134,"context_line":"            raise idlutils.RowNotFound(table\u003drow._table.name,"},{"line_number":135,"context_line":"                                       col\u003dcol, match\u003dkey) from e"},{"line_number":136,"context_line":""},{"line_number":137,"context_line":"    def _create_hm_port(self, network_id, subnet_id, project_id):"},{"line_number":138,"context_line":"        port \u003d {\u0027port\u0027: {\u0027name\u0027: ovn_const.LB_HM_PORT_PREFIX + str(subnet_id),"},{"line_number":139,"context_line":"                         \u0027network_id\u0027: network_id,"},{"line_number":140,"context_line":"                         \u0027fixed_ips\u0027: [{\u0027subnet_id\u0027: subnet_id}],"}],"source_content_type":"text/x-python","patch_set":2,"id":"e0985975_a5f38038","line":137,"range":{"start_line":137,"start_character":8,"end_line":137,"end_character":23},"in_reply_to":"f9901004_f6c058a1","updated":"2023-02-13 10:20:47.000000000","message":"Here I was following the same behaviour for the create_vip_port, if we are not able to create, we will update Octavia API that an error has occurred and the hm will not be created.","commit_id":"4f67b13af3b220fdba2fff253bc7bbf7ce833b93"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"0a08de18c87621ce4b031cbf93f25b589e69e74c","unresolved":true,"context_lines":[{"line_number":146,"context_line":"        try:"},{"line_number":147,"context_line":"            hm_port \u003d neutron_client.create_port(port)"},{"line_number":148,"context_line":"            return hm_port[\u0027port\u0027] if hm_port[\u0027port\u0027] else None"},{"line_number":149,"context_line":"        except n_exc.NeutronClientException:"},{"line_number":150,"context_line":"            # NOTE (froyo): whatever other exception as e.g. Timeout"},{"line_number":151,"context_line":"            # we should try to ensure no leftover port remains"},{"line_number":152,"context_line":"            ports \u003d neutron_client.list_ports("}],"source_content_type":"text/x-python","patch_set":2,"id":"42db27f1_ca0cfce5","line":149,"range":{"start_line":149,"start_character":6,"end_line":149,"end_character":44},"updated":"2023-02-13 07:52:27.000000000","message":"should we also handle the case the port got created in between (first member on the subnet, but belonging to another loadbalancer), so that we reuse it instead of removing it?","commit_id":"4f67b13af3b220fdba2fff253bc7bbf7ce833b93"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"a681408440ad69cbc85d9224007f926e7165c3c4","unresolved":true,"context_lines":[{"line_number":146,"context_line":"        try:"},{"line_number":147,"context_line":"            hm_port \u003d neutron_client.create_port(port)"},{"line_number":148,"context_line":"            return hm_port[\u0027port\u0027] if hm_port[\u0027port\u0027] else None"},{"line_number":149,"context_line":"        except n_exc.NeutronClientException:"},{"line_number":150,"context_line":"            # NOTE (froyo): whatever other exception as e.g. Timeout"},{"line_number":151,"context_line":"            # we should try to ensure no leftover port remains"},{"line_number":152,"context_line":"            ports \u003d neutron_client.list_ports("}],"source_content_type":"text/x-python","patch_set":2,"id":"83cef835_4da534e2","line":149,"range":{"start_line":149,"start_character":6,"end_line":149,"end_character":44},"in_reply_to":"42db27f1_ca0cfce5","updated":"2023-02-13 10:20:47.000000000","message":"If the port has already been created, we shouldn\u0027t get here, the _ensure_hm_ovn_port method will find the hm_port for the member\u0027s subnet in question and will not be called to create a new hm port.","commit_id":"4f67b13af3b220fdba2fff253bc7bbf7ce833b93"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"9e098848efdfc68b6b8251bd548c26588c340e46","unresolved":false,"context_lines":[{"line_number":146,"context_line":"        try:"},{"line_number":147,"context_line":"            hm_port \u003d neutron_client.create_port(port)"},{"line_number":148,"context_line":"            return hm_port[\u0027port\u0027] if hm_port[\u0027port\u0027] else None"},{"line_number":149,"context_line":"        except n_exc.NeutronClientException:"},{"line_number":150,"context_line":"            # NOTE (froyo): whatever other exception as e.g. Timeout"},{"line_number":151,"context_line":"            # we should try to ensure no leftover port remains"},{"line_number":152,"context_line":"            ports \u003d neutron_client.list_ports("}],"source_content_type":"text/x-python","patch_set":2,"id":"43f99912_5edcc3ec","line":149,"range":{"start_line":149,"start_character":6,"end_line":149,"end_character":44},"in_reply_to":"5ea11a8d_7b79ee60","updated":"2023-02-14 11:48:48.000000000","message":"In that case we will not have any exception, because we are trusting on neutron to assign IP so probably we will finish with two ports created 😕. I will try to reproduce that race condition and cover in another following patch, but atleast in this one we will be sure that no leftover ports remain.","commit_id":"4f67b13af3b220fdba2fff253bc7bbf7ce833b93"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"25c95902ec84470964ba26e4d8db6264cf1e96ae","unresolved":true,"context_lines":[{"line_number":146,"context_line":"        try:"},{"line_number":147,"context_line":"            hm_port \u003d neutron_client.create_port(port)"},{"line_number":148,"context_line":"            return hm_port[\u0027port\u0027] if hm_port[\u0027port\u0027] else None"},{"line_number":149,"context_line":"        except n_exc.NeutronClientException:"},{"line_number":150,"context_line":"            # NOTE (froyo): whatever other exception as e.g. Timeout"},{"line_number":151,"context_line":"            # we should try to ensure no leftover port remains"},{"line_number":152,"context_line":"            ports \u003d neutron_client.list_ports("}],"source_content_type":"text/x-python","patch_set":2,"id":"5ea11a8d_7b79ee60","line":149,"range":{"start_line":149,"start_character":6,"end_line":149,"end_character":44},"in_reply_to":"83cef835_4da534e2","updated":"2023-02-13 10:36:43.000000000","message":"not really, there may be races and when you list it is not there, but by the time you create it, it is already there. We have seen this in other points I think","commit_id":"4f67b13af3b220fdba2fff253bc7bbf7ce833b93"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"0a08de18c87621ce4b031cbf93f25b589e69e74c","unresolved":true,"context_lines":[{"line_number":165,"context_line":"        neutron_client \u003d clients.get_neutron_client()"},{"line_number":166,"context_line":"        hm_port_ip \u003d None"},{"line_number":167,"context_line":""},{"line_number":168,"context_line":"        hm_checks_port \u003d neutron_client.list_ports("},{"line_number":169,"context_line":"            name\u003df\u0027{ovn_const.LB_HM_PORT_PREFIX}{subnet_id}\u0027)"},{"line_number":170,"context_line":"        if hm_checks_port[\u0027ports\u0027]:"},{"line_number":171,"context_line":"            hm_port \u003d hm_checks_port[\u0027ports\u0027][0]"}],"source_content_type":"text/x-python","patch_set":2,"id":"41e922c9_6196ac1f","line":168,"range":{"start_line":168,"start_character":8,"end_line":168,"end_character":51},"updated":"2023-02-13 07:52:27.000000000","message":"should we ensure this does not fail to avoid leaks?","commit_id":"4f67b13af3b220fdba2fff253bc7bbf7ce833b93"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"a681408440ad69cbc85d9224007f926e7165c3c4","unresolved":true,"context_lines":[{"line_number":165,"context_line":"        neutron_client \u003d clients.get_neutron_client()"},{"line_number":166,"context_line":"        hm_port_ip \u003d None"},{"line_number":167,"context_line":""},{"line_number":168,"context_line":"        hm_checks_port \u003d neutron_client.list_ports("},{"line_number":169,"context_line":"            name\u003df\u0027{ovn_const.LB_HM_PORT_PREFIX}{subnet_id}\u0027)"},{"line_number":170,"context_line":"        if hm_checks_port[\u0027ports\u0027]:"},{"line_number":171,"context_line":"            hm_port \u003d hm_checks_port[\u0027ports\u0027][0]"}],"source_content_type":"text/x-python","patch_set":2,"id":"611b4a2c_7235b76c","line":168,"range":{"start_line":168,"start_character":8,"end_line":168,"end_character":51},"in_reply_to":"41e922c9_6196ac1f","updated":"2023-02-13 10:20:47.000000000","message":"Yes, here maybe it makes sense to tenacity retry, wdyt?","commit_id":"4f67b13af3b220fdba2fff253bc7bbf7ce833b93"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"25c95902ec84470964ba26e4d8db6264cf1e96ae","unresolved":true,"context_lines":[{"line_number":165,"context_line":"        neutron_client \u003d clients.get_neutron_client()"},{"line_number":166,"context_line":"        hm_port_ip \u003d None"},{"line_number":167,"context_line":""},{"line_number":168,"context_line":"        hm_checks_port \u003d neutron_client.list_ports("},{"line_number":169,"context_line":"            name\u003df\u0027{ovn_const.LB_HM_PORT_PREFIX}{subnet_id}\u0027)"},{"line_number":170,"context_line":"        if hm_checks_port[\u0027ports\u0027]:"},{"line_number":171,"context_line":"            hm_port \u003d hm_checks_port[\u0027ports\u0027][0]"}],"source_content_type":"text/x-python","patch_set":2,"id":"97722e26_10764119","line":168,"range":{"start_line":168,"start_character":8,"end_line":168,"end_character":51},"in_reply_to":"611b4a2c_7235b76c","updated":"2023-02-13 10:36:43.000000000","message":"agree, I think we had that for the deletion, we should have someting for the listing. But, perhaps you need to move it to a different method, not this one, as you don\u0027t want to have tenacity for the self.delete_port and then this doing it over with tenacity too","commit_id":"4f67b13af3b220fdba2fff253bc7bbf7ce833b93"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"9e098848efdfc68b6b8251bd548c26588c340e46","unresolved":false,"context_lines":[{"line_number":165,"context_line":"        neutron_client \u003d clients.get_neutron_client()"},{"line_number":166,"context_line":"        hm_port_ip \u003d None"},{"line_number":167,"context_line":""},{"line_number":168,"context_line":"        hm_checks_port \u003d neutron_client.list_ports("},{"line_number":169,"context_line":"            name\u003df\u0027{ovn_const.LB_HM_PORT_PREFIX}{subnet_id}\u0027)"},{"line_number":170,"context_line":"        if hm_checks_port[\u0027ports\u0027]:"},{"line_number":171,"context_line":"            hm_port \u003d hm_checks_port[\u0027ports\u0027][0]"}],"source_content_type":"text/x-python","patch_set":2,"id":"f8f894b0_69121549","line":168,"range":{"start_line":168,"start_character":8,"end_line":168,"end_character":51},"in_reply_to":"97722e26_10764119","updated":"2023-02-14 11:48:48.000000000","message":"Done","commit_id":"4f67b13af3b220fdba2fff253bc7bbf7ce833b93"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"0a08de18c87621ce4b031cbf93f25b589e69e74c","unresolved":true,"context_lines":[{"line_number":2645,"context_line":"        self._execute_commands(commands)"},{"line_number":2646,"context_line":""},{"line_number":2647,"context_line":"        # Delete the hm port if not in use by other healt monitors"},{"line_number":2648,"context_line":"        for subnet in member_subnets:"},{"line_number":2649,"context_line":"            self._delete_hm_port(subnet)"},{"line_number":2650,"context_line":""},{"line_number":2651,"context_line":"        status \u003d {"},{"line_number":2652,"context_line":"            constants.LOADBALANCERS: ["}],"source_content_type":"text/x-python","patch_set":2,"id":"27c6293f_8673f26c","line":2649,"range":{"start_line":2648,"start_character":1,"end_line":2649,"end_character":40},"updated":"2023-02-13 07:52:27.000000000","message":"what if there are other members in other unrelated loadbalancer pools? we should not remove the hm_port in that case","commit_id":"4f67b13af3b220fdba2fff253bc7bbf7ce833b93"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"25c95902ec84470964ba26e4d8db6264cf1e96ae","unresolved":true,"context_lines":[{"line_number":2645,"context_line":"        self._execute_commands(commands)"},{"line_number":2646,"context_line":""},{"line_number":2647,"context_line":"        # Delete the hm port if not in use by other healt monitors"},{"line_number":2648,"context_line":"        for subnet in member_subnets:"},{"line_number":2649,"context_line":"            self._delete_hm_port(subnet)"},{"line_number":2650,"context_line":""},{"line_number":2651,"context_line":"        status \u003d {"},{"line_number":2652,"context_line":"            constants.LOADBALANCERS: ["}],"source_content_type":"text/x-python","patch_set":2,"id":"5c23dd14_0de00b6c","line":2649,"range":{"start_line":2648,"start_character":1,"end_line":2649,"end_character":40},"in_reply_to":"0d7a044f_547b6867","updated":"2023-02-13 10:36:43.000000000","message":"oh, I missed that, great!","commit_id":"4f67b13af3b220fdba2fff253bc7bbf7ce833b93"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"a681408440ad69cbc85d9224007f926e7165c3c4","unresolved":true,"context_lines":[{"line_number":2645,"context_line":"        self._execute_commands(commands)"},{"line_number":2646,"context_line":""},{"line_number":2647,"context_line":"        # Delete the hm port if not in use by other healt monitors"},{"line_number":2648,"context_line":"        for subnet in member_subnets:"},{"line_number":2649,"context_line":"            self._delete_hm_port(subnet)"},{"line_number":2650,"context_line":""},{"line_number":2651,"context_line":"        status \u003d {"},{"line_number":2652,"context_line":"            constants.LOADBALANCERS: ["}],"source_content_type":"text/x-python","patch_set":2,"id":"0d7a044f_547b6867","line":2649,"range":{"start_line":2648,"start_character":1,"end_line":2649,"end_character":40},"in_reply_to":"27c6293f_8673f26c","updated":"2023-02-13 10:20:47.000000000","message":"the method _delete_hm_port takes care of that, this return [1] will only happen if we found other LB with the hm_port_ip in the ip_port_mappings.\n\n[1] https://review.opendev.org/c/openstack/ovn-octavia-provider/+/873426/2/ovn_octavia_provider/helper.py#182","commit_id":"4f67b13af3b220fdba2fff253bc7bbf7ce833b93"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"9e098848efdfc68b6b8251bd548c26588c340e46","unresolved":false,"context_lines":[{"line_number":2645,"context_line":"        self._execute_commands(commands)"},{"line_number":2646,"context_line":""},{"line_number":2647,"context_line":"        # Delete the hm port if not in use by other healt monitors"},{"line_number":2648,"context_line":"        for subnet in member_subnets:"},{"line_number":2649,"context_line":"            self._delete_hm_port(subnet)"},{"line_number":2650,"context_line":""},{"line_number":2651,"context_line":"        status \u003d {"},{"line_number":2652,"context_line":"            constants.LOADBALANCERS: ["}],"source_content_type":"text/x-python","patch_set":2,"id":"d480f3f7_e409c7b6","line":2649,"range":{"start_line":2648,"start_character":1,"end_line":2649,"end_character":40},"in_reply_to":"5c23dd14_0de00b6c","updated":"2023-02-14 11:48:48.000000000","message":"Done","commit_id":"4f67b13af3b220fdba2fff253bc7bbf7ce833b93"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"27450b7fc368c1f1a78f88bf9011dcc5d940510d","unresolved":true,"context_lines":[{"line_number":156,"context_line":"                port \u003d ports[\u0027ports\u0027][0]"},{"line_number":157,"context_line":"                LOG.debug(\u0027Leftover port %s has been found. Trying to \u0027"},{"line_number":158,"context_line":"                          \u0027delete it\u0027, port[\u0027id\u0027])"},{"line_number":159,"context_line":"                self.delete_port(port[\u0027id\u0027])"},{"line_number":160,"context_line":"            return None"},{"line_number":161,"context_line":""},{"line_number":162,"context_line":"    def _delete_hm_port(self, subnet_id):"}],"source_content_type":"text/x-python","patch_set":4,"id":"20f926a3_9264f4e7","line":159,"range":{"start_line":159,"start_character":0,"end_line":159,"end_character":44},"updated":"2023-02-14 13:39:46.000000000","message":"mix feelings about this... Is there a chance that the port got created for a different HM in this subnet and we are deleting one that it is actually used? should we call here _delete_hm_port instead to be on the safe side?","commit_id":"9344881b0908a5bfae3f71bc220fbd943a1a90ae"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"fce4aff645e1028afcfdd29c3f1b1486349482b8","unresolved":false,"context_lines":[{"line_number":156,"context_line":"                port \u003d ports[\u0027ports\u0027][0]"},{"line_number":157,"context_line":"                LOG.debug(\u0027Leftover port %s has been found. Trying to \u0027"},{"line_number":158,"context_line":"                          \u0027delete it\u0027, port[\u0027id\u0027])"},{"line_number":159,"context_line":"                self.delete_port(port[\u0027id\u0027])"},{"line_number":160,"context_line":"            return None"},{"line_number":161,"context_line":""},{"line_number":162,"context_line":"    def _delete_hm_port(self, subnet_id):"}],"source_content_type":"text/x-python","patch_set":4,"id":"beb88ba7_7d661b7b","line":159,"range":{"start_line":159,"start_character":0,"end_line":159,"end_character":44},"in_reply_to":"20f926a3_9264f4e7","updated":"2023-02-14 15:06:10.000000000","message":"Make sense, in order to reduce the possible corner case of two parallel create_hm_por for the same subnet, if hipotetically the second fails it will not delete directly the port","commit_id":"9344881b0908a5bfae3f71bc220fbd943a1a90ae"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"26cde5b758acb1649ec022eb9f8747402fcf14f3","unresolved":true,"context_lines":[{"line_number":149,"context_line":"        except n_exc.NeutronClientException:"},{"line_number":150,"context_line":"            # NOTE (froyo): whatever other exception as e.g. Timeout"},{"line_number":151,"context_line":"            # we should try to ensure no leftover port remains"},{"line_number":152,"context_line":"            ports \u003d self._neutron_list_ports(neutron_client, **{"},{"line_number":153,"context_line":"                \u0027network_id\u0027: network_id,"},{"line_number":154,"context_line":"                \u0027name\u0027: f\u0027{ovn_const.LB_HM_PORT_PREFIX}{subnet_id}\u0027})"},{"line_number":155,"context_line":"            if ports[\u0027ports\u0027]:"},{"line_number":156,"context_line":"                port \u003d ports[\u0027ports\u0027][0]"},{"line_number":157,"context_line":"                LOG.debug(\u0027Leftover port %s has been found. Trying to \u0027"},{"line_number":158,"context_line":"                          \u0027delete it\u0027, port[\u0027id\u0027])"},{"line_number":159,"context_line":"                self._delete_hm_port(subnet_id)"},{"line_number":160,"context_line":"            return None"},{"line_number":161,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"c0ca8b29_70beb518","line":158,"range":{"start_line":152,"start_character":0,"end_line":158,"end_character":50},"updated":"2023-02-15 07:19:20.000000000","message":"this is not needed, it is already done in the _delete_hm_port function","commit_id":"4bb1e426dbca6c1be57beb3056c46391dfb7d97e"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"8329ae11266271b07871394cd3616893b3d4e85d","unresolved":false,"context_lines":[{"line_number":149,"context_line":"        except n_exc.NeutronClientException:"},{"line_number":150,"context_line":"            # NOTE (froyo): whatever other exception as e.g. Timeout"},{"line_number":151,"context_line":"            # we should try to ensure no leftover port remains"},{"line_number":152,"context_line":"            ports \u003d self._neutron_list_ports(neutron_client, **{"},{"line_number":153,"context_line":"                \u0027network_id\u0027: network_id,"},{"line_number":154,"context_line":"                \u0027name\u0027: f\u0027{ovn_const.LB_HM_PORT_PREFIX}{subnet_id}\u0027})"},{"line_number":155,"context_line":"            if ports[\u0027ports\u0027]:"},{"line_number":156,"context_line":"                port \u003d ports[\u0027ports\u0027][0]"},{"line_number":157,"context_line":"                LOG.debug(\u0027Leftover port %s has been found. Trying to \u0027"},{"line_number":158,"context_line":"                          \u0027delete it\u0027, port[\u0027id\u0027])"},{"line_number":159,"context_line":"                self._delete_hm_port(subnet_id)"},{"line_number":160,"context_line":"            return None"},{"line_number":161,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"3ea797aa_185dd1f6","line":158,"range":{"start_line":152,"start_character":0,"end_line":158,"end_character":50},"in_reply_to":"c0ca8b29_70beb518","updated":"2023-02-15 08:46:33.000000000","message":"yeah, as we trust on _delete_hm_port for this check, good catch!","commit_id":"4bb1e426dbca6c1be57beb3056c46391dfb7d97e"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"26cde5b758acb1649ec022eb9f8747402fcf14f3","unresolved":true,"context_lines":[{"line_number":2615,"context_line":""},{"line_number":2616,"context_line":"        # Need to send pool info in status update to avoid immutable objects,"},{"line_number":2617,"context_line":"        # the LB should have this info. Also in order to delete the hm used for"},{"line_number":2618,"context_line":"        # health checks we need to get all subnets from the members on the pool"},{"line_number":2619,"context_line":"        pool_id \u003d None"},{"line_number":2620,"context_line":"        pool_listeners \u003d []"},{"line_number":2621,"context_line":"        member_subnets \u003d []"}],"source_content_type":"text/x-python","patch_set":5,"id":"aeef912b_ff0cab00","line":2618,"range":{"start_line":2618,"start_character":56,"end_line":2618,"end_character":79},"updated":"2023-02-15 07:19:20.000000000","message":"is from the members on this specific loadbalancer pool? This should check all the members in the subnet, regardless of the pool/loadbalancer they belong to","commit_id":"4bb1e426dbca6c1be57beb3056c46391dfb7d97e"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"8329ae11266271b07871394cd3616893b3d4e85d","unresolved":false,"context_lines":[{"line_number":2615,"context_line":""},{"line_number":2616,"context_line":"        # Need to send pool info in status update to avoid immutable objects,"},{"line_number":2617,"context_line":"        # the LB should have this info. Also in order to delete the hm used for"},{"line_number":2618,"context_line":"        # health checks we need to get all subnets from the members on the pool"},{"line_number":2619,"context_line":"        pool_id \u003d None"},{"line_number":2620,"context_line":"        pool_listeners \u003d []"},{"line_number":2621,"context_line":"        member_subnets \u003d []"}],"source_content_type":"text/x-python","patch_set":5,"id":"a693df82_992724bd","line":2618,"range":{"start_line":2618,"start_character":56,"end_line":2618,"end_character":79},"in_reply_to":"aeef912b_ff0cab00","updated":"2023-02-15 08:46:33.000000000","message":"Yeah, when a request of hm_delete is received we need to delete the hm port used for health checks packets (if any other LB is not using it), to do this we need to identify the subnet from the members associated to the pool where the HM belongs. It not neccesary to check other members from other pools or loadbalancer because _delete_hm_port will take care of it. I will add the word port to the comment that was lost.","commit_id":"4bb1e426dbca6c1be57beb3056c46391dfb7d97e"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"26cde5b758acb1649ec022eb9f8747402fcf14f3","unresolved":true,"context_lines":[{"line_number":2655,"context_line":"                                         lbhc.uuid))"},{"line_number":2656,"context_line":"        self._execute_commands(commands)"},{"line_number":2657,"context_line":""},{"line_number":2658,"context_line":"        # Delete the hm port if not in use by other healt monitors"},{"line_number":2659,"context_line":"        for subnet in member_subnets:"},{"line_number":2660,"context_line":"            self._delete_hm_port(subnet)"},{"line_number":2661,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"be1d9613_a4cd3732","line":2658,"range":{"start_line":2658,"start_character":52,"end_line":2658,"end_character":57},"updated":"2023-02-15 07:19:20.000000000","message":"super nit: health","commit_id":"4bb1e426dbca6c1be57beb3056c46391dfb7d97e"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"8329ae11266271b07871394cd3616893b3d4e85d","unresolved":false,"context_lines":[{"line_number":2655,"context_line":"                                         lbhc.uuid))"},{"line_number":2656,"context_line":"        self._execute_commands(commands)"},{"line_number":2657,"context_line":""},{"line_number":2658,"context_line":"        # Delete the hm port if not in use by other healt monitors"},{"line_number":2659,"context_line":"        for subnet in member_subnets:"},{"line_number":2660,"context_line":"            self._delete_hm_port(subnet)"},{"line_number":2661,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"c412d074_ef78984a","line":2658,"range":{"start_line":2658,"start_character":52,"end_line":2658,"end_character":57},"in_reply_to":"be1d9613_a4cd3732","updated":"2023-02-15 08:46:33.000000000","message":"Done","commit_id":"4bb1e426dbca6c1be57beb3056c46391dfb7d97e"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"4bf339c6925c1b91e3f5dbbee709d8e20b4471c3","unresolved":true,"context_lines":[{"line_number":1968,"context_line":"                    constants.OPERATING_STATUS: pool_status}"},{"line_number":1969,"context_line":"            if ovn_lb.health_check:"},{"line_number":1970,"context_line":"                self._update_hm_members(ovn_lb, pool_key)"},{"line_number":1971,"context_line":"                # NOTE(froyo): if the pool status is OFFLINE no more members"},{"line_number":1972,"context_line":"                # are there, so we should ensure the hm-port is deleted if no"},{"line_number":1973,"context_line":"                # more LB are using it. We need to do this call after the"},{"line_number":1974,"context_line":"                # cleaning of the ip_port_mappings for the ovn LB."},{"line_number":1975,"context_line":"                if pool_status \u003d\u003d constants.OFFLINE:"}],"source_content_type":"text/x-python","patch_set":9,"id":"3f766992_03fad2ea","line":1972,"range":{"start_line":1971,"start_character":31,"end_line":1972,"end_character":28},"updated":"2023-02-16 07:21:59.000000000","message":"supernit: if the pool status is OFFLINE there are no more members. So ...","commit_id":"824a1e19747989d59211db7f65d81de80af8c83c"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"6fc8b52412330767a59a469502566e3ed10ab569","unresolved":false,"context_lines":[{"line_number":1968,"context_line":"                    constants.OPERATING_STATUS: pool_status}"},{"line_number":1969,"context_line":"            if ovn_lb.health_check:"},{"line_number":1970,"context_line":"                self._update_hm_members(ovn_lb, pool_key)"},{"line_number":1971,"context_line":"                # NOTE(froyo): if the pool status is OFFLINE no more members"},{"line_number":1972,"context_line":"                # are there, so we should ensure the hm-port is deleted if no"},{"line_number":1973,"context_line":"                # more LB are using it. We need to do this call after the"},{"line_number":1974,"context_line":"                # cleaning of the ip_port_mappings for the ovn LB."},{"line_number":1975,"context_line":"                if pool_status \u003d\u003d constants.OFFLINE:"}],"source_content_type":"text/x-python","patch_set":9,"id":"08730fe0_b0330933","line":1972,"range":{"start_line":1971,"start_character":31,"end_line":1972,"end_character":28},"in_reply_to":"3f766992_03fad2ea","updated":"2023-02-16 10:22:08.000000000","message":"Done","commit_id":"824a1e19747989d59211db7f65d81de80af8c83c"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"4bf339c6925c1b91e3f5dbbee709d8e20b4471c3","unresolved":true,"context_lines":[{"line_number":2453,"context_line":""},{"line_number":2454,"context_line":"        commands \u003d []"},{"line_number":2455,"context_line":"        commands.append("},{"line_number":2456,"context_line":"            self.ovn_nbdb_api.db_clear(\u0027Load_Balancer\u0027, ovn_lb.uuid,"},{"line_number":2457,"context_line":"                                       \u0027ip_port_mappings\u0027))"},{"line_number":2458,"context_line":"        commands.append("},{"line_number":2459,"context_line":"            self.ovn_nbdb_api.db_set("},{"line_number":2460,"context_line":"                \u0027Load_Balancer\u0027, ovn_lb.uuid,"},{"line_number":2461,"context_line":"                (\u0027ip_port_mappings\u0027, mappings)))"}],"source_content_type":"text/x-python","patch_set":9,"id":"86626cb4_b4cdc3f5","line":2458,"range":{"start_line":2456,"start_character":0,"end_line":2458,"end_character":24},"updated":"2023-02-16 07:21:59.000000000","message":"perhaps leave a note about why the clear is needed","commit_id":"824a1e19747989d59211db7f65d81de80af8c83c"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"6fc8b52412330767a59a469502566e3ed10ab569","unresolved":false,"context_lines":[{"line_number":2453,"context_line":""},{"line_number":2454,"context_line":"        commands \u003d []"},{"line_number":2455,"context_line":"        commands.append("},{"line_number":2456,"context_line":"            self.ovn_nbdb_api.db_clear(\u0027Load_Balancer\u0027, ovn_lb.uuid,"},{"line_number":2457,"context_line":"                                       \u0027ip_port_mappings\u0027))"},{"line_number":2458,"context_line":"        commands.append("},{"line_number":2459,"context_line":"            self.ovn_nbdb_api.db_set("},{"line_number":2460,"context_line":"                \u0027Load_Balancer\u0027, ovn_lb.uuid,"},{"line_number":2461,"context_line":"                (\u0027ip_port_mappings\u0027, mappings)))"}],"source_content_type":"text/x-python","patch_set":9,"id":"522e393c_ef47870d","line":2458,"range":{"start_line":2456,"start_character":0,"end_line":2458,"end_character":24},"in_reply_to":"86626cb4_b4cdc3f5","updated":"2023-02-16 10:22:08.000000000","message":"Ok, indeed, also a TODO to move to the ovsdbapp commands to add/del backend members on ip_port_mappings field","commit_id":"824a1e19747989d59211db7f65d81de80af8c83c"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"4bf339c6925c1b91e3f5dbbee709d8e20b4471c3","unresolved":true,"context_lines":[{"line_number":2456,"context_line":"            self.ovn_nbdb_api.db_clear(\u0027Load_Balancer\u0027, ovn_lb.uuid,"},{"line_number":2457,"context_line":"                                       \u0027ip_port_mappings\u0027))"},{"line_number":2458,"context_line":"        commands.append("},{"line_number":2459,"context_line":"            self.ovn_nbdb_api.db_set("},{"line_number":2460,"context_line":"                \u0027Load_Balancer\u0027, ovn_lb.uuid,"},{"line_number":2461,"context_line":"                (\u0027ip_port_mappings\u0027, mappings)))"},{"line_number":2462,"context_line":"        self._execute_commands(commands)"},{"line_number":2463,"context_line":"        return True"},{"line_number":2464,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"5cde0b66_411aac7b","line":2461,"range":{"start_line":2459,"start_character":0,"end_line":2461,"end_character":48},"updated":"2023-02-16 07:21:59.000000000","message":"perhaps worth to check first if mappings is empty, as there will be no need to do the set in that case","commit_id":"824a1e19747989d59211db7f65d81de80af8c83c"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"6fc8b52412330767a59a469502566e3ed10ab569","unresolved":false,"context_lines":[{"line_number":2456,"context_line":"            self.ovn_nbdb_api.db_clear(\u0027Load_Balancer\u0027, ovn_lb.uuid,"},{"line_number":2457,"context_line":"                                       \u0027ip_port_mappings\u0027))"},{"line_number":2458,"context_line":"        commands.append("},{"line_number":2459,"context_line":"            self.ovn_nbdb_api.db_set("},{"line_number":2460,"context_line":"                \u0027Load_Balancer\u0027, ovn_lb.uuid,"},{"line_number":2461,"context_line":"                (\u0027ip_port_mappings\u0027, mappings)))"},{"line_number":2462,"context_line":"        self._execute_commands(commands)"},{"line_number":2463,"context_line":"        return True"},{"line_number":2464,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"4236cfd3_7511d6f0","line":2461,"range":{"start_line":2459,"start_character":0,"end_line":2461,"end_character":48},"in_reply_to":"5cde0b66_411aac7b","updated":"2023-02-16 10:22:08.000000000","message":"done, good catch!","commit_id":"824a1e19747989d59211db7f65d81de80af8c83c"}]}