)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"a41597b7a6be0fdd5f8739e5458fa5ed60cb11ac","unresolved":true,"context_lines":[{"line_number":18,"context_line":"support for the RAW image backend. The Flat image backend was the only"},{"line_number":19,"context_line":"backend that did not have a driver capabilities unit test."},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"Related to blueprint ephemeral-encryption-libvirt"},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"Change-Id: Ie5788c791fae000573c488ea31af7a15799e1404"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":28,"id":"97822c3f_c28c58b3","line":21,"updated":"2024-02-13 06:24:58.000000000","message":"note this is the first time the feature is useable","commit_id":"a0bb38491f7b8db23e99d00ae30ff6f4ae703749"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"990b102cdc28399276c4af240f733fb614ddb5c6","unresolved":false,"context_lines":[{"line_number":18,"context_line":"support for the RAW image backend. The Flat image backend was the only"},{"line_number":19,"context_line":"backend that did not have a driver capabilities unit test."},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"Related to blueprint ephemeral-encryption-libvirt"},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"Change-Id: Ie5788c791fae000573c488ea31af7a15799e1404"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":28,"id":"c1b99b3d_514229dd","line":21,"in_reply_to":"97822c3f_c28c58b3","updated":"2024-02-14 09:48:15.000000000","message":"Acknowledged","commit_id":"a0bb38491f7b8db23e99d00ae30ff6f4ae703749"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"9b491d1ad9ec3c8d165eeb2b0466469bbdc2c45b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"a0b2c1c7_1b6e6a87","updated":"2023-07-04 14:54:35.000000000","message":"This patch looks good to me","commit_id":"8028d6904c84da76dd7aa79698923e1919a263a2"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"2fa214491af72660ad5be4769acf13bc1bb8a7ce","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":24,"id":"3d00771a_a877635c","updated":"2024-01-31 09:22:59.000000000","message":"i think the rescue part is not correct. perhaps we should move that to a seperate patch and focus on spwan here?","commit_id":"e40a49f0c6859c9561d68105cc706b6c798a7b80"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"7ce98e988c0eb7a823406450d010f62660322d6b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":24,"id":"0ea36a8c_be26a88d","updated":"2024-01-31 08:56:04.000000000","message":"recheck the nova-lvm job has 3 unrelated failures.","commit_id":"e40a49f0c6859c9561d68105cc706b6c798a7b80"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"4ad3f7a190356f6fe5986232d6523eb5d42a2a2d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":24,"id":"962ffe9b_e78055e1","updated":"2024-01-31 11:29:03.000000000","message":"so this is the patch where i think we should be starting the release note for this feature.\n\nwith that said it wont work properly yet in real code.\n\nso im conflicted becaue i would have expecte all api actions that done work yet to be decorated with a check for encyption. and reject it on intances with encyption if they are not supported yet.\nwe have done this at the compute api level in the past for cyborg or vdpa\n\nhttps://github.com/openstack/nova/blob/a66a0dd113d0b57acf9f69d76785be39479eebc0/nova/compute/api.py#L284\n\nbut since this is libvirt specific and host specific it would have to be done in the libvirt driver instead.\n \ni expected the images backed, at least the qcow one reporting support for lux at this poitn and for the traits to be advertiesed.\n\nsince you havnt done that yet you cant realy start on the release note since it wont work in devstack to create and instnace \n\ndo you think this is somethign we can change in the time we have remaining","commit_id":"e40a49f0c6859c9561d68105cc706b6c798a7b80"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"00435d585ff7cd47a3515561cd97e334e16072d9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":24,"id":"43b4a624_d4d382a7","in_reply_to":"00db1cf4_8f536207","updated":"2024-02-29 07:37:13.000000000","message":"Done","commit_id":"e40a49f0c6859c9561d68105cc706b6c798a7b80"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"00435d585ff7cd47a3515561cd97e334e16072d9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":24,"id":"d009e3b2_a15acf46","in_reply_to":"00db1cf4_8f536207","updated":"2024-02-29 07:37:13.000000000","message":"Done","commit_id":"e40a49f0c6859c9561d68105cc706b6c798a7b80"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"7e9184b4ca5c8cfe2f2bc9ab97a0fe529a50025d","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":24,"id":"00db1cf4_8f536207","in_reply_to":"962ffe9b_e78055e1","updated":"2024-02-01 02:14:49.000000000","message":"Yeah I\u0027ve gone ahead and switched the series to use the \"API reject decorator\" pattern because it was easy to do and it makes it easier for me to test too anyways. It will also make the idea of adding to the release note in each patch make more sense.","commit_id":"e40a49f0c6859c9561d68105cc706b6c798a7b80"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"14ccd4867fcdbd1f02cfa194af92acdc1e1be5a2","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":27,"id":"a83a0dfa_7ed41353","updated":"2024-02-06 22:20:19.000000000","message":"recheck https://review.opendev.org/c/openstack/nova/+/908182 has merged","commit_id":"b1afa2328194f106d904a3b6d7ec9c875cf624e3"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"ff38c1f3767d524442ac370bf8a494757bd53578","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":29,"id":"99b6dbfc_22cea7cf","updated":"2024-02-14 13:20:04.000000000","message":"I already had a pass to this patch. Looks good on my side.","commit_id":"54aba971a29b2a245d6f57ae2fa63065b848977a"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"8596e2fcd4cee9146f5dd317983332f74bb5ee19","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":32,"id":"9b6991dc_b838274b","updated":"2024-02-27 22:19:05.000000000","message":"Man, what a mess this delete path is. I think a functional test for that scenario is a good idea, and it will sanity check whether I\u0027m following all the spaghetti correctly and/or confirm that we\u0027re doing the right thing already.","commit_id":"26cdb2b6fc4f4c45c84ce0f59ac8417f987feefa"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"27e23ee299de03ce947906d928d124bdb39338cd","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":32,"id":"b3961f2a_906ccd27","updated":"2024-02-27 19:56:07.000000000","message":"This is a big one, gonna take some time to digest, but ... some questions to help accelerate.","commit_id":"26cdb2b6fc4f4c45c84ce0f59ac8417f987feefa"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"94b0ed419416067495dd5155ea51b235c65f92e2","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":32,"id":"8f4f7dfc_8b8a441a","updated":"2024-02-24 02:53:13.000000000","message":"recheck /usr/bin/podman: stderr Error: OCI runtime error: openat2 `var/run/ceph`: Resource temporarily unavailable","commit_id":"26cdb2b6fc4f4c45c84ce0f59ac8417f987feefa"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"55b3e3256311319eab207bd00f11171705bfb963","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":32,"id":"54ed6987_7361de0c","updated":"2024-02-23 19:57:46.000000000","message":"recheck nova-ceph-multistore TIMED_OUT","commit_id":"26cdb2b6fc4f4c45c84ce0f59ac8417f987feefa"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"4ddebc703f7320a6f33dc88d82a00635a1bd2f40","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":32,"id":"4da530d8_1885ed3e","updated":"2024-02-24 00:18:23.000000000","message":"recheck nova-lvm urllib3.exceptions.ReadTimeoutError: HTTPSConnectionPool(host\u003d\u002710.209.128.100\u0027, port\u003d443): Read timed out. (read timeout\u003d60)","commit_id":"26cdb2b6fc4f4c45c84ce0f59ac8417f987feefa"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"aa5bffae92400dc7ad17474b2ab725262f3832d6","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":32,"id":"3b889be3_07f3c95c","updated":"2024-02-24 06:26:19.000000000","message":"recheck nova-lvm urllib3.exceptions.ReadTimeoutError: HTTPSConnectionPool(host\u003d\u002710.209.65.199\u0027, port\u003d443): Read timed out. (read timeout\u003d60)","commit_id":"26cdb2b6fc4f4c45c84ce0f59ac8417f987feefa"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"26d2bd38cd666cbeb995912791d12f69ec381d8e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":32,"id":"b0884f8d_d35dbfa0","updated":"2024-02-23 12:24:43.000000000","message":"recheck nova-next is now fixed","commit_id":"26cdb2b6fc4f4c45c84ce0f59ac8417f987feefa"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"e05172789961e56295719cdc0cac3696f91e0a4a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":32,"id":"80de2b77_3fe3f0ce","updated":"2024-02-22 09:36:09.000000000","message":"recheck post failue","commit_id":"26cdb2b6fc4f4c45c84ce0f59ac8417f987feefa"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"591a226603a660c6744585c72376ed068167a8a1","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":32,"id":"68ba8414_a9b5cf27","updated":"2024-02-23 22:06:28.000000000","message":"recheck tempest-integrated-compute urllib3.exceptions.ReadTimeoutError: HTTPSConnectionPool(host\u003d\u0027213.32.73.54\u0027, port\u003d443): Read timed out. (read timeout\u003d60)","commit_id":"26cdb2b6fc4f4c45c84ce0f59ac8417f987feefa"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"fbf5c6aeb4ce823cf2512900fc32cecce3cf3a28","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":33,"id":"32eb4f0a_8b231f03","updated":"2024-02-28 19:49:21.000000000","message":"I\u0027m still chewing through this, but .. any thoughts on breaking out some of the fixture and test setup stuff? We could move that ahead in the stack and maybe get it merged sooner, but at very least it seems like it would slim down this patch a bunch.","commit_id":"31a028d86a90ce86b3cbb5950932e08a812cc465"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"00435d585ff7cd47a3515561cd97e334e16072d9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":33,"id":"89c62806_d24ad0aa","in_reply_to":"17f5b4f4_d06d17d5","updated":"2024-02-29 07:37:13.000000000","message":"Done","commit_id":"31a028d86a90ce86b3cbb5950932e08a812cc465"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"62a137d49ce924dbfcd597855024c59fb7836266","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":33,"id":"17f5b4f4_d06d17d5","in_reply_to":"32eb4f0a_8b231f03","updated":"2024-02-28 21:26:44.000000000","message":"Yes, we can do that to tidy up here. I\u0027ll move it out.","commit_id":"31a028d86a90ce86b3cbb5950932e08a812cc465"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"ac9dce6d9cd3c24a148d76f6863f6091f92d4d50","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":37,"id":"fcd2280f_81311b1f","updated":"2024-03-18 17:21:41.000000000","message":"recheck [   10.424856] ---[ end Kernel panic - not syncing: Attempted to kill init! exitcode\u003d0x00001000 ]---","commit_id":"9090492a61d31e420d7aed181c710db2408757da"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"b364c96f4162e6b78e9140af0b86bd08399afc4a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":38,"id":"3fa599c1_4e056bc7","updated":"2024-05-17 23:40:43.000000000","message":"recheck nova-multi-cell server did not reach ACTIVE within 196 seconds, it took 202 seconds","commit_id":"77bf446d789f2bf4ce4adf13616ffc53aa217cd1"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"a5d618d58bbdb2c795826a3d939eeda714dba223","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":39,"id":"fa8fe466_ef18fe26","updated":"2024-06-06 06:28:20.000000000","message":"recheck 2024-06-05 00:30:58,892 93117 WARNING  [tempest.api.compute.servers.test_server_actions] Deletion of oldest backup 178fab8f-5d30-448f-a832-f5c0befd8c10 should not have been successful as it should have been deleted during rotation.","commit_id":"5b1acde7d749ffadbece050852125888877396e1"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"b117be7ef80b5219ddb2bbf3a15568855a101dcf","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":41,"id":"90e001cd_49568250","updated":"2024-06-10 16:48:22.000000000","message":"recheck ssh timeout guest sshd not completely up?\n```\n2024-06-10 05:07:33,366 84213 DEBUG    [tempest.lib.common.utils.linux.remote_client] Caller: ServerActionsTestOtherA:test_resize_volume_backed_server_confirm. Timeout trying to ssh to server {\u0027id\u0027: \u0027284a9286-1ea2-4276-9a35-41bdccd0be23\u0027, \u0027name\u0027: \u0027tempest-ServerActionsTestOtherA-server-1245180215\u0027, \u0027status\u0027: \u0027ACTIVE\u0027, \u0027tenant_id\u0027: \u00272afb179314614bd5b5e481edbabfbe4b\u0027, \u0027user_id\u0027: \u0027eb1eb9cbf2dc4a21aa68f4c748329311\u0027, \u0027metadata\u0027: {}, \u0027hostId\u0027: \u0027bc6901ca9a8cb94bd7d705503512bdb2f6b152e82b0108fdf83ffe0c\u0027, \u0027image\u0027: \u0027\u0027, \u0027flavor\u0027: {\u0027id\u0027: \u002742\u0027, \u0027links\u0027: [{\u0027rel\u0027: \u0027bookmark\u0027, \u0027href\u0027: \u0027https://10.209.100.59/compute/flavors/42\u0027}]}, \u0027created\u0027: \u00272024-06-10T05:02:57Z\u0027, \u0027updated\u0027: \u00272024-06-10T05:03:23Z\u0027, \u0027addresses\u0027: {\u0027tempest-ServerActionsTestOtherA-1990629705-network\u0027: [{\u0027version\u0027: 4, \u0027addr\u0027: \u002710.1.0.9\u0027, \u0027OS-EXT-IPS:type\u0027: \u0027fixed\u0027, \u0027OS-EXT-IPS-MAC:mac_addr\u0027: \u0027fa:16:3e:af:61:4b\u0027}]}, \u0027accessIPv4\u0027: \u0027\u0027, \u0027accessIPv6\u0027: \u0027\u0027, \u0027links\u0027: [{\u0027rel\u0027: \u0027self\u0027, \u0027href\u0027: \u0027https://10.209.100.59/compute/v2.1/servers/284a9286-1ea2-4276-9a35-41bdccd0be23\u0027}, {\u0027rel\u0027: \u0027bookmark\u0027, \u0027href\u0027: \u0027https://10.209.100.59/compute/servers/284a9286-1ea2-4276-9a35-41bdccd0be23\u0027}], \u0027OS-DCF:diskConfig\u0027: \u0027MANUAL\u0027, \u0027progress\u0027: 0, \u0027OS-EXT-AZ:availability_zone\u0027: \u0027nova\u0027, \u0027config_drive\u0027: \u0027True\u0027, \u0027key_name\u0027: \u0027tempest-keypair-1240677227\u0027, \u0027OS-SRV-USG:launched_at\u0027: \u00272024-06-10T05:03:22.000000\u0027, \u0027OS-SRV-USG:terminated_at\u0027: None, \u0027security_groups\u0027: [{\u0027name\u0027: \u0027tempest-securitygroup--3478468\u0027}], \u0027OS-EXT-STS:task_state\u0027: None, \u0027OS-EXT-STS:vm_state\u0027: \u0027active\u0027, \u0027OS-EXT-STS:power_state\u0027: 1, \u0027os-extended-volumes:volumes_attached\u0027: [{\u0027id\u0027: \u0027c3e8d29d-d9ef-4cfe-ae3c-ed09f74ee284\u0027}]}\n\n```","commit_id":"18e1f52edb0956b69e7623b62a608c3a5fb332d4"}],"nova/compute/api.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"a41597b7a6be0fdd5f8739e5458fa5ed60cb11ac","unresolved":true,"context_lines":[{"line_number":312,"context_line":"    return outer"},{"line_number":313,"context_line":""},{"line_number":314,"context_line":""},{"line_number":315,"context_line":"def reject_ephemeral_encryption_instances(operation):"},{"line_number":316,"context_line":"    \"\"\"Reject requests to decorated funcs if instance uses ephemeral encryption"},{"line_number":317,"context_line":""},{"line_number":318,"context_line":"    Raise OperationNotSupportedForEphemeralEncryption if instance uses"}],"source_content_type":"text/x-python","patch_set":28,"id":"7a7a8e7d_fbf60bbd","line":315,"updated":"2024-02-13 06:24:58.000000000","message":"+1 thanks for refactoring to this approch","commit_id":"a0bb38491f7b8db23e99d00ae30ff6f4ae703749"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"990b102cdc28399276c4af240f733fb614ddb5c6","unresolved":false,"context_lines":[{"line_number":312,"context_line":"    return outer"},{"line_number":313,"context_line":""},{"line_number":314,"context_line":""},{"line_number":315,"context_line":"def reject_ephemeral_encryption_instances(operation):"},{"line_number":316,"context_line":"    \"\"\"Reject requests to decorated funcs if instance uses ephemeral encryption"},{"line_number":317,"context_line":""},{"line_number":318,"context_line":"    Raise OperationNotSupportedForEphemeralEncryption if instance uses"}],"source_content_type":"text/x-python","patch_set":28,"id":"c4979344_f36e457f","line":315,"in_reply_to":"7a7a8e7d_fbf60bbd","updated":"2024-02-14 09:48:15.000000000","message":"Acknowledged","commit_id":"a0bb38491f7b8db23e99d00ae30ff6f4ae703749"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"ff38c1f3767d524442ac370bf8a494757bd53578","unresolved":true,"context_lines":[{"line_number":312,"context_line":"    return outer"},{"line_number":313,"context_line":""},{"line_number":314,"context_line":""},{"line_number":315,"context_line":"def reject_ephemeral_encryption_instances(operation):"},{"line_number":316,"context_line":"    \"\"\"Reject requests to decorated funcs if instance uses ephemeral encryption"},{"line_number":317,"context_line":""},{"line_number":318,"context_line":"    Raise OperationNotSupportedForEphemeralEncryption if instance uses"}],"source_content_type":"text/x-python","patch_set":29,"id":"0c3ffd10_9c9af9c7","line":315,"range":{"start_line":315,"start_character":4,"end_line":315,"end_character":41},"updated":"2024-02-14 13:20:04.000000000","message":"I had the same need to block for unsupported operation and implemented something similar on my side.\nThat sad we don\u0027t have a common API to do that. Maybe an idea for next PTG ?","commit_id":"54aba971a29b2a245d6f57ae2fa63065b848977a"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"4386cbe1c95fa1bdd109257d8931f6048d3f9dae","unresolved":true,"context_lines":[{"line_number":312,"context_line":"    return outer"},{"line_number":313,"context_line":""},{"line_number":314,"context_line":""},{"line_number":315,"context_line":"def reject_ephemeral_encryption_instances(operation):"},{"line_number":316,"context_line":"    \"\"\"Reject requests to decorated funcs if instance uses ephemeral encryption"},{"line_number":317,"context_line":""},{"line_number":318,"context_line":"    Raise OperationNotSupportedForEphemeralEncryption if instance uses"}],"source_content_type":"text/x-python","patch_set":29,"id":"9d70865c_2ed23327","line":315,"range":{"start_line":315,"start_character":4,"end_line":315,"end_character":41},"in_reply_to":"0c3ffd10_9c9af9c7","updated":"2024-02-22 09:35:42.000000000","message":"well we do this is the establised pattern.\ncreate a decorator and apply it to the unsupported apis.\n\nim not sure we should get any more complext for this.\n\nthe only other thing i can think of is to have a function that takes a list of funcitons to decorate, a predicate and an exctpion to raisse if that predicate fails but that does not really seam like an improvement over our current patteren.","commit_id":"54aba971a29b2a245d6f57ae2fa63065b848977a"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"27e23ee299de03ce947906d928d124bdb39338cd","unresolved":true,"context_lines":[{"line_number":312,"context_line":"    return outer"},{"line_number":313,"context_line":""},{"line_number":314,"context_line":""},{"line_number":315,"context_line":"def reject_ephemeral_encryption_instances(operation):"},{"line_number":316,"context_line":"    \"\"\"Reject requests to decorated funcs if instance uses ephemeral encryption"},{"line_number":317,"context_line":""},{"line_number":318,"context_line":"    Raise OperationNotSupportedForEphemeralEncryption if instance uses"}],"source_content_type":"text/x-python","patch_set":29,"id":"a8ebf2d8_476c06c5","line":315,"range":{"start_line":315,"start_character":4,"end_line":315,"end_character":41},"in_reply_to":"9d70865c_2ed23327","updated":"2024-02-27 19:56:07.000000000","message":"Agree, this sort of decorator approach is very concise, easy to understand, and conventional in nova.","commit_id":"54aba971a29b2a245d6f57ae2fa63065b848977a"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"4386cbe1c95fa1bdd109257d8931f6048d3f9dae","unresolved":true,"context_lines":[{"line_number":2363,"context_line":"            return True"},{"line_number":2364,"context_line":"        return False"},{"line_number":2365,"context_line":""},{"line_number":2366,"context_line":"    def _local_delete_cleanup(self, context, instance_uuid, bdms\u003dNone):"},{"line_number":2367,"context_line":"        # NOTE(aarents) Ensure instance allocation is cleared and instance"},{"line_number":2368,"context_line":"        # mapping queued as deleted before _delete() return"},{"line_number":2369,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":32,"id":"cea8bb90_21baa14f","line":2366,"updated":"2024-02-22 09:35:42.000000000","message":"ah right local delete... ya that should proably be supported too...\n\ni missed that last time i reviewed.","commit_id":"26cdb2b6fc4f4c45c84ce0f59ac8417f987feefa"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"27e23ee299de03ce947906d928d124bdb39338cd","unresolved":true,"context_lines":[{"line_number":2380,"context_line":"                     \"local delete cleanup.\","},{"line_number":2381,"context_line":"                     instance_uuid\u003dinstance_uuid)"},{"line_number":2382,"context_line":""},{"line_number":2383,"context_line":"        # Clean up ephemeral encryption secrets if needed."},{"line_number":2384,"context_line":"        if bdms is None:"},{"line_number":2385,"context_line":"            bdms \u003d objects.BlockDeviceMappingList.get_by_instance_uuid("},{"line_number":2386,"context_line":"                context, instance_uuid)"}],"source_content_type":"text/x-python","patch_set":32,"id":"945bc0ec_fd4e8d01","line":2383,"updated":"2024-02-27 19:56:07.000000000","message":"...in barbican, not libvirt. I haven\u0027t seen (or gone looking) but we probably also want an `init_host()` section on compute to clean up secrets for instances that aren\u0027t on our host. Nothing juicier than compromising a host and realizing there\u0027s a treasure trove of secrets for other people\u0027s instances waiting for you.","commit_id":"26cdb2b6fc4f4c45c84ce0f59ac8417f987feefa"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"a87c49c69f58a3671949e2e892ef7b32d7fd826e","unresolved":true,"context_lines":[{"line_number":2380,"context_line":"                     \"local delete cleanup.\","},{"line_number":2381,"context_line":"                     instance_uuid\u003dinstance_uuid)"},{"line_number":2382,"context_line":""},{"line_number":2383,"context_line":"        # Clean up ephemeral encryption secrets if needed."},{"line_number":2384,"context_line":"        if bdms is None:"},{"line_number":2385,"context_line":"            bdms \u003d objects.BlockDeviceMappingList.get_by_instance_uuid("},{"line_number":2386,"context_line":"                context, instance_uuid)"}],"source_content_type":"text/x-python","patch_set":32,"id":"5b09122d_597a5050","line":2383,"in_reply_to":"049e30e0_5a7407a7","updated":"2024-02-27 21:51:02.000000000","message":"Yeah, I think we should do that (the former) so that no compute host is hanging out with secrets for things it doesn\u0027t own. Or, \"need to know\" so to speak.","commit_id":"26cdb2b6fc4f4c45c84ce0f59ac8417f987feefa"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"f060ff835f652c4c9c7a07c238af147edaaa76a0","unresolved":true,"context_lines":[{"line_number":2380,"context_line":"                     \"local delete cleanup.\","},{"line_number":2381,"context_line":"                     instance_uuid\u003dinstance_uuid)"},{"line_number":2382,"context_line":""},{"line_number":2383,"context_line":"        # Clean up ephemeral encryption secrets if needed."},{"line_number":2384,"context_line":"        if bdms is None:"},{"line_number":2385,"context_line":"            bdms \u003d objects.BlockDeviceMappingList.get_by_instance_uuid("},{"line_number":2386,"context_line":"                context, instance_uuid)"}],"source_content_type":"text/x-python","patch_set":32,"id":"69fd28ba_b9ca2be8","line":2383,"in_reply_to":"5b09122d_597a5050","updated":"2024-02-27 22:04:45.000000000","message":"Yeah, makes sense. I\u0027ll add that.","commit_id":"26cdb2b6fc4f4c45c84ce0f59ac8417f987feefa"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"00435d585ff7cd47a3515561cd97e334e16072d9","unresolved":false,"context_lines":[{"line_number":2380,"context_line":"                     \"local delete cleanup.\","},{"line_number":2381,"context_line":"                     instance_uuid\u003dinstance_uuid)"},{"line_number":2382,"context_line":""},{"line_number":2383,"context_line":"        # Clean up ephemeral encryption secrets if needed."},{"line_number":2384,"context_line":"        if bdms is None:"},{"line_number":2385,"context_line":"            bdms \u003d objects.BlockDeviceMappingList.get_by_instance_uuid("},{"line_number":2386,"context_line":"                context, instance_uuid)"}],"source_content_type":"text/x-python","patch_set":32,"id":"11f81aa1_bc7eb24d","line":2383,"in_reply_to":"69fd28ba_b9ca2be8","updated":"2024-02-29 07:37:13.000000000","message":"Done","commit_id":"26cdb2b6fc4f4c45c84ce0f59ac8417f987feefa"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"641ac51e1dba3c26e9054c9c736ddf0a4b63811f","unresolved":true,"context_lines":[{"line_number":2380,"context_line":"                     \"local delete cleanup.\","},{"line_number":2381,"context_line":"                     instance_uuid\u003dinstance_uuid)"},{"line_number":2382,"context_line":""},{"line_number":2383,"context_line":"        # Clean up ephemeral encryption secrets if needed."},{"line_number":2384,"context_line":"        if bdms is None:"},{"line_number":2385,"context_line":"            bdms \u003d objects.BlockDeviceMappingList.get_by_instance_uuid("},{"line_number":2386,"context_line":"                context, instance_uuid)"}],"source_content_type":"text/x-python","patch_set":32,"id":"049e30e0_5a7407a7","line":2383,"in_reply_to":"945bc0ec_fd4e8d01","updated":"2024-02-27 21:05:16.000000000","message":"Right. I did this at the compute level instead of the virt driver level to ensure that it only happens in instance delete paths after the instance\u0027s disks have been deleted. The idea being not to delete the Barbican secrets until we are guaranteed not to need them anymore. I tied the lifetime of the Barbican secrets for an instance to the lifetime of the instance they are for.\n\nI didn\u0027t actually put a general global sweep of libvirt secrets in `init_host()` but it would be reasonable to add. Currently the only thing that happens in `init_host()` is secrets for instances that have evacuated away from that host are deleted.","commit_id":"26cdb2b6fc4f4c45c84ce0f59ac8417f987feefa"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"27e23ee299de03ce947906d928d124bdb39338cd","unresolved":true,"context_lines":[{"line_number":2385,"context_line":"            bdms \u003d objects.BlockDeviceMappingList.get_by_instance_uuid("},{"line_number":2386,"context_line":"                context, instance_uuid)"},{"line_number":2387,"context_line":"        compute_utils.delete_ephemeral_encryption_secrets("},{"line_number":2388,"context_line":"            context, instance_uuid, bdms)"},{"line_number":2389,"context_line":""},{"line_number":2390,"context_line":"    def _attempt_delete_of_buildrequest(self, context, instance):"},{"line_number":2391,"context_line":"        # If there is a BuildRequest then the instance may not have been"}],"source_content_type":"text/x-python","patch_set":32,"id":"8d9ca476_72ff75f4","line":2388,"updated":"2024-02-27 19:56:07.000000000","message":"So, hmm, how do we know that we want to delete all the secrets associated with this instance? Meaning, if we created an instance from a snapshot, did the secret for the snapshot get cloned for us to use so that we can delete ours? Obviously it would be bad if we deleted the one needed for other things.\n\nAlso, aren\u0027t the secrets passed into us via boot such that we wouldn\u0027t want to auto-delete them? I must be missing something about the workflow here.","commit_id":"26cdb2b6fc4f4c45c84ce0f59ac8417f987feefa"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"a87c49c69f58a3671949e2e892ef7b32d7fd826e","unresolved":true,"context_lines":[{"line_number":2385,"context_line":"            bdms \u003d objects.BlockDeviceMappingList.get_by_instance_uuid("},{"line_number":2386,"context_line":"                context, instance_uuid)"},{"line_number":2387,"context_line":"        compute_utils.delete_ephemeral_encryption_secrets("},{"line_number":2388,"context_line":"            context, instance_uuid, bdms)"},{"line_number":2389,"context_line":""},{"line_number":2390,"context_line":"    def _attempt_delete_of_buildrequest(self, context, instance):"},{"line_number":2391,"context_line":"        # If there is a BuildRequest then the instance may not have been"}],"source_content_type":"text/x-python","patch_set":32,"id":"92891a34_5c7f1ca7","line":2388,"in_reply_to":"06164922_6ad14096","updated":"2024-02-27 21:51:02.000000000","message":"Ah, right, okay so we copy on snapshot, and never delete those. But we assume we can always delete the secret for the instance\u0027s active disks.\n\nI should re-read the spec (again) now that I have my bearings (somewhat) in the implementation (again).","commit_id":"26cdb2b6fc4f4c45c84ce0f59ac8417f987feefa"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"641ac51e1dba3c26e9054c9c736ddf0a4b63811f","unresolved":true,"context_lines":[{"line_number":2385,"context_line":"            bdms \u003d objects.BlockDeviceMappingList.get_by_instance_uuid("},{"line_number":2386,"context_line":"                context, instance_uuid)"},{"line_number":2387,"context_line":"        compute_utils.delete_ephemeral_encryption_secrets("},{"line_number":2388,"context_line":"            context, instance_uuid, bdms)"},{"line_number":2389,"context_line":""},{"line_number":2390,"context_line":"    def _attempt_delete_of_buildrequest(self, context, instance):"},{"line_number":2391,"context_line":"        # If there is a BuildRequest then the instance may not have been"}],"source_content_type":"text/x-python","patch_set":32,"id":"06164922_6ad14096","line":2388,"in_reply_to":"8d9ca476_72ff75f4","updated":"2024-02-27 21:05:16.000000000","message":"Yeah, so this is best explained by this table in the latest proposed revision of the spec (this is a link to the doc preview):\n\nhttps://fe10dcd4a1eeb5225eb9-cd07d3426af01024dbeaac2f8639d2e0.ssl.cf5.rackcdn.com/907654/3/check/openstack-tox-docs/cbebecc/docs/specs/2024.1/approved/ephemeral-storage-encryption.html#create-a-new-key-manager-secret-for-every-new-encrypted-disk-image\n\nWhen we take the snapshot, we create a new secret for the snapshot image and put its secret in the `hw_ephemeral_encryption_secret_uuid` image property. When someone boots a new instance from it, a copy is made for the new instance\u0027s encrypted backing file so that the copy can be deleted if/when the instance is eventually deleted. The overlay/delta for the new instance will have a new secret created for it same as a normal boot.\n\nAs for the \"original\" copy of the `hw_ephemeral_encryption_secret_uuid` it is never automatically deleted by Nova. It would only be safe to delete that secret if and when the Glance image corresponding to it is deleted. So it would be up to the operator or admin when or if they want to delete the Glance image.","commit_id":"26cdb2b6fc4f4c45c84ce0f59ac8417f987feefa"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"f5f15b055e774718e4aa704e32e29bc2d9099556","unresolved":false,"context_lines":[{"line_number":2385,"context_line":"            bdms \u003d objects.BlockDeviceMappingList.get_by_instance_uuid("},{"line_number":2386,"context_line":"                context, instance_uuid)"},{"line_number":2387,"context_line":"        compute_utils.delete_ephemeral_encryption_secrets("},{"line_number":2388,"context_line":"            context, instance_uuid, bdms)"},{"line_number":2389,"context_line":""},{"line_number":2390,"context_line":"    def _attempt_delete_of_buildrequest(self, context, instance):"},{"line_number":2391,"context_line":"        # If there is a BuildRequest then the instance may not have been"}],"source_content_type":"text/x-python","patch_set":32,"id":"e2f1123d_4f84f029","line":2388,"in_reply_to":"92891a34_5c7f1ca7","updated":"2024-02-29 22:31:08.000000000","message":"Done","commit_id":"26cdb2b6fc4f4c45c84ce0f59ac8417f987feefa"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"8596e2fcd4cee9146f5dd317983332f74bb5ee19","unresolved":true,"context_lines":[{"line_number":2417,"context_line":"        # in error state), the instance has been scheduled and sent to a"},{"line_number":2418,"context_line":"        # cell/compute which means it was pulled from the cell db."},{"line_number":2419,"context_line":"        # Normal delete should be attempted."},{"line_number":2420,"context_line":"        may_have_ports_or_volumes \u003d compute_utils.may_have_ports_or_volumes("},{"line_number":2421,"context_line":"            instance)"},{"line_number":2422,"context_line":""},{"line_number":2423,"context_line":"        # Save a copy of the instance UUID early, in case"}],"source_content_type":"text/x-python","patch_set":32,"id":"fa7c8cd8_08862b89","line":2420,"updated":"2024-02-27 22:19:05.000000000","message":"I think this will be true if we\u0027re `SHELVED_OFFLOADED`","commit_id":"26cdb2b6fc4f4c45c84ce0f59ac8417f987feefa"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"f5f15b055e774718e4aa704e32e29bc2d9099556","unresolved":false,"context_lines":[{"line_number":2417,"context_line":"        # in error state), the instance has been scheduled and sent to a"},{"line_number":2418,"context_line":"        # cell/compute which means it was pulled from the cell db."},{"line_number":2419,"context_line":"        # Normal delete should be attempted."},{"line_number":2420,"context_line":"        may_have_ports_or_volumes \u003d compute_utils.may_have_ports_or_volumes("},{"line_number":2421,"context_line":"            instance)"},{"line_number":2422,"context_line":""},{"line_number":2423,"context_line":"        # Save a copy of the instance UUID early, in case"}],"source_content_type":"text/x-python","patch_set":32,"id":"47ac5c2c_3ca92535","line":2420,"in_reply_to":"fa7c8cd8_08862b89","updated":"2024-02-29 22:31:08.000000000","message":"Done","commit_id":"26cdb2b6fc4f4c45c84ce0f59ac8417f987feefa"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"8596e2fcd4cee9146f5dd317983332f74bb5ee19","unresolved":true,"context_lines":[{"line_number":2425,"context_line":"        # _local_delete_cleanup if needed."},{"line_number":2426,"context_line":"        instance_uuid \u003d instance.uuid"},{"line_number":2427,"context_line":""},{"line_number":2428,"context_line":"        if not instance.host and not may_have_ports_or_volumes:"},{"line_number":2429,"context_line":"            try:"},{"line_number":2430,"context_line":"                if self._delete_while_booting(context, instance):"},{"line_number":2431,"context_line":"                    self._local_delete_cleanup(context, instance.uuid)"}],"source_content_type":"text/x-python","patch_set":32,"id":"e8e65cea_a04f5648","line":2428,"updated":"2024-02-27 22:19:05.000000000","message":"We will pass the \"not host\" but fail the \"may_have\" so we won\u0027t run this.","commit_id":"26cdb2b6fc4f4c45c84ce0f59ac8417f987feefa"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"f5f15b055e774718e4aa704e32e29bc2d9099556","unresolved":false,"context_lines":[{"line_number":2425,"context_line":"        # _local_delete_cleanup if needed."},{"line_number":2426,"context_line":"        instance_uuid \u003d instance.uuid"},{"line_number":2427,"context_line":""},{"line_number":2428,"context_line":"        if not instance.host and not may_have_ports_or_volumes:"},{"line_number":2429,"context_line":"            try:"},{"line_number":2430,"context_line":"                if self._delete_while_booting(context, instance):"},{"line_number":2431,"context_line":"                    self._local_delete_cleanup(context, instance.uuid)"}],"source_content_type":"text/x-python","patch_set":32,"id":"e73118d5_6bd40827","line":2428,"in_reply_to":"e8e65cea_a04f5648","updated":"2024-02-29 22:31:08.000000000","message":"Done","commit_id":"26cdb2b6fc4f4c45c84ce0f59ac8417f987feefa"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"8596e2fcd4cee9146f5dd317983332f74bb5ee19","unresolved":true,"context_lines":[{"line_number":2468,"context_line":"                    self._local_delete_cleanup(context, instance_uuid)"},{"line_number":2469,"context_line":"                    return"},{"line_number":2470,"context_line":""},{"line_number":2471,"context_line":"        bdms \u003d objects.BlockDeviceMappingList.get_by_instance_uuid("},{"line_number":2472,"context_line":"                context, instance.uuid)"},{"line_number":2473,"context_line":""},{"line_number":2474,"context_line":"        # At these states an instance has a snapshot associate."}],"source_content_type":"text/x-python","patch_set":32,"id":"58c775d4_a05306a9","line":2471,"updated":"2024-02-27 22:19:05.000000000","message":"Which means we do get here.","commit_id":"26cdb2b6fc4f4c45c84ce0f59ac8417f987feefa"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"f5f15b055e774718e4aa704e32e29bc2d9099556","unresolved":false,"context_lines":[{"line_number":2468,"context_line":"                    self._local_delete_cleanup(context, instance_uuid)"},{"line_number":2469,"context_line":"                    return"},{"line_number":2470,"context_line":""},{"line_number":2471,"context_line":"        bdms \u003d objects.BlockDeviceMappingList.get_by_instance_uuid("},{"line_number":2472,"context_line":"                context, instance.uuid)"},{"line_number":2473,"context_line":""},{"line_number":2474,"context_line":"        # At these states an instance has a snapshot associate."}],"source_content_type":"text/x-python","patch_set":32,"id":"1ceed3fa_be68ec1d","line":2471,"in_reply_to":"58c775d4_a05306a9","updated":"2024-02-29 22:31:08.000000000","message":"Done","commit_id":"26cdb2b6fc4f4c45c84ce0f59ac8417f987feefa"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"8596e2fcd4cee9146f5dd317983332f74bb5ee19","unresolved":true,"context_lines":[{"line_number":2473,"context_line":""},{"line_number":2474,"context_line":"        # At these states an instance has a snapshot associate."},{"line_number":2475,"context_line":"        if instance.vm_state in (vm_states.SHELVED,"},{"line_number":2476,"context_line":"                                 vm_states.SHELVED_OFFLOADED):"},{"line_number":2477,"context_line":"            snapshot_id \u003d instance.system_metadata.get(\u0027shelved_image_id\u0027)"},{"line_number":2478,"context_line":"            LOG.info(\"Working on deleting snapshot %s \""},{"line_number":2479,"context_line":"                     \"from shelved instance...\","}],"source_content_type":"text/x-python","patch_set":32,"id":"620cf114_301b9bb0","line":2476,"updated":"2024-02-27 22:19:05.000000000","message":"We will pass here, but we only delete the image and fall out the bottom, unlike the case above.","commit_id":"26cdb2b6fc4f4c45c84ce0f59ac8417f987feefa"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"f5f15b055e774718e4aa704e32e29bc2d9099556","unresolved":false,"context_lines":[{"line_number":2473,"context_line":""},{"line_number":2474,"context_line":"        # At these states an instance has a snapshot associate."},{"line_number":2475,"context_line":"        if instance.vm_state in (vm_states.SHELVED,"},{"line_number":2476,"context_line":"                                 vm_states.SHELVED_OFFLOADED):"},{"line_number":2477,"context_line":"            snapshot_id \u003d instance.system_metadata.get(\u0027shelved_image_id\u0027)"},{"line_number":2478,"context_line":"            LOG.info(\"Working on deleting snapshot %s \""},{"line_number":2479,"context_line":"                     \"from shelved instance...\","}],"source_content_type":"text/x-python","patch_set":32,"id":"25ceb48e_f977c72e","line":2476,"in_reply_to":"620cf114_301b9bb0","updated":"2024-02-29 22:31:08.000000000","message":"Done","commit_id":"26cdb2b6fc4f4c45c84ce0f59ac8417f987feefa"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"8596e2fcd4cee9146f5dd317983332f74bb5ee19","unresolved":true,"context_lines":[{"line_number":2497,"context_line":"            instance.progress \u003d 0"},{"line_number":2498,"context_line":"            instance.save()"},{"line_number":2499,"context_line":""},{"line_number":2500,"context_line":"            if not instance.host and not may_have_ports_or_volumes:"},{"line_number":2501,"context_line":"                try:"},{"line_number":2502,"context_line":"                    with compute_utils.notify_about_instance_delete("},{"line_number":2503,"context_line":"                            self.notifier, context, instance,"}],"source_content_type":"text/x-python","patch_set":32,"id":"d72496a7_8bbb5c4d","line":2500,"updated":"2024-02-27 22:19:05.000000000","message":"We will pass the first case, but not the second so we won\u0027t run the body of this if.","commit_id":"26cdb2b6fc4f4c45c84ce0f59ac8417f987feefa"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"f5f15b055e774718e4aa704e32e29bc2d9099556","unresolved":false,"context_lines":[{"line_number":2497,"context_line":"            instance.progress \u003d 0"},{"line_number":2498,"context_line":"            instance.save()"},{"line_number":2499,"context_line":""},{"line_number":2500,"context_line":"            if not instance.host and not may_have_ports_or_volumes:"},{"line_number":2501,"context_line":"                try:"},{"line_number":2502,"context_line":"                    with compute_utils.notify_about_instance_delete("},{"line_number":2503,"context_line":"                            self.notifier, context, instance,"}],"source_content_type":"text/x-python","patch_set":32,"id":"27e173e6_84d1a1dc","line":2500,"in_reply_to":"d72496a7_8bbb5c4d","updated":"2024-02-29 22:31:08.000000000","message":"Done","commit_id":"26cdb2b6fc4f4c45c84ce0f59ac8417f987feefa"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"27e23ee299de03ce947906d928d124bdb39338cd","unresolved":true,"context_lines":[{"line_number":2510,"context_line":"                             {\u0027state\u0027: instance.vm_state},"},{"line_number":2511,"context_line":"                              instance\u003dinstance)"},{"line_number":2512,"context_line":"                    self._local_delete_cleanup("},{"line_number":2513,"context_line":"                        context, instance.uuid, bdms\u003dbdms)"},{"line_number":2514,"context_line":"                    return"},{"line_number":2515,"context_line":"                except exception.ObjectActionError as ex:"},{"line_number":2516,"context_line":"                    # The instance\u0027s host likely changed under us as"}],"source_content_type":"text/x-python","patch_set":32,"id":"549df9b7_ac520222","line":2513,"updated":"2024-02-27 19:56:07.000000000","message":"Why only here and not earlier? If we\u0027re shelved_offloaded we could have secrets for our shelved disk, no?","commit_id":"26cdb2b6fc4f4c45c84ce0f59ac8417f987feefa"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"8596e2fcd4cee9146f5dd317983332f74bb5ee19","unresolved":true,"context_lines":[{"line_number":2510,"context_line":"                             {\u0027state\u0027: instance.vm_state},"},{"line_number":2511,"context_line":"                              instance\u003dinstance)"},{"line_number":2512,"context_line":"                    self._local_delete_cleanup("},{"line_number":2513,"context_line":"                        context, instance.uuid, bdms\u003dbdms)"},{"line_number":2514,"context_line":"                    return"},{"line_number":2515,"context_line":"                except exception.ObjectActionError as ex:"},{"line_number":2516,"context_line":"                    # The instance\u0027s host likely changed under us as"}],"source_content_type":"text/x-python","patch_set":32,"id":"81eeba4a_15b31147","line":2513,"in_reply_to":"40f342a5_5c4a7966","updated":"2024-02-27 22:19:05.000000000","message":"Hmm, I\u0027m not sure because I think we skip too much if we\u0027re offloaded, and thus call a local delete without `bdms\u003d`. Let me annotate above. Perhaps I\u0027m missing something or not following the ridiculously high level of indentation here. I was thinking we might take the earlier branches for offloaded, but I think we actually do it after this.","commit_id":"26cdb2b6fc4f4c45c84ce0f59ac8417f987feefa"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"641ac51e1dba3c26e9054c9c736ddf0a4b63811f","unresolved":true,"context_lines":[{"line_number":2510,"context_line":"                             {\u0027state\u0027: instance.vm_state},"},{"line_number":2511,"context_line":"                              instance\u003dinstance)"},{"line_number":2512,"context_line":"                    self._local_delete_cleanup("},{"line_number":2513,"context_line":"                        context, instance.uuid, bdms\u003dbdms)"},{"line_number":2514,"context_line":"                    return"},{"line_number":2515,"context_line":"                except exception.ObjectActionError as ex:"},{"line_number":2516,"context_line":"                    # The instance\u0027s host likely changed under us as"}],"source_content_type":"text/x-python","patch_set":32,"id":"e56ff45c_4d493fd8","line":2513,"in_reply_to":"549df9b7_ac520222","updated":"2024-02-27 21:05:16.000000000","message":"There are two exceptions to the pattern of creating new secrets when new disks are created and they are shelve/unshelve and rebuild. One reason is to avoid a potential change in secret ownership if policy in a deployment, for example, allows an admin user in an admin project to shelve offload or rebuild instances of other users. If we deleted the Barbican secrets and recreated them in a scenario like that, the new secrets would be owned by the admin user and project and the original instance owner will no longer be able to use their instance. There is not a way in Barbican to do secret access \"on behalf of\" other users/projects.\n\nThe second reason is secret cleanup management. I have been worried about the potential of deleting a secret for an instance that is still \"alive\" so that goes along with tying the secret lifetime to the instance lifetime. To be able to be 100% sure when it\u0027s safe to delete them given the unexpected way that things could probably fail if there\u0027s a message queue error or a DB error or some other completely random failure that could happen to the instance at some point.\n\nThe libvirt secrets I\u0027m not as concerned about because worst case scenario, one could use the Barbican secrets as the source of truth to get the instance back on track.","commit_id":"26cdb2b6fc4f4c45c84ce0f59ac8417f987feefa"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"f5f15b055e774718e4aa704e32e29bc2d9099556","unresolved":false,"context_lines":[{"line_number":2510,"context_line":"                             {\u0027state\u0027: instance.vm_state},"},{"line_number":2511,"context_line":"                              instance\u003dinstance)"},{"line_number":2512,"context_line":"                    self._local_delete_cleanup("},{"line_number":2513,"context_line":"                        context, instance.uuid, bdms\u003dbdms)"},{"line_number":2514,"context_line":"                    return"},{"line_number":2515,"context_line":"                except exception.ObjectActionError as ex:"},{"line_number":2516,"context_line":"                    # The instance\u0027s host likely changed under us as"}],"source_content_type":"text/x-python","patch_set":32,"id":"cf0f37a7_19145126","line":2513,"in_reply_to":"81eeba4a_15b31147","updated":"2024-02-29 22:31:08.000000000","message":"Acknowledged","commit_id":"26cdb2b6fc4f4c45c84ce0f59ac8417f987feefa"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"f060ff835f652c4c9c7a07c238af147edaaa76a0","unresolved":true,"context_lines":[{"line_number":2510,"context_line":"                             {\u0027state\u0027: instance.vm_state},"},{"line_number":2511,"context_line":"                              instance\u003dinstance)"},{"line_number":2512,"context_line":"                    self._local_delete_cleanup("},{"line_number":2513,"context_line":"                        context, instance.uuid, bdms\u003dbdms)"},{"line_number":2514,"context_line":"                    return"},{"line_number":2515,"context_line":"                except exception.ObjectActionError as ex:"},{"line_number":2516,"context_line":"                    # The instance\u0027s host likely changed under us as"}],"source_content_type":"text/x-python","patch_set":32,"id":"40f342a5_5c4a7966","line":2513,"in_reply_to":"e235ea05_c6b0ff5f","updated":"2024-02-27 22:04:45.000000000","message":"Oh, sorry, I did miss your point. So in this case of a local delete, the Barbican secrets will be deleted here (obviously) and because libvirt secret cleanup is part of the driver destroy path, the libvirt secrets will get cleaned by the periodic reaper task that deletes instance materials if the instance is marked as deleted in the database.\n\nI will add a functional test to the shelve/unshelve related patch to make sure we cover that path. The test will be similar to the local delete test on this patch:\n\nhttps://review.opendev.org/c/openstack/nova/+/870932/32/nova/tests/functional/libvirt/test_ephemeral_encryption.py#106","commit_id":"26cdb2b6fc4f4c45c84ce0f59ac8417f987feefa"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"a87c49c69f58a3671949e2e892ef7b32d7fd826e","unresolved":true,"context_lines":[{"line_number":2510,"context_line":"                             {\u0027state\u0027: instance.vm_state},"},{"line_number":2511,"context_line":"                              instance\u003dinstance)"},{"line_number":2512,"context_line":"                    self._local_delete_cleanup("},{"line_number":2513,"context_line":"                        context, instance.uuid, bdms\u003dbdms)"},{"line_number":2514,"context_line":"                    return"},{"line_number":2515,"context_line":"                except exception.ObjectActionError as ex:"},{"line_number":2516,"context_line":"                    # The instance\u0027s host likely changed under us as"}],"source_content_type":"text/x-python","patch_set":32,"id":"e235ea05_c6b0ff5f","line":2513,"in_reply_to":"e56ff45c_4d493fd8","updated":"2024-02-27 21:51:02.000000000","message":"Okay, I\u0027m not sure I properly articulated my point. Let\u0027s say I have an instance that was running. I shelve, offload it and it\u0027s hanging out in image-form-only. Then I go and delete it from shelved state. It has no host, so we\u0027d delete the instance but never clean up the secrets, no?","commit_id":"26cdb2b6fc4f4c45c84ce0f59ac8417f987feefa"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"8596e2fcd4cee9146f5dd317983332f74bb5ee19","unresolved":true,"context_lines":[{"line_number":2511,"context_line":"                              instance\u003dinstance)"},{"line_number":2512,"context_line":"                    self._local_delete_cleanup("},{"line_number":2513,"context_line":"                        context, instance.uuid, bdms\u003dbdms)"},{"line_number":2514,"context_line":"                    return"},{"line_number":2515,"context_line":"                except exception.ObjectActionError as ex:"},{"line_number":2516,"context_line":"                    # The instance\u0027s host likely changed under us as"},{"line_number":2517,"context_line":"                    # this instance could be building and has since been"}],"source_content_type":"text/x-python","patch_set":32,"id":"099ceef3_fb262e95","line":2514,"updated":"2024-02-27 22:19:05.000000000","message":"We were never here, so we don\u0027t bail on this return, which means we fall though to the rest.","commit_id":"26cdb2b6fc4f4c45c84ce0f59ac8417f987feefa"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"f5f15b055e774718e4aa704e32e29bc2d9099556","unresolved":false,"context_lines":[{"line_number":2511,"context_line":"                              instance\u003dinstance)"},{"line_number":2512,"context_line":"                    self._local_delete_cleanup("},{"line_number":2513,"context_line":"                        context, instance.uuid, bdms\u003dbdms)"},{"line_number":2514,"context_line":"                    return"},{"line_number":2515,"context_line":"                except exception.ObjectActionError as ex:"},{"line_number":2516,"context_line":"                    # The instance\u0027s host likely changed under us as"},{"line_number":2517,"context_line":"                    # this instance could be building and has since been"}],"source_content_type":"text/x-python","patch_set":32,"id":"e83717ec_54931587","line":2514,"in_reply_to":"099ceef3_fb262e95","updated":"2024-02-29 22:31:08.000000000","message":"Done","commit_id":"26cdb2b6fc4f4c45c84ce0f59ac8417f987feefa"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"8596e2fcd4cee9146f5dd317983332f74bb5ee19","unresolved":true,"context_lines":[{"line_number":2578,"context_line":"                LOG.debug(\u0027Doing local delete in cell %s\u0027, cell.identity,"},{"line_number":2579,"context_line":"                          instance\u003dinstance)"},{"line_number":2580,"context_line":"                with nova_context.target_cell(context, cell) as cctxt:"},{"line_number":2581,"context_line":"                    self._local_delete(cctxt, instance, bdms, delete_type, cb)"},{"line_number":2582,"context_line":"                    self._record_action_start(context, instance,"},{"line_number":2583,"context_line":"                                              instance_actions.DELETE)"},{"line_number":2584,"context_line":""}],"source_content_type":"text/x-python","patch_set":32,"id":"57f243d8_96e551d1","line":2581,"updated":"2024-02-27 22:19:05.000000000","message":"I think *here* is where we do our local delete. We pass our bdms here, but I don\u0027t think this ever calls `_local_delete_cleanup()` to do the BDM/secret stuff.","commit_id":"26cdb2b6fc4f4c45c84ce0f59ac8417f987feefa"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"7dc7fedbdd44d9db1f008e55a1a3fda3c2a24add","unresolved":true,"context_lines":[{"line_number":2578,"context_line":"                LOG.debug(\u0027Doing local delete in cell %s\u0027, cell.identity,"},{"line_number":2579,"context_line":"                          instance\u003dinstance)"},{"line_number":2580,"context_line":"                with nova_context.target_cell(context, cell) as cctxt:"},{"line_number":2581,"context_line":"                    self._local_delete(cctxt, instance, bdms, delete_type, cb)"},{"line_number":2582,"context_line":"                    self._record_action_start(context, instance,"},{"line_number":2583,"context_line":"                                              instance_actions.DELETE)"},{"line_number":2584,"context_line":""}],"source_content_type":"text/x-python","patch_set":32,"id":"946989a6_acb8a2e2","line":2581,"in_reply_to":"57f243d8_96e551d1","updated":"2024-02-27 22:28:50.000000000","message":"That is correct but unfortunately there are two local delete paths: one for the \"delete while booting\" scenario which is `_local_delete_cleanup()` and the other more common one `_local_delete()` here. So I had to add secret cleanup calls to both methods, L2387 and L2696.\n\nI spent a short amount of time trying to find how to refactor to combine into only one method that we call in both places but it would take more time and careful thought so as not to break anything, at least IMHO.","commit_id":"26cdb2b6fc4f4c45c84ce0f59ac8417f987feefa"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"f5f15b055e774718e4aa704e32e29bc2d9099556","unresolved":false,"context_lines":[{"line_number":2578,"context_line":"                LOG.debug(\u0027Doing local delete in cell %s\u0027, cell.identity,"},{"line_number":2579,"context_line":"                          instance\u003dinstance)"},{"line_number":2580,"context_line":"                with nova_context.target_cell(context, cell) as cctxt:"},{"line_number":2581,"context_line":"                    self._local_delete(cctxt, instance, bdms, delete_type, cb)"},{"line_number":2582,"context_line":"                    self._record_action_start(context, instance,"},{"line_number":2583,"context_line":"                                              instance_actions.DELETE)"},{"line_number":2584,"context_line":""}],"source_content_type":"text/x-python","patch_set":32,"id":"c023f8c2_c38d85ff","line":2581,"in_reply_to":"946989a6_acb8a2e2","updated":"2024-02-29 22:31:08.000000000","message":"Done","commit_id":"26cdb2b6fc4f4c45c84ce0f59ac8417f987feefa"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"7dc7fedbdd44d9db1f008e55a1a3fda3c2a24add","unresolved":true,"context_lines":[{"line_number":2693,"context_line":"                context, instance.uuid, force\u003dTrue)"},{"line_number":2694,"context_line":""},{"line_number":2695,"context_line":"            # Clean up ephemeral encryption secrets if needed."},{"line_number":2696,"context_line":"            compute_utils.delete_ephemeral_encryption_secrets("},{"line_number":2697,"context_line":"                context, instance.uuid, bdms)"},{"line_number":2698,"context_line":""},{"line_number":2699,"context_line":"            cb(context, instance, bdms, local\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":32,"id":"2694a069_15c17e21","line":2696,"updated":"2024-02-27 22:28:50.000000000","message":"This is where we delete secrets in `_local_delete()`.","commit_id":"26cdb2b6fc4f4c45c84ce0f59ac8417f987feefa"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"03bb6dde62c1e6bafec43a41cd71c22ecc88e007","unresolved":true,"context_lines":[{"line_number":2693,"context_line":"                context, instance.uuid, force\u003dTrue)"},{"line_number":2694,"context_line":""},{"line_number":2695,"context_line":"            # Clean up ephemeral encryption secrets if needed."},{"line_number":2696,"context_line":"            compute_utils.delete_ephemeral_encryption_secrets("},{"line_number":2697,"context_line":"                context, instance.uuid, bdms)"},{"line_number":2698,"context_line":""},{"line_number":2699,"context_line":"            cb(context, instance, bdms, local\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":32,"id":"62613070_97064d62","line":2696,"in_reply_to":"2694a069_15c17e21","updated":"2024-02-27 22:39:25.000000000","message":"Oh heh, I have this downloaded locally to browse, but I was sure I didn\u0027t see it, but I was looking for a call into the other helper and saw that `_local_cleanup_bdm_volumes()` doesn\u0027t do it.\n\nEesh, this delete stuff.","commit_id":"26cdb2b6fc4f4c45c84ce0f59ac8417f987feefa"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"a00dc0e4e13a2245be1d871f13403dcaddb8014c","unresolved":true,"context_lines":[{"line_number":2693,"context_line":"                context, instance.uuid, force\u003dTrue)"},{"line_number":2694,"context_line":""},{"line_number":2695,"context_line":"            # Clean up ephemeral encryption secrets if needed."},{"line_number":2696,"context_line":"            compute_utils.delete_ephemeral_encryption_secrets("},{"line_number":2697,"context_line":"                context, instance.uuid, bdms)"},{"line_number":2698,"context_line":""},{"line_number":2699,"context_line":"            cb(context, instance, bdms, local\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":32,"id":"fcb8e3c2_21af1d48","line":2696,"in_reply_to":"62613070_97064d62","updated":"2024-02-27 22:41:35.000000000","message":"Yeah, it really is the worst.","commit_id":"26cdb2b6fc4f4c45c84ce0f59ac8417f987feefa"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"00435d585ff7cd47a3515561cd97e334e16072d9","unresolved":false,"context_lines":[{"line_number":2693,"context_line":"                context, instance.uuid, force\u003dTrue)"},{"line_number":2694,"context_line":""},{"line_number":2695,"context_line":"            # Clean up ephemeral encryption secrets if needed."},{"line_number":2696,"context_line":"            compute_utils.delete_ephemeral_encryption_secrets("},{"line_number":2697,"context_line":"                context, instance.uuid, bdms)"},{"line_number":2698,"context_line":""},{"line_number":2699,"context_line":"            cb(context, instance, bdms, local\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":32,"id":"6cc8504a_a5790333","line":2696,"in_reply_to":"fcb8e3c2_21af1d48","updated":"2024-02-29 07:37:13.000000000","message":"Acknowledged","commit_id":"26cdb2b6fc4f4c45c84ce0f59ac8417f987feefa"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"3e64382fc5b461121276bd6730e34332c32c4897","unresolved":true,"context_lines":[{"line_number":5888,"context_line":"        bdm \u003d self._get_bdm_by_volume_id("},{"line_number":5889,"context_line":"            context, volume_id, expected_attrs\u003d[\u0027instance\u0027])"},{"line_number":5890,"context_line":""},{"line_number":5891,"context_line":"        @reject_ephemeral_encryption_instances(instance_actions.CREATE_IMAGE)"},{"line_number":5892,"context_line":"        # We allow creating the snapshot in any vm_state as long as there is"},{"line_number":5893,"context_line":"        # no task being performed on the instance and it has a host."},{"line_number":5894,"context_line":"        @check_instance_host()"}],"source_content_type":"text/x-python","patch_set":38,"id":"59611d9a_d5ad1e8b","line":5891,"updated":"2024-05-21 09:44:16.000000000","message":"is this correct? isnt this the proxy api for doing a cinder snapshot?","commit_id":"77bf446d789f2bf4ce4adf13616ffc53aa217cd1"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"1f502c735ca8b401dd20e76512a71a6220310a7a","unresolved":true,"context_lines":[{"line_number":5888,"context_line":"        bdm \u003d self._get_bdm_by_volume_id("},{"line_number":5889,"context_line":"            context, volume_id, expected_attrs\u003d[\u0027instance\u0027])"},{"line_number":5890,"context_line":""},{"line_number":5891,"context_line":"        @reject_ephemeral_encryption_instances(instance_actions.CREATE_IMAGE)"},{"line_number":5892,"context_line":"        # We allow creating the snapshot in any vm_state as long as there is"},{"line_number":5893,"context_line":"        # no task being performed on the instance and it has a host."},{"line_number":5894,"context_line":"        @check_instance_host()"}],"source_content_type":"text/x-python","patch_set":38,"id":"2d07398e_b88b61e3","line":5891,"in_reply_to":"59611d9a_d5ad1e8b","updated":"2024-05-21 23:54:11.000000000","message":"Hm, I think this must be a mistake. At best, maybe I had thought that local disks are also included in the snapshot call to libvirt but I just checked the code and they are explicitly excluded as disks_to_skip. So I need to remove this. Thanks for catching it.","commit_id":"77bf446d789f2bf4ce4adf13616ffc53aa217cd1"}],"nova/compute/utils.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"fbf5c6aeb4ce823cf2512900fc32cecce3cf3a28","unresolved":true,"context_lines":[{"line_number":1625,"context_line":"                    LOG.exception("},{"line_number":1626,"context_line":"                        f\u0027Failed to delete encryption secret {secret_uuid} \u0027"},{"line_number":1627,"context_line":"                        \u0027from the key manager.\u0027, instance_uuid\u003dinstance_uuid)"},{"line_number":1628,"context_line":"                    pass"}],"source_content_type":"text/x-python","patch_set":33,"id":"47a8c85a_45296282","line":1628,"updated":"2024-02-28 19:49:21.000000000","message":"You don\u0027t need this pass :)","commit_id":"31a028d86a90ce86b3cbb5950932e08a812cc465"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"62a137d49ce924dbfcd597855024c59fb7836266","unresolved":true,"context_lines":[{"line_number":1625,"context_line":"                    LOG.exception("},{"line_number":1626,"context_line":"                        f\u0027Failed to delete encryption secret {secret_uuid} \u0027"},{"line_number":1627,"context_line":"                        \u0027from the key manager.\u0027, instance_uuid\u003dinstance_uuid)"},{"line_number":1628,"context_line":"                    pass"}],"source_content_type":"text/x-python","patch_set":33,"id":"4bbb539f_eeb938d5","line":1628,"in_reply_to":"47a8c85a_45296282","updated":"2024-02-28 21:26:44.000000000","message":"I noticed that a couple of revisions ago but forgot to remove it, thanks.","commit_id":"31a028d86a90ce86b3cbb5950932e08a812cc465"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"00435d585ff7cd47a3515561cd97e334e16072d9","unresolved":false,"context_lines":[{"line_number":1625,"context_line":"                    LOG.exception("},{"line_number":1626,"context_line":"                        f\u0027Failed to delete encryption secret {secret_uuid} \u0027"},{"line_number":1627,"context_line":"                        \u0027from the key manager.\u0027, instance_uuid\u003dinstance_uuid)"},{"line_number":1628,"context_line":"                    pass"}],"source_content_type":"text/x-python","patch_set":33,"id":"4674dbd9_2ccab422","line":1628,"in_reply_to":"4bbb539f_eeb938d5","updated":"2024-02-29 07:37:13.000000000","message":"Done","commit_id":"31a028d86a90ce86b3cbb5950932e08a812cc465"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"f5f15b055e774718e4aa704e32e29bc2d9099556","unresolved":true,"context_lines":[{"line_number":1606,"context_line":"        cyclient.delete_arqs_by_uuid(arq_uuids)"},{"line_number":1607,"context_line":""},{"line_number":1608,"context_line":""},{"line_number":1609,"context_line":"def delete_ephemeral_encryption_secrets(context, instance_uuid, bdms):"},{"line_number":1610,"context_line":"    # TODO(melwitt): This will also include the backing file secret UUID when"},{"line_number":1611,"context_line":"    # support for encrypted backing files is added."},{"line_number":1612,"context_line":"    keys \u003d [\u0027encryption_secret_uuid\u0027]"}],"source_content_type":"text/x-python","patch_set":35,"id":"989ca247_9c0d50ba","line":1609,"updated":"2024-02-29 22:31:08.000000000","message":"So, I was going to ask if we should rename this, specifically to add \"bdms\" to the name (since that\u0027s really what it\u0027s doing) but also to drop \"ephemeral\" from the name. It\u0027s not really making a distinction between ephemeral and volume here, other than that we don\u0027t have this secret uuid key on those (right?).\n\nBut that made me wonder - _should_ we put a check here for just the non-volume disks to make sure that in a later world where we do have barbican-resident secrets for volumes that we don\u0027t nuke them with the instance lifecycle like we expect for the ephemeral disks?","commit_id":"d59666b9cd2b90a5bf3091f2cc042e8b9e40dc6e"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"e6676f5c11effeffc12f304042ede07c175b53ea","unresolved":true,"context_lines":[{"line_number":1606,"context_line":"        cyclient.delete_arqs_by_uuid(arq_uuids)"},{"line_number":1607,"context_line":""},{"line_number":1608,"context_line":""},{"line_number":1609,"context_line":"def delete_ephemeral_encryption_secrets(context, instance_uuid, bdms):"},{"line_number":1610,"context_line":"    # TODO(melwitt): This will also include the backing file secret UUID when"},{"line_number":1611,"context_line":"    # support for encrypted backing files is added."},{"line_number":1612,"context_line":"    keys \u003d [\u0027encryption_secret_uuid\u0027]"}],"source_content_type":"text/x-python","patch_set":35,"id":"fea235d1_c0ef82f7","line":1609,"in_reply_to":"845d9045_1b2cc221","updated":"2024-02-29 23:24:19.000000000","message":"Yeah, I know it doesn\u0027t now (for volumes). Just thinking that this stuff assuming any BDM with an encryption_secret_uuid is safe to nuke is probably not what we\u0027ll always want. Given the bad day someone will have if this nukes everything, it\u0027s probably worth thinking about future melwitt and dansmith :)","commit_id":"d59666b9cd2b90a5bf3091f2cc042e8b9e40dc6e"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"1d8cca11c484954a058252c77f6f16f74339a62f","unresolved":true,"context_lines":[{"line_number":1606,"context_line":"        cyclient.delete_arqs_by_uuid(arq_uuids)"},{"line_number":1607,"context_line":""},{"line_number":1608,"context_line":""},{"line_number":1609,"context_line":"def delete_ephemeral_encryption_secrets(context, instance_uuid, bdms):"},{"line_number":1610,"context_line":"    # TODO(melwitt): This will also include the backing file secret UUID when"},{"line_number":1611,"context_line":"    # support for encrypted backing files is added."},{"line_number":1612,"context_line":"    keys \u003d [\u0027encryption_secret_uuid\u0027]"}],"source_content_type":"text/x-python","patch_set":35,"id":"845d9045_1b2cc221","line":1609,"in_reply_to":"989ca247_9c0d50ba","updated":"2024-02-29 23:06:28.000000000","message":"Hm, yeah, I think it would be worthwhile to check and ignore volume BDMs just in case for the future. Volume encryption secret IDs are not stored on the BDM at this time.\n\nAnd yeah renaming could make sense. In some cases I put the word \"ephemeral\" in the function name to try to help with readability.","commit_id":"d59666b9cd2b90a5bf3091f2cc042e8b9e40dc6e"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"7d17c244442d03e56bf24df724de492026acf989","unresolved":false,"context_lines":[{"line_number":1606,"context_line":"        cyclient.delete_arqs_by_uuid(arq_uuids)"},{"line_number":1607,"context_line":""},{"line_number":1608,"context_line":""},{"line_number":1609,"context_line":"def delete_ephemeral_encryption_secrets(context, instance_uuid, bdms):"},{"line_number":1610,"context_line":"    # TODO(melwitt): This will also include the backing file secret UUID when"},{"line_number":1611,"context_line":"    # support for encrypted backing files is added."},{"line_number":1612,"context_line":"    keys \u003d [\u0027encryption_secret_uuid\u0027]"}],"source_content_type":"text/x-python","patch_set":35,"id":"f0cc5d78_151bf991","line":1609,"in_reply_to":"fea235d1_c0ef82f7","updated":"2024-03-02 01:26:40.000000000","message":"Done","commit_id":"d59666b9cd2b90a5bf3091f2cc042e8b9e40dc6e"}],"nova/tests/fixtures/libvirt_imagebackend.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"f5f15b055e774718e4aa704e32e29bc2d9099556","unresolved":true,"context_lines":[{"line_number":195,"context_line":"            else:"},{"line_number":196,"context_line":"                disk.get_encryption.side_effect \u003d functools.partial("},{"line_number":197,"context_line":"                    imagebackend.Image.get_encryption, disk)"},{"line_number":198,"context_line":""},{"line_number":199,"context_line":"            return disk"},{"line_number":200,"context_line":""},{"line_number":201,"context_line":"        # Set the SUPPORTS_CLONE member variable to mimic the Image base"}],"source_content_type":"text/x-python","patch_set":35,"id":"03bc5688_8821946f","line":198,"updated":"2024-02-29 22:31:08.000000000","message":"This is maybe a bit silly, but.. won\u0027t `get_encryption()` return None if .. there is none? I\u0027m just wondering if we need this conditional or if we can do the partial always. Like, the more optionality we have in fixtures like this, the more likely it is that some test passes because the fixture does what the test is expecting instead of what the real thing does, so if we can avoid it, it seems like a better idea.","commit_id":"d59666b9cd2b90a5bf3091f2cc042e8b9e40dc6e"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"1d8cca11c484954a058252c77f6f16f74339a62f","unresolved":true,"context_lines":[{"line_number":195,"context_line":"            else:"},{"line_number":196,"context_line":"                disk.get_encryption.side_effect \u003d functools.partial("},{"line_number":197,"context_line":"                    imagebackend.Image.get_encryption, disk)"},{"line_number":198,"context_line":""},{"line_number":199,"context_line":"            return disk"},{"line_number":200,"context_line":""},{"line_number":201,"context_line":"        # Set the SUPPORTS_CLONE member variable to mimic the Image base"}],"source_content_type":"text/x-python","patch_set":35,"id":"3651a4f1_e6fc614f","line":198,"in_reply_to":"03bc5688_8821946f","updated":"2024-02-29 23:06:28.000000000","message":"Hm, yeah I think so. It should, not sure why I did this.","commit_id":"d59666b9cd2b90a5bf3091f2cc042e8b9e40dc6e"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"7d17c244442d03e56bf24df724de492026acf989","unresolved":false,"context_lines":[{"line_number":195,"context_line":"            else:"},{"line_number":196,"context_line":"                disk.get_encryption.side_effect \u003d functools.partial("},{"line_number":197,"context_line":"                    imagebackend.Image.get_encryption, disk)"},{"line_number":198,"context_line":""},{"line_number":199,"context_line":"            return disk"},{"line_number":200,"context_line":""},{"line_number":201,"context_line":"        # Set the SUPPORTS_CLONE member variable to mimic the Image base"}],"source_content_type":"text/x-python","patch_set":35,"id":"5f731194_ac136351","line":198,"in_reply_to":"3651a4f1_e6fc614f","updated":"2024-03-02 01:26:40.000000000","message":"Done","commit_id":"d59666b9cd2b90a5bf3091f2cc042e8b9e40dc6e"}],"nova/tests/functional/libvirt/test_ephemeral_encryption.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"2fa214491af72660ad5be4769acf13bc1bb8a7ce","unresolved":true,"context_lines":[{"line_number":26,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":27,"context_line":""},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"class FakeKeyManager(key_manager.KeyManager):"},{"line_number":30,"context_line":"    \"\"\"A fake key manager."},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"    This key manager implementation supports a minimum subset of methods"}],"source_content_type":"text/x-python","patch_set":24,"id":"d2fe954d_caf0e9d0","line":29,"updated":"2024-01-31 09:22:59.000000000","message":"castellan has its own fake keymanger\nalso this should proably be in the fixtured subdir so it can be shared.\n\nthis is a tivial implementation so im not agaisnt adding it but im wondering why you chosse not to use \n\nhttps://github.com/openstack/castellan/blob/master/castellan/tests/unit/key_manager/mock_key_manager.py#L45\n\nand create your own.","commit_id":"e40a49f0c6859c9561d68105cc706b6c798a7b80"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"a41597b7a6be0fdd5f8739e5458fa5ed60cb11ac","unresolved":false,"context_lines":[{"line_number":26,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":27,"context_line":""},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"class FakeKeyManager(key_manager.KeyManager):"},{"line_number":30,"context_line":"    \"\"\"A fake key manager."},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"    This key manager implementation supports a minimum subset of methods"}],"source_content_type":"text/x-python","patch_set":24,"id":"d3f07659_0de86fdb","line":29,"in_reply_to":"5f388f4f_e4ec1e88","updated":"2024-02-13 06:24:58.000000000","message":"Done","commit_id":"e40a49f0c6859c9561d68105cc706b6c798a7b80"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"912650a3790419afd772616f40ced033530d21f6","unresolved":true,"context_lines":[{"line_number":26,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":27,"context_line":""},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"class FakeKeyManager(key_manager.KeyManager):"},{"line_number":30,"context_line":"    \"\"\"A fake key manager."},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"    This key manager implementation supports a minimum subset of methods"}],"source_content_type":"text/x-python","patch_set":24,"id":"5f388f4f_e4ec1e88","line":29,"in_reply_to":"d2fe954d_caf0e9d0","updated":"2024-02-01 03:29:25.000000000","message":"Honestly I just ripped this off from the vTPM functional tests. Was just trying to get something to work but yeah it would be better to use the castellan one. I\u0027ll try to swap it out.","commit_id":"e40a49f0c6859c9561d68105cc706b6c798a7b80"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"2fa214491af72660ad5be4769acf13bc1bb8a7ce","unresolved":true,"context_lines":[{"line_number":30,"context_line":"    \"\"\"A fake key manager."},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"    This key manager implementation supports a minimum subset of methods"},{"line_number":33,"context_line":"    specified by the key manager interface that are required for vTPM. Side"},{"line_number":34,"context_line":"    effects (e.g., raising exceptions) for each method are handled as specified"},{"line_number":35,"context_line":"    by the key manager interface."},{"line_number":36,"context_line":"    \"\"\""}],"source_content_type":"text/x-python","patch_set":24,"id":"984c320c_20e7784c","line":33,"updated":"2024-01-31 09:22:59.000000000","message":"is it because of the vTPM comment or is this just proted from that test code?","commit_id":"e40a49f0c6859c9561d68105cc706b6c798a7b80"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"a41597b7a6be0fdd5f8739e5458fa5ed60cb11ac","unresolved":false,"context_lines":[{"line_number":30,"context_line":"    \"\"\"A fake key manager."},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"    This key manager implementation supports a minimum subset of methods"},{"line_number":33,"context_line":"    specified by the key manager interface that are required for vTPM. Side"},{"line_number":34,"context_line":"    effects (e.g., raising exceptions) for each method are handled as specified"},{"line_number":35,"context_line":"    by the key manager interface."},{"line_number":36,"context_line":"    \"\"\""}],"source_content_type":"text/x-python","patch_set":24,"id":"1c4e7fff_b962c24f","line":33,"in_reply_to":"6d51266f_cb82c35e","updated":"2024-02-13 06:24:58.000000000","message":"Acknowledged","commit_id":"e40a49f0c6859c9561d68105cc706b6c798a7b80"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"912650a3790419afd772616f40ced033530d21f6","unresolved":true,"context_lines":[{"line_number":30,"context_line":"    \"\"\"A fake key manager."},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"    This key manager implementation supports a minimum subset of methods"},{"line_number":33,"context_line":"    specified by the key manager interface that are required for vTPM. Side"},{"line_number":34,"context_line":"    effects (e.g., raising exceptions) for each method are handled as specified"},{"line_number":35,"context_line":"    by the key manager interface."},{"line_number":36,"context_line":"    \"\"\""}],"source_content_type":"text/x-python","patch_set":24,"id":"6d51266f_cb82c35e","line":33,"in_reply_to":"984c320c_20e7784c","updated":"2024-02-01 03:29:25.000000000","message":"It\u0027s a copy pasta yeah.","commit_id":"e40a49f0c6859c9561d68105cc706b6c798a7b80"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"4ad3f7a190356f6fe5986232d6523eb5d42a2a2d","unresolved":true,"context_lines":[{"line_number":129,"context_line":"        compute \u003d self.start_compute()"},{"line_number":130,"context_line":"        driver \u003d self.computes[compute].driver"},{"line_number":131,"context_line":"        driver.capabilities[\u0027supports_ephemeral_encryption\u0027] \u003d True"},{"line_number":132,"context_line":"        driver.capabilities[\u0027supports_ephemeral_encryption_luks\u0027] \u003d True"},{"line_number":133,"context_line":"        self._run_periodics()"},{"line_number":134,"context_line":""},{"line_number":135,"context_line":"        # ensure we are reporting the correct traits"}],"source_content_type":"text/x-python","patch_set":24,"id":"e4b5c831_c8121f82","line":132,"updated":"2024-01-31 11:29:03.000000000","message":"right now this is requried because supprots luks is disabeld on the images backend right.\n\nin the future this will be reported by the driver so we wont need to force it.","commit_id":"e40a49f0c6859c9561d68105cc706b6c798a7b80"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"a41597b7a6be0fdd5f8739e5458fa5ed60cb11ac","unresolved":false,"context_lines":[{"line_number":129,"context_line":"        compute \u003d self.start_compute()"},{"line_number":130,"context_line":"        driver \u003d self.computes[compute].driver"},{"line_number":131,"context_line":"        driver.capabilities[\u0027supports_ephemeral_encryption\u0027] \u003d True"},{"line_number":132,"context_line":"        driver.capabilities[\u0027supports_ephemeral_encryption_luks\u0027] \u003d True"},{"line_number":133,"context_line":"        self._run_periodics()"},{"line_number":134,"context_line":""},{"line_number":135,"context_line":"        # ensure we are reporting the correct traits"}],"source_content_type":"text/x-python","patch_set":24,"id":"93210a57_5d89eb7d","line":132,"in_reply_to":"03a5c3e7_2aa57c12","updated":"2024-02-13 06:24:58.000000000","message":"Acknowledged","commit_id":"e40a49f0c6859c9561d68105cc706b6c798a7b80"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"4b8c24e8dafe49d964af36e3eb1a62edde0bb47e","unresolved":true,"context_lines":[{"line_number":129,"context_line":"        compute \u003d self.start_compute()"},{"line_number":130,"context_line":"        driver \u003d self.computes[compute].driver"},{"line_number":131,"context_line":"        driver.capabilities[\u0027supports_ephemeral_encryption\u0027] \u003d True"},{"line_number":132,"context_line":"        driver.capabilities[\u0027supports_ephemeral_encryption_luks\u0027] \u003d True"},{"line_number":133,"context_line":"        self._run_periodics()"},{"line_number":134,"context_line":""},{"line_number":135,"context_line":"        # ensure we are reporting the correct traits"}],"source_content_type":"text/x-python","patch_set":24,"id":"03a5c3e7_2aa57c12","line":132,"in_reply_to":"9ebc093c_6cce8b03","updated":"2024-02-01 02:54:23.000000000","message":"OK, nevermind, it doesn\u0027t work the way I expected because of the LibvirtImageBackendFixture, it mocks imagebackend.Backend.backend:\n\nhttps://github.com/openstack/nova/blob/da918d4b958d1f1c828e84d2a5c87c2dca676c39/nova/tests/fixtures/libvirt_imagebackend.py#L195","commit_id":"e40a49f0c6859c9561d68105cc706b6c798a7b80"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"7e9184b4ca5c8cfe2f2bc9ab97a0fe529a50025d","unresolved":true,"context_lines":[{"line_number":129,"context_line":"        compute \u003d self.start_compute()"},{"line_number":130,"context_line":"        driver \u003d self.computes[compute].driver"},{"line_number":131,"context_line":"        driver.capabilities[\u0027supports_ephemeral_encryption\u0027] \u003d True"},{"line_number":132,"context_line":"        driver.capabilities[\u0027supports_ephemeral_encryption_luks\u0027] \u003d True"},{"line_number":133,"context_line":"        self._run_periodics()"},{"line_number":134,"context_line":""},{"line_number":135,"context_line":"        # ensure we are reporting the correct traits"}],"source_content_type":"text/x-python","patch_set":24,"id":"9ebc093c_6cce8b03","line":132,"in_reply_to":"e4b5c831_c8121f82","updated":"2024-02-01 02:14:49.000000000","message":"Yes that\u0027s right.","commit_id":"e40a49f0c6859c9561d68105cc706b6c798a7b80"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"4ad3f7a190356f6fe5986232d6523eb5d42a2a2d","unresolved":true,"context_lines":[{"line_number":170,"context_line":"            self.assertIsNone(s)"},{"line_number":171,"context_line":""},{"line_number":172,"context_line":"        # ensure we deleted the key now that we no longer need it"},{"line_number":173,"context_line":"        self.assertEqual(0, len(self.key_mgr._passphrases))"}],"source_content_type":"text/x-python","patch_set":24,"id":"46f20641_459c1a00","line":173,"updated":"2024-01-31 11:29:03.000000000","message":"i mention this later in the serise but all lifecycle operations should have functional tests here too.\n\nthe later patches are mostly only tested with unit tests and given the complexity of the feature that likely not sufficnet. with that said you have added a good level of unit test coverage in the following pathces but i really think we need functioanl tests as well.","commit_id":"e40a49f0c6859c9561d68105cc706b6c798a7b80"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"7e9184b4ca5c8cfe2f2bc9ab97a0fe529a50025d","unresolved":true,"context_lines":[{"line_number":170,"context_line":"            self.assertIsNone(s)"},{"line_number":171,"context_line":""},{"line_number":172,"context_line":"        # ensure we deleted the key now that we no longer need it"},{"line_number":173,"context_line":"        self.assertEqual(0, len(self.key_mgr._passphrases))"}],"source_content_type":"text/x-python","patch_set":24,"id":"a2e79e70_84670001","line":173,"in_reply_to":"46f20641_459c1a00","updated":"2024-02-01 02:14:49.000000000","message":"I agree and am working on func tests for each separate action patch.","commit_id":"e40a49f0c6859c9561d68105cc706b6c798a7b80"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"a41597b7a6be0fdd5f8739e5458fa5ed60cb11ac","unresolved":false,"context_lines":[{"line_number":170,"context_line":"            self.assertIsNone(s)"},{"line_number":171,"context_line":""},{"line_number":172,"context_line":"        # ensure we deleted the key now that we no longer need it"},{"line_number":173,"context_line":"        self.assertEqual(0, len(self.key_mgr._passphrases))"}],"source_content_type":"text/x-python","patch_set":24,"id":"8105e59c_21cc3ce7","line":173,"in_reply_to":"a2e79e70_84670001","updated":"2024-02-13 06:24:58.000000000","message":"Acknowledged","commit_id":"e40a49f0c6859c9561d68105cc706b6c798a7b80"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"a41597b7a6be0fdd5f8739e5458fa5ed60cb11ac","unresolved":true,"context_lines":[{"line_number":48,"context_line":"            self.assertIn(bdm.encryption_secret_uuid, keymgr_secrets.keys())"},{"line_number":49,"context_line":"            usage_id \u003d f\u0027{bdm.instance_uuid}_{bdm.uuid}\u0027"},{"line_number":50,"context_line":"            s \u003d driver._host.find_secret(\u0027volume\u0027, usage_id)"},{"line_number":51,"context_line":"            self.assertIn(s.value(), keymgr_secrets.values())"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":""},{"line_number":54,"context_line":"class EphemeralEncryptionTestCreate(EphemeralEncryptionTestBase):"}],"source_content_type":"text/x-python","patch_set":28,"id":"37557e23_ca8d32f2","line":51,"updated":"2024-02-13 06:24:58.000000000","message":"you could also do \nself.assertEqual(s.value(), keymgr_secrets[bdm.encryption_secret_uuid])","commit_id":"a0bb38491f7b8db23e99d00ae30ff6f4ae703749"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"e62b5419199f6b10c959f9653a0945466a397a59","unresolved":false,"context_lines":[{"line_number":48,"context_line":"            self.assertIn(bdm.encryption_secret_uuid, keymgr_secrets.keys())"},{"line_number":49,"context_line":"            usage_id \u003d f\u0027{bdm.instance_uuid}_{bdm.uuid}\u0027"},{"line_number":50,"context_line":"            s \u003d driver._host.find_secret(\u0027volume\u0027, usage_id)"},{"line_number":51,"context_line":"            self.assertIn(s.value(), keymgr_secrets.values())"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":""},{"line_number":54,"context_line":"class EphemeralEncryptionTestCreate(EphemeralEncryptionTestBase):"}],"source_content_type":"text/x-python","patch_set":28,"id":"4234faf7_d8558d4f","line":51,"in_reply_to":"37557e23_ca8d32f2","updated":"2024-02-14 08:05:56.000000000","message":"Done","commit_id":"a0bb38491f7b8db23e99d00ae30ff6f4ae703749"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"351369d79663b920ff48ba1b29ebb6b12b0567ce","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":28,"id":"7d4e1819_83a04941","line":99,"updated":"2024-02-13 06:26:44.000000000","message":"as noted in the release note you could add a few other actions here like\nstop/start/reboot\n\nthey should function at this point.\n\nthat can be done in a follow up patch","commit_id":"a0bb38491f7b8db23e99d00ae30ff6f4ae703749"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"e62b5419199f6b10c959f9653a0945466a397a59","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":28,"id":"a9193206_a0570e06","line":99,"in_reply_to":"7d4e1819_83a04941","updated":"2024-02-14 08:05:56.000000000","message":"Done","commit_id":"a0bb38491f7b8db23e99d00ae30ff6f4ae703749"}],"nova/tests/unit/virt/libvirt/test_driver.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"a41597b7a6be0fdd5f8739e5458fa5ed60cb11ac","unresolved":false,"context_lines":[{"line_number":926,"context_line":"            drvr.capabilities[\u0027supports_ephemeral_encryption_luks\u0027],"},{"line_number":927,"context_line":"            \"Driver capabilities for \u0027supports_ephemeral_encryption_luks\u0027 \""},{"line_number":928,"context_line":"            \"is invalid\","},{"line_number":929,"context_line":"        )"},{"line_number":930,"context_line":""},{"line_number":931,"context_line":"    def test_driver_raises_on_non_linux_platform(self):"},{"line_number":932,"context_line":"        with utils.temporary_mutation(sys, platform\u003d\u0027darwin\u0027):"}],"source_content_type":"text/x-python","patch_set":28,"id":"239d8bd6_999bd81a","line":929,"updated":"2024-02-13 06:24:58.000000000","message":"ack, for now this is false as initally this is enabled only for qcow\nand flat is later in the series","commit_id":"a0bb38491f7b8db23e99d00ae30ff6f4ae703749"}],"nova/virt/images.py":[{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"ff38c1f3767d524442ac370bf8a494757bd53578","unresolved":true,"context_lines":[{"line_number":41,"context_line":"IMAGE_API \u003d glance.API()"},{"line_number":42,"context_line":""},{"line_number":43,"context_line":""},{"line_number":44,"context_line":"class EncryptionOptions(ty.TypedDict):"},{"line_number":45,"context_line":"    secret: str"},{"line_number":46,"context_line":"    format: str"},{"line_number":47,"context_line":""}],"source_content_type":"text/x-python","patch_set":29,"id":"16e6b350_b0e933c7","line":44,"range":{"start_line":44,"start_character":24,"end_line":44,"end_character":36},"updated":"2024-02-14 13:20:04.000000000","message":"Cool to have typing here.","commit_id":"54aba971a29b2a245d6f57ae2fa63065b848977a"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"00435d585ff7cd47a3515561cd97e334e16072d9","unresolved":false,"context_lines":[{"line_number":41,"context_line":"IMAGE_API \u003d glance.API()"},{"line_number":42,"context_line":""},{"line_number":43,"context_line":""},{"line_number":44,"context_line":"class EncryptionOptions(ty.TypedDict):"},{"line_number":45,"context_line":"    secret: str"},{"line_number":46,"context_line":"    format: str"},{"line_number":47,"context_line":""}],"source_content_type":"text/x-python","patch_set":29,"id":"ac7d796d_bd1988a4","line":44,"range":{"start_line":44,"start_character":24,"end_line":44,"end_character":36},"in_reply_to":"16e6b350_b0e933c7","updated":"2024-02-29 07:37:13.000000000","message":"Acknowledged","commit_id":"54aba971a29b2a245d6f57ae2fa63065b848977a"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"561800bdbd4e77f4e1aec96d05b6089ab9031f41","unresolved":true,"context_lines":[{"line_number":148,"context_line":"        raise exception.ImageUnacceptable(image_id\u003dimage_id, reason\u003dmsg)"},{"line_number":149,"context_line":""},{"line_number":150,"context_line":""},{"line_number":151,"context_line":"def fetch_to_raw("},{"line_number":152,"context_line":"    context: \u0027nova.context.RequestContext\u0027,"},{"line_number":153,"context_line":"    image_href: str,"},{"line_number":154,"context_line":"    path: str,"}],"source_content_type":"text/x-python","patch_set":33,"id":"cf104adf_9bb567e8","line":151,"updated":"2024-02-28 20:31:02.000000000","message":"So it\u0027s probably (maybe) not worth it, but this used to be actually \"fetch to raw and fail if it\u0027s not raw when we\u0027re done\". But now it is, I think, \"fetch to raw or you know that encrypted raw format we call luks, but still pretty raw and stuff.\" Adding \"or_luks\" doesn\u0027t really make it clearer, but perhaps a juicy docstring explaining that \"luks\" is really \"pretty raw\" or \"raw enough for what we\u0027re doing here\" or something?\n\nI\u0027m sure in three years when I\u0027m coming back through this (the horror) it will be hard to remember that luks\u003draw+encryption.","commit_id":"31a028d86a90ce86b3cbb5950932e08a812cc465"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"00435d585ff7cd47a3515561cd97e334e16072d9","unresolved":false,"context_lines":[{"line_number":148,"context_line":"        raise exception.ImageUnacceptable(image_id\u003dimage_id, reason\u003dmsg)"},{"line_number":149,"context_line":""},{"line_number":150,"context_line":""},{"line_number":151,"context_line":"def fetch_to_raw("},{"line_number":152,"context_line":"    context: \u0027nova.context.RequestContext\u0027,"},{"line_number":153,"context_line":"    image_href: str,"},{"line_number":154,"context_line":"    path: str,"}],"source_content_type":"text/x-python","patch_set":33,"id":"f0ddd025_d4130b83","line":151,"in_reply_to":"4970af9b_b81c958f","updated":"2024-02-29 07:37:13.000000000","message":"Done","commit_id":"31a028d86a90ce86b3cbb5950932e08a812cc465"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"bf4d1571d584cd993d3a9044c1e9c054c82fe8a6","unresolved":true,"context_lines":[{"line_number":148,"context_line":"        raise exception.ImageUnacceptable(image_id\u003dimage_id, reason\u003dmsg)"},{"line_number":149,"context_line":""},{"line_number":150,"context_line":""},{"line_number":151,"context_line":"def fetch_to_raw("},{"line_number":152,"context_line":"    context: \u0027nova.context.RequestContext\u0027,"},{"line_number":153,"context_line":"    image_href: str,"},{"line_number":154,"context_line":"    path: str,"}],"source_content_type":"text/x-python","patch_set":33,"id":"f24c9e6f_11be0efd","line":151,"in_reply_to":"4ec04b7c_05e84548","updated":"2024-02-28 21:45:39.000000000","message":"Yeah I think it\u0027s to verbose to make it like that. Maybe \"fetch_to_flat\" I dunno. Either way, I think docstring will help for the future case, assuming someone at least comes to read it instead of inferring from the name.","commit_id":"31a028d86a90ce86b3cbb5950932e08a812cc465"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"62a137d49ce924dbfcd597855024c59fb7836266","unresolved":true,"context_lines":[{"line_number":148,"context_line":"        raise exception.ImageUnacceptable(image_id\u003dimage_id, reason\u003dmsg)"},{"line_number":149,"context_line":""},{"line_number":150,"context_line":""},{"line_number":151,"context_line":"def fetch_to_raw("},{"line_number":152,"context_line":"    context: \u0027nova.context.RequestContext\u0027,"},{"line_number":153,"context_line":"    image_href: str,"},{"line_number":154,"context_line":"    path: str,"}],"source_content_type":"text/x-python","patch_set":33,"id":"4ec04b7c_05e84548","line":151,"in_reply_to":"cf104adf_9bb567e8","updated":"2024-02-28 21:26:44.000000000","message":"Maybe fetch_to_raw_or_encrypted_raw? At least to me it seemed still relevant, it\u0027s just qemu that decided to use a new name/format for an encrypted raw image.\n\nEither way I think it\u0027s a good idea to add some info to the docstring, so I\u0027ll do that.","commit_id":"31a028d86a90ce86b3cbb5950932e08a812cc465"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"b97b9e00b8258660bcc8fc51e9ff37da06d5f827","unresolved":true,"context_lines":[{"line_number":148,"context_line":"        raise exception.ImageUnacceptable(image_id\u003dimage_id, reason\u003dmsg)"},{"line_number":149,"context_line":""},{"line_number":150,"context_line":""},{"line_number":151,"context_line":"def fetch_to_raw("},{"line_number":152,"context_line":"    context: \u0027nova.context.RequestContext\u0027,"},{"line_number":153,"context_line":"    image_href: str,"},{"line_number":154,"context_line":"    path: str,"}],"source_content_type":"text/x-python","patch_set":33,"id":"4970af9b_b81c958f","line":151,"in_reply_to":"f24c9e6f_11be0efd","updated":"2024-02-28 22:27:59.000000000","message":"Ack, fetch_to_flat makes sense.","commit_id":"31a028d86a90ce86b3cbb5950932e08a812cc465"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"561800bdbd4e77f4e1aec96d05b6089ab9031f41","unresolved":true,"context_lines":[{"line_number":179,"context_line":""},{"line_number":180,"context_line":"        if fmt not in (\"raw\", \"luks\") and CONF.force_raw_images:"},{"line_number":181,"context_line":"            staged \u003d \"%s.converted\" % path"},{"line_number":182,"context_line":"            dest_fmt \u003d \u0027raw\u0027 if not src_encryption else \u0027luks\u0027"},{"line_number":183,"context_line":"            LOG.debug(\"%s was %s, converting to %s\", image_href, fmt, dest_fmt)"},{"line_number":184,"context_line":"            with fileutils.remove_path_on_error(staged):"},{"line_number":185,"context_line":"                try:"}],"source_content_type":"text/x-python","patch_set":33,"id":"91575a44_c4c8e8cc","line":182,"updated":"2024-02-28 20:31:02.000000000","message":"I don\u0027t think I get this. You\u0027re saying dest_fmt will be luks (encrypted raw) if the *source* encryption is set, right? Then you\u0027re passing dest_encryption to convert_image below. If src_encryption is None (which I think it always will be according to the statement that this doesn\u0027t support encrypted backing files), won\u0027t you always be asking for dest_fmt\u003draw, and then passing in some dest_encryption details which specify otherwise?\n\nKnowing how loopy all this fetch, fetch_image, fetch_to_raw, fetch_a_bone stuff is, I\u0027m sure there\u0027s some detail I\u0027m missing here.","commit_id":"31a028d86a90ce86b3cbb5950932e08a812cc465"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"00435d585ff7cd47a3515561cd97e334e16072d9","unresolved":false,"context_lines":[{"line_number":179,"context_line":""},{"line_number":180,"context_line":"        if fmt not in (\"raw\", \"luks\") and CONF.force_raw_images:"},{"line_number":181,"context_line":"            staged \u003d \"%s.converted\" % path"},{"line_number":182,"context_line":"            dest_fmt \u003d \u0027raw\u0027 if not src_encryption else \u0027luks\u0027"},{"line_number":183,"context_line":"            LOG.debug(\"%s was %s, converting to %s\", image_href, fmt, dest_fmt)"},{"line_number":184,"context_line":"            with fileutils.remove_path_on_error(staged):"},{"line_number":185,"context_line":"                try:"}],"source_content_type":"text/x-python","patch_set":33,"id":"0443ad5a_bad1fc75","line":182,"in_reply_to":"7d46d8a5_0d91f61d","updated":"2024-02-29 07:37:13.000000000","message":"Done","commit_id":"31a028d86a90ce86b3cbb5950932e08a812cc465"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"00435d585ff7cd47a3515561cd97e334e16072d9","unresolved":true,"context_lines":[{"line_number":179,"context_line":""},{"line_number":180,"context_line":"        if fmt not in (\"raw\", \"luks\") and CONF.force_raw_images:"},{"line_number":181,"context_line":"            staged \u003d \"%s.converted\" % path"},{"line_number":182,"context_line":"            dest_fmt \u003d \u0027raw\u0027 if not src_encryption else \u0027luks\u0027"},{"line_number":183,"context_line":"            LOG.debug(\"%s was %s, converting to %s\", image_href, fmt, dest_fmt)"},{"line_number":184,"context_line":"            with fileutils.remove_path_on_error(staged):"},{"line_number":185,"context_line":"                try:"}],"source_content_type":"text/x-python","patch_set":33,"id":"df60b891_9d051461","line":182,"in_reply_to":"7d46d8a5_0d91f61d","updated":"2024-02-29 07:37:13.000000000","message":"Now that I go to change it, I realize why it\u0027s based on whether the source is encrypted. It\u0027s because this is the function that generates backing files, so if the incoming base image is unencrypted qcow2, the output should be raw. But if the incoming source base image is encrypted qcow2, then the output should be luks. So the target format does depend on the source format.\n\ndest_encryption is providing the passphrase for the target encrypted image and src_encryption is providing the passphrase for the incoming source encrypted image.\n\nSo I\u0027m not sure what to do here other than add code comments, but I\u0027ll keep thinking about it in case something makes sense.","commit_id":"31a028d86a90ce86b3cbb5950932e08a812cc465"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"7d17c244442d03e56bf24df724de492026acf989","unresolved":false,"context_lines":[{"line_number":179,"context_line":""},{"line_number":180,"context_line":"        if fmt not in (\"raw\", \"luks\") and CONF.force_raw_images:"},{"line_number":181,"context_line":"            staged \u003d \"%s.converted\" % path"},{"line_number":182,"context_line":"            dest_fmt \u003d \u0027raw\u0027 if not src_encryption else \u0027luks\u0027"},{"line_number":183,"context_line":"            LOG.debug(\"%s was %s, converting to %s\", image_href, fmt, dest_fmt)"},{"line_number":184,"context_line":"            with fileutils.remove_path_on_error(staged):"},{"line_number":185,"context_line":"                try:"}],"source_content_type":"text/x-python","patch_set":33,"id":"81390e98_5f8aa01f","line":182,"in_reply_to":"86900590_5e24603e","updated":"2024-03-02 01:26:40.000000000","message":"Done","commit_id":"31a028d86a90ce86b3cbb5950932e08a812cc465"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"b736df2b87def0f30e20e6cfaeb40bf9688c7156","unresolved":true,"context_lines":[{"line_number":179,"context_line":""},{"line_number":180,"context_line":"        if fmt not in (\"raw\", \"luks\") and CONF.force_raw_images:"},{"line_number":181,"context_line":"            staged \u003d \"%s.converted\" % path"},{"line_number":182,"context_line":"            dest_fmt \u003d \u0027raw\u0027 if not src_encryption else \u0027luks\u0027"},{"line_number":183,"context_line":"            LOG.debug(\"%s was %s, converting to %s\", image_href, fmt, dest_fmt)"},{"line_number":184,"context_line":"            with fileutils.remove_path_on_error(staged):"},{"line_number":185,"context_line":"                try:"}],"source_content_type":"text/x-python","patch_set":33,"id":"86900590_5e24603e","line":182,"in_reply_to":"87c88ac1_323cf55e","updated":"2024-02-29 16:41:59.000000000","message":"Sorry, please disregard that comment, I forgot to discard it after realizing it didn\u0027t make sense. I made the change to use dest_encryption after that and forgot to delete the comment.","commit_id":"31a028d86a90ce86b3cbb5950932e08a812cc465"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"62a137d49ce924dbfcd597855024c59fb7836266","unresolved":true,"context_lines":[{"line_number":179,"context_line":""},{"line_number":180,"context_line":"        if fmt not in (\"raw\", \"luks\") and CONF.force_raw_images:"},{"line_number":181,"context_line":"            staged \u003d \"%s.converted\" % path"},{"line_number":182,"context_line":"            dest_fmt \u003d \u0027raw\u0027 if not src_encryption else \u0027luks\u0027"},{"line_number":183,"context_line":"            LOG.debug(\"%s was %s, converting to %s\", image_href, fmt, dest_fmt)"},{"line_number":184,"context_line":"            with fileutils.remove_path_on_error(staged):"},{"line_number":185,"context_line":"                try:"}],"source_content_type":"text/x-python","patch_set":33,"id":"e81a8702_268d4f00","line":182,"in_reply_to":"91575a44_c4c8e8cc","updated":"2024-02-28 21:26:44.000000000","message":"Right. My thinking is, if you are trying to convert an encrypted qcow2 image to a raw image, you probably want the converted image to be encrypted. Otherwise we would be decrypting an encrypted image that we downloaded.\n\nYou are right that the above scenario involving \u0027luks\u0027 likely only applies when you either have encrypted qcow2 backing files or `images_type \u003d raw`. So technically I could move the addition of the \u0027luks\u0027 file format to the \"support encrypted backing files\" patch.\n\nI might have been thinking, well what if someone calls fetch_to_raw for some other reason but I guess that also wouldn\u0027t be relevant either until the \"support encryption with the raw image backend\" patch.\n\nIf you want me to move the \u0027luks\u0027 parts to the later patch, the \"support encryption with the raw image backend\" patch, I can do that.","commit_id":"31a028d86a90ce86b3cbb5950932e08a812cc465"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"b97b9e00b8258660bcc8fc51e9ff37da06d5f827","unresolved":true,"context_lines":[{"line_number":179,"context_line":""},{"line_number":180,"context_line":"        if fmt not in (\"raw\", \"luks\") and CONF.force_raw_images:"},{"line_number":181,"context_line":"            staged \u003d \"%s.converted\" % path"},{"line_number":182,"context_line":"            dest_fmt \u003d \u0027raw\u0027 if not src_encryption else \u0027luks\u0027"},{"line_number":183,"context_line":"            LOG.debug(\"%s was %s, converting to %s\", image_href, fmt, dest_fmt)"},{"line_number":184,"context_line":"            with fileutils.remove_path_on_error(staged):"},{"line_number":185,"context_line":"                try:"}],"source_content_type":"text/x-python","patch_set":33,"id":"7d46d8a5_0d91f61d","line":182,"in_reply_to":"ca07dcda_759c4934","updated":"2024-02-28 22:27:59.000000000","message":"OK, I see what you\u0027re saying now. In earlier iterations I didn\u0027t have dest_encryption passed in here and I was setting the target format based on the source. But I think you\u0027re right, since it has dest_encryption now, it should use that to decide whether to encrypt the target raw image. I\u0027ll change it.","commit_id":"31a028d86a90ce86b3cbb5950932e08a812cc465"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"3e5d31fd076371e43cc201477f6175a82e9958e1","unresolved":true,"context_lines":[{"line_number":179,"context_line":""},{"line_number":180,"context_line":"        if fmt not in (\"raw\", \"luks\") and CONF.force_raw_images:"},{"line_number":181,"context_line":"            staged \u003d \"%s.converted\" % path"},{"line_number":182,"context_line":"            dest_fmt \u003d \u0027raw\u0027 if not src_encryption else \u0027luks\u0027"},{"line_number":183,"context_line":"            LOG.debug(\"%s was %s, converting to %s\", image_href, fmt, dest_fmt)"},{"line_number":184,"context_line":"            with fileutils.remove_path_on_error(staged):"},{"line_number":185,"context_line":"                try:"}],"source_content_type":"text/x-python","patch_set":33,"id":"87c88ac1_323cf55e","line":182,"in_reply_to":"df60b891_9d051461","updated":"2024-02-29 16:35:31.000000000","message":"\u003e So I\u0027m not sure what to do here other than add code comments, but I\u0027ll keep thinking about it in case something makes sense.\n\nOkay, I\u0027m missing something I guess because it doesn\u0027t seem to make sense to me to say \"I want the dest (which is going to be a backing file, sure) to be encrypted with this key\" and then end up with a non-encrypted raw file because the thing we downloaded was raw.\n\nIf it really makes sense to be doing this, then I think we probably need a re-think on all the naming here in order to make it clear what\u0027s going on. Seems like a security auditor has to be able to understand why you\u0027re choosing a non-encrypted image.","commit_id":"31a028d86a90ce86b3cbb5950932e08a812cc465"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"bf4d1571d584cd993d3a9044c1e9c054c82fe8a6","unresolved":true,"context_lines":[{"line_number":179,"context_line":""},{"line_number":180,"context_line":"        if fmt not in (\"raw\", \"luks\") and CONF.force_raw_images:"},{"line_number":181,"context_line":"            staged \u003d \"%s.converted\" % path"},{"line_number":182,"context_line":"            dest_fmt \u003d \u0027raw\u0027 if not src_encryption else \u0027luks\u0027"},{"line_number":183,"context_line":"            LOG.debug(\"%s was %s, converting to %s\", image_href, fmt, dest_fmt)"},{"line_number":184,"context_line":"            with fileutils.remove_path_on_error(staged):"},{"line_number":185,"context_line":"                try:"}],"source_content_type":"text/x-python","patch_set":33,"id":"ca07dcda_759c4934","line":182,"in_reply_to":"e81a8702_268d4f00","updated":"2024-02-28 21:45:39.000000000","message":"Maybe I need more time to grok this, but .. I\u0027m not sure what you\u0027re saying. If I call this with dest_encryption\u003dyesplease and the src_encryption is falsey, then we\u0027ll set dest_format\u003draw and even though we call with dest_encryption\u003dyesplease, we\u0027ll get unencrypted raw? And even if that\u0027s not the case, I think it\u0027s very confusing to base the format\u0027s raw-or-encrypted format on something other than dest_encryption...","commit_id":"31a028d86a90ce86b3cbb5950932e08a812cc465"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"561800bdbd4e77f4e1aec96d05b6089ab9031f41","unresolved":true,"context_lines":[{"line_number":198,"context_line":""},{"line_number":199,"context_line":"                data \u003d qemu_img_info(staged)"},{"line_number":200,"context_line":"                if data.file_format not in (\"raw\", \"luks\"):"},{"line_number":201,"context_line":"                    raise exception.ImageUnacceptable(image_id\u003dimage_href,"},{"line_number":202,"context_line":"                        reason\u003d_(\"Converted to %s, but format is now %s\") %"},{"line_number":203,"context_line":"                        (dest_fmt, data.file_format))"},{"line_number":204,"context_line":""}],"source_content_type":"text/x-python","patch_set":33,"id":"b40575ed_234eb3a9","line":201,"updated":"2024-02-28 20:31:02.000000000","message":"Never tested? Presumably since we already can raise out of here we\u0027re in good shape for testing that this method can raise that and the caller expects it, but might be good to make sure we roll over this at least.","commit_id":"31a028d86a90ce86b3cbb5950932e08a812cc465"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"00435d585ff7cd47a3515561cd97e334e16072d9","unresolved":false,"context_lines":[{"line_number":198,"context_line":""},{"line_number":199,"context_line":"                data \u003d qemu_img_info(staged)"},{"line_number":200,"context_line":"                if data.file_format not in (\"raw\", \"luks\"):"},{"line_number":201,"context_line":"                    raise exception.ImageUnacceptable(image_id\u003dimage_href,"},{"line_number":202,"context_line":"                        reason\u003d_(\"Converted to %s, but format is now %s\") %"},{"line_number":203,"context_line":"                        (dest_fmt, data.file_format))"},{"line_number":204,"context_line":""}],"source_content_type":"text/x-python","patch_set":33,"id":"e94fae68_3be46ddc","line":201,"in_reply_to":"ac5ee6db_08f46654","updated":"2024-02-29 07:37:13.000000000","message":"Done","commit_id":"31a028d86a90ce86b3cbb5950932e08a812cc465"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"62a137d49ce924dbfcd597855024c59fb7836266","unresolved":true,"context_lines":[{"line_number":198,"context_line":""},{"line_number":199,"context_line":"                data \u003d qemu_img_info(staged)"},{"line_number":200,"context_line":"                if data.file_format not in (\"raw\", \"luks\"):"},{"line_number":201,"context_line":"                    raise exception.ImageUnacceptable(image_id\u003dimage_href,"},{"line_number":202,"context_line":"                        reason\u003d_(\"Converted to %s, but format is now %s\") %"},{"line_number":203,"context_line":"                        (dest_fmt, data.file_format))"},{"line_number":204,"context_line":""}],"source_content_type":"text/x-python","patch_set":33,"id":"ac5ee6db_08f46654","line":201,"in_reply_to":"b40575ed_234eb3a9","updated":"2024-02-28 21:26:44.000000000","message":"OK, will add that.","commit_id":"31a028d86a90ce86b3cbb5950932e08a812cc465"}],"nova/virt/libvirt/driver.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"2fa214491af72660ad5be4769acf13bc1bb8a7ce","unresolved":true,"context_lines":[{"line_number":4767,"context_line":"        # and then luksClose-ing it. Encrypting the image after creating both"},{"line_number":4768,"context_line":"        # the image and filesystem is fewer steps at the cost of being"},{"line_number":4769,"context_line":"        # slower."},{"line_number":4770,"context_line":""},{"line_number":4771,"context_line":"        # Do conversion like nova.virt.images.fetch_to_raw does."},{"line_number":4772,"context_line":"        LOG.debug(f\"Converting \u0027raw\u0027 to \u0027{encryption.get(\u0027format\u0027)}\u0027\")"},{"line_number":4773,"context_line":"        staged \u003d f\u0027{target}.converted\u0027"}],"source_content_type":"text/x-python","patch_set":24,"id":"d33d0c6f_f83f08ab","line":4770,"updated":"2024-01-31 09:22:59.000000000","message":"so the more secure approch is to create the file on disk, create a luks partion in the file then copying the data to the encupted parthionadn closign it.\n\nif we are supproting encypted source images in a future patch we should never have the content of either the souce or dest image decypted at rest so i think the luks opening and closing approch is not just faster but more correct form a security perspective too.","commit_id":"e40a49f0c6859c9561d68105cc706b6c798a7b80"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"7e9184b4ca5c8cfe2f2bc9ab97a0fe529a50025d","unresolved":true,"context_lines":[{"line_number":4767,"context_line":"        # and then luksClose-ing it. Encrypting the image after creating both"},{"line_number":4768,"context_line":"        # the image and filesystem is fewer steps at the cost of being"},{"line_number":4769,"context_line":"        # slower."},{"line_number":4770,"context_line":""},{"line_number":4771,"context_line":"        # Do conversion like nova.virt.images.fetch_to_raw does."},{"line_number":4772,"context_line":"        LOG.debug(f\"Converting \u0027raw\u0027 to \u0027{encryption.get(\u0027format\u0027)}\u0027\")"},{"line_number":4773,"context_line":"        staged \u003d f\u0027{target}.converted\u0027"}],"source_content_type":"text/x-python","patch_set":24,"id":"ea3bcff5_ee495ed8","line":4770,"in_reply_to":"d33d0c6f_f83f08ab","updated":"2024-02-01 02:14:49.000000000","message":"Agree about not having anything decrypted at rest when source images are supported (I have later patches for that).\n\nI however thought that ephemeral and swap, because they are just blank + mkfs or mkswap there isn\u0027t really anything secret about it, no? As far as I have seen, ephemeral and swap disks are never created by a provided image, so there can\u0027t be any encrypted source images for them.","commit_id":"e40a49f0c6859c9561d68105cc706b6c798a7b80"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"a41597b7a6be0fdd5f8739e5458fa5ed60cb11ac","unresolved":false,"context_lines":[{"line_number":4767,"context_line":"        # and then luksClose-ing it. Encrypting the image after creating both"},{"line_number":4768,"context_line":"        # the image and filesystem is fewer steps at the cost of being"},{"line_number":4769,"context_line":"        # slower."},{"line_number":4770,"context_line":""},{"line_number":4771,"context_line":"        # Do conversion like nova.virt.images.fetch_to_raw does."},{"line_number":4772,"context_line":"        LOG.debug(f\"Converting \u0027raw\u0027 to \u0027{encryption.get(\u0027format\u0027)}\u0027\")"},{"line_number":4773,"context_line":"        staged \u003d f\u0027{target}.converted\u0027"}],"source_content_type":"text/x-python","patch_set":24,"id":"a9c85cd5_90f728a1","line":4770,"in_reply_to":"ea3bcff5_ee495ed8","updated":"2024-02-13 06:24:58.000000000","message":"for ephemeral and swap your correct\nalthough if its still faster to do the luks open and close\ninstead of create blank,format and encypet then it might be worth it.\n\nthis is not imporant right now and we can revisit this later in the seriese if we think its worth the optimization","commit_id":"e40a49f0c6859c9561d68105cc706b6c798a7b80"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"9c604447a39b82a92c2e48f18ac52515d7ec85c3","unresolved":true,"context_lines":[{"line_number":11187,"context_line":"        # instance system metadata in _populate_instance_for_create() in"},{"line_number":11188,"context_line":"        # nova.compute.API."},{"line_number":11189,"context_line":"        secret_uuid \u003d instance.system_metadata.get("},{"line_number":11190,"context_line":"            utils.SM_IMAGE_PROP_PREFIX + \u0027hw_ephemeral_encryption_secret_uuid\u0027)"},{"line_number":11191,"context_line":"        rescue_image_secret_uuid \u003d instance.system_metadata.get("},{"line_number":11192,"context_line":"            \u0027rescue_\u0027 + utils.SM_IMAGE_PROP_PREFIX +"},{"line_number":11193,"context_line":"            \u0027hw_ephemeral_encryption_secret_uuid\u0027)"}],"source_content_type":"text/x-python","patch_set":24,"id":"872539b6_8c2cd43f","line":11190,"updated":"2024-01-31 09:45:04.000000000","message":"you do not add this to the image object until \nhttps://review.opendev.org/c/openstack/nova/+/870935/26\nso that should be before this patch","commit_id":"e40a49f0c6859c9561d68105cc706b6c798a7b80"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"7e9184b4ca5c8cfe2f2bc9ab97a0fe529a50025d","unresolved":true,"context_lines":[{"line_number":11187,"context_line":"        # instance system metadata in _populate_instance_for_create() in"},{"line_number":11188,"context_line":"        # nova.compute.API."},{"line_number":11189,"context_line":"        secret_uuid \u003d instance.system_metadata.get("},{"line_number":11190,"context_line":"            utils.SM_IMAGE_PROP_PREFIX + \u0027hw_ephemeral_encryption_secret_uuid\u0027)"},{"line_number":11191,"context_line":"        rescue_image_secret_uuid \u003d instance.system_metadata.get("},{"line_number":11192,"context_line":"            \u0027rescue_\u0027 + utils.SM_IMAGE_PROP_PREFIX +"},{"line_number":11193,"context_line":"            \u0027hw_ephemeral_encryption_secret_uuid\u0027)"}],"source_content_type":"text/x-python","patch_set":24,"id":"ab363ae4_31f278fe","line":11190,"in_reply_to":"872539b6_8c2cd43f","updated":"2024-02-01 02:14:49.000000000","message":"Yeah, I ran into this locally failing 😣 I\u0027m fixing it.","commit_id":"e40a49f0c6859c9561d68105cc706b6c798a7b80"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"a41597b7a6be0fdd5f8739e5458fa5ed60cb11ac","unresolved":false,"context_lines":[{"line_number":11187,"context_line":"        # instance system metadata in _populate_instance_for_create() in"},{"line_number":11188,"context_line":"        # nova.compute.API."},{"line_number":11189,"context_line":"        secret_uuid \u003d instance.system_metadata.get("},{"line_number":11190,"context_line":"            utils.SM_IMAGE_PROP_PREFIX + \u0027hw_ephemeral_encryption_secret_uuid\u0027)"},{"line_number":11191,"context_line":"        rescue_image_secret_uuid \u003d instance.system_metadata.get("},{"line_number":11192,"context_line":"            \u0027rescue_\u0027 + utils.SM_IMAGE_PROP_PREFIX +"},{"line_number":11193,"context_line":"            \u0027hw_ephemeral_encryption_secret_uuid\u0027)"}],"source_content_type":"text/x-python","patch_set":24,"id":"a90192d1_0dfb1ff2","line":11190,"in_reply_to":"ab363ae4_31f278fe","updated":"2024-02-13 06:24:58.000000000","message":"Done","commit_id":"e40a49f0c6859c9561d68105cc706b6c798a7b80"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"2fa214491af72660ad5be4769acf13bc1bb8a7ce","unresolved":true,"context_lines":[{"line_number":11193,"context_line":"            \u0027hw_ephemeral_encryption_secret_uuid\u0027)"},{"line_number":11194,"context_line":"        # If this is a rescue, we want to fetch the rescue image if one was"},{"line_number":11195,"context_line":"        # specified."},{"line_number":11196,"context_line":"        secret_uuid \u003d rescue_image_secret_uuid or secret_uuid"},{"line_number":11197,"context_line":""},{"line_number":11198,"context_line":"        image_encryption \u003d None"},{"line_number":11199,"context_line":"        if secret_uuid:"}],"source_content_type":"text/x-python","patch_set":24,"id":"75a4e1f0_e6f78e6e","line":11196,"updated":"2024-01-31 09:22:59.000000000","message":"i think i mentioned this before but there are 3 sources for the rescue iamge that is used.\n\nit can be provided in the api request.\nit can be specifced in the nova.conf \nif otherwise we use the same image the instance was booted with for rescue.\n\nthis does not take account fo all 3 as far as i can see.\n\nthe isntance system_metadata wont supprot the first 2 options and i dont thikn we populate the third today?","commit_id":"e40a49f0c6859c9561d68105cc706b6c798a7b80"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"7e9184b4ca5c8cfe2f2bc9ab97a0fe529a50025d","unresolved":true,"context_lines":[{"line_number":11193,"context_line":"            \u0027hw_ephemeral_encryption_secret_uuid\u0027)"},{"line_number":11194,"context_line":"        # If this is a rescue, we want to fetch the rescue image if one was"},{"line_number":11195,"context_line":"        # specified."},{"line_number":11196,"context_line":"        secret_uuid \u003d rescue_image_secret_uuid or secret_uuid"},{"line_number":11197,"context_line":""},{"line_number":11198,"context_line":"        image_encryption \u003d None"},{"line_number":11199,"context_line":"        if secret_uuid:"}],"source_content_type":"text/x-python","patch_set":24,"id":"c4798d24_2b777e57","line":11196,"in_reply_to":"75a4e1f0_e6f78e6e","updated":"2024-02-01 02:14:49.000000000","message":"Gah, another place I messed up a rebase/redistribution of update patches. The rescue image bit belongs in the rescue patch. It will make sense there (I hope 😝)","commit_id":"e40a49f0c6859c9561d68105cc706b6c798a7b80"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"a41597b7a6be0fdd5f8739e5458fa5ed60cb11ac","unresolved":false,"context_lines":[{"line_number":11193,"context_line":"            \u0027hw_ephemeral_encryption_secret_uuid\u0027)"},{"line_number":11194,"context_line":"        # If this is a rescue, we want to fetch the rescue image if one was"},{"line_number":11195,"context_line":"        # specified."},{"line_number":11196,"context_line":"        secret_uuid \u003d rescue_image_secret_uuid or secret_uuid"},{"line_number":11197,"context_line":""},{"line_number":11198,"context_line":"        image_encryption \u003d None"},{"line_number":11199,"context_line":"        if secret_uuid:"}],"source_content_type":"text/x-python","patch_set":24,"id":"b6c45fdc_00d75bc4","line":11196,"in_reply_to":"c4798d24_2b777e57","updated":"2024-02-13 06:24:58.000000000","message":"Acknowledged","commit_id":"e40a49f0c6859c9561d68105cc706b6c798a7b80"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"561800bdbd4e77f4e1aec96d05b6089ab9031f41","unresolved":true,"context_lines":[{"line_number":1794,"context_line":"                secret_usage \u003d secret.usageID()"},{"line_number":1795,"context_line":"                LOG.info("},{"line_number":1796,"context_line":"                    \u0027Cleaning up unused libvirt secret with UUID: \u0027"},{"line_number":1797,"context_line":"                    f\u0027{secret_uuid} and usage_id: {secret_usage}\u0027)"},{"line_number":1798,"context_line":"                try:"},{"line_number":1799,"context_line":"                    self._host.delete_secret(\u0027volume\u0027, secret_usage)"},{"line_number":1800,"context_line":"                except libvirt.libvirtError as e:"}],"source_content_type":"text/x-python","patch_set":33,"id":"e0d55c5e_9f8ffdec","line":1797,"updated":"2024-02-28 20:31:02.000000000","message":"Usage is something like \"volume\" or \"nic\" or something, right? The secret_uuid is the uuid in barbican? Seems like it might be good to also log the instance uuid, which I think is in the name or whatever right? Just thinking that for forensics if we come looking for \"what happened to this instance that now has no key?\" we\u0027ll be likely looking for the instance uuid in the logs.","commit_id":"31a028d86a90ce86b3cbb5950932e08a812cc465"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"bf4d1571d584cd993d3a9044c1e9c054c82fe8a6","unresolved":true,"context_lines":[{"line_number":1794,"context_line":"                secret_usage \u003d secret.usageID()"},{"line_number":1795,"context_line":"                LOG.info("},{"line_number":1796,"context_line":"                    \u0027Cleaning up unused libvirt secret with UUID: \u0027"},{"line_number":1797,"context_line":"                    f\u0027{secret_uuid} and usage_id: {secret_usage}\u0027)"},{"line_number":1798,"context_line":"                try:"},{"line_number":1799,"context_line":"                    self._host.delete_secret(\u0027volume\u0027, secret_usage)"},{"line_number":1800,"context_line":"                except libvirt.libvirtError as e:"}],"source_content_type":"text/x-python","patch_set":33,"id":"bb9a23b1_74ecb600","line":1797,"in_reply_to":"6bf6d1a9_91523f40","updated":"2024-02-28 21:45:39.000000000","message":"Oh usage_id is the combo, okay. I thought it was just going to be \"volume\". Cool, all good then.","commit_id":"31a028d86a90ce86b3cbb5950932e08a812cc465"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"00435d585ff7cd47a3515561cd97e334e16072d9","unresolved":false,"context_lines":[{"line_number":1794,"context_line":"                secret_usage \u003d secret.usageID()"},{"line_number":1795,"context_line":"                LOG.info("},{"line_number":1796,"context_line":"                    \u0027Cleaning up unused libvirt secret with UUID: \u0027"},{"line_number":1797,"context_line":"                    f\u0027{secret_uuid} and usage_id: {secret_usage}\u0027)"},{"line_number":1798,"context_line":"                try:"},{"line_number":1799,"context_line":"                    self._host.delete_secret(\u0027volume\u0027, secret_usage)"},{"line_number":1800,"context_line":"                except libvirt.libvirtError as e:"}],"source_content_type":"text/x-python","patch_set":33,"id":"6322c474_6f7d373c","line":1797,"in_reply_to":"bb9a23b1_74ecb600","updated":"2024-02-29 07:37:13.000000000","message":"Acknowledged","commit_id":"31a028d86a90ce86b3cbb5950932e08a812cc465"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"62a137d49ce924dbfcd597855024c59fb7836266","unresolved":true,"context_lines":[{"line_number":1794,"context_line":"                secret_usage \u003d secret.usageID()"},{"line_number":1795,"context_line":"                LOG.info("},{"line_number":1796,"context_line":"                    \u0027Cleaning up unused libvirt secret with UUID: \u0027"},{"line_number":1797,"context_line":"                    f\u0027{secret_uuid} and usage_id: {secret_usage}\u0027)"},{"line_number":1798,"context_line":"                try:"},{"line_number":1799,"context_line":"                    self._host.delete_secret(\u0027volume\u0027, secret_usage)"},{"line_number":1800,"context_line":"                except libvirt.libvirtError as e:"}],"source_content_type":"text/x-python","patch_set":33,"id":"6bf6d1a9_91523f40","line":1797,"in_reply_to":"e0d55c5e_9f8ffdec","updated":"2024-02-28 21:26:44.000000000","message":"\"Usage type\" is \"volume\" or similar, secret_uuid is the UUID in Barbican, and usage_id is of the pattern \u003cinstance uuid\u003e_\u003cbdm uuid\u003e. So we are already logging the instance uuid as part of logging the usage_id. I could however parse out the instance uuid from the usage_id to add logging of the instance uuid a second time via LOG(..., instance_uuid\u003dinstance_uuid) which makes the pretty brackets in the log.","commit_id":"31a028d86a90ce86b3cbb5950932e08a812cc465"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"561800bdbd4e77f4e1aec96d05b6089ab9031f41","unresolved":true,"context_lines":[{"line_number":11420,"context_line":"            image_encryption \u003d {"},{"line_number":11421,"context_line":"                \u0027secret\u0027: secret,"},{"line_number":11422,"context_line":"                \u0027format\u0027: encryption_format or"},{"line_number":11423,"context_line":"                          CONF.ephemeral_storage_encryption.default_format}"},{"line_number":11424,"context_line":"        try:"},{"line_number":11425,"context_line":"            image.cache(fetch_func\u003dfetch_func,"},{"line_number":11426,"context_line":"                        context\u003dcontext,"}],"source_content_type":"text/x-python","patch_set":33,"id":"f33db245_037d5a06","line":11423,"updated":"2024-02-28 20:31:02.000000000","message":"Hmm is this really the best idea? It seems to me like we should always be setting that key on images we create, and we should always be expecting it to match something we support. If the image was created with LUKS2, but not denoted as such and we try to parse it with LUSKS3 or some other scheme we may very well break it. LUKS has a header so it\u0027s probably good for any other LUKS version, but I think dm-crypt before it famously didn\u0027t. Taking the current compute node\u0027s default doesn\u0027t really make sense to me. When would we ever need to allow this to be unset?\n\nI know this kinda came up in the discussion about this being a user-specified thing, but the detail here is more that the image is _already_ in a format that we have to be able to support here.","commit_id":"31a028d86a90ce86b3cbb5950932e08a812cc465"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"bf4d1571d584cd993d3a9044c1e9c054c82fe8a6","unresolved":true,"context_lines":[{"line_number":11420,"context_line":"            image_encryption \u003d {"},{"line_number":11421,"context_line":"                \u0027secret\u0027: secret,"},{"line_number":11422,"context_line":"                \u0027format\u0027: encryption_format or"},{"line_number":11423,"context_line":"                          CONF.ephemeral_storage_encryption.default_format}"},{"line_number":11424,"context_line":"        try:"},{"line_number":11425,"context_line":"            image.cache(fetch_func\u003dfetch_func,"},{"line_number":11426,"context_line":"                        context\u003dcontext,"}],"source_content_type":"text/x-python","patch_set":33,"id":"a966abe2_45600eb3","line":11423,"in_reply_to":"06d8ba52_0ae0cdec","updated":"2024-02-28 21:45:39.000000000","message":"Yeah, I think you should require both set if one is.","commit_id":"31a028d86a90ce86b3cbb5950932e08a812cc465"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"00435d585ff7cd47a3515561cd97e334e16072d9","unresolved":false,"context_lines":[{"line_number":11420,"context_line":"            image_encryption \u003d {"},{"line_number":11421,"context_line":"                \u0027secret\u0027: secret,"},{"line_number":11422,"context_line":"                \u0027format\u0027: encryption_format or"},{"line_number":11423,"context_line":"                          CONF.ephemeral_storage_encryption.default_format}"},{"line_number":11424,"context_line":"        try:"},{"line_number":11425,"context_line":"            image.cache(fetch_func\u003dfetch_func,"},{"line_number":11426,"context_line":"                        context\u003dcontext,"}],"source_content_type":"text/x-python","patch_set":33,"id":"4bed7ebe_6e499824","line":11423,"in_reply_to":"a966abe2_45600eb3","updated":"2024-02-29 07:37:13.000000000","message":"Done","commit_id":"31a028d86a90ce86b3cbb5950932e08a812cc465"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"62a137d49ce924dbfcd597855024c59fb7836266","unresolved":true,"context_lines":[{"line_number":11420,"context_line":"            image_encryption \u003d {"},{"line_number":11421,"context_line":"                \u0027secret\u0027: secret,"},{"line_number":11422,"context_line":"                \u0027format\u0027: encryption_format or"},{"line_number":11423,"context_line":"                          CONF.ephemeral_storage_encryption.default_format}"},{"line_number":11424,"context_line":"        try:"},{"line_number":11425,"context_line":"            image.cache(fetch_func\u003dfetch_func,"},{"line_number":11426,"context_line":"                        context\u003dcontext,"}],"source_content_type":"text/x-python","patch_set":33,"id":"06d8ba52_0ae0cdec","line":11423,"in_reply_to":"f33db245_037d5a06","updated":"2024-02-28 21:26:44.000000000","message":"Well, I think the issue here is that the `hw_ephemeral_encryption_format` image property isn\u0027t guaranteed to be set on the image. It will be if Nova generated the image by way of the createImage API. So my thinking was, if it\u0027s not set on the image, try to use the default from the compute host.\n\nDo you think we should just assume that if `hw_ephemeral_encryption_secret_uuid` is present in the image that `hw_ephemeral_encryption_format` also will be?","commit_id":"31a028d86a90ce86b3cbb5950932e08a812cc465"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"c482c121ced50fab70e11311400d200785dd738b","unresolved":true,"context_lines":[{"line_number":11419,"context_line":"                \u0027hw_ephemeral_encryption_format\u0027)"},{"line_number":11420,"context_line":"            image_encryption \u003d {"},{"line_number":11421,"context_line":"                \u0027secret\u0027: secret,"},{"line_number":11422,"context_line":"                \u0027format\u0027: encryption_format,"},{"line_number":11423,"context_line":"            }"},{"line_number":11424,"context_line":"        try:"},{"line_number":11425,"context_line":"            image.cache(fetch_func\u003dfetch_func,"}],"source_content_type":"text/x-python","patch_set":35,"id":"f95f55ff_df8412cf","line":11422,"updated":"2024-02-29 18:54:12.000000000","message":"Due to this change in assuming/requiring the presence of `hw_ephemeral_encryption_format` if a secret is in the image properties, I think I need to add a validation method to compute/api to fail early if it\u0027s missing.","commit_id":"d59666b9cd2b90a5bf3091f2cc042e8b9e40dc6e"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"7d17c244442d03e56bf24df724de492026acf989","unresolved":false,"context_lines":[{"line_number":11419,"context_line":"                \u0027hw_ephemeral_encryption_format\u0027)"},{"line_number":11420,"context_line":"            image_encryption \u003d {"},{"line_number":11421,"context_line":"                \u0027secret\u0027: secret,"},{"line_number":11422,"context_line":"                \u0027format\u0027: encryption_format,"},{"line_number":11423,"context_line":"            }"},{"line_number":11424,"context_line":"        try:"},{"line_number":11425,"context_line":"            image.cache(fetch_func\u003dfetch_func,"}],"source_content_type":"text/x-python","patch_set":35,"id":"df430b28_f593bf3c","line":11422,"in_reply_to":"f95f55ff_df8412cf","updated":"2024-03-02 01:26:40.000000000","message":"Done","commit_id":"d59666b9cd2b90a5bf3091f2cc042e8b9e40dc6e"}],"nova/virt/libvirt/imagebackend.py":[{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"bd52b7b3a594a50aefbb4c836f44a2821f6e6b21","unresolved":true,"context_lines":[{"line_number":548,"context_line":"    def get_encryption("},{"line_number":549,"context_line":"        self,"},{"line_number":550,"context_line":"        context: \u0027nova.context.RequestContext\u0027,"},{"line_number":551,"context_line":"    ) -\u003e ty.Optional[ty.Dict[str, ty.Any]]:"},{"line_number":552,"context_line":"        \"\"\"Get encryption attributes from the disk_info_mapping."},{"line_number":553,"context_line":""},{"line_number":554,"context_line":"        Checks for encryption attributes in the disk_info_mapping and returns"}],"source_content_type":"text/x-python","patch_set":2,"id":"2de12a2b_0b1f8625","line":551,"range":{"start_line":551,"start_character":21,"end_line":551,"end_character":40},"updated":"2023-01-23 15:41:11.000000000","message":"I\u0027m not sure typing brings lots of value here as it is quite generic.\nOne of the difficulties I have with the storage code is the heavy usage of dicts. So if possible, I would tend to use dedicated objects instead of dicts. And thus, typing will be useful.","commit_id":"63f06ea973170c1b561502f3885c76e8a0828c8c"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"05e2304b0f33af88332840318f0967147fd7b42a","unresolved":true,"context_lines":[{"line_number":548,"context_line":"    def get_encryption("},{"line_number":549,"context_line":"        self,"},{"line_number":550,"context_line":"        context: \u0027nova.context.RequestContext\u0027,"},{"line_number":551,"context_line":"    ) -\u003e ty.Optional[ty.Dict[str, ty.Any]]:"},{"line_number":552,"context_line":"        \"\"\"Get encryption attributes from the disk_info_mapping."},{"line_number":553,"context_line":""},{"line_number":554,"context_line":"        Checks for encryption attributes in the disk_info_mapping and returns"}],"source_content_type":"text/x-python","patch_set":2,"id":"cfb2330d_bca23995","line":551,"range":{"start_line":551,"start_character":21,"end_line":551,"end_character":40},"in_reply_to":"2de12a2b_0b1f8625","updated":"2023-02-07 09:59:41.000000000","message":"that was plaanned then deferred.","commit_id":"63f06ea973170c1b561502f3885c76e8a0828c8c"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"a41597b7a6be0fdd5f8739e5458fa5ed60cb11ac","unresolved":false,"context_lines":[{"line_number":548,"context_line":"    def get_encryption("},{"line_number":549,"context_line":"        self,"},{"line_number":550,"context_line":"        context: \u0027nova.context.RequestContext\u0027,"},{"line_number":551,"context_line":"    ) -\u003e ty.Optional[ty.Dict[str, ty.Any]]:"},{"line_number":552,"context_line":"        \"\"\"Get encryption attributes from the disk_info_mapping."},{"line_number":553,"context_line":""},{"line_number":554,"context_line":"        Checks for encryption attributes in the disk_info_mapping and returns"}],"source_content_type":"text/x-python","patch_set":2,"id":"dbabbbae_56d5608a","line":551,"range":{"start_line":551,"start_character":21,"end_line":551,"end_character":40},"in_reply_to":"cfb2330d_bca23995","updated":"2024-02-13 06:24:58.000000000","message":"Acknowledged","commit_id":"63f06ea973170c1b561502f3885c76e8a0828c8c"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"f5f15b055e774718e4aa704e32e29bc2d9099556","unresolved":true,"context_lines":[{"line_number":702,"context_line":"        # FIXME(lyarwood): Context is provided as a kwarg here thanks to"},{"line_number":703,"context_line":"        # the legacy ephemeral encryption implementation. It should likely"},{"line_number":704,"context_line":"        # be an arg but the required refactor isn\u0027t trivial."},{"line_number":705,"context_line":"        context \u003d kwargs.get(\u0027context\u0027)"},{"line_number":706,"context_line":"        # bdm_encryption contains the encryption attributes for the destination"},{"line_number":707,"context_line":"        # image, if encryption was specified."},{"line_number":708,"context_line":"        bdm_encryption \u003d self.get_encryption(context)"}],"source_content_type":"text/x-python","patch_set":35,"id":"14c8bcb7_7c49f8c2","line":705,"updated":"2024-02-29 22:31:08.000000000","message":"This seems like an unfortunate thing to have to leave in production code. Also, using `get()` here means we\u0027ll silently get `context\u003dNone` if for some reason this doesn\u0027t get called as we expect. That sort of thing worries me because in some other places we might interpret a None context to mean \"use the admin context\" I think (don\u0027t quote me, I\u0027d have to go look for an example). \n\nI\u0027m not sure really why/when that would happen (or what Lee is referencing here exactly), but... I wonder if we should make this a standard dict operation and `try..except KeyError` around it so we can `LOG.error(\u0027Hmm, this shouldn\u0027t happen\u0027)` for obvious forensics, and specifically stop here before we call deeper with a None context? I know you\u0027re just using this to call the `crypto.get...()` and at the moment that\u0027s probably easy to prove will always fail. But if we don\u0027t have context, shouldn\u0027t we fail right here and now with a clear explanation why instead of deep into the key client? Some weird \"can\u0027t concatenate string to NoneType\" from something/something/castellan/foo.py in a customer bug report is going to be a bit of a goose chase, I suspect.","commit_id":"d59666b9cd2b90a5bf3091f2cc042e8b9e40dc6e"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"1d8cca11c484954a058252c77f6f16f74339a62f","unresolved":true,"context_lines":[{"line_number":702,"context_line":"        # FIXME(lyarwood): Context is provided as a kwarg here thanks to"},{"line_number":703,"context_line":"        # the legacy ephemeral encryption implementation. It should likely"},{"line_number":704,"context_line":"        # be an arg but the required refactor isn\u0027t trivial."},{"line_number":705,"context_line":"        context \u003d kwargs.get(\u0027context\u0027)"},{"line_number":706,"context_line":"        # bdm_encryption contains the encryption attributes for the destination"},{"line_number":707,"context_line":"        # image, if encryption was specified."},{"line_number":708,"context_line":"        bdm_encryption \u003d self.get_encryption(context)"}],"source_content_type":"text/x-python","patch_set":35,"id":"ef8fb4f6_3ed32aa9","line":705,"in_reply_to":"14c8bcb7_7c49f8c2","updated":"2024-02-29 23:06:28.000000000","message":"Yeah, I think it would make sense to do that to help with potential debugging. So much stuff in these layers are *args **kwargs opaque things.","commit_id":"d59666b9cd2b90a5bf3091f2cc042e8b9e40dc6e"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"7d17c244442d03e56bf24df724de492026acf989","unresolved":false,"context_lines":[{"line_number":702,"context_line":"        # FIXME(lyarwood): Context is provided as a kwarg here thanks to"},{"line_number":703,"context_line":"        # the legacy ephemeral encryption implementation. It should likely"},{"line_number":704,"context_line":"        # be an arg but the required refactor isn\u0027t trivial."},{"line_number":705,"context_line":"        context \u003d kwargs.get(\u0027context\u0027)"},{"line_number":706,"context_line":"        # bdm_encryption contains the encryption attributes for the destination"},{"line_number":707,"context_line":"        # image, if encryption was specified."},{"line_number":708,"context_line":"        bdm_encryption \u003d self.get_encryption(context)"}],"source_content_type":"text/x-python","patch_set":35,"id":"b5d55e8b_c94b1af3","line":705,"in_reply_to":"75a99fce_820e9879","updated":"2024-03-02 01:26:40.000000000","message":"Done","commit_id":"d59666b9cd2b90a5bf3091f2cc042e8b9e40dc6e"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"e6676f5c11effeffc12f304042ede07c175b53ea","unresolved":true,"context_lines":[{"line_number":702,"context_line":"        # FIXME(lyarwood): Context is provided as a kwarg here thanks to"},{"line_number":703,"context_line":"        # the legacy ephemeral encryption implementation. It should likely"},{"line_number":704,"context_line":"        # be an arg but the required refactor isn\u0027t trivial."},{"line_number":705,"context_line":"        context \u003d kwargs.get(\u0027context\u0027)"},{"line_number":706,"context_line":"        # bdm_encryption contains the encryption attributes for the destination"},{"line_number":707,"context_line":"        # image, if encryption was specified."},{"line_number":708,"context_line":"        bdm_encryption \u003d self.get_encryption(context)"}],"source_content_type":"text/x-python","patch_set":35,"id":"75a99fce_820e9879","line":705,"in_reply_to":"ef8fb4f6_3ed32aa9","updated":"2024-02-29 23:24:19.000000000","message":"Yeah and just thinking from whence the error will erupt in this case specifically will be a red herring about the location of the problem.\n\nWe *probably* won\u0027t have an \"admin key client\" such that None means \"use nova\u0027s token\" in the future, but it would also be a bad day if we ever got there. In general, we\u0027re probably a little loose with our context handling in a lot of places, but being careful about what and when we pass to a somewhat-opaque-to-us client library as \"context for security\" is probably a good idea.","commit_id":"d59666b9cd2b90a5bf3091f2cc042e8b9e40dc6e"}],"releasenotes/notes/support-ephemeral-encryption-4fdead844d9c86b6.yaml":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"a41597b7a6be0fdd5f8739e5458fa5ed60cb11ac","unresolved":true,"context_lines":[{"line_number":7,"context_line":""},{"line_number":8,"context_line":"    The following server actions are currently supported:"},{"line_number":9,"context_line":""},{"line_number":10,"context_line":"    * create/delete"}],"source_content_type":"text/x-yaml","patch_set":28,"id":"786f9fa3_0977ca73","line":10,"updated":"2024-02-13 06:24:58.000000000","message":"nit: technically the following will also work\n\ninterface/volume attach/delete\nstart/stop/reboot(soft/hard)\nconsole log show\nserver diagnostics\n\n\nnone of those actully depend on the encyption feature really other then the start/stop/reboot actions and even then they only depend on it beacuse they need to ensure the secrt is present when booting the vm.","commit_id":"a0bb38491f7b8db23e99d00ae30ff6f4ae703749"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"e62b5419199f6b10c959f9653a0945466a397a59","unresolved":false,"context_lines":[{"line_number":7,"context_line":""},{"line_number":8,"context_line":"    The following server actions are currently supported:"},{"line_number":9,"context_line":""},{"line_number":10,"context_line":"    * create/delete"}],"source_content_type":"text/x-yaml","patch_set":28,"id":"7234c8e9_d6332726","line":10,"in_reply_to":"786f9fa3_0977ca73","updated":"2024-02-14 08:05:56.000000000","message":"Done","commit_id":"a0bb38491f7b8db23e99d00ae30ff6f4ae703749"}]}