)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"f9722669a53351a13444cd886256c59364072bb8","unresolved":false,"context_lines":[{"line_number":14,"context_line":""},{"line_number":15,"context_line":"This is all well and good if it\u0027s the original compute host that comes"},{"line_number":16,"context_line":"back after being evacuated, but what if it\u0027s a brand new compute host"},{"line_number":17,"context_line":"with the same host name?"},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"It\u0027ll find the \u0027done\u0027 evacuations, look for the associated libvirt"},{"line_number":20,"context_line":"instances to destroy on the hypervisor, not find any because it\u0027s a"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"c4f82fcd_9e6a1cad","line":17,"updated":"2022-05-10 07:45:42.000000000","message":"so the old replaced host was never recovered to do the done -\u003e complete transition of the migrations. Nice.","commit_id":"d6de7b172209b9acd6a811c24b263e5433c70bfa"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"4e15030e6626db71b969f91553ed2075dcf00c50","unresolved":false,"context_lines":[{"line_number":14,"context_line":""},{"line_number":15,"context_line":"This is all well and good if it\u0027s the original compute host that comes"},{"line_number":16,"context_line":"back after being evacuated, but what if it\u0027s a brand new compute host"},{"line_number":17,"context_line":"with the same host name?"},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"It\u0027ll find the \u0027done\u0027 evacuations, look for the associated libvirt"},{"line_number":20,"context_line":"instances to destroy on the hypervisor, not find any because it\u0027s a"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"f61f705f_b1091da6","line":17,"in_reply_to":"c4f82fcd_9e6a1cad","updated":"2022-05-10 11:51:42.000000000","message":"this type of server replacment with the same hostname is not supproted today.\n\nwe basically expect that if the motheroard explodes you will reuse the same SSDs and data or use a different hostname.","commit_id":"d6de7b172209b9acd6a811c24b263e5433c70bfa"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"f9722669a53351a13444cd886256c59364072bb8","unresolved":false,"context_lines":[{"line_number":19,"context_line":"It\u0027ll find the \u0027done\u0027 evacuations, look for the associated libvirt"},{"line_number":20,"context_line":"instances to destroy on the hypervisor, not find any because it\u0027s a"},{"line_number":21,"context_line":"brand new compute, and I suspect at this point it will not set the"},{"line_number":22,"context_line":"migration record to \u0027completed\u0027."},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"If previously-evacuated instances are now migrated back to the new"},{"line_number":25,"context_line":"(but with the old hostname) compute, and the nova-compute service is"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"5e099198_bd001117","line":22,"updated":"2022-05-10 07:45:42.000000000","message":"Hm, that is a bug. :) If the goal is to destroy instances but those instances are not exists then the goal is achieved. :D Probably nova just forgot to do the paperwork.","commit_id":"d6de7b172209b9acd6a811c24b263e5433c70bfa"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"4e15030e6626db71b969f91553ed2075dcf00c50","unresolved":false,"context_lines":[{"line_number":19,"context_line":"It\u0027ll find the \u0027done\u0027 evacuations, look for the associated libvirt"},{"line_number":20,"context_line":"instances to destroy on the hypervisor, not find any because it\u0027s a"},{"line_number":21,"context_line":"brand new compute, and I suspect at this point it will not set the"},{"line_number":22,"context_line":"migration record to \u0027completed\u0027."},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"If previously-evacuated instances are now migrated back to the new"},{"line_number":25,"context_line":"(but with the old hostname) compute, and the nova-compute service is"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"be1e57bb_5bdcfaef","line":22,"in_reply_to":"5e099198_bd001117","updated":"2022-05-10 11:51:42.000000000","message":"well i agree this is certenly something we could add supprot for.\n\neffectivly the end goal is to ensure the instance is not present on the current host. so i think its valid to see that its not defiend in libvirt and then mark it as complete if its currently in the done state.\n\nhowever i dont realy think this is a bug.","commit_id":"d6de7b172209b9acd6a811c24b263e5433c70bfa"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"f9722669a53351a13444cd886256c59364072bb8","unresolved":false,"context_lines":[{"line_number":24,"context_line":"If previously-evacuated instances are now migrated back to the new"},{"line_number":25,"context_line":"(but with the old hostname) compute, and the nova-compute service is"},{"line_number":26,"context_line":"restarted, it\u0027ll pick up those \u0027done\u0027 migration records and destroy"},{"line_number":27,"context_line":"the libvirt instances."},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"Reproduce all of this in a functional test."},{"line_number":30,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"150cf42b_be48b71f","line":27,"updated":"2022-05-10 07:45:42.000000000","message":"And that is the magic fallout :mushroomcloud:","commit_id":"d6de7b172209b9acd6a811c24b263e5433c70bfa"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"4e15030e6626db71b969f91553ed2075dcf00c50","unresolved":false,"context_lines":[{"line_number":24,"context_line":"If previously-evacuated instances are now migrated back to the new"},{"line_number":25,"context_line":"(but with the old hostname) compute, and the nova-compute service is"},{"line_number":26,"context_line":"restarted, it\u0027ll pick up those \u0027done\u0027 migration records and destroy"},{"line_number":27,"context_line":"the libvirt instances."},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"Reproduce all of this in a functional test."},{"line_number":30,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"86e4f9ac_d742e851","line":27,"in_reply_to":"150cf42b_be48b71f","updated":"2022-05-10 11:51:42.000000000","message":"yes but i this is really a result of operator error.\n\nwhen they did the node replacemnt they did so incorrectly.\nthe should not use the same hostname if they intended it to be a \"new\" host.\n\nif they intended it to be the same host we woudl have expected them to perserve the data on the ssd.\n\nand yes im aware that if the ssd died and they had not backup or raid reducnecy they could not do that but i dont think its valid to consier the node replaced in that case until the migrations are cleaned up manually in the db in this case.\n\n\ni agree that it would be a nice enhancement to make nova handel this case so that the operator does not have too but i woudl consider the operaton the operator did to be unsupproted today.","commit_id":"d6de7b172209b9acd6a811c24b263e5433c70bfa"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"f9722669a53351a13444cd886256c59364072bb8","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"9e676f8e_05eb4263","updated":"2022-05-10 07:45:42.000000000","message":"Nice reproduction! I left some nits inline but nothing blocking","commit_id":"d6de7b172209b9acd6a811c24b263e5433c70bfa"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"dc8b2492b0b2a848c90d8d0af21b1c12d6f814f8","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"2c1c2d16_0f6e8ec9","updated":"2022-05-10 14:40:00.000000000","message":"ok im +2 on this also after fully reviewing it.\nim not sure if we want to wait for the fix before landing this\nso i wont set +w for now ","commit_id":"d6de7b172209b9acd6a811c24b263e5433c70bfa"}],"nova/tests/functional/libvirt/test_evacuate.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"f9722669a53351a13444cd886256c59364072bb8","unresolved":true,"context_lines":[{"line_number":538,"context_line":"        # Return fresh server state after evacuate"},{"line_number":539,"context_line":"        return server"},{"line_number":540,"context_line":""},{"line_number":541,"context_line":"    def test_evacuate_then_migrate(self):"},{"line_number":542,"context_line":"        # Start a compute."},{"line_number":543,"context_line":"        compute0 \u003d self._start_compute(\u0027compute0\u0027)"},{"line_number":544,"context_line":"        svc0 \u003d self.api.get_services(host\u003d\u0027compute0\u0027, binary\u003d\u0027nova-compute\u0027)[0]"}],"source_content_type":"text/x-python","patch_set":3,"id":"12e51ac2_2fb5e986","line":541,"range":{"start_line":541,"start_character":8,"end_line":541,"end_character":34},"updated":"2022-05-10 07:45:42.000000000","message":"nit: test_evacuate_redeploy_compute_migrate_back","commit_id":"d6de7b172209b9acd6a811c24b263e5433c70bfa"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"def90667cbf88b096b65eb4404611c079c30ff9b","unresolved":false,"context_lines":[{"line_number":538,"context_line":"        # Return fresh server state after evacuate"},{"line_number":539,"context_line":"        return server"},{"line_number":540,"context_line":""},{"line_number":541,"context_line":"    def test_evacuate_then_migrate(self):"},{"line_number":542,"context_line":"        # Start a compute."},{"line_number":543,"context_line":"        compute0 \u003d self._start_compute(\u0027compute0\u0027)"},{"line_number":544,"context_line":"        svc0 \u003d self.api.get_services(host\u003d\u0027compute0\u0027, binary\u003d\u0027nova-compute\u0027)[0]"}],"source_content_type":"text/x-python","patch_set":3,"id":"e59a5313_82a75822","line":541,"range":{"start_line":541,"start_character":8,"end_line":541,"end_character":34},"in_reply_to":"12e51ac2_2fb5e986","updated":"2022-05-10 16:36:13.000000000","message":"Done","commit_id":"d6de7b172209b9acd6a811c24b263e5433c70bfa"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"f9722669a53351a13444cd886256c59364072bb8","unresolved":true,"context_lines":[{"line_number":555,"context_line":""},{"line_number":556,"context_line":"        # Delete original compute record, and start a brand new one with the"},{"line_number":557,"context_line":"        # same hostname."},{"line_number":558,"context_line":"        self.api.api_delete(\u0027/os-services/%s\u0027 % svc0[\u0027id\u0027])"},{"line_number":559,"context_line":"        compute0 \u003d self._start_compute(\u0027compute0\u0027)"},{"line_number":560,"context_line":""},{"line_number":561,"context_line":"        # Make sure our new compute is empty before we migrate our instance"}],"source_content_type":"text/x-python","patch_set":3,"id":"da4be91d_2f87bf50","line":558,"updated":"2022-05-10 07:45:42.000000000","message":"I would assert that it returned positive response code just to be on the safe side","commit_id":"d6de7b172209b9acd6a811c24b263e5433c70bfa"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"29188bc7c885952ddb4a3515811ee810d9571fa7","unresolved":false,"context_lines":[{"line_number":555,"context_line":""},{"line_number":556,"context_line":"        # Delete original compute record, and start a brand new one with the"},{"line_number":557,"context_line":"        # same hostname."},{"line_number":558,"context_line":"        self.api.api_delete(\u0027/os-services/%s\u0027 % svc0[\u0027id\u0027])"},{"line_number":559,"context_line":"        compute0 \u003d self._start_compute(\u0027compute0\u0027)"},{"line_number":560,"context_line":""},{"line_number":561,"context_line":"        # Make sure our new compute is empty before we migrate our instance"}],"source_content_type":"text/x-python","patch_set":3,"id":"94239d08_19a3d530","line":558,"in_reply_to":"65472aca_e00c32d5","updated":"2022-05-10 13:02:16.000000000","message":"ohh cool then.","commit_id":"d6de7b172209b9acd6a811c24b263e5433c70bfa"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"fafc50e9390c779113e0aef88a41d9d86512e9a6","unresolved":true,"context_lines":[{"line_number":555,"context_line":""},{"line_number":556,"context_line":"        # Delete original compute record, and start a brand new one with the"},{"line_number":557,"context_line":"        # same hostname."},{"line_number":558,"context_line":"        self.api.api_delete(\u0027/os-services/%s\u0027 % svc0[\u0027id\u0027])"},{"line_number":559,"context_line":"        compute0 \u003d self._start_compute(\u0027compute0\u0027)"},{"line_number":560,"context_line":""},{"line_number":561,"context_line":"        # Make sure our new compute is empty before we migrate our instance"}],"source_content_type":"text/x-python","patch_set":3,"id":"65472aca_e00c32d5","line":558,"in_reply_to":"da4be91d_2f87bf50","updated":"2022-05-10 12:36:23.000000000","message":"this is asserted automatically\n\nhttps://github.com/openstack/nova/blob/972c06c608f0b00e9066d7f581fd81197065cf49/nova/tests/functional/api/client.py#L227-L230\u003d\n\n\u0027check_response_status\u0027, [200, 202, 204]","commit_id":"d6de7b172209b9acd6a811c24b263e5433c70bfa"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"f9722669a53351a13444cd886256c59364072bb8","unresolved":true,"context_lines":[{"line_number":556,"context_line":"        # Delete original compute record, and start a brand new one with the"},{"line_number":557,"context_line":"        # same hostname."},{"line_number":558,"context_line":"        self.api.api_delete(\u0027/os-services/%s\u0027 % svc0[\u0027id\u0027])"},{"line_number":559,"context_line":"        compute0 \u003d self._start_compute(\u0027compute0\u0027)"},{"line_number":560,"context_line":""},{"line_number":561,"context_line":"        # Make sure our new compute is empty before we migrate our instance"},{"line_number":562,"context_line":"        # onto it."}],"source_content_type":"text/x-python","patch_set":3,"id":"6c55fb84_4d23bcb9","line":559,"updated":"2022-05-10 07:45:42.000000000","message":"you can assert here that the migration record is not set to completed","commit_id":"d6de7b172209b9acd6a811c24b263e5433c70bfa"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"fafc50e9390c779113e0aef88a41d9d86512e9a6","unresolved":true,"context_lines":[{"line_number":556,"context_line":"        # Delete original compute record, and start a brand new one with the"},{"line_number":557,"context_line":"        # same hostname."},{"line_number":558,"context_line":"        self.api.api_delete(\u0027/os-services/%s\u0027 % svc0[\u0027id\u0027])"},{"line_number":559,"context_line":"        compute0 \u003d self._start_compute(\u0027compute0\u0027)"},{"line_number":560,"context_line":""},{"line_number":561,"context_line":"        # Make sure our new compute is empty before we migrate our instance"},{"line_number":562,"context_line":"        # onto it."}],"source_content_type":"text/x-python","patch_set":3,"id":"8a156cff_1aa063f6","line":559,"in_reply_to":"6c55fb84_4d23bcb9","updated":"2022-05-10 12:36:23.000000000","message":"yep that would be good to add","commit_id":"d6de7b172209b9acd6a811c24b263e5433c70bfa"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"def90667cbf88b096b65eb4404611c079c30ff9b","unresolved":false,"context_lines":[{"line_number":556,"context_line":"        # Delete original compute record, and start a brand new one with the"},{"line_number":557,"context_line":"        # same hostname."},{"line_number":558,"context_line":"        self.api.api_delete(\u0027/os-services/%s\u0027 % svc0[\u0027id\u0027])"},{"line_number":559,"context_line":"        compute0 \u003d self._start_compute(\u0027compute0\u0027)"},{"line_number":560,"context_line":""},{"line_number":561,"context_line":"        # Make sure our new compute is empty before we migrate our instance"},{"line_number":562,"context_line":"        # onto it."}],"source_content_type":"text/x-python","patch_set":3,"id":"420294b9_ba1a6b99","line":559,"in_reply_to":"8a156cff_1aa063f6","updated":"2022-05-10 16:36:13.000000000","message":"Done","commit_id":"d6de7b172209b9acd6a811c24b263e5433c70bfa"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"f9722669a53351a13444cd886256c59364072bb8","unresolved":false,"context_lines":[{"line_number":577,"context_line":"        dom \u003d conn.lookupByUUIDString(server[\u0027id\u0027])"},{"line_number":578,"context_line":"        self.assertIsNotNone(dom)"},{"line_number":579,"context_line":""},{"line_number":580,"context_line":"        # To simulate a service restart, just call the"},{"line_number":581,"context_line":"        # _destroy_evacuated_instances() method manually."},{"line_number":582,"context_line":"        ctxt \u003d context.get_admin_context()"},{"line_number":583,"context_line":"        nodes \u003d compute0.manager._get_nodes(ctxt)"}],"source_content_type":"text/x-python","patch_set":3,"id":"b5e2c910_c1d034b2","line":580,"updated":"2022-05-10 07:45:42.000000000","message":"This is fine. But there is a bit higher level compute restart simulation you can use. Just call:\n\n        self.restart_compute_service(compute0)\n\n// later\n\nIt seems that the restart_compute_service only considers the fake virt driver based func test and does not play nice with these libvirt virt drive based ones. It seems that after the restart the fake RPC becomes dead and that causes timeouts in the _is_instance_storage_shared() calls during the instance destroy. \n\nSo keep this as is.","commit_id":"d6de7b172209b9acd6a811c24b263e5433c70bfa"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"29188bc7c885952ddb4a3515811ee810d9571fa7","unresolved":false,"context_lines":[{"line_number":577,"context_line":"        dom \u003d conn.lookupByUUIDString(server[\u0027id\u0027])"},{"line_number":578,"context_line":"        self.assertIsNotNone(dom)"},{"line_number":579,"context_line":""},{"line_number":580,"context_line":"        # To simulate a service restart, just call the"},{"line_number":581,"context_line":"        # _destroy_evacuated_instances() method manually."},{"line_number":582,"context_line":"        ctxt \u003d context.get_admin_context()"},{"line_number":583,"context_line":"        nodes \u003d compute0.manager._get_nodes(ctxt)"}],"source_content_type":"text/x-python","patch_set":3,"id":"8e07d957_7686f10b","line":580,"in_reply_to":"0d1ceada_8528c8db","updated":"2022-05-10 13:02:16.000000000","message":"stop() then start() does not work as far as I know. The service interface was never designed for an in-place restart. Hence the trick to create a new compute service instead of restating the stopped one. See more in https://github.com/openstack/nova/blob/8f250f50446ca2d7aa84609d5144088aa4cded78/nova/test.py#L480-L490","commit_id":"d6de7b172209b9acd6a811c24b263e5433c70bfa"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"fafc50e9390c779113e0aef88a41d9d86512e9a6","unresolved":false,"context_lines":[{"line_number":577,"context_line":"        dom \u003d conn.lookupByUUIDString(server[\u0027id\u0027])"},{"line_number":578,"context_line":"        self.assertIsNotNone(dom)"},{"line_number":579,"context_line":""},{"line_number":580,"context_line":"        # To simulate a service restart, just call the"},{"line_number":581,"context_line":"        # _destroy_evacuated_instances() method manually."},{"line_number":582,"context_line":"        ctxt \u003d context.get_admin_context()"},{"line_number":583,"context_line":"        nodes \u003d compute0.manager._get_nodes(ctxt)"}],"source_content_type":"text/x-python","patch_set":3,"id":"0d1ceada_8528c8db","line":580,"in_reply_to":"b5e2c910_c1d034b2","updated":"2022-05-10 12:36:23.000000000","message":"hum ok i did not know restart_compute_service existed.\n\ni think i have used stop and start in the past to restart the compute agent properly for evacuate test.\nthat is proably what i would do personally.\n\n\nits what i plan to do for the vdpa evacuate tests\nhttps://review.opendev.org/c/openstack/nova/+/832330/5/nova/tests/functional/libvirt/test_pci_sriov_servers.py#1117\n\nalthouhg im not actully calling start so maybe that would have simialr issues as you suggest.\n\ni think it woudl work however.","commit_id":"d6de7b172209b9acd6a811c24b263e5433c70bfa"}]}