)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"45f2b86badeebfe60ff40549eb8ba9cccf1323c1","unresolved":false,"context_lines":[{"line_number":16,"context_line":""},{"line_number":17,"context_line":"The bug that this patch addresses was being hit in the gate with"},{"line_number":18,"context_line":"around a 30% ratio because the maintenance task was fixing it while"},{"line_number":19,"context_line":"tempest was still rying to SSH into it."},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"Change-Id: Icebf4a82f64989112c3ca810b4358de490108c2d"},{"line_number":22,"context_line":"Closes-Bug: #1835029"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"7faddb67_efd44deb","line":19,"range":{"start_line":19,"start_character":18,"end_line":19,"end_character":23},"updated":"2019-07-15 15:41:55.000000000","message":"trying","commit_id":"f4e1b837de1497bcea7f35f4510bcb0a195e86b2"},{"author":{"_account_id":27295,"name":"Li Zhouzhou","email":"lizhouzhou_yewu@cmss.chinamobile.com","username":"LiZhouzhou"},"change_message_id":"b6f3ac72ec93467ea4c5b89b860e7769ebe891cd","unresolved":false,"context_lines":[{"line_number":20,"context_line":""},{"line_number":21,"context_line":"Depends-On: https://review.opendev.org/#/c/670877/"},{"line_number":22,"context_line":"Change-Id: Icebf4a82f64989112c3ca810b4358de490108c2d"},{"line_number":23,"context_line":"Closes-Bug: #1835029"},{"line_number":24,"context_line":"Co-Authored-By: Jakub Libosvar \u003cjlibosva@redhat.com\u003e"},{"line_number":25,"context_line":"Signed-off-by: Daniel Alvarez \u003cdalvarez@redhat.com\u003e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"7faddb67_58f66d65","line":23,"updated":"2019-07-17 09:19:39.000000000","message":"Related Bug: 1833820 should be added.","commit_id":"b2cdfda7feadb88f460924aaa92b97a0d28aa1a9"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"8ed3b6e1b6347b2e77708854738c5bcc4e6ba2ac","unresolved":false,"context_lines":[{"line_number":18,"context_line":"around a 30% ratio because the maintenance task was fixing it while"},{"line_number":19,"context_line":"tempest was still rying to SSH into it."},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"Depends-On: https://review.opendev.org/#/c/670877/"},{"line_number":22,"context_line":"Change-Id: Icebf4a82f64989112c3ca810b4358de490108c2d"},{"line_number":23,"context_line":"Closes-Bug: #1835029"},{"line_number":24,"context_line":"Closes-Bug: #1833820"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"7faddb67_e7475f76","line":21,"updated":"2019-07-18 12:01:14.000000000","message":"I know it\u0027s a nice to have, but I\u0027ve hit so many times the problem that this patch is fixing in the gate that I was wondering if we could lift this depends-on here.\n\nWhat you guys think ? Should we remove it and merge this patch ?","commit_id":"a49901ea24d89db62d2f5fd6950f91f2b2a5511d"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"eef14dfb377a957a6b32fd4170be7043a68bd5f4","unresolved":false,"context_lines":[{"line_number":18,"context_line":"around a 30% ratio because the maintenance task was fixing it while"},{"line_number":19,"context_line":"tempest was still rying to SSH into it."},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"Depends-On: https://review.opendev.org/#/c/670877/"},{"line_number":22,"context_line":"Change-Id: Icebf4a82f64989112c3ca810b4358de490108c2d"},{"line_number":23,"context_line":"Closes-Bug: #1835029"},{"line_number":24,"context_line":"Closes-Bug: #1833820"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"7faddb67_472d13a6","line":21,"in_reply_to":"7faddb67_e7475f76","updated":"2019-07-18 12:03:10.000000000","message":"Makes sense. Also it should be probably the other way around cause if we merge 670877 first we\u0027ll get high rate of failures until this patch is merged.","commit_id":"a49901ea24d89db62d2f5fd6950f91f2b2a5511d"}],"networking_ovn/common/ovn_client.py":[{"author":{"_account_id":27295,"name":"Li Zhouzhou","email":"lizhouzhou_yewu@cmss.chinamobile.com","username":"LiZhouzhou"},"change_message_id":"214a7d1ccc4642a2df70cf0803e7f4574e14e772","unresolved":false,"context_lines":[{"line_number":662,"context_line":"        if self._nb_idl.is_col_present(\u0027NAT\u0027, \u0027external_ids\u0027):"},{"line_number":663,"context_line":"            columns[\u0027external_ids\u0027] \u003d ext_ids"},{"line_number":664,"context_line":""},{"line_number":665,"context_line":"        commands.append(self._nb_idl.add_nat_rule_in_lrouter(gw_lrouter_name,"},{"line_number":666,"context_line":"                                                             **columns))"},{"line_number":667,"context_line":"        self._transaction(commands, txn\u003dtxn)"},{"line_number":668,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"7faddb67_e4b0d6df","line":665,"range":{"start_line":665,"start_character":37,"end_line":665,"end_character":60},"updated":"2019-07-17 01:36:53.000000000","message":"For this bug: https://launchpad.net/bugs/1833820\nThe original code need gets the lrouter_nat_name and set a new value. But the \u0027update_fip\u0027 has a \u0027_delete_fip\u0027, it causes the lrouter_nat_name be lost, so update operator failed.\nNow, you have modified the method \u0027_create_or_update_floatingip\u0027 and it works as we expected. However, the \u0027update_fip\u0027 method also confused me, see below:","commit_id":"b2cdfda7feadb88f460924aaa92b97a0d28aa1a9"},{"author":{"_account_id":27295,"name":"Li Zhouzhou","email":"lizhouzhou_yewu@cmss.chinamobile.com","username":"LiZhouzhou"},"change_message_id":"214a7d1ccc4642a2df70cf0803e7f4574e14e772","unresolved":false,"context_lines":[{"line_number":730,"context_line":"                    ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY,"},{"line_number":731,"context_line":"                    utils.ovn_name(router_id))"},{"line_number":732,"context_line":""},{"line_number":733,"context_line":"                self._delete_floatingip(ovn_fip, lrouter, txn\u003dtxn)"},{"line_number":734,"context_line":"                fip_status \u003d const.FLOATINGIP_STATUS_DOWN"},{"line_number":735,"context_line":""},{"line_number":736,"context_line":"            if floatingip.get(\u0027port_id\u0027):"}],"source_content_type":"text/x-python","patch_set":3,"id":"7faddb67_64c4e640","line":733,"range":{"start_line":733,"start_character":21,"end_line":733,"end_character":39},"updated":"2019-07-17 01:36:53.000000000","message":"Why we should \u0027_delete_fip\u0027 and then \u0027_update_fip\u0027?\nIMO, first, \u0027_update_fip\u0027 is enough, OVN could update a new value for NAT.\n\nSecond, \u0027_delete_fip\u0027 and \u0027_update_fip\u0027 are two transaction, why we put it together?","commit_id":"b2cdfda7feadb88f460924aaa92b97a0d28aa1a9"},{"author":{"_account_id":23804,"name":"Daniel Alvarez","email":"dalvarez@redhat.com","username":"dalvarez"},"change_message_id":"8331b3d67d24c1d0348b93a1374bf5d85c52541c","unresolved":false,"context_lines":[{"line_number":730,"context_line":"                    ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY,"},{"line_number":731,"context_line":"                    utils.ovn_name(router_id))"},{"line_number":732,"context_line":""},{"line_number":733,"context_line":"                self._delete_floatingip(ovn_fip, lrouter, txn\u003dtxn)"},{"line_number":734,"context_line":"                fip_status \u003d const.FLOATINGIP_STATUS_DOWN"},{"line_number":735,"context_line":""},{"line_number":736,"context_line":"            if floatingip.get(\u0027port_id\u0027):"}],"source_content_type":"text/x-python","patch_set":3,"id":"7faddb67_ad588187","line":733,"range":{"start_line":733,"start_character":21,"end_line":733,"end_character":39},"in_reply_to":"7faddb67_64c4e640","updated":"2019-07-17 08:15:44.000000000","message":"Thanks Li for taking a look.\n\nFor the 1st question:\nThere\u0027s a strong reference in the OVN NB schema in the LR table for the NAT entries [0]. This means, you cannot move a NAT entry from one router to assign it to another one. If you remove the NAT entry from a Logical Router, this NAT entry will go away from the NAT table. Hence, you\u0027d need to recreate.\n\nThis is maybe something that we don\u0027t need to do all the time but only when moving a FIP to a different port, which is the bug that this patch is addressing. As this port can be in a network that is connected to a different router, we\u0027ll need to delete, then update (actually create :).\n\nFor the 2nd question:\nThe reason for having everything under the same transaction is that this comes all from a single API call (updating a FIP). We map Neutron objects (FIP) to OVN objects (NAT) and we use revision numbers to ensure consistency.\nSometimes, updating an OVN object requires different commands to the OVN NB database but we consolidate all those commands into a single transaction to make sure that the OVN state reflects the desired state in Neutron. If one single command fails, then the whole transaction will fail and we won\u0027t bump the revision number, hence creating an inconsistency that will be later fixed by the periodic maintenance task. More on this at [1].\n\nI hope it helps :)\n\n[0] https://github.com/openvswitch/ovs/blob/v2.11.0/ovn/ovn-nb.ovsschema#L248\n[1] https://docs.openstack.org/networking-ovn/latest/contributor/design/database_consistency.html","commit_id":"b2cdfda7feadb88f460924aaa92b97a0d28aa1a9"},{"author":{"_account_id":27295,"name":"Li Zhouzhou","email":"lizhouzhou_yewu@cmss.chinamobile.com","username":"LiZhouzhou"},"change_message_id":"a1e277ed517caa58ab288cbbe9c24f97f9fd1100","unresolved":false,"context_lines":[{"line_number":730,"context_line":"                    ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY,"},{"line_number":731,"context_line":"                    utils.ovn_name(router_id))"},{"line_number":732,"context_line":""},{"line_number":733,"context_line":"                self._delete_floatingip(ovn_fip, lrouter, txn\u003dtxn)"},{"line_number":734,"context_line":"                fip_status \u003d const.FLOATINGIP_STATUS_DOWN"},{"line_number":735,"context_line":""},{"line_number":736,"context_line":"            if floatingip.get(\u0027port_id\u0027):"}],"source_content_type":"text/x-python","patch_set":3,"id":"7faddb67_388db131","line":733,"range":{"start_line":733,"start_character":21,"end_line":733,"end_character":39},"in_reply_to":"7faddb67_ad588187","updated":"2019-07-17 09:01:58.000000000","message":"Get it. It\u0027s very clear.\n\nThanks Daniel for explaining it.","commit_id":"b2cdfda7feadb88f460924aaa92b97a0d28aa1a9"}]}