)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"6626a6c946ca46c5d061d52b06e455f1b5ac1a36","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"eec61fbf_230e272a","updated":"2025-09-25 18:35:51.000000000","message":"+2ing on the assumption that my latest comment is correct and a comment will get FUP\u0027d","commit_id":"7419855afe848bd4ff2093ab1dc2007856b480d6"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"4a1bbe541032fdebe414dfd71e942bce43ce3533","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"6bf17fa5_2a243eaf","updated":"2025-09-25 16:54:12.000000000","message":"Looks okay to me, just a couple of questions in the tests (that I\u0027m sure are obvious, I\u0027m just missing)","commit_id":"7419855afe848bd4ff2093ab1dc2007856b480d6"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"6223201665cc5647ddee18da22ab35ad4619fda1","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"8126bf5c_76efbd41","updated":"2025-10-14 14:25:29.000000000","message":"LGTM, I see what was causing the failure, thanks !","commit_id":"787d2a130053bd369d35480d6534f01b07c2310d"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"b3755516b848cd6e8b89b28fec66f40ab3080489","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"e4376ef0_2ca63642","updated":"2025-10-01 20:52:25.000000000","message":"recheck nova-live-migration-ceph failed but has no logs","commit_id":"787d2a130053bd369d35480d6534f01b07c2310d"}],"nova/compute/manager.py":[{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"6223201665cc5647ddee18da22ab35ad4619fda1","unresolved":false,"context_lines":[{"line_number":965,"context_line":"        self._delete_scheduler_instance_info(context, instance.uuid)"},{"line_number":966,"context_line":""},{"line_number":967,"context_line":"        # Delete the vTPM secret in the key manager service if needed."},{"line_number":968,"context_line":"        crypto.delete_vtpm_secret(context, instance)"},{"line_number":969,"context_line":""},{"line_number":970,"context_line":"    def _validate_pinning_configuration(self, instances):"},{"line_number":971,"context_line":"        if not self.driver.capabilities.get(\u0027supports_pcpus\u0027, False):"}],"source_content_type":"text/x-python","patch_set":3,"id":"cfca1205_be66c28d","line":968,"updated":"2025-10-14 14:25:29.000000000","message":"note to myself : delete_vtpm_secret is a noop if the instance system metadata doesn\u0027t embed the secret UUID https://github.com/openstack/nova/blob/8b81b5f91ffe1f9c38a483d151b82316d443dbf6/nova/crypto.py#L248-L250\n\n(happy to know we don\u0027t unconditionally call Barbican here 😊)","commit_id":"787d2a130053bd369d35480d6534f01b07c2310d"}],"nova/tests/functional/libvirt/test_vtpm.py":[{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"6223201665cc5647ddee18da22ab35ad4619fda1","unresolved":false,"context_lines":[{"line_number":432,"context_line":"    def setUp(self):"},{"line_number":433,"context_line":"        super().setUp()"},{"line_number":434,"context_line":"        self.useFixture(fixtures.MockPatch("},{"line_number":435,"context_line":"            \u0027nova.compute.manager.ComputeManager._is_instance_storage_shared\u0027,"},{"line_number":436,"context_line":"            return_value\u003dFalse))"}],"source_content_type":"text/x-python","patch_set":3,"id":"50c6255e_0f9c7076","line":435,"updated":"2025-10-14 14:25:29.000000000","message":"that\u0027s cool, we now test all the same methods but with shared storage 👍","commit_id":"787d2a130053bd369d35480d6534f01b07c2310d"}],"nova/tests/unit/compute/test_compute_mgr.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"4a1bbe541032fdebe414dfd71e942bce43ce3533","unresolved":true,"context_lines":[{"line_number":1775,"context_line":"            self.context, instance)"},{"line_number":1776,"context_line":"        mocks[\u0027_delete_scheduler_instance_info\u0027].assert_called_once_with("},{"line_number":1777,"context_line":"            self.context, instance.uuid)"},{"line_number":1778,"context_line":"        mock_delete_vtpm.assert_called_once_with(self.context, instance)"},{"line_number":1779,"context_line":"        if reclaim_instance_interval \u003e 0:"},{"line_number":1780,"context_line":"            mock_delete_alloc.assert_called_once_with("},{"line_number":1781,"context_line":"                self.context, instance.uuid, force\u003dFalse)"}],"source_content_type":"text/x-python","patch_set":2,"id":"71c86d31_c0c718e7","line":1778,"updated":"2025-09-25 16:54:12.000000000","message":"Hrm, if we\u0027re delaying the actual instance delete, shouldn\u0027t we only call this once we reap?","commit_id":"7419855afe848bd4ff2093ab1dc2007856b480d6"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"394b5596cf19833190c6af758f2020534d6a9baa","unresolved":true,"context_lines":[{"line_number":1775,"context_line":"            self.context, instance)"},{"line_number":1776,"context_line":"        mocks[\u0027_delete_scheduler_instance_info\u0027].assert_called_once_with("},{"line_number":1777,"context_line":"            self.context, instance.uuid)"},{"line_number":1778,"context_line":"        mock_delete_vtpm.assert_called_once_with(self.context, instance)"},{"line_number":1779,"context_line":"        if reclaim_instance_interval \u003e 0:"},{"line_number":1780,"context_line":"            mock_delete_alloc.assert_called_once_with("},{"line_number":1781,"context_line":"                self.context, instance.uuid, force\u003dFalse)"}],"source_content_type":"text/x-python","patch_set":2,"id":"a726107b_30b4700d","line":1778,"in_reply_to":"71c86d31_c0c718e7","updated":"2025-09-25 17:13:05.000000000","message":"_complete_deletion() is actually also called in the normal/immediate delete path:\n\nhttps://github.com/openstack/nova/blob/b99a882366251f88d145e27312b94692e0b2266f/nova/compute/manager.py#L3373\n\nand seemed to be where other similar cleanups were located. I also thought it\u0027s something we would want to reap if somehow we didn\u0027t delete it during the immediate deletion.","commit_id":"7419855afe848bd4ff2093ab1dc2007856b480d6"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"6626a6c946ca46c5d061d52b06e455f1b5ac1a36","unresolved":true,"context_lines":[{"line_number":1775,"context_line":"            self.context, instance)"},{"line_number":1776,"context_line":"        mocks[\u0027_delete_scheduler_instance_info\u0027].assert_called_once_with("},{"line_number":1777,"context_line":"            self.context, instance.uuid)"},{"line_number":1778,"context_line":"        mock_delete_vtpm.assert_called_once_with(self.context, instance)"},{"line_number":1779,"context_line":"        if reclaim_instance_interval \u003e 0:"},{"line_number":1780,"context_line":"            mock_delete_alloc.assert_called_once_with("},{"line_number":1781,"context_line":"                self.context, instance.uuid, force\u003dFalse)"}],"source_content_type":"text/x-python","patch_set":2,"id":"9eb3cef0_3b9a8648","line":1778,"in_reply_to":"9ba4478f_491f2aaa","updated":"2025-09-25 18:35:51.000000000","message":"Ah right of course. I didn\u0027t go look, I just saw you were handling both of those cases for it for the allocations delete, which I expected to also account for this. I guess maybe we skip allocation delete (or force or whatever) in the reap case because it\u0027s already done or something?\n\nIf so, can you just put a comment in here to that effect for the future? I figure if we\u0027re ever hunting down an errant tpm deletion (or lack thereof) and we find this test we might be similarly puzzled about why we don\u0027t see the different behavior.\n\nCould be slapped into a FUP patch if you want to avoid the rebase.","commit_id":"7419855afe848bd4ff2093ab1dc2007856b480d6"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"65d1d80919fdd4c1e774d6e008b6adc885aaa66e","unresolved":true,"context_lines":[{"line_number":1775,"context_line":"            self.context, instance)"},{"line_number":1776,"context_line":"        mocks[\u0027_delete_scheduler_instance_info\u0027].assert_called_once_with("},{"line_number":1777,"context_line":"            self.context, instance.uuid)"},{"line_number":1778,"context_line":"        mock_delete_vtpm.assert_called_once_with(self.context, instance)"},{"line_number":1779,"context_line":"        if reclaim_instance_interval \u003e 0:"},{"line_number":1780,"context_line":"            mock_delete_alloc.assert_called_once_with("},{"line_number":1781,"context_line":"                self.context, instance.uuid, force\u003dFalse)"}],"source_content_type":"text/x-python","patch_set":2,"id":"f9cfa2b1_14183857","line":1778,"in_reply_to":"9eb3cef0_3b9a8648","updated":"2025-09-25 19:00:36.000000000","message":"TBH I don\u0027t know why delete vs soft-delete is treated differently with regard to the force kwarg. The commit that added it [1] doesn\u0027t really explain either. IMO it should be treated the same and there should not be any if-else.\n\nAs for me including it in this test, it was only because the method (_complete_deletion) had no dedicated test coverage and what I was adding for vTPM is something I thought we should assert. So other than vTPM, I felt what I was doing here was adding missing test coverage. I thought if I\u0027m asserting something for vTPM I should go ahead and cover everything else that was not covered before.\n\nPrior to this the testing of _complete_deletion was only indirect by calling through other methods and I was thinking I shouldn\u0027t perpetuate that with the vTPM coverage.\n\nSo honestly I don\u0027t know what I could comment here given that I don\u0027t know why it was decided to use a difference force value for delete vs soft delete. I guess I could just link the commit that added it as a reference for anyone wondering why this is.\n\nI could also comment that _complete_deletion() is only called at reap time in the case of soft-delete.\n\n[1] https://github.com/openstack/nova/commit/c09d98dadb6cd69e294420ba7ecea0f9b9cfcd71#diff-2f66437794b23f16fe1d570b9eaae9d57c3184f11d51ae979163b1a3419b89ec","commit_id":"7419855afe848bd4ff2093ab1dc2007856b480d6"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"59a39963c64899a6b7fadc0228cf46b6c246fa5c","unresolved":true,"context_lines":[{"line_number":1775,"context_line":"            self.context, instance)"},{"line_number":1776,"context_line":"        mocks[\u0027_delete_scheduler_instance_info\u0027].assert_called_once_with("},{"line_number":1777,"context_line":"            self.context, instance.uuid)"},{"line_number":1778,"context_line":"        mock_delete_vtpm.assert_called_once_with(self.context, instance)"},{"line_number":1779,"context_line":"        if reclaim_instance_interval \u003e 0:"},{"line_number":1780,"context_line":"            mock_delete_alloc.assert_called_once_with("},{"line_number":1781,"context_line":"                self.context, instance.uuid, force\u003dFalse)"}],"source_content_type":"text/x-python","patch_set":2,"id":"cdef7610_533d5c52","line":1778,"in_reply_to":"a726107b_30b4700d","updated":"2025-09-25 17:49:48.000000000","message":"Okay but what I mean is.. if we delete the vtpm data when we do the deferred delete but then undelete the instance, the tpm will be gone right? My point is, below you\u0027re testing that we do/don\u0027t delete allocations based on if we\u0027re really deleting or soft deleting - shouldn\u0027t that apply to the vtpm data too?","commit_id":"7419855afe848bd4ff2093ab1dc2007856b480d6"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"337ee7f6d52bedf064015c4c90e7d8e0151604aa","unresolved":true,"context_lines":[{"line_number":1775,"context_line":"            self.context, instance)"},{"line_number":1776,"context_line":"        mocks[\u0027_delete_scheduler_instance_info\u0027].assert_called_once_with("},{"line_number":1777,"context_line":"            self.context, instance.uuid)"},{"line_number":1778,"context_line":"        mock_delete_vtpm.assert_called_once_with(self.context, instance)"},{"line_number":1779,"context_line":"        if reclaim_instance_interval \u003e 0:"},{"line_number":1780,"context_line":"            mock_delete_alloc.assert_called_once_with("},{"line_number":1781,"context_line":"                self.context, instance.uuid, force\u003dFalse)"}],"source_content_type":"text/x-python","patch_set":2,"id":"9ba4478f_491f2aaa","line":1778,"in_reply_to":"cdef7610_533d5c52","updated":"2025-09-25 18:29:44.000000000","message":"Ah OK, I see what you are saying now. FWIW the testing below is only checking whether the force kwarg was set to True vs False -- in both cases it is testing that we delete the allocations.\n\nThe _complete_deletion() stuff only runs at for real delete time, so either during a normal delete or during a reap in the case of soft delete.\n\nFor soft delete, none of the \"delete\" methods are called and it just does a power off instead [1]:\n\n```\n    def soft_delete_instance(self, context, instance):\n        \"\"\"Soft delete an instance on this host.\"\"\"\n        with compute_utils.notify_about_instance_delete(\n                self.notifier, context, instance, \u0027soft_delete\u0027,\n                source\u003dfields.NotificationSource.COMPUTE):\n            try:\n                self.driver.soft_delete(instance)\n            except NotImplementedError:\n                # Fallback to just powering off the instance if the\n                # hypervisor doesn\u0027t implement the soft_delete method\n                self._power_off_instance(\n                    context, instance, clean_shutdown\u003dFalse\n                )\n            instance.power_state \u003d self._get_power_state(instance)\n            instance.vm_state \u003d vm_states.SOFT_DELETED\n            instance.task_state \u003d None\n            instance.save(expected_task_state\u003d[task_states.SOFT_DELETING])\n```\n\nSo I think we should be safe for the soft delete case unless I am missing something.\n\n[1] https://github.com/openstack/nova/blob/b99a882366251f88d145e27312b94692e0b2266f/nova/compute/manager.py#L3573-L3589","commit_id":"7419855afe848bd4ff2093ab1dc2007856b480d6"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"cac58e05c9890ea0b789a4960a9a01dedb5c1340","unresolved":true,"context_lines":[{"line_number":1775,"context_line":"            self.context, instance)"},{"line_number":1776,"context_line":"        mocks[\u0027_delete_scheduler_instance_info\u0027].assert_called_once_with("},{"line_number":1777,"context_line":"            self.context, instance.uuid)"},{"line_number":1778,"context_line":"        mock_delete_vtpm.assert_called_once_with(self.context, instance)"},{"line_number":1779,"context_line":"        if reclaim_instance_interval \u003e 0:"},{"line_number":1780,"context_line":"            mock_delete_alloc.assert_called_once_with("},{"line_number":1781,"context_line":"                self.context, instance.uuid, force\u003dFalse)"}],"source_content_type":"text/x-python","patch_set":2,"id":"fe58bb92_4eadf394","line":1778,"in_reply_to":"f029b761_8983b429","updated":"2025-09-25 19:15:59.000000000","message":"Gotcha. Yeah, makes sense. I\u0027ll add a FUP patch to the end for now and can squash into this patch if it becomes convenient (as it often happens).","commit_id":"7419855afe848bd4ff2093ab1dc2007856b480d6"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"32df261d8ab7dae7b7255c5ef754cbd9ff7a4288","unresolved":true,"context_lines":[{"line_number":1775,"context_line":"            self.context, instance)"},{"line_number":1776,"context_line":"        mocks[\u0027_delete_scheduler_instance_info\u0027].assert_called_once_with("},{"line_number":1777,"context_line":"            self.context, instance.uuid)"},{"line_number":1778,"context_line":"        mock_delete_vtpm.assert_called_once_with(self.context, instance)"},{"line_number":1779,"context_line":"        if reclaim_instance_interval \u003e 0:"},{"line_number":1780,"context_line":"            mock_delete_alloc.assert_called_once_with("},{"line_number":1781,"context_line":"                self.context, instance.uuid, force\u003dFalse)"}],"source_content_type":"text/x-python","patch_set":2,"id":"f029b761_8983b429","line":1778,"in_reply_to":"f9cfa2b1_14183857","updated":"2025-09-25 19:07:22.000000000","message":"\u003e As for me including it in this test, it was only because the method (_complete_deletion) had no dedicated test coverage and what I was adding for vTPM is something I thought we should assert. So other than vTPM, I felt what I was doing here was adding missing test coverage. I thought if I\u0027m asserting something for vTPM I should go ahead and cover everything else that was not covered before.\n\nYeah, I understand and it\u0027s cool, I\u0027m just saying now there\u0027s a test that looks like it might (without enough context) should do something different for vtpm deletion.\n\n\u003e I could also comment that _complete_deletion() is only called at reap time in the case of soft-delete.\n\nThis is what I mean. Maybe \"_complete_deletion() is only called at actual delete time (either regular delete or when reaping after soft delete). The force argument differs based on actual or reap delete for other reasons.\"\n\nNot a big deal, I\u0027m just saying when I read that test case my first thought was that it seemed like it was asserting that the behavior between the two was the same, when it clearly wasn\u0027t for another type of resource. Having a comment there to make it clear it\u0027s not an oversight seems like it will help avoid having to dig up all this context later.","commit_id":"7419855afe848bd4ff2093ab1dc2007856b480d6"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"1d84115ee5caa2bb0f4fc29e3e3be61fb4b63269","unresolved":false,"context_lines":[{"line_number":1775,"context_line":"            self.context, instance)"},{"line_number":1776,"context_line":"        mocks[\u0027_delete_scheduler_instance_info\u0027].assert_called_once_with("},{"line_number":1777,"context_line":"            self.context, instance.uuid)"},{"line_number":1778,"context_line":"        mock_delete_vtpm.assert_called_once_with(self.context, instance)"},{"line_number":1779,"context_line":"        if reclaim_instance_interval \u003e 0:"},{"line_number":1780,"context_line":"            mock_delete_alloc.assert_called_once_with("},{"line_number":1781,"context_line":"                self.context, instance.uuid, force\u003dFalse)"}],"source_content_type":"text/x-python","patch_set":2,"id":"87dae5d6_5beba44b","line":1778,"in_reply_to":"fe58bb92_4eadf394","updated":"2025-09-25 19:52:12.000000000","message":"Acknowledged","commit_id":"7419855afe848bd4ff2093ab1dc2007856b480d6"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"4a1bbe541032fdebe414dfd71e942bce43ce3533","unresolved":true,"context_lines":[{"line_number":1868,"context_line":"        with mock.patch.multiple("},{"line_number":1869,"context_line":"            self.compute,"},{"line_number":1870,"context_line":"            _get_share_info\u003dmock.Mock(return_value\u003dobjects.ShareMappingList()),"},{"line_number":1871,"context_line":"            _clean_instance_console_tokens\u003dmock.DEFAULT,"},{"line_number":1872,"context_line":"        ):"},{"line_number":1873,"context_line":"            self.compute._init_instance(self.context, instance)"},{"line_number":1874,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"a8b5308f_14372f2e","line":1871,"updated":"2025-09-25 16:54:12.000000000","message":"I\u0027m not sure why this is relevant...?","commit_id":"7419855afe848bd4ff2093ab1dc2007856b480d6"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"59a39963c64899a6b7fadc0228cf46b6c246fa5c","unresolved":false,"context_lines":[{"line_number":1868,"context_line":"        with mock.patch.multiple("},{"line_number":1869,"context_line":"            self.compute,"},{"line_number":1870,"context_line":"            _get_share_info\u003dmock.Mock(return_value\u003dobjects.ShareMappingList()),"},{"line_number":1871,"context_line":"            _clean_instance_console_tokens\u003dmock.DEFAULT,"},{"line_number":1872,"context_line":"        ):"},{"line_number":1873,"context_line":"            self.compute._init_instance(self.context, instance)"},{"line_number":1874,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"8fa0ebd5_4fb3ed59","line":1871,"in_reply_to":"8c6aff40_3e1f0e22","updated":"2025-09-25 17:49:48.000000000","message":"Okay, makes sense.. kinda seems like the test is wrong to begin with, I just guess it doesn\u0027t matter until you make this change.","commit_id":"7419855afe848bd4ff2093ab1dc2007856b480d6"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"394b5596cf19833190c6af758f2020534d6a9baa","unresolved":true,"context_lines":[{"line_number":1868,"context_line":"        with mock.patch.multiple("},{"line_number":1869,"context_line":"            self.compute,"},{"line_number":1870,"context_line":"            _get_share_info\u003dmock.Mock(return_value\u003dobjects.ShareMappingList()),"},{"line_number":1871,"context_line":"            _clean_instance_console_tokens\u003dmock.DEFAULT,"},{"line_number":1872,"context_line":"        ):"},{"line_number":1873,"context_line":"            self.compute._init_instance(self.context, instance)"},{"line_number":1874,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"be9bc063_527837fb","line":1871,"in_reply_to":"a8b5308f_14372f2e","updated":"2025-09-25 17:13:05.000000000","message":"This was just to avoid a DB access attempt in a test.NoDBTestCase ... although, I don\u0027t get how it wasn\u0027t failing similarly before.\n\nWhen I tried backing out my changes to nova/compute/manager.py and nova/tests/unit/compute/test_compute_mgr.py it still fails but when I checkout the master branch and run it, it passes.\n\nI don\u0027t understand why yet but I\u0027ll keep looking to see if I can find why.","commit_id":"7419855afe848bd4ff2093ab1dc2007856b480d6"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"845ef4ff188db194208a6a84c84c40f011c99692","unresolved":true,"context_lines":[{"line_number":1868,"context_line":"        with mock.patch.multiple("},{"line_number":1869,"context_line":"            self.compute,"},{"line_number":1870,"context_line":"            _get_share_info\u003dmock.Mock(return_value\u003dobjects.ShareMappingList()),"},{"line_number":1871,"context_line":"            _clean_instance_console_tokens\u003dmock.DEFAULT,"},{"line_number":1872,"context_line":"        ):"},{"line_number":1873,"context_line":"            self.compute._init_instance(self.context, instance)"},{"line_number":1874,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"8c6aff40_3e1f0e22","line":1871,"in_reply_to":"be9bc063_527837fb","updated":"2025-09-25 17:40:38.000000000","message":"OK, this is fun. I found that there is a `except Exception:` block in this path [1]:\n\n```\n        if instance.vm_state \u003d\u003d vm_states.DELETED:\n            try:\n                self._complete_partial_deletion(context, instance)\n            except Exception:\n                # we don\u0027t want that an exception blocks the init_host\n                LOG.exception(\u0027Failed to complete a deletion\u0027,\n                              instance\u003dinstance)\n            return\n```\n\nwhich hides the exception raised by the DatabasePoisonFixture.\n\nIf I change L1879 to `mock_delete_vtpm.assert_not_called()` the test passes, which makes sense because the above block prevents the call of `crypto.delete_vtpm_secret()` from being reached.\n\nIf I leave L1879 as-is, the test fails because the assertion fails saying that the mock was called 0 times. And I can see the traceback from the poison fixture in the log output.\n\nSo in order to reach the `crypto.delete_vtpm_secret()` call I have to mock `_clean_instance_console_tokens` to avoid getting short circuited by the poison exception.\n\n[1] https://github.com/openstack/nova/blob/b99a882366251f88d145e27312b94692e0b2266f/nova/compute/manager.py#L1115-L1122","commit_id":"7419855afe848bd4ff2093ab1dc2007856b480d6"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"c472885dcf2cdb69bb587ba3427ed6656d6bb096","unresolved":true,"context_lines":[{"line_number":1776,"context_line":"        mocks[\u0027_delete_scheduler_instance_info\u0027].assert_called_once_with("},{"line_number":1777,"context_line":"            self.context, instance.uuid)"},{"line_number":1778,"context_line":"        mock_delete_vtpm.assert_called_once_with(self.context, instance)"},{"line_number":1779,"context_line":"        # _complete_deletion() is only called at actual delete time (either"},{"line_number":1780,"context_line":"        # regular delete or when reaping after soft delete). The force argument"},{"line_number":1781,"context_line":"        # differs based on actual or reap delete for other reasons."},{"line_number":1782,"context_line":"        if reclaim_instance_interval \u003e 0:"},{"line_number":1783,"context_line":"            mock_delete_alloc.assert_called_once_with("},{"line_number":1784,"context_line":"                self.context, instance.uuid, force\u003dFalse)"}],"source_content_type":"text/x-python","patch_set":3,"id":"455cf161_69678369","line":1781,"range":{"start_line":1779,"start_character":0,"end_line":1781,"end_character":67},"updated":"2025-10-01 02:42:30.000000000","message":"I have squashed the FUP into here.","commit_id":"787d2a130053bd369d35480d6534f01b07c2310d"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"6223201665cc5647ddee18da22ab35ad4619fda1","unresolved":false,"context_lines":[{"line_number":1776,"context_line":"        mocks[\u0027_delete_scheduler_instance_info\u0027].assert_called_once_with("},{"line_number":1777,"context_line":"            self.context, instance.uuid)"},{"line_number":1778,"context_line":"        mock_delete_vtpm.assert_called_once_with(self.context, instance)"},{"line_number":1779,"context_line":"        # _complete_deletion() is only called at actual delete time (either"},{"line_number":1780,"context_line":"        # regular delete or when reaping after soft delete). The force argument"},{"line_number":1781,"context_line":"        # differs based on actual or reap delete for other reasons."},{"line_number":1782,"context_line":"        if reclaim_instance_interval \u003e 0:"},{"line_number":1783,"context_line":"            mock_delete_alloc.assert_called_once_with("},{"line_number":1784,"context_line":"                self.context, instance.uuid, force\u003dFalse)"}],"source_content_type":"text/x-python","patch_set":3,"id":"fe3d73bd_740fe3cb","line":1781,"range":{"start_line":1779,"start_character":0,"end_line":1781,"end_character":67},"in_reply_to":"455cf161_69678369","updated":"2025-10-14 14:25:29.000000000","message":"Acknowledged","commit_id":"787d2a130053bd369d35480d6534f01b07c2310d"}],"nova/virt/libvirt/driver.py":[{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"08ecc44e5c68a1fb9e9cd54bdb48f3a05d17daa2","unresolved":true,"context_lines":[{"line_number":1650,"context_line":"        :param migrate_data: optional migrate_data object"},{"line_number":1651,"context_line":"        :param destroy_vifs: if plugged vifs should be unplugged"},{"line_number":1652,"context_line":"        :param destroy_secrets: Indicates if libvirt secrets for Cinder volume"},{"line_number":1653,"context_line":"                   encryption should be destroyed"},{"line_number":1654,"context_line":"        \"\"\""},{"line_number":1655,"context_line":"        cleanup_instance_dir \u003d False"},{"line_number":1656,"context_line":"        cleanup_instance_disks \u003d False"}],"source_content_type":"text/x-python","patch_set":1,"id":"9659279f_faa5b171","line":1653,"updated":"2025-09-22 18:39:24.000000000","message":"Note to reviewers: I added this detail to the docstrings for `destroy_secrets` because when I first came across this bug, I wondered why instances with encrypted Cinder volumes did not have the same problem of losing their secret with resize revert. It was because for Cinder volumes, all of the \"delete secret\" code only deals with local libvirt secrets and does not involve deleting any secrets from Barbican. I thought adding detail to the docstring would be helpful for reference in the future.","commit_id":"1b06752b1295c91444eca1320297d88d339245fc"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"cb909fb5689e8f6a909faf003364e81b129999c6","unresolved":true,"context_lines":[{"line_number":1650,"context_line":"        :param migrate_data: optional migrate_data object"},{"line_number":1651,"context_line":"        :param destroy_vifs: if plugged vifs should be unplugged"},{"line_number":1652,"context_line":"        :param destroy_secrets: Indicates if libvirt secrets for Cinder volume"},{"line_number":1653,"context_line":"                   encryption should be destroyed"},{"line_number":1654,"context_line":"        \"\"\""},{"line_number":1655,"context_line":"        cleanup_instance_dir \u003d False"},{"line_number":1656,"context_line":"        cleanup_instance_disks \u003d False"}],"source_content_type":"text/x-python","patch_set":1,"id":"c44cc90b_fbfbf460","line":1653,"in_reply_to":"9659279f_faa5b171","updated":"2025-09-22 18:41:43.000000000","message":"Test Tempest patch showing resize revert with encrypted Cinder volume has no issues \"in real life\":\n\nhttps://review.opendev.org/c/openstack/tempest/+/961756","commit_id":"1b06752b1295c91444eca1320297d88d339245fc"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"6223201665cc5647ddee18da22ab35ad4619fda1","unresolved":false,"context_lines":[{"line_number":1650,"context_line":"        :param migrate_data: optional migrate_data object"},{"line_number":1651,"context_line":"        :param destroy_vifs: if plugged vifs should be unplugged"},{"line_number":1652,"context_line":"        :param destroy_secrets: Indicates if libvirt secrets for Cinder volume"},{"line_number":1653,"context_line":"                   encryption should be destroyed"},{"line_number":1654,"context_line":"        \"\"\""},{"line_number":1655,"context_line":"        cleanup_instance_dir \u003d False"},{"line_number":1656,"context_line":"        cleanup_instance_disks \u003d False"}],"source_content_type":"text/x-python","patch_set":1,"id":"b7b456d5_a4c97f94","line":1653,"in_reply_to":"c44cc90b_fbfbf460","updated":"2025-10-14 14:25:29.000000000","message":"thanks for explaining it, Mel.","commit_id":"1b06752b1295c91444eca1320297d88d339245fc"}]}