)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"2eaf8cf9caff6e1c6979535068aafa5aad3602fe","unresolved":false,"context_lines":[{"line_number":10,"context_line":"successfully cleaned up upon the driver failing"},{"line_number":11,"context_line":"during \"confirm_migration\"."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"This backport is not clean, it needed to be modified"},{"line_number":14,"context_line":"in order to be compatible with the testing framework"},{"line_number":15,"context_line":"of this stable branch."},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"Change-Id: I9d6478f492351b58aa87b8f56e907ee633d6d1c6"},{"line_number":18,"context_line":"Related-bug: #1821594"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"dfbec78f_9da7010c","line":15,"range":{"start_line":13,"start_character":0,"end_line":15,"end_character":22},"updated":"2019-05-14 15:18:01.000000000","message":"Would be good to mention that the gap is due to If6aa37d9b6b48791e070799ab026c816fda4441c in Stein which added new assertion methods and modified assertFlavorMatchesAllocation.","commit_id":"92010eab35b1ec2f19f48a9acb5e51e393b7a9b5"}],"nova/tests/functional/test_servers.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"222b07bf930668f6ddf1ba40473a1caf0e5bb4f5","unresolved":false,"context_lines":[{"line_number":1873,"context_line":"        # After confirming and error, we should have an allocation only on the"},{"line_number":1874,"context_line":"        # destination host"},{"line_number":1875,"context_line":"        source_usages \u003d self._get_provider_usages(source_rp_uuid)"},{"line_number":1876,"context_line":"        self.assertEqual({\u0027VCPU\u0027: 0,"},{"line_number":1877,"context_line":"                          \u0027MEMORY_MB\u0027: 0,"},{"line_number":1878,"context_line":"                          \u0027DISK_GB\u0027: 0}, source_usages,"},{"line_number":1879,"context_line":"                          \u0027Target host %s still has usage after the failed \u0027"}],"source_content_type":"text/x-python","patch_set":1,"id":"dfbec78f_6a484f86","line":1876,"updated":"2019-05-13 21:12:57.000000000","message":"I think I see what\u0027s wrong.\n\nBefore my change we would cleanup the allocations after setting the migration.status to \u0027confirmed\u0027:\n\nhttps://review.opendev.org/#/c/652146/2/nova/compute/manager.py@a3908\n\nAnd _delete_allocation_after_move was checking that (which is pretty gross tight-coupling of that logic, but anyway):\n\nhttps://review.opendev.org/#/c/652146/2/nova/compute/manager.py@3974\n\nNote that before my fix, if driver.confirm_migration raised we\u0027d still not cleanup because we never called _delete_allocation_after_move. Now we\u0027re still calling _delete_allocation_after_move but the migration.status is not \u0027confirmed\u0027.\n\nSo we hit this else block:\n\nhttps://review.opendev.org/#/c/652146/2/nova/compute/manager.py@3995\n\nWhich shows up in the failing test output:\n\n    b\u00272019-05-13 16:56:02,705 ERROR [nova.compute.manager] Confirm resize failed on source host host1. Resource allocations in the placement service will be removed regardless because the instance is now on the destination host host2. You can try hard rebooting the instance to correct its state.\u0027\n    b\u0027Traceback (most recent call last):\u0027\n    b\u0027  File \"/home/osboxes/git/nova/nova/compute/manager.py\", line 3882, in do_confirm_resize\u0027\n    b\u0027    context, instance, migration\u003dmigration)\u0027\n    b\u0027  File \"/home/osboxes/git/nova/nova/compute/manager.py\", line 3929, in _confirm_resize\u0027\n    b\u0027    network_info)\u0027\n    b\u0027  File \"/home/osboxes/git/nova/.tox/py36/lib/python3.6/site-packages/mock/mock.py\", line 1062, in __call__\u0027\n    b\u0027    return _mock_self._mock_call(*args, **kwargs)\u0027\n    b\u0027  File \"/home/osboxes/git/nova/.tox/py36/lib/python3.6/site-packages/mock/mock.py\", line 1128, in _mock_call\u0027\n    b\u0027    ret_val \u003d effect(*args, **kwargs)\u0027\n    b\u0027  File \"/home/osboxes/git/nova/nova/tests/functional/test_servers.py\", line 1861, in fake_confirm_migration\u0027\n    b\"    reason\u003d\u0027test_migration_confirm_resize_error\u0027)\"\n    b\u0027nova.exception.MigrationPreCheckError: Migration pre-check error: test_migration_confirm_resize_error\u0027\n    b\u00272019-05-13 16:56:02,721 INFO [nova.api.openstack.placement.requestlog] 127.0.0.1 \"GET /placement/allocations/250c4cba-937c-4a98-b78d-220ce7174177\" status: 200 len: 230 microversion: 1.28\u0027\n    b\u00272019-05-13 16:56:02,722 INFO [nova.compute.manager] Source node host1 reverted migration 250c4cba-937c-4a98-b78d-220ce7174177; not deleting migration-based allocation\u0027\n\nThis isn\u0027t a problem in Stein because that tight-coupling logic was all removed from _delete_allocation_after_move:\n\nhttps://review.opendev.org/#/c/611970/2/nova/compute/manager.py\n\nTo fix, we could either:\n\na) do like stein and just call self.reportclient.delete_allocation_for_instance(\n                context, migration.uuid) but if we don\u0027t have a migration-based allocation, that won\u0027t clean anything up\n\nb) mutate the migration.status to \u0027confirmed\u0027 when the finally block calls _delete_allocation_after_move and let it do the cleanup like it would normally - that handles both the migration-based allocation and the case that conductor didn\u0027t move the source node allocations to the migration record.","commit_id":"1498091e5259f63707cc4d4fb3aa2029c4008593"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"0c69a216e25c905a40f10405333c204498446702","unresolved":false,"context_lines":[{"line_number":1873,"context_line":"        # After confirming and error, we should have an allocation only on the"},{"line_number":1874,"context_line":"        # destination host"},{"line_number":1875,"context_line":"        source_usages \u003d self._get_provider_usages(source_rp_uuid)"},{"line_number":1876,"context_line":"        self.assertEqual({\u0027VCPU\u0027: 0,"},{"line_number":1877,"context_line":"                          \u0027MEMORY_MB\u0027: 0,"},{"line_number":1878,"context_line":"                          \u0027DISK_GB\u0027: 0}, source_usages,"},{"line_number":1879,"context_line":"                          \u0027Target host %s still has usage after the failed \u0027"}],"source_content_type":"text/x-python","patch_set":1,"id":"dfbec78f_4a11cb5c","line":1876,"in_reply_to":"dfbec78f_6a484f86","updated":"2019-05-13 21:16:27.000000000","message":"Yup this makes the test pass:\n\ndiff --git a/nova/compute/manager.py b/nova/compute/manager.py\nindex 57c6d56d84..9facecb68e 100644\n--- a/nova/compute/manager.py\n+++ b/nova/compute/manager.py\n@@ -3897,9 +3897,11 @@ class ComputeManager(manager.Manager):\n                     # Whether an error occurred or not, at this point the\n                     # instance is on the dest host so to avoid leaking\n                     # allocations in placement, delete them here.\n-                    self._delete_allocation_after_move(\n-                        context, instance, migration, old_instance_type,\n-                        migration.source_node)\n+                    with utils.temporary_mutation(\n+                            migration, status\u003d\u0027confirmed\u0027):\n+                        self._delete_allocation_after_move(\n+                            context, instance, migration, old_instance_type,\n+                            migration.source_node)\n \n         do_confirm_resize(context, instance, migration.id)","commit_id":"1498091e5259f63707cc4d4fb3aa2029c4008593"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"222b07bf930668f6ddf1ba40473a1caf0e5bb4f5","unresolved":false,"context_lines":[{"line_number":1876,"context_line":"        self.assertEqual({\u0027VCPU\u0027: 0,"},{"line_number":1877,"context_line":"                          \u0027MEMORY_MB\u0027: 0,"},{"line_number":1878,"context_line":"                          \u0027DISK_GB\u0027: 0}, source_usages,"},{"line_number":1879,"context_line":"                          \u0027Target host %s still has usage after the failed \u0027"},{"line_number":1880,"context_line":"                          \u0027migration_confirm\u0027 % source_hostname)"},{"line_number":1881,"context_line":""},{"line_number":1882,"context_line":"        # Check that the server only allocates resource from the original host"}],"source_content_type":"text/x-python","patch_set":1,"id":"dfbec78f_4a6a2bed","line":1879,"range":{"start_line":1879,"start_character":27,"end_line":1879,"end_character":33},"updated":"2019-05-13 21:12:57.000000000","message":"Source","commit_id":"1498091e5259f63707cc4d4fb3aa2029c4008593"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"2eaf8cf9caff6e1c6979535068aafa5aad3602fe","unresolved":false,"context_lines":[{"line_number":1870,"context_line":""},{"line_number":1871,"context_line":"        # After confirming and error, we should have an allocation only on the"},{"line_number":1872,"context_line":"        # destination host"},{"line_number":1873,"context_line":"        source_usages \u003d self._get_provider_usages(source_rp_uuid)"},{"line_number":1874,"context_line":"        self.assertEqual({\u0027VCPU\u0027: 0,"},{"line_number":1875,"context_line":"                          \u0027MEMORY_MB\u0027: 0,"},{"line_number":1876,"context_line":"                          \u0027DISK_GB\u0027: 0}, source_usages,"}],"source_content_type":"text/x-python","patch_set":2,"id":"dfbec78f_1d30f1e7","line":1873,"updated":"2019-05-14 15:18:01.000000000","message":"OK we can\u0027t use assertFlavorMatchesUsage or assertRequestMatchesUsage since those were added with If6aa37d9b6b48791e070799ab026c816fda4441c in Stein.","commit_id":"92010eab35b1ec2f19f48a9acb5e51e393b7a9b5"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"2eaf8cf9caff6e1c6979535068aafa5aad3602fe","unresolved":false,"context_lines":[{"line_number":1882,"context_line":"        self.assertEqual(1, len(allocations))"},{"line_number":1883,"context_line":""},{"line_number":1884,"context_line":"        dest_allocation \u003d allocations[dest_rp_uuid][\u0027resources\u0027]"},{"line_number":1885,"context_line":"        self.assertFlavorMatchesAllocation(self.flavor1, dest_allocation)"},{"line_number":1886,"context_line":""},{"line_number":1887,"context_line":"        dest_usages \u003d self._get_provider_usages(dest_rp_uuid)"},{"line_number":1888,"context_line":"        self.assertFlavorMatchesAllocation(self.flavor1, dest_usages)"}],"source_content_type":"text/x-python","patch_set":2,"id":"dfbec78f_dd7c9984","line":1885,"range":{"start_line":1885,"start_character":13,"end_line":1885,"end_character":42},"updated":"2019-05-14 15:18:01.000000000","message":"And this also changed with If6aa37d9b6b48791e070799ab026c816fda4441c in Stein.","commit_id":"92010eab35b1ec2f19f48a9acb5e51e393b7a9b5"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"6936e657867a2d046099c4d887976f80ca00d3f1","unresolved":false,"context_lines":[{"line_number":1882,"context_line":"        self.assertEqual(1, len(allocations))"},{"line_number":1883,"context_line":""},{"line_number":1884,"context_line":"        dest_allocation \u003d allocations[dest_rp_uuid][\u0027resources\u0027]"},{"line_number":1885,"context_line":"        self.assertFlavorMatchesAllocation(self.flavor1, dest_allocation)"},{"line_number":1886,"context_line":""},{"line_number":1887,"context_line":"        dest_usages \u003d self._get_provider_usages(dest_rp_uuid)"},{"line_number":1888,"context_line":"        self.assertFlavorMatchesAllocation(self.flavor1, dest_usages)"}],"source_content_type":"text/x-python","patch_set":2,"id":"dfbec78f_3bf7f8d5","line":1885,"range":{"start_line":1885,"start_character":13,"end_line":1885,"end_character":42},"in_reply_to":"dfbec78f_02c08929","updated":"2019-05-14 21:51:03.000000000","message":"Using it here is fine.","commit_id":"92010eab35b1ec2f19f48a9acb5e51e393b7a9b5"},{"author":{"_account_id":14567,"name":"Rodrigo Barbieri","email":"rodrigo.barbieri2010@gmail.com","username":"ganso"},"change_message_id":"90aa286f71198b109f939042bae372994595fc48","unresolved":false,"context_lines":[{"line_number":1882,"context_line":"        self.assertEqual(1, len(allocations))"},{"line_number":1883,"context_line":""},{"line_number":1884,"context_line":"        dest_allocation \u003d allocations[dest_rp_uuid][\u0027resources\u0027]"},{"line_number":1885,"context_line":"        self.assertFlavorMatchesAllocation(self.flavor1, dest_allocation)"},{"line_number":1886,"context_line":""},{"line_number":1887,"context_line":"        dest_usages \u003d self._get_provider_usages(dest_rp_uuid)"},{"line_number":1888,"context_line":"        self.assertFlavorMatchesAllocation(self.flavor1, dest_usages)"}],"source_content_type":"text/x-python","patch_set":2,"id":"dfbec78f_02c08929","line":1885,"range":{"start_line":1885,"start_character":13,"end_line":1885,"end_character":42},"in_reply_to":"dfbec78f_dd7c9984","updated":"2019-05-14 18:59:41.000000000","message":"even though it has changed, it was being used for that purpose in https://review.opendev.org/#/c/607287/4/nova/tests/functional/test_servers.py \n\nAre you saying we should not use it?","commit_id":"92010eab35b1ec2f19f48a9acb5e51e393b7a9b5"}]}