)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":14394,"name":"Dale Smith","email":"dale@catalystcloud.nz","username":"dalees"},"change_message_id":"b613825c8e243ad56ee867167a562e8a15b51d7c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"c9a440e9_ae1388fc","updated":"2024-01-09 04:11:05.000000000","message":"Discussion, and tested by Nguyễn Hữu Khôi in this thread: https://kubernetes.slack.com/archives/C05Q8TDTK6Z/p1704770136965449?thread_ts\u003d1704379624.271879\u0026cid\u003dC05Q8TDTK6Z","commit_id":"51710beaf0c8d6d8989efe548c0c24a129e7e638"},{"author":{"_account_id":8064,"name":"Jake Yip","email":"jake.yip@ardc.edu.au","username":"jake"},"change_message_id":"e29127ccd4f2798d21abf3aef13d711f587ffe4c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"1216cce5_4f9e8b3e","updated":"2024-02-19 14:58:16.000000000","message":"LGTM, do you know when trust/trustee are not deleted? I\u0027m curious under what circumstances we need this.","commit_id":"51710beaf0c8d6d8989efe548c0c24a129e7e638"},{"author":{"_account_id":14394,"name":"Dale Smith","email":"dale@catalystcloud.nz","username":"dalees"},"change_message_id":"03aea8f3df45dff8fce32ff61667ba54d9da5d22","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"c8076cf1_10a63100","in_reply_to":"1216cce5_4f9e8b3e","updated":"2024-02-22 02:38:32.000000000","message":"So with the Heat driver, we do see leaking Trusts in production but I\u0027m not sure in what situations this arises. I\u0027m hoping this patchset to also address this, and it is safe to call trust_manager.delete_trustee_and_trust multiple times.\n\nTrust creation is always performed in cluster_conductor.py cluster_create() function.\n\nIn the Heat driver, the codepath that normally handles trust deletion is:\n  `update_status(magnum/service/periodic.py)-\u003e`\n  `update_cluster_status(in driver.py)-\u003e`\n  `poll_and_check-\u003e`\n  `extract_nodegroup_status-\u003e`\n  `_check_delete_complete-\u003e`\n  `_delete_complete-\u003e`\n  `trust_manager.delete_trustee_and_trust`\n\nIn this situation, the conductor creates the trusts (prior to even looking up the driver), and then the Heat driver `update_status` method ends up deleting the trusts. A bit of encapsulation mismatch there, where a driver is expected to clean up after the conductor.\n\nWhen it comes to both Cluster API drivers(StackHPC and VEXXHOST), the driver function `update_cluster_status` cleans up resources that belong to the driver, and sets the cluster status to DELETE_COMPLETE. This means the trusts reliably leak, as Heat driver previously took care of deletion.\n\nThis patch keeps the lifecycle of the Trusts in the conductor, and reliably attempts deletion on them after DELETE_COMPLETE state is observed.\n\nI will update the commit message with a bit more info on this.","commit_id":"51710beaf0c8d6d8989efe548c0c24a129e7e638"},{"author":{"_account_id":8064,"name":"Jake Yip","email":"jake.yip@ardc.edu.au","username":"jake"},"change_message_id":"dbcb4e39d261c3a37f1cd62e271ddd0e63c4317f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"c4d27481_4d9e6942","updated":"2024-02-27 14:11:53.000000000","message":"LGTM. I agree it\u0027s hard to replicate conditions where trusts are left behind, but this passes test so good to go. Hopefully it helps.","commit_id":"1b00074c6a4851769c8b4faa9a36df94aab88204"}],"magnum/tests/unit/conductor/handlers/common/test_trust_manager.py":[{"author":{"_account_id":22629,"name":"Michal Nasiadka","email":"mnasiadka@gmail.com","username":"mnasiadka"},"change_message_id":"f93aca9e8955d5e1cc8bd725529b6b2d3211dd22","unresolved":true,"context_lines":[{"line_number":89,"context_line":"            context, mock_cluster"},{"line_number":90,"context_line":"        )"},{"line_number":91,"context_line":"        mock_keystone.delete_trustee.assert_called_once_with("},{"line_number":92,"context_line":"            \u0027trustee_user_id\u0027,"},{"line_number":93,"context_line":"        )"},{"line_number":94,"context_line":""},{"line_number":95,"context_line":"    def test_delete_trustee_and_trust_without_trust_id(self):"}],"source_content_type":"text/x-python","patch_set":2,"id":"9df06b8f_eabf762d","line":92,"range":{"start_line":92,"start_character":12,"end_line":92,"end_character":29},"updated":"2024-01-09 07:16:03.000000000","message":"what was wrong with mock_cluster.trustee_user_id?","commit_id":"51710beaf0c8d6d8989efe548c0c24a129e7e638"},{"author":{"_account_id":22629,"name":"Michal Nasiadka","email":"mnasiadka@gmail.com","username":"mnasiadka"},"change_message_id":"0aec461c8b5794e274287c7dfed6d0fbaf5380b5","unresolved":false,"context_lines":[{"line_number":89,"context_line":"            context, mock_cluster"},{"line_number":90,"context_line":"        )"},{"line_number":91,"context_line":"        mock_keystone.delete_trustee.assert_called_once_with("},{"line_number":92,"context_line":"            \u0027trustee_user_id\u0027,"},{"line_number":93,"context_line":"        )"},{"line_number":94,"context_line":""},{"line_number":95,"context_line":"    def test_delete_trustee_and_trust_without_trust_id(self):"}],"source_content_type":"text/x-python","patch_set":2,"id":"25c3a669_fece812f","line":92,"range":{"start_line":92,"start_character":12,"end_line":92,"end_character":29},"in_reply_to":"4f2de762_400d9c6f","updated":"2024-01-11 15:25:39.000000000","message":"Acknowledged","commit_id":"51710beaf0c8d6d8989efe548c0c24a129e7e638"},{"author":{"_account_id":14394,"name":"Dale Smith","email":"dale@catalystcloud.nz","username":"dalees"},"change_message_id":"6d3f8e718245dc89f5f46159716b64ce96e7fc0a","unresolved":true,"context_lines":[{"line_number":89,"context_line":"            context, mock_cluster"},{"line_number":90,"context_line":"        )"},{"line_number":91,"context_line":"        mock_keystone.delete_trustee.assert_called_once_with("},{"line_number":92,"context_line":"            \u0027trustee_user_id\u0027,"},{"line_number":93,"context_line":"        )"},{"line_number":94,"context_line":""},{"line_number":95,"context_line":"    def test_delete_trustee_and_trust_without_trust_id(self):"}],"source_content_type":"text/x-python","patch_set":2,"id":"4f2de762_400d9c6f","line":92,"range":{"start_line":92,"start_character":12,"end_line":92,"end_character":29},"in_reply_to":"9df06b8f_eabf762d","updated":"2024-01-09 21:36:46.000000000","message":"mock_cluster.trustee_user_id belongs to the cluster object, and can be mutated by the function under test. This is now set to None when cleaned up, as the trustee uuid no longer exists.\n\nThis test really wants to ensure it was called with the expected \u0027trustee_user_id\u0027 string, not whatever is set on the cluster at the time this line executes.","commit_id":"51710beaf0c8d6d8989efe548c0c24a129e7e638"}]}