)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"19796f7ecacb3d63a065adafe6fd257284d81af7","unresolved":true,"context_lines":[{"line_number":23,"context_line":""},{"line_number":24,"context_line":"This patch is also removing the constraint that prevented the"},{"line_number":25,"context_line":"migration if the hybrid firewall was used."},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"Signed-off-by: Daniel Alvarez Sanchez \u003cdalvarez@redhat.com\u003e"},{"line_number":28,"context_line":"Change-Id: Iad4fae7af54cc502ac0ba02a911cdd4fefa13535"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"c7d9e5e8_2357b9dd","line":26,"updated":"2022-04-14 17:53:42.000000000","message":"surely this shoudl have at least a bug to tack this?\n\nthis is really a feature so i woudl expect an rfe bug or sepcless blueprint depending on what neutron uses to track feature.\n\nthere are upgrade impact ofcouse but im not sure this riase to the level of needing a spec but it does need to have something to track it so we can comunicate this to operators clearly in the release notes and upgrade docs.","commit_id":"f09c2f1144e4e26eb822eed73ced2d043e49a90e"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"aa30c63a736ad743fcca0dd0c7a8c75312d24c3c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"fea59c6e_69ad1705","updated":"2022-04-12 13:45:10.000000000","message":"I think tests need to be udpated too","commit_id":"c2785e41311e618873b53e229fc12ca16d93a942"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"63e5404647fcb414321cf98569bd915217f8e44d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"9439541c_ecb9bdb7","updated":"2022-04-12 15:54:15.000000000","message":"\nill try and review this later\n\nbut i guess one reservation i have is do we have testing for this in ci and more importantly\ncan we ensure that after the change to ovn that a live migration will result in also migrating form hybrid_plug\u003dtrue to hybrid_plug\u003dfalse\n\n\nthis wont work on cold migration since that has only a single port bidnign so we cant update the toplogy for a cold migration but we can for live migration since we have two port bidings.\n","commit_id":"daa2ae3f1f04bed5775ca5069d7c3e7f67d65bbb"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"d2a38bebae52724d97672089d2307836df628f96","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"ccfaff2c_a426d6b9","updated":"2022-04-13 09:53:44.000000000","message":"I didn\u0027t really test whole OVS-\u003eOVN migration with this patch but I manually added removed binding details to the existing ports and then tried to restart Vms and it worked fine.\n\nThis problem should be solved with this patch as in that way ports would be still plugged in hybrid way after e.g. reboot of vm so there will be always only one qvoXXX port in br-int (tapXXX will be in the linuxbridge qbrXXX). OVN will not care if it\u0027s tap or qvo in the br-int and all will work just as good as before migration.\nThis can be later \"fixed\" by doing e.g. live migration of the instances (excactly like we already advice users to do during change from iptables_hybrid to ovs fw driver in ML2/OVS case).\n","commit_id":"bf18f1feecb2bd7b630aeff7ace8317f5ac4eb67"},{"author":{"_account_id":23804,"name":"Daniel Alvarez","email":"dalvarez@redhat.com","username":"dalvarez"},"change_message_id":"663122fa438e9089da77b61d82240a9fe104d89c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"263cea7d_84dd59f3","updated":"2022-04-13 08:19:56.000000000","message":"recheck","commit_id":"bf18f1feecb2bd7b630aeff7ace8317f5ac4eb67"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"6e8adb48051cacaf5428ca49b826911d5d8750ce","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"7ae2a3a6_8feb50d6","in_reply_to":"ccfaff2c_a426d6b9","updated":"2022-04-13 11:22:03.000000000","message":"yep however before we merge this we need to ensure that they hybrid plug flag does not change wehn we do cold migrate or it may break subsequent hard reboots.\n\nbasically if the binding detail are updated to set hybrid_plug\u003dfalse\nwhen we update the binding:host-id to the destiation host that happens after\nwe have gerated the xml for the destion host so the vm will be created with the\nhybrid bridge.\n\nif nova network info cache is then updated with this info the next hard reboot will try to clean up as if we had booted with hybrid_plug\u003dfalse and it will leak the bridge as it will jsut try to remove the ovs port.\n\nso we need to ensure that we have tested that before we merge this as the only time we can change this is wehn creating a prot bidnign for the rist tiem like when we create an inactive port binding for the destion host using the multiple port bidnigs workflow used for live migrtation.\n\nthat process happen before the xml is generated and allow  nova ot clean up the source node properly as it has both sets of data.\n\nwe could perhaps close this gap in os-vif and have it check if there are bidges to clean up but by default i expect this not to work properly for cold migration so we shoudl ensure it does not change on the neutron side.\n\nperhaps we could use the presence of migrating too or the fact that there is only one port bindign to detect this?\n\nin any case i think it would be good to at least test that config.\ncan we do that with fullstack or tempest?","commit_id":"bf18f1feecb2bd7b630aeff7ace8317f5ac4eb67"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"0d329410b647cc00c8a3caa27643b939f4920f8f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"6eb19b04_1f63035d","updated":"2022-04-14 17:51:11.000000000","message":"-1 since there are no unit or fucntional test to validate the behavior and\nthe patch also does not contain a release note or doc to descibe the change\nin behavior to operators that will consume this.\n\n\nat a minium the ovn migration readme \nhttps://github.com/openstack/neutron/blob/master/tools/ovn_migration/README.rst\n\nneed to be updated to alter the Prerequisites\nbut i think this patch need a much greater level of testing before it is mergable","commit_id":"f09c2f1144e4e26eb822eed73ced2d043e49a90e"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"49f074cb24dd2b8467f81b75f3332b04027a74a4","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"5db7f496_fdf7cc69","updated":"2022-04-13 21:56:00.000000000","message":"recheck","commit_id":"f09c2f1144e4e26eb822eed73ced2d043e49a90e"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"023ce4a170593342bf3268e6d27b8b81dea71d24","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"b63a8636_792a042d","in_reply_to":"6eb19b04_1f63035d","updated":"2022-04-14 18:18:43.000000000","message":"The UTs are at https://review.opendev.org/c/openstack/neutron/+/837566/4/neutron/tests/unit/plugins/ml2/drivers/ovn/test_db_migration.py\n\nIt\u0027s a good point we should remove the limitation however it\u0027s not been tested well yet to claim iptables always work. We\u0027ve already found some things that don\u0027t work well - like e.g. cold migration.\n\nIdeally we should have an upstream CI that executes the migration which we lack for a long time already.","commit_id":"f09c2f1144e4e26eb822eed73ced2d043e49a90e"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"56e4e82fda957d8be284b77c88308e9454a7caff","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"5d27baa3_18c5260b","in_reply_to":"b63a8636_792a042d","updated":"2022-04-16 19:49:41.000000000","message":"yes so it likely shoudl be put in place before this is merged no?\n\nthis is very early in the zed cycle so we have time for that to get put in place so we dont need to rush to make this change.\n\ni think fixing things like cold migration need to be done before moving forward with this. at the least having a series of patches that will address the issues proposed and accompanying documentation.","commit_id":"f09c2f1144e4e26eb822eed73ced2d043e49a90e"}],"neutron/plugins/ml2/drivers/ovn/db_migration.py":[{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"978103508e98ce13edc847c51682175e4f11e5a9","unresolved":true,"context_lines":[{"line_number":31,"context_line":""},{"line_number":32,"context_line":"VIF_DETAILS_TO_REMOVE \u003d ("},{"line_number":33,"context_line":"    pb_api.OVS_HYBRID_PLUG,"},{"line_number":34,"context_line":"    pb_api.VIF_DETAILS_BRIDGE_NAME)"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"def migrate_neutron_database_to_ovn():"}],"source_content_type":"text/x-python","patch_set":4,"id":"f2acfe11_b4fdc5be","side":"PARENT","line":34,"updated":"2022-04-14 17:01:28.000000000","message":"It looks like this breaks trunk","commit_id":"1fa2e49f01e0f6779c396b8d22acd97a1e9baa72"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"462cb5e9757f57554a489ec504df7335f5a097f2","unresolved":false,"context_lines":[{"line_number":31,"context_line":""},{"line_number":32,"context_line":"VIF_DETAILS_TO_REMOVE \u003d ("},{"line_number":33,"context_line":"    pb_api.OVS_HYBRID_PLUG,"},{"line_number":34,"context_line":"    pb_api.VIF_DETAILS_BRIDGE_NAME)"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"def migrate_neutron_database_to_ovn():"}],"source_content_type":"text/x-python","patch_set":4,"id":"5bcf451f_80c7b11b","side":"PARENT","line":34,"in_reply_to":"b95e9bb7_57f360fd","updated":"2022-04-19 15:03:04.000000000","message":"Done","commit_id":"1fa2e49f01e0f6779c396b8d22acd97a1e9baa72"},{"author":{"_account_id":23804,"name":"Daniel Alvarez","email":"dalvarez@redhat.com","username":"dalvarez"},"change_message_id":"dcda33e8e9474ea436c3923782faee53ce38ec0d","unresolved":true,"context_lines":[{"line_number":31,"context_line":""},{"line_number":32,"context_line":"VIF_DETAILS_TO_REMOVE \u003d ("},{"line_number":33,"context_line":"    pb_api.OVS_HYBRID_PLUG,"},{"line_number":34,"context_line":"    pb_api.VIF_DETAILS_BRIDGE_NAME)"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"def migrate_neutron_database_to_ovn():"}],"source_content_type":"text/x-python","patch_set":4,"id":"b95e9bb7_57f360fd","side":"PARENT","line":34,"in_reply_to":"f2acfe11_b4fdc5be","updated":"2022-04-18 08:39:23.000000000","message":"shall we then remove the bridge name from the details but keep the hybrid_plug?\nI\u0027ll wait until we have more data from our testing","commit_id":"1fa2e49f01e0f6779c396b8d22acd97a1e9baa72"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"6537f6e901835faa93ae077d9250d0345ae3b8f9","unresolved":true,"context_lines":[{"line_number":31,"context_line":""},{"line_number":32,"context_line":"VIF_DETAILS_TO_REMOVE \u003d ("},{"line_number":33,"context_line":"    pb_api.OVS_HYBRID_PLUG,"},{"line_number":34,"context_line":"}"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"def migrate_neutron_database_to_ovn():"},{"line_number":37,"context_line":"    \"\"\"Change DB content from OVS to OVN mech driver."}],"source_content_type":"text/x-python","patch_set":5,"id":"bba17743_0084827b","line":34,"updated":"2022-04-19 15:03:47.000000000","message":"Argh","commit_id":"d158bf08a8bc5275aa1721e6bef63b85011e3e4c"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"348029d21df34890a65c082b69a1d77bb0a6137d","unresolved":false,"context_lines":[{"line_number":31,"context_line":""},{"line_number":32,"context_line":"VIF_DETAILS_TO_REMOVE \u003d ("},{"line_number":33,"context_line":"    pb_api.OVS_HYBRID_PLUG,"},{"line_number":34,"context_line":"}"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"def migrate_neutron_database_to_ovn():"},{"line_number":37,"context_line":"    \"\"\"Change DB content from OVS to OVN mech driver."}],"source_content_type":"text/x-python","patch_set":5,"id":"c3c7b305_87b95733","line":34,"in_reply_to":"bba17743_0084827b","updated":"2022-04-19 16:31:56.000000000","message":"Done","commit_id":"d158bf08a8bc5275aa1721e6bef63b85011e3e4c"}],"tools/ovn_migration/tripleo_environment/playbooks/roles/pre-checks/ovn-controllers/tasks/main.yml":[{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"021fa4f93954553b9e498746d3e24d90ba0869e1","unresolved":true,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"- name: Read OVS configuration file and extract \"firewall_driver\" variable."},{"line_number":3,"context_line":"  set_fact:"},{"line_number":4,"context_line":"    firewall_driver: \"{{ lookup(\u0027ini\u0027, \u0027firewall_driver section\u003dsecuritygroup file\u003d/var/lib/config-data/puppet-generated/neutron/etc/neutron/plugins/ml2/openvswitch_agent.ini\u0027, allow_no_value\u003dTrue) }}\""}],"source_content_type":"text/x-yaml","patch_set":1,"id":"7e5396b3_2eb40de8","side":"PARENT","line":1,"updated":"2022-04-12 13:32:05.000000000","message":"Was just the file deleted or also the whole directory pre-checks? Gerrit shows just the file","commit_id":"1fa2e49f01e0f6779c396b8d22acd97a1e9baa72"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"2032cca1bbd23e3f68d499f879a36c9bca2befd4","unresolved":false,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"- name: Read OVS configuration file and extract \"firewall_driver\" variable."},{"line_number":3,"context_line":"  set_fact:"},{"line_number":4,"context_line":"    firewall_driver: \"{{ lookup(\u0027ini\u0027, \u0027firewall_driver section\u003dsecuritygroup file\u003d/var/lib/config-data/puppet-generated/neutron/etc/neutron/plugins/ml2/openvswitch_agent.ini\u0027, allow_no_value\u003dTrue) }}\""}],"source_content_type":"text/x-yaml","patch_set":1,"id":"ed3c5a25_71f91423","side":"PARENT","line":1,"in_reply_to":"7e5396b3_2eb40de8","updated":"2022-04-12 13:34:18.000000000","message":"Ok, it\u0027s just not shown.","commit_id":"1fa2e49f01e0f6779c396b8d22acd97a1e9baa72"},{"author":{"_account_id":23804,"name":"Daniel Alvarez","email":"dalvarez@redhat.com","username":"dalvarez"},"change_message_id":"a213c79d525cff5bc30880eb4cc9b2787cf3df69","unresolved":false,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"- name: Read OVS configuration file and extract \"firewall_driver\" variable."},{"line_number":3,"context_line":"  set_fact:"},{"line_number":4,"context_line":"    firewall_driver: \"{{ lookup(\u0027ini\u0027, \u0027firewall_driver section\u003dsecuritygroup file\u003d/var/lib/config-data/puppet-generated/neutron/etc/neutron/plugins/ml2/openvswitch_agent.ini\u0027, allow_no_value\u003dTrue) }}\""}],"source_content_type":"text/x-yaml","patch_set":1,"id":"fcb2cbd4_2be2ea32","side":"PARENT","line":1,"in_reply_to":"ed3c5a25_71f91423","updated":"2022-04-12 13:35:23.000000000","message":"Yeah that confused me too but I did:\n\n$ git rm -r ./tripleo_environment/playbooks/roles/pre-checks/","commit_id":"1fa2e49f01e0f6779c396b8d22acd97a1e9baa72"}]}
