)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"d678ff6a65c89e0c9c492caf5d79807a63bbc57a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"77071a44_b5697b42","updated":"2023-06-09 12:59:54.000000000","message":"Nice one! I know it is WIP but I did a first round and wanted to highlight (besides the nits here and there) one important thing related to FIP to VIP association. In a multiVIP loadbalancer, there may be different FIPs associated to the VIPs, and each FIP will only be associated to one VIP.\n\nThen, related to the LB_HC, there should be one per VIP, plus one per FIP","commit_id":"69b283fd7d81cbbeb19c0ccfb5e208b46f9d9ca5"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"ba9ad49a9163362d27e4bce0a2b005ab7df8b0db","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"a86e1723_92782056","updated":"2023-06-19 07:08:10.000000000","message":"This requires a launchpad bug and release notes. Also documentation","commit_id":"e430ec987d6367dce8d6f86cf3a7f01453b23108"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"0a77d4c822a8d257fa9ef484a45532160676ed50","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":13,"id":"5ee9e762_18b86dfc","updated":"2023-07-19 08:24:05.000000000","message":"recheck ovn-octavia-provider-tempest-release LB inmutable","commit_id":"50cef542b969be159dae41e81c84a933bc1bbdc6"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"5323f4d4fa9e1f1ea6a390d28e4be59b3d2afe99","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":15,"id":"cc52bf08_028c38e9","updated":"2023-08-21 14:29:51.000000000","message":"couple of nits, other than that it looks good, but I should have an extra look to the testing side","commit_id":"64e307f2bda8e1d51e7a35b0c983382d618097ec"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"9a19e68d29aa3103ab84bbbf7ab7763a3e4a1a59","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":15,"id":"b6621bf5_f3900401","updated":"2023-08-14 09:48:14.000000000","message":"recheck ovn-octavia-provider-functional-master timeout","commit_id":"64e307f2bda8e1d51e7a35b0c983382d618097ec"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"44995e3b50a910a340e46beafaf579fc842f6293","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":18,"id":"ad55f310_3eef26a5","updated":"2023-08-28 06:46:14.000000000","message":"LGTM, just left there a couple of nits. BTW, did you check if there is any octavia tempest test covering this?","commit_id":"6fd64bd732210c6e7cd53618391a59cc038b15a6"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"6998ae5ebfbe11448cd9b3053844dd96cbae6b6c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":18,"id":"85460f7c_ea8b1840","in_reply_to":"ad55f310_3eef26a5","updated":"2023-08-28 10:01:36.000000000","message":"This one https://review.opendev.org/c/openstack/octavia-tempest-plugin/+/664462, I would take a look a look to see if it cover ovn-provider cases or will requiere any update (in any case looks a old one patch...)","commit_id":"6fd64bd732210c6e7cd53618391a59cc038b15a6"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"39d9599f04f82bd7f4daae9961ed37e78f28f871","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":21,"id":"0afd5c4c_3d0212e4","updated":"2023-09-27 14:57:55.000000000","message":"recheck just to compare kuryr output","commit_id":"b673817663b5c3d28b6c613765ca058fc3d21563"}],"etc/octavia/conf.d/ovn.conf.sample":[{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"ba9ad49a9163362d27e4bce0a2b005ab7df8b0db","unresolved":true,"context_lines":[{"line_number":1,"context_line":"[DEFAULT]"},{"line_number":2,"context_line":""},{"line_number":3,"context_line":""},{"line_number":4,"context_line":"[neutron]"}],"source_content_type":"application/octet-stream","patch_set":9,"id":"e164c85d_e6e8b674","line":1,"updated":"2023-06-19 07:08:10.000000000","message":"was this added on purpose?","commit_id":"e430ec987d6367dce8d6f86cf3a7f01453b23108"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"71005891a4e57151093c191eb3dbc043e50e69f7","unresolved":false,"context_lines":[{"line_number":1,"context_line":"[DEFAULT]"},{"line_number":2,"context_line":""},{"line_number":3,"context_line":""},{"line_number":4,"context_line":"[neutron]"}],"source_content_type":"application/octet-stream","patch_set":9,"id":"249acb63_759ace9e","line":1,"in_reply_to":"e164c85d_e6e8b674","updated":"2023-06-19 14:29:19.000000000","message":"ouch, no! I forgot to remove after running some tox command locally.","commit_id":"e430ec987d6367dce8d6f86cf3a7f01453b23108"}],"ovn_octavia_provider/common/constants.py":[{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"d678ff6a65c89e0c9c492caf5d79807a63bbc57a","unresolved":true,"context_lines":[{"line_number":46,"context_line":"# NOTE(froyo):from additional-vips feature we will mantain the old ones for"},{"line_number":47,"context_line":"# backward compatibility"},{"line_number":48,"context_line":"LB_EXT_IDS_VIP_KEY \u003d \u0027neutron:vip\u0027"},{"line_number":49,"context_line":"LB_EXT_IDS_ADDIT_VIP_KEY \u003d \u0027neutron:additional_vips\u0027"},{"line_number":50,"context_line":"LB_EXT_IDS_VIP_PORT_ID_KEY \u003d \u0027neutron:vip_port_id\u0027"},{"line_number":51,"context_line":"LB_EXT_IDS_ADDIT_VIP_PORT_ID_KEY \u003d \u0027neutron:additional_vip_port_ids\u0027"},{"line_number":52,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"08c04352_eb559774","line":49,"range":{"start_line":49,"start_character":1,"end_line":49,"end_character":52},"updated":"2023-06-09 12:59:54.000000000","message":"we will need also neutron_additional_vips_fip too, right?","commit_id":"69b283fd7d81cbbeb19c0ccfb5e208b46f9d9ca5"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"0183a8ff0aca268d95719ffb531af605eaf83497","unresolved":false,"context_lines":[{"line_number":46,"context_line":"# NOTE(froyo):from additional-vips feature we will mantain the old ones for"},{"line_number":47,"context_line":"# backward compatibility"},{"line_number":48,"context_line":"LB_EXT_IDS_VIP_KEY \u003d \u0027neutron:vip\u0027"},{"line_number":49,"context_line":"LB_EXT_IDS_ADDIT_VIP_KEY \u003d \u0027neutron:additional_vips\u0027"},{"line_number":50,"context_line":"LB_EXT_IDS_VIP_PORT_ID_KEY \u003d \u0027neutron:vip_port_id\u0027"},{"line_number":51,"context_line":"LB_EXT_IDS_ADDIT_VIP_PORT_ID_KEY \u003d \u0027neutron:additional_vip_port_ids\u0027"},{"line_number":52,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"1a65dbda_0013893d","line":49,"range":{"start_line":49,"start_character":1,"end_line":49,"end_character":52},"in_reply_to":"08c04352_eb559774","updated":"2023-06-14 14:17:01.000000000","message":"Done","commit_id":"69b283fd7d81cbbeb19c0ccfb5e208b46f9d9ca5"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"ca784c7a1914aac0d7a5c2df320de8474c888c6a","unresolved":true,"context_lines":[{"line_number":47,"context_line":"LB_EXT_IDS_HM_VIP \u003d \u0027octavia:vip\u0027"},{"line_number":48,"context_line":"LB_EXT_IDS_HMS_KEY \u003d \u0027octavia:healthmonitors\u0027"},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"# NOTE(froyo):from additional-vips feature we will mantain the old ones for"},{"line_number":51,"context_line":"# backward compatibility"},{"line_number":52,"context_line":"LB_EXT_IDS_VIP_KEY \u003d \u0027neutron:vip\u0027"},{"line_number":53,"context_line":"LB_EXT_IDS_ADDIT_VIP_KEY \u003d \u0027neutron:additional_vips\u0027"}],"source_content_type":"text/x-python","patch_set":22,"id":"030b6a28_4ef4a2fb","line":50,"range":{"start_line":50,"start_character":51,"end_line":50,"end_character":58},"updated":"2023-11-08 08:24:07.000000000","message":"nitty nit (no action needed): maintain","commit_id":"cdd932af20c18e492d8661fcdb8c398325dd5b04"}],"ovn_octavia_provider/common/utils.py":[{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"44995e3b50a910a340e46beafaf579fc842f6293","unresolved":true,"context_lines":[{"line_number":21,"context_line":""},{"line_number":22,"context_line":"def get_uuid(dynamic_string):"},{"line_number":23,"context_line":"    # If it exists get the UUID from any string"},{"line_number":24,"context_line":"    uuid_pattern \u003d r\"[\\da-fA-F]{8}-(?:[\\da-fA-F]{4}-){3}[\\da-fA-F]{12}\""},{"line_number":25,"context_line":"    uuid_match \u003d re.search(uuid_pattern, dynamic_string)"},{"line_number":26,"context_line":"    uuid \u003d \u0027\u0027"},{"line_number":27,"context_line":"    if uuid_match:"}],"source_content_type":"text/x-python","patch_set":18,"id":"003130d2_369e0797","line":24,"range":{"start_line":24,"start_character":0,"end_line":24,"end_character":71},"updated":"2023-08-28 06:46:14.000000000","message":"I\u0027m in favor of the get_uuid utils method... I\u0027m just not sure if this complex re is better than simply take the last X characters as you have before, probably simply defining a constant about the number of characters to take (e.g., UUID_LEN).","commit_id":"6fd64bd732210c6e7cd53618391a59cc038b15a6"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"6998ae5ebfbe11448cd9b3053844dd96cbae6b6c","unresolved":false,"context_lines":[{"line_number":21,"context_line":""},{"line_number":22,"context_line":"def get_uuid(dynamic_string):"},{"line_number":23,"context_line":"    # If it exists get the UUID from any string"},{"line_number":24,"context_line":"    uuid_pattern \u003d r\"[\\da-fA-F]{8}-(?:[\\da-fA-F]{4}-){3}[\\da-fA-F]{12}\""},{"line_number":25,"context_line":"    uuid_match \u003d re.search(uuid_pattern, dynamic_string)"},{"line_number":26,"context_line":"    uuid \u003d \u0027\u0027"},{"line_number":27,"context_line":"    if uuid_match:"}],"source_content_type":"text/x-python","patch_set":18,"id":"f12f1ef8_bc6cda80","line":24,"range":{"start_line":24,"start_character":0,"end_line":24,"end_character":71},"in_reply_to":"003130d2_369e0797","updated":"2023-08-28 10:01:36.000000000","message":"Probably [-36:] was the easy one, but this way looks more robust for future search, where the uuid could be located in the middle of the string or whatever. My vote to keep this get_uuid using the regex pattern.","commit_id":"6fd64bd732210c6e7cd53618391a59cc038b15a6"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"b348c9398341a98d950d4c18b4cd6d0f0dfe6f1c","unresolved":false,"context_lines":[{"line_number":21,"context_line":""},{"line_number":22,"context_line":"def get_uuid(dynamic_string):"},{"line_number":23,"context_line":"    # If it exists get the UUID from any string"},{"line_number":24,"context_line":"    uuid_pattern \u003d r\"[\\da-fA-F]{8}-(?:[\\da-fA-F]{4}-){3}[\\da-fA-F]{12}\""},{"line_number":25,"context_line":"    uuid_match \u003d re.search(uuid_pattern, dynamic_string)"},{"line_number":26,"context_line":"    uuid \u003d \u0027\u0027"},{"line_number":27,"context_line":"    if uuid_match:"}],"source_content_type":"text/x-python","patch_set":18,"id":"e9b081c4_90306e15","line":24,"range":{"start_line":24,"start_character":0,"end_line":24,"end_character":71},"in_reply_to":"f12f1ef8_bc6cda80","updated":"2023-08-28 10:11:15.000000000","message":"fine with me","commit_id":"6fd64bd732210c6e7cd53618391a59cc038b15a6"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"44995e3b50a910a340e46beafaf579fc842f6293","unresolved":true,"context_lines":[{"line_number":23,"context_line":"    # If it exists get the UUID from any string"},{"line_number":24,"context_line":"    uuid_pattern \u003d r\"[\\da-fA-F]{8}-(?:[\\da-fA-F]{4}-){3}[\\da-fA-F]{12}\""},{"line_number":25,"context_line":"    uuid_match \u003d re.search(uuid_pattern, dynamic_string)"},{"line_number":26,"context_line":"    uuid \u003d \u0027\u0027"},{"line_number":27,"context_line":"    if uuid_match:"},{"line_number":28,"context_line":"        uuid \u003d uuid_match.group()"},{"line_number":29,"context_line":"    return uuid"},{"line_number":30,"context_line":""},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"def ovn_uuid(name):"}],"source_content_type":"text/x-python","patch_set":18,"id":"7ce84c4b_a6fa6d36","line":29,"range":{"start_line":26,"start_character":1,"end_line":29,"end_character":15},"updated":"2023-08-28 06:46:14.000000000","message":"super nit, perhaps:\nif uuid_match:\n    return uuid_match.group()\nreturn \u0027\u0027","commit_id":"6fd64bd732210c6e7cd53618391a59cc038b15a6"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"2515bcb8c01c557203e33d988ae790badcec4477","unresolved":false,"context_lines":[{"line_number":23,"context_line":"    # If it exists get the UUID from any string"},{"line_number":24,"context_line":"    uuid_pattern \u003d r\"[\\da-fA-F]{8}-(?:[\\da-fA-F]{4}-){3}[\\da-fA-F]{12}\""},{"line_number":25,"context_line":"    uuid_match \u003d re.search(uuid_pattern, dynamic_string)"},{"line_number":26,"context_line":"    uuid \u003d \u0027\u0027"},{"line_number":27,"context_line":"    if uuid_match:"},{"line_number":28,"context_line":"        uuid \u003d uuid_match.group()"},{"line_number":29,"context_line":"    return uuid"},{"line_number":30,"context_line":""},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"def ovn_uuid(name):"}],"source_content_type":"text/x-python","patch_set":18,"id":"3558593e_40dbeb2f","line":29,"range":{"start_line":26,"start_character":1,"end_line":29,"end_character":15},"in_reply_to":"7ce84c4b_a6fa6d36","updated":"2023-11-08 07:10:44.000000000","message":"Done","commit_id":"6fd64bd732210c6e7cd53618391a59cc038b15a6"}],"ovn_octavia_provider/driver.py":[{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"d678ff6a65c89e0c9c492caf5d79807a63bbc57a","unresolved":true,"context_lines":[{"line_number":452,"context_line":"            vip_dict[constants.VIP_ADDRESS] \u003d ("},{"line_number":453,"context_line":"                port[\u0027fixed_ips\u0027][0][\u0027ip_address\u0027])"},{"line_number":454,"context_line":""},{"line_number":455,"context_line":"            vip_additional_port_dict \u003d []"},{"line_number":456,"context_line":"            for vip_a_port in additional_ports:"},{"line_number":457,"context_line":"                vip_additional_port_dict.append({"},{"line_number":458,"context_line":"                    \u0027port_id\u0027: vip_a_port[\u0027id\u0027],"}],"source_content_type":"text/x-python","patch_set":3,"id":"020a56ab_cda7a9dd","line":455,"updated":"2023-06-09 12:59:54.000000000","message":"supernit: parameter is named additional_vips, perhaps naming this additional_vip_port_dict instead","commit_id":"69b283fd7d81cbbeb19c0ccfb5e208b46f9d9ca5"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"0183a8ff0aca268d95719ffb531af605eaf83497","unresolved":false,"context_lines":[{"line_number":452,"context_line":"            vip_dict[constants.VIP_ADDRESS] \u003d ("},{"line_number":453,"context_line":"                port[\u0027fixed_ips\u0027][0][\u0027ip_address\u0027])"},{"line_number":454,"context_line":""},{"line_number":455,"context_line":"            vip_additional_port_dict \u003d []"},{"line_number":456,"context_line":"            for vip_a_port in additional_ports:"},{"line_number":457,"context_line":"                vip_additional_port_dict.append({"},{"line_number":458,"context_line":"                    \u0027port_id\u0027: vip_a_port[\u0027id\u0027],"}],"source_content_type":"text/x-python","patch_set":3,"id":"a126dbee_670032f3","line":455,"in_reply_to":"020a56ab_cda7a9dd","updated":"2023-06-14 14:17:01.000000000","message":"Done","commit_id":"69b283fd7d81cbbeb19c0ccfb5e208b46f9d9ca5"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"d678ff6a65c89e0c9c492caf5d79807a63bbc57a","unresolved":true,"context_lines":[{"line_number":453,"context_line":"                port[\u0027fixed_ips\u0027][0][\u0027ip_address\u0027])"},{"line_number":454,"context_line":""},{"line_number":455,"context_line":"            vip_additional_port_dict \u003d []"},{"line_number":456,"context_line":"            for vip_a_port in additional_ports:"},{"line_number":457,"context_line":"                vip_additional_port_dict.append({"},{"line_number":458,"context_line":"                    \u0027port_id\u0027: vip_a_port[\u0027id\u0027],"},{"line_number":459,"context_line":"                    constants.NETWORK_ID: vip_a_port[constants.NETWORK_ID],"}],"source_content_type":"text/x-python","patch_set":3,"id":"07a15486_2e16adf7","line":456,"range":{"start_line":456,"start_character":16,"end_line":456,"end_character":26},"updated":"2023-06-09 12:59:54.000000000","message":"ditto: a_vip_port, or simply additional_port","commit_id":"69b283fd7d81cbbeb19c0ccfb5e208b46f9d9ca5"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"0183a8ff0aca268d95719ffb531af605eaf83497","unresolved":false,"context_lines":[{"line_number":453,"context_line":"                port[\u0027fixed_ips\u0027][0][\u0027ip_address\u0027])"},{"line_number":454,"context_line":""},{"line_number":455,"context_line":"            vip_additional_port_dict \u003d []"},{"line_number":456,"context_line":"            for vip_a_port in additional_ports:"},{"line_number":457,"context_line":"                vip_additional_port_dict.append({"},{"line_number":458,"context_line":"                    \u0027port_id\u0027: vip_a_port[\u0027id\u0027],"},{"line_number":459,"context_line":"                    constants.NETWORK_ID: vip_a_port[constants.NETWORK_ID],"}],"source_content_type":"text/x-python","patch_set":3,"id":"9f1bb03b_c8d21c19","line":456,"range":{"start_line":456,"start_character":16,"end_line":456,"end_character":26},"in_reply_to":"07a15486_2e16adf7","updated":"2023-06-14 14:17:01.000000000","message":"Done","commit_id":"69b283fd7d81cbbeb19c0ccfb5e208b46f9d9ca5"}],"ovn_octavia_provider/helper.py":[{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"d678ff6a65c89e0c9c492caf5d79807a63bbc57a","unresolved":true,"context_lines":[{"line_number":1034,"context_line":"                        additional_vip_port.get(\u0027port_id\u0027, None),"},{"line_number":1035,"context_line":"                        additional_vip_port.get(constants.NETWORK_ID, None),"},{"line_number":1036,"context_line":"                        additional_vip_port.get(\u0027ip_address\u0027, None))"},{"line_number":1037,"context_line":"                    additional_ports_subnet.append((ad_port, ad_subnet))"},{"line_number":1038,"context_line":"        except Exception:"},{"line_number":1039,"context_line":"            LOG.error(\u0027Cannot get info from neutron client\u0027)"},{"line_number":1040,"context_line":"            LOG.exception(ovn_const.EXCEPTION_MSG, \"creation of loadbalancer\")"}],"source_content_type":"text/x-python","patch_set":3,"id":"092e2904_218c3dfc","line":1037,"range":{"start_line":1037,"start_character":44,"end_line":1037,"end_character":72},"updated":"2023-06-09 12:59:54.000000000","message":"perhaps better to do this a dict? so that later you can simply use additional_ports_subnet.keys or .values","commit_id":"69b283fd7d81cbbeb19c0ccfb5e208b46f9d9ca5"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"0183a8ff0aca268d95719ffb531af605eaf83497","unresolved":false,"context_lines":[{"line_number":1034,"context_line":"                        additional_vip_port.get(\u0027port_id\u0027, None),"},{"line_number":1035,"context_line":"                        additional_vip_port.get(constants.NETWORK_ID, None),"},{"line_number":1036,"context_line":"                        additional_vip_port.get(\u0027ip_address\u0027, None))"},{"line_number":1037,"context_line":"                    additional_ports_subnet.append((ad_port, ad_subnet))"},{"line_number":1038,"context_line":"        except Exception:"},{"line_number":1039,"context_line":"            LOG.error(\u0027Cannot get info from neutron client\u0027)"},{"line_number":1040,"context_line":"            LOG.exception(ovn_const.EXCEPTION_MSG, \"creation of loadbalancer\")"}],"source_content_type":"text/x-python","patch_set":3,"id":"57cf44a2_e1c3348f","line":1037,"range":{"start_line":1037,"start_character":44,"end_line":1037,"end_character":72},"in_reply_to":"092e2904_218c3dfc","updated":"2023-06-14 14:17:01.000000000","message":"Ack","commit_id":"69b283fd7d81cbbeb19c0ccfb5e208b46f9d9ca5"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"d678ff6a65c89e0c9c492caf5d79807a63bbc57a","unresolved":true,"context_lines":[{"line_number":1123,"context_line":"            network \u003d neutron_client.get_network(port.network_id)"},{"line_number":1124,"context_line":"            if not network.provider_physical_network:"},{"line_number":1125,"context_line":"                # NOTE(froyo): This is the association of the lb to the VIP ls"},{"line_number":1126,"context_line":"                # so this is executed right away. For the additional vip ports"},{"line_number":1127,"context_line":"                # this step is not required since all subnets must belong to"},{"line_number":1128,"context_line":"                # the same subnet, so just for the VIP LB port is enough."},{"line_number":1129,"context_line":"                self._update_lb_to_ls_association("},{"line_number":1130,"context_line":"                    ovn_lb, network_id\u003dport.network_id,"},{"line_number":1131,"context_line":"                    associate\u003dTrue, update_ls_ref\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":3,"id":"7ea3a5f4_504c4044","line":1128,"range":{"start_line":1126,"start_character":49,"end_line":1128,"end_character":73},"updated":"2023-06-09 12:59:54.000000000","message":"then why are you getting the extra subnets in lines 1023-1037","commit_id":"69b283fd7d81cbbeb19c0ccfb5e208b46f9d9ca5"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"0183a8ff0aca268d95719ffb531af605eaf83497","unresolved":false,"context_lines":[{"line_number":1123,"context_line":"            network \u003d neutron_client.get_network(port.network_id)"},{"line_number":1124,"context_line":"            if not network.provider_physical_network:"},{"line_number":1125,"context_line":"                # NOTE(froyo): This is the association of the lb to the VIP ls"},{"line_number":1126,"context_line":"                # so this is executed right away. For the additional vip ports"},{"line_number":1127,"context_line":"                # this step is not required since all subnets must belong to"},{"line_number":1128,"context_line":"                # the same subnet, so just for the VIP LB port is enough."},{"line_number":1129,"context_line":"                self._update_lb_to_ls_association("},{"line_number":1130,"context_line":"                    ovn_lb, network_id\u003dport.network_id,"},{"line_number":1131,"context_line":"                    associate\u003dTrue, update_ls_ref\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":3,"id":"c9a1a351_cc2d315e","line":1128,"range":{"start_line":1126,"start_character":49,"end_line":1128,"end_character":73},"in_reply_to":"7ea3a5f4_504c4044","updated":"2023-06-14 14:17:01.000000000","message":"good catch! the check is already done by Octavia API so no more actions required over additional_port subnet, so no required there.","commit_id":"69b283fd7d81cbbeb19c0ccfb5e208b46f9d9ca5"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"d678ff6a65c89e0c9c492caf5d79807a63bbc57a","unresolved":true,"context_lines":[{"line_number":1254,"context_line":"                                    str(vip_port_id))"},{"line_number":1255,"context_line":"                        self.delete_port(vip_port_id)"},{"line_number":1256,"context_line":"            except Exception:"},{"line_number":1257,"context_line":"                LOG.exception(\"Error deleting the VIP/additional vip port\")"},{"line_number":1258,"context_line":"                lbalancer_status[constants.PROVISIONING_STATUS] \u003d ("},{"line_number":1259,"context_line":"                    constants.ERROR)"},{"line_number":1260,"context_line":"                lbalancer_status[constants.OPERATING_STATUS] \u003d constants.ERROR"}],"source_content_type":"text/x-python","patch_set":3,"id":"a975dc80_8df0f09c","line":1257,"range":{"start_line":1257,"start_character":31,"end_line":1257,"end_character":74},"updated":"2023-06-09 12:59:54.000000000","message":"perhaps simply \"Error deleting the VIP port(s)\"","commit_id":"69b283fd7d81cbbeb19c0ccfb5e208b46f9d9ca5"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"0183a8ff0aca268d95719ffb531af605eaf83497","unresolved":false,"context_lines":[{"line_number":1254,"context_line":"                                    str(vip_port_id))"},{"line_number":1255,"context_line":"                        self.delete_port(vip_port_id)"},{"line_number":1256,"context_line":"            except Exception:"},{"line_number":1257,"context_line":"                LOG.exception(\"Error deleting the VIP/additional vip port\")"},{"line_number":1258,"context_line":"                lbalancer_status[constants.PROVISIONING_STATUS] \u003d ("},{"line_number":1259,"context_line":"                    constants.ERROR)"},{"line_number":1260,"context_line":"                lbalancer_status[constants.OPERATING_STATUS] \u003d constants.ERROR"}],"source_content_type":"text/x-python","patch_set":3,"id":"b52999be_fcea1c54","line":1257,"range":{"start_line":1257,"start_character":31,"end_line":1257,"end_character":74},"in_reply_to":"a975dc80_8df0f09c","updated":"2023-06-14 14:17:01.000000000","message":"Done","commit_id":"69b283fd7d81cbbeb19c0ccfb5e208b46f9d9ca5"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"d678ff6a65c89e0c9c492caf5d79807a63bbc57a","unresolved":true,"context_lines":[{"line_number":2517,"context_line":"        if ovn_const.LB_EXT_IDS_ADDIT_VIP_KEY in ovn_lb.external_ids:"},{"line_number":2518,"context_line":"            vips.extend(ovn_lb.external_ids.get("},{"line_number":2519,"context_line":"                ovn_const.LB_EXT_IDS_ADDIT_VIP_KEY).split(\u0027,\u0027))"},{"line_number":2520,"context_line":"        fip \u003d ovn_lb.external_ids.get(ovn_const.LB_EXT_IDS_VIP_FIP_KEY)"},{"line_number":2521,"context_line":"        if not vips:"},{"line_number":2522,"context_line":"            LOG.error(\"Could not find VIP for HM %s, LB external_ids: %s\","},{"line_number":2523,"context_line":"                      hm_id, ovn_lb.external_ids)"}],"source_content_type":"text/x-python","patch_set":3,"id":"60f20a80_96fa8e03","line":2520,"range":{"start_line":2520,"start_character":2,"end_line":2520,"end_character":71},"updated":"2023-06-09 12:59:54.000000000","message":"the logic in here is kind of wrong, you don\u0027t associate the FIP to the loadbalancer, but the FIP to the loadbanacer VIP port, meaning that you only need to associate the FIP to one of the VIPs, and that there may be different FIP associated to different VIPs","commit_id":"69b283fd7d81cbbeb19c0ccfb5e208b46f9d9ca5"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"0183a8ff0aca268d95719ffb531af605eaf83497","unresolved":false,"context_lines":[{"line_number":2517,"context_line":"        if ovn_const.LB_EXT_IDS_ADDIT_VIP_KEY in ovn_lb.external_ids:"},{"line_number":2518,"context_line":"            vips.extend(ovn_lb.external_ids.get("},{"line_number":2519,"context_line":"                ovn_const.LB_EXT_IDS_ADDIT_VIP_KEY).split(\u0027,\u0027))"},{"line_number":2520,"context_line":"        fip \u003d ovn_lb.external_ids.get(ovn_const.LB_EXT_IDS_VIP_FIP_KEY)"},{"line_number":2521,"context_line":"        if not vips:"},{"line_number":2522,"context_line":"            LOG.error(\"Could not find VIP for HM %s, LB external_ids: %s\","},{"line_number":2523,"context_line":"                      hm_id, ovn_lb.external_ids)"}],"source_content_type":"text/x-python","patch_set":3,"id":"e963adf5_c67819ec","line":2520,"range":{"start_line":2520,"start_character":2,"end_line":2520,"end_character":71},"in_reply_to":"60f20a80_96fa8e03","updated":"2023-06-14 14:17:01.000000000","message":"Done","commit_id":"69b283fd7d81cbbeb19c0ccfb5e208b46f9d9ca5"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"ba9ad49a9163362d27e4bce0a2b005ab7df8b0db","unresolved":true,"context_lines":[{"line_number":343,"context_line":"        port_name \u003d vip_lp.external_ids.get(ovn_const.OVN_PORT_NAME_EXT_ID_KEY)"},{"line_number":344,"context_line":"        additional_vip \u003d False"},{"line_number":345,"context_line":"        if ovn_const.LB_VIP_ADDIT_PORT_PREFIX in port_name:"},{"line_number":346,"context_line":"            pattern \u003d ovn_const.LB_VIP_ADDIT_PORT_PREFIX + r\u0027\\d+-\u0027"},{"line_number":347,"context_line":"            lb_id \u003d re.sub(pattern, \u0027\u0027, port_name)"},{"line_number":348,"context_line":"            additional_vip \u003d True"},{"line_number":349,"context_line":"        else:"},{"line_number":350,"context_line":"            lb_id \u003d port_name[len(ovn_const.LB_VIP_PORT_PREFIX):]"},{"line_number":351,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":9,"id":"bf58e01d_e141ad86","line":348,"range":{"start_line":346,"start_character":0,"end_line":348,"end_character":33},"updated":"2023-06-19 07:08:10.000000000","message":"can we do something similar to what we do for the main VIP port? simply:\nport_name[(len(ovn.const.LB_VIP_ADDIT_PORT_PREFIX):]","commit_id":"e430ec987d6367dce8d6f86cf3a7f01453b23108"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"71005891a4e57151093c191eb3dbc043e50e69f7","unresolved":false,"context_lines":[{"line_number":343,"context_line":"        port_name \u003d vip_lp.external_ids.get(ovn_const.OVN_PORT_NAME_EXT_ID_KEY)"},{"line_number":344,"context_line":"        additional_vip \u003d False"},{"line_number":345,"context_line":"        if ovn_const.LB_VIP_ADDIT_PORT_PREFIX in port_name:"},{"line_number":346,"context_line":"            pattern \u003d ovn_const.LB_VIP_ADDIT_PORT_PREFIX + r\u0027\\d+-\u0027"},{"line_number":347,"context_line":"            lb_id \u003d re.sub(pattern, \u0027\u0027, port_name)"},{"line_number":348,"context_line":"            additional_vip \u003d True"},{"line_number":349,"context_line":"        else:"},{"line_number":350,"context_line":"            lb_id \u003d port_name[len(ovn_const.LB_VIP_PORT_PREFIX):]"},{"line_number":351,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":9,"id":"2519df38_c110c8b6","line":348,"range":{"start_line":346,"start_character":0,"end_line":348,"end_character":33},"in_reply_to":"bf58e01d_e141ad86","updated":"2023-06-19 14:29:19.000000000","message":"No, because those additional ports are formed at this way:\n\novn_const.LB_VIP_ADDIT_PORT_PREFIX + index (1...n) + lb_id\n\nBut taking into account that the lb_id is an uuid you give an idea replace those two lines (346-347) by:\nlb_id \u003d port_name[-36:]","commit_id":"e430ec987d6367dce8d6f86cf3a7f01453b23108"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"ba9ad49a9163362d27e4bce0a2b005ab7df8b0db","unresolved":true,"context_lines":[{"line_number":365,"context_line":"                            \u0027vip_fip\u0027: fip}"},{"line_number":366,"context_line":"            request_info[\u0027additional_vip_fip\u0027] \u003d additional_vip"},{"line_number":367,"context_line":"            if additional_vip:"},{"line_number":368,"context_line":"                if fip and lb_addi_vip_fip and fip not in lb_addi_vip_fip:"},{"line_number":369,"context_line":"                    request_info[\u0027action\u0027] \u003d \\"},{"line_number":370,"context_line":"                        ovn_const.REQ_INFO_ACTION_ASSOCIATE"},{"line_number":371,"context_line":"                elif fip and not lb_addi_vip_fip:"},{"line_number":372,"context_line":"                    request_info[\u0027action\u0027] \u003d \\"},{"line_number":373,"context_line":"                        ovn_const.REQ_INFO_ACTION_ASSOCIATE"},{"line_number":374,"context_line":"                elif fip is None and fip !\u003d lb_addi_vip_fip:"},{"line_number":375,"context_line":"                    request_info[\u0027action\u0027] \u003d \\"},{"line_number":376,"context_line":"                        ovn_const.REQ_INFO_ACTION_DISASSOCIATE"},{"line_number":377,"context_line":"                else:"},{"line_number":378,"context_line":"                    continue"},{"line_number":379,"context_line":"            else:"}],"source_content_type":"text/x-python","patch_set":9,"id":"66eed258_38e5a6ee","line":376,"range":{"start_line":368,"start_character":1,"end_line":376,"end_character":62},"updated":"2023-06-19 07:08:10.000000000","message":"I don\u0027t get this, shouldn\u0027t we just use lb_addit_vip_fip here? we are handling the association of a FIP to a VIP, if the association is to the additional VIP, we should not care about the FIP associated to the VIP, right?","commit_id":"e430ec987d6367dce8d6f86cf3a7f01453b23108"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"a2de9538afa7b998928622c4ec0a18f4e7768442","unresolved":true,"context_lines":[{"line_number":365,"context_line":"                            \u0027vip_fip\u0027: fip}"},{"line_number":366,"context_line":"            request_info[\u0027additional_vip_fip\u0027] \u003d additional_vip"},{"line_number":367,"context_line":"            if additional_vip:"},{"line_number":368,"context_line":"                if fip and lb_addi_vip_fip and fip not in lb_addi_vip_fip:"},{"line_number":369,"context_line":"                    request_info[\u0027action\u0027] \u003d \\"},{"line_number":370,"context_line":"                        ovn_const.REQ_INFO_ACTION_ASSOCIATE"},{"line_number":371,"context_line":"                elif fip and not lb_addi_vip_fip:"},{"line_number":372,"context_line":"                    request_info[\u0027action\u0027] \u003d \\"},{"line_number":373,"context_line":"                        ovn_const.REQ_INFO_ACTION_ASSOCIATE"},{"line_number":374,"context_line":"                elif fip is None and fip !\u003d lb_addi_vip_fip:"},{"line_number":375,"context_line":"                    request_info[\u0027action\u0027] \u003d \\"},{"line_number":376,"context_line":"                        ovn_const.REQ_INFO_ACTION_DISASSOCIATE"},{"line_number":377,"context_line":"                else:"},{"line_number":378,"context_line":"                    continue"},{"line_number":379,"context_line":"            else:"}],"source_content_type":"text/x-python","patch_set":9,"id":"28ad4397_b9580808","line":376,"range":{"start_line":368,"start_character":1,"end_line":376,"end_character":62},"in_reply_to":"2772da57_131e5314","updated":"2023-06-20 05:55:22.000000000","message":"That is not what I meant. I understand we need to check if it is association or dissasociation. But now I get that vip_lp is either the FIP associated to the VIP or to the additional FIP. So this is ok!\n\nAnyway, the first two if perhaps can be summarirez with the if at linke 369, if get at line 363,364 returns [] instead of \u0027\u0027 if not found","commit_id":"e430ec987d6367dce8d6f86cf3a7f01453b23108"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"898ee29c49aa0d50d30076e0a8ef49a0c6ac16f6","unresolved":false,"context_lines":[{"line_number":365,"context_line":"                            \u0027vip_fip\u0027: fip}"},{"line_number":366,"context_line":"            request_info[\u0027additional_vip_fip\u0027] \u003d additional_vip"},{"line_number":367,"context_line":"            if additional_vip:"},{"line_number":368,"context_line":"                if fip and lb_addi_vip_fip and fip not in lb_addi_vip_fip:"},{"line_number":369,"context_line":"                    request_info[\u0027action\u0027] \u003d \\"},{"line_number":370,"context_line":"                        ovn_const.REQ_INFO_ACTION_ASSOCIATE"},{"line_number":371,"context_line":"                elif fip and not lb_addi_vip_fip:"},{"line_number":372,"context_line":"                    request_info[\u0027action\u0027] \u003d \\"},{"line_number":373,"context_line":"                        ovn_const.REQ_INFO_ACTION_ASSOCIATE"},{"line_number":374,"context_line":"                elif fip is None and fip !\u003d lb_addi_vip_fip:"},{"line_number":375,"context_line":"                    request_info[\u0027action\u0027] \u003d \\"},{"line_number":376,"context_line":"                        ovn_const.REQ_INFO_ACTION_DISASSOCIATE"},{"line_number":377,"context_line":"                else:"},{"line_number":378,"context_line":"                    continue"},{"line_number":379,"context_line":"            else:"}],"source_content_type":"text/x-python","patch_set":9,"id":"0fbc3008_f3878844","line":376,"range":{"start_line":368,"start_character":1,"end_line":376,"end_character":62},"in_reply_to":"28ad4397_b9580808","updated":"2023-08-10 12:07:08.000000000","message":"Done","commit_id":"e430ec987d6367dce8d6f86cf3a7f01453b23108"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"71005891a4e57151093c191eb3dbc043e50e69f7","unresolved":true,"context_lines":[{"line_number":365,"context_line":"                            \u0027vip_fip\u0027: fip}"},{"line_number":366,"context_line":"            request_info[\u0027additional_vip_fip\u0027] \u003d additional_vip"},{"line_number":367,"context_line":"            if additional_vip:"},{"line_number":368,"context_line":"                if fip and lb_addi_vip_fip and fip not in lb_addi_vip_fip:"},{"line_number":369,"context_line":"                    request_info[\u0027action\u0027] \u003d \\"},{"line_number":370,"context_line":"                        ovn_const.REQ_INFO_ACTION_ASSOCIATE"},{"line_number":371,"context_line":"                elif fip and not lb_addi_vip_fip:"},{"line_number":372,"context_line":"                    request_info[\u0027action\u0027] \u003d \\"},{"line_number":373,"context_line":"                        ovn_const.REQ_INFO_ACTION_ASSOCIATE"},{"line_number":374,"context_line":"                elif fip is None and fip !\u003d lb_addi_vip_fip:"},{"line_number":375,"context_line":"                    request_info[\u0027action\u0027] \u003d \\"},{"line_number":376,"context_line":"                        ovn_const.REQ_INFO_ACTION_DISASSOCIATE"},{"line_number":377,"context_line":"                else:"},{"line_number":378,"context_line":"                    continue"},{"line_number":379,"context_line":"            else:"}],"source_content_type":"text/x-python","patch_set":9,"id":"2772da57_131e5314","line":376,"range":{"start_line":368,"start_character":1,"end_line":376,"end_character":62},"in_reply_to":"66eed258_38e5a6ee","updated":"2023-06-19 14:29:19.000000000","message":"This method it to handler the assoc/disassoc of a FIP to a VIP port (independently VIP or an additional VIP port) so after determine if the port affected is the VIP or one of the additional VIP ones, we need to identify if we are on an assoc or a disassoc request.","commit_id":"e430ec987d6367dce8d6f86cf3a7f01453b23108"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"ba9ad49a9163362d27e4bce0a2b005ab7df8b0db","unresolved":true,"context_lines":[{"line_number":701,"context_line":"                # NOTE(froyo): To cover the initial lb to ls association, where"},{"line_number":702,"context_line":"                # additional vips shall be in the same network as VIP port,"},{"line_number":703,"context_line":"                # and the ls_ref[vip_network_id] should take them into account."},{"line_number":704,"context_line":"                if additional_vips:"},{"line_number":705,"context_line":"                    addi_vips \u003d ovn_lb.external_ids.get("},{"line_number":706,"context_line":"                        ovn_const.LB_EXT_IDS_ADDIT_VIP_KEY, \u0027\u0027)"},{"line_number":707,"context_line":"                    if addi_vips:"}],"source_content_type":"text/x-python","patch_set":9,"id":"c21bbed7_41313a8c","line":704,"range":{"start_line":704,"start_character":16,"end_line":704,"end_character":35},"updated":"2023-06-19 07:08:10.000000000","message":"you don\u0027t need this parameter, right? just checking if addi_vips is there is enough, or not?","commit_id":"e430ec987d6367dce8d6f86cf3a7f01453b23108"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"a2de9538afa7b998928622c4ec0a18f4e7768442","unresolved":false,"context_lines":[{"line_number":701,"context_line":"                # NOTE(froyo): To cover the initial lb to ls association, where"},{"line_number":702,"context_line":"                # additional vips shall be in the same network as VIP port,"},{"line_number":703,"context_line":"                # and the ls_ref[vip_network_id] should take them into account."},{"line_number":704,"context_line":"                if additional_vips:"},{"line_number":705,"context_line":"                    addi_vips \u003d ovn_lb.external_ids.get("},{"line_number":706,"context_line":"                        ovn_const.LB_EXT_IDS_ADDIT_VIP_KEY, \u0027\u0027)"},{"line_number":707,"context_line":"                    if addi_vips:"}],"source_content_type":"text/x-python","patch_set":9,"id":"cdd37dd9_1d888792","line":704,"range":{"start_line":704,"start_character":16,"end_line":704,"end_character":35},"in_reply_to":"962cc4f0_a827dd52","updated":"2023-06-20 05:55:22.000000000","message":"then I would change the name of this parameter as it is misleading. In addition, we have update_ls_ref already in this method, perhaps that is what we want/can use?\n\nAlso, what do you mean the addi_vips will always be there? you mean it is always on the external_ids? that should not be the case if there is no additional vips, right? (plus you are using get with default \u0027\u0027 in case of not found, which implies it may not be there, right?","commit_id":"e430ec987d6367dce8d6f86cf3a7f01453b23108"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"71005891a4e57151093c191eb3dbc043e50e69f7","unresolved":false,"context_lines":[{"line_number":701,"context_line":"                # NOTE(froyo): To cover the initial lb to ls association, where"},{"line_number":702,"context_line":"                # additional vips shall be in the same network as VIP port,"},{"line_number":703,"context_line":"                # and the ls_ref[vip_network_id] should take them into account."},{"line_number":704,"context_line":"                if additional_vips:"},{"line_number":705,"context_line":"                    addi_vips \u003d ovn_lb.external_ids.get("},{"line_number":706,"context_line":"                        ovn_const.LB_EXT_IDS_ADDIT_VIP_KEY, \u0027\u0027)"},{"line_number":707,"context_line":"                    if addi_vips:"}],"source_content_type":"text/x-python","patch_set":9,"id":"962cc4f0_a827dd52","line":704,"range":{"start_line":704,"start_character":16,"end_line":704,"end_character":35},"in_reply_to":"c21bbed7_41313a8c","updated":"2023-06-19 14:29:19.000000000","message":"No, we need this one, because addi_vips will be always there, and we use this flag on the parameters of the method to indicate when we want to increment the ls_ref due to additional_vips and when not, indeed we only do on the lb_create, the following calls (member add) we will call it without this parameter in order to not take them into account.","commit_id":"e430ec987d6367dce8d6f86cf3a7f01453b23108"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"ba9ad49a9163362d27e4bce0a2b005ab7df8b0db","unresolved":true,"context_lines":[{"line_number":1067,"context_line":"        subnet \u003d None"},{"line_number":1068,"context_line":"        additional_ports \u003d []"},{"line_number":1069,"context_line":"        try:"},{"line_number":1070,"context_line":"            neutron_client \u003d clients.get_neutron_client()"},{"line_number":1071,"context_line":"            port, subnet \u003d self._get_port_from_info("},{"line_number":1072,"context_line":"                neutron_client,"},{"line_number":1073,"context_line":"                loadbalancer.get(constants.VIP_PORT_ID, None),"}],"source_content_type":"text/x-python","patch_set":9,"id":"a92b7318_a02601fc","line":1070,"range":{"start_line":1070,"start_character":11,"end_line":1070,"end_character":57},"updated":"2023-06-19 07:08:10.000000000","message":"better moving the neutron client to the get_port_from_info instead of passing it through parameter, as it is not needed here","commit_id":"e430ec987d6367dce8d6f86cf3a7f01453b23108"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"a2de9538afa7b998928622c4ec0a18f4e7768442","unresolved":false,"context_lines":[{"line_number":1067,"context_line":"        subnet \u003d None"},{"line_number":1068,"context_line":"        additional_ports \u003d []"},{"line_number":1069,"context_line":"        try:"},{"line_number":1070,"context_line":"            neutron_client \u003d clients.get_neutron_client()"},{"line_number":1071,"context_line":"            port, subnet \u003d self._get_port_from_info("},{"line_number":1072,"context_line":"                neutron_client,"},{"line_number":1073,"context_line":"                loadbalancer.get(constants.VIP_PORT_ID, None),"}],"source_content_type":"text/x-python","patch_set":9,"id":"a99c73b5_c088314a","line":1070,"range":{"start_line":1070,"start_character":11,"end_line":1070,"end_character":57},"in_reply_to":"90800bf2_1dd04d83","updated":"2023-06-20 05:55:22.000000000","message":"umm, perhaps then it should be out of the try/except. Or instantiated later if needed. But I think it is cleaner to have that inside the auxiliar method instead","commit_id":"e430ec987d6367dce8d6f86cf3a7f01453b23108"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"71005891a4e57151093c191eb3dbc043e50e69f7","unresolved":false,"context_lines":[{"line_number":1067,"context_line":"        subnet \u003d None"},{"line_number":1068,"context_line":"        additional_ports \u003d []"},{"line_number":1069,"context_line":"        try:"},{"line_number":1070,"context_line":"            neutron_client \u003d clients.get_neutron_client()"},{"line_number":1071,"context_line":"            port, subnet \u003d self._get_port_from_info("},{"line_number":1072,"context_line":"                neutron_client,"},{"line_number":1073,"context_line":"                loadbalancer.get(constants.VIP_PORT_ID, None),"}],"source_content_type":"text/x-python","patch_set":9,"id":"90800bf2_1dd04d83","line":1070,"range":{"start_line":1070,"start_character":11,"end_line":1070,"end_character":57},"in_reply_to":"a92b7318_a02601fc","updated":"2023-06-19 14:29:19.000000000","message":"It will also needed in L1175, so I will prefer to mantain it here just to get it once in all the lb_create method instead of three calls if we move inside _get_port_from_info","commit_id":"e430ec987d6367dce8d6f86cf3a7f01453b23108"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"ba9ad49a9163362d27e4bce0a2b005ab7df8b0db","unresolved":true,"context_lines":[{"line_number":1304,"context_line":"                                    str(vip_port_id))"},{"line_number":1305,"context_line":"                        self.delete_port(vip_port_id)"},{"line_number":1306,"context_line":"            except Exception:"},{"line_number":1307,"context_line":"                LOG.exception(\"Error deleting the VIP port(s)\")"},{"line_number":1308,"context_line":"                lbalancer_status[constants.PROVISIONING_STATUS] \u003d ("},{"line_number":1309,"context_line":"                    constants.ERROR)"},{"line_number":1310,"context_line":"                lbalancer_status[constants.OPERATING_STATUS] \u003d constants.ERROR"}],"source_content_type":"text/x-python","patch_set":9,"id":"bc5873ab_fc8ddf27","line":1307,"range":{"start_line":1307,"start_character":16,"end_line":1307,"end_character":63},"updated":"2023-06-19 07:08:10.000000000","message":"perhaps now we need to print what port failed as it can  be either the VIP or the additional ones","commit_id":"e430ec987d6367dce8d6f86cf3a7f01453b23108"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"71005891a4e57151093c191eb3dbc043e50e69f7","unresolved":false,"context_lines":[{"line_number":1304,"context_line":"                                    str(vip_port_id))"},{"line_number":1305,"context_line":"                        self.delete_port(vip_port_id)"},{"line_number":1306,"context_line":"            except Exception:"},{"line_number":1307,"context_line":"                LOG.exception(\"Error deleting the VIP port(s)\")"},{"line_number":1308,"context_line":"                lbalancer_status[constants.PROVISIONING_STATUS] \u003d ("},{"line_number":1309,"context_line":"                    constants.ERROR)"},{"line_number":1310,"context_line":"                lbalancer_status[constants.OPERATING_STATUS] \u003d constants.ERROR"}],"source_content_type":"text/x-python","patch_set":9,"id":"1f006826_50a1ead0","line":1307,"range":{"start_line":1307,"start_character":16,"end_line":1307,"end_character":63},"in_reply_to":"bc5873ab_fc8ddf27","updated":"2023-06-19 14:29:19.000000000","message":"ACK, to give a more precise information in case of error or need to debug.","commit_id":"e430ec987d6367dce8d6f86cf3a7f01453b23108"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"ba9ad49a9163362d27e4bce0a2b005ab7df8b0db","unresolved":true,"context_lines":[{"line_number":2401,"context_line":"                        \"check Neutron logs. Perhaps port \""},{"line_number":2402,"context_line":"                        \"was already deleted.\", port_id)"},{"line_number":2403,"context_line":""},{"line_number":2404,"context_line":"    def handle_vip_fip(self, fip_info, additional_vip_fip\u003dFalse):"},{"line_number":2405,"context_line":"        ovn_lb \u003d fip_info[\u0027ovn_lb\u0027]"},{"line_number":2406,"context_line":"        external_ids \u003d copy.deepcopy(ovn_lb.external_ids)"},{"line_number":2407,"context_line":"        commands \u003d []"}],"source_content_type":"text/x-python","patch_set":9,"id":"0116fb04_e80f6642","line":2404,"range":{"start_line":2404,"start_character":0,"end_line":2404,"end_character":25},"updated":"2023-06-19 07:08:10.000000000","message":"I need to take a deeper look into this as it is not cristal clear to me. Here, what happen is you have an additional VIP (ipv4) over an IPv6 main VIP? In that case the members should be different for them, and I\u0027m not fully sure that is the case","commit_id":"e430ec987d6367dce8d6f86cf3a7f01453b23108"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"898ee29c49aa0d50d30076e0a8ef49a0c6ac16f6","unresolved":false,"context_lines":[{"line_number":2401,"context_line":"                        \"check Neutron logs. Perhaps port \""},{"line_number":2402,"context_line":"                        \"was already deleted.\", port_id)"},{"line_number":2403,"context_line":""},{"line_number":2404,"context_line":"    def handle_vip_fip(self, fip_info, additional_vip_fip\u003dFalse):"},{"line_number":2405,"context_line":"        ovn_lb \u003d fip_info[\u0027ovn_lb\u0027]"},{"line_number":2406,"context_line":"        external_ids \u003d copy.deepcopy(ovn_lb.external_ids)"},{"line_number":2407,"context_line":"        commands \u003d []"}],"source_content_type":"text/x-python","patch_set":9,"id":"58e0d277_3b59009c","line":2404,"range":{"start_line":2404,"start_character":0,"end_line":2404,"end_character":25},"in_reply_to":"0116fb04_e80f6642","updated":"2023-08-10 12:07:08.000000000","message":"Done","commit_id":"e430ec987d6367dce8d6f86cf3a7f01453b23108"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"a2de9538afa7b998928622c4ec0a18f4e7768442","unresolved":true,"context_lines":[{"line_number":342,"context_line":""},{"line_number":343,"context_line":"        port_name \u003d vip_lp.external_ids.get(ovn_const.OVN_PORT_NAME_EXT_ID_KEY)"},{"line_number":344,"context_line":"        additional_vip \u003d False"},{"line_number":345,"context_line":"        if ovn_const.LB_VIP_ADDIT_PORT_PREFIX in port_name:"},{"line_number":346,"context_line":"            # As LB id is an uuid at the end of string, we can get just the"},{"line_number":347,"context_line":"            # last 36 characters to get it."},{"line_number":348,"context_line":"            lb_id \u003d port_name[-36:]"}],"source_content_type":"text/x-python","patch_set":11,"id":"101c55ea_b290044d","line":345,"range":{"start_line":345,"start_character":8,"end_line":345,"end_character":59},"updated":"2023-06-20 05:55:22.000000000","message":"perhaps better to check it starts with, not it contains","commit_id":"197124dbe15d3ca832b706005bcc6708f98944d0"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"e5ca37341493f3e604134a6d23fd3ab1f9848855","unresolved":false,"context_lines":[{"line_number":342,"context_line":""},{"line_number":343,"context_line":"        port_name \u003d vip_lp.external_ids.get(ovn_const.OVN_PORT_NAME_EXT_ID_KEY)"},{"line_number":344,"context_line":"        additional_vip \u003d False"},{"line_number":345,"context_line":"        if ovn_const.LB_VIP_ADDIT_PORT_PREFIX in port_name:"},{"line_number":346,"context_line":"            # As LB id is an uuid at the end of string, we can get just the"},{"line_number":347,"context_line":"            # last 36 characters to get it."},{"line_number":348,"context_line":"            lb_id \u003d port_name[-36:]"}],"source_content_type":"text/x-python","patch_set":11,"id":"25e3bd8b_84bb1d84","line":345,"range":{"start_line":345,"start_character":8,"end_line":345,"end_character":59},"in_reply_to":"101c55ea_b290044d","updated":"2023-08-24 15:37:42.000000000","message":"Done","commit_id":"197124dbe15d3ca832b706005bcc6708f98944d0"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"5323f4d4fa9e1f1ea6a390d28e4be59b3d2afe99","unresolved":true,"context_lines":[{"line_number":345,"context_line":"        if port_name.startswith(ovn_const.LB_VIP_ADDIT_PORT_PREFIX):"},{"line_number":346,"context_line":"            # As LB id is an uuid at the end of string, we can get just the"},{"line_number":347,"context_line":"            # last 36 characters to get it."},{"line_number":348,"context_line":"            lb_id \u003d port_name[-36:]"},{"line_number":349,"context_line":"            additional_vip \u003d True"},{"line_number":350,"context_line":"        else:"},{"line_number":351,"context_line":"            lb_id \u003d port_name[len(ovn_const.LB_VIP_PORT_PREFIX):]"}],"source_content_type":"text/x-python","patch_set":15,"id":"93667b8c_40341f46","line":348,"range":{"start_line":348,"start_character":30,"end_line":348,"end_character":34},"updated":"2023-08-21 14:29:51.000000000","message":"why not doing port_name[len(ovn_const.LB_VIP_ADDIT_PORT_PREFIX):]?","commit_id":"64e307f2bda8e1d51e7a35b0c983382d618097ec"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"e5ca37341493f3e604134a6d23fd3ab1f9848855","unresolved":false,"context_lines":[{"line_number":345,"context_line":"        if port_name.startswith(ovn_const.LB_VIP_ADDIT_PORT_PREFIX):"},{"line_number":346,"context_line":"            # As LB id is an uuid at the end of string, we can get just the"},{"line_number":347,"context_line":"            # last 36 characters to get it."},{"line_number":348,"context_line":"            lb_id \u003d port_name[-36:]"},{"line_number":349,"context_line":"            additional_vip \u003d True"},{"line_number":350,"context_line":"        else:"},{"line_number":351,"context_line":"            lb_id \u003d port_name[len(ovn_const.LB_VIP_PORT_PREFIX):]"}],"source_content_type":"text/x-python","patch_set":15,"id":"c4a562ea_d71bada3","line":348,"range":{"start_line":348,"start_character":30,"end_line":348,"end_character":34},"in_reply_to":"93667b8c_40341f46","updated":"2023-08-24 15:37:42.000000000","message":"I agree, initially this part was covering a normal vip and additional vip, as the prefix was different I chose read the uuid in reverse way, but not looks better use the length of the prefix.","commit_id":"64e307f2bda8e1d51e7a35b0c983382d618097ec"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"5323f4d4fa9e1f1ea6a390d28e4be59b3d2afe99","unresolved":true,"context_lines":[{"line_number":367,"context_line":"            if port:"},{"line_number":368,"context_line":"                request_info[\u0027vip_related\u0027] \u003d ["},{"line_number":369,"context_line":"                    ip[\u0027ip_address\u0027] for ip in port.fixed_ips]"},{"line_number":370,"context_line":"            request_info[\u0027additional_vip_fip\u0027] \u003d additional_vip"},{"line_number":371,"context_line":"            request_info[\u0027action\u0027] \u003d action"},{"line_number":372,"context_line":"            self.add_request({\u0027type\u0027: ovn_const.REQ_TYPE_HANDLE_VIP_FIP,"},{"line_number":373,"context_line":"                              \u0027info\u0027: request_info})"},{"line_number":374,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"aebcda54_2394aab0","line":371,"range":{"start_line":370,"start_character":3,"end_line":371,"end_character":43},"updated":"2023-08-21 14:29:51.000000000","message":"why not having this directly on the request_info at 364?","commit_id":"64e307f2bda8e1d51e7a35b0c983382d618097ec"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"e5ca37341493f3e604134a6d23fd3ab1f9848855","unresolved":false,"context_lines":[{"line_number":367,"context_line":"            if port:"},{"line_number":368,"context_line":"                request_info[\u0027vip_related\u0027] \u003d ["},{"line_number":369,"context_line":"                    ip[\u0027ip_address\u0027] for ip in port.fixed_ips]"},{"line_number":370,"context_line":"            request_info[\u0027additional_vip_fip\u0027] \u003d additional_vip"},{"line_number":371,"context_line":"            request_info[\u0027action\u0027] \u003d action"},{"line_number":372,"context_line":"            self.add_request({\u0027type\u0027: ovn_const.REQ_TYPE_HANDLE_VIP_FIP,"},{"line_number":373,"context_line":"                              \u0027info\u0027: request_info})"},{"line_number":374,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"06f1ffcc_07df9636","line":371,"range":{"start_line":370,"start_character":3,"end_line":371,"end_character":43},"in_reply_to":"aebcda54_2394aab0","updated":"2023-08-24 15:37:42.000000000","message":"Done","commit_id":"64e307f2bda8e1d51e7a35b0c983382d618097ec"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"5323f4d4fa9e1f1ea6a390d28e4be59b3d2afe99","unresolved":true,"context_lines":[{"line_number":2413,"context_line":"        except openstack.exceptions.HttpException as e:"},{"line_number":2414,"context_line":"            # NOTE (froyo): whatever other exception as e.g. Timeout"},{"line_number":2415,"context_line":"            # we should try to ensure no leftover port remains"},{"line_number":2416,"context_line":"            port \u003d self._neutron_find_port("},{"line_number":2417,"context_line":"                neutron_client,"},{"line_number":2418,"context_line":"                network_id\u003dvip_d[constants.VIP_NETWORK_ID],"},{"line_number":2419,"context_line":"                name_or_id\u003df\u0027{ovn_const.LB_VIP_PORT_PREFIX}{lb_id}\u0027)"},{"line_number":2420,"context_line":"            if port:"},{"line_number":2421,"context_line":"                LOG.debug(\u0027Leftover port %s has been found. Trying to \u0027"},{"line_number":2422,"context_line":"                          \u0027delete it\u0027, port.id)"},{"line_number":2423,"context_line":"                self.delete_port(port.id)"},{"line_number":2424,"context_line":""},{"line_number":2425,"context_line":"            for index, additional_vip in enumerate(additional_vip_ports,"},{"line_number":2426,"context_line":"                                                   start\u003d1):"},{"line_number":2427,"context_line":"                port \u003d self._neutron_find_port("},{"line_number":2428,"context_line":"                    neutron_client,"},{"line_number":2429,"context_line":"                    network_id\u003dadditional_vip.network_id,"},{"line_number":2430,"context_line":"                    name_or_id\u003d("},{"line_number":2431,"context_line":"                        f\u0027{ovn_const.LB_VIP_ADDIT_PORT_PREFIX}{index}-{lb_id}\u0027"},{"line_number":2432,"context_line":"                    ))"},{"line_number":2433,"context_line":"                if port:"},{"line_number":2434,"context_line":"                    LOG.debug(\u0027Leftover port %s has been found. Trying to \u0027"},{"line_number":2435,"context_line":"                              \u0027delete it\u0027, port.id)"},{"line_number":2436,"context_line":"                    self.delete_port(port.id)"},{"line_number":2437,"context_line":"            raise e"},{"line_number":2438,"context_line":""},{"line_number":2439,"context_line":"    @tenacity.retry("}],"source_content_type":"text/x-python","patch_set":15,"id":"588431e0_5167a061","line":2436,"range":{"start_line":2416,"start_character":0,"end_line":2436,"end_character":45},"updated":"2023-08-21 14:29:51.000000000","message":"wondering if we can use the information already at vip_port and additional_vip_dicts to remove the ports, instead of having to do the extra finds","commit_id":"64e307f2bda8e1d51e7a35b0c983382d618097ec"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"41c3e605908f23fd0849d6a5d86ce4e5738ced30","unresolved":false,"context_lines":[{"line_number":2413,"context_line":"        except openstack.exceptions.HttpException as e:"},{"line_number":2414,"context_line":"            # NOTE (froyo): whatever other exception as e.g. Timeout"},{"line_number":2415,"context_line":"            # we should try to ensure no leftover port remains"},{"line_number":2416,"context_line":"            port \u003d self._neutron_find_port("},{"line_number":2417,"context_line":"                neutron_client,"},{"line_number":2418,"context_line":"                network_id\u003dvip_d[constants.VIP_NETWORK_ID],"},{"line_number":2419,"context_line":"                name_or_id\u003df\u0027{ovn_const.LB_VIP_PORT_PREFIX}{lb_id}\u0027)"},{"line_number":2420,"context_line":"            if port:"},{"line_number":2421,"context_line":"                LOG.debug(\u0027Leftover port %s has been found. Trying to \u0027"},{"line_number":2422,"context_line":"                          \u0027delete it\u0027, port.id)"},{"line_number":2423,"context_line":"                self.delete_port(port.id)"},{"line_number":2424,"context_line":""},{"line_number":2425,"context_line":"            for index, additional_vip in enumerate(additional_vip_ports,"},{"line_number":2426,"context_line":"                                                   start\u003d1):"},{"line_number":2427,"context_line":"                port \u003d self._neutron_find_port("},{"line_number":2428,"context_line":"                    neutron_client,"},{"line_number":2429,"context_line":"                    network_id\u003dadditional_vip.network_id,"},{"line_number":2430,"context_line":"                    name_or_id\u003d("},{"line_number":2431,"context_line":"                        f\u0027{ovn_const.LB_VIP_ADDIT_PORT_PREFIX}{index}-{lb_id}\u0027"},{"line_number":2432,"context_line":"                    ))"},{"line_number":2433,"context_line":"                if port:"},{"line_number":2434,"context_line":"                    LOG.debug(\u0027Leftover port %s has been found. Trying to \u0027"},{"line_number":2435,"context_line":"                              \u0027delete it\u0027, port.id)"},{"line_number":2436,"context_line":"                    self.delete_port(port.id)"},{"line_number":2437,"context_line":"            raise e"},{"line_number":2438,"context_line":""},{"line_number":2439,"context_line":"    @tenacity.retry("}],"source_content_type":"text/x-python","patch_set":15,"id":"4e9f2854_492a3bd5","line":2436,"range":{"start_line":2416,"start_character":0,"end_line":2436,"end_character":45},"in_reply_to":"588431e0_5167a061","updated":"2023-08-24 16:07:50.000000000","message":"Indeed we can, because delete_port has the retry for the HttpException but catch the ResourceNotFound exception, in that case just some warn lines in the log.","commit_id":"64e307f2bda8e1d51e7a35b0c983382d618097ec"}]}