)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"d31b96889fbd4a2bea1064142dc76f9e7c852f09","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"edb9bdc8_2fb82f02","updated":"2022-07-20 15:26:46.000000000","message":"Hm, looks like nondeterministic failures... need to investigate.","commit_id":"08bcc01ec7a694167719e1cadeebf1d30fc2d904"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"52f823a1b96f5e9b20f6662ec1af27d25ded69c8","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"35deafd6_60fd0e8a","updated":"2022-07-28 20:11:09.000000000","message":"recheck bug 1940425","commit_id":"f6c3bd766444265375aaad73f6e02a863bb11a3c"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"b6ad282c4a1809ac33a0551bfa58d64a6ceeb21e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"3e55982b_cdb742d7","updated":"2022-07-23 00:55:22.000000000","message":"recheck subport did not become active\n\nTraceback (most recent call last):\n  File \"/opt/stack/new/tempest/tempest/common/utils/__init__.py\", line 89, in wrapper\n    return func(*func_args, **func_kwargs)\n  File \"/opt/stack/new/tempest/tempest/common/utils/__init__.py\", line 70, in wrapper\n    return f(*func_args, **func_kwargs)\n  File \"/opt/stack/new/tempest/tempest/api/compute/admin/test_live_migration.py\", line 286, in test_live_migration_with_trunk\n    self.assertTrue(\n  File \"/usr/lib/python3.8/unittest/case.py\", line 765, in assertTrue\n    raise self.failureException(msg)\nAssertionError: False is not true","commit_id":"f6c3bd766444265375aaad73f6e02a863bb11a3c"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"5433e404b7be3d035c4a060780b168dfd12b0828","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"234b5faa_603971ff","updated":"2022-08-09 16:14:41.000000000","message":"-1 because the typo affects the name of a secret and can be seen by end users. Otherwise this is fine","commit_id":"e070c4b080d69e0472fccb45395d6c70f30f485b"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"e48a51ee54162973092cc62070734b518b224c77","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"e4d181e9_e5797aa8","updated":"2022-08-09 16:11:30.000000000","message":"recheck unrelated timeouts in nova-next","commit_id":"e070c4b080d69e0472fccb45395d6c70f30f485b"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"9f4c000aca9215ca816185d8a7e0f145b8997928","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"a7b2bcda_a554aa1a","updated":"2023-01-13 22:55:51.000000000","message":"I\u0027ve got some updates coming soon, later today likely. It\u0027s been An Experience getting stuff to work locally 😂 ","commit_id":"ff32f9caf48aba700169989c18acc36b7634d8cf"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"ddcb187114401ae69643d313b513395822edfc3e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"64af82d1_8c224d55","updated":"2023-01-19 16:06:22.000000000","message":"Want to test some more things in the series","commit_id":"538d3be2e764d1040a076d225b272bad4fd90420"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"ccd80079613e072df27ab701f70ebdc36b8d9e92","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":12,"id":"43225671_897a5409","updated":"2023-02-07 09:32:04.000000000","message":"+1 for now as im not sure if you are ready for this to be reviewed yet.\nyou has a -w on the previous verions but im not sure if you remvoed that intentrionally or if it was a result of the rebase.","commit_id":"8613ba9dc613b02b1e1a3a682bfbcd495b756209"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"97df3b648546a009bf48e8c417292e2b56ed0a44","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":12,"id":"7b9e329f_b1231c32","in_reply_to":"43225671_897a5409","updated":"2023-02-07 09:48:28.000000000","message":"Thanks for looking.\n\nIt was mostly the rebase. Most of the patches are missing test coverage. And there are a couple of things messed up out of order when I rebased this last time, that will cause failures until later patches in the series.\n\nIgnoring that, if you wanted to skim the patches to see if there\u0027s anything you obviously hate about them, I could change things sooner than later.\n\nI would say put this at the bottom of your priority list for now, given the missing test coverage.","commit_id":"8613ba9dc613b02b1e1a3a682bfbcd495b756209"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"1c2175c1cd7e6ea7ba582757365e21e860f04f5d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":13,"id":"3c17c034_3226e94c","updated":"2023-02-09 13:21:33.000000000","message":"Need some CI runs","commit_id":"9abf1bafe5c9a48055ced871aa9023f1cd6bce5a"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"44eaccd7615dca2495b285b17c54f1300734d13a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":16,"id":"898868ab_88c9ec83","updated":"2023-06-28 13:32:01.000000000","message":"This patch looks good to me.\nNote: I investigated a little because I read that secrets.token_bytes() should be use to generate a secret. But finally discover that secret.token_bytes() is just a simple wrapper over os.urandom() so what is done in this patch is perfectly fine.","commit_id":"31aaaa3e16c86146e2e13618693bd86a4c9eccc2"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"41bb694067ccbb9d4a4715d6c811e08b75cc3f0b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":16,"id":"e688c3ee_35db0f42","in_reply_to":"898868ab_88c9ec83","updated":"2023-06-28 21:05:48.000000000","message":"Oh cool, I didn\u0027t know about that method before. I looked around a little to see if I should change to token_bytes() but oddly the current python docs say that os.urandom is suitable for cryptographic use [1]. Which I guess makes sense given your finding that token_bytes() is a wrapper.\n\nBased on that plus the mention of cryptographic suitability in the \"official\" python docs, I will just leave it.\n\n[1] https://docs.python.org/3/library/os.html#os.urandom","commit_id":"31aaaa3e16c86146e2e13618693bd86a4c9eccc2"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"594f65c577d8f4cc1f8f44184f959d4303bd1574","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":20,"id":"5ed4d344_7c406ba6","updated":"2023-08-08 20:54:04.000000000","message":"Some questions inline. Given that this is security-sensitive stuff, I think it\u0027s probably a good idea to be as careful and diligent as possible when it comes to potentially leaking residue.","commit_id":"b818211ad6670c7b7f857aaa38d089fd2fd5ea27"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"e37da7d7b1b4e7ea928cb3949cdf9828604f61d3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":21,"id":"3ad98776_91efdae5","updated":"2023-11-15 21:06:36.000000000","message":"The cleanup routine fixes look good, thanks for making those changes. Still have a question about the add case though. I\u0027m sure I\u0027m missing something.","commit_id":"6c0b9f002e670e0ef78ca8a9106d3e7ac9d6fb3f"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"bcfbed56b259383c1e833cec0e467d9e88b96f5b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":21,"id":"17f32d65_754d7f5e","in_reply_to":"3ad98776_91efdae5","updated":"2023-11-16 19:54:10.000000000","message":"You\u0027re not missing something, it is I who missed something 😣","commit_id":"6c0b9f002e670e0ef78ca8a9106d3e7ac9d6fb3f"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"c4372e64776e6a918f8fa129451aa43e658c798e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":28,"id":"e53b4dea_e3eace9c","updated":"2024-01-25 12:00:51.000000000","message":"some comments in line but nothing blocking\n\ni think im ok with +2ing this but just want to ensure you see them before i do","commit_id":"d1a37b3f258f4b4539f1f846f0eccc70e027965a"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"02c0235bb54cd638ea389b53f6caabf7920281b4","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":28,"id":"e73f0301_4d81eaf7","in_reply_to":"e53b4dea_e3eace9c","updated":"2024-01-25 18:08:10.000000000","message":"Thanks. I\u0027ll try to improve the commented areas.","commit_id":"d1a37b3f258f4b4539f1f846f0eccc70e027965a"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"72fc77820567554ff7bc85f67ce87584c0a3177c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":30,"id":"13c4cf25_428886dd","updated":"2024-01-31 07:55:47.000000000","message":"recheck the unit test that failed looks unrelated and it passed on other python versions so it looks like test_resize_reschedule_uses_host_lists_3_fails is flaky…","commit_id":"af5f5193cfe0a6fe7687fdd038bf1bc86f061298"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"8858bf8756fcb96938720d642df86c54198882fe","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":30,"id":"70ee4e87_2522bf17","updated":"2024-01-31 08:31:53.000000000","message":"there is a minor test comment issue but that can be fixed in a followup\ni dont really feel like its worth respining just for that. although if you need to for other reasons then please fix it.\n \nmy comments from my last review have been adressed so im happy with this version over all.","commit_id":"af5f5193cfe0a6fe7687fdd038bf1bc86f061298"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"60b15c2d21556b364731c8234e8a08410ff32fe9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":32,"id":"ee10c1da_424df687","updated":"2024-02-06 22:15:07.000000000","message":"recheck https://review.opendev.org/c/openstack/nova/+/908182 has merged","commit_id":"bc58dbd9e62a473bea46b2e44f1d4b4bfcf406d0"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"63009b6f26c18da7f312dce3fd51f0e455e5eac5","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":32,"id":"abadf1d0_ba8b8004","updated":"2024-02-13 05:32:56.000000000","message":"recheck tempest-integrated-compute-rbac-old-defaults timed out","commit_id":"bc58dbd9e62a473bea46b2e44f1d4b4bfcf406d0"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"1edef8da1f42746d002912ee78f21d708a0d0e69","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":36,"id":"d0254a4c_e4b74e40","updated":"2024-02-29 07:39:05.000000000","message":"recheck tempest create_server urllib3.exceptions.ReadTimeoutError: HTTPSConnectionPool(host\u003d\u002710.209.36.152\u0027, port\u003d443): Read timed out. (read timeout\u003d60)","commit_id":"177c184e405f4227b430151b076f0eaa6efff1a7"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"4ecf161680f76a6a5a3c202571406681df8a1b9b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":36,"id":"416ea0ee_e246d98d","updated":"2024-02-28 14:49:37.000000000","message":"recheck two obvious unrelated fails, one in a barbican job with encrypted disks, but was an ssh timeout and likely unrelated also.","commit_id":"177c184e405f4227b430151b076f0eaa6efff1a7"}],"nova/crypto.py":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"5433e404b7be3d035c4a060780b168dfd12b0828","unresolved":true,"context_lines":[{"line_number":268,"context_line":"        os.urandom(_EPHEMERAL_ENCRYPTION_SECRET_BYTE_LENGTH)"},{"line_number":269,"context_line":"    )"},{"line_number":270,"context_line":"    secret_name \u003d ("},{"line_number":271,"context_line":"        f\"Epehemeral encryption secret for instance \""},{"line_number":272,"context_line":"        f\"{instance.uuid} {driver_bdm[\u0027uuid\u0027]}\""},{"line_number":273,"context_line":"    )"},{"line_number":274,"context_line":"    cmo \u003d passphrase.Passphrase(secret, name\u003dsecret_name)"}],"source_content_type":"text/x-python","patch_set":6,"id":"1afe6949_bf892538","line":271,"range":{"start_line":271,"start_character":10,"end_line":271,"end_character":20},"updated":"2022-08-09 16:14:41.000000000","message":"Ephemeral","commit_id":"e070c4b080d69e0472fccb45395d6c70f30f485b"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"eac98154de3c79b552112859d41f1f3087c57201","unresolved":false,"context_lines":[{"line_number":268,"context_line":"        os.urandom(_EPHEMERAL_ENCRYPTION_SECRET_BYTE_LENGTH)"},{"line_number":269,"context_line":"    )"},{"line_number":270,"context_line":"    secret_name \u003d ("},{"line_number":271,"context_line":"        f\"Epehemeral encryption secret for instance \""},{"line_number":272,"context_line":"        f\"{instance.uuid} {driver_bdm[\u0027uuid\u0027]}\""},{"line_number":273,"context_line":"    )"},{"line_number":274,"context_line":"    cmo \u003d passphrase.Passphrase(secret, name\u003dsecret_name)"}],"source_content_type":"text/x-python","patch_set":6,"id":"2fba46fb_7e504241","line":271,"range":{"start_line":271,"start_character":10,"end_line":271,"end_character":20},"in_reply_to":"1afe6949_bf892538","updated":"2022-08-16 09:02:00.000000000","message":"Done","commit_id":"e070c4b080d69e0472fccb45395d6c70f30f485b"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"5433e404b7be3d035c4a060780b168dfd12b0828","unresolved":true,"context_lines":[{"line_number":269,"context_line":"    )"},{"line_number":270,"context_line":"    secret_name \u003d ("},{"line_number":271,"context_line":"        f\"Epehemeral encryption secret for instance \""},{"line_number":272,"context_line":"        f\"{instance.uuid} {driver_bdm[\u0027uuid\u0027]}\""},{"line_number":273,"context_line":"    )"},{"line_number":274,"context_line":"    cmo \u003d passphrase.Passphrase(secret, name\u003dsecret_name)"},{"line_number":275,"context_line":"    key_mgr \u003d _get_key_manager()"}],"source_content_type":"text/x-python","patch_set":6,"id":"fffff97e_3c4e3f56","line":272,"range":{"start_line":272,"start_character":10,"end_line":272,"end_character":46},"updated":"2022-08-09 16:14:41.000000000","message":"nit: Worth single quoting these? Maybe not since they won\u0027t contain spaces. We probably do want a delimiter between the instance UUID and BDM UUID\n\n  f\"\u0027{instance.uuid}\u0027 BDM \u0027{driver_bdm[\u0027uuid\u0027]}\u0027\"","commit_id":"e070c4b080d69e0472fccb45395d6c70f30f485b"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"51fdf974cc358d9bc9e2faf26387d0568d1d1d3d","unresolved":false,"context_lines":[{"line_number":269,"context_line":"    )"},{"line_number":270,"context_line":"    secret_name \u003d ("},{"line_number":271,"context_line":"        f\"Epehemeral encryption secret for instance \""},{"line_number":272,"context_line":"        f\"{instance.uuid} {driver_bdm[\u0027uuid\u0027]}\""},{"line_number":273,"context_line":"    )"},{"line_number":274,"context_line":"    cmo \u003d passphrase.Passphrase(secret, name\u003dsecret_name)"},{"line_number":275,"context_line":"    key_mgr \u003d _get_key_manager()"}],"source_content_type":"text/x-python","patch_set":6,"id":"310a5793_6db41769","line":272,"range":{"start_line":272,"start_character":10,"end_line":272,"end_character":46},"in_reply_to":"60375e27_4fc63cef","updated":"2022-10-18 14:12:37.000000000","message":"\u003e But why would we want the name to have single quotes in it? We don\u0027t do any of our logging that way, for example.\n\u003e \n\u003e Ephemeral encryption secret for instance 974ae3e0-9cdd-4c5e-b7b6-c044c94bff94 BDM ccdf6d0c-3992-44f6-80eb-d8db01548776\n\u003e \n\u003e vs \n\u003e \n\u003e Ephemeral encryption secret for instance \u0027974ae3e0-9cdd-4c5e-b7b6-c044c94bff94\u0027 BDM \u0027ccdf6d0c-3992-44f6-80eb-d8db01548776\u0027\n\nYeah, we don\u0027t actually. I thought we did this somewhat. Ignore me!","commit_id":"e070c4b080d69e0472fccb45395d6c70f30f485b"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"87bee846f1d8b0fed70734aa7895200d58ec207b","unresolved":true,"context_lines":[{"line_number":269,"context_line":"    )"},{"line_number":270,"context_line":"    secret_name \u003d ("},{"line_number":271,"context_line":"        f\"Epehemeral encryption secret for instance \""},{"line_number":272,"context_line":"        f\"{instance.uuid} {driver_bdm[\u0027uuid\u0027]}\""},{"line_number":273,"context_line":"    )"},{"line_number":274,"context_line":"    cmo \u003d passphrase.Passphrase(secret, name\u003dsecret_name)"},{"line_number":275,"context_line":"    key_mgr \u003d _get_key_manager()"}],"source_content_type":"text/x-python","patch_set":6,"id":"ee4aa183_a0e13165","line":272,"range":{"start_line":272,"start_character":10,"end_line":272,"end_character":46},"in_reply_to":"b490eb19_fc712d90","updated":"2022-08-10 15:35:43.000000000","message":"Nope, no issues so long as the quotes are outside the curly braces:\n\n  \u003e\u003e\u003e instance \u003d \"foo\"\n  \u003e\u003e\u003e driver_bdm \u003d {\u0027uuid\u0027: \u0027bar\u0027}\n  \u003e\u003e\u003e print(f\"{instance} {driver_bdm[\u0027uuid\u0027]}\")\n  foo bar\n  \u003e\u003e\u003e print(f\"\u0027{instance}\u0027 \u0027{driver_bdm[\u0027uuid\u0027]}\u0027\")\n  \u0027foo\u0027 \u0027bar\u0027","commit_id":"e070c4b080d69e0472fccb45395d6c70f30f485b"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"eac98154de3c79b552112859d41f1f3087c57201","unresolved":true,"context_lines":[{"line_number":269,"context_line":"    )"},{"line_number":270,"context_line":"    secret_name \u003d ("},{"line_number":271,"context_line":"        f\"Epehemeral encryption secret for instance \""},{"line_number":272,"context_line":"        f\"{instance.uuid} {driver_bdm[\u0027uuid\u0027]}\""},{"line_number":273,"context_line":"    )"},{"line_number":274,"context_line":"    cmo \u003d passphrase.Passphrase(secret, name\u003dsecret_name)"},{"line_number":275,"context_line":"    key_mgr \u003d _get_key_manager()"}],"source_content_type":"text/x-python","patch_set":6,"id":"60375e27_4fc63cef","line":272,"range":{"start_line":272,"start_character":10,"end_line":272,"end_character":46},"in_reply_to":"ee4aa183_a0e13165","updated":"2022-08-16 09:02:00.000000000","message":"But why would we want the name to have single quotes in it? We don\u0027t do any of our logging that way, for example.\n\nEphemeral encryption secret for instance 974ae3e0-9cdd-4c5e-b7b6-c044c94bff94 BDM ccdf6d0c-3992-44f6-80eb-d8db01548776\n\nvs \n\nEphemeral encryption secret for instance \u0027974ae3e0-9cdd-4c5e-b7b6-c044c94bff94\u0027 BDM \u0027ccdf6d0c-3992-44f6-80eb-d8db01548776\u0027","commit_id":"e070c4b080d69e0472fccb45395d6c70f30f485b"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"444e30748096aa271d25167a1f9270ebdef4614d","unresolved":true,"context_lines":[{"line_number":269,"context_line":"    )"},{"line_number":270,"context_line":"    secret_name \u003d ("},{"line_number":271,"context_line":"        f\"Epehemeral encryption secret for instance \""},{"line_number":272,"context_line":"        f\"{instance.uuid} {driver_bdm[\u0027uuid\u0027]}\""},{"line_number":273,"context_line":"    )"},{"line_number":274,"context_line":"    cmo \u003d passphrase.Passphrase(secret, name\u003dsecret_name)"},{"line_number":275,"context_line":"    key_mgr \u003d _get_key_manager()"}],"source_content_type":"text/x-python","patch_set":6,"id":"b490eb19_fc712d90","line":272,"range":{"start_line":272,"start_character":10,"end_line":272,"end_character":46},"in_reply_to":"fffff97e_3c4e3f56","updated":"2022-08-10 09:37:53.000000000","message":"wouldn\u0027t \u0027{driver_bdm[\u0027uuid\u0027]}\u0027 not render?\n\n\nas in it would render as \u0027{driver_bdm[\u0027uuid\u0027]}\u0027 not as the uuid from the dervier_bdm is that intended?","commit_id":"e070c4b080d69e0472fccb45395d6c70f30f485b"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"50f79cb15115b743e66b65a782d71bcc76132dff","unresolved":true,"context_lines":[{"line_number":268,"context_line":"    secret \u003d oslo_base64.encode_as_text("},{"line_number":269,"context_line":"        os.urandom(_EPHEMERAL_ENCRYPTION_SECRET_BYTE_LENGTH)"},{"line_number":270,"context_line":"    )"},{"line_number":271,"context_line":"    # Let the name indicate if the secret is for a snapshot to help with"},{"line_number":272,"context_line":"    # debugging and to add clarity. Snapshots are standalone images in glance"},{"line_number":273,"context_line":"    # that are not directly associated with an instance. Noting whether the"},{"line_number":274,"context_line":"    # secret is for a snapshot lets us know whether the secret is for a"}],"source_content_type":"text/x-python","patch_set":24,"id":"cdfb24af_66f7c38c","line":271,"updated":"2023-12-20 13:32:58.000000000","message":"unlike os.random urandom should not block so ya thats a good choice for entrophy.","commit_id":"8add5a312dba1ba0241230738edff0ca6415713c"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"8858bf8756fcb96938720d642df86c54198882fe","unresolved":false,"context_lines":[{"line_number":268,"context_line":"    secret \u003d oslo_base64.encode_as_text("},{"line_number":269,"context_line":"        os.urandom(_EPHEMERAL_ENCRYPTION_SECRET_BYTE_LENGTH)"},{"line_number":270,"context_line":"    )"},{"line_number":271,"context_line":"    # Let the name indicate if the secret is for a snapshot to help with"},{"line_number":272,"context_line":"    # debugging and to add clarity. Snapshots are standalone images in glance"},{"line_number":273,"context_line":"    # that are not directly associated with an instance. Noting whether the"},{"line_number":274,"context_line":"    # secret is for a snapshot lets us know whether the secret is for a"}],"source_content_type":"text/x-python","patch_set":24,"id":"bc79afb2_aeff7d83","line":271,"in_reply_to":"cdfb24af_66f7c38c","updated":"2024-01-31 08:31:53.000000000","message":"Acknowledged","commit_id":"8add5a312dba1ba0241230738edff0ca6415713c"}],"nova/tests/unit/compute/test_compute.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"c4372e64776e6a918f8fa129451aa43e658c798e","unresolved":true,"context_lines":[{"line_number":8021,"context_line":"            mock_delete_secret.assert_has_calls([call, call])"},{"line_number":8022,"context_line":""},{"line_number":8023,"context_line":"        _test()"},{"line_number":8024,"context_line":""},{"line_number":8025,"context_line":"    def test_terminate_instance_updates_tracker(self):"},{"line_number":8026,"context_line":"        admin_context \u003d context.get_admin_context()"},{"line_number":8027,"context_line":""}],"source_content_type":"text/x-python","patch_set":28,"id":"28f5f847_ba5fa163","line":8024,"updated":"2024-01-25 12:00:51.000000000","message":"just as an aside instead of creating an inner function yuo could have just used a with statement to create the mocks.","commit_id":"d1a37b3f258f4b4539f1f846f0eccc70e027965a"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"c1a07f911be9537ddc68ff5430a1d311ef7f4896","unresolved":false,"context_lines":[{"line_number":8021,"context_line":"            mock_delete_secret.assert_has_calls([call, call])"},{"line_number":8022,"context_line":""},{"line_number":8023,"context_line":"        _test()"},{"line_number":8024,"context_line":""},{"line_number":8025,"context_line":"    def test_terminate_instance_updates_tracker(self):"},{"line_number":8026,"context_line":"        admin_context \u003d context.get_admin_context()"},{"line_number":8027,"context_line":""}],"source_content_type":"text/x-python","patch_set":28,"id":"2eb46d6e_9ee19e6d","line":8024,"in_reply_to":"28f5f847_ba5fa163","updated":"2024-01-26 08:13:52.000000000","message":"Done","commit_id":"d1a37b3f258f4b4539f1f846f0eccc70e027965a"}],"nova/tests/unit/test_crypto.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"c4372e64776e6a918f8fa129451aa43e658c798e","unresolved":true,"context_lines":[{"line_number":324,"context_line":"    @mock.patch(\u0027oslo_serialization.base64.encode_as_text\u0027)"},{"line_number":325,"context_line":"    @mock.patch(\u0027castellan.common.objects.passphrase.Passphrase\u0027)"},{"line_number":326,"context_line":"    @mock.patch.object(crypto, \u0027_get_key_manager\u0027)"},{"line_number":327,"context_line":"    def _test_create_encryption_secret("},{"line_number":328,"context_line":"            self, mock_get_manager, mock_pass, mock_text, for_snapshot\u003dFalse):"},{"line_number":329,"context_line":""},{"line_number":330,"context_line":"        instance \u003d objects.Instance(uuid\u003duuids.instance)"}],"source_content_type":"text/x-python","patch_set":28,"id":"e07543e3_3d73a388","line":327,"updated":"2024-01-25 12:00:51.000000000","message":"here creating a helper function makes more sense to me as its reused","commit_id":"d1a37b3f258f4b4539f1f846f0eccc70e027965a"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"c1a07f911be9537ddc68ff5430a1d311ef7f4896","unresolved":false,"context_lines":[{"line_number":324,"context_line":"    @mock.patch(\u0027oslo_serialization.base64.encode_as_text\u0027)"},{"line_number":325,"context_line":"    @mock.patch(\u0027castellan.common.objects.passphrase.Passphrase\u0027)"},{"line_number":326,"context_line":"    @mock.patch.object(crypto, \u0027_get_key_manager\u0027)"},{"line_number":327,"context_line":"    def _test_create_encryption_secret("},{"line_number":328,"context_line":"            self, mock_get_manager, mock_pass, mock_text, for_snapshot\u003dFalse):"},{"line_number":329,"context_line":""},{"line_number":330,"context_line":"        instance \u003d objects.Instance(uuid\u003duuids.instance)"}],"source_content_type":"text/x-python","patch_set":28,"id":"cdca6de2_10c8d5a8","line":327,"in_reply_to":"e07543e3_3d73a388","updated":"2024-01-26 08:13:52.000000000","message":"Acknowledged","commit_id":"d1a37b3f258f4b4539f1f846f0eccc70e027965a"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"8858bf8756fcb96938720d642df86c54198882fe","unresolved":false,"context_lines":[{"line_number":315,"context_line":"        )"},{"line_number":316,"context_line":""},{"line_number":317,"context_line":"    @mock.patch.object(crypto, \u0027_get_key_manager\u0027)"},{"line_number":318,"context_line":"    def test_delete_vtpm_secret_error(self, mock_get_manager):"},{"line_number":319,"context_line":"        \"\"\"Check behavior when we fail to retrieve the secret via castellan."},{"line_number":320,"context_line":""},{"line_number":321,"context_line":"        We should carry on and delete the reference from the instance."}],"source_content_type":"text/x-python","patch_set":30,"id":"1f315944_99302c53","side":"PARENT","line":318,"updated":"2024-01-31 08:31:53.000000000","message":"i guess theses were never really about vtpm but ranter secrete management so this makes sense to rename.","commit_id":"94c9c187f7b998d87b2fa3f58b7977398f7e6f23"}],"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":"ccd80079613e072df27ab701f70ebdc36b8d9e92","unresolved":true,"context_lines":[{"line_number":15869,"context_line":"            encryption_secret_uuid\u003dencryption_secret_uuid,"},{"line_number":15870,"context_line":"        )"},{"line_number":15871,"context_line":"        ephemerals \u003d [driver_block_device.DriverEphemeralBlockDevice(bdm)]"},{"line_number":15872,"context_line":"        block_device_info \u003d {\u0027ephemerals\u0027: ephemerals}"},{"line_number":15873,"context_line":""},{"line_number":15874,"context_line":"        #  Call spawn() with encrypted ephemeral block device."},{"line_number":15875,"context_line":"        drvr.spawn("}],"source_content_type":"text/x-python","patch_set":12,"id":"5bc1fa26_98628f1b","line":15872,"range":{"start_line":15872,"start_character":28,"end_line":15872,"end_character":54},"updated":"2023-02-07 09:32:04.000000000","message":"it would be nice to see testing of root and swap encyption too.\nthe ephemeral disks are part of the scope but so are the root and swap disks","commit_id":"8613ba9dc613b02b1e1a3a682bfbcd495b756209"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"bcfbed56b259383c1e833cec0e467d9e88b96f5b","unresolved":false,"context_lines":[{"line_number":15869,"context_line":"            encryption_secret_uuid\u003dencryption_secret_uuid,"},{"line_number":15870,"context_line":"        )"},{"line_number":15871,"context_line":"        ephemerals \u003d [driver_block_device.DriverEphemeralBlockDevice(bdm)]"},{"line_number":15872,"context_line":"        block_device_info \u003d {\u0027ephemerals\u0027: ephemerals}"},{"line_number":15873,"context_line":""},{"line_number":15874,"context_line":"        #  Call spawn() with encrypted ephemeral block device."},{"line_number":15875,"context_line":"        drvr.spawn("}],"source_content_type":"text/x-python","patch_set":12,"id":"006746bb_4a6c8096","line":15872,"range":{"start_line":15872,"start_character":28,"end_line":15872,"end_character":54},"in_reply_to":"261dd899_1c7afd58","updated":"2023-11-16 19:54:10.000000000","message":"Done","commit_id":"8613ba9dc613b02b1e1a3a682bfbcd495b756209"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"97df3b648546a009bf48e8c417292e2b56ed0a44","unresolved":true,"context_lines":[{"line_number":15869,"context_line":"            encryption_secret_uuid\u003dencryption_secret_uuid,"},{"line_number":15870,"context_line":"        )"},{"line_number":15871,"context_line":"        ephemerals \u003d [driver_block_device.DriverEphemeralBlockDevice(bdm)]"},{"line_number":15872,"context_line":"        block_device_info \u003d {\u0027ephemerals\u0027: ephemerals}"},{"line_number":15873,"context_line":""},{"line_number":15874,"context_line":"        #  Call spawn() with encrypted ephemeral block device."},{"line_number":15875,"context_line":"        drvr.spawn("}],"source_content_type":"text/x-python","patch_set":12,"id":"261dd899_1c7afd58","line":15872,"range":{"start_line":15872,"start_character":28,"end_line":15872,"end_character":54},"in_reply_to":"5bc1fa26_98628f1b","updated":"2023-02-07 09:48:28.000000000","message":"Ack, I will add that.","commit_id":"8613ba9dc613b02b1e1a3a682bfbcd495b756209"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"c4372e64776e6a918f8fa129451aa43e658c798e","unresolved":true,"context_lines":[{"line_number":15931,"context_line":"        mock_get_vtpm.assert_called_once_with(instance.flavor, image_meta)"},{"line_number":15932,"context_line":"        mock_ensure_vtpm.assert_called_once_with(self.context, instance)"},{"line_number":15933,"context_line":""},{"line_number":15934,"context_line":"    @mock.patch(\u0027nova.virt.block_device.DriverBlockDevice.save\u0027)"},{"line_number":15935,"context_line":"    @mock.patch(\u0027nova.crypto.delete_encryption_secret\u0027)"},{"line_number":15936,"context_line":"    @mock.patch(\u0027nova.crypto.create_encryption_secret\u0027)"},{"line_number":15937,"context_line":"    @mock.patch(\u0027nova.crypto.get_encryption_secret\u0027)"}],"source_content_type":"text/x-python","patch_set":28,"id":"4b734edc_b5a130a3","line":15934,"updated":"2024-01-25 12:00:51.000000000","message":"as a meta comment i think i would prefer in genreal if you pulled the encyption tests into there own class","commit_id":"d1a37b3f258f4b4539f1f846f0eccc70e027965a"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"02c0235bb54cd638ea389b53f6caabf7920281b4","unresolved":true,"context_lines":[{"line_number":15931,"context_line":"        mock_get_vtpm.assert_called_once_with(instance.flavor, image_meta)"},{"line_number":15932,"context_line":"        mock_ensure_vtpm.assert_called_once_with(self.context, instance)"},{"line_number":15933,"context_line":""},{"line_number":15934,"context_line":"    @mock.patch(\u0027nova.virt.block_device.DriverBlockDevice.save\u0027)"},{"line_number":15935,"context_line":"    @mock.patch(\u0027nova.crypto.delete_encryption_secret\u0027)"},{"line_number":15936,"context_line":"    @mock.patch(\u0027nova.crypto.create_encryption_secret\u0027)"},{"line_number":15937,"context_line":"    @mock.patch(\u0027nova.crypto.get_encryption_secret\u0027)"}],"source_content_type":"text/x-python","patch_set":28,"id":"833d28bb_f43c9acc","line":15934,"in_reply_to":"4b734edc_b5a130a3","updated":"2024-01-25 18:08:10.000000000","message":"Hm, OK. I\u0027m not opposed ... was mostly following what I perceived to be the existing pattern in this file (for example: vtpm).","commit_id":"d1a37b3f258f4b4539f1f846f0eccc70e027965a"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"8858bf8756fcb96938720d642df86c54198882fe","unresolved":false,"context_lines":[{"line_number":15931,"context_line":"        mock_get_vtpm.assert_called_once_with(instance.flavor, image_meta)"},{"line_number":15932,"context_line":"        mock_ensure_vtpm.assert_called_once_with(self.context, instance)"},{"line_number":15933,"context_line":""},{"line_number":15934,"context_line":"    @mock.patch(\u0027nova.virt.block_device.DriverBlockDevice.save\u0027)"},{"line_number":15935,"context_line":"    @mock.patch(\u0027nova.crypto.delete_encryption_secret\u0027)"},{"line_number":15936,"context_line":"    @mock.patch(\u0027nova.crypto.create_encryption_secret\u0027)"},{"line_number":15937,"context_line":"    @mock.patch(\u0027nova.crypto.get_encryption_secret\u0027)"}],"source_content_type":"text/x-python","patch_set":28,"id":"f8fbc808_e418ca0d","line":15934,"in_reply_to":"833d28bb_f43c9acc","updated":"2024-01-31 08:31:53.000000000","message":"Done","commit_id":"d1a37b3f258f4b4539f1f846f0eccc70e027965a"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"c4372e64776e6a918f8fa129451aa43e658c798e","unresolved":true,"context_lines":[{"line_number":15952,"context_line":"        secret_not_found\u003dFalse, secret_create_fail\u003dFalse,"},{"line_number":15953,"context_line":"    ):"},{"line_number":15954,"context_line":"        self.useFixture(nova_fixtures.LibvirtImageBackendFixture())"},{"line_number":15955,"context_line":"        mock_get_info.return_value \u003d hardware.InstanceInfo("},{"line_number":15956,"context_line":"            state\u003dpower_state.RUNNING)"},{"line_number":15957,"context_line":""},{"line_number":15958,"context_line":"        if not secret_create_fail:"}],"source_content_type":"text/x-python","patch_set":28,"id":"c4a65201_ac2b60d4","line":15955,"updated":"2024-01-25 12:00:51.000000000","message":"then you can move the fixture setup to the class setup.\n\nits not wrong to do it here its just normally done in the class setup function.","commit_id":"d1a37b3f258f4b4539f1f846f0eccc70e027965a"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"8858bf8756fcb96938720d642df86c54198882fe","unresolved":false,"context_lines":[{"line_number":15952,"context_line":"        secret_not_found\u003dFalse, secret_create_fail\u003dFalse,"},{"line_number":15953,"context_line":"    ):"},{"line_number":15954,"context_line":"        self.useFixture(nova_fixtures.LibvirtImageBackendFixture())"},{"line_number":15955,"context_line":"        mock_get_info.return_value \u003d hardware.InstanceInfo("},{"line_number":15956,"context_line":"            state\u003dpower_state.RUNNING)"},{"line_number":15957,"context_line":""},{"line_number":15958,"context_line":"        if not secret_create_fail:"}],"source_content_type":"text/x-python","patch_set":28,"id":"c446b1cc_46294b48","line":15955,"in_reply_to":"c4a65201_ac2b60d4","updated":"2024-01-31 08:31:53.000000000","message":"Done","commit_id":"d1a37b3f258f4b4539f1f846f0eccc70e027965a"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"c4372e64776e6a918f8fa129451aa43e658c798e","unresolved":true,"context_lines":[{"line_number":16050,"context_line":"            self.assertEqual("},{"line_number":16051,"context_line":"                [img_call, eph_call, swap_call], mock_create_secret.mock_calls)"},{"line_number":16052,"context_line":""},{"line_number":16053,"context_line":"            # Assert that we generated libvirt secrets."},{"line_number":16054,"context_line":"            if secret_create_fail:"},{"line_number":16055,"context_line":"                expected_libvirt_secret_calls.pop()"},{"line_number":16056,"context_line":""}],"source_content_type":"text/x-python","patch_set":28,"id":"d2fa30eb_69198a1b","line":16053,"updated":"2024-01-25 12:00:51.000000000","message":"nit this comment should be on lon 16056","commit_id":"d1a37b3f258f4b4539f1f846f0eccc70e027965a"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"8858bf8756fcb96938720d642df86c54198882fe","unresolved":false,"context_lines":[{"line_number":16050,"context_line":"            self.assertEqual("},{"line_number":16051,"context_line":"                [img_call, eph_call, swap_call], mock_create_secret.mock_calls)"},{"line_number":16052,"context_line":""},{"line_number":16053,"context_line":"            # Assert that we generated libvirt secrets."},{"line_number":16054,"context_line":"            if secret_create_fail:"},{"line_number":16055,"context_line":"                expected_libvirt_secret_calls.pop()"},{"line_number":16056,"context_line":""}],"source_content_type":"text/x-python","patch_set":28,"id":"4110efcf_b45cd9fc","line":16053,"in_reply_to":"d2fa30eb_69198a1b","updated":"2024-01-31 08:31:53.000000000","message":"Acknowledged","commit_id":"d1a37b3f258f4b4539f1f846f0eccc70e027965a"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"c4372e64776e6a918f8fa129451aa43e658c798e","unresolved":true,"context_lines":[{"line_number":16068,"context_line":"            call \u003d mock.call(self.context, encryption_secret_uuid)"},{"line_number":16069,"context_line":"            self.assertEqual([call, call, call], mock_get_secret.mock_calls)"},{"line_number":16070,"context_line":""},{"line_number":16071,"context_line":"            # Assert that we created libvirt secrets if needed."},{"line_number":16072,"context_line":"            if secret_not_found:"},{"line_number":16073,"context_line":"                expected_libvirt_secret_calls.pop()"},{"line_number":16074,"context_line":""}],"source_content_type":"text/x-python","patch_set":28,"id":"f38cc9f0_33fc1e59","line":16071,"updated":"2024-01-25 12:00:51.000000000","message":"dito: 16074","commit_id":"d1a37b3f258f4b4539f1f846f0eccc70e027965a"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"8858bf8756fcb96938720d642df86c54198882fe","unresolved":false,"context_lines":[{"line_number":16068,"context_line":"            call \u003d mock.call(self.context, encryption_secret_uuid)"},{"line_number":16069,"context_line":"            self.assertEqual([call, call, call], mock_get_secret.mock_calls)"},{"line_number":16070,"context_line":""},{"line_number":16071,"context_line":"            # Assert that we created libvirt secrets if needed."},{"line_number":16072,"context_line":"            if secret_not_found:"},{"line_number":16073,"context_line":"                expected_libvirt_secret_calls.pop()"},{"line_number":16074,"context_line":""}],"source_content_type":"text/x-python","patch_set":28,"id":"49cc6eef_c6d237b2","line":16071,"in_reply_to":"f38cc9f0_33fc1e59","updated":"2024-01-31 08:31:53.000000000","message":"Acknowledged","commit_id":"d1a37b3f258f4b4539f1f846f0eccc70e027965a"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"c4372e64776e6a918f8fa129451aa43e658c798e","unresolved":true,"context_lines":[{"line_number":16089,"context_line":"                new_block_device_info \u003d new_block_device_info[0]"},{"line_number":16090,"context_line":"            self.assertEqual("},{"line_number":16091,"context_line":"                expected_format,"},{"line_number":16092,"context_line":"                new_block_device_info[\u0027encryption_format\u0027])"},{"line_number":16093,"context_line":"            if not secret_create_fail:"},{"line_number":16094,"context_line":"                self.assertEqual("},{"line_number":16095,"context_line":"                    expected_secret_uuid,"}],"source_content_type":"text/x-python","patch_set":28,"id":"a38bb80b_154bf8f7","line":16092,"updated":"2024-01-25 12:00:51.000000000","message":"we could do a for over new_block_devices and assert the format for all ephemerals\nthat siad they will all be the same so checking the first in the list is also fine i gues","commit_id":"d1a37b3f258f4b4539f1f846f0eccc70e027965a"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"8858bf8756fcb96938720d642df86c54198882fe","unresolved":false,"context_lines":[{"line_number":16089,"context_line":"                new_block_device_info \u003d new_block_device_info[0]"},{"line_number":16090,"context_line":"            self.assertEqual("},{"line_number":16091,"context_line":"                expected_format,"},{"line_number":16092,"context_line":"                new_block_device_info[\u0027encryption_format\u0027])"},{"line_number":16093,"context_line":"            if not secret_create_fail:"},{"line_number":16094,"context_line":"                self.assertEqual("},{"line_number":16095,"context_line":"                    expected_secret_uuid,"}],"source_content_type":"text/x-python","patch_set":28,"id":"2141b2d8_f614b3bd","line":16092,"in_reply_to":"a38bb80b_154bf8f7","updated":"2024-01-31 08:31:53.000000000","message":"Acknowledged","commit_id":"d1a37b3f258f4b4539f1f846f0eccc70e027965a"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"c4372e64776e6a918f8fa129451aa43e658c798e","unresolved":true,"context_lines":[{"line_number":16124,"context_line":"        else:"},{"line_number":16125,"context_line":"            mock_delete_secret.assert_not_called()"},{"line_number":16126,"context_line":"            drvr._host.delete_secret.assert_not_called()"},{"line_number":16127,"context_line":""},{"line_number":16128,"context_line":"    def test_spawn_with_ephemeral_encryption_defaults(self):"},{"line_number":16129,"context_line":"        # Test that encryption defaults are set during spawn() if not"},{"line_number":16130,"context_line":"        # specified."}],"source_content_type":"text/x-python","patch_set":28,"id":"d72aabf4_84092a55","line":16127,"updated":"2024-01-25 12:00:51.000000000","message":"over all this has a good level of test coverate but this test funciton is doing a lot. i would personally have broken it up but i dont think you need to respin because of it.\n\n\nit very hard to keep all the code paths you could take through this funciton in memroy espcially when the function signiture is off the screen most of the time so i just gave up half way.","commit_id":"d1a37b3f258f4b4539f1f846f0eccc70e027965a"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"8858bf8756fcb96938720d642df86c54198882fe","unresolved":false,"context_lines":[{"line_number":16124,"context_line":"        else:"},{"line_number":16125,"context_line":"            mock_delete_secret.assert_not_called()"},{"line_number":16126,"context_line":"            drvr._host.delete_secret.assert_not_called()"},{"line_number":16127,"context_line":""},{"line_number":16128,"context_line":"    def test_spawn_with_ephemeral_encryption_defaults(self):"},{"line_number":16129,"context_line":"        # Test that encryption defaults are set during spawn() if not"},{"line_number":16130,"context_line":"        # specified."}],"source_content_type":"text/x-python","patch_set":28,"id":"9e1e18d0_75a3ef9c","line":16127,"in_reply_to":"48a43402_4c215923","updated":"2024-01-31 08:31:53.000000000","message":"Acknowledged","commit_id":"d1a37b3f258f4b4539f1f846f0eccc70e027965a"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"02c0235bb54cd638ea389b53f6caabf7920281b4","unresolved":true,"context_lines":[{"line_number":16124,"context_line":"        else:"},{"line_number":16125,"context_line":"            mock_delete_secret.assert_not_called()"},{"line_number":16126,"context_line":"            drvr._host.delete_secret.assert_not_called()"},{"line_number":16127,"context_line":""},{"line_number":16128,"context_line":"    def test_spawn_with_ephemeral_encryption_defaults(self):"},{"line_number":16129,"context_line":"        # Test that encryption defaults are set during spawn() if not"},{"line_number":16130,"context_line":"        # specified."}],"source_content_type":"text/x-python","patch_set":28,"id":"48a43402_4c215923","line":16127,"in_reply_to":"d72aabf4_84092a55","updated":"2024-01-25 18:08:10.000000000","message":"OK, I\u0027ll try to improve it.","commit_id":"d1a37b3f258f4b4539f1f846f0eccc70e027965a"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"c4372e64776e6a918f8fa129451aa43e658c798e","unresolved":true,"context_lines":[{"line_number":16133,"context_line":"    def test_spawn_with_ephemeral_encryption(self):"},{"line_number":16134,"context_line":"        # Test the rebuild/shelve scenario where we have existing secrets."},{"line_number":16135,"context_line":"        self._test_spawn_with_ephemeral_encryption("},{"line_number":16136,"context_line":"            encryption_format\u003d\u0027luks\u0027,"},{"line_number":16137,"context_line":"            encryption_secret_uuid\u003duuids.secret)"},{"line_number":16138,"context_line":""},{"line_number":16139,"context_line":"    def test_spawn_with_ephemeral_encryption_secret_not_found(self):"}],"source_content_type":"text/x-python","patch_set":28,"id":"6ab9a9e6_9771fa00","line":16136,"updated":"2024-01-25 12:00:51.000000000","message":"isn’t luks the default\nshould this be plain?","commit_id":"d1a37b3f258f4b4539f1f846f0eccc70e027965a"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"8858bf8756fcb96938720d642df86c54198882fe","unresolved":false,"context_lines":[{"line_number":16133,"context_line":"    def test_spawn_with_ephemeral_encryption(self):"},{"line_number":16134,"context_line":"        # Test the rebuild/shelve scenario where we have existing secrets."},{"line_number":16135,"context_line":"        self._test_spawn_with_ephemeral_encryption("},{"line_number":16136,"context_line":"            encryption_format\u003d\u0027luks\u0027,"},{"line_number":16137,"context_line":"            encryption_secret_uuid\u003duuids.secret)"},{"line_number":16138,"context_line":""},{"line_number":16139,"context_line":"    def test_spawn_with_ephemeral_encryption_secret_not_found(self):"}],"source_content_type":"text/x-python","patch_set":28,"id":"5b2fc497_e6c411b5","line":16136,"in_reply_to":"5141676b_0243e227","updated":"2024-01-31 08:31:53.000000000","message":"Acknowledged","commit_id":"d1a37b3f258f4b4539f1f846f0eccc70e027965a"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"02c0235bb54cd638ea389b53f6caabf7920281b4","unresolved":true,"context_lines":[{"line_number":16133,"context_line":"    def test_spawn_with_ephemeral_encryption(self):"},{"line_number":16134,"context_line":"        # Test the rebuild/shelve scenario where we have existing secrets."},{"line_number":16135,"context_line":"        self._test_spawn_with_ephemeral_encryption("},{"line_number":16136,"context_line":"            encryption_format\u003d\u0027luks\u0027,"},{"line_number":16137,"context_line":"            encryption_secret_uuid\u003duuids.secret)"},{"line_number":16138,"context_line":""},{"line_number":16139,"context_line":"    def test_spawn_with_ephemeral_encryption_secret_not_found(self):"}],"source_content_type":"text/x-python","patch_set":28,"id":"5141676b_0243e227","line":16136,"in_reply_to":"6ab9a9e6_9771fa00","updated":"2024-01-25 18:08:10.000000000","message":"It is the default. Not sure why I did this, maybe because plain really isn\u0027t used in the implementation (yet). I\u0027ll change this.","commit_id":"d1a37b3f258f4b4539f1f846f0eccc70e027965a"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"c4372e64776e6a918f8fa129451aa43e658c798e","unresolved":true,"context_lines":[{"line_number":20966,"context_line":"        bdi \u003d {\u0027image\u0027: [driver_block_device.DriverImageBlockDevice(bdm)]}"},{"line_number":20967,"context_line":"        # Pass clean_instance_dir\u003dFalse + clean_instance_disks\u003dTrue"},{"line_number":20968,"context_line":"        for images_type in (\u0027raw\u0027, \u0027flat\u0027, \u0027qcow2\u0027, \u0027lvm\u0027, \u0027rbd\u0027,"},{"line_number":20969,"context_line":"                            \u0027ploop\u0027, \u0027default\u0027):"},{"line_number":20970,"context_line":"            self.flags(images_type\u003dimages_type, group\u003d\u0027libvirt\u0027)"},{"line_number":20971,"context_line":"            drvr \u003d libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)"},{"line_number":20972,"context_line":"            drvr._cleanup("}],"source_content_type":"text/x-python","patch_set":28,"id":"6d5e2e64_737d2a7e","line":20969,"updated":"2024-01-25 12:00:51.000000000","message":"oh your looping here.\nyou get slightly better test messages if you use ddt to generate multiple tests but ok that works too","commit_id":"d1a37b3f258f4b4539f1f846f0eccc70e027965a"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"8858bf8756fcb96938720d642df86c54198882fe","unresolved":false,"context_lines":[{"line_number":20966,"context_line":"        bdi \u003d {\u0027image\u0027: [driver_block_device.DriverImageBlockDevice(bdm)]}"},{"line_number":20967,"context_line":"        # Pass clean_instance_dir\u003dFalse + clean_instance_disks\u003dTrue"},{"line_number":20968,"context_line":"        for images_type in (\u0027raw\u0027, \u0027flat\u0027, \u0027qcow2\u0027, \u0027lvm\u0027, \u0027rbd\u0027,"},{"line_number":20969,"context_line":"                            \u0027ploop\u0027, \u0027default\u0027):"},{"line_number":20970,"context_line":"            self.flags(images_type\u003dimages_type, group\u003d\u0027libvirt\u0027)"},{"line_number":20971,"context_line":"            drvr \u003d libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)"},{"line_number":20972,"context_line":"            drvr._cleanup("}],"source_content_type":"text/x-python","patch_set":28,"id":"f301208a_a565dd72","line":20969,"in_reply_to":"5137fe93_723b9501","updated":"2024-01-31 08:31:53.000000000","message":"Done","commit_id":"d1a37b3f258f4b4539f1f846f0eccc70e027965a"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"02c0235bb54cd638ea389b53f6caabf7920281b4","unresolved":true,"context_lines":[{"line_number":20966,"context_line":"        bdi \u003d {\u0027image\u0027: [driver_block_device.DriverImageBlockDevice(bdm)]}"},{"line_number":20967,"context_line":"        # Pass clean_instance_dir\u003dFalse + clean_instance_disks\u003dTrue"},{"line_number":20968,"context_line":"        for images_type in (\u0027raw\u0027, \u0027flat\u0027, \u0027qcow2\u0027, \u0027lvm\u0027, \u0027rbd\u0027,"},{"line_number":20969,"context_line":"                            \u0027ploop\u0027, \u0027default\u0027):"},{"line_number":20970,"context_line":"            self.flags(images_type\u003dimages_type, group\u003d\u0027libvirt\u0027)"},{"line_number":20971,"context_line":"            drvr \u003d libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)"},{"line_number":20972,"context_line":"            drvr._cleanup("}],"source_content_type":"text/x-python","patch_set":28,"id":"5137fe93_723b9501","line":20969,"in_reply_to":"6d5e2e64_737d2a7e","updated":"2024-01-25 18:08:10.000000000","message":"I\u0027ll try and see if I can do that.","commit_id":"d1a37b3f258f4b4539f1f846f0eccc70e027965a"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"8858bf8756fcb96938720d642df86c54198882fe","unresolved":true,"context_lines":[{"line_number":30098,"context_line":""},{"line_number":30099,"context_line":"    def test_spawn_with_ephemeral_encryption(self):"},{"line_number":30100,"context_line":"        # Test the rebuild/shelve scenario where we have existing secrets."},{"line_number":30101,"context_line":"        self._test_spawn_with_ephemeral_encryption(encryption_format\u003d\u0027plain\u0027)"},{"line_number":30102,"context_line":""},{"line_number":30103,"context_line":"    def test_spawn_with_ephemeral_encryption_secret_not_found(self):"},{"line_number":30104,"context_line":"        # Test that we fail if any existing key manager secret is not found."}],"source_content_type":"text/x-python","patch_set":30,"id":"9821b3dc_9275da5e","line":30101,"updated":"2024-01-31 08:31:53.000000000","message":"am no… this just tests spawn with a non default value\ni think shelve and rebuild are tested in a later patch so this is proably a rebase artifact","commit_id":"af5f5193cfe0a6fe7687fdd038bf1bc86f061298"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"63009b6f26c18da7f312dce3fd51f0e455e5eac5","unresolved":false,"context_lines":[{"line_number":30098,"context_line":""},{"line_number":30099,"context_line":"    def test_spawn_with_ephemeral_encryption(self):"},{"line_number":30100,"context_line":"        # Test the rebuild/shelve scenario where we have existing secrets."},{"line_number":30101,"context_line":"        self._test_spawn_with_ephemeral_encryption(encryption_format\u003d\u0027plain\u0027)"},{"line_number":30102,"context_line":""},{"line_number":30103,"context_line":"    def test_spawn_with_ephemeral_encryption_secret_not_found(self):"},{"line_number":30104,"context_line":"        # Test that we fail if any existing key manager secret is not found."}],"source_content_type":"text/x-python","patch_set":30,"id":"820ac4d5_a2abbbcd","line":30101,"in_reply_to":"223223d8_4e86efba","updated":"2024-02-13 05:32:56.000000000","message":"Done","commit_id":"af5f5193cfe0a6fe7687fdd038bf1bc86f061298"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"3c3e3dd13366c83a5bea8eab0839cb3a3608cc33","unresolved":true,"context_lines":[{"line_number":30098,"context_line":""},{"line_number":30099,"context_line":"    def test_spawn_with_ephemeral_encryption(self):"},{"line_number":30100,"context_line":"        # Test the rebuild/shelve scenario where we have existing secrets."},{"line_number":30101,"context_line":"        self._test_spawn_with_ephemeral_encryption(encryption_format\u003d\u0027plain\u0027)"},{"line_number":30102,"context_line":""},{"line_number":30103,"context_line":"    def test_spawn_with_ephemeral_encryption_secret_not_found(self):"},{"line_number":30104,"context_line":"        # Test that we fail if any existing key manager secret is not found."}],"source_content_type":"text/x-python","patch_set":30,"id":"223223d8_4e86efba","line":30101,"in_reply_to":"9821b3dc_9275da5e","updated":"2024-02-01 02:01:39.000000000","message":"Oh ugh, yeah that is rebase damage. Will fix.","commit_id":"af5f5193cfe0a6fe7687fdd038bf1bc86f061298"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"8858bf8756fcb96938720d642df86c54198882fe","unresolved":true,"context_lines":[{"line_number":30112,"context_line":"        # If we\u0027re trying to retrieve secrets from the key manager it\u0027s because"},{"line_number":30113,"context_line":"        # the BDMs have existing secret UUIDs."},{"line_number":30114,"context_line":"        for bdm in (self.img_bdm, self.eph_bdm, self.swap_bdm):"},{"line_number":30115,"context_line":"            bdm.encryption_secret_uuid \u003d uuids.secret"},{"line_number":30116,"context_line":""},{"line_number":30117,"context_line":"        block_device_info \u003d driver.get_block_device_info("},{"line_number":30118,"context_line":"            self.instance, [self.img_bdm, self.eph_bdm, self.swap_bdm])"}],"source_content_type":"text/x-python","patch_set":30,"id":"b0aa64e1_179d1a77","line":30115,"updated":"2024-01-31 08:31:53.000000000","message":"would it be more realistic to use differnt uuids per disk here.\n\nthis is fine but just wondering if we could miss a bug as a result fo using the same uuid in all cases.\n\nyou have kind of made this work via the side effects you have defiend \n i.e. [mock.sentinel.secret, mock.sentinel.secret, None]\n \n so i guess thats fine, just something i was wondering about when reading this.","commit_id":"af5f5193cfe0a6fe7687fdd038bf1bc86f061298"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"63009b6f26c18da7f312dce3fd51f0e455e5eac5","unresolved":false,"context_lines":[{"line_number":30112,"context_line":"        # If we\u0027re trying to retrieve secrets from the key manager it\u0027s because"},{"line_number":30113,"context_line":"        # the BDMs have existing secret UUIDs."},{"line_number":30114,"context_line":"        for bdm in (self.img_bdm, self.eph_bdm, self.swap_bdm):"},{"line_number":30115,"context_line":"            bdm.encryption_secret_uuid \u003d uuids.secret"},{"line_number":30116,"context_line":""},{"line_number":30117,"context_line":"        block_device_info \u003d driver.get_block_device_info("},{"line_number":30118,"context_line":"            self.instance, [self.img_bdm, self.eph_bdm, self.swap_bdm])"}],"source_content_type":"text/x-python","patch_set":30,"id":"55f868fa_17308294","line":30115,"in_reply_to":"1a0caaa5_3f7a7cba","updated":"2024-02-13 05:32:56.000000000","message":"Acknowledged","commit_id":"af5f5193cfe0a6fe7687fdd038bf1bc86f061298"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"3c3e3dd13366c83a5bea8eab0839cb3a3608cc33","unresolved":true,"context_lines":[{"line_number":30112,"context_line":"        # If we\u0027re trying to retrieve secrets from the key manager it\u0027s because"},{"line_number":30113,"context_line":"        # the BDMs have existing secret UUIDs."},{"line_number":30114,"context_line":"        for bdm in (self.img_bdm, self.eph_bdm, self.swap_bdm):"},{"line_number":30115,"context_line":"            bdm.encryption_secret_uuid \u003d uuids.secret"},{"line_number":30116,"context_line":""},{"line_number":30117,"context_line":"        block_device_info \u003d driver.get_block_device_info("},{"line_number":30118,"context_line":"            self.instance, [self.img_bdm, self.eph_bdm, self.swap_bdm])"}],"source_content_type":"text/x-python","patch_set":30,"id":"1a0caaa5_3f7a7cba","line":30115,"in_reply_to":"b0aa64e1_179d1a77","updated":"2024-02-01 02:01:39.000000000","message":"It would be more realistic yes. I had been thinking the uuids weren\u0027t really the point of this test, I just wanted it to behave like one was not found. But since you found it noticeable, that\u0027s a sign not to reuse the uuids anyway. I\u0027ll update it.","commit_id":"af5f5193cfe0a6fe7687fdd038bf1bc86f061298"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"8858bf8756fcb96938720d642df86c54198882fe","unresolved":false,"context_lines":[{"line_number":30149,"context_line":"        for name in (\u0027image\u0027, \u0027ephemerals\u0027, \u0027swap\u0027):"},{"line_number":30150,"context_line":"            new_block_device_info \u003d ("},{"line_number":30151,"context_line":"                block_device_info[name] if name \u003d\u003d \u0027swap\u0027 else"},{"line_number":30152,"context_line":"                block_device_info[name][0])"},{"line_number":30153,"context_line":"            self.assertIsNone(new_block_device_info[\u0027encryption_format\u0027])"},{"line_number":30154,"context_line":"            self.assertEqual("},{"line_number":30155,"context_line":"                uuids.secret, new_block_device_info[\u0027encryption_secret_uuid\u0027])"}],"source_content_type":"text/x-python","patch_set":30,"id":"d95b2d4a_07fa4c4d","line":30152,"updated":"2024-01-31 08:31:53.000000000","message":"i find it odd that image is a list given image is the root disk but ok.","commit_id":"af5f5193cfe0a6fe7687fdd038bf1bc86f061298"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"8858bf8756fcb96938720d642df86c54198882fe","unresolved":false,"context_lines":[{"line_number":30359,"context_line":"            f\u0027{self.instance.uuid}_{self.img_bdm.uuid}: error\\n\u0027"},{"line_number":30360,"context_line":"            f\u0027Failed to delete libvirt secret \u0027"},{"line_number":30361,"context_line":"            f\u0027{self.instance.uuid}_{self.eph_bdm.uuid}: error\u0027)"},{"line_number":30362,"context_line":"        self.assertEqual(expected_msg, str(exp))"},{"line_number":30363,"context_line":""},{"line_number":30364,"context_line":"    @mock.patch.object("},{"line_number":30365,"context_line":"        libvirt_driver.LibvirtDriver, \u0027_cleanup_lvm\u0027, new\u003dmock.Mock())"}],"source_content_type":"text/x-python","patch_set":30,"id":"de5b580e_cd18c0c6","line":30362,"updated":"2024-01-31 08:31:53.000000000","message":"+1","commit_id":"af5f5193cfe0a6fe7687fdd038bf1bc86f061298"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"63009b6f26c18da7f312dce3fd51f0e455e5eac5","unresolved":false,"context_lines":[{"line_number":30083,"context_line":"        keymgr_call3 \u003d mock.call(self.context, self.instance, driver_bdm)"},{"line_number":30084,"context_line":"        libvirt_call3 \u003d mock.call("},{"line_number":30085,"context_line":"            \u0027volume\u0027, f\"{self.instance.uuid}_{driver_bdm[\u0027uuid\u0027]}\","},{"line_number":30086,"context_line":"            password\u003dmock.sentinel.secret3, uuid\u003duuids.secret3)"},{"line_number":30087,"context_line":""},{"line_number":30088,"context_line":"        # Assert that we generated key manager and libvirt secrets."},{"line_number":30089,"context_line":"        self.assertEqual("}],"source_content_type":"text/x-python","patch_set":32,"id":"9b34340f_b9d97a35","line":30086,"updated":"2024-02-13 05:32:56.000000000","message":"+1\nthere is some repetion here but its explict and easy to follow","commit_id":"bc58dbd9e62a473bea46b2e44f1d4b4bfcf406d0"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"4ecf161680f76a6a5a3c202571406681df8a1b9b","unresolved":true,"context_lines":[{"line_number":30758,"context_line":"            \u0027Failed to delete libvirt secret \u0027"},{"line_number":30759,"context_line":"            f\u0027{self.instance.uuid}_{self.eph_bdm.uuid}\u0027,"},{"line_number":30760,"context_line":"            instance\u003dself.instance)"},{"line_number":30761,"context_line":"        self.assertEqual([call1, call2], mock_log_exc.mock_calls)"},{"line_number":30762,"context_line":""},{"line_number":30763,"context_line":"    def test_spawn_with_ephemeral_encryption_mix_with_without_secrets(self):"},{"line_number":30764,"context_line":"        # Test that we fail fast if we detect a mix of driver BDMs that have"}],"source_content_type":"text/x-python","patch_set":36,"id":"15a3b1bb_aab57cc4","line":30761,"updated":"2024-02-28 14:49:37.000000000","message":"`assert_has_call()`? It\u0027s not wrong, just surprised to see it :)","commit_id":"177c184e405f4227b430151b076f0eaa6efff1a7"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"8b00e227df55a0dab45d4f9493df1aa191b46b18","unresolved":true,"context_lines":[{"line_number":30758,"context_line":"            \u0027Failed to delete libvirt secret \u0027"},{"line_number":30759,"context_line":"            f\u0027{self.instance.uuid}_{self.eph_bdm.uuid}\u0027,"},{"line_number":30760,"context_line":"            instance\u003dself.instance)"},{"line_number":30761,"context_line":"        self.assertEqual([call1, call2], mock_log_exc.mock_calls)"},{"line_number":30762,"context_line":""},{"line_number":30763,"context_line":"    def test_spawn_with_ephemeral_encryption_mix_with_without_secrets(self):"},{"line_number":30764,"context_line":"        # Test that we fail fast if we detect a mix of driver BDMs that have"}],"source_content_type":"text/x-python","patch_set":36,"id":"5bd98313_ccd7de2a","line":30761,"in_reply_to":"15a3b1bb_aab57cc4","updated":"2024-02-28 20:18:45.000000000","message":"I use this because it also asserts the number of calls. `assert_has_call()` only tells you that at some point the mock was called with that call (which could be more times or fewer times than you expected).","commit_id":"177c184e405f4227b430151b076f0eaa6efff1a7"}],"nova/virt/libvirt/driver.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"ccd80079613e072df27ab701f70ebdc36b8d9e92","unresolved":true,"context_lines":[{"line_number":4354,"context_line":"        \"\"\"Add ephemeral encryption attributes to driver BDMs before use."},{"line_number":4355,"context_line":"        \"\"\""},{"line_number":4356,"context_line":"        encrypted_bdms \u003d driver.block_device_info_get_encrypted_disks("},{"line_number":4357,"context_line":"            block_device_info)"},{"line_number":4358,"context_line":""},{"line_number":4359,"context_line":"        for driver_bdm in encrypted_bdms:"},{"line_number":4360,"context_line":"            # NOTE(lyarwood): Users can request that their ephemeral storage"}],"source_content_type":"text/x-python","patch_set":12,"id":"af0d42b2_77ae4902","line":4357,"updated":"2023-02-07 09:32:04.000000000","message":"this currently does not include the swap disk its just the root disk and ephmeral disks.\nhttps://github.com/openstack/nova/blob/d892905904fd8ed7b5bb916ca4a119997ea0c9a7/nova/virt/driver.py#L108\n\nthat can be fixed later just pointing out that its not currently handeled","commit_id":"8613ba9dc613b02b1e1a3a682bfbcd495b756209"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"0ea158dc87f6c5031863fcea8df56ed3a17d41bf","unresolved":false,"context_lines":[{"line_number":4354,"context_line":"        \"\"\"Add ephemeral encryption attributes to driver BDMs before use."},{"line_number":4355,"context_line":"        \"\"\""},{"line_number":4356,"context_line":"        encrypted_bdms \u003d driver.block_device_info_get_encrypted_disks("},{"line_number":4357,"context_line":"            block_device_info)"},{"line_number":4358,"context_line":""},{"line_number":4359,"context_line":"        for driver_bdm in encrypted_bdms:"},{"line_number":4360,"context_line":"            # NOTE(lyarwood): Users can request that their ephemeral storage"}],"source_content_type":"text/x-python","patch_set":12,"id":"29cd62bb_1152db45","line":4357,"in_reply_to":"62083560_314f3af9","updated":"2023-08-09 03:33:05.000000000","message":"Done","commit_id":"8613ba9dc613b02b1e1a3a682bfbcd495b756209"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"97df3b648546a009bf48e8c417292e2b56ed0a44","unresolved":true,"context_lines":[{"line_number":4354,"context_line":"        \"\"\"Add ephemeral encryption attributes to driver BDMs before use."},{"line_number":4355,"context_line":"        \"\"\""},{"line_number":4356,"context_line":"        encrypted_bdms \u003d driver.block_device_info_get_encrypted_disks("},{"line_number":4357,"context_line":"            block_device_info)"},{"line_number":4358,"context_line":""},{"line_number":4359,"context_line":"        for driver_bdm in encrypted_bdms:"},{"line_number":4360,"context_line":"            # NOTE(lyarwood): Users can request that their ephemeral storage"}],"source_content_type":"text/x-python","patch_set":12,"id":"62083560_314f3af9","line":4357,"in_reply_to":"af0d42b2_77ae4902","updated":"2023-02-07 09:48:28.000000000","message":"It\u0027s on my todo list. I\u0027ll splice in another patch to add the swap.","commit_id":"8613ba9dc613b02b1e1a3a682bfbcd495b756209"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"594f65c577d8f4cc1f8f44184f959d4303bd1574","unresolved":true,"context_lines":[{"line_number":1729,"context_line":"        for driver_bdm in encrypted_bdms:"},{"line_number":1730,"context_line":"            secret_uuid \u003d driver_bdm.get(\u0027encryption_secret_uuid\u0027)"},{"line_number":1731,"context_line":"            if secret_uuid:"},{"line_number":1732,"context_line":"                crypto.delete_encryption_secret(context, instance, secret_uuid)"},{"line_number":1733,"context_line":"            secret_usage \u003d f\"{instance.uuid}_{driver_bdm[\u0027uuid\u0027]}\""},{"line_number":1734,"context_line":"            if self._host.find_secret(\u0027volume\u0027, secret_usage):"},{"line_number":1735,"context_line":"                self._host.delete_secret(\u0027volume\u0027, secret_usage)"}],"source_content_type":"text/x-python","patch_set":20,"id":"50e76401_d43dc010","line":1732,"updated":"2023-08-08 20:54:04.000000000","message":"This routine catches NotFound and ignores, but surely there are other ways we could fail here right? I\u0027m not sure whether it\u0027s better to fail or keep trying to clean up, but it kinda seems like if we could have already deleted some secrets before we get here, that we should try to delete as many as we can and raise the error later, no? And, presumably we want to continue on to the host delete below even if we can\u0027t delete it from the remote key manager?","commit_id":"b818211ad6670c7b7f857aaa38d089fd2fd5ea27"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"bcfbed56b259383c1e833cec0e467d9e88b96f5b","unresolved":false,"context_lines":[{"line_number":1729,"context_line":"        for driver_bdm in encrypted_bdms:"},{"line_number":1730,"context_line":"            secret_uuid \u003d driver_bdm.get(\u0027encryption_secret_uuid\u0027)"},{"line_number":1731,"context_line":"            if secret_uuid:"},{"line_number":1732,"context_line":"                crypto.delete_encryption_secret(context, instance, secret_uuid)"},{"line_number":1733,"context_line":"            secret_usage \u003d f\"{instance.uuid}_{driver_bdm[\u0027uuid\u0027]}\""},{"line_number":1734,"context_line":"            if self._host.find_secret(\u0027volume\u0027, secret_usage):"},{"line_number":1735,"context_line":"                self._host.delete_secret(\u0027volume\u0027, secret_usage)"}],"source_content_type":"text/x-python","patch_set":20,"id":"2b334a27_a106a87d","line":1732,"in_reply_to":"461da14a_94fe53fa","updated":"2023-11-16 19:54:10.000000000","message":"Done","commit_id":"b818211ad6670c7b7f857aaa38d089fd2fd5ea27"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"0ea158dc87f6c5031863fcea8df56ed3a17d41bf","unresolved":true,"context_lines":[{"line_number":1729,"context_line":"        for driver_bdm in encrypted_bdms:"},{"line_number":1730,"context_line":"            secret_uuid \u003d driver_bdm.get(\u0027encryption_secret_uuid\u0027)"},{"line_number":1731,"context_line":"            if secret_uuid:"},{"line_number":1732,"context_line":"                crypto.delete_encryption_secret(context, instance, secret_uuid)"},{"line_number":1733,"context_line":"            secret_usage \u003d f\"{instance.uuid}_{driver_bdm[\u0027uuid\u0027]}\""},{"line_number":1734,"context_line":"            if self._host.find_secret(\u0027volume\u0027, secret_usage):"},{"line_number":1735,"context_line":"                self._host.delete_secret(\u0027volume\u0027, secret_usage)"}],"source_content_type":"text/x-python","patch_set":20,"id":"461da14a_94fe53fa","line":1732,"in_reply_to":"50e76401_d43dc010","updated":"2023-08-09 03:33:05.000000000","message":"Hm, yeah, these are good points. I\u0027ll have a think about what it should do in the event of failure here.","commit_id":"b818211ad6670c7b7f857aaa38d089fd2fd5ea27"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"594f65c577d8f4cc1f8f44184f959d4303bd1574","unresolved":true,"context_lines":[{"line_number":4408,"context_line":"                # Stash the UUID of said secret in our driver BDM"},{"line_number":4409,"context_line":"                driver_bdm[\u0027encryption_secret_uuid\u0027] \u003d secret_uuid"},{"line_number":4410,"context_line":"            else:"},{"line_number":4411,"context_line":"                secret \u003d crypto.get_encryption_secret(context, secret_uuid)"},{"line_number":4412,"context_line":""},{"line_number":4413,"context_line":"            # Stash the passphrase itself in a libvirt secret using the"},{"line_number":4414,"context_line":"            # same UUID as the key manager secret for easy retrieval later"}],"source_content_type":"text/x-python","patch_set":20,"id":"8c7945cb_2c062d18","line":4411,"updated":"2023-08-08 20:54:04.000000000","message":"Same sort of question here.. We might have provisioned several secrets before we die on the one they provided and typo\u0027d. Should we clean up any of the ones we created before we fail here?","commit_id":"b818211ad6670c7b7f857aaa38d089fd2fd5ea27"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"bcfbed56b259383c1e833cec0e467d9e88b96f5b","unresolved":false,"context_lines":[{"line_number":4408,"context_line":"                # Stash the UUID of said secret in our driver BDM"},{"line_number":4409,"context_line":"                driver_bdm[\u0027encryption_secret_uuid\u0027] \u003d secret_uuid"},{"line_number":4410,"context_line":"            else:"},{"line_number":4411,"context_line":"                secret \u003d crypto.get_encryption_secret(context, secret_uuid)"},{"line_number":4412,"context_line":""},{"line_number":4413,"context_line":"            # Stash the passphrase itself in a libvirt secret using the"},{"line_number":4414,"context_line":"            # same UUID as the key manager secret for easy retrieval later"}],"source_content_type":"text/x-python","patch_set":20,"id":"5d3c69e9_e2634486","line":4411,"in_reply_to":"175a7dde_f42130ad","updated":"2023-11-16 19:54:10.000000000","message":"Done","commit_id":"b818211ad6670c7b7f857aaa38d089fd2fd5ea27"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"0ea158dc87f6c5031863fcea8df56ed3a17d41bf","unresolved":true,"context_lines":[{"line_number":4408,"context_line":"                # Stash the UUID of said secret in our driver BDM"},{"line_number":4409,"context_line":"                driver_bdm[\u0027encryption_secret_uuid\u0027] \u003d secret_uuid"},{"line_number":4410,"context_line":"            else:"},{"line_number":4411,"context_line":"                secret \u003d crypto.get_encryption_secret(context, secret_uuid)"},{"line_number":4412,"context_line":""},{"line_number":4413,"context_line":"            # Stash the passphrase itself in a libvirt secret using the"},{"line_number":4414,"context_line":"            # same UUID as the key manager secret for easy retrieval later"}],"source_content_type":"text/x-python","patch_set":20,"id":"175a7dde_f42130ad","line":4411,"in_reply_to":"8c7945cb_2c062d18","updated":"2023-08-09 03:33:05.000000000","message":"Same.","commit_id":"b818211ad6670c7b7f857aaa38d089fd2fd5ea27"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"e37da7d7b1b4e7ea928cb3949cdf9828604f61d3","unresolved":true,"context_lines":[{"line_number":4435,"context_line":"                        instance\u003dinstance)"},{"line_number":4436,"context_line":"                    # Skip creation of a libvirt secret if we couldn\u0027t find the"},{"line_number":4437,"context_line":"                    # secret in the key manager."},{"line_number":4438,"context_line":"                    continue"},{"line_number":4439,"context_line":""},{"line_number":4440,"context_line":"            # Ensure this is all saved back down in the database via the o.vo"},{"line_number":4441,"context_line":"            # BlockDeviceMapping object"}],"source_content_type":"text/x-python","patch_set":21,"id":"5276e0b1_4c129fe5","line":4438,"updated":"2023-11-15 21:06:36.000000000","message":"The cleanup changes above look good to me, but I\u0027m not sure about this one. In this loop, we might have created secrets on L4428 and then finally get to one that was specified but not found. We should clean up those secrets we created *and* fail right? This continue means we probably won\u0027t be able to read the disk because we were told to use a secret but didn\u0027t find the right one?","commit_id":"6c0b9f002e670e0ef78ca8a9106d3e7ac9d6fb3f"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"c4372e64776e6a918f8fa129451aa43e658c798e","unresolved":false,"context_lines":[{"line_number":4435,"context_line":"                        instance\u003dinstance)"},{"line_number":4436,"context_line":"                    # Skip creation of a libvirt secret if we couldn\u0027t find the"},{"line_number":4437,"context_line":"                    # secret in the key manager."},{"line_number":4438,"context_line":"                    continue"},{"line_number":4439,"context_line":""},{"line_number":4440,"context_line":"            # Ensure this is all saved back down in the database via the o.vo"},{"line_number":4441,"context_line":"            # BlockDeviceMapping object"}],"source_content_type":"text/x-python","patch_set":21,"id":"93bd79f8_ad69382e","line":4438,"in_reply_to":"3f2a6ab0_9aa0ac07","updated":"2024-01-25 12:00:51.000000000","message":"Acknowledged","commit_id":"6c0b9f002e670e0ef78ca8a9106d3e7ac9d6fb3f"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"bcfbed56b259383c1e833cec0e467d9e88b96f5b","unresolved":true,"context_lines":[{"line_number":4435,"context_line":"                        instance\u003dinstance)"},{"line_number":4436,"context_line":"                    # Skip creation of a libvirt secret if we couldn\u0027t find the"},{"line_number":4437,"context_line":"                    # secret in the key manager."},{"line_number":4438,"context_line":"                    continue"},{"line_number":4439,"context_line":""},{"line_number":4440,"context_line":"            # Ensure this is all saved back down in the database via the o.vo"},{"line_number":4441,"context_line":"            # BlockDeviceMapping object"}],"source_content_type":"text/x-python","patch_set":21,"id":"ef18d00f_52663abc","line":4438,"in_reply_to":"5276e0b1_4c129fe5","updated":"2023-11-16 19:54:10.000000000","message":"Hmmm ... I agree this doesn\u0027t seem to make sense. I think I had been thinking in earlier iterations of the series to reuse secrets in the case of a rebuild (perhaps because the BDM records themselves are reused).\n\nBut tl;dr having worked through several other server actions since then, I no longer think they should be reused and that this should only be creating new secrets, consistent with the \"server create\" path.\n\nI\u0027m going to respin with those changes.","commit_id":"6c0b9f002e670e0ef78ca8a9106d3e7ac9d6fb3f"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"3d07b8dd0c320c19014c1d40e28f9fe09dcff262","unresolved":true,"context_lines":[{"line_number":4435,"context_line":"                        instance\u003dinstance)"},{"line_number":4436,"context_line":"                    # Skip creation of a libvirt secret if we couldn\u0027t find the"},{"line_number":4437,"context_line":"                    # secret in the key manager."},{"line_number":4438,"context_line":"                    continue"},{"line_number":4439,"context_line":""},{"line_number":4440,"context_line":"            # Ensure this is all saved back down in the database via the o.vo"},{"line_number":4441,"context_line":"            # BlockDeviceMapping object"}],"source_content_type":"text/x-python","patch_set":21,"id":"3f2a6ab0_9aa0ac07","line":4438,"in_reply_to":"636bf459_9a3f7ea5","updated":"2023-12-21 19:08:43.000000000","message":"\u003e I have respun to not \"get\" encryption secrets in this method -- so the spawn path will always create new secrets. I have not added cleanup for failure to create here with the thinking that if we fail here, we will get an instance in ERROR state, which will need to be deleted by the user, at which time the created secrets will be cleaned up.\n\n\u003e (After I pushed PS22 I noticed that driver_bdm.save() is after the libvirt secret create, so technically if libvirt secret create fails then we will not have saved the encryption_secret_uuid to the BDM record in the database. To handle that, we could either do two driver_bdm.save() or maybe add something like \"created_secret \u003d True\" after the key manager create, driver_bdm.save(), then if created_secret: create the libvirt secret.)\n\n**Please disregard all of this ^.** I had forgotten 😣 why secrets need to be re-used for rebuild and shelve ... so that if something like an admin rebuilds a user\u0027s instance, the user will still have access to the secret for the instance disk. If we created a new secret in that scenario, the new secret would be owned by the admin instead of the user.\n\n\u003e You could put the driver_bdm.save() in a finally block so its called once regarless of if there is an expcetion or not perhaps.\nit looks like you added cleanup logic to the excpetion block to delete teh secrets when there is an exeption however so im not sure if this is now resolved?\n\nI think you are right, that this should be now solved by deleting the secrets in the exception block.","commit_id":"6c0b9f002e670e0ef78ca8a9106d3e7ac9d6fb3f"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"50f79cb15115b743e66b65a782d71bcc76132dff","unresolved":true,"context_lines":[{"line_number":4435,"context_line":"                        instance\u003dinstance)"},{"line_number":4436,"context_line":"                    # Skip creation of a libvirt secret if we couldn\u0027t find the"},{"line_number":4437,"context_line":"                    # secret in the key manager."},{"line_number":4438,"context_line":"                    continue"},{"line_number":4439,"context_line":""},{"line_number":4440,"context_line":"            # Ensure this is all saved back down in the database via the o.vo"},{"line_number":4441,"context_line":"            # BlockDeviceMapping object"}],"source_content_type":"text/x-python","patch_set":21,"id":"636bf459_9a3f7ea5","line":4438,"in_reply_to":"9eebb82d_4d3c5078","updated":"2023-12-20 13:32:58.000000000","message":"You could put the driver_bdm.save() in a finally block so its called once regarless of if there is an expcetion or not perhaps.\nit looks like you added cleanup logic to the excpetion block to delete teh secrets when there is an exeption however so im not sure if this is now resolved?","commit_id":"6c0b9f002e670e0ef78ca8a9106d3e7ac9d6fb3f"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"1cd5965c23007eea81cd4d86a1fb0ecabc02680c","unresolved":true,"context_lines":[{"line_number":4435,"context_line":"                        instance\u003dinstance)"},{"line_number":4436,"context_line":"                    # Skip creation of a libvirt secret if we couldn\u0027t find the"},{"line_number":4437,"context_line":"                    # secret in the key manager."},{"line_number":4438,"context_line":"                    continue"},{"line_number":4439,"context_line":""},{"line_number":4440,"context_line":"            # Ensure this is all saved back down in the database via the o.vo"},{"line_number":4441,"context_line":"            # BlockDeviceMapping object"}],"source_content_type":"text/x-python","patch_set":21,"id":"9eebb82d_4d3c5078","line":4438,"in_reply_to":"ef18d00f_52663abc","updated":"2023-11-16 23:43:08.000000000","message":"I have respun to not \"get\" encryption secrets in this method -- so the spawn path will always create new secrets. I have not added cleanup for failure to create here with the thinking that if we fail here, we will get an instance in ERROR state, which will need to be deleted by the user, at which time the created secrets will be cleaned up.\n\n(After I pushed PS22 I noticed that driver_bdm.save() is after the libvirt secret create, so technically if libvirt secret create fails then we will not have saved the encryption_secret_uuid to the BDM record in the database. To handle that, we could either do two driver_bdm.save() or maybe add something like \"created_secret \u003d True\" after the key manager create, driver_bdm.save(), then if created_secret: create the libvirt secret.)","commit_id":"6c0b9f002e670e0ef78ca8a9106d3e7ac9d6fb3f"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"50f79cb15115b743e66b65a782d71bcc76132dff","unresolved":true,"context_lines":[{"line_number":4467,"context_line":"                driver_bdm for driver_bdm in encrypted_bdms"},{"line_number":4468,"context_line":"                if driver_bdm.get(\u0027encryption_secret_uuid\u0027)]"},{"line_number":4469,"context_line":"            if (len(bdms_without_secrets) !\u003d len(encrypted_bdms) and"},{"line_number":4470,"context_line":"                    len(bdms_with_secrets) !\u003d len(encrypted_bdms)):"},{"line_number":4471,"context_line":"                msg \u003d ("},{"line_number":4472,"context_line":"                    f\u0027Found a mix of encrypted BDMs with and without existing \u0027"},{"line_number":4473,"context_line":"                    f\u0027encryption secrets: {encrypted_bdms}\u0027)"}],"source_content_type":"text/x-python","patch_set":24,"id":"106eceda_125e8e22","line":4470,"updated":"2023-12-20 13:32:58.000000000","message":"so this works but you can simplfy it to just \n\nif bdms_without_secrets and bdms_with_scerets:\n …\n \nright?\n\nan empty list is false.\nif both are true it means both have entrie and we ahve a mixed state\n\n\nalso perhaps im not following correctly but isint it valide to have encypted bdms and non encypted bdms if i ahve encypted local storage but a non encypced cinder volume?","commit_id":"8add5a312dba1ba0241230738edff0ca6415713c"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"3d07b8dd0c320c19014c1d40e28f9fe09dcff262","unresolved":true,"context_lines":[{"line_number":4467,"context_line":"                driver_bdm for driver_bdm in encrypted_bdms"},{"line_number":4468,"context_line":"                if driver_bdm.get(\u0027encryption_secret_uuid\u0027)]"},{"line_number":4469,"context_line":"            if (len(bdms_without_secrets) !\u003d len(encrypted_bdms) and"},{"line_number":4470,"context_line":"                    len(bdms_with_secrets) !\u003d len(encrypted_bdms)):"},{"line_number":4471,"context_line":"                msg \u003d ("},{"line_number":4472,"context_line":"                    f\u0027Found a mix of encrypted BDMs with and without existing \u0027"},{"line_number":4473,"context_line":"                    f\u0027encryption secrets: {encrypted_bdms}\u0027)"}],"source_content_type":"text/x-python","patch_set":24,"id":"7270f144_8a4de646","line":4470,"in_reply_to":"106eceda_125e8e22","updated":"2023-12-21 19:08:43.000000000","message":"Hm yeah, I think I was just tired or dumb or both 😆\n\nYou are correct that it is valid to have encrypted local but non-encrypted volume however the list of BDMs we\u0027re working with here (L4456) is only local encrypted disks. Cinder volumes are not included in driver.block_device_info_get_encrypted_disks (it returns \u0027image\u0027, \u0027ephemerals\u0027 and \u0027swap\u0027 as of the patch below this one).","commit_id":"8add5a312dba1ba0241230738edff0ca6415713c"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"c4372e64776e6a918f8fa129451aa43e658c798e","unresolved":false,"context_lines":[{"line_number":4467,"context_line":"                driver_bdm for driver_bdm in encrypted_bdms"},{"line_number":4468,"context_line":"                if driver_bdm.get(\u0027encryption_secret_uuid\u0027)]"},{"line_number":4469,"context_line":"            if (len(bdms_without_secrets) !\u003d len(encrypted_bdms) and"},{"line_number":4470,"context_line":"                    len(bdms_with_secrets) !\u003d len(encrypted_bdms)):"},{"line_number":4471,"context_line":"                msg \u003d ("},{"line_number":4472,"context_line":"                    f\u0027Found a mix of encrypted BDMs with and without existing \u0027"},{"line_number":4473,"context_line":"                    f\u0027encryption secrets: {encrypted_bdms}\u0027)"}],"source_content_type":"text/x-python","patch_set":24,"id":"b6e953bb_0be05ba2","line":4470,"in_reply_to":"7270f144_8a4de646","updated":"2024-01-25 12:00:51.000000000","message":"Acknowledged","commit_id":"8add5a312dba1ba0241230738edff0ca6415713c"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"50f79cb15115b743e66b65a782d71bcc76132dff","unresolved":true,"context_lines":[{"line_number":4546,"context_line":"                    LOG.error("},{"line_number":4547,"context_line":"                        f\u0027Failed to delete libvirt secret {secret_usage}\u0027,"},{"line_number":4548,"context_line":"                        instance\u003dinstance)"},{"line_number":4549,"context_line":"                driver_bdm[\u0027encryption_secret_uuid\u0027] \u003d None"},{"line_number":4550,"context_line":"                driver_bdm.save()"},{"line_number":4551,"context_line":"            raise"},{"line_number":4552,"context_line":""}],"source_content_type":"text/x-python","patch_set":24,"id":"5bc2b052_e6925339","line":4549,"updated":"2023-12-20 13:32:58.000000000","message":"i feel like this should only be done if cyrpto.delete succeeded.\nshould we move that and the bdm.save to the first try.\n\nthat why the secret uuid is still there if the delete in barbican fails and we can try again when deleting the instance?","commit_id":"8add5a312dba1ba0241230738edff0ca6415713c"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"3d07b8dd0c320c19014c1d40e28f9fe09dcff262","unresolved":true,"context_lines":[{"line_number":4546,"context_line":"                    LOG.error("},{"line_number":4547,"context_line":"                        f\u0027Failed to delete libvirt secret {secret_usage}\u0027,"},{"line_number":4548,"context_line":"                        instance\u003dinstance)"},{"line_number":4549,"context_line":"                driver_bdm[\u0027encryption_secret_uuid\u0027] \u003d None"},{"line_number":4550,"context_line":"                driver_bdm.save()"},{"line_number":4551,"context_line":"            raise"},{"line_number":4552,"context_line":""}],"source_content_type":"text/x-python","patch_set":24,"id":"a67e2ce3_0082e77e","line":4549,"in_reply_to":"5bc2b052_e6925339","updated":"2023-12-21 19:08:43.000000000","message":"Yeah, I agree, I think I made a mistake here.","commit_id":"8add5a312dba1ba0241230738edff0ca6415713c"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"c4372e64776e6a918f8fa129451aa43e658c798e","unresolved":false,"context_lines":[{"line_number":4546,"context_line":"                    LOG.error("},{"line_number":4547,"context_line":"                        f\u0027Failed to delete libvirt secret {secret_usage}\u0027,"},{"line_number":4548,"context_line":"                        instance\u003dinstance)"},{"line_number":4549,"context_line":"                driver_bdm[\u0027encryption_secret_uuid\u0027] \u003d None"},{"line_number":4550,"context_line":"                driver_bdm.save()"},{"line_number":4551,"context_line":"            raise"},{"line_number":4552,"context_line":""}],"source_content_type":"text/x-python","patch_set":24,"id":"2622599a_05db4dc9","line":4549,"in_reply_to":"a67e2ce3_0082e77e","updated":"2024-01-25 12:00:51.000000000","message":"Acknowledged","commit_id":"8add5a312dba1ba0241230738edff0ca6415713c"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"c4372e64776e6a918f8fa129451aa43e658c798e","unresolved":true,"context_lines":[{"line_number":1736,"context_line":"    def _cleanup_ephemeral_encryption_secrets("},{"line_number":1737,"context_line":"        self, context, instance, block_device_info"},{"line_number":1738,"context_line":"    ):"},{"line_number":1739,"context_line":"        exception_raised \u003d None"},{"line_number":1740,"context_line":"        encrypted_bdms \u003d driver.block_device_info_get_encrypted_disks("},{"line_number":1741,"context_line":"            block_device_info)"},{"line_number":1742,"context_line":""}],"source_content_type":"text/x-python","patch_set":28,"id":"fa16f6b7_82999237","line":1739,"updated":"2024-01-25 12:00:51.000000000","message":"i think this should be a list","commit_id":"d1a37b3f258f4b4539f1f846f0eccc70e027965a"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"8858bf8756fcb96938720d642df86c54198882fe","unresolved":false,"context_lines":[{"line_number":1736,"context_line":"    def _cleanup_ephemeral_encryption_secrets("},{"line_number":1737,"context_line":"        self, context, instance, block_device_info"},{"line_number":1738,"context_line":"    ):"},{"line_number":1739,"context_line":"        exception_raised \u003d None"},{"line_number":1740,"context_line":"        encrypted_bdms \u003d driver.block_device_info_get_encrypted_disks("},{"line_number":1741,"context_line":"            block_device_info)"},{"line_number":1742,"context_line":""}],"source_content_type":"text/x-python","patch_set":28,"id":"c27fc1bb_6e4f9db7","line":1739,"in_reply_to":"fa16f6b7_82999237","updated":"2024-01-31 08:31:53.000000000","message":"Done","commit_id":"d1a37b3f258f4b4539f1f846f0eccc70e027965a"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"c4372e64776e6a918f8fa129451aa43e658c798e","unresolved":true,"context_lines":[{"line_number":1752,"context_line":"            if self._host.find_secret(\u0027volume\u0027, secret_usage):"},{"line_number":1753,"context_line":"                try:"},{"line_number":1754,"context_line":"                    self._host.delete_secret(\u0027volume\u0027, secret_usage)"},{"line_number":1755,"context_line":"                except Exception as e:"},{"line_number":1756,"context_line":"                    LOG.exception("},{"line_number":1757,"context_line":"                            f\u0027Failed to delete libvirt secret {secret_usage}\u0027,"},{"line_number":1758,"context_line":"                            instance\u003dinstance)"}],"source_content_type":"text/x-python","patch_set":28,"id":"0605a51b_e30c4804","line":1755,"updated":"2024-01-25 12:00:51.000000000","message":"do we know what excptions in general we are expecting here.\n\nis it a always an sdk client exception perhaps?","commit_id":"d1a37b3f258f4b4539f1f846f0eccc70e027965a"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"02c0235bb54cd638ea389b53f6caabf7920281b4","unresolved":true,"context_lines":[{"line_number":1752,"context_line":"            if self._host.find_secret(\u0027volume\u0027, secret_usage):"},{"line_number":1753,"context_line":"                try:"},{"line_number":1754,"context_line":"                    self._host.delete_secret(\u0027volume\u0027, secret_usage)"},{"line_number":1755,"context_line":"                except Exception as e:"},{"line_number":1756,"context_line":"                    LOG.exception("},{"line_number":1757,"context_line":"                            f\u0027Failed to delete libvirt secret {secret_usage}\u0027,"},{"line_number":1758,"context_line":"                            instance\u003dinstance)"}],"source_content_type":"text/x-python","patch_set":28,"id":"0e322189_a77b8ada","line":1755,"in_reply_to":"0605a51b_e30c4804","updated":"2024-01-25 18:08:10.000000000","message":"It would be a libvirt error (these are libvirt secrets) so I could probably catch only those. I\u0027ll check.","commit_id":"d1a37b3f258f4b4539f1f846f0eccc70e027965a"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"8858bf8756fcb96938720d642df86c54198882fe","unresolved":false,"context_lines":[{"line_number":1752,"context_line":"            if self._host.find_secret(\u0027volume\u0027, secret_usage):"},{"line_number":1753,"context_line":"                try:"},{"line_number":1754,"context_line":"                    self._host.delete_secret(\u0027volume\u0027, secret_usage)"},{"line_number":1755,"context_line":"                except Exception as e:"},{"line_number":1756,"context_line":"                    LOG.exception("},{"line_number":1757,"context_line":"                            f\u0027Failed to delete libvirt secret {secret_usage}\u0027,"},{"line_number":1758,"context_line":"                            instance\u003dinstance)"}],"source_content_type":"text/x-python","patch_set":28,"id":"9b123077_4ec20c16","line":1755,"in_reply_to":"0e322189_a77b8ada","updated":"2024-01-31 08:31:53.000000000","message":"Acknowledged","commit_id":"d1a37b3f258f4b4539f1f846f0eccc70e027965a"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"c4372e64776e6a918f8fa129451aa43e658c798e","unresolved":true,"context_lines":[{"line_number":1756,"context_line":"                    LOG.exception("},{"line_number":1757,"context_line":"                            f\u0027Failed to delete libvirt secret {secret_usage}\u0027,"},{"line_number":1758,"context_line":"                            instance\u003dinstance)"},{"line_number":1759,"context_line":"                    exception_raised \u003d e"},{"line_number":1760,"context_line":""},{"line_number":1761,"context_line":"        if exception_raised:"},{"line_number":1762,"context_line":"            # Just raise the last exception that was raised."}],"source_content_type":"text/x-python","patch_set":28,"id":"3422a403_30059db4","line":1759,"updated":"2024-01-25 12:00:51.000000000","message":"then you should append the exceptions","commit_id":"d1a37b3f258f4b4539f1f846f0eccc70e027965a"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"c1a07f911be9537ddc68ff5430a1d311ef7f4896","unresolved":false,"context_lines":[{"line_number":1756,"context_line":"                    LOG.exception("},{"line_number":1757,"context_line":"                            f\u0027Failed to delete libvirt secret {secret_usage}\u0027,"},{"line_number":1758,"context_line":"                            instance\u003dinstance)"},{"line_number":1759,"context_line":"                    exception_raised \u003d e"},{"line_number":1760,"context_line":""},{"line_number":1761,"context_line":"        if exception_raised:"},{"line_number":1762,"context_line":"            # Just raise the last exception that was raised."}],"source_content_type":"text/x-python","patch_set":28,"id":"fe254ea6_3aa42466","line":1759,"in_reply_to":"3422a403_30059db4","updated":"2024-01-26 08:13:52.000000000","message":"Done","commit_id":"d1a37b3f258f4b4539f1f846f0eccc70e027965a"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"c4372e64776e6a918f8fa129451aa43e658c798e","unresolved":true,"context_lines":[{"line_number":1760,"context_line":""},{"line_number":1761,"context_line":"        if exception_raised:"},{"line_number":1762,"context_line":"            # Just raise the last exception that was raised."},{"line_number":1763,"context_line":"            raise exception_raised"},{"line_number":1764,"context_line":""},{"line_number":1765,"context_line":"    def cleanup_lingering_instance_resources(self, instance):"},{"line_number":1766,"context_line":"        # zero the data on backend pmem device, if fails"}],"source_content_type":"text/x-python","patch_set":28,"id":"ca2527f0_91c39ac3","line":1763,"updated":"2024-01-25 12:00:51.000000000","message":"and either convert the list of exception to a new excetion or raise them as n exception group\n\nthis willl work as written but it will only raise the last exception which might be ok give we have logged each of the failures but your discarding info currenlty so i want to make sure thats intentional.\n\nif delete_secret fails for diffent reasons you could have diffent recovery mechaniums like retry for a connection issue or treat as success for a 404","commit_id":"d1a37b3f258f4b4539f1f846f0eccc70e027965a"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"72fc77820567554ff7bc85f67ce87584c0a3177c","unresolved":true,"context_lines":[{"line_number":1760,"context_line":""},{"line_number":1761,"context_line":"        if exception_raised:"},{"line_number":1762,"context_line":"            # Just raise the last exception that was raised."},{"line_number":1763,"context_line":"            raise exception_raised"},{"line_number":1764,"context_line":""},{"line_number":1765,"context_line":"    def cleanup_lingering_instance_resources(self, instance):"},{"line_number":1766,"context_line":"        # zero the data on backend pmem device, if fails"}],"source_content_type":"text/x-python","patch_set":28,"id":"8443e959_85b1c420","line":1763,"in_reply_to":"4e01739e_2877a427","updated":"2024-01-31 07:55:47.000000000","message":"ya thats likely the best approch, not only because of the fact we need to supprot py 3.8 this cycle but also becasue we dont have error handeling that is goign to handel each sub exception seperatly","commit_id":"d1a37b3f258f4b4539f1f846f0eccc70e027965a"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"8858bf8756fcb96938720d642df86c54198882fe","unresolved":false,"context_lines":[{"line_number":1760,"context_line":""},{"line_number":1761,"context_line":"        if exception_raised:"},{"line_number":1762,"context_line":"            # Just raise the last exception that was raised."},{"line_number":1763,"context_line":"            raise exception_raised"},{"line_number":1764,"context_line":""},{"line_number":1765,"context_line":"    def cleanup_lingering_instance_resources(self, instance):"},{"line_number":1766,"context_line":"        # zero the data on backend pmem device, if fails"}],"source_content_type":"text/x-python","patch_set":28,"id":"218adb9a_eb782cff","line":1763,"in_reply_to":"8443e959_85b1c420","updated":"2024-01-31 08:31:53.000000000","message":"Done","commit_id":"d1a37b3f258f4b4539f1f846f0eccc70e027965a"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"02c0235bb54cd638ea389b53f6caabf7920281b4","unresolved":true,"context_lines":[{"line_number":1760,"context_line":""},{"line_number":1761,"context_line":"        if exception_raised:"},{"line_number":1762,"context_line":"            # Just raise the last exception that was raised."},{"line_number":1763,"context_line":"            raise exception_raised"},{"line_number":1764,"context_line":""},{"line_number":1765,"context_line":"    def cleanup_lingering_instance_resources(self, instance):"},{"line_number":1766,"context_line":"        # zero the data on backend pmem device, if fails"}],"source_content_type":"text/x-python","patch_set":28,"id":"d74b28a3_ccabdf86","line":1763,"in_reply_to":"ca2527f0_91c39ac3","updated":"2024-01-25 18:08:10.000000000","message":"OK, I wasn\u0027t familiar with \"exception groups\" so I\u0027ll look at that. This was kind of a best/medium effort handling where (I think?) the errors would likely be the same if there were multiple (libvirt errors) but I\u0027ll check out the exception group thing and see if it could fit well.","commit_id":"d1a37b3f258f4b4539f1f846f0eccc70e027965a"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"c1a07f911be9537ddc68ff5430a1d311ef7f4896","unresolved":true,"context_lines":[{"line_number":1760,"context_line":""},{"line_number":1761,"context_line":"        if exception_raised:"},{"line_number":1762,"context_line":"            # Just raise the last exception that was raised."},{"line_number":1763,"context_line":"            raise exception_raised"},{"line_number":1764,"context_line":""},{"line_number":1765,"context_line":"    def cleanup_lingering_instance_resources(self, instance):"},{"line_number":1766,"context_line":"        # zero the data on backend pmem device, if fails"}],"source_content_type":"text/x-python","patch_set":28,"id":"4e01739e_2877a427","line":1763,"in_reply_to":"d74b28a3_ccabdf86","updated":"2024-01-26 08:13:52.000000000","message":"(later) I saw that ExceptionGroup is new in Python 3.11 so I don\u0027t think we should use that until we require \u003e\u003d 3.11.\n\nI\u0027m going to join the list of exception error messages and raise an exception that contains all of them.","commit_id":"d1a37b3f258f4b4539f1f846f0eccc70e027965a"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"c4372e64776e6a918f8fa129451aa43e658c798e","unresolved":true,"context_lines":[{"line_number":4496,"context_line":"                    # instance. There is no way in the barbican API to create a"},{"line_number":4497,"context_line":"                    # secret with a different user/project than the caller, so"},{"line_number":4498,"context_line":"                    # we have to just reuse the secret."},{"line_number":4499,"context_line":"                    secret \u003d crypto.get_encryption_secret(context, secret_uuid)"},{"line_number":4500,"context_line":"                    if secret is None:"},{"line_number":4501,"context_line":"                        # If we get here, because we know this BDM is supposed"},{"line_number":4502,"context_line":"                        # to have an existing secret, we also know all of the"}],"source_content_type":"text/x-python","patch_set":28,"id":"0ca68db0_e24f99a3","line":4499,"updated":"2024-01-25 12:00:51.000000000","message":"thanks for documenting this in a comment\ni agree that the reuse is valid in this case.\n\nstrictly speaking i belive when you are sayign rebuild here you are really refering to evacuate correct.","commit_id":"d1a37b3f258f4b4539f1f846f0eccc70e027965a"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"02c0235bb54cd638ea389b53f6caabf7920281b4","unresolved":true,"context_lines":[{"line_number":4496,"context_line":"                    # instance. There is no way in the barbican API to create a"},{"line_number":4497,"context_line":"                    # secret with a different user/project than the caller, so"},{"line_number":4498,"context_line":"                    # we have to just reuse the secret."},{"line_number":4499,"context_line":"                    secret \u003d crypto.get_encryption_secret(context, secret_uuid)"},{"line_number":4500,"context_line":"                    if secret is None:"},{"line_number":4501,"context_line":"                        # If we get here, because we know this BDM is supposed"},{"line_number":4502,"context_line":"                        # to have an existing secret, we also know all of the"}],"source_content_type":"text/x-python","patch_set":28,"id":"995e8a2e_6d6cbdc4","line":4499,"in_reply_to":"0ca68db0_e24f99a3","updated":"2024-01-25 18:08:10.000000000","message":"Rebuild or evacuate. Rebuild of a non-admin\u0027s instance by an admin isn\u0027t probably likely going to happen in real life, but it is possible to do (and I tried it).","commit_id":"d1a37b3f258f4b4539f1f846f0eccc70e027965a"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"72fc77820567554ff7bc85f67ce87584c0a3177c","unresolved":false,"context_lines":[{"line_number":4496,"context_line":"                    # instance. There is no way in the barbican API to create a"},{"line_number":4497,"context_line":"                    # secret with a different user/project than the caller, so"},{"line_number":4498,"context_line":"                    # we have to just reuse the secret."},{"line_number":4499,"context_line":"                    secret \u003d crypto.get_encryption_secret(context, secret_uuid)"},{"line_number":4500,"context_line":"                    if secret is None:"},{"line_number":4501,"context_line":"                        # If we get here, because we know this BDM is supposed"},{"line_number":4502,"context_line":"                        # to have an existing secret, we also know all of the"}],"source_content_type":"text/x-python","patch_set":28,"id":"e158309e_26aa921d","line":4499,"in_reply_to":"995e8a2e_6d6cbdc4","updated":"2024-01-31 07:55:47.000000000","message":"Acknowledged","commit_id":"d1a37b3f258f4b4539f1f846f0eccc70e027965a"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"8858bf8756fcb96938720d642df86c54198882fe","unresolved":false,"context_lines":[{"line_number":1760,"context_line":"                    exception_msgs.append(msg)"},{"line_number":1761,"context_line":""},{"line_number":1762,"context_line":"        if exception_msgs:"},{"line_number":1763,"context_line":"            msg \u003d \u0027\\n\u0027.join(exception_msgs)"},{"line_number":1764,"context_line":"            raise libvirt.libvirtError(msg)"},{"line_number":1765,"context_line":""},{"line_number":1766,"context_line":"    def cleanup_lingering_instance_resources(self, instance):"}],"source_content_type":"text/x-python","patch_set":30,"id":"c09882a3_778d8e5e","line":1763,"updated":"2024-01-31 08:31:53.000000000","message":"+1","commit_id":"af5f5193cfe0a6fe7687fdd038bf1bc86f061298"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"3cc240de2d9cbb57ed833563e777855592eac3a4","unresolved":true,"context_lines":[{"line_number":1655,"context_line":"        :param cleanup_instance_dir: If the instance dir should be removed"},{"line_number":1656,"context_line":"        :param cleanup_instance_disks: If the instance disks should be removed."},{"line_number":1657,"context_line":"            Also removes ephemeral encryption secrets, if present."},{"line_number":1658,"context_line":"        :param destroy_secrets: If the cinder volume encryption secrets should"},{"line_number":1659,"context_line":"            be deleted."},{"line_number":1660,"context_line":"        \"\"\""},{"line_number":1661,"context_line":"        # zero the data on backend pmem device"}],"source_content_type":"text/x-python","patch_set":35,"id":"8e9e5481_eff596b5","line":1658,"range":{"start_line":1658,"start_character":39,"end_line":1658,"end_character":52},"updated":"2024-02-26 17:22:42.000000000","message":"Is this correct? Seems like you\u0027re honoring it only for instance disks and only based on images_type, implying ephemeral and not volume...","commit_id":"0f14ec3b2f5413084a32818dae4c8fa32ffa7041"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"4ecf161680f76a6a5a3c202571406681df8a1b9b","unresolved":false,"context_lines":[{"line_number":1655,"context_line":"        :param cleanup_instance_dir: If the instance dir should be removed"},{"line_number":1656,"context_line":"        :param cleanup_instance_disks: If the instance disks should be removed."},{"line_number":1657,"context_line":"            Also removes ephemeral encryption secrets, if present."},{"line_number":1658,"context_line":"        :param destroy_secrets: If the cinder volume encryption secrets should"},{"line_number":1659,"context_line":"            be deleted."},{"line_number":1660,"context_line":"        \"\"\""},{"line_number":1661,"context_line":"        # zero the data on backend pmem device"}],"source_content_type":"text/x-python","patch_set":35,"id":"93f5a957_ac403950","line":1658,"range":{"start_line":1658,"start_character":39,"end_line":1658,"end_character":52},"in_reply_to":"108b925e_aabed4b7","updated":"2024-02-28 14:49:37.000000000","message":"Acknowledged","commit_id":"0f14ec3b2f5413084a32818dae4c8fa32ffa7041"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"acab46f7c8433db775e6fd2f40a38d57b629bf5e","unresolved":true,"context_lines":[{"line_number":1655,"context_line":"        :param cleanup_instance_dir: If the instance dir should be removed"},{"line_number":1656,"context_line":"        :param cleanup_instance_disks: If the instance disks should be removed."},{"line_number":1657,"context_line":"            Also removes ephemeral encryption secrets, if present."},{"line_number":1658,"context_line":"        :param destroy_secrets: If the cinder volume encryption secrets should"},{"line_number":1659,"context_line":"            be deleted."},{"line_number":1660,"context_line":"        \"\"\""},{"line_number":1661,"context_line":"        # zero the data on backend pmem device"}],"source_content_type":"text/x-python","patch_set":35,"id":"f5caefa9_2d4d20e5","line":1658,"range":{"start_line":1658,"start_character":39,"end_line":1658,"end_character":52},"in_reply_to":"47d4ad96_355f9f2b","updated":"2024-02-26 19:31:27.000000000","message":"Yeah okay I guess I\u0027m thinking you\u0027re adding this here so it must not be for cinder only, but ... the param was existing you\u0027re just adding docs, so makes sense.","commit_id":"0f14ec3b2f5413084a32818dae4c8fa32ffa7041"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"23c920124bba554dafc37d09c681937afc6929c3","unresolved":true,"context_lines":[{"line_number":1655,"context_line":"        :param cleanup_instance_dir: If the instance dir should be removed"},{"line_number":1656,"context_line":"        :param cleanup_instance_disks: If the instance disks should be removed."},{"line_number":1657,"context_line":"            Also removes ephemeral encryption secrets, if present."},{"line_number":1658,"context_line":"        :param destroy_secrets: If the cinder volume encryption secrets should"},{"line_number":1659,"context_line":"            be deleted."},{"line_number":1660,"context_line":"        \"\"\""},{"line_number":1661,"context_line":"        # zero the data on backend pmem device"}],"source_content_type":"text/x-python","patch_set":35,"id":"47d4ad96_355f9f2b","line":1658,"range":{"start_line":1658,"start_character":39,"end_line":1658,"end_character":52},"in_reply_to":"8e9e5481_eff596b5","updated":"2024-02-26 19:16:37.000000000","message":"Yes, destroy_secrets was added back when native qemu volume encryption support was added in the past and it\u0027s used separately for that.\n\nFor ephemeral, it\u0027s really ultimately tied to whether or not the instance disks exist or not, so I hooked into that instead. If we\u0027re deleting disks entirely, that\u0027s when we delete secrets related to those disks.","commit_id":"0f14ec3b2f5413084a32818dae4c8fa32ffa7041"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"6fde20ad4f63b0742c515e85a788f795204d0de9","unresolved":true,"context_lines":[{"line_number":1655,"context_line":"        :param cleanup_instance_dir: If the instance dir should be removed"},{"line_number":1656,"context_line":"        :param cleanup_instance_disks: If the instance disks should be removed."},{"line_number":1657,"context_line":"            Also removes ephemeral encryption secrets, if present."},{"line_number":1658,"context_line":"        :param destroy_secrets: If the cinder volume encryption secrets should"},{"line_number":1659,"context_line":"            be deleted."},{"line_number":1660,"context_line":"        \"\"\""},{"line_number":1661,"context_line":"        # zero the data on backend pmem device"}],"source_content_type":"text/x-python","patch_set":35,"id":"108b925e_aabed4b7","line":1658,"range":{"start_line":1658,"start_character":39,"end_line":1658,"end_character":52},"in_reply_to":"f5caefa9_2d4d20e5","updated":"2024-02-27 22:17:12.000000000","message":"Yeah, I thought it might be helpful to add some clarity that destroy_secrets is only related to cinder volumes, not ephemeral encryption secrets, for anyone reading this.","commit_id":"0f14ec3b2f5413084a32818dae4c8fa32ffa7041"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"23c920124bba554dafc37d09c681937afc6929c3","unresolved":true,"context_lines":[{"line_number":1684,"context_line":"            try:"},{"line_number":1685,"context_line":"                self._disconnect_volume("},{"line_number":1686,"context_line":"                    context, connection_info, instance,"},{"line_number":1687,"context_line":"                    destroy_secrets\u003ddestroy_secrets, force\u003dTrue)"},{"line_number":1688,"context_line":"            except Exception as exc:"},{"line_number":1689,"context_line":"                with excutils.save_and_reraise_exception() as ctxt:"},{"line_number":1690,"context_line":"                    if cleanup_instance_disks:"}],"source_content_type":"text/x-python","patch_set":35,"id":"166d2992_7fa766fe","line":1687,"updated":"2024-02-26 19:16:37.000000000","message":"Here is where destroy_secrets is used.","commit_id":"0f14ec3b2f5413084a32818dae4c8fa32ffa7041"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"4ecf161680f76a6a5a3c202571406681df8a1b9b","unresolved":false,"context_lines":[{"line_number":1684,"context_line":"            try:"},{"line_number":1685,"context_line":"                self._disconnect_volume("},{"line_number":1686,"context_line":"                    context, connection_info, instance,"},{"line_number":1687,"context_line":"                    destroy_secrets\u003ddestroy_secrets, force\u003dTrue)"},{"line_number":1688,"context_line":"            except Exception as exc:"},{"line_number":1689,"context_line":"                with excutils.save_and_reraise_exception() as ctxt:"},{"line_number":1690,"context_line":"                    if cleanup_instance_disks:"}],"source_content_type":"text/x-python","patch_set":35,"id":"8982458a_5474d92e","line":1687,"in_reply_to":"166d2992_7fa766fe","updated":"2024-02-28 14:49:37.000000000","message":"Acknowledged","commit_id":"0f14ec3b2f5413084a32818dae4c8fa32ffa7041"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"3cc240de2d9cbb57ed833563e777855592eac3a4","unresolved":true,"context_lines":[{"line_number":1761,"context_line":""},{"line_number":1762,"context_line":"        if exception_msgs:"},{"line_number":1763,"context_line":"            msg \u003d \u0027\\n\u0027.join(exception_msgs)"},{"line_number":1764,"context_line":"            raise libvirt.libvirtError(msg)"},{"line_number":1765,"context_line":""},{"line_number":1766,"context_line":"    def cleanup_lingering_instance_resources(self, instance):"},{"line_number":1767,"context_line":"        # zero the data on backend pmem device, if fails"}],"source_content_type":"text/x-python","patch_set":35,"id":"da4e9aa9_9c4580d4","line":1764,"updated":"2024-02-26 17:22:42.000000000","message":"Coalescing the errors into one is good, but I think raising `libvirt.libvirtError` out of here is a bit icky, since that\u0027s not our exception. Can\u0027t we raise something else here?","commit_id":"0f14ec3b2f5413084a32818dae4c8fa32ffa7041"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"23c920124bba554dafc37d09c681937afc6929c3","unresolved":true,"context_lines":[{"line_number":1761,"context_line":""},{"line_number":1762,"context_line":"        if exception_msgs:"},{"line_number":1763,"context_line":"            msg \u003d \u0027\\n\u0027.join(exception_msgs)"},{"line_number":1764,"context_line":"            raise libvirt.libvirtError(msg)"},{"line_number":1765,"context_line":""},{"line_number":1766,"context_line":"    def cleanup_lingering_instance_resources(self, instance):"},{"line_number":1767,"context_line":"        # zero the data on backend pmem device, if fails"}],"source_content_type":"text/x-python","patch_set":35,"id":"f4105377_cc3669ce","line":1764,"in_reply_to":"da4e9aa9_9c4580d4","updated":"2024-02-26 19:16:37.000000000","message":"OK, I was thinking that since it\u0027s a stack of libvirtError then libvirtError would make sense but I see your point that it\u0027s not a literal error from libvirt so making a new exception would be more correct. I\u0027ll add a new exception here.","commit_id":"0f14ec3b2f5413084a32818dae4c8fa32ffa7041"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"4ecf161680f76a6a5a3c202571406681df8a1b9b","unresolved":false,"context_lines":[{"line_number":1761,"context_line":""},{"line_number":1762,"context_line":"        if exception_msgs:"},{"line_number":1763,"context_line":"            msg \u003d \u0027\\n\u0027.join(exception_msgs)"},{"line_number":1764,"context_line":"            raise libvirt.libvirtError(msg)"},{"line_number":1765,"context_line":""},{"line_number":1766,"context_line":"    def cleanup_lingering_instance_resources(self, instance):"},{"line_number":1767,"context_line":"        # zero the data on backend pmem device, if fails"}],"source_content_type":"text/x-python","patch_set":35,"id":"388ffaf5_0fa8e6ff","line":1764,"in_reply_to":"f4105377_cc3669ce","updated":"2024-02-28 14:49:37.000000000","message":"Done","commit_id":"0f14ec3b2f5413084a32818dae4c8fa32ffa7041"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"3cc240de2d9cbb57ed833563e777855592eac3a4","unresolved":true,"context_lines":[{"line_number":4452,"context_line":""},{"line_number":4453,"context_line":"        # Either all of the driver_bdm\u0027s should have existing encryption"},{"line_number":4454,"context_line":"        # secrets (unshelve, rebuild) or none of them should. There should"},{"line_number":4455,"context_line":"        # never be a mix of both. If there is, something is wrong."},{"line_number":4456,"context_line":"        if encrypted_bdms:"},{"line_number":4457,"context_line":"            bdms_without_secrets \u003d ["},{"line_number":4458,"context_line":"                driver_bdm for driver_bdm in encrypted_bdms"}],"source_content_type":"text/x-python","patch_set":35,"id":"32ad5406_3c913e85","line":4455,"updated":"2024-02-26 17:22:42.000000000","message":"I\u0027m a bit surprised to see this, but I\u0027ve not read up enough to know. Presumably that means if you want encrypted root you\u0027re required to take the perf hit for swap and ephemeral as well? Sometimes you\u0027ll want that for sure, but if you\u0027re using ephemeral for `/var/cache` or something that\u0027s rather unfortunate.","commit_id":"0f14ec3b2f5413084a32818dae4c8fa32ffa7041"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"23c920124bba554dafc37d09c681937afc6929c3","unresolved":true,"context_lines":[{"line_number":4452,"context_line":""},{"line_number":4453,"context_line":"        # Either all of the driver_bdm\u0027s should have existing encryption"},{"line_number":4454,"context_line":"        # secrets (unshelve, rebuild) or none of them should. There should"},{"line_number":4455,"context_line":"        # never be a mix of both. If there is, something is wrong."},{"line_number":4456,"context_line":"        if encrypted_bdms:"},{"line_number":4457,"context_line":"            bdms_without_secrets \u003d ["},{"line_number":4458,"context_line":"                driver_bdm for driver_bdm in encrypted_bdms"}],"source_content_type":"text/x-python","patch_set":35,"id":"63f2d716_797a63b2","line":4455,"in_reply_to":"32ad5406_3c913e85","updated":"2024-02-26 19:16:37.000000000","message":"Yes that\u0027s right. Originally swap was not included in the original design for this feature when lyarwood began it. But in discussions since then, consensus was that if we\u0027re saying \"encrypt local disks\" then someone would reasonably think swap is included in that.\n\nBut yeah, there is not more fine-grained control over it, it\u0027s all or nothing as currently designed.","commit_id":"0f14ec3b2f5413084a32818dae4c8fa32ffa7041"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"4ecf161680f76a6a5a3c202571406681df8a1b9b","unresolved":false,"context_lines":[{"line_number":4452,"context_line":""},{"line_number":4453,"context_line":"        # Either all of the driver_bdm\u0027s should have existing encryption"},{"line_number":4454,"context_line":"        # secrets (unshelve, rebuild) or none of them should. There should"},{"line_number":4455,"context_line":"        # never be a mix of both. If there is, something is wrong."},{"line_number":4456,"context_line":"        if encrypted_bdms:"},{"line_number":4457,"context_line":"            bdms_without_secrets \u003d ["},{"line_number":4458,"context_line":"                driver_bdm for driver_bdm in encrypted_bdms"}],"source_content_type":"text/x-python","patch_set":35,"id":"f66fe181_426234fe","line":4455,"in_reply_to":"63f2d716_797a63b2","updated":"2024-02-28 14:49:37.000000000","message":"Acknowledged","commit_id":"0f14ec3b2f5413084a32818dae4c8fa32ffa7041"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"3cc240de2d9cbb57ed833563e777855592eac3a4","unresolved":true,"context_lines":[{"line_number":4473,"context_line":"            for driver_bdm in encrypted_bdms:"},{"line_number":4474,"context_line":"                orig_encrypted_bdms.append(deepcopy(driver_bdm))"},{"line_number":4475,"context_line":"                # NOTE(lyarwood): Users can request that their ephemeral"},{"line_number":4476,"context_line":"                # storage be encrypted without providing an encryption format"},{"line_number":4477,"context_line":"                # to use.  If one isn\u0027t provided use the host default here and"},{"line_number":4478,"context_line":"                # record it in the driver BDM."},{"line_number":4479,"context_line":"                if driver_bdm.get(\u0027encryption_format\u0027) is None:"}],"source_content_type":"text/x-python","patch_set":35,"id":"a8ee1276_5508b8df","line":4476,"range":{"start_line":4476,"start_character":60,"end_line":4476,"end_character":77},"updated":"2024-02-26 17:22:42.000000000","message":"I haven\u0027t read far up enough to see, but we\u0027re allowing the user to specify \"luks\" in the BDM? I\u0027m not sure why that makes sense as it sort of implies you know a lot (too much) about the host no? And TBH, the spec doesn\u0027t even have anything under \"REST API Impact\" so...maybe I\u0027m misunderstanding what is meant here?","commit_id":"0f14ec3b2f5413084a32818dae4c8fa32ffa7041"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"5cd0132123195a8c200c3f4d235e738bc23db8bd","unresolved":true,"context_lines":[{"line_number":4473,"context_line":"            for driver_bdm in encrypted_bdms:"},{"line_number":4474,"context_line":"                orig_encrypted_bdms.append(deepcopy(driver_bdm))"},{"line_number":4475,"context_line":"                # NOTE(lyarwood): Users can request that their ephemeral"},{"line_number":4476,"context_line":"                # storage be encrypted without providing an encryption format"},{"line_number":4477,"context_line":"                # to use.  If one isn\u0027t provided use the host default here and"},{"line_number":4478,"context_line":"                # record it in the driver BDM."},{"line_number":4479,"context_line":"                if driver_bdm.get(\u0027encryption_format\u0027) is None:"}],"source_content_type":"text/x-python","patch_set":35,"id":"c5fb000a_e75f49ee","line":4476,"range":{"start_line":4476,"start_character":60,"end_line":4476,"end_character":77},"in_reply_to":"31af1b69_59e8ed73","updated":"2024-02-26 21:33:17.000000000","message":"Yeah, I agree usually such a requirement are about the algorithm used and things like that, which as you said, we are not exposing a choice about at this time.\n\nI\u0027m fine with that. I\u0027ll move the (hw:|hw_)ephemeral_encryption_format stuff to a separate patch to discuss later if we want to.","commit_id":"0f14ec3b2f5413084a32818dae4c8fa32ffa7041"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"34105538f56edeec17603b016254f9faee443921","unresolved":true,"context_lines":[{"line_number":4473,"context_line":"            for driver_bdm in encrypted_bdms:"},{"line_number":4474,"context_line":"                orig_encrypted_bdms.append(deepcopy(driver_bdm))"},{"line_number":4475,"context_line":"                # NOTE(lyarwood): Users can request that their ephemeral"},{"line_number":4476,"context_line":"                # storage be encrypted without providing an encryption format"},{"line_number":4477,"context_line":"                # to use.  If one isn\u0027t provided use the host default here and"},{"line_number":4478,"context_line":"                # record it in the driver BDM."},{"line_number":4479,"context_line":"                if driver_bdm.get(\u0027encryption_format\u0027) is None:"}],"source_content_type":"text/x-python","patch_set":35,"id":"c0133533_27e213ff","line":4476,"range":{"start_line":4476,"start_character":60,"end_line":4476,"end_character":77},"in_reply_to":"53355789_02e7b165","updated":"2024-02-26 20:49:59.000000000","message":"I see your point. Looking back at the spec:\n\nhttps://specs.openstack.org/openstack/nova-specs/specs/2024.1/approved/ephemeral-storage-encryption.html\n\nit does show the desire to be able to request a specific format but doesn\u0027t seem to mention a real world example of why you would want to provide that. So I wonder ... thinking about a use case Sean brought up of let\u0027s say it\u0027s a bank and for some things (certain VMs) they need to guarantee that those VMs are using the LUKS encryption format, to meet some security requirement required by law or maybe for some other reason. But not all VMs need to have that required encryption format. So with the flavor or image properties they can ensure that the VM they create will be encrypted with LUKS, else let it NoValidHost if the security requirement can\u0027t be met.\n\nEither way, it wouldn\u0027t be hard to remove the format related extra specs/image properties and let it be internal only, if there isn\u0027t a good reason to expose them.","commit_id":"0f14ec3b2f5413084a32818dae4c8fa32ffa7041"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"acab46f7c8433db775e6fd2f40a38d57b629bf5e","unresolved":true,"context_lines":[{"line_number":4473,"context_line":"            for driver_bdm in encrypted_bdms:"},{"line_number":4474,"context_line":"                orig_encrypted_bdms.append(deepcopy(driver_bdm))"},{"line_number":4475,"context_line":"                # NOTE(lyarwood): Users can request that their ephemeral"},{"line_number":4476,"context_line":"                # storage be encrypted without providing an encryption format"},{"line_number":4477,"context_line":"                # to use.  If one isn\u0027t provided use the host default here and"},{"line_number":4478,"context_line":"                # record it in the driver BDM."},{"line_number":4479,"context_line":"                if driver_bdm.get(\u0027encryption_format\u0027) is None:"}],"source_content_type":"text/x-python","patch_set":35,"id":"79f6ab9d_ce5043e7","line":4476,"range":{"start_line":4476,"start_character":60,"end_line":4476,"end_character":77},"in_reply_to":"6cb4a8e9_42c3060b","updated":"2024-02-26 19:31:27.000000000","message":"right, but it wouldn\u0027t be supported by vmware, hyperv, $next_thing. It also won\u0027t be supported by qemu when luks3 comes out, or whatever else. Like, the qcow encryption stuff that used to be supported has been deprecated in favor of luks, and something else will come along later. It seems like the user just needs to say \"encrypt my stuff\" and the \"how\" is out of their control.","commit_id":"0f14ec3b2f5413084a32818dae4c8fa32ffa7041"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"50e07e3b6571ff7f90be05ddb838bc0c28ef3edd","unresolved":true,"context_lines":[{"line_number":4473,"context_line":"            for driver_bdm in encrypted_bdms:"},{"line_number":4474,"context_line":"                orig_encrypted_bdms.append(deepcopy(driver_bdm))"},{"line_number":4475,"context_line":"                # NOTE(lyarwood): Users can request that their ephemeral"},{"line_number":4476,"context_line":"                # storage be encrypted without providing an encryption format"},{"line_number":4477,"context_line":"                # to use.  If one isn\u0027t provided use the host default here and"},{"line_number":4478,"context_line":"                # record it in the driver BDM."},{"line_number":4479,"context_line":"                if driver_bdm.get(\u0027encryption_format\u0027) is None:"}],"source_content_type":"text/x-python","patch_set":35,"id":"e3ec8cb1_14e31f64","line":4476,"range":{"start_line":4476,"start_character":60,"end_line":4476,"end_character":77},"in_reply_to":"79f6ab9d_ce5043e7","updated":"2024-02-26 20:02:59.000000000","message":"Yeah... for different virt drivers. I\u0027m not opposed to the idea of just saying \"encrypt local disks\" and the rest is opaque. Because currently it lets you set encryption format but the only valid choice is \u0027luks\u0027. Future work that would happen immediately after this series is the deprecation of the existing LVM backend ephemeral encryption via config option implementation and change it over to using this image/flavor based framework, in which case would be hw_ephemeral_encryption_format\u003d\u0027plain\u0027.\n\nBut again, it\u0027s true that\u0027s image backend dependent so in that case if you were to only expose hw_ephemeral_encryption\u003d\u0027true\u0027 then underneath if the image backend is LVM then the format would be implied to be \u0027plain\u0027.","commit_id":"0f14ec3b2f5413084a32818dae4c8fa32ffa7041"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"23c920124bba554dafc37d09c681937afc6929c3","unresolved":true,"context_lines":[{"line_number":4473,"context_line":"            for driver_bdm in encrypted_bdms:"},{"line_number":4474,"context_line":"                orig_encrypted_bdms.append(deepcopy(driver_bdm))"},{"line_number":4475,"context_line":"                # NOTE(lyarwood): Users can request that their ephemeral"},{"line_number":4476,"context_line":"                # storage be encrypted without providing an encryption format"},{"line_number":4477,"context_line":"                # to use.  If one isn\u0027t provided use the host default here and"},{"line_number":4478,"context_line":"                # record it in the driver BDM."},{"line_number":4479,"context_line":"                if driver_bdm.get(\u0027encryption_format\u0027) is None:"}],"source_content_type":"text/x-python","patch_set":35,"id":"6cb4a8e9_42c3060b","line":4476,"range":{"start_line":4476,"start_character":60,"end_line":4476,"end_character":77},"in_reply_to":"a8ee1276_5508b8df","updated":"2024-02-26 19:16:37.000000000","message":"Indirectly yes via the hw_ephemeral_encryption_format image property. I dunno that it implies a lot about the host? QEMU has native support for luks encryption in general and so every host has the ability to do it. And luks support is advertised by placement traits to indicate whether the host has this new feature or not. Not sure if that answers what you are asking..","commit_id":"0f14ec3b2f5413084a32818dae4c8fa32ffa7041"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"a4e9f12ff31659e0d2963462e8fb45f98ec06812","unresolved":true,"context_lines":[{"line_number":4473,"context_line":"            for driver_bdm in encrypted_bdms:"},{"line_number":4474,"context_line":"                orig_encrypted_bdms.append(deepcopy(driver_bdm))"},{"line_number":4475,"context_line":"                # NOTE(lyarwood): Users can request that their ephemeral"},{"line_number":4476,"context_line":"                # storage be encrypted without providing an encryption format"},{"line_number":4477,"context_line":"                # to use.  If one isn\u0027t provided use the host default here and"},{"line_number":4478,"context_line":"                # record it in the driver BDM."},{"line_number":4479,"context_line":"                if driver_bdm.get(\u0027encryption_format\u0027) is None:"}],"source_content_type":"text/x-python","patch_set":35,"id":"603bd868_633634b4","line":4476,"range":{"start_line":4476,"start_character":60,"end_line":4476,"end_character":77},"in_reply_to":"b36698b0_36e6499d","updated":"2024-02-27 14:35:01.000000000","message":"I\u0027m not saying we need to expose aes-256-cbc, I\u0027m saying I think the only legit claim for compliance would be if you could say something like that in your request. Saying \"luks\" doesn\u0027t mean anything (IMHO) that would make a compliance auditor happy (or more happy than just \"encrypted disk\").\n\n\u003e i would still recommend not setting it and having nova choose it on your behalf. to me sepcifying luks is like speifyign the huge pages size you shoudl just say hw:mem_page_size\u003dlarge and let nova do it but you can say hw:mem_page_size\u003d2M if you really dont want 1G hugepages for some reason.\n\nYeah, but it\u0027s not the same to me. Saying \"I need hugepages\" is like saying \"I need encryption.\" Saying \"I need 1G pages\" is like saying \"I need aes-256-cbc\". I think the fact that you say \"I would recommend not setting it\" is highlighting the point :)","commit_id":"0f14ec3b2f5413084a32818dae4c8fa32ffa7041"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"1def1aa68193169397a7882cdeb18be5a82e24c6","unresolved":true,"context_lines":[{"line_number":4473,"context_line":"            for driver_bdm in encrypted_bdms:"},{"line_number":4474,"context_line":"                orig_encrypted_bdms.append(deepcopy(driver_bdm))"},{"line_number":4475,"context_line":"                # NOTE(lyarwood): Users can request that their ephemeral"},{"line_number":4476,"context_line":"                # storage be encrypted without providing an encryption format"},{"line_number":4477,"context_line":"                # to use.  If one isn\u0027t provided use the host default here and"},{"line_number":4478,"context_line":"                # record it in the driver BDM."},{"line_number":4479,"context_line":"                if driver_bdm.get(\u0027encryption_format\u0027) is None:"}],"source_content_type":"text/x-python","patch_set":35,"id":"31af1b69_59e8ed73","line":4476,"range":{"start_line":4476,"start_character":60,"end_line":4476,"end_character":77},"in_reply_to":"c0133533_27e213ff","updated":"2024-02-26 21:20:25.000000000","message":"Yeah, I\u0027m just not sure that\u0027s a reasonable thing to require. Things that are reasonable to require (which aren\u0027t included, btw) are aes-256-cbc or something like that. I just don\u0027t think it\u0027s quite on the table to require a format. Like, if images_format\u003dqcow, you can\u0027t say \"nope, nope, I *demand* to be backed by a .raw\" right? Not only can\u0027t you require that, but vmware or hyperv couldn\u0027t (or maybe *wouldn\u0027t*) do it.\n\nAnyway, it\u0027s not a huge deal, but I think it\u0027d be better to just snip it out if it\u0027s not too hard and we can argue at the end, or maybe argue when it\u0027s time to add another option :)","commit_id":"0f14ec3b2f5413084a32818dae4c8fa32ffa7041"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"73e5d30d14ed3fa6f04b84e8c4bbd84b81e0b51f","unresolved":true,"context_lines":[{"line_number":4473,"context_line":"            for driver_bdm in encrypted_bdms:"},{"line_number":4474,"context_line":"                orig_encrypted_bdms.append(deepcopy(driver_bdm))"},{"line_number":4475,"context_line":"                # NOTE(lyarwood): Users can request that their ephemeral"},{"line_number":4476,"context_line":"                # storage be encrypted without providing an encryption format"},{"line_number":4477,"context_line":"                # to use.  If one isn\u0027t provided use the host default here and"},{"line_number":4478,"context_line":"                # record it in the driver BDM."},{"line_number":4479,"context_line":"                if driver_bdm.get(\u0027encryption_format\u0027) is None:"}],"source_content_type":"text/x-python","patch_set":35,"id":"b36698b0_36e6499d","line":4476,"range":{"start_line":4476,"start_character":60,"end_line":4476,"end_character":77},"in_reply_to":"c5fb000a_e75f49ee","updated":"2024-02-27 09:15:08.000000000","message":"so when we reviewed the spec we said we did not want to expose the ability to request aes-256-cbc we could and lee I belive originally had config options? for this but we removed the configurableity of this a few rleases ago.\n\nwe originally intended this feature to be standalone form the existing lvm ephmeral encyption since that would be deprecated and eventually replaced by this but if you want  the extra spec for \"give me an encrypted thing\" to be able to land on an lvm host we could... that might make removing that harder but im not entirly against that.\n\noptionally specifying luks to me is fine because our min version of qemu/libvirt support it so if a openstack has libvirt/qemu and is new enought to support this feature it suport luks. i would still recommend not setting it and having nova choose it on your behalf. to me sepcifying luks is like speifyign the huge pages size you shoudl just say hw:mem_page_size\u003dlarge and let nova do it but you can say hw:mem_page_size\u003d2M if you really dont want 1G hugepages for some reason.","commit_id":"0f14ec3b2f5413084a32818dae4c8fa32ffa7041"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"e66170f0a6f755d0c3477b3edf6086ebaf65d4c2","unresolved":true,"context_lines":[{"line_number":4473,"context_line":"            for driver_bdm in encrypted_bdms:"},{"line_number":4474,"context_line":"                orig_encrypted_bdms.append(deepcopy(driver_bdm))"},{"line_number":4475,"context_line":"                # NOTE(lyarwood): Users can request that their ephemeral"},{"line_number":4476,"context_line":"                # storage be encrypted without providing an encryption format"},{"line_number":4477,"context_line":"                # to use.  If one isn\u0027t provided use the host default here and"},{"line_number":4478,"context_line":"                # record it in the driver BDM."},{"line_number":4479,"context_line":"                if driver_bdm.get(\u0027encryption_format\u0027) is None:"}],"source_content_type":"text/x-python","patch_set":35,"id":"53355789_02e7b165","line":4476,"range":{"start_line":4476,"start_character":60,"end_line":4476,"end_character":77},"in_reply_to":"e3ec8cb1_14e31f64","updated":"2024-02-26 20:23:26.000000000","message":"Yeah, so it\u0027s one of those things where, IMHO, the user should say \"I want encrypted disk\" and they get either the LVM-based or LUKS-based stuff depending on where they land. but I don\u0027t think they should get (or need) to say which. If they can, and they know something about the system, it would also be a way to control where they land. Like, if they know the LVM-backed machines have better disk IO, they can say \"uh, yeah, I *really* want lvm encryption for, you know, REASONS\" and land on those hosts instead of the luks-backed ones.","commit_id":"0f14ec3b2f5413084a32818dae4c8fa32ffa7041"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"1942744ac86c4dc1f00e11220a93aa9144f6428b","unresolved":true,"context_lines":[{"line_number":4480,"context_line":"                    driver_bdm[\u0027encryption_format\u0027] \u003d ("},{"line_number":4481,"context_line":"                        CONF.ephemeral_storage_encryption.default_format)"},{"line_number":4482,"context_line":""},{"line_number":4483,"context_line":"                secret_uuid \u003d driver_bdm.get(\u0027encryption_secret_uuid\u0027)"},{"line_number":4484,"context_line":"                if secret_uuid is None:"},{"line_number":4485,"context_line":"                    # Create a passphrase and stash it in the key manager"},{"line_number":4486,"context_line":"                    secret_uuid, secret \u003d crypto.create_encryption_secret("}],"source_content_type":"text/x-python","patch_set":35,"id":"2bca5918_64947def","line":4483,"updated":"2024-02-27 22:05:16.000000000","message":"I came back here from a later patch looking for this. I was thinking we did create new secrets for each disk, but then remembered that we also fetch it below.\n\nSo, help me understand when this would be set on a BDM? Are we expecting this to be injected at boot by a user using one of our many confusing BDM-influencing options? Or is this only for a situation where we\u0027re spawning on this host for a not-new instance (i.e. migration)? If the latter, can we get a comment here to that effect? I was reading the cleanup patch and thinking that there was a situation similar to a volume where a user might or might not have provided a secret (and thus we might or might not should delete it). But I\u0027m guessing that\u0027s not the case.","commit_id":"0f14ec3b2f5413084a32818dae4c8fa32ffa7041"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"6fde20ad4f63b0742c515e85a788f795204d0de9","unresolved":true,"context_lines":[{"line_number":4480,"context_line":"                    driver_bdm[\u0027encryption_format\u0027] \u003d ("},{"line_number":4481,"context_line":"                        CONF.ephemeral_storage_encryption.default_format)"},{"line_number":4482,"context_line":""},{"line_number":4483,"context_line":"                secret_uuid \u003d driver_bdm.get(\u0027encryption_secret_uuid\u0027)"},{"line_number":4484,"context_line":"                if secret_uuid is None:"},{"line_number":4485,"context_line":"                    # Create a passphrase and stash it in the key manager"},{"line_number":4486,"context_line":"                    secret_uuid, secret \u003d crypto.create_encryption_secret("}],"source_content_type":"text/x-python","patch_set":35,"id":"f77a548c_a7aa08fc","line":4483,"in_reply_to":"2bca5918_64947def","updated":"2024-02-27 22:17:12.000000000","message":"Yeah, so I think the case where this comes up is unshelving a shelved offloaded instance. We\u0027re spawning but we\u0027re going to use the same secret the disk had before it was shelved. I think it also comes up during rebuild in the same manner.\n\nUsers aren\u0027t really intended to provide a secret directly, the only way they can do it is through the `hw_ephemeral_encryption_secret_uuid` image property which is tied to the existence of a Glance image, so Nova is never going to delete that secret anyway. As for the BDM in a case like this, there is a patch later in the series that adds another BDM attribute `backing_encryption_secret_uuid` which will refer to the instance\u0027s copy of the source Glance image secret. The handling of encrypted backing files for qcow2 is one of the last patches in the series because it touches several server actions.\n\nI\u0027ll add a code comment explaining when this would be already set on a BDM.","commit_id":"0f14ec3b2f5413084a32818dae4c8fa32ffa7041"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"4ecf161680f76a6a5a3c202571406681df8a1b9b","unresolved":false,"context_lines":[{"line_number":4480,"context_line":"                    driver_bdm[\u0027encryption_format\u0027] \u003d ("},{"line_number":4481,"context_line":"                        CONF.ephemeral_storage_encryption.default_format)"},{"line_number":4482,"context_line":""},{"line_number":4483,"context_line":"                secret_uuid \u003d driver_bdm.get(\u0027encryption_secret_uuid\u0027)"},{"line_number":4484,"context_line":"                if secret_uuid is None:"},{"line_number":4485,"context_line":"                    # Create a passphrase and stash it in the key manager"},{"line_number":4486,"context_line":"                    secret_uuid, secret \u003d crypto.create_encryption_secret("}],"source_content_type":"text/x-python","patch_set":35,"id":"891f0167_91717728","line":4483,"in_reply_to":"f77a548c_a7aa08fc","updated":"2024-02-28 14:49:37.000000000","message":"Done","commit_id":"0f14ec3b2f5413084a32818dae4c8fa32ffa7041"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"3cc240de2d9cbb57ed833563e777855592eac3a4","unresolved":true,"context_lines":[{"line_number":4480,"context_line":"                    driver_bdm[\u0027encryption_format\u0027] \u003d ("},{"line_number":4481,"context_line":"                        CONF.ephemeral_storage_encryption.default_format)"},{"line_number":4482,"context_line":""},{"line_number":4483,"context_line":"                secret_uuid \u003d driver_bdm.get(\u0027encryption_secret_uuid\u0027)"},{"line_number":4484,"context_line":"                if secret_uuid is None:"},{"line_number":4485,"context_line":"                    # Create a passphrase and stash it in the key manager"},{"line_number":4486,"context_line":"                    secret_uuid, secret \u003d crypto.create_encryption_secret("},{"line_number":4487,"context_line":"                        context, instance, driver_bdm)"},{"line_number":4488,"context_line":"                    # Stash the UUID of said secret in our driver BDM"},{"line_number":4489,"context_line":"                    driver_bdm[\u0027encryption_secret_uuid\u0027] \u003d secret_uuid"},{"line_number":4490,"context_line":"                    created_keymgr_secrets.append(secret_uuid)"},{"line_number":4491,"context_line":"                else:"},{"line_number":4492,"context_line":"                    # NOTE(melwitt): In general, we avoid reusing secrets but"},{"line_number":4493,"context_line":"                    # we need to reuse them in the case of shelve/unshelve and"},{"line_number":4494,"context_line":"                    # rebuild. The use case is if an admin user"},{"line_number":4495,"context_line":"                    # shelves/unshelves or rebuilds an instance owned by a"},{"line_number":4496,"context_line":"                    # non-admin user. If we don\u0027t reuse the non-admin user\u0027s"},{"line_number":4497,"context_line":"                    # secret and instead create a new secret, the new secret"},{"line_number":4498,"context_line":"                    # will be owned by the admin user and will prevent the"},{"line_number":4499,"context_line":"                    # non-admin user from accessing the new secret for their"},{"line_number":4500,"context_line":"                    # instance. There is no way in the barbican API to create a"},{"line_number":4501,"context_line":"                    # secret with a different user/project than the caller, so"},{"line_number":4502,"context_line":"                    # we have to just reuse the secret."},{"line_number":4503,"context_line":"                    secret \u003d crypto.get_encryption_secret(context, secret_uuid)"},{"line_number":4504,"context_line":"                    if secret is None:"},{"line_number":4505,"context_line":"                        # If we get here, because we know this BDM is supposed"},{"line_number":4506,"context_line":"                        # to have an existing secret, we also know all of the"},{"line_number":4507,"context_line":"                        # other BDMs have existing secrets too. Because we"},{"line_number":4508,"context_line":"                        # didn\u0027t create any secrets, we don\u0027t need to clean up"},{"line_number":4509,"context_line":"                        # any secrets."},{"line_number":4510,"context_line":"                        msg \u003d ("},{"line_number":4511,"context_line":"                            f\u0027Failed to find encryption secret {secret_uuid} \u0027"},{"line_number":4512,"context_line":"                            f\u0027in the key manager for driver BDM \u0027"},{"line_number":4513,"context_line":"                            f\"{driver_bdm[\u0027uuid\u0027]}\")"},{"line_number":4514,"context_line":"                        raise exception.EphemeralEncryptionSecretNotFound(msg)"},{"line_number":4515,"context_line":""},{"line_number":4516,"context_line":"                # Ensure this is all saved back down in the database via the"},{"line_number":4517,"context_line":"                # o.vo BlockDeviceMapping object"}],"source_content_type":"text/x-python","patch_set":35,"id":"676fb58f_ea6c6c38","line":4514,"range":{"start_line":4483,"start_character":0,"end_line":4514,"end_character":78},"updated":"2024-02-26 17:22:42.000000000","message":"This is a nit, but this is very deeply nested and seems like it\u0027s begging for a helper `get_or_generate_secret()` type thing.","commit_id":"0f14ec3b2f5413084a32818dae4c8fa32ffa7041"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"4ecf161680f76a6a5a3c202571406681df8a1b9b","unresolved":false,"context_lines":[{"line_number":4480,"context_line":"                    driver_bdm[\u0027encryption_format\u0027] \u003d ("},{"line_number":4481,"context_line":"                        CONF.ephemeral_storage_encryption.default_format)"},{"line_number":4482,"context_line":""},{"line_number":4483,"context_line":"                secret_uuid \u003d driver_bdm.get(\u0027encryption_secret_uuid\u0027)"},{"line_number":4484,"context_line":"                if secret_uuid is None:"},{"line_number":4485,"context_line":"                    # Create a passphrase and stash it in the key manager"},{"line_number":4486,"context_line":"                    secret_uuid, secret \u003d crypto.create_encryption_secret("},{"line_number":4487,"context_line":"                        context, instance, driver_bdm)"},{"line_number":4488,"context_line":"                    # Stash the UUID of said secret in our driver BDM"},{"line_number":4489,"context_line":"                    driver_bdm[\u0027encryption_secret_uuid\u0027] \u003d secret_uuid"},{"line_number":4490,"context_line":"                    created_keymgr_secrets.append(secret_uuid)"},{"line_number":4491,"context_line":"                else:"},{"line_number":4492,"context_line":"                    # NOTE(melwitt): In general, we avoid reusing secrets but"},{"line_number":4493,"context_line":"                    # we need to reuse them in the case of shelve/unshelve and"},{"line_number":4494,"context_line":"                    # rebuild. The use case is if an admin user"},{"line_number":4495,"context_line":"                    # shelves/unshelves or rebuilds an instance owned by a"},{"line_number":4496,"context_line":"                    # non-admin user. If we don\u0027t reuse the non-admin user\u0027s"},{"line_number":4497,"context_line":"                    # secret and instead create a new secret, the new secret"},{"line_number":4498,"context_line":"                    # will be owned by the admin user and will prevent the"},{"line_number":4499,"context_line":"                    # non-admin user from accessing the new secret for their"},{"line_number":4500,"context_line":"                    # instance. There is no way in the barbican API to create a"},{"line_number":4501,"context_line":"                    # secret with a different user/project than the caller, so"},{"line_number":4502,"context_line":"                    # we have to just reuse the secret."},{"line_number":4503,"context_line":"                    secret \u003d crypto.get_encryption_secret(context, secret_uuid)"},{"line_number":4504,"context_line":"                    if secret is None:"},{"line_number":4505,"context_line":"                        # If we get here, because we know this BDM is supposed"},{"line_number":4506,"context_line":"                        # to have an existing secret, we also know all of the"},{"line_number":4507,"context_line":"                        # other BDMs have existing secrets too. Because we"},{"line_number":4508,"context_line":"                        # didn\u0027t create any secrets, we don\u0027t need to clean up"},{"line_number":4509,"context_line":"                        # any secrets."},{"line_number":4510,"context_line":"                        msg \u003d ("},{"line_number":4511,"context_line":"                            f\u0027Failed to find encryption secret {secret_uuid} \u0027"},{"line_number":4512,"context_line":"                            f\u0027in the key manager for driver BDM \u0027"},{"line_number":4513,"context_line":"                            f\"{driver_bdm[\u0027uuid\u0027]}\")"},{"line_number":4514,"context_line":"                        raise exception.EphemeralEncryptionSecretNotFound(msg)"},{"line_number":4515,"context_line":""},{"line_number":4516,"context_line":"                # Ensure this is all saved back down in the database via the"},{"line_number":4517,"context_line":"                # o.vo BlockDeviceMapping object"}],"source_content_type":"text/x-python","patch_set":35,"id":"fd0e92c9_e4a3fa30","line":4514,"range":{"start_line":4483,"start_character":0,"end_line":4514,"end_character":78},"in_reply_to":"56551ede_ad33802b","updated":"2024-02-28 14:49:37.000000000","message":"Done","commit_id":"0f14ec3b2f5413084a32818dae4c8fa32ffa7041"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"23c920124bba554dafc37d09c681937afc6929c3","unresolved":true,"context_lines":[{"line_number":4480,"context_line":"                    driver_bdm[\u0027encryption_format\u0027] \u003d ("},{"line_number":4481,"context_line":"                        CONF.ephemeral_storage_encryption.default_format)"},{"line_number":4482,"context_line":""},{"line_number":4483,"context_line":"                secret_uuid \u003d driver_bdm.get(\u0027encryption_secret_uuid\u0027)"},{"line_number":4484,"context_line":"                if secret_uuid is None:"},{"line_number":4485,"context_line":"                    # Create a passphrase and stash it in the key manager"},{"line_number":4486,"context_line":"                    secret_uuid, secret \u003d crypto.create_encryption_secret("},{"line_number":4487,"context_line":"                        context, instance, driver_bdm)"},{"line_number":4488,"context_line":"                    # Stash the UUID of said secret in our driver BDM"},{"line_number":4489,"context_line":"                    driver_bdm[\u0027encryption_secret_uuid\u0027] \u003d secret_uuid"},{"line_number":4490,"context_line":"                    created_keymgr_secrets.append(secret_uuid)"},{"line_number":4491,"context_line":"                else:"},{"line_number":4492,"context_line":"                    # NOTE(melwitt): In general, we avoid reusing secrets but"},{"line_number":4493,"context_line":"                    # we need to reuse them in the case of shelve/unshelve and"},{"line_number":4494,"context_line":"                    # rebuild. The use case is if an admin user"},{"line_number":4495,"context_line":"                    # shelves/unshelves or rebuilds an instance owned by a"},{"line_number":4496,"context_line":"                    # non-admin user. If we don\u0027t reuse the non-admin user\u0027s"},{"line_number":4497,"context_line":"                    # secret and instead create a new secret, the new secret"},{"line_number":4498,"context_line":"                    # will be owned by the admin user and will prevent the"},{"line_number":4499,"context_line":"                    # non-admin user from accessing the new secret for their"},{"line_number":4500,"context_line":"                    # instance. There is no way in the barbican API to create a"},{"line_number":4501,"context_line":"                    # secret with a different user/project than the caller, so"},{"line_number":4502,"context_line":"                    # we have to just reuse the secret."},{"line_number":4503,"context_line":"                    secret \u003d crypto.get_encryption_secret(context, secret_uuid)"},{"line_number":4504,"context_line":"                    if secret is None:"},{"line_number":4505,"context_line":"                        # If we get here, because we know this BDM is supposed"},{"line_number":4506,"context_line":"                        # to have an existing secret, we also know all of the"},{"line_number":4507,"context_line":"                        # other BDMs have existing secrets too. Because we"},{"line_number":4508,"context_line":"                        # didn\u0027t create any secrets, we don\u0027t need to clean up"},{"line_number":4509,"context_line":"                        # any secrets."},{"line_number":4510,"context_line":"                        msg \u003d ("},{"line_number":4511,"context_line":"                            f\u0027Failed to find encryption secret {secret_uuid} \u0027"},{"line_number":4512,"context_line":"                            f\u0027in the key manager for driver BDM \u0027"},{"line_number":4513,"context_line":"                            f\"{driver_bdm[\u0027uuid\u0027]}\")"},{"line_number":4514,"context_line":"                        raise exception.EphemeralEncryptionSecretNotFound(msg)"},{"line_number":4515,"context_line":""},{"line_number":4516,"context_line":"                # Ensure this is all saved back down in the database via the"},{"line_number":4517,"context_line":"                # o.vo BlockDeviceMapping object"}],"source_content_type":"text/x-python","patch_set":35,"id":"56551ede_ad33802b","line":4514,"range":{"start_line":4483,"start_character":0,"end_line":4514,"end_character":78},"in_reply_to":"676fb58f_ea6c6c38","updated":"2024-02-26 19:16:37.000000000","message":"That\u0027s fair enough. I\u0027ll add a helper.","commit_id":"0f14ec3b2f5413084a32818dae4c8fa32ffa7041"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"3cc240de2d9cbb57ed833563e777855592eac3a4","unresolved":true,"context_lines":[{"line_number":4514,"context_line":"                        raise exception.EphemeralEncryptionSecretNotFound(msg)"},{"line_number":4515,"context_line":""},{"line_number":4516,"context_line":"                # Ensure this is all saved back down in the database via the"},{"line_number":4517,"context_line":"                # o.vo BlockDeviceMapping object"},{"line_number":4518,"context_line":"                driver_bdm.save()"},{"line_number":4519,"context_line":""},{"line_number":4520,"context_line":"                # Stash the passphrase itself in a libvirt secret using the"}],"source_content_type":"text/x-python","patch_set":35,"id":"a118b584_a52d5d76","line":4517,"updated":"2024-02-26 17:22:42.000000000","message":"Ensure what is saved? Just what we may have done on L4489? Should we just do that up there and not always down here?","commit_id":"0f14ec3b2f5413084a32818dae4c8fa32ffa7041"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"acab46f7c8433db775e6fd2f40a38d57b629bf5e","unresolved":true,"context_lines":[{"line_number":4514,"context_line":"                        raise exception.EphemeralEncryptionSecretNotFound(msg)"},{"line_number":4515,"context_line":""},{"line_number":4516,"context_line":"                # Ensure this is all saved back down in the database via the"},{"line_number":4517,"context_line":"                # o.vo BlockDeviceMapping object"},{"line_number":4518,"context_line":"                driver_bdm.save()"},{"line_number":4519,"context_line":""},{"line_number":4520,"context_line":"                # Stash the passphrase itself in a libvirt secret using the"}],"source_content_type":"text/x-python","patch_set":35,"id":"b418a234_d27f45d2","line":4517,"in_reply_to":"24dc4925_6fdab018","updated":"2024-02-26 19:31:27.000000000","message":"Oh yeah 4480 sometimes, okay.","commit_id":"0f14ec3b2f5413084a32818dae4c8fa32ffa7041"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"23c920124bba554dafc37d09c681937afc6929c3","unresolved":true,"context_lines":[{"line_number":4514,"context_line":"                        raise exception.EphemeralEncryptionSecretNotFound(msg)"},{"line_number":4515,"context_line":""},{"line_number":4516,"context_line":"                # Ensure this is all saved back down in the database via the"},{"line_number":4517,"context_line":"                # o.vo BlockDeviceMapping object"},{"line_number":4518,"context_line":"                driver_bdm.save()"},{"line_number":4519,"context_line":""},{"line_number":4520,"context_line":"                # Stash the passphrase itself in a libvirt secret using the"}],"source_content_type":"text/x-python","patch_set":35,"id":"24dc4925_6fdab018","line":4517,"in_reply_to":"a118b584_a52d5d76","updated":"2024-02-26 19:16:37.000000000","message":"Yeah, saving any things we set in driver_bdm to the matching objects.BlockDeviceMapping in the database.\n\nIt\u0027s for L4480 and L4489, so I put it here as one save instead of potentially two.","commit_id":"0f14ec3b2f5413084a32818dae4c8fa32ffa7041"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"4ecf161680f76a6a5a3c202571406681df8a1b9b","unresolved":false,"context_lines":[{"line_number":4514,"context_line":"                        raise exception.EphemeralEncryptionSecretNotFound(msg)"},{"line_number":4515,"context_line":""},{"line_number":4516,"context_line":"                # Ensure this is all saved back down in the database via the"},{"line_number":4517,"context_line":"                # o.vo BlockDeviceMapping object"},{"line_number":4518,"context_line":"                driver_bdm.save()"},{"line_number":4519,"context_line":""},{"line_number":4520,"context_line":"                # Stash the passphrase itself in a libvirt secret using the"}],"source_content_type":"text/x-python","patch_set":35,"id":"9544bbbf_0095d45b","line":4517,"in_reply_to":"b418a234_d27f45d2","updated":"2024-02-28 14:49:37.000000000","message":"Acknowledged","commit_id":"0f14ec3b2f5413084a32818dae4c8fa32ffa7041"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"3cc240de2d9cbb57ed833563e777855592eac3a4","unresolved":true,"context_lines":[{"line_number":4520,"context_line":"                # Stash the passphrase itself in a libvirt secret using the"},{"line_number":4521,"context_line":"                # same UUID as the key manager secret for easy retrieval later"},{"line_number":4522,"context_line":"                secret_usage \u003d f\"{instance.uuid}_{driver_bdm[\u0027uuid\u0027]}\""},{"line_number":4523,"context_line":"                if self._host.find_secret(\u0027volume\u0027, secret_usage) is None:"},{"line_number":4524,"context_line":"                    self._host.create_secret("},{"line_number":4525,"context_line":"                        \u0027volume\u0027, secret_usage, password\u003dsecret,"},{"line_number":4526,"context_line":"                        uuid\u003dsecret_uuid)"}],"source_content_type":"text/x-python","patch_set":35,"id":"3e67314b_2588fa88","line":4523,"updated":"2024-02-26 17:22:42.000000000","message":"This assumes that if its present it\u0027s correct right? Wondering if there\u0027s a potential for a case where we had a secret for this instance on this host, the host was down so we evacuated, then shelved/unshelved the instance or maybe rebuilt, and then we got back here via migration. The secret is still present, but stale, we find it and use it and corrupt the disk?","commit_id":"0f14ec3b2f5413084a32818dae4c8fa32ffa7041"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"e66170f0a6f755d0c3477b3edf6086ebaf65d4c2","unresolved":true,"context_lines":[{"line_number":4520,"context_line":"                # Stash the passphrase itself in a libvirt secret using the"},{"line_number":4521,"context_line":"                # same UUID as the key manager secret for easy retrieval later"},{"line_number":4522,"context_line":"                secret_usage \u003d f\"{instance.uuid}_{driver_bdm[\u0027uuid\u0027]}\""},{"line_number":4523,"context_line":"                if self._host.find_secret(\u0027volume\u0027, secret_usage) is None:"},{"line_number":4524,"context_line":"                    self._host.create_secret("},{"line_number":4525,"context_line":"                        \u0027volume\u0027, secret_usage, password\u003dsecret,"},{"line_number":4526,"context_line":"                        uuid\u003dsecret_uuid)"}],"source_content_type":"text/x-python","patch_set":35,"id":"27efbf6f_d6fa69d4","line":4523,"in_reply_to":"20a8744a_c49e8c53","updated":"2024-02-26 20:23:26.000000000","message":"I just don\u0027t think that assuming it exists by name is good enough. Either a bug, explicit hijacking, or an admin trying to help could cause us to say \"well, it exists by name, so it must be good\". Unless there\u0027s a reason *not* to compare, or not to just delete and re-add the known good one, I think it\u0027s the most paranoid/defensive to be explicit here. Sort of the same reason a website makes you re-auth before you can get into the account settings. We know you, your cookie is valid, but before we let you change your password, let\u0027s just make sure you\u0027re still the guy that had the cookie in the first place before we take the big step :)\n\nMaybe another argument that\u0027s worth making is: what about in the future where we support re-keying of some sort? Then we run through this code that assumes the keys never change and we didn\u0027t get a secret updated and then something bad happens?\n\nAnyway, maybe we can get another opinion in here. Perhaps I\u0027m being too paranoid.","commit_id":"0f14ec3b2f5413084a32818dae4c8fa32ffa7041"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"34105538f56edeec17603b016254f9faee443921","unresolved":true,"context_lines":[{"line_number":4520,"context_line":"                # Stash the passphrase itself in a libvirt secret using the"},{"line_number":4521,"context_line":"                # same UUID as the key manager secret for easy retrieval later"},{"line_number":4522,"context_line":"                secret_usage \u003d f\"{instance.uuid}_{driver_bdm[\u0027uuid\u0027]}\""},{"line_number":4523,"context_line":"                if self._host.find_secret(\u0027volume\u0027, secret_usage) is None:"},{"line_number":4524,"context_line":"                    self._host.create_secret("},{"line_number":4525,"context_line":"                        \u0027volume\u0027, secret_usage, password\u003dsecret,"},{"line_number":4526,"context_line":"                        uuid\u003dsecret_uuid)"}],"source_content_type":"text/x-python","patch_set":35,"id":"5a9ccaf3_2672973c","line":4523,"in_reply_to":"27efbf6f_d6fa69d4","updated":"2024-02-26 20:49:59.000000000","message":"Yeah, I see what you mean. There\u0027s no reason not to really, I just wasn\u0027t immediately realizing how it would come up. I think the examples you give make sense, I\u0027ll change this to delete and recreate. It agree it\u0027s the safest thing to do.","commit_id":"0f14ec3b2f5413084a32818dae4c8fa32ffa7041"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"a47844f2799f0c32fb3203487565235861db6504","unresolved":true,"context_lines":[{"line_number":4520,"context_line":"                # Stash the passphrase itself in a libvirt secret using the"},{"line_number":4521,"context_line":"                # same UUID as the key manager secret for easy retrieval later"},{"line_number":4522,"context_line":"                secret_usage \u003d f\"{instance.uuid}_{driver_bdm[\u0027uuid\u0027]}\""},{"line_number":4523,"context_line":"                if self._host.find_secret(\u0027volume\u0027, secret_usage) is None:"},{"line_number":4524,"context_line":"                    self._host.create_secret("},{"line_number":4525,"context_line":"                        \u0027volume\u0027, secret_usage, password\u003dsecret,"},{"line_number":4526,"context_line":"                        uuid\u003dsecret_uuid)"}],"source_content_type":"text/x-python","patch_set":35,"id":"dc068813_ac107434","line":4523,"in_reply_to":"3e67314b_2588fa88","updated":"2024-02-26 17:54:43.000000000","message":"I don\u0027t think you\u0027re covering the case where this is false in the tests (according to coverage).","commit_id":"0f14ec3b2f5413084a32818dae4c8fa32ffa7041"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"4ecf161680f76a6a5a3c202571406681df8a1b9b","unresolved":false,"context_lines":[{"line_number":4520,"context_line":"                # Stash the passphrase itself in a libvirt secret using the"},{"line_number":4521,"context_line":"                # same UUID as the key manager secret for easy retrieval later"},{"line_number":4522,"context_line":"                secret_usage \u003d f\"{instance.uuid}_{driver_bdm[\u0027uuid\u0027]}\""},{"line_number":4523,"context_line":"                if self._host.find_secret(\u0027volume\u0027, secret_usage) is None:"},{"line_number":4524,"context_line":"                    self._host.create_secret("},{"line_number":4525,"context_line":"                        \u0027volume\u0027, secret_usage, password\u003dsecret,"},{"line_number":4526,"context_line":"                        uuid\u003dsecret_uuid)"}],"source_content_type":"text/x-python","patch_set":35,"id":"342caecb_37afb64f","line":4523,"in_reply_to":"45df85ee_6112318b","updated":"2024-02-28 14:49:37.000000000","message":"Done","commit_id":"0f14ec3b2f5413084a32818dae4c8fa32ffa7041"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"a4e9f12ff31659e0d2963462e8fb45f98ec06812","unresolved":true,"context_lines":[{"line_number":4520,"context_line":"                # Stash the passphrase itself in a libvirt secret using the"},{"line_number":4521,"context_line":"                # same UUID as the key manager secret for easy retrieval later"},{"line_number":4522,"context_line":"                secret_usage \u003d f\"{instance.uuid}_{driver_bdm[\u0027uuid\u0027]}\""},{"line_number":4523,"context_line":"                if self._host.find_secret(\u0027volume\u0027, secret_usage) is None:"},{"line_number":4524,"context_line":"                    self._host.create_secret("},{"line_number":4525,"context_line":"                        \u0027volume\u0027, secret_usage, password\u003dsecret,"},{"line_number":4526,"context_line":"                        uuid\u003dsecret_uuid)"}],"source_content_type":"text/x-python","patch_set":35,"id":"45df85ee_6112318b","line":4523,"in_reply_to":"54a2b42c_608ef6e7","updated":"2024-02-27 14:35:01.000000000","message":"\u003e im ok with recreating the secret if we are spaning on a host for the first time but we should not do this for hard reboot / start\n\nRight, this is at the top of spawn() which is only when we\u0027re putting an instance on a host for the first time or bringing it from another host), which is why it seems like a much safer thing to be explicit about the secret being right.\n\n\u003e we should be able to start a vm with encrypted storage if Barbican is unavailable using the locally cached secrete\n\u003e \n\u003e we added code for encrypted volumes to ensure the admin could restart the VM provided the secret was still present.\n\nYep, but this is not that.\n\n\u003e In the evacuation case the secrets of this host should be cleaned up as part of agent start via init_host. i dont think we should be able to get into a case where the vm was prviously on the host, moved and came back to have a currpted key but if we want to validate the content to delete  and recreate it to be paranoid then ok we can.\n\nThe admin trying to recover after a host crash, botched evacuation, etc are easy ways that could happen. Bugs and changes to all of this in the future are also possibilities.","commit_id":"0f14ec3b2f5413084a32818dae4c8fa32ffa7041"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"73e5d30d14ed3fa6f04b84e8c4bbd84b81e0b51f","unresolved":true,"context_lines":[{"line_number":4520,"context_line":"                # Stash the passphrase itself in a libvirt secret using the"},{"line_number":4521,"context_line":"                # same UUID as the key manager secret for easy retrieval later"},{"line_number":4522,"context_line":"                secret_usage \u003d f\"{instance.uuid}_{driver_bdm[\u0027uuid\u0027]}\""},{"line_number":4523,"context_line":"                if self._host.find_secret(\u0027volume\u0027, secret_usage) is None:"},{"line_number":4524,"context_line":"                    self._host.create_secret("},{"line_number":4525,"context_line":"                        \u0027volume\u0027, secret_usage, password\u003dsecret,"},{"line_number":4526,"context_line":"                        uuid\u003dsecret_uuid)"}],"source_content_type":"text/x-python","patch_set":35,"id":"54a2b42c_608ef6e7","line":4523,"in_reply_to":"5a9ccaf3_2672973c","updated":"2024-02-27 09:15:08.000000000","message":"im ok with recreating the secret if we are spaning on a host for the first time but we should not do this for hard reboot / start\n\nwe should be able to start a vm with encrypted storage if Barbican is unavailable using the locally cached secrete\n\nwe added code for encrypted volumes to ensure the admin could restart the VM provided the secret was still present.\n\nIn the evacuation case the secrets of this host should be cleaned up as part of agent start via init_host. i dont think we should be able to get into a case where the vm was prviously on the host, moved and came back to have a currpted key but if we want to validate the content to delete  and recreate it to be paranoid then ok we can.","commit_id":"0f14ec3b2f5413084a32818dae4c8fa32ffa7041"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"acab46f7c8433db775e6fd2f40a38d57b629bf5e","unresolved":true,"context_lines":[{"line_number":4520,"context_line":"                # Stash the passphrase itself in a libvirt secret using the"},{"line_number":4521,"context_line":"                # same UUID as the key manager secret for easy retrieval later"},{"line_number":4522,"context_line":"                secret_usage \u003d f\"{instance.uuid}_{driver_bdm[\u0027uuid\u0027]}\""},{"line_number":4523,"context_line":"                if self._host.find_secret(\u0027volume\u0027, secret_usage) is None:"},{"line_number":4524,"context_line":"                    self._host.create_secret("},{"line_number":4525,"context_line":"                        \u0027volume\u0027, secret_usage, password\u003dsecret,"},{"line_number":4526,"context_line":"                        uuid\u003dsecret_uuid)"}],"source_content_type":"text/x-python","patch_set":35,"id":"e5635ce7_ecc83012","line":4523,"in_reply_to":"db0f7449_cf1bdd0a","updated":"2024-02-26 19:31:27.000000000","message":"Yeah, I guess I\u0027m concerned about the silent corruption as much as anything. If/since we never delete a key from barbican then actual loss should be minimal, but if we boot with the wrong key it\u0027ll be ensured. Seems to me like it would be safer to *always* set the key in libvirt even if we need to delete first. Or, always grab it and compare it to the one in libvirt, both name and value. If the name is present but the value is different, then abort mission, log an error, most conservative approach.u","commit_id":"0f14ec3b2f5413084a32818dae4c8fa32ffa7041"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"23c920124bba554dafc37d09c681937afc6929c3","unresolved":true,"context_lines":[{"line_number":4520,"context_line":"                # Stash the passphrase itself in a libvirt secret using the"},{"line_number":4521,"context_line":"                # same UUID as the key manager secret for easy retrieval later"},{"line_number":4522,"context_line":"                secret_usage \u003d f\"{instance.uuid}_{driver_bdm[\u0027uuid\u0027]}\""},{"line_number":4523,"context_line":"                if self._host.find_secret(\u0027volume\u0027, secret_usage) is None:"},{"line_number":4524,"context_line":"                    self._host.create_secret("},{"line_number":4525,"context_line":"                        \u0027volume\u0027, secret_usage, password\u003dsecret,"},{"line_number":4526,"context_line":"                        uuid\u003dsecret_uuid)"}],"source_content_type":"text/x-python","patch_set":35,"id":"db0f7449_cf1bdd0a","line":4523,"in_reply_to":"dc068813_ac107434","updated":"2024-02-26 19:16:37.000000000","message":"Yeah.. in the cases of shelve/unshelve and rebuild, the same secret is used (i.e. I\u0027m not deleting secrets in barbican until the instance is deleted). So it would be correct even after that sequence. With regard to instance actions that could potentially (by policy) allow users in different projects to shelve/unshelve or rebuild, if we changed the secret, the secret would become owned by someone other than the owner of the instance, and I was trying to avoid that.\n\nThe other reason is because I have been paranoid about the prospect of losing a secret and that resulting in effective data loss, so I was really thinking the barbican secrets should be tied to the instance lifecycle. So that when we delete the barbican secrets there is no possible way that we could be destroying something still being used.\n\nRe: the tests, I\u0027ll look and add where needed.","commit_id":"0f14ec3b2f5413084a32818dae4c8fa32ffa7041"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"50e07e3b6571ff7f90be05ddb838bc0c28ef3edd","unresolved":true,"context_lines":[{"line_number":4520,"context_line":"                # Stash the passphrase itself in a libvirt secret using the"},{"line_number":4521,"context_line":"                # same UUID as the key manager secret for easy retrieval later"},{"line_number":4522,"context_line":"                secret_usage \u003d f\"{instance.uuid}_{driver_bdm[\u0027uuid\u0027]}\""},{"line_number":4523,"context_line":"                if self._host.find_secret(\u0027volume\u0027, secret_usage) is None:"},{"line_number":4524,"context_line":"                    self._host.create_secret("},{"line_number":4525,"context_line":"                        \u0027volume\u0027, secret_usage, password\u003dsecret,"},{"line_number":4526,"context_line":"                        uuid\u003dsecret_uuid)"}],"source_content_type":"text/x-python","patch_set":35,"id":"20a8744a_c49e8c53","line":4523,"in_reply_to":"e5635ce7_ecc83012","updated":"2024-02-26 20:02:59.000000000","message":"The libvirt keys are identified by \u003cinstance uuid\u003e_\u003cbdm uuid\u003e and that really is unique to a specific disk on a specific instance. So I guess I don\u0027t see how you could possibly pick up the wrong key. We don\u0027t ever query for libvirt secrets using the encryption_secret_uuid.\n\nAlso, at instance create time, that\u0027s the only time driver_bdm[\u0027encryption_secret_uuid\u0027] is ever set. It remains static for the remainder of the instance\u0027s life. I had been thinking it would be safest that way.\n\nI\u0027m not opposed to being more paranoid and conservative, if I have missed something here. My goal in this series is to be paranoid.","commit_id":"0f14ec3b2f5413084a32818dae4c8fa32ffa7041"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"a47844f2799f0c32fb3203487565235861db6504","unresolved":true,"context_lines":[{"line_number":4531,"context_line":"                    crypto.delete_encryption_secret("},{"line_number":4532,"context_line":"                        context, instance.uuid, secret_uuid)"},{"line_number":4533,"context_line":"                except Exception:"},{"line_number":4534,"context_line":"                    LOG.error("},{"line_number":4535,"context_line":"                        f\u0027Failed to delete encryption secret \u0027"},{"line_number":4536,"context_line":"                        f\u0027{secret_uuid} in the key manager for driver BDM \u0027"},{"line_number":4537,"context_line":"                        f\"{driver_bdm[\u0027uuid\u0027]}\", instance\u003dinstance)"}],"source_content_type":"text/x-python","patch_set":35,"id":"51a0fb13_837e2b0d","line":4534,"updated":"2024-02-26 17:54:43.000000000","message":"I don\u0027t think you\u0027re covering this case in the tests.","commit_id":"0f14ec3b2f5413084a32818dae4c8fa32ffa7041"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"23c920124bba554dafc37d09c681937afc6929c3","unresolved":true,"context_lines":[{"line_number":4531,"context_line":"                    crypto.delete_encryption_secret("},{"line_number":4532,"context_line":"                        context, instance.uuid, secret_uuid)"},{"line_number":4533,"context_line":"                except Exception:"},{"line_number":4534,"context_line":"                    LOG.error("},{"line_number":4535,"context_line":"                        f\u0027Failed to delete encryption secret \u0027"},{"line_number":4536,"context_line":"                        f\u0027{secret_uuid} in the key manager for driver BDM \u0027"},{"line_number":4537,"context_line":"                        f\"{driver_bdm[\u0027uuid\u0027]}\", instance\u003dinstance)"}],"source_content_type":"text/x-python","patch_set":35,"id":"a1c9340d_90a5b50a","line":4534,"in_reply_to":"51a0fb13_837e2b0d","updated":"2024-02-26 19:16:37.000000000","message":"Ack, I\u0027ll look and add where needed.","commit_id":"0f14ec3b2f5413084a32818dae4c8fa32ffa7041"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"4ecf161680f76a6a5a3c202571406681df8a1b9b","unresolved":false,"context_lines":[{"line_number":4531,"context_line":"                    crypto.delete_encryption_secret("},{"line_number":4532,"context_line":"                        context, instance.uuid, secret_uuid)"},{"line_number":4533,"context_line":"                except Exception:"},{"line_number":4534,"context_line":"                    LOG.error("},{"line_number":4535,"context_line":"                        f\u0027Failed to delete encryption secret \u0027"},{"line_number":4536,"context_line":"                        f\u0027{secret_uuid} in the key manager for driver BDM \u0027"},{"line_number":4537,"context_line":"                        f\"{driver_bdm[\u0027uuid\u0027]}\", instance\u003dinstance)"}],"source_content_type":"text/x-python","patch_set":35,"id":"6390b57c_54f416d1","line":4534,"in_reply_to":"a1c9340d_90a5b50a","updated":"2024-02-28 14:49:37.000000000","message":"Done","commit_id":"0f14ec3b2f5413084a32818dae4c8fa32ffa7041"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"a47844f2799f0c32fb3203487565235861db6504","unresolved":true,"context_lines":[{"line_number":4547,"context_line":"                    if self._host.find_secret(\u0027volume\u0027, secret_usage):"},{"line_number":4548,"context_line":"                        self._host.delete_secret(\u0027volume\u0027, secret_usage)"},{"line_number":4549,"context_line":"                except Exception:"},{"line_number":4550,"context_line":"                    LOG.error("},{"line_number":4551,"context_line":"                        f\u0027Failed to delete libvirt secret {secret_usage}\u0027,"},{"line_number":4552,"context_line":"                        instance\u003dinstance)"},{"line_number":4553,"context_line":"            raise"}],"source_content_type":"text/x-python","patch_set":35,"id":"19fe6362_2a0b314a","line":4550,"updated":"2024-02-26 17:54:43.000000000","message":"Nor this one.","commit_id":"0f14ec3b2f5413084a32818dae4c8fa32ffa7041"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"23c920124bba554dafc37d09c681937afc6929c3","unresolved":true,"context_lines":[{"line_number":4547,"context_line":"                    if self._host.find_secret(\u0027volume\u0027, secret_usage):"},{"line_number":4548,"context_line":"                        self._host.delete_secret(\u0027volume\u0027, secret_usage)"},{"line_number":4549,"context_line":"                except Exception:"},{"line_number":4550,"context_line":"                    LOG.error("},{"line_number":4551,"context_line":"                        f\u0027Failed to delete libvirt secret {secret_usage}\u0027,"},{"line_number":4552,"context_line":"                        instance\u003dinstance)"},{"line_number":4553,"context_line":"            raise"}],"source_content_type":"text/x-python","patch_set":35,"id":"b81d4e54_46a8a177","line":4550,"in_reply_to":"19fe6362_2a0b314a","updated":"2024-02-26 19:16:37.000000000","message":"Ditto.","commit_id":"0f14ec3b2f5413084a32818dae4c8fa32ffa7041"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"4ecf161680f76a6a5a3c202571406681df8a1b9b","unresolved":false,"context_lines":[{"line_number":4547,"context_line":"                    if self._host.find_secret(\u0027volume\u0027, secret_usage):"},{"line_number":4548,"context_line":"                        self._host.delete_secret(\u0027volume\u0027, secret_usage)"},{"line_number":4549,"context_line":"                except Exception:"},{"line_number":4550,"context_line":"                    LOG.error("},{"line_number":4551,"context_line":"                        f\u0027Failed to delete libvirt secret {secret_usage}\u0027,"},{"line_number":4552,"context_line":"                        instance\u003dinstance)"},{"line_number":4553,"context_line":"            raise"}],"source_content_type":"text/x-python","patch_set":35,"id":"ca475c0d_c88ebf1c","line":4550,"in_reply_to":"b81d4e54_46a8a177","updated":"2024-02-28 14:49:37.000000000","message":"Done","commit_id":"0f14ec3b2f5413084a32818dae4c8fa32ffa7041"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"4ecf161680f76a6a5a3c202571406681df8a1b9b","unresolved":false,"context_lines":[{"line_number":1770,"context_line":""},{"line_number":1771,"context_line":"        if exception_msgs:"},{"line_number":1772,"context_line":"            msg \u003d \u0027\\n\u0027.join(exception_msgs)"},{"line_number":1773,"context_line":"            raise exception.EphemeralEncryptionCleanupFailed(error\u003dmsg)"},{"line_number":1774,"context_line":""},{"line_number":1775,"context_line":"    def cleanup_lingering_instance_resources(self, instance):"},{"line_number":1776,"context_line":"        # zero the data on backend pmem device, if fails"}],"source_content_type":"text/x-python","patch_set":36,"id":"9c138e18_b21430aa","line":1773,"updated":"2024-02-28 14:49:37.000000000","message":"I think this is better, thanks.","commit_id":"177c184e405f4227b430151b076f0eaa6efff1a7"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"4ecf161680f76a6a5a3c202571406681df8a1b9b","unresolved":true,"context_lines":[{"line_number":4559,"context_line":"                # secret to ensure we are creating the secret we retrieved or"},{"line_number":4560,"context_line":"                # created in the key manager just now."},{"line_number":4561,"context_line":"                if self._host.find_secret(\u0027volume\u0027, secret_usage):"},{"line_number":4562,"context_line":"                    self._host.delete_secret(\u0027volume\u0027, secret_usage)"},{"line_number":4563,"context_line":"                self._host.create_secret("},{"line_number":4564,"context_line":"                    \u0027volume\u0027, secret_usage, password\u003dsecret, uuid\u003dsecret_uuid)"},{"line_number":4565,"context_line":"                created_libvirt_secrets.append(secret_usage)"}],"source_content_type":"text/x-python","patch_set":36,"id":"83ce058a_ab6cb976","line":4562,"updated":"2024-02-28 14:49:37.000000000","message":"Might be worth a log for forensics. Both from a \"nova keeps leaving secrets on hosts because I keep seeing this message\" and also just general \"what happened here\" stuff. But we can put that in a cleanup patch or something.","commit_id":"177c184e405f4227b430151b076f0eaa6efff1a7"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"8b00e227df55a0dab45d4f9493df1aa191b46b18","unresolved":true,"context_lines":[{"line_number":4559,"context_line":"                # secret to ensure we are creating the secret we retrieved or"},{"line_number":4560,"context_line":"                # created in the key manager just now."},{"line_number":4561,"context_line":"                if self._host.find_secret(\u0027volume\u0027, secret_usage):"},{"line_number":4562,"context_line":"                    self._host.delete_secret(\u0027volume\u0027, secret_usage)"},{"line_number":4563,"context_line":"                self._host.create_secret("},{"line_number":4564,"context_line":"                    \u0027volume\u0027, secret_usage, password\u003dsecret, uuid\u003dsecret_uuid)"},{"line_number":4565,"context_line":"                created_libvirt_secrets.append(secret_usage)"}],"source_content_type":"text/x-python","patch_set":36,"id":"b6cd7cdd_dbbd8f20","line":4562,"in_reply_to":"83ce058a_ab6cb976","updated":"2024-02-28 20:18:45.000000000","message":"Sure, I\u0027ll put that on the followup list.","commit_id":"177c184e405f4227b430151b076f0eaa6efff1a7"}]}