)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"284d7c3a66c1006092c3f85e11dfb1b568c73bed","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"76f3ab76_e5df9dd0","updated":"2025-10-13 19:49:58.000000000","message":"Open to suggestions of a better way to do this...","commit_id":"c3ab178419d99fcddf53e5235ef774c471c2dba1"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"5cce4a672e2088e98f6090b5e09fa90b6b501653","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"52f9685c_f891d333","updated":"2025-10-14 13:32:17.000000000","message":"oops, somehow these comments didn\u0027t get committed yesterday","commit_id":"c3ab178419d99fcddf53e5235ef774c471c2dba1"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"19190a8d0937cf35fb58fa8de97e62d40986855d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"0527869b_133d22d8","updated":"2025-10-21 16:13:11.000000000","message":"not 100% sure I like that approach, and I would have rather preferred to ask Castellan to fix their problems, but we can\u0027t wait so much.\n\n+1 for opening the conversation, can change to +2.","commit_id":"1924e4aca69cc183f61c65d74b99198eb58b5bfd"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"caca8556a0c4f5612253bf0a8111e2e5ef528b80","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"286dfac2_6eab66ce","updated":"2025-11-03 18:44:30.000000000","message":"Re-adding my +2 after rebase","commit_id":"b4861a6d2b9fa0f77fa49156601672b51371ce0f"}],"nova/compute/manager.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"f49bee06c9f9167eb756df200ccc8a89d3252094","unresolved":true,"context_lines":[{"line_number":2371,"context_line":"    # error from the real work is not propagated back to these decorators."},{"line_number":2372,"context_line":"    # All those error needs to be handled by _locked_do_build_and_run_instance"},{"line_number":2373,"context_line":"    # and _do_build_and_run_instance"},{"line_number":2374,"context_line":"    @messaging.expected_exceptions(exception.VTPMSecretForbidden)"},{"line_number":2375,"context_line":"    @wrap_exception()"},{"line_number":2376,"context_line":"    @reverts_task_state"},{"line_number":2377,"context_line":"    @wrap_instance_fault"}],"source_content_type":"text/x-python","patch_set":1,"id":"24d006ae_857052a8","line":2374,"updated":"2025-10-13 16:48:19.000000000","message":"All of these `messaging.expected_exceptions()` seem to be really high in the stack. It also means any of these that are calls (`finish_snapshot_based_resize_at_dest`) would send this exception back to the caller, which seems too detailed. This happens super deep in libvirt\u0027s spawn path (at least in the case I noted).. why wouldn\u0027t we just catch it down there and turn it into the same sort of build failure that would happen if we couldn\u0027t allocate network, disk, etc instead of bubbling that all the way back to compute manager and the RPC interface?\n\nLike, here:\n\nhttps://github.com/openstack/nova/blob/master/nova/compute/manager.py#L2730\n\nI\u0027m not sure if that would cover all the cases you\u0027re adding here, but I suspect they\u0027re all \"start the instance\" related so... maybe? Point being, the fact that all of those are not listed as RPC-level ignores tells me that catching this deeper is the right thing to do. But, maybe I\u0027m missing something?","commit_id":"c3ab178419d99fcddf53e5235ef774c471c2dba1"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"284d7c3a66c1006092c3f85e11dfb1b568c73bed","unresolved":true,"context_lines":[{"line_number":2371,"context_line":"    # error from the real work is not propagated back to these decorators."},{"line_number":2372,"context_line":"    # All those error needs to be handled by _locked_do_build_and_run_instance"},{"line_number":2373,"context_line":"    # and _do_build_and_run_instance"},{"line_number":2374,"context_line":"    @messaging.expected_exceptions(exception.VTPMSecretForbidden)"},{"line_number":2375,"context_line":"    @wrap_exception()"},{"line_number":2376,"context_line":"    @reverts_task_state"},{"line_number":2377,"context_line":"    @wrap_instance_fault"}],"source_content_type":"text/x-python","patch_set":1,"id":"3c20d974_28eb395e","line":2374,"in_reply_to":"24d006ae_857052a8","updated":"2025-10-13 19:49:58.000000000","message":"I had been thinking it would be good for the caller to know the failure was for a policy permissions issue and not for some other problem? What makes you say that would be too detailed?\n\nAnd yeah that would not cover all of the cases here. We would need to add the same catch block to delete, start, resume, restore, because it would be raised anywhere a libvirt domain is attempted to be started.","commit_id":"c3ab178419d99fcddf53e5235ef774c471c2dba1"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"5cce4a672e2088e98f6090b5e09fa90b6b501653","unresolved":true,"context_lines":[{"line_number":2371,"context_line":"    # error from the real work is not propagated back to these decorators."},{"line_number":2372,"context_line":"    # All those error needs to be handled by _locked_do_build_and_run_instance"},{"line_number":2373,"context_line":"    # and _do_build_and_run_instance"},{"line_number":2374,"context_line":"    @messaging.expected_exceptions(exception.VTPMSecretForbidden)"},{"line_number":2375,"context_line":"    @wrap_exception()"},{"line_number":2376,"context_line":"    @reverts_task_state"},{"line_number":2377,"context_line":"    @wrap_instance_fault"}],"source_content_type":"text/x-python","patch_set":1,"id":"5e12a6ab_b427556f","line":2374,"in_reply_to":"3c20d974_28eb395e","updated":"2025-10-14 13:32:17.000000000","message":"Well, I think the fact that we don\u0027t have any of these other types of exceptions intentionally bubbling out of the RPC interface is why I think it\u0027s too detailed. And, there\u0027s prior art for a lot of it to be caught internal to the compute manager and turned into a BuildAbortException (at least in the case of the spawn-related ones).\n\nMaybe there are more that do escape (and trace) and we\u0027re just not catching them for those operations, I dunno.","commit_id":"c3ab178419d99fcddf53e5235ef774c471c2dba1"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"90c27f32c91df428e0b7756e44ce5bc6541500bd","unresolved":true,"context_lines":[{"line_number":2371,"context_line":"    # error from the real work is not propagated back to these decorators."},{"line_number":2372,"context_line":"    # All those error needs to be handled by _locked_do_build_and_run_instance"},{"line_number":2373,"context_line":"    # and _do_build_and_run_instance"},{"line_number":2374,"context_line":"    @messaging.expected_exceptions(exception.VTPMSecretForbidden)"},{"line_number":2375,"context_line":"    @wrap_exception()"},{"line_number":2376,"context_line":"    @reverts_task_state"},{"line_number":2377,"context_line":"    @wrap_instance_fault"}],"source_content_type":"text/x-python","patch_set":1,"id":"e57a9950_53971334","line":2374,"in_reply_to":"4bb087d4_d99859a7","updated":"2025-10-14 20:44:17.000000000","message":"\u003e Maybe there are more that do escape (and trace) and we\u0027re just not catching them for those operations, I dunno.\n\nI suspect this is also true. I guess we could maybe expected exception only the most common cases we expect (ha) to see, like build, start and reboot. If someone does an oopsie for delete, resume, or restore, then we let that trace.","commit_id":"c3ab178419d99fcddf53e5235ef774c471c2dba1"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"c4bcd8a8eac1e38c811411b8c4d2554488fe78f8","unresolved":true,"context_lines":[{"line_number":2371,"context_line":"    # error from the real work is not propagated back to these decorators."},{"line_number":2372,"context_line":"    # All those error needs to be handled by _locked_do_build_and_run_instance"},{"line_number":2373,"context_line":"    # and _do_build_and_run_instance"},{"line_number":2374,"context_line":"    @messaging.expected_exceptions(exception.VTPMSecretForbidden)"},{"line_number":2375,"context_line":"    @wrap_exception()"},{"line_number":2376,"context_line":"    @reverts_task_state"},{"line_number":2377,"context_line":"    @wrap_instance_fault"}],"source_content_type":"text/x-python","patch_set":1,"id":"4bb087d4_d99859a7","line":2374,"in_reply_to":"5e12a6ab_b427556f","updated":"2025-10-14 16:25:44.000000000","message":"OK, the BuildAbortException example makes it more clear and I didn\u0027t think of the possibility of translating to BuildAbortException (or something similar) while keeping the error message so that the fail reason is still clear. That makes a lot more sense 😝\n\nI\u0027ll try and rework this to look more like the usual pattern.","commit_id":"c3ab178419d99fcddf53e5235ef774c471c2dba1"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"d09f398e14d82b8122a373de19404b4e87d4ae81","unresolved":false,"context_lines":[{"line_number":2371,"context_line":"    # error from the real work is not propagated back to these decorators."},{"line_number":2372,"context_line":"    # All those error needs to be handled by _locked_do_build_and_run_instance"},{"line_number":2373,"context_line":"    # and _do_build_and_run_instance"},{"line_number":2374,"context_line":"    @messaging.expected_exceptions(exception.VTPMSecretForbidden)"},{"line_number":2375,"context_line":"    @wrap_exception()"},{"line_number":2376,"context_line":"    @reverts_task_state"},{"line_number":2377,"context_line":"    @wrap_instance_fault"}],"source_content_type":"text/x-python","patch_set":1,"id":"24bfdbed_0e252254","line":2374,"in_reply_to":"e57a9950_53971334","updated":"2025-10-20 20:07:05.000000000","message":"Done","commit_id":"c3ab178419d99fcddf53e5235ef774c471c2dba1"}],"nova/conductor/manager.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"f49bee06c9f9167eb756df200ccc8a89d3252094","unresolved":true,"context_lines":[{"line_number":305,"context_line":"        exception.InstanceInvalidState,"},{"line_number":306,"context_line":"        exception.MigrationPreCheckError,"},{"line_number":307,"context_line":"        exception.UnsupportedPolicyException,"},{"line_number":308,"context_line":"        exception.VTPMSecretForbidden)"},{"line_number":309,"context_line":"    @targets_cell"},{"line_number":310,"context_line":"    @wrap_instance_event(prefix\u003d\u0027conductor\u0027)"},{"line_number":311,"context_line":"    def migrate_server(self, context, instance, scheduler_hint, live, rebuild,"}],"source_content_type":"text/x-python","patch_set":1,"id":"f263fdb8_0b62428a","line":308,"updated":"2025-10-13 16:48:19.000000000","message":"Here, we see a lot of more fine-grained errors already being called out so I guess I\u0027m less concerned about it, although I do wonder if it makes sense for us to be doing this in the first place.","commit_id":"c3ab178419d99fcddf53e5235ef774c471c2dba1"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"884e7cb3b805f9fc0b41846789acabd30fc70729","unresolved":false,"context_lines":[{"line_number":305,"context_line":"        exception.InstanceInvalidState,"},{"line_number":306,"context_line":"        exception.MigrationPreCheckError,"},{"line_number":307,"context_line":"        exception.UnsupportedPolicyException,"},{"line_number":308,"context_line":"        exception.VTPMSecretForbidden)"},{"line_number":309,"context_line":"    @targets_cell"},{"line_number":310,"context_line":"    @wrap_instance_event(prefix\u003d\u0027conductor\u0027)"},{"line_number":311,"context_line":"    def migrate_server(self, context, instance, scheduler_hint, live, rebuild,"}],"source_content_type":"text/x-python","patch_set":1,"id":"f8301bab_5df5cacd","line":308,"in_reply_to":"f263fdb8_0b62428a","updated":"2025-10-15 02:09:21.000000000","message":"Acknowledged","commit_id":"c3ab178419d99fcddf53e5235ef774c471c2dba1"}],"nova/crypto.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"f49bee06c9f9167eb756df200ccc8a89d3252094","unresolved":true,"context_lines":[{"line_number":207,"context_line":"                secret_uuid, instance\u003dinstance)"},{"line_number":208,"context_line":"            raise"},{"line_number":209,"context_line":"        except castellan_exception.KeyManagerError as e:"},{"line_number":210,"context_line":"            if \u0027Forbidden\u0027 not in str(e):"},{"line_number":211,"context_line":"                raise"},{"line_number":212,"context_line":"            LOG.error(str(e), instance\u003dinstance)"},{"line_number":213,"context_line":"            raise exception.VTPMSecretForbidden(str(e)) from None"}],"source_content_type":"text/x-python","patch_set":1,"id":"4b62a6b1_1e656b53","line":210,"updated":"2025-10-13 16:48:19.000000000","message":"Is there no HTTP code or better thing we can key off of than this? A translated error message (which I know shouldn\u0027t be a thing anymore) would make this not match.","commit_id":"c3ab178419d99fcddf53e5235ef774c471c2dba1"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"884e7cb3b805f9fc0b41846789acabd30fc70729","unresolved":false,"context_lines":[{"line_number":207,"context_line":"                secret_uuid, instance\u003dinstance)"},{"line_number":208,"context_line":"            raise"},{"line_number":209,"context_line":"        except castellan_exception.KeyManagerError as e:"},{"line_number":210,"context_line":"            if \u0027Forbidden\u0027 not in str(e):"},{"line_number":211,"context_line":"                raise"},{"line_number":212,"context_line":"            LOG.error(str(e), instance\u003dinstance)"},{"line_number":213,"context_line":"            raise exception.VTPMSecretForbidden(str(e)) from None"}],"source_content_type":"text/x-python","patch_set":1,"id":"d4d13969_bfe4417a","line":210,"in_reply_to":"406a018b_583a3f56","updated":"2025-10-15 02:09:21.000000000","message":"Acknowledged","commit_id":"c3ab178419d99fcddf53e5235ef774c471c2dba1"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"284d7c3a66c1006092c3f85e11dfb1b568c73bed","unresolved":true,"context_lines":[{"line_number":207,"context_line":"                secret_uuid, instance\u003dinstance)"},{"line_number":208,"context_line":"            raise"},{"line_number":209,"context_line":"        except castellan_exception.KeyManagerError as e:"},{"line_number":210,"context_line":"            if \u0027Forbidden\u0027 not in str(e):"},{"line_number":211,"context_line":"                raise"},{"line_number":212,"context_line":"            LOG.error(str(e), instance\u003dinstance)"},{"line_number":213,"context_line":"            raise exception.VTPMSecretForbidden(str(e)) from None"}],"source_content_type":"text/x-python","patch_set":1,"id":"b774ea82_65fc64f6","line":210,"in_reply_to":"4b62a6b1_1e656b53","updated":"2025-10-13 19:49:58.000000000","message":"There is not unfortunately. Castellan swallows the code and all before it re-raises what we end up receiving here:\n\nhttps://github.com/openstack/castellan/blob/615e42ad304cf1d9a96b114c8d6ec3a55fbe6f66/castellan/key_manager/barbican_key_manager.py#L591-L613\n\nFurther down the stack is what came from the Barbican client, which has a `status_code` attribute:\n```\nERROR oslo_messaging.rpc.server barbicanclient.exceptions.HTTPClientError: Forbidden: Secret payload retrieval attempt not allowed - please review your user/project privileges\n```\n\nbut that\u0027s caught and re-raised by Castellan and we receive this, which does not have any status code attribute, only a `message` attribute:\n```\nERROR oslo_messaging.rpc.server castellan.common.exception.KeyManagerError: Key manager error: Forbidden: Secret payload retrieval attempt not allowed - please review your user/project privileges\n```\n\nSo I was not aware if there is anything better we can do here?","commit_id":"c3ab178419d99fcddf53e5235ef774c471c2dba1"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"5cce4a672e2088e98f6090b5e09fa90b6b501653","unresolved":true,"context_lines":[{"line_number":207,"context_line":"                secret_uuid, instance\u003dinstance)"},{"line_number":208,"context_line":"            raise"},{"line_number":209,"context_line":"        except castellan_exception.KeyManagerError as e:"},{"line_number":210,"context_line":"            if \u0027Forbidden\u0027 not in str(e):"},{"line_number":211,"context_line":"                raise"},{"line_number":212,"context_line":"            LOG.error(str(e), instance\u003dinstance)"},{"line_number":213,"context_line":"            raise exception.VTPMSecretForbidden(str(e)) from None"}],"source_content_type":"text/x-python","patch_set":1,"id":"406a018b_583a3f56","line":210,"in_reply_to":"b774ea82_65fc64f6","updated":"2025-10-14 13:32:17.000000000","message":"Yeah, maybe not, that\u0027s unfortunate.","commit_id":"c3ab178419d99fcddf53e5235ef774c471c2dba1"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"19190a8d0937cf35fb58fa8de97e62d40986855d","unresolved":true,"context_lines":[{"line_number":194,"context_line":"        # do is look for the word \"Forbidden\". Example error message:"},{"line_number":195,"context_line":"        #   \"Key manager error: Forbidden: Secret payload retrieval attempt not"},{"line_number":196,"context_line":"        #   allowed - please review your user/project privileges\""},{"line_number":197,"context_line":"        if \u0027Forbidden\u0027 not in str(e):"},{"line_number":198,"context_line":"            raise"},{"line_number":199,"context_line":"        LOG.error(str(e), instance\u003dinstance)"},{"line_number":200,"context_line":"        raise exception.VTPMSecretForbidden(str(e)) from None"}],"source_content_type":"text/x-python","patch_set":2,"id":"78c6f38e_7a48e93d","line":197,"updated":"2025-10-21 16:13:11.000000000","message":"a little bit hacky but fair, maybe we could mention a FIXME that says we could ask Castellan to do their job properly and not swallow exceptions that way ?","commit_id":"1924e4aca69cc183f61c65d74b99198eb58b5bfd"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"6f6f83b3da11427e54f425369ddb6a150aca6b35","unresolved":true,"context_lines":[{"line_number":194,"context_line":"        # do is look for the word \"Forbidden\". Example error message:"},{"line_number":195,"context_line":"        #   \"Key manager error: Forbidden: Secret payload retrieval attempt not"},{"line_number":196,"context_line":"        #   allowed - please review your user/project privileges\""},{"line_number":197,"context_line":"        if \u0027Forbidden\u0027 not in str(e):"},{"line_number":198,"context_line":"            raise"},{"line_number":199,"context_line":"        LOG.error(str(e), instance\u003dinstance)"},{"line_number":200,"context_line":"        raise exception.VTPMSecretForbidden(str(e)) from None"}],"source_content_type":"text/x-python","patch_set":2,"id":"f4db8b03_15d26d8b","line":197,"in_reply_to":"78c6f38e_7a48e93d","updated":"2025-10-21 16:15:37.000000000","message":"Yes, my same complaint in an earlier version of this patch. However, you\u0027ll see from discussion on the later one that we were currently not catching this at all and it was bubbling up VERY far. So this is at least an improvement over the current situation, which existed even before this series.","commit_id":"1924e4aca69cc183f61c65d74b99198eb58b5bfd"}],"nova/exception.py":[{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"19190a8d0937cf35fb58fa8de97e62d40986855d","unresolved":true,"context_lines":[{"line_number":2669,"context_line":""},{"line_number":2670,"context_line":""},{"line_number":2671,"context_line":"class VTPMSecretForbidden(Forbidden):"},{"line_number":2672,"context_line":"    pass"}],"source_content_type":"text/x-python","patch_set":2,"id":"7af2b994_6538319e","line":2672,"updated":"2025-10-21 16:13:11.000000000","message":"this doesn\u0027t bubble up to the API, right? If so, I\u0027m cool with it.","commit_id":"1924e4aca69cc183f61c65d74b99198eb58b5bfd"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"b14497b6171654247876ed054e6358c6cb610714","unresolved":true,"context_lines":[{"line_number":2669,"context_line":""},{"line_number":2670,"context_line":""},{"line_number":2671,"context_line":"class VTPMSecretForbidden(Forbidden):"},{"line_number":2672,"context_line":"    pass"}],"source_content_type":"text/x-python","patch_set":2,"id":"d2ccfa8d_64da4b4b","line":2672,"in_reply_to":"7af2b994_6538319e","updated":"2025-10-29 22:13:29.000000000","message":"No it does not because the places it is raised are after RPC casts (not calls).","commit_id":"1924e4aca69cc183f61c65d74b99198eb58b5bfd"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"56dfe787c31e74127179680e6c9ca266ffea51ae","unresolved":false,"context_lines":[{"line_number":2669,"context_line":""},{"line_number":2670,"context_line":""},{"line_number":2671,"context_line":"class VTPMSecretForbidden(Forbidden):"},{"line_number":2672,"context_line":"    pass"}],"source_content_type":"text/x-python","patch_set":2,"id":"1a88fe8a_82c10d63","line":2672,"in_reply_to":"d2ccfa8d_64da4b4b","updated":"2025-11-05 17:20:27.000000000","message":"cool","commit_id":"1924e4aca69cc183f61c65d74b99198eb58b5bfd"}]}